Merge pull request #12 from niftyhorde/mpetrun5/fix-cancel-swap-permi…

…ssions

Fix cancel swap permissions

.env.sample

@@ -1,5 +1,6 @@
 INFURA_PROJECT_ID="your infura project id"
 GOERLI_MNEMONIC="your goerli mnemonic"
+ROPSTEN_MNEMONIC="your ropsten mnemonic"
 MAINNET_MNEMONIC="your mainnet mnemonic"
 COINMARKETCAP_API_KEY="your coinmarketcap api key"
 REPORT_GAS=false

.github/workflows/publish-contracts.yaml

@@ -54,6 +54,11 @@ jobs:
 
       - name: Deploy local
         run: yarn run deploy:local
+      - name: Deploy ropsten
+        run: yarn run deploy:ropsten
+        env:
+          INFURA_PROJECT_ID: ${{ secrets.INFURA_PROJECT_ID }}
+          ROPSTEN_MNEMONIC: ${{ secrets.ROPSTEN_MNEMONIC }}
       - name: Deploy goerli
         run: yarn run deploy:goerli
         env:

contracts/SwapKiwi.sol

@@ -158,9 +158,14 @@ contract SwapKiwi is Ownable, IERC721Receiver {
     * @dev Returns NFTs from SwapKiwi to swap initator.
     *      Callable only if second user hasn't yet added NFTs.
     *
-    * @param swapId ID of the swap that the second user wants to cancel
+    * @param swapId ID of the swap that the swap participants want to cancel
     */
   function cancelSwap(uint256 swapId) public {
+    require(
+      _swaps[swapId].initiator == msg.sender || _swaps[swapId].secondUser == msg.sender,
+      "SwapKiwi: Can't cancel swap, must be swap participant"
+    );
+
     require(_swaps[swapId].secondUserNftAddresses.length == 0,
       "SwapKiwi: Can't cancel swap after other user added NFTs");
 

hardhat.config.ts

@@ -51,6 +51,17 @@ const config: HardhatUserConfig = {
       gasPrice: "auto",
       gasMultiplier: 1.5
     },
+    ropsten: {
+      url: `https://ropsten.infura.io/v3/${process.env.INFURA_PROJECT_ID}`,
+      accounts: {
+        mnemonic: `${process.env.ROPSTEN_MNEMONIC}`
+      },
+      chainId: 3,
+      loggingEnabled: true,
+      gas: "auto",
+      gasPrice: "auto",
+      gasMultiplier: 1.5
+    },
     mainnet: {
       url: `https://mainnet.infura.io/v3/${process.env.INFURA_PROJECT_ID}`,
       accounts: {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@niftyhorde/swap-kiwi-contracts",
-  "version": "0.2.0-beta",
+  "version": "0.2.1-beta",
   "private": false,
   "repository": {
     "url": "[email protected]:niftyhorde/swap.kiwi.git",
@@ -30,10 +30,11 @@
     "deploy:localhost": "./wait-for-it.sh localhost:8545 -- sleep 1 && hardhat deploy --network localhost",
     "deploy:local": "concurrently --raw --kill-others --success first \"hardhat node\" \"yarn deploy:localhost\"",
     "deploy:goerli": "hardhat deploy --network goerli",
+    "deploy:ropsten": "hardhat deploy --network ropsten",
     "help": "npx hardhat help",
     "node:local": "npx hardhat node",
     "script": "hardhat run",
-    "test": "hardhat test --network hardhatevm",
+    "test": "hardhat test",
     "test:localhost": "./wait-for-it.sh localhost:8545 -- hardhat test --network localhost",
     "test:ci": "yarn run test:gas && yarn run test:coverage",
     "test:gas": "REPORT_GAS=true concurrently --raw --kill-others --success first \"hardhat node\" \"yarn test:localhost\"",

test/swapKiwi.test.ts

@@ -13,20 +13,27 @@ describe("Escrow", async function () {
   let SwapKiwi: Deployment;
   let swapKiwi: SwapKiwi;
   let TestNFT: Deployment;
-  let testNFT: TestNFT;
+  let appUserNFT: TestNFT;
+  let otherAppUserNFT: TestNFT;
   let signers: Signer[];
   let appUser: SwapKiwi;
   let otherAppUser: SwapKiwi;
+  let appUserAddress: string;
+  let otherAppUserAddress: string;
   const VALID_APP_FEE = ethers.utils.parseEther("0.1");
 
   before(async () => {
     signers = await ethers.getSigners();
     ({ SwapKiwi, TestNFT } = await deployments.fixture());
     swapKiwi = await ethers.getContractAt(SwapKiwi.abi, SwapKiwi.address, signers[0]) as SwapKiwi;
-    testNFT = await ethers.getContractAt(TestNFT.abi, TestNFT.address, signers[1]) as TestNFT;
 
-    appUser = new ethers.Contract(swapKiwi.address, SwapKiwi.abi, signers[1]) as SwapKiwi;
-    otherAppUser = new ethers.Contract(swapKiwi.address, SwapKiwi.abi, signers[1]) as SwapKiwi;
+    appUserNFT = await ethers.getContractAt(TestNFT.abi, TestNFT.address, signers[2]) as TestNFT;
+    otherAppUserNFT = await ethers.getContractAt(TestNFT.abi, TestNFT.address, signers[3]) as TestNFT;
+
+    appUser = new ethers.Contract(swapKiwi.address, SwapKiwi.abi, signers[2]) as SwapKiwi;
+    otherAppUser = new ethers.Contract(swapKiwi.address, SwapKiwi.abi, signers[3]) as SwapKiwi;
+    appUserAddress = await signers[2].getAddress();
+    otherAppUserAddress = await signers[3].getAddress();
   });
 
   function getFilterName(eventName: string) {
@@ -80,85 +87,123 @@ describe("Escrow", async function () {
   });
 
   it('Should succesfully deposit NFT into escrow contract and emit "SwapProposed" event', async function () {
-    await testNFT.mint(await signers[1].getAddress(), 25);
-    await testNFT.approve(swapKiwi.address, 25);
-    expect(await testNFT.ownerOf(25)).to.be.deep.equal(await signers[1].getAddress());
+    await appUserNFT.mint(appUserAddress, 25);
+    await appUserNFT.approve(swapKiwi.address, 25);
+    expect(await appUserNFT.ownerOf(25)).to.be.deep.equal(appUserAddress);
 
-    const tx = await appUser.proposeSwap(otherAppUser.address, [testNFT.address], [25], {
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [25], {
       value: VALID_APP_FEE
     });
     const txReceipt = await tx.wait(1);
     const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
 
     // check if all values are emitted in event
     expect(logs[0].eventName).to.be.deep.equal("SwapProposed");
-    expect(logs[0].args.from).to.be.deep.equal(await signers[1].getAddress());
-    expect(logs[0].args.to).to.be.deep.equal(swapKiwi.address);
-    expect(logs[0].args.nftAddresses[0]).to.be.deep.equal(testNFT.address);
+    expect(logs[0].args.from).to.be.deep.equal(appUserAddress);
+    expect(logs[0].args.to).to.be.deep.equal(otherAppUserAddress);
+    expect(logs[0].args.nftAddresses[0]).to.be.deep.equal(appUserNFT.address);
     expect(logs[0].args.nftIds[0].toString()).to.be.deep.equal("25");
-    expect(await testNFT.ownerOf(25)).to.be.deep.equal(swapKiwi.address);
+    expect(await appUserNFT.ownerOf(25)).to.be.deep.equal(swapKiwi.address);
   });
 
-  it('Should succesfully cancel swap by second user and emit "SwapCanceled" event', async function () {
-    await testNFT.mint(await signers[1].getAddress(), 60);
-    await testNFT.approve(swapKiwi.address, 60);
-    const tx = await appUser.proposeSwap(await signers[1].getAddress(), [testNFT.address], [60], {
+  it('Should succesfully cancel swap by first user and emit "SwapCanceled" event', async function () {
+    await appUserNFT.mint(appUserAddress, 140);
+    await appUserNFT.approve(swapKiwi.address, 140);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [140], {
       value: VALID_APP_FEE
     });
     const txReceipt = await tx.wait(1);
     const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
     const swapIdFromLogs = Number(logs[logs.length - 2].args.swapId.toString());
 
-    const cancelTx = await otherAppUser.cancelSwap(swapIdFromLogs);
+    const cancelTx = await appUser.cancelSwap(swapIdFromLogs);
     const cancelTxReceipt = await cancelTx.wait(1);
     const cancelTxlogs = await getEventWithArgsFromLogs(cancelTxReceipt, "SwapCanceled");
 
     // check if all values are emitted in event
     expect(cancelTxlogs[0].eventName).to.be.deep.equal("SwapCanceled");
-    expect(cancelTxlogs[0].args.canceledBy).to.be.deep.equal(await signers[1].getAddress());
+    expect(cancelTxlogs[0].args.canceledBy).to.be.deep.equal(appUserAddress);
     // expect that swap ID from "SwapCanceled" is same as swap ID from "swapProposed" event
     expect(cancelTxlogs[0].args.swapId.toString()).to.be.deep.equal(String(swapIdFromLogs));
   });
 
-  it("Should fail to cancel swap if second user has deposited his NFTs", async function () {
+  it('Should succesfully cancel swap by second user and emit "SwapCanceled" event', async function () {
+    await appUserNFT.mint(appUserAddress, 120);
+    await appUserNFT.approve(swapKiwi.address, 120);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [120], {
+      value: VALID_APP_FEE
+    });
+    const txReceipt = await tx.wait(1);
+    const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
+    const swapIdFromLogs = Number(logs[logs.length - 2].args.swapId.toString());
+
+    const cancelTx = await otherAppUser.cancelSwap(swapIdFromLogs);
+    const cancelTxReceipt = await cancelTx.wait(1);
+    const cancelTxlogs = await getEventWithArgsFromLogs(cancelTxReceipt, "SwapCanceled");
+
+    // check if all values are emitted in event
+    expect(cancelTxlogs[1].eventName).to.be.deep.equal("SwapCanceled");
+    expect(cancelTxlogs[1].args.canceledBy).to.be.deep.equal(otherAppUserAddress);
+    // expect that swap ID from "SwapCanceled" is same as swap ID from "swapProposed" event
+    expect(cancelTxlogs[1].args.swapId.toString()).to.be.deep.equal(String(swapIdFromLogs));
+  });
+
+  it("Should fail to cancel swap if second user already added NFTs", async function () {
     // first user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 70);
-    await testNFT.approve(swapKiwi.address, 70);
-    const tx = await appUser.proposeSwap(await signers[1].getAddress(), [testNFT.address], [70], {
+    await appUserNFT.mint(appUserAddress, 160);
+    await appUserNFT.approve(swapKiwi.address, 160);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [160], {
       value: VALID_APP_FEE
     });
     const txReceipt = await tx.wait(1);
     const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
     const swapIdFromLogs = Number(logs[logs.length - 1].args.swapId.toString());
 
     // second user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 71);
-    await testNFT.approve(swapKiwi.address, 71);
-    await otherAppUser.initiateSwap(swapIdFromLogs, [testNFT.address], [71], {
+    await otherAppUserNFT.mint(otherAppUserAddress, 71);
+    await otherAppUserNFT.approve(swapKiwi.address, 71);
+    await otherAppUser.initiateSwap(swapIdFromLogs, [otherAppUserNFT.address], [71], {
       value: VALID_APP_FEE
     });
     // check that second player NFT is deposited into SwapKiwi
-    expect(await testNFT.ownerOf(71)).to.be.deep.equal(swapKiwi.address);
+    expect(await otherAppUserNFT.ownerOf(71)).to.be.deep.equal(swapKiwi.address);
 
     await expect(otherAppUser.cancelSwap(swapIdFromLogs)).to.be.rejectedWith(
       "VM Exception while processing transaction: revert SwapKiwi: Can't cancel swap after other user added NFTs");
   });
 
+  it("Should fail to cancel swap if user is not a swap participant", async function () {
+    const nonSwapParticipant = new ethers.Contract(SwapKiwi.address, SwapKiwi.abi, signers[6]) as SwapKiwi;
+
+    // first user NFT minting and swap deposit into SwapKiwi
+    await appUserNFT.mint(appUserAddress, 70);
+    await appUserNFT.approve(swapKiwi.address, 70);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [70], {
+      value: VALID_APP_FEE
+    });
+    const txReceipt = await tx.wait(1);
+    const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
+    const swapIdFromLogs = Number(logs[logs.length - 1].args.swapId.toString());
+
+    await expect(nonSwapParticipant.cancelSwap(swapIdFromLogs)).to.be.rejectedWith(
+      "VM Exception while processing transaction: revert SwapKiwi: Can't cancel swap, must be swap participant");
+  });
+
   it('Should succesfully reject swap by swap initiator and emit "swapRejected" event', async function () {
     // first user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 80);
-    await testNFT.approve(swapKiwi.address, 80);
-    const tx = await appUser.proposeSwap(await signers[1].getAddress(), [testNFT.address], [80], {
+    await appUserNFT.mint(appUserAddress, 80);
+    await appUserNFT.approve(swapKiwi.address, 80);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address], [80], {
       value: VALID_APP_FEE
     });
     const txReceipt = await tx.wait(1);
     const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
     const swapIdFromLogs = Number(logs[logs.length - 1].args.swapId.toString());
 
     // second user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 81);
-    await testNFT.approve(swapKiwi.address, 81);
-    await otherAppUser.initiateSwap(swapIdFromLogs, [testNFT.address], [81], {
+    await otherAppUserNFT.mint(otherAppUserAddress, 81);
+    await otherAppUserNFT.approve(swapKiwi.address, 81);
+    await otherAppUser.initiateSwap(swapIdFromLogs, [otherAppUserNFT.address], [81], {
       value: VALID_APP_FEE
     });
 
@@ -168,7 +213,7 @@ describe("Escrow", async function () {
 
     // check if all values are emitted in event
     expect(rejectSwapLogs[0].eventName).to.be.deep.equal("SwapRejected");
-    expect(rejectSwapLogs[0].args.rejectedBy).to.be.deep.equal(await signers[1].getAddress());
+    expect(rejectSwapLogs[0].args.rejectedBy).to.be.deep.equal(appUserAddress);
     // expect that swap ID from "SwapRejected" is same as swap ID from "swapProposed" event
     expect(rejectSwapLogs[0].args.swapId.toString()).to.be.deep.equal(String(swapIdFromLogs));
   });
@@ -182,59 +227,65 @@ describe("Escrow", async function () {
 
   it('Should successfully execute swap, transfer NFTs from SwapKiwi to new users and emit "swapExecutedEvent" and "swapInitiated" events', async function () {
     // first user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 85);
-    await testNFT.mint(await signers[1].getAddress(), 86);
-    await testNFT.approve(swapKiwi.address, 85);
-    await testNFT.approve(swapKiwi.address, 86);
-    const tx = await appUser.proposeSwap(await signers[1].getAddress(), [testNFT.address, testNFT.address], [85, 86], {
+    await appUserNFT.mint(appUserAddress, 85);
+    await appUserNFT.mint(appUserAddress, 86);
+    await appUserNFT.approve(swapKiwi.address, 85);
+    await appUserNFT.approve(swapKiwi.address, 86);
+    const tx = await appUser.proposeSwap(otherAppUserAddress, [appUserNFT.address, appUserNFT.address], [85, 86], {
       value: VALID_APP_FEE
     });
     const txReceipt = await tx.wait(1);
     const logs = await getEventWithArgsFromLogs(txReceipt, "SwapProposed");
     const swapIdFromLogs = Number(logs[logs.length - 1].args.swapId.toString());
 
     // check that first user NFTs are deposited into SwapKiwi
-    expect(await testNFT.ownerOf(85)).to.be.deep.equal(swapKiwi.address);
-    expect(await testNFT.ownerOf(86)).to.be.deep.equal(swapKiwi.address);
+    expect(await appUserNFT.ownerOf(85)).to.be.deep.equal(swapKiwi.address);
+    expect(await appUserNFT.ownerOf(86)).to.be.deep.equal(swapKiwi.address);
 
     // second user NFT minting and swap deposit into SwapKiwi
-    await testNFT.mint(await signers[1].getAddress(), 87);
-    await testNFT.mint(await signers[1].getAddress(), 88);
-    await testNFT.approve(swapKiwi.address, 87);
-    await testNFT.approve(swapKiwi.address, 88);
-    const initiateSwapTx = await otherAppUser.initiateSwap(swapIdFromLogs, [testNFT.address, testNFT.address], [87, 88], {
-      value: VALID_APP_FEE
-    });
+    await otherAppUserNFT.mint(otherAppUserAddress, 87);
+    await otherAppUserNFT.mint(otherAppUserAddress, 88);
+    await otherAppUserNFT.approve(swapKiwi.address, 87);
+    await otherAppUserNFT.approve(swapKiwi.address, 88);
+    const initiateSwapTx = await otherAppUser.initiateSwap(
+      swapIdFromLogs,
+      [otherAppUserNFT.address, otherAppUserNFT.address],
+      [87, 88],
+      {
+        value: VALID_APP_FEE
+      }
+    );
     const initiateSwapTxReceipt = await initiateSwapTx.wait(1);
     const initiateSwapLogs = await getEventWithArgsFromLogs(initiateSwapTxReceipt, "SwapInitiated");
     // check if all values are emitted in "SwapExecuted" event
     expect(initiateSwapLogs[0].eventName).to.be.deep.equal("SwapInitiated");
-    expect(initiateSwapLogs[0].args.from).to.be.deep.equal(await signers[1].getAddress());
-    expect(initiateSwapLogs[0].args.to).to.be.deep.equal(await signers[1].getAddress());
+    expect(initiateSwapLogs[0].args.from).to.be.deep.equal(otherAppUserAddress);
+    expect(initiateSwapLogs[0].args.to).to.be.deep.equal(appUserAddress);
 
     // check that second user NFTs are deposited into SwapKiwi
-    expect(await testNFT.ownerOf(87)).to.be.deep.equal(swapKiwi.address);
-    expect(await testNFT.ownerOf(88)).to.be.deep.equal(swapKiwi.address);
+    expect(await otherAppUserNFT.ownerOf(87)).to.be.deep.equal(swapKiwi.address);
+    expect(await otherAppUserNFT.ownerOf(88)).to.be.deep.equal(swapKiwi.address);
 
     const acceptSwapTx = await appUser.acceptSwap(swapIdFromLogs);
     const acceptSwapTxReceipt = await acceptSwapTx.wait(1);
     const acceptSwapLogs = await getEventWithArgsFromLogs(acceptSwapTxReceipt, "SwapExecuted");
 
     // check if all values are emitted in "SwapExecuted" event
     expect(acceptSwapLogs[0].eventName).to.be.deep.equal("SwapExecuted");
-    expect(acceptSwapLogs[0].args.from).to.be.deep.equal(await signers[1].getAddress());
-    expect(acceptSwapLogs[0].args.to).to.be.deep.equal(await signers[1].getAddress());
+    expect(acceptSwapLogs[0].args.from).to.be.deep.equal(appUserAddress);
+    expect(acceptSwapLogs[0].args.to).to.be.deep.equal(otherAppUserAddress);
     // check that NFTs are transfered from SwapKiwi to participants - same address because both have same signer
-    expect(await testNFT.ownerOf(85)).to.be.deep.equal(await signers[1].getAddress());
-    expect(await testNFT.ownerOf(86)).to.be.deep.equal(await signers[1].getAddress());
-    expect(await testNFT.ownerOf(87)).to.be.deep.equal(await signers[1].getAddress());
-    expect(await testNFT.ownerOf(88)).to.be.deep.equal(await signers[1].getAddress());
+
+    expect(await otherAppUserNFT.ownerOf(85)).to.be.deep.equal(otherAppUserAddress);
+    expect(await otherAppUserNFT.ownerOf(86)).to.be.deep.equal(otherAppUserAddress);
+    expect(await appUserNFT.ownerOf(87)).to.be.deep.equal(appUserAddress);
+    expect(await appUserNFT.ownerOf(88)).to.be.deep.equal(appUserAddress);
   });
 
   it("Should successful withdraw collected fees from SwapKiwi if called by owner", async function () {
-    await swapKiwi.withdrawEther(testNFT.address, ethers.utils.parseEther("0.1"));
+    await swapKiwi.withdrawEther(appUserNFT.address, ethers.utils.parseEther("0.1"));
 
-    expect((await ethers.provider.getBalance(testNFT.address)).toString())
+    expect((await ethers.provider.getBalance(appUserNFT.address)).toString())
       .to.be.deep.equal(ethers.utils.parseEther("0.1").toString());
   });