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

PrecompileHandle trait #122

163 changes: 132 additions & 31 deletions src/executor/stack/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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>;

Expand All @@ -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],
Copy link
Member

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.

Copy link
Contributor Author

@nanocryk nanocryk May 16, 2022

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.

Copy link
Contributor Author

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.

gas_limit: Option<u64>,
Expand All @@ -264,6 +288,7 @@ pub trait PrecompileSet {
impl PrecompileSet for () {
fn execute(
&self,
_: &mut impl PrecompileHandle,
_: H160,
_: &[u8],
_: Option<u64>,
Expand All @@ -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
Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -1173,3 +1194,83 @@ 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,
code_address: H160,
transfer: Option<Transfer>,
input: Vec<u8>,
gas_limit: Option<u64>,
is_static: bool,
context: &Context,
) -> (ExitReason, Vec<u8>) {
// For normal calls the cost is recorded at opcode level.
// Since we don't go throught opcodes we need manually record the call
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Since we don't go throught opcodes we need manually record the call
// Since we don't go through opcodes we need manually record the call

// cost. Not doing so will make the code panic as recording the call stipend
// will do an underflow.
let gas_cost = crate::gasometer::GasCost::Call {
value: transfer.clone().map(|x| x.value).unwrap_or_else(U256::zero),
gas: U256::from(gas_limit.unwrap_or(u64::MAX)),
target_is_cold: self.0.is_cold(code_address, None),
target_exists: self.0.exists(code_address),
};

// We're not reading from EVM memory, so we record the minimum MemoryCost.
let memory_cost = Some(crate::gasometer::MemoryCost {
offset: U256::zero(),
len: U256::zero(),
Copy link
Member

Choose a reason for hiding this comment

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

I think it may be safer to set this to input.len() for now. Call itself indeed copies it.

});

if let Err(error) = self
.0
.state
.metadata_mut()
.gasometer
.record_dynamic_cost(gas_cost, memory_cost)
{
return (ExitReason::Error(error), Vec::new());
}

event!(PrecompileSubcall {
code_address: code_address.clone(),
transfer: &transfer,
input: &input,
target_gas: gas_limit,
is_static,
context
});

// Perform the subcall
match Handler::call(
self.0,
code_address,
transfer,
input,
gas_limit,
is_static,
context.clone(),
) {
Capture::Exit((s, v)) => (s, v),
Capture::Trap(_) => unreachable!(),
Copy link
Member

Choose a reason for hiding this comment

The 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);
Copy link
Member

Choose a reason for hiding this comment

The 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()
}
}
4 changes: 2 additions & 2 deletions src/executor/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ mod executor;
mod memory;

pub use self::executor::{
Accessed, PrecompileFailure, PrecompileFn, PrecompileOutput, PrecompileSet, StackExecutor,
StackExitKind, StackState, StackSubstateMetadata,
Accessed, PrecompileFailure, PrecompileFn, PrecompileHandle, PrecompileOutput, PrecompileSet,
StackExecutor, StackExitKind, StackState, StackSubstateMetadata,
};

pub use self::memory::{MemoryStackAccount, MemoryStackState, MemoryStackSubstate};
Expand Down
8 changes: 8 additions & 0 deletions src/tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ pub enum Event<'a> {
gas_limit: u64,
address: H160,
},
PrecompileSubcall {
code_address: H160,
transfer: &'a Option<Transfer>,
input: &'a [u8],
target_gas: Option<u64>,
is_static: bool,
context: &'a Context,
},
}

// Expose `listener::with` to the crate only.
Expand Down