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
-/// blockchain in the tests. But that's ok, because the byte limit only impacts
-/// performance.
+/// We can't test [`MAX_CHECKPOINT_BYTE_COUNT`] directly, because we don't have access to the
+/// entire blockchain in the tests. Instead, we check the number of maximum-size blocks in a
+/// 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 =
-            (previous_height + (crate::MAX_CHECKPOINT_HEIGHT_GAP as HeightDiff)).unwrap();
+        let height_upper_limit = (previous_height + max_checkpoint_height_gap)
+            .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,
-            "Checkpoint gaps must be within MAX_CHECKPOINT_HEIGHT_GAP"
+            height <= &height_upper_limit,
+            "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