diff --git a/src/ripple/module/app/ledger/InboundLedgers.cpp b/src/ripple/module/app/ledger/InboundLedgers.cpp index 43d3cc4f5..b7716b229 100644 --- a/src/ripple/module/app/ledger/InboundLedgers.cpp +++ b/src/ripple/module/app/ledger/InboundLedgers.cpp @@ -250,7 +250,6 @@ public: return; SHAMapTreeNode newNode( - SHAMapNodeID (node.nodeid().data(), node.nodeid().size()), Blob (node.nodedata().begin(), node.nodedata().end()), 0, snfWIRE, uZero, false); diff --git a/src/ripple/module/app/shamap/SHAMap.cpp b/src/ripple/module/app/shamap/SHAMap.cpp index 5db1c9a8f..4a7e37175 100644 --- a/src/ripple/module/app/shamap/SHAMap.cpp +++ b/src/ripple/module/app/shamap/SHAMap.cpp @@ -50,7 +50,7 @@ SHAMap::SHAMap ( mTNByID.rehash (STATE_MAP_BUCKETS); SHAMapNodeID rootID{}; - root = std::make_shared (mSeq, rootID); + root = std::make_shared (mSeq); root->makeInner (); mTNByID.replace(rootID, root); } @@ -74,7 +74,7 @@ SHAMap::SHAMap ( mTNByID.rehash (STATE_MAP_BUCKETS); SHAMapNodeID rootID{}; - root = std::make_shared (mSeq, rootID); + root = std::make_shared (mSeq); root->makeInner (); mTNByID.replace(rootID, root); } @@ -357,7 +357,7 @@ SHAMapTreeNode* SHAMap::getNodePointerNT (const SHAMapNodeID& id, uint256 const& if (filter->haveNode (id, hash, nodeData)) { SHAMapTreeNode::pointer node = std::make_shared ( - id, nodeData, 0, snfPREFIX, hash, true); + nodeData, 0, snfPREFIX, hash, true); canonicalize (hash, node); // Canonicalize the node with mTNByID to make sure all threads gets the same node @@ -821,7 +821,7 @@ bool SHAMap::addGiveItem (SHAMapItem::ref item, bool isTransaction, bool hasMeta assert (node->isEmptyBranch (branch)); SHAMapNodeID newNodeID = nodeID.getChildNodeID (branch); SHAMapTreeNode::pointer newNode = - std::make_shared (newNodeID, item, type, mSeq); + std::make_shared (item, type, mSeq); if (!mTNByID.peekMap().emplace (newNodeID, newNode).second) { @@ -851,7 +851,7 @@ bool SHAMap::addGiveItem (SHAMapItem::ref item, bool isTransaction, bool hasMeta // we need a new inner node, since both go on same branch at this level SHAMapNodeID newNodeID = nodeID.getChildNodeID (b1); SHAMapTreeNode::pointer newNode = - std::make_shared (mSeq, newNodeID); + std::make_shared (mSeq); newNode->makeInner (); if (!mTNByID.peekMap().emplace (newNodeID, newNode).second) @@ -867,7 +867,7 @@ bool SHAMap::addGiveItem (SHAMapItem::ref item, bool isTransaction, bool hasMeta assert (node->isInner ()); SHAMapNodeID newNodeID = nodeID.getChildNodeID (b1); SHAMapTreeNode::pointer newNode = - std::make_shared (newNodeID, item, type, mSeq); + std::make_shared (item, type, mSeq); assert (newNode->isValid () && newNode->isLeaf ()); if (!mTNByID.peekMap().emplace (newNodeID, newNode).second) @@ -876,8 +876,7 @@ bool SHAMap::addGiveItem (SHAMapItem::ref item, bool isTransaction, bool hasMeta node->setChildHash (b1, newNode->getNodeHash ()); // OPTIMIZEME hash op not needed trackNewNode (newNode, newNodeID); newNodeID = nodeID.getChildNodeID (b2); - newNode = std::make_shared (newNodeID, otherItem, type, - mSeq); + newNode = std::make_shared (otherItem, type, mSeq); assert (newNode->isValid () && newNode->isLeaf ()); if (!mTNByID.peekMap().emplace (newNodeID, newNode).second) @@ -962,7 +961,7 @@ SHAMapTreeNode* SHAMap::getNodeAsync ( return ptr.get (); // Try the tree node cache - ptr = getCache (hash, id); + ptr = getCache (hash); if (!ptr) { @@ -974,7 +973,7 @@ SHAMapTreeNode* SHAMap::getNodeAsync ( if (filter->haveNode (id, hash, nodeData)) { ptr = std::make_shared ( - id, nodeData, 0, snfPREFIX, hash, true); + nodeData, 0, snfPREFIX, hash, true); filter->gotNode (true, id, hash, nodeData, ptr->getType ()); } } @@ -999,7 +998,7 @@ SHAMapTreeNode* SHAMap::getNodeAsync ( if (!obj) return nullptr; - ptr = std::make_shared (id, obj->getData(), 0, + ptr = std::make_shared (obj->getData(), 0, snfPREFIX, hash, true); } @@ -1036,7 +1035,7 @@ SHAMap::fetchNodeExternalNT (const SHAMapNodeID& id, uint256 const& hash) return ret; // Check the cache of shared, immutable tree nodes - ret = getCache (hash, id); + ret = getCache (hash); if (ret) { // The node was found in the TreeNodeCache assert (ret->getSeq() == 0); @@ -1059,7 +1058,7 @@ SHAMap::fetchNodeExternalNT (const SHAMapNodeID& id, uint256 const& hash) { // We make this node immutable (seq == 0) so that it can be shared // CoW is needed if it is modified - ret = std::make_shared (id, obj->getData (), 0, snfPREFIX, hash, true); + ret = std::make_shared (obj->getData (), 0, snfPREFIX, hash, true); if (ret->getNodeHash () != hash) { @@ -1116,7 +1115,7 @@ bool SHAMap::fetchRoot (uint256 const& hash, SHAMapSyncFilter* filter) if (!filter || !filter->haveNode (SHAMapNodeID (), hash, nodeData)) return false; - root = std::make_shared (SHAMapNodeID (), nodeData, + root = std::make_shared (nodeData, mSeq - 1, snfPREFIX, hash, true); mTNByID.replace(SHAMapNodeID (), root); filter->gotNode (true, SHAMapNodeID (), hash, nodeData, root->getType ()); @@ -1316,34 +1315,16 @@ void SHAMap::dump (bool hash) for (ripple::unordered_map::iterator it = mTNByID.peekMap().begin (); it != mTNByID.peekMap().end (); ++it) { - WriteLog (lsINFO, SHAMap) << it->second->getString (); + WriteLog (lsINFO, SHAMap) << it->second->getString (it->first); CondLog (hash, lsINFO, SHAMap) << it->second->getNodeHash (); } } -SHAMapTreeNode::pointer SHAMap::getCache (uint256 const& hash, SHAMapNodeID const& id) +SHAMapTreeNode::pointer SHAMap::getCache (uint256 const& hash) { SHAMapTreeNode::pointer ret = mTreeNodeCache.fetch (hash); assert (!ret || !ret->getSeq()); - - if (ret != nullptr) - { - SHAMapNodeID retID = ret->getID(); - if (retID != id) - { - // We have the data, but with a different node ID - WriteLog (lsTRACE, SHAMap) << "ID mismatch: " << id << " != " << retID; - ret = std::make_shared (*ret, 0); - ret->setID(id); - - // Future fetches are likely to use the "new" ID - mTreeNodeCache.canonicalize (hash, ret, true); - // HH I see no way to assure this at the moment - assert (ret->getID() == id); - assert (ret->getNodeHash() == hash); - } - } return ret; } @@ -1351,22 +1332,7 @@ void SHAMap::canonicalize (uint256 const& hash, SHAMapTreeNode::pointer& node) { assert (node->getSeq() == 0); - SHAMapNodeID id = node->getID(); - mTreeNodeCache.canonicalize (hash, node); - - // HH I don't see how to ensure that id has not changed under mTreeNodeCache.canonicalize - if (id != node->getID()) - { - // The cache has the node with a different ID - node = std::make_shared (*node, 0); - node->setID(id); - - // Future fetches are likely to use the newer ID - mTreeNodeCache.canonicalize (hash, node, true); - // HH I don't see how to ensure that id has not changed under mTreeNodeCache.canonicalize - assert (id == node->getID()); - } } //------------------------------------------------------------------------------ diff --git a/src/ripple/module/app/shamap/SHAMap.h b/src/ripple/module/app/shamap/SHAMap.h index 4e67df7d6..41a982010 100644 --- a/src/ripple/module/app/shamap/SHAMap.h +++ b/src/ripple/module/app/shamap/SHAMap.h @@ -114,7 +114,7 @@ public: typedef std::pair DeltaItem; typedef std::pair DeltaRef; typedef std::map Delta; - typedef ripple::unordered_map NodeMap; + typedef ripple::unordered_map NodeMap; typedef std::unordered_set DirtySet; typedef boost::shared_mutex LockType; @@ -257,7 +257,7 @@ private: void dump (bool withHashes = false); // tree node cache operations - SHAMapTreeNode::pointer getCache (uint256 const& hash, SHAMapNodeID const& id); + SHAMapTreeNode::pointer getCache (uint256 const& hash); void canonicalize (uint256 const& hash, SHAMapTreeNode::pointer&); void dirtyUp (std::stack>& stack, diff --git a/src/ripple/module/app/shamap/SHAMapSync.cpp b/src/ripple/module/app/shamap/SHAMapSync.cpp index 214dc07c8..216ef449b 100644 --- a/src/ripple/module/app/shamap/SHAMapSync.cpp +++ b/src/ripple/module/app/shamap/SHAMapSync.cpp @@ -353,14 +353,14 @@ SHAMapAddNode SHAMap::addRootNode (Blob const& rootNode, SHANodeFormat format, assert (mSeq >= 1); SHAMapTreeNode::pointer node = - std::make_shared (SHAMapNodeID (), rootNode, mSeq - 1, + std::make_shared (rootNode, mSeq - 1, format, uZero, false); if (!node) return SHAMapAddNode::invalid (); #ifdef BEAST_DEBUG - node->dump (); + node->dump (SHAMapNodeID ()); #endif root = node; @@ -395,7 +395,7 @@ SHAMapAddNode SHAMap::addRootNode (uint256 const& hash, Blob const& rootNode, SH assert (mSeq >= 1); SHAMapTreeNode::pointer node = - std::make_shared (SHAMapNodeID (), rootNode, mSeq - 1, + std::make_shared (rootNode, mSeq - 1, format, uZero, false); if (!node || node->getNodeHash () != hash) @@ -477,7 +477,7 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode, } SHAMapTreeNode::pointer newNode = - std::make_shared (node, rawNode, 0, snfWIRE, + std::make_shared (rawNode, 0, snfWIRE, uZero, false); if (childHash != newNode->getNodeHash ()) @@ -488,7 +488,7 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode, canonicalize (childHash, newNode); - if (!iNode->isInBounds ()) + if (!iNode->isInBounds (iNodeID)) { // Map is provably invalid mState = smsInvalid; diff --git a/src/ripple/module/app/shamap/SHAMapTreeNode.cpp b/src/ripple/module/app/shamap/SHAMapTreeNode.cpp index cc1f163be..a18ad4697 100644 --- a/src/ripple/module/app/shamap/SHAMapTreeNode.cpp +++ b/src/ripple/module/app/shamap/SHAMapTreeNode.cpp @@ -19,9 +19,8 @@ namespace ripple { -SHAMapTreeNode::SHAMapTreeNode (std::uint32_t seq, const SHAMapNodeID& nodeID) - : mID (nodeID) - , mHash (std::uint64_t(0)) +SHAMapTreeNode::SHAMapTreeNode (std::uint32_t seq) + : mHash (std::uint64_t(0)) , mSeq (seq) , mAccessSeq (seq) , mType (tnERROR) @@ -31,8 +30,7 @@ SHAMapTreeNode::SHAMapTreeNode (std::uint32_t seq, const SHAMapNodeID& nodeID) } SHAMapTreeNode::SHAMapTreeNode (const SHAMapTreeNode& node, std::uint32_t seq) - : mID (node.mID) - , mHash (node.mHash) + : mHash (node.mHash) , mSeq (seq) , mType (node.mType) , mIsBranch (node.mIsBranch) @@ -44,10 +42,9 @@ SHAMapTreeNode::SHAMapTreeNode (const SHAMapTreeNode& node, std::uint32_t seq) memcpy (mHashes, node.mHashes, sizeof (mHashes)); } -SHAMapTreeNode::SHAMapTreeNode (const SHAMapNodeID& id, SHAMapItem::ref item, +SHAMapTreeNode::SHAMapTreeNode (SHAMapItem::ref item, TNType type, std::uint32_t seq) - : mID (id) - , mItem (item) + : mItem (item) , mSeq (seq) , mType (type) , mIsBranch (0) @@ -57,11 +54,10 @@ SHAMapTreeNode::SHAMapTreeNode (const SHAMapNodeID& id, SHAMapItem::ref item, updateHash (); } -SHAMapTreeNode::SHAMapTreeNode (const SHAMapNodeID& id, Blob const& rawNode, +SHAMapTreeNode::SHAMapTreeNode (Blob const& rawNode, std::uint32_t seq, SHANodeFormat format, uint256 const& hash, bool hashValid) - : mID (id) - , mSeq (seq) + : mSeq (seq) , mType (tnERROR) , mIsBranch (0) , mFullBelow (false) @@ -426,17 +422,17 @@ void SHAMapTreeNode::makeInner () mHash.zero (); } -void SHAMapTreeNode::dump () +void SHAMapTreeNode::dump (const SHAMapNodeID & id) { - WriteLog (lsDEBUG, SHAMapNodeID) << "SHAMapTreeNode(" << mID.getNodeID () << ")"; + WriteLog (lsDEBUG, SHAMapNodeID) << "SHAMapTreeNode(" << id.getNodeID () << ")"; } -std::string SHAMapTreeNode::getString () const +std::string SHAMapTreeNode::getString (const SHAMapNodeID & id) const { std::string ret = "NodeID("; - ret += beast::lexicalCastThrow (mID.getDepth ()); + ret += beast::lexicalCastThrow (id.getDepth ()); ret += ","; - ret += to_string (mID.getNodeID ()); + ret += to_string (id.getNodeID ()); ret += ")"; if (isInner ()) diff --git a/src/ripple/module/app/shamap/SHAMapTreeNode.h b/src/ripple/module/app/shamap/SHAMapTreeNode.h index 9ae52b97e..41200540e 100644 --- a/src/ripple/module/app/shamap/SHAMapTreeNode.h +++ b/src/ripple/module/app/shamap/SHAMapTreeNode.h @@ -55,13 +55,12 @@ public: SHAMapTreeNode (const SHAMapTreeNode&) = delete; SHAMapTreeNode& operator= (const SHAMapTreeNode&) = delete; - SHAMapTreeNode (std::uint32_t seq, const SHAMapNodeID & nodeID); // empty node + SHAMapTreeNode (std::uint32_t seq); // empty node SHAMapTreeNode (const SHAMapTreeNode & node, std::uint32_t seq); // copy node from older tree - SHAMapTreeNode (const SHAMapNodeID & nodeID, SHAMapItem::ref item, TNType type, - std::uint32_t seq); + SHAMapTreeNode (SHAMapItem::ref item, TNType type, std::uint32_t seq); // raw node functions - SHAMapTreeNode (const SHAMapNodeID & id, Blob const & data, std::uint32_t seq, + SHAMapTreeNode (Blob const & data, std::uint32_t seq, SHANodeFormat format, uint256 const & hash, bool hashValid); void addRaw (Serializer&, SHANodeFormat format); @@ -103,10 +102,10 @@ public: { return mType == tnINNER; } - bool isInBounds () const + bool isInBounds (SHAMapNodeID const &id) const { // Nodes at depth 64 must be leaves - return (!isInner() || (mID.getDepth() < 64)); + return (!isInner() || (id.getDepth() < 64)); } bool isValid () const { @@ -173,11 +172,8 @@ public: mFullBelow = true; } - virtual void dump (); - virtual std::string getString () const; - - SHAMapNodeID const& getID() const {return mID;} - void setID(SHAMapNodeID const& id) {mID = id;} + virtual void dump (SHAMapNodeID const&); + virtual std::string getString (SHAMapNodeID const&) const; /** Descends along the specified branch * On invocation, nodeID must be the ID of this node @@ -196,7 +192,6 @@ private: // VFALCO TODO remove the use of friend friend class SHAMap; - SHAMapNodeID mID; uint256 mHash; uint256 mHashes[16]; SHAMapItem::pointer mItem;