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.

Sign up for GitHub

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

Jump to bottom

Update transfer docs and make ibc prefix a const #7926

Merged
merged 6 commits into from
Nov 13, 2020
Merged

Update transfer docs and make ibc prefix a const #7926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d212a0a
add docs and make prefix a const
colin-axner Nov 13, 2020
af06815
Merge branch 'master' into colin/7906-transfer-docs
colin-axner Nov 13, 2020
f75ea96
Merge branch 'master' into colin/7906-transfer-docs
colin-axner Nov 13, 2020
55c59c7
Merge branch 'master' into colin/7906-transfer-docs
colin-axner Nov 13, 2020
8cc664e
Merge branch 'master' into colin/7906-transfer-docs
mergify[bot] Nov 13, 2020
1230e62
Merge branch 'master' into colin/7906-transfer-docs
mergify[bot] Nov 13, 2020
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
4 changes: 3 additions & 1 deletion x/ibc/applications/transfer/keeper/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d
// DenomPathFromHash returns the full denomination path prefix from an ibc denom with a hash
// component.
func (k Keeper) DenomPathFromHash(ctx sdk.Context, denom string) (string, error) {
hexHash := denom[4:]
// trim the denomination prefix, by default "ibc/"
hexHash := denom[len(types.DenomPrefix+"/"):]

hash, err := types.ParseHexHash(hexHash)
if err != nil {
return "", sdkerrors.Wrap(types.ErrInvalidDenomForTransfer, err.Error())
Expand Down
6 changes: 6 additions & 0 deletions x/ibc/applications/transfer/spec/01_concepts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,9 @@ Cosmos Hub for the `uatom`). Sending a token back to the same chain across a dif
**not** move the token back across its timeline. If a channel in the chain history closes before the
token can be sent back across that channel, then the token will not be returnable to its original
form.


## Security Considerations

For safety, no other module must be capable of minting tokens with the `ibc/` prefix. The IBC
transfer module needs a subset of the denomination space that only it can create tokens in.
3 changes: 3 additions & 0 deletions x/ibc/applications/transfer/types/keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ const (

// QuerierRoute is the querier route for IBC transfer
QuerierRoute = ModuleName

// DenomPrefix is the prefix used for internal SDK coin representation.
DenomPrefix = "ibc"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should this be a var? I think the difference would be whether we want to allow an application creator to choose their own prefix

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, I think it makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, lets just leave as a const, if it is a var another module could simply change the value of the prefix which we can't allow for safety

)

var (
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/applications/transfer/types/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (dt DenomTrace) GetPrefix() string {
// 'ibc/{hash(tracePath + baseDenom)}'. If the trace is empty, it will return the base denomination.
func (dt DenomTrace) IBCDenom() string {
if dt.Path != "" {
return fmt.Sprintf("ibc/%s", dt.Hash())
return fmt.Sprintf("%s/%s", DenomPrefix, dt.Hash())
}
return dt.BaseDenom
}
Expand Down Expand Up @@ -172,8 +172,8 @@ func ValidateIBCDenom(denom string) error {

switch {
case strings.TrimSpace(denom) == "",
len(denomSplit) == 1 && denomSplit[0] == "ibc",
len(denomSplit) == 2 && (denomSplit[0] != "ibc" || strings.TrimSpace(denomSplit[1]) == ""):
len(denomSplit) == 1 && denomSplit[0] == DenomPrefix,
len(denomSplit) == 2 && (denomSplit[0] != DenomPrefix || strings.TrimSpace(denomSplit[1]) == ""):
return sdkerrors.Wrapf(ErrInvalidDenomForTransfer, "denomination should be prefixed with the format 'ibc/{hash(trace + \"/\" + %s)}'", denom)

case denomSplit[0] == denom && strings.TrimSpace(denom) != "":
Expand Down