Platform: Code4rena
Start Date: 04/03/2024
Pot Size: $88,500 USDC
Total HM: 31
Participants: 105
Period: 11 days
Judge: ronnyx2017
Total Solo HM: 7
Id: 342
League: ETH
Rank: 9/105
Findings: 4
Award: $1,317.98
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: Bauchibred
958.2005 USDC - $958.20
Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L357-L374
function _getReferencePoolPriceX96(IUniswapV3Pool pool, uint32 twapSeconds) internal view returns (uint256) { uint160 sqrtPriceX96; // if twap seconds set to 0 just use pool price if (twapSeconds == 0) { (sqrtPriceX96,,,,,,) = pool.slot0(); } else { uint32[] memory secondsAgos = new uint32[](2); secondsAgos[0] = 0; // from (before) secondsAgos[1] = twapSeconds; // from (before) (int56[] memory tickCumulatives,) = pool.observe(secondsAgos); // pool observe may fail when there is not enough history available (only use pool with enough history!) //@audit int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds))); sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick); } return FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, Q96); }
This function is used to calculate the reference pool price, It uses either the latest slot price or TWAP based on twapSeconds.
Now note that unlike the original uniswap implementation, here the delta of the tick cummulatives is being calculated in a different manner, i.e protocol implements (tickCumulatives[0] - tickCumulatives[1] instead of tickCumulatives[1] - (tickCumulatives[0]
which is cause here, secondsAgos[0] = 0; & secondsAgos[1] = twapSeconds;
, unlike in Uniswap OracleLibrary where secondsAgos[0] = secondsAgo; & secondsAgos[1] = 0;
, so everything checks out and the tick deltas are calculated accurately, i.e in our case tickCumulativesDelta = tickCumulatives[0] - tickCumulatives[1]
The problem now is that in the case if our tickCumulativesDelta
is negative, i.e int24(tickCumulatives[0] - tickCumulatives[1] < 0)
, then the tick should be rounded down as it's done in the uniswap library
But this is not being done and as a result, in case if int24(tickCumulatives[0] - tickCumulatives[1])
is negative and (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0
, then the returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.
In the case if int24(tickCumulatives[0] - tickCumulatives[1])
is negative and ((tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0
, then returned tick will be bigger than it should be which places protocol wanting prices to be right not be able to achieve this goal, note that where as protocol in some cases relies on multiple sources of price, they still come down and end on weighing the differences between the prices and reverting if a certain limit is passed (MIN_PRICE_DIFFERENCE
) between both the Chainlink price and Uniswap twap price.
Now in the case where the implemented pricing mode is only TWAP
then protocol would work with a flawed price since the returned price would be different than it really is, potentially leading to say for example some positions that should be liquidatable not being liquidated cause before liquidation there is a check to see if the loan is healthy, now this check queries the value of this asset via getValue() and if returned price is wrong then unhealthy loans could be pronounced as healthy and vice versa.
Also this indirectly curbs the access to functions like borrow(), transform() and decreaseLiquidityAndCollect(), since they all make a call to _requireLoanIsHealthy()
which would be unavailable due to it's dependence on _checkLoanIsHealthy()
This bug case causes the Automator's _getTWAPTick()
function to also return a wrong tick which then leads to _hasMaxTWAPTickDifference()
returning false data since the difference would now be bigger eventually leading to wrongly disabling/enabling of swaps in `AutoCompound.sol whereas it should be otherwise.
Note that for the second / third case the call route to get to _getReferencePoolPriceX96()
is : "_checkLoanIsHealthy() -> getValue() ->_getReferenceTokenPriceX96 -> _getTWAPPriceX96-> _getReferencePoolPriceX96() "
as can be seen in https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol
Add this line:
if (tickCumulatives[0] - tickCumulatives[1] < 0 && (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0) timeWeightedTick --;
Uniswap
#0 - c4-pre-sort
2024-03-22T08:20:52Z
0xEVom marked the issue as duplicate of #498
#1 - c4-pre-sort
2024-03-22T08:20:55Z
0xEVom marked the issue as sufficient quality report
#2 - c4-pre-sort
2024-03-25T12:11:18Z
0xEVom marked the issue as not a duplicate
#3 - c4-pre-sort
2024-03-25T12:11:21Z
0xEVom marked the issue as primary issue
#4 - c4-sponsor
2024-03-26T15:04:41Z
kalinbas (sponsor) confirmed
#5 - c4-judge
2024-03-31T00:04:11Z
jhsagd76 marked the issue as satisfactory
#6 - jhsagd76
2024-04-01T08:24:48Z
I think H is appropriate, but it's strange that the other wardens submitted as M. I have reviewed the other submissions and, although they are not as detailed as this report, they all point out the key issues and key impacts, so I will not reduce the reward ratio of the other reports.
#7 - c4-judge
2024-04-01T15:33:41Z
jhsagd76 marked the issue as selected for report
#8 - kalinbas
2024-04-09T17:35:35Z
🌟 Selected for report: t4sk
Also found by: Bauchibred, hunter_w3b, lanrebayode77
221.1232 USDC - $221.12
Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L137-L156
function _checkPoolPrice(address token0, address token1, uint24 fee, uint256 derivedPoolPriceX96) internal view { IUniswapV3Pool pool = _getPool(token0, token1, fee); uint256 priceX96 = _getReferencePoolPriceX96(pool, 0); _requireMaxDifference(priceX96, derivedPoolPriceX96, maxPoolPriceDifference); } function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000) internal pure { //@audit-issue M logic to verify the max difference is wrong uint256 differenceX10000 = priceX96 > verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 / priceX96 : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96; // if too big difference - revert if (differenceX10000 >= maxDifferenceX10000) { revert PriceDifferenceExceeded(); } }
These are two helper functions used in order to ensure that the correct prices are ingested, i.e by checking that difference between both the primary and secondary source is not more that the acceptable "maxDifference".
Now the issue arises in the conditional logic for calculating differenceX10000
, where the calculation dynamically chooses the base price for the percentage difference calculation based on which of the two prices is greater. This approach would result in varying methods of comparison, affecting the integrity of the price verification process.
uint256 differenceX10000 = priceX96 > verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 / priceX96 : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;
The current implementation would fail to use the right primary price as the base for comparison in as much as the secondary price is higher, leading to inconsistencies in how price differences are evaluated. This would incorrectly flag acceptable price variations as excessive or overlook actual problematic price differences.
Consider this case (Using BTC for example):
maxDifference
is as initially set of 2%
CHAINLINK_TWAP_VERIFY
$70,000
$71,428
_(1428/70000 * 70000 is > 2% of 70000)_
._requireMaxDifference()
does not actually revert, this is cause it wrongly uses the TWAP
price now as the "primary price" since it's higher, now calculating the max difference with the TWAP price as base, leads to the protocol ingesting an invalid price on the grounds of the documented max difference since we now get _(1428/71428 * 71428 which is < 2% of 71428)_
, where 71428 is the wrong base to be used.The function _requireMaxDifference
within the V3Oracle
contract is designed to enforce a maximum allowable price difference between two prices (priceX96
and verifyPriceX96
) with a specified tolerance (maxDifferenceX10000
). However, the current implementation inaccurately calculates the price difference, as it conditionally uses either price as the base for the percentage difference calculation (which is wrong cause based on historical data for some tokens either of Chainlink/ Uniswap Twap is the most reliable source of pricing and protocol would place the most reliable as the primary source). This method leads to inconsistencies in how price differences are assessed, resulting in incorrect validations and heavy discrepancies if coupled with the leveraging logic, since a price that's originally not meant to be ingested due to having pass the maximum difference would now be ingested.
Note that _maxPoolPriceDifference
can only be set higher than the 2%, and if the value is ever to be changed it only exarcebates this issue as this would even lead to allowing a higher discrepancy in prices that would be ingested in protocol.
Modify the difference calculation to always use the correct primary price as the base, that's depending on the current pricing mode for that token.
This would mean always using the primary priceX96
as the base for calculating the percentage difference to verifyPriceX96
(the secondary price), i.e apply these changes to
function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000) internal pure { uint256 differenceX10000 = priceX96 > verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 / priceX96 - : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96; + : (verifyPriceX96 - priceX96) * 10000 / priceX96; // if too big difference - revert if (differenceX10000 >= maxDifferenceX10000) { revert PriceDifferenceExceeded(); } }
Oracle
#0 - c4-pre-sort
2024-03-22T08:40:02Z
0xEVom marked the issue as duplicate of #10
#1 - c4-pre-sort
2024-03-22T08:40:05Z
0xEVom marked the issue as sufficient quality report
#2 - c4-judge
2024-04-01T13:25:35Z
jhsagd76 marked the issue as satisfactory
🌟 Selected for report: Bauchibred
Also found by: 0x11singh99, 0x175, 0xAlix2, 0xDemon, 0xGreyWolf, 0xPhantom, 0xspryon, 14si2o_Flint, Arabadzhiev, Aymen0909, Bigsam, BowTiedOriole, CRYP70, DanielArmstrong, FastChecker, JecikPo, KupiaSec, MohammedRizwan, Norah, Timenov, Topmark, VAD37, adeolu, btk, crypticdefense, cryptphi, givn, grearlake, jnforja, kennedy1030, kfx, ktg, lanrebayode77, n1punp, santiellena, stonejiajia, t4sk, thank_you, tpiliposian, wangxx2026, y0ng0p3, zaevlad
55.6122 USDC - $55.61
Issue ID | Description |
---|---|
QA-01 | MIN_TWAP_SECONDS is too low and an avenue to ingest manipulated prices |
QA-02 | Unhandled Chainlink revert would halt core functionalities |
QA-03 | _checkApprovals should be reimplemented to count for the allowance depleting |
QA-04 | Setters should always have equality checkers |
QA-05 | Allow totalRewardX64 to be set higher via AutoCompound.setReward() |
QA-06 | Pricing logic shouldn't be unavailable in any case or at least the provider of the secondary source should not halt the system |
QA-07 | Introduce better naming conventions |
QA-08 | Introduce a contract existence check before a low level call to send ETH |
QA-09 | safeIncreaseAllowance has been deprecated |
MIN_TWAP_SECONDS
is too low and an avenue to ingest manipulated pricesTake a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/automators/Automator.sol#L86-L98
function setTWAPConfig(uint16 _maxTWAPTickDifference, uint32 _TWAPSeconds) public onlyOwner { if (_TWAPSeconds < MIN_TWAP_SECONDS) { revert InvalidConfig(); } if (_maxTWAPTickDifference > MAX_TWAP_TICK_DIFFERENCE) { revert InvalidConfig(); } emit TWAPConfigChanged(_TWAPSeconds, _maxTWAPTickDifference); TWAPSeconds = _TWAPSeconds; maxTWAPTickDifference = _maxTWAPTickDifference; }
One can see that the accepted TWAP seconds could be as low as 60 seconds which is just a minute and one could say is even same with a spot price, this is cause the timeframe is too short and still allows for manipulatable prices.
Since MIN_TWAP_SECONDS
is very low, then this might lead to protocol ingesting manipulated prices
Have the MIN_TWAP_SECONDS
value be within the 10-30 minutes range.
Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L331-L349
function _getChainlinkPriceX96(address token) internal view returns (uint256) { if (token == chainlinkReferenceToken) { return Q96; } TokenConfig memory feedConfig = feedConfigs[token]; //@audit // if stale data - revert (, int256 answer,, uint256 updatedAt,) = feedConfig.feed.latestRoundData(); if (updatedAt + feedConfig.maxFeedAge < block.timestamp || answer < 0) { revert ChainlinkPriceError(); } return uint256(answer) * Q96 / (10 ** feedConfig.feedDecimals); }
One can see that all pricing logic that requires us to query chainlink at the end of the calls on the _getChainlinkPriceX96()
, i.e three out of the 5 accepted pricing modes require the call to this function not to revert.
But evidently, we can see that this call lacks error handling for the potential failure of feedConfig.feed.latestRoundData()
, note that Chainlink pricefeeds could revert due to whatever reason, i.e say maintenance or maybe the Chainlink team decide to change the underlying address. Now this omission of not considering thiks call failing would lead to systemic issues, since calls to this would now revert halting any action that requires this call to succeed.
A revert of the entire operation that requires querying the Chainlink pricing logic, This limitation poses a significant risk, since all three pricing modes that use the Chainlink mode either as a primary or secondary source would now revert.
Wrap the feedConfig.feed.latestRoundData()
call in a try-catch block, even if it's only for the pricing logic that use Chainlink as a secondary sourceand then handle the error (e.g., revert with a specific message or use an alternative pricing method) and also apply other chainlink safety integration checks from here: https://medium.com/cyfrin/chainlink-oracle-defi-attacks-93b6cb6541bf
_checkApprovals
should be reimplemented to count for the allowance depletingThe function _checkApprovals
is designed to only set approvals if the current allowance is zero. This is a one-time setup meant to minimize gas costs associated with setting allowances for token transfers.
function _checkApprovals(address token0, address token1) internal { uint256 allowance0 = IERC20(token0).allowance(address(this), address(nonfungiblePositionManager)); if (allowance0 == 0) { SafeERC20.safeApprove(IERC20(token0), address(nonfungiblePositionManager), type(uint256).max); } uint256 allowance1 = IERC20(token1).allowance(address(this), address(nonfungiblePositionManager)); if (allowance1 == 0) { SafeERC20.safeApprove(IERC20(token1), address(nonfungiblePositionManager), type(uint256).max); } }
While setting the allowance to type(uint256).max
reduces the need for repeated approvals (and thus saves gas), it neglects the scenario where the allowance might be fully utilized. In typical use cases, reaching type(uint256).max
would require an unrealistic volume of transactions. However, it does not account for potential bugs, exploits, or changes in contract logic that could deplete this allowance unexpectedly.
The current implementation of the _checkApprovals
function sets the token allowance to type(uint256).max
for the nonfungiblePositionManager
contract, intending to save on gas costs for future transactions. However, this approach introduces a vulnerability where, once the type(uint256).max
allowance is exhausted, there would be no mechanism in place to renew the approval. This could lead to a situation where the smart contract is unable to perform operations requiring token transfers on behalf of users, effectively freezing any functionality dependent on these approvals.
Instead of only checking for an allowance of zero, implement a mechanism to check if the allowance is below a certain threshold and, if so, replenish it.
Use this search prompt code: https://github.com/search?q=repo%3Acode-423n4%2F2024-03-revert-lend%20function%20set&type=code
Evidently we can see that multiple setter functions exist in scope, with them not having any check that the new accepted value is not equal to the previously stored value.
For e.g setReward() cehcks that the new value is "<=" where as it should only check "<" leading to an unnecessary update of totalRewardX64 if _totalRewardX64 is already == totalRewardX64
.
function setReward(uint64 _totalRewardX64) external onlyOwner { require(_totalRewardX64 <= totalRewardX64, ">totalRewardX64"); totalRewardX64 = _totalRewardX64; emit RewardUpdated(msg.sender, _totalRewardX64); }
Another example would be the below that does not implement any checks whatsoever:
function setWithdrawer(address _withdrawer) public onlyOwner { emit WithdrawerChanged(_withdrawer); withdrawer = _withdrawer; }
And so on.
Unnecessary code execution
Introduce equality checkers for setter functions
totalRewardX64
to be set higher via AutoCompound.setReward()
function setReward(uint64 _totalRewardX64) external onlyOwner { require(_totalRewardX64 <= totalRewardX64, ">totalRewardX64"); totalRewardX64 = _totalRewardX64; emit RewardUpdated(msg.sender, _totalRewardX64); }
This function is used to set the reward value, keep in mind that there are no checks for e.g that the provided _totalRewardX64 != 0
, now if this value gets set to 0 it can never be changed cause as implemented in code it can only be lower in value
Low since this requires admin mistake, but the whole rewards logic could be broken
Allow this value to be reset to atleast an accepted max.
The relevant code segment attempts to calculate TWAP using the pool.observe
function with two time points, 'now' and 'now - twapSeconds'. This method is prone to failure if the pool does not have enough historical data to cover the requested time span, a scenario not uncommon for newer or less active pools, i.e
function _getReferencePoolPriceX96(IUniswapV3Pool pool, uint32 twapSeconds) internal view returns (uint256) { uint160 sqrtPriceX96; // if twap seconds set to 0 just use pool price if (twapSeconds == 0) { (sqrtPriceX96,,,,,,) = pool.slot0(); } else { uint32[] memory secondsAgos = new uint32[](2); secondsAgos[0] = 0; // from (before) secondsAgos[1] = twapSeconds; // from (before) //@audit-issue pool.observe could fail due to lack of history. (int56[] memory tickCumulatives,) = pool.observe(secondsAgos); // pool observe may fail when there is not enough history available (only use pool with enough history!) int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds))); sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick); } return FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, Q96); }
Now the lack of error handling for the potential failure of pool.observe
due to insufficient historical data. This omission would lead to systemic issues, particularly in financial systems that rely on continuous access to accurate pricing data for operational stability.
The method _getReferencePoolPriceX96
within the V3Oracle
contract is designed to calculate the reference price from a Uniswap V3 pool, either directly from the latest slot price or through a Time-Weighted Average Price (TWAP) if a twapSeconds
parameter is specified. An identified issue arises when the pool.observe
function is called for TWAP calculation: if there is insufficient historical data within the specified twapSeconds
, the call to pool.observe
could fail, leading to a revert of the entire operation. This limitation poses a significant risk, since all three pricing modes that use the TWAP mode either as a primary or secondary source would now revert.
Wrap the pool.observe
call in a try-catch block, even if it's only for the pricing logic that use TWAP as a secondary source.
```solidity try pool.observe(secondsAgos) returns (int56[] memory tickCumulatives,) { int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds))); sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick); } catch { // Handle the error (e.g., revert with a specific message or use an alternative pricing method) } ```
Take a look at (https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L25-L28) we can see that this is used to declare the MIN_PRICE_DIFFERENCE
as 2%, issue is that this value is actually the min max price difference, i.e looking at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L183-L191
function setMaxPoolPriceDifference(uint16 _maxPoolPriceDifference) external onlyOwner { if (_maxPoolPriceDifference < MIN_PRICE_DIFFERENCE) { revert InvalidConfig(); } maxPoolPriceDifference = _maxPoolPriceDifference; emit SetMaxPoolPriceDifference(_maxPoolPriceDifference); }
We can see that whenever setting the max pool price difference, it's checked to not be lower than our MIN_PRICE_DIFFERENCE
Confusion in naming conventions leading to hard time of users/developers understanding code.
Consider renaming the vcariable and take into acount that it's the minimum max difference between the prices.
Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/automators/Automator.sol#L125-L138
function withdrawETH(address to) external { if (msg.sender != withdrawer) { revert Unauthorized(); } uint256 balance = address(this).balance; if (balance > 0) { //@audit QA low level call (bool sent,) = to.call{value: balance}(""); if (!sent) { revert EtherSendFailed(); } } }
Now consider this minimal contract as a POC
// SPDX-License-Identifier: MIT pragma solidity ^0.8.21; import "hardhat/console.sol"; contract test { constructor() { bytes memory txData*; (bool success,) = payable(address(0)).call{ value: 0 }(txData*); console.log("success",success); } }
One can see that the even as we used an invalid address, 0x0, the call still comes back successful, whereas it shouldn't.
Introduce a contract existence check before transferring eth.
amount1 += amountOut; } //@todo QA safeIncreaseAllowance has been deprecated SafeERC20.safeIncreaseAllowance(IERC20(token0), address(nonfungiblePositionManager), amount0); SafeERC20.safeIncreaseAllowance(IERC20(token1), address(nonfungiblePositionManager), amount1);
Usage of deprecated functions.
Do not use deprecated functions
#0 - c4-pre-sort
2024-03-24T21:02:50Z
0xEVom marked the issue as sufficient quality report
#1 - c4-pre-sort
2024-03-25T20:42:05Z
0xEVom marked the issue as high quality report
#2 - 0xEVom
2024-03-25T20:44:46Z
GA-05: reckless admin mistake, invalid QA-09: invalid
#3 - kalinbas
2024-03-27T16:05:48Z
QA-01 Depending on the network 1 minute may be enough, also TWAP is only used to verify prices in the standard scenario where Chainlink prices are used mainly. 30 minutes is way to much when prices move fast. QA-02 If chainlink is down, only liquidiate, decreaseLiquidity and borrow functionality would be down. Thats why there is the emergency mode to disable chainlink completely if needed. QA-03 This will never reach 0 for a normal token (and this code is already deployed) - so we will leave it QA-04 Only called by admin so not that important QA-05: Invalid QA-06 Will be part of adding new tokens taking care that pools have enough history. If there is a problem with it, the history size may be increased by anyone. QA-07 Ok agree QA-08 No, its ok that ETH is sent to any address. QA-09: Invalid
#4 - c4-sponsor
2024-03-27T16:09:30Z
kalinbas (sponsor) acknowledged
#5 - c4-judge
2024-03-31T00:06:59Z
jhsagd76 marked the issue as grade-a
#6 - c4-judge
2024-04-01T15:38:44Z
jhsagd76 marked the issue as selected for report
#7 - jhsagd76
2024-04-12T00:28:39Z
1 Low 2 NC + 3 Downgraded QA Low
#8 - thebrittfactor
2024-04-24T21:35:57Z
Just a note that C4 is excluding the invalid entries from the official report.
🌟 Selected for report: 14si2o_Flint
Also found by: Bauchibred, K42, Sathish9098, hunter_w3b, invitedtea, popeye, yongskiws
83.0533 USDC - $83.05
Started with a comprehensive analysis of the provided docs and whitepaper to understand protocol functionality and key points, ambiguities were discussed with the sponsors(devs), and possible risk areas were outlined.
Followed by a manual review of each contract in scope, testing function behavior, protocol logic against expectations, and working out potential attack vectors. Vulnerabilities related to dependencies and inheritances, were also assessed. Comparisons with similar protocols was also performed to identify recurring issues and evaluate fix effectiveness.
Finally, identified issues from the security review were compiled into a comprehensive audit report.
Revert Lend stands as a peer-to-pool lending platform, tailor-made for AMM Liquidity Providers , specifically within the Uniswap v3 ecosystem. It introduces an opportunity for users to secure ERC-20 token loans by using their Uniswap v3 liquidity provider positions as collateral. What seems to set this protocol apart is its capacity to allow Liquidity Providers to keep managing their capital actively within the Uniswap v3 pools, even while it serves as collateral. This functionality ensures that Liquidity Providers can continuously optimize and manage their positions to adapt to the fluctuating demands of the market, providing an uninterrupted liquidity provision experience.
This contract enables the automatic reinvestment of fees into liquidity positions with potential for swaps to optimize token ratios, all within a single transaction. Controlled by a Revert bot operator and capable of direct and vault-based position transformations, this contract keeps track of compounding actions, configuration adjustments, and token balance changes, providing a comprehensive tool for liquidity management. The system also facilitates the management of rewards and leftover balances, ensuring a seamless process for liquidity providers to maximize their yield through strategic reinvestment and position optimization.
Operators and Vaults: It allows the contract owner to designate specific addresses as operators or vaults, enabling them to execute certain privileged actions within the contract's ecosystem.
Withdrawer: A specific address that's authorized to withdraw accumulated fees or tokens from the contract. This role is typically reserved for the contract owner or a trusted entity.
TWAP Configuration: The contract owner can set parameters related to Time Weighted Average Price (TWAP) calculations, including the observation period (TWAPSeconds
) and the maximum allowable tick difference (maxTWAPTickDifference
) to mitigate price manipulation risks.
Swap Validation: Before performing swaps, the contract validates them against current market conditions and configured TWAP parameters to ensure they're executed within acceptable price ranges and slippage limits.
TWAP Calculation: Utilizes Uniswap V3's observe
function to calculate TWAP over a specified period, assisting in price verification and ensuring swaps don't deviate significantly from the market price.
Balance Withdrawals: The contract supports withdrawing ERC20 token balances and ETH, facilitating the management of accrued fees or rebalancing of assets.
WETH Handling: Includes mechanisms for wrapping and unwrapping WETH (Wrapped ETH), allowing the contract to seamlessly handle ETH in operations requiring ERC20 tokens.
Operator and Vault Checks: Enforces checks to ensure that only authorized operators can perform certain actions and that interactions with vaults are restricted to approved contracts.
Ownership Validation: For operations involving Uniswap V3 positions or vault-held assets, the contract verifies that the caller owns the relevant NFT or vault asset.
receive
function to handle ETH, specifically for unwrapping WETH, ensuring that the contract can accept ETH refunds or payments as needed.This contract introduces an innovative approach to managing Uniswap V3 positions, enabling automatic execution of limit or stop-loss orders based on specific market conditions. This functionality allows liquidity providers to either remove their liquidity position or swap it to the opposite token when the market reaches a predetermined tick, effectively serving as an automated risk management tool. Operated by a Revert controlled bot, the contract utilizes external swap routers to ensure optimized swap execution. Position owners must approve the AutoExit contract to manage their positions, which can be configured for automatic execution using the contract's configToken
method. Once a position reaches its trigger tick, the operator can execute the pre-configured action, with the event details broadcasted for transparency.
Executed
: Logs when a position is executed, including details about the execution such as whether it was a swap or removal, the amounts returned, and the tokens involved.PositionConfigured
: Logs the configuration of a position, detailing the active status, swap behavior, trigger ticks, slippage settings, and reward parameters.PositionConfig
struct: Defines the configuration for each position, including whether it's active, swap behaviors, trigger ticks for execution, slippage tolerances, whether only fees should be used for rewards, and the maximum reward percentage.configToken
: Allows the owner of a Uniswap V3 position to configure how their position should be managed by the contract, including setting up the conditions for automated exit strategies.ExecuteParams
struct: Contains parameters required for executing a position's configured strategy, including swap details, liquidity information, minimum amounts for removal, deadline, and reward percentage.execute
: Can be called by an authorized operator to execute a position's exit strategy based on its configuration. It validates the position's state, performs necessary swaps or liquidity removal, applies configured rewards, and transfers the resulting assets back to the position owner. It also deactivates the position's configuration to prevent re-entrancy or duplication of execution.
Automator
contract for validating swaps against TWAP settings and managing liquidity in Uniswap V3 pools, ensuring that exit strategies are executed in line with market conditions and within acceptable price ranges.This contract provides functionalities for directly leveraging or deleveraging positions within a single transaction in the vault system, specifically it allows users to increase their position's leverage by borrowing a specified amount, swapping borrowed tokens to desired assets, and then adding those assets to their Uniswap V3 position. Conversely, it supports the reduction of leverage by removing liquidity from the position, optionally swapping assets back to the loan token, and repaying the borrowed amount.
leverageUp
Method: Allows increasing the leverage of a Uniswap V3 position by borrowing additional assets (either token0 or token1 of the Uniswap V3 position), optionally swapping a portion of the borrowed asset to another, and adding both assets back as liquidity to the position.
Swapper
functionalities) to execute any necessary swaps and interacts with a vault contract to manage the borrowing of assets.leverageDown
Method: Facilitates decreasing the leverage of a position by removing liquidity, optionally swapping some of the removed assets back to the borrowed asset type, and repaying the borrowed amount to the vault.
leverageUp
, it uses the Swapper
functionalities for executing swaps and interacts with a vault contract for the repayment of the borrowed assets.Swapper
contract's swap mechanisms to execute asset conversions through predefined swap routes, allowing for efficient market operations based on live swap data.IVault
) for borrowing and repaying assets. This integration allows the contract to leverage or deleverage positions based on external conditions and configurations.This contract facilitates the automatic adjustment of Uniswap V3 positions' ranges via a controlled bot, aiming to optimize liquidity positions based on market movements. This contract enables operators to reconfigure the range of approved positions to maintain optimal placement within the Uniswap V3 pool. When a position is adjusted, a new position is created with settings mirroring the original, and the configuration is transferred to this new position. For positions housed within a Vault, the contract invokes a transform call to execute the range adjustment. Key features here include leveraging Uniswap's flash loans for liquidity adjustments and managing position configurations through struct mappings, ensuring operational flexibility and efficiency in liquidity management strategies.
RangeChanged
: Logs when a position's range has been successfully changed, capturing both the old and new token IDs.PositionConfigured
: Logs the configuration of a position with details on tick limits, tick deltas for range adjustment, slippage parameters, and reward settings.PositionConfig
Struct: Holds configuration details for range adjustment of a position, including tick limits for triggering adjustments, tick deltas for new range boundaries, slippage tolerances for swaps, and reward calculation parameters.configToken
Function: Allows users or designated vaults to configure how a position should automatically adjust its range based on the specified parameters.executeWithVault
and execute
Functions: Executed by the designated operator, these functions adjust the range of a configured position when market conditions meet predefined triggers. The adjustment process can include swapping tokens to optimize position composition before adjusting the range.
transform
method is called to execute the range adjustment.Swapper
functionalities to execute necessary swaps as part of the range adjustment process. This allows for efficient market operations and ensures that positions are adjusted with optimal asset composition.This contract enables automatic reinvestment of fees into liquidity positions with potential for swaps to optimize token ratios, all within a single transaction. Controlled by a Revert bot operator and capable of direct and vault-based position transformations, this contract integrates with Uniswap's INonfungiblePositionManager, supporting multicall for batch operations and safeguarded by a reentrancy guard. It keeps track of compounding actions, configuration adjustments, and token balance changes, providing a comprehensive tool for liquidity management. The system also facilitates the management of rewards and leftover balances, ensuring a seamless process for liquidity providers to maximize their yield through strategic reinvestment and position optimization.
AutoCompounded
: Logs details of the compounding action, including amounts added to the position, rewards generated, and tokens involved.RewardUpdated
: Logs changes to the reward configuration initiated by the contract owner.BalanceAdded
/ BalanceRemoved
/ BalanceWithdrawn
: Logs the management of token balances associated with positions, whether adding to the balance, removing from it, or withdrawing it to a specific address.positionBalances
) to track the accumulated fees and rewards for each position. This allows for precise management of resources when compounding positions.executeWithVault
/ execute
Functions: Allow for the compounding of positions either directly or through a vault. These functions facilitate the collection of fees, optional swapping of tokens for optimal position alignment, and the addition of liquidity to the Uniswap V3 position.
totalRewardX64
) is configurable by the contract owner, subject to a maximum limit.withdrawLeftoverBalances
: Enables position owners to withdraw any leftover tokens after compounding actions have been executed.withdrawBalances
: Allows the contract's designated withdrawer to remove accumulated protocol fees from the contract.setReward
: Allows the contract owner to adjust the reward percentage allocated to the protocol from compounding actions, providing flexibility in managing incentives for operators._checkApprovals
: Ensures that the contract has the necessary approvals to manage the position's tokens, optimizing gas costs by setting max allowances where needed.This contract provides a suite of utility functions to interact with Uniswap V3 positions, including complex operations like swap, mint, and liquidity management, without holding any ERC20 or NFT assets itself. It extends the Swapper contract for executing swaps and integrates with the Permit2 system for EIP-712 style token spending permissions. Key functionalities include executing pre-defined instructions on NFTs, such as changing liquidity ranges, compounding fees into liquidity, or withdrawing and swapping tokens, all within atomic transactions. It supports direct execution with or without EIP712 permits and leverages flash loans for liquidations. The contract operates in a completely ownerless and stateless manner, emphasizing flexibility and upgradability for Uniswap V3 position management.
Swapper
for its swap capabilities, allowing efficient market operations and liquidity management.IERC721Receiver
to handle NFTs safely, facilitating operations on Uniswap V3 positions directly through transfers.ReentrancyGuard
to prevent reentrancy attacks, ensuring the security of transactions.CompoundFees
, ChangeRange
, WithdrawAndCollectAndSwap
, etc.) are emitted to log significant actions performed on Uniswap V3 positions, aiding in transparency and traceability.V3Utils
minimizes the need for multiple transactions and approvals, streamlining operations for Uniswap V3 liquidity providers.Contract is designed to facilitate token swaps utilizing various decentralized exchange (DEX) protocols, including Uniswap V3 and 0x Exchange. It acts as an intermediary to execute trades with external routing based on pre-calculated swap instructions, ensuring slippage control through a minimum output amount parameter. Key features include integration with Uniswap V3's non-fungible position manager for liquidity management, support for WETH transactions, and the ability to interact with a universal router for executing complex trade strategies. The contract emits events for each swap, providing transparency over trade details such as input and output token addresses and amounts. r
Router Swap (_routerSwap
): Facilitates token swaps using external routing protocols, employing off-chain calculated instructions. It involves slippage checks to ensure the minimum amount of output tokens (amountOutMin
) is received. It supports integration with multiple routers like 0x Exchange Proxy and Uniswap's Universal Router.
Direct Pool Swap (_poolSwap
): Executes swaps directly within a specified Uniswap V3 pool. It requires the contract to have available amounts of both participating tokens. Similar to router swaps, it includes slippage control to ensure the output meets a predefined minimum.
uniswapV3SwapCallback
): Required for participating in Uniswap V3 swaps, this callback function facilitates the transfer of the necessary input tokens to the Uniswap pool after a swap is initiated but before it's completed.IErrors
interface for consistent error management across various operations, enhancing the contract's robustness and ease of debugging.Contract builds on swapper's functionality and utilizing Uniswap V3's flash loans, enables atomic liquidation of loans alongside necessary token swaps. It integrates with V3 vault system to perform loan liquidations by acquiring assets through Uniswap V3 flash loans, swapping assets if necessary to meet liquidation requirements, and ensuring a minimum reward for the liquidator. This process is done within the liquidate
function that orchestrates the flash loan, asset swaps, and loan liquidation in a single transaction. Upon completion, the contract settles the flash loan, returns any excess assets to the liquidator, and verifies that the liquidator's minimum reward condition is met, enhancing operational efficiency and security in liquidation scenarios.
The FlashloanLiquidator
contract extends the Swapper
contract to perform atomic liquidations of loans using Uniswap V3's flash loans. Here are its key functionalities:
uniswapV3FlashCallback
function required for handling flash loans. This function is automatically called by the Uniswap V3 pool after issuing a flash loan. Within this callback, the contract:
liquidate
function to perform the actual loan liquidation._routerSwap
to convert any borrowed assets not in the form of the liquidation asset back into the liquidation asset.Leverages inherited functionalities from the Swapper
contract to perform token swaps. This is utilized within the flash loan callback to manage assets post-liquidation:
_routerSwap
functionality.minReward
specified, with the contract reverting if this condition is not met.IErrors
interface and Swapper
contract to manage operation-specific errors, such as swap failures or insufficient rewards from liquidation.This contract offers a dynamic framework for calculating interest rates within athe Vault. It extends the Ownable contract, allowing for governance over crucial parameters affecting borrow and supply rates. This model incorporates a base rate, a multiplier effect based on utilization rates, and a kink point that transitions the interest rate calculation from a linear to a potentially exponential regime, accommodating rapid changes in utilization beyond certain thresholds.
The InterestRateModel
contract, incorporating Ownable
from OpenZeppelin alongside interfaces IInterestRateModel
and IErrors
, offers a dynamic framework for calculating both borrow and supply interest rates in a Vault, specifically designed for use within decentralized finance (DeFi) protocols. Here's an outline of its core functionalities and design:
Interest Rate Calculation: Utilizes a model that factors in the base rate, multiplier, and jump multiplier per year, alongside a "kink" utilization rate, to calculate interest rates. This model allows for the adjustment of interest rates based on the current utilization rate of the protocol, enhancing the flexibility and economic stability of lending operations.
Q96 Precision: The contract employs fixed-point arithmetic with a factor of (2^{96}) (Q96
) to maintain high precision in interest rate calculations without the need for floating-point numbers, which are not natively supported in Solidity.
Yearly Constants: Defines constants like YEAR_SECS
to account for the number of seconds in a year, considering leap years, facilitating the annualization of interest rates.
Configurable Parameters: Allows the contract owner to set parameters such as the base rate, multiplier, jump multiplier, and kink utilization rate per year, each scaled by Q96
for precision. These parameters determine how interest rates adjust with the protocol's utilization rate.
getUtilizationRateX96: Calculates the utilization rate given the current cash and debt within the protocol, returning a value scaled by Q96
. This rate is crucial for determining how close the protocol is to being fully utilized, which in turn influences interest rates.
getRatesPerSecondX96: Given the current cash and debt, calculates the borrow and supply rates per second, each scaled by Q96
. The calculation considers whether the current utilization rate is above or below the "kink" point, applying different multipliers accordingly to derive the final rates.
setValues: Allows the contract owner to update the model's parameters, including base rate, multipliers, and the kink utilization rate, each expressed on an annual basis and scaled by Q96
. This method provides the flexibility to adjust the interest rate model in response to changing market conditions or economic policies.
onlyOwner
modifier, ensuring that adjustments to the model are deliberate and authorized.This contract integrates chainlink and Uniswap v3 TWAP oracles, providing a robust mechanism for determining the value of Uniswap v3 LP positions in a specified token. It features a fallback mode for enhanced security, addressing the need for accurate and reliable price information within DeFi applications. By harnessing both Chainlink's feeds for external market prices and Uniswap v3's time-weighted average prices (TWAP), it offers flexibility in price source selection through various operational modes. These modes allow for verification of price consistency across oracles, reducing the risk of price manipulation. The oracle supports configuration for multiple tokens, enabling customization of price verification thresholds and update intervals to ensure data relevancy and integrity. This hybrid approach aims to balance decentralized oracle resilience.
The V3Vault contract utilizes Uniswap V3 LP positions as collateral within a framework that strictly adheres to the IERC4626 Vault Standard. It represents a seamless representation of ERC20 shares to denote participation in the lending pool, facilitating a singular ERC20 asset for both borrowing and lending operations. The architecture is deeply integrated with the Uniswap ecosystem, leveraging its Nonfungible Position Manager for efficient liquidity management and swaps. Incorporating a dynamic interest rate model and an oracle for accurate collateral assessment, it ensures competitive rates and secure collateralization. Moreover, it empowers administrators with extensive management capabilities, including adjustable limits and configurations, to adapt to evolving market conditions.
pool.observe()
is to to fail for any reason it's going to halt the system even if Chainlink's latestRoundData()
is the primary source for that tx and vice versa._checkApprovals()
for these tokensAutomator::setTWAPConfig()
efffectively affecting the priceof the token.<=
check.setMaxPoolPriceDifference()
to allow for the ingestion of faulty oracle data for innocent users by placing the acceptable max difference to be very high.V3Oracle._getReferencePoolPriceX96()
will show incorrect price for negative tick deltas, whcih would've been caught if these were considered during testing.MIN_PRICE_DIFFERENCE
is not the min price difference but actually the min max price difference, i.e looking at trying to set the max price difference we can see that it's checked to not be lower than our MIN_PRICE_DIFFERENCE.My attempt on reviewing the Codebase spanned over 60 hours distributed over 9 days:
The codebase was a very great learning experience, though it was a pretty hard nut to crack, nonetheless during my review, I uncovered a few issues within the protocol and they need to be fixed. Recommended steps should be taken to protect the protocol from potential attacks. Timely audits and codebase cleanup for mitigations should also be conducted to keep the codebase fresh and up to date with evolving security times.
60 hours
#0 - c4-pre-sort
2024-03-24T08:42:47Z
0xEVom marked the issue as sufficient quality report
#1 - jhsagd76
2024-04-01T11:25:21Z
emm, a little strange
#2 - c4-judge
2024-04-01T11:25:27Z
jhsagd76 marked the issue as grade-b