From f4713c44d3d1dec43ae36b745263aa87078c9398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alonso=20Gonz=C3=A1lez?= Date: Fri, 5 Jan 2024 17:18:03 +0100 Subject: [PATCH 1/2] Apply suggestions from code review Co-authored-by: Linda Guiga <101227802+LindaGuiga@users.noreply.github.com> Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> --- evm/src/cpu/kernel/asm/mpt/hash/hash.asm | 2 +- evm/src/cpu/kernel/constants/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm index 9feb4e19..6d0cf10c 100644 --- a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm +++ b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm @@ -197,7 +197,7 @@ encode_node_branch_prepend_prefix: // If result_len != 32, result is raw RLP, with an appropriate RLP prefix already. SWAP1 DUP1 %sub_const(32) %jumpi(%%unpack) // Otherwise, result is a hash, and we need to add the prefix 0x80 + 32 = 160. - // stack: result_len, result, cur_len, rlp_pos, rlp_start, node_payload_ptr, encode_value, cur_len, retdest + // stack: result_len, result, cur_len, rlp_pos, rlp_start, node_payload_ptr, encode_value, retdest PUSH 160 DUP5 // rlp_pos %mstore_rlp diff --git a/evm/src/cpu/kernel/constants/mod.rs b/evm/src/cpu/kernel/constants/mod.rs index fa25fa87..1cd99b38 100644 --- a/evm/src/cpu/kernel/constants/mod.rs +++ b/evm/src/cpu/kernel/constants/mod.rs @@ -94,7 +94,7 @@ const MISC_CONSTANTS: [(&str, [u8; 32]); 3] = [ hex!("0000000000000000000000000000000100000000000000000000000000000000"), ), // Position in SEGMENT_RLP_RAW where the empty node encoding is stored. It is - // equal to usize::MAX so that all rlp pointers all much smalled than that + // equal to usize::MAX so that all rlp pointers all much smaller than that. ( "ENCODED_EMPTY_NODE_POS", hex!("00000000000000000000000000000000000000000000000000000000FFFFFFFF"), From 1c994737ef025117333f6f0d5745d44a35a1d824 Mon Sep 17 00:00:00 2001 From: 4l0n50 Date: Fri, 5 Jan 2024 17:30:59 +0100 Subject: [PATCH 2/2] Address comments --- evm/src/cpu/kernel/asm/main.asm | 3 --- evm/src/cpu/kernel/asm/mpt/hash/hash.asm | 2 +- evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm | 3 --- evm/src/cpu/kernel/interpreter.rs | 2 +- evm/src/cpu/kernel/tests/mpt/insert.rs | 4 +--- evm/src/generation/mod.rs | 2 +- 6 files changed, 4 insertions(+), 12 deletions(-) diff --git a/evm/src/cpu/kernel/asm/main.asm b/evm/src/cpu/kernel/asm/main.asm index d1e06b66..df6b09b3 100644 --- a/evm/src/cpu/kernel/asm/main.asm +++ b/evm/src/cpu/kernel/asm/main.asm @@ -15,9 +15,6 @@ global main: %shift_table_init // Encode constant nodes %initialize_rlp_segment - - // Encode constant nodes - %initialize_rlp_segment // Initialize the state, transaction and receipt trie root pointers. PROVER_INPUT(trie_ptr::state) diff --git a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm index 6d0cf10c..c45785fa 100644 --- a/evm/src/cpu/kernel/asm/mpt/hash/hash.asm +++ b/evm/src/cpu/kernel/asm/mpt/hash/hash.asm @@ -105,7 +105,7 @@ after_packed_small_rlp: // indicating where the data lives within @SEGMENT_RLP_RAW. // // Pre stack: node_type, node_ptr, encode_value, cur_len, retdest -// Post stack: result_ptr, result_len +// Post stack: result_ptr, result_len, cur_len encode_node: // stack: node_type, node_ptr, encode_value, cur_len, retdest // Increment node_ptr, so it points to the node payload instead of its type. diff --git a/evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm b/evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm index f3ee000d..03a8d275 100644 --- a/evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm +++ b/evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm @@ -3,9 +3,6 @@ global mpt_hash_state_trie: // stack: cur_len, retdest PUSH encode_account - PUSH debug_before_encoding_child - PUSH mpt_delete - %pop2 %mload_global_metadata(@GLOBAL_METADATA_STATE_TRIE_ROOT) // stack: node_ptr, encode_account, cur_len, retdest %jump(mpt_hash) diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index 58de9e34..4641e8a1 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -1174,7 +1174,7 @@ impl<'a> Interpreter<'a> { self.generation_state.registers.context = context; } - /// Writes the encoding of 0 to position @ + /// Writes the encoding of 0 to position @ENCODED_EMPTY_NODE_POS pub(crate) fn initialize_rlp_segment(&mut self) { self.generation_state.memory.set( MemoryAddress { diff --git a/evm/src/cpu/kernel/tests/mpt/insert.rs b/evm/src/cpu/kernel/tests/mpt/insert.rs index acd42e8f..9c2fd50b 100644 --- a/evm/src/cpu/kernel/tests/mpt/insert.rs +++ b/evm/src/cpu/kernel/tests/mpt/insert.rs @@ -12,12 +12,11 @@ use crate::cpu::kernel::tests::mpt::{ }; use crate::generation::mpt::AccountRlp; use crate::generation::TrieInputs; -use crate::util::u256_to_usize; use crate::Node; #[test] fn mpt_insert_empty() -> Result<()> { - test_state_trie(Default::default(), nibbles_64(0xDEF), test_account_2()) + test_state_trie(Default::default(), nibbles_64(0xABC), test_account_2()) } #[test] @@ -234,7 +233,6 @@ fn test_state_trie( let hash = H256::from_uint(&interpreter.stack()[1]); state_trie.insert(k, rlp::encode(&account).to_vec()); - let expected_state_trie_hash = state_trie.hash(); assert_eq!(hash, expected_state_trie_hash); diff --git a/evm/src/generation/mod.rs b/evm/src/generation/mod.rs index e5dbc3e7..d691d34e 100644 --- a/evm/src/generation/mod.rs +++ b/evm/src/generation/mod.rs @@ -35,7 +35,7 @@ pub mod mpt; pub(crate) mod prover_input; pub(crate) mod rlp; pub(crate) mod state; -pub(crate) mod trie_extractor; +mod trie_extractor; use self::mpt::{load_all_mpts, TrieRootPtrs}; use crate::witness::util::mem_write_log;