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

Return IBC packet sequence number #1225

Merged
merged 4 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
18 changes: 17 additions & 1 deletion docs/proto/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
- [cosmwasm/wasm/v1/ibc.proto](#cosmwasm/wasm/v1/ibc.proto)
- [MsgIBCCloseChannel](#cosmwasm.wasm.v1.MsgIBCCloseChannel)
- [MsgIBCSend](#cosmwasm.wasm.v1.MsgIBCSend)
- [MsgIBCSendResponse](#cosmwasm.wasm.v1.MsgIBCSendResponse)

- [cosmwasm/wasm/v1/proposal.proto](#cosmwasm/wasm/v1/proposal.proto)
- [AccessConfigUpdate](#cosmwasm.wasm.v1.AccessConfigUpdate)
Expand Down Expand Up @@ -573,6 +574,21 @@ MsgIBCSend




<a name="cosmwasm.wasm.v1.MsgIBCSendResponse"></a>

### MsgIBCSendResponse
MsgIBCSendResponse


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `sequence` | [uint64](#uint64) | | Sequence number of the IBC packet sent |





<!-- end messages -->

<!-- end enums -->
Expand Down Expand Up @@ -1457,7 +1473,7 @@ MsgStoreCode submit Wasm code to the system

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `sender` | [string](#string) | | Sender is the that actor that signed the messages |
| `sender` | [string](#string) | | Sender is the actor that signed the messages |
| `wasm_byte_code` | [bytes](#bytes) | | WASMByteCode can be raw or gzip compressed |
| `instantiate_permission` | [AccessConfig](#cosmwasm.wasm.v1.AccessConfig) | | InstantiatePermission access control to apply on contract creation, optional |

Expand Down
6 changes: 6 additions & 0 deletions proto/cosmwasm/wasm/v1/ibc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ message MsgIBCSend {
message MsgIBCCloseChannel {
string channel = 2 [ (gogoproto.moretags) = "yaml:\"source_channel\"" ];
}

// MsgIBCSendResponse
message MsgIBCSendResponse {
// Sequence number of the IBC packet sent
uint64 sequence = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proto/cosmwasm/wasm/v1/tx.proto is not super consistent but usually we have the response type right after the message type. I think that's helpful for navigating. I'd move it above message MsgIBCCloseChannel {.

15 changes: 13 additions & 2 deletions x/wasm/keeper/handler_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func NewIBCRawPacketHandler(chk types.ChannelKeeper, cak types.CapabilityKeeper)
}

// DispatchMsg publishes a raw IBC packet onto the channel.
func (h IBCRawPacketHandler) DispatchMsg(ctx sdk.Context, _ sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) (events []sdk.Event, data [][]byte, err error) {
func (h IBCRawPacketHandler) DispatchMsg(ctx sdk.Context, _ sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) ([]sdk.Event, [][]byte, error) {
if msg.IBC == nil || msg.IBC.SendPacket == nil {
return nil, nil, types.ErrUnknownMsg
}
Expand Down Expand Up @@ -188,7 +188,18 @@ func (h IBCRawPacketHandler) DispatchMsg(ctx sdk.Context, _ sdk.AccAddress, cont
ConvertWasmIBCTimeoutHeightToCosmosHeight(msg.IBC.SendPacket.Timeout.Block),
msg.IBC.SendPacket.Timeout.Timestamp,
)
return nil, nil, h.channelKeeper.SendPacket(ctx, channelCap, packet)

if err := h.channelKeeper.SendPacket(ctx, channelCap, packet); err != nil {
return nil, nil, sdkerrors.Wrap(err, "failed to send packet")
}

resp := &types.MsgIBCSendResponse{Sequence: sequence}
val, err := resp.Marshal()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does work correct!
I had expected to see the codec.Codec passed to the IBCRawPacketHandler object as an abstraction layer but this is fine.

if err != nil {
return nil, nil, sdkerrors.Wrap(err, "failed to marshal sequence response")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, nil, sdkerrors.Wrap(err, "failed to marshal sequence response")
return nil, nil, sdkerrors.Wrap(err, "failed to marshal IBC send response")

}

return nil, [][]byte{val}, nil
}

var _ Messenger = MessageHandlerFunc(nil)
Expand Down
13 changes: 11 additions & 2 deletions x/wasm/keeper/handler_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,23 @@ func TestIBCRawPacketHandler(t *testing.T) {
capturedPacket = nil
// when
h := NewIBCRawPacketHandler(spec.chanKeeper, spec.capKeeper)
data, evts, gotErr := h.DispatchMsg(ctx, RandomAccountAddress(t), ibcPort, wasmvmtypes.CosmosMsg{IBC: &wasmvmtypes.IBCMsg{SendPacket: &spec.srcMsg}})
evts, data, gotErr := h.DispatchMsg(ctx, RandomAccountAddress(t), ibcPort, wasmvmtypes.CosmosMsg{IBC: &wasmvmtypes.IBCMsg{SendPacket: &spec.srcMsg}})
// then
require.True(t, spec.expErr.Is(gotErr), "exp %v but got %#+v", spec.expErr, gotErr)
if spec.expErr != nil {
return
}
assert.Nil(t, data)

assert.Nil(t, evts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? The events are nil? How do we get the ibc packet events to the relayers here?
(I guess they just pass through context and not via the return value?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, via context.

require.NotNil(t, data)

expMsg := types.MsgIBCSendResponse{Sequence: 1}

actualMsg := types.MsgIBCSendResponse{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test case

err := actualMsg.Unmarshal(data[0])
require.NoError(t, err)

assert.Equal(t, expMsg, actualMsg)
assert.Equal(t, spec.expPacketSent, capturedPacket)
})
}
Expand Down
197 changes: 177 additions & 20 deletions x/wasm/types/ibc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/wasm/types/tx.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.