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.

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

feat: smart M bridging #29

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

feat: smart M bridging #29

wants to merge 3 commits into from

Conversation

0xIryna
Copy link
Collaborator

@0xIryna 0xIryna commented Nov 26, 2024

Proposed changes:

  • add transferWrappedMToken function to transfer a generic Wrapped M token by specifying source and destination Wrapped token addresses. Under the hood Wrapped M token is unwrapped to M token on the source, M token is bridged and wrapped back on the destination. transferWrappedMToken function can be used to transfer M token extensions or convert from one wrapper on the source to another on the destination.
  • add transferSmartMToken function to transfer Smart M token cross-chain. It's a simplified version of transferWrappedMToken function that doesn't take source and destination token addresses, as Smart M token address is set in the constructor.
  • 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.
  • add setRemoteSmartMToken function to store Smart M Token address on the remote chains.
  • modify _receiveMToken to handle M Token wrapping on the destination. If wrapping fails, M token is transferred to he the recipient.

Challenges:

Due to the current Wormhole NTT design it's not possible to utilize NTTManager transfer for transferring Wrapped tokens and as a result some of the functionality is duplicated.

Next Steps and Questions:

  • decide whether we need both transferWrappedMToken and transferSmartMToken at this point.
  • decide on events. Do we need separate events for wrapped tokens or should MTokenSent and MTokenReceived be modified?
  • do we want to support M to Smart M bridging?

Copy link

github-actions bot commented Nov 26, 2024

Changes to gas cost

Generated at commit: aaa86938f4f4e82fa19c6af996d9bcaa5cb9134d, compared to commit: ddf583b9bef971752ec1360f9b089e6fefa9c526

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
SpokePortal attestationReceived +11,238 ❌ +13.11%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
SpokePortal 5,663,657 (+504,223) attestationReceived
chainId
currentIndex
initialize
mToken
mode
registrar
setPeer
setTransceiver
transfer
54,255 (+296)
337 (-22)
1,151 (+1)
127,584 (-22)
279 (-44)
309 (-66)
305 (-66)
54,216 (+66)
188,472 (+1)
137,278 (+466)
+0.55%
-6.13%
+0.09%
-0.02%
-13.62%
-17.60%
-17.79%
+0.12%
+0.00%
+0.34%
96,964 (+11,238)
337 (-22)
1,151 (+1)
127,584 (-22)
279 (-44)
309 (-66)
305 (-66)
54,216 (+66)
188,472 (+1)
137,278 (+466)
+13.11%
-6.13%
+0.09%
-0.02%
-13.62%
-17.60%
-17.79%
+0.12%
+0.00%
+0.34%
111,983 (+34,151)
337 (-22)
1,151 (+1)
127,584 (-22)
279 (-44)
309 (-66)
305 (-66)
54,216 (+66)
188,472 (+1)
137,278 (+466)
+43.88%
-6.13%
+0.09%
-0.02%
-13.62%
-17.60%
-17.79%
+0.12%
+0.00%
+0.34%
151,184 (+36,364)
337 (-22)
1,151 (+1)
127,584 (-22)
279 (-44)
309 (-66)
305 (-66)
54,216 (+66)
188,472 (+1)
137,278 (+466)
+31.67%
-6.13%
+0.09%
-0.02%
-13.62%
-17.60%
-17.79%
+0.12%
+0.00%
+0.34%
11 (+2)
1 (0)
1 (0)
14 (+2)
1 (0)
1 (0)
1 (0)
14 (+2)
14 (+2)
1 (0)
HubPortal 5,875,545 (+424,647) attestationReceived
currentIndex
disableEarning
enableEarning
initialize
mode
registrar
sendMTokenIndex
sendRegistrarKey
sendRegistrarListStatus
setPeer
transfer
54,321 (+273)
841 (+72)
2,476 (+3)
2,634 (+71)
127,584 (-22)
309 (-22)
305 (-22)
3,134 (+5)
5,976 (0)
6,469 (-22)
54,216 (+66)
134,273 (+491)
+0.51%
+9.36%
+0.12%
+2.77%
-0.02%
-6.65%
-6.73%
+0.16%
0.00%
-0.34%
+0.12%
+0.37%
82,780 (+337)
1,630 (+71)
10,147 (+5)
23,606 (+61)
127,584 (-22)
309 (-22)
305 (-22)
37,092 (+59)
37,556 (+54)
38,218 (+32)
54,216 (+66)
134,273 (+491)
+0.41%
+4.55%
+0.05%
+0.26%
-0.02%
-6.65%
-6.73%
+0.16%
+0.14%
+0.08%
+0.12%
+0.37%
92,267 (+359)
1,486 (+71)
12,704 (+5)
29,580 (+57)
127,584 (-22)
309 (-22)
305 (-22)
37,092 (+59)
37,556 (+54)
38,218 (+32)
54,216 (+66)
134,273 (+491)
+0.39%
+5.02%
+0.04%
+0.19%
-0.02%
-6.65%
-6.73%
+0.16%
+0.14%
+0.08%
+0.12%
+0.37%
92,267 (+359)
2,708 (+71)
12,704 (+5)
29,580 (+57)
127,584 (-22)
309 (-22)
305 (-22)
71,051 (+114)
69,137 (+109)
69,968 (+87)
54,216 (+66)
134,273 (+491)
+0.39%
+2.69%
+0.04%
+0.19%
-0.02%
-6.65%
-6.73%
+0.16%
+0.16%
+0.12%
+0.12%
+0.37%
4 (0)
4 (0)
4 (0)
9 (0)
20 (0)
1 (0)
1 (0)
2 (0)
2 (0)
2 (0)
20 (0)
1 (0)

Copy link

github-actions bot commented Nov 26, 2024

LCOV of commit d04ff4d during Forge Coverage #142

Summary coverage rate:
  lines......: 83.8% (268 of 320 lines)
  functions..: 37.9% (61 of 161 functions)
  branches...: 75.4% (43 of 57 branches)

Files changed coverage rate:
                                 |Lines       |Functions  |Branches    
  Filename                       |Rate     Num|Rate    Num|Rate     Num
  =====================================================================
  src/HubPortal.sol              | 100%     39|50.0%    18| 100%      5
  src/Portal.sol                 |97.4%     77|44.4%    36|84.6%     13
  src/SpokePortal.sol            | 100%     33|50.0%    12| 100%     10
  src/governance/Migrator.sol    | 0.0%     16| 0.0%    12| 0.0%      3
  src/libs/PayloadEncoder.sol    | 100%     47|50.0%    20| 100%      1
  src/libs/SafeCall.sol          | 100%      3|50.0%     2| 100%      1

Comment on lines +159 to +160
// Pre-compute the expected SpokeSmartMToken proxy address.
address expectedSmartMTokenProxy_ = ContractHelper.getContractFrom(deployer_, _SPOKE_SMART_M_TOKEN_PROXY_NONCE);
Copy link
Member

Choose a reason for hiding this comment

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

Would be best to deploy it before the SpokePortal.
Kind of tricky cause we need the SpokeVault and SpokePortal address but we can pre-compute these.
I think it's best to deploy the contracts that rely on nonces first and then we deploy the ones that rely on Create3.


/// @inheritdoc IPortal
function setRemoteSmartMToken(uint16 remoteChainId_, bytes32 smartMToken_) external onlyOwner {
remoteSmartMToken[remoteChainId_] = smartMToken_;
Copy link
Member

Choose a reason for hiding this comment

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

Should revert if trying to set it for the local network/chainId.

@0xIryna 0xIryna requested a review from toninorair November 27, 2024 21:21
@0xIryna 0xIryna force-pushed the feat/smart-m-bridging branch from 12cab5a to 73d72bb Compare December 2, 2024 01:41
@0xIryna 0xIryna force-pushed the feat/smart-m-bridging branch from 9176d84 to d04ff4d Compare December 2, 2024 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants