From 63dadbca7394d6400ccde03dfb05eaa871545163 Mon Sep 17 00:00:00 2001 From: xinifinity <113067541+xinifinity@users.noreply.github.com> Date: Wed, 4 Oct 2023 10:27:03 -0700 Subject: [PATCH] chore: naming the elements of `ExtNode` (#305) Co-authored-by: Richard Pringle --- firewood/src/merkle.rs | 133 ++++++++++++++++++------------------ firewood/src/merkle/node.rs | 61 ++++++++++------- 2 files changed, 105 insertions(+), 89 deletions(-) diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index 77b7b43b0..31b518b03 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -140,7 +140,7 @@ impl + Send + Sync> Merkle { NodeType::Leaf(n) => writeln!(w, "{n:?}").unwrap(), NodeType::Extension(n) => { writeln!(w, "{n:?}")?; - self.dump_(n.1, w)? + self.dump_(n.chd(), w)? } } Ok(()) @@ -161,7 +161,7 @@ impl + Send + Sync> Merkle { .write(|p| { match &mut p.inner { NodeType::Branch(pp) => pp.chd[*idx as usize] = Some(new_chd), - NodeType::Extension(pp) => pp.1 = new_chd, + NodeType::Extension(pp) => *pp.chd_mut() = new_chd, _ => unreachable!(), } p.rehash(); @@ -191,7 +191,7 @@ impl + Send + Sync> Merkle { .write(|u| { (*match &mut u.inner { NodeType::Leaf(u) => &mut u.0, - NodeType::Extension(u) => &mut u.0, + NodeType::Extension(u) => u.path_mut(), _ => unreachable!(), }) = PartialPath(n_path[idx + 1..].to_vec()); u.rehash(); @@ -207,9 +207,9 @@ impl + Send + Sync> Merkle { chd[rem_path[idx] as usize] = Some(leaf_ptr); chd[n_path[idx] as usize] = Some(match &u_ref.inner { NodeType::Extension(u) => { - if u.0.len() == 0 { + if u.path().len() == 0 { deleted.push(u_ptr); - u.1 + u.chd() } else { u_ptr } @@ -224,8 +224,8 @@ impl + Send + Sync> Merkle { }); let branch_ptr = self.new_node(Node::new(t))?.as_ptr(); if idx > 0 { - self.new_node(Node::new(NodeType::Extension(ExtNode( - PartialPath(rem_path[..idx].to_vec()), + self.new_node(Node::new(NodeType::Extension(ExtNode::new( + rem_path[..idx].to_vec(), branch_ptr, None, ))))? @@ -245,21 +245,23 @@ impl + Send + Sync> Merkle { match &mut u.inner { NodeType::Leaf(u) => u.1 = Data(val), NodeType::Extension(u) => { - let write_result = self.get_node(u.1).and_then(|mut b_ref| { - let write_result = b_ref.write(|b| { - b.inner.as_branch_mut().unwrap().value = - Some(Data(val)); - b.rehash() + let write_result = + self.get_node(u.chd()).and_then(|mut b_ref| { + let write_result = b_ref.write(|b| { + b.inner.as_branch_mut().unwrap().value = + Some(Data(val)); + b.rehash() + }); + + if write_result.is_err() { + *u.chd_mut() = + self.new_node(b_ref.clone())?.as_ptr(); + deleted.push(b_ref.as_ptr()); + } + + Ok(()) }); - if write_result.is_err() { - u.1 = self.new_node(b_ref.clone())?.as_ptr(); - deleted.push(b_ref.as_ptr()); - } - - Ok(()) - }); - if let Err(e) = write_result { result = Err(e); } @@ -281,7 +283,7 @@ impl + Send + Sync> Merkle { .write(|u| { (*match &mut u.inner { NodeType::Leaf(u) => &mut u.0, - NodeType::Extension(u) => &mut u.0, + NodeType::Extension(u) => u.path_mut(), _ => unreachable!(), }) = PartialPath(n_path[rem_path.len() + 1..].to_vec()); u.rehash(); @@ -290,9 +292,9 @@ impl + Send + Sync> Merkle { ( match &u_ref.inner { NodeType::Extension(u) => { - if u.0.len() == 0 { + if u.path().len() == 0 { deleted.push(u_ptr); - u.1 + u.chd() } else { u_ptr } @@ -328,8 +330,8 @@ impl + Send + Sync> Merkle { })))? .as_ptr(); if !prefix.is_empty() { - self.new_node(Node::new(NodeType::Extension(ExtNode( - PartialPath(prefix.to_vec()), + self.new_node(Node::new(NodeType::Extension(ExtNode::new( + prefix.to_vec(), branch_ptr, None, ))))? @@ -430,8 +432,8 @@ impl + Send + Sync> Merkle { break; } NodeType::Extension(n) => { - let n_path = n.0.to_vec(); - let n_ptr = n.1; + let n_path = n.path().to_vec(); + let n_ptr = n.chd(); nskip = n_path.len() - 1; let rem_path = key_nibbles .into_iter() @@ -490,14 +492,14 @@ impl + Send + Sync> Merkle { } } NodeType::Extension(n) => { - let idx = n.0[0]; - let more = if n.0.len() > 1 { - n.0 = PartialPath(n.0[1..].to_vec()); + let idx = n.path()[0]; + let more = if n.path().len() > 1 { + *n.path_mut() = PartialPath(n.path()[1..].to_vec()); true } else { false }; - Some((idx, more, Some(n.1))) + Some((idx, more, Some(n.chd()))) } }; u.rehash() @@ -587,7 +589,7 @@ impl + Send + Sync> Merkle { // to: P -> [Leaf (v)] let leaf = self .new_node(Node::new(NodeType::Leaf(LeafNode( - PartialPath(n.0.clone().into_inner()), + PartialPath(n.path().clone().into_inner()), val, ))))? .as_ptr(); @@ -610,8 +612,8 @@ impl + Send + Sync> Merkle { // \____[Leaf]x // to: [p: Branch] -> [Ext] -> [Branch] let ext = self - .new_node(Node::new(NodeType::Extension(ExtNode( - PartialPath(vec![idx]), + .new_node(Node::new(NodeType::Extension(ExtNode::new( + vec![idx], c_ptr, None, ))))? @@ -629,8 +631,8 @@ impl + Send + Sync> Merkle { p_ref, |p| { let pp = p.inner.as_extension_mut().unwrap(); - pp.0 .0.push(idx); - pp.1 = c_ptr; + pp.path_mut().0.push(idx); + *pp.chd_mut() = c_ptr; p.rehash(); }, parents, @@ -651,7 +653,7 @@ impl + Send + Sync> Merkle { let write_result = c_ref.write(|c| { let partial_path = match &mut c.inner { NodeType::Leaf(n) => &mut n.0, - NodeType::Extension(n) => &mut n.0, + NodeType::Extension(n) => n.path_mut(), _ => unreachable!(), }; @@ -688,11 +690,11 @@ impl + Send + Sync> Merkle { self, c_ref, |c| { - let mut path = n.0.clone().into_inner(); + let mut path = n.path().clone().into_inner(); path.push(idx); let path0 = match &mut c.inner { NodeType::Leaf(n) => &mut n.0, - NodeType::Extension(n) => &mut n.0, + NodeType::Extension(n) => n.path_mut(), _ => unreachable!(), }; path.extend(&**path0); @@ -738,19 +740,18 @@ impl + Send + Sync> Merkle { NodeType::Branch(n) => { // from: [Branch] -> [Branch]x -> [Branch] // to: [Branch] -> [Ext] -> [Branch] - n.chd[b_idx as usize] = - Some( - self.new_node(Node::new(NodeType::Extension( - ExtNode(PartialPath(vec![idx]), c_ptr, None), - )))? - .as_ptr(), - ); + n.chd[b_idx as usize] = Some( + self.new_node(Node::new(NodeType::Extension( + ExtNode::new(vec![idx], c_ptr, None), + )))? + .as_ptr(), + ); } NodeType::Extension(n) => { // from: [Ext] -> [Branch]x -> [Branch] // to: [Ext] -> [Branch] - n.0 .0.push(idx); - n.1 = c_ptr + n.path_mut().0.push(idx); + *n.chd_mut() = c_ptr } _ => unreachable!(), } @@ -774,7 +775,7 @@ impl + Send + Sync> Merkle { let write_result = c_ref.write(|c| { match &mut c.inner { NodeType::Leaf(n) => &mut n.0, - NodeType::Extension(n) => &mut n.0, + NodeType::Extension(n) => n.path_mut(), _ => unreachable!(), } .0 @@ -799,11 +800,11 @@ impl + Send + Sync> Merkle { // from: P -> [Ext] -> [Branch]x -> [Leaf/Ext] // to: P -> [Leaf/Ext] let write_result = c_ref.write(|c| { - let mut path = n.0.clone().into_inner(); + let mut path = n.path().clone().into_inner(); path.push(idx); let path0 = match &mut c.inner { NodeType::Leaf(n) => &mut n.0, - NodeType::Extension(n) => &mut n.0, + NodeType::Extension(n) => n.path_mut(), _ => unreachable!(), }; path.extend(&**path0); @@ -866,13 +867,13 @@ impl + Send + Sync> Merkle { break; } NodeType::Extension(n) => { - let n_path = &*n.0; + let n_path = &*n.path().0; let rem_path = &chunks[i..]; if rem_path < n_path || &rem_path[..n_path.len()] != n_path { return Ok(None); } nskip = n_path.len() - 1; - n.1 + n.chd() } }; @@ -934,7 +935,7 @@ impl + Send + Sync> Merkle { } } NodeType::Leaf(_) => (), - NodeType::Extension(n) => self.remove_tree_(n.1, deleted)?, + NodeType::Extension(n) => self.remove_tree_(n.chd(), deleted)?, } deleted.push(u); Ok(()) @@ -987,13 +988,13 @@ impl + Send + Sync> Merkle { return Ok(Some(RefMut::new(u_ptr, parents, self))); } NodeType::Extension(n) => { - let n_path = &*n.0; + let n_path = &*n.path().0; let rem_path = &chunks[i..]; if rem_path.len() < n_path.len() || &rem_path[..n_path.len()] != n_path { return Ok(None); } nskip = n_path.len() - 1; - n.1 + n.chd() } }; parents.push((u_ptr, *nib)); @@ -1067,7 +1068,7 @@ impl + Send + Sync> Merkle { NodeType::Extension(n) => { // the key passed in must match the entire remainder of this // extension node, otherwise we break out - let n_path = &*n.0; + let n_path = n.path(); let remaining_path = key_nibbles.into_iter().skip(i); if remaining_path.size_hint().0 < n_path.len() { // all bytes aren't there @@ -1078,7 +1079,7 @@ impl + Send + Sync> Merkle { break; } nskip = n_path.len() - 1; - n.1 + n.chd() } }; u_ref = self.get_node(next_ptr)?; @@ -1140,7 +1141,7 @@ impl + Send + Sync> Merkle { return Ok(Some(Ref(u_ref))); } NodeType::Extension(n) => { - let n_path = &*n.0; + let n_path = n.path(); let rem_path = key_nibbles.into_iter().skip(i); if rem_path.size_hint().0 < n_path.len() { return Ok(None); @@ -1149,7 +1150,7 @@ impl + Send + Sync> Merkle { return Ok(None); } nskip = n_path.len() - 1; - n.1 + n.chd() } }; u_ref = self.get_node(next_ptr)?; @@ -1345,8 +1346,8 @@ mod test { Node::new_from_hash( None, None, - NodeType::Extension(ExtNode( - PartialPath(vec![0x1, 0x2, 0x3]), + NodeType::Extension(ExtNode::new( + vec![0x1, 0x2, 0x3], DiskAddress::from(0x42), None, )), @@ -1354,8 +1355,8 @@ mod test { Node::new_from_hash( None, None, - NodeType::Extension(ExtNode( - PartialPath(vec![0x1, 0x2, 0x3]), + NodeType::Extension(ExtNode::new( + vec![0x1, 0x2, 0x3], DiskAddress::null(), Some(vec![0x1, 0x2, 0x3]), )), @@ -1452,8 +1453,8 @@ mod test { let new_chd_encoded = new_chd.get_encoded(merkle.store.as_ref()); assert_eq!(chd_encoded, new_chd_encoded); - let node = Node::new(NodeType::Extension(ExtNode( - PartialPath(vec![0x1, 0x2, 0x3]), + let node = Node::new(NodeType::Extension(ExtNode::new( + vec![0x1, 0x2, 0x3], DiskAddress::null(), Some(chd_encoded.to_vec()), ))); diff --git a/firewood/src/merkle/node.rs b/firewood/src/merkle/node.rs index 0a024e85e..472fd48b0 100644 --- a/firewood/src/merkle/node.rs +++ b/firewood/src/merkle/node.rs @@ -243,15 +243,19 @@ impl LeafNode { } #[derive(PartialEq, Eq, Clone)] -pub struct ExtNode( - pub(super) PartialPath, - pub(super) DiskAddress, - pub(super) Option>, -); +pub struct ExtNode { + path: PartialPath, + chd: DiskAddress, + chd_encoded: Option>, +} impl Debug for ExtNode { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { - write!(f, "[Extension {:?} {:?} {:?}]", self.0, self.1, self.2) + write!( + f, + "[Extension {:?} {:?} {:?}]", + self.path, self.chd, self.chd_encoded + ) } } @@ -260,12 +264,12 @@ impl ExtNode { let mut list = <[Encoded>; 2]>::default(); list[0] = Encoded::Data( bincode::DefaultOptions::new() - .serialize(&from_nibbles(&self.0.encode(false)).collect::>()) + .serialize(&from_nibbles(&self.path.encode(false)).collect::>()) .unwrap(), ); - if !self.1.is_null() { - let mut r = store.get_item(self.1).unwrap(); + if !self.chd.is_null() { + let mut r = store.get_item(self.chd).unwrap(); if r.is_encoded_longer_than_hash_len(store) { list[1] = Encoded::Data( @@ -284,7 +288,7 @@ impl ExtNode { } else { // Check if there is already a caclucated encoded value for the child, which // can happen when manually constructing a trie from proof. - if let Some(v) = &self.2 { + if let Some(v) = &self.chd_encoded { if v.len() == TRIE_HASH_LEN { list[1] = Encoded::Data(bincode::DefaultOptions::new().serialize(v).unwrap()); } else { @@ -299,27 +303,35 @@ impl ExtNode { } pub fn new(path: Vec, chd: DiskAddress, chd_encoded: Option>) -> Self { - ExtNode(PartialPath(path), chd, chd_encoded) + ExtNode { + path: PartialPath(path), + chd, + chd_encoded, + } } pub fn path(&self) -> &PartialPath { - &self.0 + &self.path + } + + pub fn path_mut(&mut self) -> &mut PartialPath { + &mut self.path } pub fn chd(&self) -> DiskAddress { - self.1 + self.chd } pub fn chd_encoded(&self) -> Option<&[u8]> { - self.2.as_deref() + self.chd_encoded.as_deref() } pub fn chd_mut(&mut self) -> &mut DiskAddress { - &mut self.1 + &mut self.chd } pub fn chd_encoded_mut(&mut self) -> &mut Option> { - &mut self.2 + &mut self.chd_encoded } } @@ -671,7 +683,11 @@ impl Storable for Node { Ok(Self::new_from_hash( root_hash, is_encoded_longer_than_hash_len, - NodeType::Extension(ExtNode(path, DiskAddress::from(ptr as usize), encoded)), + NodeType::Extension(ExtNode { + path, + chd: DiskAddress::from(ptr as usize), + chd_encoded: encoded, + }), )) } Self::LEAF_NODE => { @@ -739,8 +755,8 @@ impl Storable for Node { } NodeType::Extension(n) => { 1 + 8 - + n.0.dehydrated_len() - + match &n.2 { + + n.path.dehydrated_len() + + match n.chd_encoded() { Some(v) => 1 + v.len() as u64, None => 1, } @@ -802,12 +818,11 @@ impl Storable for Node { } NodeType::Extension(n) => { cur.write_all(&[Self::EXT_NODE])?; - let path: Vec = from_nibbles(&n.0.encode(false)).collect(); + let path: Vec = from_nibbles(&n.path.encode(false)).collect(); cur.write_all(&[path.len() as u8])?; - cur.write_all(&n.1.to_le_bytes())?; + cur.write_all(&n.chd.to_le_bytes())?; cur.write_all(&path)?; - if n.2.is_some() { - let encoded = n.2.as_ref().unwrap(); + if let Some(encoded) = n.chd_encoded() { cur.write_all(&[encoded.len() as u8])?; cur.write_all(encoded)?; }