bug: Restore the fallible Sprout aggregate value-balance check dropped by the zcash_primitives transaction refactor (#10461)

[alchemydc]

Summary

The zcash_primitives transaction newtype refactor (PR #10461, merged 2026-08-22) changed Transaction::sprout_value_balance() from a fallible checked sum into an infallible accessor that silently returns zero when the aggregate Sprout JoinSplit balance is out of the valid monetary range. On main, a transaction whose combined JoinSplit balance leaves [-MAX_MONEY, MAX_MONEY] is reported as moving no Sprout value at all, and the downstream fee, remaining-value, and chain-value-pool accounting all consume that zero.

No released version is affected. v6.3.0 and earlier return Err(ValueBalanceError) for the same input, and that error is a hard rejection on every consensus path. The regression commit (8b9115476) is contained in no release tag.

This is not exploitable on any network at any reachable height — see Exploitability. It is a lost invariant and a rule-level divergence from zcashd, not a live vulnerability.

This is the second found instance of the defect class that #11383 explicitly scopes itself out of: fallible accessors on the newtype that convert an out-of-range wire value into None or a default, hiding a consensus-relevant value. #11383 notes that a parse-rejection inventory does not cover this class and that the expiry_height() defect was the only instance found at the time. This is the other one. #11383's parse-time inventory remains correct and complete for its own scope: the Sprout aggregate bound was never a parse-time check.

Reported by the Zakura security team (@zakura-security), who identified the regression and correctly flagged that its exploitability was unproven.

Details

Verified on main @ 0881709ff and against tag v6.3.0.

The regression

On v6.3.0, zebra-chain/src/transaction.rs:1370:

fn sprout_value_balance(&self) -> Result<ValueBalance<NegativeAllowed>, ValueBalanceError> {
    self.sprout_joinsplit_value_balances().sum()
}

ValueBalance's Sum impl is a try_fold over the checked per-pool Add, so an out-of-range aggregate produced ValueBalanceError::Sprout(amount::Error::Constraint { range: -MAX_MONEY..=MAX_MONEY, .. }).

On main, zebra-chain/src/transaction.rs:680-690:

pub fn sprout_value_balance(&self) -> ValueBalance<NegativeAllowed> {
    let balance = self
        .sprout_bundle()
        .and_then(|b| b.value_balance())
        .unwrap_or(ZatBalance::zero());
    ...
}

The and_then is the defect. sprout::Bundle::value_balance() returns Option<ZatBalance>, so unwrap_or(ZatBalance::zero()) conflates two different cases: "this transaction has no Sprout bundle", which correctly contributes zero, and "the aggregate is outside the valid monetary range", which is a consensus-invalid transaction.

The sibling accessors are correct and should not be changed. sapling_value_balance (transaction.rs:405), orchard_value_balance (:468) and ironwood_value_balance (:553) use .map() over Option<&Bundle>, where value_balance() returns a plain already-range-checked value. Sprout is the only pool whose balance is an aggregation that can fail, and the only one where the failure is discarded.

What None means

zcash_primitives-0.30.0/src/transaction/components/sprout.rs:27-31:

pub fn value_balance(&self) -> Option<ZatBalance> {
    self.joinsplits
        .iter()
        .try_fold(ZatBalance::zero(), |total, js| total + js.net_value())
}

ZatBalance is bounded to [-MAX_BALANCE, MAX_BALANCE] where MAX_BALANCE == MAX_MONEY == 21_000_000 * COIN (zcash_protocol-0.10.1/src/value.rs:16-17, 58-66), and its Add returns None outside that range.

Note that because try_fold's accumulator is a ZatBalance rather than an Option, it short-circuits when any prefix sum in wire order leaves the range, not only when the final total does. [+MAX_MONEY, +1, -2] yields None despite a valid total. This makes the None set order-dependent and a superset of "final aggregate out of range". It does not affect the analysis below, because from Canopy onward every vpub_old is zero, so every net_value() is non-negative, the running sum is monotonic, and prefix overflow is equivalent to final overflow.

This was the only place the aggregate bound was enforced

Each vpub_old and vpub_new is individually bounded to [0, MAX_MONEY] at parse time by ZatBalance::from_u64_le_bytes (zcash_primitives-0.30.0/src/transaction/components/sprout.rs:91-102), and net_value() is therefore always in range. Nothing bounds the sum across JoinSplits.

The only two consensus consumers of the raw per-JoinSplit values are joinsplit_has_vpub_zero and disabled_add_to_sprout_pool (zebra-consensus/src/transaction/check.rs:315, :337), and neither sums across JoinSplits. zcashd rejects both directions in CheckTransactionWithoutProofVerification with "txin/txout total out of range"; on main, Zebra has no equivalent, because the aggregation that used to carry it now discards its own error.

What consumes the zeroed value

Transaction::value_balance() (zebra-chain/src/transaction.rs:710) now returns Ok with the Sprout term zeroed where it should return Err, and all three production callers inherit that:

• zebra-consensus/src/transaction.rs:1421 (miner_fee) computes an understated fee.
• zebra-state/src/service/check/utxo.rs:243 (remaining_transaction_value) evaluates the non-negativity rule against the wrong total.
• zebra-chain/src/block.rs:285 (Block::chain_value_pool_change) produces a Sprout chain pool delta of -0 instead of the true value. Its deliberate try_fold guard is intact but is fed an already-wrong number.

The ZIP-209 non-negativity check at zebra-chain/src/value_balance.rs:341 is structurally unable to catch this, because it inspects the very number the bug zeroes. If the zeroing ever fired, the Sprout pool would silently not be debited, the check would pass vacuously, and the wrong pool value would be written to tip_chain_value_pool and to per-height BlockInfo. Rollback is symmetric, so nothing would panic and nothing would surface the drift.

Exploitability

Not exploitable. Both directions are independently blocked, and the blocking checks do not depend on the zeroed value.

Negative aggregate (Σ vpub_old > MAX_MONEY). This is the direction that would matter, because zeroing would let a transaction mint Sprout notes without funding them from the transparent pool: remaining_transaction_value would see only the transparent side and pass. It is blocked twice over. ZIP-211 (disabled_add_to_sprout_pool, zebra-consensus/src/transaction/check.rs:337) rejects any non-zero vpub_old from Canopy onward; it reads raw per-JoinSplit values rather than the aggregate, so the zeroing cannot disarm it, and it runs on both verifier paths via check_common_consensus_rules (zebra-consensus/src/transaction.rs:308 block, :488 mempool) before the first sprout_value_balance() call (:349 block, :521 mempool). Independently, Zebra never full-verifies a block below Canopy on any network: mandatory_checkpoint_height() is Canopy.activation_height().previous() (zebra-chain/src/parameters/network.rs:267), and init_checkpoint_list (zebra-consensus/src/router.rs:412) starts full verification at the lowest checkpoint at or above it even when checkpoint_sync = false. Configured Testnets cannot lower it either, because ParametersBuilder::to_network rejects insufficient checkpoint coverage (zebra-chain/src/parameters/network/testnet.rs:904), and to_network_unchecked is private with no call site that lets an unchecked network escape.

Positive aggregate (Σ vpub_new > MAX_MONEY). This requires spending more than 21,000,000 ZEC of real Sprout notes. Every JoinSplit proof is verified for every non-checkpointed transaction carrying a Sprout bundle (verify_sprout_shielded_data, zebra-consensus/src/transaction.rs:1215, awaited at :345 for blocks and :555 for the mempool), and the circuit binds vpub_old and vpub_new into the primary input (zebra-consensus/src/primitives/groth16.rs:140-153), with anchors checked against real treestates (zebra-state/src/service/check/anchors.rs). The Sprout pool holds far less than MAX_MONEY. This direction is also not profitable even if it were reachable: zeroing removes value from the transaction's pool rather than adding it, so it makes the fee check stricter, not looser.

Checkpointed blocks do reach the zeroed value state-side, via prepare_chain_value_pools_batch (zebra-state/src/service/finalized_state/zebra_db/chain.rs:248), since the checkpoint verifier checks only height, proof of work, Merkle root and the pinned hash. No transaction can be injected there: the block hash must match a hardcoded checkpoint, the header commits to the Merkle root, and V4 txids hash the full serialization including the vpub values. That path only ever sees real history, which contains no out-of-range aggregate.

The reachable consequence today is therefore limited to a benign ordering artifact: on the mempool path miner_fee runs at zebra-consensus/src/transaction.rs:521, before proof verification at :555, so ZIP-317 policy is briefly computed on an understated fee for a transaction whose proofs have not yet been checked. The transaction is dropped when the proofs fail.

Suggested fix

Restore the fallible form. Change sprout_value_balance() to return Result<ValueBalance<NegativeAllowed>, ValueBalanceError>, mapping None to ValueBalanceError::Sprout(amount::Error::Constraint { .. }), and propagate it with ? at zebra-chain/src/transaction.rs:710.

This needs no new error variant and no consumer changes. All three production callers of Transaction::value_balance() already handle an Err:

• zebra-state/src/service/check/utxo.rs maps it to ValidateContextError::CalculateTransactionValueBalances.
• zebra-chain/src/block.rs:285 propagates it through the existing try_fold.
• zebra-consensus/src/transaction.rs:1425 already maps a value_balance() error to TransactionError::IncorrectFee.

The only other call site is ValueBalance::add_transaction (zebra-chain/src/value_balance.rs:258), which is #[cfg(any(test, feature = "proptest-impl"))].

Regression test

Add a test asserting that a V4 transaction with two JoinSplits whose vpub_new values sum above MAX_MONEY does not report a zero Sprout balance, alongside the existing transaction vectors in zebra-chain/src/transaction/tests/vectors.rs.

For context on why this was not caught: PR #10461 kept the regression test for #10585 (zebra-chain/src/block/tests/vectors.rs:76-100), but that test only exercises the transparent overflow path with two MAX_MONEY transparent outputs, so it still passes. There is no coverage of the Sprout aggregate on main.

Follow-up: audit the rest of the accessor class

#11383 asks for an audit of accessors that turn an out-of-range wire value into None or a default. This issue is the home for that follow-up. expiry_height() is being fixed under #11383; the Sprout balance is this issue. The remaining Option-returning accessors on the newtype still need to be checked against every consensus rule gated on them:

• network_upgrade() (zebra-chain/src/transaction.rs:94)
• lock_time() (:128)
• version_group_id() (:177)
• auth_digest() (:250)
• orchard_flags() (:454)
• ironwood_flags() (:523)

Already cleared, for the record:

• consensus_branch_id (zebra-consensus/src/transaction/check.rs:885) fails closed: let Some(tx_nu) = tx.network_upgrade() else { return Err(MissingConsensusBranchId) }.
• orchard_anchor() and ironwood_anchor() (zebra-chain/src/transaction.rs:459, :528) discard a conversion error with .ok(), and the consumer is if let Some(anchor) (zebra-state/src/service/check/anchors.rs:91, :133), which is the same shape as the expiry_height() defect. It is unreachable in practice: both sides use identical pallas::Base::from_repr / to_repr (zebra-chain/src/orchard/tree.rs:151, orchard-0.15.3/src/tree.rs:77-84), so the conversion always round-trips.
• The .expect() in inputs() (zebra-chain/src/transaction.rs:193) is guarded: Transaction::zcash_deserialize runs parse_coinbase_height over every null-prevout input (:884-891), matching exactly the condition under which compat::txin_to_input can fail.
• The BranchId::try_from(..).ok().unwrap_or(..) sites (:1193, :1215, :1274, :1309) and the serde unwrap_or sites (:1063, :1072) are behind #[cfg(any(test, feature = "proptest-impl", feature = "elasticsearch"))].