github/reth
1
1
Fork 0
mirror of https://github.com/paradigmxyz/reth.git synced 2026-02-06 04:55:00 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix(poststate): introduce wiped counter (#2480)

Browse Source
This commit is contained in:
Roman Krasiuk
2023-05-01 08:37:45 +03:00
committed by GitHub
parent 3d243d3af0
commit 005ebd8511
3 changed files with 94 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
crates/revm/src/executor.rs
View File

@@ -601,7 +601,8 @@ mod tests {
Bytecode, Bytes, ChainSpecBuilder, ForkCondition, StorageKey, H256, MAINNET, U256,
};
use reth_provider::{
post_state::Storage, AccountProvider, BlockHashProvider, StateProvider, StateRootProvider,
post_state::{ChangedStorage, Storage},
AccountProvider, BlockHashProvider, StateProvider, StateRootProvider,
};
use reth_rlp::Decodable;
use std::{collections::HashMap, str::FromStr};
@@ -819,7 +820,7 @@ mod tests {
block.number,
BTreeMap::from([(
account1,
Storage {
ChangedStorage {
wiped: false,
// Slot 1 changed from 0 to 2
storage: BTreeMap::from([(U256::from(1), U256::ZERO)])
@@ -834,7 +835,10 @@ mod tests {
post_state.storage(),
&BTreeMap::from([(
account1,
Storage { wiped: false, storage: BTreeMap::from([(U256::from(1), U256::from(2))]) }
Storage {
times_wiped: 0,
storage: BTreeMap::from([(U256::from(1), U256::from(2))])
}
)]),
"Should have changed 1 storage slot"
);
@@ -992,7 +996,7 @@ mod tests {
"Selfdestructed account should have been deleted"
);
assert!(
out.storage().get(&address_selfdestruct).unwrap().wiped,
out.storage().get(&address_selfdestruct).unwrap().wiped(),
"Selfdestructed account should have its storage wiped"
);
}

104
crates/storage/provider/src/post_state.rs
View File

@@ -27,25 +27,45 @@ pub type AccountChanges = BTreeMap<BlockNumber, BTreeMap<Address, Option<Account
/// A mapping of `block -> account -> slot -> old value` that represents what slots were changed,
/// and what their values were prior to that change.
pub type StorageChanges = BTreeMap<BlockNumber, BTreeMap<Address, Storage>>;
pub type StorageChanges = BTreeMap<BlockNumber, BTreeMap<Address, ChangedStorage>>;
/// Storage for an account.
/// Changed storage state for the account.
///
/// # Wiped Storage
///
/// The field `wiped` denotes whether the pre-existing storage in the database should be cleared or
/// not.
///
/// If `wiped` is true, then the account was selfdestructed at some point, and the values contained
/// in `storage` should be the only values written to the database.
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct Storage {
pub struct ChangedStorage {
/// Whether the storage was wiped or not.
pub wiped: bool,
/// The storage slots.
pub storage: BTreeMap<U256, U256>,
}
/// Latest storage state for the account.
///
/// # Wiped Storage
///
/// The `times_wiped` field indicates the number of times the storage was wiped in this poststate.
///
/// If `times_wiped` is greater than 0, then the account was selfdestructed at some point, and the
/// values contained in `storage` should be the only values written to the database.
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct Storage {
/// The number of times the storage was wiped.
pub times_wiped: u64,
/// The storage slots.
pub storage: BTreeMap<U256, U256>,
}
impl Storage {
/// Returns `true` if the storage was wiped at any point.
pub fn wiped(&self) -> bool {
self.times_wiped > 0
}
}
// todo: rewrite all the docs for this
/// The state of accounts after execution of one or more transactions, including receipts and new
/// bytecode.
@@ -200,7 +220,7 @@ impl PostState {
}
storages.insert(
keccak256(address),
HashedStorage { wiped: storage.wiped, storage: hashed_storage },
HashedStorage { wiped: storage.wiped(), storage: hashed_storage },
);
}
@@ -259,8 +279,8 @@ impl PostState {
self.accounts.extend(other.accounts);
for (address, their_storage) in other.storage {
let our_storage = self.storage.entry(address).or_default();
if their_storage.wiped {
our_storage.wiped = true;
if their_storage.wiped() {
our_storage.times_wiped += their_storage.times_wiped;
our_storage.storage.clear();
}
our_storage.storage.extend(their_storage.storage);
@@ -328,7 +348,9 @@ impl PostState {
for (_, storages) in storage_changes_to_revert.into_iter().rev() {
for (address, storage) in storages {
self.storage.entry(address).and_modify(|head_storage| {
head_storage.wiped = storage.wiped;
if storage.wiped {
head_storage.times_wiped -= 1;
}
head_storage.storage.extend(storage.clone().storage);
});
}
@@ -405,7 +427,7 @@ impl PostState {
.entry(address)
.or_insert(Some(account));
let storage = self.storage.entry(address).or_default();
storage.wiped = true;
storage.times_wiped += 1;
storage.storage.clear();
let storage_changes =
self.storage_changes.entry(block_number).or_default().entry(address).or_default();
@@ -493,7 +515,7 @@ impl PostState {
// Write new storage state
for (address, storage) in self.storage.into_iter() {
// If the storage was wiped, remove all previous entries from the database.
if storage.wiped {
if storage.wiped() {
tracing::trace!(target: "provider::post_state", ?address, "Wiping storage from plain state");
if storages_cursor.seek_exact(address)?.is_some() {
storages_cursor.delete_current_duplicates()?;
@@ -832,7 +854,7 @@ mod tests {
// All the storage of account A has to be deleted in the database (wiped)
assert!(
state.account_storage(&address_a).expect("Account A should have some storage").wiped,
state.account_storage(&address_a).expect("Account A should have some storage").wiped(),
"The wiped flag should be set to discard all pre-existing storage from the database"
);
// Then, we must ensure that *only* the storage from the last transition will be written
@@ -870,6 +892,50 @@ mod tests {
);
}
#[test]
fn wiped_revert() {
let address = Address::random();
let init_block_number = 1;
let init_account = Account { balance: U256::from(3), ..Default::default() };
let init_slot = U256::from(1);
// Create init state for demonstration purposes
// Block 1
// Account: exists
// Storage: 0x01: 1
let mut init_state = PostState::new();
init_state.create_account(init_block_number, address, init_account);
init_state.change_storage(
init_block_number,
address,
BTreeMap::from([(init_slot, (U256::ZERO, U256::from(1)))]),
);
let mut post_state = PostState::new();
// Block 2
// Account: destroyed
// Storage: wiped
post_state.destroy_account(2, address, init_account);
assert!(post_state.storage.get(&address).unwrap().wiped());
// Block 3
// Account: recreated
// Storage: wiped, then 0x01: 2
let recreated_account = Account { balance: U256::from(4), ..Default::default() };
post_state.create_account(3, address, recreated_account);
post_state.change_storage(
3,
address,
BTreeMap::from([(init_slot, (U256::ZERO, U256::from(2)))]),
);
assert!(post_state.storage.get(&address).unwrap().wiped());
// Revert to block 2
post_state.revert_to(2);
assert!(post_state.storage.get(&address).unwrap().wiped());
}
/// Checks that if an account is touched multiple times in the same block,
/// then the old value from the first change is kept and not overwritten.
///
@@ -983,7 +1049,7 @@ mod tests {
block,
BTreeMap::from([(
address,
Storage {
ChangedStorage {
storage: BTreeMap::from([
(U256::from(0), U256::from(0)),
(U256::from(1), U256::from(3))
@@ -1008,7 +1074,7 @@ mod tests {
(U256::from(0), U256::from(2)),
(U256::from(1), U256::from(5))
]),
wiped: false
times_wiped: 0,
}
)]),
"The latest state of the storage is incorrect"
@@ -1055,7 +1121,7 @@ mod tests {
block,
BTreeMap::from([(
address,
Storage {
ChangedStorage {
storage: BTreeMap::from([(U256::from(0), U256::from(0)),]),
wiped: false,
}
@@ -1098,7 +1164,7 @@ mod tests {
block,
BTreeMap::from([(
address,
Storage {
ChangedStorage {
storage: BTreeMap::from([(U256::from(0), U256::from(1)),]),
wiped: false,
}
@@ -1132,7 +1198,7 @@ mod tests {
block,
BTreeMap::from([(
address,
Storage {
ChangedStorage {
storage: BTreeMap::from([(U256::from(0), U256::from(0)),]),
wiped: false,
}
@@ -1154,7 +1220,7 @@ mod tests {
address,
Storage {
storage: BTreeMap::from([(U256::from(0), U256::from(2)),]),
wiped: false
times_wiped: 0,
}
)]),
"The latest state of the storage is incorrect in the merged state"

2
crates/storage/provider/src/providers/post_state_provider.rs
View File

@@ -72,7 +72,7 @@ impl<SP: StateProvider, PSDP: PostStateDataProvider> StateProvider for PostState
storage.storage.get(&U256::from_be_bytes(storage_key.to_fixed_bytes()))
{
return Ok(Some(*value))
} else if storage.wiped {
} else if storage.wiped() {
return Ok(Some(U256::ZERO))
}
}