Skip to content

Commit

Permalink
Apply review feedback (backport #1376) (#1384)
Browse files Browse the repository at this point in the history 
* Apply review feedback (#1376)

* Apply review feedback

* Better test name

* Add contract integration test

* Better doc in test and minor refactoring

(cherry picked from commit 29481db)

* Set correct test chain id

---------

Co-authored-by: Alexander Peters <[email protected]>
  • Loading branch information
@mergify @alpe
mergify[bot] and alpe authored May 10, 2023
1 parent d698df0 commit 6269668
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 26 deletions.
7 changes: 2 additions & 5 deletions x/wasm/ibc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
142 changes: 142 additions & 0 deletions x/wasm/ibc_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(1))
chainB = coord.GetChain(wasmibctesting.GetChainID(2))
)
// 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
}
22 changes: 11 additions & 11 deletions x/wasm/ibc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion x/wasm/keeper/relay.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 2 additions & 9 deletions x/wasm/keeper/relay_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Binary file modified x/wasm/keeper/testdata/ibc_reflect.wasm
View file
Binary file not shown.

0 comments on commit 6269668

Please sign in to comment.