diff --git a/src/ripple/app/transactors/CancelOffer.cpp b/src/ripple/app/transactors/CancelOffer.cpp index 7f60edace..45e0c8775 100644 --- a/src/ripple/app/transactors/CancelOffer.cpp +++ b/src/ripple/app/transactors/CancelOffer.cpp @@ -42,50 +42,52 @@ public: } - TER doApply () override + TER preCheck () override { - std::uint32_t const uOfferSequence = mTxn.getFieldU32 (sfOfferSequence); - std::uint32_t const uAccountSequenceNext = mTxnAccount->getFieldU32 (sfSequence); - - m_journal.debug << - "uAccountSequenceNext=" << uAccountSequenceNext << - " uOfferSequence=" << uOfferSequence; - std::uint32_t const uTxFlags (mTxn.getFlags ()); if (uTxFlags & tfUniversalMask) { - m_journal.trace << - "Malformed transaction: Invalid flags set."; + m_journal.trace << "Malformed transaction: " << + "Invalid flags set."; return temINVALID_FLAG; } - if (!uOfferSequence || uAccountSequenceNext - 1 <= uOfferSequence) + std::uint32_t const uOfferSequence = mTxn.getFieldU32 (sfOfferSequence); + + if (!uOfferSequence) { - m_journal.trace << - "uAccountSequenceNext=" << uAccountSequenceNext << - " uOfferSequence=" << uOfferSequence; + m_journal.trace << "Malformed transaction: " << + "No sequence specified."; + return temBAD_SEQUENCE; + } + + return Transactor::preCheck (); + } + + TER doApply () override + { + std::uint32_t const uOfferSequence = mTxn.getFieldU32 (sfOfferSequence); + + if (mTxnAccount->getFieldU32 (sfSequence) - 1 <= uOfferSequence) + { + m_journal.trace << "Malformed transaction: " << + "Sequence " << uOfferSequence << " is invalid."; return temBAD_SEQUENCE; } uint256 const offerIndex (getOfferIndex (mTxnAccountID, uOfferSequence)); - SLE::pointer sleOffer (mEngine->entryCache (ltOFFER, offerIndex)); + SLE::pointer sleOffer (mEngine->entryCache (ltOFFER, + offerIndex)); if (sleOffer) { - m_journal.debug << - "OfferCancel: uOfferSequence=" << uOfferSequence; - + m_journal.debug << "Trying to cancel offer #" << uOfferSequence; return mEngine->view ().offerDelete (sleOffer); } - m_journal.warning << - "OfferCancel: offer not found: " << - to_string (mTxnAccountID) << - " : " << uOfferSequence << - " : " << to_string (offerIndex); - + m_journal.debug << "Offer #" << uOfferSequence << " can't be found."; return tesSUCCESS; } }; diff --git a/src/ripple/app/transactors/Change.cpp b/src/ripple/app/transactors/Change.cpp index 624c75c33..5e4dda2a9 100644 --- a/src/ripple/app/transactors/Change.cpp +++ b/src/ripple/app/transactors/Change.cpp @@ -100,7 +100,6 @@ public: if (mTxnAccountID.isNonZero ()) { m_journal.warning << "Bad source id"; - return temBAD_SRC_ACCOUNT; } diff --git a/src/ripple/app/transactors/CreateOffer.cpp b/src/ripple/app/transactors/CreateOffer.cpp index e1fcdc885..2201378ec 100644 --- a/src/ripple/app/transactors/CreateOffer.cpp +++ b/src/ripple/app/transactors/CreateOffer.cpp @@ -464,6 +464,96 @@ public: account->getFieldU32 (sfOwnerCount) + 1); } + TER + preCheck () override + { + std::uint32_t const uTxFlags = mTxn.getFlags (); + + if (uTxFlags & tfOfferCreateMask) + { + if (m_journal.debug) m_journal.debug << + "Malformed transaction: Invalid flags set."; + return temINVALID_FLAG; + } + + bool const bImmediateOrCancel (uTxFlags & tfImmediateOrCancel); + bool const bFillOrKill (uTxFlags & tfFillOrKill); + + if (bImmediateOrCancel && bFillOrKill) + { + if (m_journal.debug) m_journal.debug << + "Malformed transaction: both IoC and FoK set."; + return temINVALID_FLAG; + } + + bool const bHaveExpiration (mTxn.isFieldPresent (sfExpiration)); + + if (bHaveExpiration && (mTxn.getFieldU32 (sfExpiration) == 0)) + { + if (m_journal.debug) m_journal.warning << + "Malformed offer: bad expiration"; + return temBAD_EXPIRATION; + } + + bool const bHaveCancel (mTxn.isFieldPresent (sfOfferSequence)); + + if (bHaveCancel && (mTxn.getFieldU32 (sfOfferSequence) == 0)) + { + if (m_journal.debug) m_journal.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.isNative () && saTakerGets.isNative ()) + { + if (m_journal.debug) m_journal.warning << + "Malformed offer: XRP for XRP"; + return temBAD_OFFER; + } + if (saTakerPays <= zero || saTakerGets <= zero) + { + if (m_journal.debug) m_journal.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 (m_journal.debug) m_journal.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 (m_journal.debug) m_journal.warning << + "Malformed offer: Bad currency."; + return temBAD_CURRENCY; + } + + if (saTakerPays.isNative () != !uPaysIssuerID || + saTakerGets.isNative () != !uGetsIssuerID) + { + if (m_journal.warning) m_journal.warning << + "Malformed offer: bad issuer"; + return temBAD_ISSUER; + } + + return Transactor::preCheck (); + } + TER doApply () override { @@ -484,7 +574,6 @@ public: auto const& uPaysCurrency = saTakerPays.getCurrency (); auto const& uGetsIssuerID = saTakerGets.getIssuer (); - auto const& uGetsCurrency = saTakerGets.getCurrency (); bool const bHaveExpiration (mTxn.isFieldPresent (sfExpiration)); bool const bHaveCancel (mTxn.isFieldPresent (sfOfferSequence)); @@ -519,65 +608,7 @@ public: SLE::pointer sleCreator = mEngine->entryCache ( ltACCOUNT_ROOT, getAccountRootIndex (mTxnAccountID)); - if (uTxFlags & tfOfferCreateMask) - { - if (m_journal.debug) m_journal.debug << - "Malformed transaction: Invalid flags set."; - - result = temINVALID_FLAG; - } - else if (bImmediateOrCancel && bFillOrKill) - { - if (m_journal.debug) m_journal.debug << - "Malformed transaction: both IoC and FoK set."; - - result = temINVALID_FLAG; - } - else if (bHaveExpiration && !uExpiration) - { - m_journal.warning << - "Malformed offer: bad expiration"; - - result = temBAD_EXPIRATION; - } - else if (saTakerPays.isNative () && saTakerGets.isNative ()) - { - m_journal.warning << - "Malformed offer: XRP for XRP"; - - result = temBAD_OFFER; - } - else if (saTakerPays <= zero || saTakerGets <= zero) - { - m_journal.warning << - "Malformed offer: bad amount"; - - result = temBAD_OFFER; - } - else if (uPaysCurrency == uGetsCurrency && uPaysIssuerID == uGetsIssuerID) - { - m_journal.warning << - "Malformed offer: redundant offer"; - - result = temREDUNDANT; - } - // We don't allow a non-native currency to use the currency code XRP. - else if (badCurrency() == uPaysCurrency || badCurrency() == uGetsCurrency) - { - m_journal.warning << - "Malformed offer: Bad currency."; - - result = temBAD_CURRENCY; - } - else if (saTakerPays.isNative () != !uPaysIssuerID || - saTakerGets.isNative () != !uGetsIssuerID) - { - m_journal.warning << - "Malformed offer: bad issuer"; - - result = temBAD_ISSUER; - } - else if (view.isGlobalFrozen (uPaysIssuerID) || view.isGlobalFrozen (uGetsIssuerID)) + if (view.isGlobalFrozen (uPaysIssuerID) || view.isGlobalFrozen (uGetsIssuerID)) { m_journal.warning << "Offer involves frozen asset"; @@ -594,7 +625,7 @@ public: } // This can probably be simplified to make sure that you cancel sequences // before the transaction sequence number. - else if (bHaveCancel && (!uCancelSequence || uAccountSequenceNext - 1 <= uCancelSequence)) + else if (bHaveCancel && (uAccountSequenceNext - 1 <= uCancelSequence)) { if (m_journal.debug) m_journal.debug << "uAccountSequenceNext=" << uAccountSequenceNext << diff --git a/src/ripple/app/transactors/CreateTicket.cpp b/src/ripple/app/transactors/CreateTicket.cpp index 9f5fc24d7..5f3294b59 100644 --- a/src/ripple/app/transactors/CreateTicket.cpp +++ b/src/ripple/app/transactors/CreateTicket.cpp @@ -41,6 +41,22 @@ public: } + TER + preCheck () override + { + if (mTxn.isFieldPresent (sfExpiration)) + { + if (mTxn.getFieldU32 (sfExpiration) == 0) + { + m_journal.warning << + "Malformed transaction: bad expiration"; + return temBAD_EXPIRATION; + } + } + + return Transactor::preCheck (); + } + TER doApply () override { assert (mTxnAccount); @@ -60,14 +76,6 @@ public: { expiration = mTxn.getFieldU32 (sfExpiration); - if (!expiration) - { - m_journal.warning << - "Malformed ticket requestion: bad expiration"; - - return temBAD_EXPIRATION; - } - if (mEngine->getLedger ()->getParentCloseTimeNC () >= expiration) return tesSUCCESS; } @@ -104,13 +112,14 @@ public: { Ledger::ownerDirDescriber(p, b, mTxnAccountID); }; + TER result = mEngine->view ().dirAdd ( hint, getOwnerDirIndex (mTxnAccountID), sleTicket->getIndex (), describer); - m_journal.trace << + if (m_journal.trace) m_journal.trace << "Creating ticket " << to_string (sleTicket->getIndex ()) << ": " << transHuman (result); diff --git a/src/ripple/app/transactors/Payment.cpp b/src/ripple/app/transactors/Payment.cpp index e825b2013..e4504b0ce 100644 --- a/src/ripple/app/transactors/Payment.cpp +++ b/src/ripple/app/transactors/Payment.cpp @@ -50,6 +50,120 @@ public: } + TER preCheck () override + { + std::uint32_t const uTxFlags = mTxn.getFlags (); + + if (uTxFlags & tfPaymentMask) + { + m_journal.trace << "Malformed transaction: " << + "Invalid flags set."; + return temINVALID_FLAG; + } + + 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); + + STAmount const saDstAmount (mTxn.getFieldAmount (sfAmount)); + + STAmount maxSourceAmount; + + if (bMax) + maxSourceAmount = mTxn.getFieldAmount (sfSendMax); + else if (saDstAmount.isNative ()) + maxSourceAmount = saDstAmount; + else + maxSourceAmount = STAmount ( + { saDstAmount.getCurrency (), mTxnAccountID }, + saDstAmount.mantissa(), saDstAmount.exponent (), + saDstAmount < zero); + + auto const& uSrcCurrency = maxSourceAmount.getCurrency (); + auto const& uDstCurrency = saDstAmount.getCurrency (); + + // isZero() is XRP. FIX! + bool const bXRPDirect = uSrcCurrency.isZero () && uDstCurrency.isZero (); + + if (!isLegalNet (saDstAmount) || !isLegalNet (maxSourceAmount)) + return temBAD_AMOUNT; + + Account const uDstAccountID (mTxn.getFieldAccount160 (sfDestination)); + + if (!uDstAccountID) + { + m_journal.trace << "Malformed transaction: " << + "Payment destination account not specified."; + return temDST_NEEDED; + } + if (bMax && maxSourceAmount <= zero) + { + m_journal.trace << "Malformed transaction: " << + "bad max amount: " << maxSourceAmount.getFullText (); + return temBAD_AMOUNT; + } + if (saDstAmount <= zero) + { + m_journal.trace << "Malformed transaction: "<< + "bad dst amount: " << saDstAmount.getFullText (); + return temBAD_AMOUNT; + } + if (badCurrency() == uSrcCurrency || badCurrency() == uDstCurrency) + { + m_journal.trace <<"Malformed transaction: " << + "Bad currency."; + return temBAD_CURRENCY; + } + if (mTxnAccountID == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths) + { + // You're signing yourself a payment. + // If bPaths is true, you might be trying some arbitrage. + m_journal.trace << "Malformed transaction: " << + "Redundant payment from " << to_string (mTxnAccountID) << + " to self without path for " << to_string (uDstCurrency); + return temREDUNDANT; + } + if (bXRPDirect && bMax) + { + // Consistent but redundant transaction. + m_journal.trace << "Malformed transaction: " << + "SendMax specified for XRP to XRP."; + return temBAD_SEND_XRP_MAX; + } + if (bXRPDirect && bPaths) + { + // XRP is sent without paths. + m_journal.trace << "Malformed transaction: " << + "Paths specified for XRP to XRP."; + return temBAD_SEND_XRP_PATHS; + } + if (bXRPDirect && partialPaymentAllowed) + { + // Consistent but redundant transaction. + m_journal.trace << "Malformed transaction: " << + "Partial payment specified for XRP to XRP."; + return temBAD_SEND_XRP_PARTIAL; + } + if (bXRPDirect && limitQuality) + { + // Consistent but redundant transaction. + m_journal.trace << "Malformed transaction: " << + "Limit quality specified for XRP to XRP."; + return temBAD_SEND_XRP_LIMIT; + } + if (bXRPDirect && !defaultPathsAllowed) + { + // Consistent but redundant transaction. + m_journal.trace << "Malformed transaction: " << + "No ripple direct specified for XRP to XRP."; + return temBAD_SEND_XRP_NO_DIRECT; + } + + return Transactor::preCheck (); + } + TER doApply () override { // Ripple if source or destination is non-native or if there are paths. @@ -71,109 +185,11 @@ public: {saDstAmount.getCurrency (), mTxnAccountID}, saDstAmount.mantissa(), saDstAmount.exponent (), saDstAmount < zero); - auto const& uSrcCurrency = maxSourceAmount.getCurrency (); - auto const& uDstCurrency = saDstAmount.getCurrency (); - - // isZero() is XRP. FIX! - bool const bXRPDirect = uSrcCurrency.isZero () && uDstCurrency.isZero (); m_journal.trace << "maxSourceAmount=" << maxSourceAmount.getFullText () << " saDstAmount=" << saDstAmount.getFullText (); - if (!isLegalNet (saDstAmount) || !isLegalNet (maxSourceAmount)) - return temBAD_AMOUNT; - - if (uTxFlags & tfPaymentMask) - { - m_journal.trace << - "Malformed transaction: Invalid flags set."; - - return temINVALID_FLAG; - } - else if (!uDstAccountID) - { - m_journal.trace << - "Malformed transaction: Payment destination account not specified."; - - return temDST_NEEDED; - } - else if (bMax && maxSourceAmount <= zero) - { - m_journal.trace << - "Malformed transaction: bad max amount: " << maxSourceAmount.getFullText (); - - return temBAD_AMOUNT; - } - else if (saDstAmount <= zero) - { - m_journal.trace << - "Malformed transaction: bad dst amount: " << saDstAmount.getFullText (); - - return temBAD_AMOUNT; - } - else if (badCurrency() == uSrcCurrency || badCurrency() == uDstCurrency) - { - m_journal.trace << - "Malformed transaction: Bad currency."; - - return temBAD_CURRENCY; - } - else if (mTxnAccountID == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths) - { - // You're signing yourself a payment. - // If bPaths is true, you might be trying some arbitrage. - m_journal.trace << - "Malformed transaction: Redundant transaction:" << - " src=" << to_string (mTxnAccountID) << - " dst=" << to_string (uDstAccountID) << - " src_cur=" << to_string (uSrcCurrency) << - " dst_cur=" << to_string (uDstCurrency); - - return temREDUNDANT; - } - else if (bXRPDirect && bMax) - { - // Consistent but redundant transaction. - m_journal.trace << - "Malformed transaction: SendMax specified for XRP to XRP."; - - return temBAD_SEND_XRP_MAX; - } - else if (bXRPDirect && bPaths) - { - // XRP is sent without paths. - m_journal.trace << - "Malformed transaction: Paths specified for XRP to XRP."; - - return temBAD_SEND_XRP_PATHS; - } - else if (bXRPDirect && partialPaymentAllowed) - { - // Consistent but redundant transaction. - m_journal.trace << - "Malformed transaction: Partial payment specified for XRP to XRP."; - - return temBAD_SEND_XRP_PARTIAL; - } - else if (bXRPDirect && limitQuality) - { - // Consistent but redundant transaction. - m_journal.trace << - "Malformed transaction: Limit quality specified for XRP to XRP."; - - return temBAD_SEND_XRP_LIMIT; - } - else if (bXRPDirect && !defaultPathsAllowed) - { - // Consistent but redundant transaction. - m_journal.trace << - "Malformed transaction: No ripple direct specified for XRP to XRP."; - - return temBAD_SEND_XRP_NO_DIRECT; - } - - // // Open a ledger for editing. auto const index = getAccountRootIndex (uDstAccountID); SLE::pointer sleDst (mEngine->entryCache (ltACCOUNT_ROOT, index)); diff --git a/src/ripple/app/transactors/SetAccount.cpp b/src/ripple/app/transactors/SetAccount.cpp index 6ed1eb201..cc3ab8558 100644 --- a/src/ripple/app/transactors/SetAccount.cpp +++ b/src/ripple/app/transactors/SetAccount.cpp @@ -47,6 +47,76 @@ public: } + TER preCheck () override + { + std::uint32_t const uTxFlags = mTxn.getFlags (); + + if (uTxFlags & tfAccountSetMask) + { + m_journal.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); + + if ((uSetFlag != 0) && (uSetFlag == uClearFlag)) + { + m_journal.trace << "Malformed transaction: Set and clear same flag."; + return temINVALID_FLAG; + } + + // + // RequireAuth + // + bool bSetRequireAuth = (uTxFlags & tfRequireAuth) || (uSetFlag == asfRequireAuth); + bool bClearRequireAuth = (uTxFlags & tfOptionalAuth) || (uClearFlag == asfRequireAuth); + + if (bSetRequireAuth && bClearRequireAuth) + { + m_journal.trace << "Malformed transaction: Contradictory flags set."; + return temINVALID_FLAG; + } + + // + // RequireDestTag + // + bool bSetRequireDest = (uTxFlags & TxFlag::requireDestTag) || (uSetFlag == asfRequireDest); + bool bClearRequireDest = (uTxFlags & tfOptionalDestTag) || (uClearFlag == asfRequireDest); + + if (bSetRequireDest && bClearRequireDest) + { + m_journal.trace << "Malformed transaction: Contradictory flags set."; + return temINVALID_FLAG; + } + + // + // DisallowXRP + // + bool bSetDisallowXRP = (uTxFlags & tfDisallowXRP) || (uSetFlag == asfDisallowXRP); + bool bClearDisallowXRP = (uTxFlags & tfAllowXRP) || (uClearFlag == asfDisallowXRP); + + if (bSetDisallowXRP && bClearDisallowXRP) + { + m_journal.trace << "Malformed transaction: Contradictory flags set."; + return temINVALID_FLAG; + } + + // TransferRate + if (mTxn.isFieldPresent (sfTransferRate)) + { + std::uint32_t uRate = mTxn.getFieldU32 (sfTransferRate); + + if (uRate && (uRate < QUALITY_ONE)) + { + m_journal.trace << "Malformed transaction: Bad transfer rate."; + return temBAD_TRANSFER_RATE; + } + } + + return Transactor::preCheck (); + } + TER doApply () override { std::uint32_t const uTxFlags = mTxn.getFlags (); @@ -57,12 +127,6 @@ public: std::uint32_t const uSetFlag = mTxn.getFieldU32 (sfSetFlag); std::uint32_t const uClearFlag = mTxn.getFieldU32 (sfClearFlag); - if ((uSetFlag != 0) && (uSetFlag == uClearFlag)) - { - m_journal.trace << "Malformed transaction: Set and clear same flag"; - return temINVALID_FLAG; - } - // legacy AccountSet flags bool bSetRequireDest = (uTxFlags & TxFlag::requireDestTag) || (uSetFlag == asfRequireDest); bool bClearRequireDest = (uTxFlags & tfOptionalDestTag) || (uClearFlag == asfRequireDest); @@ -71,22 +135,9 @@ public: bool bSetDisallowXRP = (uTxFlags & tfDisallowXRP) || (uSetFlag == asfDisallowXRP); bool bClearDisallowXRP = (uTxFlags & tfAllowXRP) || (uClearFlag == asfDisallowXRP); - if (uTxFlags & tfAccountSetMask) - { - m_journal.trace << "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - // // RequireAuth // - - if (bSetRequireAuth && bClearRequireAuth) - { - m_journal.trace << "Malformed transaction: Contradictory flags set."; - return temINVALID_FLAG; - } - if (bSetRequireAuth && !(uFlagsIn & lsfRequireAuth)) { if (!mEngine->view().dirIsEmpty (getOwnerDirIndex (mTxnAccountID))) @@ -108,13 +159,6 @@ public: // // RequireDestTag // - - if (bSetRequireDest && bClearRequireDest) - { - m_journal.trace << "Malformed transaction: Contradictory flags set."; - return temINVALID_FLAG; - } - if (bSetRequireDest && !(uFlagsIn & lsfRequireDestTag)) { m_journal.trace << "Set lsfRequireDestTag."; @@ -130,13 +174,6 @@ public: // // DisallowXRP // - - if (bSetDisallowXRP && bClearDisallowXRP) - { - m_journal.trace << "Malformed transaction: Contradictory flags set."; - return temINVALID_FLAG; - } - if (bSetDisallowXRP && !(uFlagsIn & lsfDisallowXRP)) { m_journal.trace << "Set lsfDisallowXRP."; @@ -152,7 +189,6 @@ public: // // DisableMaster // - if ((uSetFlag == asfDisableMaster) && !(uFlagsIn & lsfDisableMaster)) { if (!mSigMaster) @@ -174,6 +210,9 @@ public: uFlagsOut &= ~lsfDisableMaster; } + // + // DefaultRipple + // if (uSetFlag == asfDefaultRipple) { uFlagsOut |= lsfDefaultRipple; @@ -183,6 +222,9 @@ public: uFlagsOut &= ~lsfDefaultRipple; } + // + // NoFreeze + // if (uSetFlag == asfNoFreeze) { if (!mSigMaster && !(uFlagsIn & lsfDisableMaster)) @@ -215,7 +257,6 @@ public: // // Track transaction IDs signed by this account in its root // - if ((uSetFlag == asfAccountTxnID) && !mTxnAccount->isFieldPresent (sfAccountTxnID)) { m_journal.trace << "Set AccountTxnID"; @@ -231,7 +272,6 @@ public: // // EmailHash // - if (mTxn.isFieldPresent (sfEmailHash)) { uint128 const uHash = mTxn.getFieldH128 (sfEmailHash); @@ -251,7 +291,6 @@ public: // // WalletLocator // - if (mTxn.isFieldPresent (sfWalletLocator)) { uint256 const uHash = mTxn.getFieldH256 (sfWalletLocator); @@ -271,7 +310,6 @@ public: // // MessageKey // - if (mTxn.isFieldPresent (sfMessageKey)) { Blob const messageKey = mTxn.getFieldVL (sfMessageKey); @@ -297,7 +335,6 @@ public: // // Domain // - if (mTxn.isFieldPresent (sfDomain)) { Blob const domain = mTxn.getFieldVL (sfDomain); @@ -323,12 +360,11 @@ public: // // TransferRate // - if (mTxn.isFieldPresent (sfTransferRate)) { std::uint32_t uRate = mTxn.getFieldU32 (sfTransferRate); - if (!uRate || uRate == QUALITY_ONE) + if (uRate == 0 || uRate == QUALITY_ONE) { m_journal.trace << "unset transfer rate"; mTxnAccount->makeFieldAbsent (sfTransferRate); @@ -338,11 +374,6 @@ public: m_journal.trace << "set transfer rate"; mTxnAccount->setFieldU32 (sfTransferRate, uRate); } - else - { - m_journal.trace << "bad transfer rate"; - return temBAD_TRANSFER_RATE; - } } if (uFlagsIn != uFlagsOut) diff --git a/src/ripple/app/transactors/SetRegularKey.cpp b/src/ripple/app/transactors/SetRegularKey.cpp index 5f2d53d99..a8267ff4c 100644 --- a/src/ripple/app/transactors/SetRegularKey.cpp +++ b/src/ripple/app/transactors/SetRegularKey.cpp @@ -54,27 +54,30 @@ public: } - TER doApply () override + TER preCheck () override { std::uint32_t const uTxFlags = mTxn.getFlags (); if (uTxFlags & tfUniversalMask) { - m_journal.trace << + if (m_journal.trace) m_journal.trace << "Malformed transaction: Invalid flags set."; return temINVALID_FLAG; } + return Transactor::preCheck (); + } + + TER doApply () override + { if (mFeeDue == zero) - { mTxnAccount->setFlag (lsfPasswordSpent); - } if (mTxn.isFieldPresent (sfRegularKey)) { - Account uAuthKeyID = mTxn.getFieldAccount160 (sfRegularKey); - mTxnAccount->setFieldAccount (sfRegularKey, uAuthKeyID); + mTxnAccount->setFieldAccount (sfRegularKey, + mTxn.getFieldAccount160 (sfRegularKey)); } else { diff --git a/src/ripple/app/transactors/SetTrust.cpp b/src/ripple/app/transactors/SetTrust.cpp index e042472f9..72fe83b37 100644 --- a/src/ripple/app/transactors/SetTrust.cpp +++ b/src/ripple/app/transactors/SetTrust.cpp @@ -42,6 +42,57 @@ public: { } + TER preCheck () override + { + std::uint32_t const uTxFlags = mTxn.getFlags (); + + if (uTxFlags & tfTrustSetMask) + { + m_journal.trace << + "Malformed transaction: Invalid flags set."; + return temINVALID_FLAG; + } + + STAmount const saLimitAmount (mTxn.getFieldAmount (sfLimitAmount)); + + if (!isLegalNet (saLimitAmount)) + return temBAD_AMOUNT; + + if (saLimitAmount.isNative ()) + { + if (m_journal.trace) m_journal.trace << + "Malformed transaction: specifies native limit " << + saLimitAmount.getFullText (); + return temBAD_LIMIT; + } + + if (badCurrency() == saLimitAmount.getCurrency ()) + { + if (m_journal.trace) m_journal.trace << + "Malformed transaction: specifies XRP as IOU"; + return temBAD_CURRENCY; + } + + if (saLimitAmount < zero) + { + if (m_journal.trace) m_journal.trace << + "Malformed transaction: Negative credit limit."; + return temBAD_LIMIT; + } + + // Check if destination makes sense. + auto const& issuer = saLimitAmount.getIssuer (); + + if (!issuer || issuer == noAccount()) + { + if (m_journal.trace) m_journal.trace << + "Malformed transaction: no destination account."; + return temDST_NEEDED; + } + + return Transactor::preCheck (); + } + TER doApply () override { TER terResult = tesSUCCESS; @@ -77,21 +128,11 @@ public: std::uint32_t uQualityIn (bQualityIn ? mTxn.getFieldU32 (sfQualityIn) : 0); std::uint32_t uQualityOut (bQualityOut ? mTxn.getFieldU32 (sfQualityOut) : 0); - if (!isLegalNet (saLimitAmount)) - return temBAD_AMOUNT; - if (bQualityOut && QUALITY_ONE == uQualityOut) uQualityOut = 0; std::uint32_t const uTxFlags = mTxn.getFlags (); - if (uTxFlags & tfTrustSetMask) - { - m_journal.trace << - "Malformed transaction: Invalid flags set."; - return temINVALID_FLAG; - } - bool const bSetAuth = (uTxFlags & tfSetfAuth); bool const bSetNoRipple = (uTxFlags & tfSetNoRipple); bool const bClearNoRipple = (uTxFlags & tfClearNoRipple); @@ -105,32 +146,12 @@ public: return tefNO_AUTH_REQUIRED; } - if (saLimitAmount.isNative ()) - { - m_journal.trace << - "Malformed transaction: Native credit limit: " << - saLimitAmount.getFullText (); - return temBAD_LIMIT; - } - - if (saLimitAmount < zero) - { - m_journal.trace << - "Malformed transaction: Negative credit limit."; - return temBAD_LIMIT; - } - - // Check if destination makes sense. - if (!uDstAccountID || uDstAccountID == noAccount()) - { - m_journal.trace << - "Malformed transaction: Destination account not specified."; - - return temDST_NEEDED; - } - if (mTxnAccountID == uDstAccountID) { + // The only purpose here is to allow a mistakenly created + // trust line to oneself to be deleted. If no such trust + // lines exist now, why not remove this code and simply + // return an error? SLE::pointer selDelete ( mEngine->entryCache (ltRIPPLE_STATE, getRippleStateIndex ( @@ -392,10 +413,6 @@ public: // Another transaction could create the account and then this transaction would succeed. terResult = tecNO_LINE_INSUF_RESERVE; } - else if (badCurrency() == currency) - { - terResult = temBAD_CURRENCY; - } else { // Zero balance in currency. diff --git a/src/ripple/app/transactors/Transactor.cpp b/src/ripple/app/transactors/Transactor.cpp index 97066cb50..0f407c4f2 100644 --- a/src/ripple/app/transactors/Transactor.cpp +++ b/src/ripple/app/transactors/Transactor.cpp @@ -257,16 +257,16 @@ TER Transactor::apply () TER terResult (preCheck ()); if (terResult != tesSUCCESS) - return (terResult); - - mTxnAccount = mEngine->entryCache (ltACCOUNT_ROOT, - getAccountRootIndex (mTxnAccountID)); - calculateFee (); + return terResult; // Find source account + mTxnAccount = mEngine->entryCache (ltACCOUNT_ROOT, + getAccountRootIndex (mTxnAccountID)); + + calculateFee (); + // If are only forwarding, due to resource limitations, we might verifying // only some transactions, this would be probabilistic. - if (!mTxnAccount) { if (mustHaveValidAccount ())