Pack refactor downsized (#229)

* update design doc for Pack

* openPack out of gas -- fuzz test

* [M-1] Lost pack tokens result in permanently locked assets

* [G-2] Reduce function calls in openPack

* [L-3] supportsInterface() may prevent ERC721 tokens from transferring to Pack

* [H-1] Reward selection exploits

* optimize openPack

* pack refactor: adding more tokens to a pack

* add tests

* wip: reduce size

* cremoved stuffs

* restore roles, token-bundle getters; remove withdrawUnclaimedAssets, packTimestamps

* immutable forwarder; edit revert strings;

* cleanup

* update tests

* eoa-only forwarder

* run prettier

* update tests

* minor changes: supports interface, role vars, check recipient in addPackContents

* update tests

* run prettier

Co-authored-by: Jake Loo <[email protected]>

Author: kumaryash90

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() {}
+}

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) 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) {
@@ -38,8 +38,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]);
@@ -50,8 +50,8 @@ 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 {
-        require(_tokensToBind.length > 0, "TokenBundle: no tokens to bind.");
+    function _updateBundle(Token[] memory _tokensToBind, uint256 _bundleId) internal {
+        require(_tokensToBind.length > 0, "no tokens to bind");
 
         uint256 currentCount = bundle[_bundleId].count;
         uint256 targetCount = _tokensToBind.length;
@@ -84,7 +84,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;
     }
@@ -93,32 +93,32 @@ 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 {}
             }
         }
     }
 
     /// @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;
     }
 

contracts/extension/TokenStore.sol

@@ -22,9 +22,6 @@ 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;
 
@@ -36,7 +33,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);

contracts/interfaces/IPack.sol

@@ -24,12 +24,10 @@ interface IPack is ITokenBundle {
     }
 
     /// @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 recipient, uint256 totalPacksCreated);
 
     /// @notice Emitted when a pack is opened.
     event PackOpened(

contracts/openzeppelin-presets/metatx/MinimalForwarderEOAOnly.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 MinimalForwarderEOAOnly 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("MinimalForwarderEOAOnly", "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);
+    }
+}