replace channel keeper with IBC keeper in AnteDecorator (#950) (#1096)

* replace channel keeper with IBC keeper in AnteDecorator and pass message to rpc handler

* fix error checking condition

* fix for proper way of getting go context

* refactor tests for ante handler

* review comment

* review comments and some fixes

* review comments

* execute message for update client as well

* add migration

Co-authored-by: Carlos Rodriguez <crodveg@gmail.com>
(cherry picked from commit f0b94df)

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

CHANGELOG.md

@@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (testing) [\#776](https://github.com/cosmos/ibc-go/pull/776) Adding helper fn to generate capability name for testing callbacks 
 * (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. Allows for the mock module to be reused as a base application in middleware stacks.
 * (channel) [\#882](https://github.com/cosmos/ibc-go/pull/882) The `WriteAcknowledgement` API now takes `exported.Acknowledgement` instead of a byte array
+* (modules/core/ante) [\#950](https://github.com/cosmos/ibc-go/pull/950) Replaces the channel keeper with the IBC keeper in the IBC `AnteDecorator` in order to execute the entire message and be able to reject redundant messages that are in the same block as the non-redundant messages.
 
 ### State Machine Breaking
 

docs/ibc/proto-docs.md

@@ -127,6 +127,8 @@
     - [MsgTimeoutOnCloseResponse](#ibc.core.channel.v1.MsgTimeoutOnCloseResponse)
     - [MsgTimeoutResponse](#ibc.core.channel.v1.MsgTimeoutResponse)
 
+    - [ResponseResultType](#ibc.core.channel.v1.ResponseResultType)
+
     - [Msg](#ibc.core.channel.v1.Msg)
 
 - [ibc/core/client/v1/genesis.proto](#ibc/core/client/v1/genesis.proto)
@@ -1738,6 +1740,11 @@ MsgAcknowledgement receives incoming IBC acknowledgement
 MsgAcknowledgementResponse defines the Msg/Acknowledgement response type.
 
 
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `result` | [ResponseResultType](#ibc.core.channel.v1.ResponseResultType) |  |  |
+
+
 
 
 
@@ -1954,6 +1961,11 @@ MsgRecvPacket receives incoming IBC packet
 MsgRecvPacketResponse defines the Msg/RecvPacket response type.
 
 
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `result` | [ResponseResultType](#ibc.core.channel.v1.ResponseResultType) |  |  |
+
+
 
 
 
@@ -2003,6 +2015,11 @@ MsgTimeoutOnClose timed-out packet upon counterparty channel closure.
 MsgTimeoutOnCloseResponse defines the Msg/TimeoutOnClose response type.
 
 
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `result` | [ResponseResultType](#ibc.core.channel.v1.ResponseResultType) |  |  |
+
+
 
 
 
@@ -2013,11 +2030,29 @@ MsgTimeoutOnCloseResponse defines the Msg/TimeoutOnClose response type.
 MsgTimeoutResponse defines the Msg/Timeout response type.
 
 
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `result` | [ResponseResultType](#ibc.core.channel.v1.ResponseResultType) |  |  |
+
+
 
 
 
  <!-- end messages -->
 
+
+<a name="ibc.core.channel.v1.ResponseResultType"></a>
+
+### ResponseResultType
+ResponseResultType defines the possible outcomes of the execution of a message
+
+| Name | Number | Description |
+| ---- | ------ | ----------- |
+| RESPONSE_RESULT_UNSPECIFIED | 0 | Default zero value enumeration |
+| RESPONSE_RESULT_NOOP | 1 | The message did not call the IBC application callbacks (because, for example, the packet had already been relayed) |
+| RESPONSE_RESULT_SUCCESS | 2 | The message was executed successfully |
+
+
  <!-- end enums -->
 
  <!-- end HasExtensions -->

docs/migrations/v2-to-v3.md

@@ -109,6 +109,22 @@ The migration code required may look like:
     appState[icatypes.ModuleName] = clientCtx.JSONCodec.MustMarshalJSON(icaGenesisState)
 ```
 
+### Ante decorator
+
+The field of type `channelkeeper.Keeper` in the `AnteDecorator` structure has been replaced with a field of type `*keeper.Keeper`:
+
+```diff
+type AnteDecorator struct {
+-    k channelkeeper.Keeper
++    k *keeper.Keeper
+}
+
+- func NewAnteDecorator(k channelkeeper.Keeper) AnteDecorator {
++ func NewAnteDecorator(k *keeper.Keeper) AnteDecorator {
+    return AnteDecorator{k: k}
+}
+```
+
 ## IBC Apps
 
 

modules/core/ante/ante.go

@@ -4,22 +4,23 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
-	channelkeeper "github.com/cosmos/ibc-go/v3/modules/core/04-channel/keeper"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+	"github.com/cosmos/ibc-go/v3/modules/core/keeper"
 )
 
 type AnteDecorator struct {
-	k channelkeeper.Keeper
+	k *keeper.Keeper
 }
 
-func NewAnteDecorator(k channelkeeper.Keeper) AnteDecorator {
+func NewAnteDecorator(k *keeper.Keeper) AnteDecorator {
 	return AnteDecorator{k: k}
 }
 
-// AnteDecorator returns an error if a multiMsg tx only contains packet messages (Recv, Ack, Timeout) and additional update messages and all packet messages
-// are redundant. If the transaction is just a single UpdateClient message, or the multimsg transaction contains some other message type, then the antedecorator returns no error
-// and continues processing to ensure these transactions are included.
-// This will ensure that relayers do not waste fees on multiMsg transactions when another relayer has already submitted all packets, by rejecting the tx at the mempool layer.
+// AnteDecorator returns an error if a multiMsg tx only contains packet messages (Recv, Ack, Timeout) and additional update messages
+// and all packet messages are redundant. If the transaction is just a single UpdateClient message, or the multimsg transaction
+// contains some other message type, then the antedecorator returns no error and continues processing to ensure these transactions
+// are included. This will ensure that relayers do not waste fees on multiMsg transactions when another relayer has already submitted
+// all packets, by rejecting the tx at the mempool layer.
 func (ad AnteDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
 	// do not run redundancy check on DeliverTx or simulate
 	if (ctx.IsCheckTx() || ctx.IsReCheckTx()) && !simulate {
@@ -29,39 +30,57 @@ func (ad AnteDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
 		for _, m := range tx.GetMsgs() {
 			switch msg := m.(type) {
 			case *channeltypes.MsgRecvPacket:
-				if _, found := ad.k.GetPacketReceipt(ctx, msg.Packet.GetDestPort(), msg.Packet.GetDestChannel(), msg.Packet.GetSequence()); found {
+				response, err := ad.k.RecvPacket(sdk.WrapSDKContext(ctx), msg)
+				if err != nil {
+					return ctx, err
+				}
+				if response.Result == channeltypes.NOOP {
 					redundancies += 1
 				}
 				packetMsgs += 1
 
 			case *channeltypes.MsgAcknowledgement:
-				if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 {
+				response, err := ad.k.Acknowledgement(sdk.WrapSDKContext(ctx), msg)
+				if err != nil {
+					return ctx, err
+				}
+				if response.Result == channeltypes.NOOP {
 					redundancies += 1
 				}
 				packetMsgs += 1
 
 			case *channeltypes.MsgTimeout:
-				if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 {
+				response, err := ad.k.Timeout(sdk.WrapSDKContext(ctx), msg)
+				if err != nil {
+					return ctx, err
+				}
+				if response.Result == channeltypes.NOOP {
 					redundancies += 1
 				}
 				packetMsgs += 1
 
 			case *channeltypes.MsgTimeoutOnClose:
-				if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 {
+				response, err := ad.k.TimeoutOnClose(sdk.WrapSDKContext(ctx), msg)
+				if err != nil {
+					return ctx, err
+				}
+				if response.Result == channeltypes.NOOP {
 					redundancies += 1
 				}
 				packetMsgs += 1
 
 			case *clienttypes.MsgUpdateClient:
-				// do nothing here, as we want to avoid updating clients if it is batched with only redundant messages
+				_, err := ad.k.UpdateClient(sdk.WrapSDKContext(ctx), msg)
+				if err != nil {
+					return ctx, err
+				}
 
 			default:
 				// if the multiMsg tx has a msg that is not a packet msg or update msg, then we will not return error
 				// regardless of if all packet messages are redundant. This ensures that non-packet messages get processed
 				// even if they get batched with redundant packet messages.
 				return next(ctx, tx, simulate)
 			}
-
 		}
 
 		// only return error if all packet messages are redundant