Merge pull request #2267 from CosmWasm/co/ibc-callbacks-funds

Add `Transfer` special handling to IBC destination callback if it's a transfer message

Author: chipshort

app/app.go

@@ -622,7 +622,7 @@ func NewWasmApp(
 	)
 
 	// Create fee enabled wasm ibc Stack
-	wasmStackIBCHandler := wasm.NewIBCHandler(app.WasmKeeper, app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper)
+	wasmStackIBCHandler := wasm.NewIBCHandler(app.WasmKeeper, app.IBCKeeper.ChannelKeeper, app.TransferKeeper, app.IBCKeeper.ChannelKeeper)
 
 	// Create Interchain Accounts Stack
 	// SendPacket, since it is originating from the application to core IBC:

tests/e2e/ibc_callbacks_test.go

@@ -22,6 +22,25 @@ import (
 	"github.com/CosmWasm/wasmd/x/wasm/types"
 )
 
+type transferExecMsg struct {
+	ToAddress      string `json:"to_address"`
+	ChannelID      string `json:"channel_id"`
+	TimeoutSeconds uint32 `json:"timeout_seconds"`
+}
+
+// executeMsg is the ibc-callbacks contract's execute msg
+type executeMsg struct {
+	Transfer *transferExecMsg `json:"transfer"`
+}
+type queryMsg struct {
+	CallbackStats struct{} `json:"callback_stats"`
+}
+type queryResp struct {
+	IBCAckCallbacks         []wasmvmtypes.IBCPacketAckMsg           `json:"ibc_ack_callbacks"`
+	IBCTimeoutCallbacks     []wasmvmtypes.IBCPacketTimeoutMsg       `json:"ibc_timeout_callbacks"`
+	IBCDestinationCallbacks []wasmvmtypes.IBCDestinationCallbackMsg `json:"ibc_destination_callbacks"`
+}
+
 func TestIBCCallbacks(t *testing.T) {
 	// scenario:
 	// given two chains
@@ -35,7 +54,6 @@ func TestIBCCallbacks(t *testing.T) {
 	chainB := wasmibctesting.NewWasmTestChain(coord.GetChain(ibctesting.GetChainID(2)))
 
 	actorChainA := sdk.AccAddress(chainA.SenderPrivKey.PubKey().Address())
-	oneToken := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1)))
 
 	path := wasmibctesting.NewWasmPath(chainA, chainB)
 	path.EndpointA.ChannelConfig = &ibctesting.ChannelConfig{
@@ -51,47 +69,36 @@ func TestIBCCallbacks(t *testing.T) {
 	// with an ics-20 transfer channel setup between both chains
 	coord.Setup(&path.Path)
 
+	oneToken := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1)))
+	ibcDenom := ibctransfertypes.NewDenom(
+		sdk.DefaultBondDenom,
+		ibctransfertypes.NewHop(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID),
+	).IBCDenom()
+	ibcCoin := sdk.NewCoin(ibcDenom, sdkmath.NewInt(1))
+
 	// with an ibc-callbacks contract deployed on chain A
 	codeIDonA := chainA.StoreCodeFile("./testdata/ibc_callbacks.wasm").CodeID
 
 	// and on chain B
 	codeIDonB := chainB.StoreCodeFile("./testdata/ibc_callbacks.wasm").CodeID
 
-	type TransferExecMsg struct {
-		ToAddress      string `json:"to_address"`
-		ChannelID      string `json:"channel_id"`
-		TimeoutSeconds uint32 `json:"timeout_seconds"`
-	}
-	// ExecuteMsg is the ibc-callbacks contract's execute msg
-	type ExecuteMsg struct {
-		Transfer *TransferExecMsg `json:"transfer"`
-	}
-	type QueryMsg struct {
-		CallbackStats struct{} `json:"callback_stats"`
-	}
-	type QueryResp struct {
-		IBCAckCallbacks         []wasmvmtypes.IBCPacketAckMsg           `json:"ibc_ack_callbacks"`
-		IBCTimeoutCallbacks     []wasmvmtypes.IBCPacketTimeoutMsg       `json:"ibc_timeout_callbacks"`
-		IBCDestinationCallbacks []wasmvmtypes.IBCDestinationCallbackMsg `json:"ibc_destination_callbacks"`
-	}
-
 	specs := map[string]struct {
-		contractMsg ExecuteMsg
+		contractMsg executeMsg
 		// expAck is true if the packet is relayed, false if it times out
 		expAck bool
 	}{
 		"success": {
-			contractMsg: ExecuteMsg{
-				Transfer: &TransferExecMsg{
+			contractMsg: executeMsg{
+				Transfer: &transferExecMsg{
 					ChannelID:      path.EndpointA.ChannelID,
 					TimeoutSeconds: 100,
 				},
 			},
 			expAck: true,
 		},
 		"timeout": {
-			contractMsg: ExecuteMsg{
-				Transfer: &TransferExecMsg{
+			contractMsg: executeMsg{
+				Transfer: &transferExecMsg{
 					ChannelID:      path.EndpointA.ChannelID,
 					TimeoutSeconds: 1,
 				},
@@ -128,17 +135,29 @@ func TestIBCCallbacks(t *testing.T) {
 				wasmibctesting.RelayAndAckPendingPackets(path)
 
 				// then the contract on chain B should receive a receive callback
-				var response QueryResp
-				chainB.SmartQuery(contractAddrB.String(), QueryMsg{CallbackStats: struct{}{}}, &response)
+				var response queryResp
+				chainB.SmartQuery(contractAddrB.String(), queryMsg{CallbackStats: struct{}{}}, &response)
 				assert.Empty(t, response.IBCAckCallbacks)
 				assert.Empty(t, response.IBCTimeoutCallbacks)
 				assert.Len(t, response.IBCDestinationCallbacks, 1)
 
 				// and the receive callback should contain the ack
 				assert.Equal(t, []byte("{\"result\":\"AQ==\"}"), response.IBCDestinationCallbacks[0].Ack.Data)
+				assert.Equal(t,
+					wasmvmtypes.Array[wasmvmtypes.Coin]{wasmvmtypes.NewCoin(1, ibcDenom)},
+					response.IBCDestinationCallbacks[0].Transfer.Funds,
+				)
+				assert.Equal(t, contractAddrA.String(), response.IBCDestinationCallbacks[0].Transfer.Receiver)
+
+				balances := chainB.GetWasmApp().BankKeeper.GetAllBalances(chainB.GetContext(), contractAddrB)
+				// sanity check that the balance of the contract is correct
+				require.Equal(t, sdk.NewCoins(
+					sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)),
+					ibcCoin,
+				), balances)
 
 				// and the contract on chain A should receive a callback with the ack
-				chainA.SmartQuery(contractAddrA.String(), QueryMsg{CallbackStats: struct{}{}}, &response)
+				chainA.SmartQuery(contractAddrA.String(), queryMsg{CallbackStats: struct{}{}}, &response)
 				assert.Len(t, response.IBCAckCallbacks, 1)
 				assert.Empty(t, response.IBCTimeoutCallbacks)
 				assert.Empty(t, response.IBCDestinationCallbacks)
@@ -150,14 +169,14 @@ func TestIBCCallbacks(t *testing.T) {
 				require.NoError(t, wasmibctesting.TimeoutPendingPackets(coord, path))
 
 				// then the contract on chain B should not receive anything
-				var response QueryResp
-				chainB.SmartQuery(contractAddrB.String(), QueryMsg{CallbackStats: struct{}{}}, &response)
+				var response queryResp
+				chainB.SmartQuery(contractAddrB.String(), queryMsg{CallbackStats: struct{}{}}, &response)
 				assert.Empty(t, response.IBCAckCallbacks)
 				assert.Empty(t, response.IBCTimeoutCallbacks)
 				assert.Empty(t, response.IBCDestinationCallbacks)
 
 				// and the contract on chain A should receive a callback with the timeout result
-				chainA.SmartQuery(contractAddrA.String(), QueryMsg{CallbackStats: struct{}{}}, &response)
+				chainA.SmartQuery(contractAddrA.String(), queryMsg{CallbackStats: struct{}{}}, &response)
 				assert.Empty(t, response.IBCAckCallbacks)
 				assert.Len(t, response.IBCTimeoutCallbacks, 1)
 				assert.Empty(t, response.IBCDestinationCallbacks)
@@ -166,6 +185,80 @@ func TestIBCCallbacks(t *testing.T) {
 	}
 }
 
+func TestIBCDestinationCallbackTransfer(t *testing.T) {
+	// scenario:
+	// given two chains
+	//   with an ics-20 channel established
+	//   and an ibc-callbacks contract deployed on chain A
+	// when someone sends an ibc transfer to chain B and back to the contract on A
+	// then the contract on A should receive a destination chain callback with correct transfer info
+
+	coord := wasmibctesting.NewCoordinator(t, 2)
+	chainA := wasmibctesting.NewWasmTestChain(coord.GetChain(ibctesting.GetChainID(1)))
+	chainB := wasmibctesting.NewWasmTestChain(coord.GetChain(ibctesting.GetChainID(2)))
+
+	actorChainA := sdk.AccAddress(chainA.SenderPrivKey.PubKey().Address())
+	actorChainB := sdk.AccAddress(chainB.SenderPrivKey.PubKey().Address())
+
+	path := wasmibctesting.NewWasmPath(chainA, chainB)
+	path.EndpointA.ChannelConfig = &ibctesting.ChannelConfig{
+		PortID:  ibctransfertypes.PortID,
+		Version: ibctransfertypes.V1,
+		Order:   channeltypes.UNORDERED,
+	}
+	path.EndpointB.ChannelConfig = &ibctesting.ChannelConfig{
+		PortID:  ibctransfertypes.PortID,
+		Version: ibctransfertypes.V1,
+		Order:   channeltypes.UNORDERED,
+	}
+	// with an ics-20 transfer channel setup between both chains
+	coord.Setup(&path.Path)
+
+	oneToken := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1)))
+	ibcDenom := ibctransfertypes.NewDenom(
+		sdk.DefaultBondDenom,
+		ibctransfertypes.NewHop(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID),
+	).IBCDenom()
+	ibcCoin := sdk.NewCoin(ibcDenom, sdkmath.NewInt(1))
+
+	// with an ibc-callbacks contract deployed on chain A
+	codeID := chainA.StoreCodeFile("./testdata/ibc_callbacks.wasm").CodeID
+
+	contractAddr := chainA.InstantiateContract(codeID, []byte(`{}`))
+	require.NotEmpty(t, contractAddr)
+
+	// when someone sends an ibc transfer to chain B
+	chainA.SendMsgs(
+		ibctransfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
+			oneToken[0], actorChainA.String(), actorChainB.String(), chainA.GetTimeoutHeight(), 0, ""),
+	)
+
+	wasmibctesting.RelayAndAckPendingPackets(path)
+
+	// and it is transferred back to the contract on A
+	chainB.SendMsgs(
+		ibctransfertypes.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID,
+			ibcCoin, actorChainB.String(), contractAddr.String(), chainB.GetTimeoutHeight(), 0,
+			fmt.Sprintf(`{"dest_callback":{"address":"%s"}}`, contractAddr.String())),
+	)
+	wasmibctesting.RelayAndAckPendingPackets(path)
+
+	// then the contract on A should receive a destination chain callback with correct funds
+	var response queryResp
+	chainA.SmartQuery(contractAddr.String(), queryMsg{CallbackStats: struct{}{}}, &response)
+	assert.Empty(t, response.IBCAckCallbacks)
+	assert.Empty(t, response.IBCTimeoutCallbacks)
+	assert.Len(t, response.IBCDestinationCallbacks, 1)
+	assert.Equal(t, []byte("{\"result\":\"AQ==\"}"), response.IBCDestinationCallbacks[0].Ack.Data)
+	// the denom should be reversed back correctly to the original denom
+	assert.Equal(t,
+		wasmvmtypes.Array[wasmvmtypes.Coin]{wasmvmtypes.NewCoin(1, sdk.DefaultBondDenom)},
+		response.IBCDestinationCallbacks[0].Transfer.Funds,
+	)
+	assert.Equal(t, contractAddr.String(), response.IBCDestinationCallbacks[0].Transfer.Receiver)
+	assert.Equal(t, actorChainB.String(), response.IBCDestinationCallbacks[0].Transfer.Sender)
+}
+
 func TestIBCCallbacksWithoutEntrypoints(t *testing.T) {
 	// scenario:
 	// given two chains

tests/e2e/testdata/ibc_callbacks.wasm

@@ -4,6 +4,7 @@ import (
 	"math"
 
 	wasmvmtypes "github.com/CosmWasm/wasmvm/v3/types"
+	transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types"
@@ -34,11 +35,12 @@ type appVersionGetter interface {
 type IBCHandler struct {
 	keeper           types.IBCContractKeeper
 	channelKeeper    types.ChannelKeeper
+	transferKeeper   types.ICS20TransferPortSource
 	appVersionGetter appVersionGetter
 }
 
-func NewIBCHandler(k types.IBCContractKeeper, ck types.ChannelKeeper, vg appVersionGetter) IBCHandler {
-	return IBCHandler{keeper: k, channelKeeper: ck, appVersionGetter: vg}
+func NewIBCHandler(k types.IBCContractKeeper, ck types.ChannelKeeper, tk types.ICS20TransferPortSource, vg appVersionGetter) IBCHandler {
+	return IBCHandler{keeper: k, channelKeeper: ck, transferKeeper: tk, appVersionGetter: vg}
 }
 
 // OnChanOpenInit implements the IBCModule interface
@@ -412,9 +414,55 @@ func (i IBCHandler) IBCReceivePacketCallback(
 		return err
 	}
 
+	var transfer *wasmvmtypes.IBCTransferCallback
+
+	// detect successful IBC transfer, meaning:
+	// 1. it was sent to the transfer module
+	// 2. the acknowledgement was successful
+	if packet.GetDestPort() == i.transferKeeper.GetPort(cachedCtx) && ack.Success() {
+
+		transferData, err := transfertypes.UnmarshalPacketData(packet.GetData(), version, "")
+		if err != nil {
+			return errorsmod.Wrap(err, "unmarshal transfer packet data")
+		}
+
+		// just making sure we have a valid address
+		receiverAddr, err := sdk.AccAddressFromBech32(transferData.Receiver)
+		if err != nil {
+			return err
+		}
+
+		// For a more in-depth explanation of the logic here, see the transfer module implementation:
+		// https://github.com/cosmos/ibc-go/blob/a6217ab02a4d57c52a938eeaff8aeb383e523d12/modules/apps/transfer/keeper/relay.go#L147-L175
+		// and the sequence diagram in the ICS20 spec:
+		// https://github.com/cosmos/ibc/blob/9be3630/spec/app/ics-020-fungible-token-transfer/README.md#data-structures
+		if transferData.Token.Denom.HasPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) {
+			// This is a denom coming from this chain, being sent back again, so we remove the prefix.
+			// See for example the "A -> C" step in the sequence diagram.
+			transferData.Token.Denom.Trace = transferData.Token.Denom.Trace[1:]
+		} else {
+			// prefixing happens on the receiving end, so we need to do that here
+			// See for example the "C -> A" step in the sequence diagram.
+			trace := []transfertypes.Hop{transfertypes.NewHop(packet.GetDestPort(), packet.GetDestChannel())}
+			transferData.Token.Denom.Trace = append(trace, transferData.Token.Denom.Trace...)
+		}
+
+		transfer = &wasmvmtypes.IBCTransferCallback{
+			Receiver: receiverAddr.String(),
+			Sender:   transferData.Sender,
+			Funds: wasmvmtypes.Array[wasmvmtypes.Coin]{
+				{
+					Denom:  transferData.Token.GetDenom().IBCDenom(),
+					Amount: transferData.Token.GetAmount(),
+				},
+			},
+		}
+	}
+
 	msg := wasmvmtypes.IBCDestinationCallbackMsg{
-		Ack:    wasmvmtypes.IBCAcknowledgement{Data: ack.Acknowledgement()},
-		Packet: newIBCPacket(packet),
+		Ack:      wasmvmtypes.IBCAcknowledgement{Data: ack.Acknowledgement()},
+		Packet:   newIBCPacket(packet),
+		Transfer: transfer,
 	}
 
 	err = i.keeper.IBCDestinationCallback(cachedCtx, contractAddr, msg)

x/wasm/ibc.go

@@ -110,7 +110,7 @@ func TestOnRecvPacket(t *testing.T) {
 				},
 			}
 			channelVersion := ""
-			h := NewIBCHandler(&mock, nil, nil)
+			h := NewIBCHandler(&mock, nil, nil, nil)
 			em := &sdk.EventManager{}
 			ctx := sdk.Context{}.WithEventManager(em)
 			if spec.expPanic {