LendingPool — Design Decisions & Accepted Trade-offs¶
The behaviours below are properties of the LendingPool and are intentional. Each has been raised in a previous security review and accepted as design rather than fixed, and each is recorded here so that reviewers can tell a deliberate trade-off from a defect. References are to the Pashov Audit Group reviews: v4 (2026-02, CORE Lending), v3 (2025-12, CORE), sweep (2025-11) and v2 (2025-08).
Raised in the most recent review¶
Pending protocol fees remain inside totalLiquidity¶
reserve.scaledPendingProtocolFee accumulates the protocol's share of borrower interest and stays part of reserve.totalLiquidity until claimProtocolFees() moves it to the FeeCollector.
- The fee is held scaled, so it accrues with the liquidity index and no interest on it is forfeited at claim time.
- Because it is counted as liquidity, utilisation
U = totalDebt / (totalLiquidity + totalDebt)is marginally understated while fees are unclaimed, which marginally dilutes the supply rate paid to depositors.
The size of the effect is bounded by how much fee is allowed to accumulate between claims. Realising the fee by minting RToken to the collector (AAVE v3 style) would remove it entirely; that change alters the RToken supply invariant and has been deliberately deferred.
claimProtocolFees is role-gated and liquidity-checked¶
The claim is onlyRole(PROTOCOL_ROLE) and calls _ensureLiquidity() for the full pending amount before transferring, then rebalances. A claim cannot be forced by a third party at an unfavourable moment, and it cannot underflow totalLiquidity during a high-utilisation window.
Unregistering an adapter is a governance operation, not a routine one¶
unregisterAdapter() does not check for outstanding positions. Since withdrawAsset, repay, repayOnBehalf, initiateLiquidation, closeLiquidation and finalizeLiquidation are all gated by onlySupportedAdapter, removing an adapter that still has live positions freezes those positions — debt keeps accruing through the usage index while the borrower cannot repay or withdraw, and cannot be liquidated.
This is accepted because adapter removal is a deliberate, low-frequency governance action. The operational requirement is that positions are unwound (or the adapter is re-registered) before removal is finalised.
Rebalancing runs before the liquidation cash arrives¶
finalizeLiquidation() burns the borrower's debt and credits reserve.totalLiquidity with the scaled amount, then calls _rebalanceLiquidity(). At that instant the StabilityPool has not yet transferred the corresponding pmUSD to the RToken contract, so the rebalance compares an already-credited totalLiquidity against an RToken balance that has not yet increased, and computes a larger shortfall than the end state warrants. The transfer lands later in the same liquidateBorrower transaction and restores the buffer.
The effect is a transient over-withdrawal from the yield vault inside one transaction, not a loss; the vault has no strategy and no exit penalty, so a round trip costs nothing beyond gas.
The reserve vault is assumed to be loss-free¶
LendingPool stores the yield venue as IBaseVault (an ERC-4626 subset) and validates only that vault.asset() matches the reserve asset. VaultProxy then re-casts that address to the extended vault interface to reach the methods that take loss and strategy parameters, passing zero and an empty array for them.
This is safe only because the configured vault runs no strategy: there is no loss path and share price is monotonic. Pointing the pool at a vault that satisfies IBaseVault but can realise losses is outside the model, and vault configuration is a PROTOCOL_ROLE action.
Acknowledged in earlier reviews¶
The table lists every finding on the LendingPool path that a previous review raised and the protocol accepted as design. The ones with material consequences are expanded below it.
| Ref | Review | Behaviour |
|---|---|---|
| L-10 | v3 · 2025-12 | Rounding direction is not uniformly pessimistic across the protocol |
| L-15 | v2 · 2025-08 | Insured positions cannot be finalised inside the grace period, however far the health factor falls |
| L-02 | v2 · 2025-08 | The same-block guard keys on block.number |
| L-20 | v3 · 2025-12 | deposit, openVaultPosition and borrowWithInsurance take no max-fee argument |
| L-13 | v3 · 2025-12 | getAssetValue lacks the onlySupportedAdapter modifier the sibling views carry |
| L-06 | v3 · 2025-12 | ERC721AssetAdapter.getAssetValue / getWithdrawValue skip the "is deposited" check |
| L-04 | v3 · 2025-12 | RToken / DebtToken / DEToken do not require the scaled mint/burn amount to be non-zero |
| L-03 | v2 · 2025-08 | deposit succeeds while feeCollector is unset, taking no deposit fee |
| L-05 | v2 · 2025-08 | IBaseVault.pricePerShare(bool) and the concrete vault's pricePerShare() do not match |
| L-13 | v2 · 2025-08 | ERC20AssetAdapter.deposit trusts the amount argument rather than the observed balance delta |
| L-48 | ALL-V1 · 2025-05 | Risk-parameter changes take effect immediately and can make a healthy position liquidatable |
Rounding is directional, not uniformly pessimistic¶
WadRayMath and PercentageMath round half-up; rayMulDown truncates. Which one is used where is a deliberate split, not an oversight:
- Balance-facing reads round down.
RToken.balanceOf,DETokenshare value and the StabilityPool's timelocked withdrawal cap all userayMulDown, so a holder can never read out more than the pool can pay. - Debt scaling rounds half-up.
_positionScaledDebtusesrayMul, so a borrower's debt is never understated. This direction was tightened deliberately after the 2025-11 sweep. _previewHealthFactormixes the two.collateralValuearrives floor-rounded from the adapter whilecollateralThresholdispercentMul(half-up), so the health factor can be overstated at the wei scale.
Per-operation error is ±1 on the least significant digit, against a MIN_BORROW_AMOUNT of 10e18 and a minDepositAmount of 10 reserve units, so no rounding path is reachable at a size that changes an outcome. Rewriting every step to round pessimistically in the protocol's favour (the AAVE v3.5 approach) was reviewed and deferred — it touches every accrual path at once, and the current split already puts the conservative direction on the two that matter: what a depositor can take out, and what a borrower owes.
Insured positions are protected for the full grace period¶
finalizeLiquidation reverts with GracePeriodNotExpired while position.isInsured and the grace period has not elapsed — regardless of how far the health factor has fallen in the meantime. If collateral value drops sharply inside that window the position can become unrecoverable, and the shortfall reaches the StabilityPool.
This is the insurance product working as sold: the borrower paid InsuranceFee on the whole outstanding debt precisely to buy an uninterruptible window in which to repay. A second, lower "insurance override" threshold that could cut the window short would make the guarantee conditional, and the guarantee is what the fee is for. The exposure is bounded instead on the other side — through BorrowThreshold (50%), LiquidationThreshold, the LiquidationGracePeriod length, and real-estate collateral whose drawdowns are gradual.
The same-block guard keys on block.number¶
depositBlock[user] = block.number and isUserDepositInSameBlock() are what block a deposit and an RToken transfer or withdrawal landing in the same block. On a rollup with sequencer-controlled block numbers this guard is weaker than it looks, since paired actions can be split across adjacent L2 blocks inside one L1 batch.
RAAC Core deploys to Ethereum mainnet only, where block.number is exactly the atomicity boundary the guard assumes. The concern would become live on an L2 deployment, at which point the guard should move to a block.timestamp-based per-user window like the one StabilityPool already uses for withdrawals.
Risk-parameter changes take effect immediately¶
LiquidationThreshold and BorrowThreshold are applied the moment setParameter lands. Tightening LiquidationThreshold therefore makes positions that were healthy a block earlier immediately liquidatable, with no window in which the borrower can add collateral or repay.
Putting the two thresholds behind a timelock was reviewed and not adopted: a delay cuts both ways, and a threshold that cannot be tightened promptly is the wrong trade when collateral is repricing. This is the deliberate split from the table above — the fee bucket accepts governance lead time, the risk bucket does not.
What stands in for the timelock is an operational pre-check: before a threshold change is submitted, the protocol analyses the live position set to determine which positions the new value would make liquidatable, and sizes the change against that. The parameter is not moved blind.
The structural protections apply on top: BorrowThreshold must stay strictly below LiquidationThreshold, so a single write cannot cross the two; a position must first be flagged by initiateLiquidation and cannot be seized in the same transaction as the parameter change; and an insured borrower still gets the full LiquidationGracePeriod to repay. PROTOCOL_ROLE is a trusted role and every change emits ParameterChanged.
Fee parameters are not pinned per transaction¶
deposit(), openVaultPosition() and borrowWithInsurance() charge depositFee, vaultOpeningFee and insuranceFee at whatever value the parameter holds when the transaction lands; the caller cannot pass a maximum it is willing to pay. A parameter change that lands first therefore applies to transactions already in the mempool.
All four affected fees are GOVERNANCE_ROLE-gated, so a change cannot appear without warning: it is a governance proposal that has to be voted, queued and wait out the timelock before it can execute, and it is visible on-chain for that whole period. What remains is the narrow race at execution — a user who submits a deposit in the same window as the executing proposal pays the new fee. The exposure is a fee delta on in-flight transactions, not a loss of principal.
View functions are deliberately unguarded¶
LendingPool.getAssetValue() takes an arbitrary adapter without onlySupportedAdapter, and ERC721AssetAdapter.getAssetValue / getWithdrawValue return an oracle price without checking that the caller's tokenId is actually deposited.
These are view functions with no state effect. Every path that acts on a value — calculateHealthFactor, _validateBorrow, withdrawAsset, the liquidation entry points — validates the adapter itself before using it. The looser views exist so integrators can price a position that is not yet open. Integrators must not treat a non-zero return from either as proof that a position exists.
Token mint and burn do not require a non-zero scaled amount¶
RToken, DebtToken and DEToken update _rawTotalDeposits / _rawTotalBorrows with amount.rayDiv(index) while minting the unscaled amount. If the scaled value truncated to zero the raw totals would drift from the actual supply.
The path requires minDepositAmount == 0 and an index above ~2e27. minDepositAmount is set to 10 reserve units in the constructor and lowering it to zero is a PROTOCOL_ROLE action, so the precondition is a configuration the protocol does not run.
feeCollector is set at deployment and never unset¶
deposit() treats feeCollector == address(0) as a zero deposit fee rather than reverting, and the liquidity-reward leg of a liquidation would revert if it were unset with liquidityPercentage > 0. The collector is set at deployment and is not expected to be removed, so the zero-address branch is a constructor-ordering safeguard, not a supported operating state.
pricePerShare signatures differ between interfaces¶
IBaseVault declares pricePerShare(bool isFloor) while the concrete reserve vault exposes pricePerShare(). Only the no-argument form is called in practice — VaultProxy reaches it through ICurveCrvUSDVault, and LendingPool never calls the IBaseVault variant. The stale declaration is a documentation defect on the interface, not a live miscall.