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

[nullPointerEnjoyer]

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]

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

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

---

📄 api-server/scanner-lib/src/sync/tests/mod.rs (L996-L1002)

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

The expected_staked recomputation accumulates the same chained additions with a growing nest of .expect("no overflow in test amounts") at every block checkpoint. A tiny folding helper (e.g., fn sum_amounts([...]) -> Amount returning a single contextual expect) would shrink this repeated boilerplate and make each checkpoint read as one statement.

💡 Suggested Change

Before:

    let pool_id_balance = pool_staker_balance(&tf, pool_id);
    expected_staked = (((pool_id_balance + amount2).expect("no overflow in test amounts")
        + amount3)
        .expect("no overflow in test amounts")
        + amount4)
        .expect("no overflow in test amounts");
    assert_eq!(staked_statistic(&local_state).await, expected_staked);

After:

    let pool_id_balance = pool_staker_balance(&tf, pool_id);
    expected_staked = sum_amounts([pool_id_balance, amount2, amount3, amount4]);
    assert_eq!(staked_statistic(&local_state).await, expected_staked);

---

📄 api-server/scanner-lib/src/sync/tests/mod.rs (L1196-L1200)

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

Block 9 spends the delegation balance via an account input using delegated as the amount, but nothing asserts the recorded delegation balance in the chainstate equals delegated beforehand. If the recorded value ever diverged, the failure would surface as an opaque chainstate validity error rather than a clear assertion. A quick pre-check of the delegation's balance in local_state before the spend would give a targeted diagnostic.

💡 Suggested Change

Before:

    // Block 9: the delegation is spent even though pool 3 no longer exists, and the
    // statistic must be decremented by exactly the delegated amount.
    let maturity = chain_config
        .staking_pool_spend_maturity_block_count(tf.next_block_height())
        .to_int();

After:

    // Block 9: the delegation is spent even though pool 3 no longer exists, and the
    // statistic must be decremented by exactly the delegated amount.
    // (Optionally pre-assert the delegation balance in local_state equals `delegated`
    // so a mismatch fails with a clear message instead of an opaque spend error.)
    let maturity = chain_config
        .staking_pool_spend_maturity_block_count(tf.next_block_height())
        .to_int();

---

📄 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 style)

This assert can never fail: all_inputs_utxos is built by pushing exactly one entry per transaction in the loop above, so the length invariant is trivially true. The zip alone is sufficient; the assert is dead code that adds noise.

💡 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:

            for (tx, inputs_utxos) in block.transactions().iter().zip(all_inputs_utxos.iter()) {

---

📄 api-server/scanner-lib/src/sync/tests/simulation.rs (L578-L579)

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

Passing the whole TestFramework only to call get_stake_pool_data couples this statistics helper to the entire test harness and obscures its actual dependency (chainstate pool data). A narrower parameter (e.g. a closure or &dyn chainstate handle) would make the requirement explicit, consistent with the other store adapters (orders_store, pos_store) constructed by callers.

💡 Suggested Change

Before:

    tf: &TestFramework,
    inputs_utxos: &[Option<TxOutput>],

After:

    chainstate: &impl ChainstateInterface,
    inputs_utxos: &[Option<TxOutput>],

---

📄 api-server/scanner-lib/src/sync/tests/mod.rs (L795-L795)

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

Shadowing chain_config here replaces the original ChainConfig value with an Arc clone of the same config the test framework holds. Since the two are interchangeable for the rest of the test (later code calls methods on both), the shadowing makes it harder to track which type is in scope. Consider naming them distinctly, e.g. let chain_config = ...build(); ... let config = Arc::clone(tf.chainstate.get_chain_config()); or cloning the Arc up front.

💡 Suggested Change

Before:

    let chain_config = Arc::clone(tf.chainstate.get_chain_config());

After:

    let chain_config = Arc::clone(tf.chainstate.get_chain_config()); // shadowing the built config; consider a distinct name like `api_chain_config`

[github-actions]

decrease_statistic_amount internally panics via expect("Balance should not underflow"). This change increases the subtracted amount from pledge-only to the full staker_balance (pledge + staker_rewards), so underflow is now more reachable if the Staked statistic is inconsistent with pool state (e.g., after a partial reorg, an interrupted statistic update, or a pre-existing drift). In that case this will panic mid-scan instead of surfacing a recoverable error, unlike the rest of this helper which propagates Result. Consider propagating the subtraction as a typed error (or at least logging the statistic/pool IDs before the panic) so a corrupted statistic is diagnosable rather than a bare arithmetic panic.

Suggestion:

Suggested change

	decrease_statistic_amount(
	db_tx,
	CoinOrTokenStatistic::Staked,
	&staker_balance,
	CoinOrTokenId::Coin,
	block_height,
	)
	.await;
	Ok(pool_data)
	decrease_statistic_amount(
	db_tx,
	CoinOrTokenStatistic::Staked,
	&staker_balance,
	CoinOrTokenId::Coin,
	block_height,
	)
	.await
	.map_err(|error| {
	ApiServerStorageError::LowLevelStorageError(format!(
	"staked statistic underflow decommissioning pool {pool_id}: {error}",
	))
	})?;
	Ok(pool_data)

[github-actions]

The outputs side of this helper only adds the pool pledge to Staked (line ~600), but the input side subtracts the pool's full staker_balance(), which includes staker rewards accrued after pool creation. If a pool has accumulated rewards when its output is spent, the subtraction can exceed the tracked Staked amount, so (*staked - pool_balance).unwrap() panics with an underflow (and get_mut(&CoinOrTokenId::Coin).unwrap() would also panic if the Staked entry was never created). Since this is a statistics-simulation invariant check, it would be safer to handle these cases explicitly (e.g. entry().or_insert(Amount::ZERO) and a saturating/checked subtraction with a descriptive panic message) so a mismatch produces a clear failure instead of an opaque Amount-underflow panic.

Suggestion:

Suggested change

	let staked = statistics
	.entry(CoinOrTokenStatistic::Staked)
	.or_default()
	.get_mut(&CoinOrTokenId::Coin)
	.unwrap();
	*staked = (*staked - pool_balance).unwrap();
	let staked = statistics
	.entry(CoinOrTokenStatistic::Staked)
	.or_default()
	.entry(CoinOrTokenId::Coin)
	.or_insert(Amount::ZERO);
	*staked = (*staked - pool_balance).unwrap_or_else(|_| {
	panic!(
	"Staked statistic underflow spending pool {pool_id:?} \
	(balance {pool_balance})"
	)
	});