V3.3.0 #87
V3.3.0 #87
Conversation
Tracking of reserve deficit and instant removal of bad debt. This newly introduced feature allows the removal of excess debt. When a users collateral if fully liquidated but debt is left in the protocol, the debt generates yield although it is expected to never be payed back. With the reserve deficit tracking, this debt is immediately burned and tracked in a local state. In addition a new method is introduced which allows burning aTokens in order the resolve the realized debt. --------- Co-authored-by: Lukas <[email protected]>
* fix: refine docs * fix: typo
accessing the configuration cache should be save in this case and reduces gas overhead by around 300gas per access
umbrella should be a single address tied to the pool, so using acl is overkill, which also will be cheaper
- adding `getReserveVariableDebtToken` getter - adjusting close factor to apply to whole position, not per reserve - adjusting close factor to increase to 100% when debt/collateral are below a threshold - adjusting liquidations to force full liquidation/or keep a leftover that is high enough to allow further liqudiations
* fix: migrate to dedicated getters * test: cleanup tests * feat: add pdp getter * fix: remove unused validate * refacto: remove getReserveDataExtended
Co-authored-by: Lukas <[email protected]>
- refactor to assume the umbrella never has debt Co-authored-by: Ernesto Boado <[email protected]>
| @@ -50,8 +50,9 @@ library DataTypes { | |||
| uint128 variableBorrowIndex; | |||
| //the current variable borrow rate. Expressed in ray | |||
| uint128 currentVariableBorrowRate; | |||
| // DEPRECATED on v3.2.0 | |||
| uint128 __deprecatedStableBorrowRate; | |||
| /// @notice reused `__deprecatedStableBorrowRate` storage from pre 3.2 | |||
There was a problem hiding this comment.
this is more a @dev than a @notice imho
how do we know that starts with 0 value?
There was a problem hiding this comment.
we checked, but also on the upgrade proposal we're iterating all reserves and ensure it's 0.
| * @param caller The caller that triggered the DeficitCovered event | ||
| * @param amountCovered The amount of deficit covered | ||
| */ | ||
| event DeficitCovered(address indexed reserve, address caller, uint256 amountCovered); |
There was a problem hiding this comment.
does it make sense to make caller indexed? It's basically the liquidator right
There was a problem hiding this comment.
It's the address calling the function and in effect burning it's aTokens / GHO - it's a permissioned entity registered as UMBRELLA on the addresses provider.
I don't see the point in making it indexed though.
There was a problem hiding this comment.
In case there are multiple Umbrella instances could be useful. If gas costs are not relevant I may be a good idea to add it.
| * an amount of debt corresponding to `MAX_LIQUIDATION_CLOSE_FACTOR`. | ||
| * A value of 0.95e18 results in 0.95 | ||
| * @dev This constant represents a base value threshold. | ||
| * If the total collateral or debt on a position is below this threshold, the close factor is raised to 100%. |
There was a problem hiding this comment.
is it worth clarifying is in value terms? no token amounts
| * A value of 0.95e18 results in 0.95 | ||
| * @dev This constant represents a base value threshold. | ||
| * If the total collateral or debt on a position is below this threshold, the close factor is raised to 100%. | ||
| * @notice The default value assumes that the basePrice is usd denominated by 8 decimals and needs to be adjusted in a non USD-denominated pool. |
There was a problem hiding this comment.
this is more a @dev imho
* fix: remove gas diff - it's outdated and we now have it on every commit to main * fix: address miguels feedback * ci: bump workflows give permissions to write commit comments * fix: bump revisions
| : IERC20(params.asset).balanceOf(msg.sender); | ||
| require(balanceWriteOff <= userBalance, Errors.NOT_ENOUGH_AVAILABLE_USER_BALANCE); | ||
|
|
||
| if (reserveCache.reserveConfiguration.getIsVirtualAccActive()) { |
There was a problem hiding this comment.
small opt - cache this bool
| // assets without virtual accounting can never be a collateral | ||
| bool isCollateral = userConfig.isUsingAsCollateral(reserve.id); | ||
| if (isCollateral && balanceWriteOff == userBalance) { | ||
| userConfig.setUsingAsCollateral(reserve.id, false); |
There was a problem hiding this comment.
have we thought about imposing Umbrella not having the aTokens being used as collateral? Maybe we can even replace the isBorrowingAny with this check, and simplify this part.
There was a problem hiding this comment.
Initially it actually was like that, but eventually we got convinced that with umbrella being mutable via addresses provider it doesn't make to much sense to optimize here.
There was a problem hiding this comment.
not sure I follow the rationale. My point was about imposing a specific state for Umbrella and do not see how being mutable relates.
| require(isActive, Errors.RESERVE_INACTIVE); | ||
|
|
||
| uint256 balanceWriteOff = params.amount; | ||
|
|
There was a problem hiding this comment.
this line break is unnecessary imo
| ) { | ||
| bool isDebtMoreThanLeftoverThreshold = ((vars.userReserveDebt - vars.actualDebtToLiquidate) * | ||
| vars.debtAssetPrice) / | ||
| vars.debtAssetUnit >= |
There was a problem hiding this comment.
do we need this? MIN_LEFTOVER_BASE has same number of decimals (8)
Same for the collateral side
There was a problem hiding this comment.
can you elaborate, I don't understand the question tbh.
There was a problem hiding this comment.
asking if we need to divide by asset units, as both things we are comparing are denominated in same terms (8 units)
| DataTypes.UserConfigurationMap storage userConfig, | ||
| address user, | ||
| address debtAsset, | ||
| uint256 userReserveDebt, |
There was a problem hiding this comment.
missing this as @param in the docs
typo fixes Co-authored-by: miguelmtz <[email protected]>
* Added LiquidationDataProvider contract * Minor changes from comments in PR * Minor changes from comments in PR 2 * Added one more function to the LiquidationDataProvider contract + fixed one bug in the _isReserveReadyForLiquidations function * Added the LiquidationDataProvider contract to deployment scripts * Replaced the LiquidationHelper library with the LiquidationDataProvider contract in the PoolLiquidationTests test * fix: refactor tests where possible * fix: add tests * fix: add tests * Removed the _getReserveFullInfo function from the LiquidationDataProvider contract --------- Co-authored-by: Lukas <[email protected]>
Co-authored-by: Lukas <[email protected]>
Summary
v3.3 version of the Aave protocol, including:
Changelog
-> Core
ReserveConfiguration
Pool
eliminateReserveDeficit(), by a new Umbrella role registered on the PoolAddressesProvider.getReserveDataExtended(), marked as deprecated on 3.2.Errors
DataTypes
__deprecatedStableBorrowRate(not exposed already) has been replaced bydeficitConfiguratorLogic
ReserveLogic
getIsVirtualAccActive().eliminateDeficit()functionLiquidationLogic
-> Periphery
AaveProtocolDataProvider
getReserveDeficit()getter.UIPoolDataProvider
getReservesData()now has an extradeficitfield.WrappedTokenGatewayV3
ParaswapAdapters
EmissionManager
setRewardOracle()now requires a different input type for rewardOracle, but backwards compatibleRewardsController
All instances of the ChainlinkOracle interface have been aligned to a single version containing all methods.
While we are aware that non of the contract actually needs all methods, it improves DX within the aave-v3-origin repo to assume they do.
Steps planned for upgrade stage
deficitstorage variables are zero, to assure that there is not “dirty” data from the previous __deprecatedStableBorrowRate. Not on initialise because that would be the only aspects added there, and it is not worth it.