diff --git a/src/ripple/app/tx/impl/CancelOffer.cpp b/src/ripple/app/tx/impl/CancelOffer.cpp index d43fe1c9fa..7673503580 100644 --- a/src/ripple/app/tx/impl/CancelOffer.cpp +++ b/src/ripple/app/tx/impl/CancelOffer.cpp @@ -29,6 +29,10 @@ namespace ripple { TER CancelOffer::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto const uTxFlags = ctx.tx.getFlags(); if (uTxFlags & tfUniversalMask) @@ -46,7 +50,7 @@ CancelOffer::preflight (PreflightContext const& ctx) return temBAD_SEQUENCE; } - return Transactor::preflight(ctx); + return preflight2(ctx); } //------------------------------------------------------------------------------ diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index 9498bf80fd..65fb41bf00 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -32,7 +32,12 @@ CancelTicket::preflight (PreflightContext const& ctx) if (! (ctx.flags & tapENABLE_TESTING)) return temDISABLED; #endif - return Transactor::preflight(ctx); + + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + + return preflight2 (ctx); } TER diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index 517c80cc9e..737cfeeb72 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -32,13 +32,19 @@ TER Change::preflight (PreflightContext const& ctx) { auto account = ctx.tx.getAccountID(sfAccount); - - if (account.isNonZero ()) + if (account != zero) { JLOG(ctx.j.warning) << "Bad source id"; return temBAD_SRC_ACCOUNT; } + auto const fee = ctx.tx.getTransactionFee (); + if (!fee.native () || fee != beast::zero) + { + JLOG(ctx.j.warning) << "Non-zero fee"; + return temBAD_FEE; + } + return tesSUCCESS; } @@ -89,7 +95,7 @@ Change::checkSeq() TER Change::payFee() { - if (tx().getTransactionFee () != STAmount ()) + if (tx().getTransactionFee () != beast::zero) { j_.warning << "Non-zero fee"; return temBAD_FEE; diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 9557a6170e..300e874c51 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -36,6 +36,10 @@ namespace ripple { TER CreateOffer::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto& tx = ctx.tx; auto& j = ctx.j; @@ -123,7 +127,7 @@ CreateOffer::preflight (PreflightContext const& ctx) return temBAD_ISSUER; } - return Transactor::preflight(ctx); + return preflight2(ctx); } TER diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index f39eb37cb8..a05a32f61c 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -33,6 +33,10 @@ CreateTicket::preflight (PreflightContext const& ctx) return temDISABLED; #endif + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + if (ctx.tx.isFieldPresent (sfExpiration)) { if (ctx.tx.getFieldU32 (sfExpiration) == 0) @@ -43,7 +47,7 @@ CreateTicket::preflight (PreflightContext const& ctx) } } - return Transactor::preflight(ctx); + return preflight2 (ctx); } STAmount diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index c323e85164..c9ceba7fca 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -41,6 +41,10 @@ allowDeliverMin (ApplyView const& view) TER Payment::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto& tx = ctx.tx; auto& j = ctx.j; @@ -187,7 +191,7 @@ Payment::preflight (PreflightContext const& ctx) } } - return Transactor::preflight(ctx); + return preflight2 (ctx); } TER diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index c09ec654dd..a998265cb4 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -31,6 +31,10 @@ namespace ripple { TER SetAccount::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto& tx = ctx.tx; auto& j = ctx.j; @@ -99,7 +103,7 @@ SetAccount::preflight (PreflightContext const& ctx) } } - return Transactor::preflight(ctx); + return preflight2(ctx); } TER diff --git a/src/ripple/app/tx/impl/SetRegularKey.cpp b/src/ripple/app/tx/impl/SetRegularKey.cpp index 49d2525d68..a9ba213373 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.cpp +++ b/src/ripple/app/tx/impl/SetRegularKey.cpp @@ -44,6 +44,10 @@ SetRegularKey::calculateBaseFee () TER SetRegularKey::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + std::uint32_t const uTxFlags = ctx.tx.getFlags (); if (uTxFlags & tfUniversalMask) @@ -54,7 +58,7 @@ SetRegularKey::preflight (PreflightContext const& ctx) return temINVALID_FLAG; } - return Transactor::preflight(ctx); + return preflight2(ctx); } TER diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 2038a7c8d5..6f838f656e 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -80,6 +80,10 @@ SetSignerList::preflight (PreflightContext const& ctx) return temDISABLED; #endif + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j); if (std::get<0>(result) != tesSUCCESS) return std::get<0>(result); @@ -105,7 +109,7 @@ SetSignerList::preflight (PreflightContext const& ctx) } } - return Transactor::preflight(ctx); + return preflight2 (ctx); } TER diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index a2f26a0a43..b676190522 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -30,6 +30,10 @@ namespace ripple { TER SetTrust::preflight (PreflightContext const& ctx) { + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + auto& tx = ctx.tx; auto& j = ctx.j; @@ -79,7 +83,7 @@ SetTrust::preflight (PreflightContext const& ctx) return temDST_NEEDED; } - return Transactor::preflight(ctx); + return preflight2 (ctx); } TER diff --git a/src/ripple/app/tx/impl/SusPay.cpp b/src/ripple/app/tx/impl/SusPay.cpp index 7db0263cb2..f3c4259aa3 100644 --- a/src/ripple/app/tx/impl/SusPay.cpp +++ b/src/ripple/app/tx/impl/SusPay.cpp @@ -109,7 +109,7 @@ namespace ripple { SusPayCancel Any account may submit a SusPayCancel transaction. - + If the SusPay ledger entry does not specify a sfCancelAfter, the cancel transaction will fail. @@ -136,6 +136,10 @@ SusPayCreate::preflight (PreflightContext const& ctx) ctx.config.features)) return temDISABLED; + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + if (! isXRP(ctx.tx[sfAmount])) return temBAD_AMOUNT; @@ -150,7 +154,7 @@ SusPayCreate::preflight (PreflightContext const& ctx) ctx.tx[sfCancelAfter] <= ctx.tx[sfFinishAfter]) return temBAD_EXPIRATION; - return Transactor::preflight(ctx); + return preflight2 (ctx); } TER @@ -187,12 +191,12 @@ SusPayCreate::doApply() auto const sle = ctx_.view().peek( keylet::account(account)); - if (XRPAmount(STAmount((*sle)[sfBalance]).mantissa()) < + if (XRPAmount(STAmount((*sle)[sfBalance]).mantissa()) < XRPAmount(ctx_.view().fees().accountReserve( (*sle)[sfOwnerCount] + 1))) return tecINSUFFICIENT_RESERVE; - if (XRPAmount(STAmount((*sle)[sfBalance]).mantissa()) < + if (XRPAmount(STAmount((*sle)[sfBalance]).mantissa()) < amount + XRPAmount(ctx_.view().fees().accountReserve( (*sle)[sfOwnerCount] + 1))) return tecUNFUNDED; @@ -239,7 +243,7 @@ SusPayCreate::doApply() (*sle)[sfBalance] = (*sle)[sfBalance] - ctx_.tx[sfAmount]; (*sle)[sfOwnerCount] = (*sle)[sfOwnerCount] + 1; ctx_.view().update(sle); - + return tesSUCCESS; } @@ -253,6 +257,10 @@ SusPayFinish::preflight (PreflightContext const& ctx) ctx.config.features)) return temDISABLED; + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + if (ctx.tx[~sfMethod]) { // Condition @@ -291,7 +299,7 @@ SusPayFinish::preflight (PreflightContext const& ctx) return temMALFORMED; } - return Transactor::preflight(ctx); + return preflight2 (ctx); } TER @@ -362,7 +370,11 @@ SusPayCancel::preflight (PreflightContext const& ctx) ctx.config.features)) return temDISABLED; - return Transactor::preflight(ctx); + auto const ret = preflight1 (ctx); + if (!isTesSuccess (ret)) + return ret; + + return preflight2 (ctx); } TER diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index fa90f65ca8..a3cd26d448 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -30,30 +30,42 @@ namespace ripple { +/** Performs early sanity checks on the account and fee fields */ TER -Transactor::preflight (PreflightContext const& ctx) +preflight1 (PreflightContext const& ctx) { - auto& tx = ctx.tx; - auto& j = ctx.j; - - auto const id = tx.getAccountID(sfAccount); + auto const id = ctx.tx.getAccountID(sfAccount); if (id == zero) { - JLOG(j.warning) << "Transactor::preflight: bad account id"; + JLOG(ctx.j.warning) << "preflight1: bad account id"; return temBAD_SRC_ACCOUNT; } + // No point in going any further if the transaction fee is malformed. + auto const fee = ctx.tx.getTransactionFee (); + if (!fee.native () || fee < beast::zero || !isLegalNet (fee)) + { + JLOG(ctx.j.debug) << "preflight1: invalid fee"; + return temBAD_FEE; + } + + return tesSUCCESS; +} + +/** Checks whether the signature appears valid */ +TER +preflight2 (PreflightContext const& ctx) +{ // 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()); + auto const pk = RippleAddress::createAccountPublic( + ctx.tx.getSigningPubKey()); - if(! ctx.verify(tx, + if(! ctx.verify(ctx.tx, [&, ctx] (STTx const& tx) { return (ctx.flags & tapNO_CHECK_SIGN) || @@ -66,7 +78,7 @@ Transactor::preflight (PreflightContext const& ctx) ); })) { - JLOG(j.debug) << "apply: Invalid transaction (bad signature)"; + JLOG(ctx.j.debug) << "preflight2: bad signature"; return temINVALID; } @@ -202,12 +214,6 @@ void Transactor::preCompute () TER Transactor::apply () { - // No point in going any further if the transaction fee is malformed. - STAmount const saTxnFee = tx().getTransactionFee (); - - if (!saTxnFee.native () || saTxnFee.negative () || !isLegalNet (saTxnFee)) - return temBAD_FEE; - preCompute(); // Find source account diff --git a/src/ripple/app/tx/impl/Transactor.h b/src/ripple/app/tx/impl/Transactor.h index 8e8a26f1fe..8f11e4f1ca 100644 --- a/src/ripple/app/tx/impl/Transactor.h +++ b/src/ripple/app/tx/impl/Transactor.h @@ -87,10 +87,6 @@ public: return ctx_.tx; } - static - TER - preflight (PreflightContext const& ctx); - protected: TER apply(); @@ -124,6 +120,14 @@ private: TER checkMultiSign (); }; +/** Performs early sanity checks on the account and fee fields */ +TER +preflight1 (PreflightContext const& ctx); + +/** Checks whether the signature appears valid */ +TER +preflight2 (PreflightContext const& ctx); + } #endif