fix(chain)!: validate the total amount when updating a value balance by arya2 · Pull Request #10817 · ZcashFoundation/zebra

arya2 · 2026-06-26T01:33:30Z

Motivation

ValueBalance::constrain converts a value balance to a different constraint by constraining each pool independently, but never checked that the total across all pools satisfies the target constraint.

Solution

Add a ValueBalance::total() helper that sums all pool balances.
After constraining each pool, validate the total and return a new ValueBalanceError::Total variant if it does not satisfy the target constraint.

Testing

cargo clippy -p zebra-chain --all-targets -- -D warnings
cargo test -p zebra-chain value_balance

AI disclosure

Used Claude Code to complete the compile fix (missing Display arm for the new error variant), commit, and PR description.

`ValueBalance::constrain` now sums all pool balances and returns a `Total` error if the total amount does not satisfy the target constraint. Adds a `total()` helper and a `ValueBalanceError::Total` variant.

Copilot

Pull request overview

This PR updates zebra-chain’s ValueBalance constraint conversion to validate that the sum across all value pools satisfies the target Amount constraint, preventing cases where each pool is individually valid but the overall total is not.

Changes:

Added ValueBalance::total() to compute the sum across all pools.
Updated ValueBalance::constrain to validate the total after per-pool constraining, returning a new ValueBalanceError::Total on failure.
Added ValueBalanceError::Total and wired it into Display.

conradoplg

Looks good, tagged as do-not-merge due to the upcoming release

conradoplg · 2026-07-03T22:39:49Z

@arya2 needs conflict solving

…validate-value-balance-total

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

conradoplg

Considering Copilot's review I think the proper place to check the bound is when adding pools, rather then when constraining. So I did that in 591d450

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

zebra-chain/src/value_balance.rs:330

The PR title and Solution say the target constraint is validated by ValueBalance::constrain, but constrain remains pool-by-pool and the total check now applies only to add_chain_value_pool_change. Consequently, other public constrain callers can still receive a value balance whose total violates the target constraint. If this narrower consensus check is intentional, update the title/description and crate changelog to scope the behavior to chain-pool updates; otherwise introduce a separate checked-constrain API without making widening conversions fallible.

    /// We implement the consensus rules above by constraining the returned value balance to
    /// [`ValueBalance<NonNegative>`].
    #[allow(clippy::unwrap_in_result)]
    pub fn add_chain_value_pool_change(
        self,

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

…validate-value-balance-total

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

fix(chain): validate the total amount when constraining a value balance

d6d2ce8

`ValueBalance::constrain` now sums all pool balances and returns a `Total` error if the total amount does not satisfy the target constraint. Adds a `total()` helper and a `ValueBalanceError::Total` variant.

Copilot AI review requested due to automatic review settings June 26, 2026 01:33

Copilot started reviewing on behalf of arya2 June 26, 2026 01:33 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread zebra-chain/src/value_balance.rs Outdated

Comment thread zebra-chain/src/value_balance.rs Outdated

conradoplg self-assigned this Jul 1, 2026

conradoplg added the do-not-merge Tells Mergify not to merge this PR label Jul 2, 2026

conradoplg previously approved these changes Jul 2, 2026

View reviewed changes

mpguerra removed the do-not-merge Tells Mergify not to merge this PR label Jul 3, 2026

conradoplg added 2 commits July 9, 2026 14:29

Merge branch 'main' of https://github.com/ZcashFoundation/zebra into …

f17c27b

…validate-value-balance-total

add ironwood to total; fix fake_populated_pool() to work with new bound

8603cdf

Copilot AI review requested due to automatic review settings July 9, 2026 18:18

conradoplg dismissed their stale review via 8603cdf July 9, 2026 18:18

conradoplg temporarily deployed to dev July 9, 2026 18:18 — with GitHub Actions Inactive

conradoplg temporarily deployed to dev July 9, 2026 18:19 — with GitHub Actions Inactive

Copilot started reviewing on behalf of conradoplg July 9, 2026 18:19 View session

conradoplg temporarily deployed to dev July 9, 2026 18:21 — with GitHub Actions Inactive

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread zebra-chain/src/value_balance.rs Outdated

move the check from contrain() to add_chain_value_pool_change()

591d450

Copilot AI review requested due to automatic review settings July 9, 2026 20:04

conradoplg temporarily deployed to dev July 9, 2026 20:04 — with GitHub Actions Inactive

Copilot started reviewing on behalf of conradoplg July 9, 2026 20:04 View session

conradoplg reviewed Jul 9, 2026

View reviewed changes

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread zebra-chain/src/value_balance.rs

Comment thread zebra-chain/src/value_balance.rs Outdated

Comment thread zebra-chain/src/value_balance.rs

conradoplg temporarily deployed to dev July 9, 2026 20:08 — with GitHub Actions Inactive

conradoplg temporarily deployed to dev July 10, 2026 16:16 — with GitHub Actions Inactive

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread zebra-chain/src/value_balance.rs Outdated

Copilot AI review requested due to automatic review settings July 10, 2026 16:16

Copilot started reviewing on behalf of arya2 July 10, 2026 16:17 View session

conradoplg temporarily deployed to dev July 10, 2026 16:18 — with GitHub Actions Inactive

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Merge branch 'main' of https://github.com/ZcashFoundation/zebra into …

b74ad62

…validate-value-balance-total

Copilot AI review requested due to automatic review settings July 10, 2026 21:58

conradoplg temporarily deployed to dev July 10, 2026 21:58 — with GitHub Actions Inactive

conradoplg changed the title ~~fix(chain): validate the total amount when constraining a value balance~~ fix(chain)!: validate the total amount when constraining a value balance Jul 10, 2026

conradoplg temporarily deployed to dev July 10, 2026 21:58 — with GitHub Actions Inactive

Copilot started reviewing on behalf of conradoplg July 10, 2026 21:58 View session

conradoplg temporarily deployed to dev July 10, 2026 22:00 — with GitHub Actions Inactive

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread zebra-chain/CHANGELOG.md Outdated

Comment thread CHANGELOG.md

fix changelog

cdcd963

Copilot AI review requested due to automatic review settings July 10, 2026 22:11

conradoplg temporarily deployed to dev July 10, 2026 22:12 — with GitHub Actions Inactive

Copilot started reviewing on behalf of conradoplg July 10, 2026 22:12 View session

conradoplg temporarily deployed to dev July 10, 2026 22:14 — with GitHub Actions Inactive

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread zebra-chain/src/value_balance.rs

conradoplg changed the title ~~fix(chain)!: validate the total amount when constraining a value balance~~ fix(chain)!: validate the total amount when updating a value balance Jul 10, 2026

hilljessica8787-dot approved these changes Jul 14, 2026

View reviewed changes

Uh oh!

Conversation

arya2 commented Jun 26, 2026 • edited by natalieesk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Testing

AI disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

conradoplg left a comment

Choose a reason for hiding this comment

Uh oh!

conradoplg commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

conradoplg left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

arya2 commented Jun 26, 2026 •

edited by natalieesk

Loading