Make the data of a SHAMap const:

* Hold a shared_ptr<SHAMapItem const> instead of a shared_ptr<SHAMapItem>. * Compiler helps us enforce data immutability. * Make SHAMapAbstractNode::addRaw const Conflicts: src/ripple/app/ledger/Ledger.cpp src/ripple/app/ledger/Ledger.h src/ripple/app/ledger/LedgerHistory.cpp src/ripple/app/ledger/LedgerToJson.h
2025-12-06 17:27:52 +00:00 · 2015-06-16 14:03:52 -04:00
parent d468deee12
commit 1e6111c09c
13 changed files with 121 additions and 99 deletions
--- a/src/ripple/app/ledger/Ledger.cpp
+++ b/src/ripple/app/ledger/Ledger.cpp
@@ -342,9 +342,10 @@ addTransaction (Ledger& ledger,
    uint256 const& txID, const Serializer& txn)
 {
    // low-level - just add to table
-    auto item = std::make_shared<SHAMapItem> (txID, txn.peekData ());
+    auto item = std::make_shared<
+        SHAMapItem const> (txID, txn.peekData ());

-    if (! ledger.txMap().addGiveItem (item, true, false))
+    if (! ledger.txMap().addGiveItem (std::move(item), true, false))
    {
        WriteLog (lsWARNING, Ledger)
                << "Attempt to add transaction to ledger that already had it";
@@ -363,9 +364,10 @@ bool addTransaction (Ledger& ledger,
    Serializer s (txn.getDataLength () + md.getDataLength () + 16);
    s.addVL (txn.peekData ());
    s.addVL (md.peekData ());
-    auto item = std::make_shared<SHAMapItem> (txID, s.peekData ());
+    auto item = std::make_shared<
+        SHAMapItem const> (txID, std::move(s));

-    if (! ledger.txMap().addGiveItem (item, true, true))
+    if (! ledger.txMap().addGiveItem (std::move(item), true, true))
    {
        WriteLog (lsFATAL, Ledger)
                << "Attempt to add transaction+MD to ledger that already had it";
@@ -965,7 +967,7 @@ Ledger::unchecked_insert(
    Serializer ss;
    sle->add(ss);
    auto item = std::make_shared<
-        SHAMapItem>(sle->key(),
+        SHAMapItem const>(sle->key(),
            std::move(ss));
    // VFALCO NOTE addGiveItem should take ownership
    auto const success =
@@ -984,7 +986,7 @@ Ledger::unchecked_replace(
    Serializer ss;
    sle->add(ss);
    auto item = std::make_shared<
-        SHAMapItem>(sle->key(),
+        SHAMapItem const>(sle->key(),
            std::move(ss));
    // VFALCO NOTE updateGiveItem should take ownership
    auto const success =
@@ -1016,7 +1018,8 @@ Ledger::peek (Keylet const& k) const
 //------------------------------------------------------------------------------

 static void visitHelper (
-    std::function<void (SLE::ref)>& function, std::shared_ptr<SHAMapItem> const& item)
+    std::function<void (std::shared_ptr<SLE> const&)>& function,
+        std::shared_ptr<SHAMapItem const> const& item)
 {
    function (std::make_shared<SLE> (item->peekSerializer(), item->key()));
 }
@@ -1339,7 +1342,7 @@ cachedRead (Ledger const& ledger, uint256 const& key,
    SLECache& cache, boost::optional<LedgerEntryType> type)
 {
    uint256 hash;
-    auto const item =
+    auto const& item =
        ledger.stateMap().peekItem(key, hash);
    if (! item)
        return {};
--- a/src/ripple/app/ledger/LedgerHistory.cpp
+++ b/src/ripple/app/ledger/LedgerHistory.cpp
@@ -310,13 +310,13 @@ void LedgerHistory::handleMismatch (LedgerHash const& built, LedgerHash const& v
    std::vector <SHAMapItemInfo> builtTx, validTx;
    // Get built ledger hashes and metadata
    builtLedger->txMap().visitLeaves(
-        [&builtTx](std::shared_ptr<SHAMapItem> const& item)
+        [&builtTx](std::shared_ptr<SHAMapItem const> const& item)
        {
            builtTx.push_back({item->getTag(), item->peekData()});
        });
    // Get valid ledger hashes and metadata
    validLedger->txMap().visitLeaves(
-        [&validTx](std::shared_ptr<SHAMapItem> const& item)
+        [&validTx](std::shared_ptr<SHAMapItem const> const& item)
        {
            validTx.push_back({item->getTag(), item->peekData()});
        });
--- a/src/ripple/app/ledger/LedgerToJson.h
+++ b/src/ripple/app/ledger/LedgerToJson.h
@@ -198,7 +198,7 @@ void fillJson (Object& json, LedgerFill const& fill)
             if (bBinary)
             {
                 stateMap.visitLeaves (
-                     [&array] (std::shared_ptr<SHAMapItem> const& smi)
+                     [&array] (std::shared_ptr<SHAMapItem const> const& smi)
                     {
                         auto&& obj = appendObject (array);
                         obj[jss::hash] = to_string(smi->getTag ());
@@ -218,7 +218,7 @@ void fillJson (Object& json, LedgerFill const& fill)
        else
        {
            stateMap.visitLeaves(
-                [&array, &count] (std::shared_ptr<SHAMapItem> const& smi)
+                [&array, &count] (std::shared_ptr<SHAMapItem const> const& smi)
                {
                    count.yield();
                    array.append (to_string(smi->getTag ()));
--- a/src/ripple/rpc/handlers/LedgerData.cpp
+++ b/src/ripple/rpc/handlers/LedgerData.cpp
@@ -79,7 +79,7 @@ Json::Value doLedgerData (RPC::Context& context)

    for (;;)
    {
-       std::shared_ptr<SHAMapItem> item = map.peekNextItem (resumePoint);
+       auto item = map.peekNextItem (resumePoint);
       if (!item)
           break;
       resumePoint = item->getTag();
--- a/src/ripple/shamap/SHAMap.h
+++ b/src/ripple/shamap/SHAMap.h
@@ -91,8 +91,8 @@ private:
    bool                            backed_ = true; // Map is backed by the database

 public:
-    using DeltaItem = std::pair<std::shared_ptr<SHAMapItem>,
-                                std::shared_ptr<SHAMapItem>>;
+    using DeltaItem = std::pair<std::shared_ptr<SHAMapItem const>,
+                                std::shared_ptr<SHAMapItem const>>;
    using Delta     = std::map<uint256, DeltaItem>;

    ~SHAMap ();
@@ -139,8 +139,10 @@ public:
    uint256 getHash () const;

    // save a copy if you have a temporary anyway
-    bool updateGiveItem (std::shared_ptr<SHAMapItem> const&, bool isTransaction, bool hasMeta);
-    bool addGiveItem (std::shared_ptr<SHAMapItem> const&, bool isTransaction, bool hasMeta);
+    bool updateGiveItem (std::shared_ptr<SHAMapItem const> const&,
+                         bool isTransaction, bool hasMeta);
+    bool addGiveItem (std::shared_ptr<SHAMapItem const> const&,
+                      bool isTransaction, bool hasMeta);

    /** Fetch an item given its key.
        This retrieves the item whose key matches.
@@ -149,27 +151,31 @@ public:
            Can throw SHAMapMissingNode
        @note This can cause NodeStore reads
    */
-    // VFALCO NOTE This should return `const&` when SHAMapTreeNode
-    //             stores std::shared_ptr<SHAMapItem const>
-    std::shared_ptr<SHAMapItem const>
-    fetch (uint256 const& key) const;
+    std::shared_ptr<SHAMapItem const> const&
+        fetch (uint256 const& key) const;

-    // VFALCO NOTE Is "save a copy" the in imperative or indicative mood?
-    // save a copy if you only need a temporary
-    std::shared_ptr<SHAMapItem> peekItem (uint256 const& id) const;
-    std::shared_ptr<SHAMapItem> peekItem (uint256 const& id, uint256 & hash) const;
-    std::shared_ptr<SHAMapItem> peekItem (uint256 const& id, SHAMapTreeNode::TNType & type) const;
+    // Save a copy if you need to extend the life
+    // of the SHAMapItem beyond this SHAMap
+    std::shared_ptr<SHAMapItem const> const& peekItem (uint256 const& id) const;
+    std::shared_ptr<SHAMapItem const> const&
+        peekItem (uint256 const& id, uint256 & hash) const;
+    std::shared_ptr<SHAMapItem const> const&
+        peekItem (uint256 const& id, SHAMapTreeNode::TNType & type) const;

    // traverse functions
-    std::shared_ptr<SHAMapItem> peekFirstItem () const;
-    std::shared_ptr<SHAMapItem> peekFirstItem (SHAMapTreeNode::TNType & type) const;
-    std::shared_ptr<SHAMapItem> peekLastItem () const;
-    std::shared_ptr<SHAMapItem> peekNextItem (uint256 const& ) const;
-    std::shared_ptr<SHAMapItem> peekNextItem (uint256 const& , SHAMapTreeNode::TNType & type) const;
-    std::shared_ptr<SHAMapItem> peekPrevItem (uint256 const& ) const;
+    std::shared_ptr<SHAMapItem const> const& peekFirstItem () const;
+    std::shared_ptr<SHAMapItem const> const&
+        peekFirstItem (SHAMapTreeNode::TNType & type) const;
+    std::shared_ptr<SHAMapItem const> const& peekLastItem () const;
+    std::shared_ptr<SHAMapItem const> const& peekNextItem (uint256 const& ) const;
+    std::shared_ptr<SHAMapItem const> const&
+        peekNextItem (uint256 const& , SHAMapTreeNode::TNType & type) const;
+    std::shared_ptr<SHAMapItem const> const& peekPrevItem (uint256 const& ) const;

    void visitNodes (std::function<bool (SHAMapAbstractNode&)> const&) const;
-    void visitLeaves(std::function<void (std::shared_ptr<SHAMapItem> const&)> const&) const;
+    void
+        visitLeaves(
+            std::function<void(std::shared_ptr<SHAMapItem const> const&)> const&) const;

    // comparison/sync functions
    void getMissingNodes (std::vector<SHAMapNodeID>& nodeIDs, std::vector<uint256>& hashes, int max,
@@ -219,8 +225,8 @@ public:
 private:
    using SharedPtrNodeStack =
        std::stack<std::pair<std::shared_ptr<SHAMapAbstractNode>, SHAMapNodeID>>;
-    using DeltaRef = std::pair<std::shared_ptr<SHAMapItem> const&,
-                               std::shared_ptr<SHAMapItem> const&>;
+    using DeltaRef = std::pair<std::shared_ptr<SHAMapItem const> const&,
+                               std::shared_ptr<SHAMapItem const> const&>;

    int unshare ();

@@ -289,14 +295,14 @@ private:
        descendNoStore (std::shared_ptr<SHAMapInnerNode> const&, int branch) const;

    /** If there is only one leaf below this node, get its contents */
-    std::shared_ptr<SHAMapItem> onlyBelow (SHAMapAbstractNode*) const;
+    std::shared_ptr<SHAMapItem const> const& onlyBelow (SHAMapAbstractNode*) const;

    bool hasInnerNode (SHAMapNodeID const& nodeID, uint256 const& hash) const;
    bool hasLeafNode (uint256 const& tag, uint256 const& hash) const;

    bool walkBranch (SHAMapAbstractNode* node,
-                     std::shared_ptr<SHAMapItem> const& otherMapItem, bool isFirstMap,
-                     Delta & differences, int & maxCount) const;
+                     std::shared_ptr<SHAMapItem const> const& otherMapItem,
+                     bool isFirstMap, Delta & differences, int & maxCount) const;
    int walkSubTree (bool doWrite, NodeObjectType t, std::uint32_t seq);
 };

--- a/src/ripple/shamap/SHAMapItem.h
+++ b/src/ripple/shamap/SHAMapItem.h
@@ -40,6 +40,7 @@ public:
    explicit SHAMapItem (uint256 const& tag);
    SHAMapItem (uint256 const& tag, Blob const & data);
    SHAMapItem (uint256 const& tag, Serializer const& s);
+    SHAMapItem (uint256 const& key, Serializer&& s);

    Slice slice() const;

@@ -53,7 +54,7 @@ public:
    uint256 const& getTag() const;

    Blob const& peekData() const;
-    Serializer& peekSerializer();
+    Serializer const& peekSerializer() const;

 public:  // public only to SHAMapTreeNode
    std::size_t size() const;
@@ -109,8 +110,8 @@ SHAMapItem::peekData() const
 }

 inline
-Serializer& 
-SHAMapItem::peekSerializer()
+Serializer const&
+SHAMapItem::peekSerializer() const
 {
    return mData;
 }
--- a/src/ripple/shamap/SHAMapTreeNode.h
+++ b/src/ripple/shamap/SHAMapTreeNode.h
@@ -75,7 +75,7 @@ public:
    bool isInBounds (SHAMapNodeID const &id) const;

    virtual bool updateHash () = 0;
-    virtual void addRaw (Serializer&, SHANodeFormat format) = 0;
+    virtual void addRaw (Serializer&, SHANodeFormat format) const = 0;
    virtual std::string getString (SHAMapNodeID const&) const;
    virtual std::shared_ptr<SHAMapAbstractNode> clone(std::uint32_t seq) const = 0;

@@ -120,7 +120,7 @@ public:

    bool updateHash () override;
    void updateHashDeep();
-    void addRaw (Serializer&, SHANodeFormat format) override;
+    void addRaw (Serializer&, SHANodeFormat format) const override;
    std::string getString (SHAMapNodeID const&) const override;

    friend std::shared_ptr<SHAMapAbstractNode>
@@ -133,18 +133,19 @@ class SHAMapTreeNode
    : public SHAMapAbstractNode
 {
 private:
-    std::shared_ptr<SHAMapItem>     mItem;
+    std::shared_ptr<SHAMapItem const> mItem;

 public:
    SHAMapTreeNode (const SHAMapTreeNode&) = delete;
    SHAMapTreeNode& operator= (const SHAMapTreeNode&) = delete;

-    SHAMapTreeNode (std::shared_ptr<SHAMapItem> const& item, TNType type, std::uint32_t seq);
-    SHAMapTreeNode(std::shared_ptr<SHAMapItem> const& item, TNType type,
+    SHAMapTreeNode (std::shared_ptr<SHAMapItem const> const& item,
+                    TNType type, std::uint32_t seq);
+    SHAMapTreeNode(std::shared_ptr<SHAMapItem const> const& item, TNType type,
                   std::uint32_t seq, uint256 const& hash);
    std::shared_ptr<SHAMapAbstractNode> clone(std::uint32_t seq) const override;

-    void addRaw (Serializer&, SHANodeFormat format) override;
+    void addRaw (Serializer&, SHANodeFormat format) const override;

 public:  // public only to SHAMap

@@ -153,8 +154,8 @@ public:  // public only to SHAMap

    // item node function
    bool hasItem () const;
-    std::shared_ptr<SHAMapItem> const& peekItem () const;
-    bool setItem (std::shared_ptr<SHAMapItem> const& i, TNType type);
+    std::shared_ptr<SHAMapItem const> const& peekItem () const;
+    bool setItem (std::shared_ptr<SHAMapItem const> const& i, TNType type);

    std::string getString (SHAMapNodeID const&) const override;
    bool updateHash () override;
@@ -290,7 +291,7 @@ SHAMapTreeNode::hasItem () const
 }

 inline
-std::shared_ptr<SHAMapItem> const&
+std::shared_ptr<SHAMapItem const> const&
 SHAMapTreeNode::peekItem () const
 {
    return mItem;
--- a/src/ripple/shamap/impl/SHAMap.cpp
+++ b/src/ripple/shamap/impl/SHAMap.cpp
@@ -468,7 +468,9 @@ SHAMap::lastBelow (SHAMapAbstractNode* node) const
    while (true);
 }

-std::shared_ptr<SHAMapItem>
+static const std::shared_ptr<SHAMapItem const> no_item;
+
+std::shared_ptr<SHAMapItem const> const&
 SHAMap::onlyBelow (SHAMapAbstractNode* node) const
 {
    // If there is only one item below this node, return it
@@ -482,7 +484,7 @@ SHAMap::onlyBelow (SHAMapAbstractNode* node) const
            if (!inner->isEmptyBranch (i))
            {
                if (nextNode)
-                    return std::shared_ptr<SHAMapItem> ();
+                    return no_item;

                nextNode = descendThrow (inner, i);
            }
@@ -491,7 +493,7 @@ SHAMap::onlyBelow (SHAMapAbstractNode* node) const
        if (!nextNode)
        {
            assert (false);
-            return std::shared_ptr<SHAMapItem> ();
+            return no_item;
        }

        node = nextNode;
@@ -508,7 +510,7 @@ SHAMap::onlyBelow (SHAMapAbstractNode* node) const
 static std::shared_ptr<
    SHAMapItem const> const nullConstSHAMapItem;

-std::shared_ptr<SHAMapItem const>
+std::shared_ptr<SHAMapItem const> const&
 SHAMap::fetch (uint256 const& key) const
 {
    SHAMapTreeNode const* const leaf =
@@ -518,9 +520,8 @@ SHAMap::fetch (uint256 const& key) const
    return leaf->peekItem();
 }

-static const std::shared_ptr<SHAMapItem> no_item;
-
-std::shared_ptr<SHAMapItem> SHAMap::peekFirstItem () const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekFirstItem () const
 {
    SHAMapTreeNode* node = firstBelow (root_.get ());

@@ -530,7 +531,8 @@ std::shared_ptr<SHAMapItem> SHAMap::peekFirstItem () const
    return node->peekItem ();
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekFirstItem (SHAMapTreeNode::TNType& type) const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekFirstItem (SHAMapTreeNode::TNType& type) const
 {
    SHAMapTreeNode* node = firstBelow (root_.get ());

@@ -541,7 +543,8 @@ std::shared_ptr<SHAMapItem> SHAMap::peekFirstItem (SHAMapTreeNode::TNType& type)
    return node->peekItem ();
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekLastItem () const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekLastItem () const
 {
    SHAMapTreeNode* node = lastBelow (root_.get ());

@@ -551,13 +554,14 @@ std::shared_ptr<SHAMapItem> SHAMap::peekLastItem () const
    return node->peekItem ();
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekNextItem (uint256 const& id) const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekNextItem (uint256 const& id) const
 {
    SHAMapTreeNode::TNType type;
    return peekNextItem (id, type);
 }

-std::shared_ptr<SHAMapItem>
+std::shared_ptr<SHAMapItem const> const&
 SHAMap::peekNextItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
 {
    // Get a pointer to the next item in the tree after a given item - item need not be in tree
@@ -603,7 +607,7 @@ SHAMap::peekNextItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
 }

 // Get a pointer to the previous item in the tree after a given item - item need not be in tree
-std::shared_ptr<SHAMapItem>
+std::shared_ptr<SHAMapItem const> const&
 SHAMap::peekPrevItem (uint256 const& id) const
 {
    auto stack = getStack (id, true);
@@ -639,7 +643,8 @@ SHAMap::peekPrevItem (uint256 const& id) const
    return no_item;
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekItem (uint256 const& id) const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekItem (uint256 const& id) const
 {
    SHAMapTreeNode* leaf = walkToPointer (id);

@@ -649,7 +654,8 @@ std::shared_ptr<SHAMapItem> SHAMap::peekItem (uint256 const& id) const
    return leaf->peekItem ();
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekItem (uint256 const& id, SHAMapTreeNode::TNType& type) const
 {
    SHAMapTreeNode* leaf = walkToPointer (id);

@@ -660,7 +666,8 @@ std::shared_ptr<SHAMapItem> SHAMap::peekItem (uint256 const& id, SHAMapTreeNode:
    return leaf->peekItem ();
 }

-std::shared_ptr<SHAMapItem> SHAMap::peekItem (uint256 const& id, uint256& hash) const
+std::shared_ptr<SHAMapItem const> const&
+SHAMap::peekItem (uint256 const& id, uint256& hash) const
 {
    SHAMapTreeNode* leaf = walkToPointer (id);

@@ -728,7 +735,7 @@ bool SHAMap::delItem (uint256 const& id)
            else if (bc == 1)
            {
                // If there's only one item, pull up on the thread
-                std::shared_ptr<SHAMapItem> item = onlyBelow (node.get ());
+                std::shared_ptr<SHAMapItem const> item = onlyBelow (node.get ());

                if (item)
                {
@@ -762,7 +769,7 @@ bool SHAMap::delItem (uint256 const& id)
 }

 bool
-SHAMap::addGiveItem (std::shared_ptr<SHAMapItem> const& item,
+SHAMap::addGiveItem (std::shared_ptr<SHAMapItem const> const& item,
                     bool isTransaction, bool hasMeta)
 {
    // add the specified item, does not update
@@ -801,7 +808,7 @@ SHAMap::addGiveItem (std::shared_ptr<SHAMapItem> const& item,
    {
        // this is a leaf node that has to be made an inner node holding two items
        auto leaf = std::static_pointer_cast<SHAMapTreeNode>(node);
-        std::shared_ptr<SHAMapItem> otherItem = leaf->peekItem ();
+        std::shared_ptr<SHAMapItem const> otherItem = leaf->peekItem ();
        assert (otherItem && (tag != otherItem->getTag ()));

        node = std::make_shared<SHAMapInnerNode>(node->getSeq());
@@ -838,7 +845,7 @@ SHAMap::addGiveItem (std::shared_ptr<SHAMapItem> const& item,

 bool SHAMap::addItem (const SHAMapItem& i, bool isTransaction, bool hasMetaData)
 {
-    return addGiveItem (std::make_shared<SHAMapItem> (i), isTransaction, hasMetaData);
+    return addGiveItem(std::make_shared<SHAMapItem const>(i), isTransaction, hasMetaData);
 }

 uint256
@@ -854,7 +861,7 @@ SHAMap::getHash () const
 }

 bool
-SHAMap::updateGiveItem (std::shared_ptr<SHAMapItem> const& item,
+SHAMap::updateGiveItem (std::shared_ptr<SHAMapItem const> const& item,
                        bool isTransaction, bool hasMeta)
 {
    // can't change the tag but can change the hash
--- a/src/ripple/shamap/impl/SHAMapDelta.cpp
+++ b/src/ripple/shamap/impl/SHAMapDelta.cpp
@@ -31,8 +31,8 @@ namespace ripple {
 // synchronizing matching branches too.)

 bool SHAMap::walkBranch (SHAMapAbstractNode* node,
-                         std::shared_ptr<SHAMapItem> const& otherMapItem, bool isFirstMap,
-                         Delta& differences, int& maxCount) const
+                         std::shared_ptr<SHAMapItem const> const& otherMapItem,
+                         bool isFirstMap,Delta& differences, int& maxCount) const
 {
    // Walk a branch of a SHAMap that's matched by an empty branch or single item in the other map
    std::stack <SHAMapAbstractNode*, std::vector<SHAMapAbstractNode*>> nodeStack;
@@ -63,10 +63,10 @@ bool SHAMap::walkBranch (SHAMapAbstractNode* node,
                // unmatched
                if (isFirstMap)
                    differences.insert (std::make_pair (item->getTag (),
-                                      DeltaRef (item, std::shared_ptr<SHAMapItem> ())));
+                        DeltaRef (item, std::shared_ptr<SHAMapItem const> ())));
                else
                    differences.insert (std::make_pair (item->getTag (),
-                                      DeltaRef (std::shared_ptr<SHAMapItem> (), item)));
+                        DeltaRef (std::shared_ptr<SHAMapItem const> (), item)));

                if (--maxCount <= 0)
                    return false;
@@ -98,13 +98,12 @@ bool SHAMap::walkBranch (SHAMapAbstractNode* node,
    {
        // otherMapItem was unmatched, must add
        if (isFirstMap) // this is first map, so other item is from second
-            differences.insert (std::make_pair (otherMapItem->getTag (),
-                                                DeltaRef (std::shared_ptr<SHAMapItem>(),
-                                                          otherMapItem)));
+            differences.insert(std::make_pair(otherMapItem->getTag (),
+                                              DeltaRef(std::shared_ptr<SHAMapItem const>(),
+                                                       otherMapItem)));
        else
-            differences.insert (std::make_pair (otherMapItem->getTag (),
-                                                DeltaRef (otherMapItem,
-                                                      std::shared_ptr<SHAMapItem> ())));
+            differences.insert(std::make_pair(otherMapItem->getTag (),
+                DeltaRef(otherMapItem, std::shared_ptr<SHAMapItem const>())));

        if (--maxCount <= 0)
            return false;
@@ -163,13 +162,12 @@ SHAMap::compare (std::shared_ptr<SHAMap> const& otherMap,
            {
                differences.insert (std::make_pair(ours->peekItem()->getTag (),
                                                   DeltaRef(ours->peekItem(),
-                                                   std::shared_ptr<SHAMapItem> ())));
+                                                   std::shared_ptr<SHAMapItem const>())));
                if (--maxCount <= 0)
                    return false;

                differences.insert(std::make_pair(other->peekItem()->getTag (),
-                                                  DeltaRef(std::shared_ptr<SHAMapItem>(),
-                                                  other->peekItem ())));
+                    DeltaRef(std::shared_ptr<SHAMapItem const>(), other->peekItem ())));
                if (--maxCount <= 0)
                    return false;
            }
@@ -202,7 +200,7 @@ SHAMap::compare (std::shared_ptr<SHAMap> const& otherMap,
                        // We have a branch, the other tree does not
                        SHAMapAbstractNode* iNode = descendThrow (ours, i);
                        if (!walkBranch (iNode,
-                                         std::shared_ptr<SHAMapItem> (), true,
+                                         std::shared_ptr<SHAMapItem const> (), true,
                                         differences, maxCount))
                            return false;
                    }
@@ -212,7 +210,7 @@ SHAMap::compare (std::shared_ptr<SHAMap> const& otherMap,
                        SHAMapAbstractNode* iNode =
                            otherMap->descendThrow(other, i);
                        if (!otherMap->walkBranch (iNode,
-                                                   std::shared_ptr<SHAMapItem>(),
+                                                   std::shared_ptr<SHAMapItem const>(),
                                                   false, differences, maxCount))
                            return false;
                    }
--- a/src/ripple/shamap/impl/SHAMapItem.cpp
+++ b/src/ripple/shamap/impl/SHAMapItem.cpp
@@ -36,6 +36,12 @@ SHAMapItem::SHAMapItem (uint256 const& tag, const Serializer& data)
 {
 }

+SHAMapItem::SHAMapItem (uint256 const& key, Serializer&& s)
+    : mTag(key)
+    , mData(std::move(s))
+{
+}
+
 // VFALCO This function appears not to be called
 void SHAMapItem::dump (beast::Journal journal)
 {
--- a/src/ripple/shamap/impl/SHAMapSync.cpp
+++ b/src/ripple/shamap/impl/SHAMapSync.cpp
@@ -29,7 +29,7 @@ namespace ripple {
 static const uint256 uZero;

 static bool visitLeavesHelper (
-    std::function <void (std::shared_ptr<SHAMapItem> const&)> const& function,
+    std::function <void (std::shared_ptr<SHAMapItem const> const&)> const& function,
    SHAMapAbstractNode& node)
 {
    // Adapt visitNodes to visitLeaves
@@ -41,7 +41,7 @@ static bool visitLeavesHelper (

 void
 SHAMap::visitLeaves(
-    std::function<void(std::shared_ptr<SHAMapItem> const& item)> const& leafFunction) const
+    std::function<void(std::shared_ptr<SHAMapItem const> const& item)> const& leafFunction) const
 {
    visitNodes (std::bind (visitLeavesHelper,
            std::cref (leafFunction), std::placeholders::_1));
--- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp
+++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp
@@ -55,7 +55,7 @@ SHAMapTreeNode::clone(std::uint32_t seq) const
    return std::make_shared<SHAMapTreeNode>(mItem, mType, seq, mHash);
 }

-SHAMapTreeNode::SHAMapTreeNode (std::shared_ptr<SHAMapItem> const& item,
+SHAMapTreeNode::SHAMapTreeNode (std::shared_ptr<SHAMapItem const> const& item,
                                TNType type, std::uint32_t seq)
    : SHAMapAbstractNode(type, seq)
    , mItem (item)
@@ -64,7 +64,7 @@ SHAMapTreeNode::SHAMapTreeNode (std::shared_ptr<SHAMapItem> const& item,
    updateHash();
 }

-SHAMapTreeNode::SHAMapTreeNode (std::shared_ptr<SHAMapItem> const& item,
+SHAMapTreeNode::SHAMapTreeNode (std::shared_ptr<SHAMapItem const> const& item,
                                TNType type, std::uint32_t seq, uint256 const& hash)
    : SHAMapAbstractNode(type, seq, hash)
    , mItem (item)
@@ -105,7 +105,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
        if (type == 0)
        {
            // transaction
-            auto item = std::make_shared<SHAMapItem>(
+            auto item = std::make_shared<SHAMapItem const>(
                sha512Half(HashPrefix::transactionID,
                    Slice(s.data(), s.size())),
                        s.peekData());
@@ -125,7 +125,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f

            if (u.isZero ()) throw std::runtime_error ("invalid AS node");

-            auto item = std::make_shared<SHAMapItem> (u, s.peekData ());
+            auto item = std::make_shared<SHAMapItem const> (u, s.peekData ());
            if (hashValid)
                return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq, hash);
            return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq);
@@ -185,7 +185,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
            if (u.isZero ())
                throw std::runtime_error ("invalid TM node");

-            auto item = std::make_shared<SHAMapItem> (u, s.peekData ());
+            auto item = std::make_shared<SHAMapItem const> (u, s.peekData ());
            if (hashValid)
                return std::make_shared<SHAMapTreeNode>(item, tnTRANSACTION_MD, seq, hash);
            return std::make_shared<SHAMapTreeNode>(item, tnTRANSACTION_MD, seq);
@@ -211,7 +211,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f

        if (prefix == HashPrefix::transactionID)
        {
-            auto item = std::make_shared<SHAMapItem>(
+            auto item = std::make_shared<SHAMapItem const>(
                sha512Half(make_Slice(rawNode)),
                    s.peekData ());
            if (hashValid)
@@ -233,7 +233,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
                throw std::runtime_error ("invalid PLN node");
            }

-            auto item = std::make_shared<SHAMapItem> (u, s.peekData ());
+            auto item = std::make_shared<SHAMapItem const> (u, s.peekData ());
            if (hashValid)
                return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq, hash);
            return std::make_shared<SHAMapTreeNode>(item, tnACCOUNT_STATE, seq);
@@ -265,7 +265,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
            uint256 txID;
            s.get256 (txID, s.getLength () - 32);
            s.chop (32);
-            auto item = std::make_shared<SHAMapItem> (txID, s.peekData ());
+            auto item = std::make_shared<SHAMapItem const> (txID, s.peekData ());
            if (hashValid)
                return std::make_shared<SHAMapTreeNode>(item, tnTRANSACTION_MD, seq, hash);
            return std::make_shared<SHAMapTreeNode>(item, tnTRANSACTION_MD, seq);
@@ -347,7 +347,7 @@ SHAMapTreeNode::updateHash()
 }

 void
-SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format)
+SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format) const
 {
    assert ((format == snfPREFIX) || (format == snfWIRE) || (format == snfHASH));

@@ -397,7 +397,7 @@ SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format)
 }

 void
-SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format)
+SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format) const
 {
    assert ((format == snfPREFIX) || (format == snfWIRE) || (format == snfHASH));

@@ -455,7 +455,7 @@ SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format)
        assert (false);
 }

-bool SHAMapTreeNode::setItem (std::shared_ptr<SHAMapItem> const& i, TNType type)
+bool SHAMapTreeNode::setItem (std::shared_ptr<SHAMapItem const> const& i, TNType type)
 {
    mType = type;
    mItem = i;
--- a/src/ripple/shamap/tests/SHAMap.test.cpp
+++ b/src/ripple/shamap/tests/SHAMap.test.cpp
@@ -68,7 +68,7 @@ public:
        unexpected (!sMap.addItem (i2, true, false), "no add");
        unexpected (!sMap.addItem (i1, true, false), "no add");

-        std::shared_ptr<SHAMapItem> i;
+        std::shared_ptr<SHAMapItem const> i;
        i = sMap.peekFirstItem ();
        unexpected (!i || (*i != i1), "bad traverse");
        i = sMap.peekNextItem (i->getTag ());