Skip to content

Commit

Permalink
Redesign IBC on packet recv error/ result.Err handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@alpe
alpe committed Apr 21, 2023
1 parent 0d73f57 commit b561461
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 58 deletions.
34 changes: 17 additions & 17 deletions x/wasm/ibc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,26 +264,26 @@ func (i IBCHandler) OnRecvPacket(
) ibcexported.Acknowledgement {
contractAddr, err := ContractFromPortID(packet.DestinationPort)
if err != nil {
return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(err, "contract port id"))
// this must not happen as ports were registered before
panic(errorsmod.Wrapf(err, "contract port id"))
}

em := sdk.NewEventManager()
msg := wasmvmtypes.IBCPacketReceiveMsg{Packet: newIBCPacket(packet), Relayer: relayer.String()}
ack, err := i.keeper.OnRecvPacket(ctx, contractAddr, msg)
ack, err := i.keeper.OnRecvPacket(ctx.WithEventManager(em), contractAddr, msg)
if err != nil {
return channeltypes.NewErrorAcknowledgement(err)
}
return ContractConfirmStateAck(ack)
}

var _ ibcexported.Acknowledgement = ContractConfirmStateAck{}

type ContractConfirmStateAck []byte

func (w ContractConfirmStateAck) Success() bool {
return true // always commit state
}

func (w ContractConfirmStateAck) Acknowledgement() []byte {
return w
ack = channeltypes.NewErrorAcknowledgement(err)
types.EmitAcknowledgementEvent(ctx, contractAddr, ack, err)
// the state gets reverted, so we drop all captured events
return ack
}
if ack == nil || ack.Success() {
// emit all contract and submessage events on success
// nil ack is a success case, see: https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/core/keeper/msg_server.go#L453
ctx.EventManager().EmitEvents(em.Events())
}
types.EmitAcknowledgementEvent(ctx, contractAddr, ack, nil)
return ack
}

// OnAcknowledgementPacket implements the IBCModule interface
Expand Down
128 changes: 128 additions & 0 deletions x/wasm/ibc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,125 @@ import (
"testing"

wasmvmtypes "github.com/CosmWasm/wasmvm/types"
abci "github.com/cometbft/cometbft/abci/types"
"github.com/cometbft/cometbft/libs/rand"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/CosmWasm/wasmd/x/wasm/keeper"
"github.com/CosmWasm/wasmd/x/wasm/types"
)

func TestOnRecvPacket(t *testing.T) {
anyRelayerAddr := sdk.AccAddress(rand.Bytes(address.Len))
anyContractIBCPkg := IBCPacketFixture(func(p *channeltypes.Packet) {
p.DestinationPort = "wasm.cosmos1w09vr7rpe2agu0kg2zlpkdckce865l3zps8mxjurxthfh3m7035qe5hh7f"
})
myCustomEvent := sdk.NewEvent("testing")
specs := map[string]struct {
ibcPkg channeltypes.Packet
contractRsp ibcexported.Acknowledgement
contractErr error
expEvents sdk.Events
expPanic bool
expAck ibcexported.Acknowledgement
}{
"contract returns success response": {
ibcPkg: anyContractIBCPkg,
contractRsp: keeper.ContractConfirmStateAck([]byte{1}),
expAck: keeper.ContractConfirmStateAck([]byte{1}),
expEvents: sdk.Events{
myCustomEvent,
{
Type: "ibc_packet_received",
Attributes: []abci.EventAttribute{
{Key: "module", Value: "wasm"},
{Key: "_contract_address", Value: "cosmos1w09vr7rpe2agu0kg2zlpkdckce865l3zps8mxjurxthfh3m7035qe5hh7f"},
{Key: "success", Value: "true"},
},
},
},
},
"contract returns err response": {
ibcPkg: anyContractIBCPkg,
contractRsp: channeltypes.NewErrorAcknowledgement(types.ErrInvalid.Wrap("testing")),
expAck: channeltypes.NewErrorAcknowledgement(types.ErrInvalid.Wrap("testing")),
expEvents: sdk.Events{
{
Type: "ibc_packet_received",
Attributes: []abci.EventAttribute{
{Key: "module", Value: "wasm"},
{Key: "_contract_address", Value: "cosmos1w09vr7rpe2agu0kg2zlpkdckce865l3zps8mxjurxthfh3m7035qe5hh7f"},
{Key: "success", Value: "false"},
},
},
},
},
"nil considered success response": { // regression only
ibcPkg: anyContractIBCPkg,
expEvents: sdk.Events{
myCustomEvent,
{
Type: "ibc_packet_received",
Attributes: []abci.EventAttribute{
{Key: "module", Value: "wasm"},
{Key: "_contract_address", Value: "cosmos1w09vr7rpe2agu0kg2zlpkdckce865l3zps8mxjurxthfh3m7035qe5hh7f"},
{Key: "success", Value: "true"},
},
},
},
},
"unknown contract port": {
ibcPkg: IBCPacketFixture(func(p *channeltypes.Packet) {
p.DestinationPort = "not-a-contract-port"
}),
expPanic: true,
},
"contract executed with error": {
ibcPkg: anyContractIBCPkg,
contractErr: types.ErrInvalid.Wrap("testing"),
expAck: channeltypes.NewErrorAcknowledgement(types.ErrInvalid.Wrap("testing")),
expEvents: sdk.Events{{
Type: "ibc_packet_received",
Attributes: []abci.EventAttribute{
{Key: "module", Value: "wasm"},
{Key: "_contract_address", Value: "cosmos1w09vr7rpe2agu0kg2zlpkdckce865l3zps8mxjurxthfh3m7035qe5hh7f"},
{Key: "success", Value: "false"},
{Key: "error", Value: "testing: invalid"}, // not redacted
},
}},
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
mock := IBCContractKeeperMock{
OnRecvPacketFn: func(ctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.IBCPacketReceiveMsg) (ibcexported.Acknowledgement, error) {
// additional custom event to confirm event handling on state commit/ rollback
ctx.EventManager().EmitEvent(myCustomEvent)
return spec.contractRsp, spec.contractErr
},
}
h := NewIBCHandler(mock, nil, nil)
em := &sdk.EventManager{}
ctx := sdk.Context{}.WithEventManager(em)
if spec.expPanic {
require.Panics(t, func() {
_ = h.OnRecvPacket(ctx, spec.ibcPkg, anyRelayerAddr)
})
return
}
gotAck := h.OnRecvPacket(ctx, spec.ibcPkg, anyRelayerAddr)
assert.Equal(t, spec.expAck, gotAck)
assert.Equal(t, spec.expEvents, em.Events())
})
}
}

func TestMapToWasmVMIBCPacket(t *testing.T) {
var myTimestamp uint64 = 1
specs := map[string]struct {
Expand Down Expand Up @@ -80,3 +194,17 @@ func IBCPacketFixture(mutators ...func(p *channeltypes.Packet)) channeltypes.Pac
}
return r
}

var _ types.IBCContractKeeper = &IBCContractKeeperMock{}

type IBCContractKeeperMock struct {
types.IBCContractKeeper
OnRecvPacketFn func(ctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.IBCPacketReceiveMsg) (ibcexported.Acknowledgement, error)
}

func (m IBCContractKeeperMock) OnRecvPacket(ctx sdk.Context, contractAddr sdk.AccAddress, msg wasmvmtypes.IBCPacketReceiveMsg) (ibcexported.Acknowledgement, error) {
if m.OnRecvPacketFn == nil {
panic("not expected to be called")
}
return m.OnRecvPacketFn(ctx, contractAddr, msg)
}
110 changes: 69 additions & 41 deletions x/wasm/keeper/relay_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func TestOnRecvPacket(t *testing.T) {

specs := map[string]struct {
contractAddr sdk.AccAddress
contractResp *wasmvmtypes.IBCReceiveResponse
contractResp *wasmvmtypes.IBCReceiveResult
contractErr error
overwriteMessenger *wasmtesting.MockMessageHandler
mockReplyFn func(codeID wasmvm.Checksum, env wasmvmtypes.Env, reply wasmvmtypes.Reply, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error)
Expand All @@ -329,73 +329,93 @@ func TestOnRecvPacket(t *testing.T) {
expPanic bool
expEventTypes []string
}{
"consume contract gas": {
"contract returns success ack": {
contractAddr: example.Contract,
expContractGas: myContractGas,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{Acknowledgement: []byte("myAck")},
},
expAck: []byte("myAck"),
},
"can return empty ack": {
"can return empty ack data": {
contractAddr: example.Contract,
expContractGas: myContractGas,
contractResp: &wasmvmtypes.IBCReceiveResponse{},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{},
},
},
"consume gas on error, ignore events + messages": {
"contract Err result converted to error Ack": {
contractAddr: example.Contract,
expContractGas: myContractGas,
contractResp: &wasmvmtypes.IBCReceiveResult{
Err: "my-error",
},
expAck: []byte(`{"error":"my-error"}`), // without error msg redaction
},
"contract aborts tx with error": {
contractAddr: example.Contract,
expContractGas: myContractGas,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}},
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}},
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
},
},
contractErr: errors.New("test, ignore"),
expPanic: true,
},
"dispatch contract messages on success": {
contractAddr: example.Contract,
expContractGas: myContractGas,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}, {ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Custom: json.RawMessage(`{"foo":"bar"}`)}}},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}, {ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Custom: json.RawMessage(`{"foo":"bar"}`)}}},
},
},
expAck: []byte("myAck"),
},
"emit contract attributes on success": {
contractAddr: example.Contract,
expContractGas: myContractGas + 10,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
},
},
expEventTypes: []string{types.WasmModuleEventType},
expAck: []byte("myAck"),
},
"emit contract events on success": {
contractAddr: example.Contract,
expContractGas: myContractGas + 46, // charge or custom event as well
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
Events: []wasmvmtypes.Event{{
Type: "custom",
Attributes: []wasmvmtypes.EventAttribute{{
Key: "message",
Value: "to rudi",
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
Events: []wasmvmtypes.Event{{
Type: "custom",
Attributes: []wasmvmtypes.EventAttribute{{
Key: "message",
Value: "to rudi",
}},
}},
}},
},
},
expEventTypes: []string{types.WasmModuleEventType, "wasm-custom"},
expAck: []byte("myAck"),
},
"messenger errors returned, events stored": {
"messenger errors returned": {
contractAddr: example.Contract,
expContractGas: myContractGas + 10,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}, {ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Custom: json.RawMessage(`{"foo":"bar"}`)}}},
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}, {ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{Custom: json.RawMessage(`{"foo":"bar"}`)}}},
Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}},
},
},
overwriteMessenger: wasmtesting.NewErroringMessageHandler(),
expErr: true,
Expand All @@ -404,9 +424,11 @@ func TestOnRecvPacket(t *testing.T) {
"submessage reply can overwrite ack data": {
contractAddr: example.Contract,
expContractGas: myContractGas + storageCosts,
contractResp: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyAlways, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}},
contractResp: &wasmvmtypes.IBCReceiveResult{
Ok: &wasmvmtypes.IBCReceiveResponse{
Acknowledgement: []byte("myAck"),
Messages: []wasmvmtypes.SubMsg{{ReplyOn: wasmvmtypes.ReplyAlways, Msg: wasmvmtypes.CosmosMsg{Bank: &wasmvmtypes.BankMsg{}}}},
},
},
mockReplyFn: func(codeID wasmvm.Checksum, env wasmvmtypes.Env, reply wasmvmtypes.Reply, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) {
return &wasmvmtypes.Response{Data: []byte("myBetterAck")}, 0, nil
Expand All @@ -425,7 +447,7 @@ func TestOnRecvPacket(t *testing.T) {

m.IBCPacketReceiveFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, msg wasmvmtypes.IBCPacketReceiveMsg, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.IBCReceiveResult, uint64, error) {
assert.Equal(t, myPacket, msg.Packet)
return &wasmvmtypes.IBCReceiveResult{Ok: spec.contractResp}, myContractGas * DefaultGasMultiplier, spec.contractErr
return spec.contractResp, myContractGas * DefaultGasMultiplier, spec.contractErr
}
if spec.mockReplyFn != nil {
m.ReplyFn = spec.mockReplyFn
Expand Down Expand Up @@ -462,17 +484,23 @@ func TestOnRecvPacket(t *testing.T) {
return
}
require.NoError(t, err)
require.Equal(t, spec.expAck, gotAck)
require.Equal(t, spec.expAck, gotAck.Acknowledgement())

// verify gas consumed
const storageCosts = sdk.Gas(2903)
assert.Equal(t, spec.expContractGas, ctx.GasMeter().GasConsumed()-before-storageCosts)
// verify msgs dispatched
require.Len(t, *capturedMsgs, len(spec.contractResp.Messages))
for i, m := range spec.contractResp.Messages {
assert.Equal(t, (*capturedMsgs)[i], m.Msg)

// verify msgs dispatched on success/ err response
if spec.contractResp.Err != "" {
assert.Empty(t, capturedMsgs) // no messages captured on err response
assert.Equal(t, spec.expEventTypes, stripTypes(ctx.EventManager().Events()))
} else {
require.Len(t, *capturedMsgs, len(spec.contractResp.Ok.Messages))
for i, m := range spec.contractResp.Ok.Messages {
assert.Equal(t, (*capturedMsgs)[i], m.Msg)
}
assert.Equal(t, spec.expEventTypes, stripTypes(ctx.EventManager().Events()))
}
assert.Equal(t, spec.expEventTypes, stripTypes(ctx.EventManager().Events()))
})
}
}
Expand Down
Loading

0 comments on commit b561461

Please sign in to comment.