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

Implement State Overrides for Simulations #18

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Implement State Overrides for Simulations #18

merged 3 commits into from
Sep 25, 2023

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Sep 20, 2023

This PR adds support for state overrides to the simulator.

At first, I wanted to use some of the built-in state overriding methods to the foundry_evm::Executor (namely set_balance and set_nonce). The issue was that the foundry_evm::Executor does not expose APIs for overriding code or storage slots. If you use foundry_evm::Executor::backend_mut, you additionally get foundry_evm::Backend::insert_account_info, but since there is no direct mutable access to the underlying revm::CacheDB instance (it is private unfortunately) you still cannot override storage slots. In a slightly more recent version of foundry_evm (would require updating the version in Cargo.toml which felt a bit more delecate), they added methods for manipulating account storage but there is still no way to completely override the storage for an account (when using storageOverrides[address].state and not stateDiff).

The solution is to use revm::DatabaseCommit implementation (since Backend is an revm database that implements this) and update the database as if a transaction happened.

Test Plan

Should I implement automated tests for this?.

I did some (admittedly) not very complete manual testing locally:

Get vitalik.eth's CoW Protocol token balance
curl -s http://localhost:8080/api/v1/simulate -H 'Content-Type: application/json' --data '@-' <<JSON | jq
{
  "chainId": 1,
  "from": "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045",
  "to": "0xDEf1CA1fb7FBcDC777520aa7f396b4E015F497aB",
  "data": "0x70a08231000000000000000000000000d8da6bf26964af9d7eed9e03e53415d37aa96045",
  "gasLimit": 500000,
  "value": "0",
  "blockNumber": 18175734
}
JSON
{
  "simulationId": 1,
  "gasUsed": 23994,
  "blockNumber": 18175734,
  "success": true,
  "trace": [
    {
      "callType": "CALL",
      "from": "0xd8da6bf26964af9d7eed9e03e53415d37aa96045",
      "to": "0xdef1ca1fb7fbcdc777520aa7f396b4e015f497ab",
      "value": "0x0"
    }
  ],
  "formattedTrace": null,
  "logs": [],
  "exitReason": "Return",
  "returnData": "0x0000000000000000000000000000000000000000000000000de0b6b3a7640000"
}
Turn vitalik.eth into a CoW Protocol maxi
curl -s http://localhost:8080/api/v1/simulate -H 'Content-Type: application/json' --data '@-' <<JSON | jq
{
  "chainId": 1,
  "from": "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045",
  "to": "0xDEf1CA1fb7FBcDC777520aa7f396b4E015F497aB",
  "data": "0x70a08231000000000000000000000000d8da6bf26964af9d7eed9e03e53415d37aa96045",
  "gasLimit": 500000,
  "value": "0",
  "blockNumber": 18175734,
  "stateOverrides": {
    "0xDEf1CA1fb7FBcDC777520aa7f396b4E015F497aB": {
      "stateDiff": {
        "0xfca351f4d96129454cfc8ef7930b638ac71fea35eb69ee3b8d959496beb04a33":
          "100000000000000000000000000000000000000"
      }
    }
  }
}
JSON
{
  "simulationId": 1,
  "gasUsed": 23994,
  "blockNumber": 18175734,
  "success": true,
  "trace": [
    {
      "callType": "CALL",
      "from": "0xd8da6bf26964af9d7eed9e03e53415d37aa96045",
      "to": "0xdef1ca1fb7fbcdc777520aa7f396b4e015f497ab",
      "value": "0x0"
    }
  ],
  "formattedTrace": null,
  "logs": [],
  "exitReason": "Return",
  "returnData": "0x000000000000000000000000000000004b3b4ca85a86c47a098a224000000000"
}

@nlordell
Copy link
Contributor Author

nlordell commented Sep 20, 2023

I tried to propose a change to make replace_account_storage public in foundry_evm (foundry-rs/foundry#5861) which would allow this PR to implemented by copy-pasting anvil code instead of using the "lower-level" revm::DatabaseCommit API. Hopefully, this will make the code more stable to foundry updates.

@devanoneth
Copy link
Contributor

Hey @nlordell, firstly thanks a lot for contributing! 🙏

Makes sense to go down to the revm::DatabaseCommit implementation but I see that your PR (foundry-rs/foundry#5861) was merged and we are open to pointing to the latest Foundry code if you want to open a subsequent PR for that after we get this in?

I've just merged another PR which means there are some small conflicts. Do you mind resolving those please?

Finally, could you update the README to clarify that state_overrides can now be set in the request body and add an integration test into tests/api.rs?

Once these items are closed off super happy to get this in.

@nlordell
Copy link
Contributor Author

if you want to open a subsequent PR for that after we get this in?

This makes the most sense to me. AFAICT there might be some small breaking changes that will affect other code so it makes sense to do it separately.

could you update the README [...] and add an integration test into tests/api.rs?

👍

I've just merged another PR which means there are some small conflicts

Ah, nice - you merged the #[serde(rename_all)] change (I can get rid of my manual renames then 🎉).

@nlordell
Copy link
Contributor Author

@devanoneth - updated the README with the new format and added an E2E test. Let me know if it looks OK.

I also decided to go for "simpler" types to describe the state override object. Alternatively, you could specify it like (if my TS serves me well):

export type StateOverride = {
  balance?: string;
  nonce?: number;
  code?: string;
} & StateOverrideState;

export type StateOverrideState =
  | { state?: Record<string, string>, stateDiff: undefined }
  | { state: undefined, stateDiff?: Record<string, string> }

But I found this harder to grok with little benefit over the current type (which also more closely matches how the type is represented on online documentation - for example Geth RPC documentation).

@devanoneth
Copy link
Contributor

Thanks for the updates @nlordell! Agreed on the types, simpler is better here.

@devanoneth devanoneth merged commit b4eea73 into EnsoFinance:main Sep 25, 2023
3 checks passed
@nlordell
Copy link
Contributor Author

nlordell commented Sep 25, 2023

we are open to pointing to the latest Foundry code if you want to open a subsequent PR

I started down this path, but it quite annoying ATM - it seems like Foundry is part using the new alloy types, some revm specific types, and the old ethers/ethereum_types as well. Its a bit of a mess and annoying to get working; will try to pick this up sometime, but no promises as of yet.

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.

2 participants