octo
/
Zebra
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

change(state): Refactor the naming of note commitment subtrees (#7855) 

* Rename `node` & `Node` to `root` & `Root`

* Rename `end` to `end_height`

* Rename `Root` to `SubtreeRoot`
This commit is contained in:
Marek 2023-10-30 21:06:54 +01:00 committed by GitHub
parent f3048653c8
commit 0d3892f61f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 65 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
zebra-chain/src/subtree.rs
View File

@ -50,31 +50,38 @@ impl From<NoteCommitmentSubtreeIndex> for u64 {
// TODO: // TODO:
// - consider defining sapling::SubtreeRoot and orchard::SubtreeRoot types or type wrappers, // - consider defining sapling::SubtreeRoot and orchard::SubtreeRoot types or type wrappers,
// to avoid type confusion between the leaf Node and subtree root types. // to avoid type confusion between the leaf Node and subtree root types.
// - rename the `Node` generic to `SubtreeRoot`
/// Subtree root of Sapling or Orchard note commitment tree, /// Subtree root of Sapling or Orchard note commitment tree,
/// with its associated block height and subtree index. /// with its associated block height and subtree index.
#[derive(Copy, Clone, Debug, Eq, PartialEq)] #[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
pub struct NoteCommitmentSubtree<Node> { pub struct NoteCommitmentSubtree<SubtreeRoot> {
/// Index of this subtree /// Index of this subtree
pub index: NoteCommitmentSubtreeIndex, pub index: NoteCommitmentSubtreeIndex,
/// Root of this subtree. /// Root of this subtree.
pub node: Node, pub root: SubtreeRoot,
/// End boundary of this subtree, the block height of its last leaf. /// End boundary of this subtree, the block height of its last leaf.
pub end: Height, pub end_height: Height,
} }
impl<Node> NoteCommitmentSubtree<Node> { impl<SubtreeRoot> NoteCommitmentSubtree<SubtreeRoot> {
/// Creates new [`NoteCommitmentSubtree`] /// Creates new [`NoteCommitmentSubtree`]
pub fn new(index: impl Into<NoteCommitmentSubtreeIndex>, end: Height, node: Node) -> Self { pub fn new(
index: impl Into<NoteCommitmentSubtreeIndex>,
end_height: Height,
root: SubtreeRoot,
) -> Self {
let index = index.into(); let index = index.into();
Self { index, end, node } Self {
index,
end_height,
root,
}
} }
/// Converts struct to [`NoteCommitmentSubtreeData`]. /// Converts struct to [`NoteCommitmentSubtreeData`].
pub fn into_data(self) -> NoteCommitmentSubtreeData<Node> { pub fn into_data(self) -> NoteCommitmentSubtreeData<SubtreeRoot> {
NoteCommitmentSubtreeData::new(self.end, self.node) NoteCommitmentSubtreeData::new(self.end_height, self.root)
} }
} }
@ -82,29 +89,25 @@ impl<Node> NoteCommitmentSubtree<Node> {
/// Used for database key-value serialization, where the subtree index is the key, and this struct is the value. /// Used for database key-value serialization, where the subtree index is the key, and this struct is the value.
#[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize)] #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize)]
#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
pub struct NoteCommitmentSubtreeData<Node> { pub struct NoteCommitmentSubtreeData<SubtreeRoot> {
/// Merkle root of the 2^16-leaf subtree. /// Merkle root of the 2^16-leaf subtree.
// pub root: SubtreeRoot,
// TODO: rename both Rust fields to match the RPC field names
#[serde(rename = "root")]
pub node: Node,
/// Height of the block containing the note that completed this subtree. /// Height of the block containing the note that completed this subtree.
#[serde(rename = "end_height")] pub end_height: Height,
pub end: Height,
} }
impl<Node> NoteCommitmentSubtreeData<Node> { impl<SubtreeRoot> NoteCommitmentSubtreeData<SubtreeRoot> {
/// Creates new [`NoteCommitmentSubtreeData`] /// Creates new [`NoteCommitmentSubtreeData`]
pub fn new(end: Height, node: Node) -> Self { pub fn new(end_height: Height, root: SubtreeRoot) -> Self {
Self { end, node } Self { end_height, root }
} }
/// Creates new [`NoteCommitmentSubtree`] from a [`NoteCommitmentSubtreeData`] and index /// Creates new [`NoteCommitmentSubtree`] from a [`NoteCommitmentSubtreeData`] and index
pub fn with_index( pub fn with_index(
self, self,
index: impl Into<NoteCommitmentSubtreeIndex>, index: impl Into<NoteCommitmentSubtreeIndex>,
) -> NoteCommitmentSubtree<Node> { ) -> NoteCommitmentSubtree<SubtreeRoot> {
NoteCommitmentSubtree::new(index, self.end, self.node) NoteCommitmentSubtree::new(index, self.end_height, self.root)
} }
} }

8
zebra-rpc/src/methods.rs
View File

@ -1172,8 +1172,8 @@ where
let subtrees = subtrees let subtrees = subtrees
.values() .values()
.map(|subtree| SubtreeRpcData { .map(|subtree| SubtreeRpcData {
node: subtree.node.encode_hex(), root: subtree.root.encode_hex(),
end: subtree.end, end_height: subtree.end_height,
}) })
.collect(); .collect();
@ -1202,8 +1202,8 @@ where
let subtrees = subtrees let subtrees = subtrees
.values() .values()
.map(|subtree| SubtreeRpcData { .map(|subtree| SubtreeRpcData {
node: subtree.node.encode_hex(), root: subtree.root.encode_hex(),
end: subtree.end, end_height: subtree.end_height,
}) })
.collect(); .collect();

4
zebra-state/src/service/finalized_state/disk_format/shielded.rs
View File

@ -178,11 +178,11 @@ impl IntoDisk for orchard::tree::Node {
} }
} }
impl<Node: IntoDisk<Bytes = Vec<u8>>> IntoDisk for NoteCommitmentSubtreeData<Node> { impl<Root: IntoDisk<Bytes = Vec<u8>>> IntoDisk for NoteCommitmentSubtreeData<Root> {
type Bytes = Vec<u8>; type Bytes = Vec<u8>;
fn as_bytes(&self) -> Self::Bytes { fn as_bytes(&self) -> Self::Bytes {
[self.end.as_bytes().to_vec(), self.node.as_bytes()].concat() [self.end_height.as_bytes().to_vec(), self.root.as_bytes()].concat()
} }
} }

6
zebra-state/src/service/finalized_state/disk_format/tests/prop.rs
View File

@ -376,7 +376,7 @@ fn roundtrip_sapling_subtree_data() {
let _init_guard = zebra_test::init(); let _init_guard = zebra_test::init();
proptest!(|(mut val in any::<NoteCommitmentSubtreeData<sapling::tree::Node>>())| { proptest!(|(mut val in any::<NoteCommitmentSubtreeData<sapling::tree::Node>>())| {
val.end.0 %= MAX_ON_DISK_HEIGHT.0 + 1; val.end_height.0 %= MAX_ON_DISK_HEIGHT.0 + 1;
assert_value_properties(val) assert_value_properties(val)
}); });
} }
@ -461,8 +461,8 @@ fn roundtrip_orchard_subtree_data() {
let _init_guard = zebra_test::init(); let _init_guard = zebra_test::init();
proptest!(|(mut val in any::<NoteCommitmentSubtreeData<orchard::tree::Node>>())| { proptest!(|(mut val in any::<NoteCommitmentSubtreeData<orchard::tree::Node>>())| {
val.end = val.end.clamp(Height(0), MAX_ON_DISK_HEIGHT); val.end_height = val.end_height.clamp(Height(0), MAX_ON_DISK_HEIGHT);
val.end.0 %= MAX_ON_DISK_HEIGHT.0 + 1; val.end_height.0 %= MAX_ON_DISK_HEIGHT.0 + 1;
assert_value_properties(val) assert_value_properties(val)
}); });
} }

48
zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs
View File

@ -171,11 +171,11 @@ fn first_sapling_mainnet_subtree() -> NoteCommitmentSubtree<sapling::tree::Node>
// ``` // ```
NoteCommitmentSubtree { NoteCommitmentSubtree {
index: 0.into(), index: 0.into(),
node: hex!("754bb593ea42d231a7ddf367640f09bbf59dc00f2c1d2003cc340e0c016b5b13") root: hex!("754bb593ea42d231a7ddf367640f09bbf59dc00f2c1d2003cc340e0c016b5b13")
.as_slice() .as_slice()
.try_into() .try_into()
.expect("test vector is valid"), .expect("test vector is valid"),
end: Height(558822), end_height: Height(558822),
} }
} }
@ -187,11 +187,11 @@ fn first_orchard_mainnet_subtree() -> NoteCommitmentSubtree<orchard::tree::Node>
// ``` // ```
NoteCommitmentSubtree { NoteCommitmentSubtree {
index: 0.into(), index: 0.into(),
node: hex!("d4e323b3ae0cabfb6be4087fec8c66d9a9bbfc354bf1d9588b6620448182063b") root: hex!("d4e323b3ae0cabfb6be4087fec8c66d9a9bbfc354bf1d9588b6620448182063b")
.as_slice() .as_slice()
.try_into() .try_into()
.expect("test vector is valid"), .expect("test vector is valid"),
end: Height(1707429), end_height: Height(1707429),
} }
} }
@ -360,9 +360,9 @@ fn check_sapling_subtrees(
}; };
// Check that there was a sapling note at the subtree's end height. // Check that there was a sapling note at the subtree's end height.
let Some(tree) = db.sapling_tree_by_height(&subtree.end) else { let Some(tree) = db.sapling_tree_by_height(&subtree.end_height) else {
result = Err("missing note commitment tree at subtree completion height"); result = Err("missing note commitment tree at subtree completion height");
error!(?result, ?subtree.end); error!(?result, ?subtree.end_height);
continue; continue;
}; };
@ -373,7 +373,7 @@ fn check_sapling_subtrees(
error!(?result); error!(?result);
} }
if subtree.node != node { if subtree.root != node {
result = Err("completed subtree roots should match"); result = Err("completed subtree roots should match");
error!(?result); error!(?result);
} }
@ -381,13 +381,13 @@ fn check_sapling_subtrees(
// Check that the final note has a greater subtree index if it didn't complete a subtree. // Check that the final note has a greater subtree index if it didn't complete a subtree.
else { else {
let prev_height = subtree let prev_height = subtree
.end .end_height
.previous() .previous()
.expect("Note commitment subtrees should not end at the minimal height."); .expect("Note commitment subtrees should not end at the minimal height.");
let Some(prev_tree) = db.sapling_tree_by_height(&prev_height) else { let Some(prev_tree) = db.sapling_tree_by_height(&prev_height) else {
result = Err("missing note commitment tree below subtree completion height"); result = Err("missing note commitment tree below subtree completion height");
error!(?result, ?subtree.end); error!(?result, ?subtree.end_height);
continue; continue;
}; };
@ -430,15 +430,15 @@ fn check_sapling_subtrees(
}; };
// Check that the subtree end height matches that in the sapling trees. // Check that the subtree end height matches that in the sapling trees.
if subtree.end != height { if subtree.end_height != height {
let is_complete = tree.is_complete_subtree(); let is_complete = tree.is_complete_subtree();
result = Err("bad sapling subtree end height"); result = Err("bad sapling subtree end height");
error!(?result, ?subtree.end, ?height, ?index, ?is_complete, ); error!(?result, ?subtree.end_height, ?height, ?index, ?is_complete, );
} }
// Check the root if the sapling note commitment tree at this height is a complete subtree. // Check the root if the sapling note commitment tree at this height is a complete subtree.
if let Some((_index, node)) = tree.completed_subtree_index_and_root() { if let Some((_index, node)) = tree.completed_subtree_index_and_root() {
if subtree.node != node { if subtree.root != node {
result = Err("completed subtree roots should match"); result = Err("completed subtree roots should match");
error!(?result); error!(?result);
} }
@ -490,9 +490,9 @@ fn check_orchard_subtrees(
}; };
// Check that there was a orchard note at the subtree's end height. // Check that there was a orchard note at the subtree's end height.
let Some(tree) = db.orchard_tree_by_height(&subtree.end) else { let Some(tree) = db.orchard_tree_by_height(&subtree.end_height) else {
result = Err("missing note commitment tree at subtree completion height"); result = Err("missing note commitment tree at subtree completion height");
error!(?result, ?subtree.end); error!(?result, ?subtree.end_height);
continue; continue;
}; };
@ -503,7 +503,7 @@ fn check_orchard_subtrees(
error!(?result); error!(?result);
} }
if subtree.node != node { if subtree.root != node {
result = Err("completed subtree roots should match"); result = Err("completed subtree roots should match");
error!(?result); error!(?result);
} }
@ -511,13 +511,13 @@ fn check_orchard_subtrees(
// Check that the final note has a greater subtree index if it didn't complete a subtree. // Check that the final note has a greater subtree index if it didn't complete a subtree.
else { else {
let prev_height = subtree let prev_height = subtree
.end .end_height
.previous() .previous()
.expect("Note commitment subtrees should not end at the minimal height."); .expect("Note commitment subtrees should not end at the minimal height.");
let Some(prev_tree) = db.orchard_tree_by_height(&prev_height) else { let Some(prev_tree) = db.orchard_tree_by_height(&prev_height) else {
result = Err("missing note commitment tree below subtree completion height"); result = Err("missing note commitment tree below subtree completion height");
error!(?result, ?subtree.end); error!(?result, ?subtree.end_height);
continue; continue;
}; };
@ -560,15 +560,15 @@ fn check_orchard_subtrees(
}; };
// Check that the subtree end height matches that in the orchard trees. // Check that the subtree end height matches that in the orchard trees.
if subtree.end != height { if subtree.end_height != height {
let is_complete = tree.is_complete_subtree(); let is_complete = tree.is_complete_subtree();
result = Err("bad orchard subtree end height"); result = Err("bad orchard subtree end height");
error!(?result, ?subtree.end, ?height, ?index, ?is_complete, ); error!(?result, ?subtree.end_height, ?height, ?index, ?is_complete, );
} }
// Check the root if the orchard note commitment tree at this height is a complete subtree. // Check the root if the orchard note commitment tree at this height is a complete subtree.
if let Some((_index, node)) = tree.completed_subtree_index_and_root() { if let Some((_index, node)) = tree.completed_subtree_index_and_root() {
if subtree.node != node { if subtree.root != node {
result = Err("completed subtree roots should match"); result = Err("completed subtree roots should match");
error!(?result); error!(?result);
} }
@ -851,10 +851,10 @@ fn write_sapling_subtree(
.expect("writing sapling note commitment subtrees should always succeed."); .expect("writing sapling note commitment subtrees should always succeed.");
if subtree.index.0 % 100 == 0 { if subtree.index.0 % 100 == 0 {
info!(end_height = ?subtree.end, index = ?subtree.index.0, "calculated and added sapling subtree"); info!(end_height = ?subtree.end_height, index = ?subtree.index.0, "calculated and added sapling subtree");
} }
// This log happens about once per second on recent machines with SSD disks. // This log happens about once per second on recent machines with SSD disks.
debug!(end_height = ?subtree.end, index = ?subtree.index.0, "calculated and added sapling subtree"); debug!(end_height = ?subtree.end_height, index = ?subtree.index.0, "calculated and added sapling subtree");
} }
/// Writes an Orchard note commitment subtree to `upgrade_db`. /// Writes an Orchard note commitment subtree to `upgrade_db`.
@ -871,8 +871,8 @@ fn write_orchard_subtree(
.expect("writing orchard note commitment subtrees should always succeed."); .expect("writing orchard note commitment subtrees should always succeed.");
if subtree.index.0 % 100 == 0 { if subtree.index.0 % 100 == 0 {
info!(end_height = ?subtree.end, index = ?subtree.index.0, "calculated and added orchard subtree"); info!(end_height = ?subtree.end_height, index = ?subtree.index.0, "calculated and added orchard subtree");
} }
// This log happens about once per second on recent machines with SSD disks. // This log happens about once per second on recent machines with SSD disks.
debug!(end_height = ?subtree.end, index = ?subtree.index.0, "calculated and added orchard subtree"); debug!(end_height = ?subtree.end_height, index = ?subtree.index.0, "calculated and added orchard subtree");
} }

4
zebra-state/src/service/finalized_state/zebra_db/shielded.rs
View File

@ -275,7 +275,7 @@ impl ZebraDb {
) = self.db.zs_last_key_value(&sapling_subtrees)?; ) = self.db.zs_last_key_value(&sapling_subtrees)?;
let tip_height = self.finalized_tip_height()?; let tip_height = self.finalized_tip_height()?;
if subtree_data.end != tip_height { if subtree_data.end_height != tip_height {
return None; return None;
} }
@ -401,7 +401,7 @@ impl ZebraDb {
) = self.db.zs_last_key_value(&orchard_subtrees)?; ) = self.db.zs_last_key_value(&orchard_subtrees)?;
let tip_height = self.finalized_tip_height()?; let tip_height = self.finalized_tip_height()?;
if subtree_data.end != tip_height { if subtree_data.end_height != tip_height {
return None; return None;
} }

4
zebra-state/src/service/non_finalized_state/chain.rs
View File

@ -694,7 +694,7 @@ impl Chain {
self.sapling_subtrees self.sapling_subtrees
.iter() .iter()
.find(|(_index, subtree)| subtree.end == height) .find(|(_index, subtree)| subtree.end_height == height)
.map(|(index, subtree)| subtree.with_index(*index)) .map(|(index, subtree)| subtree.with_index(*index))
} }
@ -898,7 +898,7 @@ impl Chain {
self.orchard_subtrees self.orchard_subtrees
.iter() .iter()
.find(|(_index, subtree)| subtree.end == height) .find(|(_index, subtree)| subtree.end_height == height)
.map(|(index, subtree)| subtree.with_index(*index)) .map(|(index, subtree)| subtree.with_index(*index))
} }

5
zebra-state/src/service/read/tests/vectors.rs
View File

@ -179,7 +179,10 @@ async fn test_read_subtrees() -> Result<()> {
.pop_first() .pop_first()
.expect("chain_subtrees should not be empty"); .expect("chain_subtrees should not be empty");
assert_eq!(first_chain_index, index, "subtree indexes should match"); assert_eq!(first_chain_index, index, "subtree indexes should match");
assert_eq!(end_height, subtree.end, "subtree end heights should match"); assert_eq!(
end_height, subtree.end_height,
"subtree end heights should match"
);
// Check that Zebra retrieves subtrees correctly when using a range with an Excluded start bound // Check that Zebra retrieves subtrees correctly when using a range with an Excluded start bound