ChainID() and Params() do not return errors

- remove state#GenesisDoc() method

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
 	}

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:  &paramsCopy,
 	}
 }
 
-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,

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")
-}