Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rpc): validators endpoint fail during quorum rotation #959

Open
wants to merge 7 commits into
base: v1.3-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (s *heightProposerSelector) proposerFromStore(height int64) error {
"validators_hash", meta.Header.ValidatorsHash, "quorum_hash", s.valSet.QuorumHash,
"validators", s.valSet)

return fmt.Errorf("quorum hash mismatch at height %d", height)
return fmt.Errorf("validator set hash mismatch at height %d", height)
lklimek marked this conversation as resolved.
Show resolved Hide resolved
}

proposer = meta.Header.ProposerProTxHash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ func NewHeightRoundProposerSelector(vset *types.ValidatorSet, currentHeight int6

// proposerFromStore determines the proposer for the given height and round using current or previous block read from
// the block store.
//
// Requires s.valSet to be set to correct validator set.
func (s *heightRoundProposerSelector) proposerFromStore(height int64, round int32) error {
if s.bs == nil {
return fmt.Errorf("block store is nil")
Expand All @@ -85,15 +87,18 @@ func (s *heightRoundProposerSelector) proposerFromStore(height int64, round int3

meta := s.bs.LoadBlockMeta(height)
if meta != nil {
valSetHash := s.valSet.Hash()
// block already saved to store, just take the proposer
if !meta.Header.ValidatorsHash.Equal(s.valSet.Hash()) {
if !meta.Header.ValidatorsHash.Equal(valSetHash) {
// we loaded the same block, so quorum should be the same
s.logger.Error("quorum rotation detected but not expected",
"height", height,
"validators_hash", meta.Header.ValidatorsHash, "quorum_hash", s.valSet.QuorumHash,
"validators_hash", meta.Header.ValidatorsHash,
"expected_validators_hash", valSetHash,
"next_validators_hash", meta.Header.NextValidatorsHash,
"validators", s.valSet)

return fmt.Errorf("quorum hash mismatch at height %d", height)
return fmt.Errorf("validators hash mismatch at height %d", height)
}

proposer = meta.Header.ProposerProTxHash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ func NewProposerSelector(cp types.ConsensusParams, valSet *types.ValidatorSet, v
case int32(tmtypes.VersionParams_CONSENSUS_VERSION_0):
return NewHeightProposerSelector(valSet, valsetHeight, bs, logger)
case int32(tmtypes.VersionParams_CONSENSUS_VERSION_1):

return NewHeightRoundProposerSelector(valSet, valsetHeight, valsetRound, bs, logger)
default:
return nil, fmt.Errorf("unknown consensus version: %v", cp.Version.ConsensusVersion)
Expand Down
2 changes: 1 addition & 1 deletion internal/inspect/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func NewFromConfig(logger log.Logger, cfg *config.Config) (*Inspector, error) {
if err != nil {
return nil, err
}
ss := state.NewStore(sDB)
ss := state.NewStore(sDB, logger.With("module", "state_store"))
return New(cfg.RPC, bs, ss, sinks, logger), nil
}

Expand Down
93 changes: 66 additions & 27 deletions internal/state/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,12 @@ type dbStore struct {
var _ Store = (*dbStore)(nil)

// NewStore creates the dbStore of the state pkg.
func NewStore(db dbm.DB) Store {
return dbStore{db, log.NewNopLogger()}
func NewStore(db dbm.DB, logger ...log.Logger) Store {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain with a comment in the code what is happening here

if len(logger) != 1 || logger[0] == nil {
logger = []log.Logger{log.NewNopLogger()}
}

return dbStore{db, logger[0]}
}

// LoadState loads the State from the database.
Expand Down Expand Up @@ -498,30 +502,43 @@ func (store dbStore) SaveValidatorSets(lowerHeight, upperHeight int64, vals *typ
return batch.WriteSync()
}

//-----------------------------------------------------------------------------
// -----------------------------------------------------------------------------

// LoadValidators loads the ValidatorSet for a given height and round 0.
//
// Returns ErrNoValSetForHeight if the validator set can't be found for this height.
func (store dbStore) LoadValidators(height int64, bs selectproposer.BlockStore) (*types.ValidatorSet, error) {
// loadValidators is a helper that loads the validator set from height or last stored height.
// It does NOT set a correct proposer.
func (store dbStore) loadValidators(height int64) (*tmstate.ValidatorsInfo, error) {
valInfo, err := loadValidatorsInfo(store.db, height)
if err != nil {
return nil, ErrNoValSetForHeight{Height: height, Err: err}
return nil, ErrNoValSetForHeight{Height: height, Err: fmt.Errorf("failed to load validators info: %w", err)}
}

if valInfo.ValidatorSet == nil {
lastStoredHeight := lastStoredHeightFor(height, valInfo.LastHeightChanged)
store.logger.Debug("Validator set is nil, loading last stored height", "height", height, "last_height_changed", valInfo.LastHeightChanged, "last_stored_height", lastStoredHeight)
valInfo, err = loadValidatorsInfo(store.db, lastStoredHeight)
if err != nil || valInfo.ValidatorSet == nil {
return nil,
fmt.Errorf("couldn't find validators at height %d (height %d was originally requested): %w",
lastStoredHeight,
height,
err,
)
return nil, ErrNoValSetForHeight{Height: height, Err: fmt.Errorf("couldn't find validators at height %d (height %d was originally requested): %w",
lastStoredHeight,
height,
err,
)}
}
}

return valInfo, nil
}

// LoadValidators loads the ValidatorSet for a given height and round 0.
//
// Returns ErrNoValSetForHeight if the validator set can't be found for this height.
func (store dbStore) LoadValidators(height int64, bs selectproposer.BlockStore) (*types.ValidatorSet, error) {
store.logger.Debug("Loading validators", "height", height)

valInfo, err := store.loadValidators(height)
if err != nil {
return nil, ErrNoValSetForHeight{Height: height, Err: err}
}

valSet, err := types.ValidatorSetFromProto(valInfo.ValidatorSet)
if err != nil {
return nil, err
Expand Down Expand Up @@ -562,27 +579,48 @@ func (store dbStore) LoadValidators(height int64, bs selectproposer.BlockStore)
return strategy.ValidatorSet(), nil
}

// If we have that height in the block store, we just take proposer from previous block and advance it.
// If we don't have that height in the block store, we just take proposer from previous block and advance it.
// We don't use current height block because we want to return proposer at round 0.
prevMeta := bs.LoadBlockMeta(height - 1)
if prevMeta == nil {
return nil, fmt.Errorf("could not find block meta for height %d", height-1)
}
// Configure proposer strategy; first set proposer from previous block
if err := valSet.SetProposer(prevMeta.Header.ProposerProTxHash); err != nil {
return nil, fmt.Errorf("could not set proposer: %w", err)
}
strategy, err := selectproposer.NewProposerSelector(cp, valSet, prevMeta.Header.Height, prevMeta.Round, bs, store.logger)
if err != nil {
return nil, fmt.Errorf("failed to create validator scoring strategy: %w", err)
}
// Configure proposer strategy; first check if we have a quorum change
if !prevMeta.Header.ValidatorsHash.Equal(prevMeta.Header.NextValidatorsHash) {
// rotation happened - we select 1st validator as proposer
valSetHash := valSet.Hash()
if !prevMeta.Header.NextValidatorsHash.Equal(valSetHash) {
return nil, ErrNoValSetForHeight{
height,
fmt.Errorf("next validators hash mismatch at height %d, expected %X, got %X", height,
prevMeta.Header.NextValidatorsHash, valSetHash),
}
}

if err = valSet.SetProposer(valSet.GetByIndex(0).ProTxHash); err != nil {
return nil, ErrNoValSetForHeight{height, fmt.Errorf("could not set proposer: %w", err)}
}

return valSet, nil
} else {

// set proposer from previous block
if err := valSet.SetProposer(prevMeta.Header.ProposerProTxHash); err != nil {
return nil, fmt.Errorf("could not set proposer: %w", err)
}
strategy, err := selectproposer.NewProposerSelector(cp, valSet, prevMeta.Header.Height, prevMeta.Round, bs, store.logger)
if err != nil {
return nil, fmt.Errorf("failed to create validator scoring strategy: %w", err)
}

// now, advance to (height,0)
if err := strategy.UpdateHeightRound(height, 0); err != nil {
return nil, fmt.Errorf("failed to update validator scores at height %d, round 0: %w", height, err)
// now, advance to (height,0)
if err := strategy.UpdateHeightRound(height, 0); err != nil {
return nil, fmt.Errorf("failed to update validator scores at height %d, round 0: %w", height, err)
}

return strategy.ValidatorSet(), nil
}

return strategy.ValidatorSet(), nil
}

func lastStoredHeightFor(height, lastHeightChanged int64) int64 {
Expand Down Expand Up @@ -643,6 +681,7 @@ func (store dbStore) saveValidatorsInfo(
return err
}

store.logger.Debug("saving validator set", "height", height, "last_height_changed", lastHeightChanged, "validators", valSet)
return batch.Set(validatorsKey(height), bz)
}

Expand Down
113 changes: 107 additions & 6 deletions internal/state/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,18 @@ const (
// mockBlockStoreForProposerSelector creates a mock block store that returns proposers based on the height.
// It assumes every block ends in round 0 and the proposer is the next validator in the validator set.
func mockBlockStoreForProposerSelector(t *testing.T, startHeight, endHeight int64, vals *types.ValidatorSet) selectproposer.BlockStore {
vals = vals.Copy()
valsHash := vals.Hash()
blockStore := mocks.NewBlockStore(t)
blockStore.On("Base").Return(startHeight).Maybe()
configureBlockMetaWithValidators(t, blockStore, startHeight, endHeight, vals)

return blockStore
}

// configureBlockMetaWithValidators configures the block store to return proposers based on the height.
func configureBlockMetaWithValidators(_ *testing.T, blockStore *mocks.BlockStore, startHeight, endHeight int64, vals *types.ValidatorSet) {
vals = vals.Copy()
valsHash := vals.Hash()

for h := startHeight; h <= endHeight; h++ {
blockStore.On("LoadBlockMeta", h).
Return(&types.BlockMeta{
Expand All @@ -47,8 +55,6 @@ func mockBlockStoreForProposerSelector(t *testing.T, startHeight, endHeight int6
}).Maybe()
vals.IncProposerIndex(1)
}

return blockStore
}

func TestStoreBootstrap(t *testing.T) {
Expand Down Expand Up @@ -141,9 +147,9 @@ func TestStoreLoadValidators(t *testing.T) {
// check that a request will go back to the last checkpoint
_, err = stateStore.LoadValidators(valSetCheckpointInterval+1, blockStore)
require.Error(t, err)
require.Equal(t, fmt.Sprintf("couldn't find validators at height %d (height %d was originally requested): "+
require.ErrorContains(t, err, fmt.Sprintf("couldn't find validators at height %d (height %d was originally requested): "+
"value retrieved from db is empty",
valSetCheckpointInterval, valSetCheckpointInterval+1), err.Error())
valSetCheckpointInterval, valSetCheckpointInterval+1))

// now save a validator set at that checkpoint
err = stateStore.Save(makeRandomStateFromValidatorSet(vals, valSetCheckpointInterval, 1, blockStore))
Expand All @@ -167,6 +173,101 @@ func TestStoreLoadValidators(t *testing.T) {
require.Equal(t, expected, valsAtCheckpoint)
}

// Given a set of blocks in the block store and two validator sets that rotate,
// When we load the validators during quorum rotation,
// Then we receive the correct validators for each height.
func TestStoreLoadValidatorsOnRotation(t *testing.T) {
const startHeight = int64(1)

testCases := []struct {
rotations int64
nVals int
nValSets int64
consensusVersion int32
}{
{1, 3, 3, 0},
{1, 3, 3, 1},
{2, 3, 2, 0},
{2, 3, 2, 1},
{3, 2, 4, 0},
{3, 2, 4, 1},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("rotations=%d,nVals=%d,nValSets=%d,ver=%d",
tc.rotations, tc.nVals, tc.nValSets, tc.consensusVersion), func(t *testing.T) {
rotations := tc.rotations
nVals := tc.nVals
nValSets := tc.nValSets
uncommittedHeight := startHeight + rotations*int64(nVals)

stateDB := dbm.NewMemDB()
stateStore := sm.NewStore(stateDB, log.NewTestingLoggerWithLevel(t, log.LogLevelDebug))

validators := make([]*types.ValidatorSet, nValSets)
for i := int64(0); i < nValSets; i++ {
validators[i], _ = types.RandValidatorSet(nVals)
t.Logf("Validator set %d: %v", i, validators[i].Hash())
}

blockStore := mocks.NewBlockStore(t)
blockStore.On("Base").Return(startHeight).Maybe()

// configure block store and state store to return correct validators and block meta
for i := int64(0); i < rotations; i++ {
nextQuorumHeight := startHeight + (i+1)*int64(nVals)

err := stateStore.SaveValidatorSets(startHeight+i*int64(nVals), nextQuorumHeight-1, validators[i%nValSets])
require.NoError(t, err)

vals := validators[i%nValSets].Copy()
// reset proposer
require.NoError(t, vals.SetProposer(vals.GetByIndex(0).ProTxHash))
valsHash := vals.Hash()
nextValsHash := valsHash // we only change it in last block

// configure block store to return correct validator sets and proposers
for h := startHeight + i*int64(nVals); h < nextQuorumHeight; h++ {
if h == nextQuorumHeight-1 {
nextValsHash = validators[(i+1)%nValSets].Hash()
}
blockStore.On("LoadBlockMeta", h).
Return(&types.BlockMeta{
Header: types.Header{
Height: h,
ProposerProTxHash: vals.Proposer().ProTxHash,
ValidatorsHash: valsHash,
NextValidatorsHash: nextValsHash,
},
}).Maybe()
vals.IncProposerIndex(1)

// set consensus version
state := makeRandomStateFromValidatorSet(vals, h+1, startHeight+i*int64(nVals), blockStore)
state.LastHeightConsensusParamsChanged = h + 1
state.ConsensusParams.Version.ConsensusVersion = tc.consensusVersion
require.NoError(t, stateStore.Save(state))
}

assert.LessOrEqual(t, nextQuorumHeight, uncommittedHeight, "we should not save block at height {}", uncommittedHeight)
}

// now, we are at height 10, and we should rotate to next validators
// we don't have the last block yet, but we already have validator set for the next height
blockStore.On("LoadBlockMeta", uncommittedHeight).Return(nil).Maybe()

expectedValidators := validators[rotations%nValSets]
err := stateStore.SaveValidatorSets(uncommittedHeight, uncommittedHeight, expectedValidators)
require.NoError(t, err)
lklimek marked this conversation as resolved.
Show resolved Hide resolved

// We should get correct validator set from the store
readVals, err := stateStore.LoadValidators(uncommittedHeight, blockStore)
require.NoError(t, err)
assert.Equal(t, expectedValidators, readVals)
})
}
}
lklimek marked this conversation as resolved.
Show resolved Hide resolved

// This benchmarks the speed of loading validators from different heights if there is no validator set change.
// NOTE: This isn't too indicative of validator retrieval speed as the db is always (regardless of height) only
// performing two operations: 1) retrieve validator info at height x, which has a last validator set change of 1
Expand Down
2 changes: 1 addition & 1 deletion node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func makeNode(
}
closers = append(closers, dbCloser)

stateStore := sm.NewStore(stateDB)
stateStore := sm.NewStore(stateDB, logger.With("module", "state_store"))

genDoc, err := genesisDocProvider()
if err != nil {
Expand Down
Loading