Merge bitcoin/bitcoin#29509: [26.x] backports and final changes for v26.1rc2v26.1rc2

c68d4d0ac5b8537ba5e1c0c512b807768e1c5c1f [doc] update manual pages for v26.1rc2 (glozow)
bd715bfb3030f392b3b19f9a05aada48e385a0d9 [build] bump version to v26.1rc2 (glozow)
b6d006d2a2b840e4a5af96c8d838e1cf589d3bce update release notes 26.1 (glozow)
fce992b38e59c90babe505eda0d72f05d79eb2f3 fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake)
40c56a4d1341017b02dcb71882b1b1f03f880b1d test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6)
7c82b2758c6bcfb8a94d2086f0d40088286815e8 wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6)
b5419ce6b621121bb1a0ec497968eb16cc012c39 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders)
0535c253fe71ae9d875827cafed41a8889f4a702 [test] IsBlockMutated unit tests (dergoegge)
8141498f3ad3ae9c42c32346ee73dc7f29e72cb5 [validation] Cache merkle root and witness commitment checks (dergoegge)
0c5c5962cbfdfd532cebc6706d5b838488b89d53 [test] Add regression test for #27608 (dergoegge)
24736350e932799c66c999470fa3837e25576fc7 [net processing] Don't process mutated blocks (dergoegge)
50c0b61a9d562240d5fe4bd79324b0c0e79caa5c [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge)
aff368fa817b065d99729186d304fff02f6e527b [validation] Introduce IsBlockMutated (dergoegge)
076c67c3aae424e58863dde3bc37cedecc496935 [refactor] Cleanup merkle root checks (dergoegge)
97a1d0a45959a29464ae73087c7a0adcdebd5a61 [validation] Isolate merkle root checks (dergoegge)
4ac0eb543d028379bb2b86ab08bbbb2f9f48d5b1 test: Drop `x` modifier in `fsbridge::fopen` call for mingw builds (Hennadii Stepanov)

Pull request description:

  Includes:
  - #29357
  - #29412
  - #29524
  - #29510
  - #29529

  Also does:
  - update to release notes
  - bump to rc2
  - manpages
  - (no changes to bitcoin.conf)

ACKs for top commit:
  achow101:
    ACK c68d4d0ac5b8537ba5e1c0c512b807768e1c5c1f

Tree-SHA512: 2f8c3dd705e3f9f33403b3cc17e8006510ff827d7dbd609b09732a1669964e9b001cfecdc63d8d8daeb8f39c652e1e4ad0aac873d44d259c21803de85688ed2b

8 files changed, 364 insertions, 44 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 3bfb606037..7570c11d3a 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4689,6 +4689,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;
@@ -4704,7 +4715,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;
             }
diff --git a/src/primitives/block.h b/src/primitives/block.h
index 99accfc7dd..832f8a03f7 100644
--- a/src/primitives/block.h
+++ b/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()
     {
@@ -95,6 +97,8 @@ public:
         CBlockHeader::SetNull();
         vtx.clear();
         fChecked = false;
+        m_checked_witness_commitment = false;
+        m_checked_merkle_root = false;
     }
 
     CBlockHeader GetBlockHeader() const
diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp
index 87ca2f6aed..db73c19735 100644
--- a/src/test/fuzz/util.cpp
+++ b/src/test/fuzz/util.cpp
@@ -255,7 +255,7 @@ FILE* FuzzedFileProvider::open()
         [&] {
             mode = "a+";
         });
-#if defined _GNU_SOURCE && !defined __ANDROID__
+#if defined _GNU_SOURCE && (defined(__linux__) || defined(__FreeBSD__))
     const cookie_io_functions_t io_hooks = {
         FuzzedFileProvider::read,
         FuzzedFileProvider::write,
diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
index f03f7c1da2..9cca92501b 100644
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -29,7 +29,14 @@ BOOST_AUTO_TEST_CASE(xor_file)
         BOOST_CHECK_EXCEPTION(xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: file handle is nullpt"});
     }
     {
-        AutoFile xor_file{raw_file("wbx"), xor_pat};
+#ifdef __MINGW64__
+        // Our usage of mingw-w64 and the msvcrt runtime does not support
+        // the x modifier for the _wfopen().
+        const char* mode = "wb";
+#else
+        const char* mode = "wbx";
+#endif
+        AutoFile xor_file{raw_file(mode), xor_pat};
         xor_file << test1 << test2;
     }
     {
diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 14440571eb..fd5ea49ec1 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -4,12 +4,17 @@
 
 #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 <util/chaintype.h>
 #include <validation.h>
 
+#include <string>
+
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
@@ -145,4 +150,215 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo)
     BOOST_CHECK_EQUAL(out110_2.nChainTx, 111U);
 }
 
+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()
diff --git a/src/validation.cpp b/src/validation.cpp
index 5d435fef8b..f02d1e32a4 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3621,6 +3621,87 @@ 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 merkle_root = BlockMerkleRoot(block, &mutated);
+    if (block.hashMerkleRoot != merkle_root) {
+        return state.Invalid(
+            /*result=*/BlockValidationResult::BLOCK_MUTATED,
+            /*reject_reason=*/"bad-txnmrklroot",
+            /*debug_message=*/"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(
+            /*result=*/BlockValidationResult::BLOCK_MUTATED,
+            /*reject_reason=*/"bad-txns-duplicate",
+            /*debug_message=*/"duplicate transaction");
+    }
+
+    block.m_checked_merkle_root = true;
+    return true;
+}
+
+/** CheckWitnessMalleation performs checks for block malleation with regard to
+ * its witnesses.
+ *
+ * Note: If the witness commitment is expected (i.e. `expect_witness_commitment
+ * = true`), then the block is required to have at least one transaction and the
+ * first transaction needs to have at least one input. */
+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) {
+            assert(!block.vtx.empty() && !block.vtx[0]->vin.empty());
+            const auto& witness_stack{block.vtx[0]->vin[0].scriptWitness.stack};
+
+            if (witness_stack.size() != 1 || witness_stack[0].size() != 32) {
+                return state.Invalid(
+                    /*result=*/BlockValidationResult::BLOCK_MUTATED,
+                    /*reject_reason=*/"bad-witness-nonce-size",
+                    /*debug_message=*/strprintf("%s : invalid witness reserved value size", __func__));
+            }
+
+            // The malleation check is ignored; as the transaction tree itself
+            // already does not permit it, it is impossible to trigger in the
+            // witness tree.
+            uint256 hash_witness = BlockWitnessMerkleRoot(block, /*mutated=*/nullptr);
+
+            CHash256().Write(hash_witness).Write(witness_stack[0]).Finalize(hash_witness);
+            if (memcmp(hash_witness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
+                return state.Invalid(
+                    /*result=*/BlockValidationResult::BLOCK_MUTATED,
+                    /*reject_reason=*/"bad-witness-merkle-match",
+                    /*debug_message=*/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(
+                /*result=*/BlockValidationResult::BLOCK_MUTATED,
+                /*reject_reason=*/"unexpected-witness",
+                /*debug_message=*/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.
@@ -3639,17 +3720,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
@@ -3740,6 +3812,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};
@@ -3848,33 +3951,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,
diff --git a/src/validation.h b/src/validation.h
index 7ce60da634..e9f21528c6 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -360,6 +360,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);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 78febb8195..31372cb3da 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2561,8 +2561,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;
     }