-
Notifications
You must be signed in to change notification settings - Fork 360
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
PrecompileHandle trait #122
Merged
sorpaas
merged 17 commits into
rust-ethereum:master
from
moonbeam-foundation:jeremy-precompile-handle-for-subcalls
May 19, 2022
Merged
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
31dbaeb
trait and dummy example
nanocryk 1f93489
improve PrecompileHandle trait
nanocryk 2a25d1a
custom call context, handle is now used to record gas and logs
nanocryk 60f1566
fixes
nanocryk 767ac85
move type param from trait to function
nanocryk 8abb44d
handle.remaining_gas
nanocryk 5349257
update doc
nanocryk baf3b9b
private struct to prevent owners of StackExecutor to call functions o…
nanocryk de95e2c
impl Trait syntax
nanocryk 2b4e6f3
clippy
nanocryk 5096ac3
record precompile subcall intrinsic cost
nanocryk e173944
clippy
nanocryk 69a4465
PrecompileSubcall event
nanocryk 3de14be
naming + no_std fix
nanocryk 7a61772
remove duplicated exit event
nanocryk c303f20
suggested changes
nanocryk 5f91f42
record input length
nanocryk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,6 @@ use alloc::{ | |
vec::Vec, | ||
}; | ||
use core::{cmp::min, convert::Infallible}; | ||
use ethereum::Log; | ||
use evm_core::{ExitFatal, ExitRevert}; | ||
use primitive_types::{H160, H256, U256}; | ||
use sha3::{Digest, Keccak256}; | ||
|
@@ -216,27 +215,51 @@ pub trait StackState<'config>: Backend { | |
#[derive(Debug, Eq, PartialEq, Clone)] | ||
pub struct PrecompileOutput { | ||
pub exit_status: ExitSucceed, | ||
pub cost: u64, | ||
pub output: Vec<u8>, | ||
pub logs: Vec<Log>, | ||
} | ||
|
||
/// Data returned by a precompile in case of failure. | ||
#[derive(Debug, Eq, PartialEq, Clone)] | ||
pub enum PrecompileFailure { | ||
/// Reverts the state changes and consume all the gas. | ||
Error { exit_status: ExitError }, | ||
/// Reverts the state changes and consume the provided `cost`. | ||
/// Reverts the state changes. | ||
/// Returns the provided error message. | ||
Revert { | ||
exit_status: ExitRevert, | ||
output: Vec<u8>, | ||
cost: u64, | ||
}, | ||
/// Mark this failure as fatal, and all EVM execution stacks must be exited. | ||
Fatal { exit_status: ExitFatal }, | ||
} | ||
|
||
impl From<ExitError> for PrecompileFailure { | ||
fn from(error: ExitError) -> PrecompileFailure { | ||
PrecompileFailure::Error { exit_status: error } | ||
} | ||
} | ||
|
||
/// Handle provided to a precompile to interact with the EVM. | ||
pub trait PrecompileHandle { | ||
/// Perform subcall in provided context. | ||
/// Precompile specifies in which context the subcall is executed. | ||
fn call( | ||
&mut self, | ||
to: H160, | ||
transfer: Option<Transfer>, | ||
input: Vec<u8>, | ||
gas_limit: Option<u64>, | ||
is_static: bool, | ||
context: &Context, | ||
) -> (ExitReason, Vec<u8>); | ||
|
||
fn record_cost(&mut self, cost: u64) -> Result<(), ExitError>; | ||
|
||
fn remaining_gas(&self) -> u64; | ||
|
||
fn log(&mut self, address: H160, topics: Vec<H256>, data: Vec<u8>); | ||
} | ||
|
||
/// A precompile result. | ||
pub type PrecompileResult = Result<PrecompileOutput, PrecompileFailure>; | ||
|
||
|
@@ -248,6 +271,7 @@ pub trait PrecompileSet { | |
/// If the provided address is not a precompile, returns None. | ||
fn execute( | ||
&self, | ||
handle: &mut impl PrecompileHandle, | ||
address: H160, | ||
input: &[u8], | ||
gas_limit: Option<u64>, | ||
|
@@ -264,6 +288,7 @@ pub trait PrecompileSet { | |
impl PrecompileSet for () { | ||
fn execute( | ||
&self, | ||
_: &mut impl PrecompileHandle, | ||
_: H160, | ||
_: &[u8], | ||
_: Option<u64>, | ||
|
@@ -283,19 +308,30 @@ impl PrecompileSet for () { | |
/// * Gas limit | ||
/// * Context | ||
/// * Is static | ||
pub type PrecompileFn = fn(&[u8], Option<u64>, &Context, bool) -> PrecompileResult; | ||
/// | ||
/// In case of success returns the output and the cost. | ||
pub type PrecompileFn = | ||
fn(&[u8], Option<u64>, &Context, bool) -> Result<(PrecompileOutput, u64), PrecompileFailure>; | ||
|
||
impl PrecompileSet for BTreeMap<H160, PrecompileFn> { | ||
fn execute( | ||
&self, | ||
handle: &mut impl PrecompileHandle, | ||
address: H160, | ||
input: &[u8], | ||
gas_limit: Option<u64>, | ||
context: &Context, | ||
is_static: bool, | ||
) -> Option<PrecompileResult> { | ||
self.get(&address) | ||
.map(|precompile| (*precompile)(input, gas_limit, context, is_static)) | ||
self.get(&address).map(|precompile| { | ||
match (*precompile)(input, gas_limit, context, is_static) { | ||
Ok((output, cost)) => { | ||
handle.record_cost(cost)?; | ||
Ok(output) | ||
} | ||
Err(err) => Err(err), | ||
} | ||
}) | ||
} | ||
|
||
/// Check if the given address is a precompile. Should only be called to | ||
|
@@ -855,32 +891,19 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> | |
} | ||
} | ||
|
||
if let Some(result) = | ||
self.precompile_set | ||
.execute(code_address, &input, Some(gas_limit), &context, is_static) | ||
{ | ||
if let Some(result) = self.precompile_set.execute( | ||
&mut StackExecutorHandle(self), | ||
code_address, | ||
&input, | ||
Some(gas_limit), | ||
&context, | ||
is_static, | ||
) { | ||
return match result { | ||
Ok(PrecompileOutput { | ||
exit_status, | ||
output, | ||
cost, | ||
logs, | ||
}) => { | ||
for Log { | ||
address, | ||
topics, | ||
data, | ||
} in logs | ||
{ | ||
match self.log(address, topics, data) { | ||
Ok(_) => continue, | ||
Err(error) => { | ||
return Capture::Exit((ExitReason::Error(error), output)); | ||
} | ||
} | ||
} | ||
|
||
let _ = self.state.metadata_mut().gasometer.record_cost(cost); | ||
let _ = self.exit_substate(StackExitKind::Succeeded); | ||
Capture::Exit((ExitReason::Succeed(exit_status), output)) | ||
} | ||
|
@@ -891,9 +914,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> | |
Err(PrecompileFailure::Revert { | ||
exit_status, | ||
output, | ||
cost, | ||
}) => { | ||
let _ = self.state.metadata_mut().gasometer.record_cost(cost); | ||
let _ = self.exit_substate(StackExitKind::Reverted); | ||
Capture::Exit((ExitReason::Revert(exit_status), output)) | ||
} | ||
|
@@ -1173,3 +1194,46 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler | |
Ok(()) | ||
} | ||
} | ||
|
||
struct StackExecutorHandle<'inner, 'config, 'precompiles, S, P>( | ||
&'inner mut StackExecutor<'config, 'precompiles, S, P>, | ||
); | ||
|
||
impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> PrecompileHandle | ||
for StackExecutorHandle<'inner, 'config, 'precompiles, S, P> | ||
{ | ||
fn call( | ||
&mut self, | ||
to: H160, | ||
transfer: Option<Transfer>, | ||
input: Vec<u8>, | ||
gas_limit: Option<u64>, | ||
is_static: bool, | ||
context: &Context, | ||
) -> (ExitReason, Vec<u8>) { | ||
match Handler::call( | ||
self.0, | ||
to, | ||
transfer, | ||
input, | ||
gas_limit, | ||
is_static, | ||
context.clone(), | ||
) { | ||
Capture::Exit((s, v)) => emit_exit!(s, v), | ||
Capture::Trap(_) => unreachable!(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment here why it's unreachable. (StackExecutor is sync.) |
||
} | ||
} | ||
|
||
fn record_cost(&mut self, cost: u64) -> Result<(), ExitError> { | ||
self.0.state.metadata_mut().gasometer.record_cost(cost) | ||
} | ||
|
||
fn log(&mut self, address: H160, topics: Vec<H256>, data: Vec<u8>) { | ||
let _ = Handler::log(self.0, address, topics, data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error should be propagated, not ignored. |
||
} | ||
|
||
fn remaining_gas(&self) -> u64 { | ||
self.0.state.metadata().gasometer.gas() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You can move all parameters other than
address
to the handle trait. The lifetime should work, as everything will be within the scope of the overall handle lifetime.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.
Sounds good. While updating our precompiles I already noticed
target_gas
was not very useful anymore since we can obtain the remaining gas. Making the handle the only parameter will also avoid breaking changes if new data should be accessible to the precompiles.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.
Hmm also moved the address, but that shouldn't make a difference.