diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index aa42e3722..4361afac8 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -30,3 +30,5 @@ program](https://hackerone.com/tendermint). - [mempool] \#2835 Remove local int64 counter from being stored in every tx ### BUG FIXES: + +- [rpc] \#2808 RPC validators calls IncrementAccum if necessary diff --git a/state/state.go b/state/state.go index 0dbd718da..451d65442 100644 --- a/state/state.go +++ b/state/state.go @@ -64,7 +64,8 @@ type State struct { // Validators are persisted to the database separately every time they change, // so we can query for historical validator sets. // Note that if s.LastBlockHeight causes a valset change, - // we set s.LastHeightValidatorsChanged = s.LastBlockHeight + 1 + // we set s.LastHeightValidatorsChanged = s.LastBlockHeight + 1 + 1 + // Extra +1 due to nextValSet delay. NextValidators *types.ValidatorSet Validators *types.ValidatorSet LastValidators *types.ValidatorSet diff --git a/state/state_test.go b/state/state_test.go index 17293f6fe..50346025e 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -3,9 +3,10 @@ package state import ( "bytes" "fmt" - "github.com/tendermint/tendermint/libs/log" "testing" + "github.com/tendermint/tendermint/libs/log" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -260,6 +261,27 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) { } } +func TestStoreLoadValidatorsIncrementsAccum(t *testing.T) { + const valSetSize = 2 + tearDown, stateDB, state := setupTestCase(t) + state.Validators = genValSet(valSetSize) + state.NextValidators = state.Validators.CopyIncrementAccum(1) + SaveState(stateDB, state) + defer tearDown(t) + + nextHeight := state.LastBlockHeight + 1 + + v0, err := LoadValidators(stateDB, nextHeight) + assert.Nil(t, err) + acc0 := v0.Validators[0].Accum + + v1, err := LoadValidators(stateDB, nextHeight+1) + assert.Nil(t, err) + acc1 := v1.Validators[0].Accum + + assert.NotEqual(t, acc1, acc0, "expected Accum value to change between heights") +} + // TestValidatorChangesSaveLoad tests saving and loading a validator set with // changes. func TestManyValidatorChangesSaveLoad(t *testing.T) { diff --git a/state/store.go b/state/store.go index 086dcdf5a..0effe38a5 100644 --- a/state/store.go +++ b/state/store.go @@ -89,7 +89,9 @@ func saveState(db dbm.DB, state State, key []byte) { nextHeight := state.LastBlockHeight + 1 // If first block, save validators for block 1. if nextHeight == 1 { - lastHeightVoteChanged := int64(1) // Due to Tendermint validator set changes being delayed 1 block. + // This extra logic due to Tendermint validator set changes being delayed 1 block. + // It may get overwritten due to InitChain validator updates. + lastHeightVoteChanged := int64(1) saveValidatorsInfo(db, nextHeight, lastHeightVoteChanged, state.Validators) } // Save next validators. @@ -191,12 +193,14 @@ func LoadValidators(db dbm.DB, height int64) (*types.ValidatorSet, error) { ), ) } + valInfo2.ValidatorSet.IncrementAccum(int(height - valInfo.LastHeightChanged)) // mutate valInfo = valInfo2 } return valInfo.ValidatorSet, nil } +// CONTRACT: Returned ValidatorsInfo can be mutated. func loadValidatorsInfo(db dbm.DB, height int64) *ValidatorsInfo { buf := db.Get(calcValidatorsKey(height)) if len(buf) == 0 { @@ -215,18 +219,22 @@ func loadValidatorsInfo(db dbm.DB, height int64) *ValidatorsInfo { return v } -// saveValidatorsInfo persists the validator set for the next block to disk. +// saveValidatorsInfo persists the validator set. +// `height` is the effective height for which the validator is responsible for signing. // It should be called from s.Save(), right before the state itself is persisted. // If the validator set did not change after processing the latest block, // only the last height for which the validators changed is persisted. -func saveValidatorsInfo(db dbm.DB, nextHeight, changeHeight int64, valSet *types.ValidatorSet) { +func saveValidatorsInfo(db dbm.DB, height, lastHeightChanged int64, valSet *types.ValidatorSet) { + if lastHeightChanged > height { + panic("LastHeightChanged cannot be greater than ValidatorsInfo height") + } valInfo := &ValidatorsInfo{ - LastHeightChanged: changeHeight, + LastHeightChanged: lastHeightChanged, } - if changeHeight == nextHeight { + if lastHeightChanged == height { valInfo.ValidatorSet = valSet } - db.Set(calcValidatorsKey(nextHeight), valInfo.Bytes()) + db.Set(calcValidatorsKey(height), valInfo.Bytes()) } //----------------------------------------------------------------------------- diff --git a/types/validator_set.go b/types/validator_set.go index ab030d1be..f5e57077b 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -62,7 +62,11 @@ func (vals *ValidatorSet) CopyIncrementAccum(times int) *ValidatorSet { // IncrementAccum increments accum of each validator and updates the // proposer. Panics if validator set is empty. +// `times` must be positive. func (vals *ValidatorSet) IncrementAccum(times int) { + if times <= 0 { + panic("Cannot call IncrementAccum with non-positive times") + } // Add VotingPower * times to each validator and order into heap. validatorsHeap := cmn.NewHeap() diff --git a/types/validator_set_test.go b/types/validator_set_test.go index aad9d85a8..81124637f 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -86,6 +86,19 @@ func TestCopy(t *testing.T) { } } +// Test that IncrementAccum requires positive times. +func TestIncrementAccumPositiveTimes(t *testing.T) { + vset := NewValidatorSet([]*Validator{ + newValidator([]byte("foo"), 1000), + newValidator([]byte("bar"), 300), + newValidator([]byte("baz"), 330), + }) + + assert.Panics(t, func() { vset.IncrementAccum(-1) }) + assert.Panics(t, func() { vset.IncrementAccum(0) }) + vset.IncrementAccum(1) +} + func BenchmarkValidatorSetCopy(b *testing.B) { b.StopTimer() vset := NewValidatorSet([]*Validator{}) @@ -239,7 +252,7 @@ func TestProposerSelection3(t *testing.T) { mod := (cmn.RandInt() % 5) + 1 if cmn.RandInt()%mod > 0 { // sometimes its up to 5 - times = cmn.RandInt() % 5 + times = (cmn.RandInt() % 4) + 1 } vset.IncrementAccum(times)