refactor(cosmovisor)!: re-implement core logic to address reliability issues

Makefile

@@ -505,13 +505,21 @@ localnet-debug: localnet-stop localnet-build-dlv localnet-build-nodes
 
 .PHONY: localnet-start localnet-stop localnet-debug localnet-build-env localnet-build-dlv localnet-build-nodes
 
-test-system: build-v53 build
+# build-system-test-current builds the binaries necessary for running system tests, but only those on the current branch
+# this is useful if you are iterating on tests which rely on changes to the current branch only (which is most common in development)
+build-system-test-current: build cosmovisor
 	mkdir -p ./tests/systemtests/binaries/
 	cp $(BUILDDIR)/simd ./tests/systemtests/binaries/
+	cp tools/cosmovisor/cosmovisor ./tests/systemtests/binaries/
+
+# build-system-test builds the binaries necessary for runnings system tests and places them in the correct locations
+build-system-test: build-v53
 	mkdir -p ./tests/systemtests/binaries/v0.53
 	mv $(BUILDDIR)/simdv53 ./tests/systemtests/binaries/v0.53/simd
+
+test-system: build-system-test
 	$(MAKE) -C tests/systemtests test
-.PHONY: test-system
+.PHONY: build-system-test-current build-system-test test-system
 
 # build-v53 checks out the v0.53.x branch, builds the binary, and renames it to simdv53.
 build-v53:

simapp/upgrades.go

@@ -2,6 +2,8 @@ package simapp
 
 import (
 	"context"
+	"os"
+	"strconv"
 
 	storetypes "cosmossdk.io/store/types"
 
@@ -31,6 +33,25 @@ func (app SimApp) RegisterUpgradeHandlers() {
 	if err != nil {
 		panic(err)
 	}
+	if upgradeInfo.Name != "" {
+		app.Logger().Info("read upgrade info from disk", "upgrade_info", upgradeInfo)
+	}
+
+	// this allows us to check migration to v0.54.x in the system tests via a manual (non-governance upgrade)
+	if manualUpgrade, ok := os.LookupEnv("SIMAPP_MANUAL_UPGRADE_HEIGHT"); ok {
+		height, err := strconv.ParseUint(manualUpgrade, 10, 64)
+		if err != nil {
+			panic("invalid SIMAPP_MANUAL_UPGRADE_HEIGHT height: " + err.Error())
+		}
+		upgradeInfo = upgradetypes.Plan{
+			Name:   UpgradeName,
+			Height: int64(height),
+		}
+		err = app.UpgradeKeeper.SetManualUpgrade(&upgradeInfo)
+		if err != nil {
+			panic("failed to set manual upgrade: " + err.Error())
+		}
+	}
 
 	if upgradeInfo.Name == UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
 		storeUpgrades := storetypes.StoreUpgrades{

systemtests/system.go

@@ -173,12 +173,23 @@
 	MustCopyFilesInDir(src, dest)
 }
 
 func (s *SystemUnderTest) StartChain(t *testing.T, xargs ...string) {
+	s.doStartChain(t, false, xargs...)
+}
+
+func (s *SystemUnderTest) StartChainWithCosmovisor(t *testing.T, xargs ...string) {
+	s.doStartChain(t, true, xargs...)
+}
+
+func (s *SystemUnderTest) doStartChain(t *testing.T, useCosmovisor bool, xargs ...string) {
 	t.Helper()
+	if useCosmovisor {
+		s.initCosmovisor(t)
+	}
 	s.Log("Start chain\n")
 	s.ChainStarted = true
 	// HACK: force db_backend
-	s.startNodesAsync(t, append([]string{"start", "--log_level=info", "--log_no_color", "--db_backend=goleveldb"}, xargs...)...)
+	s.startNodesAsync(t, useCosmovisor, append([]string{"start", "--log_level=info", "--log_no_color", "--db_backend=goleveldb"}, xargs...)...)
 
 	s.AwaitNodeUp(t, s.rpcAddr)
 
@@ -195,6 +206,43 @@
 	s.AwaitNextBlock(t, 10e9)
 }
 
+func (s *SystemUnderTest) cosmovisorEnv(t *testing.T, home string) []string {
+	absHome, err := filepath.Abs(home)
+	require.NoError(t, err)
+	return []string{
+		fmt.Sprintf("DAEMON_HOME=%s", absHome),
+		fmt.Sprintf("DAEMON_NAME=%s", s.projectName),
+	}
+}
+
+func (s *SystemUnderTest) cosmovisorPath() string {
+	return filepath.Join(WorkDir, "binaries", "cosmovisor")
+}
+func (s *SystemUnderTest) ExecCosmovisor(t *testing.T, async bool, args ...string) {
+	s.withEachNodeHome(func(i int, home string) {
+		env := s.cosmovisorEnv(t, home)
+		t.Logf("Calling Cosmovisor with args %+v and env %+v", args, env)
+		cmd := exec.Command(
+			s.cosmovisorPath(),
+			args...,
+		)
+		cmd.Dir = WorkDir
+		env = append(env, "COSMOVISOR_COLOR_LOGS=false")
+		cmd.Env = env
+		if async {
+			require.NoError(t, cmd.Start(), "cosmovisor init %d", i)
+			s.awaitProcessCleanup(cmd)
+		} else {
+			require.NoError(t, cmd.Run(), "cosmovisor init %d", i)
+		}
+	})
+}
+
+func (s *SystemUnderTest) initCosmovisor(t *testing.T) {
+	binary := locateExecutable(s.execBinary)
+	s.ExecCosmovisor(t, false, "init", binary)
+}
+
 // MarkDirty whole chain will be reset when marked dirty
 func (s *SystemUnderTest) MarkDirty() {
 	s.dirty = true
@@ -591,16 +639,30 @@
 }
 
 // startNodesAsync runs the given app cli command for all cluster nodes and returns without waiting
-func (s *SystemUnderTest) startNodesAsync(t *testing.T, xargs ...string) {
+func (s *SystemUnderTest) startNodesAsync(t *testing.T, useCosmovisor bool, xargs ...string) {
 	t.Helper()
 	s.withEachNodeHome(func(i int, home string) {
-		args := append(xargs, "--home="+home)
+		absHome, err := filepath.Abs(home)
+		require.NoError(t, err, "failed to get absolute home path")
+		args := append(xargs, "--home="+absHome)
+		var binary string
+		var env []string
+		if useCosmovisor {
+			binary = s.cosmovisorPath()
+			args = append([]string{"run"}, args...) // cosmovisor run <args>
+			cfgPath := filepath.Join(absHome, "cosmovisor", "config.toml")
+			args = append(args, "--cosmovisor-config", cfgPath)
+			env = s.cosmovisorEnv(t, absHome)
+		} else {
+			binary = locateExecutable(s.execBinary)
+		}
 		s.Logf("Execute `%s %s`\n", s.execBinary, strings.Join(args, " "))
 		cmd := exec.Command( //nolint:gosec // used by tests only
-			locateExecutable(s.execBinary),
+			binary,
 			args...,
 		)
 		cmd.Dir = WorkDir
+		cmd.Env = env
 		s.watchLogs(i, cmd)
 		require.NoError(t, cmd.Start(), "node %d", i)
 		s.Logf("Node started: %d\n", cmd.Process.Pid)

tests/systemtests/cosmovisor_test.go

@@ -0,0 +1,180 @@
+//go:build system_test
+
+package systemtests
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tidwall/gjson"
+
+	systest "cosmossdk.io/systemtests"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/address"
+)
+
+func TestCosmovisorUpgrade(t *testing.T) {
+	t.Run("gov upgrade, then manual upgrade", func(t *testing.T) {
+		const (
+			upgrade1Height       = 25
+			upgrade1Name         = "v053-to-v054" // must match UpgradeName in simapp/upgrades.go
+			upgrade2Height int64 = 30
+			upgrade2Name         = "manual1"
+		)
+
+		// Scenario:
+		// start a legacy chain with some state
+		// when a chain upgrade proposal is executed
+		// then the chain upgrades successfully
+		systest.Sut.StopChain()
+
+		currentBranchBinary := systest.Sut.ExecBinary()
+
+		legacyBinary := systest.WorkDir + "/binaries/v0.53/simd"
+		systest.Sut.SetExecBinary(legacyBinary)
+		systest.Sut.SetupChain()
+
+		votingPeriod := 5 * time.Second // enough time to vote
+		systest.Sut.ModifyGenesisJSON(t, systest.SetGovVotingPeriod(t, votingPeriod))
+
+		systest.Sut.StartChainWithCosmovisor(t)
+
+		cli := systest.NewCLIWrapper(t, systest.Sut, systest.Verbose)
+		govAddr := sdk.AccAddress(address.Module("gov")).String()
+		// submit upgrade proposal
+		proposal := fmt.Sprintf(`
+	{
+	"messages": [
+	 {
+	  "@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade",
+	  "authority": %q,
+	  "plan": {
+	   "name": %q,
+	   "height": "%d"
+	  }
+	 }
+	],
+	"metadata": "ipfs://CID",
+	"deposit": "100000000stake",
+	"title": "my upgrade",
+	"summary": "testing"
+	}`, govAddr, upgrade1Name, upgrade1Height)
+		proposalID := cli.SubmitAndVoteGovProposal(proposal)
+
+		// add binary for gov upgrade
+		systest.Sut.ExecCosmovisor(
+			t,
+			true,
+			"add-upgrade",
+			upgrade1Name,
+			currentBranchBinary,
+		)
+
+		requireCurrentPointsTo(t, "genesis")
+
+		systest.Sut.AwaitBlockHeight(t, 21, 60*time.Second)
+
+		t.Logf("current_height: %d\n", systest.Sut.CurrentHeight())
+		raw := cli.CustomQuery("q", "gov", "proposal", proposalID)
+		proposalStatus := gjson.Get(raw, "proposal.status").String()
+		require.Equal(t, "PROPOSAL_STATUS_PASSED", proposalStatus, raw)
+
+		// add manual upgrade
+		systest.Sut.ExecCosmovisor(
+			t,
+			true,
+			"add-upgrade",
+			upgrade2Name,
+			currentBranchBinary,
+			fmt.Sprintf("--upgrade-height=%d", upgrade2Height),
+		)
+
+		systest.Sut.AwaitBlockHeight(t, upgrade1Height+1)
+
+		requireCurrentPointsTo(t, fmt.Sprintf("upgrades/%s", upgrade1Name))
+		// make sure a gov upgrade was triggered
+		regex, err := regexp.Compile(fmt.Sprintf(`UPGRADE %q NEEDED at height: %d:  module=x/upgrade`,
+			upgrade1Name, upgrade1Height))
+		require.NoError(t, err)
+		require.Equal(t, systest.Sut.NodesCount(), systest.Sut.FindLogMessage(regex))
+		// make sure the upgrade-info.json was readable by nodes when they restarted
+		regex, err = regexp.Compile("read upgrade info from disk")
+		require.NoError(t, err)
+		require.Equal(t, systest.Sut.NodesCount(), systest.Sut.FindLogMessage(regex))
+
+		systest.Sut.AwaitBlockHeight(t, upgrade2Height+1)
+
+		requireCurrentPointsTo(t, fmt.Sprintf("upgrades/%s", upgrade2Name))
+
+		// smoke test that new version runs
+		cli = systest.NewCLIWrapper(t, systest.Sut, systest.Verbose)
+		got := cli.Run("tx", "protocolpool", "fund-community-pool", "100stake", "--from=node0")
+		systest.RequireTxSuccess(t, got)
+	})
+
+	t.Run("manual upgrade", func(t *testing.T) {
+		const (
+			upgradeHeight = 10
+			upgradeName   = "v053-to-v054" // must match UpgradeName in simapp/upgrades.go
+		)
+		// Scenario:
+		// start a legacy chain with some state
+		// when a chain upgrade proposal is executed
+		// then the chain upgrades successfully
+		systest.Sut.StopChain()
+
+		currentBranchBinary := systest.Sut.ExecBinary()
+
+		legacyBinary := systest.WorkDir + "/binaries/v0.53/simd"
+		systest.Sut.SetExecBinary(legacyBinary)
+		systest.Sut.SetupChain()
+
+		systest.Sut.StartChainWithCosmovisor(t)
+		requireCurrentPointsTo(t, "genesis")
+
+		// we create a wrapper for the current branch binary which sets the
+		// SIMAPP_MANUAL_UPGRADE_HEIGHT which will cause the upgrade to be applied manually
+		wrapperTxt := fmt.Sprintf(`#!/usr/bin/env bash
+set -e
+SIMAPP_MANUAL_UPGRADE_HEIGHT="%d" exec %s "$@"`, upgradeHeight, currentBranchBinary)
+		wrapperPath := filepath.Join(systest.WorkDir, "testnet", fmt.Sprintf("%s.sh", upgradeName))
+		wrapperPath, err := filepath.Abs(wrapperPath)
+		require.NoError(t, err, "failed to get absolute path for manual upgrade script")
+		err = os.WriteFile(wrapperPath, []byte(wrapperTxt), 0o755)
+		require.NoError(t, err, "failed to write manual upgrade script")
+
+		// schedule manual upgrade to latest version
+		systest.Sut.ExecCosmovisor(
+			t,
+			true,
+			"add-upgrade",
+			upgradeName,
+			wrapperPath,
+			fmt.Sprintf("--upgrade-height=%d", upgradeHeight),
+		)
+
+		systest.Sut.AwaitBlockHeight(t, upgradeHeight+1, 60*time.Second)
+
+		requireCurrentPointsTo(t, fmt.Sprintf("upgrades/%s", upgradeName))
+
+		// smoke test that new version runs
+		cli := systest.NewCLIWrapper(t, systest.Sut, systest.Verbose)
+		got := cli.Run("tx", "protocolpool", "fund-community-pool", "100stake", "--from=node0")
+		systest.RequireTxSuccess(t, got)
+	})
+}
+
+func requireCurrentPointsTo(t *testing.T, expected string) {
+	t.Helper()
+	for i := 0; i < systest.Sut.NodesCount(); i++ {
+		curSymLink := filepath.Join(systest.Sut.NodeDir(i), "cosmovisor", "current")
+		resolved, err := os.Readlink(curSymLink)
+		require.NoError(t, err, "failed to read current symlink for node %d", i)
+		require.Equal(t, expected, resolved, "current symlink for node %d does not point to expected directory", i)
+	}
+}

tests/systemtests/upgrade_test.go

@@ -18,12 +18,15 @@
 )
 
 const (
-	testSeed            = "scene learn remember glide apple expand quality spawn property shoe lamp carry upset blossom draft reject aim file trash miss script joy only measure"
-	upgradeHeight int64 = 22
-	upgradeName         = "v053-to-v054" // must match UpgradeName in simapp/upgrades.go
+	testSeed = "scene learn remember glide apple expand quality spawn property shoe lamp carry upset blossom draft reject aim file trash miss script joy only measure"
 )
 
 func TestChainUpgrade(t *testing.T) {
+	const (
+		upgradeHeight int64 = 22
+		upgradeName         = "v053-to-v054" // must match UpgradeName in simapp/upgrades.go
+	)
+
 	// Scenario:
 	// start a legacy chain with some state
 	// when a chain upgrade proposal is executed