From 1abfba89ba3df61d38aa3ba707f47143ce0a7763 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Thu, 21 Nov 2024 16:51:16 -0300 Subject: [PATCH 1/7] start restructuring transact and validate_transaction --- crates/vm/levm/docs/validations.md | 17 ++++++++++++ crates/vm/levm/src/db.rs | 4 +++ crates/vm/levm/src/vm.rs | 44 ++++++++++-------------------- 3 files changed, 36 insertions(+), 29 deletions(-) create mode 100644 crates/vm/levm/docs/validations.md diff --git a/crates/vm/levm/docs/validations.md b/crates/vm/levm/docs/validations.md new file mode 100644 index 000000000..95c1ca44f --- /dev/null +++ b/crates/vm/levm/docs/validations.md @@ -0,0 +1,17 @@ +## Transaction Validation + +1. **GASLIMIT_PRICE_PRODUCT_OVERFLOW** -> The product of gas limit and gas price is too high. +2. **INSUFFICIENT_ACCOUNT_FUNDS** -> Sender does not have enough funds to pay for the gas. +3. **INSUFFICIENT_MAX_FEE_PER_GAS** -> The max fee per gas is lower than the base fee per gas. +4. **INITCODE_SIZE_EXCEEDED** -> The size of the initcode is too big. +5. **INTRINSIC_GAS_TOO_LOW** -> The gas limit is lower than the intrinsic gas. +6. **NONCE_IS_MAX** -> The nonce of the sender is at its maximum value. +7. **PRIORITY_GREATER_THAN_MAX_FEE_PER_GAS** -> The priority fee is greater than the max fee per gas. +8. **SENDER_NOT_EOA** -> The sender is not an EOA (it has code). +9. **GAS_ALLOWANCE_EXCEEDED** -> The gas limit is higher than the block gas limit. +10. **INSUFFICIENT_MAX_FEE_PER_BLOB_GAS** -> The max fee per blob gas is lower than the base fee per gas. +11. **TYPE_3_TX_ZERO_BLOBS** -> The transaction has zero blobs. +12. **TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH** -> The blob versioned hash is invalid. +13. **TYPE_3_TX_PRE_FORK** -> The transaction is a pre-cancun transaction. +14. **TYPE_3_TX_BLOB_COUNT_EXCEEDED** -> The blob count is higher than the max allowed. +15. **TYPE_3_TX_CONTRACT_CREATION** -> The type 3 transaction is a contract creation. diff --git a/crates/vm/levm/src/db.rs b/crates/vm/levm/src/db.rs index d9abe7681..b544b378e 100644 --- a/crates/vm/levm/src/db.rs +++ b/crates/vm/levm/src/db.rs @@ -96,6 +96,10 @@ impl Cache { self.accounts.insert(*address, account.clone()); } + pub fn remove_account(&mut self, address: &Address) { + self.accounts.remove(address); + } + pub fn write_account_storage( &mut self, address: &Address, diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index fc5a6c6bd..03e90cd56 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -338,24 +338,12 @@ impl VM { /// ## Description /// This method performs validations and returns an error if any of the validations fail. - /// The **only** change it makes to the state is incrementing the nonce of the sender by 1. - /// ## The validations are: - /// 1. **GASLIMIT_PRICE_PRODUCT_OVERFLOW** -> The product of gas limit and gas price is too high. - /// 2. **INSUFFICIENT_ACCOUNT_FUNDS** -> Sender does not have enough funds to pay for the gas. - /// 3. **INSUFFICIENT_MAX_FEE_PER_GAS** -> The max fee per gas is lower than the base fee per gas. - /// 4. **INITCODE_SIZE_EXCEEDED** -> The size of the initcode is too big. - /// 5. **INTRINSIC_GAS_TOO_LOW** -> The gas limit is lower than the intrinsic gas. - /// 6. **NONCE_IS_MAX** -> The nonce of the sender is at its maximum value. - /// 7. **PRIORITY_GREATER_THAN_MAX_FEE_PER_GAS** -> The priority fee is greater than the max fee per gas. - /// 8. **SENDER_NOT_EOA** -> The sender is not an EOA (it has code). - /// 9. **GAS_ALLOWANCE_EXCEEDED** -> The gas limit is higher than the block gas limit. - /// 10. **INSUFFICIENT_MAX_FEE_PER_BLOB_GAS** -> The max fee per blob gas is lower than the base fee per gas. - /// 11. **TYPE_3_TX_ZERO_BLOBS** -> The transaction has zero blobs. - /// 12. **TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH** -> The blob versioned hash is invalid. - /// 13. **TYPE_3_TX_PRE_FORK** -> The transaction is a pre-cancun transaction. - /// 14. **TYPE_3_TX_BLOB_COUNT_EXCEEDED** -> The blob count is higher than the max allowed. - /// 15. **TYPE_3_TX_CONTRACT_CREATION** -> The type 3 transaction is a contract creation. - fn validate_transaction(&mut self, initial_call_frame: &CallFrame) -> Result<(), VMError> { + /// It also makes initial changes alongside the validations: + /// - It increases sender nonce + /// - It substracts up-front-cost from sender balance. + /// - It calculates and adds intrinsic gas to the 'gas used' of callframe and environment. + /// See 'docs' for more information about validations. + fn validate_transaction(&mut self, initial_call_frame: &mut CallFrame) -> Result<(), VMError> { //TODO: This should revert the transaction, not throw an error. And I don't know if it should be done here... // if self.is_create() { // // If address is already in db, there's an error @@ -379,9 +367,11 @@ impl VM { .ok_or(VMError::InsufficientAccountFunds)?; // (2) INSUFFICIENT_ACCOUNT_FUNDS - if sender_account.info.balance < up_front_cost { - return Err(VMError::InsufficientAccountFunds); - } + sender_account.info.balance = sender_account + .info + .balance + .checked_sub(up_front_cost) + .ok_or(VMError::InsufficientAccountFunds)?; // (3) INSUFFICIENT_MAX_FEE_PER_GAS if self.gas_price_or_max_fee_per_gas() < self.env.base_fee_per_gas { @@ -397,11 +387,7 @@ impl VM { } // (5) INTRINSIC_GAS_TOO_LOW - // Intrinsic gas is gas used by the callframe before execution of opcodes - let intrinsic_gas = initial_call_frame.gas_used; - if self.env.gas_limit < intrinsic_gas { - return Err(VMError::IntrinsicGasTooLow); - } + self.add_intrinsic_gas(initial_call_frame)?; // (6) NONCE_IS_MAX sender_account.info.nonce = sender_account @@ -706,9 +692,9 @@ impl VM { .pop() .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?; - self.add_intrinsic_gas(&mut current_call_frame)?; - - self.validate_transaction(¤t_call_frame)?; // Fail without consuming gas + // Validates transaction and performs pre-exeuction changes. + // Errors don't revert execution, they are propagated because transaction is invalid. + self.validate_transaction(&mut current_call_frame)?; let mut report = self.execute(&mut current_call_frame)?; From 017201a846a28b65bb06aee2f3035d3ddd76ea2d Mon Sep 17 00:00:00 2001 From: JereSalo Date: Thu, 21 Nov 2024 18:06:04 -0300 Subject: [PATCH 2/7] make big refactor to transact and implement revert functionality in it --- crates/vm/levm/src/errors.rs | 4 + crates/vm/levm/src/vm.rs | 141 +++++++++++++++++++---------------- 2 files changed, 82 insertions(+), 63 deletions(-) diff --git a/crates/vm/levm/src/errors.rs b/crates/vm/levm/src/errors.rs index 510ea2c99..1646a662f 100644 --- a/crates/vm/levm/src/errors.rs +++ b/crates/vm/levm/src/errors.rs @@ -87,6 +87,8 @@ pub enum VMError { Type3TxBlobCountExceeded, #[error("Type3TxContractCreation")] Type3TxContractCreation, + #[error("Revert Create")] + RevertCreate, // OutOfGas #[error("Out Of Gas")] OutOfGas(#[from] OutOfGasError), @@ -153,6 +155,8 @@ pub enum InternalError { UtilsError, #[error("Overflow error")] OperationOverflow, + #[error("Undefined state")] + UndefinedState, // This error is temporarily for things that cause an undefined state. } impl VMError { diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 03e90cd56..cdf5552b4 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -12,6 +12,7 @@ use crate::{ opcodes::Opcode, }; use bytes::Bytes; +use core::error; use ethrex_core::{types::TxKind, Address, H256, U256}; use ethrex_rlp; use ethrex_rlp::encode::RLPEncode; @@ -501,37 +502,18 @@ impl VM { // Ok(()) // } - pub fn post_execution_changes( + pub fn create_post_execution( &mut self, + initial_call_frame: &mut CallFrame, report: &mut TransactionReport, ) -> Result<(), VMError> { - let initial_call_frame = self - .call_frames - .last() - .ok_or(VMError::Internal( - InternalError::CouldNotAccessLastCallframe, - ))? - .clone(); - - let sender = initial_call_frame.msg_sender; - if self.is_create() { - // If create should check if transaction failed. If failed should revert (delete created contract, ) - // if let TxResult::Revert(error) = report.result { - // self.revert_create()?; - // return Err(error); - // } - // Revert behavior is actually done at the end of execute, it reverts the cache to the state before execution. - // TODO: Think about the behavior when reverting a transaction. What changes to the state? The sender has to lose the gas used and coinbase gets the priority fee, nothing else has to change. + if let TxResult::Revert(error) = &report.result { + return Err(error.clone()); + } let contract_code = report.clone().output; - // // TODO: Is this the expected behavior? - // if contract_code.is_empty() { - // return Err(VMError::InvalidBytecode); - // } - // I think you are able to create a contract without bytecode. - // (6) if contract_code.len() > MAX_CODE_SIZE { return Err(VMError::ContractOutputTooBig); @@ -544,8 +526,7 @@ impl VM { let max_gas = self.env.gas_limit.low_u64(); - // If the initialization code completes successfully, a final contract-creation cost is paid, - // the code-deposit cost, c, proportional to the size of the created contract’s code + // If initialization code is successful, code-deposit cost is paid. let code_length: u64 = contract_code .len() .try_into() @@ -555,74 +536,98 @@ impl VM { .ok_or(VMError::Internal(InternalError::OperationOverflow))?; report.add_gas_with_max(code_deposit_cost, max_gas)?; - // Charge 22100 gas for each storage variable set + // Charge 22100 gas for each storage variable set (???) + // Assign bytecode to the new contract let contract_address = initial_call_frame.to; - let mut created_contract = self.get_account(&contract_address); created_contract.info.bytecode = contract_code; self.cache.add_account(&contract_address, &created_contract); } + Ok(()) + } - let mut sender_account = self.get_account(&sender); - let coinbase_address = self.env.coinbase; + /// ## Description + /// Changes to the state post-execution, if this fails we won't be able to determine a final state. + /// Errors here are mostly related to balance overflows. + /// TODO: Determine the right behavior when a balance overflow occurs. + /// 1. Refund unused gas to sender + /// 2. Transfer value to recipient or return value to sender depending on execution result. + /// 3. Pay priority fee to coinbase + pub fn post_execution_changes( + &mut self, + initial_call_frame: &CallFrame, + report: &TransactionReport, + ) -> Result<(), VMError> { + // 1. Refund unused gas to sender + let sender_address = self.env.origin; + let mut sender_account = self.get_account(&sender_address); + + let max_gas = self.env.gas_limit.low_u64(); + let consumed_gas = report.gas_used; + let refunded_gas = report.gas_refunded; + + let refundable_gas = max_gas - consumed_gas + refunded_gas; + + let refund_amount = self + .env + .gas_price + .checked_mul(U256::from(refundable_gas)) + .ok_or(VMError::Internal(InternalError::UndefinedState))?; sender_account.info.balance = sender_account .info .balance - .checked_sub( - U256::from(report.gas_used) - .checked_mul(self.env.gas_price) - .ok_or(VMError::GasLimitPriceProductOverflow)?, // TODO: Wrong error GasLimitPriceProductOverflow - ) - .ok_or(VMError::BalanceUnderflow)?; + .checked_add(refund_amount) + .ok_or(VMError::Internal(InternalError::UndefinedState))?; + + // 2. Transfer value to recipient or return value to sender depending on execution result. + let recipient_address = initial_call_frame.to; + let mut recipient_account = self.get_account(&recipient_address); - let receiver_address = initial_call_frame.to; - let mut receiver_account = self.get_account(&receiver_address); - // If execution was successful we want to transfer value from sender to receiver if report.is_success() { - // Subtract to the caller the gas sent - sender_account.info.balance = sender_account + // transfer value to recipient + recipient_account.info.balance = recipient_account .info .balance - .checked_sub(initial_call_frame.msg_value) - .ok_or(VMError::BalanceUnderflow)?; - receiver_account.info.balance = receiver_account + .checked_add(initial_call_frame.msg_value) + .ok_or(VMError::Internal(InternalError::UndefinedState))?; + } else { + // return value to sender + sender_account.info.balance = sender_account .info .balance .checked_add(initial_call_frame.msg_value) - .ok_or(VMError::BalanceUnderflow)?; + .ok_or(VMError::Internal(InternalError::UndefinedState))?; } - // Note: This is commented because it's used for debugging purposes in development. - // dbg!(&report.gas_refunded); - - self.cache.add_account(&sender, &sender_account); - self.cache.add_account(&receiver_address, &receiver_account); + self.cache.add_account(&sender_address, &sender_account); + self.cache + .add_account(&recipient_address, &recipient_account); - // Send coinbase fee + // 3. Pay priority fee to coinbase + let coinbase_address = self.env.coinbase; let priority_fee_per_gas = self .env .gas_price .checked_sub(self.env.base_fee_per_gas) - .ok_or(VMError::GasPriceIsLowerThanBaseFee)?; - let coinbase_fee = (U256::from(report.gas_used)) + .ok_or(VMError::Internal(InternalError::UndefinedState))?; + let coinbase_fee = (U256::from(consumed_gas)) .checked_mul(priority_fee_per_gas) - .ok_or(VMError::BalanceOverflow)?; + .ok_or(VMError::Internal(InternalError::UndefinedState))?; let mut coinbase_account = self.get_account(&coinbase_address); + coinbase_account.info.balance = coinbase_account .info .balance .checked_add(coinbase_fee) - .ok_or(VMError::BalanceOverflow)?; + .ok_or(VMError::Internal(InternalError::UndefinedState))?; self.cache.add_account(&coinbase_address, &coinbase_account); - report.new_state.clone_from(&self.cache.accounts); - Ok(()) } @@ -687,27 +692,37 @@ impl VM { } pub fn transact(&mut self) -> Result { - let mut current_call_frame = self + let mut initial_call_frame = self .call_frames .pop() .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?; // Validates transaction and performs pre-exeuction changes. // Errors don't revert execution, they are propagated because transaction is invalid. - self.validate_transaction(&mut current_call_frame)?; + self.validate_transaction(&mut initial_call_frame)?; - let mut report = self.execute(&mut current_call_frame)?; + // We need cache before execution because if a create transaction reverts in post-execution then we need to revert whatever the init-code did. + let cache_before_execution = self.cache.clone(); + let mut report = self.execute(&mut initial_call_frame)?; - match self.post_execution_changes(&mut report) { + // Validation of create transaction post execution + // If it fails, it reverts contract creation. + match self.create_post_execution(&mut initial_call_frame, &mut report) { Ok(_) => {} Err(error) => { if error.is_internal() { return Err(error); - } else if report.result == TxResult::Success { + } else { report.result = TxResult::Revert(error); + self.cache = cache_before_execution; + self.cache.accounts.remove(&initial_call_frame.to); } } - } + }; + + self.post_execution_changes(&initial_call_frame, &report)?; + + report.new_state.clone_from(&self.cache.accounts); Ok(report) } From 5de8312bf02c8b08f1b868d120fdafaf97ae5173 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Thu, 21 Nov 2024 18:48:53 -0300 Subject: [PATCH 3/7] change UndefinedState for debugging --- cmd/ef_tests/levm/runner.rs | 26 +++++++++++++------------- crates/vm/levm/src/errors.rs | 2 +- crates/vm/levm/src/vm.rs | 14 +++++++------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/cmd/ef_tests/levm/runner.rs b/cmd/ef_tests/levm/runner.rs index 49afa45b0..2fd7bb13e 100644 --- a/cmd/ef_tests/levm/runner.rs +++ b/cmd/ef_tests/levm/runner.rs @@ -46,19 +46,19 @@ pub fn run_ef_tests() -> Result> { continue; } // 'If' for running a specific test when necessary. - // if test - // .path() - // .file_name() - // .is_some_and(|name| name == "buffer.json") - // { - let test_result = run_ef_test( - serde_json::from_reader(std::fs::File::open(test.path())?)?, - &mut report, - ); - if test_result.is_err() { - continue; + if test + .path() + .file_name() + .is_some_and(|name| name == "valCausesOOF.json") + { + let test_result = run_ef_test( + serde_json::from_reader(std::fs::File::open(test.path())?)?, + &mut report, + ); + if test_result.is_err() { + continue; + } } - // } } spinner.update_text(report.progress()); } @@ -76,7 +76,7 @@ pub fn run_ef_test_tx( let mut evm = prepare_vm_for_tx(tx_id, test)?; ensure_pre_state(&evm, test)?; let execution_result = evm.transact(); - // ensure_post_state(execution_result, test, report, tx_id)?; + ensure_post_state(execution_result, test, report, tx_id)?; Ok(()) } diff --git a/crates/vm/levm/src/errors.rs b/crates/vm/levm/src/errors.rs index 1646a662f..ea30f1f43 100644 --- a/crates/vm/levm/src/errors.rs +++ b/crates/vm/levm/src/errors.rs @@ -156,7 +156,7 @@ pub enum InternalError { #[error("Overflow error")] OperationOverflow, #[error("Undefined state")] - UndefinedState, // This error is temporarily for things that cause an undefined state. + UndefinedState(i32), // This error is temporarily for things that cause an undefined state. } impl VMError { diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index cdf5552b4..88399d8be 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -575,13 +575,13 @@ impl VM { .env .gas_price .checked_mul(U256::from(refundable_gas)) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(1)))?; sender_account.info.balance = sender_account .info .balance .checked_add(refund_amount) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(2)))?; // 2. Transfer value to recipient or return value to sender depending on execution result. let recipient_address = initial_call_frame.to; @@ -593,14 +593,14 @@ impl VM { .info .balance .checked_add(initial_call_frame.msg_value) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(3)))?; } else { // return value to sender sender_account.info.balance = sender_account .info .balance .checked_add(initial_call_frame.msg_value) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(4)))?; } self.cache.add_account(&sender_address, &sender_account); @@ -613,10 +613,10 @@ impl VM { .env .gas_price .checked_sub(self.env.base_fee_per_gas) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(5)))?; let coinbase_fee = (U256::from(consumed_gas)) .checked_mul(priority_fee_per_gas) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(6)))?; let mut coinbase_account = self.get_account(&coinbase_address); @@ -624,7 +624,7 @@ impl VM { .info .balance .checked_add(coinbase_fee) - .ok_or(VMError::Internal(InternalError::UndefinedState))?; + .ok_or(VMError::Internal(InternalError::UndefinedState(7)))?; self.cache.add_account(&coinbase_address, &coinbase_account); From 1c286647dd1b6d84c0b358df2f2a9730805510e7 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Fri, 22 Nov 2024 09:06:16 -0300 Subject: [PATCH 4/7] implement effective gas price calculation for type 2 transactions --- cmd/ef_tests/levm/runner.rs | 26 +++++++++++++------------- crates/vm/levm/src/environment.rs | 1 + crates/vm/levm/src/vm.rs | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/cmd/ef_tests/levm/runner.rs b/cmd/ef_tests/levm/runner.rs index 2fd7bb13e..ce42a13c9 100644 --- a/cmd/ef_tests/levm/runner.rs +++ b/cmd/ef_tests/levm/runner.rs @@ -46,19 +46,19 @@ pub fn run_ef_tests() -> Result> { continue; } // 'If' for running a specific test when necessary. - if test - .path() - .file_name() - .is_some_and(|name| name == "valCausesOOF.json") - { - let test_result = run_ef_test( - serde_json::from_reader(std::fs::File::open(test.path())?)?, - &mut report, - ); - if test_result.is_err() { - continue; - } + // if test + // .path() + // .file_name() + // .is_some_and(|name| name == "valCausesOOF.json") + // { + let test_result = run_ef_test( + serde_json::from_reader(std::fs::File::open(test.path())?)?, + &mut report, + ); + if test_result.is_err() { + continue; } + // } } spinner.update_text(report.progress()); } @@ -76,7 +76,7 @@ pub fn run_ef_test_tx( let mut evm = prepare_vm_for_tx(tx_id, test)?; ensure_pre_state(&evm, test)?; let execution_result = evm.transact(); - ensure_post_state(execution_result, test, report, tx_id)?; + // ensure_post_state(execution_result, test, report, tx_id)?; Ok(()) } diff --git a/crates/vm/levm/src/environment.rs b/crates/vm/levm/src/environment.rs index d0c3b1c35..1617f3121 100644 --- a/crates/vm/levm/src/environment.rs +++ b/crates/vm/levm/src/environment.rs @@ -14,6 +14,7 @@ pub struct Environment { pub prev_randao: Option, pub chain_id: U256, pub base_fee_per_gas: U256, + // It should store effective gas price, in type 2 transactions it is not defined but we calculate if with max fee per gas and max priority fee per gas. pub gas_price: U256, pub block_excess_blob_gas: Option, pub block_blob_gas_used: Option, diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 88399d8be..3dedf7511 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -502,6 +502,20 @@ impl VM { // Ok(()) // } + /// Calculates effective gas price and updates variable gas_price in env. + pub fn update_effective_gas_price(&mut self) -> Result<(), VMError> { + if let (Some(max_priority_fee_per_gas), Some(max_fee_per_gas)) = ( + self.env.tx_max_priority_fee_per_gas, + self.env.tx_max_fee_per_gas, + ) { + // Effective Gas Price = Base Fee + min(maxPriorityFeePerGas, maxFeePerGas - Base Fee) + self.env.gas_price = self.env.base_fee_per_gas + + max_priority_fee_per_gas.min(max_fee_per_gas - self.env.base_fee_per_gas); + // TODO: Implement error in operations here + }; + Ok(()) + } + pub fn create_post_execution( &mut self, initial_call_frame: &mut CallFrame, @@ -701,6 +715,8 @@ impl VM { // Errors don't revert execution, they are propagated because transaction is invalid. self.validate_transaction(&mut initial_call_frame)?; + self.update_effective_gas_price()?; // This is for gas_price to be a reasonable value + // We need cache before execution because if a create transaction reverts in post-execution then we need to revert whatever the init-code did. let cache_before_execution = self.cache.clone(); let mut report = self.execute(&mut initial_call_frame)?; From 6a5d4e8ce5c18b43e30bf990138df5f8bec02d76 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Fri, 22 Nov 2024 09:43:48 -0300 Subject: [PATCH 5/7] make operations safe and add limit to gas refunds --- crates/vm/levm/src/vm.rs | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 3dedf7511..52f61f811 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -12,7 +12,6 @@ use crate::{ opcodes::Opcode, }; use bytes::Bytes; -use core::error; use ethrex_core::{types::TxKind, Address, H256, U256}; use ethrex_rlp; use ethrex_rlp::encode::RLPEncode; @@ -509,9 +508,20 @@ impl VM { self.env.tx_max_fee_per_gas, ) { // Effective Gas Price = Base Fee + min(maxPriorityFeePerGas, maxFeePerGas - Base Fee) - self.env.gas_price = self.env.base_fee_per_gas - + max_priority_fee_per_gas.min(max_fee_per_gas - self.env.base_fee_per_gas); - // TODO: Implement error in operations here + self.env.gas_price = self + .env + .base_fee_per_gas + .checked_add( + max_priority_fee_per_gas.min( + max_fee_per_gas + .checked_sub(self.env.base_fee_per_gas) + .ok_or(VMError::InsufficientMaxFeePerGas)?, + ), + ) + .ok_or(VMError::Internal( + InternalError::ArithmeticOperationOverflow, + ))?; + // TODO: See what to return if base fee + priority fee causes overflow. }; Ok(()) } @@ -581,20 +591,28 @@ impl VM { let max_gas = self.env.gas_limit.low_u64(); let consumed_gas = report.gas_used; - let refunded_gas = report.gas_refunded; + let refunded_gas = report.gas_refunded.min( + consumed_gas + .checked_div(5) + .ok_or(VMError::Internal(InternalError::UndefinedState(-1)))?, + ); + // "The max refundable proportion of gas was reduced from one half to one fifth by EIP-3529 by Buterin and Swende [2021] in the London release" - let refundable_gas = max_gas - consumed_gas + refunded_gas; + let gas_to_return = max_gas + .checked_sub(consumed_gas) + .and_then(|gas| gas.checked_add(refunded_gas)) + .ok_or(VMError::Internal(InternalError::UndefinedState(0)))?; - let refund_amount = self + let gas_return_amount = self .env .gas_price - .checked_mul(U256::from(refundable_gas)) + .checked_mul(U256::from(gas_to_return)) .ok_or(VMError::Internal(InternalError::UndefinedState(1)))?; sender_account.info.balance = sender_account .info .balance - .checked_add(refund_amount) + .checked_add(gas_return_amount) .ok_or(VMError::Internal(InternalError::UndefinedState(2)))?; // 2. Transfer value to recipient or return value to sender depending on execution result. From 17ec67a88463aa42466db0b579e90260f3d6dcff Mon Sep 17 00:00:00 2001 From: JereSalo Date: Fri, 22 Nov 2024 10:01:10 -0300 Subject: [PATCH 6/7] make some changes in case of reversion of create --- crates/vm/levm/src/vm.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 52f61f811..f5010b271 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -748,6 +748,8 @@ impl VM { return Err(error); } else { report.result = TxResult::Revert(error); + report.gas_refunded = 0; + report.gas_used = self.env.gas_limit.low_u64(); self.cache = cache_before_execution; self.cache.accounts.remove(&initial_call_frame.to); } From 0d972928a831d258121a52ad1d127ff8491935e5 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Fri, 22 Nov 2024 15:12:00 -0300 Subject: [PATCH 7/7] fix cippy lint --- crates/vm/levm/src/vm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index f5010b271..450611c8c 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -342,7 +342,7 @@ impl VM { /// - It increases sender nonce /// - It substracts up-front-cost from sender balance. /// - It calculates and adds intrinsic gas to the 'gas used' of callframe and environment. - /// See 'docs' for more information about validations. + /// See 'docs' for more information about validations. fn validate_transaction(&mut self, initial_call_frame: &mut CallFrame) -> Result<(), VMError> { //TODO: This should revert the transaction, not throw an error. And I don't know if it should be done here... // if self.is_create() {