Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Commit

Permalink
fix: sig_v handle invalid values (lookup) and is_valid dev_load fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@roynalnaruto
roynalnaruto committed Sep 12, 2023
1 parent bb05086 commit bf46ab3
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 5 deletions.
4 changes: 4 additions & 0 deletions eth-types/src/sign_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use halo2_proofs::{
halo2curves::{
group::{
ff::{Field as GroupField, PrimeField},
prime::PrimeCurveAffine,
Curve,
},
secp256k1::{self, Secp256k1Affine},
Expand Down Expand Up @@ -101,6 +102,9 @@ pub fn get_dummy_tx() -> (TransactionRequest, Signature) {
impl SignData {
/// Recover address of the signature
pub fn get_addr(&self) -> Address {
if self.pk == Secp256k1Affine::identity() {
return Address::zero();
}
let pk_le = pk_bytes_le(&self.pk);
let pk_be = pk_bytes_swap_endianness(&pk_le);
let pk_hash = keccak256(pk_be);
Expand Down
151 changes: 148 additions & 3 deletions zkevm-circuits/src/evm_circuit/execution/precompiles/ecrecover.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bus_mapping::precompile::PrecompileAuxData;
use eth_types::{evm_types::GasCost, word, Field, ToLittleEndian, ToScalar, U256};
use gadgets::util::{and, not, select, Expr};
use gadgets::util::{and, not, or, select, sum, Expr};
use halo2_proofs::{
circuit::Value,
plonk::{Error, Expression},
Expand All @@ -15,7 +15,7 @@ use crate::{
common_gadget::RestoreContextGadget,
constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder},
from_bytes,
math_gadget::{LtWordGadget, ModGadget},
math_gadget::{IsEqualGadget, IsZeroGadget, LtWordGadget, ModGadget},
rlc, CachedRegion, Cell, RandomLinearCombination, Word,
},
},
Expand Down Expand Up @@ -48,6 +48,11 @@ pub struct EcrecoverGadget<F> {
sig_s: Word<F>,
sig_s_canonical: LtWordGadget<F>,

sig_v: Word<F>,
sig_v_one_byte: IsZeroGadget<F>,
sig_v_eq27: IsEqualGadget<F>,
sig_v_eq28: IsEqualGadget<F>,

is_success: Cell<F>,
callee_address: Cell<F>,
caller_id: Cell<F>,
Expand Down Expand Up @@ -91,6 +96,17 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
let sig_s_canonical = LtWordGadget::construct(cb, &sig_s, &fq_modulus);
let r_s_canonical = and::expr([sig_r_canonical.expr(), sig_s_canonical.expr()]);

// sig_v is valid if sig_v == 27 || sig_v == 28
let sig_v = cb.query_word_rlc();
let sig_v_rest_bytes = sum::expr(&sig_v.cells[1..]);
let sig_v_one_byte = IsZeroGadget::construct(cb, sig_v_rest_bytes);
let sig_v_eq27 = IsEqualGadget::construct(cb, sig_v.cells[0].expr(), 27.expr());
let sig_v_eq28 = IsEqualGadget::construct(cb, sig_v.cells[0].expr(), 28.expr());
let sig_v_valid = and::expr([
or::expr([sig_v_eq27.expr(), sig_v_eq28.expr()]),
sig_v_one_byte.expr(),
]);

cb.require_equal(
"msg hash cells assigned incorrectly",
msg_hash_keccak_rlc.expr(),
Expand Down Expand Up @@ -130,6 +146,19 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
.expect("msg hash is 32 bytes"),
),
);
cb.require_equal(
"sig_v cells assigned incorrectly",
sig_v_keccak_rlc.expr(),
cb.keccak_rlc::<N_BYTES_WORD>(
sig_v
.cells
.iter()
.map(Expr::expr)
.collect::<Vec<Expression<F>>>()
.try_into()
.expect("sig_v is 32 bytes"),
),
);
cb.require_equal(
"Secp256k1::Fq modulus assigned correctly",
fq_modulus.expr(),
Expand Down Expand Up @@ -160,7 +189,11 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
cb.condition(r_s_canonical.expr(), |cb| {
cb.sig_table_lookup(
msg_hash.expr(),
sig_v_keccak_rlc.expr() - 27.expr(),
select::expr(
sig_v_valid,
sig_v.cells[0].expr() - 27.expr(),
sig_v.cells[0].expr(),
),
sig_r.expr(),
sig_s.expr(),
select::expr(
Expand Down Expand Up @@ -219,6 +252,11 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
sig_s,
sig_s_canonical,

sig_v,
sig_v_one_byte,
sig_v_eq27,
sig_v_eq28,

is_success,
callee_address,
caller_id,
Expand Down Expand Up @@ -279,6 +317,7 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
(&self.msg_hash_raw, aux_data.msg_hash),
(&self.sig_r, aux_data.sig_r),
(&self.sig_s, aux_data.sig_s),
(&self.sig_v, aux_data.sig_v),
] {
word_rlc.assign(region, offset, Some(value.to_le_bytes()))?;
}
Expand All @@ -299,6 +338,31 @@ impl<F: Field> ExecutionGadget<F> for EcrecoverGadget<F> {
.assign(region, offset, aux_data.sig_r, *FQ_MODULUS)?;
self.sig_s_canonical
.assign(region, offset, aux_data.sig_s, *FQ_MODULUS)?;
self.sig_v_one_byte.assign(
region,
offset,
F::from(
aux_data
.sig_v
.to_le_bytes()
.into_iter()
.skip(1)
.map(|b| b as u64)
.sum::<u64>(),
),
)?;
self.sig_v_eq27.assign(
region,
offset,
F::from(aux_data.sig_v.to_le_bytes()[0] as u64),
F::from(27),
)?;
self.sig_v_eq28.assign(
region,
offset,
F::from(aux_data.sig_v.to_le_bytes()[0] as u64),
F::from(28),
)?;
self.recovered_addr_keccak_rlc.assign(
region,
offset,
Expand Down Expand Up @@ -538,6 +602,87 @@ mod test {
address: PrecompileCalls::Ecrecover.address().to_word(),
..Default::default()
},
PrecompileCallArgs {
name: "ecrecover (invalid v > 28, single byte)",
setup_code: bytecode! {
// msg hash from 0x00
PUSH32(word!("0x456e9aea5e197a1f1af7a3e85a3212fa4049a3ba34c2289b4c860fc0b0c64ef3"))
PUSH1(0x00)
MSTORE
// signature v from 0x20
PUSH1(29)
PUSH1(0x20)
MSTORE
// signature r from 0x40
PUSH32(word!("0x9242685bf161793cc25603c231bc2f568eb630ea16aa137d2664ac8038825608"))
PUSH1(0x40)
MSTORE
// signature s from 0x60
PUSH32(word!("0x4f8ae3bd7535248d0bd448298cc2e2071e56992d0774dc340c368ae950852ada"))
PUSH1(0x60)
MSTORE
},
call_data_offset: 0x00.into(),
call_data_length: 0x80.into(),
ret_offset: 0x80.into(),
ret_size: 0x20.into(),
address: PrecompileCalls::Ecrecover.address().to_word(),
..Default::default()
},
PrecompileCallArgs {
name: "ecrecover (invalid v < 27, single byte)",
setup_code: bytecode! {
// msg hash from 0x00
PUSH32(word!("0x456e9aea5e197a1f1af7a3e85a3212fa4049a3ba34c2289b4c860fc0b0c64ef3"))
PUSH1(0x00)
MSTORE
// signature v from 0x20
PUSH1(26)
PUSH1(0x20)
MSTORE
// signature r from 0x40
PUSH32(word!("0x9242685bf161793cc25603c231bc2f568eb630ea16aa137d2664ac8038825608"))
PUSH1(0x40)
MSTORE
// signature s from 0x60
PUSH32(word!("0x4f8ae3bd7535248d0bd448298cc2e2071e56992d0774dc340c368ae950852ada"))
PUSH1(0x60)
MSTORE
},
call_data_offset: 0x00.into(),
call_data_length: 0x80.into(),
ret_offset: 0x80.into(),
ret_size: 0x20.into(),
address: PrecompileCalls::Ecrecover.address().to_word(),
..Default::default()
},
PrecompileCallArgs {
name: "ecrecover (invalid v, multi-byte, last byte == 28)",
setup_code: bytecode! {
// msg hash from 0x00
PUSH32(word!("0x456e9aea5e197a1f1af7a3e85a3212fa4049a3ba34c2289b4c860fc0b0c64ef3"))
PUSH1(0x00)
MSTORE
// signature v from 0x20, 1c == 28 (but not single byte)
PUSH32(word!("0x010000000000000000000000000000000000000000000000000000000000001c"))
PUSH1(0x20)
MSTORE
// signature r from 0x40
PUSH32(word!("0x9242685bf161793cc25603c231bc2f568eb630ea16aa137d2664ac8038825608"))
PUSH1(0x40)
MSTORE
// signature s from 0x60
PUSH32(word!("0x4f8ae3bd7535248d0bd448298cc2e2071e56992d0774dc340c368ae950852ada"))
PUSH1(0x60)
MSTORE
},
call_data_offset: 0x00.into(),
call_data_length: 0x80.into(),
ret_offset: 0x80.into(),
ret_size: 0x20.into(),
address: PrecompileCalls::Ecrecover.address().to_word(),
..Default::default()
},
]
};

Expand Down
7 changes: 5 additions & 2 deletions zkevm-circuits/src/table.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2268,7 +2268,6 @@ impl SigTable {
});
let sig_v = Value::known(F::from(sign_data.signature.2 as u64));
let recovered_addr = Value::known(sign_data.get_addr().to_scalar().unwrap());

region.assign_fixed(
|| format!("sig table q_enable {offset}"),
self.q_enable,
Expand All @@ -2281,7 +2280,11 @@ impl SigTable {
("sig_r_rlc", self.sig_r_rlc, sig_r_rlc),
("sig_s_rlc", self.sig_s_rlc, sig_s_rlc),
("recovered_addr", self.recovered_addr, recovered_addr),
("is_valid", self.is_valid, Value::known(F::one())),
(
"is_valid",
self.is_valid,
Value::known(F::from(!sign_data.get_addr().is_zero())),
),
] {
region.assign_advice(
|| format!("sig table {column_name} {offset}"),
Expand Down

0 comments on commit bf46ab3

Please sign in to comment.