Fix light client bisection benchmark (#226)

* Fix benchmarks * Modify other benchmark
fetchai · Nov 5, 2020 · 2df797d · 2df797d
1 parent 14695cd
commit 2df797d
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 74 deletions.
diff --git a/lite2/client_benchmark_test.go b/lite2/client_benchmark_test.go
@@ -26,25 +26,27 @@ func BenchmarkSequence(b *testing.B) {
 		benchmarkFullNode = mockp.New(GenMockNode(chainID, 1000, 100, 1, bTime))
 		genesisHeader, _  = benchmarkFullNode.SignedHeader(1)
 	)
-	c, err := lite.NewClient(
-		chainID,
-		lite.TrustOptions{
-			Period: 24 * time.Hour,
-			Height: 1,
-			Hash:   genesisHeader.Hash(),
-		},
-		benchmarkFullNode,
-		[]provider.Provider{benchmarkFullNode},
-		dbs.New(dbm.NewMemDB(), chainID),
-		lite.Logger(log.TestingLogger()),
-		lite.SequentialVerification(),
-	)
-	if err != nil {
-		b.Fatal(err)
-	}
 	b.ResetTimer()
 
 	for n := 0; n < b.N; n++ {
+		b.StopTimer()
+		c, err := lite.NewClient(
+			chainID,
+			lite.TrustOptions{
+				Period: 24 * time.Hour,
+				Height: 1,
+				Hash:   genesisHeader.Hash(),
+			},
+			benchmarkFullNode,
+			[]provider.Provider{benchmarkFullNode},
+			dbs.New(dbm.NewMemDB(), chainID),
+			lite.Logger(log.TestingLogger()),
+			lite.SequentialVerification(),
+		)
+		if err != nil {
+			b.Fatal(err)
+		}
+		b.StartTimer()
 		_, err = c.VerifyHeaderAtHeight(1000, bTime.Add(1000*time.Minute))
 		if err != nil {
 			b.Fatal(err)
@@ -57,24 +59,26 @@ func BenchmarkBisection(b *testing.B) {
 		benchmarkFullNode = mockp.New(GenMockNode(chainID, 1000, 100, 1, bTime))
 		genesisHeader, _  = benchmarkFullNode.SignedHeader(1)
 	)
-	c, err := lite.NewClient(
-		chainID,
-		lite.TrustOptions{
-			Period: 24 * time.Hour,
-			Height: 1,
-			Hash:   genesisHeader.Hash(),
-		},
-		benchmarkFullNode,
-		[]provider.Provider{benchmarkFullNode},
-		dbs.New(dbm.NewMemDB(), chainID),
-		lite.Logger(log.TestingLogger()),
-	)
-	if err != nil {
-		b.Fatal(err)
-	}
 	b.ResetTimer()
 
 	for n := 0; n < b.N; n++ {
+		b.StopTimer()
+		c, err := lite.NewClient(
+			chainID,
+			lite.TrustOptions{
+				Period: 24 * time.Hour,
+				Height: 1,
+				Hash:   genesisHeader.Hash(),
+			},
+			benchmarkFullNode,
+			[]provider.Provider{benchmarkFullNode},
+			dbs.New(dbm.NewMemDB(), chainID),
+			lite.Logger(log.TestingLogger()),
+		)
+		if err != nil {
+			b.Fatal(err)
+		}
+		b.StartTimer()
 		_, err = c.VerifyHeaderAtHeight(1000, bTime.Add(1000*time.Minute))
 		if err != nil {
 			b.Fatal(err)
@@ -86,25 +90,27 @@ func BenchmarkBackwards(b *testing.B) {
 	var (
 		benchmarkFullNode = mockp.New(GenMockNode(chainID, 1000, 100, 1, bTime))
 	)
-	trustedHeader, _ := benchmarkFullNode.SignedHeader(0)
-	c, err := lite.NewClient(
-		chainID,
-		lite.TrustOptions{
-			Period: 24 * time.Hour,
-			Height: trustedHeader.Height,
-			Hash:   trustedHeader.Hash(),
-		},
-		benchmarkFullNode,
-		[]provider.Provider{benchmarkFullNode},
-		dbs.New(dbm.NewMemDB(), chainID),
-		lite.Logger(log.TestingLogger()),
-	)
-	if err != nil {
-		b.Fatal(err)
-	}
 	b.ResetTimer()
 
+	trustedHeader, _ := benchmarkFullNode.SignedHeader(0)
 	for n := 0; n < b.N; n++ {
+		b.StopTimer()
+		c, err := lite.NewClient(
+			chainID,
+			lite.TrustOptions{
+				Period: 24 * time.Hour,
+				Height: trustedHeader.Height,
+				Hash:   trustedHeader.Hash(),
+			},
+			benchmarkFullNode,
+			[]provider.Provider{benchmarkFullNode},
+			dbs.New(dbm.NewMemDB(), chainID),
+			lite.Logger(log.TestingLogger()),
+		)
+		if err != nil {
+			b.Fatal(err)
+		}
+		b.StartTimer()
 		_, err = c.VerifyHeaderAtHeight(1, bTime)
 		if err != nil {
 			b.Fatal(err)

diff --git a/types/validator_set.go b/types/validator_set.go
@@ -652,35 +652,29 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID,
 		if len(commitSigs) != 1 {
 			return NewErrInvalidCommitSigLength(idx, len(commitSigs))
 		}
-		if commitSigs[0].Absent() {
+		if !commitSigs[0].ForBlock() {
 			continue // OK, some signatures can be absent.
 		}
 
 		// The vals and commit have a 1-to-1 correspondance.
 		// This means we don't need the validator address or to do any lookup.
 		val := vals.Validators[idx]
 
-		if commitSigs[0].ForBlock() {
-			// Check that the information signed is consistent with other votes as all signatures for the same block
-			// should have signed the same message
-			voteSignBytes := commit.VoteSignBytes(VotePrefix(chainID, valHash), idx)
-			if voteBytes == nil {
-				voteBytes = voteSignBytes
-			} else if !bytes.Equal(voteSignBytes, voteBytes) {
-				panic(fmt.Sprintf("conflicting signed vote bytes for block %v valIndex %v", height, idx))
-			}
+		// Check that the information signed is consistent with other votes as all signatures for the same block
+		// should have signed the same message
+		voteSignBytes := commit.VoteSignBytes(VotePrefix(chainID, valHash), idx)
+		if voteBytes == nil {
+			voteBytes = voteSignBytes
+		} else if !bytes.Equal(voteSignBytes, voteBytes) {
+			return fmt.Errorf("conflicting signed vote bytes for block %v valIndex %v", height, idx)
+		}
 
-			blsKey, ok := val.PubKey.(bls12_381.PubKeyBls)
-			if !ok {
-				panic(fmt.Sprintf("incorrect key type for combined signatures"))
-			}
-			pubKeys.Add(blsKey.RawString())
-			talliedVotingPower += val.VotingPower
+		blsKey, ok := val.PubKey.(bls12_381.PubKeyBls)
+		if !ok {
+			return fmt.Errorf("incorrect key type for combined signatures %T", val.PubKey)
 		}
-		// else {
-		// It's OK that the BlockID doesn't match.  We include stray
-		// signatures (~votes for nil) to measure validator availability.
-		// }
+		pubKeys.Add(blsKey.RawString())
+		talliedVotingPower += val.VotingPower
 	}
 
 	if got, needed := talliedVotingPower, votingPowerNeeded; got <= needed {
@@ -745,20 +739,19 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin
 		if len(commitSigs) != 1 {
 			return NewErrInvalidCommitSigLength(idx, len(commitSigs))
 		}
-		commitSig := commitSigs[0]
-		if commitSig.Absent() {
+		if commitSigs[0].Absent() {
 			continue // OK, some signatures can be absent.
 		}
 
 		// See if this validator is in oldVals.
-		oldIdx, val := oldVals.GetByAddress(commitSig.ValidatorAddress)
+		oldIdx, val := oldVals.GetByAddress(commitSigs[0].ValidatorAddress)
 		if val == nil || seen[oldIdx] {
 			continue // missing or double vote...
 		}
 		seen[oldIdx] = true
 
 		// Good!
-		if blockID.Equals(commitSig.BlockID(commit.BlockID)) {
+		if commitSigs[0].ForBlock() {
 			oldVotingPower += val.VotingPower
 		}
 		// else {
@@ -817,15 +810,14 @@ func (vals *ValidatorSet) VerifyValidatorSetTrust(blockID BlockID,
 		if len(commitSigs) != 1 {
 			return NewErrInvalidCommitSigLength(idx, len(commitSigs))
 		}
-		commitSig := commitSigs[0]
 		// No need to verify absent or nil votes.
-		if !commitSig.ForBlock() {
+		if !commitSigs[0].ForBlock() {
 			continue
 		}
 
 		// We don't know the validators that committed this block, so we have to
 		// check for each vote if its validator is already known.
-		valIdx, val := vals.GetByAddress(commitSig.ValidatorAddress)
+		valIdx, val := vals.GetByAddress(commitSigs[0].ValidatorAddress)
 
 		if val != nil {
 			// check for double vote of validator on the same commit