Yash/pack review (#195)

* receive function update

* tests for valid token inputs

* check for TokenType

* update tests

* run prettier

* fix/add forge dir

* add forge url

Author: kumaryash90

contracts/extension/TokenBundle.sol

@@ -2,6 +2,11 @@
 pragma solidity ^0.8.0;
 
 import "./interface/ITokenBundle.sol";
+import "../lib/CurrencyTransferLib.sol";
+
+interface IERC165 {
+    function supportsInterface(bytes4 interfaceId) external view returns (bool);
+}
 
 abstract contract TokenBundle is ITokenBundle {
     /// @dev Mapping from bundle UID => bundle info.
@@ -30,6 +35,7 @@ abstract contract TokenBundle is ITokenBundle {
         require(bundle[_bundleId].count == 0, "TokenBundle: existent at bundleId");
 
         for (uint256 i = 0; i < targetCount; i += 1) {
+            _checkTokenType(_tokensToBind[i]);
             bundle[_bundleId].tokens[i] = _tokensToBind[i];
         }
 
@@ -46,6 +52,7 @@ abstract contract TokenBundle is ITokenBundle {
 
         for (uint256 i = 0; i < check; i += 1) {
             if (i < targetCount) {
+                _checkTokenType(_tokensToBind[i]);
                 bundle[_bundleId].tokens[i] = _tokensToBind[i];
             } else if (i < currentCount) {
                 delete bundle[_bundleId].tokens[i];
@@ -57,6 +64,7 @@ abstract contract TokenBundle is ITokenBundle {
 
     /// @dev Lets the calling contract add a token to a bundle for a unique bundle id and index.
     function _addTokenInBundle(Token memory _tokenToBind, uint256 _bundleId) internal {
+        _checkTokenType(_tokenToBind);
         uint256 id = bundle[_bundleId].count;
 
         bundle[_bundleId].tokens[id] = _tokenToBind;
@@ -70,9 +78,38 @@ abstract contract TokenBundle is ITokenBundle {
         uint256 _index
     ) internal {
         require(_index < bundle[_bundleId].count, "TokenBundle: index DNE.");
+        _checkTokenType(_tokenToBind);
         bundle[_bundleId].tokens[_index] = _tokenToBind;
     }
 
+    /// @dev Checks if the type of asset-contract is same as the TokenType specified.
+    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");
+            } catch {
+                revert("Asset doesn't match TokenType");
+            }
+        } else if (_token.tokenType == TokenType.ERC1155) {
+            try IERC165(_token.assetContract).supportsInterface(0xd9b67a26) returns (bool supported1155) {
+                require(supported1155, "Asset doesn't match TokenType");
+            } catch {
+                revert("Asset doesn't match 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");
+
+                    try IERC165(_token.assetContract).supportsInterface(0xd9b67a26) returns (bool supported1155) {
+                        require(!supported1155, "Asset doesn't match 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 {
         bundle[_bundleId].uri = _uri;

contracts/extension/TokenStore.sol

@@ -11,7 +11,7 @@ import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";
 
 //  ==========  Internal imports    ==========
 
-import "./TokenBundle.sol";
+import { TokenBundle, ITokenBundle } from "./TokenBundle.sol";
 import "../lib/CurrencyTransferLib.sol";
 
 contract TokenStore is TokenBundle, ERC721Holder, ERC1155Holder {

contracts/pack/Pack.sol

@@ -109,7 +109,7 @@ contract Pack is
     }
 
     receive() external payable {
-        require(_msgSender() == nativeTokenWrapper, "Caller is not native token wrapper.");
+        require(msg.sender == nativeTokenWrapper, "Caller is not native token wrapper.");
     }
 
     /*///////////////////////////////////////////////////////////////
@@ -215,11 +215,11 @@ contract Pack is
     {
         address opener = _msgSender();
 
-        require(opener == tx.origin, "opener must be eoa");
+        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");
+        require(pack.openStartTimestamp <= block.timestamp, "cannot open yet");
 
         Token[] memory rewardUnits = getRewardUnits(_packId, _amountToOpen, pack.amountDistributedPerOpen, pack);
 
@@ -232,6 +232,7 @@ contract Pack is
         return rewardUnits;
     }
 
+    /// @dev Stores assets within the contract.
     function escrowPackContents(
         Token[] calldata _contents,
         uint256[] calldata _numOfRewardUnits,
@@ -242,6 +243,7 @@ contract Pack is
         uint256 totalRewardUnits;
 
         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,

lib/forge-std

@@ -159,6 +159,12 @@ contract PackTest is BaseTest {
                         Unit tests: `createPack`
     //////////////////////////////////////////////////////////////*/
 
+    function test_interface() public {
+        console2.logBytes4(type(IERC20).interfaceId);
+        console2.logBytes4(type(IERC721).interfaceId);
+        console2.logBytes4(type(IERC1155).interfaceId);
+    }
+
     /**
      *  note: Testing state changes; token owner calls `createPack` to pack owned tokens.
      */
@@ -383,7 +389,7 @@ contract PackTest is BaseTest {
                 totalAmount: 20 ether
             })
         );
-        numOfRewardUnits.push(1 ether);
+        numOfRewardUnits.push(1);
 
         vm.prank(address(tokenOwner));
         vm.expectRevert("msg.value != amount");
@@ -468,6 +474,52 @@ contract PackTest is BaseTest {
         pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
     }
 
+    /**
+     *  note: Testing revert condition; token owner calls `createPack` with invalid token-type.
+     */
+    function test_revert_createPack_invalidTokenType() public {
+        ITokenBundle.Token[] memory invalidContent = new ITokenBundle.Token[](1);
+        uint256[] memory rewardUnits = new uint256[](1);
+
+        invalidContent[0] = ITokenBundle.Token({
+            assetContract: address(erc721),
+            tokenType: ITokenBundle.TokenType.ERC20,
+            tokenId: 0,
+            totalAmount: 1
+        });
+        rewardUnits[0] = 1;
+
+        address recipient = address(0x123);
+
+        vm.startPrank(address(tokenOwner));
+        vm.expectRevert("Asset doesn't match TokenType");
+        pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
+    }
+
+    /**
+     *  note: Testing revert condition; token owner calls `createPack` with total-amount as 0.
+     */
+    function test_revert_createPack_zeroTotalAmount() public {
+        ITokenBundle.Token[] memory invalidContent = new ITokenBundle.Token[](1);
+        uint256[] memory rewardUnits = new uint256[](1);
+
+        invalidContent[0] = ITokenBundle.Token({
+            assetContract: address(erc20),
+            tokenType: ITokenBundle.TokenType.ERC20,
+            tokenId: 0,
+            totalAmount: 0
+        });
+        rewardUnits[0] = 10;
+
+        address recipient = address(0x123);
+
+        vm.startPrank(address(tokenOwner));
+        vm.expectRevert("amount can't be zero");
+        (, uint256 totalSupply) = pack.createPack(invalidContent, rewardUnits, packUri, 0, 1, recipient);
+
+        // assertEq(totalSupply, 10);
+    }
+
     /**
      *  note: Testing revert condition; token owner calls `createPack` with no tokens to pack.
      */

src/test/Pack.t.sol

@@ -27,4 +27,8 @@ contract MockERC1155 is ERC1155PresetMinterPauser {
 
         _mintBatch(to, ids, amounts, "");
     }
+
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+        return super.supportsInterface(interfaceId);
+    }
 }

src/test/mocks/MockERC1155.sol

@@ -17,4 +17,8 @@ contract MockERC721 is ERC721Burnable {
             tokenId += 1;
         }
     }
+
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+        return super.supportsInterface(interfaceId);
+    }
 }