diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index 91bb735086..98a6cc895d 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -254,7 +254,7 @@ public: return; s.erase(); - newNode->addRaw(s, snfPREFIX); + newNode->serializeWithPrefix(s); app_.getLedgerMaster().addFetchPack( newNode->getNodeHash().as_uint256(), diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 39e2691726..ae373801a3 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -2734,24 +2734,21 @@ PeerImp::getLedger(std::shared_ptr const& m) { // return account state root node if possible Serializer rootNode(768); - if (stateMap.getRootNode(rootNode, snfWIRE)) + + stateMap.serializeRoot(rootNode); + reply.add_nodes()->set_nodedata( + rootNode.getDataPtr(), rootNode.getLength()); + + if (ledger->info().txHash != beast::zero) { - reply.add_nodes()->set_nodedata( - rootNode.getDataPtr(), rootNode.getLength()); - - if (ledger->info().txHash != beast::zero) + auto const& txMap = ledger->txMap(); + if (txMap.getHash() != beast::zero) { - auto const& txMap = ledger->txMap(); + rootNode.erase(); - if (txMap.getHash() != beast::zero) - { - rootNode.erase(); - - if (txMap.getRootNode(rootNode, snfWIRE)) - reply.add_nodes()->set_nodedata( - rootNode.getDataPtr(), - rootNode.getLength()); - } + txMap.serializeRoot(rootNode); + reply.add_nodes()->set_nodedata( + rootNode.getDataPtr(), rootNode.getLength()); } } } diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index 5dff587964..27946b2150 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -256,8 +256,10 @@ public: bool fatLeaves, std::uint32_t depth) const; - bool - getRootNode(Serializer& s, SHANodeFormat format) const; + /** Serializes the root in a format appropriate for sending over the wire */ + void + serializeRoot(Serializer& s) const; + std::vector getNeededHashes(int max, SHAMapSyncFilter* filter); SHAMapAddNode diff --git a/src/ripple/shamap/SHAMapTreeNode.h b/src/ripple/shamap/SHAMapTreeNode.h index f453d7cd86..3d3668facc 100644 --- a/src/ripple/shamap/SHAMapTreeNode.h +++ b/src/ripple/shamap/SHAMapTreeNode.h @@ -32,12 +32,6 @@ namespace ripple { -enum SHANodeFormat { - snfPREFIX = 1, // Form that hashes to its official hash - snfWIRE = 2, // Compressed form used on the wire - snfHASH = 3, // just the hash -}; - // A SHAMapHash is the hash of a node in a SHAMap, and also the // type of the hash of the entire SHAMap. class SHAMapHash @@ -123,7 +117,6 @@ class SHAMapAbstractNode { public: enum TNType { - tnERROR = 0, tnINNER = 1, tnTRANSACTION_NM = 2, // transaction, no metadata tnTRANSACTION_MD = 3, // transaction, with metadata @@ -158,14 +151,19 @@ public: bool isInner() const; bool - isValid() const; - bool isInBounds(SHAMapNodeID const& id) const; virtual bool updateHash() = 0; + + /** Serialize the node in a format appropriate for sending over the wire */ virtual void - addRaw(Serializer&, SHANodeFormat format) const = 0; + serializeForWire(Serializer&) const = 0; + + /** Serialize the node in a format appropriate for hashing */ + virtual void + serializeWithPrefix(Serializer&) const = 0; + virtual std::string getString(SHAMapNodeID const&) const; virtual std::shared_ptr @@ -248,8 +246,13 @@ public: updateHash() override; void updateHashDeep(); + void - addRaw(Serializer&, SHANodeFormat format) const override; + serializeForWire(Serializer&) const override; + + void + serializeWithPrefix(Serializer&) const override; + std::string getString(SHAMapNodeID const&) const override; uint256 const& @@ -293,7 +296,11 @@ public: clone(std::uint32_t seq) const override; void - addRaw(Serializer&, SHANodeFormat format) const override; + serializeForWire(Serializer&) const override; + + void + serializeWithPrefix(Serializer&) const override; + uint256 const& key() const override; void @@ -366,12 +373,6 @@ SHAMapAbstractNode::isInner() const return mType == tnINNER; } -inline bool -SHAMapAbstractNode::isValid() const -{ - return mType != tnERROR; -} - inline bool SHAMapAbstractNode::isInBounds(SHAMapNodeID const& id) const { diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index db1a55480c..92336965fe 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -406,14 +406,12 @@ std::shared_ptr SHAMap::unshareNode(std::shared_ptr node, SHAMapNodeID const& nodeID) { // make sure the node is suitable for the intended operation (copy on write) - assert(node->isValid()); assert(node->getSeq() <= seq_); if (node->getSeq() != seq_) { // have a CoW assert(state_ != SHAMapState::Immutable); node = std::static_pointer_cast(node->clone(seq_)); - assert(node->isValid()); if (nodeID.isRoot()) root_ = node; } @@ -772,13 +770,13 @@ SHAMap::addGiveItem( std::shared_ptr newNode = std::make_shared(std::move(item), type, seq_); - assert(newNode->isValid() && newNode->isLeaf()); + assert(newNode->isLeaf()); auto inner = std::static_pointer_cast(node); inner->setChild(b1, newNode); newNode = std::make_shared(std::move(otherItem), type, seq_); - assert(newNode->isValid() && newNode->isLeaf()); + assert(newNode->isLeaf()); inner->setChild(b2, newNode); } @@ -907,7 +905,7 @@ SHAMap::writeNode( canonicalize(node->getNodeHash(), node); Serializer s; - node->addRaw(s, snfPREFIX); + node->serializeWithPrefix(s); f_.db().store( t, std::move(s.modData()), diff --git a/src/ripple/shamap/impl/SHAMapSync.cpp b/src/ripple/shamap/impl/SHAMapSync.cpp index 35d4f77ec1..85022621ed 100644 --- a/src/ripple/shamap/impl/SHAMapSync.cpp +++ b/src/ripple/shamap/impl/SHAMapSync.cpp @@ -39,9 +39,6 @@ void SHAMap::visitNodes( std::function const& function) const { - // Visit every node in a SHAMap - assert(root_->isValid()); - if (!root_) return; @@ -108,8 +105,6 @@ SHAMap::visitDifferences( { // Visit every node in this SHAMap that is not present // in the specified SHAMap - assert(root_->isValid()); - if (!root_) return; @@ -320,7 +315,6 @@ SHAMap::gmn_ProcessDeferredReads(MissingNodes& mn) std::vector> SHAMap::getMissingNodes(int max, SHAMapSyncFilter* filter) { - assert(root_->isValid()); assert(root_->getNodeHash().isNonZero()); assert(max > 0); @@ -493,7 +487,7 @@ SHAMap::getNodeFat( { // Add this node to the reply Serializer s; - node->addRaw(s, snfWIRE); + node->serializeForWire(s); nodeIDs.push_back(nodeID); rawNodes.push_back(std::move(s.modData())); } @@ -527,7 +521,7 @@ SHAMap::getNodeFat( { // Just include this node Serializer ns; - childNode->addRaw(ns, snfWIRE); + childNode->serializeForWire(ns); nodeIDs.push_back(childID); rawNodes.push_back(std::move(ns.modData())); } @@ -540,11 +534,10 @@ SHAMap::getNodeFat( return true; } -bool -SHAMap::getRootNode(Serializer& s, SHANodeFormat format) const +void +SHAMap::serializeRoot(Serializer& s) const { - root_->addRaw(s, format); - return true; + root_->serializeForWire(s); } SHAMapAddNode @@ -563,7 +556,7 @@ SHAMap::addRootNode( assert(seq_ >= 1); auto node = SHAMapAbstractNode::makeFromWire(rootNode); - if (!node || !node->isValid() || node->getNodeHash() != hash) + if (!node || node->getNodeHash() != hash) return SHAMapAddNode::invalid(); if (backed_) @@ -577,7 +570,7 @@ SHAMap::addRootNode( if (filter) { Serializer s; - root_->addRaw(s, snfPREFIX); + root_->serializeWithPrefix(s); filter->gotNode( false, root_->getNodeHash(), @@ -633,8 +626,7 @@ SHAMap::addKnownNode( if (iNode == nullptr) { - if (!newNode || !newNode->isValid() || - childHash != newNode->getNodeHash()) + if (!newNode || childHash != newNode->getNodeHash()) { JLOG(journal_.warn()) << "Corrupt node received"; return SHAMapAddNode::invalid(); @@ -665,7 +657,7 @@ SHAMap::addKnownNode( if (filter) { Serializer s; - newNode->addRaw(s, snfPREFIX); + newNode->serializeWithPrefix(s); filter->gotNode( false, childHash, @@ -827,7 +819,7 @@ SHAMap::getFetchPack( if (includeLeaves || smn.isInner()) { Serializer s; - smn.addRaw(s, snfPREFIX); + smn.serializeWithPrefix(s); func(smn.getNodeHash(), s.peekData()); if (--max <= 0) diff --git a/src/ripple/shamap/impl/SHAMapTreeNode.cpp b/src/ripple/shamap/impl/SHAMapTreeNode.cpp index 6fe6c156a7..acc268ac2a 100644 --- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp +++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp @@ -31,6 +31,14 @@ namespace ripple { +// These are wire-protocol identifiers used during serialization to encode the +// type of a node. They should not be arbitrarily be changed. +static constexpr unsigned char const wireTypeTransaction = 0; +static constexpr unsigned char const wireTypeAccountState = 1; +static constexpr unsigned char const wireTypeInner = 2; +static constexpr unsigned char const wireTypeCompressedInner = 3; +static constexpr unsigned char const wireTypeTransactionWithMeta = 4; + std::mutex SHAMapInnerNode::childLock; SHAMapAbstractNode::~SHAMapAbstractNode() = default; @@ -235,19 +243,19 @@ SHAMapAbstractNode::makeFromWire(Slice rawNode) std::uint32_t const seq = 0; - if (type == 0) + if (type == wireTypeTransaction) return makeTransaction(rawNode, seq, hash, hashValid); - if (type == 1) + if (type == wireTypeAccountState) return makeAccountState(rawNode, seq, hash, hashValid); - if (type == 2) + if (type == wireTypeInner) return SHAMapInnerNode::makeFullInner(rawNode, seq, hash, hashValid); - if (type == 3) + if (type == wireTypeCompressedInner) return SHAMapInnerNode::makeCompressedInner(rawNode, seq); - if (type == 4) + if (type == wireTypeTransactionWithMeta) return makeTransactionWithMeta(rawNode, seq, hash, hashValid); Throw( @@ -351,112 +359,88 @@ SHAMapTreeNode::updateHash() } void -SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format) const +SHAMapInnerNode::serializeForWire(Serializer& s) const { - assert((format == snfPREFIX) || (format == snfWIRE) || (format == snfHASH)); + assert(mType == tnINNER); + assert(!isEmpty()); - if (mType == tnERROR) - Throw("invalid I node type"); - - if (format == snfHASH) + // If the node is sparse, then only send non-empty branches: + if (getBranchCount() < 12) { - s.addBitString(mHash.as_uint256()); - } - else if (mType == tnINNER) - { - assert(!isEmpty()); - - if (format == snfPREFIX) + // compressed node + for (int i = 0; i < mHashes.size(); ++i) { - s.add32(HashPrefix::innerNode); - - for (auto const& hh : mHashes) - s.addBitString(hh.as_uint256()); - } - else // format == snfWIRE - { - if (getBranchCount() < 12) + if (!isEmptyBranch(i)) { - // compressed node - for (int i = 0; i < mHashes.size(); ++i) - if (!isEmptyBranch(i)) - { - s.addBitString(mHashes[i].as_uint256()); - s.add8(i); - } - - s.add8(3); - } - else - { - for (auto const& hh : mHashes) - s.addBitString(hh.as_uint256()); - - s.add8(2); + s.addBitString(mHashes[i].as_uint256()); + s.add8(i); } } + + s.add8(wireTypeCompressedInner); } else - assert(false); + { + for (auto const& hh : mHashes) + s.addBitString(hh.as_uint256()); + + s.add8(wireTypeInner); + } } void -SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format) const +SHAMapInnerNode::serializeWithPrefix(Serializer& s) const { - assert((format == snfPREFIX) || (format == snfWIRE) || (format == snfHASH)); + assert(mType == tnINNER); + assert(!isEmpty()); - if (mType == tnERROR) - Throw("invalid I node type"); + s.add32(HashPrefix::innerNode); + for (auto const& hh : mHashes) + s.addBitString(hh.as_uint256()); +} - if (format == snfHASH) +void +SHAMapTreeNode::serializeForWire(Serializer& s) const +{ + if (mType == tnACCOUNT_STATE) { - s.addBitString(mHash.as_uint256()); - } - else if (mType == tnACCOUNT_STATE) - { - if (format == snfPREFIX) - { - s.add32(HashPrefix::leafNode); - s.addRaw(mItem->peekData()); - s.addBitString(mItem->key()); - } - else - { - s.addRaw(mItem->peekData()); - s.addBitString(mItem->key()); - s.add8(1); - } + s.addRaw(mItem->peekData()); + s.addBitString(mItem->key()); + s.add8(wireTypeAccountState); } else if (mType == tnTRANSACTION_NM) { - if (format == snfPREFIX) - { - s.add32(HashPrefix::transactionID); - s.addRaw(mItem->peekData()); - } - else - { - s.addRaw(mItem->peekData()); - s.add8(0); - } + s.addRaw(mItem->peekData()); + s.add8(wireTypeTransaction); } else if (mType == tnTRANSACTION_MD) { - if (format == snfPREFIX) - { - s.add32(HashPrefix::txNode); - s.addRaw(mItem->peekData()); - s.addBitString(mItem->key()); - } - else - { - s.addRaw(mItem->peekData()); - s.addBitString(mItem->key()); - s.add8(4); - } + s.addRaw(mItem->peekData()); + s.addBitString(mItem->key()); + s.add8(wireTypeTransactionWithMeta); + } +} + +void +SHAMapTreeNode::serializeWithPrefix(Serializer& s) const +{ + if (mType == tnACCOUNT_STATE) + { + s.add32(HashPrefix::leafNode); + s.addRaw(mItem->peekData()); + s.addBitString(mItem->key()); + } + else if (mType == tnTRANSACTION_NM) + { + s.add32(HashPrefix::transactionID); + s.addRaw(mItem->peekData()); + } + else if (mType == tnTRANSACTION_MD) + { + s.add32(HashPrefix::txNode); + s.addRaw(mItem->peekData()); + s.addBitString(mItem->key()); } - else - assert(false); } bool diff --git a/src/test/nodestore/DatabaseShard_test.cpp b/src/test/nodestore/DatabaseShard_test.cpp index c53568e6d8..034db7c7a2 100644 --- a/src/test/nodestore/DatabaseShard_test.cpp +++ b/src/test/nodestore/DatabaseShard_test.cpp @@ -285,7 +285,7 @@ class DatabaseShard_test : public TestBase // Store the state map auto visitAcc = [&](SHAMapAbstractNode& node) { Serializer s; - node.addRaw(s, snfPREFIX); + node.serializeWithPrefix(s); db.store( node.getType() == SHAMapAbstractNode::TNType::tnINNER ? hotUNKNOWN @@ -313,7 +313,7 @@ class DatabaseShard_test : public TestBase // Store the transaction map auto visitTx = [&](SHAMapAbstractNode& node) { Serializer s; - node.addRaw(s, snfPREFIX); + node.serializeWithPrefix(s); db.store( node.getType() == SHAMapAbstractNode::TNType::tnINNER ? hotUNKNOWN @@ -359,7 +359,7 @@ class DatabaseShard_test : public TestBase // walk shamap and validate each node auto fcompAcc = [&](SHAMapAbstractNode& node) -> bool { Serializer s; - node.addRaw(s, snfPREFIX); + node.serializeWithPrefix(s); auto nSrc{NodeObject::createObject( node.getType() == SHAMapAbstractNode::TNType::tnINNER ? hotUNKNOWN @@ -383,7 +383,7 @@ class DatabaseShard_test : public TestBase auto fcompTx = [&](SHAMapAbstractNode& node) -> bool { Serializer s; - node.addRaw(s, snfPREFIX); + node.serializeWithPrefix(s); auto nSrc{NodeObject::createObject( node.getType() == SHAMapAbstractNode::TNType::tnINNER ? hotUNKNOWN