Support invalid signatures #1
Conversation
| if tx.map(|x| x.invalid_signature) == Some(true) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Generally looks good, but of course this signature verification skip will have to be replaced with an actual signature check (see taikoxyz#70). Then when the circuit has checked that the signature is invalid, the tx lookup table should mark it as invalid and then the transaction execution needs to be skipped in the EVM circuit.
zkevm-circuits/Cargo.toml
Outdated
| ecc = { git = "https://github.com/privacy-scaling-explorations/halo2wrong", tag = "v2022_10_22" } | ||
| maingate = { git = "https://github.com/privacy-scaling-explorations/halo2wrong", tag = "v2022_10_22" } | ||
| integer = { git = "https://github.com/privacy-scaling-explorations/halo2wrong", tag = "v2022_10_22" } | ||
| ecdsa = { git = "https://github.com/xiaodino/halo2wrong", rev = "dbfbcd7bc4902957f8865ca319b303e7dc51066a" } |
There was a problem hiding this comment.
Use xiaodino/halo2wrong#2 Return VerifyFailure::ConstraintNotSatisfied with offset
| @@ -113,6 +113,7 @@ impl<'a, M: MultiMillerLoop> Circuit<M::Scalar> for RootCircuit<'a, M> { | |||
| config.aggregate::<M>(&mut layouter, &self.svk, [self.snark])?; | |||
|
|
|||
| // Constrain equality to instance values | |||
| /* | |||
There was a problem hiding this comment.
It's not related to the PR, but this part returns a compiler error.
| if tx.map(|x| x.invalid_signature) == Some(true) { | ||
| continue; | ||
| } |
zkevm-circuits/src/tx_circuit.rs
Outdated
| println!("VerifyFailure::ConstraintNotSatisfied at offset {:?}. Constraint {:?}", offset, constraint); | ||
| if let Some(ref c) = constraint { | ||
| if soft_check_constraints.contains(c) { | ||
| tx.invalid_signature = true; |
There was a problem hiding this comment.
I'm afraid this is check is not good enough to ensure the prover cannot lie about the signature being invalid. A circuit can fail for many reasons, and for this to work we would have to verify that a proof for a circuit would be invalid at some specific offset which we cannot do in a practical scenario in a secure way. The only way we can be sure that a signature is invalid is by having a valid circuit that shows that the signature is invalid. So we really need to modify the constraints to be able to do so. For example this is currently done in the ECDSA circuit constraints:
// 1. check 0 < r, s
scalar_chip.assert_not_zero(ctx, &sig.r)?;
scalar_chip.assert_not_zero(ctx, &sig.s)?;
This will only pass for valid signatures, so we cannot have this hard check anymore. To be able to support invalid signatures we have to modify the constraints so the constraints can actually be valid, so we have to modify these hard checks into soft checks by doing something like this:
// 1. check 0 < r, s, if r == 0 or s == 0 the signature is marked as invalid
let is_r_invalid = scalar_chip.is_not_zero(ctx, &sig.r)?;
let is_s_invalid = scalar_chip.is_not_zero(ctx, &sig.s)?;
// Something like this, but with constraints
let is_invalid = is_r_invalid || is_s_invalid;
It's very important all these things are checked using constraints in a valid circuit, and not in some other way because the prover can manipulate everything else not directly enforced in a valid circuit.
There was a problem hiding this comment.
Updated xiaodino/halo2wrong#2 to address what you said. Please take a look.
zkevm-circuits/src/tx_circuit.rs
Outdated
| // TODO: need to update this constrain_equal when it's invalid signature | ||
| TxFieldTag::CallerAddress => region.constrain_equal( | ||
| assigned_cell.cell(), | ||
| assigned_sig_verif.address.cell(), |
There was a problem hiding this comment.
If the signature is invalid, can we set the address to 0x0? This will match ecrecover behavior.
No description provided.