privval: return errors on loadFilePV (#6185)

## Description

- return errors on `loadFilePv`

closes #6182

CHANGELOG_PENDING.md

@@ -72,6 +72,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [node] \#6059 Validate and complete genesis doc before saving to state store (@silasdavis)
 - [state] \#6067 Batch save state data (@githubsands & @cmwaters)
 - [libs/log] \#6174 Include timestamp (`ts` field; `time.RFC3339Nano` format) in JSON logger output (@melekes)
+- [privval/file] \#6185 Return error on `LoadFilePV`, `LoadFilePVEmptyState`. Allows for better programmatic control of Tendermint. 
 
 ### BUG FIXES
 

cmd/priv_val_server/main.go

@@ -64,7 +64,11 @@ func main() {
 		"rootCA", *rootCA,
 	)
 
-	pv := privval.LoadFilePV(*privValKeyPath, *privValStatePath)
+	pv, err := privval.LoadFilePV(*privValKeyPath, *privValStatePath)
+	if err != nil {
+		fmt.Fprint(os.Stderr, err)
+		os.Exit(1)
+	}
 
 	opts := []grpc.ServerOption{}
 	if !*insecure {

cmd/tendermint/commands/init.go

@@ -43,7 +43,11 @@ func initFilesWithConfig(config *cfg.Config) error {
 		err error
 	)
 	if tmos.FileExists(privValKeyFile) {
-		pv = privval.LoadFilePV(privValKeyFile, privValStateFile)
+		pv, err = privval.LoadFilePV(privValKeyFile, privValStateFile)
+		if err != nil {
+			return err
+		}
+
 		logger.Info("Found private validator", "keyFile", privValKeyFile,
 			"stateFile", privValStateFile)
 	} else {

cmd/tendermint/commands/reset_priv_validator.go

@@ -17,7 +17,7 @@ var ResetAllCmd = &cobra.Command{
 	Use:     "unsafe-reset-all",
 	Aliases: []string{"unsafe_reset_all"},
 	Short:   "(unsafe) Remove all the data and WAL, reset this node's validator to genesis state",
-	Run:     resetAll,
+	RunE:    resetAll,
 	PreRun:  deprecateSnakeCase,
 }
 
@@ -34,26 +34,26 @@ var ResetPrivValidatorCmd = &cobra.Command{
 	Use:     "unsafe-reset-priv-validator",
 	Aliases: []string{"unsafe_reset_priv_validator"},
 	Short:   "(unsafe) Reset this node's validator to genesis state",
-	Run:     resetPrivValidator,
+	RunE:    resetPrivValidator,
 	PreRun:  deprecateSnakeCase,
 }
 
 // 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.PrivValidatorKeyFile(),
+func resetAll(cmd *cobra.Command, args []string) error {
+	return 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.PrivValidatorKeyFile(), config.PrivValidatorStateFile(), logger)
+func resetPrivValidator(cmd *cobra.Command, args []string) error {
+	return resetFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile(), logger)
 }
 
 // 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, privValKeyFile, privValStateFile string, logger log.Logger) {
+func ResetAll(dbDir, addrBookFile, privValKeyFile, privValStateFile string, logger log.Logger) error {
 	if keepAddrBook {
 		logger.Info("The address book remains intact")
 	} else {
@@ -68,24 +68,28 @@ func ResetAll(dbDir, addrBookFile, privValKeyFile, privValStateFile string, logg
 	if err := tmos.EnsureDir(dbDir, 0700); err != nil {
 		logger.Error("unable to recreate dbDir", "err", err)
 	}
-	resetFilePV(privValKeyFile, privValStateFile, logger)
+	return resetFilePV(privValKeyFile, privValStateFile, logger)
 }
 
-func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) {
+func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) error {
 	if _, err := os.Stat(privValKeyFile); err == nil {
-		pv := privval.LoadFilePVEmptyState(privValKeyFile, privValStateFile)
+		pv, err := privval.LoadFilePVEmptyState(privValKeyFile, privValStateFile)
+		if err != nil {
+			return err
+		}
 		pv.Reset()
 		logger.Info("Reset private validator file to genesis state", "keyFile", privValKeyFile,
 			"stateFile", privValStateFile)
 	} else {
 		pv, err := privval.GenFilePV(privValKeyFile, privValStateFile, keyType)
 		if err != nil {
-			panic(err)
+			return err
 		}
 		pv.Save()
 		logger.Info("Generated private validator file", "keyFile", privValKeyFile,
 			"stateFile", privValStateFile)
 	}
+	return nil
 }
 
 func removeAddrBook(addrBookFile string, logger log.Logger) {

cmd/tendermint/commands/show_validator.go

@@ -47,7 +47,10 @@ func showValidator(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("private validator file %s does not exist", keyFilePath)
 		}
 
-		pv := privval.LoadFilePV(keyFilePath, config.PrivValidatorStateFile())
+		pv, err := privval.LoadFilePV(keyFilePath, config.PrivValidatorStateFile())
+		if err != nil {
+			return err
+		}
 
 		pubKey, err = pv.GetPubKey()
 		if err != nil {

cmd/tendermint/commands/testnet.go

@@ -144,7 +144,10 @@ func testnetFiles(cmd *cobra.Command, args []string) error {
 
 		pvKeyFile := filepath.Join(nodeDir, config.BaseConfig.PrivValidatorKey)
 		pvStateFile := filepath.Join(nodeDir, config.BaseConfig.PrivValidatorState)
-		pv := privval.LoadFilePV(pvKeyFile, pvStateFile)
+		pv, err := privval.LoadFilePV(pvKeyFile, pvStateFile)
+		if err != nil {
+			return err
+		}
 
 		pubKey, err := pv.GetPubKey()
 		if err != nil {

consensus/replay_test.go

@@ -672,7 +672,8 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin
 		walFile := tempWALWithData(walBody)
 		config.Consensus.SetWalFile(walFile)
 
-		privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+		privVal, err := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+		require.NoError(t, err)
 
 		wal, err := NewWAL(walFile)
 		require.NoError(t, err)
@@ -884,7 +885,8 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) {
 	//		- 0x03
 	config := ResetConfig("handshake_test_")
 	t.Cleanup(func() { os.RemoveAll(config.RootDir) })
-	privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+	privVal, err := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+	require.NoError(t, err)
 	const appVersion = 0x0
 	pubKey, err := privVal.GetPubKey()
 	require.NoError(t, err)
@@ -1220,7 +1222,8 @@ func TestHandshakeUpdatesValidators(t *testing.T) {
 	config := ResetConfig("handshake_test_")
 	t.Cleanup(func() { _ = os.RemoveAll(config.RootDir) })
 
-	privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+	privVal, err := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
+	require.NoError(t, err)
 	pubKey, err := privVal.GetPubKey()
 	require.NoError(t, err)
 	stateDB, state, store := stateAndStore(config, pubKey, 0x0)

privval/file.go

@@ -185,26 +185,26 @@ func GenFilePV(keyFilePath, stateFilePath, keyType string) (*FilePV, error) {
 // 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 {
+func LoadFilePV(keyFilePath, stateFilePath string) (*FilePV, error) {
 	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 {
+func LoadFilePVEmptyState(keyFilePath, stateFilePath string) (*FilePV, error) {
 	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 {
+func loadFilePV(keyFilePath, stateFilePath string, loadState bool) (*FilePV, error) {
 	keyJSONBytes, err := ioutil.ReadFile(keyFilePath)
 	if err != nil {
-		tmos.Exit(err.Error())
+		return nil, err
 	}
 	pvKey := FilePVKey{}
 	err = tmjson.Unmarshal(keyJSONBytes, &pvKey)
 	if err != nil {
-		tmos.Exit(fmt.Sprintf("Error reading PrivValidator key from %v: %v\n", keyFilePath, err))
+		return nil, fmt.Errorf("error reading PrivValidator key from %v: %w", keyFilePath, err)
 	}
 
 	// overwrite pubkey and address for convenience
@@ -217,11 +217,11 @@ func loadFilePV(keyFilePath, stateFilePath string, loadState bool) *FilePV {
 	if loadState {
 		stateJSONBytes, err := ioutil.ReadFile(stateFilePath)
 		if err != nil {
-			tmos.Exit(err.Error())
+			return nil, err
 		}
 		err = tmjson.Unmarshal(stateJSONBytes, &pvState)
 		if err != nil {
-			tmos.Exit(fmt.Sprintf("Error reading PrivValidator state from %v: %v\n", stateFilePath, err))
+			return nil, fmt.Errorf("error reading PrivValidator state from %v: %w", stateFilePath, err)
 		}
 	}
 
@@ -230,7 +230,7 @@ func loadFilePV(keyFilePath, stateFilePath string, loadState bool) *FilePV {
 	return &FilePV{
 		Key:           pvKey,
 		LastSignState: pvState,
-	}
+	}, nil
 }
 
 // LoadOrGenFilePV loads a FilePV from the given filePaths
@@ -241,7 +241,7 @@ func LoadOrGenFilePV(keyFilePath, stateFilePath string) (*FilePV, error) {
 		err error
 	)
 	if tmos.FileExists(keyFilePath) {
-		pv = LoadFilePV(keyFilePath, stateFilePath)
+		pv, err = LoadFilePV(keyFilePath, stateFilePath)
 	} else {
 		pv, err = GenFilePV(keyFilePath, stateFilePath, "")
 		pv.Save()

privval/file_test.go

@@ -36,7 +36,8 @@ func TestGenLoadValidator(t *testing.T) {
 	privVal.Save()
 	addr := privVal.GetAddress()
 
-	privVal = LoadFilePV(tempKeyFile.Name(), tempStateFile.Name())
+	privVal, err = LoadFilePV(tempKeyFile.Name(), tempStateFile.Name())
+	assert.NoError(err)
 	assert.Equal(addr, privVal.GetAddress(), "expected privval addr to be the same")
 	assert.Equal(height, privVal.LastSignState.Height, "expected privval.LastHeight to have been saved")
 }

test/e2e/app/main.go

@@ -176,7 +176,10 @@ func startNode(cfg *Config) error {
 
 // startSigner starts a signer server connecting to the given endpoint.
 func startSigner(cfg *Config) error {
-	filePV := privval.LoadFilePV(cfg.PrivValKey, cfg.PrivValState)
+	filePV, err := privval.LoadFilePV(cfg.PrivValKey, cfg.PrivValState)
+	if err != nil {
+		return err
+	}
 
 	protocol, address := tmnet.ProtocolAndAddress(cfg.PrivValServer)
 	var dialFn privval.SocketDialer
@@ -210,7 +213,7 @@ func startSigner(cfg *Config) error {
 	endpoint := privval.NewSignerDialerEndpoint(logger, dialFn,
 		privval.SignerDialerEndpointRetryWaitInterval(1*time.Second),
 		privval.SignerDialerEndpointConnRetries(100))
-	err := privval.NewSignerServer(endpoint, cfg.ChainID, filePV).Start()
+	err = privval.NewSignerServer(endpoint, cfg.ChainID, filePV).Start()
 	if err != nil {
 		return err
 	}

tools/tm-signer-harness/internal/test_harness.go

@@ -94,7 +94,10 @@ func NewTestHarness(logger log.Logger, cfg TestHarnessConfig) (*TestHarness, err
 	logger.Info("Loading private validator configuration", "keyFile", keyFile, "stateFile", stateFile)
 	// NOTE: LoadFilePV ultimately calls os.Exit on failure. No error will be
 	// returned if this call fails.
-	fpv := privval.LoadFilePV(keyFile, stateFile)
+	fpv, err := privval.LoadFilePV(keyFile, stateFile)
+	if err != nil {
+		return nil, err
+	}
 
 	genesisFile := ExpandPath(cfg.GenesisFile)
 	logger.Info("Loading chain ID from genesis file", "genesisFile", genesisFile)

tools/tm-signer-harness/main.go

@@ -134,7 +134,11 @@ func runTestHarness(acceptRetries int, bindAddr, tmhome string) {
 func extractKey(tmhome, outputPath string) {
 	keyFile := filepath.Join(internal.ExpandPath(tmhome), "config", "priv_validator_key.json")
 	stateFile := filepath.Join(internal.ExpandPath(tmhome), "data", "priv_validator_state.json")
-	fpv := privval.LoadFilePV(keyFile, stateFile)
+	fpv, err := privval.LoadFilePV(keyFile, stateFile)
+	if err != nil {
+		logger.Error("Can't load file pv", "err", err)
+		os.Exit(1)
+	}
 	pkb := []byte(fpv.Key.PrivKey.(ed25519.PrivKey))
 	if err := ioutil.WriteFile(internal.ExpandPath(outputPath), pkb[:32], 0600); err != nil {
 		logger.Info("Failed to write private key", "output", outputPath, "err", err)