R4R: Split immutable and mutable parts of priv_validator.json (#2870)

* split immutable and mutable parts of priv_validator.json

* fix bugs

* minor changes

* retrig test

* delete scripts/wire2amino.go

* fix test

* fixes from review

* privval: remove mtx

* rearrange priv_validator.go

* upgrade path

* write tests for the upgrade

* fix for unsafe_reset_all

* add test

* add reset test

CHANGELOG_PENDING.md

@@ -21,6 +21,7 @@ Special thanks to external contributors on this release:
 - multiple connections from the same IP are now disabled by default (see `allow_duplicate_ip` config option)
 
 ### FEATURES:
+- [privval] \#1181 Split immutable and mutable parts of priv_validator.json
 
 ### IMPROVEMENTS:
 

cmd/priv_val_server/main.go

@@ -15,7 +15,8 @@ func main() {
 	var (
 		addr             = flag.String("addr", ":26659", "Address of client to connect to")
 		chainID          = flag.String("chain-id", "mychain", "chain id")
-		privValPath = flag.String("priv", "", "priv val file path")
+		privValKeyPath   = flag.String("priv-key", "", "priv val key file path")
+		privValStatePath = flag.String("priv-state", "", "priv val state file path")
 
 		logger = log.NewTMLogger(
 			log.NewSyncWriter(os.Stdout),
@@ -27,10 +28,11 @@ func main() {
 		"Starting private validator",
 		"addr", *addr,
 		"chainID", *chainID,
-		"privPath", *privValPath,
+		"privKeyPath", *privValKeyPath,
+		"privStatePath", *privValStatePath,
 	)
 
-	pv := privval.LoadFilePV(*privValPath)
+	pv := privval.LoadFilePV(*privValKeyPath, *privValStatePath)
 
 	rs := privval.NewRemoteSigner(
 		logger,

cmd/tendermint/commands/gen_validator.go

@@ -17,7 +17,7 @@ var GenValidatorCmd = &cobra.Command{
 }
 
 func genValidator(cmd *cobra.Command, args []string) {
-	pv := privval.GenFilePV("")
+	pv := privval.GenFilePV("", "")
 	jsbz, err := cdc.MarshalJSON(pv)
 	if err != nil {
 		panic(err)

cmd/tendermint/commands/init.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/spf13/cobra"
-
 	cfg "github.com/tendermint/tendermint/config"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/p2p"
@@ -26,15 +25,18 @@ func initFiles(cmd *cobra.Command, args []string) error {
 
 func initFilesWithConfig(config *cfg.Config) error {
 	// private validator
-	privValFile := config.PrivValidatorFile()
+	privValKeyFile := config.PrivValidatorKeyFile()
+	privValStateFile := config.PrivValidatorStateFile()
 	var pv *privval.FilePV
-	if cmn.FileExists(privValFile) {
+	if cmn.FileExists(privValKeyFile) {
-		pv = privval.LoadFilePV(privValFile)
+		pv = privval.LoadFilePV(privValKeyFile, privValStateFile)
-		logger.Info("Found private validator", "path", privValFile)
+		logger.Info("Found private validator", "keyFile", privValKeyFile,
+			"stateFile", privValStateFile)
 	} else {
-		pv = privval.GenFilePV(privValFile)
+		pv = privval.GenFilePV(privValKeyFile, privValStateFile)
 		pv.Save()
-		logger.Info("Generated private validator", "path", privValFile)
+		logger.Info("Generated private validator", "keyFile", privValKeyFile,
+			"stateFile", privValStateFile)
 	}
 
 	nodeKeyFile := config.NodeKeyFile()

cmd/tendermint/commands/reset_priv_validator.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/privval"
 )
@@ -27,36 +28,41 @@ var ResetPrivValidatorCmd = &cobra.Command{
 // XXX: this is totally unsafe.
 // it's only suitable for testnets.
 func resetAll(cmd *cobra.Command, args []string) {
-	ResetAll(config.DBDir(), config.P2P.AddrBookFile(), config.PrivValidatorFile(), logger)
+	ResetAll(config.DBDir(), config.P2P.AddrBookFile(), config.PrivValidatorKeyFile(),
+		config.PrivValidatorStateFile(), logger)
 }
 
 // XXX: this is totally unsafe.
 // it's only suitable for testnets.
 func resetPrivValidator(cmd *cobra.Command, args []string) {
-	resetFilePV(config.PrivValidatorFile(), logger)
+	resetFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile(), logger)
 }
 
-// ResetAll removes the privValidator and address book files plus all data.
+// ResetAll removes address book files plus all data, and resets the privValdiator data.
 // Exported so other CLI tools can use it.
-func ResetAll(dbDir, addrBookFile, privValFile string, logger log.Logger) {
+func ResetAll(dbDir, addrBookFile, privValKeyFile, privValStateFile string, logger log.Logger) {
-	resetFilePV(privValFile, logger)
 	removeAddrBook(addrBookFile, logger)
 	if err := os.RemoveAll(dbDir); err == nil {
 		logger.Info("Removed all blockchain history", "dir", dbDir)
 	} else {
 		logger.Error("Error removing all blockchain history", "dir", dbDir, "err", err)
 	}
+	// recreate the dbDir since the privVal state needs to live there
+	cmn.EnsureDir(dbDir, 0700)
+	resetFilePV(privValKeyFile, privValStateFile, logger)
 }
 
-func resetFilePV(privValFile string, logger log.Logger) {
+func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) {
-	if _, err := os.Stat(privValFile); err == nil {
+	if _, err := os.Stat(privValKeyFile); err == nil {
-		pv := privval.LoadFilePV(privValFile)
+		pv := privval.LoadFilePVEmptyState(privValKeyFile, privValStateFile)
 		pv.Reset()
-		logger.Info("Reset private validator file to genesis state", "file", privValFile)
+		logger.Info("Reset private validator file to genesis state", "keyFile", privValKeyFile,
+			"stateFile", privValStateFile)
 	} else {
-		pv := privval.GenFilePV(privValFile)
+		pv := privval.GenFilePV(privValKeyFile, privValStateFile)
 		pv.Save()
-		logger.Info("Generated private validator file", "file", privValFile)
+		logger.Info("Generated private validator file", "file", "keyFile", privValKeyFile,
+			"stateFile", privValStateFile)
 	}
 }
 

cmd/tendermint/commands/show_validator.go

@@ -16,7 +16,7 @@ var ShowValidatorCmd = &cobra.Command{
 }
 
 func showValidator(cmd *cobra.Command, args []string) {
-	privValidator := privval.LoadOrGenFilePV(config.PrivValidatorFile())
+	privValidator := privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
 	pubKeyJSONBytes, _ := cdc.MarshalJSON(privValidator.GetPubKey())
 	fmt.Println(string(pubKeyJSONBytes))
 }

cmd/tendermint/commands/testnet.go

@@ -85,11 +85,18 @@ func testnetFiles(cmd *cobra.Command, args []string) error {
 			_ = os.RemoveAll(outputDir)
 			return err
 		}
+		err = os.MkdirAll(filepath.Join(nodeDir, "data"), nodeDirPerm)
+		if err != nil {
+			_ = os.RemoveAll(outputDir)
+			return err
+		}
 
 		initFilesWithConfig(config)
 
-		pvFile := filepath.Join(nodeDir, config.BaseConfig.PrivValidator)
+		pvKeyFile := filepath.Join(nodeDir, config.BaseConfig.PrivValidatorKey)
-		pv := privval.LoadFilePV(pvFile)
+		pvStateFile := filepath.Join(nodeDir, config.BaseConfig.PrivValidatorState)
+
+		pv := privval.LoadFilePV(pvKeyFile, pvStateFile)
 		genVals[i] = types.GenesisValidator{
 			Address: pv.GetPubKey().Address(),
 			PubKey:  pv.GetPubKey(),

config/config.go

@@ -35,17 +35,26 @@ var (
 	defaultConfigFileName  = "config.toml"
 	defaultGenesisJSONName = "genesis.json"
 
-	defaultPrivValName  = "priv_validator.json"
+	defaultPrivValKeyName   = "priv_validator_key.json"
+	defaultPrivValStateName = "priv_validator_state.json"
+
 	defaultNodeKeyName  = "node_key.json"
 	defaultAddrBookName = "addrbook.json"
 
 	defaultConfigFilePath   = filepath.Join(defaultConfigDir, defaultConfigFileName)
 	defaultGenesisJSONPath  = filepath.Join(defaultConfigDir, defaultGenesisJSONName)
-	defaultPrivValPath     = filepath.Join(defaultConfigDir, defaultPrivValName)
+	defaultPrivValKeyPath   = filepath.Join(defaultConfigDir, defaultPrivValKeyName)
+	defaultPrivValStatePath = filepath.Join(defaultDataDir, defaultPrivValStateName)
+
 	defaultNodeKeyPath  = filepath.Join(defaultConfigDir, defaultNodeKeyName)
 	defaultAddrBookPath = filepath.Join(defaultConfigDir, defaultAddrBookName)
 )
 
+var (
+	oldPrivVal     = "priv_validator.json"
+	oldPrivValPath = filepath.Join(defaultConfigDir, oldPrivVal)
+)
+
 // Config defines the top level configuration for a Tendermint node
 type Config struct {
 	// Top level options use an anonymous struct
@@ -160,7 +169,10 @@ type BaseConfig struct {
 	Genesis string `mapstructure:"genesis_file"`
 
 	// Path to the JSON file containing the private key to use as a validator in the consensus protocol
-	PrivValidator string `mapstructure:"priv_validator_file"`
+	PrivValidatorKey string `mapstructure:"priv_validator_key_file"`
+
+	// Path to the JSON file containing the last sign state of a validator
+	PrivValidatorState string `mapstructure:"priv_validator_state_file"`
 
 	// TCP or UNIX socket address for Tendermint to listen on for
 	// connections from an external PrivValidator process
@@ -184,7 +196,8 @@ type BaseConfig struct {
 func DefaultBaseConfig() BaseConfig {
 	return BaseConfig{
 		Genesis:            defaultGenesisJSONPath,
-		PrivValidator:     defaultPrivValPath,
+		PrivValidatorKey:   defaultPrivValKeyPath,
+		PrivValidatorState: defaultPrivValStatePath,
 		NodeKey:            defaultNodeKeyPath,
 		Moniker:            defaultMoniker,
 		ProxyApp:           "tcp://127.0.0.1:26658",
@@ -218,9 +231,20 @@ func (cfg BaseConfig) GenesisFile() string {
 	return rootify(cfg.Genesis, cfg.RootDir)
 }
 
-// PrivValidatorFile returns the full path to the priv_validator.json file
+// PrivValidatorKeyFile returns the full path to the priv_validator_key.json file
-func (cfg BaseConfig) PrivValidatorFile() string {
+func (cfg BaseConfig) PrivValidatorKeyFile() string {
-	return rootify(cfg.PrivValidator, cfg.RootDir)
+	return rootify(cfg.PrivValidatorKey, cfg.RootDir)
+}
+
+// PrivValidatorFile returns the full path to the priv_validator_state.json file
+func (cfg BaseConfig) PrivValidatorStateFile() string {
+	return rootify(cfg.PrivValidatorState, cfg.RootDir)
+}
+
+// OldPrivValidatorFile returns the full path of the priv_validator.json from pre v0.28.0.
+// TODO: eventually remove.
+func (cfg BaseConfig) OldPrivValidatorFile() string {
+	return rootify(oldPrivValPath, cfg.RootDir)
 }
 
 // NodeKeyFile returns the full path to the node_key.json file

config/toml.go

@@ -95,7 +95,10 @@ log_format = "{{ .BaseConfig.LogFormat }}"
 genesis_file = "{{ js .BaseConfig.Genesis }}"
 
 # Path to the JSON file containing the private key to use as a validator in the consensus protocol
-priv_validator_file = "{{ js .BaseConfig.PrivValidator }}"
+priv_validator_key_file = "{{ js .BaseConfig.PrivValidatorKey }}"
+
+# Path to the JSON file containing the last sign state of a validator
+priv_validator_state_file = "{{ js .BaseConfig.PrivValidatorState }}"
 
 # TCP or UNIX socket address for Tendermint to listen on for
 # connections from an external PrivValidator process
@@ -342,7 +345,8 @@ func ResetTestRoot(testName string) *Config {
 	baseConfig := DefaultBaseConfig()
 	configFilePath := filepath.Join(rootDir, defaultConfigFilePath)
 	genesisFilePath := filepath.Join(rootDir, baseConfig.Genesis)
-	privFilePath := filepath.Join(rootDir, baseConfig.PrivValidator)
+	privKeyFilePath := filepath.Join(rootDir, baseConfig.PrivValidatorKey)
+	privStateFilePath := filepath.Join(rootDir, baseConfig.PrivValidatorState)
 
 	// Write default config file if missing.
 	if !cmn.FileExists(configFilePath) {
@@ -352,7 +356,8 @@ func ResetTestRoot(testName string) *Config {
 		cmn.MustWriteFile(genesisFilePath, []byte(testGenesis), 0644)
 	}
 	// we always overwrite the priv val
-	cmn.MustWriteFile(privFilePath, []byte(testPrivValidator), 0644)
+	cmn.MustWriteFile(privKeyFilePath, []byte(testPrivValidatorKey), 0644)
+	cmn.MustWriteFile(privStateFilePath, []byte(testPrivValidatorState), 0644)
 
 	config := TestConfig().SetRoot(rootDir)
 	return config
@@ -374,7 +379,7 @@ var testGenesis = `{
   "app_hash": ""
 }`
 
-var testPrivValidator = `{
+var testPrivValidatorKey = `{
   "address": "A3258DCBF45DCA0DF052981870F2D1441A36D145",
   "pub_key": {
     "type": "tendermint/PubKeyEd25519",
@@ -383,8 +388,11 @@ var testPrivValidator = `{
   "priv_key": {
     "type": "tendermint/PrivKeyEd25519",
     "value": "EVkqJO/jIXp3rkASXfh9YnyToYXRXhBr6g9cQVxPFnQBP/5povV4HTjvsy530kybxKHwEi85iU8YL0qQhSYVoQ=="
-  },
+  }
-  "last_height": "0",
+}`
-  "last_round": "0",
+
-  "last_step": 0
+var testPrivValidatorState = `{
+  "height": "0",
+  "round": "0",
+  "step": 0
 }`

config/toml_test.go

@@ -60,7 +60,7 @@ func TestEnsureTestRoot(t *testing.T) {
 
 	// TODO: make sure the cfg returned and testconfig are the same!
 	baseConfig := DefaultBaseConfig()
-	ensureFiles(t, rootDir, defaultDataDir, baseConfig.Genesis, baseConfig.PrivValidator)
+	ensureFiles(t, rootDir, defaultDataDir, baseConfig.Genesis, baseConfig.PrivValidatorKey, baseConfig.PrivValidatorState)
 }
 
 func checkConfig(configFile string) bool {

consensus/common_test.go

@@ -6,14 +6,18 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path"
+	"path/filepath"
 	"reflect"
 	"sort"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/go-kit/kit/log/term"
+
 	abcicli "github.com/tendermint/tendermint/abci/client"
+	"github.com/tendermint/tendermint/abci/example/counter"
+	"github.com/tendermint/tendermint/abci/example/kvstore"
 	abci "github.com/tendermint/tendermint/abci/types"
 	bc "github.com/tendermint/tendermint/blockchain"
 	cfg "github.com/tendermint/tendermint/config"
@@ -27,11 +31,6 @@ import (
 	sm "github.com/tendermint/tendermint/state"
 	"github.com/tendermint/tendermint/types"
 	tmtime "github.com/tendermint/tendermint/types/time"
-
-	"github.com/tendermint/tendermint/abci/example/counter"
-	"github.com/tendermint/tendermint/abci/example/kvstore"
-
-	"github.com/go-kit/kit/log/term"
 )
 
 const (
@@ -281,9 +280,10 @@ func newConsensusStateWithConfigAndBlockStore(thisConfig *cfg.Config, state sm.S
 }
 
 func loadPrivValidator(config *cfg.Config) *privval.FilePV {
-	privValidatorFile := config.PrivValidatorFile()
+	privValidatorKeyFile := config.PrivValidatorKeyFile()
-	ensureDir(path.Dir(privValidatorFile), 0700)
+	ensureDir(filepath.Dir(privValidatorKeyFile), 0700)
-	privValidator := privval.LoadOrGenFilePV(privValidatorFile)
+	privValidatorStateFile := config.PrivValidatorStateFile()
+	privValidator := privval.LoadOrGenFilePV(privValidatorKeyFile, privValidatorStateFile)
 	privValidator.Reset()
 	return privValidator
 }
@@ -591,7 +591,7 @@ func randConsensusNet(nValidators int, testName string, tickerFunc func() Timeou
 		for _, opt := range configOpts {
 			opt(thisConfig)
 		}
-		ensureDir(path.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal
+		ensureDir(filepath.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal
 		app := appFunc()
 		vals := types.TM2PB.ValidatorUpdates(state.Validators)
 		app.InitChain(abci.RequestInitChain{Validators: vals})
@@ -612,16 +612,21 @@ func randConsensusNetWithPeers(nValidators, nPeers int, testName string, tickerF
 		stateDB := dbm.NewMemDB() // each state needs its own db
 		state, _ := sm.LoadStateFromDBOrGenesisDoc(stateDB, genDoc)
 		thisConfig := ResetConfig(fmt.Sprintf("%s_%d", testName, i))
-		ensureDir(path.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal
+		ensureDir(filepath.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal
 		var privVal types.PrivValidator
 		if i < nValidators {
 			privVal = privVals[i]
 		} else {
-			tempFile, err := ioutil.TempFile("", "priv_validator_")
+			tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 			if err != nil {
 				panic(err)
 			}
-			privVal = privval.GenFilePV(tempFile.Name())
+			tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+			if err != nil {
+				panic(err)
+			}
+
+			privVal = privval.GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
 		}
 
 		app := appFunc()

consensus/replay_test.go

@@ -319,7 +319,7 @@ func testHandshakeReplay(t *testing.T, nBlocks int, mode uint) {
 	walFile := tempWALWithData(walBody)
 	config.Consensus.SetWalFile(walFile)
 
-	privVal := privval.LoadFilePV(config.PrivValidatorFile())
+	privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
 
 	wal, err := NewWAL(walFile)
 	require.NoError(t, err)
@@ -633,7 +633,7 @@ func TestInitChainUpdateValidators(t *testing.T) {
 	clientCreator := proxy.NewLocalClientCreator(app)
 
 	config := ResetConfig("proxy_test_")
-	privVal := privval.LoadFilePV(config.PrivValidatorFile())
+	privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
 	stateDB, state, store := stateAndStore(config, privVal.GetPubKey(), 0x0)
 
 	oldValAddr := state.Validators.Validators[0].Address

consensus/wal_generator.go

@@ -40,8 +40,9 @@ func WALGenerateNBlocks(wr io.Writer, numBlocks int) (err error) {
 	// COPY PASTE FROM node.go WITH A FEW MODIFICATIONS
 	// NOTE: we can't import node package because of circular dependency.
 	// NOTE: we don't do handshake so need to set state.Version.Consensus.App directly.
-	privValidatorFile := config.PrivValidatorFile()
+	privValidatorKeyFile := config.PrivValidatorKeyFile()
-	privValidator := privval.LoadOrGenFilePV(privValidatorFile)
+	privValidatorStateFile := config.PrivValidatorStateFile()
+	privValidator := privval.LoadOrGenFilePV(privValidatorKeyFile, privValidatorStateFile)
 	genDoc, err := types.GenesisDocFromFile(config.GenesisFile())
 	if err != nil {
 		return errors.Wrap(err, "failed to read genesis file")

node/node.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/http"
 	_ "net/http/pprof"
+	"os"
 	"strings"
 	"time"
 
@@ -86,8 +87,26 @@ func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// Convert old PrivValidator if it exists.
+	oldPrivVal := config.OldPrivValidatorFile()
+	newPrivValKey := config.PrivValidatorKeyFile()
+	newPrivValState := config.PrivValidatorStateFile()
+	if _, err := os.Stat(oldPrivVal); !os.IsNotExist(err) {
+		oldPV, err := privval.LoadOldFilePV(oldPrivVal)
+		if err != nil {
+			return nil, fmt.Errorf("Error reading OldPrivValidator from %v: %v\n", oldPrivVal, err)
+		}
+		logger.Info("Upgrading PrivValidator file",
+			"old", oldPrivVal,
+			"newKey", newPrivValKey,
+			"newState", newPrivValState,
+		)
+		oldPV.Upgrade(newPrivValKey, newPrivValState)
+	}
+
 	return NewNode(config,
-		privval.LoadOrGenFilePV(config.PrivValidatorFile()),
+		privval.LoadOrGenFilePV(newPrivValKey, newPrivValState),
 		nodeKey,
 		proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()),
 		DefaultGenesisDocProviderFunc(config),

privval/old_priv_validator.go

@@ -0,0 +1,80 @@
+package privval
+
+import (
+	"io/ioutil"
+	"os"
+
+	"github.com/tendermint/tendermint/crypto"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/types"
+)
+
+// OldFilePV is the old version of the FilePV, pre v0.28.0.
+type OldFilePV struct {
+	Address       types.Address  `json:"address"`
+	PubKey        crypto.PubKey  `json:"pub_key"`
+	LastHeight    int64          `json:"last_height"`
+	LastRound     int            `json:"last_round"`
+	LastStep      int8           `json:"last_step"`
+	LastSignature []byte         `json:"last_signature,omitempty"`
+	LastSignBytes cmn.HexBytes   `json:"last_signbytes,omitempty"`
+	PrivKey       crypto.PrivKey `json:"priv_key"`
+
+	filePath string
+}
+
+// LoadOldFilePV loads an OldFilePV from the filePath.
+func LoadOldFilePV(filePath string) (*OldFilePV, error) {
+	pvJSONBytes, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return nil, err
+	}
+	pv := &OldFilePV{}
+	err = cdc.UnmarshalJSON(pvJSONBytes, &pv)
+	if err != nil {
+		return nil, err
+	}
+
+	// overwrite pubkey and address for convenience
+	pv.PubKey = pv.PrivKey.PubKey()
+	pv.Address = pv.PubKey.Address()
+
+	pv.filePath = filePath
+	return pv, nil
+}
+
+// Upgrade convets the OldFilePV to the new FilePV, separating the immutable and mutable components,
+// and persisting them to the keyFilePath and stateFilePath, respectively.
+// It renames the original file by adding ".bak".
+func (oldFilePV *OldFilePV) Upgrade(keyFilePath, stateFilePath string) *FilePV {
+	privKey := oldFilePV.PrivKey
+	pvKey := FilePVKey{
+		PrivKey:  privKey,
+		PubKey:   privKey.PubKey(),
+		Address:  privKey.PubKey().Address(),
+		filePath: keyFilePath,
+	}
+
+	pvState := FilePVLastSignState{
+		Height:    oldFilePV.LastHeight,
+		Round:     oldFilePV.LastRound,
+		Step:      oldFilePV.LastStep,
+		Signature: oldFilePV.LastSignature,
+		SignBytes: oldFilePV.LastSignBytes,
+		filePath:  stateFilePath,
+	}
+
+	// Save the new PV files
+	pv := &FilePV{
+		Key:           pvKey,
+		LastSignState: pvState,
+	}
+	pv.Save()
+
+	// Rename the old PV file
+	err := os.Rename(oldFilePV.filePath, oldFilePV.filePath+".bak")
+	if err != nil {
+		panic(err)
+	}
+	return pv
+}

privval/old_priv_validator_test.go

@@ -0,0 +1,77 @@
+package privval_test
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/tendermint/tendermint/privval"
+)
+
+const oldPrivvalContent = `{
+  "address": "1D8089FAFDFAE4A637F3D616E17B92905FA2D91D",
+  "pub_key": {
+    "type": "tendermint/PubKeyEd25519",
+    "value": "r3Yg2AhDZ745CNTpavsGU+mRZ8WpRXqoJuyqjN8mJq0="
+  },
+  "last_height": "5",
+  "last_round": "0",
+  "last_step": 3,
+  "last_signature": "CTr7b9ZQlrJJf+12rPl5t/YSCUc/KqV7jQogCfFJA24e7hof69X6OMT7eFLVQHyodPjD/QTA298XHV5ejxInDQ==",
+  "last_signbytes": "750802110500000000000000220B08B398F3E00510F48DA6402A480A20FC258973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4CAD312240A20C971B286ACB8AAA6FCA0365EB0A660B189EDC08B46B5AF2995DEFA51A28D215B10013211746573742D636861696E2D533245415533",
+  "priv_key": {
+    "type": "tendermint/PrivKeyEd25519",
+    "value": "7MwvTGEWWjsYwjn2IpRb+GYsWi9nnFsw8jPLLY1UtP6vdiDYCENnvjkI1Olq+wZT6ZFnxalFeqgm7KqM3yYmrQ=="
+  }
+}`
+
+func TestLoadAndUpgrade(t *testing.T) {
+
+	oldFilePath := initTmpOldFile(t)
+	defer os.Remove(oldFilePath)
+	newStateFile, err := ioutil.TempFile("", "priv_validator_state*.json")
+	defer os.Remove(newStateFile.Name())
+	require.NoError(t, err)
+	newKeyFile, err := ioutil.TempFile("", "priv_validator_key*.json")
+	defer os.Remove(newKeyFile.Name())
+	require.NoError(t, err)
+
+	oldPV, err := privval.LoadOldFilePV(oldFilePath)
+	assert.NoError(t, err)
+	newPV := oldPV.Upgrade(newKeyFile.Name(), newStateFile.Name())
+
+	assertEqualPV(t, oldPV, newPV)
+	assert.NoError(t, err)
+	upgradedPV := privval.LoadFilePV(newKeyFile.Name(), newStateFile.Name())
+	assertEqualPV(t, oldPV, upgradedPV)
+	oldPV, err = privval.LoadOldFilePV(oldFilePath + ".bak")
+	require.NoError(t, err)
+	assertEqualPV(t, oldPV, upgradedPV)
+}
+
+func assertEqualPV(t *testing.T, oldPV *privval.OldFilePV, newPV *privval.FilePV) {
+	assert.Equal(t, oldPV.Address, newPV.Key.Address)
+	assert.Equal(t, oldPV.Address, newPV.GetAddress())
+	assert.Equal(t, oldPV.PubKey, newPV.Key.PubKey)
+	assert.Equal(t, oldPV.PubKey, newPV.GetPubKey())
+	assert.Equal(t, oldPV.PrivKey, newPV.Key.PrivKey)
+
+	assert.Equal(t, oldPV.LastHeight, newPV.LastSignState.Height)
+	assert.Equal(t, oldPV.LastRound, newPV.LastSignState.Round)
+	assert.Equal(t, oldPV.LastSignature, newPV.LastSignState.Signature)
+	assert.Equal(t, oldPV.LastSignBytes, newPV.LastSignState.SignBytes)
+	assert.Equal(t, oldPV.LastStep, newPV.LastSignState.Step)
+}
+
+func initTmpOldFile(t *testing.T) string {
+	tmpfile, err := ioutil.TempFile("", "priv_validator_*.json")
+	require.NoError(t, err)
+	t.Logf("created test file %s", tmpfile.Name())
+	_, err = tmpfile.WriteString(oldPrivvalContent)
+	require.NoError(t, err)
+
+	return tmpfile.Name()
+}

privval/priv_validator.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"sync"
 	"time"
 
 	"github.com/tendermint/tendermint/crypto"
@@ -35,100 +34,90 @@ func voteToStep(vote *types.Vote) int8 {
 	}
 }
 
-// FilePV implements PrivValidator using data persisted to disk
+//-------------------------------------------------------------------------------
-// to prevent double signing.
+
-// NOTE: the directory containing the pv.filePath must already exist.
+// FilePVKey stores the immutable part of PrivValidator.
-// It includes the LastSignature and LastSignBytes so we don't lose the signature
+type FilePVKey struct {
-// if the process crashes after signing but before the resulting consensus message is processed.
-type FilePV struct {
 	Address types.Address  `json:"address"`
 	PubKey  crypto.PubKey  `json:"pub_key"`
-	LastHeight    int64          `json:"last_height"`
-	LastRound     int            `json:"last_round"`
-	LastStep      int8           `json:"last_step"`
-	LastSignature []byte         `json:"last_signature,omitempty"`
-	LastSignBytes cmn.HexBytes   `json:"last_signbytes,omitempty"`
 	PrivKey crypto.PrivKey `json:"priv_key"`
 
-	// For persistence.
-	// Overloaded for testing.
 	filePath string
-	mtx      sync.Mutex
 }
 
-// GetAddress returns the address of the validator.
+// Save persists the FilePVKey to its filePath.
-// Implements PrivValidator.
+func (pvKey FilePVKey) Save() {
-func (pv *FilePV) GetAddress() types.Address {
+	outFile := pvKey.filePath
-	return pv.Address
-}
-
-// GetPubKey returns the public key of the validator.
-// Implements PrivValidator.
-func (pv *FilePV) GetPubKey() crypto.PubKey {
-	return pv.PubKey
-}
-
-// GenFilePV generates a new validator with randomly generated private key
-// and sets the filePath, but does not call Save().
-func GenFilePV(filePath string) *FilePV {
-	privKey := ed25519.GenPrivKey()
-	return &FilePV{
-		Address:  privKey.PubKey().Address(),
-		PubKey:   privKey.PubKey(),
-		PrivKey:  privKey,
-		LastStep: stepNone,
-		filePath: filePath,
-	}
-}
-
-// LoadFilePV loads a FilePV from the filePath.  The FilePV handles double
-// signing prevention by persisting data to the filePath.  If the filePath does
-// not exist, the FilePV must be created manually and saved.
-func LoadFilePV(filePath string) *FilePV {
-	pvJSONBytes, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		cmn.Exit(err.Error())
-	}
-	pv := &FilePV{}
-	err = cdc.UnmarshalJSON(pvJSONBytes, &pv)
-	if err != nil {
-		cmn.Exit(fmt.Sprintf("Error reading PrivValidator from %v: %v\n", filePath, err))
-	}
-
-	// overwrite pubkey and address for convenience
-	pv.PubKey = pv.PrivKey.PubKey()
-	pv.Address = pv.PubKey.Address()
-
-	pv.filePath = filePath
-	return pv
-}
-
-// LoadOrGenFilePV loads a FilePV from the given filePath
-// or else generates a new one and saves it to the filePath.
-func LoadOrGenFilePV(filePath string) *FilePV {
-	var pv *FilePV
-	if cmn.FileExists(filePath) {
-		pv = LoadFilePV(filePath)
-	} else {
-		pv = GenFilePV(filePath)
-		pv.Save()
-	}
-	return pv
-}
-
-// Save persists the FilePV to disk.
-func (pv *FilePV) Save() {
-	pv.mtx.Lock()
-	defer pv.mtx.Unlock()
-	pv.save()
-}
-
-func (pv *FilePV) save() {
-	outFile := pv.filePath
 	if outFile == "" {
-		panic("Cannot save PrivValidator: filePath not set")
+		panic("Cannot save PrivValidator key: filePath not set")
 	}
-	jsonBytes, err := cdc.MarshalJSONIndent(pv, "", "  ")
+
+	jsonBytes, err := cdc.MarshalJSONIndent(pvKey, "", "  ")
+	if err != nil {
+		panic(err)
+	}
+	err = cmn.WriteFileAtomic(outFile, jsonBytes, 0600)
+	if err != nil {
+		panic(err)
+	}
+
+}
+
+//-------------------------------------------------------------------------------
+
+// FilePVLastSignState stores the mutable part of PrivValidator.
+type FilePVLastSignState struct {
+	Height    int64        `json:"height"`
+	Round     int          `json:"round"`
+	Step      int8         `json:"step"`
+	Signature []byte       `json:"signature,omitempty"`
+	SignBytes cmn.HexBytes `json:"signbytes,omitempty"`
+
+	filePath string
+}
+
+// CheckHRS checks the given height, round, step (HRS) against that of the
+// FilePVLastSignState. It returns an error if the arguments constitute a regression,
+// or if they match but the SignBytes are empty.
+// The returned boolean indicates whether the last Signature should be reused -
+// it returns true if the HRS matches the arguments and the SignBytes are not empty (indicating
+// we have already signed for this HRS, and can reuse the existing signature).
+// It panics if the HRS matches the arguments, there's a SignBytes, but no Signature.
+func (lss *FilePVLastSignState) CheckHRS(height int64, round int, step int8) (bool, error) {
+
+	if lss.Height > height {
+		return false, errors.New("Height regression")
+	}
+
+	if lss.Height == height {
+		if lss.Round > round {
+			return false, errors.New("Round regression")
+		}
+
+		if lss.Round == round {
+			if lss.Step > step {
+				return false, errors.New("Step regression")
+			} else if lss.Step == step {
+				if lss.SignBytes != nil {
+					if lss.Signature == nil {
+						panic("pv: Signature is nil but SignBytes is not!")
+					}
+					return true, nil
+				}
+				return false, errors.New("No SignBytes found")
+			}
+		}
+	}
+	return false, nil
+}
+
+// Save persists the FilePvLastSignState to its filePath.
+func (lss *FilePVLastSignState) Save() {
+	outFile := lss.filePath
+	if outFile == "" {
+		panic("Cannot save FilePVLastSignState: filePath not set")
+	}
+	jsonBytes, err := cdc.MarshalJSONIndent(lss, "", "  ")
 	if err != nil {
 		panic(err)
 	}
@@ -138,23 +127,115 @@ func (pv *FilePV) save() {
 	}
 }
 
-// Reset resets all fields in the FilePV.
+//-------------------------------------------------------------------------------
-// NOTE: Unsafe!
+
-func (pv *FilePV) Reset() {
+// FilePV implements PrivValidator using data persisted to disk
-	var sig []byte
+// to prevent double signing.
-	pv.LastHeight = 0
+// NOTE: the directories containing pv.Key.filePath and pv.LastSignState.filePath must already exist.
-	pv.LastRound = 0
+// It includes the LastSignature and LastSignBytes so we don't lose the signature
-	pv.LastStep = 0
+// if the process crashes after signing but before the resulting consensus message is processed.
-	pv.LastSignature = sig
+type FilePV struct {
-	pv.LastSignBytes = nil
+	Key           FilePVKey
+	LastSignState FilePVLastSignState
+}
+
+// GenFilePV generates a new validator with randomly generated private key
+// and sets the filePaths, but does not call Save().
+func GenFilePV(keyFilePath, stateFilePath string) *FilePV {
+	privKey := ed25519.GenPrivKey()
+
+	return &FilePV{
+		Key: FilePVKey{
+			Address:  privKey.PubKey().Address(),
+			PubKey:   privKey.PubKey(),
+			PrivKey:  privKey,
+			filePath: keyFilePath,
+		},
+		LastSignState: FilePVLastSignState{
+			Step:     stepNone,
+			filePath: stateFilePath,
+		},
+	}
+}
+
+// LoadFilePV loads a FilePV from the filePaths.  The FilePV handles double
+// signing prevention by persisting data to the stateFilePath.  If either file path
+// does not exist, the program will exit.
+func LoadFilePV(keyFilePath, stateFilePath string) *FilePV {
+	return loadFilePV(keyFilePath, stateFilePath, true)
+}
+
+// LoadFilePVEmptyState loads a FilePV from the given keyFilePath, with an empty LastSignState.
+// If the keyFilePath does not exist, the program will exit.
+func LoadFilePVEmptyState(keyFilePath, stateFilePath string) *FilePV {
+	return loadFilePV(keyFilePath, stateFilePath, false)
+}
+
+// If loadState is true, we load from the stateFilePath. Otherwise, we use an empty LastSignState.
+func loadFilePV(keyFilePath, stateFilePath string, loadState bool) *FilePV {
+	keyJSONBytes, err := ioutil.ReadFile(keyFilePath)
+	if err != nil {
+		cmn.Exit(err.Error())
+	}
+	pvKey := FilePVKey{}
+	err = cdc.UnmarshalJSON(keyJSONBytes, &pvKey)
+	if err != nil {
+		cmn.Exit(fmt.Sprintf("Error reading PrivValidator key from %v: %v\n", keyFilePath, err))
+	}
+
+	// overwrite pubkey and address for convenience
+	pvKey.PubKey = pvKey.PrivKey.PubKey()
+	pvKey.Address = pvKey.PubKey.Address()
+	pvKey.filePath = keyFilePath
+
+	pvState := FilePVLastSignState{}
+	if loadState {
+		stateJSONBytes, err := ioutil.ReadFile(stateFilePath)
+		if err != nil {
+			cmn.Exit(err.Error())
+		}
+		err = cdc.UnmarshalJSON(stateJSONBytes, &pvState)
+		if err != nil {
+			cmn.Exit(fmt.Sprintf("Error reading PrivValidator state from %v: %v\n", stateFilePath, err))
+		}
+	}
+
+	pvState.filePath = stateFilePath
+
+	return &FilePV{
+		Key:           pvKey,
+		LastSignState: pvState,
+	}
+}
+
+// LoadOrGenFilePV loads a FilePV from the given filePaths
+// or else generates a new one and saves it to the filePaths.
+func LoadOrGenFilePV(keyFilePath, stateFilePath string) *FilePV {
+	var pv *FilePV
+	if cmn.FileExists(keyFilePath) {
+		pv = LoadFilePV(keyFilePath, stateFilePath)
+	} else {
+		pv = GenFilePV(keyFilePath, stateFilePath)
 		pv.Save()
 	}
+	return pv
+}
+
+// GetAddress returns the address of the validator.
+// Implements PrivValidator.
+func (pv *FilePV) GetAddress() types.Address {
+	return pv.Key.Address
+}
+
+// GetPubKey returns the public key of the validator.
+// Implements PrivValidator.
+func (pv *FilePV) GetPubKey() crypto.PubKey {
+	return pv.Key.PubKey
+}
 
 // SignVote signs a canonical representation of the vote, along with the
 // chainID. Implements PrivValidator.
 func (pv *FilePV) SignVote(chainID string, vote *types.Vote) error {
-	pv.mtx.Lock()
-	defer pv.mtx.Unlock()
 	if err := pv.signVote(chainID, vote); err != nil {
 		return fmt.Errorf("Error signing vote: %v", err)
 	}
@@ -164,65 +245,63 @@ func (pv *FilePV) SignVote(chainID string, vote *types.Vote) error {
 // SignProposal signs a canonical representation of the proposal, along with
 // the chainID. Implements PrivValidator.
 func (pv *FilePV) SignProposal(chainID string, proposal *types.Proposal) error {
-	pv.mtx.Lock()
-	defer pv.mtx.Unlock()
 	if err := pv.signProposal(chainID, proposal); err != nil {
 		return fmt.Errorf("Error signing proposal: %v", err)
 	}
 	return nil
 }
 
-// returns error if HRS regression or no LastSignBytes. returns true if HRS is unchanged
+// Save persists the FilePV to disk.
-func (pv *FilePV) checkHRS(height int64, round int, step int8) (bool, error) {
+func (pv *FilePV) Save() {
-	if pv.LastHeight > height {
+	pv.Key.Save()
-		return false, errors.New("Height regression")
+	pv.LastSignState.Save()
 }
 
-	if pv.LastHeight == height {
+// Reset resets all fields in the FilePV.
-		if pv.LastRound > round {
+// NOTE: Unsafe!
-			return false, errors.New("Round regression")
+func (pv *FilePV) Reset() {
+	var sig []byte
+	pv.LastSignState.Height = 0
+	pv.LastSignState.Round = 0
+	pv.LastSignState.Step = 0
+	pv.LastSignState.Signature = sig
+	pv.LastSignState.SignBytes = nil
+	pv.Save()
 }
 
-		if pv.LastRound == round {
+// String returns a string representation of the FilePV.
-			if pv.LastStep > step {
+func (pv *FilePV) String() string {
-				return false, errors.New("Step regression")
+	return fmt.Sprintf("PrivValidator{%v LH:%v, LR:%v, LS:%v}", pv.GetAddress(), pv.LastSignState.Height, pv.LastSignState.Round, pv.LastSignState.Step)
-			} else if pv.LastStep == step {
-				if pv.LastSignBytes != nil {
-					if pv.LastSignature == nil {
-						panic("pv: LastSignature is nil but LastSignBytes is not!")
-					}
-					return true, nil
-				}
-				return false, errors.New("No LastSignature found")
-			}
-		}
-	}
-	return false, nil
 }
 
+//------------------------------------------------------------------------------------
+
 // signVote checks if the vote is good to sign and sets the vote signature.
 // It may need to set the timestamp as well if the vote is otherwise the same as
 // a previously signed vote (ie. we crashed after signing but before the vote hit the WAL).
 func (pv *FilePV) signVote(chainID string, vote *types.Vote) error {
 	height, round, step := vote.Height, vote.Round, voteToStep(vote)
-	signBytes := vote.SignBytes(chainID)
 
-	sameHRS, err := pv.checkHRS(height, round, step)
+	lss := pv.LastSignState
+
+	sameHRS, err := lss.CheckHRS(height, round, step)
 	if err != nil {
 		return err
 	}
 
+	signBytes := vote.SignBytes(chainID)
+
 	// We might crash before writing to the wal,
 	// causing us to try to re-sign for the same HRS.
 	// If signbytes are the same, use the last signature.
 	// If they only differ by timestamp, use last timestamp and signature
 	// Otherwise, return error
 	if sameHRS {
-		if bytes.Equal(signBytes, pv.LastSignBytes) {
+		if bytes.Equal(signBytes, lss.SignBytes) {
-			vote.Signature = pv.LastSignature
+			vote.Signature = lss.Signature
-		} else if timestamp, ok := checkVotesOnlyDifferByTimestamp(pv.LastSignBytes, signBytes); ok {
+		} else if timestamp, ok := checkVotesOnlyDifferByTimestamp(lss.SignBytes, signBytes); ok {
 			vote.Timestamp = timestamp
-			vote.Signature = pv.LastSignature
+			vote.Signature = lss.Signature
 		} else {
 			err = fmt.Errorf("Conflicting data")
 		}
@@ -230,7 +309,7 @@ func (pv *FilePV) signVote(chainID string, vote *types.Vote) error {
 	}
 
 	// It passed the checks. Sign the vote
-	sig, err := pv.PrivKey.Sign(signBytes)
+	sig, err := pv.Key.PrivKey.Sign(signBytes)
 	if err != nil {
 		return err
 	}
@@ -244,24 +323,27 @@ func (pv *FilePV) signVote(chainID string, vote *types.Vote) error {
 // a previously signed proposal ie. we crashed after signing but before the proposal hit the WAL).
 func (pv *FilePV) signProposal(chainID string, proposal *types.Proposal) error {
 	height, round, step := proposal.Height, proposal.Round, stepPropose
-	signBytes := proposal.SignBytes(chainID)
 
-	sameHRS, err := pv.checkHRS(height, round, step)
+	lss := pv.LastSignState
+
+	sameHRS, err := lss.CheckHRS(height, round, step)
 	if err != nil {
 		return err
 	}
 
+	signBytes := proposal.SignBytes(chainID)
+
 	// We might crash before writing to the wal,
 	// causing us to try to re-sign for the same HRS.
 	// If signbytes are the same, use the last signature.
 	// If they only differ by timestamp, use last timestamp and signature
 	// Otherwise, return error
 	if sameHRS {
-		if bytes.Equal(signBytes, pv.LastSignBytes) {
+		if bytes.Equal(signBytes, lss.SignBytes) {
-			proposal.Signature = pv.LastSignature
+			proposal.Signature = lss.Signature
-		} else if timestamp, ok := checkProposalsOnlyDifferByTimestamp(pv.LastSignBytes, signBytes); ok {
+		} else if timestamp, ok := checkProposalsOnlyDifferByTimestamp(lss.SignBytes, signBytes); ok {
 			proposal.Timestamp = timestamp
-			proposal.Signature = pv.LastSignature
+			proposal.Signature = lss.Signature
 		} else {
 			err = fmt.Errorf("Conflicting data")
 		}
@@ -269,7 +351,7 @@ func (pv *FilePV) signProposal(chainID string, proposal *types.Proposal) error {
 	}
 
 	// It passed the checks. Sign the proposal
-	sig, err := pv.PrivKey.Sign(signBytes)
+	sig, err := pv.Key.PrivKey.Sign(signBytes)
 	if err != nil {
 		return err
 	}
@@ -282,20 +364,15 @@ func (pv *FilePV) signProposal(chainID string, proposal *types.Proposal) error {
 func (pv *FilePV) saveSigned(height int64, round int, step int8,
 	signBytes []byte, sig []byte) {
 
-	pv.LastHeight = height
+	pv.LastSignState.Height = height
-	pv.LastRound = round
+	pv.LastSignState.Round = round
-	pv.LastStep = step
+	pv.LastSignState.Step = step
-	pv.LastSignature = sig
+	pv.LastSignState.Signature = sig
-	pv.LastSignBytes = signBytes
+	pv.LastSignState.SignBytes = signBytes
-	pv.save()
+	pv.LastSignState.Save()
 }
 
-// String returns a string representation of the FilePV.
+//-----------------------------------------------------------------------------------------
-func (pv *FilePV) String() string {
-	return fmt.Sprintf("PrivValidator{%v LH:%v, LR:%v, LS:%v}", pv.GetAddress(), pv.LastHeight, pv.LastRound, pv.LastStep)
-}
-
-//-------------------------------------
 
 // returns the timestamp from the lastSignBytes.
 // returns true if the only difference in the votes is their timestamp.

privval/priv_validator_test.go

@@ -18,36 +18,100 @@ import (
 func TestGenLoadValidator(t *testing.T) {
 	assert := assert.New(t)
 
-	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 	require.Nil(t, err)
-	privVal := GenFilePV(tempFile.Name())
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+	require.Nil(t, err)
+
+	privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
 
 	height := int64(100)
-	privVal.LastHeight = height
+	privVal.LastSignState.Height = height
 	privVal.Save()
 	addr := privVal.GetAddress()
 
-	privVal = LoadFilePV(tempFile.Name())
+	privVal = LoadFilePV(tempKeyFile.Name(), tempStateFile.Name())
 	assert.Equal(addr, privVal.GetAddress(), "expected privval addr to be the same")
-	assert.Equal(height, privVal.LastHeight, "expected privval.LastHeight to have been saved")
+	assert.Equal(height, privVal.LastSignState.Height, "expected privval.LastHeight to have been saved")
+}
+
+func TestResetValidator(t *testing.T) {
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
+	require.Nil(t, err)
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+	require.Nil(t, err)
+
+	privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
+	emptyState := FilePVLastSignState{filePath: tempStateFile.Name()}
+
+	// new priv val has empty state
+	assert.Equal(t, privVal.LastSignState, emptyState)
+
+	// test vote
+	height, round := int64(10), 1
+	voteType := byte(types.PrevoteType)
+	blockID := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{}}
+	vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID)
+	err = privVal.SignVote("mychainid", vote)
+	assert.NoError(t, err, "expected no error signing vote")
+
+	// priv val after signing is not same as empty
+	assert.NotEqual(t, privVal.LastSignState, emptyState)
+
+	// priv val after reset is same as empty
+	privVal.Reset()
+	assert.Equal(t, privVal.LastSignState, emptyState)
 }
 
 func TestLoadOrGenValidator(t *testing.T) {
 	assert := assert.New(t)
 
-	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 	require.Nil(t, err)
-	tempFilePath := tempFile.Name()
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
-	if err := os.Remove(tempFilePath); err != nil {
+	require.Nil(t, err)
+
+	tempKeyFilePath := tempKeyFile.Name()
+	if err := os.Remove(tempKeyFilePath); err != nil {
 		t.Error(err)
 	}
-	privVal := LoadOrGenFilePV(tempFilePath)
+	tempStateFilePath := tempStateFile.Name()
+	if err := os.Remove(tempStateFilePath); err != nil {
+		t.Error(err)
+	}
+
+	privVal := LoadOrGenFilePV(tempKeyFilePath, tempStateFilePath)
 	addr := privVal.GetAddress()
-	privVal = LoadOrGenFilePV(tempFilePath)
+	privVal = LoadOrGenFilePV(tempKeyFilePath, tempStateFilePath)
 	assert.Equal(addr, privVal.GetAddress(), "expected privval addr to be the same")
 }
 
-func TestUnmarshalValidator(t *testing.T) {
+func TestUnmarshalValidatorState(t *testing.T) {
+	assert, require := assert.New(t), require.New(t)
+
+	// create some fixed values
+	serialized := `{
+		"height": "1",
+		"round": "1",
+		"step": 1
+	}`
+
+	val := FilePVLastSignState{}
+	err := cdc.UnmarshalJSON([]byte(serialized), &val)
+	require.Nil(err, "%+v", err)
+
+	// make sure the values match
+	assert.EqualValues(val.Height, 1)
+	assert.EqualValues(val.Round, 1)
+	assert.EqualValues(val.Step, 1)
+
+	// export it and make sure it is the same
+	out, err := cdc.MarshalJSON(val)
+	require.Nil(err, "%+v", err)
+	assert.JSONEq(serialized, string(out))
+}
+
+func TestUnmarshalValidatorKey(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	// create some fixed values
@@ -67,22 +131,19 @@ func TestUnmarshalValidator(t *testing.T) {
     "type": "tendermint/PubKeyEd25519",
     "value": "%s"
   },
-  "last_height": "0",
-  "last_round": "0",
-  "last_step": 0,
   "priv_key": {
     "type": "tendermint/PrivKeyEd25519",
     "value": "%s"
   }
 }`, addr, pubB64, privB64)
 
-	val := FilePV{}
+	val := FilePVKey{}
 	err := cdc.UnmarshalJSON([]byte(serialized), &val)
 	require.Nil(err, "%+v", err)
 
 	// make sure the values match
-	assert.EqualValues(addr, val.GetAddress())
+	assert.EqualValues(addr, val.Address)
-	assert.EqualValues(pubKey, val.GetPubKey())
+	assert.EqualValues(pubKey, val.PubKey)
 	assert.EqualValues(privKey, val.PrivKey)
 
 	// export it and make sure it is the same
@@ -94,9 +155,12 @@ func TestUnmarshalValidator(t *testing.T) {
 func TestSignVote(t *testing.T) {
 	assert := assert.New(t)
 
-	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 	require.Nil(t, err)
-	privVal := GenFilePV(tempFile.Name())
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+	require.Nil(t, err)
+
+	privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
 
 	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{}}
 	block2 := types.BlockID{[]byte{3, 2, 1}, types.PartSetHeader{}}
@@ -104,7 +168,7 @@ func TestSignVote(t *testing.T) {
 	voteType := byte(types.PrevoteType)
 
 	// sign a vote for first time
-	vote := newVote(privVal.Address, 0, height, round, voteType, block1)
+	vote := newVote(privVal.Key.Address, 0, height, round, voteType, block1)
 	err = privVal.SignVote("mychainid", vote)
 	assert.NoError(err, "expected no error signing vote")
 
@@ -114,10 +178,10 @@ func TestSignVote(t *testing.T) {
 
 	// now try some bad votes
 	cases := []*types.Vote{
-		newVote(privVal.Address, 0, height, round-1, voteType, block1),   // round regression
+		newVote(privVal.Key.Address, 0, height, round-1, voteType, block1),   // round regression
-		newVote(privVal.Address, 0, height-1, round, voteType, block1),   // height regression
+		newVote(privVal.Key.Address, 0, height-1, round, voteType, block1),   // height regression
-		newVote(privVal.Address, 0, height-2, round+4, voteType, block1), // height regression and different round
+		newVote(privVal.Key.Address, 0, height-2, round+4, voteType, block1), // height regression and different round
-		newVote(privVal.Address, 0, height, round, voteType, block2),     // different block
+		newVote(privVal.Key.Address, 0, height, round, voteType, block2),     // different block
 	}
 
 	for _, c := range cases {
@@ -136,9 +200,12 @@ func TestSignVote(t *testing.T) {
 func TestSignProposal(t *testing.T) {
 	assert := assert.New(t)
 
-	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 	require.Nil(t, err)
-	privVal := GenFilePV(tempFile.Name())
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+	require.Nil(t, err)
+
+	privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
 
 	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{5, []byte{1, 2, 3}}}
 	block2 := types.BlockID{[]byte{3, 2, 1}, types.PartSetHeader{10, []byte{3, 2, 1}}}
@@ -175,9 +242,12 @@ func TestSignProposal(t *testing.T) {
 }
 
 func TestDifferByTimestamp(t *testing.T) {
-	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	tempKeyFile, err := ioutil.TempFile("", "priv_validator_key_")
 	require.Nil(t, err)
-	privVal := GenFilePV(tempFile.Name())
+	tempStateFile, err := ioutil.TempFile("", "priv_validator_state_")
+	require.Nil(t, err)
+
+	privVal := GenFilePV(tempKeyFile.Name(), tempStateFile.Name())
 
 	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{5, []byte{1, 2, 3}}}
 	height, round := int64(10), 1
@@ -208,7 +278,7 @@ func TestDifferByTimestamp(t *testing.T) {
 	{
 		voteType := byte(types.PrevoteType)
 		blockID := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{}}
-		vote := newVote(privVal.Address, 0, height, round, voteType, blockID)
+		vote := newVote(privVal.Key.Address, 0, height, round, voteType, blockID)
 		err := privVal.SignVote("mychainid", vote)
 		assert.NoError(t, err, "expected no error signing vote")
 

rpc/test/helpers.go

@@ -119,8 +119,9 @@ func NewTendermint(app abci.Application) *nm.Node {
 	config := GetConfig()
 	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
 	logger = log.NewFilter(logger, log.AllowError())
-	pvFile := config.PrivValidatorFile()
+	pvKeyFile := config.PrivValidatorKeyFile()
-	pv := privval.LoadOrGenFilePV(pvFile)
+	pvKeyStateFile := config.PrivValidatorStateFile()
+	pv := privval.LoadOrGenFilePV(pvKeyFile, pvKeyStateFile)
 	papp := proxy.NewLocalClientCreator(app)
 	nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
 	if err != nil {

scripts/privValUpgrade.go

@@ -0,0 +1,41 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/privval"
+)
+
+var (
+	logger = log.NewTMLogger(log.NewSyncWriter(os.Stdout))
+)
+
+func main() {
+	args := os.Args[1:]
+	if len(args) != 3 {
+		fmt.Println("Expected three args: <old path> <new key path> <new state path>")
+		fmt.Println("Eg. ~/.tendermint/config/priv_validator.json ~/.tendermint/config/priv_validator_key.json ~/.tendermint/data/priv_validator_state.json")
+		os.Exit(1)
+	}
+	err := loadAndUpgrade(args[0], args[1], args[2])
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+}
+
+func loadAndUpgrade(oldPVPath, newPVKeyPath, newPVStatePath string) error {
+	oldPV, err := privval.LoadOldFilePV(oldPVPath)
+	if err != nil {
+		return fmt.Errorf("Error reading OldPrivValidator from %v: %v\n", oldPVPath, err)
+	}
+	logger.Info("Upgrading PrivValidator file",
+		"old", oldPVPath,
+		"newKey", newPVKeyPath,
+		"newState", newPVStatePath,
+	)
+	oldPV.Upgrade(newPVKeyPath, newPVStatePath)
+	return nil
+}

scripts/privValUpgrade_test.go

@@ -0,0 +1,121 @@
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/privval"
+)
+
+const oldPrivvalContent = `{
+  "address": "1D8089FAFDFAE4A637F3D616E17B92905FA2D91D",
+  "pub_key": {
+    "type": "tendermint/PubKeyEd25519",
+    "value": "r3Yg2AhDZ745CNTpavsGU+mRZ8WpRXqoJuyqjN8mJq0="
+  },
+  "last_height": "5",
+  "last_round": "0",
+  "last_step": 3,
+  "last_signature": "CTr7b9ZQlrJJf+12rPl5t/YSCUc/KqV7jQogCfFJA24e7hof69X6OMT7eFLVQHyodPjD/QTA298XHV5ejxInDQ==",
+  "last_signbytes": "750802110500000000000000220B08B398F3E00510F48DA6402A480A20FC258973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4CAD312240A20C971B286ACB8AAA6FCA0365EB0A660B189EDC08B46B5AF2995DEFA51A28D215B10013211746573742D636861696E2D533245415533",
+  "priv_key": {
+    "type": "tendermint/PrivKeyEd25519",
+    "value": "7MwvTGEWWjsYwjn2IpRb+GYsWi9nnFsw8jPLLY1UtP6vdiDYCENnvjkI1Olq+wZT6ZFnxalFeqgm7KqM3yYmrQ=="
+  }
+}`
+
+func TestLoadAndUpgrade(t *testing.T) {
+
+	oldFilePath := initTmpOldFile(t)
+	defer os.Remove(oldFilePath)
+	newStateFile, err := ioutil.TempFile("", "priv_validator_state*.json")
+	defer os.Remove(newStateFile.Name())
+	require.NoError(t, err)
+	newKeyFile, err := ioutil.TempFile("", "priv_validator_key*.json")
+	defer os.Remove(newKeyFile.Name())
+	require.NoError(t, err)
+	emptyOldFile, err := ioutil.TempFile("", "priv_validator_empty*.json")
+	require.NoError(t, err)
+	defer os.Remove(emptyOldFile.Name())
+
+	type args struct {
+		oldPVPath      string
+		newPVKeyPath   string
+		newPVStatePath string
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantErr   bool
+		wantPanic bool
+	}{
+		{"successful upgrade",
+			args{oldPVPath: oldFilePath, newPVKeyPath: newKeyFile.Name(), newPVStatePath: newStateFile.Name()},
+			false, false,
+		},
+		{"unsuccessful upgrade: empty old privval file",
+			args{oldPVPath: emptyOldFile.Name(), newPVKeyPath: newKeyFile.Name(), newPVStatePath: newStateFile.Name()},
+			true, false,
+		},
+		{"unsuccessful upgrade: invalid new paths (1/3)",
+			args{oldPVPath: oldFilePath, newPVKeyPath: "", newPVStatePath: newStateFile.Name()},
+			false, true,
+		},
+		{"unsuccessful upgrade: invalid new paths (2/3)",
+			args{oldPVPath: oldFilePath, newPVKeyPath: newKeyFile.Name(), newPVStatePath: ""},
+			false, true,
+		},
+		{"unsuccessful upgrade: invalid new paths (3/3)",
+			args{oldPVPath: oldFilePath, newPVKeyPath: "", newPVStatePath: ""},
+			false, true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// need to re-write the file everytime because upgrading renames it
+			err := ioutil.WriteFile(oldFilePath, []byte(oldPrivvalContent), 0600)
+			require.NoError(t, err)
+			if tt.wantPanic {
+				require.Panics(t, func() { loadAndUpgrade(tt.args.oldPVPath, tt.args.newPVKeyPath, tt.args.newPVStatePath) })
+			} else {
+				err = loadAndUpgrade(tt.args.oldPVPath, tt.args.newPVKeyPath, tt.args.newPVStatePath)
+				if tt.wantErr {
+					assert.Error(t, err)
+					fmt.Println("ERR", err)
+				} else {
+					assert.NoError(t, err)
+					upgradedPV := privval.LoadFilePV(tt.args.newPVKeyPath, tt.args.newPVStatePath)
+					oldPV, err := privval.LoadOldFilePV(tt.args.oldPVPath + ".bak")
+					require.NoError(t, err)
+
+					assert.Equal(t, oldPV.Address, upgradedPV.Key.Address)
+					assert.Equal(t, oldPV.Address, upgradedPV.GetAddress())
+					assert.Equal(t, oldPV.PubKey, upgradedPV.Key.PubKey)
+					assert.Equal(t, oldPV.PubKey, upgradedPV.GetPubKey())
+					assert.Equal(t, oldPV.PrivKey, upgradedPV.Key.PrivKey)
+
+					assert.Equal(t, oldPV.LastHeight, upgradedPV.LastSignState.Height)
+					assert.Equal(t, oldPV.LastRound, upgradedPV.LastSignState.Round)
+					assert.Equal(t, oldPV.LastSignature, upgradedPV.LastSignState.Signature)
+					assert.Equal(t, oldPV.LastSignBytes, upgradedPV.LastSignState.SignBytes)
+					assert.Equal(t, oldPV.LastStep, upgradedPV.LastSignState.Step)
+
+				}
+			}
+		})
+	}
+}
+
+func initTmpOldFile(t *testing.T) string {
+	tmpfile, err := ioutil.TempFile("", "priv_validator_*.json")
+	require.NoError(t, err)
+	t.Logf("created test file %s", tmpfile.Name())
+	_, err = tmpfile.WriteString(oldPrivvalContent)
+	require.NoError(t, err)
+
+	return tmpfile.Name()
+}

scripts/wire2amino.go

@@ -1,182 +0,0 @@
-package main
-
-import (
-	"encoding/json"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"time"
-
-	"github.com/tendermint/go-amino"
-	"github.com/tendermint/tendermint/crypto/ed25519"
-	cryptoAmino "github.com/tendermint/tendermint/crypto/encoding/amino"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-
-	"github.com/tendermint/tendermint/p2p"
-	"github.com/tendermint/tendermint/privval"
-	"github.com/tendermint/tendermint/types"
-)
-
-type GenesisValidator struct {
-	PubKey Data   `json:"pub_key"`
-	Power  int64  `json:"power"`
-	Name   string `json:"name"`
-}
-
-type Genesis struct {
-	GenesisTime     time.Time              `json:"genesis_time"`
-	ChainID         string                 `json:"chain_id"`
-	ConsensusParams *types.ConsensusParams `json:"consensus_params,omitempty"`
-	Validators      []GenesisValidator     `json:"validators"`
-	AppHash         cmn.HexBytes           `json:"app_hash"`
-	AppState        json.RawMessage        `json:"app_state,omitempty"`
-	AppOptions      json.RawMessage        `json:"app_options,omitempty"` // DEPRECATED
-}
-
-type NodeKey struct {
-	PrivKey Data `json:"priv_key"`
-}
-
-type PrivVal struct {
-	Address    cmn.HexBytes `json:"address"`
-	LastHeight int64        `json:"last_height"`
-	LastRound  int          `json:"last_round"`
-	LastStep   int8         `json:"last_step"`
-	PubKey     Data         `json:"pub_key"`
-	PrivKey    Data         `json:"priv_key"`
-}
-
-type Data struct {
-	Type string       `json:"type"`
-	Data cmn.HexBytes `json:"data"`
-}
-
-func convertNodeKey(cdc *amino.Codec, jsonBytes []byte) ([]byte, error) {
-	var nodeKey NodeKey
-	err := json.Unmarshal(jsonBytes, &nodeKey)
-	if err != nil {
-		return nil, err
-	}
-
-	var privKey ed25519.PrivKeyEd25519
-	copy(privKey[:], nodeKey.PrivKey.Data)
-
-	nodeKeyNew := p2p.NodeKey{privKey}
-
-	bz, err := cdc.MarshalJSON(nodeKeyNew)
-	if err != nil {
-		return nil, err
-	}
-	return bz, nil
-}
-
-func convertPrivVal(cdc *amino.Codec, jsonBytes []byte) ([]byte, error) {
-	var privVal PrivVal
-	err := json.Unmarshal(jsonBytes, &privVal)
-	if err != nil {
-		return nil, err
-	}
-
-	var privKey ed25519.PrivKeyEd25519
-	copy(privKey[:], privVal.PrivKey.Data)
-
-	var pubKey ed25519.PubKeyEd25519
-	copy(pubKey[:], privVal.PubKey.Data)
-
-	privValNew := privval.FilePV{
-		Address:    pubKey.Address(),
-		PubKey:     pubKey,
-		LastHeight: privVal.LastHeight,
-		LastRound:  privVal.LastRound,
-		LastStep:   privVal.LastStep,
-		PrivKey:    privKey,
-	}
-
-	bz, err := cdc.MarshalJSON(privValNew)
-	if err != nil {
-		return nil, err
-	}
-	return bz, nil
-}
-
-func convertGenesis(cdc *amino.Codec, jsonBytes []byte) ([]byte, error) {
-	var genesis Genesis
-	err := json.Unmarshal(jsonBytes, &genesis)
-	if err != nil {
-		return nil, err
-	}
-
-	genesisNew := types.GenesisDoc{
-		GenesisTime:     genesis.GenesisTime,
-		ChainID:         genesis.ChainID,
-		ConsensusParams: genesis.ConsensusParams,
-		// Validators
-		AppHash:  genesis.AppHash,
-		AppState: genesis.AppState,
-	}
-
-	if genesis.AppOptions != nil {
-		genesisNew.AppState = genesis.AppOptions
-	}
-
-	for _, v := range genesis.Validators {
-		var pubKey ed25519.PubKeyEd25519
-		copy(pubKey[:], v.PubKey.Data)
-		genesisNew.Validators = append(
-			genesisNew.Validators,
-			types.GenesisValidator{
-				PubKey: pubKey,
-				Power:  v.Power,
-				Name:   v.Name,
-			},
-		)
-
-	}
-
-	bz, err := cdc.MarshalJSON(genesisNew)
-	if err != nil {
-		return nil, err
-	}
-	return bz, nil
-}
-
-func main() {
-	cdc := amino.NewCodec()
-	cryptoAmino.RegisterAmino(cdc)
-
-	args := os.Args[1:]
-	if len(args) != 1 {
-		fmt.Println("Please specify a file to convert")
-		os.Exit(1)
-	}
-
-	filePath := args[0]
-	fileName := filepath.Base(filePath)
-
-	fileBytes, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		panic(err)
-	}
-
-	var bz []byte
-
-	switch fileName {
-	case "node_key.json":
-		bz, err = convertNodeKey(cdc, fileBytes)
-	case "priv_validator.json":
-		bz, err = convertPrivVal(cdc, fileBytes)
-	case "genesis.json":
-		bz, err = convertGenesis(cdc, fileBytes)
-	default:
-		fmt.Println("Expected file name to be in (node_key.json, priv_validator.json, genesis.json)")
-		os.Exit(1)
-	}
-
-	if err != nil {
-		panic(err)
-	}
-	fmt.Println(string(bz))
-
-}