From ad78c3bb24674a7a24580a10cedd682400481b6b Mon Sep 17 00:00:00 2001 From: Maxime Date: Thu, 31 Oct 2024 10:09:21 +0000 Subject: [PATCH] refactor: apply recommendations --- src/HypLSP8.sol | 22 +++++++++++++++++----- src/HypLSP8Collateral.sol | 11 ++++++----- test/HypLSP8.t.sol | 34 +++++++++------------------------- test/LSP8Mock.sol | 21 --------------------- 4 files changed, 32 insertions(+), 56 deletions(-) diff --git a/src/HypLSP8.sol b/src/HypLSP8.sol index de265ea..a225943 100644 --- 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 index bd155ef..6ec028b 100644 --- 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,7 +34,7 @@ contract HypLSP8Collateral is TokenRouter { } function ownerOf(uint256 _tokenId) external view returns (address) { - return ILSP8IdentifiableDigitalAsset(wrappedToken).tokenOwnerOf(bytes32(_tokenId)); + return ILSP8(wrappedToken).tokenOwnerOf(bytes32(_tokenId)); } /** @@ -41,7 +42,7 @@ contract HypLSP8Collateral is TokenRouter { * @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 index f66f4a0..e406201 100644 --- 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 index 9a5815a..8ca0998 100644 --- 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";