tokenbound / iframe

Render your NFT with its TBA.
https://docs.tokenbound.org/iframe
30 stars 55 forks source link

Why is the type of tokenId set as number? What is the reason behind the size limitation in the isTokenId function? #29

Open Stpmyclique opened 10 months ago

Stpmyclique commented 10 months ago

Hi Team,

We have found that the current released code version is not compatible with long tokenId values, such as those used in ENS (Ethereum Name Service). The code treats tokenId as a number type, which seems incorrect in a JavaScript environment.

There is also a configuration option called MAX_TOKEN_ID in the code. Does the current version limit the use of long IDs?

We are currently unsure of the reasoning behind this design decision, but are there any plans to consider compatibility in the future, such as changing the tokenId type to string?

huynhr commented 10 months ago

Hi @Stpmyclique! Thanks for pointing this out.

Originally this iframe was ported over from the sapienz project and we did not anticipate the token_id to be an ENS name yet. However there are plans for that in the future implementations.

As for the MAX_TOKEN_ID, again this iframe was ported over from the sapienz project which has an upper bound number of mintable tokens. We can definetly remove those changes.

Also, this iframe is opensource so feel free to push up changes you don't see fit and we can review them and have them in.

For now, I don't see an issue with changing the token_id to a string type or removing the MAX_TOKEN_ID upper bound.