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

Update to tendermint 0.26 #208

Merged
merged 14 commits into from
Nov 16, 2022
Merged

Update to tendermint 0.26 #208

merged 14 commits into from
Nov 16, 2022

Conversation

mzabaluev
Copy link
Contributor

@mzabaluev mzabaluev commented Nov 1, 2022

Description

Updates for tendermint-rs 0.26

Requres changes from cosmos/ibc-proto-rs#31


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests.
  • 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).

Comment on lines +48 to 52
impl From<ClientTypeAttribute> for abci::EventAttribute {
fn from(attr: ClientTypeAttribute) -> Self {
Tag {
key: CLIENT_TYPE_ATTRIBUTE_KEY.parse().unwrap(),
value: attr.client_type.to_string().parse().unwrap(),
}
(CLIENT_TYPE_ATTRIBUTE_KEY, attr.client_type.as_str()).into()
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that the EventAttribute domain type also has an index field, which is set to false by the From conversion used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, it may have been too early to switch to tendermint::abci::Event before informalsystems/tendermint-rs#1204 is released. There is still tendermint_rpc::abci::Event confusingly, but it's what hermes is using.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upon further examination, the ABCI event infrastructure in Hermes now duplicates this code and is not dependent on ibc. So we can go on with this change.

Option::map_or requires the alternative value to be non-lazily
constructed, so we save some allocations.
Cargo.toml Outdated Show resolved Hide resolved
@mzabaluev mzabaluev marked this pull request as ready for review November 9, 2022 17:37
Ramify the name as to_event_attribute_value for clarity.
@mzabaluev mzabaluev marked this pull request as draft November 9, 2022 19:34
tendermint-rs is now committed to fully following Rust semver
conventions.
@mzabaluev mzabaluev marked this pull request as ready for review November 10, 2022 16:36
Copy link
Contributor

@hu55a1n1 hu55a1n1 left a comment

Choose a reason for hiding this comment

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

Love the changes in this PR! 👌 Added some minor suggestions and a question about tendermint-rs dep versions.

crates/ibc/src/core/ics03_connection/events.rs Outdated Show resolved Hide resolved
crates/ibc/src/core/ics24_host/identifier.rs Outdated Show resolved Hide resolved
crates/ibc/Cargo.toml Show resolved Hide resolved
@hu55a1n1 hu55a1n1 merged commit 69babe1 into main Nov 16, 2022
@hu55a1n1 hu55a1n1 deleted the mikhail/tendermint-0.26 branch November 16, 2022 08:08
@hdevalence hdevalence mentioned this pull request Nov 21, 2022
3 tasks
Farhad-Shabani pushed a commit that referenced this pull request Sep 9, 2024
* Update to tendermint 0.26

* Fix no-std-check with a temporary patch

* Post-merge updates for tendermint 0.26

* ics04_channel::events: Use Option::map_or_else

Option::map_or requires the alternative value to be non-lazily
constructed, so we save some allocations.

* Update ibc-proto to 0.22.0

* Rename TimeoutHeight::to_attribute_value

Ramify the name as to_event_attribute_value for clarity.

* Unpin tendermint version

tendermint-rs is now committed to fully following Rust semver
conventions.

* Convert ClientIdAttribute like the rest of them

* Changelog entry for #208

* connection events: remove an outdated comment

* Derive derive_more::Into for ClientId
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants