fix: Stop calling zcash-cli twice in zebra-checkpoints

Also:
* stop capturing zcash-cli stderr
* check subprocess exit status
* require valid UTF-8 from zcash-cli
* refactor out some repeated code

zebra-utils/src/bin/zebra-checkpoints/main.rs

@@ -11,7 +11,7 @@
 #![deny(missing_docs)]
 #![allow(clippy::try_err)]
 
-use color_eyre::eyre::Result;
+use color_eyre::eyre::{ensure, Result};
 use serde_json::Value;
 use std::process::Stdio;
 use structopt::StructOpt;
@@ -20,6 +20,9 @@ use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
 use zebra_chain::block::BlockHeaderHash;
 use zebra_chain::types::BlockHeight;
 
+#[cfg(unix)]
+use std::os::unix::process::ExitStatusExt;
+
 mod args;
 
 /// We limit the memory usage for each checkpoint, based on the cumulative size of
@@ -39,75 +42,86 @@ fn init_tracing() {
         .init();
 }
 
-/// Add passthrough arguments to `cmd`, if present in `args`.
+/// Return a new `zcash-cli` command, including the `zebra-checkpoints`
-fn passthrough(mut cmd: std::process::Command, args: &args::Args) -> std::process::Command {
+/// passthrough arguments.
+fn passthrough_cmd() -> std::process::Command {
+    let args = args::Args::from_args();
+    let mut cmd = std::process::Command::new(&args.cli);
+
     if !args.zcli_args.is_empty() {
         cmd.args(&args.zcli_args);
     }
     cmd
 }
 
+/// Run `cmd` and return its output as a string.
+fn cmd_output(cmd: &mut std::process::Command) -> Result<String> {
+    // Capture stdout, but send stderr to the user
+    let output = cmd.stderr(Stdio::inherit()).output()?;
+
+    // Make sure the command was successful
+    #[cfg(unix)]
+    ensure!(
+        output.status.success(),
+        "Process failed: exit status {:?}, signal: {:?}",
+        output.status.code(),
+        output.status.signal()
+    );
+    #[cfg(not(unix))]
+    ensure!(
+        output.status.success(),
+        "Process failed: exit status {:?}",
+        output.status.code()
+    );
+
+    // Make sure the output is valid UTF-8
+    let s = String::from_utf8(output.stdout)?;
+    Ok(s)
+}
+
 fn main() -> Result<()> {
     init_tracing();
 
     color_eyre::install()?;
 
-    // create process
+    // get the current block count
-    let args = args::Args::from_args();
+    let mut cmd = passthrough_cmd();
-    let mut cmd = std::process::Command::new(&args.cli);
+    cmd.arg("getblockcount");
-    cmd = passthrough(cmd, &args);
+    // calculate the maximum height
+    let height_limit: BlockHeight = cmd_output(&mut cmd)?.trim().parse()?;
+    let height_limit = height_limit
+        .0
+        .checked_sub(BLOCK_REORG_LIMIT.0)
+        .map(BlockHeight)
+        .expect("zcashd has some mature blocks: wait for zcashd to sync more blocks");
 
     // set up counters
     let mut cumulative_bytes: u64 = 0;
     let mut height_gap: BlockHeight = BlockHeight(0);
 
-    // get the current block count
-    cmd.arg("getblockcount");
-    let mut subprocess = cmd.stdout(Stdio::piped()).spawn().unwrap();
-    let output = cmd.output().unwrap();
-    subprocess.kill()?;
-    let mut requested_height: BlockHeight = String::from_utf8_lossy(&output.stdout)
-        .trim()
-        .parse()
-        .unwrap();
-    requested_height = BlockHeight(
-        requested_height
-            .0
-            .checked_sub(BLOCK_REORG_LIMIT.0)
-            .expect("zcashd has some mature blocks: wait for zcashd to sync more blocks"),
-    );
-
     // loop through all blocks
-    for x in 0..requested_height.0 {
+    for x in 0..height_limit.0 {
         // unfortunatly we need to create a process for each block
-        let mut cmd = std::process::Command::new(&args.cli);
+        let mut cmd = passthrough_cmd();
-        cmd = passthrough(cmd, &args);
 
         // get block data
         cmd.args(&["getblock", &x.to_string()]);
-        let mut subprocess = cmd.stdout(Stdio::piped()).spawn().unwrap();
+        let output = cmd_output(&mut cmd)?;
-        let output = cmd.output().unwrap();
+        // parse json
-        let block_raw = String::from_utf8_lossy(&output.stdout);
+        let v: Value = serde_json::from_str(&output)?;
-
-        // convert raw block to json
-        let v: Value = serde_json::from_str(block_raw.trim())?;
 
         // get the values we are interested in
         let hash: BlockHeaderHash = v["hash"]
             .as_str()
             .map(zebra_chain::utils::byte_reverse_hex)
             .unwrap()
-            .parse()
+            .parse()?;
-            .unwrap();
         let height = BlockHeight(v["height"].as_u64().unwrap() as u32);
         assert!(height <= BlockHeight::MAX);
         assert_eq!(x, height.0);
         let size = v["size"].as_u64().unwrap();
         assert!(size <= zebra_chain::block::MAX_BLOCK_BYTES);
 
-        // kill spawned
-        subprocess.wait()?;
-
         // compute
         cumulative_bytes += size;
         height_gap = BlockHeight(height_gap.0 + 1);