From 83563b8088f2c8705a1c3f8e499f01f6a010c9f7 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 7 May 2024 18:59:16 +0200 Subject: [PATCH 01/26] Remove `orchard::tree::SerializedTree` --- zebra-chain/src/orchard/tree.rs | 86 +++++-------------------- zebra-rpc/src/methods.rs | 10 ++- zebra-rpc/src/methods/tests/snapshot.rs | 2 +- 3 files changed, 22 insertions(+), 76 deletions(-) diff --git a/zebra-chain/src/orchard/tree.rs b/zebra-chain/src/orchard/tree.rs index 25cde25d962..b3433a99c6d 100644 --- a/zebra-chain/src/orchard/tree.rs +++ b/zebra-chain/src/orchard/tree.rs @@ -15,7 +15,6 @@ use std::{ fmt, hash::{Hash, Hasher}, io, - sync::Arc, }; use bitvec::prelude::*; @@ -25,7 +24,7 @@ use hex::ToHex; use incrementalmerkletree::Hashable; use lazy_static::lazy_static; use thiserror::Error; -use zcash_primitives::merkle_tree::{write_commitment_tree, HashSer}; +use zcash_primitives::merkle_tree::HashSer; use super::sinsemilla::*; @@ -243,7 +242,7 @@ impl ToHex for Node { } } -/// Required to convert [`NoteCommitmentTree`] into [`SerializedTree`]. +/// Required to serialize [`NoteCommitmentTree`]s in a format compatible with `zcashd`. /// /// Zebra stores Orchard note commitment trees as [`Frontier`][1]s while the /// [`z_gettreestate`][2] RPC requires [`CommitmentTree`][3]s. Implementing @@ -633,7 +632,21 @@ impl NoteCommitmentTree { assert_eq!(self.inner, other.inner); // Check the RPC serialization format (not the same as the Zebra database format) - assert_eq!(SerializedTree::from(self), SerializedTree::from(other)); + assert_eq!(self.to_rpc_bytes(), other.to_rpc_bytes()); + } + + /// Serializes [`Self`] to a format compatible with `zcashd`'s RPCs. + pub fn to_rpc_bytes(&self) -> Vec { + // Convert the tree from [`Frontier`](bridgetree::Frontier) to + // [`CommitmentTree`](merkle_tree::CommitmentTree). + let tree = incrementalmerkletree::frontier::CommitmentTree::from_frontier(&self.inner); + + let mut rpc_bytes = vec![]; + + zcash_primitives::merkle_tree::write_commitment_tree(&tree, &mut rpc_bytes) + .expect("serializable tree"); + + rpc_bytes } } @@ -688,68 +701,3 @@ impl From> for NoteCommitmentTree { tree } } - -/// A serialized Orchard note commitment tree. -/// -/// The format of the serialized data is compatible with -/// [`CommitmentTree`](incrementalmerkletree::frontier::CommitmentTree) from `librustzcash` and not -/// with [`Frontier`](bridgetree::Frontier) from the crate -/// [`incrementalmerkletree`]. Zebra follows the former format in order to stay -/// consistent with `zcashd` in RPCs. Note that [`NoteCommitmentTree`] itself is -/// represented as [`Frontier`](bridgetree::Frontier). -/// -/// The formats are semantically equivalent. The primary difference between them -/// is that in [`Frontier`](bridgetree::Frontier), the vector of parents is -/// dense (we know where the gaps are from the position of the leaf in the -/// overall tree); whereas in [`CommitmentTree`](incrementalmerkletree::frontier::CommitmentTree), -/// the vector of parent hashes is sparse with [`None`] values in the gaps. -/// -/// The sparse format, used in this implementation, allows representing invalid -/// commitment trees while the dense format allows representing only valid -/// commitment trees. -/// -/// It is likely that the dense format will be used in future RPCs, in which -/// case the current implementation will have to change and use the format -/// compatible with [`Frontier`](bridgetree::Frontier) instead. -#[derive(Clone, Debug, Default, Eq, PartialEq, serde::Serialize)] -pub struct SerializedTree(Vec); - -impl From<&NoteCommitmentTree> for SerializedTree { - fn from(tree: &NoteCommitmentTree) -> Self { - let mut serialized_tree = vec![]; - - // Skip the serialization of empty trees. - // - // Note: This ensures compatibility with `zcashd` in the - // [`z_gettreestate`][1] RPC. - // - // [1]: https://zcash.github.io/rpc/z_gettreestate.html - if tree.inner == bridgetree::Frontier::empty() { - return Self(serialized_tree); - } - - // Convert the note commitment tree from - // [`Frontier`](bridgetree::Frontier) to - // [`CommitmentTree`](merkle_tree::CommitmentTree). - let tree = incrementalmerkletree::frontier::CommitmentTree::from_frontier(&tree.inner); - - write_commitment_tree(&tree, &mut serialized_tree) - .expect("note commitment tree should be serializable"); - Self(serialized_tree) - } -} - -impl From>> for SerializedTree { - fn from(maybe_tree: Option>) -> Self { - match maybe_tree { - Some(tree) => tree.as_ref().into(), - None => Self(Vec::new()), - } - } -} - -impl AsRef<[u8]> for SerializedTree { - fn as_ref(&self) -> &[u8] { - &self.0 - } -} diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 92c7aa22a46..a586d69daa1 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -21,7 +21,6 @@ use tracing::Instrument; use zebra_chain::{ block::{self, Height, SerializedBlock}, chain_tip::ChainTip, - orchard, parameters::{ConsensusBranchId, Network, NetworkUpgrade}, sapling, serialization::{SerializationError, ZcashDeserialize}, @@ -1179,8 +1178,8 @@ where }; let orchard_tree = match orchard_response { - zebra_state::ReadResponse::OrchardTree(maybe_tree) => { - orchard::tree::SerializedTree::from(maybe_tree) + zebra_state::ReadResponse::OrchardTree(tree) => { + tree.map_or(vec![], |t| t.to_rpc_bytes()) } _ => unreachable!("unmatched response to an orchard tree request"), }; @@ -1684,8 +1683,7 @@ pub struct GetTreestate { sapling: Treestate, /// A treestate containing an Orchard note commitment tree, hex-encoded. - #[serde(skip_serializing_if = "Treestate::is_empty")] - orchard: Treestate, + orchard: Treestate>, } impl Default for GetTreestate { @@ -1701,7 +1699,7 @@ impl Default for GetTreestate { }, orchard: Treestate { commitments: Commitments { - final_state: orchard::tree::SerializedTree::default(), + final_state: vec![], }, }, } diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index b978805324d..e27fb812f53 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -11,7 +11,7 @@ use insta::dynamic_redaction; use tower::buffer::Buffer; use zebra_chain::{ - block::Block, chain_tip::mock::MockChainTip, parameters::Network::Mainnet, + block::Block, chain_tip::mock::MockChainTip, orchard, parameters::Network::Mainnet, serialization::ZcashDeserializeInto, subtree::NoteCommitmentSubtreeData, }; use zebra_state::{ReadRequest, ReadResponse, MAX_ON_DISK_HEIGHT}; From 892229c5b129194e613624911ea99b8669249f41 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 7 May 2024 20:53:25 +0200 Subject: [PATCH 02/26] Move `GetTreestate` to the `trees` module --- zebra-rpc/src/methods.rs | 113 ++--------------- zebra-rpc/src/methods/trees.rs | 114 +++++++++++++++++- zebra-utils/src/bin/openapi-generator/main.rs | 2 +- 3 files changed, 125 insertions(+), 104 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index a586d69daa1..e05689ff0b3 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -50,6 +50,8 @@ pub mod get_block_template_rpcs; #[cfg(feature = "getblocktemplate-rpcs")] pub use get_block_template_rpcs::{GetBlockTemplateRpc, GetBlockTemplateRpcImpl}; +use self::trees::GetTreestate; + #[cfg(test)] mod tests; @@ -1133,12 +1135,11 @@ where }; let hash = hash_or_height.hash().unwrap_or_else(|| block.hash()); - let hash_or_height = hash.into(); - // Fetch the Sapling & Orchard treestates referenced by - // [`hash_or_height`] from the state. + // Fetch the Sapling & Orchard treestates referenced by [`hash_or_height`] from the + // state. - let sapling_request = zebra_state::ReadRequest::SaplingTree(hash_or_height); + let sapling_request = zebra_state::ReadRequest::SaplingTree(hash.into()); let sapling_response = state .ready() .and_then(|service| service.call(sapling_request)) @@ -1149,7 +1150,9 @@ where data: None, })?; - let orchard_request = zebra_state::ReadRequest::OrchardTree(hash_or_height); + // TODO: Don't make the Orchard request if we're below NU5 AH. + + let orchard_request = zebra_state::ReadRequest::OrchardTree(hash.into()); let orchard_response = state .ready() .and_then(|service| service.call(orchard_request)) @@ -1170,35 +1173,23 @@ where let time = u32::try_from(block.header.time.timestamp()) .expect("Timestamps of valid blocks always fit into u32."); - let sapling_tree = match sapling_response { + let sapling = match sapling_response { zebra_state::ReadResponse::SaplingTree(maybe_tree) => { sapling::tree::SerializedTree::from(maybe_tree) } _ => unreachable!("unmatched response to a sapling tree request"), }; - let orchard_tree = match orchard_response { + let orchard = match orchard_response { zebra_state::ReadResponse::OrchardTree(tree) => { tree.map_or(vec![], |t| t.to_rpc_bytes()) } _ => unreachable!("unmatched response to an orchard tree request"), }; - Ok(GetTreestate { - hash, - height, - time, - sapling: Treestate { - commitments: Commitments { - final_state: sapling_tree, - }, - }, - orchard: Treestate { - commitments: Commitments { - final_state: orchard_tree, - }, - }, - }) + Ok(GetTreestate::from_parts( + hash, height, time, sapling, orchard, + )) } .boxed() } @@ -1659,84 +1650,6 @@ impl Default for GetBlockHash { } } -/// Response to a `z_gettreestate` RPC request. -/// -/// Contains the hex-encoded Sapling & Orchard note commitment trees, and their -/// corresponding [`block::Hash`], [`Height`], and block time. -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] -pub struct GetTreestate { - /// The block hash corresponding to the treestate, hex-encoded. - #[serde(with = "hex")] - hash: block::Hash, - - /// The block height corresponding to the treestate, numeric. - height: Height, - - /// Unix time when the block corresponding to the treestate was mined, - /// numeric. - /// - /// UTC seconds since the Unix 1970-01-01 epoch. - time: u32, - - /// A treestate containing a Sapling note commitment tree, hex-encoded. - #[serde(skip_serializing_if = "Treestate::is_empty")] - sapling: Treestate, - - /// A treestate containing an Orchard note commitment tree, hex-encoded. - orchard: Treestate>, -} - -impl Default for GetTreestate { - fn default() -> Self { - GetTreestate { - hash: block::Hash([0; 32]), - height: Height(0), - time: 0, - sapling: Treestate { - commitments: Commitments { - final_state: sapling::tree::SerializedTree::default(), - }, - }, - orchard: Treestate { - commitments: Commitments { - final_state: vec![], - }, - }, - } - } -} - -/// A treestate that is included in the [`z_gettreestate`][1] RPC response. -/// -/// [1]: https://zcash.github.io/rpc/z_gettreestate.html -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] -struct Treestate> { - /// Contains an Orchard or Sapling serialized note commitment tree, - /// hex-encoded. - commitments: Commitments, -} - -/// A wrapper that contains either an Orchard or Sapling note commitment tree. -/// -/// Note that in the original [`z_gettreestate`][1] RPC, [`Commitments`] also -/// contains the field `finalRoot`. Zebra does *not* use this field. -/// -/// [1]: https://zcash.github.io/rpc/z_gettreestate.html -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] -struct Commitments> { - /// Orchard or Sapling serialized note commitment tree, hex-encoded. - #[serde(with = "hex")] - #[serde(rename = "finalState")] - final_state: Tree, -} - -impl> Treestate { - /// Returns `true` if there's no serialized commitment tree. - fn is_empty(&self) -> bool { - self.commitments.final_state.as_ref().is_empty() - } -} - /// Response to a `getrawtransaction` RPC request. /// /// See the notes for the [`Rpc::get_raw_transaction` method]. diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index 43389c43d69..368b00e2cd9 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -1,8 +1,11 @@ //! Types and functions for note commitment tree RPCs. -// -// TODO: move the *Tree and *Commitment types into this module. -use zebra_chain::subtree::{NoteCommitmentSubtreeData, NoteCommitmentSubtreeIndex}; +use zebra_chain::{ + block::Hash, + block::Height, + sapling, + subtree::{NoteCommitmentSubtreeData, NoteCommitmentSubtreeIndex}, +}; /// A subtree data type that can hold Sapling or Orchard subtree roots. pub type SubtreeRpcData = NoteCommitmentSubtreeData; @@ -29,3 +32,108 @@ pub struct GetSubtrees { //#[serde(skip_serializing_if = "Vec::is_empty")] pub subtrees: Vec, } + +/// Response to a `z_gettreestate` RPC request. +/// +/// Contains the hex-encoded Sapling & Orchard note commitment trees, and their +/// corresponding [`block::Hash`], [`Height`], and block time. +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +pub struct GetTreestate { + /// The block hash corresponding to the treestate, hex-encoded. + #[serde(with = "hex")] + hash: Hash, + + /// The block height corresponding to the treestate, numeric. + height: Height, + + /// Unix time when the block corresponding to the treestate was mined, + /// numeric. + /// + /// UTC seconds since the Unix 1970-01-01 epoch. + time: u32, + + /// A treestate containing a Sapling note commitment tree, hex-encoded. + #[serde(skip_serializing_if = "Treestate::is_empty")] + sapling: Treestate, + + /// A treestate containing an Orchard note commitment tree, hex-encoded. + orchard: Treestate>, +} + +impl GetTreestate { + /// Constructs [`GetTreestate`] from its constituent parts. + pub fn from_parts( + hash: Hash, + height: Height, + time: u32, + sapling: sapling::tree::SerializedTree, + orchard: Vec, + ) -> Self { + Self { + hash, + height, + time, + sapling: Treestate { + commitments: Commitments { + final_state: sapling, + }, + }, + orchard: Treestate { + commitments: Commitments { + final_state: orchard, + }, + }, + } + } +} + +impl Default for GetTreestate { + fn default() -> Self { + GetTreestate { + hash: Hash([0; 32]), + height: Height(0), + time: 0, + sapling: Treestate { + commitments: Commitments { + final_state: sapling::tree::SerializedTree::default(), + }, + }, + orchard: Treestate { + commitments: Commitments { + final_state: vec![], + }, + }, + } + } +} + +/// A treestate that is included in the [`z_gettreestate`][1] RPC response. +/// +/// [1]: https://zcash.github.io/rpc/z_gettreestate.html +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +struct Treestate> { + /// Contains an Orchard or Sapling serialized note commitment tree, + /// hex-encoded. + commitments: Commitments, +} + +/// A wrapper that contains either an Orchard or Sapling note commitment tree. +/// +/// Note that in the original [`z_gettreestate`][1] RPC, [`Commitments`] also +/// contains the field `finalRoot`. Zebra does *not* use this field. +/// +/// [1]: https://zcash.github.io/rpc/z_gettreestate.html +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +struct Commitments> { + /// Orchard or Sapling serialized note commitment tree, hex-encoded. + #[serde(with = "hex")] + #[serde(rename = "finalState")] + final_state: Tree, +} + +impl> Treestate { + /// Returns `true` if there's no serialized commitment tree. + fn is_empty(&self) -> bool { + self.commitments.final_state.as_ref().is_empty() + } +} diff --git a/zebra-utils/src/bin/openapi-generator/main.rs b/zebra-utils/src/bin/openapi-generator/main.rs index 6dc24db9590..020c166534b 100644 --- a/zebra-utils/src/bin/openapi-generator/main.rs +++ b/zebra-utils/src/bin/openapi-generator/main.rs @@ -6,7 +6,7 @@ use quote::ToTokens; use serde::Serialize; use syn::LitStr; -use zebra_rpc::methods::*; +use zebra_rpc::methods::{trees::GetTreestate, *}; // The API server const SERVER: &str = "http://localhost:8232"; From b22278d4cd585eca4abff3d11b0d31c8e5ac059e Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 7 May 2024 20:54:19 +0200 Subject: [PATCH 03/26] Update comments --- zebra-rpc/src/methods.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index e05689ff0b3..849e5977ac0 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1099,14 +1099,17 @@ where data: None, })?; + // TODO: Return early if we're below Sapling AH. + // # Concurrency // - // For consistency, this lookup must be performed first, then all the other - // lookups must be based on the hash. - + // For consistency, this lookup must be performed first, then all the other lookups must + // be based on the hash. + // // Fetch the block referenced by [`hash_or_height`] from the state. - // TODO: If this RPC is called a lot, just get the block header, - // rather than the whole block. + // + // TODO: If this RPC is called a lot, just get the block header, rather than the whole + // block. let block_request = zebra_state::ReadRequest::Block(hash_or_height); let block_response = state .ready() @@ -1118,10 +1121,9 @@ where data: None, })?; - // The block hash, height, and time are all required fields in the - // RPC response. For this reason, we throw an error early if the - // state didn't return the requested block so that we prevent - // further state queries. + // The block hash, height, and time are all required fields in the RPC response. For + // this reason, we throw an error early if the state didn't return the requested block + // so that we prevent further state queries. let block = match block_response { zebra_state::ReadResponse::Block(Some(block)) => block, zebra_state::ReadResponse::Block(None) => { From ed4fef8b0958ece20321c01b2b75613836f46b78 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 7 May 2024 21:22:52 +0200 Subject: [PATCH 04/26] Remove `sapling::tree::SerializedTree` --- zebra-chain/src/sapling/tree.rs | 154 +++--------------------- zebra-rpc/src/methods.rs | 9 +- zebra-rpc/src/methods/tests/snapshot.rs | 2 +- zebra-rpc/src/methods/trees.rs | 15 +-- 4 files changed, 25 insertions(+), 155 deletions(-) diff --git a/zebra-chain/src/sapling/tree.rs b/zebra-chain/src/sapling/tree.rs index 7b137422b66..dc6aa8d2967 100644 --- a/zebra-chain/src/sapling/tree.rs +++ b/zebra-chain/src/sapling/tree.rs @@ -15,7 +15,6 @@ use std::{ fmt, hash::{Hash, Hasher}, io, - sync::Arc, }; use bitvec::prelude::*; @@ -25,7 +24,6 @@ use incrementalmerkletree::{frontier::Frontier, Hashable}; use lazy_static::lazy_static; use thiserror::Error; -use zcash_encoding::{Optional, Vector}; use zcash_primitives::merkle_tree::HashSer; use super::commitment::pedersen_hashes::pedersen_hash; @@ -38,7 +36,7 @@ use crate::{ }; pub mod legacy; -use legacy::{LegacyLeaf, LegacyNoteCommitmentTree}; +use legacy::LegacyNoteCommitmentTree; /// The type that is used to update the note commitment tree. /// @@ -219,7 +217,7 @@ impl ToHex for Node { } } -/// Required to convert [`NoteCommitmentTree`] into [`SerializedTree`]. +/// Required to serialize [`NoteCommitmentTree`]s in a format compatible with `zcashd`. /// /// Zebra stores Sapling note commitment trees as [`Frontier`]s while the /// [`z_gettreestate`][1] RPC requires [`CommitmentTree`][2]s. Implementing @@ -614,7 +612,21 @@ impl NoteCommitmentTree { assert_eq!(self.inner, other.inner); // Check the RPC serialization format (not the same as the Zebra database format) - assert_eq!(SerializedTree::from(self), SerializedTree::from(other)); + assert_eq!(self.to_rpc_bytes(), other.to_rpc_bytes()); + } + + /// Serializes [`Self`] to a format compatible with `zcashd`'s RPCs. + pub fn to_rpc_bytes(&self) -> Vec { + // Convert the tree from [`Frontier`](bridgetree::Frontier) to + // [`CommitmentTree`](merkle_tree::CommitmentTree). + let tree = incrementalmerkletree::frontier::CommitmentTree::from_frontier(&self.inner); + + let mut rpc_bytes = vec![]; + + zcash_primitives::merkle_tree::write_commitment_tree(&tree, &mut rpc_bytes) + .expect("serializable tree"); + + rpc_bytes } } @@ -670,135 +682,3 @@ impl From> for NoteCommitmentTree { tree } } - -/// A serialized Sapling note commitment tree. -/// -/// The format of the serialized data is compatible with -/// [`CommitmentTree`](incrementalmerkletree::frontier::CommitmentTree) from `librustzcash` and not -/// with [`Frontier`] from the crate -/// [`incrementalmerkletree`]. Zebra follows the former format in order to stay -/// consistent with `zcashd` in RPCs. Note that [`NoteCommitmentTree`] itself is -/// represented as [`Frontier`]. -/// -/// The formats are semantically equivalent. The primary difference between them -/// is that in [`Frontier`], the vector of parents is -/// dense (we know where the gaps are from the position of the leaf in the -/// overall tree); whereas in [`CommitmentTree`](incrementalmerkletree::frontier::CommitmentTree), -/// the vector of parent hashes is sparse with [`None`] values in the gaps. -/// -/// The sparse format, used in this implementation, allows representing invalid -/// commitment trees while the dense format allows representing only valid -/// commitment trees. -/// -/// It is likely that the dense format will be used in future RPCs, in which -/// case the current implementation will have to change and use the format -/// compatible with [`Frontier`] instead. -#[derive(Clone, Debug, Default, Eq, PartialEq, serde::Serialize)] -pub struct SerializedTree(Vec); - -impl From<&NoteCommitmentTree> for SerializedTree { - fn from(tree: &NoteCommitmentTree) -> Self { - let mut serialized_tree = vec![]; - - // - let legacy_tree = LegacyNoteCommitmentTree::from(tree.clone()); - - // Convert the note commitment tree represented as a frontier into the - // format compatible with `zcashd`. - // - // `librustzcash` has a function [`from_frontier()`][1], which returns a - // commitment tree in the sparse format. However, the returned tree - // always contains [`MERKLE_DEPTH`] parent nodes, even though some - // trailing parents are empty. Such trees are incompatible with Sapling - // commitment trees returned by `zcashd` because `zcashd` returns - // Sapling commitment trees without empty trailing parents. For this - // reason, Zebra implements its own conversion between the dense and - // sparse formats for Sapling. - // - // [1]: - if let Some(frontier) = legacy_tree.inner.frontier { - let (left_leaf, right_leaf) = match frontier.leaf { - LegacyLeaf::Left(left_value) => (Some(left_value), None), - LegacyLeaf::Right(left_value, right_value) => (Some(left_value), Some(right_value)), - }; - - // Ommers are siblings of parent nodes along the branch from the - // most recent leaf to the root of the tree. - let mut ommers_iter = frontier.ommers.iter(); - - // Set bits in the binary representation of the position indicate - // the presence of ommers along the branch from the most recent leaf - // node to the root of the tree, except for the lowest bit. - let mut position: u64 = (frontier.position.0) - .try_into() - .expect("old usize position always fit in u64"); - - // The lowest bit does not indicate the presence of any ommers. We - // clear it so that we can test if there are no set bits left in - // [`position`]. - position &= !1; - - // Run through the bits of [`position`], and push an ommer for each - // set bit, or `None` otherwise. In contrast to the 'zcashd' code - // linked above, we want to skip any trailing `None` parents at the - // top of the tree. To do that, we clear the bits as we go through - // them, and break early if the remaining bits are all zero (i.e. - // [`position`] is zero). - let mut parents = vec![]; - for i in 1..MERKLE_DEPTH { - // Test each bit in [`position`] individually. Don't test the - // lowest bit since it doesn't actually indicate the position of - // any ommer. - let bit_mask = 1 << i; - - if position & bit_mask == 0 { - parents.push(None); - } else { - parents.push(ommers_iter.next()); - // Clear the set bit so that we can test if there are no set - // bits left. - position &= !bit_mask; - // If there are no set bits left, exit early so that there - // are no empty trailing parent nodes in the serialized - // tree. - if position == 0 { - break; - } - } - } - - // Serialize the converted note commitment tree. - Optional::write(&mut serialized_tree, left_leaf, |tree, leaf| { - leaf.write(tree) - }) - .expect("A leaf in a note commitment tree should be serializable"); - - Optional::write(&mut serialized_tree, right_leaf, |tree, leaf| { - leaf.write(tree) - }) - .expect("A leaf in a note commitment tree should be serializable"); - - Vector::write(&mut serialized_tree, &parents, |tree, parent| { - Optional::write(tree, *parent, |tree, parent| parent.write(tree)) - }) - .expect("Parent nodes in a note commitment tree should be serializable"); - } - - Self(serialized_tree) - } -} - -impl From>> for SerializedTree { - fn from(maybe_tree: Option>) -> Self { - match maybe_tree { - Some(tree) => tree.as_ref().into(), - None => Self(vec![]), - } - } -} - -impl AsRef<[u8]> for SerializedTree { - fn as_ref(&self) -> &[u8] { - &self.0 - } -} diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 849e5977ac0..024404910ef 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -22,7 +22,6 @@ use zebra_chain::{ block::{self, Height, SerializedBlock}, chain_tip::ChainTip, parameters::{ConsensusBranchId, Network, NetworkUpgrade}, - sapling, serialization::{SerializationError, ZcashDeserialize}, subtree::NoteCommitmentSubtreeIndex, transaction::{self, SerializedTransaction, Transaction, UnminedTx}, @@ -1176,17 +1175,17 @@ where .expect("Timestamps of valid blocks always fit into u32."); let sapling = match sapling_response { - zebra_state::ReadResponse::SaplingTree(maybe_tree) => { - sapling::tree::SerializedTree::from(maybe_tree) + zebra_state::ReadResponse::SaplingTree(tree) => { + tree.map_or(vec![], |t| t.to_rpc_bytes()) } - _ => unreachable!("unmatched response to a sapling tree request"), + _ => unreachable!("unmatched response to a Sapling tree request"), }; let orchard = match orchard_response { zebra_state::ReadResponse::OrchardTree(tree) => { tree.map_or(vec![], |t| t.to_rpc_bytes()) } - _ => unreachable!("unmatched response to an orchard tree request"), + _ => unreachable!("unmatched response to an Orchard tree request"), }; Ok(GetTreestate::from_parts( diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index e27fb812f53..d1588d158fb 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -11,7 +11,7 @@ use insta::dynamic_redaction; use tower::buffer::Buffer; use zebra_chain::{ - block::Block, chain_tip::mock::MockChainTip, orchard, parameters::Network::Mainnet, + block::Block, chain_tip::mock::MockChainTip, orchard, parameters::Network::Mainnet, sapling, serialization::ZcashDeserializeInto, subtree::NoteCommitmentSubtreeData, }; use zebra_state::{ReadRequest, ReadResponse, MAX_ON_DISK_HEIGHT}; diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index 368b00e2cd9..bfc9415b2ff 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -3,7 +3,6 @@ use zebra_chain::{ block::Hash, block::Height, - sapling, subtree::{NoteCommitmentSubtreeData, NoteCommitmentSubtreeIndex}, }; @@ -53,8 +52,7 @@ pub struct GetTreestate { time: u32, /// A treestate containing a Sapling note commitment tree, hex-encoded. - #[serde(skip_serializing_if = "Treestate::is_empty")] - sapling: Treestate, + sapling: Treestate>, /// A treestate containing an Orchard note commitment tree, hex-encoded. orchard: Treestate>, @@ -66,7 +64,7 @@ impl GetTreestate { hash: Hash, height: Height, time: u32, - sapling: sapling::tree::SerializedTree, + sapling: Vec, orchard: Vec, ) -> Self { Self { @@ -95,7 +93,7 @@ impl Default for GetTreestate { time: 0, sapling: Treestate { commitments: Commitments { - final_state: sapling::tree::SerializedTree::default(), + final_state: vec![], }, }, orchard: Treestate { @@ -130,10 +128,3 @@ struct Commitments> { #[serde(rename = "finalState")] final_state: Tree, } - -impl> Treestate { - /// Returns `true` if there's no serialized commitment tree. - fn is_empty(&self) -> bool { - self.commitments.final_state.as_ref().is_empty() - } -} From b2dc162c4fcefc33e58b680780f8ae8788a575d2 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 13 May 2024 12:33:42 +0200 Subject: [PATCH 05/26] Make the serialization compatible with `zcashd` --- Cargo.lock | 1 + zebra-rpc/Cargo.toml | 2 + zebra-rpc/src/methods.rs | 92 +++++++++++++++++----------------- zebra-rpc/src/methods/trees.rs | 49 ++++++------------ 4 files changed, 64 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e393f2aad2d..d19d24f4192 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6191,6 +6191,7 @@ dependencies = [ "tower", "tracing", "zcash_address", + "zcash_primitives", "zebra-chain", "zebra-consensus", "zebra-network", diff --git a/zebra-rpc/Cargo.toml b/zebra-rpc/Cargo.toml index 88450a49338..a5a6396b7ac 100644 --- a/zebra-rpc/Cargo.toml +++ b/zebra-rpc/Cargo.toml @@ -64,6 +64,8 @@ tracing = "0.1.39" hex = { version = "0.4.3", features = ["serde"] } serde = { version = "1.0.201", features = ["serde_derive"] } +zcash_primitives = { version = "0.13.0" } + # Experimental feature getblocktemplate-rpcs rand = { version = "0.8.5", optional = true } # ECC deps used by getblocktemplate-rpcs feature diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 024404910ef..98997f2a719 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -18,6 +18,7 @@ use tokio::{sync::broadcast, task::JoinHandle}; use tower::{Service, ServiceExt}; use tracing::Instrument; +use zcash_primitives::consensus::Parameters; use zebra_chain::{ block::{self, Height, SerializedBlock}, chain_tip::ChainTip, @@ -1087,6 +1088,7 @@ where // - create a function that handles block hashes or heights, and use it in `get_block()` fn z_get_treestate(&self, hash_or_height: String) -> BoxFuture> { let mut state = self.state.clone(); + let network = self.network.clone(); async move { // Convert the [`hash_or_height`] string into an actual hash or height. @@ -1135,59 +1137,57 @@ where _ => unreachable!("unmatched response to a block request"), }; - let hash = hash_or_height.hash().unwrap_or_else(|| block.hash()); + let height = hash_or_height + .height_or_else(|_| block.coinbase_height()) + .expect("block height"); - // Fetch the Sapling & Orchard treestates referenced by [`hash_or_height`] from the - // state. - - let sapling_request = zebra_state::ReadRequest::SaplingTree(hash.into()); - let sapling_response = state - .ready() - .and_then(|service| service.call(sapling_request)) - .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; - - // TODO: Don't make the Orchard request if we're below NU5 AH. - - let orchard_request = zebra_state::ReadRequest::OrchardTree(hash.into()); - let orchard_response = state - .ready() - .and_then(|service| service.call(orchard_request)) - .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; - - // We've got all the data we need for the RPC response, so we - // assemble the response. - - let height = block - .coinbase_height() - .expect("verified blocks have a valid height"); - - let time = u32::try_from(block.header.time.timestamp()) - .expect("Timestamps of valid blocks always fit into u32."); - - let sapling = match sapling_response { - zebra_state::ReadResponse::SaplingTree(tree) => { - tree.map_or(vec![], |t| t.to_rpc_bytes()) + let sapling_nu = zcash_primitives::consensus::NetworkUpgrade::Sapling; + let sapling = if network.is_nu_active(sapling_nu, height.into()) { + match state + .ready() + .and_then(|service| { + service.call(zebra_state::ReadRequest::SaplingTree(height.into())) + }) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })? { + zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()), + _ => unreachable!("unmatched response to a Sapling tree request"), } - _ => unreachable!("unmatched response to a Sapling tree request"), + } else { + None }; - let orchard = match orchard_response { - zebra_state::ReadResponse::OrchardTree(tree) => { - tree.map_or(vec![], |t| t.to_rpc_bytes()) + let orchard_nu = zcash_primitives::consensus::NetworkUpgrade::Nu5; + let orchard = if network.is_nu_active(orchard_nu, height.into()) { + match state + .ready() + .and_then(|service| { + service.call(zebra_state::ReadRequest::OrchardTree(height.into())) + }) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })? { + zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()), + _ => unreachable!("unmatched response to an Orchard tree request"), } - _ => unreachable!("unmatched response to an Orchard tree request"), + } else { + None }; + let hash = hash_or_height + .hash_or_else(|_| Some(block.hash())) + .expect("block hash"); + + let time = u32::try_from(block.header.time.timestamp()) + .expect("Timestamps of valid blocks always fit into u32."); + Ok(GetTreestate::from_parts( hash, height, time, sapling, orchard, )) diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index bfc9415b2ff..b0f316ab7c7 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -52,10 +52,12 @@ pub struct GetTreestate { time: u32, /// A treestate containing a Sapling note commitment tree, hex-encoded. - sapling: Treestate>, + #[serde(skip_serializing_if = "Option::is_none")] + sapling: Option>>, /// A treestate containing an Orchard note commitment tree, hex-encoded. - orchard: Treestate>, + #[serde(skip_serializing_if = "Option::is_none")] + orchard: Option>>, } impl GetTreestate { @@ -64,43 +66,22 @@ impl GetTreestate { hash: Hash, height: Height, time: u32, - sapling: Vec, - orchard: Vec, + sapling: Option>, + orchard: Option>, ) -> Self { + let sapling = sapling.map(|tree| Treestate { + commitments: Commitments { final_state: tree }, + }); + let orchard = orchard.map(|tree| Treestate { + commitments: Commitments { final_state: tree }, + }); + Self { hash, height, time, - sapling: Treestate { - commitments: Commitments { - final_state: sapling, - }, - }, - orchard: Treestate { - commitments: Commitments { - final_state: orchard, - }, - }, - } - } -} - -impl Default for GetTreestate { - fn default() -> Self { - GetTreestate { - hash: Hash([0; 32]), - height: Height(0), - time: 0, - sapling: Treestate { - commitments: Commitments { - final_state: vec![], - }, - }, - orchard: Treestate { - commitments: Commitments { - final_state: vec![], - }, - }, + sapling, + orchard, } } } From 6a79d1cfe62be3e6031cd469fec54772376f1ca1 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 13 May 2024 15:32:46 +0200 Subject: [PATCH 06/26] Simplify error handling --- zebra-rpc/src/methods.rs | 178 +++++++++++---------------------------- 1 file changed, 49 insertions(+), 129 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 98997f2a719..ce78f26f417 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -23,7 +23,7 @@ use zebra_chain::{ block::{self, Height, SerializedBlock}, chain_tip::ChainTip, parameters::{ConsensusBranchId, Network, NetworkUpgrade}, - serialization::{SerializationError, ZcashDeserialize}, + serialization::ZcashDeserialize, subtree::NoteCommitmentSubtreeIndex, transaction::{self, SerializedTransaction, Transaction, UnminedTx}, transparent::{self, Address}, @@ -505,30 +505,18 @@ where let (tip_height, tip_hash) = self .latest_chain_tip .best_tip_height_and_hash() - .ok_or_else(|| Error { - code: ErrorCode::ServerError(0), - message: "No Chain tip available yet".to_string(), - data: None, - })?; + .ok_or(to_error("No Chain tip available yet"))?; // `estimated_height` field - let current_block_time = - self.latest_chain_tip - .best_tip_block_time() - .ok_or_else(|| Error { - code: ErrorCode::ServerError(0), - message: "No Chain tip available yet".to_string(), - data: None, - })?; + let current_block_time = self + .latest_chain_tip + .best_tip_block_time() + .ok_or(to_error("No Chain tip available yet"))?; let zebra_estimated_height = self .latest_chain_tip .estimate_network_chain_tip_height(network, Utc::now()) - .ok_or_else(|| Error { - code: ErrorCode::ServerError(0), - message: "No Chain tip available yet".to_string(), - data: None, - })?; + .ok_or(to_error("No Chain tip available yet"))?; let mut estimated_height = if current_block_time > Utc::now() || zebra_estimated_height < tip_height { @@ -607,11 +595,7 @@ where let valid_addresses = address_strings.valid_addresses()?; let request = zebra_state::ReadRequest::AddressBalance(valid_addresses); - let response = state.oneshot(request).await.map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + let response = state.oneshot(request).await.map_err(to_error)?; match response { zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance { @@ -648,11 +632,7 @@ where let transaction_parameter = mempool::Gossip::Tx(raw_transaction.into()); let request = mempool::Request::Queue(vec![transaction_parameter]); - let response = mempool.oneshot(request).await.map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + let response = mempool.oneshot(request).await.map_err(to_error)?; let queue_results = match response { mempool::Response::Queued(results) => results, @@ -667,14 +647,10 @@ where tracing::debug!("sent transaction to mempool: {:?}", &queue_results[0]); - match &queue_results[0] { - Ok(()) => Ok(SentTransactionHash(transaction_hash)), - Err(error) => Err(Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - }), - } + queue_results[0] + .as_ref() + .map(|_| SentTransactionHash(transaction_hash)) + .map_err(to_error) } .boxed() } @@ -695,14 +671,7 @@ where let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY); async move { - let hash_or_height: HashOrHeight = - hash_or_height - .parse() - .map_err(|error: SerializationError| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + let hash_or_height: HashOrHeight = hash_or_height.parse().map_err(to_error)?; if verbosity == 0 { // # Performance @@ -714,11 +683,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; match response { zebra_state::ReadResponse::Block(Some(block)) => { @@ -762,11 +727,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; match response { zebra_state::ReadResponse::BlockHash(Some(hash)) => hash, @@ -948,11 +909,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; match response { #[cfg(feature = "getblocktemplate-rpcs")] @@ -1031,11 +988,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; match response { mempool::Response::Transactions(unmined_transactions) => { @@ -1053,11 +1006,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; match response { zebra_state::ReadResponse::Transaction(Some(MinedTx { @@ -1070,11 +1019,9 @@ where confirmations, verbose, )), - zebra_state::ReadResponse::Transaction(None) => Err(Error { - code: ErrorCode::ServerError(0), - message: "Transaction not found".to_string(), - data: None, - }), + zebra_state::ReadResponse::Transaction(None) => { + Err(to_error("Transaction not found")) + } _ => unreachable!("unmatched response to a transaction request"), } } @@ -1092,23 +1039,15 @@ where async move { // Convert the [`hash_or_height`] string into an actual hash or height. - let hash_or_height = hash_or_height - .parse() - .map_err(|error: SerializationError| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; - - // TODO: Return early if we're below Sapling AH. + let hash_or_height = hash_or_height.parse().map_err(to_error)?; + // Fetch the block referenced by [`hash_or_height`] from the state. + // // # Concurrency // // For consistency, this lookup must be performed first, then all the other lookups must // be based on the hash. // - // Fetch the block referenced by [`hash_or_height`] from the state. - // // TODO: If this RPC is called a lot, just get the block header, rather than the whole // block. let block_request = zebra_state::ReadRequest::Block(hash_or_height); @@ -1116,15 +1055,11 @@ where .ready() .and_then(|service| service.call(block_request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; - // The block hash, height, and time are all required fields in the RPC response. For - // this reason, we throw an error early if the state didn't return the requested block - // so that we prevent further state queries. + // The block hash, height, and time are all required fields in the RPC response, so we + // return an error early if the state didn't return the requested block to prevent + // further state queries. let block = match block_response { zebra_state::ReadResponse::Block(Some(block)) => block, zebra_state::ReadResponse::Block(None) => { @@ -1149,11 +1084,8 @@ where service.call(zebra_state::ReadRequest::SaplingTree(height.into())) }) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })? { + .map_err(to_error)? + { zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to a Sapling tree request"), } @@ -1169,11 +1101,8 @@ where service.call(zebra_state::ReadRequest::OrchardTree(height.into())) }) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })? { + .map_err(to_error)? + { zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to an Orchard tree request"), } @@ -1212,11 +1141,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; let subtrees = match response { zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees, @@ -1242,11 +1167,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; let subtrees = match response { zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees, @@ -1307,11 +1228,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; let hashes = match response { zebra_state::ReadResponse::AddressesTransactionIds(hashes) => { @@ -1359,11 +1276,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + .map_err(to_error)?; let utxos = match response { zebra_state::ReadResponse::AddressUtxos(utxos) => utxos, _ => unreachable!("unmatched response to a UtxosByAddresses request"), @@ -1413,11 +1326,9 @@ pub fn best_chain_tip_height(latest_chain_tip: &Tip) -> Result where Tip: ChainTip + Clone + Send + Sync + 'static, { - latest_chain_tip.best_tip_height().ok_or(Error { - code: ErrorCode::ServerError(0), - message: "No blocks in state".to_string(), - data: None, - }) + latest_chain_tip + .best_tip_height() + .ok_or(to_error("No blocks in state")) } /// Response to a `getinfo` RPC request. @@ -1850,3 +1761,12 @@ pub fn height_from_signed_int(index: i32, tip_height: Height) -> Result Ok(Height(sanitized_height)) } } + +/// Turns anything implementing [`ToString`] into [`Error`]. +fn to_error(err: impl ToString) -> Error { + Error { + code: ErrorCode::ServerError(0), + message: err.to_string(), + data: None, + } +} From 46942f2d058ad40a7ecdf3ca6ea51b5d7fd76c78 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 13 May 2024 15:33:02 +0200 Subject: [PATCH 07/26] Derive `Default` for `GetTreestate` --- zebra-rpc/src/methods/trees.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index b0f316ab7c7..a9cbf48b8bc 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -36,7 +36,7 @@ pub struct GetSubtrees { /// /// Contains the hex-encoded Sapling & Orchard note commitment trees, and their /// corresponding [`block::Hash`], [`Height`], and block time. -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, Default)] pub struct GetTreestate { /// The block hash corresponding to the treestate, hex-encoded. #[serde(with = "hex")] From 8737d3e6fc5daa401c3329c4cfe6de6e663a0a1b Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 13 May 2024 15:48:31 +0200 Subject: [PATCH 08/26] Remove old TODOs --- zebra-rpc/src/methods.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index ce78f26f417..815c0f3012d 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1028,11 +1028,6 @@ where .boxed() } - // TODO: - // - use a generic error constructor (#5548) - // - use `height_from_signed_int()` to handle negative heights - // (this might be better in the state request, because it needs the state height) - // - create a function that handles block hashes or heights, and use it in `get_block()` fn z_get_treestate(&self, hash_or_height: String) -> BoxFuture> { let mut state = self.state.clone(); let network = self.network.clone(); From c7fe0672aee82122537684194f8270c43382feb3 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 13 May 2024 15:55:26 +0200 Subject: [PATCH 09/26] Simplify the `z_get_treestate` method --- zebra-rpc/src/methods.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 815c0f3012d..cc1fafee99a 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1045,17 +1045,12 @@ where // // TODO: If this RPC is called a lot, just get the block header, rather than the whole // block. - let block_request = zebra_state::ReadRequest::Block(hash_or_height); - let block_response = state + let block = match state .ready() - .and_then(|service| service.call(block_request)) + .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height))) .await - .map_err(to_error)?; - - // The block hash, height, and time are all required fields in the RPC response, so we - // return an error early if the state didn't return the requested block to prevent - // further state queries. - let block = match block_response { + .map_err(to_error)? + { zebra_state::ReadResponse::Block(Some(block)) => block, zebra_state::ReadResponse::Block(None) => { return Err(Error { @@ -1067,10 +1062,17 @@ where _ => unreachable!("unmatched response to a block request"), }; + let hash = hash_or_height + .hash_or_else(|_| Some(block.hash())) + .expect("block hash"); + let height = hash_or_height .height_or_else(|_| block.coinbase_height()) .expect("block height"); + let time = u32::try_from(block.header.time.timestamp()) + .expect("Timestamps of valid blocks always fit into u32."); + let sapling_nu = zcash_primitives::consensus::NetworkUpgrade::Sapling; let sapling = if network.is_nu_active(sapling_nu, height.into()) { match state @@ -1105,13 +1107,6 @@ where None }; - let hash = hash_or_height - .hash_or_else(|_| Some(block.hash())) - .expect("block hash"); - - let time = u32::try_from(block.header.time.timestamp()) - .expect("Timestamps of valid blocks always fit into u32."); - Ok(GetTreestate::from_parts( hash, height, time, sapling, orchard, )) From 2a5a38489f693c88ec46ff075bdf73a2fe03cf94 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 14 May 2024 22:27:22 +0200 Subject: [PATCH 10/26] Add & refactor tests --- Cargo.lock | 2 +- zebra-rpc/src/methods/tests/snapshot.rs | 140 ++++++++++++++---- ..._get_treestate_by_hash@custom_testnet.snap | 9 ++ ..._by_non_existent_hash@custom_testnet.snap} | 2 +- ...mpty_Sapling_treestate@custom_testnet.snap | 14 ++ ...xcessive_block_height@custom_testnet.snap} | 2 +- ...reestate_no_treestate@custom_testnet.snap} | 2 +- ...arsable_hash_or_height@custom_testnet.snap | 10 ++ .../z_get_treestate_valid@mainnet_10.snap | 9 -- 9 files changed, 147 insertions(+), 43 deletions(-) create mode 100644 zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap rename zebra-rpc/src/methods/tests/snapshots/{z_get_treestate_invalid_excessive_height@mainnet_10.snap => z_get_treestate_by_non_existent_hash@custom_testnet.snap} (86%) create mode 100644 zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap rename zebra-rpc/src/methods/tests/snapshots/{z_get_treestate_invalid_excessive_height@testnet_10.snap => z_get_treestate_excessive_block_height@custom_testnet.snap} (86%) rename zebra-rpc/src/methods/tests/snapshots/{z_get_treestate_valid@testnet_10.snap => z_get_treestate_no_treestate@custom_testnet.snap} (88%) create mode 100644 zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap delete mode 100644 zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@mainnet_10.snap diff --git a/Cargo.lock b/Cargo.lock index d19d24f4192..51770c5191f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6191,7 +6191,7 @@ dependencies = [ "tower", "tracing", "zcash_address", - "zcash_primitives", + "zcash_primitives 0.13.0", "zebra-chain", "zebra-consensus", "zebra-network", diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index d1588d158fb..0ee6dc90099 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -11,9 +11,18 @@ use insta::dynamic_redaction; use tower::buffer::Buffer; use zebra_chain::{ - block::Block, chain_tip::mock::MockChainTip, orchard, parameters::Network::Mainnet, sapling, - serialization::ZcashDeserializeInto, subtree::NoteCommitmentSubtreeData, + block::Block, + chain_tip::mock::MockChainTip, + orchard, + parameters::{ + testnet::{ConfiguredActivationHeights, Parameters}, + Network::Mainnet, + }, + sapling, + serialization::ZcashDeserializeInto, + subtree::NoteCommitmentSubtreeData, }; +use zebra_node_services::BoxError; use zebra_state::{ReadRequest, ReadResponse, MAX_ON_DISK_HEIGHT}; use zebra_test::mock_service::MockService; @@ -40,6 +49,105 @@ async fn test_rpc_response_data() { ); } +/// Checks the output of the [`z_get_treestate`] RPC. +/// +/// TODO: +/// 1. Check a non-empty Sapling treestate. +/// 2. Check an empty Orchard treestate at NU5 activation height. +/// 3. Check a non-empty Orchard treestate. +/// +/// To implement the todos above, we need to: +/// +/// 1. Have a block containing Sapling note commitmnets in the state. +/// 2. Activate NU5 at a height for which we have a block in the state. +/// 3. Have a block containing Orchard note commitments in the state. +#[tokio::test] +async fn test_z_get_treestate() { + let _init_guard = zebra_test::init(); + const SAPLING_ACTIVATION_HEIGHT: u32 = 2; + + let testnet = Parameters::build() + .with_activation_heights(ConfiguredActivationHeights { + sapling: Some(SAPLING_ACTIVATION_HEIGHT), + // We need to set the NU5 activation height higher than the height of the last block for + // this test because we currently have only the first 10 blocks from the public Testnet, + // none of which are compatible with NU5 due to the following consensus rule: + // + // > [NU5 onward] hashBlockCommitments MUST be set to the value of + // > hashBlockCommitments for this block, as specified in [ZIP-244]. + // + // Activating NU5 at a lower height and using the 10 blocks causes a failure in + // [`zebra_state::populated_state`]. + nu5: Some(10), + ..Default::default() + }) + .with_network_name("custom_testnet") + .to_network(); + + // Initiate the snapshots of the RPC responses. + let mut settings = insta::Settings::clone_current(); + settings.set_snapshot_suffix(network_string(&testnet).to_string()); + + let blocks: Vec<_> = testnet + .blockchain_iter() + .map(|(_, block_bytes)| block_bytes.zcash_deserialize_into().unwrap()) + .collect(); + + let (_, state, tip, _) = zebra_state::populated_state(blocks.clone(), &testnet).await; + + let (rpc, _) = RpcImpl::new( + "", + "", + testnet, + false, + true, + Buffer::new(MockService::build().for_unit_tests::<_, _, BoxError>(), 1), + state, + tip, + ); + + // Request the treestate by a hash. + let rsp = rpc + .z_get_treestate(blocks[0].hash().to_string()) + .await + .expect("genesis treestate = no treestate"); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_hash", rsp)); + + // Request the treestate by a hash for a block which is not in the state. + let rsp = rpc.z_get_treestate(block::Hash([0; 32]).to_string()).await; + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_non_existent_hash", rsp)); + + // Request the treestate before Sapling activation. + let rsp = rpc + .z_get_treestate((SAPLING_ACTIVATION_HEIGHT - 1).to_string()) + .await + .expect("no Sapling treestate and no Orchard treestate"); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_no_treestate", rsp)); + + // Request the treestate at Sapling activation. + let rsp = rpc + .z_get_treestate(SAPLING_ACTIVATION_HEIGHT.to_string()) + .await + .expect("empty Sapling treestate and no Orchard treestate"); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_empty_Sapling_treestate", rsp)); + + // Request the treestate for an invalid height. + let rsp = rpc + .z_get_treestate(EXCESSIVE_BLOCK_HEIGHT.to_string()) + .await; + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_excessive_block_height", rsp)); + + // Request the treestate for an unparsable hash or height. + let rsp = rpc.z_get_treestate("Do you even shield?".to_string()).await; + settings + .bind(|| insta::assert_json_snapshot!("z_get_treestate_unparsable_hash_or_height", rsp)); + + // TODO: + // 1. Request a non-empty Sapling treestate. + // 2. Request an empty Orchard treestate at an NU5 activation height. + // 3. Request a non-empty Orchard treestate. +} + async fn test_rpc_response_data_for_network(network: &Network) { // Create a continuous chain of mainnet and testnet blocks from genesis let block_data = network.blockchain_map(); @@ -241,18 +349,6 @@ async fn test_rpc_response_data_for_network(network: &Network) { snapshot_rpc_getrawmempool(get_raw_mempool, &settings); - // `z_gettreestate` - let tree_state = rpc - .z_get_treestate(BLOCK_HEIGHT.to_string()) - .await - .expect("We should have a GetTreestate struct"); - snapshot_rpc_z_gettreestate_valid(tree_state, &settings); - - let tree_state = rpc - .z_get_treestate(EXCESSIVE_BLOCK_HEIGHT.to_string()) - .await; - snapshot_rpc_z_gettreestate_invalid("excessive_height", tree_state, &settings); - // `getrawtransaction` verbosity=0 // // - similar to `getrawmempool` described above, a mempool request will be made to get the requested @@ -501,22 +597,6 @@ fn snapshot_rpc_getrawmempool(raw_mempool: Vec, settings: &insta::Settin settings.bind(|| insta::assert_json_snapshot!("get_raw_mempool", raw_mempool)); } -/// Snapshot a valid `z_gettreestate` response, using `cargo insta` and JSON serialization. -fn snapshot_rpc_z_gettreestate_valid(tree_state: GetTreestate, settings: &insta::Settings) { - settings.bind(|| insta::assert_json_snapshot!(format!("z_get_treestate_valid"), tree_state)); -} - -/// Snapshot an invalid `z_gettreestate` response, using `cargo insta` and JSON serialization. -fn snapshot_rpc_z_gettreestate_invalid( - variant: &'static str, - tree_state: Result, - settings: &insta::Settings, -) { - settings.bind(|| { - insta::assert_json_snapshot!(format!("z_get_treestate_invalid_{variant}"), tree_state) - }); -} - /// Snapshot `getrawtransaction` response, using `cargo insta` and JSON serialization. fn snapshot_rpc_getrawtransaction( variant: &'static str, diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap new file mode 100644 index 00000000000..5569cdc11bc --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap @@ -0,0 +1,9 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: rsp +--- +{ + "hash": "05a60a92d99d85997cce3b87616c089f6124d7342af37106edc76126334a2c38", + "height": 0, + "time": 1477648033 +} diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap similarity index 86% rename from zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@mainnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap index fd6c362fd59..0f8f4f7eaf8 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: tree_state +expression: rsp --- { "Err": { diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap new file mode 100644 index 00000000000..218d3f22251 --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap @@ -0,0 +1,14 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: rsp +--- +{ + "hash": "00f1a49e54553ac3ef735f2eb1d8247c9a87c22a47dbd7823ae70adcd6c21a18", + "height": 2, + "time": 1477676169, + "sapling": { + "commitments": { + "finalState": "000000" + } + } +} diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap similarity index 86% rename from zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@testnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap index fd6c362fd59..0f8f4f7eaf8 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_invalid_excessive_height@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: tree_state +expression: rsp --- { "Err": { diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap similarity index 88% rename from zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@testnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap index 474b781d30e..402d52a2b78 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: tree_state +expression: rsp --- { "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap new file mode 100644 index 00000000000..ecd3fdd3121 --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap @@ -0,0 +1,10 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: rsp +--- +{ + "Err": { + "code": 0, + "message": "parse error: could not convert the input string to a hash or height" + } +} diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@mainnet_10.snap deleted file mode 100644 index 845d918b214..00000000000 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_valid@mainnet_10.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: zebra-rpc/src/methods/tests/snapshot.rs -expression: tree_state ---- -{ - "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", - "height": 1, - "time": 1477671596 -} From b1bd72d5dcbe5d54ea97bd26d3bfd7ce784c7f4f Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 14 May 2024 22:30:18 +0200 Subject: [PATCH 11/26] Avoid a concurrency issue --- zebra-rpc/src/methods.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index cc1fafee99a..72464abdd08 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1078,7 +1078,7 @@ where match state .ready() .and_then(|service| { - service.call(zebra_state::ReadRequest::SaplingTree(height.into())) + service.call(zebra_state::ReadRequest::SaplingTree(hash.into())) }) .await .map_err(to_error)? @@ -1095,7 +1095,7 @@ where match state .ready() .and_then(|service| { - service.call(zebra_state::ReadRequest::OrchardTree(height.into())) + service.call(zebra_state::ReadRequest::OrchardTree(hash.into())) }) .await .map_err(to_error)? From 824e5200e22438c322914cd776f0b89808a98e97 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 14 May 2024 22:33:16 +0200 Subject: [PATCH 12/26] Fix docs --- zebra-rpc/src/methods/trees.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index a9cbf48b8bc..68336a4c425 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -34,8 +34,8 @@ pub struct GetSubtrees { /// Response to a `z_gettreestate` RPC request. /// -/// Contains the hex-encoded Sapling & Orchard note commitment trees, and their -/// corresponding [`block::Hash`], [`Height`], and block time. +/// Contains the hex-encoded Sapling & Orchard note commitment trees, and their corresponding +/// [`struct@Hash`], [`Height`], and block time. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, Default)] pub struct GetTreestate { /// The block hash corresponding to the treestate, hex-encoded. From 92018197ed9176988505f66c8f07af1fd9ec440d Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 15 May 2024 11:59:49 +0200 Subject: [PATCH 13/26] Impl `Default` for `GetTreestate` --- zebra-rpc/src/methods/trees.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index 68336a4c425..c470b283557 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -36,7 +36,7 @@ pub struct GetSubtrees { /// /// Contains the hex-encoded Sapling & Orchard note commitment trees, and their corresponding /// [`struct@Hash`], [`Height`], and block time. -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, Default)] +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] pub struct GetTreestate { /// The block hash corresponding to the treestate, hex-encoded. #[serde(with = "hex")] @@ -86,6 +86,18 @@ impl GetTreestate { } } +impl Default for GetTreestate { + fn default() -> Self { + Self { + hash: Hash([0; 32]), + height: Height::MIN, + time: Default::default(), + sapling: Default::default(), + orchard: Default::default(), + } + } +} + /// A treestate that is included in the [`z_gettreestate`][1] RPC response. /// /// [1]: https://zcash.github.io/rpc/z_gettreestate.html From 4af8c4a6b69bb65b7f0e8f2d6f6d87e0c280c5f2 Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 16 May 2024 21:35:01 +0200 Subject: [PATCH 14/26] Update zebra-rpc/src/methods.rs Co-authored-by: Arya --- zebra-rpc/src/methods.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 72464abdd08..09acb0acb1f 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -505,7 +505,7 @@ where let (tip_height, tip_hash) = self .latest_chain_tip .best_tip_height_and_hash() - .ok_or(to_error("No Chain tip available yet"))?; + .ok_or_server_error("No Chain tip available yet")?; // `estimated_height` field let current_block_time = self From 55c186e8466a3eb62eca1e1b7e1cb6003902e208 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 11:04:45 +0200 Subject: [PATCH 15/26] Update docs Co-authored-by: Arya --- zebra-chain/src/sapling/tree.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-chain/src/sapling/tree.rs b/zebra-chain/src/sapling/tree.rs index dc6aa8d2967..519d7deeeb9 100644 --- a/zebra-chain/src/sapling/tree.rs +++ b/zebra-chain/src/sapling/tree.rs @@ -217,7 +217,7 @@ impl ToHex for Node { } } -/// Required to serialize [`NoteCommitmentTree`]s in a format compatible with `zcashd`. +/// Required to serialize [`NoteCommitmentTree`]s in a format matching `zcashd`. /// /// Zebra stores Sapling note commitment trees as [`Frontier`]s while the /// [`z_gettreestate`][1] RPC requires [`CommitmentTree`][2]s. Implementing @@ -615,7 +615,7 @@ impl NoteCommitmentTree { assert_eq!(self.to_rpc_bytes(), other.to_rpc_bytes()); } - /// Serializes [`Self`] to a format compatible with `zcashd`'s RPCs. + /// Serializes [`Self`] to a format matching `zcashd`'s RPCs. pub fn to_rpc_bytes(&self) -> Vec { // Convert the tree from [`Frontier`](bridgetree::Frontier) to // [`CommitmentTree`](merkle_tree::CommitmentTree). From d7eca2fdf58cb327b1e25b84693dca51de4b1179 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 11:05:19 +0200 Subject: [PATCH 16/26] Rename `rsp` to `tree_state` Co-authored-by: Arya --- zebra-rpc/src/methods/tests/snapshot.rs | 32 +++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index 0ee6dc90099..a4d0d5ab8a3 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -107,40 +107,46 @@ async fn test_z_get_treestate() { ); // Request the treestate by a hash. - let rsp = rpc + let tree_state = rpc .z_get_treestate(blocks[0].hash().to_string()) .await .expect("genesis treestate = no treestate"); - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_hash", rsp)); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_hash", tree_state)); // Request the treestate by a hash for a block which is not in the state. - let rsp = rpc.z_get_treestate(block::Hash([0; 32]).to_string()).await; - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_non_existent_hash", rsp)); + let tree_state = rpc.z_get_treestate(block::Hash([0; 32]).to_string()).await; + settings + .bind(|| insta::assert_json_snapshot!("z_get_treestate_by_non_existent_hash", tree_state)); // Request the treestate before Sapling activation. - let rsp = rpc + let tree_state = rpc .z_get_treestate((SAPLING_ACTIVATION_HEIGHT - 1).to_string()) .await .expect("no Sapling treestate and no Orchard treestate"); - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_no_treestate", rsp)); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_no_treestate", tree_state)); // Request the treestate at Sapling activation. - let rsp = rpc + let tree_state = rpc .z_get_treestate(SAPLING_ACTIVATION_HEIGHT.to_string()) .await .expect("empty Sapling treestate and no Orchard treestate"); - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_empty_Sapling_treestate", rsp)); + settings.bind(|| { + insta::assert_json_snapshot!("z_get_treestate_empty_Sapling_treestate", tree_state) + }); // Request the treestate for an invalid height. - let rsp = rpc + let tree_state = rpc .z_get_treestate(EXCESSIVE_BLOCK_HEIGHT.to_string()) .await; - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_excessive_block_height", rsp)); + settings.bind(|| { + insta::assert_json_snapshot!("z_get_treestate_excessive_block_height", tree_state) + }); // Request the treestate for an unparsable hash or height. - let rsp = rpc.z_get_treestate("Do you even shield?".to_string()).await; - settings - .bind(|| insta::assert_json_snapshot!("z_get_treestate_unparsable_hash_or_height", rsp)); + let tree_state = rpc.z_get_treestate("Do you even shield?".to_string()).await; + settings.bind(|| { + insta::assert_json_snapshot!("z_get_treestate_unparsable_hash_or_height", tree_state) + }); // TODO: // 1. Request a non-empty Sapling treestate. From 829ae6b558f0d4cc90cd87394e272b9eb0349867 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 12:40:47 +0200 Subject: [PATCH 17/26] Describe the serialization format of treestates --- zebra-rpc/src/methods/trees.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/zebra-rpc/src/methods/trees.rs b/zebra-rpc/src/methods/trees.rs index c470b283557..81b87227b09 100644 --- a/zebra-rpc/src/methods/trees.rs +++ b/zebra-rpc/src/methods/trees.rs @@ -34,8 +34,19 @@ pub struct GetSubtrees { /// Response to a `z_gettreestate` RPC request. /// -/// Contains the hex-encoded Sapling & Orchard note commitment trees, and their corresponding +/// Contains hex-encoded Sapling & Orchard note commitment trees and their corresponding /// [`struct@Hash`], [`Height`], and block time. +/// +/// The format of the serialized trees represents `CommitmentTree`s from the crate +/// `incrementalmerkletree` and not `Frontier`s from the same crate, even though `zebrad`'s +/// `NoteCommitmentTree`s are implemented using `Frontier`s. Zebra follows the former format to stay +/// consistent with `zcashd`'s RPCs. +/// +/// The formats are semantically equivalent. The difference is that in `Frontier`s, the vector of +/// ommers is dense (we know where the gaps are from the position of the leaf in the overall tree); +/// whereas in `CommitmentTree`, the vector of ommers is sparse with [`None`] values in the gaps. +/// +/// The dense format might be used in future RPCs. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] pub struct GetTreestate { /// The block hash corresponding to the treestate, hex-encoded. From 2c3a8dbe8f3dd4a5bb256829c2e44f993d8d32de Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 13:48:40 +0200 Subject: [PATCH 18/26] Refactor error handling --- zebra-rpc/src/methods.rs | 51 +++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 09acb0acb1f..37d45fdb040 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -511,12 +511,12 @@ where let current_block_time = self .latest_chain_tip .best_tip_block_time() - .ok_or(to_error("No Chain tip available yet"))?; + .ok_or_server_error("No Chain tip available yet")?; let zebra_estimated_height = self .latest_chain_tip .estimate_network_chain_tip_height(network, Utc::now()) - .ok_or(to_error("No Chain tip available yet"))?; + .ok_or_server_error("No Chain tip available yet")?; let mut estimated_height = if current_block_time > Utc::now() || zebra_estimated_height < tip_height { @@ -595,7 +595,7 @@ where let valid_addresses = address_strings.valid_addresses()?; let request = zebra_state::ReadRequest::AddressBalance(valid_addresses); - let response = state.oneshot(request).await.map_err(to_error)?; + let response = state.oneshot(request).await.map_server_error()?; match response { zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance { @@ -632,7 +632,7 @@ where let transaction_parameter = mempool::Gossip::Tx(raw_transaction.into()); let request = mempool::Request::Queue(vec![transaction_parameter]); - let response = mempool.oneshot(request).await.map_err(to_error)?; + let response = mempool.oneshot(request).await.map_server_error()?; let queue_results = match response { mempool::Response::Queued(results) => results, @@ -650,7 +650,7 @@ where queue_results[0] .as_ref() .map(|_| SentTransactionHash(transaction_hash)) - .map_err(to_error) + .map_server_error() } .boxed() } @@ -671,7 +671,7 @@ where let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY); async move { - let hash_or_height: HashOrHeight = hash_or_height.parse().map_err(to_error)?; + let hash_or_height: HashOrHeight = hash_or_height.parse().map_server_error()?; if verbosity == 0 { // # Performance @@ -683,7 +683,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; match response { zebra_state::ReadResponse::Block(Some(block)) => { @@ -727,7 +727,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; match response { zebra_state::ReadResponse::BlockHash(Some(hash)) => hash, @@ -909,7 +909,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; match response { #[cfg(feature = "getblocktemplate-rpcs")] @@ -988,7 +988,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; match response { mempool::Response::Transactions(unmined_transactions) => { @@ -1006,7 +1006,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; match response { zebra_state::ReadResponse::Transaction(Some(MinedTx { @@ -1020,7 +1020,7 @@ where verbose, )), zebra_state::ReadResponse::Transaction(None) => { - Err(to_error("Transaction not found")) + Err("Transaction not found").map_server_error() } _ => unreachable!("unmatched response to a transaction request"), } @@ -1034,7 +1034,7 @@ where async move { // Convert the [`hash_or_height`] string into an actual hash or height. - let hash_or_height = hash_or_height.parse().map_err(to_error)?; + let hash_or_height = hash_or_height.parse().map_server_error()?; // Fetch the block referenced by [`hash_or_height`] from the state. // @@ -1049,7 +1049,7 @@ where .ready() .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height))) .await - .map_err(to_error)? + .map_server_error()? { zebra_state::ReadResponse::Block(Some(block)) => block, zebra_state::ReadResponse::Block(None) => { @@ -1081,7 +1081,7 @@ where service.call(zebra_state::ReadRequest::SaplingTree(hash.into())) }) .await - .map_err(to_error)? + .map_server_error()? { zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to a Sapling tree request"), @@ -1098,7 +1098,7 @@ where service.call(zebra_state::ReadRequest::OrchardTree(hash.into())) }) .await - .map_err(to_error)? + .map_server_error()? { zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to an Orchard tree request"), @@ -1131,7 +1131,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; let subtrees = match response { zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees, @@ -1157,7 +1157,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; let subtrees = match response { zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees, @@ -1218,7 +1218,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; let hashes = match response { zebra_state::ReadResponse::AddressesTransactionIds(hashes) => { @@ -1266,7 +1266,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_err(to_error)?; + .map_server_error()?; let utxos = match response { zebra_state::ReadResponse::AddressUtxos(utxos) => utxos, _ => unreachable!("unmatched response to a UtxosByAddresses request"), @@ -1318,7 +1318,7 @@ where { latest_chain_tip .best_tip_height() - .ok_or(to_error("No blocks in state")) + .ok_or_server_error("No blocks in state") } /// Response to a `getinfo` RPC request. @@ -1751,12 +1751,3 @@ pub fn height_from_signed_int(index: i32, tip_height: Height) -> Result Ok(Height(sanitized_height)) } } - -/// Turns anything implementing [`ToString`] into [`Error`]. -fn to_error(err: impl ToString) -> Error { - Error { - code: ErrorCode::ServerError(0), - message: err.to_string(), - data: None, - } -} From 763d896b38858982ae24d6c882c58841d83b1691 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 13:50:47 +0200 Subject: [PATCH 19/26] Refactor imports --- zebra-rpc/src/methods.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 37d45fdb040..ab120e364e8 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -33,7 +33,7 @@ use zebra_state::{HashOrHeight, MinedTx, OutputIndex, OutputLocation, Transactio use crate::{ constants::{INVALID_PARAMETERS_ERROR_CODE, MISSING_BLOCK_ERROR_CODE}, - methods::trees::{GetSubtrees, SubtreeRpcData}, + methods::trees::{GetSubtrees, GetTreestate, SubtreeRpcData}, queue::Queue, }; @@ -50,8 +50,6 @@ pub mod get_block_template_rpcs; #[cfg(feature = "getblocktemplate-rpcs")] pub use get_block_template_rpcs::{GetBlockTemplateRpc, GetBlockTemplateRpcImpl}; -use self::trees::GetTreestate; - #[cfg(test)] mod tests; From ba8def67088774db4cab2f74ad611d7c1fea8547 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 13:59:57 +0200 Subject: [PATCH 20/26] Apply suggestions from code review Co-authored-by: Arya --- zebra-rpc/src/methods.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index ab120e364e8..80e2b034c32 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1041,8 +1041,7 @@ where // For consistency, this lookup must be performed first, then all the other lookups must // be based on the hash. // - // TODO: If this RPC is called a lot, just get the block header, rather than the whole - // block. + // TODO: If this RPC is called a lot, just get the block header, rather than the whole block. let block = match state .ready() .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height))) @@ -1066,7 +1065,7 @@ where let height = hash_or_height .height_or_else(|_| block.coinbase_height()) - .expect("block height"); + .expect("verified blocks have a coinbase height"); let time = u32::try_from(block.header.time.timestamp()) .expect("Timestamps of valid blocks always fit into u32."); From 9c3605101472b7885f67fe8b126415ae18cf1dd0 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 14:08:37 +0200 Subject: [PATCH 21/26] Use `treestate` in snapshots --- zebra-rpc/src/methods/tests/snapshot.rs | 27 +++++++++---------- ..._get_treestate_by_hash@custom_testnet.snap | 2 +- ...e_by_non_existent_hash@custom_testnet.snap | 2 +- ...mpty_Sapling_treestate@custom_testnet.snap | 2 +- ...excessive_block_height@custom_testnet.snap | 2 +- ...treestate_no_treestate@custom_testnet.snap | 2 +- ...arsable_hash_or_height@custom_testnet.snap | 2 +- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index a4d0d5ab8a3..eed914c78e0 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -107,45 +107,44 @@ async fn test_z_get_treestate() { ); // Request the treestate by a hash. - let tree_state = rpc + let treestate = rpc .z_get_treestate(blocks[0].hash().to_string()) .await .expect("genesis treestate = no treestate"); - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_hash", tree_state)); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_by_hash", treestate)); // Request the treestate by a hash for a block which is not in the state. - let tree_state = rpc.z_get_treestate(block::Hash([0; 32]).to_string()).await; + let treestate = rpc.z_get_treestate(block::Hash([0; 32]).to_string()).await; settings - .bind(|| insta::assert_json_snapshot!("z_get_treestate_by_non_existent_hash", tree_state)); + .bind(|| insta::assert_json_snapshot!("z_get_treestate_by_non_existent_hash", treestate)); // Request the treestate before Sapling activation. - let tree_state = rpc + let treestate = rpc .z_get_treestate((SAPLING_ACTIVATION_HEIGHT - 1).to_string()) .await .expect("no Sapling treestate and no Orchard treestate"); - settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_no_treestate", tree_state)); + settings.bind(|| insta::assert_json_snapshot!("z_get_treestate_no_treestate", treestate)); // Request the treestate at Sapling activation. - let tree_state = rpc + let treestate = rpc .z_get_treestate(SAPLING_ACTIVATION_HEIGHT.to_string()) .await .expect("empty Sapling treestate and no Orchard treestate"); settings.bind(|| { - insta::assert_json_snapshot!("z_get_treestate_empty_Sapling_treestate", tree_state) + insta::assert_json_snapshot!("z_get_treestate_empty_Sapling_treestate", treestate) }); // Request the treestate for an invalid height. - let tree_state = rpc + let treestate = rpc .z_get_treestate(EXCESSIVE_BLOCK_HEIGHT.to_string()) .await; - settings.bind(|| { - insta::assert_json_snapshot!("z_get_treestate_excessive_block_height", tree_state) - }); + settings + .bind(|| insta::assert_json_snapshot!("z_get_treestate_excessive_block_height", treestate)); // Request the treestate for an unparsable hash or height. - let tree_state = rpc.z_get_treestate("Do you even shield?".to_string()).await; + let treestate = rpc.z_get_treestate("Do you even shield?".to_string()).await; settings.bind(|| { - insta::assert_json_snapshot!("z_get_treestate_unparsable_hash_or_height", tree_state) + insta::assert_json_snapshot!("z_get_treestate_unparsable_hash_or_height", treestate) }); // TODO: diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap index 5569cdc11bc..c262f204552 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_hash@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "hash": "05a60a92d99d85997cce3b87616c089f6124d7342af37106edc76126334a2c38", diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap index 0f8f4f7eaf8..d0013994ab0 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "Err": { diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap index 218d3f22251..3ba356fe52b 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_empty_Sapling_treestate@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "hash": "00f1a49e54553ac3ef735f2eb1d8247c9a87c22a47dbd7823ae70adcd6c21a18", diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap index 0f8f4f7eaf8..d0013994ab0 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "Err": { diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap index 402d52a2b78..7c77e4c3a6d 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_no_treestate@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap index ecd3fdd3121..a45d7e298dc 100644 --- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap +++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap @@ -1,6 +1,6 @@ --- source: zebra-rpc/src/methods/tests/snapshot.rs -expression: rsp +expression: treestate --- { "Err": { From 9a282cee80bc9e5fabf2185499671467558c151c Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 14:11:42 +0200 Subject: [PATCH 22/26] Use `ok_or_server_error` --- zebra-rpc/src/methods.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index ab120e364e8..87461a0e615 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -873,11 +873,7 @@ where self.latest_chain_tip .best_tip_hash() .map(GetBlockHash) - .ok_or(Error { - code: ErrorCode::ServerError(0), - message: "No blocks in state".to_string(), - data: None, - }) + .ok_or_server_error("No blocks in state") } // TODO: use a generic error constructor (#5548) From 0799cb23897e5a8114cceccd9f8ff5f2a108c023 Mon Sep 17 00:00:00 2001 From: Marek Date: Mon, 20 May 2024 22:37:25 +0200 Subject: [PATCH 23/26] Bump `zcash_primitives` from 0.13.0 to 0.14.0 Co-authored-by: Alfredo Garcia --- Cargo.lock | 2 +- zebra-rpc/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a08edc5f61e..303818a4c8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6191,7 +6191,7 @@ dependencies = [ "tower", "tracing", "zcash_address", - "zcash_primitives 0.13.0", + "zcash_primitives 0.14.0", "zebra-chain", "zebra-consensus", "zebra-network", diff --git a/zebra-rpc/Cargo.toml b/zebra-rpc/Cargo.toml index a5a6396b7ac..3a88ac8470f 100644 --- a/zebra-rpc/Cargo.toml +++ b/zebra-rpc/Cargo.toml @@ -64,7 +64,7 @@ tracing = "0.1.39" hex = { version = "0.4.3", features = ["serde"] } serde = { version = "1.0.201", features = ["serde_derive"] } -zcash_primitives = { version = "0.13.0" } +zcash_primitives = { version = "0.14.0" } # Experimental feature getblocktemplate-rpcs rand = { version = "0.8.5", optional = true } From 787aea079435b39971684f3bd5175957bba052e9 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 21 May 2024 00:47:13 +0200 Subject: [PATCH 24/26] Remove an outdated TODO --- zebra-rpc/src/methods.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 292ccdb7545..23c8e79d9f0 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -656,7 +656,6 @@ where // TODO: // - use `height_from_signed_int()` to handle negative heights // (this might be better in the state request, because it needs the state height) - // - create a function that handles block hashes or heights, and use it in `z_get_treestate()` fn get_block( &self, hash_or_height: String, From 641fd718a19f847e8a078f25ed63c2ab937a00cc Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 21 May 2024 00:47:28 +0200 Subject: [PATCH 25/26] Add a TODO on negative heights for treestates --- zebra-rpc/src/methods.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 23c8e79d9f0..0ff129a643f 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -1021,6 +1021,9 @@ where .boxed() } + // TODO: + // - use `height_from_signed_int()` to handle negative heights + // (this might be better in the state request, because it needs the state height) fn z_get_treestate(&self, hash_or_height: String) -> BoxFuture> { let mut state = self.state.clone(); let network = self.network.clone(); From 318f945d2026a4cdbb8323a30d4b90b99e4de3ab Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 21 May 2024 14:55:51 +0200 Subject: [PATCH 26/26] Revert "Bump `zcash_primitives` from 0.13.0 to 0.14.0" This reverts commit 0799cb23897e5a8114cceccd9f8ff5f2a108c023. --- Cargo.lock | 2 +- zebra-rpc/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fc04e7e5cbf..64be39e18e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6191,7 +6191,7 @@ dependencies = [ "tower", "tracing", "zcash_address", - "zcash_primitives 0.14.0", + "zcash_primitives 0.13.0", "zebra-chain", "zebra-consensus", "zebra-network", diff --git a/zebra-rpc/Cargo.toml b/zebra-rpc/Cargo.toml index 3c5c382a04d..2f8288fe421 100644 --- a/zebra-rpc/Cargo.toml +++ b/zebra-rpc/Cargo.toml @@ -64,7 +64,7 @@ tracing = "0.1.39" hex = { version = "0.4.3", features = ["serde"] } serde = { version = "1.0.202", features = ["serde_derive"] } -zcash_primitives = { version = "0.14.0" } +zcash_primitives = { version = "0.13.0" } # Experimental feature getblocktemplate-rpcs rand = { version = "0.8.5", optional = true }