diff --git a/include/xrpl/ledger/helpers/LendingHelpers.h b/include/xrpl/ledger/helpers/LendingHelpers.h index 6ca7163c0d..5f69a93f67 100644 --- a/include/xrpl/ledger/helpers/LendingHelpers.h +++ b/include/xrpl/ledger/helpers/LendingHelpers.h @@ -64,7 +64,7 @@ namespace detail { * unrounded delta. */ [[nodiscard]] int -getPosteriorBrokerCoverScale(SLE::ConstRef vault, SLE::ConstRef broker, STAmount const& delta); +getPosteriorBrokerCoverScale(SLE::ConstRef vault, SLE::ConstRef broker, Number const& delta); /** * Round a cover outflow delta (cover withdraw, cover clawback, the @@ -113,7 +113,7 @@ debitToPosteriorBrokerCoverScale( creditToPosteriorBrokerCoverScale( SLE::ConstRef vault, SLE::ConstRef broker, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode); /** diff --git a/include/xrpl/ledger/helpers/VaultHelpers.h b/include/xrpl/ledger/helpers/VaultHelpers.h index c9f5261a76..da95ae05c8 100644 --- a/include/xrpl/ledger/helpers/VaultHelpers.h +++ b/include/xrpl/ledger/helpers/VaultHelpers.h @@ -100,16 +100,6 @@ getVaultScale(SLE::ConstRef vault); [[nodiscard]] int getVaultBaseScale(SLE::ConstRef vault); -namespace detail { - -/** - * Return the Vault's posterior live exponent after applying an unrounded delta. - */ -[[nodiscard]] int -getPosteriorVaultScale(SLE::ConstRef vault, STAmount const& delta); - -} // namespace detail - /** * Round an amount at the Vault's posterior live exponent. */ @@ -121,6 +111,12 @@ roundToPosteriorVaultScale( namespace detail { +/** + * Return the Vault's posterior live exponent after applying an unrounded delta. + */ +[[nodiscard]] int +getPosteriorVaultScale(SLE::ConstRef vault, STAmount const& delta); + /** * Shared grid-floor core of getVaultScale / getPosteriorVaultScale / * getPosteriorBrokerCoverScale: baseScale when reference is zero, otherwise @@ -149,7 +145,7 @@ posteriorAssetScale( Asset const& asset, int baseScale, Number const& reference, - STAmount const& delta); + Number const& delta); /** * Shared core of creditToPosteriorAvailableScale and @@ -157,14 +153,15 @@ posteriorAssetScale( * returns the difference from reference, so the delta applied is exactly * what moves reference onto the floored sum. See * creditToPosteriorAvailableScale's doc for why the sum, not raw alone, - * must be floored. + * must be floored. raw is the exact amount; it is not rounded on its own + * before being added to reference. */ [[nodiscard]] STAmount creditToPosteriorScale( Asset const& asset, Number const& reference, int atScale, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode); } // namespace detail @@ -185,11 +182,14 @@ creditToPosteriorScale( * the sum instead, floor16(9999999999.999999 + 0.000011) minus * 9999999999.999999, is exact by construction, since STAmount's own * 16-digit canonical form of the sum is what gets subtracted from. + * + * raw is taken as an exact Number: rounding it to 16 digits before adding + * it to AssetsAvailable could drop a tail that moves the floored sum. */ [[nodiscard]] STAmount creditToPosteriorAvailableScale( SLE::ConstRef vault, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode); /** diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 42303e0db9..4232a4bfc3 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -345,6 +345,12 @@ enum class VaultVersion : uint8_t { FixedPrecision, }; +// Code compares VaultVersion values with < and >= (e.g. "CashBasis or later"), +// so each later version must stay numerically larger than the one before. +static_assert( + VaultVersion::Legacy < VaultVersion::CashBasis && + VaultVersion::CashBasis < VaultVersion::FixedPrecision); + /** * Vault kind. Distinguishes closed-ended vaults from the default open-ended * kind. Persisted as sfVaultKind (UINT8); absent means OpenEnded. diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index d576674ea2..b5086b6114 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -69,7 +69,7 @@ canApplyToBrokerCover( namespace detail { [[nodiscard]] int -getPosteriorBrokerCoverScale(SLE::ConstRef vault, SLE::ConstRef broker, STAmount const& delta) +getPosteriorBrokerCoverScale(SLE::ConstRef vault, SLE::ConstRef broker, Number const& delta) { XRPL_ASSERT( vault && vault->getType() == ltVAULT, @@ -77,9 +77,6 @@ getPosteriorBrokerCoverScale(SLE::ConstRef vault, SLE::ConstRef broker, STAmount XRPL_ASSERT( broker && broker->getType() == ltLOAN_BROKER, "xrpl::detail::getPosteriorBrokerCoverScale : valid LoanBroker sle"); - XRPL_ASSERT( - delta.asset() == vault->at(sfAsset), - "xrpl::detail::getPosteriorBrokerCoverScale : delta and Vault asset match"); return posteriorAssetScale( getVaultVersion(vault), @@ -116,7 +113,7 @@ roundToPosteriorBrokerCoverScale( creditToPosteriorBrokerCoverScale( SLE::ConstRef vault, SLE::ConstRef broker, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode) { XRPL_ASSERT( @@ -125,13 +122,13 @@ creditToPosteriorBrokerCoverScale( XRPL_ASSERT( broker && broker->getType() == ltLOAN_BROKER, "xrpl::creditToPosteriorBrokerCoverScale : valid LoanBroker sle"); - XRPL_ASSERT( - raw.asset() == vault->at(sfAsset), - "xrpl::creditToPosteriorBrokerCoverScale : raw and Vault asset match"); - if (raw.integral()) - return raw; - Asset const asset = vault->at(sfAsset); + if (asset.integral()) + { + NumberRoundModeGuard const rg(roundingMode); + return STAmount{asset, raw}; + } + Number const reference = broker->at(sfCoverAvailable); int const scale = detail::getPosteriorBrokerCoverScale(vault, broker, raw); return detail::creditToPosteriorScale(asset, reference, scale, raw, roundingMode); @@ -173,8 +170,10 @@ checkOptionalBrokerCoverInflow(SLE::ConstRef vault, SLE::ConstRef broker, STAmou if (getVaultVersion(vault) != VaultVersion::FixedPrecision) return tesSUCCESS; - STAmount const rounded = detail::roundToPosteriorBrokerCoverScale( - vault, broker, amount, Number::RoundingMode::TowardsZero); + // amount is the effective credit (already floored on the posterior + // CoverAvailable grid by creditToPosteriorBrokerCoverScale); rounding it + // again as a standalone delta could hide a crossing. + STAmount const& rounded = amount; int const baseScale = getVaultBaseScale(vault); // Keep this explicit even though the Open-limit capacity check below rejects // every coarsening transition too. The protocol defines both conditions diff --git a/src/libxrpl/ledger/helpers/VaultHelpers.cpp b/src/libxrpl/ledger/helpers/VaultHelpers.cpp index b9588fa1c7..4517a07256 100644 --- a/src/libxrpl/ledger/helpers/VaultHelpers.cpp +++ b/src/libxrpl/ledger/helpers/VaultHelpers.cpp @@ -48,7 +48,7 @@ fixedBaseScale(SLE::ConstRef vault) // Thin, vault-specific wrapper over posteriorAssetScale: used by // getPosteriorVaultScale and creditToPosteriorAvailableScale. [[nodiscard]] int -posteriorScale(SLE::ConstRef vault, Number const& reference, STAmount const& delta) +posteriorScale(SLE::ConstRef vault, Number const& reference, Number const& delta) { return detail::posteriorAssetScale( getVaultVersion(vault), vault->at(sfAsset), fixedBaseScale(vault), reference, delta); @@ -156,12 +156,12 @@ posteriorAssetScale( Asset const& asset, int baseScale, Number const& reference, - STAmount const& delta) + Number const& delta) { - Number const posterior = [&] { - NumberRoundModeGuard const rg(Number::RoundingMode::ToNearest); - return reference + delta; - }(); + // ToNearest for the sum and for scale()'s canonicalization of it, as the + // Legacy/CashBasis clamp always did; liveScale sets its own mode. + NumberRoundModeGuard const rg(Number::RoundingMode::ToNearest); + Number const posterior = reference + delta; switch (version) { @@ -182,14 +182,14 @@ creditToPosteriorScale( Asset const& asset, Number const& reference, int atScale, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode) { // Floor the SUM (reference + raw), not just raw, at atScale. See // creditToPosteriorAvailableScale's doc: a delta floored on its own grid // can still leave a 17-digit sum once the reference has crossed a power // of ten. - Number const flooredSum = roundToAsset(asset, reference + Number(raw), atScale, roundingMode); + Number const flooredSum = roundToAsset(asset, reference + raw, atScale, roundingMode); // flooredSum - reference can carry 17 significant digits (the sum is on // the posterior grid, the reference on the finer one); build the // STAmount under roundingMode, not the caller's ambient mode, so the @@ -198,6 +198,18 @@ creditToPosteriorScale( return STAmount{asset, flooredSum - reference}; } +[[nodiscard]] int +getPosteriorVaultScale(SLE::ConstRef vault, STAmount const& delta) +{ + XRPL_ASSERT( + vault && vault->getType() == ltVAULT, + "xrpl::detail::getPosteriorVaultScale : valid Vault sle"); + XRPL_ASSERT( + delta.asset() == vault->at(sfAsset), + "xrpl::detail::getPosteriorVaultScale : delta and Vault asset match"); + return posteriorScale(vault, getAssetsTotal(vault), delta); +} + } // namespace detail [[nodiscard]] Number @@ -330,22 +342,6 @@ getVaultBaseScale(SLE::ConstRef vault) // LCOV_EXCL_STOP } -namespace detail { - -[[nodiscard]] int -getPosteriorVaultScale(SLE::ConstRef vault, STAmount const& delta) -{ - XRPL_ASSERT( - vault && vault->getType() == ltVAULT, - "xrpl::detail::getPosteriorVaultScale : valid Vault sle"); - XRPL_ASSERT( - delta.asset() == vault->at(sfAsset), - "xrpl::detail::getPosteriorVaultScale : delta and Vault asset match"); - return posteriorScale(vault, getAssetsTotal(vault), delta); -} - -} // namespace detail - [[nodiscard]] STAmount roundToPosteriorVaultScale( SLE::ConstRef vault, @@ -365,19 +361,19 @@ roundToPosteriorVaultScale( [[nodiscard]] STAmount creditToPosteriorAvailableScale( SLE::ConstRef vault, - STAmount const& raw, + Number const& raw, Number::RoundingMode roundingMode) { XRPL_ASSERT( vault && vault->getType() == ltVAULT, "xrpl::creditToPosteriorAvailableScale : valid Vault sle"); - XRPL_ASSERT( - raw.asset() == vault->at(sfAsset), - "xrpl::creditToPosteriorAvailableScale : raw and Vault asset match"); - if (raw.integral()) - return raw; - Asset const asset = vault->at(sfAsset); + if (asset.integral()) + { + NumberRoundModeGuard const rg(roundingMode); + return STAmount{asset, raw}; + } + Number const reference = vault->at(sfAssetsAvailable); int const scale = posteriorScale(vault, reference, raw); return detail::creditToPosteriorScale(asset, reference, scale, raw, roundingMode); @@ -402,6 +398,8 @@ checkOptionalVaultInflow(SLE::ConstRef vault, STAmount const& amount) amount.asset() == vault->at(sfAsset), "xrpl::checkOptionalVaultInflow : amount and Vault asset match"); XRPL_ASSERT(!amount.negative(), "xrpl::checkOptionalVaultInflow : non-negative amount"); + if (amount.negative()) + return tefINTERNAL; // LCOV_EXCL_LINE if (getVaultVersion(vault) != VaultVersion::FixedPrecision) return tesSUCCESS; diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 9d334f072e..e57795801f 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -859,7 +859,7 @@ ValidVault::finalize( if (afterVault.assetsAvailable != kZero || afterVault.assetsTotal != kZero || afterVault.lossUnrealized != kZero || updatedShares->sharesTotal != 0 || (afterVault.version == VaultVersion::FixedPrecision && - afterVault.assetsDeployed != kZero)) + (afterVault.assetsDeployed != kZero || afterVault.yieldUnrealized != kZero))) { JLOG(j.fatal()) // << "Invariant failed: created vault must be empty"; diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index 455df9f991..4af48c9bfc 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -748,9 +748,9 @@ LoanSet::doApply() view.update(vaultSle); // Update the balances in the loan broker - // FixedPrecision default subtracts principal from DebtTotal exactly. That - // depends on origination never rounding DebtTotal at a scale coarser than - // the asset's base scale. + // On FixedPrecision Vaults, origination adds the principal to DebtTotal + // exactly, and LoanPay and default later subtract exact amounts, so + // DebtTotal must never be rounded at a scale coarser than the base scale. adjustBrokerDebtTotal(brokerSle->at(sfDebtTotal), vaultSle, debtTotalDelta, vaultScale); adjustLoanBrokerOwnerCount(view, brokerSle, 1, j_); loanSequenceProxy += 1; diff --git a/src/test/app/lending/LoanSetFixedPrecision_test.cpp b/src/test/app/lending/LoanSetFixedPrecision_test.cpp index 414ad47c3d..55abddb1db 100644 --- a/src/test/app/lending/LoanSetFixedPrecision_test.cpp +++ b/src/test/app/lending/LoanSetFixedPrecision_test.cpp @@ -214,15 +214,18 @@ class LoanSetFixedPrecision_test : public LoanFixedPrecisionBase // truncates below the full share count, so the clawback succeeds and leaves // shares behind. void - testLendingFullClawbackClampsInsteadOfHasObligations() + testLendingFullClawbackClampsInsteadOfHasObligations( + FeatureBitset const amendments, + std::string const& label) { using namespace test::jtx; testcase( "Lending: full-value VaultClawback with a large AssetsDeployed clamps to " - "AssetsAvailable instead of tecHAS_OBLIGATIONS"); + "AssetsAvailable instead of tecHAS_OBLIGATIONS (" + + label + ")"); - Env env(*this, features()); + Env env(*this, amendments); auto const [issuer, owner, depositor, asset] = setupIou(env, {.clawback = true}); auto const fixture = setupLendingVault( @@ -599,7 +602,10 @@ public: testLendingAssetsDeployedTwoLoans(); testLendingClawbackAndWithdrawWhileLoanOpen(); testLendingFinalWithdrawalWhileLoanOpen(); - testLendingFullClawbackClampsInsteadOfHasObligations(); + testLendingFullClawbackClampsInsteadOfHasObligations(features(), "all amendments"); + // FixedPrecision Vaults take the clamped path even without fixCleanup3_1_3. + testLendingFullClawbackClampsInsteadOfHasObligations( + features() - fixCleanup3_1_3, "without fixCleanup3_1_3"); testLendingCashBasisControl(); testOriginationGuardBlocksCoarsening(); testLoanSetTransferLegShapes(); diff --git a/src/test/app/vault/VaultClawback_test.cpp b/src/test/app/vault/VaultClawback_test.cpp index 40a8e7885b..554f8d9e54 100644 --- a/src/test/app/vault/VaultClawback_test.cpp +++ b/src/test/app/vault/VaultClawback_test.cpp @@ -827,12 +827,10 @@ private: // Test pre-fixCleanup3_1_3 legacy path: zero-amount clawback // returns early without clamping to assetsAvailable. This needs a - // Legacy/CashBasis Vault, not the FixedPrecision Vault the outer - // `env` produces, so it runs against its own Env with - // featureLendingProtocolV1_2 disabled; the FixedPrecision - // equivalent of this scenario is covered above (zero-amount - // clawback clamped with outstanding loan) and in - // VaultFixedPrecision_test's + // Legacy/CashBasis Vault, so it runs against its own Env with + // featureLendingProtocolV1_2 disabled. FixedPrecision Vaults always + // take the clamped path, with and without fixCleanup3_1_3; see + // LoanSetFixedPrecision_test's // testLendingFullClawbackClampsInsteadOfHasObligations. { testcase( diff --git a/src/test/app/vault/VaultHelpers_test.cpp b/src/test/app/vault/VaultHelpers_test.cpp index 7a55226e0c..b26565212f 100644 --- a/src/test/app/vault/VaultHelpers_test.cpp +++ b/src/test/app/vault/VaultHelpers_test.cpp @@ -80,6 +80,9 @@ private: { vault->at(sfLEVersion) = std::to_underlying(VaultVersion::FixedPrecision); vault->at(sfScale) = *fixedScale; + // FixedPrecision derives its total from these two fields. + vault->at(sfAssetsAvailable) = assetsTotal; + vault->at(sfAssetsDeployed) = Number{0}; } return vault; } diff --git a/src/tests/libxrpl/protocol/VaultBalanceTests.cpp b/src/tests/libxrpl/protocol/VaultBalanceTests.cpp index 32698bfda0..69e6686d00 100644 --- a/src/tests/libxrpl/protocol/VaultBalanceTests.cpp +++ b/src/tests/libxrpl/protocol/VaultBalanceTests.cpp @@ -529,5 +529,53 @@ TEST_F(VaultBalance, credit_to_posterior_scale_ignores_ambient_rounding_mode) EXPECT_EQ(Number(credit), exactDownwardCredit); } +// creditToPosteriorAvailableScale returns +// floor16(AssetsAvailable + raw) - AssetsAvailable, never finer than -Scale. + +TEST_F(VaultBalance, credit_to_posterior_available_scale_integral_passes_through) +{ + auto const vault = makeVault(xrpIssue(), Number{1'000}, VaultVersion::FixedPrecision); + STAmount const credit = + creditToPosteriorAvailableScale(vault, Number{250}, Number::RoundingMode::Downward); + EXPECT_EQ(credit, STAmount(xrpIssue(), 250)); +} + +TEST_F(VaultBalance, credit_to_posterior_available_scale_floors_to_base_grid) +{ + // Scale 6: the base grid is 1e-6, and AssetsAvailable is small enough + // that the posterior grid is the base grid. + auto const vault = iouVault(Number{0}, Number{100}); + STAmount const credit = creditToPosteriorAvailableScale( + vault, Number{12'345'678, -7}, Number::RoundingMode::Downward); + EXPECT_EQ(credit, iouAmount(Number{1'234'567, -6})); +} + +TEST_F(VaultBalance, credit_to_posterior_available_scale_floors_the_sum_across_power_of_ten) +{ + // 9999999999.999999 + 0.000011 crosses 10^10, where the 16-digit grid is + // 1e-5. Flooring the delta alone would give 0.00001; flooring the sum + // gives 10000000000.00001, so the credit is exactly 0.000011. + auto const vault = iouVault(Number{0}, Number{9'999'999'999'999'999, -6}); + STAmount const credit = + creditToPosteriorAvailableScale(vault, Number{11, -6}, Number::RoundingMode::Downward); + EXPECT_EQ(credit, iouAmount(Number{11, -6})); + EXPECT_EQ( + STAmount(iou_, Number{9'999'999'999'999'999, -6} + Number(credit)), + iouAmount(Number{1'000'000'000'000'001, -5})); +} + +TEST_F(VaultBalance, credit_to_posterior_available_scale_uses_the_exact_raw_amount) +{ + // raw has 17 significant digits. Exact: 0.000009 + 12345678901.234561 = + // 12345678901.23457 on the 1e-5 grid, so the credit is + // 12345678901.234561, stored as 12345678901.23456. Rounding raw to 16 + // digits first (12345678901.23456) would floor the sum to + // 12345678901.23456 and give a credit one unit lower, 12345678901.23455. + auto const vault = iouVault(Number{0}, Number{9, -6}); + STAmount const credit = creditToPosteriorAvailableScale( + vault, Number{12'345'678'901'234'561, -6}, Number::RoundingMode::Downward); + EXPECT_EQ(credit, iouAmount(Number{1'234'567'890'123'456, -5})); +} + } // namespace } // namespace xrpl