From 58e27c8f839e59777c9ed67a6eaa86438a99c7dd Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Mon, 20 Feb 2023 18:38:21 +0800 Subject: [PATCH 1/6] add assertion that sign_data is valid --- zkevm-circuits/src/tx_circuit.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 0a90fa55a8..0254c4fe93 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -10,16 +10,13 @@ use crate::evm_circuit::util::constraint_builder::BaseConstraintBuilder; use crate::table::{KeccakTable, LookupTable, RlpTable, TxFieldTag, TxTable}; #[cfg(not(feature = "enable-sign-verify"))] use crate::tx_circuit::sign_verify::pub_key_hash_to_address; -use crate::util::{random_linear_combine_word as rlc, SubCircuit, SubCircuitConfig}; +use crate::util::{keccak, random_linear_combine_word as rlc, SubCircuit, SubCircuitConfig}; use crate::witness; use crate::witness::{RlpDataType, RlpTxTag, Transaction}; use bus_mapping::circuit_input_builder::keccak_inputs_sign_verify; #[cfg(not(feature = "enable-sign-verify"))] use eth_types::sign_types::{pk_bytes_le, pk_bytes_swap_endianness}; -use eth_types::{ - sign_types::SignData, - {Field, ToLittleEndian, ToScalar}, -}; +use eth_types::{sign_types::SignData, {Field, ToLittleEndian, ToScalar}, ToAddress}; #[cfg(not(feature = "enable-sign-verify"))] use ethers_core::utils::keccak256; use gadgets::binary_number::{BinaryNumberChip, BinaryNumberConfig}; @@ -1675,6 +1672,16 @@ impl SubCircuit for TxCircuit { .collect::, Error>>()?; config.load_aux_tables(layouter)?; + + // assert tx.caller_address == recovered_pk + for (sign_data, tx) in keccak_inputs_sign_verify(&sign_datas) + .into_iter().zip(self.txs.iter()) + { + let pk_hash = keccak(&sign_data); + let address = pk_hash.to_address(); + assert_eq!(address, tx.caller_address); + } + #[cfg(feature = "enable-sign-verify")] { let assigned_sig_verifs = From ab1befdad36f095954dc751774b5e53f8c37ffdb Mon Sep 17 00:00:00 2001 From: zhenfei Date: Mon, 20 Feb 2023 09:16:52 -0500 Subject: [PATCH 2/6] fix pk endianess bug --- circuit-benchmarks/src/bench_params.rs | 1 + zkevm-circuits/src/tx_circuit.rs | 8 ++++++-- zkevm-circuits/src/tx_circuit/sign_verify.rs | 7 +++++-- 3 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 circuit-benchmarks/src/bench_params.rs diff --git a/circuit-benchmarks/src/bench_params.rs b/circuit-benchmarks/src/bench_params.rs new file mode 100644 index 0000000000..bfeb15511d --- /dev/null +++ b/circuit-benchmarks/src/bench_params.rs @@ -0,0 +1 @@ +pub(crate) const DEGREE: usize = 19; diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 0254c4fe93..09dc441631 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -16,7 +16,10 @@ use crate::witness::{RlpDataType, RlpTxTag, Transaction}; use bus_mapping::circuit_input_builder::keccak_inputs_sign_verify; #[cfg(not(feature = "enable-sign-verify"))] use eth_types::sign_types::{pk_bytes_le, pk_bytes_swap_endianness}; -use eth_types::{sign_types::SignData, {Field, ToLittleEndian, ToScalar}, ToAddress}; +use eth_types::{ + sign_types::SignData, + ToAddress, {Field, ToLittleEndian, ToScalar}, +}; #[cfg(not(feature = "enable-sign-verify"))] use ethers_core::utils::keccak256; use gadgets::binary_number::{BinaryNumberChip, BinaryNumberConfig}; @@ -1675,7 +1678,8 @@ impl SubCircuit for TxCircuit { // assert tx.caller_address == recovered_pk for (sign_data, tx) in keccak_inputs_sign_verify(&sign_datas) - .into_iter().zip(self.txs.iter()) + .into_iter() + .zip(self.txs.iter()) { let pk_hash = keccak(&sign_data); let address = pk_hash.to_address(); diff --git a/zkevm-circuits/src/tx_circuit/sign_verify.rs b/zkevm-circuits/src/tx_circuit/sign_verify.rs index 419fec6b39..bf63aadf18 100644 --- a/zkevm-circuits/src/tx_circuit/sign_verify.rs +++ b/zkevm-circuits/src/tx_circuit/sign_verify.rs @@ -384,12 +384,15 @@ impl SignVerifyChip { // address is the random linear combination of the public key // it is fine to use a phase 1 gate here + let (_pk, _, address) = ecdsa_chip.range.gate.inner_product( ctx, - &powers_of_256_cells[0..20].to_vec(), - &pk_hash_cells[12..].to_vec(), + &powers_of_256_cells[..20].to_vec(), + &pk_hash_cells[..20].to_vec(), )?; + println!("address in circuit {:?}", address); + let is_address_zero = ecdsa_chip.range.is_equal( ctx, &QuantumCell::Existing(&address), From 7a8b263f0bd89142e314b308fe8f106c6fae735c Mon Sep 17 00:00:00 2001 From: zhenfei Date: Mon, 20 Feb 2023 09:22:51 -0500 Subject: [PATCH 3/6] clean up --- zkevm-circuits/src/tx_circuit/sign_verify.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit/sign_verify.rs b/zkevm-circuits/src/tx_circuit/sign_verify.rs index bf63aadf18..d10fa08c9b 100644 --- a/zkevm-circuits/src/tx_circuit/sign_verify.rs +++ b/zkevm-circuits/src/tx_circuit/sign_verify.rs @@ -384,15 +384,12 @@ impl SignVerifyChip { // address is the random linear combination of the public key // it is fine to use a phase 1 gate here - let (_pk, _, address) = ecdsa_chip.range.gate.inner_product( ctx, &powers_of_256_cells[..20].to_vec(), &pk_hash_cells[..20].to_vec(), )?; - println!("address in circuit {:?}", address); - let is_address_zero = ecdsa_chip.range.is_equal( ctx, &QuantumCell::Existing(&address), From 89fafcd8034338adf8a5945433adb36c944748c7 Mon Sep 17 00:00:00 2001 From: zhenfei Date: Mon, 20 Feb 2023 09:39:40 -0500 Subject: [PATCH 4/6] address comments --- circuit-benchmarks/src/bench_params.rs | 1 - zkevm-circuits/src/tx_circuit.rs | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/circuit-benchmarks/src/bench_params.rs b/circuit-benchmarks/src/bench_params.rs index bfeb15511d..e69de29bb2 100644 --- a/circuit-benchmarks/src/bench_params.rs +++ b/circuit-benchmarks/src/bench_params.rs @@ -1 +0,0 @@ -pub(crate) const DEGREE: usize = 19; diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 09dc441631..0ea4baf96f 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -1683,7 +1683,13 @@ impl SubCircuit for TxCircuit { { let pk_hash = keccak(&sign_data); let address = pk_hash.to_address(); - assert_eq!(address, tx.caller_address); + if address != tx.caller_address { + log::error!( + "pk address from sign data {:?} does not match the one from tx address {:?}", + address, + tx.caller_address + ) + } } #[cfg(feature = "enable-sign-verify")] From ce931612946e2bf947784a936c9be8b37038885f Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 21 Feb 2023 11:10:18 +0800 Subject: [PATCH 5/6] fix errors in the sign_data check --- zkevm-circuits/src/tx_circuit.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 0ea4baf96f..d3df01dcf6 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -1676,12 +1676,20 @@ impl SubCircuit for TxCircuit { config.load_aux_tables(layouter)?; - // assert tx.caller_address == recovered_pk - for (sign_data, tx) in keccak_inputs_sign_verify(&sign_datas) + // check if tx.caller_address == recovered_pk + let recovered_pks = keccak_inputs_sign_verify(&sign_datas) .into_iter() - .zip(self.txs.iter()) - { - let pk_hash = keccak(&sign_data); + .enumerate() + .filter(|(idx, _)| { + // each sign_data produce two inputs for hashing + // pk -> pk_hash, msg -> msg_hash + idx % 2 == 0 + }) + .map(|(_, input)| input) + .collect::>(); + + for (pk, tx) in recovered_pks.into_iter().zip(self.txs.iter()) { + let pk_hash = keccak(&pk); let address = pk_hash.to_address(); if address != tx.caller_address { log::error!( From 0f70fcc30f253512f09e1dca7f978de8f67aadbe Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Tue, 21 Feb 2023 11:20:30 +0800 Subject: [PATCH 6/6] Delete bench_params.rs --- circuit-benchmarks/src/bench_params.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 circuit-benchmarks/src/bench_params.rs diff --git a/circuit-benchmarks/src/bench_params.rs b/circuit-benchmarks/src/bench_params.rs deleted file mode 100644 index e69de29bb2..0000000000