Merge pull request #3 from BoundNFT/feature/ReentrancyGuard

Add ReentrancyGuard modifier

.gitignore

@@ -36,3 +36,4 @@ node_modules
 #Hardhat files
 cache
 artifacts
+storage_layout

.solhint.json

@@ -1,7 +1,9 @@
 {
   "extends": "solhint:recommended",
   "rules": {
-    "compiler-version": ["error", "^0.8.0"],
-    "func-visibility": ["warn", { "ignoreConstructors": true }]
+    "compiler-version": ["error", "0.8.4"],
+    "func-visibility": ["warn", { "ignoreConstructors": true }],
+    "indent": ["warning", 2],
+    "mark-callable-contracts": [false]
   }
 }

abis/BNFT.json

@@ -415,6 +415,19 @@
     "stateMutability": "nonpayable",
     "type": "function"
   },
+  {
+    "inputs": [],
+    "name": "contractURI",
+    "outputs": [
+      {
+        "internalType": "string",
+        "name": "",
+        "type": "string"
+      }
+    ],
+    "stateMutability": "view",
+    "type": "function"
+  },
   {
     "inputs": [
       {

abis/BNFTRegistry.json

@@ -61,6 +61,25 @@
     "name": "BNFTUpgraded",
     "type": "event"
   },
+  {
+    "anonymous": false,
+    "inputs": [
+      {
+        "indexed": false,
+        "internalType": "address[]",
+        "name": "nftAssets",
+        "type": "address[]"
+      },
+      {
+        "indexed": false,
+        "internalType": "string[]",
+        "name": "symbols",
+        "type": "string[]"
+      }
+    ],
+    "name": "CustomeSymbolsAdded",
+    "type": "event"
+  },
   {
     "anonymous": false,
     "inputs": [

contracts/interfaces/IBNFT.sol

@@ -132,4 +132,9 @@ interface IBNFT {
    * @dev Returns the address of the underlying asset.
    */
   function underlyingAsset() external view returns (address);
+
+  /**
+   * @dev Returns the contract-level metadata.
+   */
+  function contractURI() external view returns (string memory);
 }

contracts/protocol/BNFT.sol

@@ -4,6 +4,7 @@ pragma solidity 0.8.4;
 import {IBNFT} from "../interfaces/IBNFT.sol";
 import {IFlashLoanReceiver} from "../interfaces/IFlashLoanReceiver.sol";
 
+import {StringsUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/StringsUpgradeable.sol";
 import {AddressUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
 import {ERC721Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
 import {ERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
@@ -23,6 +24,30 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
   // Mapping from token ID to minter address
   mapping(uint256 => address) private _minters;
   address private _owner;
+  uint256 private constant _NOT_ENTERED = 0;
+  uint256 private constant _ENTERED = 1;
+  uint256 private _status;
+
+  /**
+   * @dev Prevents a contract from calling itself, directly or indirectly.
+   * Calling a `nonReentrant` function from another `nonReentrant`
+   * function is not supported. It is possible to prevent this from happening
+   * by making the `nonReentrant` function external, and making it call a
+   * `private` function that does the actual work.
+   */
+  modifier nonReentrant() {
+    // On the first call to nonReentrant, _notEntered will be true
+    require(_status != _ENTERED, "ReentrancyGuard: reentrant call");
+
+    // Any calls to nonReentrant after this point will fail
+    _status = _ENTERED;
+
+    _;
+
+    // By storing the original value once again, a refund is triggered (see
+    // https://eips.ethereum.org/EIPS/eip-2200)
+    _status = _NOT_ENTERED;
+  }
 
   /**
    * @dev Initializes the bNFT
@@ -97,7 +122,7 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
    * @param to The owner address receive the bNFT token
    * @param tokenId token id of the underlying asset of NFT
    **/
-  function mint(address to, uint256 tokenId) external override {
+  function mint(address to, uint256 tokenId) external override nonReentrant {
     require(AddressUpgradeable.isContract(_msgSender()), "BNFT: caller is not contract");
     require(!_exists(tokenId), "BNFT: exist token");
     require(IERC721Upgradeable(_underlyingAsset).ownerOf(tokenId) == _msgSender(), "BNFT: caller is not owner");
@@ -121,7 +146,7 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
    *
    * @param tokenId token id of the underlying asset of NFT
    **/
-  function burn(uint256 tokenId) external override {
+  function burn(uint256 tokenId) external override nonReentrant {
     require(AddressUpgradeable.isContract(_msgSender()), "BNFT: caller is not contract");
     require(_exists(tokenId), "BNFT: nonexist token");
     require(_minters[tokenId] == _msgSender(), "BNFT: caller is not minter");
@@ -144,7 +169,7 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
     address receiverAddress,
     uint256[] calldata nftTokenIds,
     bytes calldata params
-  ) external override {
+  ) external override nonReentrant {
     uint256 i;
     IFlashLoanReceiver receiver = IFlashLoanReceiver(receiverAddress);
 
@@ -184,12 +209,21 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
     return IERC721MetadataUpgradeable(_underlyingAsset).tokenURI(tokenId);
   }
 
+  /**
+   * @dev See {IBNFT-contractURI}.
+   */
+  function contractURI() external view override returns (string memory) {
+    string memory hexAddress = StringsUpgradeable.toHexString(uint256(uint160(address(this))), 20);
+    return string(abi.encodePacked("https://metadata.benddao.xyz/", hexAddress));
+  }
+
   function claimERC20Airdrop(
     address token,
     address to,
     uint256 amount
-  ) external override onlyOwner {
+  ) external override nonReentrant onlyOwner {
     require(token != _underlyingAsset, "BNFT: token can not be underlying asset");
+    require(token != address(this), "BNFT: token can not be self address");
     IERC20Upgradeable(token).transfer(to, amount);
     emit ClaimERC20Airdrop(token, to, amount);
   }
@@ -198,8 +232,9 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
     address token,
     address to,
     uint256[] calldata ids
-  ) external override onlyOwner {
+  ) external override nonReentrant onlyOwner {
     require(token != _underlyingAsset, "BNFT: token can not be underlying asset");
+    require(token != address(this), "BNFT: token can not be self address");
     for (uint256 i = 0; i < ids.length; i++) {
       IERC721Upgradeable(token).safeTransferFrom(address(this), to, ids[i]);
     }
@@ -212,8 +247,9 @@ contract BNFT is IBNFT, ERC721EnumerableUpgradeable, IERC721ReceiverUpgradeable,
     uint256[] calldata ids,
     uint256[] calldata amounts,
     bytes calldata data
-  ) external override onlyOwner {
+  ) external override nonReentrant onlyOwner {
     require(token != _underlyingAsset, "BNFT: token can not be underlying asset");
+    require(token != address(this), "BNFT: token can not be self address");
     IERC1155Upgradeable(token).safeBatchTransferFrom(address(this), to, ids, amounts, data);
     emit ClaimERC1155Airdrop(token, to, ids, amounts, data);
   }

contracts/protocol/BNFTRegistry.sol

@@ -18,6 +18,30 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
   string public symbolPrefix;
   address public bNftGenericImpl;
   mapping(address => string) public customSymbols;
+  uint256 private constant _NOT_ENTERED = 0;
+  uint256 private constant _ENTERED = 1;
+  uint256 private _status;
+
+  /**
+   * @dev Prevents a contract from calling itself, directly or indirectly.
+   * Calling a `nonReentrant` function from another `nonReentrant`
+   * function is not supported. It is possible to prevent this from happening
+   * by making the `nonReentrant` function external, and making it call a
+   * `private` function that does the actual work.
+   */
+  modifier nonReentrant() {
+    // On the first call to nonReentrant, _notEntered will be true
+    require(_status != _ENTERED, "ReentrancyGuard: reentrant call");
+
+    // Any calls to nonReentrant after this point will fail
+    _status = _ENTERED;
+
+    _;
+
+    // By storing the original value once again, a refund is triggered (see
+    // https://eips.ethereum.org/EIPS/eip-2200)
+    _status = _NOT_ENTERED;
+  }
 
   function getBNFTAddresses(address nftAsset) external view override returns (address bNftProxy, address bNftImpl) {
     bNftProxy = bNftProxys[nftAsset];
@@ -58,7 +82,7 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
   /**
    * @dev See {IBNFTRegistry-createBNFT}.
    */
-  function createBNFT(address nftAsset) external override returns (address bNftProxy) {
+  function createBNFT(address nftAsset) external override nonReentrant returns (address bNftProxy) {
     _requireAddressIsERC721(nftAsset);
     require(bNftProxys[nftAsset] == address(0), "BNFTR: asset exist");
     require(bNftGenericImpl != address(0), "BNFTR: impl is zero address");
@@ -71,7 +95,7 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
   /**
    * @dev See {IBNFTRegistry-setBNFTGenericImpl}.
    */
-  function setBNFTGenericImpl(address genericImpl) external override onlyOwner {
+  function setBNFTGenericImpl(address genericImpl) external override nonReentrant onlyOwner {
     require(genericImpl != address(0), "BNFTR: impl is zero address");
     bNftGenericImpl = genericImpl;
 
@@ -84,6 +108,7 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
   function createBNFTWithImpl(address nftAsset, address bNftImpl)
     external
     override
+    nonReentrant
     onlyOwner
     returns (address bNftProxy)
   {
@@ -103,7 +128,7 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
     address nftAsset,
     address bNftImpl,
     bytes memory encodedCallData
-  ) external override onlyOwner {
+  ) external override nonReentrant onlyOwner {
     address bNftProxy = bNftProxys[nftAsset];
     require(bNftProxy != address(0), "BNFTR: asset nonexist");
 
@@ -123,7 +148,12 @@ contract BNFTRegistry is IBNFTRegistry, Initializable, OwnableUpgradeable {
   /**
    * @dev See {IBNFTRegistry-addCustomeSymbols}.
    */
-  function addCustomeSymbols(address[] memory nftAssets_, string[] memory symbols_) external override onlyOwner {
+  function addCustomeSymbols(address[] memory nftAssets_, string[] memory symbols_)
+    external
+    override
+    nonReentrant
+    onlyOwner
+  {
     require(nftAssets_.length == symbols_.length, "BNFTR: inconsistent parameters");
 
     for (uint256 i = 0; i < nftAssets_.length; i++) {

deployments/deployed-contracts-rinkeby.json

@@ -15,11 +15,11 @@
     "address": "0x8601bCbDC41a6Fe6422764EF1Dc8ccb7fa8B891B"
   },
   "BNFT": {
-    "address": "0xe4C064ffdc15cA42Bb2D5c4aF8D7a2A49F63d87E",
+    "address": "0x3296caB3cf58100a663A330DD79753888810907d",
     "deployer": "0xafF5C36642385b6c7Aaf7585eC785aB2316b5db6"
   },
   "BNFTRegistryImpl": {
-    "address": "0x9919603B006d34DAF2E16C55694D2bfC5594b6F6"
+    "address": "0x7Ac8723ED7FcC7476EF0a94B3475A96e70A79301"
   },
   "BNFTRegistry": {
     "address": "0xB873F088EB721261bc88BbC739B5C794e02e414b"
@@ -29,7 +29,7 @@
     "deployer": "0xafF5C36642385b6c7Aaf7585eC785aB2316b5db6"
   },
   "AirdropFlashLoanReceiver": {
-    "address": "0x07AB34B4cd19B6Fb2eFA86AD351ae095D2566258",
+    "address": "0x4F3b1073eBF56DB48095A4190286c8288dE865B4",
     "deployer": "0xafF5C36642385b6c7Aaf7585eC785aB2316b5db6"
   },
   "DOODLE": {

hardhat.config.ts

@@ -22,6 +22,8 @@ import "@nomiclabs/hardhat-etherscan";
 import "hardhat-gas-reporter";
 import "solidity-coverage";
 import { fork } from "child_process";
+import "@nomiclabs/hardhat-solhint";
+require('hardhat-storage-layout-diff');
 
 const SKIP_LOAD = process.env.SKIP_LOAD === "true";
 const DEFAULT_BLOCK_GAS_LIMIT = 12450000;

helpers/contracts-getters.ts

@@ -22,11 +22,11 @@ export const getSecondSigner = async () => (await getEthersSigners())[1];
 
 export const getThirdSigner = async () => (await getEthersSigners())[2];
 
-export const getDeploySigner = async () => (await getEthersSigners())[1];
+export const getDeploySigner = async () => (await getEthersSigners())[0];
 
-export const getPoolOwnerSigner = async () => (await getEthersSigners())[1];
+export const getPoolOwnerSigner = async () => (await getEthersSigners())[0];
 
-export const getProxyAdminSigner = async () => (await getEthersSigners())[3];
+export const getProxyAdminSigner = async () => (await getEthersSigners())[2];
 
 export const getBNFTRegistryProxy = async (address?: tEthereumAddress) => {
   return await BNFTRegistryFactory.connect(