From 78446fd99c923586a04fd6a1500be10175e87035 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 21 Aug 2017 16:31:54 -0400 Subject: [PATCH 1/5] state: persist validators --- consensus/replay.go | 3 ++ state/errors.go | 8 +++ state/execution.go | 3 ++ state/state.go | 115 ++++++++++++++++++++++++++++++++++++++------ state/state_test.go | 60 +++++++++++++++++------ 5 files changed, 161 insertions(+), 28 deletions(-) diff --git a/consensus/replay.go b/consensus/replay.go index 0c7c27e90..71dc81b5a 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -324,8 +324,11 @@ func (h *Handshaker) ReplayBlocks(appHash []byte, appBlockHeight int, proxyApp p func (h *Handshaker) replayBlocks(proxyApp proxy.AppConns, appBlockHeight, storeBlockHeight int, mutateState bool) ([]byte, error) { // App is further behind than it should be, so we need to replay blocks. // We replay all blocks from appBlockHeight+1. + // // Note that we don't have an old version of the state, // so we by-pass state validation/mutation using sm.ExecCommitBlock. + // This also means we won't be saving validator sets if they change during this period. + // // If mutateState == true, the final block is replayed with h.replayBlock() var appHash []byte diff --git a/state/errors.go b/state/errors.go index 50c5a2c04..4a87384a0 100644 --- a/state/errors.go +++ b/state/errors.go @@ -33,6 +33,10 @@ type ( Got *State Expected *State } + + ErrNoValSetForHeight struct { + Height int + } ) func (e ErrUnknownBlock) Error() string { @@ -53,3 +57,7 @@ func (e ErrLastStateMismatch) Error() string { func (e ErrStateMismatch) Error() string { return cmn.Fmt("State after replay does not match saved state. Got ----\n%v\nExpected ----\n%v\n", e.Got, e.Expected) } + +func (e ErrNoValSetForHeight) Error() string { + return cmn.Fmt("Could not find validator set for height #%d", e.Height) +} diff --git a/state/execution.go b/state/execution.go index 9e4a5fb29..869d28d48 100644 --- a/state/execution.go +++ b/state/execution.go @@ -231,6 +231,9 @@ func (s *State) ApplyBlock(eventCache types.Fireable, proxyAppConn proxy.AppConn // now update the block and validators s.SetBlockAndValidators(block.Header, partsHeader, abciResponses) + // save the validators for the next block now that we know them + s.SaveValidators() + // lock mempool, commit state, update mempoool err = s.CommitStateUpdateMempool(proxyAppConn, block, mempool) if err != nil { diff --git a/state/state.go b/state/state.go index 21ad92776..6106891b5 100644 --- a/state/state.go +++ b/state/state.go @@ -22,6 +22,10 @@ var ( abciResponsesKey = []byte("abciResponsesKey") ) +func calcValidatorsKey(height int) []byte { + return []byte(cmn.Fmt("validatorsKey:%v", height)) +} + //----------------------------------------------------------------------------- // State represents the latest committed state of the Tendermint consensus, @@ -47,8 +51,11 @@ type State struct { // AppHash is updated after Commit AppHash []byte + // XXX: do we need this json tag ? TxIndexer txindex.TxIndexer `json:"-"` // Transaction indexer. + lastHeightValidatorsChanged int + logger log.Logger } @@ -71,6 +78,14 @@ func loadState(db dbm.DB, key []byte) *State { } // TODO: ensure that buf is completely read. } + + v := s.loadValidators(s.LastBlockHeight) + if v != nil { + s.lastHeightValidatorsChanged = v.LastHeightChanged + } else { + s.lastHeightValidatorsChanged = 1 + } + return s } @@ -93,6 +108,8 @@ func (s *State) Copy() *State { AppHash: s.AppHash, TxIndexer: s.TxIndexer, // pointer here, not value logger: s.logger, + + lastHeightValidatorsChanged: s.lastHeightValidatorsChanged, } } @@ -126,6 +143,56 @@ func (s *State) LoadABCIResponses() *ABCIResponses { return abciResponses } +// SaveValidators persists the validator set for the next block to disk. +// It should be called after the validator set is updated with the results of EndBlock. +// If the validator set did not change after processing the latest block, +// only the last height for which the validators changed is persisted. +func (s *State) SaveValidators() { + lastHeight := s.lastHeightValidatorsChanged + nextHeight := s.LastBlockHeight + 1 + v := &Validators{ + LastHeightChanged: lastHeight, + } + if lastHeight == nextHeight { + v.ValidatorSet = s.Validators + } + s.db.SetSync(calcValidatorsKey(nextHeight), v.Bytes()) +} + +// LoadValidators loads the ValidatorSet for a given height. +func (s *State) LoadValidators(height int) (*types.ValidatorSet, error) { + v := s.loadValidators(height) + if v == nil { + return nil, ErrNoValSetForHeight{height} + } + + if v.ValidatorSet == nil { + v = s.loadValidators(v.LastHeightChanged) + if v == nil { + return nil, ErrNoValSetForHeight{height} + } + } + + return v.ValidatorSet, nil +} + +func (s *State) loadValidators(height int) *Validators { + buf := s.db.Get(calcValidatorsKey(height)) + if len(buf) == 0 { + return nil + } + + v := new(Validators) + r, n, err := bytes.NewReader(buf), new(int), new(error) + wire.ReadBinaryPtr(v, r, 0, n, err) + if *err != nil { + // DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED + cmn.Exit(cmn.Fmt("LoadValidators: Data has been corrupted or its spec has changed: %v\n", *err)) + } + // TODO: ensure that buf is completely read. + return v +} + // Equals returns true if the States are identical. func (s *State) Equals(s2 *State) bool { return bytes.Equal(s.Bytes(), s2.Bytes()) @@ -144,10 +211,15 @@ func (s *State) SetBlockAndValidators(header *types.Header, blockPartsHeader typ prevValSet := s.Validators.Copy() nextValSet := prevValSet.Copy() - err := updateValidators(nextValSet, abciResponses.EndBlock.Diffs) - if err != nil { - s.logger.Error("Error changing validator set", "err", err) - // TODO: err or carry on? + // update the validator set with the latest abciResponses + if len(abciResponses.EndBlock.Diffs) > 0 { + err := updateValidators(nextValSet, abciResponses.EndBlock.Diffs) + if err != nil { + s.logger.Error("Error changing validator set", "err", err) + // TODO: err or carry on? + } + // change results from this height but only applies to the next height + s.lastHeightValidatorsChanged = header.Height + 1 } // Update validator accums and set state variables @@ -182,6 +254,7 @@ func GetState(stateDB dbm.DB, genesisFile string) *State { state := LoadState(stateDB) if state == nil { state = MakeGenesisStateFromFile(stateDB, genesisFile) + state.SaveValidators() // save the validators right away for height 1 state.Save() } @@ -215,6 +288,19 @@ func (a *ABCIResponses) Bytes() []byte { return wire.BinaryBytes(*a) } +//----------------------------------------------------------------------------- + +// Validators represents the latest validator set, or the last time it changed +type Validators struct { + ValidatorSet *types.ValidatorSet + LastHeightChanged int +} + +// Bytes serializes the Validators using go-wire +func (v *Validators) Bytes() []byte { + return wire.BinaryBytes(*v) +} + //----------------------------------------------------------------------------- // Genesis @@ -260,15 +346,16 @@ func MakeGenesisState(db dbm.DB, genDoc *types.GenesisDoc) *State { } return &State{ - db: db, - GenesisDoc: genDoc, - ChainID: genDoc.ChainID, - LastBlockHeight: 0, - LastBlockID: types.BlockID{}, - LastBlockTime: genDoc.GenesisTime, - Validators: types.NewValidatorSet(validators), - LastValidators: types.NewValidatorSet(nil), - AppHash: genDoc.AppHash, - TxIndexer: &null.TxIndex{}, // we do not need indexer during replay and in tests + db: db, + GenesisDoc: genDoc, + ChainID: genDoc.ChainID, + LastBlockHeight: 0, + LastBlockID: types.BlockID{}, + LastBlockTime: genDoc.GenesisTime, + Validators: types.NewValidatorSet(validators), + LastValidators: types.NewValidatorSet(nil), + AppHash: genDoc.AppHash, + TxIndexer: &null.TxIndex{}, // we do not need indexer during replay and in tests + lastHeightValidatorsChanged: 1, } } diff --git a/state/state_test.go b/state/state_test.go index e97c3289a..22543cc17 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -1,18 +1,21 @@ package state import ( - "fmt" "testing" "github.com/stretchr/testify/assert" + abci "github.com/tendermint/abci/types" crypto "github.com/tendermint/go-crypto" - cfg "github.com/tendermint/tendermint/config" + cmn "github.com/tendermint/tmlibs/common" dbm "github.com/tendermint/tmlibs/db" "github.com/tendermint/tmlibs/log" + + cfg "github.com/tendermint/tendermint/config" ) func TestStateCopyEquals(t *testing.T) { + assert := assert.New(t) config := cfg.ResetTestRoot("state_") // Get State db @@ -22,18 +25,13 @@ func TestStateCopyEquals(t *testing.T) { stateCopy := state.Copy() - if !state.Equals(stateCopy) { - t.Fatal("expected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state) - } - + assert.True(state.Equals(stateCopy), cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", stateCopy, state)) stateCopy.LastBlockHeight += 1 - - if state.Equals(stateCopy) { - t.Fatal("expected states to be different. got same %v", state) - } + assert.False(state.Equals(stateCopy), cmn.Fmt("expected states to be different. got same %v", state)) } func TestStateSaveLoad(t *testing.T) { + assert := assert.New(t) config := cfg.ResetTestRoot("state_") // Get State db stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) @@ -44,9 +42,7 @@ func TestStateSaveLoad(t *testing.T) { state.Save() loadedState := LoadState(stateDB) - if !state.Equals(loadedState) { - t.Fatal("expected state and its copy to be identical. got %v\n expected %v\n", loadedState, state) - } + assert.True(state.Equals(loadedState), cmn.Fmt("expected state and its copy to be identical. got %v\n expected %v\n", loadedState, state)) } func TestABCIResponsesSaveLoad(t *testing.T) { @@ -74,5 +70,41 @@ func TestABCIResponsesSaveLoad(t *testing.T) { state.SaveABCIResponses(abciResponses) abciResponses2 := state.LoadABCIResponses() - assert.Equal(abciResponses, abciResponses2, fmt.Sprintf("ABCIResponses don't match: Got %v, Expected %v", abciResponses2, abciResponses)) + assert.Equal(abciResponses, abciResponses2, cmn.Fmt("ABCIResponses don't match: Got %v, Expected %v", abciResponses2, abciResponses)) +} + +func TestValidatorsSaveLoad(t *testing.T) { + assert := assert.New(t) + config := cfg.ResetTestRoot("state_") + // Get State db + stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) + state := GetState(stateDB, config.GenesisFile()) + state.SetLogger(log.TestingLogger()) + + // cant load anything for height 0 + v, err := state.LoadValidators(0) + assert.NotNil(err, "expected err at height 0") + + // should be able to load for height 1 + v, err = state.LoadValidators(1) + assert.Nil(err, "expected no err at height 1") + assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") + + // increment height, save; should be able to load for next height + state.LastBlockHeight += 1 + state.SaveValidators() + v, err = state.LoadValidators(state.LastBlockHeight + 1) + assert.Nil(err, "expected no err") + assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") + + // increment height, save; should be able to load for next height + state.LastBlockHeight += 10 + state.SaveValidators() + v, err = state.LoadValidators(state.LastBlockHeight + 1) + assert.Nil(err, "expected no err") + assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") + + // should be able to load for next next height + _, err = state.LoadValidators(state.LastBlockHeight + 2) + assert.NotNil(err, "expected err") } From e2e87460444ba0c8097c416da9b91734f74af498 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 21 Aug 2017 18:11:16 -0400 Subject: [PATCH 2/5] rpc: historical validators --- rpc/client/httpclient.go | 6 ++++-- rpc/client/interface.go | 2 +- rpc/client/localclient.go | 4 ++-- rpc/client/mock/client.go | 4 ++-- rpc/client/rpc_test.go | 2 +- rpc/core/blocks.go | 4 +++- rpc/core/consensus.go | 17 +++++++++++++---- rpc/core/pipe.go | 5 ++++- rpc/core/routes.go | 2 +- 9 files changed, 31 insertions(+), 15 deletions(-) diff --git a/rpc/client/httpclient.go b/rpc/client/httpclient.go index 9623229e6..4e54cbf7d 100644 --- a/rpc/client/httpclient.go +++ b/rpc/client/httpclient.go @@ -174,9 +174,11 @@ func (c *HTTP) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { return result, nil } -func (c *HTTP) Validators() (*ctypes.ResultValidators, error) { +func (c *HTTP) Validators(height *int) (*ctypes.ResultValidators, error) { result := new(ctypes.ResultValidators) - _, err := c.rpc.Call("validators", map[string]interface{}{}, result) + _, err := c.rpc.Call("validators", map[string]interface{}{ + "height": height, + }, result) if err != nil { return nil, errors.Wrap(err, "Validators") } diff --git a/rpc/client/interface.go b/rpc/client/interface.go index 0cd0f29bb..8845e5e5a 100644 --- a/rpc/client/interface.go +++ b/rpc/client/interface.go @@ -44,7 +44,7 @@ type ABCIClient interface { type SignClient interface { Block(height int) (*ctypes.ResultBlock, error) Commit(height int) (*ctypes.ResultCommit, error) - Validators() (*ctypes.ResultValidators, error) + Validators(height *int) (*ctypes.ResultValidators, error) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) } diff --git a/rpc/client/localclient.go b/rpc/client/localclient.go index f4eb00d78..0d8500e1e 100644 --- a/rpc/client/localclient.go +++ b/rpc/client/localclient.go @@ -101,8 +101,8 @@ func (c Local) Commit(height int) (*ctypes.ResultCommit, error) { return core.Commit(height) } -func (c Local) Validators() (*ctypes.ResultValidators, error) { - return core.Validators() +func (c Local) Validators(height *int) (*ctypes.ResultValidators, error) { + return core.Validators(height) } func (c Local) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { diff --git a/rpc/client/mock/client.go b/rpc/client/mock/client.go index bf8d78dce..be61b7af3 100644 --- a/rpc/client/mock/client.go +++ b/rpc/client/mock/client.go @@ -124,6 +124,6 @@ func (c Client) Commit(height int) (*ctypes.ResultCommit, error) { return core.Commit(height) } -func (c Client) Validators() (*ctypes.ResultValidators, error) { - return core.Validators() +func (c Client) Validators(height *int) (*ctypes.ResultValidators, error) { + return core.Validators(height) } diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index e82f8df42..304d2b906 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -87,7 +87,7 @@ func TestGenesisAndValidators(t *testing.T) { gval := gen.Genesis.Validators[0] // get the current validators - vals, err := c.Validators() + vals, err := c.Validators(nil) require.Nil(t, err, "%d: %+v", i, err) require.Equal(t, 1, len(vals.Validators)) val := vals.Validators[0] diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go index 1f9e85422..5603cdd22 100644 --- a/rpc/core/blocks.go +++ b/rpc/core/blocks.go @@ -85,6 +85,7 @@ func BlockchainInfo(minHeight, maxHeight int) (*ctypes.ResultBlockchainInfo, err } // Get block at a given height. +// If no height is provided, it will fetch the latest block. // // ```shell // curl 'localhost:46657/block?height=10' @@ -196,7 +197,8 @@ func Block(height int) (*ctypes.ResultBlock, error) { return &ctypes.ResultBlock{blockMeta, block}, nil } -// Get block commit at a given height. +// Get block commit at a given height. If the height is left out, it +// If no height is provided, it will fetch the commit for the latest block. // // ```shell // curl 'localhost:46657/commit?height=11' diff --git a/rpc/core/consensus.go b/rpc/core/consensus.go index fdce29e48..55596ec26 100644 --- a/rpc/core/consensus.go +++ b/rpc/core/consensus.go @@ -7,7 +7,8 @@ import ( "github.com/tendermint/tendermint/types" ) -// Get current validators set along with a block height. +// Get the validator set at a give block height. +// If no height is provided, it will fetch the current validator set. // // ```shell // curl 'localhost:46657/validators' @@ -41,9 +42,17 @@ import ( // "jsonrpc": "2.0" // } // ``` -func Validators() (*ctypes.ResultValidators, error) { - blockHeight, validators := consensusState.GetValidators() - return &ctypes.ResultValidators{blockHeight, validators}, nil +func Validators(height *int) (*ctypes.ResultValidators, error) { + if height == nil { + blockHeight, validators := consensusState.GetValidators() + return &ctypes.ResultValidators{blockHeight, validators}, nil + } + state := consensusState.GetState() + validators, err := state.LoadValidators(*height) + if err != nil { + return nil, err + } + return &ctypes.ResultValidators{*height, validators.Validators}, nil } // Dump consensus state. diff --git a/rpc/core/pipe.go b/rpc/core/pipe.go index 92e5746a6..86c6c65e1 100644 --- a/rpc/core/pipe.go +++ b/rpc/core/pipe.go @@ -2,18 +2,21 @@ package core import ( crypto "github.com/tendermint/go-crypto" + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/tendermint/consensus" p2p "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/proxy" + sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/state/txindex" "github.com/tendermint/tendermint/types" - "github.com/tendermint/tmlibs/log" ) //---------------------------------------------- // These interfaces are used by RPC and must be thread safe type Consensus interface { + GetState() *sm.State GetValidators() (int, []*types.Validator) GetRoundState() *consensus.RoundState } diff --git a/rpc/core/routes.go b/rpc/core/routes.go index 795acde9b..485f7a00f 100644 --- a/rpc/core/routes.go +++ b/rpc/core/routes.go @@ -18,7 +18,7 @@ var Routes = map[string]*rpc.RPCFunc{ "block": rpc.NewRPCFunc(Block, "height"), "commit": rpc.NewRPCFunc(Commit, "height"), "tx": rpc.NewRPCFunc(Tx, "hash,prove"), - "validators": rpc.NewRPCFunc(Validators, ""), + "validators": rpc.NewRPCFunc(Validators, "height"), "dump_consensus_state": rpc.NewRPCFunc(DumpConsensusState, ""), "unconfirmed_txs": rpc.NewRPCFunc(UnconfirmedTxs, ""), "num_unconfirmed_txs": rpc.NewRPCFunc(NumUnconfirmedTxs, ""), From f0f1ebe013ac4bae2b8769b9ff66704145f923ec Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 22 Aug 2017 17:42:23 -0400 Subject: [PATCH 3/5] rpc: Block and Commit take pointers; return latest on nil --- rpc/client/httpclient.go | 8 +++----- rpc/client/interface.go | 4 ++-- rpc/client/localclient.go | 4 ++-- rpc/client/mock/client.go | 4 ++-- rpc/client/rpc_test.go | 10 ++++++---- rpc/core/blocks.go | 24 ++++++++++++++++++++---- rpc/core/consensus.go | 10 ++++++---- 7 files changed, 41 insertions(+), 23 deletions(-) diff --git a/rpc/client/httpclient.go b/rpc/client/httpclient.go index 4e54cbf7d..7f29183d5 100644 --- a/rpc/client/httpclient.go +++ b/rpc/client/httpclient.go @@ -143,7 +143,7 @@ func (c *HTTP) Genesis() (*ctypes.ResultGenesis, error) { return result, nil } -func (c *HTTP) Block(height int) (*ctypes.ResultBlock, error) { +func (c *HTTP) Block(height *int) (*ctypes.ResultBlock, error) { result := new(ctypes.ResultBlock) _, err := c.rpc.Call("block", map[string]interface{}{"height": height}, result) if err != nil { @@ -152,7 +152,7 @@ func (c *HTTP) Block(height int) (*ctypes.ResultBlock, error) { return result, nil } -func (c *HTTP) Commit(height int) (*ctypes.ResultCommit, error) { +func (c *HTTP) Commit(height *int) (*ctypes.ResultCommit, error) { result := new(ctypes.ResultCommit) _, err := c.rpc.Call("commit", map[string]interface{}{"height": height}, result) if err != nil { @@ -176,9 +176,7 @@ func (c *HTTP) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) { func (c *HTTP) Validators(height *int) (*ctypes.ResultValidators, error) { result := new(ctypes.ResultValidators) - _, err := c.rpc.Call("validators", map[string]interface{}{ - "height": height, - }, result) + _, err := c.rpc.Call("validators", map[string]interface{}{"height": height}, result) if err != nil { return nil, errors.Wrap(err, "Validators") } diff --git a/rpc/client/interface.go b/rpc/client/interface.go index 8845e5e5a..ed7ccabaf 100644 --- a/rpc/client/interface.go +++ b/rpc/client/interface.go @@ -42,8 +42,8 @@ type ABCIClient interface { // SignClient groups together the interfaces need to get valid // signatures and prove anything about the chain type SignClient interface { - Block(height int) (*ctypes.ResultBlock, error) - Commit(height int) (*ctypes.ResultCommit, error) + Block(height *int) (*ctypes.ResultBlock, error) + Commit(height *int) (*ctypes.ResultCommit, error) Validators(height *int) (*ctypes.ResultValidators, error) Tx(hash []byte, prove bool) (*ctypes.ResultTx, error) } diff --git a/rpc/client/localclient.go b/rpc/client/localclient.go index 0d8500e1e..134f935ca 100644 --- a/rpc/client/localclient.go +++ b/rpc/client/localclient.go @@ -93,11 +93,11 @@ func (c Local) Genesis() (*ctypes.ResultGenesis, error) { return core.Genesis() } -func (c Local) Block(height int) (*ctypes.ResultBlock, error) { +func (c Local) Block(height *int) (*ctypes.ResultBlock, error) { return core.Block(height) } -func (c Local) Commit(height int) (*ctypes.ResultCommit, error) { +func (c Local) Commit(height *int) (*ctypes.ResultCommit, error) { return core.Commit(height) } diff --git a/rpc/client/mock/client.go b/rpc/client/mock/client.go index be61b7af3..f32694edd 100644 --- a/rpc/client/mock/client.go +++ b/rpc/client/mock/client.go @@ -116,11 +116,11 @@ func (c Client) Genesis() (*ctypes.ResultGenesis, error) { return core.Genesis() } -func (c Client) Block(height int) (*ctypes.ResultBlock, error) { +func (c Client) Block(height *int) (*ctypes.ResultBlock, error) { return core.Block(height) } -func (c Client) Commit(height int) (*ctypes.ResultCommit, error) { +func (c Client) Commit(height *int) (*ctypes.ResultCommit, error) { return core.Commit(height) } diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index 304d2b906..fee92b4a9 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -110,7 +110,8 @@ func TestAppCalls(t *testing.T) { sh := s.LatestBlockHeight // look for the future - _, err = c.Block(sh + 2) + h := sh + 2 + _, err = c.Block(&h) assert.NotNil(err) // no block yet // write something @@ -137,7 +138,7 @@ func TestAppCalls(t *testing.T) { assert.EqualValues(tx, ptx.Tx) // and we can even check the block is added - block, err := c.Block(apph) + block, err := c.Block(&apph) require.Nil(err, "%d: %+v", i, err) appHash := block.BlockMeta.Header.AppHash assert.True(len(appHash) > 0) @@ -158,14 +159,15 @@ func TestAppCalls(t *testing.T) { } // and get the corresponding commit with the same apphash - commit, err := c.Commit(apph) + commit, err := c.Commit(&apph) require.Nil(err, "%d: %+v", i, err) cappHash := commit.Header.AppHash assert.Equal(appHash, cappHash) assert.NotNil(commit.Commit) // compare the commits (note Commit(2) has commit from Block(3)) - commit2, err := c.Commit(apph - 1) + h = apph - 1 + commit2, err := c.Commit(&h) require.Nil(err, "%d: %+v", i, err) assert.Equal(block.Block.LastCommit, commit2.Commit) diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go index 5603cdd22..aa22da0f4 100644 --- a/rpc/core/blocks.go +++ b/rpc/core/blocks.go @@ -184,8 +184,16 @@ func BlockchainInfo(minHeight, maxHeight int) (*ctypes.ResultBlockchainInfo, err // "jsonrpc": "2.0" // } // ``` -func Block(height int) (*ctypes.ResultBlock, error) { - if height == 0 { +func Block(heightPtr *int) (*ctypes.ResultBlock, error) { + if heightPtr == nil { + height := blockStore.Height() + blockMeta := blockStore.LoadBlockMeta(height) + block := blockStore.LoadBlock(height) + return &ctypes.ResultBlock{blockMeta, block}, nil + } + + height := *heightPtr + if height <= 0 { return nil, fmt.Errorf("Height must be greater than 0") } if height > blockStore.Height() { @@ -267,8 +275,16 @@ func Block(height int) (*ctypes.ResultBlock, error) { // "jsonrpc": "2.0" // } // ``` -func Commit(height int) (*ctypes.ResultCommit, error) { - if height == 0 { +func Commit(heightPtr *int) (*ctypes.ResultCommit, error) { + if heightPtr == nil { + height := blockStore.Height() + header := blockStore.LoadBlockMeta(height).Header + commit := blockStore.LoadSeenCommit(height) + return &ctypes.ResultCommit{header, commit, false}, nil + } + + height := *heightPtr + if height <= 0 { return nil, fmt.Errorf("Height must be greater than 0") } storeHeight := blockStore.Height() diff --git a/rpc/core/consensus.go b/rpc/core/consensus.go index 55596ec26..ca50ff595 100644 --- a/rpc/core/consensus.go +++ b/rpc/core/consensus.go @@ -42,17 +42,19 @@ import ( // "jsonrpc": "2.0" // } // ``` -func Validators(height *int) (*ctypes.ResultValidators, error) { - if height == nil { +func Validators(heightPtr *int) (*ctypes.ResultValidators, error) { + if heightPtr == nil { blockHeight, validators := consensusState.GetValidators() return &ctypes.ResultValidators{blockHeight, validators}, nil } + + height := *heightPtr state := consensusState.GetState() - validators, err := state.LoadValidators(*height) + validators, err := state.LoadValidators(height) if err != nil { return nil, err } - return &ctypes.ResultValidators{*height, validators.Validators}, nil + return &ctypes.ResultValidators{height, validators.Validators}, nil } // Dump consensus state. From 9deb6473034e44ebf6022b95570ad9f51d27768c Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 4 Sep 2017 18:27:04 -0400 Subject: [PATCH 4/5] fixes from review --- rpc/core/blocks.go | 2 +- rpc/core/consensus.go | 2 +- state/execution.go | 5 +-- state/state.go | 87 +++++++++++++++++++------------------------ state/state_test.go | 8 ++-- 5 files changed, 46 insertions(+), 58 deletions(-) diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go index aa22da0f4..17b27303f 100644 --- a/rpc/core/blocks.go +++ b/rpc/core/blocks.go @@ -205,7 +205,7 @@ func Block(heightPtr *int) (*ctypes.ResultBlock, error) { return &ctypes.ResultBlock{blockMeta, block}, nil } -// Get block commit at a given height. If the height is left out, it +// Get block commit at a given height. // If no height is provided, it will fetch the commit for the latest block. // // ```shell diff --git a/rpc/core/consensus.go b/rpc/core/consensus.go index ca50ff595..848288447 100644 --- a/rpc/core/consensus.go +++ b/rpc/core/consensus.go @@ -7,7 +7,7 @@ import ( "github.com/tendermint/tendermint/types" ) -// Get the validator set at a give block height. +// Get the validator set at the given block height. // If no height is provided, it will fetch the current validator set. // // ```shell diff --git a/state/execution.go b/state/execution.go index 869d28d48..1785f6025 100644 --- a/state/execution.go +++ b/state/execution.go @@ -231,9 +231,6 @@ func (s *State) ApplyBlock(eventCache types.Fireable, proxyAppConn proxy.AppConn // now update the block and validators s.SetBlockAndValidators(block.Header, partsHeader, abciResponses) - // save the validators for the next block now that we know them - s.SaveValidators() - // lock mempool, commit state, update mempoool err = s.CommitStateUpdateMempool(proxyAppConn, block, mempool) if err != nil { @@ -242,7 +239,7 @@ func (s *State) ApplyBlock(eventCache types.Fireable, proxyAppConn proxy.AppConn fail.Fail() // XXX - // save the state + // save the state and the validators s.Save() return nil diff --git a/state/state.go b/state/state.go index 6106891b5..f2b02309f 100644 --- a/state/state.go +++ b/state/state.go @@ -51,10 +51,9 @@ type State struct { // AppHash is updated after Commit AppHash []byte - // XXX: do we need this json tag ? TxIndexer txindex.TxIndexer `json:"-"` // Transaction indexer. - lastHeightValidatorsChanged int + LastHeightValidatorsChanged int logger log.Logger } @@ -79,13 +78,6 @@ func loadState(db dbm.DB, key []byte) *State { // TODO: ensure that buf is completely read. } - v := s.loadValidators(s.LastBlockHeight) - if v != nil { - s.lastHeightValidatorsChanged = v.LastHeightChanged - } else { - s.lastHeightValidatorsChanged = 1 - } - return s } @@ -97,19 +89,18 @@ func (s *State) SetLogger(l log.Logger) { // Copy makes a copy of the State for mutating. func (s *State) Copy() *State { return &State{ - db: s.db, - GenesisDoc: s.GenesisDoc, - ChainID: s.ChainID, - LastBlockHeight: s.LastBlockHeight, - LastBlockID: s.LastBlockID, - LastBlockTime: s.LastBlockTime, - Validators: s.Validators.Copy(), - LastValidators: s.LastValidators.Copy(), - AppHash: s.AppHash, - TxIndexer: s.TxIndexer, // pointer here, not value - logger: s.logger, - - lastHeightValidatorsChanged: s.lastHeightValidatorsChanged, + db: s.db, + GenesisDoc: s.GenesisDoc, + ChainID: s.ChainID, + LastBlockHeight: s.LastBlockHeight, + LastBlockID: s.LastBlockID, + LastBlockTime: s.LastBlockTime, + Validators: s.Validators.Copy(), + LastValidators: s.LastValidators.Copy(), + AppHash: s.AppHash, + TxIndexer: s.TxIndexer, // pointer here, not value + LastHeightValidatorsChanged: s.LastHeightValidatorsChanged, + logger: s.logger, } } @@ -117,6 +108,7 @@ func (s *State) Copy() *State { func (s *State) Save() { s.mtx.Lock() defer s.mtx.Unlock() + s.saveValidatorsInfo() s.db.SetSync(stateKey, s.Bytes()) } @@ -143,22 +135,6 @@ func (s *State) LoadABCIResponses() *ABCIResponses { return abciResponses } -// SaveValidators persists the validator set for the next block to disk. -// It should be called after the validator set is updated with the results of EndBlock. -// If the validator set did not change after processing the latest block, -// only the last height for which the validators changed is persisted. -func (s *State) SaveValidators() { - lastHeight := s.lastHeightValidatorsChanged - nextHeight := s.LastBlockHeight + 1 - v := &Validators{ - LastHeightChanged: lastHeight, - } - if lastHeight == nextHeight { - v.ValidatorSet = s.Validators - } - s.db.SetSync(calcValidatorsKey(nextHeight), v.Bytes()) -} - // LoadValidators loads the ValidatorSet for a given height. func (s *State) LoadValidators(height int) (*types.ValidatorSet, error) { v := s.loadValidators(height) @@ -176,13 +152,13 @@ func (s *State) LoadValidators(height int) (*types.ValidatorSet, error) { return v.ValidatorSet, nil } -func (s *State) loadValidators(height int) *Validators { +func (s *State) loadValidators(height int) *ValidatorsInfo { buf := s.db.Get(calcValidatorsKey(height)) if len(buf) == 0 { return nil } - v := new(Validators) + v := new(ValidatorsInfo) r, n, err := bytes.NewReader(buf), new(int), new(error) wire.ReadBinaryPtr(v, r, 0, n, err) if *err != nil { @@ -193,6 +169,22 @@ func (s *State) loadValidators(height int) *Validators { return v } +// saveValidatorsInfo persists the validator set for the next block to disk. +// It should be called after the validator set is updated with the results of EndBlock. +// If the validator set did not change after processing the latest block, +// only the last height for which the validators changed is persisted. +func (s *State) saveValidatorsInfo() { + changeHeight := s.LastHeightValidatorsChanged + nextHeight := s.LastBlockHeight + 1 + vi := &ValidatorsInfo{ + LastHeightChanged: changeHeight, + } + if changeHeight == nextHeight { + vi.ValidatorSet = s.Validators + } + s.db.SetSync(calcValidatorsKey(nextHeight), vi.Bytes()) +} + // Equals returns true if the States are identical. func (s *State) Equals(s2 *State) bool { return bytes.Equal(s.Bytes(), s2.Bytes()) @@ -219,7 +211,7 @@ func (s *State) SetBlockAndValidators(header *types.Header, blockPartsHeader typ // TODO: err or carry on? } // change results from this height but only applies to the next height - s.lastHeightValidatorsChanged = header.Height + 1 + s.LastHeightValidatorsChanged = header.Height + 1 } // Update validator accums and set state variables @@ -254,7 +246,6 @@ func GetState(stateDB dbm.DB, genesisFile string) *State { state := LoadState(stateDB) if state == nil { state = MakeGenesisStateFromFile(stateDB, genesisFile) - state.SaveValidators() // save the validators right away for height 1 state.Save() } @@ -290,15 +281,15 @@ func (a *ABCIResponses) Bytes() []byte { //----------------------------------------------------------------------------- -// Validators represents the latest validator set, or the last time it changed -type Validators struct { +// ValidatorsInfo represents the latest validator set, or the last time it changed +type ValidatorsInfo struct { ValidatorSet *types.ValidatorSet LastHeightChanged int } -// Bytes serializes the Validators using go-wire -func (v *Validators) Bytes() []byte { - return wire.BinaryBytes(*v) +// Bytes serializes the ValidatorsInfo using go-wire +func (vi *ValidatorsInfo) Bytes() []byte { + return wire.BinaryBytes(*vi) } //----------------------------------------------------------------------------- @@ -356,6 +347,6 @@ func MakeGenesisState(db dbm.DB, genDoc *types.GenesisDoc) *State { LastValidators: types.NewValidatorSet(nil), AppHash: genDoc.AppHash, TxIndexer: &null.TxIndex{}, // we do not need indexer during replay and in tests - lastHeightValidatorsChanged: 1, + LastHeightValidatorsChanged: 1, } } diff --git a/state/state_test.go b/state/state_test.go index 22543cc17..ddcb98c22 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -83,7 +83,7 @@ func TestValidatorsSaveLoad(t *testing.T) { // cant load anything for height 0 v, err := state.LoadValidators(0) - assert.NotNil(err, "expected err at height 0") + assert.IsType(ErrNoValSetForHeight{}, err, "expected err at height 0") // should be able to load for height 1 v, err = state.LoadValidators(1) @@ -92,19 +92,19 @@ func TestValidatorsSaveLoad(t *testing.T) { // increment height, save; should be able to load for next height state.LastBlockHeight += 1 - state.SaveValidators() + state.saveValidatorsInfo() v, err = state.LoadValidators(state.LastBlockHeight + 1) assert.Nil(err, "expected no err") assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") // increment height, save; should be able to load for next height state.LastBlockHeight += 10 - state.SaveValidators() + state.saveValidatorsInfo() v, err = state.LoadValidators(state.LastBlockHeight + 1) assert.Nil(err, "expected no err") assert.Equal(v.Hash(), state.Validators.Hash(), "expected validator hashes to match") // should be able to load for next next height _, err = state.LoadValidators(state.LastBlockHeight + 2) - assert.NotNil(err, "expected err") + assert.IsType(ErrNoValSetForHeight{}, err, "expected err at unknown height") } From fae0603413e39873e04eae0abda16f812e34a8aa Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 5 Sep 2017 21:57:36 -0400 Subject: [PATCH 5/5] more fixes from review --- state/state.go | 10 ++++- state/state_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/state/state.go b/state/state.go index f2b02309f..72c3e64da 100644 --- a/state/state.go +++ b/state/state.go @@ -2,6 +2,7 @@ package state import ( "bytes" + "fmt" "io/ioutil" "sync" "time" @@ -53,6 +54,10 @@ type State struct { TxIndexer txindex.TxIndexer `json:"-"` // Transaction indexer. + // When a block returns a validator set change via EndBlock, + // the change only applies to the next block. + // So, if s.LastBlockHeight causes a valset change, + // we set s.LastHeightValidatorsChanged = s.LastBlockHeight + 1 LastHeightValidatorsChanged int logger log.Logger @@ -145,7 +150,8 @@ func (s *State) LoadValidators(height int) (*types.ValidatorSet, error) { if v.ValidatorSet == nil { v = s.loadValidators(v.LastHeightChanged) if v == nil { - return nil, ErrNoValSetForHeight{height} + cmn.PanicSanity(fmt.Sprintf(`Couldn't find validators at + height %d as last changed from height %d`, v.LastHeightChanged, height)) } } @@ -170,7 +176,7 @@ func (s *State) loadValidators(height int) *ValidatorsInfo { } // saveValidatorsInfo persists the validator set for the next block to disk. -// It should be called after the validator set is updated with the results of EndBlock. +// 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 (s *State) saveValidatorsInfo() { diff --git a/state/state_test.go b/state/state_test.go index ddcb98c22..713d76f80 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -1,6 +1,8 @@ package state import ( + "bytes" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -12,6 +14,7 @@ import ( "github.com/tendermint/tmlibs/log" cfg "github.com/tendermint/tendermint/config" + "github.com/tendermint/tendermint/types" ) func TestStateCopyEquals(t *testing.T) { @@ -73,7 +76,7 @@ func TestABCIResponsesSaveLoad(t *testing.T) { assert.Equal(abciResponses, abciResponses2, cmn.Fmt("ABCIResponses don't match: Got %v, Expected %v", abciResponses2, abciResponses)) } -func TestValidatorsSaveLoad(t *testing.T) { +func TestValidatorSimpleSaveLoad(t *testing.T) { assert := assert.New(t) config := cfg.ResetTestRoot("state_") // Get State db @@ -108,3 +111,89 @@ func TestValidatorsSaveLoad(t *testing.T) { _, err = state.LoadValidators(state.LastBlockHeight + 2) assert.IsType(ErrNoValSetForHeight{}, err, "expected err at unknown height") } + +func TestValidatorChangesSaveLoad(t *testing.T) { + assert := assert.New(t) + config := cfg.ResetTestRoot("state_") + // Get State db + stateDB := dbm.NewDB("state", config.DBBackend, config.DBDir()) + state := GetState(stateDB, config.GenesisFile()) + state.SetLogger(log.TestingLogger()) + + // change vals at these heights + changeHeights := []int{1, 2, 4, 5, 10, 15, 16, 17, 20} + N := len(changeHeights) + + // each valset is just one validator. + // create list of them + pubkeys := make([]crypto.PubKey, N+1) + pubkeys[0] = state.GenesisDoc.Validators[0].PubKey + for i := 1; i < N+1; i++ { + pubkeys[i] = crypto.GenPrivKeyEd25519().PubKey() + } + + // 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 := 1; i < highestHeight; i++ { + // when we get to a change height, + // use the next pubkey + if changeIndex < len(changeHeights) && i == changeHeights[changeIndex] { + changeIndex += 1 + 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 := 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 += 1 + pubkey = pubkeys[changeIndex] + } + testCases[i-1] = valChangeTestCase{i, pubkey} + } + + 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 makeHeaderPartsResponses(state *State, height int, pubkey crypto.PubKey) (*types.Header, types.PartSetHeader, *ABCIResponses) { + block := makeBlock(height, state) + _, val := state.Validators.GetByIndex(0) + abciResponses := &ABCIResponses{ + Height: height, + } + + // if the pubkey is new, remove the old and add the new + if !bytes.Equal(pubkey.Bytes(), val.PubKey.Bytes()) { + abciResponses.EndBlock = abci.ResponseEndBlock{ + Diffs: []*abci.Validator{ + {val.PubKey.Bytes(), 0}, + {pubkey.Bytes(), 10}, + }, + } + } + + return block.Header, types.PartSetHeader{}, abciResponses +} + +type valChangeTestCase struct { + height int + vals crypto.PubKey +}