Skip to content

removed p3 as dep#52

Merged
illuzen merged 3 commits into
mainfrom
illuzen/remove-p3
Jun 4, 2026
Merged

removed p3 as dep#52
illuzen merged 3 commits into
mainfrom
illuzen/remove-p3

Conversation

@illuzen

@illuzen illuzen commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

@n13

n13 commented Jun 4, 2026

Copy link
Copy Markdown

Review complete — worktree cleaned up, your working tree is untouched. Here's my assessment of PR #52 "removed p3 as dep".

Verdict: Looks good to merge ✅

This is a clean, well-scoped dependency consolidation. It removes the entire Plonky3 (p3-*) stack plus the qp-poseidon-constants crate and replaces them with the self-contained qp-poseidon-core 2.1.0. Crucially, it preserves hash output byte-for-byte, which is the make-or-break property for a consensus-critical hash.

What it does

  • Drops p3-field, p3-goldilocks, p3-poseidon2, p3-symmetric, and qp-poseidon-constants from core, plonky2, and verifier.
  • Bumps qp-poseidon-core 1.3.0 → 2.1.0 and sources all Poseidon2 constants from it (qp_poseidon_core::poseidon2::{INITIAL_EXTERNAL_CONSTANTS, TERMINAL_EXTERNAL_CONSTANTS, INTERNAL_CONSTANTS, MATRIX_DIAG, ...}).
  • Swaps the runtime permutation from p3's Poseidon2Goldilocks<12> to qp_poseidon_core::Poseidon2.
  • Net +87 / −367, and prunes ~16 transitive crates (p3-dft/matrix/mds/util, tracing, transpose, paste, duplicate rand 0.9/rand_chacha 0.9, etc.) from Cargo.lock.

Determinism — the critical check (verified)

Every test in this repo is a self-consistency check (e.g. poseidon2_hash_matches_qp_poseidon compares plonky2 against qp-poseidon-core itself), so they'd all pass even if 2.1.0 silently changed constants. I verified the real invariant out-of-band:

  1. Constants are byte-identical. I diffed the old qp-poseidon-constants-1.1.0 against qp-poseidon-core-2.1.0 in the cargo registry: same seed 0x3141592653589793, same 22 internal constants, same 12 diagonal values, same 4×12 initial/terminal arrays. Only the names changed (*_RAW → unsuffixed).
  2. The dependency pins itself to p3. qp-poseidon-core 2.1.0 ships its own KATs that lock its hand-written permutation to the old p3 output: test_poseidon2_matches_p3_zero_input/_sequential_input (full P3_ZERO_RESULT/P3_SEQ_RESULT vectors), per-layer S-box/MDS/linear-layer vectors, plus 18 pinned hash_bytes vectors and hash_twice/rehash vectors.
  3. The plonky2 in-circuit gate is unchanged functionally — only constant identifiers were renamed; the constraint arithmetic is untouched.

So transitively: plonky2 gate ≡ qp-poseidon-core permute ≡ p3 ≡ historical chain values.

Build & tests (in throwaway worktree)

  • cargo build --workspace → clean, qp-poseidon-core v2.1.0 resolves from crates.io (the qp-poseidon#77 dependency is already published/released).
  • cargo test -p qp-plonky2 poseidon2 → 10 passed (incl. cross-crate gate tests).
  • cargo test -p qp-plonky2-verifier poseidon2 → 2 passed.
  • cargo fmt --all --check → clean.
  • no_std safe: qp-poseidon-core 2.1.0 is #![no_std] with an empty [dependencies] block, so the no_std verifier is fine.
  • Removing the deps from core/Cargo.toml is correct — qp-plonky2-core's source references none of them (they were dead dependencies).

Minor nits (non-blocking)

  • Stale p3 naming/comments. Poseidon2Params::from_p3_constants_u64(...) and two doc comments (/// ## Structure (matches p3-goldilocks)) still reference p3. They're technically accurate (constants originate from p3's seed), but renaming would tidy the "remove p3" intent.
  • Test oracle is now same-source. The pre-PR tests compared plonky2 against an independent p3-based oracle; now the oracle is the same qp-poseidon-core 2.1.0 the prod code uses. It still validates the in-circuit gate vs. the native permutation (worthwhile), and absolute correctness is covered by the dependency's KATs — but the repo no longer independently pins output to historical values. Optional hardening: add a couple of hardcoded Poseidon2Hash output vectors to this repo's tests so a future qp-poseidon-core bump can't silently shift output.
  • No crate version bumps for qp-plonky2*. Output is unchanged so it's not semver-breaking for consumers, but worth confirming this fits the release flow.

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@illuzen illuzen merged commit 378261a into main Jun 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants