feat(crypto/bls12381): signature aggregation

[melekes]

Aggregating signatures in the LastCommit before including it in the next block (proposal block). Note we don't aggregate prevotes, nor precommits when accumulating them - only after we've received 2/3+ of them and are ready to go to the next height.

Proto changes

BlockIDFlag

This PR adds 4 new enum options to BlockIDFlag:

• BlockIDFlagAggCommit
• BlockIDFlagAggCommitAbsent
• BlockIDFlagAggNil
• BlockIDFlagAggNilAbsent

The structure of the commit becomes like this:

0: BlockIDFlagAggCommit {aggregated signature}
1: BlockIDFlagAggCommitAbsent {empty signature}
2: BlockIDFlagAggCommitAbsent {empty signature}
3: BlockIDFlagAggCommitAbsent {empty signature}

4 validators, 1 aggregated signature

When there are precommits for nil:

0: BlockIDFlagAggCommit {aggregated signature}
1: BlockIDFlagAggCommitAbsent {empty signature}
2: BlockIDFlagAggCommitAbsent {empty signature}
3: BlockIDFlagAggNil {aggregated signature}

4 validators, 1 aggregated signature for block, 1 aggregated signature for nil (2 signatures in total).

CanonicalVote

Removes Timestamp

Vote

Sets Timestamp always to zero

Breaking changes

Removes BFT time in favor of PBTS, which becomes required from height 1 (from the start). If PBTS is not enabled from height 1, the code will panic!

Removes support for all curves other than BLS12-381. CometBFT will return an error if the validator update contains ed25519 or a different curve.

[melekes]

this is probably the biggest breaking change - removing Timestamp from vote sign bytes

[jmalicevic]

I am double checking, I think it is ok because this is only for the canonical vote. Because vote timestamps are used to compute the block time when using BFT time.

[sergio-mena]

I think we should also remove the timestamp field from both vote structures: the one internal to comet, and the protobuf-generated one. Leaving that field there only adds confusion IMO (let me know if you want to discuss)

[melekes]

I think we should also remove the timestamp field from both vote structures: the one internal to comet, and the protobuf-generated one. Leaving that field there only adds confusion IMO (let me know if you want to discuss)

Should it go hand in hand with making the PBTS the default choice + ripping off median time?

[melekes]

The decision here was to add preemptive panics so we fail early in case CometBFT is misconfigured.

[melekes]

This is ugly. CometBFT requires signatures to be present (not empty), but with BLS we only have 2 signatures present at max (1 for block, 1 for nil)! We can disable the check for empty sig in ValidateBasic though ... What do people think?

[sergio-mena]

Maybe we can have two new flags BlockIDFlagAggCommit (to store the aggregated signature), BlockIDFlagAggAbsent (to denote the signature is missing because it's aggregated in another slot). And maybe BlockIDFlagAggNil (to store the aggregated signature for nil). Although I still think aggregating nil votes is not worth the extra complexity (nil votes in a commit is extremely rare in practice)

[jmalicevic]

Should we delete this or modify it so it works with the new scheme?

[melekes]

I will remove

[jmalicevic]

I am double checking, I think it is ok because this is only for the canonical vote. Because vote timestamps are used to compute the block time when using BFT time.

[jmalicevic]

BLS at the moment has a SignatureLength which is checked on vote veriification. Does that need special handling now?

[sergio-mena]

Hmmm, I might be wrong, but I don't agree with this.
When vote extensions are enabled, there is one signature for the vote and another signature for the vote extension.
I agree the extension signatures cannot be aggregated, as the extensions can be different.
But the vote themselves can still be aggregated (assuming you remove the timestamp field, or set it always to 0-time).

[melekes]

Good point. But then again, who is going to verify vote extensions? There's no point in aggregating votes if you need to verify vote extensions using individual verification (signature.Verify).

[melekes]

On the 2nd thought, we don't verify vote extensions in VerifyCommit, so I think using them while aggregating vote signatures is okay. 6b0b06e (#11)