diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index b93cf6347f1..50143d50444 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -119,9 +119,6 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash) // Notify inbound transactions of the new ledger sequence number inboundTransactions_.newRound(built->info().seq); - // Use the ledger timing rules of the acquired ledger - parms_.useRoundedCloseTime = built->rules().enabled(fix1528); - return RCLCxLedger(built); } @@ -913,9 +910,6 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr) // Notify inbound ledgers that we are starting a new round inboundTransactions_.newRound(prevLgr.seq()); - // Use parent ledger's rules to determine whether to use rounded close time - parms_.useRoundedCloseTime = prevLgr.ledger_->rules().enabled(fix1528); - // propose only if we're in sync with the network (and validating) return validating_ && synced; } diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index 3d737486693..fd85f91f006 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -999,8 +999,7 @@ BookStep::check(StrandContext const& ctx) const return temBAD_PATH_LOOP; } - if (ctx.view.rules().enabled(fix1373) && - ctx.seenDirectIssues[1].count(book_.out)) + if (ctx.seenDirectIssues[1].count(book_.out)) { JLOG(j_.debug()) << "BookStep: loop detected: " << *this; return temBAD_PATH_LOOP; diff --git a/src/ripple/app/paths/impl/PaySteps.cpp b/src/ripple/app/paths/impl/PaySteps.cpp index a7eb6da42cd..6d87c33a231 100644 --- a/src/ripple/app/paths/impl/PaySteps.cpp +++ b/src/ripple/app/paths/impl/PaySteps.cpp @@ -101,9 +101,7 @@ toStep ( JLOG (j.error()) << "Found offer/account payment step. Aborting payment strand."; assert (0); - if (ctx.view.rules().enabled(fix1373)) - return {temBAD_PATH, std::unique_ptr{}}; - Throw (tefEXCEPTION, "Found offer/account payment step."); + return {temBAD_PATH, std::unique_ptr{}}; } assert ((e2->getNodeType () & STPathElement::typeCurrency) || @@ -133,255 +131,7 @@ toStep ( } std::pair -toStrandV1 ( - ReadView const& view, - AccountID const& src, - AccountID const& dst, - Issue const& deliver, - boost::optional const& limitQuality, - boost::optional const& sendMaxIssue, - STPath const& path, - bool ownerPaysTransferFee, - bool offerCrossing, - beast::Journal j) -{ - if (isXRP (src)) - { - JLOG (j.debug()) << "toStrand with xrpAccount as src"; - return {temBAD_PATH, Strand{}}; - } - if (isXRP (dst)) - { - JLOG (j.debug()) << "toStrand with xrpAccount as dst"; - return {temBAD_PATH, Strand{}}; - } - if (!isConsistent (deliver)) - { - JLOG (j.debug()) << "toStrand inconsistent deliver issue"; - return {temBAD_PATH, Strand{}}; - } - if (sendMaxIssue && !isConsistent (*sendMaxIssue)) - { - JLOG (j.debug()) << "toStrand inconsistent sendMax issue"; - return {temBAD_PATH, Strand{}}; - } - - Issue curIssue = [&] - { - auto& currency = - sendMaxIssue ? sendMaxIssue->currency : deliver.currency; - if (isXRP (currency)) - return xrpIssue (); - return Issue{currency, src}; - }(); - - STPathElement const firstNode ( - STPathElement::typeAll, src, curIssue.currency, curIssue.account); - - boost::optional sendMaxPE; - if (sendMaxIssue && sendMaxIssue->account != src && - (path.empty () || !path[0].isAccount () || - path[0].getAccountID () != sendMaxIssue->account)) - sendMaxPE.emplace (sendMaxIssue->account, boost::none, boost::none); - - STPathElement const lastNode (dst, boost::none, boost::none); - - auto hasCurrency = [](STPathElement const* pe) - { - return pe->getNodeType () & STPathElement::typeCurrency; - }; - - boost::optional deliverOfferNode; - boost::optional deliverAccountNode; - - std::vector pes; - // reserve enough for the path, the implied source, destination, - // sendmax and deliver. - pes.reserve (4 + path.size ()); - pes.push_back (&firstNode); - if (sendMaxPE) - pes.push_back (&*sendMaxPE); - for (auto& i : path) - pes.push_back (&i); - - // Note that for offer crossing (only) we do use an offer book even if - // all that is changing is the Issue.account. - STPathElement const* const lastCurrency = - *std::find_if (pes.rbegin(), pes.rend(), hasCurrency); - if ((lastCurrency->getCurrency() != deliver.currency) || - (offerCrossing && lastCurrency->getIssuerID() != deliver.account)) - { - deliverOfferNode.emplace (boost::none, deliver.currency, deliver.account); - pes.push_back (&*deliverOfferNode); - } - if (!((pes.back ()->isAccount() && pes.back ()->getAccountID () == deliver.account) || - (lastNode.isAccount() && lastNode.getAccountID () == deliver.account))) - { - deliverAccountNode.emplace (deliver.account, boost::none, boost::none); - pes.push_back (&*deliverAccountNode); - } - if (*pes.back() != lastNode) - pes.push_back (&lastNode); - - auto const strandSrc = firstNode.getAccountID (); - auto const strandDst = lastNode.getAccountID (); - bool const isDefaultPath = path.empty(); - - Strand result; - result.reserve (2 * pes.size ()); - - /* A strand may not include the same account node more than once - in the same currency. In a direct step, an account will show up - at most twice: once as a src and once as a dst (hence the two element array). - The strandSrc and strandDst will only show up once each. - */ - std::array, 2> seenDirectIssues; - // A strand may not include the same offer book more than once - boost::container::flat_set seenBookOuts; - seenDirectIssues[0].reserve (pes.size()); - seenDirectIssues[1].reserve (pes.size()); - seenBookOuts.reserve (pes.size()); - auto ctx = [&](bool isLast = false) - { - return StrandContext{view, result, strandSrc, strandDst, deliver, - limitQuality, isLast, ownerPaysTransferFee, offerCrossing, - isDefaultPath, seenDirectIssues, seenBookOuts, j}; - }; - - for (int i = 0; i < pes.size () - 1; ++i) - { - /* Iterate through the path elements considering them in pairs. - The first element of the pair is `cur` and the second element is - `next`. When an offer is one of the pairs, the step created will be for - `next`. This means when `cur` is an offer and `next` is an - account then no step is created, as a step has already been created for - that offer. - */ - boost::optional impliedPE; - auto cur = pes[i]; - auto next = pes[i + 1]; - - if (cur->isNone() || next->isNone()) - return {temBAD_PATH, Strand{}}; - - /* If both account and issuer are set, use the account - (and don't insert an implied node for the issuer). - This matches the behavior of the previous generation payment code - */ - if (cur->isAccount()) - curIssue.account = cur->getAccountID (); - else if (cur->hasIssuer()) - curIssue.account = cur->getIssuerID (); - - if (cur->hasCurrency()) - curIssue.currency = cur->getCurrency (); - - if (cur->isAccount() && next->isAccount()) - { - if (!isXRP (curIssue.currency) && - curIssue.account != cur->getAccountID () && - curIssue.account != next->getAccountID ()) - { - JLOG (j.trace()) << "Inserting implied account"; - auto msr = make_DirectStepI (ctx(), cur->getAccountID (), - curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - Currency dummy; - impliedPE.emplace (STPathElement::typeAccount, - curIssue.account, dummy, curIssue.account); - cur = &*impliedPE; - } - } - else if (cur->isAccount() && next->isOffer()) - { - if (curIssue.account != cur->getAccountID ()) - { - JLOG (j.trace()) << "Inserting implied account before offer"; - auto msr = make_DirectStepI (ctx(), cur->getAccountID (), - curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - Currency dummy; - impliedPE.emplace (STPathElement::typeAccount, - curIssue.account, dummy, curIssue.account); - cur = &*impliedPE; - } - } - else if (cur->isOffer() && next->isAccount()) - { - if (curIssue.account != next->getAccountID () && - !isXRP (next->getAccountID ())) - { - JLOG (j.trace()) << "Inserting implied account after offer"; - auto msr = make_DirectStepI (ctx(), curIssue.account, - next->getAccountID (), curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - continue; - } - - if (!next->isOffer() && - next->hasCurrency() && next->getCurrency () != curIssue.currency) - { - auto const& nextCurrency = next->getCurrency (); - auto const& nextIssuer = - next->hasIssuer () ? next->getIssuerID () : curIssue.account; - - if (isXRP (curIssue.currency)) - { - JLOG (j.trace()) << "Inserting implied XI offer"; - auto msr = make_BookStepXI ( - ctx(), {nextCurrency, nextIssuer}); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - else if (isXRP (nextCurrency)) - { - JLOG (j.trace()) << "Inserting implied IX offer"; - auto msr = make_BookStepIX (ctx(), curIssue); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - else - { - JLOG (j.trace()) << "Inserting implied II offer"; - auto msr = make_BookStepII ( - ctx(), curIssue, {nextCurrency, nextIssuer}); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - impliedPE.emplace ( - boost::none, nextCurrency, nextIssuer); - cur = &*impliedPE; - curIssue.currency = nextCurrency; - curIssue.account = nextIssuer; - } - - auto s = - toStep (ctx (/*isLast*/ i == pes.size () - 2), cur, next, curIssue); - if (s.first == tesSUCCESS) - result.emplace_back (std::move (s.second)); - else - { - JLOG (j.debug()) << "toStep failed: " << s.first; - return {s.first, Strand{}}; - } - } - - return {tesSUCCESS, std::move (result)}; -} - - -std::pair -toStrandV2 ( +toStrand ( ReadView const& view, AccountID const& src, AccountID const& dst, @@ -689,27 +439,6 @@ toStrandV2 ( return {tesSUCCESS, std::move (result)}; } -std::pair -toStrand ( - ReadView const& view, - AccountID const& src, - AccountID const& dst, - Issue const& deliver, - boost::optional const& limitQuality, - boost::optional const& sendMaxIssue, - STPath const& path, - bool ownerPaysTransferFee, - bool offerCrossing, - beast::Journal j) -{ - if (view.rules().enabled(fix1373)) - return toStrandV2(view, src, dst, deliver, limitQuality, - sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j); - else - return toStrandV1(view, src, dst, deliver, limitQuality, - sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j); -} - std::pair> toStrands ( ReadView const& view, diff --git a/src/ripple/app/tx/impl/ApplyContext.cpp b/src/ripple/app/tx/impl/ApplyContext.cpp index 5dc068eee1c..e3e1c1ef479 100644 --- a/src/ripple/app/tx/impl/ApplyContext.cpp +++ b/src/ripple/app/tx/impl/ApplyContext.cpp @@ -90,51 +90,50 @@ ApplyContext::checkInvariantsHelper( XRPAmount const fee, std::index_sequence) { - if (view_->rules().enabled(featureEnforceInvariants)) + try { auto checkers = getInvariantChecks(); - try - { - // call each check's per-entry method - visit ( - [&checkers]( - uint256 const& index, - bool isDelete, - std::shared_ptr const& before, - std::shared_ptr const& after) - { - (..., std::get(checkers).visitEntry(isDelete, before, after)); - }); - - // Note: do not replace this logic with a `...&&` fold expression. - // The fold expression will only run until the first check fails (it - // short-circuits). While the logic is still correct, the log - // message won't be. Every failed invariant should write to the log, - // not just the first one. - std::array finalizers{ - {std::get(checkers).finalize(tx, result, fee, *view_, journal)...}}; - - // call each check's finalizer to see that it passes - if (! std::all_of( finalizers.cbegin(), finalizers.cend(), - [](auto const& b) { return b; })) - { - JLOG(journal.fatal()) << - "Transaction has failed one or more invariants: " << - to_string(tx.getJson (JsonOptions::none)); - return failInvariantCheck (result); - } - } - catch(std::exception const& ex) + // call each check's per-entry method + visit ( + [&checkers]( + uint256 const& index, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) + { + (..., std::get(checkers).visitEntry(isDelete, before, after)); + }); + + // Note: do not replace this logic with a `...&&` fold expression. + // The fold expression will only run until the first check fails (it + // short-circuits). While the logic is still correct, the log + // message won't be. Every failed invariant should write to the log, + // not just the first one. + std::array finalizers{ + {std::get( + checkers).finalize(tx, result, fee, *view_, journal)...}}; + + // call each check's finalizer to see that it passes + if (! std::all_of( finalizers.cbegin(), finalizers.cend(), + [](auto const& b) { return b; })) { JLOG(journal.fatal()) << - "Transaction caused an exception in an invariant" << - ", ex: " << ex.what() << - ", tx: " << to_string(tx.getJson (JsonOptions::none)); + "Transaction has failed one or more invariants: " << + to_string(tx.getJson (JsonOptions::none)); return failInvariantCheck (result); } } + catch(std::exception const& ex) + { + JLOG(journal.fatal()) << + "Transaction caused an exception in an invariant" << + ", ex: " << ex.what() << + ", tx: " << to_string(tx.getJson (JsonOptions::none)); + + return failInvariantCheck (result); + } return result; } diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 2af6d57cf85..e5eaa0097b1 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -82,7 +82,7 @@ namespace ripple { @param now the current time @param mark the cutoff point - @return true if \a now refers to a time strictly after \a mark, false otherwise. + @return true if \a now refers to a time strictly after \a mark, else false. */ static inline bool after (NetClock::time_point now, std::uint32_t mark) { @@ -98,9 +98,6 @@ EscrowCreate::calculateMaxSpend(STTx const& tx) NotTEC EscrowCreate::preflight (PreflightContext const& ctx) { - if (! ctx.rules.enabled(featureEscrow)) - return temDISABLED; - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -254,18 +251,13 @@ EscrowCreate::doApply() } // If it's not a self-send, add escrow to recipient's owner directory. - if (ctx_.view ().rules().enabled(fix1523)) + if (auto const dest = ctx_.tx[sfDestination]; dest != ctx_.tx[sfAccount]) { - auto const dest = ctx_.tx[sfDestination]; - - if (dest != ctx_.tx[sfAccount]) - { - auto page = dirAdd(ctx_.view(), keylet::ownerDir(dest), slep->key(), - false, describeOwnerDir(dest), ctx_.app.journal ("View")); - if (!page) - return tecDIR_FULL; - (*slep)[sfDestinationNode] = *page; - } + auto page = dirAdd(ctx_.view(), keylet::ownerDir(dest), slep->key(), + false, describeOwnerDir(dest), ctx_.app.journal ("View")); + if (!page) + return tecDIR_FULL; + (*slep)[sfDestinationNode] = *page; } // Deduct owner's balance, increment owner count @@ -300,9 +292,6 @@ checkCondition (Slice f, Slice c) NotTEC EscrowFinish::preflight (PreflightContext const& ctx) { - if (! ctx.rules.enabled(featureEscrow)) - return temDISABLED; - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -488,10 +477,10 @@ EscrowFinish::doApply() } // Remove escrow from recipient's owner directory, if present. - if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) + if (auto const optPage = (*slep)[~sfDestinationNode]) { - auto const page = (*slep)[sfDestinationNode]; - if (! ctx_.view().dirRemove(keylet::ownerDir(destID), page, k.key, true)) + if (! ctx_.view().dirRemove( + keylet::ownerDir(destID), *optPage, k.key, true)) { return tefBAD_LEDGER; } @@ -518,9 +507,6 @@ EscrowFinish::doApply() NotTEC EscrowCancel::preflight (PreflightContext const& ctx) { - if (! ctx.rules.enabled(featureEscrow)) - return temDISABLED; - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -573,11 +559,10 @@ EscrowCancel::doApply() } // Remove escrow from recipient's owner directory, if present. - if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) + if (auto const optPage = (*slep)[~sfDestinationNode]; optPage) { - auto const page = (*slep)[sfDestinationNode]; if (! ctx_.view().dirRemove( - keylet::ownerDir((*slep)[sfDestination]), page, k.key, true)) + keylet::ownerDir((*slep)[sfDestination]), *optPage, k.key, true)) { return tefBAD_LEDGER; } diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index 8659e01435a..d64a7bb30f6 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -173,9 +173,6 @@ closeChannel ( NotTEC PayChanCreate::preflight (PreflightContext const& ctx) { - if (!ctx.rules.enabled (featurePayChan)) - return temDISABLED; - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -296,9 +293,6 @@ PayChanCreate::doApply() NotTEC PayChanFund::preflight (PreflightContext const& ctx) { - if (!ctx.rules.enabled (featurePayChan)) - return temDISABLED; - if (ctx.rules.enabled(fix1543) && ctx.tx.getFlags() & tfUniversalMask) return temINVALID_FLAG; @@ -390,15 +384,6 @@ PayChanFund::doApply() NotTEC PayChanClaim::preflight (PreflightContext const& ctx) { - if (! ctx.rules.enabled(featurePayChan)) - return temDISABLED; - - // 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)) return ret; diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 93c27a616f4..aac7d109f15 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -122,7 +122,7 @@ SetAccount::preflight (PreflightContext const& ctx) return temBAD_TRANSFER_RATE; } - if (ctx.rules.enabled(fix1201) && (uRate > 2 * QUALITY_ONE)) + if (uRate > 2 * QUALITY_ONE) { JLOG(j.trace()) << "Malformed transaction: Transfer rate too large."; return temBAD_TRANSFER_RATE; @@ -132,9 +132,6 @@ SetAccount::preflight (PreflightContext const& ctx) // TickSize if (tx.isFieldPresent (sfTickSize)) { - if (!ctx.rules.enabled(featureTickSize)) - return temDISABLED; - auto uTickSize = tx[sfTickSize]; if (uTickSize && ((uTickSize < Quality::minTickSize) || diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index 2cbf7891027..c6c03fea80a 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -1697,10 +1697,7 @@ template NetClock::time_point Consensus::asCloseTime(NetClock::time_point raw) const { - if (adaptor_.parms().useRoundedCloseTime) - return roundCloseTime(raw, closeResolution_); - else - return effCloseTime(raw, closeResolution_, previousLedger_.closeTime()); + return roundCloseTime(raw, closeResolution_); } } // namespace ripple diff --git a/src/ripple/consensus/ConsensusParms.h b/src/ripple/consensus/ConsensusParms.h index 6e2d88f92bf..94e8d3ff45b 100644 --- a/src/ripple/consensus/ConsensusParms.h +++ b/src/ripple/consensus/ConsensusParms.h @@ -138,17 +138,6 @@ struct ConsensusParms //! Percentage of nodes required to reach agreement on ledger close time std::size_t avCT_CONSENSUS_PCT = 75; - - //-------------------------------------------------------------------------- - - /** Whether to use roundCloseTime or effCloseTime for reaching close time - consensus. - This was added to migrate from effCloseTime to roundCloseTime on the - live network. The desired behavior (as given by the default value) is - to use roundCloseTime during consensus voting and then use effCloseTime - when accepting the consensus ledger. - */ - bool useRoundedCloseTime = true; }; } // ripple diff --git a/src/ripple/ledger/impl/PaymentSandbox.cpp b/src/ripple/ledger/impl/PaymentSandbox.cpp index f2c57818483..6339cd75fe1 100644 --- a/src/ripple/ledger/impl/PaymentSandbox.cpp +++ b/src/ripple/ledger/impl/PaymentSandbox.cpp @@ -180,7 +180,6 @@ PaymentSandbox::balanceHook (AccountID const& account, auto const currency = amount.getCurrency (); - auto adjustedAmt = amount; auto delta = amount.zeroed (); auto lastBal = amount; auto minBal = amount; @@ -194,16 +193,12 @@ PaymentSandbox::balanceHook (AccountID const& account, minBal = lastBal; } } - adjustedAmt = std::min(amount, lastBal - delta); - if (rules().enabled(fix1368)) - { - // The adjusted amount should never be larger than the balance. In - // some circumstances, it is possible for the deferred credits table - // to compute usable balance just slightly above what the ledger - // calculates (but always less than the actual balance). - adjustedAmt = std::min(adjustedAmt, minBal); - } + // The adjusted amount should never be larger than the balance. In + // some circumstances, it is possible for the deferred credits table + // to compute usable balance just slightly above what the ledger + // calculates (but always less than the actual balance). + auto adjustedAmt = std::min({amount, lastBal - delta, minBal}); adjustedAmt.setIssuer(amount.getIssuer()); if (isXRP(issuer) && adjustedAmt < beast::zero) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index cb964037e85..6ddb5578739 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -713,104 +713,10 @@ dirAdd (ApplyView& view, std::function fDescriber, beast::Journal j) { - if (view.rules().enabled(featureSortedDirectories)) - { - if (strictOrder) - return view.dirAppend(dir, uLedgerIndex, fDescriber); - else - return view.dirInsert(dir, uLedgerIndex, fDescriber); - } - - JLOG (j.trace()) << "dirAdd:" << - " dir=" << to_string (dir.key) << - " uLedgerIndex=" << to_string (uLedgerIndex); - - auto sleRoot = view.peek(dir); - std::uint64_t uNodeDir = 0; - - if (! sleRoot) - { - // No root, make it. - sleRoot = std::make_shared(dir); - sleRoot->setFieldH256 (sfRootIndex, dir.key); - view.insert (sleRoot); - fDescriber (sleRoot); - - STVector256 v; - v.push_back (uLedgerIndex); - sleRoot->setFieldV256 (sfIndexes, v); - - JLOG (j.trace()) << - "dirAdd: created root " << to_string (dir.key) << - " for entry " << to_string (uLedgerIndex); - - return uNodeDir; - } - - SLE::pointer sleNode; - STVector256 svIndexes; - - uNodeDir = sleRoot->getFieldU64 (sfIndexPrevious); // Get index to last directory node. - - if (uNodeDir) - { - // Try adding to last node. - sleNode = view.peek (keylet::page(dir, uNodeDir)); - assert (sleNode); - } - else - { - // Try adding to root. Didn't have a previous set to the last node. - sleNode = sleRoot; - } - - svIndexes = sleNode->getFieldV256 (sfIndexes); - - if (dirNodeMaxEntries != svIndexes.size ()) - { - // Add to current node. - view.update(sleNode); - } - // Add to new node. - else if (!++uNodeDir) - { - return boost::none; - } - else - { - // Have old last point to new node - sleNode->setFieldU64 (sfIndexNext, uNodeDir); - view.update(sleNode); - - // Have root point to new node. - sleRoot->setFieldU64 (sfIndexPrevious, uNodeDir); - view.update (sleRoot); - - // Create the new node. - sleNode = std::make_shared( - keylet::page(dir, uNodeDir)); - sleNode->setFieldH256 (sfRootIndex, dir.key); - view.insert (sleNode); - - if (uNodeDir != 1) - sleNode->setFieldU64 (sfIndexPrevious, uNodeDir - 1); - - fDescriber (sleNode); - - svIndexes = STVector256 (); - } - - svIndexes.push_back (uLedgerIndex); // Append entry. - sleNode->setFieldV256 (sfIndexes, svIndexes); // Save entry. - - JLOG (j.trace()) << - "dirAdd: creating: root: " << to_string (dir.key); - JLOG (j.trace()) << - "dirAdd: appending: Entry: " << to_string (uLedgerIndex); - JLOG (j.trace()) << - "dirAdd: appending: Node: " << strHex (uNodeDir); + if (strictOrder) + return view.dirAppend(dir, uLedgerIndex, fDescriber); - return uNodeDir; + return view.dirInsert(dir, uLedgerIndex, fDescriber); } TER diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 3a7ee97862d..180f02bdd88 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -76,7 +76,6 @@ class FeatureCollections "TrustSetAuth", // Unconditionally supported. "FeeEscalation", // Unconditionally supported. "OwnerPaysFee", - "CompareFlowV1V2", "PayChan", "Flow", // Unconditionally supported. "CompareTakerFlowCross", @@ -366,24 +365,11 @@ foreachFeature(FeatureBitset bs, F&& f) extern uint256 const featureTickets; extern uint256 const featureOwnerPaysFee; -extern uint256 const featureCompareFlowV1V2; -extern uint256 const featurePayChan; extern uint256 const featureFlow; extern uint256 const featureCompareTakerFlowCross; extern uint256 const featureFlowCross; -extern uint256 const featureCryptoConditions; -extern uint256 const featureTickSize; -extern uint256 const fix1368; -extern uint256 const featureEscrow; extern uint256 const featureCryptoConditionsSuite; -extern uint256 const fix1373; -extern uint256 const featureEnforceInvariants; -extern uint256 const featureSortedDirectories; -extern uint256 const fix1201; -extern uint256 const fix1512; extern uint256 const fix1513; -extern uint256 const fix1523; -extern uint256 const fix1528; extern uint256 const featureDepositAuth; extern uint256 const featureChecks; extern uint256 const fix1571; @@ -402,6 +388,21 @@ extern uint256 const fixQualityUpperBound; extern uint256 const featureRequireFullyCanonicalSig; extern uint256 const fix1781; +// The following amendments have been active for at least two years. +// Their pre-amendment code has been removed. +extern uint256 const retiredPayChan; +extern uint256 const retiredCryptoConditions; +extern uint256 const retiredTickSize; +extern uint256 const retiredFix1368; +extern uint256 const retiredEscrow; +extern uint256 const retiredFix1373; +extern uint256 const retiredEnforceInvariants; +extern uint256 const retiredSortedDirectories; +extern uint256 const retiredFix1201; +extern uint256 const retiredFix1512; +extern uint256 const retiredFix1523; +extern uint256 const retiredFix1528; + } // ripple #endif diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index bac6e2e8f33..6a7cfc18fd7 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -156,24 +156,11 @@ uint256 bitsetIndexToFeature(size_t i) uint256 const featureTickets = *getRegisteredFeature("Tickets"); uint256 const featureOwnerPaysFee = *getRegisteredFeature("OwnerPaysFee"); -uint256 const featureCompareFlowV1V2 = *getRegisteredFeature("CompareFlowV1V2"); -uint256 const featurePayChan = *getRegisteredFeature("PayChan"); uint256 const featureFlow = *getRegisteredFeature("Flow"); uint256 const featureCompareTakerFlowCross = *getRegisteredFeature("CompareTakerFlowCross"); uint256 const featureFlowCross = *getRegisteredFeature("FlowCross"); -uint256 const featureCryptoConditions = *getRegisteredFeature("CryptoConditions"); -uint256 const featureTickSize = *getRegisteredFeature("TickSize"); -uint256 const fix1368 = *getRegisteredFeature("fix1368"); -uint256 const featureEscrow = *getRegisteredFeature("Escrow"); uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoConditionsSuite"); -uint256 const fix1373 = *getRegisteredFeature("fix1373"); -uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariants"); -uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); -uint256 const fix1201 = *getRegisteredFeature("fix1201"); -uint256 const fix1512 = *getRegisteredFeature("fix1512"); uint256 const fix1513 = *getRegisteredFeature("fix1513"); -uint256 const fix1523 = *getRegisteredFeature("fix1523"); -uint256 const fix1528 = *getRegisteredFeature("fix1528"); uint256 const featureDepositAuth = *getRegisteredFeature("DepositAuth"); uint256 const featureChecks = *getRegisteredFeature("Checks"); uint256 const fix1571 = *getRegisteredFeature("fix1571"); @@ -192,4 +179,19 @@ uint256 const fixQualityUpperBound = *getRegisteredFeature("fixQualityUpperBound uint256 const featureRequireFullyCanonicalSig = *getRegisteredFeature("RequireFullyCanonicalSig"); uint256 const fix1781 = *getRegisteredFeature("fix1781"); +// The following amendments have been active for at least two years. +// Their pre-amendment code has been removed. +uint256 const retiredPayChan = *getRegisteredFeature("PayChan"); +uint256 const retiredCryptoConditions = *getRegisteredFeature("CryptoConditions"); +uint256 const retiredTickSize = *getRegisteredFeature("TickSize"); +uint256 const retiredFix1368 = *getRegisteredFeature("fix1368"); +uint256 const retiredEscrow = *getRegisteredFeature("Escrow"); +uint256 const retiredFix1373 = *getRegisteredFeature("fix1373"); +uint256 const retiredEnforceInvariants = *getRegisteredFeature("EnforceInvariants"); +uint256 const retiredSortedDirectories = *getRegisteredFeature("SortedDirectories"); +uint256 const retiredFix1201 = *getRegisteredFeature("fix1201"); +uint256 const retiredFix1512 = *getRegisteredFeature("fix1512"); +uint256 const retiredFix1523 = *getRegisteredFeature("fix1523"); +uint256 const retiredFix1528 = *getRegisteredFeature("fix1528"); + } // ripple diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index 6f8a304fd6a..f25cb02dcbe 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -528,9 +528,8 @@ class CrossingLimits_test : public beast::unit_test::suite }; using namespace jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); - testAll(sa ); + testAll(sa - featureFlowCross); + testAll(sa ); } }; diff --git a/src/test/app/DeliverMin_test.cpp b/src/test/app/DeliverMin_test.cpp index af05d7f79ac..6309061c8ba 100644 --- a/src/test/app/DeliverMin_test.cpp +++ b/src/test/app/DeliverMin_test.cpp @@ -112,8 +112,7 @@ class DeliverMin_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - test_convert_all_of_an_asset(sa - fix1373 - featureFlowCross); - test_convert_all_of_an_asset(sa - featureFlowCross); + test_convert_all_of_an_asset(sa - featureFlowCross); test_convert_all_of_an_asset(sa); } }; diff --git a/src/test/app/Discrepancy_test.cpp b/src/test/app/Discrepancy_test.cpp index 999b00e43dc..2e6223d7a1d 100644 --- a/src/test/app/Discrepancy_test.cpp +++ b/src/test/app/Discrepancy_test.cpp @@ -146,8 +146,7 @@ class Discrepancy_test : public beast::unit_test::suite { using namespace test::jtx; auto const sa = supported_amendments(); - testXRPDiscrepancy (sa - fix1373 - featureFlowCross); - testXRPDiscrepancy (sa - featureFlowCross); + testXRPDiscrepancy (sa - featureFlowCross); testXRPDiscrepancy (sa); } }; diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index c323abf7550..6d78986c548 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -215,36 +215,25 @@ struct Escrow_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; - { // Escrow not enabled - Env env(*this, supported_amendments() - featureEscrow); - env.fund(XRP(5000), "alice", "bob"); - env(escrow("alice", "bob", XRP(1000)), - finish_time(env.now() + 1s), ter(temDISABLED)); - env(finish("bob", "alice", 1), ter(temDISABLED)); - env(cancel("bob", "alice", 1), ter(temDISABLED)); - } - - { // Escrow enabled - Env env(*this); - env.fund(XRP(5000), "alice", "bob"); - env(escrow("alice", "bob", XRP(1000)), finish_time(env.now() + 1s)); - env.close(); + Env env(*this); + env.fund(XRP(5000), "alice", "bob"); + env(escrow("alice", "bob", XRP(1000)), finish_time(env.now() + 1s)); + env.close(); - auto const seq1 = env.seq("alice"); + auto const seq1 = env.seq("alice"); - env(escrow("alice", "bob", XRP(1000)), condition (cb1), - finish_time(env.now() + 1s), fee(1500)); - env.close(); - env(finish("bob", "alice", seq1), - condition(cb1), fulfillment(fb1), fee(1500)); + env(escrow("alice", "bob", XRP(1000)), condition (cb1), + finish_time(env.now() + 1s), fee(1500)); + env.close(); + env(finish("bob", "alice", seq1), + condition(cb1), fulfillment(fb1), fee(1500)); - auto const seq2 = env.seq("alice"); + auto const seq2 = env.seq("alice"); - env(escrow("alice", "bob", XRP(1000)), condition(cb2), - finish_time(env.now() + 1s), cancel_time(env.now() + 2s), fee(1500)); - env.close(); - env(cancel("bob", "alice", seq2), fee(1500)); - } + env(escrow("alice", "bob", XRP(1000)), condition(cb2), + finish_time(env.now() + 1s), cancel_time(env.now() + 2s), fee(1500)); + env.close(); + env(cancel("bob", "alice", seq2), fee(1500)); } void @@ -1067,28 +1056,7 @@ struct Escrow_test : public beast::unit_test::suite auto const carol = Account("carol"); { - testcase ("Metadata & Ownership (without fix1523)"); - Env env(*this, supported_amendments() - fix1523); - env.fund(XRP(5000), alice, bruce, carol); - - auto const seq = env.seq(alice); - env(escrow(alice, carol, XRP(1000)), finish_time(env.now() + 1s)); - - BEAST_EXPECT((*env.meta())[sfTransactionResult] == - static_cast(tesSUCCESS)); - - auto const escrow = env.le(keylet::escrow(alice.id(), seq)); - BEAST_EXPECT(escrow); - - ripple::Dir aod (*env.current(), keylet::ownerDir(alice.id())); - BEAST_EXPECT(std::distance(aod.begin(), aod.end()) == 1); - BEAST_EXPECT(std::find(aod.begin(), aod.end(), escrow) != aod.end()); - - ripple::Dir cod (*env.current(), keylet::ownerDir(carol.id())); - BEAST_EXPECT(cod.begin() == cod.end()); - } - { - testcase ("Metadata (with fix1523, to self)"); + testcase ("Metadata to self"); Env env(*this); env.fund(XRP(5000), alice, bruce, carol); @@ -1152,7 +1120,7 @@ struct Escrow_test : public beast::unit_test::suite } } { - testcase ("Metadata (with fix1523, to other)"); + testcase ("Metadata to other"); Env env(*this); env.fund(XRP(5000), alice, bruce, carol); @@ -1277,139 +1245,6 @@ struct Escrow_test : public beast::unit_test::suite } } - void testDeletedDestination() - { - // Make sure an Escrow behaves as expected if its destination is - // deleted from the ledger. This can only happen if fix1523 is - // not enabled. fix1523 is what adds the Escrow to the Escrow's - // destination directory. - testcase ("Deleted destination"); - - using namespace jtx; - using namespace std::chrono; - - Account const alice {"alice"}; - Account const bruce {"bruce"}; - Account const carol {"carol"}; - Account const daria {"daria"}; - Account const evita {"evita"}; - Account const fiona {"fiona"}; - Account const grace {"grace"}; - - Env env(*this, supported_amendments() - fix1523); - - env.fund(XRP(5000), alice, bruce, carol, daria, evita, fiona, grace); - env.close(); - - // Because fix1523 is not in play, bruce does not have a back link - // from his directory to the escrow. So we should be able to delete - // bruce's account even though he is the destination of an escrow. - std::uint32_t const aliceEscrowSeq {env.seq(alice)}; - env(escrow(alice, bruce, XRP(1000)), finish_time(env.now() + 1s)); - - // Similar to bruce, we should be able to delete daria's account. - std::uint32_t const carolEscrowSeq {env.seq(carol)}; - env(escrow(carol, daria, XRP(1000)), - finish_time(env.now() + 1s), cancel_time(env.now() + 2s)); - env.close(); - - // Now engage fix1523 so any additional escrows we make will have - // back links from both the source and the destination. - env.enableFeature (fix1523); - env.close(); - - // Neither evita not fiona should be able to delete their accounts - // once this escrow is created. - std::uint32_t const evitaEscrowSeq {env.seq(evita)}; - env(escrow(evita, fiona, XRP(1000)), finish_time(env.now() + 1s)); - env.close(); - - // Close enough ledgers to be able to delete any of the accounts. - { - std::uint32_t const openLedgerSeq {env.current()->seq()}; - int const delta = [&]()->int { - if (env.seq(alice) + 255 > openLedgerSeq) - return env.seq(alice) - openLedgerSeq + 255; - return 0; - }(); - for (int i = 0; i < delta; ++i) - env.close(); - } - - // The presence of escrows should prevent all of the accounts from - // being deleted except for bruce and daria. - auto const acctDelFee {drops (env.current()->fees().increment)}; - env (acctdelete (alice, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env (acctdelete (bruce, grace), fee (acctDelFee)); - env (acctdelete (carol, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env (acctdelete (daria, grace), fee (acctDelFee)); - env (acctdelete (evita, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env (acctdelete (fiona, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env.close(); - - // At this point the destination of alice's escrow is missing. Make - // sure the escrow still behaves well. In particular, an EscrowFinish - // should fail but leave the Escrow object in the ledger. - env(finish(alice, alice, aliceEscrowSeq), ter(tecNO_DST)); - env.close(); - - // Verify that alice's escrow remains in the ledger. - Keylet const aliceEscrowKey { - keylet::escrow (alice.id(), aliceEscrowSeq)}; - BEAST_EXPECT (env.current()->exists (aliceEscrowKey)); - - // Since alice still has the escrow she should not be able to delete - // her account. - env (acctdelete (alice, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - - // The fact that the destination of carol's escrow has evaporated - // should have no impact on whether carol's escrow can be canceled. - env(cancel(carol, carol, carolEscrowSeq)); - env.close(); - - // Now that carol's escrow is gone carol should be able to delete - // her account. - env (acctdelete (carol, grace), fee (acctDelFee)); - env.close(); - - // We'll now resurrect bruce's account. alice should then be able to - // finish her escrow. - env(pay (grace, bruce, XRP (5000))); - env.close(); - - // bruce's account should have returned to the ledger. - BEAST_EXPECT (env.current()->exists (keylet::account (bruce.id()))); - BEAST_EXPECT (env.balance (bruce) == XRP (5000)); - - // Verify that alice's escrow is still in the ledger. - BEAST_EXPECT (env.current()->exists (aliceEscrowKey)); - - // alice finishes her escrow to bruce's resurrected account. - env(finish(alice, alice, aliceEscrowSeq)); - env.close(); - - // Verify that alice's escrow is gone from the ledger. - BEAST_EXPECT (! env.current()->exists (aliceEscrowKey)); - - // Now alice can delete her account. - env (acctdelete (alice, grace), fee (acctDelFee)); - env.close(); - - // eveta and fiona are still prevented from deletion by evita's escrow. - env (acctdelete (evita, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env (acctdelete (fiona, grace), fee (acctDelFee), ter (tecHAS_OBLIGATIONS)); - env.close(); - - // Finish evita's escrow. Then both evita's and fiona's accounts can - // be deleted. - env(finish(fiona, evita, evitaEscrowSeq)); - env.close(); - - env (acctdelete (evita, grace), fee (acctDelFee)); - env (acctdelete (fiona, grace), fee (acctDelFee)); - env.close(); - } - void run() override { testEnablement(); @@ -1422,7 +1257,6 @@ struct Escrow_test : public beast::unit_test::suite testEscrowConditions(); testMetaAndOwnership(); testConsequences(); - testDeletedDestination(); } }; diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index bfd1460b4d4..094ab8d9133 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1291,7 +1291,7 @@ struct Flow_test : public beast::unit_test::suite testSelfFundedXRPEndpoint(false, features); testSelfFundedXRPEndpoint(true, features); testUnfundedOffer(features); - testReexecuteDirectStep(features | fix1368); + testReexecuteDirectStep(features); testSelfPayLowQualityOffer(features); } @@ -1304,8 +1304,7 @@ struct Flow_test : public beast::unit_test::suite using namespace jtx; auto const sa = supported_amendments(); - testWithFeats(sa - fix1373 - featureFlowCross); - testWithFeats(sa - featureFlowCross); + testWithFeats(sa - featureFlowCross); testWithFeats(sa); testEmptyStrand(sa); } @@ -1317,16 +1316,13 @@ struct Flow_manual_test : public Flow_test { using namespace jtx; auto const all = supported_amendments(); - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; - testWithFeats(all - f1373 - flowCross - f1513); - testWithFeats(all - f1373 - flowCross ); - testWithFeats(all - flowCross - f1513); - testWithFeats(all - flowCross ); - testWithFeats(all - f1513); - testWithFeats(all ); + testWithFeats(all - flowCross - f1513); + testWithFeats(all - flowCross ); + testWithFeats(all - f1513); + testWithFeats(all ); testEmptyStrand(all - f1513); testEmptyStrand(all ); diff --git a/src/test/app/Freeze_test.cpp b/src/test/app/Freeze_test.cpp index 9b6cf5f82c0..d87d979671b 100644 --- a/src/test/app/Freeze_test.cpp +++ b/src/test/app/Freeze_test.cpp @@ -545,8 +545,7 @@ class Freeze_test : public beast::unit_test::suite }; using namespace test::jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); + testAll(sa - featureFlowCross); testAll(sa); } }; diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 2012e6b1159..9e69beede62 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -4420,20 +4420,6 @@ class Offer_test : public beast::unit_test::suite using namespace jtx; - // Should be called with TickSize enabled. - BEAST_EXPECT(features[featureTickSize]); - - // Try to set tick size without enabling feature - { - Env env{*this, features - featureTickSize}; - auto const gw = Account {"gateway"}; - env.fund (XRP(10000), gw); - - auto txn = noop(gw); - txn[sfTickSize.fieldName] = 0; - env(txn, ter(temDISABLED)); - } - // Try to set tick size out of range { Env env {*this, features}; @@ -4609,21 +4595,20 @@ class Offer_test : public beast::unit_test::suite testRCSmoketest (features); testSelfAuth (features); testDeletedOfferIssuer (features); - testTickSize (features | featureTickSize); + testTickSize (features); } void run () override { using namespace jtx; FeatureBitset const all{supported_amendments()}; - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; - testAll(all - f1373 - takerDryOffer); - testAll(all - flowCross - takerDryOffer); - testAll(all - flowCross ); - testAll(all ); + testAll(all - takerDryOffer); + testAll(all - flowCross - takerDryOffer); + testAll(all - flowCross ); + testAll(all ); testFalseAssert(); } }; @@ -4634,21 +4619,16 @@ class Offer_manual_test : public Offer_test { using namespace jtx; FeatureBitset const all{supported_amendments()}; - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; - testAll(all - f1373 - flowCross - f1513); - testAll(all - f1373 - flowCross ); - testAll(all - f1373 - f1513); - testAll(all - f1373 ); - testAll(all - flowCross - f1513); - testAll(all - flowCross ); - testAll(all - f1513); - testAll(all ); + testAll(all - flowCross - f1513); + testAll(all - flowCross ); + testAll(all - f1513); + testAll(all ); - testAll(all - f1373 - flowCross - takerDryOffer); + testAll(all - flowCross - takerDryOffer); } }; diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 59f8967e136..9ab03de2fb6 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -1050,7 +1050,6 @@ struct PayStrand_test : public beast::unit_test::suite auto const USD = gw["USD"]; auto const EUR = gw["EUR"]; - if (features[fix1373]) { Env env(*this, features); env.fund(XRP(10000), alice, bob, gw); @@ -1147,17 +1146,12 @@ 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 = [&] () -> TER { - if (!features[fix1373]) - return tesSUCCESS; - return temBAD_PATH_LOOP; - }(); // payment path: USD -> USD/XRP -> XRP/USD env(pay(alice, carol, USD(100)), sendmax(USD(100)), path(~XRP, ~USD), txflags(tfNoRippleDirect), - ter(expectedResult)); + ter(temBAD_PATH_LOOP)); } { Env env(*this, features); @@ -1245,16 +1239,13 @@ struct PayStrand_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testToStrand(sa - fix1373 - featureFlowCross); - testToStrand(sa - featureFlowCross); + testToStrand(sa - featureFlowCross); testToStrand(sa); - testRIPD1373(sa - fix1373 - featureFlowCross); - testRIPD1373(sa - featureFlowCross); + testRIPD1373(sa - featureFlowCross); testRIPD1373(sa); - testLoop(sa - fix1373 - featureFlowCross); - testLoop(sa - featureFlowCross); + testLoop(sa - featureFlowCross); testLoop(sa); testNoAccount(sa); diff --git a/src/test/app/SetAuth_test.cpp b/src/test/app/SetAuth_test.cpp index f9a2134862f..6b4a2120ba1 100644 --- a/src/test/app/SetAuth_test.cpp +++ b/src/test/app/SetAuth_test.cpp @@ -70,8 +70,7 @@ struct SetAuth_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testAuth(sa - fix1373 - featureFlowCross); - testAuth(sa - featureFlowCross); + testAuth(sa - featureFlowCross); testAuth(sa); } }; diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index bc65dec2aa9..06215eafe6a 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -510,8 +510,7 @@ class TrustAndBalance_test : public beast::unit_test::suite using namespace test::jtx; auto const sa = supported_amendments(); - testWithFeatures(sa - fix1373 - featureFlowCross); - testWithFeatures(sa -featureFlowCross); + testWithFeatures(sa -featureFlowCross); testWithFeatures(sa); } }; diff --git a/src/test/consensus/Consensus_test.cpp b/src/test/consensus/Consensus_test.cpp index 6075a320d94..6dab15685ea 100644 --- a/src/test/consensus/Consensus_test.cpp +++ b/src/test/consensus/Consensus_test.cpp @@ -592,153 +592,103 @@ class Consensus_test : public beast::unit_test::suite // This is a specialized test engineered to yield ledgers with different // close times even though the peers believe they had close time // consensus on the ledger. + ConsensusParms parms; - for (bool useRoundedCloseTime : {false, true}) - { - ConsensusParms parms; - parms.useRoundedCloseTime = useRoundedCloseTime; + Sim sim; - Sim sim; + // This requires a group of 4 fast and 2 slow peers to create a + // situation in which a subset of peers requires seeing additional + // proposals to declare consensus. + PeerGroup slow = sim.createGroup(2); + PeerGroup fast = sim.createGroup(4); + PeerGroup network = fast + slow; - // This requires a group of 4 fast and 2 slow peers to create a - // situation in which a subset of peers requires seeing additional - // proposals to declare consensus. - PeerGroup slow = sim.createGroup(2); - PeerGroup fast = sim.createGroup(4); - PeerGroup network = fast + slow; + for (Peer* peer : network) + peer->consensusParms = parms; - for (Peer* peer : network) - peer->consensusParms = parms; + // Connected trust graph + network.trust(network); - // Connected trust graph - network.trust(network); + // Fast and slow network connections + fast.connect( + fast, date::round(0.2 * parms.ledgerGRANULARITY)); + slow.connect( + network, + date::round(1.1 * parms.ledgerGRANULARITY)); - // Fast and slow network connections - fast.connect( - fast, date::round(0.2 * parms.ledgerGRANULARITY)); - slow.connect( - network, - date::round(1.1 * parms.ledgerGRANULARITY)); - - // Run to the ledger *prior* to decreasing the resolution - sim.run(increaseLedgerTimeResolutionEvery - 2); + // Run to the ledger *prior* to decreasing the resolution + sim.run(increaseLedgerTimeResolutionEvery - 2); - // In order to create the discrepency, we want a case where if - // X = effCloseTime(closeTime, resolution, parentCloseTime) - // X != effCloseTime(X, resolution, parentCloseTime) - // - // That is, the effective close time is not a fixed point. This can - // happen if X = parentCloseTime + 1, but a subsequent rounding goes - // to the next highest multiple of resolution. - - // So we want to find an offset (now + offset) % 30s = 15 - // (now + offset) % 20s = 15 - // This way, the next ledger will close and round up Due to the - // network delay settings, the round of consensus will take 5s, so - // the next ledger's close time will - - NetClock::duration when = network[0]->now().time_since_epoch(); - - // Check we are before the 30s to 20s transition - NetClock::duration resolution = - network[0]->lastClosedLedger.closeTimeResolution(); - BEAST_EXPECT(resolution == NetClock::duration{30s}); - - while ( - ((when % NetClock::duration{30s}) != NetClock::duration{15s}) || - ((when % NetClock::duration{20s}) != NetClock::duration{15s})) - when += 1s; - // Advance the clock without consensus running (IS THIS WHAT - // PREVENTS IT IN PRACTICE?) - sim.scheduler.step_for( - NetClock::time_point{when} - network[0]->now()); - - // Run one more ledger with 30s resolution - sim.run(1); - if (BEAST_EXPECT(sim.synchronized())) + // In order to create the discrepency, we want a case where if + // X = effCloseTime(closeTime, resolution, parentCloseTime) + // X != effCloseTime(X, resolution, parentCloseTime) + // + // That is, the effective close time is not a fixed point. This can + // happen if X = parentCloseTime + 1, but a subsequent rounding goes + // to the next highest multiple of resolution. + + // So we want to find an offset (now + offset) % 30s = 15 + // (now + offset) % 20s = 15 + // This way, the next ledger will close and round up Due to the + // network delay settings, the round of consensus will take 5s, so + // the next ledger's close time will + + NetClock::duration when = network[0]->now().time_since_epoch(); + + // Check we are before the 30s to 20s transition + NetClock::duration resolution = + network[0]->lastClosedLedger.closeTimeResolution(); + BEAST_EXPECT(resolution == NetClock::duration{30s}); + + while ( + ((when % NetClock::duration{30s}) != NetClock::duration{15s}) || + ((when % NetClock::duration{20s}) != NetClock::duration{15s})) + when += 1s; + // Advance the clock without consensus running (IS THIS WHAT + // PREVENTS IT IN PRACTICE?) + sim.scheduler.step_for( + NetClock::time_point{when} - network[0]->now()); + + // Run one more ledger with 30s resolution + sim.run(1); + if (BEAST_EXPECT(sim.synchronized())) + { + // close time should be ahead of clock time since we engineered + // the close time to round up + for (Peer* peer : network) { - // close time should be ahead of clock time since we engineered - // the close time to round up - for (Peer* peer : network) - { - BEAST_EXPECT( - peer->lastClosedLedger.closeTime() > peer->now()); - BEAST_EXPECT(peer->lastClosedLedger.closeAgree()); - } + BEAST_EXPECT( + peer->lastClosedLedger.closeTime() > peer->now()); + BEAST_EXPECT(peer->lastClosedLedger.closeAgree()); } + } - // All peers submit their own ID as a transaction - for (Peer* peer : network) - peer->submit(Tx{static_cast(peer->id)}); - - // Run 1 more round, this time it will have a decreased - // resolution of 20 seconds. + // All peers submit their own ID as a transaction + for (Peer* peer : network) + peer->submit(Tx{static_cast(peer->id)}); - // The network delays are engineered so that the slow peers - // initially have the wrong tx hash, but they see a majority - // of agreement from their peers and declare consensus - // - // The trick is that everyone starts with a raw close time of - // 84681s - // Which has - // effCloseTime(86481s, 20s, 86490s) = 86491s - // However, when the slow peers update their position, they change - // the close time to 86451s. The fast peers declare consensus with - // the 86481s as their position still. - // - // When accepted the ledger - // - fast peers use eff(86481s) -> 86491s as the close time - // - slow peers use eff(eff(86481s)) -> eff(86491s) -> 86500s! + // Run 1 more round, this time it will have a decreased + // resolution of 20 seconds. - sim.run(1); + // The network delays are engineered so that the slow peers + // initially have the wrong tx hash, but they see a majority + // of agreement from their peers and declare consensus + // + // The trick is that everyone starts with a raw close time of + // 84681s + // Which has + // effCloseTime(86481s, 20s, 86490s) = 86491s + // However, when the slow peers update their position, they change + // the close time to 86451s. The fast peers declare consensus with + // the 86481s as their position still. + // + // When accepted the ledger + // - fast peers use eff(86481s) -> 86491s as the close time + // - slow peers use eff(eff(86481s)) -> eff(86491s) -> 86500s! - if (parms.useRoundedCloseTime) - { - BEAST_EXPECT(sim.synchronized()); - } - else - { - // Not currently synchronized - BEAST_EXPECT(!sim.synchronized()); - - // All slow peers agreed on LCL - BEAST_EXPECT(std::all_of( - slow.begin(), slow.end(), [&slow](Peer const* p) { - return p->lastClosedLedger.id() == - slow[0]->lastClosedLedger.id(); - })); - - // All fast peers agreed on LCL - BEAST_EXPECT(std::all_of( - fast.begin(), fast.end(), [&fast](Peer const* p) { - return p->lastClosedLedger.id() == - fast[0]->lastClosedLedger.id(); - })); - - Ledger const& slowLCL = slow[0]->lastClosedLedger; - Ledger const& fastLCL = fast[0]->lastClosedLedger; - - // Agree on parent close and close resolution - BEAST_EXPECT( - slowLCL.parentCloseTime() == fastLCL.parentCloseTime()); - BEAST_EXPECT( - slowLCL.closeTimeResolution() == - fastLCL.closeTimeResolution()); + sim.run(1); - // Close times disagree ... - BEAST_EXPECT(slowLCL.closeTime() != fastLCL.closeTime()); - // Effective close times agree! The slow peer already rounded! - BEAST_EXPECT( - effCloseTime( - slowLCL.closeTime(), - slowLCL.closeTimeResolution(), - slowLCL.parentCloseTime()) == - effCloseTime( - fastLCL.closeTime(), - fastLCL.closeTimeResolution(), - fastLCL.parentCloseTime())); - } - } + BEAST_EXPECT(sim.synchronized()); } void diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 2ea9a335ace..b3c2c781141 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -748,23 +748,23 @@ class Env_test : public beast::unit_test::suite { // a Env FeatureBitset has *only* those features - Env env{*this, FeatureBitset(featureEscrow, featureFlow)}; + Env env{*this, FeatureBitset(featureMultiSignReserve, featureFlow)}; BEAST_EXPECT(env.app().config().features.size() == 2); foreachFeature(supported, [&](uint256 const& f) { - bool const has = (f == featureEscrow || f == featureFlow); + bool const has = (f == featureMultiSignReserve || f == featureFlow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } - auto const noFlowOrEscrow = - supported_amendments() - featureEscrow - featureFlow; + auto const missingSomeFeatures = + supported_amendments() - featureMultiSignReserve - featureFlow; { // a Env supported_features_except is missing *only* those features - Env env{*this, noFlowOrEscrow}; + Env env{*this, missingSomeFeatures}; BEAST_EXPECT( env.app().config().features.size() == (supported.count() - 2)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = (f == featureEscrow || f == featureFlow); + bool hasnot = (f == featureMultiSignReserve || f == featureFlow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } @@ -776,7 +776,8 @@ class Env_test : public beast::unit_test::suite // the two supported ones Env env{ *this, - FeatureBitset(featureEscrow, featureFlow, *neverSupportedFeat)}; + FeatureBitset( + featureMultiSignReserve, featureFlow, *neverSupportedFeat)}; // this app will have just 2 supported amendments and // one additional never supported feature flag @@ -784,7 +785,7 @@ class Env_test : public beast::unit_test::suite BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool has = (f == featureEscrow || f == featureFlow); + bool has = (f == featureMultiSignReserve || f == featureFlow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } @@ -794,7 +795,7 @@ class Env_test : public beast::unit_test::suite // and omit a few standard amendments // the unsupported features should be enabled Env env{*this, - noFlowOrEscrow | FeatureBitset{*neverSupportedFeat}}; + missingSomeFeatures | FeatureBitset{*neverSupportedFeat}}; // this app will have all supported amendments minus 2 and then the // one additional never supported feature flag @@ -803,7 +804,7 @@ class Env_test : public beast::unit_test::suite (supported.count() - 2 + 1)); BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = (f == featureEscrow || f == featureFlow); + bool hasnot = (f == featureMultiSignReserve || f == featureFlow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } diff --git a/src/test/ledger/BookDirs_test.cpp b/src/test/ledger/BookDirs_test.cpp index 2df04bf975d..15f556e5318 100644 --- a/src/test/ledger/BookDirs_test.cpp +++ b/src/test/ledger/BookDirs_test.cpp @@ -95,8 +95,7 @@ struct BookDirs_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - test_bookdir(sa - fix1373 - featureFlowCross); - test_bookdir(sa - featureFlowCross); + test_bookdir(sa - featureFlowCross); test_bookdir(sa); } }; diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index c3e2ab72d93..37a7f4257ed 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -85,94 +85,61 @@ struct Directory_test : public beast::unit_test::suite auto alice = Account("alice"); auto bob = Account("bob"); - { - testcase ("Directory Ordering (without 'SortedDirectories' amendment"); - - Env env( - *this, - supported_amendments().reset(featureSortedDirectories)); - env.fund(XRP(10000000), alice, bob, gw); - - // Insert 400 offers from Alice, then one from Bob: - std::uint32_t const firstOfferSeq {env.seq (alice)}; - for (std::size_t i = 1; i <= 400; ++i) - env(offer(alice, USD(10), XRP(10))); - - // Check Alice's directory: it should contain one - // entry for each offer she added. Within each - // page, the entries should be in sorted order. - { - auto dir = Dir(*env.current(), - keylet::ownerDir(alice)); + testcase ("Directory Ordering"); - std::uint32_t lastSeq = firstOfferSeq - 1; + Env env(*this); + env.fund(XRP(10000000), alice, gw); - // Check that the orders are sequential by checking - // that their sequence numbers are: - for (auto iter = dir.begin(); iter != std::end(dir); ++iter) { - BEAST_EXPECT(++lastSeq == (*iter)->getFieldU32(sfSequence)); - } - BEAST_EXPECT(lastSeq != 1); - } - } + std::uint32_t const firstOfferSeq {env.seq (alice)}; + for (std::size_t i = 1; i <= 400; ++i) + env(offer(alice, USD(i), XRP(i))); + env.close(); + // Check Alice's directory: it should contain one + // entry for each offer she added, and, within each + // page the entries should be in sorted order. { - testcase ("Directory Ordering (with 'SortedDirectories' amendment)"); + auto const view = env.closed(); - Env env(*this); - env.fund(XRP(10000000), alice, gw); + std::uint64_t page = 0; - std::uint32_t const firstOfferSeq {env.seq (alice)}; - for (std::size_t i = 1; i <= 400; ++i) - env(offer(alice, USD(i), XRP(i))); - env.close(); - - // Check Alice's directory: it should contain one - // entry for each offer she added, and, within each - // page the entries should be in sorted order. + do { - auto const view = env.closed(); + auto p = view->read(keylet::page(keylet::ownerDir(alice), page)); + + // Ensure that the entries in the page are sorted + auto const& v = p->getFieldV256(sfIndexes); + BEAST_EXPECT (std::is_sorted(v.begin(), v.end())); - std::uint64_t page = 0; + // Ensure that the page contains the correct orders by + // calculating which sequence numbers belong here. + std::uint32_t const minSeq = + firstOfferSeq + (page * dirNodeMaxEntries); + std::uint32_t const maxSeq = minSeq + dirNodeMaxEntries; - do + for (auto const& e : v) { - auto p = view->read(keylet::page(keylet::ownerDir(alice), page)); - - // Ensure that the entries in the page are sorted - auto const& v = p->getFieldV256(sfIndexes); - BEAST_EXPECT (std::is_sorted(v.begin(), v.end())); - - // Ensure that the page contains the correct orders by - // calculating which sequence numbers belong here. - std::uint32_t const minSeq = - firstOfferSeq + (page * dirNodeMaxEntries); - std::uint32_t const maxSeq = minSeq + dirNodeMaxEntries; - - for (auto const& e : v) - { - auto c = view->read(keylet::child(e)); - BEAST_EXPECT(c); - BEAST_EXPECT(c->getFieldU32(sfSequence) >= minSeq); - BEAST_EXPECT(c->getFieldU32(sfSequence) < maxSeq); - } - - page = p->getFieldU64(sfIndexNext); - } while (page != 0); - } + auto c = view->read(keylet::child(e)); + BEAST_EXPECT(c); + BEAST_EXPECT(c->getFieldU32(sfSequence) >= minSeq); + BEAST_EXPECT(c->getFieldU32(sfSequence) < maxSeq); + } - // Now check the orderbook: it should be in the order we placed - // the offers. - auto book = BookDirs(*env.current(), - Book({xrpIssue(), USD.issue()})); - int count = 1; + page = p->getFieldU64(sfIndexNext); + } while (page != 0); + } - for (auto const& offer : book) - { - count++; - BEAST_EXPECT(offer->getFieldAmount(sfTakerPays) == USD(count)); - BEAST_EXPECT(offer->getFieldAmount(sfTakerGets) == XRP(count)); - } + // Now check the orderbook: it should be in the order we placed + // the offers. + auto book = BookDirs(*env.current(), + Book({xrpIssue(), USD.issue()})); + int count = 1; + + for (auto const& offer : book) + { + count++; + BEAST_EXPECT(offer->getFieldAmount(sfTakerPays) == USD(count)); + BEAST_EXPECT(offer->getFieldAmount(sfTakerGets) == XRP(count)); } } diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp index 7cb100267ad..3fbedc64ddf 100644 --- a/src/test/ledger/Invariants_test.cpp +++ b/src/test/ledger/Invariants_test.cpp @@ -40,7 +40,7 @@ class Invariants_test : public beast::unit_test::suite ApplyContext& ac)>; void - doInvariantCheck( bool enabled, + doInvariantCheck( std::vector const& expect_logs, Precheck const& precheck, XRPAmount fee = XRPAmount{}, @@ -50,13 +50,6 @@ class Invariants_test : public beast::unit_test::suite { using namespace test::jtx; Env env {*this}; - if (! enabled) - { - auto& features = env.app().config().features; - auto it = features.find(featureEnforceInvariants); - if (it != features.end()) - features.erase(it); - } Account A1 {"A1"}; Account A2 {"A2"}; @@ -86,49 +79,26 @@ class Invariants_test : public beast::unit_test::suite for (TER const terExpect : ters) { terActual = ac.checkInvariants(terActual, fee); - if (enabled) + BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECT( + boost::starts_with(sink.messages().str(), "Invariant failed:") || + boost::starts_with(sink.messages().str(), + "Transaction caused an exception")); + //uncomment if you want to log the invariant failure message + //log << " --> " << sink.messages().str() << std::endl; + for (auto const& m : expect_logs) { - BEAST_EXPECT(terExpect == terActual); - BEAST_EXPECT( - boost::starts_with(sink.messages().str(), "Invariant failed:") || - boost::starts_with(sink.messages().str(), - "Transaction caused an exception")); - //uncomment if you want to log the invariant failure message - //log << " --> " << sink.messages().str() << std::endl; - for (auto const& m : expect_logs) - { - BEAST_EXPECT(sink.messages().str().find(m) != std::string::npos); - } - } - else - { - BEAST_EXPECT(terActual == tesSUCCESS); - BEAST_EXPECT(sink.messages().str().empty()); + BEAST_EXPECT(sink.messages().str().find(m) != std::string::npos); } } } void - testEnabled () + testXRPNotCreated () { using namespace test::jtx; - testcase ("feature enabled"); - Env env {*this}; - - auto hasInvariants = - env.app().config().features.find (featureEnforceInvariants); - BEAST_EXPECT(hasInvariants != env.app().config().features.end()); - - BEAST_EXPECT(env.current()->rules().enabled(featureEnforceInvariants)); - } - - void - testXRPNotCreated (bool enabled) - { - using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - XRP created"; - doInvariantCheck (enabled, + testcase << "XRP created"; + doInvariantCheck ( {{ "XRP net change was positive: 500" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -144,14 +114,13 @@ class Invariants_test : public beast::unit_test::suite } void - testAccountRootsNotRemoved(bool enabled) + testAccountRootsNotRemoved() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - account root removed"; + testcase << "account root removed"; // An account was deleted, but not by an AccountDelete transaction. - doInvariantCheck (enabled, + doInvariantCheck ( {{ "an account root was deleted" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -168,7 +137,7 @@ class Invariants_test : public beast::unit_test::suite // Note that this is a case where a second invocation of the invariant // checker returns a tecINVARIANT_FAILED, not a tefINVARIANT_FAILED. // After a discussion with the team, we believe that's okay. - doInvariantCheck (enabled, + doInvariantCheck ( {{ "account deletion succeeded without deleting an account" }}, [](Account const&, Account const&, ApplyContext& ac){return true;}, XRPAmount{}, @@ -176,7 +145,7 @@ class Invariants_test : public beast::unit_test::suite {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); // Successful AccountDelete that deleted more than one account. - doInvariantCheck (enabled, + doInvariantCheck ( {{ "account deletion succeeded but deleted multiple accounts" }}, [](Account const& A1, Account const& A2, ApplyContext& ac) { @@ -194,12 +163,11 @@ class Invariants_test : public beast::unit_test::suite } void - testTypesMatch(bool enabled) + testTypesMatch() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - LE types don't match"; - doInvariantCheck (enabled, + testcase << "ledger entry types don't match"; + doInvariantCheck ( {{ "ledger entry type mismatch" }, { "XRP net change of -1000000000 doesn't match fee 0" }}, [](Account const& A1, Account const&, ApplyContext& ac) @@ -213,7 +181,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "invalid ledger entry type added" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -233,12 +201,11 @@ class Invariants_test : public beast::unit_test::suite } void - testNoXRPTrustLine(bool enabled) + testNoXRPTrustLine() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - trust lines with XRP not allowed"; - doInvariantCheck (enabled, + testcase << "trust lines with XRP not allowed"; + doInvariantCheck ( {{ "an XRP trust line was created" }}, [](Account const& A1, Account const& A2, ApplyContext& ac) { @@ -252,13 +219,12 @@ class Invariants_test : public beast::unit_test::suite } void - testXRPBalanceCheck(bool enabled) + testXRPBalanceCheck() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - XRP balance checks"; + testcase << "XRP balance checks"; - doInvariantCheck (enabled, + doInvariantCheck ( {{ "Cannot return non-native STAmount as XRPAmount" }}, [](Account const& A1, Account const& A2, ApplyContext& ac) { @@ -272,7 +238,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "incorrect account XRP balance" }, { "XRP net change was positive: 99999999000000001" }}, [this](Account const& A1, Account const&, ApplyContext& ac) @@ -290,7 +256,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "incorrect account XRP balance" }, { "XRP net change of -1000000001 doesn't match fee 0" }}, [this](Account const& A1, Account const&, ApplyContext& ac) @@ -307,20 +273,19 @@ class Invariants_test : public beast::unit_test::suite } void - testTransactionFeeCheck(bool enabled) + testTransactionFeeCheck() { using namespace test::jtx; using namespace std::string_literals; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - Transaction fee checks"; + testcase << "Transaction fee checks"; - doInvariantCheck (enabled, + doInvariantCheck ( {{ "fee paid was negative: -1" }, { "XRP net change of 0 doesn't match fee -1" }}, [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{-1}); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "fee paid exceeds system limit: "s + to_string(INITIAL_XRP) }, { "XRP net change of 0 doesn't match fee "s + @@ -328,8 +293,7 @@ class Invariants_test : public beast::unit_test::suite [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{INITIAL_XRP}); - doInvariantCheck( - enabled, + doInvariantCheck ( {{"fee paid is 20 exceeds fee specified in transaction."}, {"XRP net change of 0 doesn't match fee 20"}}, [](Account const&, Account const&, ApplyContext&) { return true; }, @@ -340,13 +304,12 @@ class Invariants_test : public beast::unit_test::suite void - testNoBadOffers(bool enabled) + testNoBadOffers() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - no bad offers"; + testcase << "no bad offers"; - doInvariantCheck (enabled, + doInvariantCheck ( {{ "offer with a bad amount" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -364,7 +327,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "offer with a bad amount" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -383,7 +346,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "offer with a bad amount" }}, [](Account const& A1, Account const&, ApplyContext& ac) { @@ -404,13 +367,12 @@ class Invariants_test : public beast::unit_test::suite } void - testNoZeroEscrow(bool enabled) + testNoZeroEscrow() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - no zero escrow"; + testcase << "no zero escrow"; - doInvariantCheck (enabled, + doInvariantCheck ( {{ "Cannot return non-native STAmount as XRPAmount" }}, [](Account const& A1, Account const& A2, ApplyContext& ac) { @@ -426,7 +388,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "XRP net change of -1000000 doesn't match fee 0"}, { "escrow specifies invalid amount" }}, [](Account const& A1, Account const&, ApplyContext& ac) @@ -442,7 +404,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "XRP net change was positive: 100000000000000001" }, { "escrow specifies invalid amount" }}, [](Account const& A1, Account const&, ApplyContext& ac) @@ -463,13 +425,12 @@ class Invariants_test : public beast::unit_test::suite } void - testValidNewAccountRoot(bool enabled) + testValidNewAccountRoot() { using namespace test::jtx; - testcase << "checks " << (enabled ? "enabled" : "disabled") << - " - valid new account root"; + testcase << "valid new account root"; - doInvariantCheck (enabled, + doInvariantCheck ( {{ "account root created by a non-Payment" }}, [](Account const&, Account const&, ApplyContext& ac) { @@ -482,7 +443,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "multiple accounts created in a single transaction" }}, [](Account const&, Account const&, ApplyContext& ac) { @@ -502,7 +463,7 @@ class Invariants_test : public beast::unit_test::suite return true; }); - doInvariantCheck (enabled, + doInvariantCheck ( {{ "account created with wrong starting sequence number" }}, [](Account const&, Account const&, ApplyContext& ac) { @@ -521,22 +482,15 @@ class Invariants_test : public beast::unit_test::suite public: void run () override { - testEnabled (); - - // now run each invariant check test with - // the feature enabled and disabled - for(auto const& b : {false, true}) - { - testXRPNotCreated (b); - testAccountRootsNotRemoved (b); - testTypesMatch (b); - testNoXRPTrustLine (b); - testXRPBalanceCheck (b); - testTransactionFeeCheck(b); - testNoBadOffers (b); - testNoZeroEscrow (b); - testValidNewAccountRoot (b); - } + testXRPNotCreated (); + testAccountRootsNotRemoved (); + testTypesMatch (); + testNoXRPTrustLine (); + testXRPBalanceCheck (); + testTransactionFeeCheck(); + testNoBadOffers (); + testNoZeroEscrow (); + testValidNewAccountRoot (); } }; diff --git a/src/test/ledger/PaymentSandbox_test.cpp b/src/test/ledger/PaymentSandbox_test.cpp index 2f0ed7ce436..56bd189d91a 100644 --- a/src/test/ledger/PaymentSandbox_test.cpp +++ b/src/test/ledger/PaymentSandbox_test.cpp @@ -387,8 +387,7 @@ class PaymentSandbox_test : public beast::unit_test::suite }; using namespace jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); + testAll(sa - featureFlowCross); testAll(sa); } }; diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index ca763263437..8c893fcee0a 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -33,6 +33,8 @@ class AccountSet_test : public beast::unit_test::suite void testNullAccountSet() { + testcase ("No AccountSet"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -44,6 +46,8 @@ class AccountSet_test : public beast::unit_test::suite void testMostFlags() { + testcase ("Most Flags"); + using namespace test::jtx; Account const alice ("alice"); @@ -112,6 +116,8 @@ class AccountSet_test : public beast::unit_test::suite void testSetAndResetAccountTxnID() { + testcase ("Set and reset AccountTxnID"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -132,6 +138,8 @@ class AccountSet_test : public beast::unit_test::suite void testSetNoFreeze() { + testcase ("Set NoFreeze"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -150,6 +158,8 @@ class AccountSet_test : public beast::unit_test::suite void testDomain() { + testcase ("Domain"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -197,6 +207,8 @@ class AccountSet_test : public beast::unit_test::suite void testMessageKey() { + testcase ("MessageKey"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -219,6 +231,8 @@ class AccountSet_test : public beast::unit_test::suite void testWalletID() { + testcase ("WalletID"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -237,6 +251,8 @@ class AccountSet_test : public beast::unit_test::suite void testEmailHash() { + testcase ("EmailHash"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -262,6 +278,8 @@ class AccountSet_test : public beast::unit_test::suite double get; }; + testcase ("TransferRate"); + using namespace test::jtx; auto doTests = [this] (FeatureBitset const& features, std::initializer_list testData) @@ -274,6 +292,7 @@ class AccountSet_test : public beast::unit_test::suite for (auto const& r : testData) { env(rate(alice, r.set), ter(r.code)); + env.close(); // If the field is not present expect the default value if (!(*env.le(alice))[~sfTransferRate]) @@ -284,100 +303,117 @@ class AccountSet_test : public beast::unit_test::suite } }; - { - testcase ("Setting transfer rate (without fix1201)"); - doTests (supported_amendments().reset(fix1201), - { - { 1.0, tesSUCCESS, 1.0 }, - { 1.1, tesSUCCESS, 1.1 }, - { 2.0, tesSUCCESS, 2.0 }, - { 2.1, tesSUCCESS, 2.1 }, - { 0.0, tesSUCCESS, 1.0 }, - { 2.0, tesSUCCESS, 2.0 }, - { 0.9, temBAD_TRANSFER_RATE, 2.0 }}); - } - - { - testcase ("Setting transfer rate (with fix1201)"); - doTests (supported_amendments(), - { - { 1.0, tesSUCCESS, 1.0 }, - { 1.1, tesSUCCESS, 1.1 }, - { 2.0, tesSUCCESS, 2.0 }, - { 2.1, temBAD_TRANSFER_RATE, 2.0 }, - { 0.0, tesSUCCESS, 1.0 }, - { 2.0, tesSUCCESS, 2.0 }, - { 0.9, temBAD_TRANSFER_RATE, 2.0 }}); - } + doTests (supported_amendments(), + { + { 1.0, tesSUCCESS, 1.0 }, + { 1.1, tesSUCCESS, 1.1 }, + { 2.0, tesSUCCESS, 2.0 }, + { 2.1, temBAD_TRANSFER_RATE, 2.0 }, + { 0.0, tesSUCCESS, 1.0 }, + { 2.0, tesSUCCESS, 2.0 }, + { 0.9, temBAD_TRANSFER_RATE, 2.0 } + }); } void testGateway() { + testcase ("Gateway"); + using namespace test::jtx; - auto runTest = [](Env&& env, double tr) - { - Account const alice ("alice"); - Account const bob ("bob"); - Account const gw ("gateway"); - auto const USD = gw["USD"]; + Account const alice ("alice"); + Account const bob ("bob"); + Account const gw ("gateway"); + auto const USD = gw["USD"]; + + // Test gateway with a variety of allowed transfer rates + for (double transferRate = 1.0; + transferRate <= 2.0; transferRate += 0.03125) + { + Env env (*this); env.fund(XRP(10000), gw, alice, bob); - env.trust(USD(3), alice, bob); - env(rate(gw, tr)); + env.close(); + env.trust(USD(10), alice, bob); + env.close(); + env(rate(gw, transferRate)); env.close(); auto const amount = USD(1); - Rate const rate (tr * QUALITY_ONE); + Rate const rate (transferRate * QUALITY_ONE); auto const amountWithRate = toAmount (multiply(amount.value(), rate)); - env(pay(gw, alice, USD(3))); - env(pay(alice, bob, USD(1)), sendmax(USD(3))); + env(pay(gw, alice, USD(10))); + env.close(); + env(pay(alice, bob, USD(1)), sendmax(USD(10))); + env.close(); - env.require(balance(alice, USD(3) - amountWithRate)); + env.require(balance(alice, USD(10) - amountWithRate)); env.require(balance(bob, USD(1))); - }; - - // Test gateway with allowed transfer rates - auto const noFix1201 = supported_amendments().reset(fix1201); - runTest (Env{*this, noFix1201}, 1.02); - runTest (Env{*this, noFix1201}, 1); - runTest (Env{*this, noFix1201}, 2); - runTest (Env{*this, noFix1201}, 2.1); - runTest (Env{*this, supported_amendments()}, 1.02); - runTest (Env{*this, supported_amendments()}, 2); + } - // Test gateway when amendment is set after transfer rate + // Since fix1201 was enabled on Nov 14 2017 a rate in excess of + // 2.0 has been blocked by the transactor. But there are a few + // accounts on the MainNet that have larger-than-currently-allowed + // TransferRates. We'll bypass the transactor so we can check + // operation of these legacy TransferRates. + // + // Two out-of-bound values are currently in the ledger (March 2020) + // They are 4.0 and 4.294967295. So those are the values we test. + for (double transferRate : {4.0, 4.294967295}) { - Env env (*this, noFix1201); - Account const alice ("alice"); - Account const bob ("bob"); - Account const gw ("gateway"); - auto const USD = gw["USD"]; - double const tr = 2.75; - + Env env (*this); env.fund(XRP(10000), gw, alice, bob); - env.trust(USD(3), alice, bob); - env(rate(gw, tr)); env.close(); - env.enableFeature(fix1201); + env.trust(USD(10), alice, bob); + env.close(); + + // We'd like to use transferRate here, but the transactor + // blocks transfer rates that large. So we use an acceptable + // transfer rate here and later hack the ledger to replace + // the acceptable value with an out-of-bounds value. + env(rate(gw, 2.0)); env.close(); + // Note that we're bypassing almost all of the ledger's safety + // checks with this modify() call. If you call close() between + // here and the end of the test all the effort will be lost. + env.app().openLedger().modify( + [&gw, transferRate] (OpenView& view, beast::Journal j) + { + // Get the account root we want to hijack. + auto const sle = view.read (keylet::account(gw.id())); + if (! sle) + return false; // This would be really surprising! + + // We'll insert a replacement for the account root + // with the higher (currently invalid) transfer rate. + auto replacement = + std::make_shared(*sle, sle->key()); + (*replacement)[sfTransferRate] = + static_cast(transferRate * QUALITY_ONE); + view.rawReplace (replacement); + return true; + }); + auto const amount = USD(1); - Rate const rate (tr * QUALITY_ONE); auto const amountWithRate = - toAmount (multiply(amount.value(), rate)); + toAmount ( + multiply(amount.value(), + Rate (transferRate * QUALITY_ONE))); - env(pay(gw, alice, USD(3))); - env(pay(alice, bob, amount), sendmax(USD(3))); + env(pay(gw, alice, USD(10))); + env(pay(alice, bob, amount), sendmax(USD(10))); - env.require(balance(alice, USD(3) - amountWithRate)); + env.require(balance(alice, USD(10) - amountWithRate)); env.require(balance(bob, amount)); } } void testBadInputs() { + testcase ("Bad inputs"); + using namespace test::jtx; Env env (*this); Account const alice ("alice"); @@ -418,6 +454,8 @@ class AccountSet_test : public beast::unit_test::suite void testRequireAuthWithDir() { + testcase ("Require auth"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index f1eb37cb627..fa95f7791c2 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -59,19 +59,19 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env {*this}; - auto jrr = env.rpc("feature", "CryptoConditions") [jss::result]; + auto jrr = env.rpc("feature", "MultiSignReserve") [jss::result]; BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"); jrr.removeMember(jss::status); BEAST_EXPECT(jrr.size() == 1); auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "CryptoConditions", "name"); + BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); BEAST_EXPECTS(! feature[jss::enabled].asBool(), "enabled"); BEAST_EXPECTS(! feature[jss::vetoed].asBool(), "vetoed"); BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here - jrr = env.rpc("feature", "cryptoconditions") [jss::result]; + jrr = env.rpc("feature", "multiSignReserve") [jss::result]; BEAST_EXPECT(jrr[jss::error] == "badFeature"); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); } @@ -115,7 +115,7 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env {*this, - FeatureBitset(featureEscrow, featureCryptoConditions)}; + FeatureBitset(featureDepositAuth, featureDepositPreauth)}; auto jrr = env.rpc("feature") [jss::result]; if(! BEAST_EXPECT(jrr.isMember(jss::features))) @@ -215,40 +215,40 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env {*this, - FeatureBitset(featureCryptoConditions)}; + FeatureBitset(featureMultiSignReserve)}; - auto jrr = env.rpc("feature", "CryptoConditions") [jss::result]; + auto jrr = env.rpc("feature", "MultiSignReserve") [jss::result]; if(! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if(! BEAST_EXPECT(jrr.size() == 1)) return; auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "CryptoConditions", "name"); + BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); BEAST_EXPECTS(! feature[jss::vetoed].asBool(), "vetoed"); - jrr = env.rpc("feature", "CryptoConditions", "reject") [jss::result]; + jrr = env.rpc("feature", "MultiSignReserve", "reject") [jss::result]; if(! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if(! BEAST_EXPECT(jrr.size() == 1)) return; feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "CryptoConditions", "name"); + BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); BEAST_EXPECTS(feature[jss::vetoed].asBool(), "vetoed"); - jrr = env.rpc("feature", "CryptoConditions", "accept") [jss::result]; + jrr = env.rpc("feature", "MultiSignReserve", "accept") [jss::result]; if(! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if(! BEAST_EXPECT(jrr.size() == 1)) return; feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "CryptoConditions", "name"); + BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); BEAST_EXPECTS(! feature[jss::vetoed].asBool(), "vetoed"); // anything other than accept or reject is an error - jrr = env.rpc("feature", "CryptoConditions", "maybe"); + jrr = env.rpc("feature", "MultiSignReserve", "maybe"); BEAST_EXPECT(jrr[jss::error] == "invalidParams"); BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters."); } diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 0f09021bf07..96b786fb7f5 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -154,8 +154,7 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testGWB(sa - fix1373 - featureFlowCross); - testGWB(sa - featureFlowCross); + testGWB(sa - featureFlowCross); testGWB(sa); } }; diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 71c62b6877a..cec32b2cb45 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -269,8 +269,7 @@ class NoRipple_test : public beast::unit_test::suite }; using namespace jtx; auto const sa = supported_amendments(); - withFeatsTests(sa - fix1373 - featureFlowCross); - withFeatsTests(sa - featureFlowCross); + withFeatsTests(sa - featureFlowCross); withFeatsTests(sa); } };