diff --git a/x/wasm/ibc.go b/x/wasm/ibc.go index 977f2a0611..164230a68a 100644 --- a/x/wasm/ibc.go +++ b/x/wasm/ibc.go @@ -273,16 +273,13 @@ func (i IBCHandler) OnRecvPacket( ack, err := i.keeper.OnRecvPacket(ctx.WithEventManager(em), contractAddr, msg) if err != nil { 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() { + } else 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) + types.EmitAcknowledgementEvent(ctx, contractAddr, ack, err) return ack } diff --git a/x/wasm/ibc_integration_test.go b/x/wasm/ibc_integration_test.go index d92b8467b3..6a2320f3a1 100644 --- a/x/wasm/ibc_integration_test.go +++ b/x/wasm/ibc_integration_test.go @@ -3,6 +3,10 @@ package wasm_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/CosmWasm/wasmd/x/wasm/types" + wasmvm "github.com/CosmWasm/wasmvm" wasmvmtypes "github.com/CosmWasm/wasmvm/types" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" @@ -124,3 +128,141 @@ func TestOnChanOpenTryVersion(t *testing.T) { }) } } + +func TestOnIBCPacketReceive(t *testing.T) { + // given 2 chains with a mock on chain A to control the IBC flow + // and the ibc-reflect contract on chain B + // when the test package is relayed + // then the contract executes the flow defined for the packet data + // and the ibc Ack captured is what we expect + specs := map[string]struct { + packetData []byte + expAck []byte + expPacketNotHandled bool + }{ + "all good": { + packetData: []byte(`{"who_am_i":{}}`), + expAck: []byte(`{"ok":{"account":"cosmos1suhgf5svhu4usrurvxzlgn54ksxmn8gljarjtxqnapv8kjnp4nrs2zhgh2"}}`), + }, + "with result err": { + packetData: []byte(`{"return_err": {"text": "my error"}}`), + expAck: []byte(`{"error":"invalid packet: Generic error: my error"}`), + }, + "with returned msg fails": { + packetData: []byte(`{"return_msgs": {"msgs": [{"bank":{"send":{"to_address": "invalid-address", "amount": [{"denom": "ALX", "amount": "1"}]}}}]}}`), + expAck: []byte(`{"error":"ABCI code: 7: error handling packet: see events for details"}`), + }, + "with contract panic": { + packetData: []byte(`{"panic":{}}`), + expPacketNotHandled: true, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + mockContractEngine := NewCaptureAckTestContractEngine() + chainAOpts := []wasmkeeper.Option{ + wasmkeeper.WithWasmEngine(mockContractEngine), + } + var ( + coord = wasmibctesting.NewCoordinator(t, 2, chainAOpts) + chainA = coord.GetChain(wasmibctesting.GetChainID(0)) + chainB = coord.GetChain(wasmibctesting.GetChainID(1)) + ) + // setup chain A contract metadata for mock + myMockContractAddr := chainA.SeedNewContractInstance() // setups env but uses mock contract + + // setup chain B contracts + reflectID := chainB.StoreCodeFile("./keeper/testdata/reflect.wasm").CodeID + initMsg := wasmkeeper.IBCReflectInitMsg{ReflectCodeID: reflectID}.GetBytes(t) + codeID := chainB.StoreCodeFile("./keeper/testdata/ibc_reflect.wasm").CodeID + ibcReflectContractAddr := chainB.InstantiateContract(codeID, initMsg) + + // establish IBC channels + var ( + sourcePortID = chainA.ContractInfo(myMockContractAddr).IBCPortID + counterpartPortID = chainB.ContractInfo(ibcReflectContractAddr).IBCPortID + path = wasmibctesting.NewPath(chainA, chainB) + ) + path.EndpointA.ChannelConfig = &ibctesting.ChannelConfig{ + PortID: sourcePortID, Version: "ibc-reflect-v1", Order: channeltypes.ORDERED, + } + path.EndpointB.ChannelConfig = &ibctesting.ChannelConfig{ + PortID: counterpartPortID, Version: "ibc-reflect-v1", Order: channeltypes.ORDERED, + } + + coord.SetupConnections(path) + coord.CreateChannels(path) + coord.CommitBlock(chainA, chainB) + require.Equal(t, 0, len(chainA.PendingSendPackets)) + require.Equal(t, 0, len(chainB.PendingSendPackets)) + + // when an ibc packet is sent from chain A to chain B + capturedAck := mockContractEngine.SubmitIBCPacket(t, path, chainA, myMockContractAddr, spec.packetData) + coord.CommitBlock(chainA, chainB) + + require.Equal(t, 1, len(chainA.PendingSendPackets)) + require.Equal(t, 0, len(chainB.PendingSendPackets)) + + err := coord.RelayAndAckPendingPackets(path) + + // then + if spec.expPacketNotHandled { + const contractPanicToErrMsg = `recovered: Error calling the VM: Error executing Wasm: Wasmer runtime error: RuntimeError: Aborted: panicked at 'This page intentionally faulted', src/contract.rs:316:5` + assert.ErrorContains(t, err, contractPanicToErrMsg) + require.Nil(t, *capturedAck) + return + } + require.NoError(t, err) + if spec.expAck != nil { + assert.Equal(t, spec.expAck, *capturedAck, string(*capturedAck)) + } else { + require.Nil(t, *capturedAck) + } + }) + } +} + +// mock to submit an ibc data package from given chain and capture the ack +type captureAckTestContractEngine struct { + *wasmtesting.MockWasmer +} + +// NewCaptureAckTestContractEngine constructor +func NewCaptureAckTestContractEngine() *captureAckTestContractEngine { + m := wasmtesting.NewIBCContractMockWasmer(&wasmtesting.MockIBCContractCallbacks{ + IBCChannelOpenFn: func(codeID wasmvm.Checksum, env wasmvmtypes.Env, msg wasmvmtypes.IBCChannelOpenMsg, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.IBC3ChannelOpenResponse, uint64, error) { + return &wasmvmtypes.IBC3ChannelOpenResponse{}, 0, nil + }, + IBCChannelConnectFn: func(codeID wasmvm.Checksum, env wasmvmtypes.Env, msg wasmvmtypes.IBCChannelConnectMsg, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.IBCBasicResponse, uint64, error) { + return &wasmvmtypes.IBCBasicResponse{}, 0, nil + }, + }) + return &captureAckTestContractEngine{m} +} + +// SubmitIBCPacket starts an IBC packet transfer on given chain and captures the ack returned +func (x *captureAckTestContractEngine) SubmitIBCPacket(t *testing.T, path *wasmibctesting.Path, chainA *wasmibctesting.TestChain, senderContractAddr sdk.AccAddress, packetData []byte) *[]byte { + // prepare a bridge to send an ibc packet by an ordinary wasm execute message + x.MockWasmer.ExecuteFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, info wasmvmtypes.MessageInfo, executeMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { + return &wasmvmtypes.Response{ + Messages: []wasmvmtypes.SubMsg{{ID: 1, ReplyOn: wasmvmtypes.ReplyNever, Msg: wasmvmtypes.CosmosMsg{IBC: &wasmvmtypes.IBCMsg{SendPacket: &wasmvmtypes.SendPacketMsg{ + ChannelID: path.EndpointA.ChannelID, Data: executeMsg, Timeout: wasmvmtypes.IBCTimeout{Block: &wasmvmtypes.IBCTimeoutBlock{Revision: 1, Height: 10000000}}, + }}}}}, + }, 0, nil + } + // capture acknowledgement + var gotAck []byte + x.MockWasmer.IBCPacketAckFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, msg wasmvmtypes.IBCPacketAckMsg, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.IBCBasicResponse, uint64, error) { + gotAck = msg.Acknowledgement.Data + return &wasmvmtypes.IBCBasicResponse{}, 0, nil + } + + // start the process + _, err := chainA.SendMsgs(&types.MsgExecuteContract{ + Sender: chainA.SenderAccount.GetAddress().String(), + Contract: senderContractAddr.String(), + Msg: packetData, + }) + require.NoError(t, err) + return &gotAck +} diff --git a/x/wasm/ibc_test.go b/x/wasm/ibc_test.go index 88cdd34929..9728967516 100644 --- a/x/wasm/ibc_test.go +++ b/x/wasm/ibc_test.go @@ -25,12 +25,12 @@ func TestOnRecvPacket(t *testing.T) { }) myCustomEvent := sdk.NewEvent("testing") specs := map[string]struct { - ibcPkg channeltypes.Packet - contractRsp ibcexported.Acknowledgement - contractErr error - expEvents sdk.Events - expPanic bool - expAck ibcexported.Acknowledgement + ibcPkg channeltypes.Packet + contractRsp ibcexported.Acknowledgement + contractOkMsgExecErr error + expEvents sdk.Events + expPanic bool + expAck ibcexported.Acknowledgement }{ "contract returns success response": { ibcPkg: anyContractIBCPkg, @@ -83,10 +83,10 @@ func TestOnRecvPacket(t *testing.T) { }), expPanic: true, }, - "contract executed with error": { - ibcPkg: anyContractIBCPkg, - contractErr: types.ErrInvalid.Wrap("testing"), - expAck: channeltypes.NewErrorAcknowledgement(types.ErrInvalid.Wrap("testing")), + "returned messages executed with error": { + ibcPkg: anyContractIBCPkg, + contractOkMsgExecErr: types.ErrInvalid.Wrap("testing"), + expAck: channeltypes.NewErrorAcknowledgement(types.ErrInvalid.Wrap("testing")), expEvents: sdk.Events{{ Type: "ibc_packet_received", Attributes: []abci.EventAttribute{ @@ -104,7 +104,7 @@ func TestOnRecvPacket(t *testing.T) { 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 + return spec.contractRsp, spec.contractOkMsgExecErr }, } h := NewIBCHandler(mock, nil, nil) diff --git a/x/wasm/keeper/relay.go b/x/wasm/keeper/relay.go index af9989c683..4aaeee9571 100644 --- a/x/wasm/keeper/relay.go +++ b/x/wasm/keeper/relay.go @@ -133,7 +133,10 @@ func (k Keeper) OnRecvPacket( res, gasUsed, execErr := k.wasmVM.IBCPacketReceive(codeInfo.CodeHash, env, msg, prefixStore, cosmwasmAPI, querier, ctx.GasMeter(), gas, costJSONDeserialization) k.consumeRuntimeGas(ctx, gasUsed) if execErr != nil { - panic(execErr) // let contract fully abort IBC receive in certain case + panic(execErr) // let the contract fully abort an IBC packet receive. + // Throwing a panic here instead of an error ack will revert + // all state downstream and not persist any data in ibc-go. + // This can be triggered by throwing a panic in the contract } if res.Err != "" { // return error ACK with non-redacted contract message, state will be reverted diff --git a/x/wasm/keeper/relay_test.go b/x/wasm/keeper/relay_test.go index abb8e4b4c5..6b571594b9 100644 --- a/x/wasm/keeper/relay_test.go +++ b/x/wasm/keeper/relay_test.go @@ -355,15 +355,8 @@ func TestOnRecvPacket(t *testing.T) { "contract aborts tx with error": { contractAddr: example.Contract, expContractGas: myContractGas, - 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, + contractErr: errors.New("test, ignore"), + expPanic: true, }, "dispatch contract messages on success": { contractAddr: example.Contract, diff --git a/x/wasm/keeper/testdata/ibc_reflect.wasm b/x/wasm/keeper/testdata/ibc_reflect.wasm index ec737104c4..c51a0ba473 100644 Binary files a/x/wasm/keeper/testdata/ibc_reflect.wasm and b/x/wasm/keeper/testdata/ibc_reflect.wasm differ