feat: verkle integration with disk db #10958

1010adigupta · 2024-09-17T14:10:26Z

This is the first pr for the integration of verkle tries in reth, and finally, code modifications for stateless-reth using verkle, I am doing this project as part of my Ethereum protocol fellowship with EF-stateless team, the project will first aim to integrate verkle in reth, then it will aim to pass all the execution-spec-tests designed for Kaustinen devnet-7(verkle's devnet), latest test fixtures for this devnet were released earlier and are available here: verkle-devnet-7-test-fixtures, I have started implementing these tests, beginning with EIP-4762, EIP-6800, remaining EIPs like EIP-7709 will be implemented subsequently, tough these tests can't be added directly to reth since this is blocked by #10595 , since reth does not have yet tool like geth's evm-t8n for running execution-spec-tests (I may be missing something here), hence I am using currently hive for these tests
aim of this pr is to integrate rust-verkle into reth's storage, tough future iterations will have a native verkle-trie, using Alloy-types inherently
would like reviews, and further discussions on it @rkrasiuk @onbjerg @mattsse @gakonst
@mattsse ig a separate branch for verkle might be needed, will push my changes there, also EF-testing team requires a separate branch for final devnet tests?

1010adigupta · 2024-09-17T14:12:35Z

crates/verkle/db/src/verkle_db.rs

+pub struct VerkleDb<'a, TX> {
+ /// The underlying key value database
+ // We try to avoid fetching from this, and we only store at the end of a batch insert
+ pub tx: &'a TX,
+ /// This stores the key-value pairs that we need to insert into the storage
+ // This is flushed after every batch insert
+ pub batch: MemoryDb,
+ /// This stores the top 3 layers of the trie, since these are the most accessed
+ // in the trie on average
+ pub cache: MemoryDb,
+}


VerkleDb is the main db struct responsible for managing trie-operations, and finally flushing them to disk once cache-depth exceedes

Can you expand a bit on the intended usage here? I'm asking because generally we avoid having long-lived read transactions open as they lead to bloat because of how MDBX works

Ideally we'd have a way to feed a set of state transitions, a temporary handle to the database and then get the changes we need to apply to the database from this. This is how we do it with the current state commitment impl and it was specifically designed to avoid MDBX pitfalls

Alternatively, instead of holding a tx, you can hold a DatabaseProvider that can give you a RO tx when needed

agreed, this approach will delay the garbage collection of MDBX, the intended use case was this:
To use rust-verkle's trie, the DB-interface that will be provided to VerkleConfig to initiate the trie which in turn will be used for all trie-operation requires to implement Flush, ReadOnlyHigherDb and WriteOnlyHigherDb traits, the functionalities of these traits will then be used internally for trie actions, see test_trie_root_commitment here, of here Flush trait is used to Flush all the changes that are currently stored in cache to permanent storage, and functions of ReadOnlyHigherDb is used for trie operations for reading data, and WriteOnlyHigherDb writes data to the cache, so basically VerkleDb works like a in-memory-db, and writes final data to disk only when flush is called and reads data from disk only when it's not found in cache

there are two approaches suggested in rust-verkle:

see rocksDB impl either we implement similar functionality for our MDBX or

what I have done till now, create a DB struct and implement required traits for it
see here for more reference

will do this:

for Flush will follow the existing method for state-commitments

for other read methods, will supply DatabaseProvider , instead of keeping a single transaction open
redesigning the method, adding the fix

1010adigupta · 2024-09-17T14:13:46Z

crates/verkle/db/src/verkle_db.rs

+ let mut trie_cursor = self.tx.cursor_read::<tables::VerkleTrie>().ok()?;
+ let mut labelled_key = Vec::with_capacity(key.len() + 1);
+ labelled_key.push(LEAF_TABLE_MARKER);
+ labelled_key.extend_from_slice(&key);
+ trie_cursor.seek_exact(labelled_key)
+ .ok()
+ .and_then(|result| result)
+ .and_then(|(_, value)| value.try_into().ok())


This part is redundant in all impls of verkleDb, ig need to add functionality such that, to avoid multiple cursor creation

1010adigupta · 2024-09-17T14:14:20Z

crates/verkle/db/src/verkle_db.rs

+ use reth_db_api::{cursor::DbCursorRW, transaction::DbTxMut};
+ use reth_provider::test_utils::create_test_provider_factory;
+ #[test]
+ fn test_verkle_storage_cursor_abstraction() {


test for basic cursor abstraction

1010adigupta · 2024-09-17T14:14:51Z

crates/verkle/db/tests/trie.rs

+
+
+#[test]
+fn test_trie_root_commitment() {


test for trie operations over VerkleDb

1010adigupta · 2024-09-17T14:16:35Z

crates/storage/db/src/tables/mod.rs

@@ -387,6 +387,8 @@ tables! {
 /// From HashedAddress => NibblesSubKey => Intermediate value
 table StoragesTrie<Key = B256, Value = StorageTrieEntry, SubKey = StoredNibblesSubKey>;

+ /// Stores current state of verkle tree.
+ table VerkleTrie<Key = Vec<u8>, Value = Vec<u8>>;


I am using Vec<u8> as the specified type for key-value, since this is what was suggested in rust-verkle's trait impls, but need some advice here, ig will have to convert it to alloy or our own custom type

Where specifically? We'd prefer to use Bytes if possible, I glaced at rust-verkle and it seems they have some &[u8] keys in the repo, we can convert these to Bytes. But not necessarily a blocker

yes all keys and values are ultimately converted to &[u8] no matter what, will use Bytes for them, adding the fix

rakita · 2024-09-18T16:41:35Z

@1010adigupta we talked about expectations and verkle in DMs. Due to its complexity, this is not something that we have resources to review rn.

onbjerg · 2024-09-18T17:16:18Z

Relaying here -

I'm reopening this because I think this is valuable. However please note verkle is low prio for us right now, so reviews might not be super thorough. I also don't think we want to merge this into main, at least for now, but I'll set up a verkle branch we can merge stuff into - it's ok if this diverges from main, so don't bother rebasing it that much. This will be helpful to see what parts of the node are affected by verkle and can perhaps inform us on future direction no matter if Ethereum ends up doing verkle or some other form of state commitment.

1010adigupta · 2024-09-18T17:23:30Z

Relaying here -

I'm reopening this because I think this is valuable. However please note verkle is low prio for us right now, so reviews might not be super thorough. I also don't think we want to merge this into main, at least for now, but I'll set up a verkle branch we can merge stuff into - it's ok if this diverges from main, so don't bother rebasing it that much. This will be helpful to see what parts of the node are affected by verkle and can perhaps inform us on future direction no matter if Ethereum ends up doing verkle or some other form of state commitment.

I have changed the base to verkle, please review and merge, I understand the reviews may not be that thorough, but I will try to provide as much info as possible in my prs, for easy reference, each pr will contain tests for all functionality @onbjerg @rkrasiuk

onbjerg

Some questions and a nit:)

onbjerg · 2024-09-18T17:22:23Z

crates/storage/db/src/tables/mod.rs

@@ -387,6 +387,8 @@ tables! {
 /// From HashedAddress => NibblesSubKey => Intermediate value
 table StoragesTrie<Key = B256, Value = StorageTrieEntry, SubKey = StoredNibblesSubKey>;

+ /// Stores current state of verkle tree.
+ table VerkleTrie<Key = Vec<u8>, Value = Vec<u8>>;


Where specifically? We'd prefer to use Bytes if possible, I glaced at rust-verkle and it seems they have some &[u8] keys in the repo, we can convert these to Bytes. But not necessarily a blocker

onbjerg · 2024-09-18T17:25:38Z

crates/verkle/db/src/verkle_db.rs

+pub struct VerkleDb<'a, TX> {
+ /// The underlying key value database
+ // We try to avoid fetching from this, and we only store at the end of a batch insert
+ pub tx: &'a TX,
+ /// This stores the key-value pairs that we need to insert into the storage
+ // This is flushed after every batch insert
+ pub batch: MemoryDb,
+ /// This stores the top 3 layers of the trie, since these are the most accessed
+ // in the trie on average
+ pub cache: MemoryDb,
+}


Can you expand a bit on the intended usage here? I'm asking because generally we avoid having long-lived read transactions open as they lead to bloat because of how MDBX works

Ideally we'd have a way to feed a set of state transitions, a temporary handle to the database and then get the changes we need to apply to the database from this. This is how we do it with the current state commitment impl and it was specifically designed to avoid MDBX pitfalls

Alternatively, instead of holding a tx, you can hold a DatabaseProvider that can give you a RO tx when needed

onbjerg · 2024-09-18T17:26:18Z

crates/verkle/db/src/verkle_db.rs

+ self.tx.commit().unwrap();
+
+ let num_items = self.batch.num_items();
+ println!("write to batch time: {}, item count : {}", now.elapsed().as_millis(), num_items);


Please avoid using println! and use reth-tracing instead like the rest of the codebase, it allows us to filter the logs:) This should e.g. be a debug level trace

okay, will keep this in mind, adding fix

1010adigupta · 2024-09-18T18:38:14Z

Had to follow this method to get rust-verkle, integrated in reth's disk, so that I could start with execution-spec tests for transition, gas-costs etc(eips basically)
I am parallelly designing the trie-from scratch so that we use reth's existing crates, types and procedure as much as possible

gakonst · 2024-09-20T06:23:34Z

Just echoing Oliver's comments to ensure expectations are set!

1010adigupta added 7 commits August 27, 2024 16:55

trie root test added

4de7f34

trie root test added

90bcf43

feat: add generic database and verkle database

c829eb7

feat: add verkle trie cursor

47618b4

feat: add verkle trie cursor

67230b1

feat: db and trie tests added

b9f79ae

feat: verkle integration with disk db

6112824

1010adigupta requested review from joshieDo, rakita and gakonst as code owners September 17, 2024 14:10

1010adigupta commented Sep 17, 2024

View reviewed changes

crates/verkle/db/tests/trie.rs

#[test]

fn test_trie_root_commitment() {

Copy link

Contributor Author

1010adigupta Sep 17, 2024

Choose a reason for hiding this comment

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

test for trie operations over VerkleDb

1010adigupta commented Sep 17, 2024

View reviewed changes

1010adigupta added 3 commits September 17, 2024 20:04

fmt: cargo fmt --all

28f4bdd

fmt: cargo fmt --all

2acb503

Merge branch 'main' into verkle-genesis-changes

1266e43

1010adigupta changed the title ~~verkle integration with disk db~~ feat: verkle integration with disk db Sep 17, 2024

1010adigupta added 4 commits September 17, 2024 21:53

fix

bab2fb6

fix

82c7bcf

fix

d536540

fix

3a594e9

rakita closed this Sep 18, 2024

onbjerg reopened this Sep 18, 2024

1010adigupta changed the base branch from main to verkle September 18, 2024 17:21

onbjerg reviewed Sep 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: verkle integration with disk db #10958

feat: verkle integration with disk db #10958

1010adigupta commented Sep 17, 2024 •

edited

Loading

1010adigupta Sep 17, 2024 •

edited

Loading

onbjerg Sep 18, 2024

1010adigupta Sep 18, 2024 •

edited

Loading

1010adigupta Sep 18, 2024

1010adigupta Sep 18, 2024

1010adigupta Sep 17, 2024

1010adigupta Sep 17, 2024

1010adigupta Sep 17, 2024

1010adigupta Sep 17, 2024

onbjerg Sep 18, 2024

1010adigupta Sep 18, 2024

rakita commented Sep 18, 2024

onbjerg commented Sep 18, 2024

1010adigupta commented Sep 18, 2024 •

edited

Loading

onbjerg left a comment

onbjerg Sep 18, 2024

onbjerg Sep 18, 2024

onbjerg Sep 18, 2024

1010adigupta Sep 18, 2024

1010adigupta commented Sep 18, 2024

gakonst commented Sep 20, 2024

feat: verkle integration with disk db #10958

Are you sure you want to change the base?

feat: verkle integration with disk db #10958

Conversation

1010adigupta commented Sep 17, 2024 • edited Loading

1010adigupta Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1010adigupta Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakita commented Sep 18, 2024

onbjerg commented Sep 18, 2024

1010adigupta commented Sep 18, 2024 • edited Loading

onbjerg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1010adigupta commented Sep 18, 2024

gakonst commented Sep 20, 2024

1010adigupta commented Sep 17, 2024 •

edited

Loading

1010adigupta Sep 17, 2024 •

edited

Loading

1010adigupta Sep 18, 2024 •

edited

Loading

1010adigupta commented Sep 18, 2024 •

edited

Loading