Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat: Wrapped M bridging #34

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Feat: Wrapped M bridging #34

wants to merge 10 commits into from

Conversation

0xIryna
Copy link
Collaborator

@0xIryna 0xIryna commented Jan 16, 2025 •
edited
Loading

Motivation

Provide the ability to bridge both official M^0 Wrapped M token and generic M token extensions using M Portal by unwrapping tokens on the source, bridging M and wrapping them back on the destination. The proposed solution allows flexible bridging and wrapping. E.g., from M to Wrapped M, from M Extension to M, etc.

Proposed changes

  • add a mapping to store the relationship between Wrapped M tokens on local and remote chains. The mapping can be updated only by the owner.
  • add transferWrappedMToken function to transfer a generic Wrapped M token by specifying source token address. Under the hood, the Wrapped M token is unwrapped to M token on the source, M token is bridged and wrapped back on the destination.
  • extend Token Transfer additional payload to include the address of the Wrapped M token on the destination. If that address is zero, M Token on the destination won't be wrapped.
  • modify _receiveMToken to handle M Token wrapping on the destination. If wrapping fails, M token is transferred to the recipient.

Challenges

Due to the current Wormhole NTT design it's not possible to utilize NTTManager functions for transferring Wrapped tokens and as a result some of the functionality from NTTManager contract is duplicated in Portal. In particular, we cannot call NTTManager's transfer or _transferEntryPoint functions inside transferWrappedMToken as they attempt to transfer M token from the msg.sender. We also cannot use NTTManager's _transfer function since we cannot pass additional parameter (destination wrapped token address) to _prepareNativeTokenTransfer function.

Next Steps/Questions:

  • decide on events. Do we need separate events for wrapped tokens or should MTokenSent and MTokenReceived be modified?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 16, 2025 •
edited
Loading

LCOV of commit da911a1 during Forge Coverage #153

Summary coverage rate:
  lines......: 85.0% (289 of 340 lines)
  functions..: 78.0% (64 of 82 functions)
  branches...: 60.5% (23 of 38 branches)

Files changed coverage rate:
                                 |Lines       |Functions  |Branches    
  Filename                       |Rate     Num|Rate    Num|Rate     Num
  =====================================================================
  src/HubPortal.sol              |23.1%     39| 0.0%     9|    -      0
  src/Portal.sol                 |21.3%     94| 106%    18|    -      0
  src/SpokePortal.sol            |20.0%     35| 143%     7|    -      0
  src/libs/PayloadEncoder.sol    |21.3%     47| 0.0%    10|    -      0
  src/libs/SafeCall.sol          |33.3%      3| 0.0%     1|    -      0

@0xIryna 0xIryna marked this pull request as draft January 16, 2025 18:09
PierrickGT
PierrickGT reviewed Jan 17, 2025
View reviewed changes
src/Portal.sol Outdated
IERC20(sourceWrappedToken_).transferFrom(msg.sender, address(this), amount_);

// unwrap Wrapped M token to M Token
amount_ = IWrappedMTokenLike(sourceWrappedToken_).unwrap(address(this), amount_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we will be whitelisting the supported wrapped tokens, it seems safe to rely on the amount returned by unwrap.
Ideally we should retrieve the balance in the contract before, then after and compute the exact amount that was unwrapped.

src/Portal.sol Outdated

uint256 totalPriceQuote_ = _sendMessage(destinationChainId_, refundAddress_, message_);

emit MTokenSent(destinationChainId_, messageId_, msg.sender, recipient_, amount_, index_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should emit the WrappedMTokenSent event for Wrapped tokens.

src/Portal.sol
// unwrap Wrapped M token to M Token
amount_ = IWrappedMTokenLike(sourceWrappedToken_).unwrap(address(this), amount_);

// NOTE: the following code has been adapted from NTT manager `transfer` or `_transferEntryPoint` functions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are not using _transferEntryPoint, shouldn't this function burn M on Spoke chains and lock it on the Hub chain?

src/interfaces/IPortal.sol Outdated
* @param sender The address that bridged the M tokens via the Portal.
* @param recipient The account receiving tokens on destination chain.
* @param amount The amount of tokens.
* @param index The the M token index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param index The the M token index.
* @param index The M token index.

src/Portal.sol Outdated
_mintOrUnlock(address(this), amount_, index_);

// wraps M token and transfers it to the recipient
_wrap(destinationWrappedToken_, recipient_, amount_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should emit the WrappedMTokenReceived event for wrapped tokens.

src/interfaces/IPortal.sol Outdated
Comment on lines 100 to 102
* @param sourceWrappedToken The address of Wrapped M token on the source chain.
* @param destinationChainId The Wormhole destination chain ID.
* @param destinationWrappedToken The address of Wrapped M token on the destination chain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation can be fixed.

src/interfaces/IPortal.sol Outdated
* @notice Sets the address of Wrapped M Token on the remote chain.
* @param sourceWrappedToken The address of Wrapped M token on the source chain.
* @param destinationChainId The Wormhole destination chain ID.
* @param destinationWrappedToken The address of Wrapped M token on the destination chain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation can be fixed.

src/interfaces/IPortal.sol Outdated
/**
* @notice Transfers Wrapped M Token to the destination chain.
* @param amount The amount of tokens to transfer.
* @param sourceWrappedToken The address of the Wrapped M Token of the source chain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param sourceWrappedToken The address of the Wrapped M Token of the source chain.
* @param sourceWrappedToken The address of the Wrapped M Token on the source chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PierrickGT PierrickGT PierrickGT left review comments

@toninorair toninorair Awaiting requested review from toninorair

@deluca-mike deluca-mike Awaiting requested review from deluca-mike

@Oighty Oighty Awaiting requested review from Oighty

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xIryna @PierrickGT