test(op-revm): Adds caller nonce assertion to op-revm intergation tests#2815
test(op-revm): Adds caller nonce assertion to op-revm intergation tests#2815rakita merged 4 commits intobluealloy:mainfrom
Conversation
crates/op-revm/tests/integration.rs
Outdated
| assert_eq!( | ||
| output.state.get(&Address::ZERO).map(|a| a.info.nonce), | ||
| Some(0) | ||
| ); |
There was a problem hiding this comment.
double checking that this the expected behaviour for system call?
There was a problem hiding this comment.
Yes, system call does not touch caller to beneficiary account
There was a problem hiding this comment.
if caller account wouldn't be touched, then it should be None like for deposit, not Some(0). adding an assertion here to verifying that caller has not been marked as touched, the test fails.
assert!(!output.state.get(&Address::ZERO).unwrap().is_touched());There was a problem hiding this comment.
Ah this is touched after .replay. If you check account after system_call, it will not be touched.
Although I see expectation for replay to behave the same as system call, but this would make things complex and not performant. The intention for replay is to be removed somewhere in the future and for users to rely on .transact(tx).
There was a problem hiding this comment.
true! that replay statement isn't necessary there. thanks.
CodSpeed Performance ReportMerging #2815 will not alter performanceComparing Summary
|
|
No need to check the nonce every time with assert as it is checked/saved inside json fixture. This PR should order it so it will be easier to diff in github: #2813 |
emhane
force-pushed
the
op-nonce-check-in-tests
branch
from
c1f8e01 to
13f2736
Compare
…ts (bluealloy#2815) * Adds handler test for verifying unchanged nonce on balance too low revert * Fix system call test * fixup! Fix merge conflicts
…ts (bluealloy/revm#2815) * Adds handler test for verifying unchanged nonce on balance too low revert * Fix system call test * fixup! Fix merge conflicts
2 participants
Ref #2189, #2805