Skip to content

Commit 049c627

Browse files
committed
constrain x y canonical
1 parent 025ddd7 commit 049c627

File tree

2 files changed

+32
-16
lines changed

2 files changed

+32
-16
lines changed

zkevm-circuits/src/evm_circuit/execution.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ use pc::PcGadget;
216216
use pop::PopGadget;
217217
use precompiles::{
218218
BasePrecompileGadget, EcAddGadget, EcMulGadget, EcPairingGadget, EcrecoverGadget,
219-
IdentityGadget, ModExpGadget, SHA256Gadget,
219+
IdentityGadget, ModExpGadget, SHA256Gadget, P256VerifyGadget,
220220
};
221221
use push::PushGadget;
222222
use return_revert::ReturnRevertGadget;
@@ -381,6 +381,7 @@ pub(crate) struct ExecutionConfig<F> {
381381
precompile_bn128mul_gadget: Box<EcMulGadget<F>>,
382382
precompile_bn128pairing_gadget: Box<EcPairingGadget<F>>,
383383
precompile_blake2f_gadget: Box<BasePrecompileGadget<F, { ExecutionState::PrecompileBlake2f }>>,
384+
precompile_p256verify_gadget: Box<P256VerifyGadget<F>>,
384385
}
385386

386387
impl<F: Field> ExecutionConfig<F> {
@@ -1746,6 +1747,9 @@ impl<F: Field> ExecutionConfig<F> {
17461747
ExecutionState::PrecompileBlake2f => {
17471748
assign_exec_step!(self.precompile_blake2f_gadget)
17481749
}
1750+
ExecutionState::PrecompileP256Verify => {
1751+
assign_exec_step!(self.precompile_p256verify_gadget)
1752+
}
17491753
}
17501754

17511755
// Fill in the witness values for stored expressions

zkevm-circuits/src/evm_circuit/execution/precompiles/p256_verify.rs

+27-15
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ pub struct P256VerifyGadget<F> {
6767
pk_x_canonical: LtWordGadget<F>,
6868
pk_y: Word<F>,
6969
pk_y_canonical: LtWordGadget<F>,
70+
is_valid: Cell<F>,
7071

7172
is_success: Cell<F>,
7273
callee_address: Cell<F>,
@@ -90,11 +91,13 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
9091
cb.query_cell_phase2(),
9192
);
9293
let (
94+
is_valid,
9395
msg_hash_keccak_rlc,
9496
sig_r_keccak_rlc,
9597
sig_s_keccak_rlc,
9698
//recovered_addr_keccak_rlc,
9799
) = (
100+
cb.query_bool(),
98101
cb.query_cell_phase2(),
99102
cb.query_cell_phase2(),
100103
cb.query_cell_phase2(),
@@ -119,6 +122,8 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
119122
let pk_x_canonical = LtWordGadget::construct(cb, &pk_x, &fp_modulus);
120123
let pk_y_canonical = LtWordGadget::construct(cb, &pk_y, &fp_modulus);
121124

125+
let x_y_canonical = and::expr([pk_x_canonical.expr(), pk_y_canonical.expr()]);
126+
122127
cb.require_equal(
123128
"msg hash cells assigned incorrectly",
124129
msg_hash_keccak_rlc.expr(),
@@ -199,20 +204,25 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
199204
0.expr(),
200205
sig_r.expr(),
201206
sig_s.expr(),
202-
select::expr(
203-
recovered.expr(),
204-
from_bytes::expr(&recovered_addr_keccak_rlc.cells),
205-
0.expr(),
206-
),
207-
recovered.expr(),
207+
// recovered addr set to 0.
208+
0.expr(),
209+
is_valid.expr(),
208210
);
209211
},
210212
);
211-
// TODO: check x, y is canonical
213+
// check r, s is canonical
212214
cb.condition(not::expr(r_s_canonical.expr()), |cb| {
213215
cb.require_zero(
214-
"recovered == false if r or s not canonical",
215-
recovered.expr(),
216+
"is_valid == false if r or s not canonical",
217+
is_valid.expr(),
218+
);
219+
});
220+
221+
// check x, y is canonical
222+
cb.condition(not::expr(x_y_canonical.expr()), |cb| {
223+
cb.require_zero(
224+
"is_valid == false if x or y not canonical",
225+
is_valid.expr(),
216226
);
217227
});
218228
// cb.condition(not::expr(recovered.expr()), |cb| {
@@ -283,13 +293,10 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
283293

284294
pad_right,
285295
padding,
286-
287-
recovered,
288296
msg_hash_keccak_rlc,
289-
sig_v_keccak_rlc,
290297
sig_r_keccak_rlc,
291298
sig_s_keccak_rlc,
292-
recovered_addr_keccak_rlc,
299+
//recovered_addr_keccak_rlc,
293300

294301
msg_hash_raw,
295302
msg_hash,
@@ -380,6 +387,8 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
380387
(&self.msg_hash_raw, aux_data.msg_hash),
381388
(&self.sig_r, aux_data.sig_r),
382389
(&self.sig_s, aux_data.sig_s),
390+
(&self.pk_x, aux_data.pubkey_x),
391+
(&self.pk_y, aux_data.pubkey_y),
383392
] {
384393
word_rlc.assign(region, offset, Some(value.to_le_bytes()))?;
385394
}
@@ -402,6 +411,9 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
402411
.assign(region, offset, aux_data.sig_r, *FQ_MODULUS)?;
403412
self.sig_s_canonical
404413
.assign(region, offset, aux_data.sig_s, *FQ_MODULUS)?;
414+
// assign pk_x_canonical, pk_y_canonical
415+
self.pk_x_canonical.assign(region, offset, aux_data.pubkey_x, *FP_MODULUS)?;
416+
self.pk_y_canonical.assign(region, offset, aux_data.pubkey_y, *FP_MODULUS)?;
405417
// self.recovered_addr_keccak_rlc.assign(
406418
// region,
407419
// offset,
@@ -416,7 +428,7 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
416428
self.padding.assign(
417429
region,
418430
offset,
419-
PrecompileCalls::Ecrecover,
431+
PrecompileCalls::P256Verify,
420432
region
421433
.challenges()
422434
.keccak_input()
@@ -425,7 +437,7 @@ impl<F: Field> ExecutionGadget<F> for P256VerifyGadget<F> {
425437
region.challenges().keccak_input(),
426438
)?;
427439
} else {
428-
log::error!("unexpected aux_data {:?} for ecrecover", step.aux_data);
440+
log::error!("unexpected aux_data {:?} for p256verify", step.aux_data);
429441
return Err(Error::Synthesis);
430442
}
431443

0 commit comments

Comments
 (0)