Conversation
mattsse
left a comment
There was a problem hiding this comment.
I think this makes sense, a nicer way would be to entirely type def the evmenv but given that revm only operates on these this is fine and we could still add type Cfg, type BlockEnv separate and then there's still a use case for additional context that isnt blockenv of cfgenv
crates/evm/src/env.rs
Outdated
| #[deref] | ||
| #[deref_mut] |
There was a problem hiding this comment.
hmm, I think this seems okay, but could also move this to dedicated .ext_mut functions instead?
mattsse
left a comment
There was a problem hiding this comment.
this is better than the extension imo
pedantic nits
| /// Trait for types that can be used as a block environment. | ||
| /// | ||
| /// Assumes that the type wraps an inner [`revm::context::BlockEnv`]. | ||
| pub trait BlockEnvironment: revm::context::Block + Debug + Send + Sync + 'static { |
There was a problem hiding this comment.
I guess this trait wouldn't be necessary if the revm trait would also have setters, but we could consider adding those separately
but maybe it's a good idea to not have setters on that trait
There was a problem hiding this comment.
yeah also not sure whether it makes sense
technically we also don't need revm's trait as supertrait here because we can just call inner but it also feels somewhat wrong to heavily rely on it even for number/timestamp queries
Adds `timestamp_millis_part` field to `TempoHeader` defining milliseconds portion of the timestamp. For now it is not propagated to EVM as this would require some more upstream changes alike to alloy-rs/evm#193
* BlockEnv AT * fix * review fixes
Adds `timestamp_millis_part` field to `TempoHeader` defining milliseconds portion of the timestamp. For now it is not propagated to EVM as this would require some more upstream changes alike to alloy-rs/evm#193
Motivation
Allows defining and propagating arbtirary extensions for
EvmEnvSolution
PR Checklist