Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AINFT contract template #1
base: develop
Are you sure you want to change the base?
AINFT contract template #1
Changes from 12 commits
24861dd
6fd7788
b65bc07
b9845aa
7fff439
c0b26a2
9d59358
3242397
1114c2b
8112374
30b6c89
e41e592
c0f57dc
14836e5
2dc9ca2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should emit
BatchMetadataUpdate(start, end)
with counter's help?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered it before. However the
setBaseURI()
call occurs after the someone callsupdateTokenURI()
, the baseURI could be ignored when callingtokenURI()
. I will leave it behind, with remaining TODO commentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MetadataUpdate
/BatchMetadataUpdate
is for the NFT marketplaces or other 3rd parties, so I think this should be emitted on here.If not, we don't have to implement the ERC-4906 at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of ERC721 contracts set the
baseURI
in initialization phase and no implementation forsetBaseURI()
. In the perspective of structuring the base template,setBaseURI()
don't need to be necessary. However, the versioning of metadata is important concept of template and it'd better to put ERC4906 inside the template.As a alternative way, I'm trying to add pre-condition check of
setBaseURI()
: if the number of having been calledupdateTokenURI()
is equal or more than once,setBaseURI()
cannot be executed. In this case, theBatchMetadataUpdate
can be added.What do you think of this way?