diff --git a/pallets/mining-rewards/src/lib.rs b/pallets/mining-rewards/src/lib.rs index 919931ff..5d6a531e 100644 --- a/pallets/mining-rewards/src/lib.rs +++ b/pallets/mining-rewards/src/lib.rs @@ -227,8 +227,18 @@ pub mod pallet { } /// Round down to a multiple of the leaf quantum. Returns `(aligned, remainder)`. + /// + /// A degenerate zero quantum (only reachable if `AMOUNT_SCALE_DOWN_FACTOR` + /// cannot be represented in `BalanceOf`, which `integrity_test` already + /// rejects at startup) must never divide by zero: `%` would panic inside an + /// `on_finalize` hook and halt the chain. Treat the whole amount as dust + /// instead, which keeps it in `CollectedFees` for a later block. fn quantize(amount: BalanceOf) -> (BalanceOf, BalanceOf) { - let remainder = amount % Self::leaf_quantum(); + let quantum = Self::leaf_quantum(); + if quantum.is_zero() { + return (BalanceOf::::zero(), amount); + } + let remainder = amount % quantum; (amount.saturating_sub(remainder), remainder) } @@ -237,29 +247,48 @@ pub mod pallet { return; } - debug_assert!( - (reward % Self::leaf_quantum()).is_zero(), - "miner credits must be leaf-quantum aligned" - ); + // Leaf-quantum alignment is enforced in EVERY build profile, not just + // debug. Only whole multiples of `AMOUNT_SCALE_DOWN_FACTOR` can be + // committed to a ZK-tree leaf, so minting an unaligned credit would + // create supply that no nullifier can ever exit — silently locked + // value. The previous `debug_assert!` compiled out in release, leaving + // that invariant enforced by the caller's earlier `quantize` call + // alone. Re-deriving it here closes the gap without changing the + // amount minted on any honest path: an already-aligned reward + // re-quantizes to itself with zero dust, which `retain_unminted` + // adds nothing for. + let (aligned, dust) = Self::quantize(reward); + if !dust.is_zero() { + Self::retain_unminted(dust); + } + if aligned.is_zero() { + return; + } - match T::Currency::mint_into(miner, reward) { + match T::Currency::mint_into(miner, aligned) { Ok(_) => { T::ProofRecorder::record_transfer_proof( None, // Native token T::MintingAccount::get(), miner.clone(), - reward, + aligned, ); - Self::deposit_event(Event::MinerRewarded { miner: miner.clone(), reward }); + Self::deposit_event(Event::MinerRewarded { + miner: miner.clone(), + reward: aligned, + }); }, Err(e) => { log::warn!( target: "mining-rewards", "Failed to mint {:?} to miner {:?}: {:?}, retaining for retry", - reward, miner, e + aligned, miner, e ); - Self::retain_unminted(reward); - Self::deposit_event(Event::MinerMintFailed { miner: miner.clone(), reward }); + Self::retain_unminted(aligned); + Self::deposit_event(Event::MinerMintFailed { + miner: miner.clone(), + reward: aligned, + }); }, } } diff --git a/pallets/zk-tree/src/tree.rs b/pallets/zk-tree/src/tree.rs index a2901305..ad398b5e 100644 --- a/pallets/zk-tree/src/tree.rs +++ b/pallets/zk-tree/src/tree.rs @@ -94,26 +94,49 @@ fn bytes_to_felts_compact_lossy(input: &[u8]) -> impl Iterator(leaf: &ZkLeaf, T::AssetId, T::Balance>) -> Hash256 { use qp_poseidon_core::serialization::u64_to_felts; let mut felts = Vec::with_capacity(8); - // to_account: 4 felts (32 bytes -> 4 felts at 8 bytes/felt) + // to_account: 4 felts (32 bytes -> 4 felts at 8 bytes/felt). let to_bytes = leaf.to.as_ref(); debug_assert_eq!(to_bytes.len(), 32, "Account must be 32 bytes"); - // Encoding-safety guard: each 8-byte little-endian limb must be canonical - // (< Goldilocks prime) so the non-injective compact encoding does not silently - // reduce the recipient. See the invariant note above. - debug_assert!( - to_bytes - .chunks_exact(8) - .all(|limb| u64::from_le_bytes(limb.try_into().expect("8-byte limb")) < GOLDILOCKS_P), - "recipient account is non-canonical for the 8-byte/felt leaf encoding" - ); - felts.extend(bytes_to_felts_compact_lossy(to_bytes)); + + // Encoding-safety guard, ENFORCED IN EVERY BUILD PROFILE. + // + // The compact encoding is lossy for 8-byte limbs `>= p`, so a non-canonical + // recipient hashes identically to its canonical alias. Previously this was + // only a `debug_assert!` caller contract, which compiles out in release: a + // single caller regression would silently re-introduce the aliasing the + // invariant note above exists to prevent (two distinct deposits committing + // to one leaf => shared nullifier and one permanently unexitable deposit). + // + // Reducing the recipient here makes the invariant structural instead of + // contractual. It is fully backward compatible: canonicalizing a recipient + // whose limbs are already `< p` is the identity, and for a limb `>= p` the + // lossy encoder already subtracts `p` exactly once — which is precisely what + // `canonicalize_account_bytes` does — so every hash produced today is + // unchanged, byte for byte. + // + // The 32-byte length is also enforced here rather than trusted: an + // `AccountId` of another width cannot be canonicalized limb-wise, so it + // falls back to the previous lossy path instead of panicking in runtime + // code (the `debug_assert_eq!` above still fails loudly in dev/test builds). + match <[u8; 32]>::try_from(to_bytes) { + Ok(bytes) => { + let canonical = canonicalize_account_bytes(bytes); + felts.extend(bytes_to_felts_compact_lossy(&canonical)) + }, + Err(_) => felts.extend(bytes_to_felts_compact_lossy(to_bytes)), + } // transfer_count: 2 felts (u64 as two 32-bit limbs, high then low) felts.extend(u64_to_felts(leaf.transfer_count));