From 34f7bc750235b6cae6f0567aabc46b4969d279b4 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 19 Mar 2025 20:02:57 -0400 Subject: [PATCH] refactor: Check transaction flags in preflight0 - Adds a flagMask parameter to preflight1 so that it's impossible to forget to check flags. - Also adds a short hash prefix to all Transactor log messages. --- include/xrpl/basics/base_uint.h | 7 ++++ include/xrpl/protocol/TxFlags.h | 2 ++ .../xrpl/protocol/detail/transactions.macro | 4 ++- src/test/basics/base_uint_test.cpp | 5 +++ src/xrpld/app/tx/detail/AMMBid.cpp | 8 +---- src/xrpld/app/tx/detail/AMMClawback.cpp | 8 ++--- src/xrpld/app/tx/detail/AMMCreate.cpp | 8 +---- src/xrpld/app/tx/detail/AMMDelete.cpp | 8 +---- src/xrpld/app/tx/detail/AMMDeposit.cpp | 7 +--- src/xrpld/app/tx/detail/AMMVote.cpp | 8 +---- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 7 +--- src/xrpld/app/tx/detail/CancelCheck.cpp | 10 +----- src/xrpld/app/tx/detail/CancelOffer.cpp | 11 +----- src/xrpld/app/tx/detail/CashCheck.cpp | 10 +----- src/xrpld/app/tx/detail/Change.cpp | 8 +++-- src/xrpld/app/tx/detail/Clawback.cpp | 5 +-- src/xrpld/app/tx/detail/CreateCheck.cpp | 9 +---- src/xrpld/app/tx/detail/CreateOffer.cpp | 8 +---- src/xrpld/app/tx/detail/CreateTicket.cpp | 7 ++-- src/xrpld/app/tx/detail/Credentials.cpp | 33 +++++------------ src/xrpld/app/tx/detail/DID.cpp | 10 ++---- src/xrpld/app/tx/detail/DeleteAccount.cpp | 5 +-- src/xrpld/app/tx/detail/DeleteOracle.cpp | 8 +---- src/xrpld/app/tx/detail/DepositPreauth.cpp | 10 +----- src/xrpld/app/tx/detail/Escrow.cpp | 21 +++++------ src/xrpld/app/tx/detail/InvariantCheck.cpp | 4 +-- src/xrpld/app/tx/detail/LedgerStateFix.cpp | 5 +-- src/xrpld/app/tx/detail/MPTokenAuthorize.cpp | 5 +-- .../app/tx/detail/MPTokenIssuanceCreate.cpp | 5 +-- .../app/tx/detail/MPTokenIssuanceDestroy.cpp | 5 +-- .../app/tx/detail/MPTokenIssuanceSet.cpp | 7 ++-- .../app/tx/detail/NFTokenAcceptOffer.cpp | 5 +-- src/xrpld/app/tx/detail/NFTokenBurn.cpp | 5 +-- .../app/tx/detail/NFTokenCancelOffer.cpp | 5 +-- .../app/tx/detail/NFTokenCreateOffer.cpp | 5 +-- src/xrpld/app/tx/detail/NFTokenMint.cpp | 9 ++--- src/xrpld/app/tx/detail/NFTokenModify.cpp | 5 +-- src/xrpld/app/tx/detail/PayChan.cpp | 21 +++++------ src/xrpld/app/tx/detail/Payment.cpp | 15 +++----- .../tx/detail/PermissionedDomainDelete.cpp | 8 +---- .../app/tx/detail/PermissionedDomainSet.cpp | 8 +---- src/xrpld/app/tx/detail/SetAccount.cpp | 8 +---- src/xrpld/app/tx/detail/SetOracle.cpp | 5 +-- src/xrpld/app/tx/detail/SetRegularKey.cpp | 11 +----- src/xrpld/app/tx/detail/SetSignerList.cpp | 11 ++---- src/xrpld/app/tx/detail/SetTrust.cpp | 8 +---- src/xrpld/app/tx/detail/Transactor.cpp | 19 +++++++--- src/xrpld/app/tx/detail/Transactor.h | 13 ++++--- src/xrpld/app/tx/detail/VaultClawback.cpp | 5 +-- src/xrpld/app/tx/detail/VaultCreate.cpp | 5 +-- src/xrpld/app/tx/detail/VaultDelete.cpp | 5 +-- src/xrpld/app/tx/detail/VaultDeposit.cpp | 5 +-- src/xrpld/app/tx/detail/VaultSet.cpp | 5 +-- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 5 +-- src/xrpld/app/tx/detail/XChainBridge.cpp | 35 ++++--------------- 55 files changed, 138 insertions(+), 346 deletions(-) diff --git a/include/xrpl/basics/base_uint.h b/include/xrpl/basics/base_uint.h index 4b93330ef2..133323361d 100644 --- a/include/xrpl/basics/base_uint.h +++ b/include/xrpl/basics/base_uint.h @@ -632,6 +632,13 @@ to_string(base_uint const& a) return strHex(a.cbegin(), a.cend()); } +template +inline std::string +to_short_string(base_uint const& a) +{ + return to_string(a).substr(0, 8) + "..."; +} + template inline std::ostream& operator<<(std::ostream& out, base_uint const& u) diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index c616b638d4..ba3cf2e850 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -123,6 +123,8 @@ constexpr std::uint32_t tfTrustSetMask = // EnableAmendment flags: constexpr std::uint32_t tfGotMajority = 0x00010000; constexpr std::uint32_t tfLostMajority = 0x00020000; +constexpr std::uint32_t tfChangeMask = + ~( tfUniversal | tfGotMajority | tfLostMajority); // PaymentChannelClaim flags: constexpr std::uint32_t tfRenew = 0x00010000; diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 8327937209..c25eff0de3 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -508,6 +508,7 @@ TRANSACTION(ttVAULT_CLAWBACK, 69, VaultClawback, ({ {sfAmount, soeOPTIONAL, soeMPTSupported}, })) +#if 0 /** This transaction creates and updates a Loan Broker */ TRANSACTION(ttLOAN_BROKER_SET, 70, LoanBrokerSet, ({ {sfVaultID, soeREQUIRED}, @@ -537,7 +538,7 @@ TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 73, LoanBrokerCoverWithdraw, ({ })) /** This transaction creates a Loan */ -TRANSACTION(ttLOAN_CREATE, 74, LoanCreate, ({ +TRANSACTION(ttLOAN_SET, 74, LoanSet, ({ {sfLoanBrokerID, soeREQUIRED}, {sfData, soeOPTIONAL}, {sfCounterparty, soeOPTIONAL}, @@ -555,6 +556,7 @@ TRANSACTION(ttLOAN_CREATE, 74, LoanCreate, ({ {sfPaymentInterval, soeOPTIONAL}, {sfGracePeriod, soeOPTIONAL}, })) +#endif /** This system-generated transaction type is used to update the status of the various amendments. diff --git a/src/test/basics/base_uint_test.cpp b/src/test/basics/base_uint_test.cpp index 8058e0d6f0..a6be1f327d 100644 --- a/src/test/basics/base_uint_test.cpp +++ b/src/test/basics/base_uint_test.cpp @@ -152,6 +152,7 @@ struct base_uint_test : beast::unit_test::suite uset.insert(u); BEAST_EXPECT(raw.size() == u.size()); BEAST_EXPECT(to_string(u) == "0102030405060708090A0B0C"); + BEAST_EXPECT(to_short_string(u) == "01020304..."); BEAST_EXPECT(*u.data() == 1); BEAST_EXPECT(u.signum() == 1); BEAST_EXPECT(!!u); @@ -174,6 +175,7 @@ struct base_uint_test : beast::unit_test::suite test96 v{~u}; uset.insert(v); BEAST_EXPECT(to_string(v) == "FEFDFCFBFAF9F8F7F6F5F4F3"); + BEAST_EXPECT(to_short_string(v) == "FEFDFCFB..."); BEAST_EXPECT(*v.data() == 0xfe); BEAST_EXPECT(v.signum() == 1); BEAST_EXPECT(!!v); @@ -194,6 +196,7 @@ struct base_uint_test : beast::unit_test::suite test96 z{beast::zero}; uset.insert(z); BEAST_EXPECT(to_string(z) == "000000000000000000000000"); + BEAST_EXPECT(to_short_string(z) == "00000000..."); BEAST_EXPECT(*z.data() == 0); BEAST_EXPECT(*z.begin() == 0); BEAST_EXPECT(*std::prev(z.end(), 1) == 0); @@ -214,6 +217,7 @@ struct base_uint_test : beast::unit_test::suite BEAST_EXPECT(n == z); n--; BEAST_EXPECT(to_string(n) == "FFFFFFFFFFFFFFFFFFFFFFFF"); + BEAST_EXPECT(to_short_string(n) == "FFFFFFFF..."); n = beast::zero; BEAST_EXPECT(n == z); @@ -224,6 +228,7 @@ struct base_uint_test : beast::unit_test::suite test96 x{zm1 ^ zp1}; uset.insert(x); BEAST_EXPECTS(to_string(x) == "FFFFFFFFFFFFFFFFFFFFFFFE", to_string(x)); + BEAST_EXPECTS(to_short_string(x) == "FFFFFFFF...", to_short_string(x)); BEAST_EXPECT(uset.size() == 4); diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index 6fec46be90..c6c1b22108 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -36,15 +36,9 @@ AMMBid::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Bid: invalid flags."; - return temINVALID_FLAG; - } - if (auto const res = invalidAMMAssetPair( ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 64a42374ec..caac32fde8 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -39,13 +39,9 @@ AMMClawback::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureAMMClawback)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfAMMClawbackMask)) return ret; // LCOV_EXCL_LINE - auto const flags = ctx.tx.getFlags(); - if (flags & tfAMMClawbackMask) - return temINVALID_FLAG; - AccountID const issuer = ctx.tx[sfAccount]; AccountID const holder = ctx.tx[sfHolder]; @@ -63,6 +59,8 @@ AMMClawback::preflight(PreflightContext const& ctx) if (isXRP(asset)) return temMALFORMED; + auto const flags = ctx.tx.getFlags(); + if (flags & tfClawTwoAssets && asset.account != asset2.account) { JLOG(ctx.j.trace()) diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 95cb5bf2e6..c0b9f1685f 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -37,15 +37,9 @@ AMMCreate::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Instance: invalid flags."; - return temINVALID_FLAG; - } - auto const amount = ctx.tx[sfAmount]; auto const amount2 = ctx.tx[sfAmount2]; diff --git a/src/xrpld/app/tx/detail/AMMDelete.cpp b/src/xrpld/app/tx/detail/AMMDelete.cpp index 28d56eab98..c87731eac5 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.cpp +++ b/src/xrpld/app/tx/detail/AMMDelete.cpp @@ -33,15 +33,9 @@ AMMDelete::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Delete: invalid flags."; - return temINVALID_FLAG; - } - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 6a718a3f04..e5d7c1f5bb 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -35,15 +35,10 @@ AMMDeposit::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfDepositMask)) return ret; auto const flags = ctx.tx.getFlags(); - if (flags & tfDepositMask) - { - JLOG(ctx.j.debug()) << "AMM Deposit: invalid flags."; - return temINVALID_FLAG; - } auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index 84d0905a22..d6daf43426 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -33,7 +33,7 @@ AMMVote::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; if (auto const res = invalidAMMAssetPair( @@ -43,12 +43,6 @@ AMMVote::preflight(PreflightContext const& ctx) return res; } - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Vote: invalid flags."; - return temINVALID_FLAG; - } - if (ctx.tx[sfTradingFee] > TRADING_FEE_THRESHOLD) { JLOG(ctx.j.debug()) << "AMM Vote: invalid trading fee."; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 586f453c6f..4d30222326 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -34,15 +34,10 @@ AMMWithdraw::preflight(PreflightContext const& ctx) if (!ammEnabled(ctx.rules)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfWithdrawMask)) return ret; auto const flags = ctx.tx.getFlags(); - if (flags & tfWithdrawMask) - { - JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags."; - return temINVALID_FLAG; - } auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; diff --git a/src/xrpld/app/tx/detail/CancelCheck.cpp b/src/xrpld/app/tx/detail/CancelCheck.cpp index cfa3bd10e2..f2b3f7e020 100644 --- a/src/xrpld/app/tx/detail/CancelCheck.cpp +++ b/src/xrpld/app/tx/detail/CancelCheck.cpp @@ -35,17 +35,9 @@ CancelCheck::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureChecks)) return temDISABLED; - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CreateCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/CancelOffer.cpp b/src/xrpld/app/tx/detail/CancelOffer.cpp index 6d8c077a62..ac151052e7 100644 --- a/src/xrpld/app/tx/detail/CancelOffer.cpp +++ b/src/xrpld/app/tx/detail/CancelOffer.cpp @@ -28,18 +28,9 @@ namespace ripple { NotTEC CancelOffer::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - auto const uTxFlags = ctx.tx.getFlags(); - - if (uTxFlags & tfUniversalMask) - { - JLOG(ctx.j.trace()) - << "Malformed transaction: " << "Invalid flags set."; - return temINVALID_FLAG; - } - if (!ctx.tx[sfOfferSequence]) { JLOG(ctx.j.trace()) << "CancelOffer::preflight: missing sequence"; diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index cccda83a68..55a0b4d5a1 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -38,17 +38,9 @@ CashCheck::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureChecks)) return temDISABLED; - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CashCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - // Exactly one of Amount or DeliverMin must be present. auto const optAmount = ctx.tx[~sfAmount]; auto const optDeliverMin = ctx.tx[~sfDeliverMin]; diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 760d5b3d98..8cd13fb866 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -36,8 +36,12 @@ namespace ripple { NotTEC Change::preflight(PreflightContext const& ctx) { - auto const ret = preflight0(ctx); - if (!isTesSuccess(ret)) + // 0 means "Allow any flags" + // The check for tfChangeMask is gated by LendingProtocol because that + // feature introduced this parameter, and it's not worth adding another + // amendment just for this. + if (auto const ret = preflight0( + ctx, ctx.rules.enabled(featureLendingProtocol) ? tfChangeMask : 0)) return ret; auto account = ctx.tx.getAccountID(sfAccount); diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp index 41ab1256fb..ccec10aed8 100644 --- a/src/xrpld/app/tx/detail/Clawback.cpp +++ b/src/xrpld/app/tx/detail/Clawback.cpp @@ -81,12 +81,9 @@ Clawback::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureClawback)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfClawbackMask)) return ret; - if (ctx.tx.getFlags() & tfClawbackMask) - return temINVALID_FLAG; - if (auto const ret = std::visit( [&](T const&) { return preflightHelper(ctx); }, ctx.tx[sfAmount].asset().value()); diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp index 9baceef944..fd2a45db2e 100644 --- a/src/xrpld/app/tx/detail/CreateCheck.cpp +++ b/src/xrpld/app/tx/detail/CreateCheck.cpp @@ -34,16 +34,9 @@ CreateCheck::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureChecks)) return temDISABLED; - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - // There are no flags (other than universal) for CreateCheck yet. - JLOG(ctx.j.warn()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) { // They wrote a check to themselves. diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index d9bd57ec3c..2f40eeff92 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -42,7 +42,7 @@ CreateOffer::makeTxConsequences(PreflightContext const& ctx) NotTEC CreateOffer::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfOfferCreateMask)) return ret; auto& tx = ctx.tx; @@ -50,12 +50,6 @@ CreateOffer::preflight(PreflightContext const& ctx) std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfOfferCreateMask) - { - JLOG(j.debug()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - bool const bImmediateOrCancel(uTxFlags & tfImmediateOrCancel); bool const bFillOrKill(uTxFlags & tfFillOrKill); diff --git a/src/xrpld/app/tx/detail/CreateTicket.cpp b/src/xrpld/app/tx/detail/CreateTicket.cpp index 594335f489..ff37d44d8d 100644 --- a/src/xrpld/app/tx/detail/CreateTicket.cpp +++ b/src/xrpld/app/tx/detail/CreateTicket.cpp @@ -39,16 +39,13 @@ CreateTicket::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureTicketBatch)) return temDISABLED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; + if (NotTEC const ret{preflight1(ctx, tfUniversalMask)}) + return ret; if (std::uint32_t const count = ctx.tx[sfTicketCount]; count < minValidCount || count > maxValidCount) return temINVALID_COUNT; - if (NotTEC const ret{preflight1(ctx)}; !isTesSuccess(ret)) - return ret; - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index 73c397cf37..44ad951a61 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/src/xrpld/app/tx/detail/Credentials.cpp @@ -57,19 +57,14 @@ CredentialCreate::preflight(PreflightContext const& ctx) return temDISABLED; } - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight1( + ctx, ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0)) return ret; auto const& tx = ctx.tx; auto& j = ctx.j; - if (ctx.rules.enabled(fixInvalidTxFlags) && - (tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialCreate: invalid flags."; - return temINVALID_FLAG; - } - if (!tx[sfSubject]) { JLOG(j.trace()) << "Malformed transaction: Invalid Subject"; @@ -211,16 +206,11 @@ CredentialDelete::preflight(PreflightContext const& ctx) return temDISABLED; } - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight1( + ctx, ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0)) return ret; - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialDelete: invalid flags."; - return temINVALID_FLAG; - } - auto const subject = ctx.tx[~sfSubject]; auto const issuer = ctx.tx[~sfIssuer]; @@ -298,16 +288,11 @@ CredentialAccept::preflight(PreflightContext const& ctx) return temDISABLED; } - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight1( + ctx, ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0)) return ret; - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "CredentialAccept: invalid flags."; - return temINVALID_FLAG; - } - if (!ctx.tx[sfIssuer]) { JLOG(ctx.j.trace()) << "Malformed transaction: Issuer field zeroed."; diff --git a/src/xrpld/app/tx/detail/DID.cpp b/src/xrpld/app/tx/detail/DID.cpp index 31ce7c8770..d6bf07d632 100644 --- a/src/xrpld/app/tx/detail/DID.cpp +++ b/src/xrpld/app/tx/detail/DID.cpp @@ -48,10 +48,7 @@ DIDSet::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureDID)) return temDISABLED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; if (!ctx.tx.isFieldPresent(sfURI) && @@ -177,10 +174,7 @@ DIDDelete::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureDID)) return temDISABLED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; return preflight2(ctx); diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index d562069460..ad72a8ab32 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -47,10 +47,7 @@ DeleteAccount::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) diff --git a/src/xrpld/app/tx/detail/DeleteOracle.cpp b/src/xrpld/app/tx/detail/DeleteOracle.cpp index 78e3d55230..e9dfe185ee 100644 --- a/src/xrpld/app/tx/detail/DeleteOracle.cpp +++ b/src/xrpld/app/tx/detail/DeleteOracle.cpp @@ -32,15 +32,9 @@ DeleteOracle::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featurePriceOracle)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "Oracle Delete: invalid flags."; - return temINVALID_FLAG; - } - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/DepositPreauth.cpp b/src/xrpld/app/tx/detail/DepositPreauth.cpp index f10f09b38f..d7c129ed68 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.cpp +++ b/src/xrpld/app/tx/detail/DepositPreauth.cpp @@ -45,17 +45,9 @@ DepositPreauth::preflight(PreflightContext const& ctx) if (authCredPresent && !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - auto& tx = ctx.tx; - - if (tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - auto const optAuth = ctx.tx[~sfAuthorize]; auto const optUnauth = ctx.tx[~sfUnauthorize]; int const authPresent = static_cast(optAuth.has_value()) + diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 0b58957fcf..894f4a6f3f 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -85,10 +85,9 @@ EscrowCreate::makeTxConsequences(PreflightContext const& ctx) NotTEC EscrowCreate::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = + preflight1(ctx, ctx.rules.enabled(fix1543) ? tfUniversalMask : 0)) return ret; if (!isXRP(ctx.tx[sfAmount])) @@ -296,14 +295,13 @@ checkCondition(Slice f, Slice c) NotTEC EscrowFinish::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = + preflight1(ctx, ctx.rules.enabled(fix1543) ? tfUniversalMask : 0)) return ret; auto const cb = ctx.tx[~sfCondition]; @@ -515,10 +513,9 @@ EscrowFinish::doApply() NotTEC EscrowCancel::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = + preflight1(ctx, ctx.rules.enabled(fix1543) ? tfUniversalMask : 0)) return ret; return preflight2(ctx); diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 4f1a8e4fe1..4598cf4b0a 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1660,8 +1660,8 @@ NoModifiedUnmodifiableFields::finalize( fieldChanged(before, after, sfCloseInterestRate) || fieldChanged(before, after, sfOverpaymentInterestRate) || fieldChanged(before, after, sfStartDate) || - fieldChanged(before, after, sfPaymentInterval) - fieldChanged(before, after, sfGracePeriod); + fieldChanged(before, after, sfPaymentInterval) || + fieldChanged(before, after, sfGracePeriod); break; default: /* diff --git a/src/xrpld/app/tx/detail/LedgerStateFix.cpp b/src/xrpld/app/tx/detail/LedgerStateFix.cpp index 008d9b6ae7..715b140723 100644 --- a/src/xrpld/app/tx/detail/LedgerStateFix.cpp +++ b/src/xrpld/app/tx/detail/LedgerStateFix.cpp @@ -33,10 +33,7 @@ LedgerStateFix::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(fixNFTokenPageLinks)) return temDISABLED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; switch (ctx.tx[sfLedgerFixType]) diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp index 748c05869f..01dc497344 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp @@ -32,12 +32,9 @@ MPTokenAuthorize::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureMPTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfMPTokenAuthorizeMask)) return ret; - if (ctx.tx.getFlags() & tfMPTokenAuthorizeMask) - return temINVALID_FLAG; - if (ctx.tx[sfAccount] == ctx.tx[~sfHolder]) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp index 1b96b27f24..556e665b9c 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp @@ -31,12 +31,9 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureMPTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfMPTokenIssuanceCreateMask)) return ret; - if (ctx.tx.getFlags() & tfMPTokenIssuanceCreateMask) - return temINVALID_FLAG; - if (auto const fee = ctx.tx[~sfTransferFee]) { if (fee > maxTransferFee) diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp index d06ea3473e..d935010023 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp @@ -32,12 +32,9 @@ MPTokenIssuanceDestroy::preflight(PreflightContext const& ctx) return temDISABLED; // check flags - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfMPTokenIssuanceDestroyMask)) return ret; - if (ctx.tx.getFlags() & tfMPTokenIssuanceDestroyMask) - return temINVALID_FLAG; - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index 12208dba1b..1da95ec3d4 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -30,16 +30,13 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureMPTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfMPTokenIssuanceSetMask)) return ret; auto const txFlags = ctx.tx.getFlags(); - // check flags - if (txFlags & tfMPTokenIssuanceSetMask) - return temINVALID_FLAG; // fails if both flags are set - else if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock)) + if ((txFlags & tfMPTLock) && (txFlags & tfMPTUnlock)) return temINVALID_FLAG; auto const accountID = ctx.tx[sfAccount]; diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp index 4c5fdb7683..dc2f37d8ec 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp @@ -33,12 +33,9 @@ NFTokenAcceptOffer::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfNFTokenAcceptOfferMask)) return ret; - if (ctx.tx.getFlags() & tfNFTokenAcceptOfferMask) - return temINVALID_FLAG; - auto const bo = ctx.tx[~sfNFTokenBuyOffer]; auto const so = ctx.tx[~sfNFTokenSellOffer]; diff --git a/src/xrpld/app/tx/detail/NFTokenBurn.cpp b/src/xrpld/app/tx/detail/NFTokenBurn.cpp index 947a663f92..fe65963bd8 100644 --- a/src/xrpld/app/tx/detail/NFTokenBurn.cpp +++ b/src/xrpld/app/tx/detail/NFTokenBurn.cpp @@ -32,12 +32,9 @@ NFTokenBurn::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp index f6072bc953..424d6cc534 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp @@ -34,12 +34,9 @@ NFTokenCancelOffer::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfNFTokenCancelOfferMask)) return ret; - if (ctx.tx.getFlags() & tfNFTokenCancelOfferMask) - return temINVALID_FLAG; - if (auto const& ids = ctx.tx[sfNFTokenOffers]; ids.empty() || (ids.size() > maxTokenOfferCancelCount)) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp index 8e1a026415..400a32951f 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp @@ -32,14 +32,11 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfNFTokenCreateOfferMask)) return ret; auto const txFlags = ctx.tx.getFlags(); - if (txFlags & tfNFTokenCreateOfferMask) - return temINVALID_FLAG; - auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]); // Use implementation shared with NFTokenMint diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index 42b551b3a4..4471205e4c 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -51,9 +51,6 @@ NFTokenMint::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNFTokenMintOffer) && hasOfferFields) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - // 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 @@ -67,7 +64,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the // tfTrustLine flag as a way to prevent the attack. But until the // amendment passes we still need to keep the old behavior available. - std::uint32_t const NFTokenMintMask = + std::uint32_t const nfTokenMintMask = ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine) // if featureDynamicNFT enabled then new flag allowing mutable URI // available @@ -76,8 +73,8 @@ NFTokenMint::preflight(PreflightContext const& ctx) : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable : tfNFTokenMintOldMask; - if (ctx.tx.getFlags() & NFTokenMintMask) - return temINVALID_FLAG; + if (auto const ret = preflight1(ctx, nfTokenMintMask)) + return ret; if (auto const f = ctx.tx[~sfTransferFee]) { diff --git a/src/xrpld/app/tx/detail/NFTokenModify.cpp b/src/xrpld/app/tx/detail/NFTokenModify.cpp index a3803c423b..939acc7ca4 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.cpp +++ b/src/xrpld/app/tx/detail/NFTokenModify.cpp @@ -32,12 +32,9 @@ NFTokenModify::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featureDynamicNFT)) return temDISABLED; - if (NotTEC const ret = preflight1(ctx); !isTesSuccess(ret)) + if (NotTEC const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto owner = ctx.tx[~sfOwner]; owner == ctx.tx[sfAccount]) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index a42902f6ac..6323e370f3 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -174,10 +174,9 @@ PayChanCreate::makeTxConsequences(PreflightContext const& ctx) NotTEC PayChanCreate::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = + preflight1(ctx, ctx.rules.enabled(fix1543) ? tfUniversalMask : 0)) return ret; if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) @@ -329,10 +328,9 @@ PayChanFund::makeTxConsequences(PreflightContext const& ctx) NotTEC PayChanFund::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = + preflight1(ctx, ctx.rules.enabled(fix1543) ? tfUniversalMask : 0)) return ret; if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) @@ -423,7 +421,9 @@ PayChanClaim::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight1( + ctx, ctx.rules.enabled(fix1543) ? tfPayChanClaimMask : 0)) return ret; auto const bal = ctx.tx[~sfBalance]; @@ -440,9 +440,6 @@ PayChanClaim::preflight(PreflightContext const& ctx) { auto const flags = ctx.tx.getFlags(); - if (ctx.rules.enabled(fix1543) && (flags & tfPayChanClaimMask)) - return temINVALID_FLAG; - if ((flags & tfClose) && (flags & tfRenew)) return temMALFORMED; } diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index fe10fec7fe..dddb5acec2 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -70,28 +70,21 @@ Payment::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto& tx = ctx.tx; auto& j = ctx.j; STAmount const dstAmount(tx.getFieldAmount(sfAmount)); bool const mptDirect = dstAmount.holds(); + if (auto const ret = + preflight1(ctx, mptDirect ? tfMPTPaymentMask : tfPaymentMask)) + return ret; + if (mptDirect && !ctx.rules.enabled(featureMPTokensV1)) return temDISABLED; std::uint32_t const txFlags = tx.getFlags(); - std::uint32_t paymentMask = mptDirect ? tfMPTPaymentMask : tfPaymentMask; - - if (txFlags & paymentMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - if (mptDirect && ctx.tx.isFieldPresent(sfPaths)) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp index 64c498b68b..401c9f914a 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -30,15 +30,9 @@ PermissionedDomainDelete::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featurePermissionedDomains)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "PermissionedDomainDelete: invalid flags."; - return temINVALID_FLAG; - } - auto const domain = ctx.tx.getFieldH256(sfDomainID); if (domain == beast::zero) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index 6e2df2a082..ea29227582 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -35,15 +35,9 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featureCredentials)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "PermissionedDomainSet: invalid flags."; - return temINVALID_FLAG; - } - if (auto err = credentials::checkArray( ctx.tx.getFieldArray(sfAcceptedCredentials), maxPermissionedDomainCredentialsArraySize, diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index d871cc3280..a709c635bc 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -59,7 +59,7 @@ SetAccount::makeTxConsequences(PreflightContext const& ctx) NotTEC SetAccount::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfAccountSetMask)) return ret; auto& tx = ctx.tx; @@ -67,12 +67,6 @@ SetAccount::preflight(PreflightContext const& ctx) std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfAccountSetMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - std::uint32_t const uSetFlag = tx.getFieldU32(sfSetFlag); std::uint32_t const uClearFlag = tx.getFieldU32(sfClearFlag); diff --git a/src/xrpld/app/tx/detail/SetOracle.cpp b/src/xrpld/app/tx/detail/SetOracle.cpp index 8559c3e7b9..22f7d5eb04 100644 --- a/src/xrpld/app/tx/detail/SetOracle.cpp +++ b/src/xrpld/app/tx/detail/SetOracle.cpp @@ -42,12 +42,9 @@ SetOracle::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featurePriceOracle)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries); if (dataSeries.empty()) return temARRAY_EMPTY; diff --git a/src/xrpld/app/tx/detail/SetRegularKey.cpp b/src/xrpld/app/tx/detail/SetRegularKey.cpp index 92d130a15a..4b95b923f4 100644 --- a/src/xrpld/app/tx/detail/SetRegularKey.cpp +++ b/src/xrpld/app/tx/detail/SetRegularKey.cpp @@ -51,18 +51,9 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC SetRegularKey::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - std::uint32_t const uTxFlags = ctx.tx.getFlags(); - - if (uTxFlags & tfUniversalMask) - { - JLOG(ctx.j.trace()) << "Malformed transaction: Invalid flags set."; - - return temINVALID_FLAG; - } - if (ctx.rules.enabled(fixMasterKeyAsRegularKey) && ctx.tx.isFieldPresent(sfRegularKey) && (ctx.tx.getAccountID(sfRegularKey) == ctx.tx.getAccountID(sfAccount))) diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index 4a1ee703a0..f37670eaba 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -80,16 +80,11 @@ SetSignerList::determineOperation( NotTEC SetSignerList::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight1( + ctx, ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0)) return ret; - if (ctx.rules.enabled(fixInvalidTxFlags) && - (ctx.tx.getFlags() & tfUniversalMask)) - { - JLOG(ctx.j.debug()) << "SetSignerList: invalid flags."; - return temINVALID_FLAG; - } - auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j); if (std::get<0>(result) != tesSUCCESS) diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index fd7c2fa0b1..fcc2f9ce79 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -69,7 +69,7 @@ namespace ripple { NotTEC SetTrust::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfTrustSetMask)) return ret; auto& tx = ctx.tx; @@ -77,12 +77,6 @@ SetTrust::preflight(PreflightContext const& ctx) std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfTrustSetMask) - { - JLOG(j.trace()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - if (!ctx.rules.enabled(featureDeepFreeze)) { // Even though the deep freeze flags are included in the diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 2e49794c2d..9a3d8619c7 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -39,7 +39,7 @@ namespace ripple { /** Performs early sanity checks on the txid */ NotTEC -preflight0(PreflightContext const& ctx) +preflight0(PreflightContext const& ctx, std::uint32_t flagMask) { if (!isPseudoTx(ctx.tx) || ctx.tx.isFieldPresent(sfNetworkID)) { @@ -74,12 +74,20 @@ preflight0(PreflightContext const& ctx) return temINVALID; } + if (ctx.tx.getFlags() & flagMask) + { + JLOG(ctx.j.debug()) + << ctx.tx.peekAtField(sfTransactionType).getFullText() + << ": invalid flags."; + return temINVALID_FLAG; + } + return tesSUCCESS; } /** Performs early sanity checks on the account and fee fields */ NotTEC -preflight1(PreflightContext const& ctx) +preflight1(PreflightContext const& ctx, std::uint32_t flagMask) { // This is inappropriate in preflight0, because only Change transactions // skip this function, and those do not allow an sfTicketSequence field. @@ -89,7 +97,7 @@ preflight1(PreflightContext const& ctx) return temMALFORMED; } - auto const ret = preflight0(ctx); + auto const ret = preflight0(ctx, flagMask); if (!isTesSuccess(ret)) return ret; @@ -186,7 +194,10 @@ PreflightContext::PreflightContext( //------------------------------------------------------------------------------ Transactor::Transactor(ApplyContext& ctx) - : ctx_(ctx), j_(ctx.journal), account_(ctx.tx.getAccountID(sfAccount)) + : ctx_(ctx) + , sink_(ctx.journal, to_short_string(ctx.tx.getTransactionID()) + " ") + , j_(sink_) + , account_(ctx.tx.getAccountID(sfAccount)) { } diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index e98269c38a..f67238cc16 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -24,6 +24,7 @@ #include #include +#include #include namespace ripple { @@ -87,6 +88,7 @@ class Transactor { protected: ApplyContext& ctx_; + beast::WrappedSink sink_; beast::Journal const j_; AccountID const account_; @@ -203,13 +205,16 @@ private: void trapTransaction(uint256) const; }; -/** Performs early sanity checks on the txid */ +/** Performs early sanity checks on the txid and flags */ NotTEC -preflight0(PreflightContext const& ctx); +preflight0(PreflightContext const& ctx, std::uint32_t flagMask); -/** Performs early sanity checks on the account and fee fields */ +/** Performs early sanity checks on the account and fee fields. + + (And passes flagMask to preflight0) +*/ NotTEC -preflight1(PreflightContext const& ctx); +preflight1(PreflightContext const& ctx, std::uint32_t flagMask); /** Checks whether the signature appears valid */ NotTEC diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index f6fc10b563..c96cb5ea18 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -36,12 +36,9 @@ VaultClawback::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureSingleAssetVault)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfUniversalMask)) return ter; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 64b8170efc..32776b8afc 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -44,12 +44,9 @@ VaultCreate::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featurePermissionedDomains)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfVaultCreateMask)) return ter; - if (ctx.tx.getFlags() & tfVaultCreateMask) - return temINVALID_FLAG; - if (auto const data = ctx.tx[~sfData]) { if (data->empty() || data->length() > maxDataPayloadLength) diff --git a/src/xrpld/app/tx/detail/VaultDelete.cpp b/src/xrpld/app/tx/detail/VaultDelete.cpp index 52d2f8700b..17ef9c2ca0 100644 --- a/src/xrpld/app/tx/detail/VaultDelete.cpp +++ b/src/xrpld/app/tx/detail/VaultDelete.cpp @@ -33,12 +33,9 @@ VaultDelete::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureSingleAssetVault)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfUniversalMask)) return ter; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 8edf6b09d0..e82807bfcb 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -38,12 +38,9 @@ VaultDeposit::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureSingleAssetVault)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfUniversalMask)) return ter; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 154681e908..3862ad66ed 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -40,15 +40,12 @@ VaultSet::preflight(PreflightContext const& ctx) !ctx.rules.enabled(featurePermissionedDomains)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfUniversalMask)) return ter; if (ctx.tx[sfVaultID] == beast::zero) return temMALFORMED; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto const data = ctx.tx[~sfData]) { if (data->empty() || data->length() > maxDataPayloadLength) diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index fb5d768d6c..081c337bab 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -36,12 +36,9 @@ VaultWithdraw::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureSingleAssetVault)) return temDISABLED; - if (auto const ter = preflight1(ctx)) + if (auto const ter = preflight1(ctx, tfUniversalMask)) return ter; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) return temMALFORMED; diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 5fa03557e5..781b491425 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -1214,12 +1214,9 @@ attestationPreflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (!publicKeyType(ctx.tx[sfPublicKey])) return temMALFORMED; @@ -1381,12 +1378,9 @@ XChainCreateBridge::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const account = ctx.tx[sfAccount]; auto const reward = ctx.tx[sfSignatureReward]; auto const minAccountCreate = ctx.tx[~sfMinAccountCreateAmount]; @@ -1562,12 +1556,9 @@ BridgeModify::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfBridgeModifyMask)) return ret; - if (ctx.tx.getFlags() & tfBridgeModifyMask) - return temINVALID_FLAG; - auto const account = ctx.tx[sfAccount]; auto const reward = ctx.tx[~sfSignatureReward]; auto const minAccountCreate = ctx.tx[~sfMinAccountCreateAmount]; @@ -1672,12 +1663,9 @@ XChainClaim::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge]; auto const amount = ctx.tx[sfAmount]; @@ -1910,12 +1898,9 @@ XChainCommit::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; auto const bridgeSpec = ctx.tx[sfXChainBridge]; @@ -2024,12 +2009,9 @@ XChainCreateClaimID::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const reward = ctx.tx[sfSignatureReward]; if (!isXRP(reward) || reward.signum() < 0 || !isLegalNet(reward)) @@ -2179,12 +2161,9 @@ XChainCreateAccountCommit::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureXChainBridge)) return temDISABLED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + if (auto const ret = preflight1(ctx, tfUniversalMask)) return ret; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; if (amount.signum() <= 0 || !amount.native())