Add `msg.sender` validation to `select_fork` in forking backend

[anajuliabit]

Component

Forge

Have you ensured that all of these are up to date?

• [X] Foundry
• [] Foundryup

What version of Foundry are you on?

forge 0.2.0 (1ca9b85 2024-04-15T02:39:06.470405000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

Application panicked when running forge test on files that use deployCode cheatcode

No files changed, compilation skipped
The application panicked (crashed).
Message:  called `Option::unwrap()` on a `None` value
Location: /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/revm-8.0.0/src/journaled_state.rs:634

This is a bug. Consider reporting it at https://github.com/foundry-rs/foundry

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 8 frames hidden ⋮                               
   9: core::panicking::panic::h3131e0868b9f8622
      at <unknown source file>:<unknown line>
  10: core::option::unwrap_failed::hdb518deb484b6610
      at <unknown source file>:<unknown line>
  11: revm::journaled_state::JournaledState::sload::h8f7ffa9f39c270d7
      at <unknown source file>:<unknown line>
  12: <revm::evm::Evm<EXT,DB> as revm_interpreter::host::Host>::sload::h356ab75656352172
      at <unknown source file>:<unknown line>
  13: revm_interpreter::instructions::host::sload::h3e491ff0fb646f1b
      at <unknown source file>:<unknown line>
  14: revm::inspector::handler_register::inspector_instruction::{{closure}}::hf2b8cec9802a2650
      at <unknown source file>:<unknown line>
  15: revm::evm::Evm<EXT,DB>::transact::h3e054478c389d33e
      at <unknown source file>:<unknown line>
  16: foundry_evm::executors::Executor::call_raw_with_env::hf2f7c543935bd79d
      at <unknown source file>:<unknown line>
  17: foundry_evm::executors::Executor::setup::h4ca717801ea70b30
      at <unknown source file>:<unknown line>
  18: forge::runner::ContractRunner::setup::hd9a0c288470d9d3b
      at <unknown source file>:<unknown line>
  19: forge::runner::ContractRunner::run_tests::hb0272b743cd8900d
      at <unknown source file>:<unknown line>
  20: rayon::iter::plumbing::bridge_producer_consumer::helper::he7a18a896653c7fc
      at <unknown source file>:<unknown line>
  21: rayon_core::join::join_context::{{closure}}::he4ace586276ba6aa
      at <unknown source file>:<unknown line>
  22: rayon::iter::plumbing::bridge_producer_consumer::helper::he7a18a896653c7fc
      at <unknown source file>:<unknown line>
  23: <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute::h555d84d6204b2f94
      at <unknown source file>:<unknown line>
  24: rayon_core::registry::WorkerThread::wait_until_cold::hfab0e7f011d8b412
      at <unknown source file>:<unknown line>
  25: std::sys_common::backtrace::__rust_begin_short_backtrace::h3dcf045f8f44d235
      at <unknown source file>:<unknown line>
  26: core::ops::function::FnOnce::call_once{{vtable.shim}}::hbc383ad1ea9bc403
      at <unknown source file>:<unknown line>
  27: std::sys::pal::unix::thread::Thread::new::thread_start::h207e3c76161182d1
      at <unknown source file>:<unknown line>
  28: __pthread_joiner_wake<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
The application panicked (crashed).

[zerosnacks]

Can you give some more details on your code and specific test? A minimal reproduction would greatly help as I'm unable to reproduce it.

For context, the error thrown is on line which points to sload: https://github.com/bluealloy/revm/blob/1640b8f6989d0d4a19dfeb343e7967d501f6e195/crates/revm/src/journaled_state.rs#L634

[anajuliabit]

@zerosnacks

I'm having this error while using both 'forge test' and 'forge script'. One of the scripts that I'm having trouble with is this:

contract ValidateCalldata is Script {
    function run() public virtual {
        Addresses addresses = new Addresses("./addresses/Addresses.json");

        GovernorBravoDelegate governor = GovernorBravoDelegate(
            addresses.getAddress("PROTOCOL_GOVERNOR")
        );

        uint256 proposalId = vm.parseUint(vm.prompt("Proposal ID"));

        (uint256 id, , , , , , , , , ) = governor.proposals(proposalId);

        console.log("Proposal ID: ", id);

        string memory proposalPath = vm.prompt("Proposal path");

        Proposal proposal = Proposal(deployCode(proposalPath));

        proposal.run();

    }
}

You could fork this branch an run the script https://github.com/solidity-labs-io/forge-proposal-simulator/pull/60

The application panicked (crashed).
Message:  called `Option::unwrap()` on a `None` value
Location: /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/revm-8.0.0/src/journaled_state.rs:634

This is a bug. Consider reporting it at https://github.com/foundry-rs/foundry

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 8 frames hidden ⋮                               
   9: core::panicking::panic::h3131e0868b9f8622
      at <unknown source file>:<unknown line>
  10: core::option::unwrap_failed::hdb518deb484b6610
      at <unknown source file>:<unknown line>
  11: revm::journaled_state::JournaledState::sload::h8e49b14ff456d23b
      at <unknown source file>:<unknown line>
  12: <revm::evm::Evm<EXT,DB> as revm_interpreter::host::Host>::sload::h14a38b6e4759241c
      at <unknown source file>:<unknown line>
  13: revm_interpreter::instructions::host::sload::h0aa46a59c974d770
      at <unknown source file>:<unknown line>
  14: revm::inspector::handler_register::inspector_instruction::{{closure}}::hf6f6326c515f05f9
      at <unknown source file>:<unknown line>
  15: revm::evm::Evm<EXT,DB>::transact::h3e054478c389d33e
      at <unknown source file>:<unknown line>
  16: foundry_evm::executors::Executor::call_raw::h261240ba57942724
      at <unknown source file>:<unknown line>
  17: forge_script::runner::ScriptRunner::call::h0140cf02babf6c14
      at <unknown source file>:<unknown line>
  18: forge_script::execute::PreExecutionState::execute::{{closure}}::h265410e9338ecab4
      at <unknown source file>:<unknown line>
  19: forge_script::ScriptArgs::run_script::{{closure}}::hb6fea62893c80fe3
      at <unknown source file>:<unknown line>
  20: forge::main::hc93306eec46cd029
      at <unknown source file>:<unknown line>
  21: std::sys_common::backtrace::__rust_begin_short_backtrace::h7c3c93c18a7c8aef
      at <unknown source file>:<unknown line>
  22: _main<unknown>
      at <unknown source file>:<unknown line>

Thank you!

[mattsse]

looks like this is happening in scrip @klkvr

[klkvr]

@anajuliabit sorry this is a bit hard to reproduce without knowing details about your project structure. mind giving any hints on how to setup local anvil node for the failing script? or maybe there is a failing repro not requiring external inputs?

[anajuliabit]

@klkvr There is no need to use a local anvil node. You can run:

forge script script/ValidateCalldata.s.sol --rpc-url sepolia -vvvv

Then, pass 2 to the proposal ID prompt and examples/governor-bravo/BRAVO_01.sol to the proposal path prompt.

[klkvr]

The issue here is that you are switching forks inside of Prososal contract which is not persistent. Adding a simple vm.makePersistent(address(proposal)) solved this for me.

cc @mattsse do you think there is a way we can show more user-friendly errors for such internal EVM violations?

first step could be to disallow switching forks if caller contract is not persistent

[mattsse]

I tried mitigating this with a few checks in the backend's fork switching logic, but looks like we're missing another edge case

for example:

https://github.com/foundry-rs/foundry/blob/8513f619ca6781fe62d59b1bf2a8bb1bbab19927/crates/evm/core/src/backend/mod.rs#L1114-L1130

https://github.com/foundry-rs/foundry/blob/8513f619ca6781fe62d59b1bf2a8bb1bbab19927/crates/evm/core/src/backend/mod.rs#L1079-L1089

the revm evm relies on certain constraints and usually the unwraps are fine, but with fork modifications there can be edge cases where this is no longer true

I'd need a repro test case for this in order to debug

[anajuliabit]

@klkvr @mattsse @zerosnacks thank you very much!

[klkvr]

@mattsse hmm, I believe env.tx.caller is tx.origin and we need msg.sender validation here probably?

[mattsse]

that makes sense, so we also need to load this?

[zerosnacks]

Original issue has been resolved

Updated description to reflect the proposed further changes

[zerosnacks]

Hi @anajuliabit

We've since made a change that automatically marks contracts that modify the forks are marked as persistent, see: https://github.com/foundry-rs/foundry/issues/8004, fixed by https://github.com/foundry-rs/foundry/pull/8041

For now marking as resolved by https://github.com/foundry-rs/foundry/pull/8041

Would you mind seeing if you are still able to reproduce the issue against the latest version of Foundry? If so, please leave a comment / re-open the issue.

Thanks!