backport: Bitcoin Core v0.26 backports (batch 1)

doc/policy/packages.md

@@ -68,24 +68,37 @@ test accepts):
 If any transactions in the package are already in the mempool, they are not submitted again
 ("deduplicated") and are thus excluded from this calculation.
 
-To meet the two feerate requirements of a mempool, i.e., the pre-configured minimum relay feerate
-(`minRelayTxFee`) and the dynamic mempool minimum feerate, the total package feerate is used instead
-of the individual feerate. The individual transactions are allowed to be below the feerate
-requirements if the package meets the feerate requirements. For example, the parent(s) in the
-package can pay no fees but be paid for by the child.
-
-*Rationale*: This can be thought of as "CPFP within a package," solving the issue of a parent not
-meeting minimum fees on its own. This would allow contracting applications to adjust their fees at
-broadcast time instead of overshooting or risking becoming stuck or pinned.
-
-*Rationale*: It would be incorrect to use the fees of transactions that are already in the mempool, as
-we do not want a transaction's fees to be double-counted.
+To meet the dynamic mempool minimum feerate, i.e., the feerate determined by the transactions
+evicted when the mempool reaches capacity (not the static minimum relay feerate), the total package
+feerate instead of individual feerate can be used. For example, if the mempool minimum feerate is
+5sat/vB and a 1sat/vB parent transaction has a high-feerate child, it may be accepted if
+submitted as a package.
+
+*Rationale*: This can be thought of as "CPFP within a package," solving the issue of a presigned
+transaction (i.e. in which a replacement transaction with a higher fee cannot be signed) being
+rejected from the mempool when transaction volume is high and the mempool minimum feerate rises.
+
+Note: Package feerate cannot be used to meet the minimum relay feerate (`-minrelaytxfee`)
+requirement. For example, if the mempool minimum feerate is 5sat/vB and the minimum relay feerate is
+set to 5satvB, a 1sat/vB parent transaction with a high-feerate child will not be accepted, even if
+submitted as a package.
+
+*Rationale*: Avoid situations in which the mempool contains non-bumped transactions below min relay
+feerate (which we consider to have pay 0 fees and thus receiving free relay). While package
+submission would ensure these transactions are bumped at the time of entry, it is not guaranteed
+that the transaction will always be bumped. For example, a later transaction could replace the
+fee-bumping child without still bumping the parent. These no-longer-bumped transactions should be
+removed during a replacement, but we do not have a DoS-resistant way of removing them or enforcing a
+limit on their quantity. Instead, prevent their entry into the mempool.
 
 Implementation Note: Transactions within a package are always validated individually first, and
 package validation is used for the transactions that failed. Since package feerate is only
 calculated using transactions that are not in the mempool, this implementation detail affects the
 outcome of package validation.
 
+*Rationale*: It would be incorrect to use the fees of transactions that are already in the mempool, as
+we do not want a transaction's fees to be double-counted.
+
 *Rationale*: We must not allow a low-feerate child to prevent its parent from being accepted; fees
 of children should not negatively impact their parents, since they are not necessary for the parents
 to be mined. More generally, if transaction B is not needed in order for transaction A to be mined,

doc/release-notes-26076.md

@@ -0,0 +1,13 @@
+RPC
+---
+
+- The `listdescriptors`, `decodepsbt` and similar RPC methods now show `h` rather than apostrophe (`'`) to indicate
+  hardened derivation. This does not apply when using the `private` parameter, which
+  matches the marker used when descriptor was generated or imported. Newly created
+  wallets use `h`. This change makes it easier to handle descriptor strings manually.
+  E.g. the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`.
+  With this change `listdescriptors` will use `h`, so you can copy-paste the result,
+  without having to add escape characters or switch `'` to 'h' manually.
+  Note that this changes the descriptor checksum.
+  For legacy wallets the `hdkeypath` field in `getaddressinfo` is unchanged,
+  nor is the serialization format of wallet dumps. (#26076)

doc/release-process.md

@@ -36,6 +36,8 @@ Before every major release:
     `nBlocks` of 4096 (28 days) and a `bestblockhash` of RPC `getbestblockhash`; see
     [this pull request](https://github.com/dashpay/dash/pull/5692) for an example. Reviewers can verify the results by running
     `getchaintxstats <window_block_count> <window_final_block_hash>` with the `window_block_count` and `window_final_block_hash` from your output.
+* [ ] Prune inputs from the qa-assets repo (See [pruning
+  inputs](https://github.com/bitcoin-core/qa-assets#pruning-inputs)).
 
 ### First time / New builders
 

src/Makefile.am

@@ -270,6 +270,7 @@ BITCOIN_CORE_H = \
   instantsend/lock.h \
   instantsend/net_instantsend.h \
   instantsend/signing.h \
+  kernel/blockmanager_opts.h \
   kernel/coinstats.h \
   key.h \
   key_io.h \

src/Makefile.test.include

@@ -376,6 +376,7 @@ test_fuzz_fuzz_SOURCES = \
  test/fuzz/tx_pool.cpp \
  test/fuzz/txorphan.cpp \
  test/fuzz/utxo_snapshot.cpp \
+ test/fuzz/utxo_total_supply.cpp \
  test/fuzz/validation_load_mempool.cpp \
  test/fuzz/versionbits.cpp
 endif # ENABLE_FUZZ_BINARY

src/bench/block_assemble.cpp

@@ -32,7 +32,7 @@ static void AssembleBlock(benchmark::Bench& bench)
     std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
     for (size_t b{0}; b < NUM_BLOCKS; ++b) {
         CMutableTransaction tx;
-        tx.vin.push_back(MineBlock(test_setup->m_node, SCRIPT_PUB));
+        tx.vin.emplace_back(MineBlock(test_setup->m_node, SCRIPT_PUB));
         tx.vin.back().scriptSig = scriptSig;
         tx.vout.emplace_back(1337, SCRIPT_PUB);
         if (NUM_BLOCKS - b >= COINBASE_MATURITY)

src/bench/coin_selection.cpp

@@ -5,6 +5,7 @@
 #include <bench/bench.h>
 #include <interfaces/chain.h>
 #include <node/context.h>
+#include <policy/policy.h>
 #include <wallet/coinselection.h>
 #include <wallet/spend.h>
 #include <wallet/wallet.h>
@@ -111,7 +112,7 @@ static void BnBExhaustion(benchmark::Bench& bench)
     bench.run([&] {
         // Benchmark
         CAmount target = make_hard_case(17, utxo_pool);
-        SelectCoinsBnB(utxo_pool, target, 0); // Should exhaust
+        SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_SIZE); // Should exhaust
 
         // Cleanup
         utxo_pool.clear();

src/kernel/blockmanager_opts.h

@@ -0,0 +1,22 @@
+// 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.
+
+#ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
+#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
+
+class CChainParams;
+
+namespace kernel {
+
+/**
+ * An options struct for `BlockManager`, more ergonomically referred to as
+ * `BlockManager::Options` due to the using-declaration in `BlockManager`.
+ */
+struct BlockManagerOpts {
+    const CChainParams& chainparams;
+};
+
+} // namespace kernel
+
+#endif // BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H

src/kernel/coinstats.cpp

@@ -107,7 +107,7 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
     uint256 prevkey;
     std::map<uint32_t, Coin> outputs;
     while (pcursor->Valid()) {
-        interruption_point();
+        if (interruption_point) interruption_point();
         COutPoint key;
         Coin coin;
         if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {

src/net_processing.cpp

@@ -1039,8 +1039,11 @@ class PeerManagerImpl final : public PeerManager
     /** Remove this block from our tracked requested blocks. Called if:
      *  - the block has been recieved from a peer
      *  - the request for the block has timed out
+     * If "from_peer" is specified, then only remove the block if it is in
+     * flight from that peer (to avoid one peer's network traffic from
+     * affecting another's state).
      */
-    void RemoveBlockRequest(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    void RemoveBlockRequest(const uint256& hash, std::optional<NodeId> from_peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
     /* Mark a block as in flight
      * Returns false, still setting pit, if the block was already in flight from the same peer
@@ -1238,7 +1241,7 @@ bool PeerManagerImpl::IsBlockRequested(const uint256& hash)
     return mapBlocksInFlight.find(hash) != mapBlocksInFlight.end();
 }
 
-void PeerManagerImpl::RemoveBlockRequest(const uint256& hash)
+void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional<NodeId> from_peer)
 {
     auto it = mapBlocksInFlight.find(hash);
     if (it == mapBlocksInFlight.end()) {
@@ -1247,6 +1250,12 @@ void PeerManagerImpl::RemoveBlockRequest(const uint256& hash)
     }
 
     auto [node_id, list_it] = it->second;
+
+    if (from_peer && node_id != *from_peer) {
+        // Block was requested by another peer
+        return;
+    }
+
     CNodeState *state = State(node_id);
     assert(state != nullptr);
 
@@ -1282,7 +1291,7 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st
     }
 
     // Make sure it's not listed somewhere already.
-    RemoveBlockRequest(hash);
+    RemoveBlockRequest(hash, std::nullopt);
 
     std::list<QueuedBlock>::iterator it = state->vBlocksInFlight.insert(state->vBlocksInFlight.end(),
             {&block, std::unique_ptr<PartiallyDownloadedBlock>(pit ? new PartiallyDownloadedBlock(&m_mempool) : nullptr)});
@@ -3623,6 +3632,11 @@ void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlo
     m_chainman.ProcessNewBlock(block, force_processing, &new_block);
     if (new_block) {
         node.m_last_block_time = GetTime<std::chrono::seconds>();
+        // In case this block came from a different peer than we requested
+        // from, we can erase the block request now anyway (as we just stored
+        // this block to disk).
+        LOCK(cs_main);
+        RemoveBlockRequest(block->GetHash(), std::nullopt);
     } else {
         LOCK(cs_main);
         mapBlockSource.erase(block->GetHash());
@@ -4920,7 +4934,7 @@ void PeerManagerImpl::ProcessMessage(
                 PartiallyDownloadedBlock& partialBlock = *(*queuedBlockIt)->partialBlock;
                 ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact);
                 if (status == READ_STATUS_INVALID) {
-                    RemoveBlockRequest(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect
+                    RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect
                     Misbehaving(pfrom.GetId(), 100, "invalid compact block");
                     return;
                 } else if (status == READ_STATUS_FAILED) {
@@ -5014,7 +5028,7 @@ void PeerManagerImpl::ProcessMessage(
                 // process from some other peer.  We do this after calling
                 // ProcessNewBlock so that a malleated cmpctblock announcement
                 // can't be used to interfere with block relay.
-                RemoveBlockRequest(pblock->GetHash());
+                RemoveBlockRequest(pblock->GetHash(), std::nullopt);
             }
         }
         return;
@@ -5046,7 +5060,7 @@ void PeerManagerImpl::ProcessMessage(
             PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock;
             ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn);
             if (status == READ_STATUS_INVALID) {
-                RemoveBlockRequest(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect
+                RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect
                 Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions");
                 return;
             } else if (status == READ_STATUS_FAILED) {
@@ -5072,7 +5086,7 @@ void PeerManagerImpl::ProcessMessage(
                 // though the block was successfully read, and rely on the
                 // handling in ProcessNewBlock to ensure the block index is
                 // updated, etc.
-                RemoveBlockRequest(resp.blockhash); // it is now an empty pointer
+                RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // it is now an empty pointer
                 fBlockRead = true;
                 // mapBlockSource is used for potentially punishing peers and
                 // updating which peers send us compact blocks, so the race
@@ -5154,7 +5168,7 @@ void PeerManagerImpl::ProcessMessage(
             // Always process the block if we requested it, since we may
             // need it even when it's not a candidate for a new best tip.
             forceProcessing = IsBlockRequested(hash);
-            RemoveBlockRequest(hash);
+            RemoveBlockRequest(hash, pfrom.GetId());
             // mapBlockSource is only used for punishing peers and setting
             // which peers send us compact blocks, so the race between here and
             // cs_main in ProcessNewBlock is fine.

src/netbase.cpp

@@ -35,8 +35,8 @@ static Proxy nameProxy GUARDED_BY(g_proxyinfo_mutex);
 int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT;
 bool fNameLookup = DEFAULT_NAME_LOOKUP;
 
-// Need ample time for negotiation for very slow proxies such as Tor (milliseconds)
-int g_socks5_recv_timeout = 20 * 1000;
+// Need ample time for negotiation for very slow proxies such as Tor
+std::chrono::milliseconds g_socks5_recv_timeout = 20s;
 static std::atomic<bool> interruptSocks5Recv(false);
 
 ReachableNets g_reachable_nets;
@@ -300,7 +300,7 @@ enum class IntrRecvError {
  *
  * @param data The buffer where the read bytes should be stored.
  * @param len The number of bytes to read into the specified buffer.
- * @param timeout The total timeout in milliseconds for this read.
+ * @param timeout The total timeout for this read.
  * @param sock The socket (has to be in non-blocking mode) from which to read bytes.
  *
  * @returns An IntrRecvError indicating the resulting status of this read.
@@ -310,10 +310,10 @@ enum class IntrRecvError {
  * @see This function can be interrupted by calling InterruptSocks5(bool).
  *      Sockets can be made non-blocking with Sock::SetNonBlocking().
  */
-static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, int timeout, const Sock& sock)
+static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, std::chrono::milliseconds timeout, const Sock& sock)
 {
-    int64_t curTime = TicksSinceEpoch<std::chrono::milliseconds>(SystemClock::now());
-    int64_t endTime = curTime + timeout;
+    auto curTime{Now<SteadyMilliseconds>()};
+    const auto endTime{curTime + timeout};
     while (len > 0 && curTime < endTime) {
         ssize_t ret = sock.Recv(data, len, 0); // Optimistically try the recv first
         if (ret > 0) {
@@ -337,7 +337,7 @@ static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, int timeout, c
         }
         if (interruptSocks5Recv)
             return IntrRecvError::Interrupted;
-        curTime = TicksSinceEpoch<std::chrono::milliseconds>(SystemClock::now());
+        curTime = Now<SteadyMilliseconds>();
     }
     return len == 0 ? IntrRecvError::OK : IntrRecvError::Timeout;
 }