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

Better Native ICA support #947

Open
JakeHartnell opened this issue Aug 18, 2022 · 11 comments
Open

Better Native ICA support #947

JakeHartnell opened this issue Aug 18, 2022 · 11 comments

Comments

@JakeHartnell
Copy link

JakeHartnell commented Aug 18, 2022

Use case: I want to write a contract that stakes an asset on a non-CosmWasm chain.

Really love implementing ICA in pure CosmWasm as with Confio's cw-ibc-demo. Unfortunately this will not work with non-CosmWasm chains like the Cosmos Hub.

Reading through the latest ICA documentation, it seems we may need to call RegisterInterchainAccount and then can call SendTx? Or perhaps just SendTx is enough and it's smart enough to register the account?

Would be nice if we could extend cosmwasm_std IbcMsg to support native ICA messages. I'm hacking a bit today on getting them working using CosmosMsg::Stargate, but not sure how successful that will be and that it could be a much better dev experience to just support it outright.

In cosmwasm_std it would be ultimately nice to have something like:

#[non_exhaustive]
pub enum IbcMsg {
    // ...
    IcaTx {
        channel_id: String,
        msgs: Vec<CosmosMsg<Empty>>,
        timeout: IbcTimeout,
    },
    // Guess we need this to get the channel ID?
    RegisterIca {
        connnection_id: String,
        port_id: String,
        address: String,
    }
}

Realize this issue also touches the cosmwasm repo, but figured it would be best to start the conversation here.

@ethanfrey
Copy link
Member

This has been blocked on two items:

  1. cosmos/interchain-accounts being officially promoted to the standard controller, so we can assume almost all chains use this. (it was currently marked demo and not to use in production)

  2. the addition of callbacks to the go implementation. when you send a message to another chain, there is a good chance it failed. we need to inform the calling contract so it can handle that

I do believe IG is working on both of those items along with p2pvalidator. I have not followed their work the last month, but when it is ready, I am happy to look at this issue. (and add ics20 callbacks for success/failure in transfer)

@ethanfrey
Copy link
Member

note the links above were in the host side. while wasmd adds the demo controller, all chains I have looked at have no controller enabled, which is essential to make the call

@jtieri
Copy link

jtieri commented Aug 19, 2022

Reading through the latest ICA documentation, it seems we may need to call RegisterInterchainAccount and then can call SendTx? Or perhaps just SendTx is enough and it's smart enough to register the account?

RegisterInterchainAccount will need to be called first, this is where the logic lives for ensuring each native account on a controller chain maps 1:1 with an ICA on the host chain. This also initiates the channel handshake afaiu so it's necessary before you can send packets over the wire.

@giansalex
Copy link
Contributor

giansalex commented Aug 20, 2022

I forked interchain-accounts-demo and modified IBC middleware to call x/wasm because wasm does not support the ica-controller port.

For ica-controller only uses IBC methods:

  • OnChanOpenInit
  • OnChanOpenAck
  • OnAcknowledgementPacket
  • OnTimeoutPacket

IBC port format icacontroller-{contract-address}

The contract can call RegisterAccount, SubmitTx using CosmosMsg::Stargate and handle IBC Ack, Timeout.

demo:
contract (controller chain): juno14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9skjuwg8
ica-account (host chain): juno1vekslrpham2w97y3gsk7wpymd7u990ap4uhwyxztv2kgrars7r9qjxfvaa
Rcv and Ack txs:
image

Currently the example assumes that the interchain accounts will be contracts.
https://github.com/disperze/wasm-ica-demo/blob/wasm-ibc3/x/inter-tx/ibc_module.go
https://github.com/disperze/Juno/blob/ica-demo/app/app.go

could this be a way?

@faddat
Copy link
Contributor

faddat commented Aug 25, 2022

I have an unconfirmed hunch that this:

#938

and/ or this:

#940

get us somehow closer.

@giansalex code is likely still useful, and maybe should be upstreamed into inter-tx directly.

@alpe alpe moved this to 🆕 New in wasmd backlog Sep 23, 2022
@alpe alpe moved this from 🆕 New to ❓ Needs more info in wasmd backlog Sep 30, 2022
@ethanfrey ethanfrey added this to the v0.32 milestone Nov 4, 2022
@ethanfrey
Copy link
Member

Put this on 0.32 milestone. Hopefully with cosmos-sdk 0.47 and ibc-go (v8?) we will have a standardised controller with proper callbacks as to whether ICA packets succeeded or failed. Until then no point to add support

@alpe alpe removed this from wasmd backlog Mar 3, 2023
@alpe
Copy link
Contributor

alpe commented Apr 24, 2023

ICA has been used with ibc-go for some time now. We need ADR-8 callbacks or something similar to cover the process. I have commented on CosmWasm/cosmwasm#1652 (comment)

@alpe
Copy link
Contributor

alpe commented May 30, 2023

Moving this to a future release. We are blocked on ibc-go providing the adr-8 standard. I did some work in #1368 to help

@alpe alpe added the blocked label May 30, 2023
@alpe alpe modified the milestones: v0.41.0, v0.42.0 May 30, 2023
@srdtrk
Copy link

srdtrk commented Jul 5, 2023

It might be relevant to put this here, but it is currently possible to build a cosmwasm ica-controller that can communicate with the golang ica-host. This can handle callbacks too. I made such an implementation complete with end to end tests that run in the github ci. Does it help resolve this issue? https://github.com/srdtrk/cw-ica-controller

This contract doesn't use any api to access ICA. It completes the handshake as the ica-controller. So your entire wasm chain could have ICA feature disabled but this implementation would still work.

This contract uses ibc-go/main's simd and wasmd 0.40.2 in e2e tests. But I have tested it with wasmd 0.32.1 as well. It is also possible to modify the contract with proto libraries to have it work with older versions of ica-host (that don't support json encoded ica transactions)

@giansalex
Copy link
Contributor

This contract doesn't use any api to access ICA. It completes the handshake as the ica-controller. So your entire wasm chain could have ICA feature disabled but this implementation would still work.

Yes, following the philosophy of cw20-ics20.
Previously it was not possible due to a port name validation in ibc-go (counterparty chain), which was removed in v4.2

An implementation of ica-controller for liquid staking has been running smoothly.
image

@srdtrk
Copy link

srdtrk commented Jul 5, 2023

Good to know @giansalex. Is there a GitHub repo so that I can compare the implementations?

@alpe alpe modified the milestones: v0.42.0, v2.0.0 Jul 7, 2023
@pinosu pinosu removed this from the v2.0.0 milestone Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants