From 39ad85783b9fe34a704e511542c5c2b6a02bbcc9 Mon Sep 17 00:00:00 2001 From: ratankaliani Date: Tue, 24 Sep 2024 22:39:55 +0000 Subject: [PATCH 1/4] feat: propagate witnessgen error --- utils/host/src/witnessgen.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/utils/host/src/witnessgen.rs b/utils/host/src/witnessgen.rs index 8ba085c6..dbc0e6be 100644 --- a/utils/host/src/witnessgen.rs +++ b/utils/host/src/witnessgen.rs @@ -1,6 +1,5 @@ use anyhow::Result; use std::time::Duration; -use tokio::time::timeout; use kona_host::HostCli; @@ -117,19 +116,23 @@ impl WitnessGenExecutor { /// an error. async fn wait_for_processes(&mut self) -> Result<()> { for child in &mut self.ongoing_processes { - match timeout(self.timeout, child.child.wait()).await { - Ok(Ok(status)) if !status.success() => { - return Err(anyhow::anyhow!("Child process exited with non-zero status")); + println!("Waiting for process to finish"); + tokio::select! { + result = child.child.wait() => { + match result { + Ok(status) if !status.success() => { + return Err(anyhow::anyhow!("Child process exited with non-zero status")); + } + Err(e) => { + return Err(anyhow::anyhow!("Child process error")); + } + _ => {} + } } - Ok(Err(e)) => { - eprintln!("Child process error: {}", e); - return Err(anyhow::anyhow!("Child process error")); - } - Err(_) => { + _ = tokio::time::sleep(self.timeout) => { eprintln!("Child process timed out"); return Err(anyhow::anyhow!("Child process timed out")); } - _ => {} } } Ok(()) From f1bbedf4f2095284e7485438dd374097b598315c Mon Sep 17 00:00:00 2001 From: ratankaliani Date: Tue, 24 Sep 2024 23:01:53 +0000 Subject: [PATCH 2/4] propagate error --- Cargo.toml | 12 ++++++++++++ scripts/prove/build.rs | 2 +- scripts/witnessgen/bin/native_host_runner.rs | 10 ++++++++-- utils/host/src/witnessgen.rs | 17 ++++++++--------- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2f8e8e94..2d2b314e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,18 @@ kona-executor = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c27 kona-client = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c271377fe184729761bcb217f1f60a" } kona-host = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c271377fe184729761bcb217f1f60a" } +# kona-common = { path = "../kona/crates/common" } +# kona-common-proc = { path = "../kona/crates/common-proc" } +# kona-preimage = { path = "../kona/crates/preimage", features = [ +# "rkyv", +# ] } +# kona-primitives = { path = "../kona/crates/primitives" } +# kona-mpt = { path = "../kona/crates/mpt" } +# kona-derive = { path = "../kona/crates/derive", default-features = false } +# kona-executor = { path = "../kona/crates/executor" } +# kona-client = { path = "../kona/bin/client" } +# kona-host = { path = "../kona/bin/host" } + # op-succinct op-succinct-prove = { path = "scripts/prove" } op-succinct-witnessgen = { path = "scripts/witnessgen" } diff --git a/scripts/prove/build.rs b/scripts/prove/build.rs index da56e742..0ec85b55 100644 --- a/scripts/prove/build.rs +++ b/scripts/prove/build.rs @@ -67,5 +67,5 @@ fn main() { // build_zkvm_program("aggregation"); // Note: Don't comment this out, because the Docker program depends on the native host runner // being built. - build_native_host_runner(); + build_native_host_runner(); } diff --git a/scripts/witnessgen/bin/native_host_runner.rs b/scripts/witnessgen/bin/native_host_runner.rs index c4677123..3998cc31 100644 --- a/scripts/witnessgen/bin/native_host_runner.rs +++ b/scripts/witnessgen/bin/native_host_runner.rs @@ -9,9 +9,15 @@ async fn main() -> Result<()> { init_tracing_subscriber(cfg.v)?; if cfg.server { - start_server(cfg).await?; + let res = start_server(cfg).await; + if res.is_err() { + std::process::exit(1); + } } else { - start_server_and_native_client(cfg).await?; + let res = start_server_and_native_client(cfg).await; + if res.is_err() { + std::process::exit(1); + } } println!("Exiting host program."); diff --git a/utils/host/src/witnessgen.rs b/utils/host/src/witnessgen.rs index dbc0e6be..fbeea4f4 100644 --- a/utils/host/src/witnessgen.rs +++ b/utils/host/src/witnessgen.rs @@ -101,12 +101,10 @@ impl WitnessGenExecutor { // after your custom behavior, otherwise you won't be able to terminate "normally". // Wait for all processes to complete. - let result = self.wait_for_processes().await; - let any_failed = result.is_err(); - if any_failed { + if let Some(err) = self.wait_for_processes().await.err() { self.kill_all(binary_name).await?; - Err(anyhow::anyhow!("One or more child processes failed or timed out")) + Err(anyhow::anyhow!("Killed all witness generation processes because one failed. Error: {}", err)) } else { Ok(()) } @@ -121,17 +119,16 @@ impl WitnessGenExecutor { result = child.child.wait() => { match result { Ok(status) if !status.success() => { - return Err(anyhow::anyhow!("Child process exited with non-zero status")); + return Err(anyhow::anyhow!("Witness generation process exited because it failed.")); } Err(e) => { - return Err(anyhow::anyhow!("Child process error")); + return Err(anyhow::anyhow!("Failed to get witness generation process status: {}", e)); } _ => {} } } _ = tokio::time::sleep(self.timeout) => { - eprintln!("Child process timed out"); - return Err(anyhow::anyhow!("Child process timed out")); + return Err(anyhow::anyhow!("Witness generation process timed out.")); } } } @@ -146,7 +143,9 @@ impl WitnessGenExecutor { async fn kill_all(&mut self, binary_name: String) -> Result<()> { // Kill the "native client" processes. for mut child in self.ongoing_processes.drain(..) { - child.child.kill().await?; + if let Ok(None) = child.child.try_wait() { + child.child.kill().await?; + } } // Kill the spawned witness gen program. From 61098ad54ff174ec5d043bf55e06a4ea66e1c747 Mon Sep 17 00:00:00 2001 From: ratankaliani Date: Tue, 24 Sep 2024 23:03:04 +0000 Subject: [PATCH 3/4] fix: propagate teh witness generation errors --- Cargo.toml | 12 ------------ scripts/prove/build.rs | 2 +- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2d2b314e..2f8e8e94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,18 +50,6 @@ kona-executor = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c27 kona-client = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c271377fe184729761bcb217f1f60a" } kona-host = { git = "https://github.com/anton-rs/kona", rev = "5d0e211ca1c271377fe184729761bcb217f1f60a" } -# kona-common = { path = "../kona/crates/common" } -# kona-common-proc = { path = "../kona/crates/common-proc" } -# kona-preimage = { path = "../kona/crates/preimage", features = [ -# "rkyv", -# ] } -# kona-primitives = { path = "../kona/crates/primitives" } -# kona-mpt = { path = "../kona/crates/mpt" } -# kona-derive = { path = "../kona/crates/derive", default-features = false } -# kona-executor = { path = "../kona/crates/executor" } -# kona-client = { path = "../kona/bin/client" } -# kona-host = { path = "../kona/bin/host" } - # op-succinct op-succinct-prove = { path = "scripts/prove" } op-succinct-witnessgen = { path = "scripts/witnessgen" } diff --git a/scripts/prove/build.rs b/scripts/prove/build.rs index 0ec85b55..da56e742 100644 --- a/scripts/prove/build.rs +++ b/scripts/prove/build.rs @@ -67,5 +67,5 @@ fn main() { // build_zkvm_program("aggregation"); // Note: Don't comment this out, because the Docker program depends on the native host runner // being built. - build_native_host_runner(); + build_native_host_runner(); } From 989360ed61edbf6bbc25484e06ba88bdfde8575c Mon Sep 17 00:00:00 2001 From: ratankaliani Date: Tue, 24 Sep 2024 23:05:56 +0000 Subject: [PATCH 4/4] witnessgen --- utils/host/src/witnessgen.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/utils/host/src/witnessgen.rs b/utils/host/src/witnessgen.rs index fbeea4f4..25d5898e 100644 --- a/utils/host/src/witnessgen.rs +++ b/utils/host/src/witnessgen.rs @@ -104,7 +104,10 @@ impl WitnessGenExecutor { if let Some(err) = self.wait_for_processes().await.err() { self.kill_all(binary_name).await?; - Err(anyhow::anyhow!("Killed all witness generation processes because one failed. Error: {}", err)) + Err(anyhow::anyhow!( + "Killed all witness generation processes because one failed. Error: {}", + err + )) } else { Ok(()) }