ICS721 spec seems not matched with example implementation

[beer-1]

Problem

Spec seems saying camel case, but all cosmos ibc specs are saying like this but using snake case. Please refer below example.

interface NonFungibleTokenPacketData {
  classId: string
  classUri: string
  classData: string
  tokenIds: string[]
  tokenUris: string[]
  tokenData: string[]
  sender: string
  receiver: string
  memo: string
}

Cosmos implementation is using snake case

message NonFungibleTokenPacketData {
  // the class_id of class to be transferred
  string class_id = 1;
  // the class_uri of class to be transferred
  string class_uri = 2;
  // the class_data of class to be transferred
  string class_data = 3;
  // the non fungible tokens to be transferred
  repeated string token_ids = 4;
  // the non fungible tokens's uri to be transferred
  repeated string token_uris = 5;
  // the non fungible tokens's data to be transferred
  repeated string token_data = 6;
  // the sender address
  string sender = 7;
  // the recipient address on the destination chain
  string receiver = 8;
  // optional memo
  string memo = 9;
}

cw-ics721 is using camel case

cw-ics721/packages/ics721-types/src/ibc_types.rs

Line 10 in ad8f7b4

#[serde(rename_all = "camelCase")]

Example

ibc-fee spec is saying

interface Acknowledgement {
    appAcknowledgement: []byte
    forwardRelayerAddress: string
    underlyingAppSuccess: boolean
}

but, actual implementation is using snake case

message IncentivizedAcknowledgement {
  // the underlying app acknowledgement bytes
  bytes app_acknowledgement = 1;
  // the relayer address which submits the recv packet message
  string forward_relayer_address = 2;
  // success flag of the base application callback
  bool underlying_app_success = 3;
}

[jhernandezb]

Even tho the definition of the protobuf is snake case I do believe is working correctly based on these tests

https://github.com/bianjieai/nft-transfer/blob/c409209538452573978f0aea7a2d70badd2fdf76/types/packet_test.go#L105

It's probably using the protobuf tags in the struct by using ProtoJson marshaling instead of Go's default.

https://github.com/bianjieai/nft-transfer/blob/986de3bb91434c9efc49a82544e3ba0b208e19a7/types/packet.pb.go#L28

[beer-1]

Hey @jhernandezb, Oh sorry for late response.

Hmm not sure what you mean, it is actually not working in testing between cosmos nft-transfer module and wasm nft contract.

Currently we fixed this problem by introducing wasm specific conversion in our custom nft module - here and here.

• cosmos IBC module does not using Proto json marshal/unmarshal, but it is using Go's default json marshal/unmarshal in any place.

It does not affect when you transfer wasm<>wasm chain, but it will affect when you try to transfer wasm<>cosmosNFT module.

[jhernandezb]

Hey @jhernandezb, Oh sorry for late response.

Hmm not sure what you mean, it is actually not working in testing between cosmos nft-transfer module and wasm nft contract.

Currently we fixed this problem by introducing wasm specific conversion in our custom nft module - here and here.

• cosmos IBC module does not using Proto json marshal/unmarshal, but it is using Go's default json marshal/unmarshal in any place.

It does not affect when you transfer wasm<>wasm chain, but it will affect when you try to transfer wasm<>cosmosNFT module.

It should be using it see https://github.com/bianjieai/nft-transfer/blob/c409209538452573978f0aea7a2d70badd2fdf76/types/packet.go#L88-L101
GetBytes() invokes the ProtoJson Marshaler and when sending a packet it is using that method
https://github.com/bianjieai/nft-transfer/blob/c409209538452573978f0aea7a2d70badd2fdf76/keeper/relay.go#L89

I see that your module deviates from the iris implementation

Iris

// GetBytes is a helper for serializing
func (nftpd NonFungibleTokenPacketData) GetBytes() []byte {
	// Format will reshape tokenUris and tokenData in NonFungibleTokenPacketData:
	// 1. if tokenUris/tokenData is ["","",""] or [], then set it to nil.
	// 2. if tokenUris/tokenData is ["a","b","c"] or ["a", "", "c"], then keep it.
	// NOTE: Only use this before sending pkg.
	if requireShape(nftpd.TokenUris) {
		nftpd.TokenUris = nil
	}

	if requireShape(nftpd.TokenData) {
		nftpd.TokenData = nil
	}
	return sdk.MustSortJSON(MustProtoMarshalJSON(&nftpd))
}

Initia

// GetBytes is a helper for serializing
func (nftpd NonFungibleTokenPacketData) GetBytes(counterpartyPort string) []byte {
	var bz []byte
	var err error
	if isWasmPacket(counterpartyPort) {
		bz, err = json.Marshal(nftpd.ToWasmData())
	} else {
		bz, err = json.Marshal(nftpd)
	}
	if err != nil {
		panic(err)
	}

	return sdk.MustSortJSON(bz)
}

[jhernandezb]

I think the proto definitions should be using json tags to avoid the confusion ( like you did with NonFungibleTokenPacketDataWasm ) because in order to comply is using the proto json marshaler in the GetBytes() which uses the json tag inside protobuf

// NonFungibleTokenPacketData defines a struct for the packet payload
// See NonFungibleTokenPacketData spec:
// https://github.com/cosmos/ibc/tree/master/spec/app/ics-721-nft-transfer#data-structures
type NonFungibleTokenPacketData struct {
	// the class_id of class to be transferred
	ClassId string `protobuf:"bytes,1,opt,name=class_id,json=classId,proto3" json:"class_id,omitempty"`
	// the class_uri of class to be transferred
	ClassUri string `protobuf:"bytes,2,opt,name=class_uri,json=classUri,proto3" json:"class_uri,omitempty"`
	// the class_data of class to be transferred
	ClassData string `protobuf:"bytes,3,opt,name=class_data,json=classData,proto3" json:"class_data,omitempty"`
	// the non fungible tokens to be transferred
	TokenIds []string `protobuf:"bytes,4,rep,name=token_ids,json=tokenIds,proto3" json:"token_ids,omitempty"`
	// the non fungible tokens's uri to be transferred
	TokenUris []string `protobuf:"bytes,5,rep,name=token_uris,json=tokenUris,proto3" json:"token_uris,omitempty"`
	// the non fungible tokens's data to be transferred
	TokenData []string `protobuf:"bytes,6,rep,name=token_data,json=tokenData,proto3" json:"token_data,omitempty"`
	// the sender address
	Sender string `protobuf:"bytes,7,opt,name=sender,proto3" json:"sender,omitempty"`
	// the recipient address on the destination chain
	Receiver string `protobuf:"bytes,8,opt,name=receiver,proto3" json:"receiver,omitempty"`
	// optional memo
	Memo string `protobuf:"bytes,9,opt,name=memo,proto3" json:"memo,omitempty"`
}

[beer-1]

Okay, seems official ics721 implementation is doing that. probably good to follow this Proto json format to avoid confusion.