Skip to content

Conversation

@0xIryna
Copy link

@0xIryna 0xIryna commented Jul 7, 2025

Proposed changes:

  • add SwapFacility support
  • remove wrapWithPermit as it will be replaced with SwapFacility.swapInMWithPermit

Notes:

  • integration tests needs to be updated to use SwapFacility contract after it's deployed
  • unwrap transfers M to msg.sender (SwapFacility) rather than recipient to be consistent with MExtension contract

@github-actions
Copy link

github-actions bot commented Jul 7, 2025

LCOV of commit 49b8821 during Forge Coverage #623

Summary coverage rate:
  lines......: 99.2% (355 of 358 lines)
  functions..: 100.0% (74 of 74 functions)
  branches...: 95.8% (23 of 24 branches)

Files changed coverage rate:
                                 |Lines       |Functions  |Branches    
  Filename                       |Rate     Num|Rate    Num|Rate     Num
  =====================================================================
  src/WrappedMToken.sol          |21.7%    230| 0.0%    50|    -      0

@0xIryna 0xIryna requested review from PierrickGT and toninorair July 7, 2025 05:34
* @param amount_ The amount of M deposited.
*/
function _wrap(address account_, address recipient_, uint240 amount_) internal {
function _wrap(address recipient_, uint240 amount_) internal {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit picking here: worth keeping
_wrap(address account, address recipient, uint256 amount)
and _unwrap(address account, uint256 amount) the same as in MExtension

@toninorair toninorair self-requested a review July 10, 2025 22:37
@deluca-mike
Copy link
Contributor

deluca-mike commented Jul 31, 2025

There does not seem to be a reason or this PR as the SwapFacility could have already interacted with the contract with no issues. Also, this PR makes WrappedM more centralized, costly, and risky for current and future holders since:

  • wraps and unwraps cost more due to having to transfer tokens to and from the SwapFacility as an intermediary step
  • any existing holders or future recipients of WrappedM that are not, or cease to be, "approved swappers" can no longer unwrap, since the only entry point is SwapFacility.swapOutM which is gated by an approved swapper check
  • all existing holders or recipients of WrappedM will not be able to unwrap if the WrappedM ceases to be an approved earner

Perhaps such features are needed by a handful of 3rd party extensions with their own criteria and centralization needs, but hopefully M0's first party extension can remain as decentralized as possible.

Anyway, just my 2 cents as an outsider.

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.

5 participants