Skip to content

Commit

Permalink
refactor(levm): remove unecessary clones (#1589)
Browse files Browse the repository at this point in the history 
This PR removes some unnecessary clones but not all of them. Some clones
are harmless in practice (and they were not removed), but others are
more difficult to remove and require serious code refactors (these are
all the call frame clones and the account and storage slot accesses).
  • Loading branch information
@ilitteri
ilitteri authored Jan 14, 2025
1 parent a666b8a commit a86f4ca
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 45 deletions.
6 changes: 0 additions & 6 deletions crates/vm/levm/src/call_frame.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,6 @@ impl CallFrame {
}
}

pub fn assign_bytecode(&mut self, bytecode: Bytes) {
self.bytecode = bytecode;
self.valid_jump_destinations =
get_valid_jump_destinations(&self.bytecode).unwrap_or_default();
}

#[allow(clippy::too_many_arguments)]
pub fn new(
msg_sender: Address,
Expand Down
50 changes: 31 additions & 19 deletions crates/vm/levm/src/precompiles.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ pub fn execute_precompile(
spec_id: SpecId,
) -> Result<Bytes, VMError> {
let callee_address = current_call_frame.code_address;
let calldata = current_call_frame.calldata.clone();
let gas_for_call = current_call_frame
.gas_limit
.checked_sub(current_call_frame.gas_used)
Expand All @@ -166,45 +165,58 @@ pub fn execute_precompile(

let result = match callee_address {
address if address == ECRECOVER_ADDRESS => {
ecrecover(&calldata, gas_for_call, consumed_gas)?
ecrecover(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == IDENTITY_ADDRESS => {
identity(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == SHA2_256_ADDRESS => {
sha2_256(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == IDENTITY_ADDRESS => identity(&calldata, gas_for_call, consumed_gas)?,
address if address == SHA2_256_ADDRESS => sha2_256(&calldata, gas_for_call, consumed_gas)?,
address if address == RIPEMD_160_ADDRESS => {
ripemd_160(&calldata, gas_for_call, consumed_gas)?
ripemd_160(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == MODEXP_ADDRESS => {
modexp(&calldata, gas_for_call, consumed_gas, spec_id)?
address if address == MODEXP_ADDRESS => modexp(
&current_call_frame.calldata,
gas_for_call,
consumed_gas,
spec_id,
)?,
address if address == ECADD_ADDRESS => {
ecadd(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == ECMUL_ADDRESS => {
ecmul(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == ECADD_ADDRESS => ecadd(&calldata, gas_for_call, consumed_gas)?,
address if address == ECMUL_ADDRESS => ecmul(&calldata, gas_for_call, consumed_gas)?,
address if address == ECPAIRING_ADDRESS => {
ecpairing(&calldata, gas_for_call, consumed_gas)?
ecpairing(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLAKE2F_ADDRESS => {
blake2f(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLAKE2F_ADDRESS => blake2f(&calldata, gas_for_call, consumed_gas)?,
address if address == POINT_EVALUATION_ADDRESS => {
point_evaluation(&calldata, gas_for_call, consumed_gas)?
point_evaluation(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_G1ADD_ADDRESS => {
bls12_g1add(&calldata, gas_for_call, consumed_gas)?
bls12_g1add(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_G1MSM_ADDRESS => {
bls12_g1msm(&calldata, gas_for_call, consumed_gas)?
bls12_g1msm(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_G2ADD_ADDRESS => {
bls12_g2add(&calldata, gas_for_call, consumed_gas)?
bls12_g2add(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_G2MSM_ADDRESS => {
bls12_g2msm(&calldata, gas_for_call, consumed_gas)?
bls12_g2msm(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_PAIRING_CHECK_ADDRESS => {
bls12_pairing_check(&calldata, gas_for_call, consumed_gas)?
bls12_pairing_check(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_MAP_FP_TO_G1_ADDRESS => {
bls12_map_fp_to_g1(&calldata, gas_for_call, consumed_gas)?
bls12_map_fp_to_g1(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
address if address == BLS12_MAP_FP2_TO_G2_ADDRESS => {
bls12_map_fp2_tp_g2(&calldata, gas_for_call, consumed_gas)?
bls12_map_fp2_tp_g2(&current_call_frame.calldata, gas_for_call, consumed_gas)?
}
_ => return Err(VMError::Internal(InternalError::InvalidPrecompileAddress)),
};
Expand Down
37 changes: 19 additions & 18 deletions crates/vm/levm/src/vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl VM {
address_to,
bytecode,
value,
calldata.clone(),
calldata,
false,
env.gas_limit,
0,
Expand Down Expand Up @@ -272,11 +272,11 @@ impl VM {

return Ok(TransactionReport {
result: TxResult::Success,
new_state: self.cache.clone(),
new_state: HashMap::default(),
gas_used: self.gas_used(current_call_frame)?,
gas_refunded: 0,
output,
logs: current_call_frame.logs.clone(),
logs: std::mem::take(&mut current_call_frame.logs),
created_address: None,
});
}
Expand All @@ -296,11 +296,11 @@ impl VM {

return Ok(TransactionReport {
result: TxResult::Revert(error),
new_state: self.cache.clone(),
new_state: HashMap::default(),
gas_used: current_call_frame.gas_limit,
gas_refunded: 0,
output: Bytes::new(),
logs: current_call_frame.logs.clone(),
logs: std::mem::take(&mut current_call_frame.logs),
created_address: None,
});
}
Expand Down Expand Up @@ -428,7 +428,7 @@ impl VM {
if (self.is_create() && current_call_frame.depth == 0)
|| current_call_frame.create_op_called
{
let contract_code = current_call_frame.output.clone();
let contract_code = std::mem::take(&mut current_call_frame.output);
let code_length = contract_code.len();

let code_length_u64: u64 = code_length
Expand Down Expand Up @@ -474,11 +474,11 @@ impl VM {

return Ok(TransactionReport {
result: TxResult::Revert(error),
new_state: self.cache.clone(),
new_state: HashMap::default(),
gas_used: self.gas_used(current_call_frame)?,
gas_refunded: self.env.refunded_gas,
output: current_call_frame.output.clone(),
logs: current_call_frame.logs.clone(),
output: std::mem::take(&mut current_call_frame.output),
logs: std::mem::take(&mut current_call_frame.logs),
created_address: None,
});
}
Expand All @@ -487,11 +487,11 @@ impl VM {

return Ok(TransactionReport {
result: TxResult::Success,
new_state: self.cache.clone(),
new_state: HashMap::default(),
gas_used: self.gas_used(current_call_frame)?,
gas_refunded: self.env.refunded_gas,
output: current_call_frame.output.clone(),
logs: current_call_frame.logs.clone(),
output: std::mem::take(&mut current_call_frame.output),
logs: std::mem::take(&mut current_call_frame.logs),
created_address: None,
});
}
Expand Down Expand Up @@ -520,11 +520,11 @@ impl VM {

return Ok(TransactionReport {
result: TxResult::Revert(error),
new_state: self.cache.clone(),
new_state: HashMap::default(),
gas_used: self.gas_used(current_call_frame)?,
gas_refunded: self.env.refunded_gas,
output: current_call_frame.output.clone(), // Bytes::new() if error is not RevertOpcode
logs: current_call_frame.logs.clone(),
output: std::mem::take(&mut current_call_frame.output), // Bytes::new() if error is not RevertOpcode
logs: std::mem::take(&mut current_call_frame.logs),
created_address: None,
});
}
Expand Down Expand Up @@ -915,8 +915,9 @@ impl VM {

if self.is_create() {
// Assign bytecode to context and empty calldata
initial_call_frame.assign_bytecode(initial_call_frame.calldata.clone());
initial_call_frame.calldata = Bytes::new();
initial_call_frame.bytecode = std::mem::take(&mut initial_call_frame.calldata);
initial_call_frame.valid_jump_destinations =
get_valid_jump_destinations(&initial_call_frame.bytecode).unwrap_or_default();
}
Ok(())
}
Expand Down Expand Up @@ -1274,7 +1275,7 @@ impl VM {
gas_used: self.env.gas_limit,
gas_refunded: 0,
logs: vec![],
new_state: self.cache.clone(),
new_state: HashMap::default(),
output: Bytes::new(),
created_address: None,
};
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/levm/tests/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3461,9 +3461,9 @@ fn logs_from_multiple_callers() {
.unwrap();

let mut current_call_frame = vm.call_frames.pop().unwrap();
vm.execute(&mut current_call_frame).unwrap();
let report = vm.execute(&mut current_call_frame).unwrap();

assert_eq!(current_call_frame.logs.len(), 2)
assert_eq!(report.logs.len(), 2)
}

// #[test]
Expand Down

0 comments on commit a86f4ca

Please sign in to comment.