diff options
122 files changed, 1810 insertions, 851 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 44b49bb397..9c2598d606 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,3 +1,36 @@ +# Global defaults +timeout_in: 120m # https://cirrus-ci.org/faq/#instance-timed-out +container: + # https://cirrus-ci.org/faq/#are-there-any-limits + # Each project has 16 CPU in total, assign 2 to each container, so that 8 tasks run in parallel + cpu: 2 + memory: 6G # https://cirrus-ci.org/guide/linux/#linux-containers +env: + PACKAGE_MANAGER_INSTALL : "apt-get update && apt-get install -y" + MAKEJOBS: "-j4" + DANGER_RUN_CI_ON_HOST: "1" # Containers will be discarded after the run, so there is no risk that the ci scripts modify the system + TEST_RUNNER_PORT_MIN: "14000" # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache + CCACHE_SIZE: "200M" + CCACHE_DIR: "/tmp/ccache_dir" +# Global task template +# https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks +global_task_template: &GLOBAL_TASK_TEMPLATE + ccache_cache: + folder: "/tmp/ccache_dir" + depends_built_cache: + folder: "/tmp/cirrus-ci-build/depends/built" + depends_sdk_cache: + folder: "/tmp/cirrus-ci-build/depends/sdk-sources" + depends_releases_cache: + folder: "/tmp/cirrus-ci-build/releases" + merge_base_script: + - bash -c "$PACKAGE_MANAGER_INSTALL git" + - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH + - git config --global user.email "ci@ci.ci" + - git config --global user.name "ci" + - git merge FETCH_HEAD # Merge base to detect silent merge conflicts + ci_script: + - ./ci/test_run_all.sh #task: # name: "Windows" # windows_container: @@ -13,22 +46,11 @@ # VCPKG_COMMIT_ID: 'ed0df8ecc4ed7e755ea03e18aaf285fd9b4b4a74' # install_script: # - choco install python --version=3.7.7 -y + task: - name: "x86_64 Linux [GOAL: install] [bionic] [Using ./ci/ system]" + name: 'x86_64 Linux [GOAL: install] [focal] [no depends, only system libs, sanitizers: address/leak (ASan + LSan) + undefined (UBSan) + integer]' + << : *GLOBAL_TASK_TEMPLATE container: - image: ubuntu:18.04 - cpu: 8 - memory: 8G - timeout_in: 60m + image: ubuntu:focal env: - MAKEJOBS: "-j9" - DANGER_RUN_CI_ON_HOST: "1" - TEST_RUNNER_PORT_MIN: "14000" # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache - CCACHE_SIZE: "200M" - CCACHE_DIR: "/tmp/ccache_dir" - ccache_cache: - folder: "/tmp/ccache_dir" - depends_built_cache: - folder: "/tmp/cirrus-ci-build/depends/built" - ci_script: - - ./ci/test_run_all.sh + FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index 35b42424ad..eedeeb4e54 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -4,7 +4,11 @@ General bitcoin questions and/or support requests are best directed to the Bitco For reporting security issues, please read instructions at https://bitcoincore.org/en/contact/. -If the node is "stuck" during sync or giving "block checksum mismatch" errors, please ensure your hardware is stable by running memtest and observe CPU temperature with a load-test tool such as linpack before creating an issue! --> +If the node is "stuck" during sync or giving "block checksum mismatch" errors, please ensure your hardware is stable by running memtest and observe CPU temperature with a load-test tool such as linpack before creating an issue! + +Any report, issue or feature request related to the GUI should be reported at +https://github.com/bitcoin-core/gui/issues/ +--> <!-- Describe the issue --> <!--- What behavior did you expect? --> diff --git a/.github/ISSUE_TEMPLATE/gui_issue.md b/.github/ISSUE_TEMPLATE/gui_issue.md new file mode 100644 index 0000000000..37acc81e21 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/gui_issue.md @@ -0,0 +1,11 @@ +--- +name: An issue or feature request related to the GUI +about: Any report, issue or feature request related to the GUI should be reported at https://github.com/bitcoin-core/gui/issues/ +title: Any report, issue or feature request related to the GUI should be reported at https://github.com/bitcoin-core/gui/issues/ +labels: GUI +assignees: '' + +--- + +Any report, issue or feature request related to the GUI should be reported at +https://github.com/bitcoin-core/gui/issues/ diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index d2c3b23375..ae92fc78f2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -3,6 +3,10 @@ Pull requests without a rationale and clear improvement may be closed immediately. + +GUI-related pull requests should be opened against +https://github.com/bitcoin-core/gui +first. See CONTRIBUTING.md --> <!-- diff --git a/.travis.yml b/.travis.yml index 283df78633..9ab2227116 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,18 +1,4 @@ -# The test build matrix (stage: test) is constructed to test a wide range of -# configurations, rather than a single pass/fail. This helps to catch build -# failures and logic errors that present on platforms other than the ones the -# author has tested. -# -# Some builders use the dependency-generator in `./depends`, rather than using -# apt-get to install build dependencies. This guarantees that the tester is -# using the same versions as Gitian, so the build results are nearly identical -# to what would be found in a final release. -# -# In order to avoid rebuilding all dependencies for each build, the binaries -# are cached and re-used when possible. Changes in the dependency-generator -# will trigger cache-invalidation and rebuilds as necessary. -# -# These caches can be manually removed if necessary. This is one of the very +# Travis caches can be manually removed if necessary. This is one of the very # few manual operations that is possible with Travis, and it can be done by a # Bitcoin Core GitHub member via the Travis web interface [0]. # @@ -27,7 +13,7 @@ version: ~> 1.0 -dist: xenial +dist: bionic os: linux language: minimal arch: amd64 @@ -121,23 +107,17 @@ jobs: FILE_ENV="./ci/test/00_setup_env_native_tsan.sh" - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [no depends, only system libs, sanitizers: address/leak (ASan + LSan) + undefined (UBSan) + integer]' - env: >- - FILE_ENV="./ci/test/00_setup_env_native_asan.sh" - - - stage: test name: 'x86_64 Linux [GOAL: install] [focal] [no depends, only system libs, sanitizers: fuzzer,address,undefined]' env: >- FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [multiprocess]' - if: type != pull_request OR commit_message =~ /depends:|multiprocess:/ # Skip on non-depends, non-multiprocess PRs + name: 'x86_64 Linux [GOAL: install] [focal] [multiprocess]' env: >- FILE_ENV="./ci/test/00_setup_env_native_multiprocess.sh" - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [no wallet]' + name: 'x86_64 Linux [GOAL: install] [xenial] [no wallet]' env: >- FILE_ENV="./ci/test/00_setup_env_native_nowallet.sh" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f34b1e1e15..65bc45a00b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,6 +84,26 @@ To contribute a patch, the workflow is as follows: 1. Create topic branch 1. Commit patches +For GUI-related issues or pull requests, the https://github.com/bitcoin-core/gui repository should be used. +For all other issues and pull requests, the https://github.com/bitcoin/bitcoin node repository should be used. + +The master branch for all monotree repositories is identical. + +As a rule of thumb, everything that only modifies `src/qt` is a GUI-only pull +request. However: + +* For global refactoring or other transversal changes the node repository + should be used. +* For GUI-related build system changes, the node repository should be used + because the change needs review by the build systems reviewers. +* Changes in `src/interfaces` need to go to the node repository because they + might affect other components like the wallet. + +For large GUI changes that include build system and interface changes, it is +recommended to first open a pull request against the GUI repository. When there +is agreement to proceed with the changes, a pull request with the build system +and interfaces changes can be submitted to the node repository. + The project coding conventions in the [developer notes](doc/developer-notes.md) must be followed. @@ -25,9 +25,14 @@ information or see https://opensource.org/licenses/MIT. Development Process ------------------- -The `master` branch is regularly built (see doc/build-*.md for instructions) and tested, but is not guaranteed to be +The `master` branch is regularly built (see `doc/build-*.md` for instructions) and tested, but it is not guaranteed to be completely stable. [Tags](https://github.com/bitcoin/bitcoin/tags) are created -regularly to indicate new official, stable release versions of Bitcoin Core. +regularly from release branches to indicate new official, stable release versions of Bitcoin Core. + +The https://github.com/bitcoin-core/gui repository is used exclusively for the +development of the GUI. Its master branch is identical in all monotree +repositories. Release branches and tags do not exist, so please do not fork +that repository unless it is for development reasons. The contribution workflow is described in [CONTRIBUTING.md](CONTRIBUTING.md) and useful hints for developers can be found in [doc/developer-notes.md](doc/developer-notes.md). diff --git a/build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj b/build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj index 992f64ec2e..6a3c9f1dc1 100644 --- a/build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj +++ b/build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj @@ -35,6 +35,7 @@ <ClCompile Include="..\..\src\qt\paymentserver.cpp" /> <ClCompile Include="..\..\src\qt\peertablemodel.cpp" /> <ClCompile Include="..\..\src\qt\platformstyle.cpp" /> + <ClCompile Include="..\..\src\qt\psbtoperationsdialog.cpp" /> <ClCompile Include="..\..\src\qt\qrimagewidget.cpp" /> <ClCompile Include="..\..\src\qt\qvalidatedlineedit.cpp" /> <ClCompile Include="..\..\src\qt\qvaluecombobox.cpp" /> @@ -87,6 +88,7 @@ <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_paymentserver.cpp" /> <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_peertablemodel.cpp" /> <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_platformstyle.cpp" /> + <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_psbtoperationsdialog.cpp" /> <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_qrimagewidget.cpp" /> <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_qvalidatedlineedit.cpp" /> <ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_qvaluecombobox.cpp" /> diff --git a/ci/README.md b/ci/README.md index d2ea255b4b..3c5f04c39e 100644 --- a/ci/README.md +++ b/ci/README.md @@ -1,12 +1,8 @@ -## ci scripts +## CI Scripts This directory contains scripts for each build step in each build stage. -Currently three stages `lint`, `extended_lint` and `test` are defined. Each stage has its own lifecycle, similar to the -[Travis CI lifecycle](https://docs.travis-ci.com/user/job-lifecycle#the-job-lifecycle). Every script in here is named -and numbered according to which stage and lifecycle step it belongs to. - -### Running a stage locally +### Running a Stage Locally Be aware that the tests will be built and run in-place, so please run at your own risk. If the repository is not a fresh git clone, you might have to clean files from previous builds or test runs first. @@ -36,3 +32,34 @@ To run the test stage with a specific configuration, ``` FILE_ENV="./ci/test/00_setup_env_arm.sh" ./ci/test_run_all.sh ``` + +### Configurations + +The test files (`FILE_ENV`) are constructed to test a wide range of +configurations, rather than a single pass/fail. This helps to catch build +failures and logic errors that present on platforms other than the ones the +author has tested. + +Some builders use the dependency-generator in `./depends`, rather than using +the system package manager to install build dependencies. This guarantees that +the tester is using the same versions as the release builds, which also use +`./depends`. + +If no `FILE_ENV` has been specified or values are left out, `00_setup_env.sh` +is used as the default configuration with fallback values. + +It is also possible to force a specific configuration without modifying the +file. For example, + +``` +MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_arm.sh" ./ci/test_run_all.sh +``` + +The files starting with `0n` (`n` greater than 0) are the scripts that are run +in order. + +### Cache + +In order to avoid rebuilding all dependencies for each build, the binaries are +cached and re-used when possible. Changes in the dependency-generator will +trigger cache-invalidation and rebuilds as necessary. diff --git a/ci/test/00_setup_env_mac_host.sh b/ci/test/00_setup_env_mac_host.sh index b24dc7278d..f50efcc33a 100644 --- a/ci/test/00_setup_env_mac_host.sh +++ b/ci/test/00_setup_env_mac_host.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export HOST=x86_64-apple-darwin16 +export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic can cross-compile to macos (bionic is used in the gitian build as well) export PIP_PACKAGES="zmq" export GOAL="install" export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --enable-werror" diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 28c63f1cf6..225f77df92 100644 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -7,8 +7,8 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_asan -export PACKAGES="clang-8 llvm-8 python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" -# Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running on ppc64le +export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" +export DOCKER_NAME_TAG=ubuntu:20.04 export NO_DEPENDS=1 export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang-8 CXX=clang++-8" +export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++" diff --git a/ci/test/00_setup_env_native_multiprocess.sh b/ci/test/00_setup_env_native_multiprocess.sh index 0fc989085c..786f0f927f 100644 --- a/ci/test/00_setup_env_native_multiprocess.sh +++ b/ci/test/00_setup_env_native_multiprocess.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_multiprocess +export DOCKER_NAME_TAG=ubuntu:20.04 export PACKAGES="cmake python3" export DEP_OPTS="MULTIPROCESS=1" export GOAL="install" diff --git a/ci/test/00_setup_env_native_nowallet.sh b/ci/test/00_setup_env_native_nowallet.sh index 9c2be4cfac..553dab1491 100644 --- a/ci/test/00_setup_env_native_nowallet.sh +++ b/ci/test/00_setup_env_native_nowallet.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_nowallet +export DOCKER_NAME_TAG=ubuntu:16.04 # Use xenial to have one config run the tests in python3.5, see doc/dependencies.md export PACKAGES="python3-zmq" export DEP_OPTS="NO_WALLET=1" export GOAL="install" diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index 496251a125..6e2ff729a2 100644 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_qt5 +export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic can compile our c++17 and run our functional tests in python3 export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev" export DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index 90c3ebd28e..eb8b870dd6 100644 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -7,6 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_win64 +export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic can cross-compile to win64 (bionic is used in the gitian build as well) export HOST=x86_64-w64-mingw32 export PACKAGES="python3 nsis g++-mingw-w64-x86-64 wine-binfmt wine64" export RUN_FUNCTIONAL_TESTS=false diff --git a/configure.ac b/configure.ac index 474d8a7919..12bece6903 100644 --- a/configure.ac +++ b/configure.ac @@ -111,9 +111,6 @@ AC_PATH_TOOL(READELF, readelf) AC_PATH_TOOL(CPPFILT, c++filt) AC_PATH_TOOL(OBJCOPY, objcopy) AC_PATH_PROG(DOXYGEN, doxygen) -if test -z "$DOXYGEN"; then - AC_MSG_WARN([Doxygen not found]) -fi AM_CONDITIONAL([HAVE_DOXYGEN], [test -n "$DOXYGEN"]) AC_ARG_VAR(PYTHONPATH, Augments the default search path for python module files) diff --git a/doc/release-notes-16377.md b/doc/release-notes-16377.md new file mode 100644 index 0000000000..3442fa451b --- /dev/null +++ b/doc/release-notes-16377.md @@ -0,0 +1,9 @@ +RPC changes +----------- +- The `walletcreatefundedpsbt` RPC call will now fail with + `Insufficient funds` when inputs are manually selected but are not enough to cover + the outputs and fee. Additional inputs can automatically be added through the + new `add_inputs` option. + +- The `fundrawtransaction` RPC now supports `add_inputs` option that when `false` + prevents adding more inputs if necessary and consequently the RPC fails. diff --git a/doc/release-notes-19200.md b/doc/release-notes-19200.md new file mode 100644 index 0000000000..4670cb2e75 --- /dev/null +++ b/doc/release-notes-19200.md @@ -0,0 +1,7 @@ +## Wallet + +- Backwards compatibility has been dropped for two `getaddressinfo` RPC + deprecations, as notified in the 0.20 release notes. The deprecated `label` + field has been removed as well as the deprecated `labels` behavior of + returning a JSON object containing `name` and `purpose` key-value pairs. Since + 0.20, the `labels` field returns a JSON array of label names. (#19200) diff --git a/src/.clang-format b/src/.clang-format index aae039dd77..a8f8565f80 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -1,9 +1,10 @@ Language: Cpp AccessModifierOffset: -4 -AlignAfterOpenBracket: false +AlignAfterOpenBracket: true AlignEscapedNewlinesLeft: true AlignTrailingComments: true -AllowAllParametersOfDeclarationOnNextLine: false +AllowAllArgumentsOnNextLine : true +AllowAllParametersOfDeclarationOnNextLine: true AllowShortBlocksOnASingleLine: false AllowShortCaseLabelsOnASingleLine: true AllowShortFunctionsOnASingleLine: All diff --git a/src/Makefile.am b/src/Makefile.am index a33ff8a461..632ed3e31f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -184,6 +184,7 @@ BITCOIN_CORE_H = \ reverse_iterator.h \ rpc/blockchain.h \ rpc/client.h \ + rpc/mining.h \ rpc/protocol.h \ rpc/rawtransaction_util.h \ rpc/register.h \ diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 13bfea7646..1f66516172 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -25,6 +25,7 @@ QT_FORMS_UI = \ qt/forms/openuridialog.ui \ qt/forms/optionsdialog.ui \ qt/forms/overviewpage.ui \ + qt/forms/psbtoperationsdialog.ui \ qt/forms/receivecoinsdialog.ui \ qt/forms/receiverequestdialog.ui \ qt/forms/debugwindow.ui \ @@ -61,6 +62,7 @@ QT_MOC_CPP = \ qt/moc_overviewpage.cpp \ qt/moc_peertablemodel.cpp \ qt/moc_paymentserver.cpp \ + qt/moc_psbtoperationsdialog.cpp \ qt/moc_qrimagewidget.cpp \ qt/moc_qvalidatedlineedit.cpp \ qt/moc_qvaluecombobox.cpp \ @@ -132,6 +134,7 @@ BITCOIN_QT_H = \ qt/paymentserver.h \ qt/peertablemodel.h \ qt/platformstyle.h \ + qt/psbtoperationsdialog.h \ qt/qrimagewidget.h \ qt/qvalidatedlineedit.h \ qt/qvaluecombobox.h \ @@ -245,6 +248,7 @@ BITCOIN_QT_WALLET_CPP = \ qt/openuridialog.cpp \ qt/overviewpage.cpp \ qt/paymentserver.cpp \ + qt/psbtoperationsdialog.cpp \ qt/qrimagewidget.cpp \ qt/receivecoinsdialog.cpp \ qt/receiverequestdialog.cpp \ diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index d6d5e67c5b..f2d12531d7 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -31,7 +31,7 @@ static void CoinSelection(benchmark::State& state) { NodeContext node; auto chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); wallet.SetupLegacyScriptPubKeyMan(); std::vector<std::unique_ptr<CWalletTx>> wtxs; LOCK(wallet.cs_wallet); @@ -65,7 +65,7 @@ static void CoinSelection(benchmark::State& state) typedef std::set<CInputCoin> CoinSet; static NodeContext testNode; static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy()); +static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase()); std::vector<std::unique_ptr<CWalletTx>> wtxn; // Copied from src/wallet/test/coinselector_tests.cpp diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 810c344ab5..05cfb3438e 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -26,7 +26,7 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b NodeContext node; std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node); - CWallet wallet{chain.get(), WalletLocation(), WalletDatabase::CreateMock()}; + CWallet wallet{chain.get(), WalletLocation(), CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); bool first_run; diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 8d85789b4e..f5125f22db 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -11,6 +11,7 @@ #include <clientversion.h> #include <optional.h> #include <rpc/client.h> +#include <rpc/mining.h> #include <rpc/protocol.h> #include <rpc/request.h> #include <util/strencodings.h> @@ -39,6 +40,9 @@ static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; static const bool DEFAULT_NAMED=false; static const int CONTINUE_EXECUTION=-1; +/** Default number of blocks to generate for RPC generatetoaddress. */ +static const std::string DEFAULT_NBLOCKS = "1"; + static void SetupCliArgs() { SetupHelpOptions(gArgs); @@ -50,6 +54,7 @@ static void SetupCliArgs() gArgs.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + gArgs.AddArg("-generate", strprintf("Generate blocks immediately, equivalent to RPC generatenewaddress followed by RPC generatetoaddress. Optional positional integer arguments are number of blocks to generate (default: %s) and maximum iterations to try (default: %s), equivalent to RPC generatetoaddress nblocks and maxtries arguments. Example: bitcoin-cli -generate 4 1000", DEFAULT_NBLOCKS, DEFAULT_MAX_TRIES), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the results of -getinfo is the result of multiple non-atomic requests. Some entries in the result may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); SetupChainParamsBaseOptions(); gArgs.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -286,6 +291,28 @@ public: } }; +/** Process RPC generatetoaddress request. */ +class GenerateToAddressRequestHandler : public BaseRequestHandler +{ +public: + UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& args) override + { + address_str = args.at(1); + UniValue params{RPCConvertValues("generatetoaddress", args)}; + return JSONRPCRequestObj("generatetoaddress", params, 1); + } + + UniValue ProcessReply(const UniValue &reply) override + { + UniValue result(UniValue::VOBJ); + result.pushKV("address", address_str); + result.pushKV("blocks", reply.get_obj()["result"]); + return JSONRPCReplyObj(result, NullUniValue, 1); + } +protected: + std::string address_str; +}; + /** Process default single requests */ class DefaultRequestHandler: public BaseRequestHandler { public: @@ -453,6 +480,34 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str return response; } +/** Parse UniValue result to update the message to print to std::cout. */ +static void ParseResult(const UniValue& result, std::string& strPrint) +{ + if (result.isNull()) return; + strPrint = result.isStr() ? result.get_str() : result.write(2); +} + +/** Parse UniValue error to update the message to print to std::cerr and the code to return. */ +static void ParseError(const UniValue& error, std::string& strPrint, int& nRet) +{ + if (error.isObject()) { + const UniValue& err_code = find_value(error, "code"); + const UniValue& err_msg = find_value(error, "message"); + if (!err_code.isNull()) { + strPrint = "error code: " + err_code.getValStr() + "\n"; + } + if (err_msg.isStr()) { + strPrint += ("error message:\n" + err_msg.get_str()); + } + if (err_code.isNum() && err_code.get_int() == RPC_WALLET_NOT_SPECIFIED) { + strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line."; + } + } else { + strPrint = "error: " + error.write(); + } + nRet = abs(error["code"].get_int()); +} + /** * GetWalletBalances calls listwallets; if more than one wallet is loaded, it then * fetches mine.trusted balances for each loaded wallet and pushes them to `result`. @@ -477,6 +532,34 @@ static void GetWalletBalances(UniValue& result) result.pushKV("balances", balances); } +/** + * Call RPC getnewaddress. + * @returns getnewaddress response as a UniValue object. + */ +static UniValue GetNewAddress() +{ + Optional<std::string> wallet_name{}; + if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", ""); + std::unique_ptr<BaseRequestHandler> rh{MakeUnique<DefaultRequestHandler>()}; + return ConnectAndCallRPC(rh.get(), "getnewaddress", /* args=*/{}, wallet_name); +} + +/** + * Check bounds and set up args for RPC generatetoaddress params: nblocks, address, maxtries. + * @param[in] address Reference to const string address to insert into the args. + * @param args Reference to vector of string args to modify. + */ +static void SetGenerateToAddressArgs(const std::string& address, std::vector<std::string>& args) +{ + if (args.size() > 2) throw std::runtime_error("too many arguments (maximum 2 for nblocks and maxtries)"); + if (args.size() == 0) { + args.emplace_back(DEFAULT_NBLOCKS); + } else if (args.at(0) == "0") { + throw std::runtime_error("the first argument (number of blocks to generate, default: " + DEFAULT_NBLOCKS + ") must be an integer value greater than zero"); + } + args.emplace(args.begin() + 1, address); +} + static int CommandLineRPC(int argc, char *argv[]) { std::string strPrint; @@ -535,6 +618,15 @@ static int CommandLineRPC(int argc, char *argv[]) std::string method; if (gArgs.IsArgSet("-getinfo")) { rh.reset(new GetinfoRequestHandler()); + } else if (gArgs.GetBoolArg("-generate", false)) { + const UniValue getnewaddress{GetNewAddress()}; + const UniValue& error{find_value(getnewaddress, "error")}; + if (error.isNull()) { + SetGenerateToAddressArgs(find_value(getnewaddress, "result").get_str(), args); + rh.reset(new GenerateToAddressRequestHandler()); + } else { + ParseError(error, strPrint, nRet); + } } else { rh.reset(new DefaultRequestHandler()); if (args.size() < 1) { @@ -543,40 +635,22 @@ static int CommandLineRPC(int argc, char *argv[]) method = args[0]; args.erase(args.begin()); // Remove trailing method name from arguments vector } - Optional<std::string> wallet_name{}; - if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", ""); - const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name); - - // Parse reply - UniValue result = find_value(reply, "result"); - const UniValue& error = find_value(reply, "error"); - if (!error.isNull()) { - // Error - strPrint = "error: " + error.write(); - nRet = abs(error["code"].get_int()); - if (error.isObject()) { - const UniValue& errCode = find_value(error, "code"); - const UniValue& errMsg = find_value(error, "message"); - strPrint = errCode.isNull() ? "" : ("error code: " + errCode.getValStr() + "\n"); - - if (errMsg.isStr()) { - strPrint += ("error message:\n" + errMsg.get_str()); - } - if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) { - strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line."; + if (nRet == 0) { + // Perform RPC call + Optional<std::string> wallet_name{}; + if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", ""); + const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name); + + // Parse reply + UniValue result = find_value(reply, "result"); + const UniValue& error = find_value(reply, "error"); + if (error.isNull()) { + if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) { + GetWalletBalances(result); // fetch multiwallet balances and append to result } - } - } else { - if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) { - GetWalletBalances(result); // fetch multiwallet balances and append to result - } - // Result - if (result.isNull()) { - strPrint = ""; - } else if (result.isStr()) { - strPrint = result.get_str(); + ParseResult(result, strPrint); } else { - strPrint = result.write(2); + ParseError(error, strPrint, nRet); } } } catch (const std::exception& e) { diff --git a/src/init.cpp b/src/init.cpp index fd7c8d0f80..8d9566edc3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -152,6 +152,8 @@ NODISCARD static bool CreatePidFile() static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle; +static std::thread g_load_block; + static boost::thread_group threadGroup; void Interrupt(NodeContext& node) @@ -214,8 +216,9 @@ void Shutdown(NodeContext& node) StopTorControl(); // After everything has been shut down, but before things get flushed, stop the - // CScheduler/checkqueue threadGroup + // CScheduler/checkqueue, threadGroup and load block thread. if (node.scheduler) node.scheduler->stop(); + if (g_load_block.joinable()) g_load_block.join(); threadGroup.interrupt_all(); threadGroup.join_all(); @@ -680,7 +683,6 @@ static void CleanupBlockRevFiles() static void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles) { const CChainParams& chainparams = Params(); - util::ThreadRename("loadblk"); ScheduleBatchPriority(); { @@ -1842,7 +1844,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) vImportFiles.push_back(strFile); } - threadGroup.create_thread([=, &chainman] { ThreadImport(chainman, vImportFiles); }); + g_load_block = std::thread(&TraceThread<std::function<void()>>, "loadblk", [=, &chainman]{ ThreadImport(chainman, vImportFiles); }); // Wait for genesis block to be processed { diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 397403d308..b65eb72b1c 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -335,9 +335,10 @@ public: bool sign, bool bip32derivs, PartiallySignedTransaction& psbtx, - bool& complete) override + bool& complete, + size_t* n_signed) override { - return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs); + return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs, n_signed); } WalletBalances getBalances() override { diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 67569a3e55..e2161521f6 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -197,7 +197,8 @@ public: bool sign, bool bip32derivs, PartiallySignedTransaction& psbtx, - bool& complete) = 0; + bool& complete, + size_t* n_signed) = 0; //! Get balances. virtual WalletBalances getBalances() = 0; diff --git a/src/net.cpp b/src/net.cpp index f3b7e8dab1..371fbeed59 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -737,7 +737,7 @@ void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vec uint256 hash = Hash(msg.data.begin(), msg.data.end()); // create header - CMessageHeader hdr(Params().MessageStart(), msg.command.c_str(), msg.data.size()); + CMessageHeader hdr(Params().MessageStart(), msg.m_type.c_str(), msg.data.size()); memcpy(hdr.pchChecksum, hash.begin(), CMessageHeader::CHECKSUM_SIZE); // serialize header @@ -2794,7 +2794,7 @@ bool CConnman::NodeFullyConnected(const CNode* pnode) void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) { size_t nMessageSize = msg.data.size(); - LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", SanitizeString(msg.command), nMessageSize, pnode->GetId()); + LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", SanitizeString(msg.m_type), nMessageSize, pnode->GetId()); // make sure we use the appropriate network transport format std::vector<unsigned char> serializedHeader; @@ -2806,8 +2806,8 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) LOCK(pnode->cs_vSend); bool optimisticSend(pnode->vSendMsg.empty()); - //log total amount of bytes per command - pnode->mapSendBytesPerMsgCmd[msg.command] += nTotalSize; + //log total amount of bytes per message type + pnode->mapSendBytesPerMsgCmd[msg.m_type] += nTotalSize; pnode->nSendSize += nTotalSize; if (pnode->nSendSize > nSendBufferMaxSize) @@ -110,7 +110,7 @@ struct CSerializedNetMsg CSerializedNetMsg& operator=(const CSerializedNetMsg&) = delete; std::vector<unsigned char> data; - std::string command; + std::string m_type; }; @@ -973,19 +973,21 @@ public: } } - void PushInventory(const CInv& inv) + void PushTxInventory(const uint256& hash) { - if (inv.type == MSG_TX && m_tx_relay != nullptr) { - LOCK(m_tx_relay->cs_tx_inventory); - if (!m_tx_relay->filterInventoryKnown.contains(inv.hash)) { - m_tx_relay->setInventoryTxToSend.insert(inv.hash); - } - } else if (inv.type == MSG_BLOCK) { - LOCK(cs_inventory); - vInventoryBlockToSend.push_back(inv.hash); + if (m_tx_relay == nullptr) return; + LOCK(m_tx_relay->cs_tx_inventory); + if (!m_tx_relay->filterInventoryKnown.contains(hash)) { + m_tx_relay->setInventoryTxToSend.insert(hash); } } + void PushBlockInventory(const uint256& hash) + { + LOCK(cs_inventory); + vInventoryBlockToSend.push_back(hash); + } + void PushBlockHash(const uint256 &hash) { LOCK(cs_inventory); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index aa5c9668e3..270e415e42 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1419,10 +1419,9 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO void RelayTransaction(const uint256& txid, const CConnman& connman) { - CInv inv(MSG_TX, txid); - connman.ForEachNode([&inv](CNode* pnode) + connman.ForEachNode([&txid](CNode* pnode) { - pnode->PushInventory(inv); + pnode->PushTxInventory(txid); }); } @@ -1608,7 +1607,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // Trigger the peer node to send a getblocks request for the next batch of inventory if (inv.hash == pfrom.hashContinue) { - // Bypass PushInventory, this must send even if redundant, + // Bypass PushBlockInventory, this must send even if redundant, // and we want it right after the last block so they don't // wait for other stuff first. std::vector<CInv> vInv; @@ -1746,14 +1745,14 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); } -bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block) +static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block) { const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); size_t nCount = headers.size(); if (nCount == 0) { // Nothing interesting. Stop asking this peers for more headers. - return true; + return; } bool received_new_header = false; @@ -1786,14 +1785,14 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { Misbehaving(pfrom.GetId(), 20); } - return true; + return; } uint256 hashLastBlock; for (const CBlockHeader& header : headers) { if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) { Misbehaving(pfrom.GetId(), 20, "non-continuous headers sequence"); - return false; + return; } hashLastBlock = header.GetHash(); } @@ -1809,7 +1808,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan if (!chainman.ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); - return false; + return; } } @@ -1925,7 +1924,7 @@ bool static ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan } } - return true; + return; } void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uint256>& orphan_work_set, std::list<CTransactionRef>& removed_txn) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) @@ -2205,13 +2204,23 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa connman.PushMessage(&pfrom, std::move(msg)); } -bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc) +void ProcessMessage( + CNode& pfrom, + const std::string& msg_type, + CDataStream& vRecv, + int64_t nTimeReceived, + const CChainParams& chainparams, + ChainstateManager& chainman, + CTxMemPool& mempool, + CConnman* connman, + BanMan* banman, + const std::atomic<bool>& interruptMsgProc) { LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom.GetId()); if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0) { LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n"); - return true; + return; } @@ -2221,7 +2230,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LOCK(cs_main); Misbehaving(pfrom.GetId(), 1); - return false; + return; } int64_t nTime; @@ -2247,14 +2256,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices)); pfrom.fDisconnect = true; - return false; + return; } if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom.GetId(), nVersion); pfrom.fDisconnect = true; - return false; + return; } if (!vRecv.empty()) @@ -2274,7 +2283,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToString()); pfrom.fDisconnect = true; - return true; + return; } if (pfrom.fInbound && addrMe.IsRoutable()) @@ -2374,14 +2383,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec assert(pfrom.fInbound == false); pfrom.fDisconnect = true; } - return true; + return; } if (pfrom.nVersion == 0) { // Must have a version message before anything else LOCK(cs_main); Misbehaving(pfrom.GetId(), 1); - return false; + return; } // At this point, the outgoing message serialization version can't change. @@ -2422,14 +2431,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); } pfrom.fSuccessfullyConnected = true; - return true; + return; } if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); Misbehaving(pfrom.GetId(), 1); - return false; + return; } if (msg_type == NetMsgType::ADDR) { @@ -2438,15 +2447,15 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Don't want addr from older versions unless seeding if (pfrom.nVersion < CADDR_TIME_VERSION && connman->GetAddressCount() > 1000) - return true; + return; if (!pfrom.IsAddrRelayPeer()) { - return true; + return; } if (vAddr.size() > 1000) { LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("message addr size() = %u", vAddr.size())); - return false; + return; } // Store the new addresses @@ -2456,7 +2465,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec for (CAddress& addr : vAddr) { if (interruptMsgProc) - return true; + return; // We only bother storing full nodes, though this may include // things which we would not make an outbound connection to, in @@ -2483,13 +2492,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec pfrom.fGetAddr = false; if (pfrom.fOneShot) pfrom.fDisconnect = true; - return true; + return; } if (msg_type == NetMsgType::SENDHEADERS) { LOCK(cs_main); State(pfrom.GetId())->fPreferHeaders = true; - return true; + return; } if (msg_type == NetMsgType::SENDCMPCT) { @@ -2512,7 +2521,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 1); } } - return true; + return; } if (msg_type == NetMsgType::INV) { @@ -2522,7 +2531,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("message inv size() = %u", vInv.size())); - return false; + return; } // We won't accept tx inv's if we're in blocks-only mode, or this is a @@ -2542,7 +2551,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec for (CInv &inv : vInv) { if (interruptMsgProc) - return true; + return; bool fAlreadyHave = AlreadyHave(inv, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); @@ -2566,7 +2575,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (fBlocksOnly) { LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } else if (!fAlreadyHave && !fImporting && !fReindex && !::ChainstateActive().IsInitialBlockDownload()) { RequestTx(State(pfrom.GetId()), inv.hash, current_time); } @@ -2578,7 +2587,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom.GetId()); } - return true; + return; } if (msg_type == NetMsgType::GETDATA) { @@ -2588,7 +2597,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("message getdata size() = %u", vInv.size())); - return false; + return; } LogPrint(BCLog::NET, "received getdata (%u invsz) peer=%d\n", vInv.size(), pfrom.GetId()); @@ -2599,7 +2608,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec pfrom.vRecvGetData.insert(pfrom.vRecvGetData.end(), vInv.begin(), vInv.end()); ProcessGetData(pfrom, chainparams, connman, mempool, interruptMsgProc); - return true; + return; } if (msg_type == NetMsgType::GETBLOCKS) { @@ -2610,7 +2619,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (locator.vHave.size() > MAX_LOCATOR_SZ) { LogPrint(BCLog::NET, "getblocks locator size %lld > %d, disconnect peer=%d\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } // We might have announced the currently-being-connected tip using a @@ -2657,7 +2666,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); break; } - pfrom.PushInventory(CInv(MSG_BLOCK, pindex->GetBlockHash())); + pfrom.PushBlockInventory(pindex->GetBlockHash()); if (--nLimit <= 0) { // When this block is requested, we'll send an inv that'll @@ -2667,7 +2676,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec break; } } - return true; + return; } if (msg_type == NetMsgType::GETBLOCKTXN) { @@ -2683,7 +2692,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec } if (recent_block) { SendBlockTransactions(*recent_block, req, pfrom, connman); - return true; + return; } LOCK(cs_main); @@ -2691,7 +2700,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec const CBlockIndex* pindex = LookupBlockIndex(req.blockhash); if (!pindex || !(pindex->nStatus & BLOCK_HAVE_DATA)) { LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have\n", pfrom.GetId()); - return true; + return; } if (pindex->nHeight < ::ChainActive().Height() - MAX_BLOCKTXN_DEPTH) { @@ -2708,7 +2717,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec inv.hash = req.blockhash; pfrom.vRecvGetData.push_back(inv); // The message processing loop will go around again (without pausing) and we'll respond then (without cs_main) - return true; + return; } CBlock block; @@ -2716,7 +2725,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec assert(ret); SendBlockTransactions(block, req, pfrom, connman); - return true; + return; } if (msg_type == NetMsgType::GETHEADERS) { @@ -2727,13 +2736,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (locator.vHave.size() > MAX_LOCATOR_SZ) { LogPrint(BCLog::NET, "getheaders locator size %lld > %d, disconnect peer=%d\n", locator.vHave.size(), MAX_LOCATOR_SZ, pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } LOCK(cs_main); if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_NOBAN)) { LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom.GetId()); - return true; + return; } CNodeState *nodestate = State(pfrom.GetId()); @@ -2743,12 +2752,12 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // If locator is null, return the hashStop block pindex = LookupBlockIndex(hashStop); if (!pindex) { - return true; + return; } if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) { LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom.GetId()); - return true; + return; } } else @@ -2783,7 +2792,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // in the SendMessages logic. nodestate->pindexBestHeaderSent = pindex ? pindex : ::ChainActive().Tip(); connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); - return true; + return; } if (msg_type == NetMsgType::TX) { @@ -2794,7 +2803,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; - return true; + return; } CTransactionRef ptx; @@ -2925,7 +2934,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec state.ToString()); MaybePunishNodeForTx(pfrom.GetId(), state); } - return true; + return; } if (msg_type == NetMsgType::CMPCTBLOCK) @@ -2933,7 +2942,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Ignore cmpctblock received while importing if (fImporting || fReindex) { LogPrint(BCLog::NET, "Unexpected cmpctblock message received from peer %d\n", pfrom.GetId()); - return true; + return; } CBlockHeaderAndShortTxIDs cmpctblock; @@ -2948,7 +2957,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers if (!::ChainstateActive().IsInitialBlockDownload()) connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); - return true; + return; } if (!LookupBlockIndex(cmpctblock.header.GetHash())) { @@ -2961,7 +2970,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (!chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); - return true; + return; } } @@ -2999,7 +3008,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec bool fAlreadyInFlight = blockInFlightIt != mapBlocksInFlight.end(); if (pindex->nStatus & BLOCK_HAVE_DATA) // Nothing to do here - return true; + return; if (pindex->nChainWork <= ::ChainActive().Tip()->nChainWork || // We know something better pindex->nTx != 0) { // We had this block at some point, but pruned it @@ -3010,17 +3019,17 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); } - return true; + return; } // If we're not close to tip yet, give up and let parallel block fetch work its magic if (!fAlreadyInFlight && !CanDirectFetch(chainparams.GetConsensus())) - return true; + return; if (IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) { // Don't bother trying to process compact blocks from v1 peers // after segwit activates. - return true; + return; } // We want to be a bit conservative just to be extra careful about DoS @@ -3035,7 +3044,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec else { // The block was already in flight using compact blocks from the same peer LogPrint(BCLog::NET, "Peer sent us compact block we were already syncing!\n"); - return true; + return; } } @@ -3044,13 +3053,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom.GetId())); - return true; + return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it std::vector<CInv> vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); - return true; + return; } BlockTransactionsRequest req; @@ -3078,7 +3087,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec ReadStatus status = tempBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status != READ_STATUS_OK) { // TODO: don't ignore failures - return true; + return; } std::vector<CTransactionRef> dummy; status = tempBlock.FillBlock(*pblock, dummy); @@ -3093,7 +3102,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec std::vector<CInv> vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); - return true; + return; } else { // If this was an announce-cmpctblock, we want the same treatment as a header message fRevertToHeaderProcessing = true; @@ -3146,7 +3155,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec MarkBlockAsReceived(pblock->GetHash()); } } - return true; + return; } if (msg_type == NetMsgType::BLOCKTXN) @@ -3154,7 +3163,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Ignore blocktxn received while importing if (fImporting || fReindex) { LogPrint(BCLog::NET, "Unexpected blocktxn message received from peer %d\n", pfrom.GetId()); - return true; + return; } BlockTransactions resp; @@ -3169,7 +3178,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (it == mapBlocksInFlight.end() || !it->second.second->partialBlock || it->second.first != pfrom.GetId()) { LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); - return true; + return; } PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; @@ -3177,7 +3186,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId())); - return true; + return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( std::vector<CInv> invs; @@ -3228,7 +3237,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec mapBlockSource.erase(pblock->GetHash()); } } - return true; + return; } if (msg_type == NetMsgType::HEADERS) @@ -3236,7 +3245,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Ignore headers received while importing if (fImporting || fReindex) { LogPrint(BCLog::NET, "Unexpected headers message received from peer %d\n", pfrom.GetId()); - return true; + return; } std::vector<CBlockHeader> headers; @@ -3246,7 +3255,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (nCount > MAX_HEADERS_RESULTS) { LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount)); - return false; + return; } headers.resize(nCount); for (unsigned int n = 0; n < nCount; n++) { @@ -3262,7 +3271,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // Ignore block received while importing if (fImporting || fReindex) { LogPrint(BCLog::NET, "Unexpected block message received from peer %d\n", pfrom.GetId()); - return true; + return; } std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(); @@ -3290,7 +3299,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LOCK(cs_main); mapBlockSource.erase(pblock->GetHash()); } - return true; + return; } if (msg_type == NetMsgType::GETADDR) { @@ -3301,18 +3310,18 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // the getaddr message mitigates the attack. if (!pfrom.fInbound) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom.GetId()); - return true; + return; } if (!pfrom.IsAddrRelayPeer()) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from block-relay-only connection. peer=%d\n", pfrom.GetId()); - return true; + return; } // Only send one GetAddr response per connection to reduce resource waste // and discourage addr stamping of INV announcements. if (pfrom.fSentAddr) { LogPrint(BCLog::NET, "Ignoring repeated \"getaddr\". peer=%d\n", pfrom.GetId()); - return true; + return; } pfrom.fSentAddr = true; @@ -3324,7 +3333,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec pfrom.PushAddress(addr, insecure_rand); } } - return true; + return; } if (msg_type == NetMsgType::MEMPOOL) { @@ -3335,7 +3344,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; } - return true; + return; } if (connman->OutboundTargetReached(false) && !pfrom.HasPermission(PF_MEMPOOL)) @@ -3345,14 +3354,14 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; } - return true; + return; } if (pfrom.m_tx_relay != nullptr) { LOCK(pfrom.m_tx_relay->cs_tx_inventory); pfrom.m_tx_relay->fSendMempool = true; } - return true; + return; } if (msg_type == NetMsgType::PING) { @@ -3373,7 +3382,7 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec // return very quickly. connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); } - return true; + return; } if (msg_type == NetMsgType::PONG) { @@ -3429,13 +3438,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec if (bPingFinished) { pfrom.nPingNonceSent = 0; } - return true; + return; } if (msg_type == NetMsgType::FILTERLOAD) { if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { pfrom.fDisconnect = true; - return true; + return; } CBloomFilter filter; vRecv >> filter; @@ -3452,13 +3461,13 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec pfrom.m_tx_relay->pfilter.reset(new CBloomFilter(filter)); pfrom.m_tx_relay->fRelayTxes = true; } - return true; + return; } if (msg_type == NetMsgType::FILTERADD) { if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { pfrom.fDisconnect = true; - return true; + return; } std::vector<unsigned char> vData; vRecv >> vData; @@ -3480,21 +3489,21 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec LOCK(cs_main); Misbehaving(pfrom.GetId(), 100); } - return true; + return; } if (msg_type == NetMsgType::FILTERCLEAR) { if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { pfrom.fDisconnect = true; - return true; + return; } if (pfrom.m_tx_relay == nullptr) { - return true; + return; } LOCK(pfrom.m_tx_relay->cs_filter); pfrom.m_tx_relay->pfilter = nullptr; pfrom.m_tx_relay->fRelayTxes = true; - return true; + return; } if (msg_type == NetMsgType::FEEFILTER) { @@ -3507,22 +3516,22 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec } LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom.GetId()); } - return true; + return; } if (msg_type == NetMsgType::GETCFILTERS) { ProcessGetCFilters(pfrom, vRecv, chainparams, *connman); - return true; + return; } if (msg_type == NetMsgType::GETCFHEADERS) { ProcessGetCFHeaders(pfrom, vRecv, chainparams, *connman); - return true; + return; } if (msg_type == NetMsgType::GETCFCHECKPT) { ProcessGetCFCheckPt(pfrom, vRecv, chainparams, *connman); - return true; + return; } if (msg_type == NetMsgType::NOTFOUND) { @@ -3547,12 +3556,12 @@ bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRec } } } - return true; + return; } // Ignore unknown commands for extensibility LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom.GetId()); - return true; + return; } bool PeerLogicValidation::CheckIfBanned(CNode& pnode) @@ -3660,11 +3669,8 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter return fMoreWork; } - // Process message - bool fRet = false; - try - { - fRet = ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, connman, m_banman, interruptMsgProc); + try { + ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, connman, m_banman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) @@ -3675,10 +3681,6 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter LogPrint(BCLog::NET, "%s(%s, %u bytes): Unknown exception caught\n", __func__, SanitizeString(msg_type), nMessageSize); } - if (!fRet) { - LogPrint(BCLog::NET, "%s(%s, %u bytes) FAILED peer=%d\n", __func__, SanitizeString(msg_type), nMessageSize, pfrom->GetId()); - } - LOCK(cs_main); CheckIfBanned(*pfrom); @@ -4081,7 +4083,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // If the peer's chain has this block, don't inv it back. if (!PeerHasHeader(&state, pindex)) { - pto->PushInventory(CInv(MSG_BLOCK, hashToAnnounce)); + pto->PushBlockInventory(hashToAnnounce); LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__, pto->GetId(), hashToAnnounce.ToString()); } @@ -4385,9 +4387,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // // Message: feefilter // - // We don't want white listed peers to filter txs to us if we have -whitelistforcerelay if (pto->m_tx_relay != nullptr && pto->nVersion >= FEEFILTER_VERSION && gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && - !pto->HasPermission(PF_FORCERELAY)) { + !pto->HasPermission(PF_FORCERELAY) // peers with the forcerelay permission should not filter txs to us + ) { CAmount currentFilter = m_mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); int64_t timeNow = GetTimeMicros(); if (timeNow > pto->m_tx_relay->nextSendTimeFeeFilter) { diff --git a/src/netmessagemaker.h b/src/netmessagemaker.h index 2efb384a7b..ffb3fe2f29 100644 --- a/src/netmessagemaker.h +++ b/src/netmessagemaker.h @@ -15,18 +15,18 @@ public: explicit CNetMsgMaker(int nVersionIn) : nVersion(nVersionIn){} template <typename... Args> - CSerializedNetMsg Make(int nFlags, std::string sCommand, Args&&... args) const + CSerializedNetMsg Make(int nFlags, std::string msg_type, Args&&... args) const { CSerializedNetMsg msg; - msg.command = std::move(sCommand); + msg.m_type = std::move(msg_type); CVectorWriter{ SER_NETWORK, nFlags | nVersion, msg.data, 0, std::forward<Args>(args)... }; return msg; } template <typename... Args> - CSerializedNetMsg Make(std::string sCommand, Args&&... args) const + CSerializedNetMsg Make(std::string msg_type, Args&&... args) const { - return Make(0, std::move(sCommand), std::forward<Args>(args)...); + return Make(0, std::move(msg_type), std::forward<Args>(args)...); } private: diff --git a/src/outputtype.cpp b/src/outputtype.cpp index ea7a86d6d6..871474d56e 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -53,7 +53,7 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) case OutputType::P2SH_SEGWIT: case OutputType::BECH32: { if (!key.IsCompressed()) return PKHash(key); - CTxDestination witdest = WitnessV0KeyHash(PKHash(key)); + CTxDestination witdest = WitnessV0KeyHash(key); CScript witprog = GetScriptForDestination(witdest); if (type == OutputType::P2SH_SEGWIT) { return ScriptHash(witprog); diff --git a/src/psbt.cpp b/src/psbt.cpp index ef9781817a..10260740f0 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -214,6 +214,17 @@ bool PSBTInputSigned(const PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } +size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt) { + size_t count = 0; + for (const auto& input : psbt.inputs) { + if (!PSBTInputSigned(input)) { + count++; + } + } + + return count; +} + void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index) { const CTxOut& out = psbt.tx->vout.at(index); diff --git a/src/psbt.h b/src/psbt.h index 888e0fd119..0a8ea2ea0b 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -579,6 +579,9 @@ bool PSBTInputSigned(const PSBTInput& input); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false); +/** Counts the unsigned inputs of a PSBT. */ +size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt); + /** Updates a PSBTOutput with information from provider. * * This fills in the redeem_script, witness_script, and hd_keypaths where possible. diff --git a/src/qt/bitcoinamountfield.cpp b/src/qt/bitcoinamountfield.cpp index 4c57f1e352..a953c991bc 100644 --- a/src/qt/bitcoinamountfield.cpp +++ b/src/qt/bitcoinamountfield.cpp @@ -56,7 +56,7 @@ public: if (valid) { val = qBound(m_min_amount, val, m_max_amount); - input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::separatorAlways); + input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::SeparatorStyle::ALWAYS); lineEdit()->setText(input); } } @@ -68,7 +68,7 @@ public: void setValue(const CAmount& value) { - lineEdit()->setText(BitcoinUnits::format(currentUnit, value, false, BitcoinUnits::separatorAlways)); + lineEdit()->setText(BitcoinUnits::format(currentUnit, value, false, BitcoinUnits::SeparatorStyle::ALWAYS)); Q_EMIT valueChanged(); } @@ -102,7 +102,7 @@ public: CAmount val = value(&valid); currentUnit = unit; - lineEdit()->setPlaceholderText(BitcoinUnits::format(currentUnit, m_min_amount, false, BitcoinUnits::separatorAlways)); + lineEdit()->setPlaceholderText(BitcoinUnits::format(currentUnit, m_min_amount, false, BitcoinUnits::SeparatorStyle::ALWAYS)); if(valid) setValue(val); else @@ -122,7 +122,7 @@ public: const QFontMetrics fm(fontMetrics()); int h = lineEdit()->minimumSizeHint().height(); - int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::separatorAlways)); + int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::SeparatorStyle::ALWAYS)); w += 2; // cursor blinking space QStyleOptionSpinBox opt; diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 1092cdd754..b4182e8524 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -321,8 +321,10 @@ void BitcoinGUI::createActions() signMessageAction->setStatusTip(tr("Sign messages with your Bitcoin addresses to prove you own them")); verifyMessageAction = new QAction(tr("&Verify message..."), this); verifyMessageAction->setStatusTip(tr("Verify messages to ensure they were signed with specified Bitcoin addresses")); - m_load_psbt_action = new QAction(tr("Load PSBT..."), this); + m_load_psbt_action = new QAction(tr("&Load PSBT from file..."), this); m_load_psbt_action->setStatusTip(tr("Load Partially Signed Bitcoin Transaction")); + m_load_psbt_clipboard_action = new QAction(tr("Load PSBT from clipboard..."), this); + m_load_psbt_clipboard_action->setStatusTip(tr("Load Partially Signed Bitcoin Transaction from clipboard")); openRPCConsoleAction = new QAction(tr("Node window"), this); openRPCConsoleAction->setStatusTip(tr("Open node debugging and diagnostic console")); @@ -381,6 +383,7 @@ void BitcoinGUI::createActions() connect(signMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(signMessageAction, &QAction::triggered, [this]{ gotoSignMessageTab(); }); connect(m_load_psbt_action, &QAction::triggered, [this]{ gotoLoadPSBT(); }); + connect(m_load_psbt_clipboard_action, &QAction::triggered, [this]{ gotoLoadPSBT(true); }); connect(verifyMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); }); connect(verifyMessageAction, &QAction::triggered, [this]{ gotoVerifyMessageTab(); }); connect(usedSendingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedSendingAddresses); @@ -459,6 +462,7 @@ void BitcoinGUI::createMenuBar() file->addAction(signMessageAction); file->addAction(verifyMessageAction); file->addAction(m_load_psbt_action); + file->addAction(m_load_psbt_clipboard_action); file->addSeparator(); } file->addAction(quitAction); @@ -878,9 +882,9 @@ void BitcoinGUI::gotoVerifyMessageTab(QString addr) { if (walletFrame) walletFrame->gotoVerifyMessageTab(addr); } -void BitcoinGUI::gotoLoadPSBT() +void BitcoinGUI::gotoLoadPSBT(bool from_clipboard) { - if (walletFrame) walletFrame->gotoLoadPSBT(); + if (walletFrame) walletFrame->gotoLoadPSBT(from_clipboard); } #endif // ENABLE_WALLET diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index b009e279b6..697e83e772 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -139,6 +139,7 @@ private: QAction* signMessageAction = nullptr; QAction* verifyMessageAction = nullptr; QAction* m_load_psbt_action = nullptr; + QAction* m_load_psbt_clipboard_action = nullptr; QAction* aboutAction = nullptr; QAction* receiveCoinsAction = nullptr; QAction* receiveCoinsMenuAction = nullptr; @@ -278,8 +279,8 @@ public Q_SLOTS: void gotoSignMessageTab(QString addr = ""); /** Show Sign/Verify Message dialog and switch to verify message tab */ void gotoVerifyMessageTab(QString addr = ""); - /** Show load Partially Signed Bitcoin Transaction dialog */ - void gotoLoadPSBT(); + /** Load Partially Signed Bitcoin Transaction from file or clipboard */ + void gotoLoadPSBT(bool from_clipboard = false); /** Show open dialog */ void openClicked(); diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 318a6dcbfd..fd55c547fc 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -114,7 +114,7 @@ QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, Separator // confused with the decimal marker. QChar thin_sp(THIN_SP_CP); int q_size = quotient_str.size(); - if (separators == separatorAlways || (separators == separatorStandard && q_size > 4)) + if (separators == SeparatorStyle::ALWAYS || (separators == SeparatorStyle::STANDARD && q_size > 4)) for (int i = 3; i < q_size; i += 3) quotient_str.insert(q_size - i, thin_sp); diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index dac5484393..e22ba0a938 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -46,11 +46,11 @@ public: SAT }; - enum SeparatorStyle + enum class SeparatorStyle { - separatorNever, - separatorStandard, - separatorAlways + NEVER, + STANDARD, + ALWAYS }; //! @name Static API @@ -72,11 +72,11 @@ public: //! Number of decimals left static int decimals(int unit); //! Format as string - static QString format(int unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = separatorStandard, bool justify = false); + static QString format(int unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = SeparatorStyle::STANDARD, bool justify = false); //! Format as string (with unit) - static QString formatWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=separatorStandard); + static QString formatWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=SeparatorStyle::STANDARD); //! Format as HTML string (with unit) - static QString formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=separatorStandard); + static QString formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=SeparatorStyle::STANDARD); //! Format as string (with unit) of fixed length to preserve privacy, if it is set. static QString formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy); //! Parse string to coin amount diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index f44a9f285a..7c72858501 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -456,7 +456,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * { CPubKey pubkey; PKHash *pkhash = boost::get<PKHash>(&address); - if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey)) + if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, ToKeyID(*pkhash), pubkey)) { nBytesInputs += (pubkey.IsCompressed() ? 148 : 180); } diff --git a/src/qt/forms/psbtoperationsdialog.ui b/src/qt/forms/psbtoperationsdialog.ui new file mode 100644 index 0000000000..c2e2f5035b --- /dev/null +++ b/src/qt/forms/psbtoperationsdialog.ui @@ -0,0 +1,148 @@ +<?xml version="1.0" encoding="UTF-8"?> +<ui version="4.0"> + <class>PSBTOperationsDialog</class> + <widget class="QDialog" name="PSBTOperationsDialog"> + <property name="geometry"> + <rect> + <x>0</x> + <y>0</y> + <width>585</width> + <height>327</height> + </rect> + </property> + <property name="windowTitle"> + <string>Dialog</string> + </property> + <layout class="QVBoxLayout" name="verticalLayout"> + <property name="spacing"> + <number>12</number> + </property> + <property name="sizeConstraint"> + <enum>QLayout::SetDefaultConstraint</enum> + </property> + <property name="bottomMargin"> + <number>12</number> + </property> + <item> + <layout class="QVBoxLayout" name="mainDialogLayout"> + <property name="spacing"> + <number>5</number> + </property> + <property name="topMargin"> + <number>0</number> + </property> + <property name="bottomMargin"> + <number>0</number> + </property> + <item> + <widget class="QLabel" name="statusBar"> + <property name="font"> + <font> + <weight>75</weight> + <bold>true</bold> + </font> + </property> + <property name="autoFillBackground"> + <bool>false</bool> + </property> + <property name="styleSheet"> + <string notr="true"/> + </property> + <property name="text"> + <string/> + </property> + </widget> + </item> + <item> + <widget class="QTextEdit" name="transactionDescription"> + <property name="undoRedoEnabled"> + <bool>false</bool> + </property> + <property name="readOnly"> + <bool>true</bool> + </property> + </widget> + </item> + <item> + <layout class="QHBoxLayout" name="buttonRowLayout"> + <property name="spacing"> + <number>5</number> + </property> + <item> + <widget class="QPushButton" name="signTransactionButton"> + <property name="sizePolicy"> + <sizepolicy hsizetype="Minimum" vsizetype="Fixed"> + <horstretch>0</horstretch> + <verstretch>0</verstretch> + </sizepolicy> + </property> + <property name="font"> + <font> + <weight>50</weight> + <bold>false</bold> + </font> + </property> + <property name="text"> + <string>Sign Tx</string> + </property> + <property name="autoDefault"> + <bool>true</bool> + </property> + <property name="default"> + <bool>false</bool> + </property> + <property name="flat"> + <bool>false</bool> + </property> + </widget> + </item> + <item> + <widget class="QPushButton" name="broadcastTransactionButton"> + <property name="text"> + <string>Broadcast Tx</string> + </property> + </widget> + </item> + <item> + <spacer name="horizontalSpacer"> + <property name="orientation"> + <enum>Qt::Horizontal</enum> + </property> + <property name="sizeHint" stdset="0"> + <size> + <width>40</width> + <height>20</height> + </size> + </property> + </spacer> + </item> + <item> + <widget class="QPushButton" name="copyToClipboardButton"> + <property name="text"> + <string>Copy to Clipboard</string> + </property> + </widget> + </item> + <item> + <widget class="QPushButton" name="saveButton"> + <property name="text"> + <string>Save...</string> + </property> + </widget> + </item> + <item> + <widget class="QPushButton" name="closeButton"> + <property name="text"> + <string>Close</string> + </property> + </widget> + </item> + </layout> + </item> + </layout> + </item> + </layout> + </widget> + <resources/> + <connections/> +</ui> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index ce44d4f3a5..3cadac2f2f 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -188,7 +188,7 @@ QString formatBitcoinURI(const SendCoinsRecipient &info) if (info.amount) { - ret += QString("?amount=%1").arg(BitcoinUnits::format(BitcoinUnits::BTC, info.amount, false, BitcoinUnits::separatorNever)); + ret += QString("?amount=%1").arg(BitcoinUnits::format(BitcoinUnits::BTC, info.amount, false, BitcoinUnits::SeparatorStyle::NEVER)); paramCount++; } diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 0af70f2735..b536567c8b 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -88,7 +88,7 @@ public: foreground = option.palette.color(QPalette::Text); } painter->setPen(foreground); - QString amountText = BitcoinUnits::formatWithUnit(unit, amount, true, BitcoinUnits::separatorAlways); + QString amountText = BitcoinUnits::formatWithUnit(unit, amount, true, BitcoinUnits::SeparatorStyle::ALWAYS); if(!confirmed) { amountText = QString("[") + amountText + QString("]"); @@ -180,25 +180,25 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances) m_balances = balances; if (walletModel->wallet().isLegacy()) { if (walletModel->wallet().privateKeysDisabled()) { - ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); } else { - ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelWatchAvailable->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelWatchPending->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelWatchImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelWatchTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelWatchAvailable->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelWatchPending->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelWatchImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelWatchTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.watch_only_balance + balances.unconfirmed_watch_only_balance + balances.immature_watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); } } else { - ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); - ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::separatorAlways, m_privacy)); + ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); + ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); } // only show immature (newly mined) balance if it's non-zero, so as not to complicate things // for the non-mining users diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp new file mode 100644 index 0000000000..58167d4bb4 --- /dev/null +++ b/src/qt/psbtoperationsdialog.cpp @@ -0,0 +1,268 @@ +// Copyright (c) 2011-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <qt/psbtoperationsdialog.h> + +#include <core_io.h> +#include <interfaces/node.h> +#include <key_io.h> +#include <node/psbt.h> +#include <policy/policy.h> +#include <qt/bitcoinunits.h> +#include <qt/forms/ui_psbtoperationsdialog.h> +#include <qt/guiutil.h> +#include <qt/optionsmodel.h> +#include <util/strencodings.h> + +#include <iostream> + + +PSBTOperationsDialog::PSBTOperationsDialog( + QWidget* parent, WalletModel* wallet_model, ClientModel* client_model) : QDialog(parent), + m_ui(new Ui::PSBTOperationsDialog), + m_wallet_model(wallet_model), + m_client_model(client_model) +{ + m_ui->setupUi(this); + setWindowTitle("PSBT Operations"); + + connect(m_ui->signTransactionButton, &QPushButton::clicked, this, &PSBTOperationsDialog::signTransaction); + connect(m_ui->broadcastTransactionButton, &QPushButton::clicked, this, &PSBTOperationsDialog::broadcastTransaction); + connect(m_ui->copyToClipboardButton, &QPushButton::clicked, this, &PSBTOperationsDialog::copyToClipboard); + connect(m_ui->saveButton, &QPushButton::clicked, this, &PSBTOperationsDialog::saveTransaction); + + connect(m_ui->closeButton, &QPushButton::clicked, this, &PSBTOperationsDialog::close); + + m_ui->signTransactionButton->setEnabled(false); + m_ui->broadcastTransactionButton->setEnabled(false); +} + +PSBTOperationsDialog::~PSBTOperationsDialog() +{ + delete m_ui; +} + +void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx) +{ + m_transaction_data = psbtx; + + bool complete; + size_t n_could_sign; + FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness. + TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, m_transaction_data, complete, &n_could_sign); + if (err != TransactionError::OK) { + showStatus(tr("Failed to load transaction: %1") + .arg(QString::fromStdString(TransactionErrorString(err).translated)), StatusLevel::ERR); + return; + } + + m_ui->broadcastTransactionButton->setEnabled(complete); + m_ui->signTransactionButton->setEnabled(!complete && !m_wallet_model->wallet().privateKeysDisabled() && n_could_sign > 0); + + updateTransactionDisplay(); +} + +void PSBTOperationsDialog::signTransaction() +{ + bool complete; + size_t n_signed; + TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, true /* sign */, true /* bip32derivs */, m_transaction_data, complete, &n_signed); + + if (err != TransactionError::OK) { + showStatus(tr("Failed to sign transaction: %1") + .arg(QString::fromStdString(TransactionErrorString(err).translated)), StatusLevel::ERR); + return; + } + + updateTransactionDisplay(); + + if (!complete && n_signed < 1) { + showStatus(tr("Could not sign any more inputs."), StatusLevel::WARN); + } else if (!complete) { + showStatus(tr("Signed %1 inputs, but more signatures are still required.").arg(n_signed), + StatusLevel::INFO); + } else { + showStatus(tr("Signed transaction successfully. Transaction is ready to broadcast."), + StatusLevel::INFO); + m_ui->broadcastTransactionButton->setEnabled(true); + } +} + +void PSBTOperationsDialog::broadcastTransaction() +{ + CMutableTransaction mtx; + if (!FinalizeAndExtractPSBT(m_transaction_data, mtx)) { + // This is never expected to fail unless we were given a malformed PSBT + // (e.g. with an invalid signature.) + showStatus(tr("Unknown error processing transaction."), StatusLevel::ERR); + return; + } + + CTransactionRef tx = MakeTransactionRef(mtx); + std::string err_string; + TransactionError error = BroadcastTransaction( + *m_client_model->node().context(), tx, err_string, DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK(), /* relay */ true, /* await_callback */ false); + + if (error == TransactionError::OK) { + showStatus(tr("Transaction broadcast successfully! Transaction ID: %1") + .arg(QString::fromStdString(tx->GetHash().GetHex())), StatusLevel::INFO); + } else { + showStatus(tr("Transaction broadcast failed: %1") + .arg(QString::fromStdString(TransactionErrorString(error).translated)), StatusLevel::ERR); + } +} + +void PSBTOperationsDialog::copyToClipboard() { + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); + ssTx << m_transaction_data; + GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str()); + showStatus(tr("PSBT copied to clipboard."), StatusLevel::INFO); +} + +void PSBTOperationsDialog::saveTransaction() { + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); + ssTx << m_transaction_data; + + QString selected_filter; + QString filename_suggestion = ""; + bool first = true; + for (const CTxOut& out : m_transaction_data.tx->vout) { + if (!first) { + filename_suggestion.append("-"); + } + CTxDestination address; + ExtractDestination(out.scriptPubKey, address); + QString amount = BitcoinUnits::format(m_wallet_model->getOptionsModel()->getDisplayUnit(), out.nValue); + QString address_str = QString::fromStdString(EncodeDestination(address)); + filename_suggestion.append(address_str + "-" + amount); + first = false; + } + filename_suggestion.append(".psbt"); + QString filename = GUIUtil::getSaveFileName(this, + tr("Save Transaction Data"), filename_suggestion, + tr("Partially Signed Transaction (Binary) (*.psbt)"), &selected_filter); + if (filename.isEmpty()) { + return; + } + std::ofstream out(filename.toLocal8Bit().data()); + out << ssTx.str(); + out.close(); + showStatus(tr("PSBT saved to disk."), StatusLevel::INFO); +} + +void PSBTOperationsDialog::updateTransactionDisplay() { + m_ui->transactionDescription->setText(QString::fromStdString(renderTransaction(m_transaction_data))); + showTransactionStatus(m_transaction_data); +} + +std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransaction &psbtx) +{ + QString tx_description = ""; + CAmount totalAmount = 0; + for (const CTxOut& out : psbtx.tx->vout) { + CTxDestination address; + ExtractDestination(out.scriptPubKey, address); + totalAmount += out.nValue; + tx_description.append(tr(" * Sends %1 to %2") + .arg(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, out.nValue)) + .arg(QString::fromStdString(EncodeDestination(address)))); + tx_description.append("<br>"); + } + + PSBTAnalysis analysis = AnalyzePSBT(psbtx); + tx_description.append(" * "); + if (!*analysis.fee) { + // This happens if the transaction is missing input UTXO information. + tx_description.append(tr("Unable to calculate transaction fee or total transaction amount.")); + } else { + tx_description.append(tr("Pays transaction fee: ")); + tx_description.append(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, *analysis.fee)); + + // add total amount in all subdivision units + tx_description.append("<hr />"); + QStringList alternativeUnits; + for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) + { + if(u != m_client_model->getOptionsModel()->getDisplayUnit()) { + alternativeUnits.append(BitcoinUnits::formatHtmlWithUnit(u, totalAmount)); + } + } + tx_description.append(QString("<b>%1</b>: <b>%2</b>").arg(tr("Total Amount")) + .arg(BitcoinUnits::formatHtmlWithUnit(m_client_model->getOptionsModel()->getDisplayUnit(), totalAmount))); + tx_description.append(QString("<br /><span style='font-size:10pt; font-weight:normal;'>(=%1)</span>") + .arg(alternativeUnits.join(" " + tr("or") + " "))); + } + + size_t num_unsigned = CountPSBTUnsignedInputs(psbtx); + if (num_unsigned > 0) { + tx_description.append("<br><br>"); + tx_description.append(tr("Transaction has %1 unsigned inputs.").arg(QString::number(num_unsigned))); + } + + return tx_description.toStdString(); +} + +void PSBTOperationsDialog::showStatus(const QString &msg, StatusLevel level) { + m_ui->statusBar->setText(msg); + switch (level) { + case StatusLevel::INFO: { + m_ui->statusBar->setStyleSheet("QLabel { background-color : lightgreen }"); + break; + } + case StatusLevel::WARN: { + m_ui->statusBar->setStyleSheet("QLabel { background-color : orange }"); + break; + } + case StatusLevel::ERR: { + m_ui->statusBar->setStyleSheet("QLabel { background-color : red }"); + break; + } + } + m_ui->statusBar->show(); +} + +size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &psbtx) { + size_t n_signed; + bool complete; + TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, false /* bip32derivs */, m_transaction_data, complete, &n_signed); + + if (err != TransactionError::OK) { + return 0; + } + return n_signed; +} + +void PSBTOperationsDialog::showTransactionStatus(const PartiallySignedTransaction &psbtx) { + PSBTAnalysis analysis = AnalyzePSBT(psbtx); + size_t n_could_sign = couldSignInputs(psbtx); + + switch (analysis.next) { + case PSBTRole::UPDATER: { + showStatus(tr("Transaction is missing some information about inputs."), StatusLevel::WARN); + break; + } + case PSBTRole::SIGNER: { + QString need_sig_text = tr("Transaction still needs signature(s)."); + StatusLevel level = StatusLevel::INFO; + if (m_wallet_model->wallet().privateKeysDisabled()) { + need_sig_text += " " + tr("(But this wallet cannot sign transactions.)"); + level = StatusLevel::WARN; + } else if (n_could_sign < 1) { + need_sig_text += " " + tr("(But this wallet does not have the right keys.)"); // XXX wording + level = StatusLevel::WARN; + } + showStatus(need_sig_text, level); + break; + } + case PSBTRole::FINALIZER: + case PSBTRole::EXTRACTOR: { + showStatus(tr("Transaction is fully signed and ready for broadcast."), StatusLevel::INFO); + break; + } + default: { + showStatus(tr("Transaction status is unknown."), StatusLevel::ERR); + break; + } + } +} diff --git a/src/qt/psbtoperationsdialog.h b/src/qt/psbtoperationsdialog.h new file mode 100644 index 0000000000..f37bdbe39a --- /dev/null +++ b/src/qt/psbtoperationsdialog.h @@ -0,0 +1,54 @@ +// Copyright (c) 2011-2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_QT_PSBTOPERATIONSDIALOG_H +#define BITCOIN_QT_PSBTOPERATIONSDIALOG_H + +#include <QDialog> + +#include <psbt.h> +#include <qt/clientmodel.h> +#include <qt/walletmodel.h> + +namespace Ui { +class PSBTOperationsDialog; +} + +/** Dialog showing transaction details. */ +class PSBTOperationsDialog : public QDialog +{ + Q_OBJECT + +public: + explicit PSBTOperationsDialog(QWidget* parent, WalletModel* walletModel, ClientModel* clientModel); + ~PSBTOperationsDialog(); + + void openWithPSBT(PartiallySignedTransaction psbtx); + +public Q_SLOTS: + void signTransaction(); + void broadcastTransaction(); + void copyToClipboard(); + void saveTransaction(); + +private: + Ui::PSBTOperationsDialog* m_ui; + PartiallySignedTransaction m_transaction_data; + WalletModel* m_wallet_model; + ClientModel* m_client_model; + + enum class StatusLevel { + INFO, + WARN, + ERR + }; + + size_t couldSignInputs(const PartiallySignedTransaction &psbtx); + void updateTransactionDisplay(); + std::string renderTransaction(const PartiallySignedTransaction &psbtx); + void showStatus(const QString &msg, StatusLevel level); + void showTransactionStatus(const PartiallySignedTransaction &psbtx); +}; + +#endif // BITCOIN_QT_PSBTOPERATIONSDIALOG_H diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp index 7419297a96..3e20368a36 100644 --- a/src/qt/recentrequeststablemodel.cpp +++ b/src/qt/recentrequeststablemodel.cpp @@ -82,7 +82,7 @@ QVariant RecentRequestsTableModel::data(const QModelIndex &index, int role) cons if (rec->recipient.amount == 0 && role == Qt::DisplayRole) return tr("(no amount requested)"); else if (role == Qt::EditRole) - return BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), rec->recipient.amount, false, BitcoinUnits::separatorNever); + return BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), rec->recipient.amount, false, BitcoinUnits::SeparatorStyle::NEVER); else return BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), rec->recipient.amount); } diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 9e23fe78d8..0ac61f3adc 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -392,7 +392,7 @@ void SendCoinsDialog::on_sendButton_clicked() CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())}; PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete); + const TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete, nullptr); assert(!complete); assert(err == TransactionError::OK); // Serialize the PSBT diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 476128520c..9347ff9e42 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -59,7 +59,7 @@ void EditAddressAndSubmit( void TestAddAddressesToSendBook(interfaces::Node& node) { TestChain100Setup test; - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 8da0250e57..6648029bae 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -140,7 +140,7 @@ void TestGUI(interfaces::Node& node) } node.context()->connman = std::move(test.m_node.connman); node.context()->mempool = std::move(test.m_node.mempool); - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); { @@ -178,7 +178,7 @@ void TestGUI(interfaces::Node& node) QString balanceText = balanceLabel->text(); int unit = walletModel.getOptionsModel()->getDisplayUnit(); CAmount balance = walletModel.wallet().getBalance(); - QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::separatorAlways); + QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::SeparatorStyle::ALWAYS); QCOMPARE(balanceText, balanceComparison); } @@ -204,7 +204,7 @@ void TestGUI(interfaces::Node& node) QString balanceText = balanceLabel->text().trimmed(); int unit = walletModel.getOptionsModel()->getDisplayUnit(); CAmount balance = walletModel.wallet().getBalance(); - QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::separatorAlways); + QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::SeparatorStyle::ALWAYS); QCOMPARE(balanceText, balanceComparison); // Check Request Payment button diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 04eb1ae706..327a489488 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -524,7 +524,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case ToAddress: return formatTxToAddress(rec, false); case Amount: - return formatTxAmount(rec, true, BitcoinUnits::separatorAlways); + return formatTxAmount(rec, true, BitcoinUnits::SeparatorStyle::ALWAYS); } break; case Qt::EditRole: @@ -614,14 +614,14 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const details.append(QString::fromStdString(rec->address)); details.append(" "); } - details.append(formatTxAmount(rec, false, BitcoinUnits::separatorNever)); + details.append(formatTxAmount(rec, false, BitcoinUnits::SeparatorStyle::NEVER)); return details; } case ConfirmedRole: return rec->status.status == TransactionStatus::Status::Confirming || rec->status.status == TransactionStatus::Status::Confirmed; case FormattedAmountRole: // Used for copy/export, so don't include separators - return formatTxAmount(rec, false, BitcoinUnits::separatorNever); + return formatTxAmount(rec, false, BitcoinUnits::SeparatorStyle::NEVER); case StatusRole: return rec->status.status; } diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index f06f0ea15f..4b699d4d7d 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -101,7 +101,7 @@ private: QString formatTxDate(const TransactionRecord *wtx) const; QString formatTxType(const TransactionRecord *wtx) const; QString formatTxToAddress(const TransactionRecord *wtx, bool tooltip) const; - QString formatTxAmount(const TransactionRecord *wtx, bool showUnconfirmed=true, BitcoinUnits::SeparatorStyle separators=BitcoinUnits::separatorStandard) const; + QString formatTxAmount(const TransactionRecord *wtx, bool showUnconfirmed=true, BitcoinUnits::SeparatorStyle separators=BitcoinUnits::SeparatorStyle::STANDARD) const; QString formatTooltip(const TransactionRecord *rec) const; QVariant txStatusDecoration(const TransactionRecord *wtx) const; QVariant txWatchonlyDecoration(const TransactionRecord *wtx) const; diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 5e68ee4f93..ec56f2755f 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -165,11 +165,11 @@ void WalletFrame::gotoVerifyMessageTab(QString addr) walletView->gotoVerifyMessageTab(addr); } -void WalletFrame::gotoLoadPSBT() +void WalletFrame::gotoLoadPSBT(bool from_clipboard) { WalletView *walletView = currentWalletView(); if (walletView) { - walletView->gotoLoadPSBT(); + walletView->gotoLoadPSBT(from_clipboard); } } diff --git a/src/qt/walletframe.h b/src/qt/walletframe.h index d90ade5005..2b5f263468 100644 --- a/src/qt/walletframe.h +++ b/src/qt/walletframe.h @@ -79,7 +79,7 @@ public Q_SLOTS: void gotoVerifyMessageTab(QString addr = ""); /** Load Partially Signed Bitcoin Transaction */ - void gotoLoadPSBT(); + void gotoLoadPSBT(bool from_clipboard = false); /** Encrypt the wallet */ void encryptWallet(bool status); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 671b5e1ce6..72c75f7be0 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -536,7 +536,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) if (create_psbt) { PartiallySignedTransaction psbtx(mtx); bool complete = false; - const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete); + const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete, nullptr); if (err != TransactionError::OK || complete) { QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Can't draft transaction.")); return false; diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 861d1c5f4a..a3f303aea6 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -11,6 +11,7 @@ #include <qt/askpassphrasedialog.h> #include <qt/clientmodel.h> #include <qt/guiutil.h> +#include <qt/psbtoperationsdialog.h> #include <qt/optionsmodel.h> #include <qt/overviewpage.h> #include <qt/platformstyle.h> @@ -27,6 +28,8 @@ #include <QAction> #include <QActionGroup> +#include <QApplication> +#include <QClipboard> #include <QFileDialog> #include <QHBoxLayout> #include <QProgressDialog> @@ -204,78 +207,42 @@ void WalletView::gotoVerifyMessageTab(QString addr) signVerifyMessageDialog->setAddress_VM(addr); } -void WalletView::gotoLoadPSBT() +void WalletView::gotoLoadPSBT(bool from_clipboard) { - QString filename = GUIUtil::getOpenFileName(this, - tr("Load Transaction Data"), QString(), - tr("Partially Signed Transaction (*.psbt)"), nullptr); - if (filename.isEmpty()) return; - if (GetFileSize(filename.toLocal8Bit().data(), MAX_FILE_SIZE_PSBT) == MAX_FILE_SIZE_PSBT) { - Q_EMIT message(tr("Error"), tr("PSBT file must be smaller than 100 MiB"), CClientUIInterface::MSG_ERROR); - return; + std::string data; + + if (from_clipboard) { + std::string raw = QApplication::clipboard()->text().toStdString(); + bool invalid; + data = DecodeBase64(raw, &invalid); + if (invalid) { + Q_EMIT message(tr("Error"), tr("Unable to decode PSBT from clipboard (invalid base64)"), CClientUIInterface::MSG_ERROR); + return; + } + } else { + QString filename = GUIUtil::getOpenFileName(this, + tr("Load Transaction Data"), QString(), + tr("Partially Signed Transaction (*.psbt)"), nullptr); + if (filename.isEmpty()) return; + if (GetFileSize(filename.toLocal8Bit().data(), MAX_FILE_SIZE_PSBT) == MAX_FILE_SIZE_PSBT) { + Q_EMIT message(tr("Error"), tr("PSBT file must be smaller than 100 MiB"), CClientUIInterface::MSG_ERROR); + return; + } + std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary); + data = std::string(std::istreambuf_iterator<char>{in}, {}); } - std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary); - std::string data(std::istreambuf_iterator<char>{in}, {}); std::string error; PartiallySignedTransaction psbtx; if (!DecodeRawPSBT(psbtx, data, error)) { - Q_EMIT message(tr("Error"), tr("Unable to decode PSBT file") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR); + Q_EMIT message(tr("Error"), tr("Unable to decode PSBT") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR); return; } - CMutableTransaction mtx; - bool complete = false; - PSBTAnalysis analysis = AnalyzePSBT(psbtx); - QMessageBox msgBox; - msgBox.setText("PSBT"); - switch (analysis.next) { - case PSBTRole::CREATOR: - case PSBTRole::UPDATER: - msgBox.setInformativeText("PSBT is incomplete. Copy to clipboard for manual inspection?"); - break; - case PSBTRole::SIGNER: - msgBox.setInformativeText("Transaction needs more signatures. Copy to clipboard?"); - break; - case PSBTRole::FINALIZER: - case PSBTRole::EXTRACTOR: - complete = FinalizeAndExtractPSBT(psbtx, mtx); - if (complete) { - msgBox.setInformativeText(tr("Would you like to send this transaction?")); - } else { - // The analyzer missed something, e.g. if there are final_scriptSig/final_scriptWitness - // but with invalid signatures. - msgBox.setInformativeText(tr("There was an unexpected problem processing the PSBT. Copy to clipboard for manual inspection?")); - } - } - - msgBox.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel); - switch (msgBox.exec()) { - case QMessageBox::Yes: { - if (complete) { - std::string err_string; - CTransactionRef tx = MakeTransactionRef(mtx); - - TransactionError result = BroadcastTransaction(*clientModel->node().context(), tx, err_string, DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK(), /* relay */ true, /* wait_callback */ false); - if (result == TransactionError::OK) { - Q_EMIT message(tr("Success"), tr("Broadcasted transaction successfully."), CClientUIInterface::MSG_INFORMATION | CClientUIInterface::MODAL); - } else { - Q_EMIT message(tr("Error"), QString::fromStdString(err_string), CClientUIInterface::MSG_ERROR); - } - } else { - // Serialize the PSBT - CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << psbtx; - GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str()); - Q_EMIT message(tr("PSBT copied"), "Copied to clipboard", CClientUIInterface::MSG_INFORMATION); - return; - } - } - case QMessageBox::Cancel: - break; - default: - assert(false); - } + PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, walletModel, clientModel); + dlg->openWithPSBT(psbtx); + dlg->setAttribute(Qt::WA_DeleteOnClose); + dlg->exec(); } bool WalletView::handlePaymentRequest(const SendCoinsRecipient& recipient) diff --git a/src/qt/walletview.h b/src/qt/walletview.h index fd09456baa..f186554758 100644 --- a/src/qt/walletview.h +++ b/src/qt/walletview.h @@ -84,7 +84,7 @@ public Q_SLOTS: /** Show Sign/Verify Message dialog and switch to verify message tab */ void gotoVerifyMessageTab(QString addr = ""); /** Load Partially Signed Bitcoin Transaction */ - void gotoLoadPSBT(); + void gotoLoadPSBT(bool from_clipboard = false); /** Show incoming transaction notification for new transactions. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 3045a74d7a..66ace7263a 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -217,7 +217,7 @@ UniValue ParseNonRFCJSONValue(const std::string& strVal) UniValue jVal; if (!jVal.read(std::string("[")+strVal+std::string("]")) || !jVal.isArray() || jVal.size()!=1) - throw std::runtime_error(std::string("Error parsing JSON:")+strVal); + throw std::runtime_error(std::string("Error parsing JSON: ") + strVal); return jVal[0]; } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d5fc3f57bb..e8e0ab3179 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -17,6 +17,7 @@ #include <policy/fees.h> #include <pow.h> #include <rpc/blockchain.h> +#include <rpc/mining.h> #include <rpc/server.h> #include <rpc/util.h> #include <script/descriptor.h> @@ -207,7 +208,7 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) { {"num_blocks", RPCArg::Type::NUM, RPCArg::Optional::NO, "How many blocks are generated immediately."}, {"descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The descriptor to send the newly generated bitcoin to."}, - {"maxtries", RPCArg::Type::NUM, /* default */ "1000000", "How many iterations to try."}, + {"maxtries", RPCArg::Type::NUM, /* default */ ToString(DEFAULT_MAX_TRIES), "How many iterations to try."}, }, RPCResult{ RPCResult::Type::ARR, "", "hashes of blocks generated", @@ -221,7 +222,7 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) .Check(request); const int num_blocks{request.params[0].get_int()}; - const int64_t max_tries{request.params[2].isNull() ? 1000000 : request.params[2].get_int()}; + const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; CScript coinbase_script; std::string error; @@ -242,7 +243,7 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) { {"nblocks", RPCArg::Type::NUM, RPCArg::Optional::NO, "How many blocks are generated immediately."}, {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The address to send the newly generated bitcoin to."}, - {"maxtries", RPCArg::Type::NUM, /* default */ "1000000", "How many iterations to try."}, + {"maxtries", RPCArg::Type::NUM, /* default */ ToString(DEFAULT_MAX_TRIES), "How many iterations to try."}, }, RPCResult{ RPCResult::Type::ARR, "", "hashes of blocks generated", @@ -257,11 +258,8 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) }, }.Check(request); - int nGenerate = request.params[0].get_int(); - uint64_t nMaxTries = 1000000; - if (!request.params[2].isNull()) { - nMaxTries = request.params[2].get_int(); - } + const int num_blocks{request.params[0].get_int()}; + const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; CTxDestination destination = DecodeDestination(request.params[1].get_str()); if (!IsValidDestination(destination)) { @@ -273,7 +271,7 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) CScript coinbase_script = GetScriptForDestination(destination); - return generateBlocks(chainman, mempool, coinbase_script, nGenerate, nMaxTries); + return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); } static UniValue generateblock(const JSONRPCRequest& request) @@ -371,7 +369,7 @@ static UniValue generateblock(const JSONRPCRequest& request) } uint256 block_hash; - uint64_t max_tries{1000000}; + uint64_t max_tries{DEFAULT_MAX_TRIES}; unsigned int extra_nonce{0}; if (!GenerateBlock(EnsureChainman(request.context), block, max_tries, extra_nonce, block_hash) || block_hash.IsNull()) { diff --git a/src/rpc/mining.h b/src/rpc/mining.h new file mode 100644 index 0000000000..acc74e1dcc --- /dev/null +++ b/src/rpc/mining.h @@ -0,0 +1,11 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_RPC_MINING_H +#define BITCOIN_RPC_MINING_H + +/** Default max iterations to try in RPC generatetodescriptor, generatetoaddress, and generateblock. */ +static const uint64_t DEFAULT_MAX_TRIES{1000000}; + +#endif // BITCOIN_RPC_MINING_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index e14217c307..814f8bddfe 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -595,7 +595,7 @@ static UniValue decodescript(const JSONRPCRequest& request) if (which_type == TX_PUBKEY) { segwitScr = GetScriptForDestination(WitnessV0KeyHash(Hash160(solutions_data[0].begin(), solutions_data[0].end()))); } else if (which_type == TX_PUBKEYHASH) { - segwitScr = GetScriptForDestination(WitnessV0KeyHash(solutions_data[0])); + segwitScr = GetScriptForDestination(WitnessV0KeyHash(uint160{solutions_data[0]})); } else { // Scripts that are not fit for P2WPKH are encoded as P2WSH. // Newer segwit program versions should be considered when then become available. diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ed0175bb10..7a5421ab6f 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -139,7 +139,7 @@ std::string DescriptorChecksum(const Span<const char>& span) return ret; } -std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(MakeSpan(str)); } +std::string AddChecksum(const std::string& str) { return str + "#" + DescriptorChecksum(str); } //////////////////////////////////////////////////////////////////////////// // Internal representation // @@ -1087,7 +1087,7 @@ bool CheckChecksum(Span<const char>& sp, bool require_checksum, std::string& err std::unique_ptr<Descriptor> Parse(const std::string& descriptor, FlatSigningProvider& out, std::string& error, bool require_checksum) { - Span<const char> sp(descriptor.data(), descriptor.size()); + Span<const char> sp{descriptor}; if (!CheckChecksum(sp, require_checksum, error)) return nullptr; auto ret = ParseScript(0, sp, ParseScriptContext::TOP, out, error); if (sp.size() == 0 && ret) return std::unique_ptr<Descriptor>(std::move(ret)); @@ -1098,7 +1098,7 @@ std::string GetDescriptorChecksum(const std::string& descriptor) { std::string ret; std::string error; - Span<const char> sp(descriptor.data(), descriptor.size()); + Span<const char> sp{descriptor}; if (!CheckChecksum(sp, false, error, &ret)) return ""; return ret; } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 23d5b72a5c..9415bba585 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1522,7 +1522,7 @@ static bool ExecuteWitnessScript(const Span<const valtype>& stack_span, const CS static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion, const std::vector<unsigned char>& program, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror) { CScript scriptPubKey; - Span<const valtype> stack = MakeSpan(witness.stack); + Span<const valtype> stack{witness.stack}; if (witversion == 0) { if (program.size() == WITNESS_V0_SCRIPTHASH_SIZE) { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 1e00afcf89..43988c4fd7 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -131,7 +131,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } case TX_SCRIPTHASH: h160 = uint160(vSolutions[0]); - if (GetCScript(provider, sigdata, h160, scriptRet)) { + if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end())); return true; } @@ -165,7 +165,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator case TX_WITNESS_V0_SCRIPTHASH: CRIPEMD160().Write(&vSolutions[0][0], vSolutions[0].size()).Finalize(h160.begin()); - if (GetCScript(provider, sigdata, h160, scriptRet)) { + if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end())); return true; } @@ -458,7 +458,7 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) if (whichtype == TX_SCRIPTHASH) { auto h160 = uint160(solutions[0]); CScript subscript; - if (provider.GetCScript(h160, subscript)) { + if (provider.GetCScript(CScriptID{h160}, subscript)) { whichtype = Solver(subscript, solutions); if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; } diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp index 01757e2f65..2d8dc7d471 100644 --- a/src/script/signingprovider.cpp +++ b/src/script/signingprovider.cpp @@ -180,10 +180,10 @@ CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination& // Only supports destinations which map to single public keys, i.e. P2PKH, // P2WPKH, and P2SH-P2WPKH. if (auto id = boost::get<PKHash>(&dest)) { - return CKeyID(*id); + return ToKeyID(*id); } if (auto witness_id = boost::get<WitnessV0KeyHash>(&dest)) { - return CKeyID(*witness_id); + return ToKeyID(*witness_id); } if (auto script_hash = boost::get<ScriptHash>(&dest)) { CScript script; @@ -191,7 +191,7 @@ CKeyID GetKeyForDestination(const SigningProvider& store, const CTxDestination& CTxDestination inner_dest; if (store.GetCScript(script_id, script) && ExtractDestination(script, inner_dest)) { if (auto inner_witness_id = boost::get<WitnessV0KeyHash>(&inner_dest)) { - return CKeyID(*inner_witness_id); + return ToKeyID(*inner_witness_id); } } } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index c90c2c24a0..2adf6ce56d 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -16,11 +16,27 @@ typedef std::vector<unsigned char> valtype; bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; -CScriptID::CScriptID(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast<uint160>(in)) {} -ScriptHash::ScriptHash(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScriptID& in) : BaseHash(static_cast<uint160>(in)) {} -PKHash::PKHash(const CPubKey& pubkey) : uint160(pubkey.GetID()) {} +PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {} +PKHash::PKHash(const CKeyID& pubkey_id) : BaseHash(pubkey_id) {} + +WitnessV0KeyHash::WitnessV0KeyHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {} +WitnessV0KeyHash::WitnessV0KeyHash(const PKHash& pubkey_hash) : BaseHash(static_cast<uint160>(pubkey_hash)) {} + +CKeyID ToKeyID(const PKHash& key_hash) +{ + return CKeyID{static_cast<uint160>(key_hash)}; +} + +CKeyID ToKeyID(const WitnessV0KeyHash& key_hash) +{ + return CKeyID{static_cast<uint160>(key_hash)}; +} WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in) { @@ -241,59 +257,47 @@ bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std:: namespace { -class CScriptVisitor : public boost::static_visitor<bool> +class CScriptVisitor : public boost::static_visitor<CScript> { -private: - CScript *script; public: - explicit CScriptVisitor(CScript *scriptin) { script = scriptin; } - - bool operator()(const CNoDestination &dest) const { - script->clear(); - return false; + CScript operator()(const CNoDestination& dest) const + { + return CScript(); } - bool operator()(const PKHash &keyID) const { - script->clear(); - *script << OP_DUP << OP_HASH160 << ToByteVector(keyID) << OP_EQUALVERIFY << OP_CHECKSIG; - return true; + CScript operator()(const PKHash& keyID) const + { + return CScript() << OP_DUP << OP_HASH160 << ToByteVector(keyID) << OP_EQUALVERIFY << OP_CHECKSIG; } - bool operator()(const ScriptHash &scriptID) const { - script->clear(); - *script << OP_HASH160 << ToByteVector(scriptID) << OP_EQUAL; - return true; + CScript operator()(const ScriptHash& scriptID) const + { + return CScript() << OP_HASH160 << ToByteVector(scriptID) << OP_EQUAL; } - bool operator()(const WitnessV0KeyHash& id) const + CScript operator()(const WitnessV0KeyHash& id) const { - script->clear(); - *script << OP_0 << ToByteVector(id); - return true; + return CScript() << OP_0 << ToByteVector(id); } - bool operator()(const WitnessV0ScriptHash& id) const + CScript operator()(const WitnessV0ScriptHash& id) const { - script->clear(); - *script << OP_0 << ToByteVector(id); - return true; + return CScript() << OP_0 << ToByteVector(id); } - bool operator()(const WitnessUnknown& id) const + CScript operator()(const WitnessUnknown& id) const { - script->clear(); - *script << CScript::EncodeOP_N(id.version) << std::vector<unsigned char>(id.program, id.program + id.length); - return true; + return CScript() << CScript::EncodeOP_N(id.version) << std::vector<unsigned char>(id.program, id.program + id.length); } }; + +const CScriptVisitor g_script_visitor; + } // namespace CScript GetScriptForDestination(const CTxDestination& dest) { - CScript script; - - boost::apply_visitor(CScriptVisitor(&script), dest); - return script; + return boost::apply_visitor(::g_script_visitor, dest); } CScript GetScriptForRawPubKey(const CPubKey& pubKey) @@ -319,7 +323,7 @@ CScript GetScriptForWitness(const CScript& redeemscript) if (typ == TX_PUBKEY) { return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0].begin(), vSolutions[0].end()))); } else if (typ == TX_PUBKEYHASH) { - return GetScriptForDestination(WitnessV0KeyHash(vSolutions[0])); + return GetScriptForDestination(WitnessV0KeyHash(uint160{vSolutions[0]})); } return GetScriptForDestination(WitnessV0ScriptHash(redeemscript)); } diff --git a/src/script/standard.h b/src/script/standard.h index 2929425670..4baed6da6e 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -18,14 +18,77 @@ static const bool DEFAULT_ACCEPT_DATACARRIER = true; class CKeyID; class CScript; +struct ScriptHash; + +template<typename HashType> +class BaseHash +{ +protected: + HashType m_hash; + +public: + BaseHash() : m_hash() {} + BaseHash(const HashType& in) : m_hash(in) {} + + unsigned char* begin() + { + return m_hash.begin(); + } + + const unsigned char* begin() const + { + return m_hash.begin(); + } + + unsigned char* end() + { + return m_hash.end(); + } + + const unsigned char* end() const + { + return m_hash.end(); + } + + operator std::vector<unsigned char>() const + { + return std::vector<unsigned char>{m_hash.begin(), m_hash.end()}; + } + + std::string ToString() const + { + return m_hash.ToString(); + } + + bool operator==(const BaseHash<HashType>& other) const noexcept + { + return m_hash == other.m_hash; + } + + bool operator!=(const BaseHash<HashType>& other) const noexcept + { + return !(m_hash == other.m_hash); + } + + bool operator<(const BaseHash<HashType>& other) const noexcept + { + return m_hash < other.m_hash; + } + + size_t size() const + { + return m_hash.size(); + } +}; /** A reference to a CScript: the Hash160 of its serialization (see script.h) */ -class CScriptID : public uint160 +class CScriptID : public BaseHash<uint160> { public: - CScriptID() : uint160() {} + CScriptID() : BaseHash() {} explicit CScriptID(const CScript& in); - CScriptID(const uint160& in) : uint160(in) {} + explicit CScriptID(const uint160& in) : BaseHash(in) {} + explicit CScriptID(const ScriptHash& in); }; /** @@ -73,41 +136,44 @@ public: friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; } }; -struct PKHash : public uint160 +struct PKHash : public BaseHash<uint160> { - PKHash() : uint160() {} - explicit PKHash(const uint160& hash) : uint160(hash) {} + PKHash() : BaseHash() {} + explicit PKHash(const uint160& hash) : BaseHash(hash) {} explicit PKHash(const CPubKey& pubkey); - using uint160::uint160; + explicit PKHash(const CKeyID& pubkey_id); }; +CKeyID ToKeyID(const PKHash& key_hash); struct WitnessV0KeyHash; -struct ScriptHash : public uint160 +struct ScriptHash : public BaseHash<uint160> { - ScriptHash() : uint160() {} + ScriptHash() : BaseHash() {} // These don't do what you'd expect. // Use ScriptHash(GetScriptForDestination(...)) instead. explicit ScriptHash(const WitnessV0KeyHash& hash) = delete; explicit ScriptHash(const PKHash& hash) = delete; - explicit ScriptHash(const uint160& hash) : uint160(hash) {} + + explicit ScriptHash(const uint160& hash) : BaseHash(hash) {} explicit ScriptHash(const CScript& script); - using uint160::uint160; + explicit ScriptHash(const CScriptID& script); }; -struct WitnessV0ScriptHash : public uint256 +struct WitnessV0ScriptHash : public BaseHash<uint256> { - WitnessV0ScriptHash() : uint256() {} - explicit WitnessV0ScriptHash(const uint256& hash) : uint256(hash) {} + WitnessV0ScriptHash() : BaseHash() {} + explicit WitnessV0ScriptHash(const uint256& hash) : BaseHash(hash) {} explicit WitnessV0ScriptHash(const CScript& script); - using uint256::uint256; }; -struct WitnessV0KeyHash : public uint160 +struct WitnessV0KeyHash : public BaseHash<uint160> { - WitnessV0KeyHash() : uint160() {} - explicit WitnessV0KeyHash(const uint160& hash) : uint160(hash) {} - using uint160::uint160; + WitnessV0KeyHash() : BaseHash() {} + explicit WitnessV0KeyHash(const uint160& hash) : BaseHash(hash) {} + explicit WitnessV0KeyHash(const CPubKey& pubkey); + explicit WitnessV0KeyHash(const PKHash& pubkey_hash); }; +CKeyID ToKeyID(const WitnessV0KeyHash& key_hash); //! CTxDestination subtype to encode any future Witness version struct WitnessUnknown diff --git a/src/span.h b/src/span.h index 73b37e35f3..4931507719 100644 --- a/src/span.h +++ b/src/span.h @@ -10,6 +10,14 @@ #include <algorithm> #include <assert.h> +#ifdef DEBUG +#define CONSTEXPR_IF_NOT_DEBUG +#define ASSERT_IF_DEBUG(x) assert((x)) +#else +#define CONSTEXPR_IF_NOT_DEBUG constexpr +#define ASSERT_IF_DEBUG(x) +#endif + /** A Span is an object that can refer to a contiguous sequence of objects. * * It implements a subset of C++20's std::span. @@ -18,12 +26,29 @@ template<typename C> class Span { C* m_data; - std::ptrdiff_t m_size; + std::size_t m_size; public: constexpr Span() noexcept : m_data(nullptr), m_size(0) {} - constexpr Span(C* data, std::ptrdiff_t size) noexcept : m_data(data), m_size(size) {} - constexpr Span(C* data, C* end) noexcept : m_data(data), m_size(end - data) {} + + /** Construct a span from a begin pointer and a size. + * + * This implements a subset of the iterator-based std::span constructor in C++20, + * which is hard to implement without std::address_of. + */ + template <typename T, typename std::enable_if<std::is_convertible<T (*)[], C (*)[]>::value, int>::type = 0> + constexpr Span(T* begin, std::size_t size) noexcept : m_data(begin), m_size(size) {} + + /** Construct a span from a begin and end pointer. + * + * This implements a subset of the iterator-based std::span constructor in C++20, + * which is hard to implement without std::address_of. + */ + template <typename T, typename std::enable_if<std::is_convertible<T (*)[], C (*)[]>::value, int>::type = 0> + CONSTEXPR_IF_NOT_DEBUG Span(T* begin, T* end) noexcept : m_data(begin), m_size(end - begin) + { + ASSERT_IF_DEBUG(end >= begin); + } /** Implicit conversion of spans between compatible types. * @@ -42,18 +67,59 @@ public: /** Default assignment operator. */ Span& operator=(const Span& other) noexcept = default; + /** Construct a Span from an array. This matches the corresponding C++20 std::span constructor. */ + template <int N> + constexpr Span(C (&a)[N]) noexcept : m_data(a), m_size(N) {} + + /** Construct a Span for objects with .data() and .size() (std::string, std::array, std::vector, ...). + * + * This implements a subset of the functionality provided by the C++20 std::span range-based constructor. + * + * To prevent surprises, only Spans for constant value types are supported when passing in temporaries. + * Note that this restriction does not exist when converting arrays or other Spans (see above). + */ + template <typename V, typename std::enable_if<(std::is_const<C>::value || std::is_lvalue_reference<V>::value) && std::is_convertible<typename std::remove_pointer<decltype(std::declval<V&>().data())>::type (*)[], C (*)[]>::value && std::is_convertible<decltype(std::declval<V&>().size()), std::size_t>::value, int>::type = 0> + constexpr Span(V&& v) noexcept : m_data(v.data()), m_size(v.size()) {} + constexpr C* data() const noexcept { return m_data; } constexpr C* begin() const noexcept { return m_data; } constexpr C* end() const noexcept { return m_data + m_size; } - constexpr C& front() const noexcept { return m_data[0]; } - constexpr C& back() const noexcept { return m_data[m_size - 1]; } - constexpr std::ptrdiff_t size() const noexcept { return m_size; } - constexpr C& operator[](std::ptrdiff_t pos) const noexcept { return m_data[pos]; } - - constexpr Span<C> subspan(std::ptrdiff_t offset) const noexcept { return Span<C>(m_data + offset, m_size - offset); } - constexpr Span<C> subspan(std::ptrdiff_t offset, std::ptrdiff_t count) const noexcept { return Span<C>(m_data + offset, count); } - constexpr Span<C> first(std::ptrdiff_t count) const noexcept { return Span<C>(m_data, count); } - constexpr Span<C> last(std::ptrdiff_t count) const noexcept { return Span<C>(m_data + m_size - count, count); } + CONSTEXPR_IF_NOT_DEBUG C& front() const noexcept + { + ASSERT_IF_DEBUG(size() > 0); + return m_data[0]; + } + CONSTEXPR_IF_NOT_DEBUG C& back() const noexcept + { + ASSERT_IF_DEBUG(size() > 0); + return m_data[m_size - 1]; + } + constexpr std::size_t size() const noexcept { return m_size; } + CONSTEXPR_IF_NOT_DEBUG C& operator[](std::size_t pos) const noexcept + { + ASSERT_IF_DEBUG(size() > pos); + return m_data[pos]; + } + CONSTEXPR_IF_NOT_DEBUG Span<C> subspan(std::size_t offset) const noexcept + { + ASSERT_IF_DEBUG(size() >= offset); + return Span<C>(m_data + offset, m_size - offset); + } + CONSTEXPR_IF_NOT_DEBUG Span<C> subspan(std::size_t offset, std::size_t count) const noexcept + { + ASSERT_IF_DEBUG(size() >= offset + count); + return Span<C>(m_data + offset, count); + } + CONSTEXPR_IF_NOT_DEBUG Span<C> first(std::size_t count) const noexcept + { + ASSERT_IF_DEBUG(size() >= count); + return Span<C>(m_data, count); + } + CONSTEXPR_IF_NOT_DEBUG Span<C> last(std::size_t count) const noexcept + { + ASSERT_IF_DEBUG(size() >= count); + return Span<C>(m_data + m_size - count, count); + } friend constexpr bool operator==(const Span& a, const Span& b) noexcept { return a.size() == b.size() && std::equal(a.begin(), a.end(), b.begin()); } friend constexpr bool operator!=(const Span& a, const Span& b) noexcept { return !(a == b); } @@ -65,26 +131,20 @@ public: template <typename O> friend class Span; }; -/** Create a span to a container exposing data() and size(). - * - * This correctly deals with constness: the returned Span's element type will be - * whatever data() returns a pointer to. If either the passed container is const, - * or its element type is const, the resulting span will have a const element type. - * - * std::span will have a constructor that implements this functionality directly. - */ -template<typename A, int N> -constexpr Span<A> MakeSpan(A (&a)[N]) { return Span<A>(a, N); } - -template<typename V> -constexpr Span<typename std::remove_pointer<decltype(std::declval<V>().data())>::type> MakeSpan(V& v) { return Span<typename std::remove_pointer<decltype(std::declval<V>().data())>::type>(v.data(), v.size()); } +// MakeSpan helps constructing a Span of the right type automatically. +/** MakeSpan for arrays: */ +template <typename A, int N> Span<A> constexpr MakeSpan(A (&a)[N]) { return Span<A>(a, N); } +/** MakeSpan for temporaries / rvalue references, only supporting const output. */ +template <typename V> constexpr auto MakeSpan(V&& v) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); } +/** MakeSpan for (lvalue) references, supporting mutable output. */ +template <typename V> constexpr auto MakeSpan(V& v) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; } /** Pop the last element off a span, and return a reference to that element. */ template <typename T> T& SpanPopBack(Span<T>& span) { size_t size = span.size(); - assert(size > 0); + ASSERT_IF_DEBUG(size > 0); T& back = span[size - 1]; span = Span<T>(span.data(), size - 1); return back; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 211a84b5f2..2fa751b987 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -30,7 +30,17 @@ #include <string> #include <vector> -bool ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc); +void ProcessMessage( + CNode& pfrom, + const std::string& msg_type, + CDataStream& vRecv, + int64_t nTimeReceived, + const CChainParams& chainparams, + ChainstateManager& chainman, + CTxMemPool& mempool, + CConnman* connman, + BanMan* banman, + const std::atomic<bool>& interruptMsgProc); namespace { @@ -77,7 +87,10 @@ void test_one_input(const std::vector<uint8_t>& buffer) connman.AddTestNode(p2p_node); g_setup->m_node.peer_logic->InitializeNode(&p2p_node); try { - (void)ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false}); + ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), + Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, + g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), + std::atomic<bool>{false}); } catch (const std::ios_base::failure&) { } SyncWithValidationInterfaceQueue(); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index ad6c115a49..91ebf9fb1b 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -62,7 +62,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()}; CSerializedNetMsg net_msg; - net_msg.command = random_message_type; + net_msg.m_type = random_message_type; net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider); CNode& random_node = *peers.at(fuzzed_data_provider.ConsumeIntegralInRange<int>(0, peers.size() - 1)); diff --git a/src/test/fuzz/span.cpp b/src/test/fuzz/span.cpp index 4aea530ef2..f6b6e8f6f0 100644 --- a/src/test/fuzz/span.cpp +++ b/src/test/fuzz/span.cpp @@ -18,7 +18,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); std::string str = fuzzed_data_provider.ConsumeBytesAsString(32); - const Span<const char> span = MakeSpan(str); + const Span<const char> span{str}; (void)span.data(); (void)span.begin(); (void)span.end(); @@ -32,7 +32,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) } std::string another_str = fuzzed_data_provider.ConsumeBytesAsString(32); - const Span<const char> another_span = MakeSpan(another_str); + const Span<const char> another_span{another_str}; assert((span <= another_span) != (span > another_span)); assert((span == another_span) != (span != another_span)); assert((span >= another_span) != (span < another_span)); diff --git a/src/test/fuzz/spanparsing.cpp b/src/test/fuzz/spanparsing.cpp index 8e5e7dad11..e5bf5dd608 100644 --- a/src/test/fuzz/spanparsing.cpp +++ b/src/test/fuzz/spanparsing.cpp @@ -12,7 +12,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) const size_t query_size = fuzzed_data_provider.ConsumeIntegral<size_t>(); const std::string query = fuzzed_data_provider.ConsumeBytesAsString(std::min<size_t>(query_size, 1024 * 1024)); const std::string span_str = fuzzed_data_provider.ConsumeRemainingBytesAsString(); - const Span<const char> const_span = MakeSpan(span_str); + const Span<const char> const_span{span_str}; Span<const char> mut_span = const_span; (void)spanparsing::Const(query, mut_span); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index d0ec401f9d..0fbf257f0e 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -160,6 +160,9 @@ BOOST_AUTO_TEST_CASE(subnet_test) BOOST_CHECK(ResolveSubNet("1.2.2.20/26").Match(ResolveIP("1.2.2.63"))); // All-Matching IPv6 Matches arbitrary IPv4 and IPv6 BOOST_CHECK(ResolveSubNet("::/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); + // But not `::` or `0.0.0.0` because they are considered invalid addresses + BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("::"))); + BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("0.0.0.0"))); BOOST_CHECK(ResolveSubNet("::/0").Match(ResolveIP("1.2.3.4"))); // All-Matching IPv4 does not Match IPv6 BOOST_CHECK(!ResolveSubNet("0.0.0.0/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index cf26ca3adb..257328974b 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1829,7 +1829,7 @@ BOOST_AUTO_TEST_CASE(test_spanparsing) // Const(...): parse a constant, update span to skip it if successful input = "MilkToastHoney"; - sp = MakeSpan(input); + sp = input; success = Const("", sp); // empty BOOST_CHECK(success); BOOST_CHECK_EQUAL(SpanToStr(sp), "MilkToastHoney"); @@ -1854,7 +1854,7 @@ BOOST_AUTO_TEST_CASE(test_spanparsing) // Func(...): parse a function call, update span to argument if successful input = "Foo(Bar(xy,z()))"; - sp = MakeSpan(input); + sp = input; success = Func("FooBar", sp); BOOST_CHECK(!success); @@ -1877,31 +1877,31 @@ BOOST_AUTO_TEST_CASE(test_spanparsing) Span<const char> result; input = "(n*(n-1))/2"; - sp = MakeSpan(input); + sp = input; result = Expr(sp); BOOST_CHECK_EQUAL(SpanToStr(result), "(n*(n-1))/2"); BOOST_CHECK_EQUAL(SpanToStr(sp), ""); input = "foo,bar"; - sp = MakeSpan(input); + sp = input; result = Expr(sp); BOOST_CHECK_EQUAL(SpanToStr(result), "foo"); BOOST_CHECK_EQUAL(SpanToStr(sp), ",bar"); input = "(aaaaa,bbbbb()),c"; - sp = MakeSpan(input); + sp = input; result = Expr(sp); BOOST_CHECK_EQUAL(SpanToStr(result), "(aaaaa,bbbbb())"); BOOST_CHECK_EQUAL(SpanToStr(sp), ",c"); input = "xyz)foo"; - sp = MakeSpan(input); + sp = input; result = Expr(sp); BOOST_CHECK_EQUAL(SpanToStr(result), "xyz"); BOOST_CHECK_EQUAL(SpanToStr(sp), ")foo"); input = "((a),(b),(c)),xxx"; - sp = MakeSpan(input); + sp = input; result = Expr(sp); BOOST_CHECK_EQUAL(SpanToStr(result), "((a),(b),(c))"); BOOST_CHECK_EQUAL(SpanToStr(sp), ",xxx"); @@ -1910,7 +1910,7 @@ BOOST_AUTO_TEST_CASE(test_spanparsing) std::vector<Span<const char>> results; input = "xxx"; - results = Split(MakeSpan(input), 'x'); + results = Split(input, 'x'); BOOST_CHECK_EQUAL(results.size(), 4U); BOOST_CHECK_EQUAL(SpanToStr(results[0]), ""); BOOST_CHECK_EQUAL(SpanToStr(results[1]), ""); @@ -1918,19 +1918,19 @@ BOOST_AUTO_TEST_CASE(test_spanparsing) BOOST_CHECK_EQUAL(SpanToStr(results[3]), ""); input = "one#two#three"; - results = Split(MakeSpan(input), '-'); + results = Split(input, '-'); BOOST_CHECK_EQUAL(results.size(), 1U); BOOST_CHECK_EQUAL(SpanToStr(results[0]), "one#two#three"); input = "one#two#three"; - results = Split(MakeSpan(input), '#'); + results = Split(input, '#'); BOOST_CHECK_EQUAL(results.size(), 3U); BOOST_CHECK_EQUAL(SpanToStr(results[0]), "one"); BOOST_CHECK_EQUAL(SpanToStr(results[1]), "two"); BOOST_CHECK_EQUAL(SpanToStr(results[2]), "three"); input = "*foo*bar*"; - results = Split(MakeSpan(input), '*'); + results = Split(input, '*'); BOOST_CHECK_EQUAL(results.size(), 4U); BOOST_CHECK_EQUAL(SpanToStr(results[0]), ""); BOOST_CHECK_EQUAL(SpanToStr(results[1]), "foo"); diff --git a/src/ui_interface.h b/src/ui_interface.h index 356d30eaf6..b7895e373f 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -122,7 +122,7 @@ void InitWarning(const bilingual_str& str); /** Show error message **/ bool InitError(const bilingual_str& str); -constexpr auto AbortError = InitError; +inline bool AbortError(const bilingual_str& str) { return InitError(str); } extern CClientUIInterface uiInterface; diff --git a/src/util/error.cpp b/src/util/error.cpp index c4d9ffd037..3e29083712 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -14,7 +14,7 @@ bilingual_str TransactionErrorString(const TransactionError err) case TransactionError::OK: return Untranslated("No error"); case TransactionError::MISSING_INPUTS: - return Untranslated("Missing inputs"); + return Untranslated("Inputs missing or spent"); case TransactionError::ALREADY_IN_CHAIN: return Untranslated("Transaction already in block chain"); case TransactionError::P2P_DISABLED: @@ -24,11 +24,11 @@ bilingual_str TransactionErrorString(const TransactionError err) case TransactionError::MEMPOOL_ERROR: return Untranslated("AcceptToMemoryPool failed"); case TransactionError::INVALID_PSBT: - return Untranslated("PSBT is not sane"); + return Untranslated("PSBT is not well-formed"); case TransactionError::PSBT_MISMATCH: return Untranslated("PSBTs not compatible (different transactions)"); case TransactionError::SIGHASH_MISMATCH: - return Untranslated("Specified sighash value does not match existing value"); + return Untranslated("Specified sighash value does not match value stored in PSBT"); case TransactionError::MAX_FEE_EXCEEDED: return Untranslated("Fee exceeds maximum configured by -maxtxfee"); // no default case, so the compiler can warn about missing cases diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 7ed9c88122..125bf004e4 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -803,3 +803,67 @@ std::string BerkeleyDatabaseVersion() { return DbEnv::version(nullptr, nullptr, nullptr); } + +bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) +{ + if (!pdb) + return false; + + // Key + SafeDbt datKey(key.data(), key.size()); + + // Read + SafeDbt datValue; + int ret = pdb->get(activeTxn, datKey, datValue, 0); + if (ret == 0 && datValue.get_data() != nullptr) { + value.write((char*)datValue.get_data(), datValue.get_size()); + return true; + } + return false; +} + +bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite) +{ + if (!pdb) + return true; + if (fReadOnly) + assert(!"Write called on database in read-only mode"); + + // Key + SafeDbt datKey(key.data(), key.size()); + + // Value + SafeDbt datValue(value.data(), value.size()); + + // Write + int ret = pdb->put(activeTxn, datKey, datValue, (overwrite ? 0 : DB_NOOVERWRITE)); + return (ret == 0); +} + +bool BerkeleyBatch::EraseKey(CDataStream& key) +{ + if (!pdb) + return false; + if (fReadOnly) + assert(!"Erase called on database in read-only mode"); + + // Key + SafeDbt datKey(key.data(), key.size()); + + // Erase + int ret = pdb->del(activeTxn, datKey, 0); + return (ret == 0 || ret == DB_NOTFOUND); +} + +bool BerkeleyBatch::HasKey(CDataStream& key) +{ + if (!pdb) + return false; + + // Key + SafeDbt datKey(key.data(), key.size()); + + // Exists + int ret = pdb->exists(activeTxn, datKey, 0); + return ret == 0; +} diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 5ed364344b..c121bb4228 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -120,25 +120,6 @@ public: } } - /** Return object for accessing database at specified path. */ - static std::unique_ptr<BerkeleyDatabase> Create(const fs::path& path) - { - std::string filename; - return MakeUnique<BerkeleyDatabase>(GetWalletEnv(path, filename), std::move(filename)); - } - - /** Return object for accessing dummy database with no read/write capabilities. */ - static std::unique_ptr<BerkeleyDatabase> CreateDummy() - { - return MakeUnique<BerkeleyDatabase>(); - } - - /** Return object for accessing temporary in-memory database. */ - static std::unique_ptr<BerkeleyDatabase> CreateMock() - { - return MakeUnique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), ""); - } - /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero */ bool Rewrite(const char* pszSkip=nullptr); @@ -207,6 +188,12 @@ class BerkeleyBatch operator Dbt*(); }; +private: + bool ReadKey(CDataStream& key, CDataStream& value); + bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true); + bool EraseKey(CDataStream& key); + bool HasKey(CDataStream& key); + protected: Db* pdb; std::string strFile; @@ -236,91 +223,65 @@ public: template <typename K, typename T> bool Read(const K& key, T& value) { - if (!pdb) - return false; - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - SafeDbt datKey(ssKey.data(), ssKey.size()); - // Read - SafeDbt datValue; - int ret = pdb->get(activeTxn, datKey, datValue, 0); + CDataStream ssValue(SER_DISK, CLIENT_VERSION); bool success = false; - if (datValue.get_data() != nullptr) { + bool ret = ReadKey(ssKey, ssValue); + if (ret) { // Unserialize value try { - CDataStream ssValue((char*)datValue.get_data(), (char*)datValue.get_data() + datValue.get_size(), SER_DISK, CLIENT_VERSION); ssValue >> value; success = true; } catch (const std::exception&) { // In this case success remains 'false' } } - return ret == 0 && success; + return ret && success; } template <typename K, typename T> bool Write(const K& key, const T& value, bool fOverwrite = true) { - if (!pdb) - return true; - if (fReadOnly) - assert(!"Write called on database in read-only mode"); - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - SafeDbt datKey(ssKey.data(), ssKey.size()); // Value CDataStream ssValue(SER_DISK, CLIENT_VERSION); ssValue.reserve(10000); ssValue << value; - SafeDbt datValue(ssValue.data(), ssValue.size()); // Write - int ret = pdb->put(activeTxn, datKey, datValue, (fOverwrite ? 0 : DB_NOOVERWRITE)); - return (ret == 0); + return WriteKey(ssKey, ssValue, fOverwrite); } template <typename K> bool Erase(const K& key) { - if (!pdb) - return false; - if (fReadOnly) - assert(!"Erase called on database in read-only mode"); - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - SafeDbt datKey(ssKey.data(), ssKey.size()); // Erase - int ret = pdb->del(activeTxn, datKey, 0); - return (ret == 0 || ret == DB_NOTFOUND); + return EraseKey(ssKey); } template <typename K> bool Exists(const K& key) { - if (!pdb) - return false; - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - SafeDbt datKey(ssKey.data(), ssKey.size()); // Exists - int ret = pdb->exists(activeTxn, datKey, 0); - return (ret == 0); + return HasKey(ssKey); } Dbc* GetCursor(); diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp index c83e598825..720877ead0 100644 --- a/src/wallet/coincontrol.cpp +++ b/src/wallet/coincontrol.cpp @@ -10,6 +10,7 @@ void CCoinControl::SetNull() { destChange = CNoDestination(); m_change_type.reset(); + m_add_inputs = true; fAllowOtherInputs = false; fAllowWatchOnly = false; m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS); @@ -23,4 +24,3 @@ void CCoinControl::SetNull() m_min_depth = DEFAULT_MIN_DEPTH; m_max_depth = DEFAULT_MAX_DEPTH; } - diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 2893d0ab3d..c499b0ff25 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -26,6 +26,8 @@ public: CTxDestination destChange; //! Override the default change type if set, ignored if destChange is set Optional<OutputType> m_change_type; + //! If false, only selected inputs are used + bool m_add_inputs; //! If false, allows unselected inputs, but requires all selected inputs be used bool fAllowOtherInputs; //! Includes watch only addresses which are solvable diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d9fe741a6e..541675bcef 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -297,7 +297,7 @@ UniValue importaddress(const JSONRPCRequest& request) pwallet->ImportScripts(scripts, 0 /* timestamp */); if (fP2SH) { - scripts.insert(GetScriptForDestination(ScriptHash(CScriptID(redeem_script)))); + scripts.insert(GetScriptForDestination(ScriptHash(redeem_script))); } pwallet->ImportScriptPubKeys(strLabel, scripts, false /* have_solving_data */, true /* apply_label */, 1 /* timestamp */); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c3a64cf46a..72998c30fd 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2918,13 +2918,12 @@ static UniValue listunspent(const JSONRPCRequest& request) return results; } -void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options) +void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl) { // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - CCoinControl coinControl; change_position = -1; bool lockUnspents = false; UniValue subtractFeeFromOutputs; @@ -2939,6 +2938,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f RPCTypeCheckArgument(options, UniValue::VOBJ); RPCTypeCheckObj(options, { + {"add_inputs", UniValueType(UniValue::VBOOL)}, {"changeAddress", UniValueType(UniValue::VSTR)}, {"changePosition", UniValueType(UniValue::VNUM)}, {"change_type", UniValueType(UniValue::VSTR)}, @@ -2952,6 +2952,10 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f }, true, true); + if (options.exists("add_inputs") ) { + coinControl.m_add_inputs = options["add_inputs"].get_bool(); + } + if (options.exists("changeAddress")) { CTxDestination dest = DecodeDestination(options["changeAddress"].get_str()); @@ -3039,8 +3043,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f static UniValue fundrawtransaction(const JSONRPCRequest& request) { RPCHelpMan{"fundrawtransaction", - "\nAdd inputs to a transaction until it has enough in value to meet its out value.\n" - "This will not modify existing inputs, and will add at most one change output to the outputs.\n" + "\nIf the transaction has no inputs, they will be automatically selected to meet its out value.\n" + "It will add at most one change output to the outputs.\n" "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" @@ -3054,6 +3058,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", { + {"add_inputs", RPCArg::Type::BOOL, /* default */ "true", "For a transaction with existing inputs, automatically include more if they are not enough."}, {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, @@ -3123,7 +3128,10 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) CAmount fee; int change_position; - FundTransaction(pwallet, tx, fee, change_position, request.params[1]); + CCoinControl coin_control; + // Automatically select (additional) coins. Can be overriden by options.add_inputs. + coin_control.m_add_inputs = true; + FundTransaction(pwallet, tx, fee, change_position, request.params[1], coin_control); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -3517,7 +3525,7 @@ public: UniValue operator()(const PKHash& pkhash) const { - CKeyID keyID(pkhash); + CKeyID keyID{ToKeyID(pkhash)}; UniValue obj(UniValue::VOBJ); CPubKey vchPubKey; if (provider && provider->GetPubKey(keyID, vchPubKey)) { @@ -3542,7 +3550,7 @@ public: { UniValue obj(UniValue::VOBJ); CPubKey pubkey; - if (provider && provider->GetPubKey(CKeyID(id), pubkey)) { + if (provider && provider->GetPubKey(ToKeyID(id), pubkey)) { obj.pushKV("pubkey", HexStr(pubkey)); } return obj; @@ -3623,12 +3631,10 @@ UniValue getaddressinfo(const JSONRPCRequest& request) {RPCResult::Type::STR_HEX, "pubkey", /* optional */ true, "The hex value of the raw public key for single-key addresses (possibly embedded in P2SH or P2WSH)."}, {RPCResult::Type::OBJ, "embedded", /* optional */ true, "Information about the address embedded in P2SH or P2WSH, if relevant and known.", { - {RPCResult::Type::ELISION, "", "Includes all\n" - " getaddressinfo output fields for the embedded address, excluding metadata (timestamp, hdkeypath,\n" - "hdseedid) and relation to the wallet (ismine, iswatchonly)."}, + {RPCResult::Type::ELISION, "", "Includes all getaddressinfo output fields for the embedded address, excluding metadata (timestamp, hdkeypath, hdseedid)\n" + "and relation to the wallet (ismine, iswatchonly)."}, }}, {RPCResult::Type::BOOL, "iscompressed", /* optional */ true, "If the pubkey is compressed."}, - {RPCResult::Type::STR, "label", "DEPRECATED. The label associated with the address. Defaults to \"\". Replaced by the labels array below."}, {RPCResult::Type::NUM_TIME, "timestamp", /* optional */ true, "The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR, "hdkeypath", /* optional */ true, "The HD keypath, if the key is HD and available."}, {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "The Hash160 of the HD seed."}, @@ -3636,12 +3642,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) {RPCResult::Type::ARR, "labels", "Array of labels associated with the address. Currently limited to one label but returned\n" "as an array to keep the API stable if multiple labels are enabled in the future.", { - {RPCResult::Type::STR, "label name", "The label name. Defaults to \"\"."}, - {RPCResult::Type::OBJ, "", "label data, DEPRECATED, will be removed in 0.21. To re-enable, launch bitcoind with `-deprecatedrpc=labelspurpose`", - { - {RPCResult::Type::STR, "name", "The label name. Defaults to \"\"."}, - {RPCResult::Type::STR, "purpose", "The purpose of the associated address (send or receive)."}, - }}, + {RPCResult::Type::STR, "label name", "Label name (defaults to \"\")."}, }}, } }, @@ -3687,14 +3688,6 @@ UniValue getaddressinfo(const JSONRPCRequest& request) UniValue detail = DescribeWalletAddress(pwallet, dest); ret.pushKVs(detail); - // DEPRECATED: Return label field if existing. Currently only one label can - // be associated with an address, so the label should be equivalent to the - // value of the name key/value pair in the labels array below. - const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); - if (pwallet->chain().rpcEnableDeprecated("label") && address_book_entry) { - ret.pushKV("label", address_book_entry->GetLabel()); - } - ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey); @@ -3715,14 +3708,9 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // stable if we allow multiple labels to be associated with an address in // the future. UniValue labels(UniValue::VARR); + const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); if (address_book_entry) { - // DEPRECATED: The previous behavior of returning an array containing a - // JSON object of `name` and `purpose` key/value pairs is deprecated. - if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { - labels.push_back(AddressBookDataToJSON(*address_book_entry, true)); - } else { - labels.push_back(address_book_entry->GetLabel()); - } + labels.push_back(address_book_entry->GetLabel()); } ret.pushKV("labels", std::move(labels)); @@ -3976,16 +3964,16 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) { RPCHelpMan{"walletcreatefundedpsbt", - "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" + "\nCreates and funds a transaction in the Partially Signed Transaction format.\n" "Implements the Creator and Updater roles.\n", { - {"inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The inputs", + {"inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The inputs. Leave empty to add inputs automatically. See add_inputs option.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, - {"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"}, + {"sequence", RPCArg::Type::NUM, /* default */ "depends on the value of the 'locktime' and 'options.replaceable' arguments", "The sequence number"}, }, }, }, @@ -4010,6 +3998,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) {"locktime", RPCArg::Type::NUM, /* default */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { + {"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."}, {"changeAddress", RPCArg::Type::STR_HEX, /* default */ "pool address", "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, @@ -4071,7 +4060,11 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) rbf = replaceable_arg.isTrue(); } CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); - FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); + CCoinControl coin_control; + // Automatically select coins, unless at least one is manually selected. Can + // be overriden by options.add_inputs. + coin_control.m_add_inputs = rawTx.vin.size() == 0; + FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], coin_control); // Make a blank psbt PartiallySignedTransaction psbtx(rawTx); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 70067ebef0..d42950ee42 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -116,7 +116,7 @@ bool RecoverDatabaseFile(const fs::path& file_path) } DbTxn* ptxn = env->TxnBegin(); - CWallet dummyWallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy()); + CWallet dummyWallet(nullptr, WalletLocation(), CreateDummyWalletDatabase()); for (KeyValPair& row : salvagedData) { /* Filter for only private key type KV pairs to be added to the salvaged wallet */ diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 8a2a798644..d57f99a205 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -573,9 +573,8 @@ bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std:: SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { - CKeyID key_id(pkhash); CKey key; - if (!GetKey(key_id, key)) { + if (!GetKey(ToKeyID(pkhash), key)) { return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; } @@ -585,8 +584,11 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con return SigningResult::SIGNING_FAILED; } -TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs) const +TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const { + if (n_signed) { + *n_signed = 0; + } for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); @@ -617,6 +619,14 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb SignatureData sigdata; input.FillSignatureData(sigdata); SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, sighash_type); + + bool signed_one = PSBTInputSigned(input); + if (n_signed && (signed_one || !sign)) { + // If sign is false, we assume that we _could_ sign if we get here. This + // will never have false negatives; it is hard to tell under what i + // circumstances it could have false positives. + (*n_signed)++; + } } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change @@ -2052,9 +2062,8 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message, return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; } - CKeyID key_id(pkhash); CKey key; - if (!keys->GetKey(key_id, key)) { + if (!keys->GetKey(ToKeyID(pkhash), key)) { return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; } @@ -2064,8 +2073,11 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message, return SigningResult::OK; } -TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs) const +TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const { + if (n_signed) { + *n_signed = 0; + } for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { const CTxIn& txin = psbtx.tx->vin[i]; PSBTInput& input = psbtx.inputs.at(i); @@ -2117,6 +2129,14 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& } SignPSBTInput(HidingSigningProvider(keys.get(), !sign, !bip32derivs), psbtx, i, sighash_type); + + bool signed_one = PSBTInputSigned(input); + if (n_signed && (signed_one || !sign)) { + // If sign is false, we assume that we _could_ sign if we get here. This + // will never have false negatives; it is hard to tell under what i + // circumstances it could have false positives. + (*n_signed)++; + } } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index d62d30f339..9fa2a68284 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -234,7 +234,7 @@ public: /** Sign a message with the given script */ virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; }; /** Adds script and derivation path information to a PSBT, and optionally signs it. */ - virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false) const { return TransactionError::INVALID_PSBT; } + virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const { return TransactionError::INVALID_PSBT; } virtual uint256 GetID() const { return uint256(); } @@ -393,7 +393,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; uint256 GetID() const override; @@ -596,7 +596,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; uint256 GetID() const override; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 657d0828f2..1deedede4c 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -29,7 +29,7 @@ typedef std::set<CInputCoin> CoinSet; static std::vector<COutput> vCoins; static NodeContext testNode; static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy()); +static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase()); static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); @@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Make sure that can use BnB when there are preset inputs empty_wallet(); { - std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()); + std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); wallet->SetupLegacyScriptPubKeyMan(); diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 4c0e4dc653..e416f16044 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK compressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(pubkeys[0]); @@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK uncompressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey); @@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH compressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0])); @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH uncompressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey)); @@ -103,7 +103,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2SH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -163,11 +163,11 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); - CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(PKHash(pubkeys[0]))); + CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0])); scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessscript)); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(witnessscript)); @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2WSH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -197,12 +197,12 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH compressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); - scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(PKHash(pubkeys[0]))); + scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0])); // Keystore implicitly has key and P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); @@ -212,12 +212,12 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH uncompressed { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); - scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(PKHash(uncompressedPubkey))); + scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(uncompressedPubkey)); // Keystore has key, but no P2SH redeemScript result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); @@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // scriptPubKey multisig { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH multisig { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with compressed keys { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with uncompressed key { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig wrapped in P2SH { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // OP_RETURN { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unspendable { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unknown { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -404,7 +404,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Nonstandard { - CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp index 757865ea37..4f12079768 100644 --- a/src/wallet/test/scriptpubkeyman_tests.cpp +++ b/src/wallet/test/scriptpubkeyman_tests.cpp @@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(CanProvide) // Set up wallet and keyman variables. NodeContext node; std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan(); // Make a 1 of 2 multisig script diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 7ba3148ff3..44f9eb5ddd 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -6,7 +6,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), - m_wallet(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()) + m_wallet(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 6f8c973f25..497ccd14bb 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -80,7 +80,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions fails to read an unknown start block. { - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -99,7 +99,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -125,7 +125,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -150,7 +150,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions scans no blocks. { - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -189,7 +189,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash())); AddWallet(wallet); @@ -254,7 +254,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); @@ -276,7 +276,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); @@ -312,7 +312,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) NodeContext node; auto chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); CWalletTx wtx(&wallet, m_coinbase_txns.back()); @@ -486,7 +486,7 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()); + wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()); { LOCK2(wallet->cs_wallet, ::cs_main); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -604,7 +604,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) { NodeContext node; auto chain = interfaces::MakeChain(node); - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateDummy()); + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cf42c1cb3c..57eec9baf9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2160,6 +2160,11 @@ void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const } for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { + // Only consider selected coins if add_inputs is false + if (coinControl && !coinControl->m_add_inputs && !coinControl->IsSelected(COutPoint(entry.first, i))) { + continue; + } + if (wtx.tx->vout[i].nValue < nMinimumAmount || wtx.tx->vout[i].nValue > nMaximumAmount) continue; @@ -2471,8 +2476,11 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, return false; } -TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs) const +TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs, size_t * n_signed) const { + if (n_signed) { + *n_signed = 0; + } LOCK(cs_wallet); // Get all of the previous transactions for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { @@ -2503,10 +2511,15 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp // Fill in information from ScriptPubKeyMans for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { - TransactionError res = spk_man->FillPSBT(psbtx, sighash_type, sign, bip32derivs); + int n_signed_this_spkm = 0; + TransactionError res = spk_man->FillPSBT(psbtx, sighash_type, sign, bip32derivs, &n_signed_this_spkm); if (res != TransactionError::OK) { return res; } + + if (n_signed) { + (*n_signed) += n_signed_this_spkm; + } } // Complete if every input is now signed @@ -3705,7 +3718,7 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b } // Keep same database environment instance across Verify/Recover calls below. - std::unique_ptr<WalletDatabase> database = WalletDatabase::Create(wallet_path); + std::unique_ptr<WalletDatabase> database = CreateWalletDatabase(wallet_path); try { if (!WalletBatch::VerifyEnvironment(wallet_path, error_string)) { @@ -3729,7 +3742,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.GetBoolArg("-zapwallettxes", false)) { chain.initMessage(_("Zapping all transactions from wallet...").translated); - std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(&chain, location, WalletDatabase::Create(location.GetPath())); + std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(&chain, location, CreateWalletDatabase(location.GetPath())); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DBErrors::LOAD_OK) { error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); @@ -3743,7 +3756,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, bool fFirstRun = true; // 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, location, WalletDatabase::Create(location.GetPath())), ReleaseWallet); + std::shared_ptr<CWallet> walletInstance(new CWallet(&chain, location, CreateWalletDatabase(location.GetPath())), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cf000b0b70..9931671fb4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -964,7 +964,8 @@ public: bool& complete, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, - bool bip32derivs = true) const; + bool bip32derivs = true, + size_t* n_signed = nullptr) const; /** * Create a new transaction paying the recipients with a set of coins diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 2f6c2b883a..603887ee58 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -439,10 +439,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, // Extract some CHDChain info from this metadata if it has any if (keyMeta.nVersion >= CKeyMetadata::VERSION_WITH_HDDATA && !keyMeta.hd_seed_id.IsNull() && keyMeta.hdKeypath.size() > 0) { // Get the path from the key origin or from the path string - // Not applicable when path is "s" as that indicates a seed + // Not applicable when path is "s" or "m" as those indicate a seed + // See https://github.com/bitcoin/bitcoin/pull/12924 bool internal = false; uint32_t index = 0; - if (keyMeta.hdKeypath != "s") { + if (keyMeta.hdKeypath != "s" && keyMeta.hdKeypath != "m") { std::vector<uint32_t> path; if (keyMeta.has_key_origin) { // We have a key origin, so pull it from its path vector @@ -1023,3 +1024,22 @@ bool IsWalletLoaded(const fs::path& wallet_path) { return IsBDBWalletLoaded(wallet_path); } + +/** Return object for accessing database at specified path. */ +std::unique_ptr<BerkeleyDatabase> CreateWalletDatabase(const fs::path& path) +{ + std::string filename; + return MakeUnique<BerkeleyDatabase>(GetWalletEnv(path, filename), std::move(filename)); +} + +/** Return object for accessing dummy database with no read/write capabilities. */ +std::unique_ptr<BerkeleyDatabase> CreateDummyWalletDatabase() +{ + return MakeUnique<BerkeleyDatabase>(); +} + +/** Return object for accessing temporary in-memory database. */ +std::unique_ptr<BerkeleyDatabase> CreateMockWalletDatabase() +{ + return MakeUnique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), ""); +} diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 1037bd579f..61e0f19e56 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -293,4 +293,13 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, st /** Return whether a wallet database is currently loaded. */ bool IsWalletLoaded(const fs::path& wallet_path); +/** Return object for accessing database at specified path. */ +std::unique_ptr<BerkeleyDatabase> CreateWalletDatabase(const fs::path& path); + +/** Return object for accessing dummy database with no read/write capabilities. */ +std::unique_ptr<BerkeleyDatabase> CreateDummyWalletDatabase(); + +/** Return object for accessing temporary in-memory database. */ +std::unique_ptr<BerkeleyDatabase> CreateMockWalletDatabase(); + #endif // BITCOIN_WALLET_WALLETDB_H diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index be07c28503..77ed6beb5d 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -28,7 +28,7 @@ static std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs:: return nullptr; } // dummy chain interface - std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), WalletDatabase::Create(path)), WalletToolReleaseWallet); + std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), CreateWalletDatabase(path)), WalletToolReleaseWallet); LOCK(wallet_instance->cs_wallet); bool first_run = true; DBErrors load_wallet_ret = wallet_instance->LoadWallet(first_run); @@ -57,7 +57,7 @@ static std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::pa } // dummy chain interface - std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), WalletDatabase::Create(path)), WalletToolReleaseWallet); + std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), CreateWalletDatabase(path)), WalletToolReleaseWallet); DBErrors load_wallet_ret; try { bool first_run; @@ -107,7 +107,7 @@ static void WalletShowInfo(CWallet* wallet_instance) static bool SalvageWallet(const fs::path& path) { // Create a Database handle to allow for the db to be initialized before recovery - std::unique_ptr<WalletDatabase> database = WalletDatabase::Create(path); + std::unique_ptr<WalletDatabase> database = CreateWalletDatabase(path); // Initialize the environment before recovery bilingual_str error_string; diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py index 0ab309f9b9..5cddd6527e 100755 --- a/test/functional/feature_backwards_compatibility.py +++ b/test/functional/feature_backwards_compatibility.py @@ -27,8 +27,6 @@ from test_framework.descriptors import descsum_create from test_framework.util import ( assert_equal, - sync_blocks, - sync_mempools, ) @@ -65,7 +63,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): def run_test(self): self.nodes[0].generatetoaddress(101, self.nodes[0].getnewaddress()) - sync_blocks(self.nodes) + self.sync_blocks() # Sanity check the test framework: res = self.nodes[self.num_nodes - 1].getblockchaininfo() @@ -90,17 +88,17 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): # Create a confirmed transaction, receiving coins address = wallet.getnewaddress() self.nodes[0].sendtoaddress(address, 10) - sync_mempools(self.nodes) + self.sync_mempools() self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # Create a conflicting transaction using RBF return_address = self.nodes[0].getnewaddress() tx1_id = self.nodes[1].sendtoaddress(return_address, 1) tx2_id = self.nodes[1].bumpfee(tx1_id)["txid"] # Confirm the transaction - sync_mempools(self.nodes) + self.sync_mempools() self.nodes[0].generate(1) - sync_blocks(self.nodes) + self.sync_blocks() # Create another conflicting transaction using RBF tx3_id = self.nodes[1].sendtoaddress(return_address, 1) tx4_id = self.nodes[1].bumpfee(tx3_id)["txid"] @@ -313,12 +311,19 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): self.start_node(4) # Open most recent wallet in v0.16 (no loadwallet RPC) - self.stop_node(5) - self.start_node(5, extra_args=["-wallet=w2"]) + self.restart_node(5, extra_args=["-wallet=w2"]) wallet = node_v16.get_wallet_rpc("w2") info = wallet.getwalletinfo() assert info['keypoolsize'] == 1 + # Create upgrade wallet in v0.16 + self.restart_node(-1, extra_args=["-wallet=u1_v16"]) + wallet = node_v16.get_wallet_rpc("u1_v16") + v16_addr = wallet.getnewaddress('', "bech32") + v16_info = wallet.validateaddress(v16_addr) + v16_pubkey = v16_info['pubkey'] + self.stop_node(-1) + self.log.info("Test wallet upgrade path...") # u1: regular wallet, created with v0.17 node_v17.rpc.createwallet(wallet_name="u1_v17") @@ -328,6 +333,30 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): hdkeypath = v17_info["hdkeypath"] pubkey = v17_info["pubkey"] + # Copy the 0.16 wallet to the last Bitcoin Core version and open it: + shutil.copyfile( + os.path.join(node_v16_wallets_dir, "wallets/u1_v16"), + os.path.join(node_master_wallets_dir, "u1_v16") + ) + load_res = node_master.loadwallet("u1_v16") + # Make sure this wallet opens without warnings. See https://github.com/bitcoin/bitcoin/pull/19054 + assert_equal(load_res['warning'], '') + wallet = node_master.get_wallet_rpc("u1_v16") + info = wallet.getaddressinfo(v16_addr) + descriptor = "wpkh([" + info["hdmasterfingerprint"] + hdkeypath[1:] + "]" + v16_pubkey + ")" + assert_equal(info["desc"], descsum_create(descriptor)) + + # Now copy that same wallet back to 0.16 to make sure no automatic upgrade breaks it + os.remove(os.path.join(node_v16_wallets_dir, "wallets/u1_v16")) + shutil.copyfile( + os.path.join(node_master_wallets_dir, "u1_v16"), + os.path.join(node_v16_wallets_dir, "wallets/u1_v16") + ) + self.start_node(-1, extra_args=["-wallet=u1_v16"]) + wallet = node_v16.get_wallet_rpc("u1_v16") + info = wallet.validateaddress(v16_addr) + assert_equal(info, v16_info) + # Copy the 0.17 wallet to the last Bitcoin Core version and open it: node_v17.unloadwallet("u1_v17") shutil.copytree( diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py index 82f1331685..0a457ca17f 100755 --- a/test/functional/feature_loadblock.py +++ b/test/functional/feature_loadblock.py @@ -71,8 +71,7 @@ class LoadblockTest(BitcoinTestFramework): check=True) self.log.info("Restart second, unsynced node with bootstrap file") - self.stop_node(1) - self.start_node(1, ["-loadblock=" + bootstrap_file]) + self.restart_node(1, extra_args=["-loadblock=" + bootstrap_file]) assert_equal(self.nodes[1].getblockcount(), 100) # start_node is blocking on all block files being imported assert_equal(self.nodes[1].getblockchaininfo()['blocks'], 100) diff --git a/test/functional/feature_logging.py b/test/functional/feature_logging.py index e4bf2d849d..afcbcf099a 100755 --- a/test/functional/feature_logging.py +++ b/test/functional/feature_logging.py @@ -67,8 +67,7 @@ class LoggingTest(BitcoinTestFramework): assert not os.path.isfile(default_log_path) # just sanity check no crash here - self.stop_node(0) - self.start_node(0, ["-debuglogfile=%s" % os.devnull]) + self.restart_node(0, ["-debuglogfile=%s" % os.devnull]) if __name__ == '__main__': diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 33a308ad1b..dd4c318cee 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -93,6 +93,7 @@ class NotificationsTest(BitcoinTestFramework): self.nodes[0].sethdseed(seed=self.nodes[1].dumpprivkey(keyhash_to_p2pkh(hex_str_to_bytes(self.nodes[1].getwalletinfo()['hdseedid'])[::-1]))) self.nodes[0].rescanblockchain() self.nodes[0].generatetoaddress(100, ADDRESS_BCRT1_UNSPENDABLE) + self.sync_blocks() # Generate transaction on node 0, sync mempools, and check for # notification on node 1. diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index c9362cf5aa..e46e5aacc8 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -263,8 +263,7 @@ class PruneTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "not in prune mode", node.pruneblockchain, 500) # now re-start in manual pruning mode - self.stop_node(node_number) - self.start_node(node_number, extra_args=["-prune=1"]) + self.restart_node(node_number, extra_args=["-prune=1"]) node = self.nodes[node_number] assert_equal(node.getblockcount(), 995) @@ -326,16 +325,14 @@ class PruneTest(BitcoinTestFramework): assert not has_block(3), "blk00003.dat is still there, should be pruned by now" # stop node, start back up with auto-prune at 550 MiB, make sure still runs - self.stop_node(node_number) - self.start_node(node_number, extra_args=["-prune=550"]) + self.restart_node(node_number, extra_args=["-prune=550"]) self.log.info("Success") def wallet_test(self): # check that the pruning node's wallet is still in good shape self.log.info("Stop and start pruning node to trigger wallet rescan") - self.stop_node(2) - self.start_node(2, extra_args=["-prune=550"]) + self.restart_node(2, extra_args=["-prune=550"]) self.log.info("Success") # check that wallet loads successfully when restarting a pruned node after IBD. @@ -344,8 +341,7 @@ class PruneTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 5) nds = [self.nodes[0], self.nodes[5]] self.sync_blocks(nds, wait=5, timeout=300) - self.stop_node(5) # stop and start to trigger rescan - self.start_node(5, extra_args=["-prune=550"]) + self.restart_node(5, extra_args=["-prune=550"]) # restart to trigger rescan self.log.info("Success") def run_test(self): diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 2298485640..5195d20dcb 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -559,8 +559,7 @@ class SegWitTest(BitcoinTestFramework): assert_equal(self.nodes[1].listtransactions("*", 1, 0, True)[0]["txid"], txid) # Assert it is properly saved - self.stop_node(1) - self.start_node(1) + self.restart_node(1) assert_equal(self.nodes[1].gettransaction(txid, True)["txid"], txid) assert_equal(self.nodes[1].listtransactions("*", 1, 0, True)[0]["txid"], txid) diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 7530e7daf6..80003aca0d 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -3,9 +3,15 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test bitcoin-cli""" + from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_process_error, get_auth_cookie +from test_framework.util import ( + assert_equal, + assert_raises_process_error, + assert_raises_rpc_error, + get_auth_cookie, +) # The block reward of coinbaseoutput.nValue (50) BTC/block matures after # COINBASE_MATURITY (100) blocks. Therefore, after mining 101 blocks we expect @@ -13,6 +19,12 @@ from test_framework.util import assert_equal, assert_raises_process_error, get_a BLOCKS = 101 BALANCE = (BLOCKS - 100) * 50 +JSON_PARSING_ERROR = 'error: Error parsing JSON: foo' +BLOCKS_VALUE_OF_ZERO = 'error: the first argument (number of blocks to generate, default: 1) must be an integer value greater than zero' +TOO_MANY_ARGS = 'error: too many arguments (maximum 2 for nblocks and maxtries)' +WALLET_NOT_LOADED = 'Requested wallet does not exist or is not loaded' +WALLET_NOT_SPECIFIED = 'Wallet file not specified' + class TestBitcoinCli(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -75,7 +87,7 @@ class TestBitcoinCli(BitcoinTestFramework): assert_equal(cli_get_info['relayfee'], network_info['relayfee']) assert_equal(self.nodes[0].cli.getwalletinfo(), wallet_info) - # Setup to test -getinfo and -rpcwallet= with multiple wallets. + # Setup to test -getinfo, -generate, and -rpcwallet= with multiple wallets. wallets = ['', 'Encrypted', 'secret'] amounts = [BALANCE + Decimal('9.999928'), Decimal(9), Decimal(31)] self.nodes[0].createwallet(wallet_name=wallets[1]) @@ -83,6 +95,8 @@ class TestBitcoinCli(BitcoinTestFramework): w1 = self.nodes[0].get_wallet_rpc(wallets[0]) w2 = self.nodes[0].get_wallet_rpc(wallets[1]) w3 = self.nodes[0].get_wallet_rpc(wallets[2]) + rpcwallet2 = '-rpcwallet={}'.format(wallets[1]) + rpcwallet3 = '-rpcwallet={}'.format(wallets[2]) w1.walletpassphrase(password, self.rpc_timeout) w2.encryptwallet(password) w1.sendtoaddress(w2.getnewaddress(), amounts[1]) @@ -123,17 +137,93 @@ class TestBitcoinCli(BitcoinTestFramework): assert_equal(cli_get_info['balance'], amounts[1]) self.log.info("Test -getinfo with -rpcwallet=remaining-non-default-wallet returns only its balance") - cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[1])).send_cli() + cli_get_info = self.nodes[0].cli('-getinfo', rpcwallet2).send_cli() assert 'balances' not in cli_get_info.keys() assert_equal(cli_get_info['balance'], amounts[1]) self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balances") - cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[2])).send_cli() + cli_get_info = self.nodes[0].cli('-getinfo', rpcwallet3).send_cli() assert 'balance' not in cli_get_info_keys assert 'balances' not in cli_get_info_keys + + # Test bitcoin-cli -generate. + n1 = 3 + n2 = 4 + w2.walletpassphrase(password, self.rpc_timeout) + blocks = self.nodes[0].getblockcount() + + self.log.info('Test -generate with no args') + generate = self.nodes[0].cli('-generate').send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), 1) + assert_equal(self.nodes[0].getblockcount(), blocks + 1) + + self.log.info('Test -generate with bad args') + assert_raises_process_error(1, JSON_PARSING_ERROR, self.nodes[0].cli('-generate', 'foo').echo) + assert_raises_process_error(1, BLOCKS_VALUE_OF_ZERO, self.nodes[0].cli('-generate', 0).echo) + assert_raises_process_error(1, TOO_MANY_ARGS, self.nodes[0].cli('-generate', 1, 2, 3).echo) + + self.log.info('Test -generate with nblocks') + generate = self.nodes[0].cli('-generate', n1).send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), n1) + assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n1) + + self.log.info('Test -generate with nblocks and maxtries') + generate = self.nodes[0].cli('-generate', n2, 1000000).send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), n2) + assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n1 + n2) + + self.log.info('Test -generate -rpcwallet in single-wallet mode') + generate = self.nodes[0].cli(rpcwallet2, '-generate').send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), 1) + assert_equal(self.nodes[0].getblockcount(), blocks + 2 + n1 + n2) + + self.log.info('Test -generate -rpcwallet=unloaded wallet raises RPC error') + assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate').echo) + assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 'foo').echo) + assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 0).echo) + assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 1, 2, 3).echo) + + # Test bitcoin-cli -generate with -rpcwallet in multiwallet mode. + self.nodes[0].loadwallet(wallets[2]) + n3 = 4 + n4 = 10 + blocks = self.nodes[0].getblockcount() + + self.log.info('Test -generate -rpcwallet with no args') + generate = self.nodes[0].cli(rpcwallet2, '-generate').send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), 1) + assert_equal(self.nodes[0].getblockcount(), blocks + 1) + + self.log.info('Test -generate -rpcwallet with bad args') + assert_raises_process_error(1, JSON_PARSING_ERROR, self.nodes[0].cli(rpcwallet2, '-generate', 'foo').echo) + assert_raises_process_error(1, BLOCKS_VALUE_OF_ZERO, self.nodes[0].cli(rpcwallet2, '-generate', 0).echo) + assert_raises_process_error(1, TOO_MANY_ARGS, self.nodes[0].cli(rpcwallet2, '-generate', 1, 2, 3).echo) + + self.log.info('Test -generate -rpcwallet with nblocks') + generate = self.nodes[0].cli(rpcwallet2, '-generate', n3).send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), n3) + assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n3) + + self.log.info('Test -generate -rpcwallet with nblocks and maxtries') + generate = self.nodes[0].cli(rpcwallet2, '-generate', n4, 1000000).send_cli() + assert_equal(set(generate.keys()), {'address', 'blocks'}) + assert_equal(len(generate["blocks"]), n4) + assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n3 + n4) + + self.log.info('Test -generate without -rpcwallet in multiwallet mode raises RPC error') + assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate').echo) + assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 'foo').echo) + assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 0).echo) + assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 1, 2, 3).echo) else: self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped") - self.nodes[0].generate(1) # maintain block parity with the wallet_compiled conditional branch + self.nodes[0].generate(25) # maintain block parity with the wallet_compiled conditional branch self.log.info("Test -version with node stopped") self.stop_node(0) @@ -145,7 +235,7 @@ class TestBitcoinCli(BitcoinTestFramework): self.nodes[0].wait_for_cookie_credentials() # ensure cookie file is available to avoid race condition blocks = self.nodes[0].cli('-rpcwait').send_cli('getblockcount') self.nodes[0].wait_for_rpc_connection() - assert_equal(blocks, BLOCKS + 1) + assert_equal(blocks, BLOCKS + 25) if __name__ == '__main__': diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 9047fc6828..09b9ebeb2d 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -69,8 +69,7 @@ class DisconnectBanTest(BitcoinTestFramework): self.nodes[1].setmocktime(old_time + 3) assert_equal(len(self.nodes[1].listbanned()), 3) - self.stop_node(1) - self.start_node(1) + self.restart_node(1) listAfterShutdown = self.nodes[1].listbanned() assert_equal("127.0.0.0/24", listAfterShutdown[0]['address']) diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py index 805cb1e84f..f939ea965c 100755 --- a/test/functional/p2p_feefilter.py +++ b/test/functional/p2p_feefilter.py @@ -10,11 +10,13 @@ import time from test_framework.messages import MSG_TX, msg_feefilter from test_framework.mininode import mininode_lock, P2PInterface from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal def hashToHex(hash): return format(hash, '064x') + # Wait up to 60 secs to see if the testnode has received all the expected invs def allInvsMatch(invsExpected, testnode): for x in range(60): @@ -24,6 +26,18 @@ def allInvsMatch(invsExpected, testnode): time.sleep(1) return False + +class FeefilterConn(P2PInterface): + feefilter_received = False + + def on_feefilter(self, message): + self.feefilter_received = True + + def assert_feefilter_received(self, recv: bool): + with mininode_lock: + assert_equal(self.feefilter_received, recv) + + class TestP2PConn(P2PInterface): def __init__(self): super().__init__() @@ -38,6 +52,7 @@ class TestP2PConn(P2PInterface): with mininode_lock: self.txinvs = [] + class FeeFilterTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -46,41 +61,54 @@ class FeeFilterTest(BitcoinTestFramework): # mempool and wallet feerate calculation based on GetFee # rounding down 3 places, leading to stranded transactions. # See issue #16499 - self.extra_args = [["-minrelaytxfee=0.00000100", "-mintxfee=0.00000100"]]*self.num_nodes + self.extra_args = [["-minrelaytxfee=0.00000100", "-mintxfee=0.00000100"]] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() def run_test(self): + self.test_feefilter_forcerelay() + self.test_feefilter() + + def test_feefilter_forcerelay(self): + self.log.info('Check that peers without forcerelay permission (default) get a feefilter message') + self.nodes[0].add_p2p_connection(FeefilterConn()).assert_feefilter_received(True) + + self.log.info('Check that peers with forcerelay permission do not get a feefilter message') + self.restart_node(0, extra_args=['-whitelist=forcerelay@127.0.0.1']) + self.nodes[0].add_p2p_connection(FeefilterConn()).assert_feefilter_received(False) + + # Restart to disconnect peers and load default extra_args + self.restart_node(0) + self.connect_nodes(1, 0) + + def test_feefilter(self): node1 = self.nodes[1] node0 = self.nodes[0] - # Get out of IBD - node1.generate(1) - self.sync_blocks() - self.nodes[0].add_p2p_connection(TestP2PConn()) + conn = self.nodes[0].add_p2p_connection(TestP2PConn()) # Test that invs are received by test connection for all txs at # feerate of .2 sat/byte node1.settxfee(Decimal("0.00000200")) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)] - assert allInvsMatch(txids, self.nodes[0].p2p) - self.nodes[0].p2p.clear_invs() + assert allInvsMatch(txids, conn) + conn.clear_invs() # Set a filter of .15 sat/byte on test connection - self.nodes[0].p2p.send_and_ping(msg_feefilter(150)) + conn.send_and_ping(msg_feefilter(150)) # Test that txs are still being received by test connection (paying .15 sat/byte) node1.settxfee(Decimal("0.00000150")) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)] - assert allInvsMatch(txids, self.nodes[0].p2p) - self.nodes[0].p2p.clear_invs() + assert allInvsMatch(txids, conn) + conn.clear_invs() # Change tx fee rate to .1 sat/byte and test they are no longer received # by the test connection node1.settxfee(Decimal("0.00000100")) [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)] - self.sync_mempools() # must be sure node 0 has received all txs + self.sync_mempools() # must be sure node 0 has received all txs # Send one transaction from node0 that should be received, so that we # we can sync the test on receipt (if node1's txs were relayed, they'd @@ -91,14 +119,15 @@ class FeeFilterTest(BitcoinTestFramework): # as well. node0.settxfee(Decimal("0.00020000")) txids = [node0.sendtoaddress(node0.getnewaddress(), 1)] - assert allInvsMatch(txids, self.nodes[0].p2p) - self.nodes[0].p2p.clear_invs() + assert allInvsMatch(txids, conn) + conn.clear_invs() # Remove fee filter and check that txs are received again - self.nodes[0].p2p.send_and_ping(msg_feefilter(0)) + conn.send_and_ping(msg_feefilter(0)) txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)] - assert allInvsMatch(txids, self.nodes[0].p2p) - self.nodes[0].p2p.clear_invs() + assert allInvsMatch(txids, conn) + conn.clear_invs() + if __name__ == '__main__': FeeFilterTest().main() diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 402f72685c..d99bc621de 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -3,12 +3,14 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test node responses to invalid network messages.""" + from test_framework.messages import ( CBlockHeader, CInv, msg_getdata, msg_headers, msg_inv, + msg_ping, MSG_TX, ser_string, ) @@ -17,6 +19,10 @@ from test_framework.mininode import ( P2PInterface, ) from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + wait_until, +) MSG_LIMIT = 4 * 1000 * 1000 # 4MB, per MAX_PROTOCOL_MESSAGE_LENGTH VALID_DATA_LIMIT = MSG_LIMIT - 5 # Account for the 5-byte length prefix @@ -42,6 +48,7 @@ class InvalidMessagesTest(BitcoinTestFramework): self.setup_clean_chain = True def run_test(self): + self.test_buffer() self.test_magic_bytes() self.test_checksum() self.test_size() @@ -49,6 +56,25 @@ class InvalidMessagesTest(BitcoinTestFramework): self.test_large_inv() self.test_resource_exhaustion() + def test_buffer(self): + self.log.info("Test message with header split across two buffers, should be received") + conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + # Create valid message + msg = conn.build_message(msg_ping(nonce=12345)) + cut_pos = 12 # Chosen at an arbitrary position within the header + # Send message in two pieces + before = int(self.nodes[0].getnettotals()['totalbytesrecv']) + conn.send_raw_message(msg[:cut_pos]) + # Wait until node has processed the first half of the message + wait_until(lambda: int(self.nodes[0].getnettotals()['totalbytesrecv']) != before) + middle = int(self.nodes[0].getnettotals()['totalbytesrecv']) + # If this assert fails, we've hit an unlikely race + # where the test framework sent a message in between the two halves + assert_equal(middle, before + cut_pos) + conn.send_raw_message(msg[cut_pos:]) + conn.sync_with_ping(timeout=1) + self.nodes[0].disconnect_p2ps() + def test_magic_bytes(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART badmsg']): @@ -95,13 +121,13 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_large_inv(self): conn = self.nodes[0].add_p2p_connection(P2PInterface()) - with self.nodes[0].assert_debug_log(['Misbehaving', 'peer=4 (0 -> 20): message inv size() = 50001']): + with self.nodes[0].assert_debug_log(['Misbehaving', '(0 -> 20): message inv size() = 50001']): msg = msg_inv([CInv(MSG_TX, 1)] * 50001) conn.send_and_ping(msg) - with self.nodes[0].assert_debug_log(['Misbehaving', 'peer=4 (20 -> 40): message getdata size() = 50001']): + with self.nodes[0].assert_debug_log(['Misbehaving', '(20 -> 40): message getdata size() = 50001']): msg = msg_getdata([CInv(MSG_TX, 1)] * 50001) conn.send_and_ping(msg) - with self.nodes[0].assert_debug_log(['Misbehaving', 'peer=4 (40 -> 60): headers message size = 2001']): + with self.nodes[0].assert_debug_log(['Misbehaving', '(40 -> 60): headers message size = 2001']): msg = msg_headers([CBlockHeader()] * 2001) conn.send_and_ping(msg) self.nodes[0].disconnect_p2ps() diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 2c200fccad..bea202855d 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -43,6 +43,12 @@ class P2PPermissionsTests(BitcoinTestFramework): True) self.checkpermission( + # no permission (even with forcerelay) + ["-whitelist=@127.0.0.1", "-whitelistforcerelay=1"], + [], + False) + + self.checkpermission( # relay permission removed (no specific permissions) ["-whitelist=127.0.0.1", "-whitelistrelay=0"], ["noban", "mempool"], diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 8803086213..25dd765442 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -1898,8 +1898,7 @@ class SegWitTest(BitcoinTestFramework): def test_upgrade_after_activation(self): """Test the behavior of starting up a segwit-aware node after the softfork has activated.""" - self.stop_node(2) - self.start_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)]) + self.restart_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)]) connect_nodes(self.nodes[0], 2) # We reconnect more than 100 blocks, give it plenty of time diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 4bc4913bda..57c8f511ac 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -271,7 +271,11 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex']) + # Should fail without add_inputs: + assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + # add_inputs is enabled by default rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -299,7 +303,10 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + # Should fail without add_inputs: + assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) + dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 @@ -330,7 +337,10 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) + # Should fail without add_inputs: + assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False}) + rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True}) + dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) totalOut = 0 matchingOuts = 0 diff --git a/test/functional/rpc_getaddressinfo_label_deprecation.py b/test/functional/rpc_getaddressinfo_label_deprecation.py deleted file mode 100755 index 09545ebce7..0000000000 --- a/test/functional/rpc_getaddressinfo_label_deprecation.py +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2020-2019 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 deprecation of the RPC getaddressinfo `label` field. It has been -superseded by the `labels` field. - -""" -from test_framework.test_framework import BitcoinTestFramework - -class GetAddressInfoLabelDeprecationTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 2 - self.setup_clean_chain = False - # Start node[0] with -deprecatedrpc=label, and node[1] without. - self.extra_args = [["-deprecatedrpc=label"], []] - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def test_label_with_deprecatedrpc_flag(self): - self.log.info("Test getaddressinfo label with -deprecatedrpc flag") - node = self.nodes[0] - address = node.getnewaddress() - info = node.getaddressinfo(address) - assert "label" in info - - def test_label_without_deprecatedrpc_flag(self): - self.log.info("Test getaddressinfo label without -deprecatedrpc flag") - node = self.nodes[1] - address = node.getnewaddress() - info = node.getaddressinfo(address) - assert "label" not in info - - def run_test(self): - """Test getaddressinfo label with and without -deprecatedrpc flag.""" - self.test_label_with_deprecatedrpc_flag() - self.test_label_without_deprecatedrpc_flag() - - -if __name__ == '__main__': - GetAddressInfoLabelDeprecationTest().main() diff --git a/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py b/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py deleted file mode 100755 index 903f5536b9..0000000000 --- a/test/functional/rpc_getaddressinfo_labels_purpose_deprecation.py +++ /dev/null @@ -1,48 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2020 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 deprecation of RPC getaddressinfo `labels` returning an array -containing a JSON object of `name` and purpose` key-value pairs. It now -returns an array containing only the label name. - -""" -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal - -LABELS_TO_TEST = frozenset({"" , "New 𝅘𝅥𝅯 $<#>&!рыба Label"}) - -class GetAddressInfoLabelsPurposeDeprecationTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 2 - self.setup_clean_chain = False - # Start node[0] with -deprecatedrpc=labelspurpose and node[1] without. - self.extra_args = [["-deprecatedrpc=labelspurpose"], []] - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def test_labels(self, node_num, label_name, expected_value): - node = self.nodes[node_num] - address = node.getnewaddress() - if label_name != "": - node.setlabel(address, label_name) - self.log.info(" set label to {}".format(label_name)) - labels = node.getaddressinfo(address)["labels"] - self.log.info(" labels = {}".format(labels)) - assert_equal(labels, expected_value) - - def run_test(self): - """Test getaddressinfo labels with and without -deprecatedrpc flag.""" - self.log.info("Test getaddressinfo labels with -deprecatedrpc flag") - for label in LABELS_TO_TEST: - self.test_labels(node_num=0, label_name=label, expected_value=[{"name": label, "purpose": "receive"}]) - - self.log.info("Test getaddressinfo labels without -deprecatedrpc flag") - for label in LABELS_TO_TEST: - self.test_labels(node_num=1, label_name=label, expected_value=[label]) - - -if __name__ == '__main__': - GetAddressInfoLabelsPurposeDeprecationTest().main() diff --git a/test/functional/rpc_getblockfilter.py b/test/functional/rpc_getblockfilter.py index bd93b6f7a4..8fa36445cd 100755 --- a/test/functional/rpc_getblockfilter.py +++ b/test/functional/rpc_getblockfilter.py @@ -7,7 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_is_hex_string, assert_raises_rpc_error, - connect_nodes, disconnect_nodes, sync_blocks + connect_nodes, disconnect_nodes ) FILTER_TYPES = ["basic"] @@ -30,7 +30,7 @@ class GetBlockFilterTest(BitcoinTestFramework): # Reorg node 0 to a new chain connect_nodes(self.nodes[0], 1) - sync_blocks(self.nodes) + self.sync_blocks() assert_equal(self.nodes[0].getblockcount(), 4) chain1_hashes = [self.nodes[0].getblockhash(block_height) for block_height in range(4)] diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 9b07c39606..660953be9b 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -8,6 +8,7 @@ from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( + assert_approx, assert_equal, assert_greater_than, assert_raises_rpc_error, @@ -85,6 +86,13 @@ class PSBTTest(BitcoinTestFramework): # Create and fund a raw tx for sending 10 BTC psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt'] + # If inputs are specified, do not automatically add more: + utxo1 = self.nodes[0].listunspent()[0] + assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[0].walletcreatefundedpsbt, [{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}) + + psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}, 0, {"add_inputs": True})['psbt'] + assert_equal(len(self.nodes[0].decodepsbt(psbtx1)['tx']['vin']), 2) + # Node 1 should not be able to add anything to it but still return the psbtx same as before psbtx = self.nodes[1].walletprocesspsbt(psbtx1)['psbt'] assert_equal(psbtx1, psbtx) @@ -152,13 +160,13 @@ class PSBTTest(BitcoinTestFramework): self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) # feeRate of 0.1 BTC / KB produces a total fee slightly below -maxtxfee (~0.05280000): - res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 0.1}) - assert_greater_than(res["fee"], 0.05) - assert_greater_than(0.06, res["fee"]) + res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 0.1, "add_inputs": True}) + assert_approx(res["fee"], 0.055, 0.005) # feeRate of 10 BTC / KB produces a total fee well above -maxtxfee # previously this was silently capped at -maxtxfee - assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 10}) + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 10, "add_inputs": True}) + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():1}, 0, {"feeRate": 10, "add_inputs": False}) # partially sign multisig things with node 1 psbtx = wmulti.walletcreatefundedpsbt(inputs=[{"txid":txid,"vout":p2wsh_pos},{"txid":txid,"vout":p2sh_pos},{"txid":txid,"vout":p2sh_p2wsh_pos}], outputs={self.nodes[1].getnewaddress():29.99}, options={'changeAddress': self.nodes[1].getrawchangeaddress()})['psbt'] @@ -239,7 +247,7 @@ class PSBTTest(BitcoinTestFramework): # replaceable arg block_height = self.nodes[0].getblockcount() unspent = self.nodes[0].listunspent()[0] - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False}, False) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False, "add_inputs": True}, False) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): assert_greater_than(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) @@ -247,7 +255,7 @@ class PSBTTest(BitcoinTestFramework): assert_equal(decoded_psbt["tx"]["locktime"], block_height+2) # Same construction with only locktime set and RBF explicitly enabled - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {"replaceable": True}, True) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {"replaceable": True, "add_inputs": True}, True) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) @@ -255,7 +263,7 @@ class PSBTTest(BitcoinTestFramework): assert_equal(decoded_psbt["tx"]["locktime"], block_height) # Same construction without optional arguments - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) @@ -264,7 +272,7 @@ class PSBTTest(BitcoinTestFramework): # Same construction without optional arguments, for a node with -walletrbf=0 unspent1 = self.nodes[1].listunspent()[0] - psbtx_info = self.nodes[1].walletcreatefundedpsbt([{"txid":unspent1["txid"], "vout":unspent1["vout"]}], [{self.nodes[2].getnewaddress():unspent1["amount"]+1}], block_height) + psbtx_info = self.nodes[1].walletcreatefundedpsbt([{"txid":unspent1["txid"], "vout":unspent1["vout"]}], [{self.nodes[2].getnewaddress():unspent1["amount"]+1}], block_height, {"add_inputs": True}) decoded_psbt = self.nodes[1].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): assert_greater_than(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) @@ -275,7 +283,7 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].walletcreatefundedpsbt([], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"changeAddress":self.nodes[1].getnewaddress()}, False) # Regression test for 14473 (mishandling of already-signed witness transaction): - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], 0, {"add_inputs": True}) complete_psbt = self.nodes[0].walletprocesspsbt(psbtx_info["psbt"]) double_processed_psbt = self.nodes[0].walletprocesspsbt(complete_psbt["psbt"]) assert_equal(complete_psbt, double_processed_psbt) @@ -467,7 +475,7 @@ class PSBTTest(BitcoinTestFramework): assert_equal(analysis['next'], 'creator') assert_equal(analysis['error'], 'PSBT is not valid. Input 0 specifies invalid prevout') - assert_raises_rpc_error(-25, 'Missing inputs', self.nodes[0].walletprocesspsbt, 'cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') + 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==') if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index c9fad91481..9d9e065158 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -31,8 +31,6 @@ from .util import ( disconnect_nodes, get_datadir_path, initialize_datadir, - sync_blocks, - sync_mempools, ) @@ -355,9 +353,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # See fPreferredDownload in net_processing. # # If further outbound connections are needed, they can be added at the beginning of the test with e.g. - # connect_nodes(self.nodes[1], 2) + # self.connect_nodes(1, 2) for i in range(self.num_nodes - 1): - connect_nodes(self.nodes[i + 1], i) + self.connect_nodes(i + 1, i) self.sync_all() def setup_nodes(self): @@ -534,11 +532,17 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): def wait_for_node_exit(self, i, timeout): self.nodes[i].process.wait(timeout) + def connect_nodes(self, a, b): + connect_nodes(self.nodes[a], b) + + def disconnect_nodes(self, a, b): + disconnect_nodes(self.nodes[a], b) + def split_network(self): """ Split the network of four nodes into nodes 0/1 and 2/3. """ - disconnect_nodes(self.nodes[1], 2) + self.disconnect_nodes(1, 2) self.sync_all(self.nodes[:2]) self.sync_all(self.nodes[2:]) @@ -546,18 +550,57 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """ Join the (previously split) network halves together. """ - connect_nodes(self.nodes[1], 2) + self.connect_nodes(1, 2) self.sync_all() - def sync_blocks(self, nodes=None, **kwargs): - sync_blocks(nodes or self.nodes, **kwargs) - - def sync_mempools(self, nodes=None, **kwargs): - sync_mempools(nodes or self.nodes, **kwargs) - - def sync_all(self, nodes=None, **kwargs): - self.sync_blocks(nodes, **kwargs) - self.sync_mempools(nodes, **kwargs) + def sync_blocks(self, nodes=None, wait=1, timeout=60): + """ + Wait until everybody has the same tip. + sync_blocks needs to be called with an rpc_connections set that has least + one node already synced to the latest, stable tip, otherwise there's a + chance it might return before all nodes are stably synced. + """ + rpc_connections = nodes or self.nodes + timeout = int(timeout * self.options.timeout_factor) + stop_time = time.time() + timeout + while time.time() <= stop_time: + best_hash = [x.getbestblockhash() for x in rpc_connections] + if best_hash.count(best_hash[0]) == len(rpc_connections): + return + # Check that each peer has at least one connection + assert (all([len(x.getpeerinfo()) for x in rpc_connections])) + time.sleep(wait) + raise AssertionError("Block sync timed out after {}s:{}".format( + timeout, + "".join("\n {!r}".format(b) for b in best_hash), + )) + + def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True): + """ + Wait until everybody has the same transactions in their memory + pools + """ + rpc_connections = nodes or self.nodes + timeout = int(timeout * self.options.timeout_factor) + stop_time = time.time() + timeout + while time.time() <= stop_time: + pool = [set(r.getrawmempool()) for r in rpc_connections] + if pool.count(pool[0]) == len(rpc_connections): + if flush_scheduler: + for r in rpc_connections: + r.syncwithvalidationinterfacequeue() + return + # Check that each peer has at least one connection + assert (all([len(x.getpeerinfo()) for x in rpc_connections])) + time.sleep(wait) + raise AssertionError("Mempool sync timed out after {}s:{}".format( + timeout, + "".join("\n {!r}".format(m) for m in pool), + )) + + def sync_all(self, nodes=None): + self.sync_blocks(nodes) + self.sync_mempools(nodes) # Private helper methods. These should not be accessed by the subclass test scripts. diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 17b2cbb971..506057f1fa 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -444,50 +444,6 @@ def connect_nodes(from_connection, node_num): wait_until(lambda: all(peer['bytesrecv_per_msg'].pop('verack', 0) == 24 for peer in from_connection.getpeerinfo())) -def sync_blocks(rpc_connections, *, wait=1, timeout=60): - """ - Wait until everybody has the same tip. - - sync_blocks needs to be called with an rpc_connections set that has least - one node already synced to the latest, stable tip, otherwise there's a - chance it might return before all nodes are stably synced. - """ - stop_time = time.time() + timeout - while time.time() <= stop_time: - best_hash = [x.getbestblockhash() for x in rpc_connections] - if best_hash.count(best_hash[0]) == len(rpc_connections): - return - # Check that each peer has at least one connection - assert (all([len(x.getpeerinfo()) for x in rpc_connections])) - time.sleep(wait) - raise AssertionError("Block sync timed out after {}s:{}".format( - timeout, - "".join("\n {!r}".format(b) for b in best_hash), - )) - - -def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True): - """ - Wait until everybody has the same transactions in their memory - pools - """ - stop_time = time.time() + timeout - while time.time() <= stop_time: - pool = [set(r.getrawmempool()) for r in rpc_connections] - if pool.count(pool[0]) == len(rpc_connections): - if flush_scheduler: - for r in rpc_connections: - r.syncwithvalidationinterfacequeue() - return - # Check that each peer has at least one connection - assert (all([len(x.getpeerinfo()) for x in rpc_connections])) - time.sleep(wait) - raise AssertionError("Mempool sync timed out after {}s:{}".format( - timeout, - "".join("\n {!r}".format(m) for m in pool), - )) - - # Transaction/Block functions ############################# diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 79a172706c..41f9bde183 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -242,8 +242,6 @@ BASE_SCRIPTS = [ 'p2p_permissions.py', 'feature_blocksdir.py', 'feature_config_args.py', - 'rpc_getaddressinfo_labels_purpose_deprecation.py', - 'rpc_getaddressinfo_label_deprecation.py', 'rpc_getdescriptorinfo.py', 'rpc_help.py', 'feature_help.py', diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 90d17a806c..8837e13005 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -95,8 +95,7 @@ class AbandonConflictTest(BitcoinTestFramework): # Restart the node with a higher min relay fee so the parent tx is no longer in mempool # TODO: redo with eviction - self.stop_node(0) - self.start_node(0, extra_args=["-minrelaytxfee=0.0001"]) + self.restart_node(0, extra_args=["-minrelaytxfee=0.0001"]) assert self.nodes[0].getmempoolinfo()['loaded'] # Verify txs no longer in either node's mempool @@ -123,8 +122,7 @@ class AbandonConflictTest(BitcoinTestFramework): balance = newbalance # Verify that even with a low min relay fee, the tx is not reaccepted from wallet on startup once abandoned - self.stop_node(0) - self.start_node(0, extra_args=["-minrelaytxfee=0.00001"]) + self.restart_node(0, extra_args=["-minrelaytxfee=0.00001"]) assert self.nodes[0].getmempoolinfo()['loaded'] assert_equal(len(self.nodes[0].getrawmempool()), 0) @@ -145,8 +143,7 @@ class AbandonConflictTest(BitcoinTestFramework): balance = newbalance # Remove using high relay fee again - self.stop_node(0) - self.start_node(0, extra_args=["-minrelaytxfee=0.0001"]) + self.restart_node(0, extra_args=["-minrelaytxfee=0.0001"]) assert self.nodes[0].getmempoolinfo()['loaded'] assert_equal(len(self.nodes[0].getrawmempool()), 0) newbalance = self.nodes[0].getbalance() diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 780cce9d02..eddd938847 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -110,9 +110,7 @@ class AvoidReuseTest(BitcoinTestFramework): assert_equal(self.nodes[0].getwalletinfo()["avoid_reuse"], False) assert_equal(self.nodes[1].getwalletinfo()["avoid_reuse"], True) - # Stop and restart node 1 - self.stop_node(1) - self.start_node(1) + self.restart_node(1) connect_nodes(self.nodes[0], 1) # Flags should still be node1.avoid_reuse=false, node2.avoid_reuse=true diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 8efa66a856..31829a18b3 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -12,7 +12,6 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, connect_nodes, - sync_blocks, ) @@ -264,7 +263,7 @@ class WalletTest(BitcoinTestFramework): # Now confirm tx_orig self.restart_node(1, ['-persistmempool=0']) connect_nodes(self.nodes[0], 1) - sync_blocks(self.nodes) + self.sync_blocks() self.nodes[1].sendrawtransaction(tx_orig) self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY) self.sync_all() diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index cc349c7bc5..ba1e494d9a 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -190,8 +190,7 @@ class WalletDumpTest(BitcoinTestFramework): assert_raises_rpc_error(-8, "already exists", lambda: self.nodes[0].dumpwallet(wallet_enc_dump)) # Restart node with new wallet, and test importwallet - self.stop_node(0) - self.start_node(0, ['-wallet=w2']) + self.restart_node(0, ['-wallet=w2']) # Make sure the address is not IsMine before import result = self.nodes[0].getaddressinfo(multisig_addr) diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index c441b75652..3c336623e2 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -103,8 +103,7 @@ class WalletHDTest(BitcoinTestFramework): self.sync_all() # Needs rescan - self.stop_node(1) - self.start_node(1, extra_args=self.extra_args[1] + ['-rescan']) + self.restart_node(1, extra_args=self.extra_args[1] + ['-rescan']) assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1) # Try a RPC based rescan @@ -183,8 +182,7 @@ class WalletHDTest(BitcoinTestFramework): self.nodes[0].generate(10) # Restart node 1 with keypool of 3 and a different wallet self.nodes[1].createwallet(wallet_name='origin', blank=True) - self.stop_node(1) - self.start_node(1, extra_args=['-keypool=3', '-wallet=origin']) + self.restart_node(1, extra_args=['-keypool=3', '-wallet=origin']) connect_nodes(self.nodes[0], 1) # sethdseed restoring and seeing txs to addresses out of the keypool diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index 497a5dd95e..455f1fc5e8 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -77,8 +77,7 @@ class ReorgsRestoreTest(BitcoinTestFramework): assert_equal(conflicted["walletconflicts"][0], conflicting["txid"]) # Node0 wallet is shutdown - self.stop_node(0) - self.start_node(0) + self.restart_node(0) # The block chain re-orgs and the tx is included in a different block self.nodes[1].generate(9) diff --git a/test/functional/wallet_zapwallettxes.py b/test/functional/wallet_zapwallettxes.py index adebff360a..7f1cdbd20b 100755 --- a/test/functional/wallet_zapwallettxes.py +++ b/test/functional/wallet_zapwallettxes.py @@ -49,17 +49,15 @@ class ZapWalletTXesTest (BitcoinTestFramework): assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - # Stop-start node0. Both confirmed and unconfirmed transactions remain in the wallet. - self.stop_node(0) - self.start_node(0) + # Restart node0. Both confirmed and unconfirmed transactions remain in the wallet. + self.restart_node(0) assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - # Stop node0 and restart with zapwallettxes and persistmempool. The unconfirmed + # Restart node0 with zapwallettxes and persistmempool. The unconfirmed # transaction is zapped from the wallet, but is re-added when the mempool is reloaded. - self.stop_node(0) - self.start_node(0, ["-persistmempool=1", "-zapwallettxes=2"]) + self.restart_node(0, ["-persistmempool=1", "-zapwallettxes=2"]) wait_until(lambda: self.nodes[0].getmempoolinfo()['size'] == 1, timeout=3) self.nodes[0].syncwithvalidationinterfacequeue() # Flush mempool to wallet @@ -67,10 +65,9 @@ class ZapWalletTXesTest (BitcoinTestFramework): assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - # Stop node0 and restart with zapwallettxes, but not persistmempool. + # Restart node0 with zapwallettxes, but not persistmempool. # The unconfirmed transaction is zapped and is no longer in the wallet. - self.stop_node(0) - self.start_node(0, ["-zapwallettxes=2"]) + self.restart_node(0, ["-zapwallettxes=2"]) # tx1 is still be available because it was confirmed assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) diff --git a/test/lint/lint-spelling.ignore-words.txt b/test/lint/lint-spelling.ignore-words.txt index a7a97eb41f..34f54325b3 100644 --- a/test/lint/lint-spelling.ignore-words.txt +++ b/test/lint/lint-spelling.ignore-words.txt @@ -14,3 +14,4 @@ setban hist ser unselect +lowercased |