From 992b1bbd5da95ee782515fb0f5674bb7a02684b2 Mon Sep 17 00:00:00 2001
From: Jadi <jadijadi@gmail.com>
Date: Wed, 14 Feb 2024 17:26:19 +0330
Subject: [PATCH 01/55] qt: keep focus on "Hide" while ModalOverlay is visible

During the initial sync, the Tab moves the focus to the widgets
of the main window, even when the ModalOverlay is visible. This
creates some weird rectangular *selections on the screen*.

This PR fixes this by keeping the focus on the "Hide" button while
the ModalOverlay is visible.

Fixes #783
---
 src/qt/modaloverlay.cpp | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp
index b09e230bce..1591baea3e 100644
--- a/src/qt/modaloverlay.cpp
+++ b/src/qt/modaloverlay.cpp
@@ -23,6 +23,7 @@ ModalOverlay::ModalOverlay(bool enable_wallet, QWidget* parent)
         parent->installEventFilter(this);
         raise();
     }
+    ui->closeButton->installEventFilter(this);
 
     blockProcessTime.clear();
     setVisible(false);
@@ -58,6 +59,11 @@ bool ModalOverlay::eventFilter(QObject * obj, QEvent * ev) {
             raise();
         }
     }
+
+    if (obj == ui->closeButton && ev->type() == QEvent::FocusOut && layerIsVisible) {
+        ui->closeButton->setFocus(Qt::OtherFocusReason);
+    }
+
     return QWidget::eventFilter(obj, ev);
 }
 
@@ -185,6 +191,10 @@ void ModalOverlay::showHide(bool hide, bool userRequested)
     m_animation.setEndValue(QPoint(0, hide ? height() : 0));
     m_animation.start(QAbstractAnimation::KeepWhenStopped);
     layerIsVisible = !hide;
+
+    if (layerIsVisible) {
+        ui->closeButton->setFocus(Qt::OtherFocusReason);
+    }
 }
 
 void ModalOverlay::closeClicked()

From 6ec1ca7c85a4009b77e149a798a331592b96ea42 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon, 12 Feb 2024 14:26:00 +0000
Subject: [PATCH 02/55] build: Fix test for SSE4.1 intrinsics

This change uses the `_mm_blend_epi16` SSE4.1 function used in our code
and fixes false-positive cases, for example, when CXXFLAGS="-mno-sse4.1"
provided.
---
 configure.ac | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4f71515873..2c73e0920c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -504,11 +504,12 @@ TEMP_CXXFLAGS="$CXXFLAGS"
 CXXFLAGS="$SSE41_CXXFLAGS $CXXFLAGS"
 AC_MSG_CHECKING([for SSE4.1 intrinsics])
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
-    #include <stdint.h>
     #include <immintrin.h>
   ]],[[
-    __m128i l = _mm_set1_epi32(0);
-    return _mm_extract_epi32(l, 3);
+    __m128i a = _mm_set1_epi32(0);
+    __m128i b = _mm_set1_epi32(1);
+    __m128i r = _mm_blend_epi16(a, b, 0xFF);
+    return _mm_extract_epi32(r, 3);
   ]])],
  [ AC_MSG_RESULT([yes]); enable_sse41=yes; AC_DEFINE([ENABLE_SSE41], [1], [Define this symbol to build code that uses SSE4.1 intrinsics]) ],
  [ AC_MSG_RESULT([no])]

From d440f13db02c82c842000abe4fe4d0c721a4ad3b Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon, 12 Feb 2024 14:26:16 +0000
Subject: [PATCH 03/55] crypto: Guard code with `ENABLE_SSE41` macro

The code in `sha_x86_shani.cpp` uses the `_mm_blend_epi16` function from
the SSE4.1 instruction set. However, it is possible that SHA-NI is
enabled even when SSE4.1 is disabled.

This changes avoid compilation errors in such a condition.
---
 src/Makefile.am                 | 10 +++++-----
 src/crypto/sha256.cpp           |  2 +-
 src/crypto/sha256_x86_shani.cpp |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b5d5c4652a..9c1d890f5e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -53,15 +53,15 @@ LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE)
 if ENABLE_SSE41
 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la
 LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41)
+if ENABLE_X86_SHANI
+LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la
+LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI)
+endif
 endif
 if ENABLE_AVX2
 LIBBITCOIN_CRYPTO_AVX2 = crypto/libbitcoin_crypto_avx2.la
 LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_AVX2)
 endif
-if ENABLE_X86_SHANI
-LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la
-LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI)
-endif
 if ENABLE_ARM_SHANI
 LIBBITCOIN_CRYPTO_ARM_SHANI = crypto/libbitcoin_crypto_arm_shani.la
 LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_ARM_SHANI)
@@ -612,7 +612,7 @@ crypto_libbitcoin_crypto_x86_shani_la_LDFLAGS = $(AM_LDFLAGS) -static
 crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static
 crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS = $(AM_CPPFLAGS)
 crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS += $(X86_SHANI_CXXFLAGS)
-crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_X86_SHANI
+crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_SSE41 -DENABLE_X86_SHANI
 crypto_libbitcoin_crypto_x86_shani_la_SOURCES = crypto/sha256_x86_shani.cpp
 
 # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and
diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp
index 4c7bb6f20f..bb98cd09d2 100644
--- a/src/crypto/sha256.cpp
+++ b/src/crypto/sha256.cpp
@@ -623,7 +623,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem
         }
     }
 
-#if defined(ENABLE_X86_SHANI)
+#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI)
     if (have_x86_shani) {
         Transform = sha256_x86_shani::Transform;
         TransformD64 = TransformD64Wrapper<sha256_x86_shani::Transform>;
diff --git a/src/crypto/sha256_x86_shani.cpp b/src/crypto/sha256_x86_shani.cpp
index 79871bfcc1..7471828193 100644
--- a/src/crypto/sha256_x86_shani.cpp
+++ b/src/crypto/sha256_x86_shani.cpp
@@ -6,7 +6,7 @@
 // Written and placed in public domain by Jeffrey Walton.
 // Based on code from Intel, and by Sean Gulley for the miTLS project.
 
-#ifdef ENABLE_X86_SHANI
+#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI)
 
 #include <stdint.h>
 #include <immintrin.h>

From 080089567ca766d4c1cde8ec0e5c7df48f566e07 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Mon, 20 Nov 2023 10:34:03 +0100
Subject: [PATCH 04/55] bench: add benchmark for `SignTransaction`

---
 src/Makefile.bench.include     |  1 +
 src/bench/sign_transaction.cpp | 70 ++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100644 src/bench/sign_transaction.cpp

diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 7ba0111fa6..204a1cc162 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -52,6 +52,7 @@ bench_bench_bitcoin_SOURCES = \
   bench/rollingbloom.cpp \
   bench/rpc_blockchain.cpp \
   bench/rpc_mempool.cpp \
+  bench/sign_transaction.cpp \
   bench/streams_findbyte.cpp \
   bench/strencodings.cpp \
   bench/util_time.cpp \
diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp
new file mode 100644
index 0000000000..7a2e26e339
--- /dev/null
+++ b/src/bench/sign_transaction.cpp
@@ -0,0 +1,70 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <addresstype.h>
+#include <coins.h>
+#include <key.h>
+#include <primitives/transaction.h>
+#include <pubkey.h>
+#include <script/interpreter.h>
+#include <script/script.h>
+#include <script/sign.h>
+#include <uint256.h>
+#include <util/translation.h>
+
+enum class InputType {
+    P2WPKH, // segwitv0, witness-pubkey-hash (ECDSA signature)
+    P2TR,   // segwitv1, taproot key-path spend (Schnorr signature)
+};
+
+static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_type)
+{
+    ECC_Context ecc_context{};
+
+    FlatSigningProvider keystore;
+    std::vector<CScript> prev_spks;
+
+    // Create a bunch of keys / UTXOs to avoid signing with the same key repeatedly
+    for (int i = 0; i < 32; i++) {
+        CKey privkey = GenerateRandomKey();
+        CPubKey pubkey = privkey.GetPubKey();
+        CKeyID key_id = pubkey.GetID();
+        keystore.keys.emplace(key_id, privkey);
+        keystore.pubkeys.emplace(key_id, pubkey);
+
+        // Create specified locking script type
+        CScript prev_spk;
+        switch (input_type) {
+        case InputType::P2WPKH: prev_spk = GetScriptForDestination(WitnessV0KeyHash(pubkey)); break;
+        case InputType::P2TR:   prev_spk = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); break;
+        default: assert(false);
+        }
+        prev_spks.push_back(prev_spk);
+    }
+
+    // Simple 1-input tx with artificial outpoint
+    // (note that for the purpose of signing with SIGHASH_ALL we don't need any outputs)
+    COutPoint prevout{/*hashIn=*/Txid::FromUint256(uint256::ONE), /*nIn=*/1337};
+    CMutableTransaction unsigned_tx;
+    unsigned_tx.vin.emplace_back(prevout);
+
+    // Benchmark.
+    int iter = 0;
+    bench.minEpochIterations(100).run([&] {
+        CMutableTransaction tx{unsigned_tx};
+        std::map<COutPoint, Coin> coins;
+        CScript prev_spk = prev_spks[(iter++) % prev_spks.size()];
+        coins[prevout] = Coin(CTxOut(10000, prev_spk), /*nHeightIn=*/100, /*fCoinBaseIn=*/false);
+        std::map<int, bilingual_str> input_errors;
+        bool complete = SignTransaction(tx, &keystore, coins, SIGHASH_ALL, input_errors);
+        assert(complete);
+    });
+}
+
+static void SignTransactionECDSA(benchmark::Bench& bench)   { SignTransactionSingleInput(bench, InputType::P2WPKH); }
+static void SignTransactionSchnorr(benchmark::Bench& bench) { SignTransactionSingleInput(bench, InputType::P2TR);   }
+
+BENCHMARK(SignTransactionECDSA, benchmark::PriorityLevel::HIGH);
+BENCHMARK(SignTransactionSchnorr, benchmark::PriorityLevel::HIGH);

From fe92c15f0c44d1405b9048306736bd0eae868506 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Tue, 21 Nov 2023 16:47:35 +0100
Subject: [PATCH 05/55] script/sign: avoid duplicated signature verification
 after signing

`ProduceSignature` already calls `VerifyScript` internally as last step in
order to check whether the signature data is complete. If and only if that is
the case, the `complete` field of the `SignatureData` is set accordingly and
there is no need then to verify the script after again, as we already know that
it would succeed.

This leads to a rough ~20% speed-up for `SignTransaction` for single-input
ECDSA or Taproot inputs, according to the `SignTransaction{ECDSA,Taproot}`
benchmarks.
---
 src/script/sign.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 22ac062a63..6e26ec11e0 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -831,7 +831,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore,
         }
 
         ScriptError serror = SCRIPT_ERR_OK;
-        if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) {
+        if (!sigdata.complete && !VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) {
             if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
                 // Unable to sign input and verification failed (possible attempt to partially sign).
                 input_errors[i] = Untranslated("Unable to sign input, invalid stack size (possibly missing key)");

From 81d4dc8e8739df0e9a8e92f55071733f6500617b Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Mon, 9 Jan 2023 13:40:40 +0000
Subject: [PATCH 06/55] build: use -no_exported_symbols on macOS

This reduces the size of the binary by 2-3% when building with
`--enable-reduce-exports`.
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index ab369cc98a..1f0402c885 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1316,6 +1316,7 @@ if test "$use_reduce_exports" = "yes"; then
   AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [CORE_CXXFLAGS="$CORE_CXXFLAGS -fvisibility=hidden"],
   [AC_MSG_ERROR([Cannot set hidden symbol visibility. Use --disable-reduce-exports.])], [$CXXFLAG_WERROR])
   AX_CHECK_LINK_FLAG([-Wl,--exclude-libs,ALL], [RELDFLAGS="-Wl,--exclude-libs,ALL"], [], [$LDFLAG_WERROR])
+  AX_CHECK_LINK_FLAG([-Wl,-no_exported_symbols], [LIBTOOL_APP_LDFLAGS="$LIBTOOL_APP_LDFLAGS -Wl,-no_exported_symbols"], [], [$LDFLAG_WERROR])
 fi
 
 if test "$use_tests" = "yes"; then

From 7f61d31a5cec8fc61328bee43f90d3f1dcb0a035 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Thu, 29 Feb 2024 10:53:44 +0100
Subject: [PATCH 07/55] [refactor]: update coin selection algorithms input
 parameter `max_weight` name

- This commit renames the coin selection algorithms input parameter `max_weight`
  to `max_selection_weight` for clarity.

  The parameter represent the maximum weight of the UTXOs the coin selection algorithm
  should select, not the transaction maximum weight.

- The commit updates the parameter docstring to provide correct description.

- Also updates coin selection unit and fuzzing test variables to match the new name.
---
 src/wallet/coinselection.cpp           | 26 +++++++-------
 src/wallet/coinselection.h             | 10 +++---
 src/wallet/spend.cpp                   | 14 ++++----
 src/wallet/test/coinselector_tests.cpp | 50 +++++++++++++-------------
 src/wallet/test/fuzz/coinselection.cpp | 10 +++---
 5 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index f1706b6800..c37d42d22a 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -84,14 +84,14 @@ struct {
  *        bound of the range.
  * @param const CAmount& cost_of_change This is the cost of creating and spending a change output.
  *        This plus selection_target is the upper bound of the range.
- * @param int max_weight The maximum weight available for the input set.
+ * @param int max_selection_weight The maximum allowed weight for a selection result to be valid.
  * @returns The result of this coin selection algorithm, or std::nullopt
  */
 
 static const size_t TOTAL_TRIES = 100000;
 
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
-                                             int max_weight)
+                                             int max_selection_weight)
 {
     SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     CAmount curr_value = 0;
@@ -128,7 +128,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
             curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch
             (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing
             backtrack = true;
-        } else if (curr_selection_weight > max_weight) { // Exceeding weight for standard tx, cannot find more solutions by adding more inputs
+        } else if (curr_selection_weight > max_selection_weight) { // Selected UTXOs weight exceeds the maximum weight allowed, cannot find more solutions by adding more inputs
             max_tx_weight_exceeded = true; // at least one selection attempt exceeded the max weight
             backtrack = true;
         } else if (curr_value >= selection_target) {       // Selected value is within range
@@ -319,10 +319,10 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
  *        group with multiple as a heavier UTXO with the combined amount here.)
  * @param const CAmount& selection_target This is the minimum amount that we need for the transaction without considering change.
  * @param const CAmount& change_target The minimum budget for creating a change output, by which we increase the selection_target.
- * @param int max_weight The maximum permitted weight for the input set.
+ * @param int max_selection_weight The maximum allowed weight for a selection result to be valid.
  * @returns The result of this coin selection algorithm, or std::nullopt
  */
-util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight)
+util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight)
 {
     std::sort(utxo_pool.begin(), utxo_pool.end(), descending_effval_weight);
     // The sum of UTXO amounts after this UTXO index, e.g. lookahead[5] = Σ(UTXO[6+].amount)
@@ -359,7 +359,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
 
     // The weight of the currently selected input set, and the weight of the best selection
     int curr_weight = 0;
-    int best_selection_weight = max_weight; // Tie is fine, because we prefer lower selection amount
+    int best_selection_weight = max_selection_weight; // Tie is fine, because we prefer lower selection amount
 
     // Whether the input sets generated during this search have exceeded the maximum transaction weight at any point
     bool max_tx_weight_exceeded = false;
@@ -436,8 +436,8 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
             // Insufficient funds with lookahead: CUT
             should_cut = true;
         } else if (curr_weight > best_selection_weight) {
-            // best_selection_weight is initialized to max_weight
-            if (curr_weight > max_weight) max_tx_weight_exceeded = true;
+            // best_selection_weight is initialized to max_selection_weight
+            if (curr_weight > max_selection_weight) max_tx_weight_exceeded = true;
             // Worse weight than best solution. More UTXOs only increase weight:
             // CUT if last selected group had minimal weight, else SHIFT
             if (utxo_pool[curr_tail].m_weight <= min_tail_weight[curr_tail]) {
@@ -535,7 +535,7 @@ class MinOutputGroupComparator
 };
 
 util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
-                                             int max_weight)
+                                             int max_selection_weight)
 {
     SelectionResult result(target_value, SelectionAlgorithm::SRD);
     std::priority_queue<OutputGroup, std::vector<OutputGroup>, MinOutputGroupComparator> heap;
@@ -565,14 +565,14 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx
 
         // If the selection weight exceeds the maximum allowed size, remove the least valuable inputs until we
         // are below max weight.
-        if (weight > max_weight) {
+        if (weight > max_selection_weight) {
             max_tx_weight_exceeded = true; // mark it in case we don't find any useful result.
             do {
                 const OutputGroup& to_remove_group = heap.top();
                 selected_eff_value -= to_remove_group.GetSelectionAmount();
                 weight -= to_remove_group.m_weight;
                 heap.pop();
-            } while (!heap.empty() && weight > max_weight);
+            } while (!heap.empty() && weight > max_selection_weight);
         }
 
         // Now check if we are above the target
@@ -648,7 +648,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
 }
 
 util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
-                                             CAmount change_target, FastRandomContext& rng, int max_weight)
+                                             CAmount change_target, FastRandomContext& rng, int max_selection_weight)
 {
     SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK);
 
@@ -709,7 +709,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
         }
 
         // If the result exceeds the maximum allowed size, return closest UTXO above the target
-        if (result.GetWeight() > max_weight) {
+        if (result.GetWeight() > max_selection_weight) {
             // No coin above target, nothing to do.
             if (!lowest_larger) return ErrorMaxWeightExceeded();
 
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 9fb000422c..6b0d1ab2a4 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -440,9 +440,9 @@ struct SelectionResult
 };
 
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
-                                             int max_weight);
+                                             int max_selection_weight);
 
-util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight);
+util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight);
 
 /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible
  * outputs until the target is satisfied
@@ -450,15 +450,15 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
  * @param[in]  utxo_pool    The positive effective value OutputGroups eligible for selection
  * @param[in]  target_value The target value to select for
  * @param[in]  rng The randomness source to shuffle coins
- * @param[in]  max_weight The maximum allowed weight for a selection result to be valid
+ * @param[in]  max_selection_weight The maximum allowed weight for a selection result to be valid
  * @returns If successful, a valid SelectionResult, otherwise, util::Error
  */
 util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng,
-                                             int max_weight);
+                                             int max_selection_weight);
 
 // Original coin selection algorithm as a fallback
 util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
-                                             CAmount change_target, FastRandomContext& rng, int max_weight);
+                                             CAmount change_target, FastRandomContext& rng, int max_selection_weight);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_COINSELECTION_H
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index c16b8a9d4f..a39a4ca0d1 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -695,26 +695,26 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
         }
     };
 
-    // Maximum allowed weight
-    int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
+    // Maximum allowed weight for selected coins.
+    int max_selection_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
 
     // SFFO frequently causes issues in the context of changeless input sets: skip BnB when SFFO is active
     if (!coin_selection_params.m_subtract_fee_outputs) {
-        if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) {
+        if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_selection_weight)}) {
             results.push_back(*bnb_result);
         } else append_error(std::move(bnb_result));
     }
 
     // As Knapsack and SRD can create change, also deduce change weight.
-    max_inputs_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
+    max_selection_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
 
     // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.
-    if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) {
+    if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_selection_weight)}) {
         results.push_back(*knapsack_result);
     } else append_error(std::move(knapsack_result));
 
     if (coin_selection_params.m_effective_feerate > CFeeRate{3 * coin_selection_params.m_long_term_feerate}) { // Minimize input set for feerates of at least 3×LTFRE (default: 30 ṩ/vB+)
-        if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_inputs_weight)}) {
+        if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_selection_weight)}) {
             cg_result->RecalculateWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
             results.push_back(*cg_result);
         } else {
@@ -722,7 +722,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
         }
     }
 
-    if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) {
+    if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_selection_weight)}) {
         results.push_back(*srd_result);
     } else append_error(std::move(srd_result));
 
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 7bd92b471c..eb9c349c22 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -1097,13 +1097,13 @@ BOOST_AUTO_TEST_CASE(effective_value_test)
 static util::Result<SelectionResult> CoinGrinder(const CAmount& target,
                                                     const CoinSelectionParams& cs_params,
                                                     const node::NodeContext& m_node,
-                                                    int max_weight,
+                                                    int max_selection_weight,
                                                     std::function<CoinsResult(CWallet&)> coin_setup)
 {
     std::unique_ptr<CWallet> wallet = NewWallet(m_node);
     CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors
     Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups;
-    return CoinGrinder(group.positive_group, target, cs_params.m_min_change_target, max_weight);
+    return CoinGrinder(group.positive_group, target, cs_params.m_min_change_target, max_selection_weight);
 }
 
 BOOST_AUTO_TEST_CASE(coin_grinder_tests)
@@ -1135,8 +1135,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 1) Insufficient funds, select all provided coins and fail
         // #########################################################
         CAmount target = 49.5L * COIN;
-        int max_weight = 10'000; // high enough to not fail for this reason.
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 10'000; // high enough to not fail for this reason.
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 10; ++j) {
                 add_coin(available_coins, wallet, CAmount(1 * COIN));
@@ -1153,8 +1153,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 2) Test max weight exceeded
         // ###########################
         CAmount target = 29.5L * COIN;
-        int max_weight = 3000;
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 3000;
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 10; ++j) {
                 add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true);
@@ -1171,8 +1171,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 3) Test selection when some coins surpass the max allowed weight while others not. --> must find a good solution
         // ################################################################################################################
         CAmount target = 25.33L * COIN;
-        int max_weight = 10'000; // WU
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 10'000; // WU
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 60; ++j) { // 60 UTXO --> 19,8 BTC total --> 60 × 272 WU = 16320 WU
                 add_coin(available_coins, wallet, CAmount(0.33 * COIN), CFeeRate(5000), 144, false, 0, true);
@@ -1193,8 +1193,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 4) Test that two less valuable UTXOs with a combined lower weight are preferred over a more valuable heavier UTXO
         // #################################################################################################################
         CAmount target =  1.9L * COIN;
-        int max_weight = 400'000; // WU
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 400'000; // WU
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             add_coin(available_coins, wallet, CAmount(2 * COIN), CFeeRate(5000), 144, false, 0, true, 148);
             add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true, 68);
@@ -1215,8 +1215,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 5) Test finding a solution in a UTXO pool with mixed weights
         // ################################################################################################################
         CAmount target = 30L * COIN;
-        int max_weight = 400'000; // WU
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 400'000; // WU
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 5; ++j) {
                 // Add heavy coins {3, 6, 9, 12, 15}
@@ -1244,8 +1244,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 6) Test that the lightest solution among many clones is found
         // #################################################################################################################
         CAmount target =  9.9L * COIN;
-        int max_weight = 400'000; // WU
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 400'000; // WU
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             // Expected Result: 4 + 3 + 2 + 1 = 10 BTC at 400 vB
             add_coin(available_coins, wallet, CAmount(4 * COIN), CFeeRate(5000), 144, false, 0, true, 100);
@@ -1283,8 +1283,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
         // 7) Test that lots of tiny UTXOs can be skipped if they are too heavy while there are enough funds in lookahead
         // #################################################################################################################
         CAmount target =  1.9L * COIN;
-        int max_weight = 40000; // WU
-        const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 40000; // WU
+        const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             add_coin(available_coins, wallet, CAmount(1.8 * COIN), CFeeRate(5000), 144, false, 0, true, 2500);
             add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true, 1000);
@@ -1308,13 +1308,13 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
 static util::Result<SelectionResult> SelectCoinsSRD(const CAmount& target,
                                                     const CoinSelectionParams& cs_params,
                                                     const node::NodeContext& m_node,
-                                                    int max_weight,
+                                                    int max_selection_weight,
                                                     std::function<CoinsResult(CWallet&)> coin_setup)
 {
     std::unique_ptr<CWallet> wallet = NewWallet(m_node);
     CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors
     Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups;
-    return SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_weight);
+    return SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_selection_weight);
 }
 
 BOOST_AUTO_TEST_CASE(srd_tests)
@@ -1342,8 +1342,8 @@ BOOST_AUTO_TEST_CASE(srd_tests)
         // 1) Insufficient funds, select all provided coins and fail
         // #########################################################
         CAmount target = 49.5L * COIN;
-        int max_weight = 10000; // high enough to not fail for this reason.
-        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 10000; // high enough to not fail for this reason.
+        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 10; ++j) {
                 add_coin(available_coins, wallet, CAmount(1 * COIN));
@@ -1360,8 +1360,8 @@ BOOST_AUTO_TEST_CASE(srd_tests)
         // 2) Test max weight exceeded
         // ###########################
         CAmount target = 49.5L * COIN;
-        int max_weight = 3000;
-        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 3000;
+        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 10; ++j) {
                 /* 10 × 1 BTC + 10 × 2 BTC = 30 BTC. 20 × 272 WU = 5440 WU */
@@ -1379,8 +1379,8 @@ BOOST_AUTO_TEST_CASE(srd_tests)
         // 3) Test selection when some coins surpass the max allowed weight while others not. --> must find a good solution
         // ################################################################################################################
         CAmount target = 25.33L * COIN;
-        int max_weight = 10000; // WU
-        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) {
+        int max_selection_weight = 10000; // WU
+        const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
             CoinsResult available_coins;
             for (int j = 0; j < 60; ++j) { // 60 UTXO --> 19,8 BTC total --> 60 × 272 WU = 16320 WU
                 add_coin(available_coins, wallet, CAmount(0.33 * COIN), CFeeRate(0), 144, false, 0, true);
@@ -1415,7 +1415,7 @@ static bool has_coin(const CoinSet& set, CAmount amount)
     return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; });
 }
 
-BOOST_AUTO_TEST_CASE(check_max_weight)
+BOOST_AUTO_TEST_CASE(check_max_selection_weight)
 {
     const CAmount target = 49.5L * COIN;
     CCoinControl cc;
diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp
index 644a8dd7ad..72975c3d68 100644
--- a/src/wallet/test/fuzz/coinselection.cpp
+++ b/src/wallet/test/fuzz/coinselection.cpp
@@ -195,11 +195,11 @@ FUZZ_TARGET(coin_grinder_is_optimal)
 
     if (best_weight < std::numeric_limits<int>::max()) {
         // Sufficient funds and acceptable weight: CoinGrinder should find at least one solution
-        int high_max_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(best_weight, std::numeric_limits<int>::max());
+        int high_max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(best_weight, std::numeric_limits<int>::max());
 
-        auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, high_max_weight);
+        auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, high_max_selection_weight);
         assert(result_cg);
-        assert(result_cg->GetWeight() <= high_max_weight);
+        assert(result_cg->GetWeight() <= high_max_selection_weight);
         assert(result_cg->GetSelectedEffectiveValue() >= target + coin_params.m_min_change_target);
         assert(best_weight < result_cg->GetWeight() || (best_weight == result_cg->GetWeight() && best_amount <= result_cg->GetSelectedEffectiveValue()));
         if (result_cg->GetAlgoCompleted()) {
@@ -210,8 +210,8 @@ FUZZ_TARGET(coin_grinder_is_optimal)
     }
 
     // CoinGrinder cannot ever find a better solution than the brute-forced best, or there is none in the first place
-    int low_max_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, best_weight - 1);
-    auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, low_max_weight);
+    int low_max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, best_weight - 1);
+    auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, low_max_selection_weight);
     // Max_weight should have been exceeded, or there were insufficient funds
     assert(!result_cg);
 }

From baab0d2d43049a71dc90176bc4d72062f7b2ce19 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Thu, 29 Feb 2024 10:59:50 +0100
Subject: [PATCH 08/55] [doc]: update reason for deducting change output weight

`CoinGrinder` will also produce change output, listing all the
Coin selection algorithms that produces change output is not maintainable,
just infer that remaining algorithms all might produce change.
---
 src/wallet/spend.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index a39a4ca0d1..4759f1eb95 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -705,7 +705,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
         } else append_error(std::move(bnb_result));
     }
 
-    // As Knapsack and SRD can create change, also deduce change weight.
+    // Deduct change weight because remaining Coin Selection algorithms can create change output
     max_selection_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
 
     // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.

From b6fc5043c16c2467a2a6768a6ca9b18035fc400f Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Thu, 21 Mar 2024 17:05:51 +0100
Subject: [PATCH 09/55] [wallet]: update the data type of `change_output_size`,
 `change_spend_size` and `tx_noinputs_size` to `int`

- This change ensures consistency in transaction size and weight calculation
  within the wallet and prevents conversion overflow when calculating
  `max_selection_weight`.
---
 src/wallet/coinselection.h | 10 +++++-----
 src/wallet/spend.cpp       |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 6b0d1ab2a4..1f234427d2 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -139,9 +139,9 @@ struct CoinSelectionParams {
     /** Randomness to use in the context of coin selection. */
     FastRandomContext& rng_fast;
     /** Size of a change output in bytes, determined by the output type. */
-    size_t change_output_size = 0;
+    int change_output_size = 0;
     /** Size of the input to spend a change output in virtual bytes. */
-    size_t change_spend_size = 0;
+    int change_spend_size = 0;
     /** Mininmum change to target in Knapsack solver and CoinGrinder:
      * select coins to cover the payment and at least this value of change. */
     CAmount m_min_change_target{0};
@@ -162,7 +162,7 @@ struct CoinSelectionParams {
     CFeeRate m_discard_feerate;
     /** Size of the transaction before coin selection, consisting of the header and recipient
      * output(s), excluding the inputs and change output(s). */
-    size_t tx_noinputs_size = 0;
+    int tx_noinputs_size = 0;
     /** Indicate that we are subtracting the fee from outputs */
     bool m_subtract_fee_outputs = false;
     /** When true, always spend all (up to OUTPUT_GROUP_MAX_ENTRIES) or none of the outputs
@@ -175,9 +175,9 @@ struct CoinSelectionParams {
      */
     bool m_include_unsafe_inputs = false;
 
-    CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size,
+    CoinSelectionParams(FastRandomContext& rng_fast, int change_output_size, int change_spend_size,
                         CAmount min_change_target, CFeeRate effective_feerate,
-                        CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial)
+                        CFeeRate long_term_feerate, CFeeRate discard_feerate, int tx_noinputs_size, bool avoid_partial)
         : rng_fast{rng_fast},
           change_output_size(change_output_size),
           change_spend_size(change_spend_size),
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 4759f1eb95..e0546c6210 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -1059,7 +1059,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     if (change_spend_size == -1) {
         coin_selection_params.change_spend_size = DUMMY_NESTED_P2WPKH_INPUT_SIZE;
     } else {
-        coin_selection_params.change_spend_size = (size_t)change_spend_size;
+        coin_selection_params.change_spend_size = change_spend_size;
     }
 
     // Set discard feerate

From 734076c6de1781f957c8bc3bf7ed6951920cfcf6 Mon Sep 17 00:00:00 2001
From: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Date: Thu, 29 Feb 2024 12:43:52 +0100
Subject: [PATCH 10/55] [wallet, rpc]: add `max_tx_weight` to tx funding
 options

This allows a transaction's weight to be bound under a certain
weight if possible and desired. This can be beneficial for future
RBF attempts, or whenever a more restricted spend topology is
desired.

Co-authored-by: Greg Sanders <gsanders87@gmail.com>
---
 src/rpc/client.cpp                           |  3 ++
 src/wallet/coincontrol.h                     |  2 +
 src/wallet/coinselection.cpp                 | 31 ++++++++++----
 src/wallet/coinselection.h                   |  8 +++-
 src/wallet/rpc/spend.cpp                     | 14 ++++++
 src/wallet/spend.cpp                         | 21 +++++++--
 src/wallet/test/fuzz/coinselection.cpp       | 11 +++--
 test/functional/rpc_psbt.py                  | 45 ++++++++++++++++++++
 test/functional/test_framework/blocktools.py |  1 +
 9 files changed, 119 insertions(+), 17 deletions(-)

diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 7dfe69a6c5..b866fa484b 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -146,6 +146,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "fundrawtransaction", 1, "conf_target"},
     { "fundrawtransaction", 1, "replaceable"},
     { "fundrawtransaction", 1, "solving_data"},
+    { "fundrawtransaction", 1, "max_tx_weight"},
     { "fundrawtransaction", 2, "iswitness" },
     { "walletcreatefundedpsbt", 0, "inputs" },
     { "walletcreatefundedpsbt", 1, "outputs" },
@@ -164,6 +165,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "walletcreatefundedpsbt", 3, "conf_target"},
     { "walletcreatefundedpsbt", 3, "replaceable"},
     { "walletcreatefundedpsbt", 3, "solving_data"},
+    { "walletcreatefundedpsbt", 3, "max_tx_weight"},
     { "walletcreatefundedpsbt", 4, "bip32derivs" },
     { "walletprocesspsbt", 1, "sign" },
     { "walletprocesspsbt", 3, "bip32derivs" },
@@ -208,6 +210,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "send", 4, "conf_target"},
     { "send", 4, "replaceable"},
     { "send", 4, "solving_data"},
+    { "send", 4, "max_tx_weight"},
     { "sendall", 0, "recipients" },
     { "sendall", 1, "conf_target" },
     { "sendall", 3, "fee_rate"},
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index b2f813383d..d36314312a 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -115,6 +115,8 @@ class CCoinControl
     std::optional<uint32_t> m_locktime;
     //! Version
     std::optional<uint32_t> m_version;
+    //! Caps weight of resulting tx
+    std::optional<int> m_max_tx_weight{std::nullopt};
 
     CCoinControl();
 
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index c37d42d22a..a54d1a4668 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -597,11 +597,12 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx
  *                              nTargetValue, with indices corresponding to groups. If the ith
  *                              entry is true, that means the ith group in groups was selected.
  * param@[out]  nBest           Total amount of subset chosen that is closest to nTargetValue.
+ * paramp[in]   max_selection_weight  The maximum allowed weight for a selection result to be valid.
  * param@[in]   iterations      Maximum number of tries.
  */
 static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups,
                                   const CAmount& nTotalLower, const CAmount& nTargetValue,
-                                  std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000)
+                                  std::vector<char>& vfBest, CAmount& nBest, int max_selection_weight, int iterations = 1000)
 {
     std::vector<char> vfIncluded;
 
@@ -613,6 +614,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
     {
         vfIncluded.assign(groups.size(), false);
         CAmount nTotal = 0;
+        int selected_coins_weight{0};
         bool fReachedTarget = false;
         for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++)
         {
@@ -627,9 +629,9 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
                 if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i])
                 {
                     nTotal += groups[i].GetSelectionAmount();
+                    selected_coins_weight += groups[i].m_weight;
                     vfIncluded[i] = true;
-                    if (nTotal >= nTargetValue)
-                    {
+                    if (nTotal >= nTargetValue && selected_coins_weight <= max_selection_weight) {
                         fReachedTarget = true;
                         // If the total is between nTargetValue and nBest, it's our new best
                         // approximation.
@@ -639,6 +641,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
                             vfBest = vfIncluded;
                         }
                         nTotal -= groups[i].GetSelectionAmount();
+                        selected_coins_weight -= groups[i].m_weight;
                         vfIncluded[i] = false;
                     }
                 }
@@ -652,6 +655,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
 {
     SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK);
 
+    bool max_weight_exceeded{false};
     // List of values less than target
     std::optional<OutputGroup> lowest_larger;
     // Groups with selection amount smaller than the target and any change we might produce.
@@ -662,6 +666,10 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
     Shuffle(groups.begin(), groups.end(), rng);
 
     for (const OutputGroup& group : groups) {
+        if (group.m_weight > max_selection_weight) {
+            max_weight_exceeded = true;
+            continue;
+        }
         if (group.GetSelectionAmount() == nTargetValue) {
             result.AddInput(group);
             return result;
@@ -677,11 +685,18 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
         for (const auto& group : applicable_groups) {
             result.AddInput(group);
         }
-        return result;
+        if (result.GetWeight() <= max_selection_weight) return result;
+        else max_weight_exceeded = true;
+
+        // Try something else
+        result.Clear();
     }
 
     if (nTotalLower < nTargetValue) {
-        if (!lowest_larger) return util::Error();
+        if (!lowest_larger) {
+            if (max_weight_exceeded) return ErrorMaxWeightExceeded();
+            return util::Error();
+        }
         result.AddInput(*lowest_larger);
         return result;
     }
@@ -691,9 +706,9 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
     std::vector<char> vfBest;
     CAmount nBest;
 
-    ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest);
+    ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest, max_selection_weight);
     if (nBest != nTargetValue && nTotalLower >= nTargetValue + change_target) {
-        ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + change_target, vfBest, nBest);
+        ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + change_target, vfBest, nBest, max_selection_weight);
     }
 
     // If we have a bigger coin and (either the stochastic approximation didn't find a good solution,
@@ -728,7 +743,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c
             LogPrint(BCLog::SELECTCOINS, "%stotal %s\n", log_message, FormatMoney(nBest));
         }
     }
-
+    Assume(result.GetWeight() <= max_selection_weight);
     return result;
 }
 
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 1f234427d2..8a81cfc268 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -174,10 +174,13 @@ struct CoinSelectionParams {
      * 1) Received from other wallets, 2) replacing other txs, 3) that have been replaced.
      */
     bool m_include_unsafe_inputs = false;
+    /** The maximum weight for this transaction. */
+    std::optional<int> m_max_tx_weight{std::nullopt};
 
     CoinSelectionParams(FastRandomContext& rng_fast, int change_output_size, int change_spend_size,
                         CAmount min_change_target, CFeeRate effective_feerate,
-                        CFeeRate long_term_feerate, CFeeRate discard_feerate, int tx_noinputs_size, bool avoid_partial)
+                        CFeeRate long_term_feerate, CFeeRate discard_feerate, int tx_noinputs_size, bool avoid_partial,
+                        std::optional<int> max_tx_weight = std::nullopt)
         : rng_fast{rng_fast},
           change_output_size(change_output_size),
           change_spend_size(change_spend_size),
@@ -186,7 +189,8 @@ struct CoinSelectionParams {
           m_long_term_feerate(long_term_feerate),
           m_discard_feerate(discard_feerate),
           tx_noinputs_size(tx_noinputs_size),
-          m_avoid_partial_spends(avoid_partial)
+          m_avoid_partial_spends(avoid_partial),
+          m_max_tx_weight(max_tx_weight)
     {
     }
     CoinSelectionParams(FastRandomContext& rng_fast)
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index ac2a4826f0..8ea51e65c7 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -542,6 +542,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
                 {"minconf", UniValueType(UniValue::VNUM)},
                 {"maxconf", UniValueType(UniValue::VNUM)},
                 {"input_weights", UniValueType(UniValue::VARR)},
+                {"max_tx_weight", UniValueType(UniValue::VNUM)},
             },
             true, true);
 
@@ -701,6 +702,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact
         }
     }
 
+    if (options.exists("max_tx_weight")) {
+        coinControl.m_max_tx_weight = options["max_tx_weight"].getInt<int>();
+    }
+
     if (recipients.empty())
         throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");
 
@@ -786,6 +791,8 @@ RPCHelpMan fundrawtransaction()
                                     },
                                 },
                              },
+                            {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n"
+                                                          "Transaction building will fail if this can not be satisfied."},
                         },
                         FundTxDoc()),
                         RPCArgOptions{
@@ -1240,6 +1247,8 @@ RPCHelpMan send()
                             {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
                         },
                     },
+                    {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n"
+                                                  "Transaction building will fail if this can not be satisfied."},
                 },
                 FundTxDoc()),
                 RPCArgOptions{.oneline_description="options"}},
@@ -1287,6 +1296,9 @@ RPCHelpMan send()
             // Automatically select coins, unless at least one is manually selected. Can
             // be overridden by options.add_inputs.
             coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
+            if (options.exists("max_tx_weight")) {
+                coin_control.m_max_tx_weight = options["max_tx_weight"].getInt<int>();
+            }
             SetOptionsInputWeights(options["inputs"], options);
             // Clear tx.vout since it is not meant to be used now that we are passing outputs directly.
             // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly
@@ -1697,6 +1709,8 @@ RPCHelpMan walletcreatefundedpsbt()
                                     {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
                                 },
                             },
+                            {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n"
+                                                          "Transaction building will fail if this can not be satisfied."},
                         },
                         FundTxDoc()),
                         RPCArgOptions{.oneline_description="options"}},
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index e0546c6210..1ad570442e 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -696,7 +696,12 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
     };
 
     // Maximum allowed weight for selected coins.
-    int max_selection_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
+    int max_transaction_weight = coin_selection_params.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT);
+    int tx_weight_no_input = coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR;
+    int max_selection_weight = max_transaction_weight - tx_weight_no_input;
+    if (max_selection_weight <= 0) {
+        return util::Error{_("Maximum transaction weight is less than transaction weight without inputs")};
+    }
 
     // SFFO frequently causes issues in the context of changeless input sets: skip BnB when SFFO is active
     if (!coin_selection_params.m_subtract_fee_outputs) {
@@ -706,7 +711,11 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
     }
 
     // Deduct change weight because remaining Coin Selection algorithms can create change output
-    max_selection_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR);
+    int change_outputs_weight = coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR;
+    max_selection_weight -= change_outputs_weight;
+    if (max_selection_weight < 0 && results.empty()) {
+        return util::Error{_("Maximum transaction weight is too low, can not accommodate change output")};
+    }
 
     // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.
     if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_selection_weight)}) {
@@ -801,7 +810,7 @@ util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& av
                                                 coin_selection_params.m_change_fee);
 
         // Verify we haven't exceeded the maximum allowed weight
-        int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
+        int max_inputs_weight = coin_selection_params.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT) - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR);
         if (op_selection_result->GetWeight() > max_inputs_weight) {
             return util::Error{_("The combination of the pre-selected inputs and the wallet automatic inputs selection exceeds the transaction maximum weight. "
                                  "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")};
@@ -1002,7 +1011,11 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy
     coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
     coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs;
-
+    coin_selection_params.m_max_tx_weight = coin_control.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT);
+    int minimum_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR;
+    if (coin_selection_params.m_max_tx_weight.value() < minimum_tx_weight || coin_selection_params.m_max_tx_weight.value() > MAX_STANDARD_TX_WEIGHT) {
+        return util::Error{strprintf(_("Maximum transaction weight must be between %d and %d"), minimum_tx_weight, MAX_STANDARD_TX_WEIGHT)};
+    }
     // Set the long term feerate estimate to the wallet's consolidate feerate
     coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate;
 
diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp
index 72975c3d68..209c87fd42 100644
--- a/src/wallet/test/fuzz/coinselection.cpp
+++ b/src/wallet/test/fuzz/coinselection.cpp
@@ -256,29 +256,34 @@ FUZZ_TARGET(coinselection)
         (void)group.EligibleForSpending(filter);
     }
 
+    int max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max());
+
     // Run coinselection algorithms
     auto result_bnb = coin_params.m_subtract_fee_outputs ? util::Error{Untranslated("BnB disabled when SFFO is enabled")} :
-                      SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, MAX_STANDARD_TX_WEIGHT);
+                      SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, max_selection_weight);
     if (result_bnb) {
         assert(result_bnb->GetChange(coin_params.min_viable_change, coin_params.m_change_fee) == 0);
         assert(result_bnb->GetSelectedValue() >= target);
+        assert(result_bnb->GetWeight() <= max_selection_weight);
         (void)result_bnb->GetShuffledInputVector();
         (void)result_bnb->GetInputSet();
     }
 
-    auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, MAX_STANDARD_TX_WEIGHT);
+    auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, max_selection_weight);
     if (result_srd) {
         assert(result_srd->GetSelectedValue() >= target);
         assert(result_srd->GetChange(CHANGE_LOWER, coin_params.m_change_fee) > 0); // Demonstrate that SRD creates change of at least CHANGE_LOWER
+        assert(result_srd->GetWeight() <= max_selection_weight);
         result_srd->RecalculateWaste(coin_params.min_viable_change, coin_params.m_cost_of_change, coin_params.m_change_fee);
         (void)result_srd->GetShuffledInputVector();
         (void)result_srd->GetInputSet();
     }
 
     CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)};
-    auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, MAX_STANDARD_TX_WEIGHT);
+    auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, max_selection_weight);
     if (result_knapsack) {
         assert(result_knapsack->GetSelectedValue() >= target);
+        assert(result_knapsack->GetWeight() <= max_selection_weight);
         result_knapsack->RecalculateWaste(coin_params.min_viable_change, coin_params.m_cost_of_change, coin_params.m_change_fee);
         (void)result_knapsack->GetShuffledInputVector();
         (void)result_knapsack->GetInputSet();
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 6ee7e56886..111ca63618 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -8,6 +8,9 @@
 from itertools import product
 from random import randbytes
 
+from test_framework.blocktools import (
+    MAX_STANDARD_TX_WEIGHT,
+)
 from test_framework.descriptors import descsum_create
 from test_framework.key import H_POINT
 from test_framework.messages import (
@@ -16,6 +19,7 @@
     CTxIn,
     CTxOut,
     MAX_BIP125_RBF_SEQUENCE,
+    WITNESS_SCALE_FACTOR,
 )
 from test_framework.psbt import (
     PSBT,
@@ -30,6 +34,7 @@
     PSBT_OUT_TAP_TREE,
 )
 from test_framework.script import CScript, OP_TRUE
+from test_framework.script_util import MIN_STANDARD_TX_NONWITNESS_SIZE
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_approx,
@@ -186,6 +191,46 @@ def run_test(self):
         # Create and fund a raw tx for sending 10 BTC
         psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt']
 
+        self.log.info("Test for invalid maximum transaction weights")
+        dest_arg = [{self.nodes[0].getnewaddress(): 1}]
+        min_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR
+        assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": -1})
+        assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": 0})
+        assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": MAX_STANDARD_TX_WEIGHT + 1})
+
+        # Base transaction vsize: version (4) + locktime (4) + input count (1) + witness overhead (1) = 10 vbytes
+        base_tx_vsize = 10
+        # One P2WPKH output vsize: outpoint (31 vbytes)
+        p2wpkh_output_vsize = 31
+        # 1 vbyte for output count
+        output_count = 1
+        tx_weight_without_inputs = (base_tx_vsize + output_count + p2wpkh_output_vsize) * WITNESS_SCALE_FACTOR
+        # min_tx_weight is greater than transaction weight without inputs
+        assert_greater_than(min_tx_weight, tx_weight_without_inputs)
+
+        # In order to test for when the passed max weight is less than the transaction weight without inputs
+        # Define destination with two outputs.
+        dest_arg_large = [{self.nodes[0].getnewaddress(): 1}, {self.nodes[0].getnewaddress(): 1}]
+        large_tx_vsize_without_inputs = base_tx_vsize + output_count + (p2wpkh_output_vsize * 2)
+        large_tx_weight_without_inputs = large_tx_vsize_without_inputs * WITNESS_SCALE_FACTOR
+        assert_greater_than(large_tx_weight_without_inputs, min_tx_weight)
+        # Test for max_tx_weight less than Transaction weight without inputs
+        assert_raises_rpc_error(-4, "Maximum transaction weight is less than transaction weight without inputs", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": min_tx_weight})
+        assert_raises_rpc_error(-4, "Maximum transaction weight is less than transaction weight without inputs", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": large_tx_weight_without_inputs})
+
+        # Test for max_tx_weight just enough to include inputs but not change output
+        assert_raises_rpc_error(-4, "Maximum transaction weight is too low, can not accommodate change output", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": (large_tx_vsize_without_inputs + 1) * WITNESS_SCALE_FACTOR})
+        self.log.info("Test that a funded PSBT is always faithful to max_tx_weight option")
+        large_tx_vsize_with_change = large_tx_vsize_without_inputs + p2wpkh_output_vsize
+        # It's enough but won't accommodate selected input size
+        assert_raises_rpc_error(-4, "The inputs size exceeds the maximum weight", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": (large_tx_vsize_with_change) * WITNESS_SCALE_FACTOR})
+
+        max_tx_weight_sufficient = 1000 # 1k vbytes is enough
+        psbt = self.nodes[0].walletcreatefundedpsbt(outputs=dest_arg,locktime=0, options={"max_tx_weight": max_tx_weight_sufficient})["psbt"]
+        weight = self.nodes[0].decodepsbt(psbt)["tx"]["weight"]
+        # ensure the transaction's weight is below the specified max_tx_weight.
+        assert_greater_than_or_equal(max_tx_weight_sufficient, weight)
+
         # If inputs are specified, do not automatically add more:
         utxo1 = self.nodes[0].listunspent()[0]
         assert_raises_rpc_error(-4, "The preselected coins total amount does not cover the transaction target. "
diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py
index f0dc866f69..338b7fa3b0 100644
--- a/test/functional/test_framework/blocktools.py
+++ b/test/functional/test_framework/blocktools.py
@@ -48,6 +48,7 @@
 
 MAX_BLOCK_SIGOPS = 20000
 MAX_BLOCK_SIGOPS_WEIGHT = MAX_BLOCK_SIGOPS * WITNESS_SCALE_FACTOR
+MAX_STANDARD_TX_WEIGHT = 400000
 
 # Genesis block time (regtest)
 TIME_GENESIS_BLOCK = 1296688602

From 39cea21ec51b9838669c38fefa14f25c36ae7096 Mon Sep 17 00:00:00 2001
From: willcl-ark <will@256k1.dev>
Date: Fri, 28 Jun 2024 12:41:57 +0100
Subject: [PATCH 11/55] wallet: fix FillPSBT errantly showing as complete

Fix cases of calls to `FillPSBT` returning `complete=true` when it's not
the case.

This can happen when some inputs have been signed but the transaction is
subsequently modified, e.g. in the context of PayJoins.

Also fixes a related bug where a finalized hex string is attempted to be
added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort.

Reported in #30077.
---
 src/wallet/wallet.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d569c64b43..fb4360cfd2 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2225,8 +2225,8 @@ std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo
 
     // Complete if every input is now signed
     complete = true;
-    for (const auto& input : psbtx.inputs) {
-        complete &= PSBTInputSigned(input);
+    for (size_t i = 0; i < psbtx.inputs.size(); ++i) {
+        complete &= PSBTInputSignedAndVerified(psbtx, i, &txdata);
     }
 
     return {};

From 9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Mon, 2 Aug 2021 18:54:06 +0200
Subject: [PATCH 12/55] net: don't extra bind for Tor if binds are restricted

If only `-bind=addr:port` is given (without `-bind=...=onion`) then we
would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which
may be unexpected, assuming the semantic of `-bind=addr:port` is
"bind _only_ to `addr:port`".

Change the above to not do the additional bind: if only
`-bind=addr:port` is given (without `-bind=...=onion`) then bind to
`addr:port` (only). If we are creating a Tor hidden service then use
`addr:port` as target (same behavior as before
https://github.com/bitcoin/bitcoin/pull/19991).

This allows disabling binding on the onion port.

Fixes https://github.com/bitcoin/bitcoin/issues/22726
---
 src/init.cpp                          |  2 ++
 test/functional/feature_bind_extra.py | 23 ++++++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index c9405153b4..cf908c5058 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1890,6 +1890,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     CService onion_service_target;
     if (!connOptions.onion_binds.empty()) {
         onion_service_target = connOptions.onion_binds.front();
+    } else if (!connOptions.vBinds.empty()) {
+        onion_service_target = connOptions.vBinds.front();
     } else {
         onion_service_target = DefaultOnionServiceTarget();
         connOptions.onion_binds.push_back(onion_service_target);
diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py
index 5cd031f852..ed2328b76f 100755
--- a/test/functional/feature_bind_extra.py
+++ b/test/functional/feature_bind_extra.py
@@ -27,7 +27,7 @@ def set_test_params(self):
         # Avoid any -bind= on the command line. Force the framework to avoid
         # adding -bind=127.0.0.1.
         self.bind_to_localhost_only = False
-        self.num_nodes = 2
+        self.num_nodes = 3
 
     def skip_test_if_missing_module(self):
         # Due to OS-specific network stats queries, we only run on Linux.
@@ -60,14 +60,21 @@ def setup_network(self):
         )
         port += 2
 
+        # Node2, no -bind=...=onion, thus no extra port for Tor target.
+        self.expected.append(
+            [
+                [f"-bind=127.0.0.1:{port}"],
+                [(loopback_ipv4, port)]
+            ],
+        )
+        port += 1
+
         self.extra_args = list(map(lambda e: e[0], self.expected))
-        self.add_nodes(self.num_nodes, self.extra_args)
-        # Don't start the nodes, as some of them would collide trying to bind on the same port.
+        self.setup_nodes()
 
     def run_test(self):
-        for i in range(len(self.expected)):
-            self.log.info(f"Starting node {i} with {self.expected[i][0]}")
-            self.start_node(i)
+        for i, (args, expected_services) in enumerate(self.expected):
+            self.log.info(f"Checking listening ports of node {i} with {args}")
             pid = self.nodes[i].process.pid
             binds = set(get_bind_addrs(pid))
             # Remove IPv6 addresses because on some CI environments "::1" is not configured
@@ -78,9 +85,7 @@ def run_test(self):
             binds = set(filter(lambda e: len(e[0]) != ipv6_addr_len_bytes, binds))
             # Remove RPC ports. They are not relevant for this test.
             binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
-            assert_equal(binds, set(self.expected[i][1]))
-            self.stop_node(i)
-            self.log.info(f"Stopped node {i}")
+            assert_equal(binds, set(expected_services))
 
 if __name__ == '__main__':
     BindExtraTest().main()

From af552534ab83c572d3bc3f93ccaee5c1961ccab5 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Mon, 2 Aug 2021 19:05:35 +0200
Subject: [PATCH 13/55] net: report an error if unable to bind on the Tor
 listening addr:port

---
 src/net.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/net.cpp b/src/net.cpp
index 990c58ee3d..34972d814f 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3206,7 +3206,7 @@ bool CConnman::InitBinds(const Options& options)
         fBound |= Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags);
     }
     for (const auto& addr_bind : options.onion_binds) {
-        fBound |= Bind(addr_bind, BF_DONT_ADVERTISE, NetPermissionFlags::None);
+        fBound |= Bind(addr_bind, BF_REPORT_ERROR | BF_DONT_ADVERTISE, NetPermissionFlags::None);
     }
     if (options.bind_on_any) {
         struct in_addr inaddr_any;

From bca346a97056748f1e3fb899f336d56d9fd45a64 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Fri, 13 Aug 2021 10:33:24 +0200
Subject: [PATCH 14/55] net: require P2P binds to succeed

In the Tor case, this prevents us from telling the Tor daemon to send
our incoming connections from the Tor network to an address where we
do not listen (we tried to listen but failed probably because another
application is already listening).

In the other cases (IPv4/IPv6 binds) this also prevents unpleasant
surprises caused by continuing operations even on bind failure. For
example, another application may be listening on portX, bitcoind tries
to bind on portX and portY, only succeeds with portY and continues
operation leaving the user thinking that his bitcoind is listening on
portX whereas another application is listening (the error message in
the log could easily be missed).

Avoid having the functional testing framework start multiple `bitcoind`s
that try to listen on the same `127.0.0.1:18445` (Tor listen for
regtest) if `bind_to_localhost_only` is set to `False`.

Also fix a typo in `test-shell.md` related to `bind_to_localhost_only`.

Fixes https://github.com/bitcoin/bitcoin/issues/22727
---
 src/net.cpp                                 | 28 +++++++++++++++------
 test/functional/test-shell.md               |  2 +-
 test/functional/test_framework/test_node.py | 15 +++++++++++
 test/functional/test_framework/util.py      | 10 +++++---
 4 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 34972d814f..dbb338d54c 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3198,24 +3198,36 @@ bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlag
 
 bool CConnman::InitBinds(const Options& options)
 {
-    bool fBound = false;
     for (const auto& addrBind : options.vBinds) {
-        fBound |= Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None);
+        if (!Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None)) {
+            return false;
+        }
     }
     for (const auto& addrBind : options.vWhiteBinds) {
-        fBound |= Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags);
+        if (!Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags)) {
+            return false;
+        }
     }
     for (const auto& addr_bind : options.onion_binds) {
-        fBound |= Bind(addr_bind, BF_REPORT_ERROR | BF_DONT_ADVERTISE, NetPermissionFlags::None);
+        if (!Bind(addr_bind, BF_REPORT_ERROR | BF_DONT_ADVERTISE, NetPermissionFlags::None)) {
+            return false;
+        }
     }
     if (options.bind_on_any) {
+        // Don't consider errors to bind on IPv6 "::" fatal because the host OS
+        // may not have IPv6 support and the user did not explicitly ask us to
+        // bind on that.
+        const CService ipv6_any{in6_addr(IN6ADDR_ANY_INIT), GetListenPort()}; // ::
+        Bind(ipv6_any, BF_NONE, NetPermissionFlags::None);
+
         struct in_addr inaddr_any;
         inaddr_any.s_addr = htonl(INADDR_ANY);
-        struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT;
-        fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::None);
-        fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::None);
+        const CService ipv4_any{inaddr_any, GetListenPort()}; // 0.0.0.0
+        if (!Bind(ipv4_any, BF_REPORT_ERROR, NetPermissionFlags::None)) {
+            return false;
+        }
     }
-    return fBound;
+    return true;
 }
 
 bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
diff --git a/test/functional/test-shell.md b/test/functional/test-shell.md
index 4cd62c4ef3..d79c4a0ab6 100644
--- a/test/functional/test-shell.md
+++ b/test/functional/test-shell.md
@@ -169,7 +169,7 @@ can be called after the TestShell is shut down.
 
 | Test parameter key | Default Value | Description |
 |---|---|---|
-| `bind_to_localhost_only` | `True` | Binds bitcoind RPC services to `127.0.0.1` if set to `True`.|
+| `bind_to_localhost_only` | `True` | Binds bitcoind P2P services to `127.0.0.1` if set to `True`.|
 | `cachedir` | `"/path/to/bitcoin/test/cache"` | Sets the bitcoind datadir directory. |
 | `chain`  | `"regtest"` | Sets the chain-type for the underlying test bitcoind processes. |
 | `configfile` | `"/path/to/bitcoin/test/config.ini"` | Sets the location of the test framework config file. |
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 0ac0af27d5..216de9c309 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -39,6 +39,7 @@
     rpc_url,
     wait_until_helper_internal,
     p2p_port,
+    tor_port,
 )
 
 BITCOIND_PROC_WAIT_TIMEOUT = 60
@@ -88,8 +89,11 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
         self.coverage_dir = coverage_dir
         self.cwd = cwd
         self.descriptors = descriptors
+        self.has_explicit_bind = False
         if extra_conf is not None:
             append_config(self.datadir_path, extra_conf)
+            # Remember if there is bind=... in the config file.
+            self.has_explicit_bind = any(e.startswith("bind=") for e in extra_conf)
         # Most callers will just need to add extra args to the standard list below.
         # For those callers that need more flexibility, they can just set the args property directly.
         # Note that common args are set in the config file (see initialize_datadir)
@@ -210,6 +214,17 @@ def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None
         if extra_args is None:
             extra_args = self.extra_args
 
+        # If listening and no -bind is given, then bitcoind would bind P2P ports on
+        # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is
+        # a unique port chosen by the test framework and configured as port=P in
+        # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to
+        # 127.0.0.1:tor_port().
+        will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args)
+        has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args)
+        if will_listen and not has_explicit_bind:
+            extra_args.append(f"-bind=0.0.0.0:{p2p_port(self.index)}")
+            extra_args.append(f"-bind=127.0.0.1:{tor_port(self.index)}=onion")
+
         self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)
 
         # Add a new stdout and stderr file each time bitcoind is started
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index c5b69a3954..7be1a35d4d 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -309,9 +309,9 @@ def sha256sum_file(filename):
 
 # The maximum number of nodes a single test can spawn
 MAX_NODES = 12
-# Don't assign rpc or p2p ports lower than this
+# Don't assign p2p, rpc or tor ports lower than this
 PORT_MIN = int(os.getenv('TEST_RUNNER_PORT_MIN', default=11000))
-# The number of ports to "reserve" for p2p and rpc, each
+# The number of ports to "reserve" for p2p, rpc and tor, each
 PORT_RANGE = 5000
 
 
@@ -351,7 +351,11 @@ def p2p_port(n):
 
 
 def rpc_port(n):
-    return PORT_MIN + PORT_RANGE + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
+    return p2p_port(n) + PORT_RANGE
+
+
+def tor_port(n):
+    return p2p_port(n) + PORT_RANGE * 2
 
 
 def rpc_url(datadir, i, chain, rpchost):

From 7e36dca657c66bc70b04d5b850e5a335aecfb902 Mon Sep 17 00:00:00 2001
From: willcl-ark <will@256k1.dev>
Date: Fri, 28 Jun 2024 12:45:47 +0100
Subject: [PATCH 15/55] test: add test for modififed walletprocesspsbt calls

This test checks that we can successfully process PSBTs and opt out of
finalization.

Previously trying to call `walletprocesspsbt` would attempt to
auto-finalize (as a convenience), and would not permit opt-out of
finalization, instead aborting via `CHECK_NONFATAL`.
---
 test/functional/rpc_psbt.py | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 6ee7e56886..7d4b6655a4 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -68,6 +68,28 @@ def set_test_params(self):
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
+    def test_psbt_incomplete_after_invalid_modification(self):
+        self.log.info("Check that PSBT is correctly marked as incomplete after invalid modification")
+        node = self.nodes[2]
+        wallet = node.get_wallet_rpc(self.default_wallet_name)
+        address = wallet.getnewaddress()
+        wallet.sendtoaddress(address=address, amount=1.0)
+        self.generate(node, nblocks=1, sync_fun=lambda: self.sync_all(self.nodes[:2]))
+
+        utxos = wallet.listunspent(addresses=[address])
+        psbt = wallet.createpsbt([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{wallet.getnewaddress(): 0.9999}])
+        signed_psbt = wallet.walletprocesspsbt(psbt)["psbt"]
+
+        # Modify the raw transaction by changing the output address, so the signature is no longer valid
+        signed_psbt_obj = PSBT.from_base64(signed_psbt)
+        substitute_addr = wallet.getnewaddress()
+        raw = wallet.createrawtransaction([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{substitute_addr: 0.9999}])
+        signed_psbt_obj.g.map[PSBT_GLOBAL_UNSIGNED_TX] = bytes.fromhex(raw)
+
+        # Check that the walletprocesspsbt call succeeds but also recognizes that the transaction is not complete
+        signed_psbt_incomplete = wallet.walletprocesspsbt(signed_psbt_obj.to_base64(), finalize=False)
+        assert signed_psbt_incomplete["complete"] is False
+
     def test_utxo_conversion(self):
         self.log.info("Check that non-witness UTXOs are removed for segwit v1+ inputs")
         mining_node = self.nodes[2]
@@ -589,6 +611,7 @@ def run_test(self):
 
         if self.options.descriptors:
             self.test_utxo_conversion()
+        self.test_psbt_incomplete_after_invalid_modification()
 
         self.test_input_confs_control()
 

From 4383dc90bac1b5def73352fe222f99807d8ca4dd Mon Sep 17 00:00:00 2001
From: brunoerg <brunoely.gc@gmail.com>
Date: Fri, 14 Jun 2024 11:05:22 -0300
Subject: [PATCH 16/55] fuzz: fix key size in crypter target

Set a max length for some previous
`ConsumeRandomLengthByteVector` usage.
---
 src/wallet/test/fuzz/crypter.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/wallet/test/fuzz/crypter.cpp b/src/wallet/test/fuzz/crypter.cpp
index 62dd1bfde0..814136476b 100644
--- a/src/wallet/test/fuzz/crypter.cpp
+++ b/src/wallet/test/fuzz/crypter.cpp
@@ -27,36 +27,36 @@ FUZZ_TARGET(crypter, .init = initialize_crypter)
     // These values are regularly updated within `CallOneOf`
     std::vector<unsigned char> cipher_text_ed;
     CKeyingMaterial plain_text_ed;
-    const std::vector<unsigned char> random_key = ConsumeRandomLengthByteVector(fuzzed_data_provider);
+    const std::vector<unsigned char> random_key = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE);
 
     LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10000)
     {
         CallOneOf(
             fuzzed_data_provider,
             [&] {
-                const std::string random_string = fuzzed_data_provider.ConsumeRandomLengthString();
+                const std::string random_string = fuzzed_data_provider.ConsumeRandomLengthString(100);
                 SecureString secure_string(random_string.begin(), random_string.end());
 
                 const unsigned int derivation_method = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<unsigned int>();
 
                 // Limiting the value of nRounds since it is otherwise uselessly expensive and causes a timeout when fuzzing.
                 crypt.SetKeyFromPassphrase(/*strKeyData=*/secure_string,
-                                           /*chSalt=*/ConsumeRandomLengthByteVector(fuzzed_data_provider),
+                                           /*chSalt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE),
                                            /*nRounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 25000),
                                            /*nDerivationMethod=*/derivation_method);
             },
             [&] {
-                const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
+                const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE);
                 const CKeyingMaterial new_key(random_vector.begin(), random_vector.end());
-                const std::vector<unsigned char>& new_IV = ConsumeFixedLengthByteVector(fuzzed_data_provider, 16);
+                const std::vector<unsigned char>& new_IV = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_IV_SIZE);
                 crypt.SetKey(new_key, new_IV);
             },
             [&] {
-                const std::vector<unsigned char> random_vector = ConsumeRandomLengthByteVector(fuzzed_data_provider);
+                const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE);
                 plain_text_ed = CKeyingMaterial(random_vector.begin(), random_vector.end());
             },
             [&] {
-                cipher_text_ed = ConsumeRandomLengthByteVector(fuzzed_data_provider);
+                cipher_text_ed = ConsumeRandomLengthByteVector(fuzzed_data_provider, 64);
             },
             [&] {
                 (void)crypt.Encrypt(plain_text_ed, cipher_text_ed);
@@ -82,7 +82,7 @@ FUZZ_TARGET(crypter, .init = initialize_crypter)
                 }
                 const CPubKey pub_key = *random_pub_key;
                 const CKeyingMaterial master_key(random_key.begin(), random_key.end());
-                const std::vector<unsigned char> crypted_secret = ConsumeRandomLengthByteVector(fuzzed_data_provider);
+                const std::vector<unsigned char> crypted_secret = ConsumeRandomLengthByteVector(fuzzed_data_provider, 64);
                 CKey key;
                 DecryptKey(master_key, crypted_secret, pub_key, key);
             });

From a38c9600059a1583be244e0df198ffd9da375ba9 Mon Sep 17 00:00:00 2001
From: Cory Fields <cory-nospam-@coryfields.com>
Date: Wed, 3 Jul 2024 19:38:01 +0000
Subject: [PATCH 17/55] contrib: use c++ rather than c for binary tests

We don't actually use a c compiler as part of Core's build (only for secp).
We should be testing against what we're actually using instead.
---
 Makefile.am                             | 12 ++++++------
 contrib/devtools/test-security-check.py | 14 ++++++-------
 contrib/devtools/test-symbol-check.py   | 26 ++++++++++++-------------
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2ff6dd0a11..b746299a42 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -334,14 +334,14 @@ clean-local: clean-docs
 
 test-security-check:
 if TARGET_DARWIN
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO
 endif
 if TARGET_WINDOWS
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE
 endif
 if TARGET_LINUX
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF
-	$(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF
+	$(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF
 endif
diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py
index 7bfd4d98da..917d8f1b65 100755
--- a/contrib/devtools/test-security-check.py
+++ b/contrib/devtools/test-security-check.py
@@ -34,7 +34,7 @@ def env_flags() -> list[str]:
     # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for
     # reference.
     flags: list[str] = []
-    for var in ['CFLAGS', 'CPPFLAGS', 'LDFLAGS']:
+    for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']:
         flags += filter(None, os.environ.get(var, '').split(' '))
     return flags
 
@@ -52,9 +52,9 @@ def get_arch(cc, source, executable):
 
 class TestSecurityChecks(unittest.TestCase):
     def test_ELF(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CC', 'gcc')
+        cc = determine_wellknown_cmd('CXX', 'g++')
         write_testcode(source)
         arch = get_arch(cc, source, executable)
 
@@ -90,9 +90,9 @@ def test_ELF(self):
         clean_files(source, executable)
 
     def test_PE(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1.exe'
-        cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc')
+        cc = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
         write_testcode(source)
 
         self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']),
@@ -113,9 +113,9 @@ def test_PE(self):
         clean_files(source, executable)
 
     def test_MACHO(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CC', 'clang')
+        cc = determine_wellknown_cmd('CXX', 'clang++')
         write_testcode(source)
         arch = get_arch(cc, source, executable)
 
diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py
index b00004586c..aff22117d7 100755
--- a/contrib/devtools/test-symbol-check.py
+++ b/contrib/devtools/test-symbol-check.py
@@ -18,7 +18,7 @@ def call_symbol_check(cc: list[str], source, executable, options):
     # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for
     # reference.
     env_flags: list[str] = []
-    for var in ['CFLAGS', 'CPPFLAGS', 'LDFLAGS']:
+    for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']:
         env_flags += filter(None, os.environ.get(var, '').split(' '))
 
     subprocess.run([*cc,source,'-o',executable] + env_flags + options, check=True)
@@ -29,13 +29,13 @@ def call_symbol_check(cc: list[str], source, executable, options):
 
 class TestSymbolChecks(unittest.TestCase):
     def test_ELF(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CC', 'gcc')
+        cc = determine_wellknown_cmd('CXX', 'g++')
 
         # -lutil is part of the libc6 package so a safe bet that it's installed
         # it's also out of context enough that it's unlikely to ever become a real dependency
-        source = 'test2.c'
+        source = 'test2.cpp'
         executable = 'test2'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -53,7 +53,7 @@ def test_ELF(self):
                     executable + ': failed LIBRARY_DEPENDENCIES'))
 
         # finally, check a simple conforming binary
-        source = 'test3.c'
+        source = 'test3.cpp'
         executable = 'test3'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -70,9 +70,9 @@ def test_ELF(self):
                 (0, ''))
 
     def test_MACHO(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CC', 'clang')
+        cc = determine_wellknown_cmd('CXX', 'clang++')
 
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -90,7 +90,7 @@ def test_MACHO(self):
             (1, 'libexpat.1.dylib is not in ALLOWED_LIBRARIES!\n' +
                 f'{executable}: failed DYNAMIC_LIBRARIES MIN_OS SDK'))
 
-        source = 'test2.c'
+        source = 'test2.cpp'
         executable = 'test2'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -106,7 +106,7 @@ def test_MACHO(self):
         self.assertEqual(call_symbol_check(cc, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
                 (1, f'{executable}: failed MIN_OS SDK'))
 
-        source = 'test3.c'
+        source = 'test3.cpp'
         executable = 'test3'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -120,9 +120,9 @@ def test_MACHO(self):
                 (1, f'{executable}: failed SDK'))
 
     def test_PE(self):
-        source = 'test1.c'
+        source = 'test1.cpp'
         executable = 'test1.exe'
-        cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc')
+        cc = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
 
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -139,7 +139,7 @@ def test_PE(self):
             (1, 'pdh.dll is not in ALLOWED_LIBRARIES!\n' +
                  executable + ': failed DYNAMIC_LIBRARIES'))
 
-        source = 'test2.c'
+        source = 'test2.cpp'
         executable = 'test2.exe'
 
         with open(source, 'w', encoding="utf8") as f:
@@ -153,7 +153,7 @@ def test_PE(self):
         self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']),
             (1, executable + ': failed SUBSYSTEM_VERSION'))
 
-        source = 'test3.c'
+        source = 'test3.cpp'
         executable = 'test3.exe'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''

From 261f77033349a83f14d39b8bb8a2df46085b47ad Mon Sep 17 00:00:00 2001
From: Cory Fields <cory-nospam-@coryfields.com>
Date: Wed, 3 Jul 2024 19:41:12 +0000
Subject: [PATCH 18/55] contrib: rename cc to cxx in binary checking scripts

---
 contrib/devtools/test-security-check.py | 76 ++++++++++++-------------
 contrib/devtools/test-symbol-check.py   | 26 ++++-----
 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py
index 917d8f1b65..f1ea93dc7f 100755
--- a/contrib/devtools/test-security-check.py
+++ b/contrib/devtools/test-security-check.py
@@ -38,13 +38,13 @@ def env_flags() -> list[str]:
         flags += filter(None, os.environ.get(var, '').split(' '))
     return flags
 
-def call_security_check(cc: str, source: str, executable: str, options) -> tuple:
-    subprocess.run([*cc,source,'-o',executable] + env_flags() + options, check=True)
+def call_security_check(cxx: str, source: str, executable: str, options) -> tuple:
+    subprocess.run([*cxx,source,'-o',executable] + env_flags() + options, check=True)
     p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, text=True)
     return (p.returncode, p.stdout.rstrip())
 
-def get_arch(cc, source, executable):
-    subprocess.run([*cc, source, '-o', executable] + env_flags(), check=True)
+def get_arch(cxx, source, executable):
+    subprocess.run([*cxx, source, '-o', executable] + env_flags(), check=True)
     binary = lief.parse(executable)
     arch = binary.abstract.header.architecture
     os.remove(executable)
@@ -54,37 +54,37 @@ class TestSecurityChecks(unittest.TestCase):
     def test_ELF(self):
         source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CXX', 'g++')
+        cxx = determine_wellknown_cmd('CXX', 'g++')
         write_testcode(source)
-        arch = get_arch(cc, source, executable)
+        arch = get_arch(cxx, source, executable)
 
         if arch == lief.ARCHITECTURES.X86:
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE NX RELRO CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE RELRO CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE RELRO CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed RELRO CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']),
                     (1, executable+': failed separate_code CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']),
                     (0, ''))
         else:
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE NX RELRO'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE RELRO'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed PIE RELRO'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']),
                     (1, executable+': failed RELRO'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']),
                     (1, executable+': failed separate_code'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']),
                     (0, ''))
 
         clean_files(source, executable)
@@ -92,22 +92,22 @@ def test_ELF(self):
     def test_PE(self):
         source = 'test1.cpp'
         executable = 'test1.exe'
-        cc = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
+        cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
         write_testcode(source)
 
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW Canary'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW'))  # -pie -fPIE does nothing unless --dynamicbase is also supplied
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']),
             (1, executable+': failed CONTROL_FLOW'))
-        self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']),
+        self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']),
             (0, ''))
 
         clean_files(source, executable)
@@ -115,30 +115,30 @@ def test_PE(self):
     def test_MACHO(self):
         source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CXX', 'clang++')
+        cxx = determine_wellknown_cmd('CXX', 'clang++')
         write_testcode(source)
-        arch = get_arch(cc, source, executable)
+        arch = get_arch(cxx, source, executable)
 
         if arch == lief.ARCHITECTURES.X86:
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']),
                 (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS PIE CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']),
                 (1, executable+': failed NOUNDEFS Canary CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']),
                 (1, executable+': failed NOUNDEFS CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']),
                 (1, executable+': failed CONTROL_FLOW'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-fcf-protection=full', '-Wl,-fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-fcf-protection=full', '-Wl,-fixup_chains']),
                 (0, ''))
         else:
             # arm64 darwin doesn't support non-PIE binaries, control flow or executable stacks
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']),
                 (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS BRANCH_PROTECTION'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
                 (1, executable+': failed NOUNDEFS Canary'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
                 (1, executable+': failed NOUNDEFS'))
-            self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
+            self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']),
                 (0, ''))
 
 
diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py
index aff22117d7..517626c285 100755
--- a/contrib/devtools/test-symbol-check.py
+++ b/contrib/devtools/test-symbol-check.py
@@ -11,7 +11,7 @@
 
 from utils import determine_wellknown_cmd
 
-def call_symbol_check(cc: list[str], source, executable, options):
+def call_symbol_check(cxx: list[str], source, executable, options):
     # This should behave the same as AC_TRY_LINK, so arrange well-known flags
     # in the same order as autoconf would.
     #
@@ -21,7 +21,7 @@ def call_symbol_check(cc: list[str], source, executable, options):
     for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']:
         env_flags += filter(None, os.environ.get(var, '').split(' '))
 
-    subprocess.run([*cc,source,'-o',executable] + env_flags + options, check=True)
+    subprocess.run([*cxx,source,'-o',executable] + env_flags + options, check=True)
     p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, text=True)
     os.remove(source)
     os.remove(executable)
@@ -31,7 +31,7 @@ class TestSymbolChecks(unittest.TestCase):
     def test_ELF(self):
         source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CXX', 'g++')
+        cxx = determine_wellknown_cmd('CXX', 'g++')
 
         # -lutil is part of the libc6 package so a safe bet that it's installed
         # it's also out of context enough that it's unlikely to ever become a real dependency
@@ -48,7 +48,7 @@ def test_ELF(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-lutil']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-lutil']),
                 (1, executable + ': libutil.so.1 is not in ALLOWED_LIBRARIES!\n' +
                     executable + ': failed LIBRARY_DEPENDENCIES'))
 
@@ -66,13 +66,13 @@ def test_ELF(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, []),
+        self.assertEqual(call_symbol_check(cxx, source, executable, []),
                 (0, ''))
 
     def test_MACHO(self):
         source = 'test1.cpp'
         executable = 'test1'
-        cc = determine_wellknown_cmd('CXX', 'clang++')
+        cxx = determine_wellknown_cmd('CXX', 'clang++')
 
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -86,7 +86,7 @@ def test_MACHO(self):
 
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
             (1, 'libexpat.1.dylib is not in ALLOWED_LIBRARIES!\n' +
                 f'{executable}: failed DYNAMIC_LIBRARIES MIN_OS SDK'))
 
@@ -103,7 +103,7 @@ def test_MACHO(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
                 (1, f'{executable}: failed MIN_OS SDK'))
 
         source = 'test3.cpp'
@@ -116,13 +116,13 @@ def test_MACHO(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,11.0', '-Wl,11.4']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,11.0', '-Wl,11.4']),
                 (1, f'{executable}: failed SDK'))
 
     def test_PE(self):
         source = 'test1.cpp'
         executable = 'test1.exe'
-        cc = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
+        cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++')
 
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
@@ -135,7 +135,7 @@ def test_PE(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-lpdh', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-lpdh', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']),
             (1, 'pdh.dll is not in ALLOWED_LIBRARIES!\n' +
                  executable + ': failed DYNAMIC_LIBRARIES'))
 
@@ -150,7 +150,7 @@ def test_PE(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']),
             (1, executable + ': failed SUBSYSTEM_VERSION'))
 
         source = 'test3.cpp'
@@ -166,7 +166,7 @@ def test_PE(self):
                 }
         ''')
 
-        self.assertEqual(call_symbol_check(cc, source, executable, ['-lole32', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']),
+        self.assertEqual(call_symbol_check(cxx, source, executable, ['-lole32', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']),
                 (0, ''))
 
 

From b71bfd9eef8b0017cef3c05361c02ddbd837e6ac Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jul 2024 18:25:16 +0000
Subject: [PATCH 19/55] GUI/OptionsDialog: Prefer to stretch actual options
 area rather than waste space

---
 src/qt/optionsdialog.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index e31542961c..c79caf9b6c 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -34,6 +34,8 @@ OptionsDialog::OptionsDialog(QWidget *parent) :
     ui->setupUi(this);
     GUIUtil::restoreWindowGeometry("nOptionsDialogWindow", this->size(), this);
 
+    ui->verticalLayout->setStretchFactor(ui->tabWidget, 1);
+
     /* Main elements init */
     ui->databaseCache->setMinimum(nMinDbCache);
     ui->databaseCache->setMaximum(nMaxDbCache);

From fa690c8e532672f7ab53be6f7a0bb3070858745e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Mon, 8 Jul 2024 15:45:56 +0200
Subject: [PATCH 20/55] test: [refactor] Pass TestOpts

---
 src/bench/logging.cpp                         |  2 +-
 src/bench/mempool_stress.cpp                  |  2 +-
 src/test/checkqueue_tests.cpp                 |  2 +-
 src/test/fuzz/process_message.cpp             |  2 +-
 src/test/fuzz/process_messages.cpp            |  2 +-
 src/test/fuzz/rpc.cpp                         |  2 +-
 src/test/fuzz/utxo_total_supply.cpp           |  2 +-
 src/test/i2p_tests.cpp                        |  4 +--
 src/test/net_peer_connection_tests.cpp        |  2 +-
 src/test/txvalidationcache_tests.cpp          |  2 +-
 src/test/util/setup_common.cpp                | 26 ++++++++-----------
 src/test/util/setup_common.h                  | 26 ++++++++++---------
 .../validation_chainstatemanager_tests.cpp    |  7 ++---
 13 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/src/bench/logging.cpp b/src/bench/logging.cpp
index c97c4e151b..8a745a0ba7 100644
--- a/src/bench/logging.cpp
+++ b/src/bench/logging.cpp
@@ -20,7 +20,7 @@ static void Logging(benchmark::Bench& bench, const std::vector<const char*>& ext
 
     TestingSetup test_setup{
         ChainType::REGTEST,
-        extra_args,
+        {.extra_args = extra_args},
     };
 
     bench.run([&] { log(); });
diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp
index fe3e204fb3..3c82f55c19 100644
--- a/src/bench/mempool_stress.cpp
+++ b/src/bench/mempool_stress.cpp
@@ -106,7 +106,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
 static void MempoolCheck(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
-    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {"-checkmempool=1"});
+    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {.extra_args = {"-checkmempool=1"}});
     CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
     testing_setup->PopulateMempool(det_rand, 400, true);
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
index 023a5e8e70..7810d91a77 100644
--- a/src/test/checkqueue_tests.cpp
+++ b/src/test/checkqueue_tests.cpp
@@ -28,7 +28,7 @@
 struct NoLockLoggingTestingSetup : public TestingSetup {
     NoLockLoggingTestingSetup()
 #ifdef DEBUG_LOCKCONTENTION
-        : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-debugexclude=lock"}} {}
+        : TestingSetup{ChainType::MAIN, {.extra_args = { "-debugexclude=lock" } }} {}
 #else
         : TestingSetup{ChainType::MAIN} {}
 #endif
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index d10d9dafe8..6373eac1c3 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -42,7 +42,7 @@ void initialize_process_message()
 
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            /*extra_args=*/{"-txreconciliation"});
+            {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
         MineBlock(g_setup->m_node, CScript() << OP_TRUE);
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index 38acd432fa..62f38967a3 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -32,7 +32,7 @@ void initialize_process_messages()
 {
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            /*extra_args=*/{"-txreconciliation"});
+            {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
         MineBlock(g_setup->m_node, CScript() << OP_TRUE);
diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp
index 4e52c1c091..9122617e46 100644
--- a/src/test/fuzz/rpc.cpp
+++ b/src/test/fuzz/rpc.cpp
@@ -41,7 +41,7 @@ using util::ToString;
 
 namespace {
 struct RPCFuzzTestingSetup : public TestingSetup {
-    RPCFuzzTestingSetup(const ChainType chain_type, const std::vector<const char*>& extra_args) : TestingSetup{chain_type, extra_args}
+    RPCFuzzTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts}
     {
     }
 
diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp
index 48ed266abe..b0f1a1251a 100644
--- a/src/test/fuzz/utxo_total_supply.cpp
+++ b/src/test/fuzz/utxo_total_supply.cpp
@@ -23,7 +23,7 @@ FUZZ_TARGET(utxo_total_supply)
     ChainTestingSetup test_setup{
         ChainType::REGTEST,
         {
-            "-testactivationheight=bip34@2",
+            .extra_args = {"-testactivationheight=bip34@2"},
         },
     };
     // Create chainstate
diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp
index 0512c6134f..bb9ca88019 100644
--- a/src/test/i2p_tests.cpp
+++ b/src/test/i2p_tests.cpp
@@ -23,8 +23,8 @@ class EnvTestingSetup : public BasicTestingSetup
 {
 public:
     explicit EnvTestingSetup(const ChainType chainType = ChainType::MAIN,
-                             const std::vector<const char*>& extra_args = {})
-        : BasicTestingSetup{chainType, extra_args},
+                             TestOpts opts = {})
+        : BasicTestingSetup{chainType, opts},
           m_prev_log_level{LogInstance().LogLevel()},
           m_create_sock_orig{CreateSock}
     {
diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp
index 5f38ce112c..2dde6daee5 100644
--- a/src/test/net_peer_connection_tests.cpp
+++ b/src/test/net_peer_connection_tests.cpp
@@ -31,7 +31,7 @@
 
 struct LogIPsTestingSetup : public TestingSetup {
     LogIPsTestingSetup()
-        : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-logips"}} {}
+        : TestingSetup{ChainType::MAIN, {.extra_args = {"-logips"}}} {}
 };
 
 BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup)
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 78ef96a15d..079f361b0e 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -16,7 +16,7 @@
 
 struct Dersig100Setup : public TestChain100Setup {
     Dersig100Setup()
-        : TestChain100Setup{ChainType::REGTEST, {"-testactivationheight=dersig@102"}} {}
+        : TestChain100Setup{ChainType::REGTEST, {.extra_args = {"-testactivationheight=dersig@102"}}} {}
 };
 
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 52981bd2dc..fec4ae23e8 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -112,7 +112,7 @@ static void ExitFailure(std::string_view str_err)
     exit(EXIT_FAILURE);
 }
 
-BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
+BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
     : m_args{}
 {
     m_node.shutdown = &m_interrupt;
@@ -129,7 +129,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
             "-debugexclude=libevent",
             "-debugexclude=leveldb",
         },
-        extra_args);
+        opts.extra_args);
     if (G_TEST_COMMAND_LINE_ARGUMENTS) {
         arguments = Cat(arguments, G_TEST_COMMAND_LINE_ARGUMENTS());
     }
@@ -220,8 +220,8 @@ BasicTestingSetup::~BasicTestingSetup()
     gArgs.ClearArgs();
 }
 
-ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
-    : BasicTestingSetup(chainType, extra_args)
+ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
+    : BasicTestingSetup(chainType, opts)
 {
     const CChainParams& chainparams = Params();
 
@@ -304,13 +304,11 @@ void ChainTestingSetup::LoadVerifyActivateChainstate()
 
 TestingSetup::TestingSetup(
     const ChainType chainType,
-    const std::vector<const char*>& extra_args,
-    const bool coins_db_in_memory,
-    const bool block_tree_db_in_memory)
-    : ChainTestingSetup(chainType, extra_args)
+    TestOpts opts)
+    : ChainTestingSetup(chainType, opts)
 {
-    m_coins_db_in_memory = coins_db_in_memory;
-    m_block_tree_db_in_memory = block_tree_db_in_memory;
+    m_coins_db_in_memory = opts.coins_db_in_memory;
+    m_block_tree_db_in_memory = opts.block_tree_db_in_memory;
     // Ideally we'd move all the RPC tests to the functional testing framework
     // instead of unit tests, but for now we need these here.
     RegisterAllCoreRPCCommands(tableRPC);
@@ -339,11 +337,9 @@ TestingSetup::TestingSetup(
 }
 
 TestChain100Setup::TestChain100Setup(
-        const ChainType chain_type,
-        const std::vector<const char*>& extra_args,
-        const bool coins_db_in_memory,
-        const bool block_tree_db_in_memory)
-    : TestingSetup{ChainType::REGTEST, extra_args, coins_db_in_memory, block_tree_db_in_memory}
+    const ChainType chain_type,
+    TestOpts opts)
+    : TestingSetup{ChainType::REGTEST, opts}
 {
     SetMockTime(1598887952);
     constexpr std::array<unsigned char, 32> vchKey = {
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index dbd66e3585..e8b630af94 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -48,6 +48,12 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os
 
 static constexpr CAmount CENT{1000000};
 
+struct TestOpts {
+    std::vector<const char*> extra_args{};
+    bool coins_db_in_memory{true};
+    bool block_tree_db_in_memory{true};
+};
+
 /** Basic testing setup.
  * This just configures logging, data dir and chain parameters.
  */
@@ -55,7 +61,7 @@ struct BasicTestingSetup {
     util::SignalInterrupt m_interrupt;
     node::NodeContext m_node; // keep as first member to be destructed last
 
-    explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {});
+    explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
     ~BasicTestingSetup();
 
     fs::path m_path_root;
@@ -73,7 +79,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
     bool m_coins_db_in_memory{true};
     bool m_block_tree_db_in_memory{true};
 
-    explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {});
+    explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
     ~ChainTestingSetup();
 
     // Supplies a chainstate, if one is needed
@@ -85,9 +91,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
 struct TestingSetup : public ChainTestingSetup {
     explicit TestingSetup(
         const ChainType chainType = ChainType::MAIN,
-        const std::vector<const char*>& extra_args = {},
-        const bool coins_db_in_memory = true,
-        const bool block_tree_db_in_memory = true);
+        TestOpts = {});
 };
 
 /** Identical to TestingSetup, but chain set to regtest */
@@ -106,9 +110,7 @@ class CScript;
 struct TestChain100Setup : public TestingSetup {
     TestChain100Setup(
         const ChainType chain_type = ChainType::REGTEST,
-        const std::vector<const char*>& extra_args = {},
-        const bool coins_db_in_memory = true,
-        const bool block_tree_db_in_memory = true);
+        TestOpts = {});
 
     /**
      * Create a new block with just given transactions, coinbase paying to
@@ -220,16 +222,16 @@ struct TestChain100Setup : public TestingSetup {
  * be used in "hot loops", for example fuzzing or benchmarking.
  */
 template <class T = const BasicTestingSetup>
-std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, const std::vector<const char*>& extra_args = {})
+std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, TestOpts opts = {})
 {
-    const std::vector<const char*> arguments = Cat(
+    opts.extra_args = Cat(
         {
             "-nodebuglogfile",
             "-nodebug",
         },
-        extra_args);
+        opts.extra_args);
 
-    return std::make_unique<T>(chain_type, arguments);
+    return std::make_unique<T>(chain_type, opts);
 }
 
 CBlock getBlock13b8a();
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 1641c4cd22..f93e3cdfb1 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -167,9 +167,10 @@ struct SnapshotTestSetup : TestChain100Setup {
     // destructive filesystem operations.
     SnapshotTestSetup() : TestChain100Setup{
                               {},
-                              {},
-                              /*coins_db_in_memory=*/false,
-                              /*block_tree_db_in_memory=*/false,
+                              {
+                                  .coins_db_in_memory = false,
+                                  .block_tree_db_in_memory = false,
+                              },
                           }
     {
     }

From 3443943f86678eb27d9895f3871fcf3945eb1c4f Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Wed, 19 Jun 2024 11:36:31 +0200
Subject: [PATCH 21/55] refactor: De-globalize validation benchmark timekeeping

In future, users of the kernel library might run multiple chainstates in
parallel, or create and destroy multiple chainstates over the lifetime
of a process. Having static, mutable variables could lead to state
inconsistencies in these scenarios.
---
 src/validation.cpp | 84 +++++++++++++++++++---------------------------
 src/validation.h   | 15 +++++++++
 2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 939759f0df..812290021b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2399,15 +2399,6 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Ch
 }
 
 
-static SteadyClock::duration time_check{};
-static SteadyClock::duration time_forks{};
-static SteadyClock::duration time_connect{};
-static SteadyClock::duration time_verify{};
-static SteadyClock::duration time_undo{};
-static SteadyClock::duration time_index{};
-static SteadyClock::duration time_total{};
-static int64_t num_blocks_total = 0;
-
 /** Apply the effects of this block (with given index) on the UTXO set represented by coins.
  *  Validity checks that depend on the UTXO set are also done; ConnectBlock()
  *  can fail if those validity checks fail (among other reasons). */
@@ -2452,7 +2443,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     uint256 hashPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->GetBlockHash();
     assert(hashPrevBlock == view.GetBestBlock());
 
-    num_blocks_total++;
+    m_chainman.num_blocks_total++;
 
     // Special case for the genesis block, skipping connection of its transactions
     // (its coinbase is unspendable)
@@ -2494,11 +2485,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     }
 
     const auto time_1{SteadyClock::now()};
-    time_check += time_1 - time_start;
+    m_chainman.time_check += time_1 - time_start;
     LogPrint(BCLog::BENCH, "    - Sanity checks: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_1 - time_start),
-             Ticks<SecondsDouble>(time_check),
-             Ticks<MillisecondsDouble>(time_check) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_check),
+             Ticks<MillisecondsDouble>(m_chainman.time_check) / m_chainman.num_blocks_total);
 
     // Do not allow blocks that contain transactions which 'overwrite' older transactions,
     // unless those are already completely spent.
@@ -2596,11 +2587,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     unsigned int flags{GetBlockScriptFlags(*pindex, m_chainman)};
 
     const auto time_2{SteadyClock::now()};
-    time_forks += time_2 - time_1;
+    m_chainman.time_forks += time_2 - time_1;
     LogPrint(BCLog::BENCH, "    - Fork checks: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_2 - time_1),
-             Ticks<SecondsDouble>(time_forks),
-             Ticks<MillisecondsDouble>(time_forks) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_forks),
+             Ticks<MillisecondsDouble>(m_chainman.time_forks) / m_chainman.num_blocks_total);
 
     CBlockUndo blockundo;
 
@@ -2687,12 +2678,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         UpdateCoins(tx, view, i == 0 ? undoDummy : blockundo.vtxundo.back(), pindex->nHeight);
     }
     const auto time_3{SteadyClock::now()};
-    time_connect += time_3 - time_2;
+    m_chainman.time_connect += time_3 - time_2;
     LogPrint(BCLog::BENCH, "      - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(),
              Ticks<MillisecondsDouble>(time_3 - time_2), Ticks<MillisecondsDouble>(time_3 - time_2) / block.vtx.size(),
              nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_3 - time_2) / (nInputs - 1),
-             Ticks<SecondsDouble>(time_connect),
-             Ticks<MillisecondsDouble>(time_connect) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_connect),
+             Ticks<MillisecondsDouble>(m_chainman.time_connect) / m_chainman.num_blocks_total);
 
     CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, params.GetConsensus());
     if (block.vtx[0]->GetValueOut() > blockReward) {
@@ -2705,12 +2696,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
         return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-validation-failed");
     }
     const auto time_4{SteadyClock::now()};
-    time_verify += time_4 - time_2;
+    m_chainman.time_verify += time_4 - time_2;
     LogPrint(BCLog::BENCH, "    - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1,
              Ticks<MillisecondsDouble>(time_4 - time_2),
              nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_4 - time_2) / (nInputs - 1),
-             Ticks<SecondsDouble>(time_verify),
-             Ticks<MillisecondsDouble>(time_verify) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_verify),
+             Ticks<MillisecondsDouble>(m_chainman.time_verify) / m_chainman.num_blocks_total);
 
     if (fJustCheck)
         return true;
@@ -2720,11 +2711,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     }
 
     const auto time_5{SteadyClock::now()};
-    time_undo += time_5 - time_4;
+    m_chainman.time_undo += time_5 - time_4;
     LogPrint(BCLog::BENCH, "    - Write undo data: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_5 - time_4),
-             Ticks<SecondsDouble>(time_undo),
-             Ticks<MillisecondsDouble>(time_undo) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_undo),
+             Ticks<MillisecondsDouble>(m_chainman.time_undo) / m_chainman.num_blocks_total);
 
     if (!pindex->IsValid(BLOCK_VALID_SCRIPTS)) {
         pindex->RaiseValidity(BLOCK_VALID_SCRIPTS);
@@ -2735,11 +2726,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     view.SetBestBlock(pindex->GetBlockHash());
 
     const auto time_6{SteadyClock::now()};
-    time_index += time_6 - time_5;
+    m_chainman.time_index += time_6 - time_5;
     LogPrint(BCLog::BENCH, "    - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_6 - time_5),
-             Ticks<SecondsDouble>(time_index),
-             Ticks<MillisecondsDouble>(time_index) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_index),
+             Ticks<MillisecondsDouble>(m_chainman.time_index) / m_chainman.num_blocks_total);
 
     TRACE6(validation, block_connected,
         block_hash.data(),
@@ -3097,11 +3088,6 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra
     return true;
 }
 
-static SteadyClock::duration time_connect_total{};
-static SteadyClock::duration time_flush{};
-static SteadyClock::duration time_chainstate{};
-static SteadyClock::duration time_post_connect{};
-
 struct PerBlockConnectTrace {
     CBlockIndex* pindex = nullptr;
     std::shared_ptr<const CBlock> pblock;
@@ -3188,31 +3174,31 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
             return false;
         }
         time_3 = SteadyClock::now();
-        time_connect_total += time_3 - time_2;
-        assert(num_blocks_total > 0);
+        m_chainman.time_connect_total += time_3 - time_2;
+        assert(m_chainman.num_blocks_total > 0);
         LogPrint(BCLog::BENCH, "  - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n",
                  Ticks<MillisecondsDouble>(time_3 - time_2),
-                 Ticks<SecondsDouble>(time_connect_total),
-                 Ticks<MillisecondsDouble>(time_connect_total) / num_blocks_total);
+                 Ticks<SecondsDouble>(m_chainman.time_connect_total),
+                 Ticks<MillisecondsDouble>(m_chainman.time_connect_total) / m_chainman.num_blocks_total);
         bool flushed = view.Flush();
         assert(flushed);
     }
     const auto time_4{SteadyClock::now()};
-    time_flush += time_4 - time_3;
+    m_chainman.time_flush += time_4 - time_3;
     LogPrint(BCLog::BENCH, "  - Flush: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_4 - time_3),
-             Ticks<SecondsDouble>(time_flush),
-             Ticks<MillisecondsDouble>(time_flush) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_flush),
+             Ticks<MillisecondsDouble>(m_chainman.time_flush) / m_chainman.num_blocks_total);
     // Write the chain state to disk, if necessary.
     if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) {
         return false;
     }
     const auto time_5{SteadyClock::now()};
-    time_chainstate += time_5 - time_4;
+    m_chainman.time_chainstate += time_5 - time_4;
     LogPrint(BCLog::BENCH, "  - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_5 - time_4),
-             Ticks<SecondsDouble>(time_chainstate),
-             Ticks<MillisecondsDouble>(time_chainstate) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_chainstate),
+             Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total);
     // Remove conflicting transactions from the mempool.;
     if (m_mempool) {
         m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
@@ -3223,16 +3209,16 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
     UpdateTip(pindexNew);
 
     const auto time_6{SteadyClock::now()};
-    time_post_connect += time_6 - time_5;
-    time_total += time_6 - time_1;
+    m_chainman.time_post_connect += time_6 - time_5;
+    m_chainman.time_total += time_6 - time_1;
     LogPrint(BCLog::BENCH, "  - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_6 - time_5),
-             Ticks<SecondsDouble>(time_post_connect),
-             Ticks<MillisecondsDouble>(time_post_connect) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_post_connect),
+             Ticks<MillisecondsDouble>(m_chainman.time_post_connect) / m_chainman.num_blocks_total);
     LogPrint(BCLog::BENCH, "- Connect block: %.2fms [%.2fs (%.2fms/blk)]\n",
              Ticks<MillisecondsDouble>(time_6 - time_1),
-             Ticks<SecondsDouble>(time_total),
-             Ticks<MillisecondsDouble>(time_total) / num_blocks_total);
+             Ticks<SecondsDouble>(m_chainman.time_total),
+             Ticks<MillisecondsDouble>(m_chainman.time_total) / m_chainman.num_blocks_total);
 
     // If we are the background validation chainstate, check to see if we are done
     // validating the snapshot (i.e. our tip has reached the snapshot's base block).
diff --git a/src/validation.h b/src/validation.h
index 9f99ba796b..92377aa5fc 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -949,6 +949,21 @@ class ChainstateManager
     //! A queue for script verifications that have to be performed by worker threads.
     CCheckQueue<CScriptCheck> m_script_check_queue;
 
+    //! Timers and counters used for benchmarking validation in both background
+    //! and active chainstates.
+    SteadyClock::duration GUARDED_BY(::cs_main) time_check{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_forks{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_connect{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_verify{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_undo{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_index{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_total{};
+    int64_t GUARDED_BY(::cs_main) num_blocks_total{0};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_connect_total{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_flush{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_chainstate{};
+    SteadyClock::duration GUARDED_BY(::cs_main) time_post_connect{};
+
 public:
     using Options = kernel::ChainstateManagerOpts;
 

From 39f9b80fba85d9818222c4d76e99ea1a804f5dda Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sun, 23 Jun 2024 12:54:49 +0200
Subject: [PATCH 22/55] refactor: De-globalize last notified header index

In future, users of the kernel library might run multiple chainstates in
parallel, or create and destroy multiple chainstates over the lifetime
of a process. Having static, mutable variables could lead to state
inconsistencies in these scenarios.
---
 src/validation.cpp | 5 ++---
 src/validation.h   | 3 +++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 812290021b..270ade3557 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3409,16 +3409,15 @@ static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main)
 {
     bool fNotify = false;
     bool fInitialBlockDownload = false;
-    static CBlockIndex* pindexHeaderOld = nullptr;
     CBlockIndex* pindexHeader = nullptr;
     {
         LOCK(cs_main);
         pindexHeader = chainman.m_best_header;
 
-        if (pindexHeader != pindexHeaderOld) {
+        if (pindexHeader != chainman.m_last_notified_header) {
             fNotify = true;
             fInitialBlockDownload = chainman.IsInitialBlockDownload();
-            pindexHeaderOld = pindexHeader;
+            chainman.m_last_notified_header = pindexHeader;
         }
     }
     // Send block tip changed notifications without cs_main
diff --git a/src/validation.h b/src/validation.h
index 92377aa5fc..ea4ec84242 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1063,6 +1063,9 @@ class ChainstateManager
     /** Best header we've seen so far (used for getheaders queries' starting points). */
     CBlockIndex* m_best_header GUARDED_BY(::cs_main){nullptr};
 
+    /** The last header for which a headerTip notification was issued. */
+    CBlockIndex* m_last_notified_header GUARDED_BY(::cs_main){nullptr};
+
     //! The total number of bytes available for us to use across all in-memory
     //! coins caches. This will be split somehow across chainstates.
     int64_t m_total_coinstip_cache{0};

From 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Sun, 23 Jun 2024 13:28:11 +0200
Subject: [PATCH 23/55] refactor: Mark some static global vars as const

These were found while looking for static mutable state in the kernel
library.
---
 src/core_read.cpp         | 2 +-
 src/script/descriptor.cpp | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/core_read.cpp b/src/core_read.cpp
index 114f92fc45..0ba271a8d2 100644
--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex, uint256& result)
 
 util::Result<int> SighashFromStr(const std::string& sighash)
 {
-    static std::map<std::string, int> map_sighash_values = {
+    static const std::map<std::string, int> map_sighash_values = {
         {std::string("DEFAULT"), int(SIGHASH_DEFAULT)},
         {std::string("ALL"), int(SIGHASH_ALL)},
         {std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)},
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 0987db194c..ae9dba6a50 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -116,13 +116,13 @@ std::string DescriptorChecksum(const Span<const char>& span)
      * As a result, within-group-of-32 errors count as 1 symbol, as do cross-group errors that don't affect
      * the position within the groups.
      */
-    static std::string INPUT_CHARSET =
+    static const std::string INPUT_CHARSET =
         "0123456789()[],'/*abcdefgh@:$%{}"
         "IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~"
         "ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
 
     /** The character set for the checksum itself (same as bech32). */
-    static std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l";
+    static const std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l";
 
     uint64_t c = 1;
     int cls = 0;

From de273d53004f48e4c8c965f7ce0bd375fd8d0d69 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 9 Jul 2024 13:47:30 +0100
Subject: [PATCH 24/55] MiniMiner: use FeeFrac in AncestorFeerateComparator

Comparing using FeeFracs is more precise, allows us to simply the
code since FeeFrac comparison internally does cross-multiplication,
and avoids potential overflow in the multiplication.

Previously, we were only comparing feerates up to 0.001sat/vB precision,
since CFeeRate comparison just looks at their respective nSatoshisPerK.
This could lead to MiniMiner selecting packages in the wrong order (i.e.
by txid) if their feerates were less than 0.001sat/vB different.
---
 src/node/mini_miner.cpp | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp
index 58422c4439..d7d15554b3 100644
--- a/src/node/mini_miner.cpp
+++ b/src/node/mini_miner.cpp
@@ -174,7 +174,7 @@ MiniMiner::MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries,
     SanityCheck();
 }
 
-// Compare by min(ancestor feerate, individual feerate), then iterator
+// Compare by min(ancestor feerate, individual feerate), then txid
 //
 // Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate
 // parents do not incentive inclusion of their children. Therefore the mining algorithm only considers
@@ -183,21 +183,13 @@ struct AncestorFeerateComparator
 {
     template<typename I>
     bool operator()(const I& a, const I& b) const {
-        auto min_feerate = [](const MiniMinerMempoolEntry& e) -> CFeeRate {
-            const CAmount ancestor_fee{e.GetModFeesWithAncestors()};
-            const int64_t ancestor_size{e.GetSizeWithAncestors()};
-            const CAmount tx_fee{e.GetModifiedFee()};
-            const int64_t tx_size{e.GetTxSize()};
-            // Comparing ancestor feerate with individual feerate:
-            //     ancestor_fee / ancestor_size <= tx_fee / tx_size
-            // Avoid division and possible loss of precision by
-            // multiplying both sides by the sizes:
-            return ancestor_fee * tx_size < tx_fee * ancestor_size ?
-                       CFeeRate(ancestor_fee, ancestor_size) :
-                       CFeeRate(tx_fee, tx_size);
+        auto min_feerate = [](const MiniMinerMempoolEntry& e) -> FeeFrac {
+            FeeFrac self_feerate(e.GetModifiedFee(), e.GetTxSize());
+            FeeFrac ancestor_feerate(e.GetModFeesWithAncestors(), e.GetSizeWithAncestors());
+            return std::min(ancestor_feerate, self_feerate);
         };
-        CFeeRate a_feerate{min_feerate(a->second)};
-        CFeeRate b_feerate{min_feerate(b->second)};
+        FeeFrac a_feerate{min_feerate(a->second)};
+        FeeFrac b_feerate{min_feerate(b->second)};
         if (a_feerate != b_feerate) {
             return a_feerate > b_feerate;
         }

From 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 9 Jul 2024 14:06:30 +0100
Subject: [PATCH 25/55] fuzz: mini_miner_selection fixups.

Delete asserts that are redundant with the == assert.
Add assertion that the coinbase isn't already in mock_template_txids.
---
 src/test/fuzz/mini_miner.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp
index 3a1663364f..51de4d0166 100644
--- a/src/test/fuzz/mini_miner.cpp
+++ b/src/test/fuzz/mini_miner.cpp
@@ -188,9 +188,9 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
     auto mock_template_txids = mini_miner.GetMockTemplateTxids();
     // MiniMiner doesn't add a coinbase tx.
     assert(mock_template_txids.count(blocktemplate->block.vtx[0]->GetHash()) == 0);
-    mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash());
-    assert(mock_template_txids.size() <= blocktemplate->block.vtx.size());
-    assert(mock_template_txids.size() >= blocktemplate->block.vtx.size());
+    auto [iter, new_entry] = mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash());
+    assert(new_entry);
+
     assert(mock_template_txids.size() == blocktemplate->block.vtx.size());
     for (const auto& tx : blocktemplate->block.vtx) {
         assert(mock_template_txids.count(tx->GetHash()));

From 66673f1c1302c986e344c7f44bb0b352213d5dc8 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Thu, 4 Jul 2024 20:12:13 +0200
Subject: [PATCH 26/55] net: fix race condition in self-connect detection

Initiating an outbound network connection currently involves the
following steps after the socket connection is established (see
 `CConnman::OpenNetworkConnection` method):
    1. set up node state
    2. queue VERSION message
    3. add new node to vector `m_nodes`

If we connect to ourself, it can happen that the sent VERSION message
(step 2) is received and processed locally *before* the node object
is added to the connection manager's `m_nodes` vector (step 3). In this
case, the self-connect remains undiscovered, as the detection doesn't
find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`).

Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion`
call out of `InitializeNode` and doing that in the `SendMessages` method
instead, which is only called for `CNode` instances in `m_nodes`.

Thanks go to vasild, mzumsande, dergoegge and sipa for suggestions on
how to fix this.
---
 src/net.h              |  2 +-
 src/net_processing.cpp | 16 +++++++++++++---
 src/test/util/net.cpp  |  3 ++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/net.h b/src/net.h
index 8075975d62..050af0d5ac 100644
--- a/src/net.h
+++ b/src/net.h
@@ -991,7 +991,7 @@ class NetEventsInterface
     /** Mutex for anything that is only accessed via the msg processing thread */
     static Mutex g_msgproc_mutex;
 
-    /** Initialize a peer (setup state, queue any initial messages) */
+    /** Initialize a peer (setup state) */
     virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0;
 
     /** Handle removal of a peer (clear state) */
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 8137e17c98..080d6241c9 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -243,6 +243,9 @@ struct Peer {
      * Most peers use headers-first syncing, which doesn't use this mechanism */
     uint256 m_continuation_block GUARDED_BY(m_block_inv_mutex) {};
 
+    /** Set to true once initial VERSION message was sent (only relevant for outbound peers). */
+    bool m_outbound_version_message_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false};
+
     /** This peer's reported block height when we connected */
     std::atomic<int> m_starting_height{-1};
 
@@ -1677,9 +1680,6 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services)
         LOCK(m_peer_mutex);
         m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer);
     }
-    if (!node.IsInboundConn()) {
-        PushNodeVersion(node, *peer);
-    }
 }
 
 void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
@@ -5326,6 +5326,10 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
     PeerRef peer = GetPeerRef(pfrom->GetId());
     if (peer == nullptr) return false;
 
+    // For outbound connections, ensure that the initial VERSION message
+    // has been sent first before processing any incoming messages
+    if (!pfrom->IsInboundConn() && !peer->m_outbound_version_message_sent) return false;
+
     {
         LOCK(peer->m_getdata_requests_mutex);
         if (!peer->m_getdata_requests.empty()) {
@@ -5817,6 +5821,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
     // disconnect misbehaving peers even before the version handshake is complete.
     if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true;
 
+    // Initiate version handshake for outbound connections
+    if (!pto->IsInboundConn() && !peer->m_outbound_version_message_sent) {
+        PushNodeVersion(*pto, *peer);
+        peer->m_outbound_version_message_sent = true;
+    }
+
     // Don't send anything until the version handshake is complete
     if (!pto->fSuccessfullyConnected || pto->fDisconnect)
         return true;
diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp
index 9b38d85d58..e4b2ac5743 100644
--- a/src/test/util/net.cpp
+++ b/src/test/util/net.cpp
@@ -28,7 +28,8 @@ void ConnmanTestMsg::Handshake(CNode& node,
     auto& connman{*this};
 
     peerman.InitializeNode(node, local_services);
-    FlushSendBuffer(node); // Drop the version message added by InitializeNode.
+    peerman.SendMessages(&node);
+    FlushSendBuffer(node); // Drop the version message added by SendMessages.
 
     CSerializedNetMsg msg_version{
         NetMsg::Make(NetMsgType::VERSION,

From 0dbcd4c14855fe2cba15a32245572b693dc18c4e Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Tue, 9 Jul 2024 12:18:23 +0200
Subject: [PATCH 27/55] net: prevent sending messages in
 `NetEventsInterface::InitializeNode`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now that the queueing of the VERSION messages has been moved out of
`InitializeNode`, there is no need to pass a mutable `CNode` reference any
more. With a const reference, trying to send messages in this method would
lead to a compile-time error, e.g.:

----------------------------------------------------------------------------------------------------------------------------------
...
net_processing.cpp: In member function ‘virtual void {anonymous}::PeerManagerImpl::InitializeNode(const CNode&, ServiceFlags)’:
net_processing.cpp:1683:21: error: binding reference of type ‘CNode&’ to ‘const CNode’ discards qualifiers
 1683 |     PushNodeVersion(node, *peer);
...
----------------------------------------------------------------------------------------------------------------------------------
---
 src/net.h              | 2 +-
 src/net_processing.cpp | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/net.h b/src/net.h
index 050af0d5ac..443f6a8de8 100644
--- a/src/net.h
+++ b/src/net.h
@@ -992,7 +992,7 @@ class NetEventsInterface
     static Mutex g_msgproc_mutex;
 
     /** Initialize a peer (setup state) */
-    virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0;
+    virtual void InitializeNode(const CNode& node, ServiceFlags our_services) = 0;
 
     /** Handle removal of a peer (clear state) */
     virtual void FinalizeNode(const CNode& node) = 0;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 080d6241c9..728bfc4c18 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -501,7 +501,7 @@ class PeerManagerImpl final : public PeerManager
         EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex);
 
     /** Implement NetEventsInterface */
-    void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
+    void InitializeNode(const CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex);
     bool HasAllDesirableServiceFlags(ServiceFlags services) const override;
     bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override
@@ -1662,7 +1662,7 @@ void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_s
     if (state) state->m_last_block_announcement = time_in_seconds;
 }
 
-void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services)
+void PeerManagerImpl::InitializeNode(const CNode& node, ServiceFlags our_services)
 {
     NodeId nodeid = node.GetId();
     {

From 16bd283b3ad05daa41259a062aee0fc05b463fa6 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Thu, 4 Jul 2024 20:35:22 +0200
Subject: [PATCH 28/55] Reapply "test: p2p: check that connecting to ourself
 leads to disconnect"

This reverts commit 9ec2c53701a391629b55aeb2804e8060d2c453a4 with
a tiny change included (identation of the wait_until call).
---
 test/functional/p2p_handshake.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/test/functional/p2p_handshake.py b/test/functional/p2p_handshake.py
index 21959ae522..9536e74893 100755
--- a/test/functional/p2p_handshake.py
+++ b/test/functional/p2p_handshake.py
@@ -17,6 +17,7 @@
     NODE_WITNESS,
 )
 from test_framework.p2p import P2PInterface
+from test_framework.util import p2p_port
 
 
 # Desirable service flags for outbound non-pruned and pruned peers. Note that
@@ -88,9 +89,11 @@ def run_test(self):
         with node.assert_debug_log([f"feeler connection completed"]):
             self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True)
 
-        # TODO: re-add test introduced in commit 5d2fb14bafe4e80c0a482d99e5ebde07c477f000
-        # ("test: p2p: check that connecting to ourself leads to disconnect") once
-        # the race condition causing issue #30368 is fixed
+        self.log.info("Check that connecting to ourself leads to immediate disconnect")
+        with node.assert_debug_log(["connected to self", "disconnecting"]):
+            node_listen_addr = f"127.0.0.1:{p2p_port(0)}"
+            node.addconnection(node_listen_addr, "outbound-full-relay", self.options.v2transport)
+            self.wait_until(lambda: len(node.getpeerinfo()) == 0)
 
 
 if __name__ == '__main__':

From aaaa3323f37526862ebf2a2a4bf522c661e6976e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 11 Jul 2024 10:50:28 +0200
Subject: [PATCH 29/55] refactor: Mark IsBlockPruned const

Member fields are used read-only in this method.
---
 src/node/blockstorage.cpp | 2 +-
 src/node/blockstorage.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index fb62e78138..16d8d25615 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -588,7 +588,7 @@ const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data)
     return nullptr;
 }
 
-bool BlockManager::IsBlockPruned(const CBlockIndex& block)
+bool BlockManager::IsBlockPruned(const CBlockIndex& block) const
 {
     AssertLockHeld(::cs_main);
     return m_have_pruned && !(block.nStatus & BLOCK_HAVE_DATA) && (block.nTx > 0);
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 108a08a72b..b4e199af50 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -381,7 +381,7 @@ class BlockManager
     bool m_have_pruned = false;
 
     //! Check whether the block associated with this index entry is pruned or not.
-    bool IsBlockPruned(const CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    bool IsBlockPruned(const CBlockIndex& block) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     //! Create or update a prune lock identified by its name
     void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

From fad59a2f0f37f5b7f6076fd91be43448e35f4b7e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 11 Jul 2024 16:20:06 +0200
Subject: [PATCH 30/55] log: LogError with FlatFilePos in UndoReadFromDisk

These errors should never happen in normal operation. If they do,
knowing the FlatFilePos may be useful to determine if data corruption
happened. Also, handle the error pos.IsNull() as part of OpenUndoFile,
because it may as well have happened due to data corruption.

This mirrors the LogError behavior from ReadBlockFromDisk.
---
 src/node/blockstorage.cpp | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 16d8d25615..fe5762e2c5 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -703,15 +703,10 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in
 {
     const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())};
 
-    if (pos.IsNull()) {
-        LogError("%s: no undo data available\n", __func__);
-        return false;
-    }
-
     // Open history file to read
     AutoFile filein{OpenUndoFile(pos, true)};
     if (filein.IsNull()) {
-        LogError("%s: OpenUndoFile failed\n", __func__);
+        LogError("%s: OpenUndoFile failed for %s\n", __func__, pos.ToString());
         return false;
     }
 
@@ -723,13 +718,13 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in
         verifier >> blockundo;
         filein >> hashChecksum;
     } catch (const std::exception& e) {
-        LogError("%s: Deserialize or I/O error - %s\n", __func__, e.what());
+        LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString());
         return false;
     }
 
     // Verify checksum
     if (hashChecksum != verifier.GetHash()) {
-        LogError("%s: Checksum mismatch\n", __func__);
+        LogError("%s: Checksum mismatch at %s\n", __func__, pos.ToString());
         return false;
     }
 

From fa14e1d9d5c5dc44396a01583ae94480b7bc29ee Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 11 Jul 2024 16:41:42 +0200
Subject: [PATCH 31/55] log: Fix __func__ in LogError in blockstorage module

These errors should never happen. However, when they do happen, it is
useful to log the correct error location (function name).

For example, this fixes an incorrect "ConnectBlock()" in
"WriteUndoDataForBlock".
---
 src/node/blockstorage.cpp | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index fe5762e2c5..43801eb1d4 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -981,7 +981,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
     // Open history file to append
     AutoFile fileout{OpenBlockFile(pos)};
     if (fileout.IsNull()) {
-        LogError("WriteBlockToDisk: OpenBlockFile failed\n");
+        LogError("%s: OpenBlockFile failed\n", __func__);
         return false;
     }
 
@@ -992,7 +992,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
     // Write block
     long fileOutPos = ftell(fileout.Get());
     if (fileOutPos < 0) {
-        LogError("WriteBlockToDisk: ftell failed\n");
+        LogError("%s: ftell failed\n", __func__);
         return false;
     }
     pos.nPos = (unsigned int)fileOutPos;
@@ -1011,7 +1011,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
     if (block.GetUndoPos().IsNull()) {
         FlatFilePos _pos;
         if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) {
-            LogError("ConnectBlock(): FindUndoPos failed\n");
+            LogError("%s: FindUndoPos failed\n", __func__);
             return false;
         }
         if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) {
@@ -1050,7 +1050,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons
     // Open history file to read
     AutoFile filein{OpenBlockFile(pos, true)};
     if (filein.IsNull()) {
-        LogError("ReadBlockFromDisk: OpenBlockFile failed for %s\n", pos.ToString());
+        LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString());
         return false;
     }
 
@@ -1064,13 +1064,13 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons
 
     // Check the header
     if (!CheckProofOfWork(block.GetHash(), block.nBits, GetConsensus())) {
-        LogError("ReadBlockFromDisk: Errors in block header at %s\n", pos.ToString());
+        LogError("%s: Errors in block header at %s\n", __func__, pos.ToString());
         return false;
     }
 
     // Signet only: check block solution
     if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) {
-        LogError("ReadBlockFromDisk: Errors in block solution at %s\n", pos.ToString());
+        LogError("%s: Errors in block solution at %s\n", __func__, pos.ToString());
         return false;
     }
 
@@ -1085,8 +1085,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co
         return false;
     }
     if (block.GetHash() != index.GetBlockHash()) {
-        LogError("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s\n",
-                     index.ToString(), block_pos.ToString());
+        LogError("%s: GetHash() doesn't match index for %s at %s\n", __func__, index.ToString(), block_pos.ToString());
         return false;
     }
     return true;

From 55b6d7be68a6f6c3882588ffd5b9349d885ed953 Mon Sep 17 00:00:00 2001
From: Martin Zumsande <mzumsande@gmail.com>
Date: Thu, 20 Jun 2024 16:14:22 -0400
Subject: [PATCH 32/55] validation: Don't load a snapshot if it's not in the
 best header chain.

If the snapshot is not an ancestor of the most-work header (m_best_header),
syncing from that alternative chain should be prioritised.
Therefore don't accept loading a snapshot in this situation.

If that other chain turns out to be invalid, m_best_header
would be reset and loading the snapshot should be possible again.

Because of the work required to generate a conflicting headers chain,
this should only be possible under extreme circumstances, such as major forks.
---
 src/validation.cpp                    |  4 ++++
 test/functional/feature_assumeutxo.py | 32 ++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 74f0e4975c..e7ea6b492a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5683,6 +5683,10 @@ util::Result<void> ChainstateManager::ActivateSnapshot(
             return util::Error{strprintf(Untranslated("The base block header (%s) is part of an invalid chain"), base_blockhash.ToString())};
         }
 
+        if (!m_best_header || m_best_header->GetAncestor(base_blockheight) != snapshot_start_block) {
+            return util::Error{_("A forked headers-chain with more work than the chain with the snapshot base block header exists. Please proceed to sync without AssumeUtxo.")};
+        }
+
         auto mempool{m_active_chainstate->GetMempool()};
         if (mempool && mempool->size() > 0) {
             return util::Error{Untranslated("Can't activate a snapshot when mempool not empty")};
diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index 688e2866b2..17c1554cde 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -15,20 +15,21 @@
 
 - TODO: Valid snapshot file, but referencing a snapshot block that turns out to be
       invalid, or has an invalid parent
-- TODO: Valid snapshot file and snapshot block, but the block is not on the
-      most-work chain
 
 Interesting starting states could be loading a snapshot when the current chain tip is:
 
 - TODO: An ancestor of snapshot block
 - TODO: The snapshot block
 - TODO: A descendant of the snapshot block
-- TODO: Not an ancestor or a descendant of the snapshot block and has more work
 
 """
 from shutil import rmtree
 
 from dataclasses import dataclass
+from test_framework.blocktools import (
+        create_block,
+        create_coinbase
+)
 from test_framework.messages import tx_from_hex
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -241,6 +242,30 @@ def test_snapshot_in_a_divergent_chain(self, dump_output_path):
         self.sync_blocks(nodes=(n0, n3))
         self.wait_until(lambda: len(n3.getchainstates()['chainstates']) == 1)
 
+    def test_snapshot_not_on_most_work_chain(self, dump_output_path):
+        self.log.info("Test snapshot is not loaded when the node knows the headers of another chain with more work.")
+        node0 = self.nodes[0]
+        node1 = self.nodes[1]
+        # Create an alternative chain of 2 new blocks, forking off the main chain at the block before the snapshot block.
+        # This simulates a longer chain than the main chain when submitting these two block headers to node 1 because it is only aware of
+        # the main chain headers up to the snapshot height.
+        parent_block_hash = node0.getblockhash(SNAPSHOT_BASE_HEIGHT - 1)
+        block_time = node0.getblock(node0.getbestblockhash())['time'] + 1
+        fork_block1 = create_block(int(parent_block_hash, 16), create_coinbase(SNAPSHOT_BASE_HEIGHT), block_time)
+        fork_block1.solve()
+        fork_block2 = create_block(fork_block1.sha256, create_coinbase(SNAPSHOT_BASE_HEIGHT + 1), block_time + 1)
+        fork_block2.solve()
+        node1.submitheader(fork_block1.serialize().hex())
+        node1.submitheader(fork_block2.serialize().hex())
+        msg = "A forked headers-chain with more work than the chain with the snapshot base block header exists. Please proceed to sync without AssumeUtxo."
+        assert_raises_rpc_error(-32603, msg, node1.loadtxoutset, dump_output_path)
+        # Cleanup: submit two more headers of the snapshot chain to node 1, so that it is the most-work chain again and loading
+        # the snapshot in future subtests succeeds
+        main_block1 = node0.getblock(node0.getblockhash(SNAPSHOT_BASE_HEIGHT + 1), 0)
+        main_block2 = node0.getblock(node0.getblockhash(SNAPSHOT_BASE_HEIGHT + 2), 0)
+        node1.submitheader(main_block1)
+        node1.submitheader(main_block2)
+
     def run_test(self):
         """
         Bring up two (disconnected) nodes, mine some new blocks on the first,
@@ -330,6 +355,7 @@ def run_test(self):
         self.test_invalid_chainstate_scenarios()
         self.test_invalid_file_path()
         self.test_snapshot_block_invalidated(dump_output['path'])
+        self.test_snapshot_not_on_most_work_chain(dump_output['path'])
 
         self.log.info(f"Loading snapshot into second node from {dump_output['path']}")
         loaded = n1.loadtxoutset(dump_output['path'])

From 9010b1343b9f931f771d3d49dd03b57868c24d5d Mon Sep 17 00:00:00 2001
From: Cory Fields <cory-nospam-@coryfields.com>
Date: Thu, 4 Jul 2024 20:23:51 +0000
Subject: [PATCH 33/55] contrib: c++ify test stubs after switching to c++
 compilers

---
 contrib/devtools/test-security-check.py | 4 ++--
 contrib/devtools/test-symbol-check.py   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py
index f1ea93dc7f..de372cbd39 100755
--- a/contrib/devtools/test-security-check.py
+++ b/contrib/devtools/test-security-check.py
@@ -15,10 +15,10 @@
 def write_testcode(filename):
     with open(filename, 'w', encoding="utf8") as f:
         f.write('''
-    #include <stdio.h>
+    #include <cstdio>
     int main()
     {
-        printf("the quick brown fox jumps over the lazy god\\n");
+        std::printf("the quick brown fox jumps over the lazy god\\n");
         return 0;
     }
     ''')
diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py
index 517626c285..454dbc6bd2 100755
--- a/contrib/devtools/test-symbol-check.py
+++ b/contrib/devtools/test-symbol-check.py
@@ -57,11 +57,11 @@ def test_ELF(self):
         executable = 'test3'
         with open(source, 'w', encoding="utf8") as f:
             f.write('''
-                #include <stdio.h>
+                #include <cstdio>
 
                 int main()
                 {
-                    printf("42");
+                    std::printf("42");
                     return 0;
                 }
         ''')

From fa6270737eb9655bfb4e29b7070ecb6cd2087b7f Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu, 11 Jul 2024 17:38:43 +0200
Subject: [PATCH 34/55] rpc: Use CHECK_NONFATAL over Assert

---
 src/rpc/blockchain.cpp       |  4 ++--
 test/lint/lint-assertions.py | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 1019c0de24..9899a13a1e 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -801,14 +801,14 @@ std::optional<int> GetPruneHeight(const BlockManager& blockman, const CChain& ch
     // If the chain tip is pruned, everything is pruned.
     if (!((chain_tip->nStatus & BLOCK_HAVE_MASK) == BLOCK_HAVE_MASK)) return chain_tip->nHeight;
 
-    const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
+    const auto& first_unpruned{*CHECK_NONFATAL(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
     if (&first_unpruned == first_block) {
         // All blocks between first_block and chain_tip have data, so nothing is pruned.
         return std::nullopt;
     }
 
     // Block before the first unpruned block is the last pruned block.
-    return Assert(first_unpruned.pprev)->nHeight;
+    return CHECK_NONFATAL(first_unpruned.pprev)->nHeight;
 }
 
 static RPCHelpMan pruneblockchain()
diff --git a/test/lint/lint-assertions.py b/test/lint/lint-assertions.py
index d9f86b22b8..5d01b13fd4 100755
--- a/test/lint/lint-assertions.py
+++ b/test/lint/lint-assertions.py
@@ -27,8 +27,9 @@ def main():
     # 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([
-        "-nE",
-        r"\<(A|a)ss(ume|ert) *\(.*\);",
+        "--line-number",
+        "--extended-regexp",
+        r"\<(A|a)ss(ume|ert)\(",
         "--",
         "src/rpc/",
         "src/wallet/rpc*",
@@ -38,8 +39,9 @@ def main():
     # The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`,
     # which is an unnecessary Boost dependency.
     exit_code |= git_grep([
-        "-E",
-        r"BOOST_ASSERT *\(.*\);",
+        "--line-number",
+        "--extended-regexp",
+        r"BOOST_ASSERT\(",
         "--",
         "*.cpp",
         "*.h",

From 5fd48360198d2ac49e43b24cc1469557b03567b8 Mon Sep 17 00:00:00 2001
From: Martin Zumsande <mzumsande@gmail.com>
Date: Fri, 12 Jul 2024 01:05:16 -0400
Subject: [PATCH 35/55] init: change shutdown order of load block thread and
 scheduler

This avoids situations during a reindex in which shutdown
doesn't finish since SyncWithValidationInterfaceQueue is
called by the load block thread when the scheduler is already stopped.
---
 src/init.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index e4b65fbfa9..03969b00bb 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -296,10 +296,11 @@ void Shutdown(NodeContext& node)
 
     StopTorControl();
 
+    if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
     // After everything has been shut down, but before things get flushed, stop the
-    // scheduler and load block thread.
+    // the scheduler. After this point, SyncWithValidationInterfaceQueue() should not be called anymore
+    // as this would prevent the shutdown from completing.
     if (node.scheduler) node.scheduler->stop();
-    if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
 
     // After the threads that potentially access these pointers have been stopped,
     // destruct and reset all to nullptr.

From fab54db9f1d0e634f4a697480dbb87b87940dc5c Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Fri, 12 Jul 2024 17:48:40 +0200
Subject: [PATCH 36/55] rest: Reject negative outpoint index in getutxos
 parsing

---
 src/rest.cpp                      | 7 ++++---
 test/functional/interface_rest.py | 5 ++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/rest.cpp b/src/rest.cpp
index 4abbc4d2ca..185508d2c9 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -788,14 +788,15 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
 
         for (size_t i = (fCheckMemPool) ? 1 : 0; i < uriParts.size(); i++)
         {
-            int32_t nOutput;
             std::string strTxid = uriParts[i].substr(0, uriParts[i].find('-'));
             std::string strOutput = uriParts[i].substr(uriParts[i].find('-')+1);
+            auto output{ToIntegral<uint32_t>(strOutput)};
 
-            if (!ParseInt32(strOutput, &nOutput) || !IsHex(strTxid))
+            if (!output || !IsHex(strTxid)) {
                 return RESTERR(req, HTTP_BAD_REQUEST, "Parse error");
+            }
 
-            vOutPoints.emplace_back(TxidFromString(strTxid), (uint32_t)nOutput);
+            vOutPoints.emplace_back(TxidFromString(strTxid), *output);
         }
 
         if (vOutPoints.size() > 0)
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index ae8d6b226d..aa201cb85b 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -201,10 +201,13 @@ def run_test(self):
         json_obj = self.test_rest_request(f"/getutxos/checkmempool/{spending[0]}-{spending[1]}")
         assert_equal(len(json_obj['utxos']), 1)
 
-        # Do some invalid requests
+        self.log.info("Check some invalid requests")
         self.test_rest_request("/getutxos", http_method='POST', req_type=ReqType.JSON, body='{"checkmempool', status=400, ret_type=RetType.OBJ)
         self.test_rest_request("/getutxos", http_method='POST', req_type=ReqType.BIN, body='{"checkmempool', status=400, ret_type=RetType.OBJ)
         self.test_rest_request("/getutxos/checkmempool", http_method='POST', req_type=ReqType.JSON, status=400, ret_type=RetType.OBJ)
+        self.test_rest_request(f"/getutxos/{spending[0]}_+1", ret_type=RetType.OBJ, status=400)
+        self.test_rest_request(f"/getutxos/{spending[0]}-+1", ret_type=RetType.OBJ, status=400)
+        self.test_rest_request(f"/getutxos/{spending[0]}--1", ret_type=RetType.OBJ, status=400)
 
         # Test limits
         long_uri = '/'.join([f"{txid}-{n_}" for n_ in range(20)])

From 23333b7ed243071c9b4e4f04c727556d8065acbb Mon Sep 17 00:00:00 2001
From: Max Edwards <youwontforgetthis@gmail.com>
Date: Fri, 7 Jun 2024 13:24:04 +0100
Subject: [PATCH 37/55] net: Allow DNS lookups on nodes with IPV6 lo only

AI_ADDRCONFIG prevents ::1 from being considered a valid address on hosts that have a IPV6 loopback IP address but no other IPV6 interfaces.
---
 src/netbase.cpp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/netbase.cpp b/src/netbase.cpp
index e231766487..22326b0e98 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -48,6 +48,7 @@ std::vector<CNetAddr> WrappedGetAddrInfo(const std::string& name, bool allow_loo
     ai_hint.ai_protocol = IPPROTO_TCP;
     // We don't care which address family (IPv4 or IPv6) is returned
     ai_hint.ai_family = AF_UNSPEC;
+
     // If we allow lookups of hostnames, use the AI_ADDRCONFIG flag to only
     // return addresses whose family we have an address configured for.
     //
@@ -59,7 +60,17 @@ std::vector<CNetAddr> WrappedGetAddrInfo(const std::string& name, bool allow_loo
     addrinfo* ai_res{nullptr};
     const int n_err{getaddrinfo(name.c_str(), nullptr, &ai_hint, &ai_res)};
     if (n_err != 0) {
-        return {};
+        if ((ai_hint.ai_flags & AI_ADDRCONFIG) == AI_ADDRCONFIG) {
+            // AI_ADDRCONFIG on some systems may exclude loopback-only addresses
+            // If first lookup failed we perform a second lookup without AI_ADDRCONFIG
+            ai_hint.ai_flags = (ai_hint.ai_flags & ~AI_ADDRCONFIG);
+            const int n_err_retry{getaddrinfo(name.c_str(), nullptr, &ai_hint, &ai_res)};
+            if (n_err_retry != 0) {
+                return {};
+            }
+        } else {
+            return {};
+        }
     }
 
     // Traverse the linked list starting with ai_trav.

From 8d7340105f5299e9a45e84f1704b8b4545cb85f0 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Thu, 30 May 2024 11:14:17 +0200
Subject: [PATCH 38/55] fuzz: limit the number of sub-fragments per fragment
 for descriptors

This target may call into logic quadratic over the number of
sub-fragments. Limit the number of sub-fragments to keep the runtime
reasonable.

Thanks to Marco Falke for reporting the fuzz timeouts with a minimized
input.
---
 src/test/fuzz/descriptor_parse.cpp |  7 ++++++-
 src/test/fuzz/util/descriptor.cpp  | 26 ++++++++++++++++++++++++++
 src/test/fuzz/util/descriptor.h    | 12 ++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index b9a5560ffb..973934f0c5 100644
--- a/src/test/fuzz/descriptor_parse.cpp
+++ b/src/test/fuzz/descriptor_parse.cpp
@@ -72,6 +72,10 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
     // out strings which could correspond to a descriptor containing a too large derivation path.
     if (HasDeepDerivPath(buffer)) return;
 
+    // Some fragments can take a virtually unlimited number of sub-fragments (thresh, multi_a) but
+    // may perform quadratic operations on them. Limit the number of sub-fragments per fragment.
+    if (HasTooManySubFrag(buffer)) return;
+
     const std::string mocked_descriptor{buffer.begin(), buffer.end()};
     if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
         FlatSigningProvider signing_provider;
@@ -83,8 +87,9 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
 
 FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse)
 {
-    // See comment above for rationale.
+    // See comments above for rationales.
     if (HasDeepDerivPath(buffer)) return;
+    if (HasTooManySubFrag(buffer)) return;
 
     const std::string descriptor(buffer.begin(), buffer.end());
     FlatSigningProvider signing_provider;
diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
index 0fed2bc5e1..61c43f190a 100644
--- a/src/test/fuzz/util/descriptor.cpp
+++ b/src/test/fuzz/util/descriptor.cpp
@@ -4,6 +4,8 @@
 
 #include <test/fuzz/util/descriptor.h>
 
+#include <stack>
+
 void MockedDescriptorConverter::Init() {
     // The data to use as a private key or a seed for an xprv.
     std::array<std::byte, 32> key_data{std::byte{1}};
@@ -84,3 +86,27 @@ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth)
     }
     return false;
 }
+
+bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const size_t max_nested_subs)
+{
+    // We use a stack because there may be many nested sub-frags.
+    std::stack<int> counts;
+    for (const auto& ch: buff) {
+        // The fuzzer may generate an input with a ton of parentheses. Rule out pathological cases.
+        if (counts.size() > max_nested_subs) return true;
+
+        if (ch == '(') {
+            // A new fragment was opened, create a new sub-count for it and start as one since any fragment with
+            // parentheses has at least one sub.
+            counts.push(1);
+        } else if (ch == ',' && !counts.empty()) {
+            // When encountering a comma, account for an additional sub in the last opened fragment. If it exceeds the
+            // limit, bail.
+            if (++counts.top() > max_subs) return true;
+        } else if (ch == ')' && !counts.empty()) {
+            // Fragment closed! Drop its sub count and resume to counting the number of subs for its parent.
+            counts.pop();
+        }
+    }
+    return false;
+}
diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h
index cd41dbafa3..21cf45fcfb 100644
--- a/src/test/fuzz/util/descriptor.h
+++ b/src/test/fuzz/util/descriptor.h
@@ -55,4 +55,16 @@ constexpr int MAX_DEPTH{2};
  */
 bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH);
 
+//! Default maximum number of sub-fragments.
+constexpr int MAX_SUBS{1'000};
+//! Maximum number of nested sub-fragments we'll allow in a descriptor.
+constexpr size_t MAX_NESTED_SUBS{10'000};
+
+/**
+ * Whether the buffer, if it represents a valid descriptor, contains a fragment with more
+ * sub-fragments than the given maximum.
+ */
+bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs = MAX_SUBS,
+                       const size_t max_nested_subs = MAX_NESTED_SUBS);
+
 #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

From bc34bc288824978ef4b98e8802b47cb863c8a8c2 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Thu, 30 May 2024 12:52:44 +0200
Subject: [PATCH 39/55] fuzz: limit the number of nested wrappers in
 descriptors

The script building logic performs a quadratic number of copies in the
number of nested wrappers in the miniscript. Limit the number of nested
wrappers to avoid fuzz timeouts.

Thanks to Marco Falke for reporting the fuzz timeouts and providing a
minimal input to reproduce.
---
 src/test/fuzz/descriptor_parse.cpp |  5 +++++
 src/test/fuzz/util/descriptor.cpp  | 33 ++++++++++++++++++++++++++++++
 src/test/fuzz/util/descriptor.h    |  9 ++++++++
 3 files changed, 47 insertions(+)

diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index 973934f0c5..6a3f4d6dfe 100644
--- a/src/test/fuzz/descriptor_parse.cpp
+++ b/src/test/fuzz/descriptor_parse.cpp
@@ -76,6 +76,10 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse)
     // may perform quadratic operations on them. Limit the number of sub-fragments per fragment.
     if (HasTooManySubFrag(buffer)) return;
 
+    // The script building logic performs quadratic copies in the number of nested wrappers. Limit
+    // the number of nested wrappers per fragment.
+    if (HasTooManyWrappers(buffer)) return;
+
     const std::string mocked_descriptor{buffer.begin(), buffer.end()};
     if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) {
         FlatSigningProvider signing_provider;
@@ -90,6 +94,7 @@ FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse)
     // See comments above for rationales.
     if (HasDeepDerivPath(buffer)) return;
     if (HasTooManySubFrag(buffer)) return;
+    if (HasTooManyWrappers(buffer)) return;
 
     const std::string descriptor(buffer.begin(), buffer.end());
     FlatSigningProvider signing_provider;
diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp
index 61c43f190a..9e52e990a2 100644
--- a/src/test/fuzz/util/descriptor.cpp
+++ b/src/test/fuzz/util/descriptor.cpp
@@ -4,6 +4,7 @@
 
 #include <test/fuzz/util/descriptor.h>
 
+#include <ranges>
 #include <stack>
 
 void MockedDescriptorConverter::Init() {
@@ -110,3 +111,35 @@ bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const siz
     }
     return false;
 }
+
+bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers)
+{
+    // The number of nested wrappers. Nested wrappers are always characters which follow each other so we don't have to
+    // use a stack as we do above when counting the number of sub-fragments.
+    std::optional<int> count;
+
+    // We want to detect nested wrappers. A wrapper is a character prepended to a fragment, separated by a colon. There
+    // may be more than one wrapper, in which case the colon is not repeated. For instance `jjjjj:pk()`.  To count
+    // wrappers we iterate in reverse and use the colon to detect the end of a wrapper expression and count how many
+    // characters there are since the beginning of the expression. We stop counting when we encounter a character
+    // indicating the beginning of a new expression.
+    for (const auto ch: buff | std::views::reverse) {
+        // A colon, start counting.
+        if (ch == ':') {
+            // The colon itself is not a wrapper so we start at 0.
+            count = 0;
+        } else if (count) {
+            // If we are counting wrappers, stop when we crossed the beginning of the wrapper expression. Otherwise keep
+            // counting and bail if we reached the limit.
+            // A wrapper may only ever occur as the first sub of a descriptor/miniscript expression ('('), as the
+            // first Taproot leaf in a pair ('{') or as the nth sub in each case (',').
+            if (ch == ',' || ch == '(' || ch == '{') {
+                count.reset();
+            } else if (++*count > max_wrappers) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h
index 21cf45fcfb..ea928c39f0 100644
--- a/src/test/fuzz/util/descriptor.h
+++ b/src/test/fuzz/util/descriptor.h
@@ -67,4 +67,13 @@ constexpr size_t MAX_NESTED_SUBS{10'000};
 bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs = MAX_SUBS,
                        const size_t max_nested_subs = MAX_NESTED_SUBS);
 
+//! Default maximum number of wrappers per fragment.
+constexpr int MAX_WRAPPERS{100};
+
+/**
+ * Whether the buffer, if it represents a valid descriptor, contains a fragment with more
+ * wrappers than the given maximum.
+ */
+bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers = MAX_WRAPPERS);
+
 #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H

From c807f3322897ca8c0da114556e5936e389da5059 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Sun, 13 Aug 2023 09:29:28 +0530
Subject: [PATCH 40/55] Add fuzz test for AEADChacha20Poly1305

---
 src/Makefile.test.include                 |   1 +
 src/test/fuzz/crypto_chacha20poly1305.cpp | 103 ++++++++++++++++++++++
 2 files changed, 104 insertions(+)
 create mode 100644 src/test/fuzz/crypto_chacha20poly1305.cpp

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 8651eea942..0993a65eff 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -309,6 +309,7 @@ test_fuzz_fuzz_SOURCES = \
  test/fuzz/crypto_aes256.cpp \
  test/fuzz/crypto_aes256cbc.cpp \
  test/fuzz/crypto_chacha20.cpp \
+ test/fuzz/crypto_chacha20poly1305.cpp \
  test/fuzz/crypto_common.cpp \
  test/fuzz/crypto_diff_fuzz_chacha20.cpp \
  test/fuzz/crypto_hkdf_hmac_sha256_l32.cpp \
diff --git a/src/test/fuzz/crypto_chacha20poly1305.cpp b/src/test/fuzz/crypto_chacha20poly1305.cpp
new file mode 100644
index 0000000000..96188d2230
--- /dev/null
+++ b/src/test/fuzz/crypto_chacha20poly1305.cpp
@@ -0,0 +1,103 @@
+// Copyright (c) 2020-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <crypto/chacha20poly1305.h>
+#include <random.h>
+#include <span.h>
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+
+#include <cstddef>
+#include <cstdint>
+#include <vector>
+
+FUZZ_TARGET(crypto_aeadchacha20poly1305)
+{
+    FuzzedDataProvider provider{buffer.data(), buffer.size()};
+
+    auto key = provider.ConsumeBytes<std::byte>(32);
+    key.resize(32);
+    AEADChaCha20Poly1305 aead(key);
+
+    // Initialize RNG deterministically, to generate contents and AAD. We assume that there are no
+    // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid
+    // reading the actual data for those from the fuzzer input (which would need large amounts of
+    // data).
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
+
+    LIMITED_WHILE(provider.ConsumeBool(), 10000)
+    {
+        // Mode:
+        // - Bit 0: whether to use single-plain Encrypt/Decrypt; otherwise use a split at prefix.
+        // - Bit 2: whether this ciphertext will be corrupted (making it the last sent one)
+        // - Bit 3-4: controls the maximum aad length (max 511 bytes)
+        // - Bit 5-7: controls the maximum content length (max 16383 bytes, for performance reasons)
+        unsigned mode = provider.ConsumeIntegral<uint8_t>();
+        bool use_splits = mode & 1;
+        bool damage = mode & 4;
+        unsigned aad_length_bits = 3 * ((mode >> 3) & 3);
+        unsigned aad_length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << aad_length_bits) - 1);
+        unsigned length_bits = 2 * ((mode >> 5) & 7);
+        unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1);
+        // Generate aad and content.
+        auto aad = rng.randbytes<std::byte>(aad_length);
+        auto plain = rng.randbytes<std::byte>(length);
+        std::vector<std::byte> cipher(length + AEADChaCha20Poly1305::EXPANSION);
+        // Generate nonce
+        AEADChaCha20Poly1305::Nonce96 nonce = {(uint32_t)rng(), rng()};
+
+        if (use_splits && length > 0) {
+            size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length);
+            aead.Encrypt(Span{plain}.first(split_index), Span{plain}.subspan(split_index), aad, nonce, cipher);
+        } else {
+            aead.Encrypt(plain, aad, nonce, cipher);
+        }
+
+        // Test Keystream output
+        std::vector<std::byte> keystream(length);
+        aead.Keystream(nonce, keystream);
+        for (size_t i = 0; i < length; ++i) {
+            assert((plain[i] ^ keystream[i]) == cipher[i]);
+        }
+
+        std::vector<std::byte> decrypted_contents(length);
+        bool ok{false};
+
+        // damage the key
+        unsigned key_position = provider.ConsumeIntegralInRange<unsigned>(0, 31);
+        std::byte damage_val{(uint8_t)(1U << (key_position & 7))};
+        std::vector<std::byte> bad_key = key;
+        bad_key[key_position] ^= damage_val;
+
+        AEADChaCha20Poly1305 bad_aead(bad_key);
+        ok = bad_aead.Decrypt(cipher, aad, nonce, decrypted_contents);
+        assert(!ok);
+
+        // Optionally damage 1 bit in either the cipher (corresponding to a change in transit)
+        // or the aad (to make sure that decryption will fail if the AAD mismatches).
+        if (damage) {
+            unsigned damage_bit = provider.ConsumeIntegralInRange<unsigned>(0, (cipher.size() + aad.size()) * 8U - 1U);
+            unsigned damage_pos = damage_bit >> 3;
+            std::byte damage_val{(uint8_t)(1U << (damage_bit & 7))};
+            if (damage_pos >= cipher.size()) {
+                aad[damage_pos - cipher.size()] ^= damage_val;
+            } else {
+                cipher[damage_pos] ^= damage_val;
+            }
+        }
+
+        if (use_splits && length > 0) {
+            size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length);
+            ok = aead.Decrypt(cipher, aad, nonce, Span{decrypted_contents}.first(split_index), Span{decrypted_contents}.subspan(split_index));
+        } else {
+            ok = aead.Decrypt(cipher, aad, nonce, decrypted_contents);
+        }
+
+        // Decryption *must* fail if the packet was damaged, and succeed if it wasn't.
+        assert(!ok == damage);
+        if (!ok) break;
+        assert(decrypted_contents == plain);
+    }
+}

From 8607773750e60931e51a33e48cd077a1dedf9db3 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Sun, 13 Aug 2023 09:30:19 +0530
Subject: [PATCH 41/55] Add fuzz test for FSChaCha20Poly1305

---
 src/test/fuzz/crypto_chacha20poly1305.cpp | 97 +++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/src/test/fuzz/crypto_chacha20poly1305.cpp b/src/test/fuzz/crypto_chacha20poly1305.cpp
index 96188d2230..2b39a06094 100644
--- a/src/test/fuzz/crypto_chacha20poly1305.cpp
+++ b/src/test/fuzz/crypto_chacha20poly1305.cpp
@@ -13,6 +13,18 @@
 #include <cstdint>
 #include <vector>
 
+constexpr static inline void crypt_till_rekey(FSChaCha20Poly1305& aead, int rekey_interval, bool encrypt)
+{
+    for (int i = 0; i < rekey_interval; ++i) {
+        std::byte dummy_tag[FSChaCha20Poly1305::EXPANSION] = {{}};
+        if (encrypt) {
+            aead.Encrypt(Span{dummy_tag}.first(0), Span{dummy_tag}.first(0), dummy_tag);
+        } else {
+            aead.Decrypt(dummy_tag, Span{dummy_tag}.first(0), Span{dummy_tag}.first(0));
+        }
+    }
+}
+
 FUZZ_TARGET(crypto_aeadchacha20poly1305)
 {
     FuzzedDataProvider provider{buffer.data(), buffer.size()};
@@ -101,3 +113,88 @@ FUZZ_TARGET(crypto_aeadchacha20poly1305)
         assert(decrypted_contents == plain);
     }
 }
+
+FUZZ_TARGET(crypto_fschacha20poly1305)
+{
+    FuzzedDataProvider provider{buffer.data(), buffer.size()};
+
+    uint32_t rekey_interval = provider.ConsumeIntegralInRange<size_t>(32, 512);
+    auto key = provider.ConsumeBytes<std::byte>(32);
+    key.resize(32);
+    FSChaCha20Poly1305 enc_aead(key, rekey_interval);
+    FSChaCha20Poly1305 dec_aead(key, rekey_interval);
+
+    // Initialize RNG deterministically, to generate contents and AAD. We assume that there are no
+    // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid
+    // reading the actual data for those from the fuzzer input (which would need large amounts of
+    // data).
+    InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());
+
+    LIMITED_WHILE(provider.ConsumeBool(), 10000)
+    {
+        // Mode:
+        // - Bit 0: whether to use single-plain Encrypt/Decrypt; otherwise use a split at prefix.
+        // - Bit 2: whether this ciphertext will be corrupted (making it the last sent one)
+        // - Bit 3-4: controls the maximum aad length (max 511 bytes)
+        // - Bit 5-7: controls the maximum content length (max 16383 bytes, for performance reasons)
+        unsigned mode = provider.ConsumeIntegral<uint8_t>();
+        bool use_splits = mode & 1;
+        bool damage = mode & 4;
+        unsigned aad_length_bits = 3 * ((mode >> 3) & 3);
+        unsigned aad_length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << aad_length_bits) - 1);
+        unsigned length_bits = 2 * ((mode >> 5) & 7);
+        unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1);
+        // Generate aad and content.
+        auto aad = rng.randbytes<std::byte>(aad_length);
+        auto plain = rng.randbytes<std::byte>(length);
+        std::vector<std::byte> cipher(length + FSChaCha20Poly1305::EXPANSION);
+
+        crypt_till_rekey(enc_aead, rekey_interval, true);
+        if (use_splits && length > 0) {
+            size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length);
+            enc_aead.Encrypt(Span{plain}.first(split_index), Span{plain}.subspan(split_index), aad, cipher);
+        } else {
+            enc_aead.Encrypt(plain, aad, cipher);
+        }
+
+        std::vector<std::byte> decrypted_contents(length);
+        bool ok{false};
+
+        // damage the key
+        unsigned key_position = provider.ConsumeIntegralInRange<unsigned>(0, 31);
+        std::byte damage_val{(uint8_t)(1U << (key_position & 7))};
+        std::vector<std::byte> bad_key = key;
+        bad_key[key_position] ^= damage_val;
+
+        FSChaCha20Poly1305 bad_fs_aead(bad_key, rekey_interval);
+        crypt_till_rekey(bad_fs_aead, rekey_interval, false);
+        ok = bad_fs_aead.Decrypt(cipher, aad, decrypted_contents);
+        assert(!ok);
+
+        // Optionally damage 1 bit in either the cipher (corresponding to a change in transit)
+        // or the aad (to make sure that decryption will fail if the AAD mismatches).
+        if (damage) {
+            unsigned damage_bit = provider.ConsumeIntegralInRange<unsigned>(0, (cipher.size() + aad.size()) * 8U - 1U);
+            unsigned damage_pos = damage_bit >> 3;
+            std::byte damage_val{(uint8_t)(1U << (damage_bit & 7))};
+            if (damage_pos >= cipher.size()) {
+                aad[damage_pos - cipher.size()] ^= damage_val;
+            } else {
+                cipher[damage_pos] ^= damage_val;
+            }
+        }
+
+        crypt_till_rekey(dec_aead, rekey_interval, false);
+        if (use_splits && length > 0) {
+            size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length);
+            ok = dec_aead.Decrypt(cipher, aad, Span{decrypted_contents}.first(split_index), Span{decrypted_contents}.subspan(split_index));
+        } else {
+            ok = dec_aead.Decrypt(cipher, aad, decrypted_contents);
+        }
+
+        // Decryption *must* fail if the packet was damaged, and succeed if it wasn't.
+        assert(!ok == damage);
+        if (!ok) break;
+        assert(decrypted_contents == plain);
+    }
+}

From c6d43367a1ec47c004991143f031417c4e4b8095 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Wed, 10 Jul 2024 15:15:50 +0530
Subject: [PATCH 42/55] test: Fix intermittent failure in p2p_v2_misbehaving.py

The ellswift bytes are computed in the NetworkThread and sent in
the MainThread. Add a `wait_until()` to make sure that ellswift
computation is completed in NetworkThread before sending it in
the MainThread. Also wait until bytes sent are actually received
and use mocktime for more robust disconnection checking.
---
 test/functional/p2p_v2_misbehaving.py | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/test/functional/p2p_v2_misbehaving.py b/test/functional/p2p_v2_misbehaving.py
index e45a63b3b0..9017ee9455 100755
--- a/test/functional/p2p_v2_misbehaving.py
+++ b/test/functional/p2p_v2_misbehaving.py
@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 import random
+import time
 from enum import Enum
 
 from test_framework.messages import MAGIC_BYTES
@@ -136,6 +137,7 @@ def test_earlykeyresponse(self):
         self.log.info('Sending ellswift bytes in parts to ensure that response from responder is received only when')
         self.log.info('ellswift bytes have a mismatch from the 16 bytes(network magic followed by "version\\x00\\x00\\x00\\x00\\x00")')
         node0 = self.nodes[0]
+        node0.setmocktime(int(time.time()))
         self.log.info('Sending first 4 bytes of ellswift which match network magic')
         self.log.info('If a response is received, assertion failure would happen in our custom data_received() function')
         peer1 = node0.add_p2p_connection(MisbehavingV2Peer(TestType.EARLY_KEY_RESPONSE), wait_for_verack=False, send_version=False, supports_v2_p2p=True, wait_for_v2_handshake=False)
@@ -143,9 +145,14 @@ def test_earlykeyresponse(self):
         self.log.info('Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is')
         self.log.info('expected now, our custom data_received() function wouldn\'t result in assertion failure')
         peer1.v2_state.can_data_be_received = True
+        self.wait_until(lambda: peer1.v2_state.ellswift_ours)
         peer1.send_raw_message(peer1.v2_state.ellswift_ours[4:] + peer1.v2_state.sent_garbage)
+        node0.bumpmocktime(3)
+        # Ensure that the bytes sent after 4 bytes network magic are actually received.
+        self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4)
         with node0.assert_debug_log(['V2 handshake timeout peer=0']):
-            peer1.wait_for_disconnect(timeout=5)
+            node0.bumpmocktime(1)  # `InactivityCheck()` triggers now
+            peer1.wait_for_disconnect(timeout=1)
         self.log.info('successful disconnection since modified ellswift was sent as response')
 
     def test_v2disconnection(self):
@@ -154,7 +161,7 @@ def test_v2disconnection(self):
         expected_debug_message = [
             [],  # EARLY_KEY_RESPONSE
             ["V2 transport error: missing garbage terminator, peer=1"],  # EXCESS_GARBAGE
-            ["V2 handshake timeout peer=2"],  # WRONG_GARBAGE_TERMINATOR
+            ["V2 handshake timeout peer=3"],  # WRONG_GARBAGE_TERMINATOR
             ["V2 transport error: packet decryption failure"],  # WRONG_GARBAGE
             ["V2 transport error: packet decryption failure"],  # SEND_NO_AAD
             [],  # SEND_NON_EMPTY_VERSION_PACKET
@@ -167,8 +174,13 @@ def test_v2disconnection(self):
                 self.log.info(f"No disconnection for {test_type.name}")
             else:
                 with node0.assert_debug_log(expected_debug_message[test_type.value], timeout=5):
-                    peer = node0.add_p2p_connection(MisbehavingV2Peer(test_type), wait_for_verack=False, send_version=False, supports_v2_p2p=True, expect_success=False)
-                    peer.wait_for_disconnect()
+                    node0.setmocktime(int(time.time()))
+                    peer1 = node0.add_p2p_connection(MisbehavingV2Peer(test_type), wait_for_verack=False, send_version=False, supports_v2_p2p=True, expect_success=False)
+                    # Make a passing connection for more robust disconnection checking.
+                    peer2 = node0.add_p2p_connection(P2PInterface())
+                    assert peer2.is_connected
+                    node0.bumpmocktime(4)  # `InactivityCheck()` triggers now
+                    peer1.wait_for_disconnect()
                 self.log.info(f"Expected disconnection for {test_type.name}")
 
 

From 323cfed5959b25c98235ec988b408fc5e3391e3c Mon Sep 17 00:00:00 2001
From: Sjors Provoost <sjors@sprovoost.nl>
Date: Fri, 28 Jun 2024 10:35:13 +0200
Subject: [PATCH 43/55] refactor: use CHECK_NONFATAL to avoid single-use symbol

---
 src/rpc/mining.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 7e420dcd9b..e16f75ab78 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -778,9 +778,7 @@ static RPCHelpMan getblocktemplate()
         }
         ENTER_CRITICAL_SECTION(cs_main);
 
-        std::optional<uint256> maybe_tip{miner.getTipHash()};
-        CHECK_NONFATAL(maybe_tip);
-        tip = maybe_tip.value();
+        tip = CHECK_NONFATAL(miner.getTipHash()).value();
 
         if (!IsRPCRunning())
             throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down");

From 6b4c817d4b978adf69738677c74855ef0675f333 Mon Sep 17 00:00:00 2001
From: Sjors Provoost <sjors@sprovoost.nl>
Date: Tue, 16 Jul 2024 10:13:28 +0200
Subject: [PATCH 44/55] refactor: pass BlockCreateOptions to createNewBlock

Rather than pass options individually to createNewBlock and then
combining them into BlockAssembler::Options, this commit introduces
BlockCreateOptions and passes that instead.

Currently there's only one option (use_mempool) but the next
commit adds more.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/interfaces/mining.h | 8 +++++---
 src/node/interfaces.cpp | 9 ++++-----
 src/node/miner.cpp      | 2 +-
 src/node/miner.h        | 3 ++-
 src/node/types.h        | 9 +++++++++
 src/rpc/mining.cpp      | 2 +-
 6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
index 974490561a..cebe97edb7 100644
--- a/src/interfaces/mining.h
+++ b/src/interfaces/mining.h
@@ -5,9 +5,11 @@
 #ifndef BITCOIN_INTERFACES_MINING_H
 #define BITCOIN_INTERFACES_MINING_H
 
+#include <node/types.h>
+#include <uint256.h>
+
 #include <memory>
 #include <optional>
-#include <uint256.h>
 
 namespace node {
 struct CBlockTemplate;
@@ -41,10 +43,10 @@ class Mining
      * Construct a new block template
      *
      * @param[in] script_pub_key the coinbase output
-     * @param[in] use_mempool set false to omit mempool transactions
+     * @param[in] options options for creating the block
      * @returns a block template
      */
-    virtual std::unique_ptr<node::CBlockTemplate> createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0;
+    virtual std::unique_ptr<node::CBlockTemplate> createNewBlock(const CScript& script_pub_key, const node::BlockCreateOptions& options={}) = 0;
 
     /**
      * Processes new block. A valid new block is automatically relayed to peers.
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index ef12ffe34b..46d36f83f8 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -884,12 +884,11 @@ class MinerImpl : public Mining
         return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root);
     }
 
-    std::unique_ptr<CBlockTemplate> createNewBlock(const CScript& script_pub_key, bool use_mempool) override
+    std::unique_ptr<CBlockTemplate> createNewBlock(const CScript& script_pub_key, const BlockCreateOptions& options) override
     {
-        BlockAssembler::Options options;
-        ApplyArgsManOptions(gArgs, options);
-
-        return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr, options}.CreateNewBlock(script_pub_key);
+        BlockAssembler::Options assemble_options{options};
+        ApplyArgsManOptions(*Assert(m_node.args), assemble_options);
+        return BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key);
     }
 
     NodeContext* context() override { return &m_node; }
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 291f1d5fc7..67465b1e70 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -66,7 +66,7 @@ static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
 
 BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options)
     : chainparams{chainstate.m_chainman.GetParams()},
-      m_mempool{mempool},
+      m_mempool{options.use_mempool ? mempool : nullptr},
       m_chainstate{chainstate},
       m_options{ClampOptions(options)}
 {
diff --git a/src/node/miner.h b/src/node/miner.h
index 622ca16c8f..efd773eb31 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -6,6 +6,7 @@
 #ifndef BITCOIN_NODE_MINER_H
 #define BITCOIN_NODE_MINER_H
 
+#include <node/types.h>
 #include <policy/policy.h>
 #include <primitives/block.h>
 #include <txmempool.h>
@@ -153,7 +154,7 @@ class BlockAssembler
     Chainstate& m_chainstate;
 
 public:
-    struct Options {
+    struct Options : BlockCreateOptions {
         // Configuration parameters for the block size
         size_t nBlockMaxWeight{DEFAULT_BLOCK_MAX_WEIGHT};
         CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE};
diff --git a/src/node/types.h b/src/node/types.h
index 0461e85f43..d4e7d93015 100644
--- a/src/node/types.h
+++ b/src/node/types.h
@@ -13,6 +13,8 @@
 #ifndef BITCOIN_NODE_TYPES_H
 #define BITCOIN_NODE_TYPES_H
 
+#include <cstddef>
+
 namespace node {
 enum class TransactionError {
     OK, //!< No error
@@ -24,6 +26,13 @@ enum class TransactionError {
     MAX_BURN_EXCEEDED,
     INVALID_PACKAGE,
 };
+
+struct BlockCreateOptions {
+    /**
+     * Set false to omit mempool transactions
+     */
+    bool use_mempool{true};
+};
 } // namespace node
 
 #endif // BITCOIN_NODE_TYPES_H
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index e16f75ab78..8482ce6eb2 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -371,7 +371,7 @@ static RPCHelpMan generateblock()
 
     ChainstateManager& chainman = EnsureChainman(node);
     {
-        std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)};
+        std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, {.use_mempool = false})};
         if (!blocktemplate) {
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
         }

From 5d25a82b9a5e54f74cc066599541bc1d3da70988 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Tue, 16 Jul 2024 22:23:53 +0100
Subject: [PATCH 45/55] univalue, refactor: Convert indentation tabs to spaces

---
 src/univalue/test/object.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp
index 1c724555f3..c5f15ff474 100644
--- a/src/univalue/test/object.cpp
+++ b/src/univalue/test/object.cpp
@@ -21,16 +21,16 @@
             (stmt); \
             assert(0 && "No exception caught"); \
         } catch (excMatch & e) { \
-	} catch (...) { \
-	    assert(0 && "Wrong exception caught"); \
-	} \
+        } catch (...) { \
+            assert(0 && "Wrong exception caught"); \
+        } \
     }
 #define BOOST_CHECK_NO_THROW(stmt) { \
         try { \
             (stmt); \
-	} catch (...) { \
-	    assert(0); \
-	} \
+        } catch (...) { \
+            assert(0); \
+        } \
     }
 
 void univalue_constructor()

From 44f08786f435ed4284d39dc604c2a5fcbde9e602 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon, 26 Feb 2024 20:33:43 +0000
Subject: [PATCH 46/55] test: Fix MSVC warning C4101 "unreferenced local
 variable"

---
 src/univalue/test/object.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp
index c5f15ff474..2577c682d7 100644
--- a/src/univalue/test/object.cpp
+++ b/src/univalue/test/object.cpp
@@ -20,7 +20,7 @@
         try { \
             (stmt); \
             assert(0 && "No exception caught"); \
-        } catch (excMatch & e) { \
+        } catch (excMatch&) { \
         } catch (...) { \
             assert(0 && "Wrong exception caught"); \
         } \

From fa6390df205513319f28e35e3e17c40ecaa7d731 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Tue, 16 Jul 2024 19:23:02 +0200
Subject: [PATCH 47/55] doc: getaddressinfo[isscript] is optional

---
 src/wallet/rpc/addresses.cpp                   | 4 ++--
 test/functional/rpc_invalid_address_message.py | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 0c2ad06eea..35c93337c1 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2011-2022 The Bitcoin Core developers
+// Copyright (c) 2011-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.
 
@@ -534,7 +534,7 @@ RPCHelpMan getaddressinfo()
                         {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."},
                         {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."},
                         {RPCResult::Type::STR, "parent_desc", /*optional=*/true, "The descriptor used to derive this address if this is a descriptor wallet"},
-                        {RPCResult::Type::BOOL, "isscript", "If the key is a script."},
+                        {RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script."},
                         {RPCResult::Type::BOOL, "ischange", "If the address was used for change output."},
                         {RPCResult::Type::BOOL, "iswitness", "If the address is a witness address."},
                         {RPCResult::Type::NUM, "witness_version", /*optional=*/true, "The version number of the witness program."},
diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py
index 6759b69dd1..33f12484ad 100755
--- a/test/functional/rpc_invalid_address_message.py
+++ b/test/functional/rpc_invalid_address_message.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# Copyright (c) 2020-2022 The Bitcoin Core developers
+# Copyright (c) 2020-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.
 """Test error messages for 'getaddressinfo' and 'validateaddress' RPC commands."""
@@ -12,6 +12,7 @@
 )
 
 BECH32_VALID = 'bcrt1qtmp74ayg7p24uslctssvjm06q5phz4yrxucgnv'
+BECH32_VALID_UNKNOWN_WITNESS = 'bcrt1p424qxxyd0r'
 BECH32_VALID_CAPITALS = 'BCRT1QPLMTZKC2XHARPPZDLNPAQL78RSHJ68U33RAH7R'
 BECH32_VALID_MULTISIG = 'bcrt1qdg3myrgvzw7ml9q0ejxhlkyxm7vl9r56yzkfgvzclrf4hkpx9yfqhpsuks'
 
@@ -80,6 +81,7 @@ def test_validateaddress(self):
 
         # Valid Bech32
         self.check_valid(BECH32_VALID)
+        self.check_valid(BECH32_VALID_UNKNOWN_WITNESS)
         self.check_valid(BECH32_VALID_CAPITALS)
         self.check_valid(BECH32_VALID_MULTISIG)
 
@@ -109,6 +111,7 @@ def test_getaddressinfo(self):
         assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, BECH32_INVALID_PREFIX)
         assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", node.getaddressinfo, BASE58_INVALID_PREFIX)
         assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, INVALID_ADDRESS)
+        assert "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS)
 
     def run_test(self):
         self.test_validateaddress()

From fa927055dd43dda945396574273a210186beec9f Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 17 Jul 2024 08:58:47 +0200
Subject: [PATCH 48/55] refactor: Make m_last_notified_header private

---
 src/validation.cpp | 24 ++++++++++++------------
 src/validation.h   |  8 +++++---
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 72ff24d584..c49ec404ca 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3405,24 +3405,24 @@ static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_i
     return SynchronizationState::INIT_DOWNLOAD;
 }
 
-static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main)
+bool ChainstateManager::NotifyHeaderTip()
 {
     bool fNotify = false;
     bool fInitialBlockDownload = false;
     CBlockIndex* pindexHeader = nullptr;
     {
-        LOCK(cs_main);
-        pindexHeader = chainman.m_best_header;
+        LOCK(GetMutex());
+        pindexHeader = m_best_header;
 
-        if (pindexHeader != chainman.m_last_notified_header) {
+        if (pindexHeader != m_last_notified_header) {
             fNotify = true;
-            fInitialBlockDownload = chainman.IsInitialBlockDownload();
-            chainman.m_last_notified_header = pindexHeader;
+            fInitialBlockDownload = IsInitialBlockDownload();
+            m_last_notified_header = pindexHeader;
         }
     }
-    // Send block tip changed notifications without cs_main
+    // Send block tip changed notifications without the lock held
     if (fNotify) {
-        chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false);
+        GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false);
     }
     return fNotify;
 }
@@ -4378,7 +4378,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>&
             }
         }
     }
-    if (NotifyHeaderTip(*this)) {
+    if (NotifyHeaderTip()) {
         if (IsInitialBlockDownload() && ppindex && *ppindex) {
             const CBlockIndex& last_accepted{**ppindex};
             int64_t blocks_left{(NodeClock::now() - last_accepted.Time()) / GetConsensus().PowTargetSpacing()};
@@ -4549,7 +4549,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock>& blo
         }
     }
 
-    NotifyHeaderTip(*this);
+    NotifyHeaderTip();
 
     BlockValidationState state; // Only used to report errors, not invalidity - ignore it
     if (!ActiveChainstate().ActivateBestChain(state, block)) {
@@ -5126,7 +5126,7 @@ void ChainstateManager::LoadExternalBlockFile(
                     }
                 }
 
-                NotifyHeaderTip(*this);
+                NotifyHeaderTip();
 
                 if (!blocks_with_unknown_parent) continue;
 
@@ -5152,7 +5152,7 @@ void ChainstateManager::LoadExternalBlockFile(
                         }
                         range.first++;
                         blocks_with_unknown_parent->erase(it);
-                        NotifyHeaderTip(*this);
+                        NotifyHeaderTip();
                     }
                 }
             } catch (const std::exception& e) {
diff --git a/src/validation.h b/src/validation.h
index ea4ec84242..08e672c620 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -906,6 +906,11 @@ class ChainstateManager
 
     CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr};
 
+    /** The last header for which a headerTip notification was issued. */
+    CBlockIndex* m_last_notified_header GUARDED_BY(GetMutex()){nullptr};
+
+    bool NotifyHeaderTip() LOCKS_EXCLUDED(GetMutex());
+
     //! Internal helper for ActivateSnapshot().
     //!
     //! De-serialization of a snapshot that is created with
@@ -1063,9 +1068,6 @@ class ChainstateManager
     /** Best header we've seen so far (used for getheaders queries' starting points). */
     CBlockIndex* m_best_header GUARDED_BY(::cs_main){nullptr};
 
-    /** The last header for which a headerTip notification was issued. */
-    CBlockIndex* m_last_notified_header GUARDED_BY(::cs_main){nullptr};
-
     //! The total number of bytes available for us to use across all in-memory
     //! coins caches. This will be split somehow across chainstates.
     int64_t m_total_coinstip_cache{0};

From faed5d3870fb32fb5278961ee23e38fd9ea6ca15 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Tue, 16 Jul 2024 13:40:43 +0200
Subject: [PATCH 49/55] test: Non-Shy version sender

---
 test/functional/p2p_add_connections.py | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py
index bd766a279e..0775fd873d 100755
--- a/test/functional/p2p_add_connections.py
+++ b/test/functional/p2p_add_connections.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# Copyright (c) 2020-2021 The Bitcoin Core developers
+# Copyright (c) 2020-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.
 """Test add_outbound_p2p_connection test framework functionality"""
@@ -11,6 +11,14 @@
     check_node_connections,
 )
 
+
+class VersionSender(P2PInterface):
+    def on_open(self):
+        assert self.on_connection_send_msg is not None
+        self.send_version()
+        assert self.on_connection_send_msg is None
+
+
 class P2PFeelerReceiver(P2PInterface):
     def on_version(self, message):
         # The bitcoind node closes feeler connections as soon as a version
@@ -106,5 +114,19 @@ def run_test(self):
         # Feeler connections do not request tx relay
         assert_equal(feeler_conn.last_message["version"].relay, 0)
 
+        self.log.info("Send version message early to node")
+        # Normally the test framework would be shy and send the version message
+        # only after it received one. See the on_version method. Check that
+        # bitcoind behaves properly when a version is sent unexpectedly (but
+        # tolerably) early.
+        #
+        # This checks that bitcoind sends its own version prior to processing
+        # the remote version (and replying with a verack). Otherwise it would
+        # be violating its own rules, such as "non-version message before
+        # version handshake".
+        ver_conn = self.nodes[0].add_outbound_p2p_connection(VersionSender(), p2p_idx=6, connection_type="outbound-full-relay", supports_v2_p2p=False, advertise_v2_p2p=False)
+        ver_conn.sync_with_ping()
+
+
 if __name__ == '__main__':
     P2PAddConnections().main()

From fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 17 Jul 2024 12:17:43 +0200
Subject: [PATCH 50/55] refactor: Use util::Split to avoid a harmless
 unsigned-integer-overflow

The previous commit added a test which would fail the
unsigned-integer-overflow sanitizer. The warning is harmless and can be
triggered on any commit, since the code was introduced.

For reference, the warning would happen when the separator `-` was not
present.

For example:

  GET /rest/getutxos/6a297bfa5cb8dd976ab0207a767d6cbfaa5e876f30081127ec8674c8c52b16c0_+1.json

would result in:

rest.cpp:792:77: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'size_type' (aka 'unsigned long')
    #0 0x55ad42c16931 in rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) src/rest.cpp:792:77
    #1 0x55ad4319e3c0 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    #2 0x55ad4319e3c0 in HTTPWorkItem::operator()() src/httpserver.cpp:59:9
    #3 0x55ad431a3eea in WorkQueue<HTTPClosure>::Run() src/httpserver.cpp:114:13
    #4 0x55ad4318f961 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) src/httpserver.cpp:403:12
    #5 0x7f078ebcbbb3  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xeabb3) (BuildId: 40b9b0d17fdeebfb57331304da2b7f85e1396ef2)
    #6 0x55ad4277e01c in asan_thread_start(void*) asan_interceptors.cpp.o
    #7 0x7f078e840a93  (/lib/x86_64-linux-gnu/libc.so.6+0x9ca93) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #8 0x7f078e8cdc3b  (/lib/x86_64-linux-gnu/libc.so.6+0x129c3b) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)

SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow rest.cpp:792:77
---
 src/rest.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/rest.cpp b/src/rest.cpp
index 185508d2c9..3cf6ad343c 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -788,15 +788,17 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
 
         for (size_t i = (fCheckMemPool) ? 1 : 0; i < uriParts.size(); i++)
         {
-            std::string strTxid = uriParts[i].substr(0, uriParts[i].find('-'));
-            std::string strOutput = uriParts[i].substr(uriParts[i].find('-')+1);
-            auto output{ToIntegral<uint32_t>(strOutput)};
+            const auto txid_out{util::Split<std::string_view>(uriParts[i], '-')};
+            if (txid_out.size() != 2) {
+                return RESTERR(req, HTTP_BAD_REQUEST, "Parse error");
+            }
+            auto output{ToIntegral<uint32_t>(txid_out.at(1))};
 
-            if (!output || !IsHex(strTxid)) {
+            if (!output || !IsHex(txid_out.at(0))) {
                 return RESTERR(req, HTTP_BAD_REQUEST, "Parse error");
             }
 
-            vOutPoints.emplace_back(TxidFromString(strTxid), *output);
+            vOutPoints.emplace_back(TxidFromString(txid_out.at(0)), *output);
         }
 
         if (vOutPoints.size() > 0)

From c322bddd08ed1f1f7f0c39768b659dd62d5e2dd5 Mon Sep 17 00:00:00 2001
From: stratospher <44024636+stratospher@users.noreply.github.com>
Date: Wed, 17 Jul 2024 15:55:14 +0530
Subject: [PATCH 51/55] test: bump mocktime after node has received and sent
 bytes

a different error message "socket no message in first %i seconds"
will be displayed if m_last_send=0 or if m_last_recv is 0. make
the test robust by ensuring that they will not be 0 before
bumping mocktime.
---
 test/functional/p2p_v2_misbehaving.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/functional/p2p_v2_misbehaving.py b/test/functional/p2p_v2_misbehaving.py
index 9017ee9455..0789425bcb 100755
--- a/test/functional/p2p_v2_misbehaving.py
+++ b/test/functional/p2p_v2_misbehaving.py
@@ -147,11 +147,11 @@ def test_earlykeyresponse(self):
         peer1.v2_state.can_data_be_received = True
         self.wait_until(lambda: peer1.v2_state.ellswift_ours)
         peer1.send_raw_message(peer1.v2_state.ellswift_ours[4:] + peer1.v2_state.sent_garbage)
-        node0.bumpmocktime(3)
         # Ensure that the bytes sent after 4 bytes network magic are actually received.
         self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4)
+        self.wait_until(lambda: node0.getpeerinfo()[-1]["bytessent"] > 0)
         with node0.assert_debug_log(['V2 handshake timeout peer=0']):
-            node0.bumpmocktime(1)  # `InactivityCheck()` triggers now
+            node0.bumpmocktime(4)  # `InactivityCheck()` triggers now
             peer1.wait_for_disconnect(timeout=1)
         self.log.info('successful disconnection since modified ellswift was sent as response')
 

From c504b6997b1acc9771ad1f52efaa4be2b4966c6c Mon Sep 17 00:00:00 2001
From: Sjors Provoost <sjors@sprovoost.nl>
Date: Wed, 17 Jul 2024 18:33:15 +0200
Subject: [PATCH 52/55] refactor: add coinbase constraints to
 BlockCreateOptions

When generating a block template through e.g. getblocktemplate RPC,
we reserve 4000 weight units and 400 sigops. Pools use this space
for their coinbase outputs.

At least one pool patched their Bitcoin Core node to adjust
these hardcoded values. They eventually produced an invalid
block which exceeded the sigops limit.
https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426

The existince of such patches suggests it may be useful to
make this value configurable. This commit would make such a
change easier.

The main motivation however is that the Stratum v2 spec
requires the pool to communicate the maximum bytes they intend
to add to the coinbase outputs. A proposed change to the spec
would also require them to communicate the maximum number of sigops.

This commit also documents what happens when
-blockmaxweight is lower than the coinbase
reserved value.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/node/miner.cpp | 13 ++++++++-----
 src/node/types.h   | 11 +++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 67465b1e70..fa2d979b86 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -59,8 +59,11 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
 
 static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
 {
-    // Limit weight to between 4K and DEFAULT_BLOCK_MAX_WEIGHT for sanity:
-    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, 4000, DEFAULT_BLOCK_MAX_WEIGHT);
+    Assert(options.coinbase_max_additional_weight <= DEFAULT_BLOCK_MAX_WEIGHT);
+    Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST);
+    // Limit weight to between coinbase_max_additional_weight and DEFAULT_BLOCK_MAX_WEIGHT for sanity:
+    // Coinbase (reserved) outputs can safely exceed -blockmaxweight, but the rest of the block template will be empty.
+    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.coinbase_max_additional_weight, DEFAULT_BLOCK_MAX_WEIGHT);
     return options;
 }
 
@@ -87,8 +90,8 @@ void BlockAssembler::resetBlock()
     inBlock.clear();
 
     // Reserve space for coinbase tx
-    nBlockWeight = 4000;
-    nBlockSigOpsCost = 400;
+    nBlockWeight = m_options.coinbase_max_additional_weight;
+    nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops;
 
     // These counters do not include coinbase tx
     nBlockTx = 0;
@@ -379,7 +382,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
             ++nConsecutiveFailed;
 
             if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight >
-                    m_options.nBlockMaxWeight - 4000) {
+                    m_options.nBlockMaxWeight - m_options.coinbase_max_additional_weight) {
                 // Give up if we're close to full and haven't succeeded in a while
                 break;
             }
diff --git a/src/node/types.h b/src/node/types.h
index d4e7d93015..bb8d17ef43 100644
--- a/src/node/types.h
+++ b/src/node/types.h
@@ -32,6 +32,17 @@ struct BlockCreateOptions {
      * Set false to omit mempool transactions
      */
     bool use_mempool{true};
+    /**
+     * The maximum additional weight which the pool will add to the coinbase
+     * scriptSig, witness and outputs. This must include any additional
+     * weight needed for larger CompactSize encoded lengths.
+     */
+    size_t coinbase_max_additional_weight{4000};
+    /**
+     * The maximum additional sigops which the pool will add in coinbase
+     * transaction outputs.
+     */
+    size_t coinbase_output_max_additional_sigops{400};
 };
 } // namespace node
 

From ff4f3deb7b8adfcc90fb745440ce4be1176552ca Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Mon, 15 Apr 2024 17:15:07 +0100
Subject: [PATCH 53/55] depends: use CMake to build FreeType

---
 depends/packages/freetype.mk | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/depends/packages/freetype.mk b/depends/packages/freetype.mk
index c28259ed67..c942cbb936 100644
--- a/depends/packages/freetype.mk
+++ b/depends/packages/freetype.mk
@@ -3,14 +3,17 @@ $(package)_version=2.11.0
 $(package)_download_path=https://download.savannah.gnu.org/releases/$(package)
 $(package)_file_name=$(package)-$($(package)_version).tar.xz
 $(package)_sha256_hash=8bee39bd3968c4804b70614a0a3ad597299ad0e824bc8aad5ce8aaf48067bde7
+$(package)_build_subdir=build
 
 define $(package)_set_vars
-  $(package)_config_opts=--without-zlib --without-png --without-harfbuzz --without-bzip2 --disable-static
-  $(package)_config_opts += --enable-option-checking --without-brotli
+  $(package)_config_opts := -DCMAKE_BUILD_TYPE=None -DBUILD_SHARED_LIBS=TRUE
+  $(package)_config_opts += -DCMAKE_DISABLE_FIND_PACKAGE_ZLIB=TRUE -DCMAKE_DISABLE_FIND_PACKAGE_PNG=TRUE
+  $(package)_config_opts += -DCMAKE_DISABLE_FIND_PACKAGE_HarfBuzz=TRUE -DCMAKE_DISABLE_FIND_PACKAGE_BZip2=TRUE
+  $(package)_config_opts += -DCMAKE_DISABLE_FIND_PACKAGE_BrotliDec=TRUE
 endef
 
 define $(package)_config_cmds
-  $($(package)_autoconf)
+  $($(package)_cmake) -S .. -B .
 endef
 
 define $(package)_build_cmds
@@ -22,5 +25,5 @@ define $(package)_stage_cmds
 endef
 
 define $(package)_postprocess_cmds
-  rm -rf share/man lib/*.la
+  rm -rf share/man
 endef

From d318c4ef56465ccad1a1d4d27c52216e0b69ad4e Mon Sep 17 00:00:00 2001
From: Cory Fields <cory-nospam-@coryfields.com>
Date: Fri, 19 Jul 2024 18:32:56 +0000
Subject: [PATCH 54/55] depends: bump libmultiprocess for CMake fixes

---
 depends/packages/native_libmultiprocess.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/depends/packages/native_libmultiprocess.mk b/depends/packages/native_libmultiprocess.mk
index bcdb1f9e7c..ea08eefea6 100644
--- a/depends/packages/native_libmultiprocess.mk
+++ b/depends/packages/native_libmultiprocess.mk
@@ -1,8 +1,8 @@
 package=native_libmultiprocess
-$(package)_version=8da797c5f1644df1bffd84d10c1ae9836dc70d60
+$(package)_version=8b8a4766ce0a1892b9e8a5eb73dc39821005e520
 $(package)_download_path=https://github.com/chaincodelabs/libmultiprocess/archive
 $(package)_file_name=$($(package)_version).tar.gz
-$(package)_sha256_hash=030f4d393d2ac9deba98d2e1973e22fc439ffc009d5f8ae3225c90639f86beb0
+$(package)_sha256_hash=475c0dc2357a2ff30e9a164e4c16dc8a6597a57c9193d646fa9cbf0a55c45d78
 $(package)_dependencies=native_capnp
 
 define $(package)_config_cmds

From 7703884ab19cd7ffddc5c52ba57dec82fbc8dc2b Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon, 26 Feb 2024 18:06:32 +0000
Subject: [PATCH 55/55] Fix MSVC warning C4273 "inconsistent dll linkage"

When using CMake, the user can select the MSVC runtime library to be:
1) Statically-linked (with the corresponding `x64-windows-static` vcpkg
triplet) or
2) Dynamically-linked (with the corresponding `x64-windows` vcpkg
triplet)

In the latter case, the compiler emits the C4273 warning.

As the "Necessary on some platforms" comment does not apply to MSVC,
skip the declaration for MSVC.
---
 src/randomenv.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/randomenv.cpp b/src/randomenv.cpp
index 93d30a27fd..4754b597c5 100644
--- a/src/randomenv.cpp
+++ b/src/randomenv.cpp
@@ -58,7 +58,9 @@
 #include <sys/auxv.h>
 #endif
 
+#ifndef _MSC_VER
 extern char** environ; // NOLINT(readability-redundant-declaration): Necessary on some platforms
+#endif
 
 namespace {