feat: expose storage to tracers

[joonazan]

Tracers can now read storage slots. This is required for the validation tracer.

TODO

• check that the extra plumbing does not affect performance

[joonazan]

There is no huge performance impact at least. I cannot reliable measure any difference but I did not inspect the assembly.

[slowli]

The new interface looks weird. I guess it could be caused by performance, but ideally I'd like to see the Tracer interface left unchanged and state interface extended as either

pub trait StorageInterface {
    /// Gets value of the specified storage slot.
    fn get_storage(
        &mut self,
        address: H160,
        slot: U256,
    ) -> U256;
}

/// Public interface of the VM state. Encompasses both read and write methods.
pub trait StateInterface {
    /// Gets a storage handle.
    fn storage(&mut self) -> impl StorageInterface;

   // ...
}

... or even

pub trait StateInterface: StorageInterface { /* ... */ }

In order to do this, VirtualMachine should include the World (i.e., W param). Are there any issues preventing this? Of course, VirtualMachine should provide a public getter for World so that it can still be accessed by the glue code. Technically, there's an alternative: implementing StateInterface for an adapter type like this:

pub(crate) struct VmState<'a, T, W> {
    vm: &'a mut VirtualMachine<T, W>,
    world: &'a mut W,
}

impl<T: Tracer, W: World<T>> StateInterface for VmState<'_, T, W> {
    // ...
}

...but I don't like it because it means that StateInterface method can no longer be used on VirtualMachine (which happens in the glue code and in the unit tests in this repo).

[joonazan]

@slowli Take a look at the commit history. I initially had the adapter but it turned out very unergonomic because the state interface is used for non-tracer purposes as well.
I used to store the world in the VM, but it was problematic. I switched to the current design when I saw that Reth does the same.

[slowli]

I see. How about this alternative:

pub trait StateInterface {
    // left as-is
}

// Name TBD
pub trait TracedStateInterface: StateInterface {
    fn get_storage(
        &mut self,
        address: H160,
        slot: U256,
    ) -> U256;
}

pub trait Tracer {
    /// Executes logic before an instruction handler.
    ///
    /// The default implementation does nothing.
    fn before_instruction<OP: OpcodeType, S: TracedStateInterface>(
        &mut self,
        state: &mut S,
    ) {
       // ...
    }
}

Then, StateInterface can still be implemented for VirtualMachine, and implementing TracedStateInterface would require an adapter. A potential alternative that would further simplify the transition, but can lead to more verbose uses in tracers is

pub trait StateInterface {
    // left as-is
}

pub trait TracedStateInterface {
    fn base(&self) -> &impl StateInterface;
    fn base_mut(&mut self) -> &mut impl StateInterface;
    // a potential alternative is having an associated type:
    // type Base: StateInterface;
    // and requiring `TracedStateInterface: AsRef<Self::Base> + AsMut<Self::Base>`

    fn get_storage(
        &mut self,
        address: H160,
        slot: U256,
    ) -> U256;
}

Then, the wrapper would be as simple as

pub(crate) struct VmState<'a, T, W> {
    vm: &'a mut VirtualMachine<T, W>,
    world: &'a mut W,
}

impl<T: Tracer, W: World<T>> TracedStateInterface for VmState<'_, T, W> {
    fn base(&self) -> &impl StateInterface {
        self.vm
    }

    fn base_mut(&mut self) -> &mut impl StateInterface {
        self.vm
    }

    fn get_storage(&mut self, address: H160, slot: U256) -> U256 {
        self.vm.world_diff.just_read_storage(self.world, address, slot)
    }
}

[joonazan]

The extension trait is the best suggestion IMO because using the base method would make writing tracers clunky.
Is there some library or trick to "inherit" methods from a field like in Go?

I also have an issue with the wrapper: It seems wrong to me to pass &mut (&mut A, &mut B), though because the types are know at compile time that probably compiles away. (&mut A, &mut B) cannot be used instead because &mut is not Copy.

[slowli]

AFAICT, there are no tricks for such trait delegation in the language itself. There are some crates implementing such delegation, such as delegate-attr and ambassador (I like the former better, although I haven't used either of them in production). Subjectively, composition approach (either via getters or AsRef / AsMut) looks like an OK solution to me; it seems to correspond to the overall "composition over inheritance" Rust motto.

It seems wrong to me to pass &mut (&mut A, &mut B)

I do think that the compiler should simplify this away; logically, there's no reason not to. In general, re-borrowing references in Rust is somewhat common, and it doesn't look like an anti-pattern.

[slowli]

Nice!