refactor: remove error check to allow setting additional future types…

… of tokenIds
lukso-network · Sep 18, 2023 · 9026d61 · 9026d61
1 parent 36150f5
commit 9026d61
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 46 deletions.
diff --git a/contracts/LSP8IdentifiableDigitalAsset/LSP8Errors.sol b/contracts/LSP8IdentifiableDigitalAsset/LSP8Errors.sol
@@ -72,11 +72,6 @@ error LSP8NotifyTokenReceiverIsEOA(address tokenReceiver);
  */
 error LSP8TokenOwnerCannotBeOperator();
 
-/**
- * @dev Reverts when providing a value for tokenId type on deployment / initialization that is not between `0` and `4`.
- */
-error LSP8InvalidTokenIdType();
-
 /**
  * @dev Reverts when trying to edit the data key `LSP8TokenIdType` after the identifiable digital asset contract has been deployed.
  * The `LSP8TokenIdType` data key is located inside the ERC725Y Data key-value store of the identifiable digital asset contract.

diff --git a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.sol b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.sol
@@ -22,10 +22,7 @@ import {LSP2Utils} from "../LSP2ERC725YJSONSchema/LSP2Utils.sol";
 import {_INTERFACEID_LSP8, _LSP8_TOKENID_TYPE_KEY} from "./LSP8Constants.sol";
 
 // errors
-import {
-    LSP8InvalidTokenIdType,
-    LSP8TokenIdTypeNotEditable
-} from "./LSP8Errors.sol";
+import {LSP8TokenIdTypeNotEditable} from "./LSP8Errors.sol";
 
 import "../LSP17ContractExtension/LSP17Constants.sol";
 
@@ -55,20 +52,22 @@ abstract contract LSP8IdentifiableDigitalAsset is
 {
     /**
      * @notice Sets the token-Metadata
+     *
+     * @dev Deploy a `LSP8IdentifiableDigitalAsset` contract and set the tokenId type inside the ERC725Y storage of the contract.
+     *
      * @param name_ The name of the token
      * @param symbol_ The symbol of the token
      * @param newOwner_ The owner of the the token-Metadata
+     *
+     * @custom:warning Make sure the tokenId type provided on deployment is correct, as it can only be set once
+     * and cannot be changed in the ERC725Y storage after the contract has been deployed.
      */
     constructor(
         string memory name_,
         string memory symbol_,
         address newOwner_,
         uint256 tokenIdType
     ) LSP4DigitalAssetMetadata(name_, symbol_, newOwner_) {
-        if (tokenIdType > 4) {
-            revert LSP8InvalidTokenIdType();
-        }
-
         LSP4DigitalAssetMetadata._setData(
             _LSP8_TOKENID_TYPE_KEY,
             abi.encode(tokenIdType)

diff --git a/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetInitAbstract.sol b/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetInitAbstract.sol
@@ -22,10 +22,7 @@ import {LSP2Utils} from "../LSP2ERC725YJSONSchema/LSP2Utils.sol";
 import {_INTERFACEID_LSP8, _LSP8_TOKENID_TYPE_KEY} from "./LSP8Constants.sol";
 
 // errors
-import {
-    LSP8InvalidTokenIdType,
-    LSP8TokenIdTypeNotEditable
-} from "./LSP8Errors.sol";
+import {LSP8TokenIdTypeNotEditable} from "./LSP8Errors.sol";
 
 import "../LSP17ContractExtension/LSP17Constants.sol";
 
@@ -59,10 +56,6 @@ abstract contract LSP8IdentifiableDigitalAssetInitAbstract is
         address newOwner_,
         uint256 tokenIdType
     ) internal virtual onlyInitializing {
-        if (tokenIdType > 4) {
-            revert LSP8InvalidTokenIdType();
-        }
-
         LSP4DigitalAssetMetadataInitAbstract._initialize(
             name_,
             symbol_,

diff --git a/tests/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.behaviour.ts b/tests/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.behaviour.ts
@@ -1869,6 +1869,16 @@ export const shouldInitializeLikeLSP8 = (
         .to.emit(context.lsp8, 'DataChanged')
         .withArgs(symbolKey, expectedSymbolValue);
       expect(await context.lsp8.getData(symbolKey)).to.equal(expectedSymbolValue);
+
+      const lsp8TokenIdTypeDataKey = ERC725YDataKeys.LSP8['LSP8TokenIdType'];
+      const expectedTokenIdDataValue = abiCoder.encode(
+        ['uint256'],
+        [context.deployParams.tokenIdType],
+      );
+      await expect(context.initializeTransaction)
+        .to.emit(context.lsp8, 'DataChanged')
+        .withArgs(lsp8TokenIdTypeDataKey, expectedTokenIdDataValue);
+      expect(await context.lsp8.getData(lsp8TokenIdTypeDataKey)).to.equal(expectedTokenIdDataValue);
     });
   });
 };
diff --git a/tests/LSP8IdentifiableDigitalAsset/standard/LSP8IdentifiableDigitalAsset.test.ts b/tests/LSP8IdentifiableDigitalAsset/standard/LSP8IdentifiableDigitalAsset.test.ts
@@ -95,31 +95,6 @@ describe('LSP8IdentifiableDigitalAsset with constructor', () => {
       ).to.be.revertedWith('Ownable: new owner is the zero address');
     });
 
-    [{ tokenIdType: 6 }, { tokenIdType: 414 }, { tokenIdType: 1111111 }].forEach(
-      ({ tokenIdType }) => {
-        it(`should revert when deploying with value = ${tokenIdType} for the tokenId type`, async () => {
-          const accounts = await ethers.getSigners();
-
-          const deployParams = {
-            name: 'LSP8 - deployed with constructor',
-            symbol: 'NFT',
-            newOwner: accounts[0].address,
-          };
-
-          const lsp8ContractToDeploy = new LSP8Tester__factory(accounts[0]);
-
-          await expect(
-            lsp8ContractToDeploy.deploy(
-              deployParams.name,
-              deployParams.symbol,
-              accounts[0].address,
-              tokenIdType,
-            ),
-          ).to.be.revertedWithCustomError(lsp8ContractToDeploy, 'LSP8InvalidTokenIdType');
-        });
-      },
-    );
-
     describe('once the contract was deployed', () => {
       let context: LSP8TestContext;