cosmos / ibc-apps

IBC applications and middleware for Cosmos SDK chains.
Apache License 2.0
82 stars 62 forks source link

doc: IBC Hooks counter smart contract README contains misleading explanation #187

Open ysv opened 5 months ago

ysv commented 5 months ago

Here https://github.com/cosmos/ibc-apps/blob/main/modules/ibc-hooks/tests/unit/testdata/counter/README.md you mention that

This way we can verify that, independently of the sender, the funds will end up under the WasmHooksModuleAccount address when the contract is executed via an IBC send that goes through the wasmhooks module.

But in reality I can see that module account never receives funds. Instead intermediary account is created from channel+original sender and it temporary holds funds.

Proofs:

// Calculate the receiver / contract caller based on the packet's channel and sender
channel := packet.GetDestChannel()
sender := data.GetSender()
senderBech32, err := keeper.DeriveIntermediateSender(channel, sender, h.bech32PrefixAccAddr)
if err != nil {
return NewEmitErrorAcknowledgement(ctx, types.ErrBadSender, fmt.Sprintf("cannot convert sender address %s/%s to bech32: %s", channel, sender, err.Error()))
}

// 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)
// and execute the underlying OnRecvPacket() call (which should eventually land on the transfer app's
// relay.go and send the sunds to the intermediary account.
//
// If that succeeds, we make the contract call

https://github.com/cosmos/ibc-apps/blob/main/modules/ibc-hooks/wasm_hook.go#L71

IMO the README should be rephrased.

Reecepbcups commented 3 months ago

Assigned Nicolas from the osmosis team to take a look 🫡