Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Fix/ecdsa err Fix #344 #345

Merged
merged 7 commits into from
Feb 21, 2023
Merged

Fix/ecdsa err Fix #344 #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
58e27c8
add assertion that sign_data is valid
kunxian-xia Feb 20, 2023
ab1befd
fix pk endianess bug
zhenfeizhang Feb 20, 2023
7a8b263
clean up
zhenfeizhang Feb 20, 2023
89fafcd
address comments
zhenfeizhang Feb 20, 2023
ce93161
fix errors in the sign_data check
kunxian-xia Feb 21, 2023
0f70fcc
Delete bench_params.rs
lispc Feb 21, 2023
2b9cf3f
Merge branch 'scroll-stable' into fix/ecdsa_err
lispc Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions zkevm-circuits/src/tx_circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ 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},
ToAddress, {Field, ToLittleEndian, ToScalar},
};
#[cfg(not(feature = "enable-sign-verify"))]
use ethers_core::utils::keccak256;
Expand Down Expand Up @@ -1675,6 +1675,31 @@ impl<F: Field> SubCircuit<F> for TxCircuit<F> {
.collect::<Result<Vec<SignData>, Error>>()?;

config.load_aux_tables(layouter)?;

// check if tx.caller_address == recovered_pk
let recovered_pks = keccak_inputs_sign_verify(&sign_datas)
.into_iter()
.enumerate()
.filter(|(idx, _)| {
// each sign_data produce two inputs for hashing
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool! and LGTM!

// pk -> pk_hash, msg -> msg_hash
idx % 2 == 0
})
.map(|(_, input)| input)
.collect::<Vec<_>>();

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!(
"pk address from sign data {:?} does not match the one from tx address {:?}",
address,
tx.caller_address
)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest change to log::error! so the test tx_circuit_bad_address will not panic

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed with 89fafcd


#[cfg(feature = "enable-sign-verify")]
{
let assigned_sig_verifs =
Expand Down
4 changes: 2 additions & 2 deletions zkevm-circuits/src/tx_circuit/sign_verify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,8 @@ impl<F: Field> SignVerifyChip<F> {
// 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(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pk_hash_cells is in little endian order?

&powers_of_256_cells[..20].to_vec(),
&pk_hash_cells[..20].to_vec(),
)?;

let is_address_zero = ecdsa_chip.range.is_equal(
Expand Down