Skip to content

Commit 0c21c64

Browse files
jonastheisgreged93
andauthored
feat(e2e tests): invalid signature penalizes peer (#243)
* Add NetworkHandle to RollupManagerCommand * finish can_penalize_peer_for_invalid_block test * fix test after merge * fix linter * fix linter * remove unrelated formatting changes * fix after merge * address review comments * Fixes after merge * implement test can_penalize_peer_for_invalid_signature * Refactor `can_handle_reorgs_while_sequencing` test to also include a follower node and add a bunch of convenience methods * feat: attach L1 block number to ScrollPayloadAttributes * feat: correctly handle L1 reorgs in driver * adjust can_handle_l1_message_reorg test to assert correct reorg conditions * fix linter errors * make sequencer not issue blocks by default in tests * improve test * fixes after merge * improve test * fix test * fixes after merge --------- Co-authored-by: Gregory Edison <[email protected]> Co-authored-by: greg <[email protected]>
1 parent cff3a0a commit 0c21c64

File tree

2 files changed

+164
-4
lines changed

2 files changed

+164
-4
lines changed

crates/node/src/args.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,15 @@ impl ScrollRollupNodeConfig {
301301
};
302302

303303
// Instantiate the signer
304-
let signer = if self.test {
304+
let chain_id = chain_spec.chain().id();
305+
let signer = if let Some(configured_signer) = self.signer_args.signer(chain_id).await? {
306+
// Use the signer configured by SignerArgs
307+
Some(rollup_node_signer::Signer::spawn(configured_signer))
308+
} else if self.test {
305309
// Use a random private key signer for testing
306310
Some(rollup_node_signer::Signer::spawn(PrivateKeySigner::random()))
307311
} else {
308-
// Use the signer configured by SignerArgs
309-
let chain_id = chain_spec.chain().id();
310-
self.signer_args.signer(chain_id).await?.map(rollup_node_signer::Signer::spawn)
312+
None
311313
};
312314

313315
// Instantiate the chain orchestrator

crates/node/tests/e2e.rs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,164 @@ async fn can_penalize_peer_for_invalid_block() {
305305
.await;
306306
}
307307

308+
/// Tests that peers are penalized for broadcasting blocks with invalid signatures.
309+
///
310+
/// This test verifies the network's ability to detect and penalize peers that send
311+
/// blocks with either unauthorized or malformed signatures when using the `SystemContract`
312+
/// consensus algorithm.
313+
///
314+
/// The test proceeds in three phases:
315+
/// 1. **Valid signature verification**: Confirms that blocks signed by the authorized signer are
316+
/// accepted and processed normally without peer penalization.
317+
/// 2. **Unauthorized signer detection**: Sends a block signed by an unauthorized signer and
318+
/// verifies that the sending peer's reputation is decreased.
319+
/// 3. **Invalid signature detection**: Sends a block with a malformed signature and verifies
320+
/// further reputation decrease or peer disconnection.
321+
#[tokio::test]
322+
async fn can_penalize_peer_for_invalid_signature() -> eyre::Result<()> {
323+
reth_tracing::init_test_tracing();
324+
325+
let chain_spec = (*SCROLL_DEV).clone();
326+
327+
// Create two signers - one authorized and one unauthorized
328+
let authorized_signer = PrivateKeySigner::random().with_chain_id(Some(chain_spec.chain().id()));
329+
let authorized_address = authorized_signer.address();
330+
let unauthorized_signer =
331+
PrivateKeySigner::random().with_chain_id(Some(chain_spec.chain().id()));
332+
333+
let mut test_config = default_sequencer_test_scroll_rollup_node_config();
334+
test_config.consensus_args.algorithm = ConsensusAlgorithm::SystemContract;
335+
test_config.consensus_args.authorized_signer = Some(authorized_address);
336+
test_config.signer_args.private_key = Some(authorized_signer.clone());
337+
338+
// Setup nodes
339+
let (mut nodes, _tasks, _) =
340+
setup_engine(test_config, 2, chain_spec.clone(), false, false).await.unwrap();
341+
342+
let node0 = nodes.remove(0);
343+
let node1 = nodes.remove(0);
344+
345+
// Get handles
346+
let node0_rmn_handle = node0.inner.add_ons_handle.rollup_manager_handle.clone();
347+
let node0_network_handle = node0_rmn_handle.get_network_handle().await.unwrap();
348+
let node0_id = node0_network_handle.inner().peer_id();
349+
350+
let node1_rnm_handle = node1.inner.add_ons_handle.rollup_manager_handle.clone();
351+
let node1_network_handle = node1_rnm_handle.get_network_handle().await.unwrap();
352+
353+
// Get event streams
354+
let mut node0_events = node0_rmn_handle.get_event_listener().await.unwrap();
355+
let mut node1_events = node1_rnm_handle.get_event_listener().await.unwrap();
356+
357+
// === Phase 1: Test valid block with correct signature ===
358+
359+
// Have the legitimate sequencer build and sign a block
360+
node0_rmn_handle.build_block().await;
361+
362+
// Wait for the sequencer to build the block
363+
let block0 = if let Some(RollupManagerEvent::BlockSequenced(block)) = node0_events.next().await
364+
{
365+
assert_eq!(block.body.transactions.len(), 0, "Block should have no transactions");
366+
block
367+
} else {
368+
panic!("Failed to receive block from sequencer");
369+
};
370+
371+
// Node1 should receive and accept the valid block
372+
if let Some(RollupManagerEvent::NewBlockReceived(block_with_peer)) = node1_events.next().await {
373+
assert_eq!(block0.hash_slow(), block_with_peer.block.hash_slow());
374+
375+
// Verify the signature is from the authorized signer
376+
let hash = sig_encode_hash(&block_with_peer.block);
377+
let recovered = block_with_peer.signature.recover_address_from_prehash(&hash).unwrap();
378+
assert_eq!(recovered, authorized_address, "Block should be signed by authorized signer");
379+
} else {
380+
panic!("Failed to receive valid block at follower");
381+
}
382+
383+
// Wait for successful import
384+
wait_n_events(&mut node1_events, |e| matches!(e, RollupManagerEvent::BlockImported(_)), 1)
385+
.await;
386+
387+
// === Phase 2: Create and send valid block with unauthorized signer signature ===
388+
389+
// Get initial reputation of node0 from node1's perspective
390+
let initial_reputation =
391+
node1_network_handle.inner().reputation_by_id(*node0_id).await.unwrap().unwrap();
392+
assert_eq!(initial_reputation, 0, "Initial reputation should be zero");
393+
394+
// Create a new block manually (we'll reuse the valid block structure but with wrong signature)
395+
let mut block1 = block0.clone();
396+
block1.header.number += 1;
397+
block1.header.parent_hash = block0.hash_slow();
398+
block1.header.timestamp += 1;
399+
400+
// Sign the block with the unauthorized signer
401+
let block_hash = sig_encode_hash(&block1);
402+
let unauthorized_signature = unauthorized_signer.sign_hash(&block_hash).await.unwrap();
403+
404+
// Send the block with invalid signature from node0 to node1
405+
node0_network_handle.announce_block(block1.clone(), unauthorized_signature);
406+
407+
// Node1 should receive and process the invalid block
408+
wait_for_event_predicate_5s(&mut node1_events, |e| {
409+
if let RollupManagerEvent::NewBlockReceived(block_with_peer) = e {
410+
assert_eq!(block1.hash_slow(), block_with_peer.block.hash_slow());
411+
412+
// Verify the signature is from the unauthorized signer
413+
let hash = sig_encode_hash(&block_with_peer.block);
414+
let recovered = block_with_peer.signature.recover_address_from_prehash(&hash).unwrap();
415+
return recovered == unauthorized_signer.address();
416+
}
417+
false
418+
})
419+
.await?;
420+
421+
eventually(
422+
Duration::from_secs(5),
423+
Duration::from_millis(100),
424+
"Node0 reputation should be lower after sending block with invalid signature",
425+
|| async {
426+
let current_reputation =
427+
node1_network_handle.inner().reputation_by_id(*node0_id).await.unwrap().unwrap();
428+
current_reputation < initial_reputation
429+
},
430+
)
431+
.await;
432+
433+
// === Phase 3: Send valid block with invalid signature ===
434+
// Get current reputation of node0 from node1's perspective
435+
let current_reputation =
436+
node1_network_handle.inner().reputation_by_id(*node0_id).await.unwrap().unwrap();
437+
438+
let invalid_signature = Signature::new(U256::from(1), U256::from(1), false);
439+
440+
// Create a new block with the same structure as before but with an invalid signature.
441+
// We need to make sure the block is different so that it is not filtered.
442+
block1.header.timestamp += 1;
443+
node0_network_handle.announce_block(block1.clone(), invalid_signature);
444+
445+
eventually(
446+
Duration::from_secs(5),
447+
Duration::from_millis(100),
448+
"Node0 reputation should be lower after sending block with invalid signature",
449+
|| async {
450+
let all_peers = node1_network_handle.inner().get_all_peers().await.unwrap();
451+
if all_peers.is_empty() {
452+
return true; // No peers to check, assume penalization and peer0 is blocked and
453+
// disconnected
454+
}
455+
456+
let penalized_reputation =
457+
node1_network_handle.inner().reputation_by_id(*node0_id).await.unwrap().unwrap();
458+
penalized_reputation < current_reputation
459+
},
460+
)
461+
.await;
462+
463+
Ok(())
464+
}
465+
308466
#[allow(clippy::large_stack_frames)]
309467
#[tokio::test]
310468
async fn can_forward_tx_to_sequencer() {

0 commit comments

Comments
 (0)