[CassOnMars]

No description provided.

[felirami]

Comment-only review on head f4fc4af. I did not modify code, approval state, labels, settings, or resolve any threads. The Rust workspace check and Solidity tests passed locally; the inline comments capture the blocking correctness/build findings.

[felirami]

[P1] Bind the transfer envelope during validation. This still calls validate_against_store, which verifies the legacy bare signing_payload(). The new signing_payload_with_envelope(...) is therefore not enforced here, and the block-import path repeats the same bare validation. A relay can replace an output's valid one_time_pubkey without invalidating the input signature, changing or stranding the recorded output owner. Please verify the envelope-bound digest at both admission and import, and make the wallet sign that same digest.

[felirami]

[P1] The wire output lacks the data required to recover and spend a confidential note. scan_stealth_note needs the sender's ephemeral tx_pubkey, and a later spend needs the note's value/blinding (the crypto module already provides an encrypted note payload for this). Neither is carried here, while the wallet drops the generated tx_pubkey and returns only the message. As written, recipients cannot independently discover/open these outputs and construct a subsequent spend without an undocumented out-of-band channel.

[felirami]

[P1] Messages produced by this builder cannot pass runtime validation. The builder subtracts a random output_blinding associated with an output_commitment that is never included in the lock, so the runtime's balance-closure check does not match. It also always sends an empty range_proof, while validate_against_store explicitly rejects empty proofs. The builder should construct the exact blinding delta and range proof expected by the runtime.

[felirami]

[P1] The deployer UI does not build from this PR. These imports reference missing src/lib/merkle and src/lib/leaf modules; RootSigner also imports a missing src/lib/recover. After npm ci and forge build, npm run build fails on these modules and on missing Node typings for the build scripts (node:fs, process, ImportMeta.url). Please add the missing sources/configuration so a clean build succeeds.

[felirami]

[P1] Do not commit the state batch after block staging fails. This logs stage_block errors and then still commits the state mutations without the block, recreating the state/header divergence this atomicity change is intended to prevent. The shard-chunk path in src/storage/store/engine.rs has the same pattern. Staging failures should abort or propagate before commit.

[backmeupplz]

We read through this PR at 7eb981b against the current FIP text in discussion #19. It's a static read only, nothing was built or run, so please treat the severities as our estimate. Line numbers are at 7eb981b.

Overall the core is in good shape: shard-0 ledger math is checked everywhere, emission is integer-only with BTreeMap ordering, the Rust/Solidity header, leaf and action encodings match byte for byte, Falcon decoding is strict, the pivot boot is crash-safe, and the relay dedupes on recomputed hashes. The issues below are what we'd want resolved before mainnet.

Bridge

1. Any past validator set can anchor roots and post governance actions forever (critical). HypersnapBridgeV2.sol:276-282 (advanceRoot) and :479-499 (_applyAction) accept a header from any epoch that has a setRoot, as long as height > latestHeight. Nothing ties h.epoch to h.height (the contract has no PIVOT_HEIGHT/EPOCH_LENGTH), and nothing requires h.epoch == latestEpoch. test_advanceRoot_oldEpochStaysValid asserts this. So 2/3 of a retired set (e.g. the bootstrap set, with no slashable stake left) can certify a header with any bridge_lock_root and mint on every chain. It can also post an action at height = 2^64-1, which pins lastActionHeight so no later pause/cancel can land. Suggested fix: store pivot and epoch length as immutables, require h.epoch == epochOf(h.height) and h.epoch == latestEpoch.
2. V2 depends on a CERT-STARK verifier that doesn't exist yet, and it's immutable (high). HypersnapBridgeV2.sol:98,206-213. If it's unsound, anyone mints. If it rejects valid proofs, the bridge is bricked, because proposeUpgrade itself needs a proof. ICertificateVerifier describes bundled Falcon signatures while FIP §16.6 says an LWE-MS2 aggregate. UpgradeToV2.s.sol:60,173 only checks code.length > 0, so even the test mock verifier would pass. Pin an audited codehash and add a recovery path.
3. V1 is one ECDSA key (high, and at odds with "trustless"). HypersnapBridge.sol:188-197,229-258. The owner (the deployer EOA by default) signs any root and mints instantly, with no cap or delay. The DKLS23 signer only appears as a storage prefix. A per-root cap or delay would bound the damage until V2 ships.
4. Burn watcher finality is 64 blocks behind head (medium). bridge_burn_watcher.rs:161-173. On OP that's unsafe-head territory. A reorged burn stays in the store and gets credited on Hypersnap. Scanning to BlockNumberOrTag::Finalized fixes it.
5. Burns with the FID in the "wrong" end of bytes32 are destroyed (medium). bridge_burn_watcher.rs:249-260,304-317 reads the FID from the first 8 bytes. bytes32(uint256(fid)) puts it in the last 8, so the burn is silently skipped after the tokens are gone. Take a uint64 fid in burn(), or revert on a malformed recipient.
6. Unclaimed V1 retro allocations are stranded by the upgrade (medium). V2 keeps __v1LatestRoot but never accepts claims against it (HypersnapBridgeV2.sol:51-53,128).
7. Smaller: a newer anchor invalidates pending claim proofs, and a later action makes an earlier decided one permanently stale on that chain (:300-306,490-491; a mapping(bytes32 => bool) lockRoots plus per-kind watermarks would fix both). Processed burns are never pruned, and burns to unregistered FIDs are retried every block, so this is a cheap DoS (watchers.rs:283-300). The inbound contract address isn't in consensus params or the burn key (bridge_burn_store.rs:44-50).

Emission and PoQ

8. The DA pool never pays on mainnet (high). src/chain/scoring.rs:424 passes params_for(...), which starts from ScoringParams::default() and never sets pivot_height_0/epoch_length_blocks (:279-290). epoch_boundary_height then returns (e-1)*len without the pivot, so every answer's offset is far past the 25-block window, latency is 0, and so is every DA score. allocate_proportional returns empty, and 50% of each epoch's emission goes unminted.
9. The post-transfer filter is missing from the production reader (high). In poq_reader.rs:325-424,447, followees, engagement, casts, active days and replies ignore effective_ts. Only the age factor resets on transfer, so a bought aged FID keeps its EigenTrust inputs, growth pairs and F2/F3/F6 history. That contradicts §8.1 and the reader trait's own contract (reader.rs:14-17).
10. App PoW can be farmed (high).
    • metrics.rs:139 gives every FID client_diversity = 1.0, so credibility is at least 0.10 with zero trust and zero age.
    • app_pow.rs:57-66 uses that as w_user, where §7.1 says trust × age_factor. There's no owner≠user exclusion.
    • Separately, miniapp.rs:222,226-245: remove doesn't decrement adds_by_epoch, so add/remove cycling earns add work without limit.
11. The code and the posted FIP have drifted apart (medium). Growth uses harmonic mutuality, 1/√n, skew damping, min distinct crediters, a vouch boost, and the composite cred^4·trust^0.5·(H_in·H_out)^3·… (proof-of-quality/src/scoring.rs:177-276,335-372). EigenTrust is a fixed 30 iterations, DA_CHALLENGES is 100, and the trust floor is fixed. Doc comments cite §6.2 exponents and §6.7 vouches that aren't in the discussion text. If there's a newer draft, could it be posted? Otherwise one side needs to change.
12. Forward Growth uses lifetime counts (medium). metrics.rs:301 and poq_reader.rs:360 sum each pair over all history, so every epoch re-pays the graph the retro already paid for. That's the time-base question from the discussion, and the FIP should state it either way.
13. Retro supply isn't capped and retro isn't reproducible (medium).
    • Supply: MAINNET_RETRO_MINTED is a 50M placeholder (emission.rs:23). apply_scoring_result never asserts minted ≤ emission(epoch) (boundary.rs:160-170). retro_minted and epoch_length can be overridden from config on mainnet (hypersnap_main.rs:302-322).
    • Reproducibility: retro_rewards_finalize.rs uses f64, HashMap, SystemTime::now(), a live hub with no snapshot height and CLI-tunable exponents. v6_reproduction_test.rs:29 reads a CSV from /Users/caheart/.... Nothing in the repo builds the Merkle root. §10.5 promises a third party can re-derive it.
14. DA challenges are known about 300 blocks early, because epoch_seed only hashes reveals (seed.rs:127-137). Mixing in the boundary block hash, as epoch.rs:63 already does, closes that. Also metrics.rs:282 does a linear Vec::contains per edge, which is O(E·N) on the scoring path.

Consensus and node behavior (these also affect pre-pivot nodes)

15. New time bomb: every node halts around 2026-12-21 (high). src/consensus/read_validator.rs:280-320 (new in this PR) exits with an error once a chunk's timestamp is 60 days past the newest active_at in the binary's schedule. The latest mainnet entry is 2026-10-22, and nothing gates the check on the pivot. The 2026-08-06 → 2026-10-22 gap (77 days) would already have tripped it. We'd make it a warning or metric.
16. Unvalidated proposals are persisted (medium-high). src/chain/shard0/proposer.rs:56 and src/chain/proposer.rs:111 store every FullProposal for next/next+1 in UndecidedValues (RocksDB) before validation, with no check that the sender is the round's proposer. Any peer can fill disk at up to ~18 MB per message.
17. The stage_block failure still commits state (medium). block_engine.rs:2271-2274 and engine.rs:3423-3429 log the error and still db.commit(txn), which is the F033 divergence the change was meant to prevent. This was raised earlier on the now-removed src/hyper code and is still present.
18. ContactInfo relayed more than one hop is dropped (medium). gossip.rs:1608 compares the inner peer_id to propagation_source (the forwarder) instead of message.source. Separately, gossip.rs:1212-1225 unsubscribes and resubscribes all topics whenever a validator reconnects. That's every 15 s, but the PRUNE backoff is about 60 s, so one flapping validator keeps the mesh degraded.
19. The scoring height is a hard liveness point (medium). engine.rs:613-616,693-699 treats a not-ready or byte-different local ScoringResult as an invalid block. Slow scorers fail rounds, and any PoQ nondeterminism halts the chain. Waiting or abstaining while the result isn't ready would be safer.

Crypto, CI and scope

20. Validators sign with Falcon-512, not LWE-FSA (high, against the spec). src/core/consensus_keys.rs uses Falcon for consensus. §16.2/§16.4 reserve Falcon for user keys, and §16.7 accepts its hard-to-make-constant-time signing only because that happens on user devices. hypersnap-lattice is research-only and has no MS2. Either amend the FIP (Falcon as an interim consensus key, with a side-channel argument) or gate mainnet on LWE-FSA. QCT3 is stubbed (engine.rs:586-589 rejects it), which is fine but worth stating in the FIP.
21. CI doesn't run most of the new tests. verify.yml:71 runs plain cargo test with no default-members, so the Falcon KATs, the lattice, relay e2e and the wallet/messages/crypto tests never run. verify-impl.yml is workflow_call only and nothing calls it. No workflow runs forge test.
22. Smaller:
    • SIGNATURE_SCHEME_NONE escrow messages have malleable twins, from non-canonical data_bytes, any v, and high-s in recover_eip712. Nonce dedupe prevents double spends, but hashes change.
    • vote_digest and non-mainnet message checks don't bind the network id.
    • The wallet writes seeds before chmod 0600 and stores them in plaintext.
    • The relay allows Basic auth over http and derives Debug with the passwords in it.
23. Scope. About 300k of the 424k added lines are quil-lattice-ct and quil-lattice-proof-sys, which nothing in the workspace references. contracts/deployer-ui is only referenced from a comment and isn't in CI. Dropping those, or landing them separately, would make this reviewable. Splitting the rest into bridge / emission / pivot PRs would help even more.

Happy to help with fixes or tests on any of these, especially 1, 8, 15 and 21.