Merge pull request #1685 from thesandboxgame/passes-audit-h-03

[H-03] Game Passes Audit

Author: wojciech-turek

packages/deploy/deploy/1100_game_passes/1100_deploy_game_passes.ts

@@ -0,0 +1,59 @@
+import {DeployFunction} from 'hardhat-deploy/types';
+import {HardhatRuntimeEnvironment} from 'hardhat/types';
+import {DEPLOY_TAGS} from '../../hardhat.config';
+
+const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
+  const {deployments, getNamedAccounts} = hre;
+  const {deploy} = deployments;
+  const {
+    deployer,
+    treasury,
+    commonRoyaltyReceiver,
+    backendAuthWallet,
+    sandAdmin,
+    upgradeAdmin,
+  } = await getNamedAccounts();
+
+  const TRUSTED_FORWARDER = await deployments.get('SandboxForwarder');
+  const sandContract = await deployments.get('PolygonSand');
+  const BASE_URL = 'https://contracts.sandbox.game';
+
+  await deploy('GamePasses', {
+    from: deployer,
+    contract:
+      '@sandbox-smart-contracts/game-passes/contracts/GamePasses.sol:GamePasses',
+    proxy: {
+      owner: upgradeAdmin,
+      proxyContract: 'OpenZeppelinTransparentProxy',
+      execute: {
+        methodName: 'initialize',
+        args: [
+          {
+            baseURI: BASE_URL,
+            royaltyReceiver: commonRoyaltyReceiver,
+            royaltyFeeNumerator: 500, // 5%
+            admin: sandAdmin,
+            operator: deployer,
+            signer: backendAuthWallet,
+            paymentToken: sandContract.address,
+            trustedForwarder: TRUSTED_FORWARDER.address,
+            defaultTreasury: treasury,
+            owner: sandAdmin,
+          },
+        ],
+      },
+      upgradeIndex: 0,
+    },
+    log: true,
+  });
+};
+export default func;
+
+func.tags = [
+  'GamePass',
+  'GamePass_deploy',
+  DEPLOY_TAGS.L2,
+  DEPLOY_TAGS.L2_PROD,
+  DEPLOY_TAGS.L2_TEST,
+];
+func.dependencies = ['SandboxForwarder'];

packages/deploy/hardhat.config.ts

@@ -95,6 +95,7 @@ const importedPackages = {
   '@sandbox-smart-contracts/sandbox-forwarder': [
     'contracts/SandboxForwarder.sol',
   ],
+  '@sandbox-smart-contracts/game-passes': ['contracts/GamePasses.sol'],
 };
 
 const namedAccounts = {

packages/deploy/package.json

@@ -23,6 +23,7 @@
     "@sandbox-smart-contracts/dependency-operator-filter": "1.0.1",
     "@sandbox-smart-contracts/dependency-royalty-management": "1.0.2",
     "@sandbox-smart-contracts/faucets": "0.0.1",
+    "@sandbox-smart-contracts/game-passes": "*",
     "@sandbox-smart-contracts/giveaway": "0.0.3",
     "@sandbox-smart-contracts/land": "1.0.0-rc.1",
     "@sandbox-smart-contracts/land-sale": "1.0.0-rc.1",

packages/game-passes/contracts/GamePasses.sol

@@ -311,7 +311,6 @@ contract GamePasses is
      *        - trustedForwarder: Address of the trusted meta-transaction forwarder.
      *        - defaultTreasury: Address of the default treasury wallet.
      *        - owner: Address that will be set as the internal owner.
-     *        - sandContract: Address of the SAND contract.
      */
     function initialize(InitParams calldata params) external initializer {
         __ERC2771Handler_init(params.trustedForwarder);
@@ -582,11 +581,10 @@ contract GamePasses is
         }
 
         _updateAndCheckTotalMinted(mintTokenId, mintAmount);
-        mintConfig.mintedPerWallet[mintTo] += mintAmount;
+        _updateAndCheckMaxPerWallet(mintTokenId, mintTo, mintAmount);
 
         _burn(burnFrom, burnTokenId, burnAmount);
 
-        _checkMaxPerWallet(mintTokenId, mintTo, mintAmount);
         _mint(mintTo, mintTokenId, mintAmount, "");
     }
 
@@ -640,8 +638,7 @@ contract GamePasses is
             }
 
             _updateAndCheckTotalMinted(mintTokenIds[i], mintAmounts[i]);
-            _checkMaxPerWallet(mintTokenIds[i], mintTo, mintAmounts[i]);
-            mintConfig.mintedPerWallet[mintTo] += mintAmounts[i];
+            _updateAndCheckMaxPerWallet(mintTokenIds[i], mintTo, mintAmounts[i]);
         }
 
         _burnBatch(burnFrom, burnTokenIds, burnAmounts);
@@ -707,11 +704,9 @@ contract GamePasses is
         verifyBurnAndMintSignature(request, signature);
 
         _updateAndCheckTotalMinted(mintId, mintAmount);
-        mintConfig.mintedPerWallet[caller] += mintAmount;
+        _updateAndCheckMaxPerWallet(mintId, caller, mintAmount);
 
         _burn(caller, burnId, burnAmount);
-
-        _checkMaxPerWallet(mintId, caller, mintAmount);
         _mint(caller, mintId, mintAmount, "");
     }
 
@@ -1074,12 +1069,11 @@ contract GamePasses is
     /**
      * @notice Returns the metadata URI for a specific token ID
      * @param tokenId ID of the token to get URI for
-     * @dev Constructs the URI by concatenating baseURI + tokenId + ".json"
-     * @dev Can be overridden by derived contracts to implement different URI logic
-     * @return string The complete URI for the token metadata
+     * @dev Returns the token-specific metadata string stored in the token configuration
+     * @return string The metadata URI for the token
      */
     function uri(uint256 tokenId) public view virtual override returns (string memory) {
-        return string(abi.encodePacked(_coreStorage().baseURI, tokenId.toString(), ".json"));
+        return _tokenStorage().tokenConfigs[tokenId].metadata;
     }
 
     /**
@@ -1272,11 +1266,9 @@ contract GamePasses is
 
         verifySignature(request, signature);
 
-        _checkMaxPerWallet(request.tokenId, request.caller, request.amount);
+        _updateAndCheckMaxPerWallet(request.tokenId, request.caller, request.amount);
         _updateAndCheckTotalMinted(request.tokenId, request.amount);
 
-        config.mintedPerWallet[request.caller] += request.amount;
-
         address treasury = config.treasuryWallet;
         if (treasury == address(0)) {
             treasury = _coreStorage().defaultTreasuryWallet;
@@ -1308,11 +1300,9 @@ contract GamePasses is
                 revert TokenNotConfigured(request.tokenIds[i]);
             }
 
-            _checkMaxPerWallet(request.tokenIds[i], request.caller, request.amounts[i]);
+            _updateAndCheckMaxPerWallet(request.tokenIds[i], request.caller, request.amounts[i]);
             _updateAndCheckTotalMinted(request.tokenIds[i], request.amounts[i]);
 
-            config.mintedPerWallet[request.caller] += request.amounts[i];
-
             address treasury = config.treasuryWallet;
             if (treasury == address(0)) {
                 treasury = _coreStorage().defaultTreasuryWallet;
@@ -1391,14 +1381,16 @@ contract GamePasses is
      *      - Current wallet balance + amount would exceed max per wallet
      * @dev Skips check if maxPerWallet is type(uint256).max (unlimited)
      */
-    function _checkMaxPerWallet(uint256 tokenId, address to, uint256 amount) private view {
+    function _updateAndCheckMaxPerWallet(uint256 tokenId, address to, uint256 amount) private {
         TokenConfig storage config = _tokenStorage().tokenConfigs[tokenId];
 
+        config.mintedPerWallet[to] += amount;
+
         if (config.maxPerWallet == 0) {
             revert ExceedsMaxPerWallet(tokenId, to, amount, 0);
         }
 
-        if (config.maxPerWallet != type(uint256).max && config.mintedPerWallet[to] + amount > config.maxPerWallet) {
+        if (config.mintedPerWallet[to] > config.maxPerWallet) {
             revert ExceedsMaxPerWallet(tokenId, to, amount, config.maxPerWallet);
         }
     }

packages/game-passes/test/GamePasses.test.ts

@@ -2772,16 +2772,14 @@ describe('GamePasses', function () {
 
   describe('URI', function () {
     it('should return correct token URI', async function () {
-      const {sandboxPasses, TOKEN_ID_1, BASE_URI} =
+      const {sandboxPasses, TOKEN_ID_1, TOKEN_METADATA} =
         await loadFixture(runCreateTestSetup);
 
-      expect(await sandboxPasses.uri(TOKEN_ID_1)).to.equal(
-        `${BASE_URI}${TOKEN_ID_1}.json`,
-      );
+      expect(await sandboxPasses.uri(TOKEN_ID_1)).to.equal(TOKEN_METADATA);
     });
 
     it('should allow admin to update base URI', async function () {
-      const {sandboxPasses, admin, TOKEN_ID_1, BASE_URI} =
+      const {sandboxPasses, admin, TOKEN_ID_1, BASE_URI, TOKEN_METADATA} =
         await loadFixture(runCreateTestSetup);
 
       const newBaseURI = 'https://new-api.example.com/metadata/';
@@ -2791,9 +2789,7 @@ describe('GamePasses', function () {
         .withArgs(admin.address, BASE_URI, newBaseURI);
 
       expect(await sandboxPasses.baseURI()).to.equal(newBaseURI);
-      expect(await sandboxPasses.uri(TOKEN_ID_1)).to.equal(
-        `${newBaseURI}${TOKEN_ID_1}.json`,
-      );
+      expect(await sandboxPasses.uri(TOKEN_ID_1)).to.equal(TOKEN_METADATA);
     });
 
     it('should not allow non-admin to update base URI', async function () {

packages/sandbox-forwarder/package.json

@@ -3,9 +3,6 @@
   "version": "1.0.0",
   "description": "Meta-transaction forwarder contract for The Sandbox",
   "private": true,
-  "scripts": {
-   
-  },
   "dependencies": {
     "@openzeppelin/contracts": "5.2.0"
   },

yarn.lock

@@ -2961,6 +2961,7 @@ __metadata:
     "@sandbox-smart-contracts/dependency-operator-filter": 1.0.1
     "@sandbox-smart-contracts/dependency-royalty-management": 1.0.2
     "@sandbox-smart-contracts/faucets": 0.0.1
+    "@sandbox-smart-contracts/game-passes": "*"
     "@sandbox-smart-contracts/giveaway": 0.0.3
     "@sandbox-smart-contracts/land": 1.0.0-rc.1
     "@sandbox-smart-contracts/land-sale": 1.0.0-rc.1
@@ -3082,7 +3083,7 @@ __metadata:
   languageName: unknown
   linkType: soft
 
-"@sandbox-smart-contracts/game-passes@workspace:packages/game-passes":
+"@sandbox-smart-contracts/game-passes@*, @sandbox-smart-contracts/game-passes@workspace:packages/game-passes":
   version: 0.0.0-use.local
   resolution: "@sandbox-smart-contracts/game-passes@workspace:packages/game-passes"
   dependencies: