From c6acab4083b16bf635fa7fa1f7853f63d4d284f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jerem=C3=ADas=20Salom=C3=B3n?= <48994069+JereSalo@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:24:18 -0300 Subject: [PATCH] feat(levm): fix validation errors (part 2) and some extra things (#1345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Motivation** The main motivation is to fix validation related tests. Return a validation exception everytime that is necessary (while being careful of not returning them in cases in which they are not expected). It also fixes/changes some things that I find along the way and I consider appropriate changing. **Description** - Adds function `add_intrinsic_gas()` - Uses our gas_price as an effective gas price. So it works for transactions Type 2 and 3. - Implements blob gas cost - Implements calculation of base fee per blob gas and it's associated validation. - Fixes `add_gas_with_max` for report - It implements somewhat of a revert for create, but I'm not sure if it is well done because cache's purpose has recently changed. Diffs for `add_intrinsic_gas` and `validate_transaction` don't look very good. I recommend to watch the code directly for those functions. **End Result** ```bash ✓ Ethereum Foundation Tests: 1466 passed 2635 failed 4101 total run - 39:12 ✓ Summary: 1466/4101 (35.75) Cancun: 1356/3578 (37.90%) Shanghai: 55/221 (24.89%) Homestead: 0/17 (0.00%) Istanbul: 0/34 (0.00%) London: 19/39 (48.72%) Byzantium: 0/33 (0.00%) Berlin: 17/35 (48.57%) Constantinople: 0/66 (0.00%) Merge: 19/62 (30.65%) Frontier: 0/16 (0.00%) ``` After mergin main into branch: ``` Summary: 1471/4101 (35.87) Cancun: 1361/3578 (38.04%) ``` Closes #issue_number --------- Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> --- cmd/ef_tests/levm/runner/levm_runner.rs | 4 +- cmd/ef_tests/levm/runner/revm_runner.rs | 21 +- cmd/ef_tests/levm/utils.rs | 40 +++- crates/vm/levm/docs/validations.md | 17 ++ crates/vm/levm/src/constants.rs | 4 +- crates/vm/levm/src/errors.rs | 14 +- crates/vm/levm/src/gas_cost.rs | 55 ++++- crates/vm/levm/src/vm.rs | 284 ++++++++++++++---------- crates/vm/vm.rs | 2 +- 9 files changed, 294 insertions(+), 147 deletions(-) create mode 100644 crates/vm/levm/docs/validations.md diff --git a/cmd/ef_tests/levm/runner/levm_runner.rs b/cmd/ef_tests/levm/runner/levm_runner.rs index 5634e7fa7c..63537d8afd 100644 --- a/cmd/ef_tests/levm/runner/levm_runner.rs +++ b/cmd/ef_tests/levm/runner/levm_runner.rs @@ -2,7 +2,7 @@ use crate::{ report::{EFTestReport, TestVector}, runner::{EFTestRunnerError, InternalError}, types::EFTest, - utils, + utils::{self, effective_gas_price}, }; use ethrex_core::{ types::{code_hash, AccountInfo}, @@ -94,7 +94,7 @@ pub fn prepare_vm_for_tx(vector: &TestVector, test: &EFTest) -> Result U256 { - match tx.gas_price { - None => { - let current_base_fee = test.env.current_base_fee.unwrap(); - let priority_fee = tx.max_priority_fee_per_gas.unwrap(); - let max_fee_per_gas = tx.max_fee_per_gas.unwrap(); - std::cmp::min(max_fee_per_gas, current_base_fee + priority_fee) - } - Some(price) => price, - } -} - pub fn prepare_revm_for_tx<'state>( initial_state: &'state mut EvmState, vector: &TestVector, @@ -148,7 +135,7 @@ pub fn prepare_revm_for_tx<'state>( let tx_env = RevmTxEnv { caller: tx.sender.0.into(), gas_limit: tx.gas_limit.as_u64(), - gas_price: RevmU256::from_limbs(effective_gas_price(test, tx).0), + gas_price: RevmU256::from_limbs(effective_gas_price(test, tx)?.0), transact_to: match tx.to { TxKind::Call(to) => RevmTxKind::Call(to.0.into()), TxKind::Create => RevmTxKind::Create, diff --git a/cmd/ef_tests/levm/utils.rs b/cmd/ef_tests/levm/utils.rs index b09944b932..d20ce428de 100644 --- a/cmd/ef_tests/levm/utils.rs +++ b/cmd/ef_tests/levm/utils.rs @@ -1,5 +1,8 @@ -use crate::types::EFTest; -use ethrex_core::{types::Genesis, H256}; +use crate::{ + runner::{EFTestRunnerError, InternalError}, + types::{EFTest, EFTestTransaction}, +}; +use ethrex_core::{types::Genesis, H256, U256}; use ethrex_storage::{EngineType, Store}; use ethrex_vm::{evm_state, EvmState}; @@ -17,3 +20,36 @@ pub fn load_initial_state(test: &EFTest) -> (EvmState, H256) { genesis.get_block().header.compute_block_hash(), ) } + +// If gas price is not provided, calculate it with current base fee and priority fee +pub fn effective_gas_price( + test: &EFTest, + tx: &&EFTestTransaction, +) -> Result { + match tx.gas_price { + None => { + let current_base_fee = test + .env + .current_base_fee + .ok_or(EFTestRunnerError::Internal( + InternalError::FirstRunInternal("current_base_fee not found".to_string()), + ))?; + let priority_fee = tx + .max_priority_fee_per_gas + .ok_or(EFTestRunnerError::Internal( + InternalError::FirstRunInternal( + "max_priority_fee_per_gas not found".to_string(), + ), + ))?; + let max_fee_per_gas = tx.max_fee_per_gas.ok_or(EFTestRunnerError::Internal( + InternalError::FirstRunInternal("max_fee_per_gas not found".to_string()), + ))?; + + Ok(std::cmp::min( + max_fee_per_gas, + current_base_fee + priority_fee, + )) + } + Some(price) => Ok(price), + } +} diff --git a/crates/vm/levm/docs/validations.md b/crates/vm/levm/docs/validations.md new file mode 100644 index 0000000000..85b2033dff --- /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/constants.rs b/crates/vm/levm/src/constants.rs index 8fdc24d2da..1f5511e521 100644 --- a/crates/vm/levm/src/constants.rs +++ b/crates/vm/levm/src/constants.rs @@ -43,8 +43,8 @@ pub const MAX_BLOB_NUMBER_PER_BLOCK: usize = 6; // Blob constants pub const TARGET_BLOB_GAS_PER_BLOCK: U256 = U256([393216, 0, 0, 0]); // TARGET_BLOB_NUMBER_PER_BLOCK * GAS_PER_BLOB -pub const MIN_BASE_FEE_PER_BLOB_GAS: U256 = U256([1, 0, 0, 0]); -pub const BLOB_BASE_FEE_UPDATE_FRACTION: U256 = U256([3338477, 0, 0, 0]); +pub const MIN_BASE_FEE_PER_BLOB_GAS: u64 = 1; +pub const BLOB_BASE_FEE_UPDATE_FRACTION: u64 = 3338477; pub const MAX_BLOB_COUNT: usize = 6; pub const VALID_BLOB_PREFIXES: [u8; 2] = [0x01, 0x02]; diff --git a/crates/vm/levm/src/errors.rs b/crates/vm/levm/src/errors.rs index c75e1e746d..c56a3f428b 100644 --- a/crates/vm/levm/src/errors.rs +++ b/crates/vm/levm/src/errors.rs @@ -213,8 +213,18 @@ pub struct TransactionReport { impl TransactionReport { /// Function to add gas to report without exceeding the maximum gas limit - pub fn add_gas_with_max(&mut self, gas: u64, max: u64) { - self.gas_used = self.gas_used.saturating_add(gas).min(max); + pub fn add_gas_with_max(&mut self, gas: u64, max: u64) -> Result<(), VMError> { + let new_gas_used = self + .gas_used + .checked_add(gas) + .ok_or(OutOfGasError::MaxGasLimitExceeded)?; + + if new_gas_used > max { + return Err(VMError::OutOfGas(OutOfGasError::MaxGasLimitExceeded)); + } + + self.gas_used = new_gas_used; + Ok(()) } pub fn is_success(&self) -> bool { diff --git a/crates/vm/levm/src/gas_cost.rs b/crates/vm/levm/src/gas_cost.rs index 4d96bd5704..0830c3f5d6 100644 --- a/crates/vm/levm/src/gas_cost.rs +++ b/crates/vm/levm/src/gas_cost.rs @@ -151,6 +151,13 @@ pub const INIT_CODE_WORD_COST: U256 = U256([2, 0, 0, 0]); pub const CODE_DEPOSIT_COST: U256 = U256([200, 0, 0, 0]); pub const CREATE_BASE_COST: U256 = U256([32000, 0, 0, 0]); +// Calldata costs +pub const CALLDATA_COST_ZERO_BYTE: U256 = U256([4, 0, 0, 0]); +pub const CALLDATA_COST_NON_ZERO_BYTE: U256 = U256([16, 0, 0, 0]); + +// Blob gas costs +pub const BLOB_GAS_PER_BLOB: U256 = U256([131072, 0, 0, 0]); + pub fn exp(exponent_bits: u64) -> Result { let exponent_byte_size = (exponent_bits .checked_add(7) @@ -462,18 +469,18 @@ pub fn selfdestruct(address_was_cold: bool, account_is_empty: bool) -> Result Result { +pub fn tx_calldata(calldata: &Bytes) -> Result { // This cost applies both for call and create // 4 gas for each zero byte in the transaction data 16 gas for each non-zero byte in the transaction. - let mut calldata_cost: u64 = 0; + let mut calldata_cost: U256 = U256::zero(); for byte in calldata { if *byte != 0 { calldata_cost = calldata_cost - .checked_add(16) + .checked_add(CALLDATA_COST_NON_ZERO_BYTE) .ok_or(OutOfGasError::GasUsedOverflow)?; } else { calldata_cost = calldata_cost - .checked_add(4) + .checked_add(CALLDATA_COST_ZERO_BYTE) .ok_or(OutOfGasError::GasUsedOverflow)?; } } @@ -720,3 +727,43 @@ pub fn staticcall( .checked_add(dynamic_gas) .ok_or(OutOfGasError::GasCostOverflow)?) } + +pub fn fake_exponential(factor: u64, numerator: u64, denominator: u64) -> Result { + let mut i = 1; + let mut output: u64 = 0; + + // Initial multiplication: factor * denominator + let mut numerator_accum = factor + .checked_mul(denominator) + .ok_or(InternalError::ArithmeticOperationOverflow)?; + + while numerator_accum > 0 { + // Safe addition to output + output = output + .checked_add(numerator_accum) + .ok_or(InternalError::ArithmeticOperationOverflow)?; + + // Safe multiplication and division within loop + numerator_accum = numerator_accum + .checked_mul(numerator) + .ok_or(InternalError::ArithmeticOperationOverflow)? + .checked_div( + denominator + .checked_mul(i) + .ok_or(InternalError::ArithmeticOperationOverflow)?, + ) + .ok_or(VMError::Internal( + InternalError::ArithmeticOperationOverflow, + ))?; + + i = i + .checked_add(1) + .ok_or(InternalError::ArithmeticOperationOverflow)?; + } + + Ok(U256::from( + output + .checked_div(denominator) + .ok_or(InternalError::ArithmeticOperationOverflow)?, + )) +} diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 96e84b771f..d5361812d2 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -2,13 +2,16 @@ use crate::{ account::{Account, StorageSlot}, call_frame::CallFrame, constants::*, - db::{cache, CacheDB, Database}, + db::{ + cache::{self, remove_account}, + CacheDB, Database, + }, environment::Environment, errors::{ InternalError, OpcodeSuccess, OutOfGasError, ResultReason, TransactionReport, TxResult, TxValidationError, VMError, }, - gas_cost::{self}, + gas_cost::{self, fake_exponential, BLOB_GAS_PER_BLOB, CREATE_BASE_COST}, opcodes::Opcode, AccountInfo, }; @@ -106,7 +109,7 @@ impl VM { calldata.clone(), false, env.gas_limit, - TX_BASE_COST, + U256::zero(), 0, ); @@ -124,7 +127,6 @@ impl VM { TxKind::Create => { // CREATE tx - // (2) let new_contract_address = VM::calculate_create_address(env.origin, db.get_account_info(env.origin).nonce) .map_err(|_| { @@ -133,23 +135,21 @@ impl VM { default_touched_accounts.insert(new_contract_address); - // (3) let created_contract = Account::new(value, calldata.clone(), 1, HashMap::new()); cache::insert_account(&mut cache, new_contract_address, created_contract); - // (5) - let code: Bytes = calldata.clone(); + let bytecode: Bytes = calldata.clone(); let initial_call_frame = CallFrame::new( env.origin, new_contract_address, new_contract_address, - code, + bytecode, value, - Bytes::new(), + Bytes::new(), // Contract creation does not have calldata false, env.gas_limit, - TX_BASE_COST, + U256::zero(), 0, ); @@ -347,32 +347,93 @@ impl VM { matches!(self.tx_kind, TxKind::Create) } - fn revert_create(&mut self) -> Result<(), VMError> { - // Note: currently working with copies - let call_frame = self - .call_frames - .last() - .ok_or(VMError::Internal( - InternalError::CouldNotAccessLastCallframe, - ))? - .clone(); + fn add_intrinsic_gas(&mut self, initial_call_frame: &mut CallFrame) -> Result<(), VMError> { + // Intrinsic gas is the gas consumed by the transaction before the execution of the opcodes. Section 6.2 in the Yellow Paper. + + // Intrinsic Gas = Calldata cost + Create cost + Base cost + Access list cost + let mut intrinsic_gas = U256::zero(); + + // Calldata Cost + // 4 gas for each zero byte in the transaction data 16 gas for each non-zero byte in the transaction. + let calldata_cost = + gas_cost::tx_calldata(&initial_call_frame.calldata).map_err(VMError::OutOfGas)?; + + intrinsic_gas = intrinsic_gas + .checked_add(calldata_cost) + .ok_or(OutOfGasError::ConsumedGasOverflow)?; + + // Base Cost + intrinsic_gas = intrinsic_gas + .checked_add(TX_BASE_COST) + .ok_or(OutOfGasError::ConsumedGasOverflow)?; - self.decrement_account_nonce(call_frame.msg_sender)?; + // Create Cost + if self.is_create() { + intrinsic_gas = intrinsic_gas + .checked_add(CREATE_BASE_COST) + .ok_or(OutOfGasError::ConsumedGasOverflow)?; - let new_contract_address = call_frame.to; - if cache::remove_account(&mut self.cache, &new_contract_address).is_none() { - return Err(VMError::AddressDoesNotMatchAnAccount); // Should not be this error + let number_of_words: u64 = initial_call_frame + .calldata + .chunks(WORD_SIZE) + .len() + .try_into() + .map_err(|_| InternalError::ConversionError)?; + + intrinsic_gas = intrinsic_gas + .checked_add( + U256::from(number_of_words) + .checked_mul(U256::from(2)) + .ok_or(OutOfGasError::ConsumedGasOverflow)?, + ) + .ok_or(OutOfGasError::ConsumedGasOverflow)?; } + // Access List Cost + // TODO: Implement access list cost. + + self.increase_consumed_gas(initial_call_frame, intrinsic_gas) + .map_err(|_| TxValidationError::IntrinsicGasTooLow)?; + Ok(()) } + /// To get the maximum fee per gas that the user is willing to pay, independently of the actual gas price + /// For legacy transactions the max fee per gas is the gas price + fn max_fee_per_gas_or_gasprice(&self) -> U256 { + self.env.tx_max_fee_per_gas.unwrap_or(self.env.gas_price) + } + + /// Gets the max blob gas cost for a transaction that a user is willing to pay. + fn get_max_blob_gas_cost(&self) -> Result { + let blob_gas_used = U256::from(self.env.tx_blob_hashes.len()) + .checked_mul(BLOB_GAS_PER_BLOB) + .unwrap_or_default(); + + let blob_gas_cost = self + .env + .tx_max_fee_per_blob_gas + .unwrap_or_default() + .checked_mul(blob_gas_used) + .ok_or(TxValidationError::UndefinedState(1))?; + + Ok(blob_gas_cost) + } + + pub fn get_base_fee_per_blob_gas(&self) -> Result { + fake_exponential( + MIN_BASE_FEE_PER_BLOB_GAS, + self.env.block_excess_blob_gas.unwrap_or_default().low_u64(), //Maybe replace unwrap_or_default for sth else later. + BLOB_BASE_FEE_UPDATE_FRACTION, + ) + } + /// ## Description /// This method performs validations and returns an error if any of the validations fail. /// It also makes initial changes alongside the validations: /// - It increases sender nonce /// - It substracts up-front-cost from sender balance. (Not doing this for now) - /// - It calculates and adds intrinsic gas to the 'gas used' of callframe and environment. (Not doing this for now) + /// - 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... @@ -387,21 +448,27 @@ impl VM { let sender_account = self.get_account(sender_address); // (1) GASLIMIT_PRICE_PRODUCT_OVERFLOW - let gaslimit_price_product = - self.env - .gas_price - .checked_mul(self.env.gas_limit) - .ok_or(VMError::TxValidation( - TxValidationError::GasLimitPriceProductOverflow, - ))?; - - // Up front cost is the maximum amount of wei that a user is willing to pay for. - let up_front_cost = gaslimit_price_product - .checked_add(initial_call_frame.msg_value) + let gaslimit_price_product = self + .max_fee_per_gas_or_gasprice() + .checked_mul(self.env.gas_limit) .ok_or(VMError::TxValidation( - TxValidationError::InsufficientAccountFunds, + TxValidationError::GasLimitPriceProductOverflow, ))?; + // Up front cost is the maximum amount of wei that a user is willing to pay for. Gaslimit * gasprice + value + blob_gas_cost + let value = initial_call_frame.msg_value; + + // blob gas cost = max fee per blob gas * blob gas used + // https://eips.ethereum.org/EIPS/eip-4844 + let blob_gas_cost = self.get_max_blob_gas_cost()?; + + let up_front_cost = gaslimit_price_product + .checked_add(value) + .ok_or(TxValidationError::UndefinedState(1))? + .checked_add(blob_gas_cost) + .ok_or(TxValidationError::UndefinedState(1))?; + // There is no error specified for overflow in up_front_cost in ef_tests. Maybe we can go with GasLimitPriceProductOverflow or InsufficientAccountFunds. + // (2) INSUFFICIENT_ACCOUNT_FUNDS // NOT CHANGING SENDER BALANCE HERE FOR NOW // This will be increment_account_balance @@ -409,12 +476,10 @@ impl VM { .info .balance .checked_sub(up_front_cost) - .ok_or(VMError::TxValidation( - TxValidationError::InsufficientAccountFunds, - ))?; + .ok_or(TxValidationError::InsufficientAccountFunds)?; // (3) INSUFFICIENT_MAX_FEE_PER_GAS - if self.env.gas_price < self.env.base_fee_per_gas { + if self.max_fee_per_gas_or_gasprice() < self.env.base_fee_per_gas { return Err(VMError::TxValidation( TxValidationError::InsufficientMaxFeePerGas, )); @@ -423,7 +488,7 @@ impl VM { // (4) INITCODE_SIZE_EXCEEDED if self.is_create() { // INITCODE_SIZE_EXCEEDED - if initial_call_frame.calldata.len() >= INIT_CODE_MAX_SIZE { + if initial_call_frame.bytecode.len() > INIT_CODE_MAX_SIZE { return Err(VMError::TxValidation( TxValidationError::InitcodeSizeExceeded, )); @@ -431,8 +496,7 @@ impl VM { } // (5) INTRINSIC_GAS_TOO_LOW - // TODO: Not doing this for now - // self.add_intrinsic_gas(initial_call_frame)?; + self.add_intrinsic_gas(initial_call_frame)?; // (6) NONCE_IS_MAX self.increment_account_nonce(sender_address)?; @@ -463,15 +527,13 @@ impl VM { // (10) INSUFFICIENT_MAX_FEE_PER_BLOB_GAS if let Some(tx_max_fee_per_blob_gas) = self.env.tx_max_fee_per_blob_gas { - if tx_max_fee_per_blob_gas < self.env.base_fee_per_gas { + if tx_max_fee_per_blob_gas < self.get_base_fee_per_blob_gas()? { return Err(VMError::TxValidation( - TxValidationError::InsufficientMaxFeePerGas, + TxValidationError::InsufficientMaxFeePerBlobGas, )); } } - //TODO: Implement the rest of the validations (TYPE_3) - // Transaction is type 3 if tx_max_fee_per_blob_gas is Some if self.env.tx_max_fee_per_blob_gas.is_some() { let blob_hashes = &self.env.tx_blob_hashes; @@ -514,86 +576,32 @@ impl VM { } pub fn transact(&mut self) -> Result { - let initial_gas = Default::default(); - - self.env.consumed_gas = initial_gas; - - let mut current_call_frame = self + let mut initial_call_frame = self .call_frames .pop() .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?; - self.validate_transaction(&mut current_call_frame)?; + let cache_before_execution = self.cache.clone(); + self.validate_transaction(&mut initial_call_frame)?; - let mut report = self.execute(&mut current_call_frame)?; - - let initial_call_frame = self - .call_frames - .last() - .ok_or(VMError::Internal( - InternalError::CouldNotAccessLastCallframe, - ))? - .clone(); + let mut report = self.execute(&mut initial_call_frame)?; let sender = initial_call_frame.msg_sender; - let calldata_cost = - gas_cost::tx_calldata(&initial_call_frame.calldata).map_err(VMError::OutOfGas)?; - - report.gas_used = report - .gas_used - .checked_add(calldata_cost) - .ok_or(VMError::OutOfGas(OutOfGasError::GasUsedOverflow))?; - 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); - } - let contract_code = report.clone().output; - - // TODO: Is this the expected behavior? - if !contract_code.is_empty() { - // (6) - if contract_code.len() > MAX_CODE_SIZE { - return Err(VMError::ContractOutputTooBig); - } - // Supposing contract code has contents - if *contract_code - .first() - .ok_or(VMError::Internal(InternalError::TriedToIndexEmptyCode))? - == INVALID_CONTRACT_PREFIX - { - return Err(VMError::InvalidInitialByte); + match self.create_post_execution(&mut initial_call_frame, &mut report) { + Ok(_) => {} + Err(error) => { + if error.is_internal() { + return Err(error); + } else { + report.result = TxResult::Revert(error); + report.gas_used = self.env.gas_limit.low_u64(); + self.cache = cache_before_execution; + remove_account(&mut self.cache, &initial_call_frame.to); + } } - } - - // 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 - let number_of_words: u64 = initial_call_frame - .calldata - .chunks(WORD_SIZE) - .len() - .try_into() - .map_err(|_| VMError::Internal(InternalError::ConversionError))?; - - let code_length: u64 = contract_code - .len() - .try_into() - .map_err(|_| VMError::Internal(InternalError::ConversionError))?; - - let creation_cost = - gas_cost::tx_creation(code_length, number_of_words).map_err(VMError::OutOfGas)?; - report.gas_used = report - .gas_used - .checked_add(creation_cost) - .ok_or(VMError::OutOfGas(OutOfGasError::GasUsedOverflow))?; - // Charge 22100 gas for each storage variable set - - let contract_address = initial_call_frame.to; - - self.update_account_bytecode(contract_address, contract_code)?; + }; } let coinbase_address = self.env.coinbase; @@ -638,6 +646,48 @@ impl VM { )) } + fn create_post_execution( + &mut self, + initial_call_frame: &mut CallFrame, + report: &mut TransactionReport, + ) -> Result<(), VMError> { + if let TxResult::Revert(error) = &report.result { + return Err(error.clone()); + } + + let contract_code = report.clone().output; + + if contract_code.len() > MAX_CODE_SIZE { + return Err(VMError::ContractOutputTooBig); + } + + // If contract code is not empty then the first byte should not be 0xef + if *contract_code.first().unwrap_or(&0) == INVALID_CONTRACT_PREFIX { + return Err(VMError::InvalidInitialByte); + } + + let max_gas = self.env.gas_limit.low_u64(); + + // If initialization code is successful, code-deposit cost is paid. + let code_length: u64 = contract_code + .len() + .try_into() + .map_err(|_| VMError::Internal(InternalError::ConversionError))?; + let code_deposit_cost = code_length.checked_mul(200).ok_or(VMError::Internal( + InternalError::ArithmeticOperationOverflow, + ))?; + + report.add_gas_with_max(code_deposit_cost, max_gas)?; + // Charge 22100 gas for each storage variable set (???) + + // Assign bytecode to the new contract + let contract_address = initial_call_frame.to; + + self.update_account_bytecode(contract_address, contract_code)?; + + Ok(()) + } + // TODO: Improve and test REVERT behavior for XCALL opcodes. Issue: https://github.com/lambdaclass/ethrex/issues/1061 #[allow(clippy::too_many_arguments)] pub fn generic_call( diff --git a/crates/vm/vm.rs b/crates/vm/vm.rs index c0b11c896a..6480299a61 100644 --- a/crates/vm/vm.rs +++ b/crates/vm/vm.rs @@ -169,7 +169,7 @@ cfg_if::cfg_if! { let env = Environment { origin: tx.sender(), - consumed_gas: U256::from(21000), // Base gas cost for a transaction + consumed_gas: U256::zero(), refunded_gas: U256::zero(), gas_limit: tx.gas_limit().into(), block_number: block_header.number.into(),