Merge bitcoin/bitcoin#31104: [28.x] Backports & 28.1rc1v28.1rc1

8fef83a0a03f884e0c5399b318eb55064b84b718 doc: update manual pages for 28.1rc1 (fanquake)
df7764621e2d41e2dd35e10f68f0059c0e09bf18 build: bump version to 28.1rc1 (fanquake)
9add853b652ac37214011909e7777772ec6256fa doc: update release notes for 28.1rc1 (fanquake)
1025090fbe215286239ff6dd5dbedd3291f0b196 build: disable compiling fuzz/utxo_snapshot.cpp with MSVC (fanquake)
446f5d20d6c8da951c97e5a5e52c92b0fab4728d refactor: Drop deprecated space in operator""_mst (MarcoFalke)
9976162a0e0847502c075ae8524b6e5cfefba0ed addrman: change nid_type from int to int64_t (Martin Zumsande)
1d0411dc8fea96eb4d65ffef98d71f6cc4e12af5 addrman, refactor: introduce user-defined type for internal nId (Martin Zumsande)
7fec6382221b49b7ffd5121ee0442b4f2a5169ef depends: For mingw cross compile use -gcc-posix to prevent library conflict (laanwj)
f998ac628685c6459d5de3bd7469f9046901fc47 key: clear out secret data in `DecodeExtKey` (Sebastian Falbesoner)
0773560abf991fbc60c415e1556d86c4d5505f1b ci: add LLVM_SYMBOLIZER_PATH to Valgrind fuzz job (fanquake)
b9173342084b1340a639e7a20e2b4a0f5852304a test: add missing sync to feature_fee_estimation.py (Martin Zumsande)
f072721181c727f37cf1253e0a629f865eae7178 doc: add testnet4 section header for config file (Marnix)
6643fd2145f8ef211e719dd401b8ac966c5dc9ea doc: Archive 28.0 release notes (Ava Chow)

Pull request description:

  Backports:
  * #30568
  * #31007
  * #31013
  * #31016
  * #31035
  * #31166

  Contains:
  * A commit to do the same as #31307.

ACKs for top commit:
  willcl-ark:
    ACK 8fef83a0a03f884e0c5399b318eb55064b84b718

Tree-SHA512: 58f0c6cb9e5b7ac17ad20141acdc5423dbe8e79cc3a2cf1c4e503d289b75940632c9838c64e3ac733b1a55e65723fc1071ccdd9a860a710256cc88e29f42ccdb

8 files changed, 73 insertions, 54 deletions

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 054a9bee32..11ae49cfad 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -188,7 +188,7 @@ void AddrManImpl::Serialize(Stream& s_) const
 
     int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
     s << nUBuckets;
-    std::unordered_map<int, int> mapUnkIds;
+    std::unordered_map<nid_type, int> mapUnkIds;
     int nIds = 0;
     for (const auto& entry : mapInfo) {
         mapUnkIds[entry.first] = nIds;
@@ -398,7 +398,7 @@ void AddrManImpl::Unserialize(Stream& s_)
     }
 }
 
-AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId)
+AddrInfo* AddrManImpl::Find(const CService& addr, nid_type* pnId)
 {
     AssertLockHeld(cs);
 
@@ -413,11 +413,11 @@ AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId)
     return nullptr;
 }
 
-AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId)
+AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource, nid_type* pnId)
 {
     AssertLockHeld(cs);
 
-    int nId = nIdCount++;
+    nid_type nId = nIdCount++;
     mapInfo[nId] = AddrInfo(addr, addrSource);
     mapAddr[addr] = nId;
     mapInfo[nId].nRandomPos = vRandom.size();
@@ -438,8 +438,8 @@ void AddrManImpl::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) const
 
     assert(nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size());
 
-    int nId1 = vRandom[nRndPos1];
-    int nId2 = vRandom[nRndPos2];
+    nid_type nId1 = vRandom[nRndPos1];
+    nid_type nId2 = vRandom[nRndPos2];
 
     const auto it_1{mapInfo.find(nId1)};
     const auto it_2{mapInfo.find(nId2)};
@@ -453,7 +453,7 @@ void AddrManImpl::SwapRandom(unsigned int nRndPos1, unsigned int nRndPos2) const
     vRandom[nRndPos2] = nId1;
 }
 
-void AddrManImpl::Delete(int nId)
+void AddrManImpl::Delete(nid_type nId)
 {
     AssertLockHeld(cs);
 
@@ -476,7 +476,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos)
 
     // if there is an entry in the specified bucket, delete it.
     if (vvNew[nUBucket][nUBucketPos] != -1) {
-        int nIdDelete = vvNew[nUBucket][nUBucketPos];
+        nid_type nIdDelete = vvNew[nUBucket][nUBucketPos];
         AddrInfo& infoDelete = mapInfo[nIdDelete];
         assert(infoDelete.nRefCount > 0);
         infoDelete.nRefCount--;
@@ -488,7 +488,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos)
     }
 }
 
-void AddrManImpl::MakeTried(AddrInfo& info, int nId)
+void AddrManImpl::MakeTried(AddrInfo& info, nid_type nId)
 {
     AssertLockHeld(cs);
 
@@ -515,7 +515,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
     // first make space to add it (the existing tried entry there is moved to new, deleting whatever is there).
     if (vvTried[nKBucket][nKBucketPos] != -1) {
         // find an item to evict
-        int nIdEvict = vvTried[nKBucket][nKBucketPos];
+        nid_type nIdEvict = vvTried[nKBucket][nKBucketPos];
         assert(mapInfo.count(nIdEvict) == 1);
         AddrInfo& infoOld = mapInfo[nIdEvict];
 
@@ -554,7 +554,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, std::c
     if (!addr.IsRoutable())
         return false;
 
-    int nId;
+    nid_type nId;
     AddrInfo* pinfo = Find(addr, &nId);
 
     // Do not set a penalty for a source's self-announcement
@@ -627,7 +627,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, NodeSecond
 {
     AssertLockHeld(cs);
 
-    int nId;
+    nid_type nId;
 
     m_last_good = time;
 
@@ -753,7 +753,8 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool new_only, std::option
 
         // Iterate over the positions of that bucket, starting at the initial one,
         // and looping around.
-        int i, position, node_id;
+        int i, position;
+        nid_type node_id;
         for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) {
             position = (initial_position + i) % ADDRMAN_BUCKET_SIZE;
             node_id = GetEntry(search_tried, bucket, position);
@@ -786,7 +787,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool new_only, std::option
     }
 }
 
-int AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const
+nid_type AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const
 {
     AssertLockHeld(cs);
 
@@ -849,7 +850,7 @@ std::vector<std::pair<AddrInfo, AddressPosition>> AddrManImpl::GetEntries_(bool
     std::vector<std::pair<AddrInfo, AddressPosition>> infos;
     for (int bucket = 0; bucket < bucket_count; ++bucket) {
         for (int position = 0; position < ADDRMAN_BUCKET_SIZE; ++position) {
-            int id = GetEntry(from_tried, bucket, position);
+            nid_type id = GetEntry(from_tried, bucket, position);
             if (id >= 0) {
                 AddrInfo info = mapInfo.at(id);
                 AddressPosition location = AddressPosition(
@@ -904,8 +905,8 @@ void AddrManImpl::ResolveCollisions_()
 {
     AssertLockHeld(cs);
 
-    for (std::set<int>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) {
-        int id_new = *it;
+    for (std::set<nid_type>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) {
+        nid_type id_new = *it;
 
         bool erase_collision = false;
 
@@ -923,7 +924,7 @@ void AddrManImpl::ResolveCollisions_()
             } else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty
 
                 // Get the to-be-evicted address that is being tested
-                int id_old = vvTried[tried_bucket][tried_bucket_pos];
+                nid_type id_old = vvTried[tried_bucket][tried_bucket_pos];
                 AddrInfo& info_old = mapInfo[id_old];
 
                 const auto current_time{Now<NodeSeconds>()};
@@ -969,11 +970,11 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::SelectTriedCollision_()
 
     if (m_tried_collisions.size() == 0) return {};
 
-    std::set<int>::iterator it = m_tried_collisions.begin();
+    std::set<nid_type>::iterator it = m_tried_collisions.begin();
 
     // Selects a random element from m_tried_collisions
     std::advance(it, insecure_rand.randrange(m_tried_collisions.size()));
-    int id_new = *it;
+    nid_type id_new = *it;
 
     // If id_new not found in mapInfo remove it from m_tried_collisions
     if (mapInfo.count(id_new) != 1) {
@@ -1058,15 +1059,15 @@ int AddrManImpl::CheckAddrman() const
     LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(
         strprintf("new %i, tried %i, total %u", nNew, nTried, vRandom.size()), BCLog::ADDRMAN);
 
-    std::unordered_set<int> setTried;
-    std::unordered_map<int, int> mapNew;
+    std::unordered_set<nid_type> setTried;
+    std::unordered_map<nid_type, int> mapNew;
     std::unordered_map<Network, NewTriedCount> local_counts;
 
     if (vRandom.size() != (size_t)(nTried + nNew))
         return -7;
 
     for (const auto& entry : mapInfo) {
-        int n = entry.first;
+        nid_type n = entry.first;
         const AddrInfo& info = entry.second;
         if (info.fInTried) {
             if (!TicksSinceEpoch<std::chrono::seconds>(info.m_last_success)) {
diff --git a/src/addrman_impl.h b/src/addrman_impl.h
index dd7f7b318f..a3246da0ee 100644
--- a/src/addrman_impl.h
+++ b/src/addrman_impl.h
@@ -33,6 +33,13 @@ static constexpr int32_t ADDRMAN_BUCKET_SIZE_LOG2{6};
 static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2};
 
 /**
+ * User-defined type for the internally used nIds
+ * This used to be int, making it feasible for attackers to cause an overflow,
+ * see https://bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow/
+ */
+using nid_type = int64_t;
+
+/**
  * Extended statistics about a CAddress
  */
 class AddrInfo : public CAddress
@@ -179,36 +186,36 @@ private:
     static constexpr uint8_t INCOMPATIBILITY_BASE = 32;
 
     //! last used nId
-    int nIdCount GUARDED_BY(cs){0};
+    nid_type nIdCount GUARDED_BY(cs){0};
 
     //! table with information about all nIds
-    std::unordered_map<int, AddrInfo> mapInfo GUARDED_BY(cs);
+    std::unordered_map<nid_type, AddrInfo> mapInfo GUARDED_BY(cs);
 
     //! find an nId based on its network address and port.
-    std::unordered_map<CService, int, CServiceHash> mapAddr GUARDED_BY(cs);
+    std::unordered_map<CService, nid_type, CServiceHash> mapAddr GUARDED_BY(cs);
 
     //! randomly-ordered vector of all nIds
     //! This is mutable because it is unobservable outside the class, so any
     //! changes to it (even in const methods) are also unobservable.
-    mutable std::vector<int> vRandom GUARDED_BY(cs);
+    mutable std::vector<nid_type> vRandom GUARDED_BY(cs);
 
     // number of "tried" entries
     int nTried GUARDED_BY(cs){0};
 
     //! list of "tried" buckets
-    int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
+    nid_type vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
 
     //! number of (unique) "new" entries
     int nNew GUARDED_BY(cs){0};
 
     //! list of "new" buckets
-    int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
+    nid_type vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs);
 
     //! last time Good was called (memory only). Initially set to 1 so that "never" is strictly worse.
     NodeSeconds m_last_good GUARDED_BY(cs){1s};
 
     //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions.
-    std::set<int> m_tried_collisions;
+    std::set<nid_type> m_tried_collisions;
 
     /** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */
     const int32_t m_consistency_check_ratio;
@@ -225,22 +232,22 @@ private:
     std::unordered_map<Network, NewTriedCount> m_network_counts GUARDED_BY(cs);
 
     //! Find an entry.
-    AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    AddrInfo* Find(const CService& addr, nid_type* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom.
-    AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, nid_type* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Swap two elements in vRandom.
     void SwapRandom(unsigned int nRandomPos1, unsigned int nRandomPos2) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Delete an entry. It must not be in tried, and have refcount 0.
-    void Delete(int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    void Delete(nid_type nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Clear a position in a "new" table. This is the only place where entries are actually deleted.
     void ClearNew(int nUBucket, int nUBucketPos) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Move an entry from the "new" table(s) to the "tried" table
-    void MakeTried(AddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    void MakeTried(AddrInfo& info, nid_type nId) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Attempt to add a single address to addrman's new table.
      *  @see AddrMan::Add() for parameters. */
@@ -256,9 +263,9 @@ private:
 
     /** Helper to generalize looking up an addrman entry from either table.
      *
-     *  @return  int The nid of the entry. If the addrman position is empty or not found, returns -1.
+     *  @return  nid_type The nid of the entry. If the addrman position is empty or not found, returns -1.
      * */
-    int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    nid_type GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     std::vector<CAddress> GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
diff --git a/src/key_io.cpp b/src/key_io.cpp
index 29002afc45..6cece47e41 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -274,6 +274,9 @@ CExtKey DecodeExtKey(const std::string& str)
             key.Decode(data.data() + prefix.size());
         }
     }
+    if (!data.empty()) {
+        memory_cleanse(data.data(), data.size());
+    }
     return key;
 }
 
diff --git a/src/script/miniscript.cpp b/src/script/miniscript.cpp
index 455bd56283..4b8d3673f9 100644
--- a/src/script/miniscript.cpp
+++ b/src/script/miniscript.cpp
@@ -1,14 +1,17 @@
-// Copyright (c) 2019-2022 The Bitcoin Core developers
+// Copyright (c) 2019-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 <string>
+#include <limits>
 #include <vector>
-#include <script/script.h>
-#include <script/miniscript.h>
-#include <serialize.h>
 
-#include <assert.h>
+#include <primitives/transaction.h>
+#include <script/miniscript.h>
+#include <script/script.h>
+#include <script/solver.h>
+#include <span.h>
+#include <util/check.h>
+#include <util/vector.h>
 
 namespace miniscript {
 namespace internal {
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 58f24434f0..1ab9fdbbca 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2019-2022 The Bitcoin Core developers
+// Copyright (c) 2019-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.
 
@@ -6,20 +6,24 @@
 #define BITCOIN_SCRIPT_MINISCRIPT_H
 
 #include <algorithm>
-#include <functional>
-#include <numeric>
+#include <compare>
+#include <cstdint>
+#include <cstdlib>
+#include <iterator>
 #include <memory>
 #include <optional>
-#include <string>
+#include <set>
+#include <stdexcept>
+#include <tuple>
+#include <utility>
 #include <vector>
 
-#include <assert.h>
-#include <cstdlib>
-
+#include <consensus/consensus.h>
 #include <policy/policy.h>
-#include <primitives/transaction.h>
+#include <script/interpreter.h>
 #include <script/parsing.h>
 #include <script/script.h>
+#include <serialize.h>
 #include <span.h>
 #include <util/check.h>
 #include <util/strencodings.h>
@@ -150,7 +154,8 @@ public:
 };
 
 //! Literal operator to construct Type objects.
-inline consteval Type operator"" _mst(const char* c, size_t l) {
+inline consteval Type operator""_mst(const char* c, size_t l)
+{
     Type typ{Type::Make(0)};
 
     for (const char *p = c; p < c + l; p++) {
diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp
index dbec2bc858..f5310c45f5 100644
--- a/src/test/fuzz/addrman.cpp
+++ b/src/test/fuzz/addrman.cpp
@@ -186,7 +186,7 @@ public:
             return false;
         }
 
-        auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) {
+        auto IdsReferToSameAddress = [&](nid_type id, nid_type other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) {
             if (id == -1 && other_id == -1) {
                 return true;
             }
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 1f9ed9a064..4ad3aba4ef 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -20,7 +20,7 @@ using NodeRef = miniscript::NodeRef<CPubKey>;
 using Node = miniscript::Node<CPubKey>;
 using Type = miniscript::Type;
 using MsCtx = miniscript::MiniscriptContext;
-using miniscript::operator"" _mst;
+using miniscript::operator""_mst;
 
 //! Some pre-computed data for more efficient string roundtrips and to simulate challenges.
 struct TestData {
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 815c278b8c..4534f34296 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -290,7 +290,7 @@ public:
 
 using Fragment = miniscript::Fragment;
 using NodeRef = miniscript::NodeRef<CPubKey>;
-using miniscript::operator"" _mst;
+using miniscript::operator""_mst;
 using Node = miniscript::Node<CPubKey>;
 
 /** Compute all challenges (pubkeys, hashes, timelocks) that occur in a given Miniscript. */