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

Yield-earning Warp Routes with ERC4626 #3076

Merged
merged 36 commits into from
Mar 13, 2024
Merged

Conversation

ltyu
Copy link
Contributor

@ltyu ltyu commented Dec 18, 2023

Description

Integrates ERC4626 compatible vaults. This PR assumes that the vault asset is the same as wrappedToken

  • _transferFromSender() deposits into the vault
  • _transferTo() withdraws from the vault
  • sweep() redeems excess shares for the vault asset
  • Uses internal assetDeposited store to keep track of assets deposited. Used to calculate excess shares to be withdrawn by owner
  • Makes minor changes to existing test files to allow new tests to inherit

Drive-by changes

Related issues

Backward compatibility

Yes

Testing

Manual/Unit Tests

Copy link

changeset-bot bot commented Dec 18, 2023

⚠️ No Changeset found

Latest commit: e9ed058

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ltyu ltyu marked this pull request as ready for review December 18, 2023 23:23
@ltyu ltyu force-pushed the yield-warp-routes branch from fd9c494 to 9ca1b4e Compare December 19, 2023 15:17
@ltyu ltyu force-pushed the yield-warp-routes branch from 9ca1b4e to 0f505b8 Compare December 19, 2023 15:19
@avious00
Copy link
Contributor

hey @ltyu still working on this?

@ltyu
Copy link
Contributor Author

ltyu commented Jan 15, 2024

hey @ltyu still working on this?

This is complete. Waiting on team approval

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work!

solidity/test/isms/OPStackIsm.t.sol Outdated Show resolved Hide resolved
solidity/test/token/HypERC20CollateralVaultDeposit.t.sol Outdated Show resolved Hide resolved
solidity/test/token/HypERC20CollateralVaultDeposit.t.sol Outdated Show resolved Hide resolved
Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work!

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work!

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work!

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work!

@ltyu ltyu requested a review from yorhodes January 17, 2024 23:42
Comment on lines +42 to +45
function _depositIntoVault(uint256 _amount) internal {
assetDeposited += _amount;
vault.deposit(_amount, address(this));
}

Check warning

Code scanning / Slither

Unused return Medium

Comment on lines +19 to +25
constructor(
ERC4626 _vault,
address _mailbox
) HypERC20Collateral(_vault.asset(), _mailbox) {
vault = _vault;
wrappedToken.approve(address(vault), type(uint256).max);
}

Check warning

Code scanning / Slither

Unused return Medium

Comment on lines +64 to +67
function _withdrawFromVault(uint256 _amount, address _recipient) internal {
assetDeposited -= _amount;
vault.withdraw(_amount, _recipient, address(this));
}

Check warning

Code scanning / Slither

Unused return Medium

Comment on lines +72 to +81
function sweep() external onlyOwner {
uint256 excessShares = vault.maxRedeem(address(this)) -
vault.convertToShares(assetDeposited);
uint256 assetsRedeemed = vault.redeem(
excessShares,
owner(),
address(this)
);
emit ExcessSharesSwept(excessShares, assetsRedeemed);
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

@avious00
Copy link
Contributor

@yorhodes could you DM lee about the steps to merge

@ltyu ltyu enabled auto-merge (squash) March 13, 2024 15:56
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Merging #3076 (e9ed058) into main (8ea0bde) will increase coverage by 0.42%.
Report is 1 commits behind head on main.
The diff coverage is 92.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3076      +/-   ##
==========================================
+ Coverage   68.02%   68.45%   +0.42%     
==========================================
  Files          99      100       +1     
  Lines        1032     1046      +14     
  Branches      107      108       +1     
==========================================
+ Hits          702      716      +14     
+ Misses        284      283       -1     
- Partials       46       47       +1     
Components Coverage Δ
core 56.25% <ø> (ø)
hooks 68.30% <0.00%> (-0.49%) ⬇️
isms 65.94% <ø> (ø)
token 62.50% <100.00%> (+4.08%) ⬆️
middlewares 80.18% <ø> (ø)

@ltyu ltyu merged commit b379336 into hyperlane-xyz:main Mar 13, 2024
33 of 35 checks passed
@ltyu ltyu deleted the yield-warp-routes branch March 13, 2024 21:15
yorhodes added a commit that referenced this pull request Mar 22, 2024
### Description
Integrates ERC4626 compatible vaults. This PR assumes that the vault
`asset` is the same as `wrappedToken`
- `_transferFromSender()` deposits into the vault
- `_transferTo()` withdraws from the vault
- `sweep()` redeems excess shares for the vault asset
- Uses internal `assetDeposited` store to keep track of assets
deposited. Used to calculate excess shares to be withdrawn by owner
- Makes minor changes to existing test files to allow new tests to
inherit

### Drive-by changes

<!--
Are there any minor or drive-by changes also included?
-->

### Related issues


- Implements #2450


### Backward compatibility
Yes

### Testing
Manual/Unit Tests

---------

Co-authored-by: Yorke Rhodes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants