Dopex - 0xnev's results

A rebate system for option writers in the Dopex Protocol.

General Information

Platform: Code4rena

Start Date: 21/08/2023

Pot Size: $125,000 USDC

Total HM: 26

Participants: 189

Period: 16 days

Judge: GalloDaSballo

Total Solo HM: 3

Id: 278

League: ETH

Dopex

Findings Distribution

Researcher Performance

Rank: 10/189

Findings: 9

Award: $1,730.05

QA:
grade-b
Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Awards

96.3292 USDC - $96.33

Labels

bug
3 (High Risk)
satisfactory
upgraded by judge
sufficient quality report
edited-by-warden
duplicate-549

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L605 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L669 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L673 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L27 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L253 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L274 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L255 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L275 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1172 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1181 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L281

Vulnerability details

Impact

All prices are assume to return 8 decimal precision, but the oracle clearly return all prices in 18 decimals as seen in RdpxEthOracle.sol. The two price functions used are getLpPriceInETH() and getRdpxPriceInEth().

getRdpxPriceInEth():

RdpxEthOracle.sol#L250

/// @notice Returns the price of rDPX in ETH
/// @return price price of rDPX in ETH in 1e18 decimals
function getRdpxPriceInEth() external view override returns (uint price) {
    require(
        blockTimestampLast + timePeriod + nonUpdateTolerance >
            block.timestamp,
        "RdpxEthOracle: UPDATE_TOLERANCE_EXCEEDED"
    );

    // @audit returns prices in 1e18 decimals as seen by using `amountIn` of 1e18
->  price = consult(token0, 1e18);

    require(price > 0, "RdpxEthOracle: PRICE_ZERO");
}
  • This will cause a heavy overestimation of rdpx amount when converting amount of rDPX to equivalent wETH amount.
  • This will cause a heavy underestimation of rDPX amount when converting wETH to rDPX

getLpPriceInETH():

RdpxEthOracle.sol#L217

/// @dev Returns the price of LP in ETH in 1e18 decimals
function getLpPriceInEth() external view override returns (uint) {
    uint totalSupply = pair.totalSupply();
    (uint r0, uint r1, ) = pair.getReserves();
    uint sqrtK = HomoraMath.sqrt(r0.mul(r1)).fdiv(totalSupply); // in 2**112
    uint px0 = getETHPx(token0); // in 2**112
    uint px1 = 2 ** 112; // in 2**112
    // fair token0 amt: sqrtK * sqrt(px1/px0)
    // fair token1 amt: sqrtK * sqrt(px0/px1)
    // fair lp price = 2 * sqrtK * sqrt(px0 * px1)
    // split into 2 sqrts multiplication to prevent uint overflow (note the 2**112)
    uint lpPriceIn112x112 = sqrtK
        .mul(2)
        .mul(HomoraMath.sqrt(px0))
        .div(2 ** 56)
        .mul(HomoraMath.sqrt(px1))
        .div(2 ** 56);

    //@audit returns price in 18 decimals as noticed by the multiplication of 1e18
->  return (lpPriceIn112x112 * 1e18) >> 112;
}
  • Not used in the codebase other than in view function getLpPrice(), which is in turn used in getLpTokenBalanceInWeth()
  • But if used, will heavily overestimate Lp token balance in terms of wETH

In the RdpxV2Core.sol core contract, it can cause the following:

  • DoS of bonding mechanism (regardless of type) due to _transfer() reverting from underflow here (assuming APP puts are not allowed to be purchased)
  • DoS of upperDepeg() decollaterization, if slippage not specified, since slippage (minOut) will be overestimated here
  • Dos of lowerDepeg recollaterization, as price check here will always revert here

In the PerpetualAtlanticVault.sol contract, it can cause the following:

  • Strike price is calculated with a decimal of 18 precision, leading to required collateral being overestimated here and than the subsequent check fails. This can DoS the purchase of APP options and bonding mechanism as well since initial purchase of APP options is done through bonding.

In the ReLPContract.sol contract, it can cause the following:

  • Assuming _transfer() and purchase() did not revert, the reLP process will revert due to overestimated amount of rDPX by a factor of 10 to be swapped out using half of wETH here. This reverts the whole reLP process and consequently, the bonding process.

In the PerpetualAtlanticVaultLP.sol contract, it can cause the following:

  • Hugely overestimated total collateral value in PerpetualAtlanticVaultLP.convertToShares() here, leading to rounding down of shares to 0 and DoSing option writers from depositing collateral into LP vault due to a check reverting here and affecting APP option process.

Tools Used

Manual Analysis

Recommendation

In the following LOC, change precision of operators, division or multiplication from 1e8 to 1e18

In the function ReLPContract.reLP(), change precision dividing from 1e16 to 1e26

In the function RdpxV2Core.calculateBondCost(), multiply by 1e18 Instead of DEFAULT_PRECISION in the following LOC:

Assessed type

Context

#0 - c4-pre-sort

2023-09-09T05:08:16Z

bytes032 marked the issue as duplicate of #549

#1 - c4-pre-sort

2023-09-12T05:17:37Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-20T18:27:49Z

GalloDaSballo marked the issue as satisfactory

#3 - c4-judge

2023-10-20T18:28:12Z

GalloDaSballo changed the severity to 2 (Med Risk)

#4 - c4-judge

2023-10-20T18:28:21Z

GalloDaSballo changed the severity to 3 (High Risk)

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L964 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L975-L990

Vulnerability details

Impact

Delegates can withdraw collateral anytime if there is no existing active collaterals that are locked due to them currently being used for delegate bonding. When delegates send wETH to core contract via RdpxV2Core.addToDelegate(), the totalWethDelegated is incremented.

RdpxV2Core.sol#L964

function addToDelegate(
    uint256 _amount,
    uint256 _fee
) external returns (uint256) {
    _whenNotPaused();
    // fee less than 100%
    _validate(_fee < 100e8, 8);
    // amount greater than 0.01 WETH
    _validate(_amount > 1e16, 4);
    // fee greater than 1%
    _validate(_fee >= 1e8, 8);

    IERC20WithBurn(weth).safeTransferFrom(msg.sender, address(this), _amount);

    Delegate memory delegatePosition = Delegate({
        amount: _amount,
        fee: _fee,
        owner: msg.sender,
        activeCollateral: 0
    });
    delegates.push(delegatePosition);

    // add amount to total weth delegated
    // @audit totalWethDelegated incremented
->  totalWethDelegated += _amount;

    emit LogAddToDelegate(_amount, _fee, delegates.length - 1);
    return (delegates.length - 1);
}

At any point of time, delegate can withdraw non active collateral via RdpxV2Core.withdraw() that is not currently bonded via delegate bonding. However, when withdrawing, totalWethDelegated is not decremented.

RdpxV2Core.sol#L975-L990

function withdraw(
    uint256 delegateId
) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
}

This esentially allows users to manipulate totalWethDelegated by calling addToDelegated() and then immediately calling withdraw(). Then, since totalWethDelegated can only be decreased when bondWithDelegate() is called, totalWethDelegated will never be decremented. This can be performed multiple times to increase totalWethDelegated to a very large number, causing sync() to always revert.

This can cause the following scenarios:

  • sync() can revert and even when it does not revert, backing reserves of wETH will forever be underestimated as withdrawn wETH not decremented in totalWethDelegated. This causes a desync between backing reserves amount and actual amount of collateral in core contract.
<br/>
  • This means ReLPContract.reLP() can potentially reverts since it calls RdpxV2Core.sync(), potentially breaking reLP mechanism
<br/>
  • Similary, UniV3LiquidityAMO.addLiquidity(), UniV3LiquidityAMO.removeLiquidity() and UniV3LiquidityAMO.swap() also calls RdpxV2Core.sync() and can cause potential reverts, breaking AMO market operations.

Proof of Concept

This essentially allows any user to always make sync() revert by repeatedly calling addToDelegate() and withdraw() that results in a very large totalWethDelegated. A simplified version is shown below:

Add this test to /tests/rdpxV2-core/Unit.t.sol and run forge test --mt testWithdrawDoesNotSyncReserves -vv. Make sure to import console.log with import "forge-std/console.sol";.

function testWithdrawDoesNotSyncReserves() external {
    // 1. Transfer wETH and sync backing reserves, assume backing reserves have 5e18 wETH
    weth.transfer(address(rdpxV2Core), 5 * 1e18);
    rdpxV2Core.sync();
    (, uint256 wethBackingReserve, ) = rdpxV2Core.getReserveTokenInfo("WETH");
    console.log("wETH collateral amount:" , wethBackingReserve);

    // 2. Delegate 10 wETH, backing reserve now has 15 wETH
    uint256 delegateId = rdpxV2Core.addToDelegate(10e18, 1e8);
    
    // 3. Check delegated wETH amount before withdrawal
    uint totalWethDelegatedBefore = rdpxV2Core.totalWethDelegated();
    console.log("==================================================================");
    console.log("Total wETH delegated before withdrawal:", totalWethDelegatedBefore);
    
    // 4. Immediately calls withdraw to withdraw 10 wETH 
    rdpxV2Core.withdraw(delegateId);

    // 5. Sync backing reserves
    vm.expectRevert();
    // Revert due to underflow
    rdpxV2Core.sync();

    // 6. Check total weth delegated after withdrawal, does not change
    // Should have decremented back to 0
    uint totalWethDelegatedAfter = rdpxV2Core.totalWethDelegated();
    console.log("==================================================================");
    console.log("Total wETH delegated after withdrawal:", totalWethDelegatedAfter);
}
Running 1 test for tests/rdpxV2-core/Unit.t.sol:Unit
[PASS] testWithdrawDoesNotSyncReserves() (gas: 235320)
Logs:
  wETH collateral amount: 5000000000000000000
  ==================================================================
  Total wETH delegated before withdrawal: 10000000000000000000
  ==================================================================
  Total wETH delegated after withdrawal: 10000000000000000000

Tools Used

Manual Analysis, Foundry

Recommendation

Decrement totalWethDelegated after withdrawal and make sure to sync() backing reserves after to ensure backing reserves are always up to date.

function withdraw(
    uint256 delegateId
) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

+   totalWethDelegated -= amountWithdrawn;
+   sync();

    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
}

Assessed type

Context

#0 - c4-pre-sort

2023-09-08T07:39:36Z

bytes032 marked the issue as duplicate of #2186

#1 - c4-pre-sort

2023-09-08T07:39:42Z

bytes032 marked the issue as high quality report

#2 - c4-judge

2023-10-20T17:55:32Z

GalloDaSballo changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-10-20T17:55:46Z

GalloDaSballo marked the issue as satisfactory

#4 - c4-judge

2023-10-21T07:38:54Z

GalloDaSballo changed the severity to 3 (High Risk)

#5 - c4-judge

2023-10-21T07:45:16Z

GalloDaSballo marked the issue as partial-50

#6 - c4-judge

2023-10-21T07:45:33Z

GalloDaSballo marked the issue as full credit

#7 - GalloDaSballo

2023-10-21T07:45:46Z

Full credit despite no mention of lowerDepeg because of mention of AMOs operations

#8 - c4-judge

2023-10-21T09:07:38Z

GalloDaSballo marked the issue as satisfactory

Findings Information

🌟 Selected for report: LokiThe5th

Also found by: 0xnev, Evo, volodya

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sufficient quality report
duplicate-2130

Awards

909.7371 USDC - $909.74

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L221

Vulnerability details

Impact

PerpetualAtlanticVaultLP.sol#L221

  function _convertToAssets(
    uint256 shares
  ) internal view virtual returns (uint256 assets, uint256 rdpxAmount) {
    uint256 supply = totalSupply;
    return
      (supply == 0)
        ? (shares, 0)
        : (
          shares.mulDivDown(totalCollateral(), supply),
          shares.mulDivDown(_rdpxCollateral, supply)
        );
  }

In PerpetualAtlanticVaultLP._convertToAsset(), the wrong totalSupply variable is used which can result in wrong redemption of shares when converting shares to relevant assets values.

Proof of Concept

You can see here that even after shares is minted when depositing liquidity, collateral returns is still the same. This could always allow 1:1 redemption of shares to wETH and always returns 0 rDPX, when redeeming shares, which is not intended.

function testRedeemPreview() external {
    (uint collateral, uint rdpxPerShare) = vaultLp.redeemPreview(1 ether);
    console.log(collateral, rdpxPerShare);

    rdpx.mint(address(1), 10 ether);
    weth.mint(address(1), 1 ether);
    deposit(1 ether, address(1));

    (uint collateral1, uint rdpxPerShare1) = vaultLp.redeemPreview(1 ether);
    console.log(collateral1, rdpxPerShare1);

}

Recommendation

totalSupply() should be used instead, which represents the total supply of shares minted.

Assessed type

Context

#0 - c4-pre-sort

2023-09-13T07:24:24Z

bytes032 marked the issue as duplicate of #2130

#1 - c4-pre-sort

2023-09-14T09:17:47Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-15T18:06:26Z

GalloDaSballo marked the issue as satisfactory

#3 - c4-judge

2023-10-20T19:41:50Z

GalloDaSballo changed the severity to 2 (Med Risk)

Findings Information

🌟 Selected for report: 0Kage

Also found by: 0xnev, ABA, peakbolt, pep7siup, zzebra83

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
duplicate-1956

Awards

491.258 USDC - $491.26

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L333

Vulnerability details

Impact

In PerpetualAtlanticVault.settle(), users that have provided liquidity in vaultLP for APP options can redeem their rDPX and wETH collateral once options are settled. However, once price of rDPX increases above 75% of strike price (which is very likely), these APP options are never settled due to this check in settle():

PerpetualAtlanticVault.sol#L333

function settle(
    uint256[] memory optionIds
)
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
{
    _whenNotPaused();
    _isEligibleSender();

    updateFunding();

    for (uint256 i = 0; i < optionIds.length; i++) {
        uint256 strike = optionPositions[optionIds[i]].st
        uint256 amount = optionPositions[optionIds[i]].amount;

        // check if strike is ITM
        // @audit if strike price (75% of rDPX price at time of minting)
        // is smaller than current price, reverts
->      _validate(strike >= getUnderlyingPrice(), 7);

        ethAmount += (amount * strike) / 1e8;
        rdpxAmount += amount;
        optionsPerStrike[strike] -= amount;
        totalActiveOptions -= amount;

        // Burn option tokens from user
        _burn(optionIds[i]);

        optionPositions[optionIds[i]].strike = 0;
        ...
        ...
}

Consider the following flow:

  1. If strike price is not in the money when settle() is called, some weth and rdpx will never be unlocked for liquidity providers to redeem their shares (This can happen for a sudden increase in rDPX price within a week and price remains high without decreasing), since settle() will revert
<br/>
  1. Redemption of collateral for current epoch will not be allowed, since rdpx and wETH collateral is not unlocked yet via unlockLiquidity() and addRdpx() while liquidity providers still retain shares. Here _rdpxCollateral remains at 0, signifiying no rdpx collateral unlocked, and _activeCollateral remains constant since it is not unlocked yet.
<br/>
  1. The next epoch starts, LP deposits to provide liquidity using deposit().
<br/>
  1. More shares are minted, now assume strike price of options minted in current epoch is ITM, then settle() will not revert. Consequently, _rdpxCollateral and _activeCollateral will be increased and decreased respectively.
<br/>
  1. Now, liquidity provider will be able to redeem shares minted for current epoch liquidity providers before _rdpxCollateral underflows, assuming previously non-settled options still cannot be settled as prices remain high. They will also compete with other liquidity providers where previously minted shares that are stuck attempts to redeem their shares.

Unless price of rDPX decrease till strike price is smaller again, previous APP options can never be settled leaving collateral to be potentially forever stuck for some liquidity providers, depending on who redeems their shares first whenever APP options are settled by treasury

Proof of Concept

underflow in _rdpxCollateral, user cannot redeem shares, of 1.89e18 wETH and 1e18 rDPX.

function testCollateralStuck() external {
    // Actors:
    // Liquidity provider 1: address(1)

    // 1. Setup token balances for LPs
    rdpx.mint(address(1), 10 ether);
    weth.mint(address(1), 3 ether);

    // 2. Provide Liquidity
    deposit(1 ether, address(1));

    // 3. Purchase APP at strike price of 0.0075 gwei (75% * 0.01 gwei)
    priceOracle.updateRdpxPrice(0.010 gwei);
    (, uint256 id) = vault.purchase(1 ether, address(this)); // purchases for epoch 1; send 0.05 weth premium
    uint256[] memory optionIds = new uint256[](1);
    optionIds[0] = id;

    // 4. Settle APP at current price of 0.02 gwei
    // Reverts as strike price is smaller than current price (0.0075 gwei  < 0.02 gwei)
    skip(86400); // expire
    priceOracle.updateRdpxPrice(0.02 gwei);
    vm.expectRevert(
        abi.encodeWithSelector(
        PerpetualAtlanticVault.PerpetualAtlanticVaultError.selector,
        7
        )
    );
    (uint256 ethAmount, uint256 rdpxAmount) = vault.settle(optionIds);

    // 5. Check current available wETH and rDPX balances for redemption in vaultLP for epoch 1
    uint collateralBalanceinLPBefore = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPBefore = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralBefore = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPBefore, rdpxCollateralBalanceinLPBefore, totalAvailableCollateralBefore);

    // 6. Next epoch starts, LP provides liquidity 
    deposit(1 ether, address(1));

    // 7. Purchase APP at strike price of 0.015 gwei (75% * 0.02 gwei), assuming rDPX price has increased for epoch 2
    priceOracle.updateRdpxPrice(0.02 gwei);
    (, uint256 id2) = vault.purchase(1 ether, address(this)); // purchases for epoch 2; send 0.05 weth premium

    // 8. Settle APP at current price of 0.015 gwei, assuming rDPX price has dropped
    uint256[] memory optionIds2 = new uint256[](1);
    optionIds2[0] = id2;
    priceOracle.updateRdpxPrice(0.015 gwei);
    (uint256 ethAmount2, uint256 rdpxAmount2) = vault.settle(optionIds2);

    // 9. Check current available wETH and rDPX balances for redemption in vaultLP for epoch 2
    (uint collateral1, uint rdpxPerShare1) = vaultLp.redeemPreview(2 ether);
    console.log(collateral1, rdpxPerShare1);
    uint collateralBalanceinLPAfter = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPAfter = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralAfter = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPAfter, rdpxCollateralBalanceinLPAfter, totalAvailableCollateralAfter);

    // 10. Attempt to redeem all shares (2e18) for unlocked rDPX and wETH collateral but fails
    uint256 balance = vaultLp.balanceOf(address(1));
    console.log(balance);
    vm.expectRevert();
    (uint256 ethAmount3, uint256 rdpxAmount3) = vaultLp.redeem(balance, address(1), address(1));

    // 11. Simulate strike price decreases to 0.0075 gwei, now previous epoch options can be settled
    // This is the only way liquidity provider can get back his collateral
    priceOracle.updateRdpxPrice(0.0075 gwei);
    uint256 id3 = id;
    uint256[] memory optionIds3 = new uint256[](1);
    optionIds3[0] = id3;
    (uint256 ethAmount4, uint256 rdpxAmount4) = vault.settle(optionIds3);
    uint collateralBalanceinLPAfter1 = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPAfter1 = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralAfter1 = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPAfter1, rdpxCollateralBalanceinLPAfter1, totalAvailableCollateralAfter1);

    // 12. Liquidity provider still cannot redeem shares as `_totalCollateral` is now
    // lower then expected due to loss incurred from APP
    (uint collateral2, uint rdpxPerShare2) = vaultLp.redeemPreview(2 ether);
    console.log(collateral2, rdpxPerShare2);
    // (uint256 ethAmount5, uint256 rdpxAmount5) = vaultLp.redeem(balance, address(1), address(1));
}

Tools Used

Manual Analysis, Foundry

Recommendation

It is crucial to rewrite options for optionIds where it cannot be settled when the new premium is calculated based on current rDPX mark price to prevent price changes/stability from locking collateral provide to write APP.

This can be done by exposing another admin function where associated optionIds from previous epoch not settled has its strike price rewrittened. This should be performed before calculating funding via calculateFunding() which is then followed by payFunding().

function rewrite (
    uint256[] memory optionIds
)
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
{

    uint256 currentPrice = getUnderlyingPrice(); // price of underlying wrt collateralToken
    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price

    uint256 amount;
    uint256 strikeBefore;
    for (uint256 i = 0; i < optionIds.length; i++) {
      strikeBefore = optionPositions[optionIds[i]].strike; 
      optionPositions[optionIds[i]].strike = strike;
      uint256 amount += optionPositions[optionIds[i]].amount;
      optionsPerStrike[strikeBefore] -= amount
    }

    optionsPerStrike[strike] += amount;
}

Assessed type

Context

#0 - c4-pre-sort

2023-09-15T12:19:46Z

bytes032 marked the issue as duplicate of #1012

#1 - c4-pre-sort

2023-09-15T12:19:55Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-12T09:31:08Z

GalloDaSballo changed the severity to QA (Quality Assurance)

#3 - nevillehuang

2023-10-21T20:58:42Z

Hi @GalloDaSballo, I think this should be a duplicate of #1956, as it is also describing about the same root cause of not being able to forfeit OTM options

#4 - c4-judge

2023-10-25T07:06:24Z

This previously downgraded issue has been upgraded by GalloDaSballo

#5 - c4-judge

2023-10-25T07:06:51Z

GalloDaSballo marked the issue as not a duplicate

#6 - c4-judge

2023-10-25T07:07:30Z

GalloDaSballo marked the issue as duplicate of #1956

#7 - c4-judge

2023-10-30T20:01:56Z

GalloDaSballo marked the issue as satisfactory

#8 - GalloDaSballo

2023-10-30T20:02:54Z

Leaving #1956 as primary due to using the lingo of forfeiture

Findings Information

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
duplicate-1032

Awards

90.6302 USDC - $90.63

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L286-L295 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L264 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L283

Vulnerability details

Impact

The combination of lack of slippage and deadline checks can expose the reLP process to loss of funds via MEV sandwich attacks when when re-adding liquidity here. Additionally, the removal of liquidity here and swap from wETH to rDPX before re-adding liquidity here is also exposed to MEV sandwich attacks due to a lack of deadline check. (Note, this 2 instances of using block.timestamp as deadline is not identified as an instance in the automated report here).

Tools Used

Manual Analysis

Recommendation

Consider separating the reLP process from bonding process and trigger it manually in the core contract by the admin so that slippage and deadline can be manually inputted.

Assessed type

Context

#0 - c4-pre-sort

2023-09-07T12:54:26Z

bytes032 marked the issue as duplicate of #1259

#1 - c4-pre-sort

2023-09-11T07:51:24Z

bytes032 marked the issue as sufficient quality report

#2 - c4-pre-sort

2023-09-11T07:53:17Z

bytes032 marked the issue as duplicate of #1032

#3 - c4-judge

2023-10-15T19:21:17Z

GalloDaSballo marked the issue as satisfactory

Awards

7.8372 USDC - $7.84

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
duplicate-269

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L964

Vulnerability details

Impact

Users can call RdpxV2Core.addToDelegate() to delegate wETH for bonding in core contract. Here, totalWethDelegated is incremented. This value is subsequently decremented when delegatee calls bondWithDelegate().

RdpxV2Core.sol#L964

  function addToDelegate(
    uint256 _amount,
    uint256 _fee
  ) external returns (uint256) {
    _whenNotPaused();
    // fee less than 100%
    _validate(_fee < 100e8, 8);
    // amount greater than 0.01 WETH
    _validate(_amount > 1e16, 4);
    // fee greater than 1%
    _validate(_fee >= 1e8, 8);

    IERC20WithBurn(weth).safeTransferFrom(msg.sender, address(this), _amount);

    Delegate memory delegatePosition = Delegate({
      amount: _amount,
      fee: _fee,
      owner: msg.sender,
      activeCollateral: 0
    });
    delegates.push(delegatePosition);

    // add amount to total weth delegated
    // @audit totalWethDelegated added but `sync()` is not called to sync backing reserves
->  totalWethDelegated += _amount;

    emit LogAddToDelegate(_amount, _fee, delegates.length - 1);
    return (delegates.length - 1);
  }

However, there is a unsynced backing reserves for wETH when delegates delegate ETH for bonding as sync() is not incremented when addToDelegate() is called. This can potentially cause an overestimation of backing reserves and result in unintentional use of collateral delegated other than being used for delegate bonding, such as when purchasing options (_purchaseOptions()), providing funding for APP options (provideFunding()), settling options (settle()), Lping in curve pool(_stake()) and use of PSM (lowerDepeg()).

Proof of Concept

Consider the following scenario, where there is no bond discount (for simplicity)

  1. Assume that current delegated weth is 2 wETH, represented by totalWethDelegated = 2 wETH. reserves now has 10 - 2 = 8 wETH
  2. Bob calls addToDelegate() and delegates 1 wETH, now totalWethDelegated = 3 wETH, but reserves is still at 8 wETH instead of 7 wETH.
  3. Now at the same time, assume Alice (contract admin) calls provideFunding() to provide funding for APP puts that amounts to 7.5 ETH, this will cause 0.5 ETH delegated by Bob to be wrongly used to provide funding to APP vault.

Tools Used

Manual Analysis

Recommendation

function addToDelegate(
    uint256 _amount,
    uint256 _fee
) external returns (uint256) {
    _whenNotPaused();
    // fee less than 100%
    _validate(_fee < 100e8, 8);
    // amount greater than 0.01 WETH
    _validate(_amount > 1e16, 4);
    // fee greater than 1%
    _validate(_fee >= 1e8, 8);

    IERC20WithBurn(weth).safeTransferFrom(msg.sender, address(this), _amount);

    Delegate memory delegatePosition = Delegate({
        amount: _amount,
        fee: _fee,
        owner: msg.sender,
        activeCollateral: 0
    });
    delegates.push(delegatePosition);

    // add amount to total weth delegated
    totalWethDelegated += _amount;

+   sync();

    emit LogAddToDelegate(_amount, _fee, delegates.length - 1);
    return (delegates.length - 1);
}

Assessed type

Context

#0 - c4-pre-sort

2023-09-15T08:10:37Z

bytes032 marked the issue as duplicate of #269

#1 - c4-pre-sort

2023-09-15T08:10:48Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-15T18:12:11Z

GalloDaSballo marked the issue as satisfactory

Awards

7.8372 USDC - $7.84

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
duplicate-269

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L160-L178

Vulnerability details

Impact

When adding liquidity and removing liquidity via the UniV2AMO using addLiquidity() and removeLiquidity() to univ2 rDPX/ETH pools, the remaining liquidity not used are sent back to core contract via UniV2LiquidityAmo._sendTokensToRdpxV2Core(). Similarly, in swap(), when swapping rDPX to ETH or vice versa, when tokens are sent back to core contract, backing reserves are not updated.

As we can see here, the RdpxV2Core.sync() function that updates backing reserves is missing. Note that this is performed correctly in UniV3LiquidityAmo.sol here.

UniV2LiquidityAmo.sol#L160-L178

function _sendTokensToRdpxV2Core() internal {
    uint256 tokenABalance = IERC20WithBurn(addresses.tokenA).balanceOf(
        address(this)
    );
    uint256 tokenBBalance = IERC20WithBurn(addresses.tokenB).balanceOf(
        address(this)
    );
    // transfer token A and B from this contract to the rdpxV2Core
    IERC20WithBurn(addresses.tokenA).safeTransfer(
        addresses.rdpxV2Core,
        tokenABalance
    );
    IERC20WithBurn(addresses.tokenB).safeTransfer(
        addresses.rdpxV2Core,
        tokenBBalance
    );

    emit LogAssetsTransfered(msg.sender, tokenABalance, tokenBBalance);
}

This will cause less than expected backing reserves, which can lead to underflow when performing various other actions such as when providing funding for APP options (provideFunding()), settling options (settle()).

Tools Used

Manual Analysis

Recommendation

function _sendTokensToRdpxV2Core() internal {
    uint256 tokenABalance = IERC20WithBurn(addresses.tokenA).balanceOf(
        address(this)
    );
    uint256 tokenBBalance = IERC20WithBurn(addresses.tokenB).balanceOf(
        address(this)
    );
    // transfer token A and B from this contract to the rdpxV2Core
    IERC20WithBurn(addresses.tokenA).safeTransfer(
        addresses.rdpxV2Core,
        tokenABalance
    );
    IERC20WithBurn(addresses.tokenB).safeTransfer(
        addresses.rdpxV2Core,
        tokenBBalance
    );

+   IRdpxV2Core(rdpxV2Core).sync();

    emit LogAssetsTransfered(msg.sender, tokenABalance, tokenBBalance);
}

Assessed type

Context

#0 - c4-pre-sort

2023-09-09T03:48:06Z

bytes032 marked the issue as duplicate of #798

#1 - c4-pre-sort

2023-09-09T04:09:24Z

bytes032 marked the issue as duplicate of #269

#2 - c4-pre-sort

2023-09-11T11:58:34Z

bytes032 marked the issue as sufficient quality report

#3 - c4-judge

2023-10-15T18:12:09Z

GalloDaSballo marked the issue as satisfactory

Awards

24.8267 USDC - $24.83

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sufficient quality report
edited-by-warden
duplicate-153

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L277-L284 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L286-L295

Vulnerability details

[H-05] ReLPContract.reLP(): Unused ETH is not returned back to RdpxV2Core and is left stranded in the reLP contract

Impact

In the docs here the reLP process is mentioned as the process in which ETH received from removing liquidity is swapped to rDPX. The process can be seen here, where based on formula and dpxETH bond amount, rDPX to remove is calculated and then subsequently lp to remove is calculated.

Some of the market operations will result in some unused wETH and rDPX. Unused rDPX is safely transferred back to core contract and backing reserves but unused wETH is left stranded in the reLP contract with no form of retrieval, esentially locking it forever. While it could initially be a small amount, it can compound over time as more and more market operations are performed, causing loss of funds from the backing reserves.

This occurs in addLiquidity() here performed during reLP process, where unused wETH that is not used to add liquidity on the univ2 pool is not returned back to RdpxV2Core contract.

Tools Used

Manual Analysis

Recommendation

Return remaining wETH in reLP contract back to RdpxV2Core contract before syncing backing reserves.


  function reLP(uint256 _amount) external onlyRole(RDPXV2CORE_ROLE) {
    // get the pool reserves
    (address tokenASorted, address tokenBSorted) = UniswapV2Library.sortTokens(
      addresses.tokenA,
      addresses.tokenB
    );
    (uint256 reserveA, uint256 reserveB) = UniswapV2Library.getReserves(
      addresses.ammFactory,
      tokenASorted,
      tokenBSorted
    );

    TokenAInfo memory tokenAInfo = TokenAInfo(0, 0, 0);

    // get tokenA reserves
    tokenAInfo.tokenAReserve = IRdpxReserve(addresses.tokenAReserve)
      .rdpxReserve(); // rdpx reserves

    // get rdpx price
    tokenAInfo.tokenAPrice = IRdpxEthOracle(addresses.rdpxOracle)
      .getRdpxPriceInEth();

    tokenAInfo.tokenALpReserve = addresses.tokenA == tokenASorted
      ? reserveA
      : reserveB;

    uint256 baseReLpRatio = (reLPFactor *
      Math.sqrt(tokenAInfo.tokenAReserve) *
      1e2) / (Math.sqrt(1e18)); // 1e6 precision

    uint256 tokenAToRemove = ((((_amount * 4) * 1e18) /
      tokenAInfo.tokenAReserve) *
      tokenAInfo.tokenALpReserve *
      baseReLpRatio) / (1e18 * DEFAULT_PRECISION * 1e2);

    uint256 totalLpSupply = IUniswapV2Pair(addresses.pair).totalSupply();

    uint256 lpToRemove = (tokenAToRemove * totalLpSupply) /
      tokenAInfo.tokenALpReserve;

    // transfer LP tokens from the AMO
    IERC20WithBurn(addresses.pair).transferFrom(
      addresses.amo,
      address(this),
      lpToRemove
    );

    // calculate min amounts to remove
    uint256 mintokenAAmount = tokenAToRemove -
      ((tokenAToRemove * liquiditySlippageTolerance) / 1e8);
    uint256 mintokenBAmount = ((tokenAToRemove * tokenAInfo.tokenAPrice) /
      1e8) -
      ((tokenAToRemove * tokenAInfo.tokenAPrice) * liquiditySlippageTolerance) /
      1e16;

    (, uint256 amountB) = IUniswapV2Router(addresses.ammRouter).removeLiquidity(
      addresses.tokenA,
      addresses.tokenB,
      lpToRemove,
      mintokenAAmount,
      mintokenBAmount,
      address(this),
      block.timestamp + 10
    );

    address[] memory path;
    path = new address[](2);
    path[0] = addresses.tokenB;
    path[1] = addresses.tokenA;

    // calculate min amount of tokenA to be received
    mintokenAAmount =
      (((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
      (((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);

    uint256 tokenAAmountOut = IUniswapV2Router(addresses.ammRouter)
      .swapExactTokensForTokens(
        amountB / 2,
        mintokenAAmount,
        path,
        address(this),
        block.timestamp + 10
      )[path.length - 1];

    (, , uint256 lp) = IUniswapV2Router(addresses.ammRouter).addLiquidity(
      addresses.tokenA,
      addresses.tokenB,
      tokenAAmountOut,
      amountB / 2,
      0,
      0,
      address(this),
      block.timestamp + 10
    );

    // transfer the lp to the amo
    IERC20WithBurn(addresses.pair).safeTransfer(addresses.amo, lp);
    IUniV2LiquidityAmo(addresses.amo).sync();

    // transfer rdpx to rdpxV2Core
    IERC20WithBurn(addresses.tokenA).safeTransfer(
      addresses.rdpxV2Core,
      IERC20WithBurn(addresses.tokenA).balanceOf(address(this))
    );

+   IERC20WithBurn(addresses.tokenB).safeTransfer(
+       addresses.rdpxV2Core,
+       IERC20WithBurn(addresses.tokenB).balanceOf(address(this))
+   )

    IRdpxV2Core(addresses.rdpxV2Core).sync();
  }

Assessed type

Context

#0 - c4-pre-sort

2023-09-07T12:53:25Z

bytes032 marked the issue as duplicate of #1286

#1 - c4-pre-sort

2023-09-11T15:38:12Z

bytes032 marked the issue as sufficient quality report

#2 - c4-judge

2023-10-10T17:52:40Z

GalloDaSballo changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-10-18T12:14:11Z

GalloDaSballo marked the issue as satisfactory

Awards

19.1724 USDC - $19.17

Labels

bug
downgraded by judge
grade-b
QA (Quality Assurance)
sponsor disputed
sufficient quality report
edited-by-warden
Q-42

External Links

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L705-L717

Vulnerability details

Impact

Delegatee with only rDPX can leverage on delegates delegated ETH by calling RdpxV2Core.bondWithDelegate(), where the 25% of the rDPX required for bonding is paid by the delegatee and the other 75% of the ETH required is paid by the delegate. Additionally, there is a initial premium charged for a 25% OTM APP option, but this premium is solely paid for by the delegate. We can see this in RdpxV2Core._bondWithDelegate():

RdpxV2Core.sol#L705-L717

function _bondWithDelegate(
    uint256 _amount,
    uint256 rdpxBondId,
    uint256 delegateId
) internal returns (BondWithDelegateReturnValue memory returnValues) {
    // Compute the bond cost
    // @audit wethRequired here includes initial premium to be paid
->  (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
        _amount,
        rdpxBondId
    );

    // update ETH token reserve
    reserveAsset[reservesIndex["WETH"]].tokenBalance += wethRequired;

    Delegate storage delegate = delegates[delegateId];

    // update delegate active collateral
    _validate(delegate.amount - delegate.activeCollateral >= wethRequired, 5);
    // @audit All of the premium paid by delegate
->  delegate.activeCollateral += wethRequired;

    // update total weth delegated
    totalWethDelegated -= wethRequired;

    // Calculate the amount of bond token to mint for the delegate and user based on the fee
    (uint256 amount1, uint256 amount2) = _calculateAmounts(
        wethRequired,
        rdpxRequired,
        _amount,
        delegate.fee
    );

    // update user amounts
    // ETH token amount remaining after LP for the user
    uint256 bondAmountForUser = amount1;

    // Mint bond token for delegate
    // ETH token amount remaining after LP for the delegate
    uint256 delegateReceiptTokenAmount = _stake(delegate.owner, amount2);

    returnValues = BondWithDelegateReturnValue(
        delegateReceiptTokenAmount,
        bondAmountForUser,
        rdpxRequired,
        wethRequired
    );
    }

Here, the RdpxV2Core.calculateBondCost() will return the wethRequired including premium. This amount is than directly added to the active collateral of the delegate, indicating ALL of the premium is paid for by the delegate. Since bonding via delegate bonding is a 2 party mechanism, the premium paid should also be split accordingly between delegatee and delegate and not solely be covered by the delegate

This means that delegates will have lesser delegated wETH available for bonding due to having to pay for the premium, potentially losing out on some bonds.

While it is true that there is a fee incurred when bonding via delegate bond set by delegate, this fee may end up only covering the premium and will have no additional incentive to the delegate delegating ETH for bonding. Additionally, if fees are set too high to both cover the premium and the incentive towards delegate, it may disincentivize delegatees from utilizing the delegate bonding mechanism, causing dopex to lose out on potential customers using their option products.

Tools Used

Manual Analysis

Recommendation

Split the premium payment in a 75:25 ratio between delegate and delegatee respectively. One way of performing this is by reducing the amount of receipt tokens minted to delegatee and increasing the amount of receipt tokens minted to delegate.

Assessed type

Context

#0 - c4-pre-sort

2023-09-15T12:27:20Z

bytes032 marked the issue as sufficient quality report

#1 - c4-sponsor

2023-09-25T16:13:11Z

witherblock (sponsor) disputed

#2 - witherblock

2023-09-25T16:13:21Z

Design choice

#3 - c4-judge

2023-10-12T11:22:12Z

GalloDaSballo changed the severity to QA (Quality Assurance)

#4 - GalloDaSballo

2023-10-12T11:22:45Z

Valid gotcha, QA seems most appropriate

#5 - c4-judge

2023-10-20T18:18:37Z

GalloDaSballo marked the issue as grade-b

Findings Information

Labels

analysis-advanced
grade-b
sufficient quality report
edited-by-warden
A-13

Awards

90.0998 USDC - $90.10

External Links

1. Summary of Codebase

rDPXV2 primarily focuses on the new synthetic coin dpxETH that is pegged to ETH, which is minted during the bonding process and will be used to earn boosted yields via staking in curve pools. Dopex aims to use dpxETH as a staple collateral token for future Dopex Options Products.

1.1 Bonding Mechanism

The bonding process is where new dpxETH tokens are minted, where a user bonds with the rDPX V2 contract and receives a receipt token. A receipt token represents ETH and dpxETH LP on curve. They can then subsequently redeem back this receipt tokens after bond maturity ends which can then be used to retrieve the dpxETH and ETH associated with amount of receipt tokens minted.

There are 3 ways a user can bond on the Bonding contract; regular bonding, delegate bonding or using a decaying bond.

1.1.1 Regular Bonding

The regular bonding mechanism can be summarised in to 6 detailed steps below:

  1. User transfer required rdpx and weth + the premium for the size of the rDPX provided for an rDPX Atlantic Perpetual PUT option which is 25% OTM after accounting for bond discount, which is dependent on the balance of rDPX in the treasury reserves. (Note that this will be intially set to 12.5%) This is computed by RdpxV2Core.calculateBondCost().
<br/>
  1. Within RdpxV2Core.bond(), the amount of weth required including premium is first transferred to core contract
<br/>
  1. The APP options will then be minted to the core contract via RdpxV2Core._purchaseOptions().
<br/>
  1. After that, the amount of rdpx required to be transferred by user bonding will be performed via RdpxV2Core._transfer(). Here, rdpx transferred by user to core contract is redirected where 50% of the rDPX provided for bonding is burnt from the Treasury Reserve and another 50% is sent to thefeeDistributor as emissions for veDPX holders. These percentages are variable and can be controlled by governance. In addition, discount provided to user + the amount of rdpx sent by user will be sent back to the backing reserve.
<br/>
  1. After transferring of funds by user, the dpxETH will be minted where half of the bond amount worth of dpxETH will be minted to be paired with equaivalent amount of wETH to LP on dpxETH/ETH curve pool. The remaining wETH and rDPX not used for LPing will remain in the backing reserves as backing for dpxETH minted. Once LP is performed, users will be issued the the bond amount with equivalent receipt token amounts, that can subsequently be redeemed after bond maturity (5 days) passes. This is handled in RdpxV2Core._stake()
<br/>
  1. If reLP process is present, the reLP process will be intiated where part of the Uniswap V2 liquidity is removed, then the ETH received is swapped to rDPX. This essentially increases the price of rDPX and also increases the Backing Reserve of rDPX. This is handled by ReLPContract.relp().

1.1.2 Delegate Bonding

Delegate bonding is a system where users come in with ETH and deposit it in a pool (via the Core Contract). They set a fee for the usage of their ETH, where users holding rDPX can use this pool of ETH to bond (using the regular bonding mechanism). This way users can bond with only rdpx or only ETH, allowing both to come together and bond to receive their share of receipt tokens.

The steps for delegate bonding can be summarised below:

  1. Delegates first call RdpxV2Core.addToDelegate() to deposit wETH (minimum amount of 0.01 wETH to avoid spamming) and sets a fee (minimum of 1% but cannot be more than 100%). The totalWethDelegated state variable is updated, which represents the wETH in the core contract that is available for use in delegate bonding.
<br/>
  1. Interested users then call RdpxCore.bondWithDelegate(), supplying the relevant delegateIds, where a similar bonding mechanism to regular bonding is performed via _bondWithDelegate(). The only difference between _bond() and _bondWithDelegate() is that two seperate bonds with receipt token amounts of ratio 75:25 is minted to delegate and delegatee via _stake() after accounting for fees set by delegate. The amount of receipt tokens associated with bonds for delegate and delegatees is computed by RdpxV2Core._calculateAmounts(), called within _bondWithDelegate().

1.1.3 Bonding via Decaying Bonds

Decaying bonds are bonds carrying a specific amount of rDPX minted to users as a form of rebate for losses incurred on the Dopex Options Protocol or simply for incentives. This amount is decided when these decaying bonds are minted to users by the minter.

The steps for bonding via decaying bond can be summarised below:

  1. The minter with the MINTER_ROLE first mints a decaying bond with associated rDPX via RdpxDecayingBonds.mint(), specifying the expiry and amount of rDPX.
<br/>
  1. User with specific bondID of decaying bond minted to them can then call RdpxV2Core.bond() and specify the bondID. This initiates a similar bonding mechanism as regular bonding, with the difference lying in the amount of rDPX and wETH required computed via RdpxV2Core.calculateBondCost() where no bond discount is applied. Subsequently in the RdpxV2Core._transfer() function, since the bonds assigned to them is a form of rebate/incentive, the user do not need to pay the rDPX required for bonding, and thus only deposit to the core contract the wETH required for bonding + premium for the 25% OTM APP.

1.2 Atlantic Perpetual PUTS Options

A perpetual options pools for rDPX options the Core Contract uses for the bonding process. The liquidity for this will be provided via the open market and will be incentivized via DPX earned when shares received from providing collateral is staked. The APP contract follows a epoch system where each epoch is 1 week long.

A detailed summary of how APP system works can be summarised below:

  1. Bond writers deposit collateral required to write options via PerpetualAtlanticVaultLP.deposit() into LP Vault, receiving associated amount of shares based on amount deposited.
<br/>
  1. When bonding via RdpxV2Core.bond()/bondWithDelegate(), 25% OTM APP options is purchased for core contract via RdpxV2Core._purchaseOptions(), which in turn calls PerpetualAtlanticVault.purchase().
<br/>
  1. At the start of each epoch, the contract admin will call PerpetualAtlanticVault.calculateFunding() to calculate the amount of wETH funding required based on new APP puts strike price values minted via bonding as well as existing APP puts not settled.
<br/>
  1. After funding is calculated, admin will call RdpxV2Core.provideFunding(), which in turn calls PerpetualAtlanticVault.payFunding(), and pays the amount of collateral required to write APP puts provided by backing reserve in core contract. This funding is paid by the treasurys backing reserve.
<br/>
  1. Once treasury decides based on market conditions to settle APP options via RdpxV2Core.settle(), collateral associated with relevant optionIds of bond writers will be unlocked for them to redeem.
<br/>
  1. Bond writers can then redeem shares via PerpetualAtlanticVaultLP.redeem() to receive back the associated amount of rDPX and wETH based on amount of shares and total supply of shares, collateral and rdPX collateral available.

Note that PerpetualAtlanticVaultLP.updateFunding() which in turn calls PerpetualAtlanticVaultLP.updateFundingPaymentPointer() is called when actions are performed, which implements a drip wise funding method to transfer required collateral to write APP to PerpetualAtlanticVaultLP for each epoch based on the amount of exisitng and newly minted APP options. It also updates the epoch represented by latestFundingPaymentPointer after each epoch ends.

1.3 Automated Market Operator (AMO)

The automated market operator (AMO) aims to perform market operations of adding liquidity, removing liquidity and swapping within the rDPX/ETH pools but ensuring that dpxETH price peg is not changed. Similar to FRAX, dpxETH cannot be minted out of thin air that can potentially break the break, allowing a stable price peg of dpxETH.

Traditionally, the frax AMO holds 4 key components:

  • Decollateralize - the portion of the strategy which lowers the CR
  • Market operations - the portion of the strategy that is run in equilibrium and doesn't change the CR
  • Recollateralize - the portion of the strategy which increases the CR
  • FXS1559 - a formalized accounting of the balance sheet of the AMO which defines exactly how much FXS can be burned with profits above the target CR

In rDPXV2, the decollaterize and recollaterize components are integrated into the RdpxV2Core.sol core contract represented by the functions RdpxV2Core.upperDepeg() and RdpxV2Core.lowerDepeg() respectively. The equivalent FXS1559 accounting mechanism is also part of the core contract, and the market operations will be performed by the AMOs represented by UniV2LiquidityAmo.sol and UniV3LiquidityAmo.sol.

Here, the CR refers to the collateral ratio of rDPX to peg token of dpxETH, which is wETH. If dpxETH price is above the peg, the CR is lowered via upperDepeg(), dpxETH supply expands like usual, and AMOs keep running.

If the CR is lowered to the point that the peg slips, the AMOs have predefined recollateralize operations via lowerDepeg() which increases the CR. The system recollateralizes just like before as protocol rDPX/ETH LP tokens are redeemed and the CR goes up to return to the peg, thus also allowing continued operation of AMOs.

2. Codebase Quality

Codebase Quality CategoriesComments
TestingUnit test and Integration tests were present using Foundry, allowing ease of testing for PoCs
DocumentationThe documentation for Dopex has some inconsistencies, but overall provides a satisfactory overview of the protocol
OrganizationCodebase is decently organized with clear distinctions between the 9 contracts.

3. Improvements

3.1 Ensure LP logic is consistent with docs

In the protocol docs, it is mentioned that

The full amount of rDPX and 33% of the ETH provided (33% of 75% = 25%) is sent to the Backing Reserve and used to mint dpxETH which is then paired with the rest of the ETH to LP on curve.

But we can see in RdpxV2Core._stake() where the LP logic is performed, the core contract only mints half the amount of dpxETH associated with dpxETH amount to bond and pair with equivalent wETH. This means that there will be remaining wETH left in the backing reserve instead of the amount described in the DoCs where ALL of the wETH is used to LP on curve. Hence it is suggested to ensure the LP logic on curve pool is consistent with docs described.

3.2 Ensure maximum value is implemented when setting/changing sensitive protocol parameters

Add a maximum cap to ensure that sensitive system parameters are not accidentally set to a too large/small value.

  • slippageTolerance: Slippage tolerance RdpxV2Core.setSlippageTolerance()
<br/>
  • bondDiscountFactor: bond discount factor used to control bond discount given to user bonding set via RdpxV2Core.setBondDiscount().

3.3 Consider a brief timelock for delegated wETH

Currently, delegated wETH by delegates can be withdrawn any time, and as such delegates can DoS delegatees trying to delegate bond anytime by withdrawing required wETH collateral. As such, consider implementing a brief timelock to avoid this scenario.

3.4 Consider separating reLP process from bonding process

Since the reLP process involves market operations of swapping, adding and removing liquidity from the rDPX/ETH pools, it might be better to allow admin to manually initiate the reLP process so that they can specify the slippage and deadline required to avoid MEV sandwich attacks.

3.5 Allow partial withdrawal of delegated collateral

Consider allowing partial withdrawal logic for collateral delegated so that users that only wants to withdraw part of the wETH collateral do not have to withdraw and redelegate.

3.6 Ensure that expiry of decaying bond minted is greater than current timestamp

Add a check (expiry > block.timestamp) to avoid setting expiry smaller than current timestamp and resulting in decaying bond not being able to be used.

3.7 Create a separate admin only function to recover dust amounts in PerpetualAtlanticVault

Since the PerpetualAtlanticVault employs a drip wise style funding approach to transfer required collateral to vaultLP, there maybe rounding errors associated with division of timestamps based on when funding rate is updated via PerpetualAtlanticVault._updateFundingRate() evident in the test files here. This can accumulate over time, so consider employing a mechanism to transfer out these dust amounts to the vaultLP.

3.8 Consider using multi-hop swaps in AMOs when performing market operations

Currently, the swap functions in the respective AMOs uses a single hop fixed path swap from rDPX -> wETH or wETH -> rDPX. Rather than trading between a single pool, smart routing could use multiple hops (as many as needed) to ensure that the end result of the swap is the optimal price. This prevents potential leakage of value when swapping by using only a single fixed path.

While currently swapping via uniswap v2 only allows a direct swap from rDPX -> wETH, in the future, if there are plans to launch other pools similar to UniV3 or even other pools pairing (such as with stablecoins), the swap() function will still be restricted to a single path swap. This also ensures other forms of liquidity to perform market operations in the event there is low liquidity in the primary rDPX/wETH pools.

3.9 Consider splitting premium payments between delegate and delegatee during delegate bonding

Currently when bonding via delegate bonding, ALL of the initial premium for the 25% OTM APP is paid for by the delegate. Since bonding via delegate bonding is a 2 party mechanism, the premium paid should also be split accordingly between delegatee and delegate and not solely be covered by the delegate in a similar 75:25 ratio between delegate and delegatee respectively.

While it is true that there is a fee incurred when bonding via delegate bond set by delegate, this fee may end up only covering the premium and will have no additional incentive to the delegate delegating ETH for bonding. Additionally, if fees are set too high to both cover the premium and the incentive towards delegate, it may disincentivize delegatees from utilizing the delegate bonding mechanism, causing dopex to lose out on potential customers using their option products.

4. Centralization Risks

4.1 System Parameters

The following system parameters set only has a non-zero value check but do not have a maximum value cap that should be enforced.

  • rdpxBurnPercentage: rdpx to burn from treasury reserve during bonding set via RdpxV2Core.setRdpxBurnPercentage(). Excessive value can cause too much rdpx to be burned.
<br/>
  • rdpxFeePercentage: Fees sent as emission to veDPX holders during bonding set via RdpxV2Core.setRdpxFeePercentage(). Excessive value can cause too much fees to be sent to veDPX holders
<br/>
  • bondMaturity: Bond maturity set via setBondMaturity(), excessive values can prevent users from redeeming bonds and getting back receipt tokens

4.2 Emergency Withdrawals

  • RdpxV2Core.emergencyWithdraw(): Protocol admin can withdraw ALL of the funds in the core contract backing reserves, griefing users funds
<br/>
  • PerpetualAtlanticVaultLP.emergencyWithdraw(): Protocol admin can withdraw ALL of the initial premium provided by users bonding for APP options minted.

4.3 Pausing Mechanism

There are pause() functions present throughout the codebase where admin can pause ALL users from interacting with the protocol at any time. This includes the following contracts:

  • RdpxV2Bond.sol, RdpxV2Core.sol, DpxEthToken.sol, PerpetualAtlanticVault.sol, RdpxReserve.sol

4.4 Others

The following priviledged functions can cause DoS of important

  • RdpxV2Core.removeAssetFromtokenReserves(): Can remove any assets from interacting with rDPXV2 protocol
<br/>
  • RdpxV2Core.setPricingOracleAddresses(): Can change pricing oracle address anytime, potentially influencing (increase.decrease) price of rDPX and dpxETH.
<br/>
  • RdpxV2Core.settle(): Protocol treasury can withhold settling APP options can lock bond writers collateral.
<br/>
  • RdpxV2Core.provideFunding(): Protocol admin can choose not to provide sufficient funding from backing reserves for APP puts written by bond writers depositing collateral required in vaultLP

5. Time Spent

DayScopeDetails
1-2rDPXV2 Docs LinkReview Docs and understand key components of protocol, namely: bonding, reLP, APP and AMO
3-6rDPXV2 contracts LinkManual audits of contracts with the following flow, RdpxV2Core.sol --> PerpetualAtlanticVault.sol/PerpetualAtlanticVaultLP.sol --> ReLPContract.sol --> UniV2LiquidityAmo.sol/UniV3LiquidityAmo.sol
7-Finish up Analysis report

Time spent:

56 hours

#0 - c4-pre-sort

2023-09-15T15:57:34Z

bytes032 marked the issue as sufficient quality report

#1 - c4-judge

2023-10-20T10:02:22Z

GalloDaSballo marked the issue as grade-b

#2 - nevillehuang

2023-10-21T20:58:23Z

Hi @GalloDaSballo, this may be subjective, but if #1988 is a grade-a report, than i believe this report qualifies for grade-a too. If not, happy to hear your thoughts on why it does not qualify for a grade-a report. I am not a visual learner, so i try to keep everything written to the best of my abilities. Appreciate your huge judging efforts and thanks in advance for reviewing.

#3 - GalloDaSballo

2023-10-25T07:10:20Z

The first half of the report describes a FSM without giving much additional info

The AMO discussion is interesting and is valuable

Half of the recommendations are better sent as QAs

Centralization risk is not particularly well developed

I think the report has some value hence the B

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter