From 7939d62ef0706ea0affac1b5e3a5c4ee90c412be Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Fri, 22 Sep 2017 18:17:21 -0600 Subject: [PATCH] all, state: unexpose GenesisDoc, ChainID fields make them accessor methods Fixes #671 Unexpose GenesisDoc and ChainID fields to avoid them being serialized to the DB on every block write/state.Save() A GenesisDoc can now be alternatively written to the state's database, by serializing its JSON as a value of key "genesis-doc". There are now accessors and a setter for these attributes: - state.GenesisDoc() (*types.GenesisDoc, error) - state.ChainID() (string, error) - state.SetGenesisDoc(*types.GenesisDoc) This is a breaking change since it changes how the state's serialization and requires that if loading the GenesisDoc entirely from the database, you'll need to set its value in the database as the GenesisDoc's JSON marshaled bytes. --- blockchain/reactor.go | 10 +++- blockchain/store.go | 4 +- consensus/state.go | 40 ++++++++++++--- node/node.go | 12 ++++- state/execution.go | 8 ++- state/state.go | 73 +++++++++++++++++++++++----- state/state_test.go | 110 +++++++++++++++++++++++++++++++++++++++++- 7 files changed, 229 insertions(+), 28 deletions(-) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 91f0aded..3001206f 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -242,8 +242,14 @@ FOR_LOOP: // NOTE: we can probably make this more efficient, but note that calling // first.Hash() doesn't verify the tx contents, so MakePartSet() is // currently necessary. - err := bcR.state.Validators.VerifyCommit( - bcR.state.ChainID, types.BlockID{first.Hash(), firstPartsHeader}, first.Height, second.LastCommit) + chainID, err := bcR.state.ChainID() + if err != nil { + bcR.Logger.Info("error in retrieving chainID", "err", err) + bcR.pool.RedoRequest(first.Height) + break SYNC_LOOP + } + err = bcR.state.Validators.VerifyCommit( + chainID, types.BlockID{first.Hash(), firstPartsHeader}, first.Height, second.LastCommit) if err != nil { bcR.Logger.Error("Error in validation", "err", err) bcR.pool.RedoRequest(first.Height) diff --git a/blockchain/store.go b/blockchain/store.go index a96aa0fb..a37fda93 100644 --- a/blockchain/store.go +++ b/blockchain/store.go @@ -7,10 +7,10 @@ import ( "io" "sync" - . "github.com/tendermint/tmlibs/common" - dbm "github.com/tendermint/tmlibs/db" "github.com/tendermint/go-wire" "github.com/tendermint/tendermint/types" + . "github.com/tendermint/tmlibs/common" + dbm "github.com/tendermint/tmlibs/db" ) /* diff --git a/consensus/state.go b/consensus/state.go index f0fbad81..ac457f49 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -389,8 +389,12 @@ func (cs *ConsensusState) reconstructLastCommit(state *sm.State) { if state.LastBlockHeight == 0 { return } + chainID, err := cs.state.ChainID() + if err != nil { + cmn.PanicCrisis(cmn.Fmt("Failed to retrieve ChainID: %v", err)) + } seenCommit := cs.blockStore.LoadSeenCommit(state.LastBlockHeight) - lastPrecommits := types.NewVoteSet(cs.state.ChainID, state.LastBlockHeight, seenCommit.Round(), types.VoteTypePrecommit, state.LastValidators) + lastPrecommits := types.NewVoteSet(chainID, state.LastBlockHeight, seenCommit.Round(), types.VoteTypePrecommit, state.LastValidators) for _, precommit := range seenCommit.Precommits { if precommit == nil { continue @@ -720,7 +724,11 @@ func (cs *ConsensusState) proposalHeartbeat(height, round int) { ValidatorAddress: addr, ValidatorIndex: valIndex, } - cs.privValidator.SignHeartbeat(cs.state.ChainID, heartbeat) + chainID, err := cs.state.ChainID() + if err != nil { + return + } + cs.privValidator.SignHeartbeat(chainID, heartbeat) heartbeatEvent := types.EventDataProposalHeartbeat{heartbeat} types.FireEventProposalHeartbeat(cs.evsw, heartbeatEvent) counter += 1 @@ -797,8 +805,11 @@ func (cs *ConsensusState) defaultDecideProposal(height, round int) { // Make proposal polRound, polBlockID := cs.Votes.POLInfo() proposal := types.NewProposal(height, round, blockParts.Header(), polRound, polBlockID) - err := cs.privValidator.SignProposal(cs.state.ChainID, proposal) - if err == nil { + chainID, err := cs.state.ChainID() + if err != nil { + return + } + if err := cs.privValidator.SignProposal(chainID, proposal); err == nil { // Set fields /* fields set by setProposal and addBlockPart cs.Proposal = proposal @@ -857,8 +868,12 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts // Mempool validated transactions txs := cs.mempool.Reap(cs.config.MaxBlockSizeTxs) - - return types.MakeBlock(cs.Height, cs.state.ChainID, txs, commit, + chainID, err := cs.state.ChainID() + if err != nil { + cs.Logger.Error("chainID err", err) + return + } + return types.MakeBlock(cs.Height, chainID, txs, commit, cs.state.LastBlockID, cs.state.Validators.Hash(), cs.state.AppHash, cs.state.Params().BlockPartSizeBytes) } @@ -1263,8 +1278,13 @@ func (cs *ConsensusState) defaultSetProposal(proposal *types.Proposal) error { return ErrInvalidProposalPOLRound } + chainID, err := cs.state.ChainID() + if err != nil { + return err + } + // Verify signature - if !cs.Validators.GetProposer().PubKey.VerifyBytes(types.SignBytes(cs.state.ChainID, proposal), proposal.Signature) { + if !cs.Validators.GetProposer().PubKey.VerifyBytes(types.SignBytes(chainID, proposal), proposal.Signature) { return ErrInvalidProposalSignature } @@ -1449,6 +1469,10 @@ func (cs *ConsensusState) addVote(vote *types.Vote, peerKey string) (added bool, } func (cs *ConsensusState) signVote(type_ byte, hash []byte, header types.PartSetHeader) (*types.Vote, error) { + chainID, err := cs.state.ChainID() + if err != nil { + return nil, err + } addr := cs.privValidator.GetAddress() valIndex, _ := cs.Validators.GetByAddress(addr) vote := &types.Vote{ @@ -1459,7 +1483,7 @@ func (cs *ConsensusState) signVote(type_ byte, hash []byte, header types.PartSet Type: type_, BlockID: types.BlockID{hash, header}, } - err := cs.privValidator.SignVote(cs.state.ChainID, vote) + err = cs.privValidator.SignVote(chainID, vote) return vote, err } diff --git a/node/node.go b/node/node.go index 824a0926..9a49cd59 100644 --- a/node/node.go +++ b/node/node.go @@ -146,6 +146,10 @@ func NewNode(config *cfg.Config, } state.SetLogger(stateLogger) + genesisDoc, err := state.GenesisDoc() + if err != nil { + return nil, err + } // Create the proxyApp, which manages connections (consensus, mempool, query) // and sync tendermint and the app by replaying any necessary blocks @@ -286,7 +290,7 @@ func NewNode(config *cfg.Config, node := &Node{ config: config, - genesisDoc: state.GenesisDoc, + genesisDoc: genesisDoc, privValidator: privValidator, privKey: privKey, @@ -485,11 +489,15 @@ func (n *Node) makeNodeInfo() *p2p.NodeInfo { if _, ok := n.txIndexer.(*null.TxIndex); ok { txIndexerStatus = "off" } + chainID, err := n.consensusState.GetState().ChainID() + if err != nil { + cmn.PanicSanity(cmn.Fmt("failed ot get chainID: %v", err)) + } nodeInfo := &p2p.NodeInfo{ PubKey: n.privKey.PubKey().Unwrap().(crypto.PubKeyEd25519), Moniker: n.config.Moniker, - Network: n.consensusState.GetState().ChainID, + Network: chainID, Version: version.Version, Other: []string{ cmn.Fmt("wire_version=%v", wire.Version), diff --git a/state/execution.go b/state/execution.go index b917bfbe..e4992565 100644 --- a/state/execution.go +++ b/state/execution.go @@ -180,7 +180,11 @@ func (s *State) ValidateBlock(block *types.Block) error { func (s *State) validateBlock(block *types.Block) error { // Basic block validation. - err := block.ValidateBasic(s.ChainID, s.LastBlockHeight, s.LastBlockID, s.LastBlockTime, s.AppHash) + chainID, err := s.ChainID() + if err != nil { + return err + } + err = block.ValidateBasic(chainID, s.LastBlockHeight, s.LastBlockID, s.LastBlockTime, s.AppHash) if err != nil { return err } @@ -196,7 +200,7 @@ func (s *State) validateBlock(block *types.Block) error { s.LastValidators.Size(), len(block.LastCommit.Precommits))) } err := s.LastValidators.VerifyCommit( - s.ChainID, s.LastBlockID, block.Height-1, block.LastCommit) + chainID, s.LastBlockID, block.Height-1, block.LastCommit) if err != nil { return err } diff --git a/state/state.go b/state/state.go index 53ec8cc0..baf3b461 100644 --- a/state/state.go +++ b/state/state.go @@ -2,6 +2,7 @@ package state import ( "bytes" + "errors" "fmt" "io/ioutil" "sync" @@ -38,9 +39,11 @@ type State struct { mtx sync.Mutex db dbm.DB - // should not change - GenesisDoc *types.GenesisDoc - ChainID string + // genesisDoc is the memoized genesisDoc to cut down + // the number of unnecessary DB lookups since we no longer + // directly serialize the GenesisDoc in state. + // See https://github.com/tendermint/tendermint/issues/671. + genesisDoc *types.GenesisDoc // These fields are updated by SetBlockAndValidators. // LastBlockHeight=0 at genesis (ie. block(H=0) does not exist) @@ -100,7 +103,6 @@ func loadState(db dbm.DB, key []byte) *State { } // TODO: ensure that buf is completely read. } - return s } @@ -113,8 +115,6 @@ func (s *State) SetLogger(l log.Logger) { 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, @@ -127,12 +127,57 @@ func (s *State) Copy() *State { } } +var ( + errNilGenesisDoc = errors.New("no genesisDoc was found") + + genesisDBKey = []byte("genesis-doc") +) + +// GenesisDoc is the accessor to retrieve the genesisDoc associated +// with a state. If the state has no set GenesisDoc, it fetches from +// its database the JSON marshaled bytes keyed by "genesis-doc", and +// parses the GenesisDoc from that memoizing it for later use. +// If you'd like to change the value of the GenesisDoc, invoke SetGenesisDoc. +func (s *State) GenesisDoc() (*types.GenesisDoc, error) { + s.mtx.Lock() + defer s.mtx.Unlock() + + if s.genesisDoc == nil { + retrGenesisDocBytes := s.db.Get(genesisDBKey) + if len(retrGenesisDocBytes) == 0 { + return nil, errNilGenesisDoc + } + genDoc, err := types.GenesisDocFromJSON(retrGenesisDocBytes) + if err != nil { + return nil, err + } + s.genesisDoc = genDoc + } + return s.genesisDoc, nil +} + +func (s *State) ChainID() (string, error) { + genDoc, err := s.GenesisDoc() + if err != nil { + return "", err + } + return genDoc.ChainID, nil +} + // Save persists the State to the database. func (s *State) Save() { s.mtx.Lock() - defer s.mtx.Unlock() s.saveValidatorsInfo() s.db.SetSync(stateKey, s.Bytes()) + s.mtx.Unlock() +} + +// SetGenesisDoc sets the internal genesisDoc, but doesn't +// serialize it to the database, until Save is invoked. +func (s *State) SetGenesisDoc(genDoc *types.GenesisDoc) { + s.mtx.Lock() + s.genesisDoc = genDoc + s.mtx.Unlock() } // SaveABCIResponses persists the ABCIResponses to the database. @@ -264,12 +309,18 @@ func (s *State) GetValidators() (*types.ValidatorSet, *types.ValidatorSet) { return s.LastValidators, s.Validators } +var blankConsensusParams = types.ConsensusParams{} + // Params returns the consensus parameters used for // validating blocks func (s *State) Params() types.ConsensusParams { // TODO: this should move into the State proper // when we allow the app to change it - return *s.GenesisDoc.ConsensusParams + genDoc, err := s.GenesisDoc() + if err != nil || genDoc == nil { + return blankConsensusParams + } + return *genDoc.ConsensusParams } //------------------------------------------------------------------------ @@ -364,9 +415,9 @@ func MakeGenesisState(db dbm.DB, genDoc *types.GenesisDoc) (*State, error) { } return &State{ - db: db, - GenesisDoc: genDoc, - ChainID: genDoc.ChainID, + db: db, + + genesisDoc: genDoc, LastBlockHeight: 0, LastBlockID: types.BlockID{}, LastBlockTime: genDoc.GenesisTime, diff --git a/state/state_test.go b/state/state_test.go index 2ab2e934..c3605054 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -2,16 +2,21 @@ package state import ( "bytes" + "encoding/json" "fmt" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" cfg "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/types" abci "github.com/tendermint/abci/types" crypto "github.com/tendermint/go-crypto" + + "github.com/tendermint/go-wire/data" cmn "github.com/tendermint/tmlibs/common" dbm "github.com/tendermint/tmlibs/db" "github.com/tendermint/tmlibs/log" @@ -127,7 +132,9 @@ func TestValidatorChangesSaveLoad(t *testing.T) { // each valset is just one validator. // create list of them pubkeys := make([]crypto.PubKey, N+1) - pubkeys[0] = state.GenesisDoc.Validators[0].PubKey + genDoc, err := state.GenesisDoc() + assert.Nil(err, "want successful genDoc retrieval") + pubkeys[0] = genDoc.Validators[0].PubKey for i := 1; i < N+1; i++ { pubkeys[i] = crypto.GenPrivKeyEd25519().PubKey() } @@ -199,3 +206,104 @@ type valChangeTestCase struct { height int vals crypto.PubKey } + +var ( + aPrivKey = crypto.GenPrivKeyEd25519() + _1stGenesisDoc = &types.GenesisDoc{ + GenesisTime: time.Now().Add(-60 * time.Minute).Round(time.Second), + ChainID: "tendermint_state_test", + AppHash: data.Bytes{}, + ConsensusParams: &types.ConsensusParams{ + BlockSizeParams: types.BlockSizeParams{ + MaxBytes: 100, + MaxGas: 2000, + MaxTxs: 56, + }, + + BlockGossipParams: types.BlockGossipParams{ + BlockPartSizeBytes: 65336, + }, + }, + Validators: []types.GenesisValidator{ + {PubKey: aPrivKey.PubKey(), Power: 10000, Name: "TendermintFoo"}, + }, + } + _2ndGenesisDoc = func() *types.GenesisDoc { + copy := new(types.GenesisDoc) + *copy = *_1stGenesisDoc + copy.GenesisTime = time.Now().Round(time.Second) + return copy + }() +) + +// See Issue https://github.com/tendermint/tendermint/issues/671. +func TestGenesisDocAndChainIDAccessorsAndSetter(t *testing.T) { + tearDown, dbm, state := setupTestCase(t) + defer tearDown(t) + require := require.New(t) + + // Fire up the initial genesisDoc + _, err := state.GenesisDoc() + require.Nil(err, "expecting no error on first load of genesisDoc") + + // By contract, state doesn't expose the dbm, however we need to change + // it to test out that the respective chainID and genesisDoc will be changed + state.cachedGenesisDoc = nil + _1stBlob, err := json.Marshal(_1stGenesisDoc) + require.Nil(err, "expecting no error serializing _1stGenesisDoc") + dbm.Set(genesisDBKey, _1stBlob) + + retrGenDoc, err := state.GenesisDoc() + require.Nil(err, "unexpected error") + require.Equal(retrGenDoc, _1stGenesisDoc, "expecting the newly set-in-Db genesis doc") + chainID, err := state.ChainID() + require.Nil(err, "unexpected error") + require.Equal(chainID, _1stGenesisDoc.ChainID, "expecting the chainIDs to be equal") + + require.NotNil(state.cachedGenesisDoc, "after retrieval expecting a non-nil cachedGenesisDoc") + // Save should not discard the previous cachedGenesisDoc + // which was the point of filing https://github.com/tendermint/tendermint/issues/671. + state.Save() + require.NotNil(state.cachedGenesisDoc, "even after flush with .Save(), expecting a non-nil cachedGenesisDoc") + + // Now change up the data but ensure + // that a Save discards the old validator + _2ndBlob, err := json.Marshal(_2ndGenesisDoc) + require.Nil(err, "unexpected error") + dbm.Set(genesisDBKey, _2ndBlob) + + refreshGenDoc, err := state.GenesisDoc() + require.Nil(err, "unexpected error") + require.Equal(refreshGenDoc, _1stGenesisDoc, "despite setting the new genesisDoc in DB, it shouldn't affect the one in state") + state.SetGenesisDoc(_2ndGenesisDoc) + + refreshGenDoc, err = state.GenesisDoc() + require.Nil(err, "unexpected error") + require.Equal(refreshGenDoc, _2ndGenesisDoc, "expecting the newly set-in-Db genesis doc to have been reloaded after a .Save()") + + // Test that .Save() should never overwrite the currently set content in the DB + dbm.Set(genesisDBKey, _1stBlob) + state.Save() + require.Equal(dbm.Get(genesisDBKey), _1stBlob, ".Save() should NEVER serialize back the current genesisDoc") + + // ChainID on a nil cachedGenesisDoc should do a DB fetch + state.SetGenesisDoc(nil) + dbm.Set(genesisDBKey, _2ndBlob) + chainID, err = state.ChainID() + require.Nil(err, "unexpected error") + require.Equal(chainID, _2ndGenesisDoc.ChainID, "expecting the 2ndGenesisDoc.ChainID") + + // Now test what happens if we cannot find the genesis doc in the DB + // Checkpoint and discard + state.Save() + dbm.Set(genesisDBKey, nil) + state.SetGenesisDoc(nil) + gotGenDoc, err := state.GenesisDoc() + require.NotNil(err, "could not parse out a genesisDoc from the DB") + require.Nil(gotGenDoc, "since we couldn't parse the genesis doc, expecting a nil genesis doc") + + dbm.Set(genesisDBKey, []byte(`{}`)) + gotGenDoc, err = state.GenesisDoc() + require.NotNil(err, "despite {}, that's not a valid serialization for a genesisDoc") + require.Nil(gotGenDoc, "since we couldn't parse the genesis doc, expecting a nil genesis doc") +}