Skip to content

Commit

Permalink
fix(levm): fix various kind of errors in opcodes (#1424)
Browse files Browse the repository at this point in the history
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->
- Generic Call works well for basic call cases but it handles poorly
some scenarios, and we still have to figure out how to solve the stack
overflow problem when depth is too high.
- Fix other things I see along the way

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
- It replaces `OpcodeSuccess::Stop` for `OpcodeSuccess::Continue` in
some cases because we don't want to stop execution of current callframe,
we just want to push to it's stack and continue.
- It fixes `op_gas`, that previously was calculated based on used gas in
environment, and it should be based upon used gas in current callframe.
- It fixes `op_gaslimit`, that previously returned the transaction
gaslimit, when instead it should return the block's gas limit.


**Status**
- Tests passing with current changes: 1930/4100
<!-- Link to issues: Resolves #111, Resolves #222 -->
  • Loading branch information
JereSalo authored Dec 9, 2024
1 parent 21989f8 commit fa126ae
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 11 deletions.
2 changes: 1 addition & 1 deletion crates/vm/levm/src/opcode_handlers/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl VM {
) -> Result<OpcodeSuccess, VMError> {
self.increase_consumed_gas(current_call_frame, gas_cost::GASLIMIT)?;

current_call_frame.stack.push(self.env.gas_limit)?;
current_call_frame.stack.push(self.env.block_gas_limit)?;

Ok(OpcodeSuccess::Continue)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,9 @@ impl VM {
pub fn op_gas(&mut self, current_call_frame: &mut CallFrame) -> Result<OpcodeSuccess, VMError> {
self.increase_consumed_gas(current_call_frame, gas_cost::GAS)?;

let remaining_gas = self
.env
let remaining_gas = current_call_frame
.gas_limit
.checked_sub(self.env.consumed_gas)
.checked_sub(current_call_frame.gas_used)
.ok_or(OutOfGasError::ConsumedGasOverflow)?;
// Note: These are not consumed gas calculations, but are related, so I used this wrapping here
current_call_frame.stack.push(remaining_gas)?;
Expand Down
10 changes: 4 additions & 6 deletions crates/vm/levm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ impl VM {
current_call_frame
.stack
.push(U256::from(SUCCESS_FOR_CALL))?;
return Ok(OpcodeSuccess::Result(ResultReason::Stop));
return Ok(OpcodeSuccess::Continue);
}

// self.cache.increment_account_nonce(&code_address); // Internal call doesn't increment account nonce.
Expand All @@ -799,7 +799,7 @@ impl VM {
let new_depth = current_call_frame
.depth
.checked_add(1)
.ok_or(VMError::StackOverflow)?; // Maybe could be depthOverflow but in concept is quite similar
.ok_or(InternalError::ArithmeticOperationOverflow)?;

let mut new_call_frame = CallFrame::new(
msg_sender,
Expand All @@ -814,11 +814,9 @@ impl VM {
new_depth,
);

// TODO: Increase this to 1024
if new_call_frame.depth > 10 {
if new_call_frame.depth > 1024 {
current_call_frame.stack.push(U256::from(REVERT_FOR_CALL))?;
// return Ok(OpcodeSuccess::Result(ResultReason::Revert));
return Err(VMError::StackOverflow); // This is wrong but it is for testing purposes.
return Ok(OpcodeSuccess::Continue);
}

current_call_frame.sub_return_data_offset = ret_offset;
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/levm/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2811,7 +2811,7 @@ fn gaslimit_op() {
let operations = [Operation::Gaslimit, Operation::Stop];

let mut vm = new_vm_with_ops(&operations).unwrap();
vm.env.gas_limit = gas_limit;
vm.env.block_gas_limit = gas_limit;

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

0 comments on commit fa126ae

Please sign in to comment.