Smart contract unable to operate due to lack of token funds
Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
If Trove Manager contract contains a single trove and that trove is below its MCR all redemption attempts will revert until the the Trove is liquidated.
Vulnerability Details
The redemption method redeem_collateral() callable from the Protocol Manager contract attempts to redeem collateral from the lowest collateralized troves (those with lowest CR). It calls get_all_assets_info() which gets lowest CR trove from each asset. Then if that trove is below CR it will try to get the next one until it finds the lowest CR, but still higher than MCR:
The problem is when there is no more troves available and the get_prev() returns a null identity. The get_current_icr() will then revert because the get_current_icr() calls internal_get_current_icr(), which calls internal_get_entire_debt_and_coll(), which looks like this:
fn internal_get_entire_debt_and_coll(borrower: Identity) -> EntireTroveDebtAndColl {
let trove = storage.troves.get(borrower).try_read().unwrap_or(Trove::default()); // <- condition handled
let coll = trove.coll;
let debt = trove.debt;
let pending_coll_rewards = internal_get_pending_asset_reward(borrower); // <- issue here
let pending_debt_rewards = internal_get_pending_usdf_reward(borrower);
/// [...]
}
The revert will happen on an optimistic attmpt to get a reward_snapshot using a null identity here:
Add a condition to check if get_prev() returns a null Identity here:
while (current_borrower != null_identity_address() && current_cr < MCR) {
current_borrower = sorted_troves.get_prev(current_borrower, asset);
// if current_borrow is null, break.
current_cr = trove_manager.get_current_icr(current_borrower, price);
}
Impact Details
This situation is rare as we can expect that the liquidation will eventually proceed and there will be no troves in that Trove Manager. Hence the situation is only short lived, and the condition for it to happen is very specific. Yet it looks like the condition's logic was not programmed correctly and the system could revert where it shouldn't. As the conditions for it to happen are rare and temporary we give it a Low impact.
