[WIP] feat: Partial Liquidation

[vlad-woof-software]

Overview

This PR introduces partial liquidation support to the Comet protocol.
Instead of always seizing all collateral from an underwater account,
the liquidator now brings the account's health factor up to a configured
targetHealthFactor threshold - seizing only as much collateral as needed.
This reduces unnecessary value extraction from borrowers and better aligns
with the protocol RFC.

What has been done

Configuration

• targetHealthFactor moved into CometConfiguration.Configuration struct - no longer a separate contract/interface, but a
  first-class config field
• targetHealthFactor initialized in Comet and ScrollComet constructors from config
• targetHealthFactor() added to CometMainInterface
• Configurator updated to expose setTargetHealthFactor management

Core logic

• Partial liquidation algorithm implemented in CometWithExtendedAssetList.absorbInternal: iterates collateral assets and either
  partially seizes one asset (stopping at targetHF) or fully seizes it and moves to the next
• Formula: ∆ = (targetHF × debt − totalCV) × FACTOR_SCALE / (LP × targetHF − CF)
• Falls back to original full-seizure behaviour when targetHF == 0

Tests

• Initial test suite added in test/partial-liquidation-test.ts covering basic scenarios: single-collateral partial seizure,
  multi-collateral mixed seizure, full-liquidation fallback
• Existing test suite fixed to account for new targetHealthFactor config field and updated makeProtocol setup

What is still pending (reason for WIP)

• Test coverage: edge cases and full flow not yet covered (e.g. baseBorrowMin boundary, dust positions, rounding behaviour)
• baseBorrowMin check: if remaining debt after partial seizure falls below baseBorrowMin, the protocol should fall through to
  full liquidation instead
• Configurator-level validation that LP × targetHF > CF for all assets (prevents invalid configuration that would cause revert or
  division by zero at runtime)
• Removal of remaining console.log debug calls from contracts
• Internal review
• Scenario / integration tests
• Audit-readiness review

[Im-Vishwjeet]

setTargetHealthFactor skips the MIN_TARGET_HEALTH_FACTOR check that the constructor enforces.

CometWithExtendedAssetList.sol:130 guards deployment with:

if (config.targetHealthFactor < MIN_TARGET_HEALTH_FACTOR) revert BadHealthFactor();
where MIN_TARGET_HEALTH_FACTOR = 105e16. But this setter writes any uint64 — including 0 — directly into configuratorParams[cometProxy].targetHealthFactor. Because governorOrMarketAdmin is broader than onlyGovernor, any market admin can quietly stage an out-of-range value.

Impact path:

Setter succeeds, event emits, getter reads back the invalid value — no revert.
The next deploy() call reverts in the Comet constructor with BadHealthFactor(), potentially after a governance proposal has already queued.
The phase-1 denominator mulFactor(LF, targetHF) - BCF becomes -BCF if targetHF = 0, which underflows uint256. Enforcing the bound at write time makes that state unreachable from storage.
Suggested:


function setTargetHealthFactor(address cometProxy, uint64 newTargetHealthFactor) external governorOrMarketAdmin {
    if (newTargetHealthFactor < MIN_TARGET_HEALTH_FACTOR) revert BadHealthFactor();
    uint64 oldTargetHealthFactor = configuratorParams[cometProxy].targetHealthFactor;
    configuratorParams[cometProxy].targetHealthFactor = newTargetHealthFactor;
    emit SetTargetHealthFactor(cometProxy, oldTargetHealthFactor, newTargetHealthFactor);
}
One wrinkle: MIN_TARGET_HEALTH_FACTOR currently lives as an internal constant inside CometWithExtendedAssetList.sol. To reference it from Configurator.sol without duplicating the literal, it probably wants to move to CometConfiguration.sol (or a shared constants file).

[Im-Vishwjeet]

Is targetHealthFactor intentionally a no-op in Comet.sol / ScrollComet.sol? I see it gets stored in the constructors but isn't referenced in their absorbInternal. Asking because the constant MIN_TARGET_HEALTH_FACTOR and the corresponding revert check only exist in CometWithExtendedAssetList.sol — wanted to confirm that's by design (interface uniformity) rather than an oversight before reading further.

[Im-Vishwjeet]

Since it breaks on the first asset that satisfies phase-1, the asset at the lowest assetsIn index is the one whose balance gets reduced. Under full liquidation the order didn't matter; under partial it does.

Example: index 0 is Asset A (LF 0.85), index 1 is Asset B (LF 1.05), both $1k. wantedCollateralValue is smaller for B (higher LF in the denominator), but A gets hit first because of index order. So governance's addAsset order effectively decides which collateral the borrower loses.

Is list order intended to be part of the spec here? If yes, worth a line in SPEC.md + a comment above absorbInternal so integrators know. If not, sorting assetsIn by LF desc before the loop would make it deterministic on economics instead of history.

[siposova-304]

https://revolut.me/r/eZ37KPjLkY

[siposova-304]

Balance tranfer account https://revolut.me/r/eZ37KPjLkY