Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ysv opened this issue Apr 10, 2024 · 1 comment
Open
Assignees
Labels
ibc-hooks Label for items related to Osmosis' ibc-hooks implementation

Comments

@ysv
Copy link

ysv commented Apr 10, 2024

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
Copy link
Member

Assigned Nicolas from the osmosis team to take a look 🫡

@Reecepbcups Reecepbcups added the ibc-hooks Label for items related to Osmosis' ibc-hooks implementation label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ibc-hooks Label for items related to Osmosis' ibc-hooks implementation
Projects
None yet
Development

No branches or pull requests

3 participants