Skip to content

Commit

Permalink
fix(State): Preserve original values on delete revert (#1010)
Browse files Browse the repository at this point in the history
* fix(State): Preserve original values on delete revert

* clear storage only if original is None
  • Loading branch information
rakita authored Jan 25, 2024
1 parent b434308 commit 5431bd2
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 11 deletions.
12 changes: 10 additions & 2 deletions crates/revm/src/db/states/bundle_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,16 @@ impl BundleAccount {
AccountInfoRevert::DoNothing => (),
AccountInfoRevert::DeleteIt => {
self.info = None;
self.storage = HashMap::new();
return true;
if self.original_info.is_none() {
self.storage = HashMap::new();
return true;
} else {
// set all storage to zero but preserve original values.
self.storage.iter_mut().for_each(|(_, v)| {
v.present_value = U256::ZERO;
});
return false;
}
}
AccountInfoRevert::RevertTo(info) => self.info = Some(info),
};
Expand Down
57 changes: 48 additions & 9 deletions crates/revm/src/db/states/bundle_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,16 +628,30 @@ impl BundleState {
if let Some(reverts) = self.reverts.pop() {
for (address, revert_account) in reverts.into_iter() {
self.reverts_size -= revert_account.size_hint();
if let Entry::Occupied(mut entry) = self.state.entry(address) {
let account = entry.get_mut();
self.state_size -= account.size_hint();
if account.revert(revert_account) {
entry.remove();
} else {
self.state_size += account.size_hint();
match self.state.entry(address) {
Entry::Occupied(mut entry) => {
let account = entry.get_mut();
self.state_size -= account.size_hint();
if account.revert(revert_account) {
entry.remove();
} else {
self.state_size += account.size_hint();
}
}
Entry::Vacant(entry) => {
// create empty account that we will revert on.
// Only place where this account is not existing is if revert is DeleteIt.
let mut account = BundleAccount::new(
None,
None,
HashMap::new(),
AccountStatus::LoadedNotExisting,
);
if !account.revert(revert_account) {
self.state_size += account.size_hint();
entry.insert(account);
}
}
} else {
unreachable!("Account {address:?} {revert_account:?} for revert should exist");
}
}
return true;
Expand Down Expand Up @@ -962,6 +976,31 @@ mod tests {
sanity_path(test_bundle3(), test_bundle4());
}

#[test]
fn test_multi_reverts_with_delete() {
let mut state = BundleBuilder::new(0..=3)
.revert_address(0, account1())
.revert_account_info(2, account1(), Some(Some(AccountInfo::default())))
.revert_account_info(3, account1(), Some(None))
.build();

state.revert_latest();
// state for account one was deleted
assert_eq!(state.state.get(&account1()), None);

state.revert_latest();
// state is set to
assert_eq!(
state.state.get(&account1()),
Some(&BundleAccount::new(
None,
Some(AccountInfo::default()),
HashMap::new(),
AccountStatus::Changed
))
);
}

#[test]
fn test_revert_capacity() {
let state = BundleState::builder(0..=3)
Expand Down

0 comments on commit 5431bd2

Please sign in to comment.