Repository navigation
backport: v24.0.x backports for v24.0.0-rc.6 - #8013
Merged
PastaPastaPasta merged 11 commits intoNov 2, 2026
Merged
Conversation
…ts so 253+ input sends work 7f7b34f fix(wallet): pay for 3 byte input and output counts (pasta) Pull request description: ## Issue being fixed or feature implemented Since v24 (the bitcoin#25647 backport in dashpay#7550), sending fails with `Internal bug detected: Fee needed > fee paid` whenever the transaction spends 253 or more inputs and has a change output. A wallet with many small UTXOs can't pay an amount that needs 253+ of them. The same error happens when paying exactly 252 recipients with change (253 outputs). CoinJoin's `CTransactionBuilder` (up to 500 outputs, preselected inputs) can hit both cases. v23 is not affected. Before bitcoin#25647 the change amount was set from the measured signed size, so estimation errors were absorbed. Since then it comes from the coin selection estimate (`SelectionResult::GetChange()`). That estimate assumes a 1 byte input count, and `change_output_size` ignores that adding change can grow the output count. Why upstream Bitcoin Core does not trip on it. Upstream makes exactly the same two assumptions, and even documents one of them in the GetChange() comment ("input counter size is always assumed to be 1vbyte"). It survives only because of segwit-related overestimates that Dash removed along with segwit: - Upstream sizes every input with tx_is_segwit=true hardcoded in CalculateMaximumSignedInputSize, so each legacy input gets a phantom witness-stack byte. That rounds up to a full extra vbyte per input. - Per-input weight is converted to vsize with ceil, so each P2WPKH or P2TR input overestimates by 0.25 vbyte. - The fixed header counts 1 vbyte for marker and flag, which actually cost 0.5. At 253 inputs that slack is dozens of vbytes, far more than the 2 missing for the CompactSize. Dash sizes byte-exact, so the shortfall lands exactly on the 2 bytes PR 7990 describes. ## What was done? - `tx_noinputs_size` sizes the input count CompactSize for every coin selection could pick (preselected + available). This is an upper bound, so the selection target can only overestimate. With change, the existing "increase the change to match the fee needed" step returns the difference. The `not_input_fees`/`selection_target` computation moves below the coin fetch for this; error precedence is unchanged. - `change_output_size` includes the output count CompactSize growth when change is added (252 -> 253 outputs). - Updated the `GetChange()` doc comment, which listed segwit factors that don't apply to Dash. One side effect: in wallets with 253+ spendable coins, changeless transactions pay `2 bytes * feerate` more (2 duffs at the default feerate). Transactions with change still pay the exact fee. ## How Has This Been Tested? Added `test_compact_size_counts` to `wallet_create_tx.py`: - Spend 253+ of 260 small UTXOs with change. - Pay 252 recipients plus change. Both check that the fee covers the transaction size. On `develop` and `v24.0.x` without the fix, both cases fail with `Fee needed > fee paid`, with `--descriptors` and `--legacy-wallet`. Both pass with the fix. The commit also cherry-picks cleanly onto `v24.0.x`, and the test passes there. Also ran the `coinselector_tests`, `spend_tests` and `wallet_tests` unit tests, plus the wallet, `rpc_fundrawtransaction`, `rpc_psbt`, `rpc_rawtransaction`, `mempool_*` and fee functional tests: 117 passed, 2 skipped (`mempool_compatibility.py` and `wallet_upgradewallet.py` need previous release binaries). ## Breaking Changes None. Targeted for v24.0.0 final. This is a user-visible regression from v23 in the core send path, and it will be backported to `v24.0.x` once merged here (label: `backport-candidate-24.0.x`). ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 041b6a6dd965f5d4f1546fd9cdd9f629c31ab1e6e92c0b27fa386c1a7f34aba3ab65f3f5935ce6e2779d51942f7e826d47863d2434143104a1df49284756cee6 (cherry picked from commit 0ad28ad)
ddcfbfc test: check that -whitelist does not apply to inbound onion connections (pasta) 3b4c5ab Merge bitcoin#33395: net: do not apply whitelist permissions to onion inbounds (pasta) d91e48b Merge bitcoin#27114: p2p: Allow whitelisting manual connections (pasta) Pull request description: ## Issue being fixed or feature implemented Peers that reach us through our onion service are forwarded by the local Tor daemon, so their source address is Tor's, usually `127.0.0.1`. Today `-whitelist` is matched against that address. With a common entry like `-whitelist=noban@127.0.0.1`, every Tor peer gets `noban` (and whatever other permissions the entry grants). This has been the case since onion binds were introduced, v23 included. This complements dashpay#8003. dashpay#8003 makes sure Tor only ever forwards to a listener that tags its peers as onion. This PR then stops address-based `-whitelist` permissions from applying to those peers. `-whitebind` is unaffected because it is per listener, and dashpay#8003 already avoids pointing Tor at our own `-whitebind`. ## What was done? Backports, in upstream order: - **bitcoin#27114** (prerequisite): `-whitelist` accepts `in` and `out` flags, so permissions can also apply to manual outbound connections (`-addnode`, `-connect`, `addnode onetry`); the default stays incoming only. Whitelist ranges are split into incoming and outgoing lists, and `-whitelistrelay`/`-whitelistforcerelay` are stored in `CConnman`. Tests that whitelisted peers to speed up relay use the new `noban_tx_relay` framework option. - Dash adaptations: test hunks for files Dash doesn't have (`p2p_feefilter.py`, `p2p_segwit.py`, `wallet_address_types.py`, `wallet_avoid_mixing_output_types.py`, `wallet_bumpfee.py`) are dropped; converted tests keep their Dash-specific arguments. Masternode, probe and CoinJoin connections are not `MANUAL`, so they never pick up the outgoing whitelist. - **bitcoin#33395**: `CreateNodeFromAcceptedSocket` determines `inbound_onion` first and applies no address-based whitelist permissions to onion inbounds. Applies 1:1 on top of bitcoin#27114. - A Dash test commit adds a check to `p2p_permissions.py`, since upstream has no test for bitcoin#33395. ## How Has This Been Tested? - New check in `p2p_permissions.py`: with `-whitelist=noban@127.0.0.1` and `-bind=127.0.0.1:<port>=onion`, a peer on the clearnet bind gets `noban, download` and a peer on the onion bind (`network=onion`) gets no permissions. Without the bitcoin#33395 commit it fails with the onion peer holding `noban, download`. - Full build including the fuzz harness; `test_dash --run_test=netbase_tests,net_tests,net_peer_eviction_tests`. - Every functional test touched by bitcoin#27114 (both wallet variants where applicable, `interface_zmq.py` included), plus `rpc_net.py`, `p2p_eviction.py`, `feature_proxy.py`, `feature_bind_extra.py`, `p2p_addr_relay.py`, pass on Linux x86_64. - Merges cleanly with dashpay#8003. ## Breaking Changes - Inbound onion peers no longer receive permissions from `-whitelist`, even if an entry matches the Tor daemon's address. Address-based whitelisting can't identify Tor peers, so this is intended. - `-whitelist=in,out@<addr>` without any permission is now a startup error (from bitcoin#27114). ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: PastaPastaPasta: utACK ddcfbfc Tree-SHA512: c4d5fe9a03e49d556110efff5dc8c84cf7dbd13410ee842cfa87bac3685c5ac539a7eedd5768dafb213f9168e777c8431996a1e46979d32316fe7993e90bd2dd [v24.0.x: wallet_import_rescan.py now restarts its pruned nodes with their real -prune arguments instead of only -whitelist. On this branch, which lacks dashpay#7860, pruned nodes still print the governance-disabled warning to stderr, so the test stops them with EXPECTED_STDERR_NO_GOV_PRUNE at the end of run_test.] (cherry picked from commit 6c9bfca)
…he consensus rules f916c5d refactor: check share amounts with one consensus helper in Qt and validation (pasta) 474d4da fix(qt): validate shared-masternode shares with the consensus rules (pasta) Pull request description: ## Issue being fixed or feature implemented The shared-masternode Qt editor kept its own hand-written copy of the consensus share-list rules (`IsShareListTriviallyValid`), and the copies had drifted. Consensus rejects a non-zero early-exit penalty when there is no early period (`bad-protx-shares-penalty`), but the GUI accepted it. `lockTerms` relies only on the GUI check, and a reopened session skips `validatePage`. So the user only found out from the node's rejection, after the co-owners had already coordinated. ## What was done? The GUI no longer carries its own copy of those rules. It calls the consensus function and maps the reject reason to a translated message. - `src/evo/providertx.cpp`: the penalty rules move into `IsEarlyPenaltyValid`, which `IsShareListTriviallyValid` calls as its last step. The count, per-share bound and sum rules move into `AreShareAmountsValid`, which `IsShareListTriviallyValid` calls first. Which lists are valid is unchanged. A list that breaks several rules at once can report an amount reason where it used to report the early-period or a key or script reason. - `MnShareSession::validateShares` now only parses the address strings, which consensus cannot see, and reports those errors per share. It then calls `IsShareListTriviallyValid` and turns its reject reason into a message (`ShareListErrorText`). The hand-written checks are gone. - The exit-terms page uses `IsEarlyPenaltyValid`, and the participants page, where no addresses exist yet, uses `AreShareAmountsValid`. Trade-off: a consensus rejection is now one message without the share number, where the old copy said which share was wrong. Parse errors still name the share. Out of scope, left as a follow-up: the reward-address-reuse check in the contribution page of `sharedmncreatedialog.cpp` and in `sharedmndialogs.cpp`. ## How Has This Been Tested? Built on Linux with Qt (`--enable-debug`). Ran `test_dash-qt` (`QT_QPA_PLATFORM=minimal`, all passing) and `test_dash --run_test='evo_*'`. `feature_masternode_shares.py` passes. `MnShareSessionTests::validateSharesMirror` gains rows for a penalty with no early period, for `CheckEarlyPenalty`, and for `CheckShareAmounts`. They check that validation fails or passes and do not pin message text. I disabled the `early_period_blocks == 0` rule in `IsEarlyPenaltyValid` and confirmed the new penalty row fails; with the rule restored it passes. ## Breaking Changes None. Which transactions are valid is unchanged; only GUI validation messages change, plus the reject reason for lists that break several rules at once. ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: cbce4d93d2e2b2ce373bcb34cba2cb1b3c04767ea8098c16940c70d003563da85f0dde656c5390feb374a6f47924537d734838ac05dc85c61e1039d398373894 (cherry picked from commit 608e98f)
…ng coins when needed 877b6cf fix(wallet): fill a CoinJoin promotion with sibling coins when needed (pasta) Pull request description: ## Issue being fixed or feature implemented After V24 activates, many wallets report a CoinJoin promotion opportunity (dashpay#7052) on every mixing attempt but can never carry it out. `CWallet::GetDenominationCounts()` counts every fully-mixed coin, but `CWallet::SelectFullyMixedForPromotion()` takes at most one coin per parent transaction. Standard mixing puts up to 9 of a wallet's coins in each mixing transaction, so a wallet's fully-mixed coins are naturally clustered a handful per transaction. Such a wallet can hold well over `PROMOTION_RATIO` fully-mixed coins of a denomination that come from fewer than 10 transactions. The client then: - logs "Promotion opportunity" on every attempt; - fails selection with "Not enough fully-mixed coins for promotion", twice per attempt; - skips the "Nothing to do" / "Not enough funds" exits it would otherwise take, re-scanning the wallet's coins on every attempt; - never promotes. I reproduced this on regtest: in a real-masternode run with promoters whose coins were clustered, there were about 55 opportunity log lines, about 110 selection failures, and no promotion. ## What was done? `SelectFullyMixedForPromotion()` still prefers coins from distinct transactions, but when there aren't enough of them it now fills the rest with siblings of coins it already took. Selection can now always return as many coins as `GetDenominationCounts()` reports, so a reported promotion can be built. The strict one-per-parent rule was copied from standard mixing (`SelectTxDSInsByDenomination`), where it protects something a promotion doesn't need: - A standard mixing output inherits its inputs' rounds plus one, so a round weakened by sibling inputs would still be counted as a full round. A promotion output starts mixing over at 0 rounds (`GetRealOutpointCoinJoinRounds`), so no round is credited. - Spending siblings together links them as one owner's outputs of their last mixing round. An ordinary send of the same coins reveals the same thing, and sends already allow it. - The old comment warned that a demotion's 10 sibling outputs must never be promoted together. They start at 0 rounds, so the fully-mixed requirement keeps them out of a promotion until they have been re-mixed, and standard mixing takes at most one of them per session, which spreads them over different mixing transactions. Preferring distinct parents keeps that hint to the wallets whose coins really are clustered. An earlier revision of this PR counted fully-mixed coins once per parent transaction instead. That made the opportunity disappear rather than letting it be carried out, so promotions would rarely trigger for real wallets. ## How Has This Been Tested? - Unit test `coinjoin_promotion_prefers_coins_from_distinct_txs_tests` mines coins through zero-fee self-mixing transactions until they are fully mixed regardless of the salt. - With a full entry of `COINJOIN_ENTRY_MAX_SIZE` (9) fully-mixed coins sharing one parent plus one coin of its own, it checks that the promotion is reported and that selection returns 10 coins: the lone coin and 9 siblings. - After adding 8 more coins each in its own transaction, it checks that selection returns 10 coins from 10 distinct parents, i.e. only one coin of the cluster. - Mutation checks, run locally: with develop's strict selection the first case fails (selection returns 2 coins, not 10). If siblings are allowed with no preference, the second case fails (6 and 7 distinct parents in two runs, not 10). - `coinjoin_tests`, `coinjoin_inouts_tests`, `coinjoin_queue_tests` and `coinjoin_dstxmanager_tests` pass (`--enable-debug --enable-werror` build). This makes promotions start in cases where they used to stall silently. Each one opens a promotion queue, so it should land together with dashpay#7995, which stops a waiting conversion queue from blocking standard mixing. ## Breaking Changes None. Client-side only. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 6aea0a63ddc2182dd5e5d2e084c148be407250e9b785ed36a8ef5bfb1b759063ab535f6724e18ec31c4aaac4fcfa3c1b0fd95334d95f048125c07a99e51472fd (cherry picked from commit 28c7c6a)
…on/demotion queue block standard mixing 63fc9f0 fix(coinjoin): don't let a waiting promotion/demotion queue block mixing (pasta) Pull request description: ## Issue being fixed or feature implemented After V24 activates, a wallet that can promote or demote a denomination (dashpay#7052) stops standard mixing for as long as that stays true. `DoAutomaticDenominating` tries the conversion first. When `StartNewQueue` opens a promotion/demotion queue it returns, without ever reaching the standard `JoinExistingQueue`/`StartNewQueue`. A conversion queue needs a counterpart: another wallet doing the same conversion, or v24 standard mixers at that denomination. v23 and other pre-70241 clients are turned away from such sessions. Until a counterpart shows up, the queue expires, the opportunity is still there, and the next attempt opens another lone conversion queue. The wallet's unmixed coins never get a turn, and every lone queue also spends one of the masternode's DSQ slots. I reproduced this on regtest with 6 masternodes, real sessions and a v24-activated chain. A wallet with 13 DASH left to mix and a 1.0 → 0.1 demotion opportunity opened 13 demotion queues, all of which expired. It made no standard attempt in about 25 minutes of mocktime. Meanwhile, a second wallet opened 24 standard 1.0 queues the first never joined. On v23 those two wallets would just have mixed together. ## What was done? The opportunity scan now records every promotion/demotion opportunity once, in the `cs_wallet` section, instead of recomputing them after the lock. The attempts then run in this order: 1. join an existing queue for each conversion; joining costs nothing and is what a waiting counterpart needs; 2. standard mixing, join then start, as before, while there is balance left to anonymize; 3. start a new conversion queue, only if neither of the above found anything. A wallet with nothing left to mix (target reached, or every coin already fully mixed) reaches step 3 exactly as before. A wallet that still has coins to mix keeps mixing them, and converts when it finds a queue to join or once its standard mixing is done. Pre-V24 behaviour is unchanged: no opportunities are collected before activation. Trade-off: while a wallet has coins to mix, it no longer opens conversion queues itself; it only joins them. That can delay a rebalance until its standard mixing runs out of coins to mix, but it can no longer stop mixing, which is the right way round. Joining in step 1 is unchanged: a `dsq` carries no direction, so it joins any queue at the conversion's denomination, exactly as before. ## How Has This Been Tested? - Regtest, with V24 active and real masternode sessions: a wallet holding fully-mixed 1.0 coins and no 0.1 coins (a demotion opportunity at goal 10), plus unmixed 0.01 coins. - With 0.01 coins still to mix, its first session is a standard 0.01 queue. Without the fix it is the 0.1 demotion queue. - With the target set below its anonymized balance, or with nothing ready to mix (the target is above its balance but the 0.01 coins are locked, which is the common case because the default `-coinjoinamount` exceeds most balances), its first session is still the 0.1 demotion queue. - This check was written as a functional test and left out of the PR on purpose. It fakes mixed coins with zero-fee self-spends and needs heavy setup, so CoinJoin functional tests should first get a proper harness of their own. - `rpc_coinjoin.py`, `p2p_dstx.py` and the `coinjoin_*` unit suites pass. - A local end-to-end test, not part of this PR, ran real masternode sessions before and after activation: promotion pair, lone promoter, demotion pair, then standard mixing. Before this fix it timed out at the final standard-mixing step. With this fix and the companion promotion-count fix, every phase passes: no unexpected collateral charges, and the standard mix completes. ## Breaking Changes None. Client-side only: no protocol, policy or consensus change. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 19cf248be5145ba9af5c9dc4bdfbe5382df9470d917cc58b21b5a26e2d13ddf90de8947f4269ee1be3b27098f98956b6599b95769345475d5cf6f588a5f78a12 (cherry picked from commit 669ccc9)
… shares, unconfirmed funding and reply handling e6b3e4a fix(qt): say exactly what a shared masternode approval covers on the sign page (pasta) 3d4f99b fix(qt): make the shared masternode broadcast details readable (pasta) bab1534 feat(qt): show the whole registration under the shared masternode broadcast details (pasta) f2d0c52 feat(qt): show the approved terms again on the shared masternode sign page (pasta) 7693caf feat(qt): show what this wallet pays on the shared masternode term sheet (pasta) 12f55fb fix(qt): pin the shared masternode page actions below the scrolled content (pasta) d416648 feat(qt): mark the shared masternode term sheet addresses this wallet holds (pasta) 02ee19e feat(qt): let a shared masternode participant copy its reply again while waiting (pasta) 799748e fix(qt): only treat a P2PKH owner address as this wallet's shared masternode share (pasta) f86671b fix(qt): let shared masternode shares be funded from unconfirmed coins the wallet trusts (pasta) 67925e2 fix(qt): clear the coordinator's stale "me" check when its share is given up (pasta) 3b747c3 fix(qt): keep the shared masternode coordinator off rows other participants took (pasta) 4cc96fb fix(qt): lock the shared masternode coordinator's own row once coins are reserved (pasta) 0171f8a fix(qt): refuse to reserve a shared masternode share for an owner address outside the wallet (pasta) 7355a71 fix(qt): mark shares other participants already took and reload addresses on share switch (pasta) Pull request description: ## Issue being fixed or feature implemented When a participant imports a shared masternode invitation in which another participant has already claimed a share, the "This is me" picker on the contribution page gives no sign that the share is taken. When several shares are still open, the wallet can't tell which one is the participant's, so none is chosen. Qt still shows the first entry as selected, so the coordinator's share looks like the participant's own. Choosing a taken share is worse than confusing. The page then treats the other participant's reservation as the user's own, and its Release button removes that reservation from the user's copy of the session. The same happens when a participant chooses an open share and, before reserving coins, imports an update in which somebody else has answered for it. Switching to another share also left the owner, refund and reward addresses of the previous share in the fields. ## What was done? - Shares whose owner address is not a P2PKH address this wallet can spend are labelled "(taken)" and cannot be selected. Reserving coins for one is refused. - When no share is known, the picker starts on a "Choose your share…" entry instead of showing the first share as chosen. - If the chosen share turns out to belong to someone else after an import, it is given up and the share is inferred again. - Selecting a share loads its recorded addresses. A field that still holds an address recorded for another share gets a fresh address from the wallet. - The picker is locked once coins are reserved, or once the session is past the draft stage. - Coins can be reserved from unconfirmed coins the wallet already trusts (InstantSend-locked, or the wallet's own unconfirmed change). Before, any coin without a confirmation was skipped, so a wallet showing enough available balance was told it did not have enough. Also found while testing on testnet: - Each waiting page has a "Copy My Reply" button. A participant's reply was copied only once, when the page opened, so a clipboard overwritten before the reply reached the coordinator left only "Save…" to recover it. - On the term sheet, each owner, refund, reward or voting address this wallet can spend is marked "✓ in this wallet", so a reader can confirm their own addresses before approving. - The term sheet's Funding section shows "Your contribution": what this wallet's coins among the inputs add up to, less the change coming back to it. If that is more than 1 DASH above the wallet's share, the sheet warns not to approve unless it was meant. Signing already refused this, but only after the approvals were collected. - The Sign contribution and Signatures pages show the term sheet again, with a note once this wallet has approved: the approval covers every amount, address, key and exit term shown, but not the participant names, "Node run by" or the Identity section. Signing is the step that moves coins, and its page showed only the status board. - "Show Details" on the broadcast confirmation now holds the transaction ID and the full term sheet. It held only the session and code before. The details box is tall enough to read without scrolling through a few lines at a time. - Copy Invitation, Copy Locked Terms, Unlock terms, Copy Signing Request and Copy Final Record are pinned under the scroll area. They sat after the full term sheet, so you had to scroll to reach the page's main action. ## How Has This Been Tested? - New test `SharedMnWizardTests::participantCannotChooseATakenShare` covers three cases: - a share another participant has taken is shown "(taken)" and disabled, and no share appears chosen while several are open; - after a chosen share is answered by someone else, this wallet no longer holds it or its reservation; - switching away from a share this wallet answered no longer carries that share's addresses along. - I removed each of the three fixes (taken marking with the placeholder, giving up a share someone else answered, reloading addresses) one at a time, and the test failed every time. - `SharedMnWalkthroughTests::walkthrough` now has Alice send all her coins to herself before reserving, so her share is funded from unconfirmed change all the way to broadcast. With the old confirmation rule put back, it fails with "This wallet needs at least 300 tDASH spendable to fund your 300 tDASH share." - The walkthrough also clears the clipboard on Alice's waiting page, clicks "Copy My Reply" and checks the clipboard holds the reply she first copied. - `SharedMnWidgetTests::termSheetEscapesUntrustedText` checks that only the address the wallet holds is marked, and that nothing is marked without a wallet. - `SharedMnWidgetTests::termSheetWarnsWhenWePayMoreThanOurShare` checks the line shows only for a share of our own, warns only above 1 DASH over the share, and warns when some inputs cannot be valued yet but the ones that can already exceed it. With the warning removed, it fails. - The walkthrough checks that each participant's sign page shows the note and a term sheet with their own owner address and "Your contribution". Without the sheet on that page, it fails. - `refusesToSignCoinsOutsideOwnContribution` and `refusesToSignShortChangedOwnContribution` check the wallet's own view of the funding: a wallet coin under another contribution counts as paid, change sent elsewhere is not subtracted, the honest case comes to exactly the share, and a short-changed one shows the warning. Both fail when that calculation returns nothing. - The walkthrough checks that the broadcast confirmation's details include the transaction ID and every share's owner address. - I looked at the pinned buttons and the markers in the walkthrough's rendered screenshots. - The full `test_dash-qt` suite passes on macOS. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: ac46532b723cf4c0ac741bc06320fa8165205b3fc00d1da26f4feba93192736d8e840187109c13b5fc4be5d74a8dee031e3694d62ebdc2d1214e138f157b67b7 (cherry picked from commit 540c934)
Fold doc/release-notes-27114.md (from dashpay#8005) into the consolidated release notes and remove it, and describe dashpay#8005's onion-inbound whitelist change and the CoinJoin conversion behaviour from dashpay#7995 and dashpay#7996. The other rc.6 picks fix code that is new in v24 and need no release note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated with contrib/devtools/gen-manpages.py from a build of this branch with CLIENT_VERSION_IS_RELEASE set temporarily (not committed), so every page reads v24.0.0. They pick up the -whitelist, -whitelistrelay, -whitelistforcerelay and -blocksonly help changes from dashpay#8005, and the -assumevalid defaults from dashpay#7980, which merged after the v24.0.0 man pages were generated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
|
✅ Final review complete — no blockers (commit 914c052) · triage: critical |
…ashpay#7966 Pull requests into v24.0.x run develop's workflow files (pull_request_target). Since dashpay#7966, develop runs cppcheck in a separate job that calls test/lint/cppcheck-dash.py, so on this branch, where the script is still test/lint/lint-cppcheck-dash.py, that job fails with "No such file or directory". Take only dashpay#7966's rename (the script is unchanged). all-lint.py runs only lint-*.py, so the rename also stops develop's lint job from running cppcheck a second time. This branch's own lint workflow now calls the renamed script right after ci/dash/lint.sh, so its push CI keeps running cppcheck. The rest of dashpay#7966 (cppcheck build changes in the ci-slim image, the separate job) is left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![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: c97f3b3b4c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
thepastaclaw
approved these changes
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
No actionable backport-resolution or Dash-integration defects were found: all six develop cherry-picks preserve their source changes except for the documented pruning-stderr adaptation, and the release preparation and cppcheck rename are consistent with the final tree. Current-head lint and cppcheck pass; “Cache depends sources” failed when the miniupnpc download connection was reset and its fallback returned HTTP 404, an unrelated dependency-hosting failure that caused build/test jobs to be skipped. This was a static review; no local builds or tests were run.
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, cross-cutting diff changes coin selection and transaction fee sizing in src/wallet/spend.cpp::CreateTransactionInternal and promotion input selection in src/wallet/coinjoin.cpp::SelectFullyMixedForPromotion, meeting the critical-surface bar. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer
thepastaclaw
added
the
pastaclaw:approved
…c onion service unless -listenonion=1 8274f68 fix(net): restore the default onion bind as a best-effort listener (pasta) f2b78d3 fix(net): don't refuse to start over the automatic onion service unless -listenonion=1 (pasta) Pull request description: ## Issue being fixed or feature implemented Since dashpay#7786 (shipped to v24.0.x for rc.3 via dashpay#7808), a node with `-bind` but no `-bind=<addr>:<port>=onion` refuses to start while `-listenonion` is enabled. `-listenonion` is on by default, so this breaks every node with `bind=` and no Tor that worked on v23: dashmate evonodes, docker setups, hosts with several masternodes. It also hits 54 of DCG's 88 testnet hosts. The startup errors came from a partial backport of bitcoin#36170, which is still open upstream; released Bitcoin Core does not refuse to start here. Breakage matrix, develop / v24.0.0-rc.5 vs v23: | | Setup | rc.5 | v23 | |---|---|---|---| | S1 | `-bind`, no Tor, default `-listenonion` | refuses to start | starts | | S2 | `-bind`, zero-config Tor, default `-listenonion` | refuses to start | starts, onion service on the dedicated `127.0.0.1:9996` listener | | S3 | wildcard `-bind=0.0.0.0:P=onion`, `-listenonion` effective | refuses to start (kept by this PR) | starts, but Tor was pointed at a wildcard, which never worked, and Tor peers reaching it were treated as clearnet | | S4 | no `-bind`, default onion port already taken (e.g. a second instance with `-port`) | refuses to start, even with `-listenonion=0` | starts with a warning | The privacy fix from dashpay#7786 has to stay: Tor must never be pointed at a listener that isn't tagged as onion. Otherwise Tor-forwarded peers are treated as clearnet peers and are told the node's clearnet address. Before dashpay#7786, dashpay#7300 (backport of bitcoin#22729) caused exactly that by targeting the first `-bind`. The rule this PR implements: only an explicit `-listenonion=1` may turn a missing or unusable onion listener into a startup failure. The default `-listenonion` means "use Tor if it is there" (bitcoin#6639). The one exception is a wildcard `=onion` bind (S3). Writing `=onion` is explicit Tor intent, and a wildcard listener can't honor it: an accepted socket reports its concrete local address, which never matches the wildcard entry in `m_onion_binds`, so peers that Tor forwards there would be treated as clearnet and could be told our clearnet address. So while `-listenonion` is effective, default or explicit, any wildcard `=onion` bind, including one next to a specific bind, is a startup error with the existing message, as on rc.5 and in bitcoin#36170. With `-listenonion=0` it is accepted, as in v23 and released Bitcoin Core. Setups without Tor don't write `=onion`, so S1 always starts. ## What was done? Two commits. The second can be dropped on its own if reviewers prefer. **1. `fix(net): don't refuse to start over the automatic onion service unless -listenonion=1`** (fixes S1 and S2 at startup) - The `TorController` target is now `std::optional<CService>`. Without a target, Tor control still authenticates and runs `GETINFO net/listeners/socks` (outbound onion proxy discovery), but skips `ADD_ONION`. Once authenticated, it logs once that no onion service is created because there is no dedicated onion bind. Without Tor, the control-port connect just fails, as on v23. - In init, with `-listenonion` effective: - any wildcard onion bind: the existing wildcard `InitError`, default or explicit; - no onion bind and explicit `-listenonion=1`: the existing "requires a dedicated onion bind" `InitError`; - no onion bind and the default `-listenonion`: Tor control starts without a target. **2. `fix(net): restore the default onion bind as a best-effort listener`** (fixes S2 and S4 fully) - The default onion bind `127.0.0.1:<OnionServiceTargetPort>` is added again when there is no `=onion` bind and `-listenonion` is effective, even with `-bind`, as on v23 (the rule from dashpay#7786's first two commits). It is still added when there are no binds at all, for manual-torrc setups. With `-bind` and `-listenonion=0` it is not added. - Failing to bind it only logs a warning; explicit `-bind`, `-whitebind` and `=onion` failures stay fatal. With an explicit `-listenonion=1`, the default onion bind is required and its failure is fatal. That happens inside `InitBinds`, before network threads start. - A failed default onion bind is dropped from `CConnman`'s onion binds. Tor control now starts after `connman->Start()` and gets the target only if `CConnman::HasOnionBind()` still has it; otherwise it runs without a target. This avoids v23's misroute, where Tor forwarded to whatever held the port: another instance, or the node's own `-bind`/`-whitebind` on that address. Nothing between the old and new `StartTorControl` call sites depends on Tor control; onion proxy discovery was already asynchronous. - `p2p_permissions.py` gives its whitebind-only node a per-node onion bind, and the test framework's `p2p_port()`, `rpc_port()` and `tor_port()` never return the fixed regtest onion port `19896` (it is moved to the last port of its range, which no seed uses). `feature_bind_extra.py` and `interface_zmq.py` now get each extra port from `p2p_port()` instead of adding to one. That makes `feature_bind_extra.py` the only test binding `19896`, for every port seed and in parallel `test_runner.py` runs. Docs: the `-bind` and `-listenonion` help, `doc/release-notes-7300.md` (the v24 note that said such nodes "now refuse to start") and `doc/tor.md` describe the new behavior. Translated strings are unchanged. Commit 2 no longer uses the "requires a dedicated onion bind" message. Related: dashpay#7786, dashpay#7300, dashpay#7808, bitcoin#36170 (still open upstream), bitcoin#22729. ## How Has This Been Tested? Linux x86_64, `--enable-debug` build. - `feature_bind_extra.py`, which checks the node's actual listening sockets. Node 2 (`-bind` without an onion bind) is restarted with the framework's `listenonion=0` removed, so it runs with the default `-listenonion`: - commit 1: it starts and listens only on its `-bind`; - commit 2: it starts and also listens on the default onion target `127.0.0.1:19896`; - commit 2, with `127.0.0.1:19896` held by another socket: it still starts and listens only on its `-bind`, while an explicit `-listenonion=1` fails with the bind error; - a wildcard onion bind with the default `-listenonion` is the wildcard startup error, and with `-listenonion=0` it starts; - node 2's initial start sets `-listenonion=0` explicitly in commit 2, which checks that no default onion bind is added in that case. - Ran with both commits: `feature_bind_extra.py feature_proxy.py p2p_permissions.py interface_zmq.py rpc_bind.py feature_discover.py`, all pass (`rpc_bind.py --ipv6` skips on this host), also with `--portseed=8428` and in parallel with `TEST_RUNNER_PORT_MIN=19870`. Earlier revisions with the same `src/` also passed `rpc_net.py p2p_eviction.py feature_config_args.py p2p_addr_relay.py`, `test_dash --run_test=torcontrol_tests,net_tests,net_peer_eviction_tests`, `lint-whitespace.py` and `lint-python.py`. `check-doc.py` reports undocumented `-statsport` and `-printcrashinfo`, identically on develop. - Mutation checks, each failing `feature_bind_extra.py`: - develop's `src/`: node 2 refuses to start with the default `-listenonion`; - commit 1's `src/` under commit 2's test: no listener on `127.0.0.1:19896`; - the default onion bind made fatal again: node 2 doesn't start while the port is held; - the wildcard check removed: the node starts with a wildcard onion bind. - Commit 1 alone builds, and `feature_bind_extra.py feature_proxy.py p2p_permissions.py` pass at that commit. - Not covered by a functional test: which listener the automatic onion service is told to forward to (that would need a fake Tor control port), and that peers on our own non-onion listener at `127.0.0.1:19896` aren't treated as onion peers. Both were checked during development with a test that is no longer part of the PR. ## Breaking Changes Compared with v24.0.0-rc.1 to rc.5, none. Nodes that failed to start now start. Compared with v23: - a wildcard `=onion` bind is a startup error unless `-listenonion=0` is set; - if the default onion bind fails, no onion service is created, rather than pointing Tor at whatever holds the port; - an explicit `-listenonion=1` without a usable onion bind is a startup error; - `-bind` with `-listenonion=0` no longer binds the default onion port (from dashpay#7300). ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 5218897911c4dd8a1c4f7b5d0228f869f6ad654cfd8fd06a97462e82ffd2a3e52ba192acfda75dfe04f7b1f56c4b8f985d021df0df3766c9b1d1bce46a341865 [v24.0.x: doc/release-notes-7300.md was folded into doc/release-notes.md for v24.0.0 (dashpay#7970), so its hunk is applied there in the next commit.] (cherry picked from commit 92f2bfb)
Apply dashpay#8003's doc/release-notes-7300.md change to the consolidated release notes: the implicit onion bind is best effort again unless -listenonion=1 is explicit, -bind without an onion bind no longer refuses to start, and a wildcard onion bind is an error while -listenonion is enabled. Regenerate dashd.1 and dash-qt.1 for the new -bind and -listenonion help, from a build with CLIENT_VERSION_IS_RELEASE set temporarily (not committed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
removed
the
pastaclaw:approved
thepastaclaw
approved these changes
Nov 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The complete backport and release-preparation diff preserves the recorded develop changes, apart from the documented pruning-stderr adaptation and release-note consolidation; no actionable in-scope defects were found. Verification was static only, with no local builds or tests run. Current CI reports no failures: lint, cppcheck, multiple platform builds and Linux test jobs passed, while the Windows and multiprocess builds and ASan/TSan tests remain pending.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, multi-path change directly alters funds selection and transaction fee calculation in src/wallet/spend.cpp::CreateTransactionInternal and promotion input selection in src/wallet/coinjoin.cpp::CWallet::SelectFullyMixedForPromotion. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
3 tasks done
thepastaclaw
removed
the
pastaclaw:approved
3 tasks done
PastaPastaPasta
added a commit
that referenced
this pull request
Nov 4, 2026
2bf133c docs: cite #8042 for the index fixes in the v24.0.0 release notes (pasta) 8aa11f6 docs: add the v24.0.0-rc.7 picks and index fixes to the v24.0.0 release notes (pasta) ad4e5d6 docs: drop the index caveat from the -reindex-chainstate man page entry (pasta) 19a90bb test: blocks orphaned while the address index is disabled are removed from it (pasta) d2f2a1f fix(index): don't adopt an empty or rebuilt async index as migrated legacy data (pasta) e3bafdb fix(index): reject an uncommitted tip the block index lost (pasta) 9198962 fix(index): check the blocks governance resumes from at startup (pasta) baff347 fix(index): resume from the tip a skipped commit leaves behind (pasta) 2dbf5ca fix(index): write a rewound block's erasures in one batch with its locator (pasta) 252acbe fix(index): move the committed locator below the entries a rewind erases (Konstantin Akimov) e9f9c8a Merge bitcoin#36150: indexes: set prune lock to genesis before first block (Konstantin Akimov) 2edeb98 partial Merge bitcoin#34897: indexes: Don't commit ahead of the flushed chainstate (Konstantin Akimov) 3eba84c Merge bitcoin#32875: index: handle case where pindex_prev equals chain tip in NextSyncBlock() (Konstantin Akimov) 399125c Merge bitcoin#32882: index: remove unnecessary locator cleaning in BaseIndex::Init() (Konstantin Akimov) bb724c6 Merge bitcoin#33212: index: Don't commit state in BaseIndex::Rewind (Konstantin Akimov) d5ff431 Merge bitcoin#32878: index: fix wrong assert of current_tip == m_best_block_index (Konstantin Akimov) 31c7d8f partial Merge bitcoin#32580: wallet, test: best block locator matches scan state follow-ups (Konstantin Akimov) 94f5c58 partial Merge bitcoin#30221: wallet: Ensure best block matches wallet scan state (Konstantin Akimov) 8e0b1e8 partial Merge bitcoin#32069: test: fix intermittent failure in wallet_reorgsrestore.py (Konstantin Akimov) 4d17947 Merge bitcoin#25193: indexes: Read the locator's top block during init, allow interaction with reindex-chainstate (Konstantin Akimov) 0127df8 Merge #8045: test: scale the mempool-loaded wait by timeout_factor so TSAN reindex restarts stop timing out (pasta) 62d0cef Merge #8036: fix(qt): let a shared masternode participant follow a revised invitation that keeps their share (merge-script) 775d31c Merge #8018: fix(consensus): require IPv4 masternode primaries until IPv4 is optional, pin mainnet ports (merge-script) e1b826e Merge #8026: fix(qt): keep shared masternode contribution fields locked while coins are reserved (merge-script) 9fa3df3 Merge #8020: fix(rpc): make protx shared_sign sign only dissolutions that return every principal (merge-script) 246a465 Merge #8024: fix(qt): keep the shared masternode dissolve dialog consistent with what it will send (merge-script) 590ad0a Merge #8021: fix(qt): don't load a newer shared masternode session copy that changes your own share (merge-script) 974aaa3 Merge #8034: fix(qt): check a shared masternode's service and refund addresses before coins are reserved (merge-script) 0026e3e Merge #8027: fix(qt): fund a shared masternode's service update from this wallet by default (merge-script) 6a5d2dc Merge #8035: fix(qt): don't strand coins reserved for a shared masternode session (merge-script) df6083a Merge #8032: fix(qt): don't offer a new operator key when reopening a shared masternode session (merge-script) 2b5bf30 Merge #8029: fix: let dashd -version, -help and first start work without an existing data directory (merge-script) e33d9b1 Merge #8037: docs: list the config file path each Linux init system uses (merge-script) 6030ea0 docs: add the #8014 migratewallet fixes to the v24.0.0 release notes (pasta) 1b1ea95 Merge #8014: backport: bitcoin#28609, bitcoin#34215, bitcoin#34226, bitcoin#34370, partials bitcoin#31423, bitcoin#34156 (migratewallet fixes from 28.x) (merge-script) 608ff63 Merge bitcoin#28542: wallet: Check for uninitialized last processed and conflicting heights in MarkConflicted (DCG-Claude) 3af6f63 Merge bitcoin#28038: wallet: address book migration bug fixes (DCG-Claude) 2fe41fb docs: add ecosystem notes on Asset Unlock txids, coinbase payees and RPC errors (pasta) 0e3f653 docs: correct operator guidance in the v24.0.0 release notes for rc.7 (pasta) 11c8140 Merge #8019: fix(qt): stop reporting refused shared masternode broadcasts as sent (merge-script) Pull request description: ## Issue being fixed or feature implemented Backports and release prep for v24.0.0-rc.7, on top of `v24.0.x` at 8d195f1 (v24.0.0-rc.6, #8013). This PR collects the develop merges labeled `backport-candidate-24.0.x`, the v24.0.x port of #8042 with the index fixes from #8033, and the release notes for them, so rc.7 has a single backport PR to merge. ## What was done? ### Picked Cherry-picked with `git cherry-pick -m1 -x`, in develop merge order, except that bitcoin#28038 goes before bitcoin#28542 as in Bitcoin Core. Each pick changes exactly the same lines as its develop merge, except as noted below for bitcoin#28542, #8014, #8020 and #8018. - #8019 fix(qt): stop reporting refused shared masternode broadcasts as sent - bitcoin#28038 (from #7863): prerequisite of #8014 - bitcoin#28542 (from #7762): prerequisite of #8014. Its `test_conflict_txs` migrates through `self.migrate_wallet`, as on develop since bitcoin#28868. - #8014 backport: bitcoin#28609, bitcoin#34215, bitcoin#34226, bitcoin#34370, partials bitcoin#31423, bitcoin#34156 (migratewallet fixes from 28.x), with the Dash adaptations below - #8037 docs: list the config file path each Linux init system uses - #8029 fix: let dashd -version, -help and first start work without an existing data directory - #8032 fix(qt): don't offer a new operator key when reopening a shared masternode session - #8035 fix(qt): don't strand coins reserved for a shared masternode session - #8027 fix(qt): fund a shared masternode's service update from this wallet by default - #8034 fix(qt): check a shared masternode's service and refund addresses before coins are reserved - #8021 fix(qt): don't load a newer shared masternode session copy that changes your own share - #8024 fix(qt): keep the shared masternode dissolve dialog consistent with what it will send - #8020 fix(rpc): make protx shared_sign sign only dissolutions that return every principal. Its `doc/release-notes-7437.md` sentence goes into the same paragraph of `doc/release-notes.md`, since that file was folded in for v24.0.0. - #8026 fix(qt): keep shared masternode contribution fields locked while coins are reserved (merged on develop as cc73a04; GitHub shows the PR as closed) - #8018 fix(consensus): require IPv4 masternode primaries until IPv4 is optional, pin mainnet ports, with the adaptation below - #8036 fix(qt): let a shared masternode participant follow a revised invitation that keeps their share - #8045 test: scale the mempool-loaded wait by timeout_factor so TSAN reindex restarts stop timing out ### Index fixes: #8042 port and #8033 The index commits are #8042 (merge 9e5e3d5), ported commit by commit to `v24.0.x`, plus the parts of #8033 that #8042 doesn't cover. #8042's v24.0.x port replaces #8033's bitcoin#33212, bitcoin#34897 and rewind-locator commits, and #8033's flushed-block announcement fix (e6a687b) is dropped: bitcoin#30221 removes the wallet's `ChainStateFlushed` locator write, and nothing else on `v24.0.x` consumes that notification. This PR replaces #8033, and #8031 comes in through it (adapted to the `v24.0.x` `CCoinsViewDB` constructor), not as a separate pick. - bitcoin#25193 (from #7883, via #8033): indexes: read the locator's top block during init, allow interaction with `-reindex-chainstate` - From #8042: partial bitcoin#32069 (only `TestNode.kill_process()`, with develop's `wait_until_stopped(**kwargs)` handling it needs), partial bitcoin#30221, partial bitcoin#32580, bitcoin#32878, bitcoin#33212, bitcoin#32882, bitcoin#32875, partial bitcoin#34897, bitcoin#36150, and #8042's own fixes: move the committed locator below the entries a rewind erases; write a rewound block's erasures in one batch with its locator; resume from the tip a skipped commit leaves behind; check the blocks an index resumes from at startup; reject an uncommitted tip the block index lost - From #8033: #8031 fix(index): don't adopt an empty or rebuilt async index as migrated legacy data; test: blocks orphaned while the address index is disabled are removed from it; docs: drop the index caveat from the `-reindex-chainstate` man page entry #### Dash adaptations for #8042 - The governance collateral index doesn't exist on `v24.0.x`, so its hunks are dropped; from the startup resume check only the `base.{h,cpp}` part is kept. - `feature_pruning.py`: node 5 syncs before node 0's pruned restart, and `test_rescan_blockchain` expects the governance-disabled stderr a pruned node prints on `v24.0.x`. - The wallet's `blockConnected` has no birthday skip on `v24.0.x`. - `TestNode.wait_until_stopped()` keeps `v24.0.x`'s `None` default for a clean exit. Apart from these, the `v24.0.x` index, validation and index test sources differ from develop after #8042 only by differences that already existed before it. #### Dash adaptations for #8018 `v24.0.x` predates bitcoin#26177, so it has no `src/kernel/chainparams.cpp` and no `GetBuriedDeployment()`. The four `IPv4OptionalHeight` values (mainnet not scheduled, testnet 0, devnet 0, regtest only through `-testactivationheight=ipv4_optional@<height>`) go into the matching classes in `src/chainparams.cpp`, and `ipv4_optional` is parsed in its `MaybeUpdateHeights()`. The `GetBuriedDeployment()` hunk is dropped; the `DeploymentName()` case is kept. Every other file, including all tests, changes exactly the same lines as the develop merge. The same notes are in the pick's commit message. #### Dash adaptations for #8014 #8014 doesn't cherry-pick cleanly onto `v24.0.x`. The conflicts were resolved against `v24.0.x` APIs. No prerequisite was backported, and no hunk or test was dropped. The same notes are in the pick's commit message. - `src/wallet/bdb.{cpp,h}`, `dump.cpp`, `sqlite.{cpp,h}`, `transaction.{cpp,h}`, `wallet.cpp`: the changed lines are identical to the develop merge. In `wallet.cpp`, the conflict was with develop-only context (`CanGrindR`), which stays out. - `src/wallet/db.h`: #8014 makes `WalletDatabase::Files()` pure virtual. `v24.0.x` still defines `DummyDatabase` in `db.h`, so it gets the `Files()` override there. - `src/wallet/test/wallet_tests.cpp`: the test database mock on `v24.0.x` is `FailDatabase`, so it gets the same `Files()` override. `src/wallet/salvage.cpp` and `src/wallet/test/util.h` keep the `v24.0.x` side, since develop's `DummyDatabase`/`MockableDatabase` placement doesn't exist here. - `test/functional/tool_wallet.py`, `wallet_backup.py`, `wallet_migration.py`: the test framework on `v24.0.x` has no `TestNode.wallets_path`/`datadir_path` (bitcoin#27884), so the new tests build these paths with `os.path.join` and pass RPC paths as strings. `wallet_migration.py` takes only #8014's new tests, not the develop-only tests around them. - `test/functional/wallet_migration.py`: `unsynced_wallet_on_pruned_node_fails` stops its pruned node with `EXPECTED_STDERR_NO_GOV_PRUNE`, because on `v24.0.x` a pruned node still prints the governance-disabled warning. The rc.6 picks handled the same warning the same way. ### Separate v24.0.x PRs, merged separately - #8016 backport: v23.1.10 fixes for v24.0.x - #8022 backport: bind CoinJoin signatures to the approved transaction (#7815) for v24.0.x, which depends on #7815 merging on develop #8016 and #8022 each merge cleanly with this branch in either order, and all three merge to the same tree in any order. ### Not picked - #8025: closed; superseded by #8014 (picked above) - #8031 is not picked separately; it comes through #8033's commits above. - #8039, #8040, #8041, #8043, #8044 (CI and tooling), #7905, follow-ups to #8042, and the PRs in the 24.1 milestone. - Explicitly deferred: #7870, #7679, #7675, #7220, #7217, #7109, #6988, #4974, #8028. ### Release notes `doc/release-notes.md` corrections, checked against the code on this branch: - **Version 2 Asset Unlocks:** version 1 Asset Unlocks stay valid after `v24` activation. Platform withdrawals keep using version 1 until a future Dash Platform release issues version 2, so the stable txid, InstantSend locking and instance-hash relay don't apply to them yet. - **Masternode requirements:** the startup refusals (#7754) and the DKG bad votes for missing transaction relay or compact filters (#7755) apply from the first start of v24.0.0, not from activation. Operators should remove conflicting settings and upgrade before the `v24` start date (November 9, 2026), so their masternodes are running when the activation signal is signed. `-peerblockfilters=0` is the new refusal; `-blockfilterindex=0` already failed on v23. - **How to Upgrade:** a pruned node with `-addressindex` or `-spentindex` now refuses to start; v23 accepted that combination. - **Downgrading:** there is no supported downgrade after `v24` activation: older versions can't follow the network, and they misread the masternode list data v24.0.0 writes. Before activation, downgrading to v23.1.x works with the listed caveats. Dash-Qt's dust-protection setting is migrated to the shared settings, so it reads as disabled after a downgrade. - **Dash Platform:** Platform/dashmate v5.0.0 and later handle the hidden `service` and `softforks` RPC fields. Older Platform versions need both `-deprecatedrpc` options. - **Compatibility:** users on macOS 13 or older must upgrade macOS, or move their node, before activation. - **Legacy wallets:** in v24, `createwallet` with `descriptors=false` also needs `load_on_startup`; before v24 this applied only to `descriptors=true`. The notes now give a working example. - **Wallet migration (#8014):** failure handling in `migratewallet`, `restorewallet` and `dash-wallet createfromdump` is fixed, and the new backup and watch-only/solvables wallet names are listed. - **Ecosystem notes:** the txid of a version 2 Asset Unlock zeroes `requestedHeight`, `quorumHash` and `quorumSig` before hashing (the full-bytes hash is `instanceHash`); pools should emit one coinbase output per `masternode[]` and `superblock[]` template entry, since merged or deduplicated payees are invalid after activation; client libraries that send numbers or booleans as strings fail (#7099); and some wallet RPC error messages changed with upstream backports. - **rc.7 picks:** `doc/release-notes-8018.md` and `doc/release-notes-8029.md` are folded in and removed. #8018's IPv4-primary and mainnet standard-port rules are under Extended addresses, with a pointer from the masternode upgrade notes; #8029's `dash.conf` change is under Updated settings. The other rc.7 picks fix code that is new in v24 and need no release note. - **Index fixes (#8042, #8031):** nodes that ran a release candidate with the address, spent or timestamp index should rebuild them once (with `-reindex`, or by starting once with them disabled, deleting their `indexes/` directories and starting with them enabled). The notes also cover `-reindex-chainstate` with indexes enabled, older versions refusing a migrated index until `-reindex`, the new "the block the index was last synced to is not found. Please rebuild the index." startup error, and the known limitation shared with earlier versions in the remaining cases. - All rc.7 TODO comments are resolved. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the version string comes from the `v24.0.0-rc.7` tag. The man pages carry #8033's `-reindex-chainstate` change; no other pick changes non-debug help text (#8018's `-testactivationheight` change is debug-only). ## How Has This Been Tested? On this head (2bf133c): - Every develop pick compared with its develop merge (`git patch-id --stable` and the set of changed lines). #8019, #8037, #8032, #8035, #8027, #8034, #8021, #8024, #8026, #8036 and #8045: identical. #8029: the same changed lines; only context differs in `feature_config_args.py`. #8020: the same changed lines, in `doc/release-notes.md` instead of `doc/release-notes-7437.md`. #8018: identical except the adaptation above, checked file by file against 900f085 and reviewed separately for consensus equivalence on mainnet, testnet, devnet and regtest. #8014 and its prerequisites: as in the earlier rounds (adaptations above). - #8042 port: `v24.0.x`'s divergence from develop, before #8042 (058b060) versus after it (9e5e3d5), is unchanged in `src/index/*`, `src/validation.{h,cpp}`, the index unit tests and `feature_coinstatsindex.py`, apart from the adaptations listed above. The #8031, test and man page commits have the same patch-id and message as on #8033. - Built with `--enable-debug --enable-werror` and Qt: no warnings. - Unit suites pass: `baseindex_tests`, `index_migration_tests`, `txindex_tests`, `blockfilter_index_tests`, `coinstatsindex_tests`, `validation_flush_tests`, `wallet_tests`, `interfaces_tests`, `evo_netinfo_tests` and `evo_snapshot_tests`. With the develop picks (7275005), every `evo_*` suite and `test_dash-qt` (15 classes, 152 passed, 0 failed) passed too. - Functional tests pass: `rpc_netinfo.py`, `feature_masternode_shares.py`, `wallet_migration.py`, `feature_coinstatsindex.py` (including the unclean restart after a reorg), `feature_addressindex.py`, `feature_config_args.py` and `feature_index_prune.py`. On the same C++ tree, these pass as well: `feature_spentindex.py`, `feature_timestampindex.py`, `feature_pruning.py`, `feature_txindex.py`, `feature_reindex.py`, `feature_init.py`, `feature_abortnode.py` and `wallet_reorgsrestore.py`. - Regtest upgrade gates, run on a build of this exact head: all pass. They cover v23.1.10 index migration, index contents vs v23 and `-reindex`, a kill mid-migration, 9 wallets, a pre-activation downgrade and round trip, and v22.1.3/v23.0.2 upgrades. The crash-after-reorg probe and the deleted-index probe are both required to be not reproduced: 0/10 stale (rc.6: about 4/10), and the plain-delete rebuild matches. - Man pages: `help2man` output of the built `dashd`, `dash-cli`, `dash-tx`, `dash-wallet` and `dash-util` matched `doc/man` on the develop picks (apart from the debug-build-only `lock` log category). No later commit changes non-debug help text, so nothing is regenerated beyond #8033's `-reindex-chainstate` change. - `git merge-tree` against #8016 (cdb250c) and #8022 (62609bf), heads fetched 05:23 UTC: clean in both orders, and all three merge to the same tree in every order. From the earlier rounds: the wallet suites and functional tests listed for #8014, bitcoin#28038 and bitcoin#28542 passed, and the `createwallet` example was checked against the official v24.0.0-rc.6 binary. ## Breaking Changes - From #8014: migration backups are now named `<name>_<time>.legacy.bak`, the unnamed wallet's watch-only and solvables wallets are named `default_wallet_watchonly` and `default_wallet_solvables`, and `restorewallet` accepts an existing empty directory. - From #8018: on mainnet, a version 3 ProRegTx or ProUpServTx must list an IPv4 address on the standard port as the first entry of each address list; testnet and devnets are unaffected. - From #8029: `dashd` no longer creates an empty `dash.conf`. - From #8031: older versions refuse an index migrated by v24.0.0 until they run `-reindex`; nodes that ran a release candidate should rebuild these indexes once. - From #8042: an index whose last synced block was lost in an unclean shutdown makes startup fail with a rebuild instruction. The release notes describe these. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 3c405ef570fc1c5b46d1ba8bb2e344213cf65018abe5ef4b94b7cf2b9aa7160ba2675f71a0a365c72bc8249a2f9a5defe774af2a4e33537245aeb420c5f66711
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Backports and release prep for v24.0.0-rc.6, on top of
v24.0.xat d693c69 (the v24.0.0-rc.5 tag 8163290 plus the 2026-10 translations, #7989). After this PR, rc.6 only needs tagging and building.What was done?
Cherry-picked (
git cherry-pick -m1 -x) every develop merge since rc.5 that carriesbackport-candidate-24.0.x, in develop merge order:Every pick changes exactly the same lines as its develop merge, except for one test-only adaptation in #8005, described in its commit message, and #8003's
doc/release-notes-7300.mdhunk, which lands indoc/release-notes.md(that file was folded in for v24.0.0 by #7970). bitcoin#27114 makeswallet_import_rescan.pyrestart its pruned nodes with their real-prunearguments; before, the restart passed only-whitelist, so they never actually ran pruned. On develop, #7860 removed the governance-disabled warning that pruned nodes print to stderr. This branch doesn't have #7860, so the test failed at shutdown on the unexpected stderr. The pick now stops the pruned nodes at the end ofrun_testwithEXPECTED_STDERR_NO_GOV_PRUNE, the wayfeature_pruning.pydoes.Release prep, in commits after the picks:
doc/release-notes.md:doc/release-notes-27114.mdis folded in and removed, as rc.5 did with the other individual notes (docs: add v24.0.0 release notes and archive v23.1.8 and v23.1.9 #7970). Also added: backport: bitcoin#27114, bitcoin#33395 #8005's change that address-based-whitelistpermissions no longer apply to onion inbound peers, and the CoinJoin conversion behaviour from fix(wallet): don't let a waiting CoinJoin promotion/demotion queue block standard mixing #7995 and fix(wallet): fill a CoinJoin promotion with sibling coins when needed #7996. fix(wallet): pay for 3 byte input and output counts so 253+ input sends work #7990, fix(qt): validate shared-masternode shares with the consensus rules #7957 and fix(qt): shared masternode wizard fixes for taken shares, unconfirmed funding and reply handling #7991 fix code that is new in v24, so they get no release note.-bindand no onion bind "refuse to start", in a separate docs commit after the fix(net): don't refuse to start over the automatic onion service unless -listenonion=1 #8003 pick that also regeneratesdashd.1anddash-qt.1for its-bind/-listenonionhelp.doc/man/*.1: regenerated withcontrib/devtools/gen-manpages.pyfrom a build of this branch withCLIENT_VERSION_IS_RELEASEset temporarily (not committed), as in chore: update man pages, translation strings, SECURITY.md and flatpak for v24.0.0 #7974. Every page readsv24.0.0. They pick up backport: bitcoin#27114, bitcoin#33395 #8005's-whitelist,-whitelistrelay,-whitelistforcerelayand-blocksonlyhelp changes, and the-assumevaliddefaults from chore: update seeds, checkpoints, minimum chain work, assumevalid and tx stats for v24 #7980, which merged after the v24.0.0 man pages were generated.One CI-only commit follows the first six picks. Pull requests into
v24.0.xrun develop's workflow files (pull_request_target), and since #7966 develop runs cppcheck in its own job astest/lint/cppcheck-dash.py. This branch still hadtest/lint/lint-cppcheck-dash.py, so that job failed with "No such file or directory". The commit takes only #7966's rename (the script is unchanged), which also keeps develop's lint job from running cppcheck a second time throughall-lint.py. This branch's ownlint.ymlnow calls the renamed script afterci/dash/lint.sh, so its push CI still runs cppcheck. The rest of #7966 (the ci-slim cppcheck build and the separate job) is left out.Nothing else needs to change for the tag:
configure.acis already 24.0.0 withCLIENT_VERSION_IS_RELEASEfalse, so the build takes its version string from thev24.0.0-rc.6tag.SECURITY.mdand the flatpak entry were done in backport: update man pages, translation strings, SECURITY.md and flatpak for v24.0.0 (#7974) #7972.How Has This Been Tested?
Linux x86_64 against depends,
--enable-debug --enable-werror --disable-hardening.dashd,dash-qt,test_dashandtest_dash-qtbuild cleanly.make check: all unit suites pass, andtest_dash-qtpasses (run withQT_QPA_PLATFORM=minimal).feature_cltv.py,feature_csv_activation.py,feature_dersig.py,feature_fee_estimation.py,interface_rest.py,interface_zmq.py,mempool_packages.py,p2p_filter.py,p2p_invalid_block.py,p2p_permissions.py,rpc_packages.py,rpc_rawtransaction.py,wallet_abandonconflict.py,wallet_avoidreuse.py,wallet_backup.py,wallet_balance.py,wallet_basic.py,wallet_create_tx.py,wallet_fundrawtransaction.py,wallet_groups.py,wallet_hd.py,wallet_import_rescan.py,wallet_importdescriptors.py,wallet_listreceivedby.py,wallet_listsinceblock.py,wallet_listtransactions.py,wallet_send.py), plusfeature_bind_extra.py,feature_masternode_shares.pyandrpc_coinjoin.py.feature_bind_extra.py,feature_proxy.py,interface_zmq.py,p2p_permissions.pyandtorcontrol_testspass. fix(net): don't refuse to start over the automatic onion service unless -listenonion=1 #8003 changes the framework's port allocation, so the full non-extended functional suite was run as well: 313 passed, 12 skipped, 0 failed (test_runner.py -j12on a RAM disk).wallet_import_rescan.py --legacy-walletfailed before the adaptation above and passes with it.doc/man/*.1readsv24.0.0with no rc, git hash or-dirtysuffix.Breaking Changes
Carried over from the picked PRs:
-whitelistpermissions no longer apply to peers that connect through the onion service. Before, an entry such as-whitelist=noban@127.0.0.1gave every Tor peer its permissions.-whitelistalso acceptsin/outflags, so its permissions can apply to manual outbound connections; the default is still incoming only.-bindand no=onionbind start again, as on v23, and get the default onion bind unless-listenonion=0is set. If the default onion bind fails, the node starts without an onion service, unless-listenonion=1was set explicitly. A wildcard=onionbind is still an error while-listenonionis enabled; v23 accepted it.Checklist:
🤖 Generated with Claude Code