[Feature] Relax trait bound EVM::DB = &'db mut State<DB> for BlockExecutor impls#234
Conversation
crates/evm/src/block/mod.rs
Outdated
| /// A type which has the state of the blockchain. | ||
| /// | ||
| /// This trait encapsulates some of the functionality found in [`State`] | ||
| #[auto_impl::auto_impl(&mut)] | ||
| pub trait StateDB { |
There was a problem hiding this comment.
I think this makes sense,
I also think we'd might need something like this for BAL anyway eventually
There was a problem hiding this comment.
Funny you mention that haha. I'm working on implementing BAL for flashblocks!
klkvr
left a comment
klkvr
left a comment
There was a problem hiding this comment.
this makes sense to me, I've considered something like this before but we didn't yet have a hard requirement for non-State db on reth side
crates/evm/src/block/mod.rs
Outdated
| /// Iterates over received balances and increment all account balances. | ||
| /// | ||
| /// **Note**: If account is not found inside cache state it will be loaded from database. | ||
| /// | ||
| /// Update will create transitions for all accounts that are updated. | ||
| /// | ||
| /// If using this to implement withdrawals, zero balances must be filtered out before calling | ||
| /// this function. | ||
| fn increment_balances( | ||
| &mut self, | ||
| balances: impl IntoIterator<Item = (Address, u128)>, | ||
| ) -> Result<(), Self::Error>; |
There was a problem hiding this comment.
I think it should be possible to implement this via just Database + DatabaseCommit API but maybe not worth it
There was a problem hiding this comment.
This is. I started doing this but felt bad rewriting the impl. Would you prefer I upstream a new DatabaseCommitExt trait into revm? We could also have the trait live here. I'm not sure what makes more sense.
| ) -> Result<(), Self::Error>; | ||
| } | ||
|
|
||
| /// auto_impl unable to reconcile return associated type from supertrait |
There was a problem hiding this comment.
maybe <Self as Database>::Error works here, but we can keep this as is as welll
| /// A type which has the state of the blockchain. | ||
| /// | ||
| /// This trait encapsulates some of the functionality found in [`State`] | ||
| pub trait StateDB: revm::Database { |
There was a problem hiding this comment.
imo we can keep StateDB here, hard to find a good name here -.-
…Executor` impls (alloy-rs/evm#234) * feat: StateDB trait * feat: replace &mut State<DB> with impl StateDB * feat: move StateDB to state.rs * feat: remove StateDB associated type in favour of supertrait * feat: StateDB use fully qualified syntax * Apply suggestion from @klkvr * Apply suggestion from @klkvr * Apply suggestion from @klkvr --------- Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
4 participants
closes #233
Solution
Creates a new trait
StateDBto encapsulate some functionality fromState.We would like to reuse this code in our custom block builder but we need to pass in a wrapped version of State. Currently this is not possible. This is the only solution I could come up with that doesn't contain any breaking changes. Hope this works for you. Happy to explore other solutions as well if this doesn't meet all the requirements.
PR Checklist