Skip to content

Commit

Permalink
fill funds for ICS20 packet
Browse files Browse the repository at this point in the history
  • Loading branch information
beer-1 committed Jun 26, 2024
1 parent a4b095a commit 7fd0b5e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 24 deletions.
26 changes: 8 additions & 18 deletions app/ibc-hooks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,41 +29,32 @@ type HookData struct {
}

type MsgExecuteContract struct {
// Sender is the actor that committed the message in the sender chain
Sender string
// Contract is the address of the smart contract
Contract string
// Msg json encoded message to be passed to the contract
Msg RawContractMessage
// Funds coins that are transferred to the contract on execution
Funds sdk.Coins
}
```

So we detail where we want to get each of these fields from:

* Sender: We cannot trust the sender of an IBC packet, the counterparty chain has full ability to lie about it.
We cannot risk this sender being confused for a particular user or module address on Osmosis.
We cannot risk this sender being confused for a particular user or module address on the chain.
So we replace the sender with an account to represent the sender prefixed by the channel and a wasm module prefix.
This is done by setting the sender to `Bech32(Hash("ibc-wasm-hook-intermediary" || channelID || sender))`, where the channelId is the channel id on the local chain.
* Contract: This field should be directly obtained from the ICS-20 packet metadata
* Msg: This field should be directly obtained from the ICS-20 packet metadata.
* Funds: This field is set to the amount of funds being sent over in the ICS 20 packet. One detail is that the denom in the packet is the counterparty chains representation of the denom, so we have to translate it to Osmosis' representation.

> **_WARNING:_** Due to a [bug](https://twitter.com/SCVSecurity/status/1682329758020022272) in the packet forward middleware, we cannot trust the sender from chains that use PFM. Until that is fixed, we recommend chains to not trust the sender on contracts executed via IBC hooks.
So our constructed cosmwasm message that we execute will look like:

```go
msg := MsgExecuteContract{
// Sender is the that actor that signed the messages
Sender: "init1-hash-of-channel-and-sender",
// Contract is the address of the smart contract
Contract: packet.data.memo["wasm"]["contract"],
// Msg json encoded message to be passed to the contract
Msg: packet.data.memo["wasm"]["msg"],
// Funds coins that are transferred to the contract on execution
Funds: sdk.NewCoin{Denom: ibc.ConvertSenderDenomToLocalDenom(packet.data.Denom), Amount: packet.data.Amount}
}
```

Expand All @@ -82,13 +73,12 @@ ICS20 is JSON native, so we use JSON for the memo format.
"receiver": "contract addr or blank",
"memo": {
"wasm": {
"contract": "init1contractAddr",
"msg": {
"raw_message_fields": "raw_message_data",
},
"funds": [
{"denom": "ibc/denom", "amount": "100"}
]
"message": {
"contract": "init1contractAddr",
"msg": {
"raw_message_fields": "raw_message_data",
}
}
}
}
}
Expand All @@ -100,7 +90,7 @@ An ICS20 packet is formatted correctly for wasmhooks iff the following all hold:
* `memo` is not blank
* `memo` is valid JSON
* `memo` has at least one key, with value `"wasm"`
* `memo["wasm"]["message"]` has exactly two entries, `"contract"`, `"msg"` and `"fund"`
* `memo["wasm"]["message"]` has exactly two entries, `"contract"` and `"msg"`
* `memo["wasm"]["message"]["msg"]` is a valid JSON object
* `receiver == "" || receiver == memo["wasm"]["contract"]`

Expand Down
14 changes: 12 additions & 2 deletions app/ibc-hooks/receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"

transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
Expand Down Expand Up @@ -44,7 +45,7 @@ func (h WasmHooks) onRecvIcs20Packet(
}

// Calculate the receiver / contract caller based on the packet's channel and sender
intermediateSender := deriveIntermediateSender(packet.GetDestChannel(), data.GetSender())
intermediateSender := DeriveIntermediateSender(packet.GetDestChannel(), data.GetSender())

// The funds sent on this packet need to be transferred to the intermediary account for the sender.
// For this, we override the ICS20 packet's Receiver (essentially hijacking the funds to this new address)
Expand All @@ -64,7 +65,15 @@ func (h WasmHooks) onRecvIcs20Packet(
return ack
}

// Extract the denom and amount from the packet data
denom := MustExtractDenomFromPacketOnRecv(packet)
amount, ok := math.NewIntFromString(data.GetAmount())
if !ok {
return newEmitErrorAcknowledgement(fmt.Errorf("invalid amount: %s", data.GetAmount()))
}

msg.Sender = intermediateSender
msg.Funds = sdk.NewCoins(sdk.NewCoin(denom, amount))
_, err = h.execMsg(ctx, msg)
if err != nil {
return newEmitErrorAcknowledgement(err)
Expand Down Expand Up @@ -100,7 +109,7 @@ func (h WasmHooks) onRecvIcs721Packet(
}

// Calculate the receiver / contract caller based on the packet's channel and sender
intermediateSender := deriveIntermediateSender(packet.GetDestChannel(), data.GetSender())
intermediateSender := DeriveIntermediateSender(packet.GetDestChannel(), data.GetSender())

// The funds sent on this packet need to be transferred to the intermediary account for the sender.
// For this, we override the ICS721 packet's Receiver (essentially hijacking the funds to this new address)
Expand All @@ -121,6 +130,7 @@ func (h WasmHooks) onRecvIcs721Packet(
}

msg.Sender = intermediateSender
msg.Funds = sdk.NewCoins()
_, err = h.execMsg(ctx, msg)
if err != nil {
return newEmitErrorAcknowledgement(err)
Expand Down
22 changes: 20 additions & 2 deletions app/ibc-hooks/receive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import (
"os"
"testing"

"cosmossdk.io/math"
"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"

nfttransfertypes "github.com/initia-labs/initia/x/ibc/nft-transfer/types"
ibchooks "github.com/initia-labs/miniwasm/app/ibc-hooks"

wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
Expand Down Expand Up @@ -83,9 +86,21 @@ func Test_onReceivePacket_memo(t *testing.T) {
dataBz, err := json.Marshal(&data)
require.NoError(t, err)

// funds foo coins to the intermediate sender
intermediateSender, err := sdk.AccAddressFromBech32(ibchooks.DeriveIntermediateSender("channel-0", data.GetSender()))
require.NoError(t, err)
denom := ibchooks.MustExtractDenomFromPacketOnRecv(channeltypes.Packet{
Data: dataBz,
DestinationPort: "wasm",
DestinationChannel: "channel-0",
})
input.Faucet.Fund(ctx, intermediateSender, sdk.NewCoin(denom, math.NewInt(10000)))

// failed to due to acl
ack := input.IBCHooksMiddleware.OnRecvPacket(ctx, channeltypes.Packet{
Data: dataBz,
Data: dataBz,
DestinationPort: "wasm",
DestinationChannel: "channel-0",
}, addr)
require.False(t, ack.Success())

Expand All @@ -96,8 +111,11 @@ func Test_onReceivePacket_memo(t *testing.T) {

// success
ack = input.IBCHooksMiddleware.OnRecvPacket(ctx, channeltypes.Packet{
Data: dataBz,
Data: dataBz,
DestinationPort: "wasm",
DestinationChannel: "channel-0",
}, addr)
fmt.Println(string(ack.Acknowledgement()))
require.True(t, ack.Success())

// check the contract state
Expand Down
37 changes: 35 additions & 2 deletions app/ibc-hooks/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"

nfttransfertypes "github.com/initia-labs/initia/x/ibc/nft-transfer/types"

Expand All @@ -20,9 +21,9 @@ import (

const senderPrefix = "ibc-wasm-hook-intermediary"

// deriveIntermediateSender compute intermediate sender address
// DeriveIntermediateSender compute intermediate sender address
// Bech32(Hash(Hash("ibc-hook-intermediary") + channelID/sender))
func deriveIntermediateSender(channel, originalSender string) string {
func DeriveIntermediateSender(channel, originalSender string) string {
senderStr := fmt.Sprintf("%s/%s", channel, originalSender)
senderAddr := sdk.AccAddress(address.Hash(senderPrefix, []byte(senderStr)))
return senderAddr.String()
Expand Down Expand Up @@ -134,3 +135,35 @@ func isAckError(appCodec codec.Codec, acknowledgement []byte) bool {

return false
}

// MustExtractDenomFromPacketOnRecv takes a packet with a valid ICS20 token data in the Data field and returns the
// denom as represented in the local chain.
// If the data cannot be unmarshalled this function will panic
func MustExtractDenomFromPacketOnRecv(packet ibcexported.PacketI) string {
var data transfertypes.FungibleTokenPacketData
if err := json.Unmarshal(packet.GetData(), &data); err != nil {
panic("unable to unmarshal ICS20 packet data")
}

var denom string
if transfertypes.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) {
// remove prefix added by sender chain
voucherPrefix := transfertypes.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())

unprefixedDenom := data.Denom[len(voucherPrefix):]

// coin denomination used in sending from the escrow address
denom = unprefixedDenom

// The denomination used to send the coins is either the native denom or the hash of the path
// if the denomination is not native.
denomTrace := transfertypes.ParseDenomTrace(unprefixedDenom)
if denomTrace.Path != "" {
denom = denomTrace.IBCDenom()
}
} else {
prefixedDenom := transfertypes.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel()) + data.Denom
denom = transfertypes.ParseDenomTrace(prefixedDenom).IBCDenom()
}
return denom
}

0 comments on commit 7fd0b5e

Please sign in to comment.