From c49c09bca34515aa1bb4cf1ea14c2055681f91e4 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 25 Sep 2023 16:44:41 -0400 Subject: [PATCH] Cleanup commented code --- firewood/src/merkle/node.rs | 84 ++++------------------- firewood/src/proof.rs | 133 +++++++----------------------------- 2 files changed, 39 insertions(+), 178 deletions(-) diff --git a/firewood/src/merkle/node.rs b/firewood/src/merkle/node.rs index f8c0d9f7d..380c1116c 100644 --- a/firewood/src/merkle/node.rs +++ b/firewood/src/merkle/node.rs @@ -1,7 +1,7 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use bincode::Options; +use bincode::{Error, Options}; use enum_as_inner::EnumAsInner; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use sha3::{Digest, Keccak256}; @@ -17,7 +17,6 @@ use std::{ use crate::merkle::to_nibble_array; use crate::nibbles::Nibbles; -use thiserror::Error; use super::{from_nibbles, PartialPath, TrieHash, TRIE_HASH_LEN}; @@ -58,12 +57,6 @@ impl> Encoded { } } -#[derive(Debug, Error)] -pub enum Error { - #[error("decoding error")] - Decode(#[from] bincode::Error), -} - #[derive(PartialEq, Eq, Clone)] pub struct BranchNode { pub(super) chd: [Option; NBRANCH], @@ -133,15 +126,14 @@ impl BranchNode { } fn calc_eth_rlp>(&self, store: &S) -> Vec { - // let mut stream = rlp::RlpStream::new_list(NBRANCH + 1); let mut list = <[Encoded>; NBRANCH + 1]>::default(); for (i, c) in self.chd.iter().enumerate() { match c { Some(c) => { let mut c_ref = store.get_item(*c).unwrap(); + if c_ref.get_eth_rlp_long::(store) { - // stream.append(&&(*c_ref.get_root_hash::(store))[..]); list[i] = Encoded::Data( bincode::DefaultOptions::new() .serialize(&&(*c_ref.get_root_hash::(store))[..]) @@ -155,50 +147,26 @@ impl BranchNode { } } else { let c_rlp = &c_ref.get_eth_rlp::(store); - // stream.append_raw(c_rlp, 1); list[i] = Encoded::Raw(c_rlp.to_vec()); } } None => { // Check if there is already a calculated rlp for the child, which // can happen when manually constructing a trie from proof. - if let Some(v) = self.chd_eth_rlp[i].clone() { + if let Some(v) = &self.chd_eth_rlp[i] { if v.len() == TRIE_HASH_LEN { - // stream.append(&v); - list[i] = Encoded::Data( - bincode::DefaultOptions::new().serialize(&v).unwrap(), - ); + list[i] = + Encoded::Data(bincode::DefaultOptions::new().serialize(v).unwrap()); } else { - // stream.append_raw(&v, 1); - list[i] = Encoded::Raw(v); + list[i] = Encoded::Raw(v.clone()); } } - // if self.chd_eth_rlp[i].is_none() { - // stream.append_empty_data(); - // } else { - // let v = self.chd_eth_rlp[i].clone().unwrap(); - // if v.len() == TRIE_HASH_LEN { - // stream.append(&v); - // } else { - // stream.append_raw(&v, 1); - // } - // } } }; } - // match &self.value { - // Some(val) => stream.append(&val.to_vec()), - // None => stream.append_empty_data(), - // }; - // stream.out().into() - - if let Some(val) = self.value.clone() { - list[NBRANCH] = Encoded::Data( - bincode::DefaultOptions::new() - .serialize(&val.to_vec()) - .unwrap(), - ); + if let Some(Data(val)) = &self.value { + list[NBRANCH] = Encoded::Data(bincode::DefaultOptions::new().serialize(val).unwrap()); } bincode::DefaultOptions::new() @@ -249,13 +217,6 @@ impl Debug for LeafNode { } impl LeafNode { - // fn calc_eth_rlp(&self) -> Vec { - // rlp::encode_list::, _>(&[ - // from_nibbles(&self.0.encode(true)).collect(), - // self.1.to_vec(), - // ]) - // .into() - // } fn calc_eth_rlp(&self) -> Vec { bincode::DefaultOptions::new() .serialize( @@ -296,7 +257,6 @@ impl Debug for ExtNode { impl ExtNode { fn calc_eth_rlp>(&self, store: &S) -> Vec { - // let mut stream = rlp::RlpStream::new_list(2); let mut list = <[Encoded>; 2]>::default(); list[0] = Encoded::Data( bincode::DefaultOptions::new() @@ -306,10 +266,8 @@ impl ExtNode { if !self.1.is_null() { let mut r = store.get_item(self.1).unwrap(); - // stream.append(&from_nibbles(&self.0.encode(false)).collect::>()); if r.get_eth_rlp_long(store) { - // stream.append(&&(*r.get_root_hash(store))[..]); list[1] = Encoded::Data( bincode::DefaultOptions::new() .serialize(&&(*r.get_root_hash(store))[..]) @@ -321,34 +279,20 @@ impl ExtNode { r.lazy_dirty.store(false, Ordering::Relaxed); } } else { - // stream.append_raw(r.get_eth_rlp(store), 1); list[1] = Encoded::Raw(r.get_eth_rlp(store).to_vec()); } } else { // Check if there is already a caclucated rlp for the child, which // can happen when manually constructing a trie from proof. - // if self.2.is_none() { - // stream.append_empty_data(); - // } else { - // let v = self.2.clone().unwrap(); - // if v.len() == TRIE_HASH_LEN { - // stream.append(&v); - // } else { - // stream.append_raw(&v, 1); - // } - // } - - if let Some(v) = self.2.as_ref() { + if let Some(v) = &self.2 { if v.len() == TRIE_HASH_LEN { - // stream.append(&v); list[1] = Encoded::Data(bincode::DefaultOptions::new().serialize(v).unwrap()); } else { - // stream.append_raw(&v, 1); list[1] = Encoded::Raw(v.clone()); } } } - // stream.out().into() + bincode::DefaultOptions::new() .serialize(list.as_slice()) .unwrap() @@ -434,9 +378,7 @@ impl NodeType { } pub fn decode(buf: &[u8]) -> Result { - let items: Vec>> = bincode::DefaultOptions::new() - .deserialize(dbg!(buf)) - .map_err(Error::Decode)?; + let items: Vec>> = bincode::DefaultOptions::new().deserialize(buf)?; match items.len() { EXT_NODE_SIZE => { @@ -461,8 +403,8 @@ impl NodeType { } } BRANCH_NODE_SIZE => Ok(NodeType::Branch(BranchNode::decode(buf)?)), - _ => Err(Error::Decode(Box::new(bincode::ErrorKind::Custom( - String::from(""), + size => Err(Box::new(bincode::ErrorKind::Custom(format!( + "invalid size: {size}" )))), } } diff --git a/firewood/src/proof.rs b/firewood/src/proof.rs index 2b01779f0..69ae1931d 100644 --- a/firewood/src/proof.rs +++ b/firewood/src/proof.rs @@ -149,36 +149,31 @@ impl + Send> Proof { mut key_nibbles: NibblesIterator<'a, 0>, rlp_encoded_node: &[u8], ) -> Result<(Option, NibblesIterator<'a, 0>), ProofError> { - // let rlp = rlp::Rlp::new(rlp_encoded_node); - let items: Vec>> = bincode::DefaultOptions::new() + #[derive(serde::Serialize, serde::Deserialize)] + enum EncodedNode { + ExtensionOrLeaf([Encoded>; 2]), + Branch([Encoded>; 17]), + } + use EncodedNode::*; + + let node: EncodedNode = bincode::DefaultOptions::new() .deserialize(rlp_encoded_node) .map_err(ProofError::DecodeError)?; - // match rlp.item_count() { - match items.len() { - // Ok(EXT_NODE_SIZE) => { - // [Encoded>; 2] - // 0 is always Encoded::Data - // 1 could be either - EXT_NODE_SIZE => { - // let decoded_key = rlp.at(0).unwrap().as_val::>().unwrap(); - let mut items = items.into_iter(); - let decoded_key: Vec = items.next().unwrap().decode()?; + let Some(next_nibble) = key_nibbles.next() else { + return Err(ProofError::NoSuchNode); + }; + match node { + ExtensionOrLeaf([key, data]) => { + let decoded_key = key.decode()?; let decoded_key_nibbles = Nibbles::<0>::new(&decoded_key); let (cur_key_path, term) = PartialPath::from_nibbles(decoded_key_nibbles.into_iter()); let cur_key = cur_key_path.into_inner(); - // let rlp = rlp.at(1).unwrap(); - // let data = if rlp.is_data() { - // rlp.as_val::>().unwrap() - // } else { - // rlp.as_raw().to_vec() - // }; - - let data: Vec = items.next().unwrap().decode()?; + let data: Vec = data.decode()?; // Check if the key of current node match with the given key // and consume the current-key portion of the nibbles-iterator @@ -201,33 +196,19 @@ impl + Send> Proof { Ok((sub_proof.into(), key_nibbles)) } - // Ok(BRANCH_NODE_SIZE) if key_nibbles.size_hint().0 == 0 => Err(ProofError::NoSuchNode), - BRANCH_NODE_SIZE if key_nibbles.size_hint().0 == 0 => Err(ProofError::NoSuchNode), - - // Ok(BRANCH_NODE_SIZE) => { - BRANCH_NODE_SIZE => { + Branch(children) => { let index = key_nibbles.next().unwrap() as usize; - // let rlp = rlp.at(index).unwrap(); - - // let data = if rlp.is_data() { - // rlp.as_val::>().unwrap() - // } else { - // rlp.as_raw().to_vec() - // }; // consume items returning the item at index - - let data: Vec = items.into_element_at(index).decode()?; + let data: Vec = children[index].decode()?; self.generate_subproof(data) .map(|subproof| (Some(subproof), key_nibbles)) } - // Ok(_) => Err(ProofError::DecodeError(rlp::DecoderError::RlpInvalidLength)), - // Err(e) => Err(ProofError::DecodeError(e)), - _ => Err(ProofError::DecodeError(Box::new( - bincode::ErrorKind::Custom(String::from("")), - ))), + // size => Err(ProofError::DecodeError(Box::new( + // bincode::ErrorKind::Custom(format!("invalid size: {size}")), + // ))), } } @@ -240,6 +221,7 @@ impl + Send> Proof { hash: Some(sub_hash), }) } + 32 => { let sub_hash: &[u8] = &data; let sub_hash = sub_hash.try_into().unwrap(); @@ -249,9 +231,9 @@ impl + Send> Proof { hash: Some(sub_hash), }) } - // _ => Err(ProofError::DecodeError(rlp::DecoderError::RlpInvalidLength)), - _ => Err(ProofError::DecodeError(Box::new( - bincode::ErrorKind::Custom(String::from("")), + + len => Err(ProofError::DecodeError(Box::new( + bincode::ErrorKind::Custom(format!("invalid proof length: {len}")), ))), } } @@ -568,20 +550,11 @@ impl + Send> Proof { buf: &[u8], end_node: bool, ) -> Result<(DiskAddress, Option, usize), ProofError> { - // let rlp = rlp::Rlp::new(buf); - // let size = rlp.item_count()?; - let mut items: Vec>> = bincode::DefaultOptions::new().deserialize(buf)?; let size = items.len(); match size { EXT_NODE_SIZE => { - // let cur_key_path: Vec<_> = rlp - // .at(0)? - // .as_val::>()? - // .into_iter() - // .flat_map(to_nibble_array) - // .collect(); let mut items = items.into_iter(); let cur_key_path: Vec = items @@ -594,15 +567,6 @@ impl + Send> Proof { let (cur_key_path, term) = PartialPath::decode(&cur_key_path); let cur_key = cur_key_path.into_inner(); - - // let rlp = rlp.at(1)?; - - // let data = if rlp.is_data() { - // rlp.as_val::>()? - // } else { - // rlp.as_raw().to_vec() - // }; - let data: Vec = items.next().unwrap().decode()?; // Check if the key of current node match with the given key. @@ -629,22 +593,6 @@ impl + Send> Proof { } BRANCH_NODE_SIZE => { - // let data_rlp = rlp.at(NBRANCH)?; - - // // Extract the value of the branch node. - // // Skip if rlp is empty data - // let value = if !data_rlp.is_empty() { - // let data = if data_rlp.is_data() { - // data_rlp.as_val::>().unwrap() - // } else { - // data_rlp.as_raw().to_vec() - // }; - - // Some(data) - // } else { - // None - // }; - // we've already validated the size, that's why we can safely unwrap let data = items.pop().unwrap().decode()?; // Extract the value of the branch node and set to None if it's an empty Vec @@ -653,19 +601,6 @@ impl + Send> Proof { // Record rlp values of all children. let mut chd_eth_rlp: [Option>; NBRANCH] = Default::default(); - // for (i, chd) in rlp.into_iter().take(NBRANCH).enumerate() { - // if !chd.is_empty() { - // // Skip if chd is empty data - // let data = if chd.is_data() { - // chd.as_val()? - // } else { - // chd.as_raw().to_vec() - // }; - - // chd_eth_rlp[i] = Some(data); - // } - // } - // we popped the last element, so their should only be NBRANCH items left for (i, chd) in items.into_iter().enumerate() { let data = chd.decode()?; @@ -698,10 +633,8 @@ impl + Send> Proof { Ok((branch_ptr, Some(subproof), 1)) } - // RLP length can only be the two cases above. - // _ => Err(ProofError::DecodeError(rlp::DecoderError::RlpInvalidLength)), - _ => Err(ProofError::DecodeError(Box::new( - bincode::ErrorKind::Custom(String::from("")), + size => Err(ProofError::DecodeError(Box::new( + bincode::ErrorKind::Custom(format!("invalid size: {size}")), ))), } } @@ -1132,17 +1065,3 @@ fn unset_node_ref, S: ShaleStore + Send + Sync>( } } } - -trait IntoElementAt { - type Element; - - fn into_element_at(self, index: usize) -> Self::Element; -} - -impl IntoElementAt for Vec { - type Element = T; - - fn into_element_at(self, index: usize) -> Self::Element { - self.into_iter().nth(index).unwrap() - } -}