Skip to content

Commit 9881520

Browse files
authored
fix(levm): smod non-compliance (#1305)
Resolves #1291
1 parent d2ea924 commit 9881520

File tree

3 files changed

+45
-27
lines changed

3 files changed

+45
-27
lines changed

crates/vm/levm/src/opcode_handlers/arithmetic.rs

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,32 @@ impl VM {
134134
) -> Result<OpcodeSuccess, VMError> {
135135
self.increase_consumed_gas(current_call_frame, gas_cost::SMOD)?;
136136

137-
let dividend = current_call_frame.stack.pop()?;
138-
let divisor = current_call_frame.stack.pop()?;
139-
if divisor.is_zero() {
137+
let unchecked_dividend = current_call_frame.stack.pop()?;
138+
let unchecked_divisor = current_call_frame.stack.pop()?;
139+
140+
if unchecked_divisor.is_zero() {
140141
current_call_frame.stack.push(U256::zero())?;
141-
} else {
142-
let normalized_dividend = abs(dividend);
143-
let normalized_divisor = abs(divisor);
144-
145-
let mut remainder =
146-
normalized_dividend
147-
.checked_rem(normalized_divisor)
148-
.ok_or(VMError::Internal(
149-
InternalError::ArithmeticOperationDividedByZero,
150-
))?; // Cannot be zero bc if above;
151-
152-
// The remainder should have the same sign as the dividend
153-
if is_negative(dividend) {
154-
remainder = negate(remainder);
142+
return Ok(OpcodeSuccess::Continue);
143+
}
144+
145+
let divisor = abs(unchecked_divisor);
146+
let dividend = abs(unchecked_dividend);
147+
148+
let unchecked_remainder = match dividend.checked_rem(divisor) {
149+
Some(remainder) => remainder,
150+
None => {
151+
current_call_frame.stack.push(U256::zero())?;
152+
return Ok(OpcodeSuccess::Continue);
155153
}
154+
};
156155

157-
current_call_frame.stack.push(remainder)?;
158-
}
156+
let remainder = if is_negative(unchecked_dividend) {
157+
negate(unchecked_remainder)
158+
} else {
159+
unchecked_remainder
160+
};
161+
162+
current_call_frame.stack.push(remainder)?;
159163

160164
Ok(OpcodeSuccess::Continue)
161165
}
@@ -294,16 +298,15 @@ fn is_negative(value: U256) -> bool {
294298
}
295299

296300
/// Negates a number in two's complement
301+
fn negate(value: U256) -> U256 {
302+
let (dividend, _overflowed) = (!value).overflowing_add(U256::one());
303+
dividend
304+
}
305+
297306
fn abs(value: U256) -> U256 {
298307
if is_negative(value) {
299308
negate(value)
300309
} else {
301310
value
302311
}
303312
}
304-
305-
/// Negates a number in two's complement
306-
fn negate(value: U256) -> U256 {
307-
let inverted = !value;
308-
inverted.saturating_add(U256::one())
309-
}

crates/vm/levm/tests/edge_case_tests.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,15 @@ fn test_non_compliance_codecopy() {
225225
&U256::zero()
226226
);
227227
}
228+
229+
#[test]
230+
fn test_non_compliance_smod() {
231+
let mut vm =
232+
new_vm_with_bytecode(Bytes::copy_from_slice(&[0x60, 1, 0x60, 1, 0x19, 0x07])).unwrap();
233+
let mut current_call_frame = vm.call_frames.pop().unwrap();
234+
vm.execute(&mut current_call_frame);
235+
assert_eq!(
236+
current_call_frame.stack.stack.first().unwrap(),
237+
&U256::zero()
238+
);
239+
}

crates/vm/levm/tests/tests.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,10 @@ fn smod_op() {
176176
let mut current_call_frame = vm.call_frames.pop().unwrap();
177177
vm.execute(&mut current_call_frame);
178178

179-
assert!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap() == U256::from(1));
179+
assert_eq!(
180+
vm.current_call_frame_mut().unwrap().stack.pop().unwrap(),
181+
U256::one()
182+
);
180183

181184
// Second Example
182185
// Example taken from evm.codes
@@ -209,7 +212,7 @@ fn smod_op() {
209212
)
210213
.unwrap();
211214

212-
assert!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap() == c);
215+
assert_eq!(vm.current_call_frame_mut().unwrap().stack.pop().unwrap(), c);
213216
}
214217

215218
#[test]

0 commit comments

Comments
 (0)