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: update contract HypERC20CollateralSaving #2499

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

vinhyenvodoi98
Copy link

@vinhyenvodoi98 vinhyenvodoi98 commented Jul 9, 2023

Description

I try to implement HypERC20CollateralSaving that can send token to saving vault (erc ERC4626)

Drive-by changes

i create wrap_route from goerli to fuji

ERC20

Code

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.9;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MyToken is ERC20 {
    constructor() ERC20("DAI", "DAI") {}

    function mint(address to, uint256 amount) public {
        _mint(to, amount);
    }
}

Address

0xf4089c897f93060F3004B7160f798E9Fd0eE916b

ERC4626

Code

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import "https://github.com/Rari-Capital/solmate/blob/main/src/mixins/ERC4626.sol";

contract TokenVault is ERC4626 {
    // a mapping that checks if a user has deposited the token
    mapping(address => uint256) public shareHolder;

    constructor(
        ERC20 _asset,
        string memory _name,
        string memory _symbol
    ) ERC4626(_asset, _name, _symbol) {}

    /**
     * @notice function to deposit assets and receive vault tokens in exchange
     * @param _assets amount of the asset token
     */
    function _deposit(uint _assets) public {
        // checks that the deposited amount is greater than zero.
        require(_assets > 0, "Deposit less than Zero");
        // calling the deposit function from the ERC-4626 library to perform all the necessary functionality
        deposit(_assets, msg.sender);
        // Increase the share of the user
        shareHolder[msg.sender] += _assets;
    }

    /**
     * @notice Function to allow msg.sender to withdraw their deposit plus accrued interest
     * @param _shares amount of shares the user wants to convert
     * @param _receiver address of the user who will receive the assets
     */
    function _withdraw(uint _shares, address _receiver) public {
        // checks that the deposited amount is greater than zero.
        require(_shares > 0, "withdraw must be greater than Zero");
        // Checks that the _receiver address is not zero.
        require(_receiver != address(0), "Zero Address");
        // checks that the caller is a shareholder
        require(shareHolder[msg.sender] > 0, "Not a share holder");
        // checks that the caller has more shares than they are trying to withdraw.
        require(shareHolder[msg.sender] >= _shares, "Not enough shares");
        // Calculate 10% yield on the withdrawal amount
        uint256 percent = (10 * _shares) / 100;
        // Calculate the total asset amount as the sum of the share amount plus 10% of the share amount.
        uint256 assets = _shares + percent;
        // calling the redeem function from the ERC-4626 library to perform all the necessary functionality
        redeem(assets, _receiver, msg.sender);
        // Decrease the share of the user
        shareHolder[msg.sender] -= _shares;
    }

    // returns total number of assets
    function totalAssets() public view override returns (uint256) {
        return asset.balanceOf(address(this));
    }

    // returns total balance of user
    function totalAssetsOfUser(address _user) public view returns (uint256) {
        return asset.balanceOf(_user);
    }
}

Address

0xE32343749686f759AB771A8098672327c2837902

Wrap-route

Artifacts

[
  {
    "chainId": 5,
    "name": "DAI",
    "symbol": "DAI",
    "decimals": 18,
    "type": "collateral",
    "address": "0xf4089c897f93060F3004B7160f798E9Fd0eE916b",
    "hypCollateralAddress": "0x676dA6A9c6Bfe164B96DFB9771547c675cd29B63",
    "isNft": false,
    "erc4626": "0xE32343749686f759AB771A8098672327c2837902"
  }
]

Test

Step 1

i sent 10DAI from goerli to fuji

https://explorer.hyperlane.xyz/message/0x81da02aaea4942a7cd492e77d9e858cf96e2f68ec911203468f79fd95d59ce88

before i add 10 DAI to vault contract it already have 10DAI and 5 shareToken
So after i add more 10DAI hypCollateralSaving contract will have 5 (eth) shareToken = 10 (eth) DAI

Step 2

Add more 4 (ẹth) DAI direct to vault contract as reward

https://goerli.etherscan.io/tx/0x5b520acf7f41fde32dd369441dc806f092c05d7c1fb0abfc1b2dee10551392d5

i expect that 5 shareToken = 12 (eth)DAI

Step 3

i withdraw 10DAI by send from Fuji -> Goerli
expect that hypCollateralSaving contract have 0.83(eth) shareToken and use still get 2DAI at Goerli

https://explorer.hyperlane.xyz/message/0x1563809b6a42086310099e60cf98000894a0d6d78f54b66412f0fd00edfff0bb

Screen Shot 2023-08-24 at 17 02 00 Screen Shot 2023-08-24 at 17 02 25

Step 4

i will withdraw the rest of token by call function takeProfit
i expect that i can get 2 more DAI

https://goerli.etherscan.io/tx/0x6b4f47c15d59b27e7b99f5b6567c83d3746dab794d96140fb89f2a93621e2cd5

Check balance of my account

Screen Shot 2023-08-24 at 17 07 17

now i have 101.9999 DAI

Related issues

Backward compatibility

Yes

Testing

not yet

@vinhyenvodoi98
Copy link
Author

@nambrot
i try to solve issue 2450 but i don't know I'm really understanding it or not
Please give me some comment

Copy link
Contributor

@nambrot nambrot left a comment

Choose a reason for hiding this comment

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

I believe you need a bit more specific accounting/tracking around the shares in the vault

bytes calldata // no metadata
) internal override {
// redeem token from vault
uint256 amount = savingToken.redeem(_amount, address(this));
Copy link
Contributor

Choose a reason for hiding this comment

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

redeem takes the shares quantity which might be different from the token amount quantity, so I believe you need to track that separately.

* @title Hyperlane ERC20 Token Collateral that wraps an existing ERC20 with remote transfer functionality.
* @author Abacus Works
*/
contract HypERC20CollateralSaving is TokenRouter {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably just extend from HypErc20Collateral

@vinhyenvodoi98 vinhyenvodoi98 force-pushed the Feat/support-erc20-collateral-send-to-erc4626 branch from 5c42e4c to 1f85381 Compare July 17, 2023 09:53
@vinhyenvodoi98
Copy link
Author

@nambrot
I have updated contract, please check it out
because erc20 token will not store in Collateral contract so i add uint256 totalCollateralAssets to control that
when user transfer back it will calculate base on totalCollateralAssets and total of asset token of erc4626 token

@vinhyenvodoi98
Copy link
Author

I have deployed it successfully but still need to test it a bit

) internal override {
uint256 redeemAmount = _convertToRedeemToken(_amount);
// redeem token from vault
uint256 amount = erc4626Token.redeem(
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you are assuming that amount == shares which i don't think has to be true.

Copy link
Author

@vinhyenvodoi98 vinhyenvodoi98 Jul 19, 2023

Choose a reason for hiding this comment

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

@nambrot
it is quite difficult to implement in reverse
for example i send 10Dai from ethereum to polygon.
in polygon I �transfer 3Dai to someone else and he transfer back from polygon to ethereum so who will get the interrest.
if I use withdraw instead of redeem, the profit will be hard to calculate

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what you mean by do it in reverse? Can you not just track the amount per user and the total number of shares, which then gives you the profit at any given time for the owner?


// approve to erc4626
uint256 amount = type(uint256).max; // Maximum value of uint256
wrappedToken.approve(erc4626, amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a good idea?

Copy link
Author

Choose a reason for hiding this comment

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

because we move the token to erc4626 so we should approve it instead of approving it every time

@vinhyenvodoi98 vinhyenvodoi98 force-pushed the Feat/support-erc20-collateral-send-to-erc4626 branch from 7a5582f to d1808dc Compare July 22, 2023 22:37
@vinhyenvodoi98
Copy link
Author

@nambrot
I think most reasonable way is we will use 2 variables, asset and share.
Users can only take profit when the number of assets = 0

@yorhodes yorhodes requested a review from aroralanuk as a code owner November 16, 2023 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants