diff --git a/CHANGELOG.md b/CHANGELOG.md index 913f1d1e995..283a22cc728 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/light/detector.go b/light/detector.go index 0429bd1aad7..930644a5a48 100644 --- a/light/detector.go +++ b/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 diff --git a/light/detector_test.go b/light/detector_test.go index d8afacefd89..4afceeaf888 100644 --- a/light/detector_test.go +++ b/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) +}