From 1a98182e23ec17b508fa6a2de6f4aef0600218c9 Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 21 May 2026 18:52:41 +0100 Subject: [PATCH 1/6] refactor: Remove dead `fetchBatch` code (#7309) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- include/xrpl/nodestore/Backend.h | 4 --- .../xrpl/nodestore/detail/DatabaseNodeImp.h | 3 -- src/libxrpl/nodestore/DatabaseNodeImp.cpp | 34 ------------------- .../nodestore/backend/MemoryFactory.cpp | 23 ------------- src/libxrpl/nodestore/backend/NuDBFactory.cpp | 23 ------------- src/libxrpl/nodestore/backend/NullFactory.cpp | 8 ----- .../nodestore/backend/RocksDBFactory.cpp | 24 ------------- 7 files changed, 119 deletions(-) diff --git a/include/xrpl/nodestore/Backend.h b/include/xrpl/nodestore/Backend.h index 124cd99db5..0061890237 100644 --- a/include/xrpl/nodestore/Backend.h +++ b/include/xrpl/nodestore/Backend.h @@ -83,10 +83,6 @@ public: virtual Status fetch(uint256 const& hash, std::shared_ptr* pObject) = 0; - /** Fetch a batch synchronously. */ - virtual std::pair>, Status> - fetchBatch(std::vector const& hashes) = 0; - /** Store a single object. Depending on the implementation this may happen immediately or deferred using a scheduled task. diff --git a/include/xrpl/nodestore/detail/DatabaseNodeImp.h b/include/xrpl/nodestore/detail/DatabaseNodeImp.h index add66c51a7..dd94b27075 100644 --- a/include/xrpl/nodestore/detail/DatabaseNodeImp.h +++ b/include/xrpl/nodestore/detail/DatabaseNodeImp.h @@ -67,9 +67,6 @@ public: backend_->sync(); } - std::vector> - fetchBatch(std::vector const& hashes); - void asyncFetch( uint256 const& hash, diff --git a/src/libxrpl/nodestore/DatabaseNodeImp.cpp b/src/libxrpl/nodestore/DatabaseNodeImp.cpp index ef84862de6..a51c34079b 100644 --- a/src/libxrpl/nodestore/DatabaseNodeImp.cpp +++ b/src/libxrpl/nodestore/DatabaseNodeImp.cpp @@ -4,21 +4,16 @@ #include #include #include -#include -#include #include #include #include #include -#include -#include #include #include #include #include #include -#include namespace xrpl::NodeStore { @@ -81,33 +76,4 @@ DatabaseNodeImp::fetchNodeObject( return nodeObject; } -std::vector> -DatabaseNodeImp::fetchBatch(std::vector const& hashes) -{ - using namespace std::chrono; - auto const before = steady_clock::now(); - - // Get the node objects that match the hashes from the backend. To protect - // against the backends returning fewer or more results than expected, the - // container is resized to the number of hashes. - auto results = backend_->fetchBatch(hashes).first; - XRPL_ASSERT( - results.size() == hashes.size() || results.empty(), - "number of output objects either matches number of input hashes or is empty"); - results.resize(hashes.size()); - for (size_t i = 0; i < results.size(); ++i) - { - if (!results[i]) - { - JLOG(j_.error()) << "fetchBatch - " - << "record not found in db. hash = " << strHex(hashes[i]); - } - } - - auto fetchDurationUs = - std::chrono::duration_cast(steady_clock::now() - before).count(); - updateFetchMetrics(hashes.size(), 0, fetchDurationUs); - return results; -} - } // namespace xrpl::NodeStore diff --git a/src/libxrpl/nodestore/backend/MemoryFactory.cpp b/src/libxrpl/nodestore/backend/MemoryFactory.cpp index 621ad1357c..5bdf8e65b5 100644 --- a/src/libxrpl/nodestore/backend/MemoryFactory.cpp +++ b/src/libxrpl/nodestore/backend/MemoryFactory.cpp @@ -22,7 +22,6 @@ #include #include #include -#include namespace xrpl::NodeStore { @@ -146,28 +145,6 @@ public: return Status::Ok; } - std::pair>, Status> - fetchBatch(std::vector const& hashes) override - { - std::vector> results; - results.reserve(hashes.size()); - for (auto const& h : hashes) - { - std::shared_ptr nObj; - Status const status = fetch(h, &nObj); - if (status != Status::Ok) - { - results.push_back({}); - } - else - { - results.push_back(nObj); - } - } - - return {results, Status::Ok}; - } - void store(std::shared_ptr const& object) override { diff --git a/src/libxrpl/nodestore/backend/NuDBFactory.cpp b/src/libxrpl/nodestore/backend/NuDBFactory.cpp index d026dc254c..abf09e871d 100644 --- a/src/libxrpl/nodestore/backend/NuDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/NuDBFactory.cpp @@ -42,7 +42,6 @@ #include #include #include -#include namespace xrpl::NodeStore { @@ -232,28 +231,6 @@ public: return status; } - std::pair>, Status> - fetchBatch(std::vector const& hashes) override - { - std::vector> results; - results.reserve(hashes.size()); - for (auto const& h : hashes) - { - std::shared_ptr nObj; - Status const status = fetch(h, &nObj); - if (status != Status::Ok) - { - results.push_back({}); - } - else - { - results.push_back(nObj); - } - } - - return {results, Status::Ok}; - } - void doInsert(std::shared_ptr const& no) { diff --git a/src/libxrpl/nodestore/backend/NullFactory.cpp b/src/libxrpl/nodestore/backend/NullFactory.cpp index ae33b7daa2..36b8139984 100644 --- a/src/libxrpl/nodestore/backend/NullFactory.cpp +++ b/src/libxrpl/nodestore/backend/NullFactory.cpp @@ -12,8 +12,6 @@ #include #include #include -#include -#include namespace xrpl::NodeStore { @@ -52,12 +50,6 @@ public: return Status::NotFound; } - std::pair>, Status> - fetchBatch(std::vector const& hashes) override - { - return {}; - } - void store(std::shared_ptr const& object) override { diff --git a/src/libxrpl/nodestore/backend/RocksDBFactory.cpp b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp index c1baf6aaaa..d2c193888c 100644 --- a/src/libxrpl/nodestore/backend/RocksDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp @@ -29,8 +29,6 @@ #include #include #include -#include -#include #if XRPL_ROCKSDB_AVAILABLE #include @@ -330,28 +328,6 @@ public: return status; } - std::pair>, Status> - fetchBatch(std::vector const& hashes) override - { - std::vector> results; - results.reserve(hashes.size()); - for (auto const& h : hashes) - { - std::shared_ptr nObj; - Status const status = fetch(h, &nObj); - if (status != Status::Ok) - { - results.push_back({}); - } - else - { - results.push_back(nObj); - } - } - - return {results, Status::Ok}; - } - void store(std::shared_ptr const& object) override { From 3547a9335f148fa85c0e23831612fb3f872e9b04 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Thu, 21 May 2026 14:29:53 -0400 Subject: [PATCH 2/6] fix: Add assorted MPT/DEX fixes (#7040) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> --- include/xrpl/ledger/helpers/MPTokenHelpers.h | 22 +- include/xrpl/ledger/helpers/TokenHelpers.h | 6 + include/xrpl/tx/invariants/InvariantCheck.h | 3 +- include/xrpl/tx/invariants/MPTInvariant.h | 38 ++ src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 116 +---- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 24 + src/libxrpl/protocol/STPathSet.cpp | 10 +- src/libxrpl/tx/invariants/AMMInvariant.cpp | 33 +- src/libxrpl/tx/invariants/InvariantCheck.cpp | 2 +- src/libxrpl/tx/invariants/MPTInvariant.cpp | 150 +++++- src/libxrpl/tx/paths/BookStep.cpp | 35 +- src/libxrpl/tx/paths/MPTEndpointStep.cpp | 12 +- .../tx/transactors/check/CheckCash.cpp | 5 +- .../tx/transactors/check/CheckCreate.cpp | 20 +- src/libxrpl/tx/transactors/dex/AMMCreate.cpp | 34 +- src/libxrpl/tx/transactors/dex/AMMDeposit.cpp | 29 +- .../tx/transactors/dex/AMMWithdraw.cpp | 29 +- .../tx/transactors/dex/OfferCreate.cpp | 20 +- .../tx/transactors/token/MPTokenAuthorize.cpp | 5 +- src/test/app/AMMExtendedMPT_test.cpp | 6 +- src/test/app/AMMMPT_test.cpp | 90 ++-- src/test/app/CheckMPT_test.cpp | 8 +- src/test/app/ClawbackMPT_test.cpp | 435 +++++++++++++++++ src/test/app/Invariants_test.cpp | 184 +++++++ src/test/app/MPToken_test.cpp | 458 ++++++++++++------ src/test/app/OfferMPT_test.cpp | 184 ++++++- src/test/app/Vault_test.cpp | 81 +++- 27 files changed, 1618 insertions(+), 421 deletions(-) create mode 100644 src/test/app/ClawbackMPT_test.cpp diff --git a/include/xrpl/ledger/helpers/MPTokenHelpers.h b/include/xrpl/ledger/helpers/MPTokenHelpers.h index 491bd1933e..c709badab8 100644 --- a/include/xrpl/ledger/helpers/MPTokenHelpers.h +++ b/include/xrpl/ledger/helpers/MPTokenHelpers.h @@ -171,6 +171,15 @@ canTransfer( [[nodiscard]] TER canTrade(ReadView const& view, Asset const& asset, std::uint8_t depth = 0); +/** Convenience to combine canTrade/Transfer. Returns tesSUCCESS if Asset is Issue. + */ +[[nodiscard]] TER +canMPTTradeAndTransfer( + ReadView const& v, + Asset const& asset, + AccountID const& from, + AccountID const& to); + //------------------------------------------------------------------------------ // // Empty holding operations (MPT-specific) @@ -277,17 +286,4 @@ issuerFundsToSelfIssue(ReadView const& view, MPTIssue const& issue); void issuerSelfDebitHookMPT(ApplyView& view, MPTIssue const& issue, std::uint64_t amount); -//------------------------------------------------------------------------------ -// -// MPT DEX -// -//------------------------------------------------------------------------------ - -/* Return true if a transaction is allowed for the specified MPT/account. The - * function checks MPTokenIssuance and MPToken objects flags to determine if the - * transaction is allowed. - */ -TER -checkMPTTxAllowed(ReadView const& v, TxType tx, Asset const& asset, AccountID const& accountID); - } // namespace xrpl diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index b47286a5d5..f736e51d28 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -63,9 +63,15 @@ enum class AuthType { StrongAuth, WeakAuth, Legacy }; [[nodiscard]] bool isGlobalFrozen(ReadView const& view, Asset const& asset); +[[nodiscard]] TER +checkGlobalFrozen(ReadView const& view, Asset const& asset); + [[nodiscard]] bool isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); +[[nodiscard]] TER +checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); + /** * isFrozen check is recursive for MPT shares in a vault, descending to * assets in the vault, up to maxAssetCheckDepth recursion depth. This is diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index a58ac0df50..5996039bf0 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -401,7 +401,8 @@ using InvariantChecks = std::tuple< ValidLoanBroker, ValidLoan, ValidVault, - ValidMPTPayment>; + ValidMPTPayment, + ValidMPTTransfer>; /** * @brief get a tuple of all invariant checks diff --git a/include/xrpl/tx/invariants/MPTInvariant.h b/include/xrpl/tx/invariants/MPTInvariant.h index f2b0a18131..becb752126 100644 --- a/include/xrpl/tx/invariants/MPTInvariant.h +++ b/include/xrpl/tx/invariants/MPTInvariant.h @@ -71,4 +71,42 @@ public: finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); }; +class ValidMPTTransfer +{ + struct Value + { + std::optional amtBefore; + std::optional amtAfter; + }; + // MPTID: {holder: Value} + hash_map> amount_; + // Deleted MPToken + // MPToken key: true if MPTAuthorized is set + hash_map deletedAuthorized_; + +public: + void + visitEntry(bool, std::shared_ptr const&, std::shared_ptr const&); + + bool + finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); + +private: + /** + * @brief Check whether a holder is authorized to send or receive an MPToken. + * + * Deleted MPToken SLEs are no longer present in the view by the time + * finalize() runs, so their authorization state is captured during + * visitEntry() and stored in deletedAuthorized_. For deleted MPTokens, + * returns true if reqAuth is false or lsfMPTAuthorized was set at deletion. + * For existing MPTokens, returns the result of requireAuth() + */ + [[nodiscard]] bool + isAuthorized( + ReadView const& view, + MPTID const& mptid, + AccountID const& holder, + bool requireAuth) const; +}; + } // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 542a5b60ed..387116d820 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -383,10 +382,11 @@ requireAuth( // belong to someone who is explicitly authorized e.g. a vault owner. } - if (featureSAVEnabled) + bool const featureMPTV2Enabled = view.rules().enabled(featureMPTokensV2); + if (featureSAVEnabled || featureMPTV2Enabled) { - // Implicitly authorize Vault and LoanBroker pseudo-accounts - if (isPseudoAccount(view, account, {&sfVaultID, &sfLoanBrokerID})) + // Implicitly authorize Vault, LoanBroker, and AMM pseudo-accounts + if (isPseudoAccount(view, account, {&sfVaultID, &sfLoanBrokerID, &sfAMMID})) return tesSUCCESS; } @@ -618,6 +618,22 @@ canTrade(ReadView const& view, Asset const& asset, std::uint8_t depth) }); } +TER +canMPTTradeAndTransfer( + ReadView const& view, + Asset const& asset, + AccountID const& from, + AccountID const& to) +{ + if (!asset.holds()) + return tesSUCCESS; + + if (auto const ter = canTrade(view, asset); !isTesSuccess(ter)) + return ter; + + return canTransfer(view, asset, from, to); +} + TER lockEscrowMPT(ApplyView& view, AccountID const& sender, STAmount const& amount, beast::Journal j) { @@ -985,96 +1001,4 @@ issuerSelfDebitHookMPT(ApplyView& view, MPTIssue const& issue, std::uint64_t amo view.issuerSelfDebitHookMPT(issue, amount, available); } -static TER -checkMPTAllowed( - ReadView const& view, - TxType txType, - Asset const& asset, - AccountID const& accountID, - std::uint8_t depth = 0) -{ - if (!asset.holds()) - return tesSUCCESS; - - auto const& issuanceID = asset.get().getMptID(); - auto const validTx = txType == ttAMM_CREATE || txType == ttAMM_DEPOSIT || - txType == ttAMM_WITHDRAW || txType == ttOFFER_CREATE || txType == ttCHECK_CREATE || - txType == ttCHECK_CASH || txType == ttPAYMENT; - XRPL_ASSERT(validTx, "xrpl::checkMPTAllowed : all MPT tx or DEX"); - if (!validTx) - return tefINTERNAL; // LCOV_EXCL_LINE - - auto const& issuer = asset.getIssuer(); - if (!view.exists(keylet::account(issuer))) - return tecNO_ISSUER; // LCOV_EXCL_LINE - - auto const issuanceKey = keylet::mptIssuance(issuanceID); - auto const issuanceSle = view.read(issuanceKey); - if (!issuanceSle) - return tecOBJECT_NOT_FOUND; // LCOV_EXCL_LINE - - if (issuanceSle->isFlag(lsfMPTLocked)) - return tecLOCKED; // LCOV_EXCL_LINE - // Offer crossing and Payment - if (!issuanceSle->isFlag(lsfMPTCanTrade)) - return tecNO_PERMISSION; - - if (accountID != issuer) - { - if (!issuanceSle->isFlag(lsfMPTCanTransfer)) - return tecNO_PERMISSION; - - auto const mptSle = view.read(keylet::mptoken(issuanceKey.key, accountID)); - // Allow to succeed since some tx create MPToken if it doesn't exist. - // Tx's have their own check for missing MPToken. - if (!mptSle) - return tesSUCCESS; - - 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; -} - -TER -checkMPTTxAllowed( - ReadView const& view, - TxType txType, - Asset const& asset, - AccountID const& accountID) -{ - // use isDEXAllowed for payment/offer crossing - XRPL_ASSERT(txType != ttPAYMENT, "xrpl::checkMPTTxAllowed : not payment"); - return checkMPTAllowed(view, txType, asset, accountID); -} - } // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 71019761ed..7191456868 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -55,6 +56,14 @@ isGlobalFrozen(ReadView const& view, Asset const& asset) [&](MPTIssue const& issue) { return isGlobalFrozen(view, issue); }); } +TER +checkGlobalFrozen(ReadView const& view, Asset const& asset) +{ + if (isGlobalFrozen(view, asset)) + return asset.holds() ? tecLOCKED : tecFROZEN; + return tesSUCCESS; +} + bool isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset) { @@ -62,6 +71,14 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& [&](auto const& issue) { return isIndividualFrozen(view, account, issue); }, asset.value()); } +TER +checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset) +{ + if (isIndividualFrozen(view, account, asset)) + return asset.holds() ? tecLOCKED : tecFROZEN; + return tesSUCCESS; +} + bool isFrozen(ReadView const& view, AccountID const& account, Asset const& asset, std::uint8_t depth) { @@ -1105,6 +1122,13 @@ directSendNoFeeMPT( auto const mptokenID = keylet::mptoken(mptID.key, uReceiverID); if (auto sle = view.peek(mptokenID)) { + if (view.rules().enabled(featureMPTokensV2)) + { + if ((*sle)[sfMPTAmount] > (std::numeric_limits::max() - amt)) + { + return tecINTERNAL; // LCOV_EXCL_LINE + } + } view.creditHookMPT(uSenderID, uReceiverID, saAmount, (*sle)[sfMPTAmount], available); (*sle)[sfMPTAmount] += amt; view.update(sle); diff --git a/src/libxrpl/protocol/STPathSet.cpp b/src/libxrpl/protocol/STPathSet.cpp index 35d5fd782b..cb70ac36fe 100644 --- a/src/libxrpl/protocol/STPathSet.cpp +++ b/src/libxrpl/protocol/STPathSet.cpp @@ -90,8 +90,12 @@ STPathSet::STPathSet(SerialIter& sit, SField const& name) : STBase(name) if (hasAccount) account = sit.get160(); - XRPL_ASSERT( - !(hasCurrency && hasMPT), "xrpl::STPathSet::STPathSet : not has Currency and MPT"); + if (hasCurrency && hasMPT) + { + JLOG(debugLog().error()) << "Bad path element MPT and Currency in pathset"; + Throw("bad path element: MPT and Currency"); + } + if (hasCurrency) asset = Currency::fromRaw(sit.get160()); @@ -101,7 +105,7 @@ STPathSet::STPathSet(SerialIter& sit, SField const& name) : STBase(name) if (hasIssuer) issuer = sit.get160(); - path.emplace_back(account, asset, issuer, hasCurrency); + path.emplace_back(account, asset, issuer, hasCurrency || hasMPT); } } } diff --git a/src/libxrpl/tx/invariants/AMMInvariant.cpp b/src/libxrpl/tx/invariants/AMMInvariant.cpp index e13684eca5..be2a803e93 100644 --- a/src/libxrpl/tx/invariants/AMMInvariant.cpp +++ b/src/libxrpl/tx/invariants/AMMInvariant.cpp @@ -45,7 +45,8 @@ ValidAMM::visitEntry( // AMM pool changed else if ( (type == ltRIPPLE_STATE && after->isFlag(lsfAMMNode)) || - (type == ltACCOUNT_ROOT && after->isFieldPresent(sfAMMID))) + (type == ltACCOUNT_ROOT && after->isFieldPresent(sfAMMID)) || + (type == ltMPTOKEN && after->isFlag(lsfMPTAMM))) { ammPoolChanged_ = true; } @@ -85,9 +86,9 @@ ValidAMM::finalizeVote(bool enforce, beast::Journal const& j) const { // LPTokens and the pool can not change on vote // LCOV_EXCL_START - JLOG(j.error()) << "AMMVote invariant failed: " << lptAMMBalanceBefore_.value_or(STAmount{}) - << " " << lptAMMBalanceAfter_.value_or(STAmount{}) << " " - << ammPoolChanged_; + JLOG(j.error()) << "Invariant failed: AMMVote failed, " + << lptAMMBalanceBefore_.value_or(STAmount{}) << " " + << lptAMMBalanceAfter_.value_or(STAmount{}) << " " << ammPoolChanged_; if (enforce) return false; // LCOV_EXCL_STOP @@ -103,7 +104,7 @@ ValidAMM::finalizeBid(bool enforce, beast::Journal const& j) const { // The pool can not change on bid // LCOV_EXCL_START - JLOG(j.error()) << "AMMBid invariant failed: pool changed"; + JLOG(j.error()) << "Invariant failed: AMMBid failed, pool changed"; if (enforce) return false; // LCOV_EXCL_STOP @@ -114,7 +115,7 @@ ValidAMM::finalizeBid(bool enforce, beast::Journal const& j) const (*lptAMMBalanceAfter_ > *lptAMMBalanceBefore_ || *lptAMMBalanceAfter_ <= beast::kZero)) { // LCOV_EXCL_START - JLOG(j.error()) << "AMMBid invariant failed: " << *lptAMMBalanceBefore_ << " " + JLOG(j.error()) << "Invariant failed: AMMBid failed, " << *lptAMMBalanceBefore_ << " " << *lptAMMBalanceAfter_; if (enforce) return false; @@ -134,7 +135,7 @@ ValidAMM::finalizeCreate( if (!ammAccount_) { // LCOV_EXCL_START - JLOG(j.error()) << "AMMCreate invariant failed: AMM object is not created"; + JLOG(j.error()) << "Invariant failed: AMMCreate failed, AMM object is not created"; if (enforce) return false; // LCOV_EXCL_STOP @@ -157,8 +158,8 @@ ValidAMM::finalizeCreate( if (!validBalances(amount, amount2, *lptAMMBalanceAfter_, ZeroAllowed::No) || ammLPTokens(amount, amount2, lptAMMBalanceAfter_->get()) != *lptAMMBalanceAfter_) { - JLOG(j.error()) << "AMMCreate invariant failed: " << amount << " " << amount2 << " " - << *lptAMMBalanceAfter_; + JLOG(j.error()) << "Invariant failed: AMMCreate failed, " << amount << " " << amount2 + << " " << *lptAMMBalanceAfter_; if (enforce) return false; } @@ -176,7 +177,7 @@ ValidAMM::finalizeDelete(bool enforce, TER res, beast::Journal const& j) const // LCOV_EXCL_START std::string const msg = (isTesSuccess(res)) ? "AMM object is not deleted on tesSUCCESS" : "AMM object is changed on tecINCOMPLETE"; - JLOG(j.error()) << "AMMDelete invariant failed: " << msg; + JLOG(j.error()) << "Invariant failed: AMMDelete failed, " << msg; if (enforce) return false; // LCOV_EXCL_STOP @@ -191,7 +192,7 @@ ValidAMM::finalizeDEX(bool enforce, beast::Journal const& j) const if (ammAccount_) { // LCOV_EXCL_START - JLOG(j.error()) << "AMM swap invariant failed: AMM object changed"; + JLOG(j.error()) << "Invariant failed: AMM swap failed, AMM object changed"; if (enforce) return false; // LCOV_EXCL_STOP @@ -232,10 +233,10 @@ ValidAMM::generalInvariant( }; if (!nonNegativeBalances || (!strongInvariantCheck && !weakInvariantCheck())) { - JLOG(j.error()) << "AMM " << tx.getTxnType() - << " invariant failed: " << tx.getHash(HashPrefix::TransactionId) << " " - << ammPoolChanged_ << " " << amount << " " << amount2 << " " - << poolProductMean << " " << lptAMMBalanceAfter_->getText() << " " + JLOG(j.error()) << "Invariant failed: AMM " << tx.getTxnType() << " " + << tx.getHash(HashPrefix::TransactionId) << " " << ammPoolChanged_ << " " + << amount << " " << amount2 << " " << poolProductMean << " " + << lptAMMBalanceAfter_->getText() << " " << ((*lptAMMBalanceAfter_ == beast::kZero) ? Number{1} : ((*lptAMMBalanceAfter_ - poolProductMean) / poolProductMean)); @@ -256,7 +257,7 @@ ValidAMM::finalizeDeposit( if (!ammAccount_) { // LCOV_EXCL_START - JLOG(j.error()) << "AMMDeposit invariant failed: AMM object is deleted"; + JLOG(j.error()) << "Invariant failed: AMMDeposit failed, AMM object is deleted"; if (enforce) return false; // LCOV_EXCL_STOP diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 2019e378e2..3e51b6f877 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -840,7 +840,7 @@ ValidClawback::finalize( [&](MPTIssue const& issue) { return accountHolds( view, - issuer, + holder, issue, FreezeHandling::IgnoreFreeze, AuthHandling::IgnoreAuth, diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index ecba0b6227..635af25b61 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -442,11 +443,11 @@ ValidMPTPayment::finalize( { if (isTesSuccess(result)) { - bool const enforce = view.rules().enabled(featureMPTokensV2); + bool const invariantPasses = !view.rules().enabled(featureMPTokensV2); if (overflow_) { JLOG(j.fatal()) << "Invariant failed: OutstandingAmount overflow"; - return !enforce; + return invariantPasses; } auto const signedMax = static_cast(kMaxMpTokenAmount); @@ -464,7 +465,7 @@ ValidMPTPayment::finalize( JLOG(j.fatal()) << "Invariant failed: invalid OutstandingAmount balance " << data.outstanding[kIBefore] << " " << data.outstanding[kIAfter] << " " << data.mptAmount; - return !enforce; + return invariantPasses; } } } @@ -472,4 +473,147 @@ ValidMPTPayment::finalize( return true; } +void +ValidMPTTransfer::visitEntry( + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + // Record the before/after MPTAmount for each (issuanceID, account) pair + // so finalize() can determine whether a transfer actually occurred. + auto update = [&](SLE const& sle, bool isBefore) { + if (sle.getType() == ltMPTOKEN) + { + auto const issuanceID = sle[sfMPTokenIssuanceID]; + auto const account = sle[sfAccount]; + auto const amount = sle[sfMPTAmount]; + if (isBefore) + { + amount_[issuanceID][account].amtBefore = amount; + } + else + { + amount_[issuanceID][account].amtAfter = amount; + } + if (isDelete && isBefore) + { + deletedAuthorized_[sle.key()] = sle.isFlag(lsfMPTAuthorized); + } + } + }; + + if (before) + update(*before, true); + + if (after) + update(*after, false); +} + +bool +ValidMPTTransfer::isAuthorized( + ReadView const& view, + MPTID const& mptid, + AccountID const& holder, + bool reqAuth) const +{ + auto const key = keylet::mptoken(mptid, holder); + auto const it = deletedAuthorized_.find(key.key); + if (it != deletedAuthorized_.end()) + return !reqAuth || it->second; + return isTesSuccess(requireAuth(view, MPTIssue{mptid}, holder)); +} + +bool +ValidMPTTransfer::finalize( + STTx const& tx, + TER const, + XRPAmount const, + ReadView const& view, + beast::Journal const& j) +{ + if (hasPrivilege(tx, OverrideFreeze)) + return true; + + // DEX transactions (AMM[Create,Deposit], cross-currency payments, offer creates) are + // subject to the MPTCanTrade flag in addition to the standard transfer rules. + // A payment is only DEX if it is a cross-currency payment. + auto const txnType = tx.getTxnType(); + auto const isDEX = [&] { + if (txnType == ttPAYMENT) + { + // A payment is cross-currency (and thus DEX) only if SendMax is present + // and its asset differs from the destination asset. + auto const amount = tx[sfAmount]; + return tx[~sfSendMax].value_or(amount).asset() != amount.asset(); + } + return txnType == ttAMM_CREATE || txnType == ttAMM_DEPOSIT || txnType == ttOFFER_CREATE; + }(); + + // Only enforce once MPTokensV2 is enabled to preserve consensus with non-V2 nodes. + // Log invariant failure error even if MPTokensV2 is disabled. + auto const invariantPasses = !view.rules().enabled(featureMPTokensV2); + + for (auto const& [mptID, values] : amount_) + { + std::uint16_t senders = 0; + std::uint16_t receivers = 0; + bool invalidTransfer = false; + auto const sleIssuance = view.read(keylet::mptIssuance(mptID)); + if (!sleIssuance) + { + continue; + } + + // These transactions are recovery/settlement paths. They may move an + // existing MPT position even after the issuer clears CanTransfer, so + // holders are not trapped in AMM, vault, or loan protocol accounts. + auto const waivesCanTransfer = txnType == ttAMM_WITHDRAW || + (view.rules().enabled(fixCleanup3_2_0) && + (txnType == ttVAULT_WITHDRAW || txnType == ttLOAN_BROKER_COVER_WITHDRAW || + txnType == ttLOAN_PAY)); + auto const canTransfer = sleIssuance->isFlag(lsfMPTCanTransfer) || waivesCanTransfer; + auto const canTrade = sleIssuance->isFlag(lsfMPTCanTrade); + auto const reqAuth = sleIssuance->isFlag(lsfMPTRequireAuth); + + for (auto const& [account, value] : values) + { + // Classify each account as a sender or receiver based on whether their MPTAmount + // decreased or increased. Count new MPToken holders (no amtBefore) as receivers. + // Skip deleted MPToken holders (amtAfter is nullopt); deletion requires zero balance. + if (value.amtAfter.has_value() && value.amtBefore.value_or(0) != *value.amtAfter) + { + if (!value.amtBefore.has_value() || *value.amtAfter > *value.amtBefore) + { + ++receivers; + } + else + { + ++senders; + } + + // Check once: if any involved account is frozen, the whole + // issuance transfer is considered frozen. Only need to check for + // frozen if there is a transfer of funds. + if (!invalidTransfer && + (isFrozen(view, account, MPTIssue{mptID}) || + !isAuthorized(view, mptID, account, reqAuth))) + { + invalidTransfer = true; + } + } + } + // A transfer between holders has occurred (senders > 0 && receivers > 0). + // Fail if the issuance is frozen, does not permit transfers, or — for + // DEX transactions — does not permit trading. + if ((invalidTransfer || !canTransfer || (isDEX && !canTrade)) && senders > 0 && + receivers > 0) + { + JLOG(j.fatal()) << "Invariant failed: invalid MPToken transfer between holders"; + return invariantPasses; + } + } + + return true; +} + } // namespace xrpl diff --git a/src/libxrpl/tx/paths/BookStep.cpp b/src/libxrpl/tx/paths/BookStep.cpp index 0391aa5f77..5cc2a987b8 100644 --- a/src/libxrpl/tx/paths/BookStep.cpp +++ b/src/libxrpl/tx/paths/BookStep.cpp @@ -1361,19 +1361,18 @@ BookStep::check(StrandContext const& ctx) const return terNO_RIPPLE; return std::nullopt; }, - [&](MPTIssue const& issue) -> std::optional { - // Check if can trade on DEX. - if (auto const ter = canTrade(view, book_.in); !isTesSuccess(ter)) - return ter; - if (auto const ter = canTrade(view, book_.out); !isTesSuccess(ter)) - return ter; - return std::nullopt; - }); + [&](MPTIssue const& issue) -> std::optional { return std::nullopt; }); if (err) return *err; } } + // Check if the offer can be traded on DEX. + if (auto const ter = canTrade(ctx.view, book_.in); !isTesSuccess(ter)) + return ter; + if (auto const ter = canTrade(ctx.view, book_.out); !isTesSuccess(ter)) + return ter; + return tesSUCCESS; } @@ -1384,12 +1383,22 @@ BookStep::rate( Asset const& asset, AccountID const& dstAccount) const { - auto const& issuer = asset.getIssuer(); - if (isXRP(issuer) || issuer == dstAccount) - return kParityRate; return asset.visit( - [&](Issue const&) { return transferRate(view, issuer); }, - [&](MPTIssue const& issue) { return transferRate(view, issue.getMptID()); }); + [&](Issue const& issue) -> Rate { + if (isXRP(issue.account) || issue.account == dstAccount) + return kParityRate; + return transferRate(view, issue.account); + }, + [&](MPTIssue const& mptIssue) -> Rate { + // For MPT, parity applies only when this asset is the final strand + // delivery AND the destination is the MPT issuer (holder → issuer, + // which is fee-free). Using strandDst_ alone is wrong because it + // incorrectly suppresses the fee when MPT is an intermediate or + // the in-side of a book that precedes the issuer's XRP receipt. + if (asset == strandDeliver_ && mptIssue.getIssuer() == dstAccount) + return kParityRate; + return transferRate(view, mptIssue.getMptID()); + }); }; template diff --git a/src/libxrpl/tx/paths/MPTEndpointStep.cpp b/src/libxrpl/tx/paths/MPTEndpointStep.cpp index 65f8409379..5595f8ea65 100644 --- a/src/libxrpl/tx/paths/MPTEndpointStep.cpp +++ b/src/libxrpl/tx/paths/MPTEndpointStep.cpp @@ -395,6 +395,9 @@ MPTEndpointPaymentStep::check(StrandContext const& ctx, std::shared_ptr const&) { + // The standard checks are all we can do because any remaining checks + // require the existence of a MPToken. Offer crossing does not + // require a pre-existing MPToken. return tesSUCCESS; } @@ -838,10 +841,17 @@ MPTEndpointStep::check(StrandContext const& ctx) const } // pure issue/redeem can't be frozen (issuer/holder) + // For the first step: check global freeze of the step's own asset. + // For the last step: check only the per-holder MPToken lock. + // Global freeze of the deliver asset is not checked here + // because MPT semantics allow issuer<->holder transfers even when globally + // locked — only holder-to-holder DEX paths are restricted. if (!(ctx.isLast && ctx.isFirst)) { auto const& account = ctx.isFirst ? src_ : dst_; - if (isFrozen(ctx.view, account, mptIssue_)) + bool const frozen = (ctx.isFirst && isGlobalFrozen(ctx.view, mptIssue_)) || + isIndividualFrozen(ctx.view, account, mptIssue_); + if (frozen) return terLOCKED; } diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index f8dbae66fc..cfc133b501 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -264,9 +264,10 @@ CheckCash::preclaim(PreclaimContext const& ctx) return tecLOCKED; } - if (auto const err = canTrade(ctx.view, value.asset()); !isTesSuccess(err)) + if (auto const err = canTransfer(ctx.view, issue, srcId, dstId); + !isTesSuccess(err)) { - JLOG(ctx.j.warn()) << "MPT DEX is not allowed."; + JLOG(ctx.j.warn()) << "MPT transfer is disabled."; return err; } diff --git a/src/libxrpl/tx/transactors/check/CheckCreate.cpp b/src/libxrpl/tx/transactors/check/CheckCreate.cpp index 6373d7e343..d2d84536de 100644 --- a/src/libxrpl/tx/transactors/check/CheckCreate.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCreate.cpp @@ -111,10 +111,10 @@ CheckCreate::preclaim(PreclaimContext const& ctx) { // The currency may not be globally frozen AccountID const& issuerId{sendMax.getIssuer()}; - if (isGlobalFrozen(ctx.view, sendMax.asset())) + if (auto const ter = checkGlobalFrozen(ctx.view, sendMax.asset()); !isTesSuccess(ter)) { - JLOG(ctx.j.warn()) << "Creating a check for frozen asset"; - return sendMax.asset().holds() ? tecLOCKED : tecFROZEN; + JLOG(ctx.j.warn()) << "Creating a check for frozen or locked asset"; + return ter; } auto const err = sendMax.asset().visit( [&](Issue const& issue) -> std::optional { @@ -153,9 +153,21 @@ CheckCreate::preclaim(PreclaimContext const& ctx) }, [&](MPTIssue const& issue) -> std::optional { if (srcId != issuerId && isFrozen(ctx.view, srcId, issue)) + { + JLOG(ctx.j.warn()) << "Creating a check for locked MPT."; return tecLOCKED; + } if (dstId != issuerId && isFrozen(ctx.view, dstId, issue)) + { + JLOG(ctx.j.warn()) << "Creating a check for locked MPT."; return tecLOCKED; + } + if (auto const ter = canTransfer(ctx.view, issue, srcId, dstId); + !isTesSuccess(ter)) + { + JLOG(ctx.j.warn()) << "MPT transfer is disabled."; + return ter; + } return std::nullopt; }); @@ -169,7 +181,7 @@ CheckCreate::preclaim(PreclaimContext const& ctx) return tecEXPIRED; } - return canTrade(ctx.view, ctx.tx[sfSendMax].asset()); + return tesSUCCESS; } TER diff --git a/src/libxrpl/tx/transactors/dex/AMMCreate.cpp b/src/libxrpl/tx/transactors/dex/AMMCreate.cpp index 3e1fe97a76..60508eab85 100644 --- a/src/libxrpl/tx/transactors/dex/AMMCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMCreate.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -120,11 +119,16 @@ AMMCreate::preclaim(PreclaimContext const& ctx) } // Globally or individually frozen - if (isFrozen(ctx.view, accountID, amount.asset()) || - isFrozen(ctx.view, accountID, amount2.asset())) + if (auto const ter = checkFrozen(ctx.view, accountID, amount.asset()); !isTesSuccess(ter)) + { - JLOG(ctx.j.debug()) << "AMM Instance: involves frozen asset."; - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Instance: involves frozen or locked asset."; + return ter; + } + if (auto const ter = checkFrozen(ctx.view, accountID, amount2.asset()); !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) << "AMM Instance: involves frozen or locked asset."; + return ter; } auto noDefaultRipple = [](ReadView const& view, Asset const& asset) { @@ -191,10 +195,10 @@ AMMCreate::preclaim(PreclaimContext const& ctx) return terADDRESS_COLLISION; } - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_CREATE, amount.asset(), accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, amount.asset(), accountID, accountID); !isTesSuccess(ter)) return ter; - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_CREATE, amount2.asset(), accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, amount2.asset(), accountID, accountID); !isTesSuccess(ter)) return ter; @@ -301,22 +305,14 @@ applyCreate(ApplyContext& ctx, Sandbox& sb, AccountID const& account, beast::Jou // Authorize MPT return amount.asset().visit( [&](MPTIssue const& issue) -> TER { - // Authorize MPT auto const& mptIssue = issue; auto const& mptID = mptIssue.getMptID(); - std::uint32_t flags = lsfMPTAMM; - if (auto const err = - requireAuth(ctx.view(), mptIssue, accountId, AuthType::WeakAuth); + // Implicitly authorize MPT asset for AMM pseudo-account. + std::uint32_t const flags = lsfMPTAMM | lsfMPTAuthorized; + if (auto const err = requireAuth(sb, mptIssue, accountId, AuthType::WeakAuth); !isTesSuccess(err)) { - if (err == tecNO_AUTH) - { - flags |= lsfMPTAuthorized; - } - else - { - return err; - } + return err; } if (auto const err = createMPToken(sb, mptID, accountId, flags); !isTesSuccess(err)) diff --git a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp index 86cd52300d..f45529f617 100644 --- a/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMDeposit.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include @@ -267,12 +266,12 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return ter; } - if (isFrozen(ctx.view, accountID, asset)) + if (auto const ter = checkFrozen(ctx.view, accountID, asset); !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account or currency is frozen, " + JLOG(ctx.j.debug()) << "AMM Deposit: account or currency is frozen or locked, " << to_string(accountID) << " " << to_string(asset); - return tecFROZEN; + return ter; } return tesSUCCESS; @@ -304,18 +303,20 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } // AMM account or currency frozen - if (isFrozen(ctx.view, ammAccountID, amount->asset())) + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) - << "AMM Deposit: AMM account or currency is frozen, " << to_string(accountID); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Deposit: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; } // Account frozen - if (isIndividualFrozen(ctx.view, accountID, amount->asset())) + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen, " << to_string(accountID) - << " " << to_string(amount->asset()); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen or locked, " + << to_string(accountID) << " " << to_string(amount->asset()); + return ter; } if (checkBalance) { @@ -368,10 +369,10 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) } } - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_DEPOSIT, ctx.tx[sfAsset], accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, ctx.tx[sfAsset], accountID, accountID); !isTesSuccess(ter)) return ter; - if (auto const ter = checkMPTTxAllowed(ctx.view, ttAMM_DEPOSIT, ctx.tx[sfAsset2], accountID); + if (auto const ter = canMPTTradeAndTransfer(ctx.view, ctx.tx[sfAsset2], accountID, accountID); !isTesSuccess(ter)) return ter; diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index a4fd1518f5..352f637f2f 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -26,7 +25,6 @@ #include #include #include -#include #include #include @@ -242,24 +240,21 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) return ter; } // AMM account or currency frozen - if (isFrozen(ctx.view, ammAccountID, amount->asset())) + if (auto const ter = checkFrozen(ctx.view, ammAccountID, amount->asset()); + !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) - << "AMM Withdraw: AMM account or currency is frozen, " << to_string(accountID); - return tecFROZEN; + JLOG(ctx.j.debug()) << "AMM Withdraw: AMM account or currency is frozen or locked, " + << to_string(accountID); + return ter; } // Account frozen - if (isIndividualFrozen(ctx.view, accountID, amount->asset())) - { - JLOG(ctx.j.debug()) << "AMM Withdraw: account is frozen, " << to_string(accountID) - << " " << to_string(amount->asset()); - return tecFROZEN; - } - - if (auto const ter = - checkMPTTxAllowed(ctx.view, ttAMM_WITHDRAW, amount->asset(), accountID); + if (auto const ter = checkIndividualFrozen(ctx.view, accountID, amount->asset()); !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) << "AMM Withdraw: account is frozen or locked, " + << to_string(accountID) << " " << to_string(amount->asset()); return ter; + } } return tesSUCCESS; }; @@ -635,10 +630,6 @@ AMMWithdraw::withdraw( auto const balanceAdj = isIssue ? std::max(priorBalance, balance.xrp()) : priorBalance; if (balanceAdj < reserve) return tecINSUFFICIENT_RESERVE; - - // Update owner count. - if (!isIssue) - adjustOwnerCount(view, sleAccount, 1, journal); } return tesSUCCESS; }; diff --git a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp index ed9eb1255d..8bf69d25c0 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp @@ -181,11 +181,15 @@ OfferCreate::preclaim(PreclaimContext const& ctx) auto viewJ = ctx.registry.get().getJournal("View"); - if (isGlobalFrozen(ctx.view, saTakerPays.asset()) || - isGlobalFrozen(ctx.view, saTakerGets.asset())) + if (auto const ter = checkGlobalFrozen(ctx.view, saTakerPays.asset()); !isTesSuccess(ter)) { - JLOG(ctx.j.debug()) << "Offer involves frozen asset"; - return tecFROZEN; + JLOG(ctx.j.debug()) << "Offer involves frozen or locked asset"; + return ter; + } + if (auto const ter = checkGlobalFrozen(ctx.view, saTakerGets.asset()); !isTesSuccess(ter)) + { + JLOG(ctx.j.debug()) << "Offer involves frozen or locked asset"; + return ter; } // Allow unfunded MPT for issuer (OutstandingAmount >= MaximumAmount) @@ -320,7 +324,13 @@ OfferCreate::checkAcceptAsset( [&](MPTIssue const& issue) -> TER { // WeakAuth - don't check if MPToken exists since it's created // if needed. - return requireAuth(view, issue, id, AuthType::WeakAuth); + if (auto const ter = requireAuth(view, issue, id, AuthType::WeakAuth); + !isTesSuccess(ter)) + { + return ter; + } + + return checkFrozen(view, id, issue); }); } diff --git a/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp b/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp index 382ea7d40e..9b0c8d2b56 100644 --- a/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp +++ b/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp @@ -133,8 +133,9 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) // Can't unauthorize the pseudo-accounts because they are implicitly // always authorized. No need to amendment gate since Vault and LoanBroker - // can only be created if the Vault amendment is enabled. - if (isPseudoAccount(ctx.view, *holderID, {&sfVaultID, &sfLoanBrokerID})) + // can only be created if the Vault amendment is enabled; AMM with MPToken asset + // can only be created if MPTokensV2 is enabled. + if (isPseudoAccount(ctx.view, *holderID, {&sfVaultID, &sfLoanBrokerID, &sfAMMID})) return tecNO_PERMISSION; return tesSUCCESS; diff --git a/src/test/app/AMMExtendedMPT_test.cpp b/src/test/app/AMMExtendedMPT_test.cpp index cf1210901f..e6dcc95733 100644 --- a/src/test/app/AMMExtendedMPT_test.cpp +++ b/src/test/app/AMMExtendedMPT_test.cpp @@ -3114,10 +3114,8 @@ private: btc.set({.holder = bob, .flags = tfMPTLock}); { - // different from IOU. The offer is created but not crossed. - env(offer(bob, btc(5), XRP(25))); + env(offer(bob, btc(5), XRP(25)), Ter(tecLOCKED)); env.close(); - BEAST_EXPECT(expectOffers(env, bob, 1, {{{btc(5), XRP(25)}}})); BEAST_EXPECT(ammAlice.expectBalances(XRP(500), btc(105), ammAlice.tokens())); } @@ -3220,7 +3218,7 @@ private: btc.set({.flags = tfMPTLock}); // assets can't be bought on the market - AMM const ammA3(env, a3, btc(1), XRP(1), Ter(tecFROZEN)); + AMM const ammA3(env, a3, btc(1), XRP(1), Ter(tecLOCKED)); // direct issues can be sent env(pay(g1, a2, btc(1))); diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index cfb8edc60b..31b54ceee0 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -144,7 +144,7 @@ private: .pay = 30'000, .flags = tfMPTCanLock | kMptDexFlags}); usd.set({.flags = tfMPTLock}); - AMM const ammAliceFail(env, alice_, XRP(10'000), usd(10'000), Ter(tecFROZEN)); + AMM const ammAliceFail(env, alice_, XRP(10'000), usd(10'000), Ter(tecLOCKED)); usd.set({.flags = tfMPTUnlock}); AMM const ammAlice(env, alice_, XRP(10'000), usd(10'000)); } @@ -348,7 +348,7 @@ private: .pay = 30'000, .flags = tfMPTCanLock | kMptDexFlags}); btc.set({.flags = tfMPTLock}); - AMM const ammAlice(env, alice_, USD(10'000), btc(10'000), Ter(tecFROZEN)); + AMM const ammAlice(env, alice_, USD(10'000), btc(10'000), Ter(tecLOCKED)); BEAST_EXPECT(!ammAlice.ammExists()); } @@ -365,7 +365,7 @@ private: btc.set({.holder = alice_, .flags = tfMPTLock}); // alice's token is locked - AMM const ammAlice(env, alice_, USD(10'000), btc(10'000), Ter(tecFROZEN)); + AMM const ammAlice(env, alice_, USD(10'000), btc(10'000), Ter(tecLOCKED)); BEAST_EXPECT(!ammAlice.ammExists()); // bob can create @@ -693,15 +693,15 @@ private: btc.set({.flags = tfMPTLock}); ammAlice.deposit( - carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); - ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, USD(100), btc(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, USD(100), btc(100), std::nullopt, std::nullopt, Ter(tecLOCKED)); } // Individually lock MPT or freeze IOU (AMM) with IOU/MPT AMM @@ -722,20 +722,19 @@ private: // Carol can not deposit locked mpt ammAlice.deposit( - carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); - ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); if (!features[featureAMMClawback]) { - ammAlice.deposit( - carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); + ammAlice.deposit(carol_, USD(100), std::nullopt, std::nullopt, std::nullopt); } else { - // Carol can not deposit non-forzen token either + // Carol can not deposit non-frozen token either ammAlice.deposit( - carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, USD(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); } // Alice can deposit because she's not individually locked @@ -778,9 +777,9 @@ private: ammAlice.deposit(carol_, USD(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token - ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Unlock AMM MPT btc.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -812,10 +811,10 @@ private: // Carol's BTC is locked btc.set({.holder = carol_, .flags = tfMPTLock}); ammAlice.deposit( - carol_, usd(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, usd(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Unlock carol's BTC btc.set({.holder = carol_, .flags = tfMPTUnlock}); @@ -831,9 +830,9 @@ private: ammAlice.deposit(carol_, usd(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token BTC - ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, btc(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Unlock AMM MPT BTC btc.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -848,9 +847,9 @@ private: ammAlice.deposit(carol_, btc(100), std::nullopt, std::nullopt, std::nullopt); // Can not deposit locked token USD - ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.deposit(carol_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); ammAlice.deposit( - carol_, usd(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecFROZEN)); + carol_, usd(100), std::nullopt, std::nullopt, std::nullopt, Ter(tecLOCKED)); // Unlock AMM MPT USD usd.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -904,7 +903,7 @@ private: AMM amm(env, gw, XRP(10'000), btc(10'000)); - amm.deposit({.account = alice, .asset1In = btc(10), .err = Ter(tecNO_PERMISSION)}); + amm.deposit({.account = alice, .asset1In = btc(10), .err = Ter(tecNO_AUTH)}); } // Insufficient XRP balance @@ -2246,23 +2245,22 @@ private: Env env{*this}; env.fund(XRP(30'000), gw_, alice_); env.close(); - auto btcm = MPTTester( + auto btc = MPTTester( {.env = env, .issuer = gw_, .holders = {alice_}, .pay = 30'000, - .flags = tfMPTCanTrade, + .flags = kMptDexFlags, + .mutableFlags = tmfMPTCanMutateCanTransfer, .authHolder = true}); - MPT const btc = btcm; AMM amm(env, gw_, XRP(10'000), btc(10'000)); + amm.deposit(DepositArg{.account = alice_, .asset1In = XRP(200), .asset2In = btc(200)}); + // Allow to withdraw if transfer is disabled + btc.set({.mutableFlags = tmfMPTClearCanTransfer}); amm.withdraw( - WithdrawArg{ - .account = alice_, - .asset1Out = btc(100), - .assets = {{XRP, btc}}, - .err = Ter(tecNO_PERMISSION)}); + WithdrawArg{.account = alice_, .asset1Out = btc(100), .assets = {{XRP, btc}}}); } // Globally locked MPT @@ -2283,8 +2281,8 @@ private: btc.set({.flags = tfMPTLock}); ammAlice.withdraw( - alice_, MPT(ammAlice[1])(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); + alice_, MPT(ammAlice[1])(100), std::nullopt, std::nullopt, Ter(tecLOCKED)); + ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); // can single withdraw the other asset ammAlice.withdraw({.account = alice_, .asset1Out = XRP(100)}); @@ -2312,8 +2310,8 @@ private: // Alice's BTC is locked btc.set({.holder = alice_, .flags = tfMPTLock}); - ammAlice.withdraw(alice_, 1000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.withdraw(alice_, 1000, std::nullopt, std::nullopt, Ter(tecLOCKED)); + ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecLOCKED)); // can withdraw the other asset ammAlice.withdraw(alice_, usd(100), std::nullopt, std::nullopt); @@ -2341,8 +2339,8 @@ private: // Alice's BTC is locked btc.set({.holder = alice_, .flags = tfMPTLock}); - ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); + ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecLOCKED)); // can still single withdraw the unlocked other asset ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt); @@ -2361,8 +2359,8 @@ private: ammAlice.withdraw(alice_, USD(100), std::nullopt, std::nullopt); // Can not withdraw locked token BTC - ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecFROZEN)); - ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecFROZEN)); + ammAlice.withdraw(alice_, 1'000, std::nullopt, std::nullopt, Ter(tecLOCKED)); + ammAlice.withdraw(alice_, btc(100), std::nullopt, std::nullopt, Ter(tecLOCKED)); // Unlock AMM MPT btc.set({.holder = ammAlice.ammAccount(), .flags = tfMPTUnlock}); @@ -6886,33 +6884,33 @@ private: cb(amm, btc); }; - // Deposit two assets, one of which is frozen, - // then we should get tecFROZEN error. + // Deposit two assets, one of which is locked, + // then we should get tecLOCKED error. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& btc) { - amm.deposit(alice_, btc(100), XRP(100), std::nullopt, tfTwoAsset, Ter(tecFROZEN)); + amm.deposit(alice_, btc(100), XRP(100), std::nullopt, tfTwoAsset, Ter(tecLOCKED)); }); } - // Deposit one asset, which is the frozen token, - // then we should get tecFROZEN error. + // Deposit one asset, which is the locked token, + // then we should get tecLOCKED error. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& btc) { amm.deposit( - alice_, btc(100), std::nullopt, std::nullopt, tfSingleAsset, Ter(tecFROZEN)); + alice_, btc(100), std::nullopt, std::nullopt, tfSingleAsset, Ter(tecLOCKED)); }); } // Deposit one asset which is not the frozen token, - // but the other asset is frozen. We should get tecFROZEN error + // but the other asset is frozen. We should get tecLOCKED error // when feature AMMClawback is enabled. { Env env(*this); testAMMDeposit(env, [&](AMM& amm, MPTTester& btc) { amm.deposit( - alice_, XRP(100), std::nullopt, std::nullopt, tfSingleAsset, Ter(tecFROZEN)); + alice_, XRP(100), std::nullopt, std::nullopt, tfSingleAsset, Ter(tecLOCKED)); }); } } diff --git a/src/test/app/CheckMPT_test.cpp b/src/test/app/CheckMPT_test.cpp index a7fef5043c..1ca24051dd 100644 --- a/src/test/app/CheckMPT_test.cpp +++ b/src/test/app/CheckMPT_test.cpp @@ -1860,10 +1860,10 @@ class CheckMPT_test : public beast::unit_test::Suite // Use offers to automatically create MPT. MPT const oF4 = gw1["OF4"]; gw1.set(oF4, tfMPTLock); - env(offer(gw1, XRP(92), oF4(92)), Ter(tecFROZEN)); + env(offer(gw1, XRP(92), oF4(92)), Ter(tecLOCKED)); env.close(); BEAST_EXPECT(env.le(keylet::mptoken(oF4, alice)) == nullptr); - env(offer(alice, oF4(92), XRP(92)), Ter(tecFROZEN)); + env(offer(alice, oF4(92), XRP(92)), Ter(tecLOCKED)); env.close(); // No one's owner count should have changed. @@ -1951,10 +1951,10 @@ class CheckMPT_test : public beast::unit_test::Suite // Use offers to automatically create MPT. MPT const oF4 = gw1["OF4"]; gw1.set(oF4, tfMPTLock); - env(offer(alice, XRP(91), oF4(91)), Ter(tecFROZEN)); + env(offer(alice, XRP(91), oF4(91)), Ter(tecLOCKED)); env.close(); BEAST_EXPECT(env.le(keylet::mptoken(oF4, alice)) == nullptr); - env(offer(bob, oF4(91), XRP(91)), Ter(tecFROZEN)); + env(offer(bob, oF4(91), XRP(91)), Ter(tecLOCKED)); env.close(); // No one's owner count should have changed. diff --git a/src/test/app/ClawbackMPT_test.cpp b/src/test/app/ClawbackMPT_test.cpp new file mode 100644 index 0000000000..299e63d028 --- /dev/null +++ b/src/test/app/ClawbackMPT_test.cpp @@ -0,0 +1,435 @@ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include + +namespace xrpl { + +class ClawbackMPT_test : public beast::unit_test::Suite +{ + static std::uint32_t + ticketCount(test::jtx::Env const& env, test::jtx::Account const& acct) + { + std::uint32_t ret{0}; + if (auto const sleAcct = env.le(acct)) + ret = sleAcct->at(~sfTicketCount).value_or(0); + return ret; + } + + void + testValidation(FeatureBitset features) + { + testcase("Validation"); + using namespace test::jtx; + + // MPT clawback fails when featureMPTokensV1 is disabled + { + Env env(*this, features - featureMPTokensV1); + Account const alice{"alice"}; + Account const bob{"bob"}; + + env.fund(XRP(1000), alice, bob); + env.close(); + + auto const mpt = xrpl::test::jtx::MPT(alice.name(), makeMptID(env.seq(alice), alice)); + + env(claw(alice, mpt(5), bob), Ter(temDISABLED)); + env.close(); + } + + // MPT clawback fails when tfMPTCanClawback is not set on the issuance + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + mptAlice.claw(alice, bob, 10, tecNO_PERMISSION); + } + + // Test preflight validation failures + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + env.fund(XRP(1000), alice, bob); + env.close(); + + auto const mpt = xrpl::test::jtx::MPT(alice.name(), makeMptID(env.seq(alice), alice)); + + // fails due to invalid flag + env(claw(alice, mpt(5), bob), Txflags(0x00008000), Ter(temINVALID_FLAG)); + env.close(); + + // fails due to zero amount + env(claw(alice, mpt(0), bob), Ter(temBAD_AMOUNT)); + env.close(); + + // fails due to negative amount + env(claw(alice, mpt(-1), bob), Ter(temBAD_AMOUNT)); + env.close(); + + // fails when holder is not specified + env(claw(alice, mpt(5)), Ter(temMALFORMED)); + env.close(); + + // fails when issuer and holder are the same account + env(claw(alice, mpt(5), alice), Ter(temMALFORMED)); + env.close(); + } + + // Test preclaim failures + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + + auto const fakeMpt = + xrpl::test::jtx::MPT(alice.name(), makeMptID(env.seq(alice), alice)); + + // clawback fails when the issuance does not exist + env(claw(alice, fakeMpt(5), bob), Ter(tecOBJECT_NOT_FOUND)); + env.close(); + + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + + // clawback fails when bob has no MPToken + mptAlice.claw(alice, bob, 5, tecOBJECT_NOT_FOUND); + + mptAlice.authorize({.account = bob}); + + // clawback fails because bob's balance is 0 + mptAlice.claw(alice, bob, 5, tecINSUFFICIENT_FUNDS); + } + } + + void + testPermission(FeatureBitset features) + { + testcase("Permission"); + using namespace test::jtx; + + // Clawing back from a non-existent account fails + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + // bob is not funded and does not exist + MPTTester mptAlice(env, alice, MPTInit{}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + + mptAlice.claw(alice, bob, 5, terNO_ACCOUNT); + } + + // A non-issuer cannot claw back MPT + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const cindy{"cindy"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + env.fund(XRP(1000), cindy); + env.close(); + + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + // cindy fails to claw because she is not the issuer + mptAlice.claw(cindy, bob, 200, tecNO_PERMISSION); + } + } + + void + testEnabled(FeatureBitset features) + { + testcase("Enable clawback"); + using namespace test::jtx; + + // Test that alice is able to successfully clawback MPT from bob + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 1000)); + + // alice claws back 200 tokens from bob + mptAlice.claw(alice, bob, 200); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 800)); + + // alice claws back remaining 800 tokens + mptAlice.claw(alice, bob, 800); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + } + + void + testMultiIssuance(FeatureBitset features) + { + testcase("Multi issuance"); + using namespace test::jtx; + + // Two issuers each issue their own MPT to cindy. + // Clawback from one does not affect the other. + { + Env env(*this, features); + + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const cindy{"cindy"}; + + MPTTester mptAlice(env, alice, {.holders = {cindy}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = cindy}); + mptAlice.pay(alice, cindy, 1000); + + MPTTester mptBob(env, bob, MPTInit{}); // cindy already funded by mptAlice + mptBob.create({.ownerCount = 1, .flags = tfMPTCanClawback}); + mptBob.authorize({.account = cindy}); + mptBob.pay(bob, cindy, 1000); + + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 1000)); + BEAST_EXPECT(mptBob.checkMPTokenAmount(cindy, 1000)); + + // alice claws back 200 from cindy, bob's issuance is unaffected + mptAlice.claw(alice, cindy, 200); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 800)); + BEAST_EXPECT(mptBob.checkMPTokenAmount(cindy, 1000)); + + // bob claws back 600 from cindy, alice's issuance is unaffected + mptBob.claw(bob, cindy, 600); + BEAST_EXPECT(mptBob.checkMPTokenAmount(cindy, 400)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 800)); + } + + // One issuer issues MPT to two different holders. + // Clawback from one holder does not affect the other. + { + Env env(*this, features); + + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const cindy{"cindy"}; + + MPTTester mptAlice(env, alice, {.holders = {bob, cindy}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.authorize({.account = cindy}); + mptAlice.pay(alice, bob, 600); + mptAlice.pay(alice, cindy, 1000); + + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 600)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 1000)); + + // alice claws back 500 from bob, cindy's balance is unchanged + mptAlice.claw(alice, bob, 500); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 1000)); + + // alice claws back 300 from cindy, bob's balance is unchanged + mptAlice.claw(alice, cindy, 300); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(cindy, 700)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + } + } + + void + testZeroBalanceAfterClawback(FeatureBitset features) + { + testcase("Zero balance after clawback"); + using namespace test::jtx; + + // After clawback reduces balance to zero, the MPToken object + // still exists (unlike IOU trustlines which are deleted). + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + BEAST_EXPECT(ownerCount(env, bob) == 1); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 1000)); + + // alice claws back the full amount + mptAlice.claw(alice, bob, 1000); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + + // bob still holds the MPToken object (balance 0, not deleted) + BEAST_EXPECT(ownerCount(env, bob) == 1); + } + + void + testLockedMPT(FeatureBitset features) + { + testcase("Locked MPT"); + using namespace test::jtx; + + // Test that globally locked MPT can still be clawed back + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanLock | tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + // globally lock the issuance + mptAlice.set({.account = alice, .flags = tfMPTLock}); + + // clawback succeeds despite global lock + mptAlice.claw(alice, bob, 200); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 800)); + } + + // Test that individually locked MPT can still be clawed back + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanLock | tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + // individually lock bob's MPToken + mptAlice.set({.account = alice, .holder = bob, .flags = tfMPTLock}); + + // clawback succeeds despite individual lock + mptAlice.claw(alice, bob, 200); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 800)); + } + } + + void + testAmountExceedsAvailable(FeatureBitset features) + { + testcase("Amount exceeds available"); + using namespace test::jtx; + + // When alice tries to claw back more than bob holds, + // only the available balance is clawed back + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 1000); + + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 1000)); + + // alice tries to claw back 2000, but bob only has 1000 + mptAlice.claw(alice, bob, 2000); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(0)); + + // MPToken object still exists with 0 balance + BEAST_EXPECT(ownerCount(env, bob) == 1); + } + + void + testTickets(FeatureBitset features) + { + testcase("Tickets"); + using namespace test::jtx; + + // Tests MPT clawback using tickets + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {bob}}); + mptAlice.create({.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanClawback}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + + // alice creates 10 tickets + std::uint32_t ticketCnt = 10; + std::uint32_t aliceTicketSeq{env.seq(alice) + 1}; + env(ticket::create(alice, ticketCnt)); + env.close(); + std::uint32_t const aliceSeq{env.seq(alice)}; + BEAST_EXPECT(ticketCount(env, alice) == ticketCnt); + BEAST_EXPECT(ownerCount(env, alice) == ticketCnt + 1); // tickets + issuance + + while (ticketCnt > 0) + { + // alice claws back 5 tokens using a ticket + env(claw(alice, mptAlice.mpt(5), bob), ticket::Use(aliceTicketSeq++)); + env.close(); + + ticketCnt--; + BEAST_EXPECT(ticketCount(env, alice) == ticketCnt); + } + + // alice clawed back 50 tokens total, 50 remain + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 50)); + + // account sequence numbers did not advance + BEAST_EXPECT(env.seq(alice) == aliceSeq); + } + + void + testWithFeats(FeatureBitset features) + { + testValidation(features); + testPermission(features); + testEnabled(features); + testMultiIssuance(features); + testZeroBalanceAfterClawback(features); + testLockedMPT(features); + testAmountExceedsAvailable(features); + testTickets(features); + } + +public: + void + run() override + { + using namespace test::jtx; + FeatureBitset const all{testableAmendments()}; + testWithFeats(all); + } +}; + +BEAST_DEFINE_TESTSUITE(ClawbackMPT, app, xrpl); +} // namespace xrpl diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 5cb872f291..6fd1904992 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4313,6 +4313,189 @@ class Invariants_test : public beast::unit_test::Suite return true; }); } + + // Invalid transfer + std::array, 3> const invalidTransferTests = { + std::make_pair(ttAMM_WITHDRAW, false), + std::make_pair(ttPAYMENT, false), + std::make_pair(ttPAYMENT, true)}; + for (auto const enabled : {true, false}) + { + for (auto const& [tx, crossCurrencyPayment] : invalidTransferTests) + { + for (auto const flag : + {static_cast(lsfMPTLocked), + ~lsfMPTCanTransfer, + ~lsfMPTCanTrade, + 0u}) + { + MPTID id{}; + auto const isSuccess = !enabled || flag == 0 || + (tx == ttPAYMENT && !crossCurrencyPayment && (flag == ~lsfMPTCanTrade)) || + (tx == ttAMM_WITHDRAW && + (flag == ~lsfMPTCanTrade || flag == ~lsfMPTCanTransfer)); + std::pair const error = isSuccess + ? std::make_pair(TER(tesSUCCESS), TER(tesSUCCESS)) + : std::make_pair(TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)); + doInvariantCheck( + {{isSuccess ? "" : "invalid MPToken transfer between holders"}}, + [&](Account const& a1, Account const& a2, ApplyContext& ac) { + auto update = [&](AccountID const& a, std::uint64_t v) { + auto sle = ac.view().peek(keylet::mptoken(id, a)); + if (!sle) + return false; + sle->at(sfMPTAmount) = v; + ac.view().update(sle); + return true; + }; + auto issuanceSle = ac.view().peek(keylet::mptIssuance(id)); + if (!issuanceSle) + return false; + auto const flags = issuanceSle->at(sfFlags); + if (flag == lsfMPTLocked) + { + issuanceSle->at(sfFlags) = flags | lsfMPTLocked; + } + else if (flag != 0u) + { + issuanceSle->at(sfFlags) = flags & flag; + } + issuanceSle->at(sfOutstandingAmount) = 200; + ac.view().update(issuanceSle); + return update(a1, 101) && update(a2, 99); + }, + XRPAmount{}, + STTx{ + tx, + [&](STObject& tx) { + if (crossCurrencyPayment) + { + tx.setFieldAmount( + sfSendMax, STAmount(MPTAmount{100}, MPTIssue{id})); + } + }}, + {error.first, error.second}, + [&](Account const& a1, Account const& a2, Env& env) { + Account const gw("gw"); + env.fund(XRP(1'000), gw); + MPTTester const usd( + {.env = env, .issuer = gw, .holders = {a1, a2}, .pay = 100}); + id = usd.issuanceID(); + if (!enabled) + { + env.disableFeature(featureMPTokensV2); + } + return true; + }); + } + } + } + } + + void + testAMM() + { + testcase << "AMM"; + using namespace jtx; + + MPTID mptID{}; + uint256 ammID{}; + AccountID ammAccountID{}; + Account const gw{"gw"}; + Issue lptIssue{}; + PrettyAsset poolAsset{xrpIssue()}; + + auto deleteAMMAccount = [&](ApplyContext& ac, bool) { + auto sle = ac.view().peek(keylet::account(ammAccountID)); + if (!sle) + return false; + ac.view().erase(sle); + return true; + }; + + auto updateLPTokensBalance = [&](ApplyContext& ac, std::int64_t amount) { + auto sle = ac.view().peek(keylet::amm(ammID)); + if (!sle) + return false; + sle->setFieldAmount(sfLPTokenBalance, STAmount{lptIssue, amount}); + ac.view().update(sle); + return true; + }; + auto updateLPTokensBadAmount = [&](ApplyContext& ac, bool) { + return updateLPTokensBalance(ac, -1); + }; + auto updateLPTokensBadBalance = [&](ApplyContext& ac, bool) { + return updateLPTokensBalance(ac, 200'000'000); + }; + auto updateAMM = [&](ApplyContext& ac, bool) { return updateLPTokensBalance(ac, 10); }; + + auto updateAMMPool = [&](ApplyContext& ac, bool isMPT) { + if (isMPT) + { + auto sle = ac.view().peek(keylet::mptoken(mptID, ammAccountID)); + if (!sle) + return false; + sle->setFieldU64(sfMPTAmount, 1); + ac.view().update(sle); + return true; + } + auto sle = ac.view().peek(keylet::account(ammAccountID)); + if (!sle) + return false; + sle->setFieldAmount(sfBalance, XRP(1)); + ac.view().update(sle); + return true; + }; + + auto test = [&](auto const txType, + auto&& update, + bool isMPT, + TER error = tecINVARIANT_FAILED) { + doInvariantCheck( + {{"AMM"}}, + [&](Account const&, Account const&, ApplyContext& ac) { return update(ac, isMPT); }, + XRPAmount{}, + STTx{txType, [&](STObject& tx) {}}, + {tecINVARIANT_FAILED, error}, + [&](Account const&, Account const&, Env& env) { + env.fund(XRP(1'000), gw); + poolAsset = [&]() -> PrettyAsset { + if (isMPT) + { + MPT const mpt = MPTTester({.env = env, .issuer = gw}); + mptID = mpt.issuanceID; + return mpt; + } + return gw["USD"]; + }(); + AMM const amm(env, gw, XRP(100), poolAsset(100)); + ammAccountID = amm.ammAccount(); + ammID = amm.ammID(); + lptIssue = amm.lptIssue(); + return true; + }); + }; + + for (bool const isMPT : {false, true}) + { + auto const error = isMPT ? TER(tecINVARIANT_FAILED) : TER(tefINVARIANT_FAILED); + for (auto txType : {ttAMM_CREATE, ttAMM_DEPOSIT, ttAMM_CLAWBACK, ttAMM_WITHDRAW}) + { + test(txType, deleteAMMAccount, isMPT, tefINVARIANT_FAILED); + test(txType, updateLPTokensBadAmount, isMPT); + test(txType, updateLPTokensBadBalance, isMPT); + } + for (auto txType : {ttAMM_BID, ttAMM_VOTE}) + { + test(txType, updateAMMPool, isMPT, error); + test(txType, updateLPTokensBadAmount, isMPT); + test(txType, updateLPTokensBadBalance, isMPT); + } + for (auto txType : {ttAMM_DELETE, ttCHECK_CASH, ttOFFER_CREATE, ttPAYMENT}) + { + test(txType, updateAMM, isMPT); + } + } } // Test the invariant overwrite fix for both pre- and post-amendment @@ -4600,6 +4783,7 @@ public: testInvariantOverwrite(defaultAmendments()); testInvariantOverwrite(defaultAmendments() - fixCleanup3_1_3); testVaultComputeCoarsestScale(); + testAMM(); } }; diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 74bdc656df..6906c4aba0 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -3498,10 +3499,10 @@ class MPToken_test : public beast::unit_test::Suite auto const [errBuy, errSell] = [&]() -> std::pair { // Global lock if (lockMPTIssue) - return std::make_pair(tecFROZEN, tecFROZEN); + return std::make_pair(tecLOCKED, tecLOCKED); // Local lock if (lockMPToken) - return std::make_pair(tesSUCCESS, error(tecUNFUNDED_OFFER)); + return std::make_pair(error(tecLOCKED), error(tecUNFUNDED_OFFER)); // MPToken doesn't exist if (requireAuth) return std::make_pair(error(tecNO_AUTH), error(tecUNFUNDED_OFFER)); @@ -3564,51 +3565,77 @@ class MPToken_test : public beast::unit_test::Suite env(offer(alice, btc(10), eth(10)), Ter(tecUNFUNDED_OFFER)); } - // MPTLock flag is set and the account is not the issuer of MPT + // MPTLock flag is set: MPT/MPT offer crossing with independent issuers. + // gw2 issues BTC and gw issues ETH so each asset can be frozen independently. + // Passive setup: bob sells BTC (offer(bob, ETH, BTC)); dan sells ETH (offer(dan, BTC, + // ETH)). { + Account const gw2 = Account("gw2"); Account const bob = Account("bob"); Account const dan = Account("dan"); Env env(*this); - env.fund(XRP(1'000), gw, alice, carol, bob, dan); + env.fund(XRP(1'000), gw, gw2, alice, carol, bob, dan); MPTTester btc( {.env = env, - .issuer = gw, - .holders = {alice, carol, bob, dan}, + .issuer = gw2, + .holders = {alice, carol, bob, dan, gw}, .pay = 100, .flags = tfMPTCanLock | kMptDexFlags}); MPTTester eth( {.env = env, .issuer = gw, - .holders = {alice, carol, bob, dan}, + .holders = {alice, carol, bob, dan, gw2}, .pay = 100, .flags = tfMPTCanLock | kMptDexFlags}); + // bob sells BTC (takerGets=BTC); dan sells ETH (takerGets=ETH) env(offer(bob, eth(10), btc(10)), Txflags(tfPassive)); env(offer(dan, btc(10), eth(10)), Txflags(tfPassive)); + env.close(); - auto test = [&](auto const& flag, bool gwOwner = false) { - btc.set({.holder = carol, .flags = flag}); - btc.set({.holder = alice, .flags = flag}); + // --- Individual lock on BTC --- + // alice sells locked BTC (takerGets): balance zeroed, offer unfunded + btc.set({.holder = alice, .flags = tfMPTLock}); + env(offer(alice, eth(1), btc(1)), Ter(tecUNFUNDED_OFFER)); + btc.set({.holder = alice, .flags = tfMPTUnlock}); + // carol buys locked BTC (takerPays): locked MPToken cannot receive + btc.set({.holder = carol, .flags = tfMPTLock}); + env(offer(carol, btc(1), eth(1)), Ter(tecLOCKED)); + btc.set({.holder = carol, .flags = tfMPTUnlock}); + // gw2 is BTC issuer: individual lock on holders does not affect issuer + env(offer(gw2, eth(1), btc(1))); - if (gwOwner) - { - // Succeeds if the account is the issuer - env(offer(gw, eth(1), btc(1))); - env(offer(gw, btc(1), eth(1))); - } - else - { - auto const err = flag == tfMPTLock ? Ter(tecUNFUNDED_OFFER) : Ter(tesSUCCESS); - env(offer(alice, eth(1), btc(1)), err); - // Offer created by not crossed - env(offer(carol, btc(1), eth(1))); - BEAST_EXPECT(expectOffers(env, carol, 1, {{btc(1), eth(1)}})); - } - }; + // --- Individual lock on ETH --- + // alice sells locked ETH (takerGets): balance zeroed, offer unfunded + eth.set({.holder = alice, .flags = tfMPTLock}); + env(offer(alice, btc(1), eth(1)), Ter(tecUNFUNDED_OFFER)); + eth.set({.holder = alice, .flags = tfMPTUnlock}); + // carol buys locked ETH (takerPays): locked MPToken cannot receive + eth.set({.holder = carol, .flags = tfMPTLock}); + env(offer(carol, eth(1), btc(1)), Ter(tecLOCKED)); + eth.set({.holder = carol, .flags = tfMPTUnlock}); + // gw is ETH issuer: individual lock on holders does not affect issuer + env(offer(gw, btc(1), eth(1))); - test(tfMPTLock); - test(tfMPTLock, true); - test(tfMPTUnlock); + // --- Global lock on BTC --- + // All accounts fail regardless of role: global lock is checked in OfferCreate + // before offer crossing, so it applies even to the issuer. + btc.set({.flags = tfMPTLock}); + env(offer(alice, eth(1), btc(1)), Ter(tecLOCKED)); // alice sells BTC + env(offer(alice, btc(1), eth(1)), Ter(tecLOCKED)); // alice buys BTC + env(offer(gw2, eth(1), btc(1)), Ter(tecLOCKED)); // gw2 is BTC issuer, still fails + btc.set({.flags = tfMPTUnlock}); + + // --- Global lock on ETH --- + eth.set({.flags = tfMPTLock}); + env(offer(alice, btc(1), eth(1)), Ter(tecLOCKED)); // alice sells ETH + env(offer(alice, eth(1), btc(1)), Ter(tecLOCKED)); // alice buys ETH + env(offer(gw, btc(1), eth(1)), Ter(tecLOCKED)); // gw is ETH issuer, still fails + eth.set({.flags = tfMPTUnlock}); + + // --- After all locks cleared: normal crossing succeeds --- + env(offer(alice, eth(1), btc(1))); + env(offer(carol, btc(1), eth(1))); } // MPTRequireAuth flag is set and the account is not authorized @@ -3823,6 +3850,7 @@ class MPToken_test : public beast::unit_test::Suite testcase("Cross Asset Payment"); using namespace test::jtx; Account const gw = Account("gw"); + Account const gw2 = Account("gw2"); Account const alice = Account("alice"); Account const carol = Account("carol"); Account const bob = Account("bob"); @@ -3999,6 +4027,7 @@ class MPToken_test : public beast::unit_test::Suite env(pay(ed, gw, btc(10)), Path(~btc), Sendmax(eth(10))); // BTC is transferred from issuer to bob env(pay(gw, ed, eth(10)), Path(~eth), Sendmax(btc(10))); + // // BTC is transferred from ed to bob, ed is not authorized env(pay(ed, gw, eth(10)), Path(~eth), Sendmax(btc(10)), Ter(tecNO_AUTH)); env.close(); @@ -4013,13 +4042,11 @@ class MPToken_test : public beast::unit_test::Suite env(pay(carol, ed, btc(10)), Path(~btc), Sendmax(eth(10)), Ter(tecPATH_PARTIAL)); env(pay(ed, carol, eth(10)), Path(~eth), Sendmax(btc(10)), Ter(tecPATH_PARTIAL)); env(pay(carol, ed, eth(10)), Path(~eth), Sendmax(btc(10)), Ter(tecPATH_PARTIAL)); - // Fail because BTC, which has CanTransfer disabled, is sent to - // bob + // Fail because BTC, which has CanTransfer disabled, is sent to bob env(pay(ed, gw, eth(10)), Path(~eth), Sendmax(btc(10)), Ter(tecPATH_PARTIAL)); env(pay(ed, gw, btc(10)), Path(~btc), Sendmax(eth(10)), Ter(tesSUCCESS)); env(pay(gw, ed, eth(10)), Path(~eth), Sendmax(btc(10)), Ter(tesSUCCESS)); - // Fail because BTC, which has CanTransfer disabled, is sent to - // ed + // Fail because BTC, which has CanTransfer disabled, is sent to ed env(pay(gw, ed, btc(10)), Path(~btc), Sendmax(eth(10)), Ter(tecPATH_PARTIAL)); env.close(); env(offer(gw, eth(100), btc(100)), Txflags(tfPassive)); @@ -4194,7 +4221,17 @@ class MPToken_test : public beast::unit_test::Suite env(pay(gw, carol, usd(1)), Path(~btc, ~eth, ~usd), Sendmax(XRP(1)), - Ter(tecPATH_PARTIAL)); + Txflags(tfPartialPayment | tfNoRippleDirect), + Ter(tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT(expectOffers(env, bob, 3)); + + env(pay(carol, bob, btc(10)), Sendmax(XRP(10)), Ter(tecNO_PERMISSION)); + env(pay(carol, bob, XRP(10)), Sendmax(btc(10)), Ter(tecNO_PERMISSION)); + env(pay(gw, bob, btc(10)), Sendmax(XRP(10)), Ter(tecNO_PERMISSION)); + env(pay(gw, bob, XRP(10)), Sendmax(btc(10)), Ter(tecNO_PERMISSION)); + env(pay(carol, gw, btc(10)), Sendmax(XRP(10)), Ter(tecNO_PERMISSION)); + env(pay(carol, gw, XRP(10)), Sendmax(btc(10)), Ter(tecNO_PERMISSION)); env.close(); BEAST_EXPECT(expectOffers(env, bob, 3)); } @@ -4229,31 +4266,36 @@ class MPToken_test : public beast::unit_test::Suite auto getMPT = [&](Env& env) { MPTTester const btc( {.env = env, - .issuer = gw, - .holders = {alice, carol, bob}, + .issuer = gw2, + .holders = {alice, carol, bob, gw}, .pay = 100, .flags = tfMPTCanLock | kMptDexFlags}); MPTTester const eth( {.env = env, .issuer = gw, - .holders = {alice, carol, bob}, + .holders = {alice, carol, bob, gw2}, .pay = 100, .flags = tfMPTCanLock | kMptDexFlags}); return std::make_pair(btc, eth); }; auto getIOU = [&](Env& env) { - for (auto const& iou : {gw["BTC"], gw["ETH"]}) + for (auto const& a : {alice, carol, bob}) { - for (auto const& a : {alice, carol, bob}) - { - env(fset(a, asfDefaultRipple)); - env.close(); - env(trust(a, iou(200))); - env(pay(gw, a, iou(100))); - env.close(); - } + env(fset(a, asfDefaultRipple)); + env.close(); + env(trust(a, gw["ETH"](200))); + env(pay(gw, a, gw["ETH"](100))); + env(trust(a, gw2["BTC"](200))); + env(pay(gw2, a, gw2["BTC"](100))); + env.close(); } - return std::make_pair(gw["BTC"], gw["ETH"]); + // gw2 needs an ETH trust line to receive ETH when its offers fill + // gw needs BTC to sell BTC + env(trust(gw2, gw["ETH"](200))); + env(trust(gw, gw2["BTC"](200))); + env(pay(gw2, gw, gw2["BTC"](100))); + env.close(); + return std::make_pair(gw2["BTC"], gw["ETH"]); }; auto lock = [&]( Env& env, Account const& account, Token& token, LockType lock) { @@ -4263,12 +4305,12 @@ class MPToken_test : public beast::unit_test::Suite { if (lock == LockType::Global) { - env(fset(gw, asfGlobalFreeze)); + env(fset(token.account, asfGlobalFreeze)); } else { IOU const iou{account, token.currency}; - env(trust(gw, iou(0), tfSetFreeze)); + env(trust(token.account, iou(0), tfSetFreeze)); } } else if constexpr (std::is_same_v) @@ -4285,7 +4327,7 @@ class MPToken_test : public beast::unit_test::Suite }; auto test = [&](auto&& getTokens, TestArg const& arg) { Env env(*this); - env.fund(XRP(1'000), gw, alice, carol, bob); + env.fund(XRP(1'000), gw, gw2, alice, carol, bob); auto [btc, eth] = getTokens(env); @@ -4303,7 +4345,7 @@ class MPToken_test : public beast::unit_test::Suite } if (arg.globalFlagSell != LockType::None) { - lock(env, gw, btc, LockType::Global); + lock(env, gw2, btc, LockType::Global); } else { @@ -4321,34 +4363,58 @@ class MPToken_test : public beast::unit_test::Suite }; // clang-format off std::vector const tests = { - // src, dst, offer's owner are a holder - {.src = alice, .dst = carol, .offerOwner = bob, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, - // dst can receive IOU even if the account is frozen - {.src = alice, .dst = carol, .offerOwner = bob, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, - {.src = alice, .dst = carol, .offerOwner = bob, .globalFlagBuy = LockType::Global, .err = tecPATH_DRY}, - {.src = alice, .dst = carol, .offerOwner = bob, .globalFlagSell = LockType::Global, .err = tecPATH_DRY}, - // offer's owner can receive IOU even if the account is frozen - {.src = alice, .dst = carol, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = - tecPATH_PARTIAL, .errIOU = tesSUCCESS}, - {.src = alice, .dst = carol, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, - // src, dst are a holder, offer's owner is an issuer - {.src = alice, .dst = carol, .offerOwner = gw, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, - // dst can receive IOU even if the account is frozen - {.src = alice, .dst = carol, .offerOwner = gw, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, - {.src = alice, .dst = carol, .offerOwner = gw, .globalFlagBuy = LockType::Global, .err = tecPATH_DRY}, - {.src = alice, .dst = carol, .offerOwner = gw, .globalFlagSell = LockType::Global, .err = tecPATH_DRY}, - // src is issuer, dst and offer's owner are a holder - // dst can receive IOU even if the account is frozen - {.src = gw, .dst = carol, .offerOwner = bob, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, - // offer's owner can receive IOU from an issuer even if takerBuys is frozen, MPT offer is unfunded in this case - {.src = gw, .dst = carol, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = tecPATH_PARTIAL, .errIOU = tesSUCCESS}, - {.src = gw, .dst = carol, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, - // dst is issuer, src and offer's owner are a holder - {.src = alice, .dst = gw, .offerOwner = bob, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, - // offer's owner can receive IOU even if the account is frozen - {.src = alice, .dst = gw, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = tecPATH_PARTIAL, - .errIOU = tesSUCCESS}, - {.src = alice, .dst = gw, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, + // ----- src=alice (holder), dst=carol (holder), offerOwner=bob (holder) ----- + // alice's ETH locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = bob, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, + // carol's BTC locked: caught in MPT check(); IOU dst can still receive when frozen + {.src = alice, .dst = carol, .offerOwner = bob, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, + // ETH globally locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = bob, .globalFlagBuy = LockType::Global, .err = tecPATH_DRY}, + // BTC globally locked: bob's offer unfunded in OfferStream + {.src = alice, .dst = carol, .offerOwner = bob, .globalFlagSell = LockType::Global, .err = tecPATH_PARTIAL}, + // bob's ETH (takerPays) locked: MPT offer unfunded in OfferStream (locked holder cannot receive); IOU can still receive + {.src = alice, .dst = carol, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = tecPATH_PARTIAL, .errIOU = tesSUCCESS}, + // bob's BTC (takerGets) locked: offer unfunded in OfferStream + {.src = alice, .dst = carol, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, + // ----- src=alice (holder), dst=carol (holder), offerOwner=gw2 (BTC issuer) ----- + // alice's ETH locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = gw2, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, + // carol's BTC locked: caught in MPT check(); IOU dst can still receive when frozen + {.src = alice, .dst = carol, .offerOwner = gw2, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, + // ETH globally locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = gw2, .globalFlagBuy = LockType::Global, .err = tecPATH_DRY}, + // BTC globally locked: gw2 is the BTC issuer, offer always permitted regardless of global freeze + {.src = alice, .dst = carol, .offerOwner = gw2, .globalFlagSell = LockType::Global, .err = tesSUCCESS}, + // ----- src=alice (holder), dst=carol (holder), offerOwner=gw (ETH issuer, BTC holder) ----- + // alice's ETH locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = gw, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, + // carol's BTC locked: caught in MPT check(); IOU dst can still receive when frozen + {.src = alice, .dst = carol, .offerOwner = gw, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, + // ETH globally locked: caught in check() + {.src = alice, .dst = carol, .offerOwner = gw, .globalFlagBuy = LockType::Global, .err = tecPATH_DRY}, + // BTC globally locked: gw holds BTC as a holder (not BTC issuer), offer unfunded in OfferStream + {.src = alice, .dst = carol, .offerOwner = gw, .globalFlagSell = LockType::Global, .err = tecPATH_PARTIAL}, + // ----- src=gw (ETH issuer), dst=carol (holder), offerOwner=bob (holder) ----- + // ETH globally locked, src is ETH issuer: no first MPTEndpointStep so check() passes; + // MPT offer unfunded in OfferStream (globally-locked ETH cannot flow to holder via DEX); IOU issuer can still issue + {.src = gw, .dst = carol, .offerOwner = bob, .srcFlag = LockType::Global, .err = tecPATH_PARTIAL, .errIOU = tesSUCCESS}, + // BTC globally locked: last MPTEndpointStep only checks individual freeze, check() passes; offer unfunded in OfferStream + {.src = gw, .dst = carol, .offerOwner = bob, .dstFlag = LockType::Global, .err = tecPATH_PARTIAL}, + // carol's BTC locked: caught in MPT check(); IOU dst can still receive when frozen + {.src = gw, .dst = carol, .offerOwner = bob, .dstFlag = LockType::Individual, .err = tecPATH_DRY, .errIOU = tesSUCCESS}, + // bob's ETH (takerPays) locked: MPT offer unfunded in OfferStream (locked holder cannot receive); IOU can still receive + {.src = gw, .dst = carol, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = tecPATH_PARTIAL, .errIOU = tesSUCCESS}, + // bob's BTC (takerGets) locked: offer unfunded in OfferStream + {.src = gw, .dst = carol, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, + // ----- src=alice (holder), dst=gw2 (BTC issuer), offerOwner=bob (holder) ----- + // alice's ETH locked: caught in check() + {.src = alice, .dst = gw2, .offerOwner = bob, .srcFlag = LockType::Individual, .err = tecPATH_DRY}, + // BTC globally locked, dst is BTC issuer: no last MPTEndpointStep so check() passes; offer unfunded in OfferStream + {.src = alice, .dst = gw2, .offerOwner = bob, .dstFlag = LockType::Global, .err = tecPATH_PARTIAL}, + // bob's ETH (takerPays) locked: MPT offer unfunded in OfferStream (locked holder cannot receive); IOU can still receive + {.src = alice, .dst = gw2, .offerOwner = bob, .offerFlagBuy = LockType::Individual, .err = tecPATH_PARTIAL, .errIOU = tesSUCCESS}, + // bob's BTC (takerGets) locked: offer unfunded in OfferStream + {.src = alice, .dst = gw2, .offerOwner = bob, .offerFlagSell = LockType::Individual, .err = tecPATH_PARTIAL}, }; // clang-format on @@ -5949,7 +6015,7 @@ class MPToken_test : public beast::unit_test::Suite env.close(); } - // MPTCanTransfer disabled + // MPTCanTransfer is disabled { Env env{*this, features}; env.fund(XRP(1'000), gw, alice, carol); @@ -5990,49 +6056,50 @@ class MPToken_test : public beast::unit_test::Suite BEAST_EXPECT(env.balance(alice, mpt) == mpt(0)); BEAST_EXPECT(env.balance(gw, mpt) == mpt(0)); - // neither src nor dst is issuer, can still create + // neither src nor dst is issuer, can't create + checkId = keylet::check(alice, env.seq(alice)).key; + env(check::create(alice, carol, mpt(100)), Ter(tecNO_AUTH)); + env.close(); + + // can create now + mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTransfer}); checkId = keylet::check(alice, env.seq(alice)).key; env(check::create(alice, carol, mpt(100))); env.close(); - - // can't cash - env(check::cash(carol, checkId, mpt(10)), Ter(tecPATH_PARTIAL)); - env.close(); - - // can cash now - mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTransfer}); env(pay(gw, alice, mpt(10))); env.close(); + // can't cash + mpt.set({.account = gw, .mutableFlags = tmfMPTClearCanTransfer}); + env.close(); + env(check::cash(carol, checkId, mpt(10)), Ter(tecNO_AUTH)); + env.close(); + // can cash + mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTransfer}); env(check::cash(carol, checkId, mpt(10))); env.close(); } - // MPTCanTrade disabled + // MPTCanTrade is disabled { Env env{*this, features}; env.fund(XRP(1'000), gw, alice, carol); env.close(); - MPTTester mpt( + MPT const mpt = MPTTester( {.env = env, .issuer = gw, .holders = {alice, carol}, - .flags = tfMPTCanTransfer, - .mutableFlags = tmfMPTCanMutateCanTrade}); + .pay = 10, + .flags = tfMPTCanTransfer}); - uint256 checkId{keylet::check(gw, env.seq(gw)).key}; + uint256 const checkId{keylet::check(alice, env.seq(alice)).key}; - // can't create - env(check::create(gw, alice, mpt(100)), Ter(tecNO_PERMISSION)); + // can create + env(check::create(alice, carol, mpt(100))); env.close(); - mpt.set({.account = gw, .mutableFlags = tmfMPTSetCanTrade}); - // can't cash - checkId = keylet::check(gw, env.seq(gw)).key; - env(check::create(gw, carol, mpt(100))); - env.close(); - mpt.set({.account = gw, .mutableFlags = tmfMPTClearCanTrade}); - env(check::cash(carol, checkId, mpt(10)), Ter(tecNO_PERMISSION)); + // can cash + env(check::cash(carol, checkId, mpt(10))); env.close(); } @@ -6087,33 +6154,6 @@ class MPToken_test : public beast::unit_test::Suite env.close(); } - // MPTCanTransfer is not set and the account is not the issuer of MPT - { - Env env{*this, features}; - env.fund(XRP(1'000), gw, alice, carol); - - auto eur = MPTTester( - {.env = env, .issuer = gw, .holders = {alice, carol}, .flags = tfMPTCanTrade}); - uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; - // alice can create - env(check::create(alice, carol, eur(1))); - env.close(); - - // carol can't cash - env(check::cash(carol, chkId, eur(1)), Ter(tecPATH_PARTIAL)); - env.close(); - - // if issuer creates a check then carol can cash since - // it's a transfer from the issuer - uint256 const chkId1{getCheckIndex(gw, env.seq(gw))}; - // alice can't create since CanTransfer is not set - env(check::create(gw, carol, eur(1))); - env.close(); - - env(check::cash(carol, chkId1, eur(1))); - env.close(); - } - // Can create check if src/dst don't own MPT { Env env{*this, features}; @@ -6274,8 +6314,7 @@ class MPToken_test : public beast::unit_test::Suite AMM amm(env, gw, btc(100), usd(100)); env.close(); // alice can't deposit since MPTCanTransfer is not set - amm.deposit( - DepositArg{.account = alice, .tokens = 1'000, .err = Ter(tecNO_PERMISSION)}); + amm.deposit(DepositArg{.account = alice, .tokens = 1'000, .err = Ter(tecNO_AUTH)}); env.close(); // can't clawback since alice is not an LP @@ -6508,8 +6547,8 @@ class MPToken_test : public beast::unit_test::Suite // alice and issuer can't create usd.set({.flags = tfMPTLock}); - createFail(alice, tecFROZEN); - createFail(gw, tecFROZEN); + createFail(alice, tecLOCKED); + createFail(gw, tecLOCKED); // MPTRequireAuth is set @@ -6529,7 +6568,7 @@ class MPToken_test : public beast::unit_test::Suite usd.set({.mutableFlags = tmfMPTClearRequireAuth}); usd.set({.mutableFlags = tmfMPTClearCanTransfer}); // alice can't create - createFail(alice, tecNO_PERMISSION); + createFail(alice, tecNO_AUTH); // issuer can create createDeleteAMM(gw); usd.set({.mutableFlags = tmfMPTSetCanTransfer}); @@ -6575,12 +6614,12 @@ class MPToken_test : public beast::unit_test::Suite {.account = account, .asset1In = usd(1), .asset2In = eur(1), - .err = Ter(tecFROZEN)}); + .err = Ter(tecLOCKED)}); amm.deposit( {.account = account, .asset1In = eur(1), .assets = std::make_pair(eur, usd), - .err = Ter(tecFROZEN)}); + .err = Ter(tecLOCKED)}); } usd.set({.flags = tfMPTUnlock}); @@ -6593,8 +6632,6 @@ class MPToken_test : public beast::unit_test::Suite eur.authorize({.account = carol}); env(pay(gw, carol, eur(1'000'000))); usd.set({.mutableFlags = tmfMPTSetRequireAuth}); - // have to authorize amm account - usd.authorize({.account = gw, .holder = Account{"amm", amm.ammAccount()}}); env.close(); amm.deposit( {.account = carol, .asset1In = usd(1), .asset2In = eur(1), .err = Ter(tecNO_AUTH)}); @@ -6608,6 +6645,16 @@ class MPToken_test : public beast::unit_test::Suite // carol is authorized, can deposit usd.authorize({.account = gw, .holder = carol}); amm.deposit({.account = carol, .tokens = 1'000}); + // Can't authorize or unauthorize AMM pseudo-account + usd.authorize( + {.account = gw, + .holder = Account{"amm", amm.ammAccount()}, + .err = tecNO_PERMISSION}); + usd.authorize( + {.account = gw, + .holder = Account{"amm", amm.ammAccount()}, + .flags = tfMPTUnauthorize, + .err = tecNO_PERMISSION}); // MPTCanTransfer is not set @@ -6615,15 +6662,12 @@ class MPToken_test : public beast::unit_test::Suite usd.set({.mutableFlags = tmfMPTClearCanTransfer}); // carol can't deposit amm.deposit( - {.account = carol, - .asset1In = usd(1), - .asset2In = eur(1), - .err = Ter(tecNO_PERMISSION)}); + {.account = carol, .asset1In = usd(1), .asset2In = eur(1), .err = Ter(tecNO_AUTH)}); amm.deposit( {.account = carol, .asset1In = eur(1), .assets = std::make_pair(eur, usd), - .err = Ter(tecNO_PERMISSION)}); + .err = Ter(tecNO_AUTH)}); // issuer can deposit amm.deposit({.account = gw, .tokens = 1'000}); // carol can deposit @@ -6665,8 +6709,8 @@ class MPToken_test : public beast::unit_test::Suite {.account = account, .asset1Out = usd(1), .asset2Out = eur(1), - .err = Ter(tecFROZEN)}); - amm.withdraw({.account = account, .tokens = 1'000, .err = Ter(tecFROZEN)}); + .err = Ter(tecLOCKED)}); + amm.withdraw({.account = account, .tokens = 1'000, .err = Ter(tecLOCKED)}); // can single withdraw another asset amm.withdraw( {.account = account, .asset1Out = eur(1), .assets = std::make_pair(eur, usd)}); @@ -6692,29 +6736,38 @@ class MPToken_test : public beast::unit_test::Suite usd.authorize({.account = gw, .holder = carol}); amm.withdraw({.account = carol, .asset1Out = usd(1), .asset2Out = eur(1)}); - // MPTCanTransfer is set + // MPTCanTransfer is not set, allow to withdraw usd.set({.mutableFlags = tmfMPTClearRequireAuth}); usd.set({.mutableFlags = tmfMPTClearCanTransfer}); - // carol can't withdraw - amm.withdraw( - {.account = carol, - .asset1Out = usd(1), - .asset2Out = eur(1), - .err = Ter(tecNO_PERMISSION)}); + // carol can withdraw + amm.withdraw({.account = carol, .asset1Out = usd(1), .asset2Out = eur(1)}); // can withdraw another asset amm.withdraw( {.account = carol, .asset1Out = eur(1), .assets = std::make_pair(eur, usd)}); // issuer can withdraw amm.withdraw({.account = gw, .asset1Out = usd(1), .asset2Out = eur(1)}); + // Holder can't transfer to another holder + env.fund(XRP(1'000), bob); + usd.authorize({.account = bob}); + env(pay(carol, bob, usd(1)), Ter(tecNO_AUTH)); + usd.authorize({.account = bob, .flags = tfMPTUnauthorize}); + // Can redeem + env(pay(carol, gw, usd(1))); // carol can withdraw usd.set({.mutableFlags = tmfMPTSetCanTransfer}); amm.withdraw({.account = carol, .asset1Out = usd(1), .asset2Out = eur(1)}); usd.set({.mutableFlags = tmfMPTSetCanTransfer}); + + // MPTCanTrade is not set, allow to withdraw + usd.set({.mutableFlags = tmfMPTClearCanTrade}); - amm.withdraw({.account = gw, .tokens = 1'000, .err = Ter(tecNO_PERMISSION)}); - amm.withdraw({.account = carol, .tokens = 1'000, .err = Ter(tecNO_PERMISSION)}); + amm.withdraw({.account = gw, .tokens = 1'000}); + amm.withdraw({.account = carol, .tokens = 1'000}); + // Can't DEX + amm.deposit( + DepositArg{.account = carol, .asset1In = usd(1), .err = Ter(tecNO_PERMISSION)}); usd.set({.mutableFlags = tmfMPTSetCanTrade}); // MPToken created on withdraw @@ -6733,6 +6786,99 @@ class MPToken_test : public beast::unit_test::Suite } } + void + testFixDoubleOwnerCount(FeatureBitset all) + { + testcase("Fix Double adjustOwnerCount in AMMWithdraw"); + + using namespace jtx; + + // Carol deposits XRP into an XRP/MPT pool, then withdraws MPT. + // Carol has no MPToken before the withdrawal. If the bug exists, + // her ownerCount will be inflated by +1 extra. + Account const gw{"gw"}; + Account const alice{"alice"}; + Account const carol{"carol"}; + Env env(*this, all); + env.fund(XRP(30'000), gw, alice, carol); + env.close(); + + // Create MPT with DEX flags. Only alice is a holder initially. + MPT const btc = MPTTester( + {.env = env, .issuer = gw, .holders = {alice}, .pay = 20'000, .flags = kMptDexFlags}); + + // Alice creates XRP/MPT AMM pool + AMM amm(env, alice, XRP(10'000), btc(10'000)); + + // Carol deposits XRP (single asset) into the pool. + // Carol gets LP tokens but does NOT have an MPToken yet. + auto const carolOwnersBefore = ownerCount(env, carol); + amm.deposit(carol, XRP(1'000), std::nullopt, std::nullopt, tfSingleAsset); + auto const carolOwnersAfterDeposit = ownerCount(env, carol); + // Carol should have +1 for LP token trustline + BEAST_EXPECT(carolOwnersAfterDeposit == carolOwnersBefore + 1); + + auto const carolOwnersBeforeWithdraw = ownerCount(env, carol); + // Carol withdraws single MPT asset. She doesn't have an MPToken, + // so one must be created. Bug: ownerCount incremented twice. + amm.withdraw({.account = carol, .asset1Out = btc(100), .flags = tfSingleAsset}); + auto const carolOwnersAfterWithdraw = ownerCount(env, carol); + + // Expected: +1 for the new MPToken (so total increase = 1) + BEAST_EXPECT(carolOwnersAfterWithdraw == carolOwnersBeforeWithdraw + 1); + } + + void + testTradeAndTransfer() + { + using namespace jtx; + testcase("Trade and Transfer"); + + // Verify canMPTTradeAndTransfer validates the flags when from == to and from != to + + Account const gw{"gw"}; + Account const alice{"alice"}; + Account const carol{"carol"}; + Env env(*this); + env.fund(XRP(1'000), gw, alice, carol); + + MPTTester mpt( + {.env = env, + .issuer = gw, + .holders = {alice, carol}, + .pay = 100, + .flags = kMptDexFlags, + .mutableFlags = tmfMPTCanMutateCanTransfer | tmfMPTCanMutateCanTrade}); + + // Both flags are enabled + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, gw, gw))); + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, gw, alice))); + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, alice, alice))); + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, alice, carol))); + + // MPTCanTrade is disabled + mpt.set({.mutableFlags = tmfMPTClearCanTrade}); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, gw, gw) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, gw, alice) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, alice) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, carol) == tecNO_PERMISSION); + + // MPTCanTransfer is disabled + mpt.set({.mutableFlags = tmfMPTSetCanTrade}); + mpt.set({.mutableFlags = tmfMPTClearCanTransfer}); + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, gw, gw))); + BEAST_EXPECT(isTesSuccess(canMPTTradeAndTransfer(*env.current(), mpt, gw, alice))); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, alice) == tecNO_AUTH); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, carol) == tecNO_AUTH); + + // Both flags are disabled + mpt.set({.mutableFlags = tmfMPTClearCanTrade}); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, gw, gw) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, gw, alice) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, alice) == tecNO_PERMISSION); + BEAST_EXPECT(canMPTTradeAndTransfer(*env.current(), mpt, alice, carol) == tecNO_PERMISSION); + } + public: void run() override @@ -6839,6 +6985,12 @@ public: // Test AMM testBasicAMM(all); + + // Test Trade/Transfer + testTradeAndTransfer(); + + // Fixes + testFixDoubleOwnerCount(all); } }; diff --git a/src/test/app/OfferMPT_test.cpp b/src/test/app/OfferMPT_test.cpp index a52ba7fd58..3f88e57bc7 100644 --- a/src/test/app/OfferMPT_test.cpp +++ b/src/test/app/OfferMPT_test.cpp @@ -2764,7 +2764,7 @@ public: // USD(125) was removed from his account due to the gateway fee. // // A comparable payment would look like this: - // env (pay (bob, alice, USD(100)), sendmax(USD(125))) + // env (pay (bob, alice, USD(100)), Sendmax(USD(125))) env(offer(bob, XRP(1), usd(10'000))); env.close(); @@ -3012,6 +3012,92 @@ public: } }; testHelper2TokensMix(test); + + // Payment trIn: MPT transfer fee must be charged when the payment + // destination is the MPT issuer and MPT crosses the DEX (1-hop). + // Bug: rate() returned parity because strandDst_ == MPT issuer. + // Fix: parity only when this asset IS the final delivered asset. + { + auto const gw = Account("gw_tr1"); + auto const alice = Account("alice_tr1"); + auto const bob = Account("bob_tr1"); + + Env env{*this, features}; + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPT const usd = MPTTester( + {.env = env, .issuer = gw, .holders = {alice, bob}, .transferFee = 25'000}); + + // alice needs MPT(1250): MPT(1000) to bob's offer + MPT(250) transfer fee (25%) + env(pay(gw, alice, usd(1'250))); + // bob's offer: give XRP(1000), want MPT(1000) + env(offer(bob, usd(1'000), XRP(1'000))); + env.close(); + + // alice pays gw (MPT issuer) XRP(1000) using MPT as source + // strand: alice -> [MPT/XRP BookStep] -> gw + // strandDst_ = gw = MPT issuer, strandDeliver_ = XRP + // trIn = rate(MPT, gw): fix charges 25% (MPT != strandDeliver_) + env(pay(alice, gw, XRP(1'000)), Path(~XRP), Sendmax(usd(1'250))); + env.close(); + + // alice consumed all MPT(1250): MPT(1000) to bob + MPT(250) fee + BEAST_EXPECT(env.balance(alice, usd) == usd(0)); + // bob received MPT(1000) net + BEAST_EXPECT(env.balance(bob, usd) == usd(1'000)); + } + + // Payment trIn (2-hop): MPT transfer fee must be charged when MPT is + // intermediate and the destination is the MPT issuer. + // BookStep2(MPT/XRP) prevStep=BookStep1 returns redeems direction + // (ownerPaysTransferFee_=false for Payment), so trIn applies. + // Bug: parity because strandDst_ == MPT issuer. + // Fix: 25% fee because MPT != strandDeliver_(XRP). + { + auto const gw = Account("gw_tr2"); + auto const gw2 = Account("gw2_tr2"); + auto const alice = Account("alice_tr2"); + auto const bob = Account("bob_tr2"); + auto const carol = Account("carol_tr2"); + + Env env{*this, features}; + env.fund(XRP(10'000), gw, gw2, alice, bob, carol); + env.close(); + + MPT const musd = MPTTester( + {.env = env, .issuer = gw, .holders = {bob, carol}, .transferFee = 25'000}); + auto const gusd = gw2["USD"]; + + env(trust(alice, gusd(10'000))); + env(trust(bob, gusd(10'000))); + env.close(); + + env(pay(gw2, alice, gusd(1'000))); + env(pay(gw, bob, musd(1'000))); + env.close(); + + // bob's offer: give MPT(1000), want GUSD(1000) + env(offer(bob, gusd(1'000), musd(1'000))); + // carol's offer: give XRP(800), want MPT(800) + env(offer(carol, musd(800), XRP(800))); + env.close(); + + // Payment: alice GUSD -> [BookStep1: GUSD/MUSD] -> [BookStep2: MUSD/XRP] -> gw XRP + // strandDst_ = gw = MPT issuer, strandDeliver_ = XRP + // BookStep2 trIn: fix = 1.25 -> upstream needs MUSD(1000) for carol's MUSD(800) offer + // => alice must provide full GUSD(1000) to bob's offer; without fix alice only pays + // GUSD(800) + env(pay(alice, gw, XRP(800)), Path(~musd), Sendmax(gusd(1'000))); + env.close(); + + // alice spent all GUSD(1000); bug would leave GUSD(200) unspent + BEAST_EXPECT(env.balance(alice, gusd) == gusd(0)); + // bob gave MPT(1000) and received GUSD(1000) + BEAST_EXPECT(env.balance(bob, musd) == musd(0)); + // carol received MPT(800) net (MPT(200) went to gw as fee) + BEAST_EXPECT(env.balance(carol, musd) == musd(800)); + } } void @@ -4707,6 +4793,101 @@ public: } } + void + testAutoCreateReserve(FeatureBitset features) + { + // When an offer on the book is partially crossed, the payment engine + // auto-creates a new ledger object (MPToken or IOU trustline) for the + // offer owner to hold the incoming asset. This happens inside + // BookStep::forEachOffer (MPT: checkCreateMPT) and BookStep::consumeOffer + // (IOU: directSendNoFeeIOU -> trustCreate) without a reserve sufficiency + // check. The offer owner can therefore end up with more objects than + // their XRP balance can reserve for, consistent with IOU behavior. + + testcase("Auto-Create Object Without Reserve Check During Partial Crossing"); + + using namespace jtx; + + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const carol = Account{"carol"}; + auto const bob = Account{"bob"}; + + auto test = [&](auto&& getToken, auto&& execTx) { + // MPT/IOU: carol's existing offer buys MPT/IOU by selling XRP. + // carol has no MPToken/Trustline for this issuance. When alice partially crosses + // carol's offer, an MPToken/Trustline is auto-created for carol without checking + // that she can afford the extra reserve slot. + Env env{*this, features}; + + auto const f = env.current()->fees().base; + auto const r = reserve(env, 0); + auto const inc = reserve(env, 1) - r; + + env.fund(XRP(10'000), gw, alice, bob); + + // getToken: + // - Create MPT with CanTransfer + CanTrade; authorize alice as holder. + // - Create IOU trustline + auto const token = getToken(env); + + // carol: reserve(0) + 1 increment + fee covers placing one offer. + // After the offer tx she has exactly reserve(1) + XRP(30). + // XRP(30) < inc (50 XRP), so receiving a second object will put her + // below reserve(2). + if (BEAST_EXPECT(inc > XRP(30))) + env.fund(r + inc + f + XRP(30), carol); + + // carol's offer goes on the book (no counterpart yet). + // TakerPays=Token(30): carol will receive Token when crossed. + // TakerGets=XRP(30): carol will give XRP when crossed. + env(offer(carol, token(30), XRP(30))); + env.require(Owners(carol, 1)); + + // Execute offer create or cross-currency payment + // alice partially crosses carol's offer. + // alice sends Token(15) to carol and receives XRP(15). + // Token: + // - MPT: checkCreateMPT auto-creates an MPToken for carol (no reserve check). + // - IOU: directSendNoFeeIOU auto-creates an Trustline for carol (no reserve check). + execTx(env, token); + + // Carol now owns 2 objects (remaining offer + new MPToken) even + // though her XRP balance is only reserve(1) + XRP(15), which is + // below reserve(2) = reserve(1) + inc. + auto const carolBalance = r + inc + XRP(15); + env.require(Owners(carol, 2), Balance(carol, token(15)), Balance(carol, carolBalance)); + BEAST_EXPECT(carolBalance < r + 2 * inc); // below reserve(2) + }; + std::function const getIOU = [&](Env& env) -> PrettyAsset { + env.trust(gw["USD"](1'000), alice); + env(pay(gw, alice, gw["USD"](100))); + return gw["USD"]; + }; + std::function const getMPT = [&](Env& env) -> PrettyAsset { + MPT const mpT1 = MPTTester({.env = env, .issuer = gw, .holders = {alice}, .pay = 100}); + return mpT1; + }; + for (auto&& getToken : {getIOU, getMPT}) + { + test(getToken, [&](Env& env, PrettyAsset const& token) { + // alice partially crosses carol's offer. + // alice sends Token(15) to carol and receives XRP(15). + // Token is MPT: checkCreateMPT auto-creates an MPToken for carol (no reserve + // check). Token is IOU: directSendNoFeeIOU auto-creates a trustline for carol (no + // reserve check). + env(offer(alice, XRP(15), token(15))); + }); + test(getToken, [&](Env& env, PrettyAsset const& token) { + // Similar to above but with cross-currency payment. + env(pay(alice, bob, XRP(15)), + Sendmax(token(15)), + Path(~XRP), + Txflags(tfNoRippleDirect | tfPartialPayment)); + }); + } + } + void testAll(FeatureBitset features) { @@ -4763,6 +4944,7 @@ public: testRmSmallIncreasedQOffersMPT(features); testFillOrKill(features); testTickSize(features); + testAutoCreateReserve(features); } void diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index af6f4841f4..c6a9a54a53 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -10,9 +10,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -2377,6 +2379,83 @@ class Vault_test : public beast::unit_test::Suite env.close(); } + { + testcase("MPT locked: vault shares inherit underlying lock"); + + Env env{*this, testableAmendments()}; + Account const issuer{"issuer"}; + Account const owner{"owner"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10'000), issuer, owner, alice, bob, carol); + env.close(); + Vault const vault{env}; + + MPTTester asset{ + {.env = env, + .issuer = issuer, + .holders = {owner, alice, bob, carol}, + .flags = tfMPTCanTransfer | tfMPTCanTrade | tfMPTCanLock}}; + 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)); + }(); + auto const shareMptID = shares.raw().get().getMptID(); + auto const shareBalance = [&](Account const& account) { + auto const sle = env.le(keylet::mptoken(shareMptID, account)); + return sle ? sle->at(sfMPTAmount) : 0; + }; + + // Sanity: before the underlying lock, peer-to-peer share + // transfers are allowed. + env(pay(alice, bob, shares(1))); + env.close(); + + // Create the offer while shares are spendable, then lock the + // underlying to test whether a stale offer can still be crossed. + env(offer(alice, XRP(1), shares(1))); + env.close(); + + // Lock the underlying after the vault and share balances exist. + asset.set({.account = issuer, .flags = tfMPTLock}); + env.close(); + + // Direct vault share payment inherits the underlying lock via + // sfReferenceHolding. + BEAST_EXPECT(shareBalance(alice) == 499); + BEAST_EXPECT(shareBalance(bob) == 501); + env(pay(alice, bob, shares(1)), Ter{tecLOCKED}); + env.close(); + BEAST_EXPECT(shareBalance(alice) == 499); + BEAST_EXPECT(shareBalance(bob) == 501); + + // The same inherited lock must also block DEX payment paths that + // would consume an offer selling vault shares. + env(pay(carol, bob, shares(1)), + Sendmax(XRP(1)), + Path(BookSpec{shares.raw()}), + Ter{tecPATH_PARTIAL}); + env.close(); + BEAST_EXPECT(shareBalance(alice) == 499); + BEAST_EXPECT(shareBalance(bob) == 501); + BEAST_EXPECT(expectOffers(env, alice, 1)); + } + { testcase("MPT non-transferable: pre-fixCleanup3_2_0 share transfer succeeds"); @@ -2981,7 +3060,7 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); } - // Behavioural shift introduced by share inheritance: + // Behavioral 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). From a37afe13ff33402f72e476d304e8d5f18392611f Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Fri, 22 May 2026 04:30:37 -0700 Subject: [PATCH 3/6] ci: Re-enable full nproc for Linux (#7315) --- .github/workflows/reusable-build-test-config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 4c7c41fd0c..cc927512ea 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -181,7 +181,7 @@ jobs: - name: Build the binary working-directory: ${{ env.BUILD_DIR }} env: - BUILD_NPROC: ${{ runner.os == 'Linux' && '16' || steps.nproc.outputs.nproc }} + BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} BUILD_TYPE: ${{ inputs.build_type }} CMAKE_TARGET: ${{ inputs.cmake_target }} run: | From 15dd653e4b2cf3c0165111784d8136b91f0ef91d Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Fri, 22 May 2026 04:30:45 -0700 Subject: [PATCH 4/6] fix: Fix RPM prerelease ordering and start xrpld on DEB install (#7313) --- package/build_pkg.sh | 6 +++++- package/debian/rules | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/package/build_pkg.sh b/package/build_pkg.sh index c834951493..adac2dc169 100755 --- a/package/build_pkg.sh +++ b/package/build_pkg.sh @@ -135,8 +135,12 @@ build_rpm() { # RPM Version can't contain '-'. A pre-release goes in Release with a # leading "0." so 3.2.0-b1 sorts before the final 3.2.0-. + # The order is "0.." (e.g. 0.1.b6) — the Fedora/EPEL + # convention. Reversing to "0.." (e.g. 0.b6.1) breaks + # rpmvercmp against the former because numeric segments outrank alphabetic + # ones, so "0.1.b5" would sort newer than "0.b6.1". local rpm_release="${PKG_RELEASE}" - [[ -n "${VER_SUFFIX}" ]] && rpm_release="0.${VER_SUFFIX}.${PKG_RELEASE}" + [[ -n "${VER_SUFFIX}" ]] && rpm_release="0.${PKG_RELEASE}.${VER_SUFFIX}" set -x rpmbuild -bb \ diff --git a/package/debian/rules b/package/debian/rules index 0fae101358..cd94da7e5b 100644 --- a/package/debian/rules +++ b/package/debian/rules @@ -9,7 +9,7 @@ override_dh_auto_configure override_dh_auto_build override_dh_auto_test: @: override_dh_installsystemd: - dh_installsystemd --no-start xrpld.service + dh_installsystemd --no-stop-on-upgrade xrpld.service dh_installsystemd --name=update-xrpld --no-start update-xrpld.service update-xrpld.timer execute_before_dh_installtmpfiles: From 179e73594ae3d22afaddb144be7905549e276ce5 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 22 May 2026 12:58:48 +0100 Subject: [PATCH 5/6] fix: Check if the MPT first loss cover can be sent to the broker before deleting the broker (#7125) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> --- .../transactors/lending/LoanBrokerDelete.cpp | 14 ++ src/test/app/LoanBroker_test.cpp | 210 ++++++++++++++++++ 2 files changed, 224 insertions(+) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp index 805a4612f2..6b77914370 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -106,6 +107,19 @@ LoanBrokerDelete::preclaim(PreclaimContext const& ctx) } } + if (ctx.view.rules().enabled(fixCleanup3_2_0)) + { + if (coverAvailable > beast::kZero) + { + auto const brokerPseudo = sleBroker->at(sfAccount); + if (auto const ret = checkFrozen(ctx.view, brokerPseudo, asset)) + { + JLOG(ctx.j.warn()) << "Broker pseudo-account is frozen/locked."; + return ret; + } + } + } + return tesSUCCESS; } diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index c899778391..92949256fd 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1577,6 +1577,210 @@ class LoanBroker_test : public beast::unit_test::Suite env(loanBroker::set(lender, vaultKeylet.key), Ter(tecFROZEN)); } + void + testLoanBrokerDeleteLockedMPT(FeatureBitset features) + { + testcase << "LoanBrokerDelete - locked broker pseudo-account MPT"; + using namespace jtx; + using namespace loanBroker; + + Account const issuer("issuer"); + Account const alice("alice"); + + auto const withFix = features[fixCleanup3_2_0]; + Env env(*this, features); + env.fund(XRP(100'000), issuer, alice); + env.close(); + + // Create MPT with locking enabled + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); + + PrettyAsset const mpt{mptt.issuanceID()}; + + // Fund alice + mptt.authorize({.account = alice}); + env.close(); + env(pay(issuer, alice, mpt(100'000))); + env.close(); + + // Create vault + Vault const vault{env}; + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = mpt}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = mpt(10'000)})); + env.close(); + + // Create loan broker + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + // Deposit cover + env(coverDeposit(alice, brokerKeylet.key, mpt(5'000).value())); + env.close(); + + // Verify cover is deposited + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + BEAST_EXPECT(broker->at(sfCoverAvailable) > 0); + + // Get the broker pseudo-account ID + auto const brokerPseudoID = broker->at(sfAccount); + + // Verify the broker pseudo-account has an MPToken + auto const pseudoMptKey = keylet::mptoken(mptt.issuanceID(), brokerPseudoID); + auto const pseudoMpt = env.le(pseudoMptKey); + if (!BEAST_EXPECT(pseudoMpt)) + return; + + // Issuer locks the broker pseudo-account's individual MPToken + { + json::Value jv; + jv[jss::Account] = issuer.human(); + jv[sfMPTokenIssuanceID] = to_string(mptt.issuanceID()); + jv[jss::Holder] = toBase58(brokerPseudoID); + jv[jss::TransactionType] = jss::MPTokenIssuanceSet; + jv[jss::Flags] = tfMPTLock; + env(jv); + env.close(); + } + + // Verify the pseudo-account's MPToken is now locked + { + auto const sle = env.le(pseudoMptKey); + if (!BEAST_EXPECT(sle)) + return; + BEAST_EXPECT(sle->isFlag(lsfMPTLocked)); + } + + // Record alice's balance before deletion + auto const aliceBalanceBefore = env.balance(alice, mpt); + + // With fixCleanup3_2_0, preclaim() checks the broker pseudo-account's + // freeze/lock state via checkFrozen(), so deletion is blocked. + // Without the fix, the check is missing and the locked cover is + // returned to the owner. + if (withFix) + { + env(del(alice, brokerKeylet.key), Ter(tecLOCKED)); + env.close(); + + // Verify the broker is not deleted + BEAST_EXPECT(env.le(brokerKeylet) != nullptr); + + // Verify alice did not receive the cover despite the lock + auto const aliceBalanceAfter = env.balance(alice, mpt); + BEAST_EXPECT(aliceBalanceAfter == aliceBalanceBefore); + + // Verify the locked MPToken was not deleted + BEAST_EXPECT(env.le(pseudoMptKey) != nullptr); + } + else + { + env(del(alice, brokerKeylet.key), Ter(tesSUCCESS)); + env.close(); + + // Verify the broker is deleted + BEAST_EXPECT(env.le(brokerKeylet) == nullptr); + + // Verify alice received the cover despite the lock + auto const aliceBalanceAfter = env.balance(alice, mpt); + BEAST_EXPECT(aliceBalanceAfter > aliceBalanceBefore); + + // Verify the locked MPToken was deleted + BEAST_EXPECT(env.le(pseudoMptKey) == nullptr); + } + } + + void + testLoanBrokerDeleteFrozenIOU(FeatureBitset features) + { + testcase << "LoanBrokerDelete - frozen broker pseudo-account IOU"; + using namespace jtx; + using namespace loanBroker; + + Account const issuer("issuer"); + Account const alice("alice"); + + auto const withFix = features[fixCleanup3_2_0]; + Env env(*this, features); + env.fund(XRP(100'000), issuer, alice); + env.close(); + + auto const iou = issuer["IOU"]; + + // Set up trust lines and fund alice + env(trust(alice, iou(1'000'000))); + env.close(); + env(pay(issuer, alice, iou(100'000))); + env.close(); + + // Create vault + Vault const vault{env}; + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = iou.asset()}); + env(tx); + env.close(); + + // Deposit into vault + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = iou(10'000)})); + env.close(); + + // Create loan broker + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + // Deposit cover + env(coverDeposit(alice, brokerKeylet.key, iou(5'000))); + env.close(); + + // Verify cover is deposited + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + BEAST_EXPECT(broker->at(sfCoverAvailable) > 0); + + // Get the broker pseudo-account + auto const brokerPseudoID = broker->at(sfAccount); + auto const brokerPseudo = Account("BrokerPseudo", brokerPseudoID); + + // Issuer freezes the broker pseudo-account's trust line + env(trust(issuer, brokerPseudo["IOU"](0), tfSetFreeze)); + env.close(); + + // Record alice's balance before deletion attempt + auto const aliceBalanceBefore = env.balance(alice, iou); + + // With fixCleanup3_2_0, preclaim() checks the broker + // pseudo-account's freeze state via checkFrozen(), so + // deletion is blocked early with tecFROZEN. + // Without the fix, preclaim() does not check the pseudo-account, + // but the TransfersNotFrozen invariant catches the frozen transfer + // in doApply() and fails with tecINVARIANT_FAILED. + // Either way, the broker survives and alice's balance is unchanged. + if (withFix) + { + env(del(alice, brokerKeylet.key), Ter(tecFROZEN)); + } + else + { + env(del(alice, brokerKeylet.key), Ter(tecINVARIANT_FAILED)); + } + env.close(); + + // Broker still exists + BEAST_EXPECT(env.le(brokerKeylet) != nullptr); + + // Alice's balance unchanged + auto const aliceBalanceAfter = env.balance(alice, iou); + BEAST_EXPECT(aliceBalanceAfter == aliceBalanceBefore); + } + void testRIPD4274IOU() { @@ -2056,6 +2260,12 @@ public: testRIPD4274(); + testLoanBrokerDeleteLockedMPT(all_); + testLoanBrokerDeleteLockedMPT(all_ - fixCleanup3_2_0); + + testLoanBrokerDeleteFrozenIOU(all_); + testLoanBrokerDeleteFrozenIOU(all_ - fixCleanup3_2_0); + // TODO: Write clawback failure tests with an issuer / MPT that doesn't // have the right flags set. } From dfb9b8ed9a0ad04c1da00c9957fe3c7454930835 Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 22 May 2026 20:32:12 +0100 Subject: [PATCH 6/6] release: Bump version to 3.2.0-b7 (#7316) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 3d4fff9f04..5613a9366e 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -23,7 +23,7 @@ namespace { //------------------------------------------------------------------------------ // clang-format off // NOLINTNEXTLINE(readability-identifier-naming) -char const* const versionString = "3.2.0-b6" +char const* const versionString = "3.2.0-b7" // clang-format on ;