Skip to content

fix(scanner): point-in-time PoS reads and typed replay errors - #2149

Open
nullPointerEnjoyer wants to merge 6 commits into
mintlayer:masterfrom
nullPointerEnjoyer:fix/scanner-rescan-staker-drift
Open

nullPointerEnjoyer wants to merge 6 commits into
mintlayer:masterfrom
nullPointerEnjoyer:fix/scanner-rescan-staker-drift

Conversation

@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator

Problem

When the scanner reconnects blocks below rows that survived in its history tables, its pool and delegation reads return the latest row for the entity — including rows written by higher (future) blocks. Rewards then get credited on top of future state and the accumulated balances silently drift from the chain on every reconnection of a lower block.

Observed on mainnet 1.4.3: a scanner that reconnected from an earlier common ancestor accumulated duplicated block rewards into a staking pool balance (a +17×subsidy lump at a single height, then frozen balances until the drift burned off), which in turn under-accrued delegation shares. The first on-chain spend exceeding the under-accrued delegation balance hit .expect("not underflow") in Delegation::spend_share and panic-looped the daemon.

Root cause

update_tables_from_consensus_data, prefetch_pool_data, the delegation spend/delegate paths and the decommission helper all read pool/delegation state without an upper height bound, so a replay whose disconnect step failed to purge higher rows (interrupted scan, partial restore) reads state from the future. The panic is the downstream symptom: PoS arithmetic in the scanner storage layer panicked instead of failing typed.

Fix

  1. Point-in-time reads — all PoS state reads performed while connecting a block at height h are now bounded at-or-below reads (get_pool_data_at_or_below_height, get_delegation_at_or_below_height, get_pool_delegations_at_or_below_height), implemented for both postgres and in-memory backends (no default trait methods, so every backend implements them). The postgres bounded delegation-shares query scans the id set from the latest-state cache and resolves each row with a primary-key point lookup in the history table (no new index or schema change). Replaying any range now reproduces the same history as a clean scan.
  2. Typed errors — Delegation::spend_share and PoolDataWithExtraInfo::decrease_delegation_balance return Result<_, PosError> instead of panicking on underflow; the scanner propagates them via a new BlockchainStateError::PosAccountingError variant, so a corrupt-balance spend fails the scan instead of crash-looping the daemon.
  3. Storage version 27 → 28 — deployments with already-drifted balances re-initialize and rescan under the existing version-mismatch policy, which also repairs the drift.

Test plan

  • New regression test connecting_block_ignores_future_pool_rows: connects blocks against a clean replay and a replay seeded with a future pool row, asserts identical results (fails with +10_000 coins on the unbounded reads — verified red/green).
  • Unit tests for spend_share/decrease_delegation_balance underflow → typed error, and exact-balance spend bumping the nonce.
  • Backend-generic storage test point_in_time_pool_and_delegation_reads covering pool/delegation/shares reads at-or-below heights plus rollback visibility, run against both in-memory (115/115) and containerized postgres (8/8) suites.
  • Existing scanner sync tests unchanged and passing (12/12); cargo check --workspace --tests and clippy clean on touched crates.

Fee calculation and pool decommission now read pool state as of the
connected height, and the point-in-time delegation shares lookup uses a
lateral join over the latest-state cache with primary-key history
lookups instead of scanning the delegation history of the pool.
@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator Author

CI context: static_checks_{ubuntu,windows,macos} (cargo-deny dependency advisories) and wasm_with_nodejs (20.x) fail identically on the base commit 56cfeb6 (see its check runs) — this PR touches no manifests or lockfiles. All other checks pass on this branch, including builds on all three OSes, loom, wasm artifacts and the device test suites.

@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator Author

Note: the 4 failing checks on this PR are inherited from master and are fixed by two separate small PRs: #2150 (dependency bumps for the cargo-deny advisories behind static_checks_*) and #2151 (wasm test tooling path behind wasm_with_nodejs). Once those land this branch will be rebased and should go fully green.

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.

1 participant