Merge c0045e6cee 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

ci/test/00_setup_env_native_msan.sh

@@ -27,4 +27,3 @@ export BITCOIN_CONFIG="\
  -DAPPEND_CPPFLAGS='-U_FORTIFY_SOURCE' \
 "
 export USE_MEMORY_SANITIZER="true"
-export RUN_FUNCTIONAL_TESTS="false"

ci/test/01_base_install.sh

@@ -49,7 +49,7 @@ if [ -n "$PIP_PACKAGES" ]; then
 fi
 
 if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then
-  ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-19.1.0" /msan/llvm-project
+  ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-19.1.6" /msan/llvm-project
 
   cmake -G Ninja -B /msan/clang_build/ \
     -DLLVM_ENABLE_PROJECTS="clang" \

cmake/script/GenerateHeaderFromJson.cmake

@@ -6,7 +6,7 @@ cmake_path(GET JSON_SOURCE_PATH STEM json_source_basename)
 
 file(READ ${JSON_SOURCE_PATH} hex_content HEX)
 string(REGEX REPLACE "................" "\\0\n" formatted_bytes "${hex_content}")
-string(REGEX REPLACE "[^\n][^\n]" "0x\\0, " formatted_bytes "${formatted_bytes}")
+string(REGEX REPLACE "[^\n][^\n]" "'\\\\x\\0'," formatted_bytes "${formatted_bytes}")
 
 set(header_content
 "#include <string_view>

cmake/script/GenerateHeaderFromRaw.cmake

@@ -6,7 +6,7 @@ cmake_path(GET RAW_SOURCE_PATH STEM raw_source_basename)
 
 file(READ ${RAW_SOURCE_PATH} hex_content HEX)
 string(REGEX REPLACE "................" "\\0\n" formatted_bytes "${hex_content}")
-string(REGEX REPLACE "[^\n][^\n]" "std::byte{0x\\0}, " formatted_bytes "${formatted_bytes}")
+string(REGEX REPLACE "[^\n][^\n]" "std::byte{0x\\0}," formatted_bytes "${formatted_bytes}")
 
 set(header_content
 "#include <cstddef>

depends/packages/native_capnp.mk

@@ -1,9 +1,9 @@
 package=native_capnp
-$(package)_version=1.0.2
+$(package)_version=1.1.0
 $(package)_download_path=https://capnproto.org/
 $(package)_download_file=capnproto-c++-$($(package)_version).tar.gz
 $(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz
-$(package)_sha256_hash=9057dbc0223366b74bbeca33a05de164a229b0377927f1b7ef3828cdd8cb1d7e
+$(package)_sha256_hash=07167580e563f5e821e3b2af1c238c16ec7181612650c5901330fa9a0da50939
 
 define $(package)_set_vars
   $(package)_config_opts := -DBUILD_TESTING=OFF

doc/build-freebsd.md

@@ -96,7 +96,7 @@ There is an included test suite that is useful for testing code changes when dev
 To run the test suite (recommended), you will need to have Python 3 installed:
 
 ```bash
-pkg install python3 databases/py-sqlite3
+pkg install python3 databases/py-sqlite3 net/py-pyzmq
 ```
 ---
 

doc/build-netbsd.md

@@ -1,6 +1,6 @@
 # NetBSD Build Guide
 
-**Updated for NetBSD [10.0](https://netbsd.org/releases/formal-10/NetBSD-10.0.html)**
+**Updated for NetBSD [10.1](https://netbsd.org/releases/formal-10/NetBSD-10.1.html)**
 
 This guide describes how to build bitcoind, command-line utilities, and GUI on NetBSD.
 
@@ -83,6 +83,13 @@ pkgin install qrencode
 
 Otherwise, if you don't need QR encoding support, use the `-DWITH_QRENCODE=OFF` option to disable this feature in order to compile the GUI.
 
+#### Notifications
+###### ZeroMQ
+
+Bitcoin Core can provide notifications via ZeroMQ. If the package is installed, support will be compiled in.
+```bash
+pkgin zeromq
+```
 
 #### Test Suite Dependencies
 
@@ -90,10 +97,10 @@ There is an included test suite that is useful for testing code changes when dev
 To run the test suite (recommended), you will need to have Python 3 installed:
 
 ```bash
-pkgin install python39
+pkgin install python310 py310-zmq
 ```
 
-### Building Bitcoin Core
+## Building Bitcoin Core
 
 ### 1. Configuration
 

doc/release-notes-28121.md

@@ -0,0 +1,2 @@
+The RPC `testmempoolaccept` response now includes a "reject-details" field in some cases,
+similar to the complete error messages returned by `sendrawtransaction` (#28121)

src/arith_uint256.h

@@ -26,6 +26,7 @@ class base_uint
 protected:
     static_assert(BITS / 32 > 0 && BITS % 32 == 0, "Template parameter BITS must be a positive multiple of 32.");
     static constexpr int WIDTH = BITS / 32;
+    /** Big integer represented with 32-bit digits, least-significant first. */
     uint32_t pn[WIDTH];
 public:
 

src/crypto/chacha20.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2017-2022 The Bitcoin Core developers
+// Copyright (c) 2017-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -25,14 +25,14 @@
 void ChaCha20Aligned::SetKey(Span<const std::byte> key) noexcept
 {
     assert(key.size() == KEYLEN);
-    input[0] = ReadLE32(UCharCast(key.data() + 0));
-    input[1] = ReadLE32(UCharCast(key.data() + 4));
-    input[2] = ReadLE32(UCharCast(key.data() + 8));
-    input[3] = ReadLE32(UCharCast(key.data() + 12));
-    input[4] = ReadLE32(UCharCast(key.data() + 16));
-    input[5] = ReadLE32(UCharCast(key.data() + 20));
-    input[6] = ReadLE32(UCharCast(key.data() + 24));
-    input[7] = ReadLE32(UCharCast(key.data() + 28));
+    input[0] = ReadLE32(key.data() + 0);
+    input[1] = ReadLE32(key.data() + 4);
+    input[2] = ReadLE32(key.data() + 8);
+    input[3] = ReadLE32(key.data() + 12);
+    input[4] = ReadLE32(key.data() + 16);
+    input[5] = ReadLE32(key.data() + 20);
+    input[6] = ReadLE32(key.data() + 24);
+    input[7] = ReadLE32(key.data() + 28);
     input[8] = 0;
     input[9] = 0;
     input[10] = 0;

src/crypto/common.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2014-2020 The Bitcoin Core developers
+// Copyright (c) 2014-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -7,82 +7,99 @@
 
 #include <compat/endian.h>
 
+#include <concepts>
+#include <cstddef>
 #include <cstdint>
 #include <cstring>
 
-uint16_t static inline ReadLE16(const unsigned char* ptr)
+template <typename B>
+concept ByteType = std::same_as<B, unsigned char> || std::same_as<B, std::byte>;
+
+template <ByteType B>
+inline uint16_t ReadLE16(const B* ptr)
 {
     uint16_t x;
     memcpy(&x, ptr, 2);
     return le16toh_internal(x);
 }
 
-uint32_t static inline ReadLE32(const unsigned char* ptr)
+template <ByteType B>
+inline uint32_t ReadLE32(const B* ptr)
 {
     uint32_t x;
     memcpy(&x, ptr, 4);
     return le32toh_internal(x);
 }
 
-uint64_t static inline ReadLE64(const unsigned char* ptr)
+template <ByteType B>
+inline uint64_t ReadLE64(const B* ptr)
 {
     uint64_t x;
     memcpy(&x, ptr, 8);
     return le64toh_internal(x);
 }
 
-void static inline WriteLE16(unsigned char* ptr, uint16_t x)
+template <ByteType B>
+inline void WriteLE16(B* ptr, uint16_t x)
 {
     uint16_t v = htole16_internal(x);
     memcpy(ptr, &v, 2);
 }
 
-void static inline WriteLE32(unsigned char* ptr, uint32_t x)
+template <ByteType B>
+inline void WriteLE32(B* ptr, uint32_t x)
 {
     uint32_t v = htole32_internal(x);
     memcpy(ptr, &v, 4);
 }
 
-void static inline WriteLE64(unsigned char* ptr, uint64_t x)
+template <ByteType B>
+inline void WriteLE64(B* ptr, uint64_t x)
 {
     uint64_t v = htole64_internal(x);
     memcpy(ptr, &v, 8);
 }
 
-uint16_t static inline ReadBE16(const unsigned char* ptr)
+template <ByteType B>
+inline uint16_t ReadBE16(const B* ptr)
 {
     uint16_t x;
     memcpy(&x, ptr, 2);
     return be16toh_internal(x);
 }
 
-uint32_t static inline ReadBE32(const unsigned char* ptr)
+template <ByteType B>
+inline uint32_t ReadBE32(const B* ptr)
 {
     uint32_t x;
     memcpy(&x, ptr, 4);
     return be32toh_internal(x);
 }
 
-uint64_t static inline ReadBE64(const unsigned char* ptr)
+template <ByteType B>
+inline uint64_t ReadBE64(const B* ptr)
 {
     uint64_t x;
     memcpy(&x, ptr, 8);
     return be64toh_internal(x);
 }
 
-void static inline WriteBE16(unsigned char* ptr, uint16_t x)
+template <ByteType B>
+inline void WriteBE16(B* ptr, uint16_t x)
 {
     uint16_t v = htobe16_internal(x);
     memcpy(ptr, &v, 2);
 }
 
-void static inline WriteBE32(unsigned char* ptr, uint32_t x)
+template <ByteType B>
+inline void WriteBE32(B* ptr, uint32_t x)
 {
     uint32_t v = htobe32_internal(x);
     memcpy(ptr, &v, 4);
 }
 
-void static inline WriteBE64(unsigned char* ptr, uint64_t x)
+template <ByteType B>
+inline void WriteBE64(B* ptr, uint64_t x)
 {
     uint64_t v = htobe64_internal(x);
     memcpy(ptr, &v, 8);

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/policy/packages.h

@@ -89,8 +89,9 @@ bool IsChildWithParents(const Package& package);
  */
 bool IsChildWithParentsTree(const Package& package);
 
-/** Get the hash of these transactions' wtxids, concatenated in lexicographical order (treating the
- * wtxids as little endian encoded uint256, smallest to largest). */
+/** Get the hash of the concatenated wtxids of transactions, with wtxids
+ * treated as a little-endian numbers and sorted in ascending numeric order.
+ */
 uint256 GetPackageHash(const std::vector<CTransactionRef>& transactions);
 
 #endif // BITCOIN_POLICY_PACKAGES_H

src/policy/rbf.cpp

@@ -71,7 +71,7 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
         // descendants (i.e. if multiple conflicts share a descendant, it will be counted multiple
         // times), but we just want to be conservative to avoid doing too much work.
         if (nConflictingCount > MAX_REPLACEMENT_CANDIDATES) {
-            return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
+            return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)",
                              txid.ToString(),
                              nConflictingCount,
                              MAX_REPLACEMENT_CANDIDATES);

src/rpc/mempool.cpp

@@ -146,7 +146,8 @@ static RPCHelpMan testmempoolaccept()
                             {RPCResult{RPCResult::Type::STR_HEX, "", "transaction wtxid in hex"},
                         }},
                     }},
-                    {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"},
+                    {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection reason (only present when 'allowed' is false)"},
+                    {RPCResult::Type::STR, "reject-details", /*optional=*/true, "Rejection details (only present when 'allowed' is false and rejection details exist)"},
                 }},
             }
         },
@@ -245,6 +246,7 @@ static RPCHelpMan testmempoolaccept()
                         result_inner.pushKV("reject-reason", "missing-inputs");
                     } else {
                         result_inner.pushKV("reject-reason", state.GetRejectReason());
+                        result_inner.pushKV("reject-details", state.ToString());
                     }
                 }
                 rpc_result.push_back(std::move(result_inner));

src/rpc/mining.cpp

@@ -633,8 +633,8 @@ static RPCHelpMan getblocktemplate()
                     {RPCResult::Type::OBJ, "", "",
                     {
                         {RPCResult::Type::STR_HEX, "data", "transaction data encoded in hexadecimal (byte-for-byte)"},
-                        {RPCResult::Type::STR_HEX, "txid", "transaction id encoded in little-endian hexadecimal"},
-                        {RPCResult::Type::STR_HEX, "hash", "hash encoded in little-endian hexadecimal (including witness data)"},
+                        {RPCResult::Type::STR_HEX, "txid", "transaction hash excluding witness data, shown in byte-reversed hex"},
+                        {RPCResult::Type::STR_HEX, "hash", "transaction hash including witness data, shown in byte-reversed hex"},
                         {RPCResult::Type::ARR, "depends", "array of numbers",
                         {
                             {RPCResult::Type::NUM, "", "transactions before this one (by 1-based index in 'transactions' list) that must be present in the final block if this one is"},

src/support/allocators/secure.h

@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2021 The Bitcoin Core developers
+// Copyright (c) 2009-present The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -74,7 +74,7 @@ secure_unique_ptr<T> make_secure_unique(Args&&... as)
 
     // initialize in place, and return as secure_unique_ptr
     try {
-        return secure_unique_ptr<T>(new (p) T(std::forward(as)...));
+        return secure_unique_ptr<T>(new (p) T(std::forward<Args>(as)...));
     } catch (...) {
         secure_allocator<T>().deallocate(p, 1);
         throw;

src/test/miner_tests.cpp

@@ -8,6 +8,7 @@
 #include <consensus/consensus.h>
 #include <consensus/merkle.h>
 #include <consensus/tx_verify.h>
+#include <interfaces/mining.h>
 #include <node/miner.h>
 #include <policy/policy.h>
 #include <test/util/random.h>
@@ -15,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>
@@ -24,12 +26,14 @@
 #include <test/util/setup_common.h>
 
 #include <memory>
+#include <vector>
 
 #include <boost/test/unit_test.hpp>
 
 using namespace util::hex_literals;
+using interfaces::BlockTemplate;
+using interfaces::Mining;
 using node::BlockAssembler;
-using node::CBlockTemplate;
 
 namespace miner_tests {
 struct MinerTestingSetup : public TestingSetup {
@@ -54,7 +58,10 @@ struct MinerTestingSetup : public TestingSetup {
         Assert(error.empty());
         return *m_node.mempool;
     }
-    BlockAssembler AssemblerForTest(CTxMemPool& tx_mempool, BlockAssembler::Options options);
+    std::unique_ptr<Mining> MakeMining()
+    {
+        return interfaces::MakeMining(m_node);
+    }
 };
 } // namespace miner_tests
 
@@ -62,13 +69,6 @@ BOOST_FIXTURE_TEST_SUITE(miner_tests, MinerTestingSetup)
 
 static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE);
 
-BlockAssembler MinerTestingSetup::AssemblerForTest(CTxMemPool& tx_mempool, BlockAssembler::Options options)
-{
-    options.nBlockMaxWeight = MAX_BLOCK_WEIGHT;
-    options.blockMinFeeRate = blockMinFeeRate;
-    return BlockAssembler{m_node.chainman->ActiveChainstate(), &tx_mempool, options};
-}
-
 constexpr static struct {
     unsigned char extranonce;
     unsigned int nonce;
@@ -106,6 +106,10 @@ static std::unique_ptr<CBlockIndex> CreateBlockIndex(int nHeight, CBlockIndex* a
 void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst)
 {
     CTxMemPool& tx_mempool{MakeMempool()};
+    auto mining{MakeMining()};
+    BlockAssembler::Options options;
+    options.coinbase_output_script = scriptPubKey;
+
     LOCK(tx_mempool.cs);
     // Test the ancestor feerate transaction selection.
     TestMemPoolEntryHelper entry;
@@ -121,27 +125,45 @@ 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);
 
-    BlockAssembler::Options options;
-    options.coinbase_output_script = scriptPubKey;
-    std::unique_ptr<CBlockTemplate> pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-    BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 4U);
-    BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx);
-    BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx);
-    BOOST_CHECK(pblocktemplate->block.vtx[3]->GetHash() == hashMediumFeeTx);
+    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    CBlock block{block_template->getBlock()};
+    BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U);
+    BOOST_CHECK(block.vtx[1]->GetHash() == hashParentTx);
+    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;
@@ -158,11 +180,13 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse;
     Txid hashLowFeeTx = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(feeToUse).FromTx(tx));
-    pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
+    block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    block = block_template->getBlock();
     // Verify that the free tx and the low fee tx didn't get selected
-    for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) {
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx);
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashLowFeeTx);
+    for (size_t i=0; i<block.vtx.size(); ++i) {
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeTx);
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashLowFeeTx);
     }
 
     // Test that packages above the min relay fee do get included, even if one
@@ -172,10 +196,12 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee
     hashLowFeeTx = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(feeToUse + 2).FromTx(tx));
-    pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-    BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U);
-    BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx);
-    BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx);
+    block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    block = block_template->getBlock();
+    BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U);
+    BOOST_CHECK(block.vtx[4]->GetHash() == hashFreeTx);
+    BOOST_CHECK(block.vtx[5]->GetHash() == hashLowFeeTx);
 
     // Test that transaction selection properly updates ancestor fee
     // calculations as ancestor transactions get included in a block.
@@ -194,12 +220,14 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse;
     Txid hashLowFeeTx2 = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx));
-    pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
+    block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    block = block_template->getBlock();
 
     // Verify that this tx isn't selected.
-    for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) {
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx2);
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashLowFeeTx2);
+    for (size_t i=0; i<block.vtx.size(); ++i) {
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeTx2);
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashLowFeeTx2);
     }
 
     // This tx will be mineable, and should cause hashLowFeeTx2 to be selected
@@ -207,9 +235,11 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vin[0].prevout.n = 1;
     tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee
     AddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx));
-    pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-    BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 9U);
-    BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2);
+    block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    block = block_template->getBlock();
+    BOOST_REQUIRE_EQUAL(block.vtx.size(), 9U);
+    BOOST_CHECK(block.vtx[8]->GetHash() == hashLowFeeTx2);
 }
 
 void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst, int baseheight)
@@ -225,6 +255,9 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     const CAmount HIGHFEE = COIN;
     const CAmount HIGHERFEE = 4 * COIN;
 
+    auto mining{MakeMining()};
+    BOOST_REQUIRE(mining);
+
     BlockAssembler::Options options;
     options.coinbase_output_script = scriptPubKey;
 
@@ -233,8 +266,9 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         LOCK(tx_mempool.cs);
 
         // Just to make sure we can still make simple blocks
-        auto pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-        BOOST_CHECK(pblocktemplate);
+        auto block_template{mining->createNewBlock(options)};
+        BOOST_REQUIRE(block_template);
+        CBlock block{block_template->getBlock()};
 
         // block sigops > limit: 1000 CHECKMULTISIG + 1
         tx.vin.resize(1);
@@ -253,7 +287,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             tx.vin[0].prevout.hash = hash;
         }
 
-        BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool, options).CreateNewBlock(), std::runtime_error, HasReason("bad-blk-sigops"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-blk-sigops"));
     }
 
     {
@@ -270,7 +304,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx));
             tx.vin[0].prevout.hash = hash;
         }
-        BOOST_CHECK(AssemblerForTest(tx_mempool, options).CreateNewBlock());
+        BOOST_REQUIRE(mining->createNewBlock(options));
     }
 
     {
@@ -294,7 +328,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
             tx.vin[0].prevout.hash = hash;
         }
-        BOOST_CHECK(AssemblerForTest(tx_mempool, options).CreateNewBlock());
+        BOOST_REQUIRE(mining->createNewBlock(options));
     }
 
     {
@@ -304,7 +338,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         // orphan in tx_mempool, template creation fails
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool, options).CreateNewBlock(), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }
 
     {
@@ -325,7 +359,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_CHECK(AssemblerForTest(tx_mempool, options).CreateNewBlock());
+        BOOST_REQUIRE(mining->createNewBlock(options));
     }
 
     {
@@ -341,7 +375,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         // give it a fee so it'll get mined
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
         // Should throw bad-cb-multiple
-        BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool, options).CreateNewBlock(), std::runtime_error, HasReason("bad-cb-multiple"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-cb-multiple"));
     }
 
     {
@@ -358,7 +392,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         tx.vout[0].scriptPubKey = CScript() << OP_2;
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool, options).CreateNewBlock(), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }
 
     {
@@ -378,7 +412,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_CHECK(AssemblerForTest(tx_mempool, options).CreateNewBlock());
+        BOOST_REQUIRE(mining->createNewBlock(options));
         // Extend to a 210000-long block chain.
         while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) {
             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
@@ -390,7 +424,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_CHECK(AssemblerForTest(tx_mempool, options).CreateNewBlock());
+        BOOST_REQUIRE(mining->createNewBlock(options));
 
         // invalid p2sh txn in tx_mempool, template creation fails
         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
@@ -406,7 +440,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         tx.vout[0].nValue -= LOWFEE;
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool, options).CreateNewBlock(), std::runtime_error, HasReason("mandatory-script-verify-flag-failed"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("mandatory-script-verify-flag-failed"));
 
         // Delete the dummy blocks again.
         while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) {
@@ -508,14 +542,15 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1;
     BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail
 
-    auto pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-    BOOST_CHECK(pblocktemplate);
+    auto block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
 
     // None of the of the absolute height/time locked tx should have made
     // it into the template because we still check IsFinalTx in CreateNewBlock,
     // but relative locked txs will if inconsistently added to mempool.
     // For now these will still generate a valid template until BIP68 soft fork
-    BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3U);
+    CBlock block{block_template->getBlock()};
+    BOOST_CHECK_EQUAL(block.vtx.size(), 3U);
     // However if we advance height by 1 and time by SEQUENCE_LOCK_TIME, all of them should be mined
     for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
         CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
@@ -524,12 +559,17 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     m_node.chainman->ActiveChain().Tip()->nHeight++;
     SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1);
 
-    BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock());
-    BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5U);
+    block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    block = block_template->getBlock();
+    BOOST_CHECK_EQUAL(block.vtx.size(), 5U);
 }
 
 void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst)
 {
+    auto mining{MakeMining()};
+    BOOST_REQUIRE(mining);
+
     BlockAssembler::Options options;
     options.coinbase_output_script = scriptPubKey;
 
@@ -594,34 +634,34 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
     Txid hashFreeGrandchild = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx));
 
-    auto pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock();
-    BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U);
-    BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashFreeParent);
-    BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashFreePrioritisedTx);
-    BOOST_CHECK(pblocktemplate->block.vtx[3]->GetHash() == hashParentTx);
-    BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashPrioritsedChild);
-    BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashFreeChild);
-    for (size_t i=0; i<pblocktemplate->block.vtx.size(); ++i) {
+    auto block_template = mining->createNewBlock(options);
+    BOOST_REQUIRE(block_template);
+    CBlock block{block_template->getBlock()};
+    BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U);
+    BOOST_CHECK(block.vtx[1]->GetHash() == hashFreeParent);
+    BOOST_CHECK(block.vtx[2]->GetHash() == hashFreePrioritisedTx);
+    BOOST_CHECK(block.vtx[3]->GetHash() == hashParentTx);
+    BOOST_CHECK(block.vtx[4]->GetHash() == hashPrioritsedChild);
+    BOOST_CHECK(block.vtx[5]->GetHash() == hashFreeChild);
+    for (size_t i=0; i<block.vtx.size(); ++i) {
         // The FreeParent and FreeChild's prioritisations should not impact the child.
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeGrandchild);
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeGrandchild);
         // De-prioritised transaction should not be included.
-        BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashMediumFeeTx);
+        BOOST_CHECK(block.vtx[i]->GetHash() != hashMediumFeeTx);
     }
 }
 
 // NOTE: These tests rely on CreateNewBlock doing its own self-validation!
 BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
 {
+    auto mining{MakeMining()};
+    BOOST_REQUIRE(mining);
+
     // Note that by default, these tests run with size accounting enabled.
     CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
-    std::unique_ptr<CBlockTemplate> pblocktemplate;
-
     BlockAssembler::Options options;
     options.coinbase_output_script = scriptPubKey;
-
-    CTxMemPool& tx_mempool{*m_node.mempool};
-    // Simple block creation, nothing special yet:
-    BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool, options).CreateNewBlock());
+    std::unique_ptr<BlockTemplate> block_template;
 
     // We can't make transactions until we have inputs
     // Therefore, load 110 blocks :)
@@ -629,27 +669,48 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     int baseheight = 0;
     std::vector<CTransactionRef> txFirst;
     for (const auto& bi : BLOCKINFO) {
-        CBlock *pblock = &pblocktemplate->block; // pointer for convenience
+        const int current_height{mining->getTip()->height};
+
+        // Simple block creation, nothing special yet:
+        block_template = mining->createNewBlock(options);
+        BOOST_REQUIRE(block_template);
+
+        CBlock block{block_template->getBlock()};
+        CMutableTransaction txCoinbase(*block.vtx[0]);
         {
             LOCK(cs_main);
-            pblock->nVersion = VERSIONBITS_TOP_BITS;
-            pblock->nTime = m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1;
-            CMutableTransaction txCoinbase(*pblock->vtx[0]);
+            block.nVersion = VERSIONBITS_TOP_BITS;
+            block.nTime = Assert(m_node.chainman)->ActiveChain().Tip()->GetMedianTimePast()+1;
             txCoinbase.version = 1;
-            txCoinbase.vin[0].scriptSig = CScript{} << (m_node.chainman->ActiveChain().Height() + 1) << bi.extranonce;
+            txCoinbase.vin[0].scriptSig = CScript{} << (current_height + 1) << bi.extranonce;
             txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this)
             txCoinbase.vout[0].scriptPubKey = CScript();
-            pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase));
+            block.vtx[0] = MakeTransactionRef(txCoinbase);
             if (txFirst.size() == 0)
-                baseheight = m_node.chainman->ActiveChain().Height();
+                baseheight = current_height;
             if (txFirst.size() < 4)
-                txFirst.push_back(pblock->vtx[0]);
-            pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
-            pblock->nNonce = bi.nonce;
+                txFirst.push_back(block.vtx[0]);
+            block.hashMerkleRoot = BlockMerkleRoot(block);
+            block.nNonce = bi.nonce;
         }
-        std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
-        BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, true, nullptr));
-        pblock->hashPrevBlock = pblock->GetHash();
+        std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block);
+        // Alternate calls between Chainman's ProcessNewBlock and submitSolution
+        // via the Mining interface. The former is used by net_processing as well
+        // as the submitblock RPC.
+        if (current_height % 2 == 0) {
+            BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, /*force_processing=*/true, /*min_pow_checked=*/true, nullptr));
+        } else {
+            BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(txCoinbase)));
+        }
+        {
+            LOCK(cs_main);
+            // The above calls don't guarantee the tip is actually updated, so
+            // we explictly check this.
+            auto maybe_new_tip{Assert(m_node.chainman)->ActiveChain().Tip()};
+            BOOST_REQUIRE_EQUAL(maybe_new_tip->GetBlockHash(), block.GetHash());
+        }
+        // This just adds coverage
+        mining->waitTipChanged(block.hashPrevBlock);
     }
 
     LOCK(cs_main);

src/txmempool.cpp

@@ -442,7 +442,7 @@ void CTxMemPool::Apply(ChangeSet* changeset)
         std::optional<CTxMemPool::setEntries> ancestors;
         if (i == 0) {
             // Note: ChangeSet::CalculateMemPoolAncestors() will return a
-            // cached value if mempool ancestors for this tranaction were
+            // cached value if mempool ancestors for this transaction were
             // previously calculated.
             // We can only use a cached ancestor calculation for the first
             // transaction in a package, because in-package parents won't be

src/txmempool.h

@@ -810,7 +810,7 @@ public:
      * mempool.
      *
      * CalculateMemPoolAncestors() calculates the in-mempool (not including
-     * what is in the change set itself) ancestors of a given transacion.
+     * what is in the change set itself) ancestors of a given transaction.
      *
      * Apply() will apply the removals and additions that are staged into the
      * mempool.

src/uint256.h

@@ -69,16 +69,27 @@ public:
 
     /** @name Hex representation
      *
-     * The reverse-byte hex representation is a convenient way to view the blob
-     * as a number, because it is consistent with the way the base_uint class
-     * converts blobs to numbers.
+     * The hex representation used by GetHex(), ToString(), FromHex() and
+     * SetHexDeprecated() is unusual, since it shows bytes of the base_blob in
+     * reverse order. For example, a 4-byte blob {0x12, 0x34, 0x56, 0x78} is
+     * represented as "78563412" instead of the more typical "12345678"
+     * representation that would be shown in a hex editor or used by typical
+     * byte-array / hex conversion functions like python's bytes.hex() and
+     * bytes.fromhex().
+     *
+     * The nice thing about the reverse-byte representation, even though it is
+     * unusual, is that if a blob contains an arithmetic number in little endian
+     * format (with least significant bytes first, and most significant bytes
+     * last), the GetHex() output will match the way the number would normally
+     * be written in base-16 (with most significant digits first and least
+     * significant digits last).
+     *
+     * This means, for example, that ArithToUint256(num).GetHex() can be used to
+     * display an arith_uint256 num value as a number, because
+     * ArithToUint256() converts the number to a blob in little-endian format,
+     * so the arith_uint256 class doesn't need to have its own number parsing
+     * and formatting functions.
      *
-     * @note base_uint treats the blob as an array of bytes with the numerically
-     * least significant byte first and the most significant byte last. Because
-     * numbers are typically written with the most significant digit first and
-     * the least significant digit last, the reverse hex display of the blob
-     * corresponds to the same numeric value that base_uint interprets from the
-     * blob.
      * @{*/
     std::string GetHex() const;
     /** Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated!

src/univalue/CMakeLists.txt

@@ -15,10 +15,119 @@ target_include_directories(univalue
 target_link_libraries(univalue PRIVATE core_interface)
 
 if(BUILD_TESTS)
-  add_executable(unitester test/unitester.cpp)
-  target_compile_definitions(unitester
-    PRIVATE
-      JSON_TEST_SRC=\"${CMAKE_CURRENT_SOURCE_DIR}/test\"
+  include(GenerateHeaders)
+  generate_header_from_json(test/fail1.json)
+  generate_header_from_json(test/fail10.json)
+  generate_header_from_json(test/fail11.json)
+  generate_header_from_json(test/fail12.json)
+  generate_header_from_json(test/fail13.json)
+  generate_header_from_json(test/fail14.json)
+  generate_header_from_json(test/fail15.json)
+  generate_header_from_json(test/fail16.json)
+  generate_header_from_json(test/fail17.json)
+  generate_header_from_json(test/fail18.json)
+  generate_header_from_json(test/fail19.json)
+  generate_header_from_json(test/fail2.json)
+  generate_header_from_json(test/fail20.json)
+  generate_header_from_json(test/fail21.json)
+  generate_header_from_json(test/fail22.json)
+  generate_header_from_json(test/fail23.json)
+  generate_header_from_json(test/fail24.json)
+  generate_header_from_json(test/fail25.json)
+  generate_header_from_json(test/fail26.json)
+  generate_header_from_json(test/fail27.json)
+  generate_header_from_json(test/fail28.json)
+  generate_header_from_json(test/fail29.json)
+  generate_header_from_json(test/fail3.json)
+  generate_header_from_json(test/fail30.json)
+  generate_header_from_json(test/fail31.json)
+  generate_header_from_json(test/fail32.json)
+  generate_header_from_json(test/fail33.json)
+  generate_header_from_json(test/fail34.json)
+  generate_header_from_json(test/fail35.json)
+  generate_header_from_json(test/fail36.json)
+  generate_header_from_json(test/fail37.json)
+  generate_header_from_json(test/fail38.json)
+  generate_header_from_json(test/fail39.json)
+  generate_header_from_json(test/fail4.json)
+  generate_header_from_json(test/fail40.json)
+  generate_header_from_json(test/fail41.json)
+  generate_header_from_json(test/fail42.json)
+  generate_header_from_json(test/fail44.json)
+  generate_header_from_json(test/fail45.json)
+  generate_header_from_json(test/fail5.json)
+  generate_header_from_json(test/fail6.json)
+  generate_header_from_json(test/fail7.json)
+  generate_header_from_json(test/fail8.json)
+  generate_header_from_json(test/fail9.json)
+  generate_header_from_json(test/pass1.json)
+  generate_header_from_json(test/pass2.json)
+  generate_header_from_json(test/pass3.json)
+  generate_header_from_json(test/pass4.json)
+  generate_header_from_json(test/round1.json)
+  generate_header_from_json(test/round2.json)
+  generate_header_from_json(test/round3.json)
+  generate_header_from_json(test/round4.json)
+  generate_header_from_json(test/round5.json)
+  generate_header_from_json(test/round6.json)
+  generate_header_from_json(test/round7.json)
+  add_executable(unitester
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail1.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail10.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail11.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail12.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail13.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail14.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail15.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail16.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail17.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail18.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail19.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail2.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail20.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail21.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail22.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail23.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail24.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail25.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail26.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail27.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail28.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail29.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail3.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail30.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail31.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail32.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail33.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail34.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail35.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail36.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail37.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail38.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail39.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail4.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail40.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail41.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail42.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail44.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail45.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail5.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail6.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail7.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail8.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/fail9.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/pass1.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/pass2.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/pass3.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/pass4.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round1.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round2.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round3.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round4.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round5.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round6.json.h
+    ${CMAKE_CURRENT_BINARY_DIR}/test/round7.json.h
+    test/unitester.cpp
   )
   target_link_libraries(unitester
     PRIVATE

src/univalue/test/fail18.json

@@ -1 +1 @@
-[[[[[[[[[[[[[[[[[[[["Too deep"]]]]]]]]]]]]]]]]]]]]
+[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[["Too deep"]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]

src/univalue/test/pass2.json

@@ -1 +1 @@
-[[[[[[[[[[[[[[[[[[["Not too deep"]]]]]]]]]]]]]]]]]]]
+[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[["Not too deep"]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]

src/univalue/test/unitester.cpp

@@ -1,130 +1,159 @@
 // Copyright 2014 BitPay Inc.
-// Distributed under the MIT/X11 software license, see the accompanying
+// Copyright (c) 2015-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
 // file COPYING or https://opensource.org/licenses/mit-license.php.
 
 #include <univalue.h>
 
+#include <univalue/test/fail1.json.h>
+#include <univalue/test/fail10.json.h>
+#include <univalue/test/fail11.json.h>
+#include <univalue/test/fail12.json.h>
+#include <univalue/test/fail13.json.h>
+#include <univalue/test/fail14.json.h>
+#include <univalue/test/fail15.json.h>
+#include <univalue/test/fail16.json.h>
+#include <univalue/test/fail17.json.h>
+#include <univalue/test/fail18.json.h>
+#include <univalue/test/fail19.json.h>
+#include <univalue/test/fail2.json.h>
+#include <univalue/test/fail20.json.h>
+#include <univalue/test/fail21.json.h>
+#include <univalue/test/fail22.json.h>
+#include <univalue/test/fail23.json.h>
+#include <univalue/test/fail24.json.h>
+#include <univalue/test/fail25.json.h>
+#include <univalue/test/fail26.json.h>
+#include <univalue/test/fail27.json.h>
+#include <univalue/test/fail28.json.h>
+#include <univalue/test/fail29.json.h>
+#include <univalue/test/fail3.json.h>
+#include <univalue/test/fail30.json.h>
+#include <univalue/test/fail31.json.h>
+#include <univalue/test/fail32.json.h>
+#include <univalue/test/fail33.json.h>
+#include <univalue/test/fail34.json.h>
+#include <univalue/test/fail35.json.h>
+#include <univalue/test/fail36.json.h>
+#include <univalue/test/fail37.json.h>
+#include <univalue/test/fail38.json.h>
+#include <univalue/test/fail39.json.h>
+#include <univalue/test/fail4.json.h>
+#include <univalue/test/fail40.json.h>
+#include <univalue/test/fail41.json.h>
+#include <univalue/test/fail42.json.h>
+#include <univalue/test/fail44.json.h>
+#include <univalue/test/fail45.json.h>
+#include <univalue/test/fail5.json.h>
+#include <univalue/test/fail6.json.h>
+#include <univalue/test/fail7.json.h>
+#include <univalue/test/fail8.json.h>
+#include <univalue/test/fail9.json.h>
+#include <univalue/test/pass1.json.h>
+#include <univalue/test/pass2.json.h>
+#include <univalue/test/pass3.json.h>
+#include <univalue/test/pass4.json.h>
+#include <univalue/test/round1.json.h>
+#include <univalue/test/round2.json.h>
+#include <univalue/test/round3.json.h>
+#include <univalue/test/round4.json.h>
+#include <univalue/test/round5.json.h>
+#include <univalue/test/round6.json.h>
+#include <univalue/test/round7.json.h>
+
+#include <array>
 #include <cassert>
 #include <cstdio>
 #include <string>
 
-#ifndef JSON_TEST_SRC
-#error JSON_TEST_SRC must point to test source directory
-#endif
-
-std::string srcdir(JSON_TEST_SRC);
-
 static std::string rtrim(std::string s)
 {
-    s.erase(s.find_last_not_of(" \n\r\t")+1);
+    s.erase(s.find_last_not_of(" \n\r\t") + 1);
     return s;
 }
 
 static void runtest(std::string filename, const std::string& jdata)
 {
-        std::string prefix = filename.substr(0, 4);
+    std::string prefix = filename.substr(0, 4);
 
-        bool wantPass = (prefix == "pass") || (prefix == "roun");
-        bool wantFail = (prefix == "fail");
-        bool wantRoundTrip = (prefix == "roun");
-        assert(wantPass || wantFail);
+    bool wantPass = (prefix == "pass") || (prefix == "roun");
+    bool wantFail = (prefix == "fail");
+    bool wantRoundTrip = (prefix == "roun");
+    assert(wantPass || wantFail);
 
-        UniValue val;
-        bool testResult = val.read(jdata);
+    UniValue val;
+    bool testResult = val.read(jdata);
 
-        if (wantPass) {
-            assert(testResult == true);
-        } else {
-            assert(testResult == false);
-        }
+    if (wantPass) {
+        assert(testResult == true);
+    } else {
+        assert(testResult == false);
+    }
 
-        if (wantRoundTrip) {
-            std::string odata = val.write(0, 0);
-            assert(odata == rtrim(jdata));
-        }
+    if (wantRoundTrip) {
+        std::string odata = val.write(0, 0);
+        assert(odata == rtrim(jdata));
+    }
 }
 
-static void runtest_file(const char *filename_)
-{
-        std::string basename(filename_);
-        std::string filename = srcdir + "/" + basename;
-        FILE *f = fopen(filename.c_str(), "r");
-        assert(f != nullptr);
-
-        std::string jdata;
-
-        char buf[4096];
-        while (!feof(f)) {
-                int bread = fread(buf, 1, sizeof(buf), f);
-                assert(!ferror(f));
-
-                std::string s(buf, bread);
-                jdata += s;
-        }
-
-        assert(!ferror(f));
-        fclose(f);
-
-        runtest(basename, jdata);
-}
-
-static const char *filenames[] = {
-        "fail10.json",
-        "fail11.json",
-        "fail12.json",
-        "fail13.json",
-        "fail14.json",
-        "fail15.json",
-        "fail16.json",
-        "fail17.json",
-        //"fail18.json",             // investigate
-        "fail19.json",
-        "fail1.json",
-        "fail20.json",
-        "fail21.json",
-        "fail22.json",
-        "fail23.json",
-        "fail24.json",
-        "fail25.json",
-        "fail26.json",
-        "fail27.json",
-        "fail28.json",
-        "fail29.json",
-        "fail2.json",
-        "fail30.json",
-        "fail31.json",
-        "fail32.json",
-        "fail33.json",
-        "fail34.json",
-        "fail35.json",
-        "fail36.json",
-        "fail37.json",
-        "fail38.json",               // invalid unicode: only first half of surrogate pair
-        "fail39.json",               // invalid unicode: only second half of surrogate pair
-        "fail40.json",               // invalid unicode: broken UTF-8
-        "fail41.json",               // invalid unicode: unfinished UTF-8
-        "fail42.json",               // valid json with garbage following a nul byte
-        "fail44.json",               // unterminated string
-        "fail45.json",               // nested beyond max depth
-        "fail3.json",
-        "fail4.json",                // extra comma
-        "fail5.json",
-        "fail6.json",
-        "fail7.json",
-        "fail8.json",
-        "fail9.json",               // extra comma
-        "pass1.json",
-        "pass2.json",
-        "pass3.json",
-        "pass4.json",
-        "round1.json",              // round-trip test
-        "round2.json",              // unicode
-        "round3.json",              // bare string
-        "round4.json",              // bare number
-        "round5.json",              // bare true
-        "round6.json",              // bare false
-        "round7.json",              // bare null
-};
+#define TEST_FILE(name) {#name, json_tests::name}
+inline constexpr std::array tests{std::to_array<std::tuple<std::string_view, std::string_view>>({
+    TEST_FILE(fail1),
+    TEST_FILE(fail10),
+    TEST_FILE(fail11),
+    TEST_FILE(fail12),
+    TEST_FILE(fail13),
+    TEST_FILE(fail14),
+    TEST_FILE(fail15),
+    TEST_FILE(fail16),
+    TEST_FILE(fail17),
+    TEST_FILE(fail18),
+    TEST_FILE(fail19),
+    TEST_FILE(fail2),
+    TEST_FILE(fail20),
+    TEST_FILE(fail21),
+    TEST_FILE(fail22),
+    TEST_FILE(fail23),
+    TEST_FILE(fail24),
+    TEST_FILE(fail25),
+    TEST_FILE(fail26),
+    TEST_FILE(fail27),
+    TEST_FILE(fail28),
+    TEST_FILE(fail29),
+    TEST_FILE(fail3),
+    TEST_FILE(fail30),
+    TEST_FILE(fail31),
+    TEST_FILE(fail32),
+    TEST_FILE(fail33),
+    TEST_FILE(fail34),
+    TEST_FILE(fail35),
+    TEST_FILE(fail36),
+    TEST_FILE(fail37),
+    TEST_FILE(fail38), // invalid unicode: only first half of surrogate pair
+    TEST_FILE(fail39), // invalid unicode: only second half of surrogate pair
+    TEST_FILE(fail4),  // extra comma
+    TEST_FILE(fail40), // invalid unicode: broken UTF-8
+    TEST_FILE(fail41), // invalid unicode: unfinished UTF-8
+    TEST_FILE(fail42), // valid json with garbage following a nul byte
+    TEST_FILE(fail44), // unterminated string
+    TEST_FILE(fail45), // nested beyond max depth
+    TEST_FILE(fail5),
+    TEST_FILE(fail6),
+    TEST_FILE(fail7),
+    TEST_FILE(fail8),
+    TEST_FILE(fail9), // extra comma
+    TEST_FILE(pass1),
+    TEST_FILE(pass2),
+    TEST_FILE(pass3),
+    TEST_FILE(pass4),
+    TEST_FILE(round1), // round-trip test
+    TEST_FILE(round2), // unicode
+    TEST_FILE(round3), // bare string
+    TEST_FILE(round4), // bare number
+    TEST_FILE(round5), // bare true
+    TEST_FILE(round6), // bare false
+    TEST_FILE(round7), // bare null
+})};
 
 // Test \u handling
 void unescape_unicode_test()
@@ -156,10 +185,10 @@ void no_nul_test()
     assert(val.read({buf + 3, 7}));
 }
 
-int main (int argc, char *argv[])
+int main(int argc, char* argv[])
 {
-    for (const auto& f: filenames) {
-        runtest_file(f);
+    for (const auto& [file, json] : tests) {
+        runtest(std::string{file}, std::string{json});
     }
 
     unescape_unicode_test();
@@ -167,4 +196,3 @@ int main (int argc, char *argv[])
 
     return 0;
 }
-

test/functional/feature_cltv.py

@@ -148,6 +148,10 @@ class BIP65Test(BitcoinTestFramework):
         # create and test one invalid tx per CLTV failure reason (5 in total)
         for i in range(5):
             spendtx = wallet.create_self_transfer()['tx']
+            assert_equal(len(spendtx.vin), 1)
+            coin = spendtx.vin[0]
+            coin_txid = format(coin.prevout.hash, '064x')
+            coin_vout = coin.prevout.n
             cltv_invalidate(spendtx, i)
 
             expected_cltv_reject_reason = [
@@ -159,12 +163,15 @@ class BIP65Test(BitcoinTestFramework):
             ][i]
             # First we show that this tx is valid except for CLTV by getting it
             # rejected from the mempool for exactly that reason.
+            spendtx_txid = spendtx.hash
+            spendtx_wtxid = spendtx.getwtxid()
             assert_equal(
                 [{
-                    'txid': spendtx.hash,
-                    'wtxid': spendtx.getwtxid(),
+                    'txid': spendtx_txid,
+                    'wtxid': spendtx_wtxid,
                     'allowed': False,
                     'reject-reason': expected_cltv_reject_reason,
+                    'reject-details': expected_cltv_reject_reason + f", input 0 of {spendtx_txid} (wtxid {spendtx_wtxid}), spending {coin_txid}:{coin_vout}"
                 }],
                 self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0),
             )

test/functional/feature_dersig.py

@@ -109,18 +109,23 @@ class BIP66Test(BitcoinTestFramework):
         self.log.info("Test that transactions with non-DER signatures cannot appear in a block")
         block.nVersion = 4
 
-        spendtx = self.create_tx(self.coinbase_txids[1])
+        coin_txid = self.coinbase_txids[1]
+        spendtx = self.create_tx(coin_txid)
         unDERify(spendtx)
         spendtx.rehash()
 
         # First we show that this tx is valid except for DERSIG by getting it
         # rejected from the mempool for exactly that reason.
+        spendtx_txid = spendtx.hash
+        spendtx_wtxid = spendtx.getwtxid()
         assert_equal(
             [{
-                'txid': spendtx.hash,
-                'wtxid': spendtx.getwtxid(),
+                'txid': spendtx_txid,
+                'wtxid': spendtx_wtxid,
                 'allowed': False,
                 'reject-reason': 'mandatory-script-verify-flag-failed (Non-canonical DER signature)',
+                'reject-details': 'mandatory-script-verify-flag-failed (Non-canonical DER signature), ' +
+                                  f"input 0 of {spendtx_txid} (wtxid {spendtx_wtxid}), spending {coin_txid}:0"
             }],
             self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0),
         )

test/functional/feature_rbf.py

@@ -103,14 +103,22 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         """Simple doublespend"""
         # we use MiniWallet to create a transaction template with inputs correctly set,
         # and modify the output (amount, scriptPubKey) according to our needs
-        tx = self.wallet.create_self_transfer()["tx"]
+        tx = self.wallet.create_self_transfer(fee_rate=Decimal("0.003"))["tx"]
         tx1a_txid = self.nodes[0].sendrawtransaction(tx.serialize().hex())
 
         # Should fail because we haven't changed the fee
         tx.vout[0].scriptPubKey[-1] ^= 1
+        tx.rehash()
+        tx_hex = tx.serialize().hex()
 
         # This will raise an exception due to insufficient fee
-        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex(), 0)
+        reject_reason = "insufficient fee"
+        reject_details = f"{reject_reason}, rejecting replacement {tx.hash}; new feerate 0.00300000 BTC/kvB <= old feerate 0.00300000 BTC/kvB"
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx_hex])[0]
+        assert_equal(res["reject-reason"], reject_reason)
+        assert_equal(res["reject-details"], reject_details)
+        assert_raises_rpc_error(-26, f"{reject_details}", self.nodes[0].sendrawtransaction, tx_hex, 0)
+
 
         # Extra 0.1 BTC fee
         tx.vout[0].nValue -= int(0.1 * COIN)
@@ -154,7 +162,14 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         dbl_tx_hex = dbl_tx.serialize().hex()
 
         # This will raise an exception due to insufficient fee
-        assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0)
+        reject_reason = "insufficient fee"
+        reject_details = f"{reject_reason}, rejecting replacement {dbl_tx.hash}, less fees than conflicting txs; 3.00 < 4.00"
+        res = self.nodes[0].testmempoolaccept(rawtxs=[dbl_tx_hex])[0]
+        assert_equal(res["reject-reason"], reject_reason)
+        assert_equal(res["reject-details"], reject_details)
+        assert_raises_rpc_error(-26, f"{reject_details}", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0)
+
+
 
         # Accepted with sufficient fee
         dbl_tx.vout[0].nValue = int(0.1 * COIN)
@@ -273,22 +288,30 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         utxo1 = self.make_utxo(self.nodes[0], int(1.2 * COIN))
         utxo2 = self.make_utxo(self.nodes[0], 3 * COIN)
 
-        tx1a_utxo = self.wallet.send_self_transfer(
+        tx1a = self.wallet.send_self_transfer(
             from_node=self.nodes[0],
             utxo_to_spend=utxo1,
             sequence=0,
             fee=Decimal("0.1"),
-        )["new_utxo"]
+        )
+        tx1a_utxo = tx1a["new_utxo"]
 
         # Direct spend an output of the transaction we're replacing.
-        tx2_hex = self.wallet.create_self_transfer_multi(
+        tx2 = self.wallet.create_self_transfer_multi(
             utxos_to_spend=[utxo1, utxo2, tx1a_utxo],
             sequence=0,
             amount_per_output=int(COIN * tx1a_utxo["value"]),
-        )["hex"]
+        )["tx"]
+        tx2_hex = tx2.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+        reject_reason = "bad-txns-spends-conflicting-tx"
+        reject_details = f"{reject_reason}, {tx2.hash} spends conflicting transaction {tx1a['tx'].hash}"
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx2_hex])[0]
+        assert_equal(res["reject-reason"], reject_reason)
+        assert_equal(res["reject-details"], reject_details)
+        assert_raises_rpc_error(-26, f"{reject_details}", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+
 
         # Spend tx1a's output to test the indirect case.
         tx1b_utxo = self.wallet.send_self_transfer(
@@ -319,14 +342,21 @@ class ReplaceByFeeTest(BitcoinTestFramework):
             fee=Decimal("0.1"),
         )
 
-        tx2_hex = self.wallet.create_self_transfer_multi(
+        tx2 = self.wallet.create_self_transfer_multi(
             utxos_to_spend=[confirmed_utxo, unconfirmed_utxo],
             sequence=0,
             amount_per_output=1 * COIN,
-        )["hex"]
+        )["tx"]
+        tx2_hex = tx2.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "replacement-adds-unconfirmed", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+        reject_reason = "replacement-adds-unconfirmed"
+        reject_details = f"{reject_reason}, replacement {tx2.hash} adds unconfirmed input, idx 1"
+        res = self.nodes[0].testmempoolaccept(rawtxs=[tx2_hex])[0]
+        assert_equal(res["reject-reason"], reject_reason)
+        assert_equal(res["reject-details"], reject_details)
+        assert_raises_rpc_error(-26, f"{reject_details}", self.nodes[0].sendrawtransaction, tx2_hex, 0)
+
 
     def test_too_many_replacements(self):
         """Replacements that evict too many transactions are rejected"""
@@ -368,7 +398,13 @@ class ReplaceByFeeTest(BitcoinTestFramework):
         double_tx_hex = double_tx.serialize().hex()
 
         # This will raise an exception
-        assert_raises_rpc_error(-26, "too many potential replacements", self.nodes[0].sendrawtransaction, double_tx_hex, 0)
+        reject_reason = "too many potential replacements"
+        reject_details = f"{reject_reason}, rejecting replacement {double_tx.hash}; too many potential replacements ({MAX_REPLACEMENT_LIMIT + 1} > {MAX_REPLACEMENT_LIMIT})"
+        res = self.nodes[0].testmempoolaccept(rawtxs=[double_tx_hex])[0]
+        assert_equal(res["reject-reason"], reject_reason)
+        assert_equal(res["reject-details"], reject_details)
+        assert_raises_rpc_error(-26, f"{reject_details}", self.nodes[0].sendrawtransaction, double_tx_hex, 0)
+
 
         # If we remove an input, it should pass
         double_tx.vin.pop()

test/functional/mempool_accept.py

@@ -67,6 +67,8 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
             if "fees" in r:
                 r["fees"].pop("effective-feerate")
                 r["fees"].pop("effective-includes")
+            if "reject-details" in r:
+                r.pop("reject-details")
         assert_equal(result_expected, result_test)
         assert_equal(self.nodes[0].getmempoolinfo()['size'], self.mempool_size)  # Must not change mempool state
 

test/functional/mempool_accept_wtxid.py

@@ -100,13 +100,15 @@ class MempoolWtxidTest(BitcoinTestFramework):
             "txid": child_one_txid,
             "wtxid": child_one_wtxid,
             "allowed": False,
-            "reject-reason": "txn-already-in-mempool"
+            "reject-reason": "txn-already-in-mempool",
+            "reject-details": "txn-already-in-mempool"
         }])
         assert_equal(node.testmempoolaccept([child_two.serialize().hex()])[0], {
             "txid": child_two_txid,
             "wtxid": child_two_wtxid,
             "allowed": False,
-            "reject-reason": "txn-same-nonwitness-data-in-mempool"
+            "reject-reason": "txn-same-nonwitness-data-in-mempool",
+            "reject-details": "txn-same-nonwitness-data-in-mempool"
         })
 
         # sendrawtransaction will not throw but quits early when the exact same transaction is already in mempool

test/functional/mempool_ephemeral_dust.py

@@ -126,7 +126,7 @@ class EphemeralDustTest(BitcoinTestFramework):
         assert_equal(len(self.nodes[0].getrawmempool()), 2)
         assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])
 
-        # Node restart; doesn't allow allow ephemeral transaction back in due to individual submission
+        # Node restart; doesn't allow ephemeral transaction back in due to individual submission
         # resulting in 0-fee. Supporting re-submission of CPFP packages on restart is desired but not
         # yet implemented.
         self.restart_node(0)

test/functional/mempool_package_rbf.py

@@ -219,7 +219,7 @@ class PackageRBFTest(BitcoinTestFramework):
         package_child = self.wallet.create_self_transfer(fee_rate=child_feerate, utxo_to_spend=package_parent["new_utxos"][0])
 
         pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]], maxfeerate=0)
-        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_child['tx'].rehash()}; too many potential replacements (102 > 100)\n", pkg_results["package_msg"])
+        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_child['tx'].rehash()}; too many potential replacements (102 > 100)", pkg_results["package_msg"])
         self.assert_mempool_contents(expected=expected_txns)
 
         # Make singleton tx to conflict with in next batch
@@ -234,7 +234,7 @@ class PackageRBFTest(BitcoinTestFramework):
         package_parent = self.wallet.create_self_transfer_multi(utxos_to_spend=double_spending_coins, fee_per_output=parent_fee_per_conflict)
         package_child = self.wallet.create_self_transfer(fee_rate=child_feerate, utxo_to_spend=package_parent["new_utxos"][0])
         pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]], maxfeerate=0)
-        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_child['tx'].rehash()}; too many potential replacements (101 > 100)\n", pkg_results["package_msg"])
+        assert_equal(f"package RBF failed: too many potential replacements, rejecting replacement {package_child['tx'].rehash()}; too many potential replacements (101 > 100)", pkg_results["package_msg"])
         self.assert_mempool_contents(expected=expected_txns)
 
         # Finally, evict MAX_REPLACEMENT_CANDIDATES

test/functional/rpc_packages.py

@@ -110,17 +110,21 @@ class RPCPackagesTest(BitcoinTestFramework):
         self.assert_testres_equal(package_bad, testres_bad)
 
         self.log.info("Check testmempoolaccept tells us when some transactions completed validation successfully")
-        tx_bad_sig_hex = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}],
+        tx_bad_sig_hex = node.createrawtransaction([{"txid": coin["txid"], "vout": coin["vout"]}],
                                            {address : coin["amount"] - Decimal("0.0001")})
         tx_bad_sig = tx_from_hex(tx_bad_sig_hex)
         testres_bad_sig = node.testmempoolaccept(self.independent_txns_hex + [tx_bad_sig_hex])
         # By the time the signature for the last transaction is checked, all the other transactions
         # have been fully validated, which is why the node returns full validation results for all
         # transactions here but empty results in other cases.
+        tx_bad_sig_txid = tx_bad_sig.rehash()
+        tx_bad_sig_wtxid = tx_bad_sig.getwtxid()
         assert_equal(testres_bad_sig, self.independent_txns_testres + [{
-            "txid": tx_bad_sig.rehash(),
-            "wtxid": tx_bad_sig.getwtxid(), "allowed": False,
-            "reject-reason": "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)"
+            "txid": tx_bad_sig_txid,
+            "wtxid": tx_bad_sig_wtxid, "allowed": False,
+            "reject-reason": "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)",
+            "reject-details": "mandatory-script-verify-flag-failed (Operation not valid with the current stack size), " +
+                              f"input 0 of {tx_bad_sig_txid} (wtxid {tx_bad_sig_wtxid}), spending {coin['txid']}:{coin['vout']}"
         }])
 
         self.log.info("Check testmempoolaccept reports txns in packages that exceed max feerate")
@@ -304,7 +308,8 @@ class RPCPackagesTest(BitcoinTestFramework):
         assert testres_rbf_single[0]["allowed"]
         testres_rbf_package = self.independent_txns_testres_blank + [{
             "txid": replacement_tx["txid"], "wtxid": replacement_tx["wtxid"], "allowed": False,
-            "reject-reason": "bip125-replacement-disallowed"
+            "reject-reason": "bip125-replacement-disallowed",
+            "reject-details": "bip125-replacement-disallowed"
         }]
         self.assert_testres_equal(self.independent_txns_hex + [replacement_tx["hex"]], testres_rbf_package)
 

test/lint/lint-assertions.py

@@ -1,54 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (c) 2018-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-# Check for assertions with obvious side effects.
-
-import sys
-import subprocess
-
-
-def git_grep(params: [], error_msg: ""):
-    try:
-        output = subprocess.check_output(["git", "grep", *params], text=True, encoding="utf8")
-        print(error_msg)
-        print(output)
-        return 1
-    except subprocess.CalledProcessError as ex1:
-        if ex1.returncode > 1:
-            raise ex1
-    return 0
-
-
-def main():
-    # Aborting the whole process is undesirable for RPC code. So nonfatal
-    # checks should be used over assert. See: src/util/check.h
-    # src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
-    exit_code = git_grep([
-        "--line-number",
-        "--extended-regexp",
-        r"\<(A|a)ss(ume|ert)\(",
-        "--",
-        "src/rpc/",
-        "src/wallet/rpc*",
-        ":(exclude)src/rpc/server.cpp",
-    ], "CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.")
-
-    # The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`,
-    # which is an unnecessary Boost dependency.
-    exit_code |= git_grep([
-        "--line-number",
-        "--extended-regexp",
-        r"BOOST_ASSERT\(",
-        "--",
-        "*.cpp",
-        "*.h",
-    ], "BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK.")
-
-    sys.exit(exit_code)
-
-
-if __name__ == "__main__":
-    main()

test/lint/test_runner/src/main.rs

@@ -48,6 +48,16 @@ fn get_linter_list() -> Vec<&'static Linter> {
             name: "std_filesystem",
             lint_fn: lint_std_filesystem
         },
+        &Linter {
+            description: "Check that fatal assertions are not used in RPC code",
+            name: "rpc_assert",
+            lint_fn: lint_rpc_assert
+        },
+        &Linter {
+            description: "Check that boost assertions are not used",
+            name: "boost_assert",
+            lint_fn: lint_boost_assert
+        },
         &Linter {
             description: "Check that release note snippets are in the right folder",
             name: "doc_release_note_snippets",
@@ -237,7 +247,7 @@ fn lint_py_lint() -> LintResult {
             "F822", // undefined name name in __all__
             "F823", // local variable name … referenced before assignment
             "F841", // local variable 'foo' is assigned to but never used
-            "PLE", // Pylint errors
+            "PLE",  // Pylint errors
             "W191", // indentation contains tabs
             "W291", // trailing whitespace
             "W292", // no newline at end of file
@@ -273,6 +283,7 @@ fn lint_std_filesystem() -> LintResult {
     let found = git()
         .args([
             "grep",
+            "--line-number",
             "std::filesystem",
             "--",
             "./src/",
@@ -283,10 +294,66 @@ fn lint_std_filesystem() -> LintResult {
         .success();
     if found {
         Err(r#"
-^^^
 Direct use of std::filesystem may be dangerous and buggy. Please include <util/fs.h> and use the
 fs:: namespace, which has unsafe filesystem functions marked as deleted.
             "#
+        .trim()
+        .to_string())
+    } else {
+        Ok(())
+    }
+}
+
+fn lint_rpc_assert() -> LintResult {
+    let found = git()
+        .args([
+            "grep",
+            "--line-number",
+            "--extended-regexp",
+            r"\<(A|a)ss(ume|ert)\(",
+            "--",
+            "src/rpc/",
+            "src/wallet/rpc*",
+            ":(exclude)src/rpc/server.cpp",
+            // src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
+        ])
+        .status()
+        .expect("command error")
+        .success();
+    if found {
+        Err(r#"
+CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.
+
+Aborting the whole process is undesirable for RPC code. So nonfatal
+checks should be used over assert. See: src/util/check.h
+            "#
+        .trim()
+        .to_string())
+    } else {
+        Ok(())
+    }
+}
+
+fn lint_boost_assert() -> LintResult {
+    let found = git()
+        .args([
+            "grep",
+            "--line-number",
+            "--extended-regexp",
+            r"BOOST_ASSERT\(",
+            "--",
+            "*.cpp",
+            "*.h",
+        ])
+        .status()
+        .expect("command error")
+        .success();
+    if found {
+        Err(r#"
+BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK to avoid an unnecessary
+include of the boost/assert.hpp dependency.
+            "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -303,17 +370,15 @@ fn lint_doc_release_note_snippets() -> LintResult {
     if non_release_notes.is_empty() {
         Ok(())
     } else {
-        Err(format!(
-            r#"
-{}
-^^^
+        println!("{non_release_notes}");
+        Err(r#"
 Release note snippets and other docs must be put into the doc/ folder directly.
 
 The doc/release-notes/ folder is for archived release notes of previous releases only. Snippets are
 expected to follow the naming "/doc/release-notes-<PR number>.md".
-            "#,
-            non_release_notes
-        ))
+            "#
+        .trim()
+        .to_string())
     }
 }
 
@@ -356,7 +421,6 @@ fn lint_trailing_whitespace() -> LintResult {
         .success();
     if trailing_space {
         Err(r#"
-^^^
 Trailing whitespace (including Windows line endings [CR LF]) is problematic, because git may warn
 about it, or editors may remove it by default, forcing developers in the future to either undo the
 changes manually or spend time on review.
@@ -366,6 +430,7 @@ Thus, it is best to remove the trailing space now.
 Please add any false positives, such as subtrees, Windows-related files, patch files, or externally
 sourced files to the exclude list.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -382,7 +447,6 @@ fn lint_tabs_whitespace() -> LintResult {
         .success();
     if tabs {
         Err(r#"
-^^^
 Use of tabs in this codebase is problematic, because existing code uses spaces and tabs will cause
 display issues and conflict with editor settings.
 
@@ -390,6 +454,7 @@ Please remove the tabs.
 
 Please add any false positives, such as subtrees, or externally sourced files to the exclude list.
             "#
+        .trim()
         .to_string())
     } else {
         Ok(())
@@ -464,7 +529,6 @@ fn lint_includes_build_config() -> LintResult {
     if missing {
         return Err(format!(
             r#"
-^^^
 One or more files use a symbol declared in the bitcoin-build-config.h header. However, they are not
 including the header. This is problematic, because the header may or may not be indirectly
 included. If the indirect include were to be intentionally or accidentally removed, the build could
@@ -480,12 +544,13 @@ include again.
 #include <bitcoin-build-config.h> // IWYU pragma: keep
             "#,
             defines_regex
-        ));
+        )
+        .trim()
+        .to_string());
     }
     let redundant = print_affected_files(false);
     if redundant {
         return Err(r#"
-^^^
 None of the files use a symbol declared in the bitcoin-build-config.h header. However, they are including
 the header. Consider removing the unused include.
             "#
@@ -538,7 +603,9 @@ Markdown link errors found:
 {}
                 "#,
                 stderr
-            ))
+            )
+            .trim()
+            .to_string())
         }
         Err(e) if e.kind() == ErrorKind::NotFound => {
             println!("`mlc` was not found in $PATH, skipping markdown lint check.");
@@ -590,10 +657,9 @@ fn main() -> ExitCode {
         env::set_current_dir(&git_root).unwrap();
         if let Err(err) = (linter.lint_fn)() {
             println!(
-                "{err}\n^---- ⚠️ Failure generated from lint check '{}'!",
-                linter.name
+                "^^^\n{err}\n^---- ⚠️ Failure generated from lint check '{}' ({})!\n\n",
+                linter.name, linter.description,
             );
-            println!("{}", linter.description);
             test_failed = true;
         }
     }