diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 9b1c4349e..122fd6610 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -89,3 +89,4 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - [privval] \#5638 Increase read/write timeout to 5s and calculate ping interval based on it (@JoeKash) - [blockchain/v1] [\#5701](https://github.com/tendermint/tendermint/pull/5701) Handle peers without blocks (@melekes) - [blockchain/v1] \#5711 Fix deadlock (@melekes) +- [evidence] \#6375 Fix bug with inconsistent LightClientAttackEvidence hashing (cmwaters) diff --git a/evidence/pool.go b/evidence/pool.go index bc09ea94e..71d282436 100644 --- a/evidence/pool.go +++ b/evidence/pool.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "sort" "sync" "sync/atomic" "time" @@ -203,9 +202,11 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error { hashes := make([][]byte, len(evList)) for idx, ev := range evList { - ok := evpool.fastCheck(ev) + _, isLightEv := ev.(*types.LightClientAttackEvidence) - if !ok { + // We must verify light client attack evidence regardless because there could be a + // different conflicting block with the same hash. + if isLightEv || !evpool.isPending(ev) { // check that the evidence isn't already committed if evpool.isCommitted(ev) { return &types.ErrInvalidEvidence{Evidence: ev, Reason: errors.New("evidence was already committed")} @@ -222,7 +223,7 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error { evpool.logger.Error("failed to add evidence to pending list", "err", err, "evidence", ev) } - evpool.logger.Info("verified new evidence of byzantine behavior", "evidence", ev) + evpool.logger.Info("check evidence: verified evidence of byzantine behavior", "evidence", ev) } // check for duplicate evidence. We cache hashes so we don't have to work them out again. @@ -260,78 +261,6 @@ func (evpool *Pool) State() sm.State { return evpool.state } -// fastCheck leverages the fact that the evidence pool may have already verified -// the evidence to see if it can quickly conclude that the evidence is already -// valid. -func (evpool *Pool) fastCheck(ev types.Evidence) bool { - if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { - key := keyPending(ev) - evBytes, err := evpool.evidenceStore.Get(key) - if evBytes == nil { // the evidence is not in the nodes pending list - return false - } - - if err != nil { - evpool.logger.Error("failed to load light client attack evidence", "err", err, "key(height/hash)", key) - return false - } - - var trustedPb tmproto.LightClientAttackEvidence - - if err = trustedPb.Unmarshal(evBytes); err != nil { - evpool.logger.Error( - "failed to convert light client attack evidence from bytes", - "key(height/hash)", key, - "err", err, - ) - return false - } - - trustedEv, err := types.LightClientAttackEvidenceFromProto(&trustedPb) - if err != nil { - evpool.logger.Error( - "failed to convert light client attack evidence from protobuf", - "key(height/hash)", key, - "err", err, - ) - return false - } - - // Ensure that all the byzantine validators that the evidence pool has match - // the byzantine validators in this evidence. - if trustedEv.ByzantineValidators == nil && lcae.ByzantineValidators != nil { - return false - } - - if len(trustedEv.ByzantineValidators) != len(lcae.ByzantineValidators) { - return false - } - - byzValsCopy := make([]*types.Validator, len(lcae.ByzantineValidators)) - for i, v := range lcae.ByzantineValidators { - byzValsCopy[i] = v.Copy() - } - - // ensure that both validator arrays are in the same order - sort.Sort(types.ValidatorsByVotingPower(byzValsCopy)) - - for idx, val := range trustedEv.ByzantineValidators { - if !bytes.Equal(byzValsCopy[idx].Address, val.Address) { - return false - } - if byzValsCopy[idx].VotingPower != val.VotingPower { - return false - } - } - - return true - } - - // For all other evidence the evidence pool just checks if it is already in - // the pending db. - return evpool.isPending(ev) -} - // IsExpired checks whether evidence or a polc is expired by checking whether a height and time is older // than set by the evidence consensus parameters func (evpool *Pool) isExpired(height int64, time time.Time) bool { @@ -396,7 +325,7 @@ func (evpool *Pool) markEvidenceAsCommitted(evidence types.EvidenceList, height for _, ev := range evidence { if evpool.isPending(ev) { if err := batch.Delete(keyPending(ev)); err != nil { - evpool.logger.Error("failed to batch pending evidence", "err", err) + evpool.logger.Error("failed to batch delete pending evidence", "err", err) } blockEvidenceMap[evMapKey(ev)] = struct{}{} } @@ -546,7 +475,7 @@ func (evpool *Pool) batchExpiredPendingEvidence(batch dbm.Batch) (int64, time.Ti // else add to the batch if err := batch.Delete(iter.Key()); err != nil { - evpool.logger.Error("failed to batch evidence", "err", err, "ev", ev) + evpool.logger.Error("failed to batch delete evidence", "err", err, "ev", ev) continue } diff --git a/evidence/pool_test.go b/evidence/pool_test.go index 866ebd018..dabda6578 100644 --- a/evidence/pool_test.go +++ b/evidence/pool_test.go @@ -14,7 +14,6 @@ import ( "github.com/tendermint/tendermint/evidence" "github.com/tendermint/tendermint/evidence/mocks" "github.com/tendermint/tendermint/libs/log" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" sm "github.com/tendermint/tendermint/state" smmocks "github.com/tendermint/tendermint/state/mocks" "github.com/tendermint/tendermint/store" @@ -166,6 +165,9 @@ func TestReportConflictingVotes(t *testing.T) { // should be able to retrieve evidence from pool evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes) require.Equal(t, []types.Evidence{ev}, evList) + + next = pool.EvidenceFront() + require.NotNil(t, next) } func TestEvidencePoolUpdate(t *testing.T) { @@ -262,84 +264,61 @@ func TestVerifyDuplicatedEvidenceFails(t *testing.T) { // check that valid light client evidence is correctly validated and stored in // evidence pool -func TestCheckEvidenceWithLightClientAttack(t *testing.T) { +func TestLightClientAttackEvidenceLifecycle(t *testing.T) { var ( - nValidators = 5 - validatorPower int64 = 10 - height int64 = 10 + height int64 = 100 + commonHeight int64 = 90 ) - conflictingVals, conflictingPrivVals := types.RandValidatorSet(nValidators, validatorPower) - trustedHeader := makeHeaderRandom(height) - trustedHeader.Time = defaultEvidenceTime - - conflictingHeader := makeHeaderRandom(height) - conflictingHeader.ValidatorsHash = conflictingVals.Hash() - - trustedHeader.ValidatorsHash = conflictingHeader.ValidatorsHash - trustedHeader.NextValidatorsHash = conflictingHeader.NextValidatorsHash - trustedHeader.ConsensusHash = conflictingHeader.ConsensusHash - trustedHeader.AppHash = conflictingHeader.AppHash - trustedHeader.LastResultsHash = conflictingHeader.LastResultsHash - - // For simplicity we are simulating a duplicate vote attack where all the - // validators in the conflictingVals set voted twice. - blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) - voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) - - commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - - ev := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: conflictingHeader, - Commit: commit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 10, - TotalVotingPower: int64(nValidators) * validatorPower, - ByzantineValidators: conflictingVals.Validators, - Timestamp: defaultEvidenceTime, - } - - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) - trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) - trustedCommit, err := types.MakeCommit( - trustedBlockID, - height, - 1, - trustedVoteSet, - conflictingPrivVals, - defaultEvidenceTime, - ) - require.NoError(t, err) + ev, trusted, common := makeLunaticEvidence(t, height, commonHeight, + 10, 5, 5, defaultEvidenceTime, defaultEvidenceTime.Add(1*time.Hour)) state := sm.State{ - LastBlockTime: defaultEvidenceTime.Add(1 * time.Minute), - LastBlockHeight: 11, + LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), + LastBlockHeight: 110, ConsensusParams: *types.DefaultConsensusParams(), } stateStore := &smmocks.Store{} - stateStore.On("LoadValidators", height).Return(conflictingVals, nil) + stateStore.On("LoadValidators", height).Return(trusted.ValidatorSet, nil) + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) stateStore.On("Load").Return(state, nil) blockStore := &mocks.BlockStore{} - blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trustedHeader}) - blockStore.On("LoadBlockCommit", height).Return(trustedCommit) + blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockCommit", height).Return(trusted.Commit) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) pool, err := evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) require.NoError(t, err) + hash := ev.Hash() + require.NoError(t, pool.AddEvidence(ev)) - require.NoError(t, pool.CheckEvidence(types.EvidenceList{ev})) + require.NoError(t, pool.AddEvidence(ev)) + + pendingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Equal(t, 1, len(pendingEv)) + require.Equal(t, ev, pendingEv[0]) + + require.NoError(t, pool.CheckEvidence(pendingEv)) + require.Equal(t, ev, pendingEv[0]) - // Take away the last signature -> there are less validators then what we have detected, - // hence this should fail. - commit.Signatures = append(commit.Signatures[:nValidators-1], types.NewCommitSigAbsent()) + state.LastBlockHeight++ + state.LastBlockTime = state.LastBlockTime.Add(1 * time.Minute) + pool.Update(state, pendingEv) + require.Equal(t, hash, pendingEv[0].Hash()) + + remaindingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Empty(t, remaindingEv) + + // evidence is already committed so it shouldn't pass require.Error(t, pool.CheckEvidence(types.EvidenceList{ev})) + require.NoError(t, pool.AddEvidence(ev)) + + remaindingEv, _ = pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Empty(t, remaindingEv) } // Tests that restarting the evidence pool after a potential failure will recover the @@ -389,7 +368,7 @@ func TestRecoverPendingEvidence(t *testing.T) { Evidence: types.EvidenceParams{ MaxAgeNumBlocks: 20, MaxAgeDuration: 20 * time.Minute, - MaxBytes: 1000, + MaxBytes: defaultEvidenceMaxBytes, }, }, }, nil) diff --git a/evidence/verify.go b/evidence/verify.go index 406f6b8da..f8142add2 100644 --- a/evidence/verify.go +++ b/evidence/verify.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "sort" "time" "github.com/tendermint/tendermint/light" @@ -129,56 +128,6 @@ func (evpool *Pool) verify(evidence types.Evidence) error { if err != nil { return types.NewErrInvalidEvidence(evidence, err) } - - // 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 types.NewErrInvalidEvidence( - evidence, - 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 types.NewErrInvalidEvidence( - evidence, - fmt.Errorf("expected %d byzantine validators from evidence but got %d", exp, got), - ) - } - - // ensure that both validator arrays are in the same order - sort.Sort(types.ValidatorsByVotingPower(ev.ByzantineValidators)) - - for idx, val := range validators { - if !bytes.Equal(ev.ByzantineValidators[idx].Address, val.Address) { - return types.NewErrInvalidEvidence( - evidence, - 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 types.NewErrInvalidEvidence( - evidence, - fmt.Errorf( - "evidence contained unexpected byzantine validator power; expected %d, got %d", - val.VotingPower, ev.ByzantineValidators[idx].VotingPower, - ), - ) - } - } - return nil default: @@ -206,7 +155,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t } // In the case of equivocation and amnesia we expect all header hashes to be correctly derived - } else if isInvalidHeader(trustedHeader.Header, e.ConflictingBlock.Header) { + } else if e.ConflictingHeaderIsInvalid(trustedHeader.Header) { return errors.New("common height is the same as conflicting block height so expected the conflicting" + " block to be correctly derived yet it wasn't") } @@ -235,7 +184,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t trustedHeader.Hash()) } - return nil + return validateABCIEvidence(e, commonVals, trustedHeader) } // VerifyDuplicateVote verifies DuplicateVoteEvidence against the state of full node. This involves the @@ -306,6 +255,55 @@ 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 { @@ -320,15 +318,3 @@ func getSignedHeader(blockStore BlockStore, height int64) (*types.SignedHeader, Commit: commit, }, nil } - -// isInvalidHeader takes a trusted header and matches it againt a conflicting header -// to determine whether the conflicting header was the product of a valid state transition -// or not. If it is then all the deterministic fields of the header should be the same. -// If not, it is an invalid header and constitutes a lunatic attack. -func isInvalidHeader(trusted, conflicting *types.Header) bool { - return !bytes.Equal(trusted.ValidatorsHash, conflicting.ValidatorsHash) || - !bytes.Equal(trusted.NextValidatorsHash, conflicting.NextValidatorsHash) || - !bytes.Equal(trusted.ConsensusHash, conflicting.ConsensusHash) || - !bytes.Equal(trusted.AppHash, conflicting.AppHash) || - !bytes.Equal(trusted.LastResultsHash, conflicting.LastResultsHash) -} diff --git a/evidence/verify_test.go b/evidence/verify_test.go index dfb8cec18..e9bd43e25 100644 --- a/evidence/verify_test.go +++ b/evidence/verify_test.go @@ -1,6 +1,7 @@ package evidence_test import ( + "bytes" "context" "testing" "time" @@ -22,139 +23,170 @@ import ( "github.com/tendermint/tendermint/version" ) -func TestVerifyLightClientAttack_Lunatic(t *testing.T) { - commonVals, commonPrivVals := types.RandValidatorSet(2, 10) - - newVal, newPrivVal := types.RandValidator(false, 9) - - conflictingVals, err := types.ValidatorSetFromExistingValidators(append(commonVals.Validators, newVal)) - require.NoError(t, err) - conflictingPrivVals := append(commonPrivVals, newPrivVal) - - commonHeader := makeHeaderRandom(4) - commonHeader.Time = defaultEvidenceTime - trustedHeader := makeHeaderRandom(10) - trustedHeader.Time = defaultEvidenceTime.Add(1 * time.Hour) - - conflictingHeader := makeHeaderRandom(10) - conflictingHeader.Time = defaultEvidenceTime.Add(1 * time.Hour) - conflictingHeader.ValidatorsHash = conflictingVals.Hash() - - // we are simulating a lunatic light client attack - blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) - voteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals) - commit, err := types.MakeCommit(blockID, 10, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - ev := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: conflictingHeader, - Commit: commit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 4, - TotalVotingPower: 20, - ByzantineValidators: commonVals.Validators, - Timestamp: defaultEvidenceTime, - } +const ( + defaultVotingPower = 10 +) - commonSignedHeader := &types.SignedHeader{ - Header: commonHeader, - Commit: &types.Commit{}, - } - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) - vals, privVals := types.RandValidatorSet(3, 8) - trustedVoteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), vals) - trustedCommit, err := types.MakeCommit(trustedBlockID, 10, 1, trustedVoteSet, privVals, defaultEvidenceTime) - require.NoError(t, err) - trustedSignedHeader := &types.SignedHeader{ - Header: trustedHeader, - Commit: trustedCommit, - } +func TestVerifyLightClientAttack_Lunatic(t *testing.T) { + const ( + height int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 4 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + // create valid lunatic evidence + ev, trusted, common := makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + require.NoError(t, ev.ValidateBasic()) // good pass -> no error - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, trustedSignedHeader, commonVals, + err := evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) assert.NoError(t, err) // trusted and conflicting hashes are the same -> an error should be returned - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, ev.ConflictingBlock.SignedHeader, commonVals, + err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, ev.ConflictingBlock.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) assert.Error(t, err) // evidence with different total validator power should fail - ev.TotalVotingPower = 1 - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, trustedSignedHeader, commonVals, + 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) - ev.TotalVotingPower = 20 - - forwardConflictingHeader := makeHeaderRandom(11) - forwardConflictingHeader.Time = defaultEvidenceTime.Add(30 * time.Minute) - forwardConflictingHeader.ValidatorsHash = conflictingVals.Hash() - forwardBlockID := makeBlockID(forwardConflictingHeader.Hash(), 1000, []byte("partshash")) - forwardVoteSet := types.NewVoteSet(evidenceChainID, 11, 1, tmproto.SignedMsgType(2), conflictingVals) - forwardCommit, err := types.MakeCommit(forwardBlockID, 11, 1, forwardVoteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - forwardLunaticEv := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: forwardConflictingHeader, - Commit: forwardCommit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 4, - TotalVotingPower: 20, - ByzantineValidators: commonVals.Validators, - Timestamp: defaultEvidenceTime, - } - err = evidence.VerifyLightClientAttack(forwardLunaticEv, commonSignedHeader, trustedSignedHeader, commonVals, + + // evidence without enough malicious votes should fail + ev, trusted, common = makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals-1, totalVals-byzVals, defaultEvidenceTime, attackTime) + err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) - assert.NoError(t, err) + assert.Error(t, err) +} + +func TestVerify_LunaticAttackAgainstState(t *testing.T) { + const ( + height int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 4 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + // create valid lunatic evidence + ev, trusted, common := makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + // now we try to test verification against state state := sm.State{ LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), - LastBlockHeight: 11, + LastBlockHeight: height + 1, ConsensusParams: *types.DefaultConsensusParams(), } stateStore := &smmocks.Store{} - stateStore.On("LoadValidators", int64(4)).Return(commonVals, nil) + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) stateStore.On("Load").Return(state, nil) blockStore := &mocks.BlockStore{} - blockStore.On("LoadBlockMeta", int64(4)).Return(&types.BlockMeta{Header: *commonHeader}) - blockStore.On("LoadBlockMeta", int64(10)).Return(&types.BlockMeta{Header: *trustedHeader}) - blockStore.On("LoadBlockMeta", int64(11)).Return(nil) - blockStore.On("LoadBlockCommit", int64(4)).Return(commit) - blockStore.On("LoadBlockCommit", int64(10)).Return(trustedCommit) - blockStore.On("Height").Return(int64(10)) - + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + blockStore.On("LoadBlockCommit", height).Return(trusted.Commit) pool, err := evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) require.NoError(t, err) evList := types.EvidenceList{ev} - err = pool.CheckEvidence(evList) - assert.NoError(t, err) + // check that the evidence pool correctly verifies the evidence + assert.NoError(t, pool.CheckEvidence(evList)) + // as it was not originally in the pending bucket, it should now have been added pendingEvs, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) assert.Equal(t, 1, len(pendingEvs)) + assert.Equal(t, ev, pendingEvs[0]) // if we submit evidence only against a single byzantine validator when we see there are more validators then this // should return an error - ev.ByzantineValidators = []*types.Validator{commonVals.Validators[0]} - err = pool.CheckEvidence(evList) - assert.Error(t, err) - ev.ByzantineValidators = commonVals.Validators // restore evidence + ev.ByzantineValidators = ev.ByzantineValidators[:1] + t.Log(evList) + assert.Error(t, pool.CheckEvidence(evList)) + // restore original byz vals + ev.ByzantineValidators = ev.GetByzantineValidators(common.ValidatorSet, trusted.SignedHeader) + + // duplicate evidence should be rejected + evList = types.EvidenceList{ev, ev} + pool, err = evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.CheckEvidence(evList)) // If evidence is submitted with an altered timestamp it should return an error ev.Timestamp = defaultEvidenceTime.Add(1 * time.Minute) - err = pool.CheckEvidence(evList) - assert.Error(t, err) + pool, err = evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.AddEvidence(ev)) + ev.Timestamp = defaultEvidenceTime - evList = types.EvidenceList{forwardLunaticEv} - err = pool.CheckEvidence(evList) - assert.NoError(t, err) + // Evidence submitted with a different validator power should fail + ev.TotalVotingPower = 1 + pool, err = evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.AddEvidence(ev)) + ev.TotalVotingPower = common.ValidatorSet.TotalVotingPower() +} + +func TestVerify_ForwardLunaticAttack(t *testing.T) { + const ( + nodeHeight int64 = 8 + attackHeight int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 5 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + + // create a forward lunatic attack + ev, trusted, common := makeLunaticEvidence( + t, attackHeight, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + + // now we try to test verification against state + state := sm.State{ + LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), + LastBlockHeight: nodeHeight, + ConsensusParams: *types.DefaultConsensusParams(), + } + + // modify trusted light block so that it is of a height less than the conflicting one + trusted.Header.Height = state.LastBlockHeight + trusted.Header.Time = state.LastBlockTime + + stateStore := &smmocks.Store{} + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) + stateStore.On("Load").Return(state, nil) + blockStore := &mocks.BlockStore{} + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockMeta", nodeHeight).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockMeta", attackHeight).Return(nil) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + blockStore.On("LoadBlockCommit", nodeHeight).Return(trusted.Commit) + blockStore.On("Height").Return(nodeHeight) + pool, err := evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + + // check that the evidence pool correctly verifies the evidence + assert.NoError(t, pool.CheckEvidence(types.EvidenceList{ev})) + + // now we use a time which isn't able to contradict the FLA - thus we can't verify the evidence + oldBlockStore := &mocks.BlockStore{} + oldHeader := trusted.Header + oldHeader.Time = defaultEvidenceTime + oldBlockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + oldBlockStore.On("LoadBlockMeta", nodeHeight).Return(&types.BlockMeta{Header: *oldHeader}) + oldBlockStore.On("LoadBlockMeta", attackHeight).Return(nil) + oldBlockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + oldBlockStore.On("LoadBlockCommit", nodeHeight).Return(trusted.Commit) + oldBlockStore.On("Height").Return(nodeHeight) + require.Equal(t, defaultEvidenceTime, oldBlockStore.LoadBlockMeta(nodeHeight).Header.Time) + + pool, err = evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, oldBlockStore) + require.NoError(t, err) + assert.Error(t, pool.CheckEvidence(types.EvidenceList{ev})) } func TestVerifyLightClientAttack_Equivocation(t *testing.T) { @@ -414,6 +446,84 @@ func TestVerifyDuplicateVoteEvidence(t *testing.T) { assert.Error(t, err) } +func makeLunaticEvidence( + t *testing.T, + height, commonHeight int64, + totalVals, byzVals, phantomVals int, + commonTime, attackTime time.Time, +) (ev *types.LightClientAttackEvidence, trusted *types.LightBlock, common *types.LightBlock) { + commonValSet, commonPrivVals := types.RandValidatorSet(totalVals, defaultVotingPower) + + require.Greater(t, totalVals, byzVals) + + // extract out the subset of byzantine validators in the common validator set + byzValSet, byzPrivVals := commonValSet.Validators[:byzVals], commonPrivVals[:byzVals] + + phantomValSet, phantomPrivVals := types.RandValidatorSet(phantomVals, defaultVotingPower) + + conflictingVals := phantomValSet.Copy() + require.NoError(t, conflictingVals.UpdateWithChangeSet(byzValSet)) + conflictingPrivVals := append(phantomPrivVals, byzPrivVals...) + + conflictingPrivVals = orderPrivValsByValSet(t, conflictingVals, conflictingPrivVals) + + commonHeader := makeHeaderRandom(commonHeight) + commonHeader.Time = commonTime + trustedHeader := makeHeaderRandom(height) + + conflictingHeader := makeHeaderRandom(height) + conflictingHeader.Time = attackTime + conflictingHeader.ValidatorsHash = conflictingVals.Hash() + + blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) + voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) + commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) + require.NoError(t, err) + ev = &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: conflictingHeader, + Commit: commit, + }, + ValidatorSet: conflictingVals, + }, + CommonHeight: commonHeight, + TotalVotingPower: commonValSet.TotalVotingPower(), + ByzantineValidators: byzValSet, + Timestamp: commonTime, + } + + common = &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: commonHeader, + // we can leave this empty because we shouldn't be checking this + Commit: &types.Commit{}, + }, + ValidatorSet: commonValSet, + } + trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) + trustedVals, privVals := types.RandValidatorSet(totalVals, defaultVotingPower) + trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), trustedVals) + trustedCommit, err := types.MakeCommit(trustedBlockID, height, 1, trustedVoteSet, privVals, defaultEvidenceTime) + require.NoError(t, err) + trusted = &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: trustedHeader, + Commit: trustedCommit, + }, + ValidatorSet: trustedVals, + } + return ev, trusted, common +} + +// func makeEquivocationEvidence() *types.LightClientAttackEvidence { + +// } + +// func makeAmnesiaEvidence() *types.LightClientAttackEvidence { + +// } + func makeVote( t *testing.T, val types.PrivValidator, chainID string, valIndex int32, height int64, round int32, step int, blockID types.BlockID, time time.Time) *types.Vote { @@ -472,3 +582,20 @@ func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.Bloc }, } } + +func orderPrivValsByValSet( + t *testing.T, vals *types.ValidatorSet, privVals []types.PrivValidator) []types.PrivValidator { + output := make([]types.PrivValidator, len(privVals)) + for idx, v := range vals.Validators { + for _, p := range privVals { + pubKey, err := p.GetPubKey(context.Background()) + require.NoError(t, err) + if bytes.Equal(v.Address, pubKey.Address()) { + output[idx] = p + break + } + } + require.NotEmpty(t, output[idx]) + } + return output +} diff --git a/p2p/shim.go b/p2p/shim.go index 5f5e8cede..07d1ad156 100644 --- a/p2p/shim.go +++ b/p2p/shim.go @@ -144,7 +144,10 @@ func (rs *ReactorShim) proxyPeerEnvelopes() { } if !src.Send(cs.Descriptor.ID, bz) { - rs.Logger.Error( + // This usually happens when we try to send across a channel + // that the peer doesn't have open. To avoid bloating the + // logs we set this to be Debug + rs.Logger.Debug( "failed to proxy message to peer", "ch_id", cs.Descriptor.ID, "peer", e.To, diff --git a/test/e2e/networks/ci.toml b/test/e2e/networks/ci.toml index 3509a399d..64dcf321a 100644 --- a/test/e2e/networks/ci.toml +++ b/test/e2e/networks/ci.toml @@ -2,7 +2,7 @@ # functionality with a single network. disable_legacy_p2p = false -evidence = 0 +evidence = 5 initial_height = 1000 initial_state = {initial01 = "a", initial02 = "b", initial03 = "c"} queue_type = "priority" @@ -57,7 +57,7 @@ seeds = ["seed01"] persist_interval = 3 perturb = ["kill"] privval_protocol = "grpc" -retain_blocks = 3 +retain_blocks = 5 [node.validator04] abci_protocol = "builtin" @@ -82,7 +82,7 @@ start_at = 1010 fast_sync = "v0" persistent_peers = ["validator01", "validator02", "validator03", "validator04", "validator05"] perturb = ["restart"] -retain_blocks = 3 +retain_blocks = 5 [node.full02] mode = "full" diff --git a/test/e2e/pkg/testnet.go b/test/e2e/pkg/testnet.go index 7bb64d7a6..3fb5543a8 100644 --- a/test/e2e/pkg/testnet.go +++ b/test/e2e/pkg/testnet.go @@ -48,7 +48,7 @@ const ( PerturbationPause Perturbation = "pause" PerturbationRestart Perturbation = "restart" - EvidenceAgeHeight int64 = 3 + EvidenceAgeHeight int64 = 5 EvidenceAgeTime time.Duration = 10 * time.Second ) diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go index ece241cd2..85a177db6 100644 --- a/test/e2e/runner/evidence.go +++ b/test/e2e/runner/evidence.go @@ -19,11 +19,8 @@ import ( "github.com/tendermint/tendermint/version" ) -// 1 in 11 evidence is light client evidence, the rest is duplicate vote -// FIXME: Setting to 11 disables light client attack evidence since nodes -// don't follow a minimum retention height invariant. When we fix this we -// should use a ratio of 4. -const lightClientEvidenceRatio = 11 +// 1 in 4 evidence is light client evidence, the rest is duplicate vote evidence +const lightClientEvidenceRatio = 4 // InjectEvidence takes a running testnet and generates an amount of valid // evidence and broadcasts it to a random node through the rpc endpoint `/broadcast_evidence`. @@ -74,7 +71,7 @@ func InjectEvidence(testnet *e2e.Testnet, amount int) error { duplicateVoteTime := status.SyncInfo.LatestBlockTime var ev types.Evidence - for i := 0; i < amount; i++ { + for i := 1; i <= amount; i++ { if i%lightClientEvidenceRatio == 0 { ev, err = generateLightClientAttackEvidence( privVals, lightEvidenceCommonHeight, valSet, testnet.Name, blockRes.Block.Time, diff --git a/test/e2e/tests/block_test.go b/test/e2e/tests/block_test.go index 369b49d61..b3f4e9139 100644 --- a/test/e2e/tests/block_test.go +++ b/test/e2e/tests/block_test.go @@ -37,8 +37,12 @@ func TestBlock_Header(t *testing.T) { } resp, err := client.Block(ctx, &block.Header.Height) require.NoError(t, err) + require.Equal(t, block, resp.Block, - "block mismatch for height %v", block.Header.Height) + "block mismatch for height %d", block.Header.Height) + + require.NoError(t, resp.Block.ValidateBasic(), + "block at height %d is invalid", block.Header.Height) } }) } diff --git a/types/evidence.go b/types/evidence.go index 8e3c7decc..90d42dec1 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -296,7 +296,10 @@ func (l *LightClientAttackEvidence) ConflictingHeaderIsInvalid(trustedHeader *He // with evidence that have the same conflicting header and common height but different permutations // of validator commit signatures. The reason for this is that we don't want to allow several // permutations of the same evidence to be committed on chain. Ideally we commit the header with the -// most commit signatures (captures the most byzantine validators) but anything greater than 1/3 is sufficient. +// most commit signatures (captures the most byzantine validators) but anything greater than 1/3 is +// sufficient. +// TODO: We should change the hash to include the commit, header, total voting power, byzantine +// validators and timestamp func (l *LightClientAttackEvidence) Hash() []byte { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutVarint(buf, l.CommonHeight) @@ -315,8 +318,14 @@ func (l *LightClientAttackEvidence) Height() int64 { // String returns a string representation of LightClientAttackEvidence func (l *LightClientAttackEvidence) String() string { - return fmt.Sprintf("LightClientAttackEvidence{ConflictingBlock: %v, CommonHeight: %d}", - l.ConflictingBlock.String(), l.CommonHeight) + return fmt.Sprintf(`LightClientAttackEvidence{ + ConflictingBlock: %v, + CommonHeight: %d, + ByzatineValidators: %v, + TotalVotingPower: %d, + Timestamp: %v}#%X`, + l.ConflictingBlock.String(), l.CommonHeight, l.ByzantineValidators, + l.TotalVotingPower, l.Timestamp, l.Hash()) } // Time returns the time of the common block where the infraction leveraged off. @@ -335,8 +344,8 @@ func (l *LightClientAttackEvidence) ValidateBasic() error { return errors.New("conflicting block missing header") } - if err := l.ConflictingBlock.ValidateBasic(l.ConflictingBlock.ChainID); err != nil { - return fmt.Errorf("invalid conflicting light block: %w", err) + if l.TotalVotingPower <= 0 { + return errors.New("negative or zero total voting power") } if l.CommonHeight <= 0 { @@ -351,6 +360,10 @@ func (l *LightClientAttackEvidence) ValidateBasic() error { l.CommonHeight, l.ConflictingBlock.Height) } + if err := l.ConflictingBlock.ValidateBasic(l.ConflictingBlock.ChainID); err != nil { + return fmt.Errorf("invalid conflicting light block: %w", err) + } + return nil } @@ -422,6 +435,8 @@ func (evl EvidenceList) Hash() []byte { // the Evidence size is capped. evidenceBzs := make([][]byte, len(evl)) for i := 0; i < len(evl); i++ { + // TODO: We should change this to the hash. Using bytes contains some unexported data that + // may cause different hashes evidenceBzs[i] = evl[i].Bytes() } return merkle.HashFromByteSlices(evidenceBzs) diff --git a/types/evidence_test.go b/types/evidence_test.go index f50ed0b18..7e9bed7e6 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -89,9 +89,11 @@ func TestDuplicateVoteEvidenceValidation(t *testing.T) { } } -func TestLightClientAttackEvidence(t *testing.T) { +func TestLightClientAttackEvidenceBasic(t *testing.T) { height := int64(5) - voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, 10, 1) + commonHeight := height - 1 + nValidators := 10 + voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, nValidators, 1) header := makeHeaderRandom() header.Height = height blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt32, tmhash.Sum([]byte("partshash"))) @@ -105,56 +107,52 @@ func TestLightClientAttackEvidence(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } assert.NotNil(t, lcae.String()) assert.NotNil(t, lcae.Hash()) - // only 7 validators sign - differentCommit, err := MakeCommit(blockID, height, 1, voteSet, privVals[:7], defaultVoteTime) - require.NoError(t, err) - differentEv := &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: header, - Commit: differentCommit, - }, - ValidatorSet: valSet, - }, - CommonHeight: height - 1, - } - assert.Equal(t, lcae.Hash(), differentEv.Hash()) - // different header hash - differentHeader := makeHeaderRandom() - differentEv = &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: differentHeader, - Commit: differentCommit, - }, - ValidatorSet: valSet, - }, - CommonHeight: height - 1, + assert.Equal(t, lcae.Height(), commonHeight) // Height should be the common Height + assert.NotNil(t, lcae.Bytes()) + + // maleate evidence to test hash uniqueness + testCases := []struct { + testName string + malleateEvidence func(*LightClientAttackEvidence) + }{ + {"Different header", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.Header = makeHeaderRandom() }}, + {"Different common height", func(ev *LightClientAttackEvidence) { + ev.CommonHeight = height + 1 + }}, } - assert.NotEqual(t, lcae.Hash(), differentEv.Hash()) - // different common height should produce a different header - differentEv = &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: header, - Commit: differentCommit, + + for _, tc := range testCases { + lcae := &LightClientAttackEvidence{ + ConflictingBlock: &LightBlock{ + SignedHeader: &SignedHeader{ + Header: header, + Commit: commit, + }, + ValidatorSet: valSet, }, - ValidatorSet: valSet, - }, - CommonHeight: height - 2, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], + } + hash := lcae.Hash() + tc.malleateEvidence(lcae) + assert.NotEqual(t, hash, lcae.Hash(), tc.testName) } - assert.NotEqual(t, lcae.Hash(), differentEv.Hash()) - assert.Equal(t, lcae.Height(), int64(4)) // Height should be the common Height - assert.NotNil(t, lcae.Bytes()) } func TestLightClientAttackEvidenceValidation(t *testing.T) { height := int64(5) - voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, 10, 1) + commonHeight := height - 1 + nValidators := 10 + voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, nValidators, 1) header := makeHeaderRandom() header.Height = height header.ValidatorsHash = valSet.Hash() @@ -169,7 +167,10 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } assert.NoError(t, lcae.ValidateBasic()) @@ -178,16 +179,22 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { malleateEvidence func(*LightClientAttackEvidence) expectErr bool }{ - {"Good DuplicateVoteEvidence", func(ev *LightClientAttackEvidence) {}, false}, + {"Good LightClientAttackEvidence", func(ev *LightClientAttackEvidence) {}, false}, {"Negative height", func(ev *LightClientAttackEvidence) { ev.CommonHeight = -10 }, true}, {"Height is greater than divergent block", func(ev *LightClientAttackEvidence) { ev.CommonHeight = height + 1 }, true}, + {"Height is equal to the divergent block", func(ev *LightClientAttackEvidence) { + ev.CommonHeight = height + }, false}, {"Nil conflicting header", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.Header = nil }, true}, {"Nil conflicting blocl", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock = nil }, true}, {"Nil validator set", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.ValidatorSet = &ValidatorSet{} }, true}, + {"Negative total voting power", func(ev *LightClientAttackEvidence) { + ev.TotalVotingPower = -1 + }, true}, } for _, tc := range testCases { tc := tc @@ -200,7 +207,10 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } tc.malleateEvidence(lcae) if tc.expectErr { diff --git a/types/light.go b/types/light.go index 8f09d8205..5a650a159 100644 --- a/types/light.go +++ b/types/light.go @@ -149,7 +149,7 @@ func (sh SignedHeader) ValidateBasic(chainID string) error { if sh.Commit.Height != sh.Height { return fmt.Errorf("header and commit height mismatch: %d vs %d", sh.Height, sh.Commit.Height) } - if hhash, chash := sh.Hash(), sh.Commit.BlockID.Hash; !bytes.Equal(hhash, chash) { + if hhash, chash := sh.Header.Hash(), sh.Commit.BlockID.Hash; !bytes.Equal(hhash, chash) { return fmt.Errorf("commit signs block %X, header is block %X", chash, hhash) } diff --git a/types/light_test.go b/types/light_test.go index f4e1e52df..f3e0af1a6 100644 --- a/types/light_test.go +++ b/types/light_test.go @@ -152,11 +152,13 @@ func TestSignedHeaderValidateBasic(t *testing.T) { Header: tc.shHeader, Commit: tc.shCommit, } - assert.Equal( + err := sh.ValidateBasic(validSignedHeader.Header.ChainID) + assert.Equalf( t, tc.expectErr, - sh.ValidateBasic(validSignedHeader.Header.ChainID) != nil, + err != nil, "Validate Basic had an unexpected result", + err, ) }) } diff --git a/types/validator_set.go b/types/validator_set.go index 9b3647605..3cf1baca5 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -1028,7 +1028,9 @@ func (vals *ValidatorSet) ToProto() (*tmproto.ValidatorSet, error) { } vp.Proposer = valProposer - vp.TotalVotingPower = vals.totalVotingPower + // NOTE: Sometimes we use the bytes of the proto form as a hash. This means that we need to + // be consistent with cached data + vp.TotalVotingPower = 0 return vp, nil } @@ -1059,7 +1061,12 @@ func ValidatorSetFromProto(vp *tmproto.ValidatorSet) (*ValidatorSet, error) { vals.Proposer = p - vals.totalVotingPower = vp.GetTotalVotingPower() + // NOTE: We can't trust the total voting power given to us by other peers. If someone were to + // inject a non-zeo value that wasn't the correct voting power we could assume a wrong total + // power hence we need to recompute it. + // FIXME: We should look to remove TotalVotingPower from proto or add it in the validators hash + // so we don't have to do this + vals.TotalVotingPower() return vals, vals.ValidateBasic() }