Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions actors/evm/src/interpreter/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ pub mod opcodes {
0x1b: SHL,
0x1c: SHR,
0x1d: SAR,
0x1e: CLZ,
0x20: KECCAK256,
0x30: ADDRESS,
0x31: BALANCE,
Expand Down Expand Up @@ -355,6 +356,7 @@ mod tests {
SHL,
SHR,
SAR,
CLZ,
DUP1,
DUP2,
DUP3,
Expand Down
64 changes: 64 additions & 0 deletions actors/evm/src/interpreter/instructions/bitwise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,73 @@ pub fn sar(shift: U256, mut value: U256) -> U256 {
}
}

/// Implements EIP-7939 `CLZ`, returning the number of leading zero bits in a 256-bit word.
/// `CLZ(0)` returns 256.
#[inline]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[inline]
#[inline]
/// Implements EIP-7939 `CLZ` opcode, returns number of leading zero bits

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied this in spirit. I kept the doc comment a bit more explicit and also called out CLZ(0) = 256.

pub fn clz(value: U256) -> U256 {
U256::from(value.leading_zeros())
}
Comment on lines +48 to +51
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might be obvious to some, but my brain doesn't immediately click that clz is count leading zeroes. One line of docs would reduce mental overhead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this. Try /// docstring. This shows up in editors cleanly.

I see that this is also okay in a way that EIP calling it "CLZ", so should be fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a /// doc comment on clz clarifying that it implements EIP-7939 Count Leading Zeroes over a 256-bit word, with CLZ(0) = 256.


#[cfg(test)]
mod tests {
use super::*;
use crate::interpreter::opcodes;
use crate::interpreter::{Output, execution::Machine, system::System};
use crate::{Bytecode, EthAddress, ExecutionState};
use fil_actors_runtime::test_utils::MockRuntime;
use fvm_shared::econ::TokenAmount;

mod test_vectors {
include!(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/test_vectors.rs"));
}

#[test]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure of this test. Either we introduce a wide range of tests, or a fuzzy test here, or none at all. This seems quite arbitrary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the extra test_clz_misc_unit case. The remaining unit, machine-step, and end-to-end coverage now all use the canonical EIP-7939 vectors.

fn test_clz_eip7939_vectors_unit() {
for (input, expected) in test_vectors::clz_eip7939_test_vectors() {
assert_eq!(clz(input), expected);
}
}

fn clz_via_evm(value: U256) -> U256 {
let rt = MockRuntime::default();
rt.in_call.replace(true);

let mut state = ExecutionState::new(
EthAddress::from_id(1000),
EthAddress::from_id(1000),
TokenAmount::from_atto(0),
Vec::new(),
);

let mut imm = [0u8; 32];
value.write_as_big_endian(&mut imm);

let mut code = Vec::with_capacity(1 + 32 + 1);
code.push(opcodes::PUSH32);
code.extend_from_slice(&imm);
code.push(opcodes::CLZ);

let mut system = System::new(&rt, false);
let bytecode = Bytecode::new(code);
let mut machine = Machine {
system: &mut system,
state: &mut state,
bytecode: &bytecode,
pc: 0,
output: Output::default(),
};

machine.step().expect("PUSH32 step failed");
machine.step().expect("CLZ step failed");
machine.state.stack.pop_many::<1>().expect("missing CLZ result")[0]
}

#[test]
fn test_clz_eip7939_vectors_via_evm() {
for (input, expected) in test_vectors::clz_eip7939_test_vectors() {
assert_eq!(clz_via_evm(input), expected);
}
}

#[test]
fn test_shl() {
Expand Down
1 change: 1 addition & 0 deletions actors/evm/src/interpreter/instructions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ def_primop! { BYTE(a, b) => bitwise::byte }
def_primop! { SHL(a, b) => bitwise::shl }
def_primop! { SHR(a, b) => bitwise::shr }
def_primop! { SAR(a, b) => bitwise::sar }
def_primop! { CLZ(a) => bitwise::clz }
// dup
def_stackop! { DUP1 => stack::dup::<1> }
def_stackop! { DUP2 => stack::dup::<2> }
Expand Down
60 changes: 60 additions & 0 deletions actors/evm/tests/eip7939_clz.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use fil_actor_evm::interpreter::opcodes;
use fil_actors_evm_shared::uints::U256;

mod test_vectors;
mod util;

fn initcode_for_runtime(runtime: &[u8]) -> Vec<u8> {
// Universal runtime constructor:
// https://github.com/wjmelements/evm/pull/87
const CONSTRUCTOR: [u8; 11] = [
opcodes::PUSH1,
0x0b,
opcodes::CODESIZE,
opcodes::SUB,
opcodes::DUP1,
opcodes::PUSH1,
0x0b,
opcodes::RETURNDATASIZE,
opcodes::CODECOPY,
opcodes::RETURNDATASIZE,
opcodes::RETURN,
];

let mut code = Vec::with_capacity(CONSTRUCTOR.len() + runtime.len());
code.extend_from_slice(&CONSTRUCTOR);
code.extend_from_slice(runtime);
code
}

fn clz_runtime() -> Vec<u8> {
// Reads a 32-byte word from calldata[0..32], computes CLZ, and returns the 32-byte result.
vec![
opcodes::PUSH0,
opcodes::CALLDATALOAD,
opcodes::CLZ,
opcodes::PUSH0,
opcodes::MSTORE,
opcodes::MSIZE,
opcodes::PUSH0,
opcodes::RETURN,
]
}

fn u256_be_bytes(value: U256) -> [u8; 32] {
let mut out = [0u8; 32];
value.write_as_big_endian(&mut out);
out
}

#[test]
fn eip7939_clz_vectors_end_to_end() {
let initcode = initcode_for_runtime(&clz_runtime());
let rt = util::construct_and_verify(initcode);

for (input, expected) in test_vectors::clz_eip7939_test_vectors() {
let ret = util::invoke_contract(&rt, &u256_be_bytes(input));
assert_eq!(ret.len(), 32);
assert_eq!(ret.as_slice(), &u256_be_bytes(expected));
}
}
14 changes: 14 additions & 0 deletions actors/evm/tests/test_vectors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use fil_actors_evm_shared::uints::U256;

/// EIP-7939 `CLZ` test vectors from https://eips.ethereum.org/EIPS/eip-7939
#[allow(dead_code)]
pub fn clz_eip7939_test_vectors() -> [(U256, U256); 6] {
[
(U256::ZERO, U256::from(256)),
(U256::ONE << 255, U256::ZERO),
(U256::MAX, U256::ZERO),
(U256::ONE << 254, U256::ONE),
((U256::ONE << 255) - U256::ONE, U256::ONE),
(U256::ONE, U256::from(255)),
]
}