PrecompileHandle trait #122
Conversation
…utside of precompile context
|
Currently panics when doing subcalls because we don't record the dynamic cost of the subcall, and an underflow occurs while recording the stipend. Fixing that. |
|
Fixed the above and an issue with tracing. |
src/executor/stack/executor.rs
Outdated
| } | ||
|
|
||
| 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.
The error should be propagated, not ignored.
src/executor/stack/executor.rs
Outdated
| context.clone(), | ||
| ) { | ||
| Capture::Exit((s, v)) => (s, v), | ||
| Capture::Trap(_) => unreachable!(), |
There was a problem hiding this comment.
Add a comment here why it's unreachable. (StackExecutor is sync.)
src/executor/stack/executor.rs
Outdated
| @@ -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], | |||
There was a problem hiding this comment.
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.
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.
Hmm also moved the address, but that shouldn't make a difference.
src/executor/stack/executor.rs
Outdated
| 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 |
There was a problem hiding this comment.
| // 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 |
src/executor/stack/executor.rs
Outdated
| // 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(), |
There was a problem hiding this comment.
I think it may be safer to set this to input.len() for now. Call itself indeed copies it.
|
I still have some concerns over the trait design (mostly the |
|
@sorpaas Do you have a plan to release a new version 0.36.0 or 0.36.0-beta that includes this PR ? |
* update PrecompileHandle ref: rust-ethereum/evm#122 * update fee calculation ref: rust-ethereum/evm#132 * add code_size/code_hash fn in StackState trait ref: rust-ethereum/evm#140 * update evm call stack ref: rust-ethereum/evm#136 * update evm call stack ref: rust-ethereum/evm#155 * add shanghai eips 3651, 3855, 3860 ref: rust-ethereum/evm#152 * update is_precompile ref: rust-ethereum/evm#157 * fix eip-3860 ref: rust-ethereum/evm#160 * update runtime config ref: rust-ethereum/evm#161 * add eip-4399 ref: rust-ethereum/evm#162 * fix eip-2618 ref: rust-ethereum/evm#163 * fix nonce back to U256 ref: rust-ethereum/evm#166 * remove exit_substate in create functions ref: rust-ethereum/evm#168 * record external cost ref: rust-ethereum/evm#170 * add record_external_operation ref: rust-ethereum/evm#171 * add storage_growth ref: rust-ethereum/evm#173 * update evm * switch to shanghai hardfork * update ecrecover ref: polkadot-evm/frontier#964 (#2696)
Adds a
PrecompileHandletrait allowing to perform subcalls and register gas and logs from a precompile.Registering gas and logs with the handle provides better integration with gas and logs that could be emitted by the subcalls.
For that reason the
costand logs fields ofPrecompileOutputare removed.