From e1dff6f26e4f4431bd6ec3eaade13029568ef847 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 5 Oct 2023 14:14:44 -0400 Subject: [PATCH] Refactor to use an `UnknownTxn` class instead of exceptions * Follow-up to #4710 --- src/xrpld/app/tx/detail/applySteps.cpp | 169 ++++++++++++------------- 1 file changed, 81 insertions(+), 88 deletions(-) diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 9ddaa3051c4..8772c187cbf 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -53,25 +53,53 @@ #include #include -#include - namespace ripple { namespace { -struct UnknownTxnType : std::exception +struct UnknownTxn : Transactor { - TxType txnType; - UnknownTxnType(TxType t) : txnType{t} +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit UnknownTxn(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx) + { + // Should never happen + return temUNKNOWN; + } + + static TER + preclaim(PreclaimContext const& ctx) + { + // Should never happen + return temUNKNOWN; + } + + static XRPAmount + calculateBaseFee(ReadView const& view, STTx const& tx) + { + // Should never happen + return XRPAmount{0}; + } + + TER + doApply() override { + // Should never happen + return temUNKNOWN; } }; -// Call a lambda with the concrete transaction type as a template parameter -// throw an "UnknownTxnType" exception on error +// Call a lambda with the concrete transaction type as a template parameter. +// Use the "UnknownTxn" class on error. template auto -with_txn_type(TxType txnType, F&& f) +with_txn_type(TxType txnType, std::optional j, F&& f) { switch (txnType) { @@ -166,7 +194,13 @@ with_txn_type(TxType txnType, F&& f) case ttORACLE_DELETE: return f.template operator()(); default: - throw UnknownTxnType(txnType); + // Should never happen + if (j) + JLOG(j->fatal()) + << "Unknown transaction type in with_txn_type: " << txnType; + auto const result = f.template operator()(); + assert(false); + return result; } } } // namespace @@ -213,89 +247,59 @@ TxConsequences static std::pair invoke_preflight(PreflightContext const& ctx) { - try - { - return with_txn_type(ctx.tx.getTxnType(), [&]() { - auto const tec = T::preflight(ctx); - return std::make_pair( - tec, - isTesSuccess(tec) ? consequences_helper(ctx) - : TxConsequences{tec}); - }); - } - catch (UnknownTxnType const& e) - { - // Should never happen - JLOG(ctx.j.fatal()) - << "Unknown transaction type in preflight: " << e.txnType; - assert(false); - return {temUNKNOWN, TxConsequences{temUNKNOWN}}; - } + return with_txn_type(ctx.tx.getTxnType(), ctx.j, [&]() { + auto const tec = T::preflight(ctx); + return std::make_pair( + tec, + isTesSuccess(tec) ? consequences_helper(ctx) + : TxConsequences{tec}); + }); } static TER invoke_preclaim(PreclaimContext const& ctx) { - try - { - // use name hiding to accomplish compile-time polymorphism of static - // class functions for Transactor and derived classes. - return with_txn_type(ctx.tx.getTxnType(), [&]() { - // If the transactor requires a valid account and the transaction - // doesn't list one, preflight will have already a flagged a - // failure. - auto const id = ctx.tx.getAccountID(sfAccount); + // use name hiding to accomplish compile-time polymorphism of static + // class functions for Transactor and derived classes. + return with_txn_type(ctx.tx.getTxnType(), ctx.j, [&]() { + // If the transactor requires a valid account and the transaction + // doesn't list one, preflight will have already a flagged a + // failure. + auto const id = ctx.tx.getAccountID(sfAccount); - if (id != beast::zero) - { - TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); + if (id != beast::zero) + { + TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); - if (result != tesSUCCESS) - return result; + if (result != tesSUCCESS) + return result; - result = T::checkPriorTxAndLastLedger(ctx); + result = T::checkPriorTxAndLastLedger(ctx); - if (result != tesSUCCESS) - return result; + if (result != tesSUCCESS) + return result; - result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); + result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); - if (result != tesSUCCESS) - return result; + if (result != tesSUCCESS) + return result; - result = T::checkSign(ctx); + result = T::checkSign(ctx); - if (result != tesSUCCESS) - return result; - } + if (result != tesSUCCESS) + return result; + } - return T::preclaim(ctx); - }); - } - catch (UnknownTxnType const& e) - { - // Should never happen - JLOG(ctx.j.fatal()) - << "Unknown transaction type in preclaim: " << e.txnType; - assert(false); - return temUNKNOWN; - } + return T::preclaim(ctx); + }); } static XRPAmount invoke_calculateBaseFee(ReadView const& view, STTx const& tx) { - try - { - return with_txn_type(tx.getTxnType(), [&]() { - return T::calculateBaseFee(view, tx); - }); - } - catch (UnknownTxnType const& e) - { - assert(false); - return XRPAmount{0}; - } + return with_txn_type(tx.getTxnType(), {}, [&]() { + return T::calculateBaseFee(view, tx); + }); } TxConsequences::TxConsequences(NotTEC pfresult) @@ -340,21 +344,10 @@ TxConsequences::TxConsequences(STTx const& tx, std::uint32_t sequencesConsumed) static std::pair invoke_apply(ApplyContext& ctx) { - try - { - return with_txn_type(ctx.tx.getTxnType(), [&]() { - T p(ctx); - return p(); - }); - } - catch (UnknownTxnType const& e) - { - // Should never happen - JLOG(ctx.journal.fatal()) - << "Unknown transaction type in apply: " << e.txnType; - assert(false); - return {temUNKNOWN, false}; - } + return with_txn_type(ctx.tx.getTxnType(), ctx.journal, [&]() { + T p(ctx); + return p(); + }); } PreflightResult