bitsongofficial
SetNFT verification
In nft module, inside the SetNFT method, a verification on the owner address is done to verify it is a valid one.
I think this control should be done where nft is passed to the SetNFT method.
For example, IMHO it should not be done at line 71
https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/nft/keeper/nft.go#L60-L76
but should be done inside the CreateNFT method.
Anyway, if there exists a reason to put this check inside the SetNFT Function, I think it should be moved at the beginning of the method, so as to avoid any waste of computation if the nft owner is not valid.
What do you think?
Such a behavior is also done on the DeleteNFT method, but I asked you for more information about this method on issue #169.
SetNFT is used on several places and I think it would be not ideal to write same check on several places.
DeleteNFT is used for internally when resetting NFT data, but this can be exposed via MsgServer to burn nft,
DeleteNFT is used for internally when resetting NFT data, but this can be exposed via MsgServer to burn nft,
I think that only DeleteNFT is not enough to burn NFTs.
SetNFT is used on several places and I think it would be not ideal to write same check on several places.
I gave a look and it seems to not be used in so many places. Moreover, where it is used, we used the logic to perform other checks, so it maybe could be better to group them.
But maybe we can ask to @angelorc