chore: bump revm dec 2024 #246
Conversation
|
As discussed would like to hold on merging until we've iterated on the revm api a bit as this is used in a bunch of places and want to be thoughtful about breakages |
Doing exactly that, and I have already accounted interfaces and usage patterns when refactoring Revm. Changes that simplify integration even more, are here: bluealloy/revm#1934 Present context allows us a lot simpler usage and it is easier to navigate. Would assume this will not be merged until we have PR in Reth. |
|
@mattsse this is now integrated. We should wait for Reth integration before merging. |
Co-authored-by: Arsenii Kulikov <[email protected]>
Co-authored-by: Arsenii Kulikov <[email protected]>
| to | ||
| } else { | ||
| // We need to exclude the created address if this is a CREATE frame. | ||
| // | ||
| // This assumes that caller has already been loaded but nonce was not increased yet. | ||
| let nonce = context.journaled_state.account(from).info.nonce; | ||
| let nonce = context.journal_ref().evm_state().get(&from).unwrap().info.nonce; |
There was a problem hiding this comment.
nit, can we drop the _ref() suffix here? should just be journal and journal_mut
There was a problem hiding this comment.
In most cases a mutable journal is needed, there was few cases were ref is needed so this fn was added
There was a problem hiding this comment.
I see, then I think it should remain like that for legacy reasons
src/access_list.rs
Outdated
| where | ||
| DB: Database, | ||
| CTX: ContextTrait<Journal: JournalExt>, |
There was a problem hiding this comment.
naming a trait Trait always feels a bit weird to me
There was a problem hiding this comment.
Will rename all of them to ContextT.
src/tracing/builder/geth.rs
Outdated
| @@ -216,7 +219,7 @@ impl<'a> GethTraceBuilder<'a> { | |||
| /// * `db` - The database to fetch state pre-transaction execution. | |||
| pub fn geth_prestate_traces<DB: DatabaseRef>( | |||
| &self, | |||
| ResultAndState { state, .. }: &ResultAndState, | |||
| ResultAndState { state, .. }: &ResultAndState<impl HaltReasonTrait>, | |||
There was a problem hiding this comment.
can we make this a named generic, don't like mixing this
src/tracing/builder/parity.rs
Outdated
| @@ -170,7 +173,7 @@ impl ParityTraceBuilder { | |||
| /// with the [TracingInspector](crate::tracing::TracingInspector). | |||
| pub fn into_trace_results_with_state<DB: DatabaseRef>( | |||
| self, | |||
| res: &ResultAndState, | |||
| res: &ResultAndState<impl HaltReasonTrait>, | |||
There was a problem hiding this comment.
same, can we name this just HaltReason or HaltReasonT
| pub(crate) struct StringError(pub String); | ||
|
|
||
| impl core::error::Error for StringError {} | ||
| impl DBErrorMarker for StringError {} |
| #[derive(Clone, Debug)] | ||
| pub(crate) struct StringError(pub String); |
There was a problem hiding this comment.
needs docs, why this is necessary
| #[derive(Clone, Debug)] | ||
| pub(crate) struct StringError(pub String); |
There was a problem hiding this comment.
can we name this JsDbError
src/tracing/js/mod.rs
Outdated
| pub fn result<TX, DB>( | ||
| &mut self, | ||
| res: ResultAndState, | ||
| env: &Env, | ||
| res: ResultAndState<impl HaltReasonTrait>, | ||
| tx: &TX, | ||
| block: &impl Block, |
There was a problem hiding this comment.
can we make these consistent across all functions, don't like to mix this
klkvr
force-pushed
the
rakita/bump_revm_dec_2024
branch
from
a7ed3df to
3c6cca4
Compare
Revm Framework integration