Revert with `InvalidAmount` if unscaled amount to add / remove in `Deposit` library

[prateek105]

Description

• revert by default with InvalidAmount in Deposits.unscaledAdd and Deposits.unscaledRemove if amount is zero
• remove explicit code reverts in external LenderActions and TakerActions libraries and check amount not zero where flow should not revert (SettlerActions)

NOTE: default revert in Deposits library is still to be discussed, another approach would be to check amount and revert in LenderActions.moveQuoteToken and TakerActions._rewardBucketTake (but this will duplicate code)

Purpose

• In moveQuoteToken, some Lps were added to toBucket without any deposit being added to the bucket. This situation arises when toBucketScale is large enough to make unscaled amount to move 0.

• same situation could occur in bucketTake where the amount to remove from deposit can be 0 due to roundings

• Locations at which Deposits.unscaledAdd is used:

  LenderActions : addQuoteToken - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/LenderActions.sol#L194 moveQuoteToken - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/LenderActions.sol#L301

• Locations at which Deposits.unscaledRemove is used:

  LenderActions : moveQuoteToken - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/LenderActions.sol#L831 removeQuoteToken - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/LenderActions.sol#L831

  SettlerActions: settlePoolDebt - _settlePoolDebtWithDeposit - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/SettlerActions.sol#L408 settlePoolDebt - _forgiveBadDebt- https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/SettlerActions.sol#L483

  TakerActions: bucketTake - _rewardBucketTake - https://github.com/ajna-finance/contracts/blob/develop/src/libraries/external/TakerActions.sol#L659

Impact

Tasks

• [ ] Changes to protocol contracts are covered by unit tests executed by CI.
• [ ] Protocol contract size limits have not been exceeded.
• [ ] Gas consumption for impacted transactions have been compared with the target branch, and nontrivial changes cited in the Impact section above.
• [ ] Scope labels have been assigned as appropriate.
• [ ] Invariant tests have been manually executed as appropriate for the nature of the change.

[grandizzy]

please update comment of function with @dev calculated unscaled amount to move is 0 InvalidAmount()

[prateek105]

Looks ok. I'm little bit worried though about the long-term behavior of the pool as the scale grows indefinitely, while inflator resets

@dmitriia I think we had this discussion in our meetings and it was concluded that for the scale to grow to a number that can affect a pool in a significant manner will take a very long amount of time. Hence we ignore this. @mattcushman would be the best person to confirm this.

[mattcushman]

Ajna has some practical limitations in terms of prices and timescales/cumulative interest accrued in which it can operate well. There is a practical way to reset the inflator, but no practical way to reset the bucket scales (I suppose it could have implemented a mechanism where someone would pay the gas to iterate through all 7388 buckets, setting the scales to 1.0 and the deposit values to their underlying value). OTOH I think that this will take centuries to become an issue for any reasonable pool: the scales always accrue more slowly than the interest rate if there is no kick-able loan, and even at a sustained APY of 50%, 100 years still doesn't exceed 1 WAD in scale.

[dmitriia]

I think that this will take centuries to become an issue for any reasonable pool

Agree, it's just something to have in mind. Given that most buckets are empty and can be skipped having a scale reset function with something like scale for a given index > 100 * inflator requirement (implying that there should have been an inflator reset in the past) might be helpful.