-
Notifications
You must be signed in to change notification settings - Fork 234
fix(levm): fix precompiles problem with eip7702 #2900
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
Changes from all commits
44c124b
6172f79
37350e7
49c6377
32e990f
4109e81
f6e0fea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,7 @@ impl<'a> VM<'a> { | |
| (account.is_empty(), address_was_cold) | ||
| }; | ||
|
|
||
| let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = | ||
| let (is_delegation_7702, eip7702_gas_consumed, code_address, bytecode) = | ||
| eip7702_get_code(self.db, &mut self.substate, callee)?; | ||
|
|
||
| let gas_left = self | ||
|
|
@@ -120,7 +120,7 @@ impl<'a> VM<'a> { | |
| return_data_start_offset, | ||
| return_data_size, | ||
| bytecode, | ||
| is_delegation, | ||
| is_delegation_7702, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -176,7 +176,7 @@ impl<'a> VM<'a> { | |
| let (_account_info, address_was_cold) = | ||
| self.db.access_account(&mut self.substate, code_address)?; | ||
|
|
||
| let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = | ||
| let (is_delegation_7702, eip7702_gas_consumed, code_address, bytecode) = | ||
| eip7702_get_code(self.db, &mut self.substate, code_address)?; | ||
|
|
||
| let gas_left = self | ||
|
|
@@ -220,7 +220,7 @@ impl<'a> VM<'a> { | |
| return_data_start_offset, | ||
| return_data_size, | ||
| bytecode, | ||
| is_delegation, | ||
| is_delegation_7702, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -300,7 +300,7 @@ impl<'a> VM<'a> { | |
| calculate_memory_size(return_data_start_offset, return_data_size)?; | ||
| let new_memory_size = new_memory_size_for_args.max(new_memory_size_for_return_data); | ||
|
|
||
| let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = | ||
| let (is_delegation_7702, eip7702_gas_consumed, code_address, bytecode) = | ||
| eip7702_get_code(self.db, &mut self.substate, code_address)?; | ||
|
|
||
| let gas_left = self | ||
|
|
@@ -344,7 +344,7 @@ impl<'a> VM<'a> { | |
| return_data_start_offset, | ||
| return_data_size, | ||
| bytecode, | ||
| is_delegation, | ||
| is_delegation_7702, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -396,7 +396,7 @@ impl<'a> VM<'a> { | |
| calculate_memory_size(return_data_start_offset, return_data_size)?; | ||
| let new_memory_size = new_memory_size_for_args.max(new_memory_size_for_return_data); | ||
|
|
||
| let (is_delegation, eip7702_gas_consumed, _, bytecode) = | ||
| let (is_delegation_7702, eip7702_gas_consumed, _, bytecode) = | ||
| eip7702_get_code(self.db, &mut self.substate, code_address)?; | ||
|
|
||
| let gas_left = self | ||
|
|
@@ -437,7 +437,7 @@ impl<'a> VM<'a> { | |
| return_data_start_offset, | ||
| return_data_size, | ||
| bytecode, | ||
| is_delegation, | ||
| is_delegation_7702, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -751,7 +751,7 @@ impl<'a> VM<'a> { | |
| ret_offset: U256, | ||
| ret_size: usize, | ||
| bytecode: Bytes, | ||
| is_delegation: bool, | ||
| is_delegation_7702: bool, | ||
| ) -> Result<OpcodeResult, VMError> { | ||
| let sender_balance = self | ||
| .db | ||
|
|
@@ -803,18 +803,6 @@ impl<'a> VM<'a> { | |
| self.increase_account_balance(to, value)?; | ||
| } | ||
|
|
||
| if bytecode.is_empty() && is_delegation { | ||
| self.current_call_frame_mut()?.gas_used = self | ||
| .current_call_frame()? | ||
| .gas_used | ||
| .checked_sub(gas_limit) | ||
| .ok_or(InternalError::GasOverflow)?; | ||
| self.current_call_frame_mut()? | ||
| .stack | ||
| .push(SUCCESS_FOR_CALL)?; | ||
| return Ok(OpcodeResult::Continue { pc_increment: 1 }); | ||
| } | ||
|
|
||
| let new_call_frame = CallFrame::new( | ||
| msg_sender, | ||
| to, | ||
|
|
@@ -832,8 +820,7 @@ impl<'a> VM<'a> { | |
| ); | ||
| self.call_frames.push(new_call_frame); | ||
|
|
||
| if self.is_precompile()? { | ||
| // Execute precompile immediately and handle result. | ||
| if self.is_precompile(&code_address) && !is_delegation_7702 { | ||
|
Contributor
Author
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. This check was added because precompile shouldn't be executed if it's target of delegation. |
||
| let report = self.execute_precompile()?; | ||
| self.handle_return(&report)?; | ||
| } else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ use crate::{ | |
| call_frame::CallFrame, | ||
| db::gen_db::GeneralizedDatabase, | ||
| environment::Environment, | ||
| errors::{ExecutionReport, OpcodeResult, PrecompileError, VMError}, | ||
| errors::{ExecutionReport, OpcodeResult, VMError}, | ||
| hooks::hook::Hook, | ||
| precompiles::execute_precompile, | ||
| TransientStorage, | ||
|
|
@@ -138,7 +138,7 @@ impl<'a> VM<'a> { | |
|
|
||
| /// Main execution loop. | ||
| pub fn run_execution(&mut self) -> Result<ExecutionReport, VMError> { | ||
| if self.is_precompile()? { | ||
| if self.is_precompile(&self.current_call_frame()?.to) { | ||
| return self.execute_precompile(); | ||
| } | ||
|
|
||
|
|
@@ -166,30 +166,17 @@ impl<'a> VM<'a> { | |
| } | ||
| } | ||
|
|
||
| /// Executes precompile and handles the output that it returns, generating a report. | ||
| pub fn execute_precompile(&mut self) -> Result<ExecutionReport, VMError> { | ||
| let precompile_address = self.current_call_frame()?.code_address; | ||
|
|
||
| let precompile_result = match self.is_delegation_target(precompile_address) { | ||
| // Avoid executing precompile if it is target of a delegation in EIP-7702 transaction. | ||
| true => { | ||
| let gas_limit = self.current_call_frame()?.gas_limit; | ||
| if gas_limit == 0 { | ||
| // `pointer_to_precompile.json` tests that it should fail in a call with zero gas limit. | ||
| Err(VMError::PrecompileError(PrecompileError::NotEnoughGas)) | ||
| } else { | ||
| Ok(Bytes::new()) | ||
| } | ||
| } | ||
| // Otherwise, execute precompile | ||
| false => { | ||
| let callframe = self.current_call_frame_mut()?; | ||
| execute_precompile( | ||
| precompile_address, | ||
| &callframe.calldata, | ||
| &mut callframe.gas_used, | ||
| callframe.gas_limit, | ||
|
Comment on lines
-172
to
-190
Contributor
Author
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. We don't need these checks anymore because it's enough with the checks that are being performed in XCALL opcodes. Before we were making checks in external transactions too, which wasn't necessary at all. |
||
| ) | ||
| } | ||
| let callframe = self.current_call_frame_mut()?; | ||
|
|
||
| let precompile_result = { | ||
| execute_precompile( | ||
| callframe.code_address, | ||
| &callframe.calldata, | ||
| &mut callframe.gas_used, | ||
| callframe.gas_limit, | ||
| ) | ||
| }; | ||
|
|
||
| let report = self.handle_precompile_result(precompile_result)?; | ||
|
|
||
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.
This was removed because it was just an early return. But the result it the same if the callframe is created and the bytecode is empty. I just thought that it was best to remove it for clarity and say that we execute empty code.