From ec264aea10fe5d7a629f68f3763c83c926d519e4 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 4 Apr 2023 15:23:53 +0200 Subject: [PATCH] Update OnRecvPacket method to panic when an error is returned by the VM (backport #1298) (#1303) * Update OnRecvPacket method to panic when an error is returned (#1298) by the VM (cherry picked from commit 5edfd6c74d5bf01cf055f938e4bb5eba1fdf6367) # Conflicts: # x/wasm/keeper/relay.go # x/wasm/relay_test.go * fix conflicts --------- Co-authored-by: pinosu <95283998+pinosu@users.noreply.github.com> Co-authored-by: Pino' Surace --- x/wasm/keeper/relay.go | 2 +- x/wasm/keeper/relay_test.go | 10 ++++++++- x/wasm/relay_test.go | 41 +++++++++++++++++++++++++------------ 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/x/wasm/keeper/relay.go b/x/wasm/keeper/relay.go index 7c3f0b902c..f8450e3461 100644 --- a/x/wasm/keeper/relay.go +++ b/x/wasm/keeper/relay.go @@ -130,7 +130,7 @@ 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 { - return nil, sdkerrors.Wrap(types.ErrExecuteFailed, execErr.Error()) + panic(execErr) } if res.Err != "" { // handle error case as before https://github.com/CosmWasm/wasmvm/commit/c300106fe5c9426a495f8e10821e00a9330c56c6 return nil, sdkerrors.Wrap(types.ErrExecuteFailed, res.Err) diff --git a/x/wasm/keeper/relay_test.go b/x/wasm/keeper/relay_test.go index a1d2819e50..9fb377a09c 100644 --- a/x/wasm/keeper/relay_test.go +++ b/x/wasm/keeper/relay_test.go @@ -174,6 +174,7 @@ func TestOnConnectChannel(t *testing.T) { Channel: myChannel, }, } + err := keepers.WasmKeeper.OnConnectChannel(ctx, spec.contractAddr, msg) // then @@ -325,6 +326,7 @@ func TestOnRecvPacket(t *testing.T) { expContractGas sdk.Gas expAck []byte expErr bool + expPanic bool expEventTypes []string }{ "consume contract gas": { @@ -349,7 +351,7 @@ func TestOnRecvPacket(t *testing.T) { Attributes: []wasmvmtypes.EventAttribute{{Key: "Foo", Value: "Bar"}}, }, contractErr: errors.New("test, ignore"), - expErr: true, + expPanic: true, }, "dispatch contract messages on success": { contractAddr: example.Contract, @@ -444,6 +446,12 @@ func TestOnRecvPacket(t *testing.T) { // when msg := wasmvmtypes.IBCPacketReceiveMsg{Packet: myPacket} + if spec.expPanic { + assert.Panics(t, func() { + keepers.WasmKeeper.OnRecvPacket(ctx, spec.contractAddr, msg) + }) + return + } gotAck, err := keepers.WasmKeeper.OnRecvPacket(ctx, spec.contractAddr, msg) // then diff --git a/x/wasm/relay_test.go b/x/wasm/relay_test.go index df34534fbc..19be8b968e 100644 --- a/x/wasm/relay_test.go +++ b/x/wasm/relay_test.go @@ -31,10 +31,13 @@ func TestFromIBCTransferToContract(t *testing.T) { transferAmount := sdk.NewInt(1) specs := map[string]struct { - contract wasmtesting.IBCContractCallbacks - setupContract func(t *testing.T, contract wasmtesting.IBCContractCallbacks, chain *wasmibctesting.TestChain) - expChainABalanceDiff sdk.Int - expChainBBalanceDiff sdk.Int + contract wasmtesting.IBCContractCallbacks + setupContract func(t *testing.T, contract wasmtesting.IBCContractCallbacks, chain *wasmibctesting.TestChain) + expChainAPendingSendPackets int + expChainBPendingSendPackets int + expChainABalanceDiff sdk.Int + expChainBBalanceDiff sdk.Int + expErr bool }{ "ack": { contract: &ackReceiverContract{}, @@ -43,8 +46,10 @@ func TestFromIBCTransferToContract(t *testing.T) { c.t = t c.chain = chain }, - expChainABalanceDiff: transferAmount.Neg(), - expChainBBalanceDiff: transferAmount, + expChainAPendingSendPackets: 0, + expChainBPendingSendPackets: 0, + expChainABalanceDiff: transferAmount.Neg(), + expChainBBalanceDiff: transferAmount, }, "nack": { contract: &nackReceiverContract{}, @@ -52,8 +57,10 @@ func TestFromIBCTransferToContract(t *testing.T) { c := contract.(*nackReceiverContract) c.t = t }, - expChainABalanceDiff: sdk.ZeroInt(), - expChainBBalanceDiff: sdk.ZeroInt(), + expChainAPendingSendPackets: 0, + expChainBPendingSendPackets: 0, + expChainABalanceDiff: sdk.ZeroInt(), + expChainBBalanceDiff: sdk.ZeroInt(), }, "error": { contract: &errorReceiverContract{}, @@ -61,8 +68,11 @@ func TestFromIBCTransferToContract(t *testing.T) { c := contract.(*errorReceiverContract) c.t = t }, - expChainABalanceDiff: sdk.ZeroInt(), - expChainBBalanceDiff: sdk.ZeroInt(), + expChainAPendingSendPackets: 1, + expChainBPendingSendPackets: 0, + expChainABalanceDiff: transferAmount.Neg(), + expChainBBalanceDiff: sdk.ZeroInt(), + expErr: true, }, } for name, spec := range specs { @@ -100,6 +110,7 @@ func TestFromIBCTransferToContract(t *testing.T) { // when transfer via sdk transfer from A (module) -> B (contract) coinToSendToB := sdk.NewCoin(sdk.DefaultBondDenom, transferAmount) timeoutHeight := clienttypes.NewHeight(1, 110) + msg := ibctransfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coinToSendToB, chainA.SenderAccount.GetAddress().String(), chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0) _, err := chainA.SendMsgs(msg) require.NoError(t, err) @@ -111,11 +122,15 @@ func TestFromIBCTransferToContract(t *testing.T) { // and when relay to chain B and handle Ack on chain A err = coordinator.RelayAndAckPendingPackets(path) - require.NoError(t, err) + if spec.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } // then - require.Equal(t, 0, len(chainA.PendingSendPackets)) - require.Equal(t, 0, len(chainB.PendingSendPackets)) + require.Equal(t, spec.expChainAPendingSendPackets, len(chainA.PendingSendPackets)) + require.Equal(t, spec.expChainBPendingSendPackets, len(chainB.PendingSendPackets)) // and source chain balance was decreased newChainABalance := chainA.Balance(chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)