Update OnRecvPacket method to panic when an error is returned (#1298)

by the VM
CosmWasm · Mar 29, 2023 · 5edfd6c · 5edfd6c
1 parent f438834
commit 5edfd6c
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 15 deletions.
diff --git 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, errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
+		panic(execErr)
 	}
 	if res.Err != "" { // handle error case as before https://github.com/CosmWasm/wasmvm/commit/c300106fe5c9426a495f8e10821e00a9330c56c6
 		return nil, errorsmod.Wrap(types.ErrExecuteFailed, res.Err)

diff --git 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
@@ -32,10 +32,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 math.Int
-		expChainBBalanceDiff math.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
 	}{
 		"ack": {
 			contract: &ackReceiverContract{},
@@ -44,26 +47,33 @@ 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{},
 			setupContract: func(t *testing.T, contract wasmtesting.IBCContractCallbacks, chain *wasmibctesting.TestChain) {
 				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{},
 			setupContract: func(t *testing.T, contract wasmtesting.IBCContractCallbacks, chain *wasmibctesting.TestChain) {
 				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 {
@@ -101,6 +111,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)
@@ -112,11 +123,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)