diff options
-rw-r--r-- | doc/bips.md | 1 | ||||
-rw-r--r-- | doc/release-notes-30647.md | 4 | ||||
-rw-r--r-- | src/bench/wallet_create.cpp | 2 | ||||
-rw-r--r-- | src/consensus/consensus.h | 7 | ||||
-rw-r--r-- | src/consensus/params.h | 4 | ||||
-rw-r--r-- | src/kernel/chainparams.cpp | 4 | ||||
-rw-r--r-- | src/node/miner.cpp | 8 | ||||
-rw-r--r-- | src/test/fuzz/utxo_snapshot.cpp | 111 | ||||
-rw-r--r-- | src/test/policyestimator_tests.cpp | 5 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 50 | ||||
-rw-r--r-- | src/test/util/setup_common.h | 4 | ||||
-rw-r--r-- | src/validation.cpp | 17 | ||||
-rw-r--r-- | src/wallet/load.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpc/wallet.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/util.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 60 | ||||
-rw-r--r-- | src/wallet/wallet.h | 9 | ||||
-rwxr-xr-x | test/functional/feature_assumeutxo.py | 25 | ||||
-rwxr-xr-x | test/functional/feature_blocksxor.py | 65 | ||||
-rwxr-xr-x | test/functional/feature_reindex.py | 16 | ||||
-rwxr-xr-x | test/functional/mining_basic.py | 45 | ||||
-rwxr-xr-x | test/functional/rpc_blockchain.py | 2 | ||||
-rw-r--r-- | test/functional/test_framework/util.py | 13 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 1 |
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', |