refactor: Use const function arguments where possible

include/xrpl/basics/TaggedCache.h

@@ -17,23 +17,6 @@
 
 namespace xrpl {
 
-namespace detail {
-
-// Replace-policy tags used by TaggedCache::canonicalizeReplaceCache /
-// canonicalizeReplaceClient when calling canonicalize. With ReplaceCached the
-// cached value is always replaced with `data`, so `data` is never written back
-// and may be const. With ReplaceClient the cached value is kept and written
-// back into `data` (the client's pointer), which must therefore be writable.
-struct ReplaceCached
-{
-};
-
-struct ReplaceClient
-{
-};
-
-}  // namespace detail
-
 /** Map/cache combination.
     This class implements a cache and a map. The cache keeps objects alive
     in the map. The map allows multiple code paths that reference objects
@@ -113,16 +96,6 @@ public:
     bool
     del(key_type const& key, bool valid);
 
-private:
-    // Selects the `data` parameter type of canonicalize from the replace
-    // policy: const for detail::ReplaceCached (never written back), otherwise
-    // writable.
-    template <typename R>
-    using CanonicalizeClientPointerType = std::conditional_t<
-        std::is_same_v<detail::ReplaceCached, R>,
-        SharedPointerType const&,
-        SharedPointerType&>;
-
 public:
     /** Replace aliased objects with originals.
 
@@ -131,23 +104,15 @@ public:
         This routine eliminates the duplicate and performs a replacement
         on the callers shared pointer if needed.
 
-        `replacePolicy` is a callable taking the existing strong pointer and
-        returning whether to replace the cached value with `data` (true) or to
-        keep the cached value and write it back into `data` (false). Because the
-        write-back case mutates `data`, `data` must be writable.
-
-        (canonicalizeReplaceCache / canonicalizeReplaceClient call this with
-        internal replace-policy tags in place of a callable.)
-
         @param key The key corresponding to the object
         @param data A shared pointer to the data corresponding to the object.
-        @param replacePolicy A callable (existing strong pointer -> bool).
+        @param replace Function that decides if cache should be replaced
 
         @return `true` If the key already existed.
     */
     template <class R>
     bool
-    canonicalize(key_type const& key, CanonicalizeClientPointerType<R> data, R&& replacePolicy);
+    canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback);
 
     bool
     canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data);

include/xrpl/basics/TaggedCache.ipp

@@ -303,26 +303,10 @@ template <
 template <class R>
 inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
-    canonicalize(
-        key_type const& key,
-        CanonicalizeClientPointerType<R> data,
-        [[maybe_unused]] R&& replacePolicy)
+    canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback)
 {
     // Return canonical value, store if needed, refresh in cache
     // Return values: true=we had the data already
-
-    // `replacePolicy` is one of:
-    //   - detail::ReplaceCached: always replace the cached value with `data`;
-    //     `data` is never written back and may be const.
-    //   - detail::ReplaceClient: keep the cached value and write it back into
-    //     `data` (the client's pointer), which must therefore be writable.
-    //   - a callable(strong pointer) -> bool: decide at run time; `data` must
-    //     be writable.
-    // For the latter two the write-back below requires a mutable `data`, so
-    // passing a const argument is a compile error.
-    using C = std::remove_cvref_t<R>;
-    constexpr bool replaceCached = std::is_same_v<C, detail::ReplaceCached>;
-
     std::scoped_lock const lock(mutex_);
 
     auto cit = cache_.find(key);
@@ -340,28 +324,27 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     Entry& entry = cit->second;
     entry.touch(clock_.now());
 
-    auto shouldReplaceCached = [&] {
-        if constexpr (replaceCached)
+    auto shouldReplace = [&] {
+        if constexpr (std::is_invocable_r_v<bool, R>)
         {
-            return true;
-        }
-        else if constexpr (std::is_same_v<C, detail::ReplaceClient>)
-        {
-            return false;
+            // The reason for this extra complexity is for intrusive
+            // strong/weak combo getting a strong is relatively expensive
+            // and not needed for many cases.
+            return replaceCallback();
         }
         else
         {
-            return replacePolicy(entry.ptr.getStrong());
+            return replaceCallback(entry.ptr.getStrong());
         }
     };
 
     if (entry.isCached())
     {
-        if (shouldReplaceCached())
+        if (shouldReplace())
         {
             entry.ptr = data;
         }
-        else if constexpr (!replaceCached)
+        else
         {
             data = entry.ptr.getStrong();
         }
@@ -373,11 +356,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
 
     if (cachedData)
     {
-        if (shouldReplaceCached())
+        if (shouldReplace())
         {
             entry.ptr = data;
         }
-        else if constexpr (!replaceCached)
+        else
         {
             entry.ptr.convertToStrong();
             data = cachedData;
@@ -406,7 +389,7 @@ inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
     canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data)
 {
-    return canonicalize(key, data, detail::ReplaceCached{});
+    return canonicalize(key, const_cast<SharedPointerType&>(data), []() { return true; });
 }
 
 template <
@@ -422,7 +405,7 @@ inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
     canonicalizeReplaceClient(key_type const& key, SharedPointerType& data)
 {
-    return canonicalize(key, data, detail::ReplaceClient{});
+    return canonicalize(key, data, []() { return false; });
 }
 
 template <

include/xrpl/net/HTTPClient.h

@@ -53,7 +53,7 @@ public:
             boost::system::error_code const& ecResult,
             int iStatus,
             std::string const& strData)> complete,
-        beast::Journal& j);
+        beast::Journal const& j);
 
     static void
     get(bool bSSL,
@@ -67,7 +67,7 @@ public:
             boost::system::error_code const& ecResult,
             int iStatus,
             std::string const& strData)> complete,
-        beast::Journal& j);
+        beast::Journal const& j);
 
     static void
     request(
@@ -82,7 +82,7 @@ public:
             boost::system::error_code const& ecResult,
             int iStatus,
             std::string const& strData)> complete,
-        beast::Journal& j);
+        beast::Journal const& j);
 };
 
 }  // namespace xrpl

include/xrpl/shamap/SHAMap.h

@@ -161,7 +161,7 @@ public:
     setLedgerSeq(std::uint32_t lseq);
 
     bool
-    fetchRoot(SHAMapHash const& hash, SHAMapSyncFilter* filter);
+    fetchRoot(SHAMapHash const& hash, SHAMapSyncFilter const* filter);
 
     // normal hash access functions
 
@@ -248,7 +248,7 @@ public:
         @param return The nodes known to be missing
     */
     std::vector<std::pair<SHAMapNodeID, uint256>>
-    getMissingNodes(int maxNodes, SHAMapSyncFilter* filter);
+    getMissingNodes(int maxNodes, SHAMapSyncFilter const* filter);
 
     bool
     getNodeFat(
@@ -281,9 +281,9 @@ public:
     serializeRoot(Serializer& s) const;
 
     SHAMapAddNode
-    addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter* filter);
+    addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter const* filter);
     SHAMapAddNode
-    addKnownNode(SHAMapNodeID const& nodeID, Slice const& rawNode, SHAMapSyncFilter* filter);
+    addKnownNode(SHAMapNodeID const& nodeID, Slice const& rawNode, SHAMapSyncFilter const* filter);
 
     // status functions
     void
@@ -343,11 +343,11 @@ private:
     SHAMapTreeNodePtr
     fetchNodeNT(SHAMapHash const& hash) const;
     SHAMapTreeNodePtr
-    fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter* filter) const;
+    fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const;
     SHAMapTreeNodePtr
     fetchNode(SHAMapHash const& hash) const;
     SHAMapTreeNodePtr
-    checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const;
+    checkFilter(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const;
 
     /** Update hashes up to the root */
     void
@@ -411,7 +411,7 @@ private:
     descendAsync(
         SHAMapInnerNode* parent,
         int branch,
-        SHAMapSyncFilter* filter,
+        SHAMapSyncFilter const* filter,
         bool& pending,
         descendCallback&&) const;
 
@@ -420,7 +420,7 @@ private:
         SHAMapInnerNode* parent,
         SHAMapNodeID const& parentID,
         int branch,
-        SHAMapSyncFilter* filter) const;
+        SHAMapSyncFilter const* filter) const;
 
     // Non-storing
     // Does not hook the returned node to its parent
@@ -461,7 +461,7 @@ private:
 
         // basic parameters
         int max;
-        SHAMapSyncFilter* filter;
+        SHAMapSyncFilter const* filter;
         int const maxDefer;
         std::uint32_t generation;
 
@@ -500,7 +500,11 @@ private:
         // reads
         std::map<SHAMapInnerNode*, SHAMapNodeID> resumes;
 
-        MissingNodes(int max, SHAMapSyncFilter* filter, int maxDefer, std::uint32_t generation)
+        MissingNodes(
+            int max,
+            SHAMapSyncFilter const* filter,
+            int maxDefer,
+            std::uint32_t generation)
             : max(max), filter(filter), maxDefer(maxDefer), generation(generation), deferred(0)
         {
             missingNodes.reserve(max);

src/libxrpl/net/HTTPClient.cpp

@@ -64,7 +64,7 @@ public:
         boost::asio::io_context& ioContext,
         unsigned short const port,
         std::size_t maxResponseSize,
-        beast::Journal& j)
+        beast::Journal const& j)
         : socket_(
               ioContext,
               gHttpClientSslContext->context())  // NOLINT(bugprone-unchecked-optional-access)
@@ -552,7 +552,7 @@ HTTPClient::get(
     std::function<
         bool(boost::system::error_code const& ecResult, int iStatus, std::string const& strData)>
         complete,
-    beast::Journal& j)
+    beast::Journal const& j)
 {
     auto client = std::make_shared<HTTPClientImp>(ioContext, port, responseMax, j);
     client->get(bSSL, deqSites, strPath, timeout, complete);
@@ -570,7 +570,7 @@ HTTPClient::get(
     std::function<
         bool(boost::system::error_code const& ecResult, int iStatus, std::string const& strData)>
         complete,
-    beast::Journal& j)
+    beast::Journal const& j)
 {
     std::deque<std::string> const deqSites(1, strSite);
 
@@ -590,7 +590,7 @@ HTTPClient::request(
     std::function<
         bool(boost::system::error_code const& ecResult, int iStatus, std::string const& strData)>
         complete,
-    beast::Journal& j)
+    beast::Journal const& j)
 {
     std::deque<std::string> const deqSites(1, strSite);
 

src/libxrpl/shamap/SHAMap.cpp

@@ -206,7 +206,7 @@ SHAMap::finishFetch(SHAMapHash const& hash, std::shared_ptr<NodeObject> const& o
 
 // See if a sync filter has a node
 SHAMapTreeNodePtr
-SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
+SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const
 {
     if (auto nodeData = filter->getNode(hash))
     {
@@ -232,7 +232,7 @@ SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
 // Get a node without throwing
 // Used on maps where missing nodes are expected
 SHAMapTreeNodePtr
-SHAMap::fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
+SHAMap::fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const
 {
     auto node = cacheLookup(hash);
     if (node)
@@ -345,7 +345,7 @@ SHAMap::descend(
     SHAMapInnerNode* parent,
     SHAMapNodeID const& parentID,
     int branch,
-    SHAMapSyncFilter* filter) const
+    SHAMapSyncFilter const* filter) const
 {
     XRPL_ASSERT(parent->isInner(), "xrpl::SHAMap::descend : valid parent input");
     XRPL_ASSERT(
@@ -374,7 +374,7 @@ SHAMapTreeNode*
 SHAMap::descendAsync(
     SHAMapInnerNode* parent,
     int branch,
-    SHAMapSyncFilter* filter,
+    SHAMapSyncFilter const* filter,
     bool& pending,
     descendCallback&& callback) const
 {
@@ -885,7 +885,7 @@ SHAMap::updateGiveItem(SHAMapNodeType type, boost::intrusive_ptr<SHAMapItem cons
 }
 
 bool
-SHAMap::fetchRoot(SHAMapHash const& hash, SHAMapSyncFilter* filter)
+SHAMap::fetchRoot(SHAMapHash const& hash, SHAMapSyncFilter const* filter)
 {
     if (hash == root_->getHash())
         return true;

src/libxrpl/shamap/SHAMapSync.cpp

@@ -305,7 +305,7 @@ SHAMap::gmnProcessDeferredReads(MissingNodes& mn)
     nodes that are not permanently stored locally
 */
 std::vector<std::pair<SHAMapNodeID, uint256>>
-SHAMap::getMissingNodes(int max, SHAMapSyncFilter* filter)
+SHAMap::getMissingNodes(int max, SHAMapSyncFilter const* filter)
 {
     XRPL_ASSERT(root_->getHash().isNonZero(), "xrpl::SHAMap::getMissingNodes : nonzero root hash");
     XRPL_ASSERT(max > 0, "xrpl::SHAMap::getMissingNodes : valid max input");
@@ -507,7 +507,7 @@ SHAMap::serializeRoot(Serializer& s) const
 }
 
 SHAMapAddNode
-SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter* filter)
+SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter const* filter)
 {
     // we already have a root_ node
     if (root_->getHash().isNonZero())
@@ -542,7 +542,7 @@ SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFil
 }
 
 SHAMapAddNode
-SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncFilter* filter)
+SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncFilter const* filter)
 {
     XRPL_ASSERT(!node.isRoot(), "xrpl::SHAMap::addKnownNode : valid node input");
 

src/test/basics/TaggedCache_test.cpp

@@ -8,7 +8,6 @@
 #include <xrpl/protocol/Protocol.h>
 
 #include <memory>
-#include <utility>
 
 namespace xrpl {
 
@@ -134,129 +133,6 @@ public:
             BEAST_EXPECT(c.getCacheSize() == 0);
             BEAST_EXPECT(c.getTrackSize() == 0);
         }
-        {
-            BEAST_EXPECT(!c.insert(5, "five"));
-            BEAST_EXPECT(c.getCacheSize() == 1);
-            BEAST_EXPECT(c.size() == 1);
-
-            {
-                auto const p1 = c.fetch(5);
-                BEAST_EXPECT(p1 != nullptr);
-                BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
-
-                // Advance the clock a lot
-                ++clock;
-                c.sweep();
-                BEAST_EXPECT(c.getCacheSize() == 0);
-                BEAST_EXPECT(c.size() == 1);
-
-                auto p2 = std::make_shared<std::string>("five_2");
-                BEAST_EXPECT(c.canonicalizeReplaceCache(5, p2));
-                BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
-                // Make sure the caller's original pointer is unchanged
-                BEAST_EXPECT(p1.get() != p2.get());
-                BEAST_EXPECT(*p2 == "five_2");
-
-                auto const p3 = c.fetch(5);
-                BEAST_EXPECT(p3 != nullptr);
-                BEAST_EXPECT(p3.get() == p2.get());
-                BEAST_EXPECT(p3.get() != p1.get());
-            }
-
-            ++clock;
-            c.sweep();
-            BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
-        }
-
-        {
-            testcase("intrptr");
-
-            struct MyRefCountObject : IntrusiveRefCounts
-            {
-                std::string data;
-
-                // Needed to support weak intrusive pointers
-                virtual void
-                partialDestructor() {};
-
-                MyRefCountObject() = default;
-                explicit MyRefCountObject(std::string data) : data(std::move(data))
-                {
-                }
-                explicit MyRefCountObject(char const* data) : data(data)
-                {
-                }
-
-                MyRefCountObject&
-                operator=(MyRefCountObject const& other)
-                {
-                    data = other.data;
-                    return *this;
-                }
-
-                bool
-                operator==(MyRefCountObject const& other) const
-                {
-                    return data == other.data;
-                }
-
-                bool
-                operator==(std::string const& other) const
-                {
-                    return data == data;
-                }
-            };
-
-            using IntrPtrCache = TaggedCache<
-                Key,
-                MyRefCountObject,
-                /*IsKeyCache*/ false,
-                intr_ptr::SharedWeakUnionPtr<MyRefCountObject>,
-                intr_ptr::SharedPtr<MyRefCountObject>>;
-
-            IntrPtrCache intrPtrCache("IntrPtrTest", 1, 1s, clock, journal);
-
-            intrPtrCache.canonicalizeReplaceCache(1, intr_ptr::makeShared<MyRefCountObject>("one"));
-            BEAST_EXPECT(intrPtrCache.getCacheSize() == 1);
-            BEAST_EXPECT(intrPtrCache.size() == 1);
-
-            {
-                {
-                    intrPtrCache.canonicalizeReplaceCache(
-                        1, intr_ptr::makeShared<MyRefCountObject>("one_replaced"));
-
-                    auto p = intrPtrCache.fetch(1);
-                    BEAST_EXPECT(*p == "one_replaced");
-
-                    // Advance the clock a lot
-                    ++clock;
-                    intrPtrCache.sweep();
-                    BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
-                    BEAST_EXPECT(intrPtrCache.size() == 1);
-
-                    intrPtrCache.canonicalizeReplaceCache(
-                        1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_2"));
-
-                    auto p2 = intrPtrCache.fetch(1);
-                    BEAST_EXPECT(*p2 == "one_replaced_2");
-
-                    intrPtrCache.del(1, true);
-                }
-
-                intrPtrCache.canonicalizeReplaceCache(
-                    1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_3"));
-                auto p3 = intrPtrCache.fetch(1);
-                BEAST_EXPECT(*p3 == "one_replaced_3");
-            }
-
-            ++clock;
-            intrPtrCache.sweep();
-            BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
-            BEAST_EXPECT(intrPtrCache.size() == 0);
-        }
     }
 };
 

src/xrpld/app/ledger/InboundLedger.h

@@ -128,13 +128,13 @@ private:
     pmDowncast() override;
 
     int
-    processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& data);
+    processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData const& data);
 
     bool
     takeHeader(std::string const& data);
 
     void
-    receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode&);
+    receiveNode(protocol::TMLedgerData const& packet, SHAMapAddNode&);
 
     bool
     takeTxRootNode(Slice const& data, SHAMapAddNode&);
@@ -143,10 +143,10 @@ private:
     takeAsRootNode(Slice const& data, SHAMapAddNode&);
 
     std::vector<uint256>
-    neededTxHashes(int max, SHAMapSyncFilter* filter) const;
+    neededTxHashes(int max, SHAMapSyncFilter const* filter) const;
 
     std::vector<uint256>
-    neededStateHashes(int max, SHAMapSyncFilter* filter) const;
+    neededStateHashes(int max, SHAMapSyncFilter const* filter) const;
 
     clock_type& clock_;
     clock_type::time_point lastAction_;

src/xrpld/app/ledger/detail/InboundLedger.cpp

@@ -188,7 +188,7 @@ InboundLedger::~InboundLedger()
 }
 
 static std::vector<uint256>
-neededHashes(uint256 const& root, SHAMap& map, int max, SHAMapSyncFilter* filter)
+neededHashes(uint256 const& root, SHAMap& map, int max, SHAMapSyncFilter const* filter)
 {
     std::vector<uint256> ret;
 
@@ -211,13 +211,13 @@ neededHashes(uint256 const& root, SHAMap& map, int max, SHAMapSyncFilter* filter
 }
 
 std::vector<uint256>
-InboundLedger::neededTxHashes(int max, SHAMapSyncFilter* filter) const
+InboundLedger::neededTxHashes(int max, SHAMapSyncFilter const* filter) const
 {
     return neededHashes(ledger_->header().txHash, ledger_->txMap(), max, filter);
 }
 
 std::vector<uint256>
-InboundLedger::neededStateHashes(int max, SHAMapSyncFilter* filter) const
+InboundLedger::neededStateHashes(int max, SHAMapSyncFilter const* filter) const
 {
     return neededHashes(ledger_->header().accountHash, ledger_->stateMap(), max, filter);
 }
@@ -820,7 +820,7 @@ InboundLedger::takeHeader(std::string const& data)
     Call with a lock
 */
 void
-InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
+InboundLedger::receiveNode(protocol::TMLedgerData const& packet, SHAMapAddNode& san)
 {
     if (!haveHeader_)
     {
@@ -1026,7 +1026,7 @@ InboundLedger::gotData(
 //        TODO Change peer to Consumer
 //
 int
-InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& packet)
+InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData const& packet)
 {
     if (packet.type() == protocol::liBASE)
     {

src/xrpld/app/main/Application.cpp

@@ -492,7 +492,7 @@ public:
     void
     run() override;
     void
-    signalStop(std::string msg) override;
+    signalStop(std::string const& msg) override;
     bool
     checkSigs() const override;
     void
@@ -1602,7 +1602,7 @@ ApplicationImp::run()
 }
 
 void
-ApplicationImp::signalStop(std::string msg)
+ApplicationImp::signalStop(std::string const& msg)
 {
     if (!isTimeToStop.test_and_set(std::memory_order_acquire))
     {

src/xrpld/app/main/Application.h

@@ -111,7 +111,7 @@ public:
     virtual void
     run() = 0;
     virtual void
-    signalStop(std::string msg) = 0;
+    signalStop(std::string const& msg) = 0;
     [[nodiscard]] virtual bool
     checkSigs() const = 0;
     virtual void

src/xrpld/consensus/Validations.h

@@ -693,7 +693,7 @@ public:
         validationSET_EXPIRES ago and were not asked to keep.
     */
     void
-    expire(beast::Journal& j)
+    expire(beast::Journal const& j)
     {
         auto const start = std::chrono::steady_clock::now();
         {

src/xrpld/overlay/Overlay.h

@@ -117,11 +117,11 @@ public:
 
     /** Broadcast a proposal. */
     virtual void
-    broadcast(protocol::TMProposeSet& m) = 0;
+    broadcast(protocol::TMProposeSet const& m) = 0;
 
     /** Broadcast a validation. */
     virtual void
-    broadcast(protocol::TMValidation& m) = 0;
+    broadcast(protocol::TMValidation const& m) = 0;
 
     /** Relay a proposal.
      * @param m the serialized proposal
@@ -130,7 +130,7 @@ public:
      * @return the set of peers which have already sent us this proposal
      */
     virtual std::set<Peer::id_t>
-    relay(protocol::TMProposeSet& m, uint256 const& uid, PublicKey const& validator) = 0;
+    relay(protocol::TMProposeSet const& m, uint256 const& uid, PublicKey const& validator) = 0;
 
     /** Relay a validation.
      * @param m the serialized validation
@@ -139,7 +139,7 @@ public:
      * @return the set of peers which have already sent us this validation
      */
     virtual std::set<Peer::id_t>
-    relay(protocol::TMValidation& m, uint256 const& uid, PublicKey const& validator) = 0;
+    relay(protocol::TMValidation const& m, uint256 const& uid, PublicKey const& validator) = 0;
 
     /** Relay a transaction. If the tx reduce-relay feature is enabled then
      * randomly select peers to relay to and queue transaction's hash

src/xrpld/overlay/detail/OverlayImpl.cpp

@@ -407,7 +407,7 @@ OverlayImpl::makeErrorResponse(
     std::shared_ptr<PeerFinder::Slot> const& slot,
     http_request_type const& request,
     address_type remoteAddress,
-    std::string text)
+    std::string const& text)
 {
     boost::beast::http::response<boost::beast::http::empty_body> msg;
     msg.version(request.version());
@@ -1157,14 +1157,14 @@ OverlayImpl::findPeerByPublicKey(PublicKey const& pubKey)
 }
 
 void
-OverlayImpl::broadcast(protocol::TMProposeSet& m)
+OverlayImpl::broadcast(protocol::TMProposeSet const& m)
 {
     auto const sm = std::make_shared<Message>(m, protocol::mtPROPOSE_LEDGER);
     forEach([&](std::shared_ptr<PeerImp> const& p) { p->send(sm); });
 }
 
 std::set<Peer::id_t>
-OverlayImpl::relay(protocol::TMProposeSet& m, uint256 const& uid, PublicKey const& validator)
+OverlayImpl::relay(protocol::TMProposeSet const& m, uint256 const& uid, PublicKey const& validator)
 {
     if (auto const toSkip = app_.getHashRouter().shouldRelay(uid))
     {
@@ -1179,14 +1179,14 @@ OverlayImpl::relay(protocol::TMProposeSet& m, uint256 const& uid, PublicKey cons
 }
 
 void
-OverlayImpl::broadcast(protocol::TMValidation& m)
+OverlayImpl::broadcast(protocol::TMValidation const& m)
 {
     auto const sm = std::make_shared<Message>(m, protocol::mtVALIDATION);
     forEach([sm](std::shared_ptr<PeerImp> const& p) { p->send(sm); });
 }
 
 std::set<Peer::id_t>
-OverlayImpl::relay(protocol::TMValidation& m, uint256 const& uid, PublicKey const& validator)
+OverlayImpl::relay(protocol::TMValidation const& m, uint256 const& uid, PublicKey const& validator)
 {
     if (auto const toSkip = app_.getHashRouter().shouldRelay(uid))
     {

src/xrpld/overlay/detail/OverlayImpl.h

@@ -202,16 +202,16 @@ public:
     findPeerByPublicKey(PublicKey const& pubKey) override;
 
     void
-    broadcast(protocol::TMProposeSet& m) override;
+    broadcast(protocol::TMProposeSet const& m) override;
 
     void
-    broadcast(protocol::TMValidation& m) override;
+    broadcast(protocol::TMValidation const& m) override;
 
     std::set<Peer::id_t>
-    relay(protocol::TMProposeSet& m, uint256 const& uid, PublicKey const& validator) override;
+    relay(protocol::TMProposeSet const& m, uint256 const& uid, PublicKey const& validator) override;
 
     std::set<Peer::id_t>
-    relay(protocol::TMValidation& m, uint256 const& uid, PublicKey const& validator) override;
+    relay(protocol::TMValidation const& m, uint256 const& uid, PublicKey const& validator) override;
 
     void
     relay(
@@ -433,7 +433,7 @@ private:
         std::shared_ptr<PeerFinder::Slot> const& slot,
         http_request_type const& request,
         address_type remoteAddress,
-        std::string msg);
+        std::string const& msg);
 
     /** Handles crawl requests. Crawl returns information about the
         node and its peers so crawlers can map the network.