refactor(v7.4.x): performance improvements for update client in ante (#…

…6306) * 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 * refactor: ignore misbehaviour types for UpdateState in ante handler * chore: make lint-fix * test: add misbehaviour testcases to ante handler * lint * chore: add changelog entry --------- Co-authored-by: Colin Axnér <[email protected]>
cosmos · May 21, 2024 · ee842be · ee842be
1 parent f60998d
commit ee842be
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 2 deletions.
diff --git 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
 

diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go
@@ -1,12 +1,17 @@
 package ante
 
 import (
+	errorsmod "cosmossdk.io/errors"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	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"
+	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 {
@@ -83,8 +88,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
 				}
 
@@ -149,3 +153,44 @@ func (rrd RedundantRelayDecorator) recvPacketReCheckTx(ctx sdk.Context, msg *cha
 
 	return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil
 }
+
+// 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
+		}
+	}
+
+	// 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:
+		// 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
+}
diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go
@@ -2,17 +2,20 @@ package ante_test
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	"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"
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	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"
 )
 
@@ -342,6 +345,64 @@ func (suite *AnteTestSuite) TestAnteDecoratorCheckTx() {
 			},
 			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 {
@@ -387,6 +448,39 @@ func (suite *AnteTestSuite) TestAnteDecoratorCheckTx() {
 			},
 			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 {