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.
Adds unit-test coverage for three previously-untested branches in
VaultHelpers:
- addVaultAssets propagates a non-tes return when the underlying
accountSend fails (exercised via a sender with no trust line for the
vault asset).
- clawbackVaultAssets propagates a non-tes return when its accountSend
fails (exercised via a synthetic third-party recipient with no trust
line; the production caller always uses the asset issuer, which
cannot hit this branch).
- moveVaultAssets short-circuits and returns tesSUCCESS without
touching accountSendMulti when every recipient's amount is zero
(exercised via two zero-amount recipients, which still satisfies the
recipients.size() > 1 precondition).
- Restore the doc comment on STAmount::isRounded that was lost when it
moved out of LendingHelpers.cpp.
- Collapse LoanManage::defaultLoan's Vault-update block into a direct
return of addVaultAssets, since there is no follow-up work after it.
- Mark removeVaultAssets's amount==0 early-return with LCOV_EXCL_LINE: it
is a defensive short-circuit for a branch that is only reachable in a
vanishingly rare edge case (final withdrawal from a vault whose
sfAssetsAvailable has already been written down to zero) and is not
reached by any current transactor-level test.
Mark `assetsAvailable`/`assetsTotal` in VaultClawback::doApply and
`assetsTotalBefore` in VaultWithdraw::doApply as [[maybe_unused]]: each
is only referenced from XRPL_ASSERT, which is compiled out in Release
(NDEBUG) builds. Without the attribute clang -Werror,-Wunused-variable
would break the release CI configurations, matching the pattern already
addressed for VaultHelpers and LoanPay.
Co-authored-by: Cursor <cursoragent@cursor.com>
Mark the raw pre-rounding projections `assetsAvailableAfterRaw` and
`assetsTotalAfterRaw` in LoanPay::doApply as [[maybe_unused]]: they are
only referenced from XRPL_ASSERT_PARTS, which is compiled out in Release
(NDEBUG) builds, so clang's -Werror,-Wunused-variable was breaking the
ubuntu-clang-release-amd64 and macos-arm64-release CI jobs.
Co-authored-by: Cursor <cursoragent@cursor.com>
Mark the `Asset const asset` local in addVaultAssets and removeVaultAssets
as [[maybe_unused]]: it is only referenced from XRPL_ASSERT, which is
compiled out in Release (NDEBUG) builds, so clang's -Werror,
-Wunused-variable was breaking the ubuntu-clang-release-amd64 CI job.
The moveVaultAssets copy is used in real code paths and is unaffected.
Exercise addVaultAssets, clawbackVaultAssets, removeVaultAssets
(including FinalRemoval::Yes hard-reset and the amount > sfAssetsAvailable
failure path), and moveVaultAssets directly against a real Vault SLE and
ApplyView, built via jtx but bypassing the VaultDeposit/VaultWithdraw/
VaultClawback/LoanSet transactors. Covers zero-amount edges, negative
valueDelta, and the independent amount/valueDelta contract, in addition
to the existing transactor-level coverage in Vault_test.cpp,
LoanSet_test.cpp, etc.
Wire VaultDeposit, VaultWithdraw, VaultClawback, LoanSet, LoanManage, and
LoanPay through addVaultAssets/removeVaultAssets/clawbackVaultAssets/
moveVaultAssets, replacing their ad hoc sfAssetsTotal/sfAssetsAvailable
proxy mutations and accountSend/accountSendMulti/doWithdraw calls:
- VaultDeposit and VaultWithdraw/VaultClawback use addVaultAssets and
removeVaultAssets/clawbackVaultAssets respectively. VaultClawback uses
the plain-accountSend overload rather than the doWithdraw-based one,
since doWithdraw's self-holding-creation path would incorrectly reject
a locked/frozen MPT issuance before checking that the recipient is the
issuer (who always already holds their own asset).
- LoanSet's loan-principal-and-origination-fee disbursement uses
moveVaultAssets.
- LoanManage's default write-off and LoanPay's payment both use
addVaultAssets; addVaultAssets always applies valueDelta even when the
transferred amount is zero, since a default written off entirely by
the Vault (no first-loss capital cover) has a nonzero total delta but
a zero transferred amount.
- LoanPay's post-rounding precision-loss guard now reads the Vault's
actual post-rounding field values (after addVaultAssets/associateAsset
have mutated and rounded them) instead of comparing raw pre-mutation
arithmetic.
- associateAsset(*vault, ...) must run after the helper mutates the
Vault's fields, never before: it rounds every asset-typed field on the
Vault SLE to the asset's canonical precision, which the mutation
itself does not do. Getting this ordering backwards is what caused a
"value is already rounded" STNumber assertion failure during testing.
Also rename getAssetsTotalScale to VaultHelpers::getVaultScale at all
call sites.
Add addVaultAssets, removeVaultAssets (two overloads: plain accountSend
and doWithdraw-based), clawbackVaultAssets, and moveVaultAssets as the
single points through which a Vault's sfAssetsTotal/sfAssetsAvailable are
mutated and funds move to/from its pseudo-account:
- addVaultAssets increases both fields and transfers in from a sender.
- removeVaultAssets/clawbackVaultAssets decrease both fields equally and
transfer out; a FinalRemoval flag hard-resets both fields to exactly
zero on a Vault's last withdrawal, since the discounted exchange-rate
formula can produce values with more precision than the asset can
canonically represent, and subtracting such a value would leave a
non-canonical residual instead of an exact zero.
- moveVaultAssets decreases only sfAssetsAvailable, for disbursements
(e.g. a loan's principal and origination fee) where sfAssetsTotal
independently grows via accrued interest.
Also consolidate getAssetsTotalScale into VaultHelpers::getVaultScale,
and move isRounded from LendingHelpers into STAmount.h alongside the
other rounding utilities.
