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
Rank: 10/189
Findings: 9
Award: $1,730.05
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: LokiThe5th
Also found by: 0xPsuedoPandit, 0xTiwa, 0xnev, 0xvj, Evo, Jiamin, Juntao, QiuhaoLi, T1MOH, Udsen, circlelooper, crunch, eeshenggoh, gjaldon, hals, josephdara, kutugu, minhtrng, niki, umarkhatab_465
96.3292 USDC - $96.33
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
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()
:
/// @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"); }
getLpPriceInETH()
:
/// @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; }
getLpPrice()
, which is in turn used in getLpTokenBalanceInWeth()
In the RdpxV2Core.sol
core contract, it can cause the following:
_transfer()
reverting from underflow here (assuming APP puts are not allowed to be purchased)upperDepeg()
decollaterization, if slippage not specified, since slippage (minOut
) will be overestimated herelowerDepeg
recollaterization, as price check here will always revert hereIn the PerpetualAtlanticVault.sol
contract, it can cause the following:
In the ReLPContract.sol
contract, it can cause the following:
_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:
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.Manual Analysis
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:
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)
🌟 Selected for report: 0xrafaelnicolau
Also found by: 0x111, 0xCiphky, 0xMosh, 0xWaitress, 0xc0ffEE, 0xkazim, 0xnev, 0xvj, ABAIKUNANBAEV, Aymen0909, Baki, ElCid, HChang26, HHK, Inspex, Jorgect, Kow, Krace, KrisApostolov, LFGSecurity, MiniGlome, Nyx, QiuhaoLi, RED-LOTUS-REACH, Talfao, Toshii, Vagner, Viktor_Cortess, Yanchuan, _eperezok, asui, atrixs6, bart1e, bin2chen, carrotsmuggler, chaduke, chainsnake, deadrxsezzz, degensec, dethera, dimulski, dirk_y, ether_sky, gizzy, glcanvas, grearlake, gumgumzum, halden, hals, kodyvim, koo, ladboy233, lanrebayode77, max10afternoon, minhtrng, mussucal, nobody2018, peakbolt, pontifex, qbs, ravikiranweb3, rvierdiiev, said, tapir, ubermensch, volodya, wintermute, yashar, zaevlad, zzebra83
0.1468 USDC - $0.15
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
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.
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.
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.ReLPContract.reLP()
can potentially reverts since it calls RdpxV2Core.sync()
, potentially breaking reLP mechanismUniV3LiquidityAMO.addLiquidity()
, UniV3LiquidityAMO.removeLiquidity()
and UniV3LiquidityAMO.swap()
also calls RdpxV2Core.sync()
and can cause potential reverts, breaking AMO market operations.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
Manual Analysis, Foundry
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); }
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
🌟 Selected for report: LokiThe5th
909.7371 USDC - $909.74
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.
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); }
totalSupply()
should be used instead, which represents the total supply of shares minted.
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)
491.258 USDC - $491.26
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:
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 revertunlockLiquidity()
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.deposit()
.settle()
will not revert. Consequently, _rdpxCollateral
and _activeCollateral
will be increased and decreased respectively._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
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)); }
Manual Analysis, Foundry
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; }
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
🌟 Selected for report: juancito
Also found by: 0x3b, 0xmuxyz, 0xnev, ArmedGoose, Bauchibred, KrisApostolov, RED-LOTUS-REACH, Viktor_Cortess, ciphermarco, ginlee, ladboy233, mitko1111, nemveer
90.6302 USDC - $90.63
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
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).
Manual Analysis
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.
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
🌟 Selected for report: Vagner
Also found by: 0Kage, 0xCiphky, 0xnev, ABAIKUNANBAEV, Aymen0909, Evo, KmanOfficial, MohammedRizwan, T1MOH, Viktor_Cortess, Yanchuan, ak1, alexzoid, bin2chen, codegpt, hals, ladboy233, mrudenko, nemveer, oakcobalt, peakbolt, pep7siup, qbs, said, savi0ur, tapir, wintermute, zaevlad, zzebra83
7.8372 USDC - $7.84
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L964
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()
.
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()
).
Consider the following scenario, where there is no bond discount (for simplicity)
totalWethDelegated = 2 wETH
. reserves now has 10 - 2 = 8 wETHaddToDelegate()
and delegates 1 wETH, now totalWethDelegated = 3 wETH
, but reserves is still at 8 wETH instead of 7 wETH.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.Manual Analysis
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); }
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
🌟 Selected for report: Vagner
Also found by: 0Kage, 0xCiphky, 0xnev, ABAIKUNANBAEV, Aymen0909, Evo, KmanOfficial, MohammedRizwan, T1MOH, Viktor_Cortess, Yanchuan, ak1, alexzoid, bin2chen, codegpt, hals, ladboy233, mrudenko, nemveer, oakcobalt, peakbolt, pep7siup, qbs, said, savi0ur, tapir, wintermute, zaevlad, zzebra83
7.8372 USDC - $7.84
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L160-L178
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()
).
Manual Analysis
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); }
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
🌟 Selected for report: degensec
Also found by: 0x3b, 0xnev, HChang26, KmanOfficial, QiuhaoLi, T1MOH, WoolCentaur, Yanchuan, ayden, bart1e, jasonxiale, kutugu, mert_eren, nirlin, peakbolt, peanuts, pep7siup, qpzm, tapir, ubermensch, wintermute
24.8267 USDC - $24.83
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
ReLPContract.reLP()
: Unused ETH is not returned back to RdpxV2Core and is left stranded in the reLP contractIn 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.
Manual Analysis
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(); }
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
🌟 Selected for report: juancito
Also found by: 0xDING99YA, 0xTiwa, 0xkazim, 0xnev, ABA, ArmedGoose, Aymen0909, Bauchibred, Evo, IceBear, KrisApostolov, MohammedRizwan, Nikki, QiuhaoLi, T1MOH, Toshii, WoolCentaur, Yanchuan, __141345__, asui, bart1e, carrotsmuggler, catellatech, chaduke, codegpt, deadrxsezzz, degensec, dethera, dirk_y, erebus, ether_sky, gjaldon, glcanvas, jasonxiale, josephdara, klau5, kodyvim, ladboy233, lsaudit, minhquanym, parsely, peakbolt, pep7siup, rvierdiiev, said, savi0ur, sces60107, tapir, ubermensch, volodya, zzebra83
19.1724 USDC - $19.17
https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L705-L717
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()
:
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.
Manual Analysis
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.
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
🌟 Selected for report: catellatech
Also found by: 0xSmartContract, 0xnev, K42, LokiThe5th, RED-LOTUS-REACH, Sathish9098, __141345__, bitsurfer, hunter_w3b, mark0v, nirlin, rjs, rokinot
90.0998 USDC - $90.10
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.
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.
The regular bonding mechanism can be summarised in to 6 detailed steps below:
RdpxV2Core.calculateBondCost()
.RdpxV2Core.bond()
, the amount of weth required including premium is first transferred to core contractRdpxV2Core._purchaseOptions()
.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.RdpxV2Core._stake()
ReLPContract.relp()
.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:
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.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()
.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:
MINTER_ROLE
first mints a decaying bond with associated rDPX via RdpxDecayingBonds.mint()
, specifying the expiry and amount of rDPX.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.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:
PerpetualAtlanticVaultLP.deposit()
into LP Vault, receiving associated amount of shares based on amount deposited.RdpxV2Core.bond()/bondWithDelegate()
, 25% OTM APP options is purchased for core contract via RdpxV2Core._purchaseOptions()
, which in turn calls PerpetualAtlanticVault.purchase()
.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.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.RdpxV2Core.settle()
, collateral associated with relevant optionIds of bond writers will be unlocked for them to redeem.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.
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:
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.
Codebase Quality Categories | Comments |
---|---|
Testing | Unit test and Integration tests were present using Foundry, allowing ease of testing for PoCs |
Documentation | The documentation for Dopex has some inconsistencies, but overall provides a satisfactory overview of the protocol |
Organization | Codebase is decently organized with clear distinctions between the 9 contracts. |
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.
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()
bondDiscountFactor
: bond discount factor used to control bond discount given to user bonding set via RdpxV2Core.setBondDiscount()
.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.
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.
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.
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.
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.
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.
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.
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.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 holdersbondMaturity
: Bond maturity set via setBondMaturity()
, excessive values can prevent users from redeeming bonds and getting back receipt tokensRdpxV2Core.emergencyWithdraw()
: Protocol admin can withdraw ALL of the funds in the core contract backing reserves, griefing users fundsPerpetualAtlanticVaultLP.emergencyWithdraw()
: Protocol admin can withdraw ALL of the initial premium provided by users bonding for APP options minted.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
The following priviledged functions can cause DoS of important
RdpxV2Core.removeAssetFromtokenReserves()
: Can remove any assets from interacting with rDPXV2 protocolRdpxV2Core.setPricingOracleAddresses()
: Can change pricing oracle address anytime, potentially influencing (increase.decrease) price of rDPX and dpxETH.RdpxV2Core.settle()
: Protocol treasury can withhold settling APP options can lock bond writers collateral.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 vaultLPDay | Scope | Details |
---|---|---|
1-2 | rDPXV2 Docs Link | Review Docs and understand key components of protocol, namely: bonding, reLP, APP and AMO |
3-6 | rDPXV2 contracts Link | Manual audits of contracts with the following flow, RdpxV2Core.sol --> PerpetualAtlanticVault.sol/PerpetualAtlanticVaultLP.sol --> ReLPContract.sol --> UniV2LiquidityAmo.sol/UniV3LiquidityAmo.sol |
7 | - | Finish up Analysis report |
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