From fbef450e16f1ca139588e62e467043f126976b9e Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 14 May 2024 09:42:57 +0200 Subject: [PATCH 1/6] imp: add updateClientCheckTx to redunant relayer ante decorator (#6279) * imp: add checkTxUpdateClient to redunant relayer ante decorator * chore: update godoc and duplicate imports * test: add coverage for checkTxUpdateClient func * chore: rename ante func to updateClientCheckTx --- modules/core/ante/ante.go | 39 ++++++++++++++++++++++++++++++++-- modules/core/ante/ante_test.go | 38 ++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go index 7b8cdb326de..2295df4a328 100644 --- a/modules/core/ante/ante.go +++ b/modules/core/ante/ante.go @@ -1,10 +1,13 @@ package ante import ( + errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + "github.com/cosmos/ibc-go/v7/modules/core/exported" "github.com/cosmos/ibc-go/v7/modules/core/keeper" ) @@ -70,8 +73,7 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula packetMsgs++ case *clienttypes.MsgUpdateClient: - _, err := rrd.k.UpdateClient(sdk.WrapSDKContext(ctx), msg) - if err != nil { + if err := rrd.updateClientCheckTx(ctx, msg); err != nil { return ctx, err } @@ -90,3 +92,36 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } return next(ctx, tx, simulate) } + +// updateClientCheckTx runs a subset of ibc client update logic to be used specifically within the RedundantRelayDecorator AnteHandler. +// The following function performs ibc client message verification for CheckTx only and state updates in both CheckTx and ReCheckTx. +// Note that misbehaviour checks are omitted. +func (rrd RedundantRelayDecorator) updateClientCheckTx(ctx sdk.Context, msg *clienttypes.MsgUpdateClient) error { + clientMsg, err := clienttypes.UnpackClientMessage(msg.ClientMessage) + if err != nil { + return err + } + + clientState, found := rrd.k.ClientKeeper.GetClientState(ctx, msg.ClientId) + if !found { + return errorsmod.Wrapf(clienttypes.ErrClientNotFound, msg.ClientId) + } + + if status := rrd.k.ClientKeeper.GetClientStatus(ctx, clientState, msg.ClientId); status != exported.Active { + return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "cannot update client (%s) with status %s", msg.ClientId, status) + } + + clientStore := rrd.k.ClientKeeper.ClientStore(ctx, msg.ClientId) + + if !ctx.IsReCheckTx() { + if err := clientState.VerifyClientMessage(ctx, rrd.k.Codec(), clientStore, clientMsg); err != nil { + return err + } + } + + heights := clientState.UpdateState(ctx, rrd.k.Codec(), clientStore, clientMsg) + + ctx.Logger().With("module", "x/"+exported.ModuleName).Debug("ante ibc client update", "consensusHeights", heights) + + return nil +} diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index aca0fefd9a7..ace412d11c5 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -3,8 +3,10 @@ package ante_test import ( "testing" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" @@ -12,6 +14,7 @@ import ( host "github.com/cosmos/ibc-go/v7/modules/core/24-host" "github.com/cosmos/ibc-go/v7/modules/core/ante" "github.com/cosmos/ibc-go/v7/modules/core/exported" + ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v7/testing" ) @@ -386,6 +389,39 @@ func (suite *AnteTestSuite) TestAnteDecorator() { }, false, }, + { + "no success on one new UpdateClient message: invalid client identifier", + func(suite *AnteTestSuite) []sdk.Msg { + clientMsg, err := codectypes.NewAnyWithValue(&ibctm.Header{}) + suite.Require().NoError(err) + + msgs := []sdk.Msg{&clienttypes.MsgUpdateClient{ClientId: ibctesting.InvalidID, ClientMessage: clientMsg}} + return msgs + }, + false, + }, + { + "no success on one new UpdateClient message: client module not found", + func(suite *AnteTestSuite) []sdk.Msg { + clientMsg, err := codectypes.NewAnyWithValue(&ibctm.Header{}) + suite.Require().NoError(err) + + msgs := []sdk.Msg{&clienttypes.MsgUpdateClient{ClientId: clienttypes.FormatClientIdentifier("08-wasm", 1), ClientMessage: clientMsg}} + return msgs + }, + false, + }, + { + "no success on one new UpdateClient message: no consensus state for trusted height", + func(suite *AnteTestSuite) []sdk.Msg { + clientMsg, err := codectypes.NewAnyWithValue(&ibctm.Header{TrustedHeight: clienttypes.NewHeight(1, 10000)}) + suite.Require().NoError(err) + + msgs := []sdk.Msg{&clienttypes.MsgUpdateClient{ClientId: suite.path.EndpointA.ClientID, ClientMessage: clientMsg}} + return msgs + }, + false, + }, { "no success on three new UpdateClient messages and three redundant messages of each type", func(suite *AnteTestSuite) []sdk.Msg { From 3a37e0ed6e2365c0ca39917dba2dba380f22e270 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 14 May 2024 10:08:30 +0200 Subject: [PATCH 2/6] refactor: ignore misbehaviour types for UpdateState in ante handler --- modules/core/ante/ante.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go index 2295df4a328..d3d7b01b906 100644 --- a/modules/core/ante/ante.go +++ b/modules/core/ante/ante.go @@ -9,6 +9,8 @@ import ( channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" "github.com/cosmos/ibc-go/v7/modules/core/exported" "github.com/cosmos/ibc-go/v7/modules/core/keeper" + solomachine "github.com/cosmos/ibc-go/v7/modules/light-clients/06-solomachine" + tendermint "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" ) type RedundantRelayDecorator struct { @@ -119,9 +121,17 @@ func (rrd RedundantRelayDecorator) updateClientCheckTx(ctx sdk.Context, msg *cli } } - heights := clientState.UpdateState(ctx, rrd.k.Codec(), clientStore, clientMsg) - - ctx.Logger().With("module", "x/"+exported.ModuleName).Debug("ante ibc client update", "consensusHeights", heights) + // NOTE: the folowing avoids panics in ante handler client updates for ibc-go v7.4.x + // without state machine breaking changes within light client modules. + switch clientMsg.(type) { + case *solomachine.Misbehaviour: + // ignore solomachine misbehaviour for update state in ante + case *tendermint.Misbehaviour: + // ignore tendermint misbehaviour for update state in ante + default: + heights := clientState.UpdateState(ctx, rrd.k.Codec(), clientStore, clientMsg) + ctx.Logger().With("module", "x/"+exported.ModuleName).Debug("ante ibc client update", "consensusHeights", heights) + } return nil } From 95f04928501647a4e5a8cb7123d3bee3c175d5c8 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 14 May 2024 10:18:44 +0200 Subject: [PATCH 3/6] chore: make lint-fix --- modules/core/ante/ante.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go index d3d7b01b906..5bfbece77a8 100644 --- a/modules/core/ante/ante.go +++ b/modules/core/ante/ante.go @@ -121,7 +121,7 @@ func (rrd RedundantRelayDecorator) updateClientCheckTx(ctx sdk.Context, msg *cli } } - // NOTE: the folowing avoids panics in ante handler client updates for ibc-go v7.4.x + // NOTE: the following avoids panics in ante handler client updates for ibc-go v7.4.x // without state machine breaking changes within light client modules. switch clientMsg.(type) { case *solomachine.Misbehaviour: From 786a4b652a41adb52a4837b7826c60be06691ea4 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 15 May 2024 10:26:26 +0200 Subject: [PATCH 4/6] test: add misbehaviour testcases to ante handler --- modules/core/ante/ante_test.go | 59 ++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index ace412d11c5..7d9333ddc29 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -2,6 +2,7 @@ package ante_test import ( "testing" + "time" "github.com/stretchr/testify/require" @@ -344,6 +345,64 @@ func (suite *AnteTestSuite) TestAnteDecorator() { }, true, }, + { + "success on new UpdateClient messages: solomachine misbehaviour", + func(suite *AnteTestSuite) []sdk.Msg { + solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainB.Codec, "06-solomachine-0", "testing", 1) + suite.chainB.GetSimApp().GetIBCKeeper().ClientKeeper.SetClientState(suite.chainB.GetContext(), solomachine.ClientID, solomachine.ClientState()) + + msgUpdateClient, err := clienttypes.NewMsgUpdateClient(solomachine.ClientID, solomachine.CreateMisbehaviour(), suite.chainB.SenderAccount.GetAddress().String()) + suite.Require().NoError(err) + + msgs := []sdk.Msg{msgUpdateClient} + + return msgs + }, + true, + }, + { + "success on new UpdateClient messages: solomachine multisig misbehaviour", + func(suite *AnteTestSuite) []sdk.Msg { + solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-0", "testing", 4) + suite.chainB.GetSimApp().GetIBCKeeper().ClientKeeper.SetClientState(suite.chainB.GetContext(), solomachine.ClientID, solomachine.ClientState()) + + msgUpdateClient, err := clienttypes.NewMsgUpdateClient(solomachine.ClientID, solomachine.CreateMisbehaviour(), suite.chainB.SenderAccount.GetAddress().String()) + suite.Require().NoError(err) + + msgs := []sdk.Msg{msgUpdateClient} + + return msgs + }, + true, + }, + { + "success on new UpdateClient messages: tendermint misbehaviour", + func(suite *AnteTestSuite) []sdk.Msg { + trustedHeight := suite.path.EndpointB.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainA.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := suite.path.EndpointB.UpdateClient() + suite.Require().NoError(err) + + height := suite.path.EndpointB.GetClientState().GetLatestHeight().(clienttypes.Height) + + // construct valid fork misbehaviour: two headers at the same height with different time + misbehaviour := &ibctm.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainA.CurrentHeader.Time.Add(time.Minute), suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), + Header2: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainA.CurrentHeader.Time, suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), + } + + msgUpdateClient, err := clienttypes.NewMsgUpdateClient(suite.path.EndpointB.ClientID, misbehaviour, suite.chainB.SenderAccount.GetAddress().String()) + suite.Require().NoError(err) + + msgs := []sdk.Msg{msgUpdateClient} + + return msgs + }, + true, + }, { "no success on one redundant RecvPacket message", func(suite *AnteTestSuite) []sdk.Msg { From 38317ea0d47b9ef5da82f1ad7a757be6909c80be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 21 May 2024 12:25:51 +0200 Subject: [PATCH 5/6] lint --- modules/core/ante/ante_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index 9862cd54a27..0ce6b9df37a 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -5,11 +5,9 @@ import ( "time" "github.com/stretchr/testify/require" - - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" From d2ec6e80db8f398ffce88622465aa9400c320187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 21 May 2024 12:32:02 +0200 Subject: [PATCH 6/6] chore: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68ca42a8096..205e26b4e56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (apps/27-interchain-accounts) [\#6144](https://github.com/cosmos/ibc-go/pull/6144) Emit an event signalling that the host submodule is disabled. * (core/ante) [\#6302](https://github.com/cosmos/ibc-go/pull/6302) Performance: Skip app callbacks during RecvPacket execution in checkTx within the redundant relay ante handler. * (core/ante) [\#6280](https://github.com/cosmos/ibc-go/pull/6280) Performance: Skip redundant proof checking in RecvPacket execution in reCheckTx within the redundant relay ante handler. +* (core/ante) [\#6306](https://github.com/cosmos/ibc-go/pull/6306) Performance: Skip misbehaviour checks in UpdateClient flow and skip signature checks in reCheckTx mode. ### Features