Address review comments on PR #8002:
- LoanPay.cpp: pre-amendment description incorrectly claimed the vault
pseudo-account received the raw Number. It received the vaultScale-
rounded value via accountSendMulti; the real asymmetry is vault-side
between sfAssetsAvailable (rounded) and sfAssetsTotal (unrounded via
assetsTotalDelta).
- LoanManage.cpp: replace the associateAsset-based justification for the
T - A >= totalDefaultAmount guard with the actual invariant: the
fields loanVaultExposure differences are whole multiples of the same
immutable 10^sfLoanScale and never outgrow STAmount's 16 digits, so
the promotion round-trips losslessly. Also drops the misleading
"vault-associated" descriptor (the underlying fields live on the
loanSle).
- LoanRounding_test.cpp: the "loan scale ~ -13 vs vaultScale ~ -9" note
is impossible under computeLoanProperties (which clamps loanScale to
max(vaultScale, amount.exponent())). Rewrite to describe the actual
loanScale == vaultScale regime the scenario exercises.
Adds testMultiLoanDefaultDriftFixVsLegacy which runs the same
drift-inducing scenario (3 loans, different scales, partial payments,
impairment, then default) under both !fixCleanup3_4_0 and
fixCleanup3_4_0 and confirms both paths leave the vault
invariant-safe.
Empirically the two branches produce identical post-default state
because computeLoanProperties clamps loanScale >= vaultScale (via
std::max(minimumScale, amount.exponent())), so the pre-amendment
roundToAsset(vaultDefaultAmount, vaultScale, Down) is a no-op for
well-formed loans. The dust-reconciliation branch this test guards
against is doubly-defensive code — reachable only from a corrupted
ledger, not a valid transaction sequence.
The test acts as a regression sensor: any future change that breaks
the invariant on either branch (or lets them diverge) will surface
here.
Gated behind fixCleanup3_4_0. Pre-amendment, the payment path used the
same cross-scale pattern that `LoanManage::defaultLoan` addressed:
`totalPaidToVault` was rounded down to `vaultScale` before being applied
to `sfAssetsAvailable`, while `assetsTotalDelta` (`valueChange` for
accrual, `interestPaid` for cash-basis) was applied at the finer loan
scale. The `sfAssetsAvailable <= sfAssetsTotal` invariant was only
defended by an assertion after the fact.
Under the fix, drop the `roundToAsset(..., vaultScale, Downward)` step
and apply the raw `totalPaidToVault` to both the ledger update and the
cash transfer via `accountSendMulti`. The pseudo-account balance and
`sfAssetsAvailable` therefore land on the same STAmount value without
an intermediate scale reduction.
For integral assets this is a no-op (the removed rounding was already a
no-op there). For non-integral IOUs, the vault receives the full raw
payment (up to 1 vaultScale ULP more than pre-amendment) and the
existing `tecPRECISION_LOSS` / `tecINTERNAL` post-checks are no longer
needed as safety nets against an arithmetic-induced Available > Total.
`LoanPay_test` now runs the amendment-sensitive tests under both
branches by adding `fixCleanup3_4_0` to the `amendmentCombinations`
matrix, matching the sibling change to `LoanRounding_test`.
Gated behind fixCleanup3_4_0. Pre-amendment, `defaultLoan` mutated
`sfAssetsTotal` and `sfAssetsAvailable` with values at different scales --
`vaultDefaultAmount` rounded down to `vaultScale`, but `defaultCovered`
kept at the finer loan scale. That asymmetry could leave
`sfAssetsAvailable > sfAssetsTotal` from the arithmetic alone, and was
patched by a dust-reconciliation branch that snapped `Total` up to
`Available`. That snap effectively minted phantom assets on `Total`
(see `LoanRounding_test::testDustManipulation`).
Under the fix, the vault-side update is composed from two well-defined
STAmount operations at the vault asset's own scale:
(1) Write-off: `sfAssetsTotal -= totalDefaultAmount` -- the defaulted
loan's exposure leaves the vault.
(2) Symmetric cash inflow: `defaultCovered` returns from first-loss
capital. Apply the same STAmount to both `sfAssetsTotal` and
`sfAssetsAvailable`, preserving the gap.
Because both deltas are STAmounts of the same asset, no cross-scale
rounding step is needed and `Available > Total` cannot occur from
arithmetic. The dust-branch snap and its downstream `tecINTERNAL`
guard are unreachable under the fix and only run on the pre-amendment
path.
The pre-mutation sanity check on the fix path tightens to the correct
invariant (`sfAssetsTotal - sfAssetsAvailable >= totalDefaultAmount`);
the pre-amendment path retains its original `vaultTotalProxy <
vaultDefaultAmount` check for byte-for-byte compatibility with existing
ledgers.
`LoanRounding_test` now runs `testDustManipulation` (and the other
amendment-sensitive rounding tests) under both branches by adding
`fixCleanup3_4_0` to the `amendmentCombinations` matrix. Both branches
land the same post-default equality for the specific test setup; the
fix path reaches it by clean arithmetic rather than a snap.
