Skip to content
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

feat(cheatcodes): recordAccountAccesses and recordStorageAccesses cheatcodes #6087

Closed
wants to merge 1 commit into from
Closed

feat(cheatcodes): recordAccountAccesses and recordStorageAccesses cheatcodes #6087

wants to merge 1 commit into from

Conversation

refcell
Copy link
Contributor

@refcell refcell commented Oct 23, 2023

Motivation

Warning

This pr takes up the work from #5794 to port it to the new alloy types as well as address review comments. Picking this up since it's critical work for external stakeholders and there hasn't been an update in almost two months.

-- Original Context Below --

Currently, it is not possible to get an ordered list of account or storage accesses made by the EVM during test execution.

For account accesses, there are cheats such as vm.expectCall, but this requires knowing the exact destination and parameters of calls ahead of time.

For storage accesses, there are the vm.record/vm.accesses cheats, but these are not enumerated or ordered overall, and since it is a mapping, it require knowing which accounts exactly you'd like to check storage accesses for (this is made easier by the new mapping cheatcodes).

Further, it is useful to know the context of account and storage accesses, in particular:

For accounts

  • What account is being accessed?
  • Is the account access the result of a create or a call opcode?
  • Is the account initialized or empty?
  • Is value passed along with the account access?
  • What input data was provided to the create or call?
  • Was this access made as part of a context that was reverted?

For storage

  • Which account's storage was accessed?
  • Which slot was accessed?
  • Was this access a read or a write?
  • What was the value before this access?
  • What was the value after this access?
  • Was this access made as part of a context that was reverted?

This would allow for things like manual gas accounting due to accounts and storage slots marked as "warm" during test setup during execution.

Solution

This PR adds the following cheatcodes and structs:

struct AccountAccess {
    address account;
    bool isCreate;
    bool initialized;
    uint256 value;
    bytes data;
    bool reverted;
}

struct StorageAccess {
    address account;
    bytes32 slot;
    bool isWrite;
    bytes32 previousValue;
    bytes32 newValue;
    bool reverted;
}

// Record all account accesses as part of CREATE or CALL opcodes in order,
// along with the context of the calls
function recordAccountAccesses() external;

// Returns an ordered array of all account accesses
function getRecordedAccountAccesses() external returns (AccountAccess[] memory);

// Record all storage accesses in order, along with the context of the accesses
function recordStorageAccesses() external;

// Returns an ordered array of all storage accesses
function getRecordedStorageAccesses() external returns (StorageAccess[] memory);

For accounts

Internally, Forge maintains a Vec<Vec<RecordedAccountAccess>>. The vector is 2-dimensional to allow for grouping account accesses by relative call depth, so they can be updated with the eventual failure status of that callframe before being merged with the previous Vec to preserve absolute ordering.

On each call or create, a single-element Vec<RecordedAccountAccess> is appended to the state with the context of the CALL or CREATE opcode.

On call_end or create_end, the last Vec<RecordedAccountAccess> is pop()'d off and the accesses therein are updated with the failure status of the CREATE or CALL opcode. This Vec<RecordedAccountAccess> is then merged with the previous in the Vec<Vec<...>>, or else re-appended to the empty Vec<Vec<...>>.

For storage

Internally, Forge maintains a Vec<Vec<RecordedStorageAccess>>. The vector is 2-dimensional to allow for grouping storage accesses by relative call depth, so they can be updated with the eventual failure status of that callframe before being merged with the previous Vec to preserve absolute ordering.

On each SLOAD or SSTORE, a RecordedStorageAccess is appended to the last Vec<RecordedStorageAccess>, or else appended as a single-element Vec.

On call_end or create_end, the last Vec<RecordedStorageAccess> is pop()'d off and the accesses therein are updated with the failure status of the CREATE or CALL opcode. This Vec<RecordedStorageAccess> is then merged with the previous in the Vec<Vec<...>>, or else re-appended to the empty Vec<Vec<...>>

Reasoning

Ordering is important when manually calculating account or storage-slot "warm-ness," as is the eventual reverted status of a callframe in which an account or storage access happened.

Opcodes have different costs depending on whether or not a storage slot is "warm," "cold," or going from zero-to-non-zero value, or when sending value to an empty/uninitialized account. On an even lower-level, the EVM does not mark "cold" accounts or slots as "warm" if the context from which they were accessed reverts (with the exception of the counterfactual create address).

Thus, context like previousValue, newValue, isCreate, initialized, and reverted are important for understanding and evaluating the (hypothetical) behavior of the EVM.

@refcell refcell marked this pull request as ready for review October 23, 2023 22:12
@refcell
Copy link
Contributor Author

refcell commented Oct 23, 2023

Co-authored commits with @jameswenzel 😄

Co-authored-by: James Wenzel <wenzel.james.r@gmail.com>
@refcell
Copy link
Contributor Author

refcell commented Oct 26, 2023

This pr should be replaced by a new pr addressing the following issue: #6125

@DaniPopes
Copy link
Member

@refcell what's the status here? Should this be closed?

@mds1
Copy link
Collaborator

mds1 commented Nov 7, 2023

@refcell is out this week, I believe he's planning to open a new PR based on the spec in #6125 once back, so I'll close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants