fix(light): stop witness comparison after divergence checks (#5820)

CHANGELOG.md

@@ -6,6 +6,12 @@
 
 ### BUG FIXES
 
+- `[evidence]` Add validation for Light Client Attack evidence ByzantineValidators
+  ([\#5638](https://github.com/cometbft/cometbft/pull/5638))
+
+- `[light]`: stop witness comparison after divergence checks
+  ([\#5820](https://github.com/cometbft/cometbft/pull/5820))
+
 ### IMPROVEMENTS
 - `[statesync]` Add configurable `max-snapshot-chunks` parameter to validate max amount of chunks in a `SnapshotResponse`.
   ([\#5549](https://github.com/cometbft/cometbft/pull/5549))

light/detector.go

@@ -207,12 +207,14 @@ func (c *Client) compareNewLightBlockWithWitness(ctx context.Context, errc chan
 
 	if !bytes.Equal(h.Hash(), lightBlock.Hash()) {
 		errc <- ErrConflictingHeaders{Block: lightBlock, WitnessIndex: witnessIndex, peer: peer}
+		return
 	}
 
 	// ProposerPriorityHash is not part of the header hash, so we need to check it separately.
 	wanted, got := l.ValidatorSet.ProposerPriorityHash(), lightBlock.ValidatorSet.ProposerPriorityHash()
 	if !bytes.Equal(wanted, got) {
 		errc <- ErrProposerPrioritiesDiverge{WitnessHash: got, WitnessIndex: witnessIndex, PrimaryHash: wanted}
+		return
 	}
 
 	c.logger.Debug("Matching header received by witness", "height", h.Height, "witness", witnessIndex)
@@ -308,7 +310,6 @@ func (c *Client) examineConflictingHeaderAgainstTrace(
 	targetBlock *types.LightBlock,
 	source provider.Provider, now time.Time,
 ) ([]*types.LightBlock, *types.LightBlock, error) {
-
 	var (
 		previouslyVerifiedBlock, sourceBlock *types.LightBlock
 		sourceTrace                          []*types.LightBlock
@@ -385,7 +386,6 @@ func (c *Client) examineConflictingHeaderAgainstTrace(
 	// prerequisites to this function were not met. Namely that either trace[len(trace)-1].Height < targetBlock.Height
 	// or that trace[i].Hash() != targetBlock.Hash()
 	return nil, nil, errNoDivergence
-
 }
 
 // getTargetBlockOrLatest gets the latest height, if it is greater than the target height then it queries

light/detector_test.go

@@ -1,6 +1,7 @@
 package light_test
 
 import (
+	"errors"
 	"testing"
 	"time"
 
@@ -428,3 +429,122 @@ func TestClientDivergentTraces4(t *testing.T) {
 	assert.Error(t, err)
 	assert.Equal(t, 1, len(c.Witnesses()))
 }
+
+func TestClientDivergentHeadersNoTraces(t *testing.T) {
+	primaryHeaders, primaryVals, primaryKeys := genMockNodeWithKeys(chainID, 10, 5, 2, bTime)
+	primary := mockp.New(chainID, primaryHeaders, primaryVals)
+
+	firstBlock, err := primary.LightBlock(ctx, 1)
+	require.NoError(t, err)
+
+	keysAtH10 := primaryKeys[10]
+	valsAtH10 := primaryVals[10]
+	nextValsAtH11 := primaryKeys[11].ToValidators(2, 0)
+
+	// generate a header with an app hash that diverges from the primary
+	makeDivergentHeader := func(appHashSeed string) *types.SignedHeader {
+		return keysAtH10.GenSignedHeader(
+			chainID, 10,
+			primaryHeaders[10].Time, nil,
+			valsAtH10, nextValsAtH11,
+			hash(appHashSeed), hash("cons_hash"), hash("results_hash"),
+			0, len(keysAtH10),
+		)
+	}
+	divergentHeader1 := makeDivergentHeader("divergent_app_1")
+	divergentHeader2 := makeDivergentHeader("divergent_app_2")
+	require.NotEqual(t, primaryHeaders[10].Hash(), divergentHeader1.Hash())
+	require.NotEqual(t, primaryHeaders[10].Hash(), divergentHeader2.Hash())
+
+	// generate a witness that has this divergent app hash but the same
+	// proposer priority hash
+	makeSparseWitness := func(divergent *types.SignedHeader) *mockp.Mock {
+		return mockp.New(
+			chainID,
+			map[int64]*types.SignedHeader{1: primaryHeaders[1], 10: divergent},
+			map[int64]*types.ValidatorSet{1: primaryVals[1], 10: valsAtH10},
+		)
+	}
+
+	// run for multiple iterations since this is exercising a race, 100
+	// typically causes it
+	const iterations = 100
+	for i := 0; i < iterations; i++ {
+		witness1 := makeSparseWitness(divergentHeader1)
+		witness2 := makeSparseWitness(divergentHeader2)
+
+		c, err := light.NewClient(
+			ctx,
+			chainID,
+			light.TrustOptions{
+				Height: 1,
+				Hash:   firstBlock.Hash(),
+				Period: 4 * time.Hour,
+			},
+			primary,
+			[]provider.Provider{witness1, witness2},
+			dbs.New(dbm.NewMemDB(), chainID),
+			light.Logger(log.TestingLogger()),
+		)
+		require.NoError(t, err)
+
+		// try and validate the block using our two witnesses that have
+		// diverged from the primary, this should error
+		_, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour))
+		require.Error(t, err)
+	}
+}
+
+func TestClientDivergentProposerPriorities(t *testing.T) {
+	const (
+		latestHeight = int64(10)
+		valSize      = 5
+	)
+
+	primaryHeaders, primaryValidators, _ := genMockNodeWithKeys(chainID, latestHeight, valSize, 2, bTime)
+	primary := mockp.New(chainID, primaryHeaders, primaryValidators)
+
+	witnessHeaders := make(map[int64]*types.SignedHeader, len(primaryHeaders))
+	witnessValidators := make(map[int64]*types.ValidatorSet, len(primaryValidators))
+	for height, header := range primaryHeaders {
+		witnessHeaders[height] = header
+	}
+	for height, valSet := range primaryValidators {
+		witnessValidators[height] = valSet
+	}
+
+	// Keep header hash identical while changing only proposer priorities in the validator set.
+	witnessValidators[latestHeight] = primaryValidators[latestHeight].CopyIncrementProposerPriority(1)
+	require.Equal(t, primaryHeaders[latestHeight].Hash(), witnessHeaders[latestHeight].Hash())
+	require.Equal(t, primaryValidators[latestHeight].Hash(), witnessValidators[latestHeight].Hash())
+	require.NotEqual(t,
+		primaryValidators[latestHeight].ProposerPriorityHash(),
+		witnessValidators[latestHeight].ProposerPriorityHash(),
+	)
+
+	witness := mockp.New(chainID, witnessHeaders, witnessValidators)
+	c, err := light.NewClient(
+		ctx,
+		chainID,
+		light.TrustOptions{
+			Period: 4 * time.Hour,
+			Height: 1,
+			Hash:   primaryHeaders[1].Hash(),
+		},
+		primary,
+		[]provider.Provider{witness},
+		dbs.New(dbm.NewMemDB(), chainID),
+		light.Logger(log.TestingLogger()),
+		light.MaxRetryAttempts(1),
+	)
+	require.NoError(t, err)
+
+	_, err = c.VerifyLightBlockAtHeight(ctx, latestHeight, bTime.Add(1*time.Hour))
+	require.Error(t, err)
+
+	var divergeErr light.ErrProposerPrioritiesDiverge
+	require.True(t, errors.As(err, &divergeErr), "expected ErrProposerPrioritiesDiverge, got %T: %v", err, err)
+	assert.Equal(t, 0, divergeErr.WitnessIndex)
+	assert.Equal(t, primaryValidators[latestHeight].ProposerPriorityHash(), divergeErr.PrimaryHash)
+	assert.Equal(t, witnessValidators[latestHeight].ProposerPriorityHash(), divergeErr.WitnessHash)
+}