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.
* upstream/release/3.3.x: (41 commits)
chore: Bump version to 3.3.0
chore: Bump version to 3.3.0-rc7
fix: Increase manifest protocol message size cap and fix manifests relay
fix: Cap untrusted manifests per message and drop oversized ones
chore: Bump version to 3.2.1
chore: Bump version to 3.2.1-rc1
fix: Cap untrusted manifests per message and drop oversized ones
fix: Reject oversized validator manifest before decoding
fix: Reduce untrusted manifest cache cap to 100
fix: Bound untrusted manifest cache
chore: Bump version to 3.3.0-rc6
feat: Package validator-keys inside rippled
chore: Bump version to 3.3.0-rc5
fix: Switch SponsorshipSet to use a delta for sfFeeAmount
fix: Re-revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
chore: Bump version to 3.3.0-rc4
fix: Revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
chore: Bump version to 3.3.0-rc3
fix: Reduce untrusted manifest cache cap to 100
fix: Revert "fix: Reject oversized SHAMap nodes in gotStaleData and fetch-pack path"
...
* release/3.2.x:
chore: Bump version to 3.2.1
chore: Bump version to 3.2.1-rc1
fix: Cap untrusted manifests per message and drop oversized ones
fix: Reject oversized validator manifest before decoding
fix: Reduce untrusted manifest cache cap to 100
fix: Bound untrusted manifest cache
Bound the number of manifests carried in a single TMManifests message
(kMaxManifestsPerMessage). Trusted manifests are always included and
processed; untrusted gossip is capped per message on both send and
receive, and the sender is charged only when untrusted entries are
actually skipped. Oversized TMManifests messages are dropped without
penalty at the protocol layer so an unpatched peer is not disconnected.
Complements the cache bound from #276/#323.
