Merge pull request #661 from lukso-network/DEV-7667_C4-120-LSP8Burnab…

…le-extension-incorrectly-inherits-LSP8IdentifiableDigitalAssetCore_Jean

fix: (C4 #120) inheritance of `LSP8Burnable` + add tests to check LSP4 Metadata is set correctly on deployment + initialization

contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol

@@ -2,17 +2,17 @@
 pragma solidity ^0.8.4;
 
 import {
-    LSP8IdentifiableDigitalAssetCore
-} from "../LSP8IdentifiableDigitalAssetCore.sol";
+    LSP8IdentifiableDigitalAsset
+} from "../LSP8IdentifiableDigitalAsset.sol";
 
 // errors
 import {LSP8NotTokenOperator} from "../LSP8Errors.sol";
 
 /**
- * @dev LSP8 extension that allows token holders to destroy both
+ * @dev LSP8 extension (standard version) that allows token holders to destroy both
  * their own tokens and those that they have an allowance for as an operator.
  */
-abstract contract LSP8Burnable is LSP8IdentifiableDigitalAssetCore {
+abstract contract LSP8Burnable is LSP8IdentifiableDigitalAsset {
     function burn(bytes32 tokenId, bytes memory data) public virtual {
         if (!_isOperatorOrOwner(msg.sender, tokenId)) {
             revert LSP8NotTokenOperator(tokenId, msg.sender);

contracts/LSP8IdentifiableDigitalAsset/extensions/LSP8BurnableInitAbstract.sol

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.4;
+
+import {
+    LSP8IdentifiableDigitalAssetInitAbstract
+} from "../LSP8IdentifiableDigitalAssetInitAbstract.sol";
+
+// errors
+import {LSP8NotTokenOperator} from "../LSP8Errors.sol";
+
+/**
+ * @dev LSP8 extension (proxy version) that allows token holders to destroy both
+ * their own tokens and those that they have an allowance for as an operator.
+ */
+abstract contract LSP8BurnableInitAbstract is
+    LSP8IdentifiableDigitalAssetInitAbstract
+{
+    function burn(bytes32 tokenId, bytes memory data) public virtual {
+        if (!_isOperatorOrOwner(msg.sender, tokenId)) {
+            revert LSP8NotTokenOperator(tokenId, msg.sender);
+        }
+        _burn(tokenId, data);
+    }
+}

contracts/Mocks/Tokens/LSP8BurnableInitTester.sol

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.4;
+
+// modules
+import {
+    LSP8IdentifiableDigitalAssetInitAbstract
+} from "../../LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetInitAbstract.sol";
+import {
+    LSP8BurnableInitAbstract
+} from "../../LSP8IdentifiableDigitalAsset/extensions/LSP8BurnableInitAbstract.sol";
+
+contract LSP8BurnableInitTester is LSP8BurnableInitAbstract {
+    function initialize(
+        string memory name_,
+        string memory symbol_,
+        address newOwner_
+    ) public virtual initializer {
+        LSP8IdentifiableDigitalAssetInitAbstract._initialize(
+            name_,
+            symbol_,
+            newOwner_
+        );
+    }
+}

contracts/Mocks/Tokens/LSP8BurnableTester.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.4;
+
+// modules
+import {
+    LSP8IdentifiableDigitalAsset
+} from "../../LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.sol";
+import {
+    LSP8Burnable
+} from "../../LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol";
+
+contract LSP8BurnableTester is LSP8Burnable {
+    constructor(
+        string memory name_,
+        string memory symbol_,
+        address newOwner_
+    ) LSP8IdentifiableDigitalAsset(name_, symbol_, newOwner_) {}
+}

contracts/Mocks/Tokens/LSP8InitTester.sol

@@ -7,12 +7,12 @@ import {
     LSP8IdentifiableDigitalAssetInitAbstract
 } from "../../LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetInitAbstract.sol";
 import {
-    LSP8Burnable
-} from "../../LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol";
+    LSP8BurnableInitAbstract
+} from "../../LSP8IdentifiableDigitalAsset/extensions/LSP8BurnableInitAbstract.sol";
 
 contract LSP8InitTester is
     LSP8IdentifiableDigitalAssetInitAbstract,
-    LSP8Burnable
+    LSP8BurnableInitAbstract
 {
     function initialize(
         string memory name,

tests/LSP8IdentifiableDigitalAsset/proxy/LSP8BurnableInit.test.ts

@@ -0,0 +1,75 @@
+import { ethers } from 'hardhat';
+import { expect } from 'chai';
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
+
+import { LSP8BurnableInitTester, LSP8BurnableInitTester__factory } from '../../../types';
+
+import { shouldInitializeLikeLSP8 } from '../LSP8IdentifiableDigitalAsset.behaviour';
+
+import { deployProxy } from '../../utils/fixtures';
+
+type LSP8BurnableInitTestContext = {
+  accounts: SignerWithAddress[];
+  lsp8Burnable: LSP8BurnableInitTester;
+  deployParams: {
+    name: string;
+    symbol: string;
+    newOwner: string;
+  };
+};
+
+describe('LSP8BurnableInit with proxy', () => {
+  const buildTestContext = async () => {
+    const accounts = await ethers.getSigners();
+    const deployParams = {
+      name: 'LSP8 Burnable - deployed with constructor',
+      symbol: 'BRN',
+      newOwner: accounts[0].address,
+    };
+
+    const lsp8BurnableImplementation = await new LSP8BurnableInitTester__factory(
+      accounts[0],
+    ).deploy();
+    const lsp8BurnableProxy = await deployProxy(lsp8BurnableImplementation.address, accounts[0]);
+    const lsp8Burnable = lsp8BurnableImplementation.attach(lsp8BurnableProxy);
+
+    return { accounts, lsp8Burnable, deployParams };
+  };
+
+  const initializeProxy = async (context: LSP8BurnableInitTestContext) => {
+    return context.lsp8Burnable.initialize(
+      context.deployParams.name,
+      context.deployParams.symbol,
+      context.deployParams.newOwner,
+    );
+  };
+
+  describe('when deploying the contract as proxy', () => {
+    let context: LSP8BurnableInitTestContext;
+
+    before(async () => {
+      context = await buildTestContext();
+    });
+
+    describe('when initializing the contract', () => {
+      shouldInitializeLikeLSP8(async () => {
+        const { lsp8Burnable: lsp8, deployParams } = context;
+        const initializeTransaction = await initializeProxy(context);
+
+        return {
+          lsp8,
+          deployParams,
+          initializeTransaction,
+        };
+      });
+    });
+
+    describe('when calling initialize more than once', () => {
+      it('should revert', async () => {
+        await expect(initializeProxy(context)).to.be.revertedWith(
+          'Initializable: contract is already initialized',
+        );
+      });
+    });
+  });
+});

tests/LSP8IdentifiableDigitalAsset/standard/LSP8Burnable.test.ts

@@ -0,0 +1,53 @@
+import { ethers } from 'hardhat';
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
+
+import { LSP8BurnableTester, LSP8BurnableTester__factory } from '../../../types';
+
+import { shouldInitializeLikeLSP8 } from '../LSP8IdentifiableDigitalAsset.behaviour';
+
+type LSP8BurnableTestContext = {
+  accounts: SignerWithAddress[];
+  lsp8Burnable: LSP8BurnableTester;
+  deployParams: {
+    name: string;
+    symbol: string;
+    newOwner: string;
+  };
+};
+
+describe('LSP8Burnable with constructor', () => {
+  const buildTestContext = async () => {
+    const accounts = await ethers.getSigners();
+    const deployParams = {
+      name: 'LSP8 Burnable - deployed with constructor',
+      symbol: 'BRN',
+      newOwner: accounts[0].address,
+    };
+
+    const lsp8Burnable = await new LSP8BurnableTester__factory(accounts[0]).deploy(
+      deployParams.name,
+      deployParams.symbol,
+      deployParams.newOwner,
+    );
+
+    return { accounts, lsp8Burnable, deployParams };
+  };
+
+  describe('when deploying the contract', () => {
+    let context: LSP8BurnableTestContext;
+
+    before(async () => {
+      context = await buildTestContext();
+    });
+
+    shouldInitializeLikeLSP8(async () => {
+      const { lsp8Burnable: lsp8, deployParams } = context;
+
+      return {
+        lsp8,
+        deployParams,
+        initializeTransaction: context.lsp8Burnable.deployTransaction,
+      };
+    });
+  });
+});