From 61b6b161067c6407da7a3dd44101013318050d56 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 14:06:48 +0200 Subject: [PATCH 1/6] Fix interpreter --- evm/src/cpu/kernel/interpreter.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index a2c25bf6..0b274221 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -6,6 +6,7 @@ use anyhow::{anyhow, bail, ensure}; use ethereum_types::{U256, U512}; use keccak_hash::keccak; use plonky2::field::goldilocks_field::GoldilocksField; +use plonky2_util::ceil_div_usize; use crate::cpu::kernel::aggregator::KERNEL; use crate::cpu::kernel::assembler::Kernel; @@ -582,7 +583,7 @@ impl<'a> Interpreter<'a> { [Segment::MainMemory as usize] .content .len(); - self.push(U256::from(num_bytes)); + self.push(U256::from(ceil_div_usize(num_bytes, 32) * 32)); } fn run_jumpdest(&mut self) { From 71ed3c43ac1d738b158fc2802514c2992d2e01f3 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 14:32:28 +0200 Subject: [PATCH 2/6] Fix fix interpreter --- .../cpu/kernel/constants/context_metadata.rs | 6 +++- evm/src/cpu/kernel/interpreter.rs | 28 ++++++++--------- evm/src/cpu/kernel/tests/mpt/read.rs | 3 +- evm/src/generation/memory.rs | 30 +++++++++++++++++-- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/evm/src/cpu/kernel/constants/context_metadata.rs b/evm/src/cpu/kernel/constants/context_metadata.rs index a2c460fc..0b2dbee5 100644 --- a/evm/src/cpu/kernel/constants/context_metadata.rs +++ b/evm/src/cpu/kernel/constants/context_metadata.rs @@ -23,10 +23,12 @@ pub(crate) enum ContextMetadata { /// Pointer to the initial version of the state trie, at the creation of this context. Used when /// we need to revert a context. StateTrieCheckpointPointer = 9, + /// Size of active memory. + MSize = 10, } impl ContextMetadata { - pub(crate) const COUNT: usize = 10; + pub(crate) const COUNT: usize = 11; pub(crate) fn all() -> [Self; Self::COUNT] { [ @@ -40,6 +42,7 @@ impl ContextMetadata { Self::CallValue, Self::Static, Self::StateTrieCheckpointPointer, + Self::MSize, ] } @@ -56,6 +59,7 @@ impl ContextMetadata { ContextMetadata::CallValue => "CTX_METADATA_CALL_VALUE", ContextMetadata::Static => "CTX_METADATA_STATIC", ContextMetadata::StateTrieCheckpointPointer => "CTX_METADATA_STATE_TRIE_CHECKPOINT_PTR", + ContextMetadata::MSize => "CTX_METADATA_MSIZE", } } } diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index 0b274221..0e66a08b 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -49,7 +49,7 @@ impl InterpreterMemory { } impl InterpreterMemory { - fn mload_general(&self, context: usize, segment: Segment, offset: usize) -> U256 { + fn mload_general(&mut self, context: usize, segment: Segment, offset: usize) -> U256 { let value = self.context_memory[context].segments[segment as usize].get(offset); assert!( value.bits() <= segment.bit_range(), @@ -145,18 +145,19 @@ impl<'a> Interpreter<'a> { Ok(()) } - fn code(&self) -> &MemorySegmentState { - &self.memory.context_memory[self.context].segments[Segment::Code as usize] + fn code(&mut self) -> &mut MemorySegmentState { + &mut self.memory.context_memory[self.context].segments[Segment::Code as usize] } - fn code_slice(&self, n: usize) -> Vec { - self.code().content[self.offset..self.offset + n] + fn code_slice(&mut self, n: usize) -> Vec { + let offset = self.offset; + self.code().content[offset..offset + n] .iter() .map(|u256| u256.byte(0)) .collect::>() } - pub(crate) fn get_txn_field(&self, field: NormalizedTxnField) -> U256 { + pub(crate) fn get_txn_field(&mut self, field: NormalizedTxnField) -> U256 { self.memory.context_memory[0].segments[Segment::TxnFields as usize].get(field as usize) } @@ -169,7 +170,7 @@ impl<'a> Interpreter<'a> { &self.memory.context_memory[0].segments[Segment::TxnData as usize].content } - pub(crate) fn get_global_metadata_field(&self, field: GlobalMetadata) -> U256 { + pub(crate) fn get_global_metadata_field(&mut self, field: GlobalMetadata) -> U256 { self.memory.context_memory[0].segments[Segment::GlobalMetadata as usize].get(field as usize) } @@ -224,7 +225,8 @@ impl<'a> Interpreter<'a> { } fn run_opcode(&mut self) -> anyhow::Result<()> { - let opcode = self.code().get(self.offset).byte(0); + let offset = self.offset; + let opcode = self.code().get(offset).byte(0); self.incr(1); match opcode { 0x00 => self.run_stop(), // "STOP", @@ -579,11 +581,9 @@ impl<'a> Interpreter<'a> { } fn run_msize(&mut self) { - let num_bytes = self.memory.context_memory[self.context].segments - [Segment::MainMemory as usize] - .content - .len(); - self.push(U256::from(ceil_div_usize(num_bytes, 32) * 32)); + self.push(U256::from( + self.memory.context_memory[self.context].segments[Segment::MainMemory as usize].msize, + )) } fn run_jumpdest(&mut self) { @@ -709,7 +709,7 @@ mod tests { 0x53, ]; let pis = HashMap::new(); - let run = run(&code, 0, vec![], &pis)?; + let mut run = run(&code, 0, vec![], &pis)?; assert_eq!(run.stack(), &[0xff.into(), 0xff00.into()]); assert_eq!( run.memory.context_memory[0].segments[Segment::MainMemory as usize].get(0x27), diff --git a/evm/src/cpu/kernel/tests/mpt/read.rs b/evm/src/cpu/kernel/tests/mpt/read.rs index d8808e24..12728cd1 100644 --- a/evm/src/cpu/kernel/tests/mpt/read.rs +++ b/evm/src/cpu/kernel/tests/mpt/read.rs @@ -31,7 +31,8 @@ fn mpt_read() -> Result<()> { interpreter.push(0xdeadbeefu32.into()); interpreter.push(0xABCDEFu64.into()); interpreter.push(6.into()); - interpreter.push(interpreter.get_global_metadata_field(GlobalMetadata::StateTrieRoot)); + let state_trie_root = interpreter.get_global_metadata_field(GlobalMetadata::StateTrieRoot); + interpreter.push(state_trie_root); interpreter.run()?; assert_eq!(interpreter.stack().len(), 1); diff --git a/evm/src/generation/memory.rs b/evm/src/generation/memory.rs index 944b42a6..18ed2e16 100644 --- a/evm/src/generation/memory.rs +++ b/evm/src/generation/memory.rs @@ -1,4 +1,5 @@ use ethereum_types::U256; +use plonky2_util::ceil_div_usize; use crate::memory::memory_stark::MemoryOp; use crate::memory::segments::Segment; @@ -22,19 +23,34 @@ impl Default for MemoryState { } } -#[derive(Clone, Default, Debug)] +#[derive(Clone, Debug)] pub(crate) struct MemoryContextState { /// The content of each memory segment. pub segments: [MemorySegmentState; Segment::COUNT], } -#[derive(Clone, Default, Debug)] +impl Default for MemoryContextState { + fn default() -> Self { + Self { + segments: Segment::all().map(|segment| MemorySegmentState { + content: vec![], + segment, + msize: 0, + }), + } + } +} + +#[derive(Clone, Debug)] pub(crate) struct MemorySegmentState { pub content: Vec, + pub segment: Segment, + pub msize: usize, } impl MemorySegmentState { - pub(crate) fn get(&self, virtual_addr: usize) -> U256 { + pub(crate) fn get(&mut self, virtual_addr: usize) -> U256 { + self.update_msize(virtual_addr); self.content .get(virtual_addr) .copied() @@ -42,9 +58,17 @@ impl MemorySegmentState { } pub(crate) fn set(&mut self, virtual_addr: usize, value: U256) { + assert_eq!(value >> self.segment.bit_range(), U256::zero()); + self.update_msize(virtual_addr); if virtual_addr >= self.content.len() { self.content.resize(virtual_addr + 1, U256::zero()); } self.content[virtual_addr] = value; } + + fn update_msize(&mut self, virtual_addr: usize) { + let word_size = 256 / self.segment.bit_range(); + let new_msize = ceil_div_usize(virtual_addr + 1, word_size) * 32; + self.msize = self.msize.max(new_msize); + } } From 9982d79999cdbf577e5ba256dee3359164c81434 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 19:23:01 +0200 Subject: [PATCH 3/6] Add msize --- evm/src/cpu/kernel/asm/memory/core.asm | 18 ++++++++++++ evm/src/cpu/kernel/asm/memory/metadata.asm | 20 +++++++++++++ .../cpu/kernel/constants/context_metadata.rs | 2 +- evm/src/cpu/kernel/interpreter.rs | 18 ++++++++++-- evm/src/generation/memory.rs | 29 ++----------------- 5 files changed, 57 insertions(+), 30 deletions(-) diff --git a/evm/src/cpu/kernel/asm/memory/core.asm b/evm/src/cpu/kernel/asm/memory/core.asm index 2b4d2b68..f6bb99b6 100644 --- a/evm/src/cpu/kernel/asm/memory/core.asm +++ b/evm/src/cpu/kernel/asm/memory/core.asm @@ -393,3 +393,21 @@ %mstore_kernel_general_2 // stack: (empty) %endmacro + +%macro mload_main + // stack: offset + DUP1 + // stack: offset, offset + %update_msize + // stack: offset + %mload_current(@SEGMENT_MAIN_MEMORY) +%endmacro + +%macro mstore_main + // stack: offset, value + DUP1 + // stack: offset, offset, value + %update_msize + // stack: offset, value + %mstore_current(@SEGMENT_MAIN_MEMORY) +%endmacro diff --git a/evm/src/cpu/kernel/asm/memory/metadata.asm b/evm/src/cpu/kernel/asm/memory/metadata.asm index 644699e0..f5cc523c 100644 --- a/evm/src/cpu/kernel/asm/memory/metadata.asm +++ b/evm/src/cpu/kernel/asm/memory/metadata.asm @@ -45,3 +45,23 @@ %macro callvalue %mload_context_metadata(@CTX_METADATA_CALL_VALUE) %endmacro + +%macro msize + %mload_context_metadata(@CTX_METADATA_MSIZE) +%endmacro + +%macro update_msize + // stack: offset + %add_const(32) + // stack: 32 + offset + %div_const(32) + // stack: (offset+32)/32 = ceil_div_usize(offset+1, 32) + %mul_const(32) + // stack: ceil_div_usize(offset+1, 32)*32 + %msize + // stack: current_msize, ceil_div_usize(offset+1, 32) * 32 + %max + // stack: new_msize + %mstore_context_metadata(@CTX_METADATA_MSIZE) +%endmacro + diff --git a/evm/src/cpu/kernel/constants/context_metadata.rs b/evm/src/cpu/kernel/constants/context_metadata.rs index 0b2dbee5..fab74373 100644 --- a/evm/src/cpu/kernel/constants/context_metadata.rs +++ b/evm/src/cpu/kernel/constants/context_metadata.rs @@ -23,7 +23,7 @@ pub(crate) enum ContextMetadata { /// Pointer to the initial version of the state trie, at the creation of this context. Used when /// we need to revert a context. StateTrieCheckpointPointer = 9, - /// Size of active memory. + /// Size of the active main memory. MSize = 10, } diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index 0e66a08b..4cc8c813 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -10,6 +10,7 @@ use plonky2_util::ceil_div_usize; use crate::cpu::kernel::aggregator::KERNEL; use crate::cpu::kernel::assembler::Kernel; +use crate::cpu::kernel::constants::context_metadata::ContextMetadata; use crate::cpu::kernel::constants::global_metadata::GlobalMetadata; use crate::cpu::kernel::constants::txn_fields::NormalizedTxnField; use crate::generation::memory::{MemoryContextState, MemorySegmentState}; @@ -46,9 +47,7 @@ impl InterpreterMemory { mem } -} -impl InterpreterMemory { fn mload_general(&mut self, context: usize, segment: Segment, offset: usize) -> U256 { let value = self.context_memory[context].segments[segment as usize].get(offset); assert!( @@ -67,6 +66,16 @@ impl InterpreterMemory { ); self.context_memory[context].segments[segment as usize].set(offset, value) } + + fn update_msize(&mut self, context: usize, offset: usize) { + let current_msize = self.context_memory[context].segments + [Segment::ContextMetadata as usize] + .get(ContextMetadata::MSize as usize); + let msize = ceil_div_usize(offset + 1, 32) * 32; + let new_msize = current_msize.max(msize.into()); + self.context_memory[context].segments[Segment::ContextMetadata as usize] + .set(ContextMetadata::MSize as usize, new_msize) + } } pub struct Interpreter<'a> { @@ -502,6 +511,8 @@ impl<'a> Interpreter<'a> { fn run_keccak_general(&mut self) { let context = self.pop().as_usize(); let segment = Segment::all()[self.pop().as_usize()]; + // Not strictly needed but here to avoid surprises with MSIZE. + assert_ne!(segment, Segment::MainMemory, "Call KECCAK256 instead."); let offset = self.pop().as_usize(); let size = self.pop().as_usize(); let bytes = (offset..offset + size) @@ -582,7 +593,8 @@ impl<'a> Interpreter<'a> { fn run_msize(&mut self) { self.push(U256::from( - self.memory.context_memory[self.context].segments[Segment::MainMemory as usize].msize, + self.memory.context_memory[self.context].segments[Segment::ContextMetadata as usize] + .get(ContextMetadata::MSize as usize), )) } diff --git a/evm/src/generation/memory.rs b/evm/src/generation/memory.rs index 18ed2e16..d5dd21ac 100644 --- a/evm/src/generation/memory.rs +++ b/evm/src/generation/memory.rs @@ -23,34 +23,19 @@ impl Default for MemoryState { } } -#[derive(Clone, Debug)] +#[derive(Clone, Default, Debug)] pub(crate) struct MemoryContextState { /// The content of each memory segment. pub segments: [MemorySegmentState; Segment::COUNT], } -impl Default for MemoryContextState { - fn default() -> Self { - Self { - segments: Segment::all().map(|segment| MemorySegmentState { - content: vec![], - segment, - msize: 0, - }), - } - } -} - -#[derive(Clone, Debug)] +#[derive(Clone, Default, Debug)] pub(crate) struct MemorySegmentState { pub content: Vec, - pub segment: Segment, - pub msize: usize, } impl MemorySegmentState { - pub(crate) fn get(&mut self, virtual_addr: usize) -> U256 { - self.update_msize(virtual_addr); + pub(crate) fn get(&self, virtual_addr: usize) -> U256 { self.content .get(virtual_addr) .copied() @@ -58,17 +43,9 @@ impl MemorySegmentState { } pub(crate) fn set(&mut self, virtual_addr: usize, value: U256) { - assert_eq!(value >> self.segment.bit_range(), U256::zero()); - self.update_msize(virtual_addr); if virtual_addr >= self.content.len() { self.content.resize(virtual_addr + 1, U256::zero()); } self.content[virtual_addr] = value; } - - fn update_msize(&mut self, virtual_addr: usize) { - let word_size = 256 / self.segment.bit_range(); - let new_msize = ceil_div_usize(virtual_addr + 1, word_size) * 32; - self.msize = self.msize.max(new_msize); - } } From fab3fe77c0e68e7b238c7e513432d5ddf66b03a2 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 19:28:24 +0200 Subject: [PATCH 4/6] Minor --- evm/src/cpu/kernel/interpreter.rs | 33 +++++++++------------------- evm/src/cpu/kernel/tests/mpt/read.rs | 3 +-- evm/src/generation/memory.rs | 1 - 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index 4cc8c813..7df55d44 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -6,7 +6,6 @@ use anyhow::{anyhow, bail, ensure}; use ethereum_types::{U256, U512}; use keccak_hash::keccak; use plonky2::field::goldilocks_field::GoldilocksField; -use plonky2_util::ceil_div_usize; use crate::cpu::kernel::aggregator::KERNEL; use crate::cpu::kernel::assembler::Kernel; @@ -66,16 +65,6 @@ impl InterpreterMemory { ); self.context_memory[context].segments[segment as usize].set(offset, value) } - - fn update_msize(&mut self, context: usize, offset: usize) { - let current_msize = self.context_memory[context].segments - [Segment::ContextMetadata as usize] - .get(ContextMetadata::MSize as usize); - let msize = ceil_div_usize(offset + 1, 32) * 32; - let new_msize = current_msize.max(msize.into()); - self.context_memory[context].segments[Segment::ContextMetadata as usize] - .set(ContextMetadata::MSize as usize, new_msize) - } } pub struct Interpreter<'a> { @@ -154,19 +143,18 @@ impl<'a> Interpreter<'a> { Ok(()) } - fn code(&mut self) -> &mut MemorySegmentState { - &mut self.memory.context_memory[self.context].segments[Segment::Code as usize] + fn code(&self) -> &MemorySegmentState { + &self.memory.context_memory[self.context].segments[Segment::Code as usize] } - fn code_slice(&mut self, n: usize) -> Vec { - let offset = self.offset; - self.code().content[offset..offset + n] + fn code_slice(&self, n: usize) -> Vec { + self.code().content[self.offset..self.offset + n] .iter() .map(|u256| u256.byte(0)) .collect::>() } - pub(crate) fn get_txn_field(&mut self, field: NormalizedTxnField) -> U256 { + pub(crate) fn get_txn_field(&self, field: NormalizedTxnField) -> U256 { self.memory.context_memory[0].segments[Segment::TxnFields as usize].get(field as usize) } @@ -179,7 +167,7 @@ impl<'a> Interpreter<'a> { &self.memory.context_memory[0].segments[Segment::TxnData as usize].content } - pub(crate) fn get_global_metadata_field(&mut self, field: GlobalMetadata) -> U256 { + pub(crate) fn get_global_metadata_field(&self, field: GlobalMetadata) -> U256 { self.memory.context_memory[0].segments[Segment::GlobalMetadata as usize].get(field as usize) } @@ -234,8 +222,7 @@ impl<'a> Interpreter<'a> { } fn run_opcode(&mut self) -> anyhow::Result<()> { - let offset = self.offset; - let opcode = self.code().get(offset).byte(0); + let opcode = self.code().get(self.offset).byte(0); self.incr(1); match opcode { 0x00 => self.run_stop(), // "STOP", @@ -592,10 +579,10 @@ impl<'a> Interpreter<'a> { } fn run_msize(&mut self) { - self.push(U256::from( + self.push( self.memory.context_memory[self.context].segments[Segment::ContextMetadata as usize] .get(ContextMetadata::MSize as usize), - )) + ) } fn run_jumpdest(&mut self) { @@ -721,7 +708,7 @@ mod tests { 0x53, ]; let pis = HashMap::new(); - let mut run = run(&code, 0, vec![], &pis)?; + let run = run(&code, 0, vec![], &pis)?; assert_eq!(run.stack(), &[0xff.into(), 0xff00.into()]); assert_eq!( run.memory.context_memory[0].segments[Segment::MainMemory as usize].get(0x27), diff --git a/evm/src/cpu/kernel/tests/mpt/read.rs b/evm/src/cpu/kernel/tests/mpt/read.rs index 12728cd1..d8808e24 100644 --- a/evm/src/cpu/kernel/tests/mpt/read.rs +++ b/evm/src/cpu/kernel/tests/mpt/read.rs @@ -31,8 +31,7 @@ fn mpt_read() -> Result<()> { interpreter.push(0xdeadbeefu32.into()); interpreter.push(0xABCDEFu64.into()); interpreter.push(6.into()); - let state_trie_root = interpreter.get_global_metadata_field(GlobalMetadata::StateTrieRoot); - interpreter.push(state_trie_root); + interpreter.push(interpreter.get_global_metadata_field(GlobalMetadata::StateTrieRoot)); interpreter.run()?; assert_eq!(interpreter.stack().len(), 1); diff --git a/evm/src/generation/memory.rs b/evm/src/generation/memory.rs index d5dd21ac..944b42a6 100644 --- a/evm/src/generation/memory.rs +++ b/evm/src/generation/memory.rs @@ -1,5 +1,4 @@ use ethereum_types::U256; -use plonky2_util::ceil_div_usize; use crate::memory::memory_stark::MemoryOp; use crate::memory::segments::Segment; From 0a800f8261c312d540fbabc37f316f4904c860d2 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 19:29:35 +0200 Subject: [PATCH 5/6] Minor --- evm/src/cpu/kernel/asm/memory/metadata.asm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/src/cpu/kernel/asm/memory/metadata.asm b/evm/src/cpu/kernel/asm/memory/metadata.asm index f5cc523c..1a495682 100644 --- a/evm/src/cpu/kernel/asm/memory/metadata.asm +++ b/evm/src/cpu/kernel/asm/memory/metadata.asm @@ -57,7 +57,7 @@ %div_const(32) // stack: (offset+32)/32 = ceil_div_usize(offset+1, 32) %mul_const(32) - // stack: ceil_div_usize(offset+1, 32)*32 + // stack: ceil_div_usize(offset+1, 32) * 32 %msize // stack: current_msize, ceil_div_usize(offset+1, 32) * 32 %max From 77d5c625cd1b8e9614086a744d131c66913cf46f Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 20 Oct 2022 19:36:28 +0200 Subject: [PATCH 6/6] Minor --- 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 7df55d44..f6cae73e 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -47,7 +47,7 @@ impl InterpreterMemory { mem } - fn mload_general(&mut self, context: usize, segment: Segment, offset: usize) -> U256 { + fn mload_general(&self, context: usize, segment: Segment, offset: usize) -> U256 { let value = self.context_memory[context].segments[segment as usize].get(offset); assert!( value.bits() <= segment.bit_range(),