Skip to content

fix(scanner): decrement Staked by the full balance before zeroing it - #2140

Merged
nullPointerEnjoyer merged 4 commits into
masterfrom
fix/staked-statistic-decommission
Oct 16, 2026
Merged

nullPointerEnjoyer merged 4 commits into
masterfrom
fix/staked-statistic-decommission

Conversation

@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator

Root cause

Both stake-pool spend handlers in api-server/scanner-lib/src/blockchain_state/mod.rs zeroed the stored pool record before reading the pledge for the statistic decrement, so the decrement always subtracted zero:

let pool_data = db_tx.get_pool_data(pool_id).await?.expect(...).decommission_pool();
// ...
decrease_statistic_amount(db_tx, CoinOrTokenStatistic::Staked, &pool_data.pledge_amount(), ...)

PoolData::decommission_pool() zeroes pledge_amount and staker_rewards, so pledge_amount() read after the call is always Amount::ZERO.

Impact

Every pool closure leaves its full stake-at-death in the public staked statistic forever. On mainnet the statistic currently exceeds the real staked supply by ~33M ML (~17%), consistent with ~170 dead pools × ~179k ML.

Fix

A shared helper decommission_pool_and_decrement_staked now captures the pool's staker_balance() (pledge plus accumulated staker rewards — matching what creation and block rewards add to the statistic) before decommissioning, then decrements Staked with it:

  • OutPointSourceId::Transaction branch: pool outputs spent as regular transaction inputs.
  • OutPointSourceId::BlockReward branch: pool outputs spent as block-reward outputs (keeps the existing address-transactions bookkeeping via the returned record's decommission destination).

The stored record is still zeroed exactly as before (decommission_pool semantics unchanged); only the decrement source is corrected. The sync simulation's statistics model was updated to mirror the same behavior (it previously never decremented on pool spends at all).

Test evidence

  • New regression test decommission_and_delegation_spends_update_staked_statistic (api-server/scanner-lib/src/sync/tests/mod.rs): 9-block scenario covering pool-creation pledges, pool spends of both shapes (Transaction-sourced and BlockReward-sourced), the stake-spend maturity lock, a delegation that outlives its pool, and a delegation spend after the pool is gone — with the Staked statistic asserted after every block and decommissioned records checked for zeroed pledge/balance.
  • simulation.rs model updated to decrement on pool spends; the whole sync simulation suite passes against the scanner.
  • cargo test -p pos-accounting -p api-blockchain-scanner-lib -p api-server-backend-test-suite: 46 passed, 0 failed.
  • cargo clippy --all-targets clean; ./do_checks.sh clean.

Known follow-up (out of scope)

Databases that already synced with the previous code keep the permanently overstated statistic — no storage-version bump in this PR, so existing deployments are not forced into a full reindex. Repairing the historical mainnet value needs a reindex/migration decision (or a targeted statistic rebuild) and should be tracked separately.

Delegation accounting is unaffected: the delegation-spend decrement is separate and already correct.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 3 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📋 Routed to summary by policy: 3 comment(s)

bug · low

📄 api-server/scanner-lib/src/sync/tests/simulation.rs (L644-L646)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category bug)

For ProduceBlockFromStake inputs, the full current staker_balance is subtracted from the Staked statistic. The Staked statistic is only incremented by the initial pool pledge (on CreateStakePool) and by per-block subsidy, so if staker_balance can include accumulated staker rewards not tracked in Staked, this subtraction will underflow or produce an incorrect delta. Consider asserting/subtracting only the portion of the balance that the simulation actually tracked, or verifying that the test generator never lets pools accumulate untracked rewards.

💡 Suggested Change

Before:

                TxOutput::CreateStakePool(pool_id, _)
                | TxOutput::ProduceBlockFromStake(_, pool_id)
                    if spent_pools.insert(*pool_id) =>

After:

                TxOutput::CreateStakePool(pool_id, _)
                | TxOutput::ProduceBlockFromStake(_, pool_id)
                    if spent_pools.insert(*pool_id) =>
                {
                    // ensure staker_balance only ever contains pledge + tracked subsidy here
                }

style · low

📄 api-server/scanner-lib/src/sync/tests/simulation.rs (L661-L666)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category style)

The underflow panic message embeds only the pool balance; including the current Staked statistic value would make drift between the simulation's tracking and the real accounting much easier to diagnose when this panic fires.


maintainability · low

📄 api-server/scanner-lib/src/sync/tests/simulation.rs (L467-L468)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

all_inputs_utxos is populated with exactly one entry per transaction in the loop above, so this assert can never fire; conversely, if it ever could be unequal, zip would silently truncate rather than surface the mismatch. The guard is redundant as written — consider either dropping the assert (zip makes the pairing explicit) or iterating with all_inputs_utxos.iter().zip(block.transactions().iter()) semantics that fail loudly, e.g. itertools::zip_eq or an assert! on both lengths, to avoid a silent truncation path.

💡 Suggested Change

Before:

assert_eq!(all_inputs_utxos.len(), block.transactions().len());
for (tx, inputs_utxos) in block.transactions().iter().zip(all_inputs_utxos.iter()) {

After:

itertools::zip_eq(block.transactions().iter(), all_inputs_utxos.iter()).for_each(|(tx, inputs_utxos)| {
    ...

Comment thread api-server/scanner-lib/src/blockchain_state/mod.rs
Comment thread api-server/scanner-lib/src/sync/tests/simulation.rs Outdated
Stake-pool spends zeroed the stored pool record before reading the
pledge for the statistic decrement, so the Staked statistic was never
decremented when a pool was closed. Capture the pool's full stake
balance (pledge plus accumulated staker rewards) before decommissioning
and decrement with that, matching what creation and block rewards add.

Both spend paths are covered: pool outputs spent as regular transaction
inputs and pool outputs spent as block-reward outputs. The same fix is
mirrored in the sync simulation's statistics model.

Regression test covers decommissioning all three spend shapes with the
statistic checked after every block, including a delegation that
outlives its pool.
@erubboli
erubboli force-pushed the fix/staked-statistic-decommission branch from 1b2eb92 to 2b6d134 Compare October 15, 2026 19:36
Comment thread api-server/scanner-lib/src/sync/tests/mod.rs
Comment thread api-server/scanner-lib/src/blockchain_state/mod.rs Outdated
@nullPointerEnjoyer
nullPointerEnjoyer merged commit 26f03d6 into master Oct 16, 2026
21 checks passed
@nullPointerEnjoyer
nullPointerEnjoyer deleted the fix/staked-statistic-decommission branch October 16, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants