go-bitsong icon indicating copy to clipboard operation
go-bitsong copied to clipboard

NFT types suggestions

Open giorgionocera opened this issue 3 years ago • 4 comments

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 #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.

https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/nft/types/nft.go#L19-L52

giorgionocera avatar Oct 02 '22 10:10 giorgionocera

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

ryusmo avatar Oct 03 '22 08:10 ryusmo

For 2nd one, it's done on this commit. https://github.com/bitsongofficial/go-bitsong/commit/6afb22c08474840168f31eed12907efc1d92bc3a

ryusmo avatar Oct 03 '22 09:10 ryusmo

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

giorgionocera avatar Oct 03 '22 10:10 giorgionocera

With respect to the first point

As for point 5 in https://github.com/bitsongofficial/go-bitsong/issues/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.

giorgionocera avatar Oct 06 '22 09:10 giorgionocera