privval: remove deprecated OldFilePV (#4889)

* privval: remove deprecated `OldFilePV`

The old format was deprecated in v0.28. It's time we remove it.

* fix changelog

CHANGELOG_PENDING.md

@@ -17,6 +17,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 - Go API
 
+  - [privval] [\#4744](https://github.com/tendermint/tendermint/pull/4744) Remove deprecated `OldFilePV` (@melekes)
+
+- Blockchain Protocol
+
 ### FEATURES:
 
 

config/config.go

@@ -263,12 +263,6 @@ 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
 func (cfg BaseConfig) NodeKeyFile() string {
 	return rootify(cfg.NodeKey, cfg.RootDir)

node/node.go

@@ -7,7 +7,6 @@ import (
 	"net"
 	"net/http"
 	_ "net/http/pprof" // nolint: gosec // securely exposed on separate, optional port
-	"os"
 	"strings"
 	"time"
 
@@ -88,31 +87,13 @@ type Provider func(*cfg.Config, log.Logger) (*Node, error)
 // PrivValidator, ClientCreator, GenesisDoc, and DBProvider.
 // It implements NodeProvider.
 func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) {
-	// Generate node PrivKey
 	nodeKey, err := p2p.LoadOrGenNodeKey(config.NodeKeyFile())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to load or gen node key %s: %w", config.NodeKeyFile(), 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", oldPrivVal, err)
-		}
-		logger.Info("Upgrading PrivValidator file",
-			"old", oldPrivVal,
-			"newKey", newPrivValKey,
-			"newState", newPrivValState,
-		)
-		oldPV.Upgrade(newPrivValKey, newPrivValState)
 	}
 
 	return NewNode(config,
-		privval.LoadOrGenFilePV(newPrivValKey, newPrivValState),
+		privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()),
 		nodeKey,
 		proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()),
 		DefaultGenesisDocProviderFunc(config),

privval/file.go

@@ -32,7 +32,7 @@ func voteToStep(vote *types.Vote) int8 {
 	case types.PrecommitType:
 		return stepPrecommit
 	default:
-		panic("Unknown vote type")
+		panic(fmt.Sprintf("Unknown vote type: %v", vote.Type))
 	}
 }
 

privval/file_deprecated.go

@@ -1,81 +0,0 @@
-package privval
-
-import (
-	"io/ioutil"
-	"os"
-
-	"github.com/tendermint/tendermint/crypto"
-	"github.com/tendermint/tendermint/libs/bytes"
-	"github.com/tendermint/tendermint/types"
-)
-
-// OldFilePV is the old version of the FilePV, pre v0.28.0.
-// Deprecated: Use FilePV instead.
-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 bytes.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/file_deprecated_test.go

@@ -1,84 +0,0 @@
-package privval_test
-
-import (
-	"io/ioutil"
-	"os"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/tendermint/tendermint/privval"
-)
-
-const lastSignBytes = "750802110500000000000000220B08B398F3E00510F48DA6402A480A20F" +
-	"C258973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4" +
-	"CAD312240A20C971B286ACB8AAA6FCA0365EB0A660B189EDC08B46B5AF2" +
-	"995DEFA51A28D215B10013211746573742D636861696E2D533245415533"
-
-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": "` + lastSignBytes + `",
-  "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)
-	npv, err := newPV.GetPubKey()
-	require.NoError(t, err)
-	assert.Equal(t, oldPV.PubKey, npv)
-	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()
-}

scripts/privValUpgrade.go

@@ -1,45 +0,0 @@
-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", oldPVPath, err)
-	}
-	logger.Info("Upgrading PrivValidator file",
-		"old", oldPVPath,
-		"newKey", newPVKeyPath,
-		"newState", newPVStatePath,
-	)
-	oldPV.Upgrade(newPVKeyPath, newPVStatePath)
-	return nil
-}

scripts/privValUpgrade_test.go

@@ -1,129 +0,0 @@
-package main
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/tendermint/tendermint/privval"
-)
-
-const lastSignBytes = "750802110500000000000000220B08B398F3E00510F48DA6402A480A20FC25" +
-	"8973076512999C3E6839A22E9FBDB1B77CF993E8A9955412A41A59D4CAD312240A20C971B286ACB8AA" +
-	"A6FCA0365EB0A660B189EDC08B46B5AF2995DEFA51A28D215B10013211746573742D636861696E2D533245415533"
-
-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": "` + lastSignBytes + `",
-  "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 {
-		tt := tt
-		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)
-					upv, err := upgradedPV.GetPubKey()
-					require.NoError(t, err)
-					assert.Equal(t, oldPV.PubKey, upv)
-					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()
-}