feat!: support coordinated v2 upgrade with flag

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"
@@ -239,23 +240,21 @@ type App struct {
 }
 
 // New returns a reference to an initialized celestia app.
+//
+// NOTE: upgradeHeight refers specifically to the height that
+// a node will upgrade from v1 to v2. It will be deprecated in v3
+// in place for a dynamically signalling scheme
 func New(
 	logger log.Logger,
 	db dbm.DB,
 	traceStore io.Writer,
 	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 +333,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 +573,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"
 
@@ -45,6 +46,8 @@ const (
 
 	// FlagLogToFile specifies whether to log to file or not.
 	FlagLogToFile = "log-to-file"
+
+	UpgradeHeightFlag = "v2-upgrade-height"
 )
 
 // NewRootCmd creates a new root command for celestia-appd. It is called once in the
@@ -157,6 +160,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 from v1 to v2. Must be coordinated amongst all validators")
 }
 
 func queryCommand() *cobra.Command {
@@ -233,11 +237,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))),
@@ -260,13 +273,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/simple_test.go

@@ -52,7 +52,7 @@ func TestE2ESimple(t *testing.T) {
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
-	opts := txsim.DefaultOptions().WithSeed(seed)
+	opts := txsim.DefaultOptions().WithSeed(seed).SuppressLogs()
 	err = txsim.Run(ctx, testnet.GRPCEndpoints()[0], kr, encCfg, opts, sequences...)
 	require.True(t, errors.Is(err, context.DeadlineExceeded), err.Error())
 

test/e2e/upgrade_test.go

@@ -69,7 +69,7 @@ func TestMinorVersionCompatibility(t *testing.T) {
 
 	errCh := make(chan error)
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
-	opts := txsim.DefaultOptions().WithSeed(seed)
+	opts := txsim.DefaultOptions().WithSeed(seed).SuppressLogs()
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	go func() {

test/tokenfilter/setup.go

@@ -140,7 +140,7 @@ func SetupWithGenesisValSet(t testing.TB, valSet *tmtypes.ValidatorSet, genAccs
 	encCdc := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	genesisState := app.NewDefaultGenesisState(encCdc.Codec)
 	app := app.New(
-		log.NewNopLogger(), db, nil, true, 5, encCdc, nil, simapp.EmptyAppOptions{},
+		log.NewNopLogger(), db, nil, true, 5, encCdc, 0, simapp.EmptyAppOptions{},
 	)
 
 	// set genesis accounts

test/util/malicious/app.go

@@ -57,7 +57,7 @@ func New(
 	appOpts servertypes.AppOptions,
 	baseAppOptions ...func(*baseapp.BaseApp),
 ) *App {
-	goodApp := app.New(logger, db, traceStore, loadLatest, invCheckPeriod, encodingConfig, nil, appOpts, baseAppOptions...)
+	goodApp := app.New(logger, db, traceStore, loadLatest, invCheckPeriod, encodingConfig, 0, appOpts, baseAppOptions...)
 	badApp := &App{App: goodApp}
 
 	// set the malicious prepare proposal handler if it is set in the app options

test/util/network/network.go

@@ -77,7 +77,7 @@ func DefaultConfig() network.Config {
 		AppConstructor: func(val network.Validator) servertypes.Application {
 			return app.New(
 				val.Ctx.Logger, tmdb.NewMemDB(), nil, true, 0,
-				encCfg, nil,
+				encCfg, 0,
 				simapp.EmptyAppOptions{},
 				baseapp.SetPruning(pruningtypes.NewPruningOptionsFromString(val.AppConfig.Pruning)),
 				baseapp.SetMinGasPrices(val.AppConfig.MinGasPrices),

test/util/test_app.go

@@ -62,7 +62,7 @@ func SetupTestAppWithGenesisValSet(cparams *tmproto.ConsensusParams, genAccounts
 		log.NewNopLogger(), db, nil, true,
 		cast.ToUint(emptyOpts.Get(server.FlagInvCheckPeriod)),
 		encCfg,
-		nil,
+		0,
 		emptyOpts,
 	)
 

x/upgrade/keeper.go

@@ -1,8 +1,6 @@
 package upgrade
 
 import (
-	fmt "fmt"
-
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
@@ -16,26 +14,18 @@ type Keeper struct {
 	// safely be ported over without any migration
 	storeKey storetypes.StoreKey
 
-	// in memory copy of the upgrade schedule if any. This is local per node
+	// in memory copy of the upgrade height if any. This is local per node
 	// and configured from the config.
-	upgradeSchedule map[string]Schedule
-
-	// the app version that should be set in end blocker
-	pendingAppVersion uint64
+	upgradeHeight int64
 }
 
 type VersionSetter func(version uint64)
 
 // NewKeeper constructs an upgrade keeper
-func NewKeeper(storeKey storetypes.StoreKey, upgradeSchedule map[string]Schedule) Keeper {
-	for chainID, schedule := range upgradeSchedule {
-		if err := schedule.ValidateBasic(); err != nil {
-			panic(fmt.Sprintf("invalid schedule %s: %v", chainID, err))
-		}
-	}
+func NewKeeper(storeKey storetypes.StoreKey, upgradeHeight int64) Keeper {
 	return Keeper{
-		storeKey:        storeKey,
-		upgradeSchedule: upgradeSchedule,
+		storeKey:      storeKey,
+		upgradeHeight: upgradeHeight,
 	}
 }
 
@@ -99,3 +89,9 @@ func (k Keeper) ClearIBCState(ctx sdk.Context, lastHeight int64) {
 	store.Delete(types.UpgradedClientKey(lastHeight))
 	store.Delete(types.UpgradedConsStateKey(lastHeight))
 }
+
+// ShouldUpgrade returns true if the current height is one before
+// the locally provided upgrade height that is passed as a flag
+func (k Keeper) ShouldUpgrade(height int64) bool {
+	return k.upgradeHeight == height+1
+}