Skip to content
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

Revert BlobSidecarsByRoot/Range version bump #4077

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Revert BlobSidecarsByRoot/Range version bump #4077

merged 1 commit into from
Jan 9, 2025

Conversation

jtraglia
Copy link
Member

@jtraglia jtraglia commented Jan 8, 2025

This PR changes BlobSidecarsByRoot and BlobSidecarsByRange from v2 back to v1. This was an unnecessary change since only the request/response list lengths have changed. Thank you for noticing this @pawanjay176!

For reference, please see the Eth R&D messages here:

https://discord.com/channels/595666850260713488/598292067260825641/1326382390397898803

Have other clients implemented the v2 blobsbyrange/root rpc protocol for electra https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/p2p-interface.md#blobsidecarsbyroot-v2 ?
seems like the only difference between v1 and v2 versions is the length of the list. Not sure when we typically bump rpc protocol versions but doing this for a length increase seems unnecessary imo. Do other clients think this is good to have?

@jtraglia jtraglia mentioned this pull request Jan 8, 2025
Merged
6 tasks
pawanjay176
pawanjay176 approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@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.

Nice, thanks for the quick turnaround

@jtraglia jtraglia mentioned this pull request Jan 8, 2025
Open
@ralexstokes ralexstokes merged commit 141ad47 into ethereum:dev Jan 9, 2025
23 checks passed
@jtraglia jtraglia deleted the electra-bsbr-v1 branch January 9, 2025 14:32
@philknows philknows mentioned this pull request Jan 9, 2025
Draft
@terencechain terencechain mentioned this pull request Jan 9, 2025
Open
@Tumas Tumas mentioned this pull request Jan 9, 2025
Open
@nflaig nflaig mentioned this pull request Jan 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nflaig nflaig nflaig approved these changes

@terencechain terencechain terencechain approved these changes

@pawanjay176 pawanjay176 pawanjay176 approved these changes

@hwwhww hwwhww hwwhww approved these changes

@ralexstokes ralexstokes ralexstokes approved these changes

@lucassaldanha lucassaldanha lucassaldanha approved these changes

@ppopth ppopth ppopth approved these changes

Assignees
No one assigned
Labels
Electra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jtraglia @ppopth @lucassaldanha @ralexstokes @hwwhww @pawanjay176 @terencechain @nflaig