From 1971e149fb42e48027e30ec5cf31356b707ac7cb Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 5 Oct 2017 16:50:05 +0400 Subject: [PATCH] ChainID() and Params() do not return errors - remove state#GenesisDoc() method --- blockchain/reactor.go | 10 +-- blockchain/store.go | 2 +- consensus/byzantine_test.go | 4 +- consensus/replay_test.go | 4 +- consensus/state.go | 37 ++--------- node/node.go | 29 ++++----- state/execution.go | 7 +- state/state.go | 100 +++++++++++----------------- state/state_test.go | 126 ++++++------------------------------ 9 files changed, 84 insertions(+), 235 deletions(-) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 3001206f..6fba874b 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -187,6 +187,8 @@ func (bcR *BlockchainReactor) poolRoutine() { statusUpdateTicker := time.NewTicker(statusUpdateIntervalSeconds * time.Second) switchToConsensusTicker := time.NewTicker(switchToConsensusIntervalSeconds * time.Second) + chainID := bcR.state.ChainID() + FOR_LOOP: for { select { @@ -242,13 +244,7 @@ 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. - 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( + 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) diff --git a/blockchain/store.go b/blockchain/store.go index a37fda93..79edfeaf 100644 --- a/blockchain/store.go +++ b/blockchain/store.go @@ -7,7 +7,7 @@ import ( "io" "sync" - "github.com/tendermint/go-wire" + wire "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/byzantine_test.go b/consensus/byzantine_test.go index 38dff406..91a97c60 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -167,13 +167,13 @@ func byzantineDecideProposalFunc(t *testing.T, height, round int, cs *ConsensusS block1, blockParts1 := cs.createProposalBlock() polRound, polBlockID := cs.Votes.POLInfo() proposal1 := types.NewProposal(height, round, blockParts1.Header(), polRound, polBlockID) - cs.privValidator.SignProposal(cs.state.ChainID, proposal1) // byzantine doesnt err + cs.privValidator.SignProposal(cs.state.ChainID(), proposal1) // byzantine doesnt err // Create a new proposal block from state/txs from the mempool. block2, blockParts2 := cs.createProposalBlock() polRound, polBlockID = cs.Votes.POLInfo() proposal2 := types.NewProposal(height, round, blockParts2.Header(), polRound, polBlockID) - cs.privValidator.SignProposal(cs.state.ChainID, proposal2) // byzantine doesnt err + cs.privValidator.SignProposal(cs.state.ChainID(), proposal2) // byzantine doesnt err block1Hash := block1.Hash() block2Hash := block2.Hash() diff --git a/consensus/replay_test.go b/consensus/replay_test.go index c478a095..6e63024a 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -231,7 +231,7 @@ func TestWALCrashBeforeWritePropose(t *testing.T) { msg := readTimedWALMessage(t, proposalMsg) proposal := msg.Msg.(msgInfo).Msg.(*ProposalMessage) // Set LastSig - toPV(cs.privValidator).LastSignBytes = types.SignBytes(cs.state.ChainID, proposal.Proposal) + toPV(cs.privValidator).LastSignBytes = types.SignBytes(cs.state.ChainID(), proposal.Proposal) toPV(cs.privValidator).LastSignature = proposal.Proposal.Signature runReplayTest(t, cs, walFile, newBlockCh, thisCase, lineNum) } @@ -256,7 +256,7 @@ func testReplayCrashBeforeWriteVote(t *testing.T, thisCase *testCase, lineNum in msg := readTimedWALMessage(t, voteMsg) vote := msg.Msg.(msgInfo).Msg.(*VoteMessage) // Set LastSig - toPV(cs.privValidator).LastSignBytes = types.SignBytes(cs.state.ChainID, vote.Vote) + toPV(cs.privValidator).LastSignBytes = types.SignBytes(cs.state.ChainID(), vote.Vote) toPV(cs.privValidator).LastSignature = vote.Vote.Signature }) runReplayTest(t, cs, walFile, newBlockCh, thisCase, lineNum) diff --git a/consensus/state.go b/consensus/state.go index ac457f49..2d2a5e3d 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -389,12 +389,8 @@ 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(chainID, state.LastBlockHeight, seenCommit.Round(), types.VoteTypePrecommit, state.LastValidators) + lastPrecommits := types.NewVoteSet(state.ChainID(), state.LastBlockHeight, seenCommit.Round(), types.VoteTypePrecommit, state.LastValidators) for _, precommit := range seenCommit.Precommits { if precommit == nil { continue @@ -711,6 +707,7 @@ func (cs *ConsensusState) proposalHeartbeat(height, round int) { // not a validator valIndex = -1 } + chainID := cs.state.ChainID() for { rs := cs.GetRoundState() // if we've already moved on, no need to send more heartbeats @@ -724,10 +721,6 @@ func (cs *ConsensusState) proposalHeartbeat(height, round int) { ValidatorAddress: addr, ValidatorIndex: valIndex, } - chainID, err := cs.state.ChainID() - if err != nil { - return - } cs.privValidator.SignHeartbeat(chainID, heartbeat) heartbeatEvent := types.EventDataProposalHeartbeat{heartbeat} types.FireEventProposalHeartbeat(cs.evsw, heartbeatEvent) @@ -805,11 +798,7 @@ func (cs *ConsensusState) defaultDecideProposal(height, round int) { // Make proposal polRound, polBlockID := cs.Votes.POLInfo() proposal := types.NewProposal(height, round, blockParts.Header(), polRound, polBlockID) - chainID, err := cs.state.ChainID() - if err != nil { - return - } - if err := cs.privValidator.SignProposal(chainID, proposal); err == nil { + if err := cs.privValidator.SignProposal(cs.state.ChainID(), proposal); err == nil { // Set fields /* fields set by setProposal and addBlockPart cs.Proposal = proposal @@ -868,12 +857,7 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts // Mempool validated transactions txs := cs.mempool.Reap(cs.config.MaxBlockSizeTxs) - chainID, err := cs.state.ChainID() - if err != nil { - cs.Logger.Error("chainID err", err) - return - } - return types.MakeBlock(cs.Height, chainID, txs, commit, + return types.MakeBlock(cs.Height, cs.state.ChainID(), txs, commit, cs.state.LastBlockID, cs.state.Validators.Hash(), cs.state.AppHash, cs.state.Params().BlockPartSizeBytes) } @@ -1278,13 +1262,8 @@ 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(chainID, proposal), proposal.Signature) { + if !cs.Validators.GetProposer().PubKey.VerifyBytes(types.SignBytes(cs.state.ChainID(), proposal), proposal.Signature) { return ErrInvalidProposalSignature } @@ -1469,10 +1448,6 @@ 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{ @@ -1483,7 +1458,7 @@ func (cs *ConsensusState) signVote(type_ byte, hash []byte, header types.PartSet Type: type_, BlockID: types.BlockID{hash, header}, } - err = cs.privValidator.SignVote(chainID, vote) + err := cs.privValidator.SignVote(cs.state.ChainID(), vote) return vote, err } diff --git a/node/node.go b/node/node.go index 9a49cd59..a3267cdd 100644 --- a/node/node.go +++ b/node/node.go @@ -132,24 +132,24 @@ func NewNode(config *cfg.Config, if err != nil { return nil, err } + + genDoc, err := genesisDocProvider() + if err != nil { + return nil, err + } + state := sm.LoadState(stateDB) if state == nil { - genDoc, err := genesisDocProvider() - if err != nil { - return nil, err - } state, err = sm.MakeGenesisState(stateDB, genDoc) if err != nil { return nil, err } state.Save() + } else { + state.SetChainID(genDoc.ChainID) + state.SetParams(genDoc.ConsensusParams) } - 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 @@ -163,6 +163,8 @@ func NewNode(config *cfg.Config, // reload the state (it may have been updated by the handshake) state = sm.LoadState(stateDB) + state.SetChainID(genDoc.ChainID) + state.SetParams(genDoc.ConsensusParams) state.SetLogger(stateLogger) // Transaction indexing @@ -290,7 +292,7 @@ func NewNode(config *cfg.Config, node := &Node{ config: config, - genesisDoc: genesisDoc, + genesisDoc: genDoc, privValidator: privValidator, privKey: privKey, @@ -489,15 +491,10 @@ 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: chainID, + Network: n.genesisDoc.ChainID, Version: version.Version, Other: []string{ cmn.Fmt("wire_version=%v", wire.Version), diff --git a/state/execution.go b/state/execution.go index e4992565..603eab71 100644 --- a/state/execution.go +++ b/state/execution.go @@ -179,12 +179,9 @@ func (s *State) ValidateBlock(block *types.Block) error { } func (s *State) validateBlock(block *types.Block) error { + chainID := s.ChainID() // Basic block validation. - chainID, err := s.ChainID() - if err != nil { - return err - } - err = block.ValidateBasic(chainID, s.LastBlockHeight, s.LastBlockID, s.LastBlockTime, s.AppHash) + err := block.ValidateBasic(chainID, s.LastBlockHeight, s.LastBlockID, s.LastBlockTime, s.AppHash) if err != nil { return err } diff --git a/state/state.go b/state/state.go index baf3b461..6460f201 100644 --- a/state/state.go +++ b/state/state.go @@ -2,7 +2,6 @@ package state import ( "bytes" - "errors" "fmt" "io/ioutil" "sync" @@ -39,11 +38,10 @@ type State struct { mtx sync.Mutex db dbm.DB - // 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 + chainID string + + params *types.ConsensusParams // These fields are updated by SetBlockAndValidators. // LastBlockHeight=0 at genesis (ie. block(H=0) does not exist) @@ -73,14 +71,22 @@ type State struct { // to the database. func GetState(stateDB dbm.DB, genesisFile string) (*State, error) { var err error + genDoc, err := MakeGenesisDocFromFile(genesisFile) + if err != nil { + return nil, err + } state := LoadState(stateDB) if state == nil { - state, err = MakeGenesisStateFromFile(stateDB, genesisFile) + state, err = MakeGenesisState(stateDB, genDoc) if err != nil { return nil, err } state.Save() + } else { + state.SetChainID(genDoc.ChainID) + state.SetParams(genDoc.ConsensusParams) } + return state, nil } @@ -113,6 +119,7 @@ func (s *State) SetLogger(l log.Logger) { // Copy makes a copy of the State for mutating. func (s *State) Copy() *State { + paramsCopy := *s.params return &State{ db: s.db, LastBlockHeight: s.LastBlockHeight, @@ -123,47 +130,12 @@ func (s *State) Copy() *State { AppHash: s.AppHash, TxIndexer: s.TxIndexer, // pointer here, not value LastHeightValidatorsChanged: s.LastHeightValidatorsChanged, - logger: s.logger, + logger: s.logger, + chainID: s.chainID, + params: ¶msCopy, } } -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() @@ -172,14 +144,6 @@ func (s *State) Save() { 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. // This is useful in case we crash after app.Commit and before s.Save(). func (s *State) SaveABCIResponses(abciResponses *ABCIResponses) { @@ -213,7 +177,7 @@ func (s *State) LoadValidators(height int) (*types.ValidatorSet, error) { if v.ValidatorSet == nil { v = s.loadValidators(v.LastHeightChanged) if v == nil { - cmn.PanicSanity(fmt.Sprintf(`Couldn't find validators at + cmn.PanicSanity(fmt.Sprintf(`Couldn't find validators at height %d as last changed from height %d`, v.LastHeightChanged, height)) } } @@ -309,18 +273,26 @@ func (s *State) GetValidators() (*types.ValidatorSet, *types.ValidatorSet) { return s.LastValidators, s.Validators } -var blankConsensusParams = types.ConsensusParams{} +// ChainID returns the chain ID. +func (s *State) ChainID() string { + return s.chainID +} + +// SetChainID sets the chain ID. +func (s *State) SetChainID(chainID string) { + s.chainID = chainID +} // Params returns the consensus parameters used for -// validating blocks +// validating blocks. func (s *State) Params() types.ConsensusParams { - // TODO: this should move into the State proper - // when we allow the app to change it - genDoc, err := s.GenesisDoc() - if err != nil || genDoc == nil { - return blankConsensusParams - } - return *genDoc.ConsensusParams + return *s.params +} + +// SetParams sets the consensus parameters used for +// validating blocks. +func (s *State) SetParams(params *types.ConsensusParams) { + s.params = params } //------------------------------------------------------------------------ @@ -417,7 +389,9 @@ func MakeGenesisState(db dbm.DB, genDoc *types.GenesisDoc) (*State, error) { return &State{ db: db, - genesisDoc: genDoc, + chainID: genDoc.ChainID, + params: genDoc.ConsensusParams, + LastBlockHeight: 0, LastBlockID: types.BlockID{}, LastBlockTime: genDoc.GenesisTime, diff --git a/state/state_test.go b/state/state_test.go index c3605054..e0703d3b 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -2,13 +2,10 @@ 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" @@ -16,7 +13,6 @@ import ( 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" @@ -120,6 +116,22 @@ func TestValidatorSimpleSaveLoad(t *testing.T) { assert.IsType(ErrNoValSetForHeight{}, err, "expected err at unknown height") } +func TestChainID(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) + + state.SetChainID("test") + assert.Equal(t, "test", state.ChainID()) +} + +func TestParams(t *testing.T) { + tearDown, _, state := setupTestCase(t) + defer tearDown(t) + + state.SetParams(&types.ConsensusParams{}) + assert.Equal(t, types.ConsensusParams{}, state.Params()) +} + func TestValidatorChangesSaveLoad(t *testing.T) { tearDown, _, state := setupTestCase(t) defer tearDown(t) @@ -132,9 +144,8 @@ func TestValidatorChangesSaveLoad(t *testing.T) { // each valset is just one validator. // create list of them pubkeys := make([]crypto.PubKey, N+1) - genDoc, err := state.GenesisDoc() - assert.Nil(err, "want successful genDoc retrieval") - pubkeys[0] = genDoc.Validators[0].PubKey + _, val := state.Validators.GetByIndex(0) + pubkeys[0] = val.PubKey for i := 1; i < N+1; i++ { pubkeys[i] = crypto.GenPrivKeyEd25519().PubKey() } @@ -206,104 +217,3 @@ 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") -}