Apply review feedback

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
 }
 

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")),
+		"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)

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

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,