Use _numberMinted on edition instead of tally on minter (#233)

* Use _numberMinted on edition instead of tally on minter * additional tests * Add numberBurned and some comments Co-authored-by: Vectorized <[email protected]>
soundxyz · Sep 19, 2022 · 041e5f2 · 041e5f2
1 parent 1298050
commit 041e5f2
Show file tree

Hide file tree

Showing 11 changed files with 126 additions and 85 deletions.
diff --git a/.changeset/cold-wasps-buy.md b/.changeset/cold-wasps-buy.md
@@ -0,0 +1,5 @@
+---
+"@soundxyz/sound-protocol": patch
+---
+
+Use \_numberMinted on edition instead of tally on minter
diff --git a/contracts/core/SoundEditionV1.sol b/contracts/core/SoundEditionV1.sol
@@ -461,6 +461,20 @@ contract SoundEditionV1 is ISoundEditionV1, ERC721AQueryableUpgradeable, ERC721A
         return _nextTokenId();
     }
 
+    /**
+     * @inheritdoc ISoundEditionV1
+     */
+    function numberMinted(address owner) external view returns (uint256) {
+        return _numberMinted(owner);
+    }
+
+    /**
+     * @inheritdoc ISoundEditionV1
+     */
+    function numberBurned(address owner) external view returns (uint256) {
+        return _numberBurned(owner);
+    }
+
     /**
      * @inheritdoc ISoundEditionV1
      */

diff --git a/contracts/core/interfaces/ISoundEditionV1.sol b/contracts/core/interfaces/ISoundEditionV1.sol
@@ -523,6 +523,20 @@ interface ISoundEditionV1 is IERC721AUpgradeable, IERC2981Upgradeable {
      */
     function nextTokenId() external view returns (uint256);
 
+    /**
+     * @dev Returns the number of tokens minted by `owner`.
+     * @param owner Address to query for number minted.
+     * @return The latest value.
+     */
+    function numberMinted(address owner) external view returns (uint256);
+
+    /**
+     * @dev Returns the number of tokens burned by `owner`.
+     * @param owner Address to query for number burned.
+     * @return The latest value.
+     */
+    function numberBurned(address owner) external view returns (uint256);
+
     /**
      * @dev Returns the total amount of tokens minted.
      * @return The latest value.

diff --git a/contracts/modules/EditionMaxMinter.sol b/contracts/modules/EditionMaxMinter.sol
@@ -25,12 +25,6 @@ contract EditionMaxMinter is IEditionMaxMinter, BaseMinter {
      */
     mapping(address => mapping(uint128 => EditionMintData)) internal _editionMintData;
 
-    /**
-     * @dev Number of tokens minted by each buyer address
-     * edition => mintId => buyer => mintedTallies
-     */
-    mapping(address => mapping(uint256 => mapping(address => uint256))) public mintedTallies;
-
     // =============================================================
     //                          CONSTRUCTOR
     // =============================================================
@@ -84,14 +78,11 @@ contract EditionMaxMinter is IEditionMaxMinter, BaseMinter {
         EditionMintData storage data = _editionMintData[edition][mintId];
 
         unchecked {
-            uint256 userMintedBalance = mintedTallies[edition][mintId][msg.sender];
-            // Check the additional quantity does not exceed the set maximum.
-            // If `quantity` is large enough to cause an overflow,
-            // `_mint` will give an out of gas error.
-            uint256 tally = userMintedBalance + quantity;
-            if (tally > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
-            // Update the minted tally for this account
-            mintedTallies[edition][mintId][msg.sender] = tally;
+            // Check the additional `requestedQuantity` does not exceed the maximum mintable per account.
+            uint256 numberMinted = ISoundEditionV1(edition).numberMinted(msg.sender);
+            // Won't overflow. The total number of tokens minted in `edition` won't exceed `type(uint32).max`,
+            // and `quantity` has 32 bits.
+            if (numberMinted + quantity > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
         }
 
         _mint(edition, mintId, quantity, affiliate);

diff --git a/contracts/modules/MerkleDropMinter.sol b/contracts/modules/MerkleDropMinter.sol
@@ -8,6 +8,7 @@ import { ISoundFeeRegistry } from "@core/interfaces/ISoundFeeRegistry.sol";
 import { BaseMinter } from "@modules/BaseMinter.sol";
 import { IMerkleDropMinter, EditionMintData, MintInfo } from "./interfaces/IMerkleDropMinter.sol";
 import { IMinterModule } from "@core/interfaces/IMinterModule.sol";
+import { ISoundEditionV1 } from "@core/interfaces/ISoundEditionV1.sol";
 
 /**
  * @title MerkleDropMinter
@@ -25,12 +26,6 @@ contract MerkleDropMinter is IMerkleDropMinter, BaseMinter {
      */
     mapping(address => mapping(uint128 => EditionMintData)) internal _editionMintData;
 
-    /**
-     * @dev Number of tokens minted by each buyer address
-     *      Maps: `edition` => `mintId` => `buyer` => value.
-     */
-    mapping(address => mapping(uint128 => mapping(address => uint256))) public mintedTallies;
-
     // =============================================================
     //                          CONSTRUCTOR
     // =============================================================
@@ -99,14 +94,11 @@ contract MerkleDropMinter is IMerkleDropMinter, BaseMinter {
         if (!valid) revert InvalidMerkleProof();
 
         unchecked {
-            uint256 userMintedBalance = mintedTallies[edition][mintId][msg.sender];
-            // Check the additional requestedQuantity does not exceed the set maximum.
-            // If `requestedQuantity` is large enough to cause an overflow,
-            // `_mint` will give an out of gas error.
-            uint256 tally = userMintedBalance + requestedQuantity;
-            if (tally > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
-            // Update the minted tally for this account
-            mintedTallies[edition][mintId][msg.sender] = tally;
+            // Check the additional `requestedQuantity` does not exceed the maximum mintable per account.
+            uint256 numberMinted = ISoundEditionV1(edition).numberMinted(msg.sender);
+            // Won't overflow. The total number of tokens minted in `edition` won't exceed `type(uint32).max`,
+            // and `quantity` has 32 bits.
+            if (numberMinted + requestedQuantity > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
         }
 
         _mint(edition, mintId, requestedQuantity, affiliate);

diff --git a/contracts/modules/RangeEditionMinter.sol b/contracts/modules/RangeEditionMinter.sol
@@ -7,6 +7,7 @@ import { ISoundFeeRegistry } from "@core/interfaces/ISoundFeeRegistry.sol";
 import { IRangeEditionMinter, EditionMintData, MintInfo } from "./interfaces/IRangeEditionMinter.sol";
 import { BaseMinter } from "./BaseMinter.sol";
 import { IMinterModule } from "@core/interfaces/IMinterModule.sol";
+import { ISoundEditionV1 } from "@core/interfaces/ISoundEditionV1.sol";
 
 /*
  * @title RangeEditionMinter
@@ -24,12 +25,6 @@ contract RangeEditionMinter is IRangeEditionMinter, BaseMinter {
      */
     mapping(address => mapping(uint128 => EditionMintData)) internal _editionMintData;
 
-    /**
-     * @dev Number of tokens minted by each buyer address
-     * edition => mintId => buyer => mintedTallies
-     */
-    mapping(address => mapping(uint256 => mapping(address => uint256))) public mintedTallies;
-
     // =============================================================
     //                          CONSTRUCTOR
     // =============================================================
@@ -99,14 +94,11 @@ contract RangeEditionMinter is IRangeEditionMinter, BaseMinter {
         data.totalMinted = _incrementTotalMinted(data.totalMinted, quantity, _maxMintable);
 
         unchecked {
-            uint256 userMintedBalance = mintedTallies[edition][mintId][msg.sender];
-            // Check the additional quantity does not exceed the set maximum.
-            // If `quantity` is large enough to cause an overflow,
-            // `_mint` will give an out of gas error.
-            uint256 tally = userMintedBalance + quantity;
-            if (tally > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
-            // Update the minted tally for this account
-            mintedTallies[edition][mintId][msg.sender] = tally;
+            // Check the additional `requestedQuantity` does not exceed the maximum mintable per account.
+            uint256 numberMinted = ISoundEditionV1(edition).numberMinted(msg.sender);
+            // Won't overflow. The total number of tokens minted in `edition` won't exceed `type(uint32).max`,
+            // and `quantity` has 32 bits.
+            if (numberMinted + quantity > data.maxMintablePerAccount) revert ExceedsMaxPerAccount();
         }
 
         _mint(edition, mintId, quantity, affiliate);

diff --git a/contracts/modules/interfaces/IMerkleDropMinter.sol b/contracts/modules/interfaces/IMerkleDropMinter.sol
@@ -223,19 +223,6 @@ interface IMerkleDropMinter is IMinterModule {
     //               PUBLIC / EXTERNAL VIEW FUNCTIONS
     // =============================================================
 
-    /**
-     * @dev Returns the amount of minted tokens for `account` in `mintData`.
-     * @param edition Address of the edition.
-     * @param mintId  Mint identifier.
-     * @param account Address of the account.
-     * @return tally The number of minted tokens for the account.
-     */
-    function mintedTallies(
-        address edition,
-        uint128 mintId,
-        address account
-    ) external view returns (uint256);
-
     /**
      * @dev Returns IMerkleDropMinter.MintInfo instance containing the full minter parameter set.
      * @param edition The edition to get the mint instance for.

diff --git a/tests/core/SoundEdition/mint.t.sol b/tests/core/SoundEdition/mint.t.sol
@@ -111,6 +111,7 @@ contract SoundEdition_mint is TestConfig {
         edition.burn(TOKEN1_ID);
 
         assert(edition.balanceOf(address(this)) == 0);
+        assert(edition.numberBurned(address(this)) == ONE_TOKEN);
         assert(edition.totalSupply() == 0);
 
         // Mint another token and assert that the attacker can't burn
@@ -476,4 +477,19 @@ contract SoundEdition_mint is TestConfig {
         // Airdrop with `quantity` right at the limit is ok.
         edition.airdrop(to, limit);
     }
+
+    function test_numberMintedReturnsExpectedValue() public {
+        SoundEditionV1 edition = createGenericEdition();
+
+        address owner = address(12345);
+        edition.transferOwnership(owner);
+
+        assertTrue(edition.numberMinted(owner) == 0);
+
+        vm.prank(owner);
+        uint32 quantity = 10;
+        edition.mint(owner, quantity);
+
+        assertTrue(edition.numberMinted(owner) == quantity);
+    }
 }
diff --git a/tests/modules/EditionMaxMinter.t.sol b/tests/modules/EditionMaxMinter.t.sol
@@ -176,19 +176,44 @@ contract EditionMaxMinterTests is TestConfig {
         minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
     }
 
+    function test_mintWhenOverMaxMintableDueToPreviousMintedReverts() public {
+        (SoundEditionV1 edition, EditionMaxMinter minter) = _createEditionAndMinter(3);
+        vm.warp(START_TIME);
+
+        address caller = getFundedAccount(1);
+
+        // have 2 previously minted
+        address owner = address(12345);
+        edition.transferOwnership(owner);
+        vm.prank(owner);
+        edition.mint(caller, 2);
+
+        // attempting to mint 2 more reverts
+        vm.prank(caller);
+        vm.expectRevert(IEditionMaxMinter.ExceedsMaxPerAccount.selector);
+        minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
+    }
+
     function test_mintWhenMintablePerAccountIsSetAndSatisfied() public {
-        // Set max allowed per account to 2
-        (SoundEditionV1 edition, EditionMaxMinter minter) = _createEditionAndMinter(2);
+        // Set max allowed per account to 3
+        (SoundEditionV1 edition, EditionMaxMinter minter) = _createEditionAndMinter(3);
 
-        // Ensure we can mint the max allowed of 2 tokens
         address caller = getFundedAccount(1);
+
+        // Set 1 previous mint
+        address owner = address(12345);
+        edition.transferOwnership(owner);
+        vm.prank(owner);
+        edition.mint(caller, 1);
+
+        // Ensure we can mint the max allowed of 2 tokens
         vm.warp(START_TIME);
         vm.prank(caller);
         minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
 
-        assertEq(edition.balanceOf(caller), 2);
+        assertEq(edition.balanceOf(caller), 3);
 
-        assertEq(edition.totalMinted(), 2);
+        assertEq(edition.totalMinted(), 3);
     }
 
     function test_mintUpdatesValuesAndMintsCorrectly() public {

diff --git a/tests/modules/MerkleDropMinter.t.sol b/tests/modules/MerkleDropMinter.t.sol
@@ -170,25 +170,6 @@ contract MerkleDropMinterTests is TestConfig {
         minter.mint(address(edition), mintId, requestedQuantity, proof, address(0));
     }
 
-    function test_canGetMintedTallyForAccount() public {
-        uint32 maxMintablePerAccount = 1;
-        (SoundEditionV1 edition, MerkleDropMinter minter, uint128 mintId) = _createEditionAndMinter(
-            0,
-            6,
-            maxMintablePerAccount
-        );
-        bytes32[] memory proof = m.getProof(leaves, 0);
-
-        vm.warp(START_TIME);
-        vm.prank(accounts[0]);
-
-        uint32 requestedQuantity = maxMintablePerAccount;
-        minter.mint(address(edition), mintId, requestedQuantity, proof, address(0));
-
-        uint256 mintedTally = minter.mintedTallies(address(edition), mintId, accounts[0]);
-        assertEq(mintedTally, 1);
-    }
-
     function test_setPrice(uint96 price) public {
         (SoundEditionV1 edition, MerkleDropMinter minter, uint128 mintId) = _createEditionAndMinter(0, 0, 1);
 

diff --git a/tests/modules/RangeEditionMinter.t.sol b/tests/modules/RangeEditionMinter.t.sol
@@ -230,20 +230,44 @@ contract RangeEditionMinterTests is TestConfig {
         minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
     }
 
+    function test_mintWhenOverMaxMintableDueToPreviousMintedReverts() public {
+        (SoundEditionV1 edition, RangeEditionMinter minter) = _createEditionAndMinter(3);
+        vm.warp(START_TIME);
+
+        address caller = getFundedAccount(1);
+
+        // have 2 previously minted
+        address owner = address(12345);
+        edition.transferOwnership(owner);
+        vm.prank(owner);
+        edition.mint(caller, 2);
+
+        // attempting to mint 2 more reverts
+        vm.prank(caller);
+        vm.expectRevert(IRangeEditionMinter.ExceedsMaxPerAccount.selector);
+        minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
+    }
+
     function test_mintWhenMintablePerAccountIsSetAndSatisfied() public {
-        // Set max allowed per account to 2
-        (SoundEditionV1 edition, RangeEditionMinter minter) = _createEditionAndMinter(2);
+        // Set max allowed per account to 3
+        (SoundEditionV1 edition, RangeEditionMinter minter) = _createEditionAndMinter(3);
 
-        // Ensure we can mint the max allowed of 2 tokens
         address caller = getFundedAccount(1);
+
+        // Set 1 previous mint
+        address owner = address(12345);
+        edition.transferOwnership(owner);
+        vm.prank(owner);
+        edition.mint(caller, 1);
+
+        // Ensure we can mint the max allowed of 2 tokens
         vm.warp(START_TIME);
         vm.prank(caller);
         minter.mint{ value: PRICE * 2 }(address(edition), MINT_ID, 2, address(0));
 
-        assertEq(edition.balanceOf(caller), 2);
+        assertEq(edition.balanceOf(caller), 3);
 
-        MintInfo memory data = minter.mintInfo(address(edition), MINT_ID);
-        assertEq(data.totalMinted, 2);
+        assertEq(edition.totalMinted(), 3);
     }
 
     function test_mintUpdatesValuesAndMintsCorrectly() public {