Skip to content

Commit

Permalink
refactor(levm): clean vm.rs file (#1796)
Browse files Browse the repository at this point in the history
**Motivation**

The `vm.rs` file is filled with functions that are used as "helper"
functions. It would be better, especially for newcomers to the project,
if the `vm.rs` file had only the bare essential "big picture" functions.


**Description**

This PR aims to move a lot of the "additional" logic to `utils.rs` file
and only keep the "big picture functions" in `vm.rs`

This PR moved the following functions from `vm.rs` to `utils.rs`:
- address_to_word
- calculate_create_address
- calculate_create2_address
- get_valid_jump_destinations
- get_account
- get_account_no_push_cache
- get_account_mut_vm<'vm>
- increase_account_balance
- decrease_account_balance
- update_account_bytecode
- get_intrinsic_gas
- max_blobs_per_block
- get_blob_base_fee_update_fraction_value
- get_base_fee_per_blob_gas
- get_max_blob_gas_price
- get_blob_gas_price
- get_n_value
- get_number_of_topics
- increment_account_nonce
- decrement_account_nonce
- word_to_address
- has_delegation
- get_authorized_address
- eip7702_recover_address
- eip7702_get_code
 

And deleted the following: 
- VM::get_account
- VM::get_account_no_push_cache
These were simple wrapping functions around functions that had the same
name.

The logic inside the functions was left unchanged, besides adding
parameters to the functions.
Closes #1632
  • Loading branch information
lima-limon-inc authored Jan 24, 2025
1 parent b1c4c9c commit ef46d69
Show file tree
Hide file tree
Showing 11 changed files with 852 additions and 704 deletions.
2 changes: 1 addition & 1 deletion crates/vm/levm/bench/revm_comparison/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bytes::Bytes;
use ethrex_levm::{call_frame::CallFrame, errors::TxResult, utils::new_vm_with_bytecode};
use ethrex_levm::{call_frame::CallFrame, errors::TxResult, testing::new_vm_with_bytecode};
use revm::{
db::BenchmarkDB,
primitives::{address, Bytecode, TransactTo},
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/levm/src/call_frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
errors::{InternalError, VMError},
memory::Memory,
opcodes::Opcode,
vm::get_valid_jump_destinations,
utils::get_valid_jump_destinations,
};
use bytes::Bytes;
use ethrex_core::{types::Log, Address, U256};
Expand Down
1 change: 1 addition & 0 deletions crates/vm/levm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod opcode_handlers;
pub mod opcodes;
pub mod operations;
pub mod precompiles;
pub mod testing;
pub mod utils;
pub mod vm;
pub use account::*;
Expand Down
7 changes: 5 additions & 2 deletions crates/vm/levm/src/opcode_handlers/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use crate::{
constants::LAST_AVAILABLE_BLOCK_LIMIT,
errors::{InternalError, OpcodeSuccess, VMError},
gas_cost,
vm::{address_to_word, VM},
utils::*,
vm::VM,
};
use ethrex_core::{
types::{BLOB_BASE_FEE_UPDATE_FRACTION, MIN_BASE_FEE_PER_BLOB_GAS},
Expand Down Expand Up @@ -137,7 +138,9 @@ impl VM {
) -> Result<OpcodeSuccess, VMError> {
self.increase_consumed_gas(current_call_frame, gas_cost::SELFBALANCE)?;

let balance = self.get_account(current_call_frame.to).info.balance;
let balance = get_account(&mut self.cache, &self.db, current_call_frame.to)
.info
.balance;

current_call_frame.stack.push(balance)?;
Ok(OpcodeSuccess::Continue)
Expand Down
3 changes: 2 additions & 1 deletion crates/vm/levm/src/opcode_handlers/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::{
errors::{InternalError, OpcodeSuccess, VMError},
gas_cost::{self},
memory::{self, calculate_memory_size},
vm::{has_delegation, word_to_address, VM},
utils::{has_delegation, word_to_address},
vm::VM,
};
use ethrex_core::U256;
use keccak_hash::keccak;
Expand Down
97 changes: 72 additions & 25 deletions crates/vm/levm/src/opcode_handlers/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::{
errors::{InternalError, OpcodeSuccess, OutOfGasError, ResultReason, TxResult, VMError},
gas_cost::{self, max_message_call_gas},
memory::{self, calculate_memory_size},
vm::{address_to_word, word_to_address, VM},
utils::*,
utils::{address_to_word, word_to_address},
vm::VM,
Account,
};
use bytes::Bytes;
Expand Down Expand Up @@ -51,8 +53,12 @@ impl VM {

let (account_info, address_was_cold) = self.access_account(callee);

let (is_delegation, eip7702_gas_consumed, code_address, bytecode) =
self.eip7702_get_code(callee)?;
let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = eip7702_get_code(
&mut self.cache,
&self.db,
&mut self.accrued_substate,
callee,
)?;

let gas_left = current_call_frame
.gas_limit
Expand Down Expand Up @@ -130,8 +136,12 @@ impl VM {

let (_account_info, address_was_cold) = self.access_account(code_address);

let (is_delegation, eip7702_gas_consumed, code_address, bytecode) =
self.eip7702_get_code(code_address)?;
let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = eip7702_get_code(
&mut self.cache,
&self.db,
&mut self.accrued_substate,
code_address,
)?;

let gas_left = current_call_frame
.gas_limit
Expand Down Expand Up @@ -237,8 +247,12 @@ impl VM {
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) =
self.eip7702_get_code(code_address)?;
let (is_delegation, eip7702_gas_consumed, code_address, bytecode) = eip7702_get_code(
&mut self.cache,
&self.db,
&mut self.accrued_substate,
code_address,
)?;

let gas_left = current_call_frame
.gas_limit
Expand Down Expand Up @@ -312,8 +326,12 @@ impl VM {
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) =
self.eip7702_get_code(code_address)?;
let (is_delegation, eip7702_gas_consumed, _, bytecode) = eip7702_get_code(
&mut self.cache,
&self.db,
&mut self.accrued_substate,
code_address,
)?;

let gas_left = current_call_frame
.gas_limit
Expand Down Expand Up @@ -503,8 +521,18 @@ impl VM {

// [EIP-6780] - SELFDESTRUCT only in same transaction from CANCUN
if self.env.spec_id >= SpecId::CANCUN {
self.increase_account_balance(target_address, balance_to_transfer)?;
self.decrease_account_balance(current_call_frame.to, balance_to_transfer)?;
increase_account_balance(
&mut self.cache,
&mut self.db,
target_address,
balance_to_transfer,
)?;
decrease_account_balance(
&mut self.cache,
&mut self.db,
current_call_frame.to,
balance_to_transfer,
)?;

// Selfdestruct is executed in the same transaction as the contract was created
if self
Expand All @@ -513,15 +541,24 @@ impl VM {
.contains(&current_call_frame.to)
{
// If target is the same as the contract calling, Ether will be burnt.
self.get_account_mut(current_call_frame.to)?.info.balance = U256::zero();
get_account_mut_vm(&mut self.cache, &self.db, current_call_frame.to)?
.info
.balance = U256::zero();

self.accrued_substate
.selfdestruct_set
.insert(current_call_frame.to);
}
} else {
self.increase_account_balance(target_address, balance_to_transfer)?;
self.get_account_mut(current_call_frame.to)?.info.balance = U256::zero();
increase_account_balance(
&mut self.cache,
&mut self.db,
target_address,
balance_to_transfer,
)?;
get_account_mut_vm(&mut self.cache, &self.db, current_call_frame.to)?
.info
.balance = U256::zero();

self.accrued_substate
.selfdestruct_set
Expand Down Expand Up @@ -571,8 +608,8 @@ impl VM {
);

let new_address = match salt {
Some(salt) => Self::calculate_create2_address(deployer_address, &code, salt)?,
None => Self::calculate_create_address(deployer_address, deployer_account_info.nonce)?,
Some(salt) => calculate_create2_address(deployer_address, &code, salt)?,
None => calculate_create_address(deployer_address, deployer_account_info.nonce)?,
};

// touch account
Expand Down Expand Up @@ -601,9 +638,9 @@ impl VM {
}

// THIRD: Validations that push 0 to the stack without returning reserved gas but incrementing deployer's nonce
let new_account = self.get_account(new_address);
let new_account = get_account(&mut self.cache, &self.db, new_address);
if new_account.has_code_or_nonce() {
self.increment_account_nonce(deployer_address)?;
increment_account_nonce(&mut self.cache, &self.db, deployer_address)?;
current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
return Ok(OpcodeSuccess::Continue);
}
Expand All @@ -620,10 +657,15 @@ impl VM {
cache::insert_account(&mut self.cache, new_address, new_account);

// 2. Increment sender's nonce.
self.increment_account_nonce(deployer_address)?;
increment_account_nonce(&mut self.cache, &self.db, deployer_address)?;

// 3. Decrease sender's balance.
self.decrease_account_balance(deployer_address, value_in_wei_to_send)?;
decrease_account_balance(
&mut self.cache,
&mut self.db,
deployer_address,
value_in_wei_to_send,
)?;

let mut new_call_frame = CallFrame::new(
deployer_address,
Expand Down Expand Up @@ -662,7 +704,12 @@ impl VM {
}
TxResult::Revert(err) => {
// Return value to sender
self.increase_account_balance(deployer_address, value_in_wei_to_send)?;
increase_account_balance(
&mut self.cache,
&mut self.db,
deployer_address,
value_in_wei_to_send,
)?;

// Deployment failed so account shouldn't exist
cache::remove_account(&mut self.cache, &new_address);
Expand Down Expand Up @@ -757,8 +804,8 @@ impl VM {

// Transfer value from caller to callee.
if should_transfer_value {
self.decrease_account_balance(msg_sender, value)?;
self.increase_account_balance(to, value)?;
decrease_account_balance(&mut self.cache, &mut self.db, msg_sender, value)?;
increase_account_balance(&mut self.cache, &mut self.db, to, value)?;
}

let tx_report = self.execute(&mut new_call_frame)?;
Expand Down Expand Up @@ -791,8 +838,8 @@ impl VM {
TxResult::Revert(_) => {
// Revert value transfer
if should_transfer_value {
self.decrease_account_balance(to, value)?;
self.increase_account_balance(msg_sender, value)?;
decrease_account_balance(&mut self.cache, &mut self.db, to, value)?;
increase_account_balance(&mut self.cache, &mut self.db, msg_sender, value)?;
}
// Push 0 to stack
current_call_frame.stack.push(REVERT_FOR_CALL)?;
Expand Down
109 changes: 109 additions & 0 deletions crates/vm/levm/src/testing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use crate::{
account::{Account, AccountInfo},
db::{cache, CacheDB, Db},
environment::Environment,
errors::{InternalError, VMError},
operations::Operation,
vm::VM,
};
use bytes::Bytes;
use ethrex_core::{types::TxKind, Address, U256};
use std::{collections::HashMap, sync::Arc};

pub fn ops_to_bytecode(operations: &[Operation]) -> Result<Bytes, VMError> {
let mut bytecode = Vec::new();
for op in operations {
bytecode.extend_from_slice(
&op.to_bytecode()
.map_err(|_| VMError::Internal(InternalError::UtilsError))?,
); // for now it is just a utils error...
}
Ok(bytecode.into())
}

pub fn new_vm_with_bytecode(bytecode: Bytes) -> Result<VM, VMError> {
new_vm_with_ops_addr_bal_db(
bytecode,
Address::from_low_u64_be(100),
U256::MAX,
Db::new(),
CacheDB::default(),
)
}

pub fn new_vm_with_ops(operations: &[Operation]) -> Result<VM, VMError> {
let bytecode = ops_to_bytecode(operations)?;
new_vm_with_ops_addr_bal_db(
bytecode,
Address::from_low_u64_be(100),
U256::MAX,
Db::new(),
CacheDB::default(),
)
}

pub fn new_vm_with_ops_db(operations: &[Operation], db: Db) -> Result<VM, VMError> {
let bytecode = ops_to_bytecode(operations)?;
new_vm_with_ops_addr_bal_db(
bytecode,
Address::from_low_u64_be(100),
U256::MAX,
db,
CacheDB::default(),
)
}

/// This function is for testing purposes only.
pub fn new_vm_with_ops_addr_bal_db(
contract_bytecode: Bytes,
sender_address: Address,
sender_balance: U256,
mut db: Db,
mut cache: CacheDB,
) -> Result<VM, VMError> {
let accounts = [
// This is the contract account that is going to be executed
(
Address::from_low_u64_be(42),
Account {
info: AccountInfo {
nonce: 0,
balance: U256::MAX,
bytecode: contract_bytecode,
},
storage: HashMap::new(),
},
),
(
// This is the sender account
sender_address,
Account {
info: AccountInfo {
nonce: 0,
balance: sender_balance,
bytecode: Bytes::default(),
},
storage: HashMap::new(),
},
),
];

db.add_accounts(accounts.to_vec());

// add to cache accounts from list accounts
cache::insert_account(&mut cache, accounts[0].0, accounts[0].1.clone());
cache::insert_account(&mut cache, accounts[1].0, accounts[1].1.clone());

let env = Environment::default_from_address(sender_address);

VM::new(
TxKind::Call(Address::from_low_u64_be(42)),
env,
Default::default(),
Default::default(),
Arc::new(db),
cache,
Vec::new(),
None,
)
}
Loading

0 comments on commit ef46d69

Please sign in to comment.