Skip to content

fix(rewarding): serve pendingVoterRewardDelegates on archive readers - #5000

Draft
envestcc wants to merge 1 commit into
masterfrom
fix/4998-pending-pool-historical-read
Draft

envestcc wants to merge 1 commit into
masterfrom
fix/4998-pending-pool-historical-read

Conversation

@envestcc

Copy link
Copy Markdown
Member

Closes #4998.

What was broken

ReadState "PendingVoterRewardDelegates" enumerates the pending pool with an
ordered range scan over the V2 key prefix. An archive node serves a historical
height out of the erigon store, which addresses objects by contract slot rather
than by an ordered iotex key space, so it rejects the scan instead of handing
back a differently-ordered answer.

Every other IIP-59 read state answers historically; only this one did not:

method historical latest
voterRewardDistribution ok ok
pendingVoterReward ok ok
delegateRewardSnapshot ok ok
delegatePayoutAddress ok ok
voterRewardDestination ok ok
pendingVoterRewardDelegates ErrNotSupported ok

The practical effect is that "which delegates held a pending pool at height H"
cannot be answered from a retained archive, which is the one item blocking a
clean archive-retention sign-off for IIP-59.

Approach

Ask the question a different way instead of teaching the store a new one.

Point reads are unaffected by the slot addressing — that is exactly why the five
sibling reads already work — so the same set is recoverable by probing one key
per candidate. A pool is only ever credited under a candidate identifier and
candidate records are not deleted, so the candidate set at that height covers
every delegate that can hold a pool. Results are sorted to preserve the scan's
ascending-key contract.

Three deliberate limits:

  • Only db.ErrNotSupported takes the fallback. A malformed key or a decode
    failure is real damage in state and must reach the caller rather than be
    papered over by a second, weaker enumeration.
  • freezePendingPoolDrainWork is untouched. An era freeze that quietly
    switched enumeration source on a rejected scan would make block validity
    depend on which storage backend a node happens to run, so it must keep failing
    there. (Consensus cannot reach the erigon scan in any case: Mint/Validate
    go through workingSetStoreWithSecondary, whose States delegates to the
    statedb reader; only newReadOnlyWorkingSet swaps in an erigon-only store.
    The guard is there so that stays true if the wiring ever changes.)
  • The erigon stub is untouched. Its refusal is correct — returning a
    differently-ordered answer would be worse than an error.

No state layout change, no storage-layer change, no fork gate.

Cost

The historical read goes from one scan to O(candidates) point reads. It stays
behind the ErrNotSupported check, so latest served from the statedb still
takes the single scan. Note that on an archive node even a tip read goes through
WorkingSetAtHeight and therefore lands on the erigon-only read-only working
set, so it takes the fallback too.

An O(1)-round-trip version is possible later without a fork gate by implementing
keySplitContractStorage.Batch() (delegating suffix-free keys to the fallback
contractObjectStorage) and switching the probe to a single KeysOption read.
Materialising the pool index as its own state object would make both paths O(1)
but changes state layout and needs a fork gate — that belongs in its own
protocol change, not in a bug fix.

Correctness assumptions

Both are stated in the function comment:

  1. A pool is only credited under a candidate identifier. Both credit sites take
    the identity from the poll/staking candidate.
  2. Candidate records are not deleted. csm.delCandidate currently has no
    production caller. If candidate pruning is ever introduced while pools can
    outlive the record, this read would under-report — the consensus path would
    not be affected.

Tests

  • pending_block_reward_read_test.go — four cases against a reader that rejects
    range scans, including one asserting that non-ErrNotSupported errors are
    not swallowed, and one asserting freezePendingPoolDrainWork still halts
    on the same reader.
  • e2etest/iip59_archive_read_test.go — real erigon archive chain. Asserts the
    archive reader's answer equals a direct range scan at tip, and that re-reading
    an older height after the chain advances still returns that height's answer.

The e2e reproduces the original failure: reverting the one-line ReadState
change makes it fail with erigon store does not support ordered range scan.

go build ./...                                          ok
go test ./action/protocol/rewarding/...                 ok
go test ./action/protocol/staking/...                   ok
go test ./state/factory/...                             ok
go test ./e2etest/ -run 'IIP59|Erigon'                  ok

Base branch

Targeting master. The bug predates rc1_2.5.0 (it reproduces on 1c9a36b
too) and the three touched files are byte-identical on both branches, so this
cherry-picks to rc1_2.5.0 cleanly if the release branch wants it.

Found by

Local nightly testing of rc1_2.5.0 for IIP-59 archive-retention acceptance.

🤖 Generated with Claude Code

ReadState "PendingVoterRewardDelegates" enumerates the pending pool by an
ordered range scan over the V2 key prefix. An archive node answers a
historical height out of the erigon store, which addresses objects by
contract slot rather than by an ordered iotex key space and so rejects the
scan outright rather than hand back a differently-ordered answer. Every
other IIP-59 read state answers historically; this one returned
db.ErrNotSupported, which left "which delegates held a pending pool at
height H" unanswerable from a retained archive.

Fall back, on that error alone, to one point read per candidate: a pool is
only ever credited under a candidate identifier and candidate records are
not deleted, so the candidate set at that height covers every delegate that
can hold a pool. Point reads are unaffected by the slot addressing, which
is why the sibling reads already work. Results are sorted to preserve the
scan's ascending-key contract.

The fallback is deliberately narrow. Only db.ErrNotSupported takes it; a
malformed key or a decode failure is real damage in state and must reach
the caller rather than be papered over by a second, weaker enumeration.

freezePendingPoolDrainWork is left alone. An era freeze that quietly
switched enumeration source on a rejected scan would make block validity
depend on which storage backend a node happens to run, so it must keep
failing there. Consensus cannot reach this path in any case:
Mint/Validate go through workingSetStoreWithSecondary, whose States
delegates to the statedb reader, and only newReadOnlyWorkingSet swaps in
an erigon-only store.

Closes #4998

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

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.

[rewarding/archive] pendingVoterRewardDelegates() is unreadable at historical heights on an archive node

1 participant