diff options
66 files changed, 1137 insertions, 371 deletions
diff --git a/configure.ac b/configure.ac index e30159f91b..26092a7b8d 100644 --- a/configure.ac +++ b/configure.ac @@ -1210,8 +1210,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <xmmintrin.h>]], [[ AC_MSG_CHECKING(for strong getauxval support in the system headers) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #include <arm_acle.h> - #include <arm_neon.h> #include <sys/auxv.h> ]], [[ getauxval(AT_HWCAP); @@ -1220,19 +1218,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ [ AC_MSG_RESULT(no); HAVE_STRONG_GETAUXVAL=0 ] ) -AC_MSG_CHECKING(for weak getauxval support in the compiler) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #ifdef __linux__ - unsigned long getauxval(unsigned long type) __attribute__((weak)); - #define AT_HWCAP 16 - #endif - ]], [[ - getauxval(AT_HWCAP); - ]])], - [ AC_MSG_RESULT(yes); HAVE_WEAK_GETAUXVAL=1; AC_DEFINE(HAVE_WEAK_GETAUXVAL, 1, [Define this symbol to build code that uses getauxval (weak linking)]) ], - [ AC_MSG_RESULT(no); HAVE_WEAK_GETAUXVAL=0 ] -) - have_any_system=no AC_MSG_CHECKING([for std::system]) AC_LINK_IFELSE( @@ -1878,7 +1863,6 @@ AC_SUBST(HAVE_O_CLOEXEC) AC_SUBST(HAVE_BUILTIN_PREFETCH) AC_SUBST(HAVE_MM_PREFETCH) AC_SUBST(HAVE_STRONG_GETAUXVAL) -AC_SUBST(HAVE_WEAK_GETAUXVAL) AC_SUBST(ANDROID_ARCH) AC_CONFIG_FILES([Makefile src/Makefile doc/man/Makefile share/setup.nsi share/qt/Info.plist test/config.ini]) AC_CONFIG_FILES([contrib/devtools/split-debug.sh],[chmod +x contrib/devtools/split-debug.sh]) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index ffb6632e21..0a5a7066ab 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -963,37 +963,41 @@ Subtrees Several parts of the repository are subtrees of software maintained elsewhere. -Some of these are maintained by active developers of Bitcoin Core, in which case changes should probably go -directly upstream without being PRed directly against the project. They will be merged back in the next -subtree merge. +Some of these are maintained by active developers of Bitcoin Core, in which case +changes should go directly upstream without being PRed directly against the project. +They will be merged back in the next subtree merge. -Others are external projects without a tight relationship with our project. Changes to these should also -be sent upstream, but bugfixes may also be prudent to PR against Bitcoin Core so that they can be integrated -quickly. Cosmetic changes should be purely taken upstream. +Others are external projects without a tight relationship with our project. Changes +to these should also be sent upstream, but bugfixes may also be prudent to PR against +a Bitcoin Core subtree, so that they can be integrated quickly. Cosmetic changes +should be taken upstream. -There is a tool in `test/lint/git-subtree-check.sh` ([instructions](../test/lint#git-subtree-checksh)) to check a subtree directory for consistency with -its upstream repository. +There is a tool in `test/lint/git-subtree-check.sh` ([instructions](../test/lint#git-subtree-checksh)) +to check a subtree directory for consistency with its upstream repository. Current subtrees include: - src/leveldb - - Upstream at https://github.com/google/leveldb ; Maintained by Google, but - open important PRs to Core to avoid delay. + - Subtree at https://github.com/bitcoin-core/leveldb-subtree ; maintained by Core contributors. + - Upstream at https://github.com/google/leveldb ; maintained by Google. Open + important PRs to the subtree to avoid delay. - **Note**: Follow the instructions in [Upgrading LevelDB](#upgrading-leveldb) when merging upstream changes to the LevelDB subtree. - src/crc32c - Used by leveldb for hardware acceleration of CRC32C checksums for data integrity. - - Upstream at https://github.com/google/crc32c ; Maintained by Google. + - Subtree at https://github.com/bitcoin-core/crc32c-subtree ; maintained by Core contributors. + - Upstream at https://github.com/google/crc32c ; maintained by Google. - src/secp256k1 - - Upstream at https://github.com/bitcoin-core/secp256k1/ ; actively maintained by Core contributors. + - Upstream at https://github.com/bitcoin-core/secp256k1/ ; maintained by Core contributors. - src/crypto/ctaes - - Upstream at https://github.com/bitcoin-core/ctaes ; actively maintained by Core contributors. + - Upstream at https://github.com/bitcoin-core/ctaes ; maintained by Core contributors. - src/univalue - - Upstream at https://github.com/bitcoin-core/univalue ; actively maintained by Core contributors, deviates from upstream https://github.com/jgarzik/univalue + - Subtree at https://github.com/bitcoin-core/univalue-subtree ; maintained by Core contributors. + - Deviates from upstream https://github.com/jgarzik/univalue. Upgrading LevelDB --------------------- diff --git a/doc/fuzzing.md b/doc/fuzzing.md index ee9c65d4d4..0880f9f581 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -254,6 +254,73 @@ $ honggfuzz/honggfuzz --exit_upon_crash --quiet --timeout 4 -n 1 -Q \ -debug ``` +# Fuzzing Bitcoin Core using Eclipser (v1.x) + +## Quickstart guide + +To quickly get started fuzzing Bitcoin Core using [Eclipser v1.x](https://github.com/SoftSec-KAIST/Eclipser/tree/v1.x): + +```sh +$ git clone https://github.com/bitcoin/bitcoin +$ cd bitcoin/ +$ sudo vim /etc/apt/sources.list # Uncomment the lines starting with 'deb-src'. +$ sudo apt-get update +$ sudo apt-get build-dep qemu +$ sudo apt-get install libtool libtool-bin wget automake autoconf bison gdb +``` + +At this point, you must install the .NET core. The process differs, depending on your Linux distribution. +See [this link](https://docs.microsoft.com/en-us/dotnet/core/install/linux) for details. +On ubuntu 20.04, the following should work: + +```sh +$ wget -q https://packages.microsoft.com/config/ubuntu/20.04/packages-microsoft-prod.deb +$ sudo dpkg -i packages-microsoft-prod.deb +$ rm packages-microsoft-prod.deb +$ sudo apt-get update +$ sudo apt-get install -y dotnet-sdk-2.1 +``` + +You will also want to make sure Python is installed as `python` for the Eclipser install to succeed. + +```sh +$ git clone https://github.com/SoftSec-KAIST/Eclipser.git +$ cd Eclipser +$ git checkout v1.x +$ make +$ cd .. +$ ./autogen.sh +$ ./configure --enable-fuzz +$ make +$ mkdir -p outputs/ +$ FUZZ=bech32 dotnet Eclipser/build/Eclipser.dll fuzz -p src/test/fuzz/fuzz -t 36000 -o outputs --src stdin +``` + +This will perform 10 hours of fuzzing. + +To make further use of the inputs generated by Eclipser, you +must first decode them: + +```sh +$ dotnet Eclipser/build/Eclipser.dll decode -i outputs/testcase -o decoded_outputs +``` +This will place raw inputs in the directory `decoded_outputs/decoded_stdins`. Crashes are in the `outputs/crashes` directory, and must +be decoded in the same way. + +Fuzzing with Eclipser will likely be much more effective if using an existing corpus: + +```sh +$ git clone https://github.com/bitcoin-core/qa-assets +$ FUZZ=bech32 dotnet Eclipser/build/Eclipser.dll fuzz -p src/test/fuzz/fuzz -t 36000 -i qa-assets/fuzz_seed_corpus/bech32 outputs --src stdin +``` + +Note that fuzzing with Eclipser on certain targets (those that create 'full nodes', e.g. `process_message*`) will, +for now, slowly fill `/tmp/` with improperly cleaned-up files, which will cause spurious crashes. +See [this proposed patch](https://github.com/bitcoin/bitcoin/pull/22472) for more information. + +Read the [Eclipser documentation for v1.x](https://github.com/SoftSec-KAIST/Eclipser/tree/v1.x) for more details on using Eclipser. + + # OSS-Fuzz Bitcoin Core participates in Google's [OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/bitcoin-core) diff --git a/doc/release-notes-12677.md b/doc/release-notes-12677.md deleted file mode 100644 index d6fea9eae7..0000000000 --- a/doc/release-notes-12677.md +++ /dev/null @@ -1,8 +0,0 @@ -Notable changes -=============== - -Updated RPCs ------------- - -- `listunspent` now includes `ancestorcount`, `ancestorsize`, and -`ancestorfees` for each transaction output that is still in the mempool. diff --git a/doc/release-notes-23065.md b/doc/release-notes-23065.md deleted file mode 100644 index 6ec002b2df..0000000000 --- a/doc/release-notes-23065.md +++ /dev/null @@ -1,15 +0,0 @@ -Notable changes -=============== - -Updated RPCs ------------- - -- `lockunspent` now optionally takes a third parameter, `persistent`, which -causes the lock to be written persistently to the wallet database. This -allows UTXOs to remain locked even after node restarts or crashes. - -GUI changes ------------ - -- UTXOs which are locked via the GUI are now stored persistently in the -wallet database, so are not lost on node shutdown or crash.
\ No newline at end of file diff --git a/doc/release-notes.md b/doc/release-notes.md index a0c1ed3b31..81e79dd3a9 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -61,6 +61,13 @@ P2P and network changes They will become eligible for address gossip after sending an ADDR, ADDRV2, or GETADDR message. (#21528) +Rescan startup parameter removed +-------------------------------- + +The `-rescan` startup parameter has been removed. Wallets which require +rescanning due to corruption will still be rescanned on startup. +Otherwise, please use the `rescanblockchain` RPC to trigger a rescan. (#23123) + Updated RPCs ------------ @@ -70,6 +77,14 @@ Updated RPCs `/rest/block` no longer return the `addresses` and `reqSigs` fields, which were previously deprecated in 22.0. (#22650) +- `listunspent` now includes `ancestorcount`, `ancestorsize`, and + `ancestorfees` for each transaction output that is still in the mempool. + (#12677) + +- `lockunspent` now optionally takes a third parameter, `persistent`, which + causes the lock to be written persistently to the wallet database. This + allows UTXOs to remain locked even after node restarts or crashes. (#23065) + New RPCs -------- @@ -113,6 +128,9 @@ Wallet GUI changes ----------- +- UTXOs which are locked via the GUI are now stored persistently in the + wallet database, so are not lost on node shutdown or crash. (#23065) + Low-level changes ================= diff --git a/share/examples/bitcoin.conf b/share/examples/bitcoin.conf index 4a947001fa..c5b79709c7 100644 --- a/share/examples/bitcoin.conf +++ b/share/examples/bitcoin.conf @@ -157,7 +157,7 @@ #coinstatsindex=1 # Enable pruning to reduce storage requirements by deleting old blocks. -# This mode is incompatible with -txindex, -coinstatsindex and -rescan. +# This mode is incompatible with -txindex and -coinstatsindex. # 0 = default (no pruning). # 1 = allows manual pruning via RPC. # >=550 = target to stay under in MiB. 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/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/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/net_processing.cpp b/src/net_processing.cpp index e130272ff1..008b4d679c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -884,6 +884,12 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) { AssertLockHeld(cs_main); + + // Never request high-bandwidth mode from peers if we're blocks-only. Our + // mempool will not contain the transactions necessary to reconstruct the + // compact block. + if (m_ignore_incoming_txs) return; + CNodeState* nodestate = State(nodeid); if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { // Never ask from peers who can't provide witnesses. @@ -2165,7 +2171,11 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); } if (vGetData.size() > 0) { - if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + if (!m_ignore_incoming_txs && + nodestate->fSupportsDesiredCmpctVersion && + vGetData.size() == 1 && + mapBlocksInFlight.size() == 1 && + pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } 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/guiutil.cpp b/src/qt/guiutil.cpp index 91f2591a31..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> @@ -970,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/optionsdialog.cpp b/src/qt/optionsdialog.cpp index ac50e6518f..0cc2d61df6 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -292,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/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/transactionview.cpp b/src/qt/transactionview.cpp index 1973c9de9a..653f3dda6d 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -504,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/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/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/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/coincontrol.h b/src/wallet/coincontrol.h index 85cbec76b7..c989512d3e 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -9,9 +9,14 @@ #include <policy/feerate.h> #include <policy/fees.h> #include <primitives/transaction.h> +#include <script/keyorigin.h> +#include <script/signingprovider.h> #include <script/standard.h> #include <optional> +#include <algorithm> +#include <map> +#include <set> const int DEFAULT_MIN_DEPTH = 0; const int DEFAULT_MAX_DEPTH = 9999999; @@ -53,6 +58,8 @@ public: int m_min_depth = DEFAULT_MIN_DEPTH; //! Maximum chain depth value for coin availability int m_max_depth = DEFAULT_MAX_DEPTH; + //! SigningProvider that has pubkeys and scripts to do spend size estimation for external inputs + FlatSigningProvider m_external_provider; CCoinControl(); @@ -66,11 +73,32 @@ public: return (setSelected.count(output) > 0); } + bool IsExternalSelected(const COutPoint& output) const + { + return (m_external_txouts.count(output) > 0); + } + + bool GetExternalOutput(const COutPoint& outpoint, CTxOut& txout) const + { + const auto ext_it = m_external_txouts.find(outpoint); + if (ext_it == m_external_txouts.end()) { + return false; + } + txout = ext_it->second; + return true; + } + void Select(const COutPoint& output) { setSelected.insert(output); } + void Select(const COutPoint& outpoint, const CTxOut& txout) + { + setSelected.insert(outpoint); + m_external_txouts.emplace(outpoint, txout); + } + void UnSelect(const COutPoint& output) { setSelected.erase(output); @@ -88,6 +116,7 @@ public: private: std::set<COutPoint> setSelected; + std::map<COutPoint, CTxOut> m_external_txouts; }; #endif // BITCOIN_WALLET_COINCONTROL_H 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/coinselection.h b/src/wallet/coinselection.h index a28bee622e..78d877a10b 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -37,6 +37,18 @@ public: m_input_bytes = input_bytes; } + CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in) + { + outpoint = outpoint_in; + txout = txout_in; + effective_value = txout.nValue; + } + + CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in, int input_bytes) : CInputCoin(outpoint_in, txout_in) + { + m_input_bytes = input_bytes; + } + COutPoint outpoint; CTxOut txout; CAmount effective_value; 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 7bc2dc7b21..aa97b748b1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -43,6 +43,7 @@ #include <univalue.h> +#include <map> using interfaces::FoundBlock; @@ -2632,7 +2633,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."}, @@ -3213,6 +3214,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() {"feeRate", UniValueType()}, // will be checked by AmountFromValue() below {"psbt", UniValueType(UniValue::VBOOL)}, + {"solving_data", UniValueType(UniValue::VOBJ)}, {"subtractFeeFromOutputs", UniValueType(UniValue::VARR)}, {"subtract_fee_from_outputs", UniValueType(UniValue::VARR)}, {"replaceable", UniValueType(UniValue::VBOOL)}, @@ -3289,6 +3291,54 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, wallet); } + if (options.exists("solving_data")) { + UniValue solving_data = options["solving_data"].get_obj(); + if (solving_data.exists("pubkeys")) { + for (const UniValue& pk_univ : solving_data["pubkeys"].get_array().getValues()) { + const std::string& pk_str = pk_univ.get_str(); + if (!IsHex(pk_str)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not hex", pk_str)); + } + const std::vector<unsigned char> data(ParseHex(pk_str)); + CPubKey pubkey(data.begin(), data.end()); + if (!pubkey.IsFullyValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not a valid public key", pk_str)); + } + coinControl.m_external_provider.pubkeys.emplace(pubkey.GetID(), pubkey); + // Add witness script for pubkeys + const CScript wit_script = GetScriptForDestination(WitnessV0KeyHash(pubkey)); + coinControl.m_external_provider.scripts.emplace(CScriptID(wit_script), wit_script); + } + } + + if (solving_data.exists("scripts")) { + for (const UniValue& script_univ : solving_data["scripts"].get_array().getValues()) { + const std::string& script_str = script_univ.get_str(); + if (!IsHex(script_str)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not hex", script_str)); + } + std::vector<unsigned char> script_data(ParseHex(script_str)); + const CScript script(script_data.begin(), script_data.end()); + coinControl.m_external_provider.scripts.emplace(CScriptID(script), script); + } + } + + if (solving_data.exists("descriptors")) { + for (const UniValue& desc_univ : solving_data["descriptors"].get_array().getValues()) { + const std::string& desc_str = desc_univ.get_str(); + FlatSigningProvider desc_out; + std::string error; + std::vector<CScript> scripts_temp; + std::unique_ptr<Descriptor> desc = Parse(desc_str, desc_out, error, true); + if (!desc) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Unable to parse descriptor '%s': %s", desc_str, error)); + } + desc->Expand(0, desc_out, scripts_temp, desc_out); + coinControl.m_external_provider = Merge(coinControl.m_external_provider, desc_out); + } + } + } + if (tx.vout.size() == 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); @@ -3306,6 +3356,19 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, setSubtractFeeFromOutputs.insert(pos); } + // Fetch specified UTXOs from the UTXO set to get the scriptPubKeys and values of the outputs being selected + // and to match with the given solving_data. Only used for non-wallet outputs. + std::map<COutPoint, Coin> coins; + for (const CTxIn& txin : tx.vin) { + coins[txin.prevout]; // Create empty map entry keyed by prevout. + } + wallet.chain().findCoins(coins); + for (const auto& coin : coins) { + if (!coin.second.out.IsNull()) { + coinControl.Select(coin.first, coin.second.out); + } + } + bilingual_str error; if (!FundTransaction(wallet, tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { @@ -3321,8 +3384,9 @@ static RPCHelpMan fundrawtransaction() "No existing outputs will be modified unless \"subtractFeeFromOutputs\" is specified.\n" "Note that inputs which were signed may need to be resigned after completion since in/outputs have been added.\n" "The inputs added will not be signed, use signrawtransactionwithkey\n" - " or signrawtransactionwithwallet for that.\n" - "Note that all existing inputs must have their previous output transaction be in the wallet.\n" + "or signrawtransactionwithwallet for that.\n" + "All existing inputs must either have their previous output transaction be in the wallet\n" + "or be in the UTXO set. Solving data must be provided for non-wallet inputs.\n" "Note that all inputs selected must be of standard form and P2SH scripts must be\n" "in the wallet using importaddress or addmultisigaddress (to calculate fees).\n" "You can see whether this is the case by checking the \"solvable\" field in the listunspent output.\n" @@ -3357,6 +3421,26 @@ static RPCHelpMan fundrawtransaction() {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + "Used for fee estimation during coin selection.", + { + {"pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Public keys involved in this transaction.", + { + {"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"}, + }, + }, + {"scripts", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Scripts involved in this transaction.", + { + {"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"}, + }, + }, + {"descriptors", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Descriptors that provide solving data for this transaction.", + { + {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A descriptor"}, + }, + } + } + }, }, "options"}, {"iswitness", RPCArg::Type::BOOL, RPCArg::DefaultHint{"depends on heuristic tests"}, "Whether the transaction hex is a serialized witness transaction.\n" @@ -4202,6 +4286,26 @@ static RPCHelpMan send() }, {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Marks this transaction as BIP125 replaceable.\n" "Allows this transaction to be replaced by a transaction with higher fees"}, + {"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + "Used for fee estimation during coin selection.", + { + {"pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Public keys involved in this transaction.", + { + {"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"}, + }, + }, + {"scripts", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Scripts involved in this transaction.", + { + {"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"}, + }, + }, + {"descriptors", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Descriptors that provide solving data for this transaction.", + { + {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A descriptor"}, + }, + } + } + }, }, "options"}, }, @@ -4489,7 +4593,9 @@ static RPCHelpMan walletcreatefundedpsbt() { return RPCHelpMan{"walletcreatefundedpsbt", "\nCreates and funds a transaction in the Partially Signed Transaction format.\n" - "Implements the Creator and Updater roles.\n", + "Implements the Creator and Updater roles.\n" + "All existing inputs must either have their previous output transaction be in the wallet\n" + "or be in the UTXO set. Solving data must be provided for non-wallet inputs.\n", { {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Leave empty to add inputs automatically. See add_inputs option.", { @@ -4546,6 +4652,26 @@ static RPCHelpMan walletcreatefundedpsbt() {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + "Used for fee estimation during coin selection.", + { + {"pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Public keys involved in this transaction.", + { + {"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"}, + }, + }, + {"scripts", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Scripts involved in this transaction.", + { + {"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"}, + }, + }, + {"descriptors", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Descriptors that provide solving data for this transaction.", + { + {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A descriptor"}, + }, + } + } + }, }, "options"}, {"bip32derivs", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include BIP 32 derivation paths for public keys if we know them"}, 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/spend.cpp b/src/wallet/spend.cpp index 1724375f4c..43fcc0416d 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -5,6 +5,7 @@ #include <consensus/validation.h> #include <interfaces/chain.h> #include <policy/policy.h> +#include <script/signingprovider.h> #include <util/check.h> #include <util/fees.h> #include <util/moneystr.h> @@ -31,21 +32,27 @@ std::string COutput::ToString() const return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); } -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig) { CMutableTransaction txn; txn.vin.push_back(CTxIn(COutPoint())); - if (!wallet->DummySignInput(txn.vin[0], txout, use_max_sig)) { + if (!provider || !DummySignInput(*provider, txn.vin[0], txout, use_max_sig)) { return -1; } return GetVirtualTransactionInputSize(txn.vin[0]); } +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) +{ + const std::unique_ptr<SigningProvider> provider = wallet->GetSolvingProvider(txout.scriptPubKey); + return CalculateMaximumSignedInputSize(txout, provider.get(), use_max_sig); +} + // txouts needs to be in the order of tx.vin -TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, bool use_max_sig) +TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control) { CMutableTransaction txNew(tx); - if (!wallet->DummySignTx(txNew, txouts, use_max_sig)) { + if (!wallet->DummySignTx(txNew, txouts, coin_control)) { return TxSize{-1, -1}; } CTransaction ctx(txNew); @@ -54,19 +61,27 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle return TxSize{vsize, weight}; } -TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig) +TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control) { std::vector<CTxOut> txouts; + // Look up the inputs. The inputs are either in the wallet, or in coin_control. for (const CTxIn& input : tx.vin) { const auto mi = wallet->mapWallet.find(input.prevout.hash); // Can not estimate size without knowing the input details - if (mi == wallet->mapWallet.end()) { + if (mi != wallet->mapWallet.end()) { + assert(input.prevout.n < mi->second.tx->vout.size()); + txouts.emplace_back(mi->second.tx->vout.at(input.prevout.n)); + } else if (coin_control) { + CTxOut txout; + if (!coin_control->GetExternalOutput(input.prevout, txout)) { + return TxSize{-1, -1}; + } + txouts.emplace_back(txout); + } else { return TxSize{-1, -1}; } - assert(input.prevout.n < mi->second.tx->vout.size()); - txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); } - return CalculateMaximumSignedTxSize(tx, wallet, txouts, use_max_sig); + return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control); } void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) @@ -435,32 +450,40 @@ bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCo std::vector<COutPoint> vPresetInputs; coin_control.ListSelected(vPresetInputs); - for (const COutPoint& outpoint : vPresetInputs) - { + for (const COutPoint& outpoint : vPresetInputs) { + int input_bytes = -1; + CTxOut txout; std::map<uint256, CWalletTx>::const_iterator it = wallet.mapWallet.find(outpoint.hash); - if (it != wallet.mapWallet.end()) - { + if (it != wallet.mapWallet.end()) { const CWalletTx& wtx = it->second; // Clearly invalid input, fail if (wtx.tx->vout.size() <= outpoint.n) { return false; } - // Just to calculate the marginal byte size - CInputCoin coin(wtx.tx, outpoint.n, GetTxSpendSize(wallet, wtx, outpoint.n, false)); - nValueFromPresetInputs += coin.txout.nValue; - if (coin.m_input_bytes <= 0) { - return false; // Not solvable, can't estimate size for fee - } - coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); - if (coin_selection_params.m_subtract_fee_outputs) { - value_to_select -= coin.txout.nValue; - } else { - value_to_select -= coin.effective_value; + input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false); + txout = wtx.tx->vout.at(outpoint.n); + } + if (input_bytes == -1) { + // The input is external. We either did not find the tx in mapWallet, or we did but couldn't compute the input size with wallet data + if (!coin_control.GetExternalOutput(outpoint, txout)) { + // Not ours, and we don't have solving data. + return false; } - setPresetCoins.insert(coin); + input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true); + } + + CInputCoin coin(outpoint, txout, input_bytes); + nValueFromPresetInputs += coin.txout.nValue; + if (coin.m_input_bytes <= 0) { + return false; // Not solvable, can't estimate size for fee + } + coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); + if (coin_selection_params.m_subtract_fee_outputs) { + value_to_select -= coin.txout.nValue; } else { - return false; // TODO: Allow non-wallet inputs + value_to_select -= coin.effective_value; } + setPresetCoins.insert(coin); } // remove preset inputs from vCoins so that Coin Selection doesn't pick them. @@ -788,10 +811,10 @@ static bool CreateTransactionInternal( } // Calculate the transaction fee - TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, coin_control.fAllowWatchOnly); + TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); int nBytes = tx_sizes.vsize; if (nBytes < 0) { - error = _("Signing transaction failed"); + error = _("Missing solving data for estimating transaction size"); return false; } nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes); @@ -813,7 +836,7 @@ static bool CreateTransactionInternal( txNew.vout.erase(change_position); // Because we have dropped this change, the tx size and required fee will be different, so let's recalculate those - tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, coin_control.fAllowWatchOnly); + tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); nBytes = tx_sizes.vsize; fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); } diff --git a/src/wallet/spend.h b/src/wallet/spend.h index e39f134dc3..3a723d9832 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -66,6 +66,7 @@ public: //Get the marginal bytes of spending the specified output int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* pwallet, bool use_max_sig = false); struct TxSize { int64_t vsize{-1}; @@ -76,8 +77,8 @@ struct TxSize { * Use DummySignatureCreator, which inserts 71 byte signatures everywhere. * NOTE: this requires that all inputs must be in mapWallet (eg the tx should * be AllInputsMine). */ -TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const std::vector<CTxOut>& txouts, bool use_max_sig = false); -TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); +TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control = nullptr); +TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const CCoinControl* coin_control = nullptr) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); /** * populate vCoins with vector of available COutputs. 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 6885499be2..bf6d8e7510 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1448,19 +1448,13 @@ bool CWallet::AddWalletFlags(uint64_t flags) // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) // or a max-sized low-S signature (e.g. 72 bytes) if use_max_sig is true -bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) const +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; - std::unique_ptr<SigningProvider> provider = GetSolvingProvider(scriptPubKey); - if (!provider) { - // We don't know about this scriptpbuKey; - return false; - } - - if (!ProduceSignature(*provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { + if (!ProduceSignature(provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; } UpdateInput(tx_in, sigdata); @@ -1468,14 +1462,21 @@ bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig } // Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes) -bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, bool use_max_sig) const +bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control) const { // Fill in dummy signatures for fee calculation. int nIn = 0; for (const auto& txout : txouts) { - if (!DummySignInput(txNew.vin[nIn], txout, use_max_sig)) { - return false; + CTxIn& txin = txNew.vin[nIn]; + // Use max sig if watch only inputs were used or if this particular input is an external input + // to ensure a sufficient fee is attained for the requested feerate. + const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(txin.prevout)); + const std::unique_ptr<SigningProvider> provider = GetSolvingProvider(txout.scriptPubKey); + if (!provider || !DummySignInput(*provider, txin, txout, use_max_sig)) { + if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, use_max_sig)) { + return false; + } } nIn++; @@ -1600,7 +1601,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)); @@ -2007,10 +2008,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) @@ -2542,6 +2540,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) { @@ -2562,6 +2561,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); @@ -2753,7 +2756,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; } @@ -2775,7 +2778,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 @@ -2792,8 +2795,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 15a5933424..b949fe3040 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -337,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: /** @@ -576,14 +576,13 @@ public: /** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */ bool SubmitTxMemoryPoolAndRelay(const CWalletTx& wtx, std::string& err_string, bool relay) const; - bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const + bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const { std::vector<CTxOut> v_txouts(txouts.size()); std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); - return DummySignTx(txNew, v_txouts, use_max_sig); + return DummySignTx(txNew, v_txouts, coin_control); } - bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, bool use_max_sig = false) const; - bool DummySignInput(CTxIn &tx_in, const CTxOut &txout, bool use_max_sig = false) const; + bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const; bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportPrivKeys(const std::map<CKeyID, CKey>& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -928,4 +927,6 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); //! Remove wallet name from persistent configuration so it will not be loaded on startup. bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig); + #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c697534c06..8ff09a0878 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -311,6 +311,7 @@ public: std::map<std::pair<uint256, CKeyID>, CKey> m_descriptor_keys; std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys; std::map<uint160, CHDChain> m_hd_chains; + bool tx_corrupt{false}; CWalletScanState() { } @@ -345,7 +346,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, // LoadToWallet call below creates a new CWalletTx that fill_wtx // callback fills with transaction metadata. auto fill_wtx = [&](CWalletTx& wtx, bool new_tx) { - assert(new_tx); + if(!new_tx) { + // There's some corruption here since the tx we just tried to load was already in the wallet. + // We don't consider this type of corruption critical, and can fix it by removing tx data and + // rescanning. + wss.tx_corrupt = true; + return false; + } ssValue >> wtx; if (wtx.GetHash() != hash) return false; @@ -755,6 +762,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; bool fNoncriticalErrors = false; + bool rescan_required = false; DBErrors result = DBErrors::LOAD_OK; LOCK(pwallet->cs_wallet); @@ -818,12 +826,17 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } else if (strType == DBKeys::FLAGS) { // reading the wallet flags can only fail if unknown flags are present result = DBErrors::TOO_NEW; + } else if (wss.tx_corrupt) { + pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n"); + // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) + wss.tx_corrupt = false; + result = DBErrors::CORRUPT; } else { // Leave other errors alone, if we try to fix them we might make things worse. 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 +872,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; diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 5ef3860867..4382022a7a 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -42,7 +42,6 @@ class NotificationsTest(BitcoinTestFramework): f"-alertnotify=echo > {os.path.join(self.alertnotify_dir, '%s')}", f"-blocknotify=echo > {os.path.join(self.blocknotify_dir, '%s')}", ], [ - "-rescan", f"-walletnotify=echo %h_%b > {os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))}", ]] self.wallet_names = [self.default_wallet_name, self.wallet] @@ -91,16 +90,15 @@ class NotificationsTest(BitcoinTestFramework): # directory content should equal the generated transaction hashes tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) - self.stop_node(1) self.expect_wallet_notify(tx_details) self.log.info("test -walletnotify after rescan") - # restart node to rescan to force wallet notifications - self.start_node(1) - self.connect_nodes(0, 1) - + # rescan to force wallet notifications + self.nodes[1].rescanblockchain() self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) + self.connect_nodes(0, 1) + # directory content should equal the generated transaction hashes tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) self.expect_wallet_notify(tx_details) diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py new file mode 100755 index 0000000000..4073ec03a6 --- /dev/null +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021-2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test that a node in blocksonly mode does not request compact blocks.""" + +from test_framework.messages import ( + MSG_BLOCK, + MSG_CMPCT_BLOCK, + MSG_WITNESS_FLAG, + CBlock, + CBlockHeader, + CInv, + from_hex, + msg_block, + msg_getdata, + msg_headers, + msg_sendcmpct, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): + def set_test_params(self): + self.extra_args = [["-blocksonly"], [], [], []] + self.num_nodes = 4 + + def setup_network(self): + self.setup_nodes() + # Start network with everyone disconnected + self.sync_all() + + def build_block_on_tip(self): + blockhash = self.nodes[2].generate(1)[0] + block_hex = self.nodes[2].getblock(blockhash=blockhash, verbosity=0) + block = from_hex(CBlock(), block_hex) + block.rehash() + return block + + def run_test(self): + # Nodes will only request hb compact blocks mode when they're out of IBD + for node in self.nodes: + assert not node.getblockchaininfo()['initialblockdownload'] + + p2p_conn_blocksonly = self.nodes[0].add_p2p_connection(P2PInterface()) + p2p_conn_high_bw = self.nodes[1].add_p2p_connection(P2PInterface()) + p2p_conn_low_bw = self.nodes[3].add_p2p_connection(P2PInterface()) + for conn in [p2p_conn_blocksonly, p2p_conn_high_bw, p2p_conn_low_bw]: + assert_equal(conn.message_count['sendcmpct'], 2) + conn.send_and_ping(msg_sendcmpct(announce=False, version=2)) + + # Nodes: + # 0 -> blocksonly + # 1 -> high bandwidth + # 2 -> miner + # 3 -> low bandwidth + # + # Topology: + # p2p_conn_blocksonly ---> node0 + # p2p_conn_high_bw ---> node1 + # p2p_conn_low_bw ---> node3 + # node2 (no connections) + # + # node2 produces blocks that are passed to the rest of the nodes + # through the respective p2p connections. + + self.log.info("Test that -blocksonly nodes do not select peers for BIP152 high bandwidth mode") + + block0 = self.build_block_on_tip() + + # A -blocksonly node should not request BIP152 high bandwidth mode upon + # receiving a new valid block at the tip. + p2p_conn_blocksonly.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[0].getbestblockhash(), 16), block0.sha256) + assert_equal(p2p_conn_blocksonly.message_count['sendcmpct'], 2) + assert_equal(p2p_conn_blocksonly.last_message['sendcmpct'].announce, False) + + # A normal node participating in transaction relay should request BIP152 + # high bandwidth mode upon receiving a new valid block at the tip. + p2p_conn_high_bw.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[1].getbestblockhash(), 16), block0.sha256) + p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 3) + assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True) + + # Don't send a block from the p2p_conn_low_bw so the low bandwidth node + # doesn't select it for BIP152 high bandwidth relay. + self.nodes[3].submitblock(block0.serialize().hex()) + + self.log.info("Test that -blocksonly nodes send getdata(BLOCK) instead" + " of getdata(CMPCT) in BIP152 low bandwidth mode") + + block1 = self.build_block_on_tip() + + p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_blocksonly.sync_send_with_ping() + assert_equal(p2p_conn_blocksonly.last_message['getdata'].inv, [CInv(MSG_BLOCK | MSG_WITNESS_FLAG, block1.sha256)]) + + p2p_conn_high_bw.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_high_bw.sync_send_with_ping() + assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + + self.log.info("Test that getdata(CMPCT) is still sent on BIP152 low bandwidth connections" + " when no -blocksonly nodes are involved") + + p2p_conn_low_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_low_bw.sync_with_ping() + assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + + self.log.info("Test that -blocksonly nodes still serve compact blocks") + + def test_for_cmpctblock(block): + if 'cmpctblock' not in p2p_conn_blocksonly.last_message: + return False + return p2p_conn_blocksonly.last_message['cmpctblock'].header_and_shortids.header.rehash() == block.sha256 + + p2p_conn_blocksonly.send_message(msg_getdata([CInv(MSG_CMPCT_BLOCK, block0.sha256)])) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block0)) + + # Request BIP152 high bandwidth mode from the -blocksonly node. + p2p_conn_blocksonly.send_and_ping(msg_sendcmpct(announce=True, version=2)) + + block2 = self.build_block_on_tip() + self.nodes[0].submitblock(block1.serialize().hex()) + self.nodes[0].submitblock(block2.serialize().hex()) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block2)) + +if __name__ == '__main__': + P2PCompactBlocksBlocksOnly().main() diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index cda0ae0eeb..3b01506986 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -8,6 +8,7 @@ from decimal import Decimal from itertools import product from test_framework.descriptors import descsum_create +from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_approx, @@ -19,6 +20,7 @@ from test_framework.util import ( count_bytes, find_vout_for_address, ) +from test_framework.wallet_util import bytes_to_wif def get_unspent(listunspent, amount): @@ -132,6 +134,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_subtract_fee_with_presets() self.test_transaction_too_large() self.test_include_unsafe() + self.test_external_inputs() self.test_22670() def test_change_position(self): @@ -983,6 +986,56 @@ class RawTransactionsTest(BitcoinTestFramework): wallet.sendmany("", outputs) self.generate(self.nodes[0], 10) assert_raises_rpc_error(-4, "Transaction too large", recipient.fundrawtransaction, rawtx) + self.nodes[0].unloadwallet("large") + + def test_external_inputs(self): + self.log.info("Test funding with external inputs") + + eckey = ECKey() + eckey.generate() + privkey = bytes_to_wif(eckey.get_bytes()) + + self.nodes[2].createwallet("extfund") + wallet = self.nodes[2].get_wallet_rpc("extfund") + + # Make a weird but signable script. sh(pkh()) descriptor accomplishes this + desc = descsum_create("sh(pkh({}))".format(privkey)) + if self.options.descriptors: + res = self.nodes[0].importdescriptors([{"desc": desc, "timestamp": "now"}]) + else: + res = self.nodes[0].importmulti([{"desc": desc, "timestamp": "now"}]) + assert res[0]["success"] + addr = self.nodes[0].deriveaddresses(desc)[0] + addr_info = self.nodes[0].getaddressinfo(addr) + + self.nodes[0].sendtoaddress(addr, 10) + self.nodes[0].sendtoaddress(wallet.getnewaddress(), 10) + self.nodes[0].generate(6) + ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] + + # An external input without solving data should result in an error + raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): 15}) + assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx) + + # Error conditions + assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}}) + assert_raises_rpc_error(-5, "'01234567890a0b0c0d0e0f' is not a valid public key", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["01234567890a0b0c0d0e0f"]}}) + assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"scripts":["not a script"]}}) + assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, {"solving_data": {"descriptors":["not a descriptor"]}}) + + # But funding should work when the solving data is provided + funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) + signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex']) + assert not signed_tx['complete'] + signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx['hex']) + assert signed_tx['complete'] + + funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"descriptors": [desc]}}) + signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex']) + assert not signed_tx['complete'] + signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx['hex']) + assert signed_tx['complete'] + self.nodes[2].unloadwallet("extfund") def test_include_unsafe(self): self.log.info("Test fundrawtxn with unsafe inputs") @@ -1017,6 +1070,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert all((txin["txid"], txin["vout"]) in inputs for txin in tx_dec["vin"]) signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex']) assert wallet.testmempoolaccept([signedtx['hex']])[0]["allowed"] + self.nodes[0].unloadwallet("unsafe") def test_22670(self): # In issue #22670, it was observed that ApproximateBestSubset may diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index f330bbf1c3..6b5b2c6a0f 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -8,6 +8,8 @@ from decimal import Decimal from itertools import product +from test_framework.descriptors import descsum_create +from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_approx, @@ -16,6 +18,7 @@ from test_framework.util import ( assert_raises_rpc_error, find_output, ) +from test_framework.wallet_util import bytes_to_wif import json import os @@ -608,5 +611,42 @@ class PSBTTest(BitcoinTestFramework): assert_raises_rpc_error(-25, 'Inputs missing or spent', self.nodes[0].walletprocesspsbt, 'cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + # Test that we can fund psbts with external inputs specified + eckey = ECKey() + eckey.generate() + privkey = bytes_to_wif(eckey.get_bytes()) + + # Make a weird but signable script. sh(pkh()) descriptor accomplishes this + desc = descsum_create("sh(pkh({}))".format(privkey)) + if self.options.descriptors: + res = self.nodes[0].importdescriptors([{"desc": desc, "timestamp": "now"}]) + else: + res = self.nodes[0].importmulti([{"desc": desc, "timestamp": "now"}]) + assert res[0]["success"] + addr = self.nodes[0].deriveaddresses(desc)[0] + addr_info = self.nodes[0].getaddressinfo(addr) + + self.nodes[0].sendtoaddress(addr, 10) + self.nodes[0].generate(6) + ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] + + # An external input without solving data should result in an error + assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[1].walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 10 + ext_utxo['amount']}, 0, {'add_inputs': True}) + + # But funding should work when the solving data is provided + psbt = self.nodes[1].walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) + signed = self.nodes[1].walletprocesspsbt(psbt['psbt']) + assert not signed['complete'] + signed = self.nodes[0].walletprocesspsbt(signed['psbt']) + assert signed['complete'] + self.nodes[0].finalizepsbt(signed['psbt']) + + psbt = self.nodes[1].walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True, "solving_data":{"descriptors": [desc]}}) + signed = self.nodes[1].walletprocesspsbt(psbt['psbt']) + assert not signed['complete'] + signed = self.nodes[0].walletprocesspsbt(signed['psbt']) + assert signed['complete'] + self.nodes[0].finalizepsbt(signed['psbt']) + if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index ea5c641b4a..9f5bca6884 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -34,13 +34,14 @@ def assert_approx(v, vexp, vspan=0.00001): raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) -def assert_fee_amount(fee, tx_size, fee_per_kB): - """Assert the fee was in range""" - target_fee = round(tx_size * fee_per_kB / 1000, 8) +def assert_fee_amount(fee, tx_size, feerate_BTC_kvB): + """Assert the fee is in range.""" + feerate_BTC_vB = feerate_BTC_kvB / 1000 + target_fee = satoshi_round(tx_size * feerate_BTC_vB) if fee < target_fee: raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee))) # allow the wallet's estimation to be at most 2 bytes off - if fee > (tx_size + 2) * fee_per_kB / 1000: + if fee > (tx_size + 2) * feerate_BTC_vB: raise AssertionError("Fee of %s BTC too high! (Should be %s BTC)" % (str(fee), str(target_fee))) @@ -366,7 +367,7 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect= f.write("listenonion=0\n") # Increase peertimeout to avoid disconnects while using mocktime. # peertimeout is measured in wall clock time, so setting it to the - # duration of the longest test is sufficient. It can be overriden in + # duration of the longest test is sufficient. It can be overridden in # tests. f.write("peertimeout=999999\n") f.write("printtoconsole=0\n") diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2c2aaf6020..bb84962b75 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -98,6 +98,7 @@ BASE_SCRIPTS = [ 'rpc_fundrawtransaction.py --legacy-wallet', 'rpc_fundrawtransaction.py --descriptors', 'p2p_compactblocks.py', + 'p2p_compactblocks_blocksonly.py', 'feature_segwit.py --legacy-wallet', # vv Tests less than 2m vv 'wallet_basic.py --legacy-wallet', diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index f57f2a44bd..599e506f98 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -582,23 +582,17 @@ class WalletTest(BitcoinTestFramework): assert label in self.nodes[0].listlabels() self.nodes[0].rpc.ensure_ascii = True # restore to default - # maintenance tests - maintenance = [ - '-rescan', - '-reindex', - ] + # -reindex tests chainlimit = 6 - for m in maintenance: - self.log.info("Test " + m) - self.stop_nodes() - # set lower ancestor limit for later - self.start_node(0, [m, "-limitancestorcount=" + str(chainlimit)]) - self.start_node(1, [m, "-limitancestorcount=" + str(chainlimit)]) - self.start_node(2, [m, "-limitancestorcount=" + str(chainlimit)]) - if m == '-reindex': - # reindex will leave rpc warm up "early"; Wait for it to finish - self.wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)]) - assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)]) + self.log.info("Test -reindex") + self.stop_nodes() + # set lower ancestor limit for later + self.start_node(0, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) + self.start_node(1, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) + self.start_node(2, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) + # reindex will leave rpc warm up "early"; Wait for it to finish + self.wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)]) + assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)]) # Exercise listsinceblock with the last two blocks coinbase_tx_1 = self.nodes[0].listsinceblock(blocks[0]) diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 74f584f2cd..974ce7f381 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -103,7 +103,7 @@ class WalletHDTest(BitcoinTestFramework): self.sync_all() # Needs rescan - self.restart_node(1, extra_args=self.extra_args[1] + ['-rescan']) + self.nodes[1].rescanblockchain() assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1) # Try a RPC based rescan diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index aecdaf821f..7b23235945 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -9,6 +9,7 @@ from itertools import product from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create +from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -16,6 +17,7 @@ from test_framework.util import ( assert_greater_than, assert_raises_rpc_error, ) +from test_framework.wallet_util import bytes_to_wif class WalletSendTest(BitcoinTestFramework): def set_test_params(self): @@ -35,7 +37,7 @@ class WalletSendTest(BitcoinTestFramework): conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None, include_watching=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, - expect_error=None): + expect_error=None, solving_data=None): assert (amount is None) != (data is None) from_balance_before = from_wallet.getbalances()["mine"]["trusted"] @@ -94,6 +96,8 @@ class WalletSendTest(BitcoinTestFramework): options["replaceable"] = replaceable if subtract_fee_from_outputs is not None: options["subtract_fee_from_outputs"] = subtract_fee_from_outputs + if solving_data is not None: + options["solving_data"] = solving_data if len(options.keys()) == 0: options = None @@ -476,6 +480,46 @@ class WalletSendTest(BitcoinTestFramework): res = self.test_send(from_wallet=w5, to_wallet=w0, amount=1, include_unsafe=True) assert res["complete"] + self.log.info("External outputs") + eckey = ECKey() + eckey.generate() + privkey = bytes_to_wif(eckey.get_bytes()) + + self.nodes[1].createwallet("extsend") + ext_wallet = self.nodes[1].get_wallet_rpc("extsend") + self.nodes[1].createwallet("extfund") + ext_fund = self.nodes[1].get_wallet_rpc("extfund") + + # Make a weird but signable script. sh(pkh()) descriptor accomplishes this + desc = descsum_create("sh(pkh({}))".format(privkey)) + if self.options.descriptors: + res = ext_fund.importdescriptors([{"desc": desc, "timestamp": "now"}]) + else: + res = ext_fund.importmulti([{"desc": desc, "timestamp": "now"}]) + assert res[0]["success"] + addr = self.nodes[0].deriveaddresses(desc)[0] + addr_info = ext_fund.getaddressinfo(addr) + + self.nodes[0].sendtoaddress(addr, 10) + self.nodes[0].sendtoaddress(ext_wallet.getnewaddress(), 10) + self.nodes[0].generate(6) + ext_utxo = ext_fund.listunspent(addresses=[addr])[0] + + # An external input without solving data should result in an error + self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Insufficient funds")) + + # But funding should work when the solving data is provided + res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}) + signed = ext_wallet.walletprocesspsbt(res["psbt"]) + signed = ext_fund.walletprocesspsbt(res["psbt"]) + assert signed["complete"] + self.nodes[0].finalizepsbt(signed["psbt"]) + + res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"descriptors": [desc]}) + signed = ext_wallet.walletprocesspsbt(res["psbt"]) + signed = ext_fund.walletprocesspsbt(res["psbt"]) + assert signed["complete"] + self.nodes[0].finalizepsbt(signed["psbt"]) if __name__ == '__main__': WalletSendTest().main() diff --git a/test/lint/README.md b/test/lint/README.md index 7e06308347..c4d76eac94 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -27,10 +27,10 @@ Usage: test/lint/git-subtree-check.sh [-r] DIR [COMMIT] To do a full check with `-r`, make sure that you have fetched the upstream repository branch in which the subtree is maintained: * for `src/secp256k1`: https://github.com/bitcoin-core/secp256k1.git (branch master) -* for `src/leveldb`: https://github.com/bitcoin-core/leveldb.git (branch bitcoin-fork) -* for `src/univalue`: https://github.com/bitcoin-core/univalue.git (branch master) +* for `src/leveldb`: https://github.com/bitcoin-core/leveldb-subtree.git (branch bitcoin-fork) +* for `src/univalue`: https://github.com/bitcoin-core/univalue-subtree.git (branch master) * for `src/crypto/ctaes`: https://github.com/bitcoin-core/ctaes.git (branch master) -* for `src/crc32c`: https://github.com/google/crc32c.git (branch master) +* for `src/crc32c`: https://github.com/bitcoin-core/crc32c-subtree.git (branch bitcoin-fork) To do so, add the upstream repository as remote: diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index d6312270e7..fcc4883d0b 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -47,11 +47,11 @@ KNOWN_VIOLATIONS=( "src/test/dbwrapper_tests.cpp:.*snprintf" "src/test/fuzz/locale.cpp" "src/test/fuzz/parse_numbers.cpp:.*atoi" + "src/test/fuzz/string.cpp" "src/torcontrol.cpp:.*atoi" "src/torcontrol.cpp:.*strtol" "src/util/strencodings.cpp:.*atoi" - "src/util/strencodings.cpp:.*strtol" - "src/util/strencodings.cpp:.*strtoul" + "src/util/strencodings.cpp:.*strtoll" "src/util/strencodings.h:.*atoi" "src/util/system.cpp:.*atoi" ) diff --git a/test/lint/lint-logs.sh b/test/lint/lint-logs.sh index 2fbb4a38e7..d6c53e8ff3 100755 --- a/test/lint/lint-logs.sh +++ b/test/lint/lint-logs.sh @@ -7,7 +7,7 @@ # Check that all logs are terminated with '\n' # # Some logs are continued over multiple lines. They should be explicitly -# commented with \* Continued *\ +# commented with /* Continued */ # # There are some instances of LogPrintf() in comments. Those can be # ignored |