From 11c8140db34763c28968e9bbf18dc2e14ff9e195 Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 2 Nov 2026 21:39:10 -0600 Subject: [PATCH 01/40] Merge dashpay/dash#8019: fix(qt): stop reporting refused shared masternode broadcasts as sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e05da58e46b74ee8033f4d0d58a144cdeea71c02 fix(qt): show a shared masternode broadcast the mempool refused (pasta) Pull request description: ## Issue being fixed or feature implemented The shared masternode GUI reports a broadcast as successful when the mempool refused it. Seen on v24.0.0-rc.6 (regtest). A share owner pastes a "full principal" standby dissolution during the early period. The network refuses it (`bad-prodis-penalty-floor`), and the transaction reaches no mempool. The GUI still shows **"Standby dissolution sent"** with the txid. The registration broadcast has the same flaw. If the mempool refuses the registration (for example, a participant spent a reserved coin after signing), the coordinator is shown **"Shared masternode registered"**. The session also moves to its broadcast stage and the reserved coins are released, although nothing was registered. **Root cause:** - `SharedMnBroadcast()` passes the node's broadcast message to `MasternodeOperationRunner::errorText()`, which builds its text from `bilingual_str::translated` only. - `BroadcastTransaction()` reports a mempool rejection only in `.original` (`HandleATMPError`). - The old fallback, `if (message.empty())`, never fired, because `bilingual_str::empty()` checks `.original`. - So every refusal with a mempool reason produced an empty error string, and both callers treat an empty string as success. ## What was done? - `SharedMnBroadcast()` now passes the reject reason through as the `reject_reason` of the error, with `TransactionErrorString()` as its message. Any result other than `TransactionError::OK` therefore yields a non-empty error. Success is still decided by the node's own result; no mempool lookup was added. - **Standby paste:** shows "Broadcast failed" with the reason. - **Registration:** stays on its Signatures page with every signature kept and its coins still reserved, so it can be retried. - **`ALREADY_IN_CHAIN` and the fee-cap result:** read exactly as before, since they never carried a mempool reason. - Added a plain-language explanation for `bad-prodis-penalty-floor` to the known-error table. - The shared masternode walkthrough test now pastes the full-principal standby inside the early period. It checks that the user sees "Broadcast failed" with the explanation and the reject reason, and that the transaction is not in the mempool. The test's modal pilot also records message box titles, so the check reads the box the user actually sees. Not changed here: `Broadcast()` in `src/evo/providertx_service.cpp` can produce an empty error text for `ALREADY_IN_CHAIN` / `MAX_FEE_EXCEEDED`. That path does not mistake it for success (the error is the variant's error side), so it is left for a follow-up. ## How Has This Been Tested? - `test_dash-qt` on regtest: the full suite passes on this branch. With only the two product files reverted, the new walkthrough check fails with the old behaviour ("Standby dissolution sent / Transaction: …"). - Real GUI, before and after, on a disposable regtest network with throwaway wallets. Same chain state and same saved session file for both builds: - Before: `dash-qt` built from the exact base `c85c345bc21f9bf6268ad3ef3eae29ac055ed04b`. - After: this PR's head `e05da58e46b74ee8033f4d0d58a144cdeea71c02` for the standby case. The registration case used the earlier head `0a924cd5faa`, which differs only in the wording of the `bad-prodis-penalty-floor` explanation, a message that case never shows. - Screenshots below. The released v24.0.0-rc.6 binary shows the same false "sent" as the base. ## Screenshots Real `dash-qt` captures on a disposable regtest network with throwaway wallets. - **Before** is the exact base `c85c345bc21`; **after** is this PR's head `e05da58e46b` (the registration pair: `0a924cd5faa`, see the index). - Each pair ran from two copies of the same stopped datadir, with the same session file and the same transaction. - Build hashes, fixture details, the RPC state after each run and file hashes are in the [evidence index](https://github.com/PastaPastaPasta/dash-ui-artifacts/blob/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/README.md). **1. Pasting a full-principal standby dissolution during the early period** (txid `4a1d4735…cbdae245`; the network answers `bad-prodis-penalty-floor`) | Before — exact base | After — this PR | |---|---| | Reported as sent; the transaction is in no mempool | Reported as refused, with the reason | | | | The released v24.0.0-rc.6 shows the same false "Standby dissolution sent" ([screenshot](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/rc6/standby-2-result-full.png)). The confirmation shown before *Yes* is identical in both builds ([before](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/before/standby-1-confirm.png), [after](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/after/standby-1-confirm.png)). **2. Broadcasting a fully signed registration after a participant spent a reserved input** Same saved session, same chain. The start state is pixel-identical in both builds ([before](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/before/registration-1-signatures.png), [after](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/after/registration-1-signatures.png)), and both confirmed the same send box ([before](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/before/registration-2-confirm.png), [after](https://raw.githubusercontent.com/PastaPastaPasta/dash-ui-artifacts/768e6e5dcf50c588879826f5656c9e1f20d520bf/dash/pr-8019/comparison/after/registration-2-confirm.png)). | Before — exact base | After — this PR | |---|---| | "Shared masternode registered", though nothing reached a mempool; the coordinator's coin reservation was released (`listlockunspent` 0) | "Broadcast failed: Inputs missing or spent"; stays on Signatures with 3 of 3 signatures and *Broadcast Registration* available to retry; reservation kept (`listlockunspent` 1) | | | | ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 665c3b4436283b810aaa9b42108562a07c6cc21776f6b3fe665247ac5a073a2135a585e1dd531efb01f4550b7b5c65beda2602c039fece62ee96d5a415f9eb4d (cherry picked from commit bf9a6cb3dc473dbbb6f72d69ea0bca25a5fd080b) --- src/qt/masternodeoperationrunner.cpp | 1 + src/qt/sharedmnwidgets.cpp | 5 ++-- src/qt/test/sharedmnwalkthroughtests.cpp | 34 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/qt/masternodeoperationrunner.cpp b/src/qt/masternodeoperationrunner.cpp index 59af849d2792..cf3f78b166b2 100644 --- a/src/qt/masternodeoperationrunner.cpp +++ b/src/qt/masternodeoperationrunner.cpp @@ -226,6 +226,7 @@ const KnownError KNOWN_ERRORS[]{ {"bad-protx-version", QT_TRANSLATE_NOOP("MasternodeOperationRunner", "The network does not accept this transaction version yet. Wait for the network upgrade that introduces it to activate.")}, {"too-early", QT_TRANSLATE_NOOP("MasternodeOperationRunner", "This feature is not active on the network yet. Wait for the network upgrade that introduces it to activate.")}, {"bad-prodis-dup", QT_TRANSLATE_NOOP("MasternodeOperationRunner", "A dissolution for this masternode is already pending.")}, + {"bad-prodis-penalty-floor", QT_TRANSLATE_NOOP("MasternodeOperationRunner", "This dissolution pays a share less than the network requires: its full principal plus its part of any early-exit penalty. A dissolution that pays no penalty is accepted only once the early period has ended; if it has already ended, prepare a new dissolution.")}, }; } // namespace diff --git a/src/qt/sharedmnwidgets.cpp b/src/qt/sharedmnwidgets.cpp index c688dfcab0f9..f459f7d50e39 100644 --- a/src/qt/sharedmnwidgets.cpp +++ b/src/qt/sharedmnwidgets.cpp @@ -102,8 +102,9 @@ QString SharedMnBroadcast(interfaces::Node& node, const CTransactionRef& tx) const TransactionError error{ node.broadcastTransaction(tx, node::DEFAULT_MAX_RAW_TX_FEE_RATE.GetFee(GetVirtualTransactionSize(*tx)), message)}; if (error == TransactionError::OK) return {}; - if (message.empty()) message = TransactionErrorString(error); - return MasternodeOperationRunner::errorText({interfaces::ProviderTxErrorCode::BROADCAST_ERROR, message, {}, error}); + // BroadcastTransaction() reports a mempool rejection only in message.original + return MasternodeOperationRunner::errorText( + {interfaces::ProviderTxErrorCode::BROADCAST_ERROR, TransactionErrorString(error), message.original, error}); } SharedMnStatusBoard::SharedMnStatusBoard(QWidget* parent) : QWidget(parent) diff --git a/src/qt/test/sharedmnwalkthroughtests.cpp b/src/qt/test/sharedmnwalkthroughtests.cpp index 694ed035502f..5bcfd2bb5e3e 100644 --- a/src/qt/test/sharedmnwalkthroughtests.cpp +++ b/src/qt/test/sharedmnwalkthroughtests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,7 @@ #include