Skip to content
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

[Feature-request]: NFT minting properties #25

Open
aviggiano opened this issue May 8, 2023 · 2 comments
Open

[Feature-request]: NFT minting properties #25

aviggiano opened this issue May 8, 2023 · 2 comments

Comments

@aviggiano
Copy link
Contributor

aviggiano commented May 8, 2023

Describe the desired feature or improvement

I am reviewing a smart contract that contains a very complex logic to mint NFTs with random rarities using Chainlink VRF. Just by reading the code it is hard to understand if it correctly implements the specification. It would be nice to have pre-built echidna properties to test invariants.

In my particular example, the invariants are rather commonplace to other NFT projects, which is why I believe these are generic enough to be applied to other smart contracts:

  • max NFTs per wallet: no user can have more NFTs than the max
  • max NFTs minted: total supply must not exceed the max
  • max NFTs per project owner: the project owner cannot have more than the max
  • NFT price (in Ether, but can be extended to ERC-20 too): minting an NFT must cost exactly the NFT price
  • NFT rarity: the number of NFTs minted for a given rarity must match their assigned percentages, within an error margin
  • whitelist: no user can mint NFTs if they are not on the whitelist
  • etc

If you think this is useful I can work on these properties and submit a PR.

@montyly
Copy link
Member

montyly commented May 9, 2023

Hi @aviggiano , I think that is a great idea.

Do you see them as an extension of the current ERC721 properties, or something separate?

@tuturu-tech might be able to help if you have questions about the repo structure/contribution process

@aviggiano
Copy link
Contributor Author

aviggiano commented May 9, 2023

I think this could be something separate, since it is a bit independent from the ERC721 properties.

The contract I am reviewing includes the minting logic on the ERC721 contract itself, but I don't think this would be the case for most projects. I think they would most likely have a NFT contract and a NFTSale/NFTMinting contract.

Here's the interface of the contract I want to test:

interface NFTSale is ERC721 {
    function mintNFT(uint256 _amount, bytes calldata _signature) external payable returns (uint256[] memory);
    function setConfig(uint256 _maxSupply, uint256 _maxNFTsPerWallet, uint256 _nftPrice, uint256[] memory _percentages) external;
}

Based on this particular case, I think we could define an interface that these NFTSale/NFTMinting should follow, similar to what we have on CryticIERC4626Internal for ERC4626 vaults.

Here's an initial idea:

/// @title CryticIERC721Minting
/// @notice Interface for NFT minting with support for different prices per quantity, free mints, and payment in either ERC20 tokens or ether.
interface CryticIERC721Minting {
  /// @notice Returns the price for a specific quantity of NFTs when using a specific payment token.
  /// @dev Should pass type(uint256).max if the token specified is not allowed for this mint
  /// @param amount The quantity of NFTs.
  /// @param token The payment token address.
  /// @return The price in the payment token for the specified quantity of NFTs.
  function price(uint256 amount, address token) external view returns(uint256);

  /// @notice Returns the maximum supply of minted NFTs.
  /// @return The maximum supply of minted NFTs.
  function maxTotalMint() external view returns(uint256);

  /// @notice Returns the maximum NFTs that can be minted per wallet.
  /// @param account The wallet address.
  /// @return The maximum NFTs that can be minted per wallet.
  function maxMint(address account) external view returns(uint256);

  /// @notice Checks if an account is allowed to mint NFTs, supporting on-chain mapping, off-chain signed messages, and Merkle tree allowlists.
  /// @param account The account to check for minting permissions.
  /// @param data Additional data for checking minting permissions.
  function isAllowed(address account, bytes memory data) external view returns(bool);

  /// @notice Mints the specified quantity of NFTs, optionally charging users in ERC-20 or ether, and optionally verifying the allowlist.
  /// @dev Should pass an empty data if there is no allowlist
  /// @param amount The quantity of NFTs to mint.
  /// @param token The payment token address.
  /// @param data Additional data for minting.
  function mint(uint256 amount, address token, bytes memory data) external payable;
}

I still need to think how to handle the rarities, though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants