Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(levm): fixing vm arithmetic tests changes #1333

Merged
merged 45 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
bdcf1c8
Remove account from caché if coinbase fee is zero
maximopalopoli Nov 26, 2024
f5fe205
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 26, 2024
98d0429
Add initial account states to compare the changes if any
ilitteri Nov 26, 2024
95e219b
Cache current account update before modify target
maximopalopoli Nov 26, 2024
829d808
Use transactions gas_limit
maximopalopoli Nov 27, 2024
70a3e19
Don't cap the gas_limit with MAX_BLOCK_GAS_LIMIT
maximopalopoli Nov 27, 2024
47f5f65
Set sender account as current to in generic_call
maximopalopoli Nov 27, 2024
0efa701
Do not expand memory if size to expand is zero
maximopalopoli Nov 27, 2024
4585c1a
Add comments + fmt
maximopalopoli Nov 28, 2024
8a4cca1
Fix: use msg_sender instead of call_frame's to
maximopalopoli Nov 28, 2024
cee0dcf
general fmt
maximopalopoli Nov 28, 2024
0369471
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 28, 2024
dcb3559
Merge branch 'main' into levm/fix/vmTests-fix-changes
ilitteri Nov 28, 2024
2dc9f72
Fix: Use U256 in checked_shift right and left
maximopalopoli Nov 28, 2024
e94d0b5
Remove debug prints
maximopalopoli Nov 28, 2024
5e2c8be
Add todo comment for revert refunded gas
maximopalopoli Nov 28, 2024
06d25f2
Return EFTestRunnerError instead of using unwrap
maximopalopoli Nov 28, 2024
30e6f17
Remove TODO comment
maximopalopoli Nov 28, 2024
1060987
Move verify of expansion size to expansion_cost
maximopalopoli Nov 28, 2024
b911701
Change expansion_cost occurences
maximopalopoli Nov 28, 2024
76fab10
Change code offset and size in creates to usize
maximopalopoli Nov 28, 2024
44063e4
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 28, 2024
3af4753
Merge branch 'main' into levm/fix/vmTests-fix-changes
ilitteri Nov 28, 2024
fd59340
Cover border case in signexend implementation
maximopalopoli Nov 28, 2024
adf1ac5
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 28, 2024
ceb79b0
Mannually change CI things
maximopalopoli Nov 28, 2024
f1d02c8
Don't increase balance in coinbase fee if it's 0
maximopalopoli Nov 28, 2024
f7ae0a7
fmt changes
maximopalopoli Nov 28, 2024
5d17b28
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 29, 2024
5a52dbf
Fix selfdestruct behavior
maximopalopoli Nov 29, 2024
0cf5c1b
Add default touched accounts in call
maximopalopoli Nov 29, 2024
c9e1c88
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 29, 2024
e97e3b8
Fix balance decrease
maximopalopoli Nov 29, 2024
2efb821
fmt
maximopalopoli Nov 29, 2024
6c87ce0
Merge branch 'levm/fix/vmTests-fix-changes' into levm/fix/vmArithmeti…
maximopalopoli Nov 29, 2024
029cd43
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 29, 2024
65fee84
Revert "Mannually change CI things"
ilitteri Nov 29, 2024
fd8b91c
Remove commented code
maximopalopoli Nov 29, 2024
6ba52b7
return to previous selfdestruct implementation
maximopalopoli Nov 29, 2024
ba14b9b
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 29, 2024
464bfc8
Merge branch 'main' into levm/fix/vmTests-fix-changes
maximopalopoli Nov 29, 2024
4632fa3
Merge branch 'levm/fix/vmTests-fix-changes' into levm/fix/vmArithmeti…
maximopalopoli Nov 29, 2024
4fa02d9
Merge branch 'main' into levm/fix/vmArithmeticTests-changes
maximopalopoli Nov 29, 2024
f4e1bde
Change "over u256 zero" for is not zero
maximopalopoli Nov 29, 2024
a0dc9d2
Ditto
maximopalopoli Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions crates/vm/levm/src/opcode_handlers/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,19 +237,14 @@ impl VM {
) -> Result<OpcodeSuccess, VMError> {
self.increase_consumed_gas(current_call_frame, gas_cost::SIGNEXTEND)?;

let byte_size: usize = current_call_frame
.stack
.pop()?
.try_into()
.map_err(|_| VMError::VeryLargeNumber)?;

let byte_size = current_call_frame.stack.pop()?;
let value_to_extend = current_call_frame.stack.pop()?;

let bits_per_byte: usize = 8;
let sign_bit_position_on_byte = 7;
let bits_per_byte = U256::from(8);
let sign_bit_position_on_byte = U256::from(7);

let max_byte_size: usize = 31;
let byte_size: usize = byte_size.min(max_byte_size);
let max_byte_size = U256::from(31);
let byte_size = byte_size.min(max_byte_size);
let total_bits = bits_per_byte
.checked_mul(byte_size)
.ok_or(VMError::Internal(
Expand All @@ -261,9 +256,20 @@ impl VM {
.ok_or(VMError::Internal(
InternalError::ArithmeticOperationOverflow,
))?;

let sign_bit_index: usize = match sign_bit_index.try_into() {
Ok(val) => val,
Err(_) => {
// this means the value_to_extend was too big to extend, so remains the same.
// Maybe this verification could be done before in this function
current_call_frame.stack.push(value_to_extend)?;
return Ok(OpcodeSuccess::Continue);
}
};

let is_negative = value_to_extend.bit(sign_bit_index);

let sign_bit_mask = checked_shift_left(U256::one(), sign_bit_index)?
let sign_bit_mask = checked_shift_left(U256::one(), U256::from(sign_bit_index))?
.checked_sub(U256::one())
.ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
Expand Down
46 changes: 23 additions & 23 deletions crates/vm/levm/src/opcode_handlers/bitwise_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,10 @@ impl VM {
let shift = current_call_frame.stack.pop()?;
let value = current_call_frame.stack.pop()?;

let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256

if shift < U256::from(256) {
current_call_frame
.stack
.push(checked_shift_left(value, shift_usize)?)?;
.push(checked_shift_left(value, shift)?)?;
} else {
current_call_frame.stack.push(U256::zero())?;
}
Expand All @@ -221,12 +219,10 @@ impl VM {
let shift = current_call_frame.stack.pop()?;
let value = current_call_frame.stack.pop()?;

let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256

if shift < U256::from(256) {
current_call_frame
.stack
.push(checked_shift_right(value, shift_usize)?)?;
.push(checked_shift_right(value, shift)?)?;
} else {
current_call_frame.stack.push(U256::zero())?;
}
Expand All @@ -253,32 +249,32 @@ impl VM {
}

pub fn arithmetic_shift_right(value: U256, shift: U256) -> Result<U256, VMError> {
let shift_usize: usize = shift.try_into().map_err(|_| VMError::VeryLargeNumber)?; // we know its not bigger than 256

if value.bit(255) {
// if negative fill with 1s
let shifted = checked_shift_right(value, shift_usize)?;
let shifted = checked_shift_right(value, shift)?;
let mask = checked_shift_left(
U256::MAX,
256_usize.checked_sub(shift_usize).ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?, // Note that this is already checked in op_sar
(U256::from(256))
.checked_sub(shift)
.ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?, // Note that this is already checked in op_sar
)?;

Ok(shifted | mask)
} else {
Ok(checked_shift_right(value, shift_usize)?)
Ok(checked_shift_right(value, shift)?)
}
}

/// Instead of using unsafe <<, uses checked_mul n times, replicating n shifts.
/// Note: These (checked_shift_left and checked_shift_right) are done because
/// are not available in U256
pub fn checked_shift_left(value: U256, shift: usize) -> Result<U256, VMError> {
pub fn checked_shift_left(value: U256, shift: U256) -> Result<U256, VMError> {
let mut result = value;
let mut shifts_left = shift;

while shifts_left > 0 {
while shifts_left > U256::zero() {
maximopalopoli marked this conversation as resolved.
Show resolved Hide resolved
result = match result.checked_mul(U256::from(2)) {
Some(num) => num,
None => {
Expand All @@ -297,26 +293,30 @@ pub fn checked_shift_left(value: U256, shift: usize) -> Result<U256, VMError> {
))?
}
};
shifts_left = shifts_left.checked_sub(1).ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?; // Should not reach negative values
shifts_left = shifts_left
.checked_sub(U256::one())
.ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?; // Should not reach negative values
}

Ok(result)
}

// Instead of using unsafe >>, uses checked_div n times, replicating n shifts
fn checked_shift_right(value: U256, shift: usize) -> Result<U256, VMError> {
fn checked_shift_right(value: U256, shift: U256) -> Result<U256, VMError> {
let mut result = value;
let mut shifts_left = shift;

while shifts_left > 0 {
while shifts_left > U256::zero() {
maximopalopoli marked this conversation as resolved.
Show resolved Hide resolved
result = result.checked_div(U256::from(2)).ok_or(VMError::Internal(
InternalError::ArithmeticOperationDividedByZero,
))?; // '2' will never be zero
shifts_left = shifts_left.checked_sub(1).ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?; // Should not reach negative values
shifts_left = shifts_left
.checked_sub(U256::one())
.ok_or(VMError::Internal(
InternalError::ArithmeticOperationUnderflow,
))?; // Should not reach negative values
}

Ok(result)
Expand Down
Loading