♻️ 1155 changes

.gas-snapshot

@@ -20,7 +20,7 @@ testDeployERC20() (gas: 884085)
 testFailDoubleDeployDifferentBytecode() (gas: 277076930215043)
 testFailBoundMinBiggerThanMax() (gas: 309)
 testBound() (gas: 5520)
-testSupportsERC165Interface() (gas: 142)
+testMint() (gas: 30027)
 testInfiniteApproveTransferFrom() (gas: 387796)
 testApprove() (gas: 26558)
 testMetaData() (gas: 7032)

src/test/ERC1155.t.sol

@@ -14,10 +14,116 @@ contract ERC1155Test is DSTestPlus {
         token = new MockERC1155();
     }
 
-    function testSupportsERC165Interface() public {
-        // assertTrue(token.supportsInterface(0x01ffc9a7));
-        // assertTrue(token.supportsInterface(0xd9b67a26));
+    function testMint() public {
+        token.mint(address(0xBEEF), 1, 1, "");
+
+        assertEq(token.balanceOf(address(0xBEEF), 1), 1);
+    }
+
+    function testBatchMint(
+        address usr,
+        uint256[] memory ids,
+        uint amount
+    ) public {
+        if (usr == address(0)) return;
+
+        uint len = ids.length;
+        uint256[] memory amounts = new uint256[](len);
+        address[] memory owners = new address[](len);
+
+        for (uint i = 0; i < len; i++) {
+            //if ids array has any dupes amounts will not be correct
+            for (uint256 j = 0; j < i; j++) {
+                if (ids[i] == ids[j]) return;
+            }
+            amounts[i] = amount;
+            owners[i] = usr;
+        }
+
+        token.batchMint(usr, ids, amounts, "");
+
+        assertUintArrayEq(token.balanceOfBatch(owners, ids), amounts);
+    }
+
+    function testBatchBurn(
+        address usr,
+        uint256[] memory ids,
+        uint amount
+    ) public {
+        if (usr == address(0)) return;
+
+        uint len = ids.length;
+        uint256[] memory amounts = new uint256[](len);
+        uint256[] memory zeroAmounts = new uint256[](len);        
+        address[] memory owners = new address[](len);
+
+        for (uint i = 0; i < len; i++) {
+            //if ids array has any dupes amounts will not be correct
+            for (uint256 j = 0; j < i; j++) {
+                if (ids[i] == ids[j]) return;
+            }
+            amounts[i] = amount;
+            zeroAmounts[i] = 0;
+            owners[i] = usr;
+        }
+
+        token.batchMint(usr, ids, amounts, "");
+
+        assertUintArrayEq(token.balanceOfBatch(owners, ids), amounts);
+
+        token.batchBurn(usr, ids, amounts);
+
+        assertUintArrayEq(token.balanceOfBatch(owners, ids), zeroAmounts);
     }
 
-    // TODO:
+    function testSafeBatchTransferFromWithApproval(
+        uint256[] memory ids,
+        uint amount
+    ) public {
+        ERC1155User usr = new ERC1155User(token);
+        ERC1155User receiver = new ERC1155User(token);
+        ERC1155User operator = new ERC1155User(token);
+
+        uint len = ids.length;
+
+        uint256[] memory amounts = new uint256[](len);
+        uint256[] memory zeroAmounts = new uint256[](len);
+        address[] memory usrs = new address[](len);
+        address[] memory receivers = new address[](len);
+
+        for (uint i = 0; i < len; i++) {
+            //if ids array has any dupes amounts will not be correct
+            for (uint256 j = 0; j < i; j++) {
+                if (ids[i] == ids[j]) return;
+            }
+            amounts[i] = amount;
+            receivers[i] = address(receiver);
+            usrs[i] = address(usr);
+            zeroAmounts[i] = 0;
+        }
+
+        //mint tokens to usr
+        token.batchMint(address(usr), ids, amounts, "");
+
+        // The operator should not be able to transfer the unapproved token
+        try operator.safeBatchTransferFrom(address(usr), address(receiver), ids, amounts, "") {
+            fail();
+        } catch Error(string memory error) {
+            assertEq(error, "INVALID_OPERATOR");
+        }
+
+        usr.setApprovalForAll(address(operator), true);
+        operator.safeBatchTransferFrom(address(usr), address(receiver), ids, amounts, "");
+
+        assertUintArrayEq(token.balanceOfBatch(usrs, ids), zeroAmounts);
+        assertUintArrayEq(token.balanceOfBatch(receivers, ids), amounts);
+
+        // The operator now should not be able to transfer the token
+        // since it was not approved by the current user
+        try operator.safeBatchTransferFrom(address(receiver), address(usr), ids, amounts, "") {
+            fail();
+        } catch Error(string memory error) {
+            assertEq(error, "INVALID_OPERATOR");
+        }
+    }
 }

src/test/ERC721.t.sol

@@ -26,6 +26,7 @@ contract ERC721Test is DSTestPlus {
     }
 
     function testMint(address usr, uint256 tokenId) public {
+        if (usr == address(0)) return;
         token.mint(usr, tokenId);
 
         assertEq(token.totalSupply(), 1);
@@ -63,6 +64,11 @@ contract ERC721Test is DSTestPlus {
         if (usr == address(0)) return;
 
         for (uint256 i = 0; i < tokenIds.length; i++) {
+            //if tokenIds array has any duplicate ids mint will revert
+            for (uint256 j = 0; j < i; j++) {
+                if (tokenIds[i] == tokenIds[j]) return;
+            }
+
             token.mint(usr, tokenIds[i]);
         }
 

src/test/utils/DSTestPlus.sol

@@ -53,6 +53,13 @@ contract DSTestPlus is DSTest {
         assertEq(uint256(a), uint256(b));
     }
 
+    function assertUintArrayEq(uint256[] memory a, uint256[] memory b) internal virtual {
+        require(a.length == b.length, "UINT_ARR_LEN_MISMATCH");
+        for (uint i = 0; i < a.length; i++) {
+            assertEq(a[i], b[i]);
+        }
+    }
+
     function assertApproxEq(
         uint256 a,
         uint256 b,

src/test/utils/users/ERC1155User.sol

@@ -3,13 +3,33 @@ pragma solidity >=0.8.0;
 
 import {ERC1155, ERC1155TokenReceiver} from "../../../tokens/ERC1155.sol";
 
-contract ERC1155User {
+contract ERC1155User is ERC1155TokenReceiver{
     ERC1155 token;
 
     constructor(ERC1155 _token) {
         token = _token;
     }
 
+    function onERC1155Received(
+        address,
+        address,
+        uint256,
+        uint256,
+        bytes calldata
+    ) external virtual override returns (bytes4) { 
+        return ERC1155TokenReceiver.onERC1155Received.selector;
+    }
+
+    function onERC1155BatchReceived(
+        address,
+        address,
+        uint256[] calldata,
+        uint256[] calldata,
+        bytes calldata
+    ) external virtual override returns (bytes4) {
+        return ERC1155TokenReceiver.onERC1155BatchReceived.selector;
+    }
+
     function setApprovalForAll(address operator, bool approved) public virtual {
         token.setApprovalForAll(operator, approved);
     }

src/tokens/ERC1155.sol

@@ -63,7 +63,9 @@ abstract contract ERC1155 {
 
         // TODO: unchecekd?
         balanceOf[from][id] -= amount;
-        balanceOf[to][id] += amount;
+        unchecked {
+            balanceOf[to][id] += amount;
+        }
 
         emit TransferSingle(msg.sender, from, to, id, amount);
 
@@ -87,19 +89,25 @@ abstract contract ERC1155 {
 
         require(msg.sender == from || isApprovedForAll[from][msg.sender], "INVALID_OPERATOR");
 
-        // TODO does chaching len actually help here
         uint256 nTransfer = ids.length;
 
-        unchecked {
-            // Counter overflow is incredibly unrealistic.
-            for (uint256 i = 0; i < nTransfer; i++) {
-                // TODO: how do we unchecked just i increment
+        for (uint256 i = 0; i < nTransfer; ) {
 
-                // TODO: does caching amounts[i] help here?
-                // todo: unchecked
-                balanceOf[from][ids[i]] -= amounts[i];
+            // TODO: does caching amounts[i] help here?
+            /*
+            pre
+                testSafeBatchTransferFrom() (gas: 1607432)
+            caching amounts[i]
+                testSafeBatchTransferFrom() (gas: 1607432)
+            */
+
+            balanceOf[from][ids[i]] -= amounts[i];
+            // Counter overflow is incredibly unrealistic.
+            unchecked {
                 balanceOf[to][ids[i]] += amounts[i];
+                i++;
             }
+
         }
 
         emit TransferBatch(msg.sender, from, to, ids, amounts);
@@ -120,9 +128,10 @@ abstract contract ERC1155 {
         returns (uint256[] memory batchbalanceOf)
     {
         require(owners.length == ids.length, "ARRAY_MISMATCH");
-
+        batchbalanceOf = new uint256[](owners.length);
+        // caching owners.length costs more gas than it saves
+        // Counter overflow is incredibly unrealistic.
         unchecked {
-            // Counter overflow is incredibly unrealistic.
             for (uint256 i = 0; i < owners.length; i++) {
                 batchbalanceOf[i] = balanceOf[owners[i]][ids[i]];
             }
@@ -150,6 +159,7 @@ abstract contract ERC1155 {
         uint256 amount,
         bytes memory data
     ) internal {
+
         unchecked {
             balanceOf[to][id] += amount;
         }
@@ -173,12 +183,20 @@ abstract contract ERC1155 {
     ) internal {
         require(ids.length == amounts.length, "INVALID_ARRAYS_LENGTH");
 
-        // tODO: does this save?
+
+        /*  Batch mint with arrays of length 1
+        pre
+            testBatchMint() (gas: 12878)
+        using ids.length over nMint
+            testBatchMint() (gas: 12883)
+        pre + unchecked i increment
+            testBatchMint() (gas: 12810)
+        */
+
         uint256 nMint = ids.length;
-        // TODO: incr loop unchecked
-        for (uint256 i = 0; i < nMint; i++) {
-            // TODO: comment
-            unchecked {
+
+        unchecked {
+            for (uint256 i = 0; i < nMint; i++) {
                 balanceOf[to][ids[i]] += amounts[i];
             }
         }
@@ -199,14 +217,17 @@ abstract contract ERC1155 {
         uint256[] memory ids,
         uint256[] memory amounts
     ) internal {
-        // TODO: does this actuallys ave gas
+
         uint256 nBurn = ids.length;
 
         require(nBurn == amounts.length, "ARRAY_MISMATCH");
 
         // TODO: how do we incrmeent i without breaking everything in this looop
-        for (uint256 i = 0; i < nBurn; i++) {
-            balanceOf[from][ids[i]] -= amounts[i];
+        //can be unchecked assuming inheriting contract checks from owns ids
+        unchecked {
+            for (uint256 i = 0; i < nBurn; i++) {
+                balanceOf[from][ids[i]] -= amounts[i];
+            }
         }
 
         emit TransferBatch(msg.sender, from, address(0), ids, amounts);