From 03b83f88851d7f19e7d7bc10230ad89832e9b96a Mon Sep 17 00:00:00 2001
From: Callum Waters <cmwaters19@gmail.com>
Date: Thu, 28 Sep 2023 10:28:15 +0200
Subject: [PATCH] feat!: disable ibc upgrade proposal handler (#2574)

The proposal handler to upgrade IBC also invokes the standard
`ScheduleUpgrade` within the upgrading module. See
https://github.com/cosmos/ibc-go/blob/e2201aaf1b016356bbd40fcdc17988437adce5ae/modules/core/02-client/keeper/proposal.go#L82.
This effectively means that one can create a governance proposal to
upgrade the chain which is something we didn't want to support. As there
is no need to handle updating the IBC client yet, this temporarily
disables the proposal type.
---
 app/app.go                         |  3 +--
 app/ibc_proposal_handler.go        | 28 +++++++++++++++++++++++++++
 x/upgrade/test/integration_test.go | 31 ++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 app/ibc_proposal_handler.go

diff --git a/app/app.go b/app/app.go
index de40c4dac1..128aabc4af 100644
--- a/app/app.go
+++ b/app/app.go
@@ -72,7 +72,6 @@ import (
 	ibctransferkeeper "github.com/cosmos/ibc-go/v6/modules/apps/transfer/keeper"
 	ibctransfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
 	ibc "github.com/cosmos/ibc-go/v6/modules/core"
-	ibcclient "github.com/cosmos/ibc-go/v6/modules/core/02-client"
 	ibcclienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
 	ibcporttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	ibchost "github.com/cosmos/ibc-go/v6/modules/core/24-host"
@@ -361,7 +360,7 @@ func New(
 	govRouter := oldgovtypes.NewRouter()
 	govRouter.AddRoute(paramproposal.RouterKey, paramBlockList.GovHandler(app.ParamsKeeper)).
 		AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
-		AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
+		AddRoute(ibcclienttypes.RouterKey, NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
 
 	// Create Transfer Keepers
 	tokenFilterKeeper := tokenfilter.NewKeeper(app.IBCKeeper.ChannelKeeper)
diff --git a/app/ibc_proposal_handler.go b/app/ibc_proposal_handler.go
new file mode 100644
index 0000000000..5cc2a091df
--- /dev/null
+++ b/app/ibc_proposal_handler.go
@@ -0,0 +1,28 @@
+package app
+
+import (
+	"cosmossdk.io/errors"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
+
+	"github.com/cosmos/ibc-go/v6/modules/core/02-client/keeper"
+	"github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+)
+
+// NewClientProposalHandler defines the 02-client proposal handler. It disables the
+// UpgradeProposalType. Handling of updating the IBC Client will be done in v2 of the
+// app.
+func NewClientProposalHandler(k keeper.Keeper) govtypes.Handler {
+	return func(ctx sdk.Context, content govtypes.Content) error {
+		switch c := content.(type) {
+		case *types.ClientUpdateProposal:
+			return k.ClientUpdateProposal(ctx, c)
+		case *types.UpgradeProposal:
+			return errors.Wrap(sdkerrors.ErrInvalidRequest, "ibc upgrade proposal not supported")
+
+		default:
+			return errors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized ibc proposal content type: %T", c)
+		}
+	}
+}
diff --git a/x/upgrade/test/integration_test.go b/x/upgrade/test/integration_test.go
index 47e5c2d38b..b481c9ef3e 100644
--- a/x/upgrade/test/integration_test.go
+++ b/x/upgrade/test/integration_test.go
@@ -16,6 +16,8 @@ import (
 	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 	v1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	ibctypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	ibctmtypes "github.com/cosmos/ibc-go/v6/modules/light-clients/07-tendermint/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -149,6 +151,35 @@ func (s *UpgradeTestSuite) TestNewGovUpgradeFailure() {
 	require.Contains(t, res.RawLog, "proposal message not recognized by router")
 }
 
+func (s *UpgradeTestSuite) TestIBCUpgradeFailure() {
+	t := s.T()
+	plan := types.Plan{
+		Name:   "v2",
+		Height: 20,
+		Info:   "this should not pass",
+	}
+	upgradedClientState := &ibctmtypes.ClientState{}
+
+	upgradeMsg, err := ibctypes.NewUpgradeProposal("Upgrade to v2!", "Upgrade to v2!", plan, upgradedClientState)
+	require.NoError(t, err)
+
+	dep := sdk.NewCoins(sdk.NewCoin(app.BondDenom, sdk.NewInt(1000000000000)))
+	acc := s.unusedAccount()
+	accAddr := getAddress(acc, s.cctx.Keyring)
+	msg, err := v1beta1.NewMsgSubmitProposal(upgradeMsg, dep, accAddr)
+	require.NoError(t, err)
+
+	// submit the transaction and wait a block for it to be included
+	signer, err := testnode.NewSignerFromContext(s.cctx, acc)
+	require.NoError(t, err)
+	subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute)
+	defer cancel()
+	res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...)
+	require.Error(t, err)
+	require.EqualValues(t, 9, res.Code, res.RawLog) // we're only submitting the tx, so we expect everything to work
+	assert.Contains(t, res.RawLog, "ibc upgrade proposal not supported")
+}
+
 func getAddress(account string, kr keyring.Keyring) sdk.AccAddress {
 	rec, err := kr.Key(account)
 	if err != nil {