Merge bitcoin/bitcoin#30184: [25.x] windeploy: Renew certificate

7a4eff2c98 windeploy: Renew certificate (Ava Chow)

Pull request description:

  Github-Pull: #30149
  Rebased-From: 9f4ff1e965

ACKs for top commit:
  theuni:
    ACK 7a4eff2c98
  glozow:
    ACK 7a4eff2c98

Tree-SHA512: 827b20fad32a2f140e12595ff297fc29769a6189561f13c06e4b3dc05265f48efbf3185320d436229767918dfda9d7417ec8a39018662379641e3f7828ba93a5

configure.ac

@@ -2,7 +2,7 @@ AC_PREREQ([2.69])
 define(_CLIENT_VERSION_MAJOR, 25)
 define(_CLIENT_VERSION_MINOR, 2)
 define(_CLIENT_VERSION_BUILD, 0)
-define(_CLIENT_VERSION_RC, 1)
+define(_CLIENT_VERSION_RC, 0)
 define(_CLIENT_VERSION_IS_RELEASE, true)
 define(_COPYRIGHT_YEAR, 2023)
 define(_COPYRIGHT_HOLDERS,[The %s developers])

contrib/windeploy/win-codesign.cert

@@ -1,8 +1,8 @@
 -----BEGIN CERTIFICATE-----
-MIIHfDCCBWSgAwIBAgIQCmVvdQal72U2QxbUTT3SRTANBgkqhkiG9w0BAQsFADBp
+MIIHeTCCBWGgAwIBAgIQBzR46J2yq3g++NbQS/BBVDANBgkqhkiG9w0BAQsFADBp
 MQswCQYDVQQGEwJVUzEXMBUGA1UEChMORGlnaUNlcnQsIEluYy4xQTA/BgNVBAMT
 OERpZ2lDZXJ0IFRydXN0ZWQgRzQgQ29kZSBTaWduaW5nIFJTQTQwOTYgU0hBMzg0
-IDIwMjEgQ0ExMB4XDTIyMDUyNDAwMDAwMFoXDTI0MDUyOTIzNTk1OVowgYAxCzAJ
+IDIwMjEgQ0ExMB4XDTI0MDUyMjAwMDAwMFoXDTI3MDUzMTIzNTk1OVowgYAxCzAJ
 BgNVBAYTAlVTMREwDwYDVQQIEwhEZWxhd2FyZTEOMAwGA1UEBxMFTGV3ZXMxJjAk
 BgNVBAoTHUJpdGNvaW4gQ29yZSBDb2RlIFNpZ25pbmcgTExDMSYwJAYDVQQDEx1C
 aXRjb2luIENvcmUgQ29kZSBTaWduaW5nIExMQzCCAiIwDQYJKoZIhvcNAQEBBQAD
@@ -17,28 +17,28 @@ CDvScIgnQXmk+cbKMBtg9kM0F+aLWsN2xVf0uAj3U7sdXLrfJeW0DZIktWtTBQzX
 O/OE4Ka+1WFnDg0HJIih0cTjl9YYvfe53L4pCGy+qGt/XGBRqCMfXp3g+H9FGR5r
 pensVVcsrv3GbTfYdlpdmp9OHH5G57GTAZueobCZg7r7RKK0zPU9EiTLJxzyXuai
 v/Ksd8eIhHRjewMaQuAtQM1tO+oKAbLF0v2M7v7/aVT76X32JllYAizm3zjvAgMB
-AAGjggIGMIICAjAfBgNVHSMEGDAWgBRoN+Drtjv4XxGG+/5hewiIZfROQjAdBgNV
-HQ4EFgQUvCpU58PIuofv0kHJ3Ty0YDKEy3cwDgYDVR0PAQH/BAQDAgeAMBMGA1Ud
-JQQMMAoGCCsGAQUFBwMDMIG1BgNVHR8Ega0wgaowU6BRoE+GTWh0dHA6Ly9jcmwz
-LmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydFRydXN0ZWRHNENvZGVTaWduaW5nUlNBNDA5
-NlNIQTM4NDIwMjFDQTEuY3JsMFOgUaBPhk1odHRwOi8vY3JsNC5kaWdpY2VydC5j
-b20vRGlnaUNlcnRUcnVzdGVkRzRDb2RlU2lnbmluZ1JTQTQwOTZTSEEzODQyMDIx
-Q0ExLmNybDA+BgNVHSAENzA1MDMGBmeBDAEEATApMCcGCCsGAQUFBwIBFhtodHRw
-Oi8vd3d3LmRpZ2ljZXJ0LmNvbS9DUFMwgZQGCCsGAQUFBwEBBIGHMIGEMCQGCCsG
+AAGjggIDMIIB/zAfBgNVHSMEGDAWgBRoN+Drtjv4XxGG+/5hewiIZfROQjAdBgNV
+HQ4EFgQUvCpU58PIuofv0kHJ3Ty0YDKEy3cwPgYDVR0gBDcwNTAzBgZngQwBBAEw
+KTAnBggrBgEFBQcCARYbaHR0cDovL3d3dy5kaWdpY2VydC5jb20vQ1BTMA4GA1Ud
+DwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzCBtQYDVR0fBIGtMIGqMFOg
+UaBPhk1odHRwOi8vY3JsMy5kaWdpY2VydC5jb20vRGlnaUNlcnRUcnVzdGVkRzRD
+b2RlU2lnbmluZ1JTQTQwOTZTSEEzODQyMDIxQ0ExLmNybDBToFGgT4ZNaHR0cDov
+L2NybDQuZGlnaWNlcnQuY29tL0RpZ2lDZXJ0VHJ1c3RlZEc0Q29kZVNpZ25pbmdS
+U0E0MDk2U0hBMzg0MjAyMUNBMS5jcmwwgZQGCCsGAQUFBwEBBIGHMIGEMCQGCCsG
 AQUFBzABhhhodHRwOi8vb2NzcC5kaWdpY2VydC5jb20wXAYIKwYBBQUHMAKGUGh0
 dHA6Ly9jYWNlcnRzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydFRydXN0ZWRHNENvZGVT
-aWduaW5nUlNBNDA5NlNIQTM4NDIwMjFDQTEuY3J0MAwGA1UdEwEB/wQCMAAwDQYJ
-KoZIhvcNAQELBQADggIBABhpTZufRws1vrtI0xB1/UWrSEJxdPHivfpXE708dzum
-Jh3TFzpsEUCQX5BJJet1l7x92sKNeAL7votA+8O8YvMD64Kim7VKA2BB8AOHKQbp
-r1c2iZBwwofInviRYvsrvQta6KBy2KOe1L/l0KnpUazL9Tv4VKvuWAw/Qc0/eTQr
-NZRsmADORxnZ1qW+SpF+/WbazIYjod/Oqb1U3on+PzyiGD3SjzNhsdFRptqzrIaY
-UVV+2XHG4fN6A8wkyQL5NIVXGiK7rqS5VrRAv58Lf1ZZTghdAL+5SySE0OsR9t0K
-W73ZB9pxbuZZ6Zfxjotjw+IilCEm3ADbc7Eb2ijI4x8mix0XWMUrhL34s7/jRyDi
-P+30aSgjWp611tp/EYRW5kpIaFR8AesDdM0DSSCCRXOMwQG2Tq2+CnqItB5oLNPp
-2XySwlIWvmjbzsREfIpE3yh3bxmHY+vFIc2R0nNkbWNIT6AGtaEQ7oWkgpK8YMkA
-QCf4EUC4Qa7qHiH6YSmYJhjApBLC7UDwevgwxuDrwimWAj+tDkzdnENMcBp4SAy6
-LwUuDi2IU6HRSXWdh2YEkDbc3FdwknnnEWaB4dlRL85YjHyLXN0KiE7SKTj1LfR4
-dGeDqVUlDj9D5+X4a7F89wLP/um40/52HUQv5t5WcNr/47r9aVkx9DHs1b8oUnLg
+aWduaW5nUlNBNDA5NlNIQTM4NDIwMjFDQTEuY3J0MAkGA1UdEwQCMAAwDQYJKoZI
+hvcNAQELBQADggIBADdniG9IY9oOfw1e3+uc2lR4hoZqquJQRrCnbWJ1npnCTavI
+CfcEEMuQ5ztg4TR7tQNj2KcaHWKuPYxEz2bg8HpSPG27lnXaz4pLgfqvjdZWNH2v
+W6DGRUAwuMQHSV0qhuRcJPZuhwSFx/8y4r++jIcBxCbt/Jprt/bqc8vZZZzTDPfG
+M6cGaKMDvF//OkUPVzh4s557kV7+LoaX8CigiACZky3Zj3tkQfJYkEvdQseNvX49
+CMJ+cjN+fGweshbn/DszAT5oXW5l2PXeceyGrE+5Ex1ELXCPqNj8ZSn+S9IKZOag
+zDFBA93RTVD438peXPz//xgusgnmSqSPS5tCp9KSvew81acu4v/+egg9EgSSx5Ho
+9fkOX7JuygvN3r3UZqsddxdwf2dPvBDYlMdieF8qsR7H5DQPQoaTVrIhW4TFtJl/
+UPjVlnDwu+yvMC70F+CaVgQs01uZ0VKuG3KNkkEj6+V/SM54NVVgcY/Q7llKIFA8
+Qk8Ip8/83cVBptKW+OU+i2ZwoYskLbdfDE31X2knUIouNZgBBMhzc5WjJCEGXAPm
+9xYZMn87cc+ejxCw6/WC4b6tDCziO8drq76Pl6LTNPOtRkEVqt12p8Uqi9PgznUB
+bdHeoF5XHt1Ca2ySpSYuMz5djwIC2ws8kiMm44/AyTm6dwRcesiOTqnaRc+t
 -----END CERTIFICATE-----
 -----BEGIN CERTIFICATE-----
 MIIGsDCCBJigAwIBAgIQCK1AsmDSnEyfXs2pvZOu2TANBgkqhkiG9w0BAQwFADBi

doc/bips.md

@@ -1,4 +1,4 @@
-BIPs that are implemented by Bitcoin Core (up-to-date up to **v25.0**):
+BIPs that are implemented by Bitcoin Core (up-to-date up to **v25.2**):
 
 * [`BIP 9`](https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki): The changes allowing multiple soft-forks to be deployed in parallel have been implemented since **v0.12.1**  ([PR #7575](https://github.com/bitcoin/bitcoin/pull/7575))
 * [`BIP 11`](https://github.com/bitcoin/bips/blob/master/bip-0011.mediawiki): Multisig outputs are standard since **v0.6.0** ([PR #669](https://github.com/bitcoin/bitcoin/pull/669)).

doc/man/bitcoin-cli.1

@@ -1,7 +1,7 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-CLI "1" "February 2024" "bitcoin-cli v25.2.0rc1" "User Commands"
+.TH BITCOIN-CLI "1" "April 2024" "bitcoin-cli v25.2.0" "User Commands"
 .SH NAME
-bitcoin-cli \- manual page for bitcoin-cli v25.2.0rc1
+bitcoin-cli \- manual page for bitcoin-cli v25.2.0
 .SH SYNOPSIS
 .B bitcoin-cli
 [\fI\,options\/\fR] \fI\,<command> \/\fR[\fI\,params\/\fR]  \fI\,Send command to Bitcoin Core\/\fR
@@ -15,7 +15,7 @@ bitcoin-cli \- manual page for bitcoin-cli v25.2.0rc1
 .B bitcoin-cli
 [\fI\,options\/\fR] \fI\,help <command>      Get help for a command\/\fR
 .SH DESCRIPTION
-Bitcoin Core RPC client version v25.2.0rc1
+Bitcoin Core RPC client version v25.2.0
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-qt.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-QT "1" "February 2024" "bitcoin-qt v25.2.0rc1" "User Commands"
+.TH BITCOIN-QT "1" "April 2024" "bitcoin-qt v25.2.0" "User Commands"
 .SH NAME
-bitcoin-qt \- manual page for bitcoin-qt v25.2.0rc1
+bitcoin-qt \- manual page for bitcoin-qt v25.2.0
 .SH SYNOPSIS
 .B bitcoin-qt
 [\fI\,command-line options\/\fR]
 .SH DESCRIPTION
-Bitcoin Core version v25.2.0rc1
+Bitcoin Core version v25.2.0
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-tx.1

@@ -1,7 +1,7 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-TX "1" "February 2024" "bitcoin-tx v25.2.0rc1" "User Commands"
+.TH BITCOIN-TX "1" "April 2024" "bitcoin-tx v25.2.0" "User Commands"
 .SH NAME
-bitcoin-tx \- manual page for bitcoin-tx v25.2.0rc1
+bitcoin-tx \- manual page for bitcoin-tx v25.2.0
 .SH SYNOPSIS
 .B bitcoin-tx
 [\fI\,options\/\fR] \fI\,<hex-tx> \/\fR[\fI\,commands\/\fR]  \fI\,Update hex-encoded bitcoin transaction\/\fR
@@ -9,7 +9,7 @@ bitcoin-tx \- manual page for bitcoin-tx v25.2.0rc1
 .B bitcoin-tx
 [\fI\,options\/\fR] \fI\,-create \/\fR[\fI\,commands\/\fR]   \fI\,Create hex-encoded bitcoin transaction\/\fR
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-tx utility version v25.2.0rc1
+Bitcoin Core bitcoin\-tx utility version v25.2.0
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-util.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-UTIL "1" "February 2024" "bitcoin-util v25.2.0rc1" "User Commands"
+.TH BITCOIN-UTIL "1" "April 2024" "bitcoin-util v25.2.0" "User Commands"
 .SH NAME
-bitcoin-util \- manual page for bitcoin-util v25.2.0rc1
+bitcoin-util \- manual page for bitcoin-util v25.2.0
 .SH SYNOPSIS
 .B bitcoin-util
 [\fI\,options\/\fR] [\fI\,commands\/\fR]  \fI\,Do stuff\/\fR
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-util utility version v25.2.0rc1
+Bitcoin Core bitcoin\-util utility version v25.2.0
 .SH OPTIONS
 .HP
 \-?

doc/man/bitcoin-wallet.1

@@ -1,9 +1,9 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIN-WALLET "1" "February 2024" "bitcoin-wallet v25.2.0rc1" "User Commands"
+.TH BITCOIN-WALLET "1" "April 2024" "bitcoin-wallet v25.2.0" "User Commands"
 .SH NAME
-bitcoin-wallet \- manual page for bitcoin-wallet v25.2.0rc1
+bitcoin-wallet \- manual page for bitcoin-wallet v25.2.0
 .SH DESCRIPTION
-Bitcoin Core bitcoin\-wallet version v25.2.0rc1
+Bitcoin Core bitcoin\-wallet version v25.2.0
 .PP
 bitcoin\-wallet is an offline tool for creating and interacting with Bitcoin Core wallet files.
 By default bitcoin\-wallet will act on wallets in the default mainnet wallet directory in the datadir.

doc/man/bitcoind.1

@@ -1,12 +1,12 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.49.3.
-.TH BITCOIND "1" "February 2024" "bitcoind v25.2.0rc1" "User Commands"
+.TH BITCOIND "1" "April 2024" "bitcoind v25.2.0" "User Commands"
 .SH NAME
-bitcoind \- manual page for bitcoind v25.2.0rc1
+bitcoind \- manual page for bitcoind v25.2.0
 .SH SYNOPSIS
 .B bitcoind
 [\fI\,options\/\fR]                     \fI\,Start Bitcoin Core\/\fR
 .SH DESCRIPTION
-Bitcoin Core version v25.2.0rc1
+Bitcoin Core version v25.2.0
 .SH OPTIONS
 .HP
 \-?

doc/release-notes.md

@@ -1,9 +1,9 @@
-25.2rc1 Release Notes
+25.2 Release Notes
 ==================
 
-Bitcoin Core version 25.2rc1 is now available from:
+Bitcoin Core version 25.2 is now available from:
 
-  <https://bitcoincore.org/bin/bitcoin-core-25.2/test.rc1>
+  <https://bitcoincore.org/bin/bitcoin-core-25.2>
 
 This release includes various bug fixes and performance
 improvements, as well as updated translations.
@@ -51,6 +51,12 @@ Notable changes
 ### Wallet
 
 - #29176 wallet: Fix use-after-free in WalletBatch::EraseRecords
+- #29510 wallet: `getrawchangeaddress` and `getnewaddress` failures should not affect keypools for descriptor wallets
+
+### P2P and network changes
+
+- #29412 p2p: Don't process mutated blocks
+- #29524 p2p: Don't consider blocks mutated if they don't connect to known prev block
 
 Credits
 =======
@@ -60,6 +66,9 @@ Thanks to everyone who directly contributed to this release:
 - Martin Zumsande
 - Sebastian Falbesoner
 - MarcoFalke
+- UdjinM6
+- dergoegge
+- Greg Sanders
 
 As well as to everyone that helped with translations on
 [Transifex](https://www.transifex.com/bitcoin/bitcoin/).

src/net_processing.cpp

@@ -4629,6 +4629,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 
         LogPrint(BCLog::NET, "received block %s peer=%d\n", pblock->GetHash().ToString(), pfrom.GetId());
 
+        const CBlockIndex* prev_block{WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock))};
+
+        // Check for possible mutation if it connects to something we know so we can check for DEPLOYMENT_SEGWIT being active
+        if (prev_block && IsBlockMutated(/*block=*/*pblock,
+                           /*check_witness_root=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT))) {
+            LogPrint(BCLog::NET, "Received mutated block from peer=%d\n", peer->m_id);
+            Misbehaving(*peer, 100, "mutated block");
+            WITH_LOCK(cs_main, RemoveBlockRequest(pblock->GetHash(), peer->m_id));
+            return;
+        }
+
         bool forceProcessing = false;
         const uint256 hash(pblock->GetHash());
         bool min_pow_checked = false;
@@ -4644,7 +4655,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true));
 
             // Check work on this block against our anti-dos thresholds.
-            const CBlockIndex* prev_block = m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock);
             if (prev_block && prev_block->nChainWork + CalculateHeadersWork({pblock->GetBlockHeader()}) >= GetAntiDoSWorkThreshold()) {
                 min_pow_checked = true;
             }

src/primitives/block.h

@@ -71,8 +71,10 @@ public:
     // network and disk
     std::vector<CTransactionRef> vtx;
 
-    // memory only
-    mutable bool fChecked;
+    // Memory-only flags for caching expensive checks
+    mutable bool fChecked;                            // CheckBlock()
+    mutable bool m_checked_witness_commitment{false}; // CheckWitnessCommitment()
+    mutable bool m_checked_merkle_root{false};        // CheckMerkleRoot()
 
     CBlock()
     {
@@ -96,6 +98,8 @@ public:
         CBlockHeader::SetNull();
         vtx.clear();
         fChecked = false;
+        m_checked_witness_commitment = false;
+        m_checked_merkle_root = false;
     }
 
     CBlockHeader GetBlockHeader() const

src/test/validation_tests.cpp

@@ -4,11 +4,16 @@
 
 #include <chainparams.h>
 #include <consensus/amount.h>
+#include <consensus/merkle.h>
+#include <core_io.h>
+#include <hash.h>
 #include <net.h>
 #include <signet.h>
 #include <uint256.h>
 #include <validation.h>
 
+#include <string>
+
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
@@ -144,4 +149,215 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo)
     BOOST_CHECK_EQUAL(out210.nChainTx, 200U);
 }
 
+BOOST_AUTO_TEST_CASE(block_malleation)
+{
+    // Test utilities that calls `IsBlockMutated` and then clears the validity
+    // cache flags on `CBlock`.
+    auto is_mutated = [](CBlock& block, bool check_witness_root) {
+        bool mutated{IsBlockMutated(block, check_witness_root)};
+        block.fChecked = false;
+        block.m_checked_witness_commitment = false;
+        block.m_checked_merkle_root = false;
+        return mutated;
+    };
+    auto is_not_mutated = [&is_mutated](CBlock& block, bool check_witness_root) {
+        return !is_mutated(block, check_witness_root);
+    };
+
+    // Test utilities to create coinbase transactions and insert witness
+    // commitments.
+    //
+    // Note: this will not include the witness stack by default to avoid
+    // triggering the "no witnesses allowed for blocks that don't commit to
+    // witnesses" rule when testing other malleation vectors.
+    auto create_coinbase_tx = [](bool include_witness = false) {
+        CMutableTransaction coinbase;
+        coinbase.vin.resize(1);
+        if (include_witness) {
+            coinbase.vin[0].scriptWitness.stack.resize(1);
+            coinbase.vin[0].scriptWitness.stack[0] = std::vector<unsigned char>(32, 0x00);
+        }
+
+        coinbase.vout.resize(1);
+        coinbase.vout[0].scriptPubKey.resize(MINIMUM_WITNESS_COMMITMENT);
+        coinbase.vout[0].scriptPubKey[0] = OP_RETURN;
+        coinbase.vout[0].scriptPubKey[1] = 0x24;
+        coinbase.vout[0].scriptPubKey[2] = 0xaa;
+        coinbase.vout[0].scriptPubKey[3] = 0x21;
+        coinbase.vout[0].scriptPubKey[4] = 0xa9;
+        coinbase.vout[0].scriptPubKey[5] = 0xed;
+
+        auto tx = MakeTransactionRef(coinbase);
+        assert(tx->IsCoinBase());
+        return tx;
+    };
+    auto insert_witness_commitment = [](CBlock& block, uint256 commitment) {
+        assert(!block.vtx.empty() && block.vtx[0]->IsCoinBase() && !block.vtx[0]->vout.empty());
+
+        CMutableTransaction mtx{*block.vtx[0]};
+        CHash256().Write(commitment).Write(std::vector<unsigned char>(32, 0x00)).Finalize(commitment);
+        memcpy(&mtx.vout[0].scriptPubKey[6], commitment.begin(), 32);
+        block.vtx[0] = MakeTransactionRef(mtx);
+    };
+
+    {
+        CBlock block;
+
+        // Empty block is expected to have merkle root of 0x0.
+        BOOST_CHECK(block.vtx.empty());
+        block.hashMerkleRoot = uint256{1};
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+        block.hashMerkleRoot = uint256{};
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
+
+        // Block with a single coinbase tx is mutated if the merkle root is not
+        // equal to the coinbase tx's hash.
+        block.vtx.push_back(create_coinbase_tx());
+        BOOST_CHECK(block.vtx[0]->GetHash() != block.hashMerkleRoot);
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+        block.hashMerkleRoot = block.vtx[0]->GetHash();
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
+
+        // Block with two transactions is mutated if the merkle root does not
+        // match the double sha256 of the concatenation of the two transaction
+        // hashes.
+        block.vtx.push_back(MakeTransactionRef(CMutableTransaction{}));
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+        HashWriter hasher;
+        hasher.write(Span(reinterpret_cast<const std::byte*>(block.vtx[0]->GetHash().data()), 32));
+        hasher.write(Span(reinterpret_cast<const std::byte*>(block.vtx[1]->GetHash().data()), 32));
+        block.hashMerkleRoot = hasher.GetHash();
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
+
+        // Block with two transactions is mutated if any node is duplicate.
+        {
+            block.vtx[1] = block.vtx[0];
+            BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+            HashWriter hasher;
+            hasher.write(Span(reinterpret_cast<const std::byte*>(block.vtx[0]->GetHash().data()), 32));
+            hasher.write(Span(reinterpret_cast<const std::byte*>(block.vtx[1]->GetHash().data()), 32));
+            block.hashMerkleRoot = hasher.GetHash();
+            BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+        }
+
+        // Blocks with 64-byte coinbase transactions are not considered mutated
+        block.vtx.clear();
+        {
+            CMutableTransaction mtx;
+            mtx.vin.resize(1);
+            mtx.vout.resize(1);
+            mtx.vout[0].scriptPubKey.resize(4);
+            block.vtx.push_back(MakeTransactionRef(mtx));
+            block.hashMerkleRoot = block.vtx.back()->GetHash();
+            assert(block.vtx.back()->IsCoinBase());
+            assert(GetSerializeSize(block.vtx.back(), PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) == 64);
+        }
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
+    }
+
+    {
+        // Test merkle root malleation
+
+        // Pseudo code to mine transactions tx{1,2,3}:
+        //
+        // ```
+        // loop {
+        //   tx1 = random_tx()
+        //   tx2 = random_tx()
+        //   tx3 = deserialize_tx(txid(tx1) || txid(tx2));
+        //   if serialized_size_without_witness(tx3) == 64 {
+        //     print(hex(tx3))
+        //     break
+        //   }
+        // }
+        // ```
+        //
+        // The `random_tx` function used to mine the txs below simply created
+        // empty transactions with a random version field.
+        CMutableTransaction tx1;
+        BOOST_CHECK(DecodeHexTx(tx1, "ff204bd0000000000000", /*try_no_witness=*/true, /*try_witness=*/false));
+        CMutableTransaction tx2;
+        BOOST_CHECK(DecodeHexTx(tx2, "8ae53c92000000000000", /*try_no_witness=*/true, /*try_witness=*/false));
+        CMutableTransaction tx3;
+        BOOST_CHECK(DecodeHexTx(tx3, "cdaf22d00002c6a7f848f8ae4d30054e61dcf3303d6fe01d282163341f06feecc10032b3160fcab87bdfe3ecfb769206ef2d991b92f8a268e423a6ef4d485f06", /*try_no_witness=*/true, /*try_witness=*/false));
+        {
+            // Verify that double_sha256(txid1||txid2) == txid3
+            HashWriter hasher;
+            hasher.write(Span(reinterpret_cast<const std::byte*>(tx1.GetHash().data()), 32));
+            hasher.write(Span(reinterpret_cast<const std::byte*>(tx2.GetHash().data()), 32));
+            assert(hasher.GetHash() == tx3.GetHash());
+            // Verify that tx3 is 64 bytes in size (without witness).
+            assert(GetSerializeSize(tx3, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) == 64);
+        }
+
+        CBlock block;
+        block.vtx.push_back(MakeTransactionRef(tx1));
+        block.vtx.push_back(MakeTransactionRef(tx2));
+        uint256 merkle_root = block.hashMerkleRoot = BlockMerkleRoot(block);
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/false));
+
+        // Mutate the block by replacing the two transactions with one 64-byte
+        // transaction that serializes into the concatenation of the txids of
+        // the transactions in the unmutated block.
+        block.vtx.clear();
+        block.vtx.push_back(MakeTransactionRef(tx3));
+        BOOST_CHECK(!block.vtx.back()->IsCoinBase());
+        BOOST_CHECK(BlockMerkleRoot(block) == merkle_root);
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+    }
+
+    {
+        CBlock block;
+        block.vtx.push_back(create_coinbase_tx(/*include_witness=*/true));
+        {
+            CMutableTransaction mtx;
+            mtx.vin.resize(1);
+            mtx.vin[0].scriptWitness.stack.resize(1);
+            mtx.vin[0].scriptWitness.stack[0] = {0};
+            block.vtx.push_back(MakeTransactionRef(mtx));
+        }
+        block.hashMerkleRoot = BlockMerkleRoot(block);
+        // Block with witnesses is considered mutated if the witness commitment
+        // is not validated.
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/false));
+        // Block with invalid witness commitment is considered mutated.
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/true));
+
+        // Block with valid commitment is not mutated
+        {
+            auto commitment{BlockWitnessMerkleRoot(block)};
+            insert_witness_commitment(block, commitment);
+            block.hashMerkleRoot = BlockMerkleRoot(block);
+        }
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/true));
+
+        // Malleating witnesses should be caught by `IsBlockMutated`.
+        {
+            CMutableTransaction mtx{*block.vtx[1]};
+            assert(!mtx.vin[0].scriptWitness.stack[0].empty());
+            ++mtx.vin[0].scriptWitness.stack[0][0];
+            block.vtx[1] = MakeTransactionRef(mtx);
+        }
+        // Without also updating the witness commitment, the merkle root should
+        // not change when changing one of the witnesses.
+        BOOST_CHECK(block.hashMerkleRoot == BlockMerkleRoot(block));
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/true));
+        {
+            auto commitment{BlockWitnessMerkleRoot(block)};
+            insert_witness_commitment(block, commitment);
+            block.hashMerkleRoot = BlockMerkleRoot(block);
+        }
+        BOOST_CHECK(is_not_mutated(block, /*check_witness_root=*/true));
+
+        // Test malleating the coinbase witness reserved value
+        {
+            CMutableTransaction mtx{*block.vtx[0]};
+            mtx.vin[0].scriptWitness.stack.resize(0);
+            block.vtx[0] = MakeTransactionRef(mtx);
+            block.hashMerkleRoot = BlockMerkleRoot(block);
+        }
+        BOOST_CHECK(is_mutated(block, /*check_witness_root=*/true));
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -3486,6 +3486,60 @@ static bool CheckBlockHeader(const CBlockHeader& block, BlockValidationState& st
     return true;
 }
 
+static bool CheckMerkleRoot(const CBlock& block, BlockValidationState& state)
+{
+    if (block.m_checked_merkle_root) return true;
+
+    bool mutated;
+    uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated);
+    if (block.hashMerkleRoot != hashMerkleRoot2)
+        return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txnmrklroot", "hashMerkleRoot mismatch");
+
+    // Check for merkle tree malleability (CVE-2012-2459): repeating sequences
+    // of transactions in a block without affecting the merkle root of a block,
+    // while still invalidating it.
+    if (mutated)
+        return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txns-duplicate", "duplicate transaction");
+
+    block.m_checked_merkle_root = true;
+    return true;
+}
+
+static bool CheckWitnessMalleation(const CBlock& block, bool expect_witness_commitment, BlockValidationState& state)
+{
+    if (expect_witness_commitment) {
+        if (block.m_checked_witness_commitment) return true;
+
+        int commitpos = GetWitnessCommitmentIndex(block);
+        if (commitpos != NO_WITNESS_COMMITMENT) {
+            bool malleated = false;
+            uint256 hashWitness = BlockWitnessMerkleRoot(block, &malleated);
+            // The malleation check is ignored; as the transaction tree itself
+            // already does not permit it, it is impossible to trigger in the
+            // witness tree.
+            if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) {
+                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__));
+            }
+            CHash256().Write(hashWitness).Write(block.vtx[0]->vin[0].scriptWitness.stack[0]).Finalize(hashWitness);
+            if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
+                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
+            }
+
+            block.m_checked_witness_commitment = true;
+            return true;
+        }
+    }
+
+    // No witness data is allowed in blocks that don't commit to witness data, as this would otherwise leave room for spam
+    for (const auto& tx : block.vtx) {
+        if (tx->HasWitness()) {
+            return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__));
+        }
+    }
+
+    return true;
+}
+
 bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW, bool fCheckMerkleRoot)
 {
     // These are checks that are independent of context.
@@ -3504,17 +3558,8 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu
     }
 
     // Check the merkle root.
-    if (fCheckMerkleRoot) {
-        bool mutated;
-        uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated);
-        if (block.hashMerkleRoot != hashMerkleRoot2)
-            return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txnmrklroot", "hashMerkleRoot mismatch");
-
-        // Check for merkle tree malleability (CVE-2012-2459): repeating sequences
-        // of transactions in a block without affecting the merkle root of a block,
-        // while still invalidating it.
-        if (mutated)
-            return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txns-duplicate", "duplicate transaction");
+    if (fCheckMerkleRoot && !CheckMerkleRoot(block, state)) {
+        return false;
     }
 
     // All potential-corruption validation must be done before we do any
@@ -3605,6 +3650,37 @@ bool HasValidProofOfWork(const std::vector<CBlockHeader>& headers, const Consens
             [&](const auto& header) { return CheckProofOfWork(header.GetHash(), header.nBits, consensusParams);});
 }
 
+bool IsBlockMutated(const CBlock& block, bool check_witness_root)
+{
+    BlockValidationState state;
+    if (!CheckMerkleRoot(block, state)) {
+        LogPrint(BCLog::VALIDATION, "Block mutated: %s\n", state.ToString());
+        return true;
+    }
+
+    if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) {
+        // Consider the block mutated if any transaction is 64 bytes in size (see 3.1
+        // in "Weaknesses in Bitcoin’s Merkle Root Construction":
+        // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20190225/a27d8837/attachment-0001.pdf).
+        //
+        // Note: This is not a consensus change as this only applies to blocks that
+        // don't have a coinbase transaction and would therefore already be invalid.
+        return std::any_of(block.vtx.begin(), block.vtx.end(),
+                           [](auto& tx) { return ::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) == 64; });
+    } else {
+        // Theoretically it is still possible for a block with a 64 byte
+        // coinbase transaction to be mutated but we neglect that possibility
+        // here as it requires at least 224 bits of work.
+    }
+
+    if (!CheckWitnessMalleation(block, check_witness_root, state)) {
+        LogPrint(BCLog::VALIDATION, "Block mutated: %s\n", state.ToString());
+        return true;
+    }
+
+    return false;
+}
+
 arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers)
 {
     arith_uint256 total_work{0};
@@ -3713,33 +3789,8 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat
     // * There must be at least one output whose scriptPubKey is a single 36-byte push, the first 4 bytes of which are
     //   {0xaa, 0x21, 0xa9, 0xed}, and the following 32 bytes are SHA256^2(witness root, witness reserved value). In case there are
     //   multiple, the last one is used.
-    bool fHaveWitness = false;
-    if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT)) {
-        int commitpos = GetWitnessCommitmentIndex(block);
-        if (commitpos != NO_WITNESS_COMMITMENT) {
-            bool malleated = false;
-            uint256 hashWitness = BlockWitnessMerkleRoot(block, &malleated);
-            // The malleation check is ignored; as the transaction tree itself
-            // already does not permit it, it is impossible to trigger in the
-            // witness tree.
-            if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) {
-                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__));
-            }
-            CHash256().Write(hashWitness).Write(block.vtx[0]->vin[0].scriptWitness.stack[0]).Finalize(hashWitness);
-            if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
-                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
-            }
-            fHaveWitness = true;
-        }
-    }
-
-    // No witness data is allowed in blocks that don't commit to witness data, as this would otherwise leave room for spam
-    if (!fHaveWitness) {
-      for (const auto& tx : block.vtx) {
-            if (tx->HasWitness()) {
-                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__));
-            }
-        }
+    if (!CheckWitnessMalleation(block, DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT), state)) {
+        return false;
     }
 
     // After the coinbase witness reserved value and commitment are verified,

src/validation.h

@@ -352,6 +352,9 @@ bool TestBlockValidity(BlockValidationState& state,
 /** Check with the proof of work on each blockheader matches the value in nBits */
 bool HasValidProofOfWork(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams);
 
+/** Check if a block has been mutated (with respect to its merkle root and witness commitments). */
+bool IsBlockMutated(const CBlock& block, bool check_witness_root);
+
 /** Return the sum of the work on a given set of headers */
 arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers);
 

src/wallet/wallet.cpp

@@ -2596,8 +2596,10 @@ util::Result<CTxDestination> ReserveDestination::GetReservedDestination(bool int
 
     if (nIndex == -1) {
         CKeyPool keypool;
-        auto op_address = m_spk_man->GetReservedDestination(type, internal, nIndex, keypool);
+        int64_t index;
+        auto op_address = m_spk_man->GetReservedDestination(type, internal, index, keypool);
         if (!op_address) return op_address;
+        nIndex = index;
         address = *op_address;
         fInternal = keypool.fInternal;
     }

test/functional/p2p_mutated_blocks.py

@@ -0,0 +1,116 @@
+#!/usr/bin/env python3
+# Copyright (c) The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+"""
+Test that an attacker can't degrade compact block relay by sending unsolicited
+mutated blocks to clear in-flight blocktxn requests from other honest peers.
+"""
+
+from test_framework.p2p import P2PInterface
+from test_framework.messages import (
+    BlockTransactions,
+    msg_cmpctblock,
+    msg_block,
+    msg_blocktxn,
+    HeaderAndShortIDs,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.blocktools import (
+    COINBASE_MATURITY,
+    create_block,
+    add_witness_commitment,
+    NORMAL_GBT_REQUEST_PARAMS,
+)
+from test_framework.util import assert_equal
+from test_framework.wallet import MiniWallet
+import copy
+
+class MutatedBlocksTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+        self.extra_args = [
+            [
+                "-testactivationheight=segwit@1", # causes unconnected headers/blocks to not have segwit considered deployed
+            ],
+        ]
+
+    def run_test(self):
+        self.wallet = MiniWallet(self.nodes[0])
+        self.generate(self.wallet, COINBASE_MATURITY)
+
+        honest_relayer = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay")
+        attacker = self.nodes[0].add_p2p_connection(P2PInterface())
+
+        # Create new block with two transactions (coinbase + 1 self-transfer).
+        # The self-transfer transaction is needed to trigger a compact block
+        # `getblocktxn` roundtrip.
+        tx = self.wallet.create_self_transfer()["tx"]
+        block = create_block(tmpl=self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS), txlist=[tx])
+        add_witness_commitment(block)
+        block.solve()
+
+        # Create mutated version of the block by changing the transaction
+        # version on the self-transfer.
+        mutated_block = copy.deepcopy(block)
+        mutated_block.vtx[1].nVersion = 4
+
+        # Announce the new block via a compact block through the honest relayer
+        cmpctblock = HeaderAndShortIDs()
+        cmpctblock.initialize_from_block(block, use_witness=True)
+        honest_relayer.send_message(msg_cmpctblock(cmpctblock.to_p2p()))
+
+        # Wait for a `getblocktxn` that attempts to fetch the self-transfer
+        def self_transfer_requested():
+            if not honest_relayer.last_message.get('getblocktxn'):
+                return False
+
+            get_block_txn = honest_relayer.last_message['getblocktxn']
+            return get_block_txn.block_txn_request.blockhash == block.sha256 and \
+                   get_block_txn.block_txn_request.indexes == [1]
+        honest_relayer.wait_until(self_transfer_requested, timeout=5)
+
+        # Block at height 101 should be the only one in flight from peer 0
+        peer_info_prior_to_attack = self.nodes[0].getpeerinfo()
+        assert_equal(peer_info_prior_to_attack[0]['id'], 0)
+        assert_equal([101], peer_info_prior_to_attack[0]["inflight"])
+
+        # Attempt to clear the honest relayer's download request by sending the
+        # mutated block (as the attacker).
+        with self.nodes[0].assert_debug_log(expected_msgs=["Block mutated: bad-txnmrklroot, hashMerkleRoot mismatch"]):
+            attacker.send_message(msg_block(mutated_block))
+        # Attacker should get disconnected for sending a mutated block
+        attacker.wait_for_disconnect(timeout=5)
+
+        # Block at height 101 should *still* be the only block in-flight from
+        # peer 0
+        peer_info_after_attack = self.nodes[0].getpeerinfo()
+        assert_equal(peer_info_after_attack[0]['id'], 0)
+        assert_equal([101], peer_info_after_attack[0]["inflight"])
+
+        # The honest relayer should be able to complete relaying the block by
+        # sending the blocktxn that was requested.
+        block_txn = msg_blocktxn()
+        block_txn.block_transactions = BlockTransactions(blockhash=block.sha256, transactions=[tx])
+        honest_relayer.send_and_ping(block_txn)
+        assert_equal(self.nodes[0].getbestblockhash(), block.hash)
+
+        # Check that unexpected-witness mutation check doesn't trigger on a header that doesn't connect to anything
+        assert_equal(len(self.nodes[0].getpeerinfo()), 1)
+        attacker = self.nodes[0].add_p2p_connection(P2PInterface())
+        block_missing_prev = copy.deepcopy(block)
+        block_missing_prev.hashPrevBlock = 123
+        block_missing_prev.solve()
+
+        # Attacker gets a DoS score of 10, not immediately disconnected, so we do it 10 times to get to 100
+        for _ in range(10):
+            assert_equal(len(self.nodes[0].getpeerinfo()), 2)
+            with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
+                attacker.send_message(msg_block(block_missing_prev))
+        attacker.wait_for_disconnect(timeout=5)
+
+
+if __name__ == '__main__':
+    MutatedBlocksTest().main()

test/functional/test_runner.py

@@ -275,6 +275,7 @@ BASE_SCRIPTS = [
     'wallet_crosschain.py',
     'mining_basic.py',
     'feature_signet.py',
+    'p2p_mutated_blocks.py',
     'wallet_implicitsegwit.py --legacy-wallet',
     'rpc_named_arguments.py',
     'feature_startupnotify.py',

test/functional/wallet_keypool.py

@@ -103,11 +103,18 @@ class KeyPoolTest(BitcoinTestFramework):
         nodes[0].getrawchangeaddress()
         nodes[0].getrawchangeaddress()
         nodes[0].getrawchangeaddress()
-        addr = set()
+        # remember keypool sizes
+        wi = nodes[0].getwalletinfo()
+        kp_size_before = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
         # the next one should fail
         assert_raises_rpc_error(-12, "Keypool ran out", nodes[0].getrawchangeaddress)
+        # check that keypool sizes did not change
+        wi = nodes[0].getwalletinfo()
+        kp_size_after = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
+        assert_equal(kp_size_before, kp_size_after)
 
         # drain the external keys
+        addr = set()
         addr.add(nodes[0].getnewaddress(address_type="bech32"))
         addr.add(nodes[0].getnewaddress(address_type="bech32"))
         addr.add(nodes[0].getnewaddress(address_type="bech32"))
@@ -115,8 +122,15 @@ class KeyPoolTest(BitcoinTestFramework):
         addr.add(nodes[0].getnewaddress(address_type="bech32"))
         addr.add(nodes[0].getnewaddress(address_type="bech32"))
         assert len(addr) == 6
+        # remember keypool sizes
+        wi = nodes[0].getwalletinfo()
+        kp_size_before = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
         # the next one should fail
         assert_raises_rpc_error(-12, "Error: Keypool ran out, please call keypoolrefill first", nodes[0].getnewaddress)
+        # check that keypool sizes did not change
+        wi = nodes[0].getwalletinfo()
+        kp_size_after = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
+        assert_equal(kp_size_before, kp_size_after)
 
         # refill keypool with three new addresses
         nodes[0].walletpassphrase('test', 1)