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

[DO NOT MERGE] Use RLNC for block propagation #14813

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

[DO NOT MERGE] Use RLNC for block propagation #14813

wants to merge 16 commits into from

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Jan 20, 2025

Adds a feature flag to allow for block propagation using random linear network coding.

- Add hooks for the new topic "beacon_block_chunk"
- Add protos for the new messages
- Handle the sync package logic of signature validation
- Create native types for the chunks to be handled in the sync package

Still TODO:
- Handle chunks for pending blocks.
- Handle nodes with incoming chunks, avoid verifying signature twice
- Decode the block and send it to the blockchain package
- Finish validation of each incoming chunk including signature
  verification when needed.

Still TODO:
- Handle chunks for pending blocks
- Convert the decoded block to an actual block and send it to the
  blockchain package.
potuz added 2 commits January 20, 2025 18:15
Still TODO

- Handle chunks for pending blocks
- Handle state transition logic
- Handle rebroadcasting
- Handle block production
potuz added 2 commits January 21, 2025 10:31
Also make `ReadOnlyBeaconBlockChunk` actually read only and copy the
return values.
Load the setup from a file and use it across all clients.
The implementation of Broadcast is blocked as it needs to be changed
Comment on lines +183 to +185
if err := s.cfg.p2p.Broadcast(ctx, msg); err != nil {
logrus.WithError(err).Error("could not broadcast chunk")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is wrong, instead of broadcasting the node should send a different chunk to each one of its peers.

@@ -362,6 +416,11 @@ func (vs *Server) blobSidecarsFromUnblindedBlock(block interfaces.SignedBeaconBl
return BuildBlobSidecars(block, rawBlobs, proofs)
}

// broadcastReceiveChunkedBlock broadcasts a chunked block and handles its reception.
func (vs *Server) broadcastReceiveChunkedBlock(ctx context.Context, req *ethpb.ChunkedBeaconBlock, root [32]byte) error {
return nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be implemented like mentioned on gossip: instead of calling broadcast for a single message, each peer needs to get a different chunk.

Comment on lines +173 to +174
// We overwrite the signature in the block to keep it to be the original one in the database
// Unfortunately to do this without reflection we make extra copies of the full block. We could switch over the type instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unfortunate, depending on the cost here we may want to just overwrite directly on msg but not clear what's the best way

@@ -92,8 +92,11 @@ func (s *Service) validateBlob(ctx context.Context, pid peer.ID, msg *pubsub.Mes
return pubsub.ValidationIgnore, err
}

if err := vf.ValidProposerSignature(ctx); err != nil {
return pubsub.ValidationReject, err
// TODO: verify the proposer signature here by fetching it from the chunks cache, or putting the blob in a queue
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to update the logic to put in a pending queue when no chunk has been seen, and if a chunk has been seen, we just need to check that the signature we keep in the node is the same as in the sidecar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ultimately the right solution is to change the BeaconBlockHeader structure to be the one in the chunk, and also change the layout of the BeaconBlockChunkHeader to keep only the commitments roots and not the full commitments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant