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

Identifier validation inconsistent with ICS024 #126

Closed
4 tasks done
adizere opened this issue Jul 22, 2020 · 1 comment
Closed
4 tasks done

Identifier validation inconsistent with ICS024 #126

adizere opened this issue Jul 22, 2020 · 1 comment
Assignees

Comments

@adizere
Copy link
Contributor

adizere commented Jul 22, 2020

Summary of Bug

ICS 024 defines several criteria for an identifier to be valid:
https://github.com/cosmos/ics/tree/master/spec/ics-024-host-requirements#paths-identifiers-separators

Identifiers MUST be non-empty (of positive integer length).
Identifiers MUST consist of characters in one of the following categories only:

  • Alphanumeric
  • ., _, +, -, #
  • [, ], <, >

Our validation function implementation is more strict than the above requirements (and possibly buggy).

There are a couple of issues:

  1. we prevent characters such as ., _, +, -, #
  2. we prevent numbers (each character has to be both alphanumeric and lowercase, and numbers are not lowercase)

https://github.com/informalsystems/ibc-rs/blob/96ee9735e050a87b09b3ba7544081537953d64ba/modules/src/ics24_host/validate.rs#L41-L43

Can we align validation with ICS024? Or is there a reason why we have a more strict validation on identifiers?


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere self-assigned this Jul 22, 2020
@adizere
Copy link
Contributor Author

adizere commented Jul 22, 2020

After a closer look:

It seems that the identifier validation requirements changed more recently (April 27) than our code (originally ~March):

cosmos/ibc@dc4ecb8

adizere referenced this issue in informalsystems/hermes Jul 23, 2020
…48 #164)

* Refactored the location of JSON files for serialization testing (#118).

* Validation for channel end + tests template; needs more tests (#148).

* Refining tests for channel end.

* Aligned identifier validation with ICS024 & updated tests (#164).

* Added TODO re: version validation; fix VALID_SPECIAL_CHARS to remove space.

* Fixing the TODO for version field validation.
romac referenced this issue in informalsystems/hermes Jul 29, 2020
…48 #164)

* Refactored the location of JSON files for serialization testing (#118).

* Validation for channel end + tests template; needs more tests (#148).

* Refining tests for channel end.

* Aligned identifier validation with ICS024 & updated tests (#164).

* Added TODO re: version validation; fix VALID_SPECIAL_CHARS to remove space.

* Fixing the TODO for version field validation.
hu55a1n1 referenced this issue in hu55a1n1/hermes Sep 13, 2022
…lsystems#163 #148 #164)

* Refactored the location of JSON files for serialization testing (#118).

* Validation for channel end + tests template; needs more tests (#148).

* Refining tests for channel end.

* Aligned identifier validation with ICS024 & updated tests (#164).

* Added TODO re: version validation; fix VALID_SPECIAL_CHARS to remove space.

* Fixing the TODO for version field validation.
@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 29, 2022
shuoer86 pushed a commit to shuoer86/ibc-rs that referenced this issue Nov 4, 2023
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

No branches or pull requests

1 participant