Conversation
Miners can match snapshot rows locally and submit ownership proofs without sending keys to the claim server. Co-authored-by: Cursor <cursoragent@cursor.com>
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
Verdict: REQUEST_CHANGES — the claim protocol matches the current service, but the CLI has blocking credential-lifetime and claim-result correctness issues.
-
High — wormhole spend secrets and the mnemonic lose zeroize-on-drop protection (
src/cli/airdrop.rs:226-228,src/cli/airdrop.rs:267-295,src/cli/airdrop.rs:364-367,src/cli/airdrop.rs:388-400,src/cli/airdrop.rs:462-472). The code copies each spend secret out ofWormholePairinto a plain[u8; 32], copies it again into aClone + Debugreward enum, and copies it again when constructing the proof.Secret::try_fromonly protects its own final copy; the copies retained inCredentialsandFoundRewarddrop without being wiped and are eligible for accidental debug disclosure.take_mnemonic()likewise removes the phrase fromWalletData, whoseDropimplementation would otherwise zeroize it, and then lets the returnedStringdrop normally. These are credentials for the underlying wormhole funds, not airdrop-only tokens. Keep them in move-only zeroizing wrappers, reference credentials from match metadata without copying them, redact/removeDebug, and explicitly zeroize the mnemonic after derivation. Please add success/error-path zeroization coverage analogous towormhole_lib. -
High — submission failures still produce a successful command exit (
src/cli/airdrop.rs:197-223). Every prover, network, rejection, and response-decoding error is logged and converted intoskipped, after whichhandle_claimreturnsOk(()). A server outage or rejection of every claim therefore reportsRecorded 0 QUAN ...and exits 0, so unattended callers cannot tell that no claim was recorded. Continuing best-effort across independent matches is reasonable, but retain actual failures and return an error/non-zero exit after the summary when any submission failed; keep intentionally unsupported historical schemes separate from failures. Cover all-failed and partially-failed batches. -
Medium —
--password-filecannot support the documented default destination for the default wallet scheme (src/cli/airdrop.rs:242-250,src/cli/airdrop.rs:318-324). Wallet create/import defaults to ML-DSA-65, socollect_credentialssuccessfully unlocks such a wallet for HD wormhole derivation and then discards its keypair because it cannot make a Dilithium claim. When--tois omitted,resolve_claim_accountreopens the wallet through the generic name resolver, which has no access to the supplied password file. Non-interactive use fails despite the valid--password-file; interactive use prompts a second time. Retain the authenticated wallet account ID inCredentialsand use it as the default destination without reopening the wallet. Add a protected ML-DSA-65/password-file test. -
Medium —
--dry-rundoes not do what its help promises (src/cli/airdrop.rs:85-87,src/cli/airdrop.rs:476-478). It computes the signature/proof but prints only the address and scheme, not the signed/proved payload. Either serialize the body for inspection as advertised or narrow the option description to its actual behavior.
Validation:
- Compared the complete diff at
885bf8d0dd8915719db3611d890a0f50e304c108against baseb98083bf29a3bee5a121affd723431d3654c3247. - Cross-checked snapshot, JSON, Dilithium, historical hashing, and ownership-proof contracts against
Quantus-Network/airdrop-claimhead629f46acb4eed3e92d1d6cc8bb03a79ed8042650; they align. cargo +nightly-2026-08-31 fmt --all -- --check— pass.taplo format --check --config taplo.toml— pass.SKIP_CIRCUIT_BUILD=1 cargo clippy --all-targets --locked -- -D warnings— pass.- Focused airdrop, password, and developer-wallet tests — pass.
- Full local suite with circuit generation intentionally skipped: 340/341 library tests passed; the sole failure required the omitted generated circuit artifacts. GitHub full build/test jobs passed on Ubuntu and macOS, along with format, analysis/docs, and security audit.
…lts. - Wormhole spend secrets live in a move-only SpendSecret (zeroize on drop, redacted Debug); matches reference them by index instead of copying. The mnemonic and secret-file hex are zeroized after derivation. - Any claim submission failure now fails the command after the batch summary; intentionally unsupported schemes still count as skipped. - The unlocked wallet's account id is kept as the default --to, so ML-DSA-65 wallets with --password-file no longer reopen the wallet (prompt/failure). - --dry-run prints the serialized claim payload as documented. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed all four review points in 27cafd4:
|
The v0.9.x permutation was generated with ChaCha8 (seed 0x189189189189189) and differs from the current constants, so reconstructing the v09 sponge on the current permutation produced wrong addresses. Derive v09 Dilithium and wormhole addresses through the original crate and pin them with vectors from the tagged v0.9.5 source. Co-authored-by: Cursor <cursoragent@cursor.com>
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
Verdict: REQUEST_CHANGES — three of the four earlier blockers are fixed, but the wormhole spend-secret zeroization fix remains incomplete.
- High — matching and proving still leave unzeroized copies of the wormhole spend secret (
src/cli/airdrop.rs:579,src/cli/airdrop.rs:759-779).Secret::try_from(*secret.bytes())copies the credential into a plain array at the proving boundary. In the pinnedqp-zk-circuitsv4.4.0 dependency, thatTryFrom<[u8; 32]>implementation explicitly does not scrub its source;Secret::new(&mut bytes)is the real-secret constructor that zeroizes the source on both success and failure. Snapshot matching also materializes each secret in ordinary heapVec<Goldilocks>buffers: the v0.9 preimage, eachencode_secretresult, and the combined current-scheme preimage all deallocate without wiping, andextendcan free an earlier allocation before any drop-time cleanup. These bytes/felts are the underlying spend credential, and scanning five schemes per secret creates multiple stale copies even thoughSpendSecretitself is wiped. Use the scrubbing constructor at the prover boundary and zeroizing field-buffer wrappers with full capacity reserved before writing secret material. Add allocator-based success/error-path coverage analogous toqp-zk-circuits'wormhole/circuit/tests/heap_zeroization.rs; the current Debug-only tests cannot detect these residual copies.
The other requested fixes are sound: the mnemonic and retained secret are wiped, match metadata now holds only an index, submission failures return an error after the batch summary, the unlocked account supplies the default destination, and dry-run prints the serialized payload. The v0.9.5 derivations also match the active claim-service branch.
Validation:
- Reviewed the complete diff from base
b98083bf29a3bee5a121affd723431d3654c3247through exact heada967d9d3d0f57ef900b6a782cceff9b9f5b177e9, plus both commits since the earlier review. - Cross-checked snapshot, Dilithium, historical hashing, ownership-proof, JSON, and response contracts against
Quantus-Network/airdrop-claimbranch heada4d639075f2134ba42858acf1502c7806c386ef5— aligned. git diff --check— pass.cargo +nightly-2026-08-31 fmt --all -- --check— pass.taplo format --check --config taplo.toml— pass.cargo metadata --no-deps --locked --format-version 1— pass.SKIP_CIRCUIT_BUILD=1 cargo clippy --all-targets --locked -- -D warnings— pass.- Focused airdrop tests: 15/15 passed in both library and binary targets; password tests: 16/16 in both targets; developer-wallet tests: 2/2 in both targets.
- All current GitHub checks at this head are successful, including Ubuntu/macOS build-and-test, format, analysis/docs, security audit, examples, and dependency cooldown.
The claim matcher started at qp-poseidon 0.9.5, but shipped Resonance chains also used poseidon-resonance 0.8.0 (legacy plonky2 Poseidon, 8-byte limbs padded to 73 felts) and qp-poseidon 0.9.1 (4-byte limbs padded to 188). Derive both so early miners' rewards are found, mirroring the scheme ids the claim server accepts. Golden vectors come from the exact crates the chains pinned. Co-authored-by: Cursor <cursoragent@cursor.com>
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
Verdict: REQUEST_CHANGES — the historical address derivations align with the claim service, but two blocking secret-handling and wallet-safety issues remain.
-
High — wormhole matching and proving still leave copies of the spend secret in ordinary memory (
src/cli/airdrop.rs:579-580,src/cli/airdrop.rs:771-791).Secret::try_from(*secret.bytes())creates a plain[u8; 32]copy at the prover boundary. In the exact pinnedqp-wormhole-circuitv4.4.0 API, thisTryFromimplementation explicitly does not scrub its source;Secret::new(&mut bytes)is the real-secret constructor that zeroizes the source on success and failure. Before proving, each matching scheme also materializes the secret in ordinary heapVecbuffers throughinjective_bytes_to_felts(secret),encode_secret(secret), and the combined preimage, then frees those buffers without wiping them. These bytes/felts are the underlying wormhole spending credential. Use the scrubbing constructor and zeroize-on-drop byte/felt buffers with full capacity reserved before secret material is written. Add allocator-based success/error-path coverage like the pinned circuit crate'sheap_zeroizationregression; the current Debug-only tests cannot detect freed-memory copies. -
Medium — the new public replacement helper deletes a wallet before validating that it is a developer wallet or successfully creating its replacement (
src/wallet/mod.rs:249-253,src/cli/mod.rs:626-653). For example,recreate_developer_wallet("real_wallet")first deletesreal_wallet.json, thencreate_developer_walletrejects the unsupported name withKeyGeneration. Even for an intendedcrystal_*name, a later generation/encryption/save error leaves the old file gone, whilecreate-test-walletscatches the error, prints an overall success message, and exitsOk(()). Validate the allowed name before any deletion and replace atomically (or restore on failure); also propagate any batch failure to a nonzero command exit. Add regression coverage proving an invalid name and a failed replacement both preserve the original wallet.
Validation:
- Reviewed the complete diff from base
b98083bf29a3bee5a121affd723431d3654c3247through exact head789237846f1f9e598561fc1b49fd61b222a19487, including the incremental commits since the prior review. - Cross-checked snapshot, historical hash identifiers/vectors, claim JSON, Dilithium signature, and wormhole proof contracts against
Quantus-Network/airdrop-claimbranch headadb6c8ab9cefcda74e4ee7d53f47a9da857f5e27; they align. git diff --check, pinned rustfmt, Taplo, and locked Cargo metadata — pass.SKIP_CIRCUIT_BUILD=1 cargo clippy --all-targets --locked -- -D warnings— pass.- Focused airdrop tests: 16/16 in each library and binary target; password tests: 16/16 in each target; developer-wallet tests: 2/2 in each target.
- All exact-head GitHub checks are successful, including Ubuntu/macOS build-and-test, analysis/docs, format, examples, security audit, and dependency cooldown. Local
cargo auditwas unavailable because the subcommand is not installed; the GitHub security-audit job passed.
The middle HD path component is not always a change branch: the mobile app uses 0 (external) and 1 (dedicated change branch since July 2026), but 'wormhole multiround' uses it as a round counter, deriving m/44'/189189189'/0'/round'/index' with a default of 2 rounds. The scan only covered branch 0, so app change addresses and every multiround address were missed. Scan branches/rounds 0..=8 for each index, and stretch the BIP39 seed once instead of per path so the wider scan is also faster. Co-authored-by: Cursor <cursoragent@cursor.com>
…r wallet recreate - Matching no longer frees heap buffers still holding the spend secret: preimages are built in pre-sized zeroize-on-drop SensitiveFelts buffers, secret felt encodings stream through injective4_secret_words instead of temporary Vecs, the rate-4 sponge wipes its stack state, and the prover boundary uses Secret::new so the handoff copy is scrubbed. An allocator-based regression test scans every freed block for the secret in both raw-byte and felt encodings (with a documented exemption for the one buffer qp-poseidon-core 0.9.5 frees internally). - recreate_developer_wallet validates the crystal_* name and builds the full replacement before touching the existing file, then swaps it in atomically via Keystore::save_wallet; create-test-wallets now exits nonzero when any wallet fails. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed both remaining review items in cc31282:
Needs a human review/merge. |
n13
left a comment
There was a problem hiding this comment.
Verdict: REQUEST_CHANGES — the latest commit fixes the developer-wallet replacement order and the current-scheme secret buffers, but the historical wormhole matching path still frees an allocation containing the spend secret.
High — v0.9 matching still leaves the spend secret in freed heap memory (src/cli/airdrop.rs:856-868, src/cli/airdrop.rs:1311-1319). derive_v09 encodes the 32-byte spend secret into a 96-byte Vec and passes ownership to qp-poseidon-core 0.9.5's hash_no_pad. That method reads the vector and drops it without zeroizing it. The new allocator test explicitly exempts this exact block, so it passes while the credential remains recoverable from freed memory. I removed only the exemption in a temporary local copy of the test; it then failed on a 96-byte secret-bearing allocation. The original file was restored. Because every check and claim scans V09Injective, the leak occurs even for users whose reward uses a newer scheme. Please use a historical hash path that borrows or scrubs its preimage (or fix the pinned dependency) and make the regression test require zero leaked blocks without this exemption.
Validation at cc31282389ad3d901a1b630c3386df9de02efb5e: all 18 focused airdrop tests and all 3 developer-wallet replacement tests passed as submitted; git diff --check passed. The same zeroization test failed as expected when the exemption was removed. No source changes remain in the review worktree.
All ownership and wormhole crates are now on crates.io at 4.4.0 (same code as the v4.4.0 tag), exact-pinned. Co-authored-by: Cursor <cursoragent@cursor.com>
qp-poseidon-core 0.9.5's hash_no_pad consumes its preimage Vec and frees it unscrubbed, so the previous commit exempted that one block in the heap-zeroization test. Rebuild the v0.9.5 sponge from the same public crates (Poseidon2Goldilocks<12>, ChaCha8 constants, seed 0x189189189189189) and hash the secret preimage from a borrowed zeroize-on-drop buffer with wiped stack state. The allocator regression test now requires zero leaked blocks with no exemptions; golden vectors pin equality with the historical hasher. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the remaining High in 945992c: the v0.9.5 sponge is now rebuilt locally from the same public crates the historical qp-poseidon-core used ( Needs a human review/merge. |
…ole from Bitcoin-seed tree Mirrors the quantus-apps SDK change. The ML-DSA-87 public key for the same mnemonic changed four times (pre-FIPS SHAKE256(seed[..32]) expansion; FIPS 204 whole-seed expansion; hardened CLI paths; "Dilithium seed" BIP32 master), so find_matches now derives candidate keypairs for every era from the wallet seed and hashes each under every address scheme. Wormhole HD scanning covers the pre-2.1.0 "Bitcoin seed" tree and the legacy master-node secret as well. Both historical seed expansions are rebuilt locally from the current dilithium crate's public primitives in wipeable buffers (the historical crates' keygens free seed-bearing heap copies unscrubbed). Claims for historical matches re-derive the era's key from the stored self-zeroizing seed and sign with the current crate. Golden vectors pin pk/sk equality with the shipped dilithium 1.0.3 / 2.0.0 and hdwallet 1.0.0 crates. Co-authored-by: Cursor <cursoragent@cursor.com>
|
8cbd49c mirrors the historical-keygen coverage added to the app SDK (Quantus-Network/quantus-apps#653): The two historical seed expansions are rebuilt locally from the current dilithium crate's public primitives in wiped stack buffers (the shipped 1.0.3/2.0.0 keygens free seed-bearing heap copies unscrubbed, so they never see the seed). Historical claims re-derive the era's key from the wallet's self-zeroizing stretched seed at submit time and sign with the current crate. Golden vectors pin pk/sk byte-equality against the shipped 1.0.3 / 2.0.0 / hdwallet 1.0.0 crates. |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
Verdict: REQUEST_CHANGES — the latest historical-derivation and hashing fixes are sound, but explicit wormhole-secret ingestion still leaves the spend credential in freed heap memory, and the fixed recovery windows silently omit supported paths.
-
High —
--wormhole-secret-filefrees a decoded spend-secret allocation without wiping it (src/cli/airdrop.rs:573-581,src/cli/wormhole.rs:317-327).read_wormhole_secretcalls the sharedparse_secret_hex;hex::decodeallocates aVec<u8>containing the 32-byte credential, and the successfultry_into::<[u8; 32]>()moves the bytes out before dropping/deallocating that vector without scrubbing its backing block. Invalid-length and partially decoded error paths also discard the allocation. Every explicit-secretcheckorclaimtherefore leaves a recoverable heap copy beforeSpendSecret,SensitiveFelts, and the prover-boundary wipes take effect. Decode directly into a wipe-on-drop[u8; 32]withhex::decode_to_slice(scrubbing partial output on error), and extend the allocator regression to cover secret-file parsing on success and failure. -
Medium — hard-coded recovery windows silently miss supported historical accounts and rounds (
src/cli/airdrop.rs:31-36,src/cli/airdrop.rs:338-350,src/cli/airdrop.rs:436-450). Wallet create/import accepts and stores an arbitrary--derivation-path, butload_wallet_materialdiscards that path and the historical key list tries only account indexes 0 through 8. A mnemonic reimported at account 9 or a custom path can therefore have a rewarded historical public key that this command never derives. The same issue exists for wormhole multiround:wormhole multiround --roundshas no maximum, while airdrop matching stops at round 8 and exposes no branch/round override. These cases return “No airdrop addresses found” rather than explaining the cutoff. Retain/use the wallet's derivation path and expose explicit account/round scan controls (or another complete deterministic recovery path); add account-9 and round-9 snapshot-match tests.
Validation:
- Reviewed the complete diff from base
b98083bf29a3bee5a121affd723431d3654c3247through exact head8cbd49c649a2c4cb7a2262d87c9dfc19f5c4240e, including all commits since the earlier reviews. - Cross-checked snapshot JSON, scheme identifiers, Dilithium signature binding, expiry handling, and ownership-proof inputs against
Quantus-Network/airdrop-claimPR #1 head3bdddac18e3837fccdcd72cc1b74a369083f5c18; the wire/proof contracts align. git diff --check,cargo +nightly-2026-08-31 fmt --all -- --check,taplo format --check --config taplo.toml, and locked Cargo metadata — pass.SKIP_CIRCUIT_BUILD=1 cargo clippy --all-targets --locked -- -D warnings— pass.- Focused airdrop tests: 21/21 in each library and binary target; developer-wallet replacement tests: 3/3 in each target; password tests: 16/16 in each target.
- Exact-head GitHub format, Ubuntu/macOS build-and-test, analysis/docs, security audit, and examples jobs pass. Dependency cooldown remains failed because the exact-pinned 4.4.0 ownership/wormhole crates are four days old versus the repository's 30-day policy; that requires the normal wait or emergency-bypass process.
Summary
quantus airdrop checkandquantus airdrop claimso miners can match snapshot rows locally and submit Dilithium signatures or wormhole ownership proofs.crystal_*developer wallets oncreate-test-wallets, and try the empty password beforeQUANTUS_WALLET_PASSWORDso leftover env values do not block those wallets.Test plan
quantus developer create-test-walletsreplaces existingcrystal_alice/bob/charliewith empty-password genesis keysdata/dev_dummy.csv:quantus airdrop check --server http://127.0.0.1:8080 --wallet crystal_alicequantus airdrop claim --server http://127.0.0.1:8080 --wallet crystal_alice --to crystal_bobrecords a Dilithium claim--wormhole-secret-file(32 bytes of0x2a) records against the dummy Planck row--passwordon argv is rejected;--password-file/ prompt still workMade with Cursor