From bcfe4b6f3305c4b812df6102c9ba542af69b2459 Mon Sep 17 00:00:00 2001 From: Morty Date: Mon, 12 Jan 2026 19:49:11 +0800 Subject: [PATCH 1/2] feat: penalize peer on duplicate block --- crates/network/src/manager.rs | 39 +++++++++++++++++++++---------- crates/scroll-wire/src/manager.rs | 20 +++++++--------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/crates/network/src/manager.rs b/crates/network/src/manager.rs index f1353ac5..50772dcb 100644 --- a/crates/network/src/manager.rs +++ b/crates/network/src/manager.rs @@ -187,7 +187,7 @@ impl< // Filter the peers that have not seen this block hash. let peers: Vec> = self .scroll_wire - .state() + .block_received_state() .iter() .filter_map(|(peer_id, blocks)| (!blocks.contains(&hash)).then_some(*peer_id)) .collect(); @@ -240,15 +240,23 @@ impl< ScrollWireEvent::NewBlock { peer_id, block, signature } => { let block_hash = block.hash_slow(); trace!(target: "scroll::network::manager", peer_id = ?peer_id, block = ?block_hash, signature = ?signature, "Received new block"); + + // Update the state of the peer cache i.e. peer has seen this block. + self.scroll_wire + .block_received_state_mut() + .entry(peer_id) + .or_insert_with(|| LruCache::new(LRU_CACHE_SIZE)) + .insert(block_hash); + if self.blocks_seen.contains(&(block_hash, signature)) { + // Check if this peer has already sent this block to us, if so penalize it. + if self.scroll_wire.block_received_state().get(&peer_id).map_or(false, |cache| cache.contains(&block_hash)) { + trace!(target: "scroll::network::manager", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block, penalizing"); + self.inner_network_handle + .reputation_change(peer_id, reth_network_api::ReputationChangeKind::BadBlock); + } None } else { - // Update the state of the peer cache i.e. peer has seen this block. - self.scroll_wire - .state_mut() - .entry(peer_id) - .or_insert_with(|| LruCache::new(LRU_CACHE_SIZE)) - .insert(block_hash); // Update the state of the block cache i.e. we have seen this block. self.blocks_seen.insert((block.hash_slow(), signature)); @@ -339,18 +347,25 @@ impl< .and_then(|i| Signature::from_raw(&extra_data[i..]).ok()) { let block_hash = block.hash_slow(); - if self.blocks_seen.contains(&(block_hash, signature)) { - return None; - } - trace!(target: "scroll::bridge::import", peer_id = %peer_id, block_hash = %block_hash, signature = %signature.to_string(), extra_data = %extra_data.to_string(), "Received new block from eth-wire protocol"); // Update the state of the peer cache i.e. peer has seen this block. self.scroll_wire - .state_mut() + .block_received_state_mut() .entry(peer_id) .or_insert_with(|| LruCache::new(LRU_CACHE_SIZE)) .insert(block_hash); + if self.blocks_seen.contains(&(block_hash, signature)) { + // Check if this peer has already sent this block to us, if so penalize it. + if self.scroll_wire.block_received_state().get(&peer_id).map_or(false, |cache| cache.contains(&block_hash)) { + trace!(target: "scroll::bridge::import", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block, penalizing"); + self.inner_network_handle + .reputation_change(peer_id, reth_network_api::ReputationChangeKind::BadBlock); + } + return None; + } + trace!(target: "scroll::bridge::import", peer_id = %peer_id, block_hash = %block_hash, signature = %signature.to_string(), extra_data = %extra_data.to_string(), "Received new block from eth-wire protocol"); + // Update the state of the block cache i.e. we have seen this block. self.blocks_seen.insert((block_hash, signature)); Some(ScrollNetworkManagerEvent::NewBlock(NewBlockWithPeer { diff --git a/crates/scroll-wire/src/manager.rs b/crates/scroll-wire/src/manager.rs index 10a6b1dd..a10843e4 100644 --- a/crates/scroll-wire/src/manager.rs +++ b/crates/scroll-wire/src/manager.rs @@ -25,14 +25,14 @@ pub struct ScrollWireManager { connections: HashMap>, /// A map of the state of the scroll wire protocol. Currently the state for each peer /// is just a cache of the last 100 blocks seen by each peer. - state: HashMap>, + block_received_state: HashMap>, } impl ScrollWireManager { /// Creates a new [`ScrollWireManager`] instance. pub fn new(events: UnboundedReceiver) -> Self { trace!(target: "scroll::wire::manager", "Creating new ScrollWireManager instance"); - Self { events: events.into(), connections: HashMap::new(), state: HashMap::new() } + Self { events: events.into(), connections: HashMap::new(), block_received_state: HashMap::new() } } /// Announces a new block to the specified peer. @@ -42,27 +42,23 @@ impl ScrollWireManager { // connections map and delete its state as the connection is no longer valid. if to_connection.get().send(ScrollMessage::new_block(block.clone())).is_err() { trace!(target: "scroll::wire::manager", peer_id = %peer_id, "Failed to send block to peer - dropping peer."); - self.state.remove(&peer_id); + self.block_received_state.remove(&peer_id); to_connection.remove(); } else { // Upon successful sending of the block we update the state of the peer. trace!(target: "scroll::wire::manager", peer_id = %peer_id, "Announced block to peer"); - self.state - .entry(peer_id) - .or_insert_with(|| LruCache::new(LRU_CACHE_SIZE)) - .insert(hash); } } } /// Returns the state of the `ScrollWire` protocol. - pub const fn state(&self) -> &HashMap> { - &self.state + pub const fn block_received_state(&self) -> &HashMap> { + &self.block_received_state } /// Returns a mutable reference to the state of the `ScrollWire` protocol. - pub const fn state_mut(&mut self) -> &mut HashMap> { - &mut self.state + pub const fn block_received_state_mut(&mut self) -> &mut HashMap> { + &mut self.block_received_state } } @@ -94,7 +90,7 @@ impl Future for ScrollWireManager { direction ); this.connections.insert(peer_id, to_connection); - this.state.insert(peer_id, LruCache::new(100)); + this.block_received_state.insert(peer_id, LruCache::new(100)); } None => break, } From 4a98c535ba61995eaba04ac44ed72a2b86e81fd8 Mon Sep 17 00:00:00 2001 From: Morty Date: Mon, 12 Jan 2026 20:07:32 +0800 Subject: [PATCH 2/2] update makefile --- .github/workflows/lint.yaml | 6 ++++++ .github/workflows/test.yaml | 4 ++-- Makefile | 12 +++++++----- crates/network/src/manager.rs | 28 +++++++++++++++++++++------- crates/scroll-wire/src/manager.rs | 10 +++++++--- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index d4d06330..3f80e053 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -21,6 +21,7 @@ jobs: - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@clippy with: + toolchain: 'nightly-2026-01-05' components: clippy - uses: Swatinem/rust-cache@v2 with: @@ -38,6 +39,7 @@ jobs: - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@nightly with: + toolchain: 'nightly-2026-01-05' components: rustfmt - name: Run fmt run: cargo fmt --all --check @@ -59,6 +61,8 @@ jobs: - uses: actions/checkout@v6 - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@nightly + with: + toolchain: 'nightly-2026-01-05' - uses: Swatinem/rust-cache@v2 with: cache-on-failure: true @@ -88,6 +92,8 @@ jobs: - uses: actions/checkout@v6 - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@nightly + with: + toolchain: 'nightly-2026-01-05' - uses: Swatinem/rust-cache@v2 with: cache-on-failure: true diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 800b624b..92b9fd3c 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,7 +24,7 @@ jobs: - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@nightly with: - toolchain: 'nightly' + toolchain: 'nightly-2026-01-05' - uses: Swatinem/rust-cache@v2 with: cache-on-failure: true @@ -43,7 +43,7 @@ jobs: - uses: rui314/setup-mold@v1 - uses: dtolnay/rust-toolchain@nightly with: - toolchain: 'nightly' + toolchain: 'nightly-2026-01-05' - uses: Swatinem/rust-cache@v2 with: cache-on-failure: true diff --git a/Makefile b/Makefile index f1554d91..7da521fe 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ +NIGHTLY_TOOLCHAIN := nightly-2026-01-05 + .PHONY: fmt fmt: - cargo +nightly fmt + cargo +$(NIGHTLY_TOOLCHAIN) fmt .PHONY: lint-toml lint-toml: ensure-dprint @@ -14,7 +16,7 @@ ensure-dprint: .PHONY: clippy clippy: - cargo +nightly clippy \ + cargo +$(NIGHTLY_TOOLCHAIN) clippy \ --workspace \ --lib \ --examples \ @@ -25,7 +27,7 @@ clippy: .PHONY: clippy-fix clippy-fix: - cargo +nightly clippy \ + cargo +$(NIGHTLY_TOOLCHAIN) clippy \ --workspace \ --lib \ --examples \ @@ -37,7 +39,7 @@ clippy-fix: .PHONY: udeps udeps: - cargo +nightly udeps --workspace --lib --examples --tests --benches --all-features --locked + cargo +$(NIGHTLY_TOOLCHAIN) udeps --workspace --lib --examples --tests --benches --all-features --locked .PHONY: codespell codespell: ensure-codespell @@ -64,7 +66,7 @@ lint: fmt lint-toml clippy udeps codespell zepter .PHONY: test test: - cargo nextest run \ + cargo +$(NIGHTLY_TOOLCHAIN) nextest run \ --workspace \ --locked \ --all-features \ diff --git a/crates/network/src/manager.rs b/crates/network/src/manager.rs index 50772dcb..613691bd 100644 --- a/crates/network/src/manager.rs +++ b/crates/network/src/manager.rs @@ -227,7 +227,7 @@ impl< // Announce block to the filtered set of peers for peer_id in peers { trace!(target: "scroll::network::manager", peer_id = %peer_id, block_number = %block.block.header.number, block_hash = %hash, "Announcing new block to peer"); - self.scroll_wire.announce_block(peer_id, &block, hash); + self.scroll_wire.announce_block(peer_id, &block); } } @@ -250,10 +250,17 @@ impl< if self.blocks_seen.contains(&(block_hash, signature)) { // Check if this peer has already sent this block to us, if so penalize it. - if self.scroll_wire.block_received_state().get(&peer_id).map_or(false, |cache| cache.contains(&block_hash)) { + if self + .scroll_wire + .block_received_state() + .get(&peer_id) + .is_some_and(|cache| cache.contains(&block_hash)) + { trace!(target: "scroll::network::manager", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block, penalizing"); - self.inner_network_handle - .reputation_change(peer_id, reth_network_api::ReputationChangeKind::BadBlock); + self.inner_network_handle.reputation_change( + peer_id, + reth_network_api::ReputationChangeKind::BadBlock, + ); } None } else { @@ -357,10 +364,17 @@ impl< if self.blocks_seen.contains(&(block_hash, signature)) { // Check if this peer has already sent this block to us, if so penalize it. - if self.scroll_wire.block_received_state().get(&peer_id).map_or(false, |cache| cache.contains(&block_hash)) { + if self + .scroll_wire + .block_received_state() + .get(&peer_id) + .is_some_and(|cache| cache.contains(&block_hash)) + { trace!(target: "scroll::bridge::import", peer_id = ?peer_id, block = ?block_hash, "Peer sent duplicate block, penalizing"); - self.inner_network_handle - .reputation_change(peer_id, reth_network_api::ReputationChangeKind::BadBlock); + self.inner_network_handle.reputation_change( + peer_id, + reth_network_api::ReputationChangeKind::BadBlock, + ); } return None; } diff --git a/crates/scroll-wire/src/manager.rs b/crates/scroll-wire/src/manager.rs index a10843e4..44ce413c 100644 --- a/crates/scroll-wire/src/manager.rs +++ b/crates/scroll-wire/src/manager.rs @@ -24,7 +24,7 @@ pub struct ScrollWireManager { /// A map of connections to peers. connections: HashMap>, /// A map of the state of the scroll wire protocol. Currently the state for each peer - /// is just a cache of the last 100 blocks seen by each peer. + /// is just a cache of the last 100 blocks received from each peer. block_received_state: HashMap>, } @@ -32,11 +32,15 @@ impl ScrollWireManager { /// Creates a new [`ScrollWireManager`] instance. pub fn new(events: UnboundedReceiver) -> Self { trace!(target: "scroll::wire::manager", "Creating new ScrollWireManager instance"); - Self { events: events.into(), connections: HashMap::new(), block_received_state: HashMap::new() } + Self { + events: events.into(), + connections: HashMap::new(), + block_received_state: HashMap::new(), + } } /// Announces a new block to the specified peer. - pub fn announce_block(&mut self, peer_id: PeerId, block: &NewBlock, hash: B256) { + pub fn announce_block(&mut self, peer_id: PeerId, block: &NewBlock) { if let Entry::Occupied(to_connection) = self.connections.entry(peer_id) { // We send the block to the peer. If we receive an error we remove the peer from the // connections map and delete its state as the connection is no longer valid.