Support custom URI's for ERC721WithData Contract #60
Conversation
The ERC721WithData contract now supports custom token URI's on mint. - constructor now takes an optional baseURI string - new MintWithURI method that sets the URI for the provided token id Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
shorsher
requested review from
awrichar,
peterbroadhurst,
nguyer and
dechdev
as code owners
| @@ -2,9 +2,10 @@ | |||
|
|
|||
| pragma solidity ^0.8.0; | |||
|
|
|||
There was a problem hiding this comment.
I wonder if "NoData" and "WithData" are not the best names at this point. Really "NoData" is "the minimum viable ERC implementation that can still work somewhat with FireFly" and "WithData" is "all the extra recommended stuff for full FireFly functionality".
There was a problem hiding this comment.
Like @jimthematrix suggested, we can turn the NoData contracts into just ERC20 and ERC721. What are thoughts for the WithData contracts being called ERC721FireFly? 🤷🏻
There was a problem hiding this comment.
still prefer "WithData" since it's more specific to the difference compared to nodata. plus giving it the "firefly" suffix makes it sounds as though they are supposed to be the contract of choice for using with FireFly
There was a problem hiding this comment.
Thanks for the feedback guys, I've wrote up your comments in this issue: #63. Going to go ahead and merge this PR.
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
shorsher
changed the title
- Added new TokenFactory interface for ERC165 support - Updated ERC721WithData interace to include MintWithURI Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
|
Thanks for all the comments! Could use a double check on my interface updates for ERC165 and there's an open question around contract naming. |
|
Note: Discussion for changing the contract names has been moved to #63. |
| address to, | ||
| uint256 tokenId, | ||
| bytes calldata data, | ||
| string memory tokenURI_ |
There was a problem hiding this comment.
Should data be the last parameter to match other methods?
| string memory name, | ||
| string memory symbol, | ||
| bool is_fungible, | ||
| bytes calldata data, |
The ERC721WithData contract now supports custom token URI's on mint.