From ad7412dc4d2458acf28ae03e5d367bb5833478c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 11:20:26 +0200 Subject: [PATCH 1/9] refactor: initial introduction of typed trace --- .../apps/transfer/internal/convert/convert.go | 6 +- .../transfer/internal/convert/convert_test.go | 41 ++++-- modules/apps/transfer/keeper/relay.go | 4 +- modules/apps/transfer/types/denom.go | 28 ++-- modules/apps/transfer/types/query.pb.go | 4 +- modules/apps/transfer/types/token.go | 26 +--- modules/apps/transfer/types/token.pb.go | 79 +++++----- modules/apps/transfer/types/token_test.go | 136 +++++++----------- modules/apps/transfer/types/trace.go | 29 ++++ .../ibc/applications/transfer/v1/query.proto | 4 +- .../ibc/applications/transfer/v2/token.proto | 7 +- 11 files changed, 182 insertions(+), 182 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index 5dd36afa4f6..cc46f7d6e73 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -34,7 +34,7 @@ func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) (types.FungibleT } // extractDenomAndTraceFromV1Denom extracts the base denom and remaining trace from a v1 IBC denom. -func ExtractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { +func ExtractDenomAndTraceFromV1Denom(v1Denom string) (string, []types.Trace) { v1DenomTrace := types.ParseDenomTrace(v1Denom) // if the path string is empty, then the base denom is the full native denom. @@ -51,9 +51,9 @@ func ExtractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { // the path slices consists of entries of ports and channel ids separately, // we need to combine them to form the trace. - var trace []string + var trace []types.Trace for i := 0; i < len(splitPath); i += 2 { - trace = append(trace, strings.Join(splitPath[i:i+2], "/")) + trace = append(trace, types.NewTrace(splitPath[i], splitPath[i+1])) } return v1DenomTrace.BaseDenom, trace diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index fa289c32d7f..73f80c75cd4 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -29,8 +29,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0"}, + Base: "atom", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + }, }, Amount: "1000", }, @@ -59,8 +61,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom/withslash", - Trace: []string{"transfer/channel-0"}, + Base: "atom/withslash", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + }, }, Amount: "1000", }, @@ -74,8 +78,10 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom/", - Trace: []string{"transfer/channel-0"}, + Base: "atom/", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + }, }, Amount: "1000", }, @@ -89,8 +95,11 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom/pool", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Base: "atom/pool", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + types.NewTrace("transfer", "channel-1"), + }, }, Amount: "1000", }, @@ -104,8 +113,12 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + Base: "atom", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + types.NewTrace("transfer", "channel-1"), + types.NewTrace("transfer-custom", "channel-2"), + }, }, Amount: "1000", }, @@ -119,8 +132,12 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []types.Token{ { Denom: types.Denom{ - Base: "atom/pool", - Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + Base: "atom/pool", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + types.NewTrace("transfer", "channel-1"), + types.NewTrace("transfer-custom", "channel-2"), + }, }, Amount: "1000", }, diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 869f62b89df..e1e27c66676 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -141,7 +141,7 @@ func (k Keeper) sendTransfer( telemetry.SetGaugeWithLabels( []string{"tx", "msg", "ibc", "transfer"}, float32(amount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.GetFullDenomPath())}, + []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.Denom.FullPath())}, ) } } @@ -239,7 +239,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t // sender chain is the source, mint vouchers // since SendPacket did not prefix the denomination, we must add the destination port and channel to the trace - trace := []string{fmt.Sprintf("%s/%s", packet.DestinationPort, packet.DestinationChannel)} + trace := []types.Trace{types.NewTrace(packet.DestinationPort, packet.DestinationChannel)} token.Denom.Trace = append(trace, token.Denom.Trace...) if !k.HasDenom(ctx, token.Denom.Hash()) { diff --git a/modules/apps/transfer/types/denom.go b/modules/apps/transfer/types/denom.go index 482eac10e24..160bff16f96 100644 --- a/modules/apps/transfer/types/denom.go +++ b/modules/apps/transfer/types/denom.go @@ -16,15 +16,12 @@ func (d Denom) Validate() error { // NOTE: base denom validation cannot be performed as each chain may define // its own base denom validation if strings.TrimSpace(d.Base) == "" { - return fmt.Errorf("base denomination cannot be blank") + return errorsmod.Wrap(ErrInvalidDenomForTransfer, "base denomination cannot be blank") } - if len(d.Trace) != 0 { - trace := strings.Join(d.Trace, "/") - identifiers := strings.Split(trace, "/") - - if err := validateTraceIdentifiers(identifiers); err != nil { - return err + for _, trace := range d.Trace { + if err := trace.Validate(); err != nil { + return errorsmod.Wrap(err, "invalid trace") } } @@ -59,8 +56,8 @@ func (d Denom) FullPath() string { var sb strings.Builder for _, t := range d.Trace { - sb.WriteString(t) // nolint:revive // no error returned by WriteString - sb.WriteByte('/') //nolint:revive // no error returned by WriteByte + sb.WriteString(t.String()) // nolint:revive // no error returned by WriteString + sb.WriteByte('/') //nolint:revive // no error returned by WriteByte } sb.WriteString(d.Base) //nolint:revive return sb.String() @@ -74,24 +71,21 @@ func (d Denom) IsNative() bool { // SenderChainIsSource returns false if the denomination originally came // from the receiving chain and true otherwise. func (d Denom) SenderChainIsSource(sourcePort, sourceChannel string) bool { - // This is the prefix that would have been prefixed to the denomination - // on sender chain IF and only if the token originally came from the - // receiving chain. - + // sender chain is source, if the receiver is not source return !d.ReceiverChainIsSource(sourcePort, sourceChannel) } // ReceiverChainIsSource returns true if the denomination originally came // from the receiving chain and false otherwise. func (d Denom) ReceiverChainIsSource(sourcePort, sourceChannel string) bool { - // The first element in the Denom's trace should contain the SourcePort and SourceChannel. - // If the receiver chain originally sent the token to the sender chain, the first element of - // the denom's trace will contain the sender's SourcePort and SourceChannel. + // if the denom is native, then the receiver chain cannot be source if d.IsNative() { return false } - return d.Trace[0] == sourcePort+"/"+sourceChannel + // if the receiver chain originally sent the token to the sender chain, then the first element of + // the denom's trace (latest trace) will contain the SourcePort and SourceChannel. + return d.Trace[0].PortId == sourcePort && d.Trace[0].ChannelId == sourceChannel } // Denoms defines a wrapper type for a slice of Denom. diff --git a/modules/apps/transfer/types/query.pb.go b/modules/apps/transfer/types/query.pb.go index 0de8751b57b..b49978353ec 100644 --- a/modules/apps/transfer/types/query.pb.go +++ b/modules/apps/transfer/types/query.pb.go @@ -687,7 +687,7 @@ type QueryClient interface { // Deprecated: Please use the Denoms endpoint instead. DenomTraces(ctx context.Context, in *QueryDenomTracesRequest, opts ...grpc.CallOption) (*QueryDenomTracesResponse, error) // DenomTrace queries a denomination trace information. - // Deprecated: Please se the Denom endpoint instead. + // Deprecated: Please see the Denom endpoint instead. DenomTrace(ctx context.Context, in *QueryDenomTraceRequest, opts ...grpc.CallOption) (*QueryDenomTraceResponse, error) // Params queries all parameters of the ibc-transfer module. Params(ctx context.Context, in *QueryParamsRequest, opts ...grpc.CallOption) (*QueryParamsResponse, error) @@ -769,7 +769,7 @@ type QueryServer interface { // Deprecated: Please use the Denoms endpoint instead. DenomTraces(context.Context, *QueryDenomTracesRequest) (*QueryDenomTracesResponse, error) // DenomTrace queries a denomination trace information. - // Deprecated: Please se the Denom endpoint instead. + // Deprecated: Please see the Denom endpoint instead. DenomTrace(context.Context, *QueryDenomTraceRequest) (*QueryDenomTraceResponse, error) // Params queries all parameters of the ibc-transfer module. Params(context.Context, *QueryParamsRequest) (*QueryParamsResponse, error) diff --git a/modules/apps/transfer/types/token.go b/modules/apps/transfer/types/token.go index be8d30808ca..0b826929d0f 100644 --- a/modules/apps/transfer/types/token.go +++ b/modules/apps/transfer/types/token.go @@ -7,10 +7,10 @@ import ( sdkmath "cosmossdk.io/math" ) -// Validate validates a token denomination and trace identifiers. +// Validate validates a token denomination and amount. func (t Token) Validate() error { - if strings.TrimSpace(t.Denom.Base) == "" { - return errorsmod.Wrap(ErrInvalidDenomForTransfer, "denom cannot be empty") + if err := t.Denom.Validate(); err != nil { + return errorsmod.Wrap(err, "invalid token denom") } amount, ok := sdkmath.NewIntFromString(t.Amount) @@ -22,29 +22,9 @@ func (t Token) Validate() error { return errorsmod.Wrapf(ErrInvalidAmount, "amount must be strictly positive: got %d", amount) } - if len(t.Denom.Trace) != 0 { - trace := strings.Join(t.Denom.Trace, "/") - identifiers := strings.Split(trace, "/") - - if err := validateTraceIdentifiers(identifiers); err != nil { - return err - } - } - return nil } -// GetFullDenomPath returns the full denomination according to the ICS20 specification: -// tracePath + "/" + baseDenom -// If there exists no trace then the base denomination is returned. -func (t Token) GetFullDenomPath() string { - if len(t.Denom.Trace) == 0 { - return t.Denom.Base - } - - return strings.Join(append(t.Denom.Trace, t.Denom.Base), "/") -} - // Tokens is a set of Token type Tokens []Token diff --git a/modules/apps/transfer/types/token.pb.go b/modules/apps/transfer/types/token.pb.go index ec174e99138..82789c743c3 100644 --- a/modules/apps/transfer/types/token.pb.go +++ b/modules/apps/transfer/types/token.pb.go @@ -83,7 +83,7 @@ type Denom struct { // the base token denomination Base string `protobuf:"bytes,1,opt,name=base,proto3" json:"base,omitempty"` // the trace of the token - Trace []string `protobuf:"bytes,3,rep,name=trace,proto3" json:"trace,omitempty"` + Trace []Trace `protobuf:"bytes,3,rep,name=trace,proto3" json:"trace"` } func (m *Denom) Reset() { *m = Denom{} } @@ -126,7 +126,7 @@ func (m *Denom) GetBase() string { return "" } -func (m *Denom) GetTrace() []string { +func (m *Denom) GetTrace() []Trace { if m != nil { return m.Trace } @@ -141,9 +141,8 @@ type Trace struct { ChannelId string `protobuf:"bytes,2,opt,name=channel_id,json=channelId,proto3" json:"channel_id,omitempty"` } -func (m *Trace) Reset() { *m = Trace{} } -func (m *Trace) String() string { return proto.CompactTextString(m) } -func (*Trace) ProtoMessage() {} +func (m *Trace) Reset() { *m = Trace{} } +func (*Trace) ProtoMessage() {} func (*Trace) Descriptor() ([]byte, []int) { return fileDescriptor_732b93aa1330663e, []int{2} } @@ -199,26 +198,27 @@ func init() { } var fileDescriptor_732b93aa1330663e = []byte{ - // 303 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xb1, 0x4e, 0xc3, 0x30, - 0x18, 0x84, 0x13, 0xda, 0x14, 0xc5, 0x6c, 0x56, 0x05, 0x15, 0x02, 0x53, 0x95, 0x25, 0x0b, 0xb6, - 0x28, 0x03, 0x6c, 0x95, 0x2a, 0x96, 0x8e, 0x44, 0x9d, 0x58, 0xc0, 0x76, 0x4c, 0x6a, 0xd1, 0xf8, - 0x8f, 0x62, 0xb7, 0x12, 0x6f, 0xc1, 0x63, 0x75, 0xec, 0xc8, 0x84, 0x50, 0xfb, 0x22, 0xc8, 0x49, - 0x90, 0x3a, 0xb1, 0xdd, 0xd9, 0xdf, 0xfd, 0x27, 0x1d, 0x4a, 0xb4, 0x90, 0x8c, 0x97, 0xe5, 0x52, - 0x4b, 0xee, 0x34, 0x18, 0xcb, 0x5c, 0xc5, 0x8d, 0x7d, 0x53, 0x15, 0x5b, 0x8f, 0x99, 0x83, 0x77, - 0x65, 0x68, 0x59, 0x81, 0x03, 0x7c, 0xa1, 0x85, 0xa4, 0x87, 0x24, 0xfd, 0x23, 0xe9, 0x7a, 0x7c, - 0xde, 0xcf, 0x21, 0x87, 0x1a, 0x64, 0x5e, 0x35, 0x99, 0xd1, 0x2b, 0x8a, 0xe6, 0xfe, 0x04, 0x9e, - 0xa0, 0x28, 0x53, 0x06, 0x8a, 0x41, 0x38, 0x0c, 0x93, 0x93, 0xf1, 0x35, 0xfd, 0xef, 0x18, 0x7d, - 0xf4, 0xe8, 0xb4, 0xbb, 0xf9, 0xbe, 0x0a, 0xd2, 0x26, 0x87, 0x4f, 0x51, 0x8f, 0x17, 0xb0, 0x32, - 0x6e, 0x70, 0x34, 0x0c, 0x93, 0x38, 0x6d, 0xdd, 0xe8, 0x16, 0x45, 0x35, 0x8d, 0x31, 0xea, 0x0a, - 0x6e, 0x55, 0x5d, 0x10, 0xa7, 0xb5, 0xc6, 0x7d, 0x14, 0xb9, 0x8a, 0x4b, 0x35, 0xe8, 0x0c, 0x3b, - 0x49, 0x9c, 0x36, 0x66, 0x34, 0x41, 0xd1, 0xdc, 0x0b, 0x7c, 0x86, 0x8e, 0x4b, 0xa8, 0xdc, 0x8b, - 0xce, 0xda, 0x54, 0xcf, 0xdb, 0x59, 0x86, 0x2f, 0x11, 0x92, 0x0b, 0x6e, 0x8c, 0x5a, 0xfa, 0xbf, - 0xa6, 0x30, 0x6e, 0x5f, 0x66, 0xd9, 0xf4, 0x69, 0xb3, 0x23, 0xe1, 0x76, 0x47, 0xc2, 0x9f, 0x1d, - 0x09, 0x3f, 0xf7, 0x24, 0xd8, 0xee, 0x49, 0xf0, 0xb5, 0x27, 0xc1, 0xf3, 0x7d, 0xae, 0xdd, 0x62, - 0x25, 0xa8, 0x84, 0x82, 0x49, 0xb0, 0x05, 0x58, 0xa6, 0x85, 0xbc, 0xc9, 0x81, 0xad, 0x1f, 0x58, - 0x01, 0xd9, 0x6a, 0xa9, 0xac, 0x5f, 0xfb, 0x60, 0x65, 0xf7, 0x51, 0x2a, 0x2b, 0x7a, 0xf5, 0x5e, - 0x77, 0xbf, 0x01, 0x00, 0x00, 0xff, 0xff, 0x7e, 0xb8, 0x68, 0x44, 0x8f, 0x01, 0x00, 0x00, + // 319 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x84, 0x91, 0xb1, 0x4b, 0x3b, 0x31, + 0x14, 0xc7, 0xef, 0x7e, 0xed, 0xf5, 0x47, 0xe3, 0x16, 0x44, 0x8b, 0x68, 0x5a, 0xea, 0xd2, 0xc5, + 0x04, 0xea, 0xa0, 0xb8, 0x08, 0x45, 0x87, 0x8e, 0x96, 0x4e, 0x22, 0x68, 0x92, 0x8b, 0xd7, 0x60, + 0x2f, 0xef, 0xb8, 0xa4, 0x05, 0xff, 0x0b, 0x47, 0x47, 0xff, 0x9c, 0x8e, 0x1d, 0x9d, 0x44, 0xda, + 0x7f, 0x44, 0x92, 0x5e, 0xa1, 0x93, 0x6e, 0xef, 0xbd, 0xfb, 0x7c, 0xdf, 0xe7, 0xc8, 0x43, 0x3d, + 0x2d, 0x24, 0xe3, 0x45, 0x31, 0xd5, 0x92, 0x3b, 0x0d, 0xc6, 0x32, 0x57, 0x72, 0x63, 0x9f, 0x55, + 0xc9, 0xe6, 0x7d, 0xe6, 0xe0, 0x45, 0x19, 0x5a, 0x94, 0xe0, 0x00, 0x1f, 0x6b, 0x21, 0xe9, 0x2e, + 0x49, 0xb7, 0x24, 0x9d, 0xf7, 0x8f, 0xf6, 0x33, 0xc8, 0x20, 0x80, 0xcc, 0x57, 0x9b, 0x4c, 0xf7, + 0x09, 0x25, 0x63, 0xbf, 0x02, 0x5f, 0xa3, 0x24, 0x55, 0x06, 0xf2, 0x56, 0xdc, 0x89, 0x7b, 0x7b, + 0xfd, 0x53, 0xfa, 0xdb, 0x32, 0x7a, 0xe3, 0xd1, 0x41, 0x7d, 0xf1, 0xd5, 0x8e, 0x46, 0x9b, 0x1c, + 0x3e, 0x40, 0x0d, 0x9e, 0xc3, 0xcc, 0xb8, 0xd6, 0xbf, 0x4e, 0xdc, 0x6b, 0x8e, 0xaa, 0xae, 0xfb, + 0x80, 0x92, 0x40, 0x63, 0x8c, 0xea, 0x82, 0x5b, 0x15, 0x04, 0xcd, 0x51, 0xa8, 0xbd, 0xd5, 0x95, + 0x5c, 0xaa, 0x56, 0xad, 0x53, 0xfb, 0xdb, 0x3a, 0xf6, 0xe8, 0xd6, 0x1a, 0x72, 0xdd, 0x5b, 0x94, + 0x84, 0x29, 0x3e, 0x44, 0xff, 0x0b, 0x28, 0xdd, 0xa3, 0x4e, 0x2b, 0x41, 0xc3, 0xb7, 0xc3, 0x14, + 0x9f, 0x20, 0x24, 0x27, 0xdc, 0x18, 0x35, 0xf5, 0xdf, 0x36, 0xff, 0xd6, 0xac, 0x26, 0xc3, 0xf4, + 0xaa, 0xfe, 0xfe, 0xd1, 0x8e, 0x06, 0x77, 0x8b, 0x15, 0x89, 0x97, 0x2b, 0x12, 0x7f, 0xaf, 0x48, + 0xfc, 0xb6, 0x26, 0xd1, 0x72, 0x4d, 0xa2, 0xcf, 0x35, 0x89, 0xee, 0x2f, 0x32, 0xed, 0x26, 0x33, + 0x41, 0x25, 0xe4, 0x4c, 0x82, 0xcd, 0xc1, 0x32, 0x2d, 0xe4, 0x59, 0x06, 0x6c, 0x7e, 0xc9, 0x72, + 0x48, 0x67, 0x53, 0x65, 0xfd, 0x79, 0x76, 0xce, 0xe2, 0x5e, 0x0b, 0x65, 0x45, 0x23, 0x3c, 0xf0, + 0xf9, 0x4f, 0x00, 0x00, 0x00, 0xff, 0xff, 0xba, 0x9e, 0xff, 0x7f, 0xc0, 0x01, 0x00, 0x00, } func (m *Token) Marshal() (dAtA []byte, err error) { @@ -283,9 +283,14 @@ func (m *Denom) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = l if len(m.Trace) > 0 { for iNdEx := len(m.Trace) - 1; iNdEx >= 0; iNdEx-- { - i -= len(m.Trace[iNdEx]) - copy(dAtA[i:], m.Trace[iNdEx]) - i = encodeVarintToken(dAtA, i, uint64(len(m.Trace[iNdEx]))) + { + size, err := m.Trace[iNdEx].MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintToken(dAtA, i, uint64(size)) + } i-- dAtA[i] = 0x1a } @@ -374,8 +379,8 @@ func (m *Denom) Size() (n int) { n += 1 + l + sovToken(uint64(l)) } if len(m.Trace) > 0 { - for _, s := range m.Trace { - l = len(s) + for _, e := range m.Trace { + l = e.Size() n += 1 + l + sovToken(uint64(l)) } } @@ -585,7 +590,7 @@ func (m *Denom) Unmarshal(dAtA []byte) error { if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field Trace", wireType) } - var stringLen uint64 + var msglen int for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowToken @@ -595,23 +600,25 @@ func (m *Denom) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - stringLen |= uint64(b&0x7F) << shift + msglen |= int(b&0x7F) << shift if b < 0x80 { break } } - intStringLen := int(stringLen) - if intStringLen < 0 { + if msglen < 0 { return ErrInvalidLengthToken } - postIndex := iNdEx + intStringLen + postIndex := iNdEx + msglen if postIndex < 0 { return ErrInvalidLengthToken } if postIndex > l { return io.ErrUnexpectedEOF } - m.Trace = append(m.Trace, string(dAtA[iNdEx:postIndex])) + m.Trace = append(m.Trace, Trace{}) + if err := m.Trace[len(m.Trace)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } iNdEx = postIndex default: iNdEx = preIndex diff --git a/modules/apps/transfer/types/token_test.go b/modules/apps/transfer/types/token_test.go index 871113f48e8..e9677a76508 100644 --- a/modules/apps/transfer/types/token_test.go +++ b/modules/apps/transfer/types/token_test.go @@ -20,58 +20,6 @@ var ( receiver = sdk.AccAddress("testaddr2").String() ) -func TestGetFullDenomPath(t *testing.T) { - testCases := []struct { - name string - packetData FungibleTokenPacketDataV2 - expPath string - }{ - { - "denom path with trace", - NewFungibleTokenPacketDataV2( - []Token{ - { - Denom: Denom{ - Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, - Amount: amount, - }, - }, - sender, - receiver, - "", - ), - "transfer/channel-0/transfer/channel-1/atom/pool", - }, - { - "nil trace", - NewFungibleTokenPacketDataV2( - []Token{ - { - Denom: Denom{ - Base: denom, - Trace: []string{}, - }, - Amount: amount, - }, - }, - sender, - receiver, - "", - ), - denom, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - path := tc.packetData.Tokens[0].GetFullDenomPath() - require.Equal(t, tc.expPath, path) - }) - } -} - func TestValidate(t *testing.T) { testCases := []struct { name string @@ -82,8 +30,11 @@ func TestValidate(t *testing.T) { "success: multiple port channel pair denom", Token{ Denom: Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Base: "atom", + Trace: []Trace{ + NewTrace("transfer", "channel-0"), + NewTrace("transfer", "channel-1"), + }, }, Amount: amount, }, @@ -93,8 +44,10 @@ func TestValidate(t *testing.T) { "success: one port channel pair denom", Token{ Denom: Denom{ - Base: "uatom", - Trace: []string{"transfer/channel-1"}, + Base: "uatom", + Trace: []Trace{ + NewTrace("transfer", "channel-1"), + }, }, Amount: amount, }, @@ -104,8 +57,12 @@ func TestValidate(t *testing.T) { "success: non transfer port trace", Token{ Denom: Denom{ - Base: "uatom", - Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + Base: "uatom", + Trace: []Trace{ + NewTrace("transfer", "channel-0"), + NewTrace("transfer", "channel-1"), + NewTrace("transfer-custom", "channel-2"), + }, }, Amount: amount, }, @@ -123,8 +80,11 @@ func TestValidate(t *testing.T) { "failure: invalid amount string", Token{ Denom: Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Base: "atom", + Trace: []Trace{ + NewTrace("transfer", "channel-0"), + NewTrace("transfer", "channel-1"), + }, }, Amount: "value", }, @@ -134,8 +94,11 @@ func TestValidate(t *testing.T) { "failure: amount is zero", Token{ Denom: Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Base: "atom", + Trace: []Trace{ + NewTrace("transfer", "channel-0"), + NewTrace("transfer", "channel-1"), + }, }, Amount: "0", }, @@ -145,8 +108,11 @@ func TestValidate(t *testing.T) { "failure: amount is negative", Token{ Denom: Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Base: "atom", + Trace: []Trace{ + NewTrace("transfer", "channel-0"), + NewTrace("transfer", "channel-1"), + }, }, Amount: "-1", }, @@ -156,8 +122,11 @@ func TestValidate(t *testing.T) { "failure: invalid identifier in trace", Token{ Denom: Denom{ - Base: "uatom", - Trace: []string{"transfer/channel-1", "randomport"}, + Base: "uatom", + Trace: []Trace{ + NewTrace("transfer", "channel-1"), + NewTrace("randomport", ""), + }, }, Amount: amount, }, @@ -168,7 +137,7 @@ func TestValidate(t *testing.T) { Token{ Denom: Denom{ Base: "uatom", - Trace: []string{""}, + Trace: []Trace{Trace{}}, }, Amount: amount, }, @@ -206,7 +175,7 @@ func TestTokens_String(t *testing.T) { Token{ Denom: Denom{ Base: "tree", - Trace: []string{}, + Trace: []Trace{}, }, Amount: "1", }, @@ -218,8 +187,10 @@ func TestTokens_String(t *testing.T) { Tokens{ Token{ Denom: Denom{ - Base: "tree", - Trace: []string{"portid/channelid"}, + Base: "tree", + Trace: []Trace{ + NewTrace("portid", "channelid"), + }, }, Amount: "1", }, @@ -231,22 +202,19 @@ func TestTokens_String(t *testing.T) { Tokens{ Token{ Denom: Denom{ - Base: "tree", - Trace: []string{}, + Base: "tree", }, Amount: "1", }, Token{ Denom: Denom{ - Base: "gas", - Trace: []string{}, + Base: "gas", }, Amount: "2", }, Token{ Denom: Denom{ - Base: "mineral", - Trace: []string{}, + Base: "mineral", }, Amount: "3", }, @@ -258,22 +226,26 @@ func TestTokens_String(t *testing.T) { Tokens{ Token{ Denom: Denom{ - Base: "tree", - Trace: []string{}, + Base: "tree", }, Amount: "1", }, Token{ Denom: Denom{ - Base: "gas", - Trace: []string{"portid/channelid"}, + Base: "gas", + Trace: []Trace{ + NewTrace("portid", "channelid"), + }, }, Amount: "2", }, Token{ Denom: Denom{ - Base: "mineral", - Trace: []string{"portid/channelid", "transfer/channel-52"}, + Base: "mineral", + Trace: []Trace{ + NewTrace("portid", "channelid"), + NewTrace("transfer", "channel-52"), + }, }, Amount: "3", }, diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 2d18dc0e760..44052a8f2e3 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -17,6 +17,35 @@ import ( host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ) +// NewTrace returns a Trace type +func NewTrace(portID, channelID string) Trace { + return Trace{ + PortId: portID, + ChannelId: channelID, + } +} + +// Validate does basic validation of the trace portID and channelID. +func (t Trace) Validate() error { + if err := host.PortIdentifierValidator(t.PortId); err != nil { + return errorsmod.Wrapf(err, "invalid portID") + } + if err := host.ChannelIdentifierValidator(t.ChannelId); err != nil { + return errorsmod.Wrapf(err, "invalid channelID") + } + return nil +} + +// String returns the Trace the format: +// / +func (t Trace) String() string { + var sb strings.Builder + sb.WriteString(t.PortId) // nolint:revive // no error returned by WriteString + sb.WriteByte('/') //nolint:revive // no error returned by WriteByte + sb.WriteString(t.ChannelId) // nolint:revive + return sb.String() +} + // ParseDenomTrace parses a string with the ibc prefix (denom trace) and the base denomination // into a DenomTrace type. // diff --git a/proto/ibc/applications/transfer/v1/query.proto b/proto/ibc/applications/transfer/v1/query.proto index 697df1462ff..31df879f6af 100644 --- a/proto/ibc/applications/transfer/v1/query.proto +++ b/proto/ibc/applications/transfer/v1/query.proto @@ -15,14 +15,14 @@ service Query { // DenomTraces queries all denomination traces. // Deprecated: Please use the Denoms endpoint instead. rpc DenomTraces(QueryDenomTracesRequest) returns (QueryDenomTracesResponse) { - option deprecated = true; + option deprecated = true; option (google.api.http).get = "/ibc/apps/transfer/v1/denom_traces"; } // DenomTrace queries a denomination trace information. // Deprecated: Please see the Denom endpoint instead. rpc DenomTrace(QueryDenomTraceRequest) returns (QueryDenomTraceResponse) { - option deprecated = true; + option deprecated = true; option (google.api.http).get = "/ibc/apps/transfer/v1/denom_traces/{hash=**}"; } diff --git a/proto/ibc/applications/transfer/v2/token.proto b/proto/ibc/applications/transfer/v2/token.proto index 03c54d4f2ae..3ab833fa7fc 100644 --- a/proto/ibc/applications/transfer/v2/token.proto +++ b/proto/ibc/applications/transfer/v2/token.proto @@ -19,13 +19,14 @@ message Denom { // the base token denomination string base = 1; // the trace of the token - repeated string trace = 3; + repeated Trace trace = 3 [(gogoproto.nullable) = false]; } // Trace represents the portID and channelID the token arrived through. // When a token is sent to a new chain, the portID and channelID of the // destination chain are added to a token's trace. message Trace { - string port_id = 1; - string channel_id = 2; + option (gogoproto.goproto_stringer) = false; + string port_id = 1; + string channel_id = 2; } From 5390c3605db962894dccc572d40f779661b0366d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 11:53:35 +0200 Subject: [PATCH 2/9] fix: build --- modules/apps/transfer/ibc_module_test.go | 10 ++-- modules/apps/transfer/keeper/genesis_test.go | 16 +++--- .../apps/transfer/keeper/grpc_query_test.go | 39 ++++++++------ .../apps/transfer/keeper/mbt_relay_test.go | 3 +- modules/apps/transfer/keeper/relay_test.go | 26 ++++++---- modules/apps/transfer/transfer_test.go | 9 ++-- modules/apps/transfer/types/denom_test.go | 16 +++--- modules/apps/transfer/types/packet.go | 3 +- modules/apps/transfer/types/packet_test.go | 51 +++++++++++-------- modules/apps/transfer/types/trace.go | 31 ++--------- modules/apps/transfer/types/trace_test.go | 31 ----------- 11 files changed, 106 insertions(+), 129 deletions(-) diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index be2db2c261d..49b2ac9078a 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -556,8 +556,10 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { Tokens: []types.Token{ { Denom: types.Denom{ - Base: "atom", - Trace: []string{"transfer/channel-0"}, + Base: "atom", + Trace: []types.Trace{ + types.NewTrace("transfer", "channel-0"), + }, }, Amount: ibctesting.TestCoin.Amount.String(), }, @@ -603,7 +605,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { { Denom: types.Denom{ Base: ibctesting.TestCoin.Denom, - Trace: []string{""}, + Trace: []types.Trace{types.Trace{}}, }, Amount: ibctesting.TestCoin.Amount.String(), }, @@ -615,7 +617,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes() }, - errors.New("trace info must come in pairs of port and channel identifiers"), + errors.New("nvalid token denom: invalid trace: invalid portID: identifier cannot be blank: invalid identifier"), }, { "failure: invalid packet data", diff --git a/modules/apps/transfer/keeper/genesis_test.go b/modules/apps/transfer/keeper/genesis_test.go index 4fb809e58bb..f358dbdbc7a 100644 --- a/modules/apps/transfer/keeper/genesis_test.go +++ b/modules/apps/transfer/keeper/genesis_test.go @@ -11,22 +11,22 @@ import ( ) func (suite *KeeperTestSuite) TestGenesis() { - getTrace := func(index uint) string { - return fmt.Sprintf("transfer/channelToChain%d", index) + getTrace := func(index uint) types.Trace { + return types.NewTrace("transfer", fmt.Sprintf("channelToChain%d", index)) } var ( denoms types.Denoms escrows sdk.Coins traceAndEscrowAmounts = []struct { - trace []string + trace []types.Trace escrow string }{ - {[]string{getTrace(0)}, "10"}, - {[]string{getTrace(1), getTrace(0)}, "100000"}, - {[]string{getTrace(2), getTrace(1), getTrace(0)}, "10000000000"}, - {[]string{getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "1000000000000000"}, - {[]string{getTrace(4), getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "100000000000000000000"}, + {[]types.Trace{getTrace(0)}, "10"}, + {[]types.Trace{getTrace(1), getTrace(0)}, "100000"}, + {[]types.Trace{getTrace(2), getTrace(1), getTrace(0)}, "10000000000"}, + {[]types.Trace{getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "1000000000000000"}, + {[]types.Trace{getTrace(4), getTrace(3), getTrace(2), getTrace(1), getTrace(0)}, "100000000000000000000"}, } ) diff --git a/modules/apps/transfer/keeper/grpc_query_test.go b/modules/apps/transfer/keeper/grpc_query_test.go index 802515e85b8..0c8a1def34f 100644 --- a/modules/apps/transfer/keeper/grpc_query_test.go +++ b/modules/apps/transfer/keeper/grpc_query_test.go @@ -27,9 +27,11 @@ func (suite *KeeperTestSuite) TestQueryDenom() { "success: correct ibc denom", func() { expDenom = types.Denom{ - Base: "uatom", //nolint:goconst - Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst - } + Base: "uatom", //nolint:goconst + Trace: []types.Trace{ + types.NewTrace("transfer", "channelToA"), //nolint:goconst + types.NewTrace("transfer", "channelToB"), //nolint:goconst + }} suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom) req = &types.QueryDenomRequest{ @@ -42,9 +44,11 @@ func (suite *KeeperTestSuite) TestQueryDenom() { "success: correct hex hash", func() { expDenom = types.Denom{ - Base: "uatom", //nolint:goconst - Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst - } + Base: "uatom", //nolint:goconst + Trace: []types.Trace{ + types.NewTrace("transfer", "channelToA"), //nolint:goconst + types.NewTrace("transfer", "channelToB"), //nolint:goconst + }} suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom) @@ -67,9 +71,11 @@ func (suite *KeeperTestSuite) TestQueryDenom() { "failure: not found denom trace", func() { expDenom = types.Denom{ - Base: "uatom", //nolint:goconst - Trace: []string{"transfer/channelToA", "transfer/channelToB"}, //nolint:goconst - } + Base: "uatom", //nolint:goconst + Trace: []types.Trace{ + types.NewTrace("transfer", "channelToA"), //nolint:goconst + types.NewTrace("transfer", "channelToB"), //nolint:goconst + }} req = &types.QueryDenomRequest{ Hash: expDenom.IBCDenom(), @@ -122,8 +128,8 @@ func (suite *KeeperTestSuite) TestQueryDenoms() { "success", func() { expDenoms = append(expDenoms, types.Denom{Base: "uatom"}) - expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []string{"transfer/channelToB"}}) - expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []string{"transfer/channelToA", "transfer/channelToB"}}) + expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channelToB")}}) + expDenoms = append(expDenoms, types.Denom{Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channelToA"), types.NewTrace("transfer", "channelToB")}}) for _, trace := range expDenoms { suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), trace) @@ -170,8 +176,11 @@ func (suite *KeeperTestSuite) TestQueryParams() { func (suite *KeeperTestSuite) TestQueryDenomHash() { reqDenom := types.Denom{ - Base: "uatom", - Trace: []string{"transfer/channelToA", "transfer/channelToB"}, + Base: "uatom", + Trace: []types.Trace{ + types.NewTrace("transfer", "channelToA"), + types.NewTrace("transfer", "channelToB"), + }, } var ( @@ -336,7 +345,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() { func() { denomTrace := types.Denom{ Base: sdk.DefaultBondDenom, - Trace: []string{"transfer/channel-0"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, } suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denomTrace) @@ -355,7 +364,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() { func() { denomTrace := types.Denom{ Base: sdk.DefaultBondDenom, - Trace: []string{"transfer/channel-0"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, } req = &types.QueryTotalEscrowForDenomRequest{ diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index e074b408ae9..231aca98c43 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -341,8 +341,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { panic(errors.New("MBT failed to convert sender address")) } registerDenomFn() - denomTrace := types.ParseDenomTrace(tc.packet.Data.Tokens[0].GetFullDenomPath()) - denom := denomTrace.IBCDenom() + denom := tc.packet.Data.Tokens[0].Denom.IBCDenom() err = sdk.ValidateDenom(denom) if err == nil { amount, ok := sdkmath.NewIntFromString(tc.packet.Data.Tokens[0].Amount) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 34ddaa54ffe..c3169a6edee 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -56,8 +56,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() { func() { // send IBC token back to chainB denom := types.Denom{ - Base: coin.Denom, - Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID}, + Base: coin.Denom, + Trace: []types.Trace{ + types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), + }, } coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount) }, @@ -68,8 +70,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() { func() { // send IBC token back to chainB denom := types.Denom{ - Base: coin.Denom, - Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID}, + Base: coin.Denom, + Trace: []types.Trace{ + types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), + }, } coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount) memo = "memo" @@ -102,8 +106,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() { "failure: denom trace not found", func() { denom := types.Denom{ - Base: "randomdenom", - Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID}, + Base: "randomdenom", + Trace: []types.Trace{ + types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), + }, } coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount) }, @@ -113,8 +119,10 @@ func (suite *KeeperTestSuite) TestSendTransfer() { "failure: bank send from module account failed, insufficient balance", func() { denom := types.Denom{ - Base: coin.Denom, - Trace: []string{path.EndpointA.ChannelConfig.PortID + "/" + path.EndpointA.ChannelID}, + Base: coin.Denom, + Trace: []types.Trace{ + types.NewTrace(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), + }, } coin = sdk.NewCoin(denom.IBCDenom(), coin.Amount.Add(sdkmath.NewInt(1))) }, @@ -374,7 +382,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket_ReceiverIsNotSource() { { Denom: types.Denom{ Base: sdk.DefaultBondDenom, - Trace: []string{}, + Trace: []types.Trace{}, }, Amount: amount.String(), }, diff --git a/modules/apps/transfer/transfer_test.go b/modules/apps/transfer/transfer_test.go index 3a0596d0bde..f2777e293cf 100644 --- a/modules/apps/transfer/transfer_test.go +++ b/modules/apps/transfer/transfer_test.go @@ -1,7 +1,6 @@ package transfer_test import ( - "fmt" "testing" testifysuite "github.com/stretchr/testify/suite" @@ -71,8 +70,10 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { // check that voucher exists on chain B denom := types.Denom{ - Base: sdk.DefaultBondDenom, - Trace: []string{fmt.Sprintf("%s/%s", packet.DestinationPort, packet.DestinationChannel)}, + Base: sdk.DefaultBondDenom, + Trace: []types.Trace{ + types.NewTrace(packet.DestinationPort, packet.DestinationChannel), + }, } balance = suite.chainB.GetSimApp().BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), denom.IBCDenom()) coinSentFromAToB := sdk.NewCoin(denom.IBCDenom(), amount) @@ -97,7 +98,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { suite.Require().NoError(err) // relay committed // NOTE: fungible token is prefixed with the full trace in order to verify the packet commitment - trace := []string{fmt.Sprintf("%s/%s", pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID)} + trace := []types.Trace{types.NewTrace(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID)} denom.Trace = append(trace, denom.Trace...) // check that the balance is updated on chainC diff --git a/modules/apps/transfer/types/denom_test.go b/modules/apps/transfer/types/denom_test.go index 785c3f2fbee..5d1f9004d1c 100644 --- a/modules/apps/transfer/types/denom_test.go +++ b/modules/apps/transfer/types/denom_test.go @@ -20,21 +20,21 @@ func (suite *TypesTestSuite) TestDenomsValidate() { { "valid multiple trace info", types.Denoms{ - {Base: "uatom", Trace: []string{"transfer/channel-1", "transfer/channel-2"}}, + {Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channel-1"), types.NewTrace("transfer", "channel-2")}}, }, nil, }, { "valid multiple trace info", types.Denoms{ - {Base: "uatom", Trace: []string{"transfer/channel-1", "transfer/channel-2"}}, - {Base: "uatom", Trace: []string{"transfer/channel-1", "transfer/channel-2"}}, + {Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channel-1"), types.NewTrace("transfer", "channel-2")}}, + {Base: "uatom", Trace: []types.Trace{types.NewTrace("transfer", "channel-1"), types.NewTrace("transfer", "channel-2")}}, }, fmt.Errorf("duplicated denomination with hash"), }, { "empty base denom with trace", - types.Denoms{{Base: "", Trace: []string{"transfer/channel-1"}}}, + types.Denoms{{Base: "", Trace: []types.Trace{types.NewTrace("transfer", "channel-1")}}}, fmt.Errorf("base denomination cannot be blank"), }, } @@ -81,7 +81,7 @@ func (suite *TypesTestSuite) TestFullPath() { "1 hop denom", types.Denom{ Base: "uatom", - Trace: []string{"transfer/channel-0"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, }, "transfer/channel-0/uatom", }, @@ -89,7 +89,7 @@ func (suite *TypesTestSuite) TestFullPath() { "2 hop denom", types.Denom{ Base: "uatom", - Trace: []string{"transfer/channel-0", "transfer/channel-52"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52")}, }, "transfer/channel-0/transfer/channel-52/uatom", }, @@ -97,7 +97,7 @@ func (suite *TypesTestSuite) TestFullPath() { "3 hop denom", types.Denom{ Base: "uatom", - Trace: []string{"transfer/channel-0", "transfer/channel-52", "transfer/channel-52"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52")}, }, "transfer/channel-0/transfer/channel-52/transfer/channel-52/uatom", }, @@ -105,7 +105,7 @@ func (suite *TypesTestSuite) TestFullPath() { "4 hop denom with base denom slashes", types.Denom{ Base: "other-denom/", - Trace: []string{"transfer/channel-0", "transfer/channel-52", "transfer/channel-52", "transfer/channel-49"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-49")}, }, "transfer/channel-0/transfer/channel-52/transfer/channel-52/transfer/channel-49/other-denom/", }, diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index e706084d56a..3eea8d5724d 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -49,7 +49,8 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { if strings.TrimSpace(ftpd.Receiver) == "" { return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "receiver address cannot be blank") } - return ValidatePrefixedDenom(ftpd.Denom) + denom := ExtractDenomFromFullPath(ftpd.Denom) + return denom.Validate() } // GetBytes is a helper for serialising the packet to bytes. diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index bc80cae4061..d17206a1b24 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -27,9 +27,18 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { expPass bool }{ {"valid packet", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, ""), true}, + {"valid packet, base denom with leading slash", types.NewFungibleTokenPacketData("transfer/channel-1/uatom/", amount, sender, receiver, ""), true}, + {"valid packet, prefixed down with '/'", types.NewFungibleTokenPacketData("transfer/channel-1/gamm/pool/1", amount, sender, receiver, ""), true}, + {"valid packet, no prefix", types.NewFungibleTokenPacketData("/uatom", amount, sender, receiver, ""), true}, + {"valid packet, empty identifier prefix", types.NewFungibleTokenPacketData("//uatom", amount, sender, receiver, ""), true}, + {"valid packet, only base denom", types.NewFungibleTokenPacketData("uatom", amount, sender, receiver, ""), true}, + {"valid packet, base denom with single '/'", types.NewFungibleTokenPacketData("erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", amount, sender, receiver, ""), true}, + {"valid packet, base denom with multiple '/'", types.NewFungibleTokenPacketData("gamm/pool/1", amount, sender, receiver, ""), true}, + {"valid packet, single trace identifier", types.NewFungibleTokenPacketData("transfer/", amount, sender, receiver, ""), true}, {"valid packet with memo", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, "memo"), true}, {"valid packet with large amount", types.NewFungibleTokenPacketData(denom, largeAmount, sender, receiver, ""), true}, {"invalid denom", types.NewFungibleTokenPacketData("", amount, sender, receiver, ""), false}, + {"invalid denom, invalid portID", types.NewFungibleTokenPacketData("(tranfer)/channel-1/uatom", amount, sender, receiver, ""), false}, {"invalid empty amount", types.NewFungibleTokenPacketData(denom, "", sender, receiver, ""), false}, {"invalid zero amount", types.NewFungibleTokenPacketData(denom, "0", sender, receiver, ""), false}, {"invalid negative amount", types.NewFungibleTokenPacketData(denom, "-1", sender, receiver, ""), false}, @@ -176,7 +185,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -194,7 +203,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -212,7 +221,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: largeAmount, }, @@ -230,7 +239,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: "", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -248,7 +257,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: "", }, @@ -276,7 +285,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: "0", }, @@ -294,7 +303,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: "-100", }, @@ -312,7 +321,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: invalidLargeAmount, }, @@ -330,7 +339,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -348,7 +357,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -366,7 +375,7 @@ func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: largeAmount, }, @@ -406,7 +415,7 @@ func TestGetPacketSender(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -424,7 +433,7 @@ func TestGetPacketSender(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -457,7 +466,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -477,7 +486,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -497,7 +506,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -514,7 +523,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -531,7 +540,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -548,7 +557,7 @@ func TestPacketDataProvider(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -581,7 +590,7 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, @@ -599,7 +608,7 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { { Denom: types.Denom{ Base: denom, - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-1")}, }, Amount: amount, }, diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 6839dc7a62b..f327d409d3e 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -58,8 +58,10 @@ func (t Trace) String() string { // - "uatom" => DenomTrace{Path: "", BaseDenom: "uatom"} func ParseDenomTrace(rawDenom string) DenomTrace { denom := ExtractDenomFromFullPath(rawDenom) + path := denom.FullPath() + path = strings.TrimSuffix(path, denom.Base) return DenomTrace{ - Path: strings.Join(denom.Trace, "/"), + Path: path, BaseDenom: denom.Base, } } @@ -108,7 +110,7 @@ func ExtractDenomFromFullPath(fullPath string) Denom { } var ( - trace []string + trace []Trace baseDenomSlice []string ) @@ -124,7 +126,7 @@ func ExtractDenomFromFullPath(fullPath string) Denom { // as an IBC denomination. The hash used to store the token internally on our chain // will be the same value as the base denomination being correctly parsed. if i < length-1 && length > 2 && channeltypes.IsValidChannelID(denomSplit[i+1]) { - trace = append(trace, denomSplit[i]+"/"+denomSplit[i+1]) + trace = append(trace, NewTrace(denomSplit[i], denomSplit[i+1])) } else { baseDenomSlice = denomSplit[i:] break @@ -173,29 +175,6 @@ func (dt DenomTrace) Validate() error { return validateTraceIdentifiers(identifiers) } -// ValidatePrefixedDenom checks that the denomination for an IBC fungible token packet denom is correctly prefixed. -// The function will return no error if the given string follows one of the two formats: -// -// - Prefixed denomination: '{portIDN}/{channelIDN}/.../{portID0}/{channelID0}/baseDenom' -// - Unprefixed denomination: 'baseDenom' -// -// 'baseDenom' may or may not contain '/'s -func ValidatePrefixedDenom(fullPath string) error { - denom := ExtractDenomFromFullPath(fullPath) - if strings.TrimSpace(denom.Base) == "" { - return errorsmod.Wrap(ErrInvalidDenomForTransfer, "base denomination cannot be blank") - } - - path := strings.Join(denom.Trace, "/") - if len(denom.Trace) != 0 { - identifiers := strings.Split(path, "/") - return validateTraceIdentifiers(identifiers) - - } - - return nil -} - // validateIBCDenom validates that the given denomination is either: // // - A valid base denomination (eg: 'uatom' or 'gamm/pool/1' as in https://github.com/cosmos/ibc-go/issues/894) diff --git a/modules/apps/transfer/types/trace_test.go b/modules/apps/transfer/types/trace_test.go index 40ad3037586..238a701ff5f 100644 --- a/modules/apps/transfer/types/trace_test.go +++ b/modules/apps/transfer/types/trace_test.go @@ -94,37 +94,6 @@ func TestDenomTrace_Validate(t *testing.T) { } } -func TestValidatePrefixedDenom(t *testing.T) { - testCases := []struct { - name string - denom string - expError bool - }{ - {"prefixed denom", "transfer/channel-1/uatom", false}, - {"prefixed denom with base denom with leading slash", "transfer/channel-1/uatom/", false}, - {"prefixed denom with '/'", "transfer/channel-1/gamm/pool/1", false}, - {"empty prefix", "/uatom", false}, - {"empty identifiers", "//uatom", false}, - {"base denom", "uatom", false}, - {"base denom with single '/'", "erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", false}, - {"base denom with multiple '/'s", "gamm/pool/1", false}, - {"single trace identifier", "transfer/", false}, - {"invalid port ID", "(transfer)/channel-1/uatom", true}, - {"empty denom", "", true}, - } - - for _, tc := range testCases { - tc := tc - - err := types.ValidatePrefixedDenom(tc.denom) - if tc.expError { - require.Error(t, err, tc.name) - } else { - require.NoError(t, err, tc.name) - } - } -} - func TestValidateIBCDenom(t *testing.T) { testCases := []struct { name string From 5883616c33b9f8e7bdad719f1a332b732363cad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 12:27:08 +0200 Subject: [PATCH 3/9] fix: types test --- modules/apps/transfer/types/token_test.go | 8 ++++---- modules/apps/transfer/types/trace.go | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/apps/transfer/types/token_test.go b/modules/apps/transfer/types/token_test.go index e9677a76508..b0affe22d8b 100644 --- a/modules/apps/transfer/types/token_test.go +++ b/modules/apps/transfer/types/token_test.go @@ -130,7 +130,7 @@ func TestValidate(t *testing.T) { }, Amount: amount, }, - fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), + fmt.Errorf("invalid token denom: invalid trace: invalid channelID: identifier cannot be blank: invalid identifier"), }, { "failure: empty identifier in trace", @@ -141,7 +141,7 @@ func TestValidate(t *testing.T) { }, Amount: amount, }, - fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: "), + fmt.Errorf("invalid token denom: invalid trace: invalid portID: identifier cannot be blank: invalid identifier"), }, } @@ -195,7 +195,7 @@ func TestTokens_String(t *testing.T) { Amount: "1", }, }, - `denom: amount:"1" `, + `denom: > amount:"1" `, }, { "multiple tokens, no trace", @@ -250,7 +250,7 @@ func TestTokens_String(t *testing.T) { Amount: "3", }, }, - `denom: amount:"1" ,denom: amount:"2" ,denom: amount:"3" `, + `denom: amount:"1" ,denom: > amount:"2" ,denom: trace: > amount:"3" `, }, } diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index f327d409d3e..c9a97416354 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -58,8 +58,11 @@ func (t Trace) String() string { // - "uatom" => DenomTrace{Path: "", BaseDenom: "uatom"} func ParseDenomTrace(rawDenom string) DenomTrace { denom := ExtractDenomFromFullPath(rawDenom) - path := denom.FullPath() - path = strings.TrimSuffix(path, denom.Base) + path := "" + if !denom.IsNative() { + path = denom.FullPath() + path = strings.TrimSuffix(path, "/"+denom.Base) + } return DenomTrace{ Path: path, BaseDenom: denom.Base, From ac1d484f66ad9051ebe19a2ed3ab56223b2ced3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 12:33:40 +0200 Subject: [PATCH 4/9] fix: callbacks test --- modules/apps/callbacks/ibc_middleware_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 98a31b79918..53687fed3c7 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -169,7 +169,7 @@ func (s *CallbacksTestSuite) TestSendPacket() { { Denom: transfertypes.Denom{ Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Trace: []transfertypes.Trace{}, }, Amount: ibctesting.TestCoin.Amount.String(), }, @@ -313,7 +313,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { { Denom: transfertypes.Denom{ Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Trace: []transfertypes.Trace{}, }, Amount: ibctesting.TestCoin.Amount.String(), }, @@ -648,7 +648,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { { Denom: transfertypes.Denom{ Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Trace: []transfertypes.Trace{}, }, Amount: ibctesting.TestCoin.Amount.String(), }, @@ -782,7 +782,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() { { Denom: transfertypes.Denom{ Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Trace: []transfertypes.Trace{}, }, Amount: ibctesting.TestCoin.Amount.String(), }, From a269d93d74946dc808469afac926c66eb2a22b44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 14:55:07 +0200 Subject: [PATCH 5/9] Update modules/apps/transfer/ibc_module_test.go Co-authored-by: DimitrisJim --- modules/apps/transfer/ibc_module_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 49b2ac9078a..d195a039da3 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -617,7 +617,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes() }, - errors.New("nvalid token denom: invalid trace: invalid portID: identifier cannot be blank: invalid identifier"), + errors.New("invalid token denom: invalid trace: invalid portID: identifier cannot be blank: invalid identifier"), }, { "failure: invalid packet data", From 67abac325d78fb0e7ee91247d4e725fd9f770847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 15:00:25 +0200 Subject: [PATCH 6/9] review: apply suggestion --- modules/apps/transfer/types/trace.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index c9a97416354..6e2dec0848d 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -39,11 +39,7 @@ func (t Trace) Validate() error { // String returns the Trace the format: // / func (t Trace) String() string { - var sb strings.Builder - sb.WriteString(t.PortId) // nolint:revive // no error returned by WriteString - sb.WriteByte('/') //nolint:revive // no error returned by WriteByte - sb.WriteString(t.ChannelId) // nolint:revive - return sb.String() + return fmt.Sprintf("%s/%s", t.PortId, t.ChannelId) } // ParseDenomTrace parses a string with the ibc prefix (denom trace) and the base denomination From 369e9f32d34057148e57542b6229019fa7e1e02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 15:01:50 +0200 Subject: [PATCH 7/9] lint --- modules/apps/transfer/ibc_module_test.go | 2 +- modules/apps/transfer/keeper/grpc_query_test.go | 10 ++++++---- modules/apps/transfer/types/token_test.go | 10 +--------- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index d195a039da3..43fae9a3d2b 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -605,7 +605,7 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { { Denom: types.Denom{ Base: ibctesting.TestCoin.Denom, - Trace: []types.Trace{types.Trace{}}, + Trace: []types.Trace{{}}, }, Amount: ibctesting.TestCoin.Amount.String(), }, diff --git a/modules/apps/transfer/keeper/grpc_query_test.go b/modules/apps/transfer/keeper/grpc_query_test.go index 0c8a1def34f..516a2c2692f 100644 --- a/modules/apps/transfer/keeper/grpc_query_test.go +++ b/modules/apps/transfer/keeper/grpc_query_test.go @@ -31,7 +31,8 @@ func (suite *KeeperTestSuite) TestQueryDenom() { Trace: []types.Trace{ types.NewTrace("transfer", "channelToA"), //nolint:goconst types.NewTrace("transfer", "channelToB"), //nolint:goconst - }} + }, + } suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom) req = &types.QueryDenomRequest{ @@ -48,8 +49,8 @@ func (suite *KeeperTestSuite) TestQueryDenom() { Trace: []types.Trace{ types.NewTrace("transfer", "channelToA"), //nolint:goconst types.NewTrace("transfer", "channelToB"), //nolint:goconst - }} - + }, + } suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), expDenom) req = &types.QueryDenomRequest{ @@ -75,7 +76,8 @@ func (suite *KeeperTestSuite) TestQueryDenom() { Trace: []types.Trace{ types.NewTrace("transfer", "channelToA"), //nolint:goconst types.NewTrace("transfer", "channelToB"), //nolint:goconst - }} + }, + } req = &types.QueryDenomRequest{ Hash: expDenom.IBCDenom(), diff --git a/modules/apps/transfer/types/token_test.go b/modules/apps/transfer/types/token_test.go index b0affe22d8b..e7fdc15f982 100644 --- a/modules/apps/transfer/types/token_test.go +++ b/modules/apps/transfer/types/token_test.go @@ -5,9 +5,6 @@ import ( "testing" "github.com/stretchr/testify/require" - - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - sdk "github.com/cosmos/cosmos-sdk/types" ) const ( @@ -15,11 +12,6 @@ const ( amount = "100" ) -var ( - sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - receiver = sdk.AccAddress("testaddr2").String() -) - func TestValidate(t *testing.T) { testCases := []struct { name string @@ -137,7 +129,7 @@ func TestValidate(t *testing.T) { Token{ Denom: Denom{ Base: "uatom", - Trace: []Trace{Trace{}}, + Trace: []Trace{{}}, }, Amount: amount, }, From 4f375307220a39ae8df7369ab286bfce4e36c5be Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 30 May 2024 15:07:46 +0200 Subject: [PATCH 8/9] tiny nit --- modules/apps/transfer/types/trace.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 6e2dec0848d..01f7b3b59a0 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -36,7 +36,7 @@ func (t Trace) Validate() error { return nil } -// String returns the Trace the format: +// String returns the Trace in the format: // / func (t Trace) String() string { return fmt.Sprintf("%s/%s", t.PortId, t.ChannelId) From 19fdd2c75bd61de2975b1859acd6333a246ac55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 30 May 2024 15:10:23 +0200 Subject: [PATCH 9/9] fix: tests from merging main --- modules/apps/transfer/types/packet_test.go | 8 -------- modules/apps/transfer/types/trace_test.go | 9 +++++---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index d17206a1b24..1178d821863 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -27,14 +27,6 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { expPass bool }{ {"valid packet", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, ""), true}, - {"valid packet, base denom with leading slash", types.NewFungibleTokenPacketData("transfer/channel-1/uatom/", amount, sender, receiver, ""), true}, - {"valid packet, prefixed down with '/'", types.NewFungibleTokenPacketData("transfer/channel-1/gamm/pool/1", amount, sender, receiver, ""), true}, - {"valid packet, no prefix", types.NewFungibleTokenPacketData("/uatom", amount, sender, receiver, ""), true}, - {"valid packet, empty identifier prefix", types.NewFungibleTokenPacketData("//uatom", amount, sender, receiver, ""), true}, - {"valid packet, only base denom", types.NewFungibleTokenPacketData("uatom", amount, sender, receiver, ""), true}, - {"valid packet, base denom with single '/'", types.NewFungibleTokenPacketData("erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", amount, sender, receiver, ""), true}, - {"valid packet, base denom with multiple '/'", types.NewFungibleTokenPacketData("gamm/pool/1", amount, sender, receiver, ""), true}, - {"valid packet, single trace identifier", types.NewFungibleTokenPacketData("transfer/", amount, sender, receiver, ""), true}, {"valid packet with memo", types.NewFungibleTokenPacketData(denom, amount, sender, receiver, "memo"), true}, {"valid packet with large amount", types.NewFungibleTokenPacketData(denom, largeAmount, sender, receiver, ""), true}, {"invalid denom", types.NewFungibleTokenPacketData("", amount, sender, receiver, ""), false}, diff --git a/modules/apps/transfer/types/trace_test.go b/modules/apps/transfer/types/trace_test.go index cbe9d6a40e6..23b8950760d 100644 --- a/modules/apps/transfer/types/trace_test.go +++ b/modules/apps/transfer/types/trace_test.go @@ -133,10 +133,11 @@ func TestExtractDenomFromFullPath(t *testing.T) { {"base denom no slashes", "atom", types.Denom{Base: "atom"}}, {"base denom with trailing slash", "atom/", types.Denom{Base: "atom/"}}, {"base denom multiple trailing slash", "foo///bar//baz/atom/", types.Denom{Base: "foo///bar//baz/atom/"}}, - {"ibc denom one hop", "transfer/channel-0/atom", types.Denom{Base: "atom", Trace: []string{"transfer/channel-0"}}}, - {"ibc denom one hop trailing slash", "transfer/channel-0/atom/", types.Denom{Base: "atom/", Trace: []string{"transfer/channel-0"}}}, - {"ibc denom two hops", "transfer/channel-0/transfer/channel-60/atom", types.Denom{Base: "atom", Trace: []string{"transfer/channel-0", "transfer/channel-60"}}}, - {"ibc denom two hops trailing slash", "transfer/channel-0/transfer/channel-60/atom/", types.Denom{Base: "atom/", Trace: []string{"transfer/channel-0", "transfer/channel-60"}}}, + {"ibc denom one hop", "transfer/channel-0/atom", types.Denom{Base: "atom", Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}}}, + {"ibc denom one hop trailing slash", "transfer/channel-0/atom/", types.Denom{Base: "atom/", Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}}}, + {"ibc denom one hop multiple slashes", "transfer/channel-0//at/om/", types.Denom{Base: "/at/om/", Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}}}, + {"ibc denom two hops", "transfer/channel-0/transfer/channel-60/atom", types.Denom{Base: "atom", Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-60")}}}, + {"ibc denom two hops trailing slash", "transfer/channel-0/transfer/channel-60/atom/", types.Denom{Base: "atom/", Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-60")}}}, {"empty prefix", "/uatom", types.Denom{Base: "/uatom"}}, {"empty identifiers", "//uatom", types.Denom{Base: "//uatom"}}, {"base denom with single '/'", "erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", types.Denom{Base: "erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA"}},