From 0e75c226e4c4f995c84a0b9032a56f9c5ee37970 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 12:13:06 -0500 Subject: [PATCH 01/40] Merge #7814: fix(qt): restore HTML escaping of send confirmation recipients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit b0f86304b7d877093fc37dc6f8c782d8292ea662 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 `
` change what the user sees while reviewing the recipient. Dash used to escape the label and wallet name here. [Bitcoin Core #16826](https://github.com/bitcoin/bitcoin/pull/16826) was already backported as d7281c82efb1 (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](https://github.com/bitcoin/bitcoin/pull/15886) backport, a081afd3c10b (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 [2c530ea2](https://github.com/bitcoin/bitcoin/commit/2c530ea2ada71ca23fa17bab5023b855515463ef) and the wallet-name escape from [1770a972](https://github.com/bitcoin/bitcoin/commit/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 `
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 e07b557682ecaf76eae54f311a4210f001ab84e1) --- src/qt/sendcoinsdialog.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 1ede836f0dd4..693478d33ca5 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -358,7 +358,7 @@ bool SendCoinsDialog::send(const QList& recipients, QString& // generate amount string with wallet name in case of multiwallet QString amount = BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount); if (model->isMultiwallet()) { - amount = tr("%1 from wallet '%2'").arg(amount, model->getWalletName()); + amount = tr("%1 from wallet '%2'").arg(amount, GUIUtil::HtmlEscape(model->getWalletName())); } // generate address string @@ -369,7 +369,7 @@ bool SendCoinsDialog::send(const QList& recipients, QString& { if(rcp.label.length() > 0) // label with address { - recipientElement.append(tr("%1 to '%2'").arg(amount, rcp.label)); + recipientElement.append(tr("%1 to '%2'").arg(amount, GUIUtil::HtmlEscape(rcp.label))); recipientElement.append(QString(" (%1)").arg(address)); } else // just address From 00fc3b95802fde1450ca950053e8cb50534069e8 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 12:40:47 -0500 Subject: [PATCH 02/40] Merge #7851: fix(rpc): correct masternode status help for top-level service field 2fb1e39327548ad17252d2585e1534969302e06b 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 2340e3a265fd1d75e015915ac058142d37f9b1d4) --- src/rpc/masternode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index dce33a47300a..bba03afab880 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -181,7 +181,7 @@ static RPCHelpMan masternode_status() RPCResult::Type::OBJ, "", "", { GetRpcResult("outpoint"), - GetRpcResult("service", /*optional=*/true), + {RPCResult::Type::STR, "service", "IP address and port of this masternode"}, GetRpcResult("proTxHash", /*optional=*/true), GetRpcResult("type_str", /*optional=*/true, /*override_name=*/"type"), GetRpcResult("collateralHash", /*optional=*/true), From 886d5c40bf41b646cebba25b7a77df5d965956bd Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 17:18:59 -0500 Subject: [PATCH 03/40] Merge #7816: fix: check credit pool amount accumulation 3ef05555b40bd389ccd1f1d86f08d7ddb96df5c6 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 3ef05555b40bd389ccd1f1d86f08d7ddb96df5c6 Tree-SHA512: 461cbd9f11ea078159b1e8166fcb511920b937590dcc83c8b4ccc48db009a34cc975b3fd431920038561cc793242756f9d93dc20ab60c3be079e7f7ceb58947c (cherry picked from commit 21ea7ba5c23848e61cb50151ff2ae42254864d30) --- src/evo/creditpool.cpp | 29 +++++++++++++++++++++---- src/evo/creditpool.h | 5 ++--- src/test/evo_assetlocks_tests.cpp | 35 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index df40c6378756..cafe65b023f4 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -17,12 +17,14 @@ #include #include #include +#include #include #include #include #include #include +#include using node::ReadBlockFromDisk; @@ -41,7 +43,11 @@ static bool GetDataFromUnlockTx(const CTransaction& tx, CAmount& toUnlock, uint6 if (!MoneyRange(txout.nValue)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-txout-outofrange"); } - toUnlock += txout.nValue; + const auto total = CheckedAdd(toUnlock, txout.nValue); + if (!total) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-overflow"); + } + toUnlock = *total; } return true; } @@ -288,6 +294,16 @@ CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex* pindexP } } +CAmount CCreditPoolDiff::GetTotalLocked() const +{ + // Subtract withdrawals first to preserve a representable net balance. + auto total = CheckedAdd(pool.locked, -sessionUnlocked); + if (total) total = CheckedAdd(*total, sessionLocked); + if (total) total = CheckedAdd(*total, platformReward); + if (!total) throw std::overflow_error("credit-pool balance overflow"); + return *total; +} + bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) { if (const auto opt_assetLockTx = GetTxPayload(tx); !opt_assetLockTx) { @@ -297,7 +313,11 @@ bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) for (const CTxOut& txout : tx.vout) { if (const CScript& script = txout.scriptPubKey; script.empty() || script[0] != OP_RETURN) continue; - sessionLocked += txout.nValue; + const auto total = CheckedAdd(sessionLocked, txout.nValue); + if (!total) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-lock-overflow"); + } + sessionLocked = *total; return true; } @@ -313,7 +333,8 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s return false; } - if (sessionUnlocked + toUnlock > pool.currentLimit) { + const auto total = CheckedAdd(sessionUnlocked, toUnlock); + if (!total || *total > pool.currentLimit) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-too-much"); } @@ -323,7 +344,7 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s newIndexes.insert(index); if (inserted_index) *inserted_index = index; - sessionUnlocked += toUnlock; + sessionUnlocked = *total; return true; } diff --git a/src/evo/creditpool.h b/src/evo/creditpool.h index bb3891a37403..e7676beace54 100644 --- a/src/evo/creditpool.h +++ b/src/evo/creditpool.h @@ -100,10 +100,9 @@ class CCreditPoolDiff { /** * this function returns total amount of credits for the next block + * Throws if the total cannot be represented as CAmount. */ - CAmount GetTotalLocked() const { - return pool.locked + sessionLocked - sessionUnlocked + platformReward; - } + CAmount GetTotalLocked() const; std::string ToString() const { return strprintf("CCreditPoolDiff(sessionLocked=%lld, sessionUnlocked=%lld, platforomReward=%lld, newIndexes=%lld, pool=%s)", sessionLocked, sessionUnlocked, platformReward, newIndexes.size(), pool.ToString()); diff --git a/src/test/evo_assetlocks_tests.cpp b/src/test/evo_assetlocks_tests.cpp index f9ac499106be..7bc4c09a5e85 100644 --- a/src/test/evo_assetlocks_tests.cpp +++ b/src/test/evo_assetlocks_tests.cpp @@ -25,6 +25,8 @@ #include +#include + // Helpers: static bool IsStandardTx(const CTransaction& tx, std::string& reason) { @@ -141,6 +143,17 @@ static CTransactionRef CreateCreditPoolUnlockTx(uint64_t index, CAmount amount, return MakeTransactionRef(std::move(tx)); } +static CTransactionRef CreateCreditPoolLockTx(CAmount amount) +{ + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_ASSET_LOCK; + tx.vout.emplace_back(amount, CScript{} << OP_RETURN << OP_0); + SetTxPayload(tx, CAssetLockPayload{std::vector{{amount, GetScriptForDestination(PKHash{})}}, + CAssetLockPayload::INITIAL_VERSION}); + return MakeTransactionRef(std::move(tx)); +} + BOOST_FIXTURE_TEST_SUITE(evo_assetlocks_tests, TestChain100Setup) static void CheckAssetLockCommon(uint8_t version, bool is_v24_active) @@ -885,6 +898,28 @@ BOOST_AUTO_TEST_CASE(credit_pool_unlock_limit_v24) BOOST_CHECK_EQUAL(limit(MAX_MONEY, MAX_MONEY), MAX_MONEY * 20 / 100); } +BOOST_FIXTURE_TEST_CASE(credit_pool_lock_arithmetic, TestChain100Setup) +{ + LOCK(cs_main); + const auto* tip = m_node.chainman->ActiveChain().Tip(); + CCreditPoolDiff diff{CCreditPool{}, tip, m_node.chainman->GetConsensus(), 0}; + const CAmount maximum = std::numeric_limits::max(); + const auto lock_max_money = CreateCreditPoolLockTx(MAX_MONEY); + for (CAmount i = 0; i < maximum / MAX_MONEY; ++i) { + TxValidationState state; + BOOST_REQUIRE(diff.ProcessLockUnlockTransaction(*lock_max_money, state)); + if (i == 1) BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 2 * MAX_MONEY); + } + TxValidationState remainder_state; + BOOST_REQUIRE(diff.ProcessLockUnlockTransaction(*CreateCreditPoolLockTx(maximum % MAX_MONEY), remainder_state)); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), maximum); + + TxValidationState overflow_state; + BOOST_CHECK(!diff.ProcessLockUnlockTransaction(*CreateCreditPoolLockTx(1), overflow_state)); + BOOST_CHECK_EQUAL(overflow_state.GetRejectReason(), "failed-creditpool-lock-overflow"); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), maximum); +} + BOOST_FIXTURE_TEST_CASE(credit_pool_package_atomicity, TestChain100Setup) { LOCK(cs_main); From 86a77372733662c1d42e7b1be0ec3817c251f1bf Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 18:28:36 -0500 Subject: [PATCH 04/40] Merge #7828: fix: omit recovery words from invalid mnemonic errors e5fe71d2350b73e731f7099f821f140921f02aff 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 e075859785a6d1a18730bd248c056f0d13d9f1f2) --- src/wallet/hdchain.cpp | 2 +- src/wallet/wallet.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/hdchain.cpp b/src/wallet/hdchain.cpp index 6b5673b508f5..a8ee2b7b0344 100644 --- a/src/wallet/hdchain.cpp +++ b/src/wallet/hdchain.cpp @@ -69,7 +69,7 @@ bool CHDChain::SetMnemonic(const SecureString& ssMnemonic, const SecureString& s // printf("mnemonic: %s\n", ssMnemonicTmp.c_str()); if (!CMnemonic::Check(ssMnemonicTmp)) { - throw std::runtime_error(std::string(__func__) + ": invalid mnemonic: `" + std::string(ssMnemonicTmp) + "`"); + throw std::runtime_error(std::string(__func__) + ": invalid mnemonic"); } CMnemonic::ToSeed(ssMnemonicTmp, ssMnemonicPassphrase, vchSeed); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 81488c34206c..d1ed3b57a541 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4375,7 +4375,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const SecureString& mnemonic_arg, // TODO: remove duplicated code with CHDChain::SetMnemonic const SecureString mnemonic = mnemonic_arg.empty() ? CMnemonic::Generate(m_args.GetIntArg("-mnemonicbits", CHDChain::DEFAULT_MNEMONIC_BITS)) : mnemonic_arg; if (!CMnemonic::Check(mnemonic)) { - throw std::runtime_error(std::string(__func__) + ": invalid mnemonic: `" + std::string(mnemonic) + "`"); + throw std::runtime_error(std::string(__func__) + ": invalid mnemonic"); } SecureVector seed_key; CMnemonic::ToSeed(mnemonic, mnemonic_passphrase, seed_key); From bcefee4fce521aebfd387410781fc19da456660e Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 18:39:01 -0500 Subject: [PATCH 05/40] Merge #7822: fix: reject malformed detached quorum share batches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7a9d1b0edeba3281f908a1d7249427c686c3aae3 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. #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 063aee1541f439314f2b65126e4163a37431efa5) --- src/llmq/net_signing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/net_signing.cpp b/src/llmq/net_signing.cpp index 32cca59ac834..2e70d79d520b 100644 --- a/src/llmq/net_signing.cpp +++ b/src/llmq/net_signing.cpp @@ -417,7 +417,7 @@ void NetSigning::ProcessPendingSigShares( // we didn't check this earlier because we use a lazy BLS signature and tried to avoid doing the expensive // deserialization in the message thread if (!sig.IsValid()) { - BanNode(nodeId); + batchVerifier.badSources.emplace(nodeId); // don't process any additional shares from this node break; } From 0bf80f84d8298019750d7a6c2fca4d5c69f5f6de Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 18:48:55 -0500 Subject: [PATCH 06/40] Merge #7810: fix(rpc): handle unavailable masternode payment inputs 696c0c0ee174e115fc3b3810e0da32d1284d862d 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 a94bafc1b0252d658f2395840d473cf3dae2dd71) --- src/rpc/masternode.cpp | 7 +++++++ test/functional/rpc_masternode.py | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index bba03afab880..452297574a93 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -442,6 +442,13 @@ static RPCHelpMan masternode_payments() for (const auto& txin : tx->vin) { uint256 blockHashTmp; CTransactionRef txPrev = GetTransaction(/* block_index */ nullptr, &mempool, txin.prevout.hash, Params().GetConsensus(), blockHashTmp); + if (!txPrev) { + throw JSONRPCError(RPC_MISC_ERROR, "Unable to calculate block fees: previous transaction not " + "found. Use -txindex to enable historical transaction queries"); + } + if (txin.prevout.n >= txPrev->vout.size()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to calculate block fees: previous output not found"); + } nValueIn += txPrev->vout[txin.prevout.n].nValue; } nBlockFees += nValueIn - tx->GetValueOut(); diff --git a/test/functional/rpc_masternode.py b/test/functional/rpc_masternode.py index 7258f07d57b3..41488d31b212 100755 --- a/test/functional/rpc_masternode.py +++ b/test/functional/rpc_masternode.py @@ -3,7 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_raises_rpc_error ''' rpc_masternode.py @@ -103,5 +103,12 @@ def run_test(self): # we have 3 masternodes that are running already and 2 new outputs we just created assert_equal(len(self.nodes[0].masternode("outputs")), 5) + self.log.info("test payments without historical transaction data") + spend_blockhash = self.nodes[0].getrawtransaction(self.mninfo[0].proTxHash, True)["blockhash"] + self.restart_node(0, extra_args=self.nodes[0].extra_args + ["-txindex=0"]) + assert_raises_rpc_error(-1, "previous transaction not found", + self.nodes[0].masternode, "payments", spend_blockhash) + self.nodes[0].getblockcount() + if __name__ == '__main__': RPCMasternodeTest().main() From 3c5c03985189c0db02f7e91f4754d8abdea74d7d Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 19:02:48 -0500 Subject: [PATCH 07/40] Merge #7838: fix(wallet): authenticate unlock deadline extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 86ba039389367344463abc8537824606eec43443 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 78c5ca53f6ff, blocked in 0.12.1, reintroduced by ff5540bf7301 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 26309306348729cb4408f6fe042fb610ea5abf08) --- src/wallet/wallet.cpp | 9 ++++++--- test/functional/wallet_encryption.py | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d1ed3b57a541..b2ef87d8a731 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #ifdef USE_BDB @@ -4152,20 +4153,22 @@ bool CWallet::Lock(bool fAllowMixing) bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool fForMixingOnly) { - if (!IsLocked()) // was already fully unlocked, not only for mixing - return true; - CCrypter crypter; CKeyingMaterial _vMasterKey; { LOCK(cs_wallet); + if (!IsCrypted()) return true; for (const MasterKeyMap::value_type& pMasterKey : mapMasterKeys) { if (!crypter.SetKeyFromPassphrase(strWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod)) return false; if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, _vMasterKey)) continue; // try another master key + if (!vMasterKey.empty() && !TimingResistantEqual(_vMasterKey, vMasterKey)) { + continue; + } + if (!IsLocked()) return true; if (Unlock(_vMasterKey, fForMixingOnly)) { // Now that we've unlocked, upgrade the key metadata UpgradeKeyMetadata(); diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py index 62e4b0f85830..57ab68b0c59e 100755 --- a/test/functional/wallet_encryption.py +++ b/test/functional/wallet_encryption.py @@ -102,6 +102,13 @@ def run_test(self): assert self.nodes[0].verifymessage(address, sig, msg) self.nodes[0].walletlock() + self.log.info("Check that a wrong passphrase cannot extend a full unlock") + self.nodes[0].walletpassphrase(passphrase_with_nulls, 100) + unlocked_until = self.nodes[0].getwalletinfo()['unlocked_until'] + assert_raises_rpc_error(-14, "wallet passphrase entered", self.nodes[0].walletpassphrase, "incorrect passphrase", 1000) + assert_equal(self.nodes[0].getwalletinfo()['unlocked_until'], unlocked_until) + self.nodes[0].walletlock() + if __name__ == '__main__': WalletEncryptionTest().main() From 2fea45fe65d1da14127c7d5086f5ab68c9cae9db Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 21:10:24 -0500 Subject: [PATCH 08/40] Merge #7792: fix: treat missing snapshots during evodb repair as repair errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 581a219dbf3d83c56df72973f1728aeead0c890b 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 #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", )` record from `evodb` with a small leveldb tool, then restarted twice.
Reproduction with a dashd built from c14104b (unfixed) ``` == [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": [ ] } ```
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.
Same reproduction with this change ``` == [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." ] } ```
- `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 a0950fd6dc16ed884c19950f0beb6c3d50f528d3) --- src/evo/deterministicmns.cpp | 7 ++- src/test/evo_deterministicmns_tests.cpp | 65 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 89997142a884..7023f28b5255 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1229,6 +1229,9 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA // Storage for recalculated diffs if we plan to repair std::vector> recalculated_diffs; + // Snapshots can't be rebuilt from diffs, so in repair mode a missing one means the repair failed + auto& missing_snapshot_errors = repair ? result.repair_errors : result.verification_errors; + // Process each pair of consecutive snapshots for (size_t i = 0; i < snapshot_blocks.size() - 1; ++i) { const CBlockIndex* from_index = snapshot_blocks[i]; @@ -1252,7 +1255,7 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA __func__, from_index->nHeight); } else { // Any other missing snapshot is critical corruption beyond our repair capability - result.verification_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " + missing_snapshot_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " "This cannot be repaired by this tool - full reindex required.", from_index->nHeight)); return result; } @@ -1260,7 +1263,7 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA if (!has_to_snapshot) { // Missing target snapshot is always critical - we cannot repair snapshots, only diffs - result.verification_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " + missing_snapshot_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " "This cannot be repaired by this tool - full reindex required.", to_index->nHeight)); return result; } diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 22eba79cf53e..29571132cffb 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -3698,6 +3699,70 @@ BOOST_AUTO_TEST_CASE(test_sml_cache_basic) SmlCache(setup); } +// Startup repair marks evodb as repaired unless repair_errors is set, so a snapshot missing from disk +// must land there in repair mode, while "evodb verify" keeps reporting it as a verification error. +BOOST_AUTO_TEST_CASE(evodb_repair_missing_snapshot_is_repair_error) +{ + TestChainDIP3Setup setup; + auto& chainman = *Assert(setup.m_node.chainman); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& evodb = *Assert(setup.m_node.evodb); + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + + // CDeterministicMNManager::DISK_SNAPSHOT_PERIOD + constexpr int snapshot_period{576}; + while (WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()) < 2 * snapshot_period) { + setup.CreateAndProcessBlock({}, coinbase_pk); + } + const CBlockIndex* dip3_index; + const CBlockIndex* first_index; + const CBlockIndex* second_index; + { + LOCK(::cs_main); + dip3_index = chainman.ActiveChain()[Params().GetConsensus().DIP0003Height]; + first_index = chainman.ActiveChain()[snapshot_period]; + second_index = chainman.ActiveChain()[2 * snapshot_period]; + } + + // Diffs between intact snapshots verify, so the rebuild callback is never needed here + const auto build_list_func = [](const CBlock&, gsl::not_null, const CDeterministicMNList&, + const CCoinsViewCache&, bool, BlockValidationState&, + CDeterministicMNList&) { return false; }; + + auto result = dmnman.RecalculateAndRepairDiffs(dip3_index, second_index, build_list_func, /*repair=*/true); + BOOST_CHECK(result.verification_errors.empty()); + BOOST_CHECK(result.repair_errors.empty()); + BOOST_CHECK_EQUAL(result.snapshots_verified, 2); + + { + LOCK(::cs_main); + auto db_tx = evodb.BeginTransaction(); + // DB_LIST_SNAPSHOT + evodb.Erase(std::make_pair(std::string{"dmn_S3"}, first_index->GetBlockHash())); + db_tx->Commit(); + } + + const std::string missing_msg{strprintf("Snapshot missing at height %d", snapshot_period)}; + + // Snapshot missing as the end of a pair + result = dmnman.RecalculateAndRepairDiffs(dip3_index, second_index, build_list_func, /*repair=*/true); + BOOST_CHECK(result.verification_errors.empty()); + BOOST_REQUIRE_EQUAL(result.repair_errors.size(), 1U); + BOOST_CHECK(result.repair_errors[0].find(missing_msg) != std::string::npos); + + // Snapshot missing as the start of a pair + result = dmnman.RecalculateAndRepairDiffs(first_index, second_index, build_list_func, /*repair=*/true); + BOOST_CHECK(result.verification_errors.empty()); + BOOST_REQUIRE_EQUAL(result.repair_errors.size(), 1U); + BOOST_CHECK(result.repair_errors[0].find(missing_msg) != std::string::npos); + + // Verify-only mode has no repair errors to report + result = dmnman.RecalculateAndRepairDiffs(dip3_index, second_index, build_list_func, /*repair=*/false); + BOOST_CHECK(result.repair_errors.empty()); + BOOST_REQUIRE_EQUAL(result.verification_errors.size(), 1U); + BOOST_CHECK(result.verification_errors[0].find(missing_msg) != std::string::npos); +} + BOOST_AUTO_TEST_CASE(field_bit_migration_validation) { // Test individual field mappings for ALL 19 fields From e7e3daa7acdfe9ec1e3735541378eada60204f77 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 21:22:24 -0500 Subject: [PATCH 09/40] Merge #7821: fix: relock encrypted wallets after HD upgrade failures 973f59e21ee33384acd412b04bb6d23875e8a69c 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 66d9b6a37f117cfbdd69f23c7dd2b9d3d70bdf18) --- src/wallet/rpc/wallet.cpp | 9 +++++---- test/functional/wallet_upgradetohd.py | 29 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 9b5b43d835e2..cadd0ced6aa3 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -382,11 +383,14 @@ static RPCHelpMan upgradetohd() pwallet->WalletLogPrintf("Upgrading wallet to HD\n"); pwallet->SetMinVersion(FEATURE_HD); + std::unique_ptr relock; if (pwallet->IsCrypted()) { if (wallet_passphrase.empty()) { throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: Wallet encrypted but supplied empty wallet passphrase"); } + relock = interfaces::MakeCleanupHandler([&pwallet] { pwallet->Lock(); }); + // We are intentionally re-locking the wallet so we can validate passphrase // by verifying if it can unlock the wallet pwallet->Lock(); @@ -425,10 +429,7 @@ static RPCHelpMan upgradetohd() } } - if (pwallet->IsCrypted()) { - // Relock encrypted wallet - pwallet->Lock(); - } else if (!wallet_passphrase.empty()) { + if (!pwallet->IsCrypted() && !wallet_passphrase.empty()) { // Encrypt non-encrypted wallet if (!pwallet->EncryptWallet(wallet_passphrase)) { throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Failed to encrypt HD wallet"); diff --git a/test/functional/wallet_upgradetohd.py b/test/functional/wallet_upgradetohd.py index 36de68c23b37..b81dfb95d159 100755 --- a/test/functional/wallet_upgradetohd.py +++ b/test/functional/wallet_upgradetohd.py @@ -45,7 +45,36 @@ def recover_non_hd(self): if not self.options.descriptors: assert 'hdchainid' not in node.getwalletinfo() + def test_failed_upgrade(self): + self.log.info("Failed encrypted HD upgrades must keep existing keys locked") + node = self.nodes[0] + node.createwallet("failed-upgrade", blank=True, descriptors=self.options.descriptors) + wallet = node.get_wallet_rpc("failed-upgrade") + key = node.get_deterministic_priv_key() + wallet.importprivkey(key.key) + passphrase = "upgrade passphrase" + wallet.encryptwallet(passphrase) + mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + invalid_inputs = [("invalid mnemonic", "", "invalid mnemonic")] + if not self.options.descriptors: + invalid_inputs.append((mnemonic, "x" * 257, "Mnemonic passphrase is too long")) + for invalid_mnemonic, invalid_passphrase, error in invalid_inputs: + assert_raises_rpc_error(-1, error, wallet.upgradetohd, invalid_mnemonic, invalid_passphrase, passphrase) + assert_raises_rpc_error(-13, "Please enter the wallet passphrase", wallet.signmessage, key.address, "failed upgrade") + wallet.walletpassphrase(passphrase, 100) + sig = wallet.signmessage(key.address, "explicit unlock") + assert node.verifymessage(key.address, sig, "explicit unlock") + wallet.walletlock() + wallet.upgradetohd(mnemonic, "", passphrase, False) + assert_raises_rpc_error(-13, "Please enter the wallet passphrase", wallet.signmessage, key.address, "successful upgrade") + wallet.walletpassphrase(passphrase, 100) + sig = wallet.signmessage(key.address, "imported key after upgrade") + assert node.verifymessage(key.address, sig, "imported key after upgrade") + wallet.walletlock() + wallet.unloadwallet() + def run_test(self): + self.test_failed_upgrade() node = self.nodes[0] node.backupwallet(os.path.join(node.datadir, "non_hd.bak")) From cb393693797bad176f1c6eacaf60eb5bf9b76973 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 21:50:23 -0500 Subject: [PATCH 10/40] Merge #7834: fix: validate Electrum import input boundaries 917848344301f938429b16d99136ab6ec6165abd 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 bda514a01d099688a1f792163c9ef564e78b2f25) --- src/wallet/rpc/backup.cpp | 28 +++++++++++++++++++--------- test/functional/wallet_dump.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 0532f428a17e..12a75aca2b22 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -651,7 +651,7 @@ RPCHelpMan importelectrumwallet() "Note: This command is only compatible with legacy wallets.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The Electrum wallet export file, should be in csv or json format"}, - {"index", RPCArg::Type::NUM, RPCArg::Default{0}, "Rescan the wallet for transactions starting from this block index"}, + {"index", RPCArg::Type::NUM, RPCArg::Default{0}, "Rescan the wallet for transactions starting from this non-negative block index"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -681,6 +681,11 @@ RPCHelpMan importelectrumwallet() EnsureWalletIsUnlocked(*pwallet); + int nStartHeight = request.params[1].isNull() ? 0 : request.params[1].getInt(); + if (nStartHeight < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Rescan index must be non-negative"); + } + std::ifstream file; std::string strFileName = request.params[0].get_str(); size_t nDotPos = strFileName.find_last_of('.'); @@ -691,7 +696,7 @@ RPCHelpMan importelectrumwallet() if(strFileExt != "json" && strFileExt != "csv") throw JSONRPCError(RPC_INVALID_PARAMETER, "File has wrong extension, should be .json or .csv"); - file.open(strFileName, std::ios::in | std::ios::ate); + file.open(strFileName, std::ios::in | std::ios::ate | std::ios::binary); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open Electrum wallet export file"); @@ -699,7 +704,11 @@ RPCHelpMan importelectrumwallet() WalletBatch batch(pwallet->GetDatabase()); - int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); + const auto file_size = file.tellg(); + if (file_size < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot read Electrum wallet export file"); + } + const int64_t nFilesize = std::max(1, file_size); file.seekg(0, file.beg); pwallet->ShowProgress(_("Importing…").translated, 0); // show progress dialog in GUI @@ -750,9 +759,13 @@ RPCHelpMan importelectrumwallet() // json UniValue data(UniValue::VOBJ); { - auto buffer = std::make_unique(nFilesize); - file.read(buffer.get(), nFilesize); - if(!data.read(buffer.get())) + std::string buffer(nFilesize, '\0'); + file.read(buffer.data(), buffer.size()); + if (file.bad() || (file.fail() && !file.eof())) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot read Electrum wallet export file"); + } + buffer.resize(file.gcount()); + if (!data.read(buffer)) throw JSONRPCError(RPC_TYPE_ERROR, "Cannot parse Electrum wallet export file"); } @@ -798,9 +811,6 @@ RPCHelpMan importelectrumwallet() const int32_t tip_height = pwallet->chain().getHeight().value_or(std::numeric_limits::max()); // Whether to perform rescan after import - int nStartHeight = 0; - if (!request.params[1].isNull()) - nStartHeight = request.params[1].getInt(); if (tip_height < nStartHeight) nStartHeight = tip_height; diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 344c47e94ae1..ec2868d72083 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the dumpwallet RPC.""" import datetime +import json import os import time @@ -100,7 +101,39 @@ def setup_network(self): self.add_nodes(self.num_nodes, extra_args=self.extra_args) self.start_nodes() + def test_electrum_import(self): + node = self.nodes[0] + node.createwallet('electrum_source', descriptors=False) + node.createwallet('electrum_destination', blank=True, descriptors=False) + source = node.get_wallet_rpc('electrum_source') + destination = node.get_wallet_rpc('electrum_destination') + addresses = [source.getnewaddress() for _ in range(4)] + secrets = [source.dumpprivkey(address) for address in addresses] + json_file = os.path.join(node.datadir, 'electrum.json') + csv_file = os.path.join(node.datadir, 'electrum.csv') + with open(json_file, 'wb') as f: + f.write(json.dumps({addresses[0]: secrets[0]}).encode() + b'\x00trailing bytes') + assert_raises_rpc_error(-3, 'Cannot parse Electrum wallet export file', destination.importelectrumwallet, json_file) + assert not destination.getaddressinfo(addresses[0])['ismine'] + with open(json_file, 'w', encoding='utf8') as f: + json.dump({addresses[0]: secrets[0], addresses[1]: 'p2pkh:' + secrets[1]}, f) + with open(csv_file, 'w', encoding='utf8') as f: + f.write('address,private_key\n' + addresses[2] + ',' + secrets[2] + '\n' + addresses[3] + ',p2pkh:' + secrets[3] + '\n') + for filename, import_addresses in ((json_file, addresses[:2]), (csv_file, addresses[2:])): + assert_raises_rpc_error(-8, 'Rescan index must be non-negative', destination.importelectrumwallet, filename, -1) + assert all(not destination.getaddressinfo(address)['ismine'] for address in import_addresses) + destination.importelectrumwallet(json_file, 0) + destination.importelectrumwallet(csv_file, node.getblockcount()) + destination.importelectrumwallet(json_file, node.getblockcount() + 100) + for address, secret in zip(addresses, secrets): + assert_equal(destination.dumpprivkey(address), secret) + signature = destination.signmessage(address, 'Electrum import regression') + assert node.verifymessage(address, signature, 'Electrum import regression') + source.unloadwallet() + destination.unloadwallet() + def run_test(self): + self.test_electrum_import() self.nodes[0].createwallet("dump") wallet_unenc_dump = os.path.join(self.nodes[0].datadir, "wallet.unencrypted.dump") From 2ff010369a44f556d0237c3678c2c7bc972fd507 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 21:57:10 -0500 Subject: [PATCH 11/40] Merge #7818: fix: return REST errors for unavailable deployment data 22266827f2578c878dc19e0be820991dc65a0de8 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 b258eab403bc70654c7f346c9775d90e78e01a48) --- src/rest.cpp | 10 +++++++++- test/functional/interface_rest.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/rest.cpp b/src/rest.cpp index 586b543791e0..ac06fd635ff3 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -665,8 +665,16 @@ static bool rest_deploymentinfo(const CoreContext& context, HTTPRequest* req, co jsonRequest.params.push_back(hash_str); } + std::string result; + try { + result = getdeploymentinfo().HandleRequest(jsonRequest).write() + "\n"; + } catch (const UniValue& error) { + return RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, error["message"].get_str()); + } catch (const std::exception& error) { + return RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, error.what()); + } req->WriteHeader("Content-Type", "application/json"); - req->WriteReply(HTTP_OK, getdeploymentinfo().HandleRequest(jsonRequest).write() + "\n"); + req->WriteReply(HTTP_OK, result); return true; } default: { diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index bb48d245cd8a..7436f36fa1b9 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -435,5 +435,20 @@ def run_test(self): resp = self.test_rest_request(f"/deploymentinfo/{INVALID_PARAM}", ret_type=RetType.OBJ, status=400) assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid hash: {INVALID_PARAM}") + self.log.info("Test deployment info with unavailable block data") + self.generate(self.nodes[0], max(0, 432 - self.nodes[0].getblockcount())) + tip_before_request = self.nodes[0].getbestblockhash() + self.disconnect_nodes(0, 1) + header_only_hash = self.generate(self.nodes[1], 1, sync_fun=self.no_op)[0] + self.nodes[0].submitheader(self.nodes[1].getblockheader(header_only_hash, False)) + resp = self.test_rest_request(f"/deploymentinfo/{header_only_hash}", ret_type=RetType.OBJ, status=500) + assert_equal(resp.getheader('Content-Type'), 'text/plain') + assert_equal(resp.read().decode('utf-8').rstrip(), "failed-getehfforblock-read") + assert_equal(self.nodes[0].getbestblockhash(), tip_before_request) + assert_equal(self.test_rest_request('/deploymentinfo'), self.nodes[0].getdeploymentinfo()) + self.connect_nodes(0, 1) + self.sync_all() + assert_equal(self.test_rest_request(f"/deploymentinfo/{header_only_hash}"), self.nodes[0].getdeploymentinfo(header_only_hash)) + if __name__ == '__main__': RESTTest().main() From 50a5617fede2563393b32f33294b49591e062aae Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 9 Oct 2026 00:17:45 -0500 Subject: [PATCH 12/40] Merge #7829: fix: preserve wallet permissions when changing passphrases 6861fa51b11a5dbae70047a2027d58053ecc3c29 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 8cb2c955d769783e65e0b465ac7a3bd9d1719b95) --- src/wallet/wallet.cpp | 12 ++++++------ test/functional/wallet_encryption.py | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b2ef87d8a731..81b2e1f9f25f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -576,10 +576,13 @@ void CWallet::UpgradeDescriptorCache() bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase) { - bool fWasLocked = IsLocked(true); - { LOCK2(m_relock_mutex, cs_wallet); + const bool was_locked = IsLocked(true); + const bool was_mixing_only = fOnlyMixingAllowed; + const auto relock = interfaces::MakeCleanupHandler([this, was_locked] { + if (was_locked) Lock(); + }); Lock(); CCrypter crypter; @@ -590,7 +593,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, _vMasterKey)) return false; - if (Unlock(_vMasterKey)) + if (Unlock(_vMasterKey, was_mixing_only)) { constexpr MillisecondsDouble target{100}; auto start{SteadyClock::now()}; @@ -611,9 +614,6 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, if (!crypter.Encrypt(_vMasterKey, pMasterKey.second.vchCryptedKey)) return false; WalletBatch(GetDatabase()).WriteMasterKey(pMasterKey.first, pMasterKey.second); - if (fWasLocked) - Lock(); - return true; } } diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py index 57ab68b0c59e..6eec2e2efa24 100755 --- a/test/functional/wallet_encryption.py +++ b/test/functional/wallet_encryption.py @@ -24,6 +24,29 @@ def set_test_params(self): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_passphrase_change_permissions(self): + self.log.info("Passphrase changes preserve locked, mixing-only and full permissions") + node = self.nodes[0] + for mode in ("locked", "mixing", "full"): + node.createwallet("change-" + mode, descriptors=self.options.descriptors) + wallet = node.get_wallet_rpc("change-" + mode) + address = wallet.getnewaddress() + wallet.encryptwallet("old passphrase") + if mode != "locked": + wallet.walletpassphrase("old passphrase", 100, mode == "mixing") + wallet.walletpassphrasechange("old passphrase", "new passphrase") + if mode == "full": + signature = wallet.signmessage(address, "passphrase change") + assert node.verifymessage(address, signature, "passphrase change") + else: + assert_raises_rpc_error(-13, "Please enter the wallet passphrase", wallet.signmessage, address, "passphrase change") + if mode == "locked": + assert_raises_rpc_error(-13, "Please unlock wallet for mixing", wallet.coinjoin, "start") + else: + assert_equal(wallet.coinjoin("start"), "Mixing requested") + wallet.coinjoin("stop") + wallet.unloadwallet() + def run_test(self): passphrase = "WalletPassphrase" passphrase2 = "SecondWalletPassphrase" @@ -109,6 +132,8 @@ def run_test(self): assert_equal(self.nodes[0].getwalletinfo()['unlocked_until'], unlocked_until) self.nodes[0].walletlock() + self.test_passphrase_change_permissions() + if __name__ == '__main__': WalletEncryptionTest().main() From a1f37ea4497ae50ad5d630b1d3aee8d1935023e3 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 9 Oct 2026 00:26:16 -0500 Subject: [PATCH 13/40] Merge #7820: fix: hold chainstate lock during CoinJoin input validation a819870006e7d4f71b9cc116b89267417e22a573 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 3d18d42895ed48fc4cb3a30f95bd20a55d440561) --- src/coinjoin/coinjoin.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 6403c6b2a0a6..292e672f0a3f 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -371,7 +371,8 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll nFees -= txout.nValue; } - CCoinsViewMemPool viewMemPool(WITH_LOCK(::cs_main, return &active_chainstate.CoinsTip()), mempool); + LOCK(::cs_main); + CCoinsViewMemPool viewMemPool(&active_chainstate.CoinsTip(), mempool); for (const auto& txin : vin) { LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- txin=%s\n", __func__, txin.ToString()); From b23065710b211d53f4481a4c72d652a664617553 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 9 Oct 2026 12:43:50 -0500 Subject: [PATCH 14/40] Merge #7841: fix: authenticate cached InstantSend proof bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ecae783f2d8630124638f7c2880f7dc58f1993b6 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 524173efe6c04443fac46028e19af6b6f63b2de8) --- src/instantsend/net_instantsend.cpp | 13 +-- src/test/evo_islock_tests.cpp | 154 +++++++++++++++++++++------- 2 files changed, 122 insertions(+), 45 deletions(-) diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index 6c51289ad749..6fb5dbcc9e7a 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -127,12 +127,6 @@ std::unique_ptr NetInstantSend::BuildVeri auto id = islock->GetRequestId(); - // no need to verify an ISLOCK if we already have verified the recovered sig that belongs to it - if (m_sigman.HasRecoveredSig(llmq_params.type, id, islock->txid)) { - data->alreadyVerified++; - continue; - } - auto cycleHeightOpt = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), islock->cycleHash); if (!cycleHeightOpt) { data->batchVerifier.badSources.emplace(nodeId); @@ -156,6 +150,13 @@ std::unique_ptr NetInstantSend::BuildVeri data->batchVerifier.badMessages.emplace(hash); continue; } + llmq::CRecoveredSig recovered_sig; + if (m_sigman.GetRecoveredSig(llmq_params.type, id, islock->txid, recovered_sig) && + recovered_sig.getQuorumHash() == quorum->qc->quorumHash && recovered_sig.sig.Get() == sig) { + data->alreadyVerified++; + continue; + } + uint256 signHash = llmq::SignHash{llmq_params.type, quorum->qc->quorumHash, id, islock->txid}.Get(); data->batchVerifier.PushMessage(nodeId, hash, signHash, sig, quorum->qc->quorumPublicKey); data->verifyCount++; diff --git a/src/test/evo_islock_tests.cpp b/src/test/evo_islock_tests.cpp index c929cf884811..cf6f321d284e 100644 --- a/src/test/evo_islock_tests.cpp +++ b/src/test/evo_islock_tests.cpp @@ -14,6 +14,7 @@ #include #include #include +#include