From 493656763f73e5ef1cfb979a513c12983dca99dd Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Mon, 18 Mar 2024 16:16:48 -0400
Subject: [PATCH 01/42] desc spkm: Return SigningProvider only if we have the
 privkey

If we know about a pubkey that's in our descriptor, but we don't have
the private key, don't return a SigningProvider for that pubkey.

This is specifically an issue for Taproot outputs that use the H point
as the resulting PSBTs may end up containing irrelevant information
because the H point was detected as a pubkey each unrelated descriptor
knew about.
---
 src/script/signingprovider.cpp | 5 +++++
 src/script/signingprovider.h   | 1 +
 src/wallet/scriptpubkeyman.cpp | 6 +++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp
index baabd4d5b5..597b1a1544 100644
--- a/src/script/signingprovider.cpp
+++ b/src/script/signingprovider.cpp
@@ -62,6 +62,11 @@ bool FlatSigningProvider::GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info)
     if (ret) info = std::move(out.second);
     return ret;
 }
+bool FlatSigningProvider::HaveKey(const CKeyID &keyid) const
+{
+    CKey key;
+    return LookupHelper(keys, keyid, key);
+}
 bool FlatSigningProvider::GetKey(const CKeyID& keyid, CKey& key) const { return LookupHelper(keys, keyid, key); }
 bool FlatSigningProvider::GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const
 {
diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h
index efdfd9ee56..f0fb21c8b1 100644
--- a/src/script/signingprovider.h
+++ b/src/script/signingprovider.h
@@ -215,6 +215,7 @@ struct FlatSigningProvider final : public SigningProvider
     bool GetCScript(const CScriptID& scriptid, CScript& script) const override;
     bool GetPubKey(const CKeyID& keyid, CPubKey& pubkey) const override;
     bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override;
+    bool HaveKey(const CKeyID &keyid) const override;
     bool GetKey(const CKeyID& keyid, CKey& key) const override;
     bool GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const override;
     bool GetTaprootBuilder(const XOnlyPubKey& output_key, TaprootBuilder& builder) const override;
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 62384056dc..7787e2ff6b 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2456,7 +2456,11 @@ std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvid
     int32_t index = it->second;
 
     // Always try to get the signing provider with private keys. This function should only be called during signing anyways
-    return GetSigningProvider(index, true);
+    std::unique_ptr<FlatSigningProvider> out = GetSigningProvider(index, true);
+    if (!out->HaveKey(pubkey.GetID())) {
+        return nullptr;
+    }
+    return out;
 }
 
 std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvider(int32_t index, bool include_private) const

From 11115e9aa845d675a88e18e729913f0aaa11e322 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Thu, 19 Dec 2024 18:16:09 +0000
Subject: [PATCH 02/42] cmake: Always provide `RPATH` on NetBSD

---
 CMakeLists.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2dba6f255d..474eb3504c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -580,8 +580,13 @@ endif()
 # Relevant discussions:
 # - https://github.com/hebasto/bitcoin/pull/236#issuecomment-2183120953
 # - https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833
-set(CMAKE_SKIP_BUILD_RPATH TRUE)
-set(CMAKE_SKIP_INSTALL_RPATH TRUE)
+# NetBSD always requires runtime paths to be set for executables.
+if(CMAKE_SYSTEM_NAME STREQUAL "NetBSD")
+  set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
+else()
+  set(CMAKE_SKIP_BUILD_RPATH TRUE)
+  set(CMAKE_SKIP_INSTALL_RPATH TRUE)
+endif()
 add_subdirectory(test)
 add_subdirectory(doc)
 

From 5db7d4d3d28bd1269a09955b4695135c86c4827d Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Wed, 11 Dec 2024 22:15:22 +0100
Subject: [PATCH 03/42] doc: Correct docstring describing max block tree db
 cache

It is always applied in the same way, no matter how the txindex is
setup. This was no longer accurate after 8181db8, where their
initialization was made independent.
---
 src/txdb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/txdb.h b/src/txdb.h
index 565b060dbe..671cbd1286 100644
--- a/src/txdb.h
+++ b/src/txdb.h
@@ -27,11 +27,11 @@ static const int64_t nDefaultDbCache = 450;
 static const int64_t nDefaultDbBatchSize = 16 << 20;
 //! min. -dbcache (MiB)
 static const int64_t nMinDbCache = 4;
-//! Max memory allocated to block tree DB specific cache, if no -txindex (MiB)
+//! Max memory allocated to block tree DB specific cache (MiB)
 static const int64_t nMaxBlockDBCache = 2;
-//! Max memory allocated to block tree DB specific cache, if -txindex (MiB)
 // Unlike for the UTXO database, for the txindex scenario the leveldb cache make
 // a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
+//! Max memory allocated to tx index DB specific cache in MiB.
 static const int64_t nMaxTxIndexCache = 1024;
 //! Max memory allocated to all block filter index caches combined in MiB.
 static const int64_t max_filter_index_cache = 1024;

From 62a95f5af9b998e241eb72c16ba581e77c480126 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Thu, 14 Nov 2024 13:15:16 +0100
Subject: [PATCH 04/42] test: refactor: move `CreateDescriptor` helper to
 wallet test util module

Can be reviewed via `--color-moved=dimmed-zebra`.
---
 src/wallet/test/ismine_tests.cpp | 20 --------------------
 src/wallet/test/util.cpp         | 20 ++++++++++++++++++++
 src/wallet/test/util.h           |  4 +++-
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp
index f6688ed30a..3b27c7db9a 100644
--- a/src/wallet/test/ismine_tests.cpp
+++ b/src/wallet/test/ismine_tests.cpp
@@ -20,26 +20,6 @@ using namespace util::hex_literals;
 namespace wallet {
 BOOST_FIXTURE_TEST_SUITE(ismine_tests, BasicTestingSetup)
 
-wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
-{
-    keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-
-    FlatSigningProvider keys;
-    std::string error;
-    auto parsed_descs = Parse(desc_str, keys, error, false);
-    BOOST_CHECK(success == (!parsed_descs.empty()));
-    if (!success) return nullptr;
-    auto& desc = parsed_descs.at(0);
-
-    const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1;
-
-    WalletDescriptor w_desc(std::move(desc), timestamp, range_start, range_end, next_index);
-
-    LOCK(keystore.cs_wallet);
-
-    return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false));
-};
-
 BOOST_AUTO_TEST_CASE(ismine_standard)
 {
     CKey keys[2];
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 43fd91fe60..bc53510fe4 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -192,4 +192,24 @@ MockableDatabase& GetMockableDatabase(CWallet& wallet)
 {
     return dynamic_cast<MockableDatabase&>(wallet.GetDatabase());
 }
+
+wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
+{
+    keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+
+    FlatSigningProvider keys;
+    std::string error;
+    auto parsed_descs = Parse(desc_str, keys, error, false);
+    Assert(success == (!parsed_descs.empty()));
+    if (!success) return nullptr;
+    auto& desc = parsed_descs.at(0);
+
+    const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1;
+
+    WalletDescriptor w_desc(std::move(desc), timestamp, range_start, range_end, next_index);
+
+    LOCK(keystore.cs_wallet);
+
+    return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false));
+};
 } // namespace wallet
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index c8a89c0e64..b055c6c693 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -9,6 +9,7 @@
 
 #include <addresstype.h>
 #include <wallet/db.h>
+#include <wallet/scriptpubkeyman.h>
 
 #include <memory>
 
@@ -127,8 +128,9 @@ class MockableDatabase : public WalletDatabase
 };
 
 std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records = {});
-
 MockableDatabase& GetMockableDatabase(CWallet& wallet);
+
+ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_TEST_UTIL_H

From f6a6d912059c66792f48689632d2a7f14f8bdad9 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Thu, 14 Nov 2024 14:45:02 +0100
Subject: [PATCH 05/42] test: add check for getting SigningProvider for a
 CPubKey

Verify that the DescriptorSPKM method `GetSigningProvider` should
only return a signing provider for the passed public key if its
corresponding private key of the passed public key is available.
---
 src/wallet/scriptpubkeyman.h              |  5 +++--
 src/wallet/test/scriptpubkeyman_tests.cpp | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index d8b6c90178..4ff35a478c 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -611,8 +611,6 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     mutable std::map<int32_t, FlatSigningProvider> m_map_signing_providers;
     // Fetch the SigningProvider for the given script and optionally include private keys
     std::unique_ptr<FlatSigningProvider> GetSigningProvider(const CScript& script, bool include_private = false) const;
-    // Fetch the SigningProvider for the given pubkey and always include private keys. This should only be called by signing code.
-    std::unique_ptr<FlatSigningProvider> GetSigningProvider(const CPubKey& pubkey) const;
     // Fetch the SigningProvider for a given index and optionally include private keys. Called by the above functions.
     std::unique_ptr<FlatSigningProvider> GetSigningProvider(int32_t index, bool include_private = false) const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
 
@@ -675,6 +673,9 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
 
     bool CanProvide(const CScript& script, SignatureData& sigdata) override;
 
+    // Fetch the SigningProvider for the given pubkey and always include private keys. This should only be called by signing code.
+    std::unique_ptr<FlatSigningProvider> GetSigningProvider(const CPubKey& pubkey) const;
+
     bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
     SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
     std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp
index 15bff04221..f27865865d 100644
--- a/src/wallet/test/scriptpubkeyman_tests.cpp
+++ b/src/wallet/test/scriptpubkeyman_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <key.h>
+#include <key_io.h>
 #include <test/util/setup_common.h>
 #include <script/solver.h>
 #include <wallet/scriptpubkeyman.h>
@@ -40,5 +41,27 @@ BOOST_AUTO_TEST_CASE(CanProvide)
     BOOST_CHECK(keyman.CanProvide(p2sh_script, data));
 }
 
+BOOST_AUTO_TEST_CASE(DescriptorScriptPubKeyManTests)
+{
+    std::unique_ptr<interfaces::Chain>& chain = m_node.chain;
+
+    CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
+    auto key_scriptpath = GenerateRandomKey();
+
+    // Verify that a SigningProvider for a pubkey is only returned if its corresponding private key is available
+    auto key_internal = GenerateRandomKey();
+    std::string desc_str = "tr(" + EncodeSecret(key_internal) + ",pk(" + HexStr(key_scriptpath.GetPubKey()) + "))";
+    auto spk_man1 = dynamic_cast<DescriptorScriptPubKeyMan*>(CreateDescriptor(keystore, desc_str, true));
+    BOOST_CHECK(spk_man1 != nullptr);
+    auto signprov_keypath_spendable = spk_man1->GetSigningProvider(key_internal.GetPubKey());
+    BOOST_CHECK(signprov_keypath_spendable != nullptr);
+
+    desc_str = "tr(" + HexStr(XOnlyPubKey::NUMS_H) + ",pk(" + HexStr(key_scriptpath.GetPubKey()) + "))";
+    auto spk_man2 = dynamic_cast<DescriptorScriptPubKeyMan*>(CreateDescriptor(keystore, desc_str, true));
+    BOOST_CHECK(spk_man2 != nullptr);
+    auto signprov_keypath_nums_h = spk_man2->GetSigningProvider(XOnlyPubKey::NUMS_H.GetEvenCorrespondingCPubKey());
+    BOOST_CHECK(signprov_keypath_nums_h == nullptr);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

From 6951ddcefd9e05f31ee7634bbfbf1d19e04ec00e Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Mon, 20 Mar 2023 10:37:23 +0000
Subject: [PATCH 06/42] [txrequest] GetCandidatePeers

Needed for a later commit adding logic to ask the TxRequestTracker for a
list of announcers.  These announcers should know the parents of the
transaction they announced.
---
 src/txrequest.cpp | 18 ++++++++++++++++++
 src/txrequest.h   |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/src/txrequest.cpp b/src/txrequest.cpp
index 96ea716481..ca68a99868 100644
--- a/src/txrequest.cpp
+++ b/src/txrequest.cpp
@@ -574,6 +574,23 @@ class TxRequestTracker::Impl {
         }
     }
 
+    std::vector<NodeId> GetCandidatePeers(const CTransactionRef& tx) const
+    {
+        // Search by txid and, if the tx has a witness, wtxid
+        std::vector<uint256> hashes{tx->GetHash().ToUint256()};
+        if (tx->HasWitness()) hashes.emplace_back(tx->GetWitnessHash().ToUint256());
+
+        std::vector<NodeId> result_peers;
+        for (const uint256& txhash : hashes) {
+            auto it = m_index.get<ByTxHash>().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0});
+            while (it != m_index.get<ByTxHash>().end() && it->m_txhash == txhash && it->GetState() != State::COMPLETED) {
+                result_peers.push_back(it->m_peer);
+                ++it;
+            }
+        }
+        return result_peers;
+    }
+
     void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred,
         std::chrono::microseconds reqtime)
     {
@@ -721,6 +738,7 @@ size_t TxRequestTracker::CountInFlight(NodeId peer) const { return m_impl->Count
 size_t TxRequestTracker::CountCandidates(NodeId peer) const { return m_impl->CountCandidates(peer); }
 size_t TxRequestTracker::Count(NodeId peer) const { return m_impl->Count(peer); }
 size_t TxRequestTracker::Size() const { return m_impl->Size(); }
+std::vector<NodeId> TxRequestTracker::GetCandidatePeers(const CTransactionRef& tx) const { return m_impl->GetCandidatePeers(tx); }
 void TxRequestTracker::SanityCheck() const { m_impl->SanityCheck(); }
 
 void TxRequestTracker::PostGetRequestableSanityCheck(std::chrono::microseconds now) const
diff --git a/src/txrequest.h b/src/txrequest.h
index cd3042c87e..95a1e9e7f6 100644
--- a/src/txrequest.h
+++ b/src/txrequest.h
@@ -195,6 +195,9 @@ class TxRequestTracker {
     /** Count how many announcements are being tracked in total across all peers and transaction hashes. */
     size_t Size() const;
 
+    /** For some tx return all peers with non-COMPLETED announcements for its txid or wtxid. The resulting vector may contain duplicate NodeIds. */
+    std::vector<NodeId> GetCandidatePeers(const CTransactionRef& tx) const;
+
     /** Access to the internal priority computation (testing only) */
     uint64_t ComputePriority(const uint256& txhash, NodeId peer, bool preferred) const;
 

From 62a9ff187076686b39dca64ad4f2f439da0875d1 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 26 Jul 2024 14:10:21 +0100
Subject: [PATCH 07/42] [refactor] change type of unique_parents to Txid

---
 src/net_processing.cpp          | 2 +-
 src/node/txdownloadman.h        | 2 +-
 src/node/txdownloadman_impl.cpp | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index e503a68382..663dbb7634 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2978,7 +2978,7 @@ std::optional<node::PackageToValidate> PeerManagerImpl::ProcessInvalidTx(NodeId
     if (add_extra_compact_tx && RecursiveDynamicUsage(*ptx) < 100000) {
         AddToCompactExtraTransactions(ptx);
     }
-    for (const uint256& parent_txid : unique_parents) {
+    for (const Txid& parent_txid : unique_parents) {
         if (peer) AddKnownTx(*peer, parent_txid);
     }
 
diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h
index 81b0c76e0a..2d066cab53 100644
--- a/src/node/txdownloadman.h
+++ b/src/node/txdownloadman.h
@@ -92,7 +92,7 @@ struct PackageToValidate {
 struct RejectedTxTodo
 {
     bool m_should_add_extra_compact_tx;
-    std::vector<uint256> m_unique_parents;
+    std::vector<Txid> m_unique_parents;
     std::optional<PackageToValidate> m_package_to_validate;
 };
 
diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index f9635d049a..e9fe958f5d 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -308,7 +308,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
     // Whether we should call AddToCompactExtraTransactions at the end
     bool add_extra_compact_tx{first_time_failure};
     // Hashes to pass to AddKnownTx later
-    std::vector<uint256> unique_parents;
+    std::vector<Txid> unique_parents;
     // Populated if failure is reconsiderable and eligible package is found.
     std::optional<node::PackageToValidate> package_to_validate;
 

From e810842acda6fe56e0536ebecfbb9d17d26e1513 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 18 Apr 2023 16:02:58 +0100
Subject: [PATCH 08/42] [txorphanage] support multiple announcers

Add ability to add and track multiple announcers per orphan transaction,
erasing announcers but not the entire orphan.

The tx creation code in orphanage_tests needs to be updated so that each
tx is unique, because the CountOrphans() check assumes that calling
EraseForPeer necessarily means its orphans are deleted.

Unused for now.
---
 src/rpc/mempool.cpp          |  4 +-
 src/test/orphanage_tests.cpp |  4 +-
 src/txorphanage.cpp          | 71 +++++++++++++++++++++++++++---------
 src/txorphanage.h            | 12 +++++-
 4 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 6d462a7e85..5239886faf 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -843,7 +843,9 @@ static UniValue OrphanToJSON(const TxOrphanage::OrphanTxBase& orphan)
     o.pushKV("entry", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire - ORPHAN_TX_EXPIRE_TIME)});
     o.pushKV("expiration", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire)});
     UniValue from(UniValue::VARR);
-    from.push_back(orphan.fromPeer); // only one fromPeer for now
+    for (const auto fromPeer: orphan.announcers) {
+        from.push_back(fromPeer);
+    }
     o.pushKV("from", from);
     return o;
 }
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 799f2c0fec..07e7002cad 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.hash = Txid::FromUint256(m_rng.rand256());
         tx.vin[0].scriptSig << OP_1;
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
         orphanage.AddTx(MakeTransactionRef(tx), i);
@@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.n = 0;
         tx.vin[0].prevout.hash = txPrev->GetHash();
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         SignatureData empty;
         BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL, empty));
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index ba4ba6c3b6..a108ce9437 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -16,8 +16,11 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     const Txid& hash = tx->GetHash();
     const Wtxid& wtxid = tx->GetWitnessHash();
-    if (m_orphans.count(wtxid))
+    if (auto it{m_orphans.find(wtxid)}; it != m_orphans.end()) {
+        AddAnnouncer(wtxid, peer);
+        // No new orphan entry was created. An announcer may have been added.
         return false;
+    }
 
     // Ignore big transactions, to avoid a
     // send-big-orphans memory exhaustion attack. If a peer has a legitimate
@@ -33,7 +36,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
         return false;
     }
 
-    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, peer, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
+    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, {peer}, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
     assert(ret.second);
     m_orphan_list.push_back(ret.first);
     for (const CTxIn& txin : tx->vin) {
@@ -45,6 +48,20 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
     return true;
 }
 
+bool TxOrphanage::AddAnnouncer(const Wtxid& wtxid, NodeId peer)
+{
+    const auto it = m_orphans.find(wtxid);
+    if (it != m_orphans.end()) {
+        Assume(!it->second.announcers.empty());
+        const auto ret = it->second.announcers.insert(peer);
+        if (ret.second) {
+            LogDebug(BCLog::TXPACKAGES, "added peer=%d as announcer of orphan tx %s\n", peer, wtxid.ToString());
+            return true;
+        }
+    }
+    return false;
+}
+
 int TxOrphanage::EraseTx(const Wtxid& wtxid)
 {
     std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid);
@@ -89,9 +106,15 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     while (iter != m_orphans.end())
     {
         // increment to avoid iterator becoming invalid after erasure
-        const auto& [wtxid, orphan] = *iter++;
-        if (orphan.fromPeer == peer) {
-            nErased += EraseTx(wtxid);
+        auto& [wtxid, orphan] = *iter++;
+        auto orphan_it = orphan.announcers.find(peer);
+        if (orphan_it != orphan.announcers.end()) {
+            orphan.announcers.erase(peer);
+
+            // No remaining annnouncers: clean up entry
+            if (orphan.announcers.empty()) {
+                nErased += EraseTx(orphan.tx->GetWitnessHash());
+            }
         }
     }
     if (nErased > 0) LogDebug(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) from peer=%d\n", nErased, peer);
@@ -110,7 +133,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
         {
             std::map<Wtxid, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseTx(maybeErase->second.tx->GetWitnessHash());
+                nErased += EraseTx(maybeErase->first);
             } else {
                 nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
             }
@@ -123,7 +146,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(m_orphan_list.size());
-        EraseTx(m_orphan_list[randompos]->second.tx->GetWitnessHash());
+        EraseTx(m_orphan_list[randompos]->first);
         ++nEvicted;
     }
     if (nEvicted > 0) LogDebug(BCLog::TXPACKAGES, "orphanage overflow, removed %u tx\n", nEvicted);
@@ -135,13 +158,17 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                // Get this source peer's work set, emplacing an empty set if it didn't exist
-                // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
-                std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
-                // Add this tx to the work set
-                orphan_work_set.insert(elem->first);
-                LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
-                         tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), elem->second.fromPeer);
+                // Belt and suspenders, each orphan should always have at least 1 announcer.
+                if (!Assume(!elem->second.announcers.empty())) continue;
+                for (const auto announcer: elem->second.announcers) {
+                    // Get this source peer's work set, emplacing an empty set if it didn't exist
+                    // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
+                    std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(announcer).first->second;
+                    // Add this tx to the work set
+                    orphan_work_set.insert(elem->first);
+                    LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
+                             tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), announcer);
+                }
             }
         }
     }
@@ -152,6 +179,12 @@ bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
     return m_orphans.count(wtxid);
 }
 
+bool TxOrphanage::HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const
+{
+    auto it = m_orphans.find(wtxid);
+    return (it != m_orphans.end() && it->second.announcers.contains(peer));
+}
+
 CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
 {
     auto work_set_it = m_peer_work_set.find(peer);
@@ -219,7 +252,7 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                if (elem->second.fromPeer == nodeid) {
+                if (elem->second.announcers.contains(nodeid)) {
                     iters.emplace_back(elem);
                 }
             }
@@ -258,7 +291,7 @@ std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDiff
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                if (elem->second.fromPeer != nodeid) {
+                if (!elem->second.announcers.contains(nodeid)) {
                     iters.emplace_back(elem);
                 }
             }
@@ -273,7 +306,9 @@ std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDiff
     std::vector<std::pair<CTransactionRef, NodeId>> children_found;
     children_found.reserve(iters.size());
     for (const auto& child_iter : iters) {
-        children_found.emplace_back(child_iter->second.tx, child_iter->second.fromPeer);
+        // Use first peer in announcers list
+        auto peer = *(child_iter->second.announcers.begin());
+        children_found.emplace_back(child_iter->second.tx, peer);
     }
     return children_found;
 }
@@ -283,7 +318,7 @@ std::vector<TxOrphanage::OrphanTxBase> TxOrphanage::GetOrphanTransactions() cons
     std::vector<OrphanTxBase> ret;
     ret.reserve(m_orphans.size());
     for (auto const& o : m_orphans) {
-        ret.push_back({o.second.tx, o.second.fromPeer, o.second.nTimeExpire});
+        ret.push_back({o.second.tx, o.second.announcers, o.second.nTimeExpire});
     }
     return ret;
 }
diff --git a/src/txorphanage.h b/src/txorphanage.h
index b1fae3fa00..76ff8313a5 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -30,9 +30,15 @@ class TxOrphanage {
     /** Add a new orphan transaction */
     bool AddTx(const CTransactionRef& tx, NodeId peer);
 
+    /** Add an additional announcer to an orphan if it exists. Otherwise, do nothing. */
+    bool AddAnnouncer(const Wtxid& wtxid, NodeId peer);
+
     /** Check if we already have an orphan transaction (by wtxid only) */
     bool HaveTx(const Wtxid& wtxid) const;
 
+    /** Check if a {tx, peer} exists in the orphanage.*/
+    bool HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const;
+
     /** Extract a transaction from a peer's work set
      *  Returns nullptr if there are no transactions to work on.
      *  Otherwise returns the transaction reference, and removes
@@ -43,7 +49,8 @@ class TxOrphanage {
     /** Erase an orphan by wtxid */
     int EraseTx(const Wtxid& wtxid);
 
-    /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
+    /** Maybe erase all orphans announced by a peer (eg, after that peer disconnects). If an orphan
+     * has been announced by another peer, don't erase, just remove this peer from the list of announcers. */
     void EraseForPeer(NodeId peer);
 
     /** Erase all orphans included in or invalidated by a new block */
@@ -75,7 +82,8 @@ class TxOrphanage {
     /** Allows providing orphan information externally */
     struct OrphanTxBase {
         CTransactionRef tx;
-        NodeId fromPeer;
+        /** Peers added with AddTx or AddAnnouncer. */
+        std::set<NodeId> announcers;
         NodeSeconds nTimeExpire;
     };
 

From 04448ce32a3bc4b6d12293f71e40333abe35c224 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 3 Jan 2025 09:04:10 -0500
Subject: [PATCH 09/42] [txorphanage] add GetTx so that orphan vin can be read

---
 src/test/fuzz/txorphan.cpp   | 3 +++
 src/test/orphanage_tests.cpp | 2 ++
 src/txorphanage.cpp          | 6 ++++++
 src/txorphanage.h            | 2 ++
 4 files changed, 13 insertions(+)

diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
index 31af1afff5..09056225db 100644
--- a/src/test/fuzz/txorphan.cpp
+++ b/src/test/fuzz/txorphan.cpp
@@ -157,5 +157,8 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
             ptx_potential_parent = tx;
         }
 
+        const bool have_tx{orphanage.HaveTx(tx->GetWitnessHash())};
+        const bool get_tx_nonnull{orphanage.GetTx(tx->GetWitnessHash()) != nullptr};
+        Assert(have_tx == get_tx_nonnull);
     }
 }
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 07e7002cad..5d09fac4af 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -250,7 +250,9 @@ BOOST_AUTO_TEST_CASE(same_txid_diff_witness)
     // EraseTx fails as transaction by this wtxid doesn't exist.
     BOOST_CHECK_EQUAL(orphanage.EraseTx(mutated_wtxid), 0);
     BOOST_CHECK(orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(orphanage.GetTx(normal_wtxid) == child_normal);
     BOOST_CHECK(!orphanage.HaveTx(mutated_wtxid));
+    BOOST_CHECK(orphanage.GetTx(mutated_wtxid) == nullptr);
 
     // Must succeed. Both transactions should be present in orphanage.
     BOOST_CHECK(orphanage.AddTx(child_mutated, peer));
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index a108ce9437..17b6622a56 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -179,6 +179,12 @@ bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
     return m_orphans.count(wtxid);
 }
 
+CTransactionRef TxOrphanage::GetTx(const Wtxid& wtxid) const
+{
+    auto it = m_orphans.find(wtxid);
+    return it != m_orphans.end() ? it->second.tx : nullptr;
+}
+
 bool TxOrphanage::HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const
 {
     auto it = m_orphans.find(wtxid);
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 76ff8313a5..6998a9aab1 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -33,6 +33,8 @@ class TxOrphanage {
     /** Add an additional announcer to an orphan if it exists. Otherwise, do nothing. */
     bool AddAnnouncer(const Wtxid& wtxid, NodeId peer);
 
+    CTransactionRef GetTx(const Wtxid& wtxid) const;
+
     /** Check if we already have an orphan transaction (by wtxid only) */
     bool HaveTx(const Wtxid& wtxid) const;
 

From 96c1a822a274689611f409246ef1573906b0083e Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 31 Jul 2024 10:24:39 +0100
Subject: [PATCH 10/42] [unit test] TxOrphanage EraseForBlock

---
 src/test/orphanage_tests.cpp | 54 ++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 5d09fac4af..49ddd10863 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -392,4 +392,58 @@ BOOST_AUTO_TEST_CASE(too_large_orphan_tx)
     BOOST_CHECK(orphanage.AddTx(MakeTransactionRef(tx), 0));
 }
 
+BOOST_AUTO_TEST_CASE(process_block)
+{
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+
+    // Create outpoints that will be spent by transactions in the block
+    std::vector<COutPoint> outpoints;
+    const uint32_t num_outpoints{6};
+    outpoints.reserve(num_outpoints);
+    for (uint32_t i{0}; i < num_outpoints; ++i) {
+        // All the hashes should be different, but change the n just in case.
+        outpoints.emplace_back(Txid::FromUint256(det_rand.rand256()), i);
+    }
+
+    CBlock block;
+    const NodeId node{0};
+
+    auto control_tx = MakeTransactionSpending({}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(control_tx, node));
+
+    auto bo_tx_same_txid = MakeTransactionSpending({outpoints.at(0)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(bo_tx_same_txid, node));
+    block.vtx.emplace_back(bo_tx_same_txid);
+
+    // 2 transactions with the same txid but different witness
+    auto b_tx_same_txid_diff_witness = MakeTransactionSpending({outpoints.at(1)}, det_rand);
+    block.vtx.emplace_back(b_tx_same_txid_diff_witness);
+
+    auto o_tx_same_txid_diff_witness = MakeMutation(b_tx_same_txid_diff_witness);
+    BOOST_CHECK(orphanage.AddTx(o_tx_same_txid_diff_witness, node));
+
+    // 2 different transactions that spend the same input.
+    auto b_tx_conflict = MakeTransactionSpending({outpoints.at(2)}, det_rand);
+    block.vtx.emplace_back(b_tx_conflict);
+
+    auto o_tx_conflict = MakeTransactionSpending({outpoints.at(2)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(o_tx_conflict, node));
+
+    // 2 different transactions that have 1 overlapping input.
+    auto b_tx_conflict_partial = MakeTransactionSpending({outpoints.at(3), outpoints.at(4)}, det_rand);
+    block.vtx.emplace_back(b_tx_conflict_partial);
+
+    auto o_tx_conflict_partial_2 = MakeTransactionSpending({outpoints.at(4), outpoints.at(5)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(o_tx_conflict_partial_2, node));
+
+    orphanage.EraseForBlock(block);
+    for (const auto& expected_removed : {bo_tx_same_txid, o_tx_same_txid_diff_witness, o_tx_conflict, o_tx_conflict_partial_2}) {
+        const auto& expected_removed_wtxid = expected_removed->GetWitnessHash();
+        BOOST_CHECK(!orphanage.HaveTx(expected_removed_wtxid));
+    }
+    // Only remaining tx is control_tx
+    BOOST_CHECK_EQUAL(orphanage.Size(), 1);
+    BOOST_CHECK(orphanage.HaveTx(control_tx->GetWitnessHash()));
+}
 BOOST_AUTO_TEST_SUITE_END()

From 22b023b09da3e2fe00467c77b105a61c1961081f Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 3 Jan 2025 10:44:36 -0500
Subject: [PATCH 11/42] [unit test] multiple orphan announcers

---
 src/test/orphanage_tests.cpp | 126 +++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 49ddd10863..a33db09ad7 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -446,4 +446,130 @@ BOOST_AUTO_TEST_CASE(process_block)
     BOOST_CHECK_EQUAL(orphanage.Size(), 1);
     BOOST_CHECK(orphanage.HaveTx(control_tx->GetWitnessHash()));
 }
+
+BOOST_AUTO_TEST_CASE(multiple_announcers)
+{
+    const NodeId node0{0};
+    const NodeId node1{1};
+    const NodeId node2{2};
+    size_t expected_total_count{0};
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+
+    // Check accounting per peer.
+    // Check that EraseForPeer works with multiple announcers.
+    {
+        auto ptx = MakeTransactionSpending({}, det_rand);
+        const auto& wtxid = ptx->GetWitnessHash();
+        BOOST_CHECK(orphanage.AddTx(ptx, node0));
+        BOOST_CHECK(orphanage.HaveTx(wtxid));
+        expected_total_count += 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // Adding again should do nothing.
+        BOOST_CHECK(!orphanage.AddTx(ptx, node0));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // We can add another tx with the same txid but different witness.
+        auto ptx_mutated{MakeMutation(ptx)};
+        BOOST_CHECK(orphanage.AddTx(ptx_mutated, node0));
+        BOOST_CHECK(orphanage.HaveTx(ptx_mutated->GetWitnessHash()));
+        expected_total_count += 1;
+
+        BOOST_CHECK(!orphanage.AddTx(ptx, node0));
+
+        // Adding a new announcer should not change overall accounting.
+        BOOST_CHECK(orphanage.AddAnnouncer(ptx->GetWitnessHash(), node2));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // If we already have this announcer, AddAnnouncer returns false.
+        BOOST_CHECK(orphanage.HaveTxFromPeer(ptx->GetWitnessHash(), node2));
+        BOOST_CHECK(!orphanage.AddAnnouncer(ptx->GetWitnessHash(), node2));
+
+        // Same with using AddTx for an existing tx, which is equivalent to using AddAnnouncer
+        BOOST_CHECK(!orphanage.AddTx(ptx, node1));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // if EraseForPeer is called for an orphan with multiple announcers, the orphanage should only
+        // erase that peer from the announcers set.
+        orphanage.EraseForPeer(node0);
+        BOOST_CHECK(orphanage.HaveTx(ptx->GetWitnessHash()));
+        BOOST_CHECK(!orphanage.HaveTxFromPeer(ptx->GetWitnessHash(), node0));
+        // node0 is the only one that announced ptx_mutated
+        BOOST_CHECK(!orphanage.HaveTx(ptx_mutated->GetWitnessHash()));
+        expected_total_count -= 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // EraseForPeer should delete the orphan if it's the only announcer left.
+        orphanage.EraseForPeer(node1);
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+        BOOST_CHECK(orphanage.HaveTx(ptx->GetWitnessHash()));
+        orphanage.EraseForPeer(node2);
+        expected_total_count -= 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+        BOOST_CHECK(!orphanage.HaveTx(ptx->GetWitnessHash()));
+    }
+
+    // Check that erasure for blocks removes for all peers.
+    {
+        CBlock block;
+        auto tx_block = MakeTransactionSpending({}, det_rand);
+        block.vtx.emplace_back(tx_block);
+        BOOST_CHECK(orphanage.AddTx(tx_block, node0));
+        BOOST_CHECK(!orphanage.AddTx(tx_block, node1));
+
+        expected_total_count += 1;
+
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        orphanage.EraseForBlock(block);
+
+        expected_total_count -= 1;
+
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+    }
+}
+BOOST_AUTO_TEST_CASE(peer_worksets)
+{
+    const NodeId node0{0};
+    const NodeId node1{1};
+    const NodeId node2{2};
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+    // AddChildrenToWorkSet should pick an announcer randomly
+    {
+        auto tx_missing_parent = MakeTransactionSpending({}, det_rand);
+        auto tx_orphan = MakeTransactionSpending({COutPoint{tx_missing_parent->GetHash(), 0}}, det_rand);
+        const auto& orphan_wtxid = tx_orphan->GetWitnessHash();
+
+        // All 3 peers are announcers.
+        BOOST_CHECK(orphanage.AddTx(tx_orphan, node0));
+        BOOST_CHECK(!orphanage.AddTx(tx_orphan, node1));
+        BOOST_CHECK(orphanage.AddAnnouncer(orphan_wtxid, node2));
+        for (NodeId node = node0; node <= node2; ++node) {
+            BOOST_CHECK(orphanage.HaveTxFromPeer(orphan_wtxid, node));
+        }
+
+        // Parent accepted: add child to all 3 worksets.
+        orphanage.AddChildrenToWorkSet(*tx_missing_parent);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node0), tx_orphan);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node1), tx_orphan);
+        // Don't call GetTxToReconsider(node2) yet because it mutates the workset.
+
+        // EraseForPeer also removes that tx from the workset.
+        orphanage.EraseForPeer(node0);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node0), nullptr);
+
+        // However, the other peers' worksets are not touched.
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node2), tx_orphan);
+
+        // Delete this tx, clearing the orphanage.
+        BOOST_CHECK_EQUAL(orphanage.EraseTx(orphan_wtxid), 1);
+        BOOST_CHECK_EQUAL(orphanage.Size(), 0);
+        for (NodeId node = node0; node <= node2; ++node) {
+            BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node), nullptr);
+            BOOST_CHECK(!orphanage.HaveTxFromPeer(orphan_wtxid, node));
+        }
+    }
+}
 BOOST_AUTO_TEST_SUITE_END()

From 163aaf285af91b49c2d788463dc1e1654c88ade6 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 31 Jul 2024 10:52:01 +0100
Subject: [PATCH 12/42] [fuzz] orphanage multiple announcer functions

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

diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
index 09056225db..26afff5bff 100644
--- a/src/test/fuzz/txorphan.cpp
+++ b/src/test/fuzz/txorphan.cpp
@@ -128,6 +128,18 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                         Assert(!have_tx || !add_tx);
                     }
                 },
+                [&] {
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
+                    bool have_tx_and_peer = orphanage.HaveTxFromPeer(tx->GetWitnessHash(), peer_id);
+                    // AddAnnouncer should return false if tx doesn't exist or we already HaveTxFromPeer.
+                    {
+                        bool added_announcer = orphanage.AddAnnouncer(tx->GetWitnessHash(), peer_id);
+                        // have_tx == false -> added_announcer == false
+                        Assert(have_tx || !added_announcer);
+                        // have_tx_and_peer == true -> added_announcer == false
+                        Assert(!have_tx_and_peer || !added_announcer);
+                    }
+                },
                 [&] {
                     bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // EraseTx should return 0 if m_orphans doesn't have the tx
@@ -142,6 +154,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                 },
                 [&] {
                     orphanage.EraseForPeer(peer_id);
+                    Assert(!orphanage.HaveTxFromPeer(tx->GetWitnessHash(), peer_id));
                 },
                 [&] {
                     // test mocktime and expiry

From c6893b0f0b7b205c8da4b9d281a55c9eb843b582 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 28 Nov 2024 06:43:08 -0500
Subject: [PATCH 13/42] [txdownload] remove unique_parents that we already have

This means we no longer return parents we already have in the
m_unique_parents result from MempoolRejectedTx.

We need to separate the loop that checks AlreadyHave parents from the
loop that adds parents as announcements, because we may do the latter
loop multiple times for different peers.
---
 src/node/txdownloadman_impl.cpp | 12 +++++++-----
 src/test/txdownload_tests.cpp   |  9 +++++++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index e9fe958f5d..804c00494f 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -348,6 +348,12 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 }
             }
             if (!fRejectedParents) {
+                // Filter parents that we already have.
+                // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been
+                // previously rejected for being too low feerate. This orphan might CPFP it.
+                std::erase_if(unique_parents, [&](const auto& txid){
+                    return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+                });
                 const auto current_time{GetTime<std::chrono::microseconds>()};
 
                 for (const uint256& parent_txid : unique_parents) {
@@ -357,11 +363,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                     // Eventually we should replace this with an improved
                     // protocol for getting all unconfirmed parents.
                     const auto gtxid{GenTxid::Txid(parent_txid)};
-                    // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been
-                    // previously rejected for being too low feerate. This orphan might CPFP it.
-                    if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) {
-                        AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false);
-                    }
+                    AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false);
                 }
 
                 // Potentially flip add_extra_compact_tx to false if AddTx returns false because the tx was already there
diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp
index 6d277b5629..88064b5a5f 100644
--- a/src/test/txdownload_tests.cpp
+++ b/src/test/txdownload_tests.cpp
@@ -231,10 +231,14 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup)
         // it's in RecentRejectsFilter. Specifically, the parent is allowed to be in
         // RecentRejectsReconsiderableFilter, but it cannot be in RecentRejectsFilter.
         const bool expect_keep_orphan = !parent_recent_rej;
+        const unsigned int expected_parents = parent_recent_rej || parent_recent_conf || parent_in_mempool ? 0 : 1;
+        // If we don't expect to keep the orphan then expected_parents is 0.
+        // !expect_keep_orphan => (expected_parents == 0)
+        BOOST_CHECK(expect_keep_orphan || expected_parents == 0);
         const auto ret_1p1c = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true);
         std::string err_msg;
         const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1p1c, err_msg,
-                                            /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expect_keep_orphan ? 1 : 0);
+                                            /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expected_parents);
         BOOST_CHECK_MESSAGE(ok, err_msg);
     }
 
@@ -278,11 +282,12 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup)
             for (int32_t i = 1; i < num_parents; ++i) {
                 txdownload_impl.RecentConfirmedTransactionsFilter().insert(parents[i]->GetHash().ToUint256());
             }
+            const unsigned int expected_parents = 1;
 
             const auto ret_1recon_conf = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true);
             std::string err_msg;
             const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1recon_conf, err_msg,
-                                                /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/num_parents);
+                                                /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/expected_parents);
             BOOST_CHECK_MESSAGE(ok, err_msg);
         }
 

From 1d2e1d709ce3d95d409254c860347bc3fedf30e1 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 3 Jan 2025 09:05:04 -0500
Subject: [PATCH 14/42] [refactor] move creation of unique_parents to helper
 function

This function will be reused in a later commit.
---
 src/node/txdownloadman_impl.cpp | 23 ++++++++++++++++-------
 src/node/txdownloadman_impl.h   |  4 ++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index 804c00494f..2057c8560a 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -301,6 +301,21 @@ void TxDownloadManagerImpl::MempoolAcceptedTx(const CTransactionRef& tx)
     m_orphanage.EraseTx(tx->GetWitnessHash());
 }
 
+std::vector<Txid> TxDownloadManagerImpl::GetUniqueParents(const CTransaction& tx)
+{
+    std::vector<Txid> unique_parents;
+    unique_parents.reserve(tx.vin.size());
+    for (const CTxIn& txin : tx.vin) {
+        // We start with all parents, and then remove duplicates below.
+        unique_parents.push_back(txin.prevout.hash);
+    }
+
+    std::sort(unique_parents.begin(), unique_parents.end());
+    unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
+
+    return unique_parents;
+}
+
 node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransactionRef& ptx, const TxValidationState& state, NodeId nodeid, bool first_time_failure)
 {
     const CTransaction& tx{*ptx};
@@ -320,13 +335,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
 
             // Deduplicate parent txids, so that we don't have to loop over
             // the same parent txid more than once down below.
-            unique_parents.reserve(tx.vin.size());
-            for (const CTxIn& txin : tx.vin) {
-                // We start with all parents, and then remove duplicates below.
-                unique_parents.push_back(txin.prevout.hash);
-            }
-            std::sort(unique_parents.begin(), unique_parents.end());
-            unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
+            unique_parents = GetUniqueParents(tx);
 
             // Distinguish between parents in m_lazy_recent_rejects and m_lazy_recent_rejects_reconsiderable.
             // We can tolerate having up to 1 parent in m_lazy_recent_rejects_reconsiderable since we
diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h
index 8039ddb3cb..f97ff0609a 100644
--- a/src/node/txdownloadman_impl.h
+++ b/src/node/txdownloadman_impl.h
@@ -189,6 +189,10 @@ class TxDownloadManagerImpl {
     void CheckIsEmpty(NodeId nodeid);
 
     std::vector<TxOrphanage::OrphanTxBase> GetOrphanTransactions() const;
+
+protected:
+    /** Helper for getting deduplicated vector of Txids in vin. */
+    std::vector<Txid> GetUniqueParents(const CTransaction& tx);
 };
 } // namespace node
 #endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H

From b6ea4a9afe2d8bbf49b6b6c42f0a3ce4390c4535 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 26 Jul 2024 15:02:02 +0100
Subject: [PATCH 15/42] [p2p] try multiple peers for orphan resolution

Co-authored-by: dergoegge <n.goeggi@gmail.com>
---
 src/node/txdownloadman_impl.cpp | 104 +++++++++++++++++++++++++++-----
 src/node/txdownloadman_impl.h   |   6 ++
 test/functional/p2p_segwit.py   |   3 +
 3 files changed, 98 insertions(+), 15 deletions(-)

diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index 2057c8560a..c7eacff4b6 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -174,9 +174,37 @@ void TxDownloadManagerImpl::DisconnectedPeer(NodeId nodeid)
 
 bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv)
 {
+    // If this is an orphan we are trying to resolve, consider this peer as a orphan resolution candidate instead.
+    // - received as an p2p inv
+    // - is wtxid matching something in orphanage
+    // - exists in orphanage
+    // - peer can be an orphan resolution candidate
+    if (p2p_inv && gtxid.IsWtxid()) {
+        if (auto orphan_tx{m_orphanage.GetTx(Wtxid::FromUint256(gtxid.GetHash()))}) {
+            auto unique_parents{GetUniqueParents(*orphan_tx)};
+            std::erase_if(unique_parents, [&](const auto& txid){
+                return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+            });
+
+            if (unique_parents.empty()) return true;
+
+            if (auto delay{OrphanResolutionCandidate(peer, Wtxid::FromUint256(gtxid.GetHash()), unique_parents.size())}) {
+                m_orphanage.AddAnnouncer(Wtxid::FromUint256(gtxid.GetHash()), peer);
+
+                const auto& info = m_peer_info.at(peer).m_connection_info;
+                for (const auto& parent_txid : unique_parents) {
+                    m_txrequest.ReceivedInv(peer, GenTxid::Txid(parent_txid), info.m_preferred, now + *delay);
+                }
+
+                LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", peer, gtxid.GetHash().ToString());
+            }
+
+            // Return even if the peer isn't an orphan resolution candidate. This would be caught by AlreadyHaveTx.
+            return true;
+        }
+    }
+
     // If this is an inv received from a peer and we already have it, we can drop it.
-    // If this is a request for the parent of an orphan, we don't drop transactions that we already have. In particular,
-    // we *do* want to request parents that are in m_lazy_recent_rejects_reconsiderable, since they can be CPFP'd.
     if (p2p_inv && AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true;
 
     auto it = m_peer_info.find(peer);
@@ -204,6 +232,36 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
     return false;
 }
 
+std::optional<std::chrono::seconds> TxDownloadManagerImpl::OrphanResolutionCandidate(NodeId nodeid, const Wtxid& orphan_wtxid, size_t num_parents)
+{
+    if (m_peer_info.count(nodeid) == 0) return std::nullopt;
+    if (m_orphanage.HaveTxFromPeer(orphan_wtxid, nodeid)) return std::nullopt;
+
+    const auto& peer_entry = m_peer_info.at(nodeid);
+    const auto& info = peer_entry.m_connection_info;
+    // TODO: add delays and limits based on the amount of orphan resolution we are already doing
+    // with this peer, how much they are using the orphanage, etc.
+    if (!info.m_relay_permissions) {
+        // This mirrors the delaying and dropping behavior in AddTxAnnouncement in order to preserve
+        // existing behavior: drop if we are tracking too many invs for this peer already. Each
+        // orphan resolution involves at least 1 transaction request which may or may not be
+        // currently tracked in m_txrequest, so we include that in the count.
+        if (m_txrequest.Count(nodeid) + num_parents > MAX_PEER_TX_ANNOUNCEMENTS) return std::nullopt;
+    }
+
+    std::chrono::seconds delay{0s};
+    if (!info.m_preferred) delay += NONPREF_PEER_TX_DELAY;
+    // The orphan wtxid is used, but resolution entails requesting the parents by txid. Sometimes
+    // parent and child are announced and thus requested around the same time, and we happen to
+    // receive child sooner. Waiting a few seconds may allow us to cancel the orphan resolution
+    // request if the parent arrives in that time.
+    if (m_num_wtxid_peers > 0) delay += TXID_RELAY_DELAY;
+    const bool overloaded = !info.m_relay_permissions && m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
+    if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
+
+    return delay;
+}
+
 std::vector<GenTxid> TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time)
 {
     std::vector<GenTxid> requests;
@@ -363,26 +421,42 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 std::erase_if(unique_parents, [&](const auto& txid){
                     return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
                 });
-                const auto current_time{GetTime<std::chrono::microseconds>()};
-
-                for (const uint256& parent_txid : unique_parents) {
-                    // Here, we only have the txid (and not wtxid) of the
-                    // inputs, so we only request in txid mode, even for
-                    // wtxidrelay peers.
-                    // Eventually we should replace this with an improved
-                    // protocol for getting all unconfirmed parents.
-                    const auto gtxid{GenTxid::Txid(parent_txid)};
-                    AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false);
+                const auto now{GetTime<std::chrono::microseconds>()};
+                const auto& wtxid = ptx->GetWitnessHash();
+                // Potentially flip add_extra_compact_tx to false if tx is already in orphanage, which
+                // means it was already added to vExtraTxnForCompact.
+                add_extra_compact_tx &= !m_orphanage.HaveTx(wtxid);
+
+                auto add_orphan_reso_candidate = [&](const CTransactionRef& orphan_tx, const std::vector<Txid>& unique_parents, NodeId nodeid, std::chrono::microseconds now) {
+                    const auto& wtxid = orphan_tx->GetWitnessHash();
+                    if (auto delay{OrphanResolutionCandidate(nodeid, wtxid, unique_parents.size())}) {
+                        const auto& info = m_peer_info.at(nodeid).m_connection_info;
+                        m_orphanage.AddTx(orphan_tx, nodeid);
+
+                        // Treat finding orphan resolution candidate as equivalent to the peer announcing all missing parents
+                        // In the future, orphan resolution may include more explicit steps
+                        for (const auto& parent_txid : unique_parents) {
+                            m_txrequest.ReceivedInv(nodeid, GenTxid::Txid(parent_txid), info.m_preferred, now + *delay);
+                        }
+                        LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", nodeid, wtxid.ToString());
+                    }
+                };
+
+                // If there is no candidate for orphan resolution, AddTx will not be called. This means
+                // that if a peer is overloading us with invs and orphans, they will eventually not be
+                // able to add any more transactions to the orphanage.
+                add_orphan_reso_candidate(ptx, unique_parents, nodeid, now);
+                for (const auto& candidate : m_txrequest.GetCandidatePeers(ptx)) {
+                    add_orphan_reso_candidate(ptx, unique_parents, candidate, now);
                 }
 
-                // Potentially flip add_extra_compact_tx to false if AddTx returns false because the tx was already there
-                add_extra_compact_tx &= m_orphanage.AddTx(ptx, nodeid);
-
                 // Once added to the orphan pool, a tx is considered AlreadyHave, and we shouldn't request it anymore.
                 m_txrequest.ForgetTxHash(tx.GetHash());
                 m_txrequest.ForgetTxHash(tx.GetWitnessHash());
 
                 // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
+                // Note that, if the orphanage reaches capacity, it's possible that we immediately evict
+                // the transaction we just added.
                 m_orphanage.LimitOrphans(m_opts.m_max_orphan_txs, m_opts.m_rng);
             } else {
                 unique_parents.clear();
diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h
index f97ff0609a..e5e487c479 100644
--- a/src/node/txdownloadman_impl.h
+++ b/src/node/txdownloadman_impl.h
@@ -193,6 +193,12 @@ class TxDownloadManagerImpl {
 protected:
     /** Helper for getting deduplicated vector of Txids in vin. */
     std::vector<Txid> GetUniqueParents(const CTransaction& tx);
+
+    /** Determine candidacy (and delay) for potential orphan resolution candidate.
+     * @returns delay for orphan resolution if this peer is a good candidate for orphan resolution,
+     * std::nullopt if this peer cannot be added because it has reached download/orphanage limits.
+     * */
+    std::optional<std::chrono::seconds> OrphanResolutionCandidate(NodeId nodeid, const Wtxid& orphan_wtxid, size_t num_parents);
 };
 } // namespace node
 #endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H
diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index 677a1120d6..9caf5a19aa 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -2051,6 +2051,9 @@ def received_wtxidrelay():
             self.wtx_node.last_message.pop("getdata", None)
         test_transaction_acceptance(self.nodes[0], self.wtx_node, tx2, with_witness=True, accepted=False)
 
+        # Disconnect tx_node to avoid the possibility of it being selected for orphan resolution.
+        self.tx_node.peer_disconnect()
+
         # Expect a request for parent (tx) by txid despite use of WTX peer
         self.wtx_node.wait_for_getdata([tx.sha256], timeout=60)
         with p2p_lock:

From 0da693f7e129fccaecf9a2c177083d2e80d37781 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 26 Jul 2024 16:34:55 +0100
Subject: [PATCH 16/42] [functional test] orphan handling with multiple
 announcers

---
 test/functional/p2p_1p1c_network.py    |   6 -
 test/functional/p2p_orphan_handling.py | 181 ++++++++++++++++++++++++-
 2 files changed, 177 insertions(+), 10 deletions(-)

diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py
index cdc4e1691d..eef307d1eb 100755
--- a/test/functional/p2p_1p1c_network.py
+++ b/test/functional/p2p_1p1c_network.py
@@ -143,12 +143,6 @@ def run_test(self):
         for (i, peer) in enumerate(self.peers):
             for tx in transactions_to_presend[i]:
                 peer.send_and_ping(msg_tx(tx))
-            # This disconnect removes any sent orphans from the orphanage (EraseForPeer) and times
-            # out the in-flight requests.  It is currently required for the test to pass right now,
-            # because the node will not reconsider an orphan tx and will not (re)try requesting
-            # orphan parents from multiple peers if the first one didn't respond.
-            # TODO: remove this in the future if the node tries orphan resolution with multiple peers.
-            peer.peer_disconnect()
 
         self.log.info("Submit full packages to node0")
         for package_hex in packages_to_submit:
diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py
index 0864deeb4f..4a2d53cd2b 100755
--- a/test/functional/p2p_orphan_handling.py
+++ b/test/functional/p2p_orphan_handling.py
@@ -58,6 +58,10 @@ def wrapper(self):
             self.generate(self.nodes[0], 1)
             self.nodes[0].disconnect_p2ps()
             self.nodes[0].bumpmocktime(LONG_TIME_SKIP)
+            # Check that mempool and orphanage have been cleared
+            assert_equal(0, len(self.nodes[0].getorphantxs()))
+            assert_equal(0, len(self.nodes[0].getrawmempool()))
+            self.wallet.rescan_utxos(include_mempool=True)
     return wrapper
 
 class PeerTxRelayer(P2PTxInvStore):
@@ -533,7 +537,7 @@ def test_same_txid_orphan_of_orphan(self):
         assert tx_middle["txid"] in node_mempool
         assert tx_grandchild["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_middle["txid"])["wtxid"], tx_middle["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_orphan_txid_inv(self):
@@ -585,7 +589,7 @@ def test_orphan_txid_inv(self):
         assert tx_parent["txid"] in node_mempool
         assert tx_child["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_max_orphan_amount(self):
@@ -610,7 +614,7 @@ def test_max_orphan_amount(self):
 
         peer_1.sync_with_ping()
         orphanage = node.getorphantxs()
-        assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
+        self.wait_until(lambda: len(node.getorphantxs()) == DEFAULT_MAX_ORPHAN_TRANSACTIONS)
 
         for orphan in orphans:
             assert tx_in_orphanage(node, orphan)
@@ -626,8 +630,173 @@ def test_max_orphan_amount(self):
         self.log.info("Clearing the orphanage")
         for index, parent_orphan in enumerate(parent_orphans):
             peer_1.send_and_ping(msg_tx(parent_orphan))
-        assert_equal(len(node.getorphantxs()),0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
+
+    @cleanup
+    def test_orphan_handling_prefer_outbound(self):
+        self.log.info("Test that the node prefers requesting from outbound peers")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        peer_inbound = node.add_p2p_connection(PeerTxRelayer())
+        peer_outbound = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=1)
+
+        # Inbound peer relays the transaction.
+        peer_inbound.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_inbound.wait_for_getdata([int(orphan_wtxid, 16)])
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_outbound.send_and_ping(msg_inv([orphan_inv]))
+
+        peer_inbound.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # The outbound peer should be preferred for getting orphan parents
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_outbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        # There should be no request to the inbound peer
+        peer_inbound.assert_never_requested(int(parent_tx.rehash(), 16))
+
+        self.log.info("Test that, if the preferred peer doesn't respond, the node sends another request")
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_inbound.sync_with_ping()
+        peer_inbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_announcers_before_and_after(self):
+        self.log.info("Test that the node uses all peers who announced the tx prior to realizing it's an orphan")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # Announces before tx is sent, disconnects while node is requesting parents
+        peer_early_disconnected = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=3)
+        # Announces before tx is sent, doesn't respond to parent request
+        peer_early_unresponsive = node.add_p2p_connection(PeerTxRelayer())
+
+        # Announces after tx is sent
+        peer_late_announcer = node.add_p2p_connection(PeerTxRelayer())
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_early_disconnected.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_early_disconnected.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer_early_unresponsive.send_and_ping(msg_inv([orphan_inv]))
+        peer_early_disconnected.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(len(orphanage), 1)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # Peer disconnects before responding to request
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_early_disconnected.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+        peer_early_disconnected.peer_disconnect()
 
+        # The orphan should have 1 announcer left after the node finishes disconnecting peer_early_disconnected.
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+
+        # The node should retry with the other peer that announced the orphan earlier.
+        # This node's request was additionally delayed because it's an inbound peer.
+        self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
+        peer_early_unresponsive.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        self.log.info("Test that the node uses peers who announce the tx after realizing it's an orphan")
+        peer_late_announcer.send_and_ping(msg_inv([orphan_inv]))
+
+        # The orphan should have 2 announcers now
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_late_announcer.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_parents_change(self):
+        self.log.info("Test that, if a parent goes missing during orphan reso, it is requested")
+        node = self.nodes[0]
+        # Orphan will have 2 parents, 1 missing and 1 already in mempool when received.
+        # Create missing parent.
+        parent_missing = self.wallet.create_self_transfer()
+
+        # Create parent that will already be in mempool, but become missing during orphan resolution.
+        # Get 3 UTXOs for replacement-cycled parent, UTXOS A, B, C
+        coin_A = self.wallet.get_utxo(confirmed_only=True)
+        coin_B = self.wallet.get_utxo(confirmed_only=True)
+        coin_C = self.wallet.get_utxo(confirmed_only=True)
+        # parent_peekaboo_AB spends A and B. It is replaced by tx_replacer_BC (conflicting UTXO B),
+        # and then replaced by tx_replacer_C (conflicting UTXO C). This replacement cycle is used to
+        # ensure that parent_peekaboo_AB can be reintroduced without requiring package RBF.
+        FEE_INCREMENT = 2400
+        parent_peekaboo_AB = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_A, coin_B],
+            num_outputs=1,
+            fee_per_output=FEE_INCREMENT
+        )
+        tx_replacer_BC = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_B, coin_C],
+            num_outputs=1,
+            fee_per_output=2*FEE_INCREMENT
+        )
+        tx_replacer_C = self.wallet.create_self_transfer(
+            utxo_to_spend=coin_C,
+            fee_per_output=3*FEE_INCREMENT
+        )
+
+        # parent_peekaboo_AB starts out in the mempool
+        node.sendrawtransaction(parent_peekaboo_AB["hex"])
+
+        orphan = self.wallet.create_self_transfer_multi(utxos_to_spend=[parent_peekaboo_AB["new_utxos"][0], parent_missing["new_utxo"]])
+        orphan_wtxid = orphan["wtxid"]
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # peer1 sends the orphan and gets a request for the missing parent
+        peer1 = node.add_p2p_connection(PeerTxRelayer())
+        peer1.send_and_ping(msg_inv([orphan_inv]))
+        node.bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer1.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer1.send_and_ping(msg_tx(orphan["tx"]))
+        self.wait_until(lambda: node.getorphantxs(verbosity=0) == [orphan["txid"]])
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        peer1.wait_for_getdata([int(parent_missing["txid"], 16)])
+
+        # Replace parent_peekaboo_AB so that is is a newly missing parent.
+        # Then, replace the replacement so that it can be resubmitted.
+        node.sendrawtransaction(tx_replacer_BC["hex"])
+        assert tx_replacer_BC["txid"] in node.getrawmempool()
+        node.sendrawtransaction(tx_replacer_C["hex"])
+        assert tx_replacer_BC["txid"] not in node.getrawmempool()
+        assert tx_replacer_C["txid"] in node.getrawmempool()
+
+        # Second peer is an additional announcer for this orphan
+        peer2 = node.add_p2p_connection(PeerTxRelayer())
+        peer2.send_and_ping(msg_inv([orphan_inv]))
+        assert_equal(len(node.getorphantxs(verbosity=2)[0]["from"]), 2)
+
+        # Disconnect peer1. peer2 should become the new candidate for orphan resolution.
+        peer1.peer_disconnect()
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+        # Both parents should be requested, now that they are both missing.
+        peer2.wait_for_parent_requests([int(parent_peekaboo_AB["txid"], 16), int(parent_missing["txid"], 16)])
+        peer2.send_and_ping(msg_tx(parent_missing["tx"]))
+        peer2.send_and_ping(msg_tx(parent_peekaboo_AB["tx"]))
+
+        final_mempool = node.getrawmempool()
+        assert parent_missing["txid"] in final_mempool
+        assert parent_peekaboo_AB["txid"] in final_mempool
+        assert orphan["txid"] in final_mempool
+        assert tx_replacer_C["txid"] in final_mempool
 
     def run_test(self):
         self.nodes[0].setmocktime(int(time.time()))
@@ -635,6 +804,7 @@ def run_test(self):
         self.generate(self.wallet_nonsegwit, 10)
         self.wallet = MiniWallet(self.nodes[0])
         self.generate(self.wallet, 160)
+
         self.test_arrival_timing_orphan()
         self.test_orphan_rejected_parents_exceptions()
         self.test_orphan_multiple_parents()
@@ -645,6 +815,9 @@ def run_test(self):
         self.test_same_txid_orphan_of_orphan()
         self.test_orphan_txid_inv()
         self.test_max_orphan_amount()
+        self.test_orphan_handling_prefer_outbound()
+        self.test_announcers_before_and_after()
+        self.test_parents_change()
 
 
 if __name__ == '__main__':

From 063c1324c143d98e6d5108bb51b3ca59b45f9b85 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 11 Dec 2024 07:18:10 -0500
Subject: [PATCH 17/42] [functional test] getorphantxs reflects multiple
 announcers

---
 test/functional/rpc_orphans.py | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/test/functional/rpc_orphans.py b/test/functional/rpc_orphans.py
index 4871166a39..3dff1ac9d9 100755
--- a/test/functional/rpc_orphans.py
+++ b/test/functional/rpc_orphans.py
@@ -10,7 +10,12 @@
     ORPHAN_TX_EXPIRE_TIME,
     tx_in_orphanage,
 )
-from test_framework.messages import msg_tx
+from test_framework.messages import (
+    CInv,
+    msg_inv,
+    msg_tx,
+    MSG_WTX,
+)
 from test_framework.p2p import P2PInterface
 from test_framework.util import (
     assert_equal,
@@ -106,13 +111,19 @@ def test_orphan_details(self):
 
         self.log.info("Check that orphan 1 and 2 were from different peers")
         assert orphanage[0]["from"][0] != orphanage[1]["from"][0]
+        peer_ids = [orphanage[0]["from"][0], orphanage[1]["from"][0]]
 
         self.log.info("Unorphan child 2")
         peer_2.send_and_ping(msg_tx(tx_parent_2["tx"]))
         assert not tx_in_orphanage(node, tx_child_2["tx"])
 
+        self.log.info("Check that additional announcers are reflected in RPC result")
+        peer_2.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(tx_child_1["wtxid"], 16))]))
+
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(set(orphanage[0]["from"]), set(peer_ids))
+
         self.log.info("Checking orphan details")
-        orphanage = node.getorphantxs(verbosity=1)
         assert_equal(len(node.getorphantxs()), 1)
         orphan_1 = orphanage[0]
         self.orphan_details_match(orphan_1, tx_child_1, verbosity=1)

From f7658d9b1475ecaa5cb8e543e5c66a3a3a2dc1fb Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 6 Dec 2024 07:02:14 -0500
Subject: [PATCH 18/42] [cleanup] remove p2p_inv from AddTxAnnouncement

This param is no longer needed since orphan parent requests are added to
the TxRequestTracker directly.
---
 src/net_processing.cpp          |  2 +-
 src/node/txdownloadman.h        |  3 +--
 src/node/txdownloadman_impl.cpp | 11 +++++------
 src/node/txdownloadman_impl.h   |  2 +-
 src/test/fuzz/txdownloadman.cpp |  4 ++--
 src/test/txdownload_tests.cpp   |  4 ++--
 6 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 663dbb7634..e4c57c0336 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3935,7 +3935,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 AddKnownTx(*peer, inv.hash);
 
                 if (!m_chainman.IsInitialBlockDownload()) {
-                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time, /*p2p_inv=*/true)};
+                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time)};
                     LogDebug(BCLog::NET, "got inv: %s  %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
                 }
             } else {
diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h
index 2d066cab53..1ae833a6be 100644
--- a/src/node/txdownloadman.h
+++ b/src/node/txdownloadman.h
@@ -136,9 +136,8 @@ class TxDownloadManager {
 
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
-     * @param[in] p2p_inv     When true, only add this announcement if we don't already have the tx.
      * Returns true if this was a dropped inv (p2p_inv=true and we already have the tx), false otherwise. */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv);
+    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);
diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index c7eacff4b6..82443115ba 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -39,9 +39,9 @@ void TxDownloadManager::DisconnectedPeer(NodeId nodeid)
 {
     m_impl->DisconnectedPeer(nodeid);
 }
-bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv)
+bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
 {
-    return m_impl->AddTxAnnouncement(peer, gtxid, now, p2p_inv);
+    return m_impl->AddTxAnnouncement(peer, gtxid, now);
 }
 std::vector<GenTxid> TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time)
 {
@@ -172,14 +172,13 @@ void TxDownloadManagerImpl::DisconnectedPeer(NodeId nodeid)
 
 }
 
-bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv)
+bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
 {
     // If this is an orphan we are trying to resolve, consider this peer as a orphan resolution candidate instead.
-    // - received as an p2p inv
     // - is wtxid matching something in orphanage
     // - exists in orphanage
     // - peer can be an orphan resolution candidate
-    if (p2p_inv && gtxid.IsWtxid()) {
+    if (gtxid.IsWtxid()) {
         if (auto orphan_tx{m_orphanage.GetTx(Wtxid::FromUint256(gtxid.GetHash()))}) {
             auto unique_parents{GetUniqueParents(*orphan_tx)};
             std::erase_if(unique_parents, [&](const auto& txid){
@@ -205,7 +204,7 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
     }
 
     // If this is an inv received from a peer and we already have it, we can drop it.
-    if (p2p_inv && AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true;
+    if (AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true;
 
     auto it = m_peer_info.find(peer);
     if (it == m_peer_info.end()) return false;
diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h
index e5e487c479..ab563b2241 100644
--- a/src/node/txdownloadman_impl.h
+++ b/src/node/txdownloadman_impl.h
@@ -163,7 +163,7 @@ class TxDownloadManagerImpl {
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
      */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv);
+    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);
diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp
index 8a4d7d55c2..f78cd71ce9 100644
--- a/src/test/fuzz/txdownloadman.cpp
+++ b/src/test/fuzz/txdownloadman.cpp
@@ -227,7 +227,7 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
                 GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
                                 GenTxid::Txid(rand_tx->GetHash()) :
                                 GenTxid::Wtxid(rand_tx->GetWitnessHash());
-                txdownloadman.AddTxAnnouncement(rand_peer, gtxid, time, /*p2p_inv=*/fuzzed_data_provider.ConsumeBool());
+                txdownloadman.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
                 txdownloadman.GetRequestsToSend(rand_peer, time);
@@ -370,7 +370,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                 GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
                                 GenTxid::Txid(rand_tx->GetHash()) :
                                 GenTxid::Wtxid(rand_tx->GetWitnessHash());
-                txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time, /*p2p_inv=*/fuzzed_data_provider.ConsumeBool());
+                txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
                 const auto getdata_requests = txdownload_impl.GetRequestsToSend(rand_peer, time);
diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp
index 88064b5a5f..463eb21013 100644
--- a/src/test/txdownload_tests.cpp
+++ b/src/test/txdownload_tests.cpp
@@ -146,8 +146,8 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
                     /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid),
                     /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid),
                     /*keep=*/keep,
-                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now, /*p2p_inv=*/true),
-                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), now, /*p2p_inv=*/true),
+                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now),
+                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), now),
                 };
                 BOOST_TEST_MESSAGE("Testing behavior for " << result << (segwit_parent ? " segwit " : " nonsegwit"));
                 actual_behavior.CheckEqual(expected_behavior, /*segwit=*/segwit_parent);

From 86d7135e36efd39781cf4c969011df99f0cbb69d Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 20 Aug 2024 11:39:18 +0100
Subject: [PATCH 19/42] [p2p] only attempt 1p1c when both txns provided by the
 same peer

Now that we track all announcers of an orphan, it's not helpful to
consider an orphan provided by a peer that didn't send us this parent.
It can only hurt our chances of finding the right orphan when there are
multiple candidates.

Adapt the 2 tests in p2p_opportunistic_1p1c.py that looked at 1p1c
packages from different peers. Instead of checking that the right peer
is punished, we now check that the package is not submitted. We can't
use the functional test to see that the package was not considered
because the behavior is indistinguishable (except for the logs).
---
 src/node/txdownloadman_impl.cpp           | 34 +++--------------------
 src/test/fuzz/txorphan.cpp                |  6 ----
 src/test/orphanage_tests.cpp              | 18 ------------
 src/txorphanage.cpp                       | 32 ---------------------
 src/txorphanage.h                         |  4 ---
 test/functional/p2p_opportunistic_1p1c.py | 21 +++++++-------
 6 files changed, 14 insertions(+), 101 deletions(-)

diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index 82443115ba..fe961048d3 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -300,9 +300,11 @@ std::optional<PackageToValidate> TxDownloadManagerImpl::Find1P1CPackage(const CT
 
     Assume(RecentRejectsReconsiderableFilter().contains(parent_wtxid.ToUint256()));
 
-    // Prefer children from this peer. This helps prevent censorship attempts in which an attacker
+    // Only consider children from this peer. This helps prevent censorship attempts in which an attacker
     // sends lots of fake children for the parent, and we (unluckily) keep selecting the fake
-    // children instead of the real one provided by the honest peer.
+    // children instead of the real one provided by the honest peer. Since we track all announcers
+    // of an orphan, this does not exclude parent + orphan pairs that we happened to request from
+    // different peers.
     const auto cpfp_candidates_same_peer{m_orphanage.GetChildrenFromSamePeer(ptx, nodeid)};
 
     // These children should be sorted from newest to oldest. In the (probably uncommon) case
@@ -315,34 +317,6 @@ std::optional<PackageToValidate> TxDownloadManagerImpl::Find1P1CPackage(const CT
             return PackageToValidate{ptx, child, nodeid, nodeid};
         }
     }
-
-    // If no suitable candidate from the same peer is found, also try children that were provided by
-    // a different peer. This is useful because sometimes multiple peers announce both transactions
-    // to us, and we happen to download them from different peers (we wouldn't have known that these
-    // 2 transactions are related). We still want to find 1p1c packages then.
-    //
-    // If we start tracking all announcers of orphans, we can restrict this logic to parent + child
-    // pairs in which both were provided by the same peer, i.e. delete this step.
-    const auto cpfp_candidates_different_peer{m_orphanage.GetChildrenFromDifferentPeer(ptx, nodeid)};
-
-    // Find the first 1p1c that hasn't already been rejected. We randomize the order to not
-    // create a bias that attackers can use to delay package acceptance.
-    //
-    // Create a random permutation of the indices.
-    std::vector<size_t> tx_indices(cpfp_candidates_different_peer.size());
-    std::iota(tx_indices.begin(), tx_indices.end(), 0);
-    std::shuffle(tx_indices.begin(), tx_indices.end(), m_opts.m_rng);
-
-    for (const auto index : tx_indices) {
-        // If we already tried a package and failed for any reason, the combined hash was
-        // cached in m_lazy_recent_rejects_reconsiderable.
-        const auto [child_tx, child_sender] = cpfp_candidates_different_peer.at(index);
-        Package maybe_cpfp_package{ptx, child_tx};
-        if (!RecentRejectsReconsiderableFilter().contains(GetPackageHash(maybe_cpfp_package)) &&
-            !RecentRejectsFilter().contains(child_tx->GetHash().ToUint256())) {
-            return PackageToValidate{ptx, child_tx, nodeid, child_sender};
-        }
-    }
     return std::nullopt;
 }
 
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
index 26afff5bff..61c2308a29 100644
--- a/src/test/fuzz/txorphan.cpp
+++ b/src/test/fuzz/txorphan.cpp
@@ -88,12 +88,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     return input.prevout.hash == ptx_potential_parent->GetHash();
                 }));
             }
-            for (const auto& [child, peer] : orphanage.GetChildrenFromDifferentPeer(ptx_potential_parent, peer_id)) {
-                assert(std::any_of(child->vin.cbegin(), child->vin.cend(), [&](const auto& input) {
-                    return input.prevout.hash == ptx_potential_parent->GetHash();
-                }));
-                assert(peer != peer_id);
-            }
         }
 
         // trigger orphanage functions
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index a33db09ad7..f30d4b402f 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -93,15 +93,6 @@ static bool EqualTxns(const std::set<CTransactionRef>& set_txns, const std::vect
     }
     return true;
 }
-static bool EqualTxns(const std::set<CTransactionRef>& set_txns,
-                      const std::vector<std::pair<CTransactionRef, NodeId>>& vec_txns)
-{
-    if (vec_txns.size() != set_txns.size()) return false;
-    for (const auto& [tx, nodeid] : vec_txns) {
-        if (!set_txns.contains(tx)) return false;
-    }
-    return true;
-}
 
 BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
 {
@@ -312,9 +303,6 @@ BOOST_AUTO_TEST_CASE(get_children)
         BOOST_CHECK(EqualTxns(expected_parent1_children, orphanage.GetChildrenFromSamePeer(parent1, node1)));
         BOOST_CHECK(EqualTxns(expected_parent2_children, orphanage.GetChildrenFromSamePeer(parent2, node1)));
 
-        BOOST_CHECK(EqualTxns(expected_parent1_children, orphanage.GetChildrenFromDifferentPeer(parent1, node2)));
-        BOOST_CHECK(EqualTxns(expected_parent2_children, orphanage.GetChildrenFromDifferentPeer(parent2, node2)));
-
         // The peer must match
         BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent1, node2).empty());
         BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent2, node2).empty());
@@ -322,8 +310,6 @@ BOOST_AUTO_TEST_CASE(get_children)
         // There shouldn't be any children of this tx in the orphanage
         BOOST_CHECK(orphanage.GetChildrenFromSamePeer(child_p1n0_p2n0, node1).empty());
         BOOST_CHECK(orphanage.GetChildrenFromSamePeer(child_p1n0_p2n0, node2).empty());
-        BOOST_CHECK(orphanage.GetChildrenFromDifferentPeer(child_p1n0_p2n0, node1).empty());
-        BOOST_CHECK(orphanage.GetChildrenFromDifferentPeer(child_p1n0_p2n0, node2).empty());
     }
 
     // Orphans provided by node1 and node2
@@ -346,7 +332,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             std::set<CTransactionRef> expected_parent1_node1{child_p1n0};
 
             BOOST_CHECK(EqualTxns(expected_parent1_node1, orphanage.GetChildrenFromSamePeer(parent1, node1)));
-            BOOST_CHECK(EqualTxns(expected_parent1_node1, orphanage.GetChildrenFromDifferentPeer(parent1, node2)));
         }
 
         // Children of parent2 from node1:
@@ -354,7 +339,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             std::set<CTransactionRef> expected_parent2_node1{child_p2n1};
 
             BOOST_CHECK(EqualTxns(expected_parent2_node1, orphanage.GetChildrenFromSamePeer(parent2, node1)));
-            BOOST_CHECK(EqualTxns(expected_parent2_node1, orphanage.GetChildrenFromDifferentPeer(parent2, node2)));
         }
 
         // Children of parent1 from node2:
@@ -362,7 +346,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             std::set<CTransactionRef> expected_parent1_node2{child_p1n0_p1n1, child_p1n0_p2n0};
 
             BOOST_CHECK(EqualTxns(expected_parent1_node2, orphanage.GetChildrenFromSamePeer(parent1, node2)));
-            BOOST_CHECK(EqualTxns(expected_parent1_node2, orphanage.GetChildrenFromDifferentPeer(parent1, node1)));
         }
 
         // Children of parent2 from node2:
@@ -370,7 +353,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             std::set<CTransactionRef> expected_parent2_node2{child_p1n0_p2n0};
 
             BOOST_CHECK(EqualTxns(expected_parent2_node2, orphanage.GetChildrenFromSamePeer(parent2, node2)));
-            BOOST_CHECK(EqualTxns(expected_parent2_node2, orphanage.GetChildrenFromDifferentPeer(parent2, node1)));
         }
     }
 }
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 17b6622a56..90b3381428 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -287,38 +287,6 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac
     return children_found;
 }
 
-std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const
-{
-    // First construct vector of iterators to ensure we do not return duplicates of the same tx.
-    std::vector<OrphanMap::iterator> iters;
-
-    // For each output, get all entries spending this prevout, filtering for ones not from the specified peer.
-    for (unsigned int i = 0; i < parent->vout.size(); i++) {
-        const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
-        if (it_by_prev != m_outpoint_to_orphan_it.end()) {
-            for (const auto& elem : it_by_prev->second) {
-                if (!elem->second.announcers.contains(nodeid)) {
-                    iters.emplace_back(elem);
-                }
-            }
-        }
-    }
-
-    // Erase duplicates
-    std::sort(iters.begin(), iters.end(), IteratorComparator());
-    iters.erase(std::unique(iters.begin(), iters.end()), iters.end());
-
-    // Convert iterators to pair<CTransactionRef, NodeId>
-    std::vector<std::pair<CTransactionRef, NodeId>> children_found;
-    children_found.reserve(iters.size());
-    for (const auto& child_iter : iters) {
-        // Use first peer in announcers list
-        auto peer = *(child_iter->second.announcers.begin());
-        children_found.emplace_back(child_iter->second.tx, peer);
-    }
-    return children_found;
-}
-
 std::vector<TxOrphanage::OrphanTxBase> TxOrphanage::GetOrphanTransactions() const
 {
     std::vector<OrphanTxBase> ret;
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 6998a9aab1..868741e789 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -71,10 +71,6 @@ class TxOrphanage {
      * recent to least recent. */
     std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const;
 
-    /** Get all children that spend from this tx but were not received from nodeid. Also return
-     * which peer provided each tx. */
-    std::vector<std::pair<CTransactionRef, NodeId>> GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const;
-
     /** Return how many entries exist in the orphange */
     size_t Size() const
     {
diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py
index 4477046c8d..5cf616b3ef 100755
--- a/test/functional/p2p_opportunistic_1p1c.py
+++ b/test/functional/p2p_opportunistic_1p1c.py
@@ -215,7 +215,7 @@ def test_low_and_high_child(self, wallet):
 
     @cleanup
     def test_orphan_consensus_failure(self):
-        self.log.info("Check opportunistic 1p1c logic with consensus-invalid orphan causes disconnect of the correct peer")
+        self.log.info("Check opportunistic 1p1c logic requires parent and child to be from the same peer")
         node = self.nodes[0]
         low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
         coin = low_fee_parent["new_utxo"]
@@ -239,15 +239,17 @@ def test_orphan_consensus_failure(self):
         parent_txid_int = int(low_fee_parent["txid"], 16)
         bad_orphan_sender.wait_for_getdata([parent_txid_int])
 
-        # 3. A different peer relays the parent. Parent+Child are evaluated as a package and rejected.
-        parent_sender.send_message(msg_tx(low_fee_parent["tx"]))
+        # 3. A different peer relays the parent. Package is not evaluated because the transactions
+        # were not sent from the same peer.
+        parent_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
 
         # 4. Transactions should not be in mempool.
         node_mempool = node.getrawmempool()
         assert low_fee_parent["txid"] not in node_mempool
         assert tx_orphan_bad_wit.rehash() not in node_mempool
 
-        # 5. Peer that sent a consensus-invalid transaction should be disconnected.
+        # 5. Have the other peer send the tx too, so that tx_orphan_bad_wit package is attempted.
+        bad_orphan_sender.send_message(msg_tx(low_fee_parent["tx"]))
         bad_orphan_sender.wait_for_disconnect()
 
         # The peer that didn't provide the orphan should not be disconnected.
@@ -279,20 +281,17 @@ def test_parent_consensus_failure(self):
         package_sender.wait_for_getdata([parent_txid_int])
 
         # 3. A different node relays the parent. The parent is first evaluated by itself and
-        # rejected for being too low feerate. Then it is evaluated as a package and, after passing
-        # feerate checks, rejected for having a bad signature (consensus error).
-        fake_parent_sender.send_message(msg_tx(tx_parent_bad_wit))
+        # rejected for being too low feerate. It is not evaluated as a package because the child was
+        # sent from a different peer, so we don't find out that the child is consensus-invalid.
+        fake_parent_sender.send_and_ping(msg_tx(tx_parent_bad_wit))
 
         # 4. Transactions should not be in mempool.
         node_mempool = node.getrawmempool()
         assert tx_parent_bad_wit.rehash() not in node_mempool
         assert high_fee_child["txid"] not in node_mempool
 
-        # 5. Peer sent a consensus-invalid transaction.
-        fake_parent_sender.wait_for_disconnect()
-
         self.log.info("Check that fake parent does not cause orphan to be deleted and real package can still be submitted")
-        # 6. Child-sending should not have been punished and the orphan should remain in orphanage.
+        # 5. Child-sending should not have been punished and the orphan should remain in orphanage.
         # It can send the "real" parent transaction, and the package is accepted.
         parent_wtxid_int = int(low_fee_parent["tx"].getwtxid(), 16)
         package_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))

From 8bd5f8a38ce903c05606841ebed1902398cb0b14 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 17 Dec 2024 21:33:56 +0100
Subject: [PATCH 20/42] [refactor] init: Simplify coinsdb cache calculation

(total_cache / 4) + (1 << 23) is at least 8 MiB and nMaxCoinsDBCache is
also 8 MiB, so the minimum between the two will always be
nMaxCoinsDBCache. This is just a simplification and not changing the
result of the calculation.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/node/caches.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index dc4d98f592..01b08e320d 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -24,8 +24,7 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
         sizes.filter_index = max_cache / n_indexes;
         nTotalCache -= sizes.filter_index * n_indexes;
     }
-    sizes.coins_db = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache
-    sizes.coins_db = std::min(sizes.coins_db, nMaxCoinsDBCache << 20); // cap total coins db cache
+    sizes.coins_db = std::min(nTotalCache / 2, nMaxCoinsDBCache << 20);
     nTotalCache -= sizes.coins_db;
     sizes.coins = nTotalCache; // the rest goes to in-memory cache
     return sizes;

From 727c54276967e001f43831db83c790a1dec7a598 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Thu, 9 Jan 2025 20:24:05 +0000
Subject: [PATCH 21/42] depends: Use base system's `sha256sum` utility

On FreeBSD, the `shasum` utility is provided by the `perl5` port, which
is not part of the base system and must be installed separately.
Note that this requirement is currently not documented in
`depends/README.md`.

This change switches to using the `sha256sum` utility, which is included
in the base system.
---
 depends/builders/freebsd.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/depends/builders/freebsd.mk b/depends/builders/freebsd.mk
index 465f58e04d..18316f492e 100644
--- a/depends/builders/freebsd.mk
+++ b/depends/builders/freebsd.mk
@@ -1,5 +1,5 @@
 build_freebsd_CC=clang
 build_freebsd_CXX=clang++
 
-build_freebsd_SHA256SUM = shasum -a 256
+build_freebsd_SHA256SUM = sha256sum
 build_freebsd_DOWNLOAD = curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o

From bb5f76ee013ee439f70e86319d22f308db8a24ea Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Thu, 9 Jan 2025 14:05:12 -0500
Subject: [PATCH 22/42] doc: Archive 28.1 release notes

---
 doc/release-notes/release-notes-28.1.md | 107 ++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 doc/release-notes/release-notes-28.1.md

diff --git a/doc/release-notes/release-notes-28.1.md b/doc/release-notes/release-notes-28.1.md
new file mode 100644
index 0000000000..de3b8c5a06
--- /dev/null
+++ b/doc/release-notes/release-notes-28.1.md
@@ -0,0 +1,107 @@
+Bitcoin Core version 28.1 is now available from:
+
+  <https://bitcoincore.org/bin/bitcoin-core-28.1>
+
+This release includes new features, various bug fixes and performance
+improvements, as well as updated translations.
+
+Please report bugs using the issue tracker at GitHub:
+
+  <https://github.com/bitcoin/bitcoin/issues>
+
+To receive security and update notifications, please subscribe to:
+
+  <https://bitcoincore.org/en/list/announcements/join/>
+
+How to Upgrade
+==============
+
+If you are running an older version, shut it down. Wait until it has completely
+shut down (which might take a few minutes in some cases), then run the
+installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS)
+or `bitcoind`/`bitcoin-qt` (on Linux).
+
+Upgrading directly from a version of Bitcoin Core that has reached its EOL is
+possible, but it might take some time if the data directory needs to be migrated. Old
+wallet versions of Bitcoin Core are generally supported.
+
+Running Bitcoin Core binaries on macOS requires self signing.
+```
+cd /path/to/bitcoin-28.x/bin
+xattr -d com.apple.quarantine bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin
+codesign -s - bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin
+```
+
+Compatibility
+==============
+
+Bitcoin Core is supported and extensively tested on operating systems
+using the Linux Kernel 3.17+, macOS 11.0+, and Windows 7 and newer. Bitcoin
+Core should also work on most other UNIX-like systems but is not as
+frequently tested on them. It is not recommended to use Bitcoin Core on
+unsupported systems.
+
+Notable changes
+===============
+
+### P2P
+
+- When the `-port` configuration option is used, the default onion listening port will now
+  be derived to be that port + 1 instead of being set to a fixed value (8334 on mainnet).
+  This re-allows setups with multiple local nodes using different `-port` and not using `-bind`,
+  which would lead to a startup failure in v28.0 due to a port collision.
+
+  Note that a `HiddenServicePort` manually configured in `torrc` may need adjustment if used in
+  connection with the `-port` option.
+  For example, if you are using `-port=5555` with a non-standard value and not using `-bind=...=onion`,
+  previously Bitcoin Core would listen for incoming Tor connections on `127.0.0.1:8334`.
+  Now it would listen on `127.0.0.1:5556` (`-port` plus one). If you configured the hidden service manually
+  in torrc now you have to change it from `HiddenServicePort 8333 127.0.0.1:8334` to `HiddenServicePort 8333
+  127.0.0.1:5556`, or configure bitcoind with `-bind=127.0.0.1:8334=onion` to get the previous behavior.
+  (#31223)
+- #30568 addrman: change internal id counting to int64_t
+
+### Key
+
+- #31166 key: clear out secret data in DecodeExtKey
+
+### Build
+
+- #31013 depends: For mingw cross compile use `-gcc-posix` to prevent library conflict
+- #31502 depends: Fix CXXFLAGS on NetBSD
+
+### Test
+
+- #31016 test: add missing sync to feature_fee_estimation.py
+- #31448 fuzz: add cstdlib to FuzzedDataProvider
+- #31419 test: fix MIN macro redefinition
+- #31563 rpc: Extend scope of validation mutex in generateblock
+
+### Doc
+
+- #31007 doc: add testnet4 section header for config file
+
+### CI
+
+- #30961 ci: add LLVM_SYMBOLIZER_PATH to Valgrind fuzz job
+
+### Misc
+
+- #31267 refactor: Drop deprecated space in `operator""_mst`
+- #31431 util: use explicit cast in MultiIntBitSet::Fill()
+
+Credits
+=======
+
+Thanks to everyone who directly contributed to this release:
+
+- fanquake
+- Hennadii Stepanov
+- laanwj
+- MarcoFalke
+- Martin Zumsande
+- Marnix
+- Sebastian Falbesoner
+
+As well as to everyone that helped with translations on
+[Transifex](https://www.transifex.com/bitcoin/bitcoin/).

From fabefd9915802d53d8c83ae66e5cb259196d9422 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Mon, 13 Jan 2025 23:37:31 +0100
Subject: [PATCH 23/42] ci: Turn CentOS task into native one

---
 .cirrus.yml                                               | 4 ++--
 ...p_env_i686_centos.sh => 00_setup_env_native_centos.sh} | 8 +++-----
 2 files changed, 5 insertions(+), 7 deletions(-)
 rename ci/test/{00_setup_env_i686_centos.sh => 00_setup_env_native_centos.sh} (53%)

diff --git a/.cirrus.yml b/.cirrus.yml
index 2a3cb1c6d9..b3ac31881e 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -140,13 +140,13 @@ task:
     FILE_ENV: "./ci/test/00_setup_env_win64.sh"
 
 task:
-  name: '32-bit CentOS, dash, gui'
+  name: 'CentOS, dash, gui'
   << : *GLOBAL_TASK_TEMPLATE
   persistent_worker:
     labels:
       type: small
   env:
-    FILE_ENV: "./ci/test/00_setup_env_i686_centos.sh"
+    FILE_ENV: "./ci/test/00_setup_env_native_centos.sh"
 
 task:
   name: 'previous releases, depends DEBUG'
diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_native_centos.sh
similarity index 53%
rename from ci/test/00_setup_env_i686_centos.sh
rename to ci/test/00_setup_env_native_centos.sh
index 22657742d7..878778506f 100755
--- a/ci/test/00_setup_env_i686_centos.sh
+++ b/ci/test/00_setup_env_native_centos.sh
@@ -6,12 +6,10 @@
 
 export LC_ALL=C.UTF-8
 
-export HOST=i686-pc-linux-gnu
-export CONTAINER_NAME=ci_i686_centos
-export CI_IMAGE_NAME_TAG="quay.io/centos/amd64:stream9"
+export CONTAINER_NAME=ci_native_centos
+export CI_IMAGE_NAME_TAG="quay.io/centos/centos:stream9"
 export STREAM_GCC_V="12"
-export CI_BASE_PACKAGES="gcc-toolset-${STREAM_GCC_V}-gcc-c++ glibc-devel.x86_64 gcc-toolset-${STREAM_GCC_V}-libstdc++-devel.x86_64 glibc-devel.i686 gcc-toolset-${STREAM_GCC_V}-libstdc++-devel.i686 ccache make git python3 python3-pip which patch lbzip2 xz procps-ng dash rsync coreutils bison e2fsprogs cmake"
+export CI_BASE_PACKAGES="gcc-toolset-${STREAM_GCC_V}-gcc-c++ glibc-devel gcc-toolset-${STREAM_GCC_V}-libstdc++-devel ccache make git python3 python3-pip which patch xz procps-ng dash rsync coreutils bison e2fsprogs cmake"
 export PIP_PACKAGES="pyzmq"
 export GOAL="install"
 export BITCOIN_CONFIG="-DWITH_ZMQ=ON -DBUILD_GUI=ON -DREDUCE_EXPORTS=ON"
-export CONFIG_SHELL="/bin/dash"

From 6b3f6eae70bbf61a8a57764d60b1c34c9fd227d6 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Mon, 25 Nov 2024 15:36:38 +0100
Subject: [PATCH 24/42] test: avoid generating non-loopback traffic from
 p2p_seednode.py

`p2p_seednode.py` would try to connect to `0.0.0.1` and `0.0.0.2` as
seed nodes. This sends outbound TCP packets on a non-loopback interface
to the default router.

Configure an unavailable proxy for all executions of `bitcoind` during
this test. Also change `0.0.0.1` and `0.0.0.2` because connecting to
them would skip the `-proxy=` setting because for such an address:
* `CNetAddr::IsLocal()` is true, thus
* `CNetAddr::IsRoutable()` is false, thus
* `CNetAddr::GetNetwork()` is `NET_UNROUTABLE`, even though
  `CNetAddr::m_net` is `NET_IPV4`.

This speeds up the execution time of `p2p_seednode.py`
from 12.5s to 2.5s.
---
 test/functional/p2p_seednode.py           | 15 +++++++++------
 test/functional/test_framework/netutil.py |  4 ++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/test/functional/p2p_seednode.py b/test/functional/p2p_seednode.py
index 5a9fca3278..ceefe1aa9b 100755
--- a/test/functional/p2p_seednode.py
+++ b/test/functional/p2p_seednode.py
@@ -9,6 +9,7 @@
 import random
 import time
 
+from test_framework.netutil import UNREACHABLE_PROXY_ARG
 from test_framework.test_framework import BitcoinTestFramework
 
 ADD_NEXT_SEEDNODE = 10
@@ -17,22 +18,24 @@
 class P2PSeedNodes(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
+        # Specify a non-working proxy to make sure no actual connections to random IPs are attempted.
+        self.extra_args = [[UNREACHABLE_PROXY_ARG]]
         self.disable_autoconnect = False
 
     def test_no_seednode(self):
         self.log.info("Check that if no seednode is provided, the node proceeds as usual (without waiting)")
         with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["Empty addrman, adding seednode", f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode"], timeout=ADD_NEXT_SEEDNODE):
-            self.restart_node(0)
+            self.restart_node(0, extra_args=self.nodes[0].extra_args)
 
     def test_seednode_empty_addrman(self):
-        seed_node = "0.0.0.1"
+        seed_node = "25.0.0.1"
         self.log.info("Check that the seednode is immediately added on bootstrap on an empty addrman")
         with self.nodes[0].assert_debug_log(expected_msgs=[f"Empty addrman, adding seednode ({seed_node}) to addrfetch"], timeout=ADD_NEXT_SEEDNODE):
-            self.restart_node(0, extra_args=[f'-seednode={seed_node}'])
+            self.restart_node(0, extra_args=self.nodes[0].extra_args + [f'-seednode={seed_node}'])
 
     def test_seednode_non_empty_addrman(self):
         self.log.info("Check that if addrman is non-empty, seednodes are queried with a delay")
-        seed_node = "0.0.0.2"
+        seed_node = "25.0.0.2"
         node = self.nodes[0]
         # Fill the addrman with unreachable nodes
         for i in range(10):
@@ -40,9 +43,9 @@ def test_seednode_non_empty_addrman(self):
             port = 8333 + i
             node.addpeeraddress(ip, port)
 
-        # Restart the node so seednode is processed again. Specify a non-working proxy to make sure no actual connections to random IPs are attempted.
+        # Restart the node so seednode is processed again.
         with node.assert_debug_log(expected_msgs=["trying v1 connection"], timeout=ADD_NEXT_SEEDNODE):
-            self.restart_node(0, extra_args=[f'-seednode={seed_node}', '-proxy=127.0.0.1:1'])
+            self.restart_node(0, extra_args=self.nodes[0].extra_args + [f'-seednode={seed_node}'])
 
         with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5):
             node.setmocktime(int(time.time()) + ADD_NEXT_SEEDNODE + 1)
diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py
index f6acf926fa..6fa8a8156d 100644
--- a/test/functional/test_framework/netutil.py
+++ b/test/functional/test_framework/netutil.py
@@ -13,6 +13,10 @@
 import array
 import os
 
+# Easily unreachable address. Attempts to connect to it will stay within the machine.
+# Used to avoid non-loopback traffic or DNS queries.
+UNREACHABLE_PROXY_ARG = '-proxy=127.0.0.1:1'
+
 # STATE_ESTABLISHED = '01'
 # STATE_SYN_SENT  = '02'
 # STATE_SYN_RECV = '03'

From a5746dc559c2a0909018be6ca2f3869e237a49b7 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Mon, 25 Nov 2024 17:01:17 +0100
Subject: [PATCH 25/42] test: avoid generating non-loopback traffic from
 feature_config_args.py

`feature_config_args.py` uses a proxy address of `1.2.3.4`. This results
in actually trying to open TCP connections over the internet to
`1.2.3.4:9050`.

The test does not need those to succeed so use `127.0.0.1:1` instead.

Also avoid `-noconnect=0` because that is interpreted as `-connect=1`
which is interpreted as `-connect=0.0.0.1` and a connection to
`0.0.0.1:18444` is attempted.
---
 test/functional/feature_config_args.py | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index 31847b3fbf..b2462be951 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -11,6 +11,7 @@
 import tempfile
 import time
 
+from test_framework.netutil import UNREACHABLE_PROXY_ARG
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
 from test_framework import util
@@ -227,8 +228,8 @@ def test_invalid_command_line_options(self):
                 )
 
     def test_log_buffer(self):
-        with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -connect=0\n']):
-            self.start_node(0, extra_args=['-noconnect=0'])
+        with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -listen=0\n']):
+            self.start_node(0, extra_args=['-nolisten=0'])
         self.stop_node(0)
 
     def test_args_log(self):
@@ -259,6 +260,7 @@ def test_args_log(self):
                 '-rpcpassword=',
                 '-rpcuser=secret-rpcuser',
                 '-torpassword=secret-torpassword',
+                UNREACHABLE_PROXY_ARG,
             ])
         self.stop_node(0)
 
@@ -307,7 +309,7 @@ def test_seed_peers(self):
                 ],
                 timeout=10,
         ):
-            self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}'])
+            self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}', UNREACHABLE_PROXY_ARG])
 
         # Only regtest has no fixed seeds. To avoid connections to random
         # nodes, regtest is the only network where it is safe to enable
@@ -355,7 +357,7 @@ def test_seed_peers(self):
                 ],
                 timeout=10,
         ):
-            self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1', '-addnode=fakenodeaddr', f'-mocktime={start}'])
+            self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1', '-addnode=fakenodeaddr', f'-mocktime={start}', UNREACHABLE_PROXY_ARG])
         with self.nodes[0].assert_debug_log(expected_msgs=[
                 "Adding fixed seeds as 60 seconds have passed and addrman is empty",
         ]):
@@ -371,18 +373,18 @@ def test_connect_with_seednode(self):
         # When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode)
         # nodes is irrelevant and -seednode is ignored.
         with self.nodes[0].assert_debug_log(expected_msgs=seednode_ignored):
-            self.start_node(0, extra_args=['-connect=fakeaddress1', '-seednode=fakeaddress2'])
+            self.start_node(0, extra_args=['-connect=fakeaddress1', '-seednode=fakeaddress2', UNREACHABLE_PROXY_ARG])
 
         # With -proxy, an ADDR_FETCH connection is made to a peer that the dns seed resolves to.
         # ADDR_FETCH connections are not used when -connect is used.
         with self.nodes[0].assert_debug_log(expected_msgs=dnsseed_ignored):
-            self.restart_node(0, extra_args=['-connect=fakeaddress1', '-dnsseed=1', '-proxy=1.2.3.4'])
+            self.restart_node(0, extra_args=['-connect=fakeaddress1', '-dnsseed=1', UNREACHABLE_PROXY_ARG])
 
         # If the user did not disable -dnsseed, but it was soft-disabled because they provided -connect,
         # they shouldn't see a warning about -dnsseed being ignored.
         with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started,
                 unexpected_msgs=dnsseed_ignored):
-            self.restart_node(0, extra_args=['-connect=fakeaddress1', '-proxy=1.2.3.4'])
+            self.restart_node(0, extra_args=['-connect=fakeaddress1', UNREACHABLE_PROXY_ARG])
 
         # We have to supply expected_msgs as it's a required argument
         # The expected_msg must be something we are confident will be logged after the unexpected_msg

From 2ed161c5ce648cb66ec3d2941b02d68b6ca4c390 Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Tue, 26 Nov 2024 15:14:41 +0100
Subject: [PATCH 26/42] test: avoid generating non-loopback traffic from
 p2p_dns_seeds.py

`p2p_dns_seeds.py` would try to connect to the DNS server configured on
the machine and resolve `dummySeed.invalid`.

To block that configure an unavailable proxy which will be used also to
connect to the name server. The test needs 2 successful connections to
other peers (two Python `P2PInterface`s) and they work in spite of the
unavailable proxy because they are on `127.0.0.1` (`NET_UNROUTABLE`) and
the proxy is not used for that.
---
 test/functional/p2p_dns_seeds.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/test/functional/p2p_dns_seeds.py b/test/functional/p2p_dns_seeds.py
index a2d4ea110f..89a29c0ebd 100755
--- a/test/functional/p2p_dns_seeds.py
+++ b/test/functional/p2p_dns_seeds.py
@@ -6,6 +6,7 @@
 
 import itertools
 
+from test_framework.netutil import UNREACHABLE_PROXY_ARG
 from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 
@@ -14,7 +15,7 @@ class P2PDNSSeeds(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
-        self.extra_args = [["-dnsseed=1"]]
+        self.extra_args = [["-dnsseed=1", UNREACHABLE_PROXY_ARG]]
 
     def run_test(self):
         self.init_arg_tests()
@@ -29,11 +30,11 @@ def init_arg_tests(self):
         self.log.info("Check that setting -connect disables -dnsseed by default")
         self.nodes[0].stop_node()
         with self.nodes[0].assert_debug_log(expected_msgs=["DNS seeding disabled"]):
-            self.start_node(0, [f"-connect={fakeaddr}"])
+            self.start_node(0, extra_args=[f"-connect={fakeaddr}", UNREACHABLE_PROXY_ARG])
 
         self.log.info("Check that running -connect and -dnsseed means DNS logic runs.")
         with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
-            self.restart_node(0, [f"-connect={fakeaddr}", "-dnsseed=1"])
+            self.restart_node(0, extra_args=[f"-connect={fakeaddr}", "-dnsseed=1", UNREACHABLE_PROXY_ARG])
 
         self.log.info("Check that running -forcednsseed and -dnsseed=0 throws an error.")
         self.nodes[0].stop_node()
@@ -88,7 +89,7 @@ def force_dns_test(self):
         with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
             # -dnsseed defaults to 1 in bitcoind, but 0 in the test framework,
             # so pass it explicitly here
-            self.restart_node(0, ["-forcednsseed", "-dnsseed=1"])
+            self.restart_node(0, ["-forcednsseed", "-dnsseed=1", UNREACHABLE_PROXY_ARG])
 
         # Restore default for subsequent tests
         self.restart_node(0)

From fad4032b219e58300f8d0a74bbcf38ef26fa89d0 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 18 Dec 2024 11:02:21 +0100
Subject: [PATCH 27/42] refactor: Drop unused UCharCast

This is no longer needed after commit
6aa0e70ccbd5491ec9d7c81892820f3341ccd631
---
 src/crypto/chacha20.cpp         | 6 +++---
 src/crypto/chacha20poly1305.cpp | 4 ++--
 src/random.h                    | 6 +++---
 src/wallet/migrate.cpp          | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/crypto/chacha20.cpp b/src/crypto/chacha20.cpp
index e756e0431e..b1bd3c5490 100644
--- a/src/crypto/chacha20.cpp
+++ b/src/crypto/chacha20.cpp
@@ -59,7 +59,7 @@ void ChaCha20Aligned::Seek(Nonce96 nonce, uint32_t block_counter) noexcept
 
 inline void ChaCha20Aligned::Keystream(Span<std::byte> output) noexcept
 {
-    unsigned char* c = UCharCast(output.data());
+    std::byte* c = output.data();
     size_t blocks = output.size() / BLOCKLEN;
     assert(blocks * BLOCKLEN == output.size());
 
@@ -161,8 +161,8 @@ inline void ChaCha20Aligned::Keystream(Span<std::byte> output) noexcept
 inline void ChaCha20Aligned::Crypt(Span<const std::byte> in_bytes, Span<std::byte> out_bytes) noexcept
 {
     assert(in_bytes.size() == out_bytes.size());
-    const unsigned char* m = UCharCast(in_bytes.data());
-    unsigned char* c = UCharCast(out_bytes.data());
+    const std::byte* m = in_bytes.data();
+    std::byte* c = out_bytes.data();
     size_t blocks = out_bytes.size() / BLOCKLEN;
     assert(blocks * BLOCKLEN == out_bytes.size());
 
diff --git a/src/crypto/chacha20poly1305.cpp b/src/crypto/chacha20poly1305.cpp
index b969bb1a29..282f32a5e4 100644
--- a/src/crypto/chacha20poly1305.cpp
+++ b/src/crypto/chacha20poly1305.cpp
@@ -56,8 +56,8 @@ void ComputeTag(ChaCha20& chacha20, Span<const std::byte> aad, Span<const std::b
     poly1305.Update(cipher).Update(Span{PADDING}.first(cipher_padding_length));
     // - Process the AAD and plaintext length with Poly1305.
     std::byte length_desc[Poly1305::TAGLEN];
-    WriteLE64(UCharCast(length_desc), aad.size());
-    WriteLE64(UCharCast(length_desc + 8), cipher.size());
+    WriteLE64(length_desc, aad.size());
+    WriteLE64(length_desc + 8, cipher.size());
     poly1305.Update(length_desc);
 
     // Output tag.
diff --git a/src/random.h b/src/random.h
index 2a26ca986b..203678b17c 100644
--- a/src/random.h
+++ b/src/random.h
@@ -268,12 +268,12 @@ class RandomMixin
     {
         while (span.size() >= 8) {
             uint64_t gen = Impl().rand64();
-            WriteLE64(UCharCast(span.data()), gen);
+            WriteLE64(span.data(), gen);
             span = span.subspan(8);
         }
         if (span.size() >= 4) {
             uint32_t gen = Impl().rand32();
-            WriteLE32(UCharCast(span.data()), gen);
+            WriteLE32(span.data(), gen);
             span = span.subspan(4);
         }
         while (span.size()) {
@@ -397,7 +397,7 @@ class FastRandomContext : public RandomMixin<FastRandomContext>
         if (requires_seed) RandomSeed();
         std::array<std::byte, 8> buf;
         rng.Keystream(buf);
-        return ReadLE64(UCharCast(buf.data()));
+        return ReadLE64(buf.data());
     }
 
     /** Fill a byte Span with random bytes. This overrides the RandomMixin version. */
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index d7d8577374..d2c163027d 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -603,7 +603,7 @@ void BerkeleyRODatabase::Open()
 
     // Read subdatabase page number
     // It is written as a big endian 32 bit number
-    uint32_t main_db_page = ReadBE32(UCharCast(std::get<DataRecord>(page.records.at(1)).data.data()));
+    uint32_t main_db_page = ReadBE32(std::get<DataRecord>(page.records.at(1)).data.data());
 
     // The main database is in a page that doesn't exist
     if (main_db_page > outer_meta.last_page) {

From fabeca3458b38a3d8930cb0cbc866388c3f120f1 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Tue, 14 Jan 2025 19:11:45 +0100
Subject: [PATCH 28/42] refactor: Avoid UB in SHA3_256::Write

It is UB to apply a distance to a pointer or iterator further than the
end itself, even if the distance is (partially) revoked later on.

Fix the issue by advancing the data pointer at most to the end.
---
 src/crypto/sha3.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/crypto/sha3.cpp b/src/crypto/sha3.cpp
index 770500bfe2..56aaa4615e 100644
--- a/src/crypto/sha3.cpp
+++ b/src/crypto/sha3.cpp
@@ -105,9 +105,9 @@ void KeccakF(uint64_t (&st)[25])
 
 SHA3_256& SHA3_256::Write(Span<const unsigned char> data)
 {
-    if (m_bufsize && m_bufsize + data.size() >= sizeof(m_buffer)) {
+    if (m_bufsize && data.size() >= sizeof(m_buffer) - m_bufsize) {
         // Fill the buffer and process it.
-        std::copy(data.begin(), data.begin() + sizeof(m_buffer) - m_bufsize, m_buffer + m_bufsize);
+        std::copy(data.begin(), data.begin() + (sizeof(m_buffer) - m_bufsize), m_buffer + m_bufsize);
         data = data.subspan(sizeof(m_buffer) - m_bufsize);
         m_state[m_pos++] ^= ReadLE64(m_buffer);
         m_bufsize = 0;

From eeee6cf2ffb24d930fc2fe77912788abf685cd4b Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 9 Oct 2024 13:23:28 +0200
Subject: [PATCH 29/42] refactor: Delay translation of _() literals

This is required for a future commit that requires _() to be consteval
for format literals.

Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
---
 src/banman.cpp            |  2 +-
 src/clientversion.cpp     |  2 +-
 src/init.cpp              | 12 ++++++------
 src/net.cpp               |  2 +-
 src/util/translation.h    | 41 ++++++++++++++++++++++++---------------
 src/wallet/load.cpp       |  4 ++--
 src/wallet/rpc/backup.cpp |  2 +-
 src/wallet/wallet.cpp     | 12 ++++++------
 8 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/src/banman.cpp b/src/banman.cpp
index 2dbfc76df2..2964a37de0 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -30,7 +30,7 @@ void BanMan::LoadBanlist()
 {
     LOCK(m_banned_mutex);
 
-    if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated);
+    if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…"));
 
     const auto start{SteadyClock::now()};
     if (m_ban_db.Read(m_banned)) {
diff --git a/src/clientversion.cpp b/src/clientversion.cpp
index af58c3023f..10e9472b84 100644
--- a/src/clientversion.cpp
+++ b/src/clientversion.cpp
@@ -95,7 +95,7 @@ std::string LicenseInfo()
            strprintf(_("The source code is available from %s."), URL_SOURCE_CODE).translated +
            "\n" +
            "\n" +
-           _("This is experimental software.").translated + "\n" +
+           _("This is experimental software.") + "\n" +
            strprintf(_("Distributed under the MIT software license, see the accompanying file %s or %s"), "COPYING", "<https://opensource.org/licenses/MIT>").translated +
            "\n";
 }
diff --git a/src/init.cpp b/src/init.cpp
index cb4ff733a5..9887c07165 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1244,8 +1244,8 @@ static ChainstateLoadResult InitAndLoadChainstate(
             _("Error reading from database, shutting down."),
             "", CClientUIInterface::MSG_ERROR);
     };
-    uiInterface.InitMessage(_("Loading block index…").translated);
-    auto catch_exceptions = [](auto&& f) {
+    uiInterface.InitMessage(_("Loading block index…"));
+    auto catch_exceptions = [](auto&& f) -> ChainstateLoadResult {
         try {
             return f();
         } catch (const std::exception& e) {
@@ -1255,7 +1255,7 @@ static ChainstateLoadResult InitAndLoadChainstate(
     };
     auto [status, error] = catch_exceptions([&] { return LoadChainstate(chainman, cache_sizes, options); });
     if (status == node::ChainstateLoadStatus::SUCCESS) {
-        uiInterface.InitMessage(_("Verifying blocks…").translated);
+        uiInterface.InitMessage(_("Verifying blocks…"));
         if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
             LogWarning("pruned datadir may not have more than %d blocks; only checking available blocks\n",
                        MIN_BLOCKS_TO_KEEP);
@@ -1418,7 +1418,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
         // Initialize addrman
         assert(!node.addrman);
-        uiInterface.InitMessage(_("Loading P2P addresses…").translated);
+        uiInterface.InitMessage(_("Loading P2P addresses…"));
         auto addrman{LoadAddrman(*node.netgroupman, args)};
         if (!addrman) return InitError(util::ErrorString(addrman));
         node.addrman = std::move(*addrman);
@@ -1703,7 +1703,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         if (chainman.m_blockman.m_blockfiles_indexed) {
             LOCK(cs_main);
             for (Chainstate* chainstate : chainman.GetAll()) {
-                uiInterface.InitMessage(_("Pruning blockstore…").translated);
+                uiInterface.InitMessage(_("Pruning blockstore…"));
                 chainstate->PruneAndFlush();
             }
         }
@@ -1999,7 +1999,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // ChainstateManager's active tip.
     SetRPCWarmupFinished();
 
-    uiInterface.InitMessage(_("Done loading").translated);
+    uiInterface.InitMessage(_("Done loading"));
 
     for (const auto& client : node.chain_clients) {
         client->start(scheduler);
diff --git a/src/net.cpp b/src/net.cpp
index 8ea7f6ce44..47ef2d22d7 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3296,7 +3296,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
     }
 
     if (m_client_interface) {
-        m_client_interface->InitMessage(_("Starting network threads…").translated);
+        m_client_interface->InitMessage(_("Starting network threads…"));
     }
 
     fAddressesInitialized = true;
diff --git a/src/util/translation.h b/src/util/translation.h
index 7c734a1766..c58f7c2661 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -6,12 +6,15 @@
 #define BITCOIN_UTIL_TRANSLATION_H
 
 #include <tinyformat.h>
+#include <util/string.h>
 
+#include <cassert>
 #include <functional>
 #include <string>
 
 /** Translate a message to the native language of the user. */
-const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;
+using TranslateFn = std::function<std::string(const char*)>;
+const extern TranslateFn G_TRANSLATION_FUN;
 
 /**
  * Bilingual messages:
@@ -47,6 +50,27 @@ inline bilingual_str operator+(bilingual_str lhs, const bilingual_str& rhs)
     return lhs;
 }
 
+namespace util {
+//! Compile-time literal string that can be translated with an optional translation function.
+struct TranslatedLiteral {
+    const char* const original;
+    const TranslateFn* translate_fn;
+
+    consteval TranslatedLiteral(const char* str, const TranslateFn* fn = &G_TRANSLATION_FUN) : original{str}, translate_fn{fn} { assert(original); }
+    operator std::string() const { return translate_fn && *translate_fn ? (*translate_fn)(original) : original; }
+    operator bilingual_str() const { return {original, std::string{*this}}; }
+};
+
+// TranslatedLiteral operators for formatting and adding to strings.
+inline std::ostream& operator<<(std::ostream& os, const TranslatedLiteral& lit) { return os << std::string{lit}; }
+template<typename T>
+T operator+(const T& lhs, const TranslatedLiteral& rhs) { return lhs + static_cast<T>(rhs); }
+template<typename T>
+T operator+(const TranslatedLiteral& lhs, const T& rhs) { return static_cast<T>(lhs) + rhs; }
+} // namespace util
+
+consteval auto _(util::TranslatedLiteral str) { return str; }
+
 /** Mark a bilingual_str as untranslated */
 inline bilingual_str Untranslated(std::string original) { return {original, original}; }
 
@@ -67,19 +91,4 @@ bilingual_str format(const bilingual_str& fmt, const Args&... args)
 }
 } // namespace tinyformat
 
-struct ConstevalStringLiteral {
-    const char* const lit;
-    consteval ConstevalStringLiteral(const char* str) : lit{str} {}
-    consteval ConstevalStringLiteral(std::nullptr_t) = delete;
-};
-
-/**
- * Translation function.
- * If no translation function is set, simply return the input.
- */
-inline bilingual_str _(ConstevalStringLiteral str)
-{
-    return bilingual_str{str.lit, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(str.lit) : str.lit};
-}
-
 #endif // BITCOIN_UTIL_TRANSLATION_H
diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
index 2b5c021cda..e77999b111 100644
--- a/src/wallet/load.cpp
+++ b/src/wallet/load.cpp
@@ -52,7 +52,7 @@ bool VerifyWallets(WalletContext& context)
 
     LogPrintf("Using wallet directory %s\n", fs::PathToString(GetWalletDir()));
 
-    chain.initMessage(_("Verifying wallet(s)…").translated);
+    chain.initMessage(_("Verifying wallet(s)…"));
 
     // For backwards compatibility if an unnamed top level wallet exists in the
     // wallets directory, include it in the default list of wallets to load.
@@ -135,7 +135,7 @@ bool LoadWallets(WalletContext& context)
             if (!database && status == DatabaseStatus::FAILED_NOT_FOUND) {
                 continue;
             }
-            chain.initMessage(_("Loading wallet…").translated);
+            chain.initMessage(_("Loading wallet…"));
             std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings) : nullptr;
             if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
             if (!pwallet) {
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index bfd7249c04..79f1a40ec5 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -534,7 +534,7 @@ RPCHelpMan importwallet()
 
         // Use uiInterface.ShowProgress instead of pwallet.ShowProgress because pwallet.ShowProgress has a cancel button tied to AbortRescan which
         // we don't want for this progress bar showing the import progress. uiInterface.ShowProgress does not have a cancel button.
-        pwallet->chain().showProgress(strprintf("%s %s", pwallet->GetDisplayName(), _("Importing…").translated), 0, false); // show progress dialog in GUI
+        pwallet->chain().showProgress(strprintf("%s %s", pwallet->GetDisplayName(), _("Importing…")), 0, false); // show progress dialog in GUI
         std::vector<std::tuple<CKey, int64_t, bool, std::string>> keys;
         std::vector<std::pair<CScript, int64_t>> scripts;
         while (file.good()) {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index e7e06aec01..ea59f1a9d5 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -280,7 +280,7 @@ std::shared_ptr<CWallet> LoadWalletInternal(WalletContext& context, const std::s
             return nullptr;
         }
 
-        context.chain->initMessage(_("Loading wallet…").translated);
+        context.chain->initMessage(_("Loading wallet…"));
         std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings);
         if (!wallet) {
             error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error;
@@ -430,7 +430,7 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string&
     }
 
     // Make the wallet
-    context.chain->initMessage(_("Loading wallet…").translated);
+    context.chain->initMessage(_("Loading wallet…"));
     std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings);
     if (!wallet) {
         error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error;
@@ -1903,7 +1903,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
                     fast_rescan_filter ? "fast variant using block filters" : "slow variant inspecting all blocks");
 
     fAbortRescan = false;
-    ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption)
+    ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption)
     uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash());
     uint256 end_hash = tip_hash;
     if (max_height) chain().findAncestorByHeight(tip_hash, *max_height, FoundBlock().hash(end_hash));
@@ -1918,7 +1918,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
             m_scanning_progress = 0;
         }
         if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) {
-            ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
+            ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
         }
 
         bool next_interval = reserver.now() >= current_time + INTERVAL_TIME;
@@ -2015,7 +2015,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
         WalletLogPrintf("Scanning current mempool transactions.\n");
         WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this));
     }
-    ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), 100); // hide progress dialog in GUI
+    ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…")), 100); // hide progress dialog in GUI
     if (block_height && fAbortRescan) {
         WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current);
         result.status = ScanResult::USER_ABORT;
@@ -3353,7 +3353,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
             }
         }
 
-        chain.initMessage(_("Rescanning…").translated);
+        chain.initMessage(_("Rescanning…"));
         walletInstance->WalletLogPrintf("Rescanning last %i blocks (from block %i)...\n", *tip_height - rescan_height, rescan_height);
 
         {

From fa1d5acb8d8e1842797604fbd1e7c69f6acdb6c7 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 15 Jan 2025 12:15:40 +0100
Subject: [PATCH 30/42] refactor: Use TranslateFn type consistently

The type was introduced in the previous commit.
---
 src/bitcoin-cli.cpp            | 2 +-
 src/bitcoin-tx.cpp             | 2 +-
 src/bitcoin-util.cpp           | 2 +-
 src/bitcoin-wallet.cpp         | 2 +-
 src/bitcoind.cpp               | 2 +-
 src/kernel/bitcoinkernel.cpp   | 3 ++-
 src/qt/main.cpp                | 2 +-
 src/test/util/setup_common.cpp | 2 +-
 8 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 5c5965245b..1b2d97faff 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -50,7 +50,7 @@ using util::ToString;
 // just use a plain system_clock.
 using CliClock = std::chrono::system_clock;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 static const char DEFAULT_RPCCONNECT[] = "127.0.0.1";
 static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900;
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index a627c24e86..e2b3a3b554 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -41,7 +41,7 @@ static bool fCreateBlank;
 static std::map<std::string,UniValue> registers;
 static const int CONTINUE_EXECUTION=-1;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 static void SetupBitcoinTxArgs(ArgsManager &argsman)
 {
diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp
index ff2e4fb800..10967bd573 100644
--- a/src/bitcoin-util.cpp
+++ b/src/bitcoin-util.cpp
@@ -26,7 +26,7 @@
 
 static const int CONTINUE_EXECUTION=-1;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 static void SetupBitcoinUtilArgs(ArgsManager &argsman)
 {
diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp
index 033cf7a0f3..c37f9d2995 100644
--- a/src/bitcoin-wallet.cpp
+++ b/src/bitcoin-wallet.cpp
@@ -26,7 +26,7 @@
 
 using util::Join;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 static void SetupWalletToolArgs(ArgsManager& argsman)
 {
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index 5d5f06127d..d210e2c8ba 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -34,7 +34,7 @@
 
 using node::NodeContext;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 #if HAVE_DECL_FORK
 
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index bb101ba186..9096b6f71d 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -1,10 +1,11 @@
 // Copyright (c) 2022 The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#include <util/translation.h>
 
 #include <functional>
 #include <string>
 
 // Define G_TRANSLATION_FUN symbol in libbitcoinkernel library so users of the
 // library aren't required to export this symbol
-extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+extern const TranslateFn G_TRANSLATION_FUN{nullptr};
diff --git a/src/qt/main.cpp b/src/qt/main.cpp
index 16befd99e8..649fead901 100644
--- a/src/qt/main.cpp
+++ b/src/qt/main.cpp
@@ -13,7 +13,7 @@
 #include <string>
 
 /** Translate string to current locale using Qt. */
-extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = [](const char* psz) {
+extern const TranslateFn G_TRANSLATION_FUN = [](const char* psz) {
     return QCoreApplication::translate("bitcoin-core", psz).toStdString();
 };
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 1ebc3464d8..c66e000306 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -72,7 +72,7 @@ using node::LoadChainstate;
 using node::RegenerateCommitments;
 using node::VerifyLoadedChainstate;
 
-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+const TranslateFn G_TRANSLATION_FUN{nullptr};
 
 constexpr inline auto TEST_DIR_PATH_ELEMENT{"test_common bitcoin"}; // Includes a space to catch possible path escape issues.
 /** Random context to get unique temp data dirs. Separate from m_rng, which can be seeded from a const env var */

From fadc6b9bac82e99a422d72935a74f3a5444274ed Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 9 Oct 2024 13:24:51 +0200
Subject: [PATCH 31/42] refactor: Check translatable format strings at
 compile-time

---
 src/test/fuzz/strprintf.cpp    | 15 ---------------
 src/test/translation_tests.cpp | 19 ++++++++++++++++---
 src/util/translation.h         | 28 ++++++++++++++++++++++------
 3 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/src/test/fuzz/strprintf.cpp b/src/test/fuzz/strprintf.cpp
index 18de0e1960..be40e88cdd 100644
--- a/src/test/fuzz/strprintf.cpp
+++ b/src/test/fuzz/strprintf.cpp
@@ -18,7 +18,6 @@ FUZZ_TARGET(str_printf)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const std::string format_string = fuzzed_data_provider.ConsumeRandomLengthString(64);
-    const bilingual_str bilingual_string{format_string, format_string};
 
     const int digits_in_format_specifier = std::count_if(format_string.begin(), format_string.end(), IsDigit);
 
@@ -53,27 +52,21 @@ FUZZ_TARGET(str_printf)
             fuzzed_data_provider,
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<char>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeBool());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeBool());
             });
     } catch (const tinyformat::format_error&) {
     }
@@ -99,35 +92,27 @@ FUZZ_TARGET(str_printf)
             fuzzed_data_provider,
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
             },
             [&] {
                 (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
-                (void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
             });
     } catch (const tinyformat::format_error&) {
     }
diff --git a/src/test/translation_tests.cpp b/src/test/translation_tests.cpp
index bda5dfd099..4935830cdc 100644
--- a/src/test/translation_tests.cpp
+++ b/src/test/translation_tests.cpp
@@ -9,13 +9,26 @@
 
 BOOST_AUTO_TEST_SUITE(translation_tests)
 
+static TranslateFn translate{[](const char * str) {  return strprintf("t(%s)", str);  }};
+
+// Custom translation function _t(), similar to _() but internal to this test.
+consteval auto _t(util::TranslatedLiteral str)
+{
+    str.translate_fn = &translate;
+    return str;
+}
+
 BOOST_AUTO_TEST_CASE(translation_namedparams)
 {
     bilingual_str arg{"original", "translated"};
-    bilingual_str format{"original [%s]", "translated [%s]"};
-    bilingual_str result{strprintf(format, arg)};
+    bilingual_str result{strprintf(_t("original [%s]"), arg)};
     BOOST_CHECK_EQUAL(result.original, "original [original]");
-    BOOST_CHECK_EQUAL(result.translated, "translated [translated]");
+    BOOST_CHECK_EQUAL(result.translated, "t(original [translated])");
+
+    util::TranslatedLiteral arg2{"original", &translate};
+    bilingual_str result2{strprintf(_t("original [%s]"), arg2)};
+    BOOST_CHECK_EQUAL(result2.original, "original [original]");
+    BOOST_CHECK_EQUAL(result2.translated, "t(original [t(original)])");
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util/translation.h b/src/util/translation.h
index c58f7c2661..27747a16f2 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -67,6 +67,13 @@ template<typename T>
 T operator+(const T& lhs, const TranslatedLiteral& rhs) { return lhs + static_cast<T>(rhs); }
 template<typename T>
 T operator+(const TranslatedLiteral& lhs, const T& rhs) { return static_cast<T>(lhs) + rhs; }
+
+template <unsigned num_params>
+struct BilingualFmt {
+    const ConstevalFormatString<num_params> original;
+    TranslatedLiteral lit;
+    consteval BilingualFmt(TranslatedLiteral l) : original{l.original}, lit{l} {}
+};
 } // namespace util
 
 consteval auto _(util::TranslatedLiteral str) { return str; }
@@ -74,20 +81,29 @@ consteval auto _(util::TranslatedLiteral str) { return str; }
 /** Mark a bilingual_str as untranslated */
 inline bilingual_str Untranslated(std::string original) { return {original, original}; }
 
-// Provide an overload of tinyformat::format which can take bilingual_str arguments.
+// Provide an overload of tinyformat::format for BilingualFmt format strings and bilingual_str or TranslatedLiteral args.
 namespace tinyformat {
 template <typename... Args>
-bilingual_str format(const bilingual_str& fmt, const Args&... args)
+bilingual_str format(util::BilingualFmt<sizeof...(Args)> fmt, const Args&... args)
 {
-    const auto translate_arg{[](const auto& arg, bool translated) -> const auto& {
+    const auto original_arg{[](const auto& arg) -> const auto& {
+        if constexpr (std::is_same_v<decltype(arg), const bilingual_str&>) {
+            return arg.original;
+        } else if constexpr (std::is_same_v<decltype(arg), const util::TranslatedLiteral&>) {
+            return arg.original;
+        } else {
+            return arg;
+        }
+    }};
+    const auto translated_arg{[](const auto& arg) -> const auto& {
         if constexpr (std::is_same_v<decltype(arg), const bilingual_str&>) {
-            return translated ? arg.translated : arg.original;
+            return arg.translated;
         } else {
             return arg;
         }
     }};
-    return bilingual_str{tfm::format(fmt.original, translate_arg(args, false)...),
-                         tfm::format(fmt.translated, translate_arg(args, true)...)};
+    return bilingual_str{tfm::format(fmt.original, original_arg(args)...),
+                         tfm::format(std::string{fmt.lit}, translated_arg(args)...)};
 }
 } // namespace tinyformat
 

From fa6adb0134408d9a5cb623a41f057d3807e2b006 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Sat, 7 Dec 2024 12:24:14 +0100
Subject: [PATCH 32/42] lint: Remove unused and broken format string linter

The linter has many implementation bugs and missing features.

Also, it is completely redundant with FormatStringCheck, which
constructs from ConstevalFormatString or a runtime format string.
---
 test/lint/lint-format-strings.py     |  85 --------
 test/lint/run-lint-format-strings.py | 298 ---------------------------
 2 files changed, 383 deletions(-)
 delete mode 100755 test/lint/lint-format-strings.py
 delete mode 100755 test/lint/run-lint-format-strings.py

diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py
deleted file mode 100755
index 86a17fb0f8..0000000000
--- a/test/lint/lint-format-strings.py
+++ /dev/null
@@ -1,85 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (c) 2018-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-
-"""
-Lint format strings: This program checks that the number of arguments passed
-to a variadic format string function matches the number of format specifiers
-in the format string.
-"""
-
-import subprocess
-import re
-import sys
-
-FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [
-    'tfm::format,1',  # Assuming tfm::::format(std::ostream&, ...
-    'strprintf,0',
-]
-RUN_LINT_FILE = 'test/lint/run-lint-format-strings.py'
-
-def check_doctest():
-    command = [
-        sys.executable,
-        '-m',
-        'doctest',
-        RUN_LINT_FILE,
-    ]
-    try:
-        subprocess.run(command, check = True)
-    except subprocess.CalledProcessError:
-        sys.exit(1)
-
-def get_matching_files(function_name):
-    command = [
-        'git',
-        'grep',
-        '--full-name',
-        '-l',
-        function_name,
-        '--',
-        '*.c',
-        '*.cpp',
-        '*.h',
-    ]
-    try:
-        return subprocess.check_output(command, stderr = subprocess.STDOUT).decode('utf-8').splitlines()
-    except subprocess.CalledProcessError as e:
-        if e.returncode > 1: # return code is 1 when match is empty
-            print(e.output.decode('utf-8'), end='')
-            sys.exit(1)
-        return []
-
-def main():
-    exit_code = 0
-    check_doctest()
-    for s in FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS:
-        function_name, skip_arguments = s.split(',')
-        matching_files = get_matching_files(function_name)
-
-        matching_files_filtered = []
-        for matching_file in matching_files:
-            if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)', matching_file):
-                matching_files_filtered.append(matching_file)
-        matching_files_filtered.sort()
-
-        run_lint_args = [
-                    RUN_LINT_FILE,
-                    '--skip-arguments',
-                    skip_arguments,
-                    function_name,
-        ]
-        run_lint_args.extend(matching_files_filtered)
-
-        try:
-            subprocess.run(run_lint_args, check = True)
-        except subprocess.CalledProcessError:
-            exit_code = 1
-
-    sys.exit(exit_code)
-
-if __name__ == '__main__':
-    main()
diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py
deleted file mode 100755
index 0e08c9f974..0000000000
--- a/test/lint/run-lint-format-strings.py
+++ /dev/null
@@ -1,298 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (c) 2018-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-# Lint format strings: This program checks that the number of arguments passed
-# to a variadic format string function matches the number of format specifiers
-# in the format string.
-
-import argparse
-import re
-import sys
-
-FALSE_POSITIVES = [
-    ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION)"),
-    ("src/test/translation_tests.cpp", "strprintf(format, arg)"),
-    ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi")'),
-    ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi")'),
-]
-
-
-def parse_function_calls(function_name, source_code):
-    """Return an array with all calls to function function_name in string source_code.
-    Preprocessor directives and C++ style comments ("//") in source_code are removed.
-
-    >>> len(parse_function_calls("foo", "foo();bar();foo();bar();"))
-    2
-    >>> parse_function_calls("foo", "foo(1);bar(1);foo(2);bar(2);")[0].startswith("foo(1);")
-    True
-    >>> parse_function_calls("foo", "foo(1);bar(1);foo(2);bar(2);")[1].startswith("foo(2);")
-    True
-    >>> len(parse_function_calls("foo", "foo();bar();// foo();bar();"))
-    1
-    >>> len(parse_function_calls("foo", "#define FOO foo();"))
-    0
-    """
-    assert type(function_name) is str and type(source_code) is str and function_name
-    lines = [re.sub("// .*", " ", line).strip()
-             for line in source_code.split("\n")
-             if not line.strip().startswith("#")]
-    return re.findall(r"[^a-zA-Z_](?=({}\(.*).*)".format(function_name), " " + " ".join(lines))
-
-
-def normalize(s):
-    """Return a normalized version of string s with newlines, tabs and C style comments ("/* ... */")
-    replaced with spaces. Multiple spaces are replaced with a single space.
-
-    >>> normalize("  /* nothing */   foo\tfoo  /* bar */  foo     ")
-    'foo foo foo'
-    """
-    assert type(s) is str
-    s = s.replace("\n", " ")
-    s = s.replace("\t", " ")
-    s = re.sub(r"/\*.*?\*/", " ", s)
-    s = re.sub(" {2,}", " ", s)
-    return s.strip()
-
-
-ESCAPE_MAP = {
-    r"\n": "[escaped-newline]",
-    r"\t": "[escaped-tab]",
-    r'\"': "[escaped-quote]",
-}
-
-
-def escape(s):
-    """Return the escaped version of string s with "\\\"", "\\n" and "\\t" escaped as
-    "[escaped-backslash]", "[escaped-newline]" and "[escaped-tab]".
-
-    >>> unescape(escape("foo")) == "foo"
-    True
-    >>> escape(r'foo \\t foo \\n foo \\\\ foo \\ foo \\"bar\\"')
-    'foo [escaped-tab] foo [escaped-newline] foo \\\\\\\\ foo \\\\ foo [escaped-quote]bar[escaped-quote]'
-    """
-    assert type(s) is str
-    for raw_value, escaped_value in ESCAPE_MAP.items():
-        s = s.replace(raw_value, escaped_value)
-    return s
-
-
-def unescape(s):
-    """Return the unescaped version of escaped string s.
-    Reverses the replacements made in function escape(s).
-
-    >>> unescape(escape("bar"))
-    'bar'
-    >>> unescape("foo [escaped-tab] foo [escaped-newline] foo \\\\\\\\ foo \\\\ foo [escaped-quote]bar[escaped-quote]")
-    'foo \\\\t foo \\\\n foo \\\\\\\\ foo \\\\ foo \\\\"bar\\\\"'
-    """
-    assert type(s) is str
-    for raw_value, escaped_value in ESCAPE_MAP.items():
-        s = s.replace(escaped_value, raw_value)
-    return s
-
-
-def parse_function_call_and_arguments(function_name, function_call):
-    """Split string function_call into an array of strings consisting of:
-    * the string function_call followed by "("
-    * the function call argument #1
-    * ...
-    * the function call argument #n
-    * a trailing ");"
-
-    The strings returned are in escaped form. See escape(...).
-
-    >>> parse_function_call_and_arguments("foo", 'foo("%s", "foo");')
-    ['foo(', '"%s",', ' "foo"', ')']
-    >>> parse_function_call_and_arguments("foo", 'foo("%s", "foo");')
-    ['foo(', '"%s",', ' "foo"', ')']
-    >>> parse_function_call_and_arguments("foo", 'foo("%s %s", "foo", "bar");')
-    ['foo(', '"%s %s",', ' "foo",', ' "bar"', ')']
-    >>> parse_function_call_and_arguments("fooprintf", 'fooprintf("%050d", i);')
-    ['fooprintf(', '"%050d",', ' i', ')']
-    >>> parse_function_call_and_arguments("foo", 'foo(bar(foobar(barfoo("foo"))), foobar); barfoo')
-    ['foo(', 'bar(foobar(barfoo("foo"))),', ' foobar', ')']
-    >>> parse_function_call_and_arguments("foo", "foo()")
-    ['foo(', '', ')']
-    >>> parse_function_call_and_arguments("foo", "foo(123)")
-    ['foo(', '123', ')']
-    >>> parse_function_call_and_arguments("foo", 'foo("foo")')
-    ['foo(', '"foo"', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>,wchar_t>().to_bytes(buf), err);')
-    ['strprintf(', '"%s (%d)",', ' std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>,wchar_t>().to_bytes(buf),', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo<wchar_t>().to_bytes(buf), err);')
-    ['strprintf(', '"%s (%d)",', ' foo<wchar_t>().to_bytes(buf),', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo().to_bytes(buf), err);')
-    ['strprintf(', '"%s (%d)",', ' foo().to_bytes(buf),', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo << 1, err);')
-    ['strprintf(', '"%s (%d)",', ' foo << 1,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo<bar>() >> 1, err);')
-    ['strprintf(', '"%s (%d)",', ' foo<bar>() >> 1,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo < 1 ? bar : foobar, err);')
-    ['strprintf(', '"%s (%d)",', ' foo < 1 ? bar : foobar,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo < 1, err);')
-    ['strprintf(', '"%s (%d)",', ' foo < 1,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo > 1 ? bar : foobar, err);')
-    ['strprintf(', '"%s (%d)",', ' foo > 1 ? bar : foobar,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo > 1, err);')
-    ['strprintf(', '"%s (%d)",', ' foo > 1,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo <= 1, err);')
-    ['strprintf(', '"%s (%d)",', ' foo <= 1,', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo <= bar<1, 2>(1, 2), err);')
-    ['strprintf(', '"%s (%d)",', ' foo <= bar<1, 2>(1, 2),', ' err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo>foo<1,2>(1,2)?bar:foobar,err)');
-    ['strprintf(', '"%s (%d)",', ' foo>foo<1,2>(1,2)?bar:foobar,', 'err', ')']
-    >>> parse_function_call_and_arguments("strprintf", 'strprintf("%s (%d)", foo>foo<1,2>(1,2),err)');
-    ['strprintf(', '"%s (%d)",', ' foo>foo<1,2>(1,2),', 'err', ')']
-    """
-    assert type(function_name) is str and type(function_call) is str and function_name
-    remaining = normalize(escape(function_call))
-    expected_function_call = "{}(".format(function_name)
-    assert remaining.startswith(expected_function_call)
-    parts = [expected_function_call]
-    remaining = remaining[len(expected_function_call):]
-    open_parentheses = 1
-    open_template_arguments = 0
-    in_string = False
-    parts.append("")
-    for i, char in enumerate(remaining):
-        parts.append(parts.pop() + char)
-        if char == "\"":
-            in_string = not in_string
-            continue
-        if in_string:
-            continue
-        if char == "(":
-            open_parentheses += 1
-            continue
-        if char == ")":
-            open_parentheses -= 1
-        if open_parentheses > 1:
-            continue
-        if open_parentheses == 0:
-            parts.append(parts.pop()[:-1])
-            parts.append(char)
-            break
-        prev_char = remaining[i - 1] if i - 1 >= 0 else None
-        next_char = remaining[i + 1] if i + 1 <= len(remaining) - 1 else None
-        if char == "<" and next_char not in [" ", "<", "="] and prev_char not in [" ", "<"]:
-            open_template_arguments += 1
-            continue
-        if char == ">" and next_char not in [" ", ">", "="] and prev_char not in [" ", ">"] and open_template_arguments > 0:
-            open_template_arguments -= 1
-        if open_template_arguments > 0:
-            continue
-        if char == ",":
-            parts.append("")
-    return parts
-
-
-def parse_string_content(argument):
-    """Return the text within quotes in string argument.
-
-    >>> parse_string_content('1 "foo %d bar" 2')
-    'foo %d bar'
-    >>> parse_string_content('1 foobar 2')
-    ''
-    >>> parse_string_content('1 "bar" 2')
-    'bar'
-    >>> parse_string_content('1 "foo" 2 "bar" 3')
-    'foobar'
-    >>> parse_string_content('1 "foo" 2 " " "bar" 3')
-    'foo bar'
-    >>> parse_string_content('""')
-    ''
-    >>> parse_string_content('')
-    ''
-    >>> parse_string_content('1 2 3')
-    ''
-    """
-    assert type(argument) is str
-    string_content = ""
-    in_string = False
-    for char in normalize(escape(argument)):
-        if char == "\"":
-            in_string = not in_string
-        elif in_string:
-            string_content += char
-    return string_content
-
-
-def count_format_specifiers(format_string):
-    """Return the number of format specifiers in string format_string.
-
-    >>> count_format_specifiers("foo bar foo")
-    0
-    >>> count_format_specifiers("foo %d bar foo")
-    1
-    >>> count_format_specifiers("foo %d bar %i foo")
-    2
-    >>> count_format_specifiers("foo %d bar %i foo %% foo")
-    2
-    >>> count_format_specifiers("foo %d bar %i foo %% foo %d foo")
-    3
-    >>> count_format_specifiers("foo %d bar %i foo %% foo %*d foo")
-    4
-    >>> count_format_specifiers("foo %5$d")
-    5
-    >>> count_format_specifiers("foo %5$*7$d")
-    7
-    """
-    assert type(format_string) is str
-    format_string = format_string.replace('%%', 'X')
-    n = max_pos = 0
-    for m in re.finditer("%(.*?)[aAcdeEfFgGinopsuxX]", format_string, re.DOTALL):
-        # Increase the max position if the argument has a position number like
-        # "5$", otherwise increment the argument count.
-        pos_num, = re.match(r"(?:(^\d+)\$)?", m.group(1)).groups()
-        if pos_num is not None:
-            max_pos = max(max_pos, int(pos_num))
-        else:
-            n += 1
-
-        # Increase the max position if there is a "*" width argument with a
-        # position like "*7$", and increment the argument count if there is a
-        # "*" width argument with no position.
-        star, star_pos_num = re.match(r"(?:.*?(\*(?:(\d+)\$)?)|)", m.group(1)).groups()
-        if star_pos_num is not None:
-            max_pos = max(max_pos, int(star_pos_num))
-        elif star is not None:
-            n += 1
-    return max(n, max_pos)
-
-
-def main():
-    parser = argparse.ArgumentParser(description="This program checks that the number of arguments passed "
-                                     "to a variadic format string function matches the number of format "
-                                     "specifiers in the format string.")
-    parser.add_argument("--skip-arguments", type=int, help="number of arguments before the format string "
-                        "argument (e.g. 1 in the case of fprintf)", default=0)
-    parser.add_argument("function_name", help="function name (e.g. fprintf)", default=None)
-    parser.add_argument("file", nargs="*", help="C++ source code file (e.g. foo.cpp)")
-    args = parser.parse_args()
-    exit_code = 0
-    for filename in args.file:
-        with open(filename, "r", encoding="utf-8") as f:
-            for function_call_str in parse_function_calls(args.function_name, f.read()):
-                parts = parse_function_call_and_arguments(args.function_name, function_call_str)
-                relevant_function_call_str = unescape("".join(parts))[:512]
-                if (f.name, relevant_function_call_str) in FALSE_POSITIVES:
-                    continue
-                if len(parts) < 3 + args.skip_arguments:
-                    exit_code = 1
-                    print("{}: Could not parse function call string \"{}(...)\": {}".format(f.name, args.function_name, relevant_function_call_str))
-                    continue
-                argument_count = len(parts) - 3 - args.skip_arguments
-                format_str = parse_string_content(parts[1 + args.skip_arguments])
-                format_specifier_count = count_format_specifiers(format_str)
-                if format_specifier_count != argument_count:
-                    exit_code = 1
-                    print("{}: Expected {} argument(s) after format string but found {} argument(s): {}".format(f.name, format_specifier_count, argument_count, relevant_function_call_str))
-                    continue
-    sys.exit(exit_code)
-
-
-if __name__ == "__main__":
-    main()

From fa3efb5729091a36a0e82316e9e4b7c09115dc2e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Sat, 7 Dec 2024 11:49:48 +0100
Subject: [PATCH 33/42] refactor: Introduce struct to hold a runtime format
 string

This brings the format types closer to the standard library types:

* FormatStringCheck corresponds to std::basic_format_string, with
  compile-time checks done via ConstevalFormatString
* RuntimeFormat corresponds to std::runtime_format, with no compile-time
  checks done.

Also, it documents where no compile-time checks are done.
---
 src/test/fuzz/strprintf.cpp    | 34 ++++++++++++++++++++--------------
 src/test/fuzz/util/wallet.h    |  2 +-
 src/test/util_string_tests.cpp |  2 +-
 src/tinyformat.h               | 13 ++++++++++---
 src/util/translation.h         |  2 +-
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/test/fuzz/strprintf.cpp b/src/test/fuzz/strprintf.cpp
index be40e88cdd..c7f24fbdb4 100644
--- a/src/test/fuzz/strprintf.cpp
+++ b/src/test/fuzz/strprintf.cpp
@@ -14,6 +14,12 @@
 #include <string>
 #include <vector>
 
+template <typename... Args>
+void fuzz_fmt(const std::string& fmt, const Args&... args)
+{
+    (void)tfm::format(tfm::RuntimeFormat{fmt}, args...);
+}
+
 FUZZ_TARGET(str_printf)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
@@ -51,22 +57,22 @@ FUZZ_TARGET(str_printf)
         CallOneOf(
             fuzzed_data_provider,
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeBool());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeBool());
             });
     } catch (const tinyformat::format_error&) {
     }
@@ -91,28 +97,28 @@ FUZZ_TARGET(str_printf)
         CallOneOf(
             fuzzed_data_provider,
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
             });
     } catch (const tinyformat::format_error&) {
     }
diff --git a/src/test/fuzz/util/wallet.h b/src/test/fuzz/util/wallet.h
index 8b55b7a985..1f04b5dbf1 100644
--- a/src/test/fuzz/util/wallet.h
+++ b/src/test/fuzz/util/wallet.h
@@ -46,7 +46,7 @@ struct FuzzedWallet {
 
         for (const std::string& desc_fmt : DESCS) {
             for (bool internal : {true, false}) {
-                const auto descriptor{(strprintf)(desc_fmt, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};
+                const auto descriptor{strprintf(tfm::RuntimeFormat{desc_fmt}, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};
 
                 FlatSigningProvider keys;
                 std::string error;
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 340c650fe3..65ee140b6e 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -16,7 +16,7 @@ template <unsigned NumArgs>
 void TfmFormatZeroes(const std::string& fmt)
 {
     std::apply([&](auto... args) {
-        (void)tfm::format(fmt, args...);
+        (void)tfm::format(tfm::RuntimeFormat{fmt}, args...);
     }, std::array<int, NumArgs>{});
 }
 
diff --git a/src/tinyformat.h b/src/tinyformat.h
index 0d0e9149cc..29b0f9e3ea 100644
--- a/src/tinyformat.h
+++ b/src/tinyformat.h
@@ -142,6 +142,7 @@ namespace tfm = tinyformat;
 //------------------------------------------------------------------------------
 // Implementation details.
 #include <algorithm>
+#include <attributes.h> // Added for Bitcoin Core
 #include <iostream>
 #include <sstream>
 #include <stdexcept> // Added for Bitcoin Core
@@ -179,13 +180,19 @@ namespace tfm = tinyformat;
 
 namespace tinyformat {
 
+// Added for Bitcoin Core. Similar to std::runtime_format from C++26.
+struct RuntimeFormat {
+    const std::string& fmt; // Not a string view, because tinyformat requires a c_str
+    explicit RuntimeFormat(LIFETIMEBOUND const std::string& str) : fmt{str} {}
+};
+
 // Added for Bitcoin Core. Wrapper for checking format strings at compile time.
-// Unlike ConstevalFormatString this supports std::string for runtime string
-// formatting without compile time checks.
+// Unlike ConstevalFormatString this supports RunTimeFormat-wrapped std::string
+// for runtime string formatting without compile time checks.
 template <unsigned num_params>
 struct FormatStringCheck {
     consteval FormatStringCheck(const char* str) : fmt{util::ConstevalFormatString<num_params>{str}.fmt} {}
-    FormatStringCheck(const std::string& str) : fmt{str.c_str()} {}
+    FormatStringCheck(LIFETIMEBOUND const RuntimeFormat& run) : fmt{run.fmt.c_str()} {}
     FormatStringCheck(util::ConstevalFormatString<num_params> str) : fmt{str.fmt} {}
     operator const char*() { return fmt; }
     const char* fmt;
diff --git a/src/util/translation.h b/src/util/translation.h
index 27747a16f2..cc29eef660 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -103,7 +103,7 @@ bilingual_str format(util::BilingualFmt<sizeof...(Args)> fmt, const Args&... arg
         }
     }};
     return bilingual_str{tfm::format(fmt.original, original_arg(args)...),
-                         tfm::format(std::string{fmt.lit}, translated_arg(args)...)};
+                         tfm::format(RuntimeFormat{std::string{fmt.lit}}, translated_arg(args)...)};
 }
 } // namespace tinyformat
 

From c03a2795a8e044d17835bbf03de0c64dc7b41da8 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Thu, 9 Jan 2025 11:25:43 +0100
Subject: [PATCH 34/42] util: Add integer left shift helpers

The helpers are used in the following commits to increase the safety of
conversions during cache size calculations.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: stickies-v <stickies-v@protonmail.com>
---
 src/test/util_tests.cpp | 97 +++++++++++++++++++++++++++++++++++++++++
 src/util/byte_units.h   | 22 ++++++++++
 src/util/overflow.h     | 36 +++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 src/util/byte_units.h

diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 3f6e5b1b66..129e84bc24 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -13,6 +13,7 @@
 #include <test/util/setup_common.h>
 #include <uint256.h>
 #include <util/bitdeque.h>
+#include <util/byte_units.h>
 #include <util/fs.h>
 #include <util/fs_helpers.h>
 #include <util/moneystr.h>
@@ -1877,4 +1878,100 @@ BOOST_AUTO_TEST_CASE(clearshrink_test)
     }
 }
 
+template <typename T>
+void TestCheckedLeftShift()
+{
+    constexpr auto MAX{std::numeric_limits<T>::max()};
+
+    // Basic operations
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(0, 1), 0);
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(0, 127), 0);
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(1, 1), 2);
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(2, 2), 8);
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(MAX >> 1, 1), MAX - 1);
+
+    // Max left shift
+    BOOST_CHECK_EQUAL(CheckedLeftShift<T>(1, std::numeric_limits<T>::digits - 1), MAX / 2 + 1);
+
+    // Overflow cases
+    BOOST_CHECK(!CheckedLeftShift<T>((MAX >> 1) + 1, 1));
+    BOOST_CHECK(!CheckedLeftShift<T>(MAX, 1));
+    BOOST_CHECK(!CheckedLeftShift<T>(1, std::numeric_limits<T>::digits));
+    BOOST_CHECK(!CheckedLeftShift<T>(1, std::numeric_limits<T>::digits + 1));
+
+    if constexpr (std::is_signed_v<T>) {
+        constexpr auto MIN{std::numeric_limits<T>::min()};
+        // Negative input
+        BOOST_CHECK_EQUAL(CheckedLeftShift<T>(-1, 1), -2);
+        BOOST_CHECK_EQUAL(CheckedLeftShift<T>((MIN >> 2), 1), MIN / 2);
+        BOOST_CHECK_EQUAL(CheckedLeftShift<T>((MIN >> 1) + 1, 1), MIN + 2);
+        BOOST_CHECK_EQUAL(CheckedLeftShift<T>(MIN >> 1, 1), MIN);
+        // Overflow negative
+        BOOST_CHECK(!CheckedLeftShift<T>((MIN >> 1) - 1, 1));
+        BOOST_CHECK(!CheckedLeftShift<T>(MIN >> 1, 2));
+        BOOST_CHECK(!CheckedLeftShift<T>(-1, 100));
+    }
+}
+
+template <typename T>
+void TestSaturatingLeftShift()
+{
+    constexpr auto MAX{std::numeric_limits<T>::max()};
+
+    // Basic operations
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(0, 1), 0);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(0, 127), 0);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(1, 1), 2);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(2, 2), 8);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(MAX >> 1, 1), MAX - 1);
+
+    // Max left shift
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(1, std::numeric_limits<T>::digits - 1), MAX / 2 + 1);
+
+    // Saturation cases
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>((MAX >> 1) + 1, 1), MAX);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(MAX, 1), MAX);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(1, std::numeric_limits<T>::digits), MAX);
+    BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(1, std::numeric_limits<T>::digits + 1), MAX);
+
+    if constexpr (std::is_signed_v<T>) {
+        constexpr auto MIN{std::numeric_limits<T>::min()};
+        // Negative input
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(-1, 1), -2);
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>((MIN >> 2), 1), MIN / 2);
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>((MIN >> 1) + 1, 1), MIN + 2);
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(MIN >> 1, 1), MIN);
+        // Saturation negative
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>((MIN >> 1) - 1, 1), MIN);
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(MIN >> 1, 2), MIN);
+        BOOST_CHECK_EQUAL(SaturatingLeftShift<T>(-1, 100), MIN);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(checked_left_shift_test)
+{
+    TestCheckedLeftShift<uint8_t>();
+    TestCheckedLeftShift<int8_t>();
+    TestCheckedLeftShift<size_t>();
+    TestCheckedLeftShift<uint64_t>();
+    TestCheckedLeftShift<int64_t>();
+}
+
+BOOST_AUTO_TEST_CASE(saturating_left_shift_test)
+{
+    TestSaturatingLeftShift<uint8_t>();
+    TestSaturatingLeftShift<int8_t>();
+    TestSaturatingLeftShift<size_t>();
+    TestSaturatingLeftShift<uint64_t>();
+    TestSaturatingLeftShift<int64_t>();
+}
+
+BOOST_AUTO_TEST_CASE(mib_string_literal_test)
+{
+    BOOST_CHECK_EQUAL(0_MiB, 0);
+    BOOST_CHECK_EQUAL(1_MiB, 1024 * 1024);
+    const auto max_mib{std::numeric_limits<size_t>::max() >> 20};
+    BOOST_CHECK_EXCEPTION(operator""_MiB(static_cast<unsigned long long>(max_mib) + 1), std::overflow_error, HasReason("MiB value too large for size_t byte conversion"));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util/byte_units.h b/src/util/byte_units.h
new file mode 100644
index 0000000000..894f057a7e
--- /dev/null
+++ b/src/util/byte_units.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2025-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.
+
+#ifndef BITCOIN_UTIL_BYTE_UNITS_H
+#define BITCOIN_UTIL_BYTE_UNITS_H
+
+#include <util/overflow.h>
+
+#include <stdexcept>
+
+//! Overflow-safe conversion of MiB to bytes.
+constexpr size_t operator"" _MiB(unsigned long long mebibytes)
+{
+    auto bytes{CheckedLeftShift(mebibytes, 20)};
+    if (!bytes || *bytes > std::numeric_limits<size_t>::max()) {
+        throw std::overflow_error("MiB value too large for size_t byte conversion");
+    }
+    return *bytes;
+}
+
+#endif // BITCOIN_UTIL_BYTE_UNITS_H
diff --git a/src/util/overflow.h b/src/util/overflow.h
index 7e0cce6c27..67711af0a5 100644
--- a/src/util/overflow.h
+++ b/src/util/overflow.h
@@ -5,6 +5,8 @@
 #ifndef BITCOIN_UTIL_OVERFLOW_H
 #define BITCOIN_UTIL_OVERFLOW_H
 
+#include <climits>
+#include <concepts>
 #include <limits>
 #include <optional>
 #include <type_traits>
@@ -47,4 +49,38 @@ template <class T>
     return i + j;
 }
 
+/**
+ * @brief Left bit shift with overflow checking.
+ * @param input The input value to be left shifted.
+ * @param shift The number of bits to left shift.
+ * @return (input * 2^shift) or nullopt if it would not fit in the return type.
+ */
+template <std::integral T>
+constexpr std::optional<T> CheckedLeftShift(T input, unsigned shift) noexcept
+{
+    if (shift == 0 || input == 0) return input;
+    // Avoid undefined c++ behaviour if shift is >= number of bits in T.
+    if (shift >= sizeof(T) * CHAR_BIT) return std::nullopt;
+    // If input << shift is too big to fit in T, return nullopt.
+    if (input > (std::numeric_limits<T>::max() >> shift)) return std::nullopt;
+    if (input < (std::numeric_limits<T>::min() >> shift)) return std::nullopt;
+    return input << shift;
+}
+
+/**
+ * @brief Left bit shift with safe minimum and maximum values.
+ * @param input The input value to be left shifted.
+ * @param shift The number of bits to left shift.
+ * @return (input * 2^shift) clamped to fit between the lowest and highest
+ *         representable values of the type T.
+ */
+template <std::integral T>
+constexpr T SaturatingLeftShift(T input, unsigned shift) noexcept
+{
+    if (auto result{CheckedLeftShift(input, shift)}) return *result;
+    // If input << shift is too big to fit in T, return biggest positive or negative
+    // number that fits.
+    return input < 0 ? std::numeric_limits<T>::min() : std::numeric_limits<T>::max();
+}
+
 #endif // BITCOIN_UTIL_OVERFLOW_H

From d5e2c4a4097c799433cfc5367c61568fad2c784e Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 14 Jan 2025 22:21:45 +0100
Subject: [PATCH 35/42] fuzz: Add fuzz test for checked and saturating add and
 left shift

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/test/fuzz/CMakeLists.txt |  1 +
 src/test/fuzz/overflow.cpp   | 50 ++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 src/test/fuzz/overflow.cpp

diff --git a/src/test/fuzz/CMakeLists.txt b/src/test/fuzz/CMakeLists.txt
index f65ed62b2d..a261d3ecea 100644
--- a/src/test/fuzz/CMakeLists.txt
+++ b/src/test/fuzz/CMakeLists.txt
@@ -71,6 +71,7 @@ add_executable(fuzz
   netaddress.cpp
   netbase_dns_lookup.cpp
   node_eviction.cpp
+  overflow.cpp
   p2p_handshake.cpp
   p2p_headers_presync.cpp
   p2p_transport_serialization.cpp
diff --git a/src/test/fuzz/overflow.cpp b/src/test/fuzz/overflow.cpp
new file mode 100644
index 0000000000..0278bd0f41
--- /dev/null
+++ b/src/test/fuzz/overflow.cpp
@@ -0,0 +1,50 @@
+// Copyright (c) 2025-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.
+
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <util/check.h>
+#include <util/overflow.h>
+
+#include <algorithm>
+#include <limits>
+#include <optional>
+
+namespace {
+//! Test overflow operations for type T using a wider type, W, to verify results.
+template <typename T, typename W>
+void TestOverflow(FuzzedDataProvider& fuzzed_data_provider)
+{
+    constexpr auto min{std::numeric_limits<T>::min()};
+    constexpr auto max{std::numeric_limits<T>::max()};
+    // Range needs to be at least twice as big to allow two numbers to be added without overflowing.
+    static_assert(min >= std::numeric_limits<W>::min() / 2);
+    static_assert(max <= std::numeric_limits<W>::max() / 2);
+
+    auto widen = [](T value) -> W { return value; };
+    auto clamp = [](W value) -> W { return std::clamp<W>(value, min, max); };
+    auto check = [](W value) -> std::optional<W> { if (value >= min && value <= max) return value; else return std::nullopt; };
+
+    const T i = fuzzed_data_provider.ConsumeIntegral<T>();
+    const T j = fuzzed_data_provider.ConsumeIntegral<T>();
+    const unsigned shift = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, std::numeric_limits<W>::digits - std::numeric_limits<T>::digits);
+
+    Assert(clamp(widen(i) + widen(j)) == SaturatingAdd(i, j));
+    Assert(check(widen(i) + widen(j)) == CheckedAdd(i, j));
+
+    Assert(clamp(widen(i) << shift) == SaturatingLeftShift(i, shift));
+    Assert(check(widen(i) << shift) == CheckedLeftShift(i, shift));
+}
+} // namespace
+
+FUZZ_TARGET(overflow)
+{
+    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+    TestOverflow<int8_t, int64_t>(fuzzed_data_provider);
+    TestOverflow<int16_t, int64_t>(fuzzed_data_provider);
+    TestOverflow<int32_t, int64_t>(fuzzed_data_provider);
+    TestOverflow<uint8_t, uint64_t>(fuzzed_data_provider);
+    TestOverflow<uint16_t, uint64_t>(fuzzed_data_provider);
+    TestOverflow<uint32_t, uint64_t>(fuzzed_data_provider);
+}

From e758b26b85da27ef44f3d2c924f3f08e8c1f4fdf Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 22 Nov 2024 20:51:34 +0100
Subject: [PATCH 36/42] kernel: Move kernel-specific cache size options to
 kernel

Carrying non-kernel related fields in the cache sizes for the indexes is
confusing for kernel library users. The cache sizes also are set
currently with magic numbers in bitcoin-chainstate. The comments for the
cache size calculations are also not completely clear.

Solve these things by moving the kernel-specific cache size fields to
their own struct.

This slightly changes the way the cache is allocated if the txindex
and/or blockfilterindex is used. Since they are now given precedence
over the block tree db cache, this results in a bit less cache being
allocated to the block tree db, coinsdb and coins caches. The effect is
negligible though, i.e. cache sizes with default dbcache reported
through the logs are:

master:
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.0 MiB for transaction index database
* Using 49.0 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 335.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)

this branch:
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.2 MiB for transaction index database
* Using 49.2 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 334.5 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
---
 src/bitcoin-chainstate.cpp     |  5 +----
 src/init.cpp                   | 26 +++++++++++++-------------
 src/kernel/caches.h            | 34 ++++++++++++++++++++++++++++++++++
 src/node/caches.cpp            | 14 ++++++--------
 src/node/caches.h              | 13 ++++++++-----
 src/node/chainstate.cpp        |  4 +++-
 src/node/chainstate.h          |  8 +++++---
 src/test/util/setup_common.cpp |  7 ++-----
 src/test/util/setup_common.h   |  3 ++-
 src/txdb.h                     |  4 ----
 10 files changed, 74 insertions(+), 44 deletions(-)
 create mode 100644 src/kernel/caches.h

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index bab09f13d9..3b551e5c17 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -123,10 +123,7 @@ int main(int argc, char* argv[])
     util::SignalInterrupt interrupt;
     ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
 
-    node::CacheSizes cache_sizes;
-    cache_sizes.block_tree_db = 2 << 20;
-    cache_sizes.coins_db = 2 << 22;
-    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
+    kernel::CacheSizes cache_sizes{nDefaultDbCache << 20};
     node::ChainstateLoadOptions options;
     auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
diff --git a/src/init.cpp b/src/init.cpp
index a4ad2771cc..878e11518b 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -32,6 +32,7 @@
 #include <interfaces/ipc.h>
 #include <interfaces/mining.h>
 #include <interfaces/node.h>
+#include <kernel/caches.h>
 #include <kernel/context.h>
 #include <key.h>
 #include <logging.h>
@@ -121,7 +122,6 @@ using common::ResolveErrMsg;
 
 using node::ApplyArgsManOptions;
 using node::BlockManager;
-using node::CacheSizes;
 using node::CalculateCacheSizes;
 using node::ChainstateLoadResult;
 using node::ChainstateLoadStatus;
@@ -1177,7 +1177,7 @@ static ChainstateLoadResult InitAndLoadChainstate(
     NodeContext& node,
     bool do_reindex,
     const bool do_reindex_chainstate,
-    CacheSizes& cache_sizes,
+    const kernel::CacheSizes& cache_sizes,
     const ArgsManager& args)
 {
     const CChainParams& chainparams = Params();
@@ -1603,18 +1603,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     ReadNotificationArgs(args, kernel_notifications);
 
     // cache size calculations
-    CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size());
+    const auto [index_cache_sizes, kernel_cache_sizes] = CalculateCacheSizes(args, g_enabled_filter_types.size());
 
-    LogPrintf("Cache configuration:\n");
-    LogPrintf("* Using %.1f MiB for block index database\n", cache_sizes.block_tree_db * (1.0 / 1024 / 1024));
+    LogInfo("Cache configuration:");
+    LogInfo("* Using %.1f MiB for block index database", kernel_cache_sizes.block_tree_db * (1.0 / 1024 / 1024));
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
-        LogPrintf("* Using %.1f MiB for transaction index database\n", cache_sizes.tx_index * (1.0 / 1024 / 1024));
+        LogInfo("* Using %.1f MiB for transaction index database", index_cache_sizes.tx_index * (1.0 / 1024 / 1024));
     }
     for (BlockFilterType filter_type : g_enabled_filter_types) {
-        LogPrintf("* Using %.1f MiB for %s block filter index database\n",
-                  cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type));
+        LogInfo("* Using %.1f MiB for %s block filter index database",
+                  index_cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type));
     }
-    LogPrintf("* Using %.1f MiB for chain state database\n", cache_sizes.coins_db * (1.0 / 1024 / 1024));
+    LogInfo("* Using %.1f MiB for chain state database", kernel_cache_sizes.coins_db * (1.0 / 1024 / 1024));
 
     assert(!node.mempool);
     assert(!node.chainman);
@@ -1627,7 +1627,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         node,
         do_reindex,
         do_reindex_chainstate,
-        cache_sizes,
+        kernel_cache_sizes,
         args);
     if (status == ChainstateLoadStatus::FAILURE && !do_reindex && !ShutdownRequested(node)) {
         // suggest a reindex
@@ -1646,7 +1646,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             node,
             do_reindex,
             do_reindex_chainstate,
-            cache_sizes,
+            kernel_cache_sizes,
             args);
     }
     if (status != ChainstateLoadStatus::SUCCESS && status != ChainstateLoadStatus::INTERRUPTED) {
@@ -1672,12 +1672,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // ********************************************************* Step 8: start indexers
 
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
-        g_txindex = std::make_unique<TxIndex>(interfaces::MakeChain(node), cache_sizes.tx_index, false, do_reindex);
+        g_txindex = std::make_unique<TxIndex>(interfaces::MakeChain(node), index_cache_sizes.tx_index, false, do_reindex);
         node.indexes.emplace_back(g_txindex.get());
     }
 
     for (const auto& filter_type : g_enabled_filter_types) {
-        InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, cache_sizes.filter_index, false, do_reindex);
+        InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, index_cache_sizes.filter_index, false, do_reindex);
         node.indexes.emplace_back(GetBlockFilterIndex(filter_type));
     }
 
diff --git a/src/kernel/caches.h b/src/kernel/caches.h
new file mode 100644
index 0000000000..2eab84df6b
--- /dev/null
+++ b/src/kernel/caches.h
@@ -0,0 +1,34 @@
+// Copyright (c) 2024-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.
+
+#ifndef BITCOIN_KERNEL_CACHES_H
+#define BITCOIN_KERNEL_CACHES_H
+
+#include <util/byte_units.h>
+
+#include <algorithm>
+
+//! Max memory allocated to block tree DB specific cache (bytes)
+static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
+//! Max memory allocated to coin DB specific cache (bytes)
+static constexpr size_t MAX_COINS_DB_CACHE{8_MiB};
+
+namespace kernel {
+struct CacheSizes {
+    size_t block_tree_db;
+    size_t coins_db;
+    size_t coins;
+
+    CacheSizes(size_t total_cache)
+    {
+        block_tree_db = std::min(total_cache / 8, MAX_BLOCK_DB_CACHE);
+        total_cache -= block_tree_db;
+        coins_db = std::min(total_cache / 2, MAX_COINS_DB_CACHE);
+        total_cache -= coins_db;
+        coins = total_cache; // the rest goes to the coins cache
+    }
+};
+} // namespace kernel
+
+#endif // BITCOIN_KERNEL_CACHES_H
diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index 01b08e320d..50b8bceebe 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -6,27 +6,25 @@
 
 #include <common/args.h>
 #include <index/txindex.h>
+#include <kernel/caches.h>
 #include <txdb.h>
 
+#include <algorithm>
+#include <string>
+
 namespace node {
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
     int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20);
     nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache
-    CacheSizes sizes;
-    sizes.block_tree_db = std::min(nTotalCache / 8, nMaxBlockDBCache << 20);
-    nTotalCache -= sizes.block_tree_db;
+    IndexCacheSizes sizes;
     sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0);
     nTotalCache -= sizes.tx_index;
-    sizes.filter_index = 0;
     if (n_indexes > 0) {
         int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20);
         sizes.filter_index = max_cache / n_indexes;
         nTotalCache -= sizes.filter_index * n_indexes;
     }
-    sizes.coins_db = std::min(nTotalCache / 2, nMaxCoinsDBCache << 20);
-    nTotalCache -= sizes.coins_db;
-    sizes.coins = nTotalCache; // the rest goes to in-memory cache
-    return sizes;
+    return {sizes, kernel::CacheSizes{static_cast<size_t>(nTotalCache)}};
 }
 } // namespace node
diff --git a/src/node/caches.h b/src/node/caches.h
index 67388b91fd..1818dff1f8 100644
--- a/src/node/caches.h
+++ b/src/node/caches.h
@@ -5,18 +5,21 @@
 #ifndef BITCOIN_NODE_CACHES_H
 #define BITCOIN_NODE_CACHES_H
 
+#include <kernel/caches.h>
+
 #include <cstddef>
 #include <cstdint>
 
 class ArgsManager;
 
 namespace node {
+struct IndexCacheSizes {
+    int64_t tx_index{0};
+    int64_t filter_index{0};
+};
 struct CacheSizes {
-    int64_t block_tree_db;
-    int64_t coins_db;
-    int64_t coins;
-    int64_t tx_index;
-    int64_t filter_index;
+    IndexCacheSizes index;
+    kernel::CacheSizes kernel;
 };
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0);
 } // namespace node
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index e56896fb22..660d2c3289 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -8,9 +8,9 @@
 #include <chain.h>
 #include <coins.h>
 #include <consensus/params.h>
+#include <kernel/caches.h>
 #include <logging.h>
 #include <node/blockstorage.h>
-#include <node/caches.h>
 #include <sync.h>
 #include <threadsafety.h>
 #include <tinyformat.h>
@@ -29,6 +29,8 @@
 #include <memory>
 #include <vector>
 
+using kernel::CacheSizes;
+
 namespace node {
 // Complete initialization of chainstates after the initial call has been made
 // to ChainstateManager::InitializeChainstate().
diff --git a/src/node/chainstate.h b/src/node/chainstate.h
index bb0c4f2b87..ce414fa043 100644
--- a/src/node/chainstate.h
+++ b/src/node/chainstate.h
@@ -14,9 +14,11 @@
 
 class CTxMemPool;
 
-namespace node {
-
+namespace kernel {
 struct CacheSizes;
+} // namespace kernel
+
+namespace node {
 
 struct ChainstateLoadOptions {
     CTxMemPool* mempool{nullptr};
@@ -69,7 +71,7 @@ using ChainstateLoadResult = std::tuple<ChainstateLoadStatus, bilingual_str>;
  *
  *  LoadChainstate returns a (status code, error string) tuple.
  */
-ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const kernel::CacheSizes& cache_sizes,
                                     const ChainstateLoadOptions& options);
 ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options);
 } // namespace node
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 12feba09a5..5ede59b82a 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -66,7 +66,6 @@ using kernel::BlockTreeDB;
 using node::ApplyArgsManOptions;
 using node::BlockAssembler;
 using node::BlockManager;
-using node::CalculateCacheSizes;
 using node::KernelNotifications;
 using node::LoadChainstate;
 using node::RegenerateCommitments;
@@ -228,8 +227,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
     Assert(error.empty());
     m_node.warnings = std::make_unique<node::Warnings>();
 
-    m_cache_sizes = CalculateCacheSizes(m_args);
-
     m_node.notifications = std::make_unique<KernelNotifications>(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings));
 
     m_make_chainman = [this, &chainparams, opts] {
@@ -255,7 +252,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
         LOCK(m_node.chainman->GetMutex());
         m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
             .path = m_args.GetDataDirNet() / "blocks" / "index",
-            .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
+            .cache_bytes = static_cast<size_t>(m_kernel_cache_sizes.block_tree_db),
             .memory_only = true,
         });
     };
@@ -291,7 +288,7 @@ void ChainTestingSetup::LoadVerifyActivateChainstate()
     options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
     options.check_level = m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
     options.require_full_verification = m_args.IsArgSet("-checkblocks") || m_args.IsArgSet("-checklevel");
-    auto [status, error] = LoadChainstate(chainman, m_cache_sizes, options);
+    auto [status, error] = LoadChainstate(chainman, m_kernel_cache_sizes, options);
     assert(status == node::ChainstateLoadStatus::SUCCESS);
 
     std::tie(status, error) = VerifyLoadedChainstate(chainman, options);
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index b0f7bdade2..33ad258457 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -6,6 +6,7 @@
 #define BITCOIN_TEST_UTIL_SETUP_COMMON_H
 
 #include <common/args.h> // IWYU pragma: export
+#include <kernel/caches.h>
 #include <kernel/context.h>
 #include <key.h>
 #include <node/caches.h>
@@ -103,7 +104,7 @@ struct BasicTestingSetup {
  * initialization behaviour.
  */
 struct ChainTestingSetup : public BasicTestingSetup {
-    node::CacheSizes m_cache_sizes{};
+    kernel::CacheSizes m_kernel_cache_sizes{node::CalculateCacheSizes(m_args).kernel};
     bool m_coins_db_in_memory{true};
     bool m_block_tree_db_in_memory{true};
     std::function<void()> m_make_chainman{};
diff --git a/src/txdb.h b/src/txdb.h
index 671cbd1286..57ae526713 100644
--- a/src/txdb.h
+++ b/src/txdb.h
@@ -27,16 +27,12 @@ static const int64_t nDefaultDbCache = 450;
 static const int64_t nDefaultDbBatchSize = 16 << 20;
 //! min. -dbcache (MiB)
 static const int64_t nMinDbCache = 4;
-//! Max memory allocated to block tree DB specific cache (MiB)
-static const int64_t nMaxBlockDBCache = 2;
 // Unlike for the UTXO database, for the txindex scenario the leveldb cache make
 // a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
 //! Max memory allocated to tx index DB specific cache in MiB.
 static const int64_t nMaxTxIndexCache = 1024;
 //! Max memory allocated to all block filter index caches combined in MiB.
 static const int64_t max_filter_index_cache = 1024;
-//! Max memory allocated to coin DB specific cache (MiB)
-static const int64_t nMaxCoinsDBCache = 8;
 
 //! User-controlled performance and debug options.
 struct CoinsViewOptions {

From 8826cae285490439dc1f19b25fa70b2b9e62dfe8 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 22 Nov 2024 22:44:17 +0100
Subject: [PATCH 37/42] kernel: Move non-kernel db cache size constants

These have nothing to do with the txdb, so move them out and into the
node caches.
---
 src/node/caches.cpp | 11 +++++++++--
 src/node/caches.h   |  5 ++---
 src/txdb.h          |  6 ------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index 50b8bceebe..c1d1d81992 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -12,16 +12,23 @@
 #include <algorithm>
 #include <string>
 
+// Unlike for the UTXO database, for the txindex scenario the leveldb cache make
+// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
+//! Max memory allocated to tx index DB specific cache in MiB.
+static constexpr int64_t MAX_TX_INDEX_CACHE{1024};
+//! Max memory allocated to all block filter index caches combined in MiB.
+static constexpr int64_t MAX_FILTER_INDEX_CACHE{1024};
+
 namespace node {
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
     int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20);
     nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache
     IndexCacheSizes sizes;
-    sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0);
+    sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE << 20 : 0);
     nTotalCache -= sizes.tx_index;
     if (n_indexes > 0) {
-        int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20);
+        int64_t max_cache = std::min(nTotalCache / 8, MAX_FILTER_INDEX_CACHE << 20);
         sizes.filter_index = max_cache / n_indexes;
         nTotalCache -= sizes.filter_index * n_indexes;
     }
diff --git a/src/node/caches.h b/src/node/caches.h
index 1818dff1f8..0ef5ddb7c5 100644
--- a/src/node/caches.h
+++ b/src/node/caches.h
@@ -8,14 +8,13 @@
 #include <kernel/caches.h>
 
 #include <cstddef>
-#include <cstdint>
 
 class ArgsManager;
 
 namespace node {
 struct IndexCacheSizes {
-    int64_t tx_index{0};
-    int64_t filter_index{0};
+    size_t tx_index{0};
+    size_t filter_index{0};
 };
 struct CacheSizes {
     IndexCacheSizes index;
diff --git a/src/txdb.h b/src/txdb.h
index 57ae526713..ab2cb641fc 100644
--- a/src/txdb.h
+++ b/src/txdb.h
@@ -27,12 +27,6 @@ static const int64_t nDefaultDbCache = 450;
 static const int64_t nDefaultDbBatchSize = 16 << 20;
 //! min. -dbcache (MiB)
 static const int64_t nMinDbCache = 4;
-// Unlike for the UTXO database, for the txindex scenario the leveldb cache make
-// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
-//! Max memory allocated to tx index DB specific cache in MiB.
-static const int64_t nMaxTxIndexCache = 1024;
-//! Max memory allocated to all block filter index caches combined in MiB.
-static const int64_t max_filter_index_cache = 1024;
 
 //! User-controlled performance and debug options.
 struct CoinsViewOptions {

From 65cde3621dbb9ac7d210d4926e7601c4adf5f498 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 22 Nov 2024 22:52:02 +0100
Subject: [PATCH 38/42] kernel: Move default cache constants to caches

They are not related to the txdb, so a better place for them is the
new kernel and node cache file. Re-use the default amount of kernel
cache for the default node cache.
---
 src/bitcoin-chainstate.cpp | 4 ++--
 src/init.cpp               | 2 +-
 src/kernel/caches.h        | 2 ++
 src/node/caches.cpp        | 5 ++---
 src/node/caches.h          | 5 +++++
 src/qt/optionsdialog.cpp   | 6 +++---
 src/qt/optionsmodel.cpp    | 6 +++---
 src/txdb.h                 | 4 ----
 8 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 3b551e5c17..50c6e0b6f1 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -19,9 +19,9 @@
 
 #include <consensus/validation.h>
 #include <core_io.h>
+#include <kernel/caches.h>
 #include <logging.h>
 #include <node/blockstorage.h>
-#include <node/caches.h>
 #include <node/chainstate.h>
 #include <random.h>
 #include <script/sigcache.h>
@@ -123,7 +123,7 @@ int main(int argc, char* argv[])
     util::SignalInterrupt interrupt;
     ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
 
-    kernel::CacheSizes cache_sizes{nDefaultDbCache << 20};
+    kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE << 20};
     node::ChainstateLoadOptions options;
     auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
diff --git a/src/init.cpp b/src/init.cpp
index 878e11518b..8827c41fea 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -487,7 +487,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-conf=<file>", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location (only useable from command line, not configuration file) (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", nMinDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE, DEFAULT_DB_CACHE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-allowignoredconf", strprintf("For backwards compatibility, treat an unused %s file in the datadir as a warning, not an error.", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
diff --git a/src/kernel/caches.h b/src/kernel/caches.h
index 2eab84df6b..7343eee66a 100644
--- a/src/kernel/caches.h
+++ b/src/kernel/caches.h
@@ -9,6 +9,8 @@
 
 #include <algorithm>
 
+//! Suggested default amount of cache reserved for the kernel (MiB)
+static constexpr int64_t DEFAULT_KERNEL_CACHE{450};
 //! Max memory allocated to block tree DB specific cache (bytes)
 static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
 //! Max memory allocated to coin DB specific cache (bytes)
diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index c1d1d81992..d2606dab58 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -7,7 +7,6 @@
 #include <common/args.h>
 #include <index/txindex.h>
 #include <kernel/caches.h>
-#include <txdb.h>
 
 #include <algorithm>
 #include <string>
@@ -22,8 +21,8 @@ static constexpr int64_t MAX_FILTER_INDEX_CACHE{1024};
 namespace node {
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
-    int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20);
-    nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache
+    int64_t nTotalCache = (args.GetIntArg("-dbcache", DEFAULT_DB_CACHE) << 20);
+    nTotalCache = std::max(nTotalCache, MIN_DB_CACHE << 20);
     IndexCacheSizes sizes;
     sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE << 20 : 0);
     nTotalCache -= sizes.tx_index;
diff --git a/src/node/caches.h b/src/node/caches.h
index 0ef5ddb7c5..d79c3889d3 100644
--- a/src/node/caches.h
+++ b/src/node/caches.h
@@ -11,6 +11,11 @@
 
 class ArgsManager;
 
+//! min. -dbcache (MiB)
+static constexpr int64_t MIN_DB_CACHE{4};
+//! -dbcache default (MiB)
+static constexpr int64_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE};
+
 namespace node {
 struct IndexCacheSizes {
     size_t tx_index{0};
diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index f86b5229db..76b7852109 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -15,9 +15,9 @@
 
 #include <common/system.h>
 #include <interfaces/node.h>
-#include <node/chainstatemanager_args.h>
 #include <netbase.h>
-#include <txdb.h>
+#include <node/caches.h>
+#include <node/chainstatemanager_args.h>
 #include <util/strencodings.h>
 
 #include <chrono>
@@ -95,7 +95,7 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet)
     ui->verticalLayout->setStretchFactor(ui->tabWidget, 1);
 
     /* Main elements init */
-    ui->databaseCache->setRange(nMinDbCache, std::numeric_limits<int>::max());
+    ui->databaseCache->setRange(MIN_DB_CACHE, std::numeric_limits<int>::max());
     ui->threadsScriptVerif->setMinimum(-GetNumCores());
     ui->threadsScriptVerif->setMaximum(MAX_SCRIPTCHECK_THREADS);
     ui->pruneWarning->setVisible(false);
diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp
index a1dbc6248c..2c6f13dc6d 100644
--- a/src/qt/optionsmodel.cpp
+++ b/src/qt/optionsmodel.cpp
@@ -15,8 +15,8 @@
 #include <mapport.h>
 #include <net.h>
 #include <netbase.h>
+#include <node/caches.h>
 #include <node/chainstatemanager_args.h>
-#include <txdb.h> // for -dbcache defaults
 #include <util/string.h>
 #include <validation.h>    // For DEFAULT_SCRIPTCHECK_THREADS
 #include <wallet/wallet.h> // For DEFAULT_SPEND_ZEROCONF_CHANGE
@@ -470,7 +470,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con
                suffix.empty()          ? getOption(option, "-prev") :
                                          DEFAULT_PRUNE_TARGET_GB;
     case DatabaseCache:
-        return qlonglong(SettingToInt(setting(), nDefaultDbCache));
+        return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE));
     case ThreadsScriptVerif:
         return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS));
     case Listen:
@@ -733,7 +733,7 @@ void OptionsModel::checkAndMigrate()
         // see https://github.com/bitcoin/bitcoin/pull/8273
         // force people to upgrade to the new value if they are using 100MB
         if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toLongLong() == 100)
-            settings.setValue("nDatabaseCache", (qint64)nDefaultDbCache);
+            settings.setValue("nDatabaseCache", (qint64)DEFAULT_DB_CACHE);
 
         settings.setValue(strSettingsVersionKey, CLIENT_VERSION);
     }
diff --git a/src/txdb.h b/src/txdb.h
index ab2cb641fc..968b7c2781 100644
--- a/src/txdb.h
+++ b/src/txdb.h
@@ -21,12 +21,8 @@
 class COutPoint;
 class uint256;
 
-//! -dbcache default (MiB)
-static const int64_t nDefaultDbCache = 450;
 //! -dbbatchsize default (bytes)
 static const int64_t nDefaultDbBatchSize = 16 << 20;
-//! min. -dbcache (MiB)
-static const int64_t nMinDbCache = 4;
 
 //! User-controlled performance and debug options.
 struct CoinsViewOptions {

From 2a92702bafca5c78b270a9502a22cb9deac02cfc Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 17 Dec 2024 21:58:47 +0100
Subject: [PATCH 39/42] init: Use size_t consistently for cache sizes

This avoids having to rely on implicit casts when passing them to the
various functions allocating the caches.

This also ensures that if the requested amount of db_cache does not fit
in a size_t, it is clamped to the maximum value of a size_t.

Also take this opportunity to make the total amounts of cache in the
chainstate manager a size_t too.
---
 src/bitcoin-chainstate.cpp     |  2 +-
 src/init.cpp                   |  2 +-
 src/kernel/caches.h            |  4 ++--
 src/node/caches.cpp            | 34 +++++++++++++++++++++-------------
 src/node/caches.h              |  9 +++++----
 src/node/chainstate.cpp        |  2 +-
 src/qt/optionsdialog.cpp       |  2 +-
 src/qt/optionsmodel.cpp        |  4 ++--
 src/test/util/setup_common.cpp |  2 +-
 src/validation.h               |  4 ++--
 10 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 50c6e0b6f1..51c482607a 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -123,7 +123,7 @@ int main(int argc, char* argv[])
     util::SignalInterrupt interrupt;
     ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
 
-    kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE << 20};
+    kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE};
     node::ChainstateLoadOptions options;
     auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
diff --git a/src/init.cpp b/src/init.cpp
index 8827c41fea..5c0fc9fbff 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -487,7 +487,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-conf=<file>", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location (only useable from command line, not configuration file) (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE, DEFAULT_DB_CACHE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE >> 20, DEFAULT_DB_CACHE >> 20), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-allowignoredconf", strprintf("For backwards compatibility, treat an unused %s file in the datadir as a warning, not an error.", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
diff --git a/src/kernel/caches.h b/src/kernel/caches.h
index 7343eee66a..33ae604772 100644
--- a/src/kernel/caches.h
+++ b/src/kernel/caches.h
@@ -9,8 +9,8 @@
 
 #include <algorithm>
 
-//! Suggested default amount of cache reserved for the kernel (MiB)
-static constexpr int64_t DEFAULT_KERNEL_CACHE{450};
+//! Suggested default amount of cache reserved for the kernel (bytes)
+static constexpr size_t DEFAULT_KERNEL_CACHE{450_MiB};
 //! Max memory allocated to block tree DB specific cache (bytes)
 static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
 //! Max memory allocated to coin DB specific cache (bytes)
diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index d2606dab58..8b432637c7 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -7,30 +7,38 @@
 #include <common/args.h>
 #include <index/txindex.h>
 #include <kernel/caches.h>
+#include <logging.h>
+#include <util/byte_units.h>
 
 #include <algorithm>
 #include <string>
 
 // Unlike for the UTXO database, for the txindex scenario the leveldb cache make
 // a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
-//! Max memory allocated to tx index DB specific cache in MiB.
-static constexpr int64_t MAX_TX_INDEX_CACHE{1024};
-//! Max memory allocated to all block filter index caches combined in MiB.
-static constexpr int64_t MAX_FILTER_INDEX_CACHE{1024};
+//! Max memory allocated to tx index DB specific cache in bytes.
+static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB};
+//! Max memory allocated to all block filter index caches combined in bytes.
+static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB};
 
 namespace node {
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
-    int64_t nTotalCache = (args.GetIntArg("-dbcache", DEFAULT_DB_CACHE) << 20);
-    nTotalCache = std::max(nTotalCache, MIN_DB_CACHE << 20);
-    IndexCacheSizes sizes;
-    sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE << 20 : 0);
-    nTotalCache -= sizes.tx_index;
+    // Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
+    size_t total_cache{DEFAULT_DB_CACHE};
+    if (std::optional<int64_t> db_cache = args.GetIntArg("-dbcache")) {
+        if (*db_cache < 0) db_cache = 0;
+        uint64_t db_cache_bytes = SaturatingLeftShift<uint64_t>(*db_cache, 20);
+        total_cache = std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, std::numeric_limits<size_t>::max()));
+    }
+
+    IndexCacheSizes index_sizes;
+    index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
+    total_cache -= index_sizes.tx_index;
     if (n_indexes > 0) {
-        int64_t max_cache = std::min(nTotalCache / 8, MAX_FILTER_INDEX_CACHE << 20);
-        sizes.filter_index = max_cache / n_indexes;
-        nTotalCache -= sizes.filter_index * n_indexes;
+        size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE);
+        index_sizes.filter_index = max_cache / n_indexes;
+        total_cache -= index_sizes.filter_index * n_indexes;
     }
-    return {sizes, kernel::CacheSizes{static_cast<size_t>(nTotalCache)}};
+    return {index_sizes, kernel::CacheSizes{total_cache}};
 }
 } // namespace node
diff --git a/src/node/caches.h b/src/node/caches.h
index d79c3889d3..f24e9cc910 100644
--- a/src/node/caches.h
+++ b/src/node/caches.h
@@ -6,15 +6,16 @@
 #define BITCOIN_NODE_CACHES_H
 
 #include <kernel/caches.h>
+#include <util/byte_units.h>
 
 #include <cstddef>
 
 class ArgsManager;
 
-//! min. -dbcache (MiB)
-static constexpr int64_t MIN_DB_CACHE{4};
-//! -dbcache default (MiB)
-static constexpr int64_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE};
+//! min. -dbcache (bytes)
+static constexpr size_t MIN_DB_CACHE{4_MiB};
+//! -dbcache default (bytes)
+static constexpr size_t DEFAULT_DB_CACHE{DEFAULT_KERNEL_CACHE};
 
 namespace node {
 struct IndexCacheSizes {
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 660d2c3289..3ee557ae62 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -46,7 +46,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
     try {
         pblocktree = std::make_unique<BlockTreeDB>(DBParams{
             .path = chainman.m_options.datadir / "blocks" / "index",
-            .cache_bytes = static_cast<size_t>(cache_sizes.block_tree_db),
+            .cache_bytes = cache_sizes.block_tree_db,
             .memory_only = options.block_tree_db_in_memory,
             .wipe_data = options.wipe_block_tree_db,
             .options = chainman.m_options.block_tree_db});
diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index 76b7852109..2cea1bf6ab 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -95,7 +95,7 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet)
     ui->verticalLayout->setStretchFactor(ui->tabWidget, 1);
 
     /* Main elements init */
-    ui->databaseCache->setRange(MIN_DB_CACHE, std::numeric_limits<int>::max());
+    ui->databaseCache->setRange(MIN_DB_CACHE >> 20, std::numeric_limits<int>::max());
     ui->threadsScriptVerif->setMinimum(-GetNumCores());
     ui->threadsScriptVerif->setMaximum(MAX_SCRIPTCHECK_THREADS);
     ui->pruneWarning->setVisible(false);
diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp
index 2c6f13dc6d..5782a57a26 100644
--- a/src/qt/optionsmodel.cpp
+++ b/src/qt/optionsmodel.cpp
@@ -470,7 +470,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con
                suffix.empty()          ? getOption(option, "-prev") :
                                          DEFAULT_PRUNE_TARGET_GB;
     case DatabaseCache:
-        return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE));
+        return qlonglong(SettingToInt(setting(), DEFAULT_DB_CACHE >> 20));
     case ThreadsScriptVerif:
         return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS));
     case Listen:
@@ -733,7 +733,7 @@ void OptionsModel::checkAndMigrate()
         // see https://github.com/bitcoin/bitcoin/pull/8273
         // force people to upgrade to the new value if they are using 100MB
         if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toLongLong() == 100)
-            settings.setValue("nDatabaseCache", (qint64)DEFAULT_DB_CACHE);
+            settings.setValue("nDatabaseCache", (qint64)(DEFAULT_DB_CACHE >> 20));
 
         settings.setValue(strSettingsVersionKey, CLIENT_VERSION);
     }
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 5ede59b82a..1c7bfa85d5 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -252,7 +252,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
         LOCK(m_node.chainman->GetMutex());
         m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
             .path = m_args.GetDataDirNet() / "blocks" / "index",
-            .cache_bytes = static_cast<size_t>(m_kernel_cache_sizes.block_tree_db),
+            .cache_bytes = m_kernel_cache_sizes.block_tree_db,
             .memory_only = true,
         });
     };
diff --git a/src/validation.h b/src/validation.h
index aea7a4621b..f75337604a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1067,11 +1067,11 @@ class ChainstateManager
 
     //! 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};
+    size_t m_total_coinstip_cache{0};
     //
     //! The total number of bytes available for us to use across all leveldb
     //! coins databases. This will be split somehow across chainstates.
-    int64_t m_total_coinsdb_cache{0};
+    size_t m_total_coinsdb_cache{0};
 
     //! Instantiate a new chainstate.
     //!

From 160c27ec078346fbf07f9b84dc10cef2b9809327 Mon Sep 17 00:00:00 2001
From: Adlai Chandrasekhar <adlai.chandrasekhar@gmail.com>
Date: Thu, 9 Jan 2025 00:41:20 +0000
Subject: [PATCH 40/42] doc: Update dependency installation for Debian/Ubuntu
 and CI

According to the description for pkg-config, "pkgconf is a
replacement for pkg-config, providing additional functionality
while also maintaining compatibility. This package only provides
a dependency link to the pkgconf package to help with package
upgrades. It can be safely removed."

Thus several scripts and markdown files are updated.
---
 .github/workflows/ci.yml | 2 +-
 ci/test/00_setup_env.sh  | 2 +-
 depends/README.md        | 2 +-
 doc/build-unix.md        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index df1d12e86c..3e9f885d13 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -67,7 +67,7 @@ jobs:
           echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD $EXCLUDE_MERGE_BASE_ANCESTORS | head -1)" >> "$GITHUB_ENV"
       - run: |
           sudo apt-get update
-          sudo apt-get install clang ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
+          sudo apt-get install clang ccache build-essential cmake pkgconf python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y
       - name: Compile and run tests
         run: |
           # Run tests on commits after the last merge commit and before the PR head commit
diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh
index 021d5e1597..d7d527c7f6 100755
--- a/ci/test/00_setup_env.sh
+++ b/ci/test/00_setup_env.sh
@@ -64,7 +64,7 @@ export BASE_OUTDIR=${BASE_OUTDIR:-$BASE_SCRATCH_DIR/out}
 # The folder for previous release binaries.
 # This folder exists only on the ci guest, and on the ci host as a volume.
 export PREVIOUS_RELEASES_DIR=${PREVIOUS_RELEASES_DIR:-$BASE_ROOT_DIR/prev_releases}
-export CI_BASE_PACKAGES=${CI_BASE_PACKAGES:-build-essential pkg-config curl ca-certificates ccache python3 rsync git procps bison e2fsprogs cmake}
+export CI_BASE_PACKAGES=${CI_BASE_PACKAGES:-build-essential pkgconf curl ca-certificates ccache python3 rsync git procps bison e2fsprogs cmake}
 export GOAL=${GOAL:-install}
 export DIR_QA_ASSETS=${DIR_QA_ASSETS:-${BASE_SCRATCH_DIR}/qa-assets}
 export CI_RETRY_EXE=${CI_RETRY_EXE:-"retry --"}
diff --git a/depends/README.md b/depends/README.md
index 5ecf16e3c4..848137f03f 100644
--- a/depends/README.md
+++ b/depends/README.md
@@ -47,7 +47,7 @@ The paths are automatically configured and no other options are needed.
 
 Skip the following packages if you don't intend to use the GUI and will build with [`NO_QT=1`](#dependency-options):
 
-    apt install bison g++ pkg-config python3 xz-utils
+    apt install bison g++ pkgconf python3 xz-utils
 
 #### For macOS cross compilation
 
diff --git a/doc/build-unix.md b/doc/build-unix.md
index 4f04b4fd9f..1e88c72550 100644
--- a/doc/build-unix.md
+++ b/doc/build-unix.md
@@ -44,7 +44,7 @@ Finally, clang (often less resource hungry) can be used instead of gcc, which is
 
 Build requirements:
 
-    sudo apt-get install build-essential cmake pkg-config python3
+    sudo apt-get install build-essential cmake pkgconf python3
 
 Now, you can either build from self-compiled [depends](#dependencies) or install the required dependencies:
 

From b30cc71e853cbaaec66e7e23d7d1a32468079ee0 Mon Sep 17 00:00:00 2001
From: Adlai Chandrasekhar <adlai.chandrasekhar@gmail.com>
Date: Thu, 16 Jan 2025 17:29:12 +0200
Subject: [PATCH 41/42] doc: fix typos

---
 src/test/miner_tests.cpp             | 2 +-
 src/txorphanage.cpp                  | 2 +-
 test/functional/wallet_encryption.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index d7bc790d0e..5bfcde7718 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -705,7 +705,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
         {
             LOCK(cs_main);
             // The above calls don't guarantee the tip is actually updated, so
-            // we explictly check this.
+            // we explicitly check this.
             auto maybe_new_tip{Assert(m_node.chainman)->ActiveChain().Tip()};
             BOOST_REQUIRE_EQUAL(maybe_new_tip->GetBlockHash(), block.GetHash());
         }
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 90b3381428..07f7fa021f 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -111,7 +111,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
         if (orphan_it != orphan.announcers.end()) {
             orphan.announcers.erase(peer);
 
-            // No remaining annnouncers: clean up entry
+            // No remaining announcers: clean up entry
             if (orphan.announcers.empty()) {
                 nErased += EraseTx(orphan.tx->GetWitnessHash());
             }
diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py
index b30a1478ab..5e131405f1 100755
--- a/test/functional/wallet_encryption.py
+++ b/test/functional/wallet_encryption.py
@@ -158,7 +158,7 @@ def do_wallet_tool(*args):
             dumpfile_path = self.nodes[0].datadir_path / "noprivs_enc.dump"
             do_wallet_tool("-wallet=noprivs_enc", f"-dumpfile={dumpfile_path}", "dump")
             with open(dumpfile_path, "r", encoding="utf-8") as f:
-                # Check theres nothing with an 'mkey' prefix
+                # Check there's nothing with an 'mkey' prefix
                 assert_equal(all([not line.startswith("046d6b6579") for line in f]), True)
 
 

From 2e75ebb6169da08b04c4769555c4c84d6b5ca0ec Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 16 Jan 2025 13:56:11 -0500
Subject: [PATCH 42/42] [test] fix p2p_orphan_handling.py empty orphanage check

It's possible getorphantxs isn't empty immediately. Prevent intermittent errors.
---
 test/functional/p2p_orphan_handling.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py
index 4a2d53cd2b..dc58a22068 100755
--- a/test/functional/p2p_orphan_handling.py
+++ b/test/functional/p2p_orphan_handling.py
@@ -59,7 +59,7 @@ def wrapper(self):
             self.nodes[0].disconnect_p2ps()
             self.nodes[0].bumpmocktime(LONG_TIME_SKIP)
             # Check that mempool and orphanage have been cleared
-            assert_equal(0, len(self.nodes[0].getorphantxs()))
+            self.wait_until(lambda: len(self.nodes[0].getorphantxs()) == 0)
             assert_equal(0, len(self.nodes[0].getrawmempool()))
             self.wallet.rescan_utxos(include_mempool=True)
     return wrapper