From 8abcc8dcb8072b20765b8635100133248fc8ead6 Mon Sep 17 00:00:00 2001 From: Aitor <1726644+aaitor@users.noreply.github.com> Date: Fri, 2 Feb 2024 09:30:29 +0100 Subject: [PATCH 1/2] fix: avoiding provenance id collisions --- .../NFT1155SubscriptionUpgradeable.sol | 12 +- .../token/erc1155/NFT1155Upgradeable.sol | 10 +- package.json | 2 +- test/unit/token/NFT1155Subscription.Test.js | 136 +++++++++++------- 4 files changed, 103 insertions(+), 57 deletions(-) diff --git a/contracts/token/erc1155/NFT1155SubscriptionUpgradeable.sol b/contracts/token/erc1155/NFT1155SubscriptionUpgradeable.sol index 663bceec..ebacbbf7 100644 --- a/contracts/token/erc1155/NFT1155SubscriptionUpgradeable.sol +++ b/contracts/token/erc1155/NFT1155SubscriptionUpgradeable.sol @@ -56,9 +56,13 @@ contract NFT1155SubscriptionUpgradeable is NFT1155Upgradeable { function burn(uint256 id, uint256 amount) override public { burn(_msgSender(), id, amount); - } - + } + function burn(address to, uint256 id, uint256 amount) override public { + burn(to, id, amount, 0); + } + + function burn(address to, uint256 id, uint256 amount, uint256 seed) override public { require(balanceOf(to, id) >= amount, 'ERC1155: burn amount exceeds balance'); require( isOperator(_msgSender()) || // Or the DIDRegistry is burning the NFT @@ -72,7 +76,7 @@ contract NFT1155SubscriptionUpgradeable is NFT1155Upgradeable { _nftAttributes[id].nftSupply -= amount; // Register provenance event nftRegistry.used( - keccak256(abi.encode(id, _msgSender(), 'burn', amount, block.number)), + keccak256(abi.encode(id, _msgSender(), 'burn', amount, block.number, seed, _nftAttributes[id].nftSupply)), bytes32(id), _msgSender(), keccak256('burn'), '', 'burn'); bytes32 _key = _getTokenKey(to, id); @@ -153,7 +157,7 @@ contract NFT1155SubscriptionUpgradeable is NFT1155Upgradeable { ) external { require(ids.length == amounts.length, 'burnBatch: lengths do not match'); for (uint i = 0; i < ids.length; i++) { - burn(from, ids[i], amounts[i]); + burn(from, ids[i], amounts[i], i); } } } diff --git a/contracts/token/erc1155/NFT1155Upgradeable.sol b/contracts/token/erc1155/NFT1155Upgradeable.sol index a3866f58..92dc67da 100644 --- a/contracts/token/erc1155/NFT1155Upgradeable.sol +++ b/contracts/token/erc1155/NFT1155Upgradeable.sol @@ -142,7 +142,7 @@ contract NFT1155Upgradeable is ERC1155Upgradeable, NFTBase { _nftAttributes[id].nftSupply += amount; // Register provenance event nftRegistry.used( - keccak256(abi.encode(id, _msgSender(), 'mint', amount, block.number)), + keccak256(abi.encode(id, _msgSender(), 'mint', amount, block.number, _nftAttributes[id].nftSupply, data)), bytes32(id), _msgSender(), keccak256('mint'), '', 'mint'); _mint(to, id, amount, data); @@ -151,8 +151,12 @@ contract NFT1155Upgradeable is ERC1155Upgradeable, NFTBase { function burn(uint256 id, uint256 amount) virtual public { burn(_msgSender(), id, amount); } - + function burn(address to, uint256 id, uint256 amount) virtual public { + burn(to, id, amount, 0); + } + + function burn(address to, uint256 id, uint256 amount, uint256 seed) virtual public { require(balanceOf(to, id) >= amount, 'ERC1155: burn amount exceeds balance'); require( isOperator(_msgSender()) || // Or the DIDRegistry is burning the NFT @@ -166,7 +170,7 @@ contract NFT1155Upgradeable is ERC1155Upgradeable, NFTBase { _nftAttributes[id].nftSupply -= amount; // Register provenance event nftRegistry.used( - keccak256(abi.encode(id, _msgSender(), 'burn', amount, block.number)), + keccak256(abi.encode(id, _msgSender(), 'burn', amount, block.number, seed, _nftAttributes[id].nftSupply)), bytes32(id), _msgSender(), keccak256('burn'), '', 'burn'); _burn(to, id, amount); diff --git a/package.json b/package.json index 2c000e38..839fa3fd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nevermined-io/contracts", - "version": "3.5.5", + "version": "3.5.6", "description": "Nevermined implementation of Nevermined in Solidity", "bugs": { "url": "https://github.com/nevermined-io/contracts/issues" diff --git a/test/unit/token/NFT1155Subscription.Test.js b/test/unit/token/NFT1155Subscription.Test.js index c5b5c88d..307bbf20 100644 --- a/test/unit/token/NFT1155Subscription.Test.js +++ b/test/unit/token/NFT1155Subscription.Test.js @@ -263,73 +263,111 @@ contract('NFT1155 Subscription', (accounts) => { }) it('Tokens are minted, burned and expired', async () => { - await setupTest() + await setupTest() - let balance - const didSeed4 = testUtils.generateId() - const tokenId4 = await didRegistry.hashDID(didSeed4, minter) - await didRegistry.methods[ - 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' - ](didSeed4, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) + let balance + const didSeed4 = testUtils.generateId() + const tokenId4 = await didRegistry.hashDID(didSeed4, minter) + await didRegistry.methods[ + 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' + ](didSeed4, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) - let currentBlockNumber = await ethers.provider.getBlockNumber() + let currentBlockNumber = await ethers.provider.getBlockNumber() - // We MINT 10 tokens - await nft.methods[ - 'mint(address,uint256,uint256,uint256,bytes)' - ](account2, tokenId4, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) + // We MINT 10 tokens + await nft.methods[ + 'mint(address,uint256,uint256,uint256,bytes)' + ](account2, tokenId4, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) - // Balance is 10 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 10) + // Balance is 10 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 10) - // We BURN 2 tokens - await nft.methods[ - 'burn(address,uint256,uint256)' - ](account2, tokenId4, 2, { from: minter }) + // We BURN 2 tokens + await nft.methods[ + 'burn(address,uint256,uint256)' + ](account2, tokenId4, 2, { from: minter }) - // Balance is 8 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 10 - 2) + // Balance is 8 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 10 - 2) - await increaseTime.mineBlocks(web3, blocksExpiring) + await increaseTime.mineBlocks(web3, blocksExpiring) - // Balance is 0 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 0) + // Balance is 0 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 0) + + currentBlockNumber = await ethers.provider.getBlockNumber() + + // We MINT 15 tokens + await nft.methods[ + 'mint(address,uint256,uint256,uint256,bytes)' + ](account2, tokenId4, 15, currentBlockNumber + blocksExpiring, data, { from: minter }) + + // Balance is 15 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 15) - currentBlockNumber = await ethers.provider.getBlockNumber() + // BURN 3 tokens + await nft.methods[ + 'burn(address,uint256,uint256)' + ](account2, tokenId4, 3, { from: minter }) + + // Balance is 12 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 12) + + // EXPIRE 12 tokens + await increaseTime.mineBlocks(web3, blocksExpiring) + + // Balance is 0 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 0) + + const minted = await nft.getMintedEntries(account2, tokenId4) + for (var index = 0; index < minted.length; index++) { + console.log(`Token ${minted[index].isMintOps ? 'MINTED' : 'BURNED'} on block ${minted[index].mintBlock}, amount ${minted[index].amountMinted} and expiring = ${minted[index].expirationBlock}`) + } + assert.strictEqual(minted.length, 4) + }) - // We MINT 15 tokens + it('Multiple tokens can be burned without generating provenance issues', async () => { + + await setupTest() + + let balance + const didSeed5 = testUtils.generateId() + const tokenId5 = await didRegistry.hashDID(didSeed5, minter) + await didRegistry.methods[ + 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' + ](didSeed5, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) + + let currentBlockNumber = await ethers.provider.getBlockNumber() + + // We MINT 10 tokens await nft.methods[ 'mint(address,uint256,uint256,uint256,bytes)' - ](account2, tokenId4, 15, currentBlockNumber + blocksExpiring, data, { from: minter }) + ](account2, tokenId5, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) - // Balance is 15 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 15) - // BURN 3 tokens - await nft.methods[ - 'burn(address,uint256,uint256)' - ](account2, tokenId4, 3, { from: minter }) - // Balance is 12 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 12) + // We BURN 1 tokens 2 times + await nft.burnBatch(account2, [tokenId5, tokenId5], [1, 1], { from: minter }) +// nft.methods[ +// 'burn(address,uint256,uint256)' +// ](account2, tokenId5, 1, { from: minter }) +// +// await nft.methods[ +// 'burn(address,uint256,uint256)' +// ](account2, tokenId5, 1, { from: minter }) - // EXPIRE 12 tokens - await increaseTime.mineBlocks(web3, blocksExpiring) + await increaseTime.mineBlocks(web3, 2) - // Balance is 0 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 0) + // Balance is 8 + balance = new BigNumber(await nft.balanceOf(account2, tokenId5)) + assert.strictEqual(balance.toNumber(), 10 - 2) - const minted = await nft.getMintedEntries(account2, tokenId4) - for (var index = 0; index < minted.length; index++) { - console.log(`Token ${minted[index].isMintOps ? 'MINTED' : 'BURNED'} on block ${minted[index].mintBlock}, amount ${minted[index].amountMinted} and expiring = ${minted[index].expirationBlock}`) - } - assert.strictEqual(minted.length, 4) }) }) }) From 32178c369ad076aff09609faffb513e479dfad8b Mon Sep 17 00:00:00 2001 From: Aitor <1726644+aaitor@users.noreply.github.com> Date: Fri, 2 Feb 2024 09:33:40 +0100 Subject: [PATCH 2/2] ci: linting --- test/unit/token/NFT1155Subscription.Test.js | 119 +++++++++----------- 1 file changed, 53 insertions(+), 66 deletions(-) diff --git a/test/unit/token/NFT1155Subscription.Test.js b/test/unit/token/NFT1155Subscription.Test.js index 307bbf20..8f16c7d0 100644 --- a/test/unit/token/NFT1155Subscription.Test.js +++ b/test/unit/token/NFT1155Subscription.Test.js @@ -263,111 +263,98 @@ contract('NFT1155 Subscription', (accounts) => { }) it('Tokens are minted, burned and expired', async () => { - await setupTest() + await setupTest() - let balance - const didSeed4 = testUtils.generateId() - const tokenId4 = await didRegistry.hashDID(didSeed4, minter) - await didRegistry.methods[ - 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' - ](didSeed4, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) + let balance + const didSeed4 = testUtils.generateId() + const tokenId4 = await didRegistry.hashDID(didSeed4, minter) + await didRegistry.methods[ + 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' + ](didSeed4, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) - let currentBlockNumber = await ethers.provider.getBlockNumber() + let currentBlockNumber = await ethers.provider.getBlockNumber() - // We MINT 10 tokens - await nft.methods[ - 'mint(address,uint256,uint256,uint256,bytes)' - ](account2, tokenId4, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) + // We MINT 10 tokens + await nft.methods[ + 'mint(address,uint256,uint256,uint256,bytes)' + ](account2, tokenId4, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) - // Balance is 10 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 10) + // Balance is 10 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 10) - // We BURN 2 tokens - await nft.methods[ - 'burn(address,uint256,uint256)' - ](account2, tokenId4, 2, { from: minter }) + // We BURN 2 tokens + await nft.methods[ + 'burn(address,uint256,uint256)' + ](account2, tokenId4, 2, { from: minter }) - // Balance is 8 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 10 - 2) + // Balance is 8 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 10 - 2) - await increaseTime.mineBlocks(web3, blocksExpiring) + await increaseTime.mineBlocks(web3, blocksExpiring) - // Balance is 0 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 0) + // Balance is 0 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 0) - currentBlockNumber = await ethers.provider.getBlockNumber() + currentBlockNumber = await ethers.provider.getBlockNumber() - // We MINT 15 tokens - await nft.methods[ - 'mint(address,uint256,uint256,uint256,bytes)' - ](account2, tokenId4, 15, currentBlockNumber + blocksExpiring, data, { from: minter }) + // We MINT 15 tokens + await nft.methods[ + 'mint(address,uint256,uint256,uint256,bytes)' + ](account2, tokenId4, 15, currentBlockNumber + blocksExpiring, data, { from: minter }) - // Balance is 15 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 15) + // Balance is 15 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 15) - // BURN 3 tokens - await nft.methods[ - 'burn(address,uint256,uint256)' - ](account2, tokenId4, 3, { from: minter }) + // BURN 3 tokens + await nft.methods[ + 'burn(address,uint256,uint256)' + ](account2, tokenId4, 3, { from: minter }) - // Balance is 12 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 12) + // Balance is 12 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 12) - // EXPIRE 12 tokens - await increaseTime.mineBlocks(web3, blocksExpiring) + // EXPIRE 12 tokens + await increaseTime.mineBlocks(web3, blocksExpiring) - // Balance is 0 - balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) - assert.strictEqual(balance.toNumber(), 0) + // Balance is 0 + balance = new BigNumber(await nft.balanceOf(account2, tokenId4)) + assert.strictEqual(balance.toNumber(), 0) - const minted = await nft.getMintedEntries(account2, tokenId4) - for (var index = 0; index < minted.length; index++) { - console.log(`Token ${minted[index].isMintOps ? 'MINTED' : 'BURNED'} on block ${minted[index].mintBlock}, amount ${minted[index].amountMinted} and expiring = ${minted[index].expirationBlock}`) - } - assert.strictEqual(minted.length, 4) + const minted = await nft.getMintedEntries(account2, tokenId4) + for (var index = 0; index < minted.length; index++) { + console.log(`Token ${minted[index].isMintOps ? 'MINTED' : 'BURNED'} on block ${minted[index].mintBlock}, amount ${minted[index].amountMinted} and expiring = ${minted[index].expirationBlock}`) + } + assert.strictEqual(minted.length, 4) }) it('Multiple tokens can be burned without generating provenance issues', async () => { - await setupTest() - let balance const didSeed5 = testUtils.generateId() const tokenId5 = await didRegistry.hashDID(didSeed5, minter) await didRegistry.methods[ 'registerMintableDID(bytes32,address,bytes32,address[],string,uint256,uint256,bool,bytes32,string,string)' ](didSeed5, nft.address, checksum, [], url, 0, 0, false, constants.activities.GENERATED, '', '', { from: minter }) - let currentBlockNumber = await ethers.provider.getBlockNumber() + const currentBlockNumber = await ethers.provider.getBlockNumber() // We MINT 10 tokens await nft.methods[ 'mint(address,uint256,uint256,uint256,bytes)' ](account2, tokenId5, 10, currentBlockNumber + blocksExpiring, data, { from: minter }) - - // We BURN 1 tokens 2 times await nft.burnBatch(account2, [tokenId5, tokenId5], [1, 1], { from: minter }) -// nft.methods[ -// 'burn(address,uint256,uint256)' -// ](account2, tokenId5, 1, { from: minter }) -// -// await nft.methods[ -// 'burn(address,uint256,uint256)' -// ](account2, tokenId5, 1, { from: minter }) - await increaseTime.mineBlocks(web3, 2) // Balance is 8 - balance = new BigNumber(await nft.balanceOf(account2, tokenId5)) + const balance = new BigNumber(await nft.balanceOf(account2, tokenId5)) assert.strictEqual(balance.toNumber(), 10 - 2) - }) }) })