diff --git a/src/libxrpl/protocol/STVar.cpp b/src/libxrpl/protocol/STVar.cpp index aa693308600..e44c37259b7 100644 --- a/src/libxrpl/protocol/STVar.cpp +++ b/src/libxrpl/protocol/STVar.cpp @@ -143,13 +143,21 @@ STVar::constructST(SerializedTypeID id, int depth, Args&&... args) if constexpr (std::is_same_v< std::tuple...>, std::tuple>) + { construct(std::forward(args)...); + } else if constexpr (std::is_same_v< std::tuple...>, std::tuple>) + { construct(std::forward(args)..., depth); + } else - static_assert(false, "Invalid STVar constructor arguments"); + { + constexpr bool alwaysFalse = + !std::is_same_v, std::tuple>; + static_assert(alwaysFalse, "Invalid STVar constructor arguments"); + } }; switch (id) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index c96520e202d..e6370888e87 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -637,51 +637,173 @@ class MPToken_test : public beast::unit_test::suite Account const alice("alice"); // issuer Account const bob("bob"); // holder Account const carol("carol"); // holder + + // preflight validation + + // MPT is disabled + { + Env env{*this, features - featureMPTokensV1}; + Account const alice("alice"); + Account const bob("bob"); + + env.fund(XRP(1'000), alice); + env.fund(XRP(1'000), bob); + STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; + + env(pay(alice, bob, mpt), ter(temDISABLED)); + } + + // MPT is disabled, unsigned request + { + Env env{*this, features - featureMPTokensV1}; + Account const alice("alice"); // issuer + Account const carol("carol"); + auto const USD = alice["USD"]; + + env.fund(XRP(1'000), alice); + env.fund(XRP(1'000), carol); + STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; + + Json::Value jv; + jv[jss::secret] = alice.name(); + jv[jss::tx_json][jss::Fee] = to_string(env.current()->fees().base); + jv[jss::tx_json] = pay(alice, carol, mpt); + auto const jrr = env.rpc("json", "submit", to_string(jv)); + BEAST_EXPECT(jrr[jss::result][jss::engine_result] == "temDISABLED"); + } + + // Invalid flag + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + auto const MPT = mptAlice["MPT"]; + + mptAlice.authorize({.account = &bob}); + + for (auto flags : + {tfNoRippleDirect, tfPartialPayment, tfLimitQuality}) + env(pay(alice, bob, MPT(10)), + txflags(flags), + ter(temINVALID_FLAG)); + } + + // Invalid combination of send, sendMax, deliverMin, paths + { + Env env{*this, features}; + Account const alice("alice"); + Account const carol("carol"); + + MPTTester mptAlice(env, alice, {.holders = {&carol}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + + mptAlice.authorize({.account = &carol}); + + // sendMax and DeliverMin are valid XRP amount, + // but is invalid combination with MPT amount + auto const MPT = mptAlice["MPT"]; + env(pay(alice, carol, MPT(100)), + sendmax(XRP(100)), + ter(temMALFORMED)); + env(pay(alice, carol, MPT(100)), + delivermin(XRP(100)), + ter(temMALFORMED)); + // sendMax MPT is invalid with IOU or XRP + auto const USD = alice["USD"]; + env(pay(alice, carol, USD(100)), + sendmax(MPT(100)), + ter(temMALFORMED)); + env(pay(alice, carol, XRP(100)), + sendmax(MPT(100)), + ter(temMALFORMED)); + // sendmax and amount are different MPT issue + test::jtx::MPT const MPT1( + "MPT", makeMptID(env.seq(alice) + 10, alice)); + env(pay(alice, carol, MPT1(100)), + sendmax(MPT(100)), + ter(temMALFORMED)); + // paths is invalid + env(pay(alice, carol, MPT(100)), path(~USD), ter(temMALFORMED)); + } + + // build_path is invalid if MPT { Env env{*this, features}; + Account const alice("alice"); + Account const carol("carol"); MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); - mptAlice.create( - {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + auto const MPT = mptAlice["MPT"]; + + mptAlice.authorize({.account = &carol}); + + Json::Value payment; + payment[jss::secret] = alice.name(); + payment[jss::tx_json] = pay(alice, carol, MPT(100)); + + payment[jss::build_path] = true; + auto jrr = env.rpc("json", "submit", to_string(payment)); + BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); + BEAST_EXPECT( + jrr[jss::result][jss::error_message] == + "Field 'build_path' not allowed in this context."); + } - // env(mpt::authorize(alice, id.key, std::nullopt)); - // env.close(); + // Can't pay negative amount + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + auto const MPT = mptAlice["MPT"]; mptAlice.authorize({.account = &bob}); - mptAlice.authorize({.account = &carol}); - // issuer to holder - mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, bob, -1, temBAD_AMOUNT); - // holder to issuer - mptAlice.pay(bob, alice, 100); + env(pay(alice, bob, MPT(10)), sendmax(MPT(-1)), ter(temBAD_AMOUNT)); + } - // holder to holder - mptAlice.pay(alice, bob, 100); - mptAlice.pay(bob, carol, 50); + // Pay to self + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + + mptAlice.authorize({.account = &bob}); + + mptAlice.pay(bob, bob, 10, temREDUNDANT); } - // Holder is not authorized + // preclaim validation + + // Destination doesn't exist { Env env{*this, features}; - MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create( - {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + mptAlice.create({.ownerCount = 1, .holderCount = 0}); - // issuer to holder - mptAlice.pay(alice, bob, 100, tecNO_AUTH); + mptAlice.authorize({.account = &bob}); - // holder to issuer - mptAlice.pay(bob, alice, 100, tecNO_AUTH); + Account const bad{"bad"}; + env.memoize(bad); - // holder to holder - mptAlice.pay(bob, carol, 50, tecNO_AUTH); + mptAlice.pay(bob, bad, 10, tecNO_DST); } - // If allowlisting is enabled, Payment fails if the receiver is not + // apply validation + + // If RequireAuth is enabled, Payment fails if the receiver is not // authorized { Env env{*this, features}; @@ -698,7 +820,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(alice, bob, 100, tecNO_AUTH); } - // If allowlisting is enabled, Payment fails if the sender is not + // If RequireAuth is enabled, Payment fails if the sender is not // authorized { Env env{*this, features}; @@ -728,6 +850,54 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(bob, alice, 100, tecNO_AUTH); } + // Non-issuer cannot send to each other if MPTCanTransfer isn't set + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const cindy{"cindy"}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &cindy}}); + + // alice creates issuance without MPTCanTransfer + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + + // bob creates a MPToken + mptAlice.authorize({.account = &bob}); + + // cindy creates a MPToken + mptAlice.authorize({.account = &cindy}); + + // alice pays bob 100 tokens + mptAlice.pay(alice, bob, 100); + + // bob tries to send cindy 10 tokens, but fails because canTransfer + // is off + mptAlice.pay(bob, cindy, 10, tecNO_AUTH); + + // bob can send back to alice(issuer) just fine + mptAlice.pay(bob, alice, 10); + } + + // Holder is not authorized + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + + // issuer to holder + mptAlice.pay(alice, bob, 100, tecNO_AUTH); + + // holder to issuer + mptAlice.pay(bob, alice, 100, tecNO_AUTH); + + // holder to holder + mptAlice.pay(bob, carol, 50, tecNO_AUTH); + } + // Payer doesn't have enough funds { Env env{*this, features}; @@ -787,74 +957,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(bob, alice, 8); } - // Issuer fails trying to send more than the maximum amount allowed - { - Env env{*this, features}; - - MPTTester mptAlice(env, alice, {.holders = {&bob}}); - - mptAlice.create( - {.maxAmt = "100", - .ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer}); - - mptAlice.authorize({.account = &bob}); - - // issuer sends holder the max amount allowed - mptAlice.pay(alice, bob, 100); - - // issuer tries to exceed max amount - mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); - } - - // Issuer fails trying to send more than the default maximum - // amount allowed - { - Env env{*this, features}; - - MPTTester mptAlice(env, alice, {.holders = {&bob}}); - - mptAlice.create({.ownerCount = 1, .holderCount = 0}); - - mptAlice.authorize({.account = &bob}); - - // issuer sends holder the default max amount allowed - mptAlice.pay(alice, bob, maxMPTokenAmount); - - // issuer tries to exceed max amount - mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); - } - - // Can't pay negative amount - { - Env env{*this, features}; - - MPTTester mptAlice(env, alice, {.holders = {&bob}}); - - mptAlice.create({.ownerCount = 1, .holderCount = 0}); - - mptAlice.authorize({.account = &bob}); - - mptAlice.pay(alice, bob, -1, temBAD_AMOUNT); - } - - // pay more than max amount - // fails in the json parser before - // transactor is called - { - Env env{*this, features}; - env.fund(XRP(1'000), alice, bob); - STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; - Json::Value jv; - jv[jss::secret] = alice.name(); - jv[jss::tx_json] = pay(alice, bob, mpt); - jv[jss::tx_json][jss::Amount][jss::value] = - to_string(maxMPTokenAmount + 1); - auto const jrr = env.rpc("json", "submit", to_string(jv)); - BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); - } - // Transfer fee { Env env{*this, features}; @@ -892,133 +994,63 @@ class MPToken_test : public beast::unit_test::suite sendmax(MPT(109)), ter(tecPATH_PARTIAL)); - // Payment succeeds if SendMax is included. + // Payment succeeds if sufficient SendMax is included. env(pay(bob, carol, MPT(100)), sendmax(MPT(110))); env(pay(bob, carol, MPT(100)), sendmax(MPT(115))); } - // Test that non-issuer cannot send to each other if MPTCanTransfer - // isn't set + // Issuer fails trying to send more than the maximum amount allowed { - Env env(*this, features); - Account const alice{"alice"}; - Account const bob{"bob"}; - Account const cindy{"cindy"}; + Env env{*this, features}; - MPTTester mptAlice(env, alice, {.holders = {&bob, &cindy}}); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - // alice creates issuance without MPTCanTransfer - mptAlice.create({.ownerCount = 1, .holderCount = 0}); + mptAlice.create( + {.maxAmt = "100", + .ownerCount = 1, + .holderCount = 0, + .flags = tfMPTCanTransfer}); - // bob creates a MPToken mptAlice.authorize({.account = &bob}); - // cindy creates a MPToken - mptAlice.authorize({.account = &cindy}); - - // alice pays bob 100 tokens + // issuer sends holder the max amount allowed mptAlice.pay(alice, bob, 100); - // bob tries to send cindy 10 tokens, but fails because canTransfer - // is off - mptAlice.pay(bob, cindy, 10, tecNO_AUTH); - - // bob can send back to alice(issuer) just fine - mptAlice.pay(bob, alice, 10); - } - - // MPT is disabled - { - Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); - Account const bob("bob"); - - env.fund(XRP(1'000), alice); - env.fund(XRP(1'000), bob); - STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; - - env(pay(alice, bob, mpt), ter(temDISABLED)); - } - - // MPT is disabled, unsigned request - { - Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - Account const carol("carol"); - auto const USD = alice["USD"]; - - env.fund(XRP(1'000), alice); - env.fund(XRP(1'000), carol); - STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; - - Json::Value jv; - jv[jss::secret] = alice.name(); - jv[jss::tx_json][jss::Fee] = to_string(env.current()->fees().base); - jv[jss::tx_json] = pay(alice, carol, mpt); - auto const jrr = env.rpc("json", "submit", to_string(jv)); - BEAST_EXPECT(jrr[jss::result][jss::engine_result] == "temDISABLED"); + // issuer tries to exceed max amount + mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); } - // Invalid combination of send, sendMax, deliverMin + // Issuer fails trying to send more than the default maximum + // amount allowed { Env env{*this, features}; - Account const alice("alice"); - Account const carol("carol"); - MPTTester mptAlice(env, alice, {.holders = {&carol}}); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); mptAlice.create({.ownerCount = 1, .holderCount = 0}); - mptAlice.authorize({.account = &carol}); + mptAlice.authorize({.account = &bob}); - // sendMax and DeliverMin are valid XRP amount, - // but is invalid combination with MPT amount - auto const MPT = mptAlice["MPT"]; - env(pay(alice, carol, MPT(100)), - sendmax(XRP(100)), - ter(temMALFORMED)); - env(pay(alice, carol, MPT(100)), - delivermin(XRP(100)), - ter(temMALFORMED)); - // sendMax MPT is invalid with IOU or XRP - auto const USD = alice["USD"]; - env(pay(alice, carol, USD(100)), - sendmax(MPT(100)), - ter(temMALFORMED)); - env(pay(alice, carol, XRP(100)), - sendmax(MPT(100)), - ter(temMALFORMED)); - // sendmax and amount are different MPT issue - test::jtx::MPT const MPT1( - "MPT", makeMptID(env.seq(alice) + 10, alice)); - env(pay(alice, carol, MPT1(100)), - sendmax(MPT(100)), - ter(temMALFORMED)); + // issuer sends holder the default max amount allowed + mptAlice.pay(alice, bob, maxMPTokenAmount); + + // issuer tries to exceed max amount + mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); } - // build_path is invalid if MPT + // pay more than max amount fails in the json parser before + // transactor is called { Env env{*this, features}; - Account const alice("alice"); - Account const carol("carol"); - - MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); - - mptAlice.create({.ownerCount = 1, .holderCount = 0}); - auto const MPT = mptAlice["MPT"]; - - mptAlice.authorize({.account = &carol}); - - Json::Value payment; - payment[jss::secret] = alice.name(); - payment[jss::tx_json] = pay(alice, carol, MPT(100)); - - payment[jss::build_path] = true; - auto jrr = env.rpc("json", "submit", to_string(payment)); + env.fund(XRP(1'000), alice, bob); + STAmount mpt{MPTIssue{makeMptID(1, alice)}, UINT64_C(100)}; + Json::Value jv; + jv[jss::secret] = alice.name(); + jv[jss::tx_json] = pay(alice, bob, mpt); + jv[jss::tx_json][jss::Amount][jss::value] = + to_string(maxMPTokenAmount + 1); + auto const jrr = env.rpc("json", "submit", to_string(jv)); BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); - BEAST_EXPECT( - jrr[jss::result][jss::error_message] == - "Field 'build_path' not allowed in this context."); } // Issuer fails trying to send fund after issuance was destroyed @@ -1034,13 +1066,13 @@ class MPToken_test : public beast::unit_test::suite // alice destroys issuance mptAlice.destroy({.ownerCount = 0}); - // alice tries to send bob fund after issuance is destroy, should + // alice tries to send bob fund after issuance is destroyed, should // fail. mptAlice.pay(alice, bob, 100, tecOBJECT_NOT_FOUND); } - // Issuer fails trying to send to some who doesn't own MPT for a - // issuance that was destroyed + // Issuer fails trying to send to an account, which doesn't own MPT for + // an issuance that was destroyed { Env env{*this, features}; @@ -1077,6 +1109,29 @@ class MPToken_test : public beast::unit_test::suite // transfer max amount to another holder mptAlice.pay(bob, carol, 100); } + + // Simple payment + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + + mptAlice.authorize({.account = &bob}); + mptAlice.authorize({.account = &carol}); + + // issuer to holder + mptAlice.pay(alice, bob, 100); + + // holder to issuer + mptAlice.pay(bob, alice, 100); + + // holder to holder + mptAlice.pay(alice, bob, 100); + mptAlice.pay(bob, carol, 50); + } } void diff --git a/src/test/jtx/amount.h b/src/test/jtx/amount.h index c73ac2bcf09..698526c29f9 100644 --- a/src/test/jtx/amount.h +++ b/src/test/jtx/amount.h @@ -393,25 +393,12 @@ class MPT requires(sizeof(T) >= sizeof(int) && std::is_arithmetic_v) PrettyAmount operator()(T v) const { - // VFALCO NOTE Should throw if the - // representation of v is not exact. return {amountFromString(mpt(), std::to_string(v)), name}; } PrettyAmount operator()(epsilon_t) const; PrettyAmount operator()(detail::epsilon_multiple) const; - // VFALCO TODO - // STAmount operator()(char const* s) const; - - /** Returns None-of-Issue */ -#if 0 - None operator()(none_t) const - { - return {Issue{}}; - } -#endif - friend BookSpec operator~(MPT const& mpt) { diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 16cdc515a08..5fb2fbe1828 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -221,26 +221,28 @@ preflightHelper(PreflightContext const& ctx) if (!ctx.rules.enabled(featureMPTokensV1)) return temDISABLED; - if (ctx.tx.isFieldPresent(sfDeliverMin) || ctx.tx.isFieldPresent(sfPaths)) - return temMALFORMED; - - if (auto const sendMax = ctx.tx[~sfSendMax]; sendMax && - (!sendMax->holds() || - sendMax->asset() != ctx.tx[sfAmount].asset())) - return temMALFORMED; - auto& tx = ctx.tx; auto& j = ctx.j; std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfPaymentMask) + if (uTxFlags & tfUniversalMask) { JLOG(j.trace()) << "Malformed transaction: " << "Invalid flags set."; return temINVALID_FLAG; } + if (ctx.tx.isFieldPresent(sfDeliverMin) || ctx.tx.isFieldPresent(sfPaths)) + return temMALFORMED; + + auto const sendMax = ctx.tx[~sfSendMax]; + + if (sendMax && + (!sendMax->holds() || + sendMax->asset() != ctx.tx[sfAmount].asset())) + return temMALFORMED; + STAmount const saDstAmount(tx.getFieldAmount(sfAmount)); auto const account = tx.getAccountID(sfAccount); @@ -261,6 +263,12 @@ preflightHelper(PreflightContext const& ctx) << "bad dst amount: " << saDstAmount.getFullText(); return temBAD_AMOUNT; } + if (sendMax && *sendMax <= beast::zero) + { + JLOG(j.trace()) << "Malformed transaction: " + << "bad max amount: " << sendMax->getFullText(); + return temBAD_AMOUNT; + } if (account == uDstAccountID) { // You're signing yourself a payment. @@ -269,12 +277,6 @@ preflightHelper(PreflightContext const& ctx) << " to self without path for " << to_string(uDstMptID); return temREDUNDANT; } - if (uTxFlags & (tfPartialPayment | tfLimitQuality | tfNoRippleDirect)) - { - JLOG(j.trace()) << "Malformed transaction: invalid MPT flags: " - << uTxFlags; - return temMALFORMED; - } return preflight2(ctx); }