Merge pull request #70 from PaintSwap/allow_arbitrary_nft_withdrawal

Allow Arbitrary NFT withdrawal from Banks

contracts/Clans/Bank.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.20;
 
 import {IERC1155} from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
+import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 import {ERC1155Holder} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
@@ -28,6 +29,7 @@ contract Bank is ERC1155Holder, IBank, Initializable {
   event DepositToken(address from, uint playerId, address token, uint amount);
   event WithdrawToken(address from, uint playerId, address to, uint toPlayerId, address token, uint amount);
   event WithdrawTokens(address from, uint playerId, address[] tos, uint[] toPlayerIds, address token, uint[] amounts);
+  event WithdrawNFT(address from, uint playerId, address to, uint toPlayerId, address nft, uint tokenId, uint amount);
 
   error MaxBankCapacityReached();
   error NotClanAdmin();
@@ -37,6 +39,8 @@ contract Bank is ERC1155Holder, IBank, Initializable {
   error LengthMismatch();
   error ToIsNotOwnerOfPlayer();
   error ReentrancyGuardReentrantCall();
+  error UseWithdrawItemsForNFT();
+  error NFTTypeNotSupported();
 
   uint8 private constant NOT_ENTERED = 1;
   uint8 private constant ENTERED = 2;
@@ -269,6 +273,29 @@ contract Bank is ERC1155Holder, IBank, Initializable {
     emit WithdrawTokens(msg.sender, _playerId, _tos, _toPlayerIds, _token, _amounts);
   }
 
+  function withdrawNFT(
+    uint _playerId,
+    address _to,
+    uint _toPlayerId,
+    address _nft,
+    uint _tokenId,
+    uint _amount
+  ) external isOwnerOfPlayer(_playerId) canWithdraw(_playerId) {
+    if (_nft == address(bankRegistry.itemNFT())) {
+      revert UseWithdrawItemsForNFT();
+    }
+    if (bankRegistry.playerNFT().balanceOf(_to, _toPlayerId) != 1) {
+      revert ToIsNotOwnerOfPlayer();
+    }
+
+    if (!IERC165(_nft).supportsInterface(type(IERC1155).interfaceId)) {
+      revert NFTTypeNotSupported();
+    }
+
+    IERC1155(_nft).safeTransferFrom(address(this), _to, _tokenId, _amount, "");
+    emit WithdrawNFT(msg.sender, _playerId, _to, _toPlayerId, _nft, _tokenId, _amount);
+  }
+
   function _receivedItemUpdateUniqueItems(uint _id, uint _maxCapacity) private {
     if (!uniqueItems[_id]) {
       if (uniqueItemCount >= _maxCapacity) {

contracts/test/MockERC1155.sol

@@ -11,4 +11,8 @@ contract MockERC1155 is ERC1155 {
   function mint(address _to) external {
     _mint(_to, ++currentId, 1, "0x");
   }
+
+  function mintSpecific(address _to, uint256 _id, uint256 _amount) external {
+    _mint(_to, _id, _amount, "0x");
+  }
 }

contracts/test/MockERC721.sol

@@ -13,6 +13,6 @@ contract MockERC721 is ERC721 {
   }
 
   function mint(address _to) external {
-    _mint(_to, ++currentId);
+    _safeMint(_to, ++currentId);
   }
 }

data/abi/Bank.json

@@ -19,6 +19,11 @@
     "name": "MaxBankCapacityReached",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "NFTTypeNotSupported",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "NotClanAdmin",
@@ -39,6 +44,11 @@
     "name": "ToIsNotOwnerOfPlayer",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "UseWithdrawItemsForNFT",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "WithdrawFailed",
@@ -285,6 +295,55 @@
     "name": "WithdrawItemsBulk",
     "type": "event"
   },
+  {
+    "anonymous": false,
+    "inputs": [
+      {
+        "indexed": false,
+        "internalType": "address",
+        "name": "from",
+        "type": "address"
+      },
+      {
+        "indexed": false,
+        "internalType": "uint256",
+        "name": "playerId",
+        "type": "uint256"
+      },
+      {
+        "indexed": false,
+        "internalType": "address",
+        "name": "to",
+        "type": "address"
+      },
+      {
+        "indexed": false,
+        "internalType": "uint256",
+        "name": "toPlayerId",
+        "type": "uint256"
+      },
+      {
+        "indexed": false,
+        "internalType": "address",
+        "name": "nft",
+        "type": "address"
+      },
+      {
+        "indexed": false,
+        "internalType": "uint256",
+        "name": "tokenId",
+        "type": "uint256"
+      },
+      {
+        "indexed": false,
+        "internalType": "uint256",
+        "name": "amount",
+        "type": "uint256"
+      }
+    ],
+    "name": "WithdrawNFT",
+    "type": "event"
+  },
   {
     "anonymous": false,
     "inputs": [
@@ -694,6 +753,44 @@
     "stateMutability": "nonpayable",
     "type": "function"
   },
+  {
+    "inputs": [
+      {
+        "internalType": "uint256",
+        "name": "_playerId",
+        "type": "uint256"
+      },
+      {
+        "internalType": "address",
+        "name": "_to",
+        "type": "address"
+      },
+      {
+        "internalType": "uint256",
+        "name": "_toPlayerId",
+        "type": "uint256"
+      },
+      {
+        "internalType": "address",
+        "name": "_nft",
+        "type": "address"
+      },
+      {
+        "internalType": "uint256",
+        "name": "_tokenId",
+        "type": "uint256"
+      },
+      {
+        "internalType": "uint256",
+        "name": "_amount",
+        "type": "uint256"
+      }
+    ],
+    "name": "withdrawNFT",
+    "outputs": [],
+    "stateMutability": "nonpayable",
+    "type": "function"
+  },
   {
     "inputs": [
       {

test/Clans/Bank.ts

@@ -356,6 +356,69 @@ describe("Bank", function () {
     ).to.be.revertedWithCustomError(bank, "NotClanAdmin");
   });
 
+  it("Should be able to deposit and withdraw other ERC1155 NFTs", async function () {
+    const {
+      clans,
+      playerId,
+      alice,
+      bob,
+      clanName,
+      discord,
+      telegram,
+      twitter,
+      Bank,
+      bankFactory,
+      brush,
+      playerNFT,
+      avatarId,
+    } = await loadFixture(bankFixture);
+
+    const clanId = 1;
+    const clanBankAddress = ethers.utils.getContractAddress({
+      from: bankFactory.address,
+      nonce: clanId,
+    });
+    await clans.connect(alice).createClan(playerId, clanName, discord, telegram, twitter, 2, 1);
+
+    await brush.mint(alice.address, 1000);
+    await brush.connect(alice).approve(clanBankAddress, 1000);
+
+    const bank = await Bank.attach(clanBankAddress);
+
+    const erc1155 = await ethers.deployContract("MockERC1155");
+    const tokenId = 1;
+    await erc1155.mintSpecific(bank.address, tokenId, 1000);
+
+    const bobPlayerId = await createPlayer(playerNFT, avatarId, bob, "bob", true);
+    await expect(bank.connect(alice).withdrawNFT(playerId, bob.address, bobPlayerId, erc1155.address, tokenId, 400))
+      .to.emit(bank, "WithdrawNFT")
+      .withArgs(alice.address, playerId, bob.address, bobPlayerId, erc1155.address, tokenId, 400);
+
+    expect(await erc1155.balanceOf(bob.address, tokenId)).to.eq(400);
+    expect(await erc1155.balanceOf(bank.address, tokenId)).to.eq(600);
+
+    // Cannot send erc721s, so there's nothing to withdraw there
+    const erc721 = await ethers.deployContract("MockERC721");
+    await expect(erc721.mint(bank.address)).to.be.reverted;
+
+    await expect(
+      bank.connect(alice).withdrawNFT(playerId, bob.address, bobPlayerId, erc721.address, tokenId, 1)
+    ).to.be.revertedWithCustomError(bank, "NFTTypeNotSupported");
+
+    await expect(
+      bank.connect(alice).withdrawNFT(playerId, alice.address, bobPlayerId, erc721.address, tokenId, 1)
+    ).to.be.revertedWithCustomError(bank, "ToIsNotOwnerOfPlayer");
+
+    // Cannot withdraw itemNFTs
+    await expect(bank.connect(alice).withdrawNFT(playerId, bob.address, bobPlayerId, erc1155.address, tokenId, 400));
+
+    // Must be at least treasurer to withdraw
+    await clans.connect(alice).changeRank(clanId, playerId, ClanRank.SCOUT, playerId);
+    await expect(
+      bank.connect(alice).withdrawToken(playerId, bob.address, bobPlayerId, brush.address, 250)
+    ).to.be.revertedWithCustomError(bank, "NotClanAdmin");
+  });
+
   describe("Withdraw tokens to many users", function () {
     it("Withdrawer is not owner of player", async function () {
       const {clans, playerId, alice, clanName, discord, telegram, twitter, Bank, bankFactory, brush} =