[illuzen]

• removed substrate crate
• removed various unused functions

---

Note

High Risk
Removing hash_for_circuit, PoseidonHasher, and the qp-poseidon crate is a breaking change for any consumer still on Substrate or fixed-size padded hashes; release workflow changes affect how versions are bumped and what gets published.

Overview
This PR narrows the repo to qp-poseidon-core only and drops the Substrate-facing qp-poseidon crate (substrate/ sources and workspace membership). Release automation now locks and updates only qp-poseidon-core in Cargo.lock, and publishes just that crate to crates.io (no wait/publish step for qp-poseidon).

The core public API is simplified around injective hash_bytes / field-element hashing: circuit-padded helpers (hash_for_circuit, hash_felts_for_circuit, pad_and_hash, poseidon2_from_seed) and proof-node sizing constants are removed. Tests, benchmarks, and constant-time benches are updated accordingly; KAT vectors now cover hash_bytes only, with added stability tests for hash_twice and rehash_to_bytes. Cargo.lock shrinks sharply after shedding Substrate/sp-* dependencies.

Docs and tooling reflect the slimmer surface: root and core READMEs describe the core crate only, and clippy.sh runs workspace clippy like CI.

^{Reviewed by Cursor Bugbot for commit 88607de. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 14e50f4. Configure here.}

[n13]

Summary verdict

The PR is mostly a clean simplification — pruning the qp-poseidon Substrate wrapper, removing padded hash_for_circuit variants, and centering the API on hash_bytes/hash_twice/rehash_to_bytes. Build, tests, and CI's clippy --workspace --all-targets --all-features --locked -- -D warnings all pass on the latest commit.

The biggest soundness concern is the newly introduced hash_pubkey (and its hash_compact core), which I'll cover first.

---

1. hash_pubkey / hash_compact — the only real soundness ask

/// Hash a Dilithium ML-DSA-87 public key (2592 bytes) using compact encoding.
///
/// Uses compact encoding (8 bytes/felt) for circuit compatibility.
/// This is the recommended way to derive an account ID from a Dilithium public key.
///
/// # Safety
/// This function uses compact encoding which is only collision-resistant for fixed-size
/// inputs. The fixed array size ensures callers provide exactly 2592 bytes.
pub fn hash_pubkey(pubkey: &[u8; 2592]) -> [u8; 32] {
	hash_compact(pubkey)
}

The doc comment correctly highlights two ways bytes_to_felts_compact is non-injective:

1. No length marker — trailing zeros collide.
2. Field reduction — Goldilocks::from_int(u64) reduces values in [p, 2^64) (where p = 2^64 − 2^32 + 1) into [0, 2^32 − 1), so v and v − p map to the same felt.

Fixing the array size to 2592 bytes fully neutralizes (1). For (2), the per-8-byte-chunk probability that a uniformly random chunk has a "twin" u64 mapping to the same felt is (2^32 − 1) / 2^64 ≈ 2^-32. With 324 chunks per pubkey, the cumulative probability that any chunk is "twinnable" is ≈ 324 · 2^-32 ≈ 2^-23.4. But for an actual collision between two distinct pubkeys, both must independently have that exact byte pattern AND differ exactly there, which drops back to ≈ (per-chunk twin prob)^2 = 2^-96 per shared chunk position. So Poseidon2's birthday bound (~2^-128) still dominates the collision probability, and the doc warning is honest about the residual risk. This is acceptable as long as hash_pubkey is genuinely only ever called on Dilithium pubkeys — which the &[u8; 2592] type signature enforces at the API surface.

Concrete soundness asks:

• Justify why the chain needs hash_pubkey at all, given that chain/primitives/dilithium-crypto/src/traits.rs currently derives account IDs via hash_bytes(pubkey) (the injective 4 bytes/felt + terminator path), and chain PR #571 explicitly says it "shouldn't change any hashes". Right now hash_pubkey is completely unused in the chain repo — searching the whole chain/ tree for hash_pubkey returns nothing. Adding a less-sound primitive with a doc comment that calls it "the recommended way to derive an account ID from a Dilithium public key" is a footgun if no one actually flips the switch — the next PR author may grep, find this doc, and silently break account-ID continuity. Either:
  • Wire it up now in the same release (with explicit migration tests showing what changes), or
  • Drop the "recommended" wording until the chain is actually ready to migrate.
• Add a KAT vector for hash_pubkey. Since this is now a public, named primitive, the new KAT block (which adds vectors for hash_twice / rehash_to_bytes) should also pin at least one fixed [u8; 2592] → 32-byte answer. That's the only way circuits and runtime can be guaranteed to agree across versions; otherwise a future change to bytes_to_felts_compact, digest_to_bytes, or even Goldilocks reduction lands silently.
• Optional but worth considering: domain separation. hash_pubkey and hash_bytes write into the same sponge with the same IV and same padding rule — they only differ by encoding. If the chain ever ends up calling both on Dilithium-shaped data (e.g. hash_bytes(pubkey) for legacy accounts and hash_pubkey(pubkey) for new ones), having domain separation (e.g. an absorbed length felt or a tag felt) would prevent any cross-protocol confusion. Not strictly necessary for collision resistance given the type-fixed input, but cheap insurance.

2. hash_for_circuit / hash_felts_for_circuit removal

These were the padded compact-encoding hashes (pad_and_hash::<C>) used by:

• The Substrate Hasher::hash impl in substrate/src/lib.rs (state-trie hashing).
• A fallback path in chain/primitives/header/src/lib.rs::hash_header when header decoding fails.

Soundness implications:

• The substrate state-trie hasher is being abandoned in favor of BlakeTwo256 (per chain PR #571). That's a much more conservative trie hasher — Poseidon-as-trie-hasher had the awkward property that its preimage was the SCALE-encoded trie node (uninterpreted bytes), which is exactly the variable-length compact-encoding case where hash_for_circuit was not injective unless you trusted the size cap C = PROOF_NODE_MAX_SIZE_FELTS = 80. Switching the trie to BlakeTwo256 is a soundness improvement, not a regression.
• The hash_for_circuit fallback in hash_header was unreachable under normal operation (the if-branch always succeeds for valid encoded headers); it should also disappear in chain PR #571. Confirm there's no remaining code path that reaches it on real data, otherwise removing hash_for_circuit will hard-error those code sites.
• Block hashing is preserved: per chain PR #571's description, qp_header will compute the Poseidon block hash internally using qp_poseidon_core directly. Since the block-hash code path uses hash_to_bytes(felts) over an explicitly-laid-out felt vector (with bytes_to_digest / bytes_to_felts over a fixed-size padded digest_padded), this is injective and the hash is unchanged. Good.

Ask: the PR description should explicitly state "this MUST land together with chain PR #571" — at the moment qp-poseidon main and chain main would not compile together once this lands and the new core 1.6.0 (or whatever bump) is published.

3. hash_twice_to_felts removal

-pub fn hash_twice_to_felts(x: &[Goldilocks]) -> [Goldilocks; POSEIDON2_OUTPUT] {
-	let inner = hash_to_felts(x);
-	hash_to_felts(&inner)
-}
 pub fn hash_twice(x: &[Goldilocks]) -> [u8; 32] {
-	serialization::digest_to_bytes(&hash_twice_to_felts(x))
+	let inner = hash_to_felts(x);
+	hash_to_bytes(&inner)
}

Soundness-equivalent: hash_to_bytes(&hash_to_felts(x)) == digest_to_bytes(&hash_to_felts(hash_to_felts(x))), because hash_to_bytes(felts) runs the sponge over felts (no extra encoding) and the inner hash output is already field-aligned. Output bytes are identical to the old path. The new KAT vectors test_hash_twice_vectors lock this in. No regression.

The removal of hash_twice_to_felts is a small public-API loss for any out-of-tree caller that wants the felt output for in-circuit chaining without re-decoding 32 bytes back to 4 felts. Trivial to reconstruct (hash_to_felts(&hash_to_felts(x))), and the chain repo doesn't use it, so OK.

4. poseidon2_from_seed, PROOF_NODE_MAX_SIZE_*, COMPACT_BYTES_PER_FELT removal

poseidon2_from_seed produced a non-canonical Poseidon2 instance from a ChaCha20 seed; nothing in the chain workspace used it (grep confirms). Removing it means there's now exactly one Poseidon2 instance in the system (the one from qp_poseidon_constants::create_poseidon), which is a soundness win — there's no longer a foot-gun where a caller could accidentally hash with the wrong constants. The PR also removes the rand_chacha dep, which is a nice attack-surface reduction.

PROOF_NODE_MAX_SIZE_BYTES/PROOF_NODE_MAX_SIZE_FELTS/COMPACT_BYTES_PER_FELT were only meaningful when paired with hash_for_circuit::<PROOF_NODE_MAX_SIZE_FELTS>. With hash_for_circuit gone, they're dead. Fine to delete.

5. Outstanding bot comments on the PR

I went through the three Cursor Bugbot comments and verified them against the latest commit (0fb53cb):

#	Severity	Status	Notes
1	High	Already fixed in 0fb53cb	hash_compact was pub(crate) while imported in benches/poseidon_benchmarks.rs and bin/ct_bench.rs. The dedupe commit removed those imports — bench now uses hash_bytes, ct_bench uses * glob (which won't pick up pub(crate) items). I confirmed cargo build --all-features --all-targets passes.
2	Low	Still real	generate_test_vectors still prints tuples with both compact and bytes hashes ("\t({}, \"{}\", \"{}\"),"), but test_known_value_hashes now consumes (Vec<u8>, &str). The generator can't be copy-pasted into the consumer test without manual re-formatting. Trivial fix: drop the compact_hash column from the printer (or re-add a fixed-size compact KAT row, see (1) above).
3	Low	Already fixed in 0fb53cb	The redundant _hash_padded/_hash_variable are gone; the integrated CT test now exercises hash_bytes → rehash_to_bytes → hash_squeeze_twice, which is actually better (chains two distinct paths).

6. Other minor things I'd suggest

• The KAT comment block now reads /// hash_bytes uses injective encoding (4 bytes/felt with terminator). — good. Keep that comment, and add a parallel one near the hash_pubkey definition pointing to its KAT once added.
• core/Cargo.toml drops rand_chacha (good, consistent with poseidon2_from_seed removal). I'd also confirm the README in core/ no longer references hash_for_circuit / PoseidonCore::hash_for_circuit — line 19 in core/README.md still does (per earlier grep). Update or it'll become a stale-doc trap.
• CT-bench coverage: deleting all the constant-time tests for the compact / padded path is fine since those code paths are gone, but the new hash_pubkey should arguably grow its own CT test (single Class::Left = [0u8; 2592] vs Class::Right = random 2592 bytes) — it's the most security-sensitive of the new public functions and runs on every account creation.
• clippy.sh mirrors CI, harmless and useful. Fine.

7. Pre-merge checklist for soundness

If I were merging this, I'd want:

• hash_pubkey either gets used in the same release or its "recommended" doc wording is softened.
• Add a [u8; 2592] → 32-byte KAT for hash_pubkey.
• Confirm chain PR #571 lands together (the substrate crate removal hard-breaks chain main).
• Fix bot comment Refactored #2 (generator/consumer schema mismatch) — one-line fix.
• Update core/README.md to stop advertising hash_for_circuit.

Everything else looks clean. The mathematical/cryptographic core (Poseidon2State, sponge absorption, padding, double-hash, squeeze-twice) is byte-identical to main — I diffed it and the new hash_twice / rehash_to_bytes KAT vectors are sufficient to prove that.

Happy to draft the hash_pubkey KAT addition or the README/generator fix as a follow-up commit if you'd like.

[n13]

it looks like nobody is using hash_pubkey - maybe we can remove it?

[n13]

Re-review of PR #74

New commit since last round: 682fec1 "simplify further, remove unused" (only core/src/lib.rs, +25/-72).

What changed

The author took my biggest soundness concern head-on and removed it.

Removed:

• hash_pubkey(&[u8; 2592]) -> [u8; 32]
• hash_compact(x: &[u8]) -> [u8; 32] (was pub(crate))
• The bytes_to_felts_compact import from lib.rs
• The test_hash_bytes_vs_compact test
• The dual-column generator/consumer for compact KAT vectors

Rewired:

• All hash_compact(...) test calls now use hash_bytes(...) instead
• generate_test_vectors prints (Vec<u8>, &str) — matches test_known_value_hashes's consumer schema. Bot comment Refactored #2 is now resolved structurally, not just trivially.
• The "Test that hash_compact produces consistent results" block at the bottom of test_known_value_hashes is now testing hash_bytes instead (slightly redundant with the earlier loop, but harmless).

Soundness verdict

This is exactly the right call. All of the soundness asks I flagged last round are gone, because the API surface that introduced them is gone:

Last round's concern	Status
hash_pubkey uses non-injective compact encoding	Resolved — function removed
Field-reduction collisions on Goldilocks for u64 in [p, 2^64)	Resolved — no public hash function over raw bytes uses compact encoding anymore
"Recommended way to derive account ID" doc was a footgun while unused in chain	Resolved — doc gone, chain stays on hash_bytes(pubkey)
Need a KAT for hash_pubkey	Resolved — no hash_pubkey to lock down
Bot comment #2: generator/consumer schema mismatch	Resolved — both sides now (Vec<u8>, &str)
Bot comment #1: hash_compact visibility	Resolved earlier in 0fb53cb, doubly resolved now (function gone)
Bot comment #3: redundant _hash_padded in ct_bench	Resolved earlier in 0fb53cb

Crucially, bytes_to_felts_compact itself stays pub in core/src/serialization.rs — and chain's pallets/zk-tree/src/tree.rs still uses it to build its own felt vectors for zk-tree hashing:

	use qp_poseidon_core::serialization::{bytes_to_felts_compact, u64_to_felts};
	// ...
	felts.extend(bytes_to_felts_compact(to_bytes));
	// ...
	qp_poseidon_core::hash_to_bytes(&felts)

This is the correct shape: compact encoding stays as a low-level building block that callers compose explicitly (with full responsibility for fixed-length and prime-range assumptions), and no high-level wrapper hides those assumptions behind a misleading name.

Quick sanity check

I ran on the new head (682fec1):

• cargo build --all-features --all-targets — clean
• cargo test --all-features — 34 + 0 + 2 = 36 tests pass (one fewer than before because test_hash_bytes_vs_compact is removed)
• bash clippy.sh (i.e. cargo clippy --workspace --all-targets --all-features --locked -- -D warnings) — clean

The core hash primitives (Poseidon2State, sponge absorb, padding, double hash, squeeze-twice) are byte-identical to main. The new KAT vectors for hash_bytes / hash_twice / rehash_to_bytes pin that down.

Remaining nits (non-blocking)

1. Stale section header in core/src/lib.rs — there's a now-empty banner:

// ============================================================================
// Compact encoding hash functions
// ============================================================================

The next function under it is hash_twice, which has nothing to do with compact encoding. Delete the banner (or rename to "Hash chaining functions" to cover hash_twice + rehash_to_bytes).

2. core/README.md is significantly stale. It still claims PoseidonCore::hash_for_circuit, PoseidonCore::hash_no_pad_bytes, injective_bytes_to_felts, injective_string_to_felts, FIELD_ELEMENT_PREIMAGE_PADDING_LEN: usize = 160, and links to ../substrate. Most of those were already wrong before this PR, but every one of them now points to deleted code. Worth a 5-minute rewrite to list hash_bytes / hash_to_felts / hash_to_bytes / hash_twice / rehash_to_bytes / hash_squeeze_twice, and to drop the substrate cross-link.

3. Version is still 1.5.0 in Cargo.toml — this PR removes the entire qp-poseidon crate from the workspace and deletes several public functions from qp-poseidon-core (hash_for_circuit, hash_felts_for_circuit, hash_twice_to_felts, poseidon2_from_seed, Poseidon2State made private, plus the three public constants). That is a SemVer-major break, but create-release-proposal.yml will only auto-bump minor. Either:

   • Manually bump to 2.0.0 before publish, or
   • Accept this as "we don't promise SemVer in 1.x" and document it.

   Chain currently pins qp-poseidon-core = { version = "1.4.0", ... } (per chain/Cargo.toml), so the chain side has to bump to whatever this release becomes anyway. Worth getting right before crates.io receives it.

4. Coordinated landing with chain PR #571 — still required. The substrate crate removal hard-breaks chain/primitives/header/src/lib.rs::hash_header's fallback call to PoseidonHasher::hash_for_circuit(x) and the qp_poseidon::PoseidonHasher typedef in runtime/src/lib.rs and the wormhole mock. PR #571's description says "shouldn't change any hashes" and adds a fixed-devnet-header regression test — that test is the right safety net.

Bottom line

From a hash-function soundness standpoint, this round is clean. The previous footgun is removed, the public API is now exactly what the chain actually uses, and the test surface includes KAT vectors for every public hashing primitive. I'd merge it once:

• The dead // Compact encoding hash functions banner is removed (1-line),
• The README is refreshed (or at least its hash_for_circuit / substrate references are stripped),
• Chain PR #571 is ready to land in the same window,
• And someone consciously picks the SemVer-major-vs-minor question.

No other open issues.

[n13]

Deleted the stale header comment

Please update README, then GTG!