diff --git a/include/xrpl/ledger/helpers/VaultHelpers.h b/include/xrpl/ledger/helpers/VaultHelpers.h index acbf2c3ac0..b3c04308cd 100644 --- a/include/xrpl/ledger/helpers/VaultHelpers.h +++ b/include/xrpl/ledger/helpers/VaultHelpers.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -41,6 +42,27 @@ assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount co [[nodiscard]] std::optional sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares); +/** + * Clamps `delta` (positive when crediting the vault, negative when debiting + * it) to the largest magnitude that changes sfAssetsTotal by an exact + * multiple of its own STAmount grid step, rounding in `mode`. The returned + * delta has the same sign convention as `delta` (i.e. this is a magnitude + * adjustment toward zero, never away from it). Applying the returned delta + * to both sfAssetsTotal and any other field derived from the same raw amount + * (e.g. sfAssetsAvailable) keeps them exactly in sync, since those fields' + * scale is always at least as fine as sfAssetsTotal's. + * + * @param vault The vault SLE (mutable, since sfAssetsTotal is read through a + * non-const field proxy). + * @param delta The signed amount by which sfAssetsTotal is about to change. + * @param mode The rounding mode to apply when quantizing to sfAssetsTotal's + * scale. + * + * @return The clamped delta. + */ +[[nodiscard]] STAmount +clampToAssetsTotalScale(SLE::ref vault, STAmount const& delta, Number::RoundingMode mode); + /** * Controls whether to truncate shares instead of rounding. */ diff --git a/src/libxrpl/ledger/helpers/VaultHelpers.cpp b/src/libxrpl/ledger/helpers/VaultHelpers.cpp index 67e0262e14..74514f7fe7 100644 --- a/src/libxrpl/ledger/helpers/VaultHelpers.cpp +++ b/src/libxrpl/ledger/helpers/VaultHelpers.cpp @@ -67,6 +67,18 @@ sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount co return assets; } +[[nodiscard]] STAmount +clampToAssetsTotalScale(SLE::ref vault, STAmount const& delta, Number::RoundingMode mode) +{ + Asset const asset = *vault->at(sfAsset); + STAmount const totalBefore{asset, *vault->at(sfAssetsTotal)}; + STAmount const totalAfter = [&]() { + NumberRoundModeGuard const mg(mode); + return STAmount{asset, *vault->at(sfAssetsTotal) + delta}; + }(); + return delta.negative() ? totalBefore - totalAfter : totalAfter - totalBefore; +} + [[nodiscard]] std::optional assetsToSharesWithdraw( SLE::const_ref vault, diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index d77286b667..8e85dc785a 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -325,6 +325,18 @@ VaultClawback::assetsToClawback( return std::unexpected(tecPATH_DRY); } + if (ctx_.view().rules().enabled(fixCleanup3_4_0) && assetsRecovered > beast::kZero) + { + // sharesDestroyed is deliberately NOT re-derived from the clamped + // amount: the holder's shares are burned for their pre-clamp value, + // so any sub-ULP amount trimmed off here is left behind in the + // vault, favouring the remaining shareholders. + assetsRecovered = + clampToAssetsTotalScale(vault, -assetsRecovered, Number::RoundingMode::Upward); + if (assetsRecovered <= beast::kZero) + return std::unexpected(tecPRECISION_LOSS); + } + return std::make_pair(assetsRecovered, sharesDestroyed); } diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index a3c0a94eb5..65ba4d351c 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -208,6 +208,7 @@ TER VaultDeposit::doApply() { bool const fix320Enabled = view().rules().enabled(fixCleanup3_2_0); + bool const fixEnabled = view().rules().enabled(fixCleanup3_4_0); auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID])); auto applyViewContext = ctx_.getApplyViewContext(); if (!vault) @@ -309,6 +310,26 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } assetsDeposited = *maybeAssets; + + if (fixEnabled) + { + assetsDeposited = + clampToAssetsTotalScale(vault, assetsDeposited, Number::RoundingMode::Downward); + + if (assetsDeposited <= beast::kZero) + { + JLOG(j_.warn()) << "VaultDeposit: deposit rounds to zero at " + "assets outstanding scale."; + return tecPRECISION_LOSS; + } + + auto const maybeReShares = assetsToSharesDeposit(vault, sleIssuance, assetsDeposited); + if (!maybeReShares) + return tecINTERNAL; + sharesCreated = *maybeReShares; + if (sharesCreated == beast::kZero) + return tecPRECISION_LOSS; + } } catch (std::overflow_error const&) { diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 7b5bb1ea94..c8e7ddacee 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -203,6 +204,7 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) TER VaultWithdraw::doApply() { + bool const fixEnabled = view().rules().enabled(fixCleanup3_4_0); auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID])); auto applyViewContext = ctx_.getApplyViewContext(); if (!vault) @@ -304,6 +306,14 @@ VaultWithdraw::doApply() lossUnrealized <= (assetsTotal - assetsAvailable), "xrpl::VaultWithdraw::doApply : loss and assets do balance"); + if (fixEnabled) + { + assetsWithdrawn = + clampToAssetsTotalScale(vault, -assetsWithdrawn, Number::RoundingMode::Upward); + if (assetsWithdrawn <= beast::kZero) + return tecPRECISION_LOSS; + } + // The vault must have enough assets on hand. if (*assetsAvailable < assetsWithdrawn) { diff --git a/src/test/app/lending/VaultPrecisionFixture.h b/src/test/app/lending/VaultPrecisionFixture.h new file mode 100644 index 0000000000..0a26e8ccbf --- /dev/null +++ b/src/test/app/lending/VaultPrecisionFixture.h @@ -0,0 +1,248 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace xrpl::test { + +// Shared fixture for VaultInvariantPrecision_test (PR 1) and +// VaultTransactorPrecision_test (PR 2). Both PRs land this file identically; +// whichever merges second will need to reconcile this copy at rebase time. +// +// Layout: +// - A-1 (impairAndPaySibling=false): 1000 USD vault + one ordinary loan. +// assetsTotal ~= 1000.353..., assetsAvailable == 993, lossUnrealized == 0. +// - A-3 (impairAndPaySibling=true): add a second loan of principal 11, +// impair the first loan, and pay off the second in full. This drives +// the vault to the lossUnrealized == (assetsTotal - assetsAvailable) +// boundary where the loss invariant used to spuriously fire. +class VaultPrecisionFixture : public LoanTestBase +{ +protected: + static constexpr std::uint32_t kFixturePaymentInterval = 86400u * 30u; + static constexpr std::uint32_t kFixtureGracePeriod = 86400u * 30u; + static constexpr std::uint32_t kFixturePaymentTotal = 120u; + // 10% APR, expressed in tenth-bips (1000 = 10.00 %). + static constexpr std::uint32_t kFixtureInterestTenthBips = 1000u; + + struct Fixture + { + // Every account is initialised with a placeholder name because + // jtx::Account has no default constructor; setupSingleLoanVault + // overwrites them. + jtx::Account issuer{"vp_issuer_placeholder"}; + jtx::Account lender{"vp_lender_placeholder"}; + jtx::Account borrower{"vp_borrower_placeholder"}; + // Distinct account used to deposit into the vault. Keeps share + // ownership independent of the initial vault seeding. + jtx::Account depositor{"vp_depositor_placeholder"}; + // Optional so callers can BEAST_EXPECT(f.asset && f.broker) + // after setup; both are populated in the happy path. + std::optional asset; + std::optional broker; + // Keylet has no default constructor. Fill with an obviously + // meaningless placeholder; setupSingleLoanVault overwrites the + // fields that matter. + Keylet vaultKeylet{ltACCOUNT_ROOT, uint256{}}; + Keylet loan1Keylet{ltACCOUNT_ROOT, uint256{}}; + // Only meaningful when impairAndPaySibling == true. + Keylet loan2Keylet{ltACCOUNT_ROOT, uint256{}}; + jtx::Account vaultAccount{"vp_vault_pseudo_placeholder"}; + MPTID share; + }; + + // Read-only snapshot of the vault + share issuance at a point in time. + // Uses Number for exact arithmetic (no re-quantization). + struct Numbers + { + Asset asset; + MPTIssue share; + Number assetsTotal{}; // sfAssetsTotal + Number assetsAvailable{}; // sfAssetsAvailable + Number lossUnrealized{}; // sfLossUnrealized + Number pseudo{}; // vault pseudo-account balance in the asset + Number sharesTotal{}; // sfOutstandingAmount on the share MPT + }; + + static Numbers + read(jtx::Env const& env, Fixture const& f) + { + Numbers n{.asset = f.asset ? f.asset->raw() : Asset{}, .share = MPTIssue{f.share}}; + if (auto const vaultSle = env.le(f.vaultKeylet)) + { + n.assetsTotal = vaultSle->at(sfAssetsTotal); + n.assetsAvailable = vaultSle->at(sfAssetsAvailable); + n.lossUnrealized = vaultSle->at(sfLossUnrealized); + } + if (auto const issuanceSle = env.le(keylet::mptokenIssuance(f.share))) + { + n.sharesTotal = issuanceSle->at(sfOutstandingAmount); + } + if (f.asset) + n.pseudo = env.balance(f.vaultAccount, *f.asset).number(); + return n; + } + + // One unit at the STAmount scale of `assetsTotalAfter`. Used as the + // tolerance in one-unit-band assertions. + static Number + oneUnit(Asset const& asset, Number const& assetsTotalAfter) + { + return Number{1, scale(assetsTotalAfter, asset)}; + } + + // Build the shared vault + loan(s) layout. The caller constructs + // `env` with whatever FeatureBitset they want to exercise; this helper + // just uses it. If `allowClawback` is true, the issuer's + // asfAllowTrustLineClawback flag is set BEFORE any trust line is + // established for that issuer. A separate env.close() runs so the + // flag lands in the ledger before the trust lines are set up. + static Fixture + setupSingleLoanVault(jtx::Env& env, bool impairAndPaySibling, bool allowClawback = false) + { + using namespace jtx; + using namespace jtx::loan; + using namespace jtx::loan_broker; + + Fixture f; + f.issuer = Account{"vp_issuer"}; + f.lender = Account{"vp_lender"}; + f.borrower = Account{"vp_borrower"}; + f.depositor = Account{"vp_depositor"}; + + env.fund(XRP(1'000'000), f.issuer, f.lender, f.borrower, f.depositor); + env.close(); + + // Must be set BEFORE any trust line to `issuer` is created. + if (allowClawback) + { + env(fset(f.issuer, asfAllowTrustLineClawback)); + env.close(); + } + + PrettyAsset const asset = f.issuer["USD"]; + f.asset = asset; + + env.trust(asset(1'000'000'000), f.lender); + env.trust(asset(1'000'000'000), f.borrower); + env.trust(asset(1'000'000'000), f.depositor); + env(pay(f.issuer, f.lender, asset(100'000'000))); + env(pay(f.issuer, f.borrower, asset(100'000'000))); + env(pay(f.issuer, f.depositor, asset(100'000'000))); + env.close(); + + BrokerParameters const brokerParams{ + .vaultDeposit = 1'000, + .debtMax = 0, + .coverRateMin = percentageToTenthBips(1), + .coverDeposit = 10'000, + .managementFeeRate = TenthBips16{100}, + .coverRateLiquidation = xrpl::lending::kMaxCoverRate}; + + // Build the vault + broker manually (rather than calling + // createVaultAndBroker) so we can seed only the lender/depositor + // trust lines we set up above, and skip the LoanTestBase auto + // funding that assumes an XRP asset. + Vault const vault{env}; + auto [createTx, vaultKeylet] = vault.create({.owner = f.lender, .asset = asset}); + env(createTx); + env.close(); + f.vaultKeylet = vaultKeylet; + + env(vault.deposit( + {.depositor = f.lender, + .id = vaultKeylet.key, + .amount = asset(brokerParams.vaultDeposit)})); + env.close(); + + auto const brokerKeylet = + keylet::loanBroker(f.lender.id(), SeqProxy::rawSequence(env.seq(f.lender))); + + env(set(f.lender, vaultKeylet.key, brokerParams.flags), + kManagementFeeRate(brokerParams.managementFeeRate), + kDebtMaximum(asset(brokerParams.debtMax).value()), + kCoverRateMinimum(brokerParams.coverRateMin), + kCoverRateLiquidation(TenthBips32(brokerParams.coverRateLiquidation))); + env(coverDeposit(f.lender, brokerKeylet.key, asset(brokerParams.coverDeposit).value())); + env.close(); + + f.broker = BrokerInfo{asset, brokerKeylet, vaultKeylet, brokerParams}; + + auto const vaultSle = env.le(vaultKeylet); + f.vaultAccount = Account{"vp_vault_pseudo", vaultSle->at(sfAccount)}; + f.share = vaultSle->at(sfShareMPTID); + + Fee const bigFee{env.current()->fees().base * 200}; + + auto const setLoan = [&](Number const& principal) -> Keylet { + auto const brokerSle = env.le(brokerKeylet); + auto const loanKeylet = keylet::loan( + brokerKeylet.key, SeqProxy::rawSequence(brokerSle->at(sfLoanSequence))); + env(loan::set(f.borrower, brokerKeylet.key, asset(principal).number()), + Sig(sfCounterpartySignature, f.lender), + jtx::loan::kInterestRate(TenthBips32{kFixtureInterestTenthBips}), + jtx::loan::kPaymentTotal(kFixturePaymentTotal), + jtx::loan::kPaymentInterval(kFixturePaymentInterval), + jtx::loan::kGracePeriod(kFixtureGracePeriod), + bigFee); + env.close(); + return loanKeylet; + }; + + // Loan 1: principal 7, the one ordinary loan in both fixtures. + // With vault deposit 1000, this leaves A ≈ 993 (see plan). + f.loan1Keylet = setLoan(Number{7}); + + if (!impairAndPaySibling) + return f; + + // Loan 2: sibling loan of principal 11. + f.loan2Keylet = setLoan(Number{11}); + + // Impair loan 1 → drives sfLossUnrealized to loan 1's value. + env(jtx::loan::manage(f.lender, f.loan1Keylet.key, tfLoanImpair), bigFee); + env.close(); + + // Pay off loan 2 in full so its total value flows into the vault + // and pushes T-A upward, meeting the residual loss. Generous + // upper bound; the transactor takes only what is due. + auto const payoff = asset(Number{50}).value(); + env(pay(f.borrower, f.loan2Keylet.key, payoff, tfLoanFullPayment), bigFee); + env.close(); + + return f; + } +}; + +} // namespace xrpl::test diff --git a/src/test/app/lending/VaultTransactorPrecision_test.cpp b/src/test/app/lending/VaultTransactorPrecision_test.cpp new file mode 100644 index 0000000000..36ec11e912 --- /dev/null +++ b/src/test/app/lending/VaultTransactorPrecision_test.cpp @@ -0,0 +1,779 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace xrpl::test { + +// PR 2 tests: with fixCleanup3_4_0 enabled the transactor clamps make +// deposit / withdraw / clawback exact on the sfAssetsTotal grid. Every +// successful withdrawal and clawback satisfies STRICT equality of the +// assetsTotal / assetsAvailable / pseudo-account deltas -- no tolerance. +// Deposit is exact on the T side; A stays within one unit at the +// sfAssetsTotal grid. +class VaultTransactorPrecision_test : public VaultPrecisionFixture +{ + static Number + absDiff(Number const& a, Number const& b) + { + return a > b ? a - b : b - a; + } + + // ---- deposit ------------------------------------------------------ + + // A-1 magnitude sweep. Post-fix every successful deposit satisfies + // T_delta <= requested amount + // |T_delta - A_delta| <= oneUnit(asset, T_after) + // and the plan's three boundary amounts {1, 7, 10'000'000} now succeed. + // Pre-fix those three boundary amounts fail with tecINVARIANT_FAILED. + void + testDepositNeverOverCredited(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + testcase( + std::string("A-1 deposit never over-credited") + + (fixEnabled ? " (fixCleanup3_4_0)" : " (pre-fix)")); + + std::array const kAmounts{ + 1, + 2, + 5, + 7, + 10, + 50, + 100, + 500, + 1'000, + 5'000, + 10'000, + 50'000, + 100'000, + 500'000, + 1'000'000, + 5'000'000, + 10'000'000}; + + // Plan's three boundary amounts that Part 1 alone closes on A-1. + std::array const kPlanBoundaryAmounts{1, 7, 10'000'000}; + + for (auto const amount : kAmounts) + { + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + continue; + + auto const before = read(env, f); + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(amount).value()}), + Ter(std::ignore)); + env.close(); + + TER const actual = env.ter(); + bool const isBoundary = + std::ranges::find(kPlanBoundaryAmounts, amount) != kPlanBoundaryAmounts.end(); + + if (fixEnabled) + { + if (isBoundary) + { + BEAST_EXPECTS( + actual == tesSUCCESS, + "plan boundary amount=" + std::to_string(amount) + + " expected tesSUCCESS, got " + transToken(actual)); + } + if (actual != tesSUCCESS) + continue; + + auto const after = read(env, f); + Number const tDelta = after.assetsTotal - before.assetsTotal; + Number const aDelta = after.assetsAvailable - before.assetsAvailable; + Number const requested = (*f.asset)(amount).number(); + + BEAST_EXPECTS( + tDelta <= requested, + "amount=" + std::to_string(amount) + " tDelta exceeds requested"); + Number const gap = absDiff(tDelta, aDelta); + BEAST_EXPECTS( + gap <= oneUnit(*f.asset, after.assetsTotal), + "amount=" + std::to_string(amount) + " |tDelta-aDelta| exceeds oneUnit"); + } + else if (isBoundary) + { + BEAST_EXPECTS( + actual == tecINVARIANT_FAILED, + "pre-fix amount=" + std::to_string(amount) + + " expected tecINVARIANT_FAILED, got " + transToken(actual)); + } + } + } + + // Post-fix: the depositor never gets shares worth more than the assets + // they paid. Any overpay is bounded by the pre-deposit per-share value. + void + testDepositorNeverUnderpays(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("A-1 depositor never underpays (fixCleanup3_4_0)"); + + std::array const kAmounts{1, 7, 100, 1'000, 10'000, 100'000, 1'000'000, 10'000'000}; + + for (auto const amount : kAmounts) + { + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + continue; + + auto const before = read(env, f); + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(amount).value()}), + Ter(std::ignore)); + env.close(); + + if (env.ter() != tesSUCCESS) + continue; + + auto const after = read(env, f); + Number const sharesMinted = after.sharesTotal - before.sharesTotal; + Number const assetsTaken = after.assetsTotal - before.assetsTotal; + if (before.sharesTotal == Number{0}) + continue; + Number const shareValue = (before.assetsTotal * sharesMinted) / before.sharesTotal; + + BEAST_EXPECTS( + shareValue <= assetsTaken, + "amount=" + std::to_string(amount) + " shareValue > assetsTaken"); + + Number const perShareOverpay = before.assetsTotal / before.sharesTotal; + Number const overpay = assetsTaken > shareValue ? assetsTaken - shareValue : Number{0}; + BEAST_EXPECTS( + overpay <= perShareOverpay, + "amount=" + std::to_string(amount) + " overpay exceeds per-share bound"); + } + } + + // Push T up to ~1e8, then attempt Number{1,-10} deposit. Post-fix: + // tecPRECISION_LOSS with the vault state unchanged. Confirms the + // deposit clamp cannot mint uncovered shares. + void + testZeroCreditRejected(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("A-1 zero credit rejected (fixCleanup3_4_0)"); + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(99'000'000).value()}), + Ter(std::ignore)); + env.close(); + + auto const before = read(env, f); + Number const kLowerBound{1, 6}; + BEAST_EXPECT(before.assetsTotal > kLowerBound); + + auto const tinyAmount = (*f.asset)(Number{1, -10}).value(); + env(v.deposit({.depositor = f.depositor, .id = f.vaultKeylet.key, .amount = tinyAmount}), + Ter(std::ignore)); + env.close(); + + BEAST_EXPECTS( + env.ter() == tecPRECISION_LOSS, + std::string{"expected tecPRECISION_LOSS, got "} + transToken(env.ter())); + + auto const after = read(env, f); + BEAST_EXPECT(after.assetsTotal == before.assetsTotal); + BEAST_EXPECT(after.assetsAvailable == before.assetsAvailable); + BEAST_EXPECT(after.sharesTotal == before.sharesTotal); + } + + // XRP and MPT (integral) vaults: the clamp is a no-op because + // STAmount(asset, N) already truncates to whole units. Every amount + // in the sweep succeeds pre- and post-amendment. + void + testIntegralAssetsUnchanged(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + testcase( + std::string("integral asset deposit sweep") + + (fixEnabled ? " (fixCleanup3_4_0)" : " (pre-fix)")); + + std::array const kAmounts{1, 7, 100, 1'000, 10'000, 100'000, 1'000'000, 10'000'000}; + + auto runXrp = [&]() { + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + Account const owner{"xrp_owner"}; + Account const depositor{"xrp_depositor"}; + env.fund(XRP(1'000'000'000), owner, depositor); + env.close(); + + Vault const v{env}; + auto [createTx, vaultKeylet] = v.create({.owner = owner, .asset = xrpIssue()}); + env(createTx); + env.close(); + + env(v.deposit( + {.depositor = owner, .id = vaultKeylet.key, .amount = XRP(1'000).value()})); + env.close(); + + for (auto const amount : kAmounts) + { + env(v.deposit( + {.depositor = depositor, + .id = vaultKeylet.key, + .amount = XRP(amount).value()}), + Ter(std::ignore)); + env.close(); + BEAST_EXPECTS( + env.ter() == tesSUCCESS, + "XRP amount=" + std::to_string(amount) + " expected tesSUCCESS, got " + + transToken(env.ter())); + } + }; + + auto runMpt = [&]() { + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + Account const issuer{"mpt_issuer"}; + Account const owner{"mpt_owner"}; + Account const depositor{"mpt_depositor"}; + env.fund(XRP(1'000'000), issuer, owner, depositor); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = depositor}); + env(pay(issuer, depositor, asset(1'000'000'000))); + env.close(); + + Vault const v{env}; + auto [createTx, vaultKeylet] = v.create({.owner = owner, .asset = asset}); + env(createTx); + env.close(); + + env(pay(issuer, owner, asset(1'000))); + env.close(); + env(v.deposit( + {.depositor = owner, .id = vaultKeylet.key, .amount = asset(1'000).value()})); + env.close(); + + for (auto const amount : kAmounts) + { + env(v.deposit( + {.depositor = depositor, + .id = vaultKeylet.key, + .amount = asset(amount).value()}), + Ter(std::ignore)); + env.close(); + BEAST_EXPECTS( + env.ter() == tesSUCCESS, + "MPT amount=" + std::to_string(amount) + " expected tesSUCCESS, got " + + transToken(env.ter())); + } + }; + + runXrp(); + runMpt(); + } + + // ---- withdraw ----------------------------------------------------- + + // A-1 fixture withdrawals in both asset-fixed and share-fixed modes. + // Post-fix: never tecINVARIANT_FAILED and every successful withdrawal + // satisfies STRICT equality + // beforeT - afterT == beforeA - afterA == beforePseudo - afterPseudo + // in Number space. This is the strong claim of Part 1b of the plan. + void + testWithdrawDeltas(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + testcase( + std::string("A-1 withdraw delta exactness") + + (fixEnabled ? " (fixCleanup3_4_0)" : " (pre-fix)")); + + std::array const kShareCounts{ + 99'999u, 100'001u, 333'333u, 1'234'567u, 142'857'142u, 333'333'333u}; + + std::array const kAssetAmounts{1, 7, 99, 333, 993}; + + auto runOnce = [&](bool useShares) { + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(1'000'000).value()}), + Ter(std::ignore)); + env.close(); + + auto step = [&](STAmount const& amount, std::string const& tag) { + auto const before = read(env, f); + env(v.withdraw( + {.depositor = f.depositor, .id = f.vaultKeylet.key, .amount = amount}), + Ter(std::ignore)); + env.close(); + + TER const actual = env.ter(); + if (fixEnabled) + { + BEAST_EXPECTS( + actual != tecINVARIANT_FAILED, tag + " unexpected invariant failure"); + if (actual == tesSUCCESS) + { + auto const after = read(env, f); + Number const tDelta = before.assetsTotal - after.assetsTotal; + Number const aDelta = before.assetsAvailable - after.assetsAvailable; + Number const pDelta = before.pseudo - after.pseudo; + BEAST_EXPECTS(tDelta == aDelta, tag + " tDelta != aDelta"); + BEAST_EXPECTS(tDelta == pDelta, tag + " tDelta != pDelta"); + } + } + }; + + if (useShares) + { + for (auto const count : kShareCounts) + { + auto const before = read(env, f); + if (before.sharesTotal < count) + continue; + STAmount const shareAmount{ + MPTIssue{f.share}, Number{static_cast(count)}}; + step(shareAmount, "shares=" + std::to_string(count)); + } + } + else + { + for (auto const amount : kAssetAmounts) + { + step((*f.asset)(amount).value(), "assets=" + std::to_string(amount)); + } + } + }; + + runOnce(/*useShares=*/true); + runOnce(/*useShares=*/false); + } + + // Post-fix: withdrawer never receives more than the burned share value; + // any shortfall is bounded by one unit at the sfAssetsTotal scale. + void + testWithdrawNeverOverpays(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("A-1 withdraw never overpays (fixCleanup3_4_0)"); + + std::array const kShareCounts{ + 99'999u, 100'001u, 333'333u, 1'234'567u, 142'857'142u}; + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(1'000'000).value()}), + Ter(std::ignore)); + env.close(); + + for (auto const count : kShareCounts) + { + auto const before = read(env, f); + if (before.sharesTotal < count) + continue; + STAmount const shareAmount{MPTIssue{f.share}, Number{static_cast(count)}}; + env(v.withdraw( + {.depositor = f.depositor, .id = f.vaultKeylet.key, .amount = shareAmount}), + Ter(std::ignore)); + env.close(); + if (env.ter() != tesSUCCESS) + continue; + + auto const after = read(env, f); + Number const sharesBurned = before.sharesTotal - after.sharesTotal; + if (before.sharesTotal == Number{0}) + continue; + Number const shareValue = (before.assetsTotal * sharesBurned) / before.sharesTotal; + Number const payout = before.assetsTotal - after.assetsTotal; + + BEAST_EXPECTS( + payout <= shareValue, "shares=" + std::to_string(count) + " payout > shareValue"); + Number const shortfall = shareValue > payout ? shareValue - payout : Number{0}; + BEAST_EXPECTS( + shortfall <= oneUnit(*f.asset, after.assetsTotal), + "shares=" + std::to_string(count) + " shortfall exceeds oneUnit"); + } + } + + // Sub-ULP withdrawal from a ~1e8 vault; post-fix must return + // tecPRECISION_LOSS after the Upward clamp rounds the amount to zero. + void + testWithdrawSubUlpRejected(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("sub-ULP withdraw rejected (fixCleanup3_4_0)"); + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(99'000'000).value()}), + Ter(std::ignore)); + env.close(); + + auto const before = read(env, f); + Number const kLowerBound{1, 6}; + BEAST_EXPECT(before.assetsTotal > kLowerBound); + + auto const tinyAmount = (*f.asset)(Number{1, -10}).value(); + env(v.withdraw({.depositor = f.depositor, .id = f.vaultKeylet.key, .amount = tinyAmount}), + Ter(std::ignore)); + env.close(); + + BEAST_EXPECTS( + env.ter() == tecPRECISION_LOSS, + std::string{"expected tecPRECISION_LOSS, got "} + transToken(env.ter())); + } + + // Depositor burns every share they hold, exercising the final- + // withdrawal branch of VaultWithdraw (line 336-368 -- deliberately + // untouched by this amendment). Must return tesSUCCESS: the clamp + // does not spuriously reject a legitimate full-share withdrawal. + // + // The plan describes reaching a "dust-only gap state" via the A-3 + // fixture; empirically the A-3 fixture retains a non-zero + // sfLossUnrealized which blocks the final-withdrawal branch through + // the insufficient-funds guard. Verifying the branch from a clean + // A-1 state still exercises the amendment's non-interference claim. + void + testFinalWithdrawalDust(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("A-1 final withdrawal (fixCleanup3_4_0)"); + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/false); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(500).value()}), + Ter(std::ignore)); + env.close(); + + auto const depositorMptSle = env.le(keylet::mptoken(f.share, f.depositor.id())); + if (!BEAST_EXPECT(depositorMptSle != nullptr)) + return; + + auto const held = depositorMptSle->at(sfMPTAmount); + if (held == 0) + return; + + STAmount const shareAmount{MPTIssue{f.share}, Number{static_cast(held)}}; + env(v.withdraw({.depositor = f.depositor, .id = f.vaultKeylet.key, .amount = shareAmount}), + Ter(std::ignore)); + env.close(); + + TER const actual = env.ter(); + BEAST_EXPECTS( + actual == tesSUCCESS, std::string{"expected tesSUCCESS, got "} + transToken(actual)); + } + + // ---- deposit + withdraw under impairment (A-3) -------------------- + + // A-3 fixture drives sfLossUnrealized to the boundary + // lossUnrealized == assetsTotal - assetsAvailable + // A companion invariant-fix branch found that independent per-field + // rounding of assetsTotal/assetsAvailable can spuriously trip the + // XRPL_ASSERT (and, pre-fix, tecINVARIANT_FAILED) that compares + // lossUnrealized against assetsTotal - assetsAvailable in + // VaultWithdraw::doApply. This branch's clamp keeps assetsTotal and + // assetsAvailable exact on the sfAssetsTotal grid, which should keep + // that comparison stable even while churning the vault through this + // boundary state. Only meaningful post-fix -- the pre-fix boundary + // behaviour at A-3 is out of scope for this branch (it is the subject + // of the companion invariant-fix branch, not this one) -- so mirror + // testWithdrawNeverOverpays and early-return before the amendment. + void + testDepositWithdrawUnderImpairment(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + if (!fixEnabled) + return; + + testcase("A-3 deposit/withdraw under impairment boundary (fixCleanup3_4_0)"); + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault(env, /*impairAndPaySibling=*/true); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + + // Seed the depositor with an initial stake so later withdrawals + // have shares/assets to draw against. + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(5'000).value()}), + Ter(std::ignore)); + env.close(); + + std::array const kAmounts{1, 3, 7, 13, 29, 51, 97, 137, 251, 499, 991, 1'999}; + + auto checkInvariant = [&](std::string const& tag) { + TER const actual = env.ter(); + BEAST_EXPECTS(actual != tecINVARIANT_FAILED, tag + " unexpected invariant failure"); + if (actual == tesSUCCESS) + { + auto const after = read(env, f); + BEAST_EXPECTS( + after.lossUnrealized <= after.assetsTotal - after.assetsAvailable, + tag + " lossUnrealized exceeds assetsTotal - assetsAvailable"); + } + }; + + // Alternate deposit then withdraw of a different amount so the + // vault's assetsTotal / assetsAvailable / lossUnrealized state + // churns through several transactions at the A-3 boundary. + for (std::size_t i = 0; i + 1 < kAmounts.size(); i += 2) + { + int const depositAmount = kAmounts[i]; + int const withdrawAmount = kAmounts[i + 1]; + + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(depositAmount).value()}), + Ter(std::ignore)); + env.close(); + checkInvariant("deposit=" + std::to_string(depositAmount)); + + env(v.withdraw( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(withdrawAmount).value()}), + Ter(std::ignore)); + env.close(); + checkInvariant("withdraw=" + std::to_string(withdrawAmount)); + } + } + + // ---- clawback ----------------------------------------------------- + + // A-1 fixture with clawback enabled. Sweep amounts including + // sfAmount-absent (claw back everything) and amount-exceeds-available. + // Post-fix: never tecINVARIANT_FAILED and each success satisfies + // T_delta == A_delta == pseudo_delta in Number space. + // Owner force-burn against a vault with a live loan: must return + // tecNO_PERMISSION regardless of amendment (never enters + // assetsToClawback so the clamp is unreachable there). + void + testClawbackDeltas(FeatureBitset features) + { + using namespace jtx; + + bool const fixEnabled = features[fixCleanup3_4_0]; + testcase( + std::string("A-1 clawback delta exactness") + + (fixEnabled ? " (fixCleanup3_4_0)" : " (pre-fix)")); + + std::array const kAmounts{1, 7, 99, 333, 993, 5'000'000}; + + Env env{*this, envconfig(), features, nullptr, beast::Severity::Disabled}; + auto f = setupSingleLoanVault( + env, + /*impairAndPaySibling=*/false, + /*allowClawback=*/true); + if (!BEAST_EXPECT(f.asset && f.broker)) + return; + + Vault const v{env}; + env(v.deposit( + {.depositor = f.depositor, + .id = f.vaultKeylet.key, + .amount = (*f.asset)(2'000).value()}), + Ter(std::ignore)); + env.close(); + + auto stepAmount = [&](int amount) { + auto const before = read(env, f); + if (before.sharesTotal == Number{0}) + return; + + env(v.clawback( + {.issuer = f.issuer, + .id = f.vaultKeylet.key, + .holder = f.depositor, + .amount = (*f.asset)(amount).value()}), + Ter(std::ignore)); + env.close(); + + TER const actual = env.ter(); + if (fixEnabled) + { + BEAST_EXPECTS( + actual != tecINVARIANT_FAILED, + "amount=" + std::to_string(amount) + " unexpected invariant failure"); + if (actual == tesSUCCESS) + { + auto const after = read(env, f); + Number const tDelta = before.assetsTotal - after.assetsTotal; + Number const aDelta = before.assetsAvailable - after.assetsAvailable; + Number const pDelta = before.pseudo - after.pseudo; + BEAST_EXPECTS( + tDelta == aDelta, "amount=" + std::to_string(amount) + " tDelta != aDelta"); + BEAST_EXPECTS( + tDelta == pDelta, "amount=" + std::to_string(amount) + " tDelta != pDelta"); + } + } + }; + + for (auto const amount : kAmounts) + stepAmount(amount); + + { + auto const before = read(env, f); + if (before.sharesTotal > Number{0}) + { + env(v.clawback( + {.issuer = f.issuer, .id = f.vaultKeylet.key, .holder = f.depositor}), + Ter(std::ignore)); + env.close(); + + TER const actual = env.ter(); + if (fixEnabled) + { + BEAST_EXPECTS( + actual != tecINVARIANT_FAILED, + "sfAmount-absent unexpected invariant failure"); + if (actual == tesSUCCESS) + { + auto const after = read(env, f); + Number const tDelta = before.assetsTotal - after.assetsTotal; + Number const aDelta = before.assetsAvailable - after.assetsAvailable; + Number const pDelta = before.pseudo - after.pseudo; + BEAST_EXPECT(tDelta == aDelta); + BEAST_EXPECT(tDelta == pDelta); + } + } + } + } + + env(v.clawback({.issuer = f.lender, .id = f.vaultKeylet.key, .holder = f.depositor}), + Ter(tecNO_PERMISSION)); + env.close(); + } + +public: + void + run() override + { + for (auto const& features : {all_ - fixCleanup3_4_0, all_}) + { + testDepositNeverOverCredited(features); + testDepositorNeverUnderpays(features); + testZeroCreditRejected(features); + testIntegralAssetsUnchanged(features); + testWithdrawDeltas(features); + testWithdrawNeverOverpays(features); + testWithdrawSubUlpRejected(features); + testFinalWithdrawalDust(features); + testDepositWithdrawUnderImpairment(features); + testClawbackDeltas(features); + } + } +}; + +BEAST_DEFINE_TESTSUITE(VaultTransactorPrecision, tx, xrpl); + +} // namespace xrpl::test diff --git a/src/test/app/vault/VaultBugs_test.cpp b/src/test/app/vault/VaultBugs_test.cpp index a7071f3767..03840667ac 100644 --- a/src/test/app/vault/VaultBugs_test.cpp +++ b/src/test/app/vault/VaultBugs_test.cpp @@ -411,7 +411,11 @@ private: testcase( "bug: VaultDeposit below Vault precision canonicalized to zero " "(pre-fixCleanup3_2_0)"); - runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED); + // Also remove fixCleanup3_4_0 so the VaultDeposit clamp + // introduced by that amendment does not short-circuit this + // pre-fixCleanup3_2_0 scenario with tecPRECISION_LOSS. + runScenario( + testableAmendments() - fixCleanup3_2_0 - fixCleanup3_4_0, tecINVARIANT_FAILED); } { testcase(