aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-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
16 files changed, 186 insertions, 103 deletions
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);
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-21 04:12:13 +0000