Conversation
|
please test with load with contender just to make sure there's no regression in the caching (notably the latencies for get payload, new payload etc are not affected under load) |
crates/p2p/src/outgoing.rs
Outdated
| } | ||
| } | ||
| debug!( | ||
| info!( |
crates/p2p/src/lib.rs
Outdated
| .. | ||
| } => { | ||
| debug!("connection closed with peer {peer_id}: {cause:?}"); | ||
| info!("connection closed with peer {peer_id}: {cause:?}"); |
crates/p2p/src/behaviour.rs
Outdated
| continue; | ||
| } | ||
|
|
||
| tracing::info!("mDNS discovered peer {peer_id} at {multiaddr}"); |
There was a problem hiding this comment.
Pull Request Overview
This PR implements peer-to-peer synchronization and execution of flashblocks, allowing builder nodes to receive and execute flashblocks from other builders in addition to building their own.
Key changes:
- Added flashblock execution logic for synced blocks with validation against expected block hashes
- Enhanced P2P behavior to automatically dial discovered mDNS peers and prevent duplicate connections
- Added new metrics to distinguish between built and synced flashblocks
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/p2p/src/outgoing.rs | Enhanced error handling for message broadcasting and changed logging level from debug to info |
| crates/p2p/src/lib.rs | Added automatic dialing of mDNS discovered peers and updated event handling to pass swarm reference |
| crates/p2p/src/behaviour.rs | Modified mDNS event handling to dial discovered peers if not already connected |
| crates/op-rbuilder/src/tests/flashblocks.rs | Reformatted test configuration structs (whitespace changes only) |
| crates/op-rbuilder/src/metrics.rs | Added metrics for synced blocks and separated invalid block counters for built vs synced blocks |
| crates/op-rbuilder/src/builders/flashblocks/service.rs | Created syncer context and integrated it with payload handler |
| crates/op-rbuilder/src/builders/flashblocks/payload_handler.rs | Implemented complete flashblock execution and validation logic for synced payloads |
| crates/op-rbuilder/src/builders/flashblocks/payload.rs | Updated metrics initialization and reference to use shared instance |
| crates/op-rbuilder/src/builders/flashblocks/mod.rs | Added ctx module declaration |
| crates/op-rbuilder/src/builders/flashblocks/ctx.rs | New file defining syncer context for flashblock execution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
crates/op-rbuilder/src/builders/flashblocks/payload_handler.rs:1
- The TODO comment indicates uncertainty about cancellation token usage. If the token is truly unused due to reth's task_executor, consider documenting why it's still being created and passed through the system, or evaluate if it should be removed.
use crate::{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 Summary
instructions for local testing with builder-playground, 1 builder building and 1 builder syncing:
cd builder-playground && git checkout noot/multi-buildercd op-rbuilder && git checkout noot/p2p-sync && cargo builddocker logs devnet-op-rbuilder-sync-only-1 -fyou should see logs showing that the nodes connected and that flashblocks are syncing and successfully executing and being added to the canonical chain:
💡 Motivation and Context
--- see #275
✅ I have completed the following steps:
make lintmake test