diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index 059da7cc0f..1c32564e44 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -360,7 +360,9 @@ VaultClawback::assetsToClawback( // rails change by the same representable delta. sharesDestroyed is intentionally NOT // re-derived here: the holder's shares are burned for their pre-clamp value, so any // sub-ULP trimmed off stays in the vault for the remaining shareholders. - if (ctx_.view().rules().enabled(fixCleanup3_4_0) && assetsRecovered > beast::kZero) + if ((ctx_.view().rules().enabled(fixCleanup3_4_0) || + getVaultVersion(vault) == VaultVersion::FixedPrecision) && + assetsRecovered > beast::kZero) { auto const maybeClamped = clampToAssetsTotalScale(vault, -assetsRecovered); if (!maybeClamped) diff --git a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp index 86873432d4..f3c3347603 100644 --- a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp @@ -45,6 +45,7 @@ VaultCreate::checkExtraFeatures(PreflightContext const& ctx) return false; if (!ctx.rules.enabled(featureLendingProtocolV1_1) && + !ctx.rules.enabled(featureLendingProtocolV1_2) && (ctx.tx.isFieldPresent(sfVaultKind) || ctx.tx.isFieldPresent(sfSubscriptionDate) || ctx.tx.isFieldPresent(sfRedemptionDate))) return false; @@ -276,12 +277,12 @@ VaultCreate::doApply() } if (scale != 0u) vault->at(sfScale) = scale; - // featureLendingProtocolV1_2 is defined to require V1.1: new vaults get - // FixedPrecision plus the V1.1 VaultKind fields even if a test enables - // only V1.2. YieldUnrealized is SoeDefault, so writing zero stores the - // field as absent, matching LossUnrealized. + // Treat featureLendingProtocolV1_2 as implying V1.1 when creating a vault; + // there is no FeatureBitset-level dependency lock. YieldUnrealized is + // SoeDefault, so writing zero stores the field as absent, matching + // LossUnrealized. bool const fixedPrecision = view().rules().enabled(featureLendingProtocolV1_2); - bool const cashBasis = view().rules().enabled(featureLendingProtocolV1_1); + bool const cashBasis = view().rules().enabled(featureLendingProtocolV1_1) || fixedPrecision; if (fixedPrecision) { vault->at(sfLEVersion) = std::to_underlying(VaultVersion::FixedPrecision); diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index efb117b720..53425b1e13 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -329,7 +329,7 @@ VaultDeposit::doApply() // Post-fixCleanup3_4_0: round the deposit to the sfAssetsTotal scale so all accounting // fields (trust line / MPT, sfAssetsAvailable, sfAssetsTotal) change by the same // representable delta. - if (fix340Enabled) + if (fix340Enabled || getVaultVersion(vault) == VaultVersion::FixedPrecision) { // Round down at the posterior sfAssetsTotal scale so the vault is credited by no more // than the depositor paid. Keep the share count from the first round trip: the clamp diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 4f6f98325a..0e36a86356 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -450,7 +450,8 @@ VaultWithdraw::doApply() // permits fixed-share zero-asset withdrawals in a fully-impaired vault (where // assetsTotalForWithdrawal == 0), and clamping-then-rejecting would undo that. Also skip on // the final-withdrawal path, which overwrites assetsWithdrawn with sfAssetsAvailable below. - if (fix340Enabled && !isFinalWithdrawal && assetsWithdrawn > beast::kZero) + if ((fix340Enabled || getVaultVersion(vault) == VaultVersion::FixedPrecision) && + !isFinalWithdrawal && assetsWithdrawn > beast::kZero) { // Check availability against the unclamped amount first, so a withdrawal that is both // over the vault's available balance and sub-ULP at the posterior sfAssetsTotal scale diff --git a/src/test/app/vault/VaultClosedEnded_test.cpp b/src/test/app/vault/VaultClosedEnded_test.cpp index 6909a1bffa..9b55f5dd3a 100644 --- a/src/test/app/vault/VaultClosedEnded_test.cpp +++ b/src/test/app/vault/VaultClosedEnded_test.cpp @@ -65,7 +65,26 @@ private: auto const maxPeriod = kMaxInvestmentPeriod; auto const closedEnded = std::to_underlying(VaultKind::ClosedEnded); - // Gate: the three new fields require featureLendingProtocolV1_1. + // Gate: the three new fields require featureLendingProtocolV1_1 OR + // featureLendingProtocolV1_2 (VaultCreate.cpp treats V1.2 as + // implying V1.1). Only disabled when BOTH are absent. + withEnv( + testableAmendments() - featureLendingProtocolV1_1 - featureLendingProtocolV1_2, + [&](Env& env, Account const& owner, Vault& vault) { + auto const sub = env.now().time_since_epoch().count() + 60; + auto [tx, keylet] = vault.create( + {.owner = owner, + .asset = asset, + .vaultKind = closedEnded, + .subscriptionDate = sub, + .redemptionDate = sub + minPeriod}); + env(tx, Ter{temDISABLED}); + }); + + // V1.2 alone (no V1.1) still satisfies the gate for closed-ended + // vaults, same as it does for open-ended vaults in + // VaultFixedPrecision_test.cpp's "VaultCreate treats V1.2 as + // implying V1.1" case. withEnv( testableAmendments() - featureLendingProtocolV1_1, [&](Env& env, Account const& owner, Vault& vault) { @@ -76,7 +95,11 @@ private: .vaultKind = closedEnded, .subscriptionDate = sub, .redemptionDate = sub + minPeriod}); - env(tx, Ter{temDISABLED}); + env(tx); + env.close(); + auto const sle = env.le(keylet); + if (BEAST_EXPECT(sle)) + BEAST_EXPECT(sle->at(sfVaultKind) == closedEnded); }); /* diff --git a/src/test/app/vault/VaultFixedPrecision_test.cpp b/src/test/app/vault/VaultFixedPrecision_test.cpp index a252a2e712..c6f816de76 100644 --- a/src/test/app/vault/VaultFixedPrecision_test.cpp +++ b/src/test/app/vault/VaultFixedPrecision_test.cpp @@ -33,6 +33,24 @@ class VaultFixedPrecision_test : public VaultTestBase featureLendingProtocolV1_2; } + // Submits a VaultCreate for an open-ended vault at the given fixed + // Scale and closes the ledger. Shared by every scenario below that + // needs a Scale-6 vault rather than the protocol default. + static std::pair + createScaledVault( + test::jtx::Env& env, + test::jtx::Account const& owner, + Asset const& asset, + std::uint8_t scale) + { + test::jtx::Vault const vault{env}; + auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); + create[sfScale] = scale; + env(create); + env.close(); + return {vault, keylet}; + } + void testCreate() { @@ -54,12 +72,40 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const sle = env.le(keylet); - BEAST_EXPECT(sle); + if (!BEAST_EXPECT(sle)) + return; BEAST_EXPECT(sle->at(sfLEVersion) == std::to_underlying(VaultVersion::FixedPrecision)); BEAST_EXPECT(sle->at(sfScale) == kVaultDefaultIouScale); BEAST_EXPECT(sle->at(sfYieldUnrealized) == beast::kZero); } + { + testcase("VaultCreate treats V1.2 as implying V1.1"); + Env env(*this, features() - featureLendingProtocolV1_1); + env.fund(XRP(1'000'000), issuer, owner); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create( + {.owner = owner, + .asset = asset, + .vaultKind = std::to_underlying(VaultKind::OpenEnded)}); + tx[sfScale] = kVaultMaximumFixedIouScale; + env(tx); + env.close(); + + auto const sle = env.le(keylet); + if (!BEAST_EXPECT(sle)) + return; + BEAST_EXPECT(sle->at(sfLEVersion) == std::to_underlying(VaultVersion::FixedPrecision)); + BEAST_EXPECT(sle->at(sfVaultKind) == std::to_underlying(VaultKind::OpenEnded)); + + auto [invalid, invalidKeylet] = vault.create({.owner = owner, .asset = asset}); + invalid[sfScale] = static_cast(kVaultMaximumFixedIouScale + 1); + env(invalid, Ter(temMALFORMED)); + BEAST_EXPECT(!env.le(invalidKeylet)); + } + for (std::uint8_t const scaleValue : {kVaultMaximumFixedIouScale, static_cast(kVaultMaximumFixedIouScale + 1)}) @@ -102,7 +148,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const sle = env.le(keylet); - BEAST_EXPECT(sle); + if (!BEAST_EXPECT(sle)) + return; BEAST_EXPECT(sle->at(sfLEVersion) == std::to_underlying(VaultVersion::CashBasis)); BEAST_EXPECT(!sle->isFieldPresent(sfYieldUnrealized)); } @@ -127,18 +174,15 @@ class VaultFixedPrecision_test : public VaultTestBase env(pay(issuer, owner, asset(open + Number{1}))); env.close(); - Vault const vault{env}; - auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); - create[sfScale] = 6; - env(create); - env.close(); + auto [vault, keylet] = createScaledVault(env, owner, asset, 6); testcase("VaultDeposit admits the Open boundary"); env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(open)})); env.close(); auto const atOpen = env.le(keylet); - BEAST_EXPECT(atOpen); + if (!BEAST_EXPECT(atOpen)) + return; BEAST_EXPECT(atOpen->at(sfAssetsTotal) == open); BEAST_EXPECT(atOpen->at(sfAssetsAvailable) == open); @@ -148,7 +192,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const afterRejected = env.le(keylet); - BEAST_EXPECT(afterRejected); + if (!BEAST_EXPECT(afterRejected)) + return; BEAST_EXPECT(afterRejected->at(sfAssetsTotal) == open); BEAST_EXPECT(afterRejected->at(sfAssetsAvailable) == open); } @@ -180,7 +225,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const before = env.le(keylet); - BEAST_EXPECT(before); + if (!BEAST_EXPECT(before)) + return; BEAST_EXPECT(before->at(sfLEVersion) == std::to_underlying(VaultVersion::CashBasis)); env.enableFeature(featureLendingProtocolV1_2); @@ -189,7 +235,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const after = env.le(keylet); - BEAST_EXPECT(after); + if (!BEAST_EXPECT(after)) + return; BEAST_EXPECT(after->at(sfLEVersion) == std::to_underlying(VaultVersion::CashBasis)); BEAST_EXPECT(!after->isFieldPresent(sfYieldUnrealized)); BEAST_EXPECT(after->at(sfAssetsTotal) == deposit); @@ -222,11 +269,7 @@ class VaultFixedPrecision_test : public VaultTestBase env(pay(issuer, owner, asset(4))); env.close(); - Vault const vault{env}; - auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); - create[sfScale] = 6; - env(create); - env.close(); + auto [vault, keylet] = createScaledVault(env, owner, asset, 6); testcase("VaultDeposit books the truncated amount on the base grid"); env(vault.deposit( @@ -234,7 +277,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto afterDeposit = env.le(keylet); - BEAST_EXPECT(afterDeposit); + if (!BEAST_EXPECT(afterDeposit)) + return; BEAST_EXPECT(afterDeposit->at(sfAssetsTotal) == (Number{3'234'567, -6})); BEAST_EXPECT(afterDeposit->at(sfAssetsAvailable) == (Number{3'234'567, -6})); BEAST_EXPECT(env.balance(owner, asset) == asset(Number{765'433, -6})); @@ -245,7 +289,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto afterWithdraw = env.le(keylet); - BEAST_EXPECT(afterWithdraw); + if (!BEAST_EXPECT(afterWithdraw)) + return; BEAST_EXPECT(afterWithdraw->at(sfAssetsTotal) == (Number{2'234'567, -6})); BEAST_EXPECT(afterWithdraw->at(sfAssetsAvailable) == (Number{2'234'567, -6})); BEAST_EXPECT(env.balance(owner, asset) == asset(Number{1'765'433, -6})); @@ -259,7 +304,8 @@ class VaultFixedPrecision_test : public VaultTestBase env.close(); auto const afterClawback = env.le(keylet); - BEAST_EXPECT(afterClawback); + if (!BEAST_EXPECT(afterClawback)) + return; BEAST_EXPECT(afterClawback->at(sfAssetsTotal) == (Number{1'234'567, -6})); BEAST_EXPECT(afterClawback->at(sfAssetsAvailable) == (Number{1'234'567, -6})); } @@ -298,7 +344,8 @@ class VaultFixedPrecision_test : public VaultTestBase Ter(tecLIMIT_EXCEEDED)); auto const sle = env.le(keylet); - BEAST_EXPECT(sle); + if (!BEAST_EXPECT(sle)) + return; BEAST_EXPECT(sle->at(sfAssetsTotal) == Number{open}); } @@ -321,11 +368,7 @@ class VaultFixedPrecision_test : public VaultTestBase env(pay(issuer, owner, asset(1))); env.close(); - Vault const vault{env}; - auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); - create[sfScale] = 6; - env(create); - env.close(); + auto [vault, keylet] = createScaledVault(env, owner, asset, 6); env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(Number{1, -7})}), Ter(tecPRECISION_LOSS)); @@ -350,11 +393,7 @@ class VaultFixedPrecision_test : public VaultTestBase env(pay(issuer, owner, asset(2))); env.close(); - Vault const vault{env}; - auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); - create[sfScale] = 6; - env(create); - env.close(); + auto [vault, keylet] = createScaledVault(env, owner, asset, 6); env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(1)})); env.close(); @@ -383,11 +422,7 @@ class VaultFixedPrecision_test : public VaultTestBase env(pay(issuer, depositor, asset(2))); env.close(); - Vault const vault{env}; - auto [create, keylet] = vault.create({.owner = owner, .asset = asset}); - create[sfScale] = 6; - env(create); - env.close(); + auto [vault, keylet] = createScaledVault(env, owner, asset, 6); env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(1)})); env.close(); diff --git a/src/test/app/vault/VaultHelpers_test.cpp b/src/test/app/vault/VaultHelpers_test.cpp index 3d6ff6fc6a..5e13589604 100644 --- a/src/test/app/vault/VaultHelpers_test.cpp +++ b/src/test/app/vault/VaultHelpers_test.cpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace xrpl { @@ -58,24 +59,40 @@ private: std::optional expected; // nullopt means tecPRECISION_LOSS }; - // Builds a bare ltVAULT SLE with sfLEVersion absent, preserving the - // pre-V1.2 Legacy behavior exercised by this existing clamp table. + // Builds a bare ltVAULT SLE. With `fixedScale` absent, sfLEVersion stays + // absent too, preserving the pre-V1.2 Legacy behavior exercised by the + // existing clamp table. With `fixedScale` set, the SLE is stamped + // FixedPrecision with that Scale, exercising clampToAssetsTotalScale's + // roundToPosteriorVaultScale branch instead. static std::shared_ptr - makeVault(Asset const& asset, Number const& assetsTotal) + makeVault( + Asset const& asset, + Number const& assetsTotal, + std::optional fixedScale = std::nullopt) { auto vault = std::make_shared(keylet::vault(uint256(1))); vault->setFieldIssue(sfAsset, STIssue{sfAsset, asset}); vault->at(sfAssetsTotal) = assetsTotal; associateAsset(*vault, asset); + if (fixedScale) + { + vault->at(sfLEVersion) = std::to_underlying(VaultVersion::FixedPrecision); + vault->at(sfScale) = *fixedScale; + } return vault; } // Runs every case in `cases` against `asset`, once per ambient rounding // mode. The function must give the same answer under all four modes, // and its answer must match the hand-derived `expected` value. + // `fixedScale`, when set, builds a FixedPrecision vault at that Scale + // instead of the default Legacy vault. template void - runCases(Asset const& asset, std::array const& cases) + runCases( + Asset const& asset, + std::array const& cases, + std::optional fixedScale = std::nullopt) { std::array const modes{ Number::RoundingMode::ToNearest, @@ -87,7 +104,7 @@ private: { testcase(c.name); - auto const vault = makeVault(asset, c.assetsTotal); + auto const vault = makeVault(asset, c.assetsTotal, fixedScale); BEAST_EXPECTS( Number(vault->at(sfAssetsTotal)) == c.assetsTotal, std::string(c.name) + @@ -459,6 +476,80 @@ private: runCases(xrp, xrpCases); } + // ------------------------------------------------------------------- + // FixedPrecision vaults: clampToAssetsTotalScale takes the + // roundToPosteriorVaultScale branch instead of the Legacy/CashBasis + // scale()-of-the-sum branch. All rows below use a Scale-6 vault + // (baseScale -6) with assetsTotal already sitting on that grid, so + // TowardsZero-truncating `delta` itself to scale -6 is the whole + // story: no case here forces liveScale to coarsen past baseScale + // (that scenario needs a non-unit share price and is exercised by + // VaultFixedPrecision_test.cpp's testPartialTowardZeroRounding / + // dust tests through real transactions instead). + // ------------------------------------------------------------------- + void + testFixedPrecisionClamp(Asset const& iou) + { + std::uint8_t const fixedScale = 6; + Number const onGrid{3'234'567, -6}; // 3.234567, exact at scale -6. + + std::array const cases{ + Case{ + // delta already exact at the base grid: passes through + // unchanged, same as a Legacy on-grid debit. + .name = "FixedPrecision debit: exact on the base grid", + .assetsTotal = onGrid, + .delta = Number{-1, -6}, + .expected = Number{1, -6}, + }, + Case{ + // delta already exact at the base grid: passes through + // unchanged, same as a Legacy on-grid credit. + .name = "FixedPrecision credit: exact on the base grid", + .assetsTotal = onGrid, + .delta = Number{2, -6}, + .expected = Number{2, -6}, + }, + Case{ + // delta = -1.7 base units. TowardsZero truncates the + // magnitude to 1 base unit -- this is the "books the + // truncated amount" behavior VaultFixedPrecision_test's + // testPartialTowardZeroRounding exercises end-to-end via + // VaultWithdraw; this row pins it at the helper level. + .name = "FixedPrecision debit: truncated toward zero on the base grid", + .assetsTotal = onGrid, + .delta = Number{-17, -7}, + .expected = Number{1, -6}, + }, + Case{ + // delta = +2.3 base units, truncates to 2 base units for + // the same reason as the row above. + .name = "FixedPrecision credit: truncated toward zero on the base grid", + .assetsTotal = onGrid, + .delta = Number{23, -7}, + .expected = Number{2, -6}, + }, + Case{ + // delta = -0.3 base units: sub-ULP at the fixed grid, so + // TowardsZero truncates it entirely to zero. + .name = "FixedPrecision debit: sub-ULP dust rejected at the base grid", + .assetsTotal = onGrid, + .delta = Number{-3, -7}, + .expected = std::nullopt, + }, + Case{ + // delta = +0.4 base units: same sub-ULP rejection for a + // credit. + .name = "FixedPrecision credit: sub-ULP dust rejected at the base grid", + .assetsTotal = onGrid, + .delta = Number{4, -7}, + .expected = std::nullopt, + }, + }; + + runCases(iou, cases, fixedScale); + } + public: void run() override @@ -473,6 +564,7 @@ public: testIouDebits(iou); testIouCredits(iou); testIntegralAssets(mpt, xrp); + testFixedPrecisionClamp(iou); } }; diff --git a/src/test/app/vault/VaultTestBase.h b/src/test/app/vault/VaultTestBase.h index 93225b3794..e3711e94f8 100644 --- a/src/test/app/vault/VaultTestBase.h +++ b/src/test/app/vault/VaultTestBase.h @@ -113,8 +113,12 @@ protected: return {.vault = vault, .keylet = keylet, .sub = sub, .red = red}; } - // Keep legacy Vault suites on their pre-V1.2 behavior. Tests for the - // fixed-precision protocol enable featureLendingProtocolV1_2 explicitly. + // The IOU precision-boundary bugs in VaultBugs_test.cpp probe the + // STAmount 16-digit mantissa cliff (~1e16). FixedPrecision's Open-zone + // cap (9e(15-Scale)) makes that value unreachable at any Scale, so + // these scenarios cannot be reproduced under V1.2 by construction. + // Tests for the fixed-precision protocol enable featureLendingProtocolV1_2 + // explicitly. FeatureBitset const all_{test::jtx::testableAmendments() - featureLendingProtocolV1_2}; std::string const iouCurrency_{"IOU"}; };