From 1a1c7ac7b34c9263d0c60f3b96986b93a62a332f Mon Sep 17 00:00:00 2001 From: pinosu <95283998+pinosu@users.noreply.github.com> Date: Wed, 29 Mar 2023 13:16:02 +0200 Subject: [PATCH] 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 --- x/wasm/keeper/relay.go | 4 ++++ x/wasm/keeper/relay_test.go | 10 ++++++++- x/wasm/relay_test.go | 44 +++++++++++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/x/wasm/keeper/relay.go b/x/wasm/keeper/relay.go index 7c3f0b902c..be743c0e05 100644 --- a/x/wasm/keeper/relay.go +++ b/x/wasm/keeper/relay.go @@ -130,7 +130,11 @@ 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 { +<<<<<<< HEAD return nil, sdkerrors.Wrap(types.ErrExecuteFailed, execErr.Error()) +======= + panic(execErr) +>>>>>>> 5edfd6c (Update OnRecvPacket method to panic when an error is returned (#1298)) } 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..b3ecf36eaf 100644 --- a/x/wasm/relay_test.go +++ b/x/wasm/relay_test.go @@ -31,10 +31,20 @@ func TestFromIBCTransferToContract(t *testing.T) { transferAmount := sdk.NewInt(1) specs := map[string]struct { +<<<<<<< HEAD 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 math.Int + expChainBBalanceDiff math.Int + expErr bool +>>>>>>> 5edfd6c (Update OnRecvPacket method to panic when an error is returned (#1298)) }{ "ack": { contract: &ackReceiverContract{}, @@ -43,8 +53,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 +64,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 +75,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,7 +117,12 @@ 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) +<<<<<<< HEAD msg := ibctransfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coinToSendToB, chainA.SenderAccount.GetAddress().String(), chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0) +======= + + msg := ibctransfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coinToSendToB, chainA.SenderAccount.GetAddress().String(), chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") +>>>>>>> 5edfd6c (Update OnRecvPacket method to panic when an error is returned (#1298)) _, err := chainA.SendMsgs(msg) require.NoError(t, err) require.NoError(t, path.EndpointB.UpdateClient()) @@ -111,11 +133,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)