Mirrors the equivalent guard already applied to VaultWithdraw: wrap
the dust check in try/catch so a sufficiently abused sfScale pushing
assetsTotal/assetsAvailable out of STAmount's representable range
returns tecPATH_DRY instead of surfacing as a raw overflow_error.
Also address other outstanding review nits: fix
VaultTransactorPrecision_test's suite module (tx -> app, matching
every other Vault suite), replace the "PR 2" comment in
VaultPrecisionFixture.h with a stable description, and tighten the
clampToAssetsTotalScale docstring in VaultHelpers.h.
Add pre-fix/post-fix VaultBugs_test.cpp coverage for the two
fixCleanup3_4_0 fixes not yet exercised by a dedicated regression: a
VaultDeposit overcrediting the vault across an IOU 16-digit scale
boundary, and a partial VaultWithdraw permanently locking a large IOU
vault by draining sfAssetsTotal to zero while shares remain
outstanding.
debitIsNonZeroDust converts assetsTotal/assetsAvailable to STAmount,
which a sufficiently abused sfScale can push out of STAmount's
representable range -- the same exposure already guarded around
clampToAssetsTotalScale a few lines below. Wrap it the same way,
returning tecPATH_DRY instead of letting the exception reach the
generic tefEXCEPTION handler in applySteps.cpp.
Also marks the clampToAssetsTotalScale catch as LCOV_EXCL: with the
debitIsNonZeroDust check now guarded first, that catch is only
reachable via RoundingMode::Upward carrying a value that was in range
under the ambient rounding mode just past the max representable
exponent -- not realistically triggerable from a test.
Mirrors the try/catch already used by VaultDeposit and VaultClawback
around the same helper, returning tecPATH_DRY instead of letting the
exception propagate.
The clamp block added in VaultWithdraw::doApply unconditionally returned
tecPRECISION_LOSS when the clamped payout was zero. That undid the
develop-side guard that deliberately permits fixed-share zero-asset
withdrawals in a fully-impaired vault (assetsTotalForWithdrawal == 0),
causing xrpl.tx.LoanRounding to fail on all three build-test configs
(linux-clang, linux-gcc-coverage, windows). Skip the clamp when
assetsWithdrawn is already zero, and also on the final-withdrawal path
where the value is overwritten with sfAssetsAvailable a few lines later.
Also addresses review feedback:
- Correct clampToAssetsTotalScale docstring to reflect that it returns a
non-negative magnitude; the caller applies the sign.
- Add XRPL_ASSERT precondition matching sibling helpers, and canonicalize
totalBefore/totalAfter under a single NumberRoundModeGuard so the
returned delta reflects only `delta`, not a rounding difference between
the two endpoints.
- Move VaultPrecisionFixture.h and VaultTransactorPrecision_test.cpp
from src/test/app/lending/ to src/test/app/vault/ so vault tests live
alongside the other vault-specific suites.
- Simplify inline commentary across VaultDeposit/VaultWithdraw/
VaultClawback.
sfAssetsTotal is stored on a coarser STAmount grid than sfAssetsAvailable
and the vault's trust line, so adding the same amount to all three
quantizes differently on each and leaves the vault's books disagreeing
with its actual holdings by a sub-ULP amount. Fix by clamping the
credited/withdrawn amount to what sfAssetsTotal can represent before
applying it to the other rails, via a shared clampToAssetsTotalScale
helper used by all three transactors.
- Deposit: clamp assetsDeposited downward to the assetsTotal grid, then
re-derive shares from the clamped amount so the depositor cannot
receive shares worth more than they paid. Return tecPRECISION_LOSS if
the clamp rounds the deposit to zero.
- Withdraw: clamp assetsWithdrawn upward (i.e. the vault pays out
slightly less) so it never pays out more than it can account for.
Shares are not re-derived, so the withdrawer receives slightly less
per share, favouring remaining holders.
- Clawback: same pattern as withdraw, guarded on assetsRecovered > 0 and
placed after the existing clamp-to-available.
Gated on fixCleanup3_4_0.
