Skip to content

Commit

Permalink
nit: use uint macro & fix various small things (bluealloy#1253)
Browse files Browse the repository at this point in the history
* nit: Use uint macro & fix various small things

* Move MAX_POSITIVE_VALUE back to tests
  • Loading branch information
jtraglia authored Apr 2, 2024
1 parent b3220c6 commit c3f0503
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 123 deletions.
8 changes: 4 additions & 4 deletions bins/revme/src/cmd/statetest/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub enum TestErrorKind {
got_exception: Option<String>,
},
#[error("Unexpected output: {got_output:?} but test expects:{expected_output:?}")]
UnexpecteOutput {
UnexpectedOutput {
expected_output: Option<Bytes>,
got_output: Option<Bytes>,
},
Expand Down Expand Up @@ -79,7 +79,7 @@ fn skip_test(path: &Path) -> bool {
| "RevertPrecompiledTouch_storage.json"
| "RevertPrecompiledTouch.json"

// txbyte is of type 02 and we dont parse tx bytes for this test to fail.
// txbyte is of type 02 and we don't parse tx bytes for this test to fail.
| "typeTwoBerlin.json"

// Need to handle Test errors
Expand Down Expand Up @@ -139,7 +139,7 @@ fn check_evm_execution<EXT>(
}
};

// if we expect exception revm should return error from execution.
// If we expect exception revm should return error from execution.
// So we do not check logs and state root.
//
// Note that some tests that have exception and run tests from before state clear
Expand All @@ -155,7 +155,7 @@ fn check_evm_execution<EXT>(
// check output
if let Some((expected_output, output)) = expected_output.zip(result.output()) {
if expected_output != output {
let kind = TestErrorKind::UnexpecteOutput {
let kind = TestErrorKind::UnexpectedOutput {
expected_output: Some(expected_output.clone()),
got_output: result.output().cloned(),
};
Expand Down
2 changes: 1 addition & 1 deletion crates/interpreter/src/gas/calc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn exp_cost<SPEC: Spec>(power: U256) -> Option<u64> {
} else {
// EIP-160: EXP cost increase
let gas_byte = U256::from(if SPEC::enabled(SPURIOUS_DRAGON) {
50u64
50
} else {
10
});
Expand Down
170 changes: 70 additions & 100 deletions crates/interpreter/src/instructions/i256.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,53 +126,9 @@ pub fn i256_mod(mut first: U256, mut second: U256) -> U256 {
mod tests {
use super::*;
use core::num::Wrapping;
use revm_primitives::uint;

const ZERO: U256 = U256::ZERO;
const ONE: U256 = U256::from_limbs([
0x0000000000000001,
0x0000000000000000,
0x0000000000000000,
0x0000000000000000,
]);
const TWO: U256 = U256::from_limbs([
0x0000000000000002,
0x0000000000000000,
0x0000000000000000,
0x0000000000000000,
]);
const THREE: U256 = U256::from_limbs([
0x0000000000000003,
0x0000000000000000,
0x0000000000000000,
0x0000000000000000,
]);
const FOUR: U256 = U256::from_limbs([
0x0000000000000004,
0x0000000000000000,
0x0000000000000000,
0x0000000000000000,
]);

const NEG_ONE: U256 = U256::from_limbs([
0xffffffffffffffff,
0xffffffffffffffff,
0xffffffffffffffff,
0xffffffffffffffff,
]);
const NEG_TWO: U256 = U256::from_limbs([
0xfffffffffffffffe,
0xffffffffffffffff,
0xffffffffffffffff,
0xffffffffffffffff,
]);
const NEG_THREE: U256 = U256::from_limbs([
0xfffffffffffffffd,
0xffffffffffffffff,
0xffffffffffffffff,
0xffffffffffffffff,
]);

const I256_MAX: U256 = U256::from_limbs([
const MAX_POSITIVE_VALUE: U256 = U256::from_limbs([
0xffffffffffffffff,
0xffffffffffffffff,
0xffffffffffffffff,
Expand All @@ -186,87 +142,101 @@ mod tests {
assert_eq!(Wrapping(i8::MIN) / Wrapping(-1), Wrapping(i8::MIN));
assert_eq!(i8::MAX / -1, -i8::MAX);

// Now the same calculations based on i256
let fifty = U256::from(50);
let one_hundred = U256::from(100);

assert_eq!(i256_div(MIN_NEGATIVE_VALUE, NEG_ONE), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, ONE), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(I256_MAX, ONE), I256_MAX);
assert_eq!(i256_div(I256_MAX, NEG_ONE), NEG_ONE * I256_MAX);
assert_eq!(i256_div(one_hundred, NEG_ONE), NEG_ONE * one_hundred);
assert_eq!(i256_div(one_hundred, TWO), fifty);
uint! {
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, -1_U256), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, 1_U256), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(MAX_POSITIVE_VALUE, 1_U256), MAX_POSITIVE_VALUE);
assert_eq!(i256_div(MAX_POSITIVE_VALUE, -1_U256), -1_U256 * MAX_POSITIVE_VALUE);
assert_eq!(i256_div(100_U256, -1_U256), -100_U256);
assert_eq!(i256_div(100_U256, 2_U256), 50_U256);
}
}
#[test]
fn test_i256_sign() {
assert_eq!(i256_sign(&ZERO), Sign::Zero);
assert_eq!(i256_sign(&ONE), Sign::Plus);
assert_eq!(i256_sign(&MIN_NEGATIVE_VALUE), Sign::Minus);
uint! {
assert_eq!(i256_sign(&0_U256), Sign::Zero);
assert_eq!(i256_sign(&1_U256), Sign::Plus);
assert_eq!(i256_sign(&-1_U256), Sign::Minus);
assert_eq!(i256_sign(&MIN_NEGATIVE_VALUE), Sign::Minus);
assert_eq!(i256_sign(&MAX_POSITIVE_VALUE), Sign::Plus);
}
}

#[test]
fn test_i256_sign_compl() {
let mut zero = ZERO;
let mut positive = ONE;
let mut negative = MIN_NEGATIVE_VALUE;
assert_eq!(i256_sign_compl(&mut zero), Sign::Zero);
assert_eq!(i256_sign_compl(&mut positive), Sign::Plus);
assert_eq!(i256_sign_compl(&mut negative), Sign::Minus);
uint! {
let mut zero = 0_U256;
let mut positive = 1_U256;
let mut negative = -1_U256;
assert_eq!(i256_sign_compl(&mut zero), Sign::Zero);
assert_eq!(i256_sign_compl(&mut positive), Sign::Plus);
assert_eq!(i256_sign_compl(&mut negative), Sign::Minus);
}
}

#[test]
fn test_two_compl() {
assert_eq!(two_compl(ZERO), ZERO);
assert_eq!(two_compl(ONE), NEG_ONE);
assert_eq!(two_compl(NEG_ONE), ONE);
assert_eq!(two_compl(TWO), NEG_TWO);
assert_eq!(two_compl(NEG_TWO), TWO);
uint! {
assert_eq!(two_compl(0_U256), 0_U256);
assert_eq!(two_compl(1_U256), -1_U256);
assert_eq!(two_compl(-1_U256), 1_U256);
assert_eq!(two_compl(2_U256), -2_U256);
assert_eq!(two_compl(-2_U256), 2_U256);

// Two's complement of the min value is itself.
assert_eq!(two_compl(MIN_NEGATIVE_VALUE), MIN_NEGATIVE_VALUE);
// Two's complement of the min value is itself.
assert_eq!(two_compl(MIN_NEGATIVE_VALUE), MIN_NEGATIVE_VALUE);
}
}

#[test]
fn test_two_compl_mut() {
let mut value = ONE;
two_compl_mut(&mut value);
assert_eq!(value, NEG_ONE);
uint! {
let mut value = 1_U256;
two_compl_mut(&mut value);
assert_eq!(value, -1_U256);
}
}

#[test]
fn test_i256_cmp() {
assert_eq!(i256_cmp(&ONE, &TWO), Ordering::Less);
assert_eq!(i256_cmp(&TWO, &TWO), Ordering::Equal);
assert_eq!(i256_cmp(&THREE, &TWO), Ordering::Greater);
assert_eq!(i256_cmp(&NEG_ONE, &NEG_ONE), Ordering::Equal);
assert_eq!(i256_cmp(&NEG_ONE, &NEG_TWO), Ordering::Greater);
assert_eq!(i256_cmp(&NEG_ONE, &ZERO), Ordering::Less);
assert_eq!(i256_cmp(&NEG_TWO, &TWO), Ordering::Less);
uint! {
assert_eq!(i256_cmp(&1_U256, &2_U256), Ordering::Less);
assert_eq!(i256_cmp(&2_U256, &2_U256), Ordering::Equal);
assert_eq!(i256_cmp(&3_U256, &2_U256), Ordering::Greater);
assert_eq!(i256_cmp(&-1_U256, &-1_U256), Ordering::Equal);
assert_eq!(i256_cmp(&-1_U256, &-2_U256), Ordering::Greater);
assert_eq!(i256_cmp(&-1_U256, &0_U256), Ordering::Less);
assert_eq!(i256_cmp(&-2_U256, &2_U256), Ordering::Less);
}
}

#[test]
fn test_i256_div() {
assert_eq!(i256_div(ONE, ZERO), ZERO);
assert_eq!(i256_div(ZERO, ONE), ZERO);
assert_eq!(i256_div(ZERO, NEG_ONE), ZERO);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, ONE), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(FOUR, TWO), TWO);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, MIN_NEGATIVE_VALUE), ONE);
assert_eq!(i256_div(TWO, NEG_ONE), NEG_TWO);
assert_eq!(i256_div(NEG_TWO, NEG_ONE), TWO);
uint! {
assert_eq!(i256_div(1_U256, 0_U256), 0_U256);
assert_eq!(i256_div(0_U256, 1_U256), 0_U256);
assert_eq!(i256_div(0_U256, -1_U256), 0_U256);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, 1_U256), MIN_NEGATIVE_VALUE);
assert_eq!(i256_div(4_U256, 2_U256), 2_U256);
assert_eq!(i256_div(MIN_NEGATIVE_VALUE, MIN_NEGATIVE_VALUE), 1_U256);
assert_eq!(i256_div(2_U256, -1_U256), -2_U256);
assert_eq!(i256_div(-2_U256, -1_U256), 2_U256);
}
}

#[test]
fn test_i256_mod() {
assert_eq!(i256_mod(ZERO, ONE), ZERO);
assert_eq!(i256_mod(ONE, ZERO), ZERO);
assert_eq!(i256_mod(FOUR, TWO), ZERO);
assert_eq!(i256_mod(THREE, TWO), ONE);
assert_eq!(i256_mod(MIN_NEGATIVE_VALUE, ONE), ZERO);
assert_eq!(i256_mod(TWO, TWO), ZERO);
assert_eq!(i256_mod(TWO, THREE), TWO);
assert_eq!(i256_mod(NEG_TWO, THREE), NEG_TWO);
assert_eq!(i256_mod(TWO, NEG_THREE), TWO);
assert_eq!(i256_mod(NEG_TWO, NEG_THREE), NEG_TWO);
uint! {
assert_eq!(i256_mod(0_U256, 1_U256), 0_U256);
assert_eq!(i256_mod(1_U256, 0_U256), 0_U256);
assert_eq!(i256_mod(4_U256, 2_U256), 0_U256);
assert_eq!(i256_mod(3_U256, 2_U256), 1_U256);
assert_eq!(i256_mod(MIN_NEGATIVE_VALUE, 1_U256), 0_U256);
assert_eq!(i256_mod(2_U256, 2_U256), 0_U256);
assert_eq!(i256_mod(2_U256, 3_U256), 2_U256);
assert_eq!(i256_mod(-2_U256, 3_U256), -2_U256);
assert_eq!(i256_mod(2_U256, -3_U256), 2_U256);
assert_eq!(i256_mod(-2_U256, -3_U256), -2_U256);
}
}
}
4 changes: 2 additions & 2 deletions crates/interpreter/src/instructions/macros.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Utility macros to help implementementing opcode instruction functions.
//! Utility macros to help implementing opcode instruction functions.
/// Fails the instruction if the current call is static.
#[macro_export]
Expand Down Expand Up @@ -78,7 +78,7 @@ macro_rules! resize_memory {
return $ret;
}

// Gas is calculated in evm words (256bits).
// Gas is calculated in evm words (256 bits).
let words_num = rounded_size / 32;
if !$interp
.gas
Expand Down
6 changes: 5 additions & 1 deletion crates/interpreter/src/interpreter/shared_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,8 @@ impl SharedMemory {
}
}

/// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned.
/// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. Note, if `x`
/// is greater than `usize::MAX - 31` this will return `usize::MAX` which isn't a multiple of 32.
#[inline]
pub fn next_multiple_of_32(x: usize) -> usize {
let r = x.bitand(31).not().wrapping_add(1).bitand(31);
Expand All @@ -330,6 +331,9 @@ mod tests {
let next_multiple = x + 32 - (x % 32);
assert_eq!(next_multiple, next_multiple_of_32(x));
}

// We expect large values to saturate and not overflow.
assert_eq!(usize::MAX, next_multiple_of_32(usize::MAX));
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions crates/primitives/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ impl Bytecode {
}
}

/// Create new checked bytecode
/// Create new checked bytecode.
///
/// # Safety
///
/// Bytecode need to end with STOP (0x00) opcode as checked bytecode assumes
/// that it is safe to iterate over bytecode without checking lengths
/// Bytecode needs to end with STOP (0x00) opcode as checked bytecode assumes
/// that it is safe to iterate over bytecode without checking lengths.
pub unsafe fn new_checked(bytecode: Bytes, len: usize) -> Self {
Self {
bytecode,
Expand Down
4 changes: 2 additions & 2 deletions crates/revm/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<EXT, DB: Database> Evm<'_, EXT, DB> {
&mut self,
first_frame: Frame,
) -> Result<FrameResult, EVMError<DB::Error>> {
// take instruction talbe
// take instruction table
let table = self
.handler
.take_instruction_table()
Expand Down Expand Up @@ -265,7 +265,7 @@ impl<EXT, DB: Database> Evm<'_, EXT, DB> {
let next_action = interpreter.run(shared_memory, instruction_table, self);

// take error and break the loop if there is any.
// This error is set From Interpreter when its interacting with Host.
// This error is set From Interpreter when it's interacting with Host.
core::mem::replace(&mut self.context.evm.error, Ok(()))?;
// take shared memory back.
shared_memory = interpreter.take_memory();
Expand Down
8 changes: 4 additions & 4 deletions crates/revm/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@ use std::boxed::Box;
pub struct CallFrame {
/// Call frame has return memory range where output will be stored.
pub return_memory_range: Range<usize>,
/// Frame data
/// Frame data.
pub frame_data: FrameData,
}

#[derive(Debug)]
pub struct CreateFrame {
/// Create frame has a created address.
pub created_address: Address,
/// Frame data
/// Frame data.
pub frame_data: FrameData,
}

#[derive(Debug)]
pub struct FrameData {
/// Journal checkpoint
/// Journal checkpoint.
pub checkpoint: JournalCheckpoint,
/// Interpreter
/// Interpreter.
pub interpreter: Interpreter,
}

Expand Down
Loading

0 comments on commit c3f0503

Please sign in to comment.