Skip to content

Commit

Permalink
refactor: update state types to use address_hash instead of address t…
Browse files Browse the repository at this point in the history
…o match spec + remove recursive gossip (#1353)
  • Loading branch information
KolbyML authored Aug 6, 2024
1 parent 276e77a commit 688847c
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 483 deletions.
23 changes: 12 additions & 11 deletions ethportal-api/src/types/content_key/state.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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.
Expand All @@ -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,
}
Expand Down Expand Up @@ -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)
)
}
Expand All @@ -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;
Expand Down Expand Up @@ -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"]),
});
Expand All @@ -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"]),
});

Expand Down
47 changes: 2 additions & 45 deletions ethportal-peertest/src/scenarios/state.rs
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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:?}"
);
}
}
}
35 changes: 0 additions & 35 deletions ethportal-peertest/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,38 +188,3 @@ pub fn fixtures_state_contract_storage_trie_node() -> Vec<StateFixture> {
pub fn fixtures_state_contract_bytecode() -> Vec<StateFixture> {
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<Vec<StateRecursiveGossipFixture>> {
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)
}
20 changes: 8 additions & 12 deletions portal-bridge/src/bridge/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -207,7 +203,7 @@ impl StateBridge {
self.gossip_storage(
&account_proof,
&storage_proof,
address,
address_hash,
block_tuple.header.header.hash(),
)
.await?;
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -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
Expand All @@ -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(
Expand Down
9 changes: 0 additions & 9 deletions tests/self_peertest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions trin-execution/src/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use ethportal_api::{
},
StateContentKey, StateContentValue,
};
use revm_primitives::{Address, Bytecode};
use revm_primitives::Bytecode;

use super::types::trie_proof::TrieProof;

Expand Down Expand Up @@ -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<StateContentKey> {
Ok(StateContentKey::ContractBytecode(ContractBytecodeKey {
address,
address_hash,
code_hash,
}))
}
Expand All @@ -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<StateContentKey> {
let last_node = storage_proof
.proof
Expand All @@ -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,
},
))
}
Expand Down
38 changes: 11 additions & 27 deletions trin-execution/src/storage/evm_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address, HashSet<B256>>,
/// Storage cache for the accounts used optionally for gossiping, keyed by address hash.
pub storage_cache: HashMap<B256, HashSet<B256>>,
/// The underlying database.
pub db: Arc<RocksDB>,
/// To get proofs and to verify trie state.
Expand Down Expand Up @@ -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())
Expand All @@ -99,17 +86,14 @@ impl EvmDB {
}
}

pub fn get_address_from_hash(&self, address_hash: B256) -> Option<Address> {
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<B256, Vec<u8>> {
pub fn get_storage_trie_diff(&self, address_hash: B256) -> BrownHashMap<B256, Vec<u8>> {
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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit 688847c

Please sign in to comment.