diff --git a/src/ripple/app/tx/apply.h b/src/ripple/app/tx/apply.h index 9ff582872..a85935f17 100644 --- a/src/ripple/app/tx/apply.h +++ b/src/ripple/app/tx/apply.h @@ -30,12 +30,55 @@ namespace ripple { +/** Gate a transaction based on static information. + + The transaction is checked against all possible + validity constraints that do not require a ledger. + + @return The TER code (a `tem` or tesSUCCESS) +*/ +TER +preflight (Rules const& rules, STTx const& tx, + ApplyFlags flags, + Config const& config, beast::Journal j); + +/** Apply a prechecked transaction to an OpenView. + + See also: apply() + + Precondition: The transaction has been checked + and validated using the above function(s). + + @return A pair with the TER and a bool indicating + whether or not the transaction was applied. +*/ +std::pair +doapply(OpenView& view, STTx const& tx, + ApplyFlags flags, Config const& config, + beast::Journal j); + /** Apply a transaction to a ReadView. Throws: - Does not throw. Exceptions generated during - tx application will return tefEXCEPTION. + Does not throw. + + For open ledgers, the Transactor will catch and + return tefEXCEPTION. For closed ledgers, the + Transactor will attempt to only charge a fee, + and return tecFAILED_PROCESSING. + + If the Transactor gets an exception while trying + to charge the fee, it will be caught here and + turned into tefEXCEPTION. + + This try/catch handler is the last resort, any + uncaught exceptions will be turned into + tefEXCEPTION. + + For network health, a Transactor makes its + best effort to at least charge a fee if the + ledger is closed. @return A pair with the TER and a bool indicating whether or not the transaction was applied. diff --git a/src/ripple/app/tx/impl/ApplyContext.h b/src/ripple/app/tx/impl/ApplyContext.h index dda04edc2..4094316e2 100644 --- a/src/ripple/app/tx/impl/ApplyContext.h +++ b/src/ripple/app/tx/impl/ApplyContext.h @@ -88,7 +88,6 @@ public: private: OpenView& base_; ApplyFlags flags_; - beast::Journal j_; boost::optional view_; }; diff --git a/src/ripple/app/tx/impl/CancelOffer.cpp b/src/ripple/app/tx/impl/CancelOffer.cpp index e44272052..d43fe1c9f 100644 --- a/src/ripple/app/tx/impl/CancelOffer.cpp +++ b/src/ripple/app/tx/impl/CancelOffer.cpp @@ -1,4 +1,4 @@ - +//------------------------------------------------------------------------------ /* This file is part of rippled: https://github.com/ripple/rippled Copyright (c) 2012, 2013 Ripple Labs Inc. @@ -27,33 +27,34 @@ namespace ripple { TER -CancelOffer::preCheck () +CancelOffer::preflight (PreflightContext const& ctx) { - std::uint32_t const uTxFlags (mTxn.getFlags ()); + auto const uTxFlags = ctx.tx.getFlags(); if (uTxFlags & tfUniversalMask) { - j_.trace << "Malformed transaction: " << + JLOG(ctx.j.trace) << "Malformed transaction: " << "Invalid flags set."; return temINVALID_FLAG; } - std::uint32_t const uOfferSequence = mTxn.getFieldU32 (sfOfferSequence); - - if (!uOfferSequence) + auto const seq = ctx.tx.getFieldU32 (sfOfferSequence); + if (! seq) { - j_.trace << "Malformed transaction: " << - "No sequence specified."; + JLOG(ctx.j.trace) << + "CancelOffer::preflight: missing sequence"; return temBAD_SEQUENCE; } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } +//------------------------------------------------------------------------------ + TER CancelOffer::doApply () { - std::uint32_t const uOfferSequence = mTxn.getFieldU32 (sfOfferSequence); + std::uint32_t const uOfferSequence = tx().getFieldU32 (sfOfferSequence); auto const sle = view().read( keylet::account(account_)); diff --git a/src/ripple/app/tx/impl/CancelOffer.h b/src/ripple/app/tx/impl/CancelOffer.h index 61a65c2f6..b673306cd 100644 --- a/src/ripple/app/tx/impl/CancelOffer.h +++ b/src/ripple/app/tx/impl/CancelOffer.h @@ -31,14 +31,14 @@ class CancelOffer : public Transactor { public: - template - CancelOffer (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + CancelOffer (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck () override; + static + TER + preflight (PreflightContext const& ctx); TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index 3b8725cba..9498bf80f 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -26,19 +26,19 @@ namespace ripple { TER -CancelTicket::preCheck() +CancelTicket::preflight (PreflightContext const& ctx) { #if ! RIPPLE_ENABLE_TICKETS - if (! (view().flags() & tapENABLE_TESTING)) + if (! (ctx.flags & tapENABLE_TESTING)) return temDISABLED; #endif - return Transactor::preCheck (); + return Transactor::preflight(ctx); } TER CancelTicket::doApply () { - uint256 const ticketId = mTxn.getFieldH256 (sfTicketID); + uint256 const ticketId = tx().getFieldH256 (sfTicketID); // VFALCO This is highly suspicious, we're requiring that the // transaction provide the return value of getTicketIndex? diff --git a/src/ripple/app/tx/impl/CancelTicket.h b/src/ripple/app/tx/impl/CancelTicket.h index e92d0f48f..2e205024a 100644 --- a/src/ripple/app/tx/impl/CancelTicket.h +++ b/src/ripple/app/tx/impl/CancelTicket.h @@ -30,14 +30,15 @@ class CancelTicket : public Transactor { public: - template - CancelTicket (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + CancelTicket (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck() override; + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index 69ad701f0..517c80cc9 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -28,13 +28,35 @@ namespace ripple { +TER +Change::preflight (PreflightContext const& ctx) +{ + auto account = ctx.tx.getAccountID(sfAccount); + + if (account.isNonZero ()) + { + JLOG(ctx.j.warning) << "Bad source id"; + return temBAD_SRC_ACCOUNT; + } + + return tesSUCCESS; +} + TER Change::doApply() { - if (mTxn.getTxnType () == ttAMENDMENT) + // If tapOPEN_LEDGER is resurrected into ApplyFlags, + // this block can be moved to preflight. + if (view().open()) + { + j_.warning << "Change transaction against open ledger"; + return temINVALID; + } + + if (tx().getTxnType () == ttAMENDMENT) return applyAmendment (); - if (mTxn.getTxnType () == ttFEE) + if (tx().getTxnType () == ttFEE) return applyFee (); return temUNKNOWN; @@ -43,13 +65,7 @@ Change::doApply() TER Change::checkSign() { - if (mTxn.getAccountID (sfAccount).isNonZero ()) - { - j_.warning << "Bad source account"; - return temBAD_SRC_ACCOUNT; - } - - if (!mTxn.getSigningPubKey ().empty () || !mTxn.getSignature ().empty ()) + if (!tx().getSigningPubKey ().empty () || !tx().getSignature ().empty ()) { j_.warning << "Bad signature"; return temBAD_SIGNATURE; @@ -61,7 +77,7 @@ Change::checkSign() TER Change::checkSeq() { - if ((mTxn.getSequence () != 0) || mTxn.isFieldPresent (sfPreviousTxnID)) + if ((tx().getSequence () != 0) || tx().isFieldPresent (sfPreviousTxnID)) { j_.warning << "Bad sequence"; return temBAD_SEQUENCE; @@ -73,7 +89,7 @@ Change::checkSeq() TER Change::payFee() { - if (mTxn.getTransactionFee () != STAmount ()) + if (tx().getTransactionFee () != STAmount ()) { j_.warning << "Non-zero fee"; return temBAD_FEE; @@ -82,30 +98,17 @@ Change::payFee() return tesSUCCESS; } -TER -Change::preCheck() +void +Change::preCompute() { - account_ = mTxn.getAccountID(sfAccount); - - if (account_.isNonZero ()) - { - j_.warning << "Bad source id"; - return temBAD_SRC_ACCOUNT; - } - - if (view().open()) - { - j_.warning << "Change transaction against open ledger"; - return temINVALID; - } - - return tesSUCCESS; + account_ = tx().getAccountID(sfAccount); + assert(account_ == zero); } TER Change::applyAmendment() { - uint256 amendment (mTxn.getFieldH256 (sfAmendment)); + uint256 amendment (tx().getFieldH256 (sfAmendment)); auto const k = keylet::amendments(); @@ -125,7 +128,7 @@ Change::applyAmendment() amendment) != amendments.end ()) return tefALREADY; - auto flags = mTxn.getFlags (); + auto flags = tx().getFlags (); const bool gotMajority = (flags & tfGotMajority) != 0; const bool lostMajority = (flags & tfLostMajority) != 0; @@ -207,13 +210,13 @@ Change::applyFee() // "Previous fee object: " << feeObject->getJson (0); feeObject->setFieldU64 ( - sfBaseFee, mTxn.getFieldU64 (sfBaseFee)); + sfBaseFee, tx().getFieldU64 (sfBaseFee)); feeObject->setFieldU32 ( - sfReferenceFeeUnits, mTxn.getFieldU32 (sfReferenceFeeUnits)); + sfReferenceFeeUnits, tx().getFieldU32 (sfReferenceFeeUnits)); feeObject->setFieldU32 ( - sfReserveBase, mTxn.getFieldU32 (sfReserveBase)); + sfReserveBase, tx().getFieldU32 (sfReserveBase)); feeObject->setFieldU32 ( - sfReserveIncrement, mTxn.getFieldU32 (sfReserveIncrement)); + sfReserveIncrement, tx().getFieldU32 (sfReserveIncrement)); view().update (feeObject); diff --git a/src/ripple/app/tx/impl/Change.h b/src/ripple/app/tx/impl/Change.h index b97bb35ae..6f4de284b 100644 --- a/src/ripple/app/tx/impl/Change.h +++ b/src/ripple/app/tx/impl/Change.h @@ -33,18 +33,20 @@ class Change : public Transactor { public: - template - Change (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + Change (ApplyContext& ctx) + : Transactor(ctx) { } + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; TER checkSign () override; TER checkSeq () override; TER payFee () override; - TER preCheck () override; + void preCompute() override; private: TER applyAmendment (); diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index f60d9b1a3..9557a6170 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -33,6 +33,99 @@ namespace ripple { +TER +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; + } + + bool const bImmediateOrCancel (uTxFlags & tfImmediateOrCancel); + bool const bFillOrKill (uTxFlags & tfFillOrKill); + + if (bImmediateOrCancel && bFillOrKill) + { + JLOG(j.debug) << + "Malformed transaction: both IoC and FoK set."; + return temINVALID_FLAG; + } + + bool const bHaveExpiration (tx.isFieldPresent (sfExpiration)); + + if (bHaveExpiration && (tx.getFieldU32 (sfExpiration) == 0)) + { + JLOG(j.debug) << + "Malformed offer: bad expiration"; + return temBAD_EXPIRATION; + } + + bool const bHaveCancel (tx.isFieldPresent (sfOfferSequence)); + + if (bHaveCancel && (tx.getFieldU32 (sfOfferSequence) == 0)) + { + JLOG(j.debug) << + "Malformed offer: bad cancel sequence"; + return temBAD_SEQUENCE; + } + + STAmount saTakerPays = tx.getFieldAmount (sfTakerPays); + STAmount saTakerGets = tx.getFieldAmount (sfTakerGets); + + if (!isLegalNet (saTakerPays) || !isLegalNet (saTakerGets)) + return temBAD_AMOUNT; + + if (saTakerPays.native () && saTakerGets.native ()) + { + JLOG(j.debug) << + "Malformed offer: XRP for XRP"; + return temBAD_OFFER; + } + if (saTakerPays <= zero || saTakerGets <= zero) + { + JLOG(j.debug) << + "Malformed offer: bad amount"; + return temBAD_OFFER; + } + + auto const& uPaysIssuerID = saTakerPays.getIssuer (); + auto const& uPaysCurrency = saTakerPays.getCurrency (); + + auto const& uGetsIssuerID = saTakerGets.getIssuer (); + auto const& uGetsCurrency = saTakerGets.getCurrency (); + + if (uPaysCurrency == uGetsCurrency && uPaysIssuerID == uGetsIssuerID) + { + JLOG(j.debug) << + "Malformed offer: redundant offer"; + return temREDUNDANT; + } + // We don't allow a non-native currency to use the currency code XRP. + if (badCurrency() == uPaysCurrency || badCurrency() == uGetsCurrency) + { + JLOG(j.debug) << + "Malformed offer: Bad currency."; + return temBAD_CURRENCY; + } + + if (saTakerPays.native () != !uPaysIssuerID || + saTakerGets.native () != !uGetsIssuerID) + { + JLOG(j.warning) << + "Malformed offer: bad issuer"; + return temBAD_ISSUER; + } + + return Transactor::preflight(ctx); +} + TER CreateOffer::checkAcceptAsset(IssueRef issue) const { @@ -388,7 +481,7 @@ CreateOffer::cross ( beast::WrappedSink takerSink (j_, "Taker "); Taker taker (cross_type_, view, account_, taker_amount, - mTxn.getFlags(), ctx_.config, beast::Journal (takerSink)); + tx().getFlags(), ctx_.config, beast::Journal (takerSink)); try { @@ -432,118 +525,34 @@ CreateOffer::getAccountReserve (SLE::pointer account) account->getFieldU32 (sfOwnerCount) + 1)); } -TER -CreateOffer::preCheck () +void +CreateOffer::preCompute() { cross_type_ = CrossType::IouToIou; bool const pays_xrp = - mTxn.getFieldAmount (sfTakerPays).native (); + tx().getFieldAmount (sfTakerPays).native (); bool const gets_xrp = - mTxn.getFieldAmount (sfTakerGets).native (); + tx().getFieldAmount (sfTakerGets).native (); if (pays_xrp && !gets_xrp) cross_type_ = CrossType::IouToXrp; else if (gets_xrp && !pays_xrp) cross_type_ = CrossType::XrpToIou; - std::uint32_t const uTxFlags = mTxn.getFlags (); - - if (uTxFlags & tfOfferCreateMask) - { - if (j_.debug) j_.debug << - "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - - bool const bImmediateOrCancel (uTxFlags & tfImmediateOrCancel); - bool const bFillOrKill (uTxFlags & tfFillOrKill); - - if (bImmediateOrCancel && bFillOrKill) - { - if (j_.debug) j_.debug << - "Malformed transaction: both IoC and FoK set."; - return temINVALID_FLAG; - } - - bool const bHaveExpiration (mTxn.isFieldPresent (sfExpiration)); - - if (bHaveExpiration && (mTxn.getFieldU32 (sfExpiration) == 0)) - { - if (j_.debug) j_.warning << - "Malformed offer: bad expiration"; - return temBAD_EXPIRATION; - } - - bool const bHaveCancel (mTxn.isFieldPresent (sfOfferSequence)); - - if (bHaveCancel && (mTxn.getFieldU32 (sfOfferSequence) == 0)) - { - if (j_.debug) j_.debug << - "Malformed offer: bad cancel sequence"; - return temBAD_SEQUENCE; - } - - STAmount saTakerPays = mTxn.getFieldAmount (sfTakerPays); - STAmount saTakerGets = mTxn.getFieldAmount (sfTakerGets); - - if (!isLegalNet (saTakerPays) || !isLegalNet (saTakerGets)) - return temBAD_AMOUNT; - - if (saTakerPays.native () && saTakerGets.native ()) - { - if (j_.debug) j_.warning << - "Malformed offer: XRP for XRP"; - return temBAD_OFFER; - } - if (saTakerPays <= zero || saTakerGets <= zero) - { - if (j_.debug) j_.warning << - "Malformed offer: bad amount"; - return temBAD_OFFER; - } - - auto const& uPaysIssuerID = saTakerPays.getIssuer (); - auto const& uPaysCurrency = saTakerPays.getCurrency (); - - auto const& uGetsIssuerID = saTakerGets.getIssuer (); - auto const& uGetsCurrency = saTakerGets.getCurrency (); - - if (uPaysCurrency == uGetsCurrency && uPaysIssuerID == uGetsIssuerID) - { - if (j_.debug) j_.debug << - "Malformed offer: redundant offer"; - return temREDUNDANT; - } - // We don't allow a non-native currency to use the currency code XRP. - if (badCurrency() == uPaysCurrency || badCurrency() == uGetsCurrency) - { - if (j_.debug) j_.warning << - "Malformed offer: Bad currency."; - return temBAD_CURRENCY; - } - - if (saTakerPays.native () != !uPaysIssuerID || - saTakerGets.native () != !uGetsIssuerID) - { - if (j_.warning) j_.warning << - "Malformed offer: bad issuer"; - return temBAD_ISSUER; - } - - return Transactor::preCheck (); + return Transactor::preCompute(); } std::pair CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel) { - std::uint32_t const uTxFlags = mTxn.getFlags (); + std::uint32_t const uTxFlags = tx().getFlags (); bool const bPassive (uTxFlags & tfPassive); bool const bImmediateOrCancel (uTxFlags & tfImmediateOrCancel); bool const bFillOrKill (uTxFlags & tfFillOrKill); bool const bSell (uTxFlags & tfSell); - STAmount saTakerPays = mTxn.getFieldAmount (sfTakerPays); - STAmount saTakerGets = mTxn.getFieldAmount (sfTakerGets); + STAmount saTakerPays = tx().getFieldAmount (sfTakerPays); + STAmount saTakerGets = tx().getFieldAmount (sfTakerGets); if (!isLegalNet (saTakerPays) || !isLegalNet (saTakerGets)) return { temBAD_AMOUNT, true }; @@ -553,11 +562,11 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel) auto const& uGetsIssuerID = saTakerGets.getIssuer (); - bool const bHaveExpiration (mTxn.isFieldPresent (sfExpiration)); - bool const bHaveCancel (mTxn.isFieldPresent (sfOfferSequence)); + bool const bHaveExpiration (tx().isFieldPresent (sfExpiration)); + bool const bHaveCancel (tx().isFieldPresent (sfOfferSequence)); - std::uint32_t const uExpiration = mTxn.getFieldU32 (sfExpiration); - std::uint32_t const uCancelSequence = mTxn.getFieldU32 (sfOfferSequence); + std::uint32_t const uExpiration = tx().getFieldU32 (sfExpiration); + std::uint32_t const uCancelSequence = tx().getFieldU32 (sfOfferSequence); // FIXME understand why we use SequenceNext instead of current transaction // sequence to determine the transaction. Why is the offer sequence @@ -568,7 +577,7 @@ CreateOffer::applyGuts (ApplyView& view, ApplyView& view_cancel) deprecatedWrongOwnerCount_ = sleCreator->getFieldU32(sfOwnerCount); std::uint32_t const uAccountSequenceNext = sleCreator->getFieldU32 (sfSequence); - std::uint32_t const uSequence = mTxn.getSequence (); + std::uint32_t const uSequence = tx().getSequence (); // This is the original rate of the offer, and is the rate at which // it will be placed, even if crossing offers change the amounts that diff --git a/src/ripple/app/tx/impl/CreateOffer.h b/src/ripple/app/tx/impl/CreateOffer.h index 5295219dd..5e4a3a96e 100644 --- a/src/ripple/app/tx/impl/CreateOffer.h +++ b/src/ripple/app/tx/impl/CreateOffer.h @@ -39,20 +39,22 @@ class CreateOffer : public Transactor { public: - template - CreateOffer (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + CreateOffer (ApplyContext& ctx) + : Transactor(ctx) { } + static + TER + preflight (PreflightContext const& ctx); + /** Returns the reserve the account would have if an offer was added. */ // VFALCO This function is not needed just inline the behavior STAmount getAccountReserve (SLE::pointer account); // const? - TER - preCheck () override; + void + preCompute() override; std::pair applyGuts (ApplyView& view, ApplyView& view_cancel); diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index ef9048772..f39eb37cb 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -26,24 +26,24 @@ namespace ripple { TER -CreateTicket::preCheck () +CreateTicket::preflight (PreflightContext const& ctx) { #if ! RIPPLE_ENABLE_TICKETS - if (! (view().flags() & tapENABLE_TESTING)) + if (! (ctx.flags & tapENABLE_TESTING)) return temDISABLED; #endif - if (mTxn.isFieldPresent (sfExpiration)) + if (ctx.tx.isFieldPresent (sfExpiration)) { - if (mTxn.getFieldU32 (sfExpiration) == 0) + if (ctx.tx.getFieldU32 (sfExpiration) == 0) { - j_.warning << + JLOG(ctx.j.warning) << "Malformed transaction: bad expiration"; return temBAD_EXPIRATION; } } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } STAmount @@ -65,25 +65,25 @@ CreateTicket::doApply () std::uint32_t expiration (0); - if (mTxn.isFieldPresent (sfExpiration)) + if (tx().isFieldPresent (sfExpiration)) { - expiration = mTxn.getFieldU32 (sfExpiration); + expiration = tx().getFieldU32 (sfExpiration); if (view().parentCloseTime() >= expiration) return tesSUCCESS; } SLE::pointer sleTicket = std::make_shared(ltTICKET, - getTicketIndex (account_, mTxn.getSequence ())); + getTicketIndex (account_, tx().getSequence ())); sleTicket->setAccountID (sfAccount, account_); - sleTicket->setFieldU32 (sfSequence, mTxn.getSequence ()); + sleTicket->setFieldU32 (sfSequence, tx().getSequence ()); if (expiration != 0) sleTicket->setFieldU32 (sfExpiration, expiration); view().insert (sleTicket); - if (mTxn.isFieldPresent (sfTarget)) + if (tx().isFieldPresent (sfTarget)) { - AccountID const target_account (mTxn.getAccountID (sfTarget)); + AccountID const target_account (tx().getAccountID (sfTarget)); SLE::pointer sleTarget = view().peek (keylet::account(target_account)); diff --git a/src/ripple/app/tx/impl/CreateTicket.h b/src/ripple/app/tx/impl/CreateTicket.h index 34a2b2a66..3922d537f 100644 --- a/src/ripple/app/tx/impl/CreateTicket.h +++ b/src/ripple/app/tx/impl/CreateTicket.h @@ -31,15 +31,14 @@ class CreateTicket : public Transactor { public: - template - CreateTicket (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + CreateTicket (ApplyContext& ctx) + : Transactor(ctx) { } + static TER - preCheck () override; + preflight (PreflightContext const& ctx); /** Returns the reserve the account would have if an offer was added. */ // VFALCO Not needed, just inline the behavior. diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 8964b0837..c323e8516 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -39,13 +39,16 @@ allowDeliverMin (ApplyView const& view) } TER -Payment::preCheck () +Payment::preflight (PreflightContext const& ctx) { - std::uint32_t const uTxFlags = mTxn.getFlags (); + auto& tx = ctx.tx; + auto& j = ctx.j; + + std::uint32_t const uTxFlags = tx.getFlags (); if (uTxFlags & tfPaymentMask) { - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "Invalid flags set."; return temINVALID_FLAG; } @@ -53,20 +56,21 @@ Payment::preCheck () bool const partialPaymentAllowed = uTxFlags & tfPartialPayment; bool const limitQuality = uTxFlags & tfLimitQuality; bool const defaultPathsAllowed = !(uTxFlags & tfNoRippleDirect); - bool const bPaths = mTxn.isFieldPresent (sfPaths); - bool const bMax = mTxn.isFieldPresent (sfSendMax); + bool const bPaths = tx.isFieldPresent (sfPaths); + bool const bMax = tx.isFieldPresent (sfSendMax); - STAmount const saDstAmount (mTxn.getFieldAmount (sfAmount)); + STAmount const saDstAmount (tx.getFieldAmount (sfAmount)); STAmount maxSourceAmount; + auto const account = tx.getAccountID(sfAccount); if (bMax) - maxSourceAmount = mTxn.getFieldAmount (sfSendMax); + maxSourceAmount = tx.getFieldAmount (sfSendMax); else if (saDstAmount.native ()) maxSourceAmount = saDstAmount; else maxSourceAmount = STAmount ( - { saDstAmount.getCurrency (), account_ }, + { saDstAmount.getCurrency (), account }, saDstAmount.mantissa(), saDstAmount.exponent (), saDstAmount < zero); @@ -79,134 +83,137 @@ Payment::preCheck () if (!isLegalNet (saDstAmount) || !isLegalNet (maxSourceAmount)) return temBAD_AMOUNT; - AccountID const uDstAccountID (mTxn.getAccountID (sfDestination)); + auto const uDstAccountID = tx.getAccountID (sfDestination); if (!uDstAccountID) { - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "Payment destination account not specified."; return temDST_NEEDED; } if (bMax && maxSourceAmount <= zero) { - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "bad max amount: " << maxSourceAmount.getFullText (); return temBAD_AMOUNT; } if (saDstAmount <= zero) { - j_.trace << "Malformed transaction: "<< + JLOG(j.trace) << "Malformed transaction: "<< "bad dst amount: " << saDstAmount.getFullText (); return temBAD_AMOUNT; } if (badCurrency() == uSrcCurrency || badCurrency() == uDstCurrency) { - j_.trace <<"Malformed transaction: " << + JLOG(j.trace) <<"Malformed transaction: " << "Bad currency."; return temBAD_CURRENCY; } - if (account_ == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths) + if (account == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths) { // You're signing yourself a payment. // If bPaths is true, you might be trying some arbitrage. - j_.trace << "Malformed transaction: " << - "Redundant payment from " << to_string (account_) << + JLOG(j.trace) << "Malformed transaction: " << + "Redundant payment from " << to_string (account) << " to self without path for " << to_string (uDstCurrency); return temREDUNDANT; } if (bXRPDirect && bMax) { // Consistent but redundant transaction. - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "SendMax specified for XRP to XRP."; return temBAD_SEND_XRP_MAX; } if (bXRPDirect && bPaths) { // XRP is sent without paths. - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "Paths specified for XRP to XRP."; return temBAD_SEND_XRP_PATHS; } if (bXRPDirect && partialPaymentAllowed) { // Consistent but redundant transaction. - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "Partial payment specified for XRP to XRP."; return temBAD_SEND_XRP_PARTIAL; } if (bXRPDirect && limitQuality) { // Consistent but redundant transaction. - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "Limit quality specified for XRP to XRP."; return temBAD_SEND_XRP_LIMIT; } if (bXRPDirect && !defaultPathsAllowed) { // Consistent but redundant transaction. - j_.trace << "Malformed transaction: " << + JLOG(j.trace) << "Malformed transaction: " << "No ripple direct specified for XRP to XRP."; return temBAD_SEND_XRP_NO_DIRECT; } - if (mTxn.isFieldPresent(sfDeliverMin)) + if (tx.isFieldPresent(sfDeliverMin)) { - if (! allowDeliverMin(view())) - return temMALFORMED; - if (! partialPaymentAllowed) { - j_.trace << "Malformed transaction: Partial payment not " + JLOG(j.trace) << "Malformed transaction: Partial payment not " "specified for " << jss::DeliverMin.c_str() << "."; return temBAD_AMOUNT; } - auto const dMin = mTxn.getFieldAmount(sfDeliverMin); - if (! isLegalNet(dMin) || dMin <= zero) + auto const dMin = tx.getFieldAmount(sfDeliverMin); + if (!isLegalNet(dMin) || dMin <= zero) { - j_.trace << "Malformed transaction: Invalid " << + JLOG(j.trace) << "Malformed transaction: Invalid " << jss::DeliverMin.c_str() << " amount. " << dMin.getFullText(); return temBAD_AMOUNT; } if (dMin.issue() != saDstAmount.issue()) { - j_.trace << "Malformed transaction: Dst issue differs " + JLOG(j.trace) << "Malformed transaction: Dst issue differs " "from " << jss::DeliverMin.c_str() << ". " << dMin.getFullText(); return temBAD_AMOUNT; } if (dMin > saDstAmount) { - j_.trace << "Malformed transaction: Dst amount less than " << + JLOG(j.trace) << "Malformed transaction: Dst amount less than " << jss::DeliverMin.c_str() << ". " << dMin.getFullText(); return temBAD_AMOUNT; } } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } TER Payment::doApply () { + bool const deliverMin = tx().isFieldPresent(sfDeliverMin); + + if (deliverMin) + { + if (! allowDeliverMin(view())) + return temMALFORMED; + } + // Ripple if source or destination is non-native or if there are paths. - std::uint32_t const uTxFlags = mTxn.getFlags (); + std::uint32_t const uTxFlags = tx().getFlags (); bool const partialPaymentAllowed = uTxFlags & tfPartialPayment; bool const limitQuality = uTxFlags & tfLimitQuality; bool const defaultPathsAllowed = !(uTxFlags & tfNoRippleDirect); - bool const bPaths = mTxn.isFieldPresent (sfPaths); - bool const bMax = mTxn.isFieldPresent (sfSendMax); - bool const deliverMin = mTxn.isFieldPresent(sfDeliverMin) && - allowDeliverMin(view()); + bool const bPaths = tx().isFieldPresent (sfPaths); + bool const bMax = tx().isFieldPresent (sfSendMax); - AccountID const uDstAccountID (mTxn.getAccountID (sfDestination)); - STAmount const saDstAmount (mTxn.getFieldAmount (sfAmount)); + AccountID const uDstAccountID (tx().getAccountID (sfDestination)); + STAmount const saDstAmount (tx().getFieldAmount (sfAmount)); STAmount maxSourceAmount; if (bMax) - maxSourceAmount = mTxn.getFieldAmount (sfSendMax); + maxSourceAmount = tx().getFieldAmount (sfSendMax); else if (saDstAmount.native ()) maxSourceAmount = saDstAmount; else @@ -269,7 +276,7 @@ Payment::doApply () view().insert(sleDst); } else if ((sleDst->getFlags () & lsfRequireDestTag) && - !mTxn.isFieldPresent (sfDestinationTag)) + !tx().isFieldPresent (sfDestinationTag)) { // The tag is basically account-specific information we don't // understand, but we can require someone to fill it in. @@ -299,7 +306,7 @@ Payment::doApply () // transitive balances. // Copy paths into an editable class. - STPathSet spsPaths = mTxn.getFieldPathSet (sfPaths); + STPathSet spsPaths = tx().getFieldPathSet (sfPaths); try { @@ -345,7 +352,7 @@ Payment::doApply () rc.actualAmountOut != saDstAmount) { if (deliverMin && rc.actualAmountOut < - mTxn.getFieldAmount (sfDeliverMin)) + tx().getFieldAmount (sfDeliverMin)) rc.setResult (tecPATH_PARTIAL); else ctx_.deliver (rc.actualAmountOut); @@ -383,7 +390,7 @@ Payment::doApply () // mPriorBalance is the balance on the sending account BEFORE the // fees were charged. We want to make sure we have enough reserve // to send. Allow final spend to use reserve for fee. - auto const mmm = std::max(mTxn.getTransactionFee (), + auto const mmm = std::max(tx().getTransactionFee (), STAmount (uReserve)); if (mPriorBalance < saDstAmount + mmm) diff --git a/src/ripple/app/tx/impl/Payment.h b/src/ripple/app/tx/impl/Payment.h index 9f0ea2c86..86eb2f8fb 100644 --- a/src/ripple/app/tx/impl/Payment.h +++ b/src/ripple/app/tx/impl/Payment.h @@ -39,14 +39,15 @@ class Payment static std::size_t const MaxPathLength = 8; public: - template - Payment (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + Payment (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck () override; + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index e4eb31331..c09ec654d 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -29,22 +29,25 @@ namespace ripple { TER -SetAccount::preCheck () +SetAccount::preflight (PreflightContext const& ctx) { - std::uint32_t const uTxFlags = mTxn.getFlags (); + auto& tx = ctx.tx; + auto& j = ctx.j; + + std::uint32_t const uTxFlags = tx.getFlags (); if (uTxFlags & tfAccountSetMask) { - j_.trace << "Malformed transaction: Invalid flags set."; + JLOG(j.trace) << "Malformed transaction: Invalid flags set."; return temINVALID_FLAG; } - std::uint32_t const uSetFlag = mTxn.getFieldU32 (sfSetFlag); - std::uint32_t const uClearFlag = mTxn.getFieldU32 (sfClearFlag); + std::uint32_t const uSetFlag = tx.getFieldU32 (sfSetFlag); + std::uint32_t const uClearFlag = tx.getFieldU32 (sfClearFlag); if ((uSetFlag != 0) && (uSetFlag == uClearFlag)) { - j_.trace << "Malformed transaction: Set and clear same flag."; + JLOG(j.trace) << "Malformed transaction: Set and clear same flag."; return temINVALID_FLAG; } @@ -56,7 +59,7 @@ SetAccount::preCheck () if (bSetRequireAuth && bClearRequireAuth) { - j_.trace << "Malformed transaction: Contradictory flags set."; + JLOG(j.trace) << "Malformed transaction: Contradictory flags set."; return temINVALID_FLAG; } @@ -68,7 +71,7 @@ SetAccount::preCheck () if (bSetRequireDest && bClearRequireDest) { - j_.trace << "Malformed transaction: Contradictory flags set."; + JLOG(j.trace) << "Malformed transaction: Contradictory flags set."; return temINVALID_FLAG; } @@ -80,29 +83,29 @@ SetAccount::preCheck () if (bSetDisallowXRP && bClearDisallowXRP) { - j_.trace << "Malformed transaction: Contradictory flags set."; + JLOG(j.trace) << "Malformed transaction: Contradictory flags set."; return temINVALID_FLAG; } // TransferRate - if (mTxn.isFieldPresent (sfTransferRate)) + if (tx.isFieldPresent (sfTransferRate)) { - std::uint32_t uRate = mTxn.getFieldU32 (sfTransferRate); + std::uint32_t uRate = tx.getFieldU32 (sfTransferRate); if (uRate && (uRate < QUALITY_ONE)) { - j_.trace << "Malformed transaction: Bad transfer rate."; + JLOG(j.trace) << "Malformed transaction: Bad transfer rate."; return temBAD_TRANSFER_RATE; } } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } TER SetAccount::doApply () { - std::uint32_t const uTxFlags = mTxn.getFlags (); + std::uint32_t const uTxFlags = tx().getFlags (); auto const sle = view().peek( keylet::account(account_)); @@ -110,8 +113,8 @@ SetAccount::doApply () std::uint32_t const uFlagsIn = sle->getFieldU32 (sfFlags); std::uint32_t uFlagsOut = uFlagsIn; - std::uint32_t const uSetFlag = mTxn.getFieldU32 (sfSetFlag); - std::uint32_t const uClearFlag = mTxn.getFieldU32 (sfClearFlag); + std::uint32_t const uSetFlag = tx().getFieldU32 (sfSetFlag); + std::uint32_t const uClearFlag = tx().getFieldU32 (sfClearFlag); // legacy AccountSet flags bool bSetRequireDest = (uTxFlags & TxFlag::requireDestTag) || (uSetFlag == asfRequireDest); @@ -259,9 +262,9 @@ SetAccount::doApply () // // EmailHash // - if (mTxn.isFieldPresent (sfEmailHash)) + if (tx().isFieldPresent (sfEmailHash)) { - uint128 const uHash = mTxn.getFieldH128 (sfEmailHash); + uint128 const uHash = tx().getFieldH128 (sfEmailHash); if (!uHash) { @@ -278,9 +281,9 @@ SetAccount::doApply () // // WalletLocator // - if (mTxn.isFieldPresent (sfWalletLocator)) + if (tx().isFieldPresent (sfWalletLocator)) { - uint256 const uHash = mTxn.getFieldH256 (sfWalletLocator); + uint256 const uHash = tx().getFieldH256 (sfWalletLocator); if (!uHash) { @@ -297,9 +300,9 @@ SetAccount::doApply () // // MessageKey // - if (mTxn.isFieldPresent (sfMessageKey)) + if (tx().isFieldPresent (sfMessageKey)) { - Blob const messageKey = mTxn.getFieldVL (sfMessageKey); + Blob const messageKey = tx().getFieldVL (sfMessageKey); if (messageKey.size () > PUBLIC_BYTES_MAX) { @@ -322,9 +325,9 @@ SetAccount::doApply () // // Domain // - if (mTxn.isFieldPresent (sfDomain)) + if (tx().isFieldPresent (sfDomain)) { - Blob const domain = mTxn.getFieldVL (sfDomain); + Blob const domain = tx().getFieldVL (sfDomain); if (domain.size () > DOMAIN_BYTES_MAX) { @@ -347,9 +350,9 @@ SetAccount::doApply () // // TransferRate // - if (mTxn.isFieldPresent (sfTransferRate)) + if (tx().isFieldPresent (sfTransferRate)) { - std::uint32_t uRate = mTxn.getFieldU32 (sfTransferRate); + std::uint32_t uRate = tx().getFieldU32 (sfTransferRate); if (uRate == 0 || uRate == QUALITY_ONE) { diff --git a/src/ripple/app/tx/impl/SetAccount.h b/src/ripple/app/tx/impl/SetAccount.h index e90e9dd6d..292f3fac7 100644 --- a/src/ripple/app/tx/impl/SetAccount.h +++ b/src/ripple/app/tx/impl/SetAccount.h @@ -36,14 +36,15 @@ class SetAccount static std::size_t const PUBLIC_BYTES_MAX = 33; public: - template - SetAccount (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + SetAccount (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck () override; + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/SetRegularKey.cpp b/src/ripple/app/tx/impl/SetRegularKey.cpp index 89b3d3e69..49d2525d6 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.cpp +++ b/src/ripple/app/tx/impl/SetRegularKey.cpp @@ -42,19 +42,19 @@ SetRegularKey::calculateBaseFee () } TER -SetRegularKey::preCheck () +SetRegularKey::preflight (PreflightContext const& ctx) { - std::uint32_t const uTxFlags = mTxn.getFlags (); + std::uint32_t const uTxFlags = ctx.tx.getFlags (); if (uTxFlags & tfUniversalMask) { - if (j_.trace) j_.trace << + JLOG(ctx.j.trace) << "Malformed transaction: Invalid flags set."; return temINVALID_FLAG; } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } TER @@ -66,10 +66,10 @@ SetRegularKey::doApply () if (mFeeDue == zero) sle->setFlag (lsfPasswordSpent); - if (mTxn.isFieldPresent (sfRegularKey)) + if (tx().isFieldPresent (sfRegularKey)) { sle->setAccountID (sfRegularKey, - mTxn.getAccountID (sfRegularKey)); + tx().getAccountID (sfRegularKey)); } else { diff --git a/src/ripple/app/tx/impl/SetRegularKey.h b/src/ripple/app/tx/impl/SetRegularKey.h index 7d70f2980..67b897865 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.h +++ b/src/ripple/app/tx/impl/SetRegularKey.h @@ -33,14 +33,15 @@ class SetRegularKey std::uint64_t calculateBaseFee () override; public: - template - SetRegularKey (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + SetRegularKey (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck () override; + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index dfeb360d9..b64403075 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -32,6 +32,79 @@ namespace ripple { +std::tuple, + SetSignerList::Operation> +SetSignerList::determineOperation(STTx const& tx, + ApplyFlags flags, beast::Journal j) +{ + // Check the quorum. A non-zero quorum means we're creating or replacing + // the list. A zero quorum means we're destroying the list. + auto const quorum = tx.getFieldU32(sfSignerQuorum); + std::vector sign; + Operation op = unknown; + + bool const hasSignerEntries(tx.isFieldPresent(sfSignerEntries)); + if (quorum && hasSignerEntries) + { + SignerEntries::Decoded signers( + SignerEntries::deserialize(tx, j, "transaction")); + + if (signers.ter != tesSUCCESS) + return std::make_tuple(signers.ter, + quorum, sign, op); + + std::sort(signers.vec.begin(), signers.vec.end()); + + // Save deserialized list for later. + sign = std::move(signers.vec); + op = set; + } + else if ((quorum == 0) && !hasSignerEntries) + { + op = destroy; + } + + return std::make_tuple(tesSUCCESS, + quorum, sign, op); +} + +TER +SetSignerList::preflight (PreflightContext const& ctx) +{ +#if ! RIPPLE_ENABLE_MULTI_SIGN + if (! (ctx.flags & tapENABLE_TESTING)) + return temDISABLED; +#endif + + auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j); + if (std::get<0>(result) != tesSUCCESS) + return std::get<0>(result); + + if (std::get<3>(result) == unknown) + { + // Neither a set nor a destroy. Malformed. + JLOG(ctx.j.trace) << + "Malformed transaction: Invalid signer set list format."; + return temMALFORMED; + } + + if (std::get<3>(result) == set) + { + // Validate our settings. + auto const account = ctx.tx.getAccountID(sfAccount); + TER const ter = + validateQuorumAndSignerEntries(std::get<1>(result), + std::get<2>(result), account, ctx.j); + if (ter != tesSUCCESS) + { + return ter; + } + } + + return Transactor::preflight(ctx); +} + TER SetSignerList::doApply () { @@ -39,7 +112,7 @@ SetSignerList::doApply () // to our handlers. uint256 const index = getSignerListIndex (account_); - // Perform the operation preCheck() decided on. + // Perform the operation preCompute() decided on. switch (do_) { case set: @@ -56,60 +129,27 @@ SetSignerList::doApply () return temMALFORMED; } -TER -SetSignerList::preCheck() +void +SetSignerList::preCompute() { -#if ! RIPPLE_ENABLE_MULTI_SIGN - if (! (view().flags() & tapENABLE_TESTING)) - return temDISABLED; -#endif + // Get the quorum and operation info. + auto result = determineOperation(tx(), view().flags(), j_); + assert(std::get<0>(result) == tesSUCCESS); + assert(std::get<3>(result) != unknown); - // We need the account ID later, so do this check first. - preCheckAccount (); + quorum_ = std::get<1>(result); + signers_ = std::get<2>(result); + do_ = std::get<3>(result); - // Check the quorum. A non-zero quorum means we're creating or replacing - // the list. A zero quorum means we're destroying the list. - quorum_ = (mTxn.getFieldU32 (sfSignerQuorum)); - - bool const hasSignerEntries (mTxn.isFieldPresent (sfSignerEntries)); - if (quorum_ && hasSignerEntries) - { - SignerEntries::Decoded signers ( - SignerEntries::deserialize (mTxn, j_, "transaction")); - - if (signers.ter != tesSUCCESS) - return signers.ter; - - // Validate our settings. - if (TER const ter = - validateQuorumAndSignerEntries (quorum_, signers.vec)) - { - return ter; - } - - // Save deserialized and validated list for later. - signers_ = std::move (signers.vec); - do_ = set; - } - else if ((quorum_ == 0) && !hasSignerEntries) - { - do_ = destroy; - } - else - { - // Neither a set nor a destroy. Malformed. - if (j_.trace) j_.trace << - "Malformed transaction: Invalid signer set list format."; - return temMALFORMED; - } - - return preCheckSigningKey (); + return Transactor::preCompute(); } TER SetSignerList::validateQuorumAndSignerEntries ( std::uint32_t quorum, - std::vector& signers) const + std::vector const& signers, + AccountID const& account, + beast::Journal j) { // Reject if there are too many or too few entries in the list. { @@ -117,18 +157,18 @@ SetSignerList::validateQuorumAndSignerEntries ( if ((signerCount < SignerEntries::minEntries) || (signerCount > SignerEntries::maxEntries)) { - if (j_.trace) j_.trace << + if (j.trace) j.trace << "Too many or too few signers in signer list."; return temMALFORMED; } } // Make sure there are no duplicate signers. - std::sort (signers.begin (), signers.end ()); + assert(std::is_sorted(signers.begin(), signers.end())); if (std::adjacent_find ( signers.begin (), signers.end ()) != signers.end ()) { - if (j_.trace) j_.trace << + if (j.trace) j.trace << "Duplicate signers in signer list"; return temBAD_SIGNER; } @@ -140,9 +180,9 @@ SetSignerList::validateQuorumAndSignerEntries ( { allSignersWeight += signer.weight; - if (signer.account == account_) + if (signer.account == account) { - if (j_.trace) j_.trace << + if (j.trace) j.trace << "A signer may not self reference account."; return temBAD_SIGNER; } @@ -152,7 +192,7 @@ SetSignerList::validateQuorumAndSignerEntries ( } if ((quorum <= 0) || (allSignersWeight < quorum)) { - if (j_.trace) j_.trace << + if (j.trace) j.trace << "Quorum is unreachable"; return temBAD_QUORUM; } diff --git a/src/ripple/app/tx/impl/SetSignerList.h b/src/ripple/app/tx/impl/SetSignerList.h index 933fe5acb..7b458a713 100644 --- a/src/ripple/app/tx/impl/SetSignerList.h +++ b/src/ripple/app/tx/impl/SetSignerList.h @@ -42,28 +42,39 @@ this class implements. class SetSignerList : public Transactor { private: - // Values determined during preCheck for use later. + // Values determined during preCompute for use later. enum Operation {unknown, set, destroy}; Operation do_ {unknown}; std::uint32_t quorum_ {0}; std::vector signers_; public: - template - SetSignerList (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + SetSignerList (ApplyContext& ctx) + : Transactor(ctx) { } + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; - TER preCheck () override; + void preCompute() override; private: - // `signers` is sorted on return + static + std::tuple, + Operation> + determineOperation(STTx const& tx, + ApplyFlags flags, beast::Journal j); + + static TER validateQuorumAndSignerEntries ( std::uint32_t quorum, - std::vector& signers) const; + std::vector const& signers, + AccountID const& account, + beast::Journal j); TER replaceSignerList (uint256 const& index); TER destroySignerList (uint256 const& index); diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 4dfe9804b..a2f26a0a4 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -28,25 +28,28 @@ namespace ripple { TER -SetTrust::preCheck () +SetTrust::preflight (PreflightContext const& ctx) { - std::uint32_t const uTxFlags = mTxn.getFlags (); + auto& tx = ctx.tx; + auto& j = ctx.j; + + std::uint32_t const uTxFlags = tx.getFlags (); if (uTxFlags & tfTrustSetMask) { - j_.trace << + JLOG(j.trace) << "Malformed transaction: Invalid flags set."; return temINVALID_FLAG; } - STAmount const saLimitAmount (mTxn.getFieldAmount (sfLimitAmount)); + STAmount const saLimitAmount (tx.getFieldAmount (sfLimitAmount)); if (!isLegalNet (saLimitAmount)) return temBAD_AMOUNT; if (saLimitAmount.native ()) { - if (j_.trace) j_.trace << + JLOG(j.trace) << "Malformed transaction: specifies native limit " << saLimitAmount.getFullText (); return temBAD_LIMIT; @@ -54,14 +57,14 @@ SetTrust::preCheck () if (badCurrency() == saLimitAmount.getCurrency ()) { - if (j_.trace) j_.trace << + JLOG(j.trace) << "Malformed transaction: specifies XRP as IOU"; return temBAD_CURRENCY; } if (saLimitAmount < zero) { - if (j_.trace) j_.trace << + JLOG(j.trace) << "Malformed transaction: Negative credit limit."; return temBAD_LIMIT; } @@ -71,12 +74,12 @@ SetTrust::preCheck () if (!issuer || issuer == noAccount()) { - if (j_.trace) j_.trace << + JLOG(j.trace) << "Malformed transaction: no destination account."; return temDST_NEEDED; } - return Transactor::preCheck (); + return Transactor::preflight(ctx); } TER @@ -84,9 +87,9 @@ SetTrust::doApply () { TER terResult = tesSUCCESS; - STAmount const saLimitAmount (mTxn.getFieldAmount (sfLimitAmount)); - bool const bQualityIn (mTxn.isFieldPresent (sfQualityIn)); - bool const bQualityOut (mTxn.isFieldPresent (sfQualityOut)); + STAmount const saLimitAmount (tx().getFieldAmount (sfLimitAmount)); + bool const bQualityIn (tx().isFieldPresent (sfQualityIn)); + bool const bQualityOut (tx().isFieldPresent (sfQualityOut)); Currency const currency (saLimitAmount.getCurrency ()); AccountID uDstAccountID (saLimitAmount.getIssuer ()); @@ -115,13 +118,13 @@ SetTrust::doApply () ? 0 : view().fees().accountReserve(uOwnerCount + 1)); - std::uint32_t uQualityIn (bQualityIn ? mTxn.getFieldU32 (sfQualityIn) : 0); - std::uint32_t uQualityOut (bQualityOut ? mTxn.getFieldU32 (sfQualityOut) : 0); + std::uint32_t uQualityIn (bQualityIn ? tx().getFieldU32 (sfQualityIn) : 0); + std::uint32_t uQualityOut (bQualityOut ? tx().getFieldU32 (sfQualityOut) : 0); if (bQualityOut && QUALITY_ONE == uQualityOut) uQualityOut = 0; - std::uint32_t const uTxFlags = mTxn.getFlags (); + std::uint32_t const uTxFlags = tx().getFlags (); bool const bSetAuth = (uTxFlags & tfSetfAuth); bool const bSetNoRipple = (uTxFlags & tfSetNoRipple); diff --git a/src/ripple/app/tx/impl/SetTrust.h b/src/ripple/app/tx/impl/SetTrust.h index 265117dce..6503b1682 100644 --- a/src/ripple/app/tx/impl/SetTrust.h +++ b/src/ripple/app/tx/impl/SetTrust.h @@ -32,14 +32,15 @@ class SetTrust : public Transactor { public: - template - SetTrust (Args&&... args) - : Transactor(std::forward< - Args>(args)...) + SetTrust (ApplyContext& ctx) + : Transactor(ctx) { } - TER preCheck () override; + static + TER + preflight (PreflightContext const& ctx); + TER doApply () override; }; diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index b6f1e3746..8d3196317 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -30,10 +30,56 @@ namespace ripple { +TER +Transactor::preflight (PreflightContext const& ctx) +{ + auto& tx = ctx.tx; + auto& j = ctx.j; + + auto const id = tx.getAccountID(sfAccount); + if (id == zero) + { + JLOG(j.warning) << "Transactor::preflight: bad account id"; + return temBAD_SRC_ACCOUNT; + } + + // Extract signing key + // Transactions contain a signing key. This allows us to trivially verify a + // transaction has at least been properly signed without going to disk. + // Each transaction also notes a source account id. This is used to verify + // that the signing key is associated with the account. + // XXX This could be a lot cleaner to prevent unnecessary copying. + auto const pk = + RippleAddress::createAccountPublic( + tx.getSigningPubKey()); + if (!tx.isKnownGood ()) + { + if (tx.isKnownBad () || + (! (ctx.flags & tapNO_CHECK_SIGN) && !tx.checkSign( + ( +#if RIPPLE_ENABLE_MULTI_SIGN + true +#else + ctx.flags & tapENABLE_TESTING +#endif + )))) + { + tx.setBad(); + j.debug << "apply: Invalid transaction (bad signature)"; + return temINVALID; + } + + tx.setGood(); + } + + return tesSUCCESS; +} + +//------------------------------------------------------------------------------ + Transactor::Transactor( ApplyContext& ctx) - : mTxn (ctx.tx) - , ctx_ (ctx) + : ctx_ (ctx) , j_ (ctx.journal) , mHasAuthKey (false) , mSigMaster (false) @@ -55,7 +101,7 @@ std::uint64_t Transactor::calculateBaseFee () TER Transactor::payFee () { - STAmount saPaid = mTxn.getTransactionFee (); + STAmount saPaid = tx().getTransactionFee (); if (!isLegalNet (saPaid)) return temBAD_AMOUNT; @@ -111,7 +157,7 @@ TER Transactor::checkSeq () auto const sle = view().peek( keylet::account(account_)); - std::uint32_t const t_seq = mTxn.getSequence (); + std::uint32_t const t_seq = tx().getSequence (); std::uint32_t const a_seq = sle->getFieldU32 (sfSequence); if (t_seq != a_seq) @@ -124,7 +170,7 @@ TER Transactor::checkSeq () return terPRE_SEQ; } - if (view().txExists(mTxn.getTransactionID ())) + if (view().txExists(tx().getTransactionID ())) return tefALREADY; j_.trace << "applyTransaction: has past sequence number " << @@ -132,91 +178,41 @@ TER Transactor::checkSeq () return tefPAST_SEQ; } - if (mTxn.isFieldPresent (sfAccountTxnID) && - (sle->getFieldH256 (sfAccountTxnID) != mTxn.getFieldH256 (sfAccountTxnID))) + if (tx().isFieldPresent (sfAccountTxnID) && + (sle->getFieldH256 (sfAccountTxnID) != tx().getFieldH256 (sfAccountTxnID))) return tefWRONG_PRIOR; - if (mTxn.isFieldPresent (sfLastLedgerSequence) && - (view().seq() > mTxn.getFieldU32 (sfLastLedgerSequence))) + if (tx().isFieldPresent (sfLastLedgerSequence) && + (view().seq() > tx().getFieldU32 (sfLastLedgerSequence))) return tefMAX_LEDGER; sle->setFieldU32 (sfSequence, t_seq + 1); if (sle->isFieldPresent (sfAccountTxnID)) - sle->setFieldH256 (sfAccountTxnID, mTxn.getTransactionID ()); + sle->setFieldH256 (sfAccountTxnID, tx().getTransactionID ()); return tesSUCCESS; } // check stuff before you bother to lock the ledger -TER Transactor::preCheck () +void Transactor::preCompute () { - TER result = preCheckAccount (); - if (result != tesSUCCESS) - return result; - - return preCheckSigningKey (); -} - -TER Transactor::preCheckAccount () -{ - account_ = mTxn.getAccountID(sfAccount); - - if (!account_) - { - j_.warning << "applyTransaction: bad transaction source id"; - return temBAD_SRC_ACCOUNT; - } - return tesSUCCESS; -} - -TER Transactor::preCheckSigningKey () -{ - // Extract signing key - // Transactions contain a signing key. This allows us to trivially verify a - // transaction has at least been properly signed without going to disk. - // Each transaction also notes a source account id. This is used to verify - // that the signing key is associated with the account. - // XXX This could be a lot cleaner to prevent unnecessary copying. + account_ = tx().getAccountID(sfAccount); + assert(account_ != zero); mSigningPubKey = - RippleAddress::createAccountPublic (mTxn.getSigningPubKey ()); - - // Consistency: really signed. - if (!mTxn.isKnownGood ()) - { - if (mTxn.isKnownBad () || - (!(view().flags() & tapNO_CHECK_SIGN) && !mTxn.checkSign( - ( -#if RIPPLE_ENABLE_MULTI_SIGN - true -#else - view().flags() & tapENABLE_TESTING -#endif - )))) - { - mTxn.setBad (); - j_.debug << "apply: Invalid transaction (bad signature)"; - return temINVALID; - } - - mTxn.setGood (); - } - - return tesSUCCESS; + RippleAddress::createAccountPublic( + tx().getSigningPubKey()); } TER Transactor::apply () { // No point in going any further if the transaction fee is malformed. - STAmount const saTxnFee = mTxn.getTransactionFee (); + STAmount const saTxnFee = tx().getTransactionFee (); if (!saTxnFee.native () || saTxnFee.negative () || !isLegalNet (saTxnFee)) return temBAD_FEE; - TER terResult = preCheck (); - - if (terResult != tesSUCCESS) - return terResult; + preCompute(); // Find source account auto const sle = view().peek (keylet::account(account_)); @@ -229,9 +225,9 @@ TER Transactor::apply () { if (mustHaveValidAccount ()) { - j_.trace << + JLOG(j_.trace) << "applyTransaction: delay: source account does not exist " << - toBase58(mTxn.getAccountID(sfAccount)); + toBase58(tx().getAccountID(sfAccount)); return terNO_ACCOUNT; } } @@ -242,17 +238,17 @@ TER Transactor::apply () mHasAuthKey = sle->isFieldPresent (sfRegularKey); } - terResult = checkSeq (); + auto terResult = checkSeq (); - if (terResult != tesSUCCESS) return (terResult); + if (terResult != tesSUCCESS) return terResult; terResult = payFee (); - if (terResult != tesSUCCESS) return (terResult); + if (terResult != tesSUCCESS) return terResult; terResult = checkSign (); - if (terResult != tesSUCCESS) return (terResult); + if (terResult != tesSUCCESS) return terResult; if (sle) view().update (sle); @@ -514,7 +510,7 @@ TER Transactor::checkMultiSign () return outer.ter; // Get the actual array of transaction signers. - STArray const& multiSigners (mTxn.getFieldArray (sfMultiSigners)); + STArray const& multiSigners (tx().getFieldArray (sfMultiSigners)); // Walk the accountSigners performing a variety of checks and see if // the quorum is met. @@ -650,7 +646,7 @@ Transactor::operator()() JLOG(j_.trace) << "applyTransaction>"; - uint256 const& txID = mTxn.getTransactionID (); + uint256 const& txID = tx().getTransactionID (); if (!txID) { @@ -665,15 +661,15 @@ Transactor::operator()() #ifdef BEAST_DEBUG { Serializer ser; - mTxn.add (ser); + tx().add (ser); SerialIter sit(ser.slice()); STTx s2 (sit); - if (! s2.isEquivalent(mTxn)) + if (! s2.isEquivalent(tx())) { JLOG(j_.fatal) << "Transaction serdes mismatch"; - JLOG(j_.info) << to_string(mTxn.getJson (0)); + JLOG(j_.info) << to_string(tx().getJson (0)); JLOG(j_.fatal) << s2.getJson (0); assert (false); } @@ -716,11 +712,11 @@ Transactor::operator()() ctx_.discard(); auto const txnAcct = view().peek( - keylet::account(mTxn.getAccountID(sfAccount))); + keylet::account(tx().getAccountID(sfAccount))); if (txnAcct) { - std::uint32_t t_seq = mTxn.getSequence (); + std::uint32_t t_seq = tx().getSequence (); std::uint32_t a_seq = txnAcct->getFieldU32 (sfSequence); if (a_seq < t_seq) @@ -729,7 +725,7 @@ Transactor::operator()() terResult = tefPAST_SEQ; else { - STAmount fee = mTxn.getTransactionFee (); + STAmount fee = tx().getTransactionFee (); STAmount balance = txnAcct->getFieldAmount (sfBalance); // We retry/reject the transaction if the account @@ -774,7 +770,7 @@ Transactor::operator()() // encapsulation of STAmount here and use "special // knowledge" - namely that a native amount is // stored fully in the mantissa: - auto const fee = mTxn.getTransactionFee (); + auto const fee = tx().getTransactionFee (); // The transactor guarantees these will never trigger if (!fee.native () || fee.negative ()) diff --git a/src/ripple/app/tx/impl/Transactor.h b/src/ripple/app/tx/impl/Transactor.h index 5e476b7b7..08bb1b16b 100644 --- a/src/ripple/app/tx/impl/Transactor.h +++ b/src/ripple/app/tx/impl/Transactor.h @@ -25,10 +25,32 @@ namespace ripple { +/** State information when preflighting a tx. */ +struct PreflightContext +{ +public: + explicit PreflightContext(STTx const& tx_, + Rules const& rules_, ApplyFlags flags_, + //SigVerify verify_, + beast::Journal j_ = {}) + : tx(tx_) + , rules(rules_) + , flags(flags_) + //, verify(verify_) + , j(j_) + { + } + + STTx const& tx; + Rules const& rules; + ApplyFlags flags; + //SigVerify verify; + beast::Journal j; +}; + class Transactor { protected: - STTx const& mTxn; ApplyContext& ctx_; beast::Journal j_; @@ -57,6 +79,16 @@ public: return ctx_.view(); } + STTx const& + tx() const + { + return ctx_.tx; + } + + static + TER + preflight (PreflightContext const& ctx); + protected: TER apply(); @@ -64,8 +96,6 @@ protected: explicit Transactor (ApplyContext& ctx); - TER preCheckAccount (); - TER preCheckSigningKey (); void calculateFee (); // VFALCO This is the equivalent of dynamic_cast @@ -81,7 +111,7 @@ protected: // Returns the fee, not scaled for load (Should be in fee units. FIXME) virtual std::uint64_t calculateBaseFee (); - virtual TER preCheck (); + virtual void preCompute(); virtual TER checkSeq (); virtual TER payFee (); virtual TER checkSign (); diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index e20532b90..eb44c1d9b 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -20,7 +20,6 @@ #include #include #include - #include #include #include @@ -34,68 +33,112 @@ namespace ripple { -template static -std::pair -do_apply (Args&&... args) +TER +invoke_preflight (PreflightContext const& ctx) { - ApplyContext ctx ( - std::forward(args)...); - Processor p(ctx); - return p(); -} - -template -static -std::pair -invoke (TxType type, - Args&&... args) -{ - switch(type) + switch(ctx.tx.getTxnType()) { - case ttACCOUNT_SET: return do_apply< SetAccount >(std::forward(args)...); - case ttOFFER_CANCEL: return do_apply< CancelOffer >(std::forward(args)...); - case ttOFFER_CREATE: return do_apply< CreateOffer >(std::forward(args)...); - case ttPAYMENT: return do_apply< Payment >(std::forward(args)...); - case ttREGULAR_KEY_SET: return do_apply< SetRegularKey >(std::forward(args)...); - case ttSIGNER_LIST_SET: return do_apply< SetSignerList >(std::forward(args)...); - case ttTICKET_CANCEL: return do_apply< CancelTicket >(std::forward(args)...); - case ttTICKET_CREATE: return do_apply< CreateTicket >(std::forward(args)...); - case ttTRUST_SET: return do_apply< SetTrust >(std::forward(args)...); - - // VFALCO These are both the same? + case ttACCOUNT_SET: return SetAccount ::preflight(ctx); + case ttOFFER_CANCEL: return CancelOffer ::preflight(ctx); + case ttOFFER_CREATE: return CreateOffer ::preflight(ctx); + case ttPAYMENT: return Payment ::preflight(ctx); + case ttREGULAR_KEY_SET: return SetRegularKey ::preflight(ctx); + case ttSIGNER_LIST_SET: return SetSignerList ::preflight(ctx); + case ttTICKET_CANCEL: return CancelTicket ::preflight(ctx); + case ttTICKET_CREATE: return CreateTicket ::preflight(ctx); + case ttTRUST_SET: return SetTrust ::preflight(ctx); case ttAMENDMENT: - case ttFEE: return do_apply< Change >(std::forward(args)...); + case ttFEE: return Change ::preflight(ctx); default: - break; + return temUNKNOWN; } - return { temUNKNOWN, false }; } +static std::pair -apply (OpenView& view, - STTx const& tx, ApplyFlags flags, - Config const& config, - beast::Journal j) +invoke_apply (ApplyContext& ctx) +{ + switch(ctx.tx.getTxnType()) + { + case ttACCOUNT_SET: { SetAccount p(ctx); return p(); } + case ttOFFER_CANCEL: { CancelOffer p(ctx); return p(); } + case ttOFFER_CREATE: { CreateOffer p(ctx); return p(); } + case ttPAYMENT: { Payment p(ctx); return p(); } + case ttREGULAR_KEY_SET: { SetRegularKey p(ctx); return p(); } + case ttSIGNER_LIST_SET: { SetSignerList p(ctx); return p(); } + case ttTICKET_CANCEL: { CancelTicket p(ctx); return p(); } + case ttTICKET_CREATE: { CreateTicket p(ctx); return p(); } + case ttTRUST_SET: { SetTrust p(ctx); return p(); } + case ttAMENDMENT: + case ttFEE: { Change p(ctx); return p(); } + default: + return { temUNKNOWN, false }; + } +} + +//------------------------------------------------------------------------------ + +TER +preflight (Rules const& rules, STTx const& tx, + ApplyFlags flags, + Config const& config, beast::Journal j) { try { - return invoke (tx.getTxnType(), - view, tx, flags, config, j); + PreflightContext pfctx( + tx, rules, flags, j); + return invoke_preflight(pfctx); } - catch(std::exception const& e) + catch (std::exception const& e) { JLOG(j.fatal) << - "Caught exception: " << e.what(); + "apply: " << e.what(); + return tefEXCEPTION; + } + catch (...) + { + JLOG(j.fatal) << + "apply: "; + return tefEXCEPTION; + } +} + +std::pair +doapply(OpenView& view, STTx const& tx, + ApplyFlags flags, Config const& config, + beast::Journal j) +{ + try + { + ApplyContext ctx( + view, tx, flags, config, j); + return invoke_apply(ctx); + } + catch (std::exception const& e) + { + JLOG(j.fatal) << + "apply: " << e.what(); return { tefEXCEPTION, false }; } - catch(...) + catch (...) { JLOG(j.fatal) << - "Caught unknown exception"; + "apply: "; return { tefEXCEPTION, false }; } } +std::pair +apply (OpenView& view, STTx const& tx, + ApplyFlags flags, + Config const& config, beast::Journal j) +{ + auto pfresult = preflight(view.rules(), + tx, flags, config, j); + if (pfresult != tesSUCCESS) + return { pfresult, false }; + return doapply(view, tx, flags, config, j); +} + } // ripple