From f437964457d0db9b19d32072ddb0f187c1618ea4 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 7 Sep 2021 11:20:23 +0200 Subject: [PATCH 01/11] Merge bitcoin/bitcoin#21551: ci: Move Windows MSVC build from AppVeyor to Cirrus 97292b19140db32c6d85d63b70382e7bf60a55c4 ci: Drop AppVeyor CI integration (Hennadii Stepanov) 1fb70793b237b9a3a00ff744739e512dd7755937 ci: Add Windows task to Cirrus CI (Hennadii Stepanov) Pull request description: This PR: - gets rid off unreliable AppVeyor CI - places all CI configs in one place - allows to enable functional tests in the future (using a persistent worker) | no populated `vcpkg` cache | populated `vcpkg` cache | |---|---| | ![Screenshot from 2021-09-02 15-47-04](https://user-images.githubusercontent.com/32963518/131846156-9367bffc-9093-40ca-98c3-15db74e24113.png) | ![Screenshot from 2021-09-02 14-06-26](https://user-images.githubusercontent.com/32963518/131833053-a501454d-eecf-463c-a3a4-b89d2a494058.png) | Currently, AppVeyor builds take about 44..48 minutes. ACKs for top commit: sipsorcery: re-ACK 97292b19140db32c6d85d63b70382e7bf60a55c4. Tree-SHA512: 3af50d9fd68eb12f39724810dacf948e4068573b5dfd0dbaeb05d19d4bd6f10bd9a432656dcc32b742684b438d31305eace85c602296d7a1bf84b2f1fcc06f02 --- .cirrus.yml | 75 ++++++++++++++++++++++++++++++++++++++++++++++++--- .editorconfig | 2 +- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 2f2474302fb24..a8b1c3c46d1e3 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -10,8 +10,12 @@ env: CCACHE_DIR: "/tmp/ccache_dir" # https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks -base_template: &BASE_TEMPLATE +filter_template: &FILTER_TEMPLATE skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == "" # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution + stateful: false # https://cirrus-ci.org/guide/writing-tasks/#stateful-tasks + +base_template: &BASE_TEMPLATE + << : *FILTER_TEMPLATE merge_base_script: # Unconditionally install git (used in fingerprint_script) and set the # default git author name (used in verify-commits.py) @@ -21,7 +25,6 @@ base_template: &BASE_TEMPLATE - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH - git merge FETCH_HEAD # Merge base to detect silent merge conflicts - stateful: false # https://cirrus-ci.org/guide/writing-tasks/#stateful-tasks global_task_template: &GLOBAL_TASK_TEMPLATE << : *BASE_TEMPLATE @@ -47,7 +50,6 @@ compute_credits_template: &CREDITS_TEMPLATE # Only use credits for pull requests to the main repo use_compute_credits: $CIRRUS_REPO_FULL_NAME == 'dashpay/dash' && $CIRRUS_PR != "" - task: name: 'lint [bookworm]' << : *BASE_TEMPLATE @@ -63,6 +65,73 @@ task: lint_script: - ./ci/lint_run_all.sh +task: + name: "Win64 native [unit tests, no functional tests] [msvc]" + << : *FILTER_TEMPLATE + windows_container: + cpu: 4 + memory: 16G + image: cirrusci/windowsservercore:visualstudio2019 + timeout_in: 120m + env: + CIRRUS_SHELL: powershell + PATH: 'C:\Python39;C:\Python39\Scripts;C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\Bin;%PATH%' + PYTHONUTF8: 1 + VCPKG_TAG: '75522bb1f2e7d863078bcd06322348f053a9e33f' + VCPKG_FEATURE_FLAGS: 'manifests' + QT_DOWNLOAD_URL: 'https://github.com/sipsorcery/qt_win_binary/releases/download/qt51211x64_static_vs2019_160900/Qt5.12.11_x64_static_vs2019_160900.zip' + QT_DOWNLOAD_HASH: 'b24436bbc49ac69d992efc148e640f02e8dec426bed5f8497abf735e7d7d59d0' + QT_LOCAL_PATH: 'C:\Qt5.12.11_x64_static_vs2019_160900' + IgnoreWarnIntDirInTempDetected: 'true' + merge_script: + - git config --global user.email "ci@ci.ci" + - git config --global user.name "ci" + - git config core.filemode false + - git reset --hard + - if ($env:CIRRUS_PR -eq $null) { exit 0; } + - git fetch $env:CIRRUS_REPO_CLONE_URL $env:CIRRUS_BASE_BRANCH + - git merge FETCH_HEAD + vcpkg_cache: + folder: 'C:\Users\ContainerAdministrator\AppData\Local\vcpkg\archives' + install_python_script: + - choco install --yes --no-progress python3 --version=3.9.6 + - Write-Host "" + - python -VV + install_vcpkg_script: + - cd .. + - git clone --quiet https://github.com/microsoft/vcpkg.git + - cd vcpkg + - git -c advice.detachedHead=false checkout $env:VCPKG_TAG + - .\bootstrap-vcpkg -disableMetrics + - Add-Content "triplets\x64-windows-static.cmake" "set(VCPKG_BUILD_TYPE release)" + - .\vcpkg integrate install + - Write-Host "" + - .\vcpkg version + download_qt_binaries_script: | + Invoke-WebRequest -Uri $env:QT_DOWNLOAD_URL -Out qtdownload.zip; + Write-Host "Qt binaries successfully downloaded, checking hash against $env:QT_DOWNLOAD_HASH..."; + if ((Get-FileHash qtdownload.zip).Hash -eq $env:QT_DOWNLOAD_HASH) { + Write-Host "Downloaded Qt binaries archive matched the expected hash."; + Expand-Archive qtdownload.zip -DestinationPath $env:QT_LOCAL_PATH; + } + else { + Write-Host "ERROR: Downloaded Qt binaries archive did not match the expected hash."; + exit 1; + } + build_environment_script: + - choco list --localonly + - Write-Host "" + - msbuild -version + build_script: + - cd $env:CIRRUS_WORKING_DIR + - python build_msvc\msvc-autogen.py + - msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal -noLogo + unit_tests_script: + - src\test_bitcoin.exe + - src\bench_bitcoin.exe > $null + - python test\util\test_runner.py + - python test\util\rpcauth-test.py + task: name: 'ARM [unit tests, no functional tests] [buster]' << : *GLOBAL_TASK_TEMPLATE diff --git a/.editorconfig b/.editorconfig index 4967e675f642f..ae7e92d1c8a82 100644 --- a/.editorconfig +++ b/.editorconfig @@ -13,7 +13,7 @@ trim_trailing_whitespace = true [*.{h,cpp,py,sh}] indent_size = 4 -# .cirrus.yml, .appveyor.yml, .fuzzbuzz.yml, etc. +# .cirrus.yml, .fuzzbuzz.yml, etc. [*.yml] indent_size = 2 From 1c7535437c302352146c7ff11f789f7f82cd08f1 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 11 Feb 2025 12:40:10 +0700 Subject: [PATCH 02/11] build: remove windows native build from cirrus. We don't have native windows builds, no need to track these changes --- .cirrus.yml | 68 ----------------------------------------------------- 1 file changed, 68 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index a8b1c3c46d1e3..76b15de84167f 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -64,74 +64,6 @@ task: fingerprint_script: cat .python-version /etc/os-release lint_script: - ./ci/lint_run_all.sh - -task: - name: "Win64 native [unit tests, no functional tests] [msvc]" - << : *FILTER_TEMPLATE - windows_container: - cpu: 4 - memory: 16G - image: cirrusci/windowsservercore:visualstudio2019 - timeout_in: 120m - env: - CIRRUS_SHELL: powershell - PATH: 'C:\Python39;C:\Python39\Scripts;C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\Bin;%PATH%' - PYTHONUTF8: 1 - VCPKG_TAG: '75522bb1f2e7d863078bcd06322348f053a9e33f' - VCPKG_FEATURE_FLAGS: 'manifests' - QT_DOWNLOAD_URL: 'https://github.com/sipsorcery/qt_win_binary/releases/download/qt51211x64_static_vs2019_160900/Qt5.12.11_x64_static_vs2019_160900.zip' - QT_DOWNLOAD_HASH: 'b24436bbc49ac69d992efc148e640f02e8dec426bed5f8497abf735e7d7d59d0' - QT_LOCAL_PATH: 'C:\Qt5.12.11_x64_static_vs2019_160900' - IgnoreWarnIntDirInTempDetected: 'true' - merge_script: - - git config --global user.email "ci@ci.ci" - - git config --global user.name "ci" - - git config core.filemode false - - git reset --hard - - if ($env:CIRRUS_PR -eq $null) { exit 0; } - - git fetch $env:CIRRUS_REPO_CLONE_URL $env:CIRRUS_BASE_BRANCH - - git merge FETCH_HEAD - vcpkg_cache: - folder: 'C:\Users\ContainerAdministrator\AppData\Local\vcpkg\archives' - install_python_script: - - choco install --yes --no-progress python3 --version=3.9.6 - - Write-Host "" - - python -VV - install_vcpkg_script: - - cd .. - - git clone --quiet https://github.com/microsoft/vcpkg.git - - cd vcpkg - - git -c advice.detachedHead=false checkout $env:VCPKG_TAG - - .\bootstrap-vcpkg -disableMetrics - - Add-Content "triplets\x64-windows-static.cmake" "set(VCPKG_BUILD_TYPE release)" - - .\vcpkg integrate install - - Write-Host "" - - .\vcpkg version - download_qt_binaries_script: | - Invoke-WebRequest -Uri $env:QT_DOWNLOAD_URL -Out qtdownload.zip; - Write-Host "Qt binaries successfully downloaded, checking hash against $env:QT_DOWNLOAD_HASH..."; - if ((Get-FileHash qtdownload.zip).Hash -eq $env:QT_DOWNLOAD_HASH) { - Write-Host "Downloaded Qt binaries archive matched the expected hash."; - Expand-Archive qtdownload.zip -DestinationPath $env:QT_LOCAL_PATH; - } - else { - Write-Host "ERROR: Downloaded Qt binaries archive did not match the expected hash."; - exit 1; - } - build_environment_script: - - choco list --localonly - - Write-Host "" - - msbuild -version - build_script: - - cd $env:CIRRUS_WORKING_DIR - - python build_msvc\msvc-autogen.py - - msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal -noLogo - unit_tests_script: - - src\test_bitcoin.exe - - src\bench_bitcoin.exe > $null - - python test\util\test_runner.py - - python test\util\rpcauth-test.py - task: name: 'ARM [unit tests, no functional tests] [buster]' << : *GLOBAL_TASK_TEMPLATE From df518cf99b403ecc87b09ff1208e0906438f48f2 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 16 Sep 2021 10:55:55 +0200 Subject: [PATCH 03/11] Merge bitcoin/bitcoin#22994: ci: use Debian Bullseye in ARM CI 252d1a70fb452893efe4ab64298139eb08d8ac98 ci: use Debian Bullseye in ARM CI (fanquake) Pull request description: This works around an issue when trying to use `std::filesystem::remove_all` with the ARM GCC on Buster. Has been split out of #20744. See commentary starting here: https://github.com/bitcoin/bitcoin/pull/20744#issuecomment-810279549. Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93201. ACKs for top commit: MarcoFalke: cr ACK 252d1a70fb452893efe4ab64298139eb08d8ac98 hebasto: ACK 252d1a70fb452893efe4ab64298139eb08d8ac98, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: ca71f5cb07fe06c1c7f0160935e667ffeb62bd6a1a89b54124b5633c5c176347a2207aaa5eca68938ed89db9778d357e42b677115d4ed386fa2d7d2ffa5025ad --- .cirrus.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 76b15de84167f..20432f08ca851 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -65,10 +65,10 @@ task: lint_script: - ./ci/lint_run_all.sh task: - name: 'ARM [unit tests, no functional tests] [buster]' + name: 'ARM [unit tests, no functional tests] [bullseye]' << : *GLOBAL_TASK_TEMPLATE arm_container: - image: debian:buster + image: debian:bullseye cpu: 2 memory: 8G env: From c358d841a00212d9a7c6bebb43939d1959b2aed3 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 21 Sep 2021 08:42:08 +0200 Subject: [PATCH 04/11] Merge bitcoin/bitcoin#23014: ci: Bump distro version, disable feature_bind_extra for two configurations fa660de2ac28628e22ee0c18e04e520a41b3584e ci: Update valgrind config (MarcoFalke) fad5dbc13c26901928e5b71bff047fe273f72a45 ci: Update s390x config (MarcoFalke) Pull request description: (See commit messages) ACKs for top commit: fanquake: ACK fa660de2ac28628e22ee0c18e04e520a41b3584e Tree-SHA512: b5bf7c89042d5d8f49a92c59b934185447ae42bd055ccb1c6edb98fe104a35eb2d6c26d810bec9f52d545d09599f3d33fc8606fbfb793862610d276b2a4bac6c --- ci/test/00_setup_env_native_valgrind.sh | 2 +- ci/test/00_setup_env_s390x.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index b0793313fb1a5..576bf120c46bc 100755 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -9,6 +9,6 @@ export LC_ALL=C.UTF-8 export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev" export USE_VALGRIND=1 export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="--exclude rpc_bind --timeout-factor=4" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 +export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra --timeout-factor=4" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no --enable-suppress-external-warnings CC=clang-18 CXX=clang++-18" # TODO enable GUI diff --git a/ci/test/00_setup_env_s390x.sh b/ci/test/00_setup_env_s390x.sh index a46bde95b5584..1ade1fb5cb826 100755 --- a/ci/test/00_setup_env_s390x.sh +++ b/ci/test/00_setup_env_s390x.sh @@ -19,6 +19,7 @@ fi # Use debian to avoid 404 apt errors export CONTAINER_NAME=ci_s390x export RUN_UNIT_TESTS=true +export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export RUN_FUNCTIONAL_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests --with-boost-process" # GUI tests disabled for now, see https://github.com/bitcoin/bitcoin/issues/23730 From 0e093b3f6d6646c4166d3fc35b7e6d5ef8a4cd38 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 29 Sep 2021 13:15:21 +0300 Subject: [PATCH 05/11] Merge bitcoin-core/gui#430: Improvements to the open up transaction in third-party link action 2ccde2f9329d2685563b09ff0830f0d5916f57d5 qt: hyphenate usage of third-party modifier (Jarol Rodriguez) 8177578b296adb82fb8ab6c64cd76b832ebcc132 qt: ensure seperator when adding third-party transaction links (Jarol Rodriguez) a70a98075a0d258d41c1310553a2337538a1d80c qt: improve text for open third-party tx url action (Jarol Rodriguez) 9980f4aa5eaf3c0f62cf699d6a9c4677a1ea3365 qt, refactor: simplify third-party tx url action through overload (Jarol Rodriguez) Pull request description: [#4092](https://github.com/bitcoin/bitcoin/pull/4092) introduced the ability to open up a transaction in a block explorer. This improves the related code by simplifying the addition and connection of the action through an [overloaded](https://doc.qt.io/archives/qt-5.9/qmenu.html#addAction-5) `addAction` function and prepends action description text to the host, "Show in". The reason to add this text is to make it clear what the action does. It also creates a clearer mental correlation between a user doing the work to add the 3rd-party tx link and this new menu action popping up. This updates the setting text so that "third-party" is hyphenated. It should be hyphenated because it is being used as a modifier of both "URL" and "transaction URLs". Additionally, this fixes #431 by ensuring that the seperator will be added before creating action. Screenshots of visual changes: **Context menu actions** | master | pr | |--------------|--------| | 3pt-master | 3pt-pr | **Setting text** (tooltip text containing usage of "third-party" is also properly hyphenated) | master | pr | |--------------|--------| | ![unnamed](https://user-images.githubusercontent.com/23396902/134854070-fb299ba5-3491-487f-b37f-c0cd96514353.png) | ![pr-hyphenate](https://user-images.githubusercontent.com/23396902/134854127-88630cc2-a178-4376-a569-f413f66eba0d.png) | ACKs for top commit: stratospher: tested ACK 2ccde2f. promag: Code view ACK 2ccde2f9329d2685563b09ff0830f0d5916f57d5. hebasto: ACK 2ccde2f9329d2685563b09ff0830f0d5916f57d5 Tree-SHA512: 8dfcd539a1d41c8abf3c8208d150d1480d4ef81a008de826299e8bad1dfa6e3c49dc76d041c5946fafcf0b033eebb9b9fbd3d49ba6d8af93dd388c488e92f143 --- src/qt/forms/optionsdialog.ui | 6 +++--- src/qt/transactionview.cpp | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index ca8cf04576b20..d8f462c6e4b0c 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -1033,10 +1033,10 @@ https://explore.transifex.com/dash/dash/ - Third party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items.<br/>%s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |. + Third-party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items.<br/>%s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |. - &Third party transaction URLs + &Third-party transaction URLs thirdPartyTxUrls @@ -1046,7 +1046,7 @@ https://explore.transifex.com/dash/dash/ - Third party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items.<br/>%s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |. + Third-party URLs (e.g. a block explorer) that appear in the transactions tab as context menu items.<br/>%s in the URL is replaced by transaction hash. Multiple URLs are separated by vertical bar |. https://example.com/tx/%s diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 8f3d4b0506c2c..1ad13a300a0a0 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -226,17 +226,21 @@ void TransactionView::setModel(WalletModel *_model) { // Add third party transaction URLs to context menu QStringList listUrls = GUIUtil::SplitSkipEmptyParts(_model->getOptionsModel()->getThirdPartyTxUrls(), "|"); + bool actions_created = false; for (int i = 0; i < listUrls.size(); ++i) { QString url = listUrls[i].trimmed(); QString host = QUrl(url, QUrl::StrictMode).host(); if (!host.isEmpty()) { - QAction *thirdPartyTxUrlAction = new QAction(host, this); // use host as menu item label - if (i == 0) + if (!actions_created) { contextMenu->addSeparator(); - contextMenu->addAction(thirdPartyTxUrlAction); - connect(thirdPartyTxUrlAction, &QAction::triggered, [this, url] { openThirdPartyTxUrl(url); }); + actions_created = true; + } + /*: Transactions table context menu action to show the + selected transaction in a third-party block explorer. + %1 is a stand-in argument for the URL of the explorer. */ + contextMenu->addAction(tr("Show in %1").arg(host), [this, url] { openThirdPartyTxUrl(url); }); } } From 4a82d49766b313231c4d844b09d04441f8d6f31b Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 30 Sep 2021 08:52:12 +0800 Subject: [PATCH 06/11] Merge bitcoin/bitcoin#23126: doc: update developer docs for subtree renaming 2b90eae33c0b368cc9b3939be224c36c45abd50d doc: update developer docs for subtree renaming (fanquake) Pull request description: Update the developer docs after the [recent subtree renaming](https://github.com/bitcoin/bitcoin/pull/22646#issuecomment-921154730). ACKs for top commit: hebasto: ACK 2b90eae33c0b368cc9b3939be224c36c45abd50d Tree-SHA512: ed0eec8db888e60595c07f4fad0a506673e4b10345fb2dd6d1a98d785da22bddf1fe8896aa52fd67f5e1688e3c91c6b642739e08646f1b920f50f0d35037d961 --- doc/developer-notes.md | 32 ++++++++++++++++++-------------- test/lint/README.md | 6 +++--- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 04013dfcb690e..a8035da3461ee 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -1132,37 +1132,41 @@ Subtrees Several parts of the repository are subtrees of software maintained elsewhere. -Some of these are maintained by active developers of Bitcoin Core, in which case changes should probably go -directly upstream without being PRed directly against the project. They will be merged back in the next -subtree merge. +Some of these are maintained by active developers of Dash Core, in which case +changes should go directly upstream without being PRed directly against the project. +They will be merged back in the next subtree merge. -Others are external projects without a tight relationship with our project. Changes to these should also -be sent upstream, but bugfixes may also be prudent to PR against Dash Core so that they can be integrated -quickly. Cosmetic changes should be purely taken upstream. +Others are external projects without a tight relationship with our project. Changes +to these should also be sent upstream, but bugfixes may also be prudent to PR against +a Dash Core subtree, so that they can be integrated quickly. Cosmetic changes +should be taken upstream. -There is a tool in `test/lint/git-subtree-check.sh` ([instructions](../test/lint#git-subtree-checksh)) to check a subtree directory for consistency with -its upstream repository. +There is a tool in `test/lint/git-subtree-check.sh` ([instructions](../test/lint#git-subtree-checksh)) +to check a subtree directory for consistency with its upstream repository. Current subtrees include: - src/leveldb - - Upstream at https://github.com/google/leveldb ; Maintained by Google, but - open important PRs to Core to avoid delay. + - Subtree at https://github.com/bitcoin-core/leveldb-subtree ; maintained by Core contributors. + - Upstream at https://github.com/google/leveldb ; maintained by Google. Open + important PRs to the subtree to avoid delay. - **Note**: Follow the instructions in [Upgrading LevelDB](#upgrading-leveldb) when merging upstream changes to the LevelDB subtree. - src/crc32c - Used by leveldb for hardware acceleration of CRC32C checksums for data integrity. - - Upstream at https://github.com/google/crc32c ; Maintained by Google. + - Subtree at https://github.com/bitcoin-core/crc32c-subtree ; maintained by Core contributors. + - Upstream at https://github.com/google/crc32c ; maintained by Google. - src/secp256k1 - - Upstream at https://github.com/bitcoin-core/secp256k1/ ; actively maintained by Core contributors. + - Upstream at https://github.com/bitcoin-core/secp256k1/ ; maintained by Core contributors. - src/crypto/ctaes - - Upstream at https://github.com/bitcoin-core/ctaes ; actively maintained by Core contributors. + - Upstream at https://github.com/bitcoin-core/ctaes ; maintained by Core contributors. - src/univalue - - Upstream at https://github.com/bitcoin-core/univalue ; actively maintained by Core contributors, deviates from upstream https://github.com/jgarzik/univalue + - Subtree at https://github.com/bitcoin-core/univalue-subtree ; maintained by Core contributors. + - Deviates from upstream https://github.com/jgarzik/univalue. - src/minisketch - Upstream at https://github.com/sipa/minisketch ; maintained by Core contributors. diff --git a/test/lint/README.md b/test/lint/README.md index 748a845e59941..a23211a72b30e 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -27,10 +27,10 @@ Usage: test/lint/git-subtree-check.sh [-r] DIR [COMMIT] To do a full check with `-r`, make sure that you have fetched the upstream repository branch in which the subtree is maintained: * for `src/secp256k1`: https://github.com/bitcoin-core/secp256k1.git (branch master) -* for `src/leveldb`: https://github.com/bitcoin-core/leveldb.git (branch bitcoin-fork) -* for `src/univalue`: https://github.com/bitcoin-core/univalue.git (branch master) +* for `src/leveldb`: https://github.com/bitcoin-core/leveldb-subtree.git (branch bitcoin-fork) +* for `src/univalue`: https://github.com/bitcoin-core/univalue-subtree.git (branch master) * for `src/crypto/ctaes`: https://github.com/bitcoin-core/ctaes.git (branch master) -* for `src/crc32c`: https://github.com/google/crc32c.git (branch master) +* for `src/crc32c`: https://github.com/bitcoin-core/crc32c-subtree.git (branch bitcoin-fork) * for `src/minisketch`: https://github.com/sipa/minisketch.git (branch master) To do so, add the upstream repository as remote: From 3a5f35eb052a342687ae5bc13828e703d0eb146f Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Thu, 30 Sep 2021 10:26:37 +0200 Subject: [PATCH 07/11] Merge bitcoin-core/gui#336: Do not exit and re-enter main event loop during shutdown 451ca244db8bc71ffc3cc9982d025f144cc8f3bc qt, refactor: Drop intermediate BitcoinApplication::shutdownResult slot (Hennadii Stepanov) f3a17bbe5f7d23b6ecc20e363920492b50859dad qt: Do not exit and re-enter main event loop during shutdown (Hennadii Stepanov) b4e0d2c43181ad97c15b252e95181e2c3f6c1d2a qt, refactor: Allocate SendConfirmationDialog instances on heap (Hennadii Stepanov) 332dea2852d9c68f900ed1f0be99b6cea79c7457 qt, refactor: Keep HelpMessageDialog in the main event loop (Hennadii Stepanov) c8bae37a7a646badf8e79669bf06ac174e13cd6f qt, refactor: Keep PSBTOperationsDialog in the main event loop (Hennadii Stepanov) 7fa91e831227e556bd8a7ae3da64bd59d4f30d5f qt, refactor: Keep AskPassphraseDialog in the main event loop (Hennadii Stepanov) 6f6fde30e7601185a8f6052b3bf1770407fcc14b qt, refactor: Keep EditAddressDialog in the main event loop (Hennadii Stepanov) 59f7ba4fd7a9e4bc73d784ee74d5b777da9cc436 qt, refactor: Keep CoinControlDialog in the main event loop (Hennadii Stepanov) 7830cd0b35f315570d744f4d2719104c08b33ff1 qt, refactor: Keep OptionsDialog in the main event loop (Hennadii Stepanov) 13f618818dc57673ac0287ad8b28ceb450efb374 qt: Add GUIUtil::ShowModalDialogAndDeleteOnClose (Hennadii Stepanov) Pull request description: On master (1ef34ee25ed34b2b092f15bf3dca5c0508092829) during shutdown `QApplication` exits the main event loop, then re-enter again. This PR streamlines shutdown process by removing the need to interrupt the main event loop, that is required for #59. Also, blocking [`QDialog::exec()`](https://doc.qt.io/qt-5/qdialog.html#exec) calls are replaced with safer [`QDialog::show()`](https://doc.qt.io/qt-5/qwidget.html#show), except for `SendConfirmationDialog` as that change is not trivial (marked as TODO). The [`QDialog::open()`](https://doc.qt.io/qt-5/qdialog.html#open) was not used because the actual modality mode (application modal or window modal) of a dialog depends on whether it has a parent. This PR does not change behavior, and all touched dialogs are still application modal. As a follow up, a design research could suggest to make some dialogs window modal. NOTE for reviewers: quitting app while a dialog is open (e.g., via systray icon menu) must work fine. ACKs for top commit: laanwj: Code review and lighly tested ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc promag: ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc, just changed signal to `quitRequested`. Tree-SHA512: ef01ab6ed803b202e776019a4e1f592e816f7bc786e00574b25a0bf16be2374ddf9db21f0a26da08700df7ef0ab9e879550df46dcfe3b6d940f5ed02ca5f8447 --- src/qt/addressbookpage.cpp | 8 ++++---- src/qt/bitcoin.cpp | 20 ++++++++++---------- src/qt/bitcoin.h | 5 ++--- src/qt/bitcoingui.cpp | 21 +++++++++++---------- src/qt/bitcoingui.h | 1 + src/qt/guiutil.cpp | 8 ++++++++ src/qt/guiutil.h | 6 ++++++ src/qt/optionsdialog.cpp | 3 ++- src/qt/optionsdialog.h | 1 + src/qt/sendcoinsdialog.cpp | 13 +++++++------ src/qt/transactionview.cpp | 16 ++++++++-------- src/qt/walletframe.cpp | 5 ++--- src/qt/walletview.cpp | 24 +++++++++++------------- 13 files changed, 73 insertions(+), 58 deletions(-) diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index c0f3ebf3433b4..4a80eb0a33293 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -185,14 +185,14 @@ void AddressBookPage::onEditAction() if(indexes.isEmpty()) return; - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( tab == SendingTab ? EditAddressDialog::EditSendingAddress : EditAddressDialog::EditReceivingAddress, this); - dlg.setModel(model); + dlg->setModel(model); QModelIndex origIndex = proxyModel->mapToSource(indexes.at(0)); - dlg.loadRow(origIndex.row()); - dlg.exec(); + dlg->loadRow(origIndex.row()); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void AddressBookPage::on_newAddress_clicked() diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index bf1a76b3d9eda..34bdabddd9cd7 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -58,6 +58,7 @@ #include #include #include +#include #if defined(QT_STATIC) #include @@ -256,6 +257,8 @@ void BitcoinApplication::createOptionsModel(bool resetSettings) void BitcoinApplication::createWindow(const NetworkStyle *networkStyle) { window = new BitcoinGUI(node(), networkStyle, nullptr); + connect(window, &BitcoinGUI::quitRequested, this, &BitcoinApplication::requestShutdown); + pollShutdownTimer = new QTimer(window); connect(pollShutdownTimer, &QTimer::timeout, window, &BitcoinGUI::detectShutdown); } @@ -290,7 +293,7 @@ void BitcoinApplication::startThread() /* communication to and from thread */ connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult); - connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &BitcoinApplication::shutdownResult); + connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &QCoreApplication::quit); connect(&m_executor.value(), &InitExecutor::runawayException, this, &BitcoinApplication::handleRunawayException); connect(this, &BitcoinApplication::requestedInitialize, &m_executor.value(), &InitExecutor::initialize); connect(this, &BitcoinApplication::requestedShutdown, &m_executor.value(), &InitExecutor::shutdown); @@ -321,13 +324,17 @@ void BitcoinApplication::requestInitialize() void BitcoinApplication::requestShutdown() { + for (const auto w : QGuiApplication::topLevelWindows()) { + w->hide(); + } + // Show a simple window indicating shutdown status // Do this first as some of the steps may take some time below, // for example the RPC console may still be executing a command. shutdownWindow.reset(ShutdownWindow::showShutdownWindow(window)); qDebug() << __func__ << ": Requesting shutdown"; - window->hide(); + // Must disconnect node signals otherwise current thread can deadlock since // no event loop is running. window->unsubscribeFromCoreSignals(); @@ -407,15 +414,10 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead pollShutdownTimer->start(SHUTDOWN_POLLING_DELAY); } else { Q_EMIT splashFinished(); // Make sure splash screen doesn't stick around during shutdown - quit(); // Exit first main loop invocation + requestShutdown(); } } -void BitcoinApplication::shutdownResult() -{ - quit(); // Exit second main loop invocation after shutdown finished -} - void BitcoinApplication::handleRunawayException(const QString &message) { QMessageBox::critical( @@ -745,8 +747,6 @@ int GuiMain(int argc, char* argv[]) #if defined(Q_OS_WIN) WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely…").arg(PACKAGE_NAME), (HWND)app.getMainWinId()); #endif - app.exec(); - app.requestShutdown(); app.exec(); rv = app.getReturnValue(); } else { diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index df6e4f0687e0d..bf862c63e5a09 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -60,8 +60,6 @@ class BitcoinApplication: public QApplication /// Request core initialization void requestInitialize(); - /// Request core shutdown - void requestShutdown(); /// Get process return value int getReturnValue() const { return returnValue; } @@ -73,7 +71,8 @@ class BitcoinApplication: public QApplication public Q_SLOTS: void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info); - void shutdownResult(); + /// Request core shutdown + void requestShutdown(); /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 57aad51ab279c..702ead929d7ff 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -475,7 +475,7 @@ void BitcoinGUI::createActions() m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab")); m_mask_values_action->setCheckable(true); - connect(quitAction, &QAction::triggered, qApp, QApplication::quit); + connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested); connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked); connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt); connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked); @@ -1097,8 +1097,8 @@ void BitcoinGUI::aboutClicked() if(!clientModel) return; - HelpMessageDialog dlg(this, HelpMessageDialog::about); - dlg.exec(); + auto dlg = new HelpMessageDialog(this, HelpMessageDialog::about); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void BitcoinGUI::showDebugWindow() @@ -1353,13 +1353,14 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab) if (!clientModel || !clientModel->getOptionsModel()) return; - OptionsDialog dlg(this, enableWallet); - dlg.setCurrentTab(tab); - dlg.setModel(clientModel->getOptionsModel()); - connect(&dlg, &OptionsDialog::appearanceChanged, [=]() { + auto dlg = new OptionsDialog(this, enableWallet); + connect(dlg, &OptionsDialog::quitOnReset, this, &BitcoinGUI::quitRequested); + dlg->setCurrentTab(tab); + dlg->setModel(clientModel->getOptionsModel()); + connect(dlg, &OptionsDialog::appearanceChanged, [=]() { updateWidth(); }); - dlg.exec(); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); updateCoinJoinVisibility(); } @@ -1703,7 +1704,7 @@ void BitcoinGUI::closeEvent(QCloseEvent *event) // close rpcConsole in case it was open to make some space for the shutdown window rpcConsole->close(); - QApplication::quit(); + Q_EMIT quitRequested(); } else { @@ -1997,7 +1998,7 @@ void BitcoinGUI::detectShutdown() { if(rpcConsole) rpcConsole->hide(); - qApp->quit(); + Q_EMIT quitRequested(); } } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 0e8381ef23d57..ab2e838f9566a 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -254,6 +254,7 @@ class BitcoinGUI : public QMainWindow void openOptionsDialogWithTab(OptionsDialog::Tab tab); Q_SIGNALS: + void quitRequested(); /** Signal raised when a URI was entered or dragged to the GUI */ void receivedURI(const QString &uri); /** Signal raised when RPC console shown */ diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 33037b997c388..ae98ce5de2621 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -1949,4 +1950,11 @@ void PrintSlotException( PrintExceptionContinue(std::make_exception_ptr(exception), description.c_str()); } +void ShowModalDialogAndDeleteOnClose(QDialog* dialog) +{ + dialog->setAttribute(Qt::WA_DeleteOnClose); + dialog->setWindowModality(Qt::ApplicationModal); + dialog->show(); +} + } // namespace GUIUtil diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 26dbda32f145b..54d62622599b2 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -43,6 +43,7 @@ class QAbstractItemView; class QAction; class QButtonGroup; class QDateTime; +class QDialog; class QFont; class QKeySequence; class QLineEdit; @@ -609,6 +610,11 @@ namespace GUIUtil type); } + /** + * Shows a QDialog instance asynchronously, and deletes it on close. + */ + void ShowModalDialogAndDeleteOnClose(QDialog* dialog); + } // namespace GUIUtil #endif // BITCOIN_QT_GUIUTIL_H diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 74cd9c97ed56a..903daba278af4 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -406,7 +406,8 @@ void OptionsDialog::on_resetButton_clicked() /* reset all options and close GUI */ model->Reset(); model->resetSettingsOnShutdown = true; - QApplication::quit(); + close(); + Q_EMIT quitOnReset(); } } diff --git a/src/qt/optionsdialog.h b/src/qt/optionsdialog.h index 17c4f7b6cb2d0..a7d0032370f55 100644 --- a/src/qt/optionsdialog.h +++ b/src/qt/optionsdialog.h @@ -82,6 +82,7 @@ private Q_SLOTS: Q_SIGNALS: void appearanceChanged(); void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort); + void quitOnReset(); private: Ui::OptionsDialog *ui; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 742a43d98e18e..0b1f7f5286712 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -476,9 +476,10 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked) const QString confirmation = model->wallet().privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins"); const QString confirmButtonText = model->wallet().privateKeysDisabled() ? tr("Create Unsigned") : tr("Send"); - SendConfirmationDialog confirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); - confirmationDialog.exec(); - QMessageBox::StandardButton retval = static_cast(confirmationDialog.result()); + auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this); + confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); + // TODO: Replace QDialog::exec() with safer QDialog::show(). + const auto retval = static_cast(confirmationDialog->exec()); if(retval != QMessageBox::Yes) { @@ -937,9 +938,9 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) // Coin Control: button inputs -> show actual coin control dialog void SendCoinsDialog::coinControlButtonClicked() { - CoinControlDialog dlg(*m_coin_control, model, this); - dlg.exec(); - coinControlUpdateLabels(); + auto dlg = new CoinControlDialog(*m_coin_control, model); + connect(dlg, &QDialog::finished, this, &SendCoinsDialog::coinControlUpdateLabels); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } // Coin Control: checkbox custom change address diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 1ad13a300a0a0..2d54b8a683b57 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -510,22 +510,22 @@ void TransactionView::editLabel() // Determine type of address, launch appropriate editor dialog type QString type = modelIdx.data(AddressTableModel::TypeRole).toString(); - EditAddressDialog dlg( + auto dlg = new EditAddressDialog( type == AddressTableModel::Receive ? EditAddressDialog::EditReceivingAddress : EditAddressDialog::EditSendingAddress, this); - dlg.setModel(addressBook); - dlg.loadRow(idx); - dlg.exec(); + dlg->setModel(addressBook); + dlg->loadRow(idx); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } else { // Add sending address - EditAddressDialog dlg(EditAddressDialog::NewSendingAddress, + auto dlg = new EditAddressDialog(EditAddressDialog::NewSendingAddress, this); - dlg.setModel(addressBook); - dlg.setAddress(address); - dlg.exec(); + dlg->setModel(addressBook); + dlg->setAddress(address); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } } diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index b03163a22c979..cf92e2100b748 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -286,10 +286,9 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) return; } - PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); + auto dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); dlg->openWithPSBT(psbtx); - dlg->setAttribute(Qt::WA_DeleteOnClose); - dlg->exec(); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletFrame::encryptWallet() diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 899f0bc39c5af..8e9f9d1fbd39b 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -296,11 +296,10 @@ void WalletView::showOutOfSyncWarning(bool fShow) void WalletView::encryptWallet() { - AskPassphraseDialog dlg(AskPassphraseDialog::Encrypt, this); - dlg.setModel(walletModel); - dlg.exec(); - - Q_EMIT encryptionStatusChanged(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Encrypt, this); + dlg->setModel(walletModel); + connect(dlg, &QDialog::finished, this, &WalletView::encryptionStatusChanged); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::backupWallet() @@ -325,19 +324,18 @@ void WalletView::backupWallet() void WalletView::changePassphrase() { - AskPassphraseDialog dlg(AskPassphraseDialog::ChangePass, this); - dlg.setModel(walletModel); - dlg.exec(); + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::ChangePass, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } void WalletView::unlockWallet(bool fForMixingOnly) { // Unlock wallet when requested by wallet model - if (walletModel->getEncryptionStatus() == WalletModel::Locked || walletModel->getEncryptionStatus() == WalletModel::UnlockedForMixingOnly) - { - AskPassphraseDialog dlg(fForMixingOnly ? AskPassphraseDialog::UnlockMixing : AskPassphraseDialog::Unlock, this); - dlg.setModel(walletModel); - dlg.exec(); + if (walletModel->getEncryptionStatus() == WalletModel::Locked || walletModel->getEncryptionStatus() == WalletModel::UnlockedForMixingOnly) { + auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Unlock, this); + dlg->setModel(walletModel); + GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); } } From 2cc9aa1ccf8a7ee21e2e2a582bba77503f51ef91 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 30 Sep 2021 17:27:33 +0300 Subject: [PATCH 08/11] Merge bitcoin-core/gui#342: wallet: Move wallets loading out from the main GUI thread 2fe69efbc607fdcc3657637d59a38cc5b4db2d05 qt, wallet: Drop no longer used WalletController::getOpenWallets() (Hennadii Stepanov) f6991cb906e9dad7ff76a51e2b654f798d5c2ba6 qt, wallet: Add LoadWalletsActivity class (Hennadii Stepanov) 4a024fc310f136ce62c733fb1174b3a80ea25d0a qt, wallet, refactor: Move connection to QObject::deleteLater to ctor (Hennadii Stepanov) f9b633eeab6e9ee405bba37573aed9aa83c51ea5 qt, wallet: Move activity progress dialog from data member to local (Hennadii Stepanov) Pull request description: This PR improves the GUI responsiveness during initial wallets loading at startup (especially ones that have tons of txs), and shows a standard progress dialog for long loading: ![DeepinScreenshot_select-area_20210522230626](https://user-images.githubusercontent.com/32963518/119239625-0b3a9380-bb53-11eb-9a54-34980d8a1194.png) Fixes #247. ACKs for top commit: ryanofsky: Code review ACK 2fe69efbc607fdcc3657637d59a38cc5b4db2d05. Just suggested changes since last review: squashing commits and dropping unused method (thanks!) shaavan: reACK 2fe69efbc607fdcc3657637d59a38cc5b4db2d05 promag: Code review ACK 2fe69efbc607fdcc3657637d59a38cc5b4db2d05. Tree-SHA512: 2ac3cb48886e0005fc36b3fd0c2b35abd557186be16db3145d753c34d94188e4f4ff14dc07fb0fb7558944f84498204a3988f8284fd56c6d85b47bc9081e71a6 --- src/qt/bitcoingui.cpp | 8 ++--- src/qt/walletcontroller.cpp | 66 +++++++++++++++++-------------------- src/qt/walletcontroller.h | 17 ++++++---- 3 files changed, 43 insertions(+), 48 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 702ead929d7ff..2628bdf3afad8 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -114,7 +114,6 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const NetworkStyle* networkStyle, walletFrame = new WalletFrame(this); connect(walletFrame, &WalletFrame::createWalletButtonClicked, [this] { auto activity = new CreateWalletActivity(getWalletController(), this); - connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); connect(walletFrame, &WalletFrame::message, [this](const QString& title, const QString& message, unsigned int style) { @@ -536,7 +535,6 @@ void BitcoinGUI::createActions() connect(action, &QAction::triggered, [this, path] { auto activity = new OpenWalletActivity(m_wallet_controller, this); connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet); - connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater); activity->open(path); }); } @@ -551,7 +549,6 @@ void BitcoinGUI::createActions() connect(m_create_wallet_action, &QAction::triggered, [this] { auto activity = new CreateWalletActivity(m_wallet_controller, this); connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet); - connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); connect(m_close_all_wallets_action, &QAction::triggered, [this] { @@ -903,9 +900,8 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); - for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) { - addWallet(wallet_model); - } + auto activity = new LoadWalletsActivity(m_wallet_controller, this); + activity->load(); } WalletController* BitcoinGUI::getWalletController() diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index b39f630ccb256..fd4ceece97af3 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -42,10 +42,6 @@ WalletController::WalletController(ClientModel& client_model, QObject* parent) getOrCreateWallet(std::move(wallet)); }); - for (std::unique_ptr& wallet : m_node.walletLoader().getWallets()) { - getOrCreateWallet(std::move(wallet)); - } - m_activity_worker->moveToThread(m_activity_thread); m_activity_thread->start(); QTimer::singleShot(0, m_activity_worker, []() { @@ -62,12 +58,6 @@ WalletController::~WalletController() delete m_activity_worker; } -std::vector WalletController::getOpenWallets() const -{ - QMutexLocker locker(&m_mutex); - return m_wallets; -} - std::map WalletController::listWalletDir() const { QMutexLocker locker(&m_mutex); @@ -192,34 +182,24 @@ WalletControllerActivity::WalletControllerActivity(WalletController* wallet_cont , m_wallet_controller(wallet_controller) , m_parent_widget(parent_widget) { -} - -WalletControllerActivity::~WalletControllerActivity() -{ - delete m_progress_dialog; + connect(this, &WalletControllerActivity::finished, this, &QObject::deleteLater); } void WalletControllerActivity::showProgressDialog(const QString& title_text, const QString& label_text) { - assert(!m_progress_dialog); - m_progress_dialog = new QProgressDialog(m_parent_widget); - - m_progress_dialog->setWindowTitle(title_text); - m_progress_dialog->setLabelText(label_text); - m_progress_dialog->setRange(0, 0); - m_progress_dialog->setCancelButton(nullptr); - m_progress_dialog->setWindowModality(Qt::ApplicationModal); - GUIUtil::PolishProgressDialog(m_progress_dialog); + auto progress_dialog = new QProgressDialog(m_parent_widget); + progress_dialog->setAttribute(Qt::WA_DeleteOnClose); + connect(this, &WalletControllerActivity::finished, progress_dialog, &QWidget::close); + + progress_dialog->setWindowTitle(title_text); + progress_dialog->setLabelText(label_text); + progress_dialog->setRange(0, 0); + progress_dialog->setCancelButton(nullptr); + progress_dialog->setWindowModality(Qt::ApplicationModal); + GUIUtil::PolishProgressDialog(progress_dialog); // The setValue call forces QProgressDialog to start the internal duration estimation. // See details in https://bugreports.qt.io/browse/QTBUG-47042. - m_progress_dialog->setValue(0); -} - -void WalletControllerActivity::destroyProgressDialog() -{ - assert(m_progress_dialog); - delete m_progress_dialog; - m_progress_dialog = nullptr; + progress_dialog->setValue(0); } CreateWalletActivity::CreateWalletActivity(WalletController* wallet_controller, QWidget* parent_widget) @@ -283,8 +263,6 @@ void CreateWalletActivity::createWallet() void CreateWalletActivity::finish() { - destroyProgressDialog(); - if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message.translated)); } else if (!m_warning_message.empty()) { @@ -324,8 +302,6 @@ OpenWalletActivity::OpenWalletActivity(WalletController* wallet_controller, QWid void OpenWalletActivity::finish() { - destroyProgressDialog(); - if (!m_error_message.empty()) { QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message.translated)); } else if (!m_warning_message.empty()) { @@ -356,3 +332,21 @@ void OpenWalletActivity::open(const std::string& path) QTimer::singleShot(0, this, &OpenWalletActivity::finish); }); } + +LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget) + : WalletControllerActivity(wallet_controller, parent_widget) +{ +} + +void LoadWalletsActivity::load() +{ + showProgressDialog(tr("Loading wallets…"), tr("Loading wallets…")); + + QTimer::singleShot(0, worker(), [this] { + for (auto& wallet : node().walletLoader().getWallets()) { + m_wallet_controller->getOrCreateWallet(std::move(wallet)); + } + + QTimer::singleShot(0, this, [this] { Q_EMIT finished(); }); + }); +} diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index f65455e7c83f5..f6b8a6576e12e 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -52,9 +52,6 @@ class WalletController : public QObject WalletController(ClientModel& client_model, QObject* parent); ~WalletController(); - //! Returns wallet models currently open. - std::vector getOpenWallets() const; - WalletModel* getOrCreateWallet(std::unique_ptr wallet); //! Returns all wallet names in the wallet dir mapped to whether the wallet @@ -89,7 +86,7 @@ class WalletControllerActivity : public QObject public: WalletControllerActivity(WalletController* wallet_controller, QWidget* parent_widget); - virtual ~WalletControllerActivity(); + virtual ~WalletControllerActivity() = default; Q_SIGNALS: void finished(); @@ -99,11 +96,9 @@ class WalletControllerActivity : public QObject QObject* worker() const { return m_wallet_controller->m_activity_worker; } void showProgressDialog(const QString& title_text, const QString& label_text); - void destroyProgressDialog(); WalletController* const m_wallet_controller; QWidget* const m_parent_widget; - QProgressDialog* m_progress_dialog{nullptr}; WalletModel* m_wallet_model{nullptr}; bilingual_str m_error_message; std::vector m_warning_message; @@ -149,4 +144,14 @@ class OpenWalletActivity : public WalletControllerActivity void finish(); }; +class LoadWalletsActivity : public WalletControllerActivity +{ + Q_OBJECT + +public: + LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget); + + void load(); +}; + #endif // BITCOIN_QT_WALLETCONTROLLER_H From 8ef72e0a8ea5675e21c6db844e5a828c9c4af799 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Fri, 1 Oct 2021 10:24:11 +0200 Subject: [PATCH 09/11] Merge bitcoin/bitcoin#22585: fuzz: add guide to fuzzing with Eclipser v1.x 6e1150ea3b82d1ab557d4b74aa652b8d974876aa fuzz: add guide to fuzzing with Eclipser v1.x (Alex Groce) Pull request description: MarcoFalke and practicalswift here's an Eclipser guide, reconstructed from their documentation and my docker history getting it up and running. It might be good if someone confirmed it actually works for them in a fresh ubuntu 20.04. ACKs for top commit: practicalswift: ACK 6e1150ea3b82d1ab557d4b74aa652b8d974876aa Tree-SHA512: ca855932fd7a2c1d1005d572ab5fabc26f42d779f9baf279783f08a43dd72ec60f57239135d30c2a82781e593626fec2c96bb19fb91e1b777cef2d83a54eba35 --- doc/fuzzing.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/doc/fuzzing.md b/doc/fuzzing.md index 3b1520fc96f13..82f3ac8b31657 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -268,6 +268,73 @@ $ honggfuzz/honggfuzz --exit_upon_crash --quiet --timeout 4 -n 1 -Q \ -debug ``` +# Fuzzing Bitcoin Core using Eclipser (v1.x) + +## Quickstart guide + +To quickly get started fuzzing Bitcoin Core using [Eclipser v1.x](https://github.com/SoftSec-KAIST/Eclipser/tree/v1.x): + +```sh +$ git clone https://github.com/bitcoin/bitcoin +$ cd bitcoin/ +$ sudo vim /etc/apt/sources.list # Uncomment the lines starting with 'deb-src'. +$ sudo apt-get update +$ sudo apt-get build-dep qemu +$ sudo apt-get install libtool libtool-bin wget automake autoconf bison gdb +``` + +At this point, you must install the .NET core. The process differs, depending on your Linux distribution. +See [this link](https://docs.microsoft.com/en-us/dotnet/core/install/linux) for details. +On ubuntu 20.04, the following should work: + +```sh +$ wget -q https://packages.microsoft.com/config/ubuntu/20.04/packages-microsoft-prod.deb +$ sudo dpkg -i packages-microsoft-prod.deb +$ rm packages-microsoft-prod.deb +$ sudo apt-get update +$ sudo apt-get install -y dotnet-sdk-2.1 +``` + +You will also want to make sure Python is installed as `python` for the Eclipser install to succeed. + +```sh +$ git clone https://github.com/SoftSec-KAIST/Eclipser.git +$ cd Eclipser +$ git checkout v1.x +$ make +$ cd .. +$ ./autogen.sh +$ ./configure --enable-fuzz +$ make +$ mkdir -p outputs/ +$ FUZZ=bech32 dotnet Eclipser/build/Eclipser.dll fuzz -p src/test/fuzz/fuzz -t 36000 -o outputs --src stdin +``` + +This will perform 10 hours of fuzzing. + +To make further use of the inputs generated by Eclipser, you +must first decode them: + +```sh +$ dotnet Eclipser/build/Eclipser.dll decode -i outputs/testcase -o decoded_outputs +``` +This will place raw inputs in the directory `decoded_outputs/decoded_stdins`. Crashes are in the `outputs/crashes` directory, and must +be decoded in the same way. + +Fuzzing with Eclipser will likely be much more effective if using an existing corpus: + +```sh +$ git clone https://github.com/bitcoin-core/qa-assets +$ FUZZ=bech32 dotnet Eclipser/build/Eclipser.dll fuzz -p src/test/fuzz/fuzz -t 36000 -i qa-assets/fuzz_seed_corpus/bech32 outputs --src stdin +``` + +Note that fuzzing with Eclipser on certain targets (those that create 'full nodes', e.g. `process_message*`) will, +for now, slowly fill `/tmp/` with improperly cleaned-up files, which will cause spurious crashes. +See [this proposed patch](https://github.com/bitcoin/bitcoin/pull/22472) for more information. + +Read the [Eclipser documentation for v1.x](https://github.com/SoftSec-KAIST/Eclipser/tree/v1.x) for more details on using Eclipser. + + # OSS-Fuzz Bitcoin Core participates in Google's [OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/bitcoin-core) From 7cae481c690aedfd9957b1f560d565902c6620ad Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 23 Sep 2021 15:49:34 +0800 Subject: [PATCH 10/11] partial Merge bitcoin/bitcoin#22798: doc: Fix RPC result documentation BACKPORT NOTICE: missing: - field `signet_challenge` in rpc/mining.cpp - changes in `rpc/external_signer.cpp` fa10fbc6658fb8d87118cd550e4de406dca45f23 doc: Fix RPC result documentation (MarcoFalke) Pull request description: Fix: * Incorrectly named fields * Add missing ones * Add missing optional flag ACKs for top commit: fanquake: ACK fa10fbc6658fb8d87118cd550e4de406dca45f23 Tree-SHA512: 2b302e6f7ac8253a55882bc032ddda1932b728abddd12b0adb5fba814b12fb998a67b91e6dd124ebbe0ac16dccdace01332ade01c1dc00a3768647118dd3a2d2 --- src/rpc/blockchain.cpp | 76 +++++++++++++++++++------------------- src/rpc/mining.cpp | 4 ++ src/rpc/misc.cpp | 8 ++-- src/rpc/net.cpp | 12 +++--- src/rpc/rawtransaction.cpp | 40 ++++++++++---------- src/wallet/rpcwallet.cpp | 72 ++++++++++++++++++------------------ 6 files changed, 109 insertions(+), 103 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 1bf709aa7e4aa..0a10ea80100e4 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1064,11 +1064,11 @@ static RPCHelpMan gettxoutsetinfo() {RPCResult::Type::NUM, "bogosize", "Database-independent, meaningless metric indicating the UTXO set size"}, {RPCResult::Type::STR_HEX, "hash_serialized_2", /* optional */ true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, {RPCResult::Type::STR_HEX, "muhash", /* optional */ true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, - {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, - {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, + {RPCResult::Type::NUM, "transactions", /* optional */ true, "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, + {RPCResult::Type::NUM, "disk_size", /* optional */ true, "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of coins in the UTXO set"}, - {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, - {RPCResult::Type::OBJ, "block_info", "Info on amounts in the block at this block height (only available if coinstatsindex is used)", + {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", /* optional */ true, "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, + {RPCResult::Type::OBJ, "block_info", /* optional */ true, "Info on amounts in the block at this block height (only available if coinstatsindex is used)", { {RPCResult::Type::STR_AMOUNT, "prevout_spent", "Total amount of all prevouts spent in this block"}, {RPCResult::Type::STR_AMOUNT, "coinbase", "Coinbase subsidy amount of this block"}, @@ -1400,16 +1400,16 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in active chain, in hexadecimal"}, {RPCResult::Type::NUM, "size_on_disk", "the estimated size of the block and undo files on disk"}, {RPCResult::Type::BOOL, "pruned", "if the blocks are subject to pruning"}, - {RPCResult::Type::NUM, "pruneheight", "lowest-height complete block stored (only present if pruning is enabled)"}, - {RPCResult::Type::BOOL, "automatic_pruning", "whether automatic pruning is enabled (only present if pruning is enabled)"}, - {RPCResult::Type::NUM, "prune_target_size", "the target size used by pruning (only present if automatic pruning is enabled)"}, + {RPCResult::Type::NUM, "pruneheight", /* optional */ true, "lowest-height complete block stored (only present if pruning is enabled)"}, + {RPCResult::Type::BOOL, "automatic_pruning", /* optional */ true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, + {RPCResult::Type::NUM, "prune_target_size", /* optional */ true, "the target size used by pruning (only present if automatic pruning is enabled)"}, {RPCResult::Type::OBJ, "softforks", "status of softforks in progress", { {RPCResult::Type::STR, "type", "one of \"buried\", \"bip9\""}, - {RPCResult::Type::OBJ, "bip9", "status of bip9 softforks (only for \"bip9\" type)", + {RPCResult::Type::OBJ, "bip9", /* optional */ true, "status of bip9 softforks (only for \"bip9\" type)", { {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""}, - {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, + {RPCResult::Type::NUM, "bit", /* optional */ true, "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"}, {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"}, {RPCResult::Type::BOOL, "ehf", "returns true for EHF activated forks"}, @@ -1417,16 +1417,16 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "activation_height", "expected activation height for this softfork (only for \"locked_in\" status)"}, {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"}, - {RPCResult::Type::OBJ, "statistics", "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", + {RPCResult::Type::OBJ, "statistics", /* optional */ true, "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", { {RPCResult::Type::NUM, "period", "the length in blocks of the signalling period"}, - {RPCResult::Type::NUM, "threshold", "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, + {RPCResult::Type::NUM, "threshold", /* optional */ true, "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, {RPCResult::Type::NUM, "elapsed", "the number of blocks elapsed since the beginning of the current period"}, {RPCResult::Type::NUM, "count", "the number of blocks with the version bit set in the current period"}, - {RPCResult::Type::BOOL, "possible", "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, + {RPCResult::Type::BOOL, "possible", /* optional */ true, "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, }}, }}, - {RPCResult::Type::NUM, "height", "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, + {RPCResult::Type::NUM, "height", /* optional */ true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, {RPCResult::Type::BOOL, "active", "true if the rules are enforced for the mempool and the next block"}, }}, {RPCResult::Type::STR, "warnings", "any network and blockchain warnings"}, @@ -1937,11 +1937,11 @@ static RPCHelpMan getblockstats() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "avgfee", "Average fee in the block"}, - {RPCResult::Type::NUM, "avgfeerate", "Average feerate (in duffs per virtual byte)"}, - {RPCResult::Type::NUM, "avgtxsize", "Average transaction size"}, - {RPCResult::Type::STR_HEX, "blockhash", "The block hash (to check for potential reorgs)"}, - {RPCResult::Type::ARR_FIXED, "feerate_percentiles", "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in duffs per byte)", + {RPCResult::Type::NUM, "avgfee", /* optional */ true, "Average fee in the block"}, + {RPCResult::Type::NUM, "avgfeerate", /* optional */ true, "Average feerate (in duffs per byte)"}, + {RPCResult::Type::NUM, "avgtxsize", /* optional */ true, "Average transaction size"}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash (to check for potential reorgs)"}, + {RPCResult::Type::ARR_FIXED, "feerate_percentiles", /* optional */ true, "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile size unit (in duffs per byte)", { {RPCResult::Type::NUM, "10th_percentile_feerate", "The 10th percentile feerate"}, {RPCResult::Type::NUM, "25th_percentile_feerate", "The 25th percentile feerate"}, @@ -1949,26 +1949,26 @@ static RPCHelpMan getblockstats() {RPCResult::Type::NUM, "75th_percentile_feerate", "The 75th percentile feerate"}, {RPCResult::Type::NUM, "90th_percentile_feerate", "The 90th percentile feerate"}, }}, - {RPCResult::Type::NUM, "height", "The height of the block"}, - {RPCResult::Type::NUM, "ins", "The number of inputs (excluding coinbase)"}, - {RPCResult::Type::NUM, "maxfee", "Maximum fee in the block"}, - {RPCResult::Type::NUM, "maxfeerate", "Maximum feerate (in duffs per virtual byte)"}, - {RPCResult::Type::NUM, "maxtxsize", "Maximum transaction size"}, - {RPCResult::Type::NUM, "medianfee", "Truncated median fee in the block"}, - {RPCResult::Type::NUM, "mediantime", "The block median time past"}, - {RPCResult::Type::NUM, "mediantxsize", "Truncated median transaction size"}, - {RPCResult::Type::NUM, "minfee", "Minimum fee in the block"}, - {RPCResult::Type::NUM, "minfeerate", "Minimum feerate (in duffs per virtual byte)"}, - {RPCResult::Type::NUM, "mintxsize", "Minimum transaction size"}, - {RPCResult::Type::NUM, "outs", "The number of outputs"}, - {RPCResult::Type::NUM, "subsidy", "The block subsidy"}, - {RPCResult::Type::NUM, "time", "The block time"}, - {RPCResult::Type::NUM, "total_out", "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, - {RPCResult::Type::NUM, "total_size", "Total size of all non-coinbase transactions"}, - {RPCResult::Type::NUM, "totalfee", "The fee total"}, - {RPCResult::Type::NUM, "txs", "The number of transactions (including coinbase)"}, - {RPCResult::Type::NUM, "utxo_increase", "The increase/decrease in the number of unspent outputs"}, - {RPCResult::Type::NUM, "utxo_size_inc", "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, + {RPCResult::Type::NUM, "height", /* optional */ true, "The height of the block"}, + {RPCResult::Type::NUM, "ins", /* optional */ true, "The number of inputs (excluding coinbase)"}, + {RPCResult::Type::NUM, "maxfee", /* optional */ true, "Maximum fee in the block"}, + {RPCResult::Type::NUM, "maxfeerate", /* optional */ true, "Maximum feerate (in duffs per virtual byte)"}, + {RPCResult::Type::NUM, "maxtxsize", /* optional */ true, "Maximum transaction size"}, + {RPCResult::Type::NUM, "medianfee", /* optional */ true, "Truncated median fee in the block"}, + {RPCResult::Type::NUM, "mediantime", /* optional */ true, "The block median time past"}, + {RPCResult::Type::NUM, "mediantxsize", /* optional */ true, "Truncated median transaction size"}, + {RPCResult::Type::NUM, "minfee", /* optional */ true, "Minimum fee in the block"}, + {RPCResult::Type::NUM, "minfeerate", /* optional */ true, "Minimum feerate (in duffs per virtual byte)"}, + {RPCResult::Type::NUM, "mintxsize", /* optional */ true, "Minimum transaction size"}, + {RPCResult::Type::NUM, "outs", /* optional */ true, "The number of outputs"}, + {RPCResult::Type::NUM, "subsidy", /* optional */ true, "The block subsidy"}, + {RPCResult::Type::NUM, "time", /* optional */ true, "The block time"}, + {RPCResult::Type::NUM, "total_out", /* optional */ true, "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, + {RPCResult::Type::NUM, "total_size", /* optional */ true, "Total size of all non-coinbase transactions"}, + {RPCResult::Type::NUM, "totalfee", /* optional */ true, "The fee total"}, + {RPCResult::Type::NUM, "txs", /* optional */ true, "The number of transactions (including coinbase)"}, + {RPCResult::Type::NUM, "utxo_increase", /* optional */ true, "The increase/decrease in the number of unspent outputs"}, + {RPCResult::Type::NUM, "utxo_size_inc", /* optional */ true, "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, }}, RPCExamples{ HelpExampleCli("getblockstats", R"('"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09"' '["minfeerate","avgfeerate"]')") + diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b87e09d0508f8..0f7cad75c754c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -595,6 +595,10 @@ static RPCHelpMan getblocktemplate() { {RPCResult::Type::NUM, "rulename", "identifies the bit number as indicating acceptance and readiness for the named softfork rule"}, }}, + {RPCResult::Type::ARR, "capabilities", "", + { + {RPCResult::Type::STR, "value", "A supported feature, for example 'proposal'"}, + }}, {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 7900e99cd1f9c..347e0339828a6 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -243,9 +243,9 @@ static RPCHelpMan validateaddress() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"}, - {RPCResult::Type::STR, "address", "The Dash address validated"}, - {RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded scriptPubKey generated by the address"}, - {RPCResult::Type::BOOL, "isscript", "If the key is a script"}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address validated"}, + {RPCResult::Type::STR_HEX, "scriptPubKey", /* optional */ true, "The hex-encoded scriptPubKey generated by the address"}, + {RPCResult::Type::BOOL, "isscript", /* optional */ true, "If the key is a script"}, {RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"}, } }, @@ -1328,7 +1328,7 @@ static RPCHelpMan getindexinfo() {"index_name", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Filter results for an index with a specific name."}, }, RPCResult{ - RPCResult::Type::OBJ, "", "", { + RPCResult::Type::OBJ_DYN, "", "", { { RPCResult::Type::OBJ, "name", "The name of the index", { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 21215aef838eb..2d17257574c4f 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -106,10 +106,10 @@ static RPCHelpMan getpeerinfo() { {RPCResult::Type::NUM, "id", "Peer index"}, {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, - {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"}, - {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"}, + {RPCResult::Type::STR, "addrbind", /* optional */ true, "(ip:port) Bind address of the connection to the peer"}, + {RPCResult::Type::STR, "addrlocal", /* optional */ true, "(ip:port) Local address as reported by the peer"}, {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, - {RPCResult::Type::STR, "mapped_as", "The AS in the BGP route to the peer used for diversifying peer selection"}, + {RPCResult::Type::STR, "mapped_as", /* optional */ true, "The AS in the BGP route to the peer used for diversifying peer selection"}, {RPCResult::Type::STR_HEX, "services", "The services offered"}, {RPCResult::Type::ARR, "servicesnames", "the services offered, in human-readable form", { @@ -130,9 +130,9 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "bytesrecv", "The total bytes received"}, {RPCResult::Type::NUM_TIME, "conntime", "The " + UNIX_EPOCH_TIME + " of the connection"}, {RPCResult::Type::NUM, "timeoffset", "The time offset in seconds"}, - {RPCResult::Type::NUM, "pingtime", "ping time (if available)"}, - {RPCResult::Type::NUM, "minping", "minimum observed ping time (if any at all)"}, - {RPCResult::Type::NUM, "pingwait", "ping wait (if non-zero)"}, + {RPCResult::Type::NUM, "pingtime", /* optional */ true, "ping time (if available)"}, + {RPCResult::Type::NUM, "minping", /* optional */ true, "minimum observed ping time (if any at all)"}, + {RPCResult::Type::NUM, "pingwait", /* optional */ true, "ping wait (if non-zero)"}, {RPCResult::Type::NUM, "version", "The peer version, such as 70001"}, {RPCResult::Type::STR, "subver", "The string version"}, {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index c67a335e05bc1..0b598ae9cc2f6 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -148,7 +148,7 @@ static RPCHelpMan getrawtransaction() RPCResult{"if verbose is set to true", RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "in_active_chain", "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, + {RPCResult::Type::BOOL, "in_active_chain", /* optional */ true, "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, {RPCResult::Type::STR_HEX, "txid", "The transaction id (same as provided)"}, {RPCResult::Type::NUM, "size", "The serialized transaction size"}, {RPCResult::Type::NUM, "version", "The version"}, @@ -191,11 +191,11 @@ static RPCHelpMan getrawtransaction() {RPCResult::Type::NUM, "extraPayloadSize", true /*optional*/, "Size of DIP2 extra payload. Only present if it's a special TX"}, {RPCResult::Type::STR_HEX, "extraPayload", true /*optional*/, "Hex-encoded DIP2 extra payload data. Only present if it's a special TX"}, {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, - {RPCResult::Type::STR_HEX, "blockhash", "the block hash"}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "the block hash"}, {RPCResult::Type::NUM, "height", "The block height"}, - {RPCResult::Type::NUM, "confirmations", "The confirmations"}, - {RPCResult::Type::NUM_TIME, "blocktime", "The block time expressed in " + UNIX_EPOCH_TIME}, - {RPCResult::Type::NUM, "time", "Same as \"blocktime\""}, + {RPCResult::Type::NUM, "confirmations", /* optional */ true, "The confirmations"}, + {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME}, + {RPCResult::Type::NUM, "time", /* optional */ true, "Same as \"blocktime\""}, {RPCResult::Type::BOOL, "instantlock", "Current transaction lock state"}, {RPCResult::Type::BOOL, "instantlock_internal", "Current internal transaction lock state"}, {RPCResult::Type::BOOL, "chainlock", "The state of the corresponding block ChainLock"}, @@ -742,9 +742,10 @@ static RPCHelpMan decoderawtransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, - {RPCResult::Type::NUM, "vout", "The output number"}, - {RPCResult::Type::OBJ, "scriptSig", "The script", + {RPCResult::Type::STR_HEX, "coinbase", /* optional */ true, ""}, + {RPCResult::Type::STR_HEX, "txid", /* optional */ true, "The transaction id"}, + {RPCResult::Type::NUM, "vout", /* optional */ true, "The output number"}, + {RPCResult::Type::OBJ, "scriptSig", /* optional */ true, "The script", { {RPCResult::Type::STR, "asm", "asm"}, {RPCResult::Type::STR_HEX, "hex", "hex"}, @@ -1125,15 +1126,15 @@ static RPCHelpMan testmempoolaccept() {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, - {RPCResult::Type::STR, "package-error", "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, - {RPCResult::Type::BOOL, "allowed", "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate." + {RPCResult::Type::STR, "package-error", /* optional */ true, "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, + {RPCResult::Type::BOOL, "allowed", /* optional */ true, "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate. " "If not present, the tx was not fully validated due to a failure in another tx in the list."}, - {RPCResult::Type::NUM, "vsize", "Virtual transaction size."}, - {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)", + {RPCResult::Type::NUM, "vsize", /* optional */ true, "Transaction size."}, + {RPCResult::Type::OBJ, "fees", /* optional */ true, "Transaction fees (only present if 'allowed' is true)", { {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, }}, - {RPCResult::Type::STR, "reject-reason", "Rejection string (only present when 'allowed' is false)"}, + {RPCResult::Type::STR, "reject-reason", /* optional */ true, "Rejection string (only present when 'allowed' is false)"}, }}, } }, @@ -1300,13 +1301,14 @@ static RPCHelpMan decodepsbt() }}, {RPCResult::Type::ARR, "bip32_derivs", /* optional */ true, "", { - {RPCResult::Type::OBJ, "pubkey", /* optional */ true, "The public key with the derivation path as the value.", + {RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "pubkey", "The public key with the derivation path as the value."}, {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ, "final_scriptsig", /* optional */ true, "", + {RPCResult::Type::OBJ, "final_scriptSig", /* optional */ true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "hex", "The hex"}, @@ -1362,7 +1364,7 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ_DYN, "unknown", "The unknown global fields", + {RPCResult::Type::OBJ_DYN, "unknown", /* optional */ true, "The unknown global fields", { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, @@ -1711,8 +1713,8 @@ static RPCHelpMan finalizepsbt() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "psbt", "The base64-encoded partially signed transaction if not extracted"}, - {RPCResult::Type::STR_HEX, "hex", "The hex-encoded network transaction if extracted"}, + {RPCResult::Type::STR, "psbt", /* optional */ true, "The base64-encoded partially signed transaction if not extracted"}, + {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "The hex-encoded network transaction if extracted"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, } }, @@ -2064,7 +2066,7 @@ static RPCHelpMan analyzepsbt() RPCResult { RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "inputs", "", + {RPCResult::Type::ARR, "inputs", /* optional */ true, "", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 172bb82fefd57..6b189970390ad 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -345,7 +345,7 @@ static RPCHelpMan sendtoaddress() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, - {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + {RPCResult::Type::STR, "fee_reason", "The transaction fee reason."} }, }, }, @@ -790,7 +790,7 @@ static RPCHelpMan sendmany() { {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" "the number of addresses."}, - {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + {RPCResult::Type::STR, "fee_reason", "The transaction fee reason."} }, }, }, @@ -1099,7 +1099,7 @@ static RPCHelpMan listreceivedbyaddress() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The receiving address"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received by the address"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included.\n" @@ -1150,7 +1150,7 @@ static RPCHelpMan listreceivedbylabel() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount received by addresses with this label"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, {RPCResult::Type::STR, "label", "The label of the receiving address. The default label is \"\""}, @@ -1287,18 +1287,18 @@ static const std::vector TransactionDescriptionString() {RPCResult::Type::BOOL, "instantlock", "Current transaction lock state. Available for 'send' and 'receive' category of transactions"}, {RPCResult::Type::BOOL, "instantlock-internal", "Current internal transaction lock state. Available for 'send' and 'receive' category of transactions"}, {RPCResult::Type::BOOL, "chainlock", "The state of the corresponding block ChainLock"}, - {RPCResult::Type::BOOL, "trusted", "Whether we consider the outputs of this unconfirmed transaction safe to spend."}, - {RPCResult::Type::STR_HEX, "blockhash", "The block hash containing the transaction. Available for 'send' and 'receive'\n" + {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Whether we consider the outputs of this unconfirmed transaction safe to spend."}, + {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction. Available for 'send' and 'receive'\n" "category of transactions."}, - {RPCResult::Type::STR_HEX, "blockheight", "The block height containing the transaction."}, - {RPCResult::Type::NUM, "blockindex", "The index of the transaction in the block that includes it. Available for 'send' and 'receive'\n" + {RPCResult::Type::STR_HEX, "blockheight", /* optional */ true, "The block height containing the transaction."}, + {RPCResult::Type::NUM, "blockindex", /* optional */ true, "The index of the transaction in the block that includes it. Available for 'send' and 'receive'\n" "category of transactions."}, - {RPCResult::Type::NUM_TIME, "blocktime", "The block time expressed in " + UNIX_EPOCH_TIME + "."}, + {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR_HEX, "txid", "The transaction id. Available for 'send' and 'receive' category of transactions."}, {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + ". Available \n" "for 'send' and 'receive' category of transactions."}, - {RPCResult::Type::STR, "comment", "If a comment is associated with the transaction."}}; + {RPCResult::Type::STR, "comment", /* optional */ "If a comment is associated with the transaction."}}; } static RPCHelpMan listtransactions() @@ -1318,7 +1318,7 @@ static RPCHelpMan listtransactions() { {RPCResult::Type::OBJ, "", "", Cat(Cat>( { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The Dash address of the transaction. Not present for\n" "move transactions (category = move)."}, {RPCResult::Type::STR, "category", "The transaction category.\n" @@ -1331,14 +1331,14 @@ static RPCHelpMan listtransactions() "\"platform-transfer\" Platform Transfer transactions received.\n"}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, })}, } @@ -1435,7 +1435,7 @@ static RPCHelpMan listsinceblock() { {RPCResult::Type::OBJ, "", "", Cat(Cat>( { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The Dash address of the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -1448,16 +1448,16 @@ static RPCHelpMan listsinceblock() {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::NUM, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the 'send' category of transactions."}, + {RPCResult::Type::NUM, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the 'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions."}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any."}, + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions."}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any."}, {RPCResult::Type::STR, "to", "If a comment to is associated with the transaction."}, })}, }}, - {RPCResult::Type::ARR, "removed", "\n" + {RPCResult::Type::ARR, "removed", /* optional */ true, "\n" "Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count." , {{RPCResult::Type::ELISION, "", ""},}}, {RPCResult::Type::STR_HEX, "lastblockhash", "The hash of the block (target_confirmations-1) from the best block on the main chain, or the genesis hash if the referenced block does not exist yet. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones."} @@ -1568,7 +1568,7 @@ static RPCHelpMan gettransaction() RPCResult::Type::OBJ, "", "", Cat(Cat>( { {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), @@ -1577,8 +1577,8 @@ static RPCHelpMan gettransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", "Only returns true if imported addresses were involved in transaction"}, - {RPCResult::Type::STR, "address", "The Dash address involved in the transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address involved in the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" "\"coinjoin\" Transactions sent using CoinJoin funds.\n" @@ -1588,11 +1588,11 @@ static RPCHelpMan gettransaction() "\"orphan\" Orphaned coinbase transactions received.\n" "\"platform-transfer\" Platform Transfer transactions received.\n"}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR, "label", "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" + {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, }}, }}, @@ -2343,10 +2343,10 @@ static RPCHelpMan getbalances() {RPCResult::Type::STR_AMOUNT, "trusted", " trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", " untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", " balance from immature coinbase outputs"}, - {RPCResult::Type::STR_AMOUNT, "used", "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, + {RPCResult::Type::STR_AMOUNT, "used", /* optional */ true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, {RPCResult::Type::STR_AMOUNT, "coinjoin", " CoinJoin balance (outputs with enough rounds created by the wallet via mixing)"}, }}, - {RPCResult::Type::OBJ, "watchonly", "watchonly balances (not present if wallet does not watch anything)", + {RPCResult::Type::OBJ, "watchonly", /* optional */ true, "watchonly balances (not present if wallet does not watch anything)", { {RPCResult::Type::STR_AMOUNT, "trusted", " trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", " untrusted pending balance (outputs created by others that are in the mempool)"}, @@ -2416,9 +2416,9 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::NUM, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"}, {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"}, {RPCResult::Type::NUM_TIME, "timefirstkey", "the " + UNIX_EPOCH_TIME + " of the oldest known key in the wallet"}, - {RPCResult::Type::NUM_TIME, "keypoololdest", "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only"}, + {RPCResult::Type::NUM_TIME, "keypoololdest", /* optional */ true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only"}, {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"}, - {RPCResult::Type::NUM, "keypoolsize_hd_internal", "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, + {RPCResult::Type::NUM, "keypoolsize_hd_internal", /* optional */ true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, {RPCResult::Type::NUM, "keys_left", "how many new keys are left since last automatic backup"}, {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB"}, @@ -3056,18 +3056,18 @@ static RPCHelpMan listunspent() { {RPCResult::Type::STR_HEX, "txid", "the transaction id"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR, "address", "the Dash address"}, - {RPCResult::Type::STR, "label", "The associated label, or \"\" for the default label"}, + {RPCResult::Type::STR, "address", /* optional */ "the Dash address"}, + {RPCResult::Type::STR, "label", /* optional */ "The associated label, or \"\" for the default label"}, {RPCResult::Type::STR, "scriptPubKey", "the script key"}, {RPCResult::Type::STR_AMOUNT, "amount", "the transaction output amount in " + CURRENCY_UNIT}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, {RPCResult::Type::NUM, "ancestorcount", /* optional */ true, "The number of in-mempool ancestor transactions, including this one (if transaction is in the mempool)"}, {RPCResult::Type::NUM, "ancestorsize", /* optional */ true, "The virtual transaction size of in-mempool ancestors, including this one (if transaction is in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "ancestorfees", /* optional */ true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"}, - {RPCResult::Type::STR_HEX, "redeemScript", "The redeemScript if scriptPubKey is P2SH"}, + {RPCResult::Type::STR_HEX, "redeemScript", /* optional */ true, "The redeemScript if scriptPubKey is P2SH"}, {RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"}, {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, - {RPCResult::Type::STR, "desc", "(only when solvable) A descriptor for spending this output"}, + {RPCResult::Type::STR, "desc", /* optional */ true, "(only when solvable) A descriptor for spending this output"}, {RPCResult::Type::BOOL, "reused", /* optional*/ true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions" "from outside keys and unconfirmed replacement transactions are considered unsafe\n" @@ -4102,9 +4102,9 @@ static RPCHelpMan send() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, - {RPCResult::Type::STR_HEX, "hex", "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, - {RPCResult::Type::STR, "psbt", "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} + {RPCResult::Type::STR_HEX, "txid", /* optional */ true, "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, + {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, + {RPCResult::Type::STR, "psbt", /* optional */ true, "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} } }, RPCExamples{"" From 9faa63ff710d048bfb92054c7cf20c230b6a67e0 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Wed, 29 Sep 2021 10:07:04 +1300 Subject: [PATCH 11/11] Merge bitcoin/bitcoin#22650: Remove -deprecatedrpc=addresses flag and corresponding code/logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 43cd6b8af9d613ca033800c5cd8524c3f77e13ec doc: add release notes for removal of the -deprecatedrpc=addresses flag (Michael Dietz) 2b1fdc2c6ce1d0b0e51a3f107e23443c142d57af refactor: minor styling, prefer snake case and same line if (Michael Dietz) d64deac7b823a0eba97ab3a3686054eefe330d3c refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv (Michael Dietz) 8721638daa8502c7f8de5ae24a9393d7290a2ce5 rpc: remove deprecated addresses and reqSigs from rpc outputs (Michael Dietz) Pull request description: Resolves #21797 now that we've branched-off to v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed. `-deprecatedrpc=addresses` was initially added in this PR #20286 (which resolved the original issue #20102). Some chunks of code and logic are no longer used/necessary with the removal of this, and therefore some minor refactoring is done in this PR as well (separated commits) ACKs for top commit: MarcoFalke: re-ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec 🐉 meshcollider: Code review ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec jonatack: ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec per `git range-diff a9d0cec 92dc5e9 43cd6b8`, also rebased to latest master, debug built + quick re-review of each commit to bring back context, and ran tests locally at the final commit Tree-SHA512: fba83495e396d3c06f0dcf49292f14f4aa6b68fa758f0503941fade1a6e7271cda8378e2734af1faea550d1b43c85a36c52ebcc9dec0732936f9233b4b97901c --- doc/release-notes-22650.md | 9 +++ src/bitcoin-tx.cpp | 2 +- src/core_io.h | 6 +- src/core_write.cpp | 46 +++----------- src/rpc/blockchain.cpp | 15 +---- src/rpc/blockchain.h | 4 -- src/rpc/rawtransaction.cpp | 30 +++------ src/script/standard.cpp | 41 ------------ src/script/standard.h | 14 +---- src/test/fuzz/script.cpp | 62 +++++-------------- src/test/fuzz/transaction.cpp | 4 +- src/test/script_standard_tests.cpp | 61 ------------------ src/wallet/rpcwallet.cpp | 2 +- .../feature_dip3_deterministicmns.py | 4 +- test/functional/rpc_addresses_deprecation.py | 57 ----------------- test/functional/test_runner.py | 1 - 16 files changed, 53 insertions(+), 305 deletions(-) create mode 100644 doc/release-notes-22650.md delete mode 100755 test/functional/rpc_addresses_deprecation.py diff --git a/doc/release-notes-22650.md b/doc/release-notes-22650.md new file mode 100644 index 0000000000000..5c5e6e1032ad1 --- /dev/null +++ b/doc/release-notes-22650.md @@ -0,0 +1,9 @@ +Updated RPCs +------------ + +- The `-deprecatedrpc=addresses` configuration option has been removed. RPCs + `gettxout`, `getrawtransaction`, `decoderawtransaction`, `decodescript`, + `gettransaction verbose=true` and REST endpoints `/rest/tx`, `/rest/getutxos`, + `/rest/block` no longer return the `addresses` and `reqSigs` fields, which + were previously deprecated in 21.0. (#6569) + diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index c775afa7fcc12..ec6ea3c3e1fa4 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -691,7 +691,7 @@ static void MutateTx(CMutableTransaction& tx, const std::string& command, static void OutputTxJSON(const CTransaction& tx) { UniValue entry(UniValue::VOBJ); - TxToUniv(tx, uint256(), /* include_addresses */ false, entry); + TxToUniv(tx, uint256(), entry); std::string jsonOutput = entry.write(4); tfm::format(std::cout, "%s\n", jsonOutput); diff --git a/src/core_io.h b/src/core_io.h index c416dd8501fb7..c564e4e49cffb 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -45,8 +45,8 @@ UniValue ValueFromAmount(const CAmount amount); std::string FormatScript(const CScript& script); std::string EncodeHexTx(const CTransaction& tx); std::string SighashToStr(unsigned char sighash_type); -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses); -void ScriptToUniv(const CScript& script, UniValue& out, bool include_address); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); +void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true); +void ScriptToUniv(const CScript& script, UniValue& out); +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index 35f9bc2555912..493b83f3ac602 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -152,56 +152,28 @@ std::string EncodeHexTx(const CTransaction& tx) return HexStr(ssTx); } -void ScriptToUniv(const CScript& script, UniValue& out, bool include_address) +void ScriptToUniv(const CScript& script, UniValue& out) { - out.pushKV("asm", ScriptToAsmStr(script)); - out.pushKV("hex", HexStr(script)); - - std::vector> solns; - TxoutType type = Solver(script, solns); - out.pushKV("type", GetTxnOutputType(type)); - - CTxDestination address; - if (include_address && ExtractDestination(script, address) && type != TxoutType::PUBKEY) { - out.pushKV("address", EncodeDestination(address)); - } + ScriptPubKeyToUniv(script, out, /* include_hex */ true, /* include_address */ false); } -// TODO: from v21 ("addresses" and "reqSigs" deprecated) this method should be refactored to remove the `include_addresses` option -// this method can also be combined with `ScriptToUniv` as they will overlap -void ScriptPubKeyToUniv(const CScript& scriptPubKey, - UniValue& out, bool fIncludeHex, bool include_addresses) +void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address) { - TxoutType type; CTxDestination address; - std::vector addresses; - int nRequired; out.pushKV("asm", ScriptToAsmStr(scriptPubKey)); - if (fIncludeHex) - out.pushKV("hex", HexStr(scriptPubKey)); + if (include_hex) out.pushKV("hex", HexStr(scriptPubKey)); - if (!ExtractDestinations(scriptPubKey, type, addresses, nRequired) || type == TxoutType::PUBKEY) { - out.pushKV("type", GetTxnOutputType(type)); - return; - } + std::vector> solns; + const TxoutType type{Solver(scriptPubKey, solns)}; - if (ExtractDestination(scriptPubKey, address)) { + if (include_address && ExtractDestination(scriptPubKey, address) && type != TxoutType::PUBKEY) { out.pushKV("address", EncodeDestination(address)); } out.pushKV("type", GetTxnOutputType(type)); - - if (include_addresses) { - UniValue a(UniValue::VARR); - for (const CTxDestination& addr : addresses) { - a.push_back(EncodeDestination(addr)); - } - out.pushKV("addresses", a); - out.pushKV("reqSigs", nRequired); - } } -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_addresses, UniValue& entry, bool include_hex, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo) +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo) { uint256 txid = tx.GetHash(); entry.pushKV("txid", txid.GetHex()); @@ -269,7 +241,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, bool include_add out.pushKV("n", (int64_t)i); UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(txout.scriptPubKey, o, true, include_addresses); + ScriptPubKeyToUniv(txout.scriptPubKey, o, true); out.pushKV("scriptPubKey", o); // Add spent information if spentindex is enabled diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 0a10ea80100e4..9c5986911fd1b 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -169,7 +169,7 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn // coinbase transaction (i == 0) doesn't have undo data const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr; UniValue objTx(UniValue::VOBJ); - TxToUniv(*tx, uint256(), objTx, true, txundo); + TxToUniv(*tx, uint256(), objTx, true, 0, txundo); bool fLocked = isman.IsLocked(tx->GetHash()); objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool()); objTx.pushKV("instantlock_internal", fLocked); @@ -1207,11 +1207,8 @@ static RPCHelpMan gettxout() { {RPCResult::Type::STR, "asm", ""}, {RPCResult::Type::STR_HEX, "hex", ""}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR_HEX, "type", "The type, eg pubkeyhash"}, {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", - {{RPCResult::Type::STR, "address", "Dash address"}}}, }}, {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, }}, @@ -1899,16 +1896,6 @@ void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], s } } -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex) -{ - ScriptPubKeyToUniv(scriptPubKey, out, fIncludeHex, IsDeprecatedRPCEnabled("addresses")); -} - -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo) -{ - TxToUniv(tx, hashBlock, IsDeprecatedRPCEnabled("addresses"), entry, include_hex, txundo, ptxSpentInfo); -} - template static inline bool SetHasKeys(const std::set& set) {return false;} template diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index b282fa28951c0..13b9eac8575ee 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -6,7 +6,6 @@ #define BITCOIN_RPC_BLOCKCHAIN_H #include -#include #include #include #include @@ -49,9 +48,6 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex /** Used by getblockstats to get feerates at different percentiles by weight */ void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector>& scores, int64_t total_size); -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); - /** * Helper to create UTXO snapshots given a chainstate and a file handle. * @return a UniValue map containing metadata about the snapshot. diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0b598ae9cc2f6..d6ec322952fc8 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -96,7 +96,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, const CTxMemPool txSpentInfoPtr = &txSpentInfo; } - TxToUniv(tx, uint256(), entry, true, /* txundo = */ nullptr, txSpentInfoPtr); + TxToUniv(tx, uint256(), entry, true, 0, /* txundo = */ nullptr, txSpentInfoPtr); bool chainLock = false; if (!hashBlock.IsNull()) { @@ -178,13 +178,8 @@ static RPCHelpMan getrawtransaction() { {RPCResult::Type::STR, "asm", "the asm"}, {RPCResult::Type::STR, "hex", "the hex"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", - { - {RPCResult::Type::STR, "address", "Dash address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address (only if a well-defined address exists)"}, }}, }}, }}, @@ -763,13 +758,8 @@ static RPCHelpMan decoderawtransaction() { {RPCResult::Type::STR, "asm", "the asm"}, {RPCResult::Type::STR_HEX, "hex", "the hex"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", - { - {RPCResult::Type::STR, "address", "Dash address"}, - }}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address (only if a well-defined address exists)"}, }}, }}, }}, @@ -811,13 +801,7 @@ static RPCHelpMan decodescript() { {RPCResult::Type::STR, "asm", "Script public key"}, {RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"}, - {RPCResult::Type::STR, "address", /* optional */ true, "Dash address (only if a well-defined address exists)"}, - {RPCResult::Type::NUM, "reqSigs", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Number of required signatures"}, - {RPCResult::Type::ARR, "addresses", /* optional */ true, "(DEPRECATED, returned only if config option -deprecatedrpc=addresses is passed) Array of Dash addresses", - { - {RPCResult::Type::STR, "address", "Dash address"}, - }}, - {RPCResult::Type::STR, "p2sh", "address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH)"}, + {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address (only if a well-defined address exists)"}, }, }, RPCExamples{ @@ -836,7 +820,7 @@ static RPCHelpMan decodescript() } else { // Empty scripts are valid } - ScriptPubKeyToUniv(script, r, /* fIncludeHex */ false); + ScriptPubKeyToUniv(script, r, /* include_hex */ false); std::vector> solutions_data; const TxoutType which_type{Solver(script, solutions_data)}; @@ -1490,7 +1474,7 @@ static RPCHelpMan decodepsbt() // Redeem script if (!input.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(input.redeem_script, r, false); + ScriptToUniv(input.redeem_script, r); in.pushKV("redeem_script", r); } @@ -1588,7 +1572,7 @@ static RPCHelpMan decodepsbt() // Redeem script if (!output.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(output.redeem_script, r, false); + ScriptToUniv(output.redeem_script, r); out.pushKV("redeem_script", r); } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 03ef9dc984515..8ec9d26227ed0 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -196,47 +196,6 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) assert(false); } -// TODO: from v21 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed -bool ExtractDestinations(const CScript& scriptPubKey, TxoutType& typeRet, std::vector& addressRet, int& nRequiredRet) -{ - addressRet.clear(); - std::vector vSolutions; - typeRet = Solver(scriptPubKey, vSolutions); - if (typeRet == TxoutType::NONSTANDARD) { - return false; - } else if (typeRet == TxoutType::NULL_DATA) { - // This is data, not addresses - return false; - } - - if (typeRet == TxoutType::MULTISIG) - { - nRequiredRet = vSolutions.front()[0]; - for (unsigned int i = 1; i < vSolutions.size()-1; i++) - { - CPubKey pubKey(vSolutions[i]); - if (!pubKey.IsValid()) - continue; - - CTxDestination address = PKHash(pubKey); - addressRet.push_back(address); - } - - if (addressRet.empty()) - return false; - } - else - { - nRequiredRet = 1; - CTxDestination address; - if (!ExtractDestination(scriptPubKey, address)) - return false; - addressRet.push_back(address); - } - - return true; -} - namespace { class CScriptVisitor { diff --git a/src/script/standard.h b/src/script/standard.h index 756734a734be3..b16fe51f3704a 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -122,23 +122,11 @@ TxoutType Solver(const CScript& scriptPubKey, std::vector& addressRet, int& nRequiredRet); - /** * Generate a Bitcoin scriptPubKey for the given CTxDestination. Returns a P2PKH * script for a CKeyID destination, a P2SH script for a CScriptID, and an empty diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index 74c59537a7a0c..ebadae41421cb 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -49,46 +49,8 @@ FUZZ_TARGET(script, .init = initialize_script) assert(script == decompressed_script); } - CTxDestination address; - TxoutType type_ret; - std::vector addresses; - int required_ret; - bool extract_destinations_ret = ExtractDestinations(script, type_ret, addresses, required_ret); - bool extract_destination_ret = ExtractDestination(script, address); - if (!extract_destinations_ret) { - assert(!extract_destination_ret); - if (type_ret == TxoutType::MULTISIG) { - assert(addresses.empty() && required_ret == 0); - } else { - assert(type_ret == TxoutType::PUBKEY || - type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA); - } - } else { - assert(required_ret >= 1 && required_ret <= 16); - assert((unsigned long)required_ret == addresses.size()); - assert(type_ret == TxoutType::MULTISIG || required_ret == 1); - } - if (type_ret == TxoutType::NONSTANDARD || type_ret == TxoutType::NULL_DATA) { - assert(!extract_destinations_ret); - } - if (!extract_destination_ret) { - assert(type_ret == TxoutType::PUBKEY || - type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA || - type_ret == TxoutType::MULTISIG); - } else { - assert(address == addresses[0]); - } - if (type_ret == TxoutType::NONSTANDARD || - type_ret == TxoutType::NULL_DATA || - type_ret == TxoutType::MULTISIG) { - assert(!extract_destination_ret); - } - TxoutType which_type; bool is_standard_ret = IsStandard(script, which_type); - assert(type_ret == which_type); if (!is_standard_ret) { assert(which_type == TxoutType::NONSTANDARD || which_type == TxoutType::NULL_DATA || @@ -105,6 +67,20 @@ FUZZ_TARGET(script, .init = initialize_script) which_type == TxoutType::NONSTANDARD); } + CTxDestination address; + bool extract_destination_ret = ExtractDestination(script, address); + if (!extract_destination_ret) { + assert(which_type == TxoutType::PUBKEY || + which_type == TxoutType::NONSTANDARD || + which_type == TxoutType::NULL_DATA || + which_type == TxoutType::MULTISIG); + } + if (which_type == TxoutType::NONSTANDARD || + which_type == TxoutType::NULL_DATA || + which_type == TxoutType::MULTISIG) { + assert(!extract_destination_ret); + } + const FlatSigningProvider signing_provider; (void)InferDescriptor(script, signing_provider); (void)IsSolvable(signing_provider, script); @@ -123,15 +99,11 @@ FUZZ_TARGET(script, .init = initialize_script) (void)ScriptToAsmStr(script, true); UniValue o1(UniValue::VOBJ); - ScriptPubKeyToUniv(script, o1, true, true); - ScriptPubKeyToUniv(script, o1, true, false); + ScriptPubKeyToUniv(script, o1, true); UniValue o2(UniValue::VOBJ); - ScriptPubKeyToUniv(script, o2, false, true); - ScriptPubKeyToUniv(script, o2, false, false); + ScriptPubKeyToUniv(script, o2, false); UniValue o3(UniValue::VOBJ); - ScriptToUniv(script, o3, true); - UniValue o4(UniValue::VOBJ); - ScriptToUniv(script, o4, false); + ScriptToUniv(script, o3); { const std::vector bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 27fff4ce1e7e9..5095a984101d5 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -94,6 +94,6 @@ FUZZ_TARGET(transaction, .init = initialize_transaction) (void)AreInputsStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); - TxToUniv(tx, /* hashBlock */ uint256::ZERO, /* include_addresses */ true, u); - TxToUniv(tx, /* hashBlock */ uint256::ONE, /* include_addresses */ false, u); + TxToUniv(tx, /* hashBlock */ uint256::ZERO, u); + TxToUniv(tx, /* hashBlock */ uint256::ONE, u); } diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 53d4f7046c0bb..6eb3474362797 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -185,67 +185,6 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) BOOST_CHECK(!ExtractDestination(s, address)); } -BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) -{ - CKey keys[3]; - CPubKey pubkeys[3]; - for (int i = 0; i < 3; i++) { - keys[i].MakeNewKey(true); - pubkeys[i] = keys[i].GetPubKey(); - } - - CScript s; - TxoutType whichType; - std::vector addresses; - int nRequired; - - // TxoutType::PUBKEY - s.clear(); - s << ToByteVector(pubkeys[0]) << OP_CHECKSIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEY); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); - - // TxoutType::PUBKEYHASH - s.clear(); - s << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEYHASH); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); - - // TxoutType::SCRIPTHASH - CScript redeemScript(s); // initialize with leftover P2PKH script - s.clear(); - s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::SCRIPTHASH); - BOOST_CHECK_EQUAL(addresses.size(), 1U); - BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get(addresses[0]) == ScriptHash(redeemScript)); - - // TxoutType::MULTISIG - s.clear(); - s << OP_2 << - ToByteVector(pubkeys[0]) << - ToByteVector(pubkeys[1]) << - OP_2 << OP_CHECKMULTISIG; - BOOST_CHECK(ExtractDestinations(s, whichType, addresses, nRequired)); - BOOST_CHECK_EQUAL(whichType, TxoutType::MULTISIG); - BOOST_CHECK_EQUAL(addresses.size(), 2U); - BOOST_CHECK_EQUAL(nRequired, 2); - BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); - BOOST_CHECK(std::get(addresses[1]) == PKHash(pubkeys[1])); - - // TxoutType::NULL_DATA - s.clear(); - s << OP_RETURN << std::vector({75}); - BOOST_CHECK(!ExtractDestinations(s, whichType, addresses, nRequired)); -} - BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_) { CKey keys[3]; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6b189970390ad..5aa51e8ca6dbd 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1656,7 +1656,7 @@ static RPCHelpMan gettransaction() if (verbose) { UniValue decoded(UniValue::VOBJ); - TxToUniv(*wtx.tx, uint256(), pwallet->chain().rpcEnableDeprecated("addresses"), decoded, false); + TxToUniv(*wtx.tx, uint256(), decoded, false); entry.pushKV("decoded", decoded); } diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 3be51eba75b53..25fe244f869f9 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -177,8 +177,8 @@ def run_test(self): block = self.nodes[0].getblock(self.nodes[0].getbestblockhash()) cbtx = self.nodes[0].getrawtransaction(block['tx'][0], 1) for out in cbtx['vout']: - if 'addresses' in out['scriptPubKey']: - if expected_payee in out['scriptPubKey']['addresses'] and out['valueSat'] == expected_amount: + if 'address' in out['scriptPubKey']: + if expected_payee in out['scriptPubKey']['address'] and out['valueSat'] == expected_amount: found_multisig_payee = True assert found_multisig_payee diff --git a/test/functional/rpc_addresses_deprecation.py b/test/functional/rpc_addresses_deprecation.py deleted file mode 100755 index 1028f4b55382c..0000000000000 --- a/test/functional/rpc_addresses_deprecation.py +++ /dev/null @@ -1,57 +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 reqSigs and addresses RPC fields.""" - -from io import BytesIO - -from test_framework.messages import CTransaction -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_equal, -) - - -class AddressesDeprecationTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 2 - self.extra_args = [[], ["-deprecatedrpc=addresses"]] - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def run_test(self): - self.test_addresses_deprecation() - - def test_addresses_deprecation(self): - node = self.nodes[0] - coin = node.listunspent().pop() - - inputs = [{'txid': coin['txid'], 'vout': coin['vout']}] - outputs = {node.getnewaddress(): 0.99} - raw = node.createrawtransaction(inputs, outputs) - signed = node.signrawtransactionwithwallet(raw)['hex'] - - # This transaction is derived from test/util/data/txcreatemultisig1.json - tx = CTransaction() - tx.deserialize(BytesIO(bytes.fromhex(signed))) - tx.vout[0].scriptPubKey = bytes.fromhex("522102a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff39721021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d2102df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb48553ae") - tx_signed = node.signrawtransactionwithwallet(tx.serialize().hex())['hex'] - txid = node.sendrawtransaction(hexstring=tx_signed, maxfeerate=0) - - self.log.info("Test RPCResult scriptPubKey no longer returns the fields addresses or reqSigs by default") - hash = self.generateblock(node, output=node.getnewaddress(), transactions=[txid])['hash'] - # Ensure both nodes have the newly generated block on disk. - self.sync_blocks() - script_pub_key = node.getblock(blockhash=hash, verbose=2)['tx'][-1]['vout'][0]['scriptPubKey'] - assert 'addresses' not in script_pub_key and 'reqSigs' not in script_pub_key - - self.log.info("Test RPCResult scriptPubKey returns the addresses field with -deprecatedrpc=addresses") - script_pub_key = self.nodes[1].getblock(blockhash=hash, verbose=2)['tx'][-1]['vout'][0]['scriptPubKey'] - assert_equal(script_pub_key['addresses'], ['yb7hsErReWuYeyxH1LzXbmyU5iBdruakMi', 'yarLqM3oXZFRtuVR4SsNxqKrGxyScZhohq', 'yPfMS8LWznSCnmz1QnEnMxqHZXUrq5Lfu7']) - assert_equal(script_pub_key['reqSigs'], 2) - - -if __name__ == "__main__": - AddressesDeprecationTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 531c71e884c41..d07b3a4a69fde 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -379,7 +379,6 @@ 'feature_config_args.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py', - 'rpc_addresses_deprecation.py', 'rpc_getpeerinfo_deprecation.py', 'rpc_mempool_entry_fee_fields_deprecation.py', 'rpc_help.py',