refactor: apply recommendations

lukso-network · Oct 31, 2024 · ad78c3b · ad78c3b
1 parent ffa2248
commit ad78c3b
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 56 deletions.
diff --git a/src/HypLSP8.sol b/src/HypLSP8.sol
@@ -11,8 +11,8 @@ import { _LSP4_TOKEN_TYPE_TOKEN } from "@lukso/lsp4-contracts/contracts/LSP4Cons
 import { _LSP8_TOKENID_FORMAT_NUMBER } from "@lukso/lsp8-contracts/contracts/LSP8Constants.sol";
 
 /**
- * @title Hyperlane LSP8 Token Router that extends LSP8 with remote transfer functionality.
- * @author Abacus Works
+ * @title LSP8 version of the Hyperlane ERC721 Token Router
+ * @dev https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/main/solidity/contracts/token/HypERC721.sol
  */
 contract HypLSP8 is LSP8IdentifiableDigitalAssetInitAbstract, TokenRouter {
     constructor(address _mailbox) TokenRouter(_mailbox) { }
@@ -23,7 +23,18 @@ contract HypLSP8 is LSP8IdentifiableDigitalAssetInitAbstract, TokenRouter {
      * @param _name The name of the token.
      * @param _symbol The symbol of the token.
      */
-    function initialize(uint256 _mintAmount, string memory _name, string memory _symbol) external initializer {
+    function initialize(
+        uint256 _mintAmount,
+        address _hook,
+        address _interchainSecurityModule,
+        address _owner,
+        string memory _name,
+        string memory _symbol
+    )
+        external
+        initializer
+    {
+        _MailboxClient_initialize(_hook, _interchainSecurityModule, _owner);
         address owner = msg.sender;
         _transferOwnership(owner);
 
@@ -51,8 +62,9 @@ contract HypLSP8 is LSP8IdentifiableDigitalAssetInitAbstract, TokenRouter {
      * @inheritdoc TokenRouter
      */
     function _transferFromSender(uint256 _tokenId) internal virtual override returns (bytes memory) {
-        require(tokenOwnerOf(bytes32(_tokenId)) == msg.sender, "!owner");
-        _burn(bytes32(_tokenId), "");
+        bytes32 tokenIdAsBytes32 = bytes32(_tokenId);
+        require(tokenOwnerOf(tokenIdAsBytes32) == msg.sender, "!owner");
+        _burn(tokenIdAsBytes32, "");
         return bytes(""); // no metadata
     }
 

diff --git a/src/HypLSP8Collateral.sol b/src/HypLSP8Collateral.sol
@@ -5,21 +5,22 @@ import { TokenRouter } from "@hyperlane-xyz/core/contracts/token/libs/TokenRoute
 
 import { TokenMessage } from "@hyperlane-xyz/core/contracts/token/libs/TokenMessage.sol";
 
-import { ILSP8IdentifiableDigitalAsset } from "@lukso/lsp8-contracts/contracts/ILSP8IdentifiableDigitalAsset.sol";
+import { ILSP8IdentifiableDigitalAsset as ILSP8 } from
+    "@lukso/lsp8-contracts/contracts/ILSP8IdentifiableDigitalAsset.sol";
 
 /**
  * @title Hyperlane LSP8 Token Collateral that wraps an existing LSP8 with remote transfer functionality.
  * @author Abacus Works
  */
 contract HypLSP8Collateral is TokenRouter {
-    ILSP8IdentifiableDigitalAsset public immutable wrappedToken;
+    ILSP8 public immutable wrappedToken;
 
     /**
      * @notice Constructor
      * @param lsp8 Address of the token to keep as collateral
      */
     constructor(address lsp8, address _mailbox) TokenRouter(_mailbox) {
-        wrappedToken = ILSP8IdentifiableDigitalAsset(lsp8);
+        wrappedToken = ILSP8(lsp8);
     }
 
     /**
@@ -33,15 +34,15 @@ contract HypLSP8Collateral is TokenRouter {
     }
 
     function ownerOf(uint256 _tokenId) external view returns (address) {
-        return ILSP8IdentifiableDigitalAsset(wrappedToken).tokenOwnerOf(bytes32(_tokenId));
+        return ILSP8(wrappedToken).tokenOwnerOf(bytes32(_tokenId));
     }
 
     /**
      * @dev Returns the balance of `_account` for `wrappedToken`.
      * @inheritdoc TokenRouter
      */
     function balanceOf(address _account) external view override returns (uint256) {
-        return ILSP8IdentifiableDigitalAsset(wrappedToken).balanceOf(_account);
+        return ILSP8(wrappedToken).balanceOf(_account);
     }
 
     /**

diff --git a/test/HypLSP8.t.sol b/test/HypLSP8.t.sol
@@ -48,24 +48,13 @@ abstract contract HypTokenTest is Test {
         localMailbox.setDefaultHook(address(noopHook));
         localMailbox.setRequiredHook(address(noopHook));
 
-        remoteToken = new HypLSP8(address(remoteMailbox));
-
-        vm.prank(OWNER);
-        remoteToken.initialize(0, NAME, SYMBOL);
-
         vm.deal(ALICE, 1 ether);
     }
 
-    function _deployRemoteToken(bool isCollateral) internal {
-        if (isCollateral) {
-            remoteToken = new HypLSP8(address(remoteMailbox));
-            vm.prank(OWNER);
-            remoteToken.initialize(0, NAME, SYMBOL);
-        } else {
-            remoteToken = new HypLSP8(address(remoteMailbox));
-            vm.prank(OWNER);
-            remoteToken.initialize(0, NAME, SYMBOL);
-        }
+    function _deployRemoteToken() internal {
+        remoteToken = new HypLSP8(address(remoteMailbox));
+        vm.prank(OWNER);
+        remoteToken.initialize(0, address(noopHook), address(0), OWNER, NAME, SYMBOL);
         vm.prank(OWNER);
         remoteToken.enrollRemoteRouter(ORIGIN, address(localToken).addressToBytes32());
     }
@@ -98,11 +87,12 @@ contract HypLSP8Test is HypTokenTest {
         lsp8Token = HypLSP8(payable(address(localToken)));
 
         vm.prank(OWNER);
-        lsp8Token.initialize(INITIAL_SUPPLY, NAME, SYMBOL);
+        lsp8Token.initialize(INITIAL_SUPPLY, address(noopHook), address(0), OWNER, NAME, SYMBOL);
 
         vm.prank(OWNER);
         lsp8Token.enrollRemoteRouter(DESTINATION, address(remoteToken).addressToBytes32());
 
+        // Give accounts some ETH for gas
         vm.deal(OWNER, 1 ether);
         vm.deal(ALICE, 1 ether);
         vm.deal(BOB, 1 ether);
@@ -111,12 +101,12 @@ contract HypLSP8Test is HypTokenTest {
         vm.prank(OWNER);
         lsp8Token.transfer(OWNER, ALICE, TOKEN_ID, true, "");
 
-        _deployRemoteToken(false);
+        _deployRemoteToken();
     }
 
     function testInitialize_revert_ifAlreadyInitialized() public {
         vm.expectRevert("Initializable: contract is already initialized");
-        lsp8Token.initialize(INITIAL_SUPPLY, NAME, SYMBOL);
+        lsp8Token.initialize(INITIAL_SUPPLY, address(noopHook), address(0), OWNER, NAME, SYMBOL);
     }
 
     function testTotalSupply() public {
@@ -181,17 +171,11 @@ contract HypLSP8CollateralTest is HypTokenTest {
         localPrimaryToken.transfer(OWNER, ALICE, TOKEN_ID, true, "");
         vm.stopPrank();
 
-        // Deploy remote token
-        remoteToken = new HypLSP8(address(remoteMailbox));
-        vm.prank(OWNER);
-        remoteToken.initialize(0, NAME, SYMBOL);
+        _deployRemoteToken();
 
         // Enroll routers for both chains
         vm.prank(OWNER);
         lsp8Collateral.enrollRemoteRouter(DESTINATION, address(remoteToken).addressToBytes32());
-
-        vm.prank(OWNER);
-        remoteToken.enrollRemoteRouter(ORIGIN, address(localToken).addressToBytes32());
     }
 
     function testRemoteTransfer() public {

diff --git a/test/LSP8Mock.sol b/test/LSP8Mock.sol
@@ -57,27 +57,6 @@ contract LSP8Mock is LSP8IdentifiableDigitalAsset {
         _setData(dataKey, dataValue);
     }
 
-    function transferBatch(
-        address[] memory from,
-        address[] memory to,
-        bytes32[] memory tokenIds,
-        bool[] memory force,
-        bytes[] memory data
-    )
-        public
-        override
-    {
-        require(
-            from.length == to.length && to.length == tokenIds.length && tokenIds.length == force.length
-                && force.length == data.length,
-            "LSP8Mock: array length mismatch"
-        );
-
-        for (uint256 i = 0; i < from.length; i++) {
-            transfer(from[i], to[i], tokenIds[i], force[i], data[i]);
-        }
-    }
-
     // Override for testing purposes - allows easy token ID verification
     function tokenURI(bytes32 tokenId) public pure returns (string memory) {
         return "TEST-BASE-URI";