From 65603d57afb345ec44e5fb22e814d589e7b0b04a Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 10 Nov 2026 21:32:39 +0700 Subject: [PATCH 01/12] Merge dashpay/dash#8088: fix: fetch miniupnpc sources from miniupnp.free.fr cba106e5026c7bafefebaf1e1f9d1f3c0e27561b fix(build): fetch miniupnpc sources from miniupnp.free.fr (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented curl: (28) Failed to connect to miniupnp.tuxfamily.org port 443 after 30002 ms: Timeout was reached Fetching miniupnpc-2.2.7.tar.gz from http://dash-depends-sources.s3-website-us-west-2.amazonaws.com/ % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 570 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 curl: (22) The requested URL returned error: 404 make[1]: *** [funcs.mk:301: /home/runner/work/dash/dash/depends/sources/download-stamps/.stamp_fetched-miniupnpc-2.2.7-b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1.hash] Error 22 make[1]: Leaving directory '/home/runner/work/dash/dash/depends' So apparently build with depends without cache can't succeed anymore. ## What was done? Point the package at the author's other mirror, http://miniupnp.free.fr, which serves the tarball with the expected hash. This is what Bitcoin Core did in bitcoin#30151 for the same outage but it had been reverted later by bitcoin core by mistake and never refixed, instead bitcoin core dropped miniupnpc altogether (bitcoin#31130), so there is no later upstream fix to backport. ## How Has This Been Tested? Can be tested by CI on https://github.com/dashpay/dash/pull/7939 when rebased on top of merged this PR ## 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 _(if you have permission to set one)_ Top commit has no ACKs. Tree-SHA512: c53eb4eaed3bdeb7d024bba61acf21e79054dbb2356d408397b1c7e4232e0bee3f511201d26f1e87b9b688e996501578253377d355d64f178d1de5ef1bf53426 (cherry picked from commit 3d85a15bafc1bc96515e372770db51a09fb5ecca) --- depends/packages/miniupnpc.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index e63752406107..6a0cfa8cc166 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -1,6 +1,6 @@ package=miniupnpc $(package)_version=2.2.7 -$(package)_download_path=https://miniupnp.tuxfamily.org/files/ +$(package)_download_path=http://miniupnp.free.fr/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1 $(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch From 033dae39439c79d6abee3b93b2009e16ac26adb4 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 10 Nov 2026 22:57:09 -0600 Subject: [PATCH 02/12] Merge dashpay/dash#8052: fix(qt): reserve the same shared masternode contribution again after a release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 01cda37af25029961383752f44e91aeafe1e19f6 fix(qt): reserve the same shared masternode contribution again after a release (pasta) Pull request description: ## Issue being fixed or feature implemented In the Qt shared masternode wizard, if the coordinator releases and re-reserves its coins after a participant has replied, that participant can no longer accept the locked terms. The coordinator gets no warning. GUI QA found this on the `v24.0.0-rc.7` tag (`111a2c533e19`), on regtest: 1. alice coordinates. She reserves her coins on "Your contribution" and saves the invitation, which carries her contribution, change address included. 2. bob opens the invitation and sends his Details. 3. alice goes back to "Your contribution", presses **Release**, pastes bob's Details and presses **Next**. Next reserves her coins again. 4. alice locks the terms and sends the locked terms to bob. **Result:** bob gets "The locked terms do not contain the details you sent." - alice's board shows every reply in, with no "must reply again" warning. - In the same window, bob can't load alice's current invitation either: the copy looks like nothing new to him. - His only way forward is to open the locked terms in a fresh dialog. No funds move and nothing is signed or broadcast, but the participant is stuck with an error that doesn't say what to do. **Why it happens:** the re-reserve in step 3 selects coins afresh, so alice's contribution gets a **new change address**. bob's copy still lists the contribution from the invitation, and `MnShareSession::adoptLockedTerms` compares every contribution it knows. **Why rc.6 completes the same steps:** absorbing bob's reply used to put alice's released contribution back and relock it. That was a separate bug, fixed by #8035, which keeps released contributions out of absorbed replies. With that fixed, the new contribution from the re-reserve is what gets locked. ## What was done? `SharedMnCreateDialog::reserveCoins()` now puts back the most recently released contribution for this share, unchanged, when all of these hold: - Reserve built it in this dialog from this wallet's own coin selection. That's tracked in `m_built_contributions`, compared with `MnShareSession::SameContribution`: inputs in order with their sequences, and the same change; - every input is still one Reserve could select, from the same `fundingCandidates()` list `selectCoins()` now uses: unspent, unlocked and spendable, with no unconfirmed coins the wallet doesn't trust; - the inputs minus the change still equal what the share needs: the share amount, plus the Network fee for the coordinator. A contribution with no change may also exceed it by a dust amount, which `selectCoins()` leaves to the fee in the same way (the shared `isDust()`). Otherwise it selects coins as before. In that case the contribution really did change, for example the fee or the share amount changed, or a coin was spent. Earlier replies are then out of date, as they are today. The "built here" rule matters because of what a reply can contain. A reply can list a contribution under this wallet's share name that spends another of this wallet's coins, with any change, sequences or repeated inputs the sender chose. The wallet already refuses to sign such a contribution, but a release would put it into the released list. A contribution absorbed from a reply is therefore never put back. Reserve builds the wallet's own contribution instead. `fundingCandidates()` is the coin filter `selectCoins()` already used, moved into its own function so that reuse and fresh selection accept the same coins. `reservableAgain()` makes the reuse decision once for both Reserve and the coins card on "Your contribution". While the coins are released, the card says it uses the coins "as reserved before" and that the change "returns to the same address of yours", instead of describing a fresh selection. **Known limits, left as is:** reuse doesn't survive reopening a saved session, because neither the released nor the built list is saved. Also, two changes can cancel out. For example, the coordinator releases, raises the fee and reserves, then releases again and restores the fee. The original contribution was replaced in between, so the second re-reserve can't bring it back and builds a new one. This is rare, and it's the general "the coordinator changed their own funding after replies" case, which needs a reply-again signal on the board rather than reuse. ## How Has This Been Tested? **New scenario:** `SharedMnWizardTests::reservingAgainKeepsEarlierReplies`, with a confirmed coin in a real wallet. - **Main case:** reserve, then take the invitation, then bob's and carol's replies to it. Release, absorb bob's reply and reserve again. - The contribution must match the one bob holds. - After a real `PrepareRegistration` + `freeze`, bob's `adoptLockedTerms` on the locked terms must succeed. - **A reply plants a contribution under alice's name:** the variants are change paid elsewhere, one coin listed twice, 100 duffs of change to this wallet, and an exactly funded input at sequence 1. After Release and Reserve, the wallet must go back to its own contribution each time, and the planted coin must stay unlocked. - **No change, dust remainder:** Reserve itself builds a no-change contribution from a coin 100 duffs over the share plus fee, with the other coins locked. After a release it is reserved again unchanged. - **Coins card:** a larger coin arrives after the first reservation. After Release, the card must describe the contribution Reserve will put back, not the larger coin. - **Fee changed:** after a release and a fee change, reserving again must select afresh. **Fails without the fix:** - With the reuse removed, the scenario fails at the check that the re-reserved contribution is the one sent (`sharedmnwizardtests.cpp:471`). - With reuse allowed for any released contribution, it fails at the first planted case (`:547`). With the card still showing a fresh selection, it fails at `:504` with "Uses 1 coin (600 tDASH); change 199.999 tDASH returns to a new address of yours". **Full `test_dash-qt` with the fix**, run headless (`QT_QPA_PLATFORM=minimal`) on Linux: 164 passed, 0 failed, 2 skipped. The skips are the OptionTests font tests, which can't run on the headless `minimal` platform. SharedMnWizardTests: 22 passed. **GUI on regtest**, run as a 2-party session with the steps above. Each binary was built from its exact commit: - **PR base** `9e5e3d59a1e3` and the **`v24.0.0-rc.7`** tag `111a2c533e19`: the re-reserve picks a new change address, and bob gets "The locked terms do not contain the details you sent." - **PR head at the time,** `eb4929384856`: the change address stays the same, bob gets Approve terms, and the registration confirms. Its change pays the address bob's Details listed. - **Head `496130ce0637`** (since superseded): the same flow was rerun with the same result: unchanged change address, Approve terms, ProRegTx `2db9621d…7834` confirmed. See [that evidence comment](https://github.com/dashpay/dash/pull/8052#issuecomment-6638363938). - **Head `55672100f0b0`:** it adds the coins-card change above. The same flow was rerun on it: unchanged change address, Approve terms, ProRegTx `59ba8be9…5c95` confirmed. After Release, the card reads "as reserved before … the same address of yours", where the base reads "a new address of yours". See [the current-head evidence comment](https://github.com/dashpay/dash/pull/8052#issuecomment-6639545618). - **Current head `01cda37af25`:** after `55672100` it adds only the reuse restriction above, through the untested intermediate `2a1bcc63`. The same flow was rerun on binaries built from exact `01cda37a`: unchanged change address, the "as reserved before … same address" coins card, Approve terms, and the ProRegTx `dcea870f…105d` confirmed. See [the current-head evidence comment](https://github.com/dashpay/dash/pull/8052#issuecomment-6641074891). Screenshots and provenance are in [the UI evidence comment](https://github.com/dashpay/dash/pull/8052#issuecomment-6637437445) and at [dash-ui-artifacts@`034c3d7` / dash/pr-8052](https://github.com/PastaPastaPasta/dash-ui-artifacts/blob/034c3d7dc83b896aca0182126aa748a72b4b1be9/dash/pr-8052/README.md). On the earlier pre-review head `46de5fe1dea`, the same flow also passed. So did a participant releasing and reserving again on the contribution page. A fee change after a reply gives a new contribution, as expected. ## 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 _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 0bb7afb3948fd947a73dd5b16f7e1b75ba3e10e650383609509d1220309b6baddb2244f110623c86badf51d7c8adf84dfca36d06482d92988ec7616903661825 (cherry picked from commit b2172a9cf8a35265c517253016a836879fee4ac4) --- src/qt/mnsharesession.cpp | 26 ++-- src/qt/mnsharesession.h | 3 + src/qt/sharedmncreatedialog.cpp | 126 ++++++++++++---- src/qt/sharedmncreatedialog.h | 14 ++ src/qt/test/sharedmnwizardtests.cpp | 223 ++++++++++++++++++++++++++++ src/qt/test/sharedmnwizardtests.h | 1 + 6 files changed, 349 insertions(+), 44 deletions(-) diff --git a/src/qt/mnsharesession.cpp b/src/qt/mnsharesession.cpp index cf96952a193d..5f6bcad18b66 100644 --- a/src/qt/mnsharesession.cpp +++ b/src/qt/mnsharesession.cpp @@ -104,19 +104,6 @@ bool SameShare(const MnShareSession::Share& a, const MnShareSession::Share& b) a.rewardAddress == b.rewardAddress; } -bool SameContribution(const MnShareSession::Contribution& a, const MnShareSession::Contribution& b) -{ - if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; - if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; - for (size_t i = 0; i < a.inputs.size(); ++i) { - if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || - a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { - return false; - } - } - return true; -} - bool IsTxidHex(const QString& txid) { return txid.size() == 64 && IsHex(txid.toStdString()); @@ -191,6 +178,19 @@ bool DecodeSharedProTx(const QString& protx_hex, CMutableTransaction& tx, CProRe } // anonymous namespace +bool MnShareSession::SameContribution(const Contribution& a, const Contribution& b) +{ + if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; + if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; + for (size_t i = 0; i < a.inputs.size(); ++i) { + if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || + a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { + return false; + } + } + return true; +} + namespace { //! U+202F NARROW NO-BREAK SPACE. BitcoinUnits groups thousands with a plain //! thin space, which a layout may break a number across. diff --git a/src/qt/mnsharesession.h b/src/qt/mnsharesession.h index 0dc8f0176ce0..fae57afc8662 100644 --- a/src/qt/mnsharesession.h +++ b/src/qt/mnsharesession.h @@ -313,6 +313,9 @@ class MnShareSession //! Whether a row with `share`'s owner address has the same amount and addresses here bool hasShare(const Share& share) const; + //! Same inputs, in order and with their sequences, and the same change. + //! Labels and changeIndex are ignored. + static bool SameContribution(const Contribution& a, const Contribution& b); //! Whether a contribution with `contribution`'s label spends the same coins and pays the same change here bool hasContribution(const Contribution& contribution) const; diff --git a/src/qt/sharedmncreatedialog.cpp b/src/qt/sharedmncreatedialog.cpp index 5588e7236022..e6c85718582c 100644 --- a/src/qt/sharedmncreatedialog.cpp +++ b/src/qt/sharedmncreatedialog.cpp @@ -1986,8 +1986,18 @@ void SharedMnCreateDialog::refreshContributionPage() const CAmount target{contributionTarget()}; CoinSelection selection; QString error; + CAmount again_total{0}; if (target <= 0) { m_coins_label->setText(tr("Choose which participant you are.")); + } else if (const auto* again{reservableAgain(&again_total)}) { + const QString coins{CoinCount(again->inputs.size())}; + m_coins_label->setText( + again->hasChange + ? tr("Uses %1 (%2) as reserved before; change %3 returns to the same address of yours.") + .arg(coins, FormatAmount(m_wallet_model, again_total), + FormatAmount(m_wallet_model, again->changeAmount)) + : tr("Uses %1 (%2) as reserved before, with no change left over.") + .arg(coins, FormatAmount(m_wallet_model, again_total))); } else if (selectCoins(target, selection, error)) { const QString coins{CoinCount(selection.inputs.size())}; m_coins_label->setText( @@ -2358,6 +2368,7 @@ void SharedMnCreateDialog::startSession() unlockAllContributedCoins(); m_coins_released = false; m_released_contributions.clear(); + m_built_contributions.clear(); m_session = MnShareSession(); m_role = Role::Coordinator; m_my_share = 0; @@ -3372,23 +3383,19 @@ CAmount SharedMnCreateDialog::contributionTarget() const return m_role == Role::Coordinator ? share + m_fee_field->value() : share; } -bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +bool SharedMnCreateDialog::isDust(CAmount value) const { - selection = CoinSelection{}; - error.clear(); - if (m_wallet_model == nullptr) { - error = tr("No wallet is available."); - return false; - } - if (target <= 0) { - error = tr("Select the participant that is you."); - return false; - } + const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; + return IsDust(probe, m_node.getDustRelayFee()); +} +std::vector> SharedMnCreateDialog::fundingCandidates() const +{ + std::vector> candidates; + if (m_wallet_model == nullptr) return candidates; // listCoins() already holds back unconfirmed coins the wallet does not // trust, so an InstantSend-locked coin or this wallet's own unconfirmed // change can fund a share, as it can any other payment - std::vector> candidates; for (const auto& [dest, coins] : m_wallet_model->wallet().listCoins()) { for (const auto& [outpoint, txout] : coins) { if (txout.is_spent) continue; @@ -3397,6 +3404,58 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, candidates.emplace_back(txout.txout.nValue, outpoint); } } + return candidates; +} + +bool SharedMnCreateDialog::fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out) const +{ + if (m_wallet_model == nullptr || released.inputs.empty()) return false; + const auto candidates{fundingCandidates()}; + CAmount total{0}; + for (const auto& input : released.inputs) { + const COutPoint outpoint{uint256S(input.txid.toStdString()), input.vout}; + const auto coin{std::find_if(candidates.begin(), candidates.end(), + [&](const auto& candidate) { return candidate.second == outpoint; })}; + if (coin == candidates.end()) return false; + total += coin->first; + } + if (total_out != nullptr) *total_out = total; + const CAmount target{contributionTarget()}; + if (released.hasChange) return total - released.changeAmount == target; + // Without change, a dust remainder goes to the fee, as selectCoins() leaves it + return total == target || (total > target && isDust(total - target)); +} + +const MnShareSession::Contribution* SharedMnCreateDialog::reservableAgain(CAmount* total_out) const +{ + if (m_my_share < 0 || static_cast(m_my_share) >= m_session.shares().size()) return nullptr; + const QString& label{m_session.shares()[m_my_share].label}; + // Only a contribution this wallet built itself goes back: one absorbed from + // a reply under this share's name can carry anything the sender chose + for (auto released{m_released_contributions.rbegin()}; released != m_released_contributions.rend(); ++released) { + if (released->label != label) continue; + const auto built{std::find_if(m_built_contributions.begin(), m_built_contributions.end(), [&](const auto& c) { + return c.label == label && MnShareSession::SameContribution(c, *released); + })}; + if (built != m_built_contributions.end() && fundsShareAgain(*built, total_out)) return &*built; + } + return nullptr; +} + +bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +{ + selection = CoinSelection{}; + error.clear(); + if (m_wallet_model == nullptr) { + error = tr("No wallet is available."); + return false; + } + if (target <= 0) { + error = tr("Select the participant that is you."); + return false; + } + + std::vector> candidates{fundingCandidates()}; // Largest first keeps the input count, and with it everyone's signing work // and the transaction fee, as small as possible. std::sort(candidates.begin(), candidates.end(), @@ -3405,13 +3464,9 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, // A change output below the dust threshold cannot be relayed, so keep // adding coins until the change clears it; if nothing does, drop the // change and let the remainder go to the fee. - const auto is_dust = [this](CAmount value) { - const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; - return IsDust(probe, m_node.getDustRelayFee()); - }; size_t used{0}; for (; used < candidates.size(); ++used) { - if (selection.total >= target && (selection.total == target || !is_dust(selection.total - target))) break; + if (selection.total >= target && (selection.total == target || !isDust(selection.total - target))) break; selection.total += candidates[used].first; MnShareSession::Input input; input.txid = QString::fromStdString(candidates[used].second.hash.ToString()); @@ -3428,7 +3483,7 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, return false; } const CAmount change{selection.total - target}; - selection.change = (change > 0 && !is_dust(change)) ? change : 0; + selection.change = (change > 0 && !isDust(change)) ? change : 0; return true; } @@ -3475,28 +3530,37 @@ void SharedMnCreateDialog::reserveCoins() return; } - CoinSelection selection; - if (!selectCoins(contributionTarget(), selection, error)) { - showError(error); - return; - } + // Whoever replied before a release holds the contribution as it was then, + // and refuses locked terms with any other. While its coins are still free + // and fund the same amount, reserve exactly that contribution again. MnShareSession::Contribution contribution; - contribution.label = share.label; - contribution.inputs = selection.inputs; - if (selection.change > 0) { - const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; - if (change_address.isEmpty()) { + const auto* const again{reservableAgain()}; + if (again != nullptr) { + contribution = *again; + } else { + CoinSelection selection; + if (!selectCoins(contributionTarget(), selection, error)) { showError(error); return; } - contribution.hasChange = true; - contribution.changeAddress = change_address; - contribution.changeAmount = selection.change; + contribution.label = share.label; + contribution.inputs = selection.inputs; + if (selection.change > 0) { + const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; + if (change_address.isEmpty()) { + showError(error); + return; + } + contribution.hasChange = true; + contribution.changeAddress = change_address; + contribution.changeAmount = selection.change; + } } if (!m_session.addContribution(contribution, error)) { showError(error); return; } + if (again == nullptr) m_built_contributions.push_back(contribution); setContributionLocked(contribution, /*lock=*/true); m_coins_released = false; // Coins reserved again are no longer released diff --git a/src/qt/sharedmncreatedialog.h b/src/qt/sharedmncreatedialog.h index 9a33083524a1..808ad1088c84 100644 --- a/src/qt/sharedmncreatedialog.h +++ b/src/qt/sharedmncreatedialog.h @@ -21,6 +21,7 @@ #include #include #include +#include #include struct SharedMnOwnFunding; @@ -274,6 +275,16 @@ private Q_SLOTS: void followNewerInvitation(const MnShareSession& imported); // Funding + //! Whether a P2PKH output of `value` would be dust + bool isDust(CAmount value) const; + //! This wallet's unspent, unlocked, spendable coins that may fund a share + std::vector> fundingCandidates() const; + //! Whether `released`'s coins are all still funding candidates, and fund + //! exactly what the share needs now + bool fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out = nullptr) const; + //! The most recently released contribution of this wallet's share that + //! Reserve would put back as it was, with its input total, or nullptr + const MnShareSession::Contribution* reservableAgain(CAmount* total_out = nullptr) const; //! Confirmed, unlocked, spendable wallet coins covering `target`, largest //! first, with the change kept above the dust threshold when possible bool selectCoins(CAmount target, CoinSelection& selection, QString& error) const; @@ -409,6 +420,9 @@ private Q_SLOTS: //! less any coins reserved again since. A reply to an earlier invitation //! still carries them, and only Reserve may put them back. std::vector m_released_contributions; + //! Contributions Reserve has reserved, built from this wallet's own coin + //! selection or put back: the only ones it may put back after a release + std::vector m_built_contributions; Role m_role{Role::Undecided}; int m_my_share{-1}; std::optional m_validation_page; diff --git a/src/qt/test/sharedmnwizardtests.cpp b/src/qt/test/sharedmnwizardtests.cpp index 50ae27f77f61..f4967448649c 100644 --- a/src/qt/test/sharedmnwizardtests.cpp +++ b/src/qt/test/sharedmnwizardtests.cpp @@ -75,6 +75,7 @@ using MasternodeTestUtil::FreshOperatorPubKey; using MasternodeTestUtil::FreshP2PKHAddress; using MasternodeTestUtil::DraftReply; using MasternodeTestUtil::MakeSpendingWallet; +using MasternodeTestUtil::MakeTestWallet; using MasternodeTestUtil::PrepareRegistration; using MasternodeTestUtil::PreparedRegistration; using MasternodeTestUtil::WalletGuard; @@ -400,6 +401,228 @@ void SharedMnWizardTests::coordinatorResumesSavedSession() QCOMPARE(int(participant.currentPage()), int(SharedMnCreateDialog::PageContribution)); } +void SharedMnWizardTests::reservingAgainKeepsEarlierReplies() +{ + TestChain100Setup test; + m_node.setContext(&test.m_node); + WalletContext& context{*m_node.walletLoader().context()}; + const auto wallet{MakeTestWallet(m_node, context, "coord")}; + QVERIFY(wallet != nullptr); + WalletGuard guard{context, wallet}; + CTxDestination owner; + COutPoint coin; + COutPoint other_coin; + COutPoint twice_coin; + COutPoint dust_coin; + COutPoint dust_change_coin; + COutPoint sequence_coin; + COutPoint larger_coin; + { + LOCK(wallet->cs_wallet); + const auto destination{wallet->GetNewDestination("")}; + QVERIFY(destination); + owner = *destination; + const CBlockIndex* tip{ + WITH_LOCK(test.m_node.chainman->GetMutex(), return test.m_node.chainman->ActiveChain().Tip())}; + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); + CMutableTransaction funding; + funding.vin.emplace_back(GetRandHash(), 0); + funding.vout.emplace_back(450 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/0}) != nullptr); + coin = COutPoint{funding.GetHash(), 0}; + CMutableTransaction other_funding; + other_funding.vin.emplace_back(GetRandHash(), 0); + other_funding.vout.emplace_back(420 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(other_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}) != nullptr); + other_coin = COutPoint{other_funding.GetHash(), 0}; + // 410 for a contribution listing its coin twice; one with a dust + // remainder over a share of 400 and the default 0.001 fee + CMutableTransaction more_funding; + more_funding.vin.emplace_back(GetRandHash(), 0); + more_funding.vout.emplace_back(410 * COIN, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(405 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(more_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/2}) != nullptr); + twice_coin = COutPoint{more_funding.GetHash(), 0}; + dust_coin = COutPoint{more_funding.GetHash(), 1}; + dust_change_coin = COutPoint{more_funding.GetHash(), 2}; + sequence_coin = COutPoint{more_funding.GetHash(), 3}; + // A larger coin a fresh selection would prefer, held back until needed + CMutableTransaction larger_funding; + larger_funding.vin.emplace_back(GetRandHash(), 0); + larger_funding.vout.emplace_back(600 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(larger_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/3}) != nullptr); + larger_coin = COutPoint{larger_funding.GetHash(), 0}; + } + MasternodeTestUtil::GuiModels models{m_node}; + QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated))); + WalletModel wallet_model(interfaces::MakeWallet(context, wallet), models.client); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + + // This wallet coordinates and has answered for alice's row, but has not + // reserved its coins yet + CBLSSecretKey operator_secret; + operator_secret.MakeNewKey(); + MnShareSession draft{InvitationSession()}; + draft.setPrepareWallet(QStringLiteral("coord")); + draft.terms().operatorPubKey = + QString::fromStdString(operator_secret.GetPublicKey().ToString(/*specificLegacyScheme=*/false)); + draft.shares()[0].ownerAddress = QString::fromStdString(EncodeDestination(owner)); + draft.shares()[0].refundAddress = FreshP2PKHAddress(); + + SharedMnCreateDialog dialog(m_node, &wallet_model, /*parent=*/nullptr); + dialog.handleImportedText(draft.toJsonString()); + QCOMPARE(int(dialog.m_role), int(SharedMnCreateDialog::Role::Coordinator)); + QCOMPARE(dialog.myShareIndex(), 0); + dialog.goToPage(SharedMnCreateDialog::PageContribution); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution sent{*dialog.myContribution()}; + QVERIFY(sent.hasChange); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // bob and carol answer the invitation that carried this contribution + const MnShareSession invitation{dialog.m_session}; + const MnShareSession bob{DraftReply(invitation, 1, FakeTxid('2'))}; + const MnShareSession carol{DraftReply(invitation, 2, FakeTxid('3'))}; + QVERIFY(bob.hasContribution(sent)); + + // The coordinator releases its coins, takes in bob's reply, and reserves + // again: the same coins and change, which bob's copy already lists + dialog.releaseCoins(); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + dialog.handleImportedText(bob.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.myContribution() == nullptr); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // The coins card describes those coins, not the larger one that has + // become available since and a fresh selection would take + const auto amount = [&](CAmount value) { return SharedMnFormatAmount(SharedMnDisplayUnit(&wallet_model), value); }; + wallet_model.wallet().unlockCoin(larger_coin); + dialog.refreshContributionPage(); + const QString preview{dialog.m_coins_label->text()}; + QVERIFY2(preview.contains(amount(sent.changeAmount)) && !preview.contains(amount(600 * COIN)), qPrintable(preview)); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // So the terms locked from it are the ones bob agreed to answer + dialog.handleImportedText(carol.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.allDetailsCollected()); + MnShareSession locked{dialog.m_session}; + const PreparedRegistration prepared{PrepareRegistration(locked, operator_secret)}; + QString error; + QVERIFY2(locked.freeze(prepared.txHex, prepared.consentHashHex, prepared.collateralIndex, error), + qPrintable(error)); + MnShareSession bob_copy{bob}; + QVERIFY2(bob_copy.adoptLockedTerms(locked, error), qPrintable(error)); + + // A contribution a reply lists under alice's name is never reserved again + // after a release, however well it funds the share: this wallet goes back + // to the one it built. First, one spending another coin of this wallet + // with its change paid elsewhere. + dialog.releaseCoins(); + MnShareSession planting{dialog.m_session}; + MnShareSession::Contribution planted; + planted.label = sent.label; + planted.inputs = {{QString::fromStdString(other_coin.hash.ToString()), other_coin.n}}; + planted.hasChange = true; + planted.changeAddress = FreshP2PKHAddress(); + planted.changeAmount = 420 * COIN - (400 * COIN + dialog.m_fee_field->value()); + QVERIFY2(planting.addContribution(planted, error), qPrintable(error)); + dialog.handleImportedText(planting.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(planted)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(planted)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(other_coin)); + + // One listing a coin twice, with this wallet's own change address + const auto plant_and_reserve_again = [&](const MnShareSession::Contribution& contribution) { + dialog.releaseCoins(); + MnShareSession reply{dialog.m_session}; + QString plant_error; + QVERIFY2(reply.addContribution(contribution, plant_error), qPrintable(plant_error)); + dialog.handleImportedText(reply.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(contribution)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + }; + MnShareSession::Contribution twice{planted}; + const MnShareSession::Input twice_input{QString::fromStdString(twice_coin.hash.ToString()), twice_coin.n}; + twice.inputs = {twice_input, twice_input}; + twice.changeAddress = sent.changeAddress; + twice.changeAmount = 2 * 410 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(twice); + QVERIFY(!dialog.m_session.hasContribution(twice)); + QVERIFY(dialog.m_session.hasContribution(sent)); + + // One with 100 duffs of change to this wallet, which could never be relayed + MnShareSession::Contribution dust_change{twice}; + dust_change.inputs = {{QString::fromStdString(dust_change_coin.hash.ToString()), dust_change_coin.n}}; + dust_change.changeAmount = 100; + plant_and_reserve_again(dust_change); + QVERIFY(!dialog.m_session.hasContribution(dust_change)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(dust_change_coin)); + + // One that funds the share exactly, with change to this wallet, but whose + // input has a relative-lock sequence this wallet refuses to sign + MnShareSession::Contribution relative{twice}; + relative.inputs = {{QString::fromStdString(sequence_coin.hash.ToString()), sequence_coin.n, /*sequence=*/1}}; + relative.changeAmount = 405 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(relative); + QVERIFY(!dialog.m_session.hasContribution(relative)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(sequence_coin)); + + // Without change, a dust remainder over the share is left to the fee. A + // contribution Reserve built that way is reserved again as it was. + dialog.releaseCoins(); + const std::vector held{coin, other_coin, twice_coin, dust_change_coin, sequence_coin}; + for (const auto& outpoint : held) { + wallet_model.wallet().lockCoin(outpoint, /*write_to_db=*/false); + } + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution no_change{*dialog.myContribution()}; + QVERIFY(!no_change.hasChange); + QCOMPARE(no_change.inputs.size(), size_t{1}); + QCOMPARE(no_change.inputs[0].txid, QString::fromStdString(dust_coin.hash.ToString())); + for (const auto& outpoint : held) { + wallet_model.wallet().unlockCoin(outpoint); + } + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY(dialog.m_session.hasContribution(no_change)); + QVERIFY(wallet_model.wallet().isLockedCoin(dust_coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // With a different fee the released coins no longer fund the share as they + // did, so reserving again selects anew and earlier replies are out of date + dialog.releaseCoins(); + dialog.m_fee_field->setValue(dialog.m_fee_field->value() + COIN / 1000); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(sent)); + QCOMPARE(dialog.myContribution()->changeAmount, sent.changeAmount - COIN / 1000); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + wallet_model.wallet().unlockCoin(coin); +} + void SharedMnWizardTests::coordinatorResumesFullySignedSession() { TestingSetup test{CBaseChainParams::REGTEST}; diff --git a/src/qt/test/sharedmnwizardtests.h b/src/qt/test/sharedmnwizardtests.h index 7c96caad531f..24e961845949 100644 --- a/src/qt/test/sharedmnwizardtests.h +++ b/src/qt/test/sharedmnwizardtests.h @@ -29,6 +29,7 @@ private Q_SLOTS: void participantPagesAndRoleInference(); void participantCannotChooseATakenShare(); void coordinatorResumesSavedSession(); + void reservingAgainKeepsEarlierReplies(); void coordinatorResumesFullySignedSession(); void signedSessionReservation(); void unauthorisedSignedInputsAreDetected(); From ec9e944a26b6f1b44a213375bff3dfb907795e66 Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 11 Nov 2026 10:56:59 -0600 Subject: [PATCH 03/12] Merge dashpay/dash#8094: fix(net): stop busy-looping while a peer's send buffer is full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19da39cf27d53f3a85935ece4ad9df0b73a4d49a fix(net): wait for socket writability after failed sends (pasta) Pull request description: ## Issue being fixed or feature implemented When a nonblocking `send()` to a peer fails with a would-block error (the peer has stopped reading and its TCP window is full), `CConnman::SocketSendData` leaves `fCanSendData` set. The peer therefore stays in the sendable set, the socket handler keeps polling with a zero timeout, and the net thread busy-loops until the peer starts reading again or times out. Before the transport refactor (bitcoin#28165, merged here in 63962ec47564) a failed send cleared the flag; that reset was lost in the merge. ## What was done? Clear `fCanSendData` whenever the send returns an error, so the next iteration waits for the socket to become writable again. The empty-send path and partial successful sends are unchanged. ## How Has This Been Tested? Reproduced with a functional script that connects a peer which never reads and streams pings at the node: before the change the node's net thread used ~100% of a CPU core (9.86 s of CPU vs 9.88 s for a reference busy loop); with the change it used 0.02-0.03 s over three runs. That script measures CPU time, so it is not added as a CI test. `test_dash --run_test=net_tests,sock_tests`, `p2p_ping.py` and `p2p_v2_transport.py` pass. ## 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 _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 0af7f3b48164fae294d81f828cf1d5c25f6cb86a41257dd4c981116a30a8fc5c976be0345f389f03033ec390ecc1c42853bf161350be9f8f14e1f011f63d923d (cherry picked from commit 34f990e8701e4fd26cb702f922f70db03ce47fed) --- src/net.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net.cpp b/src/net.cpp index c248af0f4ff9..9e484785b456 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1771,6 +1771,7 @@ std::pair CConnman::SocketSendData(CNode& node) const LogPrint(BCLog::NET, "socket send error for peer=%d: %s\n", node.GetId(), NetworkErrorString(nErr)); node.fDisconnect = true; } + node.fCanSendData = false; } break; } From b051d3a0b848de7156aeed17345de03ed7bcf4e2 Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 11 Nov 2026 11:08:23 -0600 Subject: [PATCH 04/12] Merge dashpay/dash#8080: test(qt): stop address book tests from answering the shared masternode wizard's prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit a5618a0e83abfc809f5feac7c81d61ffda8fe254 test(qt): don't leave address book message answers armed after the edit (pasta) Pull request description: ## Issue being fixed or feature implemented Fixes #8075. With a distribution Qt, `test_dash-qt` fails in `SharedMnWizardTests::signedSessionReservation()`. It either hangs until QtTest's 300 s watchdog fires, or aborts with `free(): invalid size`. Builds against depends' Qt pass. The shared masternode wizard is not at fault. `AddressBookTests` leaves two answers to a message box behind. `EditAddressAndSubmit()` calls `ConfirmMessage(&warning_text, 5ms)` before every edit, but a successful edit opens no box. Its timer then outlives the function, holding a pointer to the function's stack `QString`. The two successful edits run last, so both timers are still pending when the suite ends. No later suite runs an event loop until the wizard's "Keep this session for later?" prompt (added in #8035, which is why the bisect lands there). Whether that prompt survives depends on Qt's event dispatcher: - **Unix dispatcher** (depends builds Qt with `-no-glib`): the test's own "Cancel" ends the modal loop before timers are checked. The stale timers fire later, when no box is left, and do nothing. - **GLib dispatcher** (what Fedora's and other distribution Qt packages use): the timers fire in the same iteration as the test's answer. They write the box's text through the dangling pointer and click its default button, "Save and Close", over the test's "Cancel". The dialog then opens a save-file dialog that nothing answers, which is the hang. When the stray write hits a live frame instead, the result is `free(): invalid size`. The hardening flags in the report are incidental: the same Fedora build passes with `QT_NO_GLIB=1`. Bitcoin Core has the same `EditAddressAndSubmit()`, but no modal test runs after it there. Since #8077, `AddressBookTests` also runs on macOS, so the stale timers exist there too. ## What was done? `ConfirmMessage()` takes an optional context object, and `EditAddressAndSubmit()` scopes it to the edit, so a box that never opens leaves nothing armed. The existing address book scenario now also opens a message box after the successful edits and checks that its own answer wins. No production code changes. The dialog did what the stale click told it to do. ## How Has This Been Tested? Reproduced in a Fedora 44 container (GCC 16.2.1, system Qt 5.15.18) with the exact `CFLAGS`/`CXXFLAGS`/`LDFLAGS`/`configure` line from the issue. Results are for the full `test_dash-qt`. The "develop" column is develop at 4c5c0153500, and the "This PR" column is this PR on 8dbdcb4a5b9. | Build | Dispatcher | develop | This PR | |---|---|---|---| | Fedora RPM flags | GLib (default) | hangs in `signedSessionReservation` | 163 passed, 0 failed | | Fedora RPM flags | Unix (`QT_NO_GLIB=1`) | 163 passed | 163 passed, 0 failed | | ASan+UBSan, `detect_stack_use_after_return=1` | GLib | **stack-use-after-return** in the `ConfirmMessage` lambda (`qt/test/util.cpp:22`), inside the wizard prompt's `exec()` | 163 passed, no ASan report | | ASan+UBSan | Unix | passes | 163 passed, no ASan report | | Default depends build, `--enable-werror` | Unix | passes | 163 passed, 0 failed | | Fedora RPM flags under valgrind memcheck, `QT_ENABLE_REGEXP_JIT=0` | GLib | n/a (hangs) | 163 passed, 0 errors | - A gdb breakpoint on `QMessageBox::buttonClicked` during the hang showed the test's "Cancel" click, then two clicks from `ConfirmMessage`'s lambda in the same `exec()`. - With `scope` replaced by `nullptr`, the new address book check fails with either dispatcher: "Compared pointers are not the same", or a crash in the stale callback at `util.cpp:22`. - UBSan reports the existing misaligned store in the vendored X11 code (`crypto/x11/sph_types.h:613`), with and without this PR. On this PR's base it also reports one invalid `bool` load at `qt/transactionrecord.h:70`, in code this PR does not touch: `decomposeTransaction()` copies a record into its list before setting its uninitialised `involvesWatchAddress`. With Qt's regexp JIT left on, valgrind's only reports are uninitialised-value warnings in JIT-generated code. With the JIT off there are none. - clang-format-diff (18.1.8) and `lint-whitespace.py` are clean. ## Breaking Changes None. Test code only. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: d68bacc7990c68c1966cf0d020795d7638b29b9f49d5bc42620189fffa3afb36585bcbab9653f95bccfbda1f9dcf8d0fd05ea7e2cd3c3955f762230c186e4b11 (cherry picked from commit 6cedd349cb7855bbcb059f0c0ba92172964e4769) --- src/qt/test/addressbooktests.cpp | 13 ++++++++++++- src/qt/test/util.cpp | 4 ++-- src/qt/test/util.h | 5 ++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index df8888215cad..9b9266630f4f 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -22,6 +22,7 @@ #include +#include #include #include #include @@ -47,11 +48,14 @@ void EditAddressAndSubmit( const QString& label, const QString& address, QString expected_msg) { QString warning_text; + // A successful edit opens no message box, and the answer must not wait + // for the next one, which belongs to whoever opens it + QObject scope; dialog->findChild("labelEdit")->setText(label); dialog->findChild("addressEdit")->setText(address); - ConfirmMessage(&warning_text, 5ms); + ConfirmMessage(&warning_text, 5ms, &scope); dialog->accept(); QCOMPARE(warning_text, expected_msg); } @@ -167,6 +171,13 @@ void TestAddAddressesToSendBook(interfaces::Node& node) check_addbook_size(4); QCOMPARE(table_view->model()->rowCount(), 3); + // The next message box is answered by whoever opens it + QMessageBox later(QMessageBox::Question, QString(), QString(), QMessageBox::Yes | QMessageBox::No); + later.setDefaultButton(QMessageBox::Yes); + QTimer::singleShot(100ms, &later, [&later] { later.button(QMessageBox::No)->click(); }); + later.exec(); + QCOMPARE(later.clickedButton(), later.button(QMessageBox::No)); + auto search_line = address_book.findChild("searchLineEdit"); search_line->setText(r_label); diff --git a/src/qt/test/util.cpp b/src/qt/test/util.cpp index d91b90fbba29..2646c7be4df2 100644 --- a/src/qt/test/util.cpp +++ b/src/qt/test/util.cpp @@ -13,9 +13,9 @@ #include #include -void ConfirmMessage(QString* text, std::chrono::milliseconds msec) +void ConfirmMessage(QString* text, std::chrono::milliseconds msec, QObject* context) { - QTimer::singleShot(msec, [text]() { + QTimer::singleShot(msec, context, [text]() { for (QWidget* widget : QApplication::topLevelWidgets()) { if (widget->inherits("QMessageBox")) { QMessageBox* messageBox = qobject_cast(widget); diff --git a/src/qt/test/util.h b/src/qt/test/util.h index a1788ca74e61..aa9253c25d6c 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -10,6 +10,7 @@ #include QT_BEGIN_NAMESPACE +class QObject; class QString; QT_END_NAMESPACE @@ -18,7 +19,9 @@ QT_END_NAMESPACE * * @param text - Optionally store dialog text. * @param msec - Number of milliseconds to pause before triggering the callback. + * @param context - Optionally cancel the callback when this object is destroyed, + * so it cannot answer a later message box or outlive `text`. */ -void ConfirmMessage(QString* text, std::chrono::milliseconds msec); +void ConfirmMessage(QString* text, std::chrono::milliseconds msec, QObject* context = nullptr); #endif // BITCOIN_QT_TEST_UTIL_H From d00c4e669a147f12ad8f0f139242ca0d0cc6ebb0 Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 11 Nov 2026 13:18:02 -0600 Subject: [PATCH 05/12] Merge dashpay/dash#8104: fix: keep final commitments mineable when their block is rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 393695393c4f6ca1ad2b82272f6118ba5bd4a936 fix: keep final commitments mineable when their block is rejected (pasta) Pull request description: ## Issue being fixed or feature implemented `CQuorumBlockProcessor::ProcessCommitment()` removed a block's final commitments from the mineable pool while the block was still being validated. `ConnectBlock()` processes quorum commitments early, before inputs, scripts, the coinbase amount/payee checks and several special-transaction checks. A block carrying the commitment of a finished DKG that then failed one of those later checks was never connected, yet every node that validated it (compact blocks are relayed before validation) had dropped the commitment from its pool. Nothing re-adds it, so that quorum was not mined in that DKG cycle. ## What was done? - `ProcessCommitment()` no longer touches the mineable pool, apart from dropping its caches. - New `CQuorumBlockProcessor::RemoveMineableCommitmentsForBlock()` removes the same entries as before, called once `ConnectBlock()` has accepted the block (next to where the BLS scheme switch is committed) and in `RollforwardBlock()`. - Check-only validation (`fJustCheck` / `TestBlockValidity`) returns before that point and never touches the pool. `UndoBlock()` still re-adds the commitments of a disconnected block, so reorgs behave as before. Re-adding on the failure path instead was rejected: a block can fail partway through its commitments, which could re-pool a commitment that was never verified. This doesn't overlap with #8048, which changes `AddMineableCommitment` only. ## How Has This Been Tested? `feature_llmq_dkgerrors.py`: after the existing `invalidateblock` check, the test submits the same commitment-carrying block with the coinbase paying 1 duff too much, expects `bad-cb-amount`, and asserts the commitment is still listed as mineable in `quorum dkgstatus`. It fails on `develop` and passed three times with this change. A five-node reproduction also fails on `develop`; with the fix every node keeps the commitment and the quorum is mined within its window. `llmq_*`, `evo_*` and `validation_chainstatemanager_tests` unit tests pass, as do `feature_llmq_rotation.py`, `feature_llmq_is_cl_conflicts.py`, `feature_llmq_evo.py`, `feature_llmq_signing.py`, `feature_llmq_simplepose.py`, `feature_llmq_singlenode.py`, `feature_llmq_dkg_intake.py`, `feature_dip4_coinbasemerkleroots.py`, `feature_quorum_proof_chain.py`, `rpc_quorum.py`, `feature_new_quorum_type_activation.py` and `feature_abortnode.py`. ## 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 _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: a306a6fbf6613ca25dfce8665815c94833ebeb012642f492db882ea7154ac1a987bfe5c0afbebcc7d09762fcf8373e664ea3bb2a82e0c5921d87723426e2f4b6 v24.0.x adaptations: Chainstate has no m_special_tx member here, so ConnectBlock() and RollforwardBlock() call the processor through m_chain_helper->special_tx like the neighbouring special-tx calls. CQuorumBlockProcessor keeps the v24.0.x GetCommitmentsFromBlock(), and RemoveMineableCommitmentsForBlock()'s lock annotation drops !m_commitment_payload_mutex, which only exists with #7936. (cherry picked from commit ce9ef663c9e7fbd3d52e4f8a3d7615cac5759f42) --- src/evo/specialtxman.cpp | 5 ++++ src/evo/specialtxman.h | 2 ++ src/llmq/blockprocessor.cpp | 28 ++++++++++++++++------- src/llmq/blockprocessor.h | 3 +++ src/validation.cpp | 2 ++ test/functional/feature_llmq_dkgerrors.py | 11 +++++++++ 6 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 8cbceb5d6180..7b7a29ba4c4c 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -1093,6 +1093,11 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const Chainstate& chainstate, co return true; } +void CSpecialTxProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) +{ + m_qblockman.RemoveMineableCommitmentsForBlock(block, pindex); +} + bool CSpecialTxProcessor::CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx, CAmount blockSubsidy, BlockValidationState& state) { diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 95a64bc23157..a6b5f0e9863c 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -97,6 +97,8 @@ class CSpecialTxProcessor EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool UndoSpecialTxsInBlock(const Chainstate& chainstate, const CBlock& block, const CBlockIndex* pindex, MNListUpdates& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 82b0a87a81a1..24385a7bd728 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -254,8 +255,7 @@ bool CQuorumBlockProcessor::ProcessBlock(Chainstate& chainstate, const CBlock& b // ProcessCommitment accepts a commitment identical to the stored one, so // rotation's multiple commitments per type need an explicit per-block check. - // It runs before any commitment is processed, because ProcessCommitment - // updates in-memory state that a rejected block would not restore. + // It runs before the signature checks, so a duplicate is rejected cheaply. // Null commitments stay exempt: they were never subject to bad-qc-dup. std::set> seen_quorums; for (const auto& [_, qc] : qcs) { @@ -445,12 +445,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh // invalidation should not depend on that. DropQcHashesCache(); - { - LOCK(minableCommitmentsCs); - mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash); - minableCommitmentsByQuorum.erase(cacheKey); - minableCommitments.erase(::SerializeHash(qc)); - } + WITH_LOCK(minableCommitmentsCs, mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash)); LogPrint(BCLog::LLMQ, "%s -- processed commitment from block. type=%d, quorumIndex=%d, quorumHash=%s\n", __func__, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString()); @@ -561,6 +556,23 @@ bool CQuorumBlockProcessor::UndoBlock(const Chainstate& chainstate, const CBlock return true; } +void CQuorumBlockProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) +{ + AssertLockHeld(::cs_main); + + std::multimap qcs; + if (BlockValidationState dummy; !Assume(GetCommitmentsFromBlock(block, pindex, qcs, dummy))) { + return; + } + + LOCK(minableCommitmentsCs); + for (const auto& [_, qc] : qcs) { + if (qc.IsNull()) continue; + minableCommitmentsByQuorum.erase(std::make_pair(qc.llmqType, qc.quorumHash)); + minableCommitments.erase(::SerializeHash(qc)); + } +} + bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, gsl::not_null pindex, std::multimap& ret, BlockValidationState& state) { AssertLockHeld(::cs_main); diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index c248a13fd29f..396ab1245310 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -108,6 +108,9 @@ class CQuorumBlockProcessor bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex); bool UndoBlock(const Chainstate& chainstate, const CBlock& block, gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex); + //! Drop the commitments mined by a block that has been fully connected from the mineable pool. + void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); //! it returns hash of commitment if it should be relay, otherwise nullopt std::optional AddMineableCommitment(const CFinalCommitment& fqc) EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); diff --git a/src/validation.cpp b/src/validation.cpp index 71a55d70615b..7813be6921b9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2925,6 +2925,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Block is committed: keep the scheme it switched to (fJustCheck dry runs returned above). bls_scheme_guard.Commit(); + m_chain_helper->special_tx->RemoveMineableCommitmentsForBlock(block, pindex); if (this == &m_chainman.ActiveChainstate() && mnlist_updates.diff.HasChanges()) { GetMainSignals().NotifyMasternodeListChanged(false, mnlist_updates.old_list, mnlist_updates.diff); @@ -5218,6 +5219,7 @@ bool Chainstate::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& in return false; } RecordBackgroundMNListHash(pindex, mnlist_updates.new_list); + m_chain_helper->special_tx->RemoveMineableCommitmentsForBlock(block, pindex); for (size_t i = 0; i < block.vtx.size(); i++) { const CTransactionRef& tx = block.vtx[i]; diff --git a/test/functional/feature_llmq_dkgerrors.py b/test/functional/feature_llmq_dkgerrors.py index a150a4dfc5b7..4c466279a8ce 100755 --- a/test/functional/feature_llmq_dkgerrors.py +++ b/test/functional/feature_llmq_dkgerrors.py @@ -116,6 +116,17 @@ def test_qc(self): # TODO: test signers # TODO: test validMembers + # The commitment is mineable again after invalidateblock and must stay so when a block that + # carries it is rejected by a check that runs after the commitments were processed + assert self.node_has_quorum_commitment(self.nodes[0], quorumHash, 100) + bad_block = copy.deepcopy(block) + bad_block.vtx[0].vout[0].nValue += 1 + bad_block.vtx[0].rehash() + bad_block.hashMerkleRoot = bad_block.calc_merkle_root() + bad_block.solve() + assert_equal(self.nodes[0].submitblock(bad_block.serialize().hex()), 'bad-cb-amount') + assert self.node_has_quorum_commitment(self.nodes[0], quorumHash, 100) + self.nodes[0].reconsiderblock(best) # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(self.nodes)) From 117a6cbda6af414f1f831be6396232ebdcfc6761 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 02:22:03 +0000 Subject: [PATCH 06/12] fix: match governance voting keys by registration hash across reorgs Resolve updated masternodes in the destination list and match their previous entries by registration hash. Skip missing entries when removing invalid votes. Co-Authored-By: GPT 6.1 Sol (cherry picked from commit 9071a084133d9b9d4b28694933657f824150d727) --- src/governance/governance.cpp | 18 +++-- src/test/governance_vote_processing_tests.cpp | 73 ++++++++++++++++++- 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 138cd5ac8f7f..4cc802738c60 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -1190,10 +1190,14 @@ void CGovernanceManager::RemoveInvalidVotes() std::vector changedKeyMNs; for (const auto& [internalId, pdmnState] : diff.updatedMNs) { - auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId); - // BuildDiff will construct itself with MNs that we already have knowledge - // of, meaning that fetch operations should never fail. - assert(oldDmn); + auto newDmn = tip_mn_list.GetMNByInternalId(internalId); + if (!newDmn) { + continue; + } + auto oldDmn = lastMNListForVotingKeys->GetMN(newDmn->proTxHash); + if (!oldDmn) { + continue; + } if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) { changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { @@ -1202,9 +1206,9 @@ void CGovernanceManager::RemoveInvalidVotes() } for (const auto& id : diff.removedMns) { auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(id); - // BuildDiff will construct itself with MNs that we already have knowledge - // of, meaning that fetch operations should never fail. - assert(oldDmn); + if (!oldDmn) { + continue; + } changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index e2f28c79128e..1b87452778ff 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include #include @@ -88,8 +90,8 @@ struct GovernanceVoteSetup : public TestChainSetup { int64_t proposal_time{0}; std::string proposal_payment_address; - GovernanceVoteSetup() : - TestChainSetup(DIP3_ACTIVATION_HEIGHT - 2, CBaseChainParams::REGTEST, {"-dip3params=109:500"}) + explicit GovernanceVoteSetup(const char* dip3_params = "-dip3params=109:500") : + TestChainSetup(DIP3_ACTIVATION_HEIGHT - 2, CBaseChainParams::REGTEST, {dip3_params}) { // A failed BOOST_REQUIRE below throws, and a throwing constructor means no destructor runs. // Tear the globals down by hand so one broken invariant here doesn't leave a live tx index @@ -232,6 +234,73 @@ struct TestGovernanceStore : GovernanceStore { BOOST_FIXTURE_TEST_SUITE(governance_vote_processing_tests, GovernanceVoteSetup) +struct GovernanceReorgSetup : GovernanceVoteSetup { + GovernanceReorgSetup() : + GovernanceVoteSetup("-dip3params=109:109") + { + } +}; + +BOOST_FIXTURE_TEST_CASE(votes_follow_masternode_identity_across_reorg, GovernanceReorgSetup) +{ + auto& chainman = *m_node.chainman; + auto& govman = *m_node.govman; + CKey owner_key, other_owner_key, voting_key; + CBLSSecretKey operator_key, other_operator_key; + voting_key.MakeNewKey(true); + const auto registration = CreateProRegTx(chainman, utxos, 2, payout_script(), coinbaseKey, owner_key, operator_key); + const auto other_registration = CreateProRegTx(chainman, utxos, 3, payout_script(), coinbaseKey, other_owner_key, + other_operator_key); + MineBlock({registration}); + CBlockIndex* fork_block{WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())}; + const auto old_id = tip_mn_list().GetMN(registration.GetHash())->GetInternalId(); + + CProUpRegTx update; + update.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + update.proTxHash = registration.GetHash(); + update.pubKeyOperator.Set(operator_key.GetPublicKey(), /*specificLegacyScheme=*/bls::bls_legacy_scheme); + update.keyIDVoting = voting_key.GetPubKey().GetID(); + update.scriptPayout = payout_script(); + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; + const auto spent = FundTransaction(chainman, tx, utxos, payout_script(), COIN); + update.inputsHash = CalcTxInputsHash(CTransaction(tx)); + BOOST_REQUIRE(CHashSigner::SignHash(::SerializeHash(update), owner_key, update.vchSig)); + SetTxPayload(tx, update); + SignTransaction(tx, spent, coinbaseKey); + MineBlock({tx}); + govman.UpdatedBlockTip(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); + + const auto proposal = MakeProposal(uint256::ONE); + const auto parent_hash = proposal.GetHash(); + govman.AddGovernanceObjectForTesting(proposal); + auto unchanged_vote = MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES); + SignWithVotingKey(unchanged_vote, mn_voting_key); + CGovernanceVote changed_vote{COutPoint{registration.GetHash(), 0}, parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES}; + SignWithVotingKey(changed_vote, voting_key); + CGovernanceException exception; + uint256 hash_to_request; + BOOST_REQUIRE(govman.ProcessVote(unchanged_vote, exception, hash_to_request)); + BOOST_REQUIRE(govman.ProcessVote(changed_vote, exception, hash_to_request)); + BOOST_REQUIRE(govman.HaveVoteForHash(changed_vote.GetHash())); + + BlockValidationState state; + BOOST_REQUIRE(chainman.ActiveChainstate().InvalidateBlock(state, fork_block)); + MineBlock({other_registration}); + MineBlock({registration}); + BOOST_REQUIRE(old_id != tip_mn_list().GetMN(registration.GetHash())->GetInternalId()); + govman.UpdatedBlockTip(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); + + BOOST_CHECK(!govman.HaveVoteForHash(changed_vote.GetHash())); + BOOST_CHECK(govman.GetCurrentVotes(parent_hash, changed_vote.GetMasternodeOutpoint()).empty()); + BOOST_CHECK(govman.HaveVoteForHash(unchanged_vote.GetHash())); + BOOST_CHECK_EQUAL(govman.GetCurrentVotes(parent_hash, mn_collateral).size(), 1U); + const auto stored = govman.FindConstGovernanceObject(parent_hash); + BOOST_REQUIRE(stored != nullptr); + BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1); +} + // A vote that arrives before its parent object must be kept and replayed once the object shows up, // otherwise every vote that races ahead of its proposal is lost for good. BOOST_AUTO_TEST_CASE(orphan_vote_is_cached_and_applied_when_parent_arrives) From 1c128c3b674c72943401313e8865a50e4feb4009 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 02:26:05 +0000 Subject: [PATCH 07/12] fix: skip undecoded provider payloads in transaction JSON Unsupported provider transaction versions can deserialize without network information. Omit their optional decoded payload objects instead of letting JSON rendering throw through the REST block handler, and retain the raw payload bytes. Co-Authored-By: GPT 6.1 Sol (cherry picked from commit 9560439a567d12183d95d1b0a94ecf0f47053d2a) --- src/core_write.cpp | 4 +-- test/functional/interface_rest.py | 45 +++++++++++++++++++++++++++++++ test/functional/rpc_deprecated.py | 10 ++++--- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/core_write.cpp b/src/core_write.cpp index a3ec9b5643cb..e7627095d5dc 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -289,11 +289,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry } if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - if (const auto opt_proTx = GetTxPayload(tx)) { + if (const auto opt_proTx = GetTxPayload(tx); opt_proTx && opt_proTx->netInfo) { entry.pushKV("proRegTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - if (const auto opt_proTx = GetTxPayload(tx)) { + if (const auto opt_proTx = GetTxPayload(tx); opt_proTx && opt_proTx->netInfo) { entry.pushKV("proUpServTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 0e381fdd4dab..f5fe8d2c1ea9 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -12,10 +12,16 @@ import urllib.parse +from test_framework.blocktools import create_block, create_coinbase from test_framework.messages import ( BLOCK_HEADER_SIZE, COIN, + COutPoint, + CTransaction, + CTxIn, + CTxOut, ) +from test_framework.script import CScript, OP_TRUE from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -449,5 +455,44 @@ def run_test(self): self.sync_all() assert_equal(self.test_rest_request(f"/deploymentinfo/{header_only_hash}"), self.nodes[0].getdeploymentinfo(header_only_hash)) + self.log.info("Test rendering stored ProTx payloads with unsupported versions") + node = self.nodes[0] + tip = node.getbestblockhash() + protxes = [] + for tx_type in (1, 2): + for version in (0, 65535): + tx = CTransaction() + tx.nVersion = 3 + tx.nType = tx_type + tx.vin = [CTxIn(COutPoint(0x1234, len(protxes)))] + tx.vout = [CTxOut(1000, CScript([OP_TRUE]))] + tx.vExtraPayload = version.to_bytes(2, "little") + tx.calc_sha256() + protxes.append(tx) + block = create_block( + tmpl=node.getblocktemplate(), + coinbase=create_coinbase(node.getblockcount() + 1, dip4_activated=True, v20_activated=True), + txlist=protxes, + ) + block.solve() + assert_equal(node.submitblock(block.serialize().hex()), "bad-protx-version") + assert_equal(node.getblock(block.hash, 0), block.serialize().hex()) + + for deprecated_service in (False, True): + if deprecated_service: + self.restart_node(0, extra_args=self.extra_args[0] + ["-deprecatedrpc=service"]) + self.test_rest_request(f"/block/notxdetails/{block.hash}") + block_json = self.test_rest_request(f"/block/{block.hash}") + assert_equal(block_json, node.getblock(block.hash, 2)) + assert_equal(len(block_json["tx"]), len(block.vtx)) + for tx, tx_json in zip(protxes, block_json["tx"][1:]): + field = "proRegTx" if tx.nType == 1 else "proUpServTx" + for decoded in (tx_json, node.decoderawtransaction(tx.serialize().hex()), + node.getrawtransaction(tx.hash, True, block.hash)): + assert field not in decoded + assert_equal(decoded["extraPayload"], tx.vExtraPayload.hex()) + assert_equal(node.getbestblockhash(), tip) + assert_equal(self.test_rest_request("/chaininfo")["bestblockhash"], tip) + if __name__ == '__main__': RESTTest().main() diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 7b5226591c43..e702f543109b 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -63,11 +63,13 @@ def test_malformed_deprecated_service_rawtransactions(self): self.log.info("Test malformed provider payloads do not crash -deprecatedrpc=service") node = self.nodes[2] block_count = node.getblockcount() - for tx_hex in [ - "03000100000000000000020000", # ProRegTx with only nVersion=0 payload - "03000200000000000000020000", # ProUpServTx with only nVersion=0 payload + for tx_hex, payload_field in [ + ("03000100000000000000020000", "proRegTx"), # ProRegTx with only nVersion=0 payload + ("03000200000000000000020000", "proUpServTx"), # ProUpServTx with only nVersion=0 payload ]: - assert_raises_rpc_error(-1, "Internal bug detected: obj.netInfo", node.decoderawtransaction, tx_hex) + decoded = node.decoderawtransaction(tx_hex) + assert payload_field not in decoded + assert_equal(decoded["extraPayload"], "0000") assert_equal(node.getblockcount(), block_count) if __name__ == '__main__': From 0a26d57601b678cf7d50d9808149deea5a0058de Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 18:26:57 -0600 Subject: [PATCH 08/12] fix(llmq): retry pending EHF signals on new tips Allow members to re-sign pending EHF signals after incomplete signing sessions expire. Exercise recovery with the same quorum after isolation and timeout in both connection modes. (cherry picked from commit b619a4f9c7762b037be3b331cb7aad59a28b30ca) --- src/llmq/ehf_signals.cpp | 2 +- test/functional/feature_mnehf.py | 61 +++++++++++++++++++++++++------- test/functional/test_runner.py | 1 + 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/llmq/ehf_signals.cpp b/src/llmq/ehf_signals.cpp index fc8563fdc197..a4cf7c91b6ef 100644 --- a/src/llmq/ehf_signals.cpp +++ b/src/llmq/ehf_signals.cpp @@ -94,7 +94,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind const uint256 msgHash = mnhfPayload.PrepareTx().GetHash(); WITH_LOCK(cs, ids.insert(requestId)); - shareman.AsyncSignIfMember(llmqType, requestId, msgHash, quorum->qc->quorumHash, false, true); + shareman.AsyncSignIfMember(llmqType, requestId, msgHash, quorum->qc->quorumHash, true, true); } RecoveredSigResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) diff --git a/test/functional/feature_mnehf.py b/test/functional/feature_mnehf.py index 98fa8259f47f..20ef81b40522 100755 --- a/test/functional/feature_mnehf.py +++ b/test/functional/feature_mnehf.py @@ -28,13 +28,14 @@ class MnehfTest(DashTestFramework): def add_options(self, parser): self.add_wallet_options(parser) + parser.add_argument("--spork21", default=False, action="store_true", + help="Test with all quorum members connected") def set_test_params(self): # testdummy starts as NEVER_ACTIVE, run_test opens its signalling window later - extra_args = [["-vbparams=testdummy:-2:999999999999:0:4:4:4:5:1", "-persistmempool=0"]] * 2 - # One masternode is enough: the EHF signal only needs a recovered signature of any quorum - self.set_dash_test_params(2, 1, extra_args=extra_args) - self.set_dash_llmq_test_params(1, 1) + extra_args = [["-vbparams=testdummy:-2:999999999999:0:4:4:4:5:1", "-persistmempool=0"]] * 4 + self.set_dash_test_params(4, 3, extra_args=extra_args) + self.set_dash_llmq_test_params(3, 3) def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -93,6 +94,8 @@ def set_sporks(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", spork_enabled) self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", spork_disabled) self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", spork_disabled) + if self.options.spork21: + self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", spork_enabled) self.wait_for_sporks_same() def check_fork(self, expected): @@ -125,11 +128,45 @@ def test_no_signing_outside_window(self): self.check_no_ehf_signing("before its start time") self.log.info("Check that masternodes sign testdummy once its start time is reached") + request_id = hash256(ser_string(b"mnhf") + struct.pack(" Date: Wed, 11 Nov 2026 20:17:01 -0600 Subject: [PATCH 09/12] fix(llmq): keep verified contributions cached through DKG sessions Share verified vectors with their sessions and retain them until their owners release them. Update cached secret contributions on every verified write, while preserving the database format and cold-read fallback. Guard legacy decoding and scheme-sensitive cached hashes so shared vectors match cold database reads. (cherry picked from commit 96b68d07e88ec3831aee6f4ec93cb2cabad51c02) --- src/llmq/dkgsession.cpp | 3 +- src/llmq/dkgsessionmgr.cpp | 63 ++++++++++++++++++++++++++++++++++++-- src/llmq/dkgsessionmgr.h | 10 ++++-- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 848afca9f1ee..4bbabeba07e2 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -233,7 +233,8 @@ std::optional CDKGSession::ReceiveMessage(const CDKGContribution& qc) } cxxtimer::Timer t2(true); - dkgManager.WriteVerifiedVvecContribution(params.type, m_quorum_base_block_index, qc.proTxHash, qc.vvec); + dkgManager.WriteVerifiedVvecContribution(params.type, m_quorum_base_block_index, qc.proTxHash, qc.vvec, + qc.vvecLegacyScheme); bool complain = false; CBLSSecretKey skContribution; diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 5b68ffb0059d..e5be2684996d 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -122,19 +122,67 @@ bool CDKGSessionManager::GetPrematureCommitment(const uint256& hash, CDKGPrematu return false; } -void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec) +void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, + const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, + const BLSVerificationVectorPtr& vvec, bool vvecLegacyScheme) { CDataStream s(SER_DISK, CLIENT_VERSION); WriteCompactSize(s, vvec->size()); for (auto& pubkey : *vvec) { s << CBLSPublicKeyVersionWrapper(pubkey, false); } + const bool legacyScheme{bls::bls_legacy_scheme.load()}; + bool cacheVvec{true}; + if (vvecLegacyScheme) { + // Legacy decoding skips subgroup checks. Share only vectors identical to a basic-format DB read. + CDataStream basicStream{Span{s}, SER_DISK, CLIENT_VERSION}; + ReadCompactSize(basicStream); + try { + for (const auto& pubkey : *vvec) { + CBLSPublicKey decoded; + basicStream >> CBLSPublicKeyVersionWrapper(decoded, false); + if (decoded != pubkey) { + cacheVvec = false; + break; + } + } + } catch (const std::ios_base::failure&) { + cacheVvec = false; + } + } + // Shared keys must carry the same cached hashes that a cold read would compute. + for (auto& pubkey : *vvec) { + if (pubkey.GetHash() != ::SerializeHash(CBLSPublicKeyVersionWrapper(pubkey, legacyScheme))) { + cacheVvec = false; + break; + } + } + LOCK(contributionsCacheCs); db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), Span{s}); + ContributionsCacheKey cacheKey{llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}; + if (!cacheVvec) { + contributionsCache.erase(cacheKey); + return; + } + auto [it, inserted] = contributionsCache.try_emplace(cacheKey); + if (inserted) { + db->Read(std::make_tuple(DB_SKCONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), + it->second.skContribution); + } + it->second.entryTime = SteadyClock::now(); + it->second.vvec = vvec; + it->second.legacyScheme = legacyScheme; + it->second.fromSession = true; } void CDKGSessionManager::WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution) { + LOCK(contributionsCacheCs); db->Write(std::make_tuple(DB_SKCONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), skContribution); + const ContributionsCacheKey cacheKey{llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}; + if (auto it = contributionsCache.find(cacheKey); it != contributionsCache.end()) { + it->second.skContribution = skContribution; + } } void CDKGSessionManager::WriteEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSIESMultiRecipientObjects& contributions) @@ -160,6 +208,10 @@ bool CDKGSessionManager::GetVerifiedContributions(Consensus::LLMQType llmqType, const uint256& proTxHash = members[i]->proTxHash; ContributionsCacheKey cacheKey = {llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}; auto it = contributionsCache.find(cacheKey); + if (it != contributionsCache.end() && it->second.legacyScheme != bls::bls_legacy_scheme.load()) { + contributionsCache.erase(it); + it = contributionsCache.end(); + } if (it == contributionsCache.end()) { CDataStream s(SER_DISK, CLIENT_VERSION); if (!db->ReadDataStream(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), s)) { @@ -180,7 +232,10 @@ bool CDKGSessionManager::GetVerifiedContributions(Consensus::LLMQType llmqType, CBLSSecretKey skContribution; db->Read(std::make_tuple(DB_SKCONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), skContribution); - it = contributionsCache.emplace(cacheKey, ContributionsCacheEntry{SteadyClock::now(), vvecPtr, skContribution}).first; + it = contributionsCache + .emplace(cacheKey, ContributionsCacheEntry{SteadyClock::now(), vvecPtr, skContribution, + bls::bls_legacy_scheme.load()}) + .first; } memberIndexesRet.emplace_back(i); @@ -228,8 +283,10 @@ void CDKGSessionManager::CleanupCache() const { LOCK(contributionsCacheCs); const auto curTime = SteadyClock::now(); + // Sessions retain the shared vectors through commitment and finalization. for (auto it = contributionsCache.begin(); it != contributionsCache.end(); ) { - if (curTime - it->second.entryTime > MAX_CONTRIBUTION_CACHE_TIME) { + const auto& entry = it->second; + if (entry.fromSession ? entry.vvec.use_count() == 1 : curTime - entry.entryTime > MAX_CONTRIBUTION_CACHE_TIME) { it = contributionsCache.erase(it); } else { ++it; diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index cf1cdfa52d1e..e5297c677421 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -79,6 +79,8 @@ class CDKGSessionManager SteadyClock::time_point entryTime; BLSVerificationVectorPtr vvec; CBLSSecretKey skContribution; + bool legacyScheme{false}; + bool fromSession{false}; }; mutable std::map contributionsCache GUARDED_BY(contributionsCacheCs); @@ -140,8 +142,12 @@ class CDKGSessionManager bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const; // Contributions are written while in the DKG - void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec); - void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution); + void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, + const uint256& proTxHash, const BLSVerificationVectorPtr& vvec, + bool vvecLegacyScheme = false) EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); + void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, + const uint256& proTxHash, const CBLSSecretKey& skContribution) + EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector& validMembers, std::vector& memberIndexesRet, std::vector& vvecsRet, From d3e78cccfe98c0ed60d053f361c6a95ddb10c59b Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 20:17:03 -0600 Subject: [PATCH 10/12] test(llmq): cover verified contribution cache writes and lifetime (cherry picked from commit 04e93df362a0464a86f510f9d19f756a78031068) --- src/test/llmq_dkg_tests.cpp | 193 ++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) diff --git a/src/test/llmq_dkg_tests.cpp b/src/test/llmq_dkg_tests.cpp index 6595cf457a02..253cd12aaf54 100644 --- a/src/test/llmq_dkg_tests.cpp +++ b/src/test/llmq_dkg_tests.cpp @@ -3,14 +3,25 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include +#include +#include #include #include #include +#include +#include +#include #include #include +#include +#include #include #include +#include + +#include #include BOOST_AUTO_TEST_SUITE(llmq_dkg_tests) @@ -40,6 +51,14 @@ uint256 MakeTestHash(uint8_t value) hash.begin()[0] = value; return hash; } + +struct VerifiedContributionsSetup : RegTestingSetup { + VerifiedContributionsSetup() : + RegTestingSetup{{"-dip3params=1:1", "-testactivationheight=v20@1"}} + { + } +}; + } // namespace BOOST_AUTO_TEST_CASE(pending_messages_own_messages_share_quota_path) @@ -155,4 +174,178 @@ BOOST_AUTO_TEST_CASE(contribution_records_vvec_scheme) bls::bls_legacy_scheme.store(legacy_scheme_before); } +BOOST_FIXTURE_TEST_CASE(verified_contributions_cache_tracks_writes, VerifiedContributionsSetup) +{ + using namespace llmq; + + const auto& params = llmq::testutils::GetLLMQParams(Consensus::LLMQType::LLMQ_TEST); + std::array hashes; + std::array blocks; + for (const auto i : util::irange(blocks.size())) { + hashes[i] = GetRandHash(); + blocks[i].phashBlock = &hashes[i]; + blocks[i].nHeight = i; + blocks[i].pprev = i ? &blocks[i - 1] : nullptr; + blocks[i].BuildSkip(); + } + const CBlockIndex* base = &blocks.back(); + + const auto* work = base->GetAncestor(base->nHeight - llmq::WORK_DIFF_DEPTH); + CDeterministicMNList mnList{work->GetBlockHash(), work->nHeight, 3}; + std::vector proTxHashes; + std::vector expectedVvecs; + std::vector expectedSkContributions; + for (const auto i : util::irange(3)) { + const uint256 proTxHash = MakeTestHash(static_cast(i + 10)); + auto dmn = std::make_shared(i, MnType::Regular); + dmn->proTxHash = proTxHash; + dmn->collateralOutpoint = COutPoint(proTxHash, 0); + auto state = std::make_shared(); + uint160 owner; + WriteLE64(owner.begin(), i + 1); + state->keyIDOwner = CKeyID{owner}; + state->netInfo = NetInfoInterface::MakeNetInfo(ProTxVersion::LegacyBLS); + state->UpdateConfirmedHash(proTxHash, MakeTestHash(static_cast(i + 20))); + CBLSSecretKey operatorKey; + operatorKey.MakeNewKey(); + state->pubKeyOperator.Set(operatorKey.GetPublicKey(), false); + dmn->pdmnState = std::move(state); + mnList.AddMN(dmn); + proTxHashes.emplace_back(proTxHash); + + auto vvec = std::make_shared>(); + CBLSSecretKey coefficient; + coefficient.MakeNewKey(); + vvec->emplace_back(coefficient.GetPublicKey()); + expectedVvecs.emplace_back(std::move(vvec)); + expectedSkContributions.emplace_back(coefficient); + } + m_node.dmnman->SetListForBlockForTesting(mnList); + const auto members = utils::GetAllQuorumMembers(params.type, + {*m_node.dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, base}, + true); + BOOST_REQUIRE_EQUAL(members.size(), 3U); + auto checkResult = [&](const auto& vvecs, const auto& skContributions) { + BOOST_REQUIRE_EQUAL(vvecs.size(), 3U); + BOOST_REQUIRE_EQUAL(skContributions.size(), 3U); + for (const auto i : util::irange(members.size())) { + const auto original = std::find(proTxHashes.begin(), proTxHashes.end(), members[i]->proTxHash) - + proTxHashes.begin(); + BOOST_CHECK(*vvecs[i] == *expectedVvecs[original]); + BOOST_CHECK(skContributions[i] == expectedSkContributions[original]); + BOOST_CHECK((*vvecs[i])[0].ToBytes(false) == (*expectedVvecs[original])[0].ToBytes(false)); + BOOST_CHECK(skContributions[i].ToBytes(false) == expectedSkContributions[original].ToBytes(false)); + } + }; + + const auto dbPath = m_path_root / "dkg-cache-test"; + const util::DbWrapperParams dbParams{.path = dbPath, .memory = false, .wipe = true}; + std::vector allMembers(3, true); + { + CDKGSessionManager manager{*m_node.dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, *m_node.sporkman, + dbParams}; + std::vector memberIndexes; + std::vector vvecs; + std::vector skContributions; + manager.WriteVerifiedVvecContribution(params.type, base, proTxHashes[0], expectedVvecs[0], true); + manager.WriteVerifiedSkContribution(params.type, base, proTxHashes[0], expectedSkContributions[0]); + manager.WriteVerifiedSkContribution(params.type, base, proTxHashes[1], expectedSkContributions[1]); + manager.WriteVerifiedVvecContribution(params.type, base, proTxHashes[1], expectedVvecs[1]); + BOOST_CHECK(!manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + const std::vector knownMembers = [&] { + std::vector selected(3, true); + for (const auto i : util::irange(members.size())) { + if (members[i]->proTxHash == proTxHashes[2]) selected[i] = false; + } + return selected; + }(); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, knownMembers, memberIndexes, vvecs, skContributions)); + BOOST_CHECK_EQUAL(vvecs.size(), 2U); + manager.WriteVerifiedVvecContribution(params.type, base, proTxHashes[2], expectedVvecs[2]); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + for (const auto i : util::irange(members.size())) { + if (members[i]->proTxHash == proTxHashes[2]) BOOST_CHECK(!skContributions[i].IsValid()); + } + manager.WriteVerifiedSkContribution(params.type, base, proTxHashes[2], expectedSkContributions[2]); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + BOOST_REQUIRE_EQUAL(memberIndexes.size(), 3U); + BOOST_REQUIRE_EQUAL(vvecs.size(), 3U); + BOOST_REQUIRE_EQUAL(skContributions.size(), 3U); + checkResult(vvecs, skContributions); + manager.UpdatedBlockTip(base, true); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + for (const auto i : util::irange(members.size())) { + const auto original = std::find(proTxHashes.begin(), proTxHashes.end(), members[i]->proTxHash) - + proTxHashes.begin(); + BOOST_CHECK(vvecs[i] == expectedVvecs[original]); + BOOST_CHECK_EQUAL(memberIndexes[i], i); + } + + CBLSSecretKey replacementSk; + replacementSk.MakeNewKey(); + expectedVvecs[0] = std::make_shared>( + std::vector{replacementSk.GetPublicKey()}); + manager.WriteVerifiedVvecContribution(params.type, base, proTxHashes[0], expectedVvecs[0]); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); + expectedSkContributions[0] = replacementSk; + manager.WriteVerifiedSkContribution(params.type, base, proTxHashes[0], expectedSkContributions[0]); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); + + std::vector>> sessionVvecs; + for (auto& vvec : expectedVvecs) { + sessionVvecs.emplace_back(vvec); + vvec = std::make_shared>(*vvec); + } + vvecs.clear(); + manager.UpdatedBlockTip(base, true); + for (const auto& vvec : sessionVvecs) + BOOST_CHECK(vvec.expired()); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); + + for (const auto& vvec : vvecs) { + for (const auto& pubkey : *vvec) + pubkey.GetHash(); + } + const bool originalScheme{bls::bls_legacy_scheme.load()}; + struct RestoreScheme { + const bool saved; + ~RestoreScheme() { bls::bls_legacy_scheme.store(saved); } + } restoreScheme{originalScheme}; + bls::bls_legacy_scheme.store(!originalScheme); + manager.WriteVerifiedVvecContribution(params.type, base, proTxHashes[0], expectedVvecs[0]); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); + for (const auto& vvec : vvecs) { + BOOST_CHECK(std::find(expectedVvecs.begin(), expectedVvecs.end(), vvec) == expectedVvecs.end()); + for (auto& pubkey : *vvec) { + BOOST_CHECK(pubkey.GetHash() == ::SerializeHash(CBLSPublicKeyVersionWrapper(pubkey, !originalScheme))); + } + } + bls::bls_legacy_scheme.store(originalScheme); + BOOST_REQUIRE( + manager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); + } + + CDKGSessionManager coldManager{*m_node.dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, *m_node.sporkman, + util::DbWrapperParams{.path = dbPath, .memory = false, .wipe = false}}; + std::vector memberIndexes; + std::vector vvecs; + std::vector skContributions; + BOOST_REQUIRE( + coldManager.GetVerifiedContributions(params.type, base, allMembers, memberIndexes, vvecs, skContributions)); + checkResult(vvecs, skContributions); +} + BOOST_AUTO_TEST_SUITE_END() From 32a155391a3440f2809522cae9768c8bc2e5b5e6 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 21:30:44 -0600 Subject: [PATCH 11/12] fix(llmq): process our own DKG messages ahead of queued peer messages A node's own contribution, complaint, justification and premature commitment went to the back of the pending queue, so they were only verified and relayed after every peer message already waiting. Verifying a premature commitment of a 400-member quorum takes a noticeable fraction of a second, so on a busy or slow node a backlog could hold its own commitment back for much of a short commit phase. Put messages this node produced at the front of the queue. (cherry picked from commit 68de86bf0dd2323dd894df56595cb850d1fb2f30) Co-Authored-By: Claude Fable 5.2 --- src/llmq/dkgsessionhandler.cpp | 7 ++++++- src/llmq/dkgsessionhandler.h | 3 ++- src/test/llmq_dkg_tests.cpp | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 267a5bc372a4..04ad6bf8816d 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -52,7 +52,12 @@ void CDKGPendingMessages::PushPendingMessage(NodeId from, const uint256& sender_ count++; seenMessages.emplace(hash); - pendingMessages.emplace_back(from, std::move(pm)); + if (from == -1) { + // Our own message must not wait behind a backlog of peer messages before it is relayed + pendingMessages.emplace_front(from, std::move(pm)); + } else { + pendingMessages.emplace_back(from, std::move(pm)); + } } std::list CDKGPendingMessages::PopPendingMessages(size_t maxCount) diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index f52bbc81c416..eb3d560343fe 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -71,7 +71,8 @@ class CDKGPendingMessages * Enqueue a serialized DKG message under @p from with content hash @p hash. * @p sender_protx keys the per-proTx quota: the sender's MNAuth-verified * proTxHash for remote messages, or this node's own for messages it - * produced itself (@p from == -1). Drops the message silently on quota + * produced itself (@p from == -1), which go to the front of the queue. + * Drops the message silently on quota * overflow or duplicate hash; quota-dropped messages are not marked seen, * so another peer with budget can re-deliver them. Caller is responsible * for hashing the payload and (for real peers) routing the erase-request diff --git a/src/test/llmq_dkg_tests.cpp b/src/test/llmq_dkg_tests.cpp index 253cd12aaf54..127506f46cef 100644 --- a/src/test/llmq_dkg_tests.cpp +++ b/src/test/llmq_dkg_tests.cpp @@ -79,6 +79,23 @@ BOOST_AUTO_TEST_CASE(pending_messages_own_messages_share_quota_path) BOOST_CHECK_EQUAL(pending.PopPendingMessages(3).size(), 2U); } +BOOST_AUTO_TEST_CASE(pending_messages_own_message_is_processed_first) +{ + using namespace llmq; + + CDKGPendingMessages pending{/*_maxMessagesPerProTx=*/2}; + pending.PushPendingMessage(/*from=*/1, MakeTestHash(0xa1), MakeDKGMessage(), MakeTestHash(1)); + pending.PushPendingMessage(/*from=*/2, MakeTestHash(0xa2), MakeDKGMessage(), MakeTestHash(2)); + pending.PushPendingMessage(/*from=*/-1, MakeTestHash(0xee), MakeDKGMessage(), MakeTestHash(3)); + + const auto msgs = pending.PopPendingMessages(3); + BOOST_REQUIRE_EQUAL(msgs.size(), 3U); + auto it = msgs.begin(); + BOOST_CHECK_EQUAL((it++)->first, -1); + BOOST_CHECK_EQUAL((it++)->first, 1); + BOOST_CHECK_EQUAL((it++)->first, 2); +} + BOOST_AUTO_TEST_CASE(pending_messages_quota_survives_reconnect) { using namespace llmq; From 70e7f384a54a149bcb45c7e6cbae3229b6a08363 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 11 Nov 2026 22:44:29 -0600 Subject: [PATCH 12/12] chore: prepare v24.0.1 release Bump the build version to 1, add the v24.0.1 release notes, archive the v24.0.0 notes, add the flatpak release entry and bump the version string in the man pages (no option changed since v24.0.0). Co-Authored-By: Claude Fable 5.2 --- configure.ac | 2 +- .../flatpak/org.dash.dash-core.metainfo.xml | 1 + doc/man/dash-cli.1 | 6 +- doc/man/dash-qt.1 | 6 +- doc/man/dash-tx.1 | 6 +- doc/man/dash-util.1 | 6 +- doc/man/dash-wallet.1 | 6 +- doc/man/dashd.1 | 6 +- doc/release-notes.md | 1389 ++--------------- .../dash/release-notes-24.0.0.md | 1380 ++++++++++++++++ 10 files changed, 1514 insertions(+), 1294 deletions(-) create mode 100644 doc/release-notes/dash/release-notes-24.0.0.md diff --git a/configure.ac b/configure.ac index ea1aed9c6979..d5d2b0651c14 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ AC_PREREQ([2.69]) dnl Don't forget to push a corresponding tag when updating any of _CLIENT_VERSION_* numbers define(_CLIENT_VERSION_MAJOR, 24) define(_CLIENT_VERSION_MINOR, 0) -define(_CLIENT_VERSION_BUILD, 0) +define(_CLIENT_VERSION_BUILD, 1) define(_CLIENT_VERSION_IS_RELEASE, true) define(_COPYRIGHT_YEAR, 2026) define(_COPYRIGHT_HOLDERS,[The %s developers]) diff --git a/contrib/flatpak/org.dash.dash-core.metainfo.xml b/contrib/flatpak/org.dash.dash-core.metainfo.xml index 6cbc10d952fe..4ac5f311efbf 100644 --- a/contrib/flatpak/org.dash.dash-core.metainfo.xml +++ b/contrib/flatpak/org.dash.dash-core.metainfo.xml @@ -21,6 +21,7 @@ + diff --git a/doc/man/dash-cli.1 b/doc/man/dash-cli.1 index a04d71f9b5cf..d7149d0f0286 100644 --- a/doc/man/dash-cli.1 +++ b/doc/man/dash-cli.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASH-CLI "1" "November 2026" "dash-cli v24.0.0" "User Commands" +.TH DASH-CLI "1" "November 2026" "dash-cli v24.0.1" "User Commands" .SH NAME -dash-cli \- manual page for dash-cli v24.0.0 +dash-cli \- manual page for dash-cli v24.0.1 .SH SYNOPSIS .B dash-cli [\fI\,options\/\fR] \fI\, \/\fR[\fI\,params\/\fR] \fI\,Send command to Dash Core\/\fR @@ -15,7 +15,7 @@ dash-cli \- manual page for dash-cli v24.0.0 .B dash-cli [\fI\,options\/\fR] \fI\,help Get help for a command\/\fR .SH DESCRIPTION -Dash Core RPC client version v24.0.0 +Dash Core RPC client version v24.0.1 .SH OPTIONS .HP \-? diff --git a/doc/man/dash-qt.1 b/doc/man/dash-qt.1 index b5d118c05a14..bb5400952946 100644 --- a/doc/man/dash-qt.1 +++ b/doc/man/dash-qt.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASH-QT "1" "November 2026" "dash-qt v24.0.0" "User Commands" +.TH DASH-QT "1" "November 2026" "dash-qt v24.0.1" "User Commands" .SH NAME -dash-qt \- manual page for dash-qt v24.0.0 +dash-qt \- manual page for dash-qt v24.0.1 .SH SYNOPSIS .B dash-qt [\fI\,command-line options\/\fR] [\fI\,URI\/\fR] .SH DESCRIPTION -Dash Core version v24.0.0 +Dash Core version v24.0.1 .PP Optional URI is a Dash address in BIP21 URI format. .SH OPTIONS diff --git a/doc/man/dash-tx.1 b/doc/man/dash-tx.1 index 932abfe40d4c..128aac1217d0 100644 --- a/doc/man/dash-tx.1 +++ b/doc/man/dash-tx.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASH-TX "1" "November 2026" "dash-tx v24.0.0" "User Commands" +.TH DASH-TX "1" "November 2026" "dash-tx v24.0.1" "User Commands" .SH NAME -dash-tx \- manual page for dash-tx v24.0.0 +dash-tx \- manual page for dash-tx v24.0.1 .SH SYNOPSIS .B dash-tx [\fI\,options\/\fR] \fI\, \/\fR[\fI\,commands\/\fR] \fI\,Update hex-encoded dash transaction\/\fR @@ -9,7 +9,7 @@ dash-tx \- manual page for dash-tx v24.0.0 .B dash-tx [\fI\,options\/\fR] \fI\,-create \/\fR[\fI\,commands\/\fR] \fI\,Create hex-encoded dash transaction\/\fR .SH DESCRIPTION -Dash Core dash\-tx utility version v24.0.0 +Dash Core dash\-tx utility version v24.0.1 .SH OPTIONS .HP \-? diff --git a/doc/man/dash-util.1 b/doc/man/dash-util.1 index 40637cd6c6af..35e3f5290f68 100644 --- a/doc/man/dash-util.1 +++ b/doc/man/dash-util.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASH-UTIL "1" "November 2026" "dash-util v24.0.0" "User Commands" +.TH DASH-UTIL "1" "November 2026" "dash-util v24.0.1" "User Commands" .SH NAME -dash-util \- manual page for dash-util v24.0.0 +dash-util \- manual page for dash-util v24.0.1 .SH SYNOPSIS .B dash-util [\fI\,options\/\fR] [\fI\,commands\/\fR] \fI\,Do stuff\/\fR .SH DESCRIPTION -Dash Core dash\-util utility version v24.0.0 +Dash Core dash\-util utility version v24.0.1 .SH OPTIONS .HP \-? diff --git a/doc/man/dash-wallet.1 b/doc/man/dash-wallet.1 index 73b24e314b25..6034d6b58b37 100644 --- a/doc/man/dash-wallet.1 +++ b/doc/man/dash-wallet.1 @@ -1,9 +1,9 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASH-WALLET "1" "November 2026" "dash-wallet v24.0.0" "User Commands" +.TH DASH-WALLET "1" "November 2026" "dash-wallet v24.0.1" "User Commands" .SH NAME -dash-wallet \- manual page for dash-wallet v24.0.0 +dash-wallet \- manual page for dash-wallet v24.0.1 .SH DESCRIPTION -Dash Core dash\-wallet version v24.0.0 +Dash Core dash\-wallet version v24.0.1 .PP dash\-wallet is an offline tool for creating and interacting with Dash Core wallet files. By default dash\-wallet will act on wallets in the default mainnet wallet directory in the datadir. diff --git a/doc/man/dashd.1 b/doc/man/dashd.1 index 32fd5dcb4238..8095630417fd 100644 --- a/doc/man/dashd.1 +++ b/doc/man/dashd.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH DASHD "1" "November 2026" "dashd v24.0.0" "User Commands" +.TH DASHD "1" "November 2026" "dashd v24.0.1" "User Commands" .SH NAME -dashd \- manual page for dashd v24.0.0 +dashd \- manual page for dashd v24.0.1 .SH SYNOPSIS .B dashd [\fI\,options\/\fR] \fI\,Start Dash Core\/\fR .SH DESCRIPTION -Dash Core version v24.0.0 +Dash Core version v24.0.1 .SH OPTIONS .HP \-? diff --git a/doc/release-notes.md b/doc/release-notes.md index a068e9580a50..d152442844e4 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -1,10 +1,19 @@ -# Dash Core version v24.0.0 - -This is a new major version release, bringing new features, various bugfixes and other improvements. - -This release introduces the `v24` network upgrade (hard fork) and is **mandatory** for all nodes. -Masternodes must upgrade before the hard fork start date; all other nodes must upgrade before the -hard fork activates. See [Network upgrade](#network-upgrade) below. +# Dash Core version v24.0.1 + +This is a new patch version release. It fixes two problems that have delayed +the `v24` network upgrade on mainnet: masternodes signed the `v24` EHF signal +only once, so a first attempt that fell short was never retried, and +`LLMQ_400_85` quorums were failing to form because slower masternodes could +not finish the last step of the DKG in time. It also changes how masternodes +vote on superblock triggers so that a superblock cannot be left unpaid by a +change in the approved proposals, and fixes several other bugs. + +Like v24.0.0, this release carries the `v24` network upgrade (hard fork) and is +**mandatory** for all nodes. Masternodes should upgrade to v24.0.1 as soon as +possible. All other nodes must run v24.0.0 or later before the hard fork +activates, and upgrading to v24.0.1 is strongly recommended. The upgrade itself +and everything else introduced in v24 are described in the +[v24.0.0 release notes](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-24.0.0.md). Please report bugs using the issue tracker at GitHub: @@ -19,1274 +28,108 @@ shut down (which might take a few minutes for older versions), then run the installer (on Windows) or just copy over /Applications/Dash-Qt (on Mac) or dashd/dash-qt (on Linux). -On the first start after upgrading, a node that has `-addressindex`, -`-spentindex` or `-timestampindex` enabled migrates that index to its own -database (see [Asynchronous address, spent and timestamp indexes](#asynchronous-address-spent-and-timestamp-indexes)). -This can take 20-40 minutes or longer depending on hardware and index size. -Start v24.0.0 for the first time with the same `-addressindex`, `-spentindex` -and `-timestampindex` settings the node used before (pruned nodes: see -below): an index that is left -disabled on any start of v24.0.0 can no longer reuse its old data, and is -rebuilt from genesis when it is enabled again. The governance cache -(`governance.dat`) and the spork cache (`sporks.dat`) changed format and are -discarded and re-synced from the network on the first start. - -A node that runs with `-txindex=0` must now also set `-disablegovernance`; -otherwise it refuses to start (see [Updated settings](#updated-settings)). -Pruned nodes are not affected by this, since `-prune` disables governance -validation already. - -A pruned node (`-prune`) that also has `-addressindex` or `-spentindex` set -refuses to start with an error, because these indexes now need the undo data -that pruning deletes. v23 accepted that combination. Remove the index options -from such a node before upgrading. Keeping them means running the node -unpruned, which requires `-reindex` and downloading the whole chain again. -`-timestampindex` can no longer be enabled on a datadir that has already been -pruned (see -[Asynchronous address, spent and timestamp indexes](#asynchronous-address-spent-and-timestamp-indexes)). - -Masternodes refuse to start with some settings that v23 accepted, such as -`-blocksonly=1`, `-peerblockfilters=0` or a nonzero `-maxuploadtarget`. This -applies from the first start of v24.0.0, not from hard fork activation. Check -the masternode's configuration against -[Masternode configuration requirements](#masternode-configuration-requirements) -and remove such settings before upgrading. Masternodes must be upgraded and -running before the `v24` start date (November 9, 2026 on mainnet): the -activation signal needs 85% of the signing quorum's members to run v24, and a -masternode that refuses to start cannot contribute to it (see -[Network upgrade](#network-upgrade)). - -On mainnet, a masternode that moves to extended addresses must keep an IPv4 -address on the standard port as the first entry of each address list (see -[Extended addresses](#extended-addresses)). (#8018) - -A node that ran a v24.0.0 release candidate with `-addressindex`, -`-spentindex` or `-timestampindex` enabled should rebuild these indexes once -after upgrading (see -[Asynchronous address, spent and timestamp indexes](#asynchronous-address-spent-and-timestamp-indexes)), -in one of two ways: - -- restart once with `-reindex`; or -- start v24.0.0 once with `-addressindex=0 -spentindex=0 -timestampindex=0`, - stop it, delete the `indexes/addressindex`, `indexes/spentindex` and - `indexes/timestampindex` directories, then start it again with the indexes - enabled. They are rebuilt in the background. - -Do not only delete the index directories without the intermediate start: on a -datadir a release candidate has used, that can leave empty indexes that report -themselves as synced. - -Pruned nodes now keep at least 2304 blocks below the tip instead of 288 (see -[Pruning](#pruning)). Blocks that an earlier version already pruned are not -recovered; a node that fails to validate a block after upgrading needs -`-reindex`. +Upgrading from v24.0.0 needs no further steps. If you are upgrading from +v23.1.x or older, read "How to Upgrade" in the +[v24.0.0 release notes](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-24.0.0.md) +first: the first start migrates the address, spent and timestamp indexes, and +some settings changed. ## Downgrade warning -### Downgrade to a version < v24.0.0 - -There is no supported downgrade after `v24` activation: v23 and older cannot -follow the network once `v24` is active. They also cannot correctly read the -masternode list data that v24.0.0 stores after activation, so do not start an -older version on a datadir that v24.0.0 has used after activation. - -Before activation, downgrading to v23.1.x is possible with the following -caveats: - -- The address, spent and timestamp indexes are moved out of the block index - database into separate databases on the first start of v24.0.0. A node that - had any of these indexes enabled must be restarted with `-reindex` after - downgrading. Older versions enforce this: they refuse to start with an index - that v24.0.0 has migrated until `-reindex` is given. (#8031) -- The governance and spork caches written by v24.0.0 are not readable by older - versions; they are discarded and re-synced from the network. -- v24.0.0 does not upgrade the wallet file format. Wallet records added by - v24.0.0 that older versions do not know about (such as the new CoinJoin - pending-observation record) are ignored by them. -- Dash-Qt moves the dust attack protection setting into the settings it shares - with `dashd` (see `-dustprotectionthreshold` under - [New options](#new-options)). Older versions don't read it there, so after a - downgrade dust protection is disabled and has to be enabled again in the - options dialog. - -### Downgrade to a version < v23.0.0 - -Downgrading to a version older than v23.0.0 is not supported, and will -require a reindex. - -## Compatibility - -- Dash Core binaries now require macOS 14 (Sonoma) or later. Dash Core v23.1.x - binaries supported macOS 11 (Big Sur) or later. Because v24.0.0 is mandatory, - users on macOS 13 or older must upgrade macOS, or move their node or wallet - to a supported system, before the `v24` hard fork activates. Windows 10 or - later continues to be required on Windows. -- On Linux, kernel 3.17 or later is now required. The minimum glibc version - (2.31) is unchanged. -- `PROTOCOL_VERSION` has been bumped from `70240` to `70242` (see - [P2P and network changes](#p2p-and-network-changes)). - `MIN_PEER_PROTO_VERSION` is unchanged at `70221`. -- `MIN_MASTERNODE_PROTO_VERSION` has been bumped from `70240` to `70242`. During - DKG, quorum members connected over an older protocol version are considered - to have a bad connection and are voted bad, which leads to PoSe penalties. - All masternodes must therefore upgrade to v24.0.0. - -# Network upgrade - -This release introduces two new deployments that use the masternode-activated -[Enhanced Hard Fork](https://github.com/dashpay/dips/blob/master/dip-0023.md) -(EHF, DIP-0023) mechanism: - -| Deployment | Name | Version bit | -| --- | --- | --- | -| V24 | `v24` | 13 | -| EvoNode shares | `evo_shares` | 14 | - -On mainnet both deployments use identical parameters: - -- start time: `1794182400` (Monday, November 9, 2026 00:00:00 UTC); -- timeout: `1857340800` (Thursday, November 9, 2028 00:00:00 UTC); -- signaling window: 4032 blocks (about 7 days); -- threshold: 80% (3226 of 4032 blocks) in the first signaling window, decreasing - with each window to a floor of 60% (2420 of 4032 blocks) after 10 windows. - -Activation proceeds as follows: - -1. Once the median time past passes the start time, masternodes running v24 - sign an EHF signal with the `LLMQ_400_85` quorum. This requires 85% of the - quorum's members to be running v24. -2. After the signal is mined, miner signaling starts at the next 4032-block - window boundary. -3. If the threshold is reached within a window, the hard fork locks in, and it - activates one window later. - -Mainnet is expected to be near block 2,569,500 on November 9, 2026, so the -first window boundary after the signal is block 2,572,416 (around November 14). -If that window reaches the threshold, `v24` locks in at block 2,576,448 and -activates at block 2,580,480, around November 28, 2026. That is the earliest -possible activation; every window that misses the threshold delays it by about -one week. The dates are estimates based on 2.5-minute blocks. - -All masternodes must upgrade before the start date. All other nodes must -upgrade before activation, or they will follow an invalid chain. - -v24.0.0 does **not** sign the `evo_shares` signal. Until `evo_shares` activates, -EvoNodes cannot use shared collateral or more than one owner payout (see -[EvoNode restrictions](#evonode-restrictions)). A later release (v24.1) is -expected to sign it once Dash Platform supports EvoNode shares and payouts. -Both deployments carry identical parameters so that the later release can -activate `evo_shares` without another mainnet parameter change. - -On testnet, both deployments start at `1790208000` (Thursday, September 24, 2026 -00:00:00 UTC). - -# Notable changes - -## Decentralized masternode shares and multi-party payouts - -This release implements the Decentralized Masternode Shares DIP, activating -together with DIP-0026 multi-party payouts as part of the `v24` hard fork. -For EvoNodes, shared collateral and more than one owner payout additionally -require the `evo_shares` deployment (see [EvoNode restrictions](#evonode-restrictions)). -Both features use version 3 provider transactions (see -[Provider transaction version 3 format](#provider-transaction-version-3-format)). -Before activation there is no behavior change. (#7437, #7340) - -### Multi-party payouts - -- The `payoutAddress` argument of `protx register`, `protx register_evo`, - `protx register_fund`, `protx register_fund_evo`, `protx register_prepare`, - `protx register_prepare_evo` and `protx update_registrar` accepts, in addition - to a single address, an array of `{"address": ..., "reward": ...}` objects - that splits the owner reward between several payees, with `reward` given in - basis points. Multi-party payouts are only valid once the `v24` hard fork - activates. The `..._legacy` variants build legacy-scheme transactions, which - cannot carry a payout list, and reject an array. For an EvoNode still on - version 2, `protx update_registrar` keeps version 2 and accepts only a single - payout address; upgrade the EvoNode with `protx update_service_evo` first (see - [Extended addresses](#extended-addresses)). -- For version 3 masternode states and ProTx payloads the payout list is - reported as a `payouts` array in place of the single `payoutAddress` field, - which earlier versions keep. Shared masternodes report `shares` - instead. The fields appear in `protx info`, `protx list detailed`, - `protx listdiff`, `masternode status` (`dmnState`) and in decoded - `ProRegTx`/`ProUpRegTx` payloads of the transaction-decoding RPCs; - `protx diff` includes them only with `extended=true`. (#7340) - -### Consensus changes (active with `v24`) - -- A version 3 ProRegTx (see - [Provider transaction version 3 format](#provider-transaction-version-3-format)) - may carry a collateral share table: - 2 to 8 participants fund the masternode collateral atomically in one - registration, each recording an immutable amount, refund script and share - owner key, plus an updatable reward script. Every share owner consents by - signing a digest that binds the exact funding inputs, all outputs, the share - table, the penalty terms and the registrar configuration. Consent is tied to - the shares in the table, not to the funding inputs: signing an input into a - registration neither requires nor creates a share for its owner. The - early-period penalty must be below the smallest share and must be zero when - no early period is configured. -- The shared collateral is paid to the 7-byte template script - `04445348437551` (`0x04 "DSHC" OP_DROP OP_TRUE`). From activation, an output - paying this exact script is valid only as the collateral of a valid shared - registration, and spending such an output is valid only via a ProDisTx. - Template outputs mined before activation become permanently unspendable. -- Three new special transaction types: - - **ProDisTx (type 10)** dissolves a shared masternode, refunding every - participant's principal to its immutable refund script. Exactly one - signature (unilateral, penalized during the configured early period) or one - per share (unanimous, penalty-free). Validity is monotone: a ProDisTx that - is valid at some height is valid at every later height, which makes offline - "standby dissolutions" safe. The transaction fee is capped at 1000000 duffs - and a unilateral dissolution may not pay bonuses beyond the configured - early penalty, bounding what a stolen share owner key can drain from its - own share. - - **ProUpShareTx (type 11)** lets one share owner update their reward script. - - **ProUpSharedRegTx (type 12)** updates the operator key and/or voting key - with a signature from every share owner. A plain ProUpRegTx is invalid for - shared masternodes. -- The owner reward of a shared masternode is split across the share table - proportionally to the recorded contributions (sequential floor, remainder to - the last entry), paying each share's reward script (or its refund script when - none is set). Operator rewards are unchanged. -- Withdrawal (asset unlock) transactions may not pay the template script. -- Coinbase payments are matched by multiplicity: every expected masternode - payment and every superblock payment must be paid by its own coinbase output, - so two identical expected payments (same script and amount) require two - outputs. Before activation one output could satisfy both. (#7362, #7377) - Mining pools should build exactly one coinbase output per `masternode[]` and - per `superblock[]` entry of `getblocktemplate`, in template order, even when - entries share a script or amount. After activation, a block that merges or - deduplicates them is invalid (`bad-cb-payee` / `bad-cb-amount`). - -### EvoNode restrictions - -- EvoNodes cannot register with shared collateral or have more than one owner - payout until the `evo_shares` deployment activates. An EvoNode ProRegTx with a - share table is rejected with `bad-protx-shares-evo`, and an EvoNode ProRegTx - or ProUpRegTx with more than one owner payout is rejected with - `bad-protx-payouts-evo`. Regular masternodes are not affected. -- The consent digest that share owners sign for a shared EvoNode registration - also covers the EvoNode's `platformNodeID`. The digest of a regular shared - registration is unchanged. (#7807) - -### Shared masternode relay policy - -- The template output relays only as the declared collateral output of a shared - registration, and a template prevout is accepted only inside a ProDisTx; both - remain nonstandard everywhere else. - -### Shared masternode RPCs - -- `protx shared_register_prepare` builds an unsigned shared registration from a - caller-supplied funding transaction. The result echoes the decoded terms and - carries a `warning` when `earlyPenalty` is zero, since any participant can - then force an early exit at no cost beyond the transaction fee. Before signing - its funding inputs with `signrawtransactionwithwallet`, each participant should - confirm that the share table (`proRegTx.shares` in `decoderawtransaction`) - holds its share with its own amount and addresses. -- `protx shared_register_prepare_evo` creates an unsigned shared EvoNode - registration. It takes the arguments of `protx shared_register_prepare` - followed by `platformNodeID`, `platformP2PAddrs` and `platformHTTPSAddrs`, and - fails until `evo_shares` is active. (#7807) -- `protx shared_sign` signs a shared registration, dissolution or shared - registrar update with every share owner key the wallet holds. It returns the - decoded terms being consented to (for a dissolution, including the outputs) - alongside the signatures, and repeats the zero-penalty warning for a - registration. It refuses a registration or dissolution carrying an - unsatisfied lock time or a relative input lock unless `allowTimeLocks` is - set. It signs a dissolution only when it returns every share's principal to - its refund address, less the fee from the actor's share, as - `protx shared_dissolve_prepare` builds it. -- `protx shared_combine` combines collected signatures and optionally submits. - A dissolution combined here requires a signature from every share; unilateral - dissolutions come fully signed from `protx shared_dissolve`. A - ProUpSharedRegTx must be combined in the wallet that ran - `protx shared_update_registrar_prepare`, because combining re-signs its fee - inputs. It is refused unless those inputs are unlocked coins of a single - address of this wallet, any change returns to that address, and the fee is - within `-maxtxfee`. (#7881) -- `protx shared_dissolve` creates, signs and submits a unilateral ProDisTx (or, - with `submit=false`, returns hex suitable for offline standby storage). -- `protx shared_dissolve_prepare` builds an unsigned unanimous ProDisTx. -- `protx shared_update_share` updates one share's reward address. To restore - rewards to the immutable refund address, pass that address explicitly; empty - reward scripts are not valid in update transactions. -- `protx shared_update_registrar_prepare` builds an unsigned ProUpSharedRegTx. - -### Updated masternode list RPCs - -- `masternodelist` and `masternode list` report comma-separated share owner - addresses in `owneraddress` for shared masternodes. The `json` and `recent` - modes can be filtered by any share owner address. Shared registrations and - masternode state omit the singular `ownerAddress` field in `protx` and decoded - transaction output; each participant's owner address is in `shares`. (#7437) - -### Shared masternode GUI - -- The Masternodes tab gains a "Shared Masternode…" wizard for registering a - masternode funded by several people. The coordinator enters the participants, - masternode settings and exit terms, reserves coins for their own share and - copies one invitation; every participant pastes it, reserves their coins and - copies back their details. Two more rounds complete the registration: locked - terms out and approvals back, then a signing request out and signed - contributions back. Any message can be pasted from the landing page, replies - are accepted in any order, and every message shows a short code so - participants can confirm they hold the same one. Sessions can be saved and - reopened by either role. Reserved coins are released after broadcast. (#7701) -- Shared masternodes appear in the list with the type "Shared (you hold k of - n)", a details view listing every share, and context-menu actions to change - the wallet's reward address, rotate the operator or voting key (with every - share owner's approval), dissolve now, dissolve together, or create a - standby dissolution file holding both the penalty-free and the immediate - variant. Pasting a maintenance request or a standby dissolution into the - list opens the matching dialog. (#7701) -- The owned-masternode filter includes shared masternodes when the wallet holds - a participant's refund destination, including when rewards go to a different - wallet. (#7437) - -## Provider transaction version 3 format - -Once the `v24` hard fork activates, ProRegTx, ProUpServTx, ProUpRegTx and -ProUpRevTx may use version 3. Version 3 carries both -[extended addresses](#extended-addresses) and -[multi-party payouts and shared masternodes](#decentralized-masternode-shares-and-multi-party-payouts). -(#7302) - -- Dash Core v23 defined version 3 for ProRegTx and ProUpServTx only, behind the - same `v24` deployment, and its wire format has changed since. In version 3 - ProRegTx and ProUpRegTx payloads, `scriptPayout` is replaced by a - length-prefixed payout list. A version 3 ProRegTx also carries a share table - with one join signature per share, and the early-exit terms - (`nEarlyPeriodBlocks`, `nEarlyPenalty`); these are empty or zero for an - ordinary registration. -- ProUpRegTx and ProUpRevTx may now use version 3 as well. -- Development builds briefly used a separate version 4 for multi-party payouts. - It has been folded into version 3, and no released version of Dash Core uses - version 4. -- Software that parses or builds provider transactions (explorers, SPV - libraries, external wallets) must be updated. - -## Extended addresses - -Dash Core v23 introduced extended masternode addresses (`netInfo`, which lists -the core P2P, Platform P2P and Platform HTTPS endpoints explicitly) behind the -`v24` deployment. They become usable once `v24` activates, for regular -masternodes and EvoNodes alike, and do not depend on `evo_shares`. A masternode -moves to extended addresses with a version 3 provider transaction (see -[Provider transaction version 3 format](#provider-transaction-version-3-format)). -(#7741, #7748, #7751) - -- After activation, an EvoNode service update (`ProUpServTx`) must be version 3 - and list its Platform P2P and HTTPS addresses explicitly; the separate - Platform port fields are not carried over. A version 2 EvoNode `ProUpServTx` - is rejected with `bad-protx-version-disallowed`, and raising an EvoNode with - addresses to version 3 by any other transaction is rejected with - `bad-protx-netinfo-version`. `protx update_service_evo` builds the right - transaction, but third-party tools that build version 2 EvoNode service - updates must be updated. -- Platform HTTPS entries on port 443 are accepted in extended addresses. -- A version 3 (extended addresses) ProRegTx or ProUpServTx must list an IPv4 - address as its first `coreP2PAddrs` entry, and an EvoNode's first - `platformP2PAddrs` and `platformHTTPSAddrs` entries must use that same IPv4 - address. Further entries may use other addresses and network types (IPv6, - Tor, I2P, CJDNS, and domains for Platform HTTPS). Violations are rejected - with `bad-protx-netinfo-addr-type` or `bad-protx-netinfo-addr`. This applies - until masternodes are allowed to operate without IPv4, which is not scheduled - on mainnet. Testnet and devnets already allow it, so they are unaffected. - Regtest follows mainnet unless `-testactivationheight=ipv4_optional@` - is set. (#8018) -- On mainnet, the first entry of each address list must also use that list's - standard port: 9999 for `coreP2PAddrs`, 26656 for `platformP2PAddrs` and 443 - for `platformHTTPSAddrs`, as legacy ProTx versions already require there. - Additional entries may use any otherwise valid port. Violations are rejected - with `bad-protx-netinfo-port`. (#8018) - -## Version 2 Asset Unlock transactions - -Once the `v24` hard fork activates, version 2 Asset Unlock transactions become -valid. Version 1 Asset Unlock transactions remain valid after activation, and -Platform withdrawals keep using version 1 until a future Dash Platform release -starts issuing version 2. Until then, the version 2 features described below -(the stable txid, InstantSend locking and relay by instance hash) do not apply -to Platform withdrawals. - -The transaction hash of a version 2 Asset Unlock is computed with the quorum -signing fields (`requestedHeight`, `quorumHash`, `quorumSig`) zeroed, so every -instance Platform re-signs after an expiry is the same transaction with one -stable txid. Spends of an unmined withdrawal's outputs therefore stay valid -across re-signs. (#7639) - -A version 2 Asset Unlock is InstantSend-locked as soon as it can be mined in -the next block: it carries a valid signature from a recent quorum, is inside -its height window, no other instance of its withdrawal index is in the -mempool, and the withdrawals pending in the mempool fit the credit pool's -current limit. The lock pins the withdrawal index (as the outpoint -`{DIP-27 request id, 0}`) to the txid. Spends of a locked withdrawal are -ordinary InstantSend transactions and the wallet treats the withdrawal's -outputs as trusted, so Platform-to-Core transfers become rapidly respendable. - -Version 2 unlocks are relayed by instance hash (new inventory type -`MSG_ASSET_UNLOCK`, protocol version 70242), kept in the mempool while expired -awaiting a re-signed replacement, and committed to by the coinbase transaction -(CbTx version 4, `merkleRootAssetUnlocks`). An expired instance is also -accepted into the mempool (from peers and when reloading `mempool.dat`) if its -signature was valid at a height within its window, so its spends propagate; it -is neither mined nor InstantSend-locked until Platform re-signs it. Relaying an -Asset Unlock outside the receiving node's height window or quorum range is no -longer penalized. (#7639, #7744) - -- `getmempoolinfo` reports `pendingassetunlocks`, the sum of the withdrawal - amounts of the Asset Unlock transactions in the mempool. -- `getassetunlockstatuses` reports `instantlock` for mempooled withdrawals. -- `getrawtransaction` and `decoderawtransaction` report `instanceHash` for - version 2 Asset Unlock transactions. -- For pools, explorers and indexers: the txid of a version 2 Asset Unlock is - the double SHA-256 of the transaction with `requestedHeight`, `quorumHash` - and `quorumSig` zeroed, not of its serialized bytes. The full-bytes hash is - `instanceHash`. Block merkle roots and `getblocktemplate` - `transactions[].hash` use the txid, so software that hashes raw transaction - bytes (`getblocktemplate` `data`, ZMQ `rawtx`/`rawblock`, P2P) must use the - `hash` field or apply the same zeroing. - -## Relative asset unlock limit - -- The asset unlock limit that applies once the `v24` hard fork activates is now - a relative net rule on the credit pool balance instead of a flat 4000 DASH cap - on gross unlocks per window (576 blocks on mainnet and testnet). Asset unlocks - may not leave the credit pool below its balance one window earlier minus an - allowed drop of 20% of that balance, never less than 2000 DASH and with no - upper bound. -- Deposits and the per-block Platform reward inside the window raise the - balance and are withdrawable again, so a deposit followed by its withdrawal - does not consume the allowance of other users. -- The new `getcreditpoolinfo [height]` RPC reports the credit pool balance after - a block, the balance one window earlier, the window length, the amount - unlocked inside the window and the asset unlock limit that applies to the - next block. (#7712) - -## Version 2 Asset Locks and Platform addresses - -- Asset Lock special transactions gain a payload version 2, enabled by the - `v24` hard fork. Before activation only version 1 is valid; after activation - both version 1 and version 2 are. A version 2 payload signals that Platform - assigns each credit output to the matching Identity automatically, and it - extends the allowed `creditOutputs[].scriptPubKey` forms from P2PKH only to - P2PKH and P2SH. All other Asset Lock rules are unchanged. See DIP-27. -- `validateaddress` and `getaddressinfo` now accept DIP-18 Dash Platform - addresses (`dash1k…`/`dash1s…` on mainnet, `tdash1k…`/`tdash1s…` on the test - chains) and report them with a new `isplatform` field. `getaddressinfo` - describes such an address against the credit output script an Asset Lock would - carry for it, so `ismine` and `solvable` answer whether this wallet holds the - key or script behind the address. -- `sendtoaddress` and `sendmany` accept Platform addresses as recipients. Doing - so turns the transaction into an Asset Lock with a version 2 payload whose - credit outputs are the Platform recipients, plus a single empty `OP_RETURN` - output carrying their total. `subtractfeefromamount` is not supported for - Platform recipients, because the locked amount has to match the `OP_RETURN` - value exactly. Paying a Platform address is refused before the `v24` hard - fork activates. -- `getrawtransaction`, `decoderawtransaction` and the other - transaction-decoding RPCs report `assetLockTx.creditOutputs[].address` for - version 2 payloads, the DIP-18 Platform address the credit output pays. - Version 1 payloads carry no such address and the field is omitted. - -This release carries the consensus side of version 2 Asset Locks: after the -`v24` hard fork they are valid in a block. They remain non-standard for now, so -under default policy they are not accepted into the mempool or relayed, and -sending to a Platform address fails with -`Transaction is non-standard (assetlocktx-version-2) …`. A future release will -make them standard once Dash Platform can process them; that is a policy change -only and needs no further consensus change. On testnet, devnets and regtest the -node can be started with `-acceptnonstdtxn=1` to relay them already; mainnet -does not accept that option. (#7294, #7285, #7577) - -## Masternode operator keys and BLS schemes - -- Dash Core will no longer permit the registration of new legacy BLS scheme - masternodes after the `v24` hard fork activates. Existing basic scheme - masternodes will also be prohibited from downgrading to the legacy scheme - after activation. (#6729) -- Masternode operator key uniqueness is now enforced per key rather than per - BLS encoding. After the `v24` hard fork activates, registering or updating to - an operator public key already used by another masternode is rejected - regardless of which BLS scheme (legacy or basic) either side is encoded under, - closing a gap where the same key could be claimed twice under different - encodings. -- After the `v24` hard fork activates, a legacy (LegacyBLS) masternode migrates - to the basic BLS scheme *in place* when its state version is raised, keeping - the same operator key: the stored operator public key is re-encoded from the - legacy to the basic scheme, with no key rotation required. `protx - update_service` and `protx update_registrar` build the corresponding - basic-scheme migration transaction for a legacy masternode, and the - masternode is no longer PoSe-banned by the change. The `..._legacy` RPC - variants keep the masternode on the legacy scheme. Previously a legacy - masternode could not move to the basic scheme without effectively rotating - its operator key, and `update_registrar` clamped the transaction version - silently. (#7473) - -## CoinJoin promotion and demotion - -- CoinJoin can now promote and demote between adjacent standard denominations - within a mixing session after the `v24` hard fork activates. Promotion - combines 10 inputs of one denomination into 1 output of the next larger - denomination, while demotion splits 1 input into 10 outputs of the next - smaller denomination. Pre-v24 behavior remains unchanged. -- Conversions only spend fully-mixed coins, and their outputs start mixing over - from zero rounds. The 10:1 shape of a conversion publicly clusters one - participant's coins even inside a mixing transaction, so a converted coin is - not treated as mixed: it re-enters mixing at its new denomination and - disperses normally, while the histories of the fully-mixed coins that fed the - conversion remain protected. -- A mixing session only completes once each side of its denomination is - occupied by nobody or by at least two participants, since coins are only - concealed by other coins of the same size on the same side. A session that - attracts a lone promotion or demotion participant and no counterpart - therefore waits, and expires in the queue stage without charging anyone's - collateral, rather than publishing a transaction that would identify that - participant's coins. Because admission relies on the declared directions, a - participant whose entry deviates from what it declared has its collateral - consumed. (#7052) -- A wallet that still has coins to mix keeps standard mixing. It joins - promotion and demotion queues that other wallets opened, but only opens its - own when it has no standard mixing session to join or start, so a conversion - waiting for a counterpart never holds up mixing. (#7995) -- A promotion prefers fully-mixed coins from different mixing transactions. If - there are not enough of those, it also spends coins that came out of the same - mixing transaction, which links them as one owner's coins, as an ordinary send - of the same coins would. (#7996) - -See [P2P and network changes](#p2p-and-network-changes) for the related -protocol changes. - -## Compact quorum proofs +### Downgrade to v24.0.0 -New RPCs `getquorumproofchain` and `verifyquorumproofchain` produce and verify -compact mining-transaction proofs from an independently trusted snapshot, and -`getchainlockbyheight` returns historical coinbase-carried ChainLock -certificates. The RPCs read required blocks on demand, without an additional -index or startup scan, and keep what they read in bounded in-memory caches of up -to about 50 MiB. Nodes must retain the required historical blocks to generate -proofs. (#7107, #7727) +Downgrading to v24.0.0 is possible and needs no special steps. -## Deprecated RPC fields now hidden by default - -- The deprecated keys `service`, `platformP2PPort` and `platformHTTPPort` now - require the runtime flag `-deprecatedrpc=service`. (#7181) - - The following RPCs are affected: `decoderawtransaction`, `decodepsbt`, - `getblock`, `getrawtransaction`, `gettransaction`, `masternode status` - (only the `dmnState` key), `protx diff`, `protx listdiff`. - - This runtime gate also extends to the functionally identical key, `address` - in `masternode list` (and its alias, `masternodelist`). - - The same gate applies to `getcoinjoininfo`, `protx info`, - `protx list detailed` and `quorum info`. - - `dash-tx -json` output omits these keys as well and offers no way to restore - them, since `dash-tx` does not accept `-deprecatedrpc`. -- Information on soft fork status has been moved from `getblockchaininfo` to the - new `getdeploymentinfo` RPC, which allows querying soft fork status at any - block, rather than just at the chain tip. Inclusion of soft fork status in - `getblockchaininfo` can currently be restored using the configuration - `-deprecatedrpc=softforks`, but this will be removed in a future release. Note - that in either case, the `status` field now reflects the status of the current - block rather than the next block. A new `/rest/deploymentinfo` REST endpoint - provides the same information. (#6888) - -### Note for masternode operators running Dash Platform - -Dash Platform reads the `service` field from Dash Core, and Platform versions -before v5.0.0 also read the `getblockchaininfo` `softforks` field. EvoNode -operators should upgrade Dash Platform to v5.0.0 or later together with Dash -Core: - -- dashmate v5.0.0 and later start Dash Core with `-deprecatedrpc=service`, and - Platform v5.0.0 and later read deployment status from `getdeploymentinfo`, so - no extra configuration is needed. -- Setups not managed by dashmate v5.0.0 or later must start Dash Core with - `-deprecatedrpc=service`, and Platform versions before v5.0.0 also need - `-deprecatedrpc=softforks`. Without these options Platform cannot operate - correctly. - -## Descriptor wallets by default - -Descriptor wallets are now the default wallet type. Newly created wallets will -use descriptors unless `descriptors=false` is set during `createwallet`, or the -`Descriptor wallet` checkbox is unchecked in the GUI. - -Note that wallet RPC commands like `importmulti` and `dumpprivkey` cannot be -used with descriptor wallets, so if your client code relies on these commands -without specifying `descriptors=false` during wallet creation, you will need to -update your code. (#7204) - -`createwallet` now also refuses `descriptors=false` without `load_on_startup`; -since v21 this applied only to `descriptors=true`. To create a legacy wallet, -pass both, for example: - -``` -dash-cli -named createwallet wallet_name=mywallet descriptors=false load_on_startup=true -``` - -`dash-wallet create` also creates a descriptor wallet by default. Pass the new -`-legacy` option to create a legacy wallet instead. (#7204) - -An experimental RPC `migratewallet` has been added to migrate legacy -(non-descriptor) wallets to descriptor wallets. More information about the -migration process is available in the -[documentation](https://github.com/dashpay/dash/blob/master/doc/managing-wallets.md#migrating-legacy-wallets-to-descriptor-wallets). -(#7275) - -Failure handling in `migratewallet`, `restorewallet` and -`dash-wallet createfromdump` was fixed with backports from Bitcoin Core 28.x. A -failed migration restores the original wallet and leaves the rest of the -wallet directory as it was. After a migration, the migrated wallet and the -watch-only and solvables wallets it created are reloaded from disk, and if one -of them fails to load, the migration is rolled back. Migration backups are now -named `_