From 86707862d40556d32391133773b61e27390fbf95 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 31 Aug 2020 12:47:38 +0200 Subject: [PATCH] fix validator set proposer priorities in light client provider (#5307) --- CHANGELOG_PENDING.md | 2 ++ light/provider/http/http.go | 2 +- types/validator_set.go | 33 +++++++++++++++++++++++++++++++++ types/validator_set_test.go | 18 ++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c46b481c4..6b4933da4 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -22,3 +22,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [blockchain] \#5249 Fix fast sync halt with initial height > 1 (@erikgrinaker) - [statesync] \#5302 Fix genesis state propagation to state sync routine (@erikgrinaker) + +- [light] [\#5307](https://github.com/tendermint/tendermint/pull/5307) persist correct proposer priority in light client validator sets (@cmwaters) diff --git a/light/provider/http/http.go b/light/provider/http/http.go index 4db48a5ad..5c67a9d15 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -119,7 +119,7 @@ func (p *http) ValidatorSet(height int64) (*types.ValidatorSet, error) { page++ } - return types.NewValidatorSet(vals), nil + return types.ValidatorSetFromExistingValidators(vals) } // ReportEvidence calls `/broadcast_evidence` endpoint. diff --git a/types/validator_set.go b/types/validator_set.go index 533d0e1ed..e1bfd31f7 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -822,6 +822,24 @@ func (vals *ValidatorSet) VerifyCommitLightTrusting(chainID string, commit *Comm return ErrNotEnoughVotingPowerSigned{Got: talliedVotingPower, Needed: votingPowerNeeded} } +// findPreviousProposer reverses the compare proposer priority function to find the validator +// with the lowest proposer priority which would have been the previous proposer. +// +// Is used when recreating a validator set from an existing array of validators. +func (vals *ValidatorSet) findPreviousProposer() *Validator { + var previousProposer *Validator + for _, val := range vals.Validators { + if previousProposer == nil { + previousProposer = val + continue + } + if previousProposer == previousProposer.CompareProposerPriority(val) { + previousProposer = val + } + } + return previousProposer +} + //----------------- // IsErrNotEnoughVotingPowerSigned returns true if err is @@ -966,6 +984,21 @@ func ValidatorSetFromProto(vp *tmproto.ValidatorSet) (*ValidatorSet, error) { return vals, vals.ValidateBasic() } +// ValidatorSetFromExistingValidators takes an existing array of validators and rebuilds +// the exact same validator set that corresponds to it without changing the proposer priority or power +func ValidatorSetFromExistingValidators(valz []*Validator) (*ValidatorSet, error) { + if len(valz) == 0 { + return nil, errors.New("no validators given") + } + vals := &ValidatorSet{ + Validators: valz, + } + vals.Proposer = vals.findPreviousProposer() + vals.updateTotalVotingPower() + sort.Sort(ValidatorsByVotingPower(vals.Validators)) + return vals, nil +} + //---------------------------------------- // RandValidatorSet returns a randomized validator set (size: +numValidators+), diff --git a/types/validator_set_test.go b/types/validator_set_test.go index 20a3ce2f7..6745b20e0 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -1425,6 +1425,24 @@ func verifyValSetUpdatePriorityOrder(t *testing.T, valSet *ValidatorSet, cfg tes } } +func TestNewValidatorSetFromExistingValidators(t *testing.T) { + size := 5 + vals := make([]*Validator, size) + for i := 0; i < size; i++ { + pv := NewMockPV() + vals[i] = pv.ExtractIntoValidator(int64(i + 1)) + } + valSet := NewValidatorSet(vals) + valSet.IncrementProposerPriority(5) + + newValSet := NewValidatorSet(valSet.Validators) + existingValSet, err := ValidatorSetFromExistingValidators(valSet.Validators) + require.NoError(t, err) + assert.NotEqual(t, valSet, newValSet) + assert.Equal(t, valSet, existingValSet) + assert.Equal(t, valSet.CopyIncrementProposerPriority(3), existingValSet.CopyIncrementProposerPriority(3)) +} + func TestValSetUpdateOverflowRelated(t *testing.T) { testCases := []testVSetCfg{ {