diff --git a/CMakeLists.txt b/CMakeLists.txt index b120f1123a..ab0b2971a1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1487,6 +1487,7 @@ install ( src/ripple/basics/StringUtilities.h src/ripple/basics/ToString.h src/ripple/basics/UnorderedContainers.h + src/ripple/basics/algorithm.h src/ripple/basics/base_uint.h src/ripple/basics/chrono.h src/ripple/basics/contract.h diff --git a/src/ripple/app/consensus/RCLCensorshipDetector.h b/src/ripple/app/consensus/RCLCensorshipDetector.h index ead1bb9124..3639039272 100644 --- a/src/ripple/app/consensus/RCLCensorshipDetector.h +++ b/src/ripple/app/consensus/RCLCensorshipDetector.h @@ -20,80 +20,71 @@ #ifndef RIPPLE_APP_CONSENSUS_RCLCENSORSHIPDETECTOR_H_INCLUDED #define RIPPLE_APP_CONSENSUS_RCLCENSORSHIPDETECTOR_H_INCLUDED +#include #include #include -#include +#include +#include namespace ripple { template class RCLCensorshipDetector { -private: - std::map tracker_; - - /** Removes all elements satisfying specific criteria from the tracker - - @param pred A predicate which returns true for tracking entries that - should be removed. The predicate must be callable as: - bool pred(TxID const&, Sequence) - It must return true for entries that should be removed. - - @note This could be replaced with std::erase_if when it becomes part - of the standard. For example: - - prune ([](TxID const& id, Sequence seq) - { - return id.isZero() || seq == 314159; - }); - - would become: - - std::erase_if(tracker_.begin(), tracker_.end(), - [](auto const& e) - { - return e.first.isZero() || e.second == 314159; - } - */ - template - void prune(Predicate&& pred) +public: + struct TxIDSeq { - auto t = tracker_.begin(); + TxID txid; + Sequence seq; - while (t != tracker_.end()) - { - if (pred(t->first, t->second)) - t = tracker_.erase(t); - else - t = std::next(t); - } + TxIDSeq (TxID const& txid_, Sequence const& seq_) + : txid (txid_) + , seq (seq_) + { } + }; + + friend bool operator< (TxIDSeq const& lhs, TxIDSeq const& rhs) + { + if (lhs.txid != rhs.txid) + return lhs.txid < rhs.txid; + return lhs.seq < rhs.seq; } + friend bool operator< (TxIDSeq const& lhs, TxID const& rhs) + { + return lhs.txid < rhs; + } + + friend bool operator< (TxID const& lhs, TxIDSeq const& rhs) + { + return lhs < rhs.txid; + } + + using TxIDSeqVec = std::vector; + +private: + TxIDSeqVec tracker_; + public: RCLCensorshipDetector() = default; /** Add transactions being proposed for the current consensus round. - @param seq The sequence number of the ledger being built. @param proposed The set of transactions that we are initially proposing for this round. */ - void propose( - Sequence seq, - std::vector proposed) + void propose(TxIDSeqVec proposed) { - std::sort (proposed.begin(), proposed.end()); - // We want to remove any entries that we proposed in a previous round // that did not make it in yet if we are no longer proposing them. - prune ([&proposed](TxID const& id, Sequence seq) - { - return !std::binary_search(proposed.begin(), proposed.end(), id); - }); - - // Track the entries that we are proposing in this round. - for (auto const& p : proposed) - tracker_.emplace(p, seq); // FIXME C++17: use try_emplace + // And we also want to preserve the Sequence of entries that we proposed + // in the last round and want to propose again. + std::sort(proposed.begin(), proposed.end()); + generalized_set_intersection(proposed.begin(), proposed.end(), + tracker_.cbegin(), tracker_.cend(), + [](auto& x, auto const& y) {x.seq = y.seq;}, + [](auto const& x, auto const& y) {return x.txid < y.txid;}); + tracker_ = std::move(proposed); } /** Determine which transactions made it and perform censorship detection. @@ -114,17 +105,18 @@ public: std::vector accepted, Predicate&& pred) { - std::sort (accepted.begin(), accepted.end()); + auto acceptTxid = accepted.begin(); + auto const ae = accepted.end(); + std::sort(acceptTxid, ae); // We want to remove all tracking entries for transactions that were // accepted as well as those which match the predicate. - prune ([&pred, &accepted](TxID const& id, Sequence seq) - { - if (std::binary_search(accepted.begin(), accepted.end(), id)) - return true; - return pred(id, seq); - }); + auto i = remove_if_intersect_or_match(tracker_.begin(), tracker_.end(), + accepted.begin(), accepted.end(), + [&pred](auto const& x) {return pred(x.txid, x.seq);}, + std::less{}); + tracker_.erase(i, tracker_.end()); } /** Removes all elements from the tracker diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 3c1de1a596..6a4ec04c53 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -330,15 +330,16 @@ RCLConsensus::Adaptor::onClose( if (!wrongLCL) { - std::vector proposed; + LedgerIndex const seq = prevLedger->info().seq + 1; + RCLCensorshipDetector::TxIDSeqVec proposed; initialSet->visitLeaves( - [&proposed](std::shared_ptr const& item) + [&proposed, seq](std::shared_ptr const& item) { - proposed.push_back(item->key()); + proposed.emplace_back(item->key(), seq); }); - censorshipDetector_.propose(prevLedger->info().seq + 1, std::move(proposed)); + censorshipDetector_.propose(std::move(proposed)); } // Needed because of the move below. diff --git a/src/ripple/basics/algorithm.h b/src/ripple/basics/algorithm.h new file mode 100644 index 0000000000..cc9d4aaf38 --- /dev/null +++ b/src/ripple/basics/algorithm.h @@ -0,0 +1,110 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2019 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_ALGORITHM_H_INCLUDED +#define RIPPLE_ALGORITHM_H_INCLUDED + +#include + +namespace ripple { + +// Requires: [first1, last1) and [first2, last2) are ordered ranges according to +// comp. + +// Effects: For each pair of elements {i, j} in the intersection of the sorted +// sequences [first1, last1) and [first2, last2), perform action(i, j). + +// Note: This algorithm is evolved from std::set_intersection. +template +void +generalized_set_intersection(InputIter1 first1, InputIter1 last1, + InputIter2 first2, InputIter2 last2, + Action action, Comp comp) +{ + while (first1 != last1 && first2 != last2) + { + + if (comp(*first1, *first2)) // if *first1 < *first2 + ++first1; // then reduce first range + else + { + if (!comp(*first2, *first1)) // if *first1 == *first2 + { // then this is an intersection + action(*first1, *first2); // do the action + ++first1; // reduce first range + } + ++first2; // Reduce second range because *first2 <= *first1 + } + } +} + +// Requires: [first1, last1) and [first2, last2) are ordered ranges according to +// comp. + +// Effects: Eliminates all the elements i in the range [first1, last1) which are +// equivalent to some value in [first2, last2) or for which pred(i) returns true. + +// Returns: A FwdIter1 E such that [first1, E) is the range of elements not +// removed by this algorithm. + +// Note: This algorithm is evolved from std::remove_if and std::set_intersection. +template +FwdIter1 +remove_if_intersect_or_match(FwdIter1 first1, FwdIter1 last1, + InputIter2 first2, InputIter2 last2, + Pred pred, Comp comp) +{ + // [original-first1, current-first1) is the set of elements to be preserved. + // [current-first1, i) is the set of elements that have been removed. + // [i, last1) is the set of elements not tested yet. + + // Test each *i in [first1, last1) against [first2, last2) and pred + for (auto i = first1; i != last1;) + { + // if (*i is not in [first2, last2) + if (first2 == last2 || comp(*i, *first2)) + { + if (!pred(*i)) + { + // *i should not be removed, so append it to the preserved set + if (i != first1) + *first1 = std::move(*i); + ++first1; + } + // *i has been fully tested, prepare for next i by + // appending i to the removed set, whether or not + // it has been moved from above. + ++i; + } + else // *i might be in [first2, last2) because *i >= *first2 + { + if (!comp(*first2, *i)) // if *i == *first2 + ++i; // then append *i to the removed set + // All elements in [i, last1) are known to be greater than *first2, + // so reduce the second range. + ++first2; + } + // Continue to test *i against [first2, last2) and pred + } + return first1; +} + +} // ripple + +#endif diff --git a/src/test/app/RCLCensorshipDetector_test.cpp b/src/test/app/RCLCensorshipDetector_test.cpp index 6c4fd6d426..e876557d01 100644 --- a/src/test/app/RCLCensorshipDetector_test.cpp +++ b/src/test/app/RCLCensorshipDetector_test.cpp @@ -33,7 +33,10 @@ class RCLCensorshipDetector_test : public beast::unit_test::suite std::vector remain, std::vector remove) { // Begin tracking what we're proposing this round - cdet.propose(round, std::move(proposed)); + RCLCensorshipDetector::TxIDSeqVec proposal; + for (auto const& i : proposed) + proposal.emplace_back(i, round); + cdet.propose(std::move(proposal)); // Finalize the round, by processing what we accepted; then // remove anything that needs to be removed and ensure that @@ -68,7 +71,7 @@ public: RCLCensorshipDetector cdet; int round = 0; - + // proposed accepted remain remove test(cdet, ++round, { }, { }, { }, { }); test(cdet, ++round, { 10, 11, 12, 13 }, { 11, 2 }, { 10, 13 }, { }); test(cdet, ++round, { 10, 13, 14, 15 }, { 14 }, { 10, 13, 15 }, { });