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

chore: implement required FungibleTokenPacketData v3 interface methods #6126

Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
dbe408a
chore: adding proto files for ics20-v2
chatton Apr 8, 2024
845b269
chore: add newline
chatton Apr 8, 2024
69b2643
chore: modify MsgTransfer to accept coins instead of coin
chatton Apr 8, 2024
8f4eaaa
Merge branch 'feat/ics20-v2' into cian/issue#5799-update-msg-transfer
chatton Apr 8, 2024
d8bfd45
chore: reverted unintentional comment changes
chatton Apr 8, 2024
e32a13d
chore: reverted unintentional comment changes
chatton Apr 8, 2024
e1b52c3
chore: adding test for conversion fn
chatton Apr 8, 2024
0e0e478
chore: fix e2e linter
chatton Apr 8, 2024
00e6c10
chore: adding docs
chatton Apr 8, 2024
8b5eb77
chore: addressing PR feedback
chatton Apr 8, 2024
62d76e9
chore: remove duplicate import
chatton Apr 8, 2024
2cc1804
chore: addressing PR feedback
chatton Apr 9, 2024
4b42e80
Merge branch 'cian/issue#5799-update-msg-transfer' into cian/issue#61…
chatton Apr 9, 2024
3da8e01
chore: moved extration logic into internal package
chatton Apr 9, 2024
dee5a30
chore: commented out failing test
chatton Apr 9, 2024
f7e5792
chore: adding link to issue
chatton Apr 9, 2024
bd34b69
Merge branch 'feat/ics20-v2' into cian/issue#6114-add-conversion-func…
chatton Apr 9, 2024
880b24d
chore: remove duplicate import
chatton Apr 9, 2024
8cad136
chore: fix linting errors
chatton Apr 9, 2024
5ebba40
FungibleTokenPacketData interface methods + tests
charleenfei Apr 9, 2024
867746f
linter
charleenfei Apr 9, 2024
24ed388
wip: token validation
charleenfei Apr 11, 2024
a666243
Merge branch 'cian/issue#6114-add-conversion-function' into charly/is…
charleenfei Apr 11, 2024
a85fa71
update trace identifier validation in Token + tests
charleenfei Apr 15, 2024
e10863e
rm Printf
charleenfei Apr 15, 2024
81c4d50
update with pr review
charleenfei Apr 15, 2024
15d949f
Merge branch 'feat/ics20-v2' into cian/issue#6114-add-conversion-func…
charleenfei Apr 16, 2024
555d2f2
pr review
charleenfei Apr 16, 2024
3b421b3
linter
charleenfei Apr 16, 2024
3f87adf
Merge branch 'cian/issue#6114-add-conversion-function' into charly/is…
charleenfei Apr 16, 2024
aa33c88
Merge branch 'feat/ics20-v2' into charly/issue#5795-implement-require…
charleenfei Apr 16, 2024
06ff045
rm unused function: linter
charleenfei Apr 16, 2024
1f6c780
wip pr feedback
charleenfei Apr 17, 2024
bae7e4f
fix test
charleenfei Apr 17, 2024
fa6c1ce
pr review
charleenfei Apr 18, 2024
8cb5d19
lintttttt
charleenfei Apr 18, 2024
3669a6c
update pr review
charleenfei Apr 22, 2024
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
21 changes: 21 additions & 0 deletions modules/apps/transfer/internal/denom/denom.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package denom

import (
"fmt"
"strings"

errorsmod "cosmossdk.io/errors"

channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
)

// ExtractPathAndBaseFromFullDenom returns the trace path and the base denom from
Expand Down Expand Up @@ -37,3 +41,20 @@ func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string)

return pathSlice, baseDenom
}

func ValidateTraceIdentifiers(identifiers []string) error {
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
if len(identifiers) == 0 || len(identifiers)%2 != 0 {
return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers)
}

// validate correctness of port and channel identifiers
for i := 0; i < len(identifiers); i += 2 {
if err := host.PortIdentifierValidator(identifiers[i]); err != nil {
return errorsmod.Wrapf(err, "invalid port ID at position %d", i)
}
if err := host.ChannelIdentifierValidator(identifiers[i+1]); err != nil {
return errorsmod.Wrapf(err, "invalid channel ID at position %d", i)
}
}
return nil
}
22 changes: 2 additions & 20 deletions modules/apps/transfer/types/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
cmttypes "github.com/cometbft/cometbft/types"

denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom"
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
)

// ParseDenomTrace parses a string with the ibc prefix (denom trace) and the base denomination
Expand Down Expand Up @@ -82,23 +81,6 @@ func (dt DenomTrace) IsNativeDenom() bool {
return dt.Path == ""
}

func validateTraceIdentifiers(identifiers []string) error {
if len(identifiers) == 0 || len(identifiers)%2 != 0 {
return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers)
}

// validate correctness of port and channel identifiers
for i := 0; i < len(identifiers); i += 2 {
if err := host.PortIdentifierValidator(identifiers[i]); err != nil {
return errorsmod.Wrapf(err, "invalid port ID at position %d", i)
}
if err := host.ChannelIdentifierValidator(identifiers[i+1]); err != nil {
return errorsmod.Wrapf(err, "invalid channel ID at position %d", i)
}
}
return nil
}

// Validate performs a basic validation of the DenomTrace fields.
func (dt DenomTrace) Validate() error {
// empty trace is accepted when token lives on the original chain
Expand All @@ -112,7 +94,7 @@ func (dt DenomTrace) Validate() error {
// NOTE: no base denomination validation

identifiers := strings.Split(dt.Path, "/")
return validateTraceIdentifiers(identifiers)
return denominternal.ValidateTraceIdentifiers(identifiers)
}

// Traces defines a wrapper type for a slice of DenomTrace.
Expand Down Expand Up @@ -178,7 +160,7 @@ func ValidatePrefixedDenom(denom string) error {
}

identifiers := strings.Split(path, "/")
return validateTraceIdentifiers(identifiers)
return denominternal.ValidateTraceIdentifiers(identifiers)
}

// ValidateIBCDenom validates that the given denomination is either:
Expand Down
123 changes: 122 additions & 1 deletion modules/apps/transfer/types/v3/packet.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
package v3

// NewFungibleTokenPacketData constructs a new FungibleTokenPacketData instance
import (
"encoding/json"
"errors"
"strings"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"

denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom"
"github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
)

var (
_ ibcexported.PacketData = (*FungibleTokenPacketData)(nil)
_ ibcexported.PacketDataProvider = (*FungibleTokenPacketData)(nil)
)

// NewFungibleTokenPacketData constructs a new NewFungibleTokenPacketData instance
func NewFungibleTokenPacketData(
tokens []*Token,
sender, receiver string,
Expand All @@ -13,3 +34,103 @@ func NewFungibleTokenPacketData(
Memo: memo,
}
}

// ValidateBasic is used for validating the token transfer.
// NOTE: The addresses formats are not validated as the sender and recipient can have different
// formats defined by their corresponding chains that are not known to IBC.
func (ftpd FungibleTokenPacketData) ValidateBasic() error {
if strings.TrimSpace(ftpd.Sender) == "" {
return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "sender address cannot be blank")
}

if strings.TrimSpace(ftpd.Receiver) == "" {
return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "receiver address cannot be blank")
}

if len(ftpd.Tokens) == 0 {
return errorsmod.Wrap(types.ErrInvalidAmount, "tokens cannot be empty")
}

for _, token := range ftpd.Tokens {
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
amount, ok := sdkmath.NewIntFromString(token.Amount)
if !ok {
return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", token.Amount)
}

if !amount.IsPositive() {
return errorsmod.Wrapf(types.ErrInvalidAmount, "amount must be strictly positive: got %d", amount)
}

if err := token.Validate(); err != nil {
return err
}
}

if len(ftpd.Memo) > types.MaximumMemoLength {
return errorsmod.Wrapf(types.ErrInvalidMemo, "memo must not exceed %d bytes", types.MaximumMemoLength)
}

return nil
}

// ValidateToken validates a token denomination and trace identifiers.
func (t *Token) Validate() error {
if err := sdk.ValidateDenom(t.Denom); err != nil {
return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error())
}

trace := strings.Join(t.Trace, "/")
identifiers := strings.Split(trace, "/")
Copy link
Member

Choose a reason for hiding this comment

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

Why are you joining them just to split them again? I think the reason is that the list looks like: ["channel-0/transfer", "channel-120/transfer"]. However, I still think we should do this in a cleaner way if possible such as using slices or maybe just modifying ValidateTraceIdentifiers to accept this format

Copy link
Contributor Author

@charleenfei charleenfei Apr 17, 2024

Choose a reason for hiding this comment

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

I'd rather avoid modifying ValidateTraceIdentifiers since that is being used by both DenomTrace.Validate() and ValidatePrefixedDenom() on the fungible token packet data v1, and the idea is that both v2 and v1 could use this function, without having to refactor the logic on those functions as well (for now). Potentially that refactor can be another issue later down the road. I can check out how to implement using slices.


return denominternal.ValidateTraceIdentifiers(identifiers)
}

func (t *Token) GetFullDenomPath() string {
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
if len(t.Trace) == 0 {
return t.Denom
}
return strings.Join(append(t.Trace, t.Denom), "/")
}

// GetBytes is a helper for serialising
func (ftpd FungibleTokenPacketData) GetBytes() []byte {
bz, err := json.Marshal(&ftpd)
if err != nil {
panic(errors.New("cannot marshal v3 FungibleTokenPacketData into bytes"))
}

return bz
}

// GetCustomPacketData interprets the memo field of the packet data as a JSON object
// and returns the value associated with the given key.
// If the key is missing or the memo is not properly formatted, then nil is returned.
func (ftpd FungibleTokenPacketData) GetCustomPacketData(key string) interface{} {
if len(ftpd.Memo) == 0 {
return nil
}

jsonObject := make(map[string]interface{})
err := json.Unmarshal([]byte(ftpd.Memo), &jsonObject)
if err != nil {
return nil
}

memoData, found := jsonObject[key]
if !found {
return nil
}

return memoData
}

// GetPacketSender returns the sender address embedded in the packet data.
//
// NOTE:
// - The sender address is set by the module which requested the packet to be sent,
// and this module may not have validated the sender address by a signature check.
// - The sender address must only be used by modules on the sending chain.
// - sourcePortID is not used in this implementation.
func (ftpd FungibleTokenPacketData) GetPacketSender(sourcePortID string) string {
return ftpd.Sender
}
Loading
Loading