improve: handle same votes when assembling attestation and in monitor

bnb-chain · Jul 31, 2023 · a4f1728 · a4f1728
1 parent 278608a
commit a4f1728
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 27 deletions.
diff --git a/consensus/parlia/parlia.go b/consensus/parlia/parlia.go
@@ -868,6 +868,9 @@ func (p *Parlia) assembleVoteAttestation(chain consensus.ChainHeaderReader, head
 	voteAddrSet := make(map[types.BLSPublicKey]struct{}, len(votes))
 	signatures := make([][]byte, 0, len(votes))
 	for _, vote := range votes {
+		if _, ok := voteAddrSet[vote.VoteAddress]; ok {
+			return fmt.Errorf("assemble VoteAttestation failed, double sign by %s", common.Bytes2Hex(vote.VoteAddress[:]))
+		}
 		voteAddrSet[vote.VoteAddress] = struct{}{}
 		signatures = append(signatures, vote.Signature[:])
 	}
@@ -882,6 +885,12 @@ func (p *Parlia) assembleVoteAttestation(chain consensus.ChainHeaderReader, head
 			attestation.VoteAddressSet |= 1 << (valInfo.Index - 1) //Index is offset by 1
 		}
 	}
+	// The valid voted validators should be no less than 2/3 validators,
+	// In some corner case, one vote may be included twice, so here need check again.
+	validatorsBitSet := bitset.From([]uint64{uint64(attestation.VoteAddressSet)})
+	if validatorsBitSet.Count() < uint(cmath.CeilDiv(len(snap.Validators)*2, 3)) {
+		return fmt.Errorf("invalid attestation, not enough validators voted")
+	}
 
 	// Append attestation to header extra field.
 	buf := new(bytes.Buffer)

diff --git a/core/monitor/malicious_vote_monitor.go b/core/monitor/malicious_vote_monitor.go
@@ -58,6 +58,7 @@ func (m *MaliciousVoteMonitor) ConflictDetect(newVote *types.VoteEnvelope, pendi
 	if !(blockNumber+maliciousVoteSlashScope > pendingBlockNumber) {
 		blockNumber = pendingBlockNumber - maliciousVoteSlashScope + 1
 	}
+	newVoteHash := newVote.Data.Hash()
 	for ; blockNumber <= pendingBlockNumber+upperLimitOfVoteBlockNumber; blockNumber++ {
 		if voteDataBuffer.Contains(blockNumber) {
 			voteEnvelope, ok := voteDataBuffer.Get(blockNumber)
@@ -66,7 +67,7 @@ func (m *MaliciousVoteMonitor) ConflictDetect(newVote *types.VoteEnvelope, pendi
 				continue
 			}
 			maliciousVote := false
-			if blockNumber == targetNumber {
+			if blockNumber == targetNumber && voteEnvelope.(*types.VoteEnvelope).Data.Hash() != newVoteHash {
 				violateRule1Counter.Inc(1)
 				maliciousVote = true
 			} else if (blockNumber < targetNumber && voteEnvelope.(*types.VoteEnvelope).Data.SourceNumber > sourceNumber) ||

diff --git a/core/monitor/malicious_vote_monitor_test.go b/core/monitor/malicious_vote_monitor_test.go
@@ -22,9 +22,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - maliciousVoteSlashScope - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes(("01"))),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -34,9 +34,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - maliciousVoteSlashScope - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -54,9 +54,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - maliciousVoteSlashScope - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("01")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -65,9 +65,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - maliciousVoteSlashScope - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -85,9 +85,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("01")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -96,9 +96,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: uint64(0),
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, true, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -116,9 +116,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 4,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("01")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -127,9 +127,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 2,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 3,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, true, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -147,9 +147,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 2,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 3,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("01")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -158,9 +158,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 4,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, true, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -178,9 +178,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 4,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 3,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(1)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("01")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote1, pendingBlockNumber))
@@ -189,9 +189,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 3,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 2,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote2, pendingBlockNumber))
@@ -200,9 +200,9 @@ func TestMaliciousVoteMonitor(t *testing.T) {
 			Signature:   types.BLSSignature{},
 			Data: &types.VoteData{
 				SourceNumber: pendingBlockNumber - 2,
-				SourceHash:   common.BytesToHash(common.Hex2Bytes(string(rune(0)))),
+				SourceHash:   common.BytesToHash(common.Hex2Bytes("00")),
 				TargetNumber: pendingBlockNumber - 1,
-				TargetHash:   common.BytesToHash(common.Hex2Bytes(string(rune(2)))),
+				TargetHash:   common.BytesToHash(common.Hex2Bytes("02")),
 			},
 		}
 		assert.Equal(t, false, maliciousVoteMonitor.ConflictDetect(vote3, pendingBlockNumber))