mirror of
https://github.com/XRPLF/rippled.git
synced 2026-06-02 08:17:13 +00:00
fix: Fix VaultInvariant and VaultDeposit precision bugs at IOU scale boundaries (#7272)
Co-authored-by: Bart <bthomee@users.noreply.github.com>
This commit is contained in:
@@ -189,7 +189,6 @@ public:
|
||||
/**
|
||||
* Checks if this amount evaluates to zero when constrained to a specific
|
||||
* accounting scale.
|
||||
*
|
||||
* For XRP and MPT `roundToScale` is a no-op, returns true only when the amount itself is zero.
|
||||
* The `scale` argument is ignored in that case.
|
||||
* For IOU, the amount is rounded to the given scale using Number::RoundingMode::ToNearest mode
|
||||
|
||||
@@ -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>
|
||||
@@ -79,16 +81,83 @@ private:
|
||||
std::vector<Shares> beforeMPTs_;
|
||||
std::unordered_map<uint256, DeltaInfo> deltas_;
|
||||
|
||||
/**
|
||||
* @brief Compute the minimum STAmount scale for rounding invariant
|
||||
* calculations.
|
||||
*
|
||||
* Post-amendment (@c fixCleanup3_2_0) this is simply the posterior
|
||||
* @c assetsTotal scale. Pre-amendment it is the coarsest scale across
|
||||
* @p vaultDelta and both asset-field deltas.
|
||||
*
|
||||
* @param vaultDelta Delta of the vault's asset balance for this transaction.
|
||||
* @param rules Active ledger rules (used to check the amendment).
|
||||
* @returns The minimum scale to apply when rounding vault-related amounts.
|
||||
*/
|
||||
[[nodiscard]] std::int32_t
|
||||
computeVaultMinScale(DeltaInfo const& vaultDelta, Rules const& rules) const;
|
||||
|
||||
/**
|
||||
* @brief Return the vault-asset balance-change delta for an account.
|
||||
*
|
||||
* Looks up the ledger-entry delta recorded during @c visitEntry for the
|
||||
* account entry (XRP), trust line (IOU), or MPToken (MPT) that corresponds
|
||||
* to the vault asset held by @p id.
|
||||
*
|
||||
* @param id Account whose asset delta is requested.
|
||||
* @returns The delta, or @c std::nullopt if the entry was not touched.
|
||||
*/
|
||||
[[nodiscard]] std::optional<DeltaInfo>
|
||||
deltaAssets(AccountID const& id) const;
|
||||
|
||||
/**
|
||||
* @brief Return the vault-asset delta for the transaction's sending
|
||||
* account, adjusted for the fee.
|
||||
*
|
||||
* Calls @c deltaAssets for @c tx[sfAccount] and, for non-delegated XRP
|
||||
* transactions, adds the consumed fee back so the invariant sees the net
|
||||
* asset movement rather than the fee-reduced balance change.
|
||||
*
|
||||
* @param tx The transaction being applied.
|
||||
* @param fee Fee charged by this transaction.
|
||||
* @returns The fee-adjusted delta, or @c std::nullopt if the net delta is
|
||||
* zero or the account entry was not touched.
|
||||
*/
|
||||
[[nodiscard]] std::optional<DeltaInfo>
|
||||
deltaAssetsTxAccount(STTx const& tx, XRPAmount fee) const;
|
||||
|
||||
/**
|
||||
* @brief Return the vault-share balance-change delta for an account.
|
||||
*
|
||||
* For the vault's pseudo-account the @c MPTokenIssuance outstanding-amount
|
||||
* delta is returned; for all other accounts the @c MPToken delta is
|
||||
* returned.
|
||||
*
|
||||
* @param id Account whose share delta is requested.
|
||||
* @returns The delta, or @c std::nullopt if the entry was not touched.
|
||||
*/
|
||||
[[nodiscard]] std::optional<DeltaInfo>
|
||||
deltaShares(AccountID const& id) const;
|
||||
|
||||
/**
|
||||
* @brief Check whether a vault holds no assets.
|
||||
*
|
||||
* @param vault Snapshot of the vault to test.
|
||||
* @returns @c true when both @c assetsAvailable and @c assetsTotal are
|
||||
* zero.
|
||||
*/
|
||||
[[nodiscard]] static bool
|
||||
isVaultEmpty(Vault const& vault);
|
||||
|
||||
public:
|
||||
// Compute the coarsest scale required to represent all numbers
|
||||
[[nodiscard]] static std::int32_t
|
||||
computeCoarsestScale(std::vector<DeltaInfo> const& numbers);
|
||||
|
||||
void
|
||||
visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&);
|
||||
|
||||
bool
|
||||
finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
|
||||
|
||||
// Compute the coarsest scale required to represent all numbers
|
||||
[[nodiscard]] static std::int32_t
|
||||
computeCoarsestScale(std::vector<DeltaInfo> const& numbers);
|
||||
};
|
||||
|
||||
} // namespace xrpl
|
||||
|
||||
@@ -186,6 +186,101 @@ ValidVault::visitEntry(
|
||||
}
|
||||
}
|
||||
|
||||
std::optional<ValidVault::DeltaInfo>
|
||||
ValidVault::deltaAssets(AccountID const& id) const
|
||||
{
|
||||
auto const& vaultAsset = afterVault_[0].asset;
|
||||
auto const lookup = [&](uint256 const& key) -> std::optional<DeltaInfo> {
|
||||
auto const it = deltas_.find(key);
|
||||
if (it == deltas_.end())
|
||||
return std::nullopt;
|
||||
return it->second;
|
||||
};
|
||||
|
||||
return std::visit(
|
||||
[&]<typename TIss>(TIss const& issue) -> std::optional<DeltaInfo> {
|
||||
if constexpr (std::is_same_v<TIss, Issue>)
|
||||
{
|
||||
if (isXRP(issue))
|
||||
return lookup(keylet::account(id).key);
|
||||
auto result = lookup(keylet::line(id, issue).key);
|
||||
// Trust-line balance is stored from the low-account's perspective;
|
||||
// negate if id is the high account so the delta is in id's terms.
|
||||
if (result && id > issue.getIssuer())
|
||||
result->delta = -result->delta;
|
||||
return result;
|
||||
}
|
||||
else if constexpr (std::is_same_v<TIss, MPTIssue>)
|
||||
{
|
||||
return lookup(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::isVaultEmpty(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))
|
||||
{
|
||||
NumberRoundModeGuard const roundGuard(Number::RoundingMode::ToNearest);
|
||||
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 +540,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 +669,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 +701,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 +716,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 +737,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 +776,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 +785,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 +796,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 +829,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 +840,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;
|
||||
}
|
||||
|
||||
@@ -835,63 +849,83 @@ ValidVault::finalize(
|
||||
|
||||
// the scale of destinationDelta can be coarser than
|
||||
// minScale, so we take that into account when rounding
|
||||
auto const localMinScale =
|
||||
std::max(minScale, computeCoarsestScale({destinationDelta}));
|
||||
auto const destinationScale = computeCoarsestScale({destinationDelta});
|
||||
auto const localMinScale = std::max(minScale, destinationScale);
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
auto const localPseudoDeltaAssets =
|
||||
roundToAsset(vaultAsset, vaultPseudoDeltaAssets, localMinScale);
|
||||
if (localPseudoDeltaAssets * -1 != roundedDestinationDelta)
|
||||
// For IOU assets near a precision boundary the destination's STAmount
|
||||
// exponent can shift, making part of the sent value unrepresentable at the
|
||||
// receiver's new scale — that portion is irreversibly absorbed by the IOU
|
||||
// rail. Tolerate the mismatch only when the destroyed amount (vault outflow
|
||||
// minus destination inflow, in Number space) is itself sub-ULP at the
|
||||
// destination's scale. Floor rounding is used so that values exactly at the
|
||||
// step boundary are not mistakenly dismissed. Any representable discrepancy
|
||||
// indicates a real accounting bug and must be caught.
|
||||
auto const destroyedIsSubUlp = tolerateZeroDelta &&
|
||||
roundToAsset(
|
||||
vaultAsset,
|
||||
maybeVaultDeltaAssets->delta * -1 - destinationDelta.delta,
|
||||
destinationScale,
|
||||
Number::RoundingMode::Downward) == kZero;
|
||||
if (!destroyedIsSubUlp &&
|
||||
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 +934,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 +944,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;
|
||||
}
|
||||
|
||||
@@ -929,12 +963,11 @@ ValidVault::finalize(
|
||||
// The owner can use clawback to force-burn shares when the
|
||||
// vault is empty but there are outstanding shares
|
||||
if (!(beforeShares && beforeShares->sharesTotal > 0 &&
|
||||
vaultHoldsNoAssets(beforeVault) && beforeVault.owner == tx[sfAccount]))
|
||||
isVaultEmpty(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 +975,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 +990,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,12 +1001,11 @@ 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;
|
||||
}
|
||||
}
|
||||
else if (!vaultHoldsNoAssets(beforeVault))
|
||||
else if (!isVaultEmpty(beforeVault))
|
||||
{
|
||||
JLOG(j.fatal()) << //
|
||||
"Invariant failed: clawback must change vault balance";
|
||||
@@ -998,8 +1023,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 +1038,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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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>
|
||||
@@ -13,6 +14,7 @@
|
||||
#include <xrpl/protocol/LedgerFormats.h>
|
||||
#include <xrpl/protocol/MPTIssue.h>
|
||||
#include <xrpl/protocol/SField.h>
|
||||
#include <xrpl/protocol/STAmount.h>
|
||||
#include <xrpl/protocol/STLedgerEntry.h>
|
||||
#include <xrpl/protocol/STNumber.h> // IWYU pragma: keep
|
||||
#include <xrpl/protocol/STTakesAsset.h>
|
||||
@@ -26,6 +28,24 @@
|
||||
|
||||
namespace xrpl {
|
||||
|
||||
[[nodiscard]]
|
||||
static STAmount
|
||||
roundToVaultScale(STAmount const& amount, SLE::const_ref vault)
|
||||
{
|
||||
XRPL_ASSERT(vault && vault->getType() == ltVAULT, "xrpl::roundToVaultScale : valid vault sle");
|
||||
XRPL_ASSERT(
|
||||
amount.asset() == vault->at(sfAsset), "xrpl::roundToVaultScale : valid vault asset");
|
||||
|
||||
if (amount.integral())
|
||||
return amount;
|
||||
|
||||
int const postScale = [&]() {
|
||||
NumberRoundModeGuard const rg(Number::RoundingMode::ToNearest);
|
||||
return scale(vault->at(sfAssetsTotal) + amount, vault->at(sfAsset));
|
||||
}();
|
||||
return roundToScale(amount, postScale, Number::RoundingMode::Downward);
|
||||
}
|
||||
|
||||
NotTEC
|
||||
VaultDeposit::preflight(PreflightContext const& ctx)
|
||||
{
|
||||
@@ -49,9 +69,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);
|
||||
@@ -63,7 +83,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.";
|
||||
@@ -122,28 +142,69 @@ 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)
|
||||
bool const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0);
|
||||
auto const roundedAmount = fix320Enabled ? roundToVaultScale(amount, vault) : amount;
|
||||
|
||||
if (fix320Enabled && roundedAmount == beast::kZero)
|
||||
{
|
||||
JLOG(ctx.j.warn()) << "VaultDeposit: deposit amount: " << ctx.tx[sfAmount]
|
||||
<< " is zero at vault scale";
|
||||
return tecPRECISION_LOSS;
|
||||
}
|
||||
|
||||
auto const accountBalance = accountHolds(
|
||||
ctx.view,
|
||||
account,
|
||||
vaultAsset,
|
||||
FreezeHandling::ZeroIfFrozen,
|
||||
AuthHandling::ZeroIfUnauthorized,
|
||||
ctx.j,
|
||||
SpendableHandling::FullBalance);
|
||||
|
||||
if (accountBalance < roundedAmount)
|
||||
return tecINSUFFICIENT_FUNDS;
|
||||
|
||||
// IOU precision checks
|
||||
if (fix320Enabled && !roundedAmount.integral())
|
||||
{
|
||||
// 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;
|
||||
}
|
||||
|
||||
TER
|
||||
VaultDeposit::doApply()
|
||||
{
|
||||
bool const fix320Enabled = view().rules().enabled(fixCleanup3_2_0);
|
||||
auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID]));
|
||||
if (!vault)
|
||||
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 =
|
||||
fix320Enabled ? roundToVaultScale(ctx_.tx[sfAmount], vault) : ctx_.tx[sfAmount];
|
||||
|
||||
// We validated zero-amount in preclaim, if we ended up with zero now, fail hard.
|
||||
if (amount == beast::kZero)
|
||||
{
|
||||
// LCOV_EXCL_START
|
||||
JLOG(j_.error()) << "VaultDeposit: deposit amount: " << ctx_.tx[sfAmount] << " is zero";
|
||||
return tecINTERNAL;
|
||||
// LCOV_EXCL_STOP
|
||||
}
|
||||
|
||||
// Make sure the depositor can hold shares.
|
||||
auto const mptIssuanceID = (*vault)[sfShareMPTID];
|
||||
auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
|
||||
@@ -259,7 +320,7 @@ VaultDeposit::doApply()
|
||||
// trust line into debt the exact case preclaim authorizes via SpendableHandling::FullBalance.
|
||||
// The check thus converts a preclaim- authorized deposit into tefINTERNAL after the asset
|
||||
// transfer.
|
||||
if (!view().rules().enabled(fixCleanup3_2_0))
|
||||
if (!fix320Enabled)
|
||||
{
|
||||
// Sanity check
|
||||
if (accountHolds(
|
||||
|
||||
@@ -6457,6 +6457,489 @@ class Vault_test : public beast::unit_test::Suite
|
||||
runTest(amendments);
|
||||
}
|
||||
|
||||
// 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: the equality check (vault outflow == destination inflow) was
|
||||
// skipped whenever the destination delta rounded to zero at localMinScale,
|
||||
// including cases where the vault outflow rounded to a non-zero value and
|
||||
// a representable amount of value was genuinely destroyed.
|
||||
//
|
||||
// Scenario: Bob's IOU balance sits 5 units below the 10^16 STAmount
|
||||
// precision boundary (atEdge2 = 9,999,999,999,999,995). A withdrawal of
|
||||
// 6 USD shifts his balance across that boundary: the exponent increments
|
||||
// (0 → 1), so his effective inflow in Number space is only +5 — 1 USD is
|
||||
// consumed by the precision-boundary rounding and cannot be credited.
|
||||
//
|
||||
// The destroyed amount (1 USD) is sub-ULP at destinationScale=1 (step=10),
|
||||
// so the check treats it as an unavoidable IOU-precision artefact and
|
||||
// lets the transaction succeed.
|
||||
//
|
||||
// Contrast: if 15 USD were destroyed at the same scale (destroyed ≥ step),
|
||||
// floor(15/10)=1 ≠ 0 and the invariant would fire — that discrepancy IS
|
||||
// representable and indicates a real accounting bug.
|
||||
//
|
||||
// Pre-fixCleanup3_2_0: the "must increase destination balance" check fires
|
||||
// because roundedDestinationDelta = 0 ≤ 0.
|
||||
void
|
||||
testVaultWithdrawEqualityEnforced()
|
||||
{
|
||||
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 aliceLimit{usd.raw(), 2, 16};
|
||||
STAmount const bobLimit{usd.raw(), 2, 16};
|
||||
// Bob's balance sits 5 units below the 10^16 STAmount precision
|
||||
// boundary. Receiving 6 USD shifts his exponent 0 → 1; the
|
||||
// STAmount records +5, not +6 (1 USD is lost to rounding).
|
||||
STAmount const atEdge2{usd.raw(), Number{9'999'999'999'999'995LL}};
|
||||
|
||||
env(trust(alice, aliceLimit));
|
||||
env(trust(bob, bobLimit));
|
||||
env.close();
|
||||
|
||||
env(pay(issuer, alice, usd(1'000)));
|
||||
env(pay(issuer, bob, atEdge2));
|
||||
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();
|
||||
|
||||
// Withdraw 6 USD to Bob: vault loses 6, Bob gains only 5.
|
||||
// Destroyed amount = 1 USD, which is sub-ULP at destinationScale=1.
|
||||
auto tx = vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(6)});
|
||||
tx[sfDestination] = bob.human();
|
||||
env(tx, Ter(expected));
|
||||
env.close();
|
||||
};
|
||||
|
||||
{
|
||||
testcase(
|
||||
"bug: VaultWithdraw to destination at IOU precision boundary fires "
|
||||
"invariant (pre-fixCleanup3_2_0)");
|
||||
runScenario(testableAmendments() - fixCleanup3_2_0, tecINVARIANT_FAILED);
|
||||
}
|
||||
{
|
||||
testcase(
|
||||
"bug: VaultWithdraw to destination at IOU precision boundary succeeds "
|
||||
"when destroyed amount is sub-ULP (post-fixCleanup3_2_0)");
|
||||
runScenario(testableAmendments(), 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);
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testReferenceHolding()
|
||||
{
|
||||
@@ -6940,6 +7423,12 @@ public:
|
||||
void
|
||||
run() override
|
||||
{
|
||||
testVaultWithdrawEqualityEnforced();
|
||||
testBugIssuerVaultDepositAtEdge();
|
||||
testBugMakeDeltaPosteriorScale();
|
||||
testBugMakeDeltaAnteriorScale();
|
||||
testVaultDepositCanonicalizeToZero();
|
||||
testVaultWithdrawCanonicalizeToZero();
|
||||
testVaultDepositNegativeBalanceFromOppositeLimit();
|
||||
testSequences();
|
||||
testPreflight();
|
||||
|
||||
@@ -1203,8 +1203,6 @@ public:
|
||||
}
|
||||
}
|
||||
|
||||
//--------------------------------------------------------------------------
|
||||
|
||||
void
|
||||
testIsZeroAtScale()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user