Skip to content
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

StackExecutor inner refactor #21

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented Mar 5, 2021

Draft, for @sorpaas review.

Motivation

Current design provides one entry point per context type - transact_create, transact_create2, transact_call - that trigger the recursive execution of all opcodes in the Runtime to finally capture the ExitReason.

This works but does not allow stepping the EVM. To enable tracing, we need some sort of interface that allows capturing the state between opcode executions. There is currently another proposal from @nanocryk (imo better one, the only one until now that is purely readonly), this is just an alternative approach so we can evaluate the pros and cons of each one.

This PR proposes decoupling call_inner and create_inner in 2 public (prepare, capture) and 1 private (execute) methods.

Performance penalty

Overhead of two additional function calls per call stack.

Visibility concerns

This approach modifies the public Api, and there is no guard to guarantee that for example only call_prepare is called, with no subsequent calls to call_execute and call_capture are made.

Changes

[pub] call_prepare, create_prepare

The initial gasometer corrections, balance checks, nonce handling, etc

[priv] call_execute, create_execute

Create Runtime instance and trigger the recursive opcode execution.

[pub] call_capture, create_capture

Captures the ExitReason.

@@ -428,22 +467,22 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
}
}

fn call_inner(
pub fn call_prepare(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make create_prepare and call_prepare take only &self and not mut?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately no, they both need mutable access to the state and metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants