Skip to content

Breakup RPCBlock into LookupBlock & RangeSyncBlock#8860

Merged
mergify[bot] merged 13 commits into
sigp:unstablefrom
ethDreamer:rpc_block_breakup
Mar 13, 2026
Merged

Breakup RPCBlock into LookupBlock & RangeSyncBlock#8860
mergify[bot] merged 13 commits into
sigp:unstablefrom
ethDreamer:rpc_block_breakup

Conversation

@ethDreamer
Copy link
Copy Markdown
Member

@ethDreamer ethDreamer commented Feb 18, 2026

This should make sync and the DA checker much more clear.

@ethDreamer ethDreamer requested a review from eserilev February 18, 2026 21:01
@ethDreamer ethDreamer requested a review from jxs as a code owner February 18, 2026 21:01
@ethDreamer ethDreamer changed the title Breakup RPCBlock into LookupBlock and RangeSyncBlock Breakup RPCBlock into LookupBlock & RangeSyncBlock Feb 18, 2026
eserilev
eserilev reviewed Feb 22, 2026
View reviewed changes
Comment thread beacon_node/beacon_chain/src/test_utils.rs
eserilev
eserilev reviewed Feb 22, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@eserilev eserilev left a comment

Choose a reason for hiding this comment

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

This looks great. Really makes things much clearer imo

I just had a few questions about a few test functions

Comment thread beacon_node/beacon_chain/src/test_utils.rs
Comment thread beacon_node/beacon_chain/src/test_utils.rs
Comment thread beacon_node/beacon_chain/src/test_utils.rs
eserilev
eserilev approved these changes Feb 24, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@eserilev eserilev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

thanks for answering my questions, LGTM!

@ethDreamer ethDreamer added the ready-for-review The code is ready for review label Feb 25, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 11, 2026

This pull request has merge conflicts. Could you please resolve them @ethDreamer? 🙏

@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 Mar 11, 2026
@pawanjay176 pawanjay176 self-assigned this Mar 11, 2026
@ethDreamer ethDreamer added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Mar 12, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 12, 2026

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

@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 Mar 12, 2026
pawanjay176
pawanjay176 approved these changes Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

This is great. I just have minor nits.
I initially thought that RangeSyncBlock should be just SyncBlock because block_lookup uses it too. But I just learnt that block lookups processes blocks and columns separately even for parent lookups.

Would like to get a ✅ from @dapplion as well to make sure this is in line with his plans for gloas lookups as well

Comment thread beacon_node/beacon_chain/src/block_verification_types.rs
/// verification. i.e. this block has all the required data to get verified and imported into fork choice.
///
/// 2. `BlockOnly`: This is a post-deneb block that requires blobs to be considered fully available.
pub struct LookupBlock<E: EthSpec> {
Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 Mar 13, 2026

Choose a reason for hiding this comment

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

Can we add docs to it specifying that its only a wrapper over a signed beacon block? Its useful when reviewing sync code

Comment thread beacon_node/beacon_chain/src/block_verification_types.rs
Comment thread beacon_node/beacon_chain/src/test_utils.rs Outdated
Comment thread beacon_node/beacon_chain/tests/column_verification.rs
pawanjay176
pawanjay176 reviewed Mar 13, 2026
View reviewed changes
Comment thread beacon_node/beacon_chain/src/block_verification_types.rs Outdated
pawanjay176
pawanjay176 approved these changes Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

LGTM. I know that Lion is on board with the idea so gonna go ahead and merge so that we can build other sync PRs on this

@pawanjay176 pawanjay176 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 Mar 13, 2026
@mergify mergify Bot added the queued label Mar 13, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 13, 2026 •
edited
Loading

Merge Queue Status

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

Required conditions to merge

mergify Bot added a commit that referenced this pull request Mar 13, 2026
@mergify
Merge of #8860
1413c73
This was referenced Mar 13, 2026
Closed
Merged
@mergify mergify Bot merged commit 6ca610d into sigp:unstable Mar 13, 2026
37 checks passed
@mergify mergify Bot removed the queued label Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eserilev eserilev eserilev approved these changes

@pawanjay176 pawanjay176 pawanjay176 approved these changes

@jxs jxs Awaiting requested review from jxs jxs is a code owner

@dapplion dapplion Awaiting requested review from dapplion

Assignees

@pawanjay176 pawanjay176

Labels

code-quality ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ethDreamer @eserilev @pawanjay176