bluealloy / revm

Rust implementation of the Ethereum Virtual Machine.
https://bluealloy.github.io/revm/
MIT License
1.65k stars 555 forks source link

Different OOG error types #336

Closed brockelmore closed 1 year ago

brockelmore commented 1 year ago

It would be nice for foundry (and likely reth) if there were a few different kinds of OutOfGas error types. For example, in https://github.com/foundry-rs/foundry/issues/3971 I had to patch foundry and add debugging lines to revm to figure out this was a memory limit OOG error.

Some variants may be:

pub enum OutOfGasError {
    // Basic OOG error
    OOG,
    // Tried to expand past REVM limit
    MemoryLimit,
    // Basic OOG error from memory expansion
    Memory,
    // Precompile threw OOG error
    Precompile,
    // When performing something that takes a U256 and casts down to a u64, if its too large this would fire
    // i.e. in `as_usize_or_fail`
    InvalidOperand
}

then the Return::OutOfGas would become Return::OutOfGas(OutOfGasError::OOG) or whatever variant best suits the error

rakita commented 1 year ago

That is a really good idea, and variants make senes!

Return was for a reason left as a simple dummy (not rust like) enum, so the change would be just adding a new field to it

Recently Result was renamed to InstructionResult: https://github.com/bluealloy/revm/blob/main/crates/interpreter/src/instruction_result.rs

And new type for Return is made for better ux: https://github.com/bluealloy/revm/blob/main/crates/primitives/src/result.rs#L117

chirag-bgh commented 1 year ago

i can take this on.

rakita commented 1 year ago

i can take this on.

Go for it! Add ordinary fields to InstructionResult and add struct that @brockelmore wrote to Halt: https://github.com/bluealloy/revm/blob/main/crates/primitives/src/result.rs#L117