From 65724aeac135ff002aaefd5f80ce3f429cbb9d83 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:39:12 +0200 Subject: [PATCH] feat: Move VaultSet invariants from ValidVault to VaultSet --- .../xrpl/tx/invariants/VaultInvariantData.h | 62 +++++- include/xrpl/tx/transactors/vault/VaultSet.h | 4 + src/libxrpl/tx/invariants/VaultInvariant.cpp | 54 +----- .../tx/invariants/VaultInvariantData.cpp | 177 +++++++++++++++++- src/libxrpl/tx/transactors/vault/VaultSet.cpp | 94 +++++++++- 5 files changed, 330 insertions(+), 61 deletions(-) diff --git a/include/xrpl/tx/invariants/VaultInvariantData.h b/include/xrpl/tx/invariants/VaultInvariantData.h index e20456f74b..7c6873b836 100644 --- a/include/xrpl/tx/invariants/VaultInvariantData.h +++ b/include/xrpl/tx/invariants/VaultInvariantData.h @@ -7,7 +7,9 @@ #include #include +#include #include +#include #include namespace xrpl { @@ -15,9 +17,8 @@ namespace xrpl { /** * @brief Collects vault and share-issuance snapshots from ledger entry visits. * - * Used by per-transaction invariant checks (e.g. VaultCreate) that need - * vault and MPTokenIssuance state without the full balance-delta tracking - * that ValidVault maintains. + * Used by per-transaction invariant checks (e.g. VaultCreate, VaultSet) that + * need vault and MPTokenIssuance state, optionally with balance-delta tracking. */ class VaultInvariantData { @@ -48,6 +49,24 @@ public: make(SLE const&); }; + /** + * @brief Balance-change delta for a single ledger entry. + * + * Mirrors ValidVault::DeltaInfo. @c scale carries the STAmount exponent + * so that callers can round to the coarsest representable precision. + */ + struct DeltaInfo final + { + Number delta = kNumZero; + std::optional scale; + + /** + * @brief Compute the delta between two Numbers at the coarsest scale. + */ + [[nodiscard]] static DeltaInfo + makeDelta(Number const& before, Number const& after, Asset const& asset); + }; + void visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after); @@ -67,10 +86,47 @@ public: [[nodiscard]] std::optional findShares(uint192 const& mptID) const; + /** + * @brief Find shares in beforeMPTs_ whose mptID matches (deleted entries). + */ + [[nodiscard]] std::optional + findDeletedShares(uint192 const& mptID) const; + + /** + * @brief Access the raw vector of before-state MPTokenIssuance snapshots. + */ + [[nodiscard]] std::vector const& + beforeMPTIssuances() const + { + return beforeMPTs_; + } + + /** + * @brief Return the vault-asset balance-change delta for an account. + * + * Looks up the ledger-entry delta recorded during visitEntry for the + * account entry (XRP), trust line (IOU), or MPToken (MPT) that corresponds + * to the vault asset held by @p id. + * + * @param vaultAsset The asset held by the vault. + * @param id Account whose asset delta is requested. + * @returns The delta, or std::nullopt if the entry was not touched. + */ + [[nodiscard]] std::optional + deltaAssets(Asset const& vaultAsset, AccountID const& id) const; + + /** + * @brief Compute the coarsest scale required to represent all numbers. + */ + [[nodiscard]] static std::int32_t + computeCoarsestScale(std::vector const& numbers); + private: std::vector afterVault_; std::vector beforeVault_; std::vector afterMPTs_; + std::vector beforeMPTs_; + std::unordered_map deltas_; }; } // namespace xrpl diff --git a/include/xrpl/tx/transactors/vault/VaultSet.h b/include/xrpl/tx/transactors/vault/VaultSet.h index 5c362d5db4..0e542ddac9 100644 --- a/include/xrpl/tx/transactors/vault/VaultSet.h +++ b/include/xrpl/tx/transactors/vault/VaultSet.h @@ -1,6 +1,7 @@ #pragma once #include +#include namespace xrpl { @@ -35,6 +36,9 @@ public: XRPAmount fee, ReadView const& view, beast::Journal const& j) override; + +private: + VaultInvariantData data_; }; } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index af1c722254..1208d4f967 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -545,63 +544,14 @@ ValidVault::finalize( switch (txnType) { case ttVAULT_CREATE: + case ttVAULT_SET: case ttLOAN_SET: case ttLOAN_MANAGE: case ttLOAN_PAY: // Create-specific checks live in VaultCreate::finalizeInvariants. + // Set-specific checks live in VaultSet::finalizeInvariants. // Loan checks are TBD. return true; - case ttVAULT_SET: { - bool result = true; - - XRPL_ASSERT( - !beforeVault_.empty(), "xrpl::ValidVault::finalize : set updated a vault"); - auto const& beforeVault = beforeVault_[0]; - - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (vaultDeltaAssets) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change vault balance"; - result = false; - } - - if (beforeVault.assetsTotal != afterVault.assetsTotal) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change assets " - "outstanding"; - result = false; - } - - if (afterVault.assetsMaximum > kZero && - afterVault.assetsTotal > afterVault.assetsMaximum) - { - JLOG(j.fatal()) << // - "Invariant failed: set assets outstanding must not " - "exceed assets maximum"; - result = false; - } - - if (beforeVault.assetsAvailable != afterVault.assetsAvailable) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change assets " - "available"; - result = false; - } - - if (beforeShares && updatedShares && - beforeShares->sharesTotal != updatedShares->sharesTotal) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change shares " - "outstanding"; - result = false; - } - - return result; - } case ttVAULT_DEPOSIT: { bool result = true; diff --git a/src/libxrpl/tx/invariants/VaultInvariantData.cpp b/src/libxrpl/tx/invariants/VaultInvariantData.cpp index ddc2807579..c524cf78d1 100644 --- a/src/libxrpl/tx/invariants/VaultInvariantData.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariantData.cpp @@ -1,12 +1,22 @@ #include +#include #include #include +#include #include #include #include +#include +#include #include // IWYU pragma: keep +#include +#include +#include +#include +#include + namespace xrpl { VaultInvariantData::Vault @@ -48,8 +58,57 @@ VaultInvariantData::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ after != nullptr && (before != nullptr || !isDelete), "xrpl::VaultInvariantData::visitEntry : some object is available"); - if (before && before->getType() == ltVAULT) - beforeVault_.push_back(Vault::make(*before)); + // Number balanceDelta will capture the difference (delta) between "before" + // state (zero if created) and "after" state (zero if destroyed), and + // preserves value scale (exponent) to round values to the same scale during + // validation. It is used to validate that the change in account + // balances matches the change in vault balances, stored to deltas_ at the + // end of this function. + DeltaInfo balanceDelta{.delta = kNumZero, .scale = std::nullopt}; + + std::int8_t sign = 0; + if (before) + { + switch (before->getType()) + { + case ltVAULT: + beforeVault_.push_back(Vault::make(*before)); + break; + case ltMPTOKEN_ISSUANCE: + // At this moment we have no way of telling if this object holds + // vault shares or something else. Save it for finalize. + beforeMPTs_.push_back(Shares::make(*before)); + balanceDelta.delta = + static_cast(before->getFieldU64(sfOutstandingAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; + sign = 1; + break; + case ltMPTOKEN: + balanceDelta.delta = static_cast(before->getFieldU64(sfMPTAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltACCOUNT_ROOT: + balanceDelta.delta = before->getFieldAmount(sfBalance); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltRIPPLE_STATE: { + auto const amount = before->getFieldAmount(sfBalance); + balanceDelta.delta = amount; + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } + default:; + } + } if (!isDelete && after) { @@ -59,11 +118,56 @@ VaultInvariantData::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ afterVault_.push_back(Vault::make(*after)); break; case ltMPTOKEN_ISSUANCE: + // At this moment we have no way of telling if this object holds + // vault shares or something else. Save it for finalize. afterMPTs_.push_back(Shares::make(*after)); + balanceDelta.delta -= + Number(static_cast(after->getFieldU64(sfOutstandingAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; + sign = 1; break; + case ltMPTOKEN: + balanceDelta.delta -= + Number(static_cast(after->getFieldU64(sfMPTAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltACCOUNT_ROOT: + balanceDelta.delta -= Number(after->getFieldAmount(sfBalance)); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltRIPPLE_STATE: { + auto const amount = after->getFieldAmount(sfBalance); + balanceDelta.delta -= Number(amount); + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + if (amount.exponent() > balanceDelta.scale) + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } + + uint256 const key = (before ? before->key() : after->key()); + // Append to deltas if sign is non-zero, i.e. an object of an interesting + // type has been updated. A transaction may update an object even when + // its balance has not changed, e.g. transaction fee equals the amount + // transferred to the account. We intentionally do not compare balanceDelta + // against zero, to avoid missing such updates. + if (sign != 0) + { + XRPL_ASSERT_PARTS( + balanceDelta.scale, "xrpl::VaultInvariantData::visitEntry", "scale initialized"); + balanceDelta.delta *= sign; + deltas_[key] = balanceDelta; + } } std::optional @@ -77,4 +181,73 @@ VaultInvariantData::findShares(uint192 const& mptID) const return std::nullopt; } +std::optional +VaultInvariantData::findDeletedShares(uint192 const& mptID) const +{ + for (auto const& s : beforeMPTs_) + { + if (s.share.getMptID() == mptID) + return s; + } + return std::nullopt; +} + +std::optional +VaultInvariantData::deltaAssets(Asset const& vaultAsset, AccountID const& id) const +{ + auto const lookup = [&](uint256 const& key) -> std::optional { + auto const it = deltas_.find(key); + if (it == deltas_.end()) + return std::nullopt; + return it->second; + }; + + return std::visit( + [&](TIss const& issue) -> std::optional { + if constexpr (std::is_same_v) + { + 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) + { + return lookup(keylet::mptoken(issue.getMptID(), id).key); + } + }, + vaultAsset.value()); +} + +[[nodiscard]] VaultInvariantData::DeltaInfo +VaultInvariantData::DeltaInfo::makeDelta( + Number const& before, + Number const& after, + Asset const& asset) +{ + return { + .delta = after - before, + .scale = std::max(xrpl::scale(after, asset), xrpl::scale(before, asset))}; +} + +[[nodiscard]] std::int32_t +VaultInvariantData::computeCoarsestScale(std::vector const& numbers) +{ + if (numbers.empty()) + return 0; + + auto const max = std::ranges::max_element( + numbers, [](auto const& a, auto const& b) -> bool { return a.scale < b.scale; }); + XRPL_ASSERT_PARTS( + max->scale, + "xrpl::VaultInvariantData::computeCoarsestScale", + "scale set for destinationDelta"); + return max->scale.value_or(STAmount::kMaxOffset); +} + } // namespace xrpl diff --git a/src/libxrpl/tx/transactors/vault/VaultSet.cpp b/src/libxrpl/tx/transactors/vault/VaultSet.cpp index 6d0ade6e52..93de364ec7 100644 --- a/src/libxrpl/tx/transactors/vault/VaultSet.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultSet.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include namespace xrpl { @@ -178,15 +180,99 @@ VaultSet::doApply() } void -VaultSet::visitInvariantEntry(bool, SLE::const_ref, SLE::const_ref) +VaultSet::visitInvariantEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { - // No transaction-specific invariants yet (future work). + data_.visitEntry(isDelete, before, after); } bool -VaultSet::finalizeInvariants(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) +VaultSet::finalizeInvariants( + STTx const&, + TER result, + XRPAmount, + ReadView const& view, + beast::Journal const& j) { - // No transaction-specific invariants yet (future work). + bool const enforce = view.rules().enabled(featureSingleAssetVault); + + if (!isTesSuccess(result)) + return true; + + auto const& afterVaults = data_.afterVaults(); + if (afterVaults.empty()) + return true; // Let ValidVault catch missing vault + + auto const& afterVault = afterVaults[0]; + auto const& beforeVaults = data_.beforeVaults(); + // VaultSet always modifies an existing vault; beforeVaults must be non-empty + if (beforeVaults.empty()) + return true; // Let ValidVault catch this + + auto const& beforeVault = beforeVaults[0]; + + // The MPTokenIssuance may not be in the modified set (e.g. only the vault + // was touched), so fall back to a view read if needed. + auto const updatedShares = [&]() -> std::optional { + if (auto found = data_.findShares(afterVault.shareMPTID)) + return found; + auto const sleShares = view.read(keylet::mptIssuance(afterVault.shareMPTID)); + return sleShares ? std::optional( + VaultInvariantData::Shares::make(*sleShares)) + : std::nullopt; + }(); + + auto const beforeShares = data_.findDeletedShares(afterVault.shareMPTID); + + static constexpr Number kZero{}; + bool checkResult = true; + + // 1. VaultSet must not change the vault's asset balance + auto const vaultDeltaAssets = data_.deltaAssets(afterVault.asset, afterVault.pseudoId); + if (vaultDeltaAssets) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change vault balance"; + checkResult = false; + } + + // 2. VaultSet must not change assets outstanding + if (beforeVault.assetsTotal != afterVault.assetsTotal) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change assets outstanding"; + checkResult = false; + } + + // 3. After set, assets outstanding must not exceed the new maximum + if (afterVault.assetsMaximum > kZero && afterVault.assetsTotal > afterVault.assetsMaximum) + { + JLOG(j.fatal()) << // + "Invariant failed: set assets outstanding must not exceed assets maximum"; + checkResult = false; + } + + // 4. VaultSet must not change assets available + if (beforeVault.assetsAvailable != afterVault.assetsAvailable) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change assets available"; + checkResult = false; + } + + // 5. VaultSet must not change shares outstanding + if (beforeShares && updatedShares && beforeShares->sharesTotal != updatedShares->sharesTotal) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change shares outstanding"; + checkResult = false; + } + + if (!checkResult) + { + XRPL_ASSERT(enforce, "xrpl::VaultSet::finalizeInvariants : vault set invariants"); + return !enforce; + } + return true; }