NFT types suggestions

[giorgionocera]

In the nft module, there is the nft/types/nft.go file, which contains utilities for the nft type.

I have some questions/suggestions for these:

1. As for point 5 in NFT structure #159, why do we use CollectionID, MetadataID, and SequenceNumber for each NFT as the id? Is there a motivation?
2. The two functions IsValidNftId and NftIdToBytes could be merged in a single function with error if the nftID is not valid, since they share a very important part of logic.

go-bitsong/x/nft/types/nft.go

Lines 19 to 52 in 84691c8

	func IsValidNftId(id string) bool {
	splits := strings.Split(id, ":")
	if len(splits) != 3 {
	return false
	}
	collId, err := strconv.Atoi(splits[0])
	if err != nil || collId < 0 {
	return false
	}
	metadataId, err := strconv.Atoi(splits[1])
	if err != nil || metadataId < 0 {
	return false
	}
	seq, err := strconv.Atoi(splits[2])
	if err != nil || seq < 0 {
	return false
	}
	return true
	}
	func NftIdToBytes(id string) []byte {
	splits := strings.Split(id, ":")
	if !IsValidNftId(id) {
	panic(fmt.Sprintf("invalid nft id: %s", id))
	}
	collId, _ := strconv.Atoi(splits[0])
	metadataId, _ := strconv.Atoi(splits[1])
	seq, _ := strconv.Atoi(splits[2])
	return (NFT{
	CollId: uint64(collId),
	MetadataId: uint64(metadataId),
	Seq: uint64(seq),
	}).IdBytes()
	}

[ryusmo]

For 1st one, this was what @angelorc suggested.
For 2nd one, I will make changes to use same codebase.

[ryusmo]

For 2nd one, it's done on this commit.
6afb22c

[giorgionocera]

Yeah, for me, 2nd point is ok after commit 6afb22c.

[giorgionocera]

With respect to the first point

As for point 5 in #159, why do we use CollectionID, MetadataID, and SequenceNumber for each NFT as the id? Is there a motivation?

I talked with @angelorc and he said that such a suggestion was valid at the beginning. But giving a look to the other implementation, it could be better to use a global ID for metadata and a sort of "address" (a hash) for the collection.