Repository navigation
backport: bitcoin#27571, #27662, #27739, #27783, #27790, #27846, #27889, #27907, #27932, #28194, partial #26485 #8010
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8956c0d
e028848
5c6a42b
b617006
7f10ca1
7ad03f8
b9c72b5
c1dec43
2b726d3
ac349d1
2552f14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -94,45 +94,16 @@ python3 "${CLANG_TIDY_CACHE_PY}" --show-stats 2>&1 || true | |||||||||
| cleanup_ctcache "${CTCACHE_DIR}" "${CTCACHE_MAXSIZE_MB}" | ||||||||||
| echo "==========================" | ||||||||||
|
|
||||||||||
| # Filter out files by regex here, because regex may not be | ||||||||||
| # accepted in src/.bear-tidy-config | ||||||||||
| # Filter out: | ||||||||||
| # * qt qrc and moc generated files | ||||||||||
| # * secp256k1 | ||||||||||
| jq 'map(select(.file | test("src/qt/qrc_.*\\.cpp$|/moc_.*\\.cpp$|src/secp256k1/src/") | not))' ../compile_commands.json > tmp.json | ||||||||||
| mv tmp.json ../compile_commands.json | ||||||||||
|
Comment on lines
+102
to
+103
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Retain bitcoin#27571's walletutil exclusion Removing the IWYU whitelist now includes src/wallet/walletutil.cpp, although the new comment explicitly lists walletutil as temporarily excluded. Upstream merge 4c3d67a includes src/wallet/walletutil in this exact filter before broadening IWYU to all sources; the Dash adaptation drops that alternative. walletutil.cpp remains a compiled wallet source, so the omission changes the filtering behavior rather than merely leaving a stale comment. Restore the upstream exclusion, or establish why it is no longer needed for Dash. No resulting build failure is asserted.
Suggested change
source:
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Withdrawn (re-reviewed at |
||||||||||
|
|
||||||||||
| cd "${BASE_ROOT_DIR}/build-ci/dashcore-${BUILD_TARGET}" | ||||||||||
| iwyu_tool.py \ | ||||||||||
| "src/common/init.cpp" \ | ||||||||||
| "src/common/url.cpp" \ | ||||||||||
| "src/compat" \ | ||||||||||
| "src/dbwrapper.cpp" \ | ||||||||||
| "src/init" \ | ||||||||||
| "src/kernel" \ | ||||||||||
| "src/node/chainstate.cpp" \ | ||||||||||
| "src/node/chainstatemanager_args.cpp" \ | ||||||||||
| "src/node/mempool_args.cpp" \ | ||||||||||
| "src/node/utxo_snapshot.cpp" \ | ||||||||||
| "src/node/validation_cache_args.cpp" \ | ||||||||||
| "src/node/minisketchwrapper.cpp" \ | ||||||||||
| "src/policy/feerate.cpp" \ | ||||||||||
| "src/policy/packages.cpp" \ | ||||||||||
| "src/policy/settings.cpp" \ | ||||||||||
| "src/primitives/transaction.cpp" \ | ||||||||||
| "src/random.cpp" \ | ||||||||||
| "src/rpc/fees.cpp" \ | ||||||||||
| "src/rpc/signmessage.cpp" \ | ||||||||||
| "src/test/fuzz/string.cpp" \ | ||||||||||
| "src/test/fuzz/txorphan.cpp" \ | ||||||||||
| "src/test/fuzz/util/" \ | ||||||||||
| "src/util/bip32.cpp" \ | ||||||||||
| "src/util/bytevectorhash.cpp" \ | ||||||||||
| "src/util/check.cpp" \ | ||||||||||
| "src/util/error.cpp" \ | ||||||||||
| "src/util/exception.cpp" \ | ||||||||||
| "src/util/hasher.cpp" \ | ||||||||||
| "src/util/message.cpp" \ | ||||||||||
| "src/util/moneystr.cpp" \ | ||||||||||
| "src/util/serfloat.cpp" \ | ||||||||||
| "src/util/spanparsing.cpp" \ | ||||||||||
| "src/util/string.cpp" \ | ||||||||||
| "src/util/strencodings.cpp" \ | ||||||||||
| "src/util/syserror.cpp" \ | ||||||||||
| "src/util/threadinterrupt.cpp" \ | ||||||||||
| "src/zmq" \ | ||||||||||
| -p . "${MAKEJOBS}" \ | ||||||||||
| -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_ROOT_DIR}/contrib/devtools/iwyu/bitcoin.core.imp" \ | ||||||||||
| -Xiwyu --max_line_length=160 \ | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,3 +15,5 @@ export CI_OS_NAME="macos" | |
| export NO_DEPENDS=1 | ||
| export OSX_SDK="" | ||
| export CCACHE_MAXSIZE=400M | ||
| export RUN_FUZZ_TESTS=true | ||
| export FUZZ_TESTS_CONFIG="--exclude banman" # https://github.com/bitcoin/bitcoin/issues/27924 | ||
|
Comment on lines
+18
to
+19
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Preserve bitcoin#27932's macOS fuzz CI coverage These exports reproduce upstream's configuration additions but do not enable corpus replay in Dash CI. Upstream's native macOS job selects the modified environment and ci/test/06_script_b.sh consumes RUN_FUZZ_TESTS and FUZZ_TESTS_CONFIG to launch the runner. Dash's mac-build job instead selects 00_setup_env_mac_cross.sh through ci/dash/matrix.sh, and no checked CI execution script consumes these settings. All four upstream changed areas have counterparts in this backport; the remaining defect is the ineffective CI adaptation, not an omitted production API or a demonstrated runtime failure. Adapt an existing appropriate job to execute macOS replay without adding a matrix job, or explicitly document the partial coverage and link a tracking issue before deferring it. source:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These exports are kept so the env file stays identical to upstream and later backports apply cleanly. Dash CI never replays the fuzz corpus on any platform: no script in ci/dash or .github/workflows calls test/fuzz/test_runner.py or reads RUN_FUZZ_TESTS, not even for linux64_fuzz, and no workflow selects 00_setup_env_mac_native.sh. Adding macOS corpus replay would be a new CI feature, not part of this backport. The bitcoin#27932 code changes (test_runner.py, rpc.cpp, config.ini.in) are fully included.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right: Withdrawing this finding.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Withdrawn (re-reviewed at |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ RUN set -ex; \ | |
| curl \ | ||
| g++ \ | ||
| git \ | ||
| jq \ | ||
| libbz2-dev \ | ||
| liblzma-dev \ | ||
| libreadline-dev \ | ||
|
|
@@ -79,7 +80,7 @@ ENV UV_SYSTEM_PYTHON=1 | |
| # NOTE: if versions are changed, update ci/lint/04_install.sh | ||
| RUN uv pip install --system --break-system-packages \ | ||
| codespell==2.2.1 \ | ||
| flake8==5.0.4 \ | ||
| flake8==6.1.0 \ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Adapt Dash type-comment imports for the bitcoin#28194 lint upgrade The upgrade from flake8 5.0.4 to 6.1.0 leaves Dash-specific imports incompatible with the new checker. CI run 41651190122, job 125993768620, fails lint-python.py with F401 for MasternodeInfo in feature_governance.py, feature_llmq_connections.py, feature_llmq_singlenode.py, feature_mnehf.py and interface_zmq_dash.py. In those files the name appears only in loop type comments, which the newer checker does not count as import uses. Remove the unused imports or replace the comments with supported typing constructs, retaining the synchronized flake8 pins. This is a demonstrated validation regression from the tool upgrade, not merely a request for stylistic cleanup. source:
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved (re-reviewed at
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Withdrawn (re-reviewed at |
||
| jinja2 \ | ||
| lief==0.13.2 \ | ||
| mypy==0.981 \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| JSON-RPC | ||
| --- | ||
|
|
||
| For RPC methods which accept `options` parameters ((`importmulti`, `listunspent`, `fundrawtransaction`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), it is now possible to pass the options as named parameters without the need for a nested object. (#8010) | ||
|
|
||
| This means it is possible make calls like: | ||
|
|
||
| ```sh | ||
| src/dash-cli -named listunspent coinType=1 | ||
| ``` | ||
|
|
||
| instead of | ||
|
|
||
| ```sh | ||
| src/dash-cli -named listunspent query_options='{"coinType": 1}' | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,12 @@ static const CRPCConvertParam vRPCConvertParams[] = | |
| { "listunspent", 2, "addresses" }, | ||
| { "listunspent", 3, "include_unsafe" }, | ||
| { "listunspent", 4, "query_options" }, | ||
| { "listunspent", 4, "minimumAmount" }, | ||
| { "listunspent", 4, "maximumAmount" }, | ||
| { "listunspent", 4, "maximumCount" }, | ||
| { "listunspent", 4, "minimumSumAmount" }, | ||
| { "listunspent", 4, "include_immature_coinbase" }, | ||
|
Comment on lines
+124
to
+128
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking: Adapt the portable bitcoin#26485 tests from excluded suites The advertised full bitcoin#26485 backport still omits two portable upstream test transformations. Upstream merge 34ac3f4 changes wallet_bumpfee.py:308 from listunspent(query_options={'minimumAmount': fee}) to listunspent(minimumAmount=fee), and wallet_taproot.py:381 from sendall(..., options={'psbt': True}) to sendall(..., psbt=True). Both operations are supported by Dash independently of RBF or Taproot. At this head, functional listunspent query-filter calls still use query_options, and no functional sendall call passes the named psbt option, so neither transformation has equivalent coverage. The PR description contains no explicit exclusion for these hunks. Preserve their coverage in existing suites—for example, adapt a minimumAmount call in wallet_basic.py and add an explicit named psbt case in wallet_sendall.py—without importing the unsupported subsystems. This is a full-backport completeness blocker, not evidence of a production runtime failure. source: |
||
| { "listunspent", 4, "coinType" }, | ||
| { "getblock", 1, "verbosity" }, | ||
| { "getblock", 1, "verbose" }, | ||
| { "getblockheader", 1, "verbose" }, | ||
|
|
@@ -152,10 +158,37 @@ static const CRPCConvertParam vRPCConvertParams[] = | |
| { "submitpackage", 0, "package" }, | ||
| { "combinerawtransaction", 0, "txs" }, | ||
| { "fundrawtransaction", 1, "options" }, | ||
| { "fundrawtransaction", 1, "add_inputs"}, | ||
| { "fundrawtransaction", 1, "include_unsafe"}, | ||
| { "fundrawtransaction", 1, "minconf"}, | ||
| { "fundrawtransaction", 1, "maxconf"}, | ||
| { "fundrawtransaction", 1, "changePosition"}, | ||
| { "fundrawtransaction", 1, "includeWatching"}, | ||
| { "fundrawtransaction", 1, "lockUnspents"}, | ||
| { "fundrawtransaction", 1, "fee_rate"}, | ||
| { "fundrawtransaction", 1, "feeRate"}, | ||
| { "fundrawtransaction", 1, "subtractFeeFromOutputs"}, | ||
| { "fundrawtransaction", 1, "input_sizes"}, | ||
| { "fundrawtransaction", 1, "conf_target"}, | ||
| { "fundrawtransaction", 1, "solving_data"}, | ||
| { "fundrawtransaction", 1, "use_cj"}, | ||
| { "walletcreatefundedpsbt", 0, "inputs" }, | ||
| { "walletcreatefundedpsbt", 1, "outputs" }, | ||
| { "walletcreatefundedpsbt", 2, "locktime" }, | ||
| { "walletcreatefundedpsbt", 3, "options" }, | ||
| { "walletcreatefundedpsbt", 3, "add_inputs"}, | ||
| { "walletcreatefundedpsbt", 3, "include_unsafe"}, | ||
| { "walletcreatefundedpsbt", 3, "minconf"}, | ||
| { "walletcreatefundedpsbt", 3, "maxconf"}, | ||
| { "walletcreatefundedpsbt", 3, "changePosition"}, | ||
| { "walletcreatefundedpsbt", 3, "includeWatching"}, | ||
| { "walletcreatefundedpsbt", 3, "lockUnspents"}, | ||
| { "walletcreatefundedpsbt", 3, "fee_rate"}, | ||
| { "walletcreatefundedpsbt", 3, "feeRate"}, | ||
| { "walletcreatefundedpsbt", 3, "subtractFeeFromOutputs"}, | ||
| { "walletcreatefundedpsbt", 3, "conf_target"}, | ||
| { "walletcreatefundedpsbt", 3, "solving_data"}, | ||
| { "walletcreatefundedpsbt", 3, "use_cj"}, | ||
| { "walletcreatefundedpsbt", 4, "bip32derivs" }, | ||
| { "walletprocesspsbt", 1, "sign" }, | ||
| { "walletprocesspsbt", 3, "bip32derivs" }, | ||
|
|
@@ -182,19 +215,50 @@ static const CRPCConvertParam vRPCConvertParams[] = | |
| { "send", 1, "conf_target" }, | ||
| { "send", 3, "fee_rate"}, | ||
| { "send", 4, "options" }, | ||
| { "send", 4, "add_inputs"}, | ||
| { "send", 4, "include_unsafe"}, | ||
| { "send", 4, "minconf"}, | ||
| { "send", 4, "maxconf"}, | ||
| { "send", 4, "add_to_wallet"}, | ||
| { "send", 4, "change_position"}, | ||
| { "send", 4, "fee_rate"}, | ||
| { "send", 4, "include_watching"}, | ||
| { "send", 4, "inputs"}, | ||
| { "send", 4, "locktime"}, | ||
| { "send", 4, "lock_unspents"}, | ||
| { "send", 4, "psbt"}, | ||
| { "send", 4, "subtract_fee_from_outputs"}, | ||
| { "send", 4, "conf_target"}, | ||
| { "send", 4, "solving_data"}, | ||
| { "send", 4, "use_cj"}, | ||
| { "sendall", 0, "recipients" }, | ||
| { "sendall", 1, "conf_target" }, | ||
| { "sendall", 3, "fee_rate"}, | ||
| { "sendall", 4, "options" }, | ||
| { "sendall", 4, "add_to_wallet"}, | ||
| { "sendall", 4, "fee_rate"}, | ||
| { "sendall", 4, "include_watching"}, | ||
| { "sendall", 4, "inputs"}, | ||
| { "sendall", 4, "locktime"}, | ||
| { "sendall", 4, "lock_unspents"}, | ||
| { "sendall", 4, "psbt"}, | ||
| { "sendall", 4, "send_max"}, | ||
| { "sendall", 4, "minconf"}, | ||
| { "sendall", 4, "maxconf"}, | ||
| { "sendall", 4, "conf_target"}, | ||
| { "sendall", 4, "solving_data"}, | ||
| { "sendall", 4, "use_cj"}, | ||
| { "simulaterawtransaction", 0, "rawtxs" }, | ||
| { "simulaterawtransaction", 1, "options" }, | ||
| { "simulaterawtransaction", 1, "include_watchonly"}, | ||
| { "importprivkey", 2, "rescan" }, | ||
| { "importelectrumwallet", 1, "index" }, | ||
| { "importaddress", 2, "rescan" }, | ||
| { "importaddress", 3, "p2sh" }, | ||
| { "importpubkey", 2, "rescan" }, | ||
| { "importmulti", 0, "requests" }, | ||
| { "importmulti", 1, "options" }, | ||
| { "importmulti", 1, "rescan" }, | ||
| { "importdescriptors", 0, "requests" }, | ||
| { "listdescriptors", 0, "private" }, | ||
| { "verifychain", 0, "checklevel" }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the checked
linux64_multiprocesspath,.github/workflows/build-src.yml:209-216invokes this script, but this regexp does not matchsrc/wallet/walletutil.cppeven though lines 99-102 explicitly declare it temporarily excluded. Removing the old source whitelist therefore sends that known-unready translation unit throughiwyu_tool.pyandfix_includes.py; addsrc/wallet/walletutil.cppto the filter so the intended exclusion actually applies.AGENTS.md reference: AGENTS.md:L197-L202
Useful? React with 👍 / 👎.