Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace channel keeper with IBC keeper in AnteDecorator #950

Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
35 changes: 35 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) | | |





Expand Down Expand Up @@ -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) | | |





Expand Down Expand Up @@ -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) | | |





Expand All @@ -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 -->
Expand Down
16 changes: 16 additions & 0 deletions docs/migrations/v2-to-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,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


Expand Down
299 changes: 226 additions & 73 deletions modules/core/04-channel/types/tx.pb.go

Large diffs are not rendered by default.

45 changes: 32 additions & 13 deletions modules/core/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Loading