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

Implement BlobsSidecarsByRange v1 #6572

Merged
merged 8 commits into from
Dec 13, 2022
Merged

Implement BlobsSidecarsByRange v1 #6572

merged 8 commits into from
Dec 13, 2022

Conversation

StefanBratanov
Copy link
Contributor

@StefanBratanov StefanBratanov commented Dec 8, 2022 •
edited
Loading

PR Description

  • Added new Optional RPC method in BeaconChainMethod
  • Implemented handling logic in BlobsSidecarsByRangeMessageHandler
  • Add peer rate limiting
  • Added new method to SyncSource and added the main implementation (in DefaultEth2Peer). The test fixtures can be implemented when working on the syncing issues.
  • Throttling will be done as part of Throttle BlobsSidecarsByRange requests #6585
  • Validation will be done as part of Implement BlobsSidecarsByRange validation #6584

Fixed Issue(s)

fixes #6571

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.

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 12, 2022
View reviewed changes
...s/teku/spec/datastructures/networking/libp2p/rpc/BlobsSidecarsByRangeRequestMessageTest.java
import org.junit.jupiter.params.provider.MethodSource;
import tech.pegasys.teku.infrastructure.unsigned.UInt64;

class BlobsSidecarsByRangeRequestMessageTest {

Check notice

Code scanning / CodeQL

Unused classes and interfaces

Unused class: BlobsSidecarsByRangeRequestMessageTest is not referenced within this codebase. If not used as an external API it should be removed.
@StefanBratanov StefanBratanov marked this pull request as ready for review December 12, 2022 17:04
@StefanBratanov StefanBratanov force-pushed the blobs_sidecars_by_range branch 2 times, most recently from accf52d to 55c9571 Compare December 13, 2022 06:07
@StefanBratanov StefanBratanov force-pushed the blobs_sidecars_by_range branch from 55c9571 to 3971758 Compare December 13, 2022 12:04
tbenr
tbenr approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

LGTM.
What needs to be done in a subsequent PR (once storage is there) is the return with 3: ResourceUnavailable error when turns out we cant responds due to missing blobs within the availability window

@StefanBratanov
Copy link
Contributor Author

LGTM. What needs to be done in a subsequent PR (once storage is there) is the return with 3: ResourceUnavailable error when turns out we cant responds due to missing blobs within the availability window

Yup. I have that on my local. Will raise a PR tomorrow. Not sure about the implementation but can discuss then.

@StefanBratanov StefanBratanov merged commit d7ead41 into Consensys:master Dec 13, 2022
@StefanBratanov StefanBratanov deleted the blobs_sidecars_by_range branch December 13, 2022 17:25
tbenr pushed a commit to tbenr/teku that referenced this pull request Dec 13, 2022
@StefanBratanov @tbenr
Implement BlobsSidecarsByRange v1 (Consensys#6572)
6af5124
@mehdi-aouadi mehdi-aouadi mentioned this pull request Dec 20, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbenr tbenr tbenr approved these changes

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

Successfully merging this pull request may close these issues.

Implement BlobsSidecarsByRange v1
2 participants
@StefanBratanov @tbenr