Merge pull request #825 from etherisc/feature/chainid-type-refactoring

use ChainId type instead of uint (#823)
etherisc · Dec 19, 2024 · 75bbfa9 · 75bbfa9
2 parents 4a8758c + 6cf7eeb
commit 75bbfa9
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 50 deletions.
diff --git a/contracts/registry/ChainNft.sol b/contracts/registry/ChainNft.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.20;
 import {ERC721, ERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol";
 import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 
+import {ChainId, ChainIdLib} from "../type/ChainId.sol";
 import {ITransferInterceptor} from "./ITransferInterceptor.sol";
 
 contract ChainNft is ERC721Enumerable {
@@ -53,7 +54,7 @@ contract ChainNft is ERC721Enumerable {
         // NFT contract is deployed by the registry
         _registry = registry;
 
-        _chainIdDigits = _calculateChainIdDigits(block.chainid);
+        _chainIdDigits = _calculateChainIdDigits(ChainIdLib.toChainId(block.chainid));
         _chainIdMultiplier = 10 ** _chainIdDigits;
 
         // the first object registered through normal registration starts with id 4
@@ -200,12 +201,12 @@ contract ChainNft is ERC721Enumerable {
     * (42 * 10 ** 10 + 9876543210) * 100 + 10
     * (index * 10 ** digits + chainid) * 100 + digits (1 < digits < 100)
     */
-    function calculateTokenId(uint256 idIndex, uint256 chainId) public view returns (uint256 id) {
-        if(chainId == block.chainid) {
-            return 100 * (idIndex * _chainIdMultiplier + chainId) + _chainIdDigits;
+    function calculateTokenId(uint256 idIndex, ChainId chainId) public view returns (uint256 id) {
+        if(chainId.toInt() == block.chainid) {
+            return 100 * (idIndex * _chainIdMultiplier + chainId.toInt()) + _chainIdDigits;
         } else {
             uint256 chainIdDigits = _calculateChainIdDigits(chainId);
-            return 100 * (idIndex * (10 ** chainIdDigits) + chainId) + chainIdDigits;
+            return 100 * (idIndex * (10 ** chainIdDigits) + chainId.toInt()) + chainIdDigits;
         }
     }
 
@@ -224,8 +225,8 @@ contract ChainNft is ERC721Enumerable {
         _idNext++;
     }
 
-    function _calculateChainIdDigits(uint256 chainId) private pure returns (uint256) {
-        uint256 num = chainId;
+    function _calculateChainIdDigits(ChainId chainId) private pure returns (uint256) {
+        uint256 num = chainId.toInt();
         uint256 digits = 0;
         while (num != 0) {
             digits++;

diff --git a/contracts/registry/IRegistry.sol b/contracts/registry/IRegistry.sol
@@ -5,6 +5,7 @@ import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 
 import {IRelease} from "./IRelease.sol";
 
+import {ChainId} from "../type/ChainId.sol";
 import {NftId} from "../type/NftId.sol";
 import {ObjectType} from "../type/ObjectType.sol";
 import {VersionPart} from "../type/Version.sol";
@@ -21,7 +22,7 @@ interface IRegistry is
 
     event LogRegistryObjectRegistered(NftId indexed nftId, NftId indexed parentNftId, ObjectType indexed objectType, bool isInterceptor, address objectAddress, address initialOwner);
     event LogRegistryServiceRegistered(NftId indexed nftId, VersionPart indexed majorVersion, ObjectType indexed domain);
-    event LogRegistryChainRegistryRegistered(NftId indexed nftId, uint256 indexed chainId, address indexed chainRegistryAddress);
+    event LogRegistryChainRegistryRegistered(NftId indexed nftId, ChainId indexed chainId, address indexed chainRegistryAddress);
 
     // initialize
     error ErrorRegistryCallerNotDeployer();
@@ -30,11 +31,11 @@ interface IRegistry is
     error ErrorRegistryObjectTypeNotSupported(ObjectType objectType);
 
     // registerRegistry()
-    error ErrorRegistryNotOnMainnet(uint256 chainId);
+    error ErrorRegistryNotOnMainnet(ChainId chainId);
     error ErrorRegistryChainRegistryChainIdZero(NftId nftId);
-    error ErrorRegistryChainRegistryAddressZero(NftId nftId, uint256 chainId);
-    error ErrorRegistryChainRegistryNftIdInvalid(NftId nftId, uint256 chainId);
-    error ErrorRegistryChainRegistryAlreadyRegistered(NftId nftId, uint256 chainId);
+    error ErrorRegistryChainRegistryAddressZero(NftId nftId, ChainId chainId);
+    error ErrorRegistryChainRegistryNftIdInvalid(NftId nftId, ChainId chainId);
+    error ErrorRegistryChainRegistryAlreadyRegistered(NftId nftId, ChainId chainId);
 
     // registerService()
     error ErrorRegistryServiceAddressZero(); 
@@ -71,7 +72,7 @@ interface IRegistry is
     /// Only one chain registry may be registered per chain
     function registerRegistry(
         NftId nftId, 
-        uint256 chainId, 
+        ChainId chainId, 
         address chainRegistryAddress
     ) external;
 
@@ -107,10 +108,10 @@ interface IRegistry is
     function chainIds() external view returns (uint256);
 
     /// @dev Returns the chain id at the specified index.
-    function getChainId(uint256 idx) external view returns (uint256);
+    function getChainId(uint256 idx) external view returns (ChainId);
 
     /// @dev Returns the NFT ID of the registry for the specified chain.
-    function getRegistryNftId(uint256 chainId) external returns (NftId nftId); 
+    function getRegistryNftId(ChainId chainId) external returns (NftId nftId); 
 
     function getObjectCount() external view returns (uint256);
 

diff --git a/contracts/registry/Registry.sol b/contracts/registry/Registry.sol
@@ -9,6 +9,7 @@ import {NftId, NftIdLib} from "../type/NftId.sol";
 import {VersionPart, VersionPartLib} from "../type/Version.sol";
 import {ObjectType, ObjectTypeLib, PROTOCOL, REGISTRY, SERVICE, INSTANCE, STAKE, STAKING, PRODUCT, DISTRIBUTION, DISTRIBUTOR, ORACLE, POOL, POLICY, BUNDLE} from "../type/ObjectType.sol";
 
+import {ChainId, ChainIdLib} from "../type/ChainId.sol";
 import {ChainNft} from "./ChainNft.sol";
 import {IRegistry} from "./IRegistry.sol";
 import {IRelease} from "./IRelease.sol";
@@ -63,8 +64,8 @@ contract Registry is
     string public constant EMPTY_URI = "";
 
     /// @dev keep track of different registries on different chains
-    mapping(uint256 chainId => NftId registryNftId) private _registryNftIdByChainId;
-    uint256[] private _chainId;
+    mapping(ChainId chainId => NftId registryNftId) private _registryNftIdByChainId;
+    ChainId[] private _chainId;
 
     /// @dev keep track of object info and address reverse lookup
     mapping(NftId nftId => ObjectInfo info) private _info;
@@ -143,19 +144,19 @@ contract Registry is
     /// @inheritdoc IRegistry
     function registerRegistry(
         NftId nftId,
-        uint256 chainId, 
+        ChainId chainId, 
         address registryAddress
     )
         external
         restricted()
     {
         // registration of chain registries only allowed on mainnet
         if (block.chainid != 1) {
-            revert ErrorRegistryNotOnMainnet(block.chainid);
+            revert ErrorRegistryNotOnMainnet(ChainIdLib.toChainId(block.chainid));
         }
 
         // registry chain id is not zero
-        if(chainId == 0) {
+        if(chainId.eqz()) {
             revert ErrorRegistryChainRegistryChainIdZero(nftId);
         }
 
@@ -320,11 +321,11 @@ contract Registry is
         return _chainId.length;
     }
 
-    function getChainId(uint256 idx) public view returns (uint256) {
+    function getChainId(uint256 idx) public view returns (ChainId) {
         return _chainId[idx];
     }
 
-    function getRegistryNftId(uint256 chainId) public view returns (NftId nftId) {
+    function getRegistryNftId(ChainId chainId) public view returns (NftId nftId) {
         return _registryNftIdByChainId[chainId];
     }
 
@@ -579,7 +580,7 @@ contract Registry is
 
         // register global registry
         _registerRegistryForNft(
-            1, // mainnet chain id
+            ChainIdLib.toChainId(1), // mainnet chain id
             ObjectInfo({
                 nftId: GLOBAL_REGISTRY_NFT_ID,
                 parentNftId: PROTOCOL_NFT_ID,
@@ -598,7 +599,7 @@ contract Registry is
                 CHAIN_NFT.calculateTokenId(REGISTRY_TOKEN_SEQUENCE_ID));
 
             _registerRegistryForNft(
-                block.chainid, 
+                ChainIdLib.toChainId(block.chainid), 
                 ObjectInfo({
                     nftId: registryNftId,
                     parentNftId: GLOBAL_REGISTRY_NFT_ID,
@@ -613,7 +614,7 @@ contract Registry is
 
     /// @dev staking registration
     function _registerRegistryForNft(
-        uint256 chainId,
+        ChainId chainId,
         ObjectInfo memory info,
         bool updateAddressLookup
     )

diff --git a/contracts/staking/StakingService.sol b/contracts/staking/StakingService.sol
@@ -52,7 +52,6 @@ contract StakingService is
         virtual
         restricted()
     {
-        uint256 chainId = block.chainid;
         _getStakingServiceStorage()._staking.registerTarget(
             targetNftId,
             INSTANCE(),

diff --git a/scripts/libs/registry.ts b/scripts/libs/registry.ts
@@ -153,6 +153,7 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr
         [registryAdminAddress, globalRegistry], 
         {
             libraries: {
+                ChainIdLib: libraries.chainIdLibAddress,
                 NftIdLib: libraries.nftIdLibAddress,
                 ObjectTypeLib: libraries.objectTypeLibAddress,
                 VersionPartLib: libraries.versionPartLibAddress,

diff --git a/test/base/GifTestHelper.sol b/test/base/GifTestHelper.sol
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.20;
+
+import {ChainId} from "../../contracts/type/ChainId.sol";
+import {StdAssertions} from "forge-std/StdAssertions.sol";
+
+contract GifTestHelper is StdAssertions {
+
+    function assertEqChainId(ChainId chainId1, ChainId chainId2) public pure {
+        assertEq(chainId1.toInt(), chainId2.toInt(), "ChainId not equal");
+    }
+
+        function assertEqChainId(ChainId chainId1, ChainId chainId2, string memory err) public pure {
+        assertEq(chainId1.toInt(), chainId2.toInt(), err);
+    }
+
+}