-
Notifications
You must be signed in to change notification settings - Fork 357
Das custody backfiller (DASCustodySync rewrite) #10159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
...sition/src/main/java/tech/pegasys/teku/statetransition/datacolumns/DasCustodyBackfiller.java
Fixed
Show fixed
Hide fixed
6185c8e to
942263b
Compare
942263b to
684b9fe
Compare
...sition/src/main/java/tech/pegasys/teku/statetransition/datacolumns/DasCustodyBackfiller.java
Show resolved
Hide resolved
| __ -> { | ||
| if (slot.isLessThanOrEqualTo(batchData.minCustodyPeriodSlot)) { | ||
| custodyGroupCountManager.setCustodyGroupSyncedCount( | ||
| batchData.requiredColumnsInCustody.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Column count passed instead of group count
The code calls setCustodyGroupSyncedCount(batchData.requiredColumnsInCustody.size()) where requiredColumnsInCustody is the list of column indices from getCustodyColumnIndices(). However, setCustodyGroupSyncedCount expects a custody group count, not a column count. Since each custody group can contain multiple columns (computed as NUMBER_OF_COLUMNS / NUMBER_OF_CUSTODY_GROUPS), these values differ. The method name, parameter name, and metric description ("custody_groups_backfilled") all indicate a group count is expected, not column count.
| __ -> { | ||
| if (slot.isLessThanOrEqualTo(batchData.minCustodyPeriodSlot)) { | ||
| custodyGroupCountManager.setCustodyGroupSyncedCount( | ||
| batchData.requiredColumnsInCustody.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Column count passed instead of group count to setter
The code passes batchData.requiredColumnsInCustody.size() (the column count) to setCustodyGroupSyncedCount, which expects the custody group count. Since requiredColumnsInCustody is populated from getCustodyColumnIndices() and each custody group contains multiple columns, the column count is typically larger than the group count. This will incorrectly set the synced group count to a higher value than intended, potentially preventing future custody re-sync operations from triggering when they should (since onGroupCountUpdate compares against currentSyncCustodyGroupCount, which gets its initial value from getCustodyGroupSyncedCount).
This an alternative to
DASCustodySync.The class is just introduced and not activated yet. Flags and additional DB variable will be added in subsequent PRs.
implements:
1- backfill of custody columns after checkpoint sync
2- at startup, it will check that our current recent blocks have all the custody (to mitigate custody columns not written on disk within the same block transaction)
3- when custody increases (due to increase in stacked ETH) it will make sure it will restart backfill and download the required additional columns
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog
Note
Adds a new DasCustodyBackfiller service to backfill custody columns (with batching, startup head checks, and resync on custody changes) and updates/extends tests to validate behavior.
DasCustodyBackfillerservice: Backfills custody data columns with batched processing, startup heads custody check, cursor management, and resync when custody group count increases.DataColumnSidecars via retriever/custody components.DasCustodyBackfillerTestcovering scheduling, cursor init/movement, head-check batch, missing column retrieval, gaps with no blocks, cancellation on reorg, and completion at min custody slot.DasCheckpointSyncAcceptanceTestto wait for custody backfill; adjust disabled reason.TekuBeaconNode.waitForCustodyBackfill(UInt64, int)helper used by acceptance test.Written by Cursor Bugbot for commit 2163bea. This will update automatically on new commits. Configure here.