feat!: support coordinated v2 upgrade with flag

Makefile

@@ -1,6 +1,7 @@
 VERSION := $(shell echo $(shell git describe --tags 2>/dev/null || git log -1 --format='%h') | sed 's/^v//')
 COMMIT := $(shell git log -1 --format='%H')
 DOCKER := $(shell which docker)
+ALL_VERSIONS := $(shell git tag -l)
 DOCKER_BUF := $(DOCKER) run --rm -v $(CURDIR):/workspace --workdir /workspace bufbuild/buf
 IMAGE := ghcr.io/tendermint/docker-build-proto:latest
 DOCKER_PROTO_BUILDER := docker run -v $(shell pwd):/workspace --workdir /workspace $(IMAGE)
@@ -115,10 +116,10 @@ test-short:
 	@go test ./... -short -timeout 1m
 .PHONY: test-short
 
-## test-e2e: Run end to end tests via knuu.
+## test-e2e: Run end to end tests via knuu. This command requires a kube/config file to configure kubernetes.
 test-e2e:
-	@echo "--> Running e2e tests on version: $(shell git rev-parse --short HEAD)"
-	@KNUU_NAMESPACE=test E2E_VERSION=$(shell git rev-parse --short HEAD) E2E=true go test ./test/e2e/... -timeout 10m -v
+	@echo "--> Running end to end tests"
+	@KNUU_NAMESPACE=test KNUU_TIMEOUT=20m E2E_VERSIONS="$(ALL_VERSIONS)" E2E=true go test ./test/e2e/... -timeout 20m -v
 .PHONY: test-e2e
 
 ## test-race: Run tests in race mode.

app/app.go

@@ -85,6 +85,7 @@ import (
 	"github.com/celestiaorg/celestia-app/app/ante"
 	"github.com/celestiaorg/celestia-app/app/encoding"
 	"github.com/celestiaorg/celestia-app/pkg/appconsts"
+	v2 "github.com/celestiaorg/celestia-app/pkg/appconsts/v2"
 	"github.com/celestiaorg/celestia-app/pkg/proof"
 	blobmodule "github.com/celestiaorg/celestia-app/x/blob"
 	blobmodulekeeper "github.com/celestiaorg/celestia-app/x/blob/keeper"
@@ -246,16 +247,10 @@ func New(
 	loadLatest bool,
 	invCheckPeriod uint,
 	encodingConfig encoding.Config,
-	upgradeSchedule map[string]upgrade.Schedule,
+	upgradeHeight int64,
 	appOpts servertypes.AppOptions,
 	baseAppOptions ...func(*baseapp.BaseApp),
 ) *App {
-	for _, schedule := range upgradeSchedule {
-		if err := schedule.ValidateVersions(supportedVersions); err != nil {
-			panic(err)
-		}
-	}
-
 	appCodec := encodingConfig.Codec
 	cdc := encodingConfig.Amino
 	interfaceRegistry := encodingConfig.InterfaceRegistry
@@ -334,7 +329,7 @@ func New(
 	)
 
 	app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
-	app.UpgradeKeeper = upgrade.NewKeeper(keys[upgrade.StoreKey], upgradeSchedule)
+	app.UpgradeKeeper = upgrade.NewKeeper(keys[upgrade.StoreKey], upgradeHeight)
 
 	app.BlobstreamKeeper = *bsmodulekeeper.NewKeeper(
 		appCodec,
@@ -574,14 +569,10 @@ func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.R
 // EndBlocker application updates every end block
 func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	res := app.mm.EndBlock(ctx, req)
-	if app.UpgradeKeeper.ShouldUpgrade() {
-		newAppVersion := app.UpgradeKeeper.GetNextAppVersion()
-		app.SetProtocolVersion(newAppVersion)
-		_, err := app.mm.RunMigrations(ctx, app.configurator, GetModuleVersion(newAppVersion))
-		if err != nil {
-			panic(err)
-		}
-		app.UpgradeKeeper.MarkUpgradeComplete()
+	// NOTE: this is a specific feature for upgrading to v2 as v3 and onward is expected
+	// to be coordinated through the signalling protocol
+	if app.UpgradeKeeper.ShouldUpgrade(req.Height) {
+		app.SetProtocolVersion(v2.Version)
 	}
 	return res
 }

app/prepare_proposal.go

@@ -7,7 +7,6 @@ import (
 	"github.com/celestiaorg/celestia-app/pkg/da"
 	"github.com/celestiaorg/celestia-app/pkg/shares"
 	"github.com/celestiaorg/celestia-app/pkg/square"
-	"github.com/celestiaorg/celestia-app/x/upgrade"
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	abci "github.com/tendermint/tendermint/abci/types"
 	core "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -59,27 +58,6 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
 		txs = make([][]byte, 0)
 	} else {
 		txs = FilterTxs(app.Logger(), sdkCtx, handler, app.txConfig, req.BlockData.Txs)
-
-		// TODO: this would be improved if we only attempted the upgrade in the first round of the
-		// height to still allow transactions to pass through without being delayed from trying
-		// to coordinate the upgrade height
-		if newVersion, ok := app.UpgradeKeeper.ShouldProposeUpgrade(req.ChainId, req.Height); ok && newVersion > app.GetBaseApp().AppVersion() {
-			upgradeTx, err := upgrade.NewMsgVersionChange(app.txConfig, newVersion)
-			if err != nil {
-				panic(err)
-			}
-			// the upgrade transaction must be the first transaction in the block
-			txs = append([][]byte{upgradeTx}, txs...)
-
-			// because we are adding bytes, we need to check that we are not going over the limit
-			// if we are, we continually prune the last tx (the lowest paying blobTx).
-			size := sizeOf(txs)
-			for size > int(req.BlockDataSize) {
-				lastTx := txs[len(txs)-1]
-				txs = txs[:len(txs)-1]
-				size -= len(lastTx)
-			}
-		}
 	}
 
 	// build the square from the set of valid and prioritised transactions.
@@ -124,11 +102,3 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
 		},
 	}
 }
-
-func sizeOf(txs [][]byte) int {
-	size := 0
-	for _, tx := range txs {
-		size += len(tx)
-	}
-	return size
-}

app/process_proposal.go

@@ -11,7 +11,6 @@ import (
 	"github.com/celestiaorg/celestia-app/pkg/shares"
 	"github.com/celestiaorg/celestia-app/pkg/square"
 	blobtypes "github.com/celestiaorg/celestia-app/x/blob/types"
-	"github.com/celestiaorg/celestia-app/x/upgrade"
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -76,27 +75,6 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
 				return reject()
 			}
 
-			if appVersion, ok := upgrade.IsUpgradeMsg(msgs); ok {
-				if idx != 0 {
-					logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("upgrade message %d is not the first transaction", idx))
-					return reject()
-				}
-
-				if !IsSupported(appVersion) {
-					logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("block proposes an unsupported app version %d", appVersion))
-					return reject()
-				}
-
-				// app version must always increase
-				if appVersion <= app.GetBaseApp().AppVersion() {
-					logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("block proposes an app version %d that is not greater than the current app version %d", appVersion, app.GetBaseApp().AppVersion()))
-					return reject()
-				}
-
-				// we don't need to pass this message through the ante handler
-				continue
-			}
-
 			// we need to increment the sequence for every transaction so that
 			// the signature check below is accurate. this error only gets hit
 			// if the account in question doens't exist.

cmd/celestia-appd/cmd/root.go

@@ -4,6 +4,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strconv"
 
 	bscmd "github.com/celestiaorg/celestia-app/x/blobstream/client"
 
@@ -44,6 +45,8 @@ const (
 
 	// FlagLogToFile specifies whether to log to file or not.
 	FlagLogToFile = "log-to-file"
+
+	UpgradeHeightFlag = "upgrade-height"
 )
 
 // NewRootCmd creates a new root command for celestia-appd. It is called once in the
@@ -153,6 +156,7 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig encoding.Config) {
 
 func addModuleInitFlags(startCmd *cobra.Command) {
 	crisis.AddModuleInitFlags(startCmd)
+	startCmd.Flags().Int64(UpgradeHeightFlag, 0, "Upgrade height to switch to v2. Must be coordinated amongst all validators")
 }
 
 func queryCommand() *cobra.Command {
@@ -229,11 +233,20 @@ func NewAppServer(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts se
 		panic(err)
 	}
 
+	var upgradeHeight int64
+	upgradeHeightStr, ok := appOpts.Get(UpgradeHeightFlag).(string)
+	if ok {
+		upgradeHeight, err = strconv.ParseInt(upgradeHeightStr, 10, 64)
+		if err != nil {
+			panic(err)
+		}
+	}
+
 	return app.New(
 		logger, db, traceStore, true,
 		cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)),
 		encoding.MakeConfig(app.ModuleEncodingRegisters...), // Ideally, we would reuse the one created by NewRootCmd.
-		nil,
+		upgradeHeight,
 		appOpts,
 		baseapp.SetPruning(pruningOpts),
 		baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))),
@@ -256,13 +269,13 @@ func createAppAndExport(
 	encCfg.Codec = codec.NewProtoCodec(encCfg.InterfaceRegistry)
 	var capp *app.App
 	if height != -1 {
-		capp = app.New(logger, db, traceStore, false, uint(1), encCfg, nil, appOpts)
+		capp = app.New(logger, db, traceStore, false, uint(1), encCfg, 0, appOpts)
 
 		if err := capp.LoadHeight(height); err != nil {
 			return servertypes.ExportedApp{}, err
 		}
 	} else {
-		capp = app.New(logger, db, traceStore, true, uint(1), encCfg, nil, appOpts)
+		capp = app.New(logger, db, traceStore, true, uint(1), encCfg, 0, appOpts)
 	}
 
 	return capp.ExportAppStateAndValidators(forZeroHeight, jailWhiteList)

test/e2e/node.go

@@ -50,7 +50,7 @@ func NewNode(
 	if err != nil {
 		return nil, err
 	}
-	err = instance.SetImage(fmt.Sprintf("%s:%s", dockerSrcURL, version))
+	err = instance.SetImage(DockerImageName(version))
 	if err != nil {
 		return nil, err
 	}
@@ -260,3 +260,11 @@ func (n *Node) Start() error {
 	n.grpcProxyPort = grpcProxyPort
 	return nil
 }
+
+func (n *Node) Upgrade(version string) error {
+	return n.Instance.SetImageInstant(DockerImageName(version))
+}
+
+func DockerImageName(version string) string {
+	return fmt.Sprintf("%s:%s", dockerSrcURL, version)
+}

test/e2e/simple_test.go

@@ -22,13 +22,18 @@ var latestVersion = "latest"
 // and MsgSends over 30 seconds and then asserts that at least 10 transactions were
 // committed.
 func TestE2ESimple(t *testing.T) {
-	if os.Getenv("E2E") == "" {
+	if os.Getenv("E2E") != "true" {
 		t.Skip("skipping e2e test")
 	}
 
-	if os.Getenv("E2E_VERSION") != "" {
-		latestVersion = os.Getenv("E2E_VERSION")
+	if os.Getenv("E2E_VERSIONS") != "" {
+		versionsStr := os.Getenv("E2E_VERSIONS")
+		versions := ParseVersions(versionsStr)
+		if len(versions) > 0 {
+			latestVersion = versions.GetLatest().String()
+		}
 	}
+	t.Log("Running simple e2e test", "version", latestVersion)
 
 	testnet, err := New(t.Name(), seed)
 	require.NoError(t, err)