Conversation
|
Do you plan to change the peer bit arrays to track the block hash as well? Currently we only track the votes a peer has by index using a single bitarray, but we may instead want to use one bitarray per blockhash we've seen a vote for. Of course we should limit this to two unless there is good reason to admit more |
types/vote_set.go
Outdated
| return voteSet.votesBitArray.Copy() | ||
| } | ||
|
|
||
| // NOTE: if validator has conflicting votes, picks random. |
|
This looks good other than the comments I don't understand. Let's add the bit for removing peers and merge this to develop? I don't want it to languish if "the full bft fix" might take a while longer. |
|
Comments have been fixed. |
| BlockID: maj23, | ||
| }}) | ||
| time.Sleep(peerQueryMaj23SleepDuration) | ||
| rs = conR.conS.GetRoundState() |
There was a problem hiding this comment.
are these rs = and prs = necessary?
There was a problem hiding this comment.
nope, removed on bft_fix_rebase branch
|
rebased in #314 |
specify DeliverTx/CheckTx Tags format
This is not complete, but I'll document the thinking process while working on it.
The problem is that, the way things are, due to the way peers communicate to each other which votes it has, if a validator double-signs for a given height/round/step, then it can prevent its own other double-votes from propagating correct, e.g. "occluding".
The case in which this was discovered to be an issue was during a BFT fuzz test, where it halted the blockchain with less than 1/3 of Byzantine validators, due to the Commit being occluded.
What are all the issues related to double-signing occlusion?
What about when there are more than 1 2/3 majority for a prevote or precommit step, due to 1/3+ being Byzantine? Currently we don't have logic to automatically broadcast this issue. This would be a good time to implement that logic, so we fix BFT as well as the first step in identifying an attack. This isn't the same as Tendermint V2, where a Commit is fully justified, but, it would be ideal to work toward that, while we fix this issue.
So, we want to:
I don't have the whole solution yet, but the first step might be to refactor the VoteSet structure to allow tracking of double-signing. But we need to make sure this is done in such a way that double-signers can't sign a million votes and make nodes run out of memory, or cause some other issue.
The general solution I have in mind is to do the following:
As I implement these features in VoteSet, I'll make sure the functionality is reusable for Tendermint V2, so that justifications can be requested for (H,R,S) as necessary.