Zero pack transfer fix (#238)

* set bundle to private; use PermissionsEnumerable;

* v3.1.6-1

* zero pack transfer shouldn't toggle canUpdatePack

* fix forwarder

* bump version 1 -> 2

Author: kumaryash90

contracts/extension/TokenBundle.sol

@@ -17,7 +17,7 @@ interface IERC165 {
 
 abstract contract TokenBundle is ITokenBundle {
     /// @dev Mapping from bundle UID => bundle info.
-    mapping(uint256 => BundleInfo) public bundle;
+    mapping(uint256 => BundleInfo) private bundle;
 
     /// @dev Returns the total number of assets in a particular bundle.
     function getTokenCountOfBundle(uint256 _bundleId) public view returns (uint256) {

contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.sol

@@ -26,7 +26,7 @@ contract MinimalForwarderEOAOnly is EIP712 {
 
     mapping(address => uint256) private _nonces;
 
-    constructor() EIP712("MinimalForwarderEOAOnly", "0.0.1") {}
+    constructor() EIP712("GSNv2 Forwarder", "0.0.1") {}
 
     function getNonce(address from) public view returns (uint256) {
         return _nonces[from];

contracts/pack/Pack.sol

@@ -22,15 +22,15 @@ import "../openzeppelin-presets/metatx/ERC2771ContextUpgradeable.sol";
 import "../extension/ContractMetadata.sol";
 import "../extension/Royalty.sol";
 import "../extension/Ownable.sol";
-import "../extension/Permissions.sol";
+import "../extension/PermissionsEnumerable.sol";
 import { TokenStore, ERC1155Receiver } from "../extension/TokenStore.sol";
 
 contract Pack is
     Initializable,
     ContractMetadata,
     Ownable,
     Royalty,
-    Permissions,
+    PermissionsEnumerable,
     TokenStore,
     ReentrancyGuardUpgradeable,
     ERC2771ContextUpgradeable,
@@ -43,7 +43,7 @@ contract Pack is
     //////////////////////////////////////////////////////////////*/
 
     bytes32 private constant MODULE_TYPE = bytes32("Pack");
-    uint256 private constant VERSION = 1;
+    uint256 private constant VERSION = 2;
 
     address private immutable forwarder;
 
@@ -433,7 +433,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]] && amounts[i] != 0) {
                     canUpdatePack[ids[i]] = false;
                 }
             }

contracts/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@thirdweb-dev/contracts",
   "description": "Collection of smart contracts deployable via the thirdweb SDK, dashboard and CLI",
-  "version": "3.1.6-0",
+  "version": "3.1.6-1",
   "license": "Apache-2.0",
   "repository": {
     "type": "git",

docs/ERC721Multiwrap.md

@@ -83,29 +83,6 @@ function balanceOf(address owner) external view returns (uint256)
 |---|---|---|
 | _0 | uint256 | undefined |
 
-### bundle
-
-```solidity
-function bundle(uint256) external view returns (uint256 count, string uri)
-```
-
-
-
-
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| count | uint256 | undefined |
-| uri | string | undefined |
-
 ### contractURI
 
 ```solidity

docs/Multiwrap.md

@@ -66,29 +66,6 @@ function balanceOf(address owner) external view returns (uint256)
 |---|---|---|
 | _0 | uint256 | undefined |
 
-### bundle
-
-```solidity
-function bundle(uint256) external view returns (uint256 count, string uri)
-```
-
-
-
-
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| count | uint256 | undefined |
-| uri | string | undefined |
-
 ### contractType
 
 ```solidity

docs/Pack.md

@@ -99,29 +99,6 @@ function balanceOfBatch(address[] accounts, uint256[] ids) external view returns
 |---|---|---|
 | _0 | uint256[] | undefined |
 
-### bundle
-
-```solidity
-function bundle(uint256) external view returns (uint256 count, string uri)
-```
-
-
-
-
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| count | uint256 | undefined |
-| uri | string | undefined |
-
 ### canUpdatePack
 
 ```solidity
@@ -286,6 +263,51 @@ Returns the admin role that controls the specified role.
 |---|---|---|
 | _0 | bytes32 | undefined |
 
+### getRoleMember
+
+```solidity
+function getRoleMember(bytes32 role, uint256 index) external view returns (address member)
+```
+
+Returns the role-member from a list of members for a role,                  at a given index.
+
+*Returns `member` who has `role`, at `index` of role-members list.                  See struct {RoleMembers}, and mapping {roleMembers}*
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| role | bytes32 | keccak256 hash of the role. e.g. keccak256("TRANSFER_ROLE") |
+| index | uint256 | Index in list of current members for the role. |
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| member | address |  Address of account that has `role` |
+
+### getRoleMemberCount
+
+```solidity
+function getRoleMemberCount(bytes32 role) external view returns (uint256 count)
+```
+
+Returns total number of accounts that have a role.
+
+*Returns `count` of accounts that have `role`.                  See struct {RoleMembers}, and mapping {roleMembers}*
+
+#### Parameters
+
+| Name | Type | Description |
+|---|---|---|
+| role | bytes32 | keccak256 hash of the role. e.g. keccak256("TRANSFER_ROLE") |
+
+#### Returns
+
+| Name | Type | Description |
+|---|---|---|
+| count | uint256 |   Total number of accounts that have `role` |
+
 ### getRoyaltyInfoForToken
 
 ```solidity

docs/TokenBundle.md

@@ -10,29 +10,6 @@
 
 ## Methods
 
-### bundle
-
-```solidity
-function bundle(uint256) external view returns (uint256 count, string uri)
-```
-
-
-
-*Mapping from bundle UID => bundle info.*
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| count | uint256 | undefined |
-| uri | string | undefined |
-
 ### getTokenCountOfBundle
 
 ```solidity

docs/TokenStore.md

@@ -10,29 +10,6 @@
 
 ## Methods
 
-### bundle
-
-```solidity
-function bundle(uint256) external view returns (uint256 count, string uri)
-```
-
-
-
-
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| count | uint256 | undefined |
-| uri | string | undefined |
-
 ### getTokenCountOfBundle
 
 ```solidity

src/test/Pack.t.sol

@@ -173,6 +173,33 @@ contract PackTest is BaseTest {
         pack.grantRole(keccak256("MINTER_ROLE"), address(tokenOwner));
     }
 
+    /*///////////////////////////////////////////////////////////////
+                        Unit tests: Miscellaneous
+    //////////////////////////////////////////////////////////////*/
+
+    function test_revert_addPackContents_RandomAccountGrief() public {
+        uint256 packId = pack.nextTokenIdToMint();
+        address recipient = address(1);
+
+        vm.prank(address(tokenOwner));
+        pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
+
+        // random address tries to transfer zero amount
+        address randomAccount = address(0x123);
+        vm.prank(randomAccount);
+        pack.safeTransferFrom(randomAccount, address(567), packId, 0, ""); // zero transfer
+
+        // canUpdatePack should remain true, since no packs were transferred
+        assertTrue(pack.canUpdatePack(packId));
+
+        erc20.mint(address(tokenOwner), 1000 ether);
+        erc1155.mint(address(tokenOwner), 2, 200);
+
+        vm.prank(address(tokenOwner));
+        // Should not revert
+        pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
+    }
+
     /*///////////////////////////////////////////////////////////////
                         Unit tests: `createPack`
     //////////////////////////////////////////////////////////////*/