diff --git a/README.md b/README.md index ad1c18b513b..27ed68bbbe4 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,8 @@ git-subtree. See the corresponding README for more details. * [Ripple Knowledge Center](https://ripple.com/learn/) * [Ripple Developer Center](https://ripple.com/build/) -* [Ripple Whitepapers & Reports](https://ripple.com/whitepapers-reports/) - * [Ripple Consensus Whitepaper](https://ripple.com/consensus-whitepaper/) +* Ripple Whitepapers & Reports + * [Ripple Consensus Whitepaper](https://ripple.com/files/ripple_consensus_whitepaper.pdf) * [Ripple Solutions Guide](https://ripple.com/files/ripple_solutions_guide.pdf) To learn about how Ripple is transforming global payments visit diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0ac4e3a597a..cbea134bee7 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -11,6 +11,21 @@ If you are using Red Hat Enterprise Linux 7 or CentOS 7, you can [update using ` # Releases +## Version 0.70.2 + +The `rippled` 0.70.2 release corrects an emergent behavior which causes large numbers of transactions to get +stuck in different nodes' open ledgers without being passed on to validators, resulting in a spike in the open +ledger fee on those nodes. + +**New and Updated Features** + +This release has no new features. + +**Bug Fixes** + +- Recent fee rises and TxQ issues ([#2215](https://github.com/ripple/rippled/issues/2215)) + + ## Version 0.70.1 The `rippled` 0.70.1 release corrects a technical flaw in the newly refactored consensus code that could cause a node to get stuck in consensus due to stale votes from a diff --git a/doc/rippled-example.cfg b/doc/rippled-example.cfg index b5ac0161f5c..47ce2f86543 100644 --- a/doc/rippled-example.cfg +++ b/doc/rippled-example.cfg @@ -482,6 +482,12 @@ # time a transaction with a higher fee level is added. # Default: 20. # +# minimum_queue_size = +# +# The queue will always be able to hold at least this of +# transactions, regardless of recent ledger sizes or the value of +# ledgers_in_queue. Default: 2000. +# # retry_sequence_percent = # # If a client replaces a transaction in the queue (same sequence diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 2f5edf6ced0..1c74d4134cf 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -118,6 +118,9 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& ledger) // Notify inbound transactions of the new ledger sequence number inboundTransactions_.newRound(buildLCL->info().seq); + // Use the ledger timing rules of the acquired ledger + parms_.useRoundedCloseTime = buildLCL->rules().enabled(fix1528); + return RCLCxLedger(buildLCL); } @@ -152,6 +155,7 @@ RCLConsensus::Adaptor::relay(RCLCxTx const& tx) // If we didn't relay this transaction recently, relay it to all peers if (app_.getHashRouter().shouldRelay(tx.id())) { + JLOG(j_.debug()) << "Relaying disputed tx " << tx.id(); auto const slice = tx.tx_.slice(); protocol::TMTransaction msg; msg.set_rawtransaction(slice.data(), slice.size()); @@ -161,6 +165,10 @@ RCLConsensus::Adaptor::relay(RCLCxTx const& tx) app_.overlay().foreach (send_always( std::make_shared(msg, protocol::mtTRANSACTION))); } + else + { + JLOG(j_.debug()) << "Not relaying disputed tx " << tx.id(); + } } void RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) @@ -301,6 +309,8 @@ RCLConsensus::Adaptor::onClose( // Build SHAMap containing all transactions in our open ledger for (auto const& tx : initialLedger->txs) { + JLOG(j_.trace()) << "Adding open ledger TX " << + tx.first->getTransactionID(); Serializer s(2048); tx.first->add(s); initialSet->addItem( @@ -489,7 +499,7 @@ RCLConsensus::Adaptor::doAccept( { JLOG(j_.debug()) << "Test applying disputed transaction that did" - << " not get in"; + << " not get in " << it.second.tx().id(); SerialIter sit(it.second.tx().tx_.slice()); auto txn = std::make_shared(sit); @@ -950,9 +960,12 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr) JLOG(j_.info()) << "Entering consensus process, watching"; } - // Notify inbOund ledgers that we are starting a new round + // 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_ && (app_.getOPs().getOperatingMode() == NetworkOPs::omFULL); diff --git a/src/ripple/app/ledger/OpenLedger.h b/src/ripple/app/ledger/OpenLedger.h index a6fa7d5e193..1f5a4feff4f 100644 --- a/src/ripple/app/ledger/OpenLedger.h +++ b/src/ripple/app/ledger/OpenLedger.h @@ -167,6 +167,7 @@ class OpenLedger std::string const& suffix = "", modify_type const& f = {}); +private: /** Algorithm for applying transactions. This has the retry logic and ordering semantics @@ -178,9 +179,9 @@ class OpenLedger apply (Application& app, OpenView& view, ReadView const& check, FwdRange const& txs, OrderedTxs& retries, ApplyFlags flags, - beast::Journal j); + std::map& shouldRecover, + beast::Journal j); -private: enum Result { success, @@ -197,7 +198,7 @@ class OpenLedger apply_one (Application& app, OpenView& view, std::shared_ptr< STTx const> const& tx, bool retry, ApplyFlags flags, - beast::Journal j); + bool shouldRecover, beast::Journal j); }; //------------------------------------------------------------------------------ @@ -207,7 +208,8 @@ void OpenLedger::apply (Application& app, OpenView& view, ReadView const& check, FwdRange const& txs, OrderedTxs& retries, ApplyFlags flags, - beast::Journal j) + std::map& shouldRecover, + beast::Journal j) { for (auto iter = txs.begin(); iter != txs.end(); ++iter) @@ -217,10 +219,11 @@ OpenLedger::apply (Application& app, OpenView& view, // Dereferencing the iterator can // throw since it may be transformed. auto const tx = *iter; - if (check.txExists(tx->getTransactionID())) + auto const txId = tx->getTransactionID(); + if (check.txExists(txId)) continue; auto const result = apply_one(app, view, - tx, true, flags, j); + tx, true, flags, shouldRecover[txId], j); if (result == Result::retry) retries.insert(tx); } @@ -241,7 +244,7 @@ OpenLedger::apply (Application& app, OpenView& view, { switch (apply_one(app, view, iter->second, retry, flags, - j)) + shouldRecover[iter->second->getTransactionID()], j)) { case Result::success: ++changes; diff --git a/src/ripple/app/ledger/impl/OpenLedger.cpp b/src/ripple/app/ledger/impl/OpenLedger.cpp index 955584b72f2..08493f14094 100644 --- a/src/ripple/app/ledger/impl/OpenLedger.cpp +++ b/src/ripple/app/ledger/impl/OpenLedger.cpp @@ -20,9 +20,13 @@ #include #include #include +#include #include #include #include +#include +#include +#include #include #include @@ -84,14 +88,20 @@ OpenLedger::accept(Application& app, Rules const& rules, JLOG(j_.trace()) << "accept ledger " << ledger->seq() << " " << suffix; auto next = create(rules, ledger); + std::map shouldRecover; if (retriesFirst) { + for (auto const& tx : retries) + { + auto const txID = tx.second->getTransactionID(); + shouldRecover[txID] = app.getHashRouter().shouldRecover(txID); + } // Handle disputed tx, outside lock using empty = std::vector>; apply (app, *next, *ledger, empty{}, - retries, flags, j_); + retries, flags, shouldRecover, j_); } // Block calls to modify, otherwise // new tx going into the open ledger @@ -100,6 +110,19 @@ OpenLedger::accept(Application& app, Rules const& rules, std::mutex> lock1(modify_mutex_); // Apply tx from the current open view if (! current_->txs.empty()) + { + for (auto const& tx : current_->txs) + { + auto const txID = tx.first->getTransactionID(); + auto iter = shouldRecover.lower_bound(txID); + if (iter != shouldRecover.end() + && iter->first == txID) + // already had a chance via disputes + iter->second = false; + else + shouldRecover.emplace_hint(iter, txID, + app.getHashRouter().shouldRecover(txID)); + } apply (app, *next, *ledger, boost::adaptors::transform( current_->txs, @@ -109,7 +132,8 @@ OpenLedger::accept(Application& app, Rules const& rules, { return p.first; }), - retries, flags, j_); + retries, flags, shouldRecover, j_); + } // Call the modifier if (f) f(*next, j_); @@ -117,6 +141,29 @@ OpenLedger::accept(Application& app, Rules const& rules, for (auto const& item : locals) app.getTxQ().apply(app, *next, item.second, flags, j_); + + // If we didn't relay this transaction recently, relay it to all peers + for (auto const& txpair : next->txs) + { + auto const& tx = txpair.first; + auto const txId = tx->getTransactionID(); + if (auto const toSkip = app.getHashRouter().shouldRelay(txId)) + { + JLOG(j_.debug()) << "Relaying recovered tx " << txId; + protocol::TMTransaction msg; + Serializer s; + + tx->add(s); + msg.set_rawtransaction(s.data(), s.size()); + msg.set_status(protocol::tsNEW); + msg.set_receivetimestamp( + app.timeKeeper().now().time_since_epoch().count()); + app.overlay().foreach(send_if_not( + std::make_shared(msg, protocol::mtTRANSACTION), + peer_in_set(*toSkip))); + } + } + // Switch to the new open view std::lock_guard< std::mutex> lock2(current_mutex_); @@ -138,14 +185,24 @@ OpenLedger::create (Rules const& rules, auto OpenLedger::apply_one (Application& app, OpenView& view, std::shared_ptr const& tx, - bool retry, ApplyFlags flags, + bool retry, ApplyFlags flags, bool shouldRecover, beast::Journal j) -> Result { if (retry) flags = flags | tapRETRY; - auto const result = ripple::apply( - app, view, *tx, flags, j); - if (result.second) + auto const result = [&] + { + auto const queueResult = app.getTxQ().apply( + app, view, tx, flags | tapPREFER_QUEUE, j); + // If the transaction can't get into the queue for intrinsic + // reasons, and it can still be recovered, try to put it + // directly into the open ledger, else drop it. + if (queueResult.first == telCAN_NOT_QUEUE && shouldRecover) + return ripple::apply(app, view, *tx, flags, j); + return queueResult; + }(); + if (result.second || + result.first == terQUEUED) return Result::success; if (isTefFailure (result.first) || isTemMalformed (result.first) || diff --git a/src/ripple/app/main/Amendments.cpp b/src/ripple/app/main/Amendments.cpp index 7e8c5ac4c85..c8d4ed94432 100644 --- a/src/ripple/app/main/Amendments.cpp +++ b/src/ripple/app/main/Amendments.cpp @@ -58,7 +58,9 @@ supportedAmendments () { "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" }, { "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" }, { "B4D44CC3111ADD964E846FC57760C8B50FFCD5A82C86A72756F6B058DDDF96AD fix1201" }, - { "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" } + { "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" }, + { "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" }, + { "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" } }; } diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index a2a229bdab7..946374dccfa 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -475,7 +475,8 @@ class ApplicationImp , mFeeTrack (std::make_unique(logs_->journal("LoadManager"))) , mHashRouter (std::make_unique( - stopwatch(), HashRouter::getDefaultHoldTime ())) + stopwatch(), HashRouter::getDefaultHoldTime (), + HashRouter::getDefaultRecoverLimit ())) , mValidations (ValidationParms(),stopwatch(), logs_->journal("Validations"), *this) diff --git a/src/ripple/app/misc/HashRouter.cpp b/src/ripple/app/misc/HashRouter.cpp index 7a5397f4103..8e407126058 100644 --- a/src/ripple/app/misc/HashRouter.cpp +++ b/src/ripple/app/misc/HashRouter.cpp @@ -107,4 +107,14 @@ HashRouter::shouldRelay (uint256 const& key) return s.releasePeerSet(); } +bool +HashRouter::shouldRecover(uint256 const& key) +{ + std::lock_guard lock(mutex_); + + auto& s = emplace(key).first; + + return s.shouldRecover(recoverLimit_); +} + } // ripple diff --git a/src/ripple/app/misc/HashRouter.h b/src/ripple/app/misc/HashRouter.h index 742f49e3efb..a40d3558bf9 100644 --- a/src/ripple/app/misc/HashRouter.h +++ b/src/ripple/app/misc/HashRouter.h @@ -34,7 +34,6 @@ namespace ripple { // VFALCO NOTE How can both bad and good be set on a hash? #define SF_BAD 0x02 // Temporarily bad #define SF_SAVED 0x04 -#define SF_RETRY 0x08 // Transaction can be retried #define SF_TRUSTED 0x10 // comes from trusted source // Private flags, used internally in apply.cpp. // Do not attempt to read, set, or reuse. @@ -66,7 +65,6 @@ class HashRouter static char const* getCountedObjectName () { return "HashRouterEntry"; } Entry () - : flags_ (0) { } @@ -107,12 +105,26 @@ class HashRouter return true; } + /** Determines if this item should be recovered from the open ledger. + + Counts the number of times the item has been recovered. + Every `limit` times the function is called, return false. + Else return true. + + @note The limit must be > 0 + */ + bool shouldRecover(std::uint32_t limit) + { + return ++recoveries_ % limit != 0; + } + private: - int flags_; + int flags_ = 0; std::set peers_; // This could be generalized to a map, if more // than one flag needs to expire independently. boost::optional relayed_; + std::uint32_t recoveries_ = 0; }; public: @@ -123,9 +135,16 @@ class HashRouter return 300s; } - HashRouter (Stopwatch& clock, std::chrono::seconds entryHoldTimeInSeconds) + static inline std::uint32_t getDefaultRecoverLimit() + { + return 1; + } + + HashRouter (Stopwatch& clock, std::chrono::seconds entryHoldTimeInSeconds, + std::uint32_t recoverLimit) : suppressionMap_(clock) , holdTime_ (entryHoldTimeInSeconds) + , recoverLimit_ (recoverLimit + 1u) { } @@ -164,6 +183,12 @@ class HashRouter */ boost::optional> shouldRelay(uint256 const& key); + /** Determines whether the hashed item should be recovered + + @return `bool` indicates whether the item should be relayed + */ + bool shouldRecover(uint256 const& key); + private: // pair.second indicates whether the entry was created std::pair emplace (uint256 const&); @@ -175,6 +200,8 @@ class HashRouter hardened_hash> suppressionMap_; std::chrono::seconds const holdTime_; + + std::uint32_t const recoverLimit_; }; } // ripple diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 380559859b0..1743034b118 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -841,12 +841,6 @@ void NetworkOPsImp::submitTransaction (std::shared_ptr const& iTrans auto const txid = trans->getTransactionID (); auto const flags = app_.getHashRouter().getFlags(txid); - if ((flags & SF_RETRY) != 0) - { - JLOG(m_journal.warn()) << "Redundant transactions submitted"; - return; - } - if ((flags & SF_BAD) != 0) { JLOG(m_journal.warn()) << "Submitted transaction cached bad"; @@ -1155,7 +1149,7 @@ void NetworkOPsImp::apply (std::unique_lock& batchLock) Serializer s; e.transaction->getSTransaction()->add (s); - tx.set_rawtransaction (&s.getData().front(), s.getLength()); + tx.set_rawtransaction (s.data(), s.size()); tx.set_status (protocol::tsCURRENT); tx.set_receivetimestamp (app_.timeKeeper().now().time_since_epoch().count()); tx.set_deferred(e.result == terQUEUED); diff --git a/src/ripple/app/misc/TxQ.h b/src/ripple/app/misc/TxQ.h index dd54f7ca81a..27fc524a517 100644 --- a/src/ripple/app/misc/TxQ.h +++ b/src/ripple/app/misc/TxQ.h @@ -55,6 +55,7 @@ class TxQ struct Setup { std::size_t ledgersInQueue = 20; + std::size_t queueSizeMin = 2000; std::uint32_t retrySequencePercent = 25; // TODO: eahennis. Can we remove the multi tx factor? std::int32_t multiTxnPercent = -90; diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 6763c024daa..14d6766e900 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -339,7 +339,7 @@ TxQ::canBeHeld(STTx const& tx, OpenView const& view, promise to stick around for long enough that it has a realistic chance of getting into a ledger. */ - auto lastValid = getLastLedgerSequence(tx); + auto const lastValid = getLastLedgerSequence(tx); canBeHeld = !lastValid || *lastValid >= view.info().seq + setup_.minimumLastLedgerBuffer; } @@ -349,10 +349,31 @@ TxQ::canBeHeld(STTx const& tx, OpenView const& view, can queue. Mitigates the lost cost of relaying should an early one fail or get dropped. */ - canBeHeld = accountIter == byAccount_.end() || - replacementIter || - accountIter->second.getTxnCount() < - setup_.maximumTxnPerAccount; + + // Allow if the account is not in the queue at all + canBeHeld = accountIter == byAccount_.end(); + + if(!canBeHeld) + { + // Allow this tx to replace another one + canBeHeld = replacementIter.is_initialized(); + } + + if (!canBeHeld) + { + // Allow if there are fewer than the limit + canBeHeld = accountIter->second.getTxnCount() < + setup_.maximumTxnPerAccount; + } + + if (!canBeHeld) + { + // Allow if the transaction goes in front of any + // queued transactions. Enables recovery of open + // ledger transactions, and stuck transactions. + auto const tSeq = tx.getSequence(); + canBeHeld = tSeq < accountIter->second.transactions.rbegin()->first; + } } return canBeHeld; } @@ -512,8 +533,7 @@ TxQ::tryClearAccountQueue(Application& app, OpenView& view, non-blockers? Yes: Remove the queued transaction. Continue to next step. - No: Reject `txn` with `telINSUF_FEE_P` or - `telCAN_NOT_QUEUE`. Stop. + No: Reject `txn` with `telCAN_NOT_QUEUE_FEE`. Stop. No: Continue to next step. 3. Does this tx have the expected sequence number for the account? @@ -527,11 +547,11 @@ TxQ::tryClearAccountQueue(Application& app, OpenView& view, than the previous tx? No: Reject with `telINSUF_FEE_P`. Stop. Yes: Are any of the prior sequence txs blockers? - Yes: Reject with `telCAN_NOT_QUEUE`. Stop. + Yes: Reject with `telCAN_NOT_QUEUE_BLOCKED`. Stop. No: Are the fees in-flight of the other queued txs >= than the account balance or minimum account reserve? - Yes: Reject with `telCAN_NOT_QUEUE`. Stop. + Yes: Reject with `telCAN_NOT_QUEUE_BALANCE`. Stop. No: Create a throwaway sandbox `View`. Modify the account's sequence number to match the tx (avoid `terPRE_SEQ`), and decrease @@ -550,8 +570,7 @@ TxQ::tryClearAccountQueue(Application& app, OpenView& view, it to `doApply()` and return that result. No: Continue to the next step. 6. Can the tx be held in the queue? (See TxQ::canBeHeld). - No: Reject `txn` with `telINSUF_FEE_P` if this tx - has the current sequence, or `telCAN_NOT_QUEUE` + No: Reject `txn` with `telCAN_NOT_QUEUE_FULL` if not. Stop. Yes: Continue to the next step. 7. Is the queue full? @@ -613,8 +632,15 @@ TxQ::apply(Application& app, OpenView& view, auto const baseFee = calculateBaseFee(app, view, *tx, j); auto const feeLevelPaid = getFeeLevelPaid(*tx, baseLevel, baseFee, setup_); - auto const requiredFeeLevel = FeeMetrics::scaleFeeLevel( - metricsSnapshot, view); + auto const requiredFeeLevel = [&]() + { + auto feeLevel = FeeMetrics::scaleFeeLevel(metricsSnapshot, view); + if ((flags & tapPREFER_QUEUE) && byFee_.size()) + { + return std::max(feeLevel, byFee_.begin()->feeLevel); + } + return feeLevel; + }(); auto accountIter = byAccount_.find(account); bool const accountExists = accountIter != byAccount_.end(); @@ -679,8 +705,7 @@ TxQ::apply(Application& app, OpenView& view, transactionID << " in favor of normal queued " << existingIter->second.txID; - return{existingIter == txQAcct.transactions.begin() ? - telINSUF_FEE_P : telCAN_NOT_QUEUE, false }; + return {telCAN_NOT_QUEUE_BLOCKS, false }; } } } @@ -710,7 +735,7 @@ TxQ::apply(Application& app, OpenView& view, transactionID << " in favor of queued " << existingIter->second.txID; - return{ telINSUF_FEE_P, false }; + return{ telCAN_NOT_QUEUE_FEE, false }; } } } @@ -806,7 +831,7 @@ TxQ::apply(Application& app, OpenView& view, transactionID << ". A blocker-type transaction " << "is in the queue."; - return{ telCAN_NOT_QUEUE, false }; + return{ telCAN_NOT_QUEUE_BLOCKED, false }; } multiTxn->fee += workingIter->second.consequences->fee; @@ -824,7 +849,7 @@ TxQ::apply(Application& app, OpenView& view, than the account's current balance, or the minimum reserve. If it is, then there's a risk that the fees won't get paid, so drop this - transaction with a telCAN_NOT_QUEUE result. + transaction with a telCAN_NOT_QUEUE_BALANCE result. TODO: Decide whether to count the current txn fee in this limit if it's the last transaction for this account. Currently, it will not count, @@ -866,7 +891,7 @@ TxQ::apply(Application& app, OpenView& view, "Ignoring transaction " << transactionID << ". Total fees in flight too high."; - return{ telCAN_NOT_QUEUE, false }; + return{ telCAN_NOT_QUEUE_BALANCE, false }; } // Create the test view from the current view @@ -904,24 +929,27 @@ TxQ::apply(Application& app, OpenView& view, /* Quick heuristic check to see if it's worth checking that this tx has a high enough fee to clear all the txs in the queue. - 1) Must be an account already in the queue. - 2) Must be have passed the multiTxn checks (tx is not the next + 1) Transaction is trying to get into the open ledger + 2) Must be an account already in the queue. + 3) Must be have passed the multiTxn checks (tx is not the next account seq, the skipped seqs are in the queue, the reserve doesn't get exhausted, etc). - 3) The next transaction must not have previously tried and failed + 4) The next transaction must not have previously tried and failed to apply to an open ledger. - 4) Tx must be paying more than just the required fee level to + 5) Tx must be paying more than just the required fee level to get itself into the queue. - 5) Fee level must be escalated above the default (if it's not, + 6) Fee level must be escalated above the default (if it's not, then the first tx _must_ have failed to process in `accept` for some other reason. Tx is allowed to queue in case conditions change, but don't waste the effort to clear). - 6) Tx is not a 0-fee / free transaction, regardless of fee level. + 7) Tx is not a 0-fee / free transaction, regardless of fee level. */ - if (accountExists && multiTxn.is_initialized() && - multiTxn->nextTxIter->second.retriesRemaining == MaybeTx::retriesAllowed && - feeLevelPaid > requiredFeeLevel && - requiredFeeLevel > baseLevel && baseFee != 0) + if (!(flags & tapPREFER_QUEUE) && accountExists && + multiTxn.is_initialized() && + multiTxn->nextTxIter->second.retriesRemaining == + MaybeTx::retriesAllowed && + feeLevelPaid > requiredFeeLevel && + requiredFeeLevel > baseLevel && baseFee != 0) { OpenView sandbox(open_ledger, &view, view.rules()); @@ -970,8 +998,7 @@ TxQ::apply(Application& app, OpenView& view, JLOG(j_.trace()) << "Transaction " << transactionID << " can not be held"; - return { feeLevelPaid >= requiredFeeLevel ? - telCAN_NOT_QUEUE : telINSUF_FEE_P, false }; + return { telCAN_NOT_QUEUE, false }; } // If the queue is full, decide whether to drop the current @@ -986,7 +1013,7 @@ TxQ::apply(Application& app, OpenView& view, transactionID << " would kick a transaction from the same account (" << account << ") out of the queue."; - return { telCAN_NOT_QUEUE, false }; + return { telCAN_NOT_QUEUE_FULL, false }; } auto const& endAccount = byAccount_.at(lastRIter->account); auto endEffectiveFeeLevel = [&]() @@ -1038,7 +1065,7 @@ TxQ::apply(Application& app, OpenView& view, JLOG(j_.warn()) << "Queue is full, and transaction " << transactionID << " fee is lower than end item's account average fee"; - return { telINSUF_FEE_P, false }; + return { telCAN_NOT_QUEUE_FULL, false }; } } @@ -1105,7 +1132,8 @@ TxQ::processClosedLedger(Application& app, auto ledgerSeq = view.info().seq; if (!timeLeap) - maxSize_ = snapshot.txnsExpected * setup_.ledgersInQueue; + maxSize_ = std::max (snapshot.txnsExpected * setup_.ledgersInQueue, + setup_.queueSizeMin); // Remove any queued candidates whose LastLedgerSequence has gone by. for(auto candidateIter = byFee_.begin(); candidateIter != byFee_.end(); ) @@ -1454,6 +1482,7 @@ setup_TxQ(Config const& config) TxQ::Setup setup; auto const& section = config.section("transaction_queue"); set(setup.ledgersInQueue, "ledgers_in_queue", section); + set(setup.queueSizeMin, "minimum_queue_size", section); set(setup.retrySequencePercent, "retry_sequence_percent", section); set(setup.multiTxnPercent, "multi_txn_percent", section); set(setup.minimumEscalationMultiplier, diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 3b9abf0149b..5600993beab 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -253,7 +253,7 @@ EscrowCreate::doApply() ctx_.view().insert(slep); - // Add escrow to owner directory + // Add escrow to sender's owner directory { auto page = dirAdd(ctx_.view(), keylet::ownerDir(account), slep->key(), false, describeOwnerDir(account), ctx_.app.journal ("View")); @@ -262,6 +262,21 @@ EscrowCreate::doApply() (*slep)[sfOwnerNode] = *page; } + // If it's not a self-send, add escrow to recipient's owner directory. + if (ctx_.view ().rules().enabled(fix1523)) + { + 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; + } + } + // Deduct owner's balance, increment owner count (*sle)[sfBalance] = (*sle)[sfBalance] - ctx_.tx[sfAmount]; (*sle)[sfOwnerCount] = (*sle)[sfOwnerCount] + 1; @@ -435,6 +450,16 @@ EscrowFinish::doApply() return ter; } + // Remove escrow from recipient's owner directory, if present. + if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) + { + TER const ter = dirDelete(ctx_.view(), true, + (*slep)[sfDestinationNode], keylet::ownerDir((*slep)[sfDestination]), + k.key, false, false, ctx_.app.journal ("View")); + if (! isTesSuccess(ter)) + return ter; + } + // NOTE: These payments cannot be used to fund accounts // Fetch Destination SLE @@ -488,7 +513,6 @@ EscrowCancel::doApply() ctx_.view().info().parentCloseTime.time_since_epoch().count() <= (*slep)[sfCancelAfter]) return tecNO_PERMISSION; - AccountID const account = (*slep)[sfAccount]; // Remove escrow from owner directory @@ -501,6 +525,16 @@ EscrowCancel::doApply() return ter; } + // Remove escrow from recipient's owner directory, if present. + if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) + { + TER const ter = dirDelete(ctx_.view(), true, + (*slep)[sfDestinationNode], keylet::ownerDir((*slep)[sfDestination]), + k.key, false, false, ctx_.app.journal ("View")); + if (! isTesSuccess(ter)) + return ter; + } + // Transfer amount back to owner, decrement owner count auto const sle = ctx_.view().peek( keylet::account(account)); diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index 2484a4b16be..572430f13dc 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -494,6 +494,10 @@ class Consensus void leaveConsensus(); + // The rounded or effective close time estimate from a proposer + NetClock::time_point + asCloseTime(NetClock::time_point raw) const; + private: Adaptor& adaptor_; @@ -1204,7 +1208,7 @@ Consensus::updateOurPositions() auto const ourCutoff = now_ - parms.proposeINTERVAL; // Verify freshness of peer positions and compute close times - std::map effCloseTimes; + std::map closeTimeVotes; { auto it = currPeerPositions_.begin(); while (it != currPeerPositions_.end()) @@ -1222,10 +1226,7 @@ Consensus::updateOurPositions() else { // proposal is still fresh - ++effCloseTimes[effCloseTime( - peerProp.closeTime(), - closeResolution_, - previousLedger_.closeTime())]; + ++closeTimeVotes[asCloseTime(peerProp.closeTime())]; ++it; } } @@ -1273,10 +1274,7 @@ Consensus::updateOurPositions() { // no other times haveCloseTimeConsensus_ = true; - consensusCloseTime = effCloseTime( - result_->position.closeTime(), - closeResolution_, - previousLedger_.closeTime()); + consensusCloseTime = asCloseTime(result_->position.closeTime()); } else { @@ -1294,10 +1292,7 @@ Consensus::updateOurPositions() int participants = currPeerPositions_.size(); if (mode_.get() == ConsensusMode::proposing) { - ++effCloseTimes[effCloseTime( - result_->position.closeTime(), - closeResolution_, - previousLedger_.closeTime())]; + ++closeTimeVotes[asCloseTime(result_->position.closeTime())]; ++participants; } @@ -1312,7 +1307,7 @@ Consensus::updateOurPositions() << " nw:" << neededWeight << " thrV:" << threshVote << " thrC:" << threshConsensus; - for (auto const& it : effCloseTimes) + for (auto const& it : closeTimeVotes) { JLOG(j_.debug()) << "CCTime: seq " << previousLedger_.seq() + 1 << ": " @@ -1342,11 +1337,7 @@ Consensus::updateOurPositions() } if (!ourNewSet && - ((consensusCloseTime != - effCloseTime( - result_->position.closeTime(), - closeResolution_, - previousLedger_.closeTime())) || + ((consensusCloseTime != asCloseTime(result_->position.closeTime())) || result_->position.isStale(ourCutoff))) { // close time changed or our position is stale @@ -1538,6 +1529,16 @@ Consensus::updateDisputes(NodeID_t const& node, TxSet_t const& other) } } +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()); +} + } // namespace ripple #endif diff --git a/src/ripple/consensus/ConsensusParms.h b/src/ripple/consensus/ConsensusParms.h index 02ed12a61f4..569c6547eee 100644 --- a/src/ripple/consensus/ConsensusParms.h +++ b/src/ripple/consensus/ConsensusParms.h @@ -129,6 +129,17 @@ 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/crypto/impl/GenerateDeterministicKey.cpp b/src/ripple/crypto/impl/GenerateDeterministicKey.cpp index a2265af38fc..80599debcc6 100644 --- a/src/ripple/crypto/impl/GenerateDeterministicKey.cpp +++ b/src/ripple/crypto/impl/GenerateDeterministicKey.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -74,8 +75,6 @@ copy_uint32 (FwdIt out, std::uint32_t v) *out = v & 0xff; } -// #define EC_DEBUG - // Functions to add support for deterministic EC keys // --> seed @@ -94,12 +93,12 @@ static bignum generateRootDeterministicKey (uint128 const& seed) std::copy(seed.begin(), seed.end(), buf.begin()); copy_uint32 (buf.begin() + 16, seq++); auto root = sha512Half(buf); - std::fill (buf.begin(), buf.end(), 0); // security erase + beast::secure_erase(buf.data(), buf.size()); privKey.assign (root.data(), root.size()); - root.zero(); // security erase + beast::secure_erase(root.data(), root.size()); } while (privKey.is_zero() || privKey >= secp256k1curve.order); - + beast::secure_erase(&seq, sizeof(seq)); return privKey; } @@ -153,8 +152,9 @@ static bignum makeHash (Blob const& pubGen, int seq, bignum const& order) copy_uint32 (buf.begin() + 33, seq); copy_uint32 (buf.begin() + 37, subSeq++); auto root = sha512Half_s(buf); - std::fill(buf.begin(), buf.end(), 0); // security erase + beast::secure_erase(buf.data(), buf.size()); result.assign (root.data(), root.size()); + beast::secure_erase(root.data(), root.size()); } while (result.is_zero() || result >= order); diff --git a/src/ripple/ledger/ApplyView.h b/src/ripple/ledger/ApplyView.h index c0e1a363ccd..c2d740bec7a 100644 --- a/src/ripple/ledger/ApplyView.h +++ b/src/ripple/ledger/ApplyView.h @@ -37,6 +37,11 @@ enum ApplyFlags // Transaction can be retried, soft failures allowed tapRETRY = 0x20, + // Transaction must pay more than both the open ledger + // fee and all transactions in the queue to get into the + // open ledger + tapPREFER_QUEUE = 0x40, + // Transaction came from a privileged source tapUNLIMITED = 0x400, }; diff --git a/src/ripple/ledger/Directory.h b/src/ripple/ledger/Directory.h index e2a1a381e7f..2ded082a971 100644 --- a/src/ripple/ledger/Directory.h +++ b/src/ripple/ledger/Directory.h @@ -44,9 +44,6 @@ class Dir const_iterator end() const; - - const_iterator - find(uint256 const& page_key, uint256 const& sle_key) const; }; class Dir::const_iterator diff --git a/src/ripple/ledger/impl/Directory.cpp b/src/ripple/ledger/impl/Directory.cpp index b52738336fc..d1b4b4c95b4 100644 --- a/src/ripple/ledger/impl/Directory.cpp +++ b/src/ripple/ledger/impl/Directory.cpp @@ -60,38 +60,6 @@ Dir::end() const -> return const_iterator(*view_, root_, root_); } -const_iterator -Dir::find(uint256 const& page_key, uint256 const& sle_key) const -{ - if (sle_ == nullptr) - return end(); - - auto it = const_iterator(*view_, root_, keylet::page(page_key, 0)); - if (root_.key == page_key) - { - it.sle_ = sle_; - it.indexes_ = indexes_; - } - else - { - it.sle_ = view_->read(it.page_); - if (it.sle_ == nullptr) - { - it.page_ = root_; - return it; - } - it.indexes_ = &it.sle_->getFieldV256(sfIndexes); - } - - it.it_ = std::find(std::begin(*it.indexes_), - std::end(*it.indexes_), sle_key); - if (it.it_ == std::end(*it.indexes_)) - return end(); - - it.index_ = *it.it_; - return it; -} - bool const_iterator::operator==(const_iterator const& other) const { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 26e8320814f..f8d634c1319 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1041,6 +1041,8 @@ PeerImp::onMessage (std::shared_ptr const& m) { // If we've never been in synch, there's nothing we can do // with a transaction + JLOG(p_journal_.debug()) << "Ignoring incoming transaction: " << + "Need network ledger"; return; } @@ -1060,11 +1062,10 @@ PeerImp::onMessage (std::shared_ptr const& m) if (flags & SF_BAD) { fee_ = Resource::feeInvalidSignature; + JLOG(p_journal_.debug()) << "Ignoring known bad tx " << + txID; return; } - - if (!(flags & SF_RETRY)) - return; } JLOG(p_journal_.debug()) << "Got tx " << txID; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 4a70a88a72f..aadde6e3b9c 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -49,27 +49,31 @@ namespace detail { class FeatureCollections { static constexpr char const* const featureNames[] = - {"MultiSign", - "Tickets", - "TrustSetAuth", - "FeeEscalation", - "OwnerPaysFee", - "CompareFlowV1V2", - "SHAMapV2", - "PayChan", - "Flow", - "CompareTakerFlowCross", - "FlowCross", - "CryptoConditions", - "TickSize", - "fix1368", - "Escrow", - "CryptoConditionsSuite", - "fix1373", - "EnforceInvariants", - "SortedDirectories", - "fix1201", - "fix1512"}; + { + "MultiSign", + "Tickets", + "TrustSetAuth", + "FeeEscalation", + "OwnerPaysFee", + "CompareFlowV1V2", + "SHAMapV2", + "PayChan", + "Flow", + "CompareTakerFlowCross", + "FlowCross", + "CryptoConditions", + "TickSize", + "fix1368", + "Escrow", + "CryptoConditionsSuite", + "fix1373", + "EnforceInvariants", + "SortedDirectories", + "fix1201", + "fix1512", + "fix1523", + "fix1528" + }; std::vector features; boost::container::flat_map featureToIndex; @@ -164,6 +168,8 @@ extern uint256 const featureEnforceInvariants; extern uint256 const featureSortedDirectories; extern uint256 const fix1201; extern uint256 const fix1512; +extern uint256 const fix1523; +extern uint256 const fix1528; } // ripple diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 83205b086ab..232cb4a8532 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -387,6 +387,7 @@ extern SF_U64 const sfBaseFee; extern SF_U64 const sfExchangeRate; extern SF_U64 const sfLowNode; extern SF_U64 const sfHighNode; +extern SF_U64 const sfDestinationNode; // 128-bit extern SF_U128 const sfEmailHash; diff --git a/src/ripple/protocol/SecretKey.h b/src/ripple/protocol/SecretKey.h index 20204479cca..6174ed40ecb 100644 --- a/src/ripple/protocol/SecretKey.h +++ b/src/ripple/protocol/SecretKey.h @@ -88,30 +88,6 @@ operator!= (SecretKey const& lhs, //------------------------------------------------------------------------------ -/** Produces a sequence of secp256k1 key pairs. */ -class Generator -{ -private: - Blob gen_; // VFALCO compile time size? - -public: - explicit - Generator (Seed const& seed); - - /** Generate the nth key pair. - - The seed is required to produce the private key. - */ - std::pair - operator()(Seed const& seed, std::size_t ordinal) const; - - /** Generate the nth public key. */ - PublicKey - operator()(std::size_t ordinal) const; -}; - -//------------------------------------------------------------------------------ - /** Parse a secret key */ template <> boost::optional diff --git a/src/ripple/protocol/Serializer.h b/src/ripple/protocol/Serializer.h index e389b72bee8..094898415dd 100644 --- a/src/ripple/protocol/Serializer.h +++ b/src/ripple/protocol/Serializer.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -198,8 +199,8 @@ class Serializer } void secureErase () { - memset (& (mData.front ()), 0, mData.size ()); - erase (); + beast::secure_erase(mData.data(), mData.size()); + mData.clear (); } void erase () { diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 45cd2903fb2..55e6900032f 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -47,6 +47,11 @@ enum TER telINSUF_FEE_P, telNO_DST_PARTIAL, telCAN_NOT_QUEUE, + telCAN_NOT_QUEUE_BALANCE, + telCAN_NOT_QUEUE_BLOCKS, + telCAN_NOT_QUEUE_BLOCKED, + telCAN_NOT_QUEUE_FEE, + telCAN_NOT_QUEUE_FULL, // -299 .. -200: M Malformed (bad signature) // Causes: diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 994727eeec5..08f8b767678 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -34,7 +34,7 @@ char const* const versionString = // The build version number. You must edit this for each release // and follow the format described at http://semver.org/ // - "0.80.0-rc2" + "0.80.0-rc3" #if defined(DEBUG) || defined(SANITIZER) "+" diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index dbda491a090..97a280f121d 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -116,5 +116,7 @@ uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariant uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); uint256 const fix1201 = *getRegisteredFeature("fix1201"); uint256 const fix1512 = *getRegisteredFeature("fix1512"); +uint256 const fix1523 = *getRegisteredFeature("fix1523"); +uint256 const fix1528 = *getRegisteredFeature("fix1528"); } // ripple diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index d900906c8d9..9dac61ba2fc 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -98,7 +98,8 @@ LedgerFormats::LedgerFormats () SOElement (sfDestinationTag, SOE_OPTIONAL) << SOElement (sfOwnerNode, SOE_REQUIRED) << SOElement (sfPreviousTxnID, SOE_REQUIRED) << - SOElement (sfPreviousTxnLgrSeq, SOE_REQUIRED); + SOElement (sfPreviousTxnLgrSeq, SOE_REQUIRED) << + SOElement (sfDestinationNode, SOE_OPTIONAL); add ("LedgerHashes", ltLEDGER_HASHES) << SOElement (sfFirstLedgerSequence, SOE_OPTIONAL) // Remove if we do a ledger restart diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 691c1bb8dac..a73dcd950e8 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -132,14 +132,15 @@ SF_U32 const sfSignerListID = make::one(&sfSignerListID, SF_U32 const sfSettleDelay = make::one(&sfSettleDelay, STI_UINT32, 39, "SettleDelay"); // 64-bit integers -SF_U64 const sfIndexNext = make::one(&sfIndexNext, STI_UINT64, 1, "IndexNext"); -SF_U64 const sfIndexPrevious = make::one(&sfIndexPrevious, STI_UINT64, 2, "IndexPrevious"); -SF_U64 const sfBookNode = make::one(&sfBookNode, STI_UINT64, 3, "BookNode"); -SF_U64 const sfOwnerNode = make::one(&sfOwnerNode, STI_UINT64, 4, "OwnerNode"); -SF_U64 const sfBaseFee = make::one(&sfBaseFee, STI_UINT64, 5, "BaseFee"); -SF_U64 const sfExchangeRate = make::one(&sfExchangeRate, STI_UINT64, 6, "ExchangeRate"); -SF_U64 const sfLowNode = make::one(&sfLowNode, STI_UINT64, 7, "LowNode"); -SF_U64 const sfHighNode = make::one(&sfHighNode, STI_UINT64, 8, "HighNode"); +SF_U64 const sfIndexNext = make::one(&sfIndexNext, STI_UINT64, 1, "IndexNext"); +SF_U64 const sfIndexPrevious = make::one(&sfIndexPrevious, STI_UINT64, 2, "IndexPrevious"); +SF_U64 const sfBookNode = make::one(&sfBookNode, STI_UINT64, 3, "BookNode"); +SF_U64 const sfOwnerNode = make::one(&sfOwnerNode, STI_UINT64, 4, "OwnerNode"); +SF_U64 const sfBaseFee = make::one(&sfBaseFee, STI_UINT64, 5, "BaseFee"); +SF_U64 const sfExchangeRate = make::one(&sfExchangeRate, STI_UINT64, 6, "ExchangeRate"); +SF_U64 const sfLowNode = make::one(&sfLowNode, STI_UINT64, 7, "LowNode"); +SF_U64 const sfHighNode = make::one(&sfHighNode, STI_UINT64, 8, "HighNode"); +SF_U64 const sfDestinationNode = make::one(&sfDestinationNode, STI_UINT64, 9, "DestinationNode"); // 128-bit SF_U128 const sfEmailHash = make::one(&sfEmailHash, STI_HASH128, 1, "EmailHash"); diff --git a/src/ripple/protocol/impl/SecretKey.cpp b/src/ripple/protocol/impl/SecretKey.cpp index 0a8dbca1e52..d40586c3863 100644 --- a/src/ripple/protocol/impl/SecretKey.cpp +++ b/src/ripple/protocol/impl/SecretKey.cpp @@ -56,35 +56,46 @@ SecretKey::to_string() const } //------------------------------------------------------------------------------ - -Generator::Generator (Seed const& seed) +/** Produces a sequence of secp256k1 key pairs. */ +class Generator { - uint128 ui; - std::memcpy(ui.data(), - seed.data(), seed.size()); - gen_ = generateRootDeterministicPublicKey(ui); -} +private: + Blob gen_; // VFALCO compile time size? -std::pair -Generator::operator()(Seed const& seed, std::size_t ordinal) const -{ - uint128 ui; - std::memcpy(ui.data(), seed.data(), seed.size()); - auto gsk = generatePrivateDeterministicKey(gen_, ui, ordinal); - auto gpk = generatePublicDeterministicKey(gen_, ordinal); - SecretKey const sk(Slice{ gsk.data(), gsk.size() }); - PublicKey const pk(Slice{ gpk.data(), gpk.size() }); - beast::secure_erase(ui.data(), ui.size()); - beast::secure_erase(gsk.data(), gsk.size()); - return { pk, sk }; -} +public: + explicit + Generator (Seed const& seed) + { + // FIXME: Avoid copying the seed into a uint128 key only to have + // generateRootDeterministicPublicKey copy out of it. + uint128 ui; + std::memcpy(ui.data(), + seed.data(), seed.size()); + gen_ = generateRootDeterministicPublicKey(ui); + } -PublicKey -Generator::operator()(std::size_t ordinal) const -{ - auto gpk = generatePublicDeterministicKey(gen_, ordinal); - return PublicKey(Slice{ gpk.data(), gpk.size() }); -} + /** Generate the nth key pair. + + The seed is required to produce the private key. + */ + std::pair + operator()(Seed const& seed, std::size_t ordinal) const + { + // FIXME: Avoid copying the seed into a uint128 key only to have + // generatePrivateDeterministicKey copy out of it. + uint128 ui; + std::memcpy(ui.data(), seed.data(), seed.size()); + auto gsk = generatePrivateDeterministicKey(gen_, ui, ordinal); + auto gpk = generatePublicDeterministicKey(gen_, ordinal); + SecretKey const sk(Slice + { gsk.data(), gsk.size() }); + PublicKey const pk(Slice + { gpk.data(), gpk.size() }); + beast::secure_erase(ui.data(), ui.size()); + beast::secure_erase(gsk.data(), gsk.size()); + return {pk, sk}; + } +}; //------------------------------------------------------------------------------ @@ -192,19 +203,25 @@ generateSecretKey (KeyType type, Seed const& seed) { if (type == KeyType::ed25519) { - auto const key = sha512Half_s(Slice( + auto key = sha512Half_s(Slice( seed.data(), seed.size())); - return SecretKey(Slice{ key.data(), key.size() }); + SecretKey sk = Slice{ key.data(), key.size() }; + beast::secure_erase(key.data(), key.size()); + return sk; } if (type == KeyType::secp256k1) { + // FIXME: Avoid copying the seed into a uint128 key only to have + // generateRootDeterministicPrivateKey copy out of it. uint128 ps; std::memcpy(ps.data(), seed.data(), seed.size()); auto const upk = generateRootDeterministicPrivateKey(ps); - return SecretKey(Slice{ upk.data(), upk.size() }); + SecretKey sk = Slice{ upk.data(), upk.size() }; + beast::secure_erase(ps.data(), ps.size()); + return sk; } LogicError ("generateSecretKey: unknown key type"); diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index b3a5269c085..1ce16fb5b46 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -99,7 +99,12 @@ transResults() { telFAILED_PROCESSING, { "telFAILED_PROCESSING", "Failed to correctly process transaction." } }, { telINSUF_FEE_P, { "telINSUF_FEE_P", "Fee insufficient." } }, { telNO_DST_PARTIAL, { "telNO_DST_PARTIAL", "Partial payment to create account not allowed." } }, - { telCAN_NOT_QUEUE, { "telCAN_NOT_QUEUE", "Can not queue at this time." } }, + { telCAN_NOT_QUEUE, { "telCAN_NOT_QUEUE", "Can not queue at this time." } }, + { telCAN_NOT_QUEUE_BALANCE, { "telCAN_NOT_QUEUE_BALANCE", "Can not queue at this time: insufficient balance to pay all queued fees." } }, + { telCAN_NOT_QUEUE_BLOCKS, { "telCAN_NOT_QUEUE_BLOCKS", "Can not queue at this time: would block later queued transaction(s)." } }, + { telCAN_NOT_QUEUE_BLOCKED, { "telCAN_NOT_QUEUE_BLOCKED", "Can not queue at this time: blocking transaction in queue." } }, + { telCAN_NOT_QUEUE_FEE, { "telCAN_NOT_QUEUE_FEE", "Can not queue at this time: fee insufficient to replace queued transaction." } }, + { telCAN_NOT_QUEUE_FULL, { "telCAN_NOT_QUEUE_FULL", "Can not queue at this time: queue is full." } }, { temMALFORMED, { "temMALFORMED", "Malformed transaction." } }, { temBAD_AMOUNT, { "temBAD_AMOUNT", "Can only send positive amounts." } }, diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index adb861a7717..f8df897d2ec 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -20,10 +20,13 @@ #include #include #include +#include #include #include #include #include +#include +#include namespace ripple { namespace test { @@ -120,6 +123,17 @@ struct Escrow_test : public beast::unit_test::suite return jv; } + static + Json::Value + lockup (jtx::Account const& account, jtx::Account const& to, + STAmount const& amount, NetClock::time_point const& expiry, + NetClock::time_point const& cancel) + { + Json::Value jv = lockup (account, to, amount, expiry); + jv["CancelAfter"] = cancel.time_since_epoch().count(); + return jv; + } + static Json::Value lockup (jtx::Account const& account, jtx::Account const& to, @@ -654,7 +668,7 @@ struct Escrow_test : public beast::unit_test::suite { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); - std::array cb = + std::array cb = {{ 0xA2, 0x2B, 0x80, 0x20, 0x42, 0x4A, 0x70, 0x49, 0x49, 0x52, 0x92, 0x67, 0xB6, 0x21, 0xB3, 0xD7, 0x91, 0x19, 0xD7, 0x29, @@ -671,18 +685,169 @@ struct Escrow_test : public beast::unit_test::suite } void - testMeta() + testMetaAndOwnership() { - testcase ("Metadata"); - using namespace jtx; using namespace std::chrono; - Env env(*this, with_features(featureEscrow)); - env.fund(XRP(5000), "alice", "bob", "carol"); - env(condpay("alice", "carol", XRP(1000), makeSlice(cb1), env.now() + 1s)); - auto const m = env.meta(); - BEAST_EXPECT((*m)[sfTransactionResult] == tesSUCCESS); + auto const alice = Account("alice"); + auto const bruce = Account("bruce"); + auto const carol = Account("carol"); + + { + testcase ("Metadata & Ownership (without fix1523)"); + Env env(*this, with_features(featureEscrow)); + env.fund(XRP(5000), alice, bruce, carol); + auto const seq = env.seq(alice); + + env(lockup(alice, carol, XRP(1000), env.now() + 1s)); + + BEAST_EXPECT((*env.meta())[sfTransactionResult] == 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)"); + + Env env(*this, with_features(featureEscrow, fix1523)); + env.fund(XRP(5000), alice, bruce, carol); + auto const aseq = env.seq(alice); + auto const bseq = env.seq(bruce); + + env(lockup(alice, alice, XRP(1000), env.now() + 1s, env.now() + 500s)); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + env.close(5s); + auto const aa = env.le(keylet::escrow(alice.id(), aseq)); + BEAST_EXPECT(aa); + + { + 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(), aa) != aod.end()); + } + + env(lockup(bruce, bruce, XRP(1000), env.now() + 1s, env.now() + 2s)); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + env.close(5s); + auto const bb = env.le(keylet::escrow(bruce.id(), bseq)); + BEAST_EXPECT(bb); + + { + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 1); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bb) != bod.end()); + } + + env.close(5s); + env(finish(alice, alice, aseq)); + { + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), aseq))); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + + ripple::Dir aod(*env.current(), keylet::ownerDir(alice.id())); + BEAST_EXPECT(std::distance(aod.begin(), aod.end()) == 0); + BEAST_EXPECT(std::find(aod.begin(), aod.end(), aa) == aod.end()); + + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 1); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bb) != bod.end()); + } + + env.close(5s); + env(cancel(bruce, bruce, bseq)); + { + BEAST_EXPECT(!env.le(keylet::escrow(bruce.id(), bseq))); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 0); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bb) == bod.end()); + } + } + + { + testcase ("Metadata (with fix1523, to other)"); + + Env env(*this, with_features(featureEscrow, fix1523)); + env.fund(XRP(5000), alice, bruce, carol); + auto const aseq = env.seq(alice); + auto const bseq = env.seq(bruce); + + env(lockup(alice, bruce, XRP(1000), env.now() + 1s)); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + env.close(5s); + env(lockup(bruce, carol, XRP(1000), env.now() + 1s, env.now() + 2s)); + BEAST_EXPECT((*env.meta())[sfTransactionResult] == tesSUCCESS); + env.close(5s); + + auto const ab = env.le(keylet::escrow(alice.id(), aseq)); + BEAST_EXPECT(ab); + + auto const bc = env.le(keylet::escrow(bruce.id(), bseq)); + BEAST_EXPECT(bc); + + { + 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(), ab) != aod.end()); + + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 2); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), ab) != bod.end()); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bc) != bod.end()); + + ripple::Dir cod(*env.current(), keylet::ownerDir(carol.id())); + BEAST_EXPECT(std::distance(cod.begin(), cod.end()) == 1); + BEAST_EXPECT(std::find(cod.begin(), cod.end(), bc) != cod.end()); + } + + env.close(5s); + env(finish(alice, alice, aseq)); + { + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), aseq))); + BEAST_EXPECT(env.le(keylet::escrow(bruce.id(), bseq))); + + ripple::Dir aod(*env.current(), keylet::ownerDir(alice.id())); + BEAST_EXPECT(std::distance(aod.begin(), aod.end()) == 0); + BEAST_EXPECT(std::find(aod.begin(), aod.end(), ab) == aod.end()); + + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 1); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), ab) == bod.end()); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bc) != bod.end()); + + ripple::Dir cod(*env.current(), keylet::ownerDir(carol.id())); + BEAST_EXPECT(std::distance(cod.begin(), cod.end()) == 1); + } + + env.close(5s); + env(cancel(bruce, bruce, bseq)); + { + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), aseq))); + BEAST_EXPECT(!env.le(keylet::escrow(bruce.id(), bseq))); + + ripple::Dir aod(*env.current(), keylet::ownerDir(alice.id())); + BEAST_EXPECT(std::distance(aod.begin(), aod.end()) == 0); + BEAST_EXPECT(std::find(aod.begin(), aod.end(), ab) == aod.end()); + + ripple::Dir bod(*env.current(), keylet::ownerDir(bruce.id())); + BEAST_EXPECT(std::distance(bod.begin(), bod.end()) == 0); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), ab) == bod.end()); + BEAST_EXPECT(std::find(bod.begin(), bod.end(), bc) == bod.end()); + + ripple::Dir cod(*env.current(), keylet::ownerDir(carol.id())); + BEAST_EXPECT(std::distance(cod.begin(), cod.end()) == 0); + } + } } void testConsequences() @@ -745,7 +910,7 @@ struct Escrow_test : public beast::unit_test::suite testFails(); testLockup(); testEscrowConditions(); - testMeta(); + testMetaAndOwnership(); testConsequences(); } }; diff --git a/src/test/app/HashRouter_test.cpp b/src/test/app/HashRouter_test.cpp index 1cd52b9734a..85178e08a16 100644 --- a/src/test/app/HashRouter_test.cpp +++ b/src/test/app/HashRouter_test.cpp @@ -32,7 +32,7 @@ class HashRouter_test : public beast::unit_test::suite { using namespace std::chrono_literals; TestStopwatch stopwatch; - HashRouter router(stopwatch, 2s); + HashRouter router(stopwatch, 2s, 2); uint256 const key1(1); uint256 const key2(2); @@ -69,7 +69,7 @@ class HashRouter_test : public beast::unit_test::suite { using namespace std::chrono_literals; TestStopwatch stopwatch; - HashRouter router(stopwatch, 2s); + HashRouter router(stopwatch, 2s, 2); uint256 const key1(1); uint256 const key2(2); @@ -148,7 +148,7 @@ class HashRouter_test : public beast::unit_test::suite // Normal HashRouter using namespace std::chrono_literals; TestStopwatch stopwatch; - HashRouter router(stopwatch, 2s); + HashRouter router(stopwatch, 2s, 2); uint256 const key1(1); uint256 const key2(2); @@ -178,7 +178,7 @@ class HashRouter_test : public beast::unit_test::suite { using namespace std::chrono_literals; TestStopwatch stopwatch; - HashRouter router(stopwatch, 2s); + HashRouter router(stopwatch, 2s, 2); uint256 const key1(1); BEAST_EXPECT(router.setFlags(key1, 10)); @@ -191,7 +191,7 @@ class HashRouter_test : public beast::unit_test::suite { using namespace std::chrono_literals; TestStopwatch stopwatch; - HashRouter router(stopwatch, 1s); + HashRouter router(stopwatch, 1s, 2); uint256 const key1(1); @@ -229,6 +229,41 @@ class HashRouter_test : public beast::unit_test::suite BEAST_EXPECT(peers && peers->size() == 0); } + void + testRecover() + { + using namespace std::chrono_literals; + TestStopwatch stopwatch; + HashRouter router(stopwatch, 1s, 5); + + uint256 const key1(1); + + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(!router.shouldRecover(key1)); + // Expire, but since the next search will + // be for this entry, it will get refreshed + // instead. + ++stopwatch; + BEAST_EXPECT(router.shouldRecover(key1)); + // Expire, but since the next search will + // be for this entry, it will get refreshed + // instead. + ++stopwatch; + // Recover again. Recovery is independent of + // time as long as the entry doesn't expire. + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(router.shouldRecover(key1)); + // Expire again + ++stopwatch; + BEAST_EXPECT(router.shouldRecover(key1)); + BEAST_EXPECT(!router.shouldRecover(key1)); + } + public: void @@ -239,6 +274,7 @@ class HashRouter_test : public beast::unit_test::suite testSuppression(); testSetFlags(); testRelay(); + testRecover(); } }; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 903e9d230a7..9782d3d08ab 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -109,6 +109,7 @@ class TxQ_test : public beast::unit_test::suite auto p = test::jtx::envconfig(); auto& section = p->section("transaction_queue"); section.set("ledgers_in_queue", "2"); + section.set("minimum_queue_size", "2"); section.set("min_ledgers_to_compute_size_limit", "3"); section.set("max_ledger_counts_to_store", "100"); section.set("retry_sequence_percent", "25"); @@ -240,7 +241,7 @@ class TxQ_test : public beast::unit_test::suite // Hank sees his txn got held and bumps the fee, // but doesn't even bump it enough to requeue - env(noop(hank), fee(11), ter(telINSUF_FEE_P)); + env(noop(hank), fee(11), ter(telCAN_NOT_QUEUE_FEE)); checkMetrics(env, 2, 12, 7, 6, 256); // Hank sees his txn got held and bumps the fee, @@ -303,7 +304,7 @@ class TxQ_test : public beast::unit_test::suite // Try to add another transaction with the default (low) fee, // it should fail because the queue is full. - env(noop(charlie), ter(telINSUF_FEE_P)); + env(noop(charlie), ter(telCAN_NOT_QUEUE_FULL)); // Add another transaction, with a higher fee, // Not high enough to get into the ledger, but high @@ -441,7 +442,7 @@ class TxQ_test : public beast::unit_test::suite BEAST_EXPECT(env.current()->info().seq == 6); // Fail to queue an item with a low LastLedgerSeq env(noop(alice), json(R"({"LastLedgerSequence":7})"), - ter(telINSUF_FEE_P)); + ter(telCAN_NOT_QUEUE)); // Queue an item with a sufficient LastLedgerSeq. env(noop(alice), json(R"({"LastLedgerSequence":8})"), queued); @@ -599,7 +600,7 @@ class TxQ_test : public beast::unit_test::suite // average fee. (Which is ~144,115,188,075,855,907 // because of the zero fee txn.) env(noop(carol), fee(feeCarol), - seq(seqCarol), ter(telINSUF_FEE_P)); + seq(seqCarol), ter(telCAN_NOT_QUEUE_FULL)); env.close(); // Some of Bob's transactions stay in the queue, @@ -819,13 +820,13 @@ class TxQ_test : public beast::unit_test::suite // queue. env(noop(alice), seq(aliceSeq), json(jss::LastLedgerSequence, lastLedgerSeq + 7), - fee(aliceFee), ter(telCAN_NOT_QUEUE)); + fee(aliceFee), ter(telCAN_NOT_QUEUE_FULL)); checkMetrics(env, 8, 8, 5, 4, 513); // Charlie - try to add another item to the queue, // which fails because fee is lower than Alice's // queued average. - env(noop(charlie), fee(19), ter(telINSUF_FEE_P)); + env(noop(charlie), fee(19), ter(telCAN_NOT_QUEUE_FULL)); checkMetrics(env, 8, 8, 5, 4, 513); // Charlie - add another item to the queue, which @@ -844,7 +845,7 @@ class TxQ_test : public beast::unit_test::suite // so resubmits with higher fee, but the queue // is full, and her account is the cheapest. env(noop(alice), seq(aliceSeq - 1), - fee(aliceFee), ter(telCAN_NOT_QUEUE)); + fee(aliceFee), ter(telCAN_NOT_QUEUE_FULL)); checkMetrics(env, 8, 8, 5, 4, 513); // Try to replace a middle item in the queue @@ -852,7 +853,7 @@ class TxQ_test : public beast::unit_test::suite aliceSeq = env.seq(alice) + 2; aliceFee = 25; env(noop(alice), seq(aliceSeq), - fee(aliceFee), ter(telINSUF_FEE_P)); + fee(aliceFee), ter(telCAN_NOT_QUEUE_FEE)); checkMetrics(env, 8, 8, 5, 4, 513); // Replace a middle item from the queue successfully @@ -876,7 +877,7 @@ class TxQ_test : public beast::unit_test::suite aliceFee = env.le(alice)->getFieldAmount(sfBalance).xrp().drops() - (59); env(noop(alice), seq(aliceSeq), - fee(aliceFee), ter(telCAN_NOT_QUEUE)); + fee(aliceFee), ter(telCAN_NOT_QUEUE_BALANCE)); checkMetrics(env, 4, 10, 6, 5, 256); // Try to spend more than Alice can afford with all the other txs. @@ -898,7 +899,7 @@ class TxQ_test : public beast::unit_test::suite aliceFee /= 5; ++aliceSeq; env(noop(alice), seq(aliceSeq), - fee(aliceFee), ter(telCAN_NOT_QUEUE)); + fee(aliceFee), ter(telCAN_NOT_QUEUE_BALANCE)); checkMetrics(env, 4, 10, 6, 5, 256); env.close(); @@ -1004,7 +1005,7 @@ class TxQ_test : public beast::unit_test::suite // Try to add another transaction with the default (low) fee, // it should fail because it can't replace the one already // there. - env(noop(charlie), ter(telINSUF_FEE_P)); + env(noop(charlie), ter(telCAN_NOT_QUEUE_FEE)); // Add another transaction, with a higher fee, // Not high enough to get into the ledger, but high @@ -1114,7 +1115,7 @@ class TxQ_test : public beast::unit_test::suite // is still uninitialized, so preflight succeeds here, // and this txn fails because it can't be stored in the queue. env(noop(alice), json(R"({"AccountTxnID": "0"})"), - ter(telINSUF_FEE_P)); + ter(telCAN_NOT_QUEUE)); checkMetrics(env, 0, boost::none, 2, 1, 256); env.close(); @@ -1217,7 +1218,7 @@ class TxQ_test : public beast::unit_test::suite // Try adding a new transaction. // Too many fees in flight. env(noop(alice), fee(drops(200)), seq(aliceSeq+1), - ter(telCAN_NOT_QUEUE)); + ter(telCAN_NOT_QUEUE_BALANCE)); checkMetrics(env, 4, 6, 5, 3, 256); // Close the ledger. All of Alice's transactions @@ -1229,7 +1230,7 @@ class TxQ_test : public beast::unit_test::suite // Still can't add a new transaction for Alice, // no matter the fee. env(noop(alice), fee(drops(200)), seq(aliceSeq + 1), - ter(telCAN_NOT_QUEUE)); + ter(telCAN_NOT_QUEUE_BALANCE)); checkMetrics(env, 1, 10, 3, 5, 256); /* At this point, Alice's transaction is indefinitely @@ -1288,12 +1289,12 @@ class TxQ_test : public beast::unit_test::suite env(noop(alice), seq(aliceSeq + 2), queued); // Can't replace the first tx with a blocker - env(fset(alice, asfAccountTxnID), fee(20), ter(telINSUF_FEE_P)); + env(fset(alice, asfAccountTxnID), fee(20), ter(telCAN_NOT_QUEUE_BLOCKS)); // Can't replace the second / middle tx with a blocker env(regkey(alice, bob), seq(aliceSeq + 1), fee(20), - ter(telCAN_NOT_QUEUE)); + ter(telCAN_NOT_QUEUE_BLOCKS)); env(signers(alice, 2, { {bob}, {charlie}, {daria} }), fee(20), - seq(aliceSeq + 1), ter(telCAN_NOT_QUEUE)); + seq(aliceSeq + 1), ter(telCAN_NOT_QUEUE_BLOCKS)); // CAN replace the last tx with a blocker env(signers(alice, 2, { { bob },{ charlie },{ daria } }), fee(20), seq(aliceSeq + 2), queued); @@ -1301,7 +1302,7 @@ class TxQ_test : public beast::unit_test::suite queued); // Can't queue up any more transactions after the blocker - env(noop(alice), seq(aliceSeq + 3), ter(telCAN_NOT_QUEUE)); + env(noop(alice), seq(aliceSeq + 3), ter(telCAN_NOT_QUEUE_BLOCKED)); // Other accounts are not affected env(noop(bob), queued); @@ -2108,7 +2109,7 @@ class TxQ_test : public beast::unit_test::suite } } - envs(noop(alice), fee(none), seq(none), ter(telCAN_NOT_QUEUE))(submitParams); + envs(noop(alice), fee(none), seq(none), ter(telCAN_NOT_QUEUE_BLOCKED))(submitParams); checkMetrics(env, 5, 6, 4, 3, 256); { diff --git a/src/test/consensus/Consensus_test.cpp b/src/test/consensus/Consensus_test.cpp index 1221958ad86..669bbf9fc20 100644 --- a/src/test/consensus/Consensus_test.cpp +++ b/src/test/consensus/Consensus_test.cpp @@ -521,6 +521,152 @@ class Consensus_test : public beast::unit_test::suite } } + void + testConsensusCloseTimeRounding() + { + using namespace csf; + using namespace std::chrono; + + // 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. + + for (bool useRoundedCloseTime : {false, true}) + { + ConsensusParms parms; + parms.useRoundedCloseTime = useRoundedCloseTime; + std::vector unls; + unls.push_back({0,1,2,3,4,5}); + + std::vector membership(unls[0].size(), 0); + + TrustGraph tg{unls, membership}; + + // 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. + + Sim sim( + parms, + tg, + topology(tg, [&](PeerID i, PeerID j) { + auto delayFactor = (i <= 1 || j <= 1) ? 1.1 : 0.2; + return round( + delayFactor * parms.ledgerGRANULARITY); + })); + + // 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 = sim.peers[0].now().time_since_epoch(); + + // Check we are before the 30s to 20s transition + NetClock::duration resolution = + sim.peers[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.net.step_for( + NetClock::time_point{when} - sim.peers[0].now()); + + // Run one more ledger with 30s resolution + sim.run(1); + + // close time should be ahead of clock time since we engineered + // the close time to round up + for (Peer const & peer : sim.peers) + { + BEAST_EXPECT( + peer.lastClosedLedger.closeTime() > peer.now()); + BEAST_EXPECT(peer.lastClosedLedger.closeAgree()); + BEAST_EXPECT( + peer.lastClosedLedger.id() == + sim.peers[0].lastClosedLedger.id()); + } + + + // All peers submit their own ID as a transaction + for (Peer & peer : sim.peers) + peer.submit(Tx{static_cast(peer.id)}); + + // Run 1 more round, this time it will have a decreased + // resolution of 20 seconds. + + // 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! + + sim.run(1); + + for (Peer const& peer : sim.peers) + { + BEAST_EXPECT( + peer.lastClosedLedger.id() == + sim.peers[0].lastClosedLedger.id()); + } + + if (!parms.useRoundedCloseTime) + { + auto const & slowLCL = sim.peers[0].lastClosedLedger; + auto const & fastLCL = sim.peers[2].lastClosedLedger; + + + // Agree on parent close and close resolution + BEAST_EXPECT( + slowLCL.parentCloseTime() == + fastLCL.parentCloseTime()); + BEAST_EXPECT( + slowLCL.closeTimeResolution() == + fastLCL.closeTimeResolution()); + + auto parentClose = slowLCL.parentCloseTime(); + auto closeResolution = + slowLCL.closeTimeResolution(); + + auto slowClose = sim.peers[0].lastClosedLedger.closeTime(); + auto fastClose = sim.peers[2].lastClosedLedger.closeTime(); + + // Close times disagree ... + BEAST_EXPECT(slowClose != fastClose); + // Effective close times agree! The slow peer already rounded! + BEAST_EXPECT( + effCloseTime(slowClose, closeResolution, parentClose) == + effCloseTime(fastClose, closeResolution, parentClose)); + } + } + } + void testFork() { @@ -691,6 +837,7 @@ class Consensus_test : public beast::unit_test::suite testSlowPeers(); testCloseTimeDisagree(); testWrongLCL(); + testConsensusCloseTimeRounding(); testFork(); simClockSkew(); diff --git a/src/test/csf/Ledger.h b/src/test/csf/Ledger.h index addc0642738..03d3fef001c 100644 --- a/src/test/csf/Ledger.h +++ b/src/test/csf/Ledger.h @@ -102,12 +102,6 @@ class Ledger return closeTime_; } - auto - actualCloseTime() const - { - return actualCloseTime_; - } - auto parentCloseTime() const { @@ -140,9 +134,8 @@ class Ledger res.id_.txs.insert(txs.begin(), txs.end()); res.id_.seq = seq() + 1; res.closeTimeResolution_ = closeTimeResolution; - res.actualCloseTime_ = consensusCloseTime; res.closeTime_ = effCloseTime( - consensusCloseTime, closeTimeResolution, parentCloseTime_); + consensusCloseTime, closeTimeResolution, closeTime()); res.closeTimeAgree_ = closeTimeAgree; res.parentCloseTime_ = closeTime(); res.parentID_ = id(); @@ -167,9 +160,6 @@ class Ledger //! Parent ledger close time NetClock::time_point parentCloseTime_; - - //! Close time unadjusted by closeTimeResolution - NetClock::time_point actualCloseTime_; }; inline std::ostream& diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 41e5aae9ddc..f877f38aa7a 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -311,7 +311,7 @@ struct Peer auto newLedger = prevLedger.close( result.set.txs_, closeResolution, - rawCloseTimes.self, + result.position.closeTime(), result.position.closeTime() != NetClock::time_point{}); ledgers[newLedger.id()] = newLedger; prevProposers_ = result.proposers;