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.

Sign up for GitHub

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

Jump to bottom

WIP: Upgrade to CometBFT-rs instead of Tendermint-rs #3832

Closed
wants to merge 3 commits into from
Closed

WIP: Upgrade to CometBFT-rs instead of Tendermint-rs #3832

wants to merge 3 commits into from

Conversation

adizere
Copy link
Member

@adizere adizere commented Feb 2, 2024

Closes: #XXX

Description

Can be reviewed commit by commit, though most changes are mechanical (find & replace).

  • 2c97673 - adapted chain-registry crate
  • 129a2c8 - adapted telemetry crate
  • 100364b - adapted ibc-relayer-types crate
    • work in progress
    • multiple modules depends transitively on tendermint-rs via ibc_proto; we need to upgrade ibc_proto to use cometbft-rs
  • adapt ibc-relayer
  • adapt ibc-relayer-cli
  • adapt ibc-relayer-rest

PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@adizere
Adapted chain-registry crate
2c97673
@adizere
Adapted the telemetry crate.
129a2c8 
I had to add a `features = ["clock"]` to the Cargo.toml
which was not needed before. From what I can tell nothing
changed in cometbft compared to tendermint crate features.

One potential explanation is due to transitive dependencies.
Crate ibc-telemetry depends on ibc-relayer-types which
imports tendermint using `features = ["clock"]` and maybe
this makes the feature available transivitely in telemetry.

To investigate cc @mzabaluev
@adizere
Adapted ibc-relayer-types to use CometBFT
100364b 
Multiple errors due to transitive dependencies on tendermint-rs
via ibc-proto crate. Need to upgrade ibc-proto also.
@mzabaluev
Copy link
Contributor

Re 129a2c8:

Yes, the clock feature is needed for Time::now, and it used to be added transitively via dependencies.

That said, you don't need cometbft::Time here, as you can just use std::time::SystemTime for the same purpose.

@mzabaluev mzabaluev mentioned this pull request Mar 4, 2024
Draft
7 tasks
@mzabaluev
Copy link
Contributor

I have taken this further in #3865.

@adizere adizere closed this Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adizere @mzabaluev