Merge #6284: backport: bitcoin#21713, #21856, #22061, #22122, #22172,…

… #22261, #22381, #22445, #22447

8f06ac9 Merge bitcoin/bitcoin#22172: doc: update tor.md, release notes with removal of tor v2 support (W. J. van der Laan)
9b22501 Merge bitcoin/bitcoin#22122: ci: Bump macOS image to big-sur-xcode-12.5 (MarcoFalke)
3b05a99 Merge bitcoin/bitcoin#22106: refactor: address ProcessNewBlock comments from #21713 (fanquake)
c872556 Merge bitcoin/bitcoin#21856: doc: add OSS-Fuzz section to fuzzing.md doc (MarcoFalke)
facf685 Merge bitcoin/bitcoin#22261: [p2p/mempool] Two small fixes to node broadcast logic (fanquake)
1430897 Merge bitcoin/bitcoin#22445: fuzz: Move implementations of non-template fuzz helpers from util.h to util.cpp (MarcoFalke)
f0c62d5 Merge bitcoin/bitcoin#22447: test: whitelist rpc_rawtransaction peers to speed up tests (fanquake)
b609514 Merge #22381: guix: Test security-check sanity before performing them (Carl Dong)
9ef68d1 Merge bitcoin/bitcoin#22061: ci: Bump multiprocess memory (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Regular backports from bitcoin v22

  ## What was done?
  See commits

  ## How Has This Been Tested?
  Run unit and functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 8f06ac9
  PastaPastaPasta:
    utACK 8f06ac9

Tree-SHA512: f800b7ca8d357f2d02ce5cb1fc4951c2765242676c5494efd5e22e8f6d41d889e1fa2f888930f72aded75813413c6488d8a7d96baa2cf4820e3461464708658e

.cirrus.yml

@@ -139,7 +139,10 @@ task:
   << : *GLOBAL_TASK_TEMPLATE
   container:
     image: ubuntu:focal
+    cpu: 4
+    memory: 16G  # The default memory is sometimes just a bit too small, so double everything
   env:
+    MAKEJOBS: "-j8"
     FILE_ENV: "./ci/test/00_setup_env_native_multiprocess.sh"
 
 task:
@@ -161,12 +164,11 @@ task:
 task:
   name: 'macOS 11 native [gui] [no depends]'
   brew_install_script:
-    - brew update
     - brew install boost libevent berkeley-db4 qt@5 miniupnpc ccache zeromq qrencode sqlite libtool automake pkg-config gnu-getopt
   << : *GLOBAL_TASK_TEMPLATE
   macos_instance:
     # Use latest image, but hardcode version to avoid silent upgrades (and breaks)
-    image: big-sur-xcode-12.4  # https://cirrus-ci.org/guide/macOS
+    image: big-sur-xcode-12.5  # https://cirrus-ci.org/guide/macOS
   env:
     DANGER_RUN_CI_ON_HOST: "true"
     CI_USE_APT_INSTALL: "no"

doc/fuzzing.md

@@ -259,3 +259,9 @@ $ honggfuzz/honggfuzz --exit_upon_crash --quiet --timeout 4 -n 1 -Q \
                        -nodebuglogfile -bind=127.0.0.1:18444 -logthreadnames \
                        -debug
 ```
+
+# OSS-Fuzz
+
+Bitcoin Core participates in Google's [OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/bitcoin-core)
+program, which includes a dashboard of [publicly disclosed vulnerabilities](https://bugs.chromium.org/p/oss-fuzz/issues/list).
+For more details, see [Bitcoin's OSS-fuzz](https://github.com/bitcoin/bitcoin/tree/master/doc/fuzzing.md)

doc/tor.md

@@ -8,6 +8,14 @@ may not. In particular, the Tor Browser Bundle defaults to listening on port 915
 See [Tor Project FAQ:TBBSocksPort](https://www.torproject.org/docs/faq.html.en#TBBSocksPort)
 for how to properly configure Tor.
 
+## Compatibility
+
+- Starting with version 20.0, Dash Core only supports Tor version 3 hidden
+  services (Tor v3). Tor v2 addresses are ignored by Dash Core and neither
+  relayed nor stored.
+
+- Tor removed v2 support beginning with version 0.4.6.
+
 ## How to see information about your Tor configuration via Dash Core
 
 There are several ways to see your local onion address in Dash Core:

src/net_processing.cpp

@@ -936,7 +936,8 @@ class PeerManagerImpl final : public PeerManager
 
     void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc) LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex);
 
-    void ProcessBlock(CNode& pfrom, const std::shared_ptr<const CBlock>& pblock, bool fForceProcessing);
+    /** Process a new block. Perform any post-processing housekeeping */
+    void ProcessBlock(CNode& from, const std::shared_ptr<const CBlock>& pblock, bool force_processing);
 
     /** Relay map (txid -> CTransactionRef) */
     typedef std::map<uint256, CTransactionRef> MapRelay;
@@ -3291,15 +3292,15 @@ std::pair<bool /*ret*/, bool /*do_return*/> static ValidateDSTX(CDeterministicMN
     return {true, false};
 }
 
-void PeerManagerImpl::ProcessBlock(CNode& pfrom, const std::shared_ptr<const CBlock>& pblock, bool fForceProcessing)
+void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing)
 {
-    bool fNewBlock = false;
-    m_chainman.ProcessNewBlock(m_chainparams, pblock, fForceProcessing, &fNewBlock);
-    if (fNewBlock) {
-        pfrom.m_last_block_time = GetTime<std::chrono::seconds>();
+    bool new_block{false};
+    m_chainman.ProcessNewBlock(m_chainparams, block, force_processing, &new_block);
+    if (new_block) {
+        node.m_last_block_time = GetTime<std::chrono::seconds>();
     } else {
         LOCK(cs_main);
-        mapBlockSource.erase(pblock->GetHash());
+        mapBlockSource.erase(block->GetHash());
     }
 }
 
@@ -4491,7 +4492,7 @@ void PeerManagerImpl::ProcessMessage(
                 LOCK(cs_main);
                 mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom.GetId(), false));
             }
-            // Setting fForceProcessing to true means that we bypass some of
+            // Setting force_processing to true means that we bypass some of
             // our anti-DoS protections in AcceptBlock, which filters
             // unrequested blocks that might be trying to waste our resources
             // (eg disk space). Because we only try to reconstruct blocks when
@@ -4500,7 +4501,7 @@ void PeerManagerImpl::ProcessMessage(
             // we have a chain with at least nMinimumChainWork), and we ignore
             // compact blocks with less work than our tip, it is safe to treat
             // reconstructed compact blocks as having been requested.
-            ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true);
+            ProcessBlock(pfrom, pblock, /*force_processing=*/true);
             LOCK(cs_main); // hold cs_main for CBlockIndex::IsValid()
             if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS)) {
                 // Clear download state for this block, which is in
@@ -4583,7 +4584,7 @@ void PeerManagerImpl::ProcessMessage(
             // disk-space attacks), but this should be safe due to the
             // protections in the compact block handler -- see related comment
             // in compact block optimistic reconstruction handling.
-            ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true);
+            ProcessBlock(pfrom, pblock, /*force_processing=*/true);
         }
         return;
     }

src/node/transaction.cpp

@@ -31,34 +31,42 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str
 
 TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, bilingual_str& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback, bool bypass_limits)
 {
-    // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs.
-    // node.peerman is assigned both before chain clients and before RPC server is accepting calls,
-    // and reset after chain clients and RPC sever are stopped. node.peerman should never be null here.
-    assert(node.peerman);
+    // BroadcastTransaction can be called by either sendrawtransaction RPC or the wallet.
+    // chainman, mempool and peerman are initialized before the RPC server and wallet are started
+    // and reset after the RPC sever and wallet are stopped.
+    assert(node.chainman);
     assert(node.mempool);
+    assert(node.peerman);
+
     std::promise<void> promise;
-    uint256 hashTx = tx->GetHash();
+    uint256 txid = tx->GetHash();
     bool callback_set = false;
 
-    { // cs_main scope
-    assert(node.chainman);
-    LOCK(cs_main);
-    // If the transaction is already confirmed in the chain, don't do anything
-    // and return early.
-    CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip();
-    for (size_t o = 0; o < tx->vout.size(); o++) {
-        const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o));
-        // IsSpent does not mean the coin is spent, it means the output does not exist.
-        // So if the output does exist, then this transaction exists in the chain.
-        if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN;
-    }
-    if (!node.mempool->exists(hashTx)) {
-        // Transaction is not already in the mempool.
-        if (max_tx_fee > 0) {
-            // First, call ATMP with test_accept and check the fee. If ATMP
-            // fails here, return error immediately.
-            const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx,
-                                                                  bypass_limits, true /* test_accept */);
+    {
+        LOCK(cs_main);
+
+            // If the transaction is already confirmed in the chain, don't do anything
+        // and return early.
+        CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip();
+        for (size_t o = 0; o < tx->vout.size(); o++) {
+            const Coin& existingCoin = view.AccessCoin(COutPoint(txid, o));
+            // IsSpent does not mean the coin is spent, it means the output does not exist.
+            // So if the output does exist, then this transaction exists in the chain.
+            if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN;
+        }
+        if (auto mempool_tx = node.mempool->get(txid); mempool_tx) {
+            // There's already a transaction in the mempool with this txid. Don't
+            // try to submit this transaction to the mempool (since it'll be
+            // rejected as a TX_CONFLICT), but do attempt to reannounce the mempool
+            // transaction if relay=true.
+            //
+        } else {
+            // Transaction is not already in the mempool.
+            if (max_tx_fee > 0) {
+                // First, call ATMP with test_accept and check the fee. If ATMP
+                // fails here, return error immediately.
+                const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx,
+                                                                      bypass_limits, true /* test_accept */);
             if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
                 return HandleATMPError(result.m_state, err_string.original);
             } else if (result.m_base_fees.value() > max_tx_fee) {
@@ -68,28 +76,33 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         // Try to submit the transaction to the mempool.
         const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx,
                                                               bypass_limits, false /* test_accept */);
-        if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
-            return HandleATMPError(result.m_state, err_string.original);
-        }
+            if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
+                return HandleATMPError(result.m_state, err_string.original);
+            }
 
-        // Transaction was accepted to the mempool.
+            // Transaction was accepted to the mempool.
 
-        if (wait_callback) {
-            // For transactions broadcast from outside the wallet, make sure
-            // that the wallet has been notified of the transaction before
-            // continuing.
-            //
-            // This prevents a race where a user might call sendrawtransaction
-            // with a transaction to/from their wallet, immediately call some
-            // wallet RPC, and get a stale result because callbacks have not
-            // yet been processed.
-            CallFunctionInValidationInterfaceQueue([&promise] {
-                promise.set_value();
-            });
-            callback_set = true;
-        }
-    }
+            if (relay) {
+                // the mempool tracks locally submitted transactions to make a
+                // best-effort of initial broadcast
+                node.mempool->AddUnbroadcastTx(txid);
+            }
 
+            if (wait_callback) {
+                // For transactions broadcast from outside the wallet, make sure
+                // that the wallet has been notified of the transaction before
+                // continuing.
+                //
+                // This prevents a race where a user might call sendrawtransaction
+                // with a transaction to/from their wallet, immediately call some
+                // wallet RPC, and get a stale result because callbacks have not
+                // yet been processed.
+                CallFunctionInValidationInterfaceQueue([&promise] {
+                    promise.set_value();
+                });
+                callback_set = true;
+            }
+        }
     } // cs_main
 
     if (callback_set) {
@@ -99,12 +112,8 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
     }
 
     if (relay) {
-        // the mempool tracks locally submitted transactions to make a
-        // best-effort of initial broadcast
-        node.mempool->AddUnbroadcastTx(hashTx);
-
         LOCK(cs_main);
-        node.peerman->RelayTransaction(hashTx);
+        node.peerman->RelayTransaction(txid);
     }
 
     return TransactionError::OK;