Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement new trie_walker which can iterate full+partial tries #1567

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions portal-bridge/src/bridge/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ use trin_execution::{
create_contract_content_value, create_storage_content_key, create_storage_content_value,
},
execution::TrinExecution,
trie_walker::TrieWalker,
types::{block_to_trace::BlockToTrace, trie_proof::TrieProof},
utils::full_nibble_path_to_address_hash,
walkers::{memory_db::ReadOnlyMemoryDB, trie_walker::TrieWalker},
};
use trin_metrics::bridge::BridgeMetricsReporter;
use trin_utils::dir::create_temp_dir;
Expand Down Expand Up @@ -159,13 +159,14 @@ impl StateBridge {
.header
.hash();

let walk_diff = TrieWalker::new(root_with_trie_diff.root, root_with_trie_diff.trie_diff);
let walk_diff = TrieWalker::new_partial_trie(
root_with_trie_diff.root,
ReadOnlyMemoryDB::new(root_with_trie_diff.trie_diff),
)?;

// gossip block's new state transitions
let mut content_idx = 0;
for node in walk_diff.nodes.keys() {
let account_proof = walk_diff.get_proof(*node);

for account_proof in walk_diff {
// gossip the account
self.gossip_account(&account_proof, block_hash, content_idx)
.await?;
Expand Down Expand Up @@ -213,10 +214,12 @@ impl StateBridge {
// gossip contract storage
let storage_changed_nodes = trin_execution.database.get_storage_trie_diff(address_hash);

let storage_walk_diff = TrieWalker::new(account.storage_root, storage_changed_nodes);
let storage_walk_diff = TrieWalker::new_partial_trie(
account.storage_root,
ReadOnlyMemoryDB::new(storage_changed_nodes),
)?;

for storage_node in storage_walk_diff.nodes.keys() {
let storage_proof = storage_walk_diff.get_proof(*storage_node);
for storage_proof in storage_walk_diff {
self.gossip_storage(
&account_proof,
&storage_proof,
Expand Down
2 changes: 1 addition & 1 deletion trin-execution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ pub mod execution;
pub mod metrics;
pub mod storage;
pub mod subcommands;
pub mod trie_walker;
pub mod types;
pub mod utils;
pub mod walkers;
199 changes: 0 additions & 199 deletions trin-execution/src/trie_walker.rs

This file was deleted.

47 changes: 47 additions & 0 deletions trin-execution/src/walkers/memory_db.rs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of partially implementing eth_trie::DB trait, how about we define out own trait. Something like this:

trait TrieWalkerDb {
    fn get(&self, key: &[u8]) -> anyhow::Result<Option<Bytes>>;
}

Then we can implement it for whatever structure we want

impl TrieWalkerDb for HashMap<B256, Bytes> {
    ...
}

impl TrieWalkerDb for AccountDB {
    ...
}

impl TrieWalkerDb for TrieRocksDB {
    ...
}

And then we would have something like this in the other file:

pub struct TrieWalker<DB: TrieWalkerDb> {
    is_partial_trie: bool,
    db: Arc<DB>,
    stack: Vec<TrieProof>,
}

What do you think?

If you decide to go this path, ignore the rest of the comments in this file.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can even add some custom logic if needed. Not sure how useful it is, but we can do something like this:

trait TrieWalkerDb {
    fn get(&self, key: &[u8]) -> anyhow::Result<Option<Bytes>>;

    fn get_node(&self, key: &[u8]) -> anyhow::Result<Option<Node>> {
        self.get(key)?
            .map(|bytes| decode_node(&mut bytes.as_ref()).map_err(|err| anyhow!(err)))
            .transpose()
    }
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use std::sync::Arc;

use anyhow::anyhow;
use eth_trie::DB;
use hashbrown::HashMap;
use parking_lot::Mutex;
use revm_primitives::B256;
use thiserror::Error;

#[derive(Debug, Error)]
pub enum ReadOnlyMemoryDBError {
morph-dev marked this conversation as resolved.
Show resolved Hide resolved
#[error("read only memory db error {0}")]
ANYHOW(#[from] anyhow::Error),
}

#[derive(Debug)]
pub struct ReadOnlyMemoryDB {
storage: Arc<Mutex<HashMap<B256, Vec<u8>>>>,
morph-dev marked this conversation as resolved.
Show resolved Hide resolved
}

impl ReadOnlyMemoryDB {
pub fn new(storage: HashMap<B256, Vec<u8>>) -> Self {
ReadOnlyMemoryDB {
storage: Arc::new(Mutex::new(storage)),
}
}
}

impl DB for ReadOnlyMemoryDB {
type Error = ReadOnlyMemoryDBError;

fn get(&self, key: &[u8]) -> Result<Option<Vec<u8>>, Self::Error> {
Ok(self.storage.lock().get(key).cloned())
}

fn insert(&self, _key: &[u8], _value: Vec<u8>) -> Result<(), Self::Error> {
Err(anyhow!("Cannot insert into read only memory db").into())
}

fn remove(&self, _key: &[u8]) -> Result<(), Self::Error> {
Err(anyhow!("Cannot remove from read only memory db").into())
}

fn flush(&self) -> Result<(), Self::Error> {
Err(anyhow!("Cannot flush read only memory db").into())
}
}
2 changes: 2 additions & 0 deletions trin-execution/src/walkers/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod memory_db;
pub mod trie_walker;
Loading
Loading