feat!: remove token voting upgrades (#1629)

## Overview

wraps the sdk's standard upgrade module to remove the ability to submit
upgrade proposals. The idea being that this maintains compatibility with
the IBC module (which requires access to the upgrade module's keeper),
while still removing the ability for the upgrade module to work by not
registering the upgrade module's msg server or begin block. Also, when
we implement #1014 we can progressively flesh this module out with our
own logic.

closes #1571 

## Checklist

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords

---------

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

app/app.go

@@ -66,9 +66,8 @@ 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"
-	"github.com/cosmos/cosmos-sdk/x/upgrade"
-	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
-	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/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"
@@ -91,6 +90,7 @@ import (
 	blobmodulekeeper "github.com/celestiaorg/celestia-app/x/blob/keeper"
 	blobmoduletypes "github.com/celestiaorg/celestia-app/x/blob/types"
 	"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"
@@ -149,7 +149,6 @@ var (
 		authzmodule.AppModuleBasic{},
 		feegrantmodule.AppModuleBasic{},
 		ibc.AppModuleBasic{},
-		upgrade.AppModuleBasic{},
 		evidence.AppModuleBasic{},
 		transfer.AppModuleBasic{},
 		vesting.AppModuleBasic{},
@@ -159,7 +158,7 @@ var (
 
 	// ModuleEncodingRegisters keeps track of all the module methods needed to
 	// register interfaces and specific type to encoding config
-	ModuleEncodingRegisters = moduleMapToSlice(ModuleBasics)
+	ModuleEncodingRegisters = extractRegisters(ModuleBasics, appupgrade.TypeRegister{})
 
 	// module account permissions
 	maccPerms = map[string][]string{
@@ -218,7 +217,7 @@ type App struct {
 	DistrKeeper      distrkeeper.Keeper
 	GovKeeper        govkeeper.Keeper
 	CrisisKeeper     crisiskeeper.Keeper
-	UpgradeKeeper    upgradekeeper.Keeper
+	UpgradeKeeper    sdkupgradekeeper.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
@@ -261,7 +260,7 @@ func New(
 	keys := sdk.NewKVStoreKeys(
 		authtypes.StoreKey, authzkeeper.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey,
 		minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
-		govtypes.StoreKey, paramstypes.StoreKey, upgradetypes.StoreKey, feegrant.StoreKey,
+		govtypes.StoreKey, paramstypes.StoreKey, sdkupgradetypes.StoreKey, feegrant.StoreKey,
 		evidencetypes.StoreKey, capabilitytypes.StoreKey,
 		blobmoduletypes.StoreKey,
 		qgbmoduletypes.StoreKey,
@@ -323,7 +322,7 @@ func New(
 	)
 
 	app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
-	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+	app.UpgradeKeeper = sdkupgradekeeper.NewKeeper(skipUpgradeHeights, keys[sdkupgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	app.QgbKeeper = *qgbmodulekeeper.NewKeeper(
 		appCodec,
@@ -354,7 +353,6 @@ func New(
 	govRouter.AddRoute(govtypes.RouterKey, oldgovtypes.ProposalHandler).
 		AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
 		AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
-		AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper)).
 		AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
 
 	// Create Transfer Keepers
@@ -425,7 +423,6 @@ func New(
 		slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
 		distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
 		staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
-		upgrade.NewAppModule(app.UpgradeKeeper),
 		evidence.NewAppModule(app.EvidenceKeeper),
 		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 		ibc.NewAppModule(app.IBCKeeper),
@@ -440,7 +437,6 @@ func New(
 	// CanWithdrawInvariant invariant.
 	// NOTE: staking module is required if HistoricalEntries param > 0
 	app.mm.SetOrderBeginBlockers(
-		upgradetypes.ModuleName,
 		capabilitytypes.ModuleName,
 		minttypes.ModuleName,
 		distrtypes.ModuleName,
@@ -466,7 +462,6 @@ func New(
 		crisistypes.ModuleName,
 		govtypes.ModuleName,
 		stakingtypes.ModuleName,
-		upgradetypes.ModuleName,
 		capabilitytypes.ModuleName,
 		minttypes.ModuleName,
 		distrtypes.ModuleName,
@@ -510,7 +505,7 @@ func New(
 		feegrant.ModuleName,
 		paramstypes.ModuleName,
 		authz.ModuleName,
-		upgradetypes.ModuleName,
+		sdkupgradetypes.ModuleName,
 	)
 
 	app.QueryRouter().AddRoute(proof.TxInclusionQueryPath, proof.QueryTxInclusionProof)
@@ -733,13 +728,18 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	return paramsKeeper
 }
 
-func moduleMapToSlice(m module.BasicManager) []encoding.ModuleRegister {
+// extractRegisters isolates the encoding module registers from the module
+// manager, and appends any solo registers.
+func extractRegisters(m module.BasicManager, soloRegisters ...encoding.ModuleRegister) []encoding.ModuleRegister {
 	// TODO: might be able to use some standard generics in go 1.18
-	s := make([]encoding.ModuleRegister, len(m))
+	s := make([]encoding.ModuleRegister, len(m)+len(soloRegisters))
 	i := 0
 	for _, v := range m {
 		s[i] = v
 		i++
 	}
+	for i, v := range soloRegisters {
+		s[i+len(m)] = v
+	}
 	return s
 }

app/test/integration_test.go

@@ -3,11 +3,11 @@ package app_test
 import (
 	"bytes"
 	"context"
-	"encoding/hex"
 	"testing"
 	"time"
 
 	"github.com/celestiaorg/celestia-app/test/util/blobfactory"
+	"github.com/celestiaorg/celestia-app/test/util/testnode"
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -29,7 +29,6 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
-	rpctypes "github.com/tendermint/tendermint/rpc/core/types"
 	coretypes "github.com/tendermint/tendermint/types"
 )
 
@@ -184,7 +183,7 @@ func (s *IntegrationTestSuite) TestMaxBlockSize() {
 
 			heights := make(map[int64]int)
 			for _, hash := range hashes {
-				resp, err := queryTx(val.ClientCtx, hash, true)
+				resp, err := testnode.QueryTx(val.ClientCtx, hash, true)
 				assert.NoError(err)
 				assert.NotNil(resp)
 				if resp == nil {
@@ -310,20 +309,6 @@ func (s *IntegrationTestSuite) TestUnwrappedPFBRejection() {
 	require.Equal(t, blobtypes.ErrNoBlobs.ABCICode(), res.Code)
 }
 
-func queryTx(clientCtx client.Context, hashHexStr string, prove bool) (*rpctypes.ResultTx, error) {
-	hash, err := hex.DecodeString(hashHexStr)
-	if err != nil {
-		return nil, err
-	}
-
-	node, err := clientCtx.GetNode()
-	if err != nil {
-		return nil, err
-	}
-
-	return node.Tx(context.Background(), hash, prove)
-}
-
 func (s *IntegrationTestSuite) TestShareInclusionProof() {
 	require := s.Require()
 	val := s.network.Validators[0]
@@ -352,7 +337,7 @@ func (s *IntegrationTestSuite) TestShareInclusionProof() {
 	s.WaitForBlocks(20)
 
 	for _, hash := range hashes {
-		txResp, err := queryTx(val.ClientCtx, hash, true)
+		txResp, err := testnode.QueryTx(val.ClientCtx, hash, true)
 		require.NoError(err)
 		require.Equal(abci.CodeTypeOK, txResp.TxResult.Code)
 

app/test/std_sdk_test.go

@@ -62,9 +62,10 @@ func (s *StandardSDKIntegrationTestSuite) unusedAccount() string {
 func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 	t := s.T()
 	type test struct {
-		name    string
-		msgFunc func() (msgs []sdk.Msg, signer string)
-		hash    string
+		name         string
+		msgFunc      func() (msgs []sdk.Msg, signer string)
+		hash         string
+		expectedCode uint32
 	}
 	tests := []test{
 		{
@@ -78,6 +79,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				)
 				return []sdk.Msg{msgSend}, account1
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "send 1,000,000 TIA",
@@ -90,6 +92,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				)
 				return []sdk.Msg{msgSend}, account1
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "delegate 1 TIA",
@@ -100,6 +103,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				msg := stakingtypes.NewMsgDelegate(account1Addr, valopAddr, sdk.NewCoin(app.BondDenom, sdk.NewInt(1000000)))
 				return []sdk.Msg{msg}, account1
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "undelegate 1 TIA",
@@ -109,6 +113,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				msg := stakingtypes.NewMsgUndelegate(valAccAddr, valopAddr, sdk.NewCoin(app.BondDenom, sdk.NewInt(1000000)))
 				return []sdk.Msg{msg}, "validator"
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "create validator",
@@ -130,6 +135,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				require.NoError(t, err)
 				return []sdk.Msg{msg}, account
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "create vesting account",
@@ -148,6 +154,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				)
 				return []sdk.Msg{msg}, sendAcc
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "create legacy governance proposal",
@@ -165,6 +172,9 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				require.NoError(t, err)
 				return []sdk.Msg{msg}, account
 			},
+			// despite token voting being removed, we still expect a code of 0.
+			// However, the tx still fails. See tests in local upgrade module
+			expectedCode: abci.CodeTypeOK,
 		},
 		{
 			name: "multiple send sdk.Msgs in one sdk.Tx",
@@ -183,6 +193,7 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 				)
 				return []sdk.Msg{msgSend1, msgSend2}, account1
 			},
+			expectedCode: abci.CodeTypeOK,
 		},
 	}
 
@@ -192,16 +203,16 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() {
 		res, err := testnode.SignAndBroadcastTx(s.ecfg, s.cctx.Context, signer, msgs...)
 		require.NoError(t, err)
 		require.NotNil(t, res)
-		require.Equal(t, abci.CodeTypeOK, res.Code, tt.name)
+		assert.Equal(t, abci.CodeTypeOK, res.Code, tt.name)
 		tests[i].hash = res.TxHash
 	}
 
 	require.NoError(s.T(), s.cctx.WaitForNextBlock())
 
 	for _, tt := range tests {
-		res, err := queryTx(s.cctx.Context, tt.hash, true)
-		require.NoError(t, err)
-		assert.Equal(t, abci.CodeTypeOK, res.TxResult.Code, tt.name)
+		res, err := testnode.QueryTx(s.cctx.Context, tt.hash, true)
+		assert.NoError(t, err)
+		assert.Equal(t, tt.expectedCode, res.TxResult.Code, tt.name)
 	}
 }
 

test/util/testnode/query.go

@@ -0,0 +1,23 @@
+package testnode
+
+import (
+	"context"
+	"encoding/hex"
+
+	"github.com/cosmos/cosmos-sdk/client"
+	rpctypes "github.com/tendermint/tendermint/rpc/core/types"
+)
+
+func QueryTx(clientCtx client.Context, hashHexStr string, prove bool) (*rpctypes.ResultTx, error) {
+	hash, err := hex.DecodeString(hashHexStr)
+	if err != nil {
+		return nil, err
+	}
+
+	node, err := clientCtx.GetNode()
+	if err != nil {
+		return nil, err
+	}
+
+	return node.Tx(context.Background(), hash, prove)
+}

x/upgrade/README.md

@@ -0,0 +1,8 @@
+# `x/upgrade`
+
+## Abstract
+
+The upgrade module removes the entrypoints to the standard upgrade module by not
+registering a message server. It registers the standard upgrade module types to
+preserve the ability to marshal them. Note that the keeper of the standard
+upgrade module is still added to the application.

x/upgrade/module.go

@@ -0,0 +1,21 @@
+package upgrade
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
+)
+
+// TypeRegister is used to register the upgrade modules types in the encoding
+// config without defining an entire module.
+type TypeRegister struct{}
+
+// RegisterLegacyAminoCodec registers the upgrade types on the LegacyAmino codec.
+func (TypeRegister) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
+	upgradetypes.RegisterLegacyAminoCodec(cdc)
+}
+
+// RegisterInterfaces registers the upgrade module types.
+func (TypeRegister) RegisterInterfaces(registry codectypes.InterfaceRegistry) {
+	upgradetypes.RegisterInterfaces(registry)
+}