From 926c4f389fc9e12fcb16aa308e101f550f0d7ba8 Mon Sep 17 00:00:00 2001 From: Callum Waters 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. (cherry picked from commit 03b83f88851d7f19e7d7bc10230ad89832e9b96a) --- 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 f48cdf2655..e9427f8b55 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 f545a0f41b..0d2e4b8c7e 100644 --- a/x/upgrade/test/integration_test.go +++ b/x/upgrade/test/integration_test.go @@ -14,6 +14,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" @@ -148,6 +150,35 @@ func (s *UpgradeTestSuite) TestNewGovUpgradeFailure() { require.Contains(t, finalResult.TxResult.Log, "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 {