aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--doc/bips.md1
-rw-r--r--doc/release-notes-30647.md4
-rw-r--r--src/bench/wallet_create.cpp2
-rw-r--r--src/consensus/consensus.h7
-rw-r--r--src/consensus/params.h4
-rw-r--r--src/kernel/chainparams.cpp4
-rw-r--r--src/node/miner.cpp8
-rw-r--r--src/test/fuzz/utxo_snapshot.cpp111
-rw-r--r--src/test/policyestimator_tests.cpp5
-rw-r--r--src/test/util/setup_common.cpp50
-rw-r--r--src/test/util/setup_common.h4
-rw-r--r--src/validation.cpp17
-rw-r--r--src/wallet/load.cpp2
-rw-r--r--src/wallet/rpc/wallet.cpp2
-rw-r--r--src/wallet/test/util.cpp2
-rw-r--r--src/wallet/test/wallet_tests.cpp2
-rw-r--r--src/wallet/wallet.cpp60
-rw-r--r--src/wallet/wallet.h9
-rwxr-xr-xtest/functional/feature_assumeutxo.py25
-rwxr-xr-xtest/functional/feature_blocksxor.py65
-rwxr-xr-xtest/functional/feature_reindex.py16
-rwxr-xr-xtest/functional/mining_basic.py45
-rwxr-xr-xtest/functional/rpc_blockchain.py2
-rw-r--r--test/functional/test_framework/util.py13
-rwxr-xr-xtest/functional/test_runner.py1
25 files changed, 347 insertions, 114 deletions
diff --git a/doc/bips.md b/doc/bips.md
index 87258fce93..51ffe00a76 100644
--- a/doc/bips.md
+++ b/doc/bips.md
@@ -30,6 +30,7 @@ BIPs that are implemented by Bitcoin Core:
* [`BIP 84`](https://github.com/bitcoin/bips/blob/master/bip-0084.mediawiki): The experimental descriptor wallets introduced in **v0.21.0** by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 84. ([PR #16528](https://github.com/bitcoin/bitcoin/pull/16528))
* [`BIP 86`](https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki): Descriptor wallets by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 86 since **v23.0** ([PR #22364](https://github.com/bitcoin/bitcoin/pull/22364)).
* [`BIP 90`](https://github.com/bitcoin/bips/blob/master/bip-0090.mediawiki): Trigger mechanism for activation of BIPs 34, 65, and 66 has been simplified to block height checks since **v0.14.0** ([PR #8391](https://github.com/bitcoin/bitcoin/pull/8391)).
+* [`BIP 94`](https://github.com/bitcoin/bips/blob/master/bip-0094.mediawiki): Testnet 4 (`-testnet4`) supported as of **v28.0** ([PR #29775](https://github.com/bitcoin/bitcoin/pull/29775)).
* [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641)).
* [`BIP 112`](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki): The CHECKSEQUENCEVERIFY opcode has been implemented since **v0.12.1** ([PR #7524](https://github.com/bitcoin/bitcoin/pull/7524)), and has been *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)).
* [`BIP 113`](https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki): Median time past lock-time calculations have been implemented since **v0.12.1** ([PR #6566](https://github.com/bitcoin/bitcoin/pull/6566)), and has been *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)).
diff --git a/doc/release-notes-30647.md b/doc/release-notes-30647.md
new file mode 100644
index 0000000000..ca91f0aaeb
--- /dev/null
+++ b/doc/release-notes-30647.md
@@ -0,0 +1,4 @@
+Tests
+-----
+
+- The BIP94 timewarp attack mitigation is now active on the `regtest` network
diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp
index 5c0557bf6f..1adced1e99 100644
--- a/src/bench/wallet_create.cpp
+++ b/src/bench/wallet_create.cpp
@@ -42,7 +42,7 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted)
// Release wallet
RemoveWallet(context, wallet, /*load_on_start=*/ std::nullopt);
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
fs::remove_all(wallet_path);
});
}
diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h
index 384f70bc10..cffe9cdafd 100644
--- a/src/consensus/consensus.h
+++ b/src/consensus/consensus.h
@@ -27,4 +27,11 @@ static const size_t MIN_SERIALIZABLE_TRANSACTION_WEIGHT = WITNESS_SCALE_FACTOR *
/** Interpret sequence numbers as relative lock-time constraints. */
static constexpr unsigned int LOCKTIME_VERIFY_SEQUENCE = (1 << 0);
+/**
+ * Maximum number of seconds that the timestamp of the first
+ * block of a difficulty adjustment period is allowed to
+ * be earlier than the last block of the previous period (BIP94).
+ */
+static constexpr int64_t MAX_TIMEWARP = 600;
+
#endif // BITCOIN_CONSENSUS_CONSENSUS_H
diff --git a/src/consensus/params.h b/src/consensus/params.h
index d970e41637..eadfe2ba90 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -108,6 +108,10 @@ struct Params {
/** Proof of work parameters */
uint256 powLimit;
bool fPowAllowMinDifficultyBlocks;
+ /**
+ * Enfore BIP94 timewarp attack mitigation. On testnet4 this also enforces
+ * the block storm mitigation.
+ */
bool enforce_BIP94;
bool fPowNoRetargeting;
int64_t nPowTargetSpacing;
diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
index 79d8285847..3bd662b684 100644
--- a/src/kernel/chainparams.cpp
+++ b/src/kernel/chainparams.cpp
@@ -538,10 +538,10 @@ public:
consensus.SegwitHeight = 0; // Always active unless overridden
consensus.MinBIP9WarningHeight = 0;
consensus.powLimit = uint256{"7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"};
- consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
+ consensus.nPowTargetTimespan = 24 * 60 * 60; // one day
consensus.nPowTargetSpacing = 10 * 60;
consensus.fPowAllowMinDifficultyBlocks = true;
- consensus.enforce_BIP94 = false;
+ consensus.enforce_BIP94 = true;
consensus.fPowNoRetargeting = true;
consensus.nRuleChangeActivationThreshold = 108; // 75% for testchains
consensus.nMinerConfirmationWindow = 144; // Faster than normal for regtest (144 instead of 2016)
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index fa2d979b86..5c476e154f 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -33,6 +33,14 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam
int64_t nOldTime = pblock->nTime;
int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))};
+ if (consensusParams.enforce_BIP94) {
+ // Height of block to be mined.
+ const int height{pindexPrev->nHeight + 1};
+ if (height % consensusParams.DifficultyAdjustmentInterval() == 0) {
+ nNewTime = std::max<int64_t>(nNewTime, pindexPrev->GetBlockTime() - MAX_TIMEWARP);
+ }
+ }
+
if (nOldTime < nNewTime) {
pblock->nTime = nNewTime;
}
diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
index 1d90414443..21c305e222 100644
--- a/src/test/fuzz/utxo_snapshot.cpp
+++ b/src/test/fuzz/utxo_snapshot.cpp
@@ -1,45 +1,79 @@
-// Copyright (c) 2021-2022 The Bitcoin Core developers
+// Copyright (c) 2021-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#include <chain.h>
#include <chainparams.h>
+#include <coins.h>
+#include <consensus/consensus.h>
#include <consensus/validation.h>
+#include <node/blockstorage.h>
#include <node/utxo_snapshot.h>
+#include <primitives/block.h>
+#include <primitives/transaction.h>
+#include <serialize.h>
+#include <span.h>
+#include <streams.h>
+#include <sync.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <test/util/mining.h>
#include <test/util/setup_common.h>
-#include <util/chaintype.h>
+#include <uint256.h>
+#include <util/check.h>
#include <util/fs.h>
+#include <util/result.h>
#include <validation.h>
-#include <validationinterface.h>
+
+#include <cstdint>
+#include <functional>
+#include <ios>
+#include <memory>
+#include <optional>
+#include <vector>
using node::SnapshotMetadata;
namespace {
const std::vector<std::shared_ptr<CBlock>>* g_chain;
+TestingSetup* g_setup;
+template <bool INVALID>
void initialize_chain()
{
const auto params{CreateChainParams(ArgsManager{}, ChainType::REGTEST)};
static const auto chain{CreateBlockChain(2 * COINBASE_MATURITY, *params)};
g_chain = &chain;
+ static const auto setup{
+ MakeNoLogFileContext<TestingSetup>(ChainType::REGTEST,
+ TestOpts{
+ .setup_net = false,
+ .setup_validation_interface = false,
+ .min_validation_cache = true,
+ }),
+ };
+ if constexpr (INVALID) {
+ auto& chainman{*setup->m_node.chainman};
+ for (const auto& block : chain) {
+ BlockValidationState dummy;
+ bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
+ Assert(processed);
+ const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
+ Assert(index);
+ }
+ }
+ g_setup = setup.get();
}
-FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
+template <bool INVALID>
+void utxo_snapshot_fuzz(FuzzBufferType buffer)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
- std::unique_ptr<const TestingSetup> setup{
- MakeNoLogFileContext<const TestingSetup>(
- ChainType::REGTEST,
- TestOpts{
- .setup_net = false,
- .setup_validation_interface = false,
- })};
- const auto& node = setup->m_node;
- auto& chainman{*node.chainman};
+ auto& setup{*g_setup};
+ bool dirty_chainman{false}; // Re-use the global chainman, but reset it when it is dirty
+ auto& chainman{*setup.m_node.chainman};
const auto snapshot_path = gArgs.GetDataDirNet() / "fuzzed_snapshot.dat";
@@ -52,7 +86,6 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
std::vector<uint8_t> metadata{ConsumeRandomLengthByteVector(fuzzed_data_provider)};
outfile << Span{metadata};
} else {
- DataStream data_stream{};
auto msg_start = chainman.GetParams().MessageStart();
int base_blockheight{fuzzed_data_provider.ConsumeIntegralInRange<int>(1, 2 * COINBASE_MATURITY)};
uint256 base_blockhash{g_chain->at(base_blockheight - 1)->GetHash()};
@@ -75,6 +108,16 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
height++;
}
}
+ if constexpr (INVALID) {
+ // Append an invalid coin to ensure invalidity. This error will be
+ // detected late in PopulateAndValidateSnapshot, and allows the
+ // INVALID fuzz target to reach more potential code coverage.
+ const auto& coinbase{g_chain->back()->vtx.back()};
+ outfile << coinbase->GetHash();
+ WriteCompactSize(outfile, 1); // number of coins for the hash
+ WriteCompactSize(outfile, 999); // index of coin
+ outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/0};
+ }
}
const auto ActivateFuzzedSnapshot{[&] {
@@ -90,12 +133,16 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
}};
if (fuzzed_data_provider.ConsumeBool()) {
- for (const auto& block : *g_chain) {
- BlockValidationState dummy;
- bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
- Assert(processed);
- const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
- Assert(index);
+ // Consume the bool, but skip the code for the INVALID fuzz target
+ if constexpr (!INVALID) {
+ for (const auto& block : *g_chain) {
+ BlockValidationState dummy;
+ bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
+ Assert(processed);
+ const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
+ Assert(index);
+ }
+ dirty_chainman = true;
}
}
@@ -119,11 +166,35 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
}
}
Assert(g_chain->size() == coinscache.GetCacheSize());
+ dirty_chainman = true;
} else {
Assert(!chainman.SnapshotBlockhash());
Assert(!chainman.ActiveChainstate().m_from_snapshot_blockhash);
}
// Snapshot should refuse to load a second time regardless of validity
Assert(!ActivateFuzzedSnapshot());
+ if constexpr (INVALID) {
+ // Activating the snapshot, or any other action that makes the chainman
+ // "dirty" can and must not happen for the INVALID fuzz target
+ Assert(!dirty_chainman);
+ }
+ if (dirty_chainman) {
+ setup.m_node.chainman.reset();
+ setup.m_make_chainman();
+ setup.LoadVerifyActivateChainstate();
+ }
}
+
+// There are two fuzz targets:
+//
+// The target 'utxo_snapshot', which allows valid snapshots, but is slow,
+// because it has to reset the chainstate manager on almost all fuzz inputs.
+// Otherwise, a dirty header tree or dirty chainstate could leak from one fuzz
+// input execution into the next, which makes execution non-deterministic.
+//
+// The target 'utxo_snapshot_invalid', which is fast and does not require any
+// expensive state to be reset.
+FUZZ_TARGET(utxo_snapshot /*valid*/, .init = initialize_chain<false>) { utxo_snapshot_fuzz<false>(buffer); }
+FUZZ_TARGET(utxo_snapshot_invalid, .init = initialize_chain<true>) { utxo_snapshot_fuzz<true>(buffer); }
+
} // namespace
diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index 6cadc3290a..83977c1c89 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -1,8 +1,9 @@
-// Copyright (c) 2011-2022 The Bitcoin Core developers
+// Copyright (c) 2011-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <policy/fees.h>
+#include <policy/fees_args.h>
#include <policy/policy.h>
#include <test/util/txmempool.h>
#include <txmempool.h>
@@ -18,7 +19,7 @@ BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, ChainTestingSetup)
BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
- CBlockPolicyEstimator& feeEst = *Assert(m_node.fee_estimator);
+ CBlockPolicyEstimator feeEst{FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES};
CTxMemPool& mpool = *Assert(m_node.mempool);
m_node.validation_signals->RegisterValidationInterface(&feeEst);
TestMemPoolEntryHelper entry;
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 3f48ea4375..62ff61b227 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2011-2022 The Bitcoin Core developers
+// Copyright (c) 2011-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -31,7 +31,6 @@
#include <node/warnings.h>
#include <noui.h>
#include <policy/fees.h>
-#include <policy/fees_args.h>
#include <pow.h>
#include <random.h>
#include <rpc/blockchain.h>
@@ -236,7 +235,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
m_node.validation_signals = std::make_unique<ValidationSignals>(std::make_unique<SerialTaskRunner>(*m_node.scheduler));
}
- m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES);
bilingual_str error{};
m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node), error);
Assert(error.empty());
@@ -246,24 +244,34 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings));
- const ChainstateManager::Options chainman_opts{
- .chainparams = chainparams,
- .datadir = m_args.GetDataDirNet(),
- .check_block_index = 1,
- .notifications = *m_node.notifications,
- .signals = m_node.validation_signals.get(),
- .worker_threads_num = 2,
- };
- const BlockManager::Options blockman_opts{
- .chainparams = chainman_opts.chainparams,
- .blocks_dir = m_args.GetBlocksDirPath(),
- .notifications = chainman_opts.notifications,
+ m_make_chainman = [this, &chainparams, opts] {
+ Assert(!m_node.chainman);
+ ChainstateManager::Options chainman_opts{
+ .chainparams = chainparams,
+ .datadir = m_args.GetDataDirNet(),
+ .check_block_index = 1,
+ .notifications = *m_node.notifications,
+ .signals = m_node.validation_signals.get(),
+ .worker_threads_num = 2,
+ };
+ if (opts.min_validation_cache) {
+ chainman_opts.script_execution_cache_bytes = 0;
+ chainman_opts.signature_cache_bytes = 0;
+ }
+ const BlockManager::Options blockman_opts{
+ .chainparams = chainman_opts.chainparams,
+ .blocks_dir = m_args.GetBlocksDirPath(),
+ .notifications = chainman_opts.notifications,
+ };
+ m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts);
+ LOCK(m_node.chainman->GetMutex());
+ m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
+ .path = m_args.GetDataDirNet() / "blocks" / "index",
+ .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
+ .memory_only = true,
+ });
};
- m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts);
- m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
- .path = m_args.GetDataDirNet() / "blocks" / "index",
- .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
- .memory_only = true});
+ m_make_chainman();
}
ChainTestingSetup::~ChainTestingSetup()
@@ -276,7 +284,7 @@ ChainTestingSetup::~ChainTestingSetup()
m_node.netgroupman.reset();
m_node.args = nullptr;
m_node.mempool.reset();
- m_node.fee_estimator.reset();
+ Assert(!m_node.fee_estimator); // Each test must create a local object, if they wish to use the fee_estimator
m_node.chainman.reset();
m_node.validation_signals.reset();
m_node.scheduler.reset();
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index 9515f0255e..b73acc1de5 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2015-2022 The Bitcoin Core developers
+// Copyright (c) 2015-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -55,6 +55,7 @@ struct TestOpts {
bool block_tree_db_in_memory{true};
bool setup_net{true};
bool setup_validation_interface{true};
+ bool min_validation_cache{false}; // Equivalent of -maxsigcachebytes=0
};
/** Basic testing setup.
@@ -81,6 +82,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
node::CacheSizes m_cache_sizes{};
bool m_coins_db_in_memory{true};
bool m_block_tree_db_in_memory{true};
+ std::function<void()> m_make_chainman{};
explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
~ChainTestingSetup();
diff --git a/src/validation.cpp b/src/validation.cpp
index 25da81ae8a..8f75b2e30a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -107,21 +107,6 @@ const std::vector<std::string> CHECKLEVEL_DOC {
* */
static constexpr int PRUNE_LOCK_BUFFER{10};
-/**
- * Maximum number of seconds that the timestamp of the first
- * block of a difficulty adjustment period is allowed to
- * be earlier than the last block of the previous period (BIP94).
- */
-static constexpr int64_t MAX_TIMEWARP{MAX_FUTURE_BLOCK_TIME};
-
-/**
- * If the timestamp of the last block in a difficulty period is set
- * MAX_FUTURE_BLOCK_TIME seconds in the future, an honest miner should
- * be able to mine the first block using the current time. This is not
- * a consensus rule, but changing MAX_TIMEWARP should be done with caution.
- */
-static_assert(MAX_FUTURE_BLOCK_TIME <= MAX_TIMEWARP);
-
GlobalMutex g_best_block_mutex;
std::condition_variable g_best_block_cv;
uint256 g_best_block;
@@ -4197,7 +4182,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio
if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast())
return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", "block's timestamp is too early");
- // Testnet4 only: Check timestamp against prev for difficulty-adjustment
+ // Testnet4 and regtest only: Check timestamp against prev for difficulty-adjustment
// blocks to prevent timewarp attacks (see https://github.com/bitcoin/bitcoin/pull/15482).
if (consensusParams.enforce_BIP94) {
// Check timestamp for the first block of each difficulty adjustment
diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
index fe35f6b223..e26347d437 100644
--- a/src/wallet/load.cpp
+++ b/src/wallet/load.cpp
@@ -178,7 +178,7 @@ void UnloadWallets(WalletContext& context)
wallets.pop_back();
std::vector<bilingual_str> warnings;
RemoveWallet(context, wallet, /* load_on_start= */ std::nullopt, warnings);
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
}
}
} // namespace wallet
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 7a0b0103c0..39582b3f6a 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -495,7 +495,7 @@ static RPCHelpMan unloadwallet()
}
}
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
UniValue result(UniValue::VOBJ);
PushWarnings(warnings, result);
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index b21a9a601d..ec6c9e6f3f 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -74,7 +74,7 @@ void TestUnloadWallet(std::shared_ptr<CWallet>&& wallet)
// Calls SyncWithValidationInterfaceQueue
wallet->chain().waitForNotificationsIfTipChanged({});
wallet->m_chain_notifications_handler.reset();
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
}
std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database)
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 53f3bcc421..44ffddb168 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -889,7 +889,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup)
context.args = &m_args;
auto wallet = TestLoadWallet(context);
BOOST_CHECK(wallet);
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
}
BOOST_FIXTURE_TEST_CASE(RemoveTxs, TestChain100Setup)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 8e31950beb..8df39b9f75 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -162,10 +162,14 @@ bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet
// Unregister with the validation interface which also drops shared pointers.
wallet->m_chain_notifications_handler.reset();
- LOCK(context.wallets_mutex);
- std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet);
- if (i == context.wallets.end()) return false;
- context.wallets.erase(i);
+ {
+ LOCK(context.wallets_mutex);
+ std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet);
+ if (i == context.wallets.end()) return false;
+ context.wallets.erase(i);
+ }
+ // Notify unload so that upper layers release the shared pointer.
+ wallet->NotifyUnload();
// Write the wallet setting
UpdateWalletSetting(chain, name, load_on_start, warnings);
@@ -223,38 +227,35 @@ static std::set<std::string> g_loading_wallet_set GUARDED_BY(g_loading_wallet_mu
static std::set<std::string> g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex);
// Custom deleter for shared_ptr<CWallet>.
-static void ReleaseWallet(CWallet* wallet)
+static void FlushAndDeleteWallet(CWallet* wallet)
{
const std::string name = wallet->GetName();
- wallet->WalletLogPrintf("Releasing wallet\n");
+ wallet->WalletLogPrintf("Releasing wallet %s..\n", name);
wallet->Flush();
delete wallet;
- // Wallet is now released, notify UnloadWallet, if any.
+ // Wallet is now released, notify WaitForDeleteWallet, if any.
{
LOCK(g_wallet_release_mutex);
if (g_unloading_wallet_set.erase(name) == 0) {
- // UnloadWallet was not called for this wallet, all done.
+ // WaitForDeleteWallet was not called for this wallet, all done.
return;
}
}
g_wallet_release_cv.notify_all();
}
-void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
+void WaitForDeleteWallet(std::shared_ptr<CWallet>&& wallet)
{
// Mark wallet for unloading.
const std::string name = wallet->GetName();
{
LOCK(g_wallet_release_mutex);
- auto it = g_unloading_wallet_set.insert(name);
- assert(it.second);
+ g_unloading_wallet_set.insert(name);
+ // Do not expect to be the only one removing this wallet.
+ // Multiple threads could simultaneously be waiting for deletion.
}
- // The wallet can be in use so it's not possible to explicitly unload here.
- // Notify the unload intent so that all remaining shared pointers are
- // released.
- wallet->NotifyUnload();
- // Time to ditch our shared_ptr and wait for ReleaseWallet call.
+ // Time to ditch our shared_ptr and wait for FlushAndDeleteWallet call.
wallet.reset();
{
WAIT_LOCK(g_wallet_release_mutex, lock);
@@ -1037,9 +1038,8 @@ bool CWallet::IsSpentKey(const CScript& scriptPubKey) const
if (IsAddressPreviouslySpent(dest)) {
return true;
}
- if (IsLegacy()) {
- LegacyScriptPubKeyMan* spk_man = GetLegacyScriptPubKeyMan();
- assert(spk_man != nullptr);
+
+ if (LegacyScriptPubKeyMan* spk_man = GetLegacyScriptPubKeyMan()) {
for (const auto& keyid : GetAffectedKeys(scriptPubKey, *spk_man)) {
WitnessV0KeyHash wpkh_dest(keyid);
if (IsAddressPreviouslySpent(wpkh_dest)) {
@@ -1625,7 +1625,9 @@ isminetype CWallet::IsMine(const CScript& script) const
}
// Legacy wallet
- if (IsLegacy()) return GetLegacyScriptPubKeyMan()->IsMine(script);
+ if (LegacyScriptPubKeyMan* spkm = GetLegacyScriptPubKeyMan()) {
+ return spkm->IsMine(script);
+ }
return ISMINE_NO;
}
@@ -2971,7 +2973,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
const auto start{SteadyClock::now()};
// TODO: Can't use std::make_shared because we need a custom deleter but
// should be possible to use std::allocate_shared.
- std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet);
+ std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet);
walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", "");
@@ -3558,7 +3560,8 @@ std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script) c
Assume(std::all_of(spk_mans.begin(), spk_mans.end(), [&script, &sigdata](ScriptPubKeyMan* spkm) { return spkm->CanProvide(script, sigdata); }));
// Legacy wallet
- if (IsLegacy() && GetLegacyScriptPubKeyMan()->CanProvide(script, sigdata)) spk_mans.insert(GetLegacyScriptPubKeyMan());
+ LegacyScriptPubKeyMan* spkm = GetLegacyScriptPubKeyMan();
+ if (spkm && spkm->CanProvide(script, sigdata)) spk_mans.insert(spkm);
return spk_mans;
}
@@ -3588,7 +3591,8 @@ std::unique_ptr<SigningProvider> CWallet::GetSolvingProvider(const CScript& scri
}
// Legacy wallet
- if (IsLegacy() && GetLegacyScriptPubKeyMan()->CanProvide(script, sigdata)) return GetLegacyScriptPubKeyMan()->GetSolvingProvider(script);
+ LegacyScriptPubKeyMan* spkm = GetLegacyScriptPubKeyMan();
+ if (spkm && spkm->CanProvide(script, sigdata)) return spkm->GetSolvingProvider(script);
return nullptr;
}
@@ -3845,11 +3849,7 @@ void CWallet::DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool intern
bool CWallet::IsLegacy() const
{
- if (m_internal_spk_managers.count(OutputType::LEGACY) == 0) {
- return false;
- }
- auto spk_man = dynamic_cast<LegacyScriptPubKeyMan*>(m_internal_spk_managers.at(OutputType::LEGACY));
- return spk_man != nullptr;
+ return !IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS);
}
DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const
@@ -4387,7 +4387,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) {
return util::Error{_("Unable to unload the wallet before migrating")};
}
- UnloadWallet(std::move(wallet));
+ WaitForDeleteWallet(std::move(wallet));
was_loaded = true;
} else {
// Check if the wallet is BDB
@@ -4531,7 +4531,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
error += _("\nUnable to cleanup failed migration");
return util::Error{error};
}
- UnloadWallet(std::move(w));
+ WaitForDeleteWallet(std::move(w));
} else {
// Unloading for wallets in local context
assert(w.use_count() == 1);
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 984a2d9c48..3ea1cf48b2 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -83,12 +83,9 @@ struct bilingual_str;
namespace wallet {
struct WalletContext;
-//! Explicitly unload and delete the wallet.
-//! Blocks the current thread after signaling the unload intent so that all
-//! wallet pointer owners release the wallet.
-//! Note that, when blocking is not required, the wallet is implicitly unloaded
-//! by the shared pointer deleter.
-void UnloadWallet(std::shared_ptr<CWallet>&& wallet);
+//! Explicitly delete the wallet.
+//! Blocks the current thread until the wallet is destructed.
+void WaitForDeleteWallet(std::shared_ptr<CWallet>&& wallet);
bool AddWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet);
bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings);
diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index fd6a9518e8..a212704311 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -350,6 +350,31 @@ class AssumeutxoTest(BitcoinTestFramework):
assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT)
assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT)
+ self.log.info("Check that UTXO-querying RPCs operate on snapshot chainstate")
+ snapshot_hash = loaded['tip_hash']
+ snapshot_num_coins = loaded['coins_loaded']
+ # coinstatsindex might be not caught up yet and is not relevant for this test, so don't use it
+ utxo_info = n1.gettxoutsetinfo(use_index=False)
+ assert_equal(utxo_info['txouts'], snapshot_num_coins)
+ assert_equal(utxo_info['height'], SNAPSHOT_BASE_HEIGHT)
+ assert_equal(utxo_info['bestblock'], snapshot_hash)
+
+ # find coinbase output at snapshot height on node0 and scan for it on node1,
+ # where the block is not available, but the snapshot was loaded successfully
+ coinbase_tx = n0.getblock(snapshot_hash, verbosity=2)['tx'][0]
+ assert_raises_rpc_error(-1, "Block not found on disk", n1.getblock, snapshot_hash)
+ coinbase_output_descriptor = coinbase_tx['vout'][0]['scriptPubKey']['desc']
+ scan_result = n1.scantxoutset('start', [coinbase_output_descriptor])
+ assert_equal(scan_result['success'], True)
+ assert_equal(scan_result['txouts'], snapshot_num_coins)
+ assert_equal(scan_result['height'], SNAPSHOT_BASE_HEIGHT)
+ assert_equal(scan_result['bestblock'], snapshot_hash)
+ scan_utxos = [(coin['txid'], coin['vout']) for coin in scan_result['unspents']]
+ assert (coinbase_tx['txid'], 0) in scan_utxos
+
+ txout_result = n1.gettxout(coinbase_tx['txid'], 0)
+ assert_equal(txout_result['scriptPubKey']['desc'], coinbase_output_descriptor)
+
def check_tx_counts(final: bool) -> None:
"""Check nTx and nChainTx intermediate values right after loading
the snapshot, and final values after the snapshot is validated."""
diff --git a/test/functional/feature_blocksxor.py b/test/functional/feature_blocksxor.py
new file mode 100755
index 0000000000..88e0244cd4
--- /dev/null
+++ b/test/functional/feature_blocksxor.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024 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 support for XORed block data and undo files (`-blocksxor` option)."""
+import os
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_node import ErrorMatch
+from test_framework.util import (
+ assert_equal,
+ assert_greater_than,
+ read_xor_key,
+ util_xor,
+)
+from test_framework.wallet import MiniWallet
+
+
+class BlocksXORTest(BitcoinTestFramework):
+ def set_test_params(self):
+ self.num_nodes = 1
+ self.extra_args = [[
+ '-blocksxor=1',
+ '-fastprune=1', # use smaller block files
+ '-datacarriersize=100000', # needed to pad transaction with MiniWallet
+ ]]
+
+ def run_test(self):
+ self.log.info("Mine some blocks, to create multiple blk*.dat/rev*.dat files")
+ node = self.nodes[0]
+ wallet = MiniWallet(node)
+ for _ in range(5):
+ wallet.send_self_transfer(from_node=node, target_weight=80000)
+ self.generate(wallet, 1)
+
+ block_files = list(node.blocks_path.glob('blk[0-9][0-9][0-9][0-9][0-9].dat'))
+ undo_files = list(node.blocks_path.glob('rev[0-9][0-9][0-9][0-9][0-9].dat'))
+ assert_equal(len(block_files), len(undo_files))
+ assert_greater_than(len(block_files), 1) # we want at least one full block file
+
+ self.log.info("Shut down node and un-XOR block/undo files manually")
+ self.stop_node(0)
+ xor_key = read_xor_key(node=node)
+ for data_file in sorted(block_files + undo_files):
+ self.log.debug(f"Rewriting file {data_file}...")
+ with open(data_file, 'rb+') as f:
+ xored_data = f.read()
+ f.seek(0)
+ f.write(util_xor(xored_data, xor_key, offset=0))
+
+ self.log.info("Check that restarting with 'blocksxor=0' fails if XOR key is present")
+ node.assert_start_raises_init_error(['-blocksxor=0'],
+ 'The blocksdir XOR-key can not be disabled when a random key was already stored!',
+ match=ErrorMatch.PARTIAL_REGEX)
+
+ self.log.info("Delete XOR key, restart node with '-blocksxor=0', check blk*.dat/rev*.dat file integrity")
+ os.remove(node.blocks_path / 'xor.dat')
+ self.start_node(0, extra_args=['-blocksxor=0'])
+ # checklevel=2 -> verify block validity + undo data
+ # nblocks=0 -> verify all blocks
+ node.verifychain(checklevel=2, nblocks=0)
+
+
+if __name__ == '__main__':
+ BlocksXORTest(__file__).main()
diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py
index 504b3dfff4..2961a2d356 100755
--- a/test/functional/feature_reindex.py
+++ b/test/functional/feature_reindex.py
@@ -12,7 +12,11 @@
from test_framework.test_framework import BitcoinTestFramework
from test_framework.messages import MAGIC_BYTES
-from test_framework.util import assert_equal
+from test_framework.util import (
+ assert_equal,
+ read_xor_key,
+ util_xor,
+)
class ReindexTest(BitcoinTestFramework):
@@ -39,15 +43,7 @@ class ReindexTest(BitcoinTestFramework):
# we're generating them rather than getting them from peers), so to
# test out-of-order handling, swap blocks 1 and 2 on disk.
blk0 = self.nodes[0].blocks_path / "blk00000.dat"
- with open(self.nodes[0].blocks_path / "xor.dat", "rb") as xor_f:
- NUM_XOR_BYTES = 8 # From InitBlocksdirXorKey::xor_key.size()
- xor_dat = xor_f.read(NUM_XOR_BYTES)
-
- def util_xor(data, key, *, offset):
- data = bytearray(data)
- for i in range(len(data)):
- data[i] ^= key[(i + offset) % len(key)]
- return bytes(data)
+ xor_dat = read_xor_key(node=self.nodes[0])
with open(blk0, 'r+b') as bf:
# Read at least the first few blocks (including genesis)
diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py
index 6a364a4815..c0df120c65 100755
--- a/test/functional/mining_basic.py
+++ b/test/functional/mining_basic.py
@@ -28,12 +28,16 @@ from test_framework.p2p import P2PDataStore
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
+ assert_greater_than_or_equal,
assert_raises_rpc_error,
get_fee,
)
from test_framework.wallet import MiniWallet
+DIFFICULTY_ADJUSTMENT_INTERVAL = 144
+MAX_FUTURE_BLOCK_TIME = 2 * 3600
+MAX_TIMEWARP = 600
VERSIONBITS_TOP_BITS = 0x20000000
VERSIONBITS_DEPLOYMENT_TESTDUMMY_BIT = 28
DEFAULT_BLOCK_MIN_TX_FEE = 1000 # default `-blockmintxfee` setting [sat/kvB]
@@ -115,6 +119,46 @@ class MiningTest(BitcoinTestFramework):
assert tx_below_min_feerate['txid'] not in block_template_txids
assert tx_below_min_feerate['txid'] not in block_txids
+ def test_timewarp(self):
+ self.log.info("Test timewarp attack mitigation (BIP94)")
+ node = self.nodes[0]
+
+ self.log.info("Mine until the last block of the retarget period")
+ blockchain_info = self.nodes[0].getblockchaininfo()
+ n = DIFFICULTY_ADJUSTMENT_INTERVAL - blockchain_info['blocks'] % DIFFICULTY_ADJUSTMENT_INTERVAL - 2
+ t = blockchain_info['time']
+
+ for _ in range(n):
+ t += 600
+ self.nodes[0].setmocktime(t)
+ self.generate(self.wallet, 1, sync_fun=self.no_op)
+
+ self.log.info("Create block two hours in the future")
+ self.nodes[0].setmocktime(t + MAX_FUTURE_BLOCK_TIME)
+ self.generate(self.wallet, 1, sync_fun=self.no_op)
+ assert_equal(node.getblock(node.getbestblockhash())['time'], t + MAX_FUTURE_BLOCK_TIME)
+
+ self.log.info("First block template of retarget period can't use wall clock time")
+ self.nodes[0].setmocktime(t)
+ # The template will have an adjusted timestamp, which we then modify
+ tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
+ assert_greater_than_or_equal(tmpl['curtime'], t + MAX_FUTURE_BLOCK_TIME - MAX_TIMEWARP)
+
+ block = CBlock()
+ block.nVersion = tmpl["version"]
+ block.hashPrevBlock = int(tmpl["previousblockhash"], 16)
+ block.nTime = tmpl["curtime"]
+ block.nBits = int(tmpl["bits"], 16)
+ block.nNonce = 0
+ block.vtx = [create_coinbase(height=int(tmpl["height"]))]
+ block.solve()
+ assert_template(node, block, None)
+
+ bad_block = copy.deepcopy(block)
+ bad_block.nTime = t
+ bad_block.solve()
+ assert_raises_rpc_error(-25, 'time-timewarp-attack', lambda: node.submitheader(hexdata=CBlockHeader(bad_block).serialize().hex()))
+
def run_test(self):
node = self.nodes[0]
self.wallet = MiniWallet(node)
@@ -322,6 +366,7 @@ class MiningTest(BitcoinTestFramework):
assert_equal(node.submitblock(hexdata=block.serialize().hex()), 'duplicate') # valid
self.test_blockmintxfee_parameter()
+ self.test_timewarp()
if __name__ == '__main__':
diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
index e5aca7f138..98147237b1 100755
--- a/test/functional/rpc_blockchain.py
+++ b/test/functional/rpc_blockchain.py
@@ -58,7 +58,7 @@ TIME_RANGE_STEP = 600 # ten-minute steps
TIME_RANGE_MTP = TIME_GENESIS_BLOCK + (HEIGHT - 6) * TIME_RANGE_STEP
TIME_RANGE_TIP = TIME_GENESIS_BLOCK + (HEIGHT - 1) * TIME_RANGE_STEP
TIME_RANGE_END = TIME_GENESIS_BLOCK + HEIGHT * TIME_RANGE_STEP
-DIFFICULTY_ADJUSTMENT_INTERVAL = 2016
+DIFFICULTY_ADJUSTMENT_INTERVAL = 144
class BlockchainTest(BitcoinTestFramework):
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index de756691e0..c3bc861cf6 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -311,6 +311,13 @@ def sha256sum_file(filename):
return h.digest()
+def util_xor(data, key, *, offset):
+ data = bytearray(data)
+ for i in range(len(data)):
+ data[i] ^= key[(i + offset) % len(key)]
+ return bytes(data)
+
+
# RPC/P2P connection constants and functions
############################################
@@ -508,6 +515,12 @@ def check_node_connections(*, node, num_in, num_out):
assert_equal(info["connections_out"], num_out)
+def read_xor_key(*, node):
+ with open(node.blocks_path / "xor.dat", "rb") as xor_f:
+ NUM_XOR_BYTES = 8 # From InitBlocksdirXorKey::xor_key.size()
+ return xor_f.read(NUM_XOR_BYTES)
+
+
# Transaction/Block functions
#############################
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 67693259d3..b85bf1c668 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -284,6 +284,7 @@ BASE_SCRIPTS = [
'mempool_package_limits.py',
'mempool_package_rbf.py',
'feature_versionbits_warning.py',
+ 'feature_blocksxor.py',
'rpc_preciousblock.py',
'wallet_importprunedfunds.py --legacy-wallet',
'wallet_importprunedfunds.py --descriptors',