forked from paradigmxyz/reth
-
Notifications
You must be signed in to change notification settings - Fork 10
feat: latest block updates in store_trie_updates
#304
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6b28e98
fix: storage trie cursor
dhyaniarun1993 41a68bf
storage trie cursor fixed
dhyaniarun1993 cf74177
lint fix
dhyaniarun1993 528c6ce
feat: update latest block in trie updates
dhyaniarun1993 397c0c7
lintfixes
dhyaniarun1993 8846dee
bugfix
dhyaniarun1993 f2fe8cb
lintfix
dhyaniarun1993 b4f1c9c
Merge branch 'unstable' into arun/feat/latest-block
dhyaniarun1993 8498c42
review fixes
dhyaniarun1993 89f7b91
error fixes
dhyaniarun1993 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ use crate::{ | |
| }, | ||
| BlockStateDiff, OpProofsStorageError, OpProofsStorageResult, OpProofsStore, | ||
| }; | ||
| use alloy_eips::eip1898::BlockWithParent; | ||
| use alloy_primitives::{map::HashMap, B256, U256}; | ||
| use itertools::Itertools; | ||
| use reth_db::{ | ||
|
|
@@ -238,9 +239,10 @@ impl OpProofsStore for MdbxProofsStorage { | |
|
|
||
| async fn store_trie_updates( | ||
| &self, | ||
| block_number: u64, | ||
| block_ref: BlockWithParent, | ||
| block_state_diff: BlockStateDiff, | ||
| ) -> OpProofsStorageResult<()> { | ||
| let block_number = block_ref.block.number; | ||
| let sorted_trie_updates = block_state_diff.trie_updates.into_sorted(); | ||
| let sorted_account_nodes = sorted_trie_updates.account_nodes; | ||
|
|
||
|
|
@@ -259,6 +261,18 @@ impl OpProofsStore for MdbxProofsStorage { | |
| .sorted_by_key(|(hashed_address, _)| *hashed_address) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| // check latest stored block is the parent of incoming block | ||
| // todo: move this check inside the update transaction | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, we can create an inner version of this fn that takes in a tx, but doesn't have to be in this PR |
||
| let latest_hash = | ||
| self.get_latest_block_number().await?.map(|(_, hash)| hash).unwrap_or(B256::ZERO); | ||
| if latest_hash != block_ref.parent { | ||
| return Err(OpProofsStorageError::OutOfOrder { | ||
| block_number, | ||
| parent_block_hash: block_ref.parent, | ||
| latest_block_hash: latest_hash, | ||
| }); | ||
| } | ||
|
|
||
| self.env.update(|tx| { | ||
| let mut account_trie_cursor = tx.new_cursor::<AccountTrieHistory>()?; | ||
| for (path, node) in sorted_account_nodes { | ||
|
|
@@ -296,6 +310,12 @@ impl OpProofsStore for MdbxProofsStorage { | |
| } | ||
| } | ||
|
|
||
| // update proof window latest block | ||
| let mut proof_window_cursor = tx.new_cursor::<ProofWindow>()?; | ||
| proof_window_cursor.append( | ||
| ProofWindowKey::LatestBlock, | ||
| &BlockNumberHash::new(block_number, block_ref.block.hash), | ||
| )?; | ||
| Ok(()) | ||
| })? | ||
| } | ||
|
|
@@ -339,6 +359,7 @@ mod tests { | |
| models::{AccountTrieHistory, StorageTrieHistory}, | ||
| StorageTrieKey, | ||
| }; | ||
| use alloy_eips::NumHash; | ||
| use alloy_primitives::B256; | ||
| use reth_db::{cursor::DbDupCursorRO, transaction::DbTx}; | ||
| use reth_trie::{ | ||
|
|
@@ -761,7 +782,8 @@ mod tests { | |
| let store = MdbxProofsStorage::new(dir.path()).expect("env"); | ||
|
|
||
| // Sample block number | ||
| const BLOCK: u64 = 42; | ||
| const BLOCK: BlockWithParent = | ||
| BlockWithParent::new(B256::ZERO, NumHash::new(42, B256::ZERO)); | ||
dhyaniarun1993 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Sample addresses and keys | ||
| let addr1 = B256::from([0x11; 32]); | ||
|
|
@@ -829,23 +851,27 @@ mod tests { | |
| let mut cur = tx.new_cursor::<AccountTrieHistory>().expect("cursor"); | ||
|
|
||
| // Check first node | ||
| let vv1 = | ||
| cur.seek_by_key_subkey(account_path1.into(), BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK); | ||
| let vv1 = cur | ||
| .seek_by_key_subkey(account_path1.into(), BLOCK.block.number) | ||
| .expect("seek") | ||
| .expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK.block.number); | ||
| assert!(vv1.value.0.is_some()); | ||
|
|
||
| // Check second node | ||
| let vv2 = | ||
| cur.seek_by_key_subkey(account_path2.into(), BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK); | ||
| let vv2 = cur | ||
| .seek_by_key_subkey(account_path2.into(), BLOCK.block.number) | ||
| .expect("seek") | ||
| .expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK.block.number); | ||
| assert!(vv2.value.0.is_some()); | ||
|
|
||
| // Check removed node | ||
| let vv3 = cur | ||
| .seek_by_key_subkey(removed_account_path.into(), BLOCK) | ||
| .seek_by_key_subkey(removed_account_path.into(), BLOCK.block.number) | ||
| .expect("seek") | ||
| .expect("exists"); | ||
| assert_eq!(vv3.block_number, BLOCK); | ||
| assert_eq!(vv3.block_number, BLOCK.block.number); | ||
| assert!(vv3.value.0.is_none(), "Expected node deletion"); | ||
| } | ||
|
|
||
|
|
@@ -856,14 +882,16 @@ mod tests { | |
|
|
||
| // Check node for addr1 | ||
| let key1 = StorageTrieKey::new(addr1, storage_path1.into()); | ||
| let vv1 = cur.seek_by_key_subkey(key1, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK); | ||
| let vv1 = | ||
| cur.seek_by_key_subkey(key1, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK.block.number); | ||
| assert!(vv1.value.0.is_some()); | ||
|
|
||
| // Check node for addr2 | ||
| let key2 = StorageTrieKey::new(addr2, storage_path2.into()); | ||
| let vv2 = cur.seek_by_key_subkey(key2, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK); | ||
| let vv2 = | ||
| cur.seek_by_key_subkey(key2, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK.block.number); | ||
| assert!(vv2.value.0.is_some()); | ||
| } | ||
|
|
||
|
|
@@ -873,13 +901,15 @@ mod tests { | |
| let mut cur = tx.new_cursor::<HashedAccountHistory>().expect("cursor"); | ||
|
|
||
| // Check account1 (exists) | ||
| let vv1 = cur.seek_by_key_subkey(addr1, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK); | ||
| let vv1 = | ||
| cur.seek_by_key_subkey(addr1, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK.block.number); | ||
| assert_eq!(vv1.value.0, Some(acc1)); | ||
|
|
||
| // Check account2 (deletion) | ||
| let vv2 = cur.seek_by_key_subkey(addr2, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK); | ||
| let vv2 = | ||
| cur.seek_by_key_subkey(addr2, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK.block.number); | ||
| assert!(vv2.value.0.is_none(), "Expected account deletion"); | ||
| } | ||
|
|
||
|
|
@@ -890,26 +920,97 @@ mod tests { | |
|
|
||
| // Check storage for addr1 | ||
| let key1 = HashedStorageKey::new(addr1, slot1); | ||
| let vv1 = cur.seek_by_key_subkey(key1, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK); | ||
| let vv1 = | ||
| cur.seek_by_key_subkey(key1, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv1.block_number, BLOCK.block.number); | ||
| let inner1 = vv1.value.0.as_ref().expect("Some(StorageValue)"); | ||
| assert_eq!(inner1.0, val1); | ||
|
|
||
| // Check storage for addr2 | ||
| let key2 = HashedStorageKey::new(addr2, slot2); | ||
| let vv2 = cur.seek_by_key_subkey(key2, BLOCK).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK); | ||
| let vv2 = | ||
| cur.seek_by_key_subkey(key2, BLOCK.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(vv2.block_number, BLOCK.block.number); | ||
| let inner2 = vv2.value.0.as_ref().expect("Some(StorageValue)"); | ||
| assert_eq!(inner2.0, val2); | ||
| } | ||
|
|
||
| // check the latest block number in proof window | ||
| { | ||
| let tx = store.env.tx().expect("tx"); | ||
| let mut proof_window_cursor = tx.new_cursor::<ProofWindow>().expect("cursor"); | ||
| let latest_block = proof_window_cursor | ||
| .seek(ProofWindowKey::LatestBlock) | ||
| .expect("seek") | ||
| .expect("exists"); | ||
| assert_eq!(latest_block.1.number(), BLOCK.block.number); | ||
| assert_eq!(*latest_block.1.hash(), BLOCK.block.hash); | ||
| } | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn store_trie_updates_out_of_order_rejects() { | ||
| let dir = tempfile::TempDir::new().unwrap(); | ||
| let store = MdbxProofsStorage::new(dir.path()).expect("env"); | ||
|
|
||
| // set latest to some hash H1 | ||
| let existing_block = BlockWithParent::new(B256::random(), NumHash::new(1, B256::random())); | ||
| store | ||
| .set_earliest_block_number(existing_block.block.number, existing_block.block.hash) | ||
| .await | ||
| .expect("set"); | ||
|
|
||
| // incoming block whose parent != existing latest | ||
| let bad_parent = B256::from([0xFF; 32]); | ||
| let bad_block: BlockWithParent = | ||
| BlockWithParent::new(bad_parent, NumHash::new(2, B256::ZERO)); | ||
| let diff = BlockStateDiff::default(); | ||
|
|
||
| let res = store.store_trie_updates(bad_block, diff).await; | ||
| assert!(matches!(res, Err(OpProofsStorageError::OutOfOrder { .. }))); | ||
| // verify nothing written: proof window still unchanged | ||
| let latest = store.get_latest_block_number().await.expect("get latest"); | ||
| assert_eq!(latest.unwrap().1, existing_block.block.hash); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn store_trie_updates_multiple_blocks_append_versions() { | ||
| let dir = tempfile::TempDir::new().unwrap(); | ||
| let store = MdbxProofsStorage::new(dir.path()).expect("env"); | ||
|
|
||
| let addr = B256::from([0x21; 32]); | ||
| // block A (parent = ZERO) | ||
| let block_a = BlockWithParent::new(B256::ZERO, NumHash::new(1, B256::random())); | ||
| let mut diff_a = BlockStateDiff::default(); | ||
| diff_a.post_state.accounts.insert(addr, Some(Account::default())); | ||
|
|
||
| store.store_trie_updates(block_a, diff_a).await.expect("store A"); | ||
|
|
||
| // block B (parent = hash of A) | ||
| let block_b = BlockWithParent::new(block_a.block.hash, NumHash::new(2, B256::random())); | ||
| let mut diff_b = BlockStateDiff::default(); | ||
| diff_b.post_state.accounts.insert(addr, Some(Account { nonce: 5, ..Default::default() })); | ||
|
|
||
| store.store_trie_updates(block_b, diff_b).await.expect("store B"); | ||
|
|
||
| // verify we can retrieve entries for both block numbers | ||
| let tx = store.env.tx().expect("tx"); | ||
| let mut cur = tx.new_cursor::<HashedAccountHistory>().expect("cursor"); | ||
| let v_a = | ||
| cur.seek_by_key_subkey(addr, block_a.block.number).expect("seek").expect("exists"); | ||
| let v_b = | ||
| cur.seek_by_key_subkey(addr, block_b.block.number).expect("seek").expect("exists"); | ||
| assert_eq!(v_a.block_number, block_a.block.number); | ||
| assert_eq!(v_b.block_number, block_b.block.number); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_store_trie_updates_empty_collections() { | ||
| let dir = TempDir::new().unwrap(); | ||
| let store = MdbxProofsStorage::new(dir.path()).expect("env"); | ||
|
|
||
| const BLOCK: u64 = 42; | ||
| const BLOCK: BlockWithParent = | ||
dhyaniarun1993 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| BlockWithParent::new(B256::ZERO, NumHash::new(42, B256::ZERO)); | ||
|
|
||
| // Create BlockStateDiff with empty collections | ||
| let block_state_diff = BlockStateDiff::default(); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here but out of scope, will open debt issue