diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 2fd0360e3..1150e1a80 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/cmd/priv_val_server/main.go b/cmd/priv_val_server/main.go index 03d375693..ca7b5d62f 100644 --- a/cmd/priv_val_server/main.go +++ b/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 { diff --git a/cmd/tendermint/commands/init.go b/cmd/tendermint/commands/init.go index 389f627d4..51578253b 100644 --- a/cmd/tendermint/commands/init.go +++ b/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 { diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index 3f0b51653..77a7884b0 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/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) { diff --git a/cmd/tendermint/commands/show_validator.go b/cmd/tendermint/commands/show_validator.go index 54b027961..3c83a4120 100644 --- a/cmd/tendermint/commands/show_validator.go +++ b/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 { diff --git a/cmd/tendermint/commands/testnet.go b/cmd/tendermint/commands/testnet.go index cfda5aa57..5171776f7 100644 --- a/cmd/tendermint/commands/testnet.go +++ b/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 { diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 601ffabf4..7da881e8f 100644 --- a/consensus/replay_test.go +++ b/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) diff --git a/privval/file.go b/privval/file.go index 4482e05f2..6f6e7b9b3 100644 --- a/privval/file.go +++ b/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() diff --git a/privval/file_test.go b/privval/file_test.go index 1d9c65fa3..965257daf 100644 --- a/privval/file_test.go +++ b/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") } diff --git a/test/e2e/app/main.go b/test/e2e/app/main.go index e387561c7..743b97b3a 100644 --- a/test/e2e/app/main.go +++ b/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 } diff --git a/tools/tm-signer-harness/internal/test_harness.go b/tools/tm-signer-harness/internal/test_harness.go index 33ae537fe..b0940f3db 100644 --- a/tools/tm-signer-harness/internal/test_harness.go +++ b/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) diff --git a/tools/tm-signer-harness/main.go b/tools/tm-signer-harness/main.go index d624234ae..65cba32ed 100644 --- a/tools/tm-signer-harness/main.go +++ b/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)