diff --git a/Builds/VisualStudio2013/RippleD.vcxproj b/Builds/VisualStudio2013/RippleD.vcxproj index b14790c2e7..a57bff7fec 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj +++ b/Builds/VisualStudio2013/RippleD.vcxproj @@ -1529,7 +1529,7 @@ True True - + True diff --git a/Builds/VisualStudio2013/RippleD.vcxproj.filters b/Builds/VisualStudio2013/RippleD.vcxproj.filters index 0bf6cffea5..3fa7f2d0c1 100644 --- a/Builds/VisualStudio2013/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2013/RippleD.vcxproj.filters @@ -2280,7 +2280,7 @@ ripple\app\misc - + ripple\app\misc diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 69c2124889..7b62f6c5ff 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -27,7 +27,7 @@ #include #include #include -#include +#include #include #include #include @@ -1294,7 +1294,7 @@ void Ledger::updateSkipList () */ bool Ledger::pendSaveValidated (bool isSynchronous, bool isCurrent) { - if (!getApp().getHashRouter ().setFlag (getHash (), SF_SAVED)) + if (!getApp().getHashRouter ().setFlags (getHash (), SF_SAVED)) { WriteLog (lsDEBUG, Ledger) << "Double pend save for " << info().seq; return true; diff --git a/src/ripple/app/ledger/OpenLedger.h b/src/ripple/app/ledger/OpenLedger.h index 13eedeebbc..601c7cc5ac 100644 --- a/src/ripple/app/ledger/OpenLedger.h +++ b/src/ripple/app/ledger/OpenLedger.h @@ -24,7 +24,7 @@ #include #include #include -#include +#include #include #include #include @@ -151,7 +151,7 @@ public: std::shared_ptr const& ledger, OrderedTxs const& locals, bool retriesFirst, OrderedTxs& retries, ApplyFlags flags, - IHashRouter& router, + HashRouter& router, std::string const& suffix = ""); /** Algorithm for applying transactions. @@ -164,7 +164,7 @@ public: void apply (OpenView& view, ReadView const& check, FwdRange const& txs, OrderedTxs& retries, - ApplyFlags flags, IHashRouter& router, + ApplyFlags flags, HashRouter& router, Config const& config, beast::Journal j); private: @@ -183,7 +183,7 @@ private: Result apply_one (OpenView& view, std::shared_ptr< STTx const> const& tx, bool retry, - ApplyFlags flags, IHashRouter& router, + ApplyFlags flags, HashRouter& router, Config const& config, beast::Journal j); public: @@ -205,7 +205,7 @@ void OpenLedger::apply (OpenView& view, ReadView const& check, FwdRange const& txs, OrderedTxs& retries, ApplyFlags flags, - IHashRouter& router, Config const& config, + HashRouter& router, Config const& config, beast::Journal j) { for (auto iter = txs.begin(); diff --git a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp index 7967a35121..24f0da8a42 100644 --- a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp +++ b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include @@ -1294,7 +1294,7 @@ void LedgerConsensusImp::addDisputedTransaction ( } // If we didn't relay this transaction recently, relay it - if (getApp().getHashRouter ().setFlag (txID, SF_RELAYED)) + if (getApp().getHashRouter ().setFlags (txID, SF_RELAYED)) { protocol::TMTransaction msg; msg.set_rawtransaction (& (tx.front ()), tx.size ()); diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 530ea2d476..3bda72caeb 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/ripple/app/ledger/impl/OpenLedger.cpp b/src/ripple/app/ledger/impl/OpenLedger.cpp index f2a2cfb806..215a65e93c 100644 --- a/src/ripple/app/ledger/impl/OpenLedger.cpp +++ b/src/ripple/app/ledger/impl/OpenLedger.cpp @@ -77,7 +77,7 @@ OpenLedger::accept(Rules const& rules, std::shared_ptr const& ledger, OrderedTxs const& locals, bool retriesFirst, OrderedTxs& retries, ApplyFlags flags, - IHashRouter& router, std::string const& suffix) + HashRouter& router, std::string const& suffix) { JLOG(j_.error) << "accept ledger " << ledger->seq() << " " << suffix; @@ -135,7 +135,7 @@ auto OpenLedger::apply_one (OpenView& view, std::shared_ptr const& tx, bool retry, ApplyFlags flags, - IHashRouter& router, Config const& config, + HashRouter& router, Config const& config, beast::Journal j) -> Result { if (retry) diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index bbcafa53da..0b86f215ee 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -35,7 +35,7 @@ #include #include #include -#include +#include #include #include #include @@ -292,7 +292,7 @@ public: std::unique_ptr serverHandler_; std::unique_ptr m_amendmentTable; std::unique_ptr mFeeTrack; - std::unique_ptr mHashRouter; + std::unique_ptr mHashRouter; std::unique_ptr mValidations; std::unique_ptr m_loadManager; beast::DeadlineTimer m_sweepTimer; @@ -411,7 +411,8 @@ public: , mFeeTrack (std::make_unique(m_logs.journal("LoadManager"))) - , mHashRouter (IHashRouter::New (IHashRouter::getDefaultHoldTime ())) + , mHashRouter (std::make_unique( + HashRouter::getDefaultHoldTime ())) , mValidations (make_Validations ()) @@ -574,7 +575,7 @@ public: return *mFeeTrack; } - IHashRouter& getHashRouter () + HashRouter& getHashRouter () { return *mHashRouter; } @@ -1325,7 +1326,7 @@ bool ApplicationImp::loadOldLedger ( cur->rawTxInsert(item.key(), std::make_shared( std::move(s)), nullptr); - getApp().getHashRouter().setFlag (item.key(), SF_SIGGOOD); + getApp().getHashRouter().setFlags (item.key(), SF_SIGGOOD); } // Switch to the mutable snapshot diff --git a/src/ripple/app/main/Application.h b/src/ripple/app/main/Application.h index 9d6d4f44fd..d17240f7d1 100644 --- a/src/ripple/app/main/Application.h +++ b/src/ripple/app/main/Application.h @@ -42,7 +42,7 @@ class CollectorManager; namespace shamap { class Family; } // shamap -class IHashRouter; +class HashRouter; class Logs; class LoadFeeTrack; class LocalCredentials; @@ -99,7 +99,7 @@ public: virtual NodeCache& getTempNodeCache () = 0; virtual CachedSLEs& cachedSLEs() = 0; virtual AmendmentTable& getAmendmentTable() = 0; - virtual IHashRouter& getHashRouter () = 0; + virtual HashRouter& getHashRouter () = 0; virtual LoadFeeTrack& getFeeTrack () = 0; virtual LoadManager& getLoadManager () = 0; virtual Overlay& overlay () = 0; diff --git a/src/ripple/app/misc/HashRouter.cpp b/src/ripple/app/misc/HashRouter.cpp index 1198871228..1d1f300042 100644 --- a/src/ripple/app/misc/HashRouter.cpp +++ b/src/ripple/app/misc/HashRouter.cpp @@ -18,117 +18,14 @@ //============================================================================== #include -#include +#include #include -#include -#include #include #include #include namespace ripple { -// VFALCO TODO Inline the function definitions -class HashRouter : public IHashRouter -{ -private: - /** An entry in the routing table. - */ - class Entry : public CountedObject - { - public: - static char const* getCountedObjectName () { return "HashRouterEntry"; } - - Entry () - : mFlags (0) - { - } - - std::set const& peekPeers () const - { - return mPeers; - } - - void addPeer (PeerShortID peer) - { - if (peer != 0) - mPeers.insert (peer); - } - - bool hasPeer (PeerShortID peer) const - { - return mPeers.count (peer) > 0; - } - - int getFlags (void) const - { - return mFlags; - } - - bool hasFlag (int mask) const - { - return (mFlags & mask) != 0; - } - - void setFlag (int flagsToSet) - { - mFlags |= flagsToSet; - } - - void clearFlag (int flagsToClear) - { - mFlags &= ~flagsToClear; - } - - void swapSet (std::set & other) - { - mPeers.swap (other); - } - - private: - int mFlags; - std::set mPeers; - }; - -public: - explicit HashRouter (int holdTime) - : mHoldTime (holdTime) - { - } - - bool addSuppression (uint256 const& index) override; - - bool addSuppressionPeer (uint256 const& index, PeerShortID peer) override; - bool addSuppressionPeer (uint256 const& index, PeerShortID peer, int& flags) override; - bool addSuppressionFlags (uint256 const& index, int flag) override; - bool setFlag (uint256 const& index, int flag) override; - int getFlags (uint256 const& index) override; - - bool swapSet (uint256 const& index, std::set& peers, int flag) override; - - std::function)> - sigVerify() override; - -private: - Entry getEntry (uint256 const& ); - - Entry& findCreateEntry (uint256 const& , bool& created); - - using MutexType = std::mutex; - using ScopedLockType = std::lock_guard ; - MutexType mMutex; - - // Stores all suppressed hashes and their expiration time - hash_map mSuppressionMap; - - // Stores all expiration times and the hashes indexed for them - std::map< int, std::list > mSuppressionTimes; - - int mHoldTime; -}; - -//------------------------------------------------------------------------------ - std::function)> HashRouter::sigVerify() { @@ -143,10 +40,10 @@ HashRouter::sigVerify() return false; if (! sigCheck(tx)) { - setFlag(id, SF_BAD); + setFlags(id, SF_BAD); return false; } - setFlag(id, SF_SIGGOOD); + setFlags(id, SF_SIGGOOD); return true; }; } @@ -230,28 +127,28 @@ bool HashRouter::addSuppressionFlags (uint256 const& index, int flag) ScopedLockType lock (mMutex); bool created; - findCreateEntry (index, created).setFlag (flag); + findCreateEntry (index, created).setFlags (flag); return created; } -bool HashRouter::setFlag (uint256 const& index, int flag) +bool HashRouter::setFlags (uint256 const& index, int flags) { // VFALCO NOTE Comments like this belong in the HEADER file, // and more importantly in a Javadoc comment so // they appear in the generated documentation. // // return: true = changed, false = unchanged - assert (flag != 0); + assert (flags != 0); ScopedLockType lock (mMutex); bool created; Entry& s = findCreateEntry (index, created); - if ((s.getFlags () & flag) == flag) + if ((s.getFlags () & flags) == flags) return false; - s.setFlag (flag); + s.setFlags (flags); return true; } @@ -266,14 +163,9 @@ bool HashRouter::swapSet (uint256 const& index, std::set& peers, in return false; s.swapSet (peers); - s.setFlag (flag); + s.setFlags (flag); return true; } -IHashRouter* IHashRouter::New (int holdTime) -{ - return new HashRouter (holdTime); -} - } // ripple diff --git a/src/ripple/app/misc/IHashRouter.h b/src/ripple/app/misc/HashRouter.h similarity index 52% rename from src/ripple/app/misc/IHashRouter.h rename to src/ripple/app/misc/HashRouter.h index 7f056e69c9..21ca16a005 100644 --- a/src/ripple/app/misc/IHashRouter.h +++ b/src/ripple/app/misc/HashRouter.h @@ -17,10 +17,12 @@ */ //============================================================================== -#ifndef RIPPLE_APP_MISC_IHASHROUTER_H_INCLUDED -#define RIPPLE_APP_MISC_IHASHROUTER_H_INCLUDED +#ifndef RIPPLE_APP_MISC_HASHROUTER_H_INCLUDED +#define RIPPLE_APP_MISC_HASHROUTER_H_INCLUDED #include +#include +#include #include #include #include @@ -45,12 +47,72 @@ class STTx; It is used to manage the routing and broadcasting of messages in the peer to peer overlay. */ -class IHashRouter +class HashRouter { public: // The type here *MUST* match the type of Peer::id_t using PeerShortID = std::uint32_t; +private: + /** An entry in the routing table. + */ + class Entry : public CountedObject + { + public: + static char const* getCountedObjectName () { return "HashRouterEntry"; } + + Entry () + : mFlags (0) + { + } + + std::set const& peekPeers () const + { + return mPeers; + } + + void addPeer (PeerShortID peer) + { + if (peer != 0) + mPeers.insert (peer); + } + + bool hasPeer (PeerShortID peer) const + { + return mPeers.count (peer) > 0; + } + + int getFlags (void) const + { + return mFlags; + } + + bool hasFlag (int mask) const + { + return (mFlags & mask) != 0; + } + + void setFlags (int flagsToSet) + { + mFlags |= flagsToSet; + } + + void clearFlag (int flagsToClear) + { + mFlags &= ~flagsToClear; + } + + void swapSet (std::set & other) + { + mPeers.swap (other); + } + + private: + int mFlags; + std::set mPeers; + }; + +public: // VFALCO NOTE this preferred alternative to default parameters makes // behavior clear. // @@ -59,39 +121,56 @@ public: return 300; } - // VFALCO TODO rename the parameter to entryHoldTimeInSeconds - static IHashRouter* New (int holdTime); + explicit HashRouter (int entryHoldTimeInSeconds) + : mHoldTime (entryHoldTimeInSeconds) + { + } - virtual ~IHashRouter () { } + virtual ~HashRouter() = default; // VFALCO TODO Replace "Supression" terminology with something more semantically meaningful. - virtual bool addSuppression (uint256 const& index) = 0; + bool addSuppression (uint256 const& index); - virtual bool addSuppressionPeer (uint256 const& index, PeerShortID peer) = 0; + bool addSuppressionPeer (uint256 const& index, PeerShortID peer); - virtual bool addSuppressionPeer (uint256 const& index, PeerShortID peer, int& flags) = 0; + bool addSuppressionPeer (uint256 const& index, PeerShortID peer, int& flags); - virtual bool addSuppressionFlags (uint256 const& index, int flag) = 0; + bool addSuppressionFlags (uint256 const& index, int flag); /** Set the flags on a hash. @return `true` if the flags were changed. */ - // VFALCO TODO Rename to setFlags since it works with multiple flags. - virtual bool setFlag (uint256 const& index, int mask) = 0; + bool setFlags (uint256 const& index, int flags); - virtual int getFlags (uint256 const& index) = 0; + int getFlags (uint256 const& index); - virtual bool swapSet (uint256 const& index, std::set& peers, int flag) = 0; + bool swapSet (uint256 const& index, std::set& peers, int flag); /** Function wrapper that will check the signature status of a STTx before calling an expensive signature checking function. */ - virtual std::function)> - sigVerify() = 0; + sigVerify(); + +private: + Entry getEntry (uint256 const& ); + + Entry& findCreateEntry (uint256 const& , bool& created); + + using MutexType = std::mutex; + using ScopedLockType = std::lock_guard ; + MutexType mMutex; + + // Stores all suppressed hashes and their expiration time + hash_map mSuppressionMap; + + // Stores all expiration times and the hashes indexed for them + std::map< int, std::list > mSuppressionTimes; + + int mHoldTime; }; } // ripple diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 58183f1923..1647aec64b 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -33,7 +33,7 @@ #include #include #include -#include +#include #include #include #include @@ -679,11 +679,11 @@ void NetworkOPsImp::submitTransaction (Job&, STTx::pointer iTrans) { m_journal.warning << "Submitted transaction " << (reason.empty () ? "has bad signature" : "error: " + reason); - getApp().getHashRouter ().setFlag (suppress, SF_BAD); + getApp().getHashRouter ().setFlags (suppress, SF_BAD); return; } - getApp().getHashRouter ().setFlag (suppress, SF_SIGGOOD); + getApp().getHashRouter ().setFlags (suppress, SF_SIGGOOD); } catch (...) { @@ -728,12 +728,12 @@ void NetworkOPsImp::processTransaction (Transaction::pointer& transaction, m_journal.info << "Transaction has bad signature: " << reason; transaction->setStatus (INVALID); transaction->setResult (temBAD_SIGNATURE); - getApp().getHashRouter ().setFlag (transaction->getID (), SF_BAD); + getApp().getHashRouter ().setFlags (transaction->getID (), SF_BAD); return; } } - getApp().getHashRouter ().setFlag (transaction->getID (), SF_SIGGOOD); + getApp().getHashRouter ().setFlags (transaction->getID (), SF_SIGGOOD); // canonicalize can change our pointer getApp().getMasterTransaction ().canonicalize (&transaction); @@ -916,7 +916,7 @@ void NetworkOPsImp::apply (std::unique_lock& batchLock) e.transaction->setResult (e.result); if (isTemMalformed (e.result)) - getApp().getHashRouter().setFlag (e.transaction->getID(), SF_BAD); + getApp().getHashRouter().setFlags (e.transaction->getID(), SF_BAD); #ifdef BEAST_DEBUG if (e.result != tesSUCCESS) diff --git a/src/ripple/app/tx/impl/Transaction.cpp b/src/ripple/app/tx/impl/Transaction.cpp index 9e6a7d7db2..c3937a36f8 100644 --- a/src/ripple/app/tx/impl/Transaction.cpp +++ b/src/ripple/app/tx/impl/Transaction.cpp @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 9b00fe2e03..028efc75e4 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -18,7 +18,7 @@ //============================================================================== #include -#include +#include #include #include #include diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 01c274aad4..92620f1fd4 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include #include @@ -1702,7 +1702,7 @@ PeerImp::checkTransaction (Job&, int flags, (stx->getFieldU32 (sfLastLedgerSequence) < getApp().getLedgerMaster().getValidLedgerIndex())) { - getApp().getHashRouter().setFlag(stx->getTransactionID(), SF_BAD); + getApp().getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); charge (Resource::feeUnwantedData); return; } @@ -1718,13 +1718,13 @@ PeerImp::checkTransaction (Job&, int flags, if (! reason.empty ()) p_journal_.trace << "Exception checking transaction: " << reason; - getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_BAD); + getApp().getHashRouter ().setFlags (stx->getTransactionID (), SF_BAD); charge (Resource::feeInvalidSignature); return; } else { - getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_SIGGOOD); + getApp().getHashRouter ().setFlags (stx->getTransactionID (), SF_SIGGOOD); } bool const trusted (flags & SF_TRUSTED); @@ -1733,7 +1733,7 @@ PeerImp::checkTransaction (Job&, int flags, } catch (...) { - getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_BAD); + getApp().getHashRouter ().setFlags (stx->getTransactionID (), SF_BAD); charge (Resource::feeBadData); } } diff --git a/src/ripple/rpc/handlers/Submit.cpp b/src/ripple/rpc/handlers/Submit.cpp index 26f79ab8f5..a4b9e0a696 100644 --- a/src/ripple/rpc/handlers/Submit.cpp +++ b/src/ripple/rpc/handlers/Submit.cpp @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/ripple/test/jtx/impl/Env.cpp b/src/ripple/test/jtx/impl/Env.cpp index 18ccb3031d..ea6053ba11 100644 --- a/src/ripple/test/jtx/impl/Env.cpp +++ b/src/ripple/test/jtx/impl/Env.cpp @@ -100,8 +100,7 @@ Env::close(NetClock::time_point const& closeTime) for (auto iter = cur->txs.begin(); iter != cur->txs.end(); ++iter) txs.push_back(iter->first); - std::unique_ptr router( - IHashRouter::New(60)); + auto router = std::make_unique(60); OrderedTxs retries(uint256{}); { OpenView accum(&*next);