ECDSA signature verification panics if provided invalid signature/public key

[colinnielsen]

Aim

I am trying to wrap up my dark-safe circuit:

I need a variable amount of items in an input array, so I'm using loop with some fixed MAX_ITERATIONS, then an should_calculate boolean in the input structs as a "no-op" flag.

There is an if block within the for loop, which looks for a should_calculate flag in the struct, and no-ops otherwise.

Expected Behavior

The program should not panic

Bug

Given a Prover.toml with 4 valid inputs and with 4 "empty" structs, running nargo execute, I see 4 println statements echo the array index, and "computing", then the program crashes at the blackbox-solver file.

Running the nargo execute -p provers/Prover_full.toml with a full array Prover_full.toml works as expected

NOTE:

• If I place the "empty" structs first in the Prover.toml, it will crash on the first iteration in the no-op
• I have also tried "flipping" the if statement, and putting the computation in the else branch. This makes no difference to either above case. The program crashes whenever it hits an empty struct in the loop.
• Interestingly enough, if I only pass empty structs in via nargo execute, the program will fail on the first iteration

To Reproduce

1. git clone https://github.com/colinnielsen/dark-safe.git && cd dark-safe/circuits
2. either nargo execute, nargo execute -p provers/Prover_completely_empty.toml, nargo execute -p provers/Prover_full.toml, or nargo execute -p provers/Prover_empty_first.toml

If you want to play with your own Prover.toml, (with a valid polynomial and array of signatures) run:

1. yarn install
2. yarn build
3. follow the cli prompt

Installation Method

Binary

Nargo Version

nargo 0.12.0 (git version hash: 13cc23b, is dirty: false)

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

[TomAFrench]

Thanks for reporting this @colinnielsen. The underlying issue is that verify_secp256k1_ecdsa_signature panics rather than returning false if we provide invalid signatures like you're doing here due to an error within the k256 crate.

I'll go through and update this function to return false in this case rather than panicking.

[TomAFrench]

@colinnielsen I've tested dark-safe with the fix in noir-lang/acvm#557 and I now get a failed constraint when asserting the polynomial evaluates to zero rather than a panic.

[colinnielsen]

@colinnielsen I've tested dark-safe with the fix in noir-lang/acvm#557 and I now get a failed constraint when asserting the polynomial evaluates to zero rather than a panic.

Awesome! Thank you, @TomAFrench

[TomAFrench]

This issue is closed now but Noir isn't currently pulling from the ACVM code in this repository yet.