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.

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

Implement PeerDAS #14129

Draft
wants to merge 106 commits into
base: develop
Choose a base branch
from
Draft

Implement PeerDAS #14129

wants to merge 106 commits into from

Conversation

nalepae
Copy link
Contributor

@nalepae nalepae commented Jun 21, 2024

Remaining tasks:

  • Initial sync: Backfill data columns when starting from a checkpoint.
  • Initial sync: Do not request more than 64 data columns: Request 64 data columns then reconstruct the rest.
  • Implement validator custody.
  • Peer sampling with trailing fork choice: Integrate the output of sampling in the forkchoice?
  • Check why we cannot run initial sync when starting a node during epoch 0, in slot [1, 31]. (Starting a node since genesis works, starting a node from epoch 1 and later works.) Note: This issue may be already here with blobs (without data columns.)
  • Indroduce a nasty peer flag that holds AND/OR (both are important) alterate data columns on RPC responses.
  • Add unit tests where they are not yet added.
  • Replace the cKZG library by the goKZGone (when available) - In progress.
  • ⚠️ Implement number of blobs as a CL parameter. ⚠️
  • Decouple network subnets from Core methods as referenced.
  • Peer sampling: Find more peers if a column has no corresponding peers. (Can do like in internalBroadcastDataColumn)
  • Add PeerDAS specific metrics
  • Clarify usage of --minimum-peers-per-subnet=<n>
  • Should we downscore peer on sampling? (Maybe the peer does not see the block)
  • Should we stop using a peer when failed on sampling? (To avoid DDoS)
  • Check metadataV3 interop with other clients if before EIP-7594
  • Refactor code where GetValidCustodyPeers is used. ==> We need to fetch data from peers that are not super nodes.
  • prysmctl: Implement byRoot and byRange data columns requests
  • Using subnet sampling only: Should, when using data columns by root requests (and also by range, during initial sync), the node request only custody columns, or custody columns + extra subnet sampling columns? If requesting only custody columns, then a strange situation arises: When receiving a block with a extra subnet sampling column missing, then the node will ignore the block. A few sec after, when requesting the blocks and the associated columns, the node won't need the subnet sampling extra columns to import the block.
  • Find why sometimes ENR records are not correctly updated via discovery.
  • Find why, with Grandine in the mix, discv5 iterator.Next is soooo slow.
  • Test with only one bootnode (and not all nodes)
  • By range (and by root) requests: Run verifications for finalized blocks?
  • ByRoot requests: choose all peers like on ByRange requests, not only super nodes.
  • ENR: Are we sure the sequence number cannot go backward after a reboot? (This trigs downscoring on Prysm.)
  • subscribeWithParameters: Should we subscribe to subnets / find peers 1 epoch in advance for getSubnetsToSubscribe and getSubnetsToFindPeersOnly?
  • Subnets unsubscribing: Handle the same way dynamic (fork epoch) and non dynamic (fork epoch + 1) unsubscriptions.
  • Re-implement part of code where DataColumnsAdmissibleCustodyPeers and DataColumnsAdmissibleSubnetSamplingPeers are needed. We don't want to use only "superset peers".
  • What happens if a peer announces less custody groups via Metadata/ENR than the minimal requirement? Ban?

nalepae and others added 11 commits November 27, 2024 10:11
* Add Support For Discovery Of Column Subnets

* Lint for SubnetsPerNode

* Manu's Review

* Change to a better name
* Add Data Column Subscriber

* Add Data Column Vaidator

* Wire all Handlers In

* Fix Build

* Fix Test

* Fix IP in Test

* Fix IP in Test
* Add RPC Handler

* Add Column Requests

* Update beacon-chain/db/filesystem/blob.go

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>

* Update beacon-chain/p2p/rpc_topic_mappings.go

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>

* Manu's Review

* Manu's Review

* Interface Fixes

* mock manager

---------

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>
* Bump `c-kzg-4844` lib to the `das` branch.

* Implement `MerkleProofKZGCommitments`.

* Implement `das-core.md`.

* Use `peerdas.CustodyColumnSubnets` and `peerdas.CustodyColumns`.

* `CustodyColumnSubnets`: Include `i` in the for loop.

* Remove `computeSubscribedColumnSubnet`.

* Remove `peerdas.CustodyColumns` out of the for loop.
* Remove capital letter from error messages.

* `[4]byte` => `[fieldparams.VersionLength]byte`.

* Prometheus: Remove extra `committee`.

They are probably due to a bad copy/paste.

Note: The name of the probe itself is remaining,
to ensure backward compatibility.

* Implement Proposer RPC for data columns.

* Fix TestProposer_ProposeBlock_OK test.

* Remove default peerDAS activation.

* `validateDataColumn`: Workaround to return a `VerifiedRODataColumn`
* Add new DA check

* Exit early in the event no commitments exist.

* Gazelle

* Fix Mock Broadcaster

* Fix Test Setup

* Update beacon-chain/blockchain/process_block.go

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>

* Manu's Review

* Fix Build

---------

Co-authored-by: Manu NALEPA <enalepa@offchainlabs.com>
* Update `consensus_spec_version` to `v1.5.0-alpha.1`.

* `CustodyColumns`: Fix and implement spec tests.

* Make deepsource happy.

* `^uint64(0)` => `math.MaxUint64`.

* Fix `TestLoadConfigFile` test.
@nalepae nalepae force-pushed the peerDAS branch 3 times, most recently from 30a32c4 to d7d370e Compare January 16, 2025 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Blocked by research or external factors peerDAS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants