Skip to content

Commit cdaca02

Browse files
committed
feat: improve naming
1 parent 2ee4e67 commit cdaca02

4 files changed

Lines changed: 61 additions & 104 deletions

File tree

crates/network/src/manager.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use reth_tokio_util::{EventSender, EventStream};
2020
use rollup_node_primitives::{sig_encode_hash, BlockInfo};
2121
use scroll_alloy_hardforks::ScrollHardforks;
2222
use scroll_wire::{
23-
NewBlock, PeerState, ScrollMessage, ScrollWireConfig, ScrollWireEvent, ScrollWireManager,
23+
NewBlock, ScrollMessage, ScrollWireConfig, ScrollWireEvent, ScrollWireManager,
2424
ScrollWireProtocolHandler, LRU_CACHE_SIZE,
2525
};
2626
use std::sync::Arc;
@@ -262,7 +262,7 @@ impl<
262262
.scroll_wire
263263
.peer_state()
264264
.get(&peer.remote_id)
265-
.is_some_and(|state| state.has_seen(&hash))
265+
.is_some_and(|state| state.contains(&hash))
266266
{
267267
continue;
268268
}
@@ -274,7 +274,7 @@ impl<
274274
.any(|cap| cap == &ScrollMessage::capability())
275275
{
276276
trace!(target: "scroll::network::manager", peer_id = %peer.remote_id, block_number = %block.block.header.number, block_hash = %hash, "Announcing new block to peer via scroll-wire");
277-
self.scroll_wire.announce_block(peer.remote_id, &block, hash);
277+
self.scroll_wire.announce_block(peer.remote_id, &block);
278278
} else {
279279
if should_announce_eth_wire {
280280
trace!(target: "scroll::network::manager", peer_id = %peer.remote_id, block_number = %block.block.header.number, block_hash = %hash, "Announcing new block to peer via eth-wire");
@@ -304,9 +304,9 @@ impl<
304304
.scroll_wire
305305
.peer_state_mut()
306306
.entry(peer_id)
307-
.or_insert_with(|| PeerState::new(LRU_CACHE_SIZE));
307+
.or_insert_with(|| LruCache::new(LRU_CACHE_SIZE));
308308

309-
if state.has_received(&block_hash) {
309+
if state.contains(&block_hash) {
310310
tracing::warn!(target: "scroll::network::manager", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block via scroll-wire, penalizing");
311311
self.inner_network_handle.reputation_change(
312312
peer_id,
@@ -315,7 +315,7 @@ impl<
315315
return None;
316316
}
317317
// Update the state that the peer has received this block
318-
state.insert_received(block_hash);
318+
state.insert(block_hash);
319319

320320
if self.blocks_seen.contains(&(block_hash, signature)) {
321321
None
@@ -422,16 +422,16 @@ impl<
422422
.scroll_wire
423423
.peer_state_mut()
424424
.entry(peer_id)
425-
.or_insert_with(|| PeerState::new(LRU_CACHE_SIZE));
425+
.or_insert_with(|| LruCache::new(LRU_CACHE_SIZE));
426426

427-
if state.has_received(&block_hash) {
427+
if state.contains(&block_hash) {
428428
tracing::warn!(target: "scroll::bridge::import", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block via eth-wire, penalizing");
429429
self.inner_network_handle
430430
.reputation_change(peer_id, reth_network_api::ReputationChangeKind::BadBlock);
431431
return None;
432432
}
433433
// Update the state that the peer has received this block
434-
state.insert_received(block_hash);
434+
state.insert(block_hash);
435435

436436
if self.blocks_seen.contains(&(block_hash, signature)) {
437437
None

crates/node/tests/e2e.rs

Lines changed: 46 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2217,59 +2217,57 @@ where
22172217
// Ok(())
22182218
// }
22192219

2220-
// /// Tests that peers are penalized for sending duplicate blocks via the same protocol
2221-
// /// (eth-wire).
2222-
// ///
2223-
// /// This test verifies the DoS protection mechanism that detects when a peer sends
2224-
// /// the same block multiple times through the eth-wire protocol.
2225-
// ///
2226-
// /// Test flow:
2227-
// /// 1. Node0 sends a block to node1 via eth-wire
2228-
// /// 2. Node0 sends the SAME block again via eth-wire
2229-
// /// 3. Verify that node0's reputation decreases (duplicate detected)
2230-
// #[tokio::test]
2231-
// async fn can_penalize_peer_for_duplicate_block_via_eth_wire() -> eyre::Result<()> {
2232-
// reth_tracing::init_test_tracing();
2220+
/// Tests that peers are penalized for sending duplicate blocks
2221+
///
2222+
/// This test verifies the DoS protection mechanism that detects when a peer sends
2223+
/// the same block multiple times through the eth-wire protocol.
2224+
///
2225+
/// Test flow:
2226+
/// 1. Node0 sends a block to node1 via eth-wire
2227+
/// 2. Node0 sends the SAME block again via eth-wire
2228+
/// 3. Verify that node0's reputation decreases (duplicate detected)
2229+
#[tokio::test]
2230+
async fn can_penalize_peer_for_duplicate_block_via_eth_wire() -> eyre::Result<()> {
2231+
reth_tracing::init_test_tracing();
22332232

2234-
// // Create 2 nodes with eth-scroll bridge enabled (for eth-wire)
2235-
// let mut fixture = TestFixture::builder()
2236-
// .sequencer()
2237-
// .followers(1)
2238-
// .block_time(0)
2239-
// .allow_empty_blocks(true)
2240-
// .with_eth_scroll_bridge(true)
2241-
// .payload_building_duration(1000)
2242-
// .build()
2243-
// .await?;
2233+
// Create 2 nodes with eth-scroll bridge enabled (for eth-wire)
2234+
let mut fixture = TestFixture::builder()
2235+
.sequencer()
2236+
.followers(1)
2237+
.block_time(0)
2238+
.allow_empty_blocks(true)
2239+
.payload_building_duration(1000)
2240+
.build()
2241+
.await?;
22442242

2245-
// // Set the L1 to synced on the sequencer node
2246-
// fixture.l1().for_node(0).sync().await?;
2247-
// fixture.expect_event_on(0).l1_synced().await?;
2243+
// Set the L1 to synced on the sequencer node
2244+
fixture.l1().for_node(0).sync().await?;
2245+
fixture.expect_event_on(0).l1_synced().await?;
22482246

2249-
// // Build a block
2250-
// let block = fixture.build_block().expect_tx_count(0).build_and_await_block().await?;
2247+
// Build a block
2248+
let block = fixture.build_block().expect_tx_count(0).build_and_await_block().await?;
22512249

2252-
// // Wait for node1 to receive the block via eth-wire
2253-
// fixture.expect_event_on(1).new_block_received().await?;
2250+
// Wait for node1 to receive the block
2251+
fixture.expect_event_on(1).new_block_received().await?;
22542252

2255-
// // Check initial reputation of node 0 from node 1's perspective
2256-
// fixture.check_reputation_on(1).of_node(0).await?.equals(0).await?;
2253+
// Check initial reputation of node 0 from node 1's perspective
2254+
fixture.check_reputation_on(1).of_node(0).await?.equals(0).await?;
22572255

2258-
// // Send the same block again via eth-wire (duplicate)
2259-
// fixture
2260-
// .network_on(0)
2261-
// .announce_block(block.clone(), Signature::new(U256::from(1), U256::from(1), false))
2262-
// .await?;
2256+
// Send the same block again (duplicate)
2257+
fixture
2258+
.network_on(0)
2259+
.announce_block(block.clone(), Signature::new(U256::from(1), U256::from(1), false))
2260+
.await?;
22632261

2264-
// // Wait for reputation to decrease due to duplicate block detection
2265-
// fixture
2266-
// .check_reputation_on(1)
2267-
// .of_node(0)
2268-
// .await?
2269-
// .with_timeout(Duration::from_secs(5))
2270-
// .with_poll_interval(Duration::from_millis(10))
2271-
// .eventually_less_than(0)
2272-
// .await?;
2262+
// Wait for reputation to decrease due to duplicate block detection
2263+
fixture
2264+
.check_reputation_on(1)
2265+
.of_node(0)
2266+
.await?
2267+
.with_timeout(Duration::from_secs(5))
2268+
.with_poll_interval(Duration::from_millis(10))
2269+
.eventually_less_than(0)
2270+
.await?;
22732271

2274-
// Ok(())
2275-
// }
2272+
Ok(())
2273+
}

crates/scroll-wire/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ pub use config::ScrollWireConfig;
55

66
mod connection;
77
mod manager;
8-
pub use manager::{PeerState, ScrollWireManager, LRU_CACHE_SIZE};
8+
pub use manager::{ScrollWireManager, LRU_CACHE_SIZE};
99

1010
mod protocol;
1111
pub use protocol::{NewBlock, ScrollMessage, ScrollWireEvent, ScrollWireProtocolHandler};

crates/scroll-wire/src/manager.rs

Lines changed: 5 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,51 +16,15 @@ use tracing::trace;
1616
/// The size of the LRU cache used to track blocks that have been seen by peers.
1717
pub const LRU_CACHE_SIZE: u32 = 100;
1818

19-
/// Tracks block announced and received state for a peer.
20-
#[derive(Debug)]
21-
pub struct PeerState {
22-
/// blocks announced to the peer
23-
announced: LruCache<B256>,
24-
/// blocks received, this is used to penalize peers that send duplicate blocks.
25-
received: LruCache<B256>,
26-
}
27-
28-
impl PeerState {
29-
/// Creates a new `PeerBlockState` with the specified LRU cache capacity.
30-
pub fn new(capacity: u32) -> Self {
31-
Self { announced: LruCache::new(capacity), received: LruCache::new(capacity) }
32-
}
33-
34-
/// Check if peer knows about this block (either received or announced).
35-
pub fn has_seen(&self, hash: &B256) -> bool {
36-
self.announced.contains(hash) || self.received.contains(hash)
37-
}
38-
39-
/// Check if peer has received this block.
40-
pub fn has_received(&self, hash: &B256) -> bool {
41-
self.received.contains(hash)
42-
}
43-
44-
/// Record that this peer has announced this block.
45-
pub fn insert_announced(&mut self, hash: B256) {
46-
self.announced.insert(hash);
47-
}
48-
49-
/// Record that this peer has received this block.
50-
pub fn insert_received(&mut self, hash: B256) {
51-
self.received.insert(hash); // Track for duplicate detection
52-
}
53-
}
54-
5519
/// A manager for the `ScrollWire` protocol.
5620
#[derive(Debug)]
5721
pub struct ScrollWireManager {
5822
/// A stream of [`ScrollWireEvent`]s produced by the scroll wire protocol.
5923
events: UnboundedReceiverStream<ScrollWireEvent>,
6024
/// A map of connections to peers.
6125
connections: HashMap<PeerId, UnboundedSender<ScrollMessage>>,
62-
/// Unified state tracking block state and blocks received from each peer via both protocols.
63-
peer_state: HashMap<PeerId, PeerState>,
26+
/// Tracks block hashes received from each peer for duplicate detection.
27+
peer_state: HashMap<PeerId, LruCache<B256>>,
6428
}
6529

6630
impl ScrollWireManager {
@@ -71,7 +35,7 @@ impl ScrollWireManager {
7135
}
7236

7337
/// Announces a new block to the specified peer.
74-
pub fn announce_block(&mut self, peer_id: PeerId, block: &NewBlock, hash: B256) {
38+
pub fn announce_block(&mut self, peer_id: PeerId, block: &NewBlock) {
7539
if let Entry::Occupied(to_connection) = self.connections.entry(peer_id) {
7640
// We send the block to the peer. If we receive an error we remove the peer from the
7741
// connections map and peer_block_state as the connection is no longer valid.
@@ -81,11 +45,6 @@ impl ScrollWireManager {
8145
to_connection.remove();
8246
} else {
8347
trace!(target: "scroll::wire::manager", peer_id = %peer_id, "Announced block to peer");
84-
// Record that we announced this block to the peer
85-
self.peer_state
86-
.entry(peer_id)
87-
.or_insert_with(|| PeerState::new(LRU_CACHE_SIZE))
88-
.insert_announced(hash);
8948
}
9049
}
9150
}
@@ -96,12 +55,12 @@ impl ScrollWireManager {
9655
}
9756

9857
/// Returns a reference to the peer state map.
99-
pub const fn peer_state(&self) -> &HashMap<PeerId, PeerState> {
58+
pub const fn peer_state(&self) -> &HashMap<PeerId, LruCache<B256>> {
10059
&self.peer_state
10160
}
10261

10362
/// Returns a mutable reference to the peer state map.
104-
pub const fn peer_state_mut(&mut self) -> &mut HashMap<PeerId, PeerState> {
63+
pub const fn peer_state_mut(&mut self) -> &mut HashMap<PeerId, LruCache<B256>> {
10564
&mut self.peer_state
10665
}
10766
}

0 commit comments

Comments
 (0)