Skip to content

fix(epoch): correct missed payload accounting - #849

Merged
qu0b merged 3 commits into
masterfrom
fix/epoch-payload-accounting
Aug 24, 2026
Merged

fix(epoch): correct missed payload accounting#849
qu0b merged 3 commits into
masterfrom
fix/epoch-payload-accounting

Conversation

@qu0b

@qu0b qu0b commented Aug 24, 2026

Copy link
Copy Markdown
Member

Summary

  • exclude late or orphaned post-Gloas payloads from finalized epoch payload counts
  • aggregate the epochs API over exactly the requested epoch by starting at its final slot and filtering the returned range

Reproduction

On glamsterdam devnet-8, slot 78528 in epoch 2454 had a payload revealed at 7.244 seconds, but the successor block skipped its hash. Dora classified the slot payload as orphaned while the epoch aggregate reported proposedpayloads=32 and missedpayloads=0.

With the corrected accounting, the epoch result is proposedpayloads=31 and missedpayloads=1 when rebuilt.

Verification

  • go test ./...
  • go test -race ./indexer/beacon ./handlers/api
  • go vet ./...
  • go build ./...
  • make api-docs

@redpandabot

This comment has been minimized.

@qu0b

qu0b commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Superseded by 83fad12: the historical repair and its completion marker were removed during cleanup. The final PR now contains only the two direct accounting/range fixes.

@redpandabot redpandabot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

The PR correctly fixes post-Gloas payload accounting: buildDbEpoch/buildDbBlock now count a payload as proposed only when the successor chain includes it (late/orphaned reveals become MissedPayloads), a one-shot startup repair rebuilds finalized epoch payload_count from canonical slot payload_status rows, and the epochs API now aggregates exactly the requested epoch instead of pulling in the previous epoch's last slot. I traced the counting logic against the repair query (enum values and ranges match), the one-shot marker's transactionality, and the GetDbBlocksForSlots range across all cache/DB/finalized paths — the fix is correct and the tests are meaningful (real sqlite DB, re-corrupt marker check, devnet regression range). No blockers found.

Issues

  • 🟡 handlers/api/epochs_v1.go:174slotLimit=lastSlot-firstSlot relies on GetDbBlocksForSlots' undocumented +1 behavior — see the thread on that line

Reviewed @ b64d0d9f
"A pessimist is a well-informed optimist." — Hungarian saying

Comment thread handlers/api/epochs_v1.go Outdated
@redpandabot

redpandabot Bot commented Aug 24, 2026

Copy link
Copy Markdown

Summary

The PR fixes two real Gloas/EIP-7732 payload-accounting bugs: /v1/epochs now aggregates exactly the requested epoch (GetDbBlocksForSlots walks down from its first arg and returns slotLimit+1 slots, so passing lastSlot plus a range guard is the correct fix, matching handlers/epoch.go), and buildDbEpoch no longer counts a late/orphaned-reveal payload as proposed, consistent with the slot PayloadStatus rows it already wrote as Orphaned. Both changes are correct and internally consistent; my findings are about what the final diff no longer contains versus the PR summary.

Issues

  • 🟡 indexer/beacon/writedb.go:672PR summary promises a stale-payload-count repair that the final diff dropped — The summary lists "repair stale finalized epoch payload counts ... when Dora starts after Gloas activation" and "updated API docs" among the delivered changes, but neither survives in the final diff (commit 83fad12 removed RepairEpochPayloadCountsOnce from db/epochs.go and reverted the docs). For databases that already persisted Gloas epochs under the old logic, epochs.payload_count stays wrong — the index-page payload participation and /v1/epoch, /v1/epoch/health read it directly, and synchronizer.syncEpoch (indexer/beacon/synchronizer.go:280) skips already-synchronized epochs unless Indexer.ResyncForceUpdate is set, so nothing re-fixes them. The /v1/epochs list itself is fixed even for old data (it reads slot PayloadStatus rows master already wrote as Orphaned), so the gap is narrower than the summary claims, but the summary doesn't match the diff and existing deployments are left stale until a resync.
  • 🟢 handlers/api/epochs_v1.go:164subtle range/payload-count fix ships without tests — The correctness of the range fix depends on GetDbBlocksForSlots's inclusive lower bound returning slotLimit+1 blocks (the guard at line 167 exists only to drop that one extra slot), and the buildDbEpoch change on isPayloadOrphaned being set before persistEpochData. The regression tests added in ae3a159 (epochs_v1_test.go, writedb_test.go, epochs_test.go) were all deleted by the refactor commit, so this off-by-one boundary and the orphaned-payload counting are now untested; a small regression test pinning the epoch 2454 slot range and a late-reveal payload count would lock the fix in.

Reviewed @ 83fad12c
"A pessimist is a well-informed optimist." — Hungarian saying

@qu0b
qu0b enabled auto-merge August 24, 2026 13:04
@qu0b
qu0b merged commit 7e853b6 into master Aug 24, 2026
5 checks passed
@qu0b
qu0b deleted the fix/epoch-payload-accounting branch August 24, 2026 15:05
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