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

Expose counterparty connection_id to IBCModule api #3942

Closed
3 tasks
srdtrk opened this issue Jun 22, 2023 · 8 comments
Closed
3 tasks

Expose counterparty connection_id to IBCModule api #3942

srdtrk opened this issue Jun 22, 2023 · 8 comments
Labels
change: api breaking Issues or PRs that break Go API (need to be release in a new major version) change: state machine breaking Issues or PRs that break consensus (need to be released in at least a new minor version) core needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.

Comments

@srdtrk
Copy link
Member

srdtrk commented Jun 22, 2023

Summary

Currently, none of the IBCModule's APIs, during a channel handshake, have access to the counterparty's connection_id. This is an issue during interchain-accounts handshake.

Problem Definition

During the handshake, the IBCModule receives information about the current channel's parameters so it can correctly perform validation. During the handshake, the IBCModule has access to the counterparty's ibc parameters through Counterparty message. Connection id is not a part of this message...

Moreover, counterparty connection_id is required in exchanged metadata in ica handshake. Due to this, external ica-controller implementations cannot support empty version string. And the ica-controller implementation in ibc-go gets the counterparty id by using core modules which shouldn't be necessary (and cannot be done via smart contracts)

Proposal

There are multiple solutions to this problem but I propose that connection_id be added to Counterparty proto message.

// Counterparty defines a channel end counterparty
message Counterparty {
  option (gogoproto.goproto_getters) = false;

  // port on the counterparty chain which owns the other end of the channel.
  string port_id = 1;
  // channel end on the counterparty chain
  string channel_id = 2;
  // connection end on the counterparty chain
  string connection_id = 3;
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@srdtrk srdtrk added core type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. change: api breaking Issues or PRs that break Go API (need to be release in a new major version) needs discussion Issues that need discussion before they can be worked on change: state machine breaking Issues or PRs that break consensus (need to be released in at least a new minor version) labels Jun 22, 2023
@colin-axner
Copy link
Contributor

The counterparty connection ID is in the connection end. Is that not accessible?

@srdtrk
Copy link
Member Author

srdtrk commented Jun 27, 2023

@colin-axner That is not accessible unless you use the core keepers. Which is already impossible within smart contracts. And you'd need to know how to get it from core keepers too even in go modules

@colin-axner
Copy link
Contributor

Does wasm expose core IBC grpc's? There have been assumptions made that additional information held by core IBC is accessible to IBC applications when designing the app callbacks (especially for channel upgradability). Ideally we would ensure that query functions exposed by core IBC are accessible to smart contracts as well

@srdtrk
Copy link
Member Author

srdtrk commented Jul 3, 2023

Well, I guess technically you could query any grpc if you enable stargate feature and use protobuf libraries with this query type. I've never tested/seen it being used to query core ibc. But this is very unfriendly UX at the very least.

If you're curious about what IbcQuery does, it can essentially be used to get IbcChannel but they also don't provide access to core ibc nor counterparty connection id.

@giansalex
Copy link

Juno enabled StargateQuery and /ibc.core.connection.v1.Query/Connection was included.

pub fn query_ibc_connection(deps: Deps, connection: &str) -> StdResult<Connection> {
    let r = QueryConnectionRequest {
        connection_id: connection.into(),
    };

    let q = QueryRequest::Stargate {
        path: "/ibc.core.connection.v1.Query/Connection".into(),
        data: Binary::from(r.to_bytes()?),
    };
    let res: ConnectionResponse = deps.querier.query(&q)?;

    Ok(res.connection)
}

There you can get the Counterparty connectionID

@srdtrk
Copy link
Member Author

srdtrk commented Jul 10, 2023

Thanks @giansalex. I've done more research into this and here is what I found. I was able to reproduce this in juno like you mentioned because juno has this query type for stargate whitelisted here. However, I don't know if we should rely on stargate queries for this. Because stargate queries are disabled by default and implemented as a plugin whose codec may not be the same among chains.

Osmosis (and other chains) haven't whitelisted this query yet.

@giansalex
Copy link

I hope that permission-less smart-contracts chains will include it, I think Terra2 also has stargateQueries enabled

@srdtrk
Copy link
Member Author

srdtrk commented Dec 18, 2023

We've decided that counterparty_connection_id should not be exposed to the application layer. Instead, we should deprecate the necessity of it in the ICA handshake (see #5437)

@srdtrk srdtrk closed this as completed Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change: api breaking Issues or PRs that break Go API (need to be release in a new major version) change: state machine breaking Issues or PRs that break consensus (need to be released in at least a new minor version) core needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
None yet
Development

No branches or pull requests

3 participants