status-im / nim-blscurve

Nim implementation of BLS signature scheme (Boneh-Lynn-Shacham) over Barreto-Lynn-Scott (BLS) curve BLS12-381
Apache License 2.0
26 stars 11 forks source link

Fix BLST aggregate verify with proof-of-possession #118

Closed etan-k closed 3 years ago

etan-k commented 3 years ago

In #100 a regression was introduced to BLST fastAggregateVerify.

Previous code:

  var aggAffine{.noInit.}: PublicKey
  aggAffine.point.blst_p1_to_affine(aggregate)
  return coreVerifyNoGroupCheck(aggAffine, message, signature, DST)

New code introducing regression:

  var aggAffine{.noInit.}: PublicKey
  aggAffine.finish(aggAffine)
  return coreVerifyNoGroupCheck(aggAffine, message, signature, DST)

This change led to a compilation error when using fastAggregateVerify with proof-of-possession.

Secondly, aggregateVerify with proof-possession also fails to compile. This was never working, ever since BLST support was introduced in #68.

Problematic code:

  if publicKeys.len != proofs.len or publicKeys != messages.len:
     return false

This patch addresses both compilation problems and extends the existing tests to also cover proof-of-possession functionality. Because the Eth2 vectors do not include proof-of-possession data, the test generator was temporarily extended to produce such reference data. A copy of that data is hardcoded in the eth2_vectors tests.

mratsim commented 3 years ago

Awesome, thank you!