From 68026dfc039485b023aeac62f450c476f8d8396f Mon Sep 17 00:00:00 2001 From: Matthias Wright Date: Wed, 17 Jun 2026 13:42:49 +0800 Subject: [PATCH 1/3] fix: fail e2e scenarios when a required node dies --- node/src/args.rs | 21 ++++++++++------- node/src/bin/observer.rs | 22 ++++++++++++++---- node/src/bin/protocol_params.rs | 10 ++++++-- node/src/bin/stake_and_checkpoint.rs | 23 +++++++++++++++---- .../bin/stake_and_join_with_outdated_ckpt.rs | 23 +++++++++++++++---- node/src/bin/sync_from_genesis.rs | 21 +++++++++++++---- node/src/bin/testnet.rs | 14 +++++++++-- node/src/bin/verify_consensus_state_proof.rs | 10 ++++++-- node/src/bin/withdraw_and_exit.rs | 10 ++++++-- 9 files changed, 118 insertions(+), 36 deletions(-) diff --git a/node/src/args.rs b/node/src/args.rs index 7e9d2d8e..acaa8f09 100644 --- a/node/src/args.rs +++ b/node/src/args.rs @@ -593,7 +593,7 @@ pub fn run_node_local( flags: RunFlags, checkpoint: Option, checkpoint_parent_block: Option, -) -> Handle<()> { +) -> Handle> { context.spawn(async move |context| { let key_store = expect_key_store(&flags.key_store_path); run_node_local_inner( @@ -603,7 +603,7 @@ pub fn run_node_local( checkpoint, checkpoint_parent_block, ) - .await; + .await }) } @@ -613,7 +613,7 @@ async fn run_node_local_inner( key_store: KeyStore, checkpoint: Option, checkpoint_parent_block: Option, -) { +) -> anyhow::Result<()> { let context = context.with_label("summit_cw"); let genesis = acquire_genesis(&context, &flags).await; @@ -749,14 +749,19 @@ async fn run_node_local_inner( MetricServer::new(config).serve(stop_signal).await.unwrap(); } - // Bring the node down as soon as any core task exits, then return so the runtime - // tears down cleanly and destructors run. Unlike the production path we do NOT - // `process::exit` here: this entrypoint runs inside a caller-managed runtime/thread + // bring the node down as soon as any core task exits, then return so the runtime + // tears down cleanly and destructors run. unlike the production path we do not + // process::exit here: this entrypoint runs inside a caller managed runtime/thread // (testnet and the e2e scenario binaries), and an abrupt exit would skip the caller's - // shutdown — e.g. orphaning the child Reth processes the scenarios spawn. - if let Err(e) = supervise_node_tasks(&context, p2p, engine, rpc_handle).await { + // shutdown, e.g. orphaning the child reth processes the scenarios spawn. instead we + // propagate the supervise outcome so the caller can decide: a coordinated shutdown + // (graceful stop or committee exit) returns ok, while a genuine core task failure + // returns err so the caller can fail the scenario instead of masking a dead node. + let result = supervise_node_tasks(&context, p2p, engine, rpc_handle).await; + if let Err(e) = &result { error!(?e, "node core task failed; shutting down node runtime"); } + result } /// Supervise the core node tasks (P2P, consensus engine, RPC): as soon as any of them diff --git a/node/src/bin/observer.rs b/node/src/bin/observer.rs index c8a32295..a4f86f44 100644 --- a/node/src/bin/observer.rs +++ b/node/src/bin/observer.rs @@ -189,8 +189,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); let stop_fut = stop_rx.recv().fuse(); @@ -296,10 +302,16 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, observer_flags, None, None) + let node_handle = node_context.clone().spawn(move |ctx| async move { + // the observer is a required participant: a genuine core task failure + // (err) must fail the scenario rather than be masked as a clean exit. + if let Err(e) = run_node_local(ctx, observer_flags, None, None) .await - .unwrap(); + .unwrap() + { + eprintln!("observer core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); let stop_fut = observer_stop_rx.recv().fuse(); diff --git a/node/src/bin/protocol_params.rs b/node/src/bin/protocol_params.rs index 6f301c67..6b54ea9b 100644 --- a/node/src/bin/protocol_params.rs +++ b/node/src/bin/protocol_params.rs @@ -177,8 +177,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion diff --git a/node/src/bin/stake_and_checkpoint.rs b/node/src/bin/stake_and_checkpoint.rs index 7ea08b9c..ce7e0c77 100644 --- a/node/src/bin/stake_and_checkpoint.rs +++ b/node/src/bin/stake_and_checkpoint.rs @@ -199,8 +199,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion @@ -424,7 +430,7 @@ fn main() -> Result<(), Box> { // executor.start(|node_context| async move { // let flags = get_node_flags(source_node); - // let node_handle = node_context.clone().spawn(|ctx| async move { + // let node_handle = node_context.clone().spawn(move |ctx| async move { // run_node_with_runtime(ctx, flags, None).await.unwrap(); // }); @@ -527,8 +533,15 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, Some(checkpoint_state), None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // the checkpoint restarted node is a required participant: a genuine + // core task failure (err) must fail the scenario, not be masked. + if let Err(e) = + run_node_local(ctx, flags, Some(checkpoint_state), None).await.unwrap() + { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion diff --git a/node/src/bin/stake_and_join_with_outdated_ckpt.rs b/node/src/bin/stake_and_join_with_outdated_ckpt.rs index 7c3be7f4..2bf7f83c 100644 --- a/node/src/bin/stake_and_join_with_outdated_ckpt.rs +++ b/node/src/bin/stake_and_join_with_outdated_ckpt.rs @@ -202,8 +202,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion @@ -430,7 +436,7 @@ fn main() -> Result<(), Box> { // executor.start(|node_context| async move { // let flags = get_node_flags(source_node); - // let node_handle = node_context.clone().spawn(|ctx| async move { + // let node_handle = node_context.clone().spawn(move |ctx| async move { // run_node_with_runtime(ctx, flags, None).await.unwrap(); // }); @@ -555,8 +561,15 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, Some(checkpoint_state), None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // the checkpoint restarted node is a required participant: a genuine + // core task failure (err) must fail the scenario, not be masked. + if let Err(e) = + run_node_local(ctx, flags, Some(checkpoint_state), None).await.unwrap() + { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion diff --git a/node/src/bin/sync_from_genesis.rs b/node/src/bin/sync_from_genesis.rs index c0468ae6..934092c0 100644 --- a/node/src/bin/sync_from_genesis.rs +++ b/node/src/bin/sync_from_genesis.rs @@ -214,8 +214,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion @@ -536,9 +542,14 @@ address = "{}" let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - // No checkpoint - sync from genesis - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // no checkpoint, sync from genesis. a coordinated shutdown (graceful + // stop or committee exit) returns ok; a genuine core task failure + // returns err and must fail the scenario instead of being masked. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("joining node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); let stop_fut = stop_rx.recv().fuse(); diff --git a/node/src/bin/testnet.rs b/node/src/bin/testnet.rs index ff54847c..72d6f605 100644 --- a/node/src/bin/testnet.rs +++ b/node/src/bin/testnet.rs @@ -18,6 +18,7 @@ use std::{ use alloy_node_bindings::Reth; use clap::Parser; use commonware_runtime::{Metrics as _, Runner as _, Spawner as _, tokio}; +use futures::future::try_join_all; use summit::args::{RunFlags, run_node_local}; #[derive(Parser, Debug)] @@ -163,8 +164,17 @@ fn main() -> Result<(), Box> { // for reader in read_threads { // reader.join().unwrap(); // } - if let Err(e) = futures::future::try_join_all(consensus_handles).await { - tracing::error!("Failed: {:?}", e); + match try_join_all(consensus_handles).await { + Ok(results) => { + // each node now returns its supervise outcome; surface any core task + // failure instead of dropping it on the floor. + for (idx, result) in results.into_iter().enumerate() { + if let Err(e) = result { + tracing::error!(node = idx, ?e, "node core task failed"); + } + } + } + Err(e) => tracing::error!("Failed: {:?}", e), } // Due to how alloy node_bindings work we have to do this to prevent the reth_instances from being dropped and shutdown by the compiler diff --git a/node/src/bin/verify_consensus_state_proof.rs b/node/src/bin/verify_consensus_state_proof.rs index f9b0e73c..90eb7374 100644 --- a/node/src/bin/verify_consensus_state_proof.rs +++ b/node/src/bin/verify_consensus_state_proof.rs @@ -146,8 +146,14 @@ fn main() -> Result<(), Box> { .with_catch_panics(true); let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); let stop_fut = stop_rx.recv().fuse(); pin_mut!(stop_fut); diff --git a/node/src/bin/withdraw_and_exit.rs b/node/src/bin/withdraw_and_exit.rs index 0e9eeb0b..e516fcfe 100644 --- a/node/src/bin/withdraw_and_exit.rs +++ b/node/src/bin/withdraw_and_exit.rs @@ -187,8 +187,14 @@ fn main() -> Result<(), Box> { let executor = cw_tokio::Runner::new(cfg); executor.start(|node_context| async move { - let node_handle = node_context.clone().spawn(|ctx| async move { - run_node_local(ctx, flags, None, None).await.unwrap(); + let node_handle = node_context.clone().spawn(move |ctx| async move { + // a coordinated shutdown (graceful stop or committee exit) returns + // ok; a genuine core task failure returns err and must fail the + // scenario instead of being masked as a clean node exit. + if let Err(e) = run_node_local(ctx, flags, None, None).await.unwrap() { + eprintln!("node {x} core task failed: {e:?}; failing scenario"); + std::process::exit(1); + } }); // Wait for stop signal or node completion From b4507b1b4bd8267cd988f76995ce3e313cfcda25 Mon Sep 17 00:00:00 2001 From: Matthias Wright Date: Wed, 24 Jun 2026 18:26:17 +0800 Subject: [PATCH 2/3] fix: fail e2e scenarios on unexpected node-handle exit or join failure --- node/src/bin/observer.rs | 19 +++++++++-- node/src/bin/stake_and_checkpoint.rs | 33 +++++++++++++++---- .../bin/stake_and_join_with_outdated_ckpt.rs | 26 ++++++++++++--- node/src/bin/sync_from_genesis.rs | 20 +++++++++-- node/src/bin/testnet.rs | 11 +++++-- node/src/bin/verify_consensus_state_proof.rs | 13 ++++++-- node/src/bin/withdraw_and_exit.rs | 14 +++++++- 7 files changed, 113 insertions(+), 23 deletions(-) diff --git a/node/src/bin/observer.rs b/node/src/bin/observer.rs index a4f86f44..da3d3711 100644 --- a/node/src/bin/observer.rs +++ b/node/src/bin/observer.rs @@ -207,7 +207,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // Every validator here is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -322,7 +326,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Observer handle completed"); + // The observer is a required participant; its handle completing + // without a stop signal means it went down unexpectedly, so + // fail the scenario. + eprintln!("Observer handle completed unexpectedly; failing scenario"); + std::process::exit(1); } } }); @@ -487,7 +495,12 @@ fn main() -> Result<(), Box> { println!("Waiting for node index {} to join...", idx); match node_runtime.thread.join() { Ok(_) => println!("Node index {} thread joined successfully", idx), - Err(e) => println!("Node index {} thread join failed: {:?}", idx, e), + // A join failure means the node panicked or was killed: a required + // participant going down unexpectedly, so fail the scenario. + Err(e) => { + eprintln!("Node index {} thread join failed: {:?}", idx, e); + std::process::exit(1); + } } } diff --git a/node/src/bin/stake_and_checkpoint.rs b/node/src/bin/stake_and_checkpoint.rs index ce7e0c77..44605cc4 100644 --- a/node/src/bin/stake_and_checkpoint.rs +++ b/node/src/bin/stake_and_checkpoint.rs @@ -218,7 +218,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // Every node here is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -553,7 +557,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // The checkpoint-restart node is a required participant; its + // handle completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -588,8 +596,11 @@ fn main() -> Result<(), Box> { ); loop { let mut all_ready = true; - //for idx in 0..num_nodes { - // Skip node0 + // node0 is intentionally stopped for checkpoint copying, so it is + // excluded from this progress check; its expected clean shutdown is + // asserted by the thread join below (a join failure fails the + // scenario), and the checkpoint-restart participant is asserted to + // sync above. Every other required node must reach the target height. for idx in 1..num_nodes { let rpc_port = get_node_flags(idx as usize, &e2e_genesis_path_str).rpc_port; match get_latest_height(rpc_port).await { @@ -629,12 +640,17 @@ fn main() -> Result<(), Box> { // Join all node threads outside of async context to avoid runtime drop issues println!("Waiting for all nodes to shut down..."); - // Join the stopped node0 first if it exists + // Join the stopped node0 first if it exists. A thread join failure means the + // node panicked or was killed: that is a required participant going down + // unexpectedly, so fail the scenario instead of logging and continuing. if let Some(node0) = node_runtimes.1 { println!("Joining node0 thread..."); match node0.thread.join() { Ok(_) => println!("Node0 thread joined successfully"), - Err(e) => println!("Node0 thread join failed: {:?}", e), + Err(e) => { + eprintln!("Node0 thread join failed: {:?}", e); + std::process::exit(1); + } } } @@ -643,7 +659,10 @@ fn main() -> Result<(), Box> { println!("Waiting for node index {} to join...", idx); match node_runtime.thread.join() { Ok(_) => println!("Node index {} thread joined successfully", idx), - Err(e) => println!("Node index {} thread join failed: {:?}", idx, e), + Err(e) => { + eprintln!("Node index {} thread join failed: {:?}", idx, e); + std::process::exit(1); + } } } diff --git a/node/src/bin/stake_and_join_with_outdated_ckpt.rs b/node/src/bin/stake_and_join_with_outdated_ckpt.rs index 2bf7f83c..b90def87 100644 --- a/node/src/bin/stake_and_join_with_outdated_ckpt.rs +++ b/node/src/bin/stake_and_join_with_outdated_ckpt.rs @@ -221,7 +221,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // Every node here is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -581,7 +585,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // The checkpoint-restart node is a required participant; its + // handle completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -596,8 +604,10 @@ fn main() -> Result<(), Box> { ); loop { let mut all_ready = true; - //for idx in 0..num_nodes { - // Skip node0 + // node0 is intentionally stopped for checkpoint copying, so it is + // excluded from this progress check; its expected clean shutdown is + // asserted by the node0 thread join below (which panics on failure). + // Every other required node must reach the target height. for idx in 1..num_nodes { let rpc_port = get_node_flags(idx as usize, &e2e_genesis_path_str).rpc_port; match get_latest_epoch(rpc_port).await { @@ -636,7 +646,13 @@ fn main() -> Result<(), Box> { let _ = tokio::task::spawn_blocking(move || { match node_runtime.thread.join() { Ok(_) => println!("Node index {} thread joined successfully", idx), - Err(e) => println!("Node index {} thread join failed: {:?}", idx, e), + // A join failure means the node panicked or was killed: a + // required participant going down unexpectedly, so fail the + // scenario rather than logging and continuing to exit(0). + Err(e) => { + eprintln!("Node index {} thread join failed: {:?}", idx, e); + std::process::exit(1); + } } }).await; } diff --git a/node/src/bin/sync_from_genesis.rs b/node/src/bin/sync_from_genesis.rs index 934092c0..d1ce0b01 100644 --- a/node/src/bin/sync_from_genesis.rs +++ b/node/src/bin/sync_from_genesis.rs @@ -233,7 +233,19 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // The withdrawn validator (last node) is expected to leave the + // committee and shut down cleanly; any other node's handle + // completing here means a required participant went down + // unexpectedly (e.g. a panic), so fail the scenario. + if x == NUM_NODES - 1 { + println!("Node {} (withdrawn) handle completed as expected", x); + } else { + eprintln!( + "Node {} handle completed unexpectedly; failing scenario", + x + ); + std::process::exit(1); + } } } }); @@ -560,7 +572,11 @@ address = "{}" node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // The new syncing node is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); diff --git a/node/src/bin/testnet.rs b/node/src/bin/testnet.rs index 72d6f605..2027f169 100644 --- a/node/src/bin/testnet.rs +++ b/node/src/bin/testnet.rs @@ -166,15 +166,20 @@ fn main() -> Result<(), Box> { // } match try_join_all(consensus_handles).await { Ok(results) => { - // each node now returns its supervise outcome; surface any core task - // failure instead of dropping it on the floor. + // Each node returns its supervise outcome; a core task failure means + // a node went down, so fail the process instead of only logging it. for (idx, result) in results.into_iter().enumerate() { if let Err(e) = result { tracing::error!(node = idx, ?e, "node core task failed"); + std::process::exit(1); } } } - Err(e) => tracing::error!("Failed: {:?}", e), + // A node handle panicked or was cancelled: surface it as a failure. + Err(e) => { + tracing::error!("node task panicked or was cancelled: {:?}", e); + std::process::exit(1); + } } // Due to how alloy node_bindings work we have to do this to prevent the reth_instances from being dropped and shutdown by the compiler diff --git a/node/src/bin/verify_consensus_state_proof.rs b/node/src/bin/verify_consensus_state_proof.rs index 90eb7374..f8206f15 100644 --- a/node/src/bin/verify_consensus_state_proof.rs +++ b/node/src/bin/verify_consensus_state_proof.rs @@ -163,7 +163,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // Every node here is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); @@ -527,7 +531,12 @@ fn main() -> Result<(), Box> { println!("Waiting for node {idx} to join..."); match node_runtime.thread.join() { Ok(_) => println!("Node {idx} thread joined successfully"), - Err(e) => println!("Node {idx} thread join failed: {e:?}"), + // A join failure means the node panicked or was killed: a required + // participant going down unexpectedly, so fail the scenario. + Err(e) => { + eprintln!("Node {idx} thread join failed: {e:?}"); + std::process::exit(1); + } } } diff --git a/node/src/bin/withdraw_and_exit.rs b/node/src/bin/withdraw_and_exit.rs index e516fcfe..f8dc6f77 100644 --- a/node/src/bin/withdraw_and_exit.rs +++ b/node/src/bin/withdraw_and_exit.rs @@ -206,7 +206,19 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // The withdrawn validator (last node) is expected to leave the + // committee and shut down cleanly; any other node's handle + // completing here means a required participant went down + // unexpectedly (e.g. a panic), so fail the scenario. + if x == NUM_NODES - 1 { + println!("Node {} (withdrawn) handle completed as expected", x); + } else { + eprintln!( + "Node {} handle completed unexpectedly; failing scenario", + x + ); + std::process::exit(1); + } } } }); From 018839f44275a0f12eb9b0d9b7e45d0424006dce Mon Sep 17 00:00:00 2001 From: Matthias Wright Date: Fri, 26 Jun 2026 14:06:00 +0800 Subject: [PATCH 3/3] fix: fail e2e scenarios on remaining unguarded node-exit paths --- node/src/bin/protocol_params.rs | 6 ++++- .../bin/stake_and_join_with_outdated_ckpt.rs | 22 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/node/src/bin/protocol_params.rs b/node/src/bin/protocol_params.rs index 6b54ea9b..97cad41d 100644 --- a/node/src/bin/protocol_params.rs +++ b/node/src/bin/protocol_params.rs @@ -196,7 +196,11 @@ fn main() -> Result<(), Box> { node_context.stop(0, Some(Duration::from_secs(30))).await.unwrap(); } _ = node_handle.fuse() => { - println!("Node {} handle completed", x); + // Every node here is a required participant; its handle + // completing without a stop signal means it went down + // unexpectedly, so fail the scenario. + eprintln!("Node {} handle completed unexpectedly; failing scenario", x); + std::process::exit(1); } } }); diff --git a/node/src/bin/stake_and_join_with_outdated_ckpt.rs b/node/src/bin/stake_and_join_with_outdated_ckpt.rs index b90def87..8d4d2545 100644 --- a/node/src/bin/stake_and_join_with_outdated_ckpt.rs +++ b/node/src/bin/stake_and_join_with_outdated_ckpt.rs @@ -381,9 +381,25 @@ fn main() -> Result<(), Box> { // Send stop signal and wait for runtime to shut down gracefully node0_runtime.stop_tx.send(()).expect("Failed to send stop signal"); println!("Waiting for node{} runtime to shut down...", source_node); - let _ = tokio::task::spawn_blocking(move || { - node0_runtime.thread.join().expect("Failed to join node0 thread"); - }).await; + // node0 is intentionally stopped here for checkpoint copying, but its + // shutdown must still be clean: a thread join failure means it panicked + // or was killed, i.e. a required participant going down unexpectedly. + // Treat that as a scenario failure rather than swallowing the join error + // (a panic inside spawn_blocking is otherwise captured into the discarded + // JoinError and masked). + let join_result = + tokio::task::spawn_blocking(move || node0_runtime.thread.join()).await; + match join_result { + Ok(Ok(())) => println!("node{} runtime shut down cleanly", source_node), + Ok(Err(e)) => { + eprintln!("node{source_node} thread join failed: {e:?}; failing scenario"); + std::process::exit(1); + } + Err(e) => { + eprintln!("node{source_node} join task failed: {e:?}; failing scenario"); + std::process::exit(1); + } + } // Give OS time to release ports (P2P sockets can take time to close) println!("Waiting for ports to be released...");