diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 1b4a3ad6ee..8c68181801 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -414,8 +414,8 @@ ValidMPTTransfer::finalize( if (txnType == ttAMM_CLAWBACK) return true; - // DEX transactions (cross-currency payments, offer creates) are subject to - // the MPTCanTrade flag in addition to the standard transfer rules. + // DEX transactions (AMM[Create,Deposit,Withdraw], 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 isDEX = [&tx, &txnType] { if (txnType == ttPAYMENT) @@ -425,10 +425,12 @@ ValidMPTTransfer::finalize( auto const amount = tx[sfAmount]; return tx[~sfSendMax].value_or(amount).asset() != amount.asset(); } - return txnType == ttOFFER_CREATE; + return txnType == ttAMM_CREATE || txnType == ttAMM_DEPOSIT || txnType == ttAMM_WITHDRAW || + 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 enforce = !view.rules().enabled(featureMPTokensV2); for (auto const& [mptID, values] : amount_) @@ -447,16 +449,10 @@ ValidMPTTransfer::finalize( for (auto const& [account, value] : values) { - // Check once: if any involved account is frozen, the whole - // issuance transfer is considered frozen. - if (!frozen && isFrozen(view, account, MPTIssue{mptID})) - { - frozen = true; - } - // Classify each account as a sender or receiver based on whether - // their MPTAmount decreased or increased. - if (value.amtBefore.has_value() && value.amtAfter.has_value() && - *value.amtBefore != *value.amtAfter) + // Classify each account as a sender or receiver based on whether their MPTAmount + // decreased or increased. Count new MPToken holders (no amtBefore) as receivers. + if (value.amtAfter.has_value() && + (!value.amtBefore.has_value() || *value.amtBefore != *value.amtAfter)) { if (*value.amtAfter > *value.amtBefore) { @@ -467,6 +463,13 @@ ValidMPTTransfer::finalize( ++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 (!frozen && isFrozen(view, account, MPTIssue{mptID})) + { + frozen = true; + } } // A transfer between holders has occurred (senders > 0 && receivers > 0). // Fail if the issuance is frozen, does not permit transfers, or — for diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index a16f9e20d3..088ca0a8df 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -6625,6 +6626,57 @@ class MPToken_test : public beast::unit_test::suite 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 = MPTDEXFlags, + .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 @@ -6732,6 +6784,9 @@ public: // Test AMM testBasicAMM(all); + // Test Trade/Transfer + testTradeAndTransfer(); + // Fixes testFixDoubleOwnerCount(all); }