Include 1.04 COLLATERALIZATION_FACTOR

[mattcushman]

in neutral price used in kicks

Description

Include the COLLATERALIZATION_FACTOR in the computation of the neutral price.

Purpose

Resolves observation from Kirill that there are inconsistencies in how threshold price and derived quantities are computed. In particular, the neutral price was still be computed off the "raw" threshold price, meaning debt-to-collateral without the COLLATERALIZATION_FACTOR of 1.04 included. This resulted in unintentionally low neutral prices. This commit updates the toNpTpRatio to include that factor.

Impact

Gas

_kick: increase of 10K gas
kickerInfo: increase of 400
borrowerInfo: increase of 100 gas

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.

[mattcushman]

Include 1.04 factor,
COLLATERALIZATION_FACTOR
in kicks and PoolInfoUtils

[EdNoepel]

Comment inline.

[k1rill-fedoseev]

So the Loan.thresholdPrice still contains the unadjusted TP. I don't see any places where it's externally exposed, but the naming is confusing. Maybe it's worth renaming it to something like unadjustedThresholdPrice or similar, so it's harder to confuse it with all other things called TP in the code.

Same thing about Liquidation.thresholdPrice. It's exposed outside in auctionInfo and auctionStatus, so the same naming in NatSpec will also confuse integrators.

Apart from naming, LGTM

[ith-harvey]

So the Loan.thresholdPrice still contains the unadjusted TP. I don't see any places where it's externally exposed, but the naming is confusing. Maybe it's worth renaming it to something like unadjustedThresholdPrice or similar, so it's harder to confuse it with all other things called TP in the code.

Same thing about Liquidation.thresholdPrice. It's exposed outside in auctionInfo and auctionStatus, so the same naming in NatSpec will also confuse integrators.

Apart from naming, LGTM

I second Kirill's concern... if we have the same naming convention but have a different equation applied to the value on the back end it is likely we may miss something.

[mattcushman]

How about renaming these debtToCollateral or similar?

[dmitriia]

Looks ok. It should be documented that PoolInfoUtils.auctionStatus() and Pool.auctionInfo() return unadjusted threshold price, debt / collateral, which is used in BPF for kicker's reward calculation, while loan's TP and pool's HTP include the collateralization factor, being debt * CF / collateral, and they are compared to LUP in the loan collateralization and pool LUP movement checks.

Btw, naming can be updated, e.g.:

https://github.com/ajna-finance/contracts/blob/e11c752e5b42f3d3d3509c3772e94cbd4c245176/src/libraries/helpers/PoolHelper.sol#L403-L406

-   /// @dev max bond factor.
+   /// @dev min bond factor.
    uint256 constant MIN_BOND_FACTOR = 0.005 * 1e18;
-   /// @dev max NP / TP ratio.
-   uint256 constant MAX_NP_TP_RATIO = 0.03 * 1e18;
+   /// @dev max bond factor.
+   uint256 constant MAX_BOND_FACTOR = 0.03 * 1e18;

https://github.com/ajna-finance/contracts/blob/e11c752e5b42f3d3d3509c3772e94cbd4c245176/src/libraries/helpers/PoolHelper.sol#L479-L486

-       // bondFactor = max(min(0.03,(((NP/TP_ratio)-1)/10)),0.005)
+       // bondFactor = max(min(MAX_BOND_FACTOR, (NP/TP_ratio - 1) / 10), MIN_BOND_FACTOR)
        bondFactor_ = Maths.max(
            Maths.min(
-               MAX_NP_TP_RATIO,
+               MAX_BOND_FACTOR,
                (npTpRatio_ - 1e18) / 10
            ),
            MIN_BOND_FACTOR
        );