diff --git a/src/ripple/app/tx/applySteps.h b/src/ripple/app/tx/applySteps.h index 90b83d348..b57e89a71 100644 --- a/src/ripple/app/tx/applySteps.h +++ b/src/ripple/app/tx/applySteps.h @@ -47,12 +47,12 @@ public: beast::Journal const j; /// Intermediate transaction result - TER const ter; + NotTEC const ter; /// Constructor template PreflightResult(Context const& ctx_, - TER ter_) + NotTEC ter_) : tx(ctx_.tx) , rules(ctx_.rules) , flags(ctx_.flags) diff --git a/src/ripple/app/tx/impl/ApplyContext.cpp b/src/ripple/app/tx/impl/ApplyContext.cpp index c13a9c799..59f0a07e3 100644 --- a/src/ripple/app/tx/impl/ApplyContext.cpp +++ b/src/ripple/app/tx/impl/ApplyContext.cpp @@ -104,8 +104,8 @@ ApplyContext::checkInvariantsHelper(TER terResult, std::index_sequence) [](auto const& b) { return b; })) { terResult = (terResult == tecINVARIANT_FAILED) ? - tefINVARIANT_FAILED : - tecINVARIANT_FAILED ; + TER {tefINVARIANT_FAILED} : + TER {tecINVARIANT_FAILED} ; JLOG(journal.fatal()) << "Transaction has failed one or more invariants: " << to_string(tx.getJson (0)); @@ -114,8 +114,8 @@ ApplyContext::checkInvariantsHelper(TER terResult, std::index_sequence) catch(std::exception const& ex) { terResult = (terResult == tecINVARIANT_FAILED) ? - tefINVARIANT_FAILED : - tecINVARIANT_FAILED ; + TER {tefINVARIANT_FAILED} : + TER {tecINVARIANT_FAILED} ; JLOG(journal.fatal()) << "Transaction caused an exception in an invariant" << ", ex: " << ex.what() << diff --git a/src/ripple/app/tx/impl/CancelCheck.cpp b/src/ripple/app/tx/impl/CancelCheck.cpp index bf7068043..09143c10f 100644 --- a/src/ripple/app/tx/impl/CancelCheck.cpp +++ b/src/ripple/app/tx/impl/CancelCheck.cpp @@ -28,13 +28,13 @@ namespace ripple { -TER +NotTEC CancelCheck::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled (featureChecks)) return temDISABLED; - TER const ret {preflight1 (ctx)}; + NotTEC const ret {preflight1 (ctx)}; if (! isTesSuccess (ret)) return ret; diff --git a/src/ripple/app/tx/impl/CancelCheck.h b/src/ripple/app/tx/impl/CancelCheck.h index 1c1ab3627..7e544a299 100644 --- a/src/ripple/app/tx/impl/CancelCheck.h +++ b/src/ripple/app/tx/impl/CancelCheck.h @@ -34,7 +34,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/CancelOffer.cpp b/src/ripple/app/tx/impl/CancelOffer.cpp index c7c48c118..eaa76572d 100644 --- a/src/ripple/app/tx/impl/CancelOffer.cpp +++ b/src/ripple/app/tx/impl/CancelOffer.cpp @@ -24,7 +24,7 @@ namespace ripple { -TER +NotTEC CancelOffer::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); diff --git a/src/ripple/app/tx/impl/CancelOffer.h b/src/ripple/app/tx/impl/CancelOffer.h index c471d72fb..42e9f891d 100644 --- a/src/ripple/app/tx/impl/CancelOffer.h +++ b/src/ripple/app/tx/impl/CancelOffer.h @@ -37,7 +37,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index 7e93903a9..b3d5544e8 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -26,7 +26,7 @@ namespace ripple { -TER +NotTEC CancelTicket::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureTickets)) diff --git a/src/ripple/app/tx/impl/CancelTicket.h b/src/ripple/app/tx/impl/CancelTicket.h index 2e205024a..cdc1c4f4c 100644 --- a/src/ripple/app/tx/impl/CancelTicket.h +++ b/src/ripple/app/tx/impl/CancelTicket.h @@ -36,7 +36,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER doApply () override; diff --git a/src/ripple/app/tx/impl/CashCheck.cpp b/src/ripple/app/tx/impl/CashCheck.cpp index c5e27f9e0..526805a3e 100644 --- a/src/ripple/app/tx/impl/CashCheck.cpp +++ b/src/ripple/app/tx/impl/CashCheck.cpp @@ -31,13 +31,13 @@ namespace ripple { -TER +NotTEC CashCheck::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled (featureChecks)) return temDISABLED; - TER const ret {preflight1 (ctx)}; + NotTEC const ret {preflight1 (ctx)}; if (! isTesSuccess (ret)) return ret; diff --git a/src/ripple/app/tx/impl/CashCheck.h b/src/ripple/app/tx/impl/CashCheck.h index 4114be5d0..e3b469e36 100644 --- a/src/ripple/app/tx/impl/CashCheck.h +++ b/src/ripple/app/tx/impl/CashCheck.h @@ -34,7 +34,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index 707cfa8b8..cb216e9af 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -27,7 +27,7 @@ namespace ripple { -TER +NotTEC Change::preflight (PreflightContext const& ctx) { auto const ret = preflight0(ctx); diff --git a/src/ripple/app/tx/impl/Change.h b/src/ripple/app/tx/impl/Change.h index 2a9a53f5b..47a493c71 100644 --- a/src/ripple/app/tx/impl/Change.h +++ b/src/ripple/app/tx/impl/Change.h @@ -39,7 +39,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER doApply () override; diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index 03d0b9179..5bddbbd46 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -28,13 +28,13 @@ namespace ripple { -TER +NotTEC CreateCheck::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled (featureChecks)) return temDISABLED; - TER const ret {preflight1 (ctx)}; + NotTEC const ret {preflight1 (ctx)}; if (! isTesSuccess (ret)) return ret; diff --git a/src/ripple/app/tx/impl/CreateCheck.h b/src/ripple/app/tx/impl/CreateCheck.h index 49cdfce16..805aefae0 100644 --- a/src/ripple/app/tx/impl/CreateCheck.h +++ b/src/ripple/app/tx/impl/CreateCheck.h @@ -34,7 +34,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 46dbe8c41..1bc607e21 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -38,7 +38,7 @@ CreateOffer::calculateMaxSpend(STTx const& tx) saTakerGets.xrp() : beast::zero; } -TER +NotTEC CreateOffer::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); @@ -198,7 +198,9 @@ CreateOffer::preclaim(PreclaimContext const& ctx) // // The return code change is attached to featureChecks as a convenience. // The change is not big enough to deserve its own amendment. - return ctx.view.rules().enabled(featureChecks) ? tecEXPIRED : tesSUCCESS; + return ctx.view.rules().enabled(featureChecks) + ? TER {tecEXPIRED} + : TER {tesSUCCESS}; } // Make sure that we are authorized to hold what the taker will pay us. @@ -231,8 +233,8 @@ CreateOffer::checkAcceptAsset(ReadView const& view, to_string (issue.account); return (flags & tapRETRY) - ? terNO_ACCOUNT - : tecNO_ISSUER; + ? TER {terNO_ACCOUNT} + : TER {tecNO_ISSUER}; } // This code is attached to the FlowCross amendment as a matter of @@ -250,8 +252,8 @@ CreateOffer::checkAcceptAsset(ReadView const& view, if (!trustLine) { return (flags & tapRETRY) - ? terNO_LINE - : tecNO_LINE; + ? TER {terNO_LINE} + : TER {tecNO_LINE}; } // Entries have a canonical representation, determined by a @@ -268,8 +270,8 @@ CreateOffer::checkAcceptAsset(ReadView const& view, "delay: can't receive IOUs from issuer without auth."; return (flags & tapRETRY) - ? terNO_AUTH - : tecNO_AUTH; + ? TER {terNO_AUTH} + : TER {tecNO_AUTH}; } } @@ -1073,7 +1075,7 @@ CreateOffer::applyGuts (Sandbox& sb, Sandbox& sbCancel) auto viewJ = ctx_.app.journal("View"); - auto result = tesSUCCESS; + TER result = tesSUCCESS; // Process a cancellation request that's passed along with an offer. if (cancelSequence) @@ -1107,7 +1109,7 @@ CreateOffer::applyGuts (Sandbox& sb, Sandbox& sbCancel) // The return code change is attached to featureChecks as a convenience. // The change is not big enough to deserve its own amendment. TER const ter {ctx_.view().rules().enabled( - featureChecks) ? tecEXPIRED : tesSUCCESS}; + featureChecks) ? TER {tecEXPIRED} : TER {tesSUCCESS}}; return{ ter, true }; } diff --git a/src/ripple/app/tx/impl/CreateOffer.h b/src/ripple/app/tx/impl/CreateOffer.h index 92064b0a5..93026ecc5 100644 --- a/src/ripple/app/tx/impl/CreateOffer.h +++ b/src/ripple/app/tx/impl/CreateOffer.h @@ -49,7 +49,7 @@ public: /** Enforce constraints beyond those of the Transactor base class. */ static - TER + NotTEC preflight (PreflightContext const& ctx); /** Enforce constraints beyond those of the Transactor base class. */ diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index a5dfd1f09..2f9fae474 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -26,7 +26,7 @@ namespace ripple { -TER +NotTEC CreateTicket::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureTickets)) diff --git a/src/ripple/app/tx/impl/CreateTicket.h b/src/ripple/app/tx/impl/CreateTicket.h index 57db3cad2..411307aaa 100644 --- a/src/ripple/app/tx/impl/CreateTicket.h +++ b/src/ripple/app/tx/impl/CreateTicket.h @@ -37,7 +37,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER doApply () override; diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 3a3d1b772..f14333dd9 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -92,7 +92,7 @@ EscrowCreate::calculateMaxSpend(STTx const& tx) return tx[sfAmount].xrp(); } -TER +NotTEC EscrowCreate::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureEscrow)) @@ -293,7 +293,7 @@ checkCondition (Slice f, Slice c) return validate (*fulfillment, *condition); } -TER +NotTEC EscrowFinish::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureEscrow)) @@ -503,7 +503,7 @@ EscrowFinish::doApply() //------------------------------------------------------------------------------ -TER +NotTEC EscrowCancel::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureEscrow)) diff --git a/src/ripple/app/tx/impl/Escrow.h b/src/ripple/app/tx/impl/Escrow.h index bc42c2c60..45e617daf 100644 --- a/src/ripple/app/tx/impl/Escrow.h +++ b/src/ripple/app/tx/impl/Escrow.h @@ -39,7 +39,7 @@ public: calculateMaxSpend(STTx const& tx); static - TER + NotTEC preflight (PreflightContext const& ctx); TER @@ -59,7 +59,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static @@ -83,7 +83,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index bc002cbf1..923ae0cf3 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -154,7 +154,7 @@ closeChannel ( //------------------------------------------------------------------------------ -TER +NotTEC PayChanCreate::preflight (PreflightContext const& ctx) { if (!ctx.rules.enabled (featurePayChan)) @@ -262,7 +262,7 @@ PayChanCreate::doApply() //------------------------------------------------------------------------------ -TER +NotTEC PayChanFund::preflight (PreflightContext const& ctx) { if (!ctx.rules.enabled (featurePayChan)) @@ -347,13 +347,17 @@ PayChanFund::doApply() //------------------------------------------------------------------------------ -TER +NotTEC PayChanClaim::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featurePayChan)) return temDISABLED; - bool const noTecs = ctx.rules.enabled(fix1512); + // A search through historic MainNet ledgers by the data team found no + // occurrences of a transaction with the error that fix1512 fixed. That + // means there are no old transactions with that error that we might + // need to replay. So the check for fix1512 is removed. Apr 2018. +// bool const noTecs = ctx.rules.enabled(fix1512); auto const ret = preflight1 (ctx); if (!isTesSuccess (ret)) @@ -368,12 +372,7 @@ PayChanClaim::preflight (PreflightContext const& ctx) return temBAD_AMOUNT; if (bal && amt && *bal > *amt) - { - if (noTecs) - return temBAD_AMOUNT; - else - return tecNO_PERMISSION; - } + return temBAD_AMOUNT; { auto const flags = ctx.tx.getFlags(); @@ -398,12 +397,7 @@ PayChanClaim::preflight (PreflightContext const& ctx) auto const authAmt = amt ? amt->xrp() : reqBalance; if (reqBalance > authAmt) - { - if (noTecs) - return temBAD_AMOUNT; - else - return tecNO_PERMISSION; - } + return temBAD_AMOUNT; Keylet const k (ltPAYCHAN, ctx.tx[sfPayChannel]); if (!publicKeyType(ctx.tx[sfPublicKey])) diff --git a/src/ripple/app/tx/impl/PayChan.h b/src/ripple/app/tx/impl/PayChan.h index d04687ac3..f223dcebf 100644 --- a/src/ripple/app/tx/impl/PayChan.h +++ b/src/ripple/app/tx/impl/PayChan.h @@ -35,7 +35,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static @@ -59,7 +59,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER @@ -79,7 +79,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 01308b946..eb7e7a1a5 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -44,7 +44,7 @@ Payment::calculateMaxSpend(STTx const& tx) return saDstAmount.native() ? saDstAmount.xrp() : beast::zero; } -TER +NotTEC Payment::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); diff --git a/src/ripple/app/tx/impl/Payment.h b/src/ripple/app/tx/impl/Payment.h index de925acee..66a9d688a 100644 --- a/src/ripple/app/tx/impl/Payment.h +++ b/src/ripple/app/tx/impl/Payment.h @@ -49,7 +49,7 @@ public: calculateMaxSpend(STTx const& tx); static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index e5a30177c..dfff835a5 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -48,7 +48,7 @@ SetAccount::affectsSubsequentTransactionAuth(STTx const& tx) *uClearFlag == asfAccountTxnID); } -TER +NotTEC SetAccount::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); @@ -190,7 +190,7 @@ SetAccount::preclaim(PreclaimContext const& ctx) keylet::ownerDir(id))) { JLOG(ctx.j.trace()) << "Retry: Owner directory not empty."; - return (ctx.flags & tapRETRY) ? terOWNERS : tecOWNERS; + return (ctx.flags & tapRETRY) ? TER {terOWNERS} : TER {tecOWNERS}; } } diff --git a/src/ripple/app/tx/impl/SetAccount.h b/src/ripple/app/tx/impl/SetAccount.h index af3df2e27..83eaacbf2 100644 --- a/src/ripple/app/tx/impl/SetAccount.h +++ b/src/ripple/app/tx/impl/SetAccount.h @@ -45,7 +45,7 @@ public: affectsSubsequentTransactionAuth(STTx const& tx); static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/SetRegularKey.cpp b/src/ripple/app/tx/impl/SetRegularKey.cpp index 83d7cc6da..d3e269086 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.cpp +++ b/src/ripple/app/tx/impl/SetRegularKey.cpp @@ -48,7 +48,7 @@ SetRegularKey::calculateBaseFee ( return Transactor::calculateBaseFee (ctx); } -TER +NotTEC SetRegularKey::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); diff --git a/src/ripple/app/tx/impl/SetRegularKey.h b/src/ripple/app/tx/impl/SetRegularKey.h index a7b536a77..668be7488 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.h +++ b/src/ripple/app/tx/impl/SetRegularKey.h @@ -44,7 +44,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 758515ced..1773be349 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -35,7 +35,7 @@ namespace ripple { // setting the sfSignerListID to zero in all cases. static std::uint32_t const defaultSignerListID_ = 0; -std::tuple, SetSignerList::Operation> SetSignerList::determineOperation(STTx const& tx, @@ -69,7 +69,7 @@ SetSignerList::determineOperation(STTx const& tx, return std::make_tuple(tesSUCCESS, quorum, sign, op); } -TER +NotTEC SetSignerList::preflight (PreflightContext const& ctx) { if (! ctx.rules.enabled(featureMultiSign)) @@ -95,7 +95,7 @@ SetSignerList::preflight (PreflightContext const& ctx) { // Validate our settings. auto const account = ctx.tx.getAccountID(sfAccount); - TER const ter = + NotTEC const ter = validateQuorumAndSignerEntries(std::get<1>(result), std::get<2>(result), account, ctx.j); if (ter != tesSUCCESS) @@ -141,7 +141,7 @@ SetSignerList::preCompute() return Transactor::preCompute(); } -TER +NotTEC SetSignerList::validateQuorumAndSignerEntries ( std::uint32_t quorum, std::vector const& signers, diff --git a/src/ripple/app/tx/impl/SetSignerList.h b/src/ripple/app/tx/impl/SetSignerList.h index c6f9cb563..4c2d35f26 100644 --- a/src/ripple/app/tx/impl/SetSignerList.h +++ b/src/ripple/app/tx/impl/SetSignerList.h @@ -61,7 +61,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); TER doApply () override; @@ -69,14 +69,14 @@ public: private: static - std::tuple, Operation> determineOperation(STTx const& tx, ApplyFlags flags, beast::Journal j); static - TER validateQuorumAndSignerEntries ( + NotTEC validateQuorumAndSignerEntries ( std::uint32_t quorum, std::vector const& signers, AccountID const& account, diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 5e801a311..a34922a5d 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -27,7 +27,7 @@ namespace ripple { -TER +NotTEC SetTrust::preflight (PreflightContext const& ctx) { auto const ret = preflight1 (ctx); diff --git a/src/ripple/app/tx/impl/SetTrust.h b/src/ripple/app/tx/impl/SetTrust.h index 9ec7936e6..c804bfdca 100644 --- a/src/ripple/app/tx/impl/SetTrust.h +++ b/src/ripple/app/tx/impl/SetTrust.h @@ -38,7 +38,7 @@ public: } static - TER + NotTEC preflight (PreflightContext const& ctx); static diff --git a/src/ripple/app/tx/impl/SignerEntries.cpp b/src/ripple/app/tx/impl/SignerEntries.cpp index f1e0a3ed5..e15634a9f 100644 --- a/src/ripple/app/tx/impl/SignerEntries.cpp +++ b/src/ripple/app/tx/impl/SignerEntries.cpp @@ -25,11 +25,11 @@ namespace ripple { -std::pair, TER> +std::pair, NotTEC> SignerEntries::deserialize ( STObject const& obj, beast::Journal journal, std::string const& annotation) { - std::pair, TER> s; + std::pair, NotTEC> s; if (!obj.isFieldPresent (sfSignerEntries)) { diff --git a/src/ripple/app/tx/impl/SignerEntries.h b/src/ripple/app/tx/impl/SignerEntries.h index 6a41f8f44..fab916039 100644 --- a/src/ripple/app/tx/impl/SignerEntries.h +++ b/src/ripple/app/tx/impl/SignerEntries.h @@ -20,9 +20,10 @@ #ifndef RIPPLE_TX_IMPL_SIGNER_ENTRIES_H_INCLUDED #define RIPPLE_TX_IMPL_SIGNER_ENTRIES_H_INCLUDED -#include // STTx::maxMultiSigners -#include // AccountID -#include // temMALFORMED +#include // NotTEC +#include // STTx::maxMultiSigners +#include // AccountID +#include // temMALFORMED #include // beast::Journal namespace ripple { @@ -60,7 +61,7 @@ public: // Deserialize a SignerEntries array from the network or from the ledger. static - std::pair, TER> + std::pair, NotTEC> deserialize ( STObject const& obj, beast::Journal journal, diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index c56c80019..e6a2e1faa 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -35,7 +35,7 @@ namespace ripple { /** Performs early sanity checks on the txid */ -TER +NotTEC preflight0(PreflightContext const& ctx) { auto const txID = ctx.tx.getTransactionID(); @@ -51,7 +51,7 @@ preflight0(PreflightContext const& ctx) } /** Performs early sanity checks on the account and fee fields */ -TER +NotTEC preflight1 (PreflightContext const& ctx) { auto const ret = preflight0(ctx); @@ -85,7 +85,7 @@ preflight1 (PreflightContext const& ctx) } /** Checks whether the signature appears valid */ -TER +NotTEC preflight2 (PreflightContext const& ctx) { if(!( ctx.flags & tapNO_CHECK_SIGN)) @@ -226,7 +226,7 @@ TER Transactor::payFee () return tesSUCCESS; } -TER +NotTEC Transactor::checkSeq (PreclaimContext const& ctx) { auto const id = ctx.tx.getAccountID(sfAccount); @@ -327,7 +327,7 @@ TER Transactor::apply () return doApply (); } -TER +NotTEC Transactor::checkSign (PreclaimContext const& ctx) { // Make sure multisigning is enabled before we check for multisignatures. @@ -341,7 +341,7 @@ Transactor::checkSign (PreclaimContext const& ctx) return checkSingleSign (ctx); } -TER +NotTEC Transactor::checkSingleSign (PreclaimContext const& ctx) { auto const id = ctx.tx.getAccountID(sfAccount); @@ -390,7 +390,8 @@ Transactor::checkSingleSign (PreclaimContext const& ctx) return tesSUCCESS; } -TER Transactor::checkMultiSign (PreclaimContext const& ctx) +NotTEC +Transactor::checkMultiSign (PreclaimContext const& ctx) { auto const id = ctx.tx.getAccountID(sfAccount); // Get mTxnAccountID's SignerList and Quorum. diff --git a/src/ripple/app/tx/impl/Transactor.h b/src/ripple/app/tx/impl/Transactor.h index 105688945..9ea5c7b0b 100644 --- a/src/ripple/app/tx/impl/Transactor.h +++ b/src/ripple/app/tx/impl/Transactor.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_APP_TX_TRANSACTOR_H_INCLUDED #define RIPPLE_APP_TX_TRANSACTOR_H_INCLUDED +#include #include #include #include @@ -116,7 +117,7 @@ public: */ static - TER + NotTEC checkSeq (PreclaimContext const& ctx); static @@ -124,7 +125,7 @@ public: checkFee (PreclaimContext const& ctx, std::uint64_t baseFee); static - TER + NotTEC checkSign (PreclaimContext const& ctx); // Returns the fee in fee units, not scaled for load. @@ -173,20 +174,20 @@ private: void setSeq (); TER payFee (); void claimFee (XRPAmount& fee, TER terResult, std::vector const& removedOffers); - static TER checkSingleSign (PreclaimContext const& ctx); - static TER checkMultiSign (PreclaimContext const& ctx); + static NotTEC checkSingleSign (PreclaimContext const& ctx); + static NotTEC checkMultiSign (PreclaimContext const& ctx); }; /** Performs early sanity checks on the txid */ -TER +NotTEC preflight0(PreflightContext const& ctx); /** Performs early sanity checks on the account and fee fields */ -TER +NotTEC preflight1 (PreflightContext const& ctx); /** Checks whether the signature appears valid */ -TER +NotTEC preflight2 (PreflightContext const& ctx); } diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index b64b5959d..5a28a3011 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -38,7 +38,7 @@ namespace ripple { static -TER +NotTEC invoke_preflight (PreflightContext const& ctx) { switch(ctx.tx.getTxnType()) diff --git a/src/ripple/ledger/TxMeta.h b/src/ripple/ledger/TxMeta.h index 2e7fec586..6566e5ec9 100644 --- a/src/ripple/ledger/TxMeta.h +++ b/src/ripple/ledger/TxMeta.h @@ -87,7 +87,7 @@ public: } TER getResultTER () const { - return static_cast (mResult); + return TER::fromInt (mResult); } std::uint32_t getIndex () const { diff --git a/src/ripple/ledger/impl/TxMeta.cpp b/src/ripple/ledger/impl/TxMeta.cpp index cac23a062..76f10cbee 100644 --- a/src/ripple/ledger/impl/TxMeta.cpp +++ b/src/ripple/ledger/impl/TxMeta.cpp @@ -257,7 +257,7 @@ STObject TxMeta::getAsObject () const void TxMeta::addRaw (Serializer& s, TER result, std::uint32_t index) { - mResult = static_cast (result); + mResult = TERtoInt (result); mIndex = index; assert ((mResult == 0) || ((mResult > 100) && (mResult <= 255))); diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 88978f189..79ab3a534 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1581,8 +1581,8 @@ accountSend (ApplyView& view, // VFALCO Its laborious to have to mutate the // TER based on params everywhere terResult = view.open() - ? telFAILED_PROCESSING - : tecFAILED_PROCESSING; + ? TER {telFAILED_PROCESSING} + : TER {tecFAILED_PROCESSING}; } else { @@ -1847,8 +1847,8 @@ transferXRP (ApplyView& view, // mutating these TER everywhere // FIXME: this logic should be moved to callers maybe? return view.open() - ? telFAILED_PROCESSING - : tecFAILED_PROCESSING; + ? TER {telFAILED_PROCESSING} + : TER {tecFAILED_PROCESSING}; } // Decrement XRP balance. diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 791221bd8..5af438c42 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ /* This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. + Copyright (c) 2012 - 2018 Ripple Labs Inc. Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -20,7 +20,10 @@ #ifndef RIPPLE_PROTOCOL_TER_H_INCLUDED #define RIPPLE_PROTOCOL_TER_H_INCLUDED +#include + #include +#include #include namespace ripple { @@ -30,9 +33,13 @@ namespace ripple { // "Transaction Engine Result" // or Transaction ERror. // -enum TER +using TERUnderlyingType = int; + +//------------------------------------------------------------------------------ + +enum TELcodes : TERUnderlyingType { - // Note: Range is stable. Exact numbers are currently unstable. Use tokens. + // Note: Range is stable. Exact numbers are unstable. Use tokens. // -399 .. -300: L Local error (transaction fee inadequate, exceeds local limit) // Only valid during non-consensus processing. @@ -51,7 +58,14 @@ enum TER telCAN_NOT_QUEUE_BLOCKS, telCAN_NOT_QUEUE_BLOCKED, telCAN_NOT_QUEUE_FEE, - telCAN_NOT_QUEUE_FULL, + telCAN_NOT_QUEUE_FULL +}; + +//------------------------------------------------------------------------------ + +enum TEMcodes : TERUnderlyingType +{ + // Note: Range is stable. Exact numbers are unstable. Use tokens. // -299 .. -200: M Malformed (bad signature) // Causes: @@ -95,7 +109,14 @@ enum TER // An intermediate result used internally, should never be returned. temUNCERTAIN, - temUNKNOWN, + temUNKNOWN +}; + +//------------------------------------------------------------------------------ + +enum TEFcodes : TERUnderlyingType +{ + // Note: Range is stable. Exact numbers are unstable. Use tokens. // -199 .. -100: F // Failure (sequence number previously used) @@ -127,6 +148,13 @@ enum TER tefNOT_MULTI_SIGNING, tefBAD_AUTH_MASTER, tefINVARIANT_FAILED, +}; + +//------------------------------------------------------------------------------ + +enum TERcodes : TERUnderlyingType +{ + // Note: Range is stable. Exact numbers are unstable. Use tokens. // -99 .. -1: R Retry // sequence too high, no funds for txn fee, originating -account @@ -155,7 +183,15 @@ enum TER // burden network. terLAST, // Process after all other transactions terNO_RIPPLE, // Rippling not allowed - terQUEUED, // Transaction is being held in TxQ until fee drops + terQUEUED // Transaction is being held in TxQ until fee drops +}; + +//------------------------------------------------------------------------------ + +enum TEScodes : TERUnderlyingType +{ + // Note: Exact number must stay stable. This code is stored by value + // in metadata for historic transactions. // 0: S Success (success) // Causes: @@ -163,7 +199,15 @@ enum TER // Implications: // - Applied // - Forwarded - tesSUCCESS = 0, + tesSUCCESS = 0 +}; + +//------------------------------------------------------------------------------ + +enum TECcodes : TERUnderlyingType +{ + // Note: Exact numbers must stay stable. These codes are stored by + // value in metadata for historic transactions. // 100 .. 159 C // Claim fee only (ripple transaction with no good paths, pay to @@ -218,6 +262,201 @@ enum TER tecEXPIRED = 148 }; +//------------------------------------------------------------------------------ + +// For generic purposes, a free function that returns the value of a TE*codes. +constexpr TERUnderlyingType TERtoInt (TELcodes v) +{ return static_cast(v); } + +constexpr TERUnderlyingType TERtoInt (TEMcodes v) +{ return static_cast(v); } + +constexpr TERUnderlyingType TERtoInt (TEFcodes v) +{ return static_cast(v); } + +constexpr TERUnderlyingType TERtoInt (TERcodes v) +{ return static_cast(v); } + +constexpr TERUnderlyingType TERtoInt (TEScodes v) +{ return static_cast(v); } + +constexpr TERUnderlyingType TERtoInt (TECcodes v) +{ return static_cast(v); } + +//------------------------------------------------------------------------------ +// Template class that is specific to selected ranges of error codes. The +// Trait tells std::enable_if which ranges are allowed. +template class Trait> +class TERSubset +{ + TERUnderlyingType code_; + +public: + // Constructors + constexpr TERSubset() : code_ (tesSUCCESS) { } + constexpr TERSubset (TERSubset const& rhs) = default; + constexpr TERSubset (TERSubset&& rhs) = default; +private: + constexpr explicit TERSubset (int rhs) : code_ (rhs) { } +public: + static constexpr TERSubset fromInt (int from) + { + return TERSubset (from); + } + + // Trait tells enable_if which types are allowed for construction. + template ::value>> + constexpr TERSubset (T rhs) + : code_ (TERtoInt (rhs)) + { } + + // Assignment + constexpr TERSubset& operator=(TERSubset const& rhs) = default; + constexpr TERSubset& operator=(TERSubset&& rhs) = default; + + // Trait tells enable_if which types are allowed for assignment. + template + constexpr auto + operator= (T rhs) -> std::enable_if_t::value, TERSubset&> + { + code_ = TERtoInt (rhs); + return *this; + } + + // Conversion to bool. + explicit operator bool() const + { + return code_ != tesSUCCESS; + } + + // Conversion to Json::Value allows assignment to Json::Objects + // without casting. + operator Json::Value() const + { + return Json::Value {code_}; + } + + // Streaming operator. + friend std::ostream& operator<< (std::ostream& os, TERSubset const& rhs) + { + return os << rhs.code_; + } + + // Return the underlying value. Not a member so similarly named free + // functions can do the same work for the enums. + // + // It's worth noting that an explicit conversion operator was considered + // and rejected. Consider this case, taken from Status.h + // + // class Status { + // int code_; + // public: + // Status (TER ter) + // : code_ (ter) {} + // } + // + // This code compiles with no errors or warnings if TER has an explicit + // (unnamed) conversion to int. To avoid silent conversions like these + // we provide (only) a named conversion. + friend constexpr TERUnderlyingType TERtoInt (TERSubset v) + { + return v.code_; + } +}; + +// Comparison operators. +// Only enabled if both arguments return int if TERtiInt is called with them. +template +constexpr auto +operator== (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) == TERtoInt(rhs); +} + +template +constexpr auto +operator!= (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) != TERtoInt(rhs); +} + +template +constexpr auto +operator< (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) < TERtoInt(rhs); +} + +template +constexpr auto +operator<= (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) <= TERtoInt(rhs); +} + +template +constexpr auto +operator> (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) > TERtoInt(rhs); +} + +template +constexpr auto +operator>= (L const& lhs, R const& rhs) -> std::enable_if_t< + std::is_same::value && + std::is_same::value, bool> +{ + return TERtoInt(lhs) >= TERtoInt(rhs); +} + +//------------------------------------------------------------------------------ + +// Use traits to build a TERSubset that can convert from any of the TE*codes +// enums *except* TECcodes: NotTEC + +// NOTE: NotTEC is useful for codes returned by preflight in transactors. +// Preflight checks occur prior to signature checking. If preflight returned +// a tec code, then a malicious user could submit a transaction with a very +// large fee and have that fee charged against an account without using that +// account's valid signature. +template class CanCvtToNotTEC : public std::false_type {}; +template <> class CanCvtToNotTEC : public std::true_type {}; +template <> class CanCvtToNotTEC : public std::true_type {}; +template <> class CanCvtToNotTEC : public std::true_type {}; +template <> class CanCvtToNotTEC : public std::true_type {}; +template <> class CanCvtToNotTEC : public std::true_type {}; + +using NotTEC = TERSubset; + +//------------------------------------------------------------------------------ + +// Use traits to build a TERSubset that can convert from any of the TE*codes +// enums as well as from NotTEC. +template class CanCvtToTER : public std::false_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; +template <> class CanCvtToTER : public std::true_type {}; + +// TER allows all of the subsets. +using TER = TERSubset; + +//------------------------------------------------------------------------------ + inline bool isTelLocal(TER x) { return ((x) >= telLOCAL_ERROR && (x) < temMALFORMED); @@ -248,20 +487,15 @@ inline bool isTecClaim(TER x) return ((x) >= tecCLAIM); } -// VFALCO TODO group these into a shell class along with the defines above. -extern bool transResultInfo (TER code, std::string& token, std::string& text); -extern std::string transToken (TER code); -extern std::string transHuman (TER code); -extern boost::optional transCode(std::string const& token); diff --git a/src/ripple/protocol/impl/STInteger.cpp b/src/ripple/protocol/impl/STInteger.cpp index dd8448a18..ab44f24ac 100644 --- a/src/ripple/protocol/impl/STInteger.cpp +++ b/src/ripple/protocol/impl/STInteger.cpp @@ -48,7 +48,7 @@ STUInt8::getText () const { std::string token, human; - if (transResultInfo (static_cast (value_), token, human)) + if (transResultInfo (TER::fromInt (value_), token, human)) return human; JLOG (debugLog().error()) @@ -66,7 +66,7 @@ STUInt8::getJson (int) const { std::string token, human; - if (transResultInfo (static_cast (value_), token, human)) + if (transResultInfo (TER::fromInt (value_), token, human)) return token; JLOG (debugLog().error()) diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index d8ef3adeb..ef7c95104 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -202,14 +202,16 @@ static boost::optional parseLeaf ( { auto ter = transCode(strValue); - if (!ter || *ter < minValue || *ter > maxValue) + if (!ter || + TERtoInt (*ter) < minValue || + TERtoInt (*ter) > maxValue) { error = out_of_range(json_name, fieldName); return ret; } ret = detail::make_stvar(field, - static_cast(*ter)); + static_cast(TERtoInt (*ter))); } else { diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 23a7238a8..1cc3166c8 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -28,13 +28,13 @@ namespace detail { static std::unordered_map< - std::underlying_type_t, + TERUnderlyingType, std::pair> const& transResults() { static std::unordered_map< - std::underlying_type_t, + TERUnderlyingType, std::pair> const results { @@ -162,8 +162,7 @@ bool transResultInfo (TER code, std::string& token, std::string& text) { auto& results = detail::transResults(); - auto const r = results.find ( - static_cast> (code)); + auto const r = results.find (TERtoInt (code)); if (r == results.end()) return false; @@ -207,7 +206,7 @@ transCode(std::string const& token) ); std::unordered_map< std::string, - std::underlying_type_t> const + TERUnderlyingType> const byToken(tRange.begin(), tRange.end()); return byToken; }(); @@ -217,7 +216,7 @@ transCode(std::string const& token) if (r == results.end()) return boost::none; - return static_cast(r->second); + return TER::fromInt (r->second); } } // ripple diff --git a/src/ripple/rpc/Status.h b/src/ripple/rpc/Status.h index 586bc888a..1f497a923 100644 --- a/src/ripple/rpc/Status.h +++ b/src/ripple/rpc/Status.h @@ -47,23 +47,26 @@ public: Status () = default; - Status (Code code, Strings d = {}) - : type_ (Type::none), code_ (code), messages_ (std::move (d)) + // The enable_if allows only integers (not enums). Prevents enum narrowing. + template ::value>> + Status (T code, Strings d = {}) + : type_ (Type::none), code_ (code), messages_ (std::move (d)) { } Status (TER ter, Strings d = {}) - : type_ (Type::TER), code_ (ter), messages_ (std::move (d)) + : type_ (Type::TER), code_ (TERtoInt (ter)), messages_ (std::move (d)) { } Status (error_code_i e, Strings d = {}) - : type_ (Type::error_code_i), code_ (e), messages_ (std::move (d)) + : type_ (Type::error_code_i), code_ (e), messages_ (std::move (d)) { } Status (error_code_i e, std::string const& s) - : type_ (Type::error_code_i), code_ (e), messages_ ({s}) + : type_ (Type::error_code_i), code_ (e), messages_ ({s}) { } @@ -89,7 +92,7 @@ public: TER toTER () const { assert (type_ == Type::TER); - return TER (code_); + return TER::fromInt (code_); } /** Returns the Status as an error_code_i. diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index fab30dd86..68336a69d 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -316,8 +316,9 @@ struct DepositAuth_test : public beast::unit_test::suite if (withDepositAuth) env(fset(gw1, asfDepositAuth)); - auto const result = - (noRippleNext && noRipplePrev) ? tecPATH_DRY : tesSUCCESS; + TER const result = (noRippleNext && noRipplePrev) + ? TER {tecPATH_DRY} + : TER {tesSUCCESS}; env (pay (alice, bob, USD1(10)), path (gw1), ter (result)); }; @@ -338,8 +339,9 @@ struct DepositAuth_test : public beast::unit_test::suite if (withDepositAuth) env(fset(alice, asfDepositAuth)); - auto const result = - (noRippleNext && noRipplePrev) ? tecPATH_DRY : tesSUCCESS; + TER const result = (noRippleNext && noRipplePrev) + ? TER {tecPATH_DRY} + : TER {tesSUCCESS}; env (pay (gw1, gw2, USD2 (10)), path (alice), sendmax (USD1 (10)), ter (result)); }; diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index d077e600b..9b973188d 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -1028,7 +1028,8 @@ struct Escrow_test : public beast::unit_test::suite auto const seq = env.seq(alice); env(escrow(alice, carol, XRP(1000)), finish_time(env.now() + 1s)); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); auto const escrow = env.le(keylet::escrow(alice.id(), seq)); BEAST_EXPECT(escrow); @@ -1050,7 +1051,8 @@ struct Escrow_test : public beast::unit_test::suite env(escrow(alice, alice, XRP(1000)), finish_time(env.now() + 1s), cancel_time(env.now() + 500s)); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); env.close(5s); auto const aa = env.le(keylet::escrow(alice.id(), aseq)); BEAST_EXPECT(aa); @@ -1063,7 +1065,8 @@ struct Escrow_test : public beast::unit_test::suite env(escrow(bruce, bruce, XRP(1000)), finish_time(env.now() + 1s), cancel_time(env.now() + 2s)); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); env.close(5s); auto const bb = env.le(keylet::escrow(bruce.id(), bseq)); BEAST_EXPECT(bb); @@ -1078,7 +1081,8 @@ struct Escrow_test : public beast::unit_test::suite env(finish(alice, alice, aseq)); { BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), aseq))); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); ripple::Dir aod(*env.current(), keylet::ownerDir(alice.id())); BEAST_EXPECT(std::distance(aod.begin(), aod.end()) == 0); @@ -1093,7 +1097,8 @@ struct Escrow_test : public beast::unit_test::suite env(cancel(bruce, bruce, bseq)); { BEAST_EXPECT(!env.le(keylet::escrow(bruce.id(), bseq))); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 0); @@ -1109,11 +1114,13 @@ struct Escrow_test : public beast::unit_test::suite auto const bseq = env.seq(bruce); env(escrow(alice, bruce, XRP(1000)), finish_time(env.now() + 1s)); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); env.close(5s); env(escrow(bruce, carol, XRP(1000)), finish_time(env.now() + 1s), cancel_time(env.now() + 2s)); - BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == + static_cast(tesSUCCESS)); env.close(5s); auto const ab = env.le(keylet::escrow(alice.id(), aseq)); diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index b655da7dc..081064e77 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -753,8 +753,9 @@ struct Flow_test : public beast::unit_test::suite env (offer (bob, XRP (50), USD (50))); env (offer (bob, XRP (100), USD (50))); - auto expectedResult = - closeTime < fix1141Time () ? tecPATH_DRY : tesSUCCESS; + TER const expectedResult = closeTime < fix1141Time () + ? TER {tecPATH_DRY} + : TER {tesSUCCESS}; env (pay (alice, carol, USD (100)), path (~USD), sendmax (XRP (100)), txflags (tfNoRippleDirect | tfPartialPayment | tfLimitQuality), ter (expectedResult)); @@ -1131,7 +1132,7 @@ struct Flow_test : public beast::unit_test::suite env(pay(alice, alice, XRP(1)), path(gw, bob, ~XRP), sendmax(gw["USD"](1000)), txflags(tfNoRippleDirect), - ter(withFix ? tecPATH_DRY : tesSUCCESS)); + ter(withFix ? TER {tecPATH_DRY} : TER {tesSUCCESS})); env.close(); if (withFix) @@ -1145,7 +1146,7 @@ struct Flow_test : public beast::unit_test::suite env(pay (carol, carol, gw["USD"](1000)), path(~bob["USD"], gw), sendmax(XRP(100000)), txflags(tfNoRippleDirect), - ter(withFix ? tecPATH_DRY : tesSUCCESS)); + ter(withFix ? TER {tecPATH_DRY} : TER {tesSUCCESS})); env.close(); pass(); @@ -1189,7 +1190,7 @@ struct Flow_test : public beast::unit_test::suite env(pay(alice, alice, USD(1000)), path(~bob["USD"], bob, gw), sendmax(XRP(1)), txflags(tfNoRippleDirect), - ter(withFix ? tecPATH_DRY : tesSUCCESS)); + ter(withFix ? TER {tecPATH_DRY} : TER {tesSUCCESS})); env.close(); } diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index a61bf7923..0de95f29f 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -239,7 +239,7 @@ public: env (offer (alice, XRP (5), USD (2)), json (sfExpiration.fieldName, lastClose(env)), json (jss::OfferSequence, offer2Seq), - ter (featChecks ? tecEXPIRED : tesSUCCESS)); + ter (featChecks ? TER {tecEXPIRED} : TER {tesSUCCESS})); env.close(); env.require (offers (alice, 2)); @@ -289,8 +289,8 @@ public: (hasFeature(env, featureFeeEscalation) && !hasFeature(env, fix1513)); // Will fail without the underflow fix - auto expectedResult = *stAmountCalcSwitchover ? - tesSUCCESS : tecPATH_PARTIAL; + TER const expectedResult = *stAmountCalcSwitchover ? + TER {tesSUCCESS} : TER {tecPATH_PARTIAL}; env (pay (alice, bob, EUR (epsilon)), path (~EUR), sendmax (USD (100)), ter (expectedResult)); } @@ -959,7 +959,7 @@ public: env (offer (alice, xrpOffer, usdOffer), json (sfExpiration.fieldName, lastClose(env)), - ter (featChecks ? tecEXPIRED : tesSUCCESS)); + ter (featChecks ? TER {tecEXPIRED} : TER {tesSUCCESS})); env.require ( balance (alice, startBalance - f - f), @@ -3422,7 +3422,7 @@ public: // Determine which TEC code we expect. TER const tecExpect = - features[featureFlow] ? temBAD_PATH : tecPATH_DRY; + features[featureFlow] ? TER {temBAD_PATH} : TER {tecPATH_DRY}; // This payment caused the assert. env (pay (ann, ann, D_BUX(30)), @@ -4397,7 +4397,7 @@ public: // create an offer to buy their own currency. After FlowCross // they can. env (offer (gw, gwUSD(40), XRP(4000)), - ter (flowCross ? tesSUCCESS : tecNO_LINE)); + ter (flowCross ? TER {tesSUCCESS} : TER {tecNO_LINE})); env.close(); env.require (offers (gw, flowCross ? 1 : 0)); diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 682d0549d..7a39cf586 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -1375,7 +1375,7 @@ struct PayStrand_test : public beast::unit_test::suite env(offer(bob, XRP(100), USD(100)), txflags(tfPassive)); env(offer(bob, USD(100), XRP(100)), txflags(tfPassive)); - auto const expectedResult = [&] { + auto const expectedResult = [&] () -> TER { if (features[featureFlow] && !features[fix1373]) return tesSUCCESS; return temBAD_PATH_LOOP; diff --git a/src/test/jtx/JTx.h b/src/test/jtx/JTx.h index da82be28a..dfc310208 100644 --- a/src/test/jtx/JTx.h +++ b/src/test/jtx/JTx.h @@ -42,7 +42,7 @@ struct JTx { Json::Value jv; requires_t requires; - boost::optional ter = tesSUCCESS; + boost::optional ter = TER {tesSUCCESS}; bool fill_fee = true; bool fill_seq = true; bool fill_sig = true; diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index f10e1d59a..3995b1e24 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -278,8 +278,7 @@ Env::parseResult(Json::Value const& jr) TER ter; if (jr.isObject() && jr.isMember(jss::result) && jr[jss::result].isMember(jss::engine_result_code)) - ter = static_cast( - jr[jss::result][jss::engine_result_code].asInt()); + ter = TER::fromInt (jr[jss::result][jss::engine_result_code].asInt()); else ter = temINVALID; return std::make_pair(ter, diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp index c2ecc5451..8919b9030 100644 --- a/src/test/ledger/Invariants_test.cpp +++ b/src/test/ledger/Invariants_test.cpp @@ -94,15 +94,16 @@ class Invariants_test : public beast::unit_test::suite BEAST_EXPECT(precheck(A1, A2, ac)); - auto tr = tesSUCCESS; + TER tr = tesSUCCESS; // invoke check twice to cover tec and tef cases for (auto i : {0,1}) { tr = ac.checkInvariants(tr); if (enabled) { - BEAST_EXPECT( - tr == (i == 0 ? tecINVARIANT_FAILED : tefINVARIANT_FAILED)); + BEAST_EXPECT(tr == (i == 0 + ? TER {tecINVARIANT_FAILED} + : TER {tefINVARIANT_FAILED})); BEAST_EXPECT( boost::starts_with(sink.strm_.str(), "Invariant failed:") || boost::starts_with(sink.strm_.str(), diff --git a/src/test/protocol/TER_test.cpp b/src/test/protocol/TER_test.cpp index 4cbaa0f68..7bb40baac 100644 --- a/src/test/protocol/TER_test.cpp +++ b/src/test/protocol/TER_test.cpp @@ -20,6 +20,9 @@ #include #include +#include +#include + namespace ripple { struct TER_test : public beast::unit_test::suite @@ -29,7 +32,7 @@ struct TER_test : public beast::unit_test::suite { for (auto i = -400; i < 400; ++i) { - TER t = static_cast(i); + TER t = TER::fromInt (i); auto inRange = isTelLocal(t) || isTemMalformed(t) || isTefFailure(t) || @@ -49,13 +52,214 @@ struct TER_test : public beast::unit_test::suite } } + // Helper template that makes sure two types are not convertible or + // assignable if not the same. + // o I1 one tuple index. + // o I2 other tuple index. + // o Tup is expected to be a tuple. + // It's a functor, rather than a function template, since a class template + // can be a template argument without being full specified. + template + class NotConvertible + { + public: + template + void operator()(Tup const& tup, beast::unit_test::suite&) const + { + // Entries in the tuple should not be convertible or assignable + // unless they are the same types. + using To_t = std::decay_t(tup))>; + using From_t = std::decay_t(tup))>; + static_assert (std::is_same::value == + std::is_convertible::value, "Convert err"); + static_assert (std::is_same::value == + std::is_constructible::value, "Construct err"); + static_assert (std::is_same ::value == + std::is_assignable::value, "Assign err"); + + // Assignment or conversion from integer to type should never work. + static_assert ( + ! std::is_convertible::value, "Convert err"); + static_assert ( + ! std::is_constructible::value, "Construct err"); + static_assert ( + ! std::is_assignable::value, "Assign err"); + } + }; + + // Fast iteration over the tuple. + template class Func, typename Tup> + std::enable_if_t + testIterate (Tup const& tup, beast::unit_test::suite& suite) + { + Func func; + func (tup, suite); + testIterate(tup, suite); + } + + // Slow iteration over the tuple. + template class Func, typename Tup> + std::enable_if_t + testIterate (Tup const& tup, beast::unit_test::suite& suite) + { + Func func; + func (tup, suite); + testIterate::value - 1, I2 - 1, Func>(tup, suite); + } + + // Finish iteration over the tuple. + template class Func, typename Tup> + std::enable_if_t + testIterate (Tup const& tup, beast::unit_test::suite& suite) + { + Func func; + func (tup, suite); + } + + void testConversion() + { + // Verify that valid conversions are valid and invalid conversions + // are not valid. + + // Examples of each kind of enum. + static auto const terEnums = std::make_tuple (telLOCAL_ERROR, + temMALFORMED, tefFAILURE, terRETRY, tesSUCCESS, tecCLAIM); + static const int hiIndex { + std::tuple_size::value - 1}; + + // Verify that enums cannot be converted to other enum types. + testIterate (terEnums, *this); + + // Lambda that verifies assignability and convertibility. + auto isConvertable = [] (auto from, auto to) + { + using From_t = std::decay_t; + using To_t = std::decay_t; + static_assert ( + std::is_convertible::value, "Convert err"); + static_assert ( + std::is_constructible::value, "Construct err"); + static_assert ( + std::is_assignable::value, "Assign err"); + }; + + // Verify the right types convert to NotTEC. + NotTEC const notTec; + isConvertable (telLOCAL_ERROR, notTec); + isConvertable (temMALFORMED, notTec); + isConvertable (tefFAILURE, notTec); + isConvertable (terRETRY, notTec); + isConvertable (tesSUCCESS, notTec); + isConvertable (notTec, notTec); + + // Lambda that verifies types and not assignable or convertible. + auto notConvertible = [] (auto from, auto to) + { + using To_t = std::decay_t; + using From_t = std::decay_t; + static_assert ( + !std::is_convertible::value, "Convert err"); + static_assert ( + !std::is_constructible::value, "Construct err"); + static_assert ( + !std::is_assignable::value, "Assign err"); + }; + + // Verify types that shouldn't convert to NotTEC. + TER const ter; + notConvertible (tecCLAIM, notTec); + notConvertible (ter, notTec); + notConvertible (4, notTec); + + // Verify the right types convert to TER. + isConvertable (telLOCAL_ERROR, ter); + isConvertable (temMALFORMED, ter); + isConvertable (tefFAILURE, ter); + isConvertable (terRETRY, ter); + isConvertable (tesSUCCESS, ter); + isConvertable (tecCLAIM, ter); + isConvertable (notTec, ter); + isConvertable (ter, ter); + + // Verify that you can't convert from int to ter. + notConvertible (4, ter); + } + + // Helper template that makes sure two types are comparable. Also + // verifies that one of the types does not compare to int. + // o I1 one tuple index. + // o I2 other tuple index. + // o Tup is expected to be a tuple. + // It's a functor, rather than a function template, since a class template + // can be a template argument without being full specified. + template + class CheckComparable + { + public: + template + void operator()(Tup const& tup, beast::unit_test::suite& suite) const + { + // All entries in the tuple should be comparable one to the other. + auto const lhs = std::get(tup); + auto const rhs = std::get(tup); + + static_assert (std::is_same< + decltype (operator== (lhs, rhs)), bool>::value, "== err"); + + static_assert (std::is_same< + decltype (operator!= (lhs, rhs)), bool>::value, "!= err"); + + static_assert (std::is_same< + decltype (operator< (lhs, rhs)), bool>::value, "< err"); + + static_assert (std::is_same< + decltype (operator<= (lhs, rhs)), bool>::value, "<= err"); + + static_assert (std::is_same< + decltype (operator> (lhs, rhs)), bool>::value, "> err"); + + static_assert (std::is_same< + decltype (operator>= (lhs, rhs)), bool>::value, ">= err"); + + // Make sure a sampling of TER types exhibit the expected behavior + // for all comparison operators. + suite.expect ((lhs == rhs) == (TERtoInt (lhs) == TERtoInt (rhs))); + suite.expect ((lhs != rhs) == (TERtoInt (lhs) != TERtoInt (rhs))); + suite.expect ((lhs < rhs) == (TERtoInt (lhs) < TERtoInt (rhs))); + suite.expect ((lhs <= rhs) == (TERtoInt (lhs) <= TERtoInt (rhs))); + suite.expect ((lhs > rhs) == (TERtoInt (lhs) > TERtoInt (rhs))); + suite.expect ((lhs >= rhs) == (TERtoInt (lhs) >= TERtoInt (rhs))); + } + }; + + void testComparison() + { + // All of the TER-related types should be comparable. + + // Examples of all the types we expect to successfully compare. + static auto const ters = std::make_tuple (telLOCAL_ERROR, temMALFORMED, + tefFAILURE, terRETRY, tesSUCCESS, tecCLAIM, + NotTEC {telLOCAL_ERROR}, TER {tecCLAIM}); + static const int hiIndex { + std::tuple_size::value - 1}; + + // Verify that all types in the ters tuple can be compared with all + // the other types in ters. + testIterate (ters, *this); + } + void run() override { testTransResultInfo(); + testConversion(); + testComparison(); } }; BEAST_DEFINE_TESTSUITE(TER,protocol,ripple); -} +} //namespace ripple