Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aggregate signature verification (on an Attestation) fails due to custody_bitfield #775

Closed
cemozerr opened this issue Mar 14, 2019 · 2 comments

Comments

@cemozerr
Copy link

cemozerr commented Mar 14, 2019

Honest Validator spec states that the attestation.custody_bitfield should be a byte array filled with zeros.

However, during per-block processing, when processing attestations we use attestation.custody_bitfield to get custody_bit_1_participants. These participants are later passed on to bls_aggregate_pubkeys to generate an aggregate public key. Having custody_bitfield set to all zeros, the custody_bit_1_participants becomes an empty list.

First, bls_aggregate_pubkeys function seems to be undefined when there are no public keys, as in this case, because custody_bitfield was set to all zeros.

Second, during process_attestations, bls_verify_multiple is used to check the aggregate signature on the attestation, however, bls_verify_multiple currently takes two message hashes. One where AttestationDataAndCustodyBit has its custody_bit set to false and another with its custody_bit set to true. bls_verify_multiple require two aggregate public keys when passed two message hashes. Specifically, the message hash with custody_bit set to true requires the aggregate public key of Validators who had their custody_bitfield set, which in our case is none. This makes bls_verify_multiple to produce an assert.

@JustinDrake
Copy link
Collaborator

This is a duplicate (non-)issue of #651.

bls_aggregate_pubkeys function seems to be undefined when there are no public keys

The zero sum is the point at infinity.

which in our case is none. This makes bls_verify_multiple to produce an assert.

As mentioned above it's not none, but rather the point at infinity. I've clarified this in #782.

@cemozerr
Copy link
Author

Thanks for the quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants