naddison36 commented 4 months ago

Contract Changes

Added a withdrawal queue to the OETH Vault which includes requestWithdrawal, claimWithdrawal and addWithdrawalQueueLiquidity.
Any function that transfers WETH to or from the vault modified so the withdrawal queue is prioritised. This includes allocate, mint, redeem, depositToStrategy, withdrawFromStrategy, withdrawAllFromStrategy, withdrawAllFromStrategies and swapCollateral.
redeem will only work if there is enough WETH liquidity in the Vault to satisfy all outstanding withdrawal requests.
There needs to be a minimum of 10 minute between request and claim. Once the liquidity is available in the vault after the 10 minutes, the user can claim.
OETH Vault's allocate changed to use the OETH total supply rather than calculating the vault's total assets when calculating the vault buffer.
To prevent yield theft, mints over the rebaseThreshold (1 OETH) will call the Dripper to collect any streamed rewards to that point. The minter's OETH is calculated after the rewards are collected. This prevents someone minting with a large amount and then withdrawing 10 minutes later collecting most of the yield from the Dripper.

Dependencies

https://github.com/OriginProtocol/origin-dollar/pull/2015

Interfaces

    struct WithdrawalQueueMetadata {
        // cumulative total of all withdrawal requests included the ones that have already been claimed
        uint128 queued;
        // cumulative total of all the requests that can be claimed including the ones that have already been claimed
        uint128 claimable;
        // total of all the requests that have been claimed
        uint128 claimed;
        // index of the next withdrawal request starting at 0
        uint128 nextWithdrawalIndex;
    }

    struct WithdrawalRequest {
        address withdrawer;
        bool claimed;
        // Amount of oTokens to redeem
        uint128 amount;
        // cumulative total of all withdrawal requests including this one.
        // this request can be claimed when this queued amount is less than or equal to the queue's claimable amount.
        uint128 queued;
    }

    event WithdrawalRequested(
        address indexed _withdrawer,
        uint256 indexed _requestId,
        uint256 _amount,
        uint256 _queued
    );
    event WithdrawalClaimed(
        address indexed _withdrawer,
        uint256 indexed _requestId,
        uint256 _amount
    );
    event WithdrawalClaimable(uint256 _claimable, uint256 _newClaimable);

    /**
     * @notice Request an asynchronous withdrawal of WETH in exchange for OETH.
     * The OETH is burned on request and the WETH is transferred to the withdrawer on claim.
     * This request can be claimed once the withdrawal queue's `claimable` amount
     * is greater than or equal this request's `queued` amount.
     * There is no minimum time or block number before a request can be claimed. It just needs
     * enough WETH liquidity in the Vault to satisfy all the outstanding requests to that point in the queue.
     * OETH is converted to WETH at 1:1.
     * @param _amount Amount of OETH to burn.
     * @param requestId Unique ID for the withdrawal request
     * @param queued Cumulative total of all WETH queued including already claimed requests.
     */
    function requestWithdrawal(uint256 _amount)
        external
        returns (uint256 requestId, uint256 queued);

    /**
     * @notice Claim a previously requested withdrawal once it is claimable.
     * This request can be claimed once the withdrawal queue's `claimable` amount
     * is greater than or equal this request's `queued` amount and 30 minutes has passed.
     * If the requests is not claimable, the transaction will revert with `Queue pending liquidity`.
     * If the request is not older than 30 minutes, the transaction will revert with `Claim delay not met`.
     * OETH is converted to WETH at 1:1.
     * @param _requestId Unique ID for the withdrawal request
     * @return amount Amount of WETH transferred to the withdrawer
     */
    function claimWithdrawal(uint256 requestId)
        external
        returns (uint256 amount);

    /**
     * @notice Claim a previously requested withdrawals once they are claimable.
     * This requests can be claimed once the withdrawal queue's `claimable` amount
     * is greater than or equal each request's `queued` amount and 30 minutes has passed.
     * If one of the requests is not claimable, the whole transaction will revert with `Queue pending liquidity`.
     * If one of the requests is not older than 30 minutes,
     * the whole transaction will revert with `Claim delay not met`.
     * @param _requestIds Unique ID of each withdrawal request
     * @return amounts Amount of WETH received for each request
     * @return totalAmount Total amount of WETH transferred to the withdrawer
     */
    function claimWithdrawals(uint256[] memory requestIds)
        external
        returns (uint256[] memory amounts, uint256 totalAmount);

    /// @notice Global metadata for the withdrawal queue including:
    /// queued - cumulative total of all withdrawal requests included the ones that have already been claimed
    /// claimable - cumulative total of all the requests that can be claimed including the ones already claimed
    /// claimed - total of all the requests that have been claimed
    /// nextWithdrawalIndex - index of the next withdrawal request starting at 0
    function withdrawalQueueMetadata()
        external
        view
        returns (VaultStorage.WithdrawalQueueMetadata memory);

    /// @notice Mapping of withdrawal request indices to the user withdrawal request data
    function withdrawalRequests(uint256 requestId)
        external
        view
        returns (VaultStorage.WithdrawalRequest memory);

    /// @notice Collects harvested rewards from the Dripper as WETH then
    /// adds WETH to the withdrawal queue if there is a funding shortfall.
    /// @dev is called from the Native Staking strategy when validator withdrawals are processed.
    /// It also called before any WETH is allocated to a strategy.
    function addWithdrawalQueueLiquidity() external;

Test Chains

The OETH Vault on Holesky has been upgraded to include the withdrawal queue 0x19d2bAaBA949eFfa163bFB9efB53ed8701aA5dD9

Tenderly testnet OETH ARM is an older fork of mainnet.

Tenderly testnet OETH ARM 2 is a newer fork of mainnet with the last Vault code.

Processes

requestWithdrawal

62488c55

claimWithdrawal

d11c27ca

Deployment

Holesky

The Holesky deploy script to upgrade the OETHVault is contracts/deploy/holesky/017_upgrade_vault.js

# uncomment DEPLOYER_PK and GOVERNOR_PK in the .env file for the Holesky deployer PK

yarn run deploy:holesky

echo "module.exports = [
\"0x94373a4919B3240D86eA41593D5eBa789FEF3848\", // WETH
]" > vault-args.js

npx hardhat --network holesky verify --contract contracts/vault/OETHVaultCore.sol:OETHVaultCore --constructor-args vault-args.js 0xB716b6a92B77cDF2c5d0fd46eba05838E30f77c1

npx hardhat --network holesky verify --contract contracts/vault/OETHVaultAdmin.sol:OETHVaultAdmin --constructor-args vault-args.js 0xdF96E53406F6E6873B20E996d7fcB8e1f61ecc5c

Mainnet

The deploy script for the OETH Withdrawal Queue is contracts/deploy/mainnet/103_oeth_withdraw_queue.js.

Testing

Unit Tests

The OETH Vault unit tests are in contracts/test/vault/oeth-vault.js.

yarn run test

Fork Tests

The mainnet OETH Vault fork tests are in contracts/test/vault/oeth-vault.mainnet.fork-test.js

# In one terminal
yarn run node

# In another terminal
yarn run test:fork

Holesky using Hardhat

npx hardhat snapVault --network holesky
npx hardhat rebase --network holesky
npx hardhat snapStaking --index 2 --network holesky
npx hardhat queueLiquidity --network holesky

npx hardhat depositWETH --amount 64 --network holesky
npx hardhat approve --symbol WETH --amount 10000 --spender 0x39254033945AA2E4809Cc2977E7087BEE48bd7Ab --network holesky
npx hardhat mint --asset WETH --amount 64 --network holesky
npx hardhat snapVault --network holesky

npx hardhat requestWithdrawal --amount 30 --network holesky
npx hardhat claimWithdrawal --request-id 1 --network holesky

npx hardhat balance --symbol WETH --network holesky
npx hardhat transfer --symbol WETH --amount 4 --to 0x19d2bAaBA949eFfa163bFB9efB53ed8701aA5dD9 --network holesky

Security

If you made a contract change, make sure to complete the checklist below before merging it in master.

Refer to our documentation for more details about contract security best practices.

Contract change checklist:

[ ] Code reviewed by 2 reviewers.
[ ] Copy & paste code review security checklist below this checklist.
[ ] Unit tests pass
[ ] Slither tests pass with no warning
[ ] Echidna tests pass if PR includes changes to OUSD contract (not automated, run manually on local)

github-actions[bot] commented 4 months ago

	Warnings
:warning:	:eyes: This PR needs at least 2 reviewers

Generated by :no_entry_sign: dangerJS against 0324e4146c0036ccd73c2ce67fa881bc36f422c4

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.47%. Comparing base (d4c84aa) to head (8c25150).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2062 +/- ## ========================================== - Coverage 60.20% 59.47% -0.74% ========================================== Files 69 70 +1 Lines 3365 3499 +134 Branches 664 688 +24 ========================================== + Hits 2026 2081 +55 - Misses 1336 1415 +79 Partials 3 3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

openzeppelin-code[bot] commented 4 months ago

OETH withdrawal queue

Generated at commit: 0324e4146c0036ccd73c2ce67fa881bc36f422c4

🚨 Report Summary

Severity Level Results

Contracts Critical
High
Medium
Low
Note
Total 3
3
0
18
42
66

Dependencies Critical
High
Medium
Low
Note
Total 0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	3 3 0 18 42 66
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

DanielVF commented 3 months ago

The counters are beautiful in this PR. Nice way to solve the fairness issues.

Thinking in invariants:

queue.queued >= queue.claimable => queue.claimed

DanielVF commented 3 months ago

This does basically add instant 1:1 redeems in certain situations. Fortunately our vault side mint math is now very simple, and vault side burn math is simple too.

The real question then is if there are any possible problems in the OETH token itself that would make mint->burn or burn->mint a profitable cycle.

notion-workspace[bot] commented 3 months ago

Code