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-system-test-current 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

@@ -1,7 +1,12 @@
 package simapp
 
 import (
+	"bytes"
 	"context"
+	"fmt"
+	"os"
+
+	"github.com/cosmos/gogoproto/jsonpb"
 
 	storetypes "cosmossdk.io/store/types"
 
@@ -16,21 +21,55 @@ import (
 // NOTE: This upgrade defines a reference implementation of what an upgrade
 // could look like when an application is migrating from Cosmos SDK version
 // v0.53.x to v0.54.x.
-const UpgradeName = "v053-to-v054"
+const (
+	UpgradeName       = "v053-to-v054"
+	ManualUpgradeName = "manual1"
+)
 
 func (app SimApp) RegisterUpgradeHandlers() {
 	app.UpgradeKeeper.SetUpgradeHandler(
 		UpgradeName,
 		func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
-			sdk.UnwrapSDKContext(ctx).Logger().Debug("this is a debug level message to test that verbose logging mode has properly been enabled during a chain upgrade")
+			logger := sdk.UnwrapSDKContext(ctx).Logger()
+			logger.Debug("this is a debug level message to test that verbose logging mode has properly been enabled during a chain upgrade")
+			logger.Debug(fmt.Sprintf("applying upgrade %s", UpgradeName))
 			return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM)
 		},
 	)
+	// we add another upgrade, to be performed manually which does some small state breakage
+	app.UpgradeKeeper.SetUpgradeHandler(
+		ManualUpgradeName,
+		func(ctx context.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
+			logger := sdk.UnwrapSDKContext(ctx).Logger()
+			logger.Debug(fmt.Sprintf("applying upgrade %s", ManualUpgradeName))
+			// do some minimal state breaking update
+			err := app.GovKeeper.Constitution.Set(ctx,
+				fmt.Sprintf("we have expected upgrade %q and that's now our constitution", plan.Name))
+			return fromVM, err
+		},
+	)
 
+	// we check that we can read the upgrade info from disk, which is necessary for setting store key upgrades
 	upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
 	if err != nil {
 		panic(err)
 	}
+	if upgradeInfo.Name != "" {
+		app.Logger().Info("read upgrade info from disk", "upgrade_info", upgradeInfo)
+	}
+
+	// this allows to test stateful manual upgrades with Cosmovisor
+	if manualUpgradeVar, ok := os.LookupEnv("SIMAPP_MANUAL_UPGRADE"); ok {
+		var manualUpgrade upgradetypes.Plan
+		err := (&jsonpb.Unmarshaler{}).Unmarshal(bytes.NewBufferString(manualUpgradeVar), &manualUpgrade)
+		if err != nil {
+			panic("invalid SIMAPP_MANUAL_UPGRADE: " + err.Error())
+		}
+		err = app.UpgradeKeeper.SetManualUpgrade(&manualUpgrade)
+		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

@@ -175,10 +175,24 @@
 
 func (s *SystemUnderTest) StartChain(t *testing.T, xargs ...string) {
 	t.Helper()
+	s.doStartChain(t, false, xargs...)
+}
+
+// StartChainWithCosmovisor starts the chain wrapping its execution with Cosmovisor.
+func (s *SystemUnderTest) StartChainWithCosmovisor(t *testing.T, xargs ...string) {
+	t.Helper()
+	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 +209,48 @@
 	s.AwaitNextBlock(t, 10e9)
 }
 
+func (s *SystemUnderTest) cosmovisorEnv(t *testing.T, home string) []string {
+	t.Helper()
+	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")
+}
+
+// ExecCosmovisor executes the Cosmovisor binary with the given arguments
+// for each node in the network with the home directory set properly for each node.
+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) {
+	t.Helper()
+	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 +647,32 @@
 }
 
 // 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),
+		cmd := exec.Command(
+			binary,
 			args...,
 		)
 		cmd.Dir = WorkDir
+		if useCosmovisor {
+			cmd.Env = env
+		}
 		s.watchLogs(i, cmd)
 		require.NoError(t, cmd.Start(), "node %d", i)
 		s.Logf("Node started: %d\n", cmd.Process.Pid)

systemtests/test_runner.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -17,6 +18,9 @@ var (
 	Sut            *SystemUnderTest
 	Verbose        bool
 	execBinaryName string
+	// Store original configuration for ResetSut
+	originalNodesCount int
+	originalBlockTime  time.Duration
 )
 
 func RunTests(m *testing.M) {
@@ -47,6 +51,11 @@ func RunTests(m *testing.M) {
 	}
 	execBinaryName = *execBinary
 
+	// store original configuration for ResetSut, we do this with global variables for now since Sut is global
+	// and we want the same initial configuration when we do a complete reset
+	originalNodesCount = *nodesCount
+	originalBlockTime = *blockTime
+
 	Sut = NewSystemUnderTest(*execBinary, Verbose, *nodesCount, *blockTime)
 	Sut.SetupChain() // setup chain and keyring
 
@@ -120,6 +129,25 @@ const (
 |_| \__,_|_|_|\___|\__,_|`
 )
 
+// ResetSut completely resets Sut by deleting all state and creating a fresh instance of Sut.
+func ResetSut(t *testing.T) {
+	t.Helper()
+	// stop current instance if it exists
+	if Sut != nil {
+		Sut.StopChain()
+	}
+
+	// delete entire testnet directory to remove all state
+	err := os.RemoveAll(filepath.Join(WorkDir, "testnet"))
+	if err != nil {
+		t.Fatalf("failed to remove testnet directory: %v", err)
+	}
+
+	// create fresh Sut instance with original configuration
+	Sut = NewSystemUnderTest(execBinaryName, Verbose, originalNodesCount, originalBlockTime)
+	Sut.SetupChain()
+}
+
 func printResultFlag(ok bool) {
 	if ok {
 		fmt.Println(successFlag)