evidence: separate abci specific validation (#6473)

evidence/verify.go

@@ -42,16 +42,6 @@ func (evpool *Pool) verify(evidence types.Evidence) error {
 
 	// verify the time of the evidence
 	evTime := blockMeta.Header.Time
-	if evidence.Time() != evTime {
-		return types.NewErrInvalidEvidence(
-			evidence,
-			fmt.Errorf(
-				"evidence has a different time to the block it is associated with (%v != %v)",
-				evidence.Time(), evTime,
-			),
-		)
-	}
-
 	ageDuration := state.LastBlockTime.Sub(evTime)
 
 	// check that the evidence hasn't expired
@@ -80,6 +70,16 @@ func (evpool *Pool) verify(evidence types.Evidence) error {
 			return types.NewErrInvalidEvidence(evidence, err)
 		}
 
+		_, val := valSet.GetByAddress(ev.VoteA.ValidatorAddress)
+
+		if err := ev.ValidateABCI(val, valSet, evTime); err != nil {
+			ev.GenerateABCI(val, valSet, evTime)
+			if addErr := evpool.addPendingEvidence(ev); addErr != nil {
+				evpool.logger.Error("adding pending duplicate vote evidence failed", "err", addErr)
+			}
+			return err
+		}
+
 		return nil
 
 	case *types.LightClientAttackEvidence:
@@ -128,6 +128,18 @@ func (evpool *Pool) verify(evidence types.Evidence) error {
 		if err != nil {
 			return types.NewErrInvalidEvidence(evidence, err)
 		}
+
+		// validate the ABCI component of evidence. If this fails but the rest
+		// is valid then we regenerate the ABCI component, save the rectified
+		// evidence and return an error
+		if err := ev.ValidateABCI(commonVals, trustedHeader, evTime); err != nil {
+			ev.GenerateABCI(commonVals, trustedHeader, evTime)
+			if addErr := evpool.addPendingEvidence(ev); addErr != nil {
+				evpool.logger.Error("adding pending light client attack evidence failed", "err", addErr)
+			}
+			return err
+
+		}
 		return nil
 
 	default:
@@ -166,12 +178,6 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t
 		return fmt.Errorf("invalid commit from conflicting block: %w", err)
 	}
 
-	// Assert the correct amount of voting power of the validator set
-	if evTotal, valsTotal := e.TotalVotingPower, commonVals.TotalVotingPower(); evTotal != valsTotal {
-		return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)",
-			evTotal, valsTotal)
-	}
-
 	// check in the case of a forward lunatic attack that monotonically increasing time has been violated
 	if e.ConflictingBlock.Height > trustedHeader.Height && e.ConflictingBlock.Time.After(trustedHeader.Time) {
 		return fmt.Errorf("conflicting block doesn't violate monotonically increasing time (%v is after %v)",
@@ -184,7 +190,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t
 			trustedHeader.Hash())
 	}
 
-	return validateABCIEvidence(e, commonVals, trustedHeader)
+	return nil
 }
 
 // VerifyDuplicateVote verifies DuplicateVoteEvidence against the state of full node. This involves the
@@ -232,16 +238,6 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet
 			addr, pubKey, pubKey.Address())
 	}
 
-	// validator voting power and total voting power must match
-	if val.VotingPower != e.ValidatorPower {
-		return fmt.Errorf("validator power from evidence and our validator set does not match (%d != %d)",
-			e.ValidatorPower, val.VotingPower)
-	}
-	if valSet.TotalVotingPower() != e.TotalVotingPower {
-		return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)",
-			e.TotalVotingPower, valSet.TotalVotingPower())
-	}
-
 	va := e.VoteA.ToProto()
 	vb := e.VoteB.ToProto()
 	// Signatures must be valid
@@ -255,55 +251,6 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet
 	return nil
 }
 
-// validateABCIEvidence validates the ABCI component of the light client attack
-// evidence i.e voting power and byzantine validators
-func validateABCIEvidence(
-	ev *types.LightClientAttackEvidence,
-	commonVals *types.ValidatorSet,
-	trustedHeader *types.SignedHeader,
-) error {
-	if evTotal, valsTotal := ev.TotalVotingPower, commonVals.TotalVotingPower(); evTotal != valsTotal {
-		return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)",
-			evTotal, valsTotal)
-	}
-
-	// Find out what type of attack this was and thus extract the malicious
-	// validators. Note, in the case of an Amnesia attack we don't have any
-	// malicious validators.
-	validators := ev.GetByzantineValidators(commonVals, trustedHeader)
-
-	// Ensure this matches the validators that are listed in the evidence. They
-	// should be ordered based on power.
-	if validators == nil && ev.ByzantineValidators != nil {
-		return fmt.Errorf(
-			"expected nil validators from an amnesia light client attack but got %d",
-			len(ev.ByzantineValidators),
-		)
-	}
-
-	if exp, got := len(validators), len(ev.ByzantineValidators); exp != got {
-		return fmt.Errorf("expected %d byzantine validators from evidence but got %d", exp, got)
-	}
-
-	for idx, val := range validators {
-		if !bytes.Equal(ev.ByzantineValidators[idx].Address, val.Address) {
-			return fmt.Errorf(
-				"evidence contained an unexpected byzantine validator address; expected: %v, got: %v",
-				val.Address, ev.ByzantineValidators[idx].Address,
-			)
-		}
-
-		if ev.ByzantineValidators[idx].VotingPower != val.VotingPower {
-			return fmt.Errorf(
-				"evidence contained unexpected byzantine validator power; expected %d, got %d",
-				val.VotingPower, ev.ByzantineValidators[idx].VotingPower,
-			)
-		}
-	}
-
-	return nil
-}
-
 func getSignedHeader(blockStore BlockStore, height int64) (*types.SignedHeader, error) {
 	blockMeta := blockStore.LoadBlockMeta(height)
 	if blockMeta == nil {

evidence/verify_test.go

@@ -54,7 +54,8 @@ func TestVerifyLightClientAttack_Lunatic(t *testing.T) {
 	ev.TotalVotingPower = 1 * defaultVotingPower
 	err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet,
 		defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour)
-	assert.Error(t, err)
+	assert.NoError(t, err)
+	assert.Error(t, ev.ValidateABCI(common.ValidatorSet, trusted.SignedHeader, defaultEvidenceTime))
 
 	// evidence without enough malicious votes should fail
 	ev, trusted, common = makeLunaticEvidence(

light/detector.go

@@ -183,7 +183,7 @@ func (c *Client) compareNewHeaderWithWitness(ctx context.Context, errc chan erro
 		return
 	}
 
-	if !bytes.Equal(h.Hash(), lightBlock.Hash()) {
+	if !bytes.Equal(h.Header.Hash(), lightBlock.Header.Hash()) {
 		errc <- errConflictingHeaders{Block: lightBlock, WitnessIndex: witnessIndex}
 	}
 

types/evidence.go

@@ -140,6 +140,44 @@ func (dve *DuplicateVoteEvidence) ValidateBasic() error {
 	return nil
 }
 
+// ValidateABCI validates the ABCI component of the evidence by checking the
+// timestamp, validator power and total voting power.
+func (dve *DuplicateVoteEvidence) ValidateABCI(
+	val *Validator,
+	valSet *ValidatorSet,
+	evidenceTime time.Time,
+) error {
+
+	if dve.Timestamp != evidenceTime {
+		return fmt.Errorf(
+			"evidence has a different time to the block it is associated with (%v != %v)",
+			dve.Timestamp, evidenceTime)
+	}
+
+	if val.VotingPower != dve.ValidatorPower {
+		return fmt.Errorf("validator power from evidence and our validator set does not match (%d != %d)",
+			dve.ValidatorPower, val.VotingPower)
+	}
+	if valSet.TotalVotingPower() != dve.TotalVotingPower {
+		return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)",
+			dve.TotalVotingPower, valSet.TotalVotingPower())
+	}
+
+	return nil
+}
+
+// GenerateABCI populates the ABCI component of the evidence. This includes the
+// validator power, timestamp and total voting power.
+func (dve *DuplicateVoteEvidence) GenerateABCI(
+	val *Validator,
+	valSet *ValidatorSet,
+	evidenceTime time.Time,
+) {
+	dve.ValidatorPower = val.VotingPower
+	dve.TotalVotingPower = valSet.TotalVotingPower()
+	dve.Timestamp = evidenceTime
+}
+
 // ToProto encodes DuplicateVoteEvidence to protobuf
 func (dve *DuplicateVoteEvidence) ToProto() *tmproto.DuplicateVoteEvidence {
 	voteB := dve.VoteB.ToProto()
@@ -258,12 +296,12 @@ func (l *LightClientAttackEvidence) GetByzantineValidators(commonVals *Validator
 		// only need a single loop to find the validators that voted twice.
 		for i := 0; i < len(l.ConflictingBlock.Commit.Signatures); i++ {
 			sigA := l.ConflictingBlock.Commit.Signatures[i]
-			if sigA.Absent() {
+			if !sigA.ForBlock() {
 				continue
 			}
 
 			sigB := trusted.Commit.Signatures[i]
-			if sigB.Absent() {
+			if !sigB.ForBlock() {
 				continue
 			}
 
@@ -367,6 +405,76 @@ func (l *LightClientAttackEvidence) ValidateBasic() error {
 	return nil
 }
 
+// ValidateABCI validates the ABCI component of the evidence by checking the
+// timestamp, byzantine validators and total voting power all match. ABCI
+// components are validated separately because they can be re generated if
+// invalid.
+func (l *LightClientAttackEvidence) ValidateABCI(
+	commonVals *ValidatorSet,
+	trustedHeader *SignedHeader,
+	evidenceTime time.Time,
+) error {
+
+	if evTotal, valsTotal := l.TotalVotingPower, commonVals.TotalVotingPower(); evTotal != valsTotal {
+		return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)",
+			evTotal, valsTotal)
+	}
+
+	if l.Timestamp != evidenceTime {
+		return fmt.Errorf(
+			"evidence has a different time to the block it is associated with (%v != %v)",
+			l.Timestamp, evidenceTime)
+	}
+
+	// Find out what type of attack this was and thus extract the malicious
+	// validators. Note, in the case of an Amnesia attack we don't have any
+	// malicious validators.
+	validators := l.GetByzantineValidators(commonVals, trustedHeader)
+
+	// Ensure this matches the validators that are listed in the evidence. They
+	// should be ordered based on power.
+	if validators == nil && l.ByzantineValidators != nil {
+		return fmt.Errorf(
+			"expected nil validators from an amnesia light client attack but got %d",
+			len(l.ByzantineValidators),
+		)
+	}
+
+	if exp, got := len(validators), len(l.ByzantineValidators); exp != got {
+		return fmt.Errorf("expected %d byzantine validators from evidence but got %d", exp, got)
+	}
+
+	for idx, val := range validators {
+		if !bytes.Equal(l.ByzantineValidators[idx].Address, val.Address) {
+			return fmt.Errorf(
+				"evidence contained an unexpected byzantine validator address; expected: %v, got: %v",
+				val.Address, l.ByzantineValidators[idx].Address,
+			)
+		}
+
+		if l.ByzantineValidators[idx].VotingPower != val.VotingPower {
+			return fmt.Errorf(
+				"evidence contained unexpected byzantine validator power; expected %d, got %d",
+				val.VotingPower, l.ByzantineValidators[idx].VotingPower,
+			)
+		}
+	}
+
+	return nil
+}
+
+// GenerateABCI populates the ABCI component of the evidence: the timestamp,
+// total voting power and byantine validators
+func (l *LightClientAttackEvidence) GenerateABCI(
+	commonVals *ValidatorSet,
+	trustedHeader *SignedHeader,
+	evidenceTime time.Time,
+) {
+	l.Timestamp = evidenceTime
+	l.TotalVotingPower = commonVals.TotalVotingPower()
+	l.ByzantineValidators = l.GetByzantineValidators(commonVals, trustedHeader)
+}
+
 // ToProto encodes LightClientAttackEvidence to protobuf
 func (l *LightClientAttackEvidence) ToProto() (*tmproto.LightClientAttackEvidence, error) {
 	conflictingBlock, err := l.ConflictingBlock.ToProto()