Precompile ECRECOVER (#529)

* feat: preliminary work

* wip

* finish assignment to ecrecover

* fix: input length may be different than call data length (for precompiles)

* fix: input bytes to ecrecover

* minor edits

* Fix ecrecover input rlc comparison (right-padding zeroes) (#585)

* potential approach (pow of rand lookup)

* add lookup for pow of rand

* fix: right pad only if needed

* fix: missing constraint on padded_rlc

* constrain pow of rand table

* Update step.rs

* fix: sig_v sanity check (remove assertions to allow garbage input)

* fix: calldata length == 0 handled

* chore: renaming precompile_* and parallel iter for tests

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

bus-mapping/src/circuit_input_builder/execution.rs

@@ -3,8 +3,11 @@
 use std::marker::PhantomData;
 
 use crate::{
-    circuit_input_builder::CallContext, error::ExecError, exec_trace::OperationRef,
-    operation::RWCounter, precompile::PrecompileCalls,
+    circuit_input_builder::CallContext,
+    error::ExecError,
+    exec_trace::OperationRef,
+    operation::RWCounter,
+    precompile::{PrecompileAuxData, PrecompileCalls},
 };
 use eth_types::{
     evm_types::{Gas, GasCost, OpcodeId, ProgramCounter},
@@ -51,6 +54,8 @@ pub struct ExecStep {
     pub copy_rw_counter_delta: u64,
     /// Error generated by this step
     pub error: Option<ExecError>,
+    /// Optional auxiliary data that is attached to precompile call internal states.
+    pub aux_data: Option<PrecompileAuxData>,
 }
 
 impl ExecStep {
@@ -78,6 +83,7 @@ impl ExecStep {
             bus_mapping_instance: Vec::new(),
             copy_rw_counter_delta: 0,
             error: None,
+            aux_data: None,
         }
     }
 
@@ -113,6 +119,7 @@ impl Default for ExecStep {
             bus_mapping_instance: Vec::new(),
             copy_rw_counter_delta: 0,
             error: None,
+            aux_data: None,
         }
     }
 }
@@ -212,7 +219,7 @@ impl CopyDataTypeIter {
             3usize => Some(CopyDataType::TxCalldata),
             4usize => Some(CopyDataType::TxLog),
             5usize => Some(CopyDataType::RlcAcc),
-            6usize => Some(CopyDataType::Precompile(PrecompileCalls::ECRecover)),
+            6usize => Some(CopyDataType::Precompile(PrecompileCalls::Ecrecover)),
             7usize => Some(CopyDataType::Precompile(PrecompileCalls::Sha256)),
             8usize => Some(CopyDataType::Precompile(PrecompileCalls::Ripemd160)),
             9usize => Some(CopyDataType::Precompile(PrecompileCalls::Identity)),

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -26,6 +26,7 @@ use eth_types::{
     evm_types::{
         gas_utils::memory_expansion_gas_cost, Gas, GasCost, MemoryAddress, OpcodeId, StackAddress,
     },
+    sign_types::SignData,
     Address, Bytecode, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256,
 };
 use ethers_core::utils::{get_contract_address, get_create2_address, keccak256};
@@ -1291,6 +1292,11 @@ impl<'a> CircuitInputStateRef<'a> {
         self.block.add_exp_event(event)
     }
 
+    /// Push an ecrecover event to the state.
+    pub fn push_ecrecover(&mut self, event: SignData) {
+        self.block.add_ecrecover_event(event)
+    }
+
     pub(crate) fn get_step_err(
         &self,
         step: &GethExecStep,

bus-mapping/src/evm/opcodes/callop.rs

@@ -269,12 +269,6 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                     callee_gas_left,
                 );
 
-                log::trace!(
-                    "precompile returned data len {} gas {}",
-                    result.len(),
-                    contract_gas_cost
-                );
-
                 // mutate the caller memory.
                 let caller_ctx_mut = state.caller_ctx_mut()?;
                 caller_ctx_mut.return_data = result.clone();
@@ -346,11 +340,16 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
 
                 // insert a copy event (input) for this step
                 let rw_counter_start = state.block_ctx.rwc;
-                if call.call_data_length > 0 {
+                let n_input_bytes = if let Some(input_len) = precompile_call.input_len() {
+                    std::cmp::min(input_len, call.call_data_length as usize)
+                } else {
+                    call.call_data_length as usize
+                };
+                let input_bytes = if call.call_data_length > 0 {
                     let bytes: Vec<(u8, bool)> = caller_memory
                         .iter()
                         .skip(call.call_data_offset as usize)
-                        .take(call.call_data_length as usize)
+                        .take(n_input_bytes)
                         .map(|b| (*b, false))
                         .collect();
                     for (i, &(byte, _is_code)) in bytes.iter().enumerate() {
@@ -371,49 +370,57 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                             src_id: NumberOrHash::Number(call.caller_id),
                             src_type: CopyDataType::Memory,
                             src_addr: call.call_data_offset,
-                            src_addr_end: call.call_data_offset + call.call_data_length,
+                            src_addr_end: call.call_data_offset + n_input_bytes as u64,
                             dst_id: NumberOrHash::Number(call.call_id),
                             dst_type: CopyDataType::Precompile(precompile_call),
                             dst_addr: 0,
                             log_id: None,
                             rw_counter_start,
-                            bytes,
+                            bytes: bytes.clone(),
                         },
                     );
-                }
+                    Some(bytes.iter().map(|t| t.0).collect())
+                } else {
+                    None
+                };
 
                 // write the result in the callee's memory.
                 let rw_counter_start = state.block_ctx.rwc;
-                if call.is_success() && call.call_data_length > 0 && !result.is_empty() {
-                    let bytes: Vec<(u8, bool)> = result.iter().map(|b| (*b, false)).collect();
-                    for (i, &(byte, _is_code)) in bytes.iter().enumerate() {
-                        // push callee memory write
-                        state.push_op(
+                let output_bytes =
+                    if call.is_success() && call.call_data_length > 0 && !result.is_empty() {
+                        let bytes: Vec<(u8, bool)> = result.iter().map(|b| (*b, false)).collect();
+                        for (i, &(byte, _is_code)) in bytes.iter().enumerate() {
+                            // push callee memory write
+                            state.push_op(
+                                &mut exec_step,
+                                RW::WRITE,
+                                MemoryOp::new(call.call_id, i.into(), byte),
+                            );
+                        }
+                        state.push_copy(
                             &mut exec_step,
-                            RW::WRITE,
-                            MemoryOp::new(call.call_id, i.into(), byte),
+                            CopyEvent {
+                                src_id: NumberOrHash::Number(call.call_id),
+                                src_type: CopyDataType::Precompile(precompile_call),
+                                src_addr: 0,
+                                src_addr_end: result.len() as u64,
+                                dst_id: NumberOrHash::Number(call.call_id),
+                                dst_type: CopyDataType::Memory,
+                                dst_addr: 0,
+                                log_id: None,
+                                rw_counter_start,
+                                bytes: bytes.clone(),
+                            },
                         );
-                    }
-                    state.push_copy(
-                        &mut exec_step,
-                        CopyEvent {
-                            src_id: NumberOrHash::Number(call.call_id),
-                            src_type: CopyDataType::Precompile(precompile_call),
-                            src_addr: 0,
-                            src_addr_end: result.len() as u64,
-                            dst_id: NumberOrHash::Number(call.call_id),
-                            dst_type: CopyDataType::Memory,
-                            dst_addr: 0,
-                            log_id: None,
-                            rw_counter_start,
-                            bytes,
-                        },
-                    );
-                }
+                        Some(bytes.iter().map(|t| t.0).collect())
+                    } else {
+                        None
+                    };
 
                 // insert another copy event (output) for this step.
                 let rw_counter_start = state.block_ctx.rwc;
-                if call.is_success() && call.call_data_length > 0 && length > 0 {
+                let returned_bytes = if call.is_success() && call.call_data_length > 0 && length > 0
+                {
                     let bytes: Vec<(u8, bool)> =
                         result.iter().take(length).map(|b| (*b, false)).collect();
                     for (i, &(byte, _is_code)) in bytes.iter().enumerate() {
@@ -440,18 +447,20 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                             dst_addr: call.return_data_offset,
                             log_id: None,
                             rw_counter_start,
-                            bytes,
+                            bytes: bytes.clone(),
                         },
                     );
-                }
+                    Some(bytes.iter().map(|t| t.0).collect())
+                } else {
+                    None
+                };
 
-                // TODO: when more precompiles are supported and each have their own different
-                // behaviour, we can separate out the logic specified here.
                 let mut precompile_step = precompile_associated_ops(
                     state,
                     geth_steps[1].clone(),
                     call.clone(),
                     precompile_call,
+                    (input_bytes, output_bytes, returned_bytes),
                 )?;
 
                 // Make the Precompile execution step to handle return logic and restore to caller

bus-mapping/src/evm/opcodes/precompiles/mod.rs

@@ -1,24 +1,69 @@
-use eth_types::{GethExecStep, ToWord, Word};
+use eth_types::{
+    sign_types::{recover_pk, SignData},
+    Bytes, GethExecStep, ToBigEndian, ToWord, Word,
+};
+use halo2_proofs::halo2curves::secp256k1::Fq;
 
 use crate::{
     circuit_input_builder::{Call, CircuitInputStateRef, ExecState, ExecStep},
     operation::CallContextField,
-    precompile::PrecompileCalls,
+    precompile::{EcrecoverAuxData, PrecompileAuxData, PrecompileCalls},
     Error,
 };
 
+type InOutRetData = (Option<Vec<u8>>, Option<Vec<u8>>, Option<Vec<u8>>);
+
 pub fn gen_associated_ops(
     state: &mut CircuitInputStateRef,
     geth_step: GethExecStep,
     call: Call,
     precompile: PrecompileCalls,
+    (input_bytes, output_bytes, _returned_bytes): InOutRetData,
 ) -> Result<ExecStep, Error> {
     assert_eq!(call.code_address(), Some(precompile.into()));
     let mut exec_step = state.new_step(&geth_step)?;
     exec_step.exec_state = ExecState::Precompile(precompile);
 
     common_call_ctx_reads(state, &mut exec_step, &call);
 
+    // TODO: refactor and replace with `match` once we have more branches.
+    if precompile == PrecompileCalls::Ecrecover {
+        let input_bytes = input_bytes.map_or(vec![0u8; 128], |mut bytes| {
+            bytes.resize(128, 0u8);
+            bytes
+        });
+        let output_bytes = output_bytes.map_or(vec![0u8; 32], |mut bytes| {
+            bytes.resize(32, 0u8);
+            bytes
+        });
+        let aux_data = EcrecoverAuxData::new(input_bytes, output_bytes);
+
+        // only if sig_v was a valid recovery ID, then we proceed to populate the ecrecover events.
+        if let Some(sig_v) = aux_data.recovery_id() {
+            if let Ok(recovered_pk) = recover_pk(
+                sig_v,
+                &aux_data.sig_r,
+                &aux_data.sig_s,
+                &aux_data.msg_hash.to_be_bytes(),
+            ) {
+                let sign_data = SignData {
+                    signature: (
+                        Fq::from_bytes(&aux_data.sig_r.to_be_bytes()).unwrap(),
+                        Fq::from_bytes(&aux_data.sig_s.to_be_bytes()).unwrap(),
+                        sig_v,
+                    ),
+                    pk: recovered_pk,
+                    msg: Bytes::default(),
+                    msg_hash: Fq::from_bytes(&aux_data.msg_hash.to_be_bytes()).unwrap(),
+                };
+                assert_eq!(aux_data.recovered_addr, sign_data.get_addr());
+                state.push_ecrecover(sign_data);
+            }
+        }
+
+        exec_step.aux_data = Some(PrecompileAuxData::Ecrecover(aux_data));
+    }
+
     Ok(exec_step)
 }
 

bus-mapping/src/precompile.rs

@@ -1,6 +1,6 @@
 //! precompile helpers
 
-use eth_types::{evm_types::GasCost, Address};
+use eth_types::{evm_types::GasCost, Address, ToBigEndian, Word};
 use revm_precompile::{Precompile, Precompiles};
 use strum::EnumIter;
 
@@ -27,7 +27,7 @@ pub(crate) fn execute_precompiled(address: &Address, input: &[u8], gas: u64) ->
 #[derive(Copy, Clone, Debug, Eq, PartialEq, EnumIter)]
 pub enum PrecompileCalls {
     /// Elliptic Curve Recovery
-    ECRecover = 0x01,
+    Ecrecover = 0x01,
     /// SHA2-256 hash function
     Sha256 = 0x02,
     /// Ripemd-160 hash function
@@ -48,7 +48,7 @@ pub enum PrecompileCalls {
 
 impl Default for PrecompileCalls {
     fn default() -> Self {
-        Self::ECRecover
+        Self::Ecrecover
     }
 }
 
@@ -75,7 +75,7 @@ impl From<PrecompileCalls> for usize {
 impl From<u8> for PrecompileCalls {
     fn from(value: u8) -> Self {
         match value {
-            0x01 => Self::ECRecover,
+            0x01 => Self::Ecrecover,
             0x02 => Self::Sha256,
             0x03 => Self::Ripemd160,
             0x04 => Self::Identity,
@@ -93,7 +93,7 @@ impl PrecompileCalls {
     /// Get the base gas cost for the precompile call.
     pub fn base_gas_cost(&self) -> GasCost {
         match self {
-            Self::ECRecover => GasCost::PRECOMPILE_EC_RECOVER_BASE,
+            Self::Ecrecover => GasCost::PRECOMPILE_ECRECOVER_BASE,
             Self::Sha256 => GasCost::PRECOMPILE_SHA256_BASE,
             Self::Ripemd160 => GasCost::PRECOMPILE_RIPEMD160_BASE,
             Self::Identity => GasCost::PRECOMPILE_IDENTITY_BASE,
@@ -109,4 +109,72 @@ impl PrecompileCalls {
     pub fn address(&self) -> u64 {
         (*self).into()
     }
+
+    /// Maximum length of input bytes considered for the precompile call.
+    pub fn input_len(&self) -> Option<usize> {
+        match self {
+            Self::Ecrecover | Self::Bn128Add => Some(128),
+            Self::Bn128Mul => Some(96),
+            _ => None,
+        }
+    }
+}
+
+/// Auxiliary data for Ecrecover
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub struct EcrecoverAuxData {
+    /// Keccak hash of the message being signed.
+    pub msg_hash: Word,
+    /// v-component of signature.
+    pub sig_v: Word,
+    /// r-component of signature.
+    pub sig_r: Word,
+    /// s-component of signature.
+    pub sig_s: Word,
+    /// Address that was recovered.
+    pub recovered_addr: Address,
+}
+
+impl EcrecoverAuxData {
+    /// Create a new instance of ecrecover auxiliary data.
+    pub fn new(input: Vec<u8>, output: Vec<u8>) -> Self {
+        assert_eq!(input.len(), 128);
+        assert_eq!(output.len(), 32);
+
+        // assert that recovered address is 20 bytes.
+        assert!(output[0x00..0x0c].iter().all(|&b| b == 0));
+        let recovered_addr = Address::from_slice(&output[0x0c..0x20]);
+
+        Self {
+            msg_hash: Word::from_big_endian(&input[0x00..0x20]),
+            sig_v: Word::from_big_endian(&input[0x20..0x40]),
+            sig_r: Word::from_big_endian(&input[0x40..0x60]),
+            sig_s: Word::from_big_endian(&input[0x60..0x80]),
+            recovered_addr,
+        }
+    }
+
+    /// Sanity check and returns recovery ID.
+    pub fn recovery_id(&self) -> Option<u8> {
+        let sig_v_bytes = self.sig_v.to_be_bytes();
+        let sig_v = sig_v_bytes[31];
+        if sig_v_bytes.iter().take(31).all(|&b| b == 0) && (sig_v == 27 || sig_v == 28) {
+            Some(sig_v - 27)
+        } else {
+            None
+        }
+    }
+}
+
+/// Auxiliary data attached to an internal state for precompile verification.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum PrecompileAuxData {
+    /// Ecrecover.
+    Ecrecover(EcrecoverAuxData),
+}
+
+impl Default for PrecompileAuxData {
+    fn default() -> Self {
+        Self::Ecrecover(EcrecoverAuxData::default())
+    }
 }