BLS Signature Aggregation and Verification

[torao]

Closes: #95

Description

This PR implements BLS signature aggregation and adds the aggregated signature to the Block. Fixed to introduce BLS signature aggregation into the block in conjunction with PR #87 and #101.

---

For contributor use:

• Wrote tests
• Updated CHANGELOG_PENDING.md
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation (docs/) and code comments
• Re-reviewed Files changed in the Github PR explorer

[SatoshiSakamori]

@torao It looks compiling BLS on linux is failed. so, I pushed c9d170c (but test has still failed). If my change is extra things, please let me know I will revert that change.

[egonspace]

I noticed there is a major design flaw in this implementation while looking at the code. It's hard to prove whose signature is in the aggregate signature of Commit or VoteSet. Of course, although the addresses of voter signing the vote are recorded on the block currently, this is maliciously manipulative. That is, when someone includes the address of the voter not included in the aggregated signature, or when he maliciously remove the voter contained in the aggregated signature, we must be able to verify it, which is difficult for the current structure.

You can refer about this from following: tendermint/tendermint#1319

And VoteSet and Commit can be convertible with each other so VoteSet should be able to contain aggregated signature. This could result in two types, VoteSet with aggregated signature form and VoteSet with individual signatures, which is not deterministic. We are safe to judge that only one type of VoteSet is valid.

[torao]

@egonspace
As-is:

1. It generates an aggregated signature after verifying all Votes in generating a Commit from the collected VoteSet.
2. The Signature field in all CommitSigs for aggregated signatures is set to nil. This tells us whose public key should be used to verify the aggregated signature.
3. If a Commit address is tampered with, the wrong BLS public key will be selected to verify the aggregated signature. As a result, the signature verification fails.

The problem you pointing out here is that the aggregated signature doesn't tell who has tampered with the address in order to slash the failure user? (But if that were possible, wouldn't it be possible to identify the failure user who replaced addresses without whether our BLS feature is implemented or not?)

[egonspace]

This was what I was worried about.
In the past, in order to fake Commit, a signature could only be added by knowing the private key of a particular validator. However, in the BLS signature aggregation, even if he doesn't know the private of a particular validator, he can create a Commit by adding only the public address to the Commit list. Of course, the Commit added a public address would fail to verify aggregate signature, but I thought it would be a problem that we couldn't figure out which public address was fake.

But after looking at a few possible malicious behaviors, it seems that there is no problem as long as we can judge that the aggregated signature is wrong without finding which public address is wrong. (But I'm still not sure that malicious behavior is really impossible)

Possible malicious behavior can be as follows: In other words, if the proposer who created the Commit did the right thing, but the other who propagates it adds a random public address to the Commit, we have to find a false address in the context and judge the propagator as a byzantine, but we could only punish the proposer. For now, however, this method seems impossible because the Commit hash is recorded in the block. We may need to confirm that there is no more such malicious behavior available.

The above issue seems to say that it is a problem when sending and receiving aggregated VoteSet through gossip, not individual vote.

As for the second comment, I would like to confirm that VoteSet is assuming that both the aggregated form and not aggregated form are valid. The VoteSet a next proposer are currently collecting will be in non-aggregated form, and the one made through CommitToVoteSet will be in aggregated form, are you assuming both states? I think it's a little error prone.

[egonspace]

I hope the state diagram of VoteSet is defined conceptually.

As-is:
VoteSet ------> Commit : by MakeCommit
VoteSet <----- Commit : by CommitToVoteSet
VoteSet ----> VoteSet : by AddVote

To-be:
VoteSet(not aggregated) ---> VoteSet(aggregated) : by aggregateSignature?
VoteSet(aggregated) ---> Commit : by MakeCommit
VoteSet(aggregated) <--- Commit : by CommitToVoteSet
VoteSet(not aggregated) ---> VoteSet(not aggregated) : by AddVote
VoteSet(aggregated) ---> VoteSet(not aggregated or aggregated partially) : NOT POSSIBLE?

Is it right?

[egonspace]

It seems like your work is not done yet, but I'm leaving you some suggestions.

• I think Commit.Hash() should include AggregatedSignature.
• Now we can judge that Vote or CommitSig's Signature is aggregated if it is nil, and that if it is empty splice, it is an initial condition that has not been signed, which is somewhat error prune. How about to add a bool type flag to CommitSig to indicate explicitly that it has been aggregated?
• The third requires a slightly more complicated explanation. At present, Commit can be stored in two forms: with the calcBlockCommitKey key, or with the calcSeenCommitKey key. The former is the complete Commit contained in the block, so it is probably an aggregate form, but the latter is not from the block, but from the currently collected VoteSet. The former is loaded through LoadBlockCommit() and the latter is loaded through LoadSeenCommit(). I have found that Commit obtained through LoadBlockCommit is never made into VoteSet again, and it seems that only Commit obtained through LoadSeenCommit is converted to VoteSet. So, my proposal is to remove the AggregatedSignature from VoteSet and allow the Commit to take two forms (aggregated, not aggregated). In other words, if the Commit is aggregated in the final stage of creating a block, the block will be assigned an aggregate type of Commit, and the Commit stored by the calcSeenCommitKey key is not aggregated, so it is natural to convert it to VoteSet later.

If the explanation is a bit difficult to understand, I may suggest by modifying the code myself.

[egonspace]

If the explanation is a bit difficult to understand, I may suggest by modifying the code myself.

I have written a code for this proposal to help you understand: #141

[egonspace]

There is one thing to worry about.
In gossipVotesRoutine() of state/reactor.go, a node sends votes from stored commit to a peer.
But stored commit is aggregated form, so its individual vote has no signature.
We must handle this routine.

if prs.Height != 0 && rs.Height >= prs.Height+2 {
	// Load the block commit for prs.Height,
	// which contains precommit signatures for prs.Height.
	commit := conR.conS.blockStore.LoadBlockCommit(prs.Height)
	if ps.PickSendVote(commit) {
		logger.Debug("Picked Catchup commit to send", "height", prs.Height)
		continue OUTER_LOOP
	}
}

[egonspace]

There is one thing to worry about.
In gossipVotesRoutine() of state/reactor.go, a node sends votes from stored commit to a peer.
But stored commit is aggregated form, so its individual vote has no signature.
We must handle this routine.

if prs.Height != 0 && rs.Height >= prs.Height+2 {
	// Load the block commit for prs.Height,
	// which contains precommit signatures for prs.Height.
	commit := conR.conS.blockStore.LoadBlockCommit(prs.Height)
	if ps.PickSendVote(commit) {
		logger.Debug("Picked Catchup commit to send", "height", prs.Height)
		continue OUTER_LOOP
	}
}

After my research, I don't think there will be a big problem using SeenCommit instead of BlockCommit in the code above. I committed the code modification in #141. What do you think?

[egonspace]

LGTM

[tnasu]

LGTM

[kukugi]

LGTM 👍
Leave one comment for what looks like a typo.

[kukugi]

no no individual signature is it typo?

[torao]

I fixed it. thanks!

[Kynea0b]

LGTM