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

feat: implement 08-wasm client types #1031

Merged
merged 15 commits into from
Jan 12, 2024

Conversation

Farhad-Shabani
Copy link
Member

@Farhad-Shabani Farhad-Shabani commented Jan 5, 2024

Closes: #1030

Related PR for adding required proto types: cosmos/ibc-proto-rs#170

Notes

  • In places where fields containing bytes of Vec<u8>, a Byte type alias has been defined, allowing for better encapsulation and inclusion of potential implementation of various checks in the future, such as length limitations or etc.

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

deps: bump ibc-proto rev
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (8a552f7) 71.21% compared to head (7baa0ba) 71.45%.
Report is 2 commits behind head on main.

Files Patch % Lines
ibc-clients/ics08-wasm/types/src/error.rs 0.00% 9 Missing ⚠️
...bc-clients/ics08-wasm/types/src/consensus_state.rs 74.19% 8 Missing ⚠️
ibc-clients/ics08-wasm/types/src/client_state.rs 89.36% 5 Missing ⚠️
ibc-clients/ics08-wasm/types/src/lib.rs 57.14% 3 Missing ⚠️
ibc-clients/ics08-wasm/types/src/client_message.rs 77.77% 2 Missing ⚠️
...ents/ics08-wasm/types/src/msgs/migrate_contract.rs 94.11% 1 Missing ⚠️
...ients/ics08-wasm/types/src/msgs/remove_checksum.rs 92.85% 1 Missing ⚠️
...bc-clients/ics08-wasm/types/src/msgs/store_code.rs 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1031      +/-   ##
==========================================
+ Coverage   71.21%   71.45%   +0.24%     
==========================================
  Files         178      187       +9     
  Lines       18198    18516     +318     
==========================================
+ Hits        12959    13230     +271     
- Misses       5239     5286      +47     

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

@Farhad-Shabani Farhad-Shabani marked this pull request as ready for review January 8, 2024 14:35
@Farhad-Shabani Farhad-Shabani requested a review from rnbguy January 8, 2024 14:41
Copy link
Collaborator

@rnbguy rnbguy left a comment

Choose a reason for hiding this comment

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

Thanks @Farhad-Shabani for taking care of this. 🙏

I requested some changes. The most important ones are the unit tests.

ibc-clients/Cargo.toml Show resolved Hide resolved
ibc-clients/src/lib.rs Show resolved Hide resolved
ibc-clients/ics08-wasm/types/src/error.rs Show resolved Hide resolved
ibc-clients/ics08-wasm/types/src/msgs/store_code.rs Outdated Show resolved Hide resolved
ibc-clients/ics08-wasm/types/src/msgs/remove_checksum.rs Outdated Show resolved Hide resolved
ibc-clients/ics08-wasm/types/Cargo.toml Show resolved Hide resolved
Copy link
Collaborator

@rnbguy rnbguy left a comment

Choose a reason for hiding this comment

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

Great work @Farhad-Shabani ! 👌 I am approving the PR with a question about bytes dependency.

I also added some new tests. If you approve of them, go ahead with merging.

ibc-clients/ics08-wasm/types/src/msgs/store_code.rs Outdated Show resolved Hide resolved
ibc-clients/Cargo.toml Show resolved Hide resolved
ibc-clients/ics08-wasm/types/src/lib.rs Show resolved Hide resolved
ibc-clients/ics08-wasm/types/src/lib.rs Show resolved Hide resolved
ibc-clients/ics08-wasm/types/Cargo.toml Outdated Show resolved Hide resolved
@Farhad-Shabani
Copy link
Member Author

Thank you @rnbguy for the tests. 👌🏻

@Farhad-Shabani Farhad-Shabani added this to the 0.50.0 milestone Jan 12, 2024
@Farhad-Shabani Farhad-Shabani merged commit d5e8314 into main Jan 12, 2024
16 checks passed
@Farhad-Shabani Farhad-Shabani deleted the farhad/1030-add-wasm-client-types branch January 12, 2024 02:47
Farhad-Shabani added a commit that referenced this pull request Sep 9, 2024
* feat: implement 08-wasm client types

deps: bump ibc-proto rev

* fix: no_std compatibility

* imp: add all of the Wasm msg types

* chore: update Cargo.lock

* chore: use ibc-proto-rs rev which contains sovereign protos

* nit: correct ibc-proto-rs rev

* chore: apply review comments

* fix: use From instead of TryFrom where Error = Infallible

* add WASM_CLIENT_TYPE

* add tests for Base64

* add tests for ClientType

* add tests in wasm client type crate

* use bytes over strings

* nit: remove bytes

---------

Co-authored-by: Ranadeep Biswas <mail@rnbguy.at>
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.

Implement ICS-08 Wasm light client domain types
2 participants