feat: integrate new revm #7
Conversation
klkvr
force-pushed
the
klkvr/new-revm
branch
2 times, most recently
from
804e592 to
1fa969d
Compare
| // Explicitly set nonce to 0 so revm does not do any nonce checks | ||
| nonce: 0, |
There was a problem hiding this comment.
it's not, we now have CfgEnv::disable_nonce_check for it
There was a problem hiding this comment.
Did we want to ask Dragan to bring back optional nonces?
| /// Abstraction over transaction validation error. | ||
| pub trait InvalidTxError: Error + Send + Sync + 'static { | ||
| /// Returns whether the error cause by transaction having a nonce lower than expected. | ||
| fn is_nonce_too_low(&self) -> bool; | ||
| } |
There was a problem hiding this comment.
as mentioned, I like this a lot, we can extend these as we see fit, the alternative would be an enum with variants we know exist that the network specific enum should wrap, but imo a trait like this is more convenient
| } | ||
| } | ||
|
|
||
| impl<DBError, TxError> EvmError for EVMError<DBError, TxError> |
There was a problem hiding this comment.
ideally this should only need one error
| #[derive(Debug)] | ||
| pub enum EthEvm<DB: Database, I> { | ||
| /// Simple EVM implementation. | ||
| Simple(EthEvmContext<DB>), |
There was a problem hiding this comment.
maybe?
bacuse raw evm execution?
| Simple(EthEvmContext<DB>), | |
| Raw(EthEvmContext<DB>), |
| impl<DB: Database, I> Deref for EthEvm<DB, I> { | ||
| type Target = EthEvmContext<DB>; | ||
|
|
||
| fn deref(&self) -> &Self::Target { |
There was a problem hiding this comment.
| fn deref(&self) -> &Self::Target { | |
| #[inline] | |
| fn deref(&self) -> &Self::Target { |
| let prev_block_env = self.block.clone(); | ||
| let prev_cfg_env = self.cfg.clone(); |
There was a problem hiding this comment.
These clones are OK? I would love if we didn't need them?
| #[cfg(feature = "op")] | ||
| impl InvalidTxError for revm_optimism::OpTransactionError { |
There was a problem hiding this comment.
Sad that we need this, but I get it
| /// Error type returned by EVM. Contains either errors related to invalid transactions or | ||
| /// internal irrecoverable execution errors. | ||
| type Error: EvmError; | ||
| /// Halt reason. Enum over all possible reasons for halting the execution. When execution halts, | ||
| /// it means that transaction is valid, however, it's execution was interrupted (e.g because of | ||
| /// running out of gas or overflowing stack). | ||
| type HaltReason: HaltReasonTrait + Send + Sync; |
There was a problem hiding this comment.
Should we not exposing these 2 generics, pick a concrete type for each, and have an Other(String) variant, if we want to simplify this trait a bit?
There was a problem hiding this comment.
this isn't sufficient because we need to handle errors and halts differently.
e.g. in gas estimation and when converting the proper rpc error message
| // Explicitly set nonce to 0 so revm does not do any nonce checks | ||
| nonce: 0, |
There was a problem hiding this comment.
Did we want to ask Dragan to bring back optional nonces?
Motivation
Solution
PR Checklist