Skip to content

Commit 83328d7

Browse files
rakitaRjected
authored andcommitted
fix(State): Preserve original values on delete revert (bluealloy#1010)
* fix(State): Preserve original values on delete revert * clear storage only if original is None (cherry picked from commit 5431bd2)
1 parent e13f604 commit 83328d7

File tree

2 files changed

+58
-11
lines changed

2 files changed

+58
-11
lines changed

crates/revm/src/db/states/bundle_account.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,16 @@ impl BundleAccount {
9292
AccountInfoRevert::DoNothing => (),
9393
AccountInfoRevert::DeleteIt => {
9494
self.info = None;
95-
self.storage = HashMap::new();
96-
return true;
95+
if self.original_info.is_none() {
96+
self.storage = HashMap::new();
97+
return true;
98+
} else {
99+
// set all storage to zero but preserve original values.
100+
self.storage.iter_mut().for_each(|(_, v)| {
101+
v.present_value = U256::ZERO;
102+
});
103+
return false;
104+
}
97105
}
98106
AccountInfoRevert::RevertTo(info) => self.info = Some(info),
99107
};

crates/revm/src/db/states/bundle_state.rs

+48-9
Original file line numberDiff line numberDiff line change
@@ -624,16 +624,30 @@ impl BundleState {
624624
if let Some(reverts) = self.reverts.pop() {
625625
for (address, revert_account) in reverts.into_iter() {
626626
self.reverts_size -= revert_account.size_hint();
627-
if let Entry::Occupied(mut entry) = self.state.entry(address) {
628-
let account = entry.get_mut();
629-
self.state_size -= account.size_hint();
630-
if account.revert(revert_account) {
631-
entry.remove();
632-
} else {
633-
self.state_size += account.size_hint();
627+
match self.state.entry(address) {
628+
Entry::Occupied(mut entry) => {
629+
let account = entry.get_mut();
630+
self.state_size -= account.size_hint();
631+
if account.revert(revert_account) {
632+
entry.remove();
633+
} else {
634+
self.state_size += account.size_hint();
635+
}
636+
}
637+
Entry::Vacant(entry) => {
638+
// create empty account that we will revert on.
639+
// Only place where this account is not existing is if revert is DeleteIt.
640+
let mut account = BundleAccount::new(
641+
None,
642+
None,
643+
HashMap::new(),
644+
AccountStatus::LoadedNotExisting,
645+
);
646+
if !account.revert(revert_account) {
647+
self.state_size += account.size_hint();
648+
entry.insert(account);
649+
}
634650
}
635-
} else {
636-
unreachable!("Account {address:?} {revert_account:?} for revert should exist");
637651
}
638652
}
639653
return true;
@@ -943,6 +957,31 @@ mod tests {
943957
sanity_path(test_bundle3(), test_bundle4());
944958
}
945959

960+
#[test]
961+
fn test_multi_reverts_with_delete() {
962+
let mut state = BundleBuilder::new(0..=3)
963+
.revert_address(0, account1())
964+
.revert_account_info(2, account1(), Some(Some(AccountInfo::default())))
965+
.revert_account_info(3, account1(), Some(None))
966+
.build();
967+
968+
state.revert_latest();
969+
// state for account one was deleted
970+
assert_eq!(state.state.get(&account1()), None);
971+
972+
state.revert_latest();
973+
// state is set to
974+
assert_eq!(
975+
state.state.get(&account1()),
976+
Some(&BundleAccount::new(
977+
None,
978+
Some(AccountInfo::default()),
979+
HashMap::new(),
980+
AccountStatus::Changed
981+
))
982+
);
983+
}
984+
946985
#[test]
947986
fn test_revert_capacity() {
948987
let state = BundleState::builder(0..=3)

0 commit comments

Comments
 (0)