aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/headerssync.cpp5
-rw-r--r--src/httpserver.cpp83
-rw-r--r--src/init.cpp14
-rw-r--r--src/policy/fees.cpp33
-rw-r--r--src/policy/fees.h22
-rw-r--r--src/qt/bitcoingui.cpp10
-rw-r--r--src/test/fuzz/policy_estimator.cpp2
-rw-r--r--src/test/fuzz/policy_estimator_io.cpp2
-rw-r--r--src/test/util/setup_common.cpp2
-rw-r--r--src/test/util_tests.cpp25
-rw-r--r--src/util/vector.h18
-rw-r--r--src/wallet/scriptpubkeyman.cpp17
-rw-r--r--src/wallet/scriptpubkeyman.h6
-rw-r--r--src/wallet/wallet.cpp19
14 files changed, 222 insertions, 36 deletions
diff --git a/src/headerssync.cpp b/src/headerssync.cpp
index a3adfb4f70..f891063cd2 100644
--- a/src/headerssync.cpp
+++ b/src/headerssync.cpp
@@ -7,6 +7,7 @@
#include <pow.h>
#include <timedata.h>
#include <util/check.h>
+#include <util/vector.h>
// The two constants below are computed using the simulation script on
// https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1
@@ -51,9 +52,9 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus
void HeadersSyncState::Finalize()
{
Assume(m_download_state != State::FINAL);
- m_header_commitments = {};
+ ClearShrink(m_header_commitments);
m_last_header_received.SetNull();
- m_redownloaded_headers = {};
+ ClearShrink(m_redownloaded_headers);
m_redownload_buffer_last_hash.SetNull();
m_redownload_buffer_first_prev_hash.SetNull();
m_process_all_remaining_headers = false;
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 8e49f9c0f4..e140702e98 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -15,6 +15,7 @@
#include <rpc/protocol.h> // For HTTP status codes
#include <shutdown.h>
#include <sync.h>
+#include <util/check.h>
#include <util/strencodings.h>
#include <util/syscall_sandbox.h>
#include <util/system.h>
@@ -28,7 +29,7 @@
#include <memory>
#include <optional>
#include <string>
-#include <unordered_set>
+#include <unordered_map>
#include <sys/types.h>
#include <sys/stat.h>
@@ -149,10 +150,61 @@ static GlobalMutex g_httppathhandlers_mutex;
static std::vector<HTTPPathHandler> pathHandlers GUARDED_BY(g_httppathhandlers_mutex);
//! Bound listening sockets
static std::vector<evhttp_bound_socket *> boundSockets;
+
+/**
+ * @brief Helps keep track of open `evhttp_connection`s with active `evhttp_requests`
+ *
+ */
+class HTTPRequestTracker
+{
+private:
+ mutable Mutex m_mutex;
+ mutable std::condition_variable m_cv;
+ //! For each connection, keep a counter of how many requests are open
+ std::unordered_map<const evhttp_connection*, size_t> m_tracker GUARDED_BY(m_mutex);
+
+ void RemoveConnectionInternal(const decltype(m_tracker)::iterator it) EXCLUSIVE_LOCKS_REQUIRED(m_mutex)
+ {
+ m_tracker.erase(it);
+ if (m_tracker.empty()) m_cv.notify_all();
+ }
+public:
+ //! Increase request counter for the associated connection by 1
+ void AddRequest(evhttp_request* req) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ {
+ const evhttp_connection* conn{Assert(evhttp_request_get_connection(Assert(req)))};
+ WITH_LOCK(m_mutex, ++m_tracker[conn]);
+ }
+ //! Decrease request counter for the associated connection by 1, remove connection if counter is 0
+ void RemoveRequest(evhttp_request* req) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ {
+ const evhttp_connection* conn{Assert(evhttp_request_get_connection(Assert(req)))};
+ LOCK(m_mutex);
+ auto it{m_tracker.find(conn)};
+ if (it != m_tracker.end() && it->second > 0) {
+ if (--(it->second) == 0) RemoveConnectionInternal(it);
+ }
+ }
+ //! Remove a connection entirely
+ void RemoveConnection(const evhttp_connection* conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ {
+ LOCK(m_mutex);
+ auto it{m_tracker.find(Assert(conn))};
+ if (it != m_tracker.end()) RemoveConnectionInternal(it);
+ }
+ size_t CountActiveConnections() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ {
+ return WITH_LOCK(m_mutex, return m_tracker.size());
+ }
+ //! Wait until there are no more connections with active requests in the tracker
+ void WaitUntilEmpty() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ {
+ WAIT_LOCK(m_mutex, lock);
+ m_cv.wait(lock, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_tracker.empty(); });
+ }
+};
//! Track active requests
-static GlobalMutex g_requests_mutex;
-static std::condition_variable g_requests_cv;
-static std::unordered_set<evhttp_request*> g_requests GUARDED_BY(g_requests_mutex);
+static HTTPRequestTracker g_requests;
/** Check if a network address is allowed to access the HTTP server */
static bool ClientAllowed(const CNetAddr& netaddr)
@@ -214,20 +266,20 @@ std::string RequestMethodString(HTTPRequest::RequestMethod m)
/** HTTP request callback */
static void http_request_cb(struct evhttp_request* req, void* arg)
{
- // Track requests and notify when a request is completed.
+ evhttp_connection* conn{evhttp_request_get_connection(req)};
+ // Track active requests
{
- WITH_LOCK(g_requests_mutex, g_requests.insert(req));
- g_requests_cv.notify_all();
+ g_requests.AddRequest(req);
evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) {
- auto n{WITH_LOCK(g_requests_mutex, return g_requests.erase(req))};
- assert(n == 1);
- g_requests_cv.notify_all();
+ g_requests.RemoveRequest(req);
+ }, nullptr);
+ evhttp_connection_set_closecb(conn, [](evhttp_connection* conn, void* arg) {
+ g_requests.RemoveConnection(conn);
}, nullptr);
}
// Disable reading to work around a libevent bug, fixed in 2.2.0.
if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02020001) {
- evhttp_connection* conn = evhttp_request_get_connection(req);
if (conn) {
bufferevent* bev = evhttp_connection_get_bufferevent(conn);
if (bev) {
@@ -477,13 +529,10 @@ void StopHTTPServer()
}
boundSockets.clear();
{
- WAIT_LOCK(g_requests_mutex, lock);
- if (!g_requests.empty()) {
- LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", g_requests.size());
+ if (const auto n_connections{g_requests.CountActiveConnections()}; n_connections != 0) {
+ LogPrint(BCLog::HTTP, "Waiting for %d connections to stop HTTP server\n", n_connections);
}
- g_requests_cv.wait(lock, []() EXCLUSIVE_LOCKS_REQUIRED(g_requests_mutex) {
- return g_requests.empty();
- });
+ g_requests.WaitUntilEmpty();
}
if (eventHTTP) {
// Schedule a callback to call evhttp_free in the event base thread, so
diff --git a/src/init.cpp b/src/init.cpp
index 1122496539..aa8d4b937c 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -79,6 +79,7 @@
#include <util/system.h>
#include <util/thread.h>
#include <util/threadnames.h>
+#include <util/time.h>
#include <util/translation.h>
#include <validation.h>
#include <validationinterface.h>
@@ -579,6 +580,7 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !testnetChainParams->RequireStandard()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-incrementalrelayfee=<amt>", strprintf("Fee rate (in %s/kvB) used to define cost of relay, used for mempool limiting and replacement policy. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-dustrelayfee=<amt>", strprintf("Fee rate (in %s/kvB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
+ argsman.AddArg("-acceptstalefeeestimates", strprintf("Read fee estimates even if they are stale (%sdefault: %u) fee estimates are considered stale if they are %s hours old", "regtest only; ", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES, Ticks<std::chrono::hours>(MAX_FILE_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
@@ -1251,7 +1253,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
assert(!node.fee_estimator);
// Don't initialize fee estimation with old data if we don't relay transactions,
// as they would never get updated.
- if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(args));
+ if (!ignores_incoming_txs) {
+ bool read_stale_estimates = args.GetBoolArg("-acceptstalefeeestimates", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES);
+ if (read_stale_estimates && (chainparams.NetworkIDString() != CBaseChainParams::REGTEST)) {
+ return InitError(strprintf(_("acceptstalefeeestimates is not supported on %s chain."), chainparams.NetworkIDString()));
+ }
+ node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(args), read_stale_estimates);
+
+ // Flush estimates to disk periodically
+ CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get();
+ node.scheduler->scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL);
+ }
// Check port numbers
for (const std::string port_option : {
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index 6121224979..ee9d4222d3 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -24,6 +24,7 @@
#include <algorithm>
#include <cassert>
+#include <chrono>
#include <cmath>
#include <cstddef>
#include <cstdint>
@@ -527,7 +528,7 @@ bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock)
}
}
-CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath)
+CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates)
: m_estimation_filepath{estimation_filepath}
{
static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero");
@@ -545,9 +546,22 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath
shortStats = std::unique_ptr<TxConfirmStats>(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE));
longStats = std::unique_ptr<TxConfirmStats>(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE));
- // If the fee estimation file is present, read recorded estimations
AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "rb")};
- if (est_file.IsNull() || !Read(est_file)) {
+
+ // Whenever the fee estimation file is not present return early
+ if (est_file.IsNull()) {
+ LogPrintf("%s is not found. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
+ return;
+ }
+
+ std::chrono::hours file_age = GetFeeEstimatorFileAge();
+ // fee estimate file must not be too old to avoid wrong fee estimates.
+ if (file_age > MAX_FILE_AGE && !read_stale_estimates) {
+ LogPrintf("Fee estimation file %s too old (age=%lld > %lld hours) and will not be used to avoid serving stale estimates.\n", fs::PathToString(m_estimation_filepath), Ticks<std::chrono::hours>(file_age), Ticks<std::chrono::hours>(MAX_FILE_AGE));
+ return;
+ }
+
+ if (!Read(est_file)) {
LogPrintf("Failed to read fee estimates from %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
}
}
@@ -903,10 +917,16 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
void CBlockPolicyEstimator::Flush() {
FlushUnconfirmed();
+ FlushFeeEstimates();
+}
+void CBlockPolicyEstimator::FlushFeeEstimates()
+{
AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "wb")};
if (est_file.IsNull() || !Write(est_file)) {
LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
+ } else {
+ LogPrintf("Flushed fee estimates to %s.\n", fs::PathToString(m_estimation_filepath.filename()));
}
}
@@ -1011,6 +1031,13 @@ void CBlockPolicyEstimator::FlushUnconfirmed()
LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear));
}
+std::chrono::hours CBlockPolicyEstimator::GetFeeEstimatorFileAge()
+{
+ auto file_time = std::filesystem::last_write_time(m_estimation_filepath);
+ auto now = std::filesystem::file_time_type::clock::now();
+ return std::chrono::duration_cast<std::chrono::hours>(now - file_time);
+}
+
static std::set<double> MakeFeeSet(const CFeeRate& min_incremental_fee,
double max_filter_fee_rate,
double fee_filter_spacing)
diff --git a/src/policy/fees.h b/src/policy/fees.h
index 775a72a764..52761f03ca 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -14,12 +14,25 @@
#include <util/fs.h>
#include <array>
+#include <chrono>
#include <map>
#include <memory>
#include <set>
#include <string>
#include <vector>
+
+// How often to flush fee estimates to fee_estimates.dat.
+static constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1};
+
+/** fee_estimates.dat that are more than 60 hours (2.5 days) will not be read,
+ * as the estimates in the file are stale.
+ */
+static constexpr std::chrono::hours MAX_FILE_AGE{60};
+
+// Whether we allow importing a fee_estimates file older than MAX_FILE_AGE.
+static constexpr bool DEFAULT_ACCEPT_STALE_FEE_ESTIMATES{false};
+
class AutoFile;
class CTxMemPoolEntry;
class TxConfirmStats;
@@ -183,7 +196,7 @@ private:
const fs::path m_estimation_filepath;
public:
/** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */
- CBlockPolicyEstimator(const fs::path& estimation_filepath);
+ CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates);
~CBlockPolicyEstimator();
/** Process all the transactions that have been included in a block */
@@ -239,6 +252,13 @@ public:
void Flush()
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
+ /** Record current fee estimations. */
+ void FlushFeeEstimates()
+ EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
+
+ /** Calculates the age of the file, since last modified */
+ std::chrono::hours GetFeeEstimatorFileAge();
+
private:
mutable Mutex m_cs_fee_estimator;
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index d26ef52eb4..a8452e815f 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -238,7 +238,6 @@ BitcoinGUI::~BitcoinGUI()
trayIcon->hide();
#ifdef Q_OS_MACOS
delete m_app_nap_inhibitor;
- delete appMenuBar;
MacDockIconHandler::cleanup();
#endif
@@ -466,13 +465,7 @@ void BitcoinGUI::createActions()
void BitcoinGUI::createMenuBar()
{
-#ifdef Q_OS_MACOS
- // Create a decoupled menu bar on Mac which stays even if the window is closed
- appMenuBar = new QMenuBar();
-#else
- // Get the main window's menu bar on other platforms
appMenuBar = menuBar();
-#endif
// Configure the menus
QMenu *file = appMenuBar->addMenu(tr("&File"));
@@ -860,6 +853,7 @@ void BitcoinGUI::createTrayIconMenu()
// Note: On macOS, the Dock icon is used to provide the tray's functionality.
MacDockIconHandler* dockIconHandler = MacDockIconHandler::instance();
connect(dockIconHandler, &MacDockIconHandler::dockIconClicked, [this] {
+ if (m_node.shutdownRequested()) return; // nothing to show, node is shutting down.
show();
activateWindow();
});
@@ -871,6 +865,8 @@ void BitcoinGUI::createTrayIconMenu()
// See https://bugreports.qt.io/browse/QTBUG-91697
trayIconMenu.get(), &QMenu::aboutToShow,
[this, show_hide_action, send_action, receive_action, sign_action, verify_action, options_action, node_window_action, quit_action] {
+ if (m_node.shutdownRequested()) return; // nothing to do, node is shutting down.
+
if (show_hide_action) show_hide_action->setText(
(!isHidden() && !isMinimized() && !GUIUtil::isObscured(this)) ?
tr("&Hide") :
diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp
index 116fbd9015..aa3cfe81df 100644
--- a/src/test/fuzz/policy_estimator.cpp
+++ b/src/test/fuzz/policy_estimator.cpp
@@ -31,7 +31,7 @@ void initialize_policy_estimator()
FUZZ_TARGET_INIT(policy_estimator, initialize_policy_estimator)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
- CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)};
+ CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES};
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
CallOneOf(
fuzzed_data_provider,
diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp
index 7c3289cd26..3df40197d8 100644
--- a/src/test/fuzz/policy_estimator_io.cpp
+++ b/src/test/fuzz/policy_estimator_io.cpp
@@ -28,7 +28,7 @@ FUZZ_TARGET_INIT(policy_estimator_io, initialize_policy_estimator_io)
FuzzedAutoFileProvider fuzzed_auto_file_provider = ConsumeAutoFile(fuzzed_data_provider);
AutoFile fuzzed_auto_file{fuzzed_auto_file_provider.open()};
// Re-using block_policy_estimator across runs to avoid costly creation of CBlockPolicyEstimator object.
- static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)};
+ static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES};
if (block_policy_estimator.Read(fuzzed_auto_file)) {
block_policy_estimator.Write(fuzzed_auto_file);
}
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 58593c9d5b..e9cf177666 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -173,7 +173,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);
- m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(*m_node.args));
+ m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES);
m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node));
m_cache_sizes = CalculateCacheSizes(m_args);
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 812737429d..c7358a78d1 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -1791,4 +1791,29 @@ BOOST_AUTO_TEST_CASE(util_WriteBinaryFile)
BOOST_CHECK(valid);
BOOST_CHECK_EQUAL(actual_text, expected_text);
}
+
+BOOST_AUTO_TEST_CASE(clearshrink_test)
+{
+ {
+ std::vector<uint8_t> v = {1, 2, 3};
+ ClearShrink(v);
+ BOOST_CHECK_EQUAL(v.size(), 0);
+ BOOST_CHECK_EQUAL(v.capacity(), 0);
+ }
+
+ {
+ std::vector<bool> v = {false, true, false, false, true, true};
+ ClearShrink(v);
+ BOOST_CHECK_EQUAL(v.size(), 0);
+ BOOST_CHECK_EQUAL(v.capacity(), 0);
+ }
+
+ {
+ std::deque<int> v = {1, 3, 3, 7};
+ ClearShrink(v);
+ BOOST_CHECK_EQUAL(v.size(), 0);
+ // std::deque has no capacity() we can observe.
+ }
+}
+
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util/vector.h b/src/util/vector.h
index 9b9218e54f..40ff73c293 100644
--- a/src/util/vector.h
+++ b/src/util/vector.h
@@ -49,4 +49,22 @@ inline V Cat(V v1, const V& v2)
return v1;
}
+/** Clear a vector (or std::deque) and release its allocated memory. */
+template<typename V>
+inline void ClearShrink(V& v) noexcept
+{
+ // There are various ways to clear a vector and release its memory:
+ //
+ // 1. V{}.swap(v)
+ // 2. v = V{}
+ // 3. v = {}; v.shrink_to_fit();
+ // 4. v.clear(); v.shrink_to_fit();
+ //
+ // (2) does not appear to release memory in glibc debug mode, even if v.shrink_to_fit()
+ // follows. (3) and (4) rely on std::vector::shrink_to_fit, which is only a non-binding
+ // request. Therefore, we use method (1).
+
+ V{}.swap(v);
+}
+
#endif // BITCOIN_UTIL_VECTOR_H
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 62bd0c06cd..67ed41117e 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -1712,8 +1712,23 @@ std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPub
}
// All watchonly scripts are raw
- spks.insert(setWatchOnly.begin(), setWatchOnly.end());
+ for (const CScript& script : setWatchOnly) {
+ // As the legacy wallet allowed to import any script, we need to verify the validity here.
+ // LegacyScriptPubKeyMan::IsMine() return 'ISMINE_NO' for invalid or not watched scripts (IsMineResult::INVALID or IsMineResult::NO).
+ // e.g. a "sh(sh(pkh()))" which legacy wallets allowed to import!.
+ if (IsMine(script) != ISMINE_NO) spks.insert(script);
+ }
+
+ return spks;
+}
+std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetNotMineScriptPubKeys() const
+{
+ LOCK(cs_KeyStore);
+ std::unordered_set<CScript, SaltedSipHasher> spks;
+ for (const CScript& script : setWatchOnly) {
+ if (IsMine(script) == ISMINE_NO) spks.insert(script);
+ }
return spks;
}
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 22b67c88e9..bde6eb1771 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -517,6 +517,12 @@ public:
std::set<CKeyID> GetKeys() const override;
std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override;
+ /**
+ * Retrieves scripts that were imported by bugs into the legacy spkm and are
+ * simply invalid, such as a sh(sh(pkh())) script, or not watched.
+ */
+ std::unordered_set<CScript, SaltedSipHasher> GetNotMineScriptPubKeys() const;
+
/** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan.
* Does not modify this ScriptPubKeyMan. */
std::optional<MigrationData> MigrateToDescriptor();
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 208b97bf07..07bc742090 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1323,11 +1323,14 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
{
LOCK(cs_wallet);
- int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
// If number of conflict confirms cannot be determined, this means
// that the block is still unknown or not yet part of the main chain,
// for example when loading the wallet during a reindex. Do nothing in that
// case.
+ if (m_last_block_processed_height < 0 || conflicting_height < 0) {
+ return;
+ }
+ int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
if (conflictconfirms >= 0)
return;
@@ -3899,6 +3902,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
return false;
}
+ // Get all invalid or non-watched scripts that will not be migrated
+ std::set<CTxDestination> not_migrated_dests;
+ for (const auto& script : legacy_spkm->GetNotMineScriptPubKeys()) {
+ CTxDestination dest;
+ if (ExtractDestination(script, dest)) not_migrated_dests.emplace(dest);
+ }
+
for (auto& desc_spkm : data.desc_spkms) {
if (m_spk_managers.count(desc_spkm->GetID()) > 0) {
error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted.");
@@ -4004,6 +4014,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
continue;
}
}
+
+ // Skip invalid/non-watched scripts that will not be migrated
+ if (not_migrated_dests.count(addr_pair.first) > 0) {
+ dests_to_delete.push_back(addr_pair.first);
+ continue;
+ }
+
// Not ours, not in watchonly wallet, and not in solvable
error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets");
return false;
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-18 11:17:20 +0000