-
Notifications
You must be signed in to change notification settings - Fork 146
Conversation
10d9e5d
to
b65fba3
Compare
Some odd bugs summarized at #767 |
pubkey_chia = bls_chia.PublicKey.from_bytes(pubkey) | ||
signature_chia = bls_chia.Signature.from_bytes(signature) | ||
pubkey_chia = _pubkey_from_bytes(pubkey) | ||
signature_chia = _signature_from_bytes(signature) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we want the verify
to return False or raise ValidationError for the invalid inputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I prefer ValidationError
, since the caller cannot distinguish whether it is invalid inputs or just the incorrect signatures/wrong private keys if verify
returns False
.
|
||
def test_empty_aggregation(): | ||
assert aggregate_pubkeys([]) == EMPTY_PUBKEY | ||
assert aggregate_signatures([]) == EMPTY_SIGNATURE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note py_ecc has this behavior.
pubkey_chia = bls_chia.PublicKey.from_bytes(pubkey) | ||
signature_chia = bls_chia.Signature.from_bytes(signature) | ||
pubkey_chia = _pubkey_from_bytes(pubkey) | ||
signature_chia = _signature_from_bytes(signature) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I prefer ValidationError
, since the caller cannot distinguish whether it is invalid inputs or just the incorrect signatures/wrong private keys if verify
returns False
.
Co-Authored-By: Kevin Mai-Husan Chia <[email protected]>
Co-Authored-By: Kevin Mai-Husan Chia <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me:)
Plan to rebase and merge this after #714. And will request another round of review after adding these features to this PR.
|
close in favor of #787 |
What was wrong?
Before properly addressing #746, this is a minimal way to get Chia BLS after #714 is merged.
How was it fixed?
Cute Animal Picture