manifoldfinance / mevETH2

mevETH LST Protocol - Repo has migrated see link
https://github.com/MEV-Protocol/meveth
27 stars 2 forks source link

Redemption Ratio #8

Closed sambacha closed 1 year ago

sambacha commented 1 year ago

For cream to mev eth

sandybradley commented 1 year ago

Perhaps adding a function to MevEth to redeem cream tokens for mevETH like:

    /// @notice Redeem Cream staked eth tokens for mevETH at a fixed ratio
    /// @param creamAmount The amount of Cream tokens to redeem
    function redeemCream(uint256 creamAmount) external {
        require(creamAmount > 0, "Amount must be greater than 0");

        // Calculate the equivalent mevETH to be redeemed based on the ratio
        uint256 mevEthAmount = creamAmount * creamToMevEthRatioNumerator / creamToMevEthRatioDenominator;

        // Transfer Cream tokens from the sender to the burn address
        creamToken.safeTransferFrom(msg.sender, address(0), creamAmount);

        // Convert the shares to assets and update the fraction elastic and base
        assets = convertToAssets(mevEthAmount);

        fraction.elastic += uint128(assets);
        fraction.base += uint128(mevEthAmount);

        // Mint the equivalent mevETH
        _mint(msg.sender, mevEthAmount);

        // Emit event
        emit CreamRedeemed(msg.sender, creamAmount, mevEthAmount);
    }

    // Event emitted when Cream tokens are redeemed for mevETH
    event CreamRedeemed(address indexed redeemer, uint256 creamAmount, uint256 mevEthAmount);
sandybradley commented 1 year ago

RE: Validator registration: Can these numbers not be set at initialisation? e.g.

WagyuStaker

    /// @notice Construction sets authority, MevEth, and deposit contract addresses
    /// @param _authority The address of the controlling admin authority
    /// @param _depositContract The address of the beacon deposit contract
    /// @param _mevEth The address of the mevETH contract
    /// @param _initialValidators number of cream validators taken-over
    constructor(address _authority, address _depositContract, address _mevEth, uint256 _initialValidators) Auth(_authority) {
        MEV_ETH = _mevEth;
        BEACON_CHAIN_DEPOSIT_CONTRACT = IBeaconDepositContract(_depositContract);
        beneficiary = _authority;
        validators = _initialValidators;
        record.totalDeposited = uint128(_initialValidators * VALIDATOR_DEPOSIT_SIZE); 
    }

MevEth

    /// @notice Initializes the MevEth contract, setting the staking module and share vault addresses.
    /// @param initialShareVault The initial share vault set during initialization.
    /// @param initialStakingModule The initial staking module set during initialization.
    /// @param initialAssets represents cream eth taker
    /// @dev This function can only be called once and is protected by the onlyAdmin modifier.
    function init(address initialShareVault, address initialStakingModule, uint128 initialAssets) external onlyAdmin {
        // Revert if the initial share vault or staking module is the zero address.
        if (initialShareVault == address(0)) {
            revert MevEthErrors.ZeroAddress();
        }

        if (initialStakingModule == address(0)) {
            revert MevEthErrors.ZeroAddress();
        }

        // Revert if the contract has already been initialized.
        if (initialized) {
            revert MevEthErrors.AlreadyInitialized();
        }

        // Update state variables and emit event to notify offchain listeners that the contract has been initialized.
        initialized = true;
        mevEthShareVault = initialShareVault;
        stakingModule = IStakingModule(initialStakingModule);
        emit MevEthInitialized(initialShareVault, initialStakingModule);

        fraction.elastic = initialAssets;
        fraction.base = initialAssets;
    }
ControlCplusControlV commented 1 year ago

Alright so I think I have a way to do this, we take a pure migration function from @sandybradley mention above

    /// @notice Redeem Cream staked eth tokens for mevETH at a fixed ratio
    /// @param creamAmount The amount of Cream tokens to redeem
    function redeemCream(uint256 creamAmount) external {
        require(creamAmount > 0, "Amount must be greater than 0");

        // Calculate the equivalent mevETH to be redeemed based on the ratio
        uint256 mevEthAmount = creamAmount * creamToMevEthRatioNumerator / creamToMevEthRatioDenominator;

        // Transfer Cream tokens from the sender to the burn address
        creamToken.safeTransferFrom(msg.sender, address(0), creamAmount);

        // Convert the shares to assets and update the fraction elastic and base
        assets = convertToAssets(mevEthAmount);

        fraction.elastic += uint128(assets);
        fraction.base += uint128(mevEthAmount);

        // Mint the equivalent mevETH
        _mint(msg.sender, mevEthAmount);

        // Emit event
        emit CreamRedeemed(msg.sender, creamAmount, mevEthAmount);
    }

    // Event emitted when Cream tokens are redeemed for mevETH
    event CreamRedeemed(address indexed redeemer, uint256 creamAmount, uint256 mevEthAmount);

with some objective base conversion amount, then we introduce GhostStaker.sol which is just a staking module which lets us mass register validators (using a VALIDATOR_DEPOSIT_SIZE of 0 ether) so that we can complete the initial migration, then immediately upgrade to a deployed instance of WagyuStaker

sandybradley commented 1 year ago

I think this is now the only road block to production. Seems like there are 2 parts:

sandybradley commented 1 year ago

An alternative for the validator migration, could be a function in WagyuStaker e.g.

    function batchMigrate(IStakingModule.ValidatorData[] calldata batchData) external onlyAdmin {
        uint256 length = batchData.length;
        // Update the contract balance and validator count
        unchecked {
            record.totalDeposited += uint128(length * VALIDATOR_DEPOSIT_SIZE);
            validators += length;
        }
        for (uint256 i = 0; i < length; i++) {​
            IStakingModule.ValidatorData memory data = batchData[i];
            // Emit an event inidicating a new validator has been registered, allowing for offchain listeners to track the validator registry
            emit NewValidator(data.operator, data.pubkey, data.withdrawal_credentials, data.signature, data.deposit_data_root);
        }
    }
sandybradley commented 1 year ago

Redemption rate should reflect our current total staked eth balance / eth staked Which is hopefully slightly better than the current market rate image

ControlCplusControlV commented 1 year ago

An alternative for the validator migration, could be a function in WagyuStaker e.g.

    function batchMigrate(IStakingModule.ValidatorData[] calldata batchData) external onlyAdmin {
        uint256 length = batchData.length;
        // Update the contract balance and validator count
        unchecked {
            record.totalDeposited += uint128(length * VALIDATOR_DEPOSIT_SIZE);
            validators += length;
        }
        for (uint256 i = 0; i < length; i++) {​
            IStakingModule.ValidatorData memory data = batchData[i];
            // Emit an event inidicating a new validator has been registered, allowing for offchain listeners to track the validator registry
            emit NewValidator(data.operator, data.pubkey, data.withdrawal_credentials, data.signature, data.deposit_data_root);
        }
    }

Hmm, this is an interesting approach, I guess do we want migration to be a one-time thing or a continuous process?

sandybradley commented 1 year ago

@sambacha any comments?

I think it can be either way. Advantages and disadvantages for both. I think it depends if any future migrations are expected and also event requirements for migration. For example a batch event of just public keys could be easier to set up at contract construction. 2 setups are see are:

Option 1: pure function migration and redemption

Advantages: no querky setup or contract balance altercation

Disadvantages: do these functions introduce attack vectors or unexpected consequences?

https://github.com/manifoldfinance/mevETH2/blob/968eae505e9484998890ca471b66f938e95fce7b/src/MevEth.sol#L701-L730

https://github.com/manifoldfinance/mevETH2/blob/968eae505e9484998890ca471b66f938e95fce7b/src/WagyuStaker.sol#L148-L163

Option 2: set balances and batch event at contract construction

Advantages: no balance update functions required might leave less room for attack vectors

Disadvantages: batch event will add expense to already expensive setup. timing and accuracy of initial balances matter

eg MevEth constructor

    /// @param creamToken_ Cream staked eth token address
    /// @param creamToMevEthPercent_ cream to MevEth Percent redemtion e.g. 106 == 106 % (or 1 CRETH2 = 1.06 MevEth)
    /// @param initialBase represents num validators * 32 ether
    constructor(
        address authority,
        address weth,
        address layerZeroEndpoint,
        address creamToken_,
        uint8 creamToMevEthPercent_,
        uint256 initialBase
    )
        OFTWithFee("Mev Liquid Staked Ether", "mevETH", 18, 8, authority, layerZeroEndpoint)
    {
        WETH9 = WETH(payable(weth));
        creamToken = ERC20(creamToken_);
        creamToMevEthPercent = creamToMevEthPercent_;
        fraction.elastic = initialBase;
        fraction.base = initialBase;
    }

In this scenario creamRedeem would not update balances, redeeming 1:1 for meveth as eg

/// @notice Redeem Cream staked eth tokens for mevETH at a fixed ratio
    /// @param creamAmount The amount of Cream tokens to redeem
    function redeemCream(uint256 creamAmount) external {
        if (_isZero(creamAmount)) revert MevEthErrors.ZeroValue();

        // Calculate the equivalent mevETH to be redeemed based on the ratio
        uint256 mevEthAmount = creamAmount * uint256(creamToMevEthPercent) / 100;

        // Transfer Cream tokens from the sender to the burn address
        creamToken.safeTransferFrom(msg.sender, address(0), creamAmount);

        // Convert the shares to assets and update the fraction elastic and base
        uint256 assets = convertToAssets(mevEthAmount);

        // Mint the equivalent mevETH
        _mint(msg.sender, mevEthAmount);

        // Emit event
        emit CreamRedeemed(msg.sender, creamAmount, mevEthAmount);
    }

    // Event emitted when Cream tokens are redeemed for mevETH
    event CreamRedeemed(address indexed redeemer, uint256 creamAmount, uint256 mevEthAmount);

Wagyu constructor:

    /// @param _initialValidatorsPubKeys list of public keys for migrated validators
    constructor(address _authority, address _depositContract, address _mevEth, bytes[] calldata _initialValidatorsPubKeys) Auth(_authority) {
        mevEth = _mevEth;
        BEACON_CHAIN_DEPOSIT_CONTRACT = IBeaconDepositContract(_depositContract);

        uint256 length = _initialValidatorsPubKeys.length;
        // Update the contract balance and validator count
        unchecked {
            record.totalDeposited += uint128(length * VALIDATOR_DEPOSIT_SIZE);
            validators += length;
        }
        for (uint256 i = 0; i < length; i++) {​
            // Emit an event inidicating a new validator has been migrated
            emit MigrateValidator(_initialValidatorsPubKeys[i]);
        }
    }
ControlCplusControlV commented 1 year ago

@sandybradley I would prefer an approach closer to Option 1 as imo its the best, because dealing with the "proper" conversion rate during migration time (accounting for current share prices) makes me nervous about any potential attack vectors this may create, even if it does potentially leave us vulnerable in a scenario where we manage to accrue enough slashing penalties to drop below the redemption ratio (if this happens token is fucked for other reasons though)

I would ask that we have a test which loads batchData from a json file and actually performs real migrations using a forked scenario (pick a block and some creamEth whales) to ensure this will work properly. @mevpig can provide some info on our current validator data for the batchData.

I'm more than happy to write any part of this approach or help in any way needed, but I know you have a draft PR already open so not sure if you wanted to make the changes there. Lmk if you have differing thoughts, or any questions

sandybradley commented 1 year ago

I'll be traveling this week so might be better for you to start a new PR and delete mine.