From fcbc2f0a66a644b4987dc416e3dc78a5f7e35a03 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 19 May 2026 19:45:19 +0200 Subject: [PATCH] feat: Propagate underlying MPT flags to vault shares (#7077) --- include/xrpl/ledger/View.h | 2 +- include/xrpl/ledger/helpers/MPTokenHelpers.h | 70 +- .../xrpl/ledger/helpers/RippleStateHelpers.h | 4 +- include/xrpl/ledger/helpers/TokenHelpers.h | 33 +- .../xrpl/protocol/detail/ledger_entries.macro | 1 + include/xrpl/protocol/detail/sfields.macro | 1 + .../ledger_entries/MPTokenIssuance.h | 35 + include/xrpl/tx/invariants/MPTInvariant.h | 13 + .../transactors/token/MPTokenIssuanceCreate.h | 23 +- src/libxrpl/ledger/View.cpp | 31 +- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 153 +++- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 28 +- src/libxrpl/tx/invariants/MPTInvariant.cpp | 88 +++ .../lending/LoanBrokerCoverWithdraw.cpp | 10 +- .../token/MPTokenIssuanceCreate.cpp | 22 + .../tx/transactors/vault/VaultCreate.cpp | 26 +- .../tx/transactors/vault/VaultWithdraw.cpp | 9 +- src/test/app/Invariants_test.cpp | 108 +++ src/test/app/Loan_test.cpp | 221 ++++-- src/test/app/MPToken_test.cpp | 17 + src/test/app/Vault_test.cpp | 680 +++++++++++++++++- .../ledger_entries/MPTokenIssuanceTests.cpp | 27 + 22 files changed, 1475 insertions(+), 127 deletions(-) diff --git a/include/xrpl/ledger/View.h b/include/xrpl/ledger/View.h index 4958a89d8c..0d76c98a73 100644 --- a/include/xrpl/ledger/View.h +++ b/include/xrpl/ledger/View.h @@ -57,7 +57,7 @@ isVaultPseudoAccountFrozen( ReadView const& view, AccountID const& account, MPTIssue const& mptShare, - int depth); + std::uint8_t depth); [[nodiscard]] bool isLPTokenFrozen( diff --git a/include/xrpl/ledger/helpers/MPTokenHelpers.h b/include/xrpl/ledger/helpers/MPTokenHelpers.h index 6544b18dd1..491bd1933e 100644 --- a/include/xrpl/ledger/helpers/MPTokenHelpers.h +++ b/include/xrpl/ledger/helpers/MPTokenHelpers.h @@ -27,14 +27,18 @@ isGlobalFrozen(ReadView const& view, MPTIssue const& mptIssue); isIndividualFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue); [[nodiscard]] bool -isFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue, int depth = 0); +isFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue, + std::uint8_t depth = 0); [[nodiscard]] bool isAnyFrozen( ReadView const& view, std::initializer_list const& accounts, MPTIssue const& mptIssue, - int depth = 0); + std::uint8_t depth = 0); //------------------------------------------------------------------------------ // @@ -88,7 +92,7 @@ requireAuth( MPTIssue const& mptIssue, AccountID const& account, AuthType authType = AuthType::Legacy, - int depth = 0); + std::uint8_t depth = 0); /** Enforce account has MPToken to match its authorization. * @@ -104,22 +108,68 @@ enforceMPTokenAuthorization( XRPAmount const& priorBalance, beast::Journal j); -/** Check if the destination account is allowed - * to receive MPT. Return tecNO_AUTH if it doesn't - * and tesSUCCESS otherwise. +/** Resolve the underlying asset of a vault share. + * + * Reads sfReferenceHolding from @p sleShareIssuance to determine which + * asset the vault wraps. @p sleHolding must be the SLE that + * sfReferenceHolding points to — either an ltMPTOKEN (returns its + * MPTIssue) or an ltRIPPLE_STATE (returns its low/high Issue). + * + * @pre Both SLEs must exist and @p sleHolding must be of type ltMPTOKEN + * or ltRIPPLE_STATE. Passing any other type is undefined behaviour. + * @param sleShareIssuance MPTokenIssuance SLE for the vault share token. + * @param sleHolding SLE referenced by sfReferenceHolding. + * @return The underlying Asset (MPTIssue or Issue). + */ +[[nodiscard]] Asset +assetOfHolding(SLE const& sleShareIssuance, SLE const& sleHolding); + +/** Check whether @p to may receive the given MPT from @p from. + * + * The check passes when any of the following is true: + * - @p waive is WaiveMPTCanTransfer::Yes (recovery-path exemption), or + * - @p from or @p to is the issuer, or + * - lsfMPTCanTransfer is set on the MPTokenIssuance. + * + * For vault shares (MPTokenIssuances that carry sfReferenceHolding) the + * check recurses into the underlying asset's transferability. This + * recursion is defensive; vault-of-vault-shares is rejected at vault + * creation, so in practice depth never exceeds 1. + * + * @param view Ledger state to read from. + * @param mptIssue The MPT issuance being transferred. + * @param from Sending account. + * @param to Receiving account. + * @param waive WaiveMPTCanTransfer::Yes skips the lsfMPTCanTransfer + * check. Use for recovery paths (e.g. unwinding SAV or + * Lending Protocol positions after an issuer revokes + * transferability). + * @param depth Recursion depth; bounded at kMaxAssetCheckDepth. + * @return tesSUCCESS if the transfer is allowed, tecNO_AUTH otherwise. */ [[nodiscard]] TER canTransfer( ReadView const& view, MPTIssue const& mptIssue, AccountID const& from, - AccountID const& to); + AccountID const& to, + WaiveMPTCanTransfer waive = WaiveMPTCanTransfer::No, + std::uint8_t depth = 0); -/** Check if Asset can be traded on DEX. return tecNO_PERMISSION - * if it doesn't and tesSUCCESS otherwise. +/** Check whether @p asset may be traded on the DEX. + * + * For IOU assets the check delegates to the existing offer/AMM freeze + * logic. For MPT assets it checks lsfMPTCanTrade on the MPTokenIssuance. + * Vault shares recurse into the underlying asset's tradability via + * sfReferenceHolding; depth is bounded at kMaxAssetCheckDepth. + * + * @param view Ledger state to read from. + * @param asset The asset to check. + * @param depth Recursion depth; bounded at kMaxAssetCheckDepth. + * @return tesSUCCESS if trading is allowed, tecNO_PERMISSION otherwise. */ [[nodiscard]] TER -canTrade(ReadView const& view, Asset const& asset); +canTrade(ReadView const& view, Asset const& asset, std::uint8_t depth = 0); //------------------------------------------------------------------------------ // diff --git a/include/xrpl/ledger/helpers/RippleStateHelpers.h b/include/xrpl/ledger/helpers/RippleStateHelpers.h index 17b0f7673e..2616a6d5c9 100644 --- a/include/xrpl/ledger/helpers/RippleStateHelpers.h +++ b/include/xrpl/ledger/helpers/RippleStateHelpers.h @@ -93,7 +93,7 @@ isFrozen(ReadView const& view, AccountID const& account, Issue const& issue) // Overload with depth parameter for uniformity with MPTIssue version. // The depth parameter is ignored for IOUs since they don't have vault recursion. [[nodiscard]] inline bool -isFrozen(ReadView const& view, AccountID const& account, Issue const& issue, int /*depth*/) +isFrozen(ReadView const& view, AccountID const& account, Issue const& issue, std::uint8_t /*depth*/) { return isFrozen(view, account, issue); } @@ -110,7 +110,7 @@ isDeepFrozen( ReadView const& view, AccountID const& account, Issue const& issue, - int = 0 /*ignored*/) + std::uint8_t = 0 /*ignored*/) { return isDeepFrozen(view, account, issue.currency, issue.account); } diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index 3d41ac47cd..b47286a5d5 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -34,6 +34,15 @@ enum class WaiveTransferFee : bool { No = false, Yes }; /** Controls whether accountSend is allowed to overflow OutstandingAmount **/ enum class AllowMPTOverflow : bool { No = false, Yes }; +/** Controls whether canTransfer enforces lsfMPTCanTransfer on MPTs. + * + * Default is No (enforce). Use Yes at call sites that must remain available + * even when an MPT issuer has cleared lsfMPTCanTransfer - for example, + * unwinding existing positions in SAV or the Lending Protocol. Has no + * effect on the IOU branch of canTransfer. + */ +enum class WaiveMPTCanTransfer : bool { No = false, Yes }; + /* Check if MPToken (for MPT) or trust line (for IOU) exists: * - StrongAuth - before checking if authorization is required * - WeakAuth @@ -63,7 +72,11 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& * purely defensive, as we currently do not allow such vaults to be created. */ [[nodiscard]] bool -isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, int depth = 0); +isFrozen( + ReadView const& view, + AccountID const& account, + Asset const& asset, + std::uint8_t depth = 0); [[nodiscard]] TER checkFrozen(ReadView const& view, AccountID const& account, Issue const& issue); @@ -85,14 +98,14 @@ isAnyFrozen( ReadView const& view, std::initializer_list const& accounts, Asset const& asset, - int depth = 0); + std::uint8_t depth = 0); [[nodiscard]] bool isDeepFrozen( ReadView const& view, AccountID const& account, MPTIssue const& mptIssue, - int depth = 0); + std::uint8_t depth = 0); /** * isFrozen check is recursive for MPT shares in a vault, descending to @@ -100,7 +113,11 @@ isDeepFrozen( * purely defensive, as we currently do not allow such vaults to be created. */ [[nodiscard]] bool -isDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset, int depth = 0); +isDeepFrozen( + ReadView const& view, + AccountID const& account, + Asset const& asset, + std::uint8_t depth = 0); [[nodiscard]] TER checkDeepFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue); @@ -234,7 +251,13 @@ requireAuth( AuthType authType = AuthType::Legacy); [[nodiscard]] TER -canTransfer(ReadView const& view, Asset const& asset, AccountID const& from, AccountID const& to); +canTransfer( + ReadView const& view, + Asset const& asset, + AccountID const& from, + AccountID const& to, + WaiveMPTCanTransfer waive = WaiveMPTCanTransfer::No, + std::uint8_t depth = 0); //------------------------------------------------------------------------------ // diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index c4b392a92f..19b8b44fe6 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -400,6 +400,7 @@ LEDGER_ENTRY(ltMPTOKEN_ISSUANCE, 0x007e, MPTokenIssuance, mpt_issuance, ({ {sfPreviousTxnLgrSeq, SoeRequired}, {sfDomainID, SoeOptional}, {sfMutableFlags, SoeDefault}, + {sfReferenceHolding, SoeOptional}, })) /** A ledger object which tracks MPToken diff --git a/include/xrpl/protocol/detail/sfields.macro b/include/xrpl/protocol/detail/sfields.macro index 5d1689dce9..01bb4fc480 100644 --- a/include/xrpl/protocol/detail/sfields.macro +++ b/include/xrpl/protocol/detail/sfields.macro @@ -205,6 +205,7 @@ TYPED_SFIELD(sfParentBatchID, UINT256, 36) TYPED_SFIELD(sfLoanBrokerID, UINT256, 37, SField::kSmdPseudoAccount | SField::kSmdDefault) TYPED_SFIELD(sfLoanID, UINT256, 38) +TYPED_SFIELD(sfReferenceHolding, UINT256, 39) // number (common) TYPED_SFIELD(sfNumber, NUMBER, 1) diff --git a/include/xrpl/protocol_autogen/ledger_entries/MPTokenIssuance.h b/include/xrpl/protocol_autogen/ledger_entries/MPTokenIssuance.h index 8377fdf1a4..7f772b1c74 100644 --- a/include/xrpl/protocol_autogen/ledger_entries/MPTokenIssuance.h +++ b/include/xrpl/protocol_autogen/ledger_entries/MPTokenIssuance.h @@ -278,6 +278,30 @@ public: { return this->sle_->isFieldPresent(sfMutableFlags); } + + /** + * @brief Get sfReferenceHolding (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getReferenceHolding() const + { + if (hasReferenceHolding()) + return this->sle_->at(sfReferenceHolding); + return std::nullopt; + } + + /** + * @brief Check if sfReferenceHolding is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasReferenceHolding() const + { + return this->sle_->isFieldPresent(sfReferenceHolding); + } }; /** @@ -469,6 +493,17 @@ public: return *this; } + /** + * @brief Set sfReferenceHolding (SoeOptional) + * @return Reference to this builder for method chaining. + */ + MPTokenIssuanceBuilder& + setReferenceHolding(std::decay_t const& value) + { + object_[sfReferenceHolding] = value; + return *this; + } + /** * @brief Build and return the completed MPTokenIssuance wrapper. * @param index The ledger entry index. diff --git a/include/xrpl/tx/invariants/MPTInvariant.h b/include/xrpl/tx/invariants/MPTInvariant.h index 701d8123c0..2790a7dcc3 100644 --- a/include/xrpl/tx/invariants/MPTInvariant.h +++ b/include/xrpl/tx/invariants/MPTInvariant.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -22,6 +23,18 @@ class ValidMPTIssuance // MPToken by an issuer bool mptCreatedByIssuer_ = false; + /// sfReferenceHolding is intended to be set exactly once at vault + /// creation and immutable thereafter; true when that rule was violated. + bool referenceHoldingSetOnCreate_ = false; + + /// True when sfReferenceHolding was mutated on an existing MPTokenIssuance. + bool referenceHoldingMutated_ = false; + + /// MPTokens and RippleStates deleted during apply. finalize() checks each + /// holder's AccountRoot to detect vault pseudo-account holdings deleted + /// outside VaultDelete. All these checks are gated on fixCleanup3_2_0. + std::vector> deletedHoldings_; + public: void visitEntry(bool, std::shared_ptr const&, std::shared_ptr const&); diff --git a/include/xrpl/tx/transactors/token/MPTokenIssuanceCreate.h b/include/xrpl/tx/transactors/token/MPTokenIssuanceCreate.h index 3861c19afc..6c59d85548 100644 --- a/include/xrpl/tx/transactors/token/MPTokenIssuanceCreate.h +++ b/include/xrpl/tx/transactors/token/MPTokenIssuanceCreate.h @@ -6,23 +6,28 @@ namespace xrpl { +// NOLINTBEGIN(readability-redundant-member-init) struct MPTCreateArgs { std::optional priorBalance; AccountID const& account; std::uint32_t sequence = 0; std::uint32_t flags = 0; - std::optional maxAmount = - std::nullopt; // NOLINT(readability-redundant-member-init) - std::optional assetScale = - std::nullopt; // NOLINT(readability-redundant-member-init) - std::optional transferFee = - std::nullopt; // NOLINT(readability-redundant-member-init) + std::optional maxAmount = std::nullopt; + std::optional assetScale = std::nullopt; + std::optional transferFee = std::nullopt; std::optional const& metadata{}; - std::optional domainId = std::nullopt; // NOLINT(readability-redundant-member-init) - std::optional mutableFlags = - std::nullopt; // NOLINT(readability-redundant-member-init) + std::optional domainId = std::nullopt; + std::optional mutableFlags = std::nullopt; + // Set only by callers that issue an MPT representing a wrapped asset + // (e.g. VaultCreate's share token). The keylet must point to an + // existing MPToken or RippleState owned by `account`. Surfaces on + // the resulting MPTokenIssuance via the optional sfReferenceHolding + // field. Used by readers (canTransfer, canTrade, freezing) to + // inherit the underlying asset's transferability. + std::optional referenceHolding = std::nullopt; }; +// NOLINTEND(readability-redundant-member-init) class MPTokenIssuanceCreate : public Transactor { diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 4b67bb9867..c62d79dcac 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -57,19 +58,45 @@ isVaultPseudoAccountFrozen( ReadView const& view, AccountID const& account, MPTIssue const& mptShare, - int depth) + std::uint8_t depth) { if (!view.rules().enabled(featureSingleAssetVault)) return false; if (depth >= kMaxAssetCheckDepth) - return true; // LCOV_EXCL_LINE + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::View::isVaultPseudoAccountFrozen : reached asset check depth"); + return true; + // LCOV_EXCL_STOP + } auto const mptIssuance = view.read(keylet::mptIssuance(mptShare.getMptID())); if (mptIssuance == nullptr) return false; // zero MPToken won't block deletion of MPTokenIssuance auto const issuer = mptIssuance->getAccountID(sfIssuer); + + // Post-fixCleanup3_2_0: vault shares carry sfReferenceHolding pointing + // to the vault pseudo's MPToken or RippleState for the underlying. + // Read it to derive the underlying asset and recurse, skipping the + // issuer-account-then-vault chain. Pre-amendment shares (no field) + // fall back to the chain lookup below. + if (mptIssuance->isFieldPresent(sfReferenceHolding)) + { + auto const sleHolding = + view.read(keylet::unchecked(mptIssuance->getFieldH256(sfReferenceHolding))); + if (!sleHolding) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::isVaultPseudoAccountFrozen : dangling sfReferenceHolding"); + return false; + // LCOV_EXCL_STOP + } + return isAnyFrozen( + view, {issuer, account}, assetOfHolding(*mptIssuance, *sleHolding), depth + 1); + } + auto const mptIssuer = view.read(keylet::account(issuer)); if (mptIssuer == nullptr) { diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 925f37c8ed..542a5b60ed 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -55,7 +55,11 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, MPTIssue cons } bool -isFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue, int depth) +isFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue, + std::uint8_t depth) { return isGlobalFrozen(view, mptIssue) || isIndividualFrozen(view, account, mptIssue) || isVaultPseudoAccountFrozen(view, account, mptIssue, depth); @@ -66,7 +70,7 @@ isAnyFrozen( ReadView const& view, std::initializer_list const& accounts, MPTIssue const& mptIssue, - int depth) + std::uint8_t depth) { if (isGlobalFrozen(view, mptIssue)) return true; @@ -303,7 +307,7 @@ requireAuth( MPTIssue const& mptIssue, AccountID const& account, AuthType authType, - int depth) + std::uint8_t depth) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); auto const sleIssuance = view.read(mptID); @@ -321,7 +325,12 @@ requireAuth( if (featureSAVEnabled) { if (depth >= kMaxAssetCheckDepth) - return tecINTERNAL; // LCOV_EXCL_LINE + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::MPTokenHelpers::requireAuth : reached asset check depth"); + return tecINTERNAL; + // LCOV_EXCL_STOP + } // requireAuth is recursive if the issuer is a vault pseudo-account auto const sleIssuer = view.read(keylet::account(mptIssuer)); @@ -489,28 +498,88 @@ enforceMPTokenAuthorization( // LCOV_EXCL_STOP } +[[nodiscard]] Asset +assetOfHolding(SLE const& sleShareIssuance, SLE const& sleHolding) +{ + XRPL_ASSERT_PARTS( + sleHolding.getType() == ltRIPPLE_STATE || sleHolding.getType() == ltMPTOKEN, + "xrpl::assetOfHolding", + "unexpected holding type"); + XRPL_ASSERT_PARTS( + sleShareIssuance.getType() == ltMPTOKEN_ISSUANCE, + "xrpl::assetOfHolding", + "not SLE MPTokenIssuance"); + + if (sleHolding.getType() == ltMPTOKEN) + return MPTIssue{sleHolding.getFieldH192(sfMPTokenIssuanceID)}; + + auto const vaultPseudo = sleShareIssuance.at(sfIssuer); + auto const lowLimit = sleHolding.getFieldAmount(sfLowLimit); + auto const highLimit = sleHolding.getFieldAmount(sfHighLimit); + auto const& iouIssuer = + (lowLimit.getIssuer() != vaultPseudo) ? lowLimit.getIssuer() : highLimit.getIssuer(); + return Issue{lowLimit.get().currency, iouIssuer}; +} + TER canTransfer( ReadView const& view, MPTIssue const& mptIssue, AccountID const& from, - AccountID const& to) + AccountID const& to, + WaiveMPTCanTransfer waive, + std::uint8_t depth) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); auto const sleIssuance = view.read(mptID); if (!sleIssuance) return tecOBJECT_NOT_FOUND; + auto const issuer = (*sleIssuance)[sfIssuer]; + if (waive == WaiveMPTCanTransfer::Yes || from == issuer || to == issuer) + return tesSUCCESS; + if (!sleIssuance->isFlag(lsfMPTCanTransfer)) + return TER{tecNO_AUTH}; + + // Post-fixCleanup3_2_0: vault shares carry sfReferenceHolding pointing + // to the vault pseudo's MPToken or RippleState for the underlying asset. + // Third-party transfers inherit the underlying's transferability. + // Issuer-involving transfers and waived callers returned tesSUCCESS above. + // + // The recursive call always passes WaiveMPTCanTransfer::No so that + // a waived outer caller does not transitively unlock the underlying. + if (view.rules().enabled(fixCleanup3_2_0) && sleIssuance->isFieldPresent(sfReferenceHolding)) { - if (from != (*sleIssuance)[sfIssuer] && to != (*sleIssuance)[sfIssuer]) - return TER{tecNO_AUTH}; + // Defensive depth bound on the inheritance recursion. Unreachable + // in practice (vault-of-vault-shares is forbidden at VaultCreate). + if (depth >= kMaxAssetCheckDepth) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::MPTokenHelpers::canTransfer : reached asset check depth"); + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + auto const sleHolding = + view.read(keylet::unchecked(sleIssuance->getFieldH256(sfReferenceHolding))); + if (!sleHolding) + return tefINTERNAL; // LCOV_EXCL_LINE + + return canTransfer( + view, + assetOfHolding(*sleIssuance, *sleHolding), + from, + to, + WaiveMPTCanTransfer::No, + depth + 1); } + return tesSUCCESS; } TER -canTrade(ReadView const& view, Asset const& asset) +canTrade(ReadView const& view, Asset const& asset, std::uint8_t depth) { return asset.visit( [&](Issue const&) -> TER { return tesSUCCESS; }, @@ -520,6 +589,31 @@ canTrade(ReadView const& view, Asset const& asset) return tecOBJECT_NOT_FOUND; if (!sleIssuance->isFlag(lsfMPTCanTrade)) return tecNO_PERMISSION; + + // Post-fixCleanup3_2_0: vault shares inherit the underlying + // asset's tradability. A share whose underlying has been + // removed from trading cannot itself be placed on the DEX. + if (view.rules().enabled(fixCleanup3_2_0) && + sleIssuance->isFieldPresent(sfReferenceHolding)) + { + // Defensive depth bound on the inheritance recursion. + // Unreachable in practice (vault-of-vault-shares + // forbidden at VaultCreate). + if (depth >= kMaxAssetCheckDepth) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::MPTokenHelpers::canTrade : reached asset check depth"); + return tecINTERNAL; + // LCOV_EXCL_STOP + } + auto const sleHolding = + view.read(keylet::unchecked(sleIssuance->getFieldH256(sfReferenceHolding))); + if (!sleHolding) + return tefINTERNAL; // LCOV_EXCL_LINE + + return canTrade(view, assetOfHolding(*sleIssuance, *sleHolding), depth + 1); + } + return tesSUCCESS; }); } @@ -892,7 +986,12 @@ issuerSelfDebitHookMPT(ApplyView& view, MPTIssue const& issue, std::uint64_t amo } static TER -checkMPTAllowed(ReadView const& view, TxType txType, Asset const& asset, AccountID const& accountID) +checkMPTAllowed( + ReadView const& view, + TxType txType, + Asset const& asset, + AccountID const& accountID, + std::uint8_t depth = 0) { if (!asset.holds()) return tesSUCCESS; @@ -914,17 +1013,15 @@ checkMPTAllowed(ReadView const& view, TxType txType, Asset const& asset, Account if (!issuanceSle) return tecOBJECT_NOT_FOUND; // LCOV_EXCL_LINE - auto const flags = issuanceSle->getFlags(); - - if ((flags & lsfMPTLocked) != 0u) + if (issuanceSle->isFlag(lsfMPTLocked)) return tecLOCKED; // LCOV_EXCL_LINE // Offer crossing and Payment - if ((flags & lsfMPTCanTrade) == 0) + if (!issuanceSle->isFlag(lsfMPTCanTrade)) return tecNO_PERMISSION; if (accountID != issuer) { - if ((flags & lsfMPTCanTransfer) == 0) + if (!issuanceSle->isFlag(lsfMPTCanTransfer)) return tecNO_PERMISSION; auto const mptSle = view.read(keylet::mptoken(issuanceKey.key, accountID)); @@ -935,6 +1032,34 @@ checkMPTAllowed(ReadView const& view, TxType txType, Asset const& asset, Account if (mptSle->isFlag(lsfMPTLocked)) return tecLOCKED; + + // Post-fixCleanup3_2_0: vault shares inherit the underlying + // asset's checks here too. Without this, a share could be + // placed on the AMM, in an Offer, or in a Check even after + // the issuer has restricted the underlying. Mirrors the + // canTransfer / canTrade inheritance for path-find-adjacent + // operations that don't go through canTransfer directly. + if (view.rules().enabled(fixCleanup3_2_0) && + issuanceSle->isFieldPresent(sfReferenceHolding)) + { + // Defensive depth bound on the inheritance recursion. + // Reachable only post-fixCleanup3_2_0 and unreachable in + // practice (vault-of-vault-shares forbidden at VaultCreate). + if (depth >= kMaxAssetCheckDepth) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::MPTokenHelpers::checkMPTAllowed : reached asset check depth"); + return tecINTERNAL; + // LCOV_EXCL_STOP + } + auto const sleHolding = + view.read(keylet::unchecked(issuanceSle->getFieldH256(sfReferenceHolding))); + if (!sleHolding) + return tefINTERNAL; // LCOV_EXCL_LINE + + return checkMPTAllowed( + view, txType, assetOfHolding(*issuanceSle, *sleHolding), accountID, depth + 1); + } } return tesSUCCESS; diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index c9dccb884d..71019761ed 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -63,7 +63,7 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& } bool -isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, int depth) +isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, std::uint8_t depth) { return std::visit( [&](auto const& issue) { return isFrozen(view, account, issue, depth); }, asset.value()); @@ -107,7 +107,7 @@ isAnyFrozen( ReadView const& view, std::initializer_list const& accounts, Asset const& asset, - int depth) + std::uint8_t depth) { return asset.visit( [&](Issue const& issue) { return isAnyFrozen(view, accounts, issue); }, @@ -115,7 +115,11 @@ isAnyFrozen( } bool -isDeepFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue, int depth) +isDeepFrozen( + ReadView const& view, + AccountID const& account, + MPTIssue const& mptIssue, + std::uint8_t depth) { // Unlike IOUs, frozen / locked MPTs are not allowed to send or receive // funds, so checking "deep frozen" is the same as checking "frozen". @@ -123,7 +127,7 @@ isDeepFrozen(ReadView const& view, AccountID const& account, MPTIssue const& mpt } bool -isDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset, int depth) +isDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset, std::uint8_t depth) { return std::visit( [&](auto const& issue) { return isDeepFrozen(view, account, issue, depth); }, @@ -500,13 +504,19 @@ requireAuth(ReadView const& view, Asset const& asset, AccountID const& account, } TER -canTransfer(ReadView const& view, Asset const& asset, AccountID const& from, AccountID const& to) +canTransfer( + ReadView const& view, + Asset const& asset, + AccountID const& from, + AccountID const& to, + WaiveMPTCanTransfer waive, + std::uint8_t depth) { - return std::visit( - [&](TIss const& issue) -> TER { - return canTransfer(view, issue, from, to); + return asset.visit( + [&](MPTIssue const& issue) -> TER { + return canTransfer(view, issue, from, to, waive, depth); }, - asset.value()); + [&](Issue const& issue) -> TER { return canTransfer(view, issue, from, to); }); } //------------------------------------------------------------------------------ diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 63171444bb..66b52012e8 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -5,11 +5,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -112,6 +114,13 @@ ValidMPTIssuance::visitEntry( std::shared_ptr const& before, std::shared_ptr const& after) { + // The sfReferenceHolding tracking and the deleted-holding capture are + // only meaningful post-fixCleanup3_2_0 (the field is never set + // pre-amendment, and the holding-deletion rule does not apply). + // Skip both blocks when the amendment is off so we avoid wasted work + // on the hot path. + bool const fix320Enabled = isFeatureEnabled(fixCleanup3_2_0); + if (after && after->getType() == ltMPTOKEN_ISSUANCE) { if (isDelete) @@ -121,6 +130,21 @@ ValidMPTIssuance::visitEntry( else if (!before) { mptIssuancesCreated_++; + if (fix320Enabled && after->isFieldPresent(sfReferenceHolding)) + referenceHoldingSetOnCreate_ = true; + } + else if (fix320Enabled) + { + // Modified issuance: detect any change to sfReferenceHolding. + bool const beforePresent = before->isFieldPresent(sfReferenceHolding); + bool const afterPresent = after->isFieldPresent(sfReferenceHolding); + if (beforePresent != afterPresent || + (afterPresent && + before->getFieldH256(sfReferenceHolding) != + after->getFieldH256(sfReferenceHolding))) + { + referenceHoldingMutated_ = true; + } } } @@ -129,6 +153,8 @@ ValidMPTIssuance::visitEntry( if (isDelete) { mptokensDeleted_++; + if (fix320Enabled) + deletedHoldings_.push_back(after); } else if (!before) { @@ -138,6 +164,11 @@ ValidMPTIssuance::visitEntry( mptCreatedByIssuer_ = true; } } + + // Capture deleted RippleState SLEs so finalize() can verify none of + // them were owned by a vault pseudo-account outside VaultDelete. + if (fix320Enabled && isDelete && after && after->getType() == ltRIPPLE_STATE) + deletedHoldings_.push_back(after); } bool @@ -150,6 +181,63 @@ ValidMPTIssuance::finalize( { auto const& rules = view.rules(); bool const mptV2Enabled = rules.enabled(featureMPTokensV2); + + // Post-fixCleanup3_2_0: + // - sfReferenceHolding is set only by VaultCreate at share-issuance + // creation, and is immutable thereafter. + // - A vault pseudo-account's MPToken or RippleState may only be + // deleted by VaultDelete; the share's sfReferenceHolding pointer + // must not dangle outside that controlled lifecycle. + if (rules.enabled(fixCleanup3_2_0)) + { + bool invariantPasses = true; + if (referenceHoldingMutated_) + { + JLOG(j.fatal()) << "Invariant failed: sfReferenceHolding was modified " + "on an existing MPTokenIssuance"; + invariantPasses = false; + } + if (referenceHoldingSetOnCreate_ && tx.getTxnType() != ttVAULT_CREATE) + { + JLOG(j.fatal()) << "Invariant failed: sfReferenceHolding set on a new " + "MPTokenIssuance by a non-VaultCreate transaction"; + invariantPasses = false; + } + if (!deletedHoldings_.empty() && tx.getTxnType() != ttVAULT_DELETE) + { + auto const isVaultPseudo = [&](AccountID const& acct) { + auto const sle = view.read(keylet::account(acct)); + return sle && sle->isFieldPresent(sfVaultID); + }; + for (auto const& sleHolding : deletedHoldings_) + { + bool offending = false; + if (sleHolding->getType() == ltMPTOKEN) + { + offending = isVaultPseudo(sleHolding->at(sfAccount)); + } + else // ltRIPPLE_STATE + { + auto const lowLimit = sleHolding->getFieldAmount(sfLowLimit); + auto const highLimit = sleHolding->getFieldAmount(sfHighLimit); + // Each limit's STAmount.issuer is the COUNTERPARTY of + // that side's owner: lowLimit's issuer is the high + // account, highLimit's issuer is the low account. + offending = + isVaultPseudo(lowLimit.getIssuer()) || isVaultPseudo(highLimit.getIssuer()); + } + if (offending) + { + JLOG(j.fatal()) << "Invariant failed: vault pseudo-account holding " + "deleted by a non-VaultDelete transaction"; + invariantPasses = false; + } + } + } + if (!invariantPasses) + return false; + } + if (isTesSuccess(result) || (mptV2Enabled && result == tecINCOMPLETE)) { [[maybe_unused]] diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp index 936c2f1b51..52838b2119 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -95,8 +96,13 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) // The broker's pseudo-account is the source of funds. auto const pseudoAccountID = sleBroker->at(sfAccount); - // Cannot transfer a non-transferable Asset - if (auto const ret = canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct)) + // Post-fixCleanup3_2_0: cover withdraw is a recovery path that bypasses + // the lsfMPTCanTransfer flag check, so an issuer cannot trap a broker's + // first-loss capital. Other transferability checks (IOU NoRipple, freeze, + // requireAuth) still apply. + auto const waive = ctx.view.rules().enabled(fixCleanup3_2_0) ? WaiveMPTCanTransfer::Yes + : WaiveMPTCanTransfer::No; + if (auto const ret = canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct, waive)) return ret; // Withdrawal to a 3rd party destination account is essentially a transfer. diff --git a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp index 1440b67309..64d0b01f5e 100644 --- a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp +++ b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +50,11 @@ MPTokenIssuanceCreate::getFlagsMask(PreflightContext const& ctx) NotTEC MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) { + // sfReferenceHolding is set only internally by VaultCreate. Reject + // any user-submitted MPTokenIssuanceCreate that attempts to carry it. + if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx.isFieldPresent(sfReferenceHolding)) + return temMALFORMED; + // If the mutable flags field is included, at least one flag must be // specified. if (auto const mutableFlags = ctx.tx[~sfMutableFlags]; mutableFlags && @@ -141,6 +147,22 @@ MPTokenIssuanceCreate::create(ApplyView& view, beast::Journal journal, MPTCreate if (args.mutableFlags) (*mptIssuance)[sfMutableFlags] = *args.mutableFlags; + if (args.referenceHolding) + { + // Defensive: the holding must already exist and be of an + // expected type. Callers (currently only VaultCreate) + // populate this after the pseudo-account's MPToken / + // RippleState has been installed. A missing holding here + // would dangle the pointer and is a programmer error. + auto const sleHolding = view.read(keylet::unchecked(*args.referenceHolding)); + if (!sleHolding) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + auto const type = sleHolding->getType(); + if (type != ltMPTOKEN && type != ltRIPPLE_STATE) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + (*mptIssuance)[sfReferenceHolding] = *args.referenceHolding; + } + view.insert(mptIssuance); } diff --git a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp index c6e1f28d57..7490f44619 100644 --- a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp @@ -1,12 +1,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -162,9 +164,9 @@ VaultCreate::doApply() auto maybePseudo = createPseudoAccount(view(), vault->key(), sfVaultID); if (!maybePseudo) return maybePseudo.error(); // LCOV_EXCL_LINE - auto& pseudo = *maybePseudo; - auto pseudoId = pseudo->at(sfAccount); - auto asset = tx[sfAsset]; + auto const& pseudo = *maybePseudo; + AccountID const pseudoId = pseudo->at(sfAccount); + auto const asset = tx[sfAsset]; if (auto ter = addEmptyHolding(view(), pseudoId, preFeeBalance_, asset, j_); !isTesSuccess(ter)) return ter; @@ -182,13 +184,24 @@ VaultCreate::doApply() // Note, here we are **not** creating an MPToken for the assets held in // the vault. That MPToken or TrustLine/RippleState is created above, in // addEmptyHolding. Here we are creating MPTokenIssuance for the shares - // in the vault - auto maybeShare = MPTokenIssuanceCreate::create( + // in the vault. + // + // Post-fixCleanup3_2_0: surface the vault pseudo's holding (MPToken + // for MPT, RippleState for IOU) on the share via sfReferenceHolding. + // XRP underlyings leave it unset. + auto const referenceHolding = [&]() -> std::optional { + if (!view().rules().enabled(fixCleanup3_2_0) || asset.native()) + return std::nullopt; + return asset.holds() + ? keylet::mptoken(asset.get().getMptID(), pseudoId).key + : keylet::line(pseudoId, asset.get()).key; + }(); + auto const maybeShare = MPTokenIssuanceCreate::create( view(), j_, { .priorBalance = std::nullopt, - .account = pseudoId->value(), + .account = pseudoId, .sequence = 1, .flags = mptFlags, .assetScale = scale, @@ -196,6 +209,7 @@ VaultCreate::doApply() .metadata = tx[~sfMPTokenMetadata], .domainId = tx[~sfDomainID], .mutableFlags = std::nullopt, + .referenceHolding = referenceHolding, }); if (!maybeShare) return maybeShare.error(); // LCOV_EXCL_LINE diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 0fcc1e7076..bf8e772ac2 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -67,7 +67,14 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) auto const& vaultAccount = vault->at(sfAccount); auto const& account = ctx.tx[sfAccount]; auto const& dstAcct = ctx.tx[~sfDestination].value_or(account); - if (auto ter = canTransfer(ctx.view, vaultAsset, vaultAccount, dstAcct); !isTesSuccess(ter)) + // Post-fixCleanup3_2_0: withdraw is a recovery path that bypasses the + // lsfMPTCanTransfer flag check, so an issuer cannot trap depositor funds. + // Other transferability checks (IOU NoRipple, freeze, requireAuth) still + // apply. + auto const waive = ctx.view.rules().enabled(fixCleanup3_2_0) ? WaiveMPTCanTransfer::Yes + : WaiveMPTCanTransfer::No; + if (auto ter = canTransfer(ctx.view, vaultAsset, vaultAccount, dstAcct, waive); + !isTesSuccess(ter)) { JLOG(ctx.j.debug()) << "VaultWithdraw: vault assets are non-transferable."; return ter; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 47f4572a0d..bc9bb8d204 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4148,6 +4148,114 @@ class Invariants_test : public beast::unit_test::Suite return true; }); } + + // sfReferenceHolding can only be set on creation by VaultCreate. A + // non-VaultCreate transaction that creates an MPTokenIssuance with + // sfReferenceHolding present must trip the invariant. + doInvariantCheck( + {{"sfReferenceHolding set on a new MPTokenIssuance by a " + "non-VaultCreate transaction"}}, + [](Account const& a1, Account const&, ApplyContext& ac) { + auto const sleAcct = ac.view().peek(keylet::account(a1.id())); + if (!sleAcct) + return false; + MPTIssue const mpt{makeMptID(sleAcct->getFieldU32(sfSequence), a1)}; + auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); + sleNew->setFieldH256(sfReferenceHolding, uint256{1}); + ac.view().insert(sleNew); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}); + + // sfReferenceHolding is immutable: changing the field on an + // existing MPTokenIssuance must trip the invariant. Set up a real + // vault via preclose (so the share issuance carries + // sfReferenceHolding), then mutate it in precheck to produce a + // before/after pair. + { + uint256 vaultKey; + doInvariantCheck( + {{"sfReferenceHolding was modified on an existing " + "MPTokenIssuance"}}, + [&](Account const&, Account const&, ApplyContext& ac) { + auto const sleVault = ac.view().peek(keylet::vault(vaultKey)); + if (!sleVault) + return false; + auto sleIssuance = + ac.view().peek(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + if (!sleIssuance) + return false; + sleIssuance->setFieldH256(sfReferenceHolding, uint256{2}); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& a1, Account const&, Env& env) { + Account const issuer{"issuer"}; + env.fund(XRP(10'000), issuer); + env.close(); + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = a1}); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = a1, .asset = asset}); + env(tx); + env.close(); + vaultKey = keylet.key; + return true; + }); + } + + // A vault pseudo-account's MPToken cannot be deleted by anything + // other than a VaultDelete transaction. Set up a vault, then have + // an arbitrary tx erase the pseudo's MPToken in precheck. + { + uint256 vaultKey; + doInvariantCheck( + {{"vault pseudo-account holding deleted by a " + "non-VaultDelete transaction"}}, + [&](Account const&, Account const&, ApplyContext& ac) { + auto const sleVault = ac.view().peek(keylet::vault(vaultKey)); + if (!sleVault) + return false; + auto const sleIssuance = + ac.view().peek(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + if (!sleIssuance || !sleIssuance->isFieldPresent(sfReferenceHolding)) + return false; + auto sleHolding = ac.view().peek( + keylet::unchecked(sleIssuance->getFieldH256(sfReferenceHolding))); + if (!sleHolding) + return false; + ac.view().erase(sleHolding); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& a1, Account const&, Env& env) { + Account const issuer{"issuer"}; + env.fund(XRP(10'000), issuer); + env.close(); + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = a1}); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = a1, .asset = asset}); + env(tx); + env.close(); + vaultKey = keylet.key; + return true; + }); + } } // Test the invariant overwrite fix for both pre- and post-amendment diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index a8132887fc..642f5724c7 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -5397,7 +5398,7 @@ protected: void testCoverDepositWithdrawNonTransferableMPT(FeatureBitset feature) { - testcase("CoverDeposit and CoverWithdraw reject MPT without CanTransfer"); + testcase("CoverDeposit blocked, CoverWithdraw allowed when CanTransfer cleared"); using namespace jtx; using namespace loanBroker; @@ -5409,19 +5410,14 @@ protected: env.fund(XRP(100'000), issuer, alice); env.close(); - MPTTester mpt{env, issuer, kMptInitNoFund}; - - mpt.create({.flags = tfMPTCanTransfer, .mutableFlags = tmfMPTCanMutateCanTransfer}); - - env.close(); - + MPTTester mpt( + {.env = env, + .issuer = issuer, + .holders = {alice}, + .pay = 100, + .flags = tfMPTCanTransfer, + .mutableFlags = tmfMPTCanMutateCanTransfer}); PrettyAsset const asset = mpt["MPT"]; - mpt.authorize({.account = alice}); - env.close(); - - // Issuer can fund the holder even if CanTransfer is not set. - env(pay(issuer, alice, asset(100))); - env.close(); Vault const vault{env}; auto const [createTx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); @@ -5438,30 +5434,9 @@ protected: Account const pseudoAccount{"Loan Broker pseudo-account", brokerSle->at(sfAccount)}; - // Remove CanTransfer after the broker is set up. - mpt.set({.mutableFlags = tmfMPTClearCanTransfer}); - env.close(); - - // Standard Payment path should forbid third-party transfers. - auto const err = feature[featureMPTokensV2] ? tecNO_PERMISSION : tecNO_AUTH; - env(pay(alice, pseudoAccount, asset(1)), Ter(err)); - env.close(); - - // Cover cannot be transferred to broker account + // First, deposit some cover while CanTransfer is set so we have an + // existing position to withdraw from after the governance action. auto const depositAmount = asset(1); - env(coverDeposit(alice, brokerKeylet.key, depositAmount), Ter{tecNO_AUTH}); - env.close(); - - if (auto const refreshed = env.le(brokerKeylet); BEAST_EXPECT(refreshed)) - { - BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 0); - env.require(Balance(pseudoAccount, asset(0))); - } - - // Set CanTransfer again and transfer some deposit - mpt.set({.mutableFlags = tmfMPTSetCanTransfer}); - env.close(); - env(coverDeposit(alice, brokerKeylet.key, depositAmount)); env.close(); @@ -5471,26 +5446,177 @@ protected: env.require(Balance(pseudoAccount, depositAmount)); } - // Remove CanTransfer after the deposit + // Issuer governance: clear CanTransfer. mpt.set({.mutableFlags = tmfMPTClearCanTransfer}); env.close(); - // Cover cannot be transferred from broker account - env(coverWithdraw(alice, brokerKeylet.key, depositAmount), Ter{tecNO_AUTH}); + // Standard Payment path still forbids third-party transfers. + auto const err = feature[featureMPTokensV2] ? tecNO_PERMISSION : tecNO_AUTH; + env(pay(alice, pseudoAccount, asset(1)), Ter(err)); env.close(); - // Set CanTransfer again and withdraw - mpt.set({.mutableFlags = tmfMPTSetCanTransfer}); - env.close(); - - env(coverWithdraw(alice, brokerKeylet.key, depositAmount)); + // New cover deposits are blocked - this would create new exposure. + env(coverDeposit(alice, brokerKeylet.key, depositAmount), Ter{tecNO_AUTH}); env.close(); if (auto const refreshed = env.le(brokerKeylet); BEAST_EXPECT(refreshed)) { - BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 0); - env.require(Balance(pseudoAccount, asset(0))); + BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 1); + env.require(Balance(pseudoAccount, depositAmount)); } + + bool const postAmendment = feature[fixCleanup3_2_0]; + if (postAmendment) + { + // Post-fixCleanup3_2_0: existing cover can always be withdrawn + // even when CanTransfer is cleared, so the broker is not trapped. + env(coverWithdraw(alice, brokerKeylet.key, depositAmount)); + env.close(); + + if (auto const refreshed = env.le(brokerKeylet); BEAST_EXPECT(refreshed)) + { + BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 0); + env.require(Balance(pseudoAccount, asset(0))); + } + } + else + { + // Pre-fixCleanup3_2_0 regression: cover withdraw was blocked, + // trapping the broker's first-loss capital. + env(coverWithdraw(alice, brokerKeylet.key, depositAmount), Ter{tecNO_AUTH}); + env.close(); + + if (auto const refreshed = env.le(brokerKeylet); BEAST_EXPECT(refreshed)) + { + BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 1); + env.require(Balance(pseudoAccount, depositAmount)); + } + } + } + + void + testLoanSetBlockedLoanPayAllowedWhenCanTransferCleared() + { + testcase("LoanSet blocked, LoanPay allowed when CanTransfer cleared"); + using namespace jtx; + using namespace loan; + + Env env(*this, all_); + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + MPTTester mpt( + {.env = env, + .issuer = issuer, + .holders = {lender, borrower}, + .flags = tfMPTCanTransfer | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTransfer}); + PrettyAsset const asset = mpt.issuanceID(); + env(pay(issuer, lender, asset(10'000'000))); + // Fund the borrower with enough to cover principal+interest+fees + env(pay(issuer, borrower, asset(100'000))); + env.close(); + + // Create vault and broker while CanTransfer is set. + auto const broker = createVaultAndBroker(env, asset, lender); + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + + // Create an existing loan while CanTransfer is set. + env(set(borrower, broker.brokerID, 1'000), + Sig(sfCounterpartySignature, lender), + loanSetFee); + env.close(); + auto const loanKeylet = keylet::loan(broker.brokerID, 1); + BEAST_EXPECT(env.le(loanKeylet)); + + // Issuer governance: clear CanTransfer. + mpt.set({.mutableFlags = tmfMPTClearCanTransfer}); + env.close(); + + // Issuing a NEW loan is blocked - it would create new exposure into + // a pool the issuer is restricting. + env(set(borrower, broker.brokerID, 1'000), + Sig(sfCounterpartySignature, lender), + loanSetFee, + Ter{tecNO_AUTH}); + env.close(); + + // Repaying an existing loan is always allowed - blocking it would + // create irrecoverable bad debt and trap SAV depositor principal. + env(pay(borrower, loanKeylet.key, asset(1'000))); + env.close(); + } + + void + testLendingCanTradeClearedNoImpact() + { + testcase("Lending: CanTrade cleared has no impact"); + using namespace jtx; + using namespace loan; + using namespace loanBroker; + + Env env(*this, all_); + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + MPTTester mpt( + {.env = env, + .issuer = issuer, + .holders = {lender, borrower}, + .flags = tfMPTCanTransfer | tfMPTCanTrade | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTrade}); + PrettyAsset const asset = mpt.issuanceID(); + env(pay(issuer, lender, asset(10'000'000))); + env(pay(issuer, borrower, asset(100'000))); + env.close(); + + auto const broker = createVaultAndBroker(env, asset, lender); + + // Sanity: while CanTrade is set, the asset can be placed on the DEX. + env(offer(lender, XRP(1), asset(10))); + env.close(); + + // Issuer governance: clear CanTrade. Loan origination and repayment + // are not trades: nothing in the Lending Protocol should be impacted. + mpt.set({.mutableFlags = tmfMPTClearCanTrade}); + env.close(); + + // Control: clearing CanTrade is observable on the DEX path. + env(offer(lender, XRP(1), asset(10)), Ter{tecNO_PERMISSION}); + env.close(); + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + + // New cover deposits still work. + env(coverDeposit(lender, broker.brokerID, asset(100))); + env.close(); + + // New loan issuance still works. + env(loan::set(borrower, broker.brokerID, 1'000), + Sig(sfCounterpartySignature, lender), + loanSetFee); + env.close(); + auto const loanKeylet = keylet::loan(broker.brokerID, 1); + BEAST_EXPECT(env.le(loanKeylet)); + + // Repayment still works. + env(pay(borrower, loanKeylet.key, asset(1'000))); + env.close(); + + // Cover withdrawal still works. + env(coverWithdraw(lender, broker.brokerID, asset(100))); + env.close(); } #if LOAN_TODO @@ -7631,6 +7757,9 @@ public: auto const all = jtx::testableAmendments(); testCoverDepositWithdrawNonTransferableMPT(all); testCoverDepositWithdrawNonTransferableMPT(all - featureMPTokensV2); + testCoverDepositWithdrawNonTransferableMPT(all - fixCleanup3_2_0); + testLoanSetBlockedLoanPayAllowedWhenCanTransferCleared(); + testLendingCanTradeClearedNoImpact(); testPoCUnsignedUnderflowOnFullPayAfterEarlyPeriodic(); testDisabled(); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index d4767f275c..f09ee3552d 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -197,6 +198,22 @@ class MPToken_test : public beast::unit_test::Suite .metadata = "test", .err = temMALFORMED}); } + + // sfReferenceHolding is populated internally only by VaultCreate. + // A user-submitted MPTokenIssuanceCreate carrying the field must be + // rejected at preflight under fixCleanup3_2_0. + if (features[fixCleanup3_2_0]) + { + Env env{*this, features}; + env.fund(XRP(1'000), alice); + env.close(); + + json::Value jv; + jv[sfAccount] = alice.human(); + jv[sfTransactionType] = jss::MPTokenIssuanceCreate; + jv[sfReferenceHolding] = to_string(uint256{1}); + env(jv, Ter(temMALFORMED)); + } } void diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index cd4c35f1a5..af1b13abb8 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -2241,7 +2242,7 @@ class Vault_test : public beast::unit_test::Suite PrettyAsset const& asset, Vault& vault, MPTTester& mptt) { - testcase("MPT non-transferable"); + testcase("MPT non-transferable: block deposit, allow withdraw"); auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); env(tx); @@ -2251,22 +2252,19 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); - // Remove CanTransfer + // Issuer governance: clear CanTransfer. New exposure must be + // blocked, but recovery paths must remain open so existing + // depositors are not trapped. mptt.set({.mutableFlags = tmfMPTClearCanTransfer}); env.close(); + // New deposit is blocked. env(tx, Ter{tecNO_AUTH}); env.close(); + // Existing depositor can always withdraw, even though the asset + // is no longer freely transferable. tx = vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(100)}); - - env(tx, Ter{tecNO_AUTH}); - env.close(); - - // Restore CanTransfer - mptt.set({.mutableFlags = tmfMPTSetCanTransfer}); - env.close(); - env(tx); env.close(); @@ -2274,6 +2272,245 @@ class Vault_test : public beast::unit_test::Suite env(vault.del({.owner = owner, .id = keylet.key})); }); + { + testcase("MPT non-transferable: pre-fixCleanup3_2_0 withdraw blocked"); + + // Regression: before fixCleanup3_2_0 a depositor was trapped if + // the issuer cleared lsfMPTCanTransfer. Verify that the legacy + // (broken) behavior is preserved when the amendment is disabled. + Env env{*this, testableAmendments() - fixCleanup3_2_0}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const depositor{"depositor"}; + env.fund(XRP(10'000), issuer, owner, depositor); + env.close(); + Vault const vault{env}; + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTransfer}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = depositor}); + env(pay(issuer, depositor, asset(1'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)})); + env.close(); + + mptt.set({.mutableFlags = tmfMPTClearCanTransfer}); + env.close(); + + // Pre-amendment: deposit blocked (matches new behavior). + env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)}), + Ter{tecNO_AUTH}); + env.close(); + + // Pre-amendment: withdraw is also blocked - this is the bug + // that fixCleanup3_2_0 fixes. + env(vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(100)}), + Ter{tecNO_AUTH}); + env.close(); + } + + { + testcase("MPT non-transferable: vault shares inherit restriction"); + + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(10'000), issuer, owner, alice, bob); + env.close(); + Vault const vault{env}; + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTransfer}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = alice}); + mptt.authorize({.account = bob}); + env(pay(issuer, alice, asset(1'000))); + env(pay(issuer, bob, asset(1'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = alice, .id = keylet.key, .amount = asset(500)})); + // Bob also deposits so he has a share MPToken to receive into. + env(vault.deposit({.depositor = bob, .id = keylet.key, .amount = asset(500)})); + env.close(); + + auto const shares = [&]() -> PrettyAsset { + auto const sle = env.le(keylet); + BEAST_EXPECT(sle != nullptr); + return MPTIssue(sle->at(sfShareMPTID)); + }(); + + // Sanity: while CanTransfer is set on the underlying, peer-to-peer + // share transfers are allowed. + env(pay(alice, bob, shares(1))); + env.close(); + + // Issuer governance: clear CanTransfer on the underlying. + mptt.set({.mutableFlags = tmfMPTClearCanTransfer}); + env.close(); + + // Vault shares inherit the restriction: third-party share-to-share + // payments are blocked. + env(pay(alice, bob, shares(1)), Ter{tecNO_AUTH}); + env.close(); + + // Recovery path: existing share holders can still redeem shares + // for the underlying asset via VaultWithdraw. + env(vault.withdraw({.depositor = alice, .id = keylet.key, .amount = shares(1)})); + env.close(); + } + + { + testcase("MPT non-transferable: pre-fixCleanup3_2_0 share transfer succeeds"); + + // Regression: before fixCleanup3_2_0 a peer-to-peer share Payment + // succeeded even when the underlying asset's lsfMPTCanTransfer + // was cleared. Verify that the legacy (non-inheriting) behavior + // is preserved when the amendment is disabled. + Env env{*this, testableAmendments() - fixCleanup3_2_0}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(10'000), issuer, owner, alice, bob); + env.close(); + Vault const vault{env}; + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTransfer}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = alice}); + mptt.authorize({.account = bob}); + env(pay(issuer, alice, asset(1'000))); + env(pay(issuer, bob, asset(1'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = alice, .id = keylet.key, .amount = asset(500)})); + env(vault.deposit({.depositor = bob, .id = keylet.key, .amount = asset(500)})); + env.close(); + + auto const shares = [&]() -> PrettyAsset { + auto const sle = env.le(keylet); + BEAST_EXPECT(sle != nullptr); + return MPTIssue(sle->at(sfShareMPTID)); + }(); + + mptt.set({.mutableFlags = tmfMPTClearCanTransfer}); + env.close(); + + // Pre-amendment: share transfer leaks past underlying restriction. + env(pay(alice, bob, shares(1))); + env.close(); + } + + { + testcase("MPT CanTrade governance: share inherits underlying on DEX and AMM"); + + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(100'000), issuer, owner, alice, bob); + env.close(); + Vault const vault{env}; + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create( + {.flags = tfMPTCanTransfer | tfMPTCanTrade | tfMPTCanLock, + .mutableFlags = tmfMPTCanMutateCanTrade}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = alice}); + mptt.authorize({.account = bob}); + env(pay(issuer, alice, asset(10'000))); + env(pay(issuer, bob, asset(10'000))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + // Seed shares so we can later place them on trading venues. + env(vault.deposit({.depositor = alice, .id = keylet.key, .amount = asset(5'000)})); + env(vault.deposit({.depositor = bob, .id = keylet.key, .amount = asset(5'000)})); + env.close(); + + auto const shares = [&]() -> PrettyAsset { + auto const sle = env.le(keylet); + BEAST_EXPECT(sle != nullptr); + return MPTIssue(sle->at(sfShareMPTID)); + }(); + + // Sanity: while CanTrade is set on the underlying, both the asset + // and the vault share can be placed on the DEX. + env(offer(alice, XRP(1), asset(10))); + env(offer(alice, XRP(1), shares(1))); + env.close(); + + // Issuer governance: clear CanTrade on the underlying. + mptt.set({.mutableFlags = tmfMPTClearCanTrade}); + env.close(); + + // Control: clearing CanTrade on the underlying is observable on + // the DEX path for that asset. + env(offer(alice, XRP(1), asset(10)), Ter{tecNO_PERMISSION}); + env.close(); + + // Control: clearing CanTrade on the underlying is also observable + // on the AMM path for that asset. + AMM const ammUnderlyingFails( + env, alice, XRP(1'000), asset(1'000), Ter{tecNO_PERMISSION}); + + // Post-fixCleanup3_2_0: vault shares inherit the underlying's + // CanTrade restriction on the DEX path (canTrade reads the + // share's sfReferenceHolding and dispatches to the underlying). + env(offer(bob, XRP(1), shares(1)), Ter{tecNO_PERMISSION}); + env.close(); + + // checkMPTAllowed mirrors the inheritance for AMM/Offer- + // crossing/Check paths, so a share AMM also cannot be created + // when the underlying CanTrade is cleared. + AMM const ammShares(env, alice, XRP(1'000), shares(100), Ter{tecNO_PERMISSION}); + + // Deposit still works (canAddHolding does not consult the field). + env(vault.deposit({.depositor = alice, .id = keylet.key, .amount = asset(100)})); + env.close(); + + // Peer-to-peer share transfers still work (CanTransfer is set on + // both layers). + env(pay(alice, bob, shares(1))); + env.close(); + + // Withdraw still works. + env(vault.withdraw({.depositor = alice, .id = keylet.key, .amount = asset(100)})); + env.close(); + } + { testcase("MPT OutstandingAmount > MaximumAmount"); @@ -2744,16 +2981,17 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); } - env(pay(owner, charlie, shares(100))); - env.close(); - - // Charlie cannot withdraw - auto tx3 = vault.withdraw( - {.depositor = charlie, .id = keylet.key, .amount = shares(100)}); - env(tx3, Ter{terNO_RIPPLE}); - env.close(); - - env(pay(charlie, owner, shares(100))); + // Behavioural shift introduced by share inheritance: + // before fixCleanup3_2_0 this share Payment succeeded + // and the underlying IOU's NoRipple restriction surfaced + // only later on Charlie's withdrawal (terNO_RIPPLE). + // Post-amendment, canTransfer reads the share's + // sfReferenceHolding and dispatches to the underlying IOU; + // rippling is disabled between owner and charlie so the + // share payment itself is now blocked. tecPATH_DRY is + // the path-find layer's translation of the underlying + // terNO_RIPPLE under featureMPTokensV2. + env(pay(owner, charlie, shares(100)), Ter{tecPATH_DRY}); env.close(); } @@ -6140,6 +6378,406 @@ class Vault_test : public beast::unit_test::Suite runTest(amendments); } + void + testReferenceHolding() + { + using namespace test::jtx; + + auto readReferenceHolding = [&](Env const& env, + Keylet const& vaultKeylet) -> std::optional { + auto const sleVault = env.le(vaultKeylet); + if (!sleVault) + return std::nullopt; + auto const sleIssuance = env.le(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + if (!sleIssuance || !sleIssuance->isFieldPresent(sfReferenceHolding)) + return std::nullopt; + return sleIssuance->getFieldH256(sfReferenceHolding); + }; + + // Post-fixCleanup3_2_0: vault share carries sfReferenceHolding + // pointing to the vault pseudo's MPToken (for MPT-backed vaults) + // or RippleState (for IOU-backed vaults). + { + testcase("sfReferenceHolding: MPT-backed vault, post-amendment"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + auto const sleVault = env.le(keylet); + BEAST_EXPECT(sleVault != nullptr); + auto const pseudoId = sleVault->at(sfAccount); + auto const expected = keylet::mptoken(mptt.issuanceID(), pseudoId).key; + + auto const stored = readReferenceHolding(env, keylet); + BEAST_EXPECT(stored.has_value()); + BEAST_EXPECT(stored && *stored == expected); + // The pointed-to MPToken must actually exist. + BEAST_EXPECT(env.le(keylet::mptoken(mptt.issuanceID(), pseudoId)) != nullptr); + } + + { + testcase("sfReferenceHolding: IOU-backed vault, post-amendment"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env(fset(issuer, asfDefaultRipple)); + env.close(); + + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + auto const sleVault = env.le(keylet); + BEAST_EXPECT(sleVault != nullptr); + auto const pseudoId = sleVault->at(sfAccount); + auto const expected = keylet::line(pseudoId, asset.raw().get()).key; + + auto const stored = readReferenceHolding(env, keylet); + BEAST_EXPECT(stored.has_value()); + BEAST_EXPECT(stored && *stored == expected); + // The pointed-to RippleState must actually exist. + BEAST_EXPECT(env.le(keylet::line(pseudoId, asset.raw().get())) != nullptr); + } + + // XRP-backed vaults leave the field absent: XRP has no separate + // holding ledger entry and no transferability concept to inherit. + { + testcase("sfReferenceHolding: XRP-backed vault, field absent"); + Env env{*this, testableAmendments()}; + Account const owner{"owner"}; + env.fund(XRP(10'000), owner); + env.close(); + + PrettyAsset const asset{xrpIssue(), 1'000'000}; + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + BEAST_EXPECT(!readReferenceHolding(env, keylet).has_value()); + } + + // Pre-fixCleanup3_2_0: vault share has the field absent regardless + // of underlying type. + { + testcase("sfReferenceHolding: vault share, pre-amendment"); + Env env{*this, testableAmendments() - fixCleanup3_2_0}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + BEAST_EXPECT(!readReferenceHolding(env, keylet).has_value()); + } + + // Plain MPTokenIssuanceCreate (not a vault share) must never + // populate the field. Only the post-amendment case is + // interesting; pre-amendment nothing writes the field at all. + { + testcase("sfReferenceHolding: plain MPT issuance never set"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + env.fund(XRP(10'000), issuer); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + env.close(); + + auto const sleIssuance = env.le(keylet::mptIssuance(mptt.issuanceID())); + if (BEAST_EXPECT(sleIssuance)) + BEAST_EXPECT(!sleIssuance->isFieldPresent(sfReferenceHolding)); + } + } + + // Probe every transactor surface that might delete the vault pseudo- + // account's underlying holding (the MPToken or RippleState pointed to + // by sfReferenceHolding). Each scenario asserts either that the + // existing pseudo-account guards stop the deletion at preclaim, or + // that the ledger leaves the holding intact afterwards. This is a + // regression guard: if any of these guards regresses, the share's + // sfReferenceHolding pointer would dangle and the new ValidMPTIssuance + // invariant would catch it - but we want to fail much earlier, at + // the transactor's preclaim / doApply, not at invariant time. + void + testHoldingDeletionBlocked() + { + using namespace test::jtx; + + // Helper: read the share's referenced holding and confirm the + // pointed-to SLE still exists after the probe. + auto referencedHoldingExists = [&](Env const& env, Keylet const& vaultKeylet) -> bool { + auto const sleVault = env.le(vaultKeylet); + if (!sleVault) + return false; + auto const sleIssuance = env.le(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + if (!sleIssuance || !sleIssuance->isFieldPresent(sfReferenceHolding)) + return false; + auto const holdingKey = sleIssuance->getFieldH256(sfReferenceHolding); + return env.le(keylet::unchecked(holdingKey)) != nullptr; + }; + + // ---- MPT-backed vault ---------------------------------------- + { + testcase("vault pseudo MPToken: Clawback blocked by tecPSEUDO_ACCOUNT"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const depositor{"depositor"}; + env.fund(XRP(10'000), issuer, owner, depositor); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanClawback}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = depositor}); + env(pay(issuer, depositor, asset(1'000))); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(500)})); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + + Account const pseudoAccount{"vault-pseudo", env.le(keylet)->at(sfAccount)}; + // Issuer attempts to claw back the FULL underlying balance + // (500) directly from the vault pseudo-account. With the + // full amount, the doApply path would drain the pseudo's + // MPToken to zero and removeEmptyHolding would erase it - + // if doApply ever ran. SAV's pseudo-account guard at + // Clawback.cpp:201 refuses at preclaim with + // tecPSEUDO_ACCOUNT before any state change. + env(claw(issuer, asset(500), pseudoAccount), Ter{tecPSEUDO_ACCOUNT}); + env.close(); + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + // Sanity: pseudo's full balance is intact. + BEAST_EXPECT(env.balance(pseudoAccount, asset).number() == 500); + } + + { + testcase("vault pseudo MPToken: Issuer cannot Unauthorize pseudo"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = issuer, .holder = owner}); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + + auto const pseudoId = env.le(keylet)->at(sfAccount); + // Issuer attempts MPTokenAuthorize against the pseudo with + // tfMPTUnauthorize. MPTokenAuthorize.cpp blocks pseudo + // accounts via isPseudoAccount; the pseudo's MPToken is + // preserved. Construct the tx manually since the pseudo + // lacks a signing key, and the issuer-driven flavour is + // expressed via sfHolder. + json::Value jv; + jv[sfAccount] = issuer.human(); + jv[sfHolder] = toBase58(pseudoId); + jv[sfMPTokenIssuanceID] = to_string(mptt.issuanceID()); + jv[sfFlags] = tfMPTUnauthorize; + jv[sfTransactionType] = jss::MPTokenAuthorize; + env(jv, Ter{tecNO_PERMISSION}); + env.close(); + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + } + + { + testcase("vault pseudo MPToken: MPTokenIssuanceDestroy blocked while vault holds"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const depositor{"depositor"}; + env.fund(XRP(10'000), issuer, owner, depositor); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + mptt.authorize({.account = depositor}); + env(pay(issuer, depositor, asset(1'000))); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(500)})); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + + // While the vault holds outstanding underlying, the issuer + // cannot destroy the issuance. tecHAS_OBLIGATIONS confirms + // the protection - and as a side effect, the share's + // sfReferenceHolding pointer cannot be left pointing at a + // ghost issuance. + mptt.destroy({.id = mptt.issuanceID(), .err = tecHAS_OBLIGATIONS}); + env.close(); + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + } + + // ---- IOU-backed vault ---------------------------------------- + { + testcase("vault pseudo trust line: Clawback blocked by tecPSEUDO_ACCOUNT"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env(fset(issuer, asfAllowTrustLineClawback)); + env.close(); + + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env(pay(issuer, owner, asset(1'000))); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(500)})); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + + Account const pseudoAccount{"vault-pseudo", env.le(keylet)->at(sfAccount)}; + // Issuer attempts to claw back the FULL IOU balance (500) + // directly from the vault pseudo. With the full amount, the + // doApply path would drain the trust line to zero and (if + // both reserve flags clear) trustDelete would erase it - if + // doApply ever ran. The same SAV pseudo-account guard + // refuses at preclaim with tecPSEUDO_ACCOUNT. The amount's + // STAmount issuer field is the holder, per IOU clawback + // convention. + env(claw(issuer, pseudoAccount["IOU"](500)), Ter{tecPSEUDO_ACCOUNT}); + env.close(); + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + // Sanity: pseudo's full balance is intact. + BEAST_EXPECT(env.balance(pseudoAccount, asset).number() == 500); + } + + { + testcase("vault pseudo trust line: TrustSet limit=0 from issuer preserves line"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env(fset(issuer, asfDefaultRipple)); + env.close(); + + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1'000'000), owner); + env(pay(issuer, owner, asset(1'000))); + env.close(); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = owner, .id = keylet.key, .amount = asset(500)})); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + + // Issuer submits TrustSet with limit=0 against the vault + // pseudo. The pseudo's side of the line still has the + // original (non-zero) limit and a non-zero balance, so the + // line is preserved - even though the issuer cleared its + // own side. trustDelete only fires when both limits clear + // and the balance is zero. + Account const pseudoAccount{"vault-pseudo", env.le(keylet)->at(sfAccount)}; + env(trust(issuer, pseudoAccount["IOU"](0))); + env.close(); + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + } + + // ---- Positive control: VaultDelete is the only legitimate path + { + testcase("vault pseudo holding: VaultDelete is the legitimate cleanup path"); + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + env.fund(XRP(10'000), issuer, owner); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + mptt.authorize({.account = owner}); + + Vault const vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + BEAST_EXPECT(referencedHoldingExists(env, keylet)); + auto const pseudoId = env.le(keylet)->at(sfAccount); + auto const sharedMptId = env.le(keylet)->at(sfShareMPTID); + auto const holdingKeylet = keylet::mptoken(mptt.issuanceID(), pseudoId); + + // VaultDelete tears down the vault pseudo's holding, the + // share issuance, and the pseudo-account itself. Invariant + // permits this because the tx is ttVAULT_DELETE. + env(vault.del({.owner = owner, .id = keylet.key})); + env.close(); + + BEAST_EXPECT(env.le(keylet) == nullptr); + BEAST_EXPECT(env.le(holdingKeylet) == nullptr); + BEAST_EXPECT(env.le(keylet::mptIssuance(sharedMptId)) == nullptr); + } + } + public: void run() override @@ -6163,6 +6801,8 @@ public: testAssetsMaximum(); testBug6LimitBypassWithShares(); testRemoveEmptyHoldingLockedAmount(); + testReferenceHolding(); + testHoldingDeletionBlocked(); } }; diff --git a/src/tests/libxrpl/protocol_autogen/ledger_entries/MPTokenIssuanceTests.cpp b/src/tests/libxrpl/protocol_autogen/ledger_entries/MPTokenIssuanceTests.cpp index e74af94a5f..7479f0c63c 100644 --- a/src/tests/libxrpl/protocol_autogen/ledger_entries/MPTokenIssuanceTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/ledger_entries/MPTokenIssuanceTests.cpp @@ -33,6 +33,7 @@ TEST(MPTokenIssuanceTests, BuilderSettersRoundTrip) auto const previousTxnLgrSeqValue = canonical_UINT32(); auto const domainIDValue = canonical_UINT256(); auto const mutableFlagsValue = canonical_UINT32(); + auto const referenceHoldingValue = canonical_UINT256(); MPTokenIssuanceBuilder builder{ issuerValue, @@ -50,6 +51,7 @@ TEST(MPTokenIssuanceTests, BuilderSettersRoundTrip) builder.setMPTokenMetadata(mPTokenMetadataValue); builder.setDomainID(domainIDValue); builder.setMutableFlags(mutableFlagsValue); + builder.setReferenceHolding(referenceHoldingValue); builder.setLedgerIndex(index); builder.setFlags(0x1u); @@ -152,6 +154,14 @@ TEST(MPTokenIssuanceTests, BuilderSettersRoundTrip) EXPECT_TRUE(entry.hasMutableFlags()); } + { + auto const& expected = referenceHoldingValue; + auto const actualOpt = entry.getReferenceHolding(); + ASSERT_TRUE(actualOpt.has_value()); + expectEqualField(expected, *actualOpt, "sfReferenceHolding"); + EXPECT_TRUE(entry.hasReferenceHolding()); + } + EXPECT_TRUE(entry.hasLedgerIndex()); auto const ledgerIndex = entry.getLedgerIndex(); ASSERT_TRUE(ledgerIndex.has_value()); @@ -178,6 +188,7 @@ TEST(MPTokenIssuanceTests, BuilderFromSleRoundTrip) auto const previousTxnLgrSeqValue = canonical_UINT32(); auto const domainIDValue = canonical_UINT256(); auto const mutableFlagsValue = canonical_UINT32(); + auto const referenceHoldingValue = canonical_UINT256(); auto sle = std::make_shared(MPTokenIssuance::entryType, index); @@ -194,6 +205,7 @@ TEST(MPTokenIssuanceTests, BuilderFromSleRoundTrip) sle->at(sfPreviousTxnLgrSeq) = previousTxnLgrSeqValue; sle->at(sfDomainID) = domainIDValue; sle->at(sfMutableFlags) = mutableFlagsValue; + sle->at(sfReferenceHolding) = referenceHoldingValue; MPTokenIssuanceBuilder builderFromSle{sle}; EXPECT_TRUE(builderFromSle.validate()); @@ -355,6 +367,19 @@ TEST(MPTokenIssuanceTests, BuilderFromSleRoundTrip) expectEqualField(expected, *fromBuilderOpt, "sfMutableFlags"); } + { + auto const& expected = referenceHoldingValue; + + auto const fromSleOpt = entryFromSle.getReferenceHolding(); + auto const fromBuilderOpt = entryFromBuilder.getReferenceHolding(); + + ASSERT_TRUE(fromSleOpt.has_value()); + ASSERT_TRUE(fromBuilderOpt.has_value()); + + expectEqualField(expected, *fromSleOpt, "sfReferenceHolding"); + expectEqualField(expected, *fromBuilderOpt, "sfReferenceHolding"); + } + EXPECT_EQ(entryFromSle.getKey(), index); EXPECT_EQ(entryFromBuilder.getKey(), index); } @@ -433,5 +458,7 @@ TEST(MPTokenIssuanceTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(entry.getDomainID().has_value()); EXPECT_FALSE(entry.hasMutableFlags()); EXPECT_FALSE(entry.getMutableFlags().has_value()); + EXPECT_FALSE(entry.hasReferenceHolding()); + EXPECT_FALSE(entry.getReferenceHolding().has_value()); } }