Skip to content

Comments

Post Fulu BlobSidecars RPC deprecation#9408

Merged
zilm13 merged 10 commits intoConsensys:dasfrom
jeroost:blobssidecarsbyroot-and-by-range-deprecation-support
May 23, 2025
Merged

Post Fulu BlobSidecars RPC deprecation#9408
zilm13 merged 10 commits intoConsensys:dasfrom
jeroost:blobssidecarsbyroot-and-by-range-deprecation-support

Conversation

@jeroost
Copy link
Contributor

@jeroost jeroost commented May 9, 2025

PR Description

Implement the PeerDas consensus spec change here
ethereum/consensus-specs#4286

  • allow BlobSidecarByRoot on non-finalized pre-Fulu activation epoch blocks, ignore post-fulu block requests
  • allow BlobSidecarByRange clipped to the slot before Fulu activation epoch.

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@zilm13 zilm13 self-assigned this May 9, 2025
@zilm13 zilm13 changed the title Post Fulu deprecation Post Fulu BlobSidecars RPC deprecation May 14, 2025
@jeroost jeroost force-pushed the blobssidecarsbyroot-and-by-range-deprecation-support branch from 97f151f to 667b106 Compare May 15, 2025 11:26
@jeroost jeroost marked this pull request as ready for review May 15, 2025 12:10
jeroost added 4 commits May 20, 2025 15:02
…d-by-range-deprecation-support

# Conflicts:
#	networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BlobSidecarsByRangeMessageHandler.java
finalizedSlot,
specConfig.getMaxRequestBlobSidecars());
if (message.getCount().isZero()) {
if (message.getCount().isZero() || !availabilityRequiredAtStart) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about this.
Say, (forget about fulu) we are at epoch 5096, requested slot at epoch 999 with count 100, so we should reply with some blobSidecars starting from epoch 1000, but we will answer with empty response in this case. Also I'd cover this case with a test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're 100% right.
I've updated both code and added a new test for this case. Please have a look

Copy link
Contributor

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you for your help. Great work!

@zilm13 zilm13 merged commit 6e89613 into Consensys:das May 23, 2025
14 checks passed
@zilm13 zilm13 mentioned this pull request May 24, 2025
2 tasks
lucassaldanha pushed a commit to lucassaldanha/teku that referenced this pull request May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants