Skip to content

Avoid attempting to serve blobs after Fulu fork #7756

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.

Sign up for GitHub

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

Jump to bottom
Merged
mergify merged 30 commits into from
Oct 20, 2025
Merged

Avoid attempting to serve blobs after Fulu fork #7756

mergify merged 30 commits into from
Oct 20, 2025

Conversation

@chong-he
Copy link
Member

@chong-he chong-he commented Jul 18, 2025

@chong-he chong-he requested a review from jxs as a code owner July 18, 2025 01:33
@chong-he chong-he added the work-in-progress PR is a work-in-progress label Jul 18, 2025
@chong-he chong-he changed the title Das blobs by range Do not response to blobs request after Fulu fork Jul 18, 2025
@chong-he chong-he changed the title Do not response to blobs request after Fulu fork Avoid attempting to serve Blobs after Fulu fork Jul 18, 2025
@chong-he chong-he changed the title Avoid attempting to serve Blobs after Fulu fork Avoid attempting to serve blobs after Fulu fork Jul 18, 2025
@chong-he chong-he added ready-for-review The code is ready for review fulu Required for the upcoming Fulu hard fork and removed work-in-progress PR is a work-in-progress labels Jul 22, 2025
@chong-he chong-he requested a review from jimmygchen July 22, 2025 01:15
@jimmygchen jimmygchen mentioned this pull request Jul 22, 2025
Closed
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Jul 28, 2025
View reviewed changes
jimmygchen
jimmygchen requested changes Jul 28, 2025
View reviewed changes
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

Hi @chong-he I've added some comments.
I think it might be worth adding something similar to data columns methods, to avoid block roots retrieval if the slots are before Fulu

lighthouse/beacon_node/network/src/network_beacon_processor/rpc_methods.rs

Lines 1064 to 1085 in befef72

pub fn handle_data_columns_by_range_request_inner(
&self,
peer_id: PeerId,
inbound_request_id: InboundRequestId,
req: DataColumnsByRangeRequest,
) -> Result<(), (RpcErrorResponse, &'static str)> {
debug!(
%peer_id,
count = req.count,
start_slot = req.start_slot,
"Received DataColumnsByRange Request"
);
// Should not send more than max request data columns
if req.max_requested::<T::EthSpec>() > self.chain.spec.max_request_data_column_sidecars {
return Err((
RpcErrorResponse::InvalidRequest,
"Request exceeded `MAX_REQUEST_BLOBS_SIDECARS`",
));
}
let request_start_slot = Slot::from(req.start_slot);

@jimmygchen jimmygchen 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 Jul 28, 2025
jimmygchen
jimmygchen reviewed Oct 16, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Oct 16, 2025
View reviewed changes
jimmygchen
jimmygchen reviewed Oct 16, 2025
View reviewed changes
@jimmygchen jimmygchen 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 Oct 17, 2025
@jimmygchen jimmygchen mentioned this pull request Oct 17, 2025
Closed
7 tasks
@chong-he chong-he 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 Oct 20, 2025
jimmygchen
jimmygchen reviewed Oct 20, 2025
View reviewed changes
jimmygchen
jimmygchen approved these changes Oct 20, 2025
View reviewed changes
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

Great work! Thanks for the patience and thoroughness 🙏

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 20, 2025
@jimmygchen jimmygchen mentioned this pull request Oct 20, 2025
Closed
@chong-he
Copy link
Member Author

chong-he commented Oct 20, 2025

Great work! Thanks for the patience and thoroughness 🙏

Thanks Jimmy. I was waiting for the CI to pass to only reply, thanks for the thorough review ❤️

@mergify mergify bot added the queued label Oct 20, 2025
mergify bot added a commit that referenced this pull request Oct 20, 2025
@mergify
Merge of #7756
a1b8d4c
@mergify mergify bot mentioned this pull request Oct 20, 2025
Closed
6 tasks
mergify bot added a commit that referenced this pull request Oct 20, 2025
@mergify
Merge of #7756
8ef8ab0
@mergify mergify bot merged commit 2b30c96 into sigp:unstable Oct 20, 2025
37 checks passed
@mergify mergify bot removed the queued label Oct 20, 2025
@chong-he chong-he deleted the das-BlobsByRange branch October 20, 2025 07:25
jchavarri pushed a commit to jchavarri/lighthouse that referenced this pull request Oct 21, 2025
@chong-he @jchavarri
Avoid attempting to serve blobs after Fulu fork (sigp#7756)
ed6ca37 
* sigp#7122


  


Co-Authored-By: Tan Chee Keong <[email protected]>

Co-Authored-By: chonghe <[email protected]>
@chong-he chong-he mentioned this pull request Oct 27, 2025
Merged
mergify bot pushed a commit that referenced this pull request Oct 27, 2025
@chong-he
Revise logging in BlobsByRoot requests (#8296)
ba706ce 
#7756 introduces a logging issue, where the relevant log:
https://github.com/sigp/lighthouse/blob/da5b2317205efc72b98cdc922289acefe3f76a13/beacon_node/network/src/network_beacon_processor/rpc_methods.rs#L380-L385

obtains the `block_root` from `slots_by_block_root.keys()`. If the `block_root` is empty (block not found in the data availability checker), then the log will not show any block root:

`DEBUG BlobsByRoot outgoing response processed       peer_id: 16Uiu2HAmCBxs1ZFfsbAfhSA98rUUL8Q1egLPb6WpGdKZxX6HqQYX, block_root: [], returned: 4`

This PR revises to return the `block_root` in the request as a vector of block root


  


Co-Authored-By: Tan Chee Keong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelsproul michaelsproul michaelsproul left review comments

@jimmygchen jimmygchen jimmygchen approved these changes

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

Assignees

No one assigned

Labels

fulu Required for the upcoming Fulu hard fork ready-for-merge This PR is ready to merge. v8.0.0 Q4 2025 Fusaka Mainnet Release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chong-he @jimmygchen @michaelsproul