diff --git a/include/xrpl/protocol/Permissions.h b/include/xrpl/protocol/Permissions.h index 2eca441124..d3f5253cd0 100644 --- a/include/xrpl/protocol/Permissions.h +++ b/include/xrpl/protocol/Permissions.h @@ -86,6 +86,9 @@ public: std::optional getGranularTxType(GranularPermissionType const& gpType) const; + std::optional> const + getTxFeature(TxType txType) const; + bool isDelegatable(std::uint32_t const& permissionValue, Rules const& rules) const; diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 9ace6b80f8..0a3a3b999e 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -673,7 +673,8 @@ isTerRetry(TER x) noexcept inline bool isTesSuccess(TER x) noexcept { - return (x == tesSUCCESS); + // Makes use of TERSubset::operator bool() + return !(x); } inline bool diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index 6a4b0678e0..c9e32c5056 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -147,6 +147,19 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const return std::nullopt; } +std::optional> const +Permission::getTxFeature(TxType txType) const +{ + auto const txFeaturesIt = txFeatureMap_.find(txType); + XRPL_ASSERT( + txFeaturesIt != txFeatureMap_.end(), + "ripple::Permissions::getTxFeature : tx exists in txFeatureMap_"); + + if (txFeaturesIt->second == uint256{}) + return std::nullopt; + return txFeaturesIt->second; +} + bool Permission::isDelegatable( std::uint32_t const& permissionValue, @@ -166,16 +179,12 @@ Permission::isDelegatable( if (it == delegatableTx_.end()) return false; - auto const txFeaturesIt = txFeatureMap_.find(txType); - XRPL_ASSERT( - txFeaturesIt != txFeatureMap_.end(), - "ripple::Permissions::isDelegatable : tx exists in txFeatureMap_"); + auto const feature = getTxFeature(txType); // fixDelegateV1_1: Delegation is only allowed if the required amendment // for the transaction is enabled. For transactions that do not require // an amendment, delegation is always allowed. - if (txFeaturesIt->second != uint256{} && - !rules.enabled(txFeaturesIt->second)) + if (feature && !rules.enabled(*feature)) return false; } diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index cfe1ffab16..1fe37bb7c1 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -3572,7 +3572,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf == temDISABLED); env.app().config().features.insert(featureAMM); } @@ -3587,7 +3587,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf != tesSUCCESS); } @@ -3602,7 +3602,7 @@ private: env.current()->rules(), tapNONE, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = Transactor::invokePreflight(pfctx); BEAST_EXPECT(pf == temBAD_AMM_TOKENS); } } diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 3615a715cd..52dc331a2b 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -19,6 +19,8 @@ #include +#include + #include #include #include @@ -578,6 +580,32 @@ public: env.close(); } + void + testBadSigningKey() + { + using namespace test::jtx; + testcase("Bad signing key"); + Env env(*this); + Account const alice("alice"); + + env.fund(XRP(10000), alice); + env.close(); + + auto jtx = env.jt(noop("alice"), ter(temBAD_SIGNATURE)); + if (!BEAST_EXPECT(jtx.stx)) + return; + auto stx = std::make_shared(*jtx.stx); + stx->at(sfSigningPubKey) = makeSlice(std::string("badkey")); + + env.app().openLedger().modify([&](OpenView& view, beast::Journal j) { + auto const result = + ripple::apply(env.app(), view, *stx, tapNONE, j); + BEAST_EXPECT(result.ter == temBAD_SIGNATURE); + BEAST_EXPECT(!result.applied); + return result.applied; + }); + } + void run() override { @@ -594,6 +622,7 @@ public: testRequireAuthWithDir(); testTransferRate(); testTicket(); + testBadSigningKey(); } }; diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index 806c075c4f..769668b07b 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -30,21 +30,15 @@ namespace ripple { +bool +AMMBid::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMBid::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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())) { @@ -95,7 +89,7 @@ AMMBid::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMBid.h b/src/xrpld/app/tx/detail/AMMBid.h index 4bb3a2adfd..4a527b6a93 100644 --- a/src/xrpld/app/tx/detail/AMMBid.h +++ b/src/xrpld/app/tx/detail/AMMBid.h @@ -71,6 +71,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 634b948a64..9a79c94a58 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -33,19 +33,15 @@ namespace ripple { +std::uint32_t +AMMClawback::getFlagsMask(PreflightContext const& ctx) +{ + return tfAMMClawbackMask; +} + NotTEC AMMClawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureAMMClawback)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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()) @@ -88,7 +86,7 @@ AMMClawback::preflight(PreflightContext const& ctx) if (clawAmount && *clawAmount <= beast::zero) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h index fdcfc53e2c..1984937971 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.h +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 03c972f1cd..63e20b42fb 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -31,21 +31,15 @@ namespace ripple { +bool +AMMCreate::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMCreate::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -74,14 +68,14 @@ AMMCreate::preflight(PreflightContext const& ctx) return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount AMMCreate::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for AMMCreate is one owner reserve. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER diff --git a/src/xrpld/app/tx/detail/AMMCreate.h b/src/xrpld/app/tx/detail/AMMCreate.h index 189d66a55a..98231e5554 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.h +++ b/src/xrpld/app/tx/detail/AMMCreate.h @@ -63,6 +63,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMDelete.cpp b/src/xrpld/app/tx/detail/AMMDelete.cpp index 004e0b2229..663a4c4b0a 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.cpp +++ b/src/xrpld/app/tx/detail/AMMDelete.cpp @@ -27,22 +27,16 @@ namespace ripple { +bool +AMMDelete::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMDelete::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "AMM Delete: invalid flags."; - return temINVALID_FLAG; - } - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMDelete.h b/src/xrpld/app/tx/detail/AMMDelete.h index 19885b1dad..36dace2e18 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.h +++ b/src/xrpld/app/tx/detail/AMMDelete.h @@ -39,6 +39,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 614d788c71..8a3e50ed63 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -29,21 +29,23 @@ namespace ripple { +bool +AMMDeposit::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + +std::uint32_t +AMMDeposit::getFlagsMask(PreflightContext const& ctx) + +{ + return tfDepositMask; +} + NotTEC AMMDeposit::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -159,7 +161,7 @@ AMMDeposit::preflight(PreflightContext const& ctx) return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMDeposit.h b/src/xrpld/app/tx/detail/AMMDeposit.h index 0acb1dd9ab..c1a37be452 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.h +++ b/src/xrpld/app/tx/detail/AMMDeposit.h @@ -68,6 +68,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index 6fbff86056..0ffbb38b37 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -27,15 +27,15 @@ namespace ripple { +bool +AMMVote::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + NotTEC AMMVote::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - if (auto const res = invalidAMMAssetPair( ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { @@ -43,19 +43,13 @@ 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."; return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/AMMVote.h b/src/xrpld/app/tx/detail/AMMVote.h index 2bee01aff5..dc99480111 100644 --- a/src/xrpld/app/tx/detail/AMMVote.h +++ b/src/xrpld/app/tx/detail/AMMVote.h @@ -56,6 +56,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 9bc36efc81..f5af9dfb9c 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -28,21 +28,22 @@ namespace ripple { +bool +AMMWithdraw::checkExtraFeatures(PreflightContext const& ctx) +{ + return ammEnabled(ctx.rules); +} + +std::uint32_t +AMMWithdraw::getFlagsMask(PreflightContext const& ctx) +{ + return tfWithdrawMask; +} + NotTEC AMMWithdraw::preflight(PreflightContext const& ctx) { - if (!ammEnabled(ctx.rules)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -150,7 +151,7 @@ AMMWithdraw::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } static std::optional diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index e9a597bdb7..31a7904626 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -76,6 +76,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Batch.cpp b/src/xrpld/app/tx/detail/Batch.cpp index 86d6e8a8f4..cba89348d0 100644 --- a/src/xrpld/app/tx/detail/Batch.cpp +++ b/src/xrpld/app/tx/detail/Batch.cpp @@ -164,6 +164,12 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) return signerFees + txnFees + batchBase; } +std::uint32_t +Batch::getFlagsMask(PreflightContext const& ctx) +{ + return tfBatchMask; +} + /** * @brief Performs preflight validation checks for a Batch transaction. * @@ -200,23 +206,9 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC Batch::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureBatch)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const parentBatchId = ctx.tx.getTransactionID(); - auto const outerAccount = ctx.tx.getAccountID(sfAccount); auto const flags = ctx.tx.getFlags(); - if (flags & tfBatchMask) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:" - << "invalid flags."; - return temINVALID_FLAG; - } - if (std::popcount( flags & (tfAllOrNothing | tfOnlyOne | tfUntilFailure | tfIndependent)) != 1) @@ -242,7 +234,6 @@ Batch::preflight(PreflightContext const& ctx) } // Validation Inner Batch Txns - std::unordered_set requiredSigners; std::unordered_set uniqueHashes; std::unordered_map> accountSeqTicket; @@ -372,6 +363,23 @@ Batch::preflight(PreflightContext const& ctx) } } } + } + + return tesSUCCESS; +} + +NotTEC +Batch::preflightSigValidated(PreflightContext const& ctx) +{ + auto const parentBatchId = ctx.tx.getTransactionID(); + auto const outerAccount = ctx.tx.getAccountID(sfAccount); + auto const& rawTxns = ctx.tx.getFieldArray(sfRawTransactions); + + // Build the signers list + std::unordered_set requiredSigners; + for (STObject const& rb : rawTxns) + { + auto const innerAccount = rb.getAccountID(sfAccount); // If the inner account is the same as the outer account, do not add the // inner account to the required signers set. @@ -379,11 +387,6 @@ Batch::preflight(PreflightContext const& ctx) requiredSigners.insert(innerAccount); } - // LCOV_EXCL_START - if (auto const ret = preflight2(ctx); !isTesSuccess(ret)) - return ret; - // LCOV_EXCL_STOP - // Validation Batch Signers std::unordered_set batchSigners; if (ctx.tx.isFieldPresent(sfBatchSigners)) diff --git a/src/xrpld/app/tx/detail/Batch.h b/src/xrpld/app/tx/detail/Batch.h index 211bce0589..07863a5f33 100644 --- a/src/xrpld/app/tx/detail/Batch.h +++ b/src/xrpld/app/tx/detail/Batch.h @@ -40,9 +40,15 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + static NotTEC checkSign(PreclaimContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CancelCheck.cpp b/src/xrpld/app/tx/detail/CancelCheck.cpp index 39d0d23096..f1a9b42a89 100644 --- a/src/xrpld/app/tx/detail/CancelCheck.cpp +++ b/src/xrpld/app/tx/detail/CancelCheck.cpp @@ -32,21 +32,7 @@ namespace ripple { NotTEC CancelCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - 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); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CancelOffer.cpp b/src/xrpld/app/tx/detail/CancelOffer.cpp index e0a5c7baa7..e7ec28ce17 100644 --- a/src/xrpld/app/tx/detail/CancelOffer.cpp +++ b/src/xrpld/app/tx/detail/CancelOffer.cpp @@ -28,25 +28,13 @@ namespace ripple { NotTEC CancelOffer::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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"; return temBAD_SEQUENCE; } - return preflight2(ctx); + return tesSUCCESS; } //------------------------------------------------------------------------------ diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index 0f1d08689c..f8ab6189a3 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -35,20 +35,6 @@ namespace ripple { NotTEC CashCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - 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]; @@ -76,7 +62,7 @@ CashCheck::preflight(PreflightContext const& ctx) return temBAD_CURRENCY; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index de30ed5f62..d6a31024f3 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -33,11 +33,12 @@ namespace ripple { +template <> NotTEC -Change::preflight(PreflightContext const& ctx) +Transactor::invokePreflight(PreflightContext const& ctx) { - auto const ret = preflight0(ctx); - if (!isTesSuccess(ret)) + // 0 means "Allow any flags" + if (auto const ret = preflight0(ctx, 0)) return ret; auto account = ctx.tx.getAccountID(sfAccount); diff --git a/src/xrpld/app/tx/detail/Change.h b/src/xrpld/app/tx/detail/Change.h index d710827dd6..7b8fbf3421 100644 --- a/src/xrpld/app/tx/detail/Change.h +++ b/src/xrpld/app/tx/detail/Change.h @@ -33,9 +33,6 @@ public: { } - static NotTEC - preflight(PreflightContext const& ctx); - TER doApply() override; void diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp index 08cf4baef0..b346e4a1c1 100644 --- a/src/xrpld/app/tx/detail/Clawback.cpp +++ b/src/xrpld/app/tx/detail/Clawback.cpp @@ -75,25 +75,22 @@ preflightHelper(PreflightContext const& ctx) return tesSUCCESS; } +std::uint32_t +Clawback::getFlagsMask(PreflightContext const& ctx) +{ + return tfClawbackMask; +} + NotTEC Clawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureClawback)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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()); !isTesSuccess(ret)) return ret; - return preflight2(ctx); + return tesSUCCESS; } template diff --git a/src/xrpld/app/tx/detail/Clawback.h b/src/xrpld/app/tx/detail/Clawback.h index d908a2e4ef..b02233c2ed 100644 --- a/src/xrpld/app/tx/detail/Clawback.h +++ b/src/xrpld/app/tx/detail/Clawback.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp index 4dbfd1f81d..57f3a92255 100644 --- a/src/xrpld/app/tx/detail/CreateCheck.cpp +++ b/src/xrpld/app/tx/detail/CreateCheck.cpp @@ -31,19 +31,6 @@ namespace ripple { NotTEC CreateCheck::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureChecks)) - return temDISABLED; - - NotTEC const ret{preflight1(ctx)}; - if (!isTesSuccess(ret)) - 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. @@ -76,7 +63,7 @@ CreateCheck::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 6185e52183..86750eb51d 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -43,30 +43,36 @@ CreateOffer::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, calculateMaxXRPSpend(ctx.tx)}; } -NotTEC -CreateOffer::preflight(PreflightContext const& ctx) +bool +CreateOffer::checkExtraFeatures(PreflightContext const& ctx) { if (ctx.tx.isFieldPresent(sfDomainID) && !ctx.rules.enabled(featurePermissionedDEX)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +CreateOffer::getFlagsMask(PreflightContext const& ctx) +{ + // The tfOfferCreateMask is built assuming that PermissionedDEX is + // enabled + if (ctx.rules.enabled(featurePermissionedDEX)) + return tfOfferCreateMask; + // If PermissionedDEX is not enabled, add tfHybrid to the mask, + // indicating it is not allowed. + return tfOfferCreateMask | tfHybrid; +} + +NotTEC +CreateOffer::preflight(PreflightContext const& ctx) +{ auto& tx = ctx.tx; auto& j = ctx.j; std::uint32_t const uTxFlags = tx.getFlags(); - if (uTxFlags & tfOfferCreateMask) - { - JLOG(j.debug()) << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - - if (!ctx.rules.enabled(featurePermissionedDEX) && tx.isFlag(tfHybrid)) - return temINVALID_FLAG; - if (tx.isFlag(tfHybrid) && !tx.isFieldPresent(sfDomainID)) return temINVALID_FLAG; @@ -136,7 +142,7 @@ CreateOffer::preflight(PreflightContext const& ctx) return temBAD_ISSUER; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/CreateOffer.h b/src/xrpld/app/tx/detail/CreateOffer.h index 6e3d6145b1..c38e244b34 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.h +++ b/src/xrpld/app/tx/detail/CreateOffer.h @@ -43,6 +43,12 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + /** Enforce constraints beyond those of the Transactor base class. */ static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/CreateTicket.cpp b/src/xrpld/app/tx/detail/CreateTicket.cpp index 594335f489..d48da2d780 100644 --- a/src/xrpld/app/tx/detail/CreateTicket.cpp +++ b/src/xrpld/app/tx/detail/CreateTicket.cpp @@ -36,20 +36,11 @@ CreateTicket::makeTxConsequences(PreflightContext const& ctx) NotTEC CreateTicket::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureTicketBatch)) - return temDISABLED; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - 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); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index b30ae200b7..4b77163c5d 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/src/xrpld/app/tx/detail/Credentials.cpp @@ -48,28 +48,19 @@ using namespace credentials; // ------- CREATE -------------------------- +std::uint32_t +CredentialCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialCreate::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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"; @@ -91,7 +82,7 @@ CredentialCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -202,25 +193,17 @@ CredentialCreate::doApply() } // ------- DELETE -------------------------- + +std::uint32_t +CredentialDelete::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -248,7 +231,7 @@ CredentialDelete::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -289,25 +272,16 @@ CredentialDelete::doApply() // ------- APPLY -------------------------- +std::uint32_t +CredentialAccept::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC CredentialAccept::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureCredentials)) - { - JLOG(ctx.j.trace()) << "featureCredentials is disabled."; - return temDISABLED; - } - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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."; @@ -322,7 +296,7 @@ CredentialAccept::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Credentials.h b/src/xrpld/app/tx/detail/Credentials.h index 5b4acb3998..a5885a2226 100644 --- a/src/xrpld/app/tx/detail/Credentials.h +++ b/src/xrpld/app/tx/detail/Credentials.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -54,6 +57,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -75,6 +81,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/DID.cpp b/src/xrpld/app/tx/detail/DID.cpp index 8c4a220844..b38b207d36 100644 --- a/src/xrpld/app/tx/detail/DID.cpp +++ b/src/xrpld/app/tx/detail/DID.cpp @@ -45,15 +45,6 @@ namespace ripple { NotTEC 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)) - return ret; - if (!ctx.tx.isFieldPresent(sfURI) && !ctx.tx.isFieldPresent(sfDIDDocument) && !ctx.tx.isFieldPresent(sfData)) return temEMPTY_DID; @@ -74,7 +65,7 @@ DIDSet::preflight(PreflightContext const& ctx) isTooLong(sfData, maxDIDAttestationLength)) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -174,16 +165,7 @@ DIDSet::doApply() NotTEC 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)) - return ret; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DelegateSet.cpp b/src/xrpld/app/tx/detail/DelegateSet.cpp index 53052fd75b..e769f75d8a 100644 --- a/src/xrpld/app/tx/detail/DelegateSet.cpp +++ b/src/xrpld/app/tx/detail/DelegateSet.cpp @@ -30,12 +30,6 @@ namespace ripple { NotTEC DelegateSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionDelegation)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const& permissions = ctx.tx.getFieldArray(sfPermissions); if (permissions.size() > permissionMaxSize) return temARRAY_TOO_LARGE; @@ -57,7 +51,7 @@ DelegateSet::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 02f84adcc3..565d938c83 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -38,22 +38,22 @@ namespace ripple { -NotTEC -DeleteAccount::preflight(PreflightContext const& ctx) +bool +DeleteAccount::checkExtraFeatures(PreflightContext const& ctx) { if (!ctx.rules.enabled(featureDeletableAccounts)) - return temDISABLED; + return false; if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; + return false; - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +NotTEC +DeleteAccount::preflight(PreflightContext const& ctx) +{ if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) // An account cannot be deleted and give itself the resulting XRP. return temDST_IS_SRC; @@ -62,14 +62,14 @@ DeleteAccount::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } XRPAmount DeleteAccount::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for AccountDelete is one owner reserve. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } namespace { diff --git a/src/xrpld/app/tx/detail/DeleteAccount.h b/src/xrpld/app/tx/detail/DeleteAccount.h index c9d3305562..ee9db97d50 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.h +++ b/src/xrpld/app/tx/detail/DeleteAccount.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/DeleteOracle.cpp b/src/xrpld/app/tx/detail/DeleteOracle.cpp index ac195d100c..7dba477aaa 100644 --- a/src/xrpld/app/tx/detail/DeleteOracle.cpp +++ b/src/xrpld/app/tx/detail/DeleteOracle.cpp @@ -29,19 +29,7 @@ namespace ripple { NotTEC DeleteOracle::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePriceOracle)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - { - JLOG(ctx.j.debug()) << "Oracle Delete: invalid flags."; - return temINVALID_FLAG; - } - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DepositPreauth.cpp b/src/xrpld/app/tx/detail/DepositPreauth.cpp index 0e8c5c05d2..236b59a173 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.cpp +++ b/src/xrpld/app/tx/detail/DepositPreauth.cpp @@ -30,32 +30,29 @@ namespace ripple { +bool +DepositPreauth::checkExtraFeatures(PreflightContext const& ctx) +{ + bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials); + bool const unauthArrPresent = + ctx.tx.isFieldPresent(sfUnauthorizeCredentials); + bool const authCredPresent = authArrPresent || unauthArrPresent; + + if (authCredPresent && !ctx.rules.enabled(featureCredentials)) + return false; + + return true; +} + NotTEC DepositPreauth::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDepositPreauth)) - return temDISABLED; - bool const authArrPresent = ctx.tx.isFieldPresent(sfAuthorizeCredentials); bool const unauthArrPresent = ctx.tx.isFieldPresent(sfUnauthorizeCredentials); int const authCredPresent = static_cast(authArrPresent) + static_cast(unauthArrPresent); - if (authCredPresent && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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()) + @@ -102,7 +99,7 @@ DepositPreauth::preflight(PreflightContext const& ctx) return err; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/DepositPreauth.h b/src/xrpld/app/tx/detail/DepositPreauth.h index 76a7c08073..ead17742cd 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.h +++ b/src/xrpld/app/tx/detail/DepositPreauth.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index f1d1db79a0..969fd4dd4c 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -118,15 +118,16 @@ escrowCreatePreflightHelper(PreflightContext const& ctx) return tesSUCCESS; } +std::uint32_t +EscrowCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + 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)) - return ret; - STAmount const amount{ctx.tx[sfAmount]}; if (!isXRP(amount)) { @@ -189,7 +190,7 @@ EscrowCreate::preflight(PreflightContext const& ctx) return temDISABLED; } - return preflight2(ctx); + return tesSUCCESS; } template @@ -629,19 +630,23 @@ checkCondition(Slice f, Slice c) return validate(*fulfillment, *condition); } +bool +EscrowFinish::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + ctx.rules.enabled(featureCredentials); +} + +std::uint32_t +EscrowFinish::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + 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)) - return ret; - auto const cb = ctx.tx[~sfCondition]; auto const fb = ctx.tx[~sfFulfillment]; @@ -650,13 +655,14 @@ EscrowFinish::preflight(PreflightContext const& ctx) if (static_cast(cb) != static_cast(fb)) return temMALFORMED; - // Verify the transaction signature. If it doesn't work - // then don't do any more work. - { - auto const ret = preflight2(ctx); - if (!isTesSuccess(ret)) - return ret; - } + return tesSUCCESS; +} + +NotTEC +EscrowFinish::preflightSigValidated(PreflightContext const& ctx) +{ + auto const cb = ctx.tx[~sfCondition]; + auto const fb = ctx.tx[~sfFulfillment]; if (cb && fb) { @@ -1207,16 +1213,17 @@ EscrowFinish::doApply() //------------------------------------------------------------------------------ +std::uint32_t +EscrowCancel::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + 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)) - return ret; - - return preflight2(ctx); + return tesSUCCESS; } template diff --git a/src/xrpld/app/tx/detail/Escrow.h b/src/xrpld/app/tx/detail/Escrow.h index 2225c94f16..8956be2939 100644 --- a/src/xrpld/app/tx/detail/Escrow.h +++ b/src/xrpld/app/tx/detail/Escrow.h @@ -36,6 +36,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -57,9 +60,18 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); @@ -81,6 +93,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/LedgerStateFix.cpp b/src/xrpld/app/tx/detail/LedgerStateFix.cpp index b861f1d0ef..6059e15313 100644 --- a/src/xrpld/app/tx/detail/LedgerStateFix.cpp +++ b/src/xrpld/app/tx/detail/LedgerStateFix.cpp @@ -30,15 +30,6 @@ namespace ripple { NotTEC 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)) - return ret; - switch (ctx.tx[sfLedgerFixType]) { case FixType::nfTokenPageLink: @@ -50,7 +41,7 @@ LedgerStateFix::preflight(PreflightContext const& ctx) return tefINVALID_LEDGER_FIX_TYPE; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount @@ -58,7 +49,7 @@ LedgerStateFix::calculateBaseFee(ReadView const& view, STTx const& tx) { // The fee required for LedgerStateFix is one owner reserve, just like // the fee for AccountDelete. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp index 1c6d153ec5..edeb12e5c0 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp @@ -26,22 +26,19 @@ namespace ripple { +std::uint32_t +MPTokenAuthorize::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenAuthorizeMask; +} + NotTEC MPTokenAuthorize::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfMPTokenAuthorizeMask) - return temINVALID_FLAG; - if (ctx.tx[sfAccount] == ctx.tx[~sfHolder]) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.h b/src/xrpld/app/tx/detail/MPTokenAuthorize.h index 85e8edcf9f..43a962e24e 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.h +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.h @@ -42,6 +42,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp index 478ef17bb0..eec4187573 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp @@ -25,31 +25,37 @@ namespace ripple { -NotTEC -MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) +bool +MPTokenIssuanceCreate::checkExtraFeatures(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - if (ctx.tx.isFieldPresent(sfDomainID) && !(ctx.rules.enabled(featurePermissionedDomains) && ctx.rules.enabled(featureSingleAssetVault))) - return temDISABLED; + return false; if (ctx.tx.isFieldPresent(sfMutableFlags) && !ctx.rules.enabled(featureDynamicMPT)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +MPTokenIssuanceCreate::getFlagsMask(PreflightContext const& ctx) +{ + // This mask is only compared against sfFlags + return tfMPTokenIssuanceCreateMask; +} + +NotTEC +MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) +{ + // If the mutable flags field is included, at least one flag must be + // specified. if (auto const mutableFlags = ctx.tx[~sfMutableFlags]; mutableFlags && (!*mutableFlags || *mutableFlags & tmfMPTokenIssuanceCreateMutableMask)) return temINVALID_FLAG; - if (ctx.tx.getFlags() & tfMPTokenIssuanceCreateMask) - return temINVALID_FLAG; - if (auto const fee = ctx.tx[~sfTransferFee]) { if (fee > maxTransferFee) @@ -87,7 +93,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) if (maxAmt > maxMPTokenAmount) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } Expected diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h index 0527b9602f..842ed88641 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h @@ -50,6 +50,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp index 2c330ba8f7..4c502f1106 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp @@ -25,20 +25,16 @@ namespace ripple { +std::uint32_t +MPTokenIssuanceDestroy::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenIssuanceDestroyMask; +} + NotTEC MPTokenIssuanceDestroy::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - // check flags - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfMPTokenIssuanceDestroyMask) - return temINVALID_FLAG; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h index 69abb99feb..2cebdb7352 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index 37c563460a..c406a8ec5f 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -26,6 +26,20 @@ namespace ripple { +bool +MPTokenIssuanceSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfDomainID) || + (ctx.rules.enabled(featurePermissionedDomains) && + ctx.rules.enabled(featureSingleAssetVault)); +} + +std::uint32_t +MPTokenIssuanceSet::getFlagsMask(PreflightContext const& ctx) +{ + return tfMPTokenIssuanceSetMask; +} + // Maps set/clear mutable flags in an MPTokenIssuanceSet transaction to the // corresponding ledger mutable flags that control whether the change is // allowed. @@ -49,14 +63,6 @@ static constexpr std::array mptMutabilityFlags = { NotTEC MPTokenIssuanceSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !(ctx.rules.enabled(featurePermissionedDomains) && - ctx.rules.enabled(featureSingleAssetVault))) - return temDISABLED; - auto const mutableFlags = ctx.tx[~sfMutableFlags]; auto const metadata = ctx.tx[~sfMPTokenMetadata]; auto const transferFee = ctx.tx[~sfTransferFee]; @@ -68,16 +74,10 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) if (ctx.tx.isFieldPresent(sfDomainID) && ctx.tx.isFieldPresent(sfHolder)) return temMALFORMED; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -133,7 +133,7 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h index 5b3db0e75b..f63812097e 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h @@ -33,6 +33,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp index 0cf6a86a37..3b4a27ffd7 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp @@ -27,18 +27,15 @@ namespace ripple { +std::uint32_t +NFTokenAcceptOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenAcceptOfferMask; +} + NotTEC NFTokenAcceptOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfNFTokenAcceptOfferMask) - return temINVALID_FLAG; - auto const bo = ctx.tx[~sfNFTokenBuyOffer]; auto const so = ctx.tx[~sfNFTokenSellOffer]; @@ -57,7 +54,7 @@ NFTokenAcceptOffer::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h index dff3febbb2..995581d1ff 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.h @@ -51,6 +51,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenBurn.cpp b/src/xrpld/app/tx/detail/NFTokenBurn.cpp index 947a663f92..cb1b564402 100644 --- a/src/xrpld/app/tx/detail/NFTokenBurn.cpp +++ b/src/xrpld/app/tx/detail/NFTokenBurn.cpp @@ -29,16 +29,7 @@ namespace ripple { NotTEC NFTokenBurn::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp index 3d0bf04a1b..86e804b1a5 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp @@ -28,18 +28,15 @@ namespace ripple { +std::uint32_t +NFTokenCancelOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenCancelOfferMask; +} + NotTEC NFTokenCancelOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfNFTokenCancelOfferMask) - return temINVALID_FLAG; - if (auto const& ids = ctx.tx[sfNFTokenOffers]; ids.empty() || (ids.size() > maxTokenOfferCancelCount)) return temMALFORMED; @@ -51,7 +48,7 @@ NFTokenCancelOffer::preflight(PreflightContext const& ctx) if (std::adjacent_find(ids.begin(), ids.end()) != ids.end()) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.h b/src/xrpld/app/tx/detail/NFTokenCancelOffer.h index d460675711..b35be0e757 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp index f9cc8c1fc8..2a02fed797 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.cpp @@ -26,20 +26,17 @@ namespace ripple { +std::uint32_t +NFTokenCreateOffer::getFlagsMask(PreflightContext const& ctx) +{ + return tfNFTokenCreateOfferMask; +} + NotTEC NFTokenCreateOffer::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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 @@ -55,7 +52,7 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx) !isTesSuccess(notTec)) return notTec; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenCreateOffer.h b/src/xrpld/app/tx/detail/NFTokenCreateOffer.h index 075a5a712f..0a1c631298 100644 --- a/src/xrpld/app/tx/detail/NFTokenCreateOffer.h +++ b/src/xrpld/app/tx/detail/NFTokenCreateOffer.h @@ -33,6 +33,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index 4c07a6e499..8149d3b59d 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -38,22 +38,23 @@ extractNFTokenFlagsFromTxFlags(std::uint32_t txFlags) return static_cast(txFlags & 0x0000FFFF); } -NotTEC -NFTokenMint::preflight(PreflightContext const& ctx) +static bool +hasOfferFields(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1)) - return temDISABLED; - - bool const hasOfferFields = ctx.tx.isFieldPresent(sfAmount) || + return ctx.tx.isFieldPresent(sfAmount) || ctx.tx.isFieldPresent(sfDestination) || ctx.tx.isFieldPresent(sfExpiration); +} - if (!ctx.rules.enabled(featureNFTokenMintOffer) && hasOfferFields) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; +bool +NFTokenMint::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureNFTokenMintOffer) || !hasOfferFields(ctx); +} +std::uint32_t +NFTokenMint::getFlagsMask(PreflightContext const& ctx) +{ // 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 +68,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,9 +77,12 @@ NFTokenMint::preflight(PreflightContext const& ctx) : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable : tfNFTokenMintOldMask; - if (ctx.tx.getFlags() & NFTokenMintMask) - return temINVALID_FLAG; + return nfTokenMintMask; +} +NotTEC +NFTokenMint::preflight(PreflightContext const& ctx) +{ if (auto const f = ctx.tx[~sfTransferFee]) { if (f > maxTransferFee) @@ -100,7 +104,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (hasOfferFields) + if (hasOfferFields(ctx)) { // The Amount field must be present if either the Destination or // Expiration fields are present. @@ -123,7 +127,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } uint256 diff --git a/src/xrpld/app/tx/detail/NFTokenMint.h b/src/xrpld/app/tx/detail/NFTokenMint.h index f606120c54..1606514559 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.h +++ b/src/xrpld/app/tx/detail/NFTokenMint.h @@ -36,6 +36,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenModify.cpp b/src/xrpld/app/tx/detail/NFTokenModify.cpp index a3803c423b..6ae095411b 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.cpp +++ b/src/xrpld/app/tx/detail/NFTokenModify.cpp @@ -25,19 +25,15 @@ namespace ripple { +bool +NFTokenModify::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureNonFungibleTokensV1_1); +} + NotTEC NFTokenModify::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureNonFungibleTokensV1_1) || - !ctx.rules.enabled(featureDynamicNFT)) - return temDISABLED; - - if (NotTEC const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto owner = ctx.tx[~sfOwner]; owner == ctx.tx[sfAccount]) return temMALFORMED; @@ -47,7 +43,7 @@ NFTokenModify::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/NFTokenModify.h b/src/xrpld/app/tx/detail/NFTokenModify.h index 0d1e72ade1..04784381fb 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.h +++ b/src/xrpld/app/tx/detail/NFTokenModify.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index bdfe0d5c95..32c0abeb93 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -171,15 +171,16 @@ PayChanCreate::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()}; } +std::uint32_t +PayChanCreate::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + 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)) - return ret; - if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) return temBAD_AMOUNT; @@ -189,7 +190,7 @@ PayChanCreate::preflight(PreflightContext const& ctx) if (!publicKeyType(ctx.tx[sfPublicKey])) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -330,19 +331,20 @@ PayChanFund::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, ctx.tx[sfAmount].xrp()}; } +std::uint32_t +PayChanFund::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfUniversalMask : 0; +} + 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)) - return ret; - if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::zero)) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -420,16 +422,23 @@ PayChanFund::doApply() //------------------------------------------------------------------------------ +bool +PayChanClaim::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + ctx.rules.enabled(featureCredentials); +} + +std::uint32_t +PayChanClaim::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fix1543) ? tfPayChanClaimMask : 0; +} + NotTEC PayChanClaim::preflight(PreflightContext const& ctx) { - if (ctx.tx.isFieldPresent(sfCredentialIDs) && - !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto const bal = ctx.tx[~sfBalance]; if (bal && (!isXRP(*bal) || *bal <= beast::zero)) return temBAD_AMOUNT; @@ -444,9 +453,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; } @@ -481,7 +487,7 @@ PayChanClaim::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PayChan.h b/src/xrpld/app/tx/detail/PayChan.h index 2e09c473dc..b25a4529be 100644 --- a/src/xrpld/app/tx/detail/PayChan.h +++ b/src/xrpld/app/tx/detail/PayChan.h @@ -36,6 +36,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -62,6 +65,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); @@ -82,6 +88,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index e622d54498..8bc0e891d0 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -65,20 +65,33 @@ getMaxSourceAmount( dstAmount < beast::zero); } -NotTEC -Payment::preflight(PreflightContext const& ctx) +bool +Payment::checkExtraFeatures(PreflightContext const& ctx) { if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - + return false; if (ctx.tx.isFieldPresent(sfDomainID) && !ctx.rules.enabled(featurePermissionedDEX)) - return temDISABLED; + return false; - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return true; +} +std::uint32_t +Payment::getFlagsMask(PreflightContext const& ctx) +{ + auto& tx = ctx.tx; + + STAmount const dstAmount(tx.getFieldAmount(sfAmount)); + bool const mptDirect = dstAmount.holds(); + + return mptDirect ? tfMPTPaymentMask : tfPaymentMask; +} + +NotTEC +Payment::preflight(PreflightContext const& ctx) +{ auto& tx = ctx.tx; auto& j = ctx.j; @@ -90,14 +103,6 @@ Payment::preflight(PreflightContext const& ctx) 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; @@ -242,7 +247,7 @@ Payment::preflight(PreflightContext const& ctx) !isTesSuccess(err)) return err; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Payment.h b/src/xrpld/app/tx/detail/Payment.h index 010a2453cf..04bba390e2 100644 --- a/src/xrpld/app/tx/detail/Payment.h +++ b/src/xrpld/app/tx/detail/Payment.h @@ -42,6 +42,12 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp index 76224ba6b3..9fe48ba515 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -27,23 +27,11 @@ namespace ripple { NotTEC PermissionedDomainDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index cc25809aa1..d9fa481bb6 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -28,22 +28,15 @@ namespace ripple { +bool +PermissionedDomainSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return ctx.rules.enabled(featureCredentials); +} + NotTEC PermissionedDomainSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePermissionedDomains) || - !ctx.rules.enabled(featureCredentials)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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, @@ -55,7 +48,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) if (domain && *domain == beast::zero) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.h b/src/xrpld/app/tx/detail/PermissionedDomainSet.h index 502d576e32..ed27896a3b 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.h +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index dc84c7cc7e..c2129ba1e1 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -57,23 +57,20 @@ SetAccount::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, getTxConsequencesCategory(ctx.tx)}; } +std::uint32_t +SetAccount::getFlagsMask(PreflightContext const& ctx) +{ + return tfAccountSetMask; +} + NotTEC SetAccount::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto& tx = ctx.tx; auto& j = ctx.j; 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); @@ -186,7 +183,7 @@ SetAccount::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetAccount.h b/src/xrpld/app/tx/detail/SetAccount.h index ed4242c250..bcc0a61b1b 100644 --- a/src/xrpld/app/tx/detail/SetAccount.h +++ b/src/xrpld/app/tx/detail/SetAccount.h @@ -38,6 +38,9 @@ public: static TxConsequences makeTxConsequences(PreflightContext const& ctx); + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetOracle.cpp b/src/xrpld/app/tx/detail/SetOracle.cpp index 78ff8e2953..81f20b4342 100644 --- a/src/xrpld/app/tx/detail/SetOracle.cpp +++ b/src/xrpld/app/tx/detail/SetOracle.cpp @@ -39,15 +39,6 @@ tokenPairKey(STObject const& pair) NotTEC SetOracle::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featurePriceOracle)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries); if (dataSeries.empty()) return temARRAY_EMPTY; @@ -64,7 +55,7 @@ SetOracle::preflight(PreflightContext const& ctx) isInvalidLength(sfAssetClass, maxOracleSymbolClass)) return temMALFORMED; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetRegularKey.cpp b/src/xrpld/app/tx/detail/SetRegularKey.cpp index 92d130a15a..4e063e7d1f 100644 --- a/src/xrpld/app/tx/detail/SetRegularKey.cpp +++ b/src/xrpld/app/tx/detail/SetRegularKey.cpp @@ -51,18 +51,6 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC SetRegularKey::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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))) @@ -70,7 +58,7 @@ SetRegularKey::preflight(PreflightContext const& ctx) return temBAD_REGKEY; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index 60f92cf87b..b5d9d4d5b8 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -77,19 +77,16 @@ SetSignerList::determineOperation( return std::make_tuple(tesSUCCESS, quorum, sign, op); } +std::uint32_t +SetSignerList::getFlagsMask(PreflightContext const& ctx) +{ + // 0 means "Allow any flags" + return ctx.rules.enabled(fixInvalidTxFlags) ? tfUniversalMask : 0; +} + NotTEC SetSignerList::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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) @@ -119,7 +116,7 @@ SetSignerList::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetSignerList.h b/src/xrpld/app/tx/detail/SetSignerList.h index 1827aca975..be2df8152e 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.h +++ b/src/xrpld/app/tx/detail/SetSignerList.h @@ -51,6 +51,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 87f1721b29..21d4534f93 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -67,23 +67,20 @@ computeFreezeFlags( namespace ripple { +std::uint32_t +SetTrust::getFlagsMask(PreflightContext const& ctx) +{ + return tfTrustSetMask; +} + NotTEC SetTrust::preflight(PreflightContext const& ctx) { - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - auto& tx = ctx.tx; auto& j = ctx.j; 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 @@ -127,7 +124,7 @@ SetTrust::preflight(PreflightContext const& ctx) return temDST_NEEDED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/SetTrust.h b/src/xrpld/app/tx/detail/SetTrust.h index a0476918ac..443080bf74 100644 --- a/src/xrpld/app/tx/detail/SetTrust.h +++ b/src/xrpld/app/tx/detail/SetTrust.h @@ -35,6 +35,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index fd396e4556..1c98233964 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -41,7 +41,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.isFlag(tfInnerBatchTxn)) { @@ -83,12 +83,84 @@ 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; } +namespace detail { + +/** Checks the validity of the transactor signing key. + * + * Normally called from preflight1. + */ +NotTEC +preflightCheckSigningKey(STObject const& sigObject, beast::Journal j) +{ + if (auto const spk = sigObject.getFieldVL(sfSigningPubKey); + !spk.empty() && !publicKeyType(makeSlice(spk))) + { + JLOG(j.debug()) << "preflightCheckSigningKey: invalid signing key"; + return temBAD_SIGNATURE; + } + return tesSUCCESS; +} + +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j) +{ + if (flags & tapDRY_RUN) // simulation + { + std::optional const signature = sigObject[~sfTxnSignature]; + if (signature && !signature->empty()) + { + // NOTE: This code should never be hit because it's checked in the + // `simulate` RPC + return temINVALID; // LCOV_EXCL_LINE + } + + if (!sigObject.isFieldPresent(sfSigners)) + { + // no signers, no signature - a valid simulation + return tesSUCCESS; + } + + for (auto const& signer : sigObject.getFieldArray(sfSigners)) + { + if (signer.isFieldPresent(sfTxnSignature) && + !signer[sfTxnSignature].empty()) + { + // NOTE: This code should never be hit because it's + // checked in the `simulate` RPC + return temINVALID; // LCOV_EXCL_LINE + } + } + + Slice const signingPubKey = sigObject[sfSigningPubKey]; + if (!signingPubKey.empty()) + { + // trying to single-sign _and_ multi-sign a transaction + return temINVALID; + } + return tesSUCCESS; + } + return {}; +} + +} // namespace detail + /** Performs early sanity checks on the account and fee fields */ NotTEC -preflight1(PreflightContext const& ctx) +Transactor::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. @@ -107,8 +179,7 @@ preflight1(PreflightContext const& ctx) return temBAD_SIGNER; } - auto const ret = preflight0(ctx); - if (!isTesSuccess(ret)) + if (auto const ret = preflight0(ctx, flagMask)) return ret; auto const id = ctx.tx.getAccountID(sfAccount); @@ -126,13 +197,8 @@ preflight1(PreflightContext const& ctx) return temBAD_FEE; } - auto const spk = ctx.tx.getSigningPubKey(); - - if (!spk.empty() && !publicKeyType(makeSlice(spk))) - { - JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; - return temBAD_SIGNATURE; - } + if (auto const ret = detail::preflightCheckSigningKey(ctx.tx, ctx.j)) + return ret; // An AccountTxnID field constrains transaction ordering more than the // Sequence field. Tickets, on the other hand, reduce ordering @@ -157,41 +223,13 @@ preflight1(PreflightContext const& ctx) /** Checks whether the signature appears valid */ NotTEC -preflight2(PreflightContext const& ctx) +Transactor::preflight2(PreflightContext const& ctx) { - if (ctx.flags & tapDRY_RUN) // simulation - { - if (!ctx.tx.getSignature().empty()) - { - // NOTE: This code should never be hit because it's checked in the - // `simulate` RPC - return temINVALID; // LCOV_EXCL_LINE - } - - if (!ctx.tx.isFieldPresent(sfSigners)) - { - // no signers, no signature - a valid simulation - return tesSUCCESS; - } - - for (auto const& signer : ctx.tx.getFieldArray(sfSigners)) - { - if (signer.isFieldPresent(sfTxnSignature) && - !signer[sfTxnSignature].empty()) - { - // NOTE: This code should never be hit because it's - // checked in the `simulate` RPC - return temINVALID; // LCOV_EXCL_LINE - } - } - - if (!ctx.tx.getSigningPubKey().empty()) - { - // trying to single-sign _and_ multi-sign a transaction - return temINVALID; - } - return tesSUCCESS; - } + if (auto const ret = + detail::preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j)) + // Skips following checks if the transaction is being simulated, + // regardless of success or failure + return *ret; auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); @@ -213,6 +251,28 @@ Transactor::Transactor(ApplyContext& ctx) { } +bool +Transactor::validDataLength( + std::optional const& slice, + std::size_t maxLength) +{ + if (!slice) + return true; + return !slice->empty() && slice->length() <= maxLength; +} + +std::uint32_t +Transactor::getFlagsMask(PreflightContext const& ctx) +{ + return tfUniversalMask; +} + +NotTEC +Transactor::preflightSigValidated(PreflightContext const& ctx) +{ + return tesSUCCESS; +} + TER Transactor::checkPermission(ReadView const& view, STTx const& tx) { @@ -247,6 +307,27 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx) return baseFee + (signerCount * baseFee); } +// Returns the fee in fee units, not scaled for load. +XRPAmount +Transactor::calculateOwnerReserveFee(ReadView const& view, STTx const& tx) +{ + // Assumption: One reserve increment is typically much greater than one base + // fee. + // This check is in an assert so that it will come to the attention of + // developers if that assumption is not correct. If the owner reserve is not + // significantly larger than the base fee (or even worse, smaller), we will + // need to rethink charging an owner reserve as a transaction fee. + // TODO: This function is static, and I don't want to add more parameters. + // When it is finally refactored to be in a context that has access to the + // Application, include "app().overlay().networkID() > 2 ||" in the + // condition. + XRPL_ASSERT( + view.fees().increment > view.fees().base * 100, + "ripple::Transactor::calculateOwnerReserveFee : Owner reserve is " + "reasonable"); + return view.fees().increment; +} + XRPAmount Transactor::minimumFee( Application& app, diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index e94b93523d..88b0664ea2 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -134,6 +134,8 @@ public: class TxConsequences; struct PreflightResult; +// Needed for preflight specialization +class Change; class Transactor { @@ -198,6 +200,35 @@ public: static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); + /* Do NOT define an invokePreflight function in a derived class. + Instead, define: + + // Optional if the transaction is gated on an amendment that + // isn't specified in transactions.macro + static bool + checkExtraFeatures(PreflightContext const& ctx); + + // Optional if the transaction uses any flags other than tfUniversal + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + + // Required, even if it just returns tesSUCCESS. + static NotTEC + preflight(PreflightContext const& ctx); + + // Optional, rarely needed, if the transaction does any expensive + // checks after the signature is verified. + static NotTEC preflightSigValidated(PreflightContext const& ctx); + + * Do not try to call preflight1 or preflight2 directly. + * Do not check whether relevant amendments are enabled in preflight. + Instead, define checkExtraFeatures. + * Do not check flags in preflight. Instead, define getFlagsMask. + */ + template + static NotTEC + invokePreflight(PreflightContext const& ctx); + static TER preclaim(PreclaimContext const& ctx) { @@ -246,6 +277,36 @@ protected: Fees const& fees, ApplyFlags flags); + // Returns the fee in fee units, not scaled for load. + static XRPAmount + calculateOwnerReserveFee(ReadView const& view, STTx const& tx); + + // Base class always returns true + static bool + checkExtraFeatures(PreflightContext const& ctx); + + // Base class always returns tfUniversalMask + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + + // Base class always returns tesSUCCESS + static NotTEC + preflightSigValidated(PreflightContext const& ctx); + + static bool + validDataLength(std::optional const& slice, std::size_t maxLength); + + template + static bool + validNumericRange(std::optional value, T max, T min = {}); + + template + static bool + validNumericRange( + std::optional value, + unit::ValueUnit max, + unit::ValueUnit min = {}); + private: std::pair reset(XRPAmount fee); @@ -270,19 +331,106 @@ private: beast::Journal j); void trapTransaction(uint256) const; + + /** Performs early sanity checks on the account and fee fields. + + (And passes flagMask to preflight0) + + Do not try to call preflight1 from preflight() in derived classes. See + the description of invokePreflight for details. + */ + static NotTEC + preflight1(PreflightContext const& ctx, std::uint32_t flagMask); + + /** Checks whether the signature appears valid + + Do not try to call preflight2 from preflight() in derived classes. See + the description of invokePreflight for details. + */ + static NotTEC + preflight2(PreflightContext const& ctx); }; -/** Performs early sanity checks on the txid */ -NotTEC -preflight0(PreflightContext const& ctx); +inline bool +Transactor::checkExtraFeatures(PreflightContext const& ctx) +{ + return true; +} -/** Performs early sanity checks on the account and fee fields */ +/** Performs early sanity checks on the txid and flags */ NotTEC -preflight1(PreflightContext const& ctx); +preflight0(PreflightContext const& ctx, std::uint32_t flagMask); -/** Checks whether the signature appears valid */ +namespace detail { + +/** Checks the validity of the transactor signing key. + * + * Normally called from preflight1 with ctx.tx. + */ NotTEC -preflight2(PreflightContext const& ctx); +preflightCheckSigningKey(STObject const& sigObject, beast::Journal j); + +/** Checks the special signing key state needed for simulation + * + * Normally called from preflight2 with ctx.tx. + */ +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j); +} // namespace detail + +// Defined in Change.cpp +template <> +NotTEC +Transactor::invokePreflight(PreflightContext const& ctx); + +template +NotTEC +Transactor::invokePreflight(PreflightContext const& ctx) +{ + // Using this lookup does NOT require checking the fixDelegateV1_1. The data + // exists regardless of whether it is enabled. + auto const feature = + Permission::getInstance().getTxFeature(ctx.tx.getTxnType()); + + if (feature && !ctx.rules.enabled(*feature)) + return temDISABLED; + + if (!T::checkExtraFeatures(ctx)) + return temDISABLED; + + if (auto const ret = preflight1(ctx, T::getFlagsMask(ctx))) + return ret; + + if (auto const ret = T::preflight(ctx)) + return ret; + + if (auto const ret = preflight2(ctx)) + return ret; + + return T::preflightSigValidated(ctx); +} + +template +bool +Transactor::validNumericRange(std::optional value, T max, T min) +{ + if (!value) + return true; + return value >= min && value <= max; +} + +template +bool +Transactor::validNumericRange( + std::optional value, + unit::ValueUnit max, + unit::ValueUnit min) +{ + return validNumericRange(value, max.value(), min.value()); +} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 061aacdbb8..45a56a6292 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -35,15 +35,6 @@ namespace ripple { NotTEC VaultClawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultClawback: zero/empty vault ID."; @@ -78,7 +69,7 @@ VaultClawback::preflight(PreflightContext const& ctx) } } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 855275bf4e..9447976a32 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -35,28 +35,27 @@ namespace ripple { +bool +VaultCreate::checkExtraFeatures(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featureMPTokensV1)) + return false; + + return !ctx.tx.isFieldPresent(sfDomainID) || + ctx.rules.enabled(featurePermissionedDomains); +} + +std::uint32_t +VaultCreate::getFlagsMask(PreflightContext const& ctx) +{ + return tfVaultCreateMask; +} + NotTEC VaultCreate::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault) || - !ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfVaultCreateMask) - return temINVALID_FLAG; - - if (auto const data = ctx.tx[~sfData]) - { - if (data->empty() || data->length() > maxDataPayloadLength) - return temMALFORMED; - } + if (!validDataLength(ctx.tx[~sfData], maxDataPayloadLength)) + return temMALFORMED; if (auto const withdrawalPolicy = ctx.tx[~sfWithdrawalPolicy]) { @@ -96,14 +95,14 @@ VaultCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } XRPAmount VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx) { // One reserve increment is typically much greater than one base fee. - return view.fees().increment; + return calculateOwnerReserveFee(view, tx); } TER @@ -112,32 +111,8 @@ VaultCreate::preclaim(PreclaimContext const& ctx) auto const vaultAsset = ctx.tx[sfAsset]; auto const account = ctx.tx[sfAccount]; - if (vaultAsset.native()) - ; // No special checks for XRP - else if (vaultAsset.holds()) - { - auto mptID = vaultAsset.get().getMptID(); - auto issuance = ctx.view.read(keylet::mptIssuance(mptID)); - if (!issuance) - return tecOBJECT_NOT_FOUND; - if (!issuance->isFlag(lsfMPTCanTransfer)) - { - // NOTE: flag lsfMPTCanTransfer is immutable, so this is debug in - // VaultCreate only; in other vault function it's an error. - JLOG(ctx.j.debug()) - << "VaultCreate: vault assets are non-transferable."; - return tecNO_AUTH; - } - } - else if (vaultAsset.holds()) - { - auto const issuer = - ctx.view.read(keylet::account(vaultAsset.getIssuer())); - if (!issuer) - return terNO_ACCOUNT; - else if (!issuer->isFlag(lsfDefaultRipple)) - return terNO_RIPPLE; - } + if (auto const ter = canAddHolding(ctx.view, vaultAsset)) + return ter; // Check for pseudo-account issuers - we do not want a vault to hold such // assets (e.g. MPT shares to other vaults or AMM LPTokens) as they would be diff --git a/src/xrpld/app/tx/detail/VaultCreate.h b/src/xrpld/app/tx/detail/VaultCreate.h index 5555644629..3f952d540a 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.h +++ b/src/xrpld/app/tx/detail/VaultCreate.h @@ -33,6 +33,12 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/VaultDelete.cpp b/src/xrpld/app/tx/detail/VaultDelete.cpp index 5e4e16a99b..ab7db78956 100644 --- a/src/xrpld/app/tx/detail/VaultDelete.cpp +++ b/src/xrpld/app/tx/detail/VaultDelete.cpp @@ -31,22 +31,13 @@ namespace ripple { NotTEC VaultDelete::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultDelete: zero/empty vault ID."; return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 3d346d63a2..75cf81b0b0 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -36,15 +36,6 @@ namespace ripple { NotTEC VaultDeposit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultDeposit: zero/empty vault ID."; @@ -54,7 +45,7 @@ VaultDeposit::preflight(PreflightContext const& ctx) if (ctx.tx[sfAmount] <= beast::zero) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 5a519f81cf..6057e40cfa 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -30,28 +30,22 @@ namespace ripple { +bool +VaultSet::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfDomainID) || + ctx.rules.enabled(featurePermissionedDomains); +} + NotTEC VaultSet::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDomainID) && - !ctx.rules.enabled(featurePermissionedDomains)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultSet: zero/empty vault ID."; return temMALFORMED; } - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (auto const data = ctx.tx[~sfData]) { if (data->empty() || data->length() > maxDataPayloadLength) @@ -78,7 +72,7 @@ VaultSet::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/VaultSet.h b/src/xrpld/app/tx/detail/VaultSet.h index f16aa6c284..b3eecbbab5 100644 --- a/src/xrpld/app/tx/detail/VaultSet.h +++ b/src/xrpld/app/tx/detail/VaultSet.h @@ -33,6 +33,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 63cc22fe48..509b795058 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -33,15 +33,6 @@ namespace ripple { NotTEC VaultWithdraw::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureSingleAssetVault)) - return temDISABLED; - - if (auto const ter = preflight1(ctx)) - return ter; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (ctx.tx[sfVaultID] == beast::zero) { JLOG(ctx.j.debug()) << "VaultWithdraw: zero/empty vault ID."; @@ -68,7 +59,7 @@ VaultWithdraw::preflight(PreflightContext const& ctx) return temMALFORMED; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 92e3c7f625..2587845df5 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -1210,17 +1210,8 @@ toClaim(STTx const& tx) template NotTEC -attestationPreflight(PreflightContext const& ctx) +attestationpreflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - if (!publicKeyType(ctx.tx[sfPublicKey])) return temMALFORMED; @@ -1241,7 +1232,7 @@ attestationPreflight(PreflightContext const& ctx) if (att->sendingAmount.issue() != expectedIssue) return temXCHAIN_BAD_PROOF; - return preflight2(ctx); + return tesSUCCESS; } template @@ -1379,15 +1370,6 @@ attestationDoApply(ApplyContext& ctx) NotTEC XChainCreateBridge::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -1457,7 +1439,7 @@ XChainCreateBridge::preflight(PreflightContext const& ctx) return temXCHAIN_BRIDGE_BAD_ISSUES; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1557,18 +1539,15 @@ XChainCreateBridge::doApply() //------------------------------------------------------------------------------ +std::uint32_t +BridgeModify::getFlagsMask(PreflightContext const& ctx) +{ + return tfBridgeModifyMask; +} + NotTEC BridgeModify::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - 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]; @@ -1607,7 +1586,7 @@ BridgeModify::preflight(PreflightContext const& ctx) return temXCHAIN_BRIDGE_BAD_MIN_ACCOUNT_CREATE_AMOUNT; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1670,15 +1649,6 @@ BridgeModify::doApply() NotTEC XChainClaim::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge]; auto const amount = ctx.tx[sfAmount]; @@ -1689,7 +1659,7 @@ XChainClaim::preflight(PreflightContext const& ctx) return temBAD_AMOUNT; } - return preflight2(ctx); + return tesSUCCESS; } TER @@ -1908,15 +1878,6 @@ XChainCommit::makeTxConsequences(PreflightContext const& ctx) NotTEC XChainCommit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; auto const bridgeSpec = ctx.tx[sfXChainBridge]; @@ -1927,7 +1888,7 @@ XChainCommit::preflight(PreflightContext const& ctx) amount.issue() != bridgeSpec.issuingChainIssue()) return temBAD_ISSUER; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -2022,21 +1983,12 @@ XChainCommit::doApply() NotTEC XChainCreateClaimID::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const reward = ctx.tx[sfSignatureReward]; if (!isXRP(reward) || reward.signum() < 0 || !isLegalNet(reward)) return temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER @@ -2137,7 +2089,7 @@ XChainCreateClaimID::doApply() NotTEC XChainAddClaimAttestation::preflight(PreflightContext const& ctx) { - return attestationPreflight(ctx); + return attestationpreflight(ctx); } TER @@ -2157,7 +2109,7 @@ XChainAddClaimAttestation::doApply() NotTEC XChainAddAccountCreateAttestation::preflight(PreflightContext const& ctx) { - return attestationPreflight(ctx); + return attestationpreflight(ctx); } TER @@ -2177,15 +2129,6 @@ XChainAddAccountCreateAttestation::doApply() NotTEC XChainCreateAccountCommit::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureXChainBridge)) - return temDISABLED; - - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (ctx.tx.getFlags() & tfUniversalMask) - return temINVALID_FLAG; - auto const amount = ctx.tx[sfAmount]; if (amount.signum() <= 0 || !amount.native()) @@ -2198,7 +2141,7 @@ XChainCreateAccountCommit::preflight(PreflightContext const& ctx) if (reward.issue() != amount.issue()) return temBAD_AMOUNT; - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/XChainBridge.h b/src/xrpld/app/tx/detail/XChainBridge.h index 82b64cc0e3..0e9c0358d2 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.h +++ b/src/xrpld/app/tx/detail/XChainBridge.h @@ -58,6 +58,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 543bedcd47..c2e4e13f08 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -119,7 +119,7 @@ invoke_preflight(PreflightContext const& ctx) try { return with_txn_type(ctx.tx.getTxnType(), [&]() { - auto const tec = T::preflight(ctx); + auto const tec = Transactor::invokePreflight(ctx); return std::make_pair( tec, isTesSuccess(tec) ? consequences_helper(ctx)