Summary
The LendingPool rebalances liquidity after most transactions including deposit and withdraw. In the test suite this does not occur since they do not assign a CurveVault. But once a vault is assigned, LendingPool transactions always revert.
Finding Description
LendingPool.sol will attempt to rebalance liquidity after most transactions, e.g.:
function deposit(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
...
_rebalanceLiquidity();
...
}
function withdraw(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
...
_rebalanceLiquidity();
...
}
Source: LendingPool.sol#L225-L263
First we notice that the helper simply returns if the curveVault has not been set (and searching for "setCurveVault" confirms that no tests attempt this).
function _rebalanceLiquidity() internal {
if (address(curveVault) == address(0)) {
return;
}
...
}
Source: LendingPool.sol#L773-L776
Once it has been set, this function will calculate the target balances (configured as 20/80 by default, meaning 80% should go to the vault). Then it will either _depositIntoVault or _withdrawFromVault:
function _rebalanceLiquidity() internal {
...
uint256 totalDeposits = reserve.totalLiquidity;
uint256 desiredBuffer = totalDeposits.percentMul(liquidityBufferRatio);
uint256 currentBuffer = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
if (currentBuffer > desiredBuffer) {
uint256 excess = currentBuffer - desiredBuffer;
_depositIntoVault(excess);
} else if (currentBuffer < desiredBuffer) {
uint256 shortage = desiredBuffer - currentBuffer;
_withdrawFromVault(shortage);
}
...
}
Source: LendingPool.sol#L778-L790
Looking at deposit, it approves the vault and calls its deposit function:
function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
...
}
Source: LendingPool.sol#L800-L801
The vault is a ERC-4626: Tokenized Vaults style token vault. But the only part of interest is that we are expecting standard ERC-20 transfer from msg.sender call like so:
function deposit(uint256 assets, address receiver) external returns (uint256 shares) {
...
asset.safeTransferFrom(msg.sender, address(this), assets);
...
}
However the issue is that the LendingPool escrows tokens in the RToken contract instead.
function deposit(ReserveData storage reserve,ReserveRateData storage rateData,uint256 amount,address depositor) internal returns (uint256 amountMinted) {
...
IERC20(reserve.reserveAssetAddress).safeTransferFrom(
msg.sender,
reserve.reserveRTokenAddress,
amount
);
...
}
Source: ReserveLibrary.sol#L330-L334
Since the LendingPool has 0 reserve asset balance, _depositIntoVault causes the deposit transaction to revert.
Similarly, _withdrawFromVault would cause the Vault to send tokens to the LendingPool address and not the RToken address as expected.
Once a vault has been assigned, it may not be cleared:
function setCurveVault(address newVault) external onlyOwner {
require(newVault != address(0), "Invalid vault address");
...
}
Source: LendingPool.sol#L703-L708
However it that check can be bypassed by deploying a contract with an empty fallback function and then setting the vault to that address. That effectively makes _rebalanceLiquidity a no-op again.
Impact Explanation
The feature for rebalancing with a Curve Vault cannot be used. Once one is assigned, transactions to the LendingPool will revert.
The owner can then workaround this by changing to an empty mock vault contract instead, allowing the LendingPool to continue functioning without the rebalancing feature.
So overall it causes temporary down time and it's a feature they cannot enable.
Likelihood Explanation
Balancing with a vault is a significant feature. The team will likely want to enable this in production.
Proof of Concept
The following updates the e2e protocol-tests to show that a call to LendingPool.deposit reverts once a curve vault has been assigned. The second "it" test block is there to help explain what happened. Run with npm run test:e2e.
First create a new mock contract:
pragma solidity ^0.8.19;
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
contract MockYearnVaultV3 {
using SafeERC20 for IERC20;
IERC20 public immutable asset;
constructor(IERC20 _asset) {
asset = _asset;
}
function deposit(uint256 assets, address receiver) external returns (uint256 shares) {
asset.safeTransferFrom(msg.sender, address(this), assets);
}
function withdraw(
uint256 assets,
address receiver,
address owner,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256 shares) {
asset.safeTransfer(msg.sender, assets);
}
}
Then we can add the test with this git patch:
diff --git a/test/e2e/protocols-tests.js b/test/e2e/protocols-tests.js
index 3040a7e..02760a6 100644
--- a/test/e2e/protocols-tests.js
+++ b/test/e2e/protocols-tests.js
@@ -17,7 +17,8 @@ describe('Protocol E2E Tests', function () {
const FOUR_YEARS = 4 * ONE_YEAR;
const BASIS_POINTS = 10000;
@@ -17,7 +17,8 @@ describe('Protocol E2E Tests', function () {
const FOUR_YEARS = 4 * ONE_YEAR;
const BASIS_POINTS = 10000;
- before(async function () {
+
+ beforeEach(async function () {
[owner, user1, user2, user3, treasury, repairFund] = await ethers.getSigners();
contracts = await deployContracts(owner, user1, user2, user3);
const displayContracts = Object.fromEntries(Object.entries(contracts).map(([key, value]) => [key, value.target]));
@@ -32,6 +33,55 @@ describe('Protocol E2E Tests', function () {
}
});
+ it.only('Bug', async () => {
+
+ {
+
+ const MockYearnVault = await ethers.getContractFactory("MockYearnVaultV3");
+ const mockYearnVault = await MockYearnVault.deploy(contracts.crvUSD.target);
+
+
+ await contracts.lendingPool.setCurveVault(mockYearnVault.target)
+ }
+
+
+ const DEPOSIT_AMOUNT = ethers.parseEther('100');
+ await contracts.crvUSD.connect(user1).approve(contracts.lendingPool.target, DEPOSIT_AMOUNT);
+
+
+ await expect(
+ contracts.lendingPool.connect(user1).deposit(DEPOSIT_AMOUNT)
+ ).to.be.revertedWithCustomError(contracts.crvUSD, "ERC20InsufficientBalance").withArgs(
+ contracts.lendingPool.target,
+ 0,
+ DEPOSIT_AMOUNT * 80n / 100n
+ );
+
+
+ await expect(
+ contracts.lendingPool.setCurveVault(ethers.ZeroAddress)
+ ).to.be.revertedWith("Invalid vault address");
+
+
+
+
+ })
+
+ it.only('FYI: The same test but w/o a curve vault', async () => {
+
+
+
+ const DEPOSIT_AMOUNT = ethers.parseEther('100');
+ await contracts.crvUSD.connect(user1).approve(contracts.lendingPool.target, DEPOSIT_AMOUNT);
+ await contracts.lendingPool.connect(user1).deposit(DEPOSIT_AMOUNT);
+
+
+ const usdBalanceEscrowedInLendingPool = await contracts.crvUSD.balanceOf(contracts.lendingPool.target);
+ expect(usdBalanceEscrowedInLendingPool).to.be.eq(0);
+ const usdBalanceEscrowedInRToken = await contracts.crvUSD.balanceOf(contracts.rToken.target);
+ expect(usdBalanceEscrowedInRToken).to.be.eq(DEPOSIT_AMOUNT);
+ })
+
describe('RAAC Token', function () {
const TRANSFER_AMOUNT = ethers.parseEther('100');
Recommendation
This git patch will correct the issue by forwarding assets to/from the RToken escrow:
When applied, the POC test above fails (since deposit no longer reverts).
@@ -797,6 +797,8 @@ contract LendingPool is ILendingPool, Ownable, ReentrancyGuard, ERC721Holder, Pa
* @param amount The amount to deposit
*/
function _depositIntoVault(uint256 amount) internal {
+ // First we pull the assets out of RToken escrow:
+ IRToken(reserve.reserveRTokenAddress).transferAsset(address(this), amount);
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
@@ -809,5 +811,7 @@ contract LendingPool is ILendingPool, Ownable, ReentrancyGuard, ERC721Holder, Pa
function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
totalVaultDeposits -= amount;
+ // Transfer the withdrawn assets back to the RToken escrow
+ IERC20(reserve.reserveAssetAddress).transfer(reserve.reserveRTokenAddress, amount);
}
}
\ No newline at end of file
