Merge 9908ab0581 into 66aa6a47bd

7c123c08dd  miner: add package feerate vector to CBlockTemplate (ismaelsadeeq)

Pull request description:

  This PR enables `BlockAssembler` to add all selected packages' fee and virtual size to a vector, and then return the vector as a member of `CBlockTemplate` struct.

  This PR is the first step in the https://github.com/bitcoin/bitcoin/issues/30392 project.

  The packages' vsize and fee are used in #30157 to select a percentile fee rate of the top block in the mempool.

ACKs for top commit:
  rkrux:
    tACK 7c123c08dd
  ryanofsky:
    Code review ACK 7c123c08dd. Changes since last review are rebasing due to a test conflict, giving the new field a better name and description, resolving the test conflict, and renaming a lot of test variables. The actual code change is still one-line change.
  glozow:
    reACK 7c123c08dd

Tree-SHA512: 767b0b3d4273cf1589fd2068d729a66c7414c0f9574b15989fbe293f8c85cd6c641dd783cde55bfabab32cd047d7d8a071d6897b06ed4295c0d071e588de0861

src/crypto/chacha20.cpp

@@ -59,7 +59,7 @@ void ChaCha20Aligned::Seek(Nonce96 nonce, uint32_t block_counter) noexcept
 
 inline void ChaCha20Aligned::Keystream(Span<std::byte> output) noexcept
 {
-    unsigned char* c = UCharCast(output.data());
+    std::byte* c = output.data();
     size_t blocks = output.size() / BLOCKLEN;
     assert(blocks * BLOCKLEN == output.size());
 
@@ -161,8 +161,8 @@ inline void ChaCha20Aligned::Keystream(Span<std::byte> output) noexcept
 inline void ChaCha20Aligned::Crypt(Span<const std::byte> in_bytes, Span<std::byte> out_bytes) noexcept
 {
     assert(in_bytes.size() == out_bytes.size());
-    const unsigned char* m = UCharCast(in_bytes.data());
-    unsigned char* c = UCharCast(out_bytes.data());
+    const std::byte* m = in_bytes.data();
+    std::byte* c = out_bytes.data();
     size_t blocks = out_bytes.size() / BLOCKLEN;
     assert(blocks * BLOCKLEN == out_bytes.size());
 

src/crypto/chacha20poly1305.cpp

@@ -56,8 +56,8 @@ void ComputeTag(ChaCha20& chacha20, Span<const std::byte> aad, Span<const std::b
     poly1305.Update(cipher).Update(Span{PADDING}.first(cipher_padding_length));
     // - Process the AAD and plaintext length with Poly1305.
     std::byte length_desc[Poly1305::TAGLEN];
-    WriteLE64(UCharCast(length_desc), aad.size());
-    WriteLE64(UCharCast(length_desc + 8), cipher.size());
+    WriteLE64(length_desc, aad.size());
+    WriteLE64(length_desc + 8, cipher.size());
     poly1305.Update(length_desc);
 
     // Output tag.

src/crypto/common.h

@@ -13,96 +13,84 @@
 #include <cstring>
 
 template <typename B>
-concept ByteType = std::same_as<B, unsigned char> || std::same_as<B, std::byte>;
+concept ByteType = std::same_as<B, uint8_t> || std::same_as<B, std::byte>;
 
-template <ByteType B>
-inline uint16_t ReadLE16(const B* ptr)
+uint16_t ReadLE16(const ByteType auto* ptr) noexcept
 {
     uint16_t x;
-    memcpy(&x, ptr, 2);
+    std::memcpy(&x, ptr, sizeof(x));
     return le16toh_internal(x);
 }
 
-template <ByteType B>
-inline uint32_t ReadLE32(const B* ptr)
+uint32_t ReadLE32(const ByteType auto* ptr) noexcept
 {
     uint32_t x;
-    memcpy(&x, ptr, 4);
+    std::memcpy(&x, ptr, sizeof(x));
     return le32toh_internal(x);
 }
 
-template <ByteType B>
-inline uint64_t ReadLE64(const B* ptr)
+uint64_t ReadLE64(const ByteType auto* ptr) noexcept
 {
     uint64_t x;
-    memcpy(&x, ptr, 8);
+    std::memcpy(&x, ptr, sizeof(x));
     return le64toh_internal(x);
 }
 
-template <ByteType B>
-inline void WriteLE16(B* ptr, uint16_t x)
+void WriteLE16(ByteType auto* ptr, const uint16_t x) noexcept
 {
-    uint16_t v = htole16_internal(x);
-    memcpy(ptr, &v, 2);
+    const uint16_t v{htole16_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
-template <ByteType B>
-inline void WriteLE32(B* ptr, uint32_t x)
+void WriteLE32(ByteType auto* ptr, const uint32_t x) noexcept
 {
-    uint32_t v = htole32_internal(x);
-    memcpy(ptr, &v, 4);
+    const uint32_t v{htole32_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
-template <ByteType B>
-inline void WriteLE64(B* ptr, uint64_t x)
+void WriteLE64(ByteType auto* ptr, const uint64_t x) noexcept
 {
-    uint64_t v = htole64_internal(x);
-    memcpy(ptr, &v, 8);
+    const uint64_t v{htole64_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
-template <ByteType B>
-inline uint16_t ReadBE16(const B* ptr)
+uint16_t ReadBE16(const ByteType auto* ptr) noexcept
 {
     uint16_t x;
-    memcpy(&x, ptr, 2);
+    std::memcpy(&x, ptr, sizeof(x));
     return be16toh_internal(x);
 }
 
-template <ByteType B>
-inline uint32_t ReadBE32(const B* ptr)
+uint32_t ReadBE32(const ByteType auto* ptr) noexcept
 {
     uint32_t x;
-    memcpy(&x, ptr, 4);
+    std::memcpy(&x, ptr, sizeof(x));
     return be32toh_internal(x);
 }
 
-template <ByteType B>
-inline uint64_t ReadBE64(const B* ptr)
+uint64_t ReadBE64(const ByteType auto* ptr) noexcept
 {
     uint64_t x;
-    memcpy(&x, ptr, 8);
+    std::memcpy(&x, ptr, sizeof(x));
     return be64toh_internal(x);
 }
 
-template <ByteType B>
-inline void WriteBE16(B* ptr, uint16_t x)
+void WriteBE16(ByteType auto* ptr, const uint16_t x) noexcept
 {
-    uint16_t v = htobe16_internal(x);
-    memcpy(ptr, &v, 2);
+    const uint16_t v{htobe16_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
-template <ByteType B>
-inline void WriteBE32(B* ptr, uint32_t x)
+void WriteBE32(ByteType auto* ptr, const uint32_t x) noexcept
 {
-    uint32_t v = htobe32_internal(x);
-    memcpy(ptr, &v, 4);
+    const uint32_t v{htobe32_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
-template <ByteType B>
-inline void WriteBE64(B* ptr, uint64_t x)
+void WriteBE64(ByteType auto* ptr, const uint64_t x) noexcept
 {
-    uint64_t v = htobe64_internal(x);
-    memcpy(ptr, &v, 8);
+    const uint64_t v{htobe64_internal(x)};
+    std::memcpy(ptr, &v, sizeof(v));
 }
 
 #endif // BITCOIN_CRYPTO_COMMON_H

src/node/miner.cpp

@@ -421,6 +421,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
         }
 
         ++nPackagesSelected;
+        pblocktemplate->m_package_feerates.emplace_back(packageFees, static_cast<int32_t>(packageSize));
 
         // Update transactions that depend on each of these
         nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);

src/node/miner.h

@@ -10,6 +10,7 @@
 #include <policy/policy.h>
 #include <primitives/block.h>
 #include <txmempool.h>
+#include <util/feefrac.h>
 
 #include <memory>
 #include <optional>
@@ -39,6 +40,9 @@ struct CBlockTemplate
     std::vector<CAmount> vTxFees;
     std::vector<int64_t> vTxSigOpsCost;
     std::vector<unsigned char> vchCoinbaseCommitment;
+    /* A vector of package fee rates, ordered by the sequence in which
+     * packages are selected for inclusion in the block template.*/
+    std::vector<FeeFrac> m_package_feerates;
 };
 
 // Container for tracking updates to ancestor feerate as we include (parent)

src/random.h

@@ -268,12 +268,12 @@ public:
     {
         while (span.size() >= 8) {
             uint64_t gen = Impl().rand64();
-            WriteLE64(UCharCast(span.data()), gen);
+            WriteLE64(span.data(), gen);
             span = span.subspan(8);
         }
         if (span.size() >= 4) {
             uint32_t gen = Impl().rand32();
-            WriteLE32(UCharCast(span.data()), gen);
+            WriteLE32(span.data(), gen);
             span = span.subspan(4);
         }
         while (span.size()) {
@@ -397,7 +397,7 @@ public:
         if (requires_seed) RandomSeed();
         std::array<std::byte, 8> buf;
         rng.Keystream(buf);
-        return ReadLE64(UCharCast(buf.data()));
+        return ReadLE64(buf.data());
     }
 
     /** Fill a byte Span with random bytes. This overrides the RandomMixin version. */

src/test/miner_tests.cpp

@@ -16,6 +16,7 @@
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/check.h>
+#include <util/feefrac.h>
 #include <util/strencodings.h>
 #include <util/time.h>
 #include <util/translation.h>
@@ -25,6 +26,7 @@
 #include <test/util/setup_common.h>
 
 #include <memory>
+#include <vector>
 
 #include <boost/test/unit_test.hpp>
 
@@ -123,19 +125,22 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 1000;
     // This tx has a low fee: 1000 satoshis
     Txid hashParentTx = tx.GetHash(); // save this txid for later use
-    AddToMempool(tx_mempool, entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
+    const auto parent_tx{entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
+    AddToMempool(tx_mempool, parent_tx);
 
     // This tx has a medium fee: 10000 satoshis
     tx.vin[0].prevout.hash = txFirst[1]->GetHash();
     tx.vout[0].nValue = 5000000000LL - 10000;
     Txid hashMediumFeeTx = tx.GetHash();
-    AddToMempool(tx_mempool, entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
+    const auto medium_fee_tx{entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
+    AddToMempool(tx_mempool, medium_fee_tx);
 
     // This tx has a high fee, but depends on the first transaction
     tx.vin[0].prevout.hash = hashParentTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee
     Txid hashHighFeeTx = tx.GetHash();
-    AddToMempool(tx_mempool, entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
+    const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
+    AddToMempool(tx_mempool, high_fee_tx);
 
     std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
     BOOST_REQUIRE(block_template);
@@ -145,6 +150,21 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
     BOOST_CHECK(block.vtx[3]->GetHash() == hashMediumFeeTx);
 
+    // Test the inclusion of package feerates in the block template and ensure they are sequential.
+    const auto block_package_feerates = BlockAssembler{m_node.chainman->ActiveChainstate(), &tx_mempool, options}.CreateNewBlock()->m_package_feerates;
+    BOOST_CHECK(block_package_feerates.size() == 2);
+
+    // parent_tx and high_fee_tx are added to the block as a package.
+    const auto combined_txs_fee = parent_tx.GetFee() + high_fee_tx.GetFee();
+    const auto combined_txs_size = parent_tx.GetTxSize() + high_fee_tx.GetTxSize();
+    FeeFrac package_feefrac{combined_txs_fee, combined_txs_size};
+    // The package should be added first.
+    BOOST_CHECK(block_package_feerates[0] == package_feefrac);
+
+    // The medium_fee_tx should be added next.
+    FeeFrac medium_tx_feefrac{medium_fee_tx.GetFee(), medium_fee_tx.GetTxSize()};
+    BOOST_CHECK(block_package_feerates[1] == medium_tx_feefrac);
+
     // Test that a package below the block min tx fee doesn't get included
     tx.vin[0].prevout.hash = hashHighFeeTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee

src/wallet/migrate.cpp

@@ -603,7 +603,7 @@ void BerkeleyRODatabase::Open()
 
     // Read subdatabase page number
     // It is written as a big endian 32 bit number
-    uint32_t main_db_page = ReadBE32(UCharCast(std::get<DataRecord>(page.records.at(1)).data.data()));
+    uint32_t main_db_page = ReadBE32(std::get<DataRecord>(page.records.at(1)).data.data());
 
     // The main database is in a page that doesn't exist
     if (main_db_page > outer_meta.last_page) {