diff options
Diffstat (limited to 'src')
68 files changed, 706 insertions, 623 deletions
diff --git a/src/Makefile.crc32c.include b/src/Makefile.crc32c.include index 113272e65e..3cbe71792c 100644 --- a/src/Makefile.crc32c.include +++ b/src/Makefile.crc32c.include @@ -14,7 +14,6 @@ CRC32C_CPPFLAGS_INT += -I$(srcdir)/crc32c/include CRC32C_CPPFLAGS_INT += -DHAVE_BUILTIN_PREFETCH=@HAVE_BUILTIN_PREFETCH@ CRC32C_CPPFLAGS_INT += -DHAVE_MM_PREFETCH=@HAVE_MM_PREFETCH@ CRC32C_CPPFLAGS_INT += -DHAVE_STRONG_GETAUXVAL=@HAVE_STRONG_GETAUXVAL@ -CRC32C_CPPFLAGS_INT += -DHAVE_WEAK_GETAUXVAL=@HAVE_WEAK_GETAUXVAL@ CRC32C_CPPFLAGS_INT += -DCRC32C_TESTS_BUILT_WITH_GLOG=0 if ENABLE_SSE42 diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 58c51bd8e0..98916460aa 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -727,7 +727,7 @@ static void MutateTx(CMutableTransaction& tx, const std::string& command, static void OutputTxJSON(const CTransaction& tx) { UniValue entry(UniValue::VOBJ); - TxToUniv(tx, uint256(), /* include_addresses */ false, entry); + TxToUniv(tx, uint256(), entry); std::string jsonOutput = entry.write(4); tfm::format(std::cout, "%s\n", jsonOutput); diff --git a/src/bloom.cpp b/src/bloom.cpp index d0128a26d7..15e06389de 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -4,20 +4,22 @@ #include <bloom.h> -#include <primitives/transaction.h> #include <hash.h> +#include <primitives/transaction.h> +#include <random.h> #include <script/script.h> #include <script/standard.h> -#include <random.h> +#include <span.h> #include <streams.h> -#include <math.h> -#include <stdlib.h> - #include <algorithm> +#include <cmath> +#include <cstdlib> +#include <limits> +#include <vector> -#define LN2SQUARED 0.4804530139182014246671025263266649717305529515945455 -#define LN2 0.6931471805599453094172321214581765680755001343602552 +static constexpr double LN2SQUARED = 0.4804530139182014246671025263266649717305529515945455; +static constexpr double LN2 = 0.6931471805599453094172321214581765680755001343602552; CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn, unsigned char nFlagsIn) : /** @@ -37,13 +39,13 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c { } -inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const +inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const { // 0xFBA4C795 chosen as it guarantees a reasonable bit difference between nHashNum values. return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (vData.size() * 8); } -void CBloomFilter::insert(const std::vector<unsigned char>& vKey) +void CBloomFilter::insert(Span<const unsigned char> vKey) { if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700) return; @@ -59,17 +61,10 @@ void CBloomFilter::insert(const COutPoint& outpoint) { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << outpoint; - std::vector<unsigned char> data(stream.begin(), stream.end()); - insert(data); + insert(stream); } -void CBloomFilter::insert(const uint256& hash) -{ - std::vector<unsigned char> data(hash.begin(), hash.end()); - insert(data); -} - -bool CBloomFilter::contains(const std::vector<unsigned char>& vKey) const +bool CBloomFilter::contains(Span<const unsigned char> vKey) const { if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700) return true; @@ -87,14 +82,7 @@ bool CBloomFilter::contains(const COutPoint& outpoint) const { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << outpoint; - std::vector<unsigned char> data(stream.begin(), stream.end()); - return contains(data); -} - -bool CBloomFilter::contains(const uint256& hash) const -{ - std::vector<unsigned char> data(hash.begin(), hash.end()); - return contains(data); + return contains(MakeUCharSpan(stream)); } bool CBloomFilter::IsWithinSizeConstraints() const @@ -198,7 +186,8 @@ CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const dou } /* Similar to CBloomFilter::Hash */ -static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, const std::vector<unsigned char>& vDataToHash) { +static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, Span<const unsigned char> vDataToHash) +{ return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash); } @@ -210,7 +199,7 @@ static inline uint32_t FastMod(uint32_t x, size_t n) { return ((uint64_t)x * (uint64_t)n) >> 32; } -void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey) +void CRollingBloomFilter::insert(Span<const unsigned char> vKey) { if (nEntriesThisGeneration == nEntriesPerGeneration) { nEntriesThisGeneration = 0; @@ -241,13 +230,7 @@ void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey) } } -void CRollingBloomFilter::insert(const uint256& hash) -{ - std::vector<unsigned char> vData(hash.begin(), hash.end()); - insert(vData); -} - -bool CRollingBloomFilter::contains(const std::vector<unsigned char>& vKey) const +bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const { for (int n = 0; n < nHashFuncs; n++) { uint32_t h = RollingBloomHash(n, nTweak, vKey); @@ -261,12 +244,6 @@ bool CRollingBloomFilter::contains(const std::vector<unsigned char>& vKey) const return true; } -bool CRollingBloomFilter::contains(const uint256& hash) const -{ - std::vector<unsigned char> vData(hash.begin(), hash.end()); - return contains(vData); -} - void CRollingBloomFilter::reset() { nTweak = GetRand(std::numeric_limits<unsigned int>::max()); diff --git a/src/bloom.h b/src/bloom.h index fdaa8abfb2..422646d8b9 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -6,16 +6,16 @@ #define BITCOIN_BLOOM_H #include <serialize.h> +#include <span.h> #include <vector> class COutPoint; class CTransaction; -class uint256; //! 20,000 items with fp rate < 0.1% or 10,000 items and <0.0001% -static const unsigned int MAX_BLOOM_FILTER_SIZE = 36000; // bytes -static const unsigned int MAX_HASH_FUNCS = 50; +static constexpr unsigned int MAX_BLOOM_FILTER_SIZE = 36000; // bytes +static constexpr unsigned int MAX_HASH_FUNCS = 50; /** * First two bits of nFlags control how much IsRelevantAndUpdate actually updates @@ -49,7 +49,7 @@ private: unsigned int nTweak; unsigned char nFlags; - unsigned int Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const; + unsigned int Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const; public: /** @@ -66,13 +66,11 @@ public: SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); } - void insert(const std::vector<unsigned char>& vKey); + void insert(Span<const unsigned char> vKey); void insert(const COutPoint& outpoint); - void insert(const uint256& hash); - bool contains(const std::vector<unsigned char>& vKey) const; + bool contains(Span<const unsigned char> vKey) const; bool contains(const COutPoint& outpoint) const; - bool contains(const uint256& hash) const; //! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS //! (catch a filter which was just deserialized which was too big) @@ -112,10 +110,8 @@ class CRollingBloomFilter public: CRollingBloomFilter(const unsigned int nElements, const double nFPRate); - void insert(const std::vector<unsigned char>& vKey); - void insert(const uint256& hash); - bool contains(const std::vector<unsigned char>& vKey) const; - bool contains(const uint256& hash) const; + void insert(Span<const unsigned char> vKey); + bool contains(Span<const unsigned char> vKey) const; void reset(); diff --git a/src/chainparams.cpp b/src/chainparams.cpp index c1e59d36e0..b155745794 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -282,7 +282,7 @@ public: bin = ParseHex("512103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae"); vSeeds.emplace_back("seed.signet.bitcoin.sprovoost.nl"); - // TODO: remove hardcoded nodes once there are more DNS seeds + // Hardcoded nodes can be removed once there are more DNS seeds vSeeds.emplace_back("178.128.221.177"); vSeeds.emplace_back("v7ajjeirttkbnt32wpy3c6w3emwnfr3fkla7hpxcfokr3ysd3kqtzmqd.onion:38333"); diff --git a/src/core_io.h b/src/core_io.h index 3b9b66574c..f00f155249 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -44,8 +44,8 @@ UniValue ValueFromAmount(const CAmount amount); std::string FormatScript(const CScript& script); std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags = 0); std::string SighashToStr(unsigned char sighash_type); -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses); -void ScriptToUniv(const CScript& script, UniValue& out, bool include_address); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr); +void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true); +void ScriptToUniv(const CScript& script, UniValue& out); +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index b35f835f42..d92c970cb6 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -141,56 +141,28 @@ std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags) return HexStr(ssTx); } -void ScriptToUniv(const CScript& script, UniValue& out, bool include_address) +void ScriptToUniv(const CScript& script, UniValue& out) { - out.pushKV("asm", ScriptToAsmStr(script)); - out.pushKV("hex", HexStr(script)); - - std::vector<std::vector<unsigned char>> solns; - TxoutType type = Solver(script, solns); - out.pushKV("type", GetTxnOutputType(type)); - - CTxDestination address; - if (include_address && ExtractDestination(script, address) && type != TxoutType::PUBKEY) { - out.pushKV("address", EncodeDestination(address)); - } + ScriptPubKeyToUniv(script, out, /* include_hex */ true, /* include_address */ false); } -// TODO: from v23 ("addresses" and "reqSigs" deprecated) this method should be refactored to remove the `include_addresses` option -// this method can also be combined with `ScriptToUniv` as they will overlap -void ScriptPubKeyToUniv(const CScript& scriptPubKey, - UniValue& out, bool fIncludeHex, bool include_addresses) +void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address) { - TxoutType type; CTxDestination address; - std::vector<CTxDestination> addresses; - int nRequired; out.pushKV("asm", ScriptToAsmStr(scriptPubKey)); - if (fIncludeHex) - out.pushKV("hex", HexStr(scriptPubKey)); + if (include_hex) out.pushKV("hex", HexStr(scriptPubKey)); - if (!ExtractDestinations(scriptPubKey, type, addresses, nRequired) || type == TxoutType::PUBKEY) { - out.pushKV("type", GetTxnOutputType(type)); - return; - } + std::vector<std::vector<unsigned char>> solns; + const TxoutType type{Solver(scriptPubKey, solns)}; - if (ExtractDestination(scriptPubKey, address)) { + if (include_address && ExtractDestination(scriptPubKey, address) && type != TxoutType::PUBKEY) { out.pushKV("address", EncodeDestination(address)); } out.pushKV("type", GetTxnOutputType(type)); - - if (include_addresses) { - UniValue a(UniValue::VARR); - for (const CTxDestination& addr : addresses) { - a.push_back(EncodeDestination(addr)); - } - out.pushKV("addresses", a); - out.pushKV("reqSigs", nRequired); - } } -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo) +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo) { entry.pushKV("txid", tx.GetHash().GetHex()); entry.pushKV("hash", tx.GetWitnessHash().GetHex()); @@ -249,7 +221,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_add out.pushKV("n", (int64_t)i); UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(txout.scriptPubKey, o, true, include_addresses); + ScriptPubKeyToUniv(txout.scriptPubKey, o, true); out.pushKV("scriptPubKey", o); vout.push_back(out); diff --git a/src/crc32c/.travis.yml b/src/crc32c/.travis.yml index d990a89f07..183a5fba45 100644 --- a/src/crc32c/.travis.yml +++ b/src/crc32c/.travis.yml @@ -4,7 +4,7 @@ language: cpp dist: bionic -osx_image: xcode10.3 +osx_image: xcode12.5 compiler: - gcc @@ -24,20 +24,20 @@ env: addons: apt: sources: - - sourceline: 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-9 main' + - sourceline: 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-12 main' key_url: 'https://apt.llvm.org/llvm-snapshot.gpg.key' - sourceline: 'ppa:ubuntu-toolchain-r/test' packages: - - clang-9 + - clang-12 - cmake - - gcc-9 - - g++-9 + - gcc-11 + - g++-11 - ninja-build homebrew: packages: - cmake - - gcc@9 - - llvm@9 + - gcc@11 + - llvm@12 - ninja update: true @@ -48,14 +48,14 @@ install: export PATH="$(brew --prefix llvm)/bin:$PATH"; fi # /usr/bin/gcc points to an older compiler on both Linux and macOS. -- if [ "$CXX" = "g++" ]; then export CXX="g++-9" CC="gcc-9"; fi +- if [ "$CXX" = "g++" ]; then export CXX="g++-11" CC="gcc-11"; fi # /usr/bin/clang points to an older compiler on both Linux and macOS. # # Homebrew's llvm package doesn't ship a versioned clang++ binary, so the values # below don't work on macOS. Fortunately, the path change above makes the # default values (clang and clang++) resolve to the correct compiler on macOS. - if [ "$TRAVIS_OS_NAME" = "linux" ]; then - if [ "$CXX" = "clang++" ]; then export CXX="clang++-9" CC="clang-9"; fi; + if [ "$CXX" = "clang++" ]; then export CXX="clang++-12" CC="clang-12"; fi; fi - echo ${CC} - echo ${CXX} diff --git a/src/crc32c/.ycm_extra_conf.py b/src/crc32c/.ycm_extra_conf.py index 536aadcec8..62daa8a4ac 100644 --- a/src/crc32c/.ycm_extra_conf.py +++ b/src/crc32c/.ycm_extra_conf.py @@ -4,10 +4,10 @@ """YouCompleteMe configuration that interprets a .clang_complete file. This module implementes the YouCompleteMe configuration API documented at: -https://github.com/Valloric/ycmd#ycm_extra_confpy-specification +https://github.com/ycm-core/ycmd#ycm_extra_confpy-specification The implementation loads and processes a .clang_complete file, documented at: -https://github.com/Rip-Rip/clang_complete/blob/master/README.md +https://github.com/xavierd/clang_complete/blob/master/README.md """ import os diff --git a/src/crc32c/README.md b/src/crc32c/README.md index 0bd69f7f09..58ba38e611 100644 --- a/src/crc32c/README.md +++ b/src/crc32c/README.md @@ -65,7 +65,7 @@ apm install autocomplete-clang build build-cmake clang-format language-cmake \ If you don't mind more setup in return for more speed, replace `autocomplete-clang` and `linter-clang` with `you-complete-me`. This requires -[setting up ycmd](https://github.com/Valloric/ycmd#building). +[setting up ycmd](https://github.com/ycm-core/ycmd#building). ```bash apm install autocomplete-plus build build-cmake clang-format language-cmake \ diff --git a/src/crc32c/src/crc32c_arm64_check.h b/src/crc32c/src/crc32c_arm64_check.h index 62a07aba09..6b80f70037 100644 --- a/src/crc32c/src/crc32c_arm64_check.h +++ b/src/crc32c/src/crc32c_arm64_check.h @@ -40,7 +40,15 @@ inline bool CanUseArm64Crc32() { // From 'arch/arm64/include/uapi/asm/hwcap.h' in Linux kernel source code. constexpr unsigned long kHWCAP_PMULL = 1 << 4; constexpr unsigned long kHWCAP_CRC32 = 1 << 7; - unsigned long hwcap = (&getauxval != nullptr) ? getauxval(AT_HWCAP) : 0; + unsigned long hwcap = +#if HAVE_STRONG_GETAUXVAL + // Some compilers warn on (&getauxval != nullptr) in the block below. + getauxval(AT_HWCAP); +#elif HAVE_WEAK_GETAUXVAL + (&getauxval != nullptr) ? getauxval(AT_HWCAP) : 0; +#else +#error This is supposed to be nested inside a check for HAVE_*_GETAUXVAL. +#endif // HAVE_STRONG_GETAUXVAL return (hwcap & (kHWCAP_PMULL | kHWCAP_CRC32)) == (kHWCAP_PMULL | kHWCAP_CRC32); #elif defined(__APPLE__) diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 8caeb32627..7f6471740f 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -39,8 +39,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-maxtxfee=<amt>", "-mintxfee=<amt>", "-paytxfee=<amt>", - "-rescan", - "-salvagewallet", "-signer=<cmd>", "-spendzeroconfchange", "-txconfirmtarget=<n>", diff --git a/src/hash.cpp b/src/hash.cpp index 3465caa3a9..92c923fbd2 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <hash.h> +#include <span.h> #include <crypto/common.h> #include <crypto/hmac_sha512.h> diff --git a/src/init.cpp b/src/init.cpp index 25b9c6b9ec..65e3c275e9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -397,7 +397,7 @@ void SetupServerArgs(ArgsManager& argsman) -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -coinstatsindex and -rescan. " + argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and -coinstatsindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -878,6 +878,8 @@ bool AppInitParameterInteraction(const ArgsManager& args) #else int fd_max = FD_SETSIZE; #endif + // Trim requested connection counts, to fit into system limitations + // <int> in std::min<int>(...) to work around FreeBSD compilation issue described in #2695 nMaxConnections = std::max(std::min<int>(nMaxConnections, fd_max - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS - NUM_FDS_MESSAGE_CAPTURE), 0); if (nFD < MIN_CORE_FILEDESCRIPTORS) return InitError(_("Not enough file descriptors available.")); diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index e78594390b..a617bb4451 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -182,14 +182,14 @@ void AddressBookPage::onEditAction() if(indexes.isEmpty()) return; - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( tab == SendingTab ? EditAddressDialog::EditSendingAddress : EditAddressDialog::EditReceivingAddress, this); - dlg.setModel(model); + dlg->setModel(model); QModelIndex origIndex = proxyModel->mapToSource(indexes.at(0)); - dlg.loadRow(origIndex.row()); - dlg.exec(); + dlg->loadRow(origIndex.row()); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void AddressBookPage::on_newAddress_clicked() diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index d4895ea6ff..00c9fd3059 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -55,6 +55,7 @@ #include <QThread> #include <QTimer> #include <QTranslator> +#include <QWindow> #if defined(QT_STATICPLUGIN) #include <QtPlugin> @@ -259,6 +260,7 @@ void BitcoinApplication::createOptionsModel(bool resetSettings) void BitcoinApplication::createWindow(const NetworkStyle *networkStyle) { window = new BitcoinGUI(node(), platformStyle, networkStyle, nullptr); + connect(window, &BitcoinGUI::quitRequested, this, &BitcoinApplication::requestShutdown); pollShutdownTimer = new QTimer(window); connect(pollShutdownTimer, &QTimer::timeout, window, &BitcoinGUI::detectShutdown); @@ -296,7 +298,7 @@ void BitcoinApplication::startThread() /* communication to and from thread */ connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult); - connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &BitcoinApplication::shutdownResult); + connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &QCoreApplication::quit); connect(&m_executor.value(), &InitExecutor::runawayException, this, &BitcoinApplication::handleRunawayException); connect(this, &BitcoinApplication::requestedInitialize, &m_executor.value(), &InitExecutor::initialize); connect(this, &BitcoinApplication::requestedShutdown, &m_executor.value(), &InitExecutor::shutdown); @@ -326,13 +328,17 @@ void BitcoinApplication::requestInitialize() void BitcoinApplication::requestShutdown() { + for (const auto w : QGuiApplication::topLevelWindows()) { + w->hide(); + } + // Show a simple window indicating shutdown status // Do this first as some of the steps may take some time below, // for example the RPC console may still be executing a command. shutdownWindow.reset(ShutdownWindow::showShutdownWindow(window)); qDebug() << __func__ << ": Requesting shutdown"; - window->hide(); + // Must disconnect node signals otherwise current thread can deadlock since // no event loop is running. window->unsubscribeFromCoreSignals(); @@ -409,15 +415,10 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead pollShutdownTimer->start(200); } else { Q_EMIT splashFinished(); // Make sure splash screen doesn't stick around during shutdown - quit(); // Exit first main loop invocation + requestShutdown(); } } -void BitcoinApplication::shutdownResult() -{ - quit(); // Exit second main loop invocation after shutdown finished -} - void BitcoinApplication::handleRunawayException(const QString &message) { QMessageBox::critical( @@ -640,8 +641,6 @@ int GuiMain(int argc, char* argv[]) WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely…").arg(PACKAGE_NAME), (HWND)app.getMainWinId()); #endif app.exec(); - app.requestShutdown(); - app.exec(); rv = app.getReturnValue(); } else { // A dialog with detailed error will have been shown by InitError() diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index 602b76052c..5678ca90d2 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -61,8 +61,6 @@ public: /// Request core initialization void requestInitialize(); - /// Request core shutdown - void requestShutdown(); /// Get process return value int getReturnValue() const { return returnValue; } @@ -77,7 +75,8 @@ public: public Q_SLOTS: void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); - void shutdownResult(); + /// Request core shutdown + void requestShutdown(); /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 862bdd3bfe..610637360b 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -107,7 +107,6 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty walletFrame = new WalletFrame(_platformStyle, this); connect(walletFrame, &WalletFrame::createWalletButtonClicked, [this] { auto activity = new CreateWalletActivity(getWalletController(), this); - connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); connect(walletFrame, &WalletFrame::message, [this](const QString& title, const QString& message, unsigned int style) { @@ -171,7 +170,9 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty frameBlocksLayout->addWidget(unitDisplayControl); frameBlocksLayout->addStretch(); frameBlocksLayout->addWidget(labelWalletEncryptionIcon); + labelWalletEncryptionIcon->hide(); frameBlocksLayout->addWidget(labelWalletHDStatusIcon); + labelWalletHDStatusIcon->hide(); } frameBlocksLayout->addWidget(labelProxyIcon); frameBlocksLayout->addStretch(); @@ -371,7 +372,7 @@ void BitcoinGUI::createActions() m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab")); m_mask_values_action->setCheckable(true); - connect(quitAction, &QAction::triggered, qApp, QApplication::quit); + connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested); connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked); connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt); connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked); @@ -416,7 +417,6 @@ void BitcoinGUI::createActions() connect(action, &QAction::triggered, [this, path] { auto activity = new OpenWalletActivity(m_wallet_controller, this); connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet); - connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater); activity->open(path); }); } @@ -431,7 +431,6 @@ void BitcoinGUI::createActions() connect(m_create_wallet_action, &QAction::triggered, [this] { auto activity = new CreateWalletActivity(m_wallet_controller, this); connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet); - connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); connect(m_close_all_wallets_action, &QAction::triggered, [this] { @@ -662,9 +661,8 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); - for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) { - addWallet(wallet_model); - } + auto activity = new LoadWalletsActivity(m_wallet_controller, this); + activity->load(); } WalletController* BitcoinGUI::getWalletController() @@ -848,8 +846,8 @@ void BitcoinGUI::aboutClicked() if(!clientModel) return; - HelpMessageDialog dlg(this, true); - dlg.exec(); + auto dlg = new HelpMessageDialog(this, /* about */ true); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void BitcoinGUI::showDebugWindow() @@ -990,10 +988,11 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab) if (!clientModel || !clientModel->getOptionsModel()) return; - OptionsDialog dlg(this, enableWallet); - dlg.setCurrentTab(tab); - dlg.setModel(clientModel->getOptionsModel()); - dlg.exec(); + auto dlg = new OptionsDialog(this, enableWallet); + connect(dlg, &OptionsDialog::quitOnReset, this, &BitcoinGUI::quitRequested); + dlg->setCurrentTab(tab); + dlg->setModel(clientModel->getOptionsModel()); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool header, SynchronizationState sync_state) @@ -1216,7 +1215,7 @@ void BitcoinGUI::closeEvent(QCloseEvent *event) // close rpcConsole in case it was open to make some space for the shutdown window rpcConsole->close(); - QApplication::quit(); + Q_EMIT quitRequested(); } else { @@ -1410,7 +1409,7 @@ void BitcoinGUI::detectShutdown() { if(rpcConsole) rpcConsole->hide(); - qApp->quit(); + Q_EMIT quitRequested(); } } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index c83cd446a0..27045f5cc3 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -214,6 +214,7 @@ private: void openOptionsDialogWithTab(OptionsDialog::Tab tab); Q_SIGNALS: + void quitRequested(); /** Signal raised when a URI was entered or dragged to the GUI */ void receivedURI(const QString &uri); /** Signal raised when RPC console shown */ diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 86dbd05b1a..e93fedad28 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -55,7 +55,7 @@ CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _m contextMenu->addAction(tr("&Copy address"), this, &CoinControlDialog::copyAddress); contextMenu->addAction(tr("Copy &label"), this, &CoinControlDialog::copyLabel); contextMenu->addAction(tr("Copy &amount"), this, &CoinControlDialog::copyAmount); - copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction &ID"), this, &CoinControlDialog::copyTransactionHash); + m_copy_transaction_outpoint_action = contextMenu->addAction(tr("Copy transaction &ID and output index"), this, &CoinControlDialog::copyTransactionOutpoint); contextMenu->addSeparator(); lockAction = contextMenu->addAction(tr("L&ock unspent"), this, &CoinControlDialog::lockCoin); unlockAction = contextMenu->addAction(tr("&Unlock unspent"), this, &CoinControlDialog::unlockCoin); @@ -180,7 +180,7 @@ void CoinControlDialog::showMenu(const QPoint &point) // disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu if (item->data(COLUMN_ADDRESS, TxHashRole).toString().length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { - copyTransactionHashAction->setEnabled(true); + m_copy_transaction_outpoint_action->setEnabled(true); if (model->wallet().isLockedCoin(COutPoint(uint256S(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt()))) { lockAction->setEnabled(false); @@ -194,7 +194,7 @@ void CoinControlDialog::showMenu(const QPoint &point) } else // this means click on parent node in tree mode -> disable all { - copyTransactionHashAction->setEnabled(false); + m_copy_transaction_outpoint_action->setEnabled(false); lockAction->setEnabled(false); unlockAction->setEnabled(false); } @@ -228,10 +228,14 @@ void CoinControlDialog::copyAddress() GUIUtil::setClipboard(contextMenuItem->text(COLUMN_ADDRESS)); } -// context menu action: copy transaction id -void CoinControlDialog::copyTransactionHash() +// context menu action: copy transaction id and vout index +void CoinControlDialog::copyTransactionOutpoint() { - GUIUtil::setClipboard(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString()); + const QString address = contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString(); + const QString vout = contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toString(); + const QString outpoint = QString("%1:%2").arg(address).arg(vout); + + GUIUtil::setClipboard(outpoint); } // context menu action: lock coin diff --git a/src/qt/coincontroldialog.h b/src/qt/coincontroldialog.h index 3a03341c9e..bcaf45df42 100644 --- a/src/qt/coincontroldialog.h +++ b/src/qt/coincontroldialog.h @@ -63,7 +63,7 @@ private: QMenu *contextMenu; QTreeWidgetItem *contextMenuItem; - QAction *copyTransactionHashAction; + QAction* m_copy_transaction_outpoint_action; QAction *lockAction; QAction *unlockAction; @@ -95,7 +95,7 @@ private Q_SLOTS: void copyAmount(); void copyLabel(); void copyAddress(); - void copyTransactionHash(); + void copyTransactionOutpoint(); void lockCoin(); void unlockCoin(); void clipboardQuantity(); diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 59d220636d..1c22124616 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -33,7 +33,7 @@ <string>Automatically start %1 after logging in to the system.</string> </property> <property name="text"> - <string>&Start %1 on system login</string> + <string>Start %1 on system &login</string> </property> </widget> </item> @@ -179,7 +179,7 @@ <property name="sizeHint" stdset="0"> <size> <width>40</width> - <height>20</height> + <height>40</height> </size> </property> </spacer> @@ -187,6 +187,16 @@ </layout> </item> <item> + <widget class="QCheckBox" name="enableServer"> + <property name="toolTip"> + <string extracomment="Tooltip text for Options window setting that enables the RPC server.">This allows you or a third party tool to communicate with the node through command-line and JSON-RPC commands.</string> + </property> + <property name="text"> + <string extracomment="An Options window setting to enable the RPC server.">Enable RPC &server</string> + </property> + </widget> + </item> + <item> <spacer name="verticalSpacer_Main"> <property name="orientation"> <enum>Qt::Vertical</enum> @@ -729,10 +739,10 @@ <item> <widget class="QLabel" name="thirdPartyTxUrlsLabel"> <property name="toolTip"> - <string>Third party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items. %s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |.</string> + <string>Third-party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items. %s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |.</string> </property> <property name="text"> - <string>&Third party transaction URLs</string> + <string>&Third-party transaction URLs</string> </property> <property name="buddy"> <cstring>thirdPartyTxUrls</cstring> @@ -742,7 +752,7 @@ <item> <widget class="QLineEdit" name="thirdPartyTxUrls"> <property name="toolTip"> - <string>Third party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items. %s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |.</string> + <string>Third-party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items. %s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |.</string> </property> <property name="placeholderText"> <string notr="true">https://example.com/tx/%s</string> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index e98e50ba14..7b1384b485 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -36,6 +36,7 @@ #include <QClipboard> #include <QDateTime> #include <QDesktopServices> +#include <QDialog> #include <QDoubleValidator> #include <QFileDialog> #include <QFont> @@ -673,14 +674,26 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction { QString prefix; if (prepend_direction) { - prefix = (conn_type == ConnectionType::INBOUND) ? QObject::tr("Inbound") : QObject::tr("Outbound") + " "; + prefix = (conn_type == ConnectionType::INBOUND) ? + /*: An inbound connection from a peer. An inbound connection + is a connection initiated by a peer. */ + QObject::tr("Inbound") : + /*: An outbound connection to a peer. An outbound connection + is a connection initiated by us. */ + QObject::tr("Outbound") + " "; } switch (conn_type) { case ConnectionType::INBOUND: return prefix; + //: Peer connection type that relays all network information. case ConnectionType::OUTBOUND_FULL_RELAY: return prefix + QObject::tr("Full Relay"); + /*: Peer connection type that relays network information about + blocks and not transactions or addresses. */ case ConnectionType::BLOCK_RELAY: return prefix + QObject::tr("Block Relay"); + //: Peer connection type established manually through one of several methods. case ConnectionType::MANUAL: return prefix + QObject::tr("Manual"); + //: Short-lived peer connection type that tests the aliveness of known addresses. case ConnectionType::FEELER: return prefix + QObject::tr("Feeler"); + //: Short-lived peer connection type that solicits known addresses from a peer. case ConnectionType::ADDR_FETCH: return prefix + QObject::tr("Address Fetch"); } // no default case, so the compiler can warn about missing cases assert(false); @@ -958,4 +971,11 @@ void PrintSlotException( PrintExceptionContinue(exception, description.c_str()); } +void ShowModalDialogAndDeleteOnClose(QDialog* dialog) +{ + dialog->setAttribute(Qt::WA_DeleteOnClose); + dialog->setWindowModality(Qt::ApplicationModal); + dialog->show(); +} + } // namespace GUIUtil diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 06a3b63668..274f0bdcbf 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -41,6 +41,7 @@ class QAbstractButton; class QAbstractItemView; class QAction; class QDateTime; +class QDialog; class QFont; class QKeySequence; class QLineEdit; @@ -417,6 +418,11 @@ namespace GUIUtil type); } + /** + * Shows a QDialog instance asynchronously, and deletes it on close. + */ + void ShowModalDialogAndDeleteOnClose(QDialog* dialog); + } // namespace GUIUtil #endif // BITCOIN_QT_GUIUTIL_H diff --git a/src/qt/initexecutor.cpp b/src/qt/initexecutor.cpp index 7060f74dab..24ae7ba73d 100644 --- a/src/qt/initexecutor.cpp +++ b/src/qt/initexecutor.cpp @@ -5,6 +5,7 @@ #include <qt/initexecutor.h> #include <interfaces/node.h> +#include <qt/guiutil.h> #include <util/system.h> #include <util/threadnames.h> @@ -18,7 +19,7 @@ InitExecutor::InitExecutor(interfaces::Node& node) : QObject(), m_node(node) { - this->moveToThread(&m_thread); + m_context.moveToThread(&m_thread); m_thread.start(); } @@ -38,29 +39,33 @@ void InitExecutor::handleRunawayException(const std::exception* e) void InitExecutor::initialize() { - try { - util::ThreadRename("qt-init"); - qDebug() << __func__ << ": Running initialization in thread"; - interfaces::BlockAndHeaderTipInfo tip_info; - bool rv = m_node.appInitMain(&tip_info); - Q_EMIT initializeResult(rv, tip_info); - } catch (const std::exception& e) { - handleRunawayException(&e); - } catch (...) { - handleRunawayException(nullptr); - } + GUIUtil::ObjectInvoke(&m_context, [this] { + try { + util::ThreadRename("qt-init"); + qDebug() << "Running initialization in thread"; + interfaces::BlockAndHeaderTipInfo tip_info; + bool rv = m_node.appInitMain(&tip_info); + Q_EMIT initializeResult(rv, tip_info); + } catch (const std::exception& e) { + handleRunawayException(&e); + } catch (...) { + handleRunawayException(nullptr); + } + }); } void InitExecutor::shutdown() { - try { - qDebug() << __func__ << ": Running Shutdown in thread"; - m_node.appShutdown(); - qDebug() << __func__ << ": Shutdown finished"; - Q_EMIT shutdownResult(); - } catch (const std::exception& e) { - handleRunawayException(&e); - } catch (...) { - handleRunawayException(nullptr); - } + GUIUtil::ObjectInvoke(&m_context, [this] { + try { + qDebug() << "Running Shutdown in thread"; + m_node.appShutdown(); + qDebug() << "Shutdown finished"; + Q_EMIT shutdownResult(); + } catch (const std::exception& e) { + handleRunawayException(&e); + } catch (...) { + handleRunawayException(nullptr); + } + }); } diff --git a/src/qt/initexecutor.h b/src/qt/initexecutor.h index 319ce40465..410c44fa2d 100644 --- a/src/qt/initexecutor.h +++ b/src/qt/initexecutor.h @@ -40,6 +40,7 @@ private: void handleRunawayException(const std::exception* e); interfaces::Node& m_node; + QObject m_context; QThread m_thread; }; diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 92644ef24b..0cc2d61df6 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -210,6 +210,7 @@ void OptionsDialog::setModel(OptionsModel *_model) connect(ui->spendZeroConfChange, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning); /* Network */ connect(ui->allowIncoming, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning); + connect(ui->enableServer, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning); connect(ui->connectSocks, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning); connect(ui->connectSocksTor, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning); /* Display */ @@ -246,6 +247,7 @@ void OptionsDialog::setMapper() mapper->addMapping(ui->mapPortUpnp, OptionsModel::MapPortUPnP); mapper->addMapping(ui->mapPortNatpmp, OptionsModel::MapPortNatpmp); mapper->addMapping(ui->allowIncoming, OptionsModel::Listen); + mapper->addMapping(ui->enableServer, OptionsModel::Server); mapper->addMapping(ui->connectSocks, OptionsModel::ProxyUse); mapper->addMapping(ui->proxyIp, OptionsModel::ProxyIP); @@ -290,7 +292,8 @@ void OptionsDialog::on_resetButton_clicked() /* reset all options and close GUI */ model->Reset(); - QApplication::quit(); + close(); + Q_EMIT quitOnReset(); } } diff --git a/src/qt/optionsdialog.h b/src/qt/optionsdialog.h index ba35ff3b67..f14aec3449 100644 --- a/src/qt/optionsdialog.h +++ b/src/qt/optionsdialog.h @@ -68,6 +68,7 @@ private Q_SLOTS: Q_SIGNALS: void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort); + void quitOnReset(); private: Ui::OptionsDialog *ui; diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index d87fc1f84a..9e2f38f7ec 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -149,6 +149,13 @@ void OptionsModel::Init(bool resetSettings) if (!gArgs.SoftSetBoolArg("-listen", settings.value("fListen").toBool())) addOverriddenOption("-listen"); + 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")) @@ -363,6 +370,8 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const return settings.value("nThreadsScriptVerif"); case Listen: return settings.value("fListen"); + case Server: + return settings.value("server"); default: return QVariant(); } @@ -528,6 +537,12 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in setRestartRequired(true); } break; + case Server: + if (settings.value("server") != value) { + settings.setValue("server", value); + setRestartRequired(true); + } + break; default: break; } diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 203ee27ad8..65544acfbd 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -69,6 +69,7 @@ public: ExternalSignerPath, // QString SpendZeroConfChange, // bool Listen, // bool + Server, // bool OptionIDRowCount, }; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 4554f11a41..1c8ed22ada 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -495,14 +495,28 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty constexpr QChar nonbreaking_hyphen(8209); const std::vector<QString> CONNECTION_TYPE_DOC{ + //: Explanatory text for an inbound peer connection. tr("Inbound: initiated by peer"), + /*: Explanatory text for an outbound peer connection that + relays all network information. This is the default behavior for + outbound connections. */ tr("Outbound Full Relay: default"), + /*: Explanatory text for an outbound peer connection that relays + network information about blocks and not transactions or addresses. */ tr("Outbound Block Relay: does not relay transactions or addresses"), + /*: Explanatory text for an outbound peer connection that was + established manually through one of several methods. The numbered + arguments are stand-ins for the methods available to establish + manual connections. */ tr("Outbound Manual: added using RPC %1 or %2/%3 configuration options") .arg("addnode") .arg(QString(nonbreaking_hyphen) + "addnode") .arg(QString(nonbreaking_hyphen) + "connect"), + /*: Explanatory text for a short-lived outbound peer connection that + is used to test the aliveness of known addresses. */ tr("Outbound Feeler: short-lived, for testing addresses"), + /*: Explanatory text for a short-lived outbound peer connection that is used + to request addresses from a peer. */ tr("Outbound Address Fetch: short-lived, for soliciting addresses")}; const QString list{"<ul><li>" + Join(CONNECTION_TYPE_DOC, QString("</li><li>")) + "</li></ul>"}; ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(list)); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index ff53d8160f..2718392940 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -399,9 +399,10 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) const QString confirmation = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner() ? tr("Confirm transaction proposal") : tr("Confirm send coins"); const QString confirmButtonText = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner() ? tr("Create Unsigned") : tr("Sign and send"); - SendConfirmationDialog confirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); - confirmationDialog.exec(); - QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); + auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); + confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); + // TODO: Replace QDialog::exec() with safer QDialog::show(). + const auto retval = static_cast<QMessageBox::StandardButton>(confirmationDialog->exec()); if(retval != QMessageBox::Yes) { @@ -914,9 +915,9 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) // Coin Control: button inputs -> show actual coin control dialog void SendCoinsDialog::coinControlButtonClicked() { - CoinControlDialog dlg(*m_coin_control, model, platformStyle); - dlg.exec(); - coinControlUpdateLabels(); + auto dlg = new CoinControlDialog(*m_coin_control, model, platformStyle); + connect(dlg, &QDialog::finished, this, &SendCoinsDialog::coinControlUpdateLabels); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } // Coin Control: checkbox custom change address diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 884ed25637..55d00bb37e 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -9,7 +9,6 @@ #include <interfaces/init.h> #include <interfaces/node.h> #include <qt/bitcoin.h> -#include <qt/initexecutor.h> #include <qt/test/apptests.h> #include <qt/test/rpcnestedtests.h> #include <qt/test/uritests.h> diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 2f16e6edb4..653f3dda6d 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -222,17 +222,21 @@ void TransactionView::setModel(WalletModel *_model) { // Add third party transaction URLs to context menu QStringList listUrls = GUIUtil::SplitSkipEmptyParts(_model->getOptionsModel()->getThirdPartyTxUrls(), "|"); + bool actions_created = false; for (int i = 0; i < listUrls.size(); ++i) { QString url = listUrls[i].trimmed(); QString host = QUrl(url, QUrl::StrictMode).host(); if (!host.isEmpty()) { - QAction *thirdPartyTxUrlAction = new QAction(host, this); // use host as menu item label - if (i == 0) + if (!actions_created) { contextMenu->addSeparator(); - contextMenu->addAction(thirdPartyTxUrlAction); - connect(thirdPartyTxUrlAction, &QAction::triggered, [this, url] { openThirdPartyTxUrl(url); }); + actions_created = true; + } + /*: Transactions table context menu action to show the + selected transaction in a third-party block explorer. + %1 is a stand-in argument for the URL of the explorer. */ + contextMenu->addAction(tr("Show in %1").arg(host), [this, url] { openThirdPartyTxUrl(url); }); } } } @@ -500,22 +504,22 @@ void TransactionView::editLabel() // Determine type of address, launch appropriate editor dialog type QString type = modelIdx.data(AddressTableModel::TypeRole).toString(); - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( type == AddressTableModel::Receive ? EditAddressDialog::EditReceivingAddress : EditAddressDialog::EditSendingAddress, this); - dlg.setModel(addressBook); - dlg.loadRow(idx); - dlg.exec(); + dlg->setModel(addressBook); + dlg->loadRow(idx); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } else { // Add sending address - EditAddressDialog dlg(EditAddressDialog::NewSendingAddress, + auto dlg = new EditAddressDialog(EditAddressDialog::NewSendingAddress, this); - dlg.setModel(addressBook); - dlg.setAddress(address); - dlg.exec(); + dlg->setModel(addressBook); + dlg->setAddress(address); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } } diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 3cceb5ca5a..4c74bcd480 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -41,10 +41,6 @@ WalletController::WalletController(ClientModel& client_model, const PlatformStyl getOrCreateWallet(std::move(wallet)); }); - for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.walletClient().getWallets()) { - getOrCreateWallet(std::move(wallet)); - } - m_activity_worker->moveToThread(m_activity_thread); m_activity_thread->start(); QTimer::singleShot(0, m_activity_worker, []() { @@ -61,12 +57,6 @@ WalletController::~WalletController() delete m_activity_worker; } -std::vector<WalletModel*> WalletController::getOpenWallets() const -{ - QMutexLocker locker(&m_mutex); - return m_wallets; -} - std::map<std::string, bool> WalletController::listWalletDir() const { QMutexLocker locker(&m_mutex); @@ -191,33 +181,23 @@ WalletControllerActivity::WalletControllerActivity(WalletController* wallet_cont , m_wallet_controller(wallet_controller) , m_parent_widget(parent_widget) { -} - -WalletControllerActivity::~WalletControllerActivity() -{ - delete m_progress_dialog; + connect(this, &WalletControllerActivity::finished, this, &QObject::deleteLater); } void WalletControllerActivity::showProgressDialog(const QString& label_text) { - assert(!m_progress_dialog); - m_progress_dialog = new QProgressDialog(m_parent_widget); - - m_progress_dialog->setLabelText(label_text); - m_progress_dialog->setRange(0, 0); - m_progress_dialog->setCancelButton(nullptr); - m_progress_dialog->setWindowModality(Qt::ApplicationModal); - GUIUtil::PolishProgressDialog(m_progress_dialog); + auto progress_dialog = new QProgressDialog(m_parent_widget); + progress_dialog->setAttribute(Qt::WA_DeleteOnClose); + connect(this, &WalletControllerActivity::finished, progress_dialog, &QWidget::close); + + progress_dialog->setLabelText(label_text); + progress_dialog->setRange(0, 0); + progress_dialog->setCancelButton(nullptr); + progress_dialog->setWindowModality(Qt::ApplicationModal); + GUIUtil::PolishProgressDialog(progress_dialog); // The setValue call forces QProgressDialog to start the internal duration estimation. // See details in https://bugreports.qt.io/browse/QTBUG-47042. - m_progress_dialog->setValue(0); -} - -void WalletControllerActivity::destroyProgressDialog() -{ - assert(m_progress_dialog); - delete m_progress_dialog; - m_progress_dialog = nullptr; + progress_dialog->setValue(0); } CreateWalletActivity::CreateWalletActivity(WalletController* wallet_controller, QWidget* parent_widget) @@ -279,8 +259,6 @@ void CreateWalletActivity::createWallet() void CreateWalletActivity::finish() { - destroyProgressDialog(); - if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message.translated)); } else if (!m_warning_message.empty()) { @@ -329,8 +307,6 @@ OpenWalletActivity::OpenWalletActivity(WalletController* wallet_controller, QWid void OpenWalletActivity::finish() { - destroyProgressDialog(); - if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message.translated)); } else if (!m_warning_message.empty()) { @@ -356,3 +332,21 @@ void OpenWalletActivity::open(const std::string& path) QTimer::singleShot(0, this, &OpenWalletActivity::finish); }); } + +LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget) + : WalletControllerActivity(wallet_controller, parent_widget) +{ +} + +void LoadWalletsActivity::load() +{ + showProgressDialog(tr("Loading wallets…")); + + QTimer::singleShot(0, worker(), [this] { + for (auto& wallet : node().walletClient().getWallets()) { + m_wallet_controller->getOrCreateWallet(std::move(wallet)); + } + + QTimer::singleShot(0, this, [this] { Q_EMIT finished(); }); + }); +} diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index f7e366878d..f97a7a1e84 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -52,9 +52,6 @@ public: WalletController(ClientModel& client_model, const PlatformStyle* platform_style, QObject* parent); ~WalletController(); - //! Returns wallet models currently open. - std::vector<WalletModel*> getOpenWallets() const; - WalletModel* getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet); //! Returns all wallet names in the wallet dir mapped to whether the wallet @@ -90,7 +87,7 @@ class WalletControllerActivity : public QObject public: WalletControllerActivity(WalletController* wallet_controller, QWidget* parent_widget); - virtual ~WalletControllerActivity(); + virtual ~WalletControllerActivity() = default; Q_SIGNALS: void finished(); @@ -100,11 +97,9 @@ protected: QObject* worker() const { return m_wallet_controller->m_activity_worker; } void showProgressDialog(const QString& label_text); - void destroyProgressDialog(); WalletController* const m_wallet_controller; QWidget* const m_parent_widget; - QProgressDialog* m_progress_dialog{nullptr}; WalletModel* m_wallet_model{nullptr}; bilingual_str m_error_message; std::vector<bilingual_str> m_warning_message; @@ -150,4 +145,14 @@ private: void finish(); }; +class LoadWalletsActivity : public WalletControllerActivity +{ + Q_OBJECT + +public: + LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget); + + void load(); +}; + #endif // BITCOIN_QT_WALLETCONTROLLER_H diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 5eeb2d5308..4ff92bf82c 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -221,10 +221,9 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) return; } - PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); + auto dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); dlg->openWithPSBT(psbtx); - dlg->setAttribute(Qt::WA_DeleteOnClose); - dlg->exec(); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletFrame::encryptWallet() diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 967dd588b4..052453cf65 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -506,9 +506,10 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy.")); } - SendConfirmationDialog confirmationDialog(tr("Confirm fee bump"), questionString); - confirmationDialog.exec(); - QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); + auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString); + confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); + // TODO: Replace QDialog::exec() with safer QDialog::show(). + const auto retval = static_cast<QMessageBox::StandardButton>(confirmationDialog->exec()); // cancel sign&broadcast if user doesn't want to bump the fee if (retval != QMessageBox::Yes) { diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 309806a1c4..7813b89e41 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -205,11 +205,10 @@ void WalletView::showOutOfSyncWarning(bool fShow) void WalletView::encryptWallet() { - AskPassphraseDialog dlg(AskPassphraseDialog::Encrypt, this); - dlg.setModel(walletModel); - dlg.exec(); - - Q_EMIT encryptionStatusChanged(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Encrypt, this); + dlg->setModel(walletModel); + connect(dlg, &QDialog::finished, this, &WalletView::encryptionStatusChanged); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::backupWallet() @@ -234,19 +233,18 @@ void WalletView::backupWallet() void WalletView::changePassphrase() { - AskPassphraseDialog dlg(AskPassphraseDialog::ChangePass, this); - dlg.setModel(walletModel); - dlg.exec(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::ChangePass, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::unlockWallet() { // Unlock wallet when requested by wallet model - if (walletModel->getEncryptionStatus() == WalletModel::Locked) - { - AskPassphraseDialog dlg(AskPassphraseDialog::Unlock, this); - dlg.setModel(walletModel); - dlg.exec(); + if (walletModel->getEncryptionStatus() == WalletModel::Locked) { + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Unlock, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } diff --git a/src/randomenv.cpp b/src/randomenv.cpp index fa2a3a0607..bf23ea4a12 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -53,7 +53,7 @@ #include <sys/vmmeter.h> #endif #endif -#if defined(HAVE_STRONG_GETAUXVAL) || defined(HAVE_WEAK_GETAUXVAL) +#if defined(HAVE_STRONG_GETAUXVAL) #include <sys/auxv.h> #endif @@ -326,7 +326,7 @@ void RandAddStaticEnv(CSHA512& hasher) // Bitcoin client version hasher << CLIENT_VERSION; -#if defined(HAVE_STRONG_GETAUXVAL) || defined(HAVE_WEAK_GETAUXVAL) +#if defined(HAVE_STRONG_GETAUXVAL) // Information available through getauxval() # ifdef AT_HWCAP hasher << getauxval(AT_HWCAP); @@ -346,7 +346,7 @@ void RandAddStaticEnv(CSHA512& hasher) const char* exec_str = (const char*)getauxval(AT_EXECFN); if (exec_str) hasher.Write((const unsigned char*)exec_str, strlen(exec_str) + 1); # endif -#endif // HAVE_STRONG_GETAUXVAL || HAVE_WEAK_GETAUXVAL +#endif // HAVE_STRONG_GETAUXVAL #ifdef HAVE_GETCPUID AddAllCPUID(hasher); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 3ee65922f9..3370afc75f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1256,11 +1256,8 @@ static RPCHelpMan gettxout() {RPCResult::Type::OBJ, "scriptPubKey", "", { {RPCResult::Type::STR, "asm", ""}, {RPCResult::Type::STR_HEX, "hex", ""}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR, "type", "The type, eg pubkeyhash"}, - {RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses", - {{RPCResult::Type::STR, "address", "bitcoin address"}}}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, }}, {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, }}, @@ -1933,16 +1930,6 @@ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], } } -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex) -{ - ScriptPubKeyToUniv(scriptPubKey, out, fIncludeHex, IsDeprecatedRPCEnabled("addresses")); -} - -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo) -{ - TxToUniv(tx, hashBlock, IsDeprecatedRPCEnabled("addresses"), entry, include_hex, serialize_flags, txundo); -} - template<typename T> static inline bool SetHasKeys(const std::set<T>& set) {return false;} template<typename T, typename Tk, typename... Args> diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index ffb6f03b47..4b0d855685 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -6,7 +6,6 @@ #define BITCOIN_RPC_BLOCKCHAIN_H #include <amount.h> -#include <core_io.h> #include <streams.h> #include <sync.h> @@ -53,9 +52,6 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex /** Used by getblockstats to get feerates at different percentiles by weight */ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_weight); -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr); - NodeContext& EnsureAnyNodeContext(const std::any& context); CTxMemPool& EnsureMemPool(const NodeContext& node); CTxMemPool& EnsureAnyMemPool(const std::any& context); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index eb2d5b9c5f..066a60b71b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1103,6 +1103,8 @@ static RPCHelpMan estimatesmartfee() RPCTypeCheckArgument(request.params[0], UniValue::VNUM); CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context); + const NodeContext& node = EnsureAnyNodeContext(request.context); + const CTxMemPool& mempool = EnsureMemPool(node); unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target); @@ -1118,7 +1120,10 @@ static RPCHelpMan estimatesmartfee() UniValue result(UniValue::VOBJ); UniValue errors(UniValue::VARR); FeeCalculation feeCalc; - CFeeRate feeRate = fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative); + CFeeRate feeRate{fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative)}; + CFeeRate min_mempool_feerate{mempool.GetMinFee(gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000)}; + CFeeRate min_relay_feerate{::minRelayTxFee}; + feeRate = std::max({feeRate, min_mempool_feerate, min_relay_feerate}); if (feeRate != CFeeRate(0)) { result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK())); } else { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 737abf6ca9..b32bc670b6 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -131,13 +131,8 @@ static RPCHelpMan getrawtransaction() { {RPCResult::Type::STR, "asm", "the asm"}, {RPCResult::Type::STR, "hex", "the hex"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses", - { - {RPCResult::Type::STR, "address", "bitcoin address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, }}, }}, }}, @@ -495,13 +490,8 @@ static RPCHelpMan decoderawtransaction() { {RPCResult::Type::STR, "asm", "the asm"}, {RPCResult::Type::STR_HEX, "hex", "the hex"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses", - { - {RPCResult::Type::STR, "address", "bitcoin address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, }}, }}, }}, @@ -554,24 +544,14 @@ static RPCHelpMan decodescript() { {RPCResult::Type::STR, "asm", "Script public key"}, {RPCResult::Type::STR, "type", "The output type (e.g. "+GetAllOutputTypes()+")"}, - {RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses", - { - {RPCResult::Type::STR, "address", "bitcoin address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, {RPCResult::Type::STR, "p2sh", /* optional */ true, "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, {RPCResult::Type::OBJ, "segwit", /* optional */ true, "Result of a witness script public key wrapping this redeem script (not returned if the script is a P2SH or witness)", { {RPCResult::Type::STR, "asm", "String representation of the script public key"}, {RPCResult::Type::STR_HEX, "hex", "Hex string of the script public key"}, {RPCResult::Type::STR, "type", "The type of the script public key (e.g. witness_v0_keyhash or witness_v0_scripthash)"}, - {RPCResult::Type::STR, "address", /* optional */ true, "bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of bitcoin addresses", - { - {RPCResult::Type::STR, "address", "segwit address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, {RPCResult::Type::STR, "p2sh-segwit", "address of the P2SH script wrapping this witness redeem script"}, }}, } @@ -592,7 +572,7 @@ static RPCHelpMan decodescript() } else { // Empty scripts are valid } - ScriptPubKeyToUniv(script, r, /* fIncludeHex */ false); + ScriptPubKeyToUniv(script, r, /* include_hex */ false); UniValue type; type = find_value(r, "type"); @@ -626,7 +606,7 @@ static RPCHelpMan decodescript() // Newer segwit program versions should be considered when then become available. segwitScr = GetScriptForDestination(WitnessV0ScriptHash(script)); } - ScriptPubKeyToUniv(segwitScr, sr, /* fIncludeHex */ true); + ScriptPubKeyToUniv(segwitScr, sr, /* include_hex */ true); sr.pushKV("p2sh-segwit", EncodeDestination(ScriptHash(segwitScr))); r.pushKV("segwit", sr); } @@ -1061,7 +1041,7 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /*optional=*/true, "Bitcoin address if there is one"}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, }}, }}, {RPCResult::Type::OBJ_DYN, "partial_signatures", /* optional */ true, "", @@ -1181,7 +1161,7 @@ static RPCHelpMan decodepsbt() txout = input.witness_utxo; UniValue o(UniValue::VOBJ); - ScriptToUniv(txout.scriptPubKey, o, true); + ScriptPubKeyToUniv(txout.scriptPubKey, o, /* include_hex */ true); UniValue out(UniValue::VOBJ); out.pushKV("amount", ValueFromAmount(txout.nValue)); @@ -1228,12 +1208,12 @@ static RPCHelpMan decodepsbt() // Redeem script and witness script if (!input.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(input.redeem_script, r, false); + ScriptToUniv(input.redeem_script, r); in.pushKV("redeem_script", r); } if (!input.witness_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(input.witness_script, r, false); + ScriptToUniv(input.witness_script, r); in.pushKV("witness_script", r); } @@ -1288,12 +1268,12 @@ static RPCHelpMan decodepsbt() // Redeem script and witness script if (!output.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(output.redeem_script, r, false); + ScriptToUniv(output.redeem_script, r); out.pushKV("redeem_script", r); } if (!output.witness_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(output.witness_script, r, false); + ScriptToUniv(output.witness_script, r); out.pushKV("witness_script", r); } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 67a79a157c..d9656c781d 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -266,47 +266,6 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) assert(false); } -// TODO: from v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed -bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet) -{ - addressRet.clear(); - std::vector<valtype> vSolutions; - typeRet = Solver(scriptPubKey, vSolutions); - if (typeRet == TxoutType::NONSTANDARD) { - return false; - } else if (typeRet == TxoutType::NULL_DATA) { - // This is data, not addresses - return false; - } - - if (typeRet == TxoutType::MULTISIG) - { - nRequiredRet = vSolutions.front()[0]; - for (unsigned int i = 1; i < vSolutions.size()-1; i++) - { - CPubKey pubKey(vSolutions[i]); - if (!pubKey.IsValid()) - continue; - - CTxDestination address = PKHash(pubKey); - addressRet.push_back(address); - } - - if (addressRet.empty()) - return false; - } - else - { - nRequiredRet = 1; - CTxDestination address; - if (!ExtractDestination(scriptPubKey, address)) - return false; - addressRet.push_back(address); - } - - return true; -} - namespace { class CScriptVisitor { diff --git a/src/script/standard.h b/src/script/standard.h index 78492733db..a8e57231bf 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -176,28 +176,12 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector<std::vector<unsigned c /** * Parse a standard scriptPubKey for the destination address. Assigns result to - * the addressRet parameter and returns true if successful. For multisig - * scripts, instead use ExtractDestinations. Currently only works for P2PK, + * the addressRet parameter and returns true if successful. Currently only works for P2PK, * P2PKH, P2SH, P2WPKH, and P2WSH scripts. */ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet); /** - * Parse a standard scriptPubKey with one or more destination addresses. For - * multisig scripts, this populates the addressRet vector with the pubkey IDs - * and nRequiredRet with the n required to spend. For other destinations, - * addressRet is populated with a single value and nRequiredRet is set to 1. - * Returns true if successful. - * - * Note: this function confuses destinations (a subset of CScripts that are - * encodable as an address) with key identifiers (of keys involved in a - * CScript), and its use should be phased out. - * - * TODO: from v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed - */ -bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector<CTxDestination>& addressRet, int& nRequiredRet); - -/** * Generate a Bitcoin scriptPubKey for the given CTxDestination. Returns a P2PKH * script for a CKeyID destination, a P2SH script for a CScriptID, and an empty * script for CNoDestination. diff --git a/src/sync.cpp b/src/sync.cpp index 98e6d3d65d..c9fd8e347e 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -97,27 +97,29 @@ static void potential_deadlock_detected(const LockPair& mismatch, const LockStac LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); LogPrintf("Previous lock order was:\n"); for (const LockStackItem& i : s1) { + std::string prefix{}; if (i.first == mismatch.first) { - LogPrintf(" (1)"); /* Continued */ + prefix = " (1)"; } if (i.first == mismatch.second) { - LogPrintf(" (2)"); /* Continued */ + prefix = " (2)"; } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } std::string mutex_a, mutex_b; LogPrintf("Current lock order is:\n"); for (const LockStackItem& i : s2) { + std::string prefix{}; if (i.first == mismatch.first) { - LogPrintf(" (1)"); /* Continued */ + prefix = " (1)"; mutex_a = i.second.Name(); } if (i.first == mismatch.second) { - LogPrintf(" (2)"); /* Continued */ + prefix = " (2)"; mutex_b = i.second.Name(); } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } if (g_debug_lockorder_abort) { tfm::format(std::cerr, "Assertion failed: detected inconsistent lock order for %s, details in debug log.\n", s2.back().second.ToString()); @@ -131,10 +133,11 @@ static void double_lock_detected(const void* mutex, const LockStack& lock_stack) LogPrintf("DOUBLE LOCK DETECTED\n"); LogPrintf("Lock order:\n"); for (const LockStackItem& i : lock_stack) { + std::string prefix{}; if (i.first == mutex) { - LogPrintf(" (*)"); /* Continued */ + prefix = " (*)"; } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } if (g_debug_lockorder_abort) { tfm::format(std::cerr, diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 5a98558240..23ef2062ef 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_key) CBloomFilter filter(2, 0.001, 0, BLOOM_UPDATE_ALL); filter.insert(vchPubKey); uint160 hash = pubkey.GetID(); - filter.insert(std::vector<unsigned char>(hash.begin(), hash.end())); + filter.insert(hash); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << filter; diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index 950ee45d1d..74c576322a 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -56,46 +56,8 @@ FUZZ_TARGET_INIT(script, initialize_script) assert(script == decompressed_script); } - CTxDestination address; - TxoutType type_ret; - std::vector<CTxDestination> addresses; - int required_ret; - bool extract_destinations_ret = ExtractDestinations(script, type_ret, addresses, required_ret); - bool extract_destination_ret = ExtractDestination(script, address); - if (!extract_destinations_ret) { - assert(!extract_destination_ret); - if (type_ret == TxoutType::MULTISIG) { - assert(addresses.empty() && required_ret == 0); - } else { - assert(type_ret == TxoutType::PUBKEY || - type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA); - } - } else { - assert(required_ret >= 1 && required_ret <= 16); - assert((unsigned long)required_ret == addresses.size()); - assert(type_ret == TxoutType::MULTISIG || required_ret == 1); - } - if (type_ret == TxoutType::NONSTANDARD || type_ret == TxoutType::NULL_DATA) { - assert(!extract_destinations_ret); - } - if (!extract_destination_ret) { - assert(type_ret == TxoutType::PUBKEY || - type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA || - type_ret == TxoutType::MULTISIG); - } else { - assert(address == addresses[0]); - } - if (type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA || - type_ret == TxoutType::MULTISIG) { - assert(!extract_destination_ret); - } - TxoutType which_type; bool is_standard_ret = IsStandard(script, which_type); - assert(type_ret == which_type); if (!is_standard_ret) { assert(which_type == TxoutType::NONSTANDARD || which_type == TxoutType::NULL_DATA || @@ -112,6 +74,20 @@ FUZZ_TARGET_INIT(script, initialize_script) which_type == TxoutType::NONSTANDARD); } + CTxDestination address; + bool extract_destination_ret = ExtractDestination(script, address); + if (!extract_destination_ret) { + assert(which_type == TxoutType::PUBKEY || + which_type == TxoutType::NONSTANDARD || + which_type == TxoutType::NULL_DATA || + which_type == TxoutType::MULTISIG); + } + if (which_type == TxoutType::NONSTANDARD || + which_type == TxoutType::NULL_DATA || + which_type == TxoutType::MULTISIG) { + assert(!extract_destination_ret); + } + const FlatSigningProvider signing_provider; (void)InferDescriptor(script, signing_provider); (void)IsSegWitOutput(signing_provider, script); @@ -133,15 +109,11 @@ FUZZ_TARGET_INIT(script, initialize_script) (void)ScriptToAsmStr(script, true); UniValue o1(UniValue::VOBJ); - ScriptPubKeyToUniv(script, o1, true, true); - ScriptPubKeyToUniv(script, o1, true, false); + ScriptPubKeyToUniv(script, o1, true); UniValue o2(UniValue::VOBJ); - ScriptPubKeyToUniv(script, o2, false, true); - ScriptPubKeyToUniv(script, o2, false, false); + ScriptPubKeyToUniv(script, o2, false); UniValue o3(UniValue::VOBJ); - ScriptToUniv(script, o3, true); - UniValue o4(UniValue::VOBJ); - ScriptToUniv(script, o4, false); + ScriptToUniv(script, o3); { const std::vector<uint8_t> bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 0c1b45b86c..dc2bf7c860 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -31,9 +31,99 @@ #include <version.h> #include <cstdint> +#include <cstdlib> #include <string> #include <vector> +namespace { +bool LegacyParsePrechecks(const std::string& str) +{ + if (str.empty()) // No empty string allowed + return false; + if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size() - 1]))) // No padding allowed + return false; + if (!ValidAsCString(str)) // No embedded NUL characters allowed + return false; + return true; +} + +bool LegacyParseInt32(const std::string& str, int32_t* out) +{ + if (!LegacyParsePrechecks(str)) + return false; + char* endp = nullptr; + errno = 0; // strtol will not set errno if valid + long int n = strtol(str.c_str(), &endp, 10); + if (out) *out = (int32_t)n; + // Note that strtol returns a *long int*, so even if strtol doesn't report an over/underflow + // we still have to check that the returned value is within the range of an *int32_t*. On 64-bit + // platforms the size of these types may be different. + return endp && *endp == 0 && !errno && + n >= std::numeric_limits<int32_t>::min() && + n <= std::numeric_limits<int32_t>::max(); +} + +bool LegacyParseInt64(const std::string& str, int64_t* out) +{ + if (!LegacyParsePrechecks(str)) + return false; + char* endp = nullptr; + errno = 0; // strtoll will not set errno if valid + long long int n = strtoll(str.c_str(), &endp, 10); + if (out) *out = (int64_t)n; + // Note that strtoll returns a *long long int*, so even if strtol doesn't report an over/underflow + // we still have to check that the returned value is within the range of an *int64_t*. + return endp && *endp == 0 && !errno && + n >= std::numeric_limits<int64_t>::min() && + n <= std::numeric_limits<int64_t>::max(); +} + +bool LegacyParseUInt32(const std::string& str, uint32_t* out) +{ + if (!LegacyParsePrechecks(str)) + return false; + if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoul accepts these by default if they fit in the range + return false; + char* endp = nullptr; + errno = 0; // strtoul will not set errno if valid + unsigned long int n = strtoul(str.c_str(), &endp, 10); + if (out) *out = (uint32_t)n; + // Note that strtoul returns a *unsigned long int*, so even if it doesn't report an over/underflow + // we still have to check that the returned value is within the range of an *uint32_t*. On 64-bit + // platforms the size of these types may be different. + return endp && *endp == 0 && !errno && + n <= std::numeric_limits<uint32_t>::max(); +} + +bool LegacyParseUInt8(const std::string& str, uint8_t* out) +{ + uint32_t u32; + if (!LegacyParseUInt32(str, &u32) || u32 > std::numeric_limits<uint8_t>::max()) { + return false; + } + if (out != nullptr) { + *out = static_cast<uint8_t>(u32); + } + return true; +} + +bool LegacyParseUInt64(const std::string& str, uint64_t* out) +{ + if (!LegacyParsePrechecks(str)) + return false; + if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoull accepts these by default if they fit in the range + return false; + char* endp = nullptr; + errno = 0; // strtoull will not set errno if valid + unsigned long long int n = strtoull(str.c_str(), &endp, 10); + if (out) *out = (uint64_t)n; + // Note that strtoull returns a *unsigned long long int*, so even if it doesn't report an over/underflow + // we still have to check that the returned value is within the range of an *uint64_t*. + return endp && *endp == 0 && !errno && + n <= std::numeric_limits<uint64_t>::max(); +} +}; // namespace + FUZZ_TARGET(string) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); @@ -133,4 +223,49 @@ FUZZ_TARGET(string) const bilingual_str bs2{random_string_2, random_string_1}; (void)(bs1 + bs2); } + { + int32_t i32; + int64_t i64; + uint32_t u32; + uint64_t u64; + uint8_t u8; + const bool ok_i32 = ParseInt32(random_string_1, &i32); + const bool ok_i64 = ParseInt64(random_string_1, &i64); + const bool ok_u32 = ParseUInt32(random_string_1, &u32); + const bool ok_u64 = ParseUInt64(random_string_1, &u64); + const bool ok_u8 = ParseUInt8(random_string_1, &u8); + + int32_t i32_legacy; + int64_t i64_legacy; + uint32_t u32_legacy; + uint64_t u64_legacy; + uint8_t u8_legacy; + const bool ok_i32_legacy = LegacyParseInt32(random_string_1, &i32_legacy); + const bool ok_i64_legacy = LegacyParseInt64(random_string_1, &i64_legacy); + const bool ok_u32_legacy = LegacyParseUInt32(random_string_1, &u32_legacy); + const bool ok_u64_legacy = LegacyParseUInt64(random_string_1, &u64_legacy); + const bool ok_u8_legacy = LegacyParseUInt8(random_string_1, &u8_legacy); + + assert(ok_i32 == ok_i32_legacy); + assert(ok_i64 == ok_i64_legacy); + assert(ok_u32 == ok_u32_legacy); + assert(ok_u64 == ok_u64_legacy); + assert(ok_u8 == ok_u8_legacy); + + if (ok_i32) { + assert(i32 == i32_legacy); + } + if (ok_i64) { + assert(i64 == i64_legacy); + } + if (ok_u32) { + assert(u32 == u32_legacy); + } + if (ok_u64) { + assert(u64 == u64_legacy); + } + if (ok_u8) { + assert(u8 == u8_legacy); + } + } } diff --git a/src/test/fuzz/system.cpp b/src/test/fuzz/system.cpp index 00403c1a32..dc3f9c8b8f 100644 --- a/src/test/fuzz/system.cpp +++ b/src/test/fuzz/system.cpp @@ -5,6 +5,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <test/util/setup_common.h> #include <util/system.h> #include <cstdint> @@ -12,6 +13,11 @@ #include <vector> namespace { +void initialize_system() +{ + static const auto testing_setup = MakeNoLogFileContext<>(); +} + std::string GetArgumentName(const std::string& name) { size_t idx = name.find('='); @@ -20,9 +26,8 @@ std::string GetArgumentName(const std::string& name) } return name.substr(0, idx); } -} // namespace -FUZZ_TARGET(system) +FUZZ_TARGET_INIT(system, initialize_system) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); ArgsManager args_manager{}; @@ -114,3 +119,4 @@ FUZZ_TARGET(system) (void)HelpRequested(args_manager); } +} // namespace diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index ff34cc87b2..a21e5cea0c 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -103,6 +103,6 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) (void)IsWitnessStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); - TxToUniv(tx, /* hashBlock */ uint256::ZERO, /* include_addresses */ true, u); - TxToUniv(tx, /* hashBlock */ uint256::ONE, /* include_addresses */ false, u); + TxToUniv(tx, /* hashBlock */ uint256::ZERO, u); + TxToUniv(tx, /* hashBlock */ uint256::ONE, u); } diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index a01d3fa03a..bf8ff5f5e2 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -252,67 +252,6 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) BOOST_CHECK(std::get<WitnessUnknown>(address) == unk); } -BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) -{ - CKey keys[3]; - CPubKey pubkeys[3]; - for (int i = 0; i < 3; i++) { - keys[i].MakeNewKey(true); - pubkeys[i] = keys[i].GetPubKey(); - } - - CScript s; - TxoutType whichType; - std::vector<CTxDestination> addresses; - int nRequired; - - // TxoutType::PUBKEY - s.clear(); - s << ToByteVector(pubkeys[0]) << OP_CHECKSIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEY); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get<PKHash>(addresses[0]) == PKHash(pubkeys[0])); - - // TxoutType::PUBKEYHASH - s.clear(); - s << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEYHASH); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get<PKHash>(addresses[0]) == PKHash(pubkeys[0])); - - // TxoutType::SCRIPTHASH - CScript redeemScript(s); // initialize with leftover P2PKH script - s.clear(); - s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::SCRIPTHASH); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get<ScriptHash>(addresses[0]) == ScriptHash(redeemScript)); - - // TxoutType::MULTISIG - s.clear(); - s << OP_2 << - ToByteVector(pubkeys[0]) << - ToByteVector(pubkeys[1]) << - OP_2 << OP_CHECKMULTISIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::MULTISIG); - BOOST_CHECK_EQUAL(addresses.size(), 2U); - BOOST_CHECK_EQUAL(nRequired, 2); - BOOST_CHECK(std::get<PKHash>(addresses[0]) == PKHash(pubkeys[0])); - BOOST_CHECK(std::get<PKHash>(addresses[1]) == PKHash(pubkeys[1])); - - // TxoutType::NULL_DATA - s.clear(); - s << OP_RETURN << std::vector<unsigned char>({75}); - BOOST_CHECK(!ExtractDestinations(s, whichType, addresses, nRequired)); -} - BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_) { CKey keys[3]; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a5c9d2ef6f..a13700d733 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1474,6 +1474,81 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32) BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr)); } +BOOST_AUTO_TEST_CASE(test_ToIntegral) +{ + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("1234").value(), 1'234); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("0").value(), 0); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("01234").value(), 1'234); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("00000000000000001234").value(), 1'234); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-00000000000000001234").value(), -1'234); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("00000000000000000000").value(), 0); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-00000000000000000000").value(), 0); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1234").value(), -1'234); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-1").value(), -1); + + BOOST_CHECK(!ToIntegral<int32_t>(" 1")); + BOOST_CHECK(!ToIntegral<int32_t>("1 ")); + BOOST_CHECK(!ToIntegral<int32_t>("1a")); + BOOST_CHECK(!ToIntegral<int32_t>("1.1")); + BOOST_CHECK(!ToIntegral<int32_t>("1.9")); + BOOST_CHECK(!ToIntegral<int32_t>("+01.9")); + BOOST_CHECK(!ToIntegral<int32_t>(" -1")); + BOOST_CHECK(!ToIntegral<int32_t>("-1 ")); + BOOST_CHECK(!ToIntegral<int32_t>(" -1 ")); + BOOST_CHECK(!ToIntegral<int32_t>("+1")); + BOOST_CHECK(!ToIntegral<int32_t>(" +1")); + BOOST_CHECK(!ToIntegral<int32_t>(" +1 ")); + BOOST_CHECK(!ToIntegral<int32_t>("+-1")); + BOOST_CHECK(!ToIntegral<int32_t>("-+1")); + BOOST_CHECK(!ToIntegral<int32_t>("++1")); + BOOST_CHECK(!ToIntegral<int32_t>("--1")); + BOOST_CHECK(!ToIntegral<int32_t>("")); + BOOST_CHECK(!ToIntegral<int32_t>("aap")); + BOOST_CHECK(!ToIntegral<int32_t>("0x1")); + BOOST_CHECK(!ToIntegral<int32_t>("-32482348723847471234")); + BOOST_CHECK(!ToIntegral<int32_t>("32482348723847471234")); + + BOOST_CHECK(!ToIntegral<int64_t>("-9223372036854775809")); + BOOST_CHECK_EQUAL(ToIntegral<int64_t>("-9223372036854775808").value(), -9'223'372'036'854'775'807LL - 1LL); + BOOST_CHECK_EQUAL(ToIntegral<int64_t>("9223372036854775807").value(), 9'223'372'036'854'775'807); + BOOST_CHECK(!ToIntegral<int64_t>("9223372036854775808")); + + BOOST_CHECK(!ToIntegral<uint64_t>("-1")); + BOOST_CHECK_EQUAL(ToIntegral<uint64_t>("0").value(), 0U); + BOOST_CHECK_EQUAL(ToIntegral<uint64_t>("18446744073709551615").value(), 18'446'744'073'709'551'615ULL); + BOOST_CHECK(!ToIntegral<uint64_t>("18446744073709551616")); + + BOOST_CHECK(!ToIntegral<int32_t>("-2147483649")); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("-2147483648").value(), -2'147'483'648LL); + BOOST_CHECK_EQUAL(ToIntegral<int32_t>("2147483647").value(), 2'147'483'647); + BOOST_CHECK(!ToIntegral<int32_t>("2147483648")); + + BOOST_CHECK(!ToIntegral<uint32_t>("-1")); + BOOST_CHECK_EQUAL(ToIntegral<uint32_t>("0").value(), 0U); + BOOST_CHECK_EQUAL(ToIntegral<uint32_t>("4294967295").value(), 4'294'967'295U); + BOOST_CHECK(!ToIntegral<uint32_t>("4294967296")); + + BOOST_CHECK(!ToIntegral<int16_t>("-32769")); + BOOST_CHECK_EQUAL(ToIntegral<int16_t>("-32768").value(), -32'768); + BOOST_CHECK_EQUAL(ToIntegral<int16_t>("32767").value(), 32'767); + BOOST_CHECK(!ToIntegral<int16_t>("32768")); + + BOOST_CHECK(!ToIntegral<uint16_t>("-1")); + BOOST_CHECK_EQUAL(ToIntegral<uint16_t>("0").value(), 0U); + BOOST_CHECK_EQUAL(ToIntegral<uint16_t>("65535").value(), 65'535U); + BOOST_CHECK(!ToIntegral<uint16_t>("65536")); + + BOOST_CHECK(!ToIntegral<int8_t>("-129")); + BOOST_CHECK_EQUAL(ToIntegral<int8_t>("-128").value(), -128); + BOOST_CHECK_EQUAL(ToIntegral<int8_t>("127").value(), 127); + BOOST_CHECK(!ToIntegral<int8_t>("128")); + + BOOST_CHECK(!ToIntegral<uint8_t>("-1")); + BOOST_CHECK_EQUAL(ToIntegral<uint8_t>("0").value(), 0U); + BOOST_CHECK_EQUAL(ToIntegral<uint8_t>("255").value(), 255U); + BOOST_CHECK(!ToIntegral<uint8_t>("256")); +} + BOOST_AUTO_TEST_CASE(test_ParseInt64) { int64_t n; diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index f514613f0d..0aa80ea0ae 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -11,8 +11,7 @@ #include <algorithm> #include <cstdlib> #include <cstring> -#include <errno.h> -#include <limits> +#include <optional> static const std::string CHARS_ALPHA_NUM = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; @@ -282,6 +281,32 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid) return std::string((const char*)vchRet.data(), vchRet.size()); } +[[nodiscard]] static bool ParsePrechecks(const std::string&); + +namespace { +template <typename T> +bool ParseIntegral(const std::string& str, T* out) +{ + static_assert(std::is_integral<T>::value); + if (!ParsePrechecks(str)) { + return false; + } + // Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when + // handling leading +/- for backwards compatibility. + if (str.length() >= 2 && str[0] == '+' && str[1] == '-') { + return false; + } + const std::optional<T> opt_int = ToIntegral<T>((!str.empty() && str[0] == '+') ? str.substr(1) : str); + if (!opt_int) { + return false; + } + if (out != nullptr) { + *out = *opt_int; + } + return true; +} +}; // namespace + [[nodiscard]] static bool ParsePrechecks(const std::string& str) { if (str.empty()) // No empty string allowed @@ -293,95 +318,36 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid) return true; } -bool ParseInt32(const std::string& str, int32_t *out) +bool ParseInt32(const std::string& str, int32_t* out) { - if (!ParsePrechecks(str)) - return false; - char *endp = nullptr; - errno = 0; // strtol will not set errno if valid - long int n = strtol(str.c_str(), &endp, 10); - if(out) *out = (int32_t)n; - // Note that strtol returns a *long int*, so even if strtol doesn't report an over/underflow - // we still have to check that the returned value is within the range of an *int32_t*. On 64-bit - // platforms the size of these types may be different. - return endp && *endp == 0 && !errno && - n >= std::numeric_limits<int32_t>::min() && - n <= std::numeric_limits<int32_t>::max(); + return ParseIntegral<int32_t>(str, out); } -bool ParseInt64(const std::string& str, int64_t *out) +bool ParseInt64(const std::string& str, int64_t* out) { - if (!ParsePrechecks(str)) - return false; - char *endp = nullptr; - errno = 0; // strtoll will not set errno if valid - long long int n = strtoll(str.c_str(), &endp, 10); - if(out) *out = (int64_t)n; - // Note that strtoll returns a *long long int*, so even if strtol doesn't report an over/underflow - // we still have to check that the returned value is within the range of an *int64_t*. - return endp && *endp == 0 && !errno && - n >= std::numeric_limits<int64_t>::min() && - n <= std::numeric_limits<int64_t>::max(); + return ParseIntegral<int64_t>(str, out); } -bool ParseUInt8(const std::string& str, uint8_t *out) +bool ParseUInt8(const std::string& str, uint8_t* out) { - uint32_t u32; - if (!ParseUInt32(str, &u32) || u32 > std::numeric_limits<uint8_t>::max()) { - return false; - } - if (out != nullptr) { - *out = static_cast<uint8_t>(u32); - } - return true; + return ParseIntegral<uint8_t>(str, out); } bool ParseUInt16(const std::string& str, uint16_t* out) { - uint32_t u32; - if (!ParseUInt32(str, &u32) || u32 > std::numeric_limits<uint16_t>::max()) { - return false; - } - if (out != nullptr) { - *out = static_cast<uint16_t>(u32); - } - return true; + return ParseIntegral<uint16_t>(str, out); } -bool ParseUInt32(const std::string& str, uint32_t *out) +bool ParseUInt32(const std::string& str, uint32_t* out) { - if (!ParsePrechecks(str)) - return false; - if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoul accepts these by default if they fit in the range - return false; - char *endp = nullptr; - errno = 0; // strtoul will not set errno if valid - unsigned long int n = strtoul(str.c_str(), &endp, 10); - if(out) *out = (uint32_t)n; - // Note that strtoul returns a *unsigned long int*, so even if it doesn't report an over/underflow - // we still have to check that the returned value is within the range of an *uint32_t*. On 64-bit - // platforms the size of these types may be different. - return endp && *endp == 0 && !errno && - n <= std::numeric_limits<uint32_t>::max(); + return ParseIntegral<uint32_t>(str, out); } -bool ParseUInt64(const std::string& str, uint64_t *out) +bool ParseUInt64(const std::string& str, uint64_t* out) { - if (!ParsePrechecks(str)) - return false; - if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoull accepts these by default if they fit in the range - return false; - char *endp = nullptr; - errno = 0; // strtoull will not set errno if valid - unsigned long long int n = strtoull(str.c_str(), &endp, 10); - if(out) *out = (uint64_t)n; - // Note that strtoull returns a *unsigned long long int*, so even if it doesn't report an over/underflow - // we still have to check that the returned value is within the range of an *uint64_t*. - return endp && *endp == 0 && !errno && - n <= std::numeric_limits<uint64_t>::max(); + return ParseIntegral<uint64_t>(str, out); } - bool ParseDouble(const std::string& str, double *out) { if (!ParsePrechecks(str)) diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 26dc0a0ce3..1217572c45 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -12,8 +12,10 @@ #include <attributes.h> #include <span.h> +#include <charconv> #include <cstdint> #include <iterator> +#include <optional> #include <string> #include <vector> @@ -95,6 +97,24 @@ constexpr inline bool IsSpace(char c) noexcept { } /** + * Convert string to integral type T. + * + * @returns std::nullopt if the entire string could not be parsed, or if the + * parsed value is not in the range representable by the type T. + */ +template <typename T> +std::optional<T> ToIntegral(const std::string& str) +{ + static_assert(std::is_integral<T>::value); + T result; + const auto [first_nonmatching, error_condition] = std::from_chars(str.data(), str.data() + str.size(), result); + if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) { + return std::nullopt; + } + return {result}; +} + +/** * Convert string to signed 32-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index d2f30abf23..8d08153501 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -305,13 +305,13 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group } if (LogAcceptCategory(BCLog::SELECTCOINS)) { - LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); /* Continued */ + std::string log_message{"Coin selection best subset: "}; for (unsigned int i = 0; i < applicable_groups.size(); i++) { if (vfBest[i]) { - LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(applicable_groups[i].m_value)); /* Continued */ + log_message += strprintf("%s ", FormatMoney(applicable_groups[i].m_value)); } } - LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + LogPrint(BCLog::SELECTCOINS, "%stotal %s\n", log_message, FormatMoney(nBest)); } } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7abdbb0e55..59a59f9794 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -62,7 +62,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-paytxfee=<amt>", strprintf("Fee rate (in %s/kvB) to add to transactions you send (default: %s)", CURRENCY_UNIT, FormatMoney(CFeeRate{DEFAULT_PAY_TX_FEE}.GetFeePerK())), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-rescan", "Rescan the block chain for missing wallet transactions on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #ifdef ENABLE_EXTERNAL_SIGNER argsman.AddArg("-signer=<cmd>", "External signing tool, see doc/external-signer.md", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 382e8b6116..4d7fb2d38c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1439,7 +1439,7 @@ RPCHelpMan importmulti() "and coins using this key may not appear in the wallet. This error could be " "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " - "and -rescan options).", + "option and rescanblockchain RPC).", GetImportTimestamp(request, now), scannedTime - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } @@ -1744,7 +1744,7 @@ RPCHelpMan importdescriptors() "and coins using this desc may not appear in the wallet. This error could be " "caused by pruning or data corruption (see bitcoind log for details) and could " "be dealt with by downloading and rescanning the relevant blocks (see -reindex " - "and -rescan options).", + "option and rescanblockchain RPC).", GetImportTimestamp(request, now), scanned_time - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); response.push_back(std::move(result)); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c430b1db5c..f6cf8868de 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1762,7 +1762,7 @@ static RPCHelpMan gettransaction() if (verbose) { UniValue decoded(UniValue::VOBJ); - TxToUniv(*wtx.tx, uint256(), pwallet->chain().rpcEnableDeprecated("addresses"), decoded, false); + TxToUniv(*wtx.tx, uint256(), decoded, false); entry.pushKV("decoded", decoded); } @@ -2632,7 +2632,7 @@ static RPCHelpMan loadwallet() return RPCHelpMan{"loadwallet", "\nLoads a wallet from a wallet file or directory." "\nNote that all wallet command-line options used when starting bitcoind will be" - "\napplied to the new wallet (eg -rescan, etc).\n", + "\napplied to the new wallet.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, @@ -3799,7 +3799,6 @@ public: obj.pushKV("embedded", std::move(subobj)); } else if (which_type == TxoutType::MULTISIG) { // Also report some information on multisig scripts (which do not have a corresponding address). - // TODO: abstract out the common functionality between this logic and ExtractDestinations. obj.pushKV("sigsrequired", solutions_data[0][0]); UniValue pubkeys(UniValue::VARR); for (size_t i = 1; i < solutions_data.size() - 1; ++i) { diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index ea045eb6d8..4151099c1f 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -45,7 +45,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v // Call Salvage with fAggressive=true to // get as much data as possible. // Rewrite salvaged data to fresh wallet file - // Set -rescan so any missing transactions will be + // Rescan so any missing transactions will be // found. int64_t now = GetTime(); std::string newFilename = strprintf("%s.%d.bak", filename, now); diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 2e60aca017..815d17967c 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -212,6 +212,10 @@ void SQLiteDatabase::Open() if (ret != SQLITE_OK) { throw std::runtime_error(strprintf("SQLiteDatabase: Failed to open database: %s\n", sqlite3_errstr(ret))); } + ret = sqlite3_extended_result_codes(m_db, 1); + if (ret != SQLITE_OK) { + throw std::runtime_error(strprintf("SQLiteDatabase: Failed to enable extended result codes: %s\n", sqlite3_errstr(ret))); + } } if (sqlite3_db_readonly(m_db, "main") != 0) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5431a38bee..9938380369 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -232,8 +232,8 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) "seconds of key creation, and could contain transactions pertaining to the key. As a result, " "transactions and coins using this key may not appear in the wallet. This error could be caused " "by pruning or data corruption (see bitcoind log for details) and could be dealt with by " - "downloading and rescanning the relevant blocks (see -reindex and -rescan " - "options).\"}},{\"success\":true}]", + "downloading and rescanning the relevant blocks (see -reindex option and rescanblockchain " + "RPC).\"}},{\"success\":true}]", 0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW)); RemoveWallet(context, wallet, /* load_on_start= */ std::nullopt); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7a7ff3ee33..1c18cdb1bc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -884,7 +884,7 @@ bool CWallet::IsSpentKey(const uint256& hash, unsigned int n) const return false; } -CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose) +CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose, bool rescanning_old_block) { LOCK(cs_wallet); @@ -914,7 +914,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio wtx.nTimeReceived = chain().getAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); - wtx.nTimeSmart = ComputeTimeSmart(wtx); + wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block); AddToSpends(hash, &batch); } @@ -1031,7 +1031,7 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx return true; } -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate, bool rescanning_old_block) { const CTransaction& tx = *ptx; { @@ -1069,7 +1069,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again - return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false); + return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false, rescanning_old_block); } } return false; @@ -1198,9 +1198,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx) +void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx, bool rescanning_old_block) { - if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx)) + if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx, rescanning_old_block)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1600,7 +1600,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan started from block %s...\n", start_block.ToString()); fAbortRescan = false; - ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup + ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption) uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash()); uint256 end_hash = tip_hash; if (max_height) chain().findAncestorByHeight(tip_hash, *max_height, FoundBlock().hash(end_hash)); @@ -1643,7 +1643,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { - SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, fUpdate); + SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, fUpdate, /* rescanning_old_block */ true); } // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; @@ -2007,10 +2007,7 @@ DBErrors CWallet::LoadWallet() assert(m_internal_spk_managers.empty()); } - if (nLoadWalletRet != DBErrors::LOAD_OK) - return nLoadWalletRet; - - return DBErrors::LOAD_OK; + return nLoadWalletRet; } DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) @@ -2384,6 +2381,8 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const { * - If sending a transaction, assign its timestamp to the current time. * - If receiving a transaction outside a block, assign its timestamp to the * current time. + * - If receiving a transaction during a rescanning process, assign all its + * (not already known) transactions' timestamps to the block time. * - If receiving a block with a future timestamp, assign all its (not already * known) transactions' timestamps to the current time. * - If receiving a block with a past timestamp, before the most recent known @@ -2398,38 +2397,43 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const { * https://bitcointalk.org/?topic=54527, or * https://github.com/bitcoin/bitcoin/pull/1393. */ -unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const +unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const { unsigned int nTimeSmart = wtx.nTimeReceived; if (!wtx.isUnconfirmed() && !wtx.isAbandoned()) { int64_t blocktime; - if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime))) { - int64_t latestNow = wtx.nTimeReceived; - int64_t latestEntry = 0; - - // Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future - int64_t latestTolerated = latestNow + 300; - const TxItems& txOrdered = wtxOrdered; - for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { - CWalletTx* const pwtx = it->second; - if (pwtx == &wtx) { - continue; - } - int64_t nSmartTime; - nSmartTime = pwtx->nTimeSmart; - if (!nSmartTime) { - nSmartTime = pwtx->nTimeReceived; - } - if (nSmartTime <= latestTolerated) { - latestEntry = nSmartTime; - if (nSmartTime > latestNow) { - latestNow = nSmartTime; + int64_t block_max_time; + if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime).maxTime(block_max_time))) { + if (rescanning_old_block) { + nTimeSmart = block_max_time; + } else { + int64_t latestNow = wtx.nTimeReceived; + int64_t latestEntry = 0; + + // Tolerate times up to the last timestamp in the wallet not more than 5 minutes into the future + int64_t latestTolerated = latestNow + 300; + const TxItems& txOrdered = wtxOrdered; + for (auto it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { + CWalletTx* const pwtx = it->second; + if (pwtx == &wtx) { + continue; + } + int64_t nSmartTime; + nSmartTime = pwtx->nTimeSmart; + if (!nSmartTime) { + nSmartTime = pwtx->nTimeReceived; + } + if (nSmartTime <= latestTolerated) { + latestEntry = nSmartTime; + if (nSmartTime > latestNow) { + latestNow = nSmartTime; + } + break; } - break; } - } - nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); + } } else { WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.m_confirm.hashBlock.ToString()); } @@ -2535,6 +2539,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet); + bool rescan_required = false; DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { @@ -2555,6 +2560,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri { error = strprintf(_("Wallet needed to be rewritten: restart %s to complete"), PACKAGE_NAME); return nullptr; + } else if (nLoadWalletRet == DBErrors::NEED_RESCAN) { + warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect." + " Rescanning wallet."), walletFile)); + rescan_required = true; } else { error = strprintf(_("Error loading %s"), walletFile); @@ -2746,7 +2755,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri LOCK(walletInstance->cs_wallet); - if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { + if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { return nullptr; } @@ -2768,7 +2777,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri return walletInstance; } -bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interfaces::Chain& chain, bilingual_str& error, std::vector<bilingual_str>& warnings) +bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector<bilingual_str>& warnings) { LOCK(walletInstance->cs_wallet); // allow setting the chain if it hasn't been set already but prevent changing it @@ -2785,8 +2794,9 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf // interface. walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance); + // If rescan_required = true, rescan_height remains equal to 0 int rescan_height = 0; - if (!gArgs.GetBoolArg("-rescan", false)) + if (!rescan_required) { WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6b4bcf31c4..cedccf7d44 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -271,8 +271,11 @@ private: * abandoned is an indication that it is not safe to be considered abandoned. * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. + * + * Should be called with rescanning_old_block set to true, if the transaction is + * not discovered in real time, but during a rescan of old blocks. */ - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); @@ -284,7 +287,7 @@ private: /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */ - void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true, bool rescanning_old_block = false) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** WalletFlags set on this wallet. */ std::atomic<uint64_t> m_wallet_flags{0}; @@ -334,7 +337,7 @@ private: * block locator and m_last_block_processed, and registering for * notifications about new blocks and transactions. */ - static bool AttachChain(const std::shared_ptr<CWallet>& wallet, interfaces::Chain& chain, bilingual_str& error, std::vector<bilingual_str>& warnings); + static bool AttachChain(const std::shared_ptr<CWallet>& wallet, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector<bilingual_str>& warnings); public: /** @@ -484,7 +487,7 @@ public: bool EncryptWallet(const SecureString& strWalletPassphrase); void GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; + unsigned int ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old_block) const; /** * Increment the next transaction order id @@ -503,7 +506,7 @@ public: //! @return true if wtx is changed and needs to be saved to disk, otherwise false using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>; - CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true); + CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override; void blockConnected(const CBlock& block, int height) override; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c697534c06..528f77b14c 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -755,6 +755,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; bool fNoncriticalErrors = false; + bool rescan_required = false; DBErrors result = DBErrors::LOAD_OK; LOCK(pwallet->cs_wallet); @@ -823,7 +824,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) fNoncriticalErrors = true; // ... but do warn the user there is something wrong. if (strType == DBKeys::TX) // Rescan if there is a bad transaction record: - gArgs.SoftSetBoolArg("-rescan", true); + rescan_required = true; } } if (!strErr.empty()) @@ -859,8 +860,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) ((DescriptorScriptPubKeyMan*)spk_man)->AddCryptedKey(desc_key_pair.first.second, desc_key_pair.second.first, desc_key_pair.second.second); } - if (fNoncriticalErrors && result == DBErrors::LOAD_OK) + if (rescan_required && result == DBErrors::LOAD_OK) { + result = DBErrors::NEED_RESCAN; + } else if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { result = DBErrors::NONCRITICAL_ERROR; + } // Any wallet corruption at all: skip any rewriting or // upgrading, we don't want to make it worse. diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a549c8039c..715d9012ed 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -48,7 +48,8 @@ enum class DBErrors NONCRITICAL_ERROR, TOO_NEW, LOAD_FAIL, - NEED_REWRITE + NEED_REWRITE, + NEED_RESCAN }; namespace DBKeys { diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 50b6c9d29f..e3cb5cee5d 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -76,6 +76,10 @@ static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::pa } else if (load_wallet_ret == DBErrors::NEED_REWRITE) { tfm::format(std::cerr, "Wallet needed to be rewritten: restart %s to complete", PACKAGE_NAME); return nullptr; + } else if (load_wallet_ret == DBErrors::NEED_RESCAN) { + tfm::format(std::cerr, "Error reading %s! Some transaction data might be missing or" + " incorrect. Wallet requires a rescan.", + name); } else { tfm::format(std::cerr, "Error loading %s", name); return nullptr; |