Merge pull request #517 from nevermined-io/fix/provider-burn

Changes for burning tokens

contracts/interfaces/IExternalRegistry.sol

@@ -20,8 +20,18 @@ interface IExternalRegistry {
     external
     returns (bool success);
 
+
     function getDIDOwner(bytes32 _did)
     external
     view
     returns (address didOwner);
+
+
+    function isDIDProvider(
+        bytes32 _did,
+        address _provider
+    )
+    external
+    view
+    returns (bool);
 }

contracts/token/erc1155/NFT1155SubscriptionUpgradeable.sol

@@ -63,6 +63,7 @@ contract NFT1155SubscriptionUpgradeable is NFT1155Upgradeable {
         require(
             isOperator(_msgSender()) || // Or the DIDRegistry is burning the NFT 
             to == _msgSender() || // Or the NFT owner is _msgSender() 
+            nftRegistry.isDIDProvider(bytes32(id), _msgSender()) || // Or the DID Provider (Node) is burning the NFT
             isApprovedForAll(to, _msgSender()), // Or the _msgSender() is approved
             'ERC1155: caller is not owner nor approved'
         );

contracts/token/erc1155/NFT1155Upgradeable.sol

@@ -154,9 +154,10 @@ contract NFT1155Upgradeable is ERC1155Upgradeable, NFTBase {
 
     function burn(address to, uint256 id, uint256 amount) virtual public {
         require(balanceOf(to, id) >= amount, 'ERC1155: burn amount exceeds balance');
-        require(
+        require(         
             isOperator(_msgSender()) || // Or the DIDRegistry is burning the NFT 
-            to == _msgSender() || // Or the NFT owner is _msgSender() 
+            to == _msgSender() || // Or the NFT owner is _msgSender()
+            nftRegistry.isDIDProvider(bytes32(id), _msgSender()) || // Or the DID Provider (Node) is burning the NFT
             isApprovedForAll(to, _msgSender()), // Or the _msgSender() is approved
             'ERC1155: caller is not owner nor approved'
         );

contracts/token/erc721/NFT721Upgradeable.sol

@@ -139,6 +139,7 @@ contract NFT721Upgradeable is ERC721Upgradeable, NFTBase {
 
         // Update nftSupply
         _counterMinted.increment();
+        // the supply will just become the number of minted even if something was burned before?
         _nftAttributes[tokenId].nftSupply = _counterMinted.current();
         _nftAttributes[tokenId].nftURI = '';
 
@@ -178,8 +179,8 @@ contract NFT721Upgradeable is ERC721Upgradeable, NFTBase {
     public 
     {
         require(
-            isOperator(_msgSender()) || // Or the DIDRegistry is burning the NFT 
-            balanceOf(_msgSender()) > 0, // Or the _msgSender() is owner and have balance
+            isOperator(_msgSender()) || // The DIDRegistry is burning the NFT 
+            ownerOf(tokenId) == _msgSender(), // Or the _msgSender() is owner of the tokenId
             'ERC721: caller is not owner or not have balance'
         );
         // Update nftSupply

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@nevermined-io/contracts",
-  "version": "3.5.3",
+  "version": "3.5.4",
   "description": "Nevermined implementation of Nevermined in Solidity",
   "bugs": {
     "url": "https://github.com/nevermined-io/contracts/issues"

test/unit/token/NFT1155Subscription.Test.js

@@ -59,6 +59,58 @@ contract('NFT1155 Subscription', (accounts) => {
         await config.grantNVMOperatorRole(minter, { from: owner })
     }
 
+    describe('Providers can burn', () => {
+        const initialAmount = 10
+        let tokenId
+
+        it('As a minter can register a DID without providers', async () => {
+            await setupTest()
+            const didSeed = testUtils.generateId()
+
+            tokenId = await didRegistry.hashDID(didSeed, minter)
+            await didRegistry.methods[
+                'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)'
+            ](didSeed, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter })
+
+            await nft.methods[
+                'mint(address,uint256,uint256,uint256,bytes)'
+            ](account1, tokenId, initialAmount, 0, data, { from: minter })
+
+            const balance = new BigNumber(await nft.balanceOf(account1, tokenId))
+            assert.strictEqual(balance.toNumber(), initialAmount)
+        })
+
+        it('NFT holder can burn', async () => {
+            await nft.methods[
+                'burn(address,uint256,uint256)'
+            ](account1, tokenId, 1, { from: account1 })
+
+            const balance = new BigNumber(await nft.balanceOf(account1, tokenId))
+            assert.strictEqual(balance.toNumber(), initialAmount - 1)
+        })
+
+        it('Account can not burn unless is a provider', async () => {
+            await assert.isRejected(
+                nft.methods[
+                    'burn(address,uint256,uint256)'
+                ](account1, tokenId, 1, { from: account2 }),
+                'ERC1155: caller is not owner nor approved'
+            )
+
+            let balance = new BigNumber(await nft.balanceOf(account1, tokenId))
+            assert.strictEqual(balance.toNumber(), initialAmount - 1)
+
+            await didRegistry.addDIDProvider(tokenId, account2, { from: minter })
+
+            await nft.methods[
+                'burn(address,uint256,uint256)'
+            ](account1, tokenId, 1, { from: account2 })
+
+            balance = new BigNumber(await nft.balanceOf(account1, tokenId))
+            assert.strictEqual(balance.toNumber(), initialAmount - 2)
+        })
+    })
+
     describe('As a minter I want to use NFTs as subscriptions', () => {
         it('As a minter I am minting a subscription that will expire in a few blocks', async () => {
             await setupTest()