Repository navigation
backport: v24.0.x backports for v24.0.0-rc.4 - #7917
Merged
PastaPastaPasta merged 40 commits intoOct 17, 2026
Merged
Conversation
…on recipients b0f8630 fix: restore HTML escaping of send confirmation recipients (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented A payment URI can supply a label containing HTML. The send confirmation inserts that label, along with the amount, wallet name and address, into rich text, so tags such as `<br>` change what the user sees while reviewing the recipient. Dash used to escape the label and wallet name here. [Bitcoin Core bitcoin#16826](bitcoin#16826) was already backported as d7281c8 (2021), but that merge did not take the `sendcoinsdialog.cpp` hunks, because Dash's older escaping was still in place at the time. The [bitcoin#15886](bitcoin#15886) backport, a081afd (2022), then rewrote the confirmation text and dropped both `GUIUtil::HtmlEscape(rcp.label)` and `GUIUtil::HtmlEscape(model->getWalletName())`. Since then neither has been escaped. ## What was done? This is a fix that restores the lost escaping. It is not a new backport of bitcoin#16826. Each recipient line is HTML-escaped just before it is joined into the rich-text confirmation. That covers the `sendcoinsdialog.cpp` hunks of both upstream commits from bitcoin#16826: the label escape from [2c530ea](bitcoin@2c530ea) and the wallet-name escape from [1770a97](bitcoin@1770a972d471). The plain-text "Show Details" view is built from the same list before escaping and stays unchanged, so labels with ampersands, angle brackets, quotes or entity text still show literally there. A regression was added to the existing funded-wallet GUI test. It drives labels, including the URI-supplied label `<br>Forged recipient`, through the real send dialog, checks the rendered confirmation text, and cancels the confirmation before the existing send and transaction-list checks run. ## How Has This Been Tested? On macOS arm64 with prebuilt depends and Qt 5.15: - Built `qt/dash-qt` and `qt/test/test_dash-qt`. - `QT_QPA_PLATFORM=cocoa src/qt/test/test_dash-qt` passed in full, exit status 0, including `WalletTests`. The `minimal` platform skips `WalletTests` on macOS. - With the new regression and the `sendcoinsdialog.cpp` change reverted, `WalletTests::walletTests()` failed. The rendered-text check fails for the markup label, and the test then hangs until the Qt test timeout. ## Breaking Changes None. Labels and wallet names now display literally in the rich-text confirmation, as before the bitcoin#15886 backport. Stored labels, transaction recipients, amounts and send behavior are unchanged. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation (not applicable to this display fix) - [ ] I have assigned this pull request to a milestone (maintainer decision) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: f1490fff7d2b2355135cb041f2c36fb3d54b439af64de82a9e6527b566d8a3654f55f76aca400936d48a463174a2588899d0f72b00fa0207311bce9584c02723 (cherry picked from commit e07b557)
…level service field 2fb1e39 fix(rpc): correct masternode status help for top-level service field (thephez) Pull request description: ## Issue being fixed or feature implemented The `masternode status` help says the top-level `service` field is deprecated, optional, and only returned when `-deprecatedrpc=service` is passed. In fact the RPC returns it every time. This key reports the host node's own address, so it was deliberately left out of the `service` deprecation (see `rpc_netinfo.py`). The help text was taken from the shared deprecated entry, which only applies to the nested `dmnState.service` key. ## What was done? - Replaced the shared `GetRpcResult("service", /*optional=*/true)` lookup in `masternode status` with a dedicated, non-optional result entry: "IP address and port of this masternode". - The nested `dmnState.service` key and the shared help entry used by other RPCs are unchanged and still gated by `-deprecatedrpc=service`. ## How Has This Been Tested? Help-text-only change. `rpc_netinfo.py` already asserts that the top-level `service` key is always present in `masternode status` output, which matches the field now being non-optional. ## Breaking Changes None. RPC output is unchanged; only the help text differs. ## Checklist: - [ ] 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 _(for repository code-owners and collaborators only)_ Top commit has no ACKs. Tree-SHA512: a18b253e6ccc67b54b77805396b53b4ced59c1ce7722c5fef9d96d2ee3b6a749173e205b42bd2bcb4dbee6b44f68d7c02fc322b91e82efecbdd00307e7fbfa71 (cherry picked from commit 2340e3a)
3ef0555 fix(evo): check credit pool amount accumulation (pasta) Pull request description: ## Issue being fixed or feature implemented Credit-pool deposit accounting uses unchecked signed addition. `ConnectBlock` runs `ProcessSpecialTxsInBlock` (and with it the credit-pool diff) before `CheckTxInputs`, so asset-lock amounts are accumulated before their funding is checked. A block that stacks enough unfunded `MAX_MONEY` asset locks overflows `sessionLocked`, which is signed-overflow undefined behaviour and terminates trapping builds. Checking representability before mutation turns this into a normal validation failure and keeps correct accounting for representable deposit/withdrawal flows. ## What was done? Use the existing `CheckedAdd` helper for session deposit and withdrawal amounts and the withdrawal output sum. Assign session amounts only after the candidate sum passes. Calculate the final balance with checked arithmetic, subtracting withdrawals before adding deposits and the Platform reward. An unrepresentable final balance throws through the existing validation/mining error paths. The `CAmount` getter API, withdrawal limits and atomic package rollback remain intact. No aggregate or final `MAX_MONEY` rule was added, and the final net balance is checked after processing rather than after each deposit. ## How Has This Been Tested? - Local arm64 macOS build using prebuilt depends: `make -C src dashd test/test_dash` passed. - `./src/test/test_dash --run_test=evo_assetlocks_tests` passed. The new `credit_pool_lock_arithmetic` case accumulates individually in-range deposits to the `CAmount` boundary through the public `ProcessLockUnlockTransaction` API and checks that the next deposit is rejected and leaves the diff unchanged. The existing suite still covers ordinary asset locks and package rollback. The fixture exercises the public bookkeeping interface; it is not a funded-block proof, and full-node exploitability was not reproduced. ## Breaking Changes Successful representable accounting retains its mathematical result. Unrepresentable sums now report an error. No monetary cap or activation rule was added. ## 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 ACKs for top commit: knst: utACK 3ef0555 Tree-SHA512: 461cbd9f11ea078159b1e8166fcb511920b937590dcc83c8b4ccc48db009a34cc975b3fd431920038561cc793242756f9d93dc20ab60c3be079e7f7ceb58947c (cherry picked from commit 21ea7ba)
…rors e5fe71d fix: omit recovery words from invalid mnemonic errors (pasta) Pull request description: ## Issue being fixed or feature implemented Invalid-mnemonic errors included the submitted recovery words. The legacy wallet startup path forwards those errors to stderr and debug.log, exposing recovery material in diagnostics. ## What was done? Remove the words from the two existing error messages. Mnemonic validation, seed derivation and error propagation are unchanged. ## How Has This Been Tested? `make -j1`, the existing `bip39_tests` and `wallet_crypto_tests`, and diff, whitespace and include checks pass. The production edits are identical to the previously validated version: a standalone before/after check with a synthetic public mnemonic confirmed redaction in legacy startup stderr/debug.log and descriptor RPC errors. No test is added by this PR. ## Breaking Changes Invalid-mnemonic errors no longer echo recovery words. ## Checklist: - [x] I have performed a self-review of my own code - [x] Documentation is not needed for this diagnostic change - [ ] I have assigned this pull request to a milestone This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: b4aa992abcf5d79f04da63cda5297160dac8fccc01bb4cb3b7fee9259158b43f8e66f24062e35cdaffe1a5bdb5ad70432eb791357ab3fc51a98fcb48f7cadecf (cherry picked from commit e075859)
7a9d1b0 fix: reject malformed detached quorum share batches (pasta) Pull request description: ## Issue being fixed or feature implemented When a detached sig share's lazy BLS signature fails to deserialize, `NetSigning::ProcessPendingSigShares` bans the source and stops collecting its shares, but never adds it to `batchVerifier.badSources`. The dispatch loop then passes that source's whole vector to `CSigSharesManager::ProcessPendingSigShares`, including the malformed share and every unverified share after it. That method assumes its input is already verified, so unverified shares can reach the local share manager and interfere with recovery. ## What was done? In the malformed-signature branch, replace `BanNode(nodeId)` with `batchVerifier.badSources.emplace(nodeId)`. The existing dispatch loop already bans every node in `badSources` once (which also re-requests its shares) and skips its vector, so the malformed source's shares are no longer dispatched. Valid sources go through the same verification path as before. dashpay#7840 changes the same `ProcessPendingSigShares` block (per-signature BLS authentication) and should be rebased over whichever of the two merges first. ## How Has This Been Tested? Verified by inspection and build. No regression test is added, because the one-line change is self-evident from the diff: `badSources` now drives the existing ban-and-skip dispatch loop, and `CBLSBatchVerifier::Verify()` only adds to `badSources` and never clears it. - macOS arm64: `make -C src dashd test/test_dash dash-cli` built cleanly. - `./src/test/test_dash --run_test=llmq_utils_tests` passed. - `test/functional/test_runner.py feature_llmq_signing.py "feature_llmq_signing.py --spork21"` passed. ## Breaking Changes None to consensus rules, historical commitment validation, wire formats, or valid signatures. Detached batches from a source with a malformed share are now dropped instead of dispatched. ## 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 No user-facing documentation change is needed for this internal validation repair. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 26cfc448c36e93973224b4cad31b24bfee9341e3632cb73ebda5270f3a55bc0591bd2e5e919869c6793029ed8d89ea713b6099b6a0e1bbf8d876dc44a770e79c (cherry picked from commit 063aee1)
…nputs 696c0c0 fix(rpc): handle unavailable masternode payment inputs (pasta) Pull request description: ## Issue being fixed or feature implemented Source inspection shows that `masternode payments` dereferences a nullable previous-transaction lookup while calculating historical block fees. Querying a spending block when historical input data is unavailable can terminate the node. Returning a normal RPC error before the dereference preserves node availability and successful fee calculations. ## What was done? Check the transaction lookup and output index before reading the input amount. Missing transaction data returns `RPC_MISC_ERROR`; an unavailable referenced output returns `RPC_INTERNAL_ERROR`. Queries that do not need historical inputs continue to work without the index, and indexed fee and payment calculations remain unchanged. ## How Has This Been Tested? - `test/functional/test_runner.py rpc_masternode.py` passed on arm64 macOS. It restarts a node with `-txindex=0` and queries `masternode payments` for a block that spends a non-coinbase input; the RPC returns error -1 (`previous transaction not found`) and the node stays responsive. - The pre-fix crash was not reproduced; the out-of-range output check is covered by review only. ## Breaking Changes No changes to successful RPC results or consensus behavior. Missing input data now returns a normal RPC error. ## 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 Top commit has no ACKs. Tree-SHA512: d85a94f78763e41d8be33db80e64045d50a01cd02d32da8fd000a9339d8b79708c92613ed714ffc4e48b5a1c6b69b8e126cad9535b05dc0f149337257fb6667c (cherry picked from commit a94bafc)
86ba039 fix(wallet): authenticate unlock deadline extensions (pasta) Pull request description: ## Issue being fixed or feature implemented `CWallet::Unlock(SecureString)` returned `true` without checking the passphrase whenever the encrypted wallet was already fully unlocked. `walletpassphrase` treats that result as authentication and replaces the relock deadline and timer. So while a wallet is fully unlocked, a caller allowed to use `walletpassphrase` could keep it unlocked past the owner's deadline with any nonempty wrong passphrase. Spending or exporting keys after the original deadline still needs those RPC permissions too. A locked or mixing-only wallet cannot be unlocked this way, and RPC authentication is not bypassed. The early return is Dash-specific (added in 78c5ca5, blocked in 0.12.1, reintroduced by ff5540b in 0.16.0.0). Bitcoin Core decrypts the master key on every `Unlock`. ## What was done? - Removed the early return. Every request on an encrypted wallet now derives the key from the passphrase and decrypts the master key, as Bitcoin Core does. - When a master key is already held (full or mixing-only unlock), the decrypted candidate must match it, compared with `TimingResistantEqual`. A mismatch moves on to the next master-key entry. - This matters because the already-unlocked path skips `CheckDecryptionKey`, and a blank wallet passes that check trivially. Without the comparison, a wrong passphrase that happens to produce valid padding would still be accepted. - An authenticated request on a fully unlocked wallet is a no-op success. The metadata and descriptor-cache upgrades are not repeated. - The mixing-only → full upgrade path is unchanged, and so are the RPC timer logic, full → mixing rejection and the unencrypted-wallet result. ## How Has This Been Tested? macOS arm64, depends build. - **Regression test.** `wallet_encryption.py` now ends with one deterministic check under the mocktime the test already sets: with the wallet fully unlocked for 100 seconds, `walletpassphrase` with a wrong passphrase and a 1000-second timeout fails with `-14`, and `unlocked_until` does not change. Against an unpatched `wallet.cpp` the check fails ("No exception raised"). With the fix it passes. - `wallet_encryption.py --legacy-wallet` and `wallet_encryption.py --descriptors` both pass. - `test_dash --run_test=bip39_tests,wallet_crypto_tests` passes, and `lint-python.py` is clean. - All `Unlock(SecureString)` callers were checked: Qt unlock dialog and wallet controller, createwallet, EncryptWallet, migration and upgradetohd. Limits: - The valid-padding mismatch guard was reviewed in the source, not fault-injected. A wrong passphrase gets past decrypt padding only about 1 in 256 times, so a functional test cannot exercise the guard deterministically. - A *locked* blank encrypted wallet still accepts about 1 in 256 wrong passphrases. That gap already existed and Bitcoin Core has it too. Closing it needs a stored key check, so it is out of scope here. ## Breaking Changes None. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 18f0978db2da05fb2b658b83c0796fafc758859f27cc777d9f7b85196339f0824f128e94ad552feca7b903a3f387de61957e2900ae2995dace349cea9536b190 (cherry picked from commit 2630930)
…as repair errors 581a219 fix(evo): treat missing snapshots during evodb repair as repair errors (pasta) Pull request description: ## Issue being fixed or feature implemented When the evodb is missing a masternode list snapshot record, the automatic evodb verify/repair at startup (added in dashpay#6999) does not fail. It logs a warning, reports success and marks the database as repaired, so the check never runs again. `RecalculateAndRepairDiffs` treats a missing snapshot as unrecoverable. It stops processing and emits `CRITICAL: Snapshot missing at height N. This cannot be repaired by this tool - full reindex required.` That message goes into `verification_errors`, not `repair_errors`. The startup caller only treats `repair_errors` as fatal, so for a missing snapshot it: - prints `WARNING: Verification errors:` and then `Successfully repaired 0 masternode list diffs`; - calls `CompleteRepair()`, which sets `DB_LIST_REPAIRED`, so every later start logs `Masternode list diffs are already repaired` and skips the check, unless the debug-only `-forceevodbrepair` is passed; - discards any diffs it already recalculated for earlier snapshot pairs, because of the early return, and never checks the pairs after the missing snapshot. The `evodb repair` RPC has the same problem. It returns an empty `repairErrors` array, although the help text defines a non-empty `repairErrors` as "full reindex required". This only affects nodes whose own evodb is already missing a snapshot record. ### Why this is a real problem Code path at c14104b: - A missing `from` snapshot (except the initial DIP0003 one) and a missing `to` snapshot are both pushed to `verification_errors`, followed by `return result;` before `WriteRepairedDiffs`: [deterministicmns.cpp#L1245-L1266](https://github.com/dashpay/dash/blob/c14104b172a2/src/evo/deterministicmns.cpp#L1245-L1266), [#L1290-L1293](https://github.com/dashpay/dash/blob/c14104b172a2/src/evo/deterministicmns.cpp#L1290-L1293) - Startup only logs `verification_errors`, shuts down only on `repair_errors`, and otherwise calls `CompleteRepair()`: [init.cpp#L2517-L2533](https://github.com/dashpay/dash/blob/c14104b172a2/src/init.cpp#L2517-L2533) - `CompleteRepair()` persists `DB_LIST_REPAIRED`, and once it is set startup skips the check: [deterministicmns.cpp#L1298-L1310](https://github.com/dashpay/dash/blob/c14104b172a2/src/evo/deterministicmns.cpp#L1298-L1310), [init.cpp#L2491](https://github.com/dashpay/dash/blob/c14104b172a2/src/init.cpp#L2491) - RPC contract for `repairErrors`: [rpc/evo.cpp#L2132](https://github.com/dashpay/dash/blob/c14104b172a2/src/rpc/evo.cpp#L2132) End-to-end reproduction on regtest. I mined 1200 blocks, so snapshots exist at heights 432, 576 and 1152. I stopped the node, deleted the `("dmn_S3", <hash of block 576>)` record from `evodb` with a small leveldb tool, then restarted twice. <details> <summary>Reproduction with a <code>dashd</code> built from c14104b (unfixed)</summary> ``` == [unfixed] mine 1200 regtest blocks (snapshots at DIP0003 height 432, 576 and 1152) tip height: 1200 block 576: 0aa9baa5c36d2f2f357c320bc33c7893677b60681fc2d3dae17faa0705bbc917 == [unfixed] remove the masternode list snapshot record at height 576 from evodb snapshot record existed=1 (41 bytes), delete=OK == [unfixed] restart #1 (startup evodb verify/repair runs) node state after restart #1: RUNNING Verifying and repairing masternode list diffs... WARNING: Verification errors: CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required. Successfully repaired 0 masternode list diffs, verified 0 snapshots in 0s == [unfixed] restart #2 node state after restart #2: RUNNING Masternode list diffs are already repaired ``` `evodb repair` on the same node: ``` $ dash-cli evodb repair { "startHeight": 432, "stopHeight": 1200, "diffsRecalculated": 0, "snapshotsVerified": 0, "verificationErrors": [ "CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required." ], "repairErrors": [ ] } ``` </details> The new unit test also fails on the unfixed code: ``` $ src/test/test_dash --run_test=evo_dip3_activation_tests/evodb_repair_missing_snapshot_is_repair_error Running 1 test case... test/evo_deterministicmns_tests.cpp:3559: error: in "evo_dip3_activation_tests/evodb_repair_missing_snapshot_is_repair_error": check result.verification_errors.empty() has failed test/evo_deterministicmns_tests.cpp:3560: fatal error: in "evo_dip3_activation_tests/evodb_repair_missing_snapshot_is_repair_error": critical check result.repair_errors.size() == 1U has failed [0 != 1] *** 2 failures are detected in the test module "Dash Core Test Suite" ``` ### Why it matters The automatic startup check exists to find and fix bad masternode list diffs. When it hits a missing snapshot, the operator is told the repair succeeded, the database is marked as repaired, and the check is turned off for later starts. Diffs before the missing snapshot that the repair already found to be bad stay bad on disk, and nothing after it is checked. The node keeps running on evodb data the repair itself rated "full reindex required". Other nodes are not affected. ## What was done? In `RecalculateAndRepairDiffs`, both missing-snapshot errors go into `repair_errors` when `repair` is true, and into `verification_errors` as before when `repair` is false. The message text is unchanged. The existing startup path then does what it already does for other unrecoverable repair failures. It logs `Failed to repair masternode list diffs. Database corruption detected. Please restart with -reindex to rebuild the database.` with the error, calls `StartShutdown()`, and returns without calling `CompleteRepair()`. `evodb repair` reports the error under `repairErrors`, and `evodb verify` still reports it under `verificationErrors`. Added a regression test, `evodb_repair_missing_snapshot_is_repair_error`, to `src/test/evo_deterministicmns_tests.cpp`. It mines past two snapshot periods and first checks that the intact range verifies without errors. It then erases the snapshot record at height 576 and checks that repair mode reports it in `repair_errors`, both when the missing snapshot ends a pair and when it starts one. Finally it checks that verify-only mode still reports it in `verification_errors`. ### Why this is the correct minimal fix The root cause is that this condition was put in the wrong error category. Everywhere else in this function, an error that means "cannot repair, full reindex required" goes into `repair_errors`, which is what both callers act on. Moving the error there reuses the existing startup handling and the documented RPC contract, so `init.cpp`, the RPC code and `RecalcDiffsResult` need no change. Alternatives considered: - Skipping `CompleteRepair()` while letting the node keep running would also need a separate signal, either a new result field or a check in init. It would also rerun a check that cannot succeed on every start without telling the operator what to do. - Writing the diffs already recalculated before returning would not help. The snapshot still can't be rebuilt, and the operator still has to reindex. Intentionally unchanged: - The other early returns (`Stop height ... below DIP0003`, `Could not find starting snapshot`, `Need at least 2 snapshots`) stay in `verification_errors`. They are normal on short chains. - A missing initial snapshot at the DIP0003 height is still replaced by an empty list, as before. ## How Has This Been Tested? macOS arm64, `--enable-debug --enable-werror`, depends build. - `src/test/test_dash --run_test=evo_dip3_activation_tests/evodb_repair_missing_snapshot_is_repair_error`: fails without the fix (output above) and passes with it. - `src/test/test_dash --run_test=evo_dip3_activation_tests`: all 41 test cases pass with the fix. - I re-ran the regtest reproduction above with a `dashd` built with this change. <details> <summary>Same reproduction with this change</summary> ``` == [fixed] restart #1 (startup evodb verify/repair runs) node state after restart #1: NOT RUNNING Verifying and repairing masternode list diffs... Failed to repair masternode list diffs. Database corruption detected. Please restart with -reindex to rebuild the database. CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required. Shutdown: done == [fixed] restart #2 node state after restart #2: NOT RUNNING Verifying and repairing masternode list diffs... Failed to repair masternode list diffs. Database corruption detected. Please restart with -reindex to rebuild the database. CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required. Shutdown: done == [fixed] restart with -reindex after the repair failure tip height after reindex: 1200 Skipping evodb repair during reindex $ dash-cli evodb verify { "startHeight": 432, "stopHeight": 1200, "diffsRecalculated": 0, "snapshotsVerified": 2, "verificationErrors": [ ] } ``` RPC output with this change on a node that is missing the snapshot: ``` $ dash-cli evodb verify { ... "snapshotsVerified": 0, "verificationErrors": [ "CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required." ] } $ dash-cli evodb repair { ... "snapshotsVerified": 0, "verificationErrors": [ ], "repairErrors": [ "CRITICAL: Snapshot missing at height 576. This cannot be repaired by this tool - full reindex required." ] } ``` </details> - `test/lint/lint-whitespace.py` and `test/lint/lint-includes.py` pass, and `clang-format-diff.py` is clean on the new test code. ## Breaking Changes A node whose evodb is missing a masternode list snapshot now shuts down at startup and asks for `-reindex` until it is reindexed. Previously it kept running and marked the evodb as repaired. `evodb repair` now reports this condition in `repairErrors` instead of `verificationErrors`. Healthy nodes are unaffected. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 1cc63dc64c8e0ef18899150d39d40529dd630246853f983f080e91078d88c1940c04c243c9deddbbe21115932e0324e9d7133c5eb1235e494853675aadebe3d8 (cherry picked from commit a0950fd)
…ilures 973f59e fix: relock encrypted wallets after HD upgrade failures (pasta) Pull request description: ## Issue being fixed or feature implemented An encrypted wallet temporarily unlocked by `upgradetohd` remained fully unlocked when mnemonic validation or later HD setup threw. An already authenticated caller with signing/spending permissions could then use existing keys until another lock event. This does not bypass RPC authentication. ## What was done? Install the existing cleanup handler before the temporary unlock and relock on every subsequent exit, including success, while the wallet locks are still held. Keep early validation behavior and relock before any rescan. ## How Has This Been Tested? On macOS arm64 with prebuilt depends, built baseline and candidate with `make -j1`. The added regression fails on the baseline in both legacy and descriptor modes because signing succeeds after a rejected upgrade. Full `wallet_upgradetohd.py --legacy-wallet` and `--descriptors` pass with the fix, covering signing denial after invalid mnemonic, legacy overlength passphrase and successful upgrade, plus verified signatures after explicit unlock. Existing recovery/rescan tests remain covered by those full runs. Ran `test_dash --run_test=bip39_tests,wallet_crypto_tests`, changed-file Python lint using the repository's enabled rules, and `git diff --check`. An independent four-pass review approved the final source and inspected the reproduced baseline evidence; its remaining cache/test suggestions were resolved by the complete reruns. No fund movement or RPC authentication bypass was demonstrated. ## Breaking Changes None. ## 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 This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: 01898d5180b07590481c09119cdcc7fcdc0cbefd347d1a938348c42b3471041641fe68b3b065551ec2bf48dd1bed8f9cd9fc237261632245e679155c446520a9 (cherry picked from commit 66d9b6a)
9178483 fix: validate Electrum import input boundaries (pasta) Pull request description: ## Issue being fixed or feature implemented `importelectrumwallet` reads an exact-sized JSON file allocation through UniValue's pointer-only parser, which determines length using `strlen`. A full read supplies no terminator. It also accepts negative rescan indices, imports keys and then passes the negative height to an exact chain lookup that aborts the node. Both paths require an authorized RPC caller, a readable local export and an unlocked legacy wallet. Method-limited deployments make the negative-index abort relevant beyond an administrator's existing ability to stop a node. ## What was done? Use a binary, length-managed string for JSON input, validate file size/read errors, resize to the actual byte count and invoke the bounded parser overload. The string also supplies the parser's sentinel. Reject a negative rescan index before processing the export or installing keys; preserve the existing clamp for indices above the tip and document the non-negative argument. Extend `wallet_dump.py` to reject a valid JSON prefix followed by embedded NUL/trailing bytes, check negative indices for JSON and CSV without importing keys, and retain valid bare/p2pkh-prefixed WIF imports at zero, tip and above-tip indices. Verify the imported private keys through dump/sign/verify RPCs and unload the temporary wallets before the existing dumpwallet scenarios. ## How Has This Been Tested? On macOS arm64 with BDB and SQLite enabled: full `make -j1`, the complete `wallet_dump.py --legacy-wallet --portseed=1`, a separate negative-index node-liveness regression (both serially at TEST_RUNNER_PORT_MIN=12000), 31 wallet/walletdb unit cases, and both UniValue object/data test executables passed. Python lint with the repository selection, assertion/include/whitespace lints and `git diff --check` passed. The UniValue data executable requires `src` as its working directory; an initial root-directory invocation failed to find its fixtures and the corrected invocation passed. Independent four-pass review found no blocking source defects; the final transferred patch exactly matches the reviewed file/diff hashes. Baseline defensive RPC regressions accepted the embedded-NUL JSON prefix, and a separate disposable regtest negative-index RPC terminated the node at the production `getBlockHash` assertion. The earlier isolated actual-UniValue ASan proof demonstrates the allocation/parser primitive, not a full-daemon disclosure or code-execution outcome. No deployed restricted account or production crash probability was measured. Stream-error injection, concurrent file modification and exact rescan-start observation were not tested. ## Breaking Changes Negative rescan indices now return RPC_INVALID_PARAMETER before import. JSON bytes after an embedded NUL are parsed and rejected instead of silently ignored. Valid export formats and the above-tip clamp remain supported. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: b5809feb6b963b43c9e7d567bb7991c1695b35b2415f33c19afe54811c4098be1d7d751181019476cbb3101383e265c2d4916797bacc9c05be47a2166ef9c924 (cherry picked from commit bda514a)
…t data 2226682 fix: return REST errors for unavailable deployment data (pasta) Pull request description: ## Issue being fixed or feature implemented REST deployment queries call the deployment RPC directly. When EHF reconstruction needs an unavailable block body, its exception can escape the HTTP worker. This requires REST to be enabled and the client to pass the HTTP address ACL; REST is disabled by default. ## What was done? Translate deployment RPC exceptions into a plain-text HTTP 500 response before writing successful response headers. Valid requests retain the existing JSON response. The change leaves deployment rules and reconstruction unchanged. ## How Has This Been Tested? On macOS arm64 with prebuilt depends: `make -j1`, `test_dash --run_test=rpc_tests`, `interface_rest.py` with an isolated port range, selected Python lint, whitespace lint, circular-dependency lint, and `git diff --check` passed. The functional test requests a known post-V20 header without its body, checks the error and continued RPC/REST responses, then supplies the body and checks the successful response. The first runner attempt failed during cache startup because an HTTP port was occupied; the isolated retry passed. Independent final four-pass review completed. No pre-fix daemon termination or external exposure demonstration was performed, and the test is not claimed as a before/after crash reproduction. ## Breaking Changes Unavailable deployment reconstruction returns HTTP 500 instead of allowing an exception to escape the callback. ## 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 This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: 79824586e1311ca3d23b110b5cff11c75f1afdd6f42103cd802dce7fa6b990934f37bc86d5ffd6d187d76a4d3812f5aa67dcc2cde96f75b6a18a2dae8c2062fe (cherry picked from commit b258eab)
…ssphrases 6861fa5 fix: preserve wallet permissions when changing passphrases (pasta) Pull request description: ## Issue being fixed or feature implemented A successful `walletpassphrasechange` during a CoinJoin mixing-only unlock silently left the wallet fully unlocked. An already authenticated caller with signing/spending/export permission could use keys beyond the owner's prior mixing restriction. This does not bypass RPC authentication, and the existing unlock timer still bounds the interval. ## What was done? Capture the original locked/mixing mode under the wallet's existing locks. Unlock a mixing-only wallet as mixing-only so it stays that way, and relock wallets that started locked on every exit path via a cleanup handler. Successful maintenance preserves full unlock where it was already allowed; incorrect-old-passphrase behavior, encryption format and unlock timers remain unchanged. ## How Has This Been Tested? Built on macOS arm64 with prebuilt depends. The added regression fails against baseline in both wallet backends because signing succeeds after changing a mixing-only wallet's passphrase. `wallet_encryption.py --legacy-wallet` and `--descriptors` pass with the fix. The regression runs a locked/mixing/full input table: after `walletpassphrasechange`, it checks `signmessage` (with signature verification) and `coinjoin start`/`stop` authorization for each mode. Existing encryption, passphrase-change, null-byte and timeout-bound coverage in `run_test` is unchanged. Ran `test_dash --run_test=bip39_tests,wallet_crypto_tests`, Python lint, whitespace lint and `git diff --check`. ## Breaking Changes None. Successful passphrase changes retain the existing wallet permissions. ## 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 Top commit has no ACKs. Tree-SHA512: 66b98c25a9fdb88366367ad91a64490c63ec4e8b4d89506c132f6baf833b0312bf4e321a0204045babb0d66e3e56f425469099fd7826ea480c3a67986dfe8e12 (cherry picked from commit 8cb2c95)
…alidation a819870 fix: hold chainstate lock during CoinJoin input validation (pasta) Pull request description: ## Issue being fixed or feature implemented CoinJoin input validation obtained a pointer to the shared chainstate coins cache under `cs_main`, then released that lock before reading the coins. Confirmed-coin lookups can populate the cache, so those reads could race block connection, disconnection or flushing. CoinJoin is opt-in for clients and requires an active mixing session on coordinators. The source lock-contract violation is established; no race, crash or state corruption has been reproduced. ## What was done? Hold `cs_main` for the complete `IsValidInOuts()` input-validation path, including every `CCoinsViewMemPool::GetCoin()` fallback to the shared cache. The RAII guard also covers early returns and releases before callers perform wallet or session cleanup. This follows the existing locking used by collateral validation. ## How Has This Been Tested? macOS arm64 with the prebuilt depends prefix, configured with `--without-gui --disable-bench --disable-fuzz-binary`: - `make -j1` passed. - `test_dash --run_test=coinjoin_inouts_tests,coinjoin_tests,coinjoin_dstxmanager_tests,coinjoin_queue_tests --log_level=message` passed (82 cases). - `test_runner.py -j1 --portseed=1 rpc_coinjoin.py` passed with the assigned `TEST_RUNNER_PORT_MIN=20000`, along with the runner's framework unit tests. - Include, whitespace and circular-dependency linters and `git diff --check` passed. Independent four-pass review checked the shared-cache access, caller lock ordering, mempool and InstantSend nesting, and error paths. No blocking issue was found. No timing-sensitive regression test was added; no TSAN, scheduled race or crash consequence is claimed. GUI and other build configurations were not validated locally. ## Breaking Changes None. No wire-format, wallet-format, fee-policy or consensus-rule change. ## 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 This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: bab0dbd4041ab10bd6387820b27c37b0d8a47d658213e84337c6a1d8925395b8e63680c79057d16d769e33e8d42ad1da4f9af1d700a39b2afc0e2c71cb0e072b (cherry picked from commit 3d18d42)
ecae783 fix: authenticate cached InstantSend proof bytes (pasta) Pull request description: ## Issue being fixed or feature implemented When an ISDLOCK arrives for a transaction whose recovered signature this node already has, the verifier skipped checking the lock's own signature and cycle. The cached recovered signature proves that the quorum signed this txid. It does not prove that the bytes in the received lock are that signature. The unchecked lock was then stored, relayed by its hash and returned by `getislocks`. A lock with the same txid that arrives later is rejected once one is stored. So a peer that sends an on-curve but invalid signature first, or a different known cycle hash, can keep the authentic proof from being stored and relayed. This does not authorize any new spend or cause a consensus split. It needs the recovered signature to be cached while no lock is stored, for example on a `-watchquorums` node. ## What was done? `BuildVerificationBatch` now resolves the quorum from the received lock's cycle before it consults the cache. It skips verification only when the cached recovered signature came from that same quorum and has the same signature bytes. Any other lock goes through normal batch verification. `missing_quorum_does_not_drop_batch` now uses a real mined non-rotating quorum, because a cache hit now needs a quorum that can be resolved. Rotating missing-quorum coverage stays in `received_genesis_cycle_has_no_quorum`. This is a Dash-specific fix. Bitcoin Core has nothing to backport. ## How Has This Been Tested? macOS arm64, existing depends build: - Full `make -j1` passed. - `test/functional/p2p_instantsend.py` passed. It uses non-rotating quorums. - `test/functional/rpc_verifyislock.py` passed. It uses rotating quorums, so masternodes that already hold the recovered signature receive the network locks. - `lint-includes.py`, `lint-circular-dependencies.py` and `git diff --check` passed. Linux x86_64 debug build, after the tests were simplified: - `./src/test/test_dash --run_test=evo_islock_tests` passed all 14 cases, on 3 runs in a row. New test `cached_recovered_sig_does_not_authenticate_other_lock`: - It adds mined commitments for two non-rotating quorums and caches a genuine recovered signature for a mempool transaction. It then passes a changed lock straight to `ProcessPendingInstantSendLocks`: once signed by another key, and once with the other cycle hash. It checks that the changed lock is not stored, and that the authentic lock processed afterwards is stored byte for byte. - With `net_instantsend.cpp` reverted to the base, both cases failed: the changed lock was stored and the authentic lock was not (4 failed checks, exit 201). An independent review approved the change. Beyond `rpc_verifyislock.py`, rotating-quorum cache hits are covered by reasoning, not by a dedicated test. No end-to-end P2P timing demonstration was done. ## Breaking Changes None. The wire format, quorum selection and signature validity rules are unchanged. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: ce382fe64d1a7fd18a6199d4fbde73ca488857bdc08f855f927694cbab645ed20a6376df587b8ba7adea8a390ce103d07ff0b4e77f5f0a6d3ce40f1817994e05 (cherry picked from commit 524173e)
… the governance cache 9e5ee16 fix: bound governance object signatures (pasta) f2f1dd4 fix: reset the governance cache when upgrading (pasta) Pull request description: ## Issue being fixed or feature implemented Governance proposal validation limits proposal data to 512 bytes but ignores its separate signature field. A paid, confirmed proposal can therefore carry megabytes of unused signature bytes that peers retain and persist. Each distinct signature-bearing object requires its own collateral payment, so this is economically constrained memory/disk amplification rather than a vote-weight or issuance defect. ## What was done? Two commits, migration first and then enforcement: 1. **fix: reset the governance cache when upgrading.** Bumps the governance cache version (`CGovernanceManager-Version-16` → `17`), so every node discards its existing `governance.dat` once and syncs governance objects and votes again from the network. This is the migration for the next commit: proposal signatures were never bounded or checked, so an existing cache may hold oversized signatures, and bounding disk reads without a version bump would make such a cache fail to load. Since the format version changes anyway, this also drops the historical invalid-vote-cache field, which two TODOs (from 0a8c4da) asked to remove on the next disk-format bump. 2. **fix: bound governance object signatures.** Bounds the signature vector to 96 bytes on every read (network and disk) before its elements are allocated. Empty, 65-byte (legacy ECDSA) and 96-byte (BLS) signatures are still accepted. Writes and object/signature hashing are unchanged, and the existing log-only decode error handling adds no peer penalty. Release note added. ## How Has This Been Tested? On macOS arm64 with the existing depends prefix: - `governance_vote_wire_tests/object_signature_size_limit` (next to the existing vote-signature cap tests): `SER_NETWORK` and `SER_DISK` reads accept 0/65/96-byte signatures and reject a 97-byte one. With the bound removed, this case fails. - `governance_vote_processing_tests/legacy_governance_cache_is_discarded` (replaces `legacy_invalid_vote_cache_is_discarded`): a version 16 cache holding a proposal and a legacy invalid vote loads empty; a version 17 cache loads and round-trips. With the version string left at 16, this test fails. - All governance unit suites pass (`governance_vote_wire_tests`, `governance_vote_processing_tests`, `governance_vote_sync_tests`, `governance_inv_tests`, `governance_superblock_tests`, `governance_validators_tests`). - `feature_governance.py`, `feature_governance_cl.py`, `feature_governance_objects.py` and `feature_governance_txindex_devnet.py` pass. ## Breaking Changes - Governance object signatures over 96 bytes are rejected on network and disk reads. - On the first start after upgrading, the existing governance cache is discarded and governance data is synced again from the network (see `doc/release-notes-7831.md`). ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 78fb7a852f3d67163dc35cf6f6f7c7e3d650e2834cf69b97a3d5be92641be73121b16b52bd7317c44e079062396678b6e4f294c55d59c2ecb1eca111960f713b (cherry picked from commit 41cf1a7)
…nance trigger path a551177 fix: update masternode rate limit on failed governance trigger path (pasta) Pull request description: ## Issue being fixed or feature implemented In `AddGovernanceObjectInternal()`, a trigger object is emplaced into `mapObjects`, then `AddTrigger()` is attempted. On failure the function calls `PrepareDeletion()` and returns early, before `MasternodeRateUpdate()`. `MasternodeRateCheck()` allows any masternode that has no entry in `mapLastMasternodeObject`, and only `MasternodeRateUpdate()` creates that entry. A masternode that never lands a *successful* trigger therefore never gets a rate-buffer entry, and every malformed but signed trigger it sends passes the rate check. That defeats the limiter meant to stop exactly this. Each rejected trigger still costs a BLS verification, a `mapObjects` entry held for the deletion delay, and, after erasure, a `mapErasedGovernanceObjects` entry retained for about two superblock cycles (roughly 60 days on mainnet). Sending one needs a valid operator key for a masternode in the tip list, and the victim must have requested the hash after an INV. Rejected triggers are not relayed, so there is no fan-out; the attacker has to connect to each victim directly. ## What was done? - Call `MasternodeRateUpdate()` before the `AddTrigger` check, so failed attempts advance the masternode's rate buffer too. - `MasternodeRateUpdate()` also queued near-future triggers for the deferred re-announcement in `CheckPostponedObjects`. That part is split out into `ScheduleTriggerRelay()` and still runs only after `AddTrigger` succeeds. Otherwise moving the rate update would make the node re-announce, and serve on GETDATA, a trigger it had just undone and marked for deletion. ### Scope note: creation-time rate limiting on short-cycle networks The rate buffer stores the object's signer-chosen creation time, and `MasternodeRateCheck` accepts creation times from `now - 2 * cycle` to `now + 1h`, with a maximum rate of `2.2 / cycle` over five entries. On mainnet (cycle = 16616 blocks, about 28.8 days) the widest possible span is about `2 * cycle`, so a full buffer is always rejected (`5 / (2 * cycle)` = `2.5 / cycle` > `2.2 / cycle`). On testnet, devnet (1 h cycle) and regtest the accepted window is wider than `5 / 2.2` cycles, so an operator who alternates creation times between the window ends can stay under the rate. That weakness is in the existing limiter and applies equally to well-formed triggers today. This PR only brings failed triggers under the same limiter. Moving the limiter to receipt time would change how valid triggers are throttled and is left for a separate change. ## How Has This Been Tested? Two new cases in `src/test/governance_vote_processing_tests.cpp`, using its existing chain-backed fixture, which already registers a masternode with a known operator key. No new file or fixture, and `governance_inv_tests` is unchanged. - `failed_triggers_advance_the_masternode_rate_limit`: submits 12 signed triggers that fail `AddTrigger` and checks that exactly `RATE_BUFFER_SIZE - 1` (4) are stored. It fails with `12 != 4` on develop's `governance.cpp`/`governance.h`. - `failed_trigger_is_not_scheduled_for_deferred_relay`: a failed trigger created inside the deferred-relay window is never announced, even after it ages past `RELIABLE_PROPAGATION_TIME`. It fails when `ScheduleTriggerRelay()` is called before `AddTrigger`, which is the regression the split prevents. Run locally on macOS arm64 with `--enable-debug --enable-werror`: - `src/test/test_dash --run_test=governance_vote_processing_tests,governance_inv_tests,governance_superblock_tests,governance_validators_tests` passes. - `test/functional/test_runner.py feature_governance.py feature_governance_objects.py feature_governance_cl.py` passes. - `test/lint/lint-whitespace.py` passes. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone Top commit has no ACKs. Tree-SHA512: 2892edb480e2c3c63c1be7d231d745ecea2d2dcf6161f48ec0cd3f3e6484a120af77de540db06e3e7892840dc6a1fb234df98025686fb27a121731a7996338ac (cherry picked from commit 8db4b76)
2c12027 fix: stop `masternode payments` at the genesis block (pasta) Pull request description: ## Issue being fixed or feature implemented `masternode payments` terminates dashd when it reaches the genesis block. For every block it visits, it reads the masternode list of the previous block with `GetListForBlock(pindex->pprev)`. That function takes a `gsl::not_null<const CBlockIndex*>`, so the genesis block's null `pprev` calls `std::terminate()`. Any RPC user can reach it in three ways: - `masternode payments <genesis hash>` - a negative count that walks back past block 1, e.g. `masternode payments <block 1 hash> -2` - `masternode payments` with no arguments on a chain still at height 0 ## What was done? Payees for a block are chosen from the previous block's list, so the genesis block has no masternode payments. The loop now stops at the genesis block. Asking for the genesis block returns an empty array, and a backwards walk ends at block 1. ## How Has This Been Tested? - Reproduced the crash by hand on an unpatched regtest build (`std::terminate() called` in debug.log), both at height 0 and at height 1. - `rpc_masternode.py` gains two assertions: payments for the genesis block, and a backwards walk from block 1. Against the unpatched dashd the test fails at the first of them because the node dies. With the fix it passes. - macOS arm64, depends build. ## Breaking Changes None. Requests that used to crash the node now return results. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 2c12027 Tree-SHA512: 5b0a7646cde26c0c6de1a3341847e0619718acacbf392fc15e4cee034ab0bd680dd263d69d6937b1db0b43b9c1ae776cd9a1b5c5eaaffecd6d812dfbc18b7647 (cherry picked from commit 1aed5c3)
…de the active chain ec93e69 fix: ProcessBlock now stores replaces existing items in cache of mn-lists (Konstantin Akimov) 0f5b8d6 fix: refuse masternode list RPCs for blocks outside the active chain (pasta) Pull request description: ## Issue being fixed or feature implemented If an RPC asks for the masternode list at a block whose header the node has accepted but which it has never connected, the node caches a wrong list. It then rejects the next valid block and stalls. `CDeterministicMNManager::GetListForBlockInternal()` walks back from the requested block looking for a list snapshot or diff in EvoDB. If it finds neither, it treats that block as the initial snapshot: it caches an empty list under the block's hash and points `m_initial_snapshot_index` at it. A block that was never connected has no diff, so it lands in that fallback. When the block is connected later, `ProcessBlock()` adds the real list with `emplace`, which keeps the existing entry, so the empty list stays cached. The next block is then built on an empty list and rejected with `bad-cbtx-mnmerkleroot`. Steps on regtest with registered masternodes: 1. `submitheader` block N 2. `protx info <proTxHash> <hash N>` (returns "not found" because the list it read is the fabricated empty one) 3. `submitblock` block N 4. A correct block N+1 from a peer is rejected with `bad-cbtx-mnmerkleroot` The rejection is saved as `BLOCK_FAILED_VALID` in the block index. A restart drops the bad cache entry, but the block is still marked invalid, so the node stays stuck until someone runs `reconsiderblock`. I confirmed this on an unpatched build: after the restart, `getchaintips` still lists the peer's block as `invalid`. Three stock RPCs can reach this, all of which need an authenticated RPC user: - `protx info` with a block hash - `protx listdiff` - `masternode payments`, when it is given a block that is stored but whose parent is header-only `protx diff` and the P2P `getmnlistd`/`getqrinfo` handlers are not affected. They go through `BuildSimplifiedMNListDiff()`, which already requires both blocks to be in the active chain. ## What was done? - `protx info`, `protx listdiff` and `masternode payments` now return `RPC_INVALID_PARAMETER` ("Block is not in main chain") for a block outside the active chain. This is the same rule `protx diff` already follows. - `CDeterministicMNManager::ProcessBlock()` now stores each connected block's list with `insert_or_assign` instead of `emplace`. Whatever an earlier lookup cached under that block hash, connecting the block replaces it with the correct list. This closes the cache poisoning for any caller, not only these three RPCs; the RPC checks stay so callers get an error instead of a made-up empty list. ## How Has This Been Tested? - The new section in `rpc_masternode.py` follows the steps above: `generateblock` with `submit=False`, `submitheader`, then calls to the three RPCs at that hash. It then submits the block and mines the next one. - Unpatched dashd: the test fails, because the RPCs still answer. - Patched dashd: the test passes. - A separate probe run on the unpatched dashd went further. The node rejected a peer-mined block N+1 with `bad-cbtx-mnmerkleroot`, and still showed it as `invalid` after a restart. - Also passed on the patched build: - `feature_dip3_deterministicmns.py --descriptors` (uses `protx info` and `protx listdiff` at historical active-chain blocks) - `feature_llmq_simplepose.py` and `feature_llmq_simplepose.py --disable-spork23` - `rpc_quorum.py` - `evo_dip3_activation_tests/dip3_activation_legacy` now submits a block's header, reads the list for it, then connects the block and checks the tip list contains the newly registered masternode. With `emplace` restored the check fails (the stale empty list is still cached); with `insert_or_assign` it passes. - macOS arm64, depends build. ## Breaking Changes `protx info <proTxHash> <blockHash>`, `protx listdiff` and `masternode payments <blockHash>` now return an error for a block outside the active chain, including stale-fork blocks that were connected at some point. Before, they answered for any known block. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: self-ACK ec93e69 Tree-SHA512: aabc1c038cf43f6c601b93d81c4d2dac6b2d6f38796730b0d9fdd5ccdd38a687fa6d4d18df19473d1bff5c03ac5ba1d9938b705b6986b7969f140533e48487e2 (cherry picked from commit 1cc3a63)
…se of purpose strings with an enum 57bd6a1 Merge bitcoin#27217: wallet: Replace use of purpose strings with an enum (fanquake) Pull request description: ## Issue being fixed or feature implemented Instead of storing and passing around fixed strings for the purpose of an address, use an enum. This backport bitcoin#27217 is touching multiple wallet's files and method signatures and skipping it for v24 will block many other backports until v25. Let's merge it in v24.0 better. ## What was done? Address Purposes strings are now restricted to the currently known values of "send", "receive", and "refund". Wallets that have unrecognized purpose strings will have loading warnings, and the `listlabels` RPC will raise an error if an unrecognized purpose is requested. ## How Has This Been Tested? See changes in tests ## Breaking Changes Wallet ------ * Address Purposes strings are now restricted to the currently known values of "send", "receive", and "refund". Wallets that have unrecognized purpose strings will have loading warnings, and the `listlabels` RPC will raise an error if an unrecognized purpose is requested. ## 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 Top commit has no ACKs. Tree-SHA512: e56c86181d4f72721621e2bc69c312c09e8f6a36f5d2205eb663f1884551f87bf365cbdace89a277bf03051c6da1cd6662debba5d47a21dda608011c8aee1bb5 (cherry picked from commit c8bfd7a)
…es with unchanged bytes in state diffs c9552ef test: cover operator key scheme change with unchanged bytes in state diffs (UdjinM6) 020ddd1 fix(evo): capture operator key scheme changes with unchanged bytes in state diffs (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Syncing mainnet from scratch on recent `develop` fails at block 2005288 (`000000000000004f95a6f3b181270e2644ab76abd14bb9402cca7a1f3f3eea93`) with `bad-cbtx-mnmerkleroot`. Block 2005287 has a ProUpRegTx that moves masternode `af40de...9024` from LegacyBLS to BasicBLS. The new basic-scheme operator key has the same bytes as the old legacy key. Those bytes decode to different keys under the two schemes. `CDeterministicMNStateDiff` decided whether `pubKeyOperator` changed by comparing `GetHash()`, which only covers the encoded bytes. So the stored diff carried the new `nVersion` but not the key. A list built online is correct. A list reconstructed from the stored diff keeps the legacy-scheme key under `nVersion=2`. Its SML entry then serializes the key as zeros, and the merkle root in the next block's CbTx doesn't match. Recent masternode list cache eviction changes exposed it during mainnet sync by forcing reconstruction from diffs. ## What was done? - `CDeterministicMNStateDiff` now also records `pubKeyOperator` when the BLS scheme implied by `nVersion` changes, not only when the encoded bytes change. The diff format and field layout are unchanged; the existing field is just filled in one more case. - The existing same-key re-encoding test is now table-driven, with the mainnet block 2005287 key added as a second case. Nodes that already stored the bad diff need `-reindex` to rebuild it with the fixed code. `-reindex-chainstate` also rebuilds evodb, but it keeps the block index, so nodes that already rejected block 2005288 keep it marked invalid. ## How Has This Been Tested? Unit tests, mutation check and lint ran on macOS 27 arm64, configured with `--enable-debug --enable-werror --enable-crash-hooks` against depends, at `c9552ef5f9`. - `./src/test/test_dash --run_test=evo_dip3_activation_tests` passes. Removing the new scheme check from the fix makes only the new "same bytes, different key" case fail. The existing case still passes. - Before the fix, instrumenting a mainnet sync confirmed the diagnosis: the list rebuilt from the diff at 2005287 had the masternode with a legacy-scheme key under `nVersion=2`, and the list built online did not. - A full mainnet `-reindex` with the fixed binary synced past 2005288 with no new issues. - `test/lint/all-lint.py`: the only failure is an existing cppcheck `knownConditionTrueFalse` warning at `src/governance/object.cpp:856`, which this PR doesn't touch. ## Breaking Changes None. Diffs written by the fixed code use the existing format. Nodes that already stored the bad diff need `-reindex` (see above). ## 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: 77d9d2e794fe3f784c0b82ae5acf3e78d6447d7cc6278b1876439a232a4ca8c3519e061964638c7075307c18ccfda6cd165d900b62c8e902d0ae079ba9048a33 (cherry picked from commit 1bc5197)
969c5a5 docs(rpc): qualify ChainLock lookup failures (PastaClaw) 5e1bfe7 docs: omit release note for RPC help clarification (PastaClaw) a088909 docs: note historical ChainLock help clarification (PastaClaw) d60b248 docs(rpc): clarify historical ChainLock lookup (PastaClaw) Pull request description: ## Issue being fixed or feature implemented Fixes dashpay#7872. The `getchainlockbyheight` help currently makes it easy to expect any ChainLocked height to be available, including the current best ChainLock and heights skipped by embedded ChainLocks. Requested by thephez via PastaClaw. Source: dashpay#7872 ## What was done? - Explain that only ChainLocks embedded in a later active-chain block's coinbase can be returned. - Document that the current best ChainLock is unavailable until embedded, and direct callers to `getbestchainlock` for the live result. - Explain that skipped heights cannot be returned even if they are ChainLocked, and document the exact `Chainlock not found for height` error for completed lookups. - Document that unavailable block data, such as pruned carrier blocks, can cause the lookup to fail instead. No release note is included, as requested in human review for this help-only clarification. Keep the coinbase-backed behavior unchanged. An in-memory best-ChainLock fallback would lack the carrier block required by the existing `cbtx_height` result field, so it is not a clearly better small behavior change for this help fix. ## How Has This Been Tested? On macOS/Apple Silicon: - Reviewed the help against `CoinbaseChainLockReader::Find`, `LoadCarrier`, the active-chain carrier check, and `getbestchainlock`. Also inspected the existing unavailable-carrier-data unit test (not run). - `git diff origin/develop..HEAD --check` and `COMMIT_RANGE=origin/develop..HEAD python3 test/lint/lint-whitespace.py` passed, including the pruning clarification. - `python3 test/lint/lint-format-strings.py` passed again for the pruning clarification. - `./autogen.sh` and minimal wallet/GUI-disabled configuration succeeded. - `make -C src -j2 rpc/libbitcoin_node_a-quorums.o` passed again, compiling the changed RPC translation unit including the pruning clarification. No new regression test was added for this help-only change; the documentation correction is directly reviewable in the diff. No daemon build or functional tests were run. ## Breaking Changes None. RPC lookup behavior, response fields, and error codes are unchanged. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- pastaclaw-job: cj-20261013-175557-050a --> Top commit has no ACKs. Tree-SHA512: 92d14b1a8904561a1bbb302802c844a1d3d7a877ef309d6192406b9c690e5fb979355cbc2b91bd36882834349c065ddad9f6c8b7fbb9883ae29c11be7784693e (cherry picked from commit 09174e0)
…viction a01acfd fix: keep another transaction's InstantSend outpoint entry on removal (pasta) d82e327 test: check InstantSend release through the kept transaction ref (pasta) 312fb0d fix: release InstantSend tracking after mempool eviction (pasta) Pull request description: ## Issue being fixed or feature implemented InstantSend tracks every unlocked transaction that enters the mempool. When an ordinary unlocked transaction left the mempool through expiry or size trimming, `TransactionIsRemoved` returned early: only Platform transfers were released. The confirmation cleanup only handles mined entries, so the evicted transaction stayed tracked for good. Its full object, parent links, spent-outpoint index and timing record were all kept outside the mempool size limit. Sustained churn of transactions that are never locked or mined grows this memory without bound. Normal lock delivery limits the exposure in practice. The achievable growth rate has not been measured. ## What was done? - `TransactionIsRemoved` now releases the tracking of any unlocked transaction through the existing `RemoveNonLockedTx` cleanup, which handles parent links, outpoint index and retry entries. It also drops the timing record. - `TransactionIsRemoved` is also called when a peer's transaction is rejected. On a node without txindex, a mined transaction that is not yet locked can be replayed and rejected after a reorg clears the recent-confirmed filter. That transaction's tracking must survive so that a later conflicting lock still finds its block. `RemoveNonLockedTx` therefore takes a `keepMined` flag, and the mined check and the erase happen together under `cs_nonLocked`. - `RemoveNonLockedTx` now erases an outpoint-index entry only if the removed transaction owns it. The index keeps the first tracked spender of each outpoint. Before this, removing an unmined transaction that spends the same input as a tracked mined one, for example one admitted after a reorg, dropped the mined transaction's entry. Eviction now reaches this cleanup for every unlocked transaction. - Platform transfers keep their existing unconditional removal. Locked-transaction handling is unchanged. Block removals never emit this callback. This is a Dash-specific fix. Bitcoin Core has nothing to backport. ## How Has This Been Tested? macOS arm64, existing depends build: - Full `make -j1` passed. - `./src/test/test_dash --run_test=evo_islock_tests` passed all 14 cases. - `test/functional/p2p_instantsend.py` and `test/functional/feature_llmq_is_retroactive.py` passed. - `lint-includes.py`, `lint-include-guards.py`, `lint-circular-dependencies.py` and `git diff --check` passed. New test `nonlocked_ordinary_tx_released_on_mempool_removal`: - It registers the real `NetInstantSend` validation listener and adds standard valid transactions to the mempool. It then expires the mempool and, in a second pass, trims it. The test keeps its own `CTransactionRef` and checks that its `use_count()` drops to 1, so the mempool and the InstantSend tracker have both released the transaction. - With the production code unchanged, both release checks fail (`2 != 1`). - A negative case tracks a transaction as mined and calls the public `TransactionIsRemoved` entry point. It checks that the tracker still holds the transaction and that `RetrieveISConflicts` still reports it at that block. An earlier version of this fix removed mined entries unconditionally, and this case fails against it. The same case then removes an unmined spend of the mined transaction's input and checks that the conflict is still reported. With the old unconditional outpoint erase, that check fails. An independent review approved the change. No sustained churn or memory-exhaustion run was done. The mined negative case uses the manager's public tracking calls, not a mined block or an end-to-end replay. ## Breaking Changes None. Tracking for mined transactions is kept, and locked-transaction and Platform-transfer handling are unchanged. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK a01acfd Tree-SHA512: fbc683cf3b30c3052962c5c803ac34c139c775a6d73e4fb2ab24e9d876da41f063a8a7e22c26bc75d1a111a0428093a9721e71e47f1e3ec98523ccabce369ef3 (cherry picked from commit 552d552)
ce5b786 partial Merge bitcoin#35984: sign: skip signing SIGHASH_SINGLE inputs with no corresponding output (pasta) Pull request description: ## Issue being fixed or feature implemented When a signer explicitly requests `SINGLE` or `SINGLE|ANYONECANPAY` for a PSBT input that has no output at the same index, the legacy signature hash is the constant one, so the signature does not bind the transaction as the signer expects. `SignTransaction()` already skipped such inputs but `SignPSBTInput()` did not. Default `ALL` signing and the wallet's requested-sighash agreement checks limit exposure to explicit nondefault signing. ## What was done? Partial backport of bitcoin#35984 (only test hunks omitted; see below). The missing-output check moves into `MutableTransactionSignatureCreator::CreateSig`, so PSBT and raw transaction signing share the same refusal, and the redundant raw-signing check is removed. Dash adaptations: - Uses `nHashType & 0x1f` with the legacy `SignatureHash`, since Dash lacks `SIGHASH_OUTPUT_MASK` and the upstream signing-options interface. - The regression test is ported for P2PKH only (Dash has no segwit or taproot) and extended with `SINGLE|ANYONECANPAY`, `ALL` and raw-signing controls. - Omitted, hence partial: upstream's `test_psbt_roundtrip`/`test_psbt_version` hunks, which replace `listunspent()[0]` with picking a mature 50-coin coinbase. Both tests arrive with PSBTv2 (bitcoin#21283), which Dash does not have, so there is nothing to apply them to. No ordering-dependent `listunspent()[0]` call runs after the new test in Dash's `rpc_psbt.py`, so no Dash equivalent is needed. ## How Has This Been Tested? On macOS arm64 with the existing depends prefix and `--enable-werror`: - Full `make -j1` passed before and after the change. - The new signing scenario failed on the baseline in both legacy and descriptor modes (input 1 finalized under `SINGLE`) and passes after the fix. - `rpc_psbt.py` passed in both descriptor and legacy variants, including the `SINGLE|ANYONECANPAY`, `ALL` and raw-signing controls. - `src/test/test_dash --run_test=sighash_tests,script_tests,psbt_wallet_tests` passed. - Targeted Python lint, whitespace lint and `git diff --check` passed. ## Breaking Changes The shared signer refuses to create `SINGLE` signatures when the matching output is missing. No consensus or RPC schema changes. ## 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: 4848c67cdbb8f53429b1c3f2f7a80aea652426a198947b175137c3a8fe2381ab284cf8086950d4434f6eeafa5caa78822f2b38da8e51b2ae1fadcfe1e969fd61 (cherry picked from commit 3dc3d05)
…tory reads 1f33db3 fix: retain initialized DKG sessions during inventory reads (pasta) Pull request description: ## Issue being fixed or feature implemented DKG round initialization replaces the current session on a phase thread while inventory getters read it from peer processing. The unsynchronized owner can be destroyed during a getter, and the replacement was visible before initialization completed. This fixes that object lifetime race; reliable crashes and stronger exploitation have not been demonstrated. ## What was done? Protect publication and acquisition with one handler mutex, initialize the next session privately, and return shared ownership to inventory and phase callers. Release the publication mutex before session work or retirement destruction. Failed initialization retires prior-round inventory. ## How Has This Been Tested? No new test is added: the race needs a getter running concurrently with a session replacement, which a deterministic unit test cannot reproduce. Verification was code review plus the existing tests: - On macOS arm64 with the configured depends build: `./src/test/test_dash --run_test=llmq_dkg_tests` passed, and `test/functional/test_runner.py -j1 feature_llmq_connections.py` passed, including normal DKG, quorum connections, probes and restarts. - The existing `feature_llmq_*` functional tests pass in CI under TSan (`linux64_tsan-test`) and ASan (`linux64_asan-test`), which exercise the session lifetime path. No baseline mutation or runtime race reproduction is claimed. ## Breaking Changes None. No wire, consensus, activation or persisted-format changes. ## 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 Top commit has no ACKs. Tree-SHA512: 7c1aef760dfb132d781de4c27c50c8f63770a57ed3ceb648061fefccc17ae939c728c88ba0594309b49418b456165f7ad74dfef8377afd4c4bcf99dac0fa3760 (cherry picked from commit c989ec6)
37258c0 Merge bitcoin#29855: psbt: Check non witness utxo outpoint early (pasta) cfdbb32 partial Merge bitcoin#25812: psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATION (pasta) Pull request description: ## Issue being fixed or feature implemented PSBT deserialization verified that a supplied previous transaction has the expected hash but accepted outpoint indices beyond its output vector, and `decodepsbt` then copied the referenced output without its own bounds check. Upstream rejects the invalid reference at the parser boundary, which prevents that out-of-bounds access. Triggering it requires RPC access or an operator decoding an externally supplied PSBT; no wallet unlock is needed. ## What was done? Backport of bitcoin#29855, preceded by a partial backport of bitcoin#25812. - **partial bitcoin#25812:** only the test harness, i.e. the `invalid_with_msg` section of `test/functional/data/rpc_psbt.json` and the `rpc_psbt.py` loop that checks each entry fails `decodepsbt` with its specific reason. The `PSBT_IN_TAP_BIP32_DERIVATION` length check and its Taproot test vector are omitted because Dash has no Taproot PSBT fields; they become applicable only if Dash adopts them. - **bitcoin#29855:** the existing hash check is retained and nonexistent previous-output indices are rejected during PSBT deserialization. Dash adaptations in bitcoin#29855: - The two new invalid fixtures were witness-stripped (Dash serialization has no witness framing or stacks) without changing txids, scripts, outputs, locktimes or outpoints. - Because the fixtures were edited, they are placed in `invalid_with_msg` with their expected reasons ("Non-witness UTXO does not match outpoint hash" and "Input specifies output index that does not exist") rather than in `invalid` as upstream does. This confirms they still fail for their intended reasons and not on an unrelated parse error. - The existing analyzer and wallet tests are updated to expect the earlier decode error. ## How Has This Been Tested? On macOS arm64 with the existing depends prefix: - Built `dashd`, `dash-cli` and `dash-wallet`. - `rpc_psbt.py` passed in both `--legacy-wallet` and `--descriptors` variants; both fixtures fail with their intended specific reasons. - Offline comparison against the exact upstream fixtures verified the normalization, txids and distinct rejection cases. ## Breaking Changes PSBTs containing nonexistent previous-output references now fail deserialization with RPC error `-22`, including `analyzepsbt` and `walletprocesspsbt`. Valid PSBT behavior and consensus rules are unchanged. ## 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: 9d213b13b1022fa45203c42fd22eb8b5196a510637163d21b8578f93d530a177fbf626753989ceb3c8bec3674dd65671f2194642b17cfd8fb17f2b091cb8bf30 (cherry picked from commit b2f6d4b)
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 6c02b2b) · triage: critical |
PastaPastaPasta
force-pushed
the
backport/v24.0.0-rc.4
branch
from
October 16, 2026 20:06
e8d6880 to
e2da323
Compare
… move the funder's coins 520a7e3 fix(qt): always show the requested change of a key rotation (pasta) 6116d4d fix(evo): refuse to sign a shared registrar update that moves the funder's coins (pasta) Pull request description: ## Issue being fixed or feature implemented A shared masternode key rotation (`ProUpSharedRegTx`) is approved by every share owner, but those approvals sign only the payload: the new operator/voting keys plus `inputsHash`, which covers the input outpoints. Nothing covers the outputs. `protx shared_combine` then re-signs the fee inputs with the calling wallet and, with `submit=true`, broadcasts the result. This lets a co-owner steal from another owner. Coin outpoints are public, so the attacker builds a rotation request that spends one of the victim's coins as its "fee input" and sends almost all of the value to themselves. They add their own approval and pass the request on. The victim's wallet can sign every input, so the Qt dialog (`RotateSharedKeysDialog::preparedByThisWallet()`) concludes the victim prepared the request. That hides the decoded key-change summary and enables Send. Clicking Approve and then Send signs and broadcasts the theft. The same trick can also rotate keys without showing the user what changes. The feature has not shipped yet. ## What was done? - **`CombineShared` (registrar-update path):** before re-signing the fee inputs, require the shape that funding gives the transaction: - every input is an unlocked coin of this wallet, and all of them belong to one address (the fee source). Masternode collaterals are locked, and funding never selects a locked coin; this stops a request that "returns" a collateral to its own address from spending it; - every output that carries value pays back to that address (the zero-valued `OP_RETURN` marker that `fundTransaction` leaves when there is no change is allowed); - the fee (inputs minus outputs) is no more than the wallet's `-maxtxfee`. A registrar update is now always combined to completion on the funding wallet. The partial-signing path is gone for this type: it let a co-owner who paid themselves combine the request and leave the funder only an input to sign. The RPC help used to suggest finishing such hex with `signrawtransactionwithwallet`; it now describes the new rule instead. - **`RotateSharedKeysDialog::showRequestedChange()`:** show the decoded operator/voting change whenever a request is loaded. Whether this wallet prepared the request is only a guess, and it no longer decides what the user sees. It still controls which editing controls and send hints appear: a wrong guess there at worst produces one of the errors above. The Send failure hint is reworded to match the new error. ## How Has This Been Tested? `feature_masternode_shares.py`, in the existing "unanimous registrar signing works across wallets" scenario, with a fully approved request whose outputs were changed after approval: - the funding wallet refuses outputs rewritten to another wallet's address, or to a different address of its own; - the funding wallet refuses the genuine request while its fee input is locked; - a co-owner whose own address was put in the outputs cannot combine it with `submit=false`; - the funding wallet refuses a request whose change was reduced so the difference goes to fees; - a non-funding wallet is refused with both `submit=true` and `submit=false` (expected message updated); - a real no-change preparation keeps only the zero-valued marker output, combines, and is accepted by `testmempoolaccept`. Run locally on macOS arm64 (non-debug build). The test passes. With each new guard disabled in turn (input ownership, locked coins, single address, fee ceiling, marker exemption), the test fails at the matching assertion. With the whole check reverted, the funding wallet signs and broadcasts the redirected transaction. `test_dash-qt` passes; `rotationSenderComesFromTheInputs` now expects the summary to be visible for a locally prepared request too. The dialog was not exercised interactively. ## Breaking Changes `protx shared_combine` on a `ProUpSharedRegTx` is now refused unless its inputs are unlocked coins of a single address of the calling wallet, any change returns to that address, and the fee is within `-maxtxfee`. It no longer returns partially signed hex for another wallet to finish. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: d5a9508b300dd3d471cb61203471b0fd24a5dbf6f1e5cfaa262b3b6394d7e4c02795a05a129dbb822835cf001f13e7303c3df656cb0a14014f498ee954f254b3 (cherry picked from commit 33670ce)
18484d1 fix(instantsend): forget the seen time of txs that are no longer tracked (pasta) 09703a4 doc: add release note for dashpay#7858 (pasta) 8fa5ae4 fix(instantsend): do not lock provider transactions (pasta) 2f33a23 test: sync mempools before burying a tx (pasta) Pull request description: ## Issue being fixed or feature implemented Provider transactions (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx and the new v24 provider types) can be removed from the mempool for conflicts that are not input conflicts: a duplicate collateral, owner/operator key, service address or proTxHash. An InstantSend lock on such a transaction therefore can't be relied on the way a lock on a normal transaction can. Masternodes should stop locking them. ## What was done? - `CheckCanLock(tx)` refuses all provider special transaction types, so masternodes no longer cast input-lock votes for them, and `TrySignInstantSendLock` won't sign an islock for one even when the input sigs were recovered by other signers. - A provider transaction that isn't locked is an unlocked parent like any other, so transactions spending its outputs become lockable once it is mined. A provider transaction that still gets a lock from signers without this change keeps it, and its children stay lockable as they are today. - This only changes what masternodes sign. Islocks for provider transactions received from other signers are still processed as before, so nodes that upgrade and nodes that don't keep the same view of locks during the transition. - Provider transactions are now confirmed the way unlocked transactions already are: miners include them after they have been in the mempool for 10 minutes, and the block is ChainLocked once its transactions are old enough. Mined provider transactions are not retroactively locked either, so if signers first see a provider transaction in a block, the ChainLock on that block waits for the same 10 minutes. This is a deliberate tradeoff. - With StatsD or `-debug=instantsend` active, the time a transaction was first seen is now forgotten once it stops being tracked. Before, a transaction that was mined without a lock and then ChainLocked kept its entry until restart, and every provider transaction now takes that path. dashpay#7839 already dropped the entry when a transaction leaves the mempool. - `bury_tx` in the test framework now syncs mempools before bumping mocktime. ChainLock signers measure a transaction's age from when they first saw it, so a provider transaction relayed after the bump would hold up ChainLocks in tests now that it isn't locked. ## How Has This Been Tested? - New `test_provider_tx_not_locked` case in `p2p_instantsend.py`: a ProUpServTx is not locked on any node while a later normal transaction is, and once it is mined after 10 minutes the block is ChainLocked. The case fails without the `signing.cpp` change (the ProTx gets locked) and passes with it. - Ran locally on macOS (arm64): p2p_instantsend.py, feature_llmq_chainlocks.py, feature_llmq_is_retroactive.py, feature_dip3_deterministicmns.py, feature_asset_locks.py, feature_masternode_payout_shares.py, feature_protx_version.py, feature_evo_shares_activation.py, feature_llmq_evo.py, feature_llmq_signing.py (with and without --spork21), feature_llmq_singlenode.py, feature_masternode_shares.py, p2p_platform_ban.py, rpc_netinfo.py, rpc_masternode.py, wallet_dash_rpcs.py, feature_llmq_simplepose.py (with and without --disable-spork23). feature_llmq_chainlocks.py failed before the `bury_tx` change and passes with it. - After rebasing onto develop with dashpay#7839, reran the full unit suite and p2p_instantsend.py, feature_llmq_is_retroactive.py, feature_asset_locks.py, feature_llmq_chainlocks.py, feature_llmq_is_cl_conflicts.py, interface_zmq_dash.py, feature_dip3_deterministicmns.py and mempool_limit.py; all pass. The seen-time map has no public accessor, so it has no unit test; functional tests run with `-debug`, so the code path is active in them. ## Breaking Changes Provider transactions no longer receive InstantSend locks from upgraded masternodes and take about 10 minutes to be mined and ChainLocked. Software that waits for an islock on a provider transaction should wait for a ChainLock or confirmations instead. There are no consensus changes. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation (release note) - [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) ACKs for top commit: knst: utACK 18484d1 Tree-SHA512: 9891883263ab67152d751765fd6270ea634c195a1757150c3ea162614706fbbddce78adf59d456572416965f67515084122af65de68da16cf26cea684913ac24 (cherry picked from commit a5e863a)
…wnership b3e27d9 backport: partial bitcoin#34156 restore only owned directories (pasta) Pull request description: ## Issue being fixed or feature implemented A filesystem exception while checking a restore backup can reach failure cleanup before the destination directory was created. An authorized `restorewallet` request can consequently delete an existing node-writable destination. Cleanup must only remove a directory owned by the current restore attempt. ## What was done? Partially adapts Bitcoin PR bitcoin#34156, specifically commit `4ed0693a3f2a427ef9e7ad016930ec29fa244995`: track successful destination creation and gate cleanup on that ownership. Set an explicit load-failure status in the exception handler so RPC callers do not inspect an uninitialized status. Dash still rejects existing destinations and always loads the restored wallet. The copied-file tracking, empty-directory assertion, and nonrecursive removal in upstream's commit are omitted: failed BDB verification can create `.walletlock`, `database/`, and `db.log` in a newly created directory, so Dash retains recursive cleanup exclusively for that owned directory. The remaining migration/restore-in-existing-directory changes in Bitcoin bitcoin#34156 are outside this partial backport. No upstream tests are claimed as backported; adapted functional coverage exercises Dash's restore behavior. ## How Has This Been Tested? On macOS arm64 with the existing depends-backed BDB/SQLite/Qt build: - `make -j1` passed. - `wallet_backup.py --legacy-wallet` and `wallet_backup.py --descriptors` passed, including restored balances, missing-backup/existing-destination controls, and new relative/absolute temporary-directory preservation checks. - The new preservation regression failed with the baseline binary because the nested sentinel was deleted after the early filesystem exception. The fixed code preserves it and returns a wallet error. - A truncated, recognized BDB backup fails verification and the newly created restore directory is removed, retaining Dash's auxiliary-file cleanup. - `test_dash --run_test=walletdb_tests`, whitespace lint, targeted repository Python lint, and `git diff --check` passed. The filesystem exception regression uses an overlong path component in temporary test data. It does not operate on an existing real wallet; filesystem-error details can vary by platform. ## Breaking Changes None. Existing restore destinations remain rejected. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation (no documentation change required) - [ ] I have assigned this pull request to a milestone This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: 8f96dbd792c42ddea6005c9198153c274bd72ef5983fc459cf899ee9c0c00945bd383a30f17dbcad420cf0ff279d1adf1bb0c436c60aec81a0ec5a05464341d3 (cherry picked from commit 66c7308)
…had been broken while 2e49361 fix: due to logical conflicts feature_pruning.py had been broken while (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Test feature_pruning.py is broken awhile. Discovered when testing dashpay#7861 ## What was done? Added missing args for restarted nodes. ## How Has This Been Tested? Run test/functional/feature_pruning.py directly ## Breaking Changes N/A ## 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)_ Top commit has no ACKs. Tree-SHA512: 98b13451c4f99e3e810ffc91095c3b9e97eca5a5ac3d4ea7e3b638a2828b992ba853cbad03185b928d60a337ed8ab7616fe9f4348151b25b860ef83d69b77b77 (cherry picked from commit cf91169)
…ing mnsync On test networks a node only fetched sporks by sending GETSPORKS from the mnsync tick. That request was skipped once mnsync finished, skipped for any peer address with a still-valid "spork-sync" entry in netfulfilled.dat (which survives restarts), and skipped for inbound peers on an active masternode. A node that came up without sporks, e.g. after the sporks.dat format change in v24, could therefore finish mnsync without them and never ask again, leaving DKG and ChainLocks disabled until `mnsync reset`. Announce every active spork as an inv to each peer when the handshake completes, in both directions. Like spork relay, this includes masternode connections, which only become relay-capable after MNAUTH. Peers request the ones they lack with getdata, and a node that newly accepts a spork already relays it, so the spork set converges without any opt-in or sync state. The GETSPORKS handler is kept so v23 nodes can still ask us. Fixes dashpay#7891 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 45acdf3) Backport notes: develop merged dashpay#7893 twice. The first merge (f99212b) carried an earlier revision of this commit, and the second merge (72a5997) carried this one, so develop now announces sporks in two places in the VERACK handler. Only the PR's final commit is picked here. Conflict in src/node/sync_manager.cpp: on this branch the mnsync tick still sent GETSPORKS on every network, because dashpay#7847 (which limited it to test chains) is not included. The request is removed entirely, as on develop. Mainnet spork values are hardcoded in CSporkManager::GetSporkValue, so the mainnet request had no effect.
…led upgrade instead creating brand-new scenario (cherry picked from commit 4a19742)
…d derivation truncates 6051735 fix(wallet): reject mnemonic passphrases that seed derivation truncates (pasta) Pull request description: ## Issue being fixed or feature implemented Seed derivation (`CMnemonic::ToSeed`) uses the BIP39 salt `"mnemonic"` followed by the passphrase. History: - **Up to 19.1.0** the full salt was used. A very long passphrase could crash the node. - **19.2.0** (backport of d53e657, "truncate mnemonic passphrase instead of crashing") started cutting the salt at 256 bytes with `strnlen`. Every passphrase byte after the 248th has been silently ignored since then, so two passphrases that differ only after that point derive the same seed. A user who picks a long passphrase believes all of it protects the seed, but it doesn't. - **Before 23.0.0** the `strnlen` cut also stopped at the first NUL byte. 7f53b9f fixed that for v23. Fixing `ToSeed` to use the full salt would bring back the pre-19.2.0 derivation. That breaks mnemonic recovery for every wallet created on 19.2.0 or later with a long passphrase. Keeping the cap preserves those wallets, but wallets created before 19.2.0 with a long passphrase already can't be recovered from their mnemonic on current software, with or without any option. Whichever derivation is chosen, someone's mnemonic stops reproducing their keys. This PR therefore leaves `ToSeed` unchanged and rejects passphrases that it would truncate when a new seed is created. ## What was done? - `upgradetohd` and `createwallet` (RPC and GUI, using `-mnemonicpassphrase`) reject a mnemonic passphrase longer than 248 bytes. Both checks run before the wallet is modified or its database is created. - A new startup option, `-allow-deprecated-long-mnemonics`, raises that limit to 256 bytes so a wallet created with a passphrase of 249 to 256 bytes can still be recovered with the same (truncated) derivation. Passphrases over 256 bytes are rejected by both checks even with the option, because `CHDChain::SetMnemonic` throws for them. - The `createwallet` check is unconditional, so a `-mnemonicpassphrase` longer than 248 bytes is also refused on a node that would ignore it (`-usehd=0` or `-hdseed`). That is deliberate: it keeps the guard simple, and such a value is a misconfiguration that would otherwise be silently ignored. - Existing wallets, `ToSeed`, loading, restore and migration are unchanged. ## How Has This Been Tested? `wallet_upgradetohd.py` (legacy and descriptor modes) gains a short case: a 248-byte passphrase is accepted by `upgradetohd` and kept, a 249-byte passphrase is rejected by `upgradetohd` and by `createwallet` with `-mnemonicpassphrase`, and after a restart with `-allow-deprecated-long-mnemonics` both `createwallet` (same wallet name, so the rejection left nothing behind) and `upgradetohd` accept it and store the full passphrase, while a 257-byte passphrase is still rejected by `upgradetohd` with the option set. Also ran `wallet_createwallet.py`, `wallet_mnemonicbits.py`, `rpc_help.py`, and `test_dash --run_test=bip39_tests`. ## Breaking Changes Creating an HD wallet with a mnemonic passphrase longer than 248 bytes now fails unless `-allow-deprecated-long-mnemonics` is set, and one longer than 256 bytes always fails. Existing wallets are not affected. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation - [ ] 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: da4b2cf7dbae37db67df7374814106621f5e53b813a5d8acbed4a5e909ae51a108d2d9a0bf71640710c0ebfceb1d27ce7500ddb8f403fe91b22eee9f04b7193c (cherry picked from commit 8e4cd3d)
…ge of its masternode 64b828d test: mine a pending service update before a shared masternode's key change (pasta) 7645a26 docs: add release note for mining service updates before key changes (UdjinM6) f2c84a8 test: block assembly mines a pending ProUpServTx before a key change of its masternode (UdjinM6) 0295a51 fix(mining): mine a pending ProUpServTx before a key change of its masternode (UdjinM6) 89c40ec fix: move the masternode list tip before a reorg re-adds transactions to the mempool (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Fixes dashpay#7582 at the block assembly level, as an alternative to dashpay#7728. `CheckProUpServTx()` verifies the operator signature against the masternode list of the previous block, and block connection applies provider transactions in block order without checking again who signed them. A `ProUpServTx` signed with the operator key that an earlier transaction in the same block changes (`ProUpRegTx` or `ProUpSharedRegTx` with a new operator key) or revokes (`ProUpRevTx`) therefore still applies: it writes the previous operator's service fields back and, after a key change, revives the masternode the change just PoSe-banned. The mempool accepts both transactions together, and block assembly orders them by feerate, so a displaced operator can broadcast a low-feerate service update and wait for the key change to be mined. The reverse order is harmless: the key change resets the operator fields the service update set. This PR makes honest block assembly always produce that order, instead of keeping the two transactions out of the mempool as dashpay#7728 does. Compared to dashpay#7728, no valid service update the operator already broadcast is evicted or refused for being paired with a key change, and mempool contents do not depend on the order in which a node received the two transactions. Consensus is unchanged: a block with the harmful order is still valid, so a miner ignoring this policy can still produce one, as with dashpay#7728. ## What was done? - **Block assembly** (`BlockAssembler::addPackageTxs`): when a package contains an operator key change or revocation, the masternode's pending `ProUpServTx`s, with their unconfirmed ancestors, are pulled into the package and placed before it. New helper `CTxMemPool::GetServiceUpdatesBeforeKeyChange()`. - A service update is left out if its own package fails the usual transaction checks, if it is one the mempool would refuse as unorderable (see below; the mempool rejects those on entry, but a reorg can return such ancestors below an update already in the pool), or if its package has a service update of a masternode whose key already changed in the block. - If the pulled-in updates do not fit, they are all left out and the key change goes in without them, so pending updates, however many, cannot hold it back. - Once a key change is in the template, no later `ProUpServTx` of that masternode is added, so an update left out, or one that only becomes eligible while the template is built, cannot follow it. - Whether a transaction is a key change is decided by `CTxMemPool::GetKeyChangeTarget()` against the current tip list, not when the transaction entered the mempool, so a registrar update that becomes a key change after a reorg is handled too. - **Mempool policy**: a `ProUpServTx` that block assembly could not reliably put ahead of a key change is rejected with `protx-dup` (`CTxMemPool::IsUnorderableServiceUpdate()`, called next to `existsProviderTxConflict()`): - **With a pending operator key change among its ancestors** (it spends one, directly or through other pending transactions). It can only be mined after that change. For its own masternode it would restore the previous operator's fields. For another masternode, pulling it in would require that other change. - **With a pending asset lock, asset unlock or MNHF signal among its ancestors.** Their own checks during block assembly could keep the pulled-in update, and the key change with it, out of the block. Such a transaction can only arrive after the ancestor it spends, so every node makes the same decision. Package acceptance checks package members against the pool before adding any of them, so a package carrying both transactions is not caught. Packages are regtest-only. - **Reorg**: `MaybeUpdateMempoolForReorg()` moves the deterministic masternode list tip to the new chain tip before re-adding disconnected transactions. Before, a key change returned to the mempool was compared with the list of the disconnected block, which already had the new key, and was not recognized as a key change. This change was first proposed in dashpay#7728 and is kept as its own commit. - Release note `doc/release-notes-7857.md`. This keeps pending provider transactions in the mempool and only changes their order in block templates, so it does not depend on how they were relayed. ## How Has This Been Tested? Built on macOS arm64 with `--enable-debug --enable-werror --enable-crash-hooks`. New unit tests in `src/test/evo_deterministicmns_tests.cpp`, using block templates unless noted: - `test_miner_service_update_before_key_change`: - a low-fee `ProUpServTx` is placed before a high-fee key change or revocation of its masternode; - a registrar update that keeps the operator key is ordered by fee as usual; - a `ProUpServTx` that cannot be mined (it takes another masternode's address) does not hold the key change back. - `test_mempool_rejects_unorderable_service_update`, through `ProcessTransaction`: a `ProUpServTx` is rejected with `protx-dup` when funded from a pending key change of its masternode, from a transaction spending one, from a pending key change of another masternode, or from a pending asset lock. An independently funded one is accepted. - `test_miner_service_update_before_reorged_key_change`: a key change returned to the mempool by `InvalidateBlock` still counts as the masternode's pending key change, so a second one is rejected with `protx-dup`, and a pending `ProUpServTx` is still placed before it. - `test_miner_service_update_before_key_change_surviving_reorg`: a registrar update that kept the operator key when it entered the mempool becomes a key change after a reorg, and a service update signed with the restored key still goes in before it. - `test_miner_service_update_never_after_key_change`: a `ProUpServTx` that follows a key change in the pool (it spends it) is never mined after it, while one following a registrar update that keeps the key still is. - `test_miner_key_change_not_held_back_after_reorg`: a reorg returns a zero-fee key change of another masternode below a pending `ProUpServTx`; the key change of the update's own masternode is mined and the update is left out. - `test_miner_service_update_with_parent_before_key_change`: a `ProUpServTx` funded by an ordinary pending transaction goes in as parent, service update, key change. - `test_miner_key_change_without_room_for_service_update`: with room for either the parent and the service update or the key change, but not all three, the key change goes in without the service update, and the template stays within its size limit. - `test_miner_key_change_not_held_back_by_service_updates`: two independently funded `ProUpServTx`s that fit ahead of a key change only one at a time; the key change goes in and neither update does. - `test_miner_key_change_not_held_back_by_other_key_change`: a key change of masternode A goes in without A's service updates; a service update of masternode B funded by one of them is left out instead of holding back B's key change. Reverting each piece in turn makes the corresponding case fail: - the pull-in in block assembly (both ordering tests); - the reorg tip move (only the reorg test); - limiting the rejection to the update's own masternode (the other-masternode case); - removing asset locks from the rejection (the asset-lock case); - leaving unorderable service updates in the pull-in (`test_miner_key_change_not_held_back_after_reorg`); - holding the key change back when the pulled-in updates do not fit (both block-space tests); - pulling in an update package with a service update of a masternode whose key already changed in the block (`test_miner_key_change_not_held_back_by_other_key_change`). Run at the head of this PR: - **Unit tests:** all pass (`test_dash`, 1071 cases). - **Functional tests:** all pass: `feature_asset_locks.py`, `feature_dip3_deterministicmns.py --descriptors`, `feature_llmq_chainlocks.py`, `feature_masternode_shares.py`, `feature_protx_version.py`, `mempool_packages.py`, `mempool_reorg.py`, `mining_basic.py`, `rpc_masternode.py`, `rpc_packages.py`. - **Lint:** `test/lint/all-lint.py` reports only an existing cppcheck warning in `src/governance/object.cpp`, which this PR does not touch. ## Breaking Changes None for consensus. Mempool policy: a `ProUpServTx` spending an output of a pending operator key change, asset lock, asset unlock or MNHF signal is rejected until that transaction confirms. It can then be sent again, signed with the operator key current at that point. Mining policy: pending service updates of a masternode that do not fit in the block ahead of its key change are left out of that block, and become invalid once the key change confirms. ## 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) ACKs for top commit: knst: utACK 64b828d Tree-SHA512: 59bd24403894100976ef6388b1efd95df53a83c8d45101a70d37083865884c1d8786c58d737518783deee8a403bba02ecef15db5c7706e9dbb0db68c6da83f6f (cherry picked from commit cd7520e) Backport notes: develop folded CChainstateHelper into Chainstate in d5c1bd7, which is not on this branch. Here MaybeUpdateMempoolForReorg reaches the special tx processor through ChainHelper().special_tx instead of m_special_tx. In src/evo/specialtxman.{h,cpp}, only UpdatedMNListTip is added. GetDeterministicMNListHash, which sits next to it on develop, belongs to unrelated develop-only work. In src/test/evo_deterministicmns_tests.cpp, the branch's evo/chainhelper.h include is kept next to the new evo/assetlocktx.h.
…Ds as one conflict namespace 43ae7ac fix: treat pending owner keys and Platform node IDs as one namespace (pasta) Pull request description: ## Issue being fixed or feature implemented The deterministic masternode list keeps owner key IDs and Platform node IDs in one uniqueness map. It is keyed by `SerializeHash`, and a `CKeyID` serializes exactly like a `uint160` with the same bytes. So once one masternode uses a 160-bit value as its owner key ID, no masternode can use it as a Platform node ID, and vice versa. The mempool tracks the two fields in separate maps (`mapProTxPubKeyIDs`, `mapProTxPlatformNodeIDs`) and checks each field only against its own map. Two pending provider transactions can therefore claim the same value, one as an owner key and one as a Platform node ID. Each is valid against the tip list and both are admitted. If block assembly selects both, `BuildNewListFromBlock` rejects the pair and `CreateNewBlock` / `getblocktemplate` fails until one of them leaves the mempool. The same applies when a block confirms one claim while the other is still pending: the pending one can no longer be mined, but nothing evicts it. Both transactions must be funded, collateral-backed provider transactions, so the cost is not trivial. A mempool that can stop template creation is still worth closing, and the fix is policy-only. ## What was done? `src/txmempool.cpp`. Consensus rules are unchanged. - `existsProviderTxConflict`: - A `ProRegTx` owner key, or each share owner key for a shared registration, conflicts with a pending owner key *or* a pending Platform node ID of the same value. - An EvoNode `ProRegTx` or `ProUpServTx` Platform node ID conflicts with a pending Platform node ID *or* a pending owner key of the same value. - A registration that uses one value for both its own owner key and its own Platform node ID is still accepted. The masternode list allows that within one masternode. - `removeProTxPubKeyConflicts(tx, CKeyID)` also evicts a pending transaction claiming the value as a Platform node ID. `removeProTxPlatformNodeIDConflicts` also evicts one claiming it as an owner key. A confirmed claim therefore removes the pending claim in the other field, together with its descendants. Not covered here: conflicts between provider transactions inside one submitted package. Package `PreChecks` run before any package transaction is in the mempool maps, so same-field conflicts inside a package aren't caught either. That gap predates this change, and `submitpackage` is a test-only RPC. ## How Has This Been Tested? macOS arm64, depends build. The coverage extends the existing `FuncTestMempoolDualProregtx` scenario (`evo_dip3_activation_tests/test_mempool_dual_proregtx_legacy` and `_basic`) in `src/test/evo_deterministicmns_tests.cpp`. It uses the public mempool API (`addUnchecked`, `existsProviderTxConflict`, `removeForBlock`) with unsigned provider payloads. It checks: - Non-shared registrations: a pending owner key blocks a `ProRegTx` and a `ProUpServTx` that claim the same value as their Platform node ID, and a pending Platform node ID blocks a `ProRegTx` owner key of the same value. An unrelated Platform node ID doesn't conflict. - Eviction: a confirmed owner key evicts a pending Platform node ID claim, and a confirmed Platform node ID evicts a pending owner key claim. - Shared registrations, in both directions: a share owner key colliding with a pending Platform node ID is rejected, and a Platform node ID colliding with a pending share owner key is rejected. Confirming either one evicts the other. The registrations use distinct operator keys, no addresses and internal collateral, so only the cross-namespace collision can conflict. - Negative case: an otherwise non-conflicting EvoNode registration that uses the same value for its own owner key and Platform node ID is accepted, with an unrelated registration pending. Mutation checks: - Restoring the owner-only lookup (`mapProTxPubKeyIDs.count`) in the shares branch of `existsProviderTxConflict` makes the shared-registration admission check fail in both test cases. - Making an EvoNode registration conflict with its own owner key equal to its Platform node ID makes the negative check fail in both test cases. All of `evo_dip3_activation_tests` (42 cases) and the other mempool unit tests pass. `git diff --check` and `lint-whitespace.py` are clean. Limits: no functional test submits two funded, collateral-backed provider transactions and then calls `getblocktemplate`. ## Breaking Changes None. Provider transactions whose owner key and Platform node ID collide across two pending transactions are now rejected from the mempool, or evicted, instead of breaking block assembly. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) ACKs for top commit: knst: utACK 43ae7ac Tree-SHA512: a410e026a0790685fdd0a4ad55c76a9586577c037929d5010706848836bf967a78b4cd04b6611c7fe1bcc88655693c552bd86b61ac3c84ad89f21752caf65752 (cherry picked from commit 41fdf14)
…wording 52cf9ae test: cover retroactive refusal of provider tx InstantSend locks (UdjinM6) 95fed01 doc: fix when children of provider txs become lockable (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Follow-up to dashpay#7858, which stopped masternodes from InstantSend-locking provider transactions. `test_provider_tx_not_locked` had gaps: - The mempool case checked that the ProTx had no lock right after an unrelated later transaction was locked. That is a timing proxy: a regression where the ProTx lock arrives slightly later than the other lock would still pass. - A provider transaction that signers first see in a block (`NetInstantSend::BlockConnected` → `ProcessTx(tx, /*fRetroactive=*/true)`) was not covered. The existing test passed a mutation that disabled the provider checks only for retroactive processing. - The release note said transactions spending provider outputs can be locked once the provider transaction is mined. A mined parent whose block is not ChainLocked is only accepted after `nInstantSendConfirmationsRequired` confirmations, and its children are retried when its block is ChainLocked, so "once the block that includes the provider transaction is ChainLocked" is the accurate condition. Nothing tested this. ## What was done? `doc/release-notes-7858.md`: children of provider transactions can be locked once the block that includes the parent is ChainLocked, not once the parent is mined. Test changes are in `test_provider_tx_not_locked` in `p2p_instantsend.py`: - Mempool case: the ProTx is built with `submit=False` and broadcast inside `assert_debug_log` on the masternode, which waits for the txid-specific `CheckCanLock -- txid=…: provider txs are not locked` and `ProcessTx -- txid=…: CheckCanLock returned false` lines. These confirm that `ProcessTx` returned before input signing, rather than inferring it from another transaction's lock. A redundant `sync_mempools()` is removed. - New case: node 0 is isolated, a ProUpServTx and a normal transaction are mined there, and node 0 then reconnects, so the masternode first sees both in a block. The same log barrier, taken around the reconnect, proves the retroactive path refused the ProTx. The test then waits for the normal transaction's retroactive `instantlock_internal` as a positive control, and checks that the ProTx has no lock and the block is not yet ChainLocked. - Child check, in the same case: while that block is mined (one confirmation) but not ChainLocked, a child spending the ProTx output is broadcast inside a log barrier waiting for `CheckCanLock -- txid=<child>: outpoint <protx>-0 too new and not ChainLocked`, and has no `instantlock_internal` on any node. Mocktime is then advanced so the same block is ChainLocked without mining another one, and the child must get `instantlock_internal` on all nodes while the tip is unchanged. This covers both the refusal before the ChainLock and the retry of pending children when it arrives. The normal-transaction and child checks poll `instantlock_internal` to require an InstantSend lock; `instantlock` is also true when the transaction's block is ChainLocked. ## How Has This Been Tested? - `p2p_instantsend.py` passed repeatedly on macOS (arm64) with an `--enable-debug` build, on top of current develop. A full GitHub CI run passed on an earlier revision of this branch. - Mutation checks, each with a rebuilt `dashd`: - Skip the provider refusal only when `fRetroactive` is set and drop the `TrySignInstantSendLock` guard: the retroactive case fails at its log barrier. The original test passed under this mutation. - Drop the provider refusal in `CheckCanLock(tx)` and the `TrySignInstantSendLock` guard entirely: the mempool case fails at its log barrier. - Don't retry children when the parent's block is ChainLocked (`RemoveNonLockedTx(txid, false)` in `RemoveConfirmedInstantSendLocks`): the child never gets locked and the test fails. - Drop the "too new and not ChainLocked" refusal in `CheckCanLock(outpoint)`: the child check fails at its log barrier. - The child check would also pass before dashpay#7858. It guards against breaking the descendants of provider transactions, not against reverting the refusal. - These tests don't exercise the `TrySignInstantSendLock` provider guard on its own; `CheckCanLock(tx)` refuses the provider transaction before input signing. ## Breaking Changes None. Test and release-note changes only. ## 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 _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 056ab842e7163e42973e62237c248d65e51023496c75def7434536cc2579b2029d66d98e3f52053ec1aa8af1d05afb1be27ee6fb9b50d75ab4cdb8153c46e6b5 (cherry picked from commit 4461504)
…ally during block assembly d9d3704 test: block assembly leaves out a provider package that conflicts with an earlier one (UdjinM6) dae84b9 feat(mining): build the masternode list incrementally during block assembly (UdjinM6) cb07b73 refactor: split RebuildListFromBlock into per-block, per-tx and final phases (UdjinM6) Pull request description: ## Issue being fixed or feature implemented In-block conflicts between provider transactions can make the masternode-list build after transaction selection fail, causing `CreateNewBlock` and `getblocktemplate` to fail. Block assembly should skip packages that conflict with the list built so far and continue selecting other transactions. ## What was done? - Split `CSpecialTxProcessor::RebuildListFromBlock` into `BeginListForBlock`, `ApplyTxToList` and `FinishListForBlock`, preserving the existing list-transition logic and validation rules. Add `CMNListBuilder` to drive these phases incrementally; its list copies share persistent maps. - When DIP3 is active, `CreateNewBlock` starts the builder from the tip's list and applies the quorum commitments before selecting mempool transactions. For each eligible package containing a special transaction, `addPackageTxs` applies its transactions in block order to a copy of the builder. A failed application or `std::runtime_error` causes the package to be skipped. Existing ordering, EHF-signal and current-minability checks precede this trial; credit-pool checks follow it. The copy replaces the builder only after all package checks pass. - Use `Finish()` to scan the complete block for collateral spends and perform payee bookkeeping, producing the final list for the CbTx masternode-list merkle root. - Add `template_skips_conflicting_provider_package` to cover conflict handling and discarded package trials. No consensus rule changes. ## How Has This Been Tested? macOS arm64, depends build, `--enable-debug --enable-werror`. New unit test `evo_dip3_activation_tests/template_skips_conflicting_provider_package` checks that: - Template creation succeeds, keeping the first selected conflicting transaction and an unrelated transaction while excluding the other conflicting transaction and its descendant. - A transaction the list rejects by throwing (a duplicate `platformNodeID`, which `AddMN` reports with an exception) is left out the same way. - A rejected package does not prevent selection of a later transaction that would conflict only with that rejected trial. The rejected package's transactions remain excluded. The test checks the ancestor-feerate ranking needed to exercise this case before assembly. Local regression checks: without the `feat(mining)` commit, `CreateNewBlock` fails; without the `catch`, `CreateNewBlock` throws; committing a rejected builder trial makes the test fail. Also checked locally, with a check not included in this PR: a simulated later credit-pool rejection discards the trial's masternode-list changes. Also run: - Unit suites: `evo_*`, `miner_tests`, `mempool_tests`, `txvalidation_tests`, `validation_block_tests`, `block_reward_reallocation_tests`. - Functional tests: `feature_dip3_deterministicmns.py`, `feature_masternode_shares.py`, `rpc_netinfo.py`, `mining_*.py`, `feature_llmq_simplepose.py`, `feature_llmq_evo.py`, `feature_llmq_rotation.py`, `feature_asset_locks.py`, `feature_mnehf.py`, `rpc_masternode.py`, `p2p_instantsend.py`. - `test/lint/all-lint.py`: only a pre-existing cppcheck warning in `src/governance/object.cpp`, which this PR does not modify. ## 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 Top commit has no ACKs. Tree-SHA512: e7a7d894449ba5738d044ab70a853ece979d18fd24e44882a2b8017aa1767cd92ca4b7c032b38c2b6a928c8b6c582ea450fbbd3563a0db56dc10a02e145674fb (cherry picked from commit 8a17a55) Backport notes: develop folded CChainstateHelper into Chainstate in d5c1bd7, which is not on this branch, so BlockAssembler builds its CMNListBuilder from *m_chain_helper.special_tx instead of m_chainstate.m_special_tx. In src/evo/specialtxman.cpp, GetDeterministicMNListHash, which sits next to the new code on develop, belongs to unrelated develop-only work and is left out.
…oinJoin server a231f05 fix(coinjoin): apply a DSSIGNFINALTX message atomically (pasta) 873295f fix(coinjoin): bind completion tail to session snapshot (pasta) d1e23c7 fix(coinjoin): prioritize progress over timeout reset (pasta) 0ba94a6 fix(coinjoin): serialize timeout transitions (pasta) 97daa21 fix(coinjoin): guard the session collaterals with cs_coinjoin (pasta) 3fdd82d fix(coinjoin): decide server state transitions under cs_coinjoin (pasta) Pull request description: ## Issue being fixed or feature implemented `CCoinJoinServer` state is touched by two threads: `msghand` (single-threaded, via `ProcessDS*`) and the 1s scheduler tick (`CheckForCompleteQueue` / `CheckPool` / `CheckTimeout`), plus RPC threads in `GetJsonInfo()`. `CheckPool()` runs on **both**: the scheduler tick and `msghand` via `ProcessDSVIN` / `ProcessDSSIGNFINALTX`. Several pieces of the original version of this PR have since landed on develop separately (dashpay#7597 `nSessionDenom` atomic, dashpay#7599 entry admission bound to the session snapshot, plus the single-snapshot `CheckPool()` and the session-bound `CreateFinalTransaction()` from the promotion/demotion work). This PR is now rebased on top of those and contains what is still missing: 1. **`CheckPool()` was not single-flight.** Two concurrent calls (scheduler + `msghand`) could both act on the same snapshot. If both finalize, clients receive `DSFINALTX` twice and sign twice, and the duplicate signatures make `AddScriptSig()` fail, which relays `STATUS_REJECTED` and aborts the session for everyone. Both calls could also commit the same final transaction. 2. **`CommitFinalTransaction()` was not bound to a session.** It committed whatever `finalMutableTransaction` held, even when the session had timed out and been reset after the decision. That pushes a cleared transaction through ATMP and notifies participants of a failure that never happened. 3. **`CheckTimeout()` raced finalization and commit.** It tested `HasTimedOut()` without a lock and could reset a session halfway through `CheckPool()` finalizing or committing it. It also reset sessions that could still advance, because the final collateral, entry or signature can arrive after the scheduler's `CheckPool()` snapshot. In that case `msghand` skips its own `CheckPool()` while the scheduler holds the guard. 4. **Offender selection and charging were not atomic with closing admission.** `ChargeFees()` sampled `nState` three times, so it could select "didn't send" offenders and then charge and log them as "didn't sign". On the timeout-finalize path it ran before eligibility was rechecked, so an ineligible session was charged once there and again by the `CheckTimeout()` reset that followed. 5. **The session collaterals were not `GUARDED_BY`.** `vecSessionCollaterals` was unannotated while its companion prevout index was guarded. `CreateNewSession()` still read it after releasing the lock, and nothing prevented the next unlocked read. 6. **The completion tail mutated live state.** If every participant had disconnected, `RelayCompletedTransaction()` could reset the session. A replacement session could then open before random charging and the unconditional tail reset ran, and those would charge or clear the replacement. The invalid-transaction path also reset the pool *before* notifying anyone, so `ERR_INVALID_TX` reached nobody. 7. **A `DSSIGNFINALTX` message was not applied atomically.** Each input went through its own locked `AddScriptSig()`. A signing timeout landing between two inputs of one already-received message would select the sender as a non-signer, charge them, and reset the session. ## What was done? Six commits, each standalone: **`fix(coinjoin): decide server state transitions under cs_coinjoin`**: addresses 1, 2 and the triple sample in 4. A new `cs_check_pool`, acquired **only** via `TRY_LOCK`, makes `CheckPool()` single-shot without ever blocking `msghand`. Signature completeness joins the existing pool snapshot. `CommitFinalTransaction()` takes the session id the decision was made for and revalidates it. `SetState()` and `IsSignaturesComplete()` now require `cs_coinjoin`, and `ChargeFees()` samples `nState` once. **`fix(coinjoin): guard the session collaterals with cs_coinjoin`**: addresses 5. The collateral transactions and their prevout index become a single `SessionCollaterals` member marked `GUARDED_BY(cs_coinjoin)`, so they cannot drift apart or be annotated inconsistently. Reintroducing an unlocked read is now a `-Wthread-safety` compile error. **`fix(coinjoin): serialize timeout transitions`**: addresses 3 (first half) and the rest of 4. `CheckTimeout()` uses the same `cs_check_pool` guard. Offender selection moves into `SelectCollateralToCharge()` under `cs_coinjoin`, in the same lock scope that resets the session (`CheckTimeout()`) or moves it to `SIGNING` (`CreateFinalTransaction(session_id, charge_fees)`). The timeout-finalize path only charges once the live session is confirmed eligible. Collateral is consumed after `cs_coinjoin` is released, because `ConsumeCollateral()` takes `cs_main`. The session may be reset before that consumption runs, so the selected collateral stays reserved in `m_collaterals_being_charged` (which survives `SetNull()`) until `ConsumeCollateral()` has finished. During that time `CreateNewSession()` and `AddUserToExistingSession()` refuse a dsa whose collateral spends one of its inputs. The charged participant therefore cannot be readmitted into the next session with a collateral that is about to be spent, and the fee is still taken. `AddEntry()`'s `ConsumeCollateralIfCurrentSession()` takes the same reservation. **`fix(coinjoin): prioritize progress over timeout reset`**: addresses 3 (second half). Before resetting, `CheckTimeout()` rechecks under `cs_coinjoin` whether the session can still advance: queue ready, an entry for every collateral, enough entries covering both sides of the session denomination, or every signature in. If so it leaves the session to the next tick. The rules mirror the decisions `CheckPool()` and `CreateFinalTransaction()` make, so a deferred session is always acted on. The decision is the pure function `ShouldResetTimedOutSession()`, which `CheckTimeout()` calls with the session's progress read under `cs_coinjoin`. **`fix(coinjoin): bind completion tail to session snapshot`**: addresses 6. The committed transaction, participants and collaterals are captured under `cs_coinjoin`. Completion notification is side-effect-free, and only the matching signing session is reset. The invalid-transaction path notifies the captured participants. `CheckPool()`'s uncovered-session reset captures its participants and resets in one step, then tells them. **`fix(coinjoin): apply a DSSIGNFINALTX message atomically`**: addresses 7. The signing session the sender was checked against is revalidated after decoding, and the whole batch is applied in one `cs_coinjoin` critical section. The rejection relay for a failed input happens inside that same section. ### Lock discipline No new `cs_coinjoin`-across-`cs_main` edge is introduced: offender collateral is consumed only after `cs_coinjoin` is released, and signature verification already ran under `cs_coinjoin`. `cs_check_pool` is `TRY_LOCK`-only, taken only by `CheckPool()` and `CheckTimeout()` while holding nothing else, so `cs_check_pool` → `cs_coinjoin` / `cs_main` is unidirectional and no ABBA cycle is possible. ## How Has This Been Tested? macOS arm64, depends build, clang with `-Wthread-safety -Werror`, rebased on develop c8bfd7a. - Every one of the six commits builds `test_dash` and passes `coinjoin_inouts_tests` on its own. - Unit suites `coinjoin_inouts_tests`, `coinjoin_tests`, `coinjoin_queue_tests`, `coinjoin_dstxmanager_tests` pass. - Functional `rpc_coinjoin.py` and `p2p_dstx.py` pass. - `lint-whitespace.py`, `lint-includes.py`, `lint-circular-dependencies.py` pass. New/updated unit tests in `coinjoin_inouts_tests`: - `server_timeout_resets_only_sessions_that_cannot_advance` is one table over `ShouldResetTimedOutSession()`. Rows that must be left alone: a session that has not timed out; a ready queue; an entry for every collateral (even with a side uncovered, which `CheckPool()` resets itself and tells the participants); enough covered entries with one straggler; every input signed. Rows that must be reset: a queue that is not ready; too few entries; enough entries with a side uncovered and one straggler; a missing signature. Mutations checked: removing the deferral, the signing-advance clause, either accepting-advance clause, or the side-coverage condition each fails at least one row. - `server_finalization_rechecks_live_side_coverage` now exercises the charging finalize path. The `cs_check_pool` single-flight guard, the completion-tail snapshot and the `DSSIGNFINALTX` atomicity change close timing windows that cannot be reproduced deterministically in a unit test without driving private synchronization state, so they have no dedicated test. The completion tail is bound to the session id and participant list captured under `cs_coinjoin`, and the reset checks both the session id and `nState`, which is visible in the diff. They were verified by inspection and by the existing `server_signfinaltx_*` tests, which still pass. The charge reservation is in the same position: its window lies between two lock scopes inside one call. It was verified by checking that every reservation is released exactly once, after `ConsumeCollateral()` returns, on every path; that admission checks it under the same `cs_coinjoin` scope that admits; by `-Wthread-safety -Werror`; and by an independent code review. ## Breaking Changes None. No protocol, serialization or RPC change. Two debug log lines have `vecSessionCollaterals.size():` renamed to `participants:`. ## 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 _(for repository code-owners and collaborators only)_ Top commit has no ACKs. Tree-SHA512: 0938a87650a83d76c71121b9defd3e0c2548f7749eaf0eb049a7c61a37344ac20011eba50a246d3bcdbcf5870f74d60b6b0fbe55578291de64f15537df353b2e (cherry picked from commit 6d5ffe8)
… strict netInfo decoding 516913a fix: return error netinfo has neither service nor domain entries (Konstantin Akimov) e3e7808 fix(evo): keep -allowprivatenet out of masternode address validation (Konstantin Akimov) 4714075 refactor(evo): compute ExtNetInfo flattened entries on demand (Konstantin Akimov) 567d146 fix(evo): require ascending known purposes in ExtNetInfo deserialization (Konstantin Akimov) 51a09b7 refactor(evo): drop entry validity checks made unreachable by strict decoding (Konstantin Akimov) 2186380 fix(evo): reject repeated purpose in ExtNetInfo deserialization (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Fixes dashpay#7864. `ExtNetInfo` read its purpose map through the generic `std::map` deserializer, which keeps the first copy of a repeated key and silently drops the rest. `Validate()` only saw the surviving map, so a ProTx whose `netInfo` carried a second `CORE_P2P` pair with an unknown entry type or an unreadable service passed validation and could be mined, while the dropped bytes were still covered by the txid. Strict third-party parsers (rust-dashcore, DashSync, dashj, Insight) reject such a transaction and with it the whole block. Reviewing the surrounding code showed that the repeated key was one of several ways one masternode configuration could have more than one accepted encoding: purposes in any order, an unknown purpose rejected only later by `Validate()`, and `NetInfoEntry` turning an unknown type byte, an invalid service or a failed service read into a cleared entry and reading on from an arbitrary stream offset. The ProUpServTx operator signature covers the re-serialized form, so every accepted variant shared one valid signature. While looking at `ValidateService` it also turned out that `CNetAddr::IsRoutable()` consults the node-local `-allowprivatenet` option, so a mainnet or testnet node started with it accepts an RFC1918 masternode address that every other node rejects. ## What was done? - fix: return error netinfo has neither service nor domain entries - fix(evo): keep -allowprivatenet out of masternode address validation - fix: make output ExtNetInfo's purposes addresses cannonical - fix: reject repeated purpose in ExtNetInfo deserialization ## How Has This Been Tested? See updates in regression tests. ## Breaking Changes it's gated beside v24 activation which is not active yet, so, no breaking changes yet. But changes are tighten consensus rules. ## 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)_ Top commit has no ACKs. Tree-SHA512: 62199f95fbe43d5eab3df086bf96ad75c7275a27a54d135bb2b5192fe68ad8c48af15fc9f401cbb7f6515886ea0e27bc7ed509bff14e8a5fc02ebca441ce2f6f (cherry picked from commit fad547f) Backport notes: develop's src/test/evo_netinfo_tests.cpp writes the "unspecified address" entry with CNetAddr::V2(), which comes from the serialization-parameter API of bitcoin#25284 and is not on this branch. The test writes it through an OverrideStream with ADDRV2_FORMAT instead, the way NetInfoEntry::Serialize does on this branch.
Pull requests into v24.0.x run develop's workflow files (pull_request_target), but with this branch's ci/ scripts. Since dashpay#7901, develop checks the depends cache on x86_64 light runners and passes the build runner's arch in DEPENDS_ARCH, expecting ci/test/00_setup_env_native_{multiprocess,tsan}.sh to use it. Without that, check-cache computes HOST=x86_64-pc-linux-gnu, the aarch64 build runner computes aarch64-linux-gnu, and "Build depends" fails whenever the depends cache misses. This takes only the ci/test hunks of 6f71fa5 (dashpay#7901). Without DEPENDS_ARCH the scripts fall back to uname -m, so this branch's own workflows behave as before. (partially cherry picked from commit 6f71fa5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
added
pastaclaw:commented
PastaPastaPasta
force-pushed
the
backport/v24.0.0-rc.4
branch
from
October 16, 2026 21:37
3336e73 to
6c02b2b
Compare
thepastaclaw
removed
the
pastaclaw:commented
thepastaclaw
approved these changes
Oct 16, 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
Static verification of the complete backport range found no actionable defects, and the prior wallet-upgrade error-code mismatch is fixed. The nine upstream-test suggestions concern three explicitly partial backports whose excluded behavior and tests are documented in the picked commits, not missing prerequisites for the selected fixes. Current CI has passing lint, dependency builds, and the fuzz source build, with nine other source-build jobs still pending; no local builds or tests were run.
🔴 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: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: backport-reviewer); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 7: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 10: 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, intricate diff changes consensus-sensitive provider transaction validation in src/evo/specialtxman.cpp, masternode state construction in src/node/miner.cpp, network encoding in src/evo/netinfo.cpp, and wallet key handling. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 71% left, 5h 100% left - 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-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— backport-reviewer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— 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 - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 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.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Track the declared bitcoin#34156 restore and migration test omissions — INTENTIONAL_EXCLUSION: 98d92f6 explicitly selects restore-directory cleanup ownership, leaves migration and restore-in-existing-directory behavior outside the backport, and states that no upstream tests are claimed. Upstream 90d651a adds those broader restore and migration scenarios, whereas HEAD retains rejection of existing destinations and adds preservation and corrupt-BDB cleanup regressions for the selected behavior. Their omission is documented scope, not an uncovered prerequisite for the ownership guard.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
thepastaclaw
added
the
pastaclaw:approved
thepastaclaw
removed
the
pastaclaw:approved
This was referenced Oct 29, 2026
PastaPastaPasta
added a commit
that referenced
this pull request
Oct 29, 2026
…labeled: #7916) d885ecd Merge #7916: backport: bitcoin#27800, bitcoin#28125, bitcoin#28232, bitcoin#27695 (merge-script) f55ad1d Merge bitcoin#28139: test: create wallet specific for test_locked_wallet case (DCG-Claude) Pull request description: ## Issue being fixed or feature implemented Second v24.0.0-rc.5 backport PR, on top of `v24.0.x` at v24.0.0-rc.4 (0e7382d) and independent of #7965. It brings over the last merged `backport-candidate-24.0.x` PR that `v24.0.x` does not have. All 49 merged PRs with that label were checked against `v24.0.x`. 46 were picked for rc.3 (#7808) or rc.4 (#7917), and #7893 was picked there as its PR commit. #7947 is in #7965. That leaves #7916. #7638, also labeled, is still open and is not included. ## What was done? Cherry-picked with `git cherry-pick -x` (`-m1` for the merge), in develop order: - 5df4af4 bitcoin#28139: test: create wallet specific for test_locked_wallet case. This is a test-only prerequisite, picked as its commit from #7863. bitcoin#28232 in #7916 edits the `test_locked_wallet` scenario that bitcoin#28139 rewrote. Without it, `wallet_fundrawtransaction.py` conflicts. The rest of #7863 (bitcoin#27866, bitcoin#28038, bitcoin#28039) is routine and left out. - #7916 backport: bitcoin#27800, bitcoin#28125, bitcoin#28232, bitcoin#27695. The label is there for bitcoin#28125 (`wallet: bugfix, disallow migration of invalid scripts`); the other three come along as one merge. Both picks change exactly the same lines as their develop commits. ## How Has This Been Tested? Linux x86_64 against depends, `--enable-debug --enable-werror --disable-hardening --without-gui`: build is clean. - Unit suites, passing: `wallet_tests`, `scriptpubkeyman_tests`, `streams_tests`, `serialize_tests`, `llmq_dkg_tests`, `governance_inv_tests`. - Functional tests, passing: `wallet_fundrawtransaction.py` (`--legacy-wallet`, `--legacy-wallet --nohd`, `--descriptors`), `wallet_migration.py`, `p2p_leak_tx.py` (v1/v2), `wallet_dump.py`, `feature_governance_objects.py`, `p2p_quorum_data.py`. ## Breaking Changes Only bitcoin#28125: during legacy-wallet migration, invalid or unwatched watch-only scripts are skipped instead of being migrated. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 89d59df1a643cba15ae39a1acee9cf4d2122b663a9b8dc2d0ac7fdab31853e855f46bba467dffc58cadbc9527d9dcc85ceed6825e9d83a9b18f270f5829ca943
PastaPastaPasta
added a commit
that referenced
this pull request
Oct 29, 2026
…ixes: #7682, #7869, #7861, #7951) 1ba2609 Merge #7951: fix(net): stop peers lingering after disconnect under edge-triggered epoll (pasta) 696d45f Merge #7861: feat: keep at least 4-days worth blocks after pruning (merge-script) 9cd1c72 Merge #7869: fix: let startup block replay handle quorum commitments and asset unlocks (pasta) 73e7254 Merge #7682: build: finish relic_conf.h regeneration before compiling BLS consumers (pasta) Pull request description: ## Issue being fixed or feature implemented Third v24.0.0-rc.5 backport PR, on top of `v24.0.x` at v24.0.0-rc.4 (0e7382d). It is independent of #7965 (v23.1.9 fixes and #7947) and #7967 (#7916). rc.5 is meant to be ready to become the final release, so every develop merge since the `v24.0.x` branch point that is neither picked nor labeled was audited again. These four unlabeled fixes are needed for a final v24.0.0. #7869 was already listed in #7917 as "worth considering before the final release". ## What was done? Cherry-picked with `git cherry-pick -m1 -x`, in develop merge order: - #7682 build: finish relic_conf.h regeneration before compiling BLS consumers. Fixes an intermittent parallel-build failure. - #7869 fix: let startup block replay handle quorum commitments and asset unlocks. After an unclean shutdown, block replay asserted on a quorum commitment, so the node needed `-reindex-chainstate` to recover. - #7861 feat: keep at least 4-days worth blocks after pruning. A pruned node could prune blocks that a credit-pool rebuild, or the work block of a rotated LLMQ cycle, still needs. It then rejected a canonical block after restart, or computed different quorum members than the network. - #7951 fix(net): stop peers lingering after disconnect under edge-triggered epoll #7682 and #7951 change exactly the same lines as their develop merges. Two picks needed adaptation, described in their commit messages: - **#7869**: develop folded `CChainstateHelper` into `Chainstate` (d5c1bd7), which is not on this branch, so `RollforwardBlock()` keeps calling `m_chain_helper->special_tx`. - **#7861**: chain parameters live in `src/chainparams.{h,cpp}` here (develop moved them to `src/kernel/`), and regtest's `nPruneAfterHeight` still reads `-fastprune` through `ArgsManager`. No caller of the old `MIN_BLOCKS_TO_KEEP` is left on this branch. `doc/release-notes-7861.md` comes with the pick and needs folding into the final release notes. Unlabeled develop merges since the branch point that were audited and left out: - #7918: not needed. The duplicate spork announcement it removes was never on this branch, because #7917 picked #7893's final commit once. - Left for 24.1: - perf work (#7934, #7935, #7936), because #7934 reworks MN list construction and payee validation; - #7847, which changes spork signing and stops using sporks on mainnet; - #7868 (GETMNLISTDIFF/GETQRINFO rate limit), #7844 and #7812; - #7260 (Qt feature) and its follow-up #7804; - #7937 (maintainer script); - the routine upstream backports and CI/docs/test-only changes. ## How Has This Been Tested? Linux x86_64 against depends, `--enable-debug --enable-werror --disable-hardening --without-gui`: build is clean. - Unit suites, passing: `evo_assetlocks_tests` (including #7861's new case), `blockmanager_tests`, `net_tests`, `denialofservice_tests`. - Functional tests, passing: `feature_asset_locks.py` (including #7869's replay case), `feature_pruning.py` (extended), `feature_index_prune.py`, `p2p_node_network_limited.py` (v1/v2), `rpc_blockchain.py` (v1/v2), `p2p_addr_relay.py`, `p2p_disconnect_ban.py` (v1/v2). - `feature_asset_locks.py` failed once in a parallel run, and the log was not kept. It then passed 5 of 5 runs, 4 of them in parallel. - An independent review compared each adapted pick with its develop merge and checked v24.0.x for remaining users of the old pruning constant. ## Breaking Changes - #7861: pruned nodes keep at least 2304 blocks (four days) instead of 288 on mainnet, testnet and devnet, so they use slightly more disk. On testnet and devnet, pruning starts after height 3000 instead of 1000. Regtest is unchanged. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: a8e39c6025e1737bcd55e7aa50c1419633a67dd82a0585f0586a63b8fb14f4770e9e24b7e7d2605da44c12e6c98768a893fccf6972aa71c1eaab23eec7e5b59f
PastaPastaPasta
added a commit
that referenced
this pull request
Oct 29, 2026
…xes, #7947) 4722ad9 chore: prepare v23.1.9 release (pasta) 71b3f0c Merge #7556: fix: gate BLS vote-signature failure log behind gobject category (pasta) 14dddb1 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta) 5f73355 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta) 0d2e579 fix(evo): keep -allowprivatenet out of masternode address validation (Konstantin Akimov) b4cd3fa fix(evo): only write credit pool snapshots inside a block-scoped EvoDB transaction (#7549) (pasta) d215280 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta) db9ec2b Merge #7826: fix: skip repeated authenticated Platform bans (pasta) dcc9596 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta) 197b14a Merge #7866: fix: stop `masternode payments` at the genesis block (pasta) 3055a19 Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta) 4da0099 Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta) c7393c0 Merge #7833: fix: gate signing intake before quorum retrieval (pasta) fdf40fe Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta) 8888af8 Merge #7753: fix: bound masternode list caches by size instead of by new block (pasta) 457af6d Merge #7738: refactor: avoid double lookups and default-inserting map access in governance (pasta) be3b55e Merge #7662: fix: isolate missing quorums in InstantSend batches (pasta) 9bc1e77 Merge #7489: fix: conflict ProRegTx collateral reuse with in-mempool masternode updates (pasta) 5278ce8 Merge #7583: fix: bound pending DKG message retention per proTxHash (pasta) 9ebf78f Merge #7522: fix(net): bound verified inbound connections per proRegTxHash (pasta) 9f2946e Merge #7487: fix: bound DomainPort string deserialization and stop swallowing deser failures (pasta) c077d32 Merge #7527: fix: require a valid masternode signature before caching an orphan governance vote (pasta) dfa7db1 Merge #7518: fix: memoize governance vote signature checks to bound govsync cost (pasta) bb09c49 Merge #7532: fix: bound DYNBITSET allocation against remaining stream size (pasta) 9739f35 Merge #7488: fix: validate ProUpServTx nType at mempool acceptance (pasta) 4687550 Merge #7385: fix: reject invalid extended masternode net info (Pasta) 58b9bce fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites (Konstantin Akimov) 7a21752 Merge bitcoin#36667: net: enforce timeout interval for paused send peers (Konstantin Akimov) 58abb28 Merge bitcoin#36665: sock: align exception monitoring between poll() and select() backends (Konstantin Akimov) 8030deb Merge bitcoin#36669: refactor: store positions as a 64 bits integer in flatfiles (Konstantin Akimov) b1178f9 fix(llmq): judge every sig-share sender by its own verified shares (pasta) 33ec74e fix(governance): cap the orphan parent objects re-requested per tick (pasta) 7fc0edb doc(rpc): mark provider payload network fields optional in the help (pasta) 2aa3a87 fix(rpc): don't dereference a missing netInfo when decoding provider payloads (pasta) 120b5db fix(llmq): verify DKG contributions individually when a batch vvec check fails (pasta) 808b105 Merge #7486: fix: guard VerifyContributionShares against an empty verification-vector set (pasta) 8351db9 fix(llmq): reject invalid decrypted DKG shares before aggregation (pasta) 07494dd fix(consensus): reject negative transaction nVersion after DIP3 (pasta) 11ea306 Merge #7947: fix: stop quorum key warming from stalling DKG on 2-core masternodes (pasta) 25d7c85 Merge #7826: fix: skip repeated authenticated Platform bans (pasta) 18073dd Merge #7850: perf: let CHECKDATASIG use the signature cache (pasta) 3759397 Merge #7833: fix: gate signing intake before quorum retrieval (pasta) db98ee2 fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites (Konstantin Akimov) 54c9738 Merge bitcoin#36667: net: enforce timeout interval for paused send peers (Konstantin Akimov) f1adb2c Merge bitcoin#36665: sock: align exception monitoring between poll() and select() backends (Konstantin Akimov) 8becf6a Merge bitcoin#36669: refactor: store positions as a 64 bits integer in flatfiles (Konstantin Akimov) 4c0cbd6 fix(llmq): judge every sig-share sender by its own verified shares (pasta) 649259f fix(governance): cap the orphan parent objects re-requested per tick (pasta) 418f5ee fix(llmq): verify DKG contributions individually when a batch vvec check fails (pasta) abf2eb0 fix(llmq): reject invalid decrypted DKG shares before aggregation (pasta) 39bda8f fix(consensus): reject negative transaction nVersion after DIP3 (pasta) Pull request description: ## Issue being fixed or feature implemented Backports for v24.0.0-rc.5, on top of `v24.0.x` at the v24.0.0-rc.4 tag (0e7382d, #7917). This PR holds the must-haves for rc.5: the v23.1.9 fixes that v24.0.x does not have yet, and #7947 (DKG stalls that get 2-core masternodes PoSe-banned). Without the v23.1.9 part, rc.5 would be missing fixes that the already published v23.1.9 ships, and it would accept transactions that v23.1.9 nodes reject (see Breaking Changes). The remaining `backport-candidate-24.0.x` work (#7916 and its test prerequisite) will follow in a separate PR so this one stays reviewable. ## What was done? ### v23.1.9 fixes not yet on v24.0.x v23.1.9 (tag 67c106f) contains 46 commits since v23.1.8. Each one was checked by content against `v24.0.x`. Most are backports of develop PRs that `v24.0.x` already has, either from before its branch point or picked for rc.3/rc.4. The rest are picked here with `git cherry-pick -x` from `v23.1.x`, in v23.1.9 merge order: - `fix(consensus): reject negative transaction nVersion after DIP3` (07494dd), **consensus**. Context-only conflict in `primitives/transaction.h`. - `fix(llmq): reject invalid decrypted DKG shares before aggregation` (8351db9). The QDATA path that v23 has in `src/active/quorums.cpp` is `src/llmq/net_quorum.cpp` on this branch. The new `feature_llmq_dkgerrors.py` case passes numeric `dkgsimerror` rates. - `fix(llmq): verify DKG contributions individually when a batch vvec check fails` (120b5db). The new `bls_tests` case calls `CBLSWorker::Start(4)`, since `Start()` takes an explicit worker count here. - `fix(governance): cap the orphan parent objects re-requested per tick` (33ec74e). The test helper uses this branch's `governance::OrphanVote`. - `fix(llmq): judge every sig-share sender by its own verified shares` (b1178f9). Ported into `NetSigning::ProcessPendingSigShares` (`llmq/net_signing.cpp`). The second half of the v23 fix (malformed share → `badSources`) is already on this branch. - bitcoin#36669 (8030deb). The `util/system` hunks land in `util/fs_helpers.{h,cpp}`. Two upstream hunks that the v23 port dropped are restored here, because this branch has their equivalents: `BlockInfo::data_pos` becomes `uint64_t` (in `interfaces/chain.h`), and `AutoFile`'s XOR read/write paths use `FileTell64`. - bitcoin#36665 (58abb28), bitcoin#36667 (7a21752). Clean. - `fix: keep flat-file positions 64-bit clean at the remaining raw seek/tell sites` (58b9bce). Also covers `BlockFilterIndex::LookupFilterRange`, which only this branch has. - #7833 fix: gate signing intake before quorum retrieval - #7850 perf: let CHECKDATASIG use the signature cache - #7826 fix: skip repeated authenticated Platform bans The last three are develop merges, picked with `-m1 -x` from develop; they change exactly the same lines as their develop merges. Every adaptation above is described in its commit message, and each matches how the open v23.1.9 → develop merge (#7961) resolves the same commit. Already on `v24.0.x` in another form, so not picked: - The credit-pool snapshot fix for #7549 (b4cd3fa) is a v23 hand-port of develop's 8503c5d / 0325195 (#7639), which this branch already has. - `fix(evo): keep -allowprivatenet out of masternode address validation` (0d2e579) is already covered by #7888. - `fix(rpc): don't dereference a missing netInfo when decoding provider payloads` (2aa3a87) and its help-text follow-up (7fc0edb). Here those accesses go through `CHECK_NONFATAL`, so an unknown-version ProTx payload gets an RPC error instead of a crash. The v23 commit message names this as the develop equivalent. ### Merge of v23.1.9 The last commit merges the v23.1.9 tag (67c106f, now `master`) with the tree unchanged (`-s ours`). `master` moved to v23.1.9 today, and `check_merge` requires `master` to fast-forward to `v24.0.x` plus the PR. The final v24.0.0 also has to contain every v23.1.9 commit. All of v23.1.9's content is already on the branch through the picks above, so every commit resolves to the `v24.0.x` side, as #7961 did on develop. Release bookkeeping (version, v23.1.9 release notes, man pages, flatpak entry) stays with v24's own release process. A trial real merge confirmed this: its only clean additions were those docs and three duplicate re-insertions of code the branch already has. ### #7947 fix: stop quorum key warming from stalling DKG on 2-core masternodes Picked with `-m1 -x`, before the merge. It changes exactly the same lines as its develop merge, and `v24.0.x` has 756fc4f (#7659), the commit it reverts. No version change is needed. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the build takes its version string from the `v24.0.0-rc.5` tag. ## How Has This Been Tested? Linux x86_64 against depends, `--enable-debug --enable-werror --disable-hardening --without-gui`: `dashd` and `test_dash` build cleanly. - Unit suites, all passing (149 cases): `transaction_tests`, `txvalidation_tests`, `sighash_tests`, `bls_tests`, `governance_inv_tests`, `llmq_utils_tests`, `llmq_dkg_tests`, `quorum_proofs_tests`, `flatfile_tests`, `fs_tests`, `net_tests`, `sock_tests`, `evo_netinfo_tests`, `evo_assetlocks_tests`, `blockfilter_index_tests`, `txindex_tests`, `blockmanager_tests`, `checkdatasig_tests`. - Functional tests, all passing: `feature_llmq_dkgerrors.py` (including the new contribution-invalid case), `feature_llmq_signing.py` (plain and `--spork21`), `feature_llmq_simplepose.py` (plain and `--disable-spork23`), `feature_llmq_rotation.py`, `feature_llmq_data_recovery.py`, `p2p_timeouts.py` (v1/v2), `mempool_accept.py`, `p2p_platform_ban.py`. In the first parallel run, `feature_llmq_data_recovery.py` (known flaky, see #7947), one `feature_llmq_signing.py --spork21` run and `p2p_platform_ban.py` failed while another build was loading the machine. All three pass when re-run on their own. - Each port was compared with its v23.1.9 commit and with #7961's develop resolution. The per-file changes are identical apart from the branch's own API differences listed above. A separate review checked v24-only call sites for second copies of each bug: - other `SigShareKey` batch verifiers; - other decrypt-then-aggregate paths; - other orphan re-request loops; - other tx version packing sites; - other raw `FlatFilePos` seeks. It also checked that the batch-vvec fallback cannot deadlock on a single-thread BLS pool; it runs on the DKG phase threads, not on a BLS worker. ## Breaking Changes - Consensus: once DIP3 is active, transactions with a negative `nVersion` are rejected (`bad-txns-version`). Every v23.1.9 node already enforces this, so mixed v23.1.9/rc.4 networks could disagree until rc.5 is deployed. Valid transactions and their txids are unchanged. - From #7947: quorum key share warming runs on its own `q-cache` thread again, which is one more thread per node. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: eaf46d1446c5f6752f2ac5d7aeaa37234cb6da4430db0a1a432320cfe61242dc3bb90c42e0a364238c2817ce0b1b1654dcc0cdd1ed9570f3b35cd2af5330e98e
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 for v24.0.0-rc.4, on top of
v24.0.xat the v24.0.0-rc.3 tag (0364ed6, #7808).What was done?
Cherry-picked (
git cherry-pick -m1 -x) every develop merge since rc.3 that carriesbackport-candidate-24.0.x, plus eight unlabeled fixes picked after an audit of every other develop merge since rc.3 (marked † below). They are in develop merge order:masternode paymentsat the genesis block #7866 fix: stopmasternode paymentsat the genesis blocktest_failed_upgrade()expects RPC error -1 for a 257-byte mnemonic passphrase, which fix(wallet): reject mnemonic passphrases that seed derivation truncates #7842's new precheck rejects with -8 first, sowallet_upgradetohd.py --legacy-walletfails. On develop, test: update guidance for agents for writing tests and follow-up fixes for lately added tests. #7859 replaced that scenario before fix(wallet): reject mnemonic passphrases that seed derivation truncates #7842 merged. The rest of test: update guidance for agents for writing tests and follow-up fixes for lately added tests. #7859 is test cleanup and docs and is left out.34 of the 39 picks change exactly the same lines as their develop merges. #7888 and #7782 needed test-only changes (see testing), and three others needed adaptation, described in their commit messages:
src/node/sync_manager.cpp, this branch still sentGETSPORKSfrom the mnsync tick on every network, because fix: sign sporks over their serialized fields and stop using sporks on mainnet #7847, which limited it to test chains, is not included. The request is removed entirely, as on develop. Mainnet spork values are hardcoded inCSporkManager::GetSporkValue, so the mainnet request had no effect.CChainstateHelperintoChainstate(d5c1bd7), which is not on this branch, so the new code goes throughChainHelper().special_tx/m_chain_helper.special_txinstead ofm_special_tx. The neighbouring develop-onlyCSpecialTxProcessor::GetDeterministicMNListHashis left out.#7888 tightens consensus rules for ExtAddr provider transactions behind V24, so I checked it against testnet. V24 is not active there yet (the tip's CbTx is still version 3), so the chain has no ExtAddr ProTxs the new rules could reject. rc.3's own serializer always writes canonical, sorted purposes, so every ProTx it builds passes the new rules. Mixed rc.3/rc.4 networks could only split on a hand-crafted non-canonical ExtAddr ProTx mined after V24 activates. So testnet miners and masternodes should move to rc.4 before V24 is signalled.
Unlabeled develop fixes since rc.3 that were audited and left out:
-reindex-chainstate; not a v24 regression), fix: gate signing intake before quorum retrieval #7833 (gate signing intake before quorum retrieval, DoS hardening), build: finish relic_conf.h regeneration before compiling BLS consumers #7682 (intermittentrelic_conf.hparallel-build failure).One CI-only commit comes last. It takes only the
ci/testhunks of 6f71fa5 (#7901), which makeci/test/00_setup_env_native_{multiprocess,tsan}.shhonorDEPENDS_ARCH. Pull requests intov24.0.xrun develop's workflow files (pull_request_target), and since #7901 those check the depends cache on x86_64 runners and pass the aarch64 build arch inDEPENDS_ARCH. Without the hunks,linux64_multiprocess / Build dependsfails on any depends cache miss (Build runner computes HOST=aarch64-linux-gnu but check-cache used x86_64-pc-linux-gnu). WithoutDEPENDS_ARCHthe scripts still useuname -m, so this branch's own push CI is unchanged.No version change is needed.
configure.acis already 24.0.0 withCLIENT_VERSION_IS_RELEASEfalse, so the build takes its version string from thev24.0.0-rc.4tag.How Has This Been Tested?
Built and tested at 6c02b2b on macOS arm64 against depends, with
--enable-debug --enable-werror --disable-hardening.make check: all unit test suites pass, includingtest_dash-qt.Functional tests, all passing:
feature_evo_shares_activation.py,feature_governance_objects.py,feature_llmq_signing.py(plain and--spork21),feature_masternode_shares.py,feature_sporks.py,interface_rest.py,p2p_instantsend.py,p2p_quorum_data.py,rpc_masternode.py,rpc_psbt.py,rpc_quorum.py,wallet_backup.py,wallet_dump.py,wallet_encryption.py,wallet_labels.pyandwallet_upgradetohd.py, each with--descriptorsand--legacy-walletwhere it supports them.Each pick's changed lines were compared with its develop merge (or, for fix(net): announce sporks to new peers instead of requesting them during mnsync #7893, its PR commit). They are identical except for the fix(net): announce sporks to new peers instead of requesting them during mnsync #7893, fix: mine a pending ProUpServTx before a key change of its masternode #7857 and feat(mining): build the masternode list incrementally during block assembly #7909 adaptations above, and the fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding #7888 test adaptation below.
A local macOS arm64 debug build (
--enable-debug --enable-werror) compiled most of the tree and stopped on a single error insrc/test/evo_netinfo_tests.cpp. fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding #7888's new test usedCNetAddr::V2(), the serialization-parameter API from net: Use serialization parameters for CAddress serialization bitcoin/bitcoin#25284, which this branch lacks. The pick now writes that entry through anOverrideStreamwithADDRV2_FORMAT, the wayNetInfoEntry::Serializedoes on this branch (see its commit message).fix(wallet): persist pending-observation coin locks once the record becomes readable #7782's new unit test reloads the wallet with develop's three-argument
CWalletconstructor. On this branch the constructor still takes the CoinJoin loader and anArgsManager, so the test passes them the way its fixture already does (see the commit message).With both test fixes, a full local
make(dashd,dash-qt,test_dash,test_dash-qt) completes with no errors.feature_pruning.pyis an extended test that CI does not run. fix: due to logical conflicts feature_pruning.py had been broken while #7894 is included because fix: treat missing snapshots during evodb repair as repair errors #7792 makes its restart path fail.Breaking Changes
Carried over from the picked PRs:
-reindex;-reindex-chainstateis not enough for nodes that already marked the following block invalid.netInfodecoding and validation behind V24 (see above). The RFC1918 check now applies even with-allowprivatenet.ProUpServTxspending an output of a pending operator key change, asset lock, asset unlock or MNHF signal is rejected from the mempool until that transaction confirms.doc/release-notes-7831.md).send,receiveandrefund. Wallets with other purpose strings load with warnings, andlistlabelserrors on an unknown purpose (doc/release-notes-27217.md).-allow-deprecated-long-mnemonicsis set, and over 256 bytes always fails (doc/release-notes-7842.md).-reindex, andevodb repairreports it inrepairErrors.protx info <proTxHash> <blockHash>,protx listdiffandmasternode payments <blockHash>return an error for blocks outside the active chain.protx shared_combineon aProUpSharedRegTxis refused unless its inputs are unlocked coins of a single address of the calling wallet, change returns to that address, and the fee is within-maxtxfee.GETSPORKSduring mnsync (no effect, mainnet spork values are hardcoded).Checklist:
🤖 Generated with Claude Code