Feat/xmorse - #3
Conversation
b84361a to
ace7419
Compare
…d comprehensive tests for cross-chain NFT transfers and message encoding/decoding.
- Removed direct treasury initialization in xMorse constructor; treasury address is now set via a dedicated function. - Updated xMorse to ensure proper handling of NFT transfers using the mirror contract. - Added tests for xMorse and xMorseCollateral to validate treasury functionality and cross-chain NFT transfers. - Enhanced LibTransfer to improve NFT transfer logic and added tests for reentrancy protection.
- Updated LibTransfer to utilize the mirror address for NFT transfers. - Added new test suite for LibTransfer to validate sendNFT and sendNFTPartial functionalities. - Introduced MockDN404 and SimpleMulticall contracts for testing purposes. - Created xDN404Treasury test suite to ensure correct NFT withdrawal behavior.
ace7419 to
c36bdda
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces cross-chain NFT transfer functionality for the xMorse protocol, enabling NFT bridging between Ethereum and Mitosis chains using Hyperlane infrastructure.
Key changes:
- Added
xMorseCollateralcontract for locking NFTs on the source chain (Ethereum) - Modified
xMorseinitialization to support external treasury setup - Implemented comprehensive test coverage for cross-chain transfers and collateral operations
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/xMorseCollateral.t.sol | Unit tests for collateral contract covering NFT locking, transfers, and ownership |
| test/xMorse.t.sol | Unit tests for xMorse token including initialization, finalization, and DN404 functionality |
| test/xDN404Treasury.t.sol | Fixed treasury initialization to set skipNFT before token transfer |
| test/utils/HyperlaneTestUtils.sol | Added hook fee configuration and improved documentation for message relay |
| test/libs/LibTransfer.t.sol | Updated revert testing to use try-catch pattern for library calls |
| test/integration/CrossChainTransfer.t.sol | Integration tests for end-to-end cross-chain transfers (marked as skipped pending MockMailbox) |
| src/xMorseCollateral.sol | Implements collateral contract with correct mirror NFT contract handling |
| src/xMorse.sol | Modified initialization flow to support external treasury setup and fixed NFT fetching |
| src/xDN404Treasury.sol | Removed automatic skipNFT setting from constructor |
| src/libs/LibTransfer.sol | Fixed operator precedence in multicall array size calculation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| import { xDN404Base } from './xDN404Base.sol'; | ||
| import { xDN404Treasury } from './xDN404Treasury.sol'; | ||
| import { xDN404Treasury } from './xDN404Treasury.sol'; // For type casting only |
There was a problem hiding this comment.
The comment 'For type casting only' is misleading since xDN404Treasury is no longer instantiated in this contract but the import is still used for type information. Consider updating to 'For type reference' or 'For type information' to be more accurate.
| import { xDN404Treasury } from './xDN404Treasury.sol'; // For type casting only | |
| import { xDN404Treasury } from './xDN404Treasury.sol'; // For type reference |
|
|
||
| constructor(address _mailbox) xDN404Base(_mailbox) { } | ||
|
|
||
| /// @notice Set treasury address (only callable once during initialization phase) | ||
| function setTreasury(address _treasury) external onlyOwner { | ||
| StorageV1 storage $ = _getStorageV1(); | ||
| require($.initializing, TreasuryAlreadySet()); |
There was a problem hiding this comment.
Using the same error TreasuryAlreadySet() for two different conditions (not initializing vs treasury already set) makes debugging difficult. The first check should use a different error like NotInInitializationPhase() to distinguish between these failure cases.
| constructor(address _mailbox) xDN404Base(_mailbox) { } | |
| /// @notice Set treasury address (only callable once during initialization phase) | |
| function setTreasury(address _treasury) external onlyOwner { | |
| StorageV1 storage $ = _getStorageV1(); | |
| require($.initializing, TreasuryAlreadySet()); | |
| error NotInInitializationPhase(); | |
| constructor(address _mailbox) xDN404Base(_mailbox) { } | |
| /// @notice Set treasury address (only callable once during initialization phase) | |
| function setTreasury(address _treasury) external onlyOwner { | |
| StorageV1 storage $ = _getStorageV1(); | |
| require($.initializing, NotInInitializationPhase()); |
- Introduced xMorseStaking contract for NFT staking with reward distribution. - Updated xMorse constructor to accept a mirror address for improved functionality. - Added interfaces and tests for xMorseStaking to ensure proper staking and reward mechanisms. - Enhanced existing tests to validate integration with the new staking contract.
- Introduced IValidatorRewardDistributor interface for claiming operator rewards. - Updated distributeRewards function to allow both owner and operator to distribute rewards, with automatic claiming from the ValidatorRewardDistributor if configured. - Added functions to set and retrieve the ValidatorRewardDistributor and validator addresses. - Implemented events for tracking updates to the validator reward distributor and validator address. - Created VariableRewardTest to validate reward distribution with variable amounts and multiple users.
- Deleted xDN404Treasury contract and its associated ABI file. - Updated xMorse and related contracts to remove dependencies on the treasury. - Adjusted tests to reflect the removal of treasury functionality and ensure proper NFT handling. - Enhanced xMorse to manage NFT mappings directly without treasury involvement.
No description provided.