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

imp(ics24)!: enhancements and fixes to ChainId impl and validation #762

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

Farhad-Shabani
Copy link
Member

@Farhad-Shabani Farhad-Shabani commented Jul 13, 2023

Closes: #761


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).

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Patch coverage: 94.30% and project coverage change: -2.09% ⚠️

Comparison is base (5784770) 73.41% compared to head (963b7be) 71.32%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #762      +/-   ##
==========================================
- Coverage   73.41%   71.32%   -2.09%     
==========================================
  Files         125      124       -1     
  Lines       16127    14796    -1331     
==========================================
- Hits        11839    10553    -1286     
+ Misses       4288     4243      -45     
Files Changed Coverage Δ
...ibc/src/core/ics02_client/handler/update_client.rs 96.15% <ø> (ø)
...s/ibc/src/hosts/tendermint/validate_self_client.rs 0.00% <0.00%> (ø)
crates/ibc/src/mock/ics18_relayer/context.rs 65.00% <ø> (ø)
crates/ibc/src/clients/ics07_tendermint/error.rs 29.16% <75.00%> (+9.16%) ⬆️
crates/ibc/src/core/ics24_host/identifier.rs 67.39% <92.75%> (+9.42%) ⬆️
...s/ibc/src/clients/ics07_tendermint/client_state.rs 72.27% <100.00%> (-0.57%) ⬇️
crates/ibc/src/clients/ics07_tendermint/header.rs 71.56% <100.00%> (+0.27%) ⬆️
...s/ibc/src/clients/ics07_tendermint/misbehaviour.rs 68.08% <100.00%> (-0.61%) ⬇️
...src/core/ics03_connection/handler/conn_open_ack.rs 96.62% <100.00%> (ø)
...src/core/ics03_connection/handler/conn_open_try.rs 94.02% <100.00%> (ø)
... and 3 more

... and 14 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Farhad-Shabani Farhad-Shabani changed the title imp(ics24)!: refactor ChainId to create and validate them more precisely and efficiently imp(ics24)!: refactor ChainId to construct and validate more precisely and efficiently Jul 13, 2023
@Farhad-Shabani Farhad-Shabani changed the title imp(ics24)!: refactor ChainId to construct and validate more precisely and efficiently imp(ics24)!: enhancements and fixes to ChainId impl and validation Jul 28, 2023
@Farhad-Shabani Farhad-Shabani marked this pull request as ready for review July 28, 2023 14:49
Copy link
Contributor

@plafer plafer left a comment

Choose a reason for hiding this comment

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

LGTM! Let's run the integration tests before we merge though

@Farhad-Shabani Farhad-Shabani merged commit 01e3ab8 into main Jul 28, 2023
@Farhad-Shabani Farhad-Shabani deleted the farhad/refactor-chain-id branch July 28, 2023 18:37
dzmitry-lahoda pushed a commit to dzmitry-lahoda-forks/ibc-rs that referenced this pull request Aug 1, 2023
…osmos#762)

* imp(ChainId): refactor abstraction and validations

* misc: add unclog

* fix: clippy catches

* fix: revert ChainId definition + improve checks

* fix: set_revision_number
Farhad-Shabani added a commit that referenced this pull request Sep 9, 2024
…762)

* imp(ChainId): refactor abstraction and validations

* misc: add unclog

* fix: clippy catches

* fix: revert ChainId definition + improve checks

* fix: set_revision_number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[ICS24] Enhancements and fixes for ChainId handling and validation
2 participants