-
Notifications
You must be signed in to change notification settings - Fork 64
feat: add State generic to Evm trait #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add State generic to Evm trait #160
Conversation
|
hey @mattsse sorry to bump this PR, I'd happy if you give it a look. I think this PR is important as it lets implementers to return a custom state (that still needs to be converted to the evm state somehow) but at least you can pass the evm a state that contain more info than the usual evm one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's an example for this?
with still we still only can commit the evmstate so this the expectation that this is like MyState {EvmState, extrastuff}?
correct, basically it lets implementers to use a custom state that wraps the evm state. Your example perfectly fits. This is important because then in the What do you think? |
|
Do you have any specific usecase that this will enable? I could see how generic state would allow returning more context from EVM, however currently we're still bottlenecked by the database APIs I believe? i.e you can't commit anything besides The way I've thought about state abstraction since the begging was that the key abstraction point will be an |
sure I can bring my example here. I use a custom evm that has a custom state. This custom state is something like: struct MyEvmState {
evm_state: EvmState,
extra_state: ExtraState
}
// this struct contains some data that I need but you can imagine it to
// just be something like this.
struct ExtraState {
counter: u64,
}Then in the Then, when I want to use my custom evm in the executor of the node, I need to also impl the But right now, without this PR, I cannot do it because I'm constrained to use an evm that implements this But ideally I'd like this PR to be merged so that in the impl BlockExecutor for MyBlockExecutor {
fn execute_transaction_with_commit_condition(
&mut self,
tx: impl ExecutableTx<Self>,
f: impl FnOnce(&ExecutionResult<<Self::Evm as Evm>::HaltReason>) -> CommitChanges,
) -> Result<Option<u64>, BlockExecutionError> {
// ... some code here for gas checks and so on
// execute the tx
let ResultAndState {
result,
state: my_custom_state,
} = self.evm.transact(tx)?
// get evm state
let evm_state = my_custom_state.evm_state;
// commit them to the db
self.evm.db_mut().commit(evm_state);
// get my extra state
let extra_state = my_custom_state.extra_state;
// and then do whatever I want here with my extra state.
// For example I can increment a counter in MyBlockExecutor that is used somewhere in the code
self.counter += extra_state.counter;
// ... rest of the code
}
} |
|
yeah but what would this abstraction use enable exactly? is it required to re-implement some existing EVM? I don't think this pattern can be integrated into e.g reth right now And even in your example the |
The idea is to use this on a custom node that uses Reth SDK, not reth (meaning the ethereum or op-reth optimism node) directly.
That's true but only because my example is simple. I actually have a method on my custom evm where I commit the extra state like this: // execute the tx
let ResultAndState {
result,
state: my_custom_state,
} = self.evm.transact(tx)?
// get evm state
let evm_state = my_custom_state.evm_state;
// commit them to the db
self.evm.db_mut().commit(evm_state);
// get my extra state
let extra_state = my_custom_state.extra_state;
// commit it
self.evm.commit_extra_state(extra_state);The main blocker without this PR is that I cannot return a custom state from the |
|
is this an extra byproduct that you need to include in the block? |
I'll probably need to add it to the block as well. Right now I'm using it in the evm execution though. Basically based on this extra state, some valid (for the vanilla evm) transactions can be flagged as invalid. |
Problem
Evmtrait hard codes theResultAndState<Self::HaltReason>without providing the possibility to use a custom state, different than the evm state.ResultAndStatethough has the possibility to use a genericS(it defaults toEvmState).Solution
State: Into<EvmState>and use it in theResultAndStatetype:ResultAndState<Self::HaltReason, Self::State>.Stateassociated type to be convertible into aEvmStatenow, but at least we let implementers of theEvmtrait to use a custom state that wraps theEvmStatewith something more.What do you think? @mattsse