From 03e602deb9c8b31ca3a518df190d7a64b7514904 Mon Sep 17 00:00:00 2001 From: 0xaatif Date: Thu, 25 Jul 2024 11:50:34 +0100 Subject: [PATCH 1/4] mark: 0xaatif/fix-bad-storage-tries From afba8ea125c25938adffd1de81cf6d2c409a3029 Mon Sep 17 00:00:00 2001 From: 0xaatif Date: Thu, 25 Jul 2024 12:27:16 +0100 Subject: [PATCH 2/4] run: git checkout 4da33514bb6ec6be834311df954e6979905be0ee -- trace_decoder/src/typed_mpt.rs --- trace_decoder/src/typed_mpt.rs | 84 +++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 16 deletions(-) diff --git a/trace_decoder/src/typed_mpt.rs b/trace_decoder/src/typed_mpt.rs index 2f68a0234..b1d487cae 100644 --- a/trace_decoder/src/typed_mpt.rs +++ b/trace_decoder/src/typed_mpt.rs @@ -95,6 +95,12 @@ impl TypedMpt { Some((path, self.get(path)?)) }) } + /// This allows users to break the [`TypedMpt`] invariant. + /// If data that isn't an [`rlp::encode`]-ed `T` is inserted, + /// subsequent API calls may panic. + pub fn as_mut_hashed_partial_trie_unchecked(&mut self) -> &mut HashedPartialTrie { + &mut self.inner + } } impl Default for TypedMpt { @@ -124,7 +130,7 @@ pub struct Error { /// used as a key for [`TypedMpt`]. /// /// Semantically equivalent to [`mpt_trie::nibbles::Nibbles`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] pub struct TriePath(CopyVec); impl TriePath { @@ -142,7 +148,7 @@ impl TriePath { fn from_address(address: Address) -> Self { Self::from_hash(keccak_hash::keccak(address)) } - fn from_hash(H256(bytes): H256) -> Self { + pub fn from_hash(H256(bytes): H256) -> Self { Self::new(AsNibbles(bytes)).expect("32 bytes is 64 nibbles, which fits") } fn from_txn_ix(txn_ix: usize) -> Self { @@ -178,18 +184,25 @@ impl TriePath { /// See #[derive(Debug, Clone, Default)] pub struct TransactionTrie { - typed: TypedMpt>, + untyped: HashedPartialTrie, } impl TransactionTrie { pub fn insert(&mut self, txn_ix: usize, val: Vec) -> Result>, Error> { - self.typed.insert(TriePath::from_txn_ix(txn_ix), val) + let prev = self + .untyped + .get(TriePath::from_txn_ix(txn_ix).into_nibbles()) + .map(Vec::from); + self.untyped + .insert(TriePath::from_txn_ix(txn_ix).into_nibbles(), val) + .map_err(|source| Error { source })?; + Ok(prev) } pub fn root(&self) -> H256 { - self.typed.root() + self.untyped.hash() } pub fn as_hashed_partial_trie(&self) -> &mpt_trie::partial_trie::HashedPartialTrie { - self.typed.as_hashed_partial_trie() + &self.untyped } } @@ -198,15 +211,25 @@ impl TransactionTrie { /// See #[derive(Debug, Clone, Default)] pub struct ReceiptTrie { - typed: TypedMpt>, + untyped: HashedPartialTrie, } impl ReceiptTrie { pub fn insert(&mut self, txn_ix: usize, val: Vec) -> Result>, Error> { - self.typed.insert(TriePath::from_txn_ix(txn_ix), val) + let prev = self + .untyped + .get(TriePath::from_txn_ix(txn_ix).into_nibbles()) + .map(Vec::from); + self.untyped + .insert(TriePath::from_txn_ix(txn_ix).into_nibbles(), val) + .map_err(|source| Error { source })?; + Ok(prev) + } + pub fn root(&self) -> H256 { + self.untyped.hash() } pub fn as_hashed_partial_trie(&self) -> &mpt_trie::partial_trie::HashedPartialTrie { - self.typed.as_hashed_partial_trie() + &self.untyped } } @@ -251,6 +274,13 @@ impl StateTrie { pub fn as_hashed_partial_trie(&self) -> &mpt_trie::partial_trie::HashedPartialTrie { self.typed.as_hashed_partial_trie() } + + /// This allows users to break the [`TypedMpt`] invariant. + /// If data that isn't an [`rlp::encode`]-ed `T` is inserted, + /// subsequent API calls may panic. + pub fn as_mut_hashed_partial_trie_unchecked(&mut self) -> &mut HashedPartialTrie { + self.typed.as_mut_hashed_partial_trie_unchecked() + } } impl<'a> IntoIterator for &'a StateTrie { @@ -268,23 +298,45 @@ impl<'a> IntoIterator for &'a StateTrie { /// See #[derive(Debug, Clone, Default)] pub struct StorageTrie { - /// This does NOT use [`TypedMpt`] - T could be anything! - typed: TypedMpt>, + untyped: HashedPartialTrie, } impl StorageTrie { pub fn insert(&mut self, path: TriePath, value: Vec) -> Result>, Error> { - self.typed.insert(path, value) + let prev = self.untyped.get(path.into_nibbles()).map(Vec::from); + self.untyped + .insert(path.into_nibbles(), value) + .map_err(|source| Error { source })?; + Ok(prev) } pub fn insert_hash(&mut self, path: TriePath, hash: H256) -> Result<(), Error> { - self.typed.insert_hash(path, hash) + self.untyped + .insert(path.into_nibbles(), hash) + .map_err(|source| Error { source }) } pub fn root(&self) -> H256 { - self.typed.root() + self.untyped.hash() } pub fn remove(&mut self, path: TriePath) -> Result>, Error> { - self.typed.remove(path) + self.untyped + .delete(path.into_nibbles()) + .map_err(|source| Error { source }) } pub fn as_hashed_partial_trie(&self) -> &mpt_trie::partial_trie::HashedPartialTrie { - self.typed.as_hashed_partial_trie() + &self.untyped } + + pub fn as_mut_hashed_partial_trie_unchecked(&mut self) -> &mut HashedPartialTrie { + &mut self.untyped + } +} + +#[test] +fn test() { + let hash = H256(std::array::from_fn(|ix| ix as _)); + let mut ours = StorageTrie::default(); + ours.insert_hash(TriePath::default(), hash).unwrap(); + assert_eq!( + ours.as_hashed_partial_trie(), + &HashedPartialTrie::new(Node::Hash(hash)) + ); } From 45577cd8d020874e74cc28ecc05b8009ee2c11bb Mon Sep 17 00:00:00 2001 From: 0xaatif Date: Thu, 25 Jul 2024 12:42:32 +0100 Subject: [PATCH 3/4] run: git checkout 4da33514bb6ec6be834311df954e6979905be0ee -- trace_decoder/src/type1.rs --- trace_decoder/src/type1.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace_decoder/src/type1.rs b/trace_decoder/src/type1.rs index 9f0cc171b..e7dbcf696 100644 --- a/trace_decoder/src/type1.rs +++ b/trace_decoder/src/type1.rs @@ -132,7 +132,7 @@ fn node2storagetrie(node: Node) -> anyhow::Result { match value { Either::Left(Value { raw_value }) => mpt.insert( TriePath::new(path.iter().copied().chain(key))?, - raw_value.into_vec(), + rlp::encode(&raw_value.as_slice()).to_vec(), )?, Either::Right(_) => bail!("unexpected account node in storage trie"), }; From b8dc9b80a24891d3bdb9347daf6880b28b94b834 Mon Sep 17 00:00:00 2001 From: 0xaatif Date: Thu, 25 Jul 2024 12:43:09 +0100 Subject: [PATCH 4/4] chore: trim fat --- trace_decoder/src/typed_mpt.rs | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/trace_decoder/src/typed_mpt.rs b/trace_decoder/src/typed_mpt.rs index b1d487cae..947727737 100644 --- a/trace_decoder/src/typed_mpt.rs +++ b/trace_decoder/src/typed_mpt.rs @@ -63,22 +63,6 @@ impl TypedMpt { and only encoded `T`s are ever inserted", )) } - /// # Panics - /// - If [`rlp::decode`]-ing for `T` doesn't round-trip. - fn remove(&mut self, path: TriePath) -> Result, Error> - where - T: rlp::Decodable, - { - match self.inner.delete(path.into_nibbles()) { - Ok(None) => Ok(None), - Ok(Some(bytes)) => Ok(Some(rlp::decode(&bytes).expect( - "T encoding/decoding should round-trip,\ - and only encoded `T`s are ever inserted", - ))), - // TODO(0xaatif): why is this fallible if `get` isn't? - Err(source) => Err(Error { source }), - } - } fn as_hashed_partial_trie(&self) -> &HashedPartialTrie { &self.inner } @@ -329,14 +313,3 @@ impl StorageTrie { &mut self.untyped } } - -#[test] -fn test() { - let hash = H256(std::array::from_fn(|ix| ix as _)); - let mut ours = StorageTrie::default(); - ours.insert_hash(TriePath::default(), hash).unwrap(); - assert_eq!( - ours.as_hashed_partial_trie(), - &HashedPartialTrie::new(Node::Hash(hash)) - ); -}