fix(security): enforce release-safe leaf canonicalization and mining reward quantum alignment

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<T>`, 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<T>) -> (BalanceOf<T>, BalanceOf<T>) {
-			let remainder = amount % Self::leaf_quantum();
+			let quantum = Self::leaf_quantum();
+			if quantum.is_zero() {
+				return (BalanceOf::<T>::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,
+					});
 				},
 			}
 		}

pallets/zk-tree/src/tree.rs

@@ -94,26 +94,49 @@ fn bytes_to_felts_compact_lossy(input: &[u8]) -> impl Iterator<Item = Goldilocks
 /// deposits can commit to the *same* leaf, which in the wormhole means a shared
 /// nullifier and one permanently unexitable deposit. The withdrawal circuit binds
 /// the leaf's `to_account` felts to `WA(secret)` (a canonical Poseidon output), so
-/// canonicalizing the recipient never changes who can exit a leaf. The
-/// `debug_assert` below enforces the caller contract in test/dev builds.
+/// canonicalizing the recipient never changes who can exit a leaf.
+///
+/// This contract is now enforced by the code itself in every build profile:
+/// `hash_leaf` canonicalizes the 32-byte recipient before encoding, so a caller
+/// that forgets to canonicalize can no longer change what is committed. The
+/// `debug_assert_eq!` below still fails loudly in dev/test builds when an
+/// `AccountId` is not 32 bytes wide.
 pub fn hash_leaf<T: Config>(leaf: &ZkLeaf<AccountIdOf<T>, 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));