IBC modules and relayer - Formal specifications and Rust implementation

Informal Systems

Last update: Dec 31, 2022

Overview

ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.

This project comprises primarily four crates:

The ibc crate defines the main data structures and on-chain logic for the IBC protocol.
The ibc-relayer crate provides an implementation of an IBC relayer, as a library.
The ibc-relayer-cli is a CLI (a wrapper over the ibc-relayer library), comprising the hermes binary.
The ibc-proto is a library with proto definitions necessary for interacting with Cosmos SDK IBC structs.

See the table below for more details.

Includes TLA+ specifications.

Crate name	Type	Version	Docs
ibc (modules)	lib
ibc-relayer	lib
ibc-relayer-cli	bin: hermes
ibc-proto	lib

Requirements

Developed with the latest stable version of Rust: 1.49.0. (May work with older versions.)

Hermes Guide

The relayer CLI binary, called hermes, has a comprehensive guide at hermes.informal.systems.

Contributing

IBC is specified in English in the cosmos/ics repo. Any protocol changes or clarifications should be contributed there.

This repo contains the TLA+ specification and Rust implementation for the IBC modules and relayer. If you're interested in contributing, please comment on an issue or open a new one!

Versioning

We follow Semantic Versioning, though APIs are still under active development.

Resources

License

Licensed under the Apache License, Version 2.0 (the "License"); you may not use the files in this repository except in compliance with the License. You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.

Comments

Setup IBC
Crate

ibc

Version

v0.5.0

Summary

Hermes fails with TxNoConfirmation error constantly when trying to communicate with a full node that does not have indexing enabled.

The error can be tracked down using a patched version of Hermes, and looks like this:

Jun 30 13:39:20.800 TRACE waiting for commit of block(s) Jun 30 13:39:21.001 ERROR query_txs fail: Internal error: transaction indexing is disabled (code: -32603)

Many thanks to @Fraccaman for helping us uncover this corner-case.

A separate, related problem was due to misconfiguration (see https://github.com/heliaxdev/ibc-setup/pull/1).

Acceptance criteria:

[ ] Hermes should catch the "transaction indexing is disabled" error and propagate it up the stack (instead of masking it under the hood of TxNoConfirmation)

[ ] Ideally, Hermes should be able to detect the health of a full node upon startup (similar to how we plan in #1057), so that the end-user is warned when Hermes is unable to correctly communicate with a full node

Original discussion

Summary of Bug

Probably this is not a bug, but I can't understand whats wrong. I'm trying to open a channel between a cosmos mainnet node and a "own gaia testnet" node. I am able to build the relayer and configure it correctly (It doesn't complain so I'm assuming it is correct) but as soon as I try to run hermes channel handshake id-1 id-2 transfer transfer it throw the following error: {"status":"error","result":"chain runtime/handle error: Light client supervisor error for chain id heliax: empty witness list"}.

p.s: @andynog suggested to open an issue

Version

v0.1.1

Steps to Reproduce

I have created a repository with a series of bash scripts to reproduce this use case here.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

A: bug A: question
opened by Fraccaman 65
Gaiad manager
Closes: #902

Description

This PR introduces the gm command described in issue #902. All functionality of the issue is implemented with some extras described in the documentation.

Start with the README!

For contributor use:

[X] Updated the Unreleased section of CHANGELOG.md with the issue.

[ ] If applicable: Unit tests written, added test to CI.

[X] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.

[ ] Updated relevant documentation (docs/) and code comments.

[X] Re-reviewed Files changed in the Github PR explorer.
opened by greg-szabo 25
Tracking issue for no-std support
This is a tracking issue to track the progress of no-std support for ibc-rs to close informalsystems/hermes#29. The change set for PR informalsystems/hermes#1109 is pretty broad, and as a result it is difficult to review and merge the PR all in one go. I would suggest that we split the changes into multiple smaller PRs, so that the changes can be incremental with the goal of eventually supporting no-std.

Based on the changes in PR informalsystems/hermes#1109, I'd suggest we have separate PRs for the following changes:

[x] informalsystems/hermes#1846

[x] informalsystems/hermes#1359

[x] informalsystems/hermes#1390

[x] informalsystems/tendermint-rs/pull/988

[x] informalsystems/ibc-proto-rs-tmp#7

[x] informalsystems/tendermint-rs/pull/986

[x] informalsystems/tendermint-rs/issues/1027

[x] informalsystems/hermes#1612

[x] informalsystems/hermes#1439

[x] informalsystems/tendermint-rs#981

[x] informalsystems/tendermint-rs#993

[x] informalsystems/tendermint-rs#983

[x] informalsystems/tendermint-rs#985

[x] informalsystems/tendermint-rs/issues/992

[x] Use flex-error to define error types (#988)

This is to replace anomaly for fine grained error handling. While not directly related to no-std, the PR removes majority use of std::error::Error and allows different error tracers such as anyhow, eyre, or naive StringTracer to be used, depending on their support in no-std environment.

[x] Use core and alloc crates in place of std crate whenever possible (#1156)

The crates core and alloc can be accessed by both std and no-std code, and therefore do not require feature flag to switch between importing from core/alloc or std.

To use alloc in std mode, we just have to add an extern crate alloc in the root module, and that is automatically linked by Rust in std mode.

There are still a small number of imports from std that are not available in core nor alloc. In that case, the PR leaves the use of std in those cases, and have them fixed in follow up PRs.

The changes to use core and alloc can be done in a separate PR independent of other changes. While we cannot ensure that the change covers everything, this will reduce the number of changes required in subsequent PRs and make it easier for review.

[x] Use a wrapper struct for Box<dyn std::error::Error> for remaining errors not updated by informalsystems/hermes#988.

PR informalsystems/hermes#988 have removed most of the uses of Box<dyn std::error::Error>, however there are still a few places remaining that are still using std::error::Error.

PR informalsystems/hermes#1109 attempts to polyfill std::error::Error by aliasing it to a DummyError struct in no-std mode. However doing so does not prevent dependencies from constructing Box<dyn std::error::Error>, which would prevent no-std from being supported. On the other hand, if we have control of all places that produce Box<dyn std::error::Error>, we can also modify them to return something other than Box<dyn std::error::Error>.

I'd propose that we define a type like LegacyError, which is either a newtype wrapper to Box<dyn std::error::Error> in std mode, or a newtype wrapper to String in no-std mode. By always using a newtype wrapper, we can avoid accidental implicit coercion from Box<dyn std::error::Error> to LegacyError in std mode, which will prevent no-std mode to work.

There should be a Cargo separate feature flag to switch between the two modes ot LegacyError. The purpose is to be able to test the two modes of LegacyError without fully activating no-std.

This change can be done inside ~#1109~ informalsystems/hermes#988, or a follow up PR to ~#1109~ informalsystems/hermes#988. It is also possible to make this change against master, although there will be much more changes needed.

Update: This is no longer needed as all use of std::error::Error are now removed.

[x] Find alternative crates that replace missing functionalities in core and alloc.

There are still a small number of essential types like SystemTime that are provided by std, but not provided by core nor alloc.

PR informalsystems/hermes#1109 attempts to polyfill the types like SystemTime inside no-std mode. However this makes it difficult to maintain two version of the code depending on the std/no-std mode.

Ideally we should instead try and find alternative crates that can work in both std and no-std mode.

For example, we may want to investigate whether it is possible to use chrono::DateTime directly instead of SystemTime. If we can, then it would reduce our burden of having to maintain the code in different modes.

[x] Update Cargo.toml to use updated dependencies that have been published and support no-std, including tendermint-rs crates.

PR informalsystems/hermes#1109 has a [patch.crates-io] section that refers to upstream dependencies like tendermint-rs that are also being patched to support no-std. This makes it difficult to review and merge the PR, because we cannot easily see what is being changed in the upstream dependencies. Even if the reviews pass, the PR cannot be merged because we cannot publish Cargo crates with git dependencies inside.

We should first make changes in ibc-rs that do not require upstream dependencies to be updated. Even if that do not result in supporting no-std in one go, making independent changes will help reduce the number of changes necessary when the dependencies are updated.

The PR for updating upstream dependencies should be done after the other PRs for no-std have been merged, and also when the upstream dependencies have been published. Ideally this PR should have minimal changes, other than updating Cargo.toml.

[x] Add Cargo feature flags to activate #![no_std].

Other than the issues listed above, there might be other blockers that prevent no-std mode from being enabled. In such case, it is still better to merge the other PRs first, and only activate no-std when everything is fixed.

I am listing this as a separate step, so that it does not become a blocker for the other PRs. The point is we do not need to have one or few big PRs that immediately pass when #![no_std] is enabled.

The priority for the other PRs is to continue to have ibc-rs work in std mode, and not be concerned with whether they compiles in no-std. We can still maintain our own private branches that activate no-std during development.

Ideally, this final PR should have just few line changes to introduce the feature flags and #![no_std].

Note that as mentioned in informalsystems/hermes#1164, #![no_std] alone is still not sufficient for true no-std support, as dependencies may still use std behind the scene. The current known way to verify for true no-std support is to compile the crate with the target wasm32-unknown-unknown.

This is also the PR that enables the CI checks to ensure that no-std continues to be supported in the future.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

E: no-std
opened by soareschen 22

Packets stuck due to overflowing Timeout

Crate

hermes relayer

Summary of Bug

Pending packets are stucked, can't be relayed

[bitcanna-1:transfer/channel-0 -> osmosis-1] packets that still have commitments on bitcanna-1: 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 (first 10 shown here; total=53)
Oct 08 13:56:25.293 DEBUG ThreadId(01) [bitcanna-1:transfer/channel-0 -> osmosis-1] recv packets to send out to osmosis-1 of the ones with commitments on source bitcanna-1: 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 (first 10 shown here; total=52)
The application panicked (crashed).
Message:  called `Result::unwrap()` on an `Err` value: 
   0: error converting from u64 to i64
   1: out of range integral type conversion attempted

Location:
   /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs:31

Backtrace omitted.
Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
Location: modules/src/ics04_channel/events.rs:139

Another logs:

ct 08 14:03:04 BCNA-SentryNode2 hermes[533309]: Oct 08 14:03:04.625  INFO ThreadId(46) [osmosis-1:transfer/channel-41 -> bitcanna-1] clearing pending packets height=Some(Height { revision: 1, height: 1500899 })
Oct 08 14:03:04 BCNA-SentryNode2 hermes[533309]: Oct 08 14:03:04.638 ERROR ThreadId(46) [osmosis-1:transfer/channel-41 -> bitcanna-1] worker: handling command encountered error:
Oct 08 14:03:04 BCNA-SentryNode2 hermes[533309]:    0: clearing of old packets failed

This error is in bucle in the relayer log:

Oct 08 13:09:22 BCNA-SentryNode2 hermes[533309]: Oct 08 13:09:22.894 ERROR ThreadId(46) [osmosis-1:transfer/channel-41 -> bitcanna-1] worker: handling command encountered error:
Oct 08 13:09:22 BCNA-SentryNode2 hermes[533309]:    0: clearing of old packets failed
Oct 08 13:09:22 BCNA-SentryNode2 hermes[533309]: Location:
Oct 08 13:09:22 BCNA-SentryNode2 hermes[533309]:    /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs:10
Oct 08 13:09:22 BCNA-SentryNode2 hermes[533309]: Backtrace omitted.

hermes 0.7.3+5d53e52 (binary download). chain SDK v.0.44.1 / tendermint v.0.34.13

Steps to Reproduce

The stacked packets are there.. stacked in the Osmosis chain. You can see at logs. To get the panic error:

hermes tx raw packet-recv  osmosis-1 bitcanna-1 transfer channel-0
Oct 08 21:56:52.931  INFO ThreadId(01) using default configuration from '/home/raul/.hermes/config.toml'
Oct 08 21:56:52.934 TRACE ThreadId(01) [bitcanna-1] subscribing to query: tm.event = 'Tx'
Oct 08 21:56:52.935 TRACE ThreadId(01) [bitcanna-1] subscribing to query: tm.event = 'NewBlock'
Oct 08 21:56:52.935 TRACE ThreadId(01) [bitcanna-1] subscribed to all queries
Oct 08 21:56:52.936 DEBUG ThreadId(07) [bitcanna-1] starting event monitor
Oct 08 21:56:52.938 TRACE ThreadId(01) [osmosis-1] subscribing to query: tm.event = 'Tx'
Oct 08 21:56:52.939 TRACE ThreadId(01) [osmosis-1] subscribing to query: tm.event = 'NewBlock'
Oct 08 21:56:52.940 TRACE ThreadId(01) [osmosis-1] subscribed to all queries
Oct 08 21:56:52.940 DEBUG ThreadId(14) [osmosis-1] starting event monitor
Oct 08 21:56:53.019 DEBUG ThreadId(01) [bitcanna-1:transfer/channel-0 -> osmosis-1] packets that still have commitments on bitcanna-1: 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 (first 10 shown here; total=54)
Oct 08 21:56:53.019 DEBUG ThreadId(01) [bitcanna-1:transfer/channel-0 -> osmosis-1] recv packets to send out to osmosis-1 of the ones with commitments on source bitcanna-1: 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 (first 10 shown here; total=54)
The application panicked (crashed).
Message:  called `Result::unwrap()` on an `Err` value: 
   0: error converting from u64 to i64
   1: out of range integral type conversion attempted

Location:
   /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs:31

Backtrace omitted.
Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
Location: modules/src/ics04_channel/events.rs:139

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
Error: 
   0: link error
   1: failed with underlying error
   2: failed to receive through channel
   3: receiving on an empty and disconnected channel

Location:
   /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/flex-error-0.4.3/src/tracer_impl/eyre.rs:31

Backtrace omitted.
Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Info of the client/connection/channel/path:

CHAIN      CLIENT    		CONNECTION        CHANNEL
bitcanna-1 07-tendermint-0   	connection-0      channel-0
osmosis-1  07-tendermint-1456  	connection-1141   channel-41

The healthy check:

hermes health-check 
Oct 08 22:02:56.931  INFO ThreadId(01) using default configuration from '/home/raul/.hermes/config.toml'
Oct 08 22:02:56.933  INFO ThreadId(01) [bitcanna-1] performing health check...
Oct 08 22:02:56.958  WARN ThreadId(01) Health checkup for chain 'bitcanna-1' failed
Oct 08 22:02:56.958  WARN ThreadId(01)     Reason: Hermes health check failed while verifying the application compatibility for chain bitcanna-1:http://localhost:9093/; caused by: SDK module at version '0.44.1' does not meet compatibility requirements >=0.41.3, <=0.44.0 for application bcnad:1.1-0740c191bf290b9a6a69976c4a90520e6d4fb7ce
Oct 08 22:02:56.958  WARN ThreadId(01)     Some Hermes features may not work in this mode!
Oct 08 22:02:56.958 ERROR ThreadId(01) [bitcanna-1] chain is unhealthy
Oct 08 22:02:56.960  INFO ThreadId(01) [osmosis-1] performing health check...
Oct 08 22:02:56.982  INFO ThreadId(01) [osmosis-1] chain is healthy

Connection:

bcnad query ibc  connection  connections
connections:
- client_id: 07-tendermint-0
  counterparty:
    client_id: 07-tendermint-1456
    connection_id: connection-1141
    prefix:
      key_prefix: aWJj
  delay_period: "0"
  id: connection-0
  state: STATE_OPEN
  versions:
  - features:
    - ORDER_ORDERED
    - ORDER_UNORDERED
    identifier: "1"
height:
  revision_height: "128344"
  revision_number: "1"
pagination:
  next_key: null
  total: "0"

unreceived-acks

bcnad query ibc channel unreceived-acks transfer channel-0 --sequences=11,12,13,30,54,65
height:
  revision_height: "128392"
  revision_number: "1"
sequences:
- "11"
- "12"
- "13"
- "30"
- "54"
- "65"

Acceptance Criteria

The relaying of the packets

For Admin Use

[ ] Not duplicate issue
[ ] Appropriate labels applied
[ ] Appropriate milestone (priority) applied
[ ] Appropriate contributors tagged
[ ] Contributor assigned/self-assigned

A: bug I: logic O: code-hygiene O: usability

opened by RaulBernal 21

Hermes always uses `max_gas` for transactions
Crate

Hermes ibc_relayer v0.7.1

Summary of Bug

Hermes transaction always use max_gas amount.

https://www.mintscan.io/osmosis/txs/E742F9EFE2E97A5FCD71FDB3CCDF2FF02BEE47DE5B2D0A51110805736F67C0E4

Version

v0.7.1

Acceptance Criteria

Hermes use automatic gas adjustment and gas_adjustment from config

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

A: question I: logic O: usability
opened by Galadrin 21
Hermes ports not getting detected
Crate

ibc-rs (Hermes)

Summary of Bug

Creating channels between Band Protocol and another chain from it's oracle port for fetching prices, it throws could not resolve channel and port is invalid.

Version

hermes - release - v0.11.1

Steps to Reproduce

ibc-0 = Band Protocol (band-laozi-testnet4) ibc-1 = Any chain with oracle port*

* = https://github.com/comdex-official/comdex/tree/oracle-liquidation (This binary and branch contains bandoracle port for fetching prices)

hermes create channel test-1 band-laozi-testnet4 --port-a bandoracle --port-b oracle

Logs:

WARN ThreadId(01) cannot resolve channel version for unknown port id 'oracle' Error: channel error: could not resolve channel version because the port is invalid: oracle

Acceptance Criteria

Port from Band or Comdex getting detected for channel creation.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned
opened by dhogaivannan 20
Support for priority mempool chains
Summary

Starting with tm v0.34.20 we may see cosmos-sdk chains using priority enabled mempool. We need to make sure we support these chains.

Problem Definition

Here is my understanding about how prio mempool will be used from hermes pov.

Let’s say hermes submits 2 Tx-es, tx1 and tx2, with incrementing nonces, using broadcast_tx_sync. The application runs checkTx for each of them and returns an abci.ResponseCheckTx that includes a priority. The prio mempool then stores this and when it’s time to create a block it picks Tx-es in priority order. In sdk chains, during checkTx, the priority is set by a TxFeeChecker and the default is defined in the auth module: https://github.com/cosmos/cosmos-sdk/blob/main/x/auth/ante/validator_tx_fee.go.

In this case the priority is computed based on the Tx fee. Different apps/chains will have the ability to overwrite this handler. But staying with the default behavior for now, it is possible that tx2 will be included in a block before tx1, so deliverTx(tx2) will be executed first and will fail with account sequence mismatch. tx1 will be successful.

If we try to use multiple wallets to go around the account sequence mismatch we will still see issues, e.g.:

tx2 may be rejected if tx1 includes a client update needed for msgs in tx2, or

the msgs in the tx-es are for ordered channels.

While we push for a solution in tendermint or SDK (e.g. to not reorder Tx-es from same account or allow out of order execution during deliverTx), we should remove our assumption that Tx-es are executed in order, figure out all the cases where we are exposed to this and how to handle them.
For the examples above, include update client in every tx (see also #2191), don’t send multiple Tx-es for ordered channels in same block (i.e. wait for tx1 hash to show up in a block before sending tx2), etc.

Acceptance Criteria

hermes works with prio mempool chains

[x] depends on #2455

[x] update compatibility.rs to mark compat w/ sdk 0.46

https://github.com/informalsystems/ibc-rs/blob/5df35478607452baa8db004efb1d7aa38dfd8d89/relayer/src/chain/cosmos/compatibility.rs#L8-L23

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

A: P-low
opened by ancazamfir 19
Replace positional arguments with flags in CLI
Closes: #2239

Description

This PR adds flags instead of positional arguments to all CLI commands. Long flags have been used in order to avoid conflicts or confusing flags.

WIP

At commit 51852ae51b91471e24f7b4b57116749b2d24d39c: Some short flags are still present for some commands, but a discussion in order to clarify if all the short flags will be removed or if some strategic short flags will be kept is ongoing.

At commit 88474db7e7a2957f9ab0f3b5273f82ec3b2ee544 and a76599fa918e7fc6f55235f1b70a782ebc274b8c: Removed all short flags.

PR author checklist:

[x] Added changelog entry, using unclog.

[ ] ~Added tests: integration (for Hermes) or unit/mock tests (for modules).~

[x] Linked to GitHub issue.

[x] Updated guide (e.g., guide/).

Reviewer checklist:

[ ] Reviewed Files changed in the GitHub PR explorer.

[x] Manually tested (in case integration/unit/mock tests are absent).
opened by ljoss17 19
Sequence number caching in Hermes is unreliable
Crate

Hermes

Summary

Query the state for every chain in config every X sec for the relayer account's seq number, and update cached seq number with it. Make X a configurable param.

Problem Definition

At some point (around height 1008 on Cosmos hub), for some reason, the cached seq number on our relayer instance got incremented, but the actual on-chain seq number wasn't. From there, our relayer hasn't been able to relay, producing the following error:

Aug 06 15:51:41.681 INFO [irishub-1:transfer/channel-12 -> cosmoshub-4] result events: ChainErrorEv(deliver_tx on chain cosmoshub-4 for Tx hash EC50AD139769F2585F351CDE69636B0C82F522A5B29ADDD1C279737088F4E098 reports error: code=Err(32), log=Log("account sequence mismatch, expected 1008, got 1358: incorrect account sequence"))

While we don't know what caused this mismatch to happen, we couldn't detect it and thought our relayer was running properly. It would be good to figure out why the mismatch happened, but it would also be good to have a failsafe that updates the cached seq number with the actual on-chain seq number periodically so that the relayer can recover on its own.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

A: bug I: logic E: gravity
opened by gamarin2 18
Use `#![no_std]` for ibc-proto crate
Partially Closes: informalsystems/hermes#1158

Description

This is a follow up on @romac's comment in informalsystems/hermes#1156

c) Were the changes in the ibc-proto crate done manually? If so, can we automate that somehow? eg. via a prost flag/feature or within the proto-compiler crate as a post-processing step?

I just realized ibc-proto crate do not use much of std features, and we can enable #![no_std] for the crate rather easily without modifying the generated code. This PR does the fix separately from informalsystems/hermes#1156.

One thing I notice: even though the ibc-proto crate successfully compiles with #![no_std], it is in fact not fully no-std-compatible as at least one of its transitive dependencies, socket2, does not work in no_std. I could not find easy way to verify that other than building ibc-proto with the target wasm32-unknown-unknown:

$ cargo build --target wasm32-unknown-unknown error[E0432]: unresolved import `crate::sys` --> /home/ubuntu/.cargo/registry/src/github.com-1ecc6299db9ec823/socket2-0.4.0/src/sockaddr.rs:5:12 | 5 | use crate::sys::{ | ^^^ could not find `sys` in the crate root ...

The crate socket2 is imported via hyper, which is used by tonic. When inspecting the tonic and socket2 crate, I also find various uses of std::io and std::net, which are not available in no_std.

This shows that supporting no_std is more tricky than just enabling the #![no_std] flag in our own crate. The no_std flag does not check on the compliance on dependencies, and I cannot find a good way to check on that other than building with the target wasm32-unknown-unknown. Other than tonic, it is also not clear how many other dependencies we have are unsupported in the real no_std environment.

@DaviRain-Su do you have any experience in how to verify if a crate truly support no-std environment?

For contributor use:

[ ] Updated the Unreleased section of CHANGELOG.md with the issue.

[ ] If applicable: Unit tests written, added test to CI.

[ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.

[ ] Updated relevant documentation (docs/) and code comments.

[ ] Re-reviewed Files changed in the Github PR explorer.

E: no-std
opened by soareschen 18
Repo split
Summary

It has been proposed that we split the ibc-rs monorepo into two repos - one for the proto & module crates and the other for the relayer crate (and its associated crates). Additionally, the relayer code must not depend on the ibc crate to have clear boundaries/separation and allow the modules to evolve independently.

Problem Statement

The monorepo approach is restrictive and becoming difficult to maintain. (See https://github.com/informalsystems/ibc-rs/discussions/1437#discussioncomment-1458578) Moreover, having the relayer crate depend on the ibc crate makes it difficult to modify one without touching the other, and have resulted in code bloat and unclear boundaries - for e.g. the modules code contains relayer specific code.

Proposal

Using the ideas described in https://github.com/informalsystems/ibc-rs/issues/2541 and https://github.com/informalsystems/ibc-rs/discussions/1437, the repo hosting the relayer crates (the relayer repo) would look like so ->

<relayer> ├── CHANGELOG.md ├── CONTRIBUTING.md ├── Cargo.lock ├── Cargo.toml │ ├── crates │ ├── ibc-relayer │ ├── ibc-relayer-cli │ ├── ibc-relayer-rest │ ├── ibc-telemetry │ └── ibc-chain-registry │ ├── etc...

And the modules repo would look like so ->

<modules> ├── CHANGELOG.md ├── CONTRIBUTING.md ├── Cargo.toml │ ├── crates │ ├── ibc │ └── proto │ ├── ibc-proto │ └── ibc-proto-compiler │ ├── etc...

Acceptance Criteria

The ibc-rs repo is split into two repos as proposed above, ideally preserving history.

TODOs

[ ] Rename the default branches of both repos from master to main

[ ] Split gm out of the repository

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

O: code-hygiene
opened by hu55a1n1 16
Backport changes to the current relayer in relayer-next back to main branch

#2478 introduces a number of minor changes to the current relayer code base at crates/relayer/src and crates/relayer-types/src. As the development is ongoing, changes in these places frequently introduce merge conflicts that can be tedious to resolve.

To minimize conflicts, we should backport the changes back to the master branch in one or more PRs separately from the relayer-next changes.
O: relayer-next

opened by soareschen 0
Move message building methods from relay context to chain context

#2960 introduces the IncomingPacket and OutgoingPacket type to allow the chain context to access IBC packets. Inside the PR, only the build_receive_packet_message method is moved from the relay context to the chain context to demonstrate how this simplifies the implementation of the relay context.

The same pattern can be applied to build_ack_packet_message and build_timeout_unordered_packet_message so that they become methods for the chain context. The refactoring should be simple, and it is a good exercise to understand how IBC messages are built in the new relayer.
O: relayer-next I: refactoring

opened by soareschen 0
Query counterparty chain id instead of requiring user to pass it via CLI when making a client update

Currently, when making a client update manually, Hermes will source the chain-id from the command line.

When Hermes makes the client update, it should pull the chain-id from the counterparty chain itself, because the chain-id can at times change.

This way, it's much harder for a user to make a client update that doesn't correspond to the correct chain-id.
I: cli O: usability A: P-low

opened by faddat 1
A proposal to streamline workflow with organized GitHub tags
Overview

This initiative aims to give a systematic approach to defining and using Github tags. It is developed with a look at maximizing coverage, attaching more purposeful information, and organizing issues and pull requests in a simple and effective way.

Description

The proposal originated from the idea of grouping tags into a few categories so that whenever an issue or pull request is opened, a collaborator can simply ask some simple questions to assign the right tags. He or she should answer Why (the purpose of fulfilling the issue/PR), Which (the part of the product/project it relates to), and How (The way it should be organized to perform).

To make it easier to identify and distinguish tags, they have been split into four categories: Objective-level, External-level, Internal-level, and Administrative-level. Each category has a designated color range and begins with the first letter of the group. This allows individuals to easily search, attach or review tags when necessary.

Before providing more information, it's important to note that each group has been further divided into two sub-categories for the sake of clarity, having more context, to make it easier to define new tags. A tag can be created from any perspective and does not have to come from these sub-categories. Here are more details about each group(You can also navigate to this diagram):

Objective-level tags

Objectives can be broadly categorized into two groups: functional and quality (non-functional). These objectives consist of tags that identify which aspects of the product's quality would be improved or which functionalities or features will be added when addressing an issue. Considering the objective standpoint makes it easier to determine how to prioritize and allocate resources.

Internal-level tags

The internal level provides an inside view and focuses on anything related to the current scope of the product. These tags identify the relevant units/modules or processes within that product.

External-level tags

This level of tags encompasses anything that is connected to external projects, products, or businesses and aims to address user requirements that fall outside the current scope of the product.

Administrative-level tags

This level tracks administrative aspects such as status, priority, complexity, and anything related to time and resource management. This includes:

Tracking and workflow-related issues

Priority (urgent vs. non-urgent)

Importance (trivial vs. critical)

Complexity (to help allocate development resources based on expertise)

Status (including blocked, breaking, etc.)

This level also offers a wider range of colors to provide more options for defining tags.

Wrapping up

Given the above descriptions, members can search for the appropriate tags by asking themselves the following questions and looking for the corresponding tag that begins with the first letter of its group:

What is the purpose (Why)? It is implicitly known by selecting which quality/functionality would be impacted after resolving it.

Is this issue related to the internal or external scope of the product/project? (Which)

To which unit or party of that boundary does this issue pertain? (Which)

Should it be marked as a specific task, so as to adjust workflow, plan, or resource allocations? (How)

It can be viewed as a flexible repository-wide system that can be used as a basis for defining tags for any internal project. Having the categories can help to organize a large number of tags under a few classifications. Though, this might not completely cover every angle, making it easier for collaborators to find the fit tags, and requiring less maintenance from admins.

Furthermore, if multiple projects adopt the same approach while using their own tags, other teammates will be able to understand issues agreeably and cross-team communication will be more practical, particularly from objective and administrative standpoints.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

A: admin
opened by Farhad-Shabani 5

Client update error while forking from sovereign to consumer chain

Summary of Bug

I am trying to setup relayer using hermes v0.15.0 between two Cosmos chains for interchain-security provider chain and consumer chain after soft fork from sovereign to consumer chain. To setup minimal version, locally, installed one node provider chain, 2 nodes consumer chain. And when setting up connection this issue persists.

An action done on the first block after soft fork is validator set update to provider chain's one. And this hermes operation started after waiting 10s executing the validator set update block.

ERROR log on hermes

2022-12-28T16:59:35.808947Z ERROR ThreadId(89) send_tx_commit{id=ConnectionOpenTry}:send_tx_with_account_sequence_retry{id=provider}:estimate_gas: failed to simulate tx. propagating error to caller: gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 0: cannot update client with ID 07-tendermint-0: trusted validators validators:<address:\"\\355D(\\260\\363S\\246\\014b`\\375\\252\\347H6\\006\\204\\002\\234Z\" pub_key:<ed25519:\"`V\\256\\241\\365D\\376\\306\\277\\227\\271`\\246\\201!\\347\\227v\\311\\022\\350c^\\270\\\\@\\254F\\220\\273\\374F\" > voting_power:10000 > proposer:<address:\"\\355D(\\260\\363S\\246\\014b`\\375\\252\\347H6\\006\\204\\002\\234Z\" pub_key:<ed25519:\"`V\\256\\241\\365D\\376\\306\\277\\227\\271`\\246\\201!\\347\\227v\\311\\022\\350c^\\270\\\\@\\254F\\220\\273\\374F\" > voting_power:10000 > total_voting_power:10000 , does not hash to latest trusted validators. Expected: B6CFE4517DBE86988EE1F71C11F48BA275B5170FF08514DD0AA078FFEBC44DAC, got: B02F61DF3987DDB895F5608E6519A18887E831F4BDB83C1C2D1FDA88AFAB15CB: invalid validator set [informalsystems/ibc-go/[email protected]/modules/light-clients/07-tendermint/types/update.go:156] With gas wanted: '0' and gas used: '100724' ", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "31"} }
2022-12-28T16:59:35.809033Z ERROR ThreadId(01) failed ConnTry ConnectionSide { chain: BaseChainHandle { chain_id: ChainId { id: "provider", version: 0 }, runtime_sender: Sender { .. } }, client_id: ClientId("07-tendermint-0"), connection_id: None }: failed during a transaction submission step to chain 'provider': gRPC call failed with status: status: Unknown, message: "failed to execute message; message index: 0: cannot update client with ID 07-tendermint-0: trusted validators validators:<address:\"\\355D(\\260\\363S\\246\\014b`\\375\\252\\347H6\\006\\204\\002\\234Z\" pub_key:<ed25519:\"`V\\256\\241\\365D\\376\\306\\277\\227\\271`\\246\\201!\\347\\227v\\311\\022\\350c^\\270\\\\@\\254F\\220\\273\\374F\" > voting_power:10000 > proposer:<address:\"\\355D(\\260\\363S\\246\\014b`\\375\\252\\347H6\\006\\204\\002\\234Z\" pub_key:<ed25519:\"`V\\256\\241\\365D\\376\\306\\277\\227\\271`\\246\\201!\\347\\227v\\311\\022\\350c^\\270\\\\@\\254F\\220\\273\\374F\" > voting_power:10000 > total_voting_power:10000 , does not hash to latest trusted validators. Expected: B6CFE4517DBE86988EE1F71C11F48BA275B5170FF08514DD0AA078FFEBC44DAC, got: B02F61DF3987DDB895F5608E6519A18887E831F4BDB83C1C2D1FDA88AFAB15CB: invalid validator set [informalsystems/ibc-go/[email protected]/modules/light-clients/07-tendermint/types/update.go:156] With gas wanted: '0' and gas used: '100724' ", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "31"} }
Error: failed due to missing counterparty connection id

Version

$ hermes version
hermes 0.15.0

Steps to Reproduce

The error can be reproduced here by running follow commands https://github.com/Stride-Labs/interchain-security/pull/1

rm -rf $HOME/.provider1
rm -rf $HOME/.provider
rm -rf $HOME/.consumer1
rm -rf $HOME/.consumer
rm -rf $HOME/.sovereign
sh run.sh

Acceptance Criteria

Find the solution to bypass this issue with current version
Figure out the reason of the issue

For Admin Use

[ ] Not duplicate issue
[ ] Appropriate labels applied
[ ] Appropriate milestone (priority) applied
[ ] Appropriate contributors tagged
[ ] Contributor assigned/self-assigned

opened by jstr1121 4

Clarify the purpose of `ibc-relayer-types` crate compared to `cosmos/ibc-rs`
Summary

Developers who want to build their own relayer may be confused about which crate to use when it comes to importing IBC types. Remarkably, there is not enough context around the core purpose of the ibc-relayer-types crate.

Problem Definition

ibc-relayer-types and cosmos/ibc-rs crates have many common functionality and abstractions but with different roles. The role of ibc-relayer-types should be clearly presented from the client's perspective and be introduced as a toolkit for relayer development, with the latter focusing on on-chain operations. It became necessary when you look at it from developers' perspective, specifically those working on non-Cosmos chains. The ibc-relayer-types is the only crate they have to use among the Hermes crates. It’s needed since it enables them to interact with blockchains, while the others are mainly related to relaying logic and could be implemented by them internally.

Proposal

Though the reason behind having ibc-relayer-types and cosmos/ibc-rs crates in parallel can be found here, the purpose of each should be clearly explained in the Hermes root README and the README of ibc-relayer-types.Both mentioned repos use the same readme descriptions at the moment.

For Admin Use

[ ] Not duplicate issue

[ ] Appropriate labels applied

[ ] Appropriate milestone (priority) applied

[ ] Appropriate contributors tagged

[ ] Contributor assigned/self-assigned

I: documentation
opened by Farhad-Shabani 0