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

ICS24 Path variants as types #1760

Merged
merged 7 commits into from
Jan 17, 2022
Merged

Conversation

hu55a1n1
Copy link
Member

@hu55a1n1 hu55a1n1 commented Jan 12, 2022

Partially addresses: cosmos/ibc-rs#25

Description

ics24_host::Path variants as types - Since Rust doesn't treat enum variants as types, it might be a good idea to extract these variants as distinct types and use those types as the actual enum variants. This will allow us to enforce that certain functions accept only a certain type of path. eg. verify_client_consensus_state() should only accept a Path::ClientConsensusState.


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

Reviewer checklist:

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

@hu55a1n1 hu55a1n1 marked this pull request as ready for review January 13, 2022 18:43
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Love it!

@@ -28,44 +28,123 @@ const UPGRADED_CLIENT_STATE: &str = "upgradedClient";
const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState";

/// The Path enum abstracts out the different sub-paths.
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From, Display)]
Copy link
Member

Choose a reason for hiding this comment

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

Lovely!

@romac romac merged commit 88c03ed into master Jan 17, 2022
@romac romac deleted the hu55a1n1/1696-module-api-improvements branch January 17, 2022 12:14
hu55a1n1 added a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Path variants as types

* Use derive_more crate

* Simplify Path usage

* Minor refactoring

* More cleanup

* Add .changelog entry
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.

2 participants