From 5f30c99fa3270ae7d443414a47e242f1c6655aa6 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Fri, 15 Jul 2022 01:18:44 +0530
Subject: [PATCH 01/21] update design doc for Pack

---
 contracts/pack/pack.md | 45 +++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/contracts/pack/pack.md b/contracts/pack/pack.md
index 7533c69ae..a2ae2bfe0 100644
--- a/contracts/pack/pack.md
+++ b/contracts/pack/pack.md
@@ -63,13 +63,14 @@ And so, what can be packed in packs are *n* number of configurations like ‘500
 ```solidity
 enum TokenType { ERC20, ERC721, ERC1155 }
 
-struct PackContent {
+struct Token {
     address assetContract;
     TokenType tokenType;
     uint256 tokenId;
-    uint256 totalAmountPacked;
-    uint256 amountDistributedPerOpen;
+    uint256 totalAmount;
 }
+
+uint256 perUnitAmount;
 ```
 
 | Value | Description |
@@ -77,8 +78,8 @@ struct PackContent {
 | assetContract | The contract address of the token. |
 | tokenType | The type of the token -- ERC20 / ERC721 / ERC1155 |
 | tokenId | The tokenId of the the token. (Not applicable for ERC20 tokens. The contract will ignore this value for ERC20 tokens.) |
-| totalAmountPacked | The total amount of this token packed in the pack. (Not applicable for ERC721 tokens. The contract will always consider this as 1 for ERC721 tokens.) |
-| amountDistributedPerOpen | The amount of this token to distribute as a unit, on opening a pack. (Not applicable for ERC721 tokens. The contract will always consider this as 1 for ERC721 tokens.) |
+| totalAmount | The total amount of this token packed in the pack. (Not applicable for ERC721 tokens. The contract will always consider this as 1 for ERC721 tokens.) |
+| perUnitAmount | The amount of this token to distribute as a unit, on opening a pack. (Not applicable for ERC721 tokens. The contract will always consider this as 1 for ERC721 tokens.) |
 
 **Note:** A pack can contain different configurations for the same token. For example, the same set of packs can contain ‘500 units of 20 USDC’ and ‘10 units of 1000 USDC’ as two independent types of underlying rewards.
 
@@ -87,26 +88,31 @@ struct PackContent {
 You can create packs with any ERC20, ERC721 or ERC1155 tokens that you own. To create packs, you must specify the following:
 
 ```solidity
+/// @dev Creates a pack with the stated contents.
 function createPack(
-    PackContent[] calldata contents,
+    Token[] calldata contents,
+    uint256[] calldata numOfRewardUnits,
     string calldata packUri,
     uint128 openStartTimestamp,
-    uint128 amountDistributedPerOpen
-) external;
+    uint128 amountDistributedPerOpen,
+    address recipient
+) external
 ```
 
 | Parameter | Description |
 | --- | --- |
-| contents | The reward units packed in the packs. |
+| contents | Tokens/assets packed in the set of pack. |
+| numOfRewardUnits | Number of reward units for each asset, where each reward unit contains per unit amount of corresponding asset. |
 | packUri | The (metadata) URI assigned to the packs created. |
 | openStartTimestamp | The timestamp after which packs can be opened. |
 | amountDistributedPerOpen | The number of reward units distributed per open. |
+| recipient | The recipient of the packs created. |
 
 ### Packs are ERC1155 tokens i.e. NFTs
 
 Packs themselves are ERC1155 tokens. And so, a set of packs created with your tokens is itself identified by a unique tokenId, has an associated metadata URI and a variable supply.
 
-In the example given in the previous section — ‘How packs work (without web3 terminology)’, there is a set of 100 packs created, where that entire set of packs is identified by a unique tokenId.
+In the example given in the previous section — ‘Non technical overview’, there is a set of 100 packs created, where that entire set of packs is identified by a unique tokenId.
 
 Since packs are ERC1155 tokens, you can publish multiple sets of packs using the same `Pack` contract.
 
@@ -133,23 +139,24 @@ function openPack(uint256 packId, uint256 amountToOpen) external;
 
 ### How reward units are selected to distribute on opening packs
 
-We build on the example in the previous section — ‘How packs work (without web3 terminology)’.
+We build on the example in the previous section — ‘Non-technical overview’.
 
-Each single **square**, **circle** or **star** is considered as a ‘reward unit’. For example, the 5 **stars** in the packs may be “5 units of 1000 USDC”, which is represented in the `Pack` contract as a single `PackContent` as follows:
+Each single **square**, **circle** or **star** is considered as a ‘reward unit’. For example, the 5 **stars** in the packs may be “5 units of 1000 USDC”, which is represented in the `Pack` contract by the following information
 
 ```solidity
-struct PackContent {
+struct Token {
     address assetContract; // USDC address
     TokenType tokenType; // TokenType.ERC20
     uint256 tokenId; // Not applicable
-    uint256 totalAmountPacked; // 5000
-    uint256 amountDistributedPerOpen; // 1000
+    uint256 totalAmount; // 5000
 }
+
+uint256 perUnitAmount; // 1000
 ```
 
 The percentage chance of receiving a particular kind of reward (e.g. a **star**) on opening a pack is calculated as:`(number_of_stars_packed) / (total number of packs)`. Here, `number_of_stars_packed` refers to the total number of reward units of the **star** kind inside the set of packs e.g. a total of 5 units of 1000 USDC.
 
-Going back to the example in the previous section — ‘How packs work (without web3 terminology)’. — the supply of the reward units in the relevant set of packs - 80 **circles**, 15 **squares**, and 5 **stars -**  can be represented on a number line, from zero to the total supply of packs - in this case, 100.
+Going back to the example in the previous section — ‘Non-technical overview’. — the supply of the reward units in the relevant set of packs - 80 **circles**, 15 **squares**, and 5 **stars -**  can be represented on a number line, from zero to the total supply of packs - in this case, 100.
 
 ![pack-diag-2.png](/assets/pack-diag-2.png)
 
@@ -181,9 +188,7 @@ The `Pack` contract requires a design where a pack owner *cannot possibly* predi
 To ensure the above, we make a simple check in the `openPack` function:
 
 ```solidity
-require(msg.sender == tx.origin, "opener cannot be smart contract");
-
-require(_msgSender() == tx.origin, "opener cannot be smart contract");
+require(isTrustedForwarder(msg.sender) || _msgSender() == tx.origin, "opener cannot be smart contract");
 ```
 
 `tx.origin` returns the address of the external account that initiated the transaction, of which the `openPack` function call is a part of.
@@ -191,7 +196,7 @@ require(_msgSender() == tx.origin, "opener cannot be smart contract");
 The above check essentially means that only an external account i.e. an end user wallet, and no smart contract, can open packs. This lets us generate a pseudo random number using block variables, for the purpose of `openPack`:
 
 ```solidity
-uint256 random = uint(keccak256(abi.encodePacked(msg.sender, blockhash(block.number), block.difficulty)));
+uint256 random = uint256(keccak256(abi.encodePacked(_msgSender(), blockhash(block.number - 1), block.difficulty)));
 ```
 
 Since only end user wallets can open packs, a pack owner *cannot possibly* predict the random number that will be used in the process of their pack opening. That is because a pack opener cannot query the result of the random number calculation during a given block, and call `openPack` within that same block.

From f69fc984982a8d34fe6c55d79d6ddcdd653486f4 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 25 Jul 2022 13:05:58 +0530
Subject: [PATCH 02/21] openPack out of gas -- fuzz test

---
 lib/forge-std       |   2 +-
 src/test/Pack.t.sol | 123 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/lib/forge-std b/lib/forge-std
index c66b4ac5d..27e14b7f2 160000
--- a/lib/forge-std
+++ b/lib/forge-std
@@ -1 +1 @@
-Subproject commit c66b4ac5d83c01b6b5c8da437910b92352a6a6d5
+Subproject commit 27e14b7f2448e5f5ac32719f51fe652aa0b0733e
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index 7502c9128..30a1ade42 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -876,6 +876,129 @@ contract PackTest is BaseTest {
         assertEq(packUri, pack.uri(packId));
     }
 
+    function test_fuzz_state_openPack(
+        uint256 x,
+        uint128 y,
+        uint256 z
+    ) public {
+        // vm.assume(x == 1574 && y == 22 && z == 392);
+        (ITokenBundle.Token[] memory tokensToPack, uint256[] memory rewardUnits) = getTokensToPack(x);
+        if (tokensToPack.length == 0) {
+            return;
+        }
+
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(0x123);
+        uint256 totalRewardUnits;
+        uint256 nativeTokenPacked;
+
+        for (uint256 i = 0; i < tokensToPack.length; i += 1) {
+            totalRewardUnits += rewardUnits[i];
+            if (tokensToPack[i].assetContract == address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE)) {
+                nativeTokenPacked += tokensToPack[i].totalAmount;
+            }
+        }
+        vm.assume(y > 0 && totalRewardUnits % y == 0);
+        vm.deal(address(tokenOwner), nativeTokenPacked);
+
+        vm.prank(address(tokenOwner));
+        (, uint256 totalSupply) = pack.createPack{ value: nativeTokenPacked }(
+            tokensToPack,
+            rewardUnits,
+            packUri,
+            0,
+            y,
+            recipient
+        );
+        console2.log("total supply: ", totalSupply);
+        console2.log("total reward units: ", totalRewardUnits);
+
+        vm.assume(z <= totalSupply);
+        vm.prank(recipient, recipient);
+        ITokenBundle.Token[] memory rewardsReceived = pack.openPack(packId, z);
+        console2.log("received reward units: ", rewardsReceived.length);
+
+        assertEq(packUri, pack.uri(packId));
+
+        (
+            uint256 nativeTokenAmount,
+            uint256 erc20Amount,
+            uint256[] memory erc1155Amounts,
+            uint256 erc721Amount
+        ) = checkBalances(rewardsReceived, recipient);
+
+        assertEq(address(recipient).balance, nativeTokenAmount);
+        assertEq(erc20.balanceOf(address(recipient)), erc20Amount);
+        assertEq(erc721.balanceOf(address(recipient)), erc721Amount);
+
+        for (uint256 i = 0; i < erc1155Amounts.length; i += 1) {
+            assertEq(erc1155.balanceOf(address(recipient), i), erc1155Amounts[i]);
+        }
+    }
+
+    function test_fuzz_failing_state_openPack() public {
+        // x: 446, y: 22, z: 890 (gas: 8937393460518282035), total supply: 10203, total reward units: 224466
+        // x: 335, y: 3, z: 1570 (gas: 8937393460517864076), total supply: 54915, total reward units: 164745
+        // x: 1962, y: 282, z: 219 (gas: 8937393460523355524), total supply: 3239, total reward units: 913398
+
+        uint256 x = 1962;
+        uint128 y = 282;
+        uint256 z = 219;
+
+        (ITokenBundle.Token[] memory tokensToPack, uint256[] memory rewardUnits) = getTokensToPack(x);
+        if (tokensToPack.length == 0) {
+            return;
+        }
+
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(0x123);
+        uint256 totalRewardUnits;
+        uint256 nativeTokenPacked;
+
+        for (uint256 i = 0; i < tokensToPack.length; i += 1) {
+            totalRewardUnits += rewardUnits[i];
+            if (tokensToPack[i].assetContract == address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE)) {
+                nativeTokenPacked += tokensToPack[i].totalAmount;
+            }
+        }
+        vm.assume(y > 0 && totalRewardUnits % y == 0);
+        vm.deal(address(tokenOwner), nativeTokenPacked);
+
+        vm.prank(address(tokenOwner));
+        (, uint256 totalSupply) = pack.createPack{ value: nativeTokenPacked }(
+            tokensToPack,
+            rewardUnits,
+            packUri,
+            0,
+            y,
+            recipient
+        );
+        console2.log("total supply: ", totalSupply);
+        console2.log("total reward units: ", totalRewardUnits);
+
+        vm.assume(z <= totalSupply);
+        vm.prank(recipient, recipient);
+        ITokenBundle.Token[] memory rewardsReceived = pack.openPack(packId, z);
+        console2.log("received reward units: ", rewardsReceived.length);
+
+        assertEq(packUri, pack.uri(packId));
+
+        (
+            uint256 nativeTokenAmount,
+            uint256 erc20Amount,
+            uint256[] memory erc1155Amounts,
+            uint256 erc721Amount
+        ) = checkBalances(rewardsReceived, recipient);
+
+        assertEq(address(recipient).balance, nativeTokenAmount);
+        assertEq(erc20.balanceOf(address(recipient)), erc20Amount);
+        assertEq(erc721.balanceOf(address(recipient)), erc721Amount);
+
+        for (uint256 i = 0; i < erc1155Amounts.length; i += 1) {
+            assertEq(erc1155.balanceOf(address(recipient), i), erc1155Amounts[i]);
+        }
+    }
+
     /*///////////////////////////////////////////////////////////////
                             Scenario/Exploit tests
     //////////////////////////////////////////////////////////////*/

From 16922e95cf2a265887ba6285e9a1230d806fcb34 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 1 Aug 2022 19:36:18 +0530
Subject: [PATCH 03/21] [M-1] Lost pack tokens result in permanently locked
 assets

---
 contracts/interfaces/IPack.sol |   3 +
 contracts/pack/Pack.sol        |  28 ++++++-
 lib/forge-std                  |   2 +-
 src/test/Pack.t.sol            | 136 ++++++++++++++++++++++++++-------
 src/test/PackBenchmark.t.sol   |   4 +-
 5 files changed, 142 insertions(+), 31 deletions(-)

diff --git a/contracts/interfaces/IPack.sol b/contracts/interfaces/IPack.sol
index 0035d3619..66947abc8 100644
--- a/contracts/interfaces/IPack.sol
+++ b/contracts/interfaces/IPack.sol
@@ -21,6 +21,8 @@ interface IPack is ITokenBundle {
         uint256[] perUnitAmounts;
         uint128 openStartTimestamp;
         uint128 amountDistributedPerOpen;
+        uint256 expirationTimestamp;
+        address creator;
     }
 
     /// @notice Emitted when a set of packs is created.
@@ -58,6 +60,7 @@ interface IPack is ITokenBundle {
         string calldata packUri,
         uint128 openStartTimestamp,
         uint128 amountDistributedPerOpen,
+        uint256 expirationTimestamp,
         address recipient
     ) external payable returns (uint256 packId, uint256 packTotalSupply);
 
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index e038bb271..57052660c 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -175,6 +175,7 @@ contract Pack is
         string calldata _packUri,
         uint128 _openStartTimestamp,
         uint128 _amountDistributedPerOpen,
+        uint256 _expirationTimestamp,
         address _recipient
     )
         external
@@ -200,6 +201,8 @@ contract Pack is
 
         packInfo[packId].openStartTimestamp = _openStartTimestamp;
         packInfo[packId].amountDistributedPerOpen = _amountDistributedPerOpen;
+        packInfo[packId].expirationTimestamp = _expirationTimestamp == 0 ? type(uint256).max : _expirationTimestamp;
+        packInfo[packId].creator = _msgSender();
 
         _mint(_recipient, packId, packTotalSupply, "");
 
@@ -215,11 +218,12 @@ contract Pack is
     {
         address opener = _msgSender();
 
-        require(isTrustedForwarder(opener) || opener == tx.origin, "opener must be eoa");
+        require(opener == tx.origin, "opener must be eoa");
         require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
 
         PackInfo memory pack = packInfo[_packId];
         require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
+        require(pack.expirationTimestamp > block.timestamp, "pack has expired");
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
@@ -232,6 +236,17 @@ contract Pack is
         return rewardUnits;
     }
 
+    function withdrawUnclaimedAssets(uint256 _packId) 
+        external
+        nonReentrant
+    {
+        PackInfo memory pack = packInfo[_packId];
+        require(block.timestamp >= pack.expirationTimestamp, "pack not expired yet");
+        require(_msgSender() == pack.creator, "not creator");
+
+        _releaseTokens(_msgSender(), _packId);
+    }
+
     /// @dev Stores assets within the contract.
     function escrowPackContents(
         Token[] calldata _contents,
@@ -324,6 +339,17 @@ contract Pack is
         }
     }
 
+    /// @dev Returns opening and expiration timestamps of a pack.
+    function getPackTimestamps(uint256 _packId)
+        external
+        view
+        returns (uint128 openStartTimestamp, uint256 expirationTimestamp)
+    {
+        PackInfo memory pack = packInfo[_packId];
+        openStartTimestamp = pack.openStartTimestamp;
+        expirationTimestamp = pack.expirationTimestamp;
+    }
+
     /*///////////////////////////////////////////////////////////////
                         Internal functions
     //////////////////////////////////////////////////////////////*/
diff --git a/lib/forge-std b/lib/forge-std
index f32dbdc3f..27e14b7f2 160000
--- a/lib/forge-std
+++ b/lib/forge-std
@@ -1 +1 @@
-Subproject commit f32dbdc3f014176539d95099cbe617968fd0b059
+Subproject commit 27e14b7f2448e5f5ac32719f51fe652aa0b0733e
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index 7502c9128..6e897e5a7 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -155,6 +155,87 @@ contract PackTest is BaseTest {
         pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
     }
 
+    /*///////////////////////////////////////////////////////////////
+                            Miscellaneous
+    //////////////////////////////////////////////////////////////*/
+
+    function test_state_createPack_timestamps() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+
+        (, uint256 expirationTimestamp) = pack.getPackTimestamps(packId);
+
+        assertEq(expirationTimestamp, type(uint256).max);
+    }
+
+    function test_revert_openPack_expirationTimestamp() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(0x123);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
+
+        vm.warp(101);
+        vm.prank(recipient, recipient);
+        vm.expectRevert("pack has expired");
+        pack.openPack(packId, 1);
+
+        vm.warp(99);
+        vm.prank(recipient, recipient);
+        pack.openPack(packId, 1);
+    }
+
+    function test_state_withdrawUnclaimedAssets() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(0x123);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
+
+        vm.warp(101);
+        vm.prank(address(tokenOwner));
+        pack.withdrawUnclaimedAssets(packId);
+
+        // ERC20 balance
+        assertEq(erc20.balanceOf(address(tokenOwner)), 2000 ether);
+        assertEq(erc20.balanceOf(address(pack)), 0);
+
+        // ERC721 balance
+        assertEq(erc721.ownerOf(0), address(tokenOwner));
+        assertEq(erc721.ownerOf(1), address(tokenOwner));
+        assertEq(erc721.ownerOf(2), address(tokenOwner));
+        assertEq(erc721.ownerOf(3), address(tokenOwner));
+        assertEq(erc721.ownerOf(4), address(tokenOwner));
+        assertEq(erc721.ownerOf(5), address(tokenOwner));
+
+        // ERC1155 balance
+        assertEq(erc1155.balanceOf(address(tokenOwner), 0), 100);
+        assertEq(erc1155.balanceOf(address(pack), 0), 0);
+        assertEq(erc1155.balanceOf(address(tokenOwner), 1), 500);
+        assertEq(erc1155.balanceOf(address(pack), 1), 0);
+    }
+
+    function test_revert_withdrawUnclaimedAssets() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(0x123);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
+
+        vm.warp(99);
+        vm.prank(address(tokenOwner));
+        vm.expectRevert("pack not expired yet");
+        pack.withdrawUnclaimedAssets(packId);
+
+        vm.warp(101);
+        vm.prank(address(345));
+        vm.expectRevert("not creator");
+        pack.withdrawUnclaimedAssets(packId);
+    }
+
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `createPack`
     //////////////////////////////////////////////////////////////*/
@@ -173,7 +254,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -208,7 +289,7 @@ contract PackTest is BaseTest {
         numOfRewardUnits.push(20);
 
         vm.prank(address(tokenOwner));
-        pack.createPack{ value: 20 ether }(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack{ value: 20 ether }(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -242,7 +323,7 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -269,7 +350,7 @@ contract PackTest is BaseTest {
         vm.expectEmit(true, true, true, true);
         emit PackCreated(packId, address(tokenOwner), recipient, 226);
 
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         vm.stopPrank();
     }
@@ -301,7 +382,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         // ERC20 balance
         assertEq(erc20.balanceOf(address(tokenOwner)), 0);
@@ -347,7 +428,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert(bytes(errorMsg));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -370,7 +451,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert(bytes(errorMsg));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -393,7 +474,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert("msg.value != amount");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -406,7 +487,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC20: transfer amount exceeds balance");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -419,7 +500,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC721: transfer caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -432,7 +513,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC1155: insufficient balance for transfer");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -445,7 +526,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC20: insufficient allowance");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -458,7 +539,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC721: transfer caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -471,7 +552,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC1155: caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -493,7 +574,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("Asset doesn't match TokenType");
-        pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -515,7 +596,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("amount can't be zero");
-        (, uint256 totalSupply) = pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
+        (, uint256 totalSupply) = pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, 0, recipient);
 
         // assertEq(totalSupply, 10);
     }
@@ -531,7 +612,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("nothing to pack");
-        pack.createPack(emptyContent, rewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(emptyContent, rewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /**
@@ -544,7 +625,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("invalid reward units");
-        pack.createPack(packContents, rewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, rewardUnits, packUri, 0, 1, 0, recipient);
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -561,7 +642,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, 0, recipient);
 
         vm.prank(recipient, recipient);
         ITokenBundle.Token[] memory rewardUnits = pack.openPack(packId, packsToOpen);
@@ -597,7 +678,7 @@ contract PackTest is BaseTest {
         ITokenBundle.Token[] memory emptyRewardUnitsForTestingEvent;
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         vm.expectEmit(true, true, false, false);
         emit PackOpened(packId, recipient, 1, emptyRewardUnitsForTestingEvent);
@@ -612,7 +693,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, 0, recipient);
 
         // ERC20 balance
         assertEq(erc20.balanceOf(address(recipient)), 0);
@@ -678,7 +759,7 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         vm.startPrank(recipient, address(27));
         vm.expectRevert("opener must be eoa");
@@ -693,7 +774,7 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         vm.startPrank(recipient, recipient);
         vm.expectRevert("opening more than owned");
@@ -707,7 +788,7 @@ contract PackTest is BaseTest {
         uint256 packId = pack.nextTokenIdToMint();
         address recipient = address(0x123);
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 1000, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 1000, 1, 0, recipient);
 
         vm.startPrank(recipient, recipient);
         vm.expectRevert("cannot open yet");
@@ -721,7 +802,7 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
 
         vm.startPrank(recipient, recipient);
         vm.expectRevert("opening more than owned");
@@ -857,6 +938,7 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
+            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
@@ -905,7 +987,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ReentrancyGuard: reentrant call");
-        pack.createPack(content, rewards, packUri, 0, 1, recipient);
+        pack.createPack(content, rewards, packUri, 0, 1, 0, recipient);
     }
 }
 
@@ -925,7 +1007,7 @@ contract MaliciousERC20 is MockERC20, ITokenBundle {
         uint256[] memory rewards = new uint256[](1);
 
         address recipient = address(0x123);
-        pack.createPack(content, rewards, "", 0, 1, recipient);
+        pack.createPack(content, rewards, "", 0, 1, 0, recipient);
         return super.transferFrom(from, to, amount);
     }
 }
diff --git a/src/test/PackBenchmark.t.sol b/src/test/PackBenchmark.t.sol
index f1a9be149..6a64f20ec 100644
--- a/src/test/PackBenchmark.t.sol
+++ b/src/test/PackBenchmark.t.sol
@@ -128,7 +128,7 @@ contract CreatePackBenchmarkTest is BaseTest {
      *  note: Testing state changes; token owner calls `createPack` to pack owned tokens.
      */
     function test_benchmark_createPack() public {
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, address(0x123));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, address(0x123));
     }
 }
 
@@ -240,7 +240,7 @@ contract OpenPackBenchmarkTest is BaseTest {
         pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, address(0x123));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, address(0x123));
 
         vm.startPrank(address(0x123), address(0x123));
     }

From 58cbb2b70b7dd6b2f794b47ff85310a4b647c17f Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 1 Aug 2022 19:42:29 +0530
Subject: [PATCH 04/21] [G-2] Reduce function calls in openPack

---
 contracts/pack/Pack.sol | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 57052660c..024f0e8d5 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -227,11 +227,11 @@ contract Pack is
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
-        _burn(_msgSender(), _packId, _amountToOpen);
+        _burn(opener, _packId, _amountToOpen);
 
-        _transferTokenBatch(address(this), _msgSender(), rewardUnits);
+        _transferTokenBatch(address(this), opener, rewardUnits);
 
-        emit PackOpened(_packId, _msgSender(), _amountToOpen, rewardUnits);
+        emit PackOpened(_packId, opener, _amountToOpen, rewardUnits);
 
         return rewardUnits;
     }

From 728abff16435a59d2758f974988b9beba3147ba8 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 1 Aug 2022 20:01:55 +0530
Subject: [PATCH 05/21] [L-3] supportsInterface() may prevent ERC721 tokens
 from transferring to Pack

---
 contracts/pack/Pack.sol | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 024f0e8d5..0b0a56cce 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -9,6 +9,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/interfaces/IERC2981Upgradeable.sol";
+import "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
 
 //  ==========  Internal imports    ==========
 
@@ -161,7 +162,9 @@ contract Pack is
         override(ERC1155Receiver, ERC1155Upgradeable, IERC165)
         returns (bool)
     {
-        return super.supportsInterface(interfaceId) || type(IERC2981Upgradeable).interfaceId == interfaceId;
+        return super.supportsInterface(interfaceId) 
+                || type(IERC2981Upgradeable).interfaceId == interfaceId
+                || type(IERC721Receiver).interfaceId == interfaceId;
     }
 
     /*///////////////////////////////////////////////////////////////

From 39ac9726b0b7cf9ac61370031ebbf6a8cb17e0c6 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Fri, 5 Aug 2022 19:18:38 +0530
Subject: [PATCH 06/21] [H-1] Reward selection exploits

---
 contracts/Forwarder.sol                       |  2 +-
 .../metatx/MinimalForwarder.sol               | 68 +++++++++++++++++++
 contracts/pack/Pack.sol                       | 11 +--
 3 files changed, 76 insertions(+), 5 deletions(-)
 create mode 100644 contracts/openzeppelin-presets/metatx/MinimalForwarder.sol

diff --git a/contracts/Forwarder.sol b/contracts/Forwarder.sol
index e6a6ae9f9..8327c29f5 100644
--- a/contracts/Forwarder.sol
+++ b/contracts/Forwarder.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.11;
 
-import "@openzeppelin/contracts/metatx/MinimalForwarder.sol";
+import "./openzeppelin-presets/metatx/MinimalForwarder.sol";
 
 /*
  * @dev Minimal forwarder for GSNv2
diff --git a/contracts/openzeppelin-presets/metatx/MinimalForwarder.sol b/contracts/openzeppelin-presets/metatx/MinimalForwarder.sol
new file mode 100644
index 000000000..895f380ea
--- /dev/null
+++ b/contracts/openzeppelin-presets/metatx/MinimalForwarder.sol
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.5.0) (metatx/MinimalForwarder.sol)
+
+pragma solidity ^0.8.0;
+
+import "../utils/cryptography/ECDSA.sol";
+import "../utils/cryptography/EIP712.sol";
+
+/**
+ * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
+ */
+contract MinimalForwarder is EIP712 {
+    using ECDSA for bytes32;
+
+    struct ForwardRequest {
+        address from;
+        address to;
+        uint256 value;
+        uint256 gas;
+        uint256 nonce;
+        bytes data;
+    }
+
+    bytes32 private constant _TYPEHASH =
+        keccak256("ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data)");
+
+    mapping(address => uint256) private _nonces;
+
+    constructor() EIP712("MinimalForwarder", "0.0.1") {}
+
+    function getNonce(address from) public view returns (uint256) {
+        return _nonces[from];
+    }
+
+    function verify(ForwardRequest calldata req, bytes calldata signature) public view returns (bool) {
+        address signer = _hashTypedDataV4(
+            keccak256(abi.encode(_TYPEHASH, req.from, req.to, req.value, req.gas, req.nonce, keccak256(req.data)))
+        ).recover(signature);
+        return _nonces[req.from] == req.nonce && signer == req.from;
+    }
+
+    function execute(ForwardRequest calldata req, bytes calldata signature)
+        public
+        payable
+        returns (bool, bytes memory)
+    {
+        require(msg.sender == tx.origin, "not EOA");
+        require(verify(req, signature), "MinimalForwarder: signature does not match request");
+        _nonces[req.from] = req.nonce + 1;
+
+        (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(
+            abi.encodePacked(req.data, req.from)
+        );
+
+        // Validate that the relayer has sent enough gas for the call.
+        // See https://ronan.eth.link/blog/ethereum-gas-dangers/
+        if (gasleft() <= req.gas / 63) {
+            // We explicitly trigger invalid opcode to consume all gas and bubble-up the effects, since
+            // neither revert or assert consume all gas since Solidity 0.8.0
+            // https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require
+            assembly {
+                invalid()
+            }
+        }
+
+        return (success, returndata);
+    }
+}
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 0b0a56cce..530498099 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -44,6 +44,8 @@ contract Pack is
     bytes32 private constant MODULE_TYPE = bytes32("Pack");
     uint256 private constant VERSION = 1;
 
+    address[] private forwarders;
+
     // Token name
     string public name;
 
@@ -74,7 +76,9 @@ contract Pack is
                     Constructor + initializer logic
     //////////////////////////////////////////////////////////////*/
 
-    constructor(address _nativeTokenWrapper) TokenStore(_nativeTokenWrapper) initializer {}
+    constructor(address _nativeTokenWrapper, address[] memory _trustedForwarders) TokenStore(_nativeTokenWrapper) initializer {
+        forwarders = _trustedForwarders;
+    }
 
     /// @dev Initiliazes the contract, like a constructor.
     function initialize(
@@ -82,13 +86,12 @@ contract Pack is
         string memory _name,
         string memory _symbol,
         string memory _contractURI,
-        address[] memory _trustedForwarders,
         address _royaltyRecipient,
         uint256 _royaltyBps
     ) external initializer {
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
-        __ERC2771Context_init(_trustedForwarders);
+        __ERC2771Context_init(forwarders);
         __ERC1155Pausable_init();
         __ERC1155_init(_contractURI);
 
@@ -221,7 +224,7 @@ contract Pack is
     {
         address opener = _msgSender();
 
-        require(opener == tx.origin, "opener must be eoa");
+        require(isTrustedForwarder(msg.sender) || opener == tx.origin, "opener must be eoa");
         require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
 
         PackInfo memory pack = packInfo[_packId];

From e660250646dfb00008c36074325b5cbf21851239 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Fri, 5 Aug 2022 20:38:10 +0530
Subject: [PATCH 07/21] optimize openPack

---
 contracts/extension/TokenBundle.sol |   2 +-
 contracts/pack/Pack.sol             |  31 ++--
 src/test/Pack.t.sol                 |  59 ++++----
 src/test/PackBenchmark.t.sol        | 220 ++++++++++++++++++++++++++++
 4 files changed, 269 insertions(+), 43 deletions(-)

diff --git a/contracts/extension/TokenBundle.sol b/contracts/extension/TokenBundle.sol
index e3b83fec9..3fe61b07d 100644
--- a/contracts/extension/TokenBundle.sol
+++ b/contracts/extension/TokenBundle.sol
@@ -43,7 +43,7 @@ abstract contract TokenBundle is ITokenBundle {
     }
 
     /// @dev Lets the calling contract update a bundle, by passing in a list of tokens and a unique id.
-    function _updateBundle(Token[] calldata _tokensToBind, uint256 _bundleId) internal {
+    function _updateBundle(Token[] memory _tokensToBind, uint256 _bundleId) internal {
         require(_tokensToBind.length > 0, "TokenBundle: no tokens to bind.");
 
         uint256 currentCount = bundle[_bundleId].count;
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index e038bb271..2eb1c5bd1 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.11;
 
+import "/Users/yash/thirdweb/pack-audit-fixes/lib/forge-std/src/console2.sol";
 //  ==========  External imports    ==========
 
 import "@openzeppelin/contracts-upgradeable/token/ERC1155/extensions/ERC1155PausableUpgradeable.sol";
@@ -214,15 +215,12 @@ contract Pack is
         returns (Token[] memory)
     {
         address opener = _msgSender();
-
         require(isTrustedForwarder(opener) || opener == tx.origin, "opener must be eoa");
         require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
-
         PackInfo memory pack = packInfo[_packId];
         require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
-
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
-
+        console2.log("here 5");
         _burn(_msgSender(), _packId, _amountToOpen);
 
         _transferTokenBatch(address(this), _msgSender(), rewardUnits);
@@ -270,27 +268,28 @@ contract Pack is
     ) internal returns (Token[] memory rewardUnits) {
         uint256 numOfRewardUnitsToDistribute = _numOfPacksToOpen * _rewardUnitsPerOpen;
         rewardUnits = new Token[](numOfRewardUnitsToDistribute);
-
         uint256 totalRewardUnits = totalSupply[_packId] * _rewardUnitsPerOpen;
         uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
 
         uint256 random = generateRandomValue();
 
+        (Token[] memory _token,) = getPackContents(_packId);
+        bool[] memory _isUpdated = new bool[](totalRewardKinds);
         for (uint256 i = 0; i < numOfRewardUnitsToDistribute; i += 1) {
             uint256 randomVal = uint256(keccak256(abi.encode(random, i)));
             uint256 target = randomVal % totalRewardUnits;
             uint256 step;
 
             for (uint256 j = 0; j < totalRewardKinds; j += 1) {
-                uint256 id = _packId;
-
-                Token memory _token = getTokenOfBundle(id, j);
-                uint256 totalRewardUnitsOfKind = _token.totalAmount / pack.perUnitAmounts[j];
+                uint256 totalRewardUnitsOfKind = _token[j].totalAmount / pack.perUnitAmounts[j];
 
                 if (target < step + totalRewardUnitsOfKind) {
-                    _token.totalAmount -= pack.perUnitAmounts[j];
-                    _updateTokenInBundle(_token, id, j);
-                    rewardUnits[i] = _token;
+                    _token[j].totalAmount -= pack.perUnitAmounts[j];
+                    _isUpdated[j] = true;
+        
+                    rewardUnits[i].assetContract = _token[j].assetContract;
+                    rewardUnits[i].tokenType = _token[j].tokenType;
+                    rewardUnits[i].tokenId = _token[j].tokenId;
                     rewardUnits[i].totalAmount = pack.perUnitAmounts[j];
 
                     totalRewardUnits -= 1;
@@ -301,6 +300,12 @@ contract Pack is
                 }
             }
         }
+
+        for(uint256 i = 0; i < totalRewardKinds; i += 1) {
+            if(_isUpdated[i]) {
+                _updateTokenInBundle(_token[i], _packId, i);
+            }
+        }
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -309,7 +314,7 @@ contract Pack is
 
     /// @dev Returns the underlying contents of a pack.
     function getPackContents(uint256 _packId)
-        external
+        public
         view
         returns (Token[] memory contents, uint256[] memory perUnitAmounts)
     {
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index 30a1ade42..d306825e5 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -803,30 +803,30 @@ contract PackTest is BaseTest {
         erc1155Amounts = new uint256[](MAX_TOKENS);
 
         for (uint256 i = 0; i < rewardUnits.length; i++) {
-            console2.log("----- reward unit number: ", i, "------");
-            console2.log("asset contract: ", rewardUnits[i].assetContract);
-            console2.log("token type: ", uint256(rewardUnits[i].tokenType));
-            console2.log("tokenId: ", rewardUnits[i].tokenId);
+            // console2.log("----- reward unit number: ", i, "------");
+            // console2.log("asset contract: ", rewardUnits[i].assetContract);
+            // console2.log("token type: ", uint256(rewardUnits[i].tokenType));
+            // console2.log("tokenId: ", rewardUnits[i].tokenId);
             if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC20) {
                 if (rewardUnits[i].assetContract == address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE)) {
-                    console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
-                    console.log("balance of recipient: ", address(recipient).balance);
+                    // console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
+                    // console.log("balance of recipient: ", address(recipient).balance);
                     nativeTokenAmount += rewardUnits[i].totalAmount;
                 } else {
-                    console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
-                    console.log("balance of recipient: ", erc20.balanceOf(address(recipient)) / 1 ether, "ether");
+                    // console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
+                    // console.log("balance of recipient: ", erc20.balanceOf(address(recipient)) / 1 ether, "ether");
                     erc20Amount += rewardUnits[i].totalAmount;
                 }
             } else if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC1155) {
-                console2.log("total amount: ", rewardUnits[i].totalAmount);
-                console.log("balance of recipient: ", erc1155.balanceOf(address(recipient), rewardUnits[i].tokenId));
+                // console2.log("total amount: ", rewardUnits[i].totalAmount);
+                // console.log("balance of recipient: ", erc1155.balanceOf(address(recipient), rewardUnits[i].tokenId));
                 erc1155Amounts[rewardUnits[i].tokenId] += rewardUnits[i].totalAmount;
             } else if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC721) {
-                console2.log("total amount: ", rewardUnits[i].totalAmount);
-                console.log("balance of recipient: ", erc721.balanceOf(address(recipient)));
+                // console2.log("total amount: ", rewardUnits[i].totalAmount);
+                // console.log("balance of recipient: ", erc721.balanceOf(address(recipient)));
                 erc721Amount += rewardUnits[i].totalAmount;
             }
-            console2.log("");
+            // console2.log("");
         }
     }
 
@@ -940,10 +940,11 @@ contract PackTest is BaseTest {
         // x: 446, y: 22, z: 890 (gas: 8937393460518282035), total supply: 10203, total reward units: 224466
         // x: 335, y: 3, z: 1570 (gas: 8937393460517864076), total supply: 54915, total reward units: 164745
         // x: 1962, y: 282, z: 219 (gas: 8937393460523355524), total supply: 3239, total reward units: 913398
+        // x: 570, y: 497, z: 435 (gas: 8937393460523355524), total supply: 3239, total reward units: 913398
 
-        uint256 x = 1962;
-        uint128 y = 282;
-        uint256 z = 219;
+        uint256 x = 115;
+        uint128 y = 191;
+        uint256 z = 253;
 
         (ITokenBundle.Token[] memory tokensToPack, uint256[] memory rewardUnits) = getTokensToPack(x);
         if (tokensToPack.length == 0) {
@@ -981,22 +982,22 @@ contract PackTest is BaseTest {
         ITokenBundle.Token[] memory rewardsReceived = pack.openPack(packId, z);
         console2.log("received reward units: ", rewardsReceived.length);
 
-        assertEq(packUri, pack.uri(packId));
+        // assertEq(packUri, pack.uri(packId));
 
-        (
-            uint256 nativeTokenAmount,
-            uint256 erc20Amount,
-            uint256[] memory erc1155Amounts,
-            uint256 erc721Amount
-        ) = checkBalances(rewardsReceived, recipient);
+        // (
+        //     uint256 nativeTokenAmount,
+        //     uint256 erc20Amount,
+        //     uint256[] memory erc1155Amounts,
+        //     uint256 erc721Amount
+        // ) = checkBalances(rewardsReceived, recipient);
 
-        assertEq(address(recipient).balance, nativeTokenAmount);
-        assertEq(erc20.balanceOf(address(recipient)), erc20Amount);
-        assertEq(erc721.balanceOf(address(recipient)), erc721Amount);
+        // assertEq(address(recipient).balance, nativeTokenAmount);
+        // assertEq(erc20.balanceOf(address(recipient)), erc20Amount);
+        // assertEq(erc721.balanceOf(address(recipient)), erc721Amount);
 
-        for (uint256 i = 0; i < erc1155Amounts.length; i += 1) {
-            assertEq(erc1155.balanceOf(address(recipient), i), erc1155Amounts[i]);
-        }
+        // for (uint256 i = 0; i < erc1155Amounts.length; i += 1) {
+        //     assertEq(erc1155.balanceOf(address(recipient), i), erc1155Amounts[i]);
+        // }
     }
 
     /*///////////////////////////////////////////////////////////////
diff --git a/src/test/PackBenchmark.t.sol b/src/test/PackBenchmark.t.sol
index f1a9be149..3546bd63c 100644
--- a/src/test/PackBenchmark.t.sol
+++ b/src/test/PackBenchmark.t.sol
@@ -256,3 +256,223 @@ contract OpenPackBenchmarkTest is BaseTest {
         pack.openPack(0, 1);
     }
 }
+
+contract OpenPackLargeInputsTest is BaseTest {
+    Pack internal pack;
+
+    Wallet internal tokenOwner;
+    address recipient = address(0x123);
+    string internal packUri;
+
+    uint256 packId;
+    uint256 totalRewardUnits;
+    uint256 totalSupply;
+
+    uint256 x;
+    uint128 y;
+    uint256 z;
+
+    uint256 internal constant MAX_TOKENS = 2000;
+
+    function setUp() public override {
+        super.setUp();
+
+        pack = Pack(payable(getContract("Pack")));
+
+        tokenOwner = getWallet();
+        packUri = "ipfs://";
+
+        tokenOwner.setAllowanceERC20(address(erc20), address(pack), type(uint256).max);
+        tokenOwner.setApprovalForAllERC721(address(erc721), address(pack), true);
+        tokenOwner.setApprovalForAllERC1155(address(erc1155), address(pack), true);
+
+        vm.prank(deployer);
+        pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
+
+        // pass (1478, 4, 1890).. (gas: 8937393460521767975), total supply: 177189, total reward units: 708756
+        // pass (472, 1, 543).. (gas: 8937393460518373840), total supply: 236220, total reward units: 236220
+        // pass (96, 112, 447).. (gas: 8937393460517062690), total supply: 467, total reward units: 52304
+        // (506, 6, 12950).. (gas: 8937393460518476945), total supply: 41699, total reward units: 250194
+        // pass (164, 20, 922).. (gas: 8937393460517318850), total supply: 4335, total reward units: 86700
+        // pass (138, 2, 948).. (gas: 8937393460517220399), total supply: 37959, total reward units: 75918
+        // pass (32, 11, 978).. (gas: 8937393460516848598), total supply: 1456, total reward units: 16016
+
+        // pass x: 446, y: 22, z: 890 (gas: 8937393460518282035), total supply: 10203, total reward units: 224466
+        // pass x: 335, y: 3, z: 1570 (gas: 8937393460517864076), total supply: 54915, total reward units: 164745
+        // pass x: 1962, y: 282, z: 219 (gas: 8937393460523355524), total supply: 3239, total reward units: 913398
+
+        // x: 570, y: 497, z: 435 (gas: 8937393460523355524), total supply: 548, total reward units: 272356
+        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute); 
+        // x: 412, y: 7, z: 11830 (gas: 8937393460523355524), total supply: 29834, total reward units: 208838
+        //                          reverts while transferring reward units to receipient
+        // x: 1322, y: 211, z: 1994 (gas: 8937393460523355524), total supply: 3104, total reward units: 6544944
+        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute);
+        // x: 1578, y: 1294, z: 515 (gas: 8937393460523355524), total supply: 580, total reward units: 750520
+        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute);
+        // x: 404, y: 38, z: 3950 (gas: 8937393460523355524), total supply: 5201, total reward units: 197638
+        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute);
+        x = 404;
+        y = 38;
+        z = 1700;
+
+        (ITokenBundle.Token[] memory tokensToPack, uint256[] memory rewardUnits) = getTokensToPack(x);
+        if (tokensToPack.length == 0) {
+            return;
+        }
+
+        packId = pack.nextTokenIdToMint();
+        uint256 nativeTokenPacked;
+
+        for (uint256 i = 0; i < tokensToPack.length; i += 1) {
+            totalRewardUnits += rewardUnits[i];
+            if (tokensToPack[i].assetContract == address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE)) {
+                nativeTokenPacked += tokensToPack[i].totalAmount;
+            }
+        }
+        vm.assume(y > 0 && totalRewardUnits % y == 0);
+        vm.deal(address(tokenOwner), nativeTokenPacked);
+
+        vm.prank(address(tokenOwner));
+        (, totalSupply) = pack.createPack{ value: nativeTokenPacked }(
+            tokensToPack,
+            rewardUnits,
+            packUri,
+            0,
+            y,
+            recipient
+        );
+        
+        vm.assume(z <= totalSupply);
+    }
+
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: `openPack`
+    //////////////////////////////////////////////////////////////*/
+
+    /**
+     *  note: Testing state changes; pack owner calls `openPack` to redeem underlying rewards.
+     */
+    function test_fuzz_failing_state_openPack() public {
+
+        console.log(gasleft());
+        console2.log("total supply: ", totalSupply);
+        console2.log("total reward units: ", totalRewardUnits);
+
+        vm.prank(recipient, recipient);
+        ITokenBundle.Token[] memory rewardsReceived = pack.openPack(packId, z);
+        console2.log("received reward units: ", rewardsReceived.length);
+
+        assertEq(packUri, pack.uri(packId));
+
+        (
+            uint256 nativeTokenAmount,
+            uint256 erc20Amount,
+            uint256[] memory erc1155Amounts,
+            uint256 erc721Amount
+        ) = checkBalances(rewardsReceived, recipient);
+
+        assertEq(address(recipient).balance, nativeTokenAmount);
+        assertEq(erc20.balanceOf(address(recipient)), erc20Amount);
+        assertEq(erc721.balanceOf(address(recipient)), erc721Amount);
+
+        for (uint256 i = 0; i < erc1155Amounts.length; i += 1) {
+            assertEq(erc1155.balanceOf(address(recipient), i), erc1155Amounts[i]);
+        }
+    }
+
+    function getTokensToPack(uint256 len)
+        internal
+        returns (ITokenBundle.Token[] memory tokensToPack, uint256[] memory rewardUnits)
+    {
+        vm.assume(len < MAX_TOKENS);
+        tokensToPack = new ITokenBundle.Token[](len);
+        rewardUnits = new uint256[](len);
+
+        for (uint256 i = 0; i < len; i += 1) {
+            uint256 random = uint256(keccak256(abi.encodePacked(len + i))) % MAX_TOKENS;
+            uint256 selector = random % 4;
+
+            if (selector == 0) {
+                tokensToPack[i] = ITokenBundle.Token({
+                    assetContract: address(erc20),
+                    tokenType: ITokenBundle.TokenType.ERC20,
+                    tokenId: 0,
+                    totalAmount: (random + 1) * 10 ether
+                });
+                rewardUnits[i] = random + 1;
+
+                erc20.mint(address(tokenOwner), tokensToPack[i].totalAmount);
+            } else if (selector == 1) {
+                uint256 tokenId = erc721.nextTokenIdToMint();
+
+                tokensToPack[i] = ITokenBundle.Token({
+                    assetContract: address(erc721),
+                    tokenType: ITokenBundle.TokenType.ERC721,
+                    tokenId: tokenId,
+                    totalAmount: 1
+                });
+                rewardUnits[i] = 1;
+
+                erc721.mint(address(tokenOwner), 1);
+            } else if (selector == 2) {
+                tokensToPack[i] = ITokenBundle.Token({
+                    assetContract: address(erc1155),
+                    tokenType: ITokenBundle.TokenType.ERC1155,
+                    tokenId: random,
+                    totalAmount: (random + 1) * 10
+                });
+                rewardUnits[i] = random + 1;
+
+                erc1155.mint(address(tokenOwner), tokensToPack[i].tokenId, tokensToPack[i].totalAmount);
+            } else if (selector == 3) {
+                tokensToPack[i] = ITokenBundle.Token({
+                    assetContract: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE),
+                    tokenType: ITokenBundle.TokenType.ERC20,
+                    tokenId: 0,
+                    totalAmount: 5 ether
+                });
+                rewardUnits[i] = 5;
+            }
+        }
+    }
+
+    function checkBalances(ITokenBundle.Token[] memory rewardUnits, address recipient)
+        internal
+        view
+        returns (
+            uint256 nativeTokenAmount,
+            uint256 erc20Amount,
+            uint256[] memory erc1155Amounts,
+            uint256 erc721Amount
+        )
+    {
+        erc1155Amounts = new uint256[](MAX_TOKENS);
+
+        for (uint256 i = 0; i < rewardUnits.length; i++) {
+            // console2.log("----- reward unit number: ", i, "------");
+            // console2.log("asset contract: ", rewardUnits[i].assetContract);
+            // console2.log("token type: ", uint256(rewardUnits[i].tokenType));
+            // console2.log("tokenId: ", rewardUnits[i].tokenId);
+            if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC20) {
+                if (rewardUnits[i].assetContract == address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE)) {
+                    // console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
+                    // console.log("balance of recipient: ", address(recipient).balance);
+                    nativeTokenAmount += rewardUnits[i].totalAmount;
+                } else {
+                    // console2.log("total amount: ", rewardUnits[i].totalAmount / 1 ether, "ether");
+                    // console.log("balance of recipient: ", erc20.balanceOf(address(recipient)) / 1 ether, "ether");
+                    erc20Amount += rewardUnits[i].totalAmount;
+                }
+            } else if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC1155) {
+                // console2.log("total amount: ", rewardUnits[i].totalAmount);
+                // console.log("balance of recipient: ", erc1155.balanceOf(address(recipient), rewardUnits[i].tokenId));
+                erc1155Amounts[rewardUnits[i].tokenId] += rewardUnits[i].totalAmount;
+            } else if (rewardUnits[i].tokenType == ITokenBundle.TokenType.ERC721) {
+                // console2.log("total amount: ", rewardUnits[i].totalAmount);
+                // console.log("balance of recipient: ", erc721.balanceOf(address(recipient)));
+                erc721Amount += rewardUnits[i].totalAmount;
+            }
+            // console2.log("");
+        }
+    }
+}

From b92d8365a83da50804962508595eda3b0fd579bd Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Sat, 6 Aug 2022 01:25:01 +0530
Subject: [PATCH 08/21] pack refactor: adding more tokens to a pack

---
 contracts/extension/TokenBundle.sol |  2 +-
 contracts/extension/TokenStore.sol  |  2 +-
 contracts/interfaces/IPack.sol      |  8 +++
 contracts/pack/Pack.sol             | 77 ++++++++++++++++++++++++-----
 4 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/contracts/extension/TokenBundle.sol b/contracts/extension/TokenBundle.sol
index 3fe61b07d..1bb23eb27 100644
--- a/contracts/extension/TokenBundle.sol
+++ b/contracts/extension/TokenBundle.sol
@@ -111,7 +111,7 @@ abstract contract TokenBundle is ITokenBundle {
     }
 
     /// @dev Lets the calling contract set/update the uri of a particular bundle.
-    function _setUriOfBundle(string calldata _uri, uint256 _bundleId) internal {
+    function _setUriOfBundle(string memory _uri, uint256 _bundleId) internal {
         bundle[_bundleId].uri = _uri;
     }
 
diff --git a/contracts/extension/TokenStore.sol b/contracts/extension/TokenStore.sol
index acbea92d7..90ad15983 100644
--- a/contracts/extension/TokenStore.sol
+++ b/contracts/extension/TokenStore.sol
@@ -29,7 +29,7 @@ contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {
     function _storeTokens(
         address _tokenOwner,
         Token[] calldata _tokens,
-        string calldata _uriForTokens,
+        string memory _uriForTokens,
         uint256 _idForTokens
     ) internal {
         _createBundle(_tokens, _idForTokens);
diff --git a/contracts/interfaces/IPack.sol b/contracts/interfaces/IPack.sol
index 66947abc8..ece5534a7 100644
--- a/contracts/interfaces/IPack.sol
+++ b/contracts/interfaces/IPack.sol
@@ -33,6 +33,14 @@ interface IPack is ITokenBundle {
         uint256 totalPacksCreated
     );
 
+    /// @notice Emitted when more packs are minted for a packId.
+    event PackUpdated(
+        uint256 indexed packId,
+        address indexed packCreator,
+        address recipient,
+        uint256 totalPacksCreated
+    );
+
     /// @notice Emitted when a pack is opened.
     event PackOpened(
         uint256 indexed packId,
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 8e1f9258b..1d151d584 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.11;
 
-import "/Users/yash/thirdweb/pack-audit-fixes/lib/forge-std/src/console2.sol";
 //  ==========  External imports    ==========
 
 import "@openzeppelin/contracts-upgradeable/token/ERC1155/extensions/ERC1155PausableUpgradeable.sol";
@@ -73,6 +72,9 @@ contract Pack is
     /// @dev Mapping from pack ID => The state of that set of packs.
     mapping(uint256 => PackInfo) private packInfo;
 
+    /// @dev Checks if pack-creator allowed to add more tokens to a packId; set to false after first transfer
+    mapping(uint256 => bool) public canUpdatePack;
+
     /*///////////////////////////////////////////////////////////////
                     Constructor + initializer logic
     //////////////////////////////////////////////////////////////*/
@@ -179,7 +181,7 @@ contract Pack is
     function createPack(
         Token[] calldata _contents,
         uint256[] calldata _numOfRewardUnits,
-        string calldata _packUri,
+        string memory _packUri,
         uint128 _openStartTimestamp,
         uint128 _amountDistributedPerOpen,
         uint256 _expirationTimestamp,
@@ -204,18 +206,55 @@ contract Pack is
         packId = nextTokenIdToMint;
         nextTokenIdToMint += 1;
 
-        packTotalSupply = escrowPackContents(_contents, _numOfRewardUnits, _packUri, packId, _amountDistributedPerOpen);
+        packTotalSupply = escrowPackContents(_contents, _numOfRewardUnits, _packUri, packId, _amountDistributedPerOpen, false);
 
         packInfo[packId].openStartTimestamp = _openStartTimestamp;
         packInfo[packId].amountDistributedPerOpen = _amountDistributedPerOpen;
         packInfo[packId].expirationTimestamp = _expirationTimestamp == 0 ? type(uint256).max : _expirationTimestamp;
         packInfo[packId].creator = _msgSender();
 
+        canUpdatePack[packId] = true;
+
         _mint(_recipient, packId, packTotalSupply, "");
 
         emit PackCreated(packId, _msgSender(), _recipient, packTotalSupply);
     }
 
+    function addPackContents(
+        uint256 _packId,
+        Token[] calldata _contents,
+        uint256[] calldata _numOfRewardUnits,
+        address _recipient
+    )
+        external
+        payable
+        onlyRoleWithSwitch(MINTER_ROLE)
+        nonReentrant
+        whenNotPaused
+        returns (uint256 packTotalSupply, uint256 newSupplyAdded)
+    {
+        require(canUpdatePack[_packId], "not allowed");
+        require(_msgSender() == packInfo[_packId].creator, "not creator");
+
+        require(_contents.length > 0, "nothing to pack");
+        require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
+
+        if (!hasRole(ASSET_ROLE, address(0))) {
+            for (uint256 i = 0; i < _contents.length; i += 1) {
+                _checkRole(ASSET_ROLE, _contents[i].assetContract);
+            }
+        }
+
+        uint256 amountPerOpen = packInfo[_packId].amountDistributedPerOpen;
+
+        newSupplyAdded = escrowPackContents(_contents, _numOfRewardUnits, "", _packId, amountPerOpen, true);
+        packTotalSupply = totalSupply[_packId] + newSupplyAdded;
+
+        _mint(_recipient, _packId, newSupplyAdded, "");
+
+        emit PackUpdated(_packId, _msgSender(), _recipient, newSupplyAdded);
+    }
+
     /// @notice Lets a pack owner open packs and receive the packs' reward units.
     function openPack(uint256 _packId, uint256 _amountToOpen)
         external
@@ -257,11 +296,12 @@ contract Pack is
     function escrowPackContents(
         Token[] calldata _contents,
         uint256[] calldata _numOfRewardUnits,
-        string calldata _packUri,
+        string memory _packUri,
         uint256 packId,
-        uint256 amountPerOpen
-    ) internal returns (uint256 packTotalSupply) {
-        uint256 totalRewardUnits;
+        uint256 amountPerOpen,
+        bool isUpdate
+    ) internal returns (uint256 supplyToMint) {
+        uint256 sumOfRewardUnits;
 
         for (uint256 i = 0; i < _contents.length; i += 1) {
             require(_contents[i].totalAmount != 0, "amount can't be zero");
@@ -271,15 +311,23 @@ contract Pack is
                 "invalid erc721 rewards"
             );
 
-            totalRewardUnits += _numOfRewardUnits[i];
+            sumOfRewardUnits += _numOfRewardUnits[i];
 
             packInfo[packId].perUnitAmounts.push(_contents[i].totalAmount / _numOfRewardUnits[i]);
         }
 
-        require(totalRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
-        packTotalSupply = totalRewardUnits / amountPerOpen;
+        require(sumOfRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
+        supplyToMint = sumOfRewardUnits / amountPerOpen;
+
+        if(isUpdate) {
+            for(uint256 i = 0; i < _contents.length; i += 1) {
+                _addTokenInBundle(_contents[i], packId);
+            }
+            _transferTokenBatch(_msgSender(), address(this), _contents);
+        } else {
+            _storeTokens(_msgSender(), _contents, _packUri, packId);
+        }
 
-        _storeTokens(_msgSender(), _contents, _packUri, packId);
     }
 
     /// @dev Returns the reward units to distribute.
@@ -412,6 +460,13 @@ contract Pack is
             for (uint256 i = 0; i < ids.length; ++i) {
                 totalSupply[ids[i]] += amounts[i];
             }
+        } else {
+            for (uint256 i = 0; i < ids.length; ++i) {
+                // pack can no longer be updated after first transfer to non-zero address
+                if(canUpdatePack[ids[i]]) {
+                    canUpdatePack[ids[i]] = false;
+                }
+            }
         }
 
         if (to == address(0)) {

From 4dacb78c3044bd656c8a6f6369aeb53677713e2f Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Sat, 6 Aug 2022 01:26:18 +0530
Subject: [PATCH 09/21] add tests

---
 lib/forge-std                |   2 +-
 src/test/Pack.t.sol          | 163 +++++++++++++++++++++++++++++++++++
 src/test/PackBenchmark.t.sol |   1 +
 src/test/utils/BaseTest.sol  |   6 +-
 4 files changed, 168 insertions(+), 4 deletions(-)

diff --git a/lib/forge-std b/lib/forge-std
index 27e14b7f2..2c7cbfc6f 160000
--- a/lib/forge-std
+++ b/lib/forge-std
@@ -1 +1 @@
-Subproject commit 27e14b7f2448e5f5ac32719f51fe652aa0b0733e
+Subproject commit 2c7cbfc6fbede6d7c9e6b17afe997e3fdfe22fef
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index 10e0a1d02..c0fb508d6 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -32,7 +32,9 @@ contract PackTest is BaseTest {
     Wallet internal tokenOwner;
     string internal packUri;
     ITokenBundle.Token[] internal packContents;
+    ITokenBundle.Token[] internal additionalContents;
     uint256[] internal numOfRewardUnits;
+    uint256[] internal additionalContentsRewardUnits;
 
     function setUp() public override {
         super.setUp();
@@ -147,6 +149,27 @@ contract PackTest is BaseTest {
         erc1155.mint(address(tokenOwner), 0, 100);
         erc1155.mint(address(tokenOwner), 1, 500);
 
+        // additional contents, to check `addPackContents`
+        additionalContents.push(
+            ITokenBundle.Token({
+                assetContract: address(erc1155),
+                tokenType: ITokenBundle.TokenType.ERC1155,
+                tokenId: 2,
+                totalAmount: 200
+            })
+        );
+        additionalContentsRewardUnits.push(50);
+
+        additionalContents.push(
+            ITokenBundle.Token({
+                assetContract: address(erc20),
+                tokenType: ITokenBundle.TokenType.ERC20,
+                tokenId: 0,
+                totalAmount: 1000 ether
+            })
+        );
+        additionalContentsRewardUnits.push(100);
+
         tokenOwner.setAllowanceERC20(address(erc20), address(pack), type(uint256).max);
         tokenOwner.setApprovalForAllERC721(address(erc721), address(pack), true);
         tokenOwner.setApprovalForAllERC1155(address(erc1155), address(pack), true);
@@ -628,6 +651,144 @@ contract PackTest is BaseTest {
         pack.createPack(packContents, rewardUnits, packUri, 0, 1, 0, recipient);
     }
 
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: `addPackContents`
+    //////////////////////////////////////////////////////////////*/
+
+    /**
+     *  note: Testing state changes; token owner calls `addPackContents` to pack more tokens.
+     */
+    function test_state_addPackContents() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+
+        (ITokenBundle.Token[] memory packed, ) = pack.getPackContents(packId);
+        assertEq(packed.length, packContents.length);
+        for (uint256 i = 0; i < packed.length; i += 1) {
+            assertEq(packed[i].assetContract, packContents[i].assetContract);
+            assertEq(uint256(packed[i].tokenType), uint256(packContents[i].tokenType));
+            assertEq(packed[i].tokenId, packContents[i].tokenId);
+            assertEq(packed[i].totalAmount, packContents[i].totalAmount);
+        }
+
+        erc20.mint(address(tokenOwner), 1000 ether);
+        erc1155.mint(address(tokenOwner), 2, 200);
+
+        vm.prank(address(tokenOwner));
+        pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+
+        (packed, ) = pack.getPackContents(packId);
+        assertEq(packed.length, packContents.length + additionalContents.length);
+        for (uint256 i = packContents.length; i < packed.length; i += 1) {
+            assertEq(packed[i].assetContract, additionalContents[i - packContents.length].assetContract);
+            assertEq(uint256(packed[i].tokenType), uint256(additionalContents[i - packContents.length].tokenType));
+            assertEq(packed[i].tokenId, additionalContents[i - packContents.length].tokenId);
+            assertEq(packed[i].totalAmount, additionalContents[i - packContents.length].totalAmount);
+        }
+    }
+
+    /**
+     *  note: Testing token balances; token owner calls `addPackContents` to pack more tokens
+     *        in an already existing pack.
+     */
+    function test_balances_addPackContents() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+
+        // ERC20 balance
+        assertEq(erc20.balanceOf(address(tokenOwner)), 0);
+        assertEq(erc20.balanceOf(address(pack)), 2000 ether);
+
+        // ERC721 balance
+        assertEq(erc721.ownerOf(0), address(pack));
+        assertEq(erc721.ownerOf(1), address(pack));
+        assertEq(erc721.ownerOf(2), address(pack));
+        assertEq(erc721.ownerOf(3), address(pack));
+        assertEq(erc721.ownerOf(4), address(pack));
+        assertEq(erc721.ownerOf(5), address(pack));
+
+        // ERC1155 balance
+        assertEq(erc1155.balanceOf(address(tokenOwner), 0), 0);
+        assertEq(erc1155.balanceOf(address(pack), 0), 100);
+
+        assertEq(erc1155.balanceOf(address(tokenOwner), 1), 0);
+        assertEq(erc1155.balanceOf(address(pack), 1), 500);
+
+        // Pack wrapped token balance
+        assertEq(pack.balanceOf(address(recipient), packId), totalSupply);
+
+        erc20.mint(address(tokenOwner), 1000 ether);
+        erc1155.mint(address(tokenOwner), 2, 200);
+
+        vm.prank(address(tokenOwner));
+        (uint256 newTotalSupply, uint256 additionalSupply) = pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+
+        // ERC20 balance after adding more tokens
+        assertEq(erc20.balanceOf(address(tokenOwner)), 0);
+        assertEq(erc20.balanceOf(address(pack)), 3000 ether);
+
+        // ERC1155 balance after adding more tokens
+        assertEq(erc1155.balanceOf(address(tokenOwner), 2), 0);
+        assertEq(erc1155.balanceOf(address(pack), 2), 200);
+
+        // Pack wrapped token balance
+        assertEq(pack.balanceOf(address(recipient), packId), newTotalSupply);
+        assertEq(totalSupply + additionalSupply, newTotalSupply);
+    }
+
+    /**
+     *  note: Testing revert condition; non-creator calls `addPackContents`.
+     */
+    function test_revert_addPackContents_NotCreator() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+
+        address randomAccount = address(0x123);
+
+        vm.prank(deployer);
+        pack.grantRole(keccak256("MINTER_ROLE"), randomAccount);
+
+        vm.prank(randomAccount);
+        vm.expectRevert("not creator");
+        pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+    }
+
+    /**
+     *  note: Testing revert condition; adding tokens to non-existent pack.
+     */
+    function test_revert_addPackContents_PackNonExistent() public {
+        vm.prank(address(tokenOwner));
+        vm.expectRevert("not allowed");
+        pack.addPackContents(0, packContents, numOfRewardUnits, address(1));
+    }
+
+    /**
+     *  note: Testing revert condition; adding tokens after packs have been distributed.
+     */
+    function test_revert_addPackContents_CantUpdateAnymore() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+
+        vm.prank(recipient);
+        pack.safeTransferFrom(recipient, address(567), packId, 1, "");
+
+        vm.prank(address(tokenOwner));
+        vm.expectRevert("not allowed");
+        pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+    }
+    
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `openPack`
     //////////////////////////////////////////////////////////////*/
@@ -990,6 +1151,7 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
+            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
@@ -1054,6 +1216,7 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
+            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
diff --git a/src/test/PackBenchmark.t.sol b/src/test/PackBenchmark.t.sol
index 24f08bb02..b60970709 100644
--- a/src/test/PackBenchmark.t.sol
+++ b/src/test/PackBenchmark.t.sol
@@ -339,6 +339,7 @@ contract OpenPackLargeInputsTest is BaseTest {
             packUri,
             0,
             y,
+            0,
             recipient
         );
         
diff --git a/src/test/utils/BaseTest.sol b/src/test/utils/BaseTest.sol
index ec7333c66..e1fe96ee2 100644
--- a/src/test/utils/BaseTest.sol
+++ b/src/test/utils/BaseTest.sol
@@ -9,7 +9,7 @@ import "../mocks/WETH9.sol";
 import "../mocks/MockERC20.sol";
 import "../mocks/MockERC721.sol";
 import "../mocks/MockERC1155.sol";
-import "contracts/Forwarder.sol";
+import { Forwarder } from "contracts/Forwarder.sol";
 import "contracts/TWFee.sol";
 import "contracts/TWRegistry.sol";
 import "contracts/TWFactory.sol";
@@ -91,7 +91,7 @@ abstract contract BaseTest is DSTest, Test {
         TWFactory(factory).addImplementation(address(new Split(fee)));
         TWFactory(factory).addImplementation(address(new Multiwrap(address(weth))));
         TWFactory(factory).addImplementation(address(new MockContract(bytes32("Pack"), 1)));
-        TWFactory(factory).addImplementation(address(new Pack(address(weth))));
+        TWFactory(factory).addImplementation(address(new Pack(address(weth), forwarders())));
         TWFactory(factory).addImplementation(address(new VoteERC20()));
         vm.stopPrank();
 
@@ -236,7 +236,7 @@ abstract contract BaseTest is DSTest, Test {
             "Pack",
             abi.encodeCall(
                 Pack.initialize,
-                (deployer, NAME, SYMBOL, CONTRACT_URI, forwarders(), royaltyRecipient, royaltyBps)
+                (deployer, NAME, SYMBOL, CONTRACT_URI, royaltyRecipient, royaltyBps)
             )
         );
     }

From 83c21f247a42011ee8f67478e84d040b92745801 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Thu, 18 Aug 2022 12:55:27 +0530
Subject: [PATCH 10/21] wip: reduce size

---
 contracts/extension/TokenBundle.sol |  20 ++--
 contracts/extension/TokenStore.sol  |   9 +-
 contracts/multiwrap/Multiwrap.sol   |   9 +-
 contracts/pack/Pack.sol             | 169 +++++++++++++++++++---------
 docs/ContractPublisher.md           |   8 +-
 docs/IContractPublisher.md          |   8 +-
 docs/IPack.md                       |  22 +++-
 docs/Pack.md                        | 112 +++++++++++++++++-
 hardhat.config.ts                   |   2 +-
 9 files changed, 275 insertions(+), 84 deletions(-)

diff --git a/contracts/extension/TokenBundle.sol b/contracts/extension/TokenBundle.sol
index 1bb23eb27..9ddd0c75d 100644
--- a/contracts/extension/TokenBundle.sol
+++ b/contracts/extension/TokenBundle.sol
@@ -10,22 +10,22 @@ interface IERC165 {
 
 abstract contract TokenBundle is ITokenBundle {
     /// @dev Mapping from bundle UID => bundle info.
-    mapping(uint256 => BundleInfo) private bundle;
+    mapping(uint256 => BundleInfo) public bundle;
 
     /// @dev Returns the total number of assets in a particular bundle.
-    function getTokenCountOfBundle(uint256 _bundleId) public view returns (uint256) {
-        return bundle[_bundleId].count;
-    }
+    // function getTokenCountOfBundle(uint256 _bundleId) public view returns (uint256) {
+    //     return bundle[_bundleId].count;
+    // }
 
     /// @dev Returns an asset contained in a particular bundle, at a particular index.
-    function getTokenOfBundle(uint256 _bundleId, uint256 index) public view returns (Token memory) {
-        return bundle[_bundleId].tokens[index];
-    }
+    // function getTokenOfBundle(uint256 _bundleId, uint256 index) public view returns (Token memory) {
+    //     return bundle[_bundleId].tokens[index];
+    // }
 
     /// @dev Returns the uri of a particular bundle.
-    function getUriOfBundle(uint256 _bundleId) public view returns (string memory) {
-        return bundle[_bundleId].uri;
-    }
+    // function getUriOfBundle(uint256 _bundleId) public view returns (string memory) {
+    //     return bundle[_bundleId].uri;
+    // }
 
     /// @dev Lets the calling contract create a bundle, by passing in a list of tokens and a unique id.
     function _createBundle(Token[] calldata _tokensToBind, uint256 _bundleId) internal {
diff --git a/contracts/extension/TokenStore.sol b/contracts/extension/TokenStore.sol
index 90ad15983..69ec0f35c 100644
--- a/contracts/extension/TokenStore.sol
+++ b/contracts/extension/TokenStore.sol
@@ -15,9 +15,6 @@ import { TokenBundle, ITokenBundle } from "./TokenBundle.sol";
 import "../lib/CurrencyTransferLib.sol";
 
 contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {
-    /// @dev The address interpreted as native token of the chain.
-    address public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
-
     /// @dev The address of the native token wrapper contract.
     address internal immutable nativeTokenWrapper;
 
@@ -39,11 +36,13 @@ contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {
 
     /// @dev Release stored / escrowed ERC1155, ERC721, ERC20 tokens.
     function _releaseTokens(address _recipient, uint256 _idForContent) internal {
-        uint256 count = getTokenCountOfBundle(_idForContent);
+        // uint256 count = getTokenCountOfBundle(_idForContent);
+        uint256 count = bundle[_idForContent].count;
         Token[] memory tokensToRelease = new Token[](count);
 
         for (uint256 i = 0; i < count; i += 1) {
-            tokensToRelease[i] = getTokenOfBundle(_idForContent, i);
+            // tokensToRelease[i] = getTokenOfBundle(_idForContent, i);
+            tokensToRelease[i] = bundle[_idForContent].tokens[i];
         }
 
         _deleteBundle(_idForContent);
diff --git a/contracts/multiwrap/Multiwrap.sol b/contracts/multiwrap/Multiwrap.sol
index 1b8b7b33c..5fe2c8c8f 100644
--- a/contracts/multiwrap/Multiwrap.sol
+++ b/contracts/multiwrap/Multiwrap.sol
@@ -127,7 +127,8 @@ contract Multiwrap is
 
     /// @dev Returns the URI for a given tokenId.
     function tokenURI(uint256 _tokenId) public view override returns (string memory) {
-        return getUriOfBundle(_tokenId);
+        // return getUriOfBundle(_tokenId);
+        return bundle[_tokenId].uri;
     }
 
     /// @dev See ERC 165
@@ -188,11 +189,13 @@ contract Multiwrap is
 
     /// @dev Returns the underlying contents of a wrapped NFT.
     function getWrappedContents(uint256 _tokenId) external view returns (Token[] memory contents) {
-        uint256 total = getTokenCountOfBundle(_tokenId);
+        // uint256 total = getTokenCountOfBundle(_tokenId);
+        uint256 total = bundle[_tokenId].count;
         contents = new Token[](total);
 
         for (uint256 i = 0; i < total; i += 1) {
-            contents[i] = getTokenOfBundle(_tokenId, i);
+            // contents[i] = getTokenOfBundle(_tokenId, i);
+            contents[i] = bundle[_tokenId].tokens[i];
         }
     }
 
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 1d151d584..ccc997ab2 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -29,12 +29,12 @@ contract Pack is
     ContractMetadata,
     Ownable,
     Royalty,
-    PermissionsEnumerable,
+    Permissions,
     TokenStore,
     ReentrancyGuardUpgradeable,
     ERC2771ContextUpgradeable,
     MulticallUpgradeable,
-    ERC1155PausableUpgradeable,
+    ERC1155Upgradeable,
     IPack
 {
     /*///////////////////////////////////////////////////////////////
@@ -53,11 +53,14 @@ contract Pack is
     string public symbol;
 
     /// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
-    bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
+    // bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
+    bytes32 private transferRole;
     /// @dev Only MINTER_ROLE holders can create packs.
-    bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    // bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    bytes32 private minterRole;
     /// @dev Only assets with ASSET_ROLE can be packed, when packing is restricted to particular assets.
-    bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");
+    // bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");
+    bytes32 private assetRole;
 
     /// @dev The token Id of the next set of packs to be minted.
     uint256 public nextTokenIdToMint;
@@ -92,10 +95,13 @@ contract Pack is
         address _royaltyRecipient,
         uint256 _royaltyBps
     ) external initializer {
+        transferRole = keccak256("TRANSFER_ROLE");
+        minterRole = keccak256("MINTER_ROLE");
+        assetRole = keccak256("ASSET_ROLE");
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
         __ERC2771Context_init(forwarders);
-        __ERC1155Pausable_init();
+        // __ERC1155Pausable_init();
         __ERC1155_init(_contractURI);
 
         name = _name;
@@ -105,18 +111,21 @@ contract Pack is
         _setupOwner(_defaultAdmin);
 
         _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
-        _setupRole(TRANSFER_ROLE, _defaultAdmin);
-        _setupRole(MINTER_ROLE, _defaultAdmin);
-        _setupRole(TRANSFER_ROLE, address(0));
+        _setupRole(transferRole, _defaultAdmin);
+        _setupRole(minterRole, _defaultAdmin);
+        _setupRole(transferRole, address(0));
 
         // note: see `onlyRoleWithSwitch` for ASSET_ROLE behaviour.
-        _setupRole(ASSET_ROLE, address(0));
+        _setupRole(assetRole, address(0));
 
         _setupDefaultRoyaltyInfo(_royaltyRecipient, _royaltyBps);
     }
 
     receive() external payable {
-        require(msg.sender == nativeTokenWrapper, "Caller is not native token wrapper.");
+        // require(msg.sender == nativeTokenWrapper, "Caller is not native token wrapper.");
+        if(msg.sender != nativeTokenWrapper) {
+            revert("Not native token wrapper");
+        }
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -143,13 +152,13 @@ contract Pack is
     }
 
     /// @dev Pauses / unpauses contract.
-    function pause(bool _toPause) internal onlyRole(DEFAULT_ADMIN_ROLE) {
-        if (_toPause) {
-            _pause();
-        } else {
-            _unpause();
-        }
-    }
+    // function pause(bool _toPause) internal onlyRole(DEFAULT_ADMIN_ROLE) {
+    //     if (_toPause) {
+    //         _pause();
+    //     } else {
+    //         _unpause();
+    //     }
+    // }
 
     /*///////////////////////////////////////////////////////////////
                         ERC 165 / 1155 / 2981 logic
@@ -157,7 +166,8 @@ contract Pack is
 
     /// @dev Returns the URI for a given tokenId.
     function uri(uint256 _tokenId) public view override returns (string memory) {
-        return getUriOfBundle(_tokenId);
+        // return getUriOfBundle(_tokenId);
+        return bundle[_tokenId].uri;
     }
 
     /// @dev See ERC 165
@@ -189,17 +199,23 @@ contract Pack is
     )
         external
         payable
-        onlyRoleWithSwitch(MINTER_ROLE)
+        onlyRoleWithSwitch(minterRole)
         nonReentrant
-        whenNotPaused
+        
         returns (uint256 packId, uint256 packTotalSupply)
     {
-        require(_contents.length > 0, "nothing to pack");
-        require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
+        // require(_contents.length > 0, "nothing to pack");
+        if(_contents.length == 0) {
+            revert("zero contents");
+        }
+        // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
+        if(_contents.length != _numOfRewardUnits.length) {
+            revert("invalid reward units");
+        }
 
-        if (!hasRole(ASSET_ROLE, address(0))) {
+        if (!hasRole(assetRole, address(0))) {
             for (uint256 i = 0; i < _contents.length; i += 1) {
-                _checkRole(ASSET_ROLE, _contents[i].assetContract);
+                _checkRole(assetRole, _contents[i].assetContract);
             }
         }
 
@@ -228,20 +244,32 @@ contract Pack is
     )
         external
         payable
-        onlyRoleWithSwitch(MINTER_ROLE)
+        onlyRoleWithSwitch(minterRole)
         nonReentrant
-        whenNotPaused
+        
         returns (uint256 packTotalSupply, uint256 newSupplyAdded)
     {
-        require(canUpdatePack[_packId], "not allowed");
-        require(_msgSender() == packInfo[_packId].creator, "not creator");
+        // require(canUpdatePack[_packId], "not allowed");
+        if(!canUpdatePack[_packId]) {
+            revert("cant update");
+        }
+        // require(_msgSender() == packInfo[_packId].creator, "not creator");
+        if(_msgSender() != packInfo[_packId].creator) {
+            revert("Not authorized");
+        }
 
-        require(_contents.length > 0, "nothing to pack");
-        require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
+        // require(_contents.length > 0, "nothing to pack");
+        if(_contents.length == 0) {
+            revert("empty contents");
+        }
+        // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
+        if(_contents.length != _numOfRewardUnits.length) {
+            revert("invalid reward units");
+        }
 
-        if (!hasRole(ASSET_ROLE, address(0))) {
+        if (!hasRole(assetRole, address(0))) {
             for (uint256 i = 0; i < _contents.length; i += 1) {
-                _checkRole(ASSET_ROLE, _contents[i].assetContract);
+                _checkRole(assetRole, _contents[i].assetContract);
             }
         }
 
@@ -259,16 +287,29 @@ contract Pack is
     function openPack(uint256 _packId, uint256 _amountToOpen)
         external
         nonReentrant
-        whenNotPaused
+        
         returns (Token[] memory)
     {
         address opener = _msgSender();
 
-        require(isTrustedForwarder(msg.sender) || opener == tx.origin, "opener must be eoa");
-        require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
+        // require(isTrustedForwarder(msg.sender) || opener == tx.origin, "opener must be eoa");
+        if(!isTrustedForwarder(msg.sender) && opener != tx.origin) {
+            revert("not eoa");
+        }
+        // require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
+        if(balanceOf(opener, _packId) < _amountToOpen) {
+            revert("opening more than owned");
+        }
+
         PackInfo memory pack = packInfo[_packId];
-        require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
-        require(pack.expirationTimestamp > block.timestamp, "pack has expired");
+        // require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
+        if(pack.openStartTimestamp > block.timestamp) {
+            revert("cant open yet");
+        }
+        // require(pack.expirationTimestamp > block.timestamp, "pack has expired");
+        if(pack.expirationTimestamp <= block.timestamp) {
+            revert("expired");
+        }
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
@@ -286,8 +327,14 @@ contract Pack is
         nonReentrant
     {
         PackInfo memory pack = packInfo[_packId];
-        require(block.timestamp >= pack.expirationTimestamp, "pack not expired yet");
-        require(_msgSender() == pack.creator, "not creator");
+        // require(block.timestamp >= pack.expirationTimestamp, "pack not expired yet");
+        if(block.timestamp < pack.expirationTimestamp) {
+            revert("pack not expired yet");
+        }
+        // require(_msgSender() == pack.creator, "not creator");
+        if(_msgSender() != pack.creator) {
+            revert("Not authorized");
+        }
 
         _releaseTokens(_msgSender(), _packId);
     }
@@ -304,19 +351,31 @@ contract Pack is
         uint256 sumOfRewardUnits;
 
         for (uint256 i = 0; i < _contents.length; i += 1) {
-            require(_contents[i].totalAmount != 0, "amount can't be zero");
-            require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid reward units");
-            require(
-                _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
-                "invalid erc721 rewards"
-            );
+            // require(_contents[i].totalAmount != 0, "amount can't be zero");
+            if(_contents[i].totalAmount == 0) {
+                revert("zero amount");
+            }
+            // require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid reward units");
+            if(_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
+                revert("invalid reward units");
+            }
+            // require(
+            //     _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
+            //     "invalid erc721 rewards"
+            // );
+            if(_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
+                revert("invalid erc721 rewards");
+            }
 
             sumOfRewardUnits += _numOfRewardUnits[i];
 
             packInfo[packId].perUnitAmounts.push(_contents[i].totalAmount / _numOfRewardUnits[i]);
         }
 
-        require(sumOfRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
+        // require(sumOfRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
+        if(sumOfRewardUnits % amountPerOpen != 0) {
+            revert("invalid amount per open");
+        }
         supplyToMint = sumOfRewardUnits / amountPerOpen;
 
         if(isUpdate) {
@@ -340,7 +399,8 @@ contract Pack is
         uint256 numOfRewardUnitsToDistribute = _numOfPacksToOpen * _rewardUnitsPerOpen;
         rewardUnits = new Token[](numOfRewardUnitsToDistribute);
         uint256 totalRewardUnits = totalSupply[_packId] * _rewardUnitsPerOpen;
-        uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
+        // uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
+        uint256 totalRewardKinds = bundle[_packId].count;
 
         uint256 random = generateRandomValue();
 
@@ -390,14 +450,17 @@ contract Pack is
         returns (Token[] memory contents, uint256[] memory perUnitAmounts)
     {
         PackInfo memory pack = packInfo[_packId];
-        uint256 total = getTokenCountOfBundle(_packId);
+        // uint256 total = getTokenCountOfBundle(_packId);
+        uint256 total = bundle[_packId].count;
         contents = new Token[](total);
         perUnitAmounts = new uint256[](total);
 
         for (uint256 i = 0; i < total; i += 1) {
-            contents[i] = getTokenOfBundle(_packId, i);
-            perUnitAmounts[i] = pack.perUnitAmounts[i];
+            // contents[i] = getTokenOfBundle(_packId, i);
+            contents[i] = bundle[_packId].tokens[i];
+            // perUnitAmounts[i] = pack.perUnitAmounts[i];
         }
+        perUnitAmounts = pack.perUnitAmounts;
     }
 
     /// @dev Returns opening and expiration timestamps of a pack.
@@ -452,8 +515,8 @@ contract Pack is
         super._beforeTokenTransfer(operator, from, to, ids, amounts, data);
 
         // if transfer is restricted on the contract, we still want to allow burning and minting
-        if (!hasRole(TRANSFER_ROLE, address(0)) && from != address(0) && to != address(0)) {
-            require(hasRole(TRANSFER_ROLE, from) || hasRole(TRANSFER_ROLE, to), "restricted to TRANSFER_ROLE holders.");
+        if (!hasRole(transferRole, address(0)) && from != address(0) && to != address(0)) {
+            require(hasRole(transferRole, from) || hasRole(transferRole, to), "!TRANSFER_ROLE");
         }
 
         if (from == address(0)) {
diff --git a/docs/ContractPublisher.md b/docs/ContractPublisher.md
index c290a6582..15b3278e7 100644
--- a/docs/ContractPublisher.md
+++ b/docs/ContractPublisher.md
@@ -123,7 +123,7 @@ Retrieve the published metadata URI from a compiler metadata URI
 function getPublisherProfileUri(address publisher) external view returns (string uri)
 ```
 
-get the publisher profile uri
+Get the publisher profile uri for a given publisher.
 
 
 
@@ -252,9 +252,9 @@ function hasRole(bytes32 role, address account) external view returns (bool)
 function isPaused() external view returns (bool)
 ```
 
+Whether the contract publisher is paused.
 
 
-*Whether the registry is paused.*
 
 
 #### Returns
@@ -313,7 +313,7 @@ function multicall(bytes[] data) external nonpayable returns (bytes[] results)
 function publishContract(address _publisher, string _contractId, string _publishMetadataUri, string _compilerMetadataUri, bytes32 _bytecodeHash, address _implementation) external nonpayable
 ```
 
-Let&#39;s an account publish a contract. The account must be approved by the publisher, or be the publisher.
+Let&#39;s an account publish a contract.
 
 
 
@@ -423,7 +423,7 @@ function supportsInterface(bytes4 interfaceId) external view returns (bool)
 function unpublishContract(address _publisher, string _contractId) external nonpayable
 ```
 
-Lets an account unpublish a contract and all its versions. The account must be approved by the publisher, or be the publisher.
+Lets a publisher unpublish a contract and all its versions.
 
 
 
diff --git a/docs/IContractPublisher.md b/docs/IContractPublisher.md
index 8c174b009..cf69fd6bc 100644
--- a/docs/IContractPublisher.md
+++ b/docs/IContractPublisher.md
@@ -84,7 +84,7 @@ Returns all versions of a published contract.
 function getPublishedUriFromCompilerUri(string compilerMetadataUri) external view returns (string[] publishedMetadataUris)
 ```
 
-Retrieve the published metadata URI from a compiler metadata URI
+Retrieve the published metadata URI from a compiler metadata URI.
 
 
 
@@ -106,7 +106,7 @@ Retrieve the published metadata URI from a compiler metadata URI
 function getPublisherProfileUri(address publisher) external view returns (string uri)
 ```
 
-get the publisher profile uri
+Get the publisher profile uri for a given publisher.
 
 
 
@@ -128,7 +128,7 @@ get the publisher profile uri
 function publishContract(address publisher, string contractId, string publishMetadataUri, string compilerMetadataUri, bytes32 bytecodeHash, address implementation) external nonpayable
 ```
 
-Let&#39;s an account publish a contract. The account must be approved by the publisher, or be the publisher.
+Let&#39;s an account publish a contract.
 
 
 
@@ -166,7 +166,7 @@ Lets an account set its publisher profile uri
 function unpublishContract(address publisher, string contractId) external nonpayable
 ```
 
-Lets an account unpublish a contract and all its versions. The account must be approved by the publisher, or be the publisher.
+Lets a publisher unpublish a contract and all its versions.
 
 
 
diff --git a/docs/IPack.md b/docs/IPack.md
index 32df02d77..58f6c5bf0 100644
--- a/docs/IPack.md
+++ b/docs/IPack.md
@@ -13,7 +13,7 @@ The thirdweb `Pack` contract is a lootbox mechanism. An account can bundle up ar
 ### createPack
 
 ```solidity
-function createPack(ITokenBundle.Token[] contents, uint256[] numOfRewardUnits, string packUri, uint128 openStartTimestamp, uint128 amountDistributedPerOpen, address recipient) external payable returns (uint256 packId, uint256 packTotalSupply)
+function createPack(ITokenBundle.Token[] contents, uint256[] numOfRewardUnits, string packUri, uint128 openStartTimestamp, uint128 amountDistributedPerOpen, uint256 expirationTimestamp, address recipient) external payable returns (uint256 packId, uint256 packTotalSupply)
 ```
 
 Creates a pack with the stated contents.
@@ -29,6 +29,7 @@ Creates a pack with the stated contents.
 | packUri | string | The (metadata) URI assigned to the packs created.
 | openStartTimestamp | uint128 | The timestamp after which packs can be opened.
 | amountDistributedPerOpen | uint128 | The number of reward units distributed per open.
+| expirationTimestamp | uint256 | undefined
 | recipient | address | The recipient of the packs created.
 
 #### Returns
@@ -103,5 +104,24 @@ Emitted when a pack is opened.
 | numOfPacksOpened  | uint256 | undefined |
 | rewardUnitsDistributed  | ITokenBundle.Token[] | undefined |
 
+### PackUpdated
+
+```solidity
+event PackUpdated(uint256 indexed packId, address indexed packCreator, address recipient, uint256 totalPacksCreated)
+```
+
+Emitted when more packs are minted for a packId.
+
+
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| packId `indexed` | uint256 | undefined |
+| packCreator `indexed` | address | undefined |
+| recipient  | address | undefined |
+| totalPacksCreated  | uint256 | undefined |
+
 
 
diff --git a/docs/Pack.md b/docs/Pack.md
index 2c623cc62..e52e05d53 100644
--- a/docs/Pack.md
+++ b/docs/Pack.md
@@ -44,6 +44,32 @@ function NATIVE_TOKEN() external view returns (address)
 |---|---|---|
 | _0 | address | undefined
 
+### addPackContents
+
+```solidity
+function addPackContents(uint256 _packId, ITokenBundle.Token[] _contents, uint256[] _numOfRewardUnits, address _recipient) external payable returns (uint256 packTotalSupply, uint256 newSupplyAdded)
+```
+
+
+
+
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| _packId | uint256 | undefined
+| _contents | ITokenBundle.Token[] | undefined
+| _numOfRewardUnits | uint256[] | undefined
+| _recipient | address | undefined
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| packTotalSupply | uint256 | undefined
+| newSupplyAdded | uint256 | undefined
+
 ### balanceOf
 
 ```solidity
@@ -90,6 +116,28 @@ function balanceOfBatch(address[] accounts, uint256[] ids) external view returns
 |---|---|---|
 | _0 | uint256[] | undefined
 
+### canUpdatePack
+
+```solidity
+function canUpdatePack(uint256) external view returns (bool)
+```
+
+
+
+*Checks if pack-creator allowed to add more tokens to a packId; set to false after first transfer*
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| _0 | uint256 | undefined
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| _0 | bool | undefined
+
 ### contractType
 
 ```solidity
@@ -144,7 +192,7 @@ function contractVersion() external pure returns (uint8)
 ### createPack
 
 ```solidity
-function createPack(ITokenBundle.Token[] _contents, uint256[] _numOfRewardUnits, string _packUri, uint128 _openStartTimestamp, uint128 _amountDistributedPerOpen, address _recipient) external payable returns (uint256 packId, uint256 packTotalSupply)
+function createPack(ITokenBundle.Token[] _contents, uint256[] _numOfRewardUnits, string _packUri, uint128 _openStartTimestamp, uint128 _amountDistributedPerOpen, uint256 _expirationTimestamp, address _recipient) external payable returns (uint256 packId, uint256 packTotalSupply)
 ```
 
 
@@ -160,6 +208,7 @@ function createPack(ITokenBundle.Token[] _contents, uint256[] _numOfRewardUnits,
 | _packUri | string | undefined
 | _openStartTimestamp | uint128 | undefined
 | _amountDistributedPerOpen | uint128 | undefined
+| _expirationTimestamp | uint256 | undefined
 | _recipient | address | undefined
 
 #### Returns
@@ -210,6 +259,29 @@ function getPackContents(uint256 _packId) external view returns (struct ITokenBu
 | contents | ITokenBundle.Token[] | undefined
 | perUnitAmounts | uint256[] | undefined
 
+### getPackTimestamps
+
+```solidity
+function getPackTimestamps(uint256 _packId) external view returns (uint128 openStartTimestamp, uint256 expirationTimestamp)
+```
+
+
+
+*Returns opening and expiration timestamps of a pack.*
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| _packId | uint256 | undefined
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| openStartTimestamp | uint128 | undefined
+| expirationTimestamp | uint256 | undefined
+
 ### getRoleAdmin
 
 ```solidity
@@ -433,7 +505,7 @@ function hasRoleWithSwitch(bytes32 role, address account) external view returns
 ### initialize
 
 ```solidity
-function initialize(address _defaultAdmin, string _name, string _symbol, string _contractURI, address[] _trustedForwarders, address _royaltyRecipient, uint256 _royaltyBps) external nonpayable
+function initialize(address _defaultAdmin, string _name, string _symbol, string _contractURI, address _royaltyRecipient, uint256 _royaltyBps) external nonpayable
 ```
 
 
@@ -448,7 +520,6 @@ function initialize(address _defaultAdmin, string _name, string _symbol, string
 | _name | string | undefined
 | _symbol | string | undefined
 | _contractURI | string | undefined
-| _trustedForwarders | address[] | undefined
 | _royaltyRecipient | address | undefined
 | _royaltyBps | uint256 | undefined
 
@@ -952,6 +1023,22 @@ function uri(uint256 _tokenId) external view returns (string)
 |---|---|---|
 | _0 | string | undefined
 
+### withdrawUnclaimedAssets
+
+```solidity
+function withdrawUnclaimedAssets(uint256 _packId) external nonpayable
+```
+
+
+
+
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| _packId | uint256 | undefined
+
 
 
 ## Events
@@ -1063,6 +1150,25 @@ Emitted when a pack is opened.
 | numOfPacksOpened  | uint256 | undefined |
 | rewardUnitsDistributed  | ITokenBundle.Token[] | undefined |
 
+### PackUpdated
+
+```solidity
+event PackUpdated(uint256 indexed packId, address indexed packCreator, address recipient, uint256 totalPacksCreated)
+```
+
+Emitted when more packs are minted for a packId.
+
+
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| packId `indexed` | uint256 | undefined |
+| packCreator `indexed` | address | undefined |
+| recipient  | address | undefined |
+| totalPacksCreated  | uint256 | undefined |
+
 ### Paused
 
 ```solidity
diff --git a/hardhat.config.ts b/hardhat.config.ts
index 6f73039e4..9fa0f90b9 100644
--- a/hardhat.config.ts
+++ b/hardhat.config.ts
@@ -135,7 +135,7 @@ const config: HardhatUserConfig = {
     enabled: process.env.REPORT_GAS ? true : false,
   },
   dodoc: {
-    runOnCompile: true,
+    runOnCompile: false,
   },
 };
 

From 0405fd04420ba9a352088eca1c1828a1032fd8b4 Mon Sep 17 00:00:00 2001
From: Jake Loo <2171134+jakeloo@users.noreply.github.com>
Date: Mon, 22 Aug 2022 21:44:10 +0000
Subject: [PATCH 11/21] cremoved stuffs

---
 contracts/interfaces/IPack.sol |  17 +--
 contracts/pack/Pack.sol        | 183 ++++++++++-----------------------
 2 files changed, 59 insertions(+), 141 deletions(-)

diff --git a/contracts/interfaces/IPack.sol b/contracts/interfaces/IPack.sol
index ece5534a7..d60a4f554 100644
--- a/contracts/interfaces/IPack.sol
+++ b/contracts/interfaces/IPack.sol
@@ -21,25 +21,13 @@ interface IPack is ITokenBundle {
         uint256[] perUnitAmounts;
         uint128 openStartTimestamp;
         uint128 amountDistributedPerOpen;
-        uint256 expirationTimestamp;
-        address creator;
     }
 
     /// @notice Emitted when a set of packs is created.
-    event PackCreated(
-        uint256 indexed packId,
-        address indexed packCreator,
-        address recipient,
-        uint256 totalPacksCreated
-    );
+    event PackCreated(uint256 indexed packId, address recipient, uint256 totalPacksCreated);
 
     /// @notice Emitted when more packs are minted for a packId.
-    event PackUpdated(
-        uint256 indexed packId,
-        address indexed packCreator,
-        address recipient,
-        uint256 totalPacksCreated
-    );
+    event PackUpdated(uint256 indexed packId, address recipient, uint256 totalPacksCreated);
 
     /// @notice Emitted when a pack is opened.
     event PackOpened(
@@ -68,7 +56,6 @@ interface IPack is ITokenBundle {
         string calldata packUri,
         uint128 openStartTimestamp,
         uint128 amountDistributedPerOpen,
-        uint256 expirationTimestamp,
         address recipient
     ) external payable returns (uint256 packId, uint256 packTotalSupply);
 
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index ccc997ab2..22880c822 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -21,7 +21,7 @@ import "../openzeppelin-presets/metatx/ERC2771ContextUpgradeable.sol";
 import "../extension/ContractMetadata.sol";
 import "../extension/Royalty.sol";
 import "../extension/Ownable.sol";
-import "../extension/PermissionsEnumerable.sol";
+import "../extension/Permissions.sol";
 import { TokenStore, ERC1155Receiver } from "../extension/TokenStore.sol";
 
 contract Pack is
@@ -52,16 +52,6 @@ contract Pack is
     // Token symbol
     string public symbol;
 
-    /// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
-    // bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
-    bytes32 private transferRole;
-    /// @dev Only MINTER_ROLE holders can create packs.
-    // bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
-    bytes32 private minterRole;
-    /// @dev Only assets with ASSET_ROLE can be packed, when packing is restricted to particular assets.
-    // bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");
-    bytes32 private assetRole;
-
     /// @dev The token Id of the next set of packs to be minted.
     uint256 public nextTokenIdToMint;
 
@@ -82,7 +72,10 @@ contract Pack is
                     Constructor + initializer logic
     //////////////////////////////////////////////////////////////*/
 
-    constructor(address _nativeTokenWrapper, address[] memory _trustedForwarders) TokenStore(_nativeTokenWrapper) initializer {
+    constructor(address _nativeTokenWrapper, address[] memory _trustedForwarders)
+        TokenStore(_nativeTokenWrapper)
+        initializer
+    {
         forwarders = _trustedForwarders;
     }
 
@@ -95,9 +88,6 @@ contract Pack is
         address _royaltyRecipient,
         uint256 _royaltyBps
     ) external initializer {
-        transferRole = keccak256("TRANSFER_ROLE");
-        minterRole = keccak256("MINTER_ROLE");
-        assetRole = keccak256("ASSET_ROLE");
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
         __ERC2771Context_init(forwarders);
@@ -111,20 +101,14 @@ contract Pack is
         _setupOwner(_defaultAdmin);
 
         _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
-        _setupRole(transferRole, _defaultAdmin);
-        _setupRole(minterRole, _defaultAdmin);
-        _setupRole(transferRole, address(0));
-
-        // note: see `onlyRoleWithSwitch` for ASSET_ROLE behaviour.
-        _setupRole(assetRole, address(0));
 
         _setupDefaultRoyaltyInfo(_royaltyRecipient, _royaltyBps);
     }
 
     receive() external payable {
         // require(msg.sender == nativeTokenWrapper, "Caller is not native token wrapper.");
-        if(msg.sender != nativeTokenWrapper) {
-            revert("Not native token wrapper");
+        if (msg.sender != nativeTokenWrapper) {
+            revert("!WRAP");
         }
     }
 
@@ -151,15 +135,6 @@ contract Pack is
         return uint8(VERSION);
     }
 
-    /// @dev Pauses / unpauses contract.
-    // function pause(bool _toPause) internal onlyRole(DEFAULT_ADMIN_ROLE) {
-    //     if (_toPause) {
-    //         _pause();
-    //     } else {
-    //         _unpause();
-    //     }
-    // }
-
     /*///////////////////////////////////////////////////////////////
                         ERC 165 / 1155 / 2981 logic
     //////////////////////////////////////////////////////////////*/
@@ -178,9 +153,10 @@ contract Pack is
         override(ERC1155Receiver, ERC1155Upgradeable, IERC165)
         returns (bool)
     {
-        return super.supportsInterface(interfaceId) 
-                || type(IERC2981Upgradeable).interfaceId == interfaceId
-                || type(IERC721Receiver).interfaceId == interfaceId;
+        return
+            super.supportsInterface(interfaceId) ||
+            type(IERC2981Upgradeable).interfaceId == interfaceId ||
+            type(IERC721Receiver).interfaceId == interfaceId;
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -194,46 +170,43 @@ contract Pack is
         string memory _packUri,
         uint128 _openStartTimestamp,
         uint128 _amountDistributedPerOpen,
-        uint256 _expirationTimestamp,
         address _recipient
     )
         external
         payable
-        onlyRoleWithSwitch(minterRole)
+        onlyRoleWithSwitch(DEFAULT_ADMIN_ROLE)
         nonReentrant
-        
         returns (uint256 packId, uint256 packTotalSupply)
     {
         // require(_contents.length > 0, "nothing to pack");
-        if(_contents.length == 0) {
-            revert("zero contents");
+        if (_contents.length == 0) {
+            revert("!C");
         }
         // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
-        if(_contents.length != _numOfRewardUnits.length) {
-            revert("invalid reward units");
-        }
-
-        if (!hasRole(assetRole, address(0))) {
-            for (uint256 i = 0; i < _contents.length; i += 1) {
-                _checkRole(assetRole, _contents[i].assetContract);
-            }
+        if (_contents.length != _numOfRewardUnits.length) {
+            revert("!R");
         }
 
         packId = nextTokenIdToMint;
         nextTokenIdToMint += 1;
 
-        packTotalSupply = escrowPackContents(_contents, _numOfRewardUnits, _packUri, packId, _amountDistributedPerOpen, false);
+        packTotalSupply = escrowPackContents(
+            _contents,
+            _numOfRewardUnits,
+            _packUri,
+            packId,
+            _amountDistributedPerOpen,
+            false
+        );
 
         packInfo[packId].openStartTimestamp = _openStartTimestamp;
         packInfo[packId].amountDistributedPerOpen = _amountDistributedPerOpen;
-        packInfo[packId].expirationTimestamp = _expirationTimestamp == 0 ? type(uint256).max : _expirationTimestamp;
-        packInfo[packId].creator = _msgSender();
 
         canUpdatePack[packId] = true;
 
         _mint(_recipient, packId, packTotalSupply, "");
 
-        emit PackCreated(packId, _msgSender(), _recipient, packTotalSupply);
+        emit PackCreated(packId, _recipient, packTotalSupply);
     }
 
     function addPackContents(
@@ -244,33 +217,22 @@ contract Pack is
     )
         external
         payable
-        onlyRoleWithSwitch(minterRole)
+        onlyRoleWithSwitch(DEFAULT_ADMIN_ROLE)
         nonReentrant
-        
         returns (uint256 packTotalSupply, uint256 newSupplyAdded)
     {
         // require(canUpdatePack[_packId], "not allowed");
-        if(!canUpdatePack[_packId]) {
-            revert("cant update");
-        }
-        // require(_msgSender() == packInfo[_packId].creator, "not creator");
-        if(_msgSender() != packInfo[_packId].creator) {
-            revert("Not authorized");
+        if (!canUpdatePack[_packId]) {
+            revert("!U");
         }
 
         // require(_contents.length > 0, "nothing to pack");
-        if(_contents.length == 0) {
-            revert("empty contents");
+        if (_contents.length == 0) {
+            revert("!C");
         }
         // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
-        if(_contents.length != _numOfRewardUnits.length) {
-            revert("invalid reward units");
-        }
-
-        if (!hasRole(assetRole, address(0))) {
-            for (uint256 i = 0; i < _contents.length; i += 1) {
-                _checkRole(assetRole, _contents[i].assetContract);
-            }
+        if (_contents.length != _numOfRewardUnits.length) {
+            revert("!RU");
         }
 
         uint256 amountPerOpen = packInfo[_packId].amountDistributedPerOpen;
@@ -280,35 +242,26 @@ contract Pack is
 
         _mint(_recipient, _packId, newSupplyAdded, "");
 
-        emit PackUpdated(_packId, _msgSender(), _recipient, newSupplyAdded);
+        emit PackUpdated(_packId, _recipient, newSupplyAdded);
     }
 
     /// @notice Lets a pack owner open packs and receive the packs' reward units.
-    function openPack(uint256 _packId, uint256 _amountToOpen)
-        external
-        nonReentrant
-        
-        returns (Token[] memory)
-    {
+    function openPack(uint256 _packId, uint256 _amountToOpen) external nonReentrant returns (Token[] memory) {
         address opener = _msgSender();
 
         // require(isTrustedForwarder(msg.sender) || opener == tx.origin, "opener must be eoa");
-        if(!isTrustedForwarder(msg.sender) && opener != tx.origin) {
-            revert("not eoa");
+        if (!isTrustedForwarder(msg.sender) && opener != tx.origin) {
+            revert("!EOA");
         }
         // require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
-        if(balanceOf(opener, _packId) < _amountToOpen) {
-            revert("opening more than owned");
+        if (balanceOf(opener, _packId) < _amountToOpen) {
+            revert("!O");
         }
 
         PackInfo memory pack = packInfo[_packId];
         // require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
-        if(pack.openStartTimestamp > block.timestamp) {
-            revert("cant open yet");
-        }
-        // require(pack.expirationTimestamp > block.timestamp, "pack has expired");
-        if(pack.expirationTimestamp <= block.timestamp) {
-            revert("expired");
+        if (pack.openStartTimestamp > block.timestamp) {
+            revert("!C");
         }
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
@@ -322,19 +275,8 @@ contract Pack is
         return rewardUnits;
     }
 
-    function withdrawUnclaimedAssets(uint256 _packId) 
-        external
-        nonReentrant
-    {
+    function withdrawUnclaimedAssets(uint256 _packId) external nonReentrant {
         PackInfo memory pack = packInfo[_packId];
-        // require(block.timestamp >= pack.expirationTimestamp, "pack not expired yet");
-        if(block.timestamp < pack.expirationTimestamp) {
-            revert("pack not expired yet");
-        }
-        // require(_msgSender() == pack.creator, "not creator");
-        if(_msgSender() != pack.creator) {
-            revert("Not authorized");
-        }
 
         _releaseTokens(_msgSender(), _packId);
     }
@@ -352,19 +294,19 @@ contract Pack is
 
         for (uint256 i = 0; i < _contents.length; i += 1) {
             // require(_contents[i].totalAmount != 0, "amount can't be zero");
-            if(_contents[i].totalAmount == 0) {
-                revert("zero amount");
+            if (_contents[i].totalAmount == 0) {
+                revert("!Z");
             }
             // require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid reward units");
-            if(_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
-                revert("invalid reward units");
+            if (_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
+                revert("4");
             }
             // require(
             //     _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
             //     "invalid erc721 rewards"
             // );
-            if(_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
-                revert("invalid erc721 rewards");
+            if (_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
+                revert("3");
             }
 
             sumOfRewardUnits += _numOfRewardUnits[i];
@@ -373,20 +315,19 @@ contract Pack is
         }
 
         // require(sumOfRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
-        if(sumOfRewardUnits % amountPerOpen != 0) {
-            revert("invalid amount per open");
+        if (sumOfRewardUnits % amountPerOpen != 0) {
+            revert("2");
         }
         supplyToMint = sumOfRewardUnits / amountPerOpen;
 
-        if(isUpdate) {
-            for(uint256 i = 0; i < _contents.length; i += 1) {
+        if (isUpdate) {
+            for (uint256 i = 0; i < _contents.length; i += 1) {
                 _addTokenInBundle(_contents[i], packId);
             }
             _transferTokenBatch(_msgSender(), address(this), _contents);
         } else {
             _storeTokens(_msgSender(), _contents, _packUri, packId);
         }
-
     }
 
     /// @dev Returns the reward units to distribute.
@@ -404,7 +345,7 @@ contract Pack is
 
         uint256 random = generateRandomValue();
 
-        (Token[] memory _token,) = getPackContents(_packId);
+        (Token[] memory _token, ) = getPackContents(_packId);
         bool[] memory _isUpdated = new bool[](totalRewardKinds);
         for (uint256 i = 0; i < numOfRewardUnitsToDistribute; i += 1) {
             uint256 randomVal = uint256(keccak256(abi.encode(random, i)));
@@ -417,7 +358,7 @@ contract Pack is
                 if (target < step + totalRewardUnitsOfKind) {
                     _token[j].totalAmount -= pack.perUnitAmounts[j];
                     _isUpdated[j] = true;
-        
+
                     rewardUnits[i].assetContract = _token[j].assetContract;
                     rewardUnits[i].tokenType = _token[j].tokenType;
                     rewardUnits[i].tokenId = _token[j].tokenId;
@@ -432,8 +373,8 @@ contract Pack is
             }
         }
 
-        for(uint256 i = 0; i < totalRewardKinds; i += 1) {
-            if(_isUpdated[i]) {
+        for (uint256 i = 0; i < totalRewardKinds; i += 1) {
+            if (_isUpdated[i]) {
                 _updateTokenInBundle(_token[i], _packId, i);
             }
         }
@@ -464,14 +405,9 @@ contract Pack is
     }
 
     /// @dev Returns opening and expiration timestamps of a pack.
-    function getPackTimestamps(uint256 _packId)
-        external
-        view
-        returns (uint128 openStartTimestamp, uint256 expirationTimestamp)
-    {
+    function getPackTimestamps(uint256 _packId) external view returns (uint128 openStartTimestamp) {
         PackInfo memory pack = packInfo[_packId];
         openStartTimestamp = pack.openStartTimestamp;
-        expirationTimestamp = pack.expirationTimestamp;
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -514,11 +450,6 @@ contract Pack is
     ) internal virtual override {
         super._beforeTokenTransfer(operator, from, to, ids, amounts, data);
 
-        // if transfer is restricted on the contract, we still want to allow burning and minting
-        if (!hasRole(transferRole, address(0)) && from != address(0) && to != address(0)) {
-            require(hasRole(transferRole, from) || hasRole(transferRole, to), "!TRANSFER_ROLE");
-        }
-
         if (from == address(0)) {
             for (uint256 i = 0; i < ids.length; ++i) {
                 totalSupply[ids[i]] += amounts[i];
@@ -526,7 +457,7 @@ contract Pack is
         } else {
             for (uint256 i = 0; i < ids.length; ++i) {
                 // pack can no longer be updated after first transfer to non-zero address
-                if(canUpdatePack[ids[i]]) {
+                if (canUpdatePack[ids[i]]) {
                     canUpdatePack[ids[i]] = false;
                 }
             }

From 6f1fc78fd762ce1b2fb03d4862439690cbe345ed Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 24 Aug 2022 23:17:22 +0530
Subject: [PATCH 12/21] restore roles, token-bundle getters; remove
 withdrawUnclaimedAssets, packTimestamps

---
 contracts/extension/TokenBundle.sol |  18 +--
 contracts/pack/Pack.sol             | 180 ++++++++++++++++------------
 2 files changed, 113 insertions(+), 85 deletions(-)

diff --git a/contracts/extension/TokenBundle.sol b/contracts/extension/TokenBundle.sol
index 9ddd0c75d..fbe69d0f4 100644
--- a/contracts/extension/TokenBundle.sol
+++ b/contracts/extension/TokenBundle.sol
@@ -13,19 +13,19 @@ abstract contract TokenBundle is ITokenBundle {
     mapping(uint256 => BundleInfo) public bundle;
 
     /// @dev Returns the total number of assets in a particular bundle.
-    // function getTokenCountOfBundle(uint256 _bundleId) public view returns (uint256) {
-    //     return bundle[_bundleId].count;
-    // }
+    function getTokenCountOfBundle(uint256 _bundleId) public view returns (uint256) {
+        return bundle[_bundleId].count;
+    }
 
     /// @dev Returns an asset contained in a particular bundle, at a particular index.
-    // function getTokenOfBundle(uint256 _bundleId, uint256 index) public view returns (Token memory) {
-    //     return bundle[_bundleId].tokens[index];
-    // }
+    function getTokenOfBundle(uint256 _bundleId, uint256 index) public view returns (Token memory) {
+        return bundle[_bundleId].tokens[index];
+    }
 
     /// @dev Returns the uri of a particular bundle.
-    // function getUriOfBundle(uint256 _bundleId) public view returns (string memory) {
-    //     return bundle[_bundleId].uri;
-    // }
+    function getUriOfBundle(uint256 _bundleId) public view returns (string memory) {
+        return bundle[_bundleId].uri;
+    }
 
     /// @dev Lets the calling contract create a bundle, by passing in a list of tokens and a unique id.
     function _createBundle(Token[] calldata _tokensToBind, uint256 _bundleId) internal {
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 22880c822..c73a1ed6b 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -52,6 +52,18 @@ contract Pack is
     // Token symbol
     string public symbol;
 
+    /// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
+    // bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
+    bytes32 private transferRole;
+
+    /// @dev Only MINTER_ROLE holders can create packs.
+    // bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
+    bytes32 private minterRole;
+
+    /// @dev Only assets with ASSET_ROLE can be packed, when packing is restricted to particular assets.
+    // bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");
+    bytes32 private assetRole;
+
     /// @dev The token Id of the next set of packs to be minted.
     uint256 public nextTokenIdToMint;
 
@@ -88,6 +100,9 @@ contract Pack is
         address _royaltyRecipient,
         uint256 _royaltyBps
     ) external initializer {
+        transferRole = keccak256("TRANSFER_ROLE");
+        minterRole = keccak256("MINTER_ROLE");
+        assetRole = keccak256("ASSET_ROLE");
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
         __ERC2771Context_init(forwarders);
@@ -102,14 +117,21 @@ contract Pack is
 
         _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
 
+        _setupRole(transferRole, _defaultAdmin);
+        _setupRole(minterRole, _defaultAdmin);
+        _setupRole(transferRole, address(0));
+
+        // note: see `onlyRoleWithSwitch` for ASSET_ROLE behaviour.
+        _setupRole(assetRole, address(0));
+
         _setupDefaultRoyaltyInfo(_royaltyRecipient, _royaltyBps);
     }
 
     receive() external payable {
-        // require(msg.sender == nativeTokenWrapper, "Caller is not native token wrapper.");
-        if (msg.sender != nativeTokenWrapper) {
-            revert("!WRAP");
-        }
+        require(msg.sender == nativeTokenWrapper, "!nativeTokenWrapper.");
+        // if (msg.sender != nativeTokenWrapper) {
+        //     revert("!WRAP");
+        // }
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -141,8 +163,8 @@ contract Pack is
 
     /// @dev Returns the URI for a given tokenId.
     function uri(uint256 _tokenId) public view override returns (string memory) {
-        // return getUriOfBundle(_tokenId);
-        return bundle[_tokenId].uri;
+        return getUriOfBundle(_tokenId);
+        // return bundle[_tokenId].uri;
     }
 
     /// @dev See ERC 165
@@ -174,17 +196,24 @@ contract Pack is
     )
         external
         payable
-        onlyRoleWithSwitch(DEFAULT_ADMIN_ROLE)
+        onlyRoleWithSwitch(minterRole)
         nonReentrant
         returns (uint256 packId, uint256 packTotalSupply)
     {
-        // require(_contents.length > 0, "nothing to pack");
-        if (_contents.length == 0) {
-            revert("!C");
-        }
-        // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
-        if (_contents.length != _numOfRewardUnits.length) {
-            revert("!R");
+        require(_contents.length > 0, "!Contents");
+        // if (_contents.length == 0) {
+        //     revert("!C");
+        // }
+
+        require(_contents.length == _numOfRewardUnits.length, "invalid rewards");
+        // if (_contents.length != _numOfRewardUnits.length) {
+        //     revert("!R");
+        // }
+
+        if (!hasRole(assetRole, address(0))) {
+            for (uint256 i = 0; i < _contents.length; i += 1) {
+                _checkRole(assetRole, _contents[i].assetContract);
+            }
         }
 
         packId = nextTokenIdToMint;
@@ -217,22 +246,28 @@ contract Pack is
     )
         external
         payable
-        onlyRoleWithSwitch(DEFAULT_ADMIN_ROLE)
+        onlyRoleWithSwitch(minterRole)
         nonReentrant
         returns (uint256 packTotalSupply, uint256 newSupplyAdded)
     {
-        // require(canUpdatePack[_packId], "not allowed");
-        if (!canUpdatePack[_packId]) {
-            revert("!U");
-        }
-
-        // require(_contents.length > 0, "nothing to pack");
-        if (_contents.length == 0) {
-            revert("!C");
-        }
-        // require(_contents.length == _numOfRewardUnits.length, "invalid reward units");
-        if (_contents.length != _numOfRewardUnits.length) {
-            revert("!RU");
+        require(canUpdatePack[_packId], "not allowed");
+        // if (!canUpdatePack[_packId]) {
+        //     revert("!U");
+        // }
+
+        require(_contents.length > 0, "!Contents");
+        // if (_contents.length == 0) {
+        //     revert("!C");
+        // }
+        require(_contents.length == _numOfRewardUnits.length, "invalid rewards");
+        // if (_contents.length != _numOfRewardUnits.length) {
+        //     revert("!RU");
+        // }
+
+        if (!hasRole(assetRole, address(0))) {
+            for (uint256 i = 0; i < _contents.length; i += 1) {
+                _checkRole(assetRole, _contents[i].assetContract);
+            }
         }
 
         uint256 amountPerOpen = packInfo[_packId].amountDistributedPerOpen;
@@ -249,20 +284,20 @@ contract Pack is
     function openPack(uint256 _packId, uint256 _amountToOpen) external nonReentrant returns (Token[] memory) {
         address opener = _msgSender();
 
-        // require(isTrustedForwarder(msg.sender) || opener == tx.origin, "opener must be eoa");
-        if (!isTrustedForwarder(msg.sender) && opener != tx.origin) {
-            revert("!EOA");
-        }
-        // require(balanceOf(opener, _packId) >= _amountToOpen, "opening more than owned");
-        if (balanceOf(opener, _packId) < _amountToOpen) {
-            revert("!O");
-        }
+        require(isTrustedForwarder(msg.sender) || opener == tx.origin, "!EOA");
+        // if (!isTrustedForwarder(msg.sender) && opener != tx.origin) {
+        //     revert("!EOA");
+        // }
+        require(balanceOf(opener, _packId) >= _amountToOpen, "!Balance");
+        // if (balanceOf(opener, _packId) < _amountToOpen) {
+        //     revert("!O");
+        // }
 
         PackInfo memory pack = packInfo[_packId];
-        // require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
-        if (pack.openStartTimestamp > block.timestamp) {
-            revert("!C");
-        }
+        require(pack.openStartTimestamp <= block.timestamp, "cant open");
+        // if (pack.openStartTimestamp > block.timestamp) {
+        //     revert("!C");
+        // }
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
@@ -275,12 +310,6 @@ contract Pack is
         return rewardUnits;
     }
 
-    function withdrawUnclaimedAssets(uint256 _packId) external nonReentrant {
-        PackInfo memory pack = packInfo[_packId];
-
-        _releaseTokens(_msgSender(), _packId);
-    }
-
     /// @dev Stores assets within the contract.
     function escrowPackContents(
         Token[] calldata _contents,
@@ -293,31 +322,31 @@ contract Pack is
         uint256 sumOfRewardUnits;
 
         for (uint256 i = 0; i < _contents.length; i += 1) {
-            // require(_contents[i].totalAmount != 0, "amount can't be zero");
-            if (_contents[i].totalAmount == 0) {
-                revert("!Z");
-            }
-            // require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid reward units");
-            if (_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
-                revert("4");
-            }
-            // require(
-            //     _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
-            //     "invalid erc721 rewards"
-            // );
-            if (_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
-                revert("3");
-            }
+            require(_contents[i].totalAmount != 0, "0 amt");
+            // if (_contents[i].totalAmount == 0) {
+            //     revert("!Z");
+            // }
+            require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid rewards");
+            // if (_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
+            //     revert("4");
+            // }
+            require(
+                _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
+                "invalid rewards"
+            );
+            // if (_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
+            //     revert("3");
+            // }
 
             sumOfRewardUnits += _numOfRewardUnits[i];
 
             packInfo[packId].perUnitAmounts.push(_contents[i].totalAmount / _numOfRewardUnits[i]);
         }
 
-        // require(sumOfRewardUnits % amountPerOpen == 0, "invalid amount to distribute per open");
-        if (sumOfRewardUnits % amountPerOpen != 0) {
-            revert("2");
-        }
+        require(sumOfRewardUnits % amountPerOpen == 0, "invalid amounts");
+        // if (sumOfRewardUnits % amountPerOpen != 0) {
+        //     revert("2");
+        // }
         supplyToMint = sumOfRewardUnits / amountPerOpen;
 
         if (isUpdate) {
@@ -340,8 +369,8 @@ contract Pack is
         uint256 numOfRewardUnitsToDistribute = _numOfPacksToOpen * _rewardUnitsPerOpen;
         rewardUnits = new Token[](numOfRewardUnitsToDistribute);
         uint256 totalRewardUnits = totalSupply[_packId] * _rewardUnitsPerOpen;
-        // uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
-        uint256 totalRewardKinds = bundle[_packId].count;
+        uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
+        // uint256 totalRewardKinds = bundle[_packId].count;
 
         uint256 random = generateRandomValue();
 
@@ -391,25 +420,19 @@ contract Pack is
         returns (Token[] memory contents, uint256[] memory perUnitAmounts)
     {
         PackInfo memory pack = packInfo[_packId];
-        // uint256 total = getTokenCountOfBundle(_packId);
-        uint256 total = bundle[_packId].count;
+        uint256 total = getTokenCountOfBundle(_packId);
+        // uint256 total = bundle[_packId].count;
         contents = new Token[](total);
         perUnitAmounts = new uint256[](total);
 
         for (uint256 i = 0; i < total; i += 1) {
-            // contents[i] = getTokenOfBundle(_packId, i);
-            contents[i] = bundle[_packId].tokens[i];
+            contents[i] = getTokenOfBundle(_packId, i);
+            // contents[i] = bundle[_packId].tokens[i];
             // perUnitAmounts[i] = pack.perUnitAmounts[i];
         }
         perUnitAmounts = pack.perUnitAmounts;
     }
 
-    /// @dev Returns opening and expiration timestamps of a pack.
-    function getPackTimestamps(uint256 _packId) external view returns (uint128 openStartTimestamp) {
-        PackInfo memory pack = packInfo[_packId];
-        openStartTimestamp = pack.openStartTimestamp;
-    }
-
     /*///////////////////////////////////////////////////////////////
                         Internal functions
     //////////////////////////////////////////////////////////////*/
@@ -450,6 +473,11 @@ contract Pack is
     ) internal virtual override {
         super._beforeTokenTransfer(operator, from, to, ids, amounts, data);
 
+        // if transfer is restricted on the contract, we still want to allow burning and minting
+        if (!hasRole(transferRole, address(0)) && from != address(0) && to != address(0)) {
+            require(hasRole(transferRole, from) || hasRole(transferRole, to), "!TRANSFER_ROLE");
+        }
+
         if (from == address(0)) {
             for (uint256 i = 0; i < ids.length; ++i) {
                 totalSupply[ids[i]] += amounts[i];

From 610e3bdb0200bd274148066fce1c383d1bd38d8f Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 24 Aug 2022 23:30:56 +0530
Subject: [PATCH 13/21] immutable forwarder; edit revert strings;

---
 contracts/extension/TokenBundle.sol | 20 ++++++++++----------
 contracts/pack/Pack.sol             | 21 ++++++++++++---------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/contracts/extension/TokenBundle.sol b/contracts/extension/TokenBundle.sol
index fbe69d0f4..57d853fb5 100644
--- a/contracts/extension/TokenBundle.sol
+++ b/contracts/extension/TokenBundle.sol
@@ -31,8 +31,8 @@ abstract contract TokenBundle is ITokenBundle {
     function _createBundle(Token[] calldata _tokensToBind, uint256 _bundleId) internal {
         uint256 targetCount = _tokensToBind.length;
 
-        require(targetCount > 0, "TokenBundle: no tokens to bind.");
-        require(bundle[_bundleId].count == 0, "TokenBundle: existent at bundleId");
+        require(targetCount > 0, "no tokens to bind");
+        require(bundle[_bundleId].count == 0, "existent at bundleId");
 
         for (uint256 i = 0; i < targetCount; i += 1) {
             _checkTokenType(_tokensToBind[i]);
@@ -44,7 +44,7 @@ abstract contract TokenBundle is ITokenBundle {
 
     /// @dev Lets the calling contract update a bundle, by passing in a list of tokens and a unique id.
     function _updateBundle(Token[] memory _tokensToBind, uint256 _bundleId) internal {
-        require(_tokensToBind.length > 0, "TokenBundle: no tokens to bind.");
+        require(_tokensToBind.length > 0, "no tokens to bind");
 
         uint256 currentCount = bundle[_bundleId].count;
         uint256 targetCount = _tokensToBind.length;
@@ -77,7 +77,7 @@ abstract contract TokenBundle is ITokenBundle {
         uint256 _bundleId,
         uint256 _index
     ) internal {
-        require(_index < bundle[_bundleId].count, "TokenBundle: index DNE.");
+        require(_index < bundle[_bundleId].count, "index DNE");
         _checkTokenType(_tokenToBind);
         bundle[_bundleId].tokens[_index] = _tokenToBind;
     }
@@ -86,24 +86,24 @@ abstract contract TokenBundle is ITokenBundle {
     function _checkTokenType(Token memory _token) internal view {
         if (_token.tokenType == TokenType.ERC721) {
             try IERC165(_token.assetContract).supportsInterface(0x80ac58cd) returns (bool supported721) {
-                require(supported721, "Asset doesn't match TokenType");
+                require(supported721, "!TokenType");
             } catch {
-                revert("Asset doesn't match TokenType");
+                revert("!TokenType");
             }
         } else if (_token.tokenType == TokenType.ERC1155) {
             try IERC165(_token.assetContract).supportsInterface(0xd9b67a26) returns (bool supported1155) {
-                require(supported1155, "Asset doesn't match TokenType");
+                require(supported1155, "!TokenType");
             } catch {
-                revert("Asset doesn't match TokenType");
+                revert("!TokenType");
             }
         } else if (_token.tokenType == TokenType.ERC20) {
             if (_token.assetContract != CurrencyTransferLib.NATIVE_TOKEN) {
                 // 0x36372b07
                 try IERC165(_token.assetContract).supportsInterface(0x80ac58cd) returns (bool supported721) {
-                    require(!supported721, "Asset doesn't match TokenType");
+                    require(!supported721, "!TokenType");
 
                     try IERC165(_token.assetContract).supportsInterface(0xd9b67a26) returns (bool supported1155) {
-                        require(!supported1155, "Asset doesn't match TokenType");
+                        require(!supported1155, "!TokenType");
                     } catch Error(string memory) {} catch {}
                 } catch Error(string memory) {} catch {}
             }
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index c73a1ed6b..def22a50f 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -44,7 +44,7 @@ contract Pack is
     bytes32 private constant MODULE_TYPE = bytes32("Pack");
     uint256 private constant VERSION = 1;
 
-    address[] private forwarders;
+    address private immutable forwarder;
 
     // Token name
     string public name;
@@ -84,11 +84,11 @@ contract Pack is
                     Constructor + initializer logic
     //////////////////////////////////////////////////////////////*/
 
-    constructor(address _nativeTokenWrapper, address[] memory _trustedForwarders)
+    constructor(address _nativeTokenWrapper, address _trustedForwarder)
         TokenStore(_nativeTokenWrapper)
         initializer
     {
-        forwarders = _trustedForwarders;
+        forwarder = _trustedForwarder;
     }
 
     /// @dev Initiliazes the contract, like a constructor.
@@ -105,6 +105,9 @@ contract Pack is
         assetRole = keccak256("ASSET_ROLE");
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
+
+        address[] memory forwarders = new address[](1);
+        forwarders[0] = forwarder;
         __ERC2771Context_init(forwarders);
         // __ERC1155Pausable_init();
         __ERC1155_init(_contractURI);
@@ -205,7 +208,7 @@ contract Pack is
         //     revert("!C");
         // }
 
-        require(_contents.length == _numOfRewardUnits.length, "invalid rewards");
+        require(_contents.length == _numOfRewardUnits.length, "!Rewards");
         // if (_contents.length != _numOfRewardUnits.length) {
         //     revert("!R");
         // }
@@ -250,7 +253,7 @@ contract Pack is
         nonReentrant
         returns (uint256 packTotalSupply, uint256 newSupplyAdded)
     {
-        require(canUpdatePack[_packId], "not allowed");
+        require(canUpdatePack[_packId], "!Allowed");
         // if (!canUpdatePack[_packId]) {
         //     revert("!U");
         // }
@@ -259,7 +262,7 @@ contract Pack is
         // if (_contents.length == 0) {
         //     revert("!C");
         // }
-        require(_contents.length == _numOfRewardUnits.length, "invalid rewards");
+        require(_contents.length == _numOfRewardUnits.length, "!Rewards");
         // if (_contents.length != _numOfRewardUnits.length) {
         //     revert("!RU");
         // }
@@ -326,13 +329,13 @@ contract Pack is
             // if (_contents[i].totalAmount == 0) {
             //     revert("!Z");
             // }
-            require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "invalid rewards");
+            require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "!Rewards");
             // if (_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
             //     revert("4");
             // }
             require(
                 _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
-                "invalid rewards"
+                "!Rewards"
             );
             // if (_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
             //     revert("3");
@@ -343,7 +346,7 @@ contract Pack is
             packInfo[packId].perUnitAmounts.push(_contents[i].totalAmount / _numOfRewardUnits[i]);
         }
 
-        require(sumOfRewardUnits % amountPerOpen == 0, "invalid amounts");
+        require(sumOfRewardUnits % amountPerOpen == 0, "!Amounts");
         // if (sumOfRewardUnits % amountPerOpen != 0) {
         //     revert("2");
         // }

From 58f6629ecb2eebe2dfd19fd775dc9d45427aed96 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 24 Aug 2022 23:52:15 +0530
Subject: [PATCH 14/21] cleanup

---
 contracts/extension/TokenStore.sol |  6 ++--
 contracts/multiwrap/Multiwrap.sol  |  9 ++----
 contracts/pack/Pack.sol            | 50 ------------------------------
 3 files changed, 5 insertions(+), 60 deletions(-)

diff --git a/contracts/extension/TokenStore.sol b/contracts/extension/TokenStore.sol
index 69ec0f35c..ab43fcb19 100644
--- a/contracts/extension/TokenStore.sol
+++ b/contracts/extension/TokenStore.sol
@@ -36,13 +36,11 @@ contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {
 
     /// @dev Release stored / escrowed ERC1155, ERC721, ERC20 tokens.
     function _releaseTokens(address _recipient, uint256 _idForContent) internal {
-        // uint256 count = getTokenCountOfBundle(_idForContent);
-        uint256 count = bundle[_idForContent].count;
+        uint256 count = getTokenCountOfBundle(_idForContent);
         Token[] memory tokensToRelease = new Token[](count);
 
         for (uint256 i = 0; i < count; i += 1) {
-            // tokensToRelease[i] = getTokenOfBundle(_idForContent, i);
-            tokensToRelease[i] = bundle[_idForContent].tokens[i];
+            tokensToRelease[i] = getTokenOfBundle(_idForContent, i);
         }
 
         _deleteBundle(_idForContent);
diff --git a/contracts/multiwrap/Multiwrap.sol b/contracts/multiwrap/Multiwrap.sol
index 5fe2c8c8f..1b8b7b33c 100644
--- a/contracts/multiwrap/Multiwrap.sol
+++ b/contracts/multiwrap/Multiwrap.sol
@@ -127,8 +127,7 @@ contract Multiwrap is
 
     /// @dev Returns the URI for a given tokenId.
     function tokenURI(uint256 _tokenId) public view override returns (string memory) {
-        // return getUriOfBundle(_tokenId);
-        return bundle[_tokenId].uri;
+        return getUriOfBundle(_tokenId);
     }
 
     /// @dev See ERC 165
@@ -189,13 +188,11 @@ contract Multiwrap is
 
     /// @dev Returns the underlying contents of a wrapped NFT.
     function getWrappedContents(uint256 _tokenId) external view returns (Token[] memory contents) {
-        // uint256 total = getTokenCountOfBundle(_tokenId);
-        uint256 total = bundle[_tokenId].count;
+        uint256 total = getTokenCountOfBundle(_tokenId);
         contents = new Token[](total);
 
         for (uint256 i = 0; i < total; i += 1) {
-            // contents[i] = getTokenOfBundle(_tokenId, i);
-            contents[i] = bundle[_tokenId].tokens[i];
+            contents[i] = getTokenOfBundle(_tokenId, i);
         }
     }
 
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index def22a50f..20bccc746 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -53,15 +53,12 @@ contract Pack is
     string public symbol;
 
     /// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
-    // bytes32 private constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE");
     bytes32 private transferRole;
 
     /// @dev Only MINTER_ROLE holders can create packs.
-    // bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes32 private minterRole;
 
     /// @dev Only assets with ASSET_ROLE can be packed, when packing is restricted to particular assets.
-    // bytes32 private constant ASSET_ROLE = keccak256("ASSET_ROLE");
     bytes32 private assetRole;
 
     /// @dev The token Id of the next set of packs to be minted.
@@ -109,7 +106,6 @@ contract Pack is
         address[] memory forwarders = new address[](1);
         forwarders[0] = forwarder;
         __ERC2771Context_init(forwarders);
-        // __ERC1155Pausable_init();
         __ERC1155_init(_contractURI);
 
         name = _name;
@@ -132,9 +128,6 @@ contract Pack is
 
     receive() external payable {
         require(msg.sender == nativeTokenWrapper, "!nativeTokenWrapper.");
-        // if (msg.sender != nativeTokenWrapper) {
-        //     revert("!WRAP");
-        // }
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -167,7 +160,6 @@ contract Pack is
     /// @dev Returns the URI for a given tokenId.
     function uri(uint256 _tokenId) public view override returns (string memory) {
         return getUriOfBundle(_tokenId);
-        // return bundle[_tokenId].uri;
     }
 
     /// @dev See ERC 165
@@ -204,14 +196,7 @@ contract Pack is
         returns (uint256 packId, uint256 packTotalSupply)
     {
         require(_contents.length > 0, "!Contents");
-        // if (_contents.length == 0) {
-        //     revert("!C");
-        // }
-
         require(_contents.length == _numOfRewardUnits.length, "!Rewards");
-        // if (_contents.length != _numOfRewardUnits.length) {
-        //     revert("!R");
-        // }
 
         if (!hasRole(assetRole, address(0))) {
             for (uint256 i = 0; i < _contents.length; i += 1) {
@@ -254,18 +239,8 @@ contract Pack is
         returns (uint256 packTotalSupply, uint256 newSupplyAdded)
     {
         require(canUpdatePack[_packId], "!Allowed");
-        // if (!canUpdatePack[_packId]) {
-        //     revert("!U");
-        // }
-
         require(_contents.length > 0, "!Contents");
-        // if (_contents.length == 0) {
-        //     revert("!C");
-        // }
         require(_contents.length == _numOfRewardUnits.length, "!Rewards");
-        // if (_contents.length != _numOfRewardUnits.length) {
-        //     revert("!RU");
-        // }
 
         if (!hasRole(assetRole, address(0))) {
             for (uint256 i = 0; i < _contents.length; i += 1) {
@@ -288,19 +263,10 @@ contract Pack is
         address opener = _msgSender();
 
         require(isTrustedForwarder(msg.sender) || opener == tx.origin, "!EOA");
-        // if (!isTrustedForwarder(msg.sender) && opener != tx.origin) {
-        //     revert("!EOA");
-        // }
         require(balanceOf(opener, _packId) >= _amountToOpen, "!Balance");
-        // if (balanceOf(opener, _packId) < _amountToOpen) {
-        //     revert("!O");
-        // }
 
         PackInfo memory pack = packInfo[_packId];
         require(pack.openStartTimestamp <= block.timestamp, "cant open");
-        // if (pack.openStartTimestamp > block.timestamp) {
-        //     revert("!C");
-        // }
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
@@ -326,20 +292,11 @@ contract Pack is
 
         for (uint256 i = 0; i < _contents.length; i += 1) {
             require(_contents[i].totalAmount != 0, "0 amt");
-            // if (_contents[i].totalAmount == 0) {
-            //     revert("!Z");
-            // }
             require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "!Rewards");
-            // if (_contents[i].totalAmount % _numOfRewardUnits[i] != 0) {
-            //     revert("4");
-            // }
             require(
                 _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
                 "!Rewards"
             );
-            // if (_contents[i].tokenType == TokenType.ERC721 && _contents[i].totalAmount != 1) {
-            //     revert("3");
-            // }
 
             sumOfRewardUnits += _numOfRewardUnits[i];
 
@@ -347,9 +304,6 @@ contract Pack is
         }
 
         require(sumOfRewardUnits % amountPerOpen == 0, "!Amounts");
-        // if (sumOfRewardUnits % amountPerOpen != 0) {
-        //     revert("2");
-        // }
         supplyToMint = sumOfRewardUnits / amountPerOpen;
 
         if (isUpdate) {
@@ -373,7 +327,6 @@ contract Pack is
         rewardUnits = new Token[](numOfRewardUnitsToDistribute);
         uint256 totalRewardUnits = totalSupply[_packId] * _rewardUnitsPerOpen;
         uint256 totalRewardKinds = getTokenCountOfBundle(_packId);
-        // uint256 totalRewardKinds = bundle[_packId].count;
 
         uint256 random = generateRandomValue();
 
@@ -424,14 +377,11 @@ contract Pack is
     {
         PackInfo memory pack = packInfo[_packId];
         uint256 total = getTokenCountOfBundle(_packId);
-        // uint256 total = bundle[_packId].count;
         contents = new Token[](total);
         perUnitAmounts = new uint256[](total);
 
         for (uint256 i = 0; i < total; i += 1) {
             contents[i] = getTokenOfBundle(_packId, i);
-            // contents[i] = bundle[_packId].tokens[i];
-            // perUnitAmounts[i] = pack.perUnitAmounts[i];
         }
         perUnitAmounts = pack.perUnitAmounts;
     }

From 30e1f86fb63643f54f27ea7c6c9f400e9b5de5db Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 29 Aug 2022 20:32:44 +0530
Subject: [PATCH 15/21] update tests

---
 lib/forge-std                |   2 +-
 src/test/Multiwrap.t.sol     |   2 +-
 src/test/Pack.t.sol          | 184 ++++++++++-------------------------
 src/test/PackBenchmark.t.sol |   5 +-
 src/test/utils/BaseTest.sol  |   2 +-
 5 files changed, 58 insertions(+), 137 deletions(-)

diff --git a/lib/forge-std b/lib/forge-std
index 2c7cbfc6f..51a29d7f1 160000
--- a/lib/forge-std
+++ b/lib/forge-std
@@ -1 +1 @@
-Subproject commit 2c7cbfc6fbede6d7c9e6b17afe997e3fdfe22fef
+Subproject commit 51a29d7f15c574189518afd42680d16f367b7d59
diff --git a/src/test/Multiwrap.t.sol b/src/test/Multiwrap.t.sol
index 7377cd040..694703d53 100644
--- a/src/test/Multiwrap.t.sol
+++ b/src/test/Multiwrap.t.sol
@@ -508,7 +508,7 @@ contract MultiwrapTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        vm.expectRevert("TokenBundle: no tokens to bind.");
+        vm.expectRevert("no tokens to bind");
         multiwrap.wrap(emptyContent, uriForWrappedToken, recipient);
     }
 
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index c0fb508d6..d524fb651 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -14,7 +14,6 @@ contract PackTest is BaseTest {
     /// @notice Emitted when a set of packs is created.
     event PackCreated(
         uint256 indexed packId,
-        address indexed packCreator,
         address recipient,
         uint256 totalPacksCreated
     );
@@ -178,87 +177,6 @@ contract PackTest is BaseTest {
         pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
     }
 
-    /*///////////////////////////////////////////////////////////////
-                            Miscellaneous
-    //////////////////////////////////////////////////////////////*/
-
-    function test_state_createPack_timestamps() public {
-        uint256 packId = pack.nextTokenIdToMint();
-        address recipient = address(1);
-
-        vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
-
-        (, uint256 expirationTimestamp) = pack.getPackTimestamps(packId);
-
-        assertEq(expirationTimestamp, type(uint256).max);
-    }
-
-    function test_revert_openPack_expirationTimestamp() public {
-        uint256 packId = pack.nextTokenIdToMint();
-        address recipient = address(0x123);
-
-        vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
-
-        vm.warp(101);
-        vm.prank(recipient, recipient);
-        vm.expectRevert("pack has expired");
-        pack.openPack(packId, 1);
-
-        vm.warp(99);
-        vm.prank(recipient, recipient);
-        pack.openPack(packId, 1);
-    }
-
-    function test_state_withdrawUnclaimedAssets() public {
-        uint256 packId = pack.nextTokenIdToMint();
-        address recipient = address(0x123);
-
-        vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
-
-        vm.warp(101);
-        vm.prank(address(tokenOwner));
-        pack.withdrawUnclaimedAssets(packId);
-
-        // ERC20 balance
-        assertEq(erc20.balanceOf(address(tokenOwner)), 2000 ether);
-        assertEq(erc20.balanceOf(address(pack)), 0);
-
-        // ERC721 balance
-        assertEq(erc721.ownerOf(0), address(tokenOwner));
-        assertEq(erc721.ownerOf(1), address(tokenOwner));
-        assertEq(erc721.ownerOf(2), address(tokenOwner));
-        assertEq(erc721.ownerOf(3), address(tokenOwner));
-        assertEq(erc721.ownerOf(4), address(tokenOwner));
-        assertEq(erc721.ownerOf(5), address(tokenOwner));
-
-        // ERC1155 balance
-        assertEq(erc1155.balanceOf(address(tokenOwner), 0), 100);
-        assertEq(erc1155.balanceOf(address(pack), 0), 0);
-        assertEq(erc1155.balanceOf(address(tokenOwner), 1), 500);
-        assertEq(erc1155.balanceOf(address(pack), 1), 0);
-    }
-
-    function test_revert_withdrawUnclaimedAssets() public {
-        uint256 packId = pack.nextTokenIdToMint();
-        address recipient = address(0x123);
-
-        vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 100, recipient);
-
-        vm.warp(99);
-        vm.prank(address(tokenOwner));
-        vm.expectRevert("pack not expired yet");
-        pack.withdrawUnclaimedAssets(packId);
-
-        vm.warp(101);
-        vm.prank(address(345));
-        vm.expectRevert("not creator");
-        pack.withdrawUnclaimedAssets(packId);
-    }
-
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `createPack`
     //////////////////////////////////////////////////////////////*/
@@ -277,7 +195,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -312,7 +230,7 @@ contract PackTest is BaseTest {
         numOfRewardUnits.push(20);
 
         vm.prank(address(tokenOwner));
-        pack.createPack{ value: 20 ether }(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack{ value: 20 ether }(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -346,7 +264,7 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         assertEq(packId + 1, pack.nextTokenIdToMint());
 
@@ -371,9 +289,9 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectEmit(true, true, true, true);
-        emit PackCreated(packId, address(tokenOwner), recipient, 226);
+        emit PackCreated(packId, recipient, 226);
 
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.stopPrank();
     }
@@ -405,7 +323,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         // ERC20 balance
         assertEq(erc20.balanceOf(address(tokenOwner)), 0);
@@ -451,7 +369,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert(bytes(errorMsg));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -474,7 +392,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert(bytes(errorMsg));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -497,7 +415,7 @@ contract PackTest is BaseTest {
 
         vm.prank(address(tokenOwner));
         vm.expectRevert("msg.value != amount");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -510,7 +428,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC20: transfer amount exceeds balance");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -523,7 +441,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC721: transfer caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -536,7 +454,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC1155: insufficient balance for transfer");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -549,7 +467,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC20: insufficient allowance");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -562,7 +480,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC721: transfer caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -575,7 +493,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ERC1155: caller is not owner nor approved");
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -596,8 +514,8 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.startPrank(address(tokenOwner));
-        vm.expectRevert("Asset doesn't match TokenType");
-        pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, 0, recipient);
+        vm.expectRevert("!TokenType");
+        pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -618,8 +536,8 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.startPrank(address(tokenOwner));
-        vm.expectRevert("amount can't be zero");
-        (, uint256 totalSupply) = pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, 0, recipient);
+        vm.expectRevert("0 amt");
+        (, uint256 totalSupply) = pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
 
         // assertEq(totalSupply, 10);
     }
@@ -634,8 +552,8 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.startPrank(address(tokenOwner));
-        vm.expectRevert("nothing to pack");
-        pack.createPack(emptyContent, rewardUnits, packUri, 0, 1, 0, recipient);
+        vm.expectRevert("!Contents");
+        pack.createPack(emptyContent, rewardUnits, packUri, 0, 1, recipient);
     }
 
     /**
@@ -647,8 +565,8 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.startPrank(address(tokenOwner));
-        vm.expectRevert("invalid reward units");
-        pack.createPack(packContents, rewardUnits, packUri, 0, 1, 0, recipient);
+        vm.expectRevert("!Rewards");
+        pack.createPack(packContents, rewardUnits, packUri, 0, 1, recipient);
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -663,7 +581,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         (ITokenBundle.Token[] memory packed, ) = pack.getPackContents(packId);
         assertEq(packed.length, packContents.length);
@@ -699,7 +617,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         // ERC20 balance
         assertEq(erc20.balanceOf(address(tokenOwner)), 0);
@@ -745,20 +663,26 @@ contract PackTest is BaseTest {
     /**
      *  note: Testing revert condition; non-creator calls `addPackContents`.
      */
-    function test_revert_addPackContents_NotCreator() public {
+    function test_revert_addPackContents_NotMinterRole() public {
         uint256 packId = pack.nextTokenIdToMint();
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         address randomAccount = address(0x123);
 
-        vm.prank(deployer);
-        pack.grantRole(keccak256("MINTER_ROLE"), randomAccount);
+        string memory errorMsg = string(
+            abi.encodePacked(
+                "Permissions: account ",
+                Strings.toHexString(uint160(address(randomAccount)), 20),
+                " is missing role ",
+                Strings.toHexString(uint256(keccak256("MINTER_ROLE")), 32)
+            )
+        );
 
         vm.prank(randomAccount);
-        vm.expectRevert("not creator");
+        vm.expectRevert(bytes(errorMsg));
         pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
     }
 
@@ -767,7 +691,7 @@ contract PackTest is BaseTest {
      */
     function test_revert_addPackContents_PackNonExistent() public {
         vm.prank(address(tokenOwner));
-        vm.expectRevert("not allowed");
+        vm.expectRevert("!Allowed");
         pack.addPackContents(0, packContents, numOfRewardUnits, address(1));
     }
 
@@ -779,13 +703,13 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.prank(recipient);
         pack.safeTransferFrom(recipient, address(567), packId, 1, "");
 
         vm.prank(address(tokenOwner));
-        vm.expectRevert("not allowed");
+        vm.expectRevert("!Allowed");
         pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
     }
     
@@ -803,7 +727,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, 0, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, recipient);
 
         vm.prank(recipient, recipient);
         ITokenBundle.Token[] memory rewardUnits = pack.openPack(packId, packsToOpen);
@@ -839,7 +763,7 @@ contract PackTest is BaseTest {
         ITokenBundle.Token[] memory emptyRewardUnitsForTestingEvent;
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.expectEmit(true, true, false, false);
         emit PackOpened(packId, recipient, 1, emptyRewardUnitsForTestingEvent);
@@ -854,7 +778,7 @@ contract PackTest is BaseTest {
         address recipient = address(1);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 2, recipient);
 
         // ERC20 balance
         assertEq(erc20.balanceOf(address(recipient)), 0);
@@ -920,10 +844,11 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.startPrank(recipient, address(27));
-        vm.expectRevert("opener must be eoa");
+        string memory err = "!EOA";
+        vm.expectRevert(bytes(err));
         pack.openPack(packId, 1);
     }
 
@@ -935,10 +860,10 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        (, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.startPrank(recipient, recipient);
-        vm.expectRevert("opening more than owned");
+        vm.expectRevert("!Balance");
         pack.openPack(packId, totalSupply + 1);
     }
 
@@ -949,10 +874,10 @@ contract PackTest is BaseTest {
         uint256 packId = pack.nextTokenIdToMint();
         address recipient = address(0x123);
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 1000, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 1000, 1, recipient);
 
         vm.startPrank(recipient, recipient);
-        vm.expectRevert("cannot open yet");
+        vm.expectRevert("cant open");
         pack.openPack(packId, 1);
     }
 
@@ -963,10 +888,10 @@ contract PackTest is BaseTest {
         address recipient = address(0x123);
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, recipient);
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
 
         vm.startPrank(recipient, recipient);
-        vm.expectRevert("opening more than owned");
+        vm.expectRevert("!Balance");
         pack.openPack(2, 1);
     }
 
@@ -1099,7 +1024,6 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
-            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
@@ -1151,7 +1075,6 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
-            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
@@ -1216,7 +1139,6 @@ contract PackTest is BaseTest {
             packUri,
             0,
             y,
-            0,
             recipient
         );
         console2.log("total supply: ", totalSupply);
@@ -1274,7 +1196,7 @@ contract PackTest is BaseTest {
 
         vm.startPrank(address(tokenOwner));
         vm.expectRevert("ReentrancyGuard: reentrant call");
-        pack.createPack(content, rewards, packUri, 0, 1, 0, recipient);
+        pack.createPack(content, rewards, packUri, 0, 1, recipient);
     }
 }
 
@@ -1294,7 +1216,7 @@ contract MaliciousERC20 is MockERC20, ITokenBundle {
         uint256[] memory rewards = new uint256[](1);
 
         address recipient = address(0x123);
-        pack.createPack(content, rewards, "", 0, 1, 0, recipient);
+        pack.createPack(content, rewards, "", 0, 1, recipient);
         return super.transferFrom(from, to, amount);
     }
 }
diff --git a/src/test/PackBenchmark.t.sol b/src/test/PackBenchmark.t.sol
index b60970709..3546bd63c 100644
--- a/src/test/PackBenchmark.t.sol
+++ b/src/test/PackBenchmark.t.sol
@@ -128,7 +128,7 @@ contract CreatePackBenchmarkTest is BaseTest {
      *  note: Testing state changes; token owner calls `createPack` to pack owned tokens.
      */
     function test_benchmark_createPack() public {
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, address(0x123));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, address(0x123));
     }
 }
 
@@ -240,7 +240,7 @@ contract OpenPackBenchmarkTest is BaseTest {
         pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
 
         vm.prank(address(tokenOwner));
-        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, 0, address(0x123));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, address(0x123));
 
         vm.startPrank(address(0x123), address(0x123));
     }
@@ -339,7 +339,6 @@ contract OpenPackLargeInputsTest is BaseTest {
             packUri,
             0,
             y,
-            0,
             recipient
         );
         
diff --git a/src/test/utils/BaseTest.sol b/src/test/utils/BaseTest.sol
index e1fe96ee2..066039544 100644
--- a/src/test/utils/BaseTest.sol
+++ b/src/test/utils/BaseTest.sol
@@ -91,7 +91,7 @@ abstract contract BaseTest is DSTest, Test {
         TWFactory(factory).addImplementation(address(new Split(fee)));
         TWFactory(factory).addImplementation(address(new Multiwrap(address(weth))));
         TWFactory(factory).addImplementation(address(new MockContract(bytes32("Pack"), 1)));
-        TWFactory(factory).addImplementation(address(new Pack(address(weth), forwarders())));
+        TWFactory(factory).addImplementation(address(new Pack(address(weth), forwarder)));
         TWFactory(factory).addImplementation(address(new VoteERC20()));
         vm.stopPrank();
 

From b4d53f2493ef7287145467919dafa1c0b6d90d10 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 29 Aug 2022 20:43:40 +0530
Subject: [PATCH 16/21] eoa-only forwarder

---
 contracts/Forwarder.sol                              |  2 +-
 contracts/ForwarderEOAOnly.sol                       | 12 ++++++++++++
 ...imalForwarder.sol => MinimalForwarderEOAOnly.sol} |  4 ++--
 3 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 contracts/ForwarderEOAOnly.sol
 rename contracts/openzeppelin-presets/metatx/{MinimalForwarder.sol => MinimalForwarderEOAOnly.sol} (95%)

diff --git a/contracts/Forwarder.sol b/contracts/Forwarder.sol
index 8327c29f5..e6a6ae9f9 100644
--- a/contracts/Forwarder.sol
+++ b/contracts/Forwarder.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.11;
 
-import "./openzeppelin-presets/metatx/MinimalForwarder.sol";
+import "@openzeppelin/contracts/metatx/MinimalForwarder.sol";
 
 /*
  * @dev Minimal forwarder for GSNv2
diff --git a/contracts/ForwarderEOAOnly.sol b/contracts/ForwarderEOAOnly.sol
new file mode 100644
index 000000000..6a4b26f1d
--- /dev/null
+++ b/contracts/ForwarderEOAOnly.sol
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.11;
+
+import "./openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol";
+
+/*
+ * @dev Minimal forwarder for GSNv2
+ */
+contract ForwarderEOAOnly is MinimalForwarderEOAOnly {
+    // solhint-disable-next-line no-empty-blocks
+    constructor() MinimalForwarderEOAOnly() {}
+}
diff --git a/contracts/openzeppelin-presets/metatx/MinimalForwarder.sol b/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
similarity index 95%
rename from contracts/openzeppelin-presets/metatx/MinimalForwarder.sol
rename to contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
index 895f380ea..437f7a48b 100644
--- a/contracts/openzeppelin-presets/metatx/MinimalForwarder.sol
+++ b/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
@@ -9,7 +9,7 @@ import "../utils/cryptography/EIP712.sol";
 /**
  * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
  */
-contract MinimalForwarder is EIP712 {
+contract MinimalForwarderEOAOnly is EIP712 {
     using ECDSA for bytes32;
 
     struct ForwardRequest {
@@ -26,7 +26,7 @@ contract MinimalForwarder is EIP712 {
 
     mapping(address => uint256) private _nonces;
 
-    constructor() EIP712("MinimalForwarder", "0.0.1") {}
+    constructor() EIP712("MinimalForwarderEOAOnly", "0.0.1") {}
 
     function getNonce(address from) public view returns (uint256) {
         return _nonces[from];

From 0ad85bb2a23414c9295ba6b76b2c7a3626f83518 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 29 Aug 2022 20:44:04 +0530
Subject: [PATCH 17/21] run prettier

---
 .../metatx/MinimalForwarderEOAOnly.sol         |  2 +-
 contracts/pack/Pack.sol                        | 18 +++---------------
 src/test/Pack.t.sol                            | 15 ++++++++-------
 src/test/PackBenchmark.t.sol                   |  5 ++---
 src/test/utils/BaseTest.sol                    |  5 +----
 5 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol b/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
index 437f7a48b..abf340cd1 100644
--- a/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
+++ b/contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol
@@ -48,7 +48,7 @@ contract MinimalForwarderEOAOnly is EIP712 {
         require(verify(req, signature), "MinimalForwarder: signature does not match request");
         _nonces[req.from] = req.nonce + 1;
 
-        (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(
+        (bool success, bytes memory returndata) = req.to.call{ gas: req.gas, value: req.value }(
             abi.encodePacked(req.data, req.from)
         );
 
diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index 20bccc746..b1fb78491 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -81,10 +81,7 @@ contract Pack is
                     Constructor + initializer logic
     //////////////////////////////////////////////////////////////*/
 
-    constructor(address _nativeTokenWrapper, address _trustedForwarder)
-        TokenStore(_nativeTokenWrapper)
-        initializer
-    {
+    constructor(address _nativeTokenWrapper, address _trustedForwarder) TokenStore(_nativeTokenWrapper) initializer {
         forwarder = _trustedForwarder;
     }
 
@@ -188,13 +185,7 @@ contract Pack is
         uint128 _openStartTimestamp,
         uint128 _amountDistributedPerOpen,
         address _recipient
-    )
-        external
-        payable
-        onlyRoleWithSwitch(minterRole)
-        nonReentrant
-        returns (uint256 packId, uint256 packTotalSupply)
-    {
+    ) external payable onlyRoleWithSwitch(minterRole) nonReentrant returns (uint256 packId, uint256 packTotalSupply) {
         require(_contents.length > 0, "!Contents");
         require(_contents.length == _numOfRewardUnits.length, "!Rewards");
 
@@ -293,10 +284,7 @@ contract Pack is
         for (uint256 i = 0; i < _contents.length; i += 1) {
             require(_contents[i].totalAmount != 0, "0 amt");
             require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "!Rewards");
-            require(
-                _contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1,
-                "!Rewards"
-            );
+            require(_contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1, "!Rewards");
 
             sumOfRewardUnits += _numOfRewardUnits[i];
 
diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index d524fb651..be8ce37d1 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -12,11 +12,7 @@ import "./utils/BaseTest.sol";
 
 contract PackTest is BaseTest {
     /// @notice Emitted when a set of packs is created.
-    event PackCreated(
-        uint256 indexed packId,
-        address recipient,
-        uint256 totalPacksCreated
-    );
+    event PackCreated(uint256 indexed packId, address recipient, uint256 totalPacksCreated);
 
     /// @notice Emitted when a pack is opened.
     event PackOpened(
@@ -645,7 +641,12 @@ contract PackTest is BaseTest {
         erc1155.mint(address(tokenOwner), 2, 200);
 
         vm.prank(address(tokenOwner));
-        (uint256 newTotalSupply, uint256 additionalSupply) = pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+        (uint256 newTotalSupply, uint256 additionalSupply) = pack.addPackContents(
+            packId,
+            additionalContents,
+            additionalContentsRewardUnits,
+            recipient
+        );
 
         // ERC20 balance after adding more tokens
         assertEq(erc20.balanceOf(address(tokenOwner)), 0);
@@ -712,7 +713,7 @@ contract PackTest is BaseTest {
         vm.expectRevert("!Allowed");
         pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
     }
-    
+
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `openPack`
     //////////////////////////////////////////////////////////////*/
diff --git a/src/test/PackBenchmark.t.sol b/src/test/PackBenchmark.t.sol
index 3546bd63c..144fcb578 100644
--- a/src/test/PackBenchmark.t.sol
+++ b/src/test/PackBenchmark.t.sol
@@ -302,7 +302,7 @@ contract OpenPackLargeInputsTest is BaseTest {
         // pass x: 1962, y: 282, z: 219 (gas: 8937393460523355524), total supply: 3239, total reward units: 913398
 
         // x: 570, y: 497, z: 435 (gas: 8937393460523355524), total supply: 548, total reward units: 272356
-        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute); 
+        //                          reverts at rewardUnits = new Token[](numOfRewardUnitsToDistribute);
         // x: 412, y: 7, z: 11830 (gas: 8937393460523355524), total supply: 29834, total reward units: 208838
         //                          reverts while transferring reward units to receipient
         // x: 1322, y: 211, z: 1994 (gas: 8937393460523355524), total supply: 3104, total reward units: 6544944
@@ -341,7 +341,7 @@ contract OpenPackLargeInputsTest is BaseTest {
             y,
             recipient
         );
-        
+
         vm.assume(z <= totalSupply);
     }
 
@@ -353,7 +353,6 @@ contract OpenPackLargeInputsTest is BaseTest {
      *  note: Testing state changes; pack owner calls `openPack` to redeem underlying rewards.
      */
     function test_fuzz_failing_state_openPack() public {
-
         console.log(gasleft());
         console2.log("total supply: ", totalSupply);
         console2.log("total reward units: ", totalRewardUnits);
diff --git a/src/test/utils/BaseTest.sol b/src/test/utils/BaseTest.sol
index 066039544..7e29d7b45 100644
--- a/src/test/utils/BaseTest.sol
+++ b/src/test/utils/BaseTest.sol
@@ -234,10 +234,7 @@ abstract contract BaseTest is DSTest, Test {
         );
         deployContractProxy(
             "Pack",
-            abi.encodeCall(
-                Pack.initialize,
-                (deployer, NAME, SYMBOL, CONTRACT_URI, royaltyRecipient, royaltyBps)
-            )
+            abi.encodeCall(Pack.initialize, (deployer, NAME, SYMBOL, CONTRACT_URI, royaltyRecipient, royaltyBps))
         );
     }
 

From bc9986b757aba7f965d6a8be5a500a90cc84d24a Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Mon, 29 Aug 2022 21:14:33 +0530
Subject: [PATCH 18/21] update tests

---
 src/test/sdk/extension/TokenBundle.t.sol | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/test/sdk/extension/TokenBundle.t.sol b/src/test/sdk/extension/TokenBundle.t.sol
index 204c54b35..e72e7be75 100644
--- a/src/test/sdk/extension/TokenBundle.t.sol
+++ b/src/test/sdk/extension/TokenBundle.t.sol
@@ -107,14 +107,14 @@ contract ExtensionTokenBundle is DSTest, Test {
     function test_revert_createBundle_emptyBundle() public {
         ITokenBundle.Token[] memory emptyBundle;
 
-        vm.expectRevert("TokenBundle: no tokens to bind.");
+        vm.expectRevert("no tokens to bind");
         ext.createBundle(emptyBundle, 0);
     }
 
     function test_revert_createBundle_existingBundleId() public {
         ext.createBundle(bundleContent, 0);
 
-        vm.expectRevert("TokenBundle: existent at bundleId");
+        vm.expectRevert("existent at bundleId");
         ext.createBundle(bundleContent, 0);
     }
 
@@ -128,7 +128,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
 
         bundleContent.pop();
@@ -141,7 +141,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
 
         bundleContent.pop();
@@ -154,7 +154,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
 
         bundleContent.pop();
@@ -167,7 +167,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
 
         bundleContent.pop();
@@ -180,7 +180,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
 
         bundleContent.pop();
@@ -193,7 +193,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             })
         );
 
-        vm.expectRevert("Asset doesn't match TokenType");
+        vm.expectRevert("!TokenType");
         ext.createBundle(bundleContent, 0);
     }
 
@@ -246,7 +246,7 @@ contract ExtensionTokenBundle is DSTest, Test {
         ext.createBundle(bundleContent, 0);
 
         ITokenBundle.Token[] memory emptyBundle;
-        vm.expectRevert("TokenBundle: no tokens to bind.");
+        vm.expectRevert("no tokens to bind");
         ext.updateBundle(emptyBundle, 0);
     }
 
@@ -320,7 +320,7 @@ contract ExtensionTokenBundle is DSTest, Test {
             totalAmount: 200
         });
 
-        vm.expectRevert("TokenBundle: index DNE.");
+        vm.expectRevert("index DNE");
         ext.updateTokenInBundle(newToken, 0, 3);
     }
 }

From fd0e73f76e48aaab4ec8816b74e7e35be2c7b643 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 31 Aug 2022 23:52:41 +0530
Subject: [PATCH 19/21] minor changes: supports interface, role vars, check
 recipient in addPackContents

---
 contracts/pack/Pack.sol | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index b1fb78491..da4469c8a 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -10,6 +10,7 @@ import "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/interfaces/IERC2981Upgradeable.sol";
 import "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
+import { IERC1155Receiver } from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol";
 
 //  ==========  Internal imports    ==========
 
@@ -94,9 +95,9 @@ contract Pack is
         address _royaltyRecipient,
         uint256 _royaltyBps
     ) external initializer {
-        transferRole = keccak256("TRANSFER_ROLE");
-        minterRole = keccak256("MINTER_ROLE");
-        assetRole = keccak256("ASSET_ROLE");
+        bytes32 _transferRole = keccak256("TRANSFER_ROLE");
+        bytes32 _minterRole = keccak256("MINTER_ROLE");
+        bytes32 _assetRole = keccak256("ASSET_ROLE");
         // Initialize inherited contracts, most base-like -> most derived.
         __ReentrancyGuard_init();
 
@@ -113,14 +114,18 @@ contract Pack is
 
         _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
 
-        _setupRole(transferRole, _defaultAdmin);
-        _setupRole(minterRole, _defaultAdmin);
-        _setupRole(transferRole, address(0));
+        _setupRole(_transferRole, _defaultAdmin);
+        _setupRole(_minterRole, _defaultAdmin);
+        _setupRole(_transferRole, address(0));
 
         // note: see `onlyRoleWithSwitch` for ASSET_ROLE behaviour.
-        _setupRole(assetRole, address(0));
+        _setupRole(_assetRole, address(0));
 
         _setupDefaultRoyaltyInfo(_royaltyRecipient, _royaltyBps);
+
+        transferRole = _transferRole;
+        minterRole = _minterRole;
+        assetRole = _assetRole;
     }
 
     receive() external payable {
@@ -170,7 +175,8 @@ contract Pack is
         return
             super.supportsInterface(interfaceId) ||
             type(IERC2981Upgradeable).interfaceId == interfaceId ||
-            type(IERC721Receiver).interfaceId == interfaceId;
+            type(IERC721Receiver).interfaceId == interfaceId || 
+            type(IERC1155Receiver).interfaceId == interfaceId;
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -232,6 +238,7 @@ contract Pack is
         require(canUpdatePack[_packId], "!Allowed");
         require(_contents.length > 0, "!Contents");
         require(_contents.length == _numOfRewardUnits.length, "!Rewards");
+        require(balanceOf(_recipient, _packId) != 0, "!Recipient");
 
         if (!hasRole(assetRole, address(0))) {
             for (uint256 i = 0; i < _contents.length; i += 1) {

From c7f16885417563bf880e051c222a994bc798bc62 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 31 Aug 2022 23:53:12 +0530
Subject: [PATCH 20/21] update tests

---
 src/test/Pack.t.sol | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/test/Pack.t.sol b/src/test/Pack.t.sol
index be8ce37d1..a52fdd014 100644
--- a/src/test/Pack.t.sol
+++ b/src/test/Pack.t.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.0;
 
-import { Pack } from "contracts/pack/Pack.sol";
+import { Pack, IERC2981Upgradeable, IERC721Receiver, IERC1155Upgradeable } from "contracts/pack/Pack.sol";
 import { IPack } from "contracts/interfaces/IPack.sol";
 import { ITokenBundle } from "contracts/extension/interface/ITokenBundle.sol";
 
@@ -183,6 +183,13 @@ contract PackTest is BaseTest {
         console2.logBytes4(type(IERC1155).interfaceId);
     }
 
+    function test_supportsInterface() public {
+        assertEq(pack.supportsInterface(type(IERC2981Upgradeable).interfaceId), true);
+        assertEq(pack.supportsInterface(type(IERC721Receiver).interfaceId), true);
+        assertEq(pack.supportsInterface(type(IERC1155Receiver).interfaceId), true);
+        assertEq(pack.supportsInterface(type(IERC1155Upgradeable).interfaceId), true);
+    }
+
     /**
      *  note: Testing state changes; token owner calls `createPack` to pack owned tokens.
      */
@@ -714,6 +721,23 @@ contract PackTest is BaseTest {
         pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
     }
 
+    /**
+     *  note: Testing revert condition; adding tokens with a different recipient.
+     */
+    function test_revert_addPackContents_NotRecipient() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+
+        address randomRecipient = address(0x12345);
+
+        vm.expectRevert("!Recipient");
+        vm.prank(address(tokenOwner));
+        pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, randomRecipient);
+    }
+
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `openPack`
     //////////////////////////////////////////////////////////////*/

From 263d982737f7ede4a4387c022ac047bc100a10f4 Mon Sep 17 00:00:00 2001
From: Yash <kumaryashcse@gmail.com>
Date: Wed, 31 Aug 2022 23:55:22 +0530
Subject: [PATCH 21/21] run prettier

---
 contracts/pack/Pack.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/pack/Pack.sol b/contracts/pack/Pack.sol
index da4469c8a..ad58cd400 100644
--- a/contracts/pack/Pack.sol
+++ b/contracts/pack/Pack.sol
@@ -175,7 +175,7 @@ contract Pack is
         return
             super.supportsInterface(interfaceId) ||
             type(IERC2981Upgradeable).interfaceId == interfaceId ||
-            type(IERC721Receiver).interfaceId == interfaceId || 
+            type(IERC721Receiver).interfaceId == interfaceId ||
             type(IERC1155Receiver).interfaceId == interfaceId;
     }