change(utils): Add a direct connection mode to zebra-checkpoints (#6516)

* Rename variables so it's clearer what they do

* Fully document zebra-checkpoints arguments

* Remove some outdated references to zcashd

* Add a json-conversion feature for converting JSON to valid Heights

* Simplify zebra-checkpoints code using conversion methods

* Track the last checkpoint height rather than the height gap

* Move all the CLI-specific code into a single function

* Remove cfg(feature) from the test RPC client API

* Move the RpcRequestClient into zebra-node-services

* Fix the case of RpcRequestClient

* Add transport and addr arguments to zebra-checkpoints

* Make zebra-checkpoints support both CLI and direct JSON-RPC connections

* Fix RpcRequestClient compilation

* Add a suggestion for zcashd authentication failures

* Document required features

* Handle differences in CLI & direct parameter and response formats

* Replace a custom function with an existing dependency function

* Add a checkpoint list test for minimum height gaps

Cargo.lock

@@ -5924,6 +5924,7 @@ dependencies = [
  "itertools",
  "jubjub 0.10.0",
  "lazy_static",
+ "num-integer",
  "orchard 0.4.0",
  "primitive-types",
  "proptest",
@@ -5938,6 +5939,7 @@ dependencies = [
  "secp256k1",
  "serde",
  "serde-big-array",
+ "serde_json",
  "serde_with 2.3.2",
  "sha2 0.9.9",
  "spandoc",
@@ -5982,6 +5984,7 @@ dependencies = [
  "jubjub 0.10.0",
  "lazy_static",
  "metrics",
+ "num-integer",
  "once_cell",
  "orchard 0.4.0",
  "proptest",
@@ -6049,6 +6052,11 @@ dependencies = [
 name = "zebra-node-services"
 version = "1.0.0-beta.23"
 dependencies = [
+ "color-eyre",
+ "jsonrpc-core",
+ "reqwest",
+ "serde",
+ "serde_json",
  "zebra-chain",
 ]
 
@@ -6170,6 +6178,7 @@ version = "1.0.0-beta.23"
 dependencies = [
  "color-eyre",
  "hex",
+ "itertools",
  "regex",
  "reqwest",
  "serde_json",
@@ -6217,7 +6226,6 @@ dependencies = [
  "rand 0.8.5",
  "rayon",
  "regex",
- "reqwest",
  "semver 1.0.17",
  "sentry",
  "serde",

zebra-chain/Cargo.toml

@@ -12,6 +12,11 @@ default = []
 
 # Production features that activate extra functionality
 
+# Consensus-critical conversion from JSON to Zcash types
+json-conversion = [
+    "serde_json",
+]
+
 # Experimental mining RPC support
 getblocktemplate-rpcs = [
     "zcash_address",
@@ -45,6 +50,7 @@ group = "0.13.0"
 incrementalmerkletree = "0.3.1"
 jubjub = "0.10.0"
 lazy_static = "1.4.0"
+num-integer = "0.1.45"
 primitive-types = "0.11.1"
 rand_core = "0.6.4"
 ripemd = "0.1.3"
@@ -88,6 +94,9 @@ ed25519-zebra = "3.1.0"
 redjubjub = "0.7.0"
 reddsa = "0.5.0"
 
+# Production feature json-conversion
+serde_json = { version = "1.0.95", optional = true }
+
 # Experimental feature getblocktemplate-rpcs
 zcash_address = { version = "0.2.1", optional = true }
 

zebra-chain/src/block.rs

@@ -35,7 +35,7 @@ pub use commitment::{
 };
 pub use hash::Hash;
 pub use header::{BlockTimeError, CountedHeader, Header, ZCASH_BLOCK_VERSION};
-pub use height::{Height, HeightDiff};
+pub use height::{Height, HeightDiff, TryIntoHeight};
 pub use serialize::{SerializedBlock, MAX_BLOCK_BYTES};
 
 #[cfg(any(test, feature = "proptest-impl"))]

zebra-chain/src/block/height.rs

@@ -2,7 +2,10 @@
 
 use std::ops::{Add, Sub};
 
-use crate::serialization::SerializationError;
+use crate::{serialization::SerializationError, BoxError};
+
+#[cfg(feature = "json-conversion")]
+pub mod json_conversion;
 
 /// The length of the chain back to the genesis block.
 ///
@@ -70,6 +73,9 @@ impl Height {
 /// even if they are outside the valid height range (for example, in buggy RPC code).
 pub type HeightDiff = i64;
 
+// We don't implement TryFrom<u64>, because it causes type inference issues for integer constants.
+// Instead, use 1u64.try_into_height().
+
 impl TryFrom<u32> for Height {
     type Error = &'static str;
 
@@ -84,6 +90,47 @@ impl TryFrom<u32> for Height {
     }
 }
 
+/// Convenience trait for converting a type into a valid Zcash [`Height`].
+pub trait TryIntoHeight {
+    /// The error type returned by [`Height`] conversion failures.
+    type Error;
+
+    /// Convert `self` to a `Height`, if possible.
+    fn try_into_height(&self) -> Result<Height, Self::Error>;
+}
+
+impl TryIntoHeight for u64 {
+    type Error = BoxError;
+
+    fn try_into_height(&self) -> Result<Height, Self::Error> {
+        u32::try_from(*self)?.try_into().map_err(Into::into)
+    }
+}
+
+impl TryIntoHeight for usize {
+    type Error = BoxError;
+
+    fn try_into_height(&self) -> Result<Height, Self::Error> {
+        u32::try_from(*self)?.try_into().map_err(Into::into)
+    }
+}
+
+impl TryIntoHeight for str {
+    type Error = BoxError;
+
+    fn try_into_height(&self) -> Result<Height, Self::Error> {
+        self.parse().map_err(Into::into)
+    }
+}
+
+impl TryIntoHeight for String {
+    type Error = BoxError;
+
+    fn try_into_height(&self) -> Result<Height, Self::Error> {
+        self.as_str().try_into_height()
+    }
+}
+
 // We don't implement Add<u32> or Sub<u32>, because they cause type inference issues for integer constants.
 
 impl Sub<Height> for Height {

zebra-chain/src/block/height/json_conversion.rs

@@ -0,0 +1,24 @@
+//! Consensus-critical conversion from JSON [`Value`] to [`Height`].
+
+use serde_json::Value;
+
+use crate::BoxError;
+
+use super::{Height, TryIntoHeight};
+
+impl TryIntoHeight for Value {
+    type Error = BoxError;
+
+    fn try_into_height(&self) -> Result<Height, Self::Error> {
+        if self.is_number() {
+            let height = self.as_u64().ok_or("JSON value outside u64 range")?;
+            return height.try_into_height();
+        }
+
+        if let Some(height) = self.as_str() {
+            return height.try_into_height();
+        }
+
+        Err("JSON value must be a number or string".into())
+    }
+}

zebra-chain/src/transaction/unmined/zip317.rs

@@ -4,6 +4,8 @@
 
 use std::cmp::max;
 
+use num_integer::div_ceil;
+
 use crate::{
     amount::{Amount, NonNegative},
     block::MAX_BLOCK_BYTES,
@@ -137,19 +139,3 @@ fn conventional_actions(transaction: &Transaction) -> u32 {
 
     max(GRACE_ACTIONS, logical_actions)
 }
-
-/// Divide `quotient` by `divisor`, rounding the result up to the nearest integer.
-///
-/// # Correctness
-///
-/// `quotient + divisor` must be less than `usize::MAX`.
-/// `divisor` must not be zero.
-//
-// TODO: replace with usize::div_ceil() when int_roundings stabilises:
-// https://github.com/rust-lang/rust/issues/88581
-fn div_ceil(quotient: usize, divisor: usize) -> usize {
-    // Rust uses truncated integer division, so this is equivalent to:
-    // `ceil(quotient/divisor)`
-    // as long as the addition doesn't overflow or underflow.
-    (quotient + divisor - 1) / divisor
-}

zebra-chain/src/transparent.rs

@@ -46,6 +46,8 @@ use proptest_derive::Arbitrary;
 /// outputs of coinbase transactions include Founders' Reward outputs and
 /// transparent Funding Stream outputs."
 /// [7.1](https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus)
+//
+// TODO: change type to HeightDiff
 pub const MIN_TRANSPARENT_COINBASE_MATURITY: u32 = 100;
 
 /// Extra coinbase data that identifies some coinbase transactions generated by Zebra.

zebra-consensus/Cargo.toml

@@ -75,6 +75,7 @@ color-eyre = "0.6.2"
 tinyvec = { version = "1.6.0", features = ["rustc_1_55"] }
 
 hex = "0.4.3"
+num-integer = "0.1.45"
 proptest = "1.1.0"
 proptest-derive = "0.3.0"
 spandoc = "0.2.2"

zebra-consensus/src/checkpoint/list/tests.rs

@@ -2,11 +2,14 @@
 
 use std::sync::Arc;
 
+use num_integer::div_ceil;
+
 use zebra_chain::{
-    block::{self, Block, HeightDiff},
+    block::{self, Block, HeightDiff, MAX_BLOCK_BYTES},
     parameters::{Network, Network::*},
     serialization::ZcashDeserialize,
 };
+use zebra_node_services::constants::{MAX_CHECKPOINT_BYTE_COUNT, MAX_CHECKPOINT_HEIGHT_GAP};
 
 use super::*;
 
@@ -274,14 +277,21 @@ fn checkpoint_list_hard_coded_max_gap_testnet() -> Result<(), BoxError> {
     checkpoint_list_hard_coded_max_gap(Testnet)
 }
 
-/// Check that the hard-coded checkpoints are within `MAX_CHECKPOINT_HEIGHT_GAP`.
+/// Check that the hard-coded checkpoints are within [`MAX_CHECKPOINT_HEIGHT_GAP`],
+/// and a calculated minimum number of blocks. This also checks the heights are in order.
 ///
-/// We can't test the block byte limit, because we don't have access to the entire
+/// We can't test [`MAX_CHECKPOINT_BYTE_COUNT`] directly, because we don't have access to the
-/// blockchain in the tests. But that's ok, because the byte limit only impacts
+/// entire blockchain in the tests. Instead, we check the number of maximum-size blocks in a
-/// performance.
+/// checkpoint. (This is ok, because the byte count only impacts performance.)
 fn checkpoint_list_hard_coded_max_gap(network: Network) -> Result<(), BoxError> {
     let _init_guard = zebra_test::init();
 
+    let max_checkpoint_height_gap =
+        HeightDiff::try_from(MAX_CHECKPOINT_HEIGHT_GAP).expect("constant fits in HeightDiff");
+    let min_checkpoint_height_gap =
+        HeightDiff::try_from(div_ceil(MAX_CHECKPOINT_BYTE_COUNT, MAX_BLOCK_BYTES))
+            .expect("constant fits in HeightDiff");
+
     let list = CheckpointList::new(network);
     let mut heights = list.0.keys();
 
@@ -290,12 +300,27 @@ fn checkpoint_list_hard_coded_max_gap(network: Network) -> Result<(), BoxError>
     assert_eq!(heights.next(), Some(&previous_height));
 
     for height in heights {
-        let height_limit =
+        let height_upper_limit = (previous_height + max_checkpoint_height_gap)
-            (previous_height + (crate::MAX_CHECKPOINT_HEIGHT_GAP as HeightDiff)).unwrap();
+            .expect("checkpoint heights are valid blockchain heights");
+
+        let height_lower_limit = (previous_height + min_checkpoint_height_gap)
+            .expect("checkpoint heights are valid blockchain heights");
+
         assert!(
-            height <= &height_limit,
+            height <= &height_upper_limit,
-            "Checkpoint gaps must be within MAX_CHECKPOINT_HEIGHT_GAP"
+            "Checkpoint gaps must be MAX_CHECKPOINT_HEIGHT_GAP or less \
+             actually: {height:?} - {previous_height:?} = {} \
+             should be: less than or equal to {max_checkpoint_height_gap}",
+            *height - previous_height,
         );
+        assert!(
+            height >= &height_lower_limit,
+            "Checkpoint gaps must be ceil(MAX_CHECKPOINT_BYTE_COUNT/MAX_BLOCK_BYTES) or greater \
+             actually: {height:?} - {previous_height:?} = {} \
+             should be: greater than or equal to {min_checkpoint_height_gap}",
+            *height - previous_height,
+        );
+
         previous_height = *height;
     }
 

zebra-node-services/Cargo.toml

@@ -16,5 +16,24 @@ getblocktemplate-rpcs = [
     "zebra-chain/getblocktemplate-rpcs",
 ]
 
+# Tool and test features
+
+rpc-client = [
+    "color-eyre",
+    "jsonrpc-core",
+    "reqwest",
+    "serde",
+    "serde_json",
+]
+
 [dependencies]
 zebra-chain = { path = "../zebra-chain" }
+
+# Optional dependencies
+
+# Tool and test feature rpc-client
+color-eyre = { version = "0.6.2", optional = true }
+jsonrpc-core = { version = "18.0.0", optional = true }
+reqwest = { version = "0.11.16", optional = true }
+serde = { version = "1.0.160", optional = true }
+serde_json = { version = "1.0.95", optional = true }

zebra-node-services/src/lib.rs

@@ -3,6 +3,9 @@
 pub mod constants;
 pub mod mempool;
 
+#[cfg(any(test, feature = "rpc-client"))]
+pub mod rpc_client;
+
 /// Error type alias to make working with tower traits easier.
 ///
 /// Note: the 'static lifetime bound means that the *type* cannot have any

zebra-node-services/src/rpc_client.rs

@@ -1,20 +1,21 @@
-//! A client for calling Zebra's Json-RPC methods
+//! A client for calling Zebra's JSON-RPC methods.
+//!
+//! Only used in tests and tools.
 
 use std::net::SocketAddr;
 
 use reqwest::Client;
 
-#[cfg(feature = "getblocktemplate-rpcs")]
 use color_eyre::{eyre::eyre, Result};
 
-/// An http client for making Json-RPC requests
+/// An HTTP client for making JSON-RPC requests.
 #[derive(Clone, Debug)]
-pub struct RPCRequestClient {
+pub struct RpcRequestClient {
     client: Client,
     rpc_address: SocketAddr,
 }
 
-impl RPCRequestClient {
+impl RpcRequestClient {
     /// Creates new RPCRequestSender
     pub fn new(rpc_address: SocketAddr) -> Self {
         Self {
@@ -26,10 +27,12 @@ impl RPCRequestClient {
     /// Builds rpc request
     pub async fn call(
         &self,
-        method: &'static str,
+        method: impl AsRef<str>,
-        params: impl Into<String>,
+        params: impl AsRef<str>,
     ) -> reqwest::Result<reqwest::Response> {
-        let params = params.into();
+        let method = method.as_ref();
+        let params = params.as_ref();
+
         self.client
             .post(format!("http://{}", &self.rpc_address))
             .body(format!(
@@ -43,8 +46,8 @@ impl RPCRequestClient {
     /// Builds rpc request and gets text from response
     pub async fn text_from_call(
         &self,
-        method: &'static str,
+        method: impl AsRef<str>,
-        params: impl Into<String>,
+        params: impl AsRef<str>,
     ) -> reqwest::Result<String> {
         self.call(method, params).await?.text().await
     }
@@ -54,18 +57,16 @@ impl RPCRequestClient {
     ///
     /// Returns Ok with json result from response if successful.
     /// Returns an error if the call or result deserialization fail.
-    #[cfg(feature = "getblocktemplate-rpcs")]
     pub async fn json_result_from_call<T: serde::de::DeserializeOwned>(
         &self,
-        method: &'static str,
+        method: impl AsRef<str>,
-        params: impl Into<String>,
+        params: impl AsRef<str>,
     ) -> Result<T> {
         Self::json_result_from_response_text(&self.text_from_call(method, params).await?)
     }
 
     /// Accepts response text from an RPC call
     /// Returns `Ok` with a deserialized `result` value in the expected type, or an error report.
-    #[cfg(feature = "getblocktemplate-rpcs")]
     fn json_result_from_response_text<T: serde::de::DeserializeOwned>(
         response_text: &str,
     ) -> Result<T> {

zebra-rpc/Cargo.toml

@@ -63,7 +63,7 @@ zcash_address = { version = "0.2.1", optional = true }
 # Test-only feature proptest-impl
 proptest = { version = "1.1.0", optional = true }
 
-zebra-chain = { path = "../zebra-chain" }
+zebra-chain = { path = "../zebra-chain", features = ["json-conversion"] }
 zebra-consensus = { path = "../zebra-consensus" }
 zebra-network = { path = "../zebra-network" }
 zebra-node-services = { path = "../zebra-node-services" }

zebra-utils/Cargo.toml

@@ -5,10 +5,16 @@ license = "MIT OR Apache-2.0"
 version = "1.0.0-beta.23"
 edition = "2021"
 
-# Prevent accidental publication of this utility crate.
+[[bin]]
-publish = false
+name = "zebra-checkpoints"
+# this setting is required for Zebra's Docker build caches
+path = "src/bin/zebra-checkpoints/main.rs"
+required-features = ["zebra-checkpoints"]
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+[[bin]]
+name = "search-issue-refs"
+path = "src/bin/search-issue-refs/main.rs"
+required-features = ["search-issue-refs"]
 
 [[bin]]
 name = "block-template-to-proposal"
@@ -16,19 +22,25 @@ name = "block-template-to-proposal"
 path = "src/bin/block-template-to-proposal/main.rs"
 required-features = ["getblocktemplate-rpcs"]
 
-[[bin]]
-name = "search-issue-refs"
-path = "src/bin/search-issue-refs/main.rs"
-required-features = ["search-issue-refs"]
-
 [features]
 default = []
 
-search-issue-refs = ["regex", "reqwest", "tokio"]
+# Each binary has a feature that activates the extra dependencies it needs
 
-# Production features that activate extra dependencies, or extra features in dependencies
+zebra-checkpoints = [
+    "itertools",
+    "tokio",
+    "zebra-chain/json-conversion",
+    "zebra-node-services/rpc-client"
+]
 
-# Experimental mining RPC support
+search-issue-refs = [
+    "regex",
+    "reqwest",
+    "tokio"
+]
+
+# block-template-to-proposal uses the experimental mining RPC support feature name
 getblocktemplate-rpcs = [
     "zebra-rpc/getblocktemplate-rpcs",
     "zebra-node-services/getblocktemplate-rpcs",
@@ -48,13 +60,18 @@ tracing-error = "0.2.0"
 tracing-subscriber = "0.3.17"
 thiserror = "1.0.40"
 
-# These crates are needed for the search-issue-refs binary
-regex = { version = "1.8.1", optional = true }
-reqwest =  { version = "0.11.14", optional = true }
-tokio = { version = "1.27.0", features = ["full"], optional = true }
-
 zebra-node-services = { path = "../zebra-node-services" }
 zebra-chain = { path = "../zebra-chain" }
 
-# Experimental feature getblocktemplate-rpcs
+# These crates are needed for the zebra-checkpoints binary
+itertools = { version = "0.10.5", optional = true }
+
+# These crates are needed for the search-issue-refs binary
+regex = { version = "1.8.1", optional = true }
+reqwest =  { version = "0.11.14", optional = true }
+
+# These crates are needed for the zebra-checkpoints and search-issue-refs binaries
+tokio = { version = "1.27.0", features = ["full"], optional = true }
+
+# These crates are needed for the block-template-to-proposal binary
 zebra-rpc = { path = "../zebra-rpc", optional = true }

zebra-utils/README.md

@@ -20,7 +20,7 @@ To create checkpoints, you need a synchronized instance of `zebrad` or `zcashd`,
 `zebra-checkpoints` is a standalone rust binary, you can compile it using:
 
 ```sh
-cargo install --locked --git https://github.com/ZcashFoundation/zebra zebra-utils
+cargo install --locked --features zebra-checkpoints --git https://github.com/ZcashFoundation/zebra zebra-utils
 ```
 
 Then update the checkpoints using these commands:

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

@@ -2,51 +2,123 @@
 //!
 //! For usage please refer to the program help: `zebra-checkpoints --help`
 
+use std::{net::SocketAddr, str::FromStr};
+
 use structopt::StructOpt;
 use thiserror::Error;
 
-use std::str::FromStr;
+use zebra_chain::block::Height;
 
 /// The backend type the zebra-checkpoints utility will use to get data from.
+///
+/// This changes which RPCs the tool calls, and which fields it expects them to have.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Backend {
+    /// Expect a Zebra-style backend with limited RPCs and fields.
+    ///
+    /// Calls these specific RPCs:
+    /// - `getblock` with `verbose=0`, manually calculating `hash`, `height`, and `size`
+    /// - `getblockchaininfo`, expecting a `blocks` field
+    ///
+    /// Supports both `zebrad` and `zcashd` nodes.
     Zebrad,
+
+    /// Expect a `zcashd`-style backend with all available RPCs and fields.
+    ///
+    /// Calls these specific RPCs:
+    /// - `getblock` with `verbose=1`, expecting `hash`, `height`, and `size` fields
+    /// - `getblockchaininfo`, expecting a `blocks` field
+    ///
+    /// Currently only supported with `zcashd`.
     Zcashd,
 }
 
 impl FromStr for Backend {
-    type Err = InvalidModeError;
+    type Err = InvalidBackendError;
 
     fn from_str(string: &str) -> Result<Self, Self::Err> {
         match string.to_lowercase().as_str() {
             "zebrad" => Ok(Backend::Zebrad),
             "zcashd" => Ok(Backend::Zcashd),
-            _ => Err(InvalidModeError(string.to_owned())),
+            _ => Err(InvalidBackendError(string.to_owned())),
         }
     }
 }
 
-#[derive(Debug, Error)]
+/// An error indicating that the supplied string is not a valid [`Backend`] name.
-#[error("Invalid mode: {0}")]
+#[derive(Clone, Debug, Error, PartialEq, Eq)]
-pub struct InvalidModeError(String);
+#[error("Invalid backend: {0}")]
+pub struct InvalidBackendError(String);
+
+/// The transport used by the zebra-checkpoints utility to connect to the [`Backend`].
+///
+/// This changes how the tool makes RPC requests.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum Transport {
+    /// Launch the `zcash-cli` command in a subprocess, and read its output.
+    ///
+    /// The RPC name and parameters are sent as command-line arguments.
+    /// Responses are read from the command's standard output.
+    ///
+    /// Requires the `zcash-cli` command, which is part of `zcashd`'s tools.
+    /// Supports both `zebrad` and `zcashd` nodes.
+    Cli,
+
+    /// Connect directly to the node using TCP, and use the JSON-RPC protocol.
+    ///
+    /// Uses JSON-RPC over HTTP for sending the RPC name and parameters, and
+    /// receiving responses.
+    ///
+    /// Always supports the `zebrad` node.
+    /// Only supports `zcashd` nodes using a JSON-RPC TCP port with no authentication.
+    Direct,
+}
+
+impl FromStr for Transport {
+    type Err = InvalidTransportError;
+
+    fn from_str(string: &str) -> Result<Self, Self::Err> {
+        match string.to_lowercase().as_str() {
+            "cli" | "zcash-cli" | "zcashcli" | "zcli" | "z-cli" => Ok(Transport::Cli),
+            "direct" => Ok(Transport::Direct),
+            _ => Err(InvalidTransportError(string.to_owned())),
+        }
+    }
+}
+
+/// An error indicating that the supplied string is not a valid [`Transport`] name.
+#[derive(Clone, Debug, Error, PartialEq, Eq)]
+#[error("Invalid transport: {0}")]
+pub struct InvalidTransportError(String);
 
 /// zebra-checkpoints arguments
 #[derive(Clone, Debug, Eq, PartialEq, StructOpt)]
 pub struct Args {
-    /// Backend type
+    /// Backend type: the node we're connecting to.
     #[structopt(default_value = "zebrad", short, long)]
     pub backend: Backend,
 
-    /// Path to zcash-cli command
+    /// Transport type: the way we connect.
+    #[structopt(default_value = "cli", short, long)]
+    pub transport: Transport,
+
+    /// Path or name of zcash-cli command.
+    /// Only used if the transport is [`Cli`](Transport::Cli).
     #[structopt(default_value = "zcash-cli", short, long)]
     pub cli: String,
 
+    /// Address and port for RPC connections.
+    /// Used for all transports.
+    #[structopt(short, long)]
+    pub addr: Option<SocketAddr>,
+
     /// Start looking for checkpoints after this height.
     /// If there is no last checkpoint, we start looking at the Genesis block (height 0).
     #[structopt(short, long)]
-    pub last_checkpoint: Option<u32>,
+    pub last_checkpoint: Option<Height>,
 
-    /// Passthrough args for `zcash-cli`
+    /// Passthrough args for `zcash-cli`.
+    /// Only used if the transport is [`Cli`](Transport::Cli).
     #[structopt(last = true)]
     pub zcli_args: Vec<String>,
 }

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

@@ -8,39 +8,106 @@
 //! zebra-consensus accepts an ordered list of checkpoints, starting with the
 //! genesis block. Checkpoint heights can be chosen arbitrarily.
 
-use std::process::Stdio;
+use std::{ffi::OsString, process::Stdio};
 
 #[cfg(unix)]
 use std::os::unix::process::ExitStatusExt;
 
-use color_eyre::eyre::{ensure, Result};
+use color_eyre::{
-use hex::FromHex;
+    eyre::{ensure, Result},
+    Help,
+};
+use itertools::Itertools;
 use serde_json::Value;
 use structopt::StructOpt;
 
 use zebra_chain::{
-    block, serialization::ZcashDeserializeInto, transparent::MIN_TRANSPARENT_COINBASE_MATURITY,
+    block::{self, Block, Height, HeightDiff, TryIntoHeight},
+    serialization::ZcashDeserializeInto,
+    transparent::MIN_TRANSPARENT_COINBASE_MATURITY,
+};
+use zebra_node_services::{
+    constants::{MAX_CHECKPOINT_BYTE_COUNT, MAX_CHECKPOINT_HEIGHT_GAP},
+    rpc_client::RpcRequestClient,
 };
-use zebra_node_services::constants::{MAX_CHECKPOINT_BYTE_COUNT, MAX_CHECKPOINT_HEIGHT_GAP};
 use zebra_utils::init_tracing;
 
-mod args;
+pub mod args;
 
-/// Return a new `zcash-cli` command, including the `zebra-checkpoints`
+use args::{Args, Backend, Transport};
-/// 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() {
+/// Make an RPC call based on `our_args` and `rpc_command`, and return the response as a [`Value`].
-        cmd.args(&args.zcli_args);
+async fn rpc_output<M, I>(our_args: &Args, method: M, params: I) -> Result<Value>
+where
+    M: AsRef<str>,
+    I: IntoIterator<Item = String>,
+{
+    match our_args.transport {
+        Transport::Cli => cli_output(our_args, method, params),
+        Transport::Direct => direct_output(our_args, method, params).await,
     }
-    cmd
 }
 
-/// Run `cmd` and return its output as a string.
+/// Connect to the node with `our_args` and `rpc_command`, and return the response as a [`Value`].
-fn cmd_output(cmd: &mut std::process::Command) -> Result<String> {
+///
-    // Capture stdout, but send stderr to the user
+/// Only used if the transport is [`Direct`](Transport::Direct).
+async fn direct_output<M, I>(our_args: &Args, method: M, params: I) -> Result<Value>
+where
+    M: AsRef<str>,
+    I: IntoIterator<Item = String>,
+{
+    // Get a new RPC client that will connect to our node
+    let addr = our_args
+        .addr
+        .unwrap_or_else(|| "127.0.0.1:8232".parse().expect("valid address"));
+    let client = RpcRequestClient::new(addr);
+
+    // Launch a request with the RPC method and arguments
+    //
+    // The params are a JSON array with typed arguments.
+    // TODO: accept JSON value arguments, and do this formatting using serde_json
+    let params = format!("[{}]", params.into_iter().join(", "));
+    let response = client.text_from_call(method, params).await?;
+
+    // Extract the "result" field from the RPC response
+    let mut response: Value = serde_json::from_str(&response)?;
+    let response = response["result"].take();
+
+    Ok(response)
+}
+
+/// Run `cmd` with `our_args` and `rpc_command`, and return its output as a [`Value`].
+///
+/// Only used if the transport is [`Cli`](Transport::Cli).
+fn cli_output<M, I>(our_args: &Args, method: M, params: I) -> Result<Value>
+where
+    M: AsRef<str>,
+    I: IntoIterator<Item = String>,
+{
+    // Get a new `zcash-cli` command configured for our node,
+    // including the `zebra-checkpoints` passthrough arguments.
+    let mut cmd = std::process::Command::new(&our_args.cli);
+    cmd.args(&our_args.zcli_args);
+
+    // Turn the address into command-line arguments
+    if let Some(addr) = our_args.addr {
+        cmd.arg(format!("-rpcconnect={}", addr.ip()));
+        cmd.arg(format!("-rpcport={}", addr.port()));
+    }
+
+    // Add the RPC method and arguments
+    let method: OsString = method.as_ref().into();
+    cmd.arg(method);
+
+    for param in params {
+        // Remove JSON string/int type formatting, because zcash-cli will add it anyway
+        // TODO: accept JSON value arguments, and do this formatting using serde_json?
+        let param = param.trim_matches('"');
+        let param: OsString = param.into();
+        cmd.arg(param);
+    }
+
+    // Launch a CLI request, capturing stdout, but sending stderr to the user
     let output = cmd.stderr(Stdio::inherit()).output()?;
 
     // Make sure the command was successful
@@ -58,87 +125,111 @@ fn cmd_output(cmd: &mut std::process::Command) -> Result<String> {
         output.status.code()
     );
 
-    // Make sure the output is valid UTF-8
+    // Make sure the output is valid UTF-8 JSON
-    let s = String::from_utf8(output.stdout)?;
+    let response = String::from_utf8(output.stdout)?;
-    Ok(s)
+    // zcash-cli returns raw strings without JSON type info.
+    // As a workaround, assume that invalid responses are strings.
+    let response: Value = serde_json::from_str(&response)
+        .unwrap_or_else(|_error| Value::String(response.trim().to_string()));
+
+    Ok(response)
 }
 
 /// Process entry point for `zebra-checkpoints`
+#[tokio::main]
 #[allow(clippy::print_stdout)]
-fn main() -> Result<()> {
+async fn main() -> Result<()> {
     // initialise
     init_tracing();
     color_eyre::install()?;
 
-    // get the current block count
+    let args = args::Args::from_args();
-    let mut cmd = passthrough_cmd();
-    cmd.arg("getblockchaininfo");
 
-    let output = cmd_output(&mut cmd)?;
+    // get the current block count
-    let get_block_chain_info: Value = serde_json::from_str(&output)?;
+    let get_block_chain_info = rpc_output(&args, "getblockchaininfo", None)
+        .await
+        .with_suggestion(|| {
+            "Is the RPC server address and port correct? Is authentication configured correctly?"
+        })?;
 
     // calculate the maximum height
-    let height_limit = block::Height(get_block_chain_info["blocks"].as_u64().unwrap() as u32);
+    let height_limit = get_block_chain_info["blocks"]
+        .try_into_height()
+        .expect("height: unexpected invalid value, missing field, or field type");
 
-    assert!(height_limit <= block::Height::MAX);
     // Checkpoints must be on the main chain, so we skip blocks that are within the
     // Zcash reorg limit.
     let height_limit = height_limit
-        .0
+        - HeightDiff::try_from(MIN_TRANSPARENT_COINBASE_MATURITY).expect("constant fits in i32");
-        .checked_sub(MIN_TRANSPARENT_COINBASE_MATURITY)
+    let height_limit =
-        .map(block::Height)
+        height_limit.expect("node has some mature blocks: wait for it to sync more blocks");
-        .expect("zcashd has some mature blocks: wait for zcashd to sync more blocks");
 
-    let starting_height = args::Args::from_args().last_checkpoint.map(block::Height);
-    if starting_height.is_some() {
-        // Since we're about to add 1, height needs to be strictly less than the maximum
-        assert!(starting_height.unwrap() < block::Height::MAX);
-    }
     // Start at the next block after the last checkpoint.
     // If there is no last checkpoint, start at genesis (height 0).
-    let starting_height = starting_height.map_or(0, |block::Height(h)| h + 1);
+    let starting_height = if let Some(last_checkpoint) = args.last_checkpoint {
+        (last_checkpoint + 1)
+            .expect("invalid last checkpoint height, must be less than the max height")
+    } else {
+        Height::MIN
+    };
 
     assert!(
-        starting_height < height_limit.0,
+        starting_height < height_limit,
-        "No mature blocks after the last checkpoint: wait for zcashd to sync more blocks"
+        "No mature blocks after the last checkpoint: wait for node to sync more blocks"
     );
 
     // set up counters
     let mut cumulative_bytes: u64 = 0;
-    let mut height_gap: block::Height = block::Height(0);
+    let mut last_checkpoint_height = args.last_checkpoint.unwrap_or(Height::MIN);
+    let max_checkpoint_height_gap =
+        HeightDiff::try_from(MAX_CHECKPOINT_HEIGHT_GAP).expect("constant fits in HeightDiff");
 
     // loop through all blocks
-    for x in starting_height..height_limit.0 {
+    for request_height in starting_height.0..height_limit.0 {
-        // unfortunately we need to create a process for each block
+        // In `Cli` transport mode we need to create a process for each block
-        let mut cmd = passthrough_cmd();
 
-        let (hash, height, size) = match args::Args::from_args().backend {
+        let (hash, response_height, size) = match args.backend {
-            args::Backend::Zcashd => {
+            Backend::Zcashd => {
                 // get block data from zcashd using verbose=1
-                cmd.args(["getblock", &x.to_string(), "1"]);
+                let get_block = rpc_output(
-                let output = cmd_output(&mut cmd)?;
+                    &args,
-
+                    "getblock",
-                // parse json
+                    [format!(r#""{request_height}""#), 1.to_string()],
-                let v: Value = serde_json::from_str(&output)?;
+                )
+                .await?;
 
                 // get the values we are interested in
-                let hash: block::Hash = v["hash"].as_str().unwrap().parse()?;
+                let hash: block::Hash = get_block["hash"]
-                let height = block::Height(v["height"].as_u64().unwrap() as u32);
+                    .as_str()
+                    .expect("hash: unexpected missing field or field type")
+                    .parse()?;
+                let response_height: Height = get_block["height"]
+                    .try_into_height()
+                    .expect("height: unexpected invalid value, missing field, or field type");
 
-                let size = v["size"].as_u64().unwrap();
+                let size = get_block["size"]
+                    .as_u64()
+                    .expect("size: unexpected invalid value, missing field, or field type");
 
-                (hash, height, size)
+                (hash, response_height, size)
             }
-            args::Backend::Zebrad => {
+            Backend::Zebrad => {
-                // get block data from zebrad by deserializing the raw block
+                // get block data from zebrad (or zcashd) by deserializing the raw block
-                cmd.args(["getblock", &x.to_string(), "0"]);
+                let block_bytes = rpc_output(
-                let output = cmd_output(&mut cmd)?;
+                    &args,
+                    "getblock",
+                    [format!(r#""{request_height}""#), 0.to_string()],
+                )
+                .await?;
+                let block_bytes = block_bytes
+                    .as_str()
+                    .expect("block bytes: unexpected missing field or field type");
 
-                let block_bytes = <Vec<u8>>::from_hex(output.trim_end_matches('\n'))?;
+                let block_bytes: Vec<u8> = hex::decode(block_bytes)?;
 
-                let block = block_bytes
+                // TODO: is it faster to call both `getblock height 0` and `getblock height 1`,
-                    .zcash_deserialize_into::<block::Block>()
+                //       rather than deserializing the block and calculating its hash?
-                    .expect("obtained block should deserialize");
+                let block: Block = block_bytes.zcash_deserialize_into()?;
 
                 (
                     block.hash(),
@@ -150,24 +241,27 @@ fn main() -> Result<()> {
             }
         };
 
-        assert!(height <= block::Height::MAX);
+        assert_eq!(
-        assert_eq!(x, height.0);
+            request_height, response_height.0,
+            "node returned a different block than requested"
+        );
 
-        // compute
+        // compute cumulative totals
         cumulative_bytes += size;
-        height_gap = block::Height(height_gap.0 + 1);
 
-        // check if checkpoint
+        let height_gap = response_height - last_checkpoint_height;
-        if height == block::Height(0)
+
+        // check if this block should be a checkpoint
+        if response_height == Height::MIN
             || cumulative_bytes >= MAX_CHECKPOINT_BYTE_COUNT
-            || height_gap.0 >= MAX_CHECKPOINT_HEIGHT_GAP as u32
+            || height_gap >= max_checkpoint_height_gap
         {
             // print to output
-            println!("{} {hash}", height.0);
+            println!("{} {hash}", response_height.0);
 
-            // reset counters
+            // reset cumulative totals
             cumulative_bytes = 0;
-            height_gap = block::Height(0);
+            last_checkpoint_height = response_height;
         }
     }
 

zebrad/Cargo.toml

@@ -198,8 +198,6 @@ serde_json = { version = "1.0.96", features = ["preserve_order"] }
 tempfile = "3.5.0"
 
 hyper = { version = "0.14.26", features = ["http1", "http2", "server"]}
-reqwest = "0.11.16"
-
 tokio = { version = "1.27.0", features = ["full", "tracing", "test-util"] }
 tokio-stream = "0.1.12"
 
@@ -211,10 +209,13 @@ proptest = "1.1.0"
 proptest-derive = "0.3.0"
 
 # enable span traces and track caller in tests
-color-eyre = { version = "0.6.2", features = ["issue-url"] }
+color-eyre = { version = "0.6.2" }
 
 zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] }
 zebra-consensus = { path = "../zebra-consensus", features = ["proptest-impl"] }
 zebra-network = { path = "../zebra-network", features = ["proptest-impl"] }
 zebra-state = { path = "../zebra-state", features = ["proptest-impl"] }
+
+zebra-node-services = { path = "../zebra-node-services", features = ["rpc-client"] }
+
 zebra-test = { path = "../zebra-test" }

zebrad/tests/acceptance.rs

@@ -144,6 +144,7 @@ use zebra_chain::{
     parameters::Network::{self, *},
 };
 use zebra_network::constants::PORT_IN_USE_ERROR;
+use zebra_node_services::rpc_client::RpcRequestClient;
 use zebra_state::constants::LOCK_FILE_ERROR;
 
 use zebra_test::{args, command::ContextFrom, net::random_known_port, prelude::*};
@@ -167,8 +168,6 @@ use common::{
     test_type::TestType::{self, *},
 };
 
-use crate::common::rpc_client::RPCRequestClient;
-
 /// The maximum amount of time that we allow the creation of a future to block the `tokio` executor.
 ///
 /// This should be larger than the amount of time between thread time slices on a busy test VM.
@@ -1367,7 +1366,7 @@ async fn rpc_endpoint(parallel_cpu_threads: bool) -> Result<()> {
     )?;
 
     // Create an http client
-    let client = RPCRequestClient::new(config.rpc.listen_addr.unwrap());
+    let client = RpcRequestClient::new(config.rpc.listen_addr.unwrap());
 
     // Make the call to the `getinfo` RPC method
     let res = client.call("getinfo", "[]".to_string()).await?;
@@ -1435,7 +1434,7 @@ fn non_blocking_logger() -> Result<()> {
         )?;
 
         // Create an http client
-        let client = RPCRequestClient::new(zebra_rpc_address);
+        let client = RpcRequestClient::new(zebra_rpc_address);
 
         // Most of Zebra's lines are 100-200 characters long, so 500 requests should print enough to fill the unix pipe,
         // fill the channel that tracing logs are queued onto, and drop logs rather than block execution.
@@ -2058,7 +2057,7 @@ async fn fully_synced_rpc_test() -> Result<()> {
 
     zebrad.expect_stdout_line_matches(format!("Opened RPC endpoint at {zebra_rpc_address}"))?;
 
-    let client = RPCRequestClient::new(zebra_rpc_address);
+    let client = RpcRequestClient::new(zebra_rpc_address);
 
     // Make a getblock test that works only on synced node (high block number).
     // The block is before the mandatory checkpoint, so the checkpoint cached state can be used

zebrad/tests/common/cached_state.rs

@@ -5,30 +5,27 @@
 
 #![allow(dead_code)]
 
-use std::path::{Path, PathBuf};
+use std::{
-
+    path::{Path, PathBuf},
-use std::time::Duration;
+    time::Duration,
+};
 
 use color_eyre::eyre::{eyre, Result};
 use tempfile::TempDir;
 use tokio::fs;
 use tower::{util::BoxService, Service};
 
-use zebra_chain::block::Block;
-use zebra_chain::serialization::ZcashDeserializeInto;
 use zebra_chain::{
-    block::{self, Height},
+    block::{self, Block, Height},
     chain_tip::ChainTip,
     parameters::Network,
+    serialization::ZcashDeserializeInto,
 };
-use zebra_state::{ChainTipChange, LatestChainTip};
+use zebra_node_services::rpc_client::RpcRequestClient;
-
+use zebra_state::{ChainTipChange, LatestChainTip, MAX_BLOCK_REORG_HEIGHT};
-use crate::common::config::testdir;
-use crate::common::rpc_client::RPCRequestClient;
-
-use zebra_state::MAX_BLOCK_REORG_HEIGHT;
 
 use crate::common::{
+    config::testdir,
     launch::spawn_zebrad_for_rpc,
     sync::{check_sync_logs_until, MempoolBehavior, SYNC_FINISHED_REGEX},
     test_type::TestType,
@@ -230,7 +227,7 @@ pub async fn get_raw_future_blocks(
     )?;
 
     // Create an http client
-    let rpc_client = RPCRequestClient::new(rpc_address);
+    let rpc_client = RpcRequestClient::new(rpc_address);
 
     let blockchain_info: serde_json::Value = serde_json::from_str(
         &rpc_client

zebrad/tests/common/get_block_template_rpcs/get_block_template.rs

@@ -10,7 +10,9 @@ use std::time::Duration;
 use color_eyre::eyre::{eyre, Context, Result};
 
 use futures::FutureExt;
+
 use zebra_chain::{parameters::Network, serialization::ZcashSerialize};
+use zebra_node_services::rpc_client::RpcRequestClient;
 use zebra_rpc::methods::get_block_template_rpcs::{
     get_block_template::{
         proposal::TimeSource, GetBlockTemplate, JsonParameters, ProposalResponse,
@@ -20,7 +22,6 @@ use zebra_rpc::methods::get_block_template_rpcs::{
 
 use crate::common::{
     launch::{can_spawn_zebrad_for_rpc, spawn_zebrad_for_rpc},
-    rpc_client::RPCRequestClient,
     sync::{check_sync_logs_until, MempoolBehavior, SYNC_FINISHED_REGEX},
     test_type::TestType,
 };
@@ -90,7 +91,7 @@ pub(crate) async fn run() -> Result<()> {
         true,
     )?;
 
-    let client = RPCRequestClient::new(rpc_address);
+    let client = RpcRequestClient::new(rpc_address);
 
     tracing::info!(
         "calling getblocktemplate RPC method at {rpc_address}, \
@@ -135,7 +136,7 @@ pub(crate) async fn run() -> Result<()> {
         .wrap_err("Possible port conflict. Are there other acceptance tests running?")
 }
 
-/// Accepts an [`RPCRequestClient`], calls getblocktemplate in template mode,
+/// Accepts an [`RpcRequestClient`], calls getblocktemplate in template mode,
 /// deserializes and transforms the block template in the response into block proposal data,
 /// then calls getblocktemplate RPC in proposal mode with the serialized and hex-encoded data.
 ///
@@ -148,7 +149,7 @@ pub(crate) async fn run() -> Result<()> {
 /// If an RPC call returns a failure
 /// If the response result cannot be deserialized to `GetBlockTemplate` in 'template' mode
 /// or `ProposalResponse` in 'proposal' mode.
-async fn try_validate_block_template(client: &RPCRequestClient) -> Result<()> {
+async fn try_validate_block_template(client: &RpcRequestClient) -> Result<()> {
     let mut response_json_result: GetBlockTemplate = client
         .json_result_from_call("getblocktemplate", "[]".to_string())
         .await

zebrad/tests/common/get_block_template_rpcs/get_peer_info.rs

@@ -3,11 +3,11 @@
 use color_eyre::eyre::{Context, Result};
 
 use zebra_chain::parameters::Network;
+use zebra_node_services::rpc_client::RpcRequestClient;
 use zebra_rpc::methods::get_block_template_rpcs::types::peer_info::PeerInfo;
 
 use crate::common::{
     launch::{can_spawn_zebrad_for_rpc, spawn_zebrad_for_rpc},
-    rpc_client::RPCRequestClient,
     test_type::TestType,
 };
 
@@ -39,7 +39,7 @@ pub(crate) async fn run() -> Result<()> {
     tracing::info!(?rpc_address, "zebrad opened its RPC port",);
 
     // call `getpeerinfo` RPC method
-    let peer_info_result: Vec<PeerInfo> = RPCRequestClient::new(rpc_address)
+    let peer_info_result: Vec<PeerInfo> = RpcRequestClient::new(rpc_address)
         .json_result_from_call("getpeerinfo", "[]".to_string())
         .await?;
 

zebrad/tests/common/get_block_template_rpcs/submit_block.rs

@@ -11,11 +11,11 @@
 use color_eyre::eyre::{Context, Result};
 
 use zebra_chain::parameters::Network;
+use zebra_node_services::rpc_client::RpcRequestClient;
 
 use crate::common::{
     cached_state::get_raw_future_blocks,
     launch::{can_spawn_zebrad_for_rpc, spawn_zebrad_for_rpc},
-    rpc_client::RPCRequestClient,
     test_type::TestType,
 };
 
@@ -64,7 +64,7 @@ pub(crate) async fn run() -> Result<()> {
     tracing::info!(?rpc_address, "zebrad opened its RPC port",);
 
     // Create an http client
-    let client = RPCRequestClient::new(rpc_address);
+    let client = RpcRequestClient::new(rpc_address);
 
     for raw_block in raw_blocks {
         let res = client

zebrad/tests/common/lightwalletd/sync.rs

@@ -8,12 +8,12 @@ use std::{
 
 use tempfile::TempDir;
 
+use zebra_node_services::rpc_client::RpcRequestClient;
 use zebra_test::prelude::*;
 
 use crate::common::{
     launch::ZebradTestDirExt,
     lightwalletd::wallet_grpc::{connect_to_lightwalletd, ChainSpec},
-    rpc_client::RPCRequestClient,
     test_type::TestType,
 };
 
@@ -183,7 +183,7 @@ pub fn are_zebrad_and_lightwalletd_tips_synced(
         let lightwalletd_tip_height = lightwalletd_tip_block.height;
 
         // Get the block tip from zebrad
-        let client = RPCRequestClient::new(zebra_rpc_address);
+        let client = RpcRequestClient::new(zebra_rpc_address);
         let zebrad_blockchain_info = client
             .text_from_call("getblockchaininfo", "[]".to_string())
             .await?;

zebrad/tests/common/mod.rs

@@ -13,10 +13,10 @@ pub mod cached_state;
 pub mod check;
 pub mod config;
 pub mod failure_messages;
-#[cfg(feature = "getblocktemplate-rpcs")]
-pub mod get_block_template_rpcs;
 pub mod launch;
 pub mod lightwalletd;
-pub mod rpc_client;
 pub mod sync;
 pub mod test_type;
+
+#[cfg(feature = "getblocktemplate-rpcs")]
+pub mod get_block_template_rpcs;