Apply FixedPrecision rounding clamp to Deposit/Withdraw/Clawback; fix vault gate/test gaps

VaultDeposit/VaultWithdraw/VaultClawback now apply the fixCleanup3_4_0
posterior-scale rounding clamp unconditionally for FixedPrecision vaults,
not only when fix340Enabled. VaultCreate's featureLendingProtocolV1_1 gate
check now also accepts V1_2, matching the "V1.2 implies V1.1" semantics
already encoded in doApply.

Test changes:
- VaultHelpers_test: add a FixedPrecision-tagged clamp table to
  clampToAssetsTotalScale coverage; previously only Legacy/CashBasis was
  exercised.
- VaultClosedEnded_test: fix the closed-ended gate test, which asserted
  temDISABLED after subtracting only V1_1 even though V1_2 alone already
  satisfies the gate; add a case proving V1_2-only still opens it.
- VaultFixedPrecision_test: dedupe repeated scaled-vault setup into a
  shared helper.
- VaultTestBase: keep V1_2 excluded from all_ with a comment explaining
  why VaultBugs_test's precision-boundary scenarios are structurally
  unreachable under FixedPrecision's Open-zone cap, not just deferred.
This commit is contained in:
Vito
2026-09-22 15:39:12 +02:00
parent 9ae863d89e
commit 5e0e36f87d
8 changed files with 210 additions and 52 deletions

View File

@@ -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)

View File

@@ -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);

View File

@@ -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

View File

@@ -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

View File

@@ -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);
});
/*

View File

@@ -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<test::jtx::Vault, Keylet>
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<std::uint8_t>(kVaultMaximumFixedIouScale + 1);
env(invalid, Ter(temMALFORMED));
BEAST_EXPECT(!env.le(invalidKeylet));
}
for (std::uint8_t const scaleValue :
{kVaultMaximumFixedIouScale,
static_cast<std::uint8_t>(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();

View File

@@ -24,6 +24,7 @@
#include <memory>
#include <optional>
#include <string>
#include <utility>
namespace xrpl {
@@ -58,24 +59,40 @@ private:
std::optional<Number> 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<SLE>
makeVault(Asset const& asset, Number const& assetsTotal)
makeVault(
Asset const& asset,
Number const& assetsTotal,
std::optional<std::uint8_t> fixedScale = std::nullopt)
{
auto vault = std::make_shared<SLE>(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 <std::size_t N>
void
runCases(Asset const& asset, std::array<Case, N> const& cases)
runCases(
Asset const& asset,
std::array<Case, N> const& cases,
std::optional<std::uint8_t> fixedScale = std::nullopt)
{
std::array<Number::RoundingMode, 4> 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<Case, 6> 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);
}
};

View File

@@ -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"};
};