Merge PR #6818: x/upgrade: Fix ReadUpgradeInfoFromDisk

x/upgrade/doc.go

@@ -75,7 +75,11 @@ Here is a sample code to set store migrations with an upgrade:
 		// upgrade changes here
 	})
 
-	upgradeInfo := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
+	upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
+	if err != nil {
+		// handle error
+	}
+
 	if upgradeInfo.Name == "my-fancy-upgrade" && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
 		storeUpgrades := store.StoreUpgrades{
 			Renamed: []store.StoreRename{{

x/upgrade/keeper/keeper.go

@@ -9,8 +9,6 @@ import (
 	"path"
 	"path/filepath"
 
-	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
-
 	"github.com/tendermint/tendermint/libs/log"
 	tmos "github.com/tendermint/tendermint/libs/os"
 
@@ -19,6 +17,7 @@ import (
 	store "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
 // UpgradeInfoFileName file to store upgrade information
@@ -179,23 +178,31 @@ func (k Keeper) getHomeDir() string {
 	return k.homePath
 }
 
-// ReadUpgradeInfoFromDisk returns the name and height of the upgrade
-// which is written to disk by the old binary when panic'ing
-// if there's an error in reading the info,
-// it assumes that the upgrade info is not available
-func (k Keeper) ReadUpgradeInfoFromDisk() (upgradeInfo store.UpgradeInfo) {
+// ReadUpgradeInfoFromDisk returns the name and height of the upgrade which is
+// written to disk by the old binary when panicking. An error is returned if
+// the upgrade path directory cannot be created or if the file exists and
+// cannot be read or if the upgrade info fails to unmarshal.
+func (k Keeper) ReadUpgradeInfoFromDisk() (store.UpgradeInfo, error) {
+	var upgradeInfo store.UpgradeInfo
+
 	upgradeInfoPath, err := k.GetUpgradeInfoPath()
-	// if error in reading the path, assume there are no upgrades
 	if err != nil {
-		return upgradeInfo
+		return upgradeInfo, err
 	}
 
 	data, err := ioutil.ReadFile(upgradeInfoPath)
-	// if error in reading the file, assume there are no upgrades
 	if err != nil {
-		return upgradeInfo
+		// if file does not exist, assume there are no upgrades
+		if os.IsNotExist(err) {
+			return upgradeInfo, nil
 		}
 
-	json.Unmarshal(data, &upgradeInfo)
-	return
+		return upgradeInfo, err
+	}
+
+	if err := json.Unmarshal(data, &upgradeInfo); err != nil {
+		return upgradeInfo, err
+	}
+
+	return upgradeInfo, nil
 }

x/upgrade/keeper/keeper_test.go

@@ -0,0 +1,58 @@
+package keeper_test
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+
+	"github.com/cosmos/cosmos-sdk/simapp"
+	store "github.com/cosmos/cosmos-sdk/store/types"
+	"github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
+	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
+)
+
+type KeeperTestSuite struct {
+	suite.Suite
+
+	homeDir string
+	app     *simapp.SimApp
+}
+
+func (s *KeeperTestSuite) SetupTest() {
+	app := simapp.Setup(false)
+
+	homeDir, err := ioutil.TempDir(os.TempDir(), "x_upgrade_keeper_test")
+	s.Require().NoError(err)
+
+	// recreate keeper in order to use a custom home path
+	app.UpgradeKeeper = keeper.NewKeeper(
+		make(map[int64]bool), app.GetKey(types.StoreKey), app.AppCodec(), homeDir,
+	)
+
+	s.homeDir = homeDir
+	s.app = app
+}
+
+func (s *KeeperTestSuite) TestReadUpgradeInfoFromDisk() {
+	// require no error when the upgrade info file does not exist
+	_, err := s.app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
+	s.Require().NoError(err)
+
+	expected := store.UpgradeInfo{
+		Name:   "test_upgrade",
+		Height: 100,
+	}
+
+	// create an upgrade info file
+	s.Require().NoError(s.app.UpgradeKeeper.DumpUpgradeInfoToDisk(expected.Height, expected.Name))
+
+	ui, err := s.app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
+	s.Require().NoError(err)
+	s.Require().Equal(expected, ui)
+}
+
+func TestKeeperTestSuite(t *testing.T) {
+	suite.Run(t, new(KeeperTestSuite))
+}