Skip to content

Commit

Permalink
fix(rpc): Refactor the serialization of note commitment trees (#8533)
Browse files Browse the repository at this point in the history 
* Remove `orchard::tree::SerializedTree`

* Move `GetTreestate` to the `trees` module

* Update comments

* Remove `sapling::tree::SerializedTree`

* Make the serialization compatible with `zcashd`

* Simplify error handling

* Derive `Default` for `GetTreestate`

* Remove old TODOs

* Simplify the `z_get_treestate` method

* Add & refactor tests

* Avoid a concurrency issue

* Fix docs

* Impl `Default` for `GetTreestate`

* Update zebra-rpc/src/methods.rs

Co-authored-by: Arya <aryasolhi@gmail.com>

* Update docs

Co-authored-by: Arya <aryasolhi@gmail.com>

* Rename `rsp` to `tree_state`

Co-authored-by: Arya <aryasolhi@gmail.com>

* Describe the serialization format of treestates

* Refactor error handling

* Refactor imports

* Apply suggestions from code review

Co-authored-by: Arya <aryasolhi@gmail.com>

* Use `treestate` in snapshots

* Use `ok_or_server_error`

* Bump  `zcash_primitives` from 0.13.0 to 0.14.0

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

* Remove an outdated TODO

* Add a TODO on negative heights for treestates

* Revert "Bump `zcash_primitives` from 0.13.0 to 0.14.0"

This reverts commit 0799cb2.

---------

Co-authored-by: Arya <aryasolhi@gmail.com>
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
  • Loading branch information
@upbqdn @arya2 @oxarbitrage
3 people authored May 22, 2024
1 parent b06a122 commit eade2a8
Show file tree
Hide file tree
Showing 15 changed files with 376 additions and 525 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6171,6 +6171,7 @@ dependencies = [
"tower",
"tracing",
"zcash_address",
"zcash_primitives 0.13.0",
"zebra-chain",
"zebra-consensus",
"zebra-network",
Expand Down
86 changes: 17 additions & 69 deletions zebra-chain/src/orchard/tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use std::{
fmt,
hash::{Hash, Hasher},
io,
sync::Arc,
};

use bitvec::prelude::*;
Expand All @@ -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::*;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<u8> {
// 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
}
}

Expand Down Expand Up @@ -688,68 +701,3 @@ impl From<Vec<pallas::Base>> 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<u8>);

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<Option<Arc<NoteCommitmentTree>>> for SerializedTree {
fn from(maybe_tree: Option<Arc<NoteCommitmentTree>>) -> 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
}
}
154 changes: 17 additions & 137 deletions zebra-chain/src/sapling/tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use std::{
fmt,
hash::{Hash, Hasher},
io,
sync::Arc,
};

use bitvec::prelude::*;
Expand All @@ -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;
Expand All @@ -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.
///
Expand Down Expand Up @@ -219,7 +217,7 @@ impl ToHex for Node {
}
}

/// Required to convert [`NoteCommitmentTree`] into [`SerializedTree`].
/// 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
Expand Down Expand Up @@ -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 matching `zcashd`'s RPCs.
pub fn to_rpc_bytes(&self) -> Vec<u8> {
// 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
}
}

Expand Down Expand Up @@ -670,135 +682,3 @@ impl From<Vec<jubjub::Fq>> 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<u8>);

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]: <https://github.com/zcash/librustzcash/blob/a63a37a/zcash_primitives/src/merkle_tree.rs#L125>
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<Option<Arc<NoteCommitmentTree>>> for SerializedTree {
fn from(maybe_tree: Option<Arc<NoteCommitmentTree>>) -> 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
}
}
2 changes: 2 additions & 0 deletions zebra-rpc/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ tracing = "0.1.39"
hex = { version = "0.4.3", features = ["serde"] }
serde = { version = "1.0.202", 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
Expand Down
Loading

0 comments on commit eade2a8

Please sign in to comment.