Skip to content

Commit

Permalink
Merge pull request #3 from unzvfu/combine_arithmetic
Browse files Browse the repository at this point in the history 
Cleaner way to handle "simulated" operations SHL and SHR
  • Loading branch information
@Nashtare
Nashtare authored Sep 14, 2023
2 parents f0c920c + 5a7356f commit 18ad823
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 72 deletions.
22 changes: 8 additions & 14 deletions evm/src/arithmetic/arithmetic_stark.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ mod tests {
};

// 123 + 456 == 579
let add = Operation::binary(BinaryOperator::Add, U256::from(123), U256::from(456), false);
let add = Operation::binary(BinaryOperator::Add, U256::from(123), U256::from(456));
// (123 * 456) % 1007 == 703
let mulmod = Operation::ternary(
TernaryOperator::MulMod,
Expand All @@ -319,27 +319,22 @@ mod tests {
U256::from(1007),
);
// 123 * 456 == 56088
let mul = Operation::binary(BinaryOperator::Mul, U256::from(123), U256::from(456), false);
let mul = Operation::binary(BinaryOperator::Mul, U256::from(123), U256::from(456));
// 128 / 13 == 9
let div = Operation::binary(BinaryOperator::Div, U256::from(128), U256::from(13), false);
let div = Operation::binary(BinaryOperator::Div, U256::from(128), U256::from(13));

// 128 < 13 == 0
let lt1 = Operation::binary(BinaryOperator::Lt, U256::from(128), U256::from(13), false);
let lt1 = Operation::binary(BinaryOperator::Lt, U256::from(128), U256::from(13));
// 13 < 128 == 1
let lt2 = Operation::binary(BinaryOperator::Lt, U256::from(13), U256::from(128), false);
let lt2 = Operation::binary(BinaryOperator::Lt, U256::from(13), U256::from(128));
// 128 < 128 == 0
let lt3 = Operation::binary(BinaryOperator::Lt, U256::from(128), U256::from(128), false);
let lt3 = Operation::binary(BinaryOperator::Lt, U256::from(128), U256::from(128));

// 128 % 13 == 11
let modop = Operation::binary(BinaryOperator::Mod, U256::from(128), U256::from(13), false);
let modop = Operation::binary(BinaryOperator::Mod, U256::from(128), U256::from(13));

// byte(30, 0xABCD) = 0xAB
let byte = Operation::binary(
BinaryOperator::Byte,
U256::from(30),
U256::from(0xABCD),
false,
);
let byte = Operation::binary(BinaryOperator::Byte, U256::from(30), U256::from(0xABCD));

let ops: Vec<Operation> = vec![add, mulmod, addmod, mul, modop, lt1, lt2, lt3, div, byte];

Expand Down Expand Up @@ -402,7 +397,6 @@ mod tests {
BinaryOperator::Mul,
U256::from(rng.gen::<[u8; 32]>()),
U256::from(rng.gen::<[u8; 32]>()),
false,
)
})
.collect::<Vec<_>>();
Expand Down
77 changes: 32 additions & 45 deletions evm/src/arithmetic/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,17 @@ pub(crate) enum BinaryOperator {
MulFp254,
SubFp254,
Byte,
Shl, // simulated with MUL
Shr, // simulated with DIV
}

impl BinaryOperator {
pub(crate) fn result(&self, input0: U256, input1: U256) -> U256 {
match self {
BinaryOperator::Add => input0.overflowing_add(input1).0,
BinaryOperator::Mul => input0.overflowing_mul(input1).0,
BinaryOperator::Mul | BinaryOperator::Shl => input0.overflowing_mul(input1).0,
BinaryOperator::Sub => input0.overflowing_sub(input1).0,
BinaryOperator::Div => {
BinaryOperator::Div | BinaryOperator::Shr => {
if input1.is_zero() {
U256::zero()
} else {
Expand Down Expand Up @@ -64,31 +66,21 @@ impl BinaryOperator {
}
}

pub(crate) fn row_filter(&self, is_simulated: bool) -> usize {
pub(crate) fn row_filter(&self) -> usize {
match self {
BinaryOperator::Add => columns::IS_ADD,
BinaryOperator::Mul => {
if is_simulated {
columns::IS_SHL
} else {
columns::IS_MUL
}
}
BinaryOperator::Mul => columns::IS_MUL,
BinaryOperator::Sub => columns::IS_SUB,
BinaryOperator::Div => {
if is_simulated {
columns::IS_SHR
} else {
columns::IS_DIV
}
}
BinaryOperator::Div => columns::IS_DIV,
BinaryOperator::Mod => columns::IS_MOD,
BinaryOperator::Lt => columns::IS_LT,
BinaryOperator::Gt => columns::IS_GT,
BinaryOperator::AddFp254 => columns::IS_ADDFP254,
BinaryOperator::MulFp254 => columns::IS_MULFP254,
BinaryOperator::SubFp254 => columns::IS_SUBFP254,
BinaryOperator::Byte => columns::IS_BYTE,
BinaryOperator::Shl => columns::IS_SHL,
BinaryOperator::Shr => columns::IS_SHR,
}
}
}
Expand Down Expand Up @@ -120,18 +112,13 @@ impl TernaryOperator {
}

/// An enum representing arithmetic operations that can be either binary or ternary.
///
/// Binary operations include a special `is_simulated` flag to differentiate SHL
/// and SHR operations from MUL and DIV respectively, as the former are simulated
/// with the latter by scaling their inputs.
#[derive(Debug)]
pub(crate) enum Operation {
BinaryOperation {
operator: BinaryOperator,
input0: U256,
input1: U256,
result: U256,
is_simulated: bool,
},
TernaryOperation {
operator: TernaryOperator,
Expand All @@ -143,19 +130,28 @@ pub(crate) enum Operation {
}

impl Operation {
pub(crate) fn binary(
operator: BinaryOperator,
input0: U256,
input1: U256,
is_simulated: bool,
) -> Self {
/// Create a binary operator with given inputs.
///
/// NB: This works as you would expect, EXCEPT for SHL and SHR,
/// whose inputs need a small amount of preprocessing. Specifically,
/// to create `SHL(shift, value)`, call (note the reversal of
/// argument order):
///
/// `Operation::binary(BinaryOperator::Shl, value, 1 << shift)`
///
/// Similarly, to create `SHR(shift, value)`, call
///
/// `Operation::binary(BinaryOperator::Shr, value, 1 << shift)`
///
/// See witness/operation.rs::append_shift() for an example (indeed
/// the only call site for such inputs).
pub(crate) fn binary(operator: BinaryOperator, input0: U256, input1: U256) -> Self {
let result = operator.result(input0, input1);
Self::BinaryOperation {
operator,
input0,
input1,
result,
is_simulated,
}
}

Expand Down Expand Up @@ -199,8 +195,7 @@ impl Operation {
input0,
input1,
result,
is_simulated,
} => binary_op_to_rows(operator, is_simulated, input0, input1, result),
} => binary_op_to_rows(operator, input0, input1, result),
Operation::TernaryOperation {
operator,
input0,
Expand Down Expand Up @@ -231,37 +226,29 @@ fn ternary_op_to_rows<F: PrimeField64>(

fn binary_op_to_rows<F: PrimeField64>(
op: BinaryOperator,
is_simulated: bool,
input0: U256,
input1: U256,
result: U256,
) -> (Vec<F>, Option<Vec<F>>) {
let mut row = vec![F::ZERO; columns::NUM_ARITH_COLUMNS];
row[op.row_filter(is_simulated)] = F::ONE;
row[op.row_filter()] = F::ONE;

match op {
BinaryOperator::Add | BinaryOperator::Sub | BinaryOperator::Lt | BinaryOperator::Gt => {
addcy::generate(&mut row, op.row_filter(false), input0, input1);
addcy::generate(&mut row, op.row_filter(), input0, input1);
(row, None)
}
BinaryOperator::Mul => {
BinaryOperator::Mul | BinaryOperator::Shl => {
mul::generate(&mut row, input0, input1);
(row, None)
}
BinaryOperator::Div | BinaryOperator::Mod => {
BinaryOperator::Div | BinaryOperator::Mod | BinaryOperator::Shr => {
let mut nv = vec![F::ZERO; columns::NUM_ARITH_COLUMNS];
divmod::generate(
&mut row,
&mut nv,
op.row_filter(is_simulated),
input0,
input1,
result,
);
divmod::generate(&mut row, &mut nv, op.row_filter(), input0, input1, result);
(row, Some(nv))
}
BinaryOperator::AddFp254 | BinaryOperator::MulFp254 | BinaryOperator::SubFp254 => {
ternary_op_to_rows::<F>(op.row_filter(false), input0, input1, BN_BASE, result)
ternary_op_to_rows::<F>(op.row_filter(), input0, input1, BN_BASE, result)
}
BinaryOperator::Byte => {
byte::generate(&mut row, input0, input1);
Expand Down
4 changes: 2 additions & 2 deletions evm/src/witness/gas.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ pub(crate) fn gas_to_charge(op: Operation) -> u64 {
BinaryArithmetic(Lt) => G_VERYLOW,
BinaryArithmetic(Gt) => G_VERYLOW,
BinaryArithmetic(Byte) => G_VERYLOW,
Shl => G_VERYLOW,
Shr => G_VERYLOW,
BinaryArithmetic(Shl) => G_VERYLOW,
BinaryArithmetic(Shr) => G_VERYLOW,
BinaryArithmetic(AddFp254) => KERNEL_ONLY_INSTR,
BinaryArithmetic(MulFp254) => KERNEL_ONLY_INSTR,
BinaryArithmetic(SubFp254) => KERNEL_ONLY_INSTR,
Expand Down
10 changes: 4 additions & 6 deletions evm/src/witness/operation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ use crate::{arithmetic, logic};
pub(crate) enum Operation {
Iszero,
Not,
Shl,
Shr,
Syscall(u8, usize, bool), // (syscall number, minimum stack length, increases stack length)
Eq,
BinaryLogic(logic::Op),
Expand Down Expand Up @@ -79,7 +77,7 @@ pub(crate) fn generate_binary_arithmetic_op<F: Field>(
) -> Result<(), ProgramError> {
let [(input0, log_in0), (input1, log_in1)] =
stack_pop_with_log_and_fill::<2, _>(state, &mut row)?;
let operation = arithmetic::Operation::binary(operator, input0, input1, false);
let operation = arithmetic::Operation::binary(operator, input0, input1);
let log_out = stack_push_log_and_fill(state, &mut row, operation.result())?;

if operator == arithmetic::BinaryOperator::AddFp254
Expand Down Expand Up @@ -502,11 +500,11 @@ fn append_shift<F: Field>(
U256::one() << input0
};
let operator = if is_shl {
BinaryOperator::Mul
BinaryOperator::Shl
} else {
BinaryOperator::Div
BinaryOperator::Shr
};
let operation = arithmetic::Operation::binary(operator, input1, input0, true);
let operation = arithmetic::Operation::binary(operator, input1, input0);

state.traces.push_arithmetic(operation);
state.traces.push_memory(log_in0);
Expand Down
11 changes: 6 additions & 5 deletions evm/src/witness/transition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ fn decode(registers: RegistersState, opcode: u8) -> Result<Operation, ProgramErr
(0x1a, _) => Ok(Operation::BinaryArithmetic(
arithmetic::BinaryOperator::Byte,
)),
(0x1b, _) => Ok(Operation::Shl),
(0x1c, _) => Ok(Operation::Shr),
(0x1b, _) => Ok(Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shl)),
(0x1c, _) => Ok(Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shr)),
(0x1d, _) => Ok(Operation::Syscall(opcode, 2, false)), // SAR
(0x20, _) => Ok(Operation::Syscall(opcode, 2, false)), // KECCAK256
(0x21, true) => Ok(Operation::KeccakGeneral),
Expand Down Expand Up @@ -165,8 +165,9 @@ fn fill_op_flag<F: Field>(op: Operation, row: &mut CpuColumnsView<F>) {
Operation::BinaryArithmetic(arithmetic::BinaryOperator::AddFp254)
| Operation::BinaryArithmetic(arithmetic::BinaryOperator::MulFp254)
| Operation::BinaryArithmetic(arithmetic::BinaryOperator::SubFp254) => &mut flags.fp254_op,
Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shl)
| Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shr) => &mut flags.shift,
Operation::BinaryArithmetic(_) => &mut flags.binary_op,
Operation::Shl | Operation::Shr => &mut flags.shift,
Operation::TernaryArithmetic(_) => &mut flags.ternary_op,
Operation::KeccakGeneral => &mut flags.keccak_general,
Operation::ProverInput => &mut flags.prover_input,
Expand Down Expand Up @@ -194,8 +195,8 @@ fn perform_op<F: Field>(
Operation::Swap(n) => generate_swap(n, state, row)?,
Operation::Iszero => generate_iszero(state, row)?,
Operation::Not => generate_not(state, row)?,
Operation::Shl => generate_shl(state, row)?,
Operation::Shr => generate_shr(state, row)?,
Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shl) => generate_shl(state, row)?,
Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shr) => generate_shr(state, row)?,
Operation::Syscall(opcode, stack_values_read, stack_len_increased) => {
generate_syscall(opcode, stack_values_read, stack_len_increased, state, row)?
}
Expand Down

0 comments on commit 18ad823

Please sign in to comment.