From 189362ee7990d4445f5e27d267626e4b96e52fee Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Tue, 13 Aug 2024 11:25:00 -0400 Subject: [PATCH] Add invalid MPT amount check in STTx::passesLocalChecks() to provide one single check for all tx. Update unit-tests. --- include/xrpl/protocol/MPTIssue.h | 3 + include/xrpl/protocol/SOTemplate.h | 8 +- include/xrpl/protocol/STEitherAmount.h | 3 - include/xrpl/protocol/STObject.h | 2 + include/xrpl/protocol/XChainAttestations.h | 6 +- src/libxrpl/protocol/MPTIssue.cpp | 9 + src/libxrpl/protocol/STEitherAmount.cpp | 12 +- src/libxrpl/protocol/STObject.cpp | 19 +- src/libxrpl/protocol/STTx.cpp | 32 ++ src/libxrpl/protocol/XChainAttestations.cpp | 8 +- src/test/app/MPToken_test.cpp | 447 +++++++++++++----- src/test/jtx/attester.h | 6 +- src/test/jtx/impl/attester.cpp | 6 +- src/test/jtx/impl/xchain_bridge.cpp | 8 +- src/test/jtx/xchain_bridge.h | 2 +- src/xrpld/app/tx/detail/AMMCreate.cpp | 3 - src/xrpld/app/tx/detail/AMMDeposit.cpp | 3 - src/xrpld/app/tx/detail/AMMWithdraw.cpp | 3 - src/xrpld/app/tx/detail/CashCheck.cpp | 3 - .../app/tx/detail/NFTokenCreateOffer.cpp | 3 - src/xrpld/app/tx/detail/NFTokenMint.cpp | 3 - src/xrpld/app/tx/detail/XChainBridge.cpp | 12 - src/xrpld/rpc/handlers/Submit.cpp | 2 - 23 files changed, 407 insertions(+), 196 deletions(-) diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h index dc16d207a87..6c01baccf08 100644 --- a/include/xrpl/protocol/MPTIssue.h +++ b/include/xrpl/protocol/MPTIssue.h @@ -84,6 +84,9 @@ isXRP(uint192 const&) return false; } +Json::Value +to_json(MPTIssue const& issue); + } // namespace ripple #endif // RIPPLE_PROTOCOL_MPTISSUE_H_INCLUDED diff --git a/include/xrpl/protocol/SOTemplate.h b/include/xrpl/protocol/SOTemplate.h index 0ea420a483b..56a0d471150 100644 --- a/include/xrpl/protocol/SOTemplate.h +++ b/include/xrpl/protocol/SOTemplate.h @@ -69,9 +69,7 @@ class SOElement public: SOElement(SField const& fieldName, SOEStyle style) - : sField_(fieldName) - , style_(style) - , supportMpt_(SOETxMPTAmount::soeMPTNone) + : sField_(fieldName), style_(style), supportMpt_(soeMPTNone) { init(fieldName); } @@ -101,10 +99,10 @@ class SOElement return style_; } - bool + SOETxMPTAmount supportMPT() const { - return supportMpt_ == soeMPTSupported; + return supportMpt_; } }; diff --git a/include/xrpl/protocol/STEitherAmount.h b/include/xrpl/protocol/STEitherAmount.h index 3c62b42986c..c16c8a1d6a1 100644 --- a/include/xrpl/protocol/STEitherAmount.h +++ b/include/xrpl/protocol/STEitherAmount.h @@ -205,9 +205,6 @@ operator==(STEitherAmount const& lhs, STEitherAmount const& rhs) }, lhs.getValue(), rhs.getValue()); - if (lhs.isIssue() == rhs.isIssue()) - return lhs.getValue() == rhs.getValue(); - return false; } inline bool diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index fb7d15d3630..26cdf5a55ea 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -240,6 +240,8 @@ class STObject : public STBase, public CountedObject Blob getFieldVL(SField const& field) const; STEitherAmount const& + getFieldEitherAmount(SField const& field) const; + STEitherAmount const& getFieldAmount(SField const& field) const; STAmount const& getFieldAmount(TypedFieldAmount const& field) const; diff --git a/include/xrpl/protocol/XChainAttestations.h b/include/xrpl/protocol/XChainAttestations.h index 721950ca9c1..ea715aa30ab 100644 --- a/include/xrpl/protocol/XChainAttestations.h +++ b/include/xrpl/protocol/XChainAttestations.h @@ -143,7 +143,7 @@ struct AttestationClaim : AttestationBase message( STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, + STEitherAmount const& sendingAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t claimID, @@ -226,8 +226,8 @@ struct AttestationCreateAccount : AttestationBase message( STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, - STAmount const& rewardAmount, + STEitherAmount const& sendingAmount, + STEitherAmount const& rewardAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t createCount, diff --git a/src/libxrpl/protocol/MPTIssue.cpp b/src/libxrpl/protocol/MPTIssue.cpp index 28806dce3aa..4737f46a938 100644 --- a/src/libxrpl/protocol/MPTIssue.cpp +++ b/src/libxrpl/protocol/MPTIssue.cpp @@ -19,6 +19,7 @@ #include #include +#include namespace ripple { @@ -67,4 +68,12 @@ getMPT(uint192 const& id) return std::make_pair(sequence, account); } +Json::Value +to_json(MPTIssue const& issue) +{ + Json::Value jv; + jv[jss::mpt_issuance_id] = to_string(issue.getMptID()); + return jv; +} + } // namespace ripple diff --git a/src/libxrpl/protocol/STEitherAmount.cpp b/src/libxrpl/protocol/STEitherAmount.cpp index a060d434ff6..eac706a8729 100644 --- a/src/libxrpl/protocol/STEitherAmount.cpp +++ b/src/libxrpl/protocol/STEitherAmount.cpp @@ -18,8 +18,6 @@ //============================================================================== #include -#include -#include #include #include #include @@ -246,14 +244,10 @@ amountFromJson(SField const& name, Json::Value const& v) value = v[jss::value]; if (v.isMember(jss::mpt_issuance_id)) { - std::optional const& rules = getCurrentTransactionRules(); - if (!rules || (rules && rules->enabled(featureMPTokensV1))) - { - isMPT = true; - currencyOrMPTID = v[jss::mpt_issuance_id]; - } + isMPT = true; + currencyOrMPTID = v[jss::mpt_issuance_id]; } - if (!isMPT) + else { currencyOrMPTID = v[jss::currency]; issuer = v[jss::issuer]; diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index a5eb0e6f45b..ec23b16793d 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -165,18 +165,6 @@ STObject::applyTemplate(const SOTemplate& type) e.sField().fieldName, "may not be explicitly set to default."); } - if (iter->get().getSType() == STI_AMOUNT && !e.supportMPT()) - { - if (auto const v = dynamic_cast(&iter->get()); - v && v->isMPT()) - { - std::optional const& rules = - getCurrentTransactionRules(); - if (!rules || rules->enabled(featureMPTokensV1)) - throwFieldErr( - e.sField().fieldName, "doesn't support MPT"); - } - } v.emplace_back(std::move(*iter)); v_.erase(iter); } @@ -642,6 +630,13 @@ STObject::getFieldVL(SField const& field) const return Blob(b.data(), b.data() + b.size()); } +STEitherAmount const& +STObject::getFieldEitherAmount(SField const& field) const +{ + static STEitherAmount const empty{}; + return getFieldByConstRef(field, empty); +} + STEitherAmount const& STObject::getFieldAmount(SField const& field) const { diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 149186d43ce..86dc28fe3ee 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -543,6 +543,31 @@ isAccountFieldOkay(STObject const& st) return true; } +static bool +invalidMPTAmountInTx(STObject const& tx) +{ + auto const txType = tx[~sfTransactionType]; + if (!txType) + return false; + if (auto const* item = TxFormats::getInstance().findByType(safe_cast(*txType))) + { + for (auto const& e : item->getSOTemplate()) + { + if (tx.isFieldPresent(e.sField()) && e.supportMPT() != soeMPTNone) + { + if (auto const& field = tx.peekAtField(e.sField()); + field.getSType() == STI_AMOUNT && + static_cast(field).isMPT()) + { + if (e.supportMPT() == soeMPTNotSupported) + return true; + } + } + } + } + return false; +} + bool passesLocalChecks(STObject const& st, std::string& reason) { @@ -560,6 +585,13 @@ passesLocalChecks(STObject const& st, std::string& reason) reason = "Cannot submit pseudo transactions."; return false; } + + if (invalidMPTAmountInTx(st)) + { + reason = "Amount can not be MPT."; + return false; + } + return true; } diff --git a/src/libxrpl/protocol/XChainAttestations.cpp b/src/libxrpl/protocol/XChainAttestations.cpp index 2b0c1004a3a..2d736ebff43 100644 --- a/src/libxrpl/protocol/XChainAttestations.cpp +++ b/src/libxrpl/protocol/XChainAttestations.cpp @@ -216,7 +216,7 @@ std::vector AttestationClaim::message( STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, + STEitherAmount const& sendingAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t claimID, @@ -225,7 +225,7 @@ AttestationClaim::message( STObject o{sfGeneric}; // Serialize in SField order to make python serializers easier to write o[sfXChainClaimID] = claimID; - o[sfAmount] = STEitherAmount{sendingAmount}; + o[sfAmount] = sendingAmount; if (dst) o[sfDestination] = *dst; o[sfOtherChainSource] = sendingAccount; @@ -361,8 +361,8 @@ std::vector AttestationCreateAccount::message( STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, - STAmount const& rewardAmount, + STEitherAmount const& sendingAmount, + STEitherAmount const& rewardAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t createCount, diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index f46b5053c6f..f68f1c65bcf 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -647,7 +648,7 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); mptAlice.create( - {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); // issuer to holder mptAlice.pay(alice, bob, 100, tecNO_AUTH); @@ -881,43 +882,49 @@ class MPToken_test : public beast::unit_test::suite 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); + STMPTAmount mpt{ + MPTIssue{std::make_pair(1, alice.id())}, 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 combination of send, sendMax, deliverMin { Env env{*this, features}; Account const alice("alice"); Account const carol("carol"); - Account const bob("bob"); - MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + MPTTester mptAlice(env, alice, {.holders = {&carol}}); mptAlice.create({.ownerCount = 1, .holderCount = 0}); - mptAlice.authorize({.account = &bob}); - mptAlice.authorize({.account = &carol}); - env(pay(alice, bob, mptAlice.mpt(100)), + // sendMax and DeliverMin are valid XRP amount, + // but is invalid combination with MPT amount + env(pay(alice, carol, mptAlice.mpt(100)), sendmax(XRP(100)), - delivermin(XRP(100)), - ter(temMALFORMED)); - - env(pay(alice, bob, mptAlice.mpt(100)), - sendmax(mptAlice.mpt(100)), - delivermin(XRP(100)), ter(temMALFORMED)); - - env(pay(alice, bob, XRP(100)), - sendmax(mptAlice.mpt(100)), + env(pay(alice, carol, mptAlice.mpt(100)), delivermin(XRP(100)), ter(temMALFORMED)); - - env(pay(alice, bob, XRP(100)), - sendmax(XRP(100)), - delivermin(mptAlice.mpt(100)), - ter(temMALFORMED)); } - // Unsigned request + // build_path is invalid if MPT { Env env{*this, features}; Account const alice("alice"); @@ -932,37 +939,13 @@ class MPToken_test : public beast::unit_test::suite Json::Value payment; payment[jss::secret] = alice.name(); payment[jss::tx_json] = pay(alice, carol, mptAlice.mpt(100)); - payment[jss::tx_json][jss::SendMax] = - mptAlice.mpt(100).getJson(JsonOptions::none); - payment[jss::tx_json][jss::Fee] = - to_string(env.current()->fees().base); - 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 'SendMax' doesn't support MPT"); - payment[jss::tx_json].removeMember(jss::SendMax); - payment[jss::tx_json][jss::DeliverMin] = - mptAlice.mpt(100).getJson(JsonOptions::none); - 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 'DeliverMin' doesn't support MPT"); - - payment[jss::tx_json].removeMember(jss::DeliverMin); payment[jss::build_path] = true; - jrr = env.rpc("json", "submit", to_string(payment)); + 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."); - - payment.removeMember(jss::build_path); - jrr = env.rpc("json", "submit", to_string(payment)); - BEAST_EXPECT(jrr[jss::result][jss::status] == "success"); - BEAST_EXPECT(jrr[jss::result][jss::engine_result] == "tesSUCCESS"); } } @@ -972,82 +955,312 @@ class MPToken_test : public beast::unit_test::suite testcase("MPT Amount Invalid in Transaction"); using namespace test::jtx; + std::set txWithAmounts; + for (auto const& format : TxFormats::getInstance()) { - Env env{*this, features}; - Account const alice("alice"); // issuer - - MPTTester mptAlice(env, alice); - - mptAlice.create(); - - env(offer(alice, mptAlice.mpt(100), XRP(100)), ter(temMALFORMED)); - env.close(); - - BEAST_EXPECT(expectOffers(env, alice, 0)); - } - - // Unsigned request - { - Env env{*this, features}; - Account const alice("alice"); // issuer - auto const USD = alice["USD"]; - - MPTTester mptAlice(env, alice); - - mptAlice.create(); - - Json::Value jv; - jv[jss::secret] = alice.name(); - jv[jss::tx_json] = offer(alice, USD(100), mptAlice.mpt(100)); - jv[jss::tx_json][jss::Fee] = to_string(env.current()->fees().base); - auto 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 'TakerGets' doesn't support MPT"); - - jv[jss::tx_json] = offer(alice, mptAlice.mpt(100), USD(100)); - 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 'TakerPays' doesn't support MPT"); - } - - { - Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); - - env.fund(XRP(1'000), alice); - STMPTAmount mpt{ - MPTIssue{std::make_pair(1, alice.id())}, UINT64_C(100)}; - - env(offer(alice, mpt, XRP(100)), ter(temMALFORMED)); - env.close(); - - BEAST_EXPECT(expectOffers(env, alice, 0)); + for (auto const& e : format.getSOTemplate()) + { + // Transaction has amount fields. + // Exclude Clawback, which only supports sfAmount and is checked + // in the transactor for amendment enable/disable. Exclude + // pseudo-transaction SetFee. Don't consider the Fee field since + // it's included in every transaction. + if (e.supportMPT() != soeMPTNone && + e.sField().getName() != jss::Fee && + format.getName() != jss::Clawback && + format.getName() != jss::SetFee) + { + txWithAmounts.insert(format.getName()); + break; + } + } } - // Unsigned request, MPT amendment not enabled + Account const alice("alice"); + auto const USD = alice["USD"]; + Account const carol("carol"); + MPTIssue issue(std::make_pair(1, alice.id())); + STMPTAmount mpt{issue, UINT64_C(100)}; + auto const jvb = bridge(alice, USD, alice, USD); + for (auto const& feature : {features, features - featureMPTokensV1}) { - Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - auto const USD = alice["USD"]; - + Env env{*this, feature}; env.fund(XRP(1'000), alice); - STMPTAmount mpt{ - MPTIssue{std::make_pair(1, alice.id())}, UINT64_C(100)}; - - Json::Value jv; - jv[jss::secret] = alice.name(); - jv[jss::tx_json] = offer(alice, USD(100), mpt); - jv[jss::tx_json][jss::Fee] = to_string(env.current()->fees().base); - auto 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 'tx_json.TakerGets' has invalid data."); + env.fund(XRP(1'000), carol); + auto test = [&](Json::Value const& jv) { + txWithAmounts.erase(jv[jss::TransactionType].asString()); + + // tx is signed + auto jtx = env.jt(jv); + Serializer s; + jtx.stx->add(s); + auto jrr = env.rpc("submit", strHex(s.slice())); + BEAST_EXPECT( + jrr[jss::result][jss::error] == "invalidTransaction"); + + // tx is unsigned + Json::Value jv1; + jv1[jss::secret] = alice.name(); + jv1[jss::tx_json] = jv; + jrr = env.rpc("json", "submit", to_string(jv1)); + BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); + }; + // All transactions with sfAmount, which don't support MPT + // and transactions with amount fields, which can't be MPT + + // AMMCreate + auto ammCreate = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMCreate; + jv[jss::Account] = alice.human(); + jv[jss::Amount] = (field.fieldName == sfAmount.fieldName) + ? mpt.getJson(JsonOptions::none) + : "100000000"; + jv[jss::Amount2] = (field.fieldName == sfAmount2.fieldName) + ? mpt.getJson(JsonOptions::none) + : "100000000"; + jv[jss::TradingFee] = 0; + test(jv); + }; + ammCreate(sfAmount); + ammCreate(sfAmount2); + // AMMDeposit + auto ammDeposit = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMDeposit; + jv[jss::Account] = alice.human(); + jv[jss::Asset] = to_json(xrpIssue()); + jv[jss::Asset2] = to_json(USD.issue()); + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + jv[jss::Flags] = tfSingleAsset; + test(jv); + }; + ammDeposit(sfAmount); + for (SField const& field : + {std::ref(sfAmount2), + std::ref(sfEPrice), + std::ref(sfLPTokenOut)}) + ammDeposit(field); + // AMMWithdraw + auto ammWithdraw = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMWithdraw; + jv[jss::Account] = alice.human(); + jv[jss::Asset] = to_json(xrpIssue()); + jv[jss::Asset2] = to_json(USD.issue()); + jv[jss::Flags] = tfSingleAsset; + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + test(jv); + }; + ammWithdraw(sfAmount); + for (SField const& field : + {std::ref(sfAmount2), + std::ref(sfEPrice), + std::ref(sfLPTokenIn)}) + ammWithdraw(field); + // AMMBid + auto ammBid = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMBid; + jv[jss::Account] = alice.human(); + jv[jss::Asset] = to_json(xrpIssue()); + jv[jss::Asset2] = to_json(USD.issue()); + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + test(jv); + }; + ammBid(sfBidMin); + ammBid(sfBidMax); + // CheckCash + auto checkCash = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::CheckCash; + jv[jss::Account] = alice.human(); + jv[sfCheckID.fieldName] = to_string(uint256{1}); + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + test(jv); + }; + checkCash(sfAmount); + checkCash(sfDeliverMin); + // CheckCreate + { + Json::Value jv; + jv[jss::TransactionType] = jss::CheckCreate; + jv[jss::Account] = alice.human(); + jv[jss::Destination] = carol.human(); + jv[jss::SendMax] = mpt.getJson(JsonOptions::none); + test(jv); + } + // Clawback + if (!feature[featureMPTokensV1]) + { + } + // EscrowCreate + { + Json::Value jv; + jv[jss::TransactionType] = jss::EscrowCreate; + jv[jss::Account] = alice.human(); + jv[jss::Destination] = carol.human(); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // OfferCreate + { + Json::Value const jv = offer(alice, USD(100), mpt); + test(jv); + } + // PaymentChannelCreate + { + Json::Value jv; + jv[jss::TransactionType] = jss::PaymentChannelCreate; + jv[jss::Account] = alice.human(); + jv[jss::Destination] = carol.human(); + jv[jss::SettleDelay] = 1; + jv[sfPublicKey.fieldName] = strHex(alice.pk().slice()); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // PaymentChannelFund + { + Json::Value jv; + jv[jss::TransactionType] = jss::PaymentChannelFund; + jv[jss::Account] = alice.human(); + jv[sfChannel.fieldName] = to_string(uint256{1}); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // PaymentChannelClaim + { + Json::Value jv; + jv[jss::TransactionType] = jss::PaymentChannelClaim; + jv[jss::Account] = alice.human(); + jv[sfChannel.fieldName] = to_string(uint256{1}); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // Payment + auto payment = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::Payment; + jv[jss::Account] = alice.human(); + jv[jss::Destination] = carol.human(); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + if (field == sfSendMax) + jv[jss::SendMax] = mpt.getJson(JsonOptions::none); + else + jv[jss::DeliverMin] = mpt.getJson(JsonOptions::none); + test(jv); + }; + payment(sfSendMax); + payment(sfDeliverMin); + // NFTokenCreateOffer + { + Json::Value jv; + jv[jss::TransactionType] = jss::NFTokenCreateOffer; + jv[jss::Account] = alice.human(); + jv[sfNFTokenID.fieldName] = to_string(uint256{1}); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // NFTokenAcceptOffer + { + Json::Value jv; + jv[jss::TransactionType] = jss::NFTokenAcceptOffer; + jv[jss::Account] = alice.human(); + jv[sfNFTokenBrokerFee.fieldName] = + mpt.getJson(JsonOptions::none); + test(jv); + } + // NFTokenMint + { + Json::Value jv; + jv[jss::TransactionType] = jss::NFTokenMint; + jv[jss::Account] = alice.human(); + jv[sfNFTokenTaxon.fieldName] = 1; + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv); + } + // TrustSet + auto trustSet = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::TrustSet; + jv[jss::Account] = alice.human(); + jv[jss::Flags] = 0; + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + test(jv); + }; + trustSet(sfLimitAmount); + trustSet(sfFee); + // XChainCommit + { + Json::Value const jv = xchain_commit(alice, jvb, 1, mpt); + test(jv); + } + // XChainClaim + { + Json::Value const jv = xchain_claim(alice, jvb, 1, mpt, alice); + test(jv); + } + // XChainCreateClaimID + { + Json::Value const jv = + xchain_create_claim_id(alice, jvb, mpt, alice); + test(jv); + } + // XChainAddClaimAttestation + { + Json::Value const jv = claim_attestation( + alice, + jvb, + alice, + mpt, + alice, + true, + 1, + alice, + signer(alice)); + test(jv); + } + // XChainAddAccountCreateAttestation + { + Json::Value const jv = create_account_attestation( + alice, + jvb, + alice, + mpt, + XRP(10), + alice, + false, + 1, + alice, + signer(alice)); + test(jv); + } + // XChainAccountCreateCommit + { + Json::Value const jv = sidechain_xchain_account_create( + alice, jvb, alice, mpt, XRP(10)); + test(jv); + } + // XChain[Create|Modify]Bridge + auto bridgeTx = [&](Json::StaticString const& tt, + bool minAmount = false) { + Json::Value jv; + jv[jss::TransactionType] = tt; + jv[jss::Account] = alice.human(); + jv[sfXChainBridge.fieldName] = jvb; + jv[sfSignatureReward.fieldName] = + mpt.getJson(JsonOptions::none); + if (minAmount) + jv[sfMinAccountCreateAmount.fieldName] = + mpt.getJson(JsonOptions::none); + test(jv); + }; + bridgeTx(jss::XChainCreateBridge); + bridgeTx(jss::XChainCreateBridge, true); + bridgeTx(jss::XChainModifyBridge); + bridgeTx(jss::XChainModifyBridge, true); } + BEAST_EXPECT(txWithAmounts.empty()); } void @@ -1347,7 +1560,7 @@ class MPToken_test : public beast::unit_test::suite // Test Direct Payment testPayment(all); - // Test MPT Amount is invalid in non-Payment Tx + // Test MPT Amount is invalid in Tx, which don't support MPT testMPTInvalidInTx(all); // Test parsed MPTokenIssuanceID in API response metadata diff --git a/src/test/jtx/attester.h b/src/test/jtx/attester.h index 327fb2e4873..8bb5d57196a 100644 --- a/src/test/jtx/attester.h +++ b/src/test/jtx/attester.h @@ -42,7 +42,7 @@ sign_claim_attestation( SecretKey const& sk, STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, + STEitherAmount const& sendingAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t claimID, @@ -54,8 +54,8 @@ sign_create_account_attestation( SecretKey const& sk, STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, - STAmount const& rewardAmount, + STEitherAmount const& sendingAmount, + STEitherAmount const& rewardAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t createCount, diff --git a/src/test/jtx/impl/attester.cpp b/src/test/jtx/impl/attester.cpp index 66be9da83b3..02a61ffe9bd 100644 --- a/src/test/jtx/impl/attester.cpp +++ b/src/test/jtx/impl/attester.cpp @@ -35,7 +35,7 @@ sign_claim_attestation( SecretKey const& sk, STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, + STEitherAmount const& sendingAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t claimID, @@ -58,8 +58,8 @@ sign_create_account_attestation( SecretKey const& sk, STXChainBridge const& bridge, AccountID const& sendingAccount, - STAmount const& sendingAmount, - STAmount const& rewardAmount, + STEitherAmount const& sendingAmount, + STEitherAmount const& rewardAmount, AccountID const& rewardAccount, bool wasLockingChainSend, std::uint64_t createCount, diff --git a/src/test/jtx/impl/xchain_bridge.cpp b/src/test/jtx/impl/xchain_bridge.cpp index 53b972288a6..32d24c0027a 100644 --- a/src/test/jtx/impl/xchain_bridge.cpp +++ b/src/test/jtx/impl/xchain_bridge.cpp @@ -115,7 +115,7 @@ Json::Value xchain_create_claim_id( Account const& acc, Json::Value const& bridge, - STAmount const& reward, + STEitherAmount const& reward, Account const& otherChainSource) { Json::Value jv; @@ -216,7 +216,7 @@ claim_attestation( sk, stBridge, sendingAccount, - get(sendingAmount.value), + sendingAmount.value, rewardAccount, wasLockingChainSend, claimID, @@ -269,8 +269,8 @@ create_account_attestation( sk, stBridge, sendingAccount, - get(sendingAmount.value), - get(rewardAmount.value), + sendingAmount.value, + rewardAmount.value, rewardAccount, wasLockingChainSend, createCount, diff --git a/src/test/jtx/xchain_bridge.h b/src/test/jtx/xchain_bridge.h index 9968317c8de..dd219d0dfcd 100644 --- a/src/test/jtx/xchain_bridge.h +++ b/src/test/jtx/xchain_bridge.h @@ -61,7 +61,7 @@ Json::Value xchain_create_claim_id( Account const& acc, Json::Value const& bridge, - STAmount const& reward, + STEitherAmount const& reward, Account const& otherChainSource); Json::Value diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 12fee14e6ec..3ed35d72656 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -41,9 +41,6 @@ AMMCreate::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) { JLOG(ctx.j.debug()) << "AMM Instance: invalid flags."; diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index be6f2303e52..75993ad4189 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -41,9 +41,6 @@ AMMDeposit::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[~sfAmount])) - return temMALFORMED; - auto const flags = ctx.tx.getFlags(); if (flags & tfDepositMask) { diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 110291e9ac7..394fd79dd8c 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -41,9 +41,6 @@ AMMWithdraw::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[~sfAmount])) - return temMALFORMED; - auto const flags = ctx.tx.getFlags(); if (flags & tfWithdrawMask) { diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index 37b9c1348b9..585bf9dcc99 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -42,9 +42,6 @@ CashCheck::preflight(PreflightContext const& ctx) if (!isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[~sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) { // There are no flags (other than universal) for CashCheck yet. diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp index 4321253204d..c132df1ffda 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp @@ -36,9 +36,6 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - auto const txFlags = ctx.tx.getFlags(); if (txFlags & tfNFTokenCreateOfferMask) diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index bea1fee4b7e..06ff1932f3f 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -53,9 +53,6 @@ NFTokenMint::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[~sfAmount])) - return temMALFORMED; - // Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between // accounts allowed a TrustLine to be added to the issuer of that token // without explicit permission from that issuer. This was enabled by diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index f611aed7d48..3154d605ca1 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -1214,9 +1214,6 @@ attestationPreflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -1675,9 +1672,6 @@ XChainClaim::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -1916,9 +1910,6 @@ XChainCommit::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -2190,9 +2181,6 @@ XChainCreateAccountCommit::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.rules.enabled(featureMPTokensV1) && isMPT(ctx.tx[sfAmount])) - return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; diff --git a/src/xrpld/rpc/handlers/Submit.cpp b/src/xrpld/rpc/handlers/Submit.cpp index b323f592ab1..73fdc3822c2 100644 --- a/src/xrpld/rpc/handlers/Submit.cpp +++ b/src/xrpld/rpc/handlers/Submit.cpp @@ -50,8 +50,6 @@ doSubmit(RPC::JsonContext& context) if (!context.params.isMember(jss::tx_blob)) { - CurrentTransactionRulesGuard currentTransactionRulesGuard( - context.ledgerMaster.getCurrentLedger()->rules()); auto const failType = getFailHard(context); if (context.role != Role::ADMIN && !context.app.config().canSign())