phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-29 14:59:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#29531: [25.x] backports

Browse source 
27cfda1bae doc: Update release notes for 25.2rc2 (Ava Chow)
daba5e2c5b doc: Update manpages for 25.2rc2 (Ava Chow)
8a0c980d6e build: Bump to 25.2rc2 (Ava Chow)
cf7d3a8cd0 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders)
3eaaafa225 [test] IsBlockMutated unit tests (dergoegge)
0667441a7b [validation] Cache merkle root and witness commitment checks (dergoegge)
de97ecf14f [test] Add regression test for #27608 (dergoegge)
8cc4b24c74 [net processing] Don't process mutated blocks (dergoegge)
098f07dc8d [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge)
8804c368f5 [validation] Introduce IsBlockMutated (dergoegge)
4f5baac6ca [validation] Isolate merkle root checks (dergoegge)
f93be0103f test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6)
7c08ccf19b wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6)

Pull request description:

  Backport:

  * #29510
  * #29412
  * #29524

ACKs for top commit:
  glozow:
    utACK 27cfda1bae

Tree-SHA512: 37feadd65d9ea55c0a92c9d2a6f74f87cafed3bc67f8deeaaafc5b7042f954e55ea34816612e1a49088f4f1906f104e00c7c3bec7affd1c1f48220b57a8769c5
This commit is contained in:
fanquake 2024-03-22 13:39:23 +00:00
commit d5bad0d2d1
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
17 changed files with 491 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
configure.ac
View file

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

6
doc/man/bitcoin-cli.1
View file

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

6
doc/man/bitcoin-qt.1
View file

@ -1,12 +1,12 @@
.\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. .\" 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" "March 2024" "bitcoin-qt v25.2.0rc2" "User Commands"
.SH NAME .SH NAME
bitcoin-qt \- manual page for bitcoin-qt v25.2.0rc1 bitcoin-qt \- manual page for bitcoin-qt v25.2.0rc2
.SH SYNOPSIS .SH SYNOPSIS
.B bitcoin-qt .B bitcoin-qt
[\fI\,command-line options\/\fR] [\fI\,command-line options\/\fR]
.SH DESCRIPTION .SH DESCRIPTION
Bitcoin Core version v25.2.0rc1 Bitcoin Core version v25.2.0rc2
.SH OPTIONS .SH OPTIONS
.HP .HP
\-? \-?

6
doc/man/bitcoin-tx.1
View file

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

6
doc/man/bitcoin-util.1
View file

@ -1,12 +1,12 @@
.\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. .\" 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" "March 2024" "bitcoin-util v25.2.0rc2" "User Commands"
.SH NAME .SH NAME
bitcoin-util \- manual page for bitcoin-util v25.2.0rc1 bitcoin-util \- manual page for bitcoin-util v25.2.0rc2
.SH SYNOPSIS .SH SYNOPSIS
.B bitcoin-util .B bitcoin-util
[\fI\,options\/\fR] [\fI\,commands\/\fR] \fI\,Do stuff\/\fR [\fI\,options\/\fR] [\fI\,commands\/\fR] \fI\,Do stuff\/\fR
.SH DESCRIPTION .SH DESCRIPTION
Bitcoin Core bitcoin\-util utility version v25.2.0rc1 Bitcoin Core bitcoin\-util utility version v25.2.0rc2
.SH OPTIONS .SH OPTIONS
.HP .HP
\-? \-?

6
doc/man/bitcoin-wallet.1
View file

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

6
doc/man/bitcoind.1
View file

@ -1,12 +1,12 @@
.\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. .\" 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" "March 2024" "bitcoind v25.2.0rc2" "User Commands"
.SH NAME .SH NAME
bitcoind \- manual page for bitcoind v25.2.0rc1 bitcoind \- manual page for bitcoind v25.2.0rc2
.SH SYNOPSIS .SH SYNOPSIS
.B bitcoind .B bitcoind
[\fI\,options\/\fR] \fI\,Start Bitcoin Core\/\fR [\fI\,options\/\fR] \fI\,Start Bitcoin Core\/\fR
.SH DESCRIPTION .SH DESCRIPTION
Bitcoin Core version v25.2.0rc1 Bitcoin Core version v25.2.0rc2
.SH OPTIONS .SH OPTIONS
.HP .HP
\-? \-?

15
doc/release-notes.md
View file

@ -1,9 +1,9 @@
25.2rc1 Release Notes 25.2rc2 Release Notes
================== ==================
Bitcoin Core version 25.2rc1 is now available from: Bitcoin Core version 25.2rc2 is now available from:
<https://bitcoincore.org/bin/bitcoin-core-25.2/test.rc1> <https://bitcoincore.org/bin/bitcoin-core-25.2/test.rc2>
This release includes various bug fixes and performance This release includes various bug fixes and performance
improvements, as well as updated translations. improvements, as well as updated translations.
@ -51,6 +51,12 @@ Notable changes
### Wallet ### Wallet
- #29176 wallet: Fix use-after-free in WalletBatch::EraseRecords - #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 Credits
======= =======
@ -60,6 +66,9 @@ Thanks to everyone who directly contributed to this release:
- Martin Zumsande - Martin Zumsande
- Sebastian Falbesoner - Sebastian Falbesoner
- MarcoFalke - MarcoFalke
- UdjinM6
- dergoegge
- Greg Sanders
As well as to everyone that helped with translations on As well as to everyone that helped with translations on
[Transifex](https://www.transifex.com/bitcoin/bitcoin/). [Transifex](https://www.transifex.com/bitcoin/bitcoin/).

12
src/net_processing.cpp
View file

@ -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()); 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; bool forceProcessing = false;
const uint256 hash(pblock->GetHash()); const uint256 hash(pblock->GetHash());
bool min_pow_checked = false; 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)); mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true));
// Check work on this block against our anti-dos thresholds. // 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()) { if (prev_block && prev_block->nChainWork + CalculateHeadersWork({pblock->GetBlockHeader()}) >= GetAntiDoSWorkThreshold()) {
min_pow_checked = true; min_pow_checked = true;
} }

8
src/primitives/block.h
View file

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

216
src/test/validation_tests.cpp
View file

@ -4,11 +4,16 @@
#include <chainparams.h> #include <chainparams.h>
#include <consensus/amount.h> #include <consensus/amount.h>
#include <consensus/merkle.h>
#include <core_io.h>
#include <hash.h>
#include <net.h> #include <net.h>
#include <signet.h> #include <signet.h>
#include <uint256.h> #include <uint256.h>
#include <validation.h> #include <validation.h>
#include <string>
#include <test/util/setup_common.h> #include <test/util/setup_common.h>
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
@ -144,4 +149,215 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo)
BOOST_CHECK_EQUAL(out210.nChainTx, 200U); 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() BOOST_AUTO_TEST_SUITE_END()

127
src/validation.cpp
View file

@ -3486,6 +3486,60 @@ static bool CheckBlockHeader(const CBlockHeader& block, BlockValidationState& st
return true; 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) bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW, bool fCheckMerkleRoot)
{ {
// These are checks that are independent of context. // 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. // Check the merkle root.
if (fCheckMerkleRoot) { if (fCheckMerkleRoot && !CheckMerkleRoot(block, state)) {
bool mutated; return false;
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");
} }
// All potential-corruption validation must be done before we do any // 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);}); [&](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 CalculateHeadersWork(const std::vector<CBlockHeader>& headers)
{ {
arith_uint256 total_work{0}; 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 // * 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 // {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. // multiple, the last one is used.
bool fHaveWitness = false; if (!CheckWitnessMalleation(block, DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT), state)) {
if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_SEGWIT)) { return false;
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__));
}
}
} }
// After the coinbase witness reserved value and commitment are verified, // After the coinbase witness reserved value and commitment are verified,

3
src/validation.h
View file

@ -352,6 +352,9 @@ bool TestBlockValidity(BlockValidationState& state,
/** Check with the proof of work on each blockheader matches the value in nBits */ /** 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); 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 */ /** Return the sum of the work on a given set of headers */
arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers); arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers);

4
src/wallet/wallet.cpp
View file

@ -2596,8 +2596,10 @@ util::Result<CTxDestination> ReserveDestination::GetReservedDestination(bool int
if (nIndex == -1) { if (nIndex == -1) {
CKeyPool keypool; 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; if (!op_address) return op_address;
nIndex = index;
address = *op_address; address = *op_address;
fInternal = keypool.fInternal; fInternal = keypool.fInternal;
} }

116
test/functional/p2p_mutated_blocks.py Executable file
View file

@ -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()

1
test/functional/test_runner.py
View file

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

16
test/functional/wallet_keypool.py
View file

@ -103,11 +103,18 @@ class KeyPoolTest(BitcoinTestFramework):
nodes[0].getrawchangeaddress() nodes[0].getrawchangeaddress()
nodes[0].getrawchangeaddress() 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 # the next one should fail
assert_raises_rpc_error(-12, "Keypool ran out", nodes[0].getrawchangeaddress) 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 # 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")) 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"))
addr.add(nodes[0].getnewaddress(address_type="bech32")) addr.add(nodes[0].getnewaddress(address_type="bech32"))
assert len(addr) == 6 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 # the next one should fail
assert_raises_rpc_error(-12, "Error: Keypool ran out, please call keypoolrefill first", nodes[0].getnewaddress) 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 # refill keypool with three new addresses
nodes[0].walletpassphrase('test', 1) nodes[0].walletpassphrase('test', 1)