Skip to content

Gloas range sync#9362

Merged
mergify[bot] merged 42 commits into
sigp:unstablefrom
pawanjay176:gloas-range-sync
Jun 10, 2026
Merged

Gloas range sync#9362
mergify[bot] merged 42 commits into
sigp:unstablefrom
pawanjay176:gloas-range-sync

Conversation

@pawanjay176

@pawanjay176 pawanjay176 commented May 27, 2026

Copy link
Copy Markdown
Member

Issue Addressed

N/A

Proposed Changes

Implement range sync in gloas.
Basically requests blocks and payloads post gloas from the same peer, couples them and sends it for processing.
Does not change sync much at all other than adding the machinery for payloads by range requests.

Main changes are:
RangeSyncBlock which used to be a struct is an enum to account for the Gloas case. This allows a clear separation between gloas and pre-gloas code.
AvailableBlockData now has a BlockInEnvelope variant. This is to clearly indicate the post gloas case. I feel this is simpler to follow compared to NoData variant.

Tries to extract post gloas logic into its own functions so that there is minimal logic change in mainnet range sync behaviour.

This is meant as a stable base on which we can iterate further to make range sync cleaner and for unblocking range sync support on devnet. Some ideas for later is removing the retry mechanism in favour of delegating column fetching to lookup sync which can be done post #9155 and batch signature verifying envelopes.

@pawanjay176 pawanjay176 requested a review from jxs as a code owner May 27, 2026 18:17
@pawanjay176 pawanjay176 added the work-in-progress PR is a work-in-progress label May 27, 2026
@pawanjay176 pawanjay176 added ready-for-review The code is ready for review syncing gloas and removed work-in-progress PR is a work-in-progress labels May 28, 2026
@pawanjay176

Copy link
Copy Markdown
Member Author

Tested on a gloas kurtosis network and mainnet and it works fine. Some cases where we went into optimistic mode because the parent envelope was not downloaded by range sync. Can be handled with #9039
Does not implement backfill, will be done in a future PR.

@mergify

mergify Bot commented May 28, 2026

Copy link
Copy Markdown

Some required checks have failed. Could you please take a look @pawanjay176? 🙏

@mergify mergify Bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 28, 2026

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good on the whole. Had some thoughts about a few things, lmk what you think

Comment thread beacon_node/beacon_chain/src/data_availability_checker.rs Outdated
Comment thread beacon_node/beacon_chain/src/block_verification.rs Outdated
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
Comment thread beacon_node/beacon_chain/src/block_verification.rs
Comment thread beacon_node/beacon_chain/src/block_verification_types.rs Outdated
Comment thread beacon_node/beacon_chain/src/block_verification_types.rs
Comment thread beacon_node/beacon_chain/src/data_availability_checker.rs Outdated
Comment thread beacon_node/beacon_chain/src/block_verification_types.rs
Comment thread beacon_node/beacon_chain/src/block_verification_types.rs
DataColumnSidecarGloas::<E>::max_size(
spec.max_blobs_per_block(current_digest_epoch) as usize
),
DataColumnSidecarFulu::<E>::max_size(max_blobs),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be DataColumnSidecarGloas right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It should be fulu because the check happens based on wall clock based current_digest_epoch. So if we are in gloas and are requesting fulu columns, we start failing because fulu columns are larger than gloas ones.

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is pretty much good to go, just a few minor suggestions

@pawanjay176

Copy link
Copy Markdown
Member Author

Thanks for fixing up stuff @dapplion !

@dapplion dapplion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good to go!

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@eserilev eserilev added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 10, 2026
@mergify mergify Bot added the queued label Jun 10, 2026
@mergify

mergify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 20 minutes 40 seconds in the queue, including 19 minutes 3 seconds running CI.

Waiting for
  • check-success=local-testnet-success
  • check-success=test-suite-success
All conditions
  • check-success=local-testnet-success
  • check-success=test-suite-success
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]

Reason

The merge conditions cannot be satisfied due to failing checks

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

mergify Bot added a commit that referenced this pull request Jun 10, 2026
@mergify mergify Bot added dequeued and removed queued labels Jun 10, 2026
@mergify mergify Bot removed the dequeued label Jun 10, 2026
@eserilev

Copy link
Copy Markdown
Member

@mergify queue

@mergify

mergify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merge Queue Status

🛑 Queue command has been cancelled

@mergify mergify Bot added the queued label Jun 10, 2026
@mergify

mergify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 29 minutes 33 seconds in the queue, including 27 minutes 59 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • check-success=local-testnet-success
  • check-success=test-suite-success

mergify Bot added a commit that referenced this pull request Jun 10, 2026
@mergify mergify Bot merged commit db3192e into sigp:unstable Jun 10, 2026
38 checks passed
@mergify mergify Bot removed the queued label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gloas ready-for-merge This PR is ready to merge. syncing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants