Fixing the Locked issue

audit/README.md

@@ -0,0 +1,49 @@
+# Audit Report Summary
+
+This repository contains the code audited by EtherAuthority, specifically the commit at https://github.com/superpowerlabs/in-game-assets/tree/b17e7203ab2853d7dad037b7f48257fef9932b43. The full audit report can be found [here](https://etherauthority.io/mobland-protocol-smart-contract-audit/), and a PDF version is also available in this folder.
+
+We appreciate the professionalism and thoroughness of the audit conducted by EtherAuthority. However, we would like to address some of the issues mentioned in the report.
+
+## Deposit ID Override Concern: GamePool.sol: \_depositFT()
+
+This issue is not a vulnerability. The depositId is generated off-chain by the Game App and passed to the functions depositBud and depositSeed along with a signature to guarantee the correctness of the parameters passed to the contract.
+
+For example, the depositSeed function is shown below:
+
+```solidity
+function depositSeed(
+  uint256 amount,
+  uint64 depositId,
+  uint256 randomNonce,
+  bytes calldata signature0
+) external override {
+  if (!isSignedByAValidator(0, 2, hashDeposit(_msgSender(), amount, depositId, randomNonce), signature0))
+    revert invalidPrimarySignature();
+  _saveSignatureAsUsed(signature0);
+  _depositFT(SEED, amount, depositId, _msgSender());
+}
+
+```
+
+The uniqueness of the depositId is ensured by the Game dApp, which also generates the signature, and by the fact that the signature is saved as used with \_saveSignatureAsUsed(signature0) and cannot be used again.
+
+Since the smart contract must trust the signature accompanying the request, checking for the uniqueness of the depositId would only waste gas.
+
+## Infinite Loops Possibility: NftFactory.sol: newSale()
+
+There is no risk of infinite loops. The factory was designed as a generic factory for flexibility, but it was only used to sell two NFTs—Turf and Farm tokens—using two payment tokens—SEED and USDC.
+
+As a result, we did not check the gasleft() in the loop to ensure the transaction would not run out of gas.
+
+## Solhint Warnings
+
+We initially used Solhint to check for lint warnings and errors. After the audit report, we integrated it into the pre-commit process, allowing commits only if Solhint returns no warnings and all tests pass. In both scenarios, we did not encounter any of the issues mentioned in the report.
+
+Many of these issues appear to be false positives, as they involve errors such as:
+
+```
+NftFactory.sol:3565:64: Error: Parse error: mismatched input '('
+expecting {';', '='}
+```
+
+A missing `;` would result in a compilation error, not a warning. And we would not be able to deploy the contracts and make the sale.

bin/clean-licenses-in-flattened.js

@@ -2,20 +2,73 @@
 
 const fs = require("fs");
 const path = require("path");
+const ethers = require("ethers");
 
 const [, , contract] = process.argv;
 const fn = path.resolve(__dirname, "../flattened", `${contract}-flattened.sol`);
-const flattened = fs.readFileSync(fn, "utf8");
+const flattened =
+  fs.readFileSync(fn, "utf8") +
+  `
+// Flattened on ${new Date().toISOString()}
+`;
 
-flattened
-  .replace(/SPDX-License-Identifier/, "LICENSE-ID")
-  .replace(/SPDX-License-Identifier/g, "License")
-  .replace(/LICENSE-ID/, "SPDX-License-Identifier");
+function removeAndCompact(contents) {
+  // Remove single-line comments
+  const singleLineComments = /\/\/.*$/gm;
+  contents = contents.replace(singleLineComments, "");
 
-fs.writeFileSync(
-  fn,
+  // Remove multi-line comments
+  const multiLineComments = /\/\*[\s\S]*?\*\//gm;
+  contents = contents.replace(multiLineComments, "");
+
+  // Remove new lines and extra spaces
+  const newLines = /\s*[\r\n]+\s*/g;
+  contents = contents.replace(newLines, " ");
+
+  // Remove extra spaces between words
+  const extraSpaces = / +/g;
+  contents = contents.replace(extraSpaces, " ");
+
+  // Remove space before and after curly braces
+  const spacesAroundBraces = /\s*([{}])\s*/g;
+  contents = contents.replace(spacesAroundBraces, "$1");
+
+  // Remove space before and after parentheses
+  const spacesAroundParentheses = /\s*([()])\s*/g;
+  contents = contents.replace(spacesAroundParentheses, "$1");
+
+  // Remove space before and after semicolons
+  const spacesAroundSemicolons = /\s*;\s*/g;
+  contents = contents.replace(spacesAroundSemicolons, ";");
+
+  // Remove space before and after commas
+  const spacesAroundCommas = /\s*,\s*/g;
+  contents = contents.replace(spacesAroundCommas, ",");
+  return contents.trim();
+}
+
+const hash = ethers.utils.id(removeAndCompact(flattened)).substring(2, 10);
+
+const nfn = fn.replace(/flattened\.sol$/, `${hash}.sol`);
+if (fs.existsSync(nfn)) {
+  const prev = fs.readFileSync(nfn, "utf8").split("\n// Flattened on ")[1].split("\n")[0].split("T")[0];
+  console.log(`
+An identical version of ${contract} has been previously flattened on ${prev}.
+
+`);
+} else {
   flattened
     .replace(/SPDX-License-Identifier/, "LICENSE-ID")
     .replace(/SPDX-License-Identifier/g, "License")
-    .replace(/LICENSE-ID/, "SPDX-License-Identifier")
-);
+    .replace(/LICENSE-ID/, "SPDX-License-Identifier");
+
+  fs.writeFileSync(
+    fn.replace(/flattened\.sol$/, `${hash}.sol`),
+    flattened
+      .replace(/SPDX-License-Identifier/, "LICENSE-ID")
+      .replace(/SPDX-License-Identifier/g, "License")
+      .replace(/LICENSE-ID/, "SPDX-License-Identifier")
+  );
+
+  fs.unlinkSync(fn);
+}

contracts/GamePool.sol

@@ -25,30 +25,6 @@ import "./utils/SignableStakes.sol";
 contract GamePool is IGamePool, SignableStakes, Constants, UUPSUpgradableTemplate, IAttributablePlayer {
   using SafeMathUpgradeable for uint256;
 
-  event AssetStaked(address from, uint8 tokenType, uint16 tokenId);
-  event AssetUnstaked(address to, uint8 tokenType, uint16 tokenId);
-  event WithdrawnFT(uint8 tokenType, uint256 amount, address beneficiary);
-
-  error turfNotERC721();
-  error farmNotERC721();
-  error seedNotSEED();
-  error budNotBUD();
-  error turfAlreadyLocked();
-  error farmAlreadyLocked();
-  error invalidTokenType();
-  error invalidPrimarySignature();
-  error invalidSecondarySignature();
-  error assetNotFound();
-  error turfNotLocked();
-  error farmNotLocked();
-  error signatureAlreadyUsed();
-  error invalidRecipient();
-  error invalidNFT();
-  error harvestingExpired();
-  error amountNotAvailable();
-  error unsupportedNFT();
-  error depositAlreadyExists();
-
   Conf public conf;
   mapping(address => User) internal _users;
   mapping(bytes32 => bool) private _usedSignatures;
@@ -427,13 +403,12 @@ contract GamePool is IGamePool, SignableStakes, Constants, UUPSUpgradableTemplat
     emit Harvested(_msgSender(), amount, opId);
   }
 
-  // THIS IS NOT USED, can we remove it?
-  // /// @notice Returns true if the signature has been used before
-  // /// @param signature bytes for the signature
-  // function isSignatureUsed(bytes calldata signature) external view returns (bool) {
-  //   bytes32 key = bytes32(keccak256(abi.encodePacked(signature)));
-  //   return _usedSignatures[key];
-  // }
+  /// @notice Returns true if the signature has been used before
+  /// @param signature bytes for the signature
+  function isSignatureUsed(bytes calldata signature) external view returns (bool) {
+    bytes32 key = bytes32(keccak256(abi.encodePacked(signature)));
+    return _usedSignatures[key];
+  }
 
   /// @notice Withdraws an amount of funds in SEEDS or BUDS, or all of them if amount is 0
   /// @dev The token emits a Transfer event with the pool as the sender,

contracts/SuperpowerNFT.sol

@@ -110,4 +110,6 @@ abstract contract SuperpowerNFT is ISuperpowerNFT, SuperpowerNFTBase {
   function nextTokenId() external view override returns (uint256) {
     return _nextTokenId;
   }
+
+  uint256[50] private __gap;
 }

contracts/SuperpowerNFTBase.sol

@@ -12,7 +12,7 @@ import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721Enumer
 import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/utils/StringsUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
-import "./wormhole721/Wormhole721Upgradeable.sol";
+import "@ndujalabs/wormhole721/contracts/Wormhole721Upgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol";
 
 import "./interfaces/IAttributable.sol";
@@ -154,7 +154,6 @@ abstract contract SuperpowerNFTBase is
     returns (bool)
   {
     return
-      interfaceId == type(IERC5192).interfaceId ||
       interfaceId == type(IAttributable).interfaceId ||
       interfaceId == type(ILockable).interfaceId ||
       super.supportsInterface(interfaceId);
@@ -241,7 +240,7 @@ abstract contract SuperpowerNFTBase is
       revert LockerNotApproved();
     }
     _lockedBy[tokenId] = _msgSender();
-    emit Locked(tokenId);
+    emit Locked(tokenId, true);
   }
 
   function unlock(uint256 tokenId) external override onlyLocker {
@@ -250,7 +249,7 @@ abstract contract SuperpowerNFTBase is
       revert WrongLocker();
     }
     delete _lockedBy[tokenId];
-    emit Unlocked(tokenId);
+    emit Locked(tokenId, false);
   }
 
   // emergency function in case a compromised locker is removed
@@ -314,7 +313,7 @@ abstract contract SuperpowerNFTBase is
     uint16 recipientChain,
     bytes32 recipient,
     uint32 nonce
-  ) public payable override returns (uint64 sequence) {
+  ) public payable override whenNotPaused returns (uint64 sequence) {
     if (locked(tokenID)) revert LockedAsset();
     return super.wormholeTransfer(tokenID, recipientChain, recipient, nonce);
   }

contracts/interfaces/IERC721DefaultLockable.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: CC0-1.0
+pragma solidity ^0.8.0;
+
+interface IERC721DefaultLockable {
+  // Must be emitted one time, when the contract is deployed,
+  // defining the default status of any token that will be minted
+  event DefaultLocked(bool locked);
+
+  // Must be emitted any time the status changes
+  event Locked(uint256 indexed tokenId, bool locked);
+
+  // Returns the status of the token.
+  // It should revert if the token does not exist.
+  function locked(uint256 tokenId) external view returns (bool);
+}

contracts/interfaces/IGamePool.sol

@@ -11,6 +11,30 @@ interface IGamePool is IUserSimplified {
   event Harvested(address user, uint256 amount, uint64 opId);
   event NewDepositAndPay(uint64 depositId, address user, address otherUser);
   event NewDeposit(uint64 depositId, address user, uint8 tokenType, uint256 amount);
+  event AssetStaked(address from, uint8 tokenType, uint16 tokenId);
+  event AssetUnstaked(address to, uint8 tokenType, uint16 tokenId);
+  event WithdrawnFT(uint8 tokenType, uint256 amount, address beneficiary);
+
+  error turfNotERC721();
+  error farmNotERC721();
+  error seedNotSEED();
+  error budNotBUD();
+  error turfAlreadyLocked();
+  error farmAlreadyLocked();
+  error invalidTokenType();
+  error invalidPrimarySignature();
+  error invalidSecondarySignature();
+  error assetNotFound();
+  error turfNotLocked();
+  error farmNotLocked();
+  error signatureAlreadyUsed();
+  error invalidRecipient();
+  error invalidNFT();
+  error harvestingExpired();
+  error amountNotAvailable();
+  error unsupportedNFT();
+  error depositAlreadyExists();
+
   /**
       @dev to have a quick vision of the TVL
     */

contracts/interfaces/ILockable.sol

@@ -5,10 +5,10 @@ pragma solidity ^0.8.4;
 // Francesco Sullo <[email protected]>
 // Taken from https://github.com/ndujaLabs/lockable
 
-import "./IERC5192.sol";
+import "./IERC721DefaultLockable.sol";
 
 // ERC165 interface id is 0xd8e4c296
-interface ILockable is IERC5192 {
+interface ILockable is IERC721DefaultLockable {
   event LockerSet(address locker);
   event LockerRemoved(address locker);
   event ForcefullyUnlocked(uint256 tokenId);

contracts/interfaces/ISuperpowerNFT.sol

@@ -4,7 +4,9 @@ pragma solidity 0.8.17;
 // Authors: Francesco Sullo <[email protected]>
 // (c) Superpower Labs Inc
 
-interface ISuperpowerNFT {
+import "./ISuperpowerNFTBase.sol";
+
+interface ISuperpowerNFT is ISuperpowerNFTBase {
   error Forbidden();
   error CannotMint();
   error InvalidSupply();

contracts/mocks/FarmMock.sol

@@ -18,4 +18,8 @@ contract FarmMock {
   function mintTokens(address to, uint256 amount) external {
     nft.mint(to, amount);
   }
+
+  function lockToken(uint256 tokenId) external {
+    nft.lock(tokenId);
+  }
 }

contracts/tokens/EventPatch.sol

@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.17;
+
+// Authors: Francesco Sullo <[email protected]>
+// (c) Superpower Labs Inc.
+
+import "../SuperpowerNFT.sol";
+
+//import "hardhat/console.sol";
+// Intermediate contract to fix a typo in the Locked and Unlocked events
+
+contract EventPatch is SuperpowerNFT {
+  // Unfortunately, there was a typo in the parameter "tokenId" of the events
+  // Locked and Unlocked, and it was spelled as "tokendId".
+  // This patch fixes the typo and emits new events with the correct parameter name.
+
+  error DefaultLockedAlreadyEmitted();
+  error NewLockedAlreadyEmitted();
+
+  bool public defaultLockedEmitted;
+  uint256 public lastCheckedId;
+  bool public allNewLockedEmitted;
+
+  function emitNewLockedEvent() public {
+    if (!defaultLockedEmitted) {
+      emit DefaultLocked(true);
+      defaultLockedEmitted = true;
+    }
+    if (allNewLockedEmitted) {
+      revert NewLockedAlreadyEmitted();
+    }
+    uint256 fromId = lastCheckedId + 1;
+    for (uint256 i = fromId; i < _nextTokenId; i++) {
+      if (locked(i)) {
+        emit Locked(i, true);
+      }
+      if (gasleft() < 30000 || i == totalSupply()) {
+        lastCheckedId = i;
+        if (i == totalSupply()) {
+          allNewLockedEmitted = true;
+        }
+        break;
+      }
+    }
+  }
+
+  uint256[49] private __gap;
+}