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

x/ibc/24-host: fix some comments and dedup a function #7923

Merged
merged 3 commits into from
Nov 13, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 19 additions & 30 deletions x/ibc/core/24-host/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,36 +56,37 @@ func defaultIdentifierValidator(id string, min, max int) error { //nolint:unpara
}

// ClientIdentifierValidator is the default validator function for Client identifiers.
// A valid Identifier must be between 9-64 characters and only contain lowercase
// alphabetic characters.
// A valid Identifier must be between 9-64 characters and only contain alphanumeric and some allowed
// special characters (see IsValidID).
func ClientIdentifierValidator(id string) error {
return defaultIdentifierValidator(id, 9, DefaultMaxCharacterLength)
}

// ConnectionIdentifierValidator is the default validator function for Connection identifiers.
// A valid Identifier must be between 10-64 characters and only contain lowercase
// alphabetic characters.
// A valid Identifier must be between 10-64 characters and only contain alphanumeric and some allowed
// special characters (see IsValidID).
func ConnectionIdentifierValidator(id string) error {
return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength)
}

// ChannelIdentifierValidator is the default validator function for Channel identifiers.
// A valid Identifier must be between 10-64 characters and only contain lowercase
// alphabetic characters.
// A valid Identifier must be between 10-64 characters and only contain alphanumeric and some allowed
// special characters (see IsValidID).
func ChannelIdentifierValidator(id string) error {
return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength)
}

// PortIdentifierValidator is the default validator function for Port identifiers.
// A valid Identifier must be between 2-64 characters and only contain lowercase
// alphabetic characters.
// A valid Identifier must be between 2-64 characters and only contain alphanumeric and some allowed
// special characters (see IsValidID).
func PortIdentifierValidator(id string) error {
return defaultIdentifierValidator(id, 2, DefaultMaxCharacterLength)
}

// NewPathValidator takes in a Identifier Validator function and returns
// a Path Validator function which requires path only has valid identifiers
// alphanumeric character strings, and "/" separators
// a Path Validator function which requires path to consist of `/`-separated valid identifiers,
// where a valid identifier is between 1-64 characters, contains only alphanumeric and some allowed
// special characters (see IsValidID), and satisfies the custom `idValidator` function.
func NewPathValidator(idValidator ValidateFn) ValidateFn {
return func(path string) error {
pathArr := strings.Split(path, "/")
Expand All @@ -112,25 +113,13 @@ func NewPathValidator(idValidator ValidateFn) ValidateFn {
}
}

// PathValidator takes in path string and validates with default identifier rules.
// This is optimized by simply checking that all path elements are alphanumeric.
// PathValidator takes in path string and validates with default identifier rules:
// path consists of `/`-separated valid identifiers,
// each identifier is between 1-64 characters and contains only alphanumeric and some allowed
// special characters (see IsValidID).
func PathValidator(path string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

@fedekunze why do we have this function?

It seems to be only used in connection genesis validating ConnectionPaths but from what I can tell, a connection path is simply all the connection ids attached to a client id. Instead we should be calling connection id validation. I don't think there are any / in a connection path as we currently use them

Copy link
Contributor

Choose a reason for hiding this comment

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

I can open a followup fixing this, here is where we use ConnectionPaths

pathArr := strings.Split(path, "/")
if len(pathArr) > 0 && pathArr[0] == path {
return sdkerrors.Wrapf(ErrInvalidPath, "path %s doesn't contain any separator '/'", path)
}

for _, p := range pathArr {
// a path beginning or ending in a separator returns empty string elements.
if p == "" {
return sdkerrors.Wrapf(ErrInvalidPath, "path %s cannot begin or end with '/'", path)
}

// Each path element must be a valid identifier or constant number
if err := defaultIdentifierValidator(p, 1, DefaultMaxCharacterLength); err != nil {
return sdkerrors.Wrapf(err, "path %s contains an invalid identifier: '%s'", path, p)
}
}

return nil
f := NewPathValidator(func(path string) error {
return nil
})
return f(path)
}