diff --git a/internal/consensus/state_data.go b/internal/consensus/state_data.go index 20b60e428..425e4fd2d 100644 --- a/internal/consensus/state_data.go +++ b/internal/consensus/state_data.go @@ -280,7 +280,7 @@ func (s *StateData) updateToState(state sm.State, commit *types.Commit, blockSto s.Validators, state.InitialHeight, 0, - nil, + blockStore, ) } else { // validators == state.Validators contain validator set at (state.LastBlockHeight, 0) @@ -289,7 +289,7 @@ func (s *StateData) updateToState(state sm.State, commit *types.Commit, blockSto s.Validators, state.LastBlockHeight, 0, - nil, + blockStore, ) } diff --git a/internal/evidence/mocks/block_store.go b/internal/evidence/mocks/block_store.go index 41fff57cd..0062c4c65 100644 --- a/internal/evidence/mocks/block_store.go +++ b/internal/evidence/mocks/block_store.go @@ -12,6 +12,24 @@ type BlockStore struct { mock.Mock } +// Base provides a mock function with given fields: +func (_m *BlockStore) Base() int64 { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Base") + } + + var r0 int64 + if rf, ok := ret.Get(0).(func() int64); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(int64) + } + + return r0 +} + // Height provides a mock function with given fields: func (_m *BlockStore) Height() int64 { ret := _m.Called() diff --git a/internal/evidence/services.go b/internal/evidence/services.go index c71783a27..eb7b53aeb 100644 --- a/internal/evidence/services.go +++ b/internal/evidence/services.go @@ -9,5 +9,6 @@ import ( type BlockStore interface { LoadBlockMeta(height int64) *types.BlockMeta LoadBlockCommit(height int64) *types.Commit + Base() int64 Height() int64 } diff --git a/internal/features/validatorscoring/height_round_score.go b/internal/features/validatorscoring/height_round_score.go index 50e20e7d5..32f0ae2e0 100644 --- a/internal/features/validatorscoring/height_round_score.go +++ b/internal/features/validatorscoring/height_round_score.go @@ -10,6 +10,7 @@ import ( type BlockCommitStore interface { LoadBlockCommit(height int64) *types.Commit LoadBlockMeta(height int64) *types.BlockMeta + Base() int64 } type heightRoundBasedScoringStrategy struct { diff --git a/internal/features/validatorscoring/height_score.go b/internal/features/validatorscoring/height_score.go index 88d3ce3cb..0cbc00cbf 100644 --- a/internal/features/validatorscoring/height_score.go +++ b/internal/features/validatorscoring/height_score.go @@ -4,12 +4,16 @@ import ( "fmt" "sync" + "github.com/dashpay/tenderdash/libs/bytes" + "github.com/dashpay/tenderdash/libs/log" "github.com/dashpay/tenderdash/types" ) type heightBasedScoringStrategy struct { valSet *types.ValidatorSet height int64 + bs BlockCommitStore + logger log.Logger mtx sync.Mutex } @@ -32,23 +36,30 @@ func NewHeightBasedScoringStrategy(vset *types.ValidatorSet, currentHeight int64 return nil, fmt.Errorf("empty validator set") } + logger, err := log.NewDefaultLogger(log.LogFormatPlain, log.LogLevelDebug) // TODO: make configurable + if err != nil { + return nil, fmt.Errorf("could not create logger: %w", err) + } + s := &heightBasedScoringStrategy{ valSet: vset, height: currentHeight, + bs: bs, + logger: logger, } // if we have a block store, we can determine the proposer for the current height; // otherwise we just trust the state of `vset` - if bs != nil { - if err := s.initProposer(currentHeight, bs); err != nil { - return nil, err + if bs != nil && bs.Base() > 0 && currentHeight >= bs.Base() { + if err := s.selectProposer(currentHeight, bs); err != nil { + return nil, fmt.Errorf("could not initialize proposer: %w", err) } } return s, nil } -// initProposer determines the proposer for the given height using block store and consensus params. -func (s *heightBasedScoringStrategy) initProposer(height int64, bs BlockCommitStore) error { +// selectProposer determines the proposer for the given height using block store and consensus params. +func (s *heightBasedScoringStrategy) selectProposer(height int64, bs BlockCommitStore) error { if bs == nil { return fmt.Errorf("block store is nil") } @@ -57,16 +68,18 @@ func (s *heightBasedScoringStrategy) initProposer(height int64, bs BlockCommitSt if height == 0 || height == 1 { // we take first proposer from the validator set if err := s.valSet.SetProposer(s.valSet.Validators[0].ProTxHash); err != nil { - return fmt.Errorf("could not set initial proposer: %w", err) + return fmt.Errorf("could not determine proposer: %w", err) } return nil } meta := bs.LoadBlockMeta(height) + var proposer bytes.HexBytes + addToIdx := int32(0) if meta == nil { - // we use previous height, and will just add 1 to proposer index + // block not found; we try previous height, and will just add 1 to proposer index meta = bs.LoadBlockMeta(height - 1) if meta == nil { return fmt.Errorf("could not find block meta for previous height %d", height-1) @@ -74,7 +87,23 @@ func (s *heightBasedScoringStrategy) initProposer(height int64, bs BlockCommitSt addToIdx = 1 } - if err := s.valSet.SetProposer(meta.Header.ProposerProTxHash); err != nil { + if !meta.Header.ValidatorsHash.Equal(s.valSet.Hash()) { + s.logger.Debug("quorum rotation happened", + "height", height, + "validators_hash", meta.Header.ValidatorsHash, "quorum_hash", s.valSet.QuorumHash, + "validators", s.valSet, + "meta", meta) + // quorum rotation happened - we select 1st validator as proposer, and don't rotate + // NOTE: We use index 1 due to bug in original code - we need to preserve the original bad behavior + // to avoid breaking consensus + proposer = s.valSet.GetByIndex(1).ProTxHash + + addToIdx = 0 + } else { + proposer = meta.Header.ProposerProTxHash + } + + if err := s.valSet.SetProposer(proposer); err != nil { return fmt.Errorf("could not set proposer: %w", err) } @@ -107,7 +136,17 @@ func (s *heightBasedScoringStrategy) updateScores(newHeight int64, _round int32) } if heightDiff > 1 { - return fmt.Errorf("cannot jump more than one height: %d -> %d", s.height, newHeight) + if s.bs == nil || s.bs.Base() > s.height { + return fmt.Errorf("cannot jump more than one height without data in block store: %d -> %d", s.height, newHeight) + } + // FIXME: we assume that no consensus version update happened in the meantime + + if err := s.selectProposer(newHeight, s.bs); err != nil { + return fmt.Errorf("could not determine proposer: %w", err) + } + + s.height = newHeight + return nil } s.valSet.IncProposerIndex(int32(heightDiff)) //nolint:gosec diff --git a/internal/state/state_test.go b/internal/state/state_test.go index c687f1a68..e7561138c 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -263,11 +263,7 @@ func TestValidatorSimpleSaveLoad(t *testing.T) { assert.Equal(t, v.Hash(), state.Validators.Hash(), "expected validator hashes to match") // Should NOT be able to load for height 2. - blockStore.On("LoadBlockMeta", int64(2)).Return(&types.BlockMeta{ - Header: types.Header{ - Height: 2, - ProposerProTxHash: state.Validators.GetByIndex((int32(state.LastBlockHeight) % int32(state.Validators.Size()))).ProTxHash, - }}) + blockStore.On("LoadBlockMeta", int64(2)).Return(nil) _, err = statestore.LoadValidators(2, blockStore) require.Error(t, err, "expected no err at height 2") @@ -332,6 +328,12 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) { state, err = state.Update(blockID, &header, &changes) + blockStore.On("LoadBlockMeta", header.Height).Return(&types.BlockMeta{ + Header: types.Header{ + Height: header.Height, + ProposerProTxHash: header.ProposerProTxHash, + }}).Maybe() + require.NoError(t, err) validator := state.Validators.Validators[0] keys[height+1] = validator.PubKey @@ -656,12 +658,13 @@ func TestManyValidatorChangesSaveLoad(t *testing.T) { require.Equal(t, int64(0), state.LastBlockHeight) state.Validators, _ = types.RandValidatorSet(valSetSize) err := stateStore.Save(state) + require.NoError(t, err) + blockStore.On("LoadBlockMeta", state.LastBlockHeight).Return(&types.BlockMeta{ Header: types.Header{ Height: state.LastBlockHeight, - ProposerProTxHash: state.Validators.GetByIndex(int32(state.LastBlockHeight-state.InitialHeight) % valSetSize).ProTxHash, + ProposerProTxHash: state.Validators.GetByIndex(0).ProTxHash, }}).Maybe() - require.NoError(t, err) // ====== HEIGHT 2 ====== // @@ -697,6 +700,11 @@ func TestManyValidatorChangesSaveLoad(t *testing.T) { err = stateStore.Save(state) require.NoError(t, err) + blockStore.On("LoadBlockMeta", currentHeight).Return(&types.BlockMeta{ + Header: types.Header{ + Height: currentHeight, + ProposerProTxHash: state.Validators.GetByIndex(int32(currentHeight-state.InitialHeight) % valSetSize).ProTxHash, + }}).Maybe() // Load height, it should be the oldpubkey. v0, err := stateStore.LoadValidators(currentHeight-1, blockStore) diff --git a/proto/tendermint/types/params.pb.go b/proto/tendermint/types/params.pb.go index 489c129f1..323345f82 100644 --- a/proto/tendermint/types/params.pb.go +++ b/proto/tendermint/types/params.pb.go @@ -30,7 +30,9 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type VersionParams_ConsensusVersion int32 const ( + // CONSENSUS_VERSION_0 is the original version of the consensus protocol. VersionParams_CONSENSUS_VERSION_0 VersionParams_ConsensusVersion = 0 + // CONSENSUS_VERSION_1 changes proposer selection algorithm to not double-propose when previous proposer is offline. VersionParams_CONSENSUS_VERSION_1 VersionParams_ConsensusVersion = 1 ) @@ -327,7 +329,7 @@ func (m *ValidatorParams) GetPubKeyTypes() []string { // VersionParams contains the ABCI application version. type VersionParams struct { AppVersion uint64 `protobuf:"varint,1,opt,name=app_version,json=appVersion,proto3" json:"app_version,omitempty"` - // consensus_version + // Version of consensus protocol, used to upgrade consensus without hard forks ConsensusVersion VersionParams_ConsensusVersion `protobuf:"varint,2,opt,name=consensus_version,json=consensusVersion,proto3,enum=tendermint.types.VersionParams_ConsensusVersion" json:"consensus_version,omitempty"` }