From 070a39e11b007d77f6ba3aa732fa291b7126f240 Mon Sep 17 00:00:00 2001 From: Linda Guiga Date: Mon, 25 Sep 2023 13:28:16 -0400 Subject: [PATCH 1/6] Combine mload_32bytes and mstore_32bytes flags --- evm/src/all_stark.rs | 16 ++----- evm/src/cpu/columns/ops.rs | 3 +- evm/src/cpu/cpu_stark.rs | 32 ++------------ evm/src/cpu/decode.rs | 39 ++++++++++++++--- evm/src/cpu/gas.rs | 3 +- evm/src/cpu/kernel/asm/memory/packing.asm | 4 +- evm/src/cpu/kernel/asm/memory/syscalls.asm | 1 + evm/src/cpu/memio.rs | 50 ++++++++++++++++++++++ evm/src/cpu/stack.rs | 22 +++++----- evm/src/witness/operation.rs | 2 +- evm/src/witness/transition.rs | 3 +- 11 files changed, 109 insertions(+), 66 deletions(-) diff --git a/evm/src/all_stark.rs b/evm/src/all_stark.rs index 068b0bcbf9..50d2f8e29c 100644 --- a/evm/src/all_stark.rs +++ b/evm/src/all_stark.rs @@ -125,25 +125,17 @@ fn ctl_arithmetic() -> CrossTableLookup { } fn ctl_byte_packing() -> CrossTableLookup { - let cpu_packing_looking = TableWithColumns::new( + let cpu_byte32_looking = TableWithColumns::new( Table::Cpu, - cpu_stark::ctl_data_byte_packing(), - Some(cpu_stark::ctl_filter_byte_packing()), - ); - let cpu_unpacking_looking = TableWithColumns::new( - Table::Cpu, - cpu_stark::ctl_data_byte_unpacking(), - Some(cpu_stark::ctl_filter_byte_unpacking()), + cpu_stark::ctl_data_byte32(), + Some(cpu_stark::ctl_filter_byte32()), ); let byte_packing_looked = TableWithColumns::new( Table::BytePacking, byte_packing_stark::ctl_looked_data(), Some(byte_packing_stark::ctl_looked_filter()), ); - CrossTableLookup::new( - vec![cpu_packing_looking, cpu_unpacking_looking], - byte_packing_looked, - ) + CrossTableLookup::new(vec![cpu_byte32_looking], byte_packing_looked) } fn ctl_keccak() -> CrossTableLookup { diff --git a/evm/src/cpu/columns/ops.rs b/evm/src/cpu/columns/ops.rs index d4d753f7cf..0d6db44636 100644 --- a/evm/src/cpu/columns/ops.rs +++ b/evm/src/cpu/columns/ops.rs @@ -25,8 +25,7 @@ pub struct OpsColumnsView { pub dup: T, pub swap: T, pub context_op: T, - pub mstore_32bytes: T, - pub mload_32bytes: T, + pub memop_32bytes: T, pub exit_kernel: T, pub m_op_general: T, diff --git a/evm/src/cpu/cpu_stark.rs b/evm/src/cpu/cpu_stark.rs index f23ff308b6..691116cdb2 100644 --- a/evm/src/cpu/cpu_stark.rs +++ b/evm/src/cpu/cpu_stark.rs @@ -125,38 +125,12 @@ pub fn ctl_arithmetic_shift_rows() -> TableWithColumns { TableWithColumns::new(Table::Cpu, columns, Some(Column::single(COL_MAP.op.shift))) } -pub fn ctl_data_byte_packing() -> Vec> { +pub fn ctl_data_byte32() -> Vec> { ctl_data_keccak_sponge() } -pub fn ctl_filter_byte_packing() -> Column { - Column::single(COL_MAP.op.mload_32bytes) -} - -pub fn ctl_data_byte_unpacking() -> Vec> { - // When executing MSTORE_32BYTES, the GP memory channels are used as follows: - // GP channel 0: stack[-1] = context - // GP channel 1: stack[-2] = segment - // GP channel 2: stack[-3] = virt - // GP channel 3: stack[-4] = val - // GP channel 4: stack[-5] = len - let context = Column::single(COL_MAP.mem_channels[0].value[0]); - let segment = Column::single(COL_MAP.mem_channels[1].value[0]); - let virt = Column::single(COL_MAP.mem_channels[2].value[0]); - let val = Column::singles(COL_MAP.mem_channels[3].value); - let len = Column::single(COL_MAP.mem_channels[4].value[0]); - - let num_channels = F::from_canonical_usize(NUM_CHANNELS); - let timestamp = Column::linear_combination([(COL_MAP.clock, num_channels)]); - - let mut res = vec![context, segment, virt, len, timestamp]; - res.extend(val); - - res -} - -pub fn ctl_filter_byte_unpacking() -> Column { - Column::single(COL_MAP.op.mstore_32bytes) +pub fn ctl_filter_byte32() -> Column { + Column::single(COL_MAP.op.memop_32bytes) } pub const MEM_CODE_CHANNEL_IDX: usize = 0; diff --git a/evm/src/cpu/decode.rs b/evm/src/cpu/decode.rs index c1c43a0bb1..7b02826f7c 100644 --- a/evm/src/cpu/decode.rs +++ b/evm/src/cpu/decode.rs @@ -23,7 +23,7 @@ use crate::cpu::columns::{CpuColumnsView, COL_MAP}; /// behavior. /// Note: invalid opcodes are not represented here. _Any_ opcode is permitted to decode to /// `is_invalid`. The kernel then verifies that the opcode was _actually_ invalid. -const OPCODES: [(u8, usize, bool, usize); 16] = [ +const OPCODES: [(u8, usize, bool, usize); 14] = [ // (start index of block, number of top bits to check (log2), kernel-only, flag column) // ADD, MUL, SUB, DIV, MOD, LT, GT and BYTE flags are handled partly manually here, and partly through the Arithmetic table CTL. // ADDMOD, MULMOD and SUBMOD flags are handled partly manually here, and partly through the Arithmetic table CTL. @@ -39,14 +39,13 @@ const OPCODES: [(u8, usize, bool, usize); 16] = [ (0x58, 0, false, COL_MAP.op.pc), (0x5b, 0, false, COL_MAP.op.jumpdest), (0x5f, 0, false, COL_MAP.op.push0), - (0x60, 5, false, COL_MAP.op.push), // 0x60-0x7f - (0x80, 4, false, COL_MAP.op.dup), // 0x80-0x8f - (0x90, 4, false, COL_MAP.op.swap), // 0x90-0x9f - (0xee, 0, true, COL_MAP.op.mstore_32bytes), + (0x60, 5, false, COL_MAP.op.push), // 0x60-0x7f + (0x80, 4, false, COL_MAP.op.dup), // 0x80-0x8f + (0x90, 4, false, COL_MAP.op.swap), // 0x90-0x9f (0xf6, 1, true, COL_MAP.op.context_op), // 0xf6-0xf7 - (0xf8, 0, true, COL_MAP.op.mload_32bytes), (0xf9, 0, true, COL_MAP.op.exit_kernel), // MLOAD_GENERAL and MSTORE_GENERAL flags are handled manually here. + // MLOAD_32BYTES and MSTORE_32BYTES flags are handled manually here. ]; /// List of combined opcodes requiring a special handling. @@ -104,6 +103,10 @@ pub fn generate(lv: &mut CpuColumnsView) { if opcode == 0xfb || opcode == 0xfc { lv.op.m_op_general = F::from_bool(kernel); } + + if opcode == 0xee || opcode == 0xf8 { + lv.op.memop_32bytes = F::from_bool(kernel); + } } /// Break up an opcode (which is 8 bits long) into its eight bits. @@ -192,6 +195,13 @@ pub fn eval_packed_generic( * (opcode - P::Scalar::from_canonical_usize(0xfc_usize)) * lv.op.m_op_general; yield_constr.constraint(m_op_constr); + + // Manually check lv.memop_32bytes. + yield_constr.constraint((P::ONES - kernel_mode) * lv.op.memop_32bytes); + let memop_32bytes_constr = (opcode - P::Scalar::from_canonical_usize(0xee_usize)) + * (opcode - P::Scalar::from_canonical_usize(0xf8_usize)) + * lv.op.memop_32bytes; + yield_constr.constraint(memop_32bytes_constr); } pub fn eval_ext_circuit, const D: usize>( @@ -294,4 +304,21 @@ pub fn eval_ext_circuit, const D: usize>( m_op_constr = builder.mul_extension(m_op_constr, lv.op.m_op_general); yield_constr.constraint(builder, m_op_constr); + + // Manually check lv.memop_32bytes. + let mload_32bytes_opcode = + builder.constant_extension(F::Extension::from_canonical_usize(0xf8_usize)); + let mstore_32bytes_opcode = + builder.constant_extension(F::Extension::from_canonical_usize(0xee_usize)); + + let constr = builder.mul_extension(is_not_kernel_mode, lv.op.memop_32bytes); + yield_constr.constraint(builder, constr); + + let mload_32bytes_constr = builder.sub_extension(opcode, mload_32bytes_opcode); + let mstore_32bytes_constr = builder.sub_extension(opcode, mstore_32bytes_opcode); + let mut memop_32bytes_constr = + builder.mul_extension(mstore_32bytes_constr, mload_32bytes_constr); + memop_32bytes_constr = builder.mul_extension(memop_32bytes_constr, lv.op.memop_32bytes); + + yield_constr.constraint(builder, memop_32bytes_constr); } diff --git a/evm/src/cpu/gas.rs b/evm/src/cpu/gas.rs index 51f375c056..dc2a169542 100644 --- a/evm/src/cpu/gas.rs +++ b/evm/src/cpu/gas.rs @@ -37,8 +37,7 @@ const SIMPLE_OPCODES: OpsColumnsView> = OpsColumnsView { dup: G_VERYLOW, swap: G_VERYLOW, context_op: KERNEL_ONLY_INSTR, - mstore_32bytes: KERNEL_ONLY_INSTR, - mload_32bytes: KERNEL_ONLY_INSTR, + memop_32bytes: KERNEL_ONLY_INSTR, exit_kernel: None, m_op_general: KERNEL_ONLY_INSTR, syscall: None, diff --git a/evm/src/cpu/kernel/asm/memory/packing.asm b/evm/src/cpu/kernel/asm/memory/packing.asm index 1dbbf39362..cf9d6234a7 100644 --- a/evm/src/cpu/kernel/asm/memory/packing.asm +++ b/evm/src/cpu/kernel/asm/memory/packing.asm @@ -42,8 +42,8 @@ global mload_packing_u64_LE: // Post stack: offset' global mstore_unpacking: // stack: context, segment, offset, value, len, retdest - %stack(context, segment, offset, value, len, retdest) -> (context, segment, offset, value, len, offset, len, retdest) - // stack: context, segment, offset, value, len, offset, len, retdest + %stack(context, segment, offset, value, len, retdest) -> (context, segment, offset, len, value, offset, len, retdest) + // stack: context, segment, offset, len, value, offset, len, retdest MSTORE_32BYTES // stack: offset, len, retdest ADD SWAP1 diff --git a/evm/src/cpu/kernel/asm/memory/syscalls.asm b/evm/src/cpu/kernel/asm/memory/syscalls.asm index 3548930c36..a5c49360ba 100644 --- a/evm/src/cpu/kernel/asm/memory/syscalls.asm +++ b/evm/src/cpu/kernel/asm/memory/syscalls.asm @@ -30,6 +30,7 @@ global sys_mstore: PUSH @SEGMENT_MAIN_MEMORY GET_CONTEXT // stack: addr: 3, value, len, kexit_info + %stack (ADDR: 3, value, len, kexit_info) -> (ADDR, len, value, kexit_info) MSTORE_32BYTES // stack: kexit_info EXIT_KERNEL diff --git a/evm/src/cpu/memio.rs b/evm/src/cpu/memio.rs index aa3749cab2..5b9aa305ca 100644 --- a/evm/src/cpu/memio.rs +++ b/evm/src/cpu/memio.rs @@ -50,6 +50,18 @@ fn eval_packed_load( stack::MLOAD_GENERAL_OP.unwrap(), yield_constr, ); + + // Check the stack for MLOAD_32BYTES. + // The second bit (in little-endian) of MLOAD_32BYTES is 0. + let filter = lv.op.memop_32bytes * (P::ONES - lv.opcode_bits[1]); + + stack::eval_packed_one( + lv, + nv, + filter, + stack::MLOAD_32BYTES_OP.unwrap(), + yield_constr, + ); } fn eval_ext_circuit_load, const D: usize>( @@ -106,6 +118,20 @@ fn eval_ext_circuit_load, const D: usize>( stack::MLOAD_GENERAL_OP.unwrap(), yield_constr, ); + + // Check the stack for MLOAD_32BYTES. + let one = builder.one_extension(); + let mut filter = builder.sub_extension(one, lv.opcode_bits[1]); + filter = builder.mul_extension(lv.op.memop_32bytes, filter); + + stack::eval_ext_circuit_one( + builder, + lv, + nv, + filter, + stack::MLOAD_32BYTES_OP.unwrap(), + yield_constr, + ); } fn eval_packed_store( @@ -141,6 +167,18 @@ fn eval_packed_store( stack::MSTORE_GENERAL_OP.unwrap(), yield_constr, ); + + // Check the stack for MSTORE_32BYTES. + // The second bit (in little-endian) of MSTORE_32BYTES is 1. + let filter = lv.op.memop_32bytes * lv.opcode_bits[1]; + + stack::eval_packed_one( + lv, + nv, + filter, + stack::MSTORE_32BYTES_OP.unwrap(), + yield_constr, + ); } fn eval_ext_circuit_store, const D: usize>( @@ -199,6 +237,18 @@ fn eval_ext_circuit_store, const D: usize>( stack::MSTORE_GENERAL_OP.unwrap(), yield_constr, ); + + // Check the stack for MSTORE_32BYTES. + let filter = builder.mul_extension(lv.op.memop_32bytes, lv.opcode_bits[1]); + + stack::eval_ext_circuit_one( + builder, + lv, + nv, + filter, + stack::MSTORE_32BYTES_OP.unwrap(), + yield_constr, + ); } pub fn eval_packed( diff --git a/evm/src/cpu/stack.rs b/evm/src/cpu/stack.rs index 28abf077cb..622d07d26b 100644 --- a/evm/src/cpu/stack.rs +++ b/evm/src/cpu/stack.rs @@ -56,6 +56,17 @@ pub(crate) const MSTORE_GENERAL_OP: Option = Some(StackBehavior { disable_other_channels: false, }); +pub(crate) const MLOAD_32BYTES_OP: Option = Some(StackBehavior { + num_pops: 4, + pushes: true, + disable_other_channels: false, +}); + +pub(crate) const MSTORE_32BYTES_OP: Option = Some(StackBehavior { + num_pops: 5, + pushes: false, + disable_other_channels: false, +}); // AUDITORS: If the value below is `None`, then the operation must be manually checked to ensure // that every general-purpose memory channel is either disabled or has its read flag and address // propertly constrained. The same applies when `disable_other_channels` is set to `false`, @@ -104,16 +115,7 @@ const STACK_BEHAVIORS: OpsColumnsView> = OpsColumnsView { dup: None, swap: None, context_op: None, // SET_CONTEXT is special since it involves the old and the new stack. - mstore_32bytes: Some(StackBehavior { - num_pops: 5, - pushes: false, - disable_other_channels: false, - }), - mload_32bytes: Some(StackBehavior { - num_pops: 4, - pushes: true, - disable_other_channels: false, - }), + memop_32bytes: None, exit_kernel: Some(StackBehavior { num_pops: 1, pushes: false, diff --git a/evm/src/witness/operation.rs b/evm/src/witness/operation.rs index 8349d56dfd..af0c534cb7 100644 --- a/evm/src/witness/operation.rs +++ b/evm/src/witness/operation.rs @@ -760,7 +760,7 @@ pub(crate) fn generate_mstore_32bytes( state: &mut GenerationState, mut row: CpuColumnsView, ) -> Result<(), ProgramError> { - let [(context, log_in0), (segment, log_in1), (base_virt, log_in2), (val, log_in3), (len, log_in4)] = + let [(context, log_in0), (segment, log_in1), (base_virt, log_in2), (len, log_in3), (val, log_in4)] = stack_pop_with_log_and_fill::<5, _>(state, &mut row)?; let len = len.as_usize(); diff --git a/evm/src/witness/transition.rs b/evm/src/witness/transition.rs index 1418beba8d..3cad8e3db4 100644 --- a/evm/src/witness/transition.rs +++ b/evm/src/witness/transition.rs @@ -176,8 +176,7 @@ fn fill_op_flag(op: Operation, row: &mut CpuColumnsView) { Operation::Pc => &mut flags.pc, Operation::Jumpdest => &mut flags.jumpdest, Operation::GetContext | Operation::SetContext => &mut flags.context_op, - Operation::Mload32Bytes => &mut flags.mload_32bytes, - Operation::Mstore32Bytes => &mut flags.mstore_32bytes, + Operation::Mload32Bytes | Operation::Mstore32Bytes => &mut flags.memop_32bytes, Operation::ExitKernel => &mut flags.exit_kernel, Operation::MloadGeneral | Operation::MstoreGeneral => &mut flags.m_op_general, } = F::ONE; From 2ca7fdb07ccd800db680aba7ba48de5e2800c4fb Mon Sep 17 00:00:00 2001 From: Linda Guiga <101227802+LindaGuiga@users.noreply.github.com> Date: Tue, 26 Sep 2023 03:59:57 +0100 Subject: [PATCH 2/6] Fix run_mstore_u32bytes in interpreter --- evm/src/cpu/kernel/interpreter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index c4deba99c5..682bf7260c 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -1066,8 +1066,8 @@ impl<'a> Interpreter<'a> { let context = self.pop().as_usize(); let segment = Segment::all()[self.pop().as_usize()]; let offset = self.pop().as_usize(); - let value = self.pop(); let len = self.pop().as_usize(); + let value = self.pop(); let mut bytes = vec![0; 32]; value.to_little_endian(&mut bytes); From 1b05c520713161c779faad82e7c4d42aa87fa818 Mon Sep 17 00:00:00 2001 From: Linda Guiga Date: Tue, 26 Sep 2023 13:00:07 -0400 Subject: [PATCH 3/6] Add memop_32bytes to list of combined flags in decode.rs --- evm/src/cpu/decode.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/evm/src/cpu/decode.rs b/evm/src/cpu/decode.rs index 7b02826f7c..54dc3ec258 100644 --- a/evm/src/cpu/decode.rs +++ b/evm/src/cpu/decode.rs @@ -51,13 +51,14 @@ const OPCODES: [(u8, usize, bool, usize); 14] = [ /// List of combined opcodes requiring a special handling. /// Each index in the list corresponds to an arbitrary combination /// of opcodes defined in evm/src/cpu/columns/ops.rs. -const COMBINED_OPCODES: [usize; 6] = [ +const COMBINED_OPCODES: [usize; 7] = [ COL_MAP.op.logic_op, COL_MAP.op.fp254_op, COL_MAP.op.binary_op, COL_MAP.op.ternary_op, COL_MAP.op.shift, COL_MAP.op.m_op_general, + COL_MAP.op.memop_32bytes, ]; pub fn generate(lv: &mut CpuColumnsView) { From f83828270a15bd0ce186e89ac9aa3327878449b2 Mon Sep 17 00:00:00 2001 From: Linda Guiga Date: Tue, 7 Nov 2023 12:12:58 -0500 Subject: [PATCH 4/6] Change the order of the stack for mstore_unpacking and mstore_unpacking_rlp --- .../cpu/kernel/asm/core/create_addresses.asm | 6 +++--- .../cpu/kernel/asm/core/precompiles/bn_add.asm | 4 ++-- .../cpu/kernel/asm/core/precompiles/bn_mul.asm | 4 ++-- .../cpu/kernel/asm/core/precompiles/ecrec.asm | 2 +- .../cpu/kernel/asm/core/precompiles/expmod.asm | 4 ++-- .../cpu/kernel/asm/core/precompiles/rip160.asm | 2 +- .../cpu/kernel/asm/core/precompiles/sha256.asm | 2 +- .../cpu/kernel/asm/core/precompiles/snarkv.asm | 2 +- evm/src/cpu/kernel/asm/main.asm | 7 +++---- evm/src/cpu/kernel/asm/memory/core.asm | 2 +- evm/src/cpu/kernel/asm/memory/packing.asm | 6 +++--- evm/src/cpu/kernel/asm/mpt/hash/hash.asm | 8 ++++---- evm/src/cpu/kernel/asm/rlp/encode.asm | 17 ++++++++--------- .../cpu/kernel/asm/rlp/encode_rlp_string.asm | 2 +- evm/src/cpu/kernel/asm/util/keccak.asm | 6 +++--- evm/src/cpu/kernel/tests/packing.rs | 4 ++-- 16 files changed, 38 insertions(+), 40 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/create_addresses.asm b/evm/src/cpu/kernel/asm/core/create_addresses.asm index 70f57b6f0b..5fb2138b41 100644 --- a/evm/src/cpu/kernel/asm/core/create_addresses.asm +++ b/evm/src/cpu/kernel/asm/core/create_addresses.asm @@ -41,15 +41,15 @@ global get_create_address: global get_create2_address: // stack: sender, code_hash, salt, retdest PUSH 0xff PUSH 0 %mstore_kernel_general - %stack (sender, code_hash, salt, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 1, sender, 20, get_create2_address_contd, salt, code_hash, retdest) + %stack (sender, code_hash, salt, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 1, 20, sender, get_create2_address_contd, salt, code_hash, retdest) %jump(mstore_unpacking) get_create2_address_contd: POP - %stack (salt, code_hash, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 21, salt, 32, get_create2_address_contd2, code_hash, retdest) + %stack (salt, code_hash, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 21, 32, salt, get_create2_address_contd2, code_hash, retdest) %jump(mstore_unpacking) get_create2_address_contd2: POP - %stack (code_hash, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 53, code_hash, 32, get_create2_address_finish, retdest) + %stack (code_hash, retdest) -> (0, @SEGMENT_KERNEL_GENERAL, 53, 32, code_hash, get_create2_address_finish, retdest) %jump(mstore_unpacking) get_create2_address_finish: POP diff --git a/evm/src/cpu/kernel/asm/core/precompiles/bn_add.asm b/evm/src/cpu/kernel/asm/core/precompiles/bn_add.asm index 1dafbe8a43..725cdbfbf0 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/bn_add.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/bn_add.asm @@ -49,9 +49,9 @@ bn_add_return: // Store the result (x, y) to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 64) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, x, y) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, x, 32, bn_add_contd6, parent_ctx, y) + %stack (parent_ctx, x, y) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, x, bn_add_contd6, parent_ctx, y) %jump(mstore_unpacking) bn_add_contd6: POP - %stack (parent_ctx, y) -> (parent_ctx, @SEGMENT_RETURNDATA, 32, y, 32, pop_and_return_success) + %stack (parent_ctx, y) -> (parent_ctx, @SEGMENT_RETURNDATA, 32, 32, y, pop_and_return_success) %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/core/precompiles/bn_mul.asm b/evm/src/cpu/kernel/asm/core/precompiles/bn_mul.asm index b3865506d8..a08d8b242e 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/bn_mul.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/bn_mul.asm @@ -44,9 +44,9 @@ bn_mul_return: // Store the result (Px, Py) to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 64) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, Px, Py) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, Px, 32, bn_mul_contd6, parent_ctx, Py) + %stack (parent_ctx, Px, Py) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, Px, bn_mul_contd6, parent_ctx, Py) %jump(mstore_unpacking) bn_mul_contd6: POP - %stack (parent_ctx, Py) -> (parent_ctx, @SEGMENT_RETURNDATA, 32, Py, 32, pop_and_return_success) + %stack (parent_ctx, Py) -> (parent_ctx, @SEGMENT_RETURNDATA, 32, 32, Py, pop_and_return_success) %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/core/precompiles/ecrec.asm b/evm/src/cpu/kernel/asm/core/precompiles/ecrec.asm index b38307c4db..5e06a44a21 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/ecrec.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/ecrec.asm @@ -45,7 +45,7 @@ ecrec_return: // Store the result address to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 32) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, address) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, address, 32, pop_and_return_success) + %stack (parent_ctx, address) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, address, pop_and_return_success) %jump(mstore_unpacking) // On bad input, return empty return data but still return success. diff --git a/evm/src/cpu/kernel/asm/core/precompiles/expmod.asm b/evm/src/cpu/kernel/asm/core/precompiles/expmod.asm index 2185ee2c9d..0eac7cbe14 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/expmod.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/expmod.asm @@ -413,7 +413,7 @@ expmod_contd: // stack: cur_address=out+l_M_128-1, end_address=out-1, l_M_128, l_M%16, kexit_info DUP1 %mload_current_general %stack (cur_limb, cur_address, end_address, l_M_128, l_M_mod16, kexit_info) -> - (@SEGMENT_RETURNDATA, 0, cur_limb, l_M_mod16, cur_address, end_address, l_M_128, kexit_info) + (@SEGMENT_RETURNDATA, 0, l_M_mod16, cur_limb, cur_address, end_address, l_M_128, kexit_info) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) %mstore_unpacking // stack: offset, cur_address, end_address, l_M_128, kexit_info @@ -428,7 +428,7 @@ expmod_store_loop: DUP1 %mload_current_general %stack (cur_limb, cur_address, offset, end_address, l_M_128, kexit_info) -> (offset, cur_limb, cur_address, end_address, l_M_128, kexit_info) - %stack (offset, cur_limb) -> (@SEGMENT_RETURNDATA, offset, cur_limb, 16) + %stack (offset, cur_limb) -> (@SEGMENT_RETURNDATA, offset, 16, cur_limb) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) %mstore_unpacking // stack: offset', cur_address, end_address, l_M_128, kexit_info) diff --git a/evm/src/cpu/kernel/asm/core/precompiles/rip160.asm b/evm/src/cpu/kernel/asm/core/precompiles/rip160.asm index 20ea42cb58..f04f51e332 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/rip160.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/rip160.asm @@ -54,5 +54,5 @@ rip160_contd: // Store the result hash to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 32) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, hash) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, hash, 32, pop_and_return_success) + %stack (parent_ctx, hash) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, hash, pop_and_return_success) %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/core/precompiles/sha256.asm b/evm/src/cpu/kernel/asm/core/precompiles/sha256.asm index 5987bf0f9d..ca23a33e0d 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/sha256.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/sha256.asm @@ -59,5 +59,5 @@ sha256_contd: // Store the result hash to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 32) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, hash) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, hash, 32, pop_and_return_success) + %stack (parent_ctx, hash) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, hash, pop_and_return_success) %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/core/precompiles/snarkv.asm b/evm/src/cpu/kernel/asm/core/precompiles/snarkv.asm index f128cd51ad..cc40602362 100644 --- a/evm/src/cpu/kernel/asm/core/precompiles/snarkv.asm +++ b/evm/src/cpu/kernel/asm/core/precompiles/snarkv.asm @@ -118,5 +118,5 @@ got_result: // Store the result bool (repr. by a U256) to the parent's return data using `mstore_unpacking`. %mstore_parent_context_metadata(@CTX_METADATA_RETURNDATA_SIZE, 32) %mload_context_metadata(@CTX_METADATA_PARENT_CONTEXT) - %stack (parent_ctx, address) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, address, 32, pop_and_return_success) + %stack (parent_ctx, address) -> (parent_ctx, @SEGMENT_RETURNDATA, 0, 32, address, pop_and_return_success) %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/main.asm b/evm/src/cpu/kernel/asm/main.asm index 612b4bcecd..8dffc7070c 100644 --- a/evm/src/cpu/kernel/asm/main.asm +++ b/evm/src/cpu/kernel/asm/main.asm @@ -55,11 +55,10 @@ initialize_block_bloom: initialize_bloom_loop: // stack: i, len, offset, retdest DUP2 DUP2 EQ %jumpi(initialize_bloom_loop_end) - PUSH 32 // Bloom word length - // stack: word_len, i, len, offset, retdest // Load the next `block_bloom_before` word. - DUP2 %add_const(8) %mload_kernel(@SEGMENT_GLOBAL_BLOCK_BLOOM) - // stack: bloom_word, word_len, i, len, offset, retdest + DUP1 %add_const(8) %mload_kernel(@SEGMENT_GLOBAL_BLOCK_BLOOM) + PUSH 32 // Bloom word length + // stack: word_len, bloom_word, i, len, offset, retdest DUP5 PUSH @SEGMENT_BLOCK_BLOOM PUSH 0 // Bloom word address in SEGMENT_BLOCK_BLOOM %mstore_unpacking // stack: new_offset, i, len, old_offset, retdest diff --git a/evm/src/cpu/kernel/asm/memory/core.asm b/evm/src/cpu/kernel/asm/memory/core.asm index 41d4927bf7..87e3553d28 100644 --- a/evm/src/cpu/kernel/asm/memory/core.asm +++ b/evm/src/cpu/kernel/asm/memory/core.asm @@ -71,7 +71,7 @@ // Store a big-endian u32, consisting of 4 bytes (c_3, c_2, c_1, c_0). %macro mstore_u32 // stack: context, segment, offset, value - %stack (addr: 3, value) -> (addr, value, 4, %%after) + %stack (addr: 3, value) -> (addr, 4, value, %%after) %jump(mstore_unpacking) %%after: // stack: offset diff --git a/evm/src/cpu/kernel/asm/memory/packing.asm b/evm/src/cpu/kernel/asm/memory/packing.asm index cf9d6234a7..9cc726ee73 100644 --- a/evm/src/cpu/kernel/asm/memory/packing.asm +++ b/evm/src/cpu/kernel/asm/memory/packing.asm @@ -41,8 +41,8 @@ global mload_packing_u64_LE: // Pre stack: context, segment, offset, value, len, retdest // Post stack: offset' global mstore_unpacking: - // stack: context, segment, offset, value, len, retdest - %stack(context, segment, offset, value, len, retdest) -> (context, segment, offset, len, value, offset, len, retdest) + // stack: context, segment, offset, len, value, retdest + %stack(context, segment, offset, len, value, retdest) -> (context, segment, offset, len, value, offset, len, retdest) // stack: context, segment, offset, len, value, offset, len, retdest MSTORE_32BYTES // stack: offset, len, retdest @@ -51,7 +51,7 @@ global mstore_unpacking: JUMP %macro mstore_unpacking - %stack (addr: 3, value, len) -> (addr, value, len, %%after) + %stack (addr: 3, len, value) -> (addr, len, value, %%after) %jump(mstore_unpacking) %%after: %endmacro diff --git a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm index 4209f06c75..75158efa6d 100644 --- a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm +++ b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm @@ -24,8 +24,8 @@ mpt_hash_hash_if_rlp: mpt_hash_hash_rlp: // stack: result, result_len, retdest %stack (result, result_len) - // context, segment, offset, value, len, retdest - -> (0, @SEGMENT_RLP_RAW, 0, result, result_len, mpt_hash_hash_rlp_after_unpacking) + // context, segment, offset, len, value, retdest + -> (0, @SEGMENT_RLP_RAW, 0, result_len, result, mpt_hash_hash_rlp_after_unpacking) %jump(mstore_unpacking) mpt_hash_hash_rlp_after_unpacking: // stack: result_len, retdest @@ -226,7 +226,7 @@ encode_node_branch_prepend_prefix: SWAP2 %increment SWAP2 // rlp_pos += 1 %%unpack: %stack (result_len, result, rlp_pos, rlp_start, base_offset, node_payload_ptr, encode_value, retdest) - -> (rlp_pos, result, result_len, %%after_unpacking, + -> (rlp_pos, result_len, result, %%after_unpacking, rlp_start, base_offset, node_payload_ptr, encode_value, retdest) %jump(mstore_unpacking_rlp) %%after_unpacking: @@ -265,7 +265,7 @@ encode_node_extension_after_hex_prefix: %increment // rlp_pos += 1 encode_node_extension_unpack: %stack (rlp_pos, rlp_start, result, result_len, node_payload_ptr) - -> (rlp_pos, result, result_len, encode_node_extension_after_unpacking, rlp_start) + -> (rlp_pos, result_len, result, encode_node_extension_after_unpacking, rlp_start) %jump(mstore_unpacking_rlp) encode_node_extension_after_unpacking: // stack: rlp_pos, rlp_start, retdest diff --git a/evm/src/cpu/kernel/asm/rlp/encode.asm b/evm/src/cpu/kernel/asm/rlp/encode.asm index fd42fe5221..1dec6a07c8 100644 --- a/evm/src/cpu/kernel/asm/rlp/encode.asm +++ b/evm/src/cpu/kernel/asm/rlp/encode.asm @@ -39,8 +39,8 @@ global encode_rlp_fixed: SWAP1 %increment // increment pos // stack: pos, len, string, retdest - %stack (pos, len, string) -> (pos, string, len, encode_rlp_fixed_finish) - // stack: context, segment, pos, string, len, encode_rlp_fixed_finish, retdest + %stack (pos, len, string) -> (pos, len, string, encode_rlp_fixed_finish) + // stack: context, segment, pos, len, string, encode_rlp_fixed_finish, retdest %jump(mstore_unpacking_rlp) encode_rlp_fixed_finish: // stack: pos', retdest @@ -68,8 +68,8 @@ global doubly_encode_rlp_fixed: SWAP1 %add_const(2) // advance past the two prefix bytes // stack: pos'', len, string, retdest - %stack (pos, len, string) -> (pos, string, len, encode_rlp_fixed_finish) - // stack: context, segment, pos'', string, len, encode_rlp_fixed_finish, retdest + %stack (pos, len, string) -> (pos, len, string, encode_rlp_fixed_finish) + // stack: context, segment, pos'', len, string, encode_rlp_fixed_finish, retdest %jump(mstore_unpacking_rlp) // Writes the RLP prefix for a string of the given length. This does not handle @@ -113,7 +113,6 @@ encode_rlp_multi_byte_string_prefix_large: // stack: pos, len_of_len, str_len, retdest %increment // stack: pos', len_of_len, str_len, retdest - %stack (pos, len_of_len, str_len) -> (pos, str_len, len_of_len) %jump(mstore_unpacking_rlp) %macro encode_rlp_multi_byte_string_prefix @@ -155,7 +154,7 @@ encode_rlp_list_prefix_large: SWAP1 %increment // stack: pos', len_of_len, payload_len, retdest %stack (pos, len_of_len, payload_len) - -> (pos, payload_len, len_of_len, + -> (pos, len_of_len, payload_len, encode_rlp_list_prefix_large_done_writing_len) %jump(mstore_unpacking_rlp) encode_rlp_list_prefix_large_done_writing_len: @@ -211,7 +210,7 @@ prepend_rlp_list_prefix_big: // stack: prefix_start_pos, len_of_len, payload_len, end_pos, start_pos, retdest DUP1 %increment // start_len_pos = prefix_start_pos + 1 %stack (start_len_pos, prefix_start_pos, len_of_len, payload_len, end_pos, start_pos, retdest) - -> (start_len_pos, payload_len, len_of_len, + -> (start_len_pos, len_of_len, payload_len, prepend_rlp_list_prefix_big_done_writing_len, prefix_start_pos, end_pos, retdest) %jump(mstore_unpacking_rlp) @@ -276,10 +275,10 @@ prepend_rlp_list_prefix_big_done_writing_len: %endmacro // Like mstore_unpacking, but specifically for the RLP segment. -// Pre stack: offset, value, len, retdest +// Pre stack: offset, len, value, retdest // Post stack: offset' global mstore_unpacking_rlp: - // stack: offset, value, len, retdest + // stack: offset, len, value, retdest PUSH @SEGMENT_RLP_RAW PUSH 0 // context %jump(mstore_unpacking) diff --git a/evm/src/cpu/kernel/asm/rlp/encode_rlp_string.asm b/evm/src/cpu/kernel/asm/rlp/encode_rlp_string.asm index 1065c61209..0efc5ce2f0 100644 --- a/evm/src/cpu/kernel/asm/rlp/encode_rlp_string.asm +++ b/evm/src/cpu/kernel/asm/rlp/encode_rlp_string.asm @@ -63,7 +63,7 @@ global encode_rlp_string_large: %increment // stack: pos', len_of_len, ADDR: 3, len, retdest %stack (pos, len_of_len, ADDR: 3, len) - -> (pos, len, len_of_len, encode_rlp_string_large_after_writing_len, ADDR, len) + -> (pos, len_of_len, len, encode_rlp_string_large_after_writing_len, ADDR, len) %jump(mstore_unpacking_rlp) global encode_rlp_string_large_after_writing_len: // stack: pos'', ADDR: 3, len, retdest diff --git a/evm/src/cpu/kernel/asm/util/keccak.asm b/evm/src/cpu/kernel/asm/util/keccak.asm index 1a1f437287..1f9a8d5ca5 100644 --- a/evm/src/cpu/kernel/asm/util/keccak.asm +++ b/evm/src/cpu/kernel/asm/util/keccak.asm @@ -37,7 +37,7 @@ sys_keccak256_empty: %macro keccak256_word(num_bytes) // Since KECCAK_GENERAL takes its input from memory, we will first write // input_word's bytes to @SEGMENT_KERNEL_GENERAL[0..$num_bytes]. - %stack (word) -> (0, @SEGMENT_KERNEL_GENERAL, 0, word, $num_bytes, %%after_mstore) + %stack (word) -> (0, @SEGMENT_KERNEL_GENERAL, 0, $num_bytes, word, %%after_mstore) %jump(mstore_unpacking) %%after_mstore: // stack: offset @@ -53,10 +53,10 @@ sys_keccak256_empty: // Since KECCAK_GENERAL takes its input from memory, we will first write // a's bytes to @SEGMENT_KERNEL_GENERAL[0..32], then b's bytes to // @SEGMENT_KERNEL_GENERAL[32..64]. - %stack (a) -> (0, @SEGMENT_KERNEL_GENERAL, 0, a, 32, %%after_mstore_a) + %stack (a) -> (0, @SEGMENT_KERNEL_GENERAL, 0, 32, a, %%after_mstore_a) %jump(mstore_unpacking) %%after_mstore_a: - %stack (offset, b) -> (0, @SEGMENT_KERNEL_GENERAL, 32, b, 32, %%after_mstore_b) + %stack (offset, b) -> (0, @SEGMENT_KERNEL_GENERAL, 32, 32, b, %%after_mstore_b) %jump(mstore_unpacking) %%after_mstore_b: %stack (offset) -> (0, @SEGMENT_KERNEL_GENERAL, 0, 64) // context, segment, offset, len diff --git a/evm/src/cpu/kernel/tests/packing.rs b/evm/src/cpu/kernel/tests/packing.rs index 43ca9b5fc2..016c7888a8 100644 --- a/evm/src/cpu/kernel/tests/packing.rs +++ b/evm/src/cpu/kernel/tests/packing.rs @@ -70,12 +70,12 @@ fn test_mstore_unpacking() -> Result<()> { let mstore_unpacking = KERNEL.global_labels["mstore_unpacking"]; let retdest = 0xDEADBEEFu32.into(); - let len = 4.into(); let value = 0xABCD1234u32.into(); + let len = 4.into(); let offset = 0.into(); let segment = (Segment::TxnData as u32).into(); let context = 0.into(); - let initial_stack = vec![retdest, len, value, offset, segment, context]; + let initial_stack = vec![retdest, value, len, offset, segment, context]; let mut interpreter = Interpreter::new_with_kernel(mstore_unpacking, initial_stack); From 3cd6008300ce0c1478c24778541894ed15951c68 Mon Sep 17 00:00:00 2001 From: Linda Guiga Date: Tue, 7 Nov 2023 12:16:33 -0500 Subject: [PATCH 5/6] Add comment for the stack constraints --- evm/src/cpu/stack.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/src/cpu/stack.rs b/evm/src/cpu/stack.rs index 622d07d26b..6db9e0d60c 100644 --- a/evm/src/cpu/stack.rs +++ b/evm/src/cpu/stack.rs @@ -115,7 +115,7 @@ const STACK_BEHAVIORS: OpsColumnsView> = OpsColumnsView { dup: None, swap: None, context_op: None, // SET_CONTEXT is special since it involves the old and the new stack. - memop_32bytes: None, + memop_32bytes: None, // Manually checked in `memio.rs`. exit_kernel: Some(StackBehavior { num_pops: 1, pushes: false, From 1933143e45ca6d683017c650a3ff946383aafdeb Mon Sep 17 00:00:00 2001 From: Linda Guiga Date: Thu, 9 Nov 2023 12:58:37 -0500 Subject: [PATCH 6/6] Add comment for ctl_data_byte32 --- evm/src/cpu/cpu_stark.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/evm/src/cpu/cpu_stark.rs b/evm/src/cpu/cpu_stark.rs index 691116cdb2..46d6d328b5 100644 --- a/evm/src/cpu/cpu_stark.rs +++ b/evm/src/cpu/cpu_stark.rs @@ -125,6 +125,10 @@ pub fn ctl_arithmetic_shift_rows() -> TableWithColumns { TableWithColumns::new(Table::Cpu, columns, Some(Column::single(COL_MAP.op.shift))) } +/// We use the same columns for `memop_32bytes` and `is_keccak_sponge`. +/// However, while `MLOAD_32BYTES` and `KeccakSponge` share the same +/// data structure for the memory columns, MSTORE_32BYTES reads from +/// GP channel 4, instead of pushing into it. pub fn ctl_data_byte32() -> Vec> { ctl_data_keccak_sponge() }