Conversation
src/NFATRedeemer.sol
Outdated
| /// @param tokenId The NFAT to fund | ||
| /// @param amount The amount of sUSDS to deposit | ||
| function fund(uint256 tokenId, uint256 amount) external roleAuth notStopped { | ||
| function fund(uint256 tokenId, uint256 amount) external { |
There was a problem hiding this comment.
Might be worth pointing out that if the NFAT is put on sale while funded[tokenId] > 0, the seller could frontrun the buyer to redeem the funds before the trade confirms.
Accruing the funds to the tokenId is still probably a better design than accruing the funds to ownerOf(tokenId) as that would prevent the NFAT from being held by a contract as collateral.
src/NFATFacility.sol
Outdated
|
|
||
| bool public whitelistEnabled; | ||
| mapping(address account => bool allowed) public whitelist; | ||
| string public name; |
There was a problem hiding this comment.
tokenURI is optional for ERC721. Currently this (and Archon's) implementation returns the empty string for tokenURI. OZ gives us a baseURI string which we could opt to make file-able, in which case tokenURI will return concat(baseURI, tokenID) once baseURI is set. This could increase the chance of the NFAT being compatible with existing NFT exchanges and protocols.
| function claim(address target, uint256 amount) external roleAuth notStopped { | ||
| /// @param amount The amount of gem to issue | ||
| /// @param tokenId The token ID for the new NFAT | ||
| function issue(address target, uint256 amount, uint256 tokenId) external toll notStopped { |
There was a problem hiding this comment.
One question is whether we want to disallow minting tokenId == 0. Not an issue in our code but might help prevent the funder from mistakenly funding the wrong tokenId (due to an uninitialized argument) or some other hypothetical integration bugs.
1 participant
No description provided.