ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-03 00:36:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Fix VaultInvariant and VaultDeposit precision bugs at IOU scale boundaries (#7272)

Browse Source
This commit is contained in:
Vito
2026-05-19 19:50:36 +02:00
parent 7829e63020
commit 19296eba73
5 changed files with 688 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
include/xrpl/tx/invariants/VaultInvariant.h
View File

@@ -4,9 +4,11 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/MPTIssue.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <optional>
#include <unordered_map>
@@ -89,6 +91,21 @@ public:
// Compute the coarsest scale required to represent all numbers
[[nodiscard]] static std::int32_t
computeCoarsestScale(std::vector<DeltaInfo> const& numbers);
[[nodiscard]] std::int32_t
computeVaultMinScale(DeltaInfo const& vaultDelta, Rules const& rules) const;
[[nodiscard]] std::optional<DeltaInfo>
deltaAssets(AccountID const& id) const;
[[nodiscard]] std::optional<DeltaInfo>
deltaAssetsTxAccount(STTx const& tx, XRPAmount fee) const;
[[nodiscard]] std::optional<DeltaInfo>
deltaShares(AccountID const& id) const;
[[nodiscard]] static bool
vaultHoldsNoAssets(Vault const& vault);
};
} // namespace xrpl

311
src/libxrpl/tx/invariants/VaultInvariant.cpp
View File

@@ -186,6 +186,93 @@ ValidVault::visitEntry(
}
}
std::optional<ValidVault::DeltaInfo>
ValidVault::deltaAssets(AccountID const& id) const
{
auto const& vaultAsset = afterVault_[0].asset;
auto const get = [&](auto const& it, std::int8_t sign = 1) -> std::optional<DeltaInfo> {
if (it == deltas_.end())
return std::nullopt;
return DeltaInfo{it->second.delta * sign, it->second.scale};
};
return std::visit(
[&]<typename TIss>(TIss const& issue) -> std::optional<DeltaInfo> {
if constexpr (std::is_same_v<TIss, Issue>)
{
if (isXRP(issue))
return get(deltas_.find(keylet::account(id).key));
return get(
deltas_.find(keylet::line(id, issue).key), id > issue.getIssuer() ? -1 : 1);
}
else if constexpr (std::is_same_v<TIss, MPTIssue>)
{
return get(deltas_.find(keylet::mptoken(issue.getMptID(), id).key));
}
},
vaultAsset.value());
}
std::optional<ValidVault::DeltaInfo>
ValidVault::deltaAssetsTxAccount(STTx const& tx, XRPAmount fee) const
{
auto const& vaultAsset = afterVault_[0].asset;
auto ret = deltaAssets(tx[sfAccount]);
if (!ret.has_value() || !vaultAsset.native())
return ret;
if (auto const delegate = tx[~sfDelegate]; delegate.has_value() && *delegate != tx[sfAccount])
return ret;
ret->delta += fee.drops();
if (ret->delta == kZero)
return std::nullopt;
return ret;
}
std::optional<ValidVault::DeltaInfo>
ValidVault::deltaShares(AccountID const& id) const
{
auto const& afterVault = afterVault_[0];
auto const it = [&]() {
if (id == afterVault.pseudoId)
return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key);
return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key);
}();
return it != deltas_.end() ? std::optional<DeltaInfo>(it->second) : std::nullopt;
}
bool
ValidVault::vaultHoldsNoAssets(Vault const& vault)
{
return vault.assetsAvailable == 0 && vault.assetsTotal == 0;
}
std::int32_t
ValidVault::computeVaultMinScale(DeltaInfo const& vaultDelta, Rules const& rules) const
{
// Returns the posterior `assetsTotal` scale.
//
// 1. Because STAmounts are normalized, `assetsTotal` (being >= `assetsAvailable`)
// safely represents the coarsest exponent needed for both fields.
//
// 2. The scale may decrease (withdraw/clawback) or increase (deposit). In both cases
// we ensure the vault is in a legitimate state in the post-transaction scale.
auto const& afterVault = afterVault_[0];
auto const& vaultAsset = afterVault.asset;
if (rules.enabled(fixCleanup3_2_0))
return scale(afterVault.assetsTotal, vaultAsset);
auto const& beforeVault = beforeVault_[0];
auto const totalDelta =
DeltaInfo::makeDelta(beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset);
auto const availableDelta =
DeltaInfo::makeDelta(beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset);
return computeCoarsestScale({vaultDelta, totalDelta, availableDelta});
}
bool
ValidVault::finalize(
STTx const& tx,
@@ -445,61 +532,6 @@ ValidVault::finalize(
}
auto const& vaultAsset = afterVault.asset;
auto const deltaAssets = [&](AccountID const& id) -> std::optional<DeltaInfo> {
auto const get = //
[&](auto const& it, std::int8_t sign = 1) -> std::optional<DeltaInfo> {
if (it == deltas_.end())
return std::nullopt;
return DeltaInfo{it->second.delta * sign, it->second.scale};
};
return std::visit(
[&]<typename TIss>(TIss const& issue) {
if constexpr (std::is_same_v<TIss, Issue>)
{
if (isXRP(issue))
return get(deltas_.find(keylet::account(id).key));
return get(
deltas_.find(keylet::line(id, issue).key), id > issue.getIssuer() ? -1 : 1);
}
else if constexpr (std::is_same_v<TIss, MPTIssue>)
{
return get(deltas_.find(keylet::mptoken(issue.getMptID(), id).key));
}
},
vaultAsset.value());
};
auto const deltaAssetsTxAccount = [&]() -> std::optional<DeltaInfo> {
auto ret = deltaAssets(tx[sfAccount]);
// Nothing returned or not XRP transaction
if (!ret.has_value() || !vaultAsset.native())
return ret;
// Delegated transaction; no need to compensate for fees
if (auto const delegate = tx[~sfDelegate];
delegate.has_value() && *delegate != tx[sfAccount])
return ret;
ret->delta += fee.drops();
if (ret->delta == kZero)
return std::nullopt;
return ret;
};
auto const deltaShares = [&](AccountID const& id) -> std::optional<DeltaInfo> {
auto const it = [&]() {
if (id == afterVault.pseudoId)
return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key);
return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key);
}();
return it != deltas_.end() ? std::optional<DeltaInfo>(it->second) : std::nullopt;
};
auto const vaultHoldsNoAssets = [&](Vault const& vault) {
return vault.assetsAvailable == 0 && vault.assetsTotal == 0;
};
// Technically this does not need to be a lambda, but it's more
// convenient thanks to early "return false"; the not-so-nice
@@ -629,16 +661,8 @@ ValidVault::finalize(
return false; // That's all we can do
}
// Get the coarsest scale to round calculations to
auto const totalDelta = DeltaInfo::makeDelta(
beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset);
auto const availableDelta = DeltaInfo::makeDelta(
beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset);
auto const minScale = computeCoarsestScale({
*maybeVaultDeltaAssets,
totalDelta,
availableDelta,
});
// Get the posterior scale to round calculations to
auto const minScale = computeVaultMinScale(*maybeVaultDeltaAssets, view.rules());
auto const vaultDeltaAssets =
roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale);
@@ -669,12 +693,11 @@ ValidVault::finalize(
if (!issuerDeposit)
{
auto const maybeAccDeltaAssets = deltaAssetsTxAccount();
auto const maybeAccDeltaAssets = deltaAssetsTxAccount(tx, fee);
if (!maybeAccDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor "
"balance";
JLOG(j.fatal())
<< "Invariant failed: deposit must change depositor balance";
return false;
}
auto const localMinScale =
@@ -685,19 +708,20 @@ ValidVault::finalize(
auto const localVaultDeltaAssets =
roundToAsset(vaultAsset, vaultDeltaAssets, localMinScale);
// For IOUs, if the deposit amount is not-representable at depositor trustline
// scale deposit amount could round to zero, giving depositor shares for no
// assets. Unlike withdrawal, we do not allow that.
if (accountDeltaAssets >= kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must decrease depositor "
"balance";
JLOG(j.fatal())
<< "Invariant failed: deposit must decrease depositor balance";
result = false;
}
if (localVaultDeltaAssets * -1 != accountDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change vault and "
"depositor balance by equal amount";
JLOG(j.fatal()) << "Invariant failed: " << //
"deposit must change vault and depositor balance by equal amount";
result = false;
}
}
@@ -705,45 +729,38 @@ ValidVault::finalize(
if (afterVault.assetsMaximum > kZero &&
afterVault.assetsTotal > afterVault.assetsMaximum)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit assets outstanding must not "
"exceed assets maximum";
JLOG(j.fatal()) << "Invariant failed: " << //
"deposit assets outstanding must not exceed assets maximum";
result = false;
}
auto const maybeAccDeltaShares = deltaShares(tx[sfAccount]);
if (!maybeAccDeltaShares)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor "
"shares";
JLOG(j.fatal()) << "Invariant failed: deposit must change depositor shares";
return false; // That's all we can do
}
// We don't need to round shares, they are integral MPT
// We don't round shares, they are integral MPT
auto const& accountDeltaShares = *maybeAccDeltaShares;
if (accountDeltaShares.delta <= kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must increase depositor "
"shares";
JLOG(j.fatal()) << "Invariant failed: deposit must increase depositor shares";
result = false;
}
auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId);
if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change vault shares";
JLOG(j.fatal()) << "Invariant failed: deposit must change vault shares";
return false; // That's all we can do
}
// We don't need to round shares, they are integral MPT
// We don't round shares, they are integral MPT
auto const& vaultDeltaShares = *maybeVaultDeltaShares;
if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor and "
"vault shares by equal amount";
JLOG(j.fatal()) << "Invariant failed: " << //
"deposit must change depositor and vault shares by equal amount";
result = false;
}
@@ -751,8 +768,8 @@ ValidVault::finalize(
vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale);
if (assetTotalDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: deposit and assets "
"outstanding must add up";
JLOG(j.fatal())
<< "Invariant failed: deposit and assets outstanding must add up";
result = false;
}
@@ -760,8 +777,7 @@ ValidVault::finalize(
vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale);
if (assetAvailableDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: deposit and assets "
"available must add up";
JLOG(j.fatal()) << "Invariant failed: deposit and assets available must add up";
result = false;
}
@@ -772,34 +788,25 @@ ValidVault::finalize(
XRPL_ASSERT(
!beforeVault_.empty(),
"xrpl::ValidVault::finalize : withdrawal updated a "
"vault");
"xrpl::ValidVault::finalize : withdrawal updated a vault");
auto const& beforeVault = beforeVault_[0];
auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId);
if (!maybeVaultDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal must "
"change vault balance";
JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault balance";
return false; // That's all we can do
}
// Get the most coarse scale to round calculations to
auto const totalDelta = DeltaInfo::makeDelta(
beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset);
auto const availableDelta = DeltaInfo::makeDelta(
beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset);
auto const minScale =
computeCoarsestScale({*maybeVaultDeltaAssets, totalDelta, availableDelta});
// Get the posterior scale to round calculations to
auto const minScale = computeVaultMinScale(*maybeVaultDeltaAssets, view.rules());
auto const vaultPseudoDeltaAssets =
roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale);
if (vaultPseudoDeltaAssets >= kZero)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal must "
"decrease vault balance";
JLOG(j.fatal()) << "Invariant failed: withdrawal must decrease vault balance";
result = false;
}
@@ -814,7 +821,7 @@ ValidVault::finalize(
if (!issuerWithdrawal)
{
auto const maybeAccDelta = deltaAssetsTxAccount();
auto const maybeAccDelta = deltaAssetsTxAccount(tx, fee);
auto const maybeOtherAccDelta = [&]() -> std::optional<DeltaInfo> {
if (auto const destination = tx[~sfDestination];
destination && *destination != tx[sfAccount])
@@ -825,8 +832,7 @@ ValidVault::finalize(
if (maybeAccDelta.has_value() == maybeOtherAccDelta.has_value())
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change one "
"destination balance";
"Invariant failed: withdrawal must change one destination balance";
return false;
}
@@ -841,11 +847,21 @@ ValidVault::finalize(
auto const roundedDestinationDelta =
roundToAsset(vaultAsset, destinationDelta.delta, localMinScale);
if (roundedDestinationDelta <= kZero)
// Post-fixCleanup3_2_0: Tolerate zero-rounded destination deltas for IOUs only.
// If the receiver's trust line sits at a coarser scale, the inflow may
// safely round down to zero.
//
// XRP and MPT remain strict. Because they are integer-exact, a zero
// destination delta indicates a true accounting bug, not a rounding artifact.
bool const tolerateZeroDelta =
view.rules().enabled(fixCleanup3_2_0) && !vaultAsset.integral();
auto const invalidBalanceChange = tolerateZeroDelta
? roundedDestinationDelta < kZero
: roundedDestinationDelta <= kZero;
if (invalidBalanceChange)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must increase "
"destination balance";
"Invariant failed: withdrawal must increase destination balance";
result = false;
}
@@ -853,45 +869,39 @@ ValidVault::finalize(
roundToAsset(vaultAsset, vaultPseudoDeltaAssets, localMinScale);
if (localPseudoDeltaAssets * -1 != roundedDestinationDelta)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change vault "
"and destination balance by equal amount";
JLOG(j.fatal()) << "Invariant failed: " << //
"withdrawal must change vault and destination balance by equal amount";
result = false;
}
}
// We don't need to round shares, they are integral MPT
// We don't round shares, they are integral MPT
auto const accountDeltaShares = deltaShares(tx[sfAccount]);
if (!accountDeltaShares)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change depositor "
"shares";
JLOG(j.fatal()) << "Invariant failed: withdrawal must change depositor shares";
return false;
}
if (accountDeltaShares->delta >= kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must decrease depositor "
"shares";
JLOG(j.fatal())
<< "Invariant failed: withdrawal must decrease depositor shares";
result = false;
}
// We don't need to round shares, they are integral MPT
// We don't round shares, they are integral MPT
auto const vaultDeltaShares = deltaShares(afterVault.pseudoId);
if (!vaultDeltaShares || vaultDeltaShares->delta == kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change vault shares";
JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault shares";
return false; // That's all we can do
}
if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change depositor "
"and vault shares by equal amount";
JLOG(j.fatal()) << "Invariant failed: " << //
"withdrawal must change depositor and vault shares by equal amount";
result = false;
}
@@ -900,8 +910,8 @@ ValidVault::finalize(
// Note, vaultBalance is negative (see check above)
if (assetTotalDelta != vaultPseudoDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal and "
"assets outstanding must add up";
JLOG(j.fatal())
<< "Invariant failed: withdrawal and assets outstanding must add up";
result = false;
}
@@ -910,8 +920,8 @@ ValidVault::finalize(
if (assetAvailableDelta != vaultPseudoDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal and "
"assets available must add up";
JLOG(j.fatal())
<< "Invariant failed: withdrawal and assets available must add up";
result = false;
}
@@ -931,10 +941,9 @@ ValidVault::finalize(
if (!(beforeShares && beforeShares->sharesTotal > 0 &&
vaultHoldsNoAssets(beforeVault) && beforeVault.owner == tx[sfAccount]))
{
JLOG(j.fatal()) << //
"Invariant failed: clawback may only be performed "
"by the asset issuer, or by the vault owner of an "
"empty vault";
JLOG(j.fatal()) << "Invariant failed: " << //
"clawback may only be performed by the asset issuer, or by the vault "
"owner of an empty vault";
return false; // That's all we can do
}
}
@@ -942,19 +951,13 @@ ValidVault::finalize(
auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId);
if (maybeVaultDeltaAssets)
{
auto const totalDelta = DeltaInfo::makeDelta(
beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset);
auto const availableDelta = DeltaInfo::makeDelta(
beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset);
auto const minScale =
computeCoarsestScale({*maybeVaultDeltaAssets, totalDelta, availableDelta});
computeVaultMinScale(*maybeVaultDeltaAssets, view.rules());
auto const vaultDeltaAssets =
roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale);
if (vaultDeltaAssets >= kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must decrease vault "
"balance";
JLOG(j.fatal()) << "Invariant failed: clawback must decrease vault balance";
result = false;
}
@@ -963,8 +966,7 @@ ValidVault::finalize(
if (assetsTotalDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback and assets outstanding "
"must add up";
"Invariant failed: clawback and assets outstanding must add up";
result = false;
}
@@ -975,8 +977,7 @@ ValidVault::finalize(
if (assetAvailableDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback and assets available "
"must add up";
"Invariant failed: clawback and assets available must add up";
result = false;
}
}
@@ -998,8 +999,7 @@ ValidVault::finalize(
if (maybeAccountDeltaShares->delta >= kZero)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must decrease holder "
"shares";
"Invariant failed: clawback must decrease holder shares";
result = false;
}
@@ -1014,9 +1014,8 @@ ValidVault::finalize(
if (vaultDeltaShares->delta * -1 != maybeAccountDeltaShares->delta)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must change holder and "
"vault shares by equal amount";
JLOG(j.fatal()) << "Invariant failed: " << //
"clawback must change holder and vault shares by equal amount";
result = false;
}

61
src/libxrpl/tx/transactors/vault/VaultDeposit.cpp
View File

@@ -1,6 +1,7 @@
#include <xrpl/tx/transactors/vault/VaultDeposit.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/Number.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/helpers/CredentialHelpers.h>
@@ -51,9 +52,9 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
return tecNO_ENTRY;
auto const& account = ctx.tx[sfAccount];
auto const assets = ctx.tx[sfAmount];
auto const amount = ctx.tx[sfAmount];
auto const vaultAsset = vault->at(sfAsset);
if (assets.asset() != vaultAsset)
if (amount.asset() != vaultAsset)
return tecWRONG_ASSET;
auto const& vaultAccount = vault->at(sfAccount);
@@ -65,7 +66,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
auto const mptIssuanceID = vault->at(sfShareMPTID);
auto const vaultShare = MPTIssue(mptIssuanceID);
if (vaultShare == assets.asset())
if (vaultShare == amount.asset())
{
// LCOV_EXCL_START
JLOG(ctx.j.error()) << "VaultDeposit: vault shares and assets cannot be same.";
@@ -124,16 +125,33 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
if (auto const ter = requireAuth(ctx.view, vaultAsset, account); !isTesSuccess(ter))
return ter;
if (accountHolds(
ctx.view,
account,
vaultAsset,
FreezeHandling::ZeroIfFrozen,
AuthHandling::ZeroIfUnauthorized,
ctx.j,
SpendableHandling::FullBalance) < assets)
auto const accountBalance = accountHolds(
ctx.view,
account,
vaultAsset,
FreezeHandling::ZeroIfFrozen,
AuthHandling::ZeroIfUnauthorized,
ctx.j,
SpendableHandling::FullBalance);
if (accountBalance < amount)
return tecINSUFFICIENT_FUNDS;
// IOU precision checks
if (ctx.view.rules().enabled(fixCleanup3_2_0) && amount.holds<Issue>())
{
// reject deposits that would canonicalize to a no-op at the depositor's trustline scale.
// Skipped for issuer-as-depositor: accountHolds returns (kMaxValue @ kMaxOffset) which
// would always trip the predicate.
if (account != amount.getIssuer() &&
amount.isZeroAtScale(scale(accountBalance, vaultAsset)))
{
JLOG(ctx.j.warn()) << "VaultDeposit: amount " << amount.getFullText()
<< " rounds to zero at counterparty trust-line scale";
return tecPRECISION_LOSS;
}
}
return tesSUCCESS;
}
@@ -145,7 +163,26 @@ VaultDeposit::doApply()
return tefINTERNAL; // LCOV_EXCL_LINE
auto const vaultAsset = vault->at(sfAsset);
auto const amount = ctx_.tx[sfAmount];
// Post-amendment IOU only: round Downward to the AssetsTotal precision so
// a sub-ULP tail can't be silently absorbed by one rail and not the other.
auto const amount = [&]() -> STAmount {
if (!view().rules().enabled(fixCleanup3_2_0) || !ctx_.tx[sfAmount].holds<Issue>())
return ctx_.tx[sfAmount];
STAmount const amt = ctx_.tx[sfAmount];
return roundToScale(
amt,
scale(vault->at(sfAssetsTotal) + amt, vault->at(sfAsset)),
Number::RoundingMode::Downward);
}();
if (amount == beast::kZero)
{
JLOG(j_.warn()) << "VaultDeposit: amount " << ctx_.tx[sfAmount]
<< " rounds to zero at vault scale";
return tecPRECISION_LOSS;
}
// Make sure the depositor can hold shares.
auto const mptIssuanceID = (*vault)[sfShareMPTID];
auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));

403
src/test/app/Vault_test.cpp
View File

@@ -6857,11 +6857,414 @@ class Vault_test : public beast::unit_test::Suite
}
}
// Bug: DeltaInfo::makeDelta uses max(scale(after), scale(before)) for the
// sfAssetsTotal and sfAssetsAvailable deltas, and visitEntry applies the
// same max() for the vault pseudo-account RippleState. When
// sfAssetsTotal sits exactly at 1e16 (IOU exponent 1, ULP = 10) and a
// withdrawal of 5 USD brings it to 9.999...995e15 (IOU exponent 0,
// ULP = 1), all three computations pick the anterior coarser scale 1.
// roundToAsset(-5, scale=1) collapses to 0, so the invariant check
// vaultPseudoDeltaAssets >= kZero fires even though the state change is
// valid and fully consistent at IOU precision.
//
// Fix (fixCleanup3_2_0): finalize compares the vault pseudo-account and
// sfAssetsTotal/Available deltas directly in Number space, bypassing
// scale-coarsened rounding.
void
testBugMakeDeltaAnteriorScale()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
env.fund(XRP(100'000), issuer, alice);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
// Trust limit of 2e16, fund exactly 1e16 so deposit lands at the
// IOU scale-1 boundary (exponent 1, ULP = 10).
STAmount const fundAndDeposit{usd.raw(), Number{1, 16}};
env(trust(alice, STAmount{usd.raw(), 2, 16}));
env.close();
env(pay(issuer, alice, fundAndDeposit));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
vaultTx[sfScale] = 0;
env(vaultTx);
env.close();
// sfAssetsTotal = sfAssetsAvailable = 1e16 (exponent 1, ULP = 10).
env(vault.deposit(
{.depositor = alice, .id = vaultKeylet.key, .amount = fundAndDeposit}));
env.close();
// Withdraw 5 USD: -5 is sub-ULP at the anterior scale (ULP = 10)
// but exact at the posterior scale (ULP = 1). The state change is
// consistent; only the invariant's scale selection is wrong.
env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(5)}),
Ter(expected));
env.close();
};
{
testcase(
"bug: VaultWithdraw across IOU scale boundary fires invariant "
"(pre-fixCleanup3_2_0)");
runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultWithdraw across IOU scale boundary succeeds "
"(post-fixCleanup3_2_0)");
runScenario(testableAmendments(), tesSUCCESS);
}
}
// Bug: DeltaInfo::makeDelta uses max(scale(after), scale(before)) for
// sfAssetsTotal/Available deltas. This is symmetric to
// testBugMakeDeltaAnteriorScale but in the opposite direction: a deposit
// pushes assetsTotal from just below 1e16 (IOU exponent 0, ULP = 1) to just
// above it (exponent 1, ULP = 10). makeDelta picks the coarser *posterior*
// scale 1. The trust line balance rounds from atEdge + 2 = 10,000,000,000,000,001
// → 1e16, so the pseudo-account delta is only +1 in IOU space.
// roundToAsset(+1, scale=1) = 0 fires "deposit must increase vault balance"
// even though the state change is consistent at every precision boundary.
//
// Fix (fixCleanup3_2_0): computeVaultMinScale uses the posterior Number-space
// scale of sfAssetsTotal (which retains the full value 10,000,000,000,000,001,
// exponent 0), giving minScale = 0. roundToAsset(+1, scale=0) = 1 > 0 and
// the invariant passes. However the transactor's own precision guard fires
// first (bob pays 2 USD, vault receives only 1 due to IOU rounding), so the
// post-amendment result is tecPRECISION_LOSS rather than tesSUCCESS —
// the depositor is protected from silently losing 1 USD to rounding.
void
testBugMakeDeltaPosteriorScale()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100'000), issuer, alice, bob);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
// atEdge is the largest IOU value with exponent 0 (ULP = 1).
// A deposit of 2 USD brings assetsTotal to 10,000,000,000,000,001
// in Number space, crossing the 1e16 boundary in IOU space.
STAmount const atEdge{usd.raw(), Number{9'999'999'999'999'999LL}};
env(trust(alice, STAmount{usd.raw(), 2, 16}));
env(trust(bob, usd(100)));
env.close();
env(pay(issuer, alice, atEdge));
env(pay(issuer, bob, usd(2)));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
vaultTx[sfScale] = 0;
env(vaultTx);
env.close();
// sfAssetsTotal = sfAssetsAvailable = atEdge (exponent 0, ULP = 1)
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = atEdge}));
env.close();
// Deposit 2 USD: +2 is sub-ULP at the posterior IOU scale (ULP = 10)
// but exact at the Number scale retained by sfAssetsTotal.
env(vault.deposit({.depositor = bob, .id = vaultKeylet.key, .amount = usd(2)}),
Ter(expected));
env.close();
};
{
testcase(
"bug: VaultDeposit across IOU scale boundary fires invariant "
"(pre-fixCleanup3_2_0)");
runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultDeposit across IOU scale boundary succeeds "
"(post-fixCleanup3_2_0)");
runScenario(testableAmendments(), tecPRECISION_LOSS);
}
}
// Bug: ValidVault::visitEntry computes destinationDelta.scale as
// max(before_exponent, after_exponent) for RippleState entries. When a
// withdrawal credits a destination whose IOU balance sits just below a
// power-of-10 boundary (atEdge = 9'999'999'999'999'999), the post-credit
// STAmount rounds up one exponent (exponent 0 → 1), making
// destinationDelta.scale = 1. The invariant then calls
// roundToAsset(+2 USD, scale=1) = 0 and incorrectly fires
// "withdrawal must increase destination balance".
//
// Fix (fixCleanup3_2_0): finalize compares destination delta directly in
// Number space, bypassing scale-coarsened rounding. The transaction
// itself succeeds because the effective IOU credit is non-trivial at
// Number precision even though the STAmount exponent shifted.
void
testVaultWithdrawCanonicalizeToZero()
{
using namespace test::jtx;
enum class DestKind : bool { ThirdParty = false, Self = true };
auto runScenario = [this](FeatureBitset features, DestKind destKind, TER expected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100'000), issuer, alice, bob);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
STAmount const aliceLimit{usd.raw(), 2, 16};
STAmount const bobLimit{usd.raw(), 2, 16};
STAmount const atEdge{usd.raw(), Number{9'999'999'999'999'999LL}};
env(trust(alice, aliceLimit));
if (destKind == DestKind::ThirdParty)
env(trust(bob, bobLimit));
env.close();
env(pay(issuer, alice, usd(1'000)));
if (destKind == DestKind::ThirdParty)
env(pay(issuer, bob, atEdge));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
vaultTx[sfScale] = 0;
env(vaultTx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(1'000)}));
env.close();
// For the self-destination case, push alice's own trust line to
// the IOU edge so the next withdraw inflow crosses the boundary.
if (destKind == DestKind::Self)
{
env(pay(issuer, alice, atEdge));
env.close();
}
auto tx = vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(2)});
if (destKind == DestKind::ThirdParty)
tx[sfDestination] = bob.human();
env(tx, Ter(expected));
env.close();
};
{
testcase(
"bug: VaultWithdraw to third-party at IOU edge fires invariant "
"(pre-fixCleanup3_2_0)");
runScenario(
testableAmendments() - fixCleanup3_2_0, DestKind::ThirdParty, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultWithdraw to third-party at IOU edge succeeds "
"(post-fixCleanup3_2_0)");
runScenario(testableAmendments(), DestKind::ThirdParty, tesSUCCESS);
}
{
testcase(
"bug: VaultWithdraw to self at IOU edge fires invariant "
"(pre-fixCleanup3_2_0)");
runScenario(
testableAmendments() - fixCleanup3_2_0, DestKind::Self, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultWithdraw to self at IOU edge succeeds "
"(post-fixCleanup3_2_0)");
runScenario(testableAmendments(), DestKind::Self, tesSUCCESS);
}
}
// Bug: when a depositor's IOU trustline balance is very large (e.g.
// ~1e17), adding a small deposit (e.g. 1 USD) leaves sfAssetsTotal
// unchanged at IOU precision because the increment is sub-ULP at the
// vault's current asset scale. The vault records the deposit, mints
// shares, and decrements the depositor's trustline, but sfAssetsTotal
// does not change — the conservation invariant fires because the rail
// delta is zero.
//
// Two sub-cases are exercised:
// 1. First-ever deposit into an empty vault: the depositor's own
// trustline has a large balance so 1 USD canonicalizes to zero
// when written back through the IOU rail.
// 2. Subsequent deposit after the vault already holds a large
// sfAssetsTotal: a different depositor (bob, with a small balance)
// sends 1 USD, which again rounds to zero at the vault's coarse
// asset scale.
//
// Fix (fixCleanup3_2_0): the deposit transactor checks whether
// roundToAsset(amount, vault_scale) == 0 and rejects early with
// tecPRECISION_LOSS before any state is modified.
void
testVaultDepositCanonicalizeToZero()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100'000), issuer, alice, bob);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
STAmount const trustLimit{usd.raw(), Number{99'999'999'999'999'999LL}};
STAmount const aliceFund{usd.raw(), Number{99'999'999'999'999'999LL}};
env(trust(alice, trustLimit));
env(trust(bob, trustLimit));
env.close();
env(pay(issuer, alice, aliceFund));
env(pay(issuer, bob, usd(1000)));
env.close();
Vault const vault{env};
// Scale=0 so sfAssetsTotal stores whole USD
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
vaultTx[sfScale] = 0;
env(vaultTx);
env.close();
// Alice's deposit canonicalizes to zero at her own trustline scale
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(1)}),
Ter(expected));
// Increase vault-scale
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = aliceFund}));
env.close();
env(vault.deposit({.depositor = bob, .id = vaultKeylet.key, .amount = usd(1)}),
Ter(expected));
env.close();
};
{
testcase(
"bug: VaultDeposit below Vault precision canonicalized to zero "
"(pre-fixCleanup3_2_0)");
runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultDeposit below Vault precision canonicalized to zero "
"(post-fixCleanup3_2_0)");
runScenario(testableAmendments(), tecPRECISION_LOSS);
}
}
// VaultDeposit by issuer with the vault parked at the IOU 16-digit
// edge (9.999e15). Issuer mints 2 more USD; the vault trust line
// goes 9.999e15 → 10^16, gaining 1 unit instead of 2 (canonicalization).
//
// Pre-fixCleanup3_2_0: the proactive check is absent; the deposit
// applies, then VaultInvariant's "deposit must increase vault
// balance" assertion fires at finalize time on the rounded vault
// delta of zero, returning tecINVARIANT_FAILED.
// Post-amendment: reject deposit that is not representable at Vault scale.
void
testBugIssuerVaultDepositAtEdge()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const owner{"owner"};
env.fund(XRP(100'000), issuer, owner);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
STAmount const trustLimit{usd.raw(), 2, 16};
STAmount const ownerFund{usd.raw(), Number{9'999'999'999'999'999LL}};
env(trust(owner, trustLimit));
env.close();
env(pay(issuer, owner, ownerFund));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = usd});
vaultTx[sfScale] = 0;
env(vaultTx);
env.close();
env(vault.deposit({.depositor = owner, .id = vaultKeylet.key, .amount = ownerFund}));
env.close();
// Vault pseudo-account is now at 9.999e15. Issuer mints 2
// more USD. Pre: tecINVARIANT_FAILED at finalize. Post:
// tecPRECISION_LOSS proactively. Either way, no value moves.
env(vault.deposit({.depositor = issuer, .id = vaultKeylet.key, .amount = usd(2)}),
Ter(expected));
env.close();
};
{
testcase(
"bug: VaultDeposit by issuer at IOU edge fires "
"tecINVARIANT_FAILED at finalize (pre-fixCleanup3_2_0)");
runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED);
}
{
testcase(
"bug: VaultDeposit by issuer at IOU edge rejects with "
"tecPRECISION_LOSS proactively (post-fixCleanup3_2_0)");
runScenario(testableAmendments(), tecPRECISION_LOSS);
}
}
public:
void
run() override
{
testVaultDepositNegativeBalanceFromOppositeLimit();
testBugIssuerVaultDepositAtEdge();
testBugMakeDeltaPosteriorScale();
testBugMakeDeltaAnteriorScale();
testVaultDepositCanonicalizeToZero();
testVaultWithdrawCanonicalizeToZero();
testSequences();
testPreflight();
testCreateFailXRP();

64
src/test/protocol/STAmount_test.cpp
View File

@@ -1203,6 +1203,70 @@ public:
}
}
void
testIsZeroAtScale()
{
testcase("isZeroAtScale");
Issue const usd{Currency(0x5553440000000000), AccountID(0x4985601)};
// IOU: 10 IOU — mantissa = kMinValue (10^15), exponent = -14.
// One ULP at this scale is 10^-14; half-ULP is 5*10^-15.
{
STAmount const ref{usd, STAmount::kMinValue, -14};
int const refScale = ref.exponent(); // -14
BEAST_EXPECT(refScale == -14);
// Zero rounds to zero at any scale.
STAmount const iouZero{usd, 0};
BEAST_EXPECT(iouZero.isZeroAtScale(refScale));
// Sub-ULP: 1e-16 IOU (mantissa = kMinValue, exponent = -31).
// Far below half-ULP → rounds to zero.
STAmount const subUlp{usd, STAmount::kMinValue, -31};
BEAST_EXPECT(subUlp.isZeroAtScale(refScale));
// One ULP: 1e-14 IOU (mantissa = kMinValue, exponent = -29).
// Exactly the smallest representable unit at refScale → not zero.
STAmount const oneUlp{usd, STAmount::kMinValue, -29};
BEAST_EXPECT(!oneUlp.isZeroAtScale(refScale));
// The reference value itself: exponent == scale → returned
// unchanged → not zero.
BEAST_EXPECT(!ref.isZeroAtScale(refScale));
// A much larger value: certainly not zero at this scale.
STAmount const large{usd, STAmount::kMinValue, 0}; // 1e15 IOU
BEAST_EXPECT(!large.isZeroAtScale(refScale));
// When scale equals the value's own exponent, roundToScale
// short-circuits and returns the value unchanged.
BEAST_EXPECT(!subUlp.isZeroAtScale(subUlp.exponent()));
BEAST_EXPECT(!oneUlp.isZeroAtScale(oneUlp.exponent()));
}
// XRP is integral — roundToScale short-circuits, value is preserved.
{
STAmount const xrp{XRPAmount{1}};
BEAST_EXPECT(!xrp.isZeroAtScale(-14));
BEAST_EXPECT(!xrp.isZeroAtScale(0));
STAmount const xrpZero{XRPAmount{0}};
BEAST_EXPECT(xrpZero.isZeroAtScale(-14));
}
// MPT is integral — same short-circuit behaviour as XRP.
{
MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))};
STAmount const mptAmt{mpt, 1};
BEAST_EXPECT(!mptAmt.isZeroAtScale(0));
BEAST_EXPECT(!mptAmt.isZeroAtScale(-14));
STAmount const mptZero{mpt, 0};
BEAST_EXPECT(mptZero.isZeroAtScale(0));
}
}
//--------------------------------------------------------------------------
void