fast-verification-of-multiple-bls-signatures #67
Conversation
ChihChengLiang
force-pushed
the
fast-verification-of-multiple-bls-signatures
branch
4 times, most recently
from
cd57296 to
026b2f5
Compare
tests/test_bls.py
Outdated
|
|
||
| class Attestation: | ||
| def __init__(self, msg_1, msg_2): | ||
| msg_1_validators = sample(validator_indices, 3) |
There was a problem hiding this comment.
Anytime we use randomness in tests we should be using the mechanisms provided by hypothesis since they allow us to reproduce failures when they occur. Otherwise failures due to random data will be opaque and we won't be able to recreate the conditions that produced them.
See: https://hypothesis.readthedocs.io/en/latest/data.html#hypothesis.strategies.random_module
There was a problem hiding this comment.
Removed randomness for now. Planning to rewrite bls tests with pytest fixtures and hypothesis.
| @@ -109,3 +113,48 @@ def verify_multiple(pubkeys: Sequence[BLSPubkey], | |||
| return final_exponentiation == FQ12.one() | |||
| except (ValidationError, ValueError, AssertionError): | |||
| return False | |||
|
|
|||
|
|
|||
| def verify_multiple_multiple( | |||
There was a problem hiding this comment.
Renaming ideas: optimized_verify_multiple, fast_verify_multiple...
There was a problem hiding this comment.
If it's faster why not just replace the existing implementation instead of maintaining two?
Also, if there's an existing one and we are keeping both around, seems prudent to test them against each other to ensure they have equal behavior (sorry if I missed the test that does this)
There was a problem hiding this comment.
The new function is an optimized version of doing multiple times of verify_multiple. The later verifies a single attestation and the former verifies many attestations in a block.
There was a problem hiding this comment.
I do desperately want to get rid of verify_multiple_multiple and rename it properly. Besides the name of the function, the function parameter pubkeys_and_messages: Sequence[Tuple[Sequence[BLSPubkey], Sequence[Hash32]]] is also confusing. Should we create some objects like
class SignedMessage:
signature: BLSSignature
public_keys: Sequence[BLSPubkey]
message_hashes: Sequence[Hash32]
def __init__(self, signature, public_keys, message_hashes):
self.signature = signature
if len(public_keys) != len(message_hashes)
raise ValidationError()
self.public_keys = public_keys
self.message_hashes = message_hashesThen write the function parameters in this fashion?
def verify_multiple(signed_message: SignedMessage):
def verify_multiple_multiple(signed_messages: Sequence[SignedMessage]): 
There was a problem hiding this comment.
I do desperately want to get rid of verify_multiple_multiple and rename it properly.
how about batch_verify ?
There was a problem hiding this comment.
Sounds good. We could also do "verify_multiple_aggregate_signatures" to match milagro's function name.
|
benchmark before rename and pr feedback
|
ChihChengLiang
force-pushed
the
fast-verification-of-multiple-bls-signatures
branch
2 times, most recently
from
a93dfd3 to
295b71b
Compare
py_ecc/bls/api.py
Outdated
|
|
||
| if len(pubkeys) != len_msgs: | ||
| o = FQ12.one() | ||
| for message_hash, group_pubkeys in _group_by_messages(message_hashes, pubkeys): |
There was a problem hiding this comment.
@hwwhww This groupby step is actually useless. The use cases in the spec suggest that bls_verify_multiple shouldn't group the message_hash
There was a problem hiding this comment.
Brought back groupby, since it's more like an optimization to save pairings for more general cases.
|
@ChihChengLiang glancing at the library I don't see any benchmarking tools which suggests you did the benchmarking manually. Would you mind contributing some version of that into the core library so we can have a starting point for future library optimizations? |
|
@pipermerriam manually in this PR 😂 |
ChihChengLiang
force-pushed
the
fast-verification-of-multiple-bls-signatures
branch
from
80b7c32 to
e733f5e
Compare
ChihChengLiang
force-pushed
the
fast-verification-of-multiple-bls-signatures
branch
from
e733f5e to
6f3382d
Compare
What was wrong?
https://ethresear.ch/t/fast-verification-of-multiple-bls-signatures/5407
How was it fixed?
Cute Animal Picture