From cf0b5d3715962220fafa0402e8fee7a1b11cda4b Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 22 Dec 2017 14:07:46 -0600 Subject: [PATCH] enforce <1/3 validator updates Refs #950 --- consensus/reactor_test.go | 4 +- docs/app-development.rst | 18 +-- docs/specification/light-client-protocol.rst | 4 +- state/execution.go | 43 +++++--- state/state_test.go | 109 +++++++++---------- 5 files changed, 91 insertions(+), 87 deletions(-) diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 7383c790c..e6f5b4b19 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -194,8 +194,8 @@ func TestReactorVotingPowerChange(t *testing.T) { } func TestReactorValidatorSetChanges(t *testing.T) { - nPeers := 7 - nVals := 4 + nPeers := 9 + nVals := 6 css := randConsensusNetWithPeers(nVals, nPeers, "consensus_val_set_changes_test", newMockTickerFunc(true), newPersistentDummy) logger := log.TestingLogger() diff --git a/docs/app-development.rst b/docs/app-development.rst index 2614a264c..9574f8630 100644 --- a/docs/app-development.rst +++ b/docs/app-development.rst @@ -403,14 +403,16 @@ pick up from when it restarts. See information on the Handshake, below. EndBlock ^^^^^^^^ -The EndBlock request can be used to run some code at the end of every -block. Additionally, the response may contain a list of validators, -which can be used to update the validator set. To add a new validator or -update an existing one, simply include them in the list returned in the -EndBlock response. To remove one, include it in the list with a -``power`` equal to ``0``. Tendermint core will take care of updating the -validator set. Note validator set changes are only available in v0.8.0 -and up. +The EndBlock request can be used to run some code at the end of every block. +Additionally, the response may contain a list of validators, which can be used +to update the validator set. To add a new validator or update an existing one, +simply include them in the list returned in the EndBlock response. To remove +one, include it in the list with a ``power`` equal to ``0``. Tendermint core +will take care of updating the validator set. Note you can not update more than +1/3 of validators in one block because this will make it impossible for a light +client to prove the transition externally. See the `light client docs +`__ +for details on how it tracks validators. .. container:: toggle diff --git a/docs/specification/light-client-protocol.rst b/docs/specification/light-client-protocol.rst index 6c2531d71..6c6083b45 100644 --- a/docs/specification/light-client-protocol.rst +++ b/docs/specification/light-client-protocol.rst @@ -5,8 +5,8 @@ Light clients are an important part of the complete blockchain system for most applications. Tendermint provides unique speed and security properties for light client applications. -See our developing `light-client -repository `__. +See our `lite package +`__. Overview -------- diff --git a/state/execution.go b/state/execution.go index e90c8e710..f2cc3a4ee 100644 --- a/state/execution.go +++ b/state/execution.go @@ -10,7 +10,6 @@ import ( crypto "github.com/tendermint/go-crypto" "github.com/tendermint/tendermint/proxy" "github.com/tendermint/tendermint/types" - cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" ) @@ -112,9 +111,9 @@ func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn p return nil, err } - valUpdates := abciResponses.EndBlock.ValidatorUpdates - logger.Info("Executed block", "height", block.Height, "validTxs", validTxs, "invalidTxs", invalidTxs) + + valUpdates := abciResponses.EndBlock.ValidatorUpdates if len(valUpdates) > 0 { logger.Info("Updates to validators", "updates", abci.ValidatorsString(valUpdates)) } @@ -122,10 +121,22 @@ func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn p return abciResponses, nil } -func updateValidators(validators *types.ValidatorSet, changedValidators []*abci.Validator) error { - // TODO: prevent change of 1/3+ at once +func updateValidators(currentSet *types.ValidatorSet, updates []*abci.Validator) error { + // ## prevent update of 1/3+ at once + // + // If more than 1/3 validators changed in one block, then a light + // client could never prove the transition externally. See + // ./lite/doc.go for details on how a light client tracks + // validators. + maxUpdates := currentSet.Size() / 3 + if maxUpdates == 0 { // if current set size is less than 3 + maxUpdates = 1 + } + if len(updates) > maxUpdates { + return errors.New("Can not update more than 1/3 of validators at once") + } - for _, v := range changedValidators { + for _, v := range updates { pubkey, err := crypto.PubKeyFromBytes(v.PubKey) // NOTE: expects go-wire encoded pubkey if err != nil { return err @@ -135,28 +146,28 @@ func updateValidators(validators *types.ValidatorSet, changedValidators []*abci. power := int64(v.Power) // mind the overflow from int64 if power < 0 { - return errors.New(cmn.Fmt("Power (%d) overflows int64", v.Power)) + return fmt.Errorf("Power (%d) overflows int64", v.Power) } - _, val := validators.GetByAddress(address) + _, val := currentSet.GetByAddress(address) if val == nil { // add val - added := validators.Add(types.NewValidator(pubkey, power)) + added := currentSet.Add(types.NewValidator(pubkey, power)) if !added { - return errors.New(cmn.Fmt("Failed to add new validator %X with voting power %d", address, power)) + return fmt.Errorf("Failed to add new validator %X with voting power %d", address, power) } } else if v.Power == 0 { // remove val - _, removed := validators.Remove(address) + _, removed := currentSet.Remove(address) if !removed { - return errors.New(cmn.Fmt("Failed to remove validator %X)")) + return fmt.Errorf("Failed to remove validator %X", address) } } else { // update val val.VotingPower = power - updated := validators.Update(val) + updated := currentSet.Update(val) if !updated { - return errors.New(cmn.Fmt("Failed to update validator %X with voting power %d", address, power)) + return fmt.Errorf("Failed to update validator %X with voting power %d", address, power) } } } @@ -243,8 +254,8 @@ func (s *State) validateBlock(b *types.Block) error { } } else { if len(b.LastCommit.Precommits) != s.LastValidators.Size() { - return errors.New(cmn.Fmt("Invalid block commit size. Expected %v, got %v", - s.LastValidators.Size(), len(b.LastCommit.Precommits))) + return fmt.Errorf("Invalid block commit size. Expected %v, got %v", + s.LastValidators.Size(), len(b.LastCommit.Precommits)) } err := s.LastValidators.VerifyCommit( s.ChainID, s.LastBlockID, b.Height-1, b.LastCommit) diff --git a/state/state_test.go b/state/state_test.go index 1840d5e15..1df300c7f 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" @@ -130,81 +131,56 @@ func TestValidatorSimpleSaveLoad(t *testing.T) { assert.IsType(ErrNoValSetForHeight{}, err, "expected err at unknown height") } -// TestValidatorChangesSaveLoad tests saving and loading a validator set with changes. +// TestValidatorChangesSaveLoad tests saving and loading a validator set with +// changes. func TestValidatorChangesSaveLoad(t *testing.T) { + const valSetSize = 6 tearDown, _, state := setupTestCase(t) + state.Validators = genValSet(valSetSize) + state.Save() defer tearDown(t) - // nolint: vetshadow - assert := assert.New(t) - // change vals at these heights - changeHeights := []int64{1, 2, 4, 5, 10, 15, 16, 17, 20} - N := len(changeHeights) + const height = 1 + pubkey := crypto.GenPrivKeyEd25519().PubKey() + // swap the first validator with a new one ^^^ (validator set size stays the same) + header, parts, responses := makeHeaderPartsResponses(state, height, pubkey) + err := state.SetBlockAndValidators(header, parts, responses) + require.Nil(t, err) + state.saveValidatorsInfo() - // each valset is just one validator. - // create list of them - pubkeys := make([]crypto.PubKey, N+1) - _, val := state.Validators.GetByIndex(0) - pubkeys[0] = val.PubKey - for i := 1; i < N+1; i++ { - pubkeys[i] = crypto.GenPrivKeyEd25519().PubKey() - } + v, err := state.LoadValidators(height + 1) + assert.Nil(t, err) + assert.Equal(t, valSetSize, v.Size()) - // build the validator history by running SetBlockAndValidators - // with the right validator set for each height - highestHeight := changeHeights[N-1] + 5 - changeIndex := 0 - pubkey := pubkeys[changeIndex] - for i := int64(1); i < highestHeight; i++ { - // when we get to a change height, - // use the next pubkey - if changeIndex < len(changeHeights) && i == changeHeights[changeIndex] { - changeIndex++ - pubkey = pubkeys[changeIndex] - } - header, parts, responses := makeHeaderPartsResponses(state, i, pubkey) - state.SetBlockAndValidators(header, parts, responses) - state.saveValidatorsInfo() - } - - // make all the test cases by using the same validator until after the change - testCases := make([]valChangeTestCase, highestHeight) - changeIndex = 0 - pubkey = pubkeys[changeIndex] - for i := int64(1); i < highestHeight+1; i++ { - // we we get to the height after a change height - // use the next pubkey (note our counter starts at 0 this time) - if changeIndex < len(changeHeights) && i == changeHeights[changeIndex]+1 { - changeIndex++ - pubkey = pubkeys[changeIndex] - } - testCases[i-1] = valChangeTestCase{i, pubkey} + index, val := v.GetByAddress(pubkey.Address()) + assert.NotNil(t, val) + if index < 0 { + t.Fatal("expected to find newly added validator") } +} - for _, testCase := range testCases { - v, err := state.LoadValidators(testCase.height) - assert.Nil(err, fmt.Sprintf("expected no err at height %d", testCase.height)) - assert.Equal(v.Size(), 1, "validator set size is greater than 1: %d", v.Size()) - addr, _ := v.GetByIndex(0) - - assert.Equal(addr, testCase.vals.Address(), fmt.Sprintf(`unexpected pubkey at - height %d`, testCase.height)) +func genValSet(size int) *types.ValidatorSet { + vals := make([]*types.Validator, size) + for i := 0; i < size; i++ { + vals[i] = types.NewValidator(crypto.GenPrivKeyEd25519().PubKey(), 10) } + return types.NewValidatorSet(vals) } -// TestConsensusParamsChangesSaveLoad tests saving and loading consensus params with changes. +// TestConsensusParamsChangesSaveLoad tests saving and loading consensus params +// with changes. func TestConsensusParamsChangesSaveLoad(t *testing.T) { tearDown, _, state := setupTestCase(t) + const valSetSize = 20 + state.Validators = genValSet(valSetSize) + state.Save() defer tearDown(t) - // nolint: vetshadow - assert := assert.New(t) // change vals at these heights changeHeights := []int64{1, 2, 4, 5, 10, 15, 16, 17, 20} N := len(changeHeights) - // each valset is just one validator. - // create list of them + // create list of new vals params := make([]types.ConsensusParams, N+1) params[0] = state.ConsensusParams for i := 1; i < N+1; i++ { @@ -225,7 +201,8 @@ func TestConsensusParamsChangesSaveLoad(t *testing.T) { cp = params[changeIndex] } header, parts, responses := makeHeaderPartsResponsesParams(state, i, cp) - state.SetBlockAndValidators(header, parts, responses) + err := state.SetBlockAndValidators(header, parts, responses) + require.Nil(t, err) state.saveConsensusParamsInfo() } @@ -245,8 +222,8 @@ func TestConsensusParamsChangesSaveLoad(t *testing.T) { for _, testCase := range testCases { p, err := state.LoadConsensusParams(testCase.height) - assert.Nil(err, fmt.Sprintf("expected no err at height %d", testCase.height)) - assert.Equal(testCase.params, p, fmt.Sprintf(`unexpected consensus params at + assert.Nil(t, err, fmt.Sprintf("expected no err at height %d", testCase.height)) + assert.Equal(t, testCase.params, p, fmt.Sprintf(`unexpected consensus params at height %d`, testCase.height)) } } @@ -270,6 +247,20 @@ func makeParams(blockBytes, blockTx, blockGas, txBytes, } } +func TestLessThanOneThirdOfValidatorUpdatesEnforced(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) + + height := state.LastBlockHeight + 1 + block := makeBlock(state, height) + abciResponses := &ABCIResponses{ + Height: height, + EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: []*abci.Validator{{PubKey: []byte("a"), Power: 10}}}, + } + err := state.SetBlockAndValidators(block.Header, types.PartSetHeader{}, abciResponses) + assert.NotNil(t, err, "expected err when trying to update more than 1/3 of validators") +} + func TestApplyUpdates(t *testing.T) { initParams := makeParams(1, 2, 3, 4, 5, 6)