From de6e5d3a94ef79b25b2036d4f02e229c5eddc822 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:51:39 +0000 Subject: [PATCH] fix: Keep VaultDeposit share count after the assetsTotal clamp (#8140) --- .../tx/transactors/vault/VaultDeposit.cpp | 29 ++----------------- .../vault/VaultTransactorPrecision_test.cpp | 9 ++++-- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index adb8b3f8f2..fc72159444 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -347,37 +347,14 @@ VaultDeposit::doApply() if (fix340Enabled) { // Round down at the posterior sfAssetsTotal scale so the vault is credited by no more - // than the depositor paid. + // than the depositor paid. Keep the share count from the first round trip: the clamp + // only drops a last digit of the new total. Converting the clamped amount back to + // shares would mint fewer shares while still charging the N-share debit. auto const maybeClamped = clampToAssetsTotalScale(vault, assetsDeposited); if (!maybeClamped) return maybeClamped.error(); assetsDeposited = *maybeClamped; - // The pre-clamp share count would over-issue by the trimmed ULP and give the depositor - // more value than they credited. - auto const maybeReShares = assetsToSharesDeposit(vault, sleIssuance, assetsDeposited); - if (!maybeReShares) - return tecINTERNAL; // LCOV_EXCL_LINE - - sharesCreated = *maybeReShares; - - if (sharesCreated == beast::kZero) - return tecPRECISION_LOSS; - - // The re-derived share count would over-issue if it round-trips back to more assets - // than the clamped amount actually paid. Unreachable unless a conversion helper is - // broken. - // LCOV_EXCL_START - auto const maybeReAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); - if (!maybeReAssets) - return tecINTERNAL; - if (*maybeReAssets > assetsDeposited) - { - JLOG(j_.error()) << "VaultDeposit: would take more than offered."; - return tecINTERNAL; - } - // LCOV_EXCL_STOP - // The actual deposit amount is truncated to whole shares, converted back to assets, // and clamped to the sfAssetsTotal scale (post-fixCleanup3_4_0). Check the depositor's // balance here—after clamping—before making any state changes. diff --git a/src/test/app/vault/VaultTransactorPrecision_test.cpp b/src/test/app/vault/VaultTransactorPrecision_test.cpp index e06c87ee68..8e8ee2d629 100644 --- a/src/test/app/vault/VaultTransactorPrecision_test.cpp +++ b/src/test/app/vault/VaultTransactorPrecision_test.cpp @@ -92,9 +92,14 @@ class VaultTransactorPrecision_test : public VaultPrecisionFixture if (before.sharesTotal == Number{0}) continue; Number const shareValue = (before.assetsTotal * sharesMinted) / before.sharesTotal; + // The depositor is never charged more than the shares they received are worth. BEAST_EXPECTS( - shareValue <= tDelta, - "amount=" + std::to_string(amount) + " shareValue > assetsTaken"); + tDelta <= shareValue, + "amount=" + std::to_string(amount) + " assetsTaken > shareValue"); + // Discount is strictly less than one ULP of the new AssetsTotal + BEAST_EXPECTS( + shareValue - tDelta < oneUnit(asset.raw(), after.assetsTotal), + "amount=" + std::to_string(amount) + " discount is not below one unit"); } {