From 688847c64c1ef15df20828aa44ef871d3345fc98 Mon Sep 17 00:00:00 2001 From: Kolby Moroz Liebl <31669092+KolbyML@users.noreply.github.com> Date: Tue, 6 Aug 2024 11:07:49 -0600 Subject: [PATCH] refactor: update state types to use address_hash instead of address to match spec + remove recursive gossip (#1353) --- ethportal-api/src/types/content_key/state.rs | 23 +-- ethportal-peertest/src/scenarios/state.rs | 47 +---- ethportal-peertest/src/utils.rs | 35 ---- portal-bridge/src/bridge/state.rs | 20 +-- portal-spec-tests | 2 +- tests/self_peertest.rs | 9 - trin-execution/src/content.rs | 10 +- trin-execution/src/storage/evm_db.rs | 38 ++-- trin-state/src/validation/mod.rs | 1 - .../src/validation/recursive_trie_proof.rs | 164 ------------------ trin-state/src/validation/trie.rs | 47 ++--- trin-state/src/validation/validator.rs | 162 ++--------------- 12 files changed, 75 insertions(+), 483 deletions(-) delete mode 100644 trin-state/src/validation/recursive_trie_proof.rs diff --git a/ethportal-api/src/types/content_key/state.rs b/ethportal-api/src/types/content_key/state.rs index ba73838af..924a83a70 100644 --- a/ethportal-api/src/types/content_key/state.rs +++ b/ethportal-api/src/types/content_key/state.rs @@ -1,4 +1,4 @@ -use alloy_primitives::{Address, B256}; +use alloy_primitives::B256; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use sha2::{Digest as Sha2Digest, Sha256}; use ssz::{Decode, DecodeError, Encode}; @@ -46,8 +46,8 @@ pub struct AccountTrieNodeKey { /// A key for a trie node from some account's contract storage. #[derive(Clone, Debug, Decode, Encode, Eq, PartialEq)] pub struct ContractStorageTrieNodeKey { - /// Address of the account. - pub address: Address, + /// Address hash of the account. + pub address_hash: B256, /// Trie path of the node. pub path: Nibbles, /// Hash of the node. @@ -57,8 +57,8 @@ pub struct ContractStorageTrieNodeKey { /// A key for an account's contract bytecode. #[derive(Clone, Debug, Decode, Encode, Eq, PartialEq)] pub struct ContractBytecodeKey { - /// Address of the account. - pub address: Address, + /// Address hash of the account. + pub address_hash: B256, /// Hash of the bytecode. pub code_hash: B256, } @@ -163,16 +163,16 @@ impl fmt::Display for StateContentKey { ), Self::ContractStorageTrieNode(key) => { format!( - "ContractStorageTrieNode {{ address: {}, path: {}, node_hash: {} }}", - hex_encode_compact(key.address), + "ContractStorageTrieNode {{ address_hash: {}, path: {}, node_hash: {} }}", + hex_encode_compact(key.address_hash), &key.path, hex_encode_compact(key.node_hash) ) } Self::ContractBytecode(key) => { format!( - "ContractBytecode {{ address: {}, code_hash: {} }}", - hex_encode_compact(key.address), + "ContractBytecode {{ address_hash: {}, code_hash: {} }}", + hex_encode_compact(key.address_hash), hex_encode_compact(key.code_hash) ) } @@ -187,6 +187,7 @@ impl fmt::Display for StateContentKey { mod test { use std::{path::PathBuf, str::FromStr}; + use alloy_primitives::{keccak256, Address}; use anyhow::Result; use rstest::rstest; use serde_yaml::Value; @@ -217,7 +218,7 @@ mod test { let expected_content_key = StateContentKey::ContractStorageTrieNode(ContractStorageTrieNodeKey { - address: yaml_as_address(&yaml["address"]), + address_hash: keccak256(yaml_as_address(&yaml["address"])), path: yaml_as_nibbles(&yaml["path"]), node_hash: yaml_as_b256(&yaml["node_hash"]), }); @@ -231,7 +232,7 @@ mod test { let yaml = yaml.as_mapping().unwrap(); let expected_content_key = StateContentKey::ContractBytecode(ContractBytecodeKey { - address: yaml_as_address(&yaml["address"]), + address_hash: keccak256(yaml_as_address(&yaml["address"])), code_hash: yaml_as_b256(&yaml["code_hash"]), }); diff --git a/ethportal-peertest/src/scenarios/state.rs b/ethportal-peertest/src/scenarios/state.rs index fe5c48e23..9e080fb7c 100644 --- a/ethportal-peertest/src/scenarios/state.rs +++ b/ethportal-peertest/src/scenarios/state.rs @@ -1,15 +1,11 @@ use crate::{ utils::{ fixtures_state_account_trie_node, fixtures_state_contract_bytecode, - fixtures_state_contract_storage_trie_node, fixtures_state_recursive_gossip, - wait_for_state_content, StateFixture, + fixtures_state_contract_storage_trie_node, wait_for_state_content, StateFixture, }, Peertest, PeertestNode, }; -use ethportal_api::{ - jsonrpsee::async_client::Client, types::content_value::state::TrieNode, StateContentValue, - StateNetworkApiClient, -}; +use ethportal_api::{jsonrpsee::async_client::Client, StateNetworkApiClient}; use tracing::info; pub async fn test_state_offer_account_trie_node(peertest: &Peertest, target: &Client) { @@ -56,42 +52,3 @@ async fn test_state_offer(fixture: &StateFixture, target: &Client, peer: &Peerte wait_for_state_content(&peer.ipc_client, fixture.content_data.key.clone()).await; assert_eq!(lookup_content_value, fixture.content_data.lookup_value); } - -pub async fn test_state_recursive_gossip(peertest: &Peertest, target: &Client) { - let _ = target.ping(peertest.bootnode.enr.clone()).await.unwrap(); - - for fixture in fixtures_state_recursive_gossip().unwrap() { - let (first_key, first_value) = &fixture.key_value_pairs.first().unwrap(); - info!( - "Testing recursive gossip starting with key: {:?}", - first_key - ); - - target - .gossip(first_key.clone(), first_value.clone()) - .await - .unwrap(); - - // Verify that every key/value is fully propagated - for (key, value) in fixture.key_value_pairs { - let expected_lookup_trie_node = match value { - StateContentValue::AccountTrieNodeWithProof(value) => { - value.proof.last().unwrap().clone() - } - StateContentValue::ContractStorageTrieNodeWithProof(value) => { - value.storage_proof.last().unwrap().clone() - } - _ => panic!("Unexpected state content value: {value:?}"), - }; - let expected_lookup_value = StateContentValue::TrieNode(TrieNode { - node: expected_lookup_trie_node, - }); - - assert_eq!( - wait_for_state_content(target, key.clone()).await, - expected_lookup_value, - "Expecting lookup for {key:?} to return {expected_lookup_value:?}" - ); - } - } -} diff --git a/ethportal-peertest/src/utils.rs b/ethportal-peertest/src/utils.rs index 95c5e6ea2..584eef468 100644 --- a/ethportal-peertest/src/utils.rs +++ b/ethportal-peertest/src/utils.rs @@ -188,38 +188,3 @@ pub fn fixtures_state_contract_storage_trie_node() -> Vec { pub fn fixtures_state_contract_bytecode() -> Vec { read_state_fixture("portal-spec-tests/tests/mainnet/state/validation/contract_bytecode.yaml") } - -pub struct StateRecursiveGossipFixture { - pub state_root: B256, - pub key_value_pairs: Vec<(StateContentKey, StateContentValue)>, -} - -pub fn fixtures_state_recursive_gossip() -> Result> { - let yaml_content = fs::read_to_string( - "portal-spec-tests/tests/mainnet/state/validation/recursive_gossip.yaml", - )?; - let value: Value = serde_yaml::from_str(&yaml_content)?; - - let mut result = vec![]; - - for fixture in value.as_sequence().unwrap() { - result.push(StateRecursiveGossipFixture { - state_root: B256::deserialize(&fixture["state_root"])?, - key_value_pairs: fixture["recursive_gossip"] - .as_sequence() - .unwrap() - .iter() - .map(|key_value_container| { - let key = - StateContentKey::deserialize(&key_value_container["content_key"]).unwrap(); - let value = - StateContentValue::deserialize(&key_value_container["content_value"]) - .unwrap(); - (key, value) - }) - .collect(), - }) - } - - Ok(result) -} diff --git a/portal-bridge/src/bridge/state.rs b/portal-bridge/src/bridge/state.rs index c0952ac8d..f2625a7ee 100644 --- a/portal-bridge/src/bridge/state.rs +++ b/portal-bridge/src/bridge/state.rs @@ -10,7 +10,7 @@ use ethportal_api::{ StateContentValue, }; use revm::DatabaseRef; -use revm_primitives::{keccak256, Address, Bytecode, SpecId, B256}; +use revm_primitives::{keccak256, Bytecode, SpecId, B256}; use surf::{Client, Config}; use tokio::{ sync::{OwnedSemaphorePermit, Semaphore}, @@ -180,15 +180,11 @@ impl StateBridge { let full_key_path = [&account_proof.path.clone(), partial_key_path.as_slice()].concat(); let address_hash = full_nibble_path_to_address_hash(&full_key_path); - let address = state - .database - .get_address_from_hash(address_hash) - .expect("Contracts should always have an address in the database"); // gossip contract bytecode let code = state.database.code_by_hash_ref(account.code_hash)?; self.gossip_contract_bytecode( - address, + address_hash, &account_proof, block_tuple.header.header.hash(), account.code_hash, @@ -197,7 +193,7 @@ impl StateBridge { .await?; // gossip contract storage - let storage_changed_nodes = state.database.get_storage_trie_diff(address); + let storage_changed_nodes = state.database.get_storage_trie_diff(address_hash); let storage_walk_diff = TrieWalker::new(account.storage_root, storage_changed_nodes); @@ -207,7 +203,7 @@ impl StateBridge { self.gossip_storage( &account_proof, &storage_proof, - address, + address_hash, block_tuple.header.header.hash(), ) .await?; @@ -245,7 +241,7 @@ impl StateBridge { async fn gossip_contract_bytecode( &self, - address: Address, + address_hash: B256, account_proof: &TrieProof, block_hash: B256, code_hash: B256, @@ -257,7 +253,7 @@ impl StateBridge { .acquire_owned() .await .expect("to be able to acquire semaphore"); - let code_content_key = create_contract_content_key(address, code_hash)?; + let code_content_key = create_contract_content_key(address_hash, code_hash)?; let code_content_value = create_contract_content_value(block_hash, account_proof, code)?; Self::spawn_serve_state_proof( self.portal_client.clone(), @@ -273,7 +269,7 @@ impl StateBridge { &self, account_proof: &TrieProof, storage_proof: &TrieProof, - address: Address, + address_hash: B256, block_hash: B256, ) -> anyhow::Result<()> { let permit = self @@ -282,7 +278,7 @@ impl StateBridge { .acquire_owned() .await .expect("to be able to acquire semaphore"); - let storage_content_key = create_storage_content_key(storage_proof, address)?; + let storage_content_key = create_storage_content_key(storage_proof, address_hash)?; let storage_content_value = create_storage_content_value(block_hash, account_proof, storage_proof)?; Self::spawn_serve_state_proof( diff --git a/portal-spec-tests b/portal-spec-tests index 0c2434bd7..928aaa4a2 160000 --- a/portal-spec-tests +++ b/portal-spec-tests @@ -1 +1 @@ -Subproject commit 0c2434bd7876673f22f660c69c147c9300acf719 +Subproject commit 928aaa4a2c96b8be6de4c9515e5be8bdad8158af diff --git a/tests/self_peertest.rs b/tests/self_peertest.rs index 11c0e5b95..38dfac2b5 100644 --- a/tests/self_peertest.rs +++ b/tests/self_peertest.rs @@ -215,15 +215,6 @@ async fn peertest_state_offer_contract_bytecode() { handle.stop().unwrap(); } -#[tokio::test(flavor = "multi_thread")] -#[serial] -async fn peertest_state_recursive_gossip() { - let (peertest, target, handle) = setup_peertest("mainnet").await; - peertest::scenarios::state::test_state_recursive_gossip(&peertest, &target).await; - peertest.exit_all_nodes(); - handle.stop().unwrap(); -} - #[tokio::test(flavor = "multi_thread")] #[serial] async fn peertest_history_offer_propagates_gossip() { diff --git a/trin-execution/src/content.rs b/trin-execution/src/content.rs index e9066bd4f..7902039c2 100644 --- a/trin-execution/src/content.rs +++ b/trin-execution/src/content.rs @@ -10,7 +10,7 @@ use ethportal_api::{ }, StateContentKey, StateContentValue, }; -use revm_primitives::{Address, Bytecode}; +use revm_primitives::Bytecode; use super::types::trie_proof::TrieProof; @@ -39,11 +39,11 @@ pub fn create_account_content_value( } pub fn create_contract_content_key( - address: Address, + address_hash: B256, code_hash: B256, ) -> anyhow::Result { Ok(StateContentKey::ContractBytecode(ContractBytecodeKey { - address, + address_hash, code_hash, })) } @@ -64,7 +64,7 @@ pub fn create_contract_content_value( pub fn create_storage_content_key( storage_proof: &TrieProof, - address: Address, + address_hash: B256, ) -> anyhow::Result { let last_node = storage_proof .proof @@ -75,7 +75,7 @@ pub fn create_storage_content_key( ContractStorageTrieNodeKey { path: Nibbles::try_from_unpacked_nibbles(&storage_proof.path)?, node_hash: keccak256(last_node), - address, + address_hash, }, )) } diff --git a/trin-execution/src/storage/evm_db.rs b/trin-execution/src/storage/evm_db.rs index f9d1843c7..4676748d5 100644 --- a/trin-execution/src/storage/evm_db.rs +++ b/trin-execution/src/storage/evm_db.rs @@ -21,14 +21,12 @@ use super::{ trie_db::TrieRocksDB, }; -const REVERSE_HASH_LOOKUP_PREFIX: &[u8] = b"reverse hash lookup"; - #[derive(Debug, Clone)] pub struct EvmDB { /// State config pub config: StateConfig, - /// Storage cache for the accounts used optionally for gossiping. - pub storage_cache: HashMap>, + /// Storage cache for the accounts used optionally for gossiping, keyed by address hash. + pub storage_cache: HashMap>, /// The underlying database. pub db: Arc, /// To get proofs and to verify trie state. @@ -70,24 +68,13 @@ impl EvmDB { /// the account and instead map it to the code hash. /// /// Note: This will not insert into the underlying external database. - pub fn insert_contract(&mut self, address: Address, account: &mut AccountInfo) { + pub fn insert_contract(&mut self, account: &mut AccountInfo) { if let Some(code) = &account.code { if !code.is_empty() { if account.code_hash == KECCAK_EMPTY { account.code_hash = code.hash_slow(); } - // Insert address lookup into the database so that we can look up the address for - // smart contracts - if self.config.cache_contract_storage_changes { - self.db - .put( - [REVERSE_HASH_LOOKUP_PREFIX, keccak256(address).as_slice()].concat(), - address.as_slice(), - ) - .expect("Inserting address should never fail"); - } - // Insert contract code into the database self.db .put(account.code_hash, code.original_bytes().as_ref()) @@ -99,17 +86,14 @@ impl EvmDB { } } - pub fn get_address_from_hash(&self, address_hash: B256) -> Option
{ - self.db - .get([REVERSE_HASH_LOOKUP_PREFIX, address_hash.as_slice()].concat()) - .expect("Getting address from the database should never fail") - .map(|raw_address| Address::from_slice(&raw_address)) - } - - pub fn get_storage_trie_diff(&self, address: Address) -> BrownHashMap> { + pub fn get_storage_trie_diff(&self, address_hash: B256) -> BrownHashMap> { let mut trie_diff = BrownHashMap::new(); - for key in self.storage_cache.get(&address).unwrap_or(&HashSet::new()) { + for key in self + .storage_cache + .get(&address_hash) + .unwrap_or(&HashSet::new()) + { let value = self .db .get(key) @@ -161,7 +145,7 @@ impl DatabaseCommit for EvmDB { continue; } let is_newly_created = account.is_created(); - self.insert_contract(address, &mut account.info); + self.insert_contract(&mut account.info); let mut rocks_account: RocksAccount = match self .db @@ -230,7 +214,7 @@ impl DatabaseCommit for EvmDB { .expect("Getting the root hash should never fail"); if self.config.cache_contract_storage_changes { - let account_storage_cache = self.storage_cache.entry(address).or_default(); + let account_storage_cache = self.storage_cache.entry(address_hash).or_default(); for key in trie_diff.keys() { account_storage_cache.insert(*key); } diff --git a/trin-state/src/validation/mod.rs b/trin-state/src/validation/mod.rs index 81832ca89..6245bf6b9 100644 --- a/trin-state/src/validation/mod.rs +++ b/trin-state/src/validation/mod.rs @@ -1,5 +1,4 @@ mod error; -mod recursive_trie_proof; mod trie; mod validator; diff --git a/trin-state/src/validation/recursive_trie_proof.rs b/trin-state/src/validation/recursive_trie_proof.rs deleted file mode 100644 index f703ea683..000000000 --- a/trin-state/src/validation/recursive_trie_proof.rs +++ /dev/null @@ -1,164 +0,0 @@ -use alloy_primitives::B256; -use anyhow::{ensure, Result}; -use ethportal_api::types::state_trie::{nibbles::Nibbles, TrieProof}; - -use super::trie::TrieProofValidationInfo; - -/// The information needed in order to construct recursive gossip content key/value. -#[derive(Debug, PartialEq, Eq)] -pub struct RecursiveGossipInfo { - pub path: Nibbles, - pub proof: TrieProof, - pub last_node_hash: B256, -} - -/// Returns information needed for recursive gossip. -/// -/// Error should happen only if validation_info was not correctly created, or was created for a -/// different trie proof. -pub fn recursive_gossip( - trie_proof: &TrieProof, - validation_info: TrieProofValidationInfo, -) -> Result> { - // Check correctness of input data - ensure!(!trie_proof.is_empty(), "Empty trie proof"); - ensure!( - validation_info.remaining_path.is_empty(), - "Remaining path in the validation_info is not empty" - ); - - let new_proof_len = trie_proof.len() - 1; - if new_proof_len == 0 { - // Original proof contained only one item indicating that it was a proof for the root node. - // That means that there is no recursive gossip. - return Ok(None); - } - - ensure!( - validation_info.inner_nodes_consumed_path.len() == new_proof_len, - "Invalid length of inner_nodes_consumed_path, expected: {} actual: {}", - new_proof_len, - validation_info.inner_nodes_consumed_path.len() - ); - - let new_last_node = &trie_proof[new_proof_len - 1]; - // The new path is concatenation of all consumed nibbles up to the last node - let new_path: Vec = validation_info.inner_nodes_consumed_path[..new_proof_len - 1] - .iter() - .flatten() - .copied() - .collect(); - let new_proof = Vec::from(&trie_proof[..new_proof_len]); - - Ok(Some(RecursiveGossipInfo { - path: Nibbles::try_from_unpacked_nibbles(&new_path)?, - proof: new_proof.into(), - last_node_hash: new_last_node.node_hash(), - })) -} - -#[cfg(test)] -mod tests { - use std::collections::VecDeque; - - use ethportal_api::types::state_trie::EncodedTrieNode; - - use super::*; - - #[test] - fn single_node() { - let node = EncodedTrieNode::from(vec![0x12, 0x34]); - - let recursive_gossip_info = recursive_gossip( - &vec![node.clone()].into(), - TrieProofValidationInfo { - last_node: &node.clone(), - remaining_path: &[], - inner_nodes_consumed_path: vec![], - }, - ) - .unwrap(); - assert_eq!(recursive_gossip_info, None); - } - - #[test] - fn multiple_nodes() { - let root_node = EncodedTrieNode::from(vec![0x12, 0x34]); - let new_last_node = EncodedTrieNode::from(vec![0x56, 0x78]); - let last_node = EncodedTrieNode::from(vec![0x9a, 0xbc]); - - let consumed_paths: Vec> = vec![[1, 2, 3].into(), [4, 5, 6].into()]; - - let recursive_gossip_info = recursive_gossip( - &vec![root_node.clone(), new_last_node.clone(), last_node.clone()].into(), - TrieProofValidationInfo { - last_node: &last_node.clone(), - remaining_path: &[], - inner_nodes_consumed_path: consumed_paths, - }, - ) - .unwrap(); - - assert_eq!( - recursive_gossip_info, - Some(RecursiveGossipInfo { - path: Nibbles::try_from_unpacked_nibbles(&[1, 2, 3]).unwrap(), - proof: vec![root_node, new_last_node.clone()].into(), - last_node_hash: new_last_node.node_hash(), - }) - ) - } - - #[test] - #[should_panic = "Empty trie proof"] - fn empty_proof() { - let node = EncodedTrieNode::from(vec![0x12, 0x34]); - - recursive_gossip( - &vec![].into(), - TrieProofValidationInfo { - last_node: &node.clone(), - remaining_path: &[], - inner_nodes_consumed_path: vec![], - }, - ) - .unwrap(); - } - - #[test] - #[should_panic = "Remaining path in the validation_info is not empty"] - fn non_empty_remaining_path() { - let node = EncodedTrieNode::from(vec![0x12, 0x34]); - - recursive_gossip( - &vec![node.clone()].into(), - TrieProofValidationInfo { - last_node: &node.clone(), - remaining_path: &[1], - inner_nodes_consumed_path: vec![], - }, - ) - .unwrap(); - } - - #[test] - #[should_panic = "Invalid length of inner_nodes_consumed_path"] - fn invalid_inner_nodes_consumed_path_length() { - let root_node = EncodedTrieNode::from(vec![0x12, 0x34]); - let new_last_node = EncodedTrieNode::from(vec![0x56, 0x78]); - let last_node = EncodedTrieNode::from(vec![0x9a, 0xbc]); - - // We have 2 inner nodes, but only one consumed_path - let consumed_paths: Vec> = vec![[1, 2, 3].into()]; - - recursive_gossip( - &vec![root_node.clone(), new_last_node.clone(), last_node.clone()].into(), - TrieProofValidationInfo { - last_node: &last_node.clone(), - remaining_path: &[], - inner_nodes_consumed_path: consumed_paths, - }, - ) - .unwrap(); - } -} diff --git a/trin-state/src/validation/trie.rs b/trin-state/src/validation/trie.rs index 5fd84765a..074efc96c 100644 --- a/trin-state/src/validation/trie.rs +++ b/trin-state/src/validation/trie.rs @@ -1,12 +1,11 @@ use std::collections::VecDeque; -use alloy_primitives::{Address, B256}; +use alloy_primitives::B256; use alloy_rlp::Decodable; use eth_trie::node::Node; use ethportal_api::types::state_trie::{ account_state::AccountState, nibbles::Nibbles, EncodedTrieNode, TrieProof, }; -use keccak_hash::keccak; use super::error::StateValidationError; @@ -49,11 +48,11 @@ pub fn validate_node_trie_proof<'proof, 'path>( /// Validate the trie proof associated with the account state. pub fn validate_account_state( root_hash: Option, - account: &Address, + address_hash: &B256, proof: &TrieProof, ) -> Result { - let path: Vec = keccak(account) - .as_bytes() + let path: Vec = address_hash + .as_slice() .iter() .flat_map(Nibbles::unpack_nibble_pair) .collect(); @@ -205,7 +204,7 @@ fn traverse_node<'path>( mod tests { use std::str::FromStr; - use alloy_primitives::U256; + use alloy_primitives::{keccak256, Address, U256}; use anyhow::Result; use eth_trie::{nibbles::Nibbles as EthTrieNibbles, node::empty_children}; use ethportal_api::utils::bytes::hex_decode; @@ -361,6 +360,7 @@ mod tests { let state_root = B256::from_str("0x1ad7b80af0c28bc1489513346d2706885be90abb07f23ca28e50482adb392d61")?; let address = Address::from_str("0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2")?; + let address_hash = keccak256(address); let account_state = AccountState { nonce: 1, balance: U256::from(3272363543482522011582395_u128), @@ -384,7 +384,11 @@ mod tests { ].map(|trie_node| EncodedTrieNode::from(hex_decode(trie_node).unwrap())); assert_eq!( - validate_account_state(Some(state_root), &address, &account_proof.to_vec().into())?, + validate_account_state( + Some(state_root), + &address_hash, + &account_proof.to_vec().into() + )?, account_state ); @@ -393,9 +397,8 @@ mod tests { #[test] fn validate_account_state_just_leaf() { - let address = Address::random(); - let path: Vec = keccak(address) - .as_bytes() + let address_hash = B256::random(); + let path: Vec = address_hash .iter() .flat_map(Nibbles::unpack_nibble_pair) .collect(); @@ -407,7 +410,7 @@ mod tests { }; let node = EncodedTrieNode::from(&create_leaf(&path, &alloy_rlp::encode(&account_state))); assert_eq!( - validate_account_state(None, &address, &vec![node].into()).unwrap(), + validate_account_state(None, &address_hash, &vec![node].into()).unwrap(), account_state ); } @@ -415,17 +418,16 @@ mod tests { #[test] #[should_panic = "LeafNodeExpected"] fn validate_account_state_last_node_is_not_leaf() { - let address = Address::random(); + let address_hash = B256::random(); let node = EncodedTrieNode::from(&create_branch_with_child(Node::Empty, 1)); - validate_account_state(None, &address, &vec![node].into()).unwrap(); + validate_account_state(None, &address_hash, &vec![node].into()).unwrap(); } #[test] #[should_panic = "InvalidLeafPath"] fn validate_account_state_invalid_leaf_path() { - let address = Address::random(); - let mut path: Vec = keccak(address) - .as_bytes() + let address_hash = B256::random(); + let mut path: Vec = address_hash .iter() .flat_map(Nibbles::unpack_nibble_pair) .collect(); @@ -439,28 +441,27 @@ mod tests { code_hash: B256::random(), }; let node = EncodedTrieNode::from(&create_leaf(&path, &alloy_rlp::encode(account_state))); - validate_account_state(None, &address, &vec![node].into()).unwrap(); + validate_account_state(None, &address_hash, &vec![node].into()).unwrap(); } #[test] #[should_panic = "DecodingAccountState"] fn validate_account_state_non_decodable_account_state() { - let address = Address::random(); - let path: Vec = keccak(address) - .as_bytes() + let address_hash = B256::random(); + let path: Vec = address_hash .iter() .flat_map(Nibbles::unpack_nibble_pair) .collect(); let node = EncodedTrieNode::from(&create_leaf(&path, &[0x12, 0x34])); - validate_account_state(None, &address, &vec![node].into()).unwrap(); + validate_account_state(None, &address_hash, &vec![node].into()).unwrap(); } #[test] #[should_panic = "DecodingNode"] fn validate_account_state_non_decodable_leaf() { - let address = Address::random(); + let address_hash = B256::random(); let node = EncodedTrieNode::from(vec![0x12, 0x34]); - validate_account_state(None, &address, &vec![node].into()).unwrap(); + validate_account_state(None, &address_hash, &vec![node].into()).unwrap(); } #[test] diff --git a/trin-state/src/validation/validator.rs b/trin-state/src/validation/validator.rs index 5bbbf0e60..f9aee3536 100644 --- a/trin-state/src/validation/validator.rs +++ b/trin-state/src/validation/validator.rs @@ -3,14 +3,13 @@ use std::sync::Arc; use alloy_primitives::{keccak256, B256}; use anyhow::anyhow; use ethportal_api::{ - types::{ - content_key::state::{AccountTrieNodeKey, ContractBytecodeKey, ContractStorageTrieNodeKey}, - content_value::state::{AccountTrieNodeWithProof, ContractStorageTrieNodeWithProof}, + types::content_key::state::{ + AccountTrieNodeKey, ContractBytecodeKey, ContractStorageTrieNodeKey, }, ContentValue, StateContentKey, StateContentValue, }; use tokio::sync::RwLock; -use tracing::{debug, error}; +use tracing::debug; use trin_validation::{ oracle::HeaderOracle, validator::{ValidationResult, Validator}, @@ -18,7 +17,6 @@ use trin_validation::{ use super::{ error::StateValidationError, - recursive_trie_proof::recursive_gossip, trie::{validate_account_state, validate_node_trie_proof}, }; @@ -68,35 +66,9 @@ impl StateValidator { } StateContentValue::AccountTrieNodeWithProof(value) => { let state_root = self.get_state_root(value.block_hash).await; - let validation_info = - validate_node_trie_proof(state_root, key.node_hash, &key.path, &value.proof)?; - - let recursive_gossip_info = match recursive_gossip(&value.proof, validation_info) { - Ok(Some(recursive_gossip_info)) => recursive_gossip_info, - Ok(None) => { - // No recursive gossip - return Ok(ValidationResult::new(/* valid_for_storing= */ true)); - } - Err(err) => { - // Log the error and assume no recursive gossip - error!(error = %err, "Error while creating recursive gossip"); - return Ok(ValidationResult::new(/* valid_for_storing= */ true)); - } - }; - - let recursive_gossip_key = StateContentKey::AccountTrieNode(AccountTrieNodeKey { - path: recursive_gossip_info.path, - node_hash: recursive_gossip_info.last_node_hash, - }); - let recursive_gossip_value = - StateContentValue::AccountTrieNodeWithProof(AccountTrieNodeWithProof { - proof: recursive_gossip_info.proof, - block_hash: value.block_hash, - }); - Ok(ValidationResult::new_with_additional_content_to_propagate( - recursive_gossip_key, - recursive_gossip_value.encode(), - )) + validate_node_trie_proof(state_root, key.node_hash, &key.path, &value.proof)?; + + Ok(ValidationResult::new(/* valid_for_storing= */ true)) } _ => Err(StateValidationError::InvalidContentValueType( "AccountTrieNodeKey", @@ -123,45 +95,15 @@ impl StateValidator { StateContentValue::ContractStorageTrieNodeWithProof(value) => { let state_root = self.get_state_root(value.block_hash).await; let account_state = - validate_account_state(state_root, &key.address, &value.account_proof)?; - let validation_info = validate_node_trie_proof( + validate_account_state(state_root, &key.address_hash, &value.account_proof)?; + validate_node_trie_proof( Some(account_state.storage_root), key.node_hash, &key.path, &value.storage_proof, )?; - let recursive_gossip_info = - match recursive_gossip(&value.storage_proof, validation_info) { - Ok(Some(recursive_gossip_info)) => recursive_gossip_info, - Ok(None) => { - // No recursive gossip - return Ok(ValidationResult::new(/* valid_for_storing= */ true)); - } - Err(err) => { - // Log the error and assume no recursive gossip - error!(error = %err, "Error while creating recursive gossip"); - return Ok(ValidationResult::new(/* valid_for_storing= */ true)); - } - }; - - let recursive_gossip_key = - StateContentKey::ContractStorageTrieNode(ContractStorageTrieNodeKey { - address: key.address, - path: recursive_gossip_info.path, - node_hash: recursive_gossip_info.last_node_hash, - }); - let recursive_gossip_value = StateContentValue::ContractStorageTrieNodeWithProof( - ContractStorageTrieNodeWithProof { - storage_proof: recursive_gossip_info.proof, - account_proof: value.account_proof, - block_hash: value.block_hash, - }, - ); - Ok(ValidationResult::new_with_additional_content_to_propagate( - recursive_gossip_key, - recursive_gossip_value.encode(), - )) + Ok(ValidationResult::new(/* valid_for_storing= */ true)) } _ => Err(StateValidationError::InvalidContentValueType( "ContractStorageTrieNodeKey", @@ -197,7 +139,7 @@ impl StateValidator { let state_root = self.get_state_root(value.block_hash).await; let account_state = - validate_account_state(state_root, &key.address, &value.account_proof)?; + validate_account_state(state_root, &key.address_hash, &value.account_proof)?; if account_state.code_hash == key.code_hash { Ok(ValidationResult::new(/* valid_for_storing= */ true)) } else { @@ -286,23 +228,9 @@ mod tests { .validate_content(&content_key, &hex_decode(&content_value)?) .await?; - let recursive_gossip = &test_case["recursive_gossip"]; - let expected_result = if recursive_gossip.is_null() { - ValidationResult::new(true) - } else { - let recursive_gossip = recursive_gossip - .as_mapping() - .expect("recursive_gossip as mapping"); - ValidationResult::new_with_additional_content_to_propagate( - StateContentKey::deserialize(&recursive_gossip["content_key"])?, - hex_decode(&String::deserialize( - &recursive_gossip["content_value_offer"], - )?)?, - ) - }; assert_eq!( validation_result, - expected_result, + ValidationResult::new(true), "testing content_key: {}", content_key.to_hex() ); @@ -347,23 +275,9 @@ mod tests { .validate_content(&content_key, &hex_decode(&content_value)?) .await?; - let recursive_gossip = &test_case["recursive_gossip"]; - let expected_result = if recursive_gossip.is_null() { - ValidationResult::new(true) - } else { - let recursive_gossip = recursive_gossip - .as_mapping() - .expect("recursive_gossip as mapping"); - ValidationResult::new_with_additional_content_to_propagate( - StateContentKey::deserialize(&recursive_gossip["content_key"])?, - hex_decode(&String::deserialize( - &recursive_gossip["content_value_offer"], - )?)?, - ) - }; assert_eq!( validation_result, - expected_result, + ValidationResult::new(true), "testing content_key: {}", content_key.to_hex() ); @@ -417,56 +331,4 @@ mod tests { Ok(()) } - - #[tokio::test] - async fn recursive_gossip() -> Result<()> { - let validator = create_validator(); - - let test_cases = read_yaml_file_as_sequence("recursive_gossip.yaml"); - for test_case in test_cases { - let test_case_items = test_case["recursive_gossip"] - .as_sequence() - .expect("test_case as sequence"); - for i in 0..test_case_items.len() { - let case_item = test_case_items[i] - .as_mapping() - .expect("case item to be mapping"); - let content_key = StateContentKey::deserialize(&case_item["content_key"])?; - let content_value = String::deserialize(&case_item["content_value"])?; - - let validation_result = validator - .validate_content(&content_key, &hex_decode(&content_value)?) - .await?; - if i + 1 < test_case_items.len() { - // Next case item is recursive gossip - let next_case_item = test_case_items[i + 1] - .as_mapping() - .expect("case item to be mapping"); - let recursive_gossip_content_key = - StateContentKey::deserialize(&next_case_item["content_key"])?; - let recursive_gossip_content_value = - String::deserialize(&next_case_item["content_value"])?; - assert_eq!( - validation_result, - ValidationResult::new_with_additional_content_to_propagate( - recursive_gossip_content_key, - hex_decode(&recursive_gossip_content_value)?, - ), - "testing content_key: {}", - content_key.to_hex() - ); - } else { - // No recursive gossip - assert_eq!( - validation_result, - ValidationResult::new(true), - "testing content_key: {}", - content_key.to_hex() - ); - } - } - } - - Ok(()) - } }