Clang-tidy changes

include/xrpl/protocol/Serializer.h

@@ -24,7 +24,7 @@ private:
     Blob data_;
 
 public:
-    explicit Serializer(int n = 256)
+    explicit Serializer(std::size_t n = 256)
     {
         data_.reserve(n);
     }

include/xrpl/shamap/SHAMapAccountStateLeafNode.h

@@ -45,12 +45,21 @@ public:
         hash_ = SHAMapHash{sha512Half(HashPrefix::LeafNode, item_->slice(), item_->key())};
     }
 
-    void
-    serializeForWire(Serializer& s) const final
+    std::size_t
+    sizeForWire() const final
     {
-        s.addRaw(item_->slice());
-        s.addBitString(item_->key());
-        s.add8(kWireTypeAccountState);
+        return item_->size() + uint256::kBytes + sizeof(std::uint8_t);
+    }
+
+    void
+    serializeForWire(std::uint8_t* out) const final
+    {
+        auto const sz = item_->size();
+        std::memcpy(out, item_->data(), sz);
+        out += sz;
+        std::memcpy(out, item_->key().data(), uint256::kBytes);
+        out += uint256::kBytes;
+        *out = kWireTypeAccountState;
     }
 
     void

include/xrpl/shamap/SHAMapInnerNode.h

@@ -17,6 +17,12 @@ public:
     /** Each inner node has 16 children (the 'radix tree' part of the map) */
     static constexpr unsigned int kBranchFactor = 16;
 
+    /** Branch count below which compressed wire format is used instead of full.
+        Compressed format encodes only populated branches, costing 33 bytes each
+        plus a 1-byte type tag.  Full format always emits all 16 hashes (513
+        bytes). At this threshold, compressed is still smaller than full. */
+    static constexpr unsigned int kCompressedThreshold = 12;
+
 private:
     /** Opaque type that contains the `hashes` array (array of type
        `SHAMapHash`) and the `children` array (array of type
@@ -149,8 +155,11 @@ public:
     void
     updateHashDeep();
 
+    std::size_t
+    sizeForWire() const override;
+
     void
-    serializeForWire(Serializer&) const override;
+    serializeForWire(std::uint8_t* out) const override;
 
     void
     serializeWithPrefix(Serializer&) const override;

include/xrpl/shamap/SHAMapLeafNode.h

@@ -36,6 +36,12 @@ public:
         return false;
     }
 
+    std::size_t
+    sizeForWire() const override = 0;
+
+    void
+    serializeForWire(std::uint8_t* out) const override = 0;
+
     void
     invariants(bool isRoot = false) const final;
 

include/xrpl/shamap/SHAMapTreeNode.h

@@ -142,9 +142,13 @@ public:
     virtual bool
     isInner() const = 0;
 
-    /** Serialize the node in a format appropriate for sending over the wire */
+    /** Returns the exact number of bytes that serializeForWire will emit. */
+    virtual std::size_t
+    sizeForWire() const = 0;
+
+    /** Serialize the node into @p out, which must point to at least sizeForWire() bytes. */
     virtual void
-    serializeForWire(Serializer&) const = 0;
+    serializeForWire(std::uint8_t* out) const = 0;
 
     /** Serialize the node in a format appropriate for hashing */
     virtual void

include/xrpl/shamap/SHAMapTxLeafNode.h

@@ -44,11 +44,19 @@ public:
         hash_ = SHAMapHash{sha512Half(HashPrefix::TransactionId, item_->slice())};
     }
 
-    void
-    serializeForWire(Serializer& s) const final
+    std::size_t
+    sizeForWire() const final
     {
-        s.addRaw(item_->slice());
-        s.add8(kWireTypeTransaction);
+        return item_->size() + sizeof(std::uint8_t);
+    }
+
+    void
+    serializeForWire(std::uint8_t* out) const final
+    {
+        auto const sz = item_->size();
+        std::memcpy(out, item_->data(), sz);
+        out += sz;
+        *out = kWireTypeTransaction;
     }
 
     void

include/xrpl/shamap/SHAMapTxPlusMetaLeafNode.h

@@ -45,12 +45,21 @@ public:
         hash_ = SHAMapHash{sha512Half(HashPrefix::TxNode, item_->slice(), item_->key())};
     }
 
-    void
-    serializeForWire(Serializer& s) const final
+    std::size_t
+    sizeForWire() const final
     {
-        s.addRaw(item_->slice());
-        s.addBitString(item_->key());
-        s.add8(kWireTypeTransactionWithMeta);
+        return item_->size() + uint256::kBytes + sizeof(std::uint8_t);
+    }
+
+    void
+    serializeForWire(std::uint8_t* out) const final
+    {
+        auto const sz = item_->size();
+        std::memcpy(out, item_->data(), sz);
+        out += sz;
+        std::memcpy(out, item_->key().data(), uint256::kBytes);
+        out += uint256::kBytes;
+        *out = kWireTypeTransactionWithMeta;
     }
 
     void

src/libxrpl/shamap/SHAMapInnerNode.cpp

@@ -18,6 +18,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <cstring>
 #include <mutex>
 #include <optional>
 #include <stdexcept>
@@ -219,26 +220,37 @@ SHAMapInnerNode::updateHashDeep()
     updateHash();
 }
 
+std::size_t
+SHAMapInnerNode::sizeForWire() const
+{
+    auto const n = getBranchCount();
+    if (n < kCompressedThreshold)
+        return (n * (uint256::kBytes + sizeof(std::uint8_t))) + sizeof(std::uint8_t);
+    return (kBranchFactor * uint256::kBytes) + sizeof(std::uint8_t);
+}
+
 void
-SHAMapInnerNode::serializeForWire(Serializer& s) const
+SHAMapInnerNode::serializeForWire(std::uint8_t* out) const
 {
     XRPL_ASSERT(!isEmpty(), "xrpl::SHAMapInnerNode::serializeForWire : is non-empty");
 
-    // If the node is sparse, then only send non-empty branches:
-    if (getBranchCount() < 12)
+    if (getBranchCount() < kCompressedThreshold)
     {
-        // compressed node
         auto hashes = hashesAndChildren_.getHashes();
         iterNonEmptyChildIndexes([&](auto branchNum, auto indexNum) {
-            s.addBitString(hashes[indexNum].asUInt256());
-            s.add8(branchNum);
+            std::memcpy(out, hashes[indexNum].asUInt256().data(), uint256::kBytes);
+            out += uint256::kBytes;
+            *out++ = static_cast<std::uint8_t>(branchNum);
         });
-        s.add8(kWireTypeCompressedInner);
+        *out = kWireTypeCompressedInner;
     }
     else
     {
-        iterChildren([&](SHAMapHash const& hh) { s.addBitString(hh.asUInt256()); });
-        s.add8(kWireTypeInner);
+        iterChildren([&](SHAMapHash const& hh) {
+            std::memcpy(out, hh.asUInt256().data(), uint256::kBytes);
+            out += uint256::kBytes;
+        });
+        *out = kWireTypeInner;
     }
 }
 

src/libxrpl/shamap/SHAMapSync.cpp

@@ -449,28 +449,25 @@ SHAMap::getNodeFat(
     std::stack<std::tuple<SHAMapTreeNode*, SHAMapNodeID, int>> stack;
     stack.emplace(node, nodeID, depth);
 
-    Serializer s(8192);
-
     while (!stack.empty())
     {
         std::tie(node, nodeID, depth) = stack.top();
         stack.pop();
 
-        // Add this node to the reply
-        s.erase();
-        node->serializeForWire(s);
-        data.emplace_back(nodeID, s.getData());
+        // Add this node to the reply.
+        Blob nodeData(node->sizeForWire());
+        node->serializeForWire(nodeData.data());
+        data.emplace_back(nodeID, std::move(nodeData));
 
         if (node->isInner())
         {
-            // We descend inner nodes with only a single child
-            // without decrementing the depth
+            // We descend inner nodes with only a single child without decrementing the depth.
             auto inner = safeDowncast<SHAMapInnerNode*>(node);
             int const bc = inner->getBranchCount();
 
             if ((depth > 0) || (bc == 1))
             {
-                // We need to process this node's children
+                // We need to process this node's children.
                 for (int i = 0; i < 16; ++i)
                 {
                     if (!inner->isEmptyBranch(i))
@@ -480,16 +477,16 @@ SHAMap::getNodeFat(
 
                         if (childNode->isInner() && ((depth > 1) || (bc == 1)))
                         {
-                            // If there's more than one child, reduce the depth
-                            // If only one child, follow the chain
+                            // If there's more than one child, reduce the depth. If there's only one
+                            // child, follow the chain.
                             stack.emplace(childNode, childID, (bc > 1) ? (depth - 1) : depth);
                         }
                         else if (childNode->isInner() || fatLeaves)
                         {
-                            // Just include this node
-                            s.erase();
-                            childNode->serializeForWire(s);
-                            data.emplace_back(childID, s.getData());
+                            // Just include this node.
+                            Blob childData(childNode->sizeForWire());
+                            childNode->serializeForWire(childData.data());
+                            data.emplace_back(childID, std::move(childData));
                         }
                     }
                 }
@@ -503,7 +500,10 @@ SHAMap::getNodeFat(
 void
 SHAMap::serializeRoot(Serializer& s) const
 {
-    root_->serializeForWire(s);
+    auto const n = root_->sizeForWire();
+    auto const offset = s.size();
+    s.resize(offset + n);
+    root_->serializeForWire(static_cast<std::uint8_t*>(s.getDataPtr()) + offset);
 }
 
 SHAMapAddNode
@@ -794,9 +794,9 @@ SHAMap::getProofPath(uint256 const& key) const
     path.reserve(stack.size());
     while (!stack.empty())
     {
-        Serializer s;
-        stack.top().first->serializeForWire(s);
-        path.emplace_back(std::move(s.modData()));
+        Blob nodeData(stack.top().first->sizeForWire());
+        stack.top().first->serializeForWire(nodeData.data());
+        path.emplace_back(std::move(nodeData));
         stack.pop();
     }
 

src/tests/libxrpl/CMakeLists.txt

@@ -27,6 +27,7 @@ set(test_modules
     basics
     crypto
     json
+    shamap
     tx
     protocol_autogen
 )

src/tests/libxrpl/shamap/SHAMapNodeSize.cpp

@@ -0,0 +1,120 @@
+#include <xrpl/basics/Blob.h>
+#include <xrpl/basics/IntrusivePointer.h>
+#include <xrpl/basics/IntrusivePointer.ipp>  // IWYU pragma: keep
+#include <xrpl/basics/Slice.h>
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/shamap/SHAMapAccountStateLeafNode.h>
+#include <xrpl/shamap/SHAMapInnerNode.h>
+#include <xrpl/shamap/SHAMapItem.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
+#include <xrpl/shamap/SHAMapTxLeafNode.h>
+#include <xrpl/shamap/SHAMapTxPlusMetaLeafNode.h>
+
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <map>
+#include <vector>
+
+namespace xrpl {
+
+// Build an inner node with exactly n populated branches via the full wire format round-trip so we
+// get a properly initialized SHAMapInnerNode.
+static SHAMapTreeNodePtr
+makeInnerWithBranches(unsigned int n)
+{
+    std::vector<std::uint8_t> buf(SHAMapInnerNode::kBranchFactor * uint256::kBytes, 0);
+    for (unsigned int i = 0; i < n; ++i)
+    {
+        std::uint8_t* p = buf.data() + (i * uint256::kBytes);
+        std::fill(p, p + uint256::kBytes, static_cast<std::uint8_t>(i + 1));
+    }
+
+    return SHAMapInnerNode::makeFullInner(Slice(buf.data(), buf.size()), SHAMapHash{}, false);
+}
+
+// Verifies sizeForWire() for every branch count from 1 to 16, which covers the compressed format
+// (< kCompressedThreshold), the threshold boundary, and the full format (>= kCompressedThreshold).
+TEST(SHAMapNodeSize, InnerNode)
+{
+    // Concrete size spot-checks at key boundary points (manually verified):
+    //   compressed (n=1):  1*(32+1)+1 = 34
+    //   compressed (n=11): 11*(32+1)+1 = 364  (last compressed)
+    //   full (n=12):       16*32+1 = 513       (first full)
+    //   full (n=16):       16*32+1 = 513
+    static std::map<unsigned int, std::size_t> const kSizeCheckpoints = {
+        {1, 34},
+        {11, 364},
+        {12, 513},
+        {16, 513},
+    };
+
+    for (unsigned int n = 1; n <= SHAMapInnerNode::kBranchFactor; ++n)
+    {
+        auto const node = makeInnerWithBranches(n);
+        auto const sz = node->sizeForWire();
+
+        if (auto it = kSizeCheckpoints.find(n); it != kSizeCheckpoints.end())
+        {
+            EXPECT_EQ(sz, it->second) << "branch count: " << n;
+        }
+
+        // Sentinel byte catches overruns in serializeForWire() independently of sizeForWire().
+        Blob buf(sz + 1, 0xCD);
+        node->serializeForWire(buf.data());
+        EXPECT_EQ(buf[sz], 0xCD) << "branch count: " << n;
+        EXPECT_EQ(
+            buf[sz - 1],
+            (n < SHAMapInnerNode::kCompressedThreshold) ? kWireTypeCompressedInner : kWireTypeInner)
+            << "branch count: " << n;
+
+        if (n < SHAMapInnerNode::kCompressedThreshold)
+        {
+            // makeInnerWithBranches populates branches 0..n-1 in order. Each compressed entry is
+            // [32-byte hash][1-byte branch index], so verify each branch index byte.
+            for (unsigned int i = 0; i < n; ++i)
+            {
+                EXPECT_EQ(buf[(i * (uint256::kBytes + 1)) + uint256::kBytes], i)
+                    << "branch count: " << n;
+            }
+        }
+    }
+}
+
+// Verifies sizeForWire() for each leaf node type across a range of payload sizes.
+TEST(SHAMapNodeSize, LeafNodes)
+{
+    for (std::size_t const payloadSize : {12u, 64u, 256u})
+    {
+        uint256 const key{};
+        std::vector<std::uint8_t> const payload(payloadSize, 0xAB);
+        Slice const data(payload.data(), payload.size());
+        auto item = makeShamapitem(key, data);
+
+        auto const checkNode = [&](auto const& node, std::size_t expectedSize, std::uint8_t tag) {
+            auto const sz = node->sizeForWire();
+            EXPECT_EQ(sz, expectedSize) << "payload: " << payloadSize;
+            Blob buf(sz + 1, 0xCD);
+            node->serializeForWire(buf.data());
+            EXPECT_EQ(buf[sz], 0xCD) << "payload: " << payloadSize;
+            EXPECT_EQ(buf[sz - 1], tag) << "payload: " << payloadSize;
+        };
+
+        checkNode(
+            intr_ptr::makeShared<SHAMapTxLeafNode>(item, 1),
+            payloadSize + sizeof(std::uint8_t),
+            kWireTypeTransaction);
+        checkNode(
+            intr_ptr::makeShared<SHAMapTxPlusMetaLeafNode>(item, 1),
+            payloadSize + uint256::kBytes + sizeof(std::uint8_t),
+            kWireTypeTransactionWithMeta);
+        checkNode(
+            intr_ptr::makeShared<SHAMapAccountStateLeafNode>(item, 1),
+            payloadSize + uint256::kBytes + sizeof(std::uint8_t),
+            kWireTypeAccountState);
+    }
+}
+
+}  // namespace xrpl