all: remove kilic dependency from bls12381 fuzzers #30296
Conversation
jwasinger
force-pushed
the
remove-kilic
branch
from
c4e99a1
to
084a110
Compare
|
Looks like this fails currently: |
tests/fuzzers/bls12381: add blst fuzzer for multiexp
| // compute pairing using blst | ||
| blstResult := blst.Fp12MillerLoop(blG2, blG1) | ||
| blstResult.FinalExp() | ||
| res := massageBLST(blstResult.ToBendian()) | ||
| if !(bytes.Equal(res, bls12381.NewGT().ToBytes(kResult))) { | ||
| if !(bytes.Equal(res, cResult.Marshal())) { | ||
| panic("pairing mismatch blst / geth") |
There was a problem hiding this comment.
This was there before, so just inquiring and not a blocker: Most of the functions below use panic("{string}) while the subgroup check fuzzing function uses panic(fmt.Sprintf) -- Is there a reason for this?
There was a problem hiding this comment.
I don't think there is a good/intentional reason. But I'd lean towards not printing the GT elements here as we already have the input that caused the diff.
|
This looks good to me! |
|
Ah there's one thing: We only fuzz the MSM precompiles using random inputs. I'm going to add two additional fuzzers that fuzz these with inputs that are known to be valid. |
I think it would be good to add the identity/zero point as input to one of these, along with other known valid points |
|
Yeah definitely. Actually, I need to give the added fuzzers a longer run and prob adjust the seed corpus to increase the coverage. |
| @@ -51,9 +51,9 @@ func FuzzG1Add(f *testing.F) { | |||
| }) | |||
| } | |||
|
|
|||
| func FuzzG1Mul(f *testing.F) { | |||
| func FuzzCrossG1Mul(f *testing.F) { | |||
There was a problem hiding this comment.
If you change these names, or add new targets, you need to also update oss-fuzz.sh accordingly
There was a problem hiding this comment.
I think you should just leave this name be instead
| func FuzzCrossG1Mul(f *testing.F) { | |
| func FuzzG1Mul(f *testing.F) { |
There was a problem hiding this comment.
I think you should just leave this name be instead
The PR expands these to be cross-library fuzzers. Doesn't it make sense to have consistent naming with the other fuzzers of the same type?
There was a problem hiding this comment.
Ok, but if so you need to update oss-fuzz.sh
| @@ -69,9 +69,9 @@ func FuzzG2Add(f *testing.F) { | |||
| }) | |||
| } | |||
|
|
|||
| func FuzzG2Mul(f *testing.F) { | |||
| func FuzzCrossG2Mul(f *testing.F) { | |||
There was a problem hiding this comment.
| func FuzzCrossG2Mul(f *testing.F) { | |
| func FuzzG2Mul(f *testing.F) { |
The kilic bls12381 implementation has been archived. It shouldn't be necessary to include it as a fuzzing target any longer.
This also adds fuzzers for G1/G2 mul that use inputs that are guaranteed to be valid. Previously, we just did random input fuzzing for these precompiles.