From ab77444fa3c739b126a222b22d10fac0597f45b7 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Mon, 21 Sep 2020 21:33:27 -0700 Subject: [PATCH] Simplify SHAMapNodeID: The existing SHAMapNodeID object has both a valid and an invalid state and requirs callers to verify the state of an instance prior to using it. A simple set of changes removes that restriction and ensures that all instances are valid, making the code more robust. This change also: 1. Introduces a new function to construct a SHAMapNodeID from a serialized blob; and 2. Reduces the amount of constructors the class exposes. --- src/ripple/app/ledger/impl/InboundLedger.cpp | 14 +- .../app/ledger/impl/InboundTransactions.cpp | 14 +- src/ripple/overlay/impl/PeerImp.cpp | 14 +- src/ripple/shamap/README.md | 6 +- src/ripple/shamap/SHAMap.h | 58 +++-- src/ripple/shamap/SHAMapNodeID.h | 208 ++++++++---------- src/ripple/shamap/impl/SHAMap.cpp | 35 +-- src/ripple/shamap/impl/SHAMapNodeID.cpp | 135 +++++------- src/ripple/shamap/impl/SHAMapSync.cpp | 11 +- src/ripple/shamap/impl/SHAMapTreeNode.cpp | 11 +- src/test/overlay/compression_test.cpp | 4 +- src/test/shamap/FetchPack_test.cpp | 1 + 12 files changed, 245 insertions(+), 266 deletions(-) diff --git a/src/ripple/app/ledger/impl/InboundLedger.cpp b/src/ripple/app/ledger/impl/InboundLedger.cpp index 39fe904016..7513a2248b 100644 --- a/src/ripple/app/ledger/impl/InboundLedger.cpp +++ b/src/ripple/app/ledger/impl/InboundLedger.cpp @@ -947,14 +947,20 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san) { for (auto const& node : packet.nodes()) { - SHAMapNodeID const nodeID( - node.nodeid().data(), node.nodeid().size()); - if (nodeID.isRoot()) + auto const nodeID = deserializeSHAMapNodeID(node.nodeid()); + + if (!nodeID) + { + san.incInvalid(); + return; + } + + if (nodeID->isRoot()) san += map.addRootNode( rootHash, makeSlice(node.nodedata()), filter.get()); else san += map.addKnownNode( - nodeID, makeSlice(node.nodedata()), filter.get()); + *nodeID, makeSlice(node.nodedata()), filter.get()); if (!san.isGood()) { diff --git a/src/ripple/app/ledger/impl/InboundTransactions.cpp b/src/ripple/app/ledger/impl/InboundTransactions.cpp index b4c2cf734b..db67f1738b 100644 --- a/src/ripple/app/ledger/impl/InboundTransactions.cpp +++ b/src/ripple/app/ledger/impl/InboundTransactions.cpp @@ -159,15 +159,21 @@ public: std::list nodeData; for (auto const& node : packet.nodes()) { - if (!node.has_nodeid() || !node.has_nodedata() || - (node.nodeid().size() != 33)) + if (!node.has_nodeid() || !node.has_nodedata()) { peer->charge(Resource::feeInvalidRequest); return; } - nodeIDs.emplace_back( - node.nodeid().data(), static_cast(node.nodeid().size())); + auto const id = deserializeSHAMapNodeID(node.nodeid()); + + if (!id) + { + peer->charge(Resource::feeBadData); + return; + } + + nodeIDs.emplace_back(*id); nodeData.emplace_back( node.nodedata().begin(), node.nodedata().end()); } diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 63c1b1c5c6..39e2691726 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -2794,12 +2794,12 @@ PeerImp::getLedger(std::shared_ptr const& m) (reply.nodes().size() < Tuning::maxReplyNodes)); ++i) { - SHAMapNodeID mn(packet.nodeids(i).data(), packet.nodeids(i).size()); + auto const mn = deserializeSHAMapNodeID(packet.nodeids(i)); - if (!mn.isValid()) + if (!mn) { JLOG(p_journal_.warn()) << "GetLedger: Invalid node " << logMe; - charge(Resource::feeInvalidRequest); + charge(Resource::feeBadData); return; } @@ -2808,7 +2808,7 @@ PeerImp::getLedger(std::shared_ptr const& m) try { - if (map->getNodeFat(mn, nodeIDs, rawNodes, fatLeaves, depth)) + if (map->getNodeFat(*mn, nodeIDs, rawNodes, fatLeaves, depth)) { assert(nodeIDs.size() == rawNodes.size()); JLOG(p_journal_.trace()) << "GetLedger: getNodeFat got " @@ -2821,10 +2821,8 @@ PeerImp::getLedger(std::shared_ptr const& m) nodeIDIterator != nodeIDs.end(); ++nodeIDIterator, ++rawNodeIterator) { - Serializer nID(33); - nodeIDIterator->addIDRaw(nID); protocol::TMLedgerNode* node = reply.add_nodes(); - node->set_nodeid(nID.getDataPtr(), nID.getLength()); + node->set_nodeid(nodeIDIterator->getRawString()); node->set_nodedata( &rawNodeIterator->front(), rawNodeIterator->size()); } @@ -2852,7 +2850,7 @@ PeerImp::getLedger(std::shared_ptr const& m) info += ", no hash specified"; JLOG(p_journal_.warn()) - << "getNodeFat( " << mn << ") throws exception: " << info; + << "getNodeFat( " << *mn << ") throws exception: " << info; } } diff --git a/src/ripple/shamap/README.md b/src/ripple/shamap/README.md index a04a63c5f5..76d514f47c 100644 --- a/src/ripple/shamap/README.md +++ b/src/ripple/shamap/README.md @@ -184,9 +184,9 @@ by 4 bits, and then packed in sequence into a `uint256` (such that the longest path possible has 256 / 4 = 64 steps). The high 4 bits of the first byte identify which child of the root is chosen, the lower 4 bits of the first byte identify the child of that node, and so on. The `SHAMapNodeID` identifying the -root node has an ID of 0 and a depth of 0. See `SHAMapNodeID::selectBranch` for -details of how a `SHAMapNodeID` selects a "branch" (child) by indexing into its -path with its depth. +root node has an ID of 0 and a depth of 0. See `selectBranch` for details of +how we use a `SHAMapNodeID` to select a "branch" (child) by indexing into a +path at a given depth. While the current node is an inner node, traversing down the trie from the root continues, unless the path indicates a child that does not exist. And in this diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index 972ea88f26..5dff587964 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -29,8 +29,6 @@ #include #include #include -#include -#include #include #include #include @@ -39,18 +37,35 @@ namespace ripple { -enum class SHAMapState { - Modifying = 0, // Objects can be added and removed (like an open ledger) - Immutable = 1, // Map cannot be changed (like a closed ledger) - Synching = 2, // Map's hash is locked in, valid nodes can be added (like a - // peer's closing ledger) - Floating = 3, // Map is free to change hash (like a synching open ledger) - Invalid = - 4, // Map is known not to be valid (usually synching a corrupt ledger) -}; +class SHAMapNodeID; +class SHAMapSyncFilter; -/** Function object which handles missing nodes. */ -using MissingNodeHandler = std::function; +/** Describes the current state of a given SHAMap */ +enum class SHAMapState { + /** The map is in flux and objects can be added and removed. + + Example: map underlying the open ledger. + */ + Modifying = 0, + + /** The map is set in stone and cannot be changed. + + Example: a map underlying a given closed ledger. + */ + Immutable = 1, + + /** The map's hash is fixed but valid nodes may be missing and can be added. + + Example: a map that's syncing a given peer's closing ledger. + */ + Synching = 2, + + /** The map is known to not be valid. + + Example: usually synching a corrupt ledger. + */ + Invalid = 3, +}; /** A SHAMap is both a radix tree with a fan-out of 16 and a Merkle tree. @@ -61,11 +76,11 @@ using MissingNodeHandler = std::function; 2. A node with only one child is merged with that child (the "merge property") - These properties in a significantly smaller memory footprint for a radix - tree. + These properties result in a significantly smaller memory footprint for + a radix tree. - And a fan-out of 16 means that each node in the tree has at most 16 - children. See https://en.wikipedia.org/wiki/Radix_tree + A fan-out of 16 means that each node in the tree has at most 16 + children. See https://en.wikipedia.org/wiki/Radix_tree A Merkle tree is a tree where each non-leaf node is labelled with the hash of the combined labels of its children nodes. @@ -89,6 +104,12 @@ private: bool full_ = false; // Map is believed complete in database public: + /** Each non-leaf node has 16 children (the 'radix tree' part of the map) */ + static inline constexpr unsigned int branchFactor = 16; + + /** The depth of the hash map: data is only present in the leaves */ + static inline constexpr unsigned int leafDepth = 64; + using DeltaItem = std::pair< std::shared_ptr, std::shared_ptr>; @@ -499,8 +520,7 @@ SHAMap::setImmutable() inline bool SHAMap::isSynching() const { - return (state_ == SHAMapState::Floating) || - (state_ == SHAMapState::Synching); + return state_ == SHAMapState::Synching; } inline void diff --git a/src/ripple/shamap/SHAMapNodeID.h b/src/ripple/shamap/SHAMapNodeID.h index a0041eecec..0fcc685ca7 100644 --- a/src/ripple/shamap/SHAMapNodeID.h +++ b/src/ripple/shamap/SHAMapNodeID.h @@ -21,160 +21,130 @@ #define RIPPLE_SHAMAP_SHAMAPNODEID_H_INCLUDED #include -#include -#include +#include #include #include #include namespace ripple { -// Identifies a node in a half-SHA512 (256 bit) hash map +/** Identifies a node inside a SHAMap */ class SHAMapNodeID { private: - uint256 mNodeID; - int mDepth; + uint256 id_; + unsigned int depth_ = 0; public: - SHAMapNodeID(); - SHAMapNodeID(int depth, uint256 const& hash); - SHAMapNodeID(void const* ptr, int len); + SHAMapNodeID() = default; + SHAMapNodeID(SHAMapNodeID const& other) = default; + SHAMapNodeID(unsigned int depth, uint256 const& hash); + + SHAMapNodeID& operator=(SHAMapNodeID const& other) = default; bool - isValid() const; - bool - isRoot() const; + isRoot() const + { + return depth_ == 0; + } - // Convert to/from wire format (256-bit nodeID, 1-byte depth) - void - addIDRaw(Serializer& s) const; + // Get the wire format (256-bit nodeID, 1-byte depth) std::string getRawString() const; - bool - operator==(const SHAMapNodeID& n) const; - bool - operator!=(const SHAMapNodeID& n) const; - - bool - operator<(const SHAMapNodeID& n) const; - bool - operator>(const SHAMapNodeID& n) const; - bool - operator<=(const SHAMapNodeID& n) const; - bool - operator>=(const SHAMapNodeID& n) const; - - std::string - getString() const; - void - dump(beast::Journal journal) const; - - // Only used by SHAMap and SHAMapTreeNode + unsigned int + getDepth() const + { + return depth_; + } uint256 const& - getNodeID() const; + getNodeID() const + { + return id_; + } + SHAMapNodeID - getChildNodeID(int m) const; - int - selectBranch(uint256 const& hash) const; - int - getDepth() const; + getChildNodeID(unsigned int m) const; + + // FIXME-C++20: use spaceship and operator synthesis + /** Comparison operators */ bool - has_common_prefix(SHAMapNodeID const& other) const; + operator<(SHAMapNodeID const& n) const + { + return std::tie(depth_, id_) < std::tie(n.depth_, n.id_); + } -private: - static uint256 const& - Masks(int depth); + bool + operator>(SHAMapNodeID const& n) const + { + return n < *this; + } - friend std::ostream& - operator<<(std::ostream& out, SHAMapNodeID const& node); + bool + operator<=(SHAMapNodeID const& n) const + { + return !(n < *this); + } -private: // Currently unused - SHAMapNodeID - getParentNodeID() const; + bool + operator>=(SHAMapNodeID const& n) const + { + return !(*this < n); + } + + bool + operator==(SHAMapNodeID const& n) const + { + return (depth_ == n.depth_) && (id_ == n.id_); + } + + bool + operator!=(SHAMapNodeID const& n) const + { + return !(*this == n); + } }; -//------------------------------------------------------------------------------ - -inline SHAMapNodeID::SHAMapNodeID() : mDepth(0) +inline std::string +to_string(SHAMapNodeID const& node) { -} + if (node.isRoot()) + return "NodeID(root)"; -inline int -SHAMapNodeID::getDepth() const -{ - return mDepth; -} - -inline uint256 const& -SHAMapNodeID::getNodeID() const -{ - return mNodeID; -} - -inline bool -SHAMapNodeID::isValid() const -{ - return (mDepth >= 0) && (mDepth <= 64); -} - -inline bool -SHAMapNodeID::isRoot() const -{ - return mDepth == 0; -} - -inline SHAMapNodeID -SHAMapNodeID::getParentNodeID() const -{ - assert(mDepth); - return SHAMapNodeID(mDepth - 1, mNodeID & Masks(mDepth - 1)); -} - -inline bool -SHAMapNodeID::operator<(const SHAMapNodeID& n) const -{ - return std::tie(mDepth, mNodeID) < std::tie(n.mDepth, n.mNodeID); -} - -inline bool -SHAMapNodeID::operator>(const SHAMapNodeID& n) const -{ - return n < *this; -} - -inline bool -SHAMapNodeID::operator<=(const SHAMapNodeID& n) const -{ - return !(n < *this); -} - -inline bool -SHAMapNodeID::operator>=(const SHAMapNodeID& n) const -{ - return !(*this < n); -} - -inline bool -SHAMapNodeID::operator==(const SHAMapNodeID& n) const -{ - return (mDepth == n.mDepth) && (mNodeID == n.mNodeID); -} - -inline bool -SHAMapNodeID::operator!=(const SHAMapNodeID& n) const -{ - return !(*this == n); + return "NodeID(" + std::to_string(node.getDepth()) + "," + + to_string(node.getNodeID()) + ")"; } inline std::ostream& operator<<(std::ostream& out, SHAMapNodeID const& node) { - return out << node.getString(); + return out << to_string(node); } +/** Return an object representing a serialized SHAMap Node ID + * + * @param s A string of bytes + * @param data a non-null pointer to a buffer of @param size bytes. + * @param size the size, in bytes, of the buffer pointed to by @param data. + * @return A seated optional if the buffer contained a serialized SHAMap + * node ID and an unseated optional otherwise. + */ +/** @{ */ +[[nodiscard]] std::optional +deserializeSHAMapNodeID(void const* data, std::size_t size); + +[[nodiscard]] inline std::optional +deserializeSHAMapNodeID(std::string const& s) +{ + return deserializeSHAMapNodeID(s.data(), s.size()); +} +/** @} */ + +/** Returns the branch that would contain the given hash */ +[[nodiscard]] unsigned int +selectBranch(SHAMapNodeID const& id, uint256 const& hash); + } // namespace ripple #endif diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 19dff57c9c..db1a55480c 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -19,6 +19,8 @@ #include #include +#include +#include namespace ripple { @@ -99,7 +101,7 @@ SHAMap::dirtyUp( stack.pop(); assert(node != nullptr); - int branch = nodeID.selectBranch(target); + int branch = selectBranch(nodeID, target); assert(branch >= 0); node = unshareNode(std::move(node), nodeID); @@ -122,7 +124,7 @@ SHAMap::walkTowardsKey(uint256 const& id, SharedPtrNodeStack* stack) const stack->push({inNode, nodeID}); auto const inner = std::static_pointer_cast(inNode); - auto const branch = nodeID.selectBranch(id); + auto const branch = selectBranch(nodeID, id); if (inner->isEmptyBranch(branch)) return nullptr; @@ -333,7 +335,7 @@ SHAMap::descend( SHAMapSyncFilter* filter) const { assert(parent->isInner()); - assert((branch >= 0) && (branch < 16)); + assert((branch >= 0) && (branch < branchFactor)); assert(!parent->isEmptyBranch(branch)); SHAMapAbstractNode* child = parent->getChildPointer(branch); @@ -428,7 +430,7 @@ SHAMap::firstBelow( if (node->isLeaf()) { auto n = std::static_pointer_cast(node); - stack.push({node, {64, n->peekItem()->key()}}); + stack.push({node, {leafDepth, n->peekItem()->key()}}); return n.get(); } auto inner = std::static_pointer_cast(node); @@ -436,7 +438,7 @@ SHAMap::firstBelow( stack.push({inner, SHAMapNodeID{}}); else stack.push({inner, stack.top().second.getChildNodeID(branch)}); - for (int i = 0; i < 16;) + for (int i = 0; i < branchFactor;) { if (!inner->isEmptyBranch(i)) { @@ -445,7 +447,7 @@ SHAMap::firstBelow( if (node->isLeaf()) { auto n = std::static_pointer_cast(node); - stack.push({n, {64, n->peekItem()->key()}}); + stack.push({n, {leafDepth, n->peekItem()->key()}}); return n.get(); } inner = std::static_pointer_cast(node); @@ -469,7 +471,7 @@ SHAMap::onlyBelow(SHAMapAbstractNode* node) const { SHAMapAbstractNode* nextNode = nullptr; auto inner = static_cast(node); - for (int i = 0; i < 16; ++i) + for (int i = 0; i < branchFactor; ++i) { if (!inner->isEmptyBranch(i)) { @@ -522,7 +524,7 @@ SHAMap::peekNextItem(uint256 const& id, SharedPtrNodeStack& stack) const auto [node, nodeID] = stack.top(); assert(!node->isLeaf()); auto inner = std::static_pointer_cast(node); - for (auto i = nodeID.selectBranch(id) + 1; i < 16; ++i) + for (auto i = selectBranch(nodeID, id) + 1; i < branchFactor; ++i) { if (!inner->isEmptyBranch(i)) { @@ -595,7 +597,8 @@ SHAMap::upper_bound(uint256 const& id) const else { auto inner = std::static_pointer_cast(node); - for (auto branch = nodeID.selectBranch(id) + 1; branch < 16; + for (auto branch = selectBranch(nodeID, id) + 1; + branch < branchFactor; ++branch) { if (!inner->isEmptyBranch(branch)) @@ -654,7 +657,7 @@ SHAMap::delItem(uint256 const& id) stack.pop(); node = unshareNode(std::move(node), nodeID); - node->setChild(nodeID.selectBranch(id), prevNode); + node->setChild(selectBranch(nodeID, id), prevNode); if (!nodeID.isRoot()) { @@ -673,7 +676,7 @@ SHAMap::delItem(uint256 const& id) if (item) { - for (int i = 0; i < 16; ++i) + for (int i = 0; i < branchFactor; ++i) { if (!node->isEmptyBranch(i)) { @@ -735,7 +738,7 @@ SHAMap::addGiveItem( { // easy case, we end on an inner node auto inner = std::static_pointer_cast(node); - int branch = nodeID.selectBranch(tag); + int branch = selectBranch(nodeID, tag); assert(inner->isEmptyBranch(branch)); auto newNode = std::make_shared(std::move(item), type, seq_); @@ -753,8 +756,8 @@ SHAMap::addGiveItem( int b1, b2; - while ((b1 = nodeID.selectBranch(tag)) == - (b2 = nodeID.selectBranch(otherItem->key()))) + while ((b1 = selectBranch(nodeID, tag)) == + (b2 = selectBranch(nodeID, otherItem->key()))) { stack.push({node, nodeID}); @@ -988,7 +991,7 @@ SHAMap::walkSubTree(bool doWrite, NodeObjectType t, std::uint32_t seq) // We can't flush an inner node until we flush its children while (1) { - while (pos < 16) + while (pos < branchFactor) { if (node->isEmptyBranch(pos)) { @@ -1095,7 +1098,7 @@ SHAMap::dump(bool hash) const if (node->isInner()) { auto inner = static_cast(node); - for (int i = 0; i < 16; ++i) + for (int i = 0; i < branchFactor; ++i) { if (!inner->isEmptyBranch(i)) { diff --git a/src/ripple/shamap/impl/SHAMapNodeID.cpp b/src/ripple/shamap/impl/SHAMapNodeID.cpp index 5b14585943..b239bb890a 100644 --- a/src/ripple/shamap/impl/SHAMapNodeID.cpp +++ b/src/ripple/shamap/impl/SHAMapNodeID.cpp @@ -17,18 +17,17 @@ */ //============================================================================== -#include #include #include +#include +#include #include -#include #include -#include namespace ripple { -uint256 const& -SHAMapNodeID::Masks(int depth) +static uint256 const& +depthMask(unsigned int depth) { enum { mask_size = 65 }; @@ -49,106 +48,88 @@ SHAMapNodeID::Masks(int depth) entry[mask_size - 1] = selector; } }; + static masks_t const masks; return masks.entry[depth]; } // canonicalize the hash to a node ID for this depth -SHAMapNodeID::SHAMapNodeID(int depth, uint256 const& hash) - : mNodeID(hash), mDepth(depth) +SHAMapNodeID::SHAMapNodeID(unsigned int depth, uint256 const& hash) + : id_(hash), depth_(depth) { - assert((depth >= 0) && (depth < 65)); - assert(mNodeID == (mNodeID & Masks(depth))); -} - -SHAMapNodeID::SHAMapNodeID(void const* ptr, int len) -{ - if (len < 33) - mDepth = -1; - else - { - std::memcpy(mNodeID.begin(), ptr, 32); - mDepth = *(static_cast(ptr) + 32); - } -} - -std::string -SHAMapNodeID::getString() const -{ - if ((mDepth == 0) && (mNodeID.isZero())) - return "NodeID(root)"; - - return "NodeID(" + std::to_string(mDepth) + "," + to_string(mNodeID) + ")"; -} - -void -SHAMapNodeID::addIDRaw(Serializer& s) const -{ - s.addBitString(mNodeID); - s.add8(mDepth); + assert(depth <= SHAMap::leafDepth); + assert(id_ == (id_ & depthMask(depth))); } std::string SHAMapNodeID::getRawString() const { Serializer s(33); - addIDRaw(s); + s.addBitString(id_); + s.add8(depth_); return s.getString(); } -// This can be optimized to avoid the << if needed SHAMapNodeID -SHAMapNodeID::getChildNodeID(int m) const +SHAMapNodeID::getChildNodeID(unsigned int m) const { - assert((m >= 0) && (m < 16)); - assert(mDepth < 64); + assert(m < SHAMap::branchFactor); - uint256 child(mNodeID); - child.begin()[mDepth / 2] |= (mDepth & 1) ? m : (m << 4); + // A SHAMap has exactly 65 levels, so nodes must not exceed that + // depth; if they do, this breaks the invariant of never allowing + // the construction of a SHAMapNodeID at an invalid depth. We assert + // to catch this in debug builds. + // + // We throw (but never assert) if the node is at level 64, since + // entries at that depth are leaf nodes and have no children and even + // constructing a child node from them would break the above invariant. + assert(depth_ <= SHAMap::leafDepth); - return SHAMapNodeID(mDepth + 1, child); + if (depth_ >= SHAMap::leafDepth) + Throw( + "Request for child node ID of " + to_string(*this)); + + if (id_ != (id_ & depthMask(depth_))) + Throw("Incorrect mask for " + to_string(*this)); + + SHAMapNodeID node{depth_ + 1, id_}; + node.id_.begin()[depth_ / 2] |= (depth_ & 1) ? m : (m << 4); + return node; } -// Which branch would contain the specified hash -int -SHAMapNodeID::selectBranch(uint256 const& hash) const +[[nodiscard]] std::optional +deserializeSHAMapNodeID(void const* data, std::size_t size) { - int branch = *(hash.begin() + (mDepth / 2)); + std::optional ret; - if (mDepth & 1) + if (size == 33) + { + unsigned int depth = *(static_cast(data) + 32); + if (depth <= SHAMap::leafDepth) + { + auto const id = uint256::fromVoid(data); + + if (id == (id & depthMask(depth))) + ret.emplace(depth, id); + } + } + + return ret; +} + +[[nodiscard]] unsigned int +selectBranch(SHAMapNodeID const& id, uint256 const& hash) +{ + auto const depth = id.getDepth(); + auto branch = static_cast(*(hash.begin() + (depth / 2))); + + if (depth & 1) branch &= 0xf; else branch >>= 4; - assert((branch >= 0) && (branch < 16)); - + assert(branch < SHAMap::branchFactor); return branch; } -bool -SHAMapNodeID::has_common_prefix(SHAMapNodeID const& other) const -{ - assert(mDepth <= other.mDepth); - auto x = mNodeID.begin(); - auto y = other.mNodeID.begin(); - for (unsigned i = 0; i < mDepth / 2; ++i, ++x, ++y) - { - if (*x != *y) - return false; - } - if (mDepth & 1) - { - auto i = mDepth / 2; - return (*(mNodeID.begin() + i) & 0xF0) == - (*(other.mNodeID.begin() + i) & 0xF0); - } - return true; -} - -void -SHAMapNodeID::dump(beast::Journal journal) const -{ - JLOG(journal.debug()) << getString(); -} - } // namespace ripple diff --git a/src/ripple/shamap/impl/SHAMapSync.cpp b/src/ripple/shamap/impl/SHAMapSync.cpp index 19f3937985..35d4f77ec1 100644 --- a/src/ripple/shamap/impl/SHAMapSync.cpp +++ b/src/ripple/shamap/impl/SHAMapSync.cpp @@ -18,8 +18,8 @@ //============================================================================== #include -#include #include +#include namespace ripple { @@ -459,7 +459,7 @@ SHAMap::getNodeFat( while (node && node->isInner() && (nodeID.getDepth() < wanted.getDepth())) { - int branch = nodeID.selectBranch(wanted.getNodeID()); + int branch = selectBranch(nodeID, wanted.getNodeID()); auto inner = static_cast(node); if (inner->isEmptyBranch(branch)) return false; @@ -595,7 +595,6 @@ SHAMap::addKnownNode( Slice const& rawNode, SHAMapSyncFilter* filter) { - // return value: true=okay, false=error assert(!node.isRoot()); if (!isSynching()) @@ -613,7 +612,7 @@ SHAMap::addKnownNode( !static_cast(iNode)->isFullBelow(generation) && (iNodeID.getDepth() < node.getDepth())) { - int branch = iNodeID.selectBranch(node.getNodeID()); + int branch = selectBranch(iNodeID, node.getNodeID()); assert(branch >= 0); auto inner = static_cast(iNode); if (inner->isEmptyBranch(branch)) @@ -765,7 +764,7 @@ SHAMap::hasInnerNode( while (node->isInner() && (nodeID.getDepth() < targetNodeID.getDepth())) { - int branch = nodeID.selectBranch(targetNodeID.getNodeID()); + int branch = selectBranch(nodeID, targetNodeID.getNodeID()); auto inner = static_cast(node); if (inner->isEmptyBranch(branch)) return false; @@ -790,7 +789,7 @@ SHAMap::hasLeafNode(uint256 const& tag, SHAMapHash const& targetNodeHash) const do { - int branch = nodeID.selectBranch(tag); + int branch = selectBranch(nodeID, tag); auto inner = static_cast(node); if (inner->isEmptyBranch(branch)) return false; // Dead end, node must not be here diff --git a/src/ripple/shamap/impl/SHAMapTreeNode.cpp b/src/ripple/shamap/impl/SHAMapTreeNode.cpp index 147787ae92..6fe6c156a7 100644 --- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp +++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp @@ -491,12 +491,7 @@ SHAMapInnerNode::getBranchCount() const std::string SHAMapAbstractNode::getString(const SHAMapNodeID& id) const { - std::string ret = "NodeID("; - ret += beast::lexicalCastThrow(id.getDepth()); - ret += ","; - ret += to_string(id.getNodeID()); - ret += ")"; - return ret; + return to_string(id); } std::string @@ -507,8 +502,8 @@ SHAMapInnerNode::getString(const SHAMapNodeID& id) const { if (!isEmptyBranch(i)) { - ret += "\nb"; - ret += beast::lexicalCastThrow(i); + ret += "\n"; + ret += std::to_string(i); ret += " = "; ret += to_string(mHashes[i]); } diff --git a/src/test/overlay/compression_test.cpp b/src/test/overlay/compression_test.cpp index 965fc7d6d1..f5f0817a9d 100644 --- a/src/test/overlay/compression_test.cpp +++ b/src/test/overlay/compression_test.cpp @@ -249,7 +249,7 @@ public: uint256 const hash(ripple::sha512Half(123456789)); getLedger->set_ledgerhash(hash.begin(), hash.size()); getLedger->set_ledgerseq(123456789); - ripple::SHAMapNodeID sha(hash.data(), hash.size()); + ripple::SHAMapNodeID sha(17, hash); getLedger->add_nodeids(sha.getRawString()); getLedger->set_requestcookie(123456789); getLedger->set_querytype(protocol::qtINDIRECT); @@ -309,7 +309,7 @@ public: uint256 hash(ripple::sha512Half(i)); auto object = getObject->add_objects(); object->set_hash(hash.data(), hash.size()); - ripple::SHAMapNodeID sha(hash.data(), hash.size()); + ripple::SHAMapNodeID sha(i % 55, hash); object->set_nodeid(sha.getRawString()); object->set_index(""); object->set_data(""); diff --git a/src/test/shamap/FetchPack_test.cpp b/src/test/shamap/FetchPack_test.cpp index 348e59e704..f420b814eb 100644 --- a/src/test/shamap/FetchPack_test.cpp +++ b/src/test/shamap/FetchPack_test.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include