Leaf Proof Count 7

[illuzen]

also updated metadata to latest for chain

this needs to update to latest version of qp-zk-circuits

---

Note

High Risk
Large generated metadata and circuit-parameter changes can break installed binaries until bins regenerate and must match the deployed runtime; multisig/high-security API removals are breaking for older integrations.

Overview
This PR retargets wormhole/ZK circuit generation for 7 leaf proofs per batch (down from 16), aimed at lower peak memory on mobile, and bumps the CLI to qp-poseidon-core 2.0.2 while dropping the separate qp-poseidon dependency.

Circuit binaries: DEFAULT_NUM_LEAF_PROOFS is now 7 and is public for reward aggregation. Runtime is_ready() also treats cached generated-bins as stale when config.json’s num_leaf_proofs or the CLI version marker no longer match; build.rs only watches QP_NUM_LEAF_PROOFS for rebuilds. A clippy.sh script mirrors CI’s locked clippy invocation.

Chain metadata: quantus_subxt.rs is regenerated for the latest runtime—ZkTree pallet/API, zk_tree_root on block headers, wormhole events carrying leaf_index (Merkle proofs via RPC), removal of wormhole TransferProof storage, reversible-transfers guardian naming/storage (GuardianIndex, next_transaction_id), and multisig pallet changes (e.g. no approve_dissolve, bounded propose calls, SignerApproved event, MaxInnerCallWeight). The high-security CLI follows guardian_index; the multisig example prints proposal_nonce instead of active proposal count.

Note: Example text may still mention dissolve APIs that no longer exist on-chain until follow-up doc fixes.

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

[n13]

Please address the bugbot issue - can just rename the binaries or version them

[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 2525b29. Configure here.}

[n13]

I've reviewed PR #106 thoroughly — fetched the branch locally, read the diffs, the hand-written code, the build/runtime logic, the dependency changes, and the bugbot/reviewer threads. Here's my assessment.

Overall

Solid, well-reasoned PR. It does two largely independent things: (1) retargets ZK circuit generation from 16 → 7 leaf proofs per batch for mobile memory, and (2) regenerates quantus_subxt.rs against the latest runtime (ZkTree pallet, guardian rename, multisig dissolve removal, leaf_index transfer events). CI is fully green (build matrix on both OSes, clippy, examples, format, bugbot). The previously-flagged high-severity stale-bins bug has been genuinely addressed. There are a few real issues worth resolving before merge, none of them catastrophic.

Status of prior review items

• Bugbot HIGH — "Stale bins after leaf count" → resolved. is_ready() now regenerates when config.json's num_leaf_proofs doesn't match, in addition to the version marker:

Ok(config) => config.num_leaf_proofs == env_num_leaf_proofs(),

This is the right fix and correctly forces regeneration for installed users who already have 16-leaf bins.

• Bugbot MEDIUM — "Build script trigger file mtime" → resolved. The trigger-file/timestamp approach the bot complained about was removed; the latest commit relies on env + default fingerprinting instead. (But see the comment-correctness note below.)

• n13 — "version/rename the binaries" → functionally addressed via the num_leaf_proofs config check, though the crate version itself was not bumped (see below).

Issues worth addressing

1. Operational (highest risk): the leaf count must match the deployed runtime. Going 16 → 7 changes the aggregation circuit and therefore its verifier key. If the on-chain aggregation verifier was not deployed with num_leaf_proofs = 7, every reward-collection / dissolve aggregation the CLI submits will fail on-chain verification. This is the critical coordination item — confirm the chain release this targets is built with 7 before shipping. (The PR description already flags this as high risk; just calling it out as the thing to verify.)

2. build.rs comment is technically incorrect about Cargo. The new comment claims:

// No `cargo:rerun-if-changed` directives: Cargo uses default fingerprinting,
// re-running this script when any source file in the package changes.
...
println!("cargo:rerun-if-env-changed=QP_NUM_LEAF_PROOFS");

Once you emit any rerun-if-* directive, Cargo opts out of the "re-run when any package file changes" default and only re-runs on the emitted conditions. So the stated reasoning is wrong. In practice the important case still works — build.rs does include!("src/bins_consts.rs"), so editing DEFAULT_NUM_LEAF_PROOFS recompiles the build script (via include! dep tracking) and thus re-runs it; circuit-crate version bumps also recompile it. So behavior is acceptable, but please fix the comment so a future maintainer doesn't rely on a false premise.

3. Crate version not bumped (1.3.4 → 1.3.4). Two different builds labelled 1.3.4 now exist with different leaf counts. The num_leaf_proofs config check saves you, but you're relying entirely on it — the VERSION_MARKER check is a no-op for this change. Bumping the version is cheap belt-and-suspenders and is just good release hygiene for a behavior change. Recommend bumping it.

4. Batch-size source of truth is inconsistent (DRY / correctness). run_dissolve batches using the actual config value:

let batch_size = agg_config.num_leaf_proofs;

but collect_rewards batches using the compile-time constant:

.chunks(DEFAULT_NUM_LEAF_PROOFS)

If QP_NUM_LEAF_PROOFS is ever overridden (that env path exists in both build.rs and bins.rs), the circuits are generated for the override while collect_rewards still chunks by 7 → aggregation size mismatch. For the default it's fine, but the two call sites should read from the same source of truth (ideally the agg config, like run_dissolve does) rather than diverging between a constant and the config.

5. Stale dissolve UX/docs. handle_dissolve now correctly errors out loudly, but the Dissolve subcommand and its help text remain:

/// Dissolve a multisig and recover the creation deposit
Dissolve { ... }

So --help still advertises functionality that always errors. Cleaner to remove the subcommand (and its routing) entirely, or at least update the help string. Same for the now-stale field doc on MultisigInfo.creator: /// Creator address (SS58 format) - receives deposit back on dissolve. The PR description acknowledges doc cleanup is pending.

6. Breaking library/SDK surface (note for changelog). Removed public items: approve_dissolve_multisig, MultisigInfo::{deposit, active_proposals}, ProposalStatus::{Executed, Cancelled}. Downstream SDK consumers will fail to compile. Expected given the chain removed the feature, but worth a changelog entry (and reinforces bumping the version).

7. Dependency cleanup is partial. Dropping the direct qp-poseidon = "1.4.0" from Cargo.toml didn't remove it from the build — Cargo.lock still has qp-poseidon 1.4.0 transitively, pulling qp-poseidon-core 1.5.0, so qp-poseidon-core is now duplicated (1.5.0 and 2.0.2). Harmless for the build.rs display hash and the dev-dep, but worth confirming the actual leaf/Merkle hashing path (qp_zk_circuits_common::zk_merkle, used by compute_merkle_positions) resolves to the 2.0.x line consistently, since that's what determines on-chain hash compatibility.

Nice work

• The high-severity regeneration bug is properly fixed with layered checks (REQUIRED_FILES + version marker + num_leaf_proofs).
• collect_rewards_lib removing the local MAX_PROOFS_PER_BATCH = 16 in favor of the shared constant is a good DRY improvement (and motivates fixing Use subxt generated types #4 to finish the job).
• clippy.sh mirroring the locked CI invocation is a handy addition.
• Clear rationale comment on DEFAULT_NUM_LEAF_PROOFS (degree_bits/memory tradeoff).

Recommendation

Approve-with-changes. Before merge I'd want: confirmation that the target chain runtime uses num_leaf_proofs = 7 (#1), the build.rs comment corrected (#2), and the collect_rewards batch-size source unified (#4). The version bump (#3) and dissolve subcommand/doc cleanup (#5) are quick and worth folding in.

Want me to push fixes for the quick ones (#2 comment, #4 batch-size unification, #3 version bump, #5 dissolve removal) on the PR branch? I won't commit or comment on GitHub without your go-ahead.

[n13]

4, 5, and 7 should be fixed IMO