From 2c5347c45f07f31cdfd7a183cdf5f9873d5c32e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alonso=20Gonz=C3=A1lez?= Date: Fri, 15 Dec 2023 09:49:19 +0100 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> --- evm/src/cpu/kernel/asm/core/call.asm | 1 - evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm | 10 +++++----- evm/src/cpu/kernel/interpreter.rs | 2 +- evm/src/generation/mod.rs | 4 ++-- evm/src/generation/prover_input.rs | 14 +++++++------- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/call.asm b/evm/src/cpu/kernel/asm/core/call.asm index 46765954..5173d358 100644 --- a/evm/src/cpu/kernel/asm/core/call.asm +++ b/evm/src/cpu/kernel/asm/core/call.asm @@ -370,7 +370,6 @@ call_too_deep: GET_CONTEXT // stack: ctx, code_size, retdest %validate_jumpdest_table - PUSH 0 // jump dest EXIT_KERNEL // (Old context) stack: new_ctx diff --git a/evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm b/evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm index 76c25fa0..79475b37 100644 --- a/evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm +++ b/evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm @@ -122,9 +122,9 @@ global is_jumpdest: //stack: jumpdest, ctx, proof_prefix_addr, retdest SWAP2 DUP1 // stack: proof_prefix_addr, proof_prefix_addr, ctx, jumpdest - %eq_const(0) + IS_ZERO %jumpi(verify_path) - //stack: proof_prefix_addr, ctx, jumpdest, retdest + // stack: proof_prefix_addr, ctx, jumpdest, retdest // If we are here we need to check that the next 32 bytes are less // than JUMPXX for XX < 32 - i <=> opcode < 0x7f - i = 127 - i, 0 <= i < 32, // or larger than 127 @@ -141,7 +141,7 @@ global is_jumpdest: %jump(verify_path) return_is_jumpdest: - //stack: proof_prefix_addr, jumpdest, ctx, retdest + // stack: proof_prefix_addr, jumpdest, ctx, retdest %pop3 JUMP @@ -187,7 +187,7 @@ global validate_jumpdest_table: // and the next prover input should contain a proof for address'. PROVER_INPUT(jumpdest_table::next_address) DUP1 %jumpi(check_proof) - // If proof == 0 there are no more jump destionations to check + // If proof == 0 there are no more jump destinations to check POP // This is just a hook used for avoiding verification of the jumpdest // table in another contexts. It is useful during proof generation, @@ -196,7 +196,7 @@ global validate_jumpdest_table_end: POP JUMP check_proof: - %sub_const(1) + %decrement DUP2 DUP2 // stack: address, ctx, address, ctx // We read the proof diff --git a/evm/src/cpu/kernel/interpreter.rs b/evm/src/cpu/kernel/interpreter.rs index a16d2d3a..9b66b5de 100644 --- a/evm/src/cpu/kernel/interpreter.rs +++ b/evm/src/cpu/kernel/interpreter.rs @@ -303,7 +303,7 @@ impl<'a> Interpreter<'a> { ) } - fn incr(&mut self, n: usize) { + const fn incr(&mut self, n: usize) { self.generation_state.registers.program_counter += n; } diff --git a/evm/src/generation/mod.rs b/evm/src/generation/mod.rs index 71176da5..9af830d4 100644 --- a/evm/src/generation/mod.rs +++ b/evm/src/generation/mod.rs @@ -293,7 +293,7 @@ fn simulate_cpu_between_labels_and_get_user_jumps( state.registers.program_counter = KERNEL.global_labels[initial_label]; let context = state.registers.context; - log::debug!("Simulating CPU for jumpdest analysis "); + log::debug!("Simulating CPU for jumpdest analysis."); loop { if state.registers.program_counter == KERNEL.global_labels["validate_jumpdest_table"] { @@ -317,7 +317,7 @@ fn simulate_cpu_between_labels_and_get_user_jumps( { // TODO: hotfix for avoiding deeper calls to abort let jumpdest = u256_to_usize(state.registers.stack_top) - .map_err(|_| anyhow::Error::msg("Not a valid jump destination"))?; + .map_err(|_| anyhow!("Not a valid jump destination"))?; state.memory.set( MemoryAddress { context: state.registers.context, diff --git a/evm/src/generation/prover_input.rs b/evm/src/generation/prover_input.rs index 53e25db4..1808f3f3 100644 --- a/evm/src/generation/prover_input.rs +++ b/evm/src/generation/prover_input.rs @@ -274,7 +274,7 @@ impl GenerationState { } } - /// Return the proof for the last jump adddress + /// Returns the proof for the last jump address. fn run_next_jumpdest_table_proof(&mut self) -> Result { let code = (0..self.last_jumpdest_address) .map(|i| { @@ -286,12 +286,12 @@ impl GenerationState { }) .collect::, _>>()?; - // TODO: The proof searching algorithm is not very eficient. But luckyly it doesn't seem + // TODO: The proof searching algorithm is not very efficient. But luckily it doesn't seem // a problem as is done natively. - // Search the closest address to last_jumpdest_address for which none of + // Search the closest address to `last_jumpdest_address` for which none of // the previous 32 bytes in the code (including opcodes and pushed bytes) - // are PUSHXX and the address is in its range + // are PUSHXX and the address is in its range. let proof = CodeIterator::until(&code, self.last_jumpdest_address + 1).fold( 0, @@ -340,9 +340,9 @@ impl GenerationState { }) .collect::, _>>()?; - // We need to set the the simulated jumpdest bits to one as otherwise - // the simulation will fail - let mut jumpdest_table = vec![]; + // We need to set the simulated jumpdest bits to one as otherwise + // the simulation will fail. + let mut jumpdest_table = Vec::with_capacity(code.len()); for (pos, opcode) in CodeIterator::new(&code) { jumpdest_table.push((pos, opcode == get_opcode("JUMPDEST"))); if opcode == get_opcode("JUMPDEST") {