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

ICS 24 Implementation #5229

Merged
merged 4 commits into from
Oct 22, 2019
Merged
Show file tree
Hide file tree
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
6 changes: 6 additions & 0 deletions types/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ var (
// ErrNoSignatures to doc
ErrNoSignatures = Register(RootCodespace, 16, "no signatures supplied")

// ErrInvalidId to doc
ErrInvalidID = Register(RootCodespace, 17, "invalid identifier")
fedekunze marked this conversation as resolved.
Show resolved Hide resolved

// ErrInvalidPath to doc
ErrInvalidPath = Register(RootCodespace, 18, "invalid path")

// ErrPanic is only set when we recover from a panic, so we know to
// redact potentially sensitive system info
ErrPanic = Register(UndefinedCodespace, 111222, "panic")
Expand Down
10 changes: 8 additions & 2 deletions x/ibc/23-commitment/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ func (k Keeper) BatchVerifyMembership(ctx sdk.Context, proof exported.ProofI, it
continue
}

path := types.ApplyPrefix(k.prefix, pathStr)
path, err := types.ApplyPrefix(k.prefix, pathStr)
if err != nil {
return false
}
ok = proof.VerifyMembership(root, path, value)
if !ok {
return false
Expand All @@ -69,7 +72,10 @@ func (k Keeper) BatchVerifyNonMembership(ctx sdk.Context, proof exported.ProofI,
continue
}

path := types.ApplyPrefix(k.prefix, pathStr)
path, err := types.ApplyPrefix(k.prefix, pathStr)
if err != nil {
return false
}
ok = proof.VerifyNonMembership(root, path)
if !ok {
return false
Expand Down
9 changes: 7 additions & 2 deletions x/ibc/23-commitment/types/merkle.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/cosmos/cosmos-sdk/store/rootmulti"
"github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported"
host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
)

// ICS 023 Merkle Types Implementation
Expand Down Expand Up @@ -103,13 +104,17 @@ func (p Path) String() string {
//
// CONTRACT: provided path string MUST be a well formated path. See ICS24 for
// reference.
func ApplyPrefix(prefix exported.PrefixI, path string) Path {
func ApplyPrefix(prefix exported.PrefixI, path string) (Path, error) {
err := host.DefaultPathValidator(path)
if err != nil {
return Path{}, err
}
// Split paths by the separator
pathSlice := strings.Split(path, "/")
commitmentPath := NewPath(pathSlice)

commitmentPath.KeyPath = commitmentPath.KeyPath.AppendKey(prefix.Bytes(), merkle.KeyEncodingHex)
return commitmentPath
return commitmentPath, nil
}

var _ exported.ProofI = Proof{}
Expand Down
16 changes: 16 additions & 0 deletions x/ibc/24-host/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package host

import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// IBCCodeSpace is the codespace for all errors defined in the ibc module
const IBCCodeSpace = "ibc"

var (
// ErrInvalidID is returned if identifier string is invalid
ErrInvalidID = sdkerrors.Register(IBCCodeSpace, 1, "invalid identifier")

// ErrInvalidPath is returned if path string is invalid
ErrInvalidPath = sdkerrors.Register(IBCCodeSpace, 2, "invalid path")
)
74 changes: 74 additions & 0 deletions x/ibc/24-host/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package host

import (
"regexp"
"strings"

sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// ICS 024 Identifier and Path Validation Implementation
//
// This file defines ValidateFn to validate identifier and path strings
// The spec for ICS 024 can be located here:
// https://github.com/cosmos/ics/tree/master/spec/ics-024-host-requirements

// regular expression to check string is lowercase alphabetic characters only
var isAlphaLower = regexp.MustCompile(`^[a-z]+$`).MatchString

// regular expression to check string is alphanumeric
var isAlphaNumeric = regexp.MustCompile(`^[a-zA-Z0-9]+$`).MatchString

// ValidateFn function type to validate path and identifier bytestrings
type ValidateFn func(string) error

// Default validator function for Client, Connection, and Channel
// identifiers
// Valid Identifier must be between 10-20 characters and only
// contain lowercase alphabetic characters
func DefaultIdentifierValidator(id string) error {
// valid id MUST NOT contain "/" separator
if strings.Contains(id, "/") {
return sdkerrors.Wrap(ErrInvalidID, "identifier cannot contain separator: /")
}
// valid id must be between 10 and 20 characters
if len(id) < 10 || len(id) > 20 {
return sdkerrors.Wrapf(ErrInvalidID, "identifier has invalid length: %d, must be between 10-20 characters", len(id))
}
// valid id must contain only lower alphabetic characters
if !isAlphaLower(id) {
return sdkerrors.Wrap(ErrInvalidID, "identifier must contain only lowercase alphabetic characters")
}
return nil
}

// 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
func NewPathValidator(idValidator ValidateFn) ValidateFn {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nowhere atm, just have it there so that someone can pass in a custom identifier validatefn, and get the appropriate path validatefn back

Can remove it if its not necessary to have right now

return func(path string) error {
pathArr := strings.Split(path, "/")
for _, p := range pathArr {
// Each path element must either be valid identifier or alphanumeric
err := idValidator(p)
if err != nil && !isAlphaNumeric(p) {
return sdkerrors.Wrapf(ErrInvalidPath, "path contains invalid identifier or non-alphanumeric path element: %s", p)
}
}
return nil
}
}

// Default Path Validator takes in path string and validates
// with default identifier rules. This is optimized by simply
// checking that all path elements are alphanumeric
func DefaultPathValidator(path string) error {
pathArr := strings.Split(path, "/")
for _, p := range pathArr {
// Each path element must either be alphanumeric
if !isAlphaNumeric(p) {
return sdkerrors.Wrapf(ErrInvalidPath, "invalid path element containing non-alphanumeric characters: %s", p)
}
}
return nil
}