Merge pull request #306 from nevermined-io/feat/more-tests

Adding more test coverage

.github/workflows/build.yml

@@ -30,7 +30,7 @@ jobs:
           yarn lint
       - name: Generate circuits
         run: |
-          sh ./scripts/build.sh
+          sh ./scripts/build-circuit.sh
           git diff --exit-code
       - name: Compile
         run: |
@@ -62,7 +62,7 @@ jobs:
         run: |
           yarn lint
           yarn clean
-          sh ./scripts/build.sh
+          sh ./scripts/build-circuit.sh
           yarn compile
       - name: Run Upgrade Tests
         run: |
@@ -88,9 +88,6 @@ jobs:
           yarn install --frozen-lockfile
           git submodule init
           git submodule update
-      - name: Compile and Lint
-        run: |
-          sh ./scripts/build.sh
       - name: Run Upgrade Tests
         run: |
           bash ./scripts/test-upgradeability.sh

.github/workflows/upload-artifacts.yml

@@ -70,7 +70,7 @@ jobs:
         run: |
           yarn lint
           yarn clean
-          sh ./scripts/build.sh
+          sh ./scripts/build-circuit.sh
           yarn compile
 
       # Impersonate AWS role

Dockerfile

@@ -20,7 +20,7 @@ COPY . /nevermined-contracts
 WORKDIR /nevermined-contracts
 
 RUN yarn
-RUN sh ./scripts/build.sh
+RUN sh ./scripts/build-circuit.sh
 
 RUN yarn clean
 RUN yarn compile

contracts/conditions/LockPaymentCondition.sol

@@ -316,23 +316,13 @@ contract LockPaymentCondition is ILockPayment, ReentrancyGuardUpgradeable, Condi
     }
 
    /**
-    * @notice _transferERC20 transfer ERC20 tokens 
+    * @notice _transferERC20Proxy transfer ERC20 tokens 
+    * @param _senderAddress the address to send the tokens from
     * @param _rewardAddress the address to receive the tokens
     * @param _tokenAddress the ERC20 contract address to use during the payment
     * @param _amount token amount to be locked/released
     * @dev Will throw if transfer fails
     */
-    function _transferERC20(
-        address _rewardAddress,
-        address _tokenAddress,
-        uint256 _amount
-    )
-    internal
-    {
-        IERC20Upgradeable token = ERC20Upgradeable(_tokenAddress);
-        token.safeTransferFrom(msg.sender, _rewardAddress, _amount);
-    }
-
     function _transferERC20Proxy(
         address _senderAddress,
         address _rewardAddress,

contracts/conditions/NFTs/TransferNFT721Condition.sol

@@ -304,52 +304,7 @@ contract TransferNFT721Condition is Condition, ITransferNFT, ReentrancyGuardUpgr
     returns (ConditionStoreLibrary.ConditionState)
     {
         require(hasRole(MARKET_ROLE, msg.sender) || erc721.isApprovedForAll(_nftHolder, msg.sender), 'Invalid access role');
-
-        bytes32 _id = generateId(
-            _agreementId,
-            hashValues(_did, _nftHolder, _nftReceiver, _nftAmount, _lockPaymentCondition, address(erc721), _transfer)
-        );
-
-        require(
-            conditionStoreManager.getConditionState(_lockPaymentCondition) == ConditionStoreLibrary.ConditionState.Fulfilled,
-            'LockCondition needs to be Fulfilled'
-        );
-
-        if (_transfer)  {
-            require(
-                _nftAmount == 0 || (_nftAmount == 1 && erc721.ownerOf(uint256(_did)) == msg.sender),
-                'Not enough balance'
-            );
-
-            if (_nftAmount == 1)
-                erc721.safeTransferFrom(
-                    erc721.ownerOf(uint256(_did)), 
-                    _nftReceiver, 
-                    uint256(_did)
-                );
-        }   else {
-            require(
-                didRegistry.isDIDProviderOrOwner(_did, msg.sender), 
-                'Only owner or provider'
-            );
-            erc721.mint(_nftReceiver, uint256(_did));
-        }
-
-        ConditionStoreLibrary.ConditionState state = super.fulfill(
-            _id,
-            ConditionStoreLibrary.ConditionState.Fulfilled
-        );
-
-        emit Fulfilled(
-            _agreementId,
-            _did,
-            _nftReceiver,
-            _nftAmount,
-            _id,
-            address(erc721)
-        );
-
-        return state;
-    }
+        return fulfillInternal(_nftHolder, _agreementId, _did, _nftReceiver, _nftAmount, _lockPaymentCondition, address(erc721), _transfer);
+    }        
 }
 

contracts/conditions/NFTs/TransferNFTCondition.sol

@@ -259,21 +259,6 @@ contract TransferNFTCondition is Condition, ITransferNFT, ReentrancyGuardUpgrade
         return fulfillInternal(msg.sender, _agreementId, _did, _nftReceiver, _nftAmount, _lockPaymentCondition, _nftContractAddress, _transfer);
     }
 
-    function fulfillInternal(
-        address _account,
-        bytes32 _agreementId,
-        bytes32 _did,
-        address _nftReceiver,
-        uint256 _nftAmount,
-        bytes32 _lockPaymentCondition,
-        address _nftContractAddress
-    )
-    internal
-    returns (ConditionStoreLibrary.ConditionState)
-    {
-        return fulfillInternal(_account, _agreementId, _did, _nftReceiver, _nftAmount, _lockPaymentCondition, _nftContractAddress, true);
-    }
-
     function fulfillInternal(
         address _account,
         bytes32 _agreementId,
@@ -353,43 +338,7 @@ contract TransferNFTCondition is Condition, ITransferNFT, ReentrancyGuardUpgrade
     returns (ConditionStoreLibrary.ConditionState)
     {
         require(hasRole(MARKET_ROLE, msg.sender) || erc1155.isApprovedForAll(_nftHolder, msg.sender), 'Invalid access role');
-
-        bytes32 _id = generateId(
-            _agreementId,
-            hashValues(_did, _nftHolder, _nftReceiver, _nftAmount, _lockPaymentCondition)
-        );
-
-        require(
-            conditionStoreManager.getConditionState(_lockPaymentCondition) == ConditionStoreLibrary.ConditionState.Fulfilled,
-            'LockCondition needs to be Fulfilled'
-        );
-
-        if (_transfer)  {
-            require(
-                erc1155.balanceOf(_nftHolder, uint256(_did)) >= _nftAmount,
-                'Not enough balance'
-            );
-
-            erc1155.safeTransferFrom(_nftHolder, _nftReceiver, uint256(_did), _nftAmount, '');
-        }   else {
-            erc1155.mint(_nftReceiver, uint256(_did), _nftAmount, '');
-        }
-
-        ConditionStoreLibrary.ConditionState state = super.fulfill(
-            _id,
-            ConditionStoreLibrary.ConditionState.Fulfilled
-        );
-
-        emit Fulfilled(
-            _agreementId,
-            _did,
-            _nftReceiver,
-            _nftAmount,
-            _id,
-            address(erc1155)
-        );
-
-        return state;
+        return fulfillInternal(_nftHolder, _agreementId, _did, _nftReceiver, _nftAmount, _lockPaymentCondition, address(erc1155), _transfer);
     }    
 
 }

keeper-geth.Dockerfile

@@ -23,7 +23,7 @@ COPY . /nevermined-contracts
 WORKDIR /nevermined-contracts
 
 RUN yarn
-RUN sh ./scripts/build.sh
+RUN sh ./scripts/build-circuit.sh
 
 ENV MNEMONIC="taxi music thumb unique chat sand crew more leg another off lamp"
 ENV DEPLOY_CONTRACTS=true

keeper-polygon-localnet.Dockerfile

@@ -25,7 +25,7 @@ COPY . /nevermined-contracts
 WORKDIR /nevermined-contracts
 
 RUN yarn
-RUN sh ./scripts/build.sh
+RUN sh ./scripts/build-circuit.sh
 
 ENV MNEMONIC="taxi music thumb unique chat sand crew more leg another off lamp"
 ENV DEPLOY_CONTRACTS=true

keeper-spree.Dockerfile

@@ -30,7 +30,7 @@ COPY . /nevermined-contracts
 WORKDIR /nevermined-contracts
 
 RUN yarn
-RUN sh ./scripts/build.sh
+RUN sh ./scripts/build-circuit.sh
 
 ENV MNEMONIC="taxi music thumb unique chat sand crew more leg another off lamp"
 ENV DEPLOY_CONTRACTS=true

scripts/build.sh → scripts/build-circuit.sh

@@ -1,5 +1,7 @@
 #!/bin/sh
 
+rm contracts/verifier.sol
+
 wget https://hermez.s3-eu-west-1.amazonaws.com/powersOfTau28_hez_final_14.ptau
 
 git clone https://github.com/iden3/circom.git

scripts/test-upgradeability.sh

@@ -7,7 +7,7 @@ export TESTNET=true
 
 # rm -rf artifacts/*.external.json deploy-cache.json
 rm -f .openzeppelin/unknown-31337.json
-git checkout $BASE
+git checkout $BASE || exit 1
 yarn
 yarn compile
 
@@ -19,6 +19,7 @@ npx hardhat run ./scripts/deploy/truffle-wrapper/deployContractsWrapper.js --net
 
 git checkout $BRANCH
 yarn
+./scripts/build-circuit.sh
 
 export FAIL=true
 

test/int/nft/NFT721_e2e.Test.js

@@ -17,6 +17,7 @@ const deployConditions = require('../../helpers/deployConditions.js')
 contract('End to End NFT721 Scenarios', (accounts) => {
     const royalties = 10 // 10% of royalties in the secondary market
     const didSeed = testUtils.generateId()
+    const didSeed2 = testUtils.generateId()
     let did
     let agreementId
     const checksum = testUtils.generateId()
@@ -142,7 +143,8 @@ contract('End to End NFT721 Scenarios', (accounts) => {
         getBalance = async (a, b) => getTokenBalance(a, b, checkpoint)
 
         return {
-            didRegistry
+            didRegistry,
+            nft
         }
     }
 
@@ -352,6 +354,8 @@ contract('End to End NFT721 Scenarios', (accounts) => {
                     (await conditionStoreManager.getConditionState(conditionIds[1])).toNumber(),
                     constants.condition.state.fulfilled
                 )
+
+                assert(await accessCondition.checkPermissions(collector1, nftAccessAgreement.did))
             })
         })
 
@@ -469,4 +473,95 @@ contract('End to End NFT721 Scenarios', (accounts) => {
 
         runTests()
     })
+
+    describe('As market I want to be able to transfer nfts and release rewards on behalf of the artist', () => {
+        let nftSalesAgreement
+        let conditionIds
+        it('Artist registers a new artwork and tokenize (via NFT)', async () => {
+            const { didRegistry, nft } = await setupTest()
+
+            did = await didRegistry.hashDID(didSeed2, artist)
+
+            await didRegistry.registerMintableDID721(
+                didSeed2, checksum, [], url, royalties, false, constants.activities.GENERATED, '', { from: artist })
+            await didRegistry.mint721(did, { from: artist })
+            await nft.setApprovalForAll(transferCondition.address, true, { from: artist })
+
+            assert.strictEqual(artist, await nft.ownerOf(did))
+        })
+
+        it('Collector sets an agreement for buying a NFT', async () => {
+            const data = await prepareNFTSaleAgreement({
+                did: did,
+                agreementId: agreementId,
+                _seller: artist,
+                _buyer: collector1
+            })
+            nftSalesAgreement = data.nftSalesAgreement
+            conditionIds = data.conditionIds
+            agreementId = data.agreementId
+
+            // The Collector creates an agreement on-chain for purchasing a specific NFT attached to a DID
+            const result = await nftSalesTemplate.createAgreement(...Object.values(nftSalesAgreement))
+
+            testUtils.assertEmitted(result, 1, 'AgreementCreated')
+        })
+
+        it('Collector locks the payment', async () => {
+            await token.mint(collector1, nftPrice, { from: owner })
+            await token.approve(lockPaymentCondition.address, nftPrice, { from: collector1 })
+            await token.approve(escrowCondition.address, nftPrice, { from: collector1 })
+
+            await lockPaymentCondition.fulfill(agreementId, did, escrowCondition.address, token.address, amounts, receivers, { from: collector1 })
+
+            const { state } = await conditionStoreManager.getCondition(conditionIds[0])
+            assert.strictEqual(state.toNumber(), constants.condition.state.fulfilled)
+            const collector1Balance = await getBalance(token, collector1)
+            assert.strictEqual(collector1Balance, 0)
+        })
+
+        it('The market can check the payment and can transfer the NFT to the collector', async () => {
+            assert.strictEqual(artist, await nft.ownerOf(did))
+
+            await nft.setApprovalForAll(market, true, { from: artist })
+            await transferCondition.fulfillForDelegate(
+                agreementId,
+                did,
+                artist,
+                collector1,
+                numberNFTs,
+                conditionIds[0],
+                true,
+                { from: market }
+            )
+
+            const { state } = await conditionStoreManager.getCondition(conditionIds[1])
+            assert.strictEqual(state.toNumber(), constants.condition.state.fulfilled)
+
+            assert.strictEqual(collector1, await nft.ownerOf(did))
+        })
+
+        it('The market can release the payment to the artist', async () => {
+            await escrowCondition.fulfill(
+                agreementId,
+                did,
+                amounts,
+                receivers,
+                collector1,
+                escrowCondition.address,
+                token.address,
+                conditionIds[0],
+                conditionIds[1],
+                { from: market })
+
+            const { state } = await conditionStoreManager.getCondition(conditionIds[2])
+            assert.strictEqual(state.toNumber(), constants.condition.state.fulfilled)
+
+            assert.strictEqual(await getBalance(token, collector1), 0)
+            assert.strictEqual(await getBalance(token, lockPaymentCondition.address), 0)
+            assert.strictEqual(await getBalance(token, escrowCondition.address), 0)
+            assert.strictEqual(await getBalance(token, receivers[0]), amounts[0])
+            assert.strictEqual(await getBalance(token, receivers[1]), amounts[1])
+        })
+    })
 })