diff options
60 files changed, 701 insertions, 497 deletions
diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index cdf0020d4d..44c769f463 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -250,7 +250,7 @@ case "$HOST" in esac case "$HOST" in - powerpc64-linux-*|riscv64-linux-*) HOST_LDFLAGS="${HOST_LDFLAGS} -Wl,-z,noexecstack" ;; + powerpc64-linux-*) HOST_LDFLAGS="${HOST_LDFLAGS} -Wl,-z,noexecstack" ;; esac # Make $HOST-specific native binaries from depends available in $PATH diff --git a/doc/developer-notes.md b/doc/developer-notes.md index bd385efdfd..d1b660efff 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -921,14 +921,19 @@ Threads and synchronization - Prefer `Mutex` type to `RecursiveMutex` one. - Consistently use [Clang Thread Safety Analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) annotations to - get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with - run-time asserts in function definitions: + get compile-time warnings about potential race conditions or deadlocks in code. - In functions that are declared separately from where they are defined, the thread safety annotations should be added exclusively to the function declaration. Annotations on the definition could lead to false positives (lack of compile failure) at call sites between the two. + - Prefer locks that are in a class rather than global, and that are + internal to a class (private or protected) rather than public. + + - Combine annotations in function declarations with run-time asserts in + function definitions: + ```C++ // txmempool.h class CTxMemPool @@ -952,21 +957,37 @@ void CTxMemPool::UpdateTransactionsFromBlock(...) ```C++ // validation.h -class ChainstateManager +class CChainState { +protected: + ... + Mutex m_chainstate_mutex; + ... public: ... - bool ProcessNewBlock(...) LOCKS_EXCLUDED(::cs_main); + bool ActivateBestChain( + BlockValidationState& state, + std::shared_ptr<const CBlock> pblock = nullptr) + EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) + LOCKS_EXCLUDED(::cs_main); + ... + bool PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) + EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) + LOCKS_EXCLUDED(::cs_main); ... } // validation.cpp -bool ChainstateManager::ProcessNewBlock(...) +bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) { + AssertLockNotHeld(m_chainstate_mutex); AssertLockNotHeld(::cs_main); - ... - LOCK(::cs_main); - ... + { + LOCK(cs_main); + ... + } + + return ActivateBestChain(state, std::shared_ptr<const CBlock>()); } ``` diff --git a/doc/productivity.md b/doc/productivity.md index a01c6f545d..e9b7bc270c 100644 --- a/doc/productivity.md +++ b/doc/productivity.md @@ -9,6 +9,7 @@ Table of Contents * [Disable features with `./configure`](#disable-features-with-configure) * [Make use of your threads with `make -j`](#make-use-of-your-threads-with-make--j) * [Only build what you need](#only-build-what-you-need) + * [Compile on multiple machines](#compile-on-multiple-machines) * [Multiple working directories with `git worktrees`](#multiple-working-directories-with-git-worktrees) * [Interactive "dummy rebases" for fixups and execs with `git merge-base`](#interactive-dummy-rebases-for-fixups-and-execs-with-git-merge-base) * [Writing code](#writing-code) @@ -81,6 +82,10 @@ make -C src bitcoin_bench (You can and should combine this with `-j`, as above, for a parallel build.) +### Compile on multiple machines + +If you have more than one computer at your disposal, you can use [distcc](https://www.distcc.org) to speed up compilation. This is easiest when all computers run the same operating system and compiler version. + ### Multiple working directories with `git worktrees` If you work with multiple branches or multiple copies of the repository, you should try `git worktrees`. diff --git a/doc/release-notes-15936.md b/doc/release-notes-15936.md new file mode 100644 index 0000000000..90c0413b9a --- /dev/null +++ b/doc/release-notes-15936.md @@ -0,0 +1,15 @@ +GUI changes +----------- + +Configuration changes made in the bitcoin GUI (such as the pruning setting, +proxy settings, UPNP preferences) are now saved to `<datadir>/settings.json` +file rather than to the Qt settings backend (windows registry or unix desktop +config files), so these settings will now apply to bitcoind, instead of being +ignored. + +Also, the interaction between GUI settings and `bitcoin.conf` settings is +simplified. Settings from `bitcoin.conf` are now displayed normally in the GUI +settings dialog, instead of in a separate warning message ("Options set in this +dialog are overridden by the configuration file: -setting=value"). And these +settings can now be edited because `settings.json` values take precedence over +`bitcoin.conf` values. diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index b9e5a81f8d..0db2b75384 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -9,6 +9,7 @@ #include <chainparamsbase.h> #include <clientversion.h> +#include <compat.h> #include <compat/stdin.h> #include <policy/feerate.h> #include <rpc/client.h> @@ -1212,19 +1213,11 @@ static int CommandLineRPC(int argc, char *argv[]) return nRet; } -#ifdef WIN32 -// Export main() and ensure working ASLR on Windows. -// Exporting a symbol will prevent the linker from stripping -// the .reloc section from the binary, which is a requirement -// for ASLR. This is a temporary workaround until a fixed -// version of binutils is used for releases. -__declspec(dllexport) int main(int argc, char* argv[]) +MAIN_FUNCTION { +#ifdef WIN32 util::WinCmdLineArgs winArgs; std::tie(argc, argv) = winArgs.get(); -#else -int main(int argc, char* argv[]) -{ #endif SetupEnvironment(); if (!SetupNetworking()) { diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index bcefb3f18e..e0d5c6e5dc 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -8,6 +8,7 @@ #include <clientversion.h> #include <coins.h> +#include <compat.h> #include <consensus/amount.h> #include <consensus/consensus.h> #include <core_io.h> @@ -854,7 +855,7 @@ static int CommandLineRawTx(int argc, char* argv[]) return nRet; } -int main(int argc, char* argv[]) +MAIN_FUNCTION { SetupEnvironment(); diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index 1aeac3cef0..1739804edb 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -11,6 +11,7 @@ #include <chainparams.h> #include <chainparamsbase.h> #include <clientversion.h> +#include <compat.h> #include <core_io.h> #include <streams.h> #include <util/system.h> @@ -142,16 +143,7 @@ static int Grind(const std::vector<std::string>& args, std::string& strPrint) return EXIT_SUCCESS; } -#ifdef WIN32 -// Export main() and ensure working ASLR on Windows. -// Exporting a symbol will prevent the linker from stripping -// the .reloc section from the binary, which is a requirement -// for ASLR. This is a temporary workaround until a fixed -// version of binutils is used for releases. -__declspec(dllexport) int main(int argc, char* argv[]) -#else -int main(int argc, char* argv[]) -#endif +MAIN_FUNCTION { ArgsManager& args = gArgs; SetupEnvironment(); diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 2f3dd45267..7bec3292a1 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -9,6 +9,7 @@ #include <chainparams.h> #include <chainparamsbase.h> #include <clientversion.h> +#include <compat.h> #include <interfaces/init.h> #include <key.h> #include <logging.h> @@ -88,7 +89,7 @@ static bool WalletAppInit(ArgsManager& args, int argc, char* argv[]) return true; } -int main(int argc, char* argv[]) +MAIN_FUNCTION { ArgsManager& args = gArgs; #ifdef WIN32 diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 92e73d7c2a..227bc47793 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -256,7 +256,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) return fRet; } -int main(int argc, char* argv[]) +MAIN_FUNCTION { #ifdef WIN32 util::WinCmdLineArgs winArgs; diff --git a/src/compat.h b/src/compat.h index 3ec4ab53fd..f41c501c84 100644 --- a/src/compat.h +++ b/src/compat.h @@ -86,6 +86,17 @@ typedef void* sockopt_arg_type; typedef char* sockopt_arg_type; #endif +#ifdef WIN32 +// Export main() and ensure working ASLR when using mingw-w64. +// Exporting a symbol will prevent the linker from stripping +// the .reloc section from the binary, which is a requirement +// for ASLR. While release builds are not affected, anyone +// building with a binutils < 2.36 is subject to this ld bug. +#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[]) +#else +#define MAIN_FUNCTION int main(int argc, char* argv[]) +#endif + // Note these both should work with the current usage of poll, but best to be safe // WIN32 poll is broken https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/ // __APPLE__ poll is broke https://github.com/bitcoin/bitcoin/pull/14336#issuecomment-437384408 diff --git a/src/init.cpp b/src/init.cpp index d0fd6074b1..f6e6f7cf43 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -599,7 +599,7 @@ void SetupServerArgs(ArgsManager& argsman) } static bool fHaveGenesis = false; -static Mutex g_genesis_wait_mutex; +static GlobalMutex g_genesis_wait_mutex; static std::condition_variable g_genesis_wait_cv; static void BlockNotifyGenesisWait(const CBlockIndex* pBlockIndex) diff --git a/src/net.cpp b/src/net.cpp index 82b5a69eb5..0cdf98c40f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -114,7 +114,7 @@ static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256 // bool fDiscover = true; bool fListen = true; -Mutex g_maplocalhost_mutex; +GlobalMutex g_maplocalhost_mutex; std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(g_maplocalhost_mutex); static bool vfLimited[NET_MAX] GUARDED_BY(g_maplocalhost_mutex) = {}; std::string strSubVersion; @@ -248,7 +248,7 @@ struct LocalServiceInfo { uint16_t nPort; }; -extern Mutex g_maplocalhost_mutex; +extern GlobalMutex g_maplocalhost_mutex; extern std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(g_maplocalhost_mutex); extern const std::string NET_MESSAGE_TYPE_OTHER; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 30d5548385..1d69975f55 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -292,7 +292,7 @@ struct Peer { return m_tx_relay.get(); }; - TxRelay* GetTxRelay() + TxRelay* GetTxRelay() EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) { return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); }; diff --git a/src/netbase.cpp b/src/netbase.cpp index 8ff3b7a68c..030f462ed9 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -30,7 +30,7 @@ #endif // Settings -static Mutex g_proxyinfo_mutex; +static GlobalMutex g_proxyinfo_mutex; static Proxy proxyInfo[NET_MAX] GUARDED_BY(g_proxyinfo_mutex); static Proxy nameProxy GUARDED_BY(g_proxyinfo_mutex); int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 53fbac0106..bd6a1f5eca 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -259,9 +259,26 @@ void BitcoinApplication::createPaymentServer() } #endif -void BitcoinApplication::createOptionsModel(bool resetSettings) +bool BitcoinApplication::createOptionsModel(bool resetSettings) { - optionsModel = new OptionsModel(node(), this, resetSettings); + optionsModel = new OptionsModel(node(), this); + if (resetSettings) { + optionsModel->Reset(); + } + bilingual_str error; + if (!optionsModel->Init(error)) { + fs::path settings_path; + if (gArgs.GetSettingsPath(&settings_path)) { + error += Untranslated("\n"); + std::string quoted_path = strprintf("%s", fs::quoted(fs::PathToString(settings_path))); + error.original += strprintf("Settings file %s might be corrupt or invalid.", quoted_path); + error.translated += tr("Settings file %1 might be corrupt or invalid.").arg(QString::fromStdString(quoted_path)).toStdString(); + } + InitError(error); + QMessageBox::critical(nullptr, PACKAGE_NAME, QString::fromStdString(error.translated)); + return false; + } + return true; } void BitcoinApplication::createWindow(const NetworkStyle *networkStyle) @@ -327,7 +344,7 @@ void BitcoinApplication::parameterSetup() void BitcoinApplication::InitPruneSetting(int64_t prune_MiB) { - optionsModel->SetPruneTargetGB(PruneMiBtoGB(prune_MiB), true); + optionsModel->SetPruneTargetGB(PruneMiBtoGB(prune_MiB)); } void BitcoinApplication::requestInitialize() @@ -641,7 +658,9 @@ int GuiMain(int argc, char* argv[]) app.createNode(*init); // Load GUI settings from QSettings - app.createOptionsModel(gArgs.GetBoolArg("-resetguisettings", false)); + if (!app.createOptionsModel(gArgs.GetBoolArg("-resetguisettings", false))) { + return EXIT_FAILURE; + } if (did_show_intro) { // Store intro dialog settings other than datadir (network specific) diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 7a6aa5cdc9..9ad37ca6c9 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -47,7 +47,7 @@ public: /// parameter interaction/setup based on rules void parameterSetup(); /// Create options model - void createOptionsModel(bool resetSettings); + [[nodiscard]] bool createOptionsModel(bool resetSettings); /// Initialize prune setting void InitPruneSetting(int64_t prune_MiB); /// Create main window diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index b10ffb4523..81f03a58ec 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -105,7 +105,7 @@ private: //! A thread to interact with m_node asynchronously QThread* const m_thread; - void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header); + void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header) EXCLUSIVE_LOCKS_REQUIRED(!m_cached_tip_mutex); void subscribeToCoreSignals(); void unsubscribeFromCoreSignals(); diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 5438811aff..582f02132a 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -896,7 +896,7 @@ <item> <widget class="QLabel" name="overriddenByCommandLineInfoLabel"> <property name="text"> - <string>Options set in this dialog are overridden by the command line or in the configuration file:</string> + <string>Options set in this dialog are overridden by the command line:</string> </property> <property name="textFormat"> <enum>Qt::PlainText</enum> diff --git a/src/qt/main.cpp b/src/qt/main.cpp index 6e772d58c5..38b0ac71a3 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,6 +4,7 @@ #include <qt/bitcoin.h> +#include <compat.h> #include <util/translation.h> #include <util/url.h> @@ -18,4 +19,7 @@ extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = [](cons }; UrlDecodeFn* const URL_DECODE = urlDecode; -int main(int argc, char* argv[]) { return GuiMain(argc, argv); } +MAIN_FUNCTION +{ + return GuiMain(argc, argv); +} diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index e6ff43a379..f3c3af10e0 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -26,7 +26,6 @@ #include <QIntValidator> #include <QLocale> #include <QMessageBox> -#include <QSettings> #include <QSystemTrayIcon> #include <QTimer> @@ -56,10 +55,6 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) : #ifndef USE_NATPMP ui->mapPortNatpmp->setEnabled(false); #endif - connect(this, &QDialog::accepted, [this](){ - QSettings settings; - model->node().mapPort(settings.value("fUseUPnP").toBool(), settings.value("fUseNatpmp").toBool()); - }); ui->proxyIp->setEnabled(false); ui->proxyPort->setEnabled(false); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 612d3009c1..0b4359a917 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -19,6 +19,7 @@ #include <txdb.h> // for -dbcache defaults #include <util/string.h> #include <validation.h> // For DEFAULT_SCRIPTCHECK_THREADS +#include <wallet/wallet.h> // For DEFAULT_SPEND_ZEROCONF_CHANGE #include <QDebug> #include <QLatin1Char> @@ -26,14 +27,99 @@ #include <QStringList> #include <QVariant> +#include <univalue.h> + const char *DEFAULT_GUI_PROXY_HOST = "127.0.0.1"; static const QString GetDefaultProxyAddress(); -OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent, bool resetSettings) : +/** Map GUI option ID to node setting name. */ +static const char* SettingName(OptionsModel::OptionID option) +{ + switch (option) { + case OptionsModel::DatabaseCache: return "dbcache"; + case OptionsModel::ThreadsScriptVerif: return "par"; + case OptionsModel::SpendZeroConfChange: return "spendzeroconfchange"; + case OptionsModel::ExternalSignerPath: return "signer"; + case OptionsModel::MapPortUPnP: return "upnp"; + case OptionsModel::MapPortNatpmp: return "natpmp"; + case OptionsModel::Listen: return "listen"; + case OptionsModel::Server: return "server"; + case OptionsModel::PruneSize: return "prune"; + case OptionsModel::Prune: return "prune"; + case OptionsModel::ProxyIP: return "proxy"; + case OptionsModel::ProxyPort: return "proxy"; + case OptionsModel::ProxyUse: return "proxy"; + case OptionsModel::ProxyIPTor: return "onion"; + case OptionsModel::ProxyPortTor: return "onion"; + case OptionsModel::ProxyUseTor: return "onion"; + case OptionsModel::Language: return "lang"; + default: throw std::logic_error(strprintf("GUI option %i has no corresponding node setting.", option)); + } +} + +/** Call node.updateRwSetting() with Bitcoin 22.x workaround. */ +static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const util::SettingsValue& value) +{ + if (value.isNum() && + (option == OptionsModel::DatabaseCache || + option == OptionsModel::ThreadsScriptVerif || + option == OptionsModel::Prune || + option == OptionsModel::PruneSize)) { + // Write certain old settings as strings, even though they are numbers, + // because Bitcoin 22.x releases try to read these specific settings as + // strings in addOverriddenOption() calls at startup, triggering + // uncaught exceptions in UniValue::get_str(). These errors were fixed + // in later releases by https://github.com/bitcoin/bitcoin/pull/24498. + // If new numeric settings are added, they can be written as numbers + // instead of strings, because bitcoin 22.x will not try to read these. + node.updateRwSetting(SettingName(option), value.getValStr()); + } else { + node.updateRwSetting(SettingName(option), value); + } +} + +//! Convert enabled/size values to bitcoin -prune setting. +static util::SettingsValue PruneSetting(bool prune_enabled, int prune_size_gb) +{ + assert(!prune_enabled || prune_size_gb >= 1); // PruneSizeGB and ParsePruneSizeGB never return less + return prune_enabled ? PruneGBtoMiB(prune_size_gb) : 0; +} + +//! Get pruning enabled value to show in GUI from bitcoin -prune setting. +static bool PruneEnabled(const util::SettingsValue& prune_setting) +{ + // -prune=1 setting is manual pruning mode, so disabled for purposes of the gui + return SettingToInt(prune_setting, 0) > 1; +} + +//! Get pruning size value to show in GUI from bitcoin -prune setting. If +//! pruning is not enabled, just show default recommended pruning size (2GB). +static int PruneSizeGB(const util::SettingsValue& prune_setting) +{ + int value = SettingToInt(prune_setting, 0); + return value > 1 ? PruneMiBtoGB(value) : DEFAULT_PRUNE_TARGET_GB; +} + +//! Parse pruning size value provided by user in GUI or loaded from QSettings +//! (windows registry key or qt .conf file). Smallest value that the GUI can +//! display is 1 GB, so round up if anything less is parsed. +static int ParsePruneSizeGB(const QVariant& prune_size) +{ + return std::max(1, prune_size.toInt()); +} + +struct ProxySetting { + bool is_set; + QString ip; + QString port; +}; +static ProxySetting ParseProxyString(const std::string& proxy); +static std::string ProxyString(bool is_set, QString ip, QString port); + +OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent) : QAbstractListModel(parent), m_node{node} { - Init(resetSettings); } void OptionsModel::addOverriddenOption(const std::string &option) @@ -42,10 +128,17 @@ void OptionsModel::addOverriddenOption(const std::string &option) } // Writes all missing QSettings with their default values -void OptionsModel::Init(bool resetSettings) +bool OptionsModel::Init(bilingual_str& error) { - if (resetSettings) - Reset(); + // Initialize display settings from stored settings. + m_prune_size_gb = PruneSizeGB(node().getPersistentSetting("prune")); + ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString())); + m_proxy_ip = proxy.ip; + m_proxy_port = proxy.port; + ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString())); + m_onion_ip = onion.ip; + m_onion_port = onion.port; + language = QString::fromStdString(SettingToString(node().getPersistentSetting("lang"), "")); checkAndMigrate(); @@ -98,130 +191,43 @@ void OptionsModel::Init(bool resetSettings) // These are shared with the core or have a command-line parameter // and we want command-line parameters to overwrite the GUI settings. - // + for (OptionID option : {DatabaseCache, ThreadsScriptVerif, SpendZeroConfChange, ExternalSignerPath, MapPortUPnP, + MapPortNatpmp, Listen, Server, Prune, ProxyUse, ProxyUseTor, Language}) { + std::string setting = SettingName(option); + if (node().isSettingIgnored(setting)) addOverriddenOption("-" + setting); + try { + getOption(option); + } catch (const std::exception& e) { + // This handles exceptions thrown by univalue that can happen if + // settings in settings.json don't have the expected types. + error.original = strprintf("Could not read setting \"%s\", %s.", setting, e.what()); + error.translated = tr("Could not read setting \"%1\", %2.").arg(QString::fromStdString(setting), e.what()).toStdString(); + return false; + } + } + // If setting doesn't exist create it with defaults. - // - // If gArgs.SoftSetArg() or gArgs.SoftSetBoolArg() return false we were overridden - // by command-line and show this in the UI. // Main - if (!settings.contains("bPrune")) - settings.setValue("bPrune", false); - if (!settings.contains("nPruneSize")) - settings.setValue("nPruneSize", DEFAULT_PRUNE_TARGET_GB); - SetPruneEnabled(settings.value("bPrune").toBool()); - - if (!settings.contains("nDatabaseCache")) - settings.setValue("nDatabaseCache", (qint64)nDefaultDbCache); - if (!gArgs.SoftSetArg("-dbcache", settings.value("nDatabaseCache").toString().toStdString())) - addOverriddenOption("-dbcache"); - - if (!settings.contains("nThreadsScriptVerif")) - settings.setValue("nThreadsScriptVerif", DEFAULT_SCRIPTCHECK_THREADS); - if (!gArgs.SoftSetArg("-par", settings.value("nThreadsScriptVerif").toString().toStdString())) - addOverriddenOption("-par"); - if (!settings.contains("strDataDir")) settings.setValue("strDataDir", GUIUtil::getDefaultDataDirectory()); // Wallet #ifdef ENABLE_WALLET - if (!settings.contains("bSpendZeroConfChange")) - settings.setValue("bSpendZeroConfChange", true); - if (!gArgs.SoftSetBoolArg("-spendzeroconfchange", settings.value("bSpendZeroConfChange").toBool())) - addOverriddenOption("-spendzeroconfchange"); - - if (!settings.contains("external_signer_path")) - settings.setValue("external_signer_path", ""); - - if (!gArgs.SoftSetArg("-signer", settings.value("external_signer_path").toString().toStdString())) { - addOverriddenOption("-signer"); - } - if (!settings.contains("SubFeeFromAmount")) { settings.setValue("SubFeeFromAmount", false); } m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool(); #endif - // Network - if (!settings.contains("fUseUPnP")) - settings.setValue("fUseUPnP", DEFAULT_UPNP); - if (!gArgs.SoftSetBoolArg("-upnp", settings.value("fUseUPnP").toBool())) - addOverriddenOption("-upnp"); - - if (!settings.contains("fUseNatpmp")) { - settings.setValue("fUseNatpmp", DEFAULT_NATPMP); - } - if (!gArgs.SoftSetBoolArg("-natpmp", settings.value("fUseNatpmp").toBool())) { - addOverriddenOption("-natpmp"); - } - - if (!settings.contains("fListen")) - settings.setValue("fListen", DEFAULT_LISTEN); - const bool listen{settings.value("fListen").toBool()}; - if (!gArgs.SoftSetBoolArg("-listen", listen)) { - addOverriddenOption("-listen"); - } else if (!listen) { - // We successfully set -listen=0, thus mimic the logic from InitParameterInteraction(): - // "parameter interaction: -listen=0 -> setting -listenonion=0". - // - // Both -listen and -listenonion default to true. - // - // The call order is: - // - // InitParameterInteraction() - // would set -listenonion=0 if it sees -listen=0, but for bitcoin-qt with - // fListen=false -listen is 1 at this point - // - // OptionsModel::Init() - // (this method) can flip -listen from 1 to 0 if fListen=false - // - // AppInitParameterInteraction() - // raises an error if -listen=0 and -listenonion=1 - gArgs.SoftSetBoolArg("-listenonion", false); - } - - if (!settings.contains("server")) { - settings.setValue("server", false); - } - if (!gArgs.SoftSetBoolArg("-server", settings.value("server").toBool())) { - addOverriddenOption("-server"); - } - - if (!settings.contains("fUseProxy")) - settings.setValue("fUseProxy", false); - if (!settings.contains("addrProxy")) - settings.setValue("addrProxy", GetDefaultProxyAddress()); - // Only try to set -proxy, if user has enabled fUseProxy - if ((settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString()))) - addOverriddenOption("-proxy"); - else if(!settings.value("fUseProxy").toBool() && !gArgs.GetArg("-proxy", "").empty()) - addOverriddenOption("-proxy"); - - if (!settings.contains("fUseSeparateProxyTor")) - settings.setValue("fUseSeparateProxyTor", false); - if (!settings.contains("addrSeparateProxyTor")) - settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress()); - // Only try to set -onion, if user has enabled fUseSeparateProxyTor - if ((settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString()))) - addOverriddenOption("-onion"); - else if(!settings.value("fUseSeparateProxyTor").toBool() && !gArgs.GetArg("-onion", "").empty()) - addOverriddenOption("-onion"); - // Display - if (!settings.contains("language")) - settings.setValue("language", ""); - if (!gArgs.SoftSetArg("-lang", settings.value("language").toString().toStdString())) - addOverriddenOption("-lang"); - - language = settings.value("language").toString(); - if (!settings.contains("UseEmbeddedMonospacedFont")) { settings.setValue("UseEmbeddedMonospacedFont", "true"); } m_use_embedded_monospaced_font = settings.value("UseEmbeddedMonospacedFont").toBool(); Q_EMIT useEmbeddedMonospacedFontChanged(m_use_embedded_monospaced_font); + + return true; } /** Helper function to copy contents from one QSettings to another. @@ -245,6 +251,9 @@ static void BackupSettings(const fs::path& filename, const QSettings& src) void OptionsModel::Reset() { + // Backup and reset settings.json + node().resetSettings(); + QSettings settings; // Backup old settings to chain-specific datadir for troubleshooting @@ -273,21 +282,15 @@ int OptionsModel::rowCount(const QModelIndex & parent) const return OptionIDRowCount; } -struct ProxySetting { - bool is_set; - QString ip; - QString port; -}; - -static ProxySetting GetProxySetting(QSettings &settings, const QString &name) +static ProxySetting ParseProxyString(const QString& proxy) { static const ProxySetting default_val = {false, DEFAULT_GUI_PROXY_HOST, QString("%1").arg(DEFAULT_GUI_PROXY_PORT)}; // Handle the case that the setting is not set at all - if (!settings.contains(name)) { + if (proxy.isEmpty()) { return default_val; } // contains IP at index 0 and port at index 1 - QStringList ip_port = GUIUtil::SplitSkipEmptyParts(settings.value(name).toString(), ":"); + QStringList ip_port = GUIUtil::SplitSkipEmptyParts(proxy, ":"); if (ip_port.size() == 2) { return {true, ip_port.at(0), ip_port.at(1)}; } else { // Invalid: return default @@ -295,39 +298,42 @@ static ProxySetting GetProxySetting(QSettings &settings, const QString &name) } } -static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port) +static ProxySetting ParseProxyString(const std::string& proxy) { - settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port}); + return ParseProxyString(QString::fromStdString(proxy)); } -static const QString GetDefaultProxyAddress() +static std::string ProxyString(bool is_set, QString ip, QString port) { - return QString("%1:%2").arg(DEFAULT_GUI_PROXY_HOST).arg(DEFAULT_GUI_PROXY_PORT); + return is_set ? QString(ip + ":" + port).toStdString() : ""; } -void OptionsModel::SetPruneEnabled(bool prune, bool force) +static const QString GetDefaultProxyAddress() { - QSettings settings; - settings.setValue("bPrune", prune); - const int64_t prune_target_mib = PruneGBtoMiB(settings.value("nPruneSize").toInt()); - std::string prune_val = prune ? ToString(prune_target_mib) : "0"; - if (force) { - gArgs.ForceSetArg("-prune", prune_val); - return; - } - if (!gArgs.SoftSetArg("-prune", prune_val)) { - addOverriddenOption("-prune"); - } + return QString("%1:%2").arg(DEFAULT_GUI_PROXY_HOST).arg(DEFAULT_GUI_PROXY_PORT); } -void OptionsModel::SetPruneTargetGB(int prune_target_gb, bool force) +void OptionsModel::SetPruneTargetGB(int prune_target_gb) { - const bool prune = prune_target_gb > 0; - if (prune) { - QSettings settings; - settings.setValue("nPruneSize", prune_target_gb); + const util::SettingsValue cur_value = node().getPersistentSetting("prune"); + const util::SettingsValue new_value = PruneSetting(prune_target_gb > 0, prune_target_gb); + + m_prune_size_gb = prune_target_gb; + + // Force setting to take effect. It is still safe to change the value at + // this point because this function is only called after the intro screen is + // shown, before the node starts. + node().forceSetting("prune", new_value); + + // Update settings.json if value configured in intro screen is different + // from saved value. Avoid writing settings.json if bitcoin.conf value + // doesn't need to be overridden. + if (PruneEnabled(cur_value) != PruneEnabled(new_value) || + PruneSizeGB(cur_value) != PruneSizeGB(new_value)) { + // Call UpdateRwSetting() instead of setOption() to avoid setting + // RestartRequired flag + UpdateRwSetting(node(), Prune, new_value); } - SetPruneEnabled(prune, force); } // read QSettings values and return them @@ -356,6 +362,8 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in QVariant OptionsModel::getOption(OptionID option) const { + auto setting = [&]{ return node().getPersistentSetting(SettingName(option)); }; + QSettings settings; switch (option) { case StartAtStartup: @@ -366,13 +374,13 @@ QVariant OptionsModel::getOption(OptionID option) const return fMinimizeToTray; case MapPortUPnP: #ifdef USE_UPNP - return settings.value("fUseUPnP"); + return SettingToBool(setting(), DEFAULT_UPNP); #else return false; #endif // USE_UPNP case MapPortNatpmp: #ifdef USE_NATPMP - return settings.value("fUseNatpmp"); + return SettingToBool(setting(), DEFAULT_NATPMP); #else return false; #endif // USE_NATPMP @@ -381,25 +389,25 @@ QVariant OptionsModel::getOption(OptionID option) const // default proxy case ProxyUse: - return settings.value("fUseProxy", false); + return ParseProxyString(SettingToString(setting(), "")).is_set; case ProxyIP: - return GetProxySetting(settings, "addrProxy").ip; + return m_proxy_ip; case ProxyPort: - return GetProxySetting(settings, "addrProxy").port; + return m_proxy_port; // separate Tor proxy case ProxyUseTor: - return settings.value("fUseSeparateProxyTor", false); + return ParseProxyString(SettingToString(setting(), "")).is_set; case ProxyIPTor: - return GetProxySetting(settings, "addrSeparateProxyTor").ip; + return m_onion_ip; case ProxyPortTor: - return GetProxySetting(settings, "addrSeparateProxyTor").port; + return m_onion_port; #ifdef ENABLE_WALLET case SpendZeroConfChange: - return settings.value("bSpendZeroConfChange"); + return SettingToBool(setting(), wallet::DEFAULT_SPEND_ZEROCONF_CHANGE); case ExternalSignerPath: - return settings.value("external_signer_path"); + return QString::fromStdString(SettingToString(setting(), "")); case SubFeeFromAmount: return m_sub_fee_from_amount; #endif @@ -408,7 +416,7 @@ QVariant OptionsModel::getOption(OptionID option) const case ThirdPartyTxUrls: return strThirdPartyTxUrls; case Language: - return settings.value("language"); + return QString::fromStdString(SettingToString(setting(), "")); case UseEmbeddedMonospacedFont: return m_use_embedded_monospaced_font; case CoinControlFeatures: @@ -416,17 +424,17 @@ QVariant OptionsModel::getOption(OptionID option) const case EnablePSBTControls: return settings.value("enable_psbt_controls"); case Prune: - return settings.value("bPrune"); + return PruneEnabled(setting()); case PruneSize: - return settings.value("nPruneSize"); + return m_prune_size_gb; case DatabaseCache: - return settings.value("nDatabaseCache"); + return qlonglong(SettingToInt(setting(), nDefaultDbCache)); case ThreadsScriptVerif: - return settings.value("nThreadsScriptVerif"); + return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS)); case Listen: - return settings.value("fListen"); + return SettingToBool(setting(), DEFAULT_LISTEN); case Server: - return settings.value("server"); + return SettingToBool(setting(), false); default: return QVariant(); } @@ -434,6 +442,9 @@ QVariant OptionsModel::getOption(OptionID option) const bool OptionsModel::setOption(OptionID option, const QVariant& value) { + auto changed = [&] { return value.isValid() && value != getOption(option); }; + auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, value); }; + bool successful = true; /* set to false on parse error */ QSettings settings; @@ -451,10 +462,16 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) settings.setValue("fMinimizeToTray", fMinimizeToTray); break; case MapPortUPnP: // core option - can be changed on-the-fly - settings.setValue("fUseUPnP", value.toBool()); + if (changed()) { + update(value.toBool()); + node().mapPort(value.toBool(), getOption(MapPortNatpmp).toBool()); + } break; case MapPortNatpmp: // core option - can be changed on-the-fly - settings.setValue("fUseNatpmp", value.toBool()); + if (changed()) { + update(value.toBool()); + node().mapPort(getOption(MapPortUPnP).toBool(), value.toBool()); + } break; case MinimizeOnClose: fMinimizeOnClose = value.toBool(); @@ -463,66 +480,66 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) // default proxy case ProxyUse: - if (settings.value("fUseProxy") != value) { - settings.setValue("fUseProxy", value.toBool()); + if (changed()) { + update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port)); setRestartRequired(true); } break; - case ProxyIP: { - auto ip_port = GetProxySetting(settings, "addrProxy"); - if (!ip_port.is_set || ip_port.ip != value.toString()) { - ip_port.ip = value.toString(); - SetProxySetting(settings, "addrProxy", ip_port); - setRestartRequired(true); + case ProxyIP: + if (changed()) { + m_proxy_ip = value.toString(); + if (getOption(ProxyUse).toBool()) { + update(ProxyString(true, m_proxy_ip, m_proxy_port)); + setRestartRequired(true); + } } - } - break; - case ProxyPort: { - auto ip_port = GetProxySetting(settings, "addrProxy"); - if (!ip_port.is_set || ip_port.port != value.toString()) { - ip_port.port = value.toString(); - SetProxySetting(settings, "addrProxy", ip_port); - setRestartRequired(true); + break; + case ProxyPort: + if (changed()) { + m_proxy_port = value.toString(); + if (getOption(ProxyUse).toBool()) { + update(ProxyString(true, m_proxy_ip, m_proxy_port)); + setRestartRequired(true); + } } - } - break; + break; // separate Tor proxy case ProxyUseTor: - if (settings.value("fUseSeparateProxyTor") != value) { - settings.setValue("fUseSeparateProxyTor", value.toBool()); + if (changed()) { + update(ProxyString(value.toBool(), m_onion_ip, m_onion_port)); setRestartRequired(true); } break; - case ProxyIPTor: { - auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor"); - if (!ip_port.is_set || ip_port.ip != value.toString()) { - ip_port.ip = value.toString(); - SetProxySetting(settings, "addrSeparateProxyTor", ip_port); - setRestartRequired(true); + case ProxyIPTor: + if (changed()) { + m_onion_ip = value.toString(); + if (getOption(ProxyUseTor).toBool()) { + update(ProxyString(true, m_onion_ip, m_onion_port)); + setRestartRequired(true); + } } - } - break; - case ProxyPortTor: { - auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor"); - if (!ip_port.is_set || ip_port.port != value.toString()) { - ip_port.port = value.toString(); - SetProxySetting(settings, "addrSeparateProxyTor", ip_port); - setRestartRequired(true); + break; + case ProxyPortTor: + if (changed()) { + m_onion_port = value.toString(); + if (getOption(ProxyUseTor).toBool()) { + update(ProxyString(true, m_onion_ip, m_onion_port)); + setRestartRequired(true); + } } - } - break; + break; #ifdef ENABLE_WALLET case SpendZeroConfChange: - if (settings.value("bSpendZeroConfChange") != value) { - settings.setValue("bSpendZeroConfChange", value); + if (changed()) { + update(value.toBool()); setRestartRequired(true); } break; case ExternalSignerPath: - if (settings.value("external_signer_path") != value.toString()) { - settings.setValue("external_signer_path", value.toString()); + if (changed()) { + update(value.toString().toStdString()); setRestartRequired(true); } break; @@ -542,8 +559,8 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) } break; case Language: - if (settings.value("language") != value) { - settings.setValue("language", value); + if (changed()) { + update(value.toString().toStdString()); setRestartRequired(true); } break; @@ -562,38 +579,36 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value) settings.setValue("enable_psbt_controls", m_enable_psbt_controls); break; case Prune: - if (settings.value("bPrune") != value) { - settings.setValue("bPrune", value); + if (changed()) { + update(PruneSetting(value.toBool(), m_prune_size_gb)); setRestartRequired(true); } break; case PruneSize: - if (settings.value("nPruneSize") != value) { - settings.setValue("nPruneSize", value); - setRestartRequired(true); + if (changed()) { + m_prune_size_gb = ParsePruneSizeGB(value); + if (getOption(Prune).toBool()) { + update(PruneSetting(true, m_prune_size_gb)); + setRestartRequired(true); + } } break; case DatabaseCache: - if (settings.value("nDatabaseCache") != value) { - settings.setValue("nDatabaseCache", value); + if (changed()) { + update(static_cast<int64_t>(value.toLongLong())); setRestartRequired(true); } break; case ThreadsScriptVerif: - if (settings.value("nThreadsScriptVerif") != value) { - settings.setValue("nThreadsScriptVerif", value); + if (changed()) { + update(static_cast<int64_t>(value.toLongLong())); setRestartRequired(true); } break; case Listen: - if (settings.value("fListen") != value) { - settings.setValue("fListen", value); - setRestartRequired(true); - } - break; case Server: - if (settings.value("server") != value) { - settings.setValue("server", value); + if (changed()) { + update(value.toBool()); setRestartRequired(true); } break; @@ -654,4 +669,49 @@ void OptionsModel::checkAndMigrate() if (settings.contains("addrSeparateProxyTor") && settings.value("addrSeparateProxyTor").toString().endsWith("%2")) { settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress()); } + + // Migrate and delete legacy GUI settings that have now moved to <datadir>/settings.json. + auto migrate_setting = [&](OptionID option, const QString& qt_name) { + if (!settings.contains(qt_name)) return; + QVariant value = settings.value(qt_name); + if (node().getPersistentSetting(SettingName(option)).isNull()) { + if (option == ProxyIP) { + ProxySetting parsed = ParseProxyString(value.toString()); + setOption(ProxyIP, parsed.ip); + setOption(ProxyPort, parsed.port); + } else if (option == ProxyIPTor) { + ProxySetting parsed = ParseProxyString(value.toString()); + setOption(ProxyIPTor, parsed.ip); + setOption(ProxyPortTor, parsed.port); + } else { + setOption(option, value); + } + } + settings.remove(qt_name); + }; + + migrate_setting(DatabaseCache, "nDatabaseCache"); + migrate_setting(ThreadsScriptVerif, "nThreadsScriptVerif"); +#ifdef ENABLE_WALLET + migrate_setting(SpendZeroConfChange, "bSpendZeroConfChange"); + migrate_setting(ExternalSignerPath, "external_signer_path"); +#endif + migrate_setting(MapPortUPnP, "fUseUPnP"); + migrate_setting(MapPortNatpmp, "fUseNatpmp"); + migrate_setting(Listen, "fListen"); + migrate_setting(Server, "server"); + migrate_setting(PruneSize, "nPruneSize"); + migrate_setting(Prune, "bPrune"); + migrate_setting(ProxyIP, "addrProxy"); + migrate_setting(ProxyUse, "fUseProxy"); + migrate_setting(ProxyIPTor, "addrSeparateProxyTor"); + migrate_setting(ProxyUseTor, "fUseSeparateProxyTor"); + migrate_setting(Language, "language"); + + // In case migrating QSettings caused any settings value to change, rerun + // parameter interaction code to update other settings. This is particularly + // important for the -listen setting, which should cause -listenonion, -upnp, + // and other settings to default to false if it was set to false. + // (https://github.com/bitcoin-core/gui/issues/567). + node().initParameterInteraction(); } diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 92f80ecf21..42b89c5029 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -13,6 +13,7 @@ #include <assert.h> +struct bilingual_str; namespace interfaces { class Node; } @@ -41,7 +42,7 @@ class OptionsModel : public QAbstractListModel Q_OBJECT public: - explicit OptionsModel(interfaces::Node& node, QObject *parent = nullptr, bool resetSettings = false); + explicit OptionsModel(interfaces::Node& node, QObject *parent = nullptr); enum OptionID { StartAtStartup, // bool @@ -74,7 +75,7 @@ public: OptionIDRowCount, }; - void Init(bool resetSettings = false); + bool Init(bilingual_str& error); void Reset(); int rowCount(const QModelIndex & parent = QModelIndex()) const override; @@ -98,8 +99,7 @@ public: const QString& getOverriddenByCommandLine() { return strOverriddenByCommandLine; } /* Explicit setters */ - void SetPruneEnabled(bool prune, bool force = false); - void SetPruneTargetGB(int prune_target_gb, bool force = false); + void SetPruneTargetGB(int prune_target_gb); /* Restart flag helper */ void setRestartRequired(bool fRequired); @@ -120,6 +120,16 @@ private: bool fCoinControlFeatures; bool m_sub_fee_from_amount; bool m_enable_psbt_controls; + + //! In-memory settings for display. These are stored persistently by the + //! bitcoin node but it's also nice to store them in memory to prevent them + //! getting cleared when enable/disable toggles are used in the GUI. + int m_prune_size_gb; + QString m_proxy_ip; + QString m_proxy_port; + QString m_onion_ip; + QString m_onion_port; + /* settings that were overridden by command-line */ QString strOverriddenByCommandLine; @@ -128,6 +138,7 @@ private: // Check settings version and upgrade default values if required void checkAndMigrate(); + Q_SIGNALS: void displayUnitChanged(BitcoinUnit unit); void coinControlFeaturesChanged(bool); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index d8daccecbd..61d2782222 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -543,15 +543,8 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) // failed, or more signatures are needed. if (broadcast) { // now send the prepared transaction - WalletModel::SendCoinsReturn sendStatus = model->sendCoins(*m_current_transaction); - // process sendStatus and on error generate message shown to user - processSendCoinsReturn(sendStatus); - - if (sendStatus.status == WalletModel::OK) { - Q_EMIT coinsSent(m_current_transaction->getWtx()->GetHash()); - } else { - send_failure = true; - } + model->sendCoins(*m_current_transaction); + Q_EMIT coinsSent(m_current_transaction->getWtx()->GetHash()); } } if (!send_failure) { diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 3b7a40438b..581735263d 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -128,6 +128,8 @@ void TestAddAddressesToSendBook(interfaces::Node& node) // Initialize relevant QT models. std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other")); OptionsModel optionsModel(node); + bilingual_str error; + QVERIFY(optionsModel.Init(error)); ClientModel clientModel(node, &optionsModel); WalletContext& context = *node.walletLoader().context(); AddWallet(context, wallet); diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 9648ef6188..6fc7a52435 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -70,14 +70,9 @@ void AppTests::appTests() } #endif - fs::create_directories([] { - BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to - return gArgs.GetDataDirNet() / "blocks"; - }()); - qRegisterMetaType<interfaces::BlockAndHeaderTipInfo>("interfaces::BlockAndHeaderTipInfo"); m_app.parameterSetup(); - m_app.createOptionsModel(true /* reset settings */); + QVERIFY(m_app.createOptionsModel(true /* reset settings */)); QScopedPointer<const NetworkStyle> style(NetworkStyle::instantiate(Params().NetworkIDString())); m_app.setupPlatformStyle(); m_app.createWindow(style.data()); diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index 2ef9230c59..3bd0af19ad 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -13,6 +13,63 @@ #include <univalue.h> +#include <fstream> + +OptionTests::OptionTests(interfaces::Node& node) : m_node(node) +{ + gArgs.LockSettings([&](util::Settings& s) { m_previous_settings = s; }); +} + +void OptionTests::init() +{ + // reset args + gArgs.LockSettings([&](util::Settings& s) { s = m_previous_settings; }); + gArgs.ClearPathCache(); +} + +void OptionTests::migrateSettings() +{ + // Set legacy QSettings and verify that they get cleared and migrated to + // settings.json + QSettings settings; + settings.setValue("nDatabaseCache", 600); + settings.setValue("nThreadsScriptVerif", 12); + settings.setValue("fUseUPnP", false); + settings.setValue("fListen", false); + settings.setValue("bPrune", true); + settings.setValue("nPruneSize", 3); + settings.setValue("fUseProxy", true); + settings.setValue("addrProxy", "proxy:123"); + settings.setValue("fUseSeparateProxyTor", true); + settings.setValue("addrSeparateProxyTor", "onion:234"); + + settings.sync(); + + OptionsModel options{m_node}; + bilingual_str error; + QVERIFY(options.Init(error)); + QVERIFY(!settings.contains("nDatabaseCache")); + QVERIFY(!settings.contains("nThreadsScriptVerif")); + QVERIFY(!settings.contains("fUseUPnP")); + QVERIFY(!settings.contains("fListen")); + QVERIFY(!settings.contains("bPrune")); + QVERIFY(!settings.contains("nPruneSize")); + QVERIFY(!settings.contains("fUseProxy")); + QVERIFY(!settings.contains("addrProxy")); + QVERIFY(!settings.contains("fUseSeparateProxyTor")); + QVERIFY(!settings.contains("addrSeparateProxyTor")); + + std::ifstream file(gArgs.GetDataDirNet() / "settings.json"); + QCOMPARE(std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n" + " \"dbcache\": \"600\",\n" + " \"listen\": false,\n" + " \"onion\": \"onion:234\",\n" + " \"par\": \"12\",\n" + " \"proxy\": \"proxy:123\",\n" + " \"prune\": \"2861\"\n" + "}\n"); +} + void OptionTests::integerGetArgBug() { // Test regression https://github.com/bitcoin/bitcoin/issues/24457. Ensure @@ -23,7 +80,8 @@ void OptionTests::integerGetArgBug() settings.rw_settings["prune"] = 3814; }); gArgs.WriteSettingsFile(); - OptionsModel{m_node}; + bilingual_str error; + QVERIFY(OptionsModel{m_node}.Init(error)); gArgs.LockSettings([&](util::Settings& settings) { settings.rw_settings.erase("prune"); }); @@ -36,8 +94,6 @@ void OptionTests::parametersInteraction() // It was fixed via https://github.com/bitcoin-core/gui/pull/568. // With fListen=false in ~/.config/Bitcoin/Bitcoin-Qt.conf and all else left as default, // bitcoin-qt should set both -listen and -listenonion to false and start successfully. - gArgs.ClearPathCache(); - gArgs.LockSettings([&](util::Settings& s) { s.forced_settings.erase("listen"); s.forced_settings.erase("listenonion"); @@ -48,7 +104,8 @@ void OptionTests::parametersInteraction() QSettings settings; settings.setValue("fListen", false); - OptionsModel{m_node}; + bilingual_str error; + QVERIFY(OptionsModel{m_node}.Init(error)); const bool expected{false}; diff --git a/src/qt/test/optiontests.h b/src/qt/test/optiontests.h index 257a0b65be..286d785572 100644 --- a/src/qt/test/optiontests.h +++ b/src/qt/test/optiontests.h @@ -6,6 +6,8 @@ #define BITCOIN_QT_TEST_OPTIONTESTS_H #include <qt/optionsmodel.h> +#include <univalue.h> +#include <util/settings.h> #include <QObject> @@ -13,14 +15,17 @@ class OptionTests : public QObject { Q_OBJECT public: - explicit OptionTests(interfaces::Node& node) : m_node(node) {} + explicit OptionTests(interfaces::Node& node); private Q_SLOTS: + void init(); // called before each test function execution. + void migrateSettings(); void integerGetArgBug(); void parametersInteraction(); private: interfaces::Node& m_node; + util::Settings m_previous_settings; }; #endif // BITCOIN_QT_TEST_OPTIONTESTS_H diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index aeedd92834..de23f51c92 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -58,9 +58,10 @@ int main(int argc, char* argv[]) // regtest params. // // All tests must use their own testing setup (if needed). - { + fs::create_directories([] { BasicTestingSetup dummy{CBaseChainParams::REGTEST}; - } + return gArgs.GetDataDirNet() / "blocks"; + }()); std::unique_ptr<interfaces::Init> init = interfaces::MakeGuiInit(argc, argv); gArgs.ForceSetArg("-listen", "0"); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index bc06f0f23b..7671bfc739 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -185,6 +185,8 @@ void TestGUI(interfaces::Node& node) SendCoinsDialog sendCoinsDialog(platformStyle.get()); TransactionView transactionView(platformStyle.get()); OptionsModel optionsModel(node); + bilingual_str error; + QVERIFY(optionsModel.Init(error)); ClientModel clientModel(node, &optionsModel); WalletContext& context = *node.walletLoader().context(); AddWallet(context, wallet); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 1f6c90af4a..454c9a05d0 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -234,7 +234,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact return SendCoinsReturn(OK); } -WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &transaction) +void WalletModel::sendCoins(WalletModelTransaction& transaction) { QByteArray transaction_array; /* store serialized transaction */ @@ -280,8 +280,6 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran } checkBalanceChanged(m_wallet->getBalances()); // update balance immediately, otherwise there could be a short noticeable delay until pollBalanceChanged hits - - return SendCoinsReturn(OK); } OptionsModel* WalletModel::getOptionsModel() const diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index d524d48111..a52290dee8 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -102,7 +102,7 @@ public: SendCoinsReturn prepareTransaction(WalletModelTransaction &transaction, const wallet::CCoinControl& coinControl); // Send coins to a list of recipients - SendCoinsReturn sendCoins(WalletModelTransaction &transaction); + void sendCoins(WalletModelTransaction& transaction); // Wallet encryption bool setWalletEncrypted(const SecureString& passphrase); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d682a7d3e8..bf4bfbb95f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -66,7 +66,7 @@ struct CUpdatedBlock int height; }; -static Mutex cs_blockchange; +static GlobalMutex cs_blockchange; static std::condition_variable cond_blockchange; static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index af00acdc9f..66ed18045e 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -19,14 +19,14 @@ #include <mutex> #include <unordered_map> -static Mutex g_rpc_warmup_mutex; +static GlobalMutex g_rpc_warmup_mutex; static std::atomic<bool> g_rpc_running{false}; static bool fRPCInWarmup GUARDED_BY(g_rpc_warmup_mutex) = true; static std::string rpcWarmupStatus GUARDED_BY(g_rpc_warmup_mutex) = "RPC server started"; /* Timer-creating functions */ static RPCTimerInterface* timerInterface = nullptr; /* Map of name to timer. */ -static Mutex g_deadline_timers_mutex; +static GlobalMutex g_deadline_timers_mutex; static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers GUARDED_BY(g_deadline_timers_mutex); static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler); diff --git a/src/secp256k1/build-aux/m4/bitcoin_secp.m4 b/src/secp256k1/build-aux/m4/bitcoin_secp.m4 index dda770e001..9cb54de098 100644 --- a/src/secp256k1/build-aux/m4/bitcoin_secp.m4 +++ b/src/secp256k1/build-aux/m4/bitcoin_secp.m4 @@ -1,7 +1,7 @@ dnl escape "$0x" below using the m4 quadrigaph @S|@, and escape it again with a \ for the shell. AC_DEFUN([SECP_64BIT_ASM_CHECK],[ AC_MSG_CHECKING(for x86_64 assembly availability) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +AC_LINK_IFELSE([AC_LANG_PROGRAM([[ #include <stdint.h>]],[[ uint64_t a = 11, tmp; __asm__ __volatile__("movq \@S|@0x100000000,%1; mulq %%rsi" : "+a"(a) : "S"(tmp) : "cc", "%rdx"); diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h index 86ab7e556d..dddab346ae 100644 --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -141,9 +141,13 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_NO_BUILD #endif +/** At secp256k1 build-time DLL_EXPORT is defined when building objects destined + * for a shared library, but not for those intended for static libraries. + */ + #ifndef SECP256K1_API # if defined(_WIN32) -# ifdef SECP256K1_BUILD +# if defined(SECP256K1_BUILD) && defined(DLL_EXPORT) # define SECP256K1_API __declspec(dllexport) # else # define SECP256K1_API diff --git a/src/secp256k1/sage/prove_group_implementations.sage b/src/secp256k1/sage/prove_group_implementations.sage index 96ce33506a..652bd87f11 100644 --- a/src/secp256k1/sage/prove_group_implementations.sage +++ b/src/secp256k1/sage/prove_group_implementations.sage @@ -40,29 +40,26 @@ def formula_secp256k1_gej_add_var(branch, a, b): s2 = s2 * a.Z h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if branch == 2: r = formula_secp256k1_gej_double_var(a) return (constraints(), constraints(zero={h : 'h=0', i : 'i=0', a.Infinity : 'a_finite', b.Infinity : 'b_finite'}), r) if branch == 3: return (constraints(), constraints(zero={h : 'h=0', a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 + t = h * b.Z + rz = a.Z * t h2 = h^2 + h2 = -h2 h3 = h2 * h - h = h * b.Z - rz = a.Z * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) @@ -80,28 +77,25 @@ def formula_secp256k1_gej_add_ge_var(branch, a, b): s2 = s2 * a.Z h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if (branch == 2): r = formula_secp256k1_gej_double_var(a) return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0', i : 'i=0'}), r) if (branch == 3): return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 - h2 = h^2 - h3 = h * h2 rz = a.Z * h + h2 = h^2 + h2 = -h2 + h3 = h2 * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(zero={b.Z - 1 : 'b.z=1'}), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) @@ -109,14 +103,15 @@ def formula_secp256k1_gej_add_zinv_var(branch, a, b): """libsecp256k1's secp256k1_gej_add_zinv_var""" bzinv = b.Z^(-1) if branch == 0: - return (constraints(), constraints(nonzero={b.Infinity : 'b_infinite'}), a) - if branch == 1: + rinf = b.Infinity bzinv2 = bzinv^2 bzinv3 = bzinv2 * bzinv rx = b.X * bzinv2 ry = b.Y * bzinv3 rz = 1 - return (constraints(), constraints(zero={b.Infinity : 'b_finite'}, nonzero={a.Infinity : 'a_infinite'}), jacobianpoint(rx, ry, rz)) + return (constraints(), constraints(nonzero={a.Infinity : 'a_infinite'}), jacobianpoint(rx, ry, rz, rinf)) + if branch == 1: + return (constraints(), constraints(zero={a.Infinity : 'a_finite'}, nonzero={b.Infinity : 'b_infinite'}), a) azz = a.Z * bzinv z12 = azz^2 u1 = a.X @@ -126,29 +121,25 @@ def formula_secp256k1_gej_add_zinv_var(branch, a, b): s2 = s2 * azz h = -u1 h = h + u2 - i = -s1 - i = i + s2 + i = -s2 + i = i + s1 if branch == 2: r = formula_secp256k1_gej_double_var(a) return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0', i : 'i=0'}), r) if branch == 3: return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite', h : 'h=0'}, nonzero={i : 'i!=0'}), point_at_infinity()) - i2 = i^2 + rz = a.Z * h h2 = h^2 - h3 = h * h2 - rz = a.Z - rz = rz * h + h2 = -h2 + h3 = h2 * h t = u1 * h2 - rx = t - rx = rx * 2 + rx = i^2 rx = rx + h3 - rx = -rx - rx = rx + i2 - ry = -rx - ry = ry + t - ry = ry * i + rx = rx + t + rx = rx + t + t = t + rx + ry = t * i h3 = h3 * s1 - h3 = -h3 ry = ry + h3 return (constraints(), constraints(zero={a.Infinity : 'a_finite', b.Infinity : 'b_finite'}, nonzero={h : 'h!=0'}), jacobianpoint(rx, ry, rz)) diff --git a/src/secp256k1/src/bench_internal.c b/src/secp256k1/src/bench_internal.c index 3c145f306c..7eb3af28d7 100644 --- a/src/secp256k1/src/bench_internal.c +++ b/src/secp256k1/src/bench_internal.c @@ -254,6 +254,15 @@ void bench_group_add_affine_var(void* arg, int iters) { } } +void bench_group_add_zinv_var(void* arg, int iters) { + int i; + bench_inv *data = (bench_inv*)arg; + + for (i = 0; i < iters; i++) { + secp256k1_gej_add_zinv_var(&data->gej[0], &data->gej[0], &data->ge[1], &data->gej[0].y); + } +} + void bench_group_to_affine_var(void* arg, int iters) { int i; bench_inv *data = (bench_inv*)arg; @@ -376,6 +385,7 @@ int main(int argc, char **argv) { if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_var", bench_group_add_var, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine", bench_group_add_affine, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine_var", bench_group_add_affine_var, bench_setup, NULL, &data, 10, iters*10); + if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_zinv_var", bench_group_add_zinv_var, bench_setup, NULL, &data, 10, iters*10); if (d || have_flag(argc, argv, "group") || have_flag(argc, argv, "to_affine")) run_benchmark("group_to_affine_var", bench_group_to_affine_var, bench_setup, NULL, &data, 10, iters); if (d || have_flag(argc, argv, "ecmult") || have_flag(argc, argv, "wnaf")) run_benchmark("wnaf_const", bench_wnaf_const, bench_setup, NULL, &data, 10, iters); diff --git a/src/secp256k1/src/group_impl.h b/src/secp256k1/src/group_impl.h index b19b02a01f..63735ab682 100644 --- a/src/secp256k1/src/group_impl.h +++ b/src/secp256k1/src/group_impl.h @@ -330,15 +330,14 @@ static void secp256k1_gej_double_var(secp256k1_gej *r, const secp256k1_gej *a, s } static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_gej *b, secp256k1_fe *rzr) { - /* Operations: 12 mul, 4 sqr, 2 normalize, 12 mul_int/add/negate */ - secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 12 mul, 4 sqr, 11 add/negate/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, h2, h3, t; if (a->infinity) { VERIFY_CHECK(rzr == NULL); *r = *b; return; } - if (b->infinity) { if (rzr != NULL) { secp256k1_fe_set_int(rzr, 1); @@ -347,7 +346,6 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons return; } - r->infinity = 0; secp256k1_fe_sqr(&z22, &b->z); secp256k1_fe_sqr(&z12, &a->z); secp256k1_fe_mul(&u1, &a->x, &z22); @@ -355,7 +353,7 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons secp256k1_fe_mul(&s1, &a->y, &z22); secp256k1_fe_mul(&s1, &s1, &b->z); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &a->z); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, rzr); @@ -367,24 +365,33 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons } return; } - secp256k1_fe_sqr(&i2, &i); - secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); - secp256k1_fe_mul(&h, &h, &b->z); + + r->infinity = 0; + secp256k1_fe_mul(&t, &h, &b->z); if (rzr != NULL) { - *rzr = h; + *rzr = t; } - secp256k1_fe_mul(&r->z, &a->z, &h); + secp256k1_fe_mul(&r->z, &a->z, &t); + + secp256k1_fe_sqr(&h2, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_ge *b, secp256k1_fe *rzr) { - /* 8 mul, 3 sqr, 4 normalize, 12 mul_int/add/negate */ - secp256k1_fe z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 8 mul, 3 sqr, 13 add/negate/normalize_weak/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe z12, u1, u2, s1, s2, h, i, h2, h3, t; if (a->infinity) { VERIFY_CHECK(rzr == NULL); secp256k1_gej_set_ge(r, b); @@ -397,7 +404,6 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c *r = *a; return; } - r->infinity = 0; secp256k1_fe_sqr(&z12, &a->z); u1 = a->x; secp256k1_fe_normalize_weak(&u1); @@ -405,7 +411,7 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c s1 = a->y; secp256k1_fe_normalize_weak(&s1); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &a->z); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, rzr); @@ -417,28 +423,33 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c } return; } - secp256k1_fe_sqr(&i2, &i); - secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); + + r->infinity = 0; if (rzr != NULL) { *rzr = h; } secp256k1_fe_mul(&r->z, &a->z, &h); + + secp256k1_fe_sqr(&h2, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_ge *b, const secp256k1_fe *bzinv) { - /* 9 mul, 3 sqr, 4 normalize, 12 mul_int/add/negate */ - secp256k1_fe az, z12, u1, u2, s1, s2, h, i, i2, h2, h3, t; + /* 9 mul, 3 sqr, 13 add/negate/normalize_weak/normalizes_to_zero (ignoring special cases) */ + secp256k1_fe az, z12, u1, u2, s1, s2, h, i, h2, h3, t; - if (b->infinity) { - *r = *a; - return; - } if (a->infinity) { secp256k1_fe bzinv2, bzinv3; r->infinity = b->infinity; @@ -449,7 +460,10 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, secp256k1_fe_set_int(&r->z, 1); return; } - r->infinity = 0; + if (b->infinity) { + *r = *a; + return; + } /** We need to calculate (rx,ry,rz) = (ax,ay,az) + (bx,by,1/bzinv). Due to * secp256k1's isomorphism we can multiply the Z coordinates on both sides @@ -467,7 +481,7 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, s1 = a->y; secp256k1_fe_normalize_weak(&s1); secp256k1_fe_mul(&s2, &b->y, &z12); secp256k1_fe_mul(&s2, &s2, &az); secp256k1_fe_negate(&h, &u1, 1); secp256k1_fe_add(&h, &u2); - secp256k1_fe_negate(&i, &s1, 1); secp256k1_fe_add(&i, &s2); + secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); if (secp256k1_fe_normalizes_to_zero_var(&h)) { if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, NULL); @@ -476,14 +490,23 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, } return; } - secp256k1_fe_sqr(&i2, &i); + + r->infinity = 0; + secp256k1_fe_mul(&r->z, &a->z, &h); + secp256k1_fe_sqr(&h2, &h); - secp256k1_fe_mul(&h3, &h, &h2); - r->z = a->z; secp256k1_fe_mul(&r->z, &r->z, &h); + secp256k1_fe_negate(&h2, &h2, 1); + secp256k1_fe_mul(&h3, &h2, &h); secp256k1_fe_mul(&t, &u1, &h2); - r->x = t; secp256k1_fe_mul_int(&r->x, 2); secp256k1_fe_add(&r->x, &h3); secp256k1_fe_negate(&r->x, &r->x, 3); secp256k1_fe_add(&r->x, &i2); - secp256k1_fe_negate(&r->y, &r->x, 5); secp256k1_fe_add(&r->y, &t); secp256k1_fe_mul(&r->y, &r->y, &i); - secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_negate(&h3, &h3, 1); + + secp256k1_fe_sqr(&r->x, &i); + secp256k1_fe_add(&r->x, &h3); + secp256k1_fe_add(&r->x, &t); + secp256k1_fe_add(&r->x, &t); + + secp256k1_fe_add(&t, &r->x); + secp256k1_fe_mul(&r->y, &t, &i); + secp256k1_fe_mul(&h3, &h3, &s1); secp256k1_fe_add(&r->y, &h3); } diff --git a/src/sync.h b/src/sync.h index a175926113..7ec4b668ac 100644 --- a/src/sync.h +++ b/src/sync.h @@ -129,10 +129,22 @@ using RecursiveMutex = AnnotatedMixin<std::recursive_mutex>; /** Wrapped mutex: supports waiting but not recursive locking */ using Mutex = AnnotatedMixin<std::mutex>; +/** Different type to mark Mutex at global scope + * + * Thread safety analysis can't handle negative assertions about mutexes + * with global scope well, so mark them with a separate type, and + * eventually move all the mutexes into classes so they are not globally + * visible. + * + * See: https://github.com/bitcoin/bitcoin/pull/20272#issuecomment-720755781 + */ +class GlobalMutex : public Mutex { }; + #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) inline void AssertLockNotHeldInline(const char* name, const char* file, int line, Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) { AssertLockNotHeldInternal(name, file, line, cs); } inline void AssertLockNotHeldInline(const char* name, const char* file, int line, RecursiveMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } +inline void AssertLockNotHeldInline(const char* name, const char* file, int line, GlobalMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); } #define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs) /** Wrapper around std::unique_lock style lock for Mutex. */ @@ -232,12 +244,26 @@ public: template<typename MutexArg> using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>; -#define LOCK(cs) DebugLock<decltype(cs)> UNIQUE_NAME(criticalblock)(cs, #cs, __FILE__, __LINE__) +// When locking a Mutex, require negative capability to ensure the lock +// is not already held +inline Mutex& MaybeCheckNotHeld(Mutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; } +inline Mutex* MaybeCheckNotHeld(Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; } + +// When locking a GlobalMutex, just check it is not locked in the surrounding scope +inline GlobalMutex& MaybeCheckNotHeld(GlobalMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } +inline GlobalMutex* MaybeCheckNotHeld(GlobalMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } + +// When locking a RecursiveMutex, it's okay to already hold the lock +// but check that it is not known to be locked in the surrounding scope anyway +inline RecursiveMutex& MaybeCheckNotHeld(RecursiveMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } +inline RecursiveMutex* MaybeCheckNotHeld(RecursiveMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; } + +#define LOCK(cs) DebugLock<decltype(cs)> UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define LOCK2(cs1, cs2) \ - DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \ - DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__); -#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true) -#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__) + DebugLock<decltype(cs1)> criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \ + DebugLock<decltype(cs2)> criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__); +#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true) +#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ @@ -276,7 +302,7 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove //! //! The above is detectable at compile-time with the -Wreturn-local-addr flag in //! gcc and the -Wreturn-stack-address flag in clang, both enabled by default. -#define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }() +#define WITH_LOCK(cs, code) (MaybeCheckNotHeld(cs), [&]() -> decltype(auto) { LOCK(cs); code; }()) class CSemaphore { diff --git a/src/timedata.cpp b/src/timedata.cpp index 7faf22aba0..d0ca609a48 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -16,7 +16,7 @@ #include <util/translation.h> #include <warnings.h> -static Mutex g_timeoffset_mutex; +static GlobalMutex g_timeoffset_mutex; static int64_t nTimeOffset GUARDED_BY(g_timeoffset_mutex) = 0; /** diff --git a/src/util/system.cpp b/src/util/system.cpp index f88b0fac77..6e2638a5d6 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -96,7 +96,7 @@ const char * const BITCOIN_SETTINGS_FILENAME = "settings.json"; ArgsManager gArgs; /** Mutex to protect dir_locks. */ -static Mutex cs_dir_locks; +static GlobalMutex cs_dir_locks; /** A map that contains all the currently held directory locks. After * successful locking, these will be held here until the global destructor * cleans them up and thus automatically unlocks them, or ReleaseDirectoryLocks diff --git a/src/validation.cpp b/src/validation.cpp index 23ad221ffe..1a8f501863 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -122,7 +122,7 @@ static constexpr int PRUNE_LOCK_BUFFER{10}; */ RecursiveMutex cs_main; -Mutex g_best_block_mutex; +GlobalMutex g_best_block_mutex; std::condition_variable g_best_block_cv; uint256 g_best_block; bool g_parallel_script_checks{false}; diff --git a/src/validation.h b/src/validation.h index cc94add3cb..70b6c072e6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -113,7 +113,7 @@ enum class SynchronizationState { }; extern RecursiveMutex cs_main; -extern Mutex g_best_block_mutex; +extern GlobalMutex g_best_block_mutex; extern std::condition_variable g_best_block_cv; /** Used to notify getblocktemplate RPC of new tips. */ extern uint256 g_best_block; diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 2515df3177..053fb8f983 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -23,7 +23,7 @@ namespace wallet { static constexpr int32_t WALLET_SCHEMA_VERSION = 0; -static Mutex g_sqlite_mutex; +static GlobalMutex g_sqlite_mutex; static int g_sqlite_count GUARDED_BY(g_sqlite_mutex) = 0; static void ErrorLogCallback(void* arg, int code, const char* msg) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0c83c3d954..910562e669 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -173,8 +173,8 @@ void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr<CWallet>& } } -static Mutex g_loading_wallet_mutex; -static Mutex g_wallet_release_mutex; +static GlobalMutex g_loading_wallet_mutex; +static GlobalMutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; static std::set<std::string> g_loading_wallet_set GUARDED_BY(g_loading_wallet_mutex); static std::set<std::string> g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex); diff --git a/src/warnings.cpp b/src/warnings.cpp index 60388cc713..dabb194ce1 100644 --- a/src/warnings.cpp +++ b/src/warnings.cpp @@ -12,7 +12,7 @@ #include <vector> -static Mutex g_warnings_mutex; +static GlobalMutex g_warnings_mutex; static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex); static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false; diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py index 6802da8d48..5de9ff203c 100755 --- a/test/functional/feature_bind_extra.py +++ b/test/functional/feature_bind_extra.py @@ -18,12 +18,12 @@ from test_framework.test_framework import ( SkipTest, ) from test_framework.util import ( - PORT_MIN, - PORT_RANGE, assert_equal, + p2p_port, rpc_port, ) + class BindExtraTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -33,11 +33,6 @@ class BindExtraTest(BitcoinTestFramework): self.num_nodes = 2 def setup_network(self): - # Override setup_network() because we want to put the result of - # p2p_port() in self.extra_args[], before the nodes are started. - # p2p_port() is not usable in set_test_params() because PortSeed.n is - # not set at that time. - # Due to OS-specific network stats queries, we only run on Linux. self.log.info("Checking for Linux") if not sys.platform.startswith('linux'): @@ -45,8 +40,8 @@ class BindExtraTest(BitcoinTestFramework): loopback_ipv4 = addr_to_hex("127.0.0.1") - # Start custom ports after p2p and rpc ports. - port = PORT_MIN + 2 * PORT_RANGE + # Start custom ports by reusing unused p2p ports + port = p2p_port(self.num_nodes) # Array of tuples [command line arguments, expected bind addresses]. self.expected = [] diff --git a/test/functional/feature_proxy.py b/test/functional/feature_proxy.py index 50e0e2c4cc..dd3cdc96ca 100755 --- a/test/functional/feature_proxy.py +++ b/test/functional/feature_proxy.py @@ -40,19 +40,15 @@ addnode connect to a CJDNS address """ import socket -import os from test_framework.socks5 import Socks5Configuration, Socks5Command, Socks5Server, AddressType from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( - PORT_MIN, - PORT_RANGE, assert_equal, + p2p_port, ) from test_framework.netutil import test_ipv6_local -RANGE_BEGIN = PORT_MIN + 2 * PORT_RANGE # Start after p2p and rpc ports - # Networks returned by RPC getpeerinfo. NET_UNROUTABLE = "not_publicly_routable" NET_IPV4 = "ipv4" @@ -75,19 +71,19 @@ class ProxyTest(BitcoinTestFramework): # Create two proxies on different ports # ... one unauthenticated self.conf1 = Socks5Configuration() - self.conf1.addr = ('127.0.0.1', RANGE_BEGIN + (os.getpid() % 1000)) + self.conf1.addr = ('127.0.0.1', p2p_port(self.num_nodes)) self.conf1.unauth = True self.conf1.auth = False # ... one supporting authenticated and unauthenticated (Tor) self.conf2 = Socks5Configuration() - self.conf2.addr = ('127.0.0.1', RANGE_BEGIN + 1000 + (os.getpid() % 1000)) + self.conf2.addr = ('127.0.0.1', p2p_port(self.num_nodes + 1)) self.conf2.unauth = True self.conf2.auth = True if self.have_ipv6: # ... one on IPv6 with similar configuration self.conf3 = Socks5Configuration() self.conf3.af = socket.AF_INET6 - self.conf3.addr = ('::1', RANGE_BEGIN + 2000 + (os.getpid() % 1000)) + self.conf3.addr = ('::1', p2p_port(self.num_nodes + 2)) self.conf3.unauth = True self.conf3.auth = True else: diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index a8492bd6eb..4c1e86e93d 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -684,7 +684,6 @@ class ReplaceByFeeTest(BitcoinTestFramework): utxo_to_spend=parent_utxo, sequence=SEQUENCE_FINAL, fee_rate=Decimal('0.02'), - mempool_valid=False, ) # Broadcast replacement child tx diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 3619e05761..88fb09e08b 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -7,12 +7,12 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import COIN from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, + create_lots_of_big_transactions, gen_return_txouts, ) from test_framework.wallet import MiniWallet @@ -29,16 +29,6 @@ class MempoolLimitTest(BitcoinTestFramework): ]] self.supports_cli = False - def send_large_txs(self, node, miniwallet, txouts, fee, tx_batch_size): - for _ in range(tx_batch_size): - tx = miniwallet.create_self_transfer(from_node=node, fee_rate=0, mempool_valid=False)['tx'] - for txout in txouts: - tx.vout.append(txout) - tx.vout[0].nValue -= int(fee * COIN) - res = node.testmempoolaccept([tx.serialize().hex()])[0] - assert_equal(res['fees']['base'], fee) - miniwallet.sendrawtransaction(from_node=node, tx_hex=tx.serialize().hex()) - def run_test(self): txouts = gen_return_txouts() node = self.nodes[0] @@ -71,7 +61,7 @@ class MempoolLimitTest(BitcoinTestFramework): self.log.info("Fill up the mempool with txs with higher fee rate") for batch_of_txid in range(num_of_batches): fee = (batch_of_txid + 1) * base_fee - self.send_large_txs(node, miniwallet, txouts, fee, tx_batch_size) + create_lots_of_big_transactions(miniwallet, node, fee, tx_batch_size, txouts) self.log.info('The tx should be evicted by now') # The number of transactions created should be greater than the ones present in the mempool @@ -85,7 +75,7 @@ class MempoolLimitTest(BitcoinTestFramework): # Deliberately try to create a tx with a fee less than the minimum mempool fee to assert that it does not get added to the mempool self.log.info('Create a mempool tx that will not pass mempoolminfee') - assert_raises_rpc_error(-26, "mempool min fee not met", miniwallet.send_self_transfer, from_node=node, fee_rate=relayfee, mempool_valid=False) + assert_raises_rpc_error(-26, "mempool min fee not met", miniwallet.send_self_transfer, from_node=node, fee_rate=relayfee) if __name__ == '__main__': diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py index 91f2d0051a..8dd6cc9cea 100755 --- a/test/functional/mempool_reorg.py +++ b/test/functional/mempool_reorg.py @@ -53,8 +53,7 @@ class MempoolCoinbaseTest(BitcoinTestFramework): utxo = wallet.get_utxo(txid=coinbase_txids[0]) timelock_tx = wallet.create_self_transfer( utxo_to_spend=utxo, - mempool_valid=False, - locktime=self.nodes[0].getblockcount() + 2 + locktime=self.nodes[0].getblockcount() + 2, )['hex'] self.log.info("Check that the time-locked transaction is too immature to spend") diff --git a/test/functional/mempool_spend_coinbase.py b/test/functional/mempool_spend_coinbase.py index 9c43ddaf6f..3585871350 100755 --- a/test/functional/mempool_spend_coinbase.py +++ b/test/functional/mempool_spend_coinbase.py @@ -40,7 +40,7 @@ class MempoolSpendCoinbaseTest(BitcoinTestFramework): spend_mature_id = wallet.send_self_transfer(from_node=self.nodes[0], utxo_to_spend=utxo_mature)["txid"] # other coinbase should be too immature to spend - immature_tx = wallet.create_self_transfer(utxo_to_spend=utxo_immature, mempool_valid=False) + immature_tx = wallet.create_self_transfer(utxo_to_spend=utxo_immature) assert_raises_rpc_error(-26, "bad-txns-premature-spend-of-coinbase", lambda: self.nodes[0].sendrawtransaction(immature_tx['hex'])) diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index a15fbe5a24..e0f2446b2d 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -7,16 +7,22 @@ from decimal import Decimal import time -from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import COIN, MAX_BLOCK_WEIGHT +from test_framework.messages import ( + COIN, + MAX_BLOCK_WEIGHT, +) from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, create_confirmed_utxos, create_lots_of_big_transactions, gen_return_txouts +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + create_lots_of_big_transactions, + gen_return_txouts, +) from test_framework.wallet import MiniWallet class PrioritiseTransactionTest(BitcoinTestFramework): def set_test_params(self): - self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ "-printpriority=1", @@ -24,12 +30,8 @@ class PrioritiseTransactionTest(BitcoinTestFramework): ]] * self.num_nodes self.supports_cli = False - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - def test_diamond(self): self.log.info("Test diamond-shape package with priority") - self.generate(self.wallet, COINBASE_MATURITY + 1) mock_time = int(time.time()) self.nodes[0].setmocktime(mock_time) @@ -104,6 +106,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): def run_test(self): self.wallet = MiniWallet(self.nodes[0]) + self.wallet.rescan_utxos() # Test `prioritisetransaction` required parameters assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction) @@ -131,7 +134,10 @@ class PrioritiseTransactionTest(BitcoinTestFramework): self.relayfee = self.nodes[0].getnetworkinfo()['relayfee'] utxo_count = 90 - utxos = create_confirmed_utxos(self, self.relayfee, self.nodes[0], utxo_count) + utxos = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=utxo_count)['new_utxos'] + self.generate(self.wallet, 1) + assert_equal(len(self.nodes[0].getrawmempool()), 0) + base_fee = self.relayfee*100 # our transactions are smaller than 100kb txids = [] @@ -141,7 +147,13 @@ class PrioritiseTransactionTest(BitcoinTestFramework): txids.append([]) start_range = i * range_size end_range = start_range + range_size - txids[i] = create_lots_of_big_transactions(self.nodes[0], self.txouts, utxos[start_range:end_range], end_range - start_range, (i+1)*base_fee) + txids[i] = create_lots_of_big_transactions( + self.wallet, + self.nodes[0], + (i+1) * base_fee, + end_range - start_range, + self.txouts, + utxos[start_range:end_range]) # Make sure that the size of each group of transactions exceeds # MAX_BLOCK_WEIGHT // 4 -- otherwise the test needs to be revised to @@ -200,17 +212,9 @@ class PrioritiseTransactionTest(BitcoinTestFramework): assert x not in mempool # Create a free transaction. Should be rejected. - utxo_list = self.nodes[0].listunspent() - assert len(utxo_list) > 0 - utxo = utxo_list[0] - - inputs = [] - outputs = {} - inputs.append({"txid" : utxo["txid"], "vout" : utxo["vout"]}) - outputs[self.nodes[0].getnewaddress()] = utxo["amount"] - raw_tx = self.nodes[0].createrawtransaction(inputs, outputs) - tx_hex = self.nodes[0].signrawtransactionwithwallet(raw_tx)["hex"] - tx_id = self.nodes[0].decoderawtransaction(tx_hex)["txid"] + tx_res = self.wallet.create_self_transfer(from_node=self.nodes[0], fee_rate=0) + tx_hex = tx_res['hex'] + tx_id = tx_res['txid'] # This will raise an exception due to min relay fee not being met assert_raises_rpc_error(-26, "min relay fee not met", self.nodes[0].sendrawtransaction, tx_hex) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py index c934a97729..8907c34a89 100755 --- a/test/functional/p2p_getaddr_caching.py +++ b/test/functional/p2p_getaddr_caching.py @@ -14,8 +14,7 @@ from test_framework.p2p import ( from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - PORT_MIN, - PORT_RANGE, + p2p_port, ) # As defined in net_processing. @@ -44,10 +43,9 @@ class AddrReceiver(P2PInterface): class AddrTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - # Start onion ports after p2p and rpc ports. - port = PORT_MIN + 2 * PORT_RANGE - self.onion_port1 = port - self.onion_port2 = port + 1 + # Use some of the remaining p2p ports for the onion binds. + self.onion_port1 = p2p_port(self.num_nodes) + self.onion_port2 = p2p_port(self.num_nodes + 1) self.extra_args = [ [f"-bind=127.0.0.1:{self.onion_port1}=onion", f"-bind=127.0.0.1:{self.onion_port2}=onion"], ] diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 1f95814e18..dee6d777af 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -285,7 +285,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Test a transaction with a large fee. # Fee rate is 0.20000000 BTC/kvB - tx = self.wallet.create_self_transfer(mempool_valid=False, from_node=self.nodes[0], fee_rate=Decimal('0.20000000')) + tx = self.wallet.create_self_transfer(from_node=self.nodes[0], fee_rate=Decimal("0.20000000")) # Thus, testmempoolaccept should reject testres = self.nodes[2].testmempoolaccept([tx['hex']])[0] assert_equal(testres['allowed'], False) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index a39ee003ef..3f02d21d42 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -223,11 +223,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # It still needs to exist and be None in order for tests to work however. self.options.descriptors = None + PortSeed.n = self.options.port_seed + def setup(self): """Call this method to start up the test framework object with options set.""" - PortSeed.n = self.options.port_seed - check_json_precision() self.options.cachedir = os.path.abspath(self.options.cachedir) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index b043d1a70d..21d8baf3e9 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -553,24 +553,22 @@ def gen_return_txouts(): # Create a spend of each passed-in utxo, splicing in "txouts" to each raw # transaction to make it large. See gen_return_txouts() above. -def create_lots_of_big_transactions(node, txouts, utxos, num, fee): - addr = node.getnewaddress() +def create_lots_of_big_transactions(mini_wallet, node, fee, tx_batch_size, txouts, utxos=None): + from .messages import COIN + fee_sats = int(fee * COIN) txids = [] - from .messages import tx_from_hex - for _ in range(num): - t = utxos.pop() - inputs = [{"txid": t["txid"], "vout": t["vout"]}] - outputs = {} - change = t['amount'] - fee - outputs[addr] = satoshi_round(change) - rawtx = node.createrawtransaction(inputs, outputs) - tx = tx_from_hex(rawtx) - for txout in txouts: - tx.vout.append(txout) - newtx = tx.serialize().hex() - signresult = node.signrawtransactionwithwallet(newtx, None, "NONE") - txid = node.sendrawtransaction(signresult["hex"], 0) - txids.append(txid) + use_internal_utxos = utxos is None + for _ in range(tx_batch_size): + tx = mini_wallet.create_self_transfer( + from_node=node, + utxo_to_spend=None if use_internal_utxos else utxos.pop(), + fee_rate=0, + )["tx"] + tx.vout[0].nValue -= fee_sats + tx.vout.extend(txouts) + res = node.testmempoolaccept([tx.serialize().hex()])[0] + assert_equal(res['fees']['base'], fee) + txids.append(node.sendrawtransaction(tx.serialize().hex())) return txids @@ -578,13 +576,8 @@ def mine_large_block(test_framework, mini_wallet, node): # generate a 66k transaction, # and 14 of them is close to the 1MB block limit txouts = gen_return_txouts() - from .messages import COIN - fee = 100 * int(node.getnetworkinfo()["relayfee"] * COIN) - for _ in range(14): - tx = mini_wallet.create_self_transfer(from_node=node, fee_rate=0, mempool_valid=False)['tx'] - tx.vout[0].nValue -= fee - tx.vout.extend(txouts) - mini_wallet.sendrawtransaction(from_node=node, tx_hex=tx.serialize().hex()) + fee = 100 * node.getnetworkinfo()["relayfee"] + create_lots_of_big_transactions(mini_wallet, node, fee, 14, txouts) test_framework.generate(node, 1) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index e43dd9f61a..93f3ea9e81 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -193,7 +193,7 @@ class MiniWallet: Returns a tuple (txid, n) referring to the created external utxo outpoint. """ - tx = self.create_self_transfer(from_node=from_node, fee_rate=0, mempool_valid=False)['tx'] + tx = self.create_self_transfer(from_node=from_node, fee_rate=0)["tx"] assert_greater_than_or_equal(tx.vout[0].nValue, amount + fee) tx.vout[0].nValue -= (amount + fee) # change output -> MiniWallet tx.vout.append(CTxOut(amount, scriptPubKey)) # arbitrary output -> to be returned @@ -230,7 +230,7 @@ class MiniWallet: # create simple tx template (1 input, 1 output) tx = self.create_self_transfer( fee_rate=0, from_node=from_node, - utxo_to_spend=utxos_to_spend[0], sequence=sequence, mempool_valid=False)['tx'] + utxo_to_spend=utxos_to_spend[0], sequence=sequence)["tx"] # duplicate inputs, witnesses and outputs tx.vin = [deepcopy(tx.vin[0]) for _ in range(len(utxos_to_spend))] @@ -248,9 +248,8 @@ class MiniWallet: o.nValue = outputs_value_total // num_outputs return tx - def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node=None, utxo_to_spend=None, mempool_valid=True, locktime=0, sequence=0): - """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed. - Checking mempool validity via the testmempoolaccept RPC can be skipped by setting mempool_valid to False.""" + def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node=None, utxo_to_spend=None, locktime=0, sequence=0): + """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" from_node = from_node or self._test_node utxo_to_spend = utxo_to_spend or self.get_utxo() if self._priv_key is None: @@ -277,11 +276,7 @@ class MiniWallet: tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE]), bytes([LEAF_VERSION_TAPSCRIPT]) + self._internal_key] tx_hex = tx.serialize().hex() - if mempool_valid: - tx_info = from_node.testmempoolaccept([tx_hex])[0] - assert_equal(tx_info['allowed'], True) - assert_equal(tx_info['vsize'], vsize) - assert_equal(tx_info['fees']['base'], utxo_to_spend['value'] - Decimal(send_value) / COIN) + assert_equal(tx.get_vsize(), vsize) return {'txid': tx.rehash(), 'wtxid': tx.getwtxid(), 'hex': tx_hex, 'tx': tx} diff --git a/test/get_previous_releases.py b/test/get_previous_releases.py index cbdb67216c..7b7cfbfef5 100755 --- a/test/get_previous_releases.py +++ b/test/get_previous_releases.py @@ -106,7 +106,7 @@ def download_binary(tag, args) -> int: bin_path = 'bin/bitcoin-core-{}/test.{}'.format( match.group(1), match.group(2)) platform = args.platform - if tag < "v23" and platform in ["x86_64-apple-darwin", "aarch64-apple-darwin"]: + if tag < "v23" and platform in ["x86_64-apple-darwin", "arm64-apple-darwin"]: platform = "osx64" tarball = 'bitcoin-{tag}-{platform}.tar.gz'.format( tag=tag[1:], platform=platform) @@ -214,7 +214,7 @@ def check_host(args) -> int: 'aarch64-*-linux*': 'aarch64-linux-gnu', 'x86_64-*-linux*': 'x86_64-linux-gnu', 'x86_64-apple-darwin*': 'x86_64-apple-darwin', - 'aarch64-apple-darwin*': 'aarch64-apple-darwin', + 'aarch64-apple-darwin*': 'arm64-apple-darwin', } args.platform = '' for pattern, target in platforms.items(): |