backport: Bitcoin Core v0.26 backports (batch 3)#7433
Conversation
|
This pull request has conflicts, please rebase. |
thepastaclaw
force-pushed
the
backport-0.26-batch-3-staging
branch
from
cb684ab to
5f660ae
Compare
|
CI scope check for tracker #1918: the failing job is the known flaky |
|
🔍 Review in progress — actively reviewing now (commit 3dc9d29) |
There was a problem hiding this comment.
Code Review
Source: reviewers claude/general=opus(failed), codex/general=gpt-5.5(completed), claude/dash-core-commit-history=opus(failed), codex/dash-core-commit-history=gpt-5.5(completed), claude/backport-reviewer=opus(failed), codex/backport-reviewer=gpt-5.5(completed); verifier=gpt-5.5.
Verified head 5f660ae. The dashd init split contains a real merge-resolution regression, and several advertised full Bitcoin PR backports omit upstream hunks because Dash is missing prerequisite Bitcoin infrastructure. The deterministic backport-prereq policy gate was applied and its restored prerequisite evidence has been merged into the findings below. No CodeRabbit inline findings were present.
5 blocking | 1 nitpick
Note: GitHub does not allow me to request changes on my own PR, so this is posted as a COMMENT while preserving the verified blocking findings.
Verified Findings
blocking: -printcrashinfo no longer exits after printing
src/bitcoind.cpp:121-123
ParseArgs() prints decoded -printcrashinfo output and returns true, but main() treats that only as successful argument parsing and then continues through ProcessInitCommands() and AppInit(). ProcessInitCommands() only handles help/version, so dashd -printcrashinfo=... can proceed into normal node startup after printing. The same split also moved help/version handling behind datadir/config/settings initialization, while the pre-backport code handled these informational commands before datadir/config validation.
blocking: Missing prerequisite: bitcoin#18044 for bitcoin#27625 wtxid coverage
test/functional/p2p_leak_tx.py:7-8
Upstream bitcoin#27625 changes p2p_leak_tx.py to import MSG_WTX and P2PTxInvStore, adds test_notfound_on_replaced_tx(), and requests both txid and wtxid inventory for the replaced transaction. The Dash cherry-pick only adds disconnect_p2ps() to the older txid-only test because Dash lacks the wtxid relay/test framework; tracing MSG_WTX points to bitcoin#18044. Since the PR advertises a full backport of bitcoin#27625, this omitted upstream test path is a missing prerequisite that needs either the prerequisite backport or an explicit documented exclusion.
blocking: Missing prerequisite: bitcoin#27150 common::InitConfig for bitcoin#27708
src/bitcoind.cpp:126-142
In upstream bitcoin#27708, ParseArgs delegates datadir/config/settings setup to common::InitConfig(args). Dash substitutes the older local sequence (CheckDataDirOption(), ReadConfigFiles(), SelectParams(), InitSettings()) because common::InitConfig is absent; that helper and supporting common/init split were introduced by bitcoin#27150. For a full advertised backport, this is a missing prerequisite rather than a faithful application of the upstream starting state.
blocking: Missing prerequisite: bitcoin#19988 txrequest.cpp hunk for bitcoin#27783
test/lint/lint-includes.py:26-41
Upstream bitcoin#27783 adds explicit Boost public headers in src/txrequest.cpp, including boost/tuple/tuple.hpp, and updates the lint allowlist accordingly. Dash has no src/txrequest.cpp, so the cherry-pick omits that upstream hunk and does not add the matching lint entry. The missing module comes from bitcoin#19988, making this a missing prerequisite for a full backport of bitcoin#27783 unless the exclusion is documented.
blocking: Missing prerequisite: bitcoin#27556/bitcoin#26715 wallet test hunks for bitcoin#27783
src/wallet/test/walletdb_tests.cpp:30-30
Upstream bitcoin#27783 changes BOOST_ASSERT uses in walletdb_read_write_deadlock and wallet_load_ckey. In this Dash tree, walletdb_tests.cpp ends after walletdb_readkeyvalue, and src/wallet/test/walletload_tests.cpp is absent, so both upstream hunks were omitted. Bitcoin history shows walletdb_read_write_deadlock was introduced by bitcoin#27556, while the wallet_load_ckey shape touched by bitcoin#27783 depends on wallet-test work from bitcoin#26715. These are test-only omissions, but they are missing prerequisites under the full-backport completeness rule unless explicitly excluded.
nitpick: Remove transient recovery note from commit message
<commit:6be2980>:1-1
Commit 6be2980 includes an operational recovery note about a Codex worker failure, parent-session staging, and a stray Qt conflict artifact. That is permanent git log noise and does not help future backport archaeology. Keep the durable backport provenance and validation, but drop the internal recovery details before merge.
Potential PR merge conflictsThis is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order. If this PR merges firstThese open PRs will likely need a rebase:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddf030f4c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
WalkthroughThe pull request centralizes configuration initialization, reorganizes daemon and GUI startup sequencing, propagates atomic exit status through node lifecycle and shutdown paths, and changes fatal failures to use Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MAIN_FUNCTION
participant ParseArgs
participant AppInit
participant NodeContext
participant Shutdown
MAIN_FUNCTION->>ParseArgs: parse arguments and configuration
ParseArgs-->>MAIN_FUNCTION: success or failure
MAIN_FUNCTION->>AppInit: initialize NodeContext
AppInit->>NodeContext: register and update exit_status
MAIN_FUNCTION->>Shutdown: Interrupt and Shutdown
Shutdown-->>MAIN_FUNCTION: return exit_status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shutdown.cpp`:
- Around line 25-38: Initialize the shutdown state for standalone chainstate
execution before any validation or blockstorage work can call AbortNode. Locate
the chainstate entry point in bitcoin-chainstate.cpp and invoke
InitShutdownState with the appropriate exit-status storage during startup,
ensuring g_exit_status is set before failures are handled; preserve the existing
initialization path in init.cpp.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: faecc176-cea3-4d9f-ae2e-e77d77a67d34
📒 Files selected for processing (21)
ci/test/00_setup_env_native_asan.shci/test/00_setup_env_native_fuzz.shsrc/bitcoin-chainstate.cppsrc/bitcoind.cppsrc/index/base.cppsrc/init.cppsrc/init.hsrc/interfaces/node.hsrc/net_processing.cppsrc/node/blockstorage.cppsrc/node/context.hsrc/node/interfaces.cppsrc/qt/bitcoin.cppsrc/qt/bitcoin.hsrc/shutdown.cppsrc/shutdown.hsrc/test/fuzz/utxo_total_supply.cppsrc/wallet/test/wallet_tests.cpptest/functional/feature_abortnode.pytest/functional/p2p_leak_tx.pytest/functional/test_framework/test_node.py
💤 Files with no reviewable changes (1)
- src/qt/bitcoin.h
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: reviewers claude/general=opus(completed), codex/general=gpt-5.5(failed: ACP refresh token revoked), claude/dash-core-commit-history=opus(completed), codex/dash-core-commit-history=gpt-5.5(failed: ACP refresh token revoked), claude/backport-reviewer=opus(completed), codex/backport-reviewer=gpt-5.5(failed: ACP refresh token revoked); verifier=opus.
Full cumulative verification at head ddf030f. Delta from prior 5f660ae is one empty CI-retrigger commit with zero tree changes, so all prior verified blocking findings still apply. Confirmed the -printcrashinfo merge-resolution regression at src/bitcoind.cpp:121-124/286-288 (ParseArgs returns true, main() only bypasses AppInit for ParseArgs failure or ProcessInitCommands success, so -printcrashinfo falls through to normal node startup). Confirmed four missing upstream prerequisites via direct filesystem/grep checks: no src/txrequest.cpp (bitcoin#19988), no src/wallet/test/walletload_tests.cpp and no walletdb_read_write_deadlock (bitcoin#27556/bitcoin#26715), no common::InitConfig (bitcoin#27150), and p2p_leak_tx.py lacks wtxid coverage (bitcoin#18044). Two style findings on commit hygiene (empty CI-retrigger commit at HEAD, transient recovery note in 6be2980) also verified. Codex lanes failed to produce JSON due to ACP refresh token revocation; no attribution possible. CodeRabbit had no findings.
Commit-history nit: 6be29805584 still embeds transient Codex worker/recovery diagnostics in its permanent commit message; retain durable provenance/validation and remove that operational note before merge.
🔴 5 blocking | 🟡 1 suggestion(s) | 💬 1 nitpick(s)
5 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/bitcoind.cpp`:
- [BLOCKING] src/bitcoind.cpp:121-123: -printcrashinfo no longer exits after printing
`ParseArgs()` prints decoded `-printcrashinfo` output and returns `true` (lines 121-124), but `main()` at lines 286-288 only bypasses `AppInit` when `ParseArgs` returns false or when `ProcessInitCommands` returns true. `ProcessInitCommands` (lines 153-172) only handles `-help`/`-version`, so `dashd -printcrashinfo=...` proceeds through `AppInit(node)` into full node startup after printing the decoded crash info. Before the bitcoin#27708 split, the `-printcrashinfo` branch lived inside `AppInit` and returned `true`, and `main` skipped `WaitForShutdown` on that path; the split moved the branch to `ParseArgs` without preserving the exit semantics. The Dash-only clause needs its own early-exit path — either return a sentinel that `main` treats as EXIT_SUCCESS, or handle `-printcrashinfo` in `ProcessInitCommands` alongside help/version. The same split also moves help/version behind datadir/config/settings initialization; the pre-backport code handled those informational commands before datadir validation, which is a secondary behavioral regression worth revisiting as part of the fix.
- [BLOCKING] src/bitcoind.cpp:126-142: Missing prerequisite: bitcoin#27150 common::InitConfig for bitcoin#27708
Upstream bitcoin#27708 delegates datadir/config/settings setup to `common::InitConfig(args)`, which centralizes error handling and exit semantics. Dash substitutes the older local sequence — `CheckDataDirOption()`, `ReadConfigFiles()`, `SelectParams()`, `InitSettings()` — because `common::InitConfig` is absent (grep for `common::InitConfig` returns no hits). The helper and supporting `common/init.{h,cpp}` split were introduced by bitcoin#27150. For a full advertised backport of bitcoin#27708, this is a missing prerequisite rather than a faithful application of the upstream starting state. Either backport bitcoin#27150 (and the short chain leading to it) so `ParseArgs` can use `common::InitConfig`, or document the omission and its scope in the commit/PR body.
In `test/functional/p2p_leak_tx.py`:
- [BLOCKING] test/functional/p2p_leak_tx.py:7-8: Missing prerequisite: bitcoin#18044 for bitcoin#27625 wtxid coverage
Upstream bitcoin#27625 modifies `p2p_leak_tx.py` to import `MSG_WTX` and `P2PTxInvStore`, adds `test_notfound_on_replaced_tx()`, and requests both txid and wtxid inventory for the replaced transaction. The Dash cherry-pick only adds `self.nodes[0].disconnect_p2ps()` to the older txid-only test and omits the wtxid coverage entirely because Dash's node lacks wtxid relay (bitcoin#18044). Even though `MSG_WTX` and `P2PTxInvStore` exist in Dash's `test_framework/p2p.py`, the omitted test path exercises wtxid `notfound` delivery from the node, which requires the wtxid-relay backport. Since the PR advertises a full backport of bitcoin#27625, this is a missing prerequisite: either backport bitcoin#18044's wtxid-relay path or document the intentional exclusion in the commit/PR body.
In `test/lint/lint-includes.py`:
- [BLOCKING] test/lint/lint-includes.py:26-41: Missing prerequisite: bitcoin#19988 txrequest.cpp hunk for bitcoin#27783
Upstream bitcoin#27783 adds explicit public Boost headers (including `boost/tuple/tuple.hpp`) in `src/txrequest.cpp` and updates the `lint-includes.py` `EXPECTED_BOOST_INCLUDES` allowlist accordingly. Dash has no `src/txrequest.cpp` (confirmed absent by `ls`), so the cherry-pick omits both the source hunk and the matching lint-allowlist entry. The missing module was introduced by bitcoin#19988 (Overhaul transaction request logic). For a full advertised backport of bitcoin#27783 this is a missing prerequisite: either backport bitcoin#19988's txrequest overhaul, or explicitly note in the commit/PR body that the txrequest.cpp hunk is intentionally excluded because the module does not exist in Dash.
In `src/wallet/test/walletdb_tests.cpp`:
- [BLOCKING] src/wallet/test/walletdb_tests.cpp:30: Missing prerequisite: bitcoin#27556/#26715 wallet test hunks for bitcoin#27783
Upstream bitcoin#27783 changes `BOOST_ASSERT` uses in `walletdb_read_write_deadlock` and `wallet_load_ckey`. In this Dash tree, `walletdb_tests.cpp` lacks `walletdb_read_write_deadlock` entirely, and `src/wallet/test/walletload_tests.cpp` is absent (confirmed by `ls`), so both upstream hunks were omitted. Bitcoin history shows `walletdb_read_write_deadlock` was introduced by bitcoin#27556, and the `wallet_load_ckey` shape touched by #27783 depends on the MockableDatabase wallet-test work from bitcoin#26715. Under the full-backport completeness rule these are missing prerequisites: either backport the wallet-test prerequisites, or document that these `BOOST_ASSERT` hunks are intentionally excluded because the receiving tests do not yet exist in Dash.
In `<commit:ddf030f>`:
- [SUGGESTION] <commit:ddf030f>:1: Drop empty CI-retrigger commit before merge
Commit `ddf030f4c5e` ("ci: retrigger linux64_ubsan-test after feature_governance flake") introduces zero tree changes — `git show --stat` confirms no files touched. Because Dash Core merges without squashing, this empty commit will land in permanent `develop` history where it becomes noise for `git log`, `git bisect`, and `git blame`. CI reruns should be triggered from the CI UI or a comment trigger rather than by pushing an empty commit. Please drop this commit before merge so the PR tip is the actual backport `5f660ae6`. If a marker is needed for reviewer visibility, use a PR comment instead.
Note: GitHub does not allow me to submit REQUEST_CHANGES on my own PR, so this is posted as a COMMENT while preserving the verified blocking findings.
thepastaclaw
force-pushed
the
backport-0.26-batch-3-staging
branch
from
ddf030f to
a1f31c5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1f31c56cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| node.exit_status = EXIT_FAILURE; | ||
| } | ||
| Interrupt(node); | ||
| Shutdown(node); |
There was a problem hiding this comment.
When AppInit() returns false before AppInitMain() runs—for example AppInitLockDataDirectory() fails because another dashd already owns the datadir—this unconditional Shutdown() now runs anyway. Shutdown() removes GetPidFile(*node.args) in src/init.cpp, but in that lock-failure path this process never created its own pid file because CreatePidFile() is inside AppInitMain(), so a failed second start can delete the running daemon's dashd.pid; the old early-return path did not call Shutdown() for these pre-main failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
Source: reviewers claude/general=opus(completed), codex/general=gpt-5.5(failed), claude/dash-core-commit-history=opus(completed), codex/dash-core-commit-history=gpt-5.5(failed), claude/backport-reviewer=opus(completed), codex/backport-reviewer=gpt-5.5(failed); verifier=opus.
Cumulative review at head a1f31c5. Carried-forward prior findings: none — all 7 prior findings from ddf030f are reconciled (2 FIXED for the -printcrashinfo relocation and commit-hygiene cleanup, 4 INTENTIONALLY DEFERRED for missing prerequisites now explicitly documented in both PR description and per-commit bodies, 1 FIXED for the dropped empty CI-retrigger commit). New findings in latest delta: none — the ddf030f..a1f31c5 delta only relocates the -printcrashinfo handler from ParseArgs() into ProcessInitCommands() so main() returns EXIT_SUCCESS before AppInit(), and wires InitShutdownState() into the standalone dash-chainstate binary so AbortNode()'s Assert(g_exit_status) has a valid target. Both changes were verified against the checked-out worktree (src/bitcoind.cpp:150-153,288 and src/bitcoin-chainstate.cpp:62-69,308). CodeRabbit has no actionable inline findings.
Prior finding reconciliation
- FIXED — -printcrashinfo no longer exits after printing: src/bitcoind.cpp:150-153 relocates the -printcrashinfo handler into ProcessInitCommands(); src/bitcoind.cpp:288 short-circuits main() with
return EXIT_SUCCESSwhen ProcessInitCommands() returns true. - INTENTIONALLY DEFERRED — Missing prerequisite: bitcoin#18044 for bitcoin#27625 wtxid coverage: Both the PR description under
Intentional exclusions / Dash adaptationsand commit 00d6eee's body explicitly document that upstreamtest_notfound_on_replaced_tx()and the MSG_WTX/P2PTxInvStore imports are omitted because Dash disables RBF and has not backported wtxid relay (bitcoin#18044). - INTENTIONALLY DEFERRED — Missing prerequisite: bitcoin#27150 common::InitConfig for bitcoin#27708: PR description and commit 8d9ba7d body both explicitly state that
common::InitConfig()comes from bitcoin#27150, which is not backported, so ParseArgs() (src/bitcoind.cpp:112-146) keeps Dash's existing CheckDataDirOption()/ReadConfigFiles()/SelectParams()/InitSettings() sequence with unchanged semantics. - INTENTIONALLY DEFERRED — Missing prerequisite: bitcoin#19988 txrequest.cpp hunk for bitcoin#27783: Commit a1f31c5 body explicitly documents omission of the src/txrequest.cpp hunk and the boost/tuple/tuple.hpp lint-includes.py allowlist entry because Dash has no txrequest module (bitcoin#19988 not backported) and adding the allowlist entry with no user would fail lint-includes.py's unused-dependency check.
- INTENTIONALLY DEFERRED — Missing prerequisite: bitcoin#27556/bitcoin#26715 wallet test hunks for bitcoin#27783: Commit a1f31c5 body explicitly excludes the walletdb_read_write_deadlock and wallet_load_ckey hunks because the receiving tests do not exist in Dash (bitcoin#27556 and the bitcoin#26715 wallet-test rework not backported); the coinselector_tests.cpp BOOST_ASSERT line touched upstream also does not exist in Dash.
- FIXED — Drop empty CI-retrigger commit before merge:
git log develop..a1f31c56shows the branch was rebuilt with new SHAs (e9f53a6, 7d29963, 00d6eee, 8d9ba7d, 58cf2cd, 7370b87, a1f31c5). - FIXED — Remove transient recovery note from commit message: The prior 6be2980 commit was replaced during rebase by 8d9ba7d.
Posting note: review_poster could not post inline/top-level review cleanly; posting exact-SHA body-only review from the same verifier output.
Verified Findings
No active findings.
|
This pull request has conflicts, please rebase. |
Drop the now-unused CGovernanceManager stand-in and align the LoadChainstate/VerifyLoadedChainstate call sites with their current signatures (mn_sync in, is_addrindex/is_spentindex/is_timeindex out) so the dash-chainstate binary compiles again after the batch 3 backports.
Backport of bitcoin#27778 Automated backport - cherry-pick of f08bde7.
Backport of bitcoin#27625 Automated backport — cherry-pick of `fbe48f97df`. Excluded upstream hunks (intentional): - test/functional/p2p_leak_tx.py: upstream's new test_notfound_on_replaced_tx() and the MSG_WTX/P2PTxInvStore imports are omitted. The test replaces a mempool transaction and then requests it by txid and wtxid; Dash supports neither mempool replacement (RBF is disabled) nor wtxid relay (bitcoin#18044 is not backported), so the scenario cannot be exercised. The disconnect_p2ps() hardening for the existing txid-only test is kept.
thepastaclaw
force-pushed
the
backport-0.26-batch-3-staging
branch
from
a1f31c5 to
f8000e2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8000e2d7f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (duplicate_coinbase_height == ActiveHeight()) { | ||
| // we mined the duplicate coinbase | ||
| assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); |
There was a problem hiding this comment.
When the fuzzer has already reached duplicate_coinbase_height, any later invalid block leaves ActiveHeight() unchanged at that same value, so this moved assertion checks the next block template's coinbase script against the earlier duplicate height and can abort even though no duplicate coinbase was just mined. This creates false fuzz crashes for inputs that make the block after the duplicate height invalid; keep the check under was_valid or compare against the attempted block height instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d0e40990488 by backporting the upstream bitcoin#27810 follow-up immediately after bitcoin#27780. The duplicate-coinbase assertion is now evaluated only when was_valid, matching upstream. The rewritten stack also adds the independently reviewed bitcoin#27170 prerequisite for bitcoin#27150; exact-head prerequisite review passed at 3dc9d29f285.
…ystem.cpp Backport of bitcoin#27170 (merge `8303f11e10`). Squashes the two upstream topic commits in order: - `b20b34f5b3` refactor: use ArgsManager::GetConfigFilePath() - `9a9d5da11f` refactor: Stop using gArgs global in system.cpp Dash adaptation: - Pass the current ArgsManager to Dash’s existing empty dash.conf creation path after changing GetConfigFile() to require an explicit ArgsManager. No upstream hunks are excluded; upstream bitcoin#27170 added no tests.
Backport of bitcoin#27150 (merge `fc037c8c83`). Squashes the four upstream topic commits in order: - `c361df90b9` scripted-diff: Remove double newlines after some init errors - `3db2874bd7` Extend bilingual_str support for tinyformat - `d172b5c671` Add InitError(error, details) overload - `802cc1ef53` Deduplicate bitcoind and bitcoin-qt init code Dash adaptations: - Keep Dash CoreContext construction and -printcrashinfo handling in AppInit, and retain dashd/dash-qt/dash.conf naming in user-facing text. - Apply the InitError fatal-index change to Dash current BaseIndex::FatalErrorImpl implementation. - Qualify nested tfm::format() calls because Dash builds as C++20, where upstream unqualified calls collide with std::format through ADL. This was fixed later upstream by `fa8ef7d138`. - Register src/common/init.cpp with IWYU in ci/dash/lint-tidy.sh; upstream ci/test/06_script_b.sh was removed when Dash migrated CI. No source or test hunks are excluded. The AbortError-to-InitError shutdown change is retained here with its upstream owner, d172b5c.
… errors Backport of bitcoin#27708 Automated backport — cherry-pick of `c92fd63886`. Prerequisite: - bitcoin#27150 is backported in the immediately preceding commit, so ParseArgs() now uses common::InitConfig() exactly as upstream intends. The AbortError-to-InitError conversion remains in that prerequisite with its upstream owner, d172b5c. Dash adaptations: - Move Dash-only -printcrashinfo handling into ProcessInitCommands() so it remains an early successful-return command alongside -help and -version. - Retain Dash CoreContext context{node}; in AppInit() for Dash subsystem lifetime management after upstream splits argument handling from startup. - Adapt the fatal-index change to Dash current BaseIndex::FatalErrorImpl(). - Keep Dash interfaces::Node and AppInitParameterInteraction API shapes while threading the new exit status through them. - Initialize shutdown state and return its status in the Dash-specific bitcoin-chainstate executable, which can also reach AbortNode(). Excluded upstream hunk (intentional): - src/qt/bitcoin.cpp does not add node/context.h because Dash reads the status through interfaces::Node::getExitStatus(), so the header would be unused.
Backport of bitcoin#27780 Automated backport — cherry-pick of `2a786ea349`.
Backport of bitcoin#27810 Automated backport — cherry-pick of `a1db4476bd`.
Backport of bitcoin#27783 Automated backport — cherry-pick of `361a0c00b3`. Excluded upstream hunks (intentional): - src/txrequest.cpp and the matching boost/tuple/tuple.hpp entry in test/lint/lint-includes.py: Dash has no txrequest module (bitcoin#19988 is not backported), and allowlisting the header with no remaining user would itself fail the lint's unused-dependency check. - src/node/miner.h, src/txmempool.h and the remaining lint-includes.py allowlist entries: the multi_index identity/indexed_by/tag includes and their allowlist entries are already present in Dash. - src/wallet/test/walletdb_tests.cpp (walletdb_read_write_deadlock) and src/wallet/test/walletload_tests.cpp (wallet_load_ckey): the receiving tests do not exist in Dash (bitcoin#27556 and the bitcoin#26715 wallet-test rework are not backported). - src/wallet/test/coinselector_tests.cpp: the BOOST_ASSERT line touched upstream does not exist in Dash's adapted max-weight test.
thepastaclaw
force-pushed
the
backport-0.26-batch-3-staging
branch
from
f8000e2 to
3dc9d29
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
The current macOS failure is CI infrastructure, not a #7433 source failure. The
|
✅ Action performedReviews resumed. |
1 participant
Batch 3 of Bitcoin Core v0.26 backports.
Dash support commit:
This keeps the experimental
dash-chainstatecall sites aligned with the current DashLoadChainstate/VerifyLoadedChainstatesignatures. The bitcoin#27708 backport also wiresInitShutdownState()and the recorded exit status into the standalone binary soAbortNode()can report fatal exits correctly.8 Bitcoin backports:
Intentional exclusions / Dash adaptations:
test_notfound_on_replaced_tx()and itsMSG_WTX/P2PTxInvStoreimports are omitted. The test needs mempool replacement (disabled in Dash) and wtxid relay (Use wtxid for transaction relay bitcoin/bitcoin#18044, not backported). Thedisconnect_p2ps()hardening of the existing txid-only test is retained.dash.confcreation path now passes the currentArgsManagertoGetConfigFile()after the upstream API is made explicit. No upstream hunks are excluded.-printcrashinfo, and Dash-specific binary/config names. Its sharedcommon::InitConfig()usesCheckDataDirOption(args)from the immediately preceding refactor: Stop using gArgs global in system.cpp bitcoin/bitcoin#27170 prerequisite. The fatal-index change is adapted to Dash's currentBaseIndex::FatalErrorImpl(). Nestedtfm::format()calls are qualified for Dash's C++20 build, matching the later upstream ADL fix.src/common/init.cppis registered in Dash's IWYU script because upstream's CI script no longer exists here.ParseArgs()usescommon::InitConfig()as upstream intends. Dash-only-printcrashinforemains an early successful-return command, and Dash's CoreContext lifetime handling is retained. Thedash-chainstateshutdown-state wiring is included in this commit.src/txrequest.cppand theboost/tuple/tuple.hpplint allowlist entry are omitted because Dash has no txrequest module (Overhaul transaction request logic bitcoin/bitcoin#19988 is not backported); adding the allowlist entry alone would fail the unused-dependency lint. Thewalletdb_read_write_deadlock/wallet_load_ckeyhunks are omitted because their receiving tests are absent (wallet: fix deadlock in bdb read write operation bitcoin/bitcoin#27556/IntroduceMockableDatabasefor wallet unit tests bitcoin/bitcoin#26715 are not backported). The miner.h/txmempool.h include hunks are already present in Dash.Validation on the current exact head:
git diff --check upstream/develop..HEADtest/lint/lint-includes.pytest/lint/lint-whitespace.pytest/lint/lint-format-strings.pyFresh GitHub CI is running on the updated head.
Automated backport via PastaClaw backport pipeline. Each Bitcoin backport was individually cherry-picked and reviewer-verified before batching.