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

x/ibc: transfer specs #7580

Merged
merged 23 commits into from
Nov 9, 2020
Merged

x/ibc: transfer specs #7580

merged 23 commits into from
Nov 9, 2020

Conversation

fedekunze
Copy link
Collaborator

No description provided.

x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
fedekunze and others added 3 commits October 29, 2020 17:16
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK though I still don't quite see how the proposed relayer service would work (in a verifiable fashion).

Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe connecting to all chains is a workable solution for multi-hop tokens. Client-trusted IBC explorers seem like the only viable approach to me.

EDIT: See comment below, retracted

Comment on lines 61 to 64
1. `GET /ibc_transfer/v1beta1/denom_traces/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2` -> `{"path": "transfer/channelToA", "base_denom": "uatom"}`
2. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer/client_state"` -> `{"client_id": "clientA", "chain-id": "chainA", ...}`
3. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer"` -> `{"channel_id": "channelToA", port_id": "transfer", counterparty: {"channel_id": "channelToB", port_id": "transfer"}, ...}`
4. `GET /ibc/channel/v1beta1/channels/channelToB/ports/transfer/client_state" -> {"client_id": "clientB", "chain-id": "chainB",}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong, in a direct connection there would be no need to query the counterparty chainID information because we already know our own chain-id.

Suggested change
1. `GET /ibc_transfer/v1beta1/denom_traces/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2` -> `{"path": "transfer/channelToA", "base_denom": "uatom"}`
2. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer/client_state"` -> `{"client_id": "clientA", "chain-id": "chainA", ...}`
3. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer"` -> `{"channel_id": "channelToA", port_id": "transfer", counterparty: {"channel_id": "channelToB", port_id": "transfer"}, ...}`
4. `GET /ibc/channel/v1beta1/channels/channelToB/ports/transfer/client_state" -> {"client_id": "clientB", "chain-id": "chainB",}`
1. `GET /ibc_transfer/v1beta1/denom_traces/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2` -> `{"path": "transfer/channelToA", "base_denom": "uatom"}`
2. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer/client_state"` -> `{"client_id": "clientA", "chain-id": "chainA", ...}`

4. Retrieve the the client identifier or chain identifier from the client state (eg: on
Tendermint clients) and store it locally.

Using the gRPC gataway client service the steps above would be, with a given IBC token `ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Using the gRPC gataway client service the steps above would be, with a given IBC token `ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2`:
Using the gRPC gataway client service the steps above would be, with a given IBC token `ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2` stored on `chainB`:

Comment on lines +52 to +57
2. Query the channel with the `portID/channelID` pair, which corresponds to the first destination of the
token.
3. Query the client state using the identifiers pair. Note that this query will return a `"Not
Found"` response if the current chain is not connected to this channel.
4. Retrieve the the client identifier or chain identifier from the client state (eg: on
Tendermint clients) and store it locally.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. Query the channel with the `portID/channelID` pair, which corresponds to the first destination of the
token.
3. Query the client state using the identifiers pair. Note that this query will return a `"Not
Found"` response if the current chain is not connected to this channel.
4. Retrieve the the client identifier or chain identifier from the client state (eg: on
Tendermint clients) and store it locally.
2. Query the client state associated with the `portID/channelID` channel, which corresponds to the source of the
token.
Note that this query will return a `"Not Found"` response if the current chain is not connected to this channel.
3. Retrieve the the client identifier or chain identifier from the client state (eg: on
Tendermint clients) and store it locally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to perform the query using the channel counterparty to get the origin chain

3. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer"` -> `{"channel_id": "channelToA", port_id": "transfer", counterparty: {"channel_id": "channelToB", port_id": "transfer"}, ...}`
4. `GET /ibc/channel/v1beta1/channels/channelToB/ports/transfer/client_state" -> {"client_id": "clientB", "chain-id": "chainB",}`

Then, the token transfer chain path for the `uatom` denomination would be: `chainB` -> `chainA`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Then, the token transfer chain path for the `uatom` denomination would be: `chainB` -> `chainA`.
Then, the token transfer chain path for the `uatom` denomination would be: `chainA` -> `chainB`.

x/ibc/applications/transfer/spec/01_concepts.md Outdated Show resolved Hide resolved
For clients that want to display the source of the token, it is recommended to use the following
alternatives for each of the following cases:

#### Direct connection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### Direct connection
#### Single Channel Hop

Connection is a name-clash with IBC connection which is separate

Comment on lines 81 to 84
- **Relayer as a Service (RaaS)**: A longer term solution is to use/create a relayer service that
could map the denomination trace to the chain path timeline for each token (i.e `origin chain ->
chain #1 -> ... -> chain #(n-1) -> final chain`). Clients would be advised to connect to public
relayers that support the largest number of connections between chains in the ecosystem. Unfortunately, none of the existing public relayers (in [Golang](https://github.com/cosmos/relayer) and [Rust](https://github.com/informalsystems/ibc-rs)), provide this service to clients.
Copy link
Member

@AdityaSripal AdityaSripal Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessarily a relayer function (though relayers could also provide this service). This really is describing an IBC explorer which is a separate thing from a relayer service.

Suggested change
- **Relayer as a Service (RaaS)**: A longer term solution is to use/create a relayer service that
could map the denomination trace to the chain path timeline for each token (i.e `origin chain ->
chain #1 -> ... -> chain #(n-1) -> final chain`). Clients would be advised to connect to public
relayers that support the largest number of connections between chains in the ecosystem. Unfortunately, none of the existing public relayers (in [Golang](https://github.com/cosmos/relayer) and [Rust](https://github.com/informalsystems/ibc-rs)), provide this service to clients.
- **IBC Explorer**: Another solution is to use/create an IBC explorer service that maps the locally used channel and client identifiers used by each chain to the global identifiers (chain-id) that they correspond to. A client can then take a given denomination trace and iteratively query each channel identifier against the chain that uses that identifier in the token's timeline. This prevents clients from having to establish connections to every chain in the timeline, but it does introduce a trusted party in the IBC explorer.
Given the existence of an IBC explorer service. A client psuedo algorithm may look like this:
1. Retrieve the denomination trace of the token => `transfer/channelC/transfer/channelB/transfer/channelA` on the current chain `chainD`.
2. Retrieve the immediately preceding blockchain `chainC` in the timeline by querying the explorer against our current `chainD` with the first channel-id `channelC` in the trace.
3. Use the resulting `chainC` to query for the prior blockchain in the timeline by querying the explorer for the blockchain corresponding to `channelB` on `chainC`.
4. Use the resulting `chainB` to query for the prior blockchain in the timeline by querying the explorer for the blockchain corresponding to `channelA` on `chainB`.
This will provide a full history of the token: `chainA (source location)` -> `chainB` -> `chainC` -> `chainD (current location)`
**Important Note**: The only thing that is provable from `chainD` is the token trace which is a list of the local channel- and port-identifiers the token passed through from source to the current location. From this information, the chain-id of the immediate predecessor is also provable as shown in the `Single channel hop` example. IBC explorers must be trusted by clients to provide the correct chain history given a list of local channel identifiers. Since this is a relaxation of the security model of the core IBC spec, any chain history provided by an IBC explorer must be explicitly understood and displayed as coming from a (potentially) untrustworthy IBC explorer so that end users may decide whether or not to trust that information.
An implementation of an IBC explorer does not currently exist, though they may become an important part of the ecosystem as multi-chain token histories become more common.

The only viable alternative for clients (at the time of writing) to tokens with multiple connection hops, is to connect to all chains directly and perform relevant queries to each of them in the sequence.
:::

## Locked Funds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to this, we should also mention that to retrieve a token in its original form, the token must be sent back along the exact route that it took originally. Sending a token back to the same chain across a different channel will not move the token back across its timeline. If a channel in the chain history closes before the token can be sent back across that channel, then the token will not be returnable to its original form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really important point and something user interfaces should help with

@AdityaSripal
Copy link
Member

Misunderstood the Connecting to All Chains solution. Going to resolve all irrelevant commments

@AdityaSripal
Copy link
Member

One very important thing to note in the context of transfer security is that chain-ids (or denominations) are not globally unique. I can very easily create my own blockchain called cosmos-hub with my own tokens uatom and send them around with IBC and the token history will show that these uatom tokens originated from a cosmos-hub blockchain. From a user standpoint, this is very easily confusable with the real cosmos-hub and real uatom and in my opinion is the easiest attack vector in ibc-transfer at the moment.

This is IBC working exactly as intended, but leaves users vulnerable to potentially catastrophic mistakes imo. Perhaps the IBC team should suggest a way to mitigate this potential attack as well in this doc.

Comment on lines +61 to +64
1. `GET /ibc_transfer/v1beta1/denom_traces/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2` -> `{"path": "transfer/channelToA", "base_denom": "uatom"}`
2. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer/client_state"` -> `{"client_id": "clientA", "chain-id": "chainA", ...}`
3. `GET /ibc/channel/v1beta1/channels/channelToA/ports/transfer"` -> `{"channel_id": "channelToA", port_id": "transfer", counterparty: {"channel_id": "channelToB", port_id": "transfer"}, ...}`
4. `GET /ibc/channel/v1beta1/channels/channelToB/ports/transfer/client_state" -> {"client_id": "clientB", "chain-id": "chainB", ...}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a route that does this instead of pushing this work on to clients

Comment on lines 85 to 86
Additionally, client would be advised in the future to use RaaS that support the largest number of
connections between chains in the ecosystem. Unfortunately, none of the existing public relayers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding support for this in the upcoming relayer rest interface would be doable.

The only viable alternative for clients (at the time of writing) to tokens with multiple connection hops, is to connect to all chains directly and perform relevant queries to each of them in the sequence.
:::

## Locked Funds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really important point and something user interfaces should help with

Comment on lines +11 to +14
| `tx_msg_ibc_transfer` | The total amount of tokens transferred via IBC in a `MsgTransfer` (source or sink chain) | token | gauge |
| `ibc_transfer_packet_receive` | The total amount of tokens received in a `FungibleTokenPacketData` (source or sink chain) | token | gauge |
| `ibc_transfer_send` | Total number of IBC transfers sent from a chain (source or sink) | transfer | counter |
| `ibc_transfer_receive` | Total number of IBC transfers received to a chain (source or sink) | transfer | counter |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tagged with src/dst chan/port and other metadata I would assume?

@okwme
Copy link
Contributor

okwme commented Nov 3, 2020

Relevant to this topic is ongoing conversation at the Chain Agnostic Improvement Proposal repo:
ChainAgnostic/CAIPs#27

@fedekunze
Copy link
Collaborator Author

I agree with @jackzampolin that we should create a route that allows clients to get the client and chain data from a given IBC denomination. Although it is out-of-scope for this spec and should be done in a separate PR.

@fedekunze fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label Nov 9, 2020
@mergify mergify bot merged commit f294db9 into master Nov 9, 2020
@mergify mergify bot deleted the fedekunze/ibc-transfer-spec branch November 9, 2020 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants