chore: merge master (v23.1.8) back into develop - #7534
Conversation
…ed masternodes 536f388 fix(qt): keep PoSe score visible when hiding banned masternodes (PastaClaw) Pull request description: # PR description ## Summary - Keep the Masternodes tab PoSe Score column visible when "Hide banned" is checked. - Continue filtering banned masternodes via the existing proxy filter. Closes dashpay#7286. ## Validation - `git diff --check` - Pre-PR review gate: ship - Not run: full build / GUI smoke test; no build directory was available in this worktree. Top commit has no ACKs. Tree-SHA512: ca69dd31322d78ced7d48621ea4dbef8ec65305411abcd245a9b8c7b3059870dfc174caec2ec6d9a05f7940263379e4d5991ad5d1eeb69f5b4894d5aa4751bde (cherry picked from commit 71453a8)
…otx listdiff results Backport of dashpay#7360 (upstream merge fb31170, cherry-picked with -m1). v23.1.x adaptation: upstream wraps the deprecated platformP2PPort/platformHTTPPort fields in IsServiceDeprecatedRPCEnabled(); that gate does not exist on this branch (removed by bbcd9d5 - the fields deliberately remain unenforced through gating) and v23.1.7 always returned them. Replaced the condition with 'if (true)' to keep the block structurally aligned with develop, per review feedback from UdjinM6 on the previous attempt. The genuine fix (reading the live Platform ports from netInfo instead of the always-zero scalar fields) is unchanged from upstream. (cherry picked from commit fb31170e24c1793d442ede1a3aeb00f89f52fdcb)
…tibility error for freetype 4a8a5a3 Merge bitcoin#32693: depends: fix cmake compatibility error for freetype (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It fixes freetype dependency build on Kubuntu 26.04 which provide cmake-4 by default. ## What was done? Backport bitcoin#32693 ## How Has This Been Tested? Build succeed ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 4a8a5a3 Tree-SHA512: b9b796ef3a6e39a1acd58bee1f990d7a5c1b9bcc1b75bc5cb8f9e00ad9cc57cd85c5fd62f585ba7cb57493e55236d8d95069a739acad271cfd01dc6da23065ab (cherry picked from commit b281041)
4e5cc4b fix: stabilize par help text in manpages (PastaClaw) Pull request description: # fix: stabilize par help text in manpages ## Issue being fixed or feature implemented Regenerating manpages currently records the local machine's CPU count in the `-par` and `-parbls` help text. That makes otherwise unrelated release manpage regeneration change those entries depending on which machine generated the pages. ## What was done? Updated the `-par` and `-parbls` help text to avoid printing the dynamic lower bound derived from `GetNumCores()`. Runtime behavior is unchanged: negative values still mean "leave that many cores free", `0` still auto-detects, and the existing max/default values remain documented. The checked-in `dashd` and `dash-qt` manpages were updated to match the new stable generated text. ## How Has This Been Tested? Tested on macOS arm64: ```bash git diff --check python3 -m py_compile contrib/devtools/gen-manpages.py rg -n -e '\\fB\\-[0-9]+\\fR to' doc/man/dashd.1 doc/man/dash-qt.1 ``` The grep command returned no matches, confirming the affected generated manpages no longer contain a CPU-count-specific lower bound. Also ran a pre-PR code review gate against the exact worktree diff: ```text Recommendation: ship ``` Note: this worktree did not have configured Dash Core build artifacts and `help2man` is not installed locally, so I did not rerun full manpage generation from rebuilt binaries here. The manpage edits mirror the changed `src/init.cpp` help text. ## Breaking Changes None. ## Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 4e5cc4b UdjinM6: utACK 4e5cc4b Tree-SHA512: a547caf7f7dce3c2d4dc8295cba75d23da3870b90fd274ee9a3ebbcb2320d8415ef682afe5ce8a9cd56f6b1c979a9c5b7f680032aa2774fd2d61282fbb4007e5 (cherry picked from commit dfe1e5d)
Backport of dashpay#7395 (upstream merge 547bf5e, cherry-picked with -m1). v23.1.x adaptation: in release_docker_hub.yml the branch pins actions/github-script at v6 (develop was at v7); applied the same bump to v8 that the PR makes. No other pins in that file were changed - develop-only bumps from unrelated PRs were not pulled in. (cherry picked from commit 547bf5eee2405ec53d213bad9b31c3d7b6284546)
7cb0c29 fix: fall back to serial map when fork is unavailable, drop multiprocess dep (UdjinM6) de1cc2c fix: run of circular-dependencies with python3.15 (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Multiprocess uses dill to pickle the nested handle_module2 closure and dill's Python 3.15 support is broken (co_lnotab was removed from code objects). ## What was done? Moved some functions and variables to global namespace. ## How Has This Been Tested? Run `test/lint/lint-circular-dependencies.py` with python3.15 Review hint: use `git show -w --color-moved=dimmed-zebra` ## Breaking Changes _Please describe any breaking changes your code introduces_ ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 7cb0c29 Tree-SHA512: cc1aef094f8ed0ffd17e4ef7fdb3bb20af048b8de5c2120b0283fcf7ebb3957f8a8736d97dcd543ef5ac1d3210ed39934fde898d5c47a0ad482d4bf0e263bb91 (cherry picked from commit 87ac140)
…ate for other peers Cherry-pick of commit b89e9a6 from merged Dash PR dashpay#7237 (Bitcoin Core v0.26 backports, batch 1). Only this single commit from dashpay#7237 is taken: it is the piece the compact block relay hardening in dashpay#7398 depends on; the rest of the v0.26 batch is deliberately excluded from v23.1.x. Verified line-for-line identical (content) to the genuine upstream Bitcoin commit: bitcoin/bitcoin@dbfc748d3df (merge of bitcoin#27608, author Suhas Daftuar). NOTE FOR REVIEWERS: there is no reviewed Dash PR for this exact slice against v23.1.x - please validate taking this single commit rather than all of dashpay#7237. (cherry picked from commit b89e9a6)
…26898, bitcoin#27626, bitcoin#27743, bitcoin#26969, bitcoin#29412, bitcoin#32646, bitcoin#33296) Backport of dashpay#7398 (upstream merge cfd18f1, cherry-picked with -m1). v23.1.x adaptations: (1) Misbehaving() on this branch only has the NodeId overload (develop's Peer& overload does not exist here), so the three new call sites use pfrom.GetId() instead of peer/*peer. (2) CheckBlock() keeps this branch's signature without the develop-only known_hash parameter, which predates this PR on develop and is not part of it. All other hunks are unchanged from upstream. (cherry picked from commit cfd18f13f5896c1b2a851764d99d38e2e21071c1)
…it tests Backport of dashpay#7387 (upstream merge da52293, cherry-picked with -m1). Included because dashpay#7414/dashpay#7442/dashpay#7450 modify src/test/governance_inv_tests.cpp, which this PR introduces; it also replaces the p2p_governance_invs.py functional test with equivalent unit-test coverage. v23.1.x adaptations: (1) governance.h on this branch has no 'namespace governance' block at the top - added one containing only the RELIABLE_PROPAGATION_TIME constant this PR introduces (develop's SuperblockManager forward declaration does not exist here and was not brought along). (2) ProcessVoteAndRelay keeps this branch's 'override' specifier. (3) Makefile.test.include entry added without develop-only governance_superblock_tests.cpp. All other hunks unchanged from upstream. (cherry picked from commit da52293e8d6ba51fca27a557f6efd83a439a4b70)
Backport of dashpay#7408 (upstream merge 72f53da). v23.1.x adaptations: (1) develop's CheckDKGMessageStructure lives in src/llmq/net_dkg.cpp, which does not exist on this branch - the same change is applied to the identical function in src/llmq/dkgsessionmgr.cpp (where the earlier intake hardening 31142da placed it). (2) This branch's QCONTRIB condition was 'blobs.size() == size' where develop pre-PR had '<= size'; the condition is updated to upstream's post-PR form 'blobs.size() >= min_size && <= size', making the resulting code match develop exactly. The functional test change applied cleanly and is unchanged from upstream. (cherry picked from commit 72f53dad67f00c1c62b119765f8c33a19d55c0d9)
Cherry-picked from upstream 11f524b. One adaptation: the include block additionally carries <ranges>. Upstream's diff adds only <algorithm> because develop already included <ranges>; v23.1.x did not, and the backported GetSessionCount()/GetAnnouncementSessionCount() use std::ranges::count_if.
… remote OOM Backport of dashpay#7402 (upstream merge 976e15a, cherry-picked with -m1). v23.1.x adaptation: the new RemoveNodesIf call site uses PeerManagerInternal::PeerIsBanned, which develop got from the dashpay#7115 net/consensus separation refactor (not on this branch). The three-line accessor (pure-virtual declaration in PeerManagerInternal, override declaration and trivial forwarder to the pre-existing IsBanned in PeerManagerImpl) is ported here verbatim from develop so the dashpay#7402 hunks apply unchanged. Everything else is unchanged from upstream. (cherry picked from commit 976e15ad0114d475db0dc51ae125da4be6bea01a)
…c requests 5ad1ef1 fix: throttle governance vote sync requests (PastaClaw) Pull request description: ## Issue being fixed or feature implemented - Per-object `MNGOVERNANCESYNC` vote requests were not recorded in `NetFulfilledRequestManager`, unlike full governance sync requests. - A peer could repeatedly ask for votes for the same governance object and make the node rescan/build the same vote inventory again. This is a low-cost CPU/lock-work concern, not a crash/OOM primitive. ## What was done? - Add a fulfilled-request key for per-object vote sync requests: `MNGOVERNANCESYNC-votes-<object hash>`. - Return early and score repeat requests from the same peer/address. - Add unit coverage asserting the per-object request is registered as fulfilled. ## How Has This Been Tested? - `git diff --check upstream/develop..HEAD` - `COMMIT_RANGE=upstream/develop..HEAD test/lint/lint-whitespace.py` - Not run locally: `src/test/test_dash --run_test=governance_inv_tests/per_object_vote_sync_is_fulfilled_request_limited` because this fresh worktree has no configured build/test binary. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 5ad1ef1 Tree-SHA512: 4d72826ddf3a5e1834281fb53a0c7af724ac97eba7a1a0cc73936641015215511c3cc9c409812cc8168be0523d53f2395d6e871f42ab0fcb36ed67404ab63a9e (cherry picked from commit c168228)
af6a0fd feat(serialize): add bounded-vector deserialization primitives (PastaClaw) Pull request description: ## Issue being fixed or feature implemented Network messages often have protocol-specific vector limits below the generic serialization limit. Callers need to enforce those limits before vector allocation and element decoding, without changing the wire format. ## What was done? - Factor the existing batched vector element decoder into a shared internal helper. - Add `UnserializeVectorWithMaxSize` for runtime bounds. - Add `LIMITED_VECTOR` / `LimitedVectorFormatter` for compile-time bounds in `READWRITE` declarations. - Keep serialization byte-for-byte compatible with ordinary vectors; only deserialization is bounded. - Compare CompactSize counts before narrowing or allocating, including counts at and above `MAX_SIZE`. ## Stacked adopters Each consumer remains a separate command-specific PR: - dashpay#7416 — quorum-data response vectors - dashpay#7418 — LLMQ signing message vectors - dashpay#7419 — CoinJoin message vectors - dashpay#7438 — SPORK signature vector Reviewing dashpay#7439 first leaves each child PR with only its protocol-specific policy, punishment, and regression tests. ## How Has This Been Tested? - `src/test/test_dash --run_test=serialize_tests` - Exact and over-limit boundaries, zero limits, custom element formatters, `MAX_SIZE` and `MAX_SIZE + 1` declarations, 64-bit CompactSize counts, wire compatibility, and rejection before element decode are covered. ## Breaking Changes None. Existing vector serialization and deserialization behavior is unchanged. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone Top commit has no ACKs. Tree-SHA512: 61e4183a5a2a173254f3105d30969704c2c53dac3ce421f0b3e8e989bc87ec1a94d2757944694db219802403a668d7460943d507ab7948db30b97145d57f26d6 (cherry picked from commit 9474fc5)
Backport of dashpay#7259 (upstream merge ee94b48, cherry-picked with -m1). v23.1.x adaptations: develop had already absorbed the CoinJoinWalletManager map into CJWalletManagerImpl before this PR; on this branch the map still lives in the separate CoinJoinWalletManager class (coinjoin/client.h). The PR's getClient->doForClient replacement is therefore realized here as a locked DoForClient template on CoinJoinWalletManager (added beside the existing ForEachCJClientMan/ForAnyCJClientMan helpers, with the same body upstream gives CJWalletManagerImpl::doForClient), with CJWalletManagerImpl::doForClient forwarding to it. CoinJoinWalletManager::Get and ::Flush are removed: Get is the raw-pointer accessor this PR eliminates (no callers remain), and Flush called the ResetPool/StopMixing methods this PR removes (superseded by doForClient + resetPool/stopMixing at the flushWallet call site, as upstream). client.h keeps this branch's extra includes. All other hunks unchanged from upstream. (cherry picked from commit ee94b484fc1a10cbb2ebae170322e97035d1c297)
Backport of dashpay#7438 (upstream merge 4537f37, cherry-picked with -m1). v23.1.x adaptations: (1) on this branch spork messages are deserialized in CSporkManager::ProcessSpork (src/spork.cpp), not in net_processing's SPORK handler as on develop - the same try/catch is applied there, reporting the failure as MisbehavingError{100, ...} through the existing MessageProcessingResult path (equivalent to develop's Misbehaving(*peer, 100, ...)). (2) The functional test's msg_spork_raw/SporkP2PInterface helpers come from dashpay#7343 (commit 60efd84), which is not on this branch; they are included verbatim, and the new test registers MESSAGEMAP[b"spork"] itself since dashpay#7343's test method (which did the registration on develop) is absent here. The spork.h LIMITED_VECTOR bound is unchanged from upstream. (cherry picked from commit 4537f37d51d5b62be3aca4bfc9e40404db76ffc9)
b5a7ba0 test: drop redundant quorum manager include (PastaClaw) 59fafb0 fix: bound ChainLock seen cache (PastaClaw) Pull request description: ## Issue being fixed or feature implemented Bounds ChainLock seen-CLSIG duplicate tracking so unvalidated or non-current CLSIG inventory cannot grow retained handler state without limit. This improves defensive handling for peer-supplied ChainLock messages while preserving the existing best ChainLock validation and update flow. ## What was done? - Replaced the unbounded `seenChainLocks` map with a bounded `unordered_limitedmap`. - Moved same-height/older ChainLock rejection before inserting into duplicate-tracking state. - Updated cleanup for the bounded cache. - Added focused unit coverage for stale CLSIG retention and cache bounding. ## How Has This Been Tested? Tested locally on macOS in a fresh worktree rebased on current `upstream/develop`: - `make -C src test/test_dash -j$(sysctl -n hw.ncpu)` - `src/test/test_dash --run_test=llmq_chainlock_tests,chainlock_handler_tests` - `git diff --check upstream/develop..HEAD` - `COMMIT_RANGE=upstream/develop..HEAD test/lint/lint-whitespace.py` - Scoped Codex review of `upstream/develop..HEAD`: no significant issues found; recommendation ship. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: utACK b5a7ba0 Tree-SHA512: 5626a69dfb1a1fbcbee56b62cc9fa442922a877f2d4ce33c51ade33f0718fe6cfae272e2136fb7e128e790edc8aacbbf694d0daae3a78406a98cb0543127d3ec (cherry picked from commit e002c28)
Backport of dashpay#7416 (upstream merge 5260466, cherry-picked with -m1). v23.1.x adaptations: develop's QDATA handling lives in src/llmq/net_quorum.cpp, which does not exist on this branch. The verification-vector bound is applied to the identical code in CQuorumManager::ProcessMessage (src/llmq/quorumsman.cpp) and the encrypted-contributions bound to QuorumParticipant::ProcessContribQDATA (src/active/quorums.cpp). Both report the violation as 'return MisbehavingError{100, ...}' through this branch's MessageProcessingResult flow instead of develop's m_peer_manager->PeerMisbehaving(...), matching the surrounding error handling. The functional test change applied cleanly and is unchanged from upstream. (cherry picked from commit 52604668ed7965936a0e9af9e2ab21b04ecfe087)
Backport of dashpay#7415 (upstream merge ecf2382, cherry-picked with -m1). v23.1.x adaptations: (1) the per-node/global pending caps, TryAddPendingIncomingSigShare, CountedBucketMap and the share-count batch bound apply unchanged; only develop-only context around them (the dashpay#7115 NetSigning dispatcher, GetMaxSessionsForPeer session caps) was not brought along. (2) Develop's MAX_UNVERIFIED_BATCHES cap on dispatched-but-unverified worker-pool batches has no counterpart here: on this branch shares leave the capped pending maps only inside the worker that immediately verifies them (ProcessPendingSigSharesLoop), so shares never accumulate in the pool's task queue and there is nothing to bound. (3) llmq_utils_tests.cpp gains the PR's three new test cases plus the MakeSigShare helper they need (taken verbatim from develop, where an out-of-scope earlier PR introduced it); develop-only session-limit tests are not included. (cherry picked from commit ecf2382)
…cation Backport of dashpay#7444 (upstream merge b9d956d, cherry-picked with -m1). v23.1.x adaptation: the two new Misbehaving call sites use the NodeId overload (pfrom.GetId()) since develop's Peer& overload does not exist on this branch. All other hunks unchanged from upstream. (cherry picked from commit b9d956de36a4a2d0a2ba0ecc46a2c5083f8058bf)
…the net-layer per-peer request tracker Backport of dashpay#7442 (upstream merge 13b9071, cherry-picked with -m1). v23.1.x adaptations, all dropping develop-only context rather than changing the fix: (1) develop registers NetGovernance unconditionally and implements AlreadyHave/ProcessGetData on it (both from out-of-scope commit 58ab8b3); this branch keeps its conditional registration and answers governance invs inline in net_processing (which already consults ConfirmInventoryRequest), so those context blocks were not brought along. (2) PeerManagerInternal here has no PeerPushInventory - the adjacent context line was not added. (3) ProcessVoteAndRelay keeps this branch's override specifier and ProcessObject keeps this branch's CNode& parameter. The substantive changes - ConsumeObjectRequest/PeerConsumeObjectRequest, the SendMessages stale-entry drain, removal of the governance-side m_requested_hash_time cache/AcceptMessage, the PeerConsumeObjectRequest authorization gates in NetGovernance::ProcessMessage, and both test files - are unchanged from upstream. (cherry picked from commit 13b9071880b0e995ba7a1233be2b95dd7e7c56cf)
…lization 8f0b813 fix(governance): bound vote signature deserialization (PastaClaw) Pull request description: Uses the shared bounded-vector deserialization primitive merged in dashpay#7439. ## Motivation Governance vote signatures were deserialized through the generic byte-vector path. A peer could declare a very large signature length, causing allocation before the stream reported truncation. The outer message-processing catch did not score or disconnect the peer, allowing repeated malformed messages. ## Changes - bound network governance-vote signature reads to 96 bytes before allocation - require one of the two structurally valid encodings: 65-byte compact ECDSA or 96-byte BLS - score malformed or truncated governance vote messages with 100 misbehavior points - preserve disk, hash, and outbound serialization behavior - add focused unit coverage ## Testing - `./src/test/test_dash --run_test=governance_vote_wire_tests` (4/4 tests) - `./src/test/test_dash --run_test=serialize_tests` (10/10 tests) - `test/lint/lint-python.py` - `git diff --check upstream/develop...HEAD` ACKs for top commit: knst: utACK 8f0b813 Tree-SHA512: 5cd804ffb410f47936615d230f5a5ebe4b2b4e1dd85455acb42df37fb65c0397f05c38704900a3ecf5d2729e157d03b298c4a7774b7764b3830cb57273a724cc (cherry picked from commit 24c0ead)
e058152 test: make governance vote fixtures wire-valid (PastaClaw) Pull request description: ## Issue being fixed or feature implemented The governance inventory tests merged in dashpay#7442 construct unsigned synthetic votes and round-trip them through the network parser. Develop now requires governance vote signatures to use a structurally valid 65- or 96-byte encoding, so the fixture is rejected as malformed before the authorization behavior under test is reached. This causes six unit-test failures across CI configurations. ## What was done? Give the synthetic `VOTE_SIGNAL_FUNDING` vote a compact-signature-sized placeholder. The 65-byte encoding matches the real ECDSA voting-key form for funding votes. These tests deliberately use a missing parent object, so vote processing takes the orphan path before cryptographic signature verification; production validation is unchanged. ## How Has This Been Tested? - Built `src/test/test_dash` in an isolated macOS arm64 worktree using the depends toolchain - `./src/test/test_dash --run_test=governance_inv_tests` - `./src/test/test_dash --run_test=governance_vote_wire_tests` - `git diff --check` - Mandatory independent pre-PR review gate: `ship` with no findings ## Breaking Changes None. Test-only change. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK e058152 Tree-SHA512: 5caca1e34ce563444ded42f669ff599f7658c6d1dbea1d541588211ac542ee45f3dd768e158899cb23150c5d45ec8dbcbf412a1472f73a3cc3e72a5ac37636e0 (cherry picked from commit f343d58)
Backport of dashpay#7418 (upstream merge 2406ebc, cherry-picked with -m1). v23.1.x adaptations: develop's QSIGSHARE/QSIGSESANN/QSIGSHARESINV/QGETSIGSHARES/QBSIGSHARES intake lives in NetSigning::ProcessMessage (moved there by the out-of-scope dashpay#7115 refactor); on this branch the same handlers live in CSigSharesManager::ProcessMessage and receive the identical transformation (UnserializeVectorWithMaxSize pre-decode bounds with log+ban+rethrow, and the UnserializeBatchedSigShares running-total decode for QBSIGSHARES). This branch keeps separate MAX_MSGS_CNT_QSIGSHARESINV/MAX_MSGS_CNT_QGETSIGSHARES constants where develop has a single merged MAX_MSGS_CNT_QSIGSHARES; both are 200, so the effective bounds are unchanged. The UnserializeBatchedSigShares helper and its doc comment live in signing_shares.{h,cpp} beside the handlers rather than in net_signing (develop's location), which here would create a signing_shares<->net_signing circular include. The CBatchedSigShares LIMITED_VECTOR change and the new unit tests are unchanged from upstream. (cherry picked from commit 2406ebcb9593e01ed4d55ac0dcf2a06423b8f2ad)
Backport of dashpay#7419 (upstream merge 5d83098, cherry-picked with -m1). v23.1.x adaptation: the coinjoin_inouts_tests.cpp additions are taken as upstream wrote them (including the TestableCoinJoinServer/MakePeer test helpers this PR introduces); develop's pre-existing entry_addscriptsig_matches_and_rejects test case, which sat adjacent in the conflict region but comes from an out-of-scope PR, was not brought along. All src/coinjoin hunks applied cleanly and are unchanged from upstream. (cherry picked from commit 5d8309898467301e3b1489b13e6a1b0a7062b5f2)
…dgets 67d25da test: require fonts for pixel-sized widget regression (PastaClaw) 469abf8 test: cover pixel-sized font conversion (PastaClaw) 443e1d3 qt: convert pixel font sizes in class and QTextEdit paths (PastaClaw) 322f257 qt: fix abort in updateFonts() with pixel-sized fonts (PastaClaw) b6dd65a qt: add effectivePointSize() helper for pixel-sized fonts (PastaClaw) Cherry-picked from upstream dashpay#7465; the backported code is unchanged. Two adaptations were needed for v23.1.x: - Conflicts in guiutil_font.cpp were limited to surrounding context that does not exist on this branch (develop-only helpers and a differing include block). - optiontests.cpp additionally includes qt/guiutil_font.h: fontsLoaded() and updateFonts() are declared there on v23.1.x, whereas develop declares them in qt/guiutil.h, which is all the upstream test includes.
4329a4f refactor: use named result type and score enum for ValidateDSTX (PastaClaw) 7d073b9 fix: penalize unknown-masternode dstx relays with a small score (PastaClaw) 9c68646 fix: avoid punishing unverifiable dstx relays (PastaClaw) c25cf3d fix: punish invalid dstx messages (PastaClaw) Pull request description: # fix: punish invalid dstx messages ## Issue being fixed or feature implemented Invalid CoinJoin broadcast transaction (`dstx`) messages can return early from validation without applying the same peer-accounting consequence used by other invalid P2P messages. This hardens DSTX handling by making invalid validation results contribute a low misbehavior score while preserving benign early returns for duplicate or otherwise non-error DSTX cases. ## What was done? - Apply a low misbehavior score when DSTX validation returns an invalid/error result. - Preserve existing behavior for successful validation and benign early-return cases. - Add P2P test framework serialization for `dstx` messages. - Add a focused functional test covering invalid DSTX peer accounting. ## How Has This Been Tested? Tested on macOS arm64. - `git diff --check` - `python3 -m py_compile test/functional/p2p_dstx.py test/functional/test_framework/messages.py` - Configured with depends `config.site`, `--without-gui`, `--disable-bench`, and `--disable-fuzz-binary` - `make -j8 src/dashd` - `test/functional/p2p_dstx.py --tmpdir=/tmp/dash_func_dstx` - `test/functional/p2p_dstx.py --tmpdir=/tmp/dash_func_dstx_2` ## Breaking Changes None. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 4329a4f Tree-SHA512: 79ac5b9bf9d359d68abf9b8a890fd09719c642635f199568efe4cb61c9979aa8cf89df48e6229c5c807b12b4ae127be701aef95ec04f4518fcabcdde305ce9b0 (cherry picked from commit a59ad9e)
a97b7b4 fix: align oversized notfound penalty (PastaClaw) a43a27f test: avoid hardcoded oversized notfound count (PastaClaw) f84962b fix: penalize oversized notfound messages (PastaClaw) Pull request description: # fix: penalize oversized notfound messages ## Issue being fixed or feature implemented Oversized `notfound` inventory vectors were ignored after deserialization. This change gives them a small misbehavior score, matching the defensive treatment used for other inventory-vector messages, and avoids holding `cs_main` while deserializing the vector. ## What was done? - Deserialize `notfound` inventory vectors before taking `cs_main`. - Return early with a small misbehavior score when a `notfound` vector exceeds the maximum outstanding object/block request count. - Add functional coverage for the oversized `notfound` path. ## How Has This Been Tested? Environment: macOS 15.6 arm64, local Dash Core autotools build from this branch. - `git diff --check` - Local build configured with: ```bash ./configure --without-gui --disable-bench --disable-fuzz-binary \ --disable-tests --disable-wallet ``` - `make -j8 src/dashd src/dash-cli` - Initial parallel build hit a generated dependency-file race while building `dash-cli`; `dashd` linked successfully. - `make -j1 src/dash-cli` - `test/functional/p2p_tx_download.py --cachedir=/tmp/dash_func_cache_notfound` ## Breaking Changes None. ## Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone *(for repository code-owners and collaborators only)* ACKs for top commit: UdjinM6: utACK a97b7b4 Tree-SHA512: 35ff6b0b9e12e377e5600f4b0b2caee9582d0b4022dad04727fab54599807a5659c1837e43d30205386be34ce4dda33d4d9f687101ce67e8a694fb1a614d3914 (cherry picked from commit 6a051f5)
Bumps _CLIENT_VERSION_BUILD to 8 (_CLIENT_VERSION_IS_RELEASE stays true, no RC), adds the flatpak metainfo release entry, writes the v23.1.8 release notes and archives the v23.1.7 notes under doc/release-notes/dash/. Man pages regenerated with contrib/devtools/gen-manpages.py from a release build (no --enable-debug, so DEBUG_LOCKCONTENTION is undefined). Verified the conditionally-compiled 'lock' debug category does not appear in any generated page; it is the only category behind an #ifdef in src/logging.h.
24920a0 chore: prepare v23.1.8 release (pasta) 2194248 Merge dashpay#7348: fix: penalize oversized notfound messages (pasta) f5c72c3 Merge dashpay#7347: fix: punish invalid dstx messages (pasta) 550caf7 Merge dashpay#7465: fix(qt): handle pixel-sized fonts when scaling widgets (pasta) e203710 Merge dashpay#7419: fix(net): bound CoinJoin message vector intake (pasta) 5f5b960 Merge dashpay#7418: fix(net): bound signing message vector intake (Pasta) 7cc2cca Merge dashpay#7450: test: make governance vote fixtures wire-valid (Pasta) f011c80 Merge dashpay#7440: fix(net): bound governance vote signature deserialization (Pasta) 4b4d96a Merge dashpay#7442: fix(net): authorize governance inv responses via the net-layer per-peer request tracker (Pasta) f855b13 Merge dashpay#7444: fix(net): bound bloom message vectors before allocation (Pasta) 5b5c6fb Merge dashpay#7415: fix: bound pending sig share queue (Pasta) 9bbe808 Merge dashpay#7416: fix(net): bound quorum data response vectors (Pasta) da42f50 Merge dashpay#7424: fix: bound ChainLock seen cache (Pasta) 5b310df Merge dashpay#7438: fix: bound SPORK signature deserialization (Pasta) e118d0c Merge dashpay#7259: fix: dangling point to cj client (Pasta) 9921621 Merge dashpay#7439: refactor: add bounded vector deserialization (Pasta) 89bdf7c Merge dashpay#7414: fix(net): throttle per-object governance vote sync requests (Pasta) 44c396d Merge dashpay#7402: fix: bound pending recovered sig queue to prevent remote OOM (Pasta) 05cfe27 Merge dashpay#7351: fix: limit signing share sessions per peer (pasta) 3ef3a5b Merge dashpay#7408: fix: bound DKG contribution blob intake (pasta) 0ea6532 Merge dashpay#7387: test: migrate governance inv cache coverage to unit tests (Pasta) 8ffdf7f Merge dashpay#7398: backport: compact block relay hardening (bitcoin#26898, bitcoin#27626, bitcoin#27743, bitcoin#26969, bitcoin#29412, bitcoin#32646, bitcoin#33296) (Pasta) 2915142 backport: bitcoin#27608 - p2p: Avoid prematurely clearing download state for other peers (PastaClaw) 90b5473 Merge dashpay#7396: fix: run of circular-dependencies with python3.15 (Pasta) b003cdc Merge dashpay#7395: ci: update GitHub Actions pins for Node 24 (pasta) 97c3dd1 Merge dashpay#7394: fix: stabilize par help text in manpages (pasta) 8f8616b Merge dashpay#7372: backport: bitcoin#32693: depends: fix cmake compatibility error for freetype (pasta) 48f72be Merge dashpay#7360: fix: empty platformP2PPort deprecated field in protx listdiff results (pasta) a8cccff Merge dashpay#7298: fix(qt): keep PoSe score visible when hiding banned masternodes (pasta) Pull request description: Release PR for Dash Core v23.1.8, a patch release on top of v23.1.7. Fast-forwards from `v23.1.x` (currently at `chore: prepare v23.1.7 release`), 29 commits, no merge commits, no conflicts. ## Contents Backports of PRs already reviewed and merged on `develop`: `dashpay#7259` `dashpay#7347` `dashpay#7348` `dashpay#7351` `dashpay#7298` `dashpay#7360` `dashpay#7372` `dashpay#7387` `dashpay#7394` `dashpay#7395` `dashpay#7396` `dashpay#7398` `dashpay#7402` `dashpay#7408` `dashpay#7414` `dashpay#7415` `dashpay#7416` `dashpay#7418` `dashpay#7419` `dashpay#7424` `dashpay#7438` `dashpay#7439` `dashpay#7440` `dashpay#7442` `dashpay#7444` `dashpay#7450` `dashpay#7465` Plus `backport: bitcoin#27608`, a single commit taken from Dash dashpay#7237 because dashpay#7398's compact-block hardening depends on it. The rest of that v0.26 batch is intentionally not included on v23.1.x. The commit is byte-identical to its reviewed counterpart inside dashpay#7237. And release preparation: version bump, regenerated man pages, release notes, archived 23.1.7 notes. ## Note for reviewers: this branch was rebuilt An earlier revision of this PR was discarded and the branch rebuilt from scratch. Review comments on the previous revision point at commits that no longer exist, though the feedback itself was carried over (see below). The reason: several commits titled `Merge #NNNN` in the earlier revision contained substantial code that exists nowhere upstream — apparently written from a description of each PR rather than ported from its diff. For example, `feature_llmq_simplepose.py` is byte-identical between v23.1.7 and `develop`, yet the earlier `Merge dashpay#7408` rewrote 66 lines of it; `test/functional/p2p_governance_invs.py` does not exist on `develop` at all, yet had grown from 62 to 148 lines. That mislabeling matters because a commit titled `Merge #NNNN` invites less scrutiny, not more. It also had consequences: the earlier revision was **missing dashpay#7440 entirely**, and contained eleven consecutive commits that did not compile (code written against newer upstream APIs this branch does not have — `Misbehaving(Peer&)`, and `PeerIsBanned` used five commits before it was declared). Every commit on this branch has now been diffed against its upstream merge commit. Where a backport differs, it is because v23.1.x predates an upstream refactor and the change had to be applied to the pre-refactor file — for example dashpay#7418 and dashpay#7438 patch `signing_shares.cpp` / `spork.cpp` where upstream patches `net_signing.cpp` / `net_processing.cpp`. ## Dropped from this branch - **dashpay#7350** (`net: don't lock cs_main while reading blocks`) — dropped on review feedback. It is a 110-line lock-structure refactor of `ProcessGetBlockData` with no measured benefit, and it would add avoidable churn to the eventual master→develop merge-back. Nothing on this branch depends on it: dashpay#7398's compact-block work precedes it, and the remaining 14 commits replay with zero conflicts once it is removed. Thanks @knst. ## Added after the initial review pass - **dashpay#7351** (`fix: limit signing share sessions per peer`) — cherry-picked as a single commit and placed before dashpay#7402, matching upstream's merge order. The include block additionally carries `<ranges>`: upstream's diff adds only `<algorithm>` because develop already had it, whereas v23.1.x did not and the backported `GetSessionCount()` / `GetAnnouncementSessionCount()` use `std::ranges::count_if`. - **dashpay#7465** (`fix(qt): handle pixel-sized fonts when scaling widgets`) — cherry-picked from the five upstream commits. `optiontests.cpp` additionally includes `qt/guiutil_font.h`, because `fontsLoaded()` and `updateFonts()` are declared there on v23.1.x while develop declares them in `qt/guiutil.h`, which is all the upstream test includes. Two further backports were added later and applied without any adaptation -- their diffs are byte-for-byte identical to upstream: - **dashpay#7347** (`fix: punish invalid dstx messages`) - **dashpay#7348** (`fix: penalize oversized notfound messages`) ## Adaptations worth flagging - **dashpay#7360** — upstream gates `platformP2PPort` / `platformHTTPPort` in `protx listdiff` behind `IsServiceDeprecatedRPCEnabled()`. On 23.x those deprecated fields are deliberately not enforced through gating (see `bbcd9d543e6`), so shipping the gate as-is would silently drop two fields that v23.1.7 always returned. Changed to `if (true)` with a comment, per review feedback, keeping the block aligned with `develop`. The substantive fix from dashpay#7360 — reading the live port from `netInfo` instead of the always-zero scalar — is retained. - **dashpay#7415** — the pending-map caps (`MAX_PENDING_SIG_SHARES_PER_NODE`, `MAX_PENDING_SIG_SHARES_TOTAL`) are backported. The additional bound upstream places on batches awaiting verification is not, because it guards a condition that does not exist here: upstream's dispatcher pushes one task per batch inside an inner loop, whereas v23.1.x pushes a single looping worker per 10 ms tick. There is no unbounded task queue to bound. - **Man pages** — regenerated without the `lock` debug category, which only exists under `DEBUG_LOCKCONTENTION` and so is absent from release binaries. Thanks @UdjinM6 for catching this. ## Known CI failure macOS jobs are expected to fail. `actions/upload-artifact@v6` rejects filenames containing `:`, and the Xcode SDK ships Perl man pages with `::` in the name. A release-branch-only workaround existed on the earlier revision but was dropped as it corresponds to no upstream PR. This is accepted for this release. ## Testing - Every commit through dashpay#7465 compiles individually (verified for 27 of the 29; the three additions below were verified at the tip) — verified individually, not just at the tip. - Full build clean; no new warnings. - Unit tests pass. - Functional tests pass: `feature_llmq_signing` (both variants), `feature_llmq_chainlocks`, `feature_llmq_dkgerrors`, `feature_llmq_is_cl_conflicts`, `p2p_instantsend`, `feature_dip3_deterministicmns` (both wallet types), `rpc_coinjoin`. - Qt unit tests pass (32 cases, run under the `cocoa` platform plugin so the pixel-sized font regression from dashpay#7465 actually executes rather than self-skipping). - Lint: one pre-existing `lint-cppcheck-dash` failure, identical on v23.1.7, in files this branch does not touch. Top commit has no ACKs. Tree-SHA512: 0fa469c9a33820aa85fbb8b90c5877409d09490f746f1300b05ceda470a600765f900bec42e5aad5d90a2d46b44e28c20e44dc4a8fe719553a072298069eaec4
…e mempool removeProTxKeyChangedConflicts() snapshots every mempool transaction that a masternode's operator-key change invalidates into a std::set<uint256>, then calls removeRecursive() on each in turn. When one conflicting transaction is an in-mempool descendant of another, removing the ancestor already erased the descendant, so the later mapTx.find() for it returned end() and dereferencing that iterator aborted the node. Skip entries that are no longer in the mempool, matching the guard every sibling removeProTx*Conflicts helper already applies. Reachable from CTxMemPool::removeForBlock() whenever a block carries a provider registrar update or revocation for a masternode with chained service updates pending in the mempool. Adds a regression test that builds a parent/child ProUpServ chain for one masternode, grinds the service port so the ancestor sorts first, and drives removeForBlock() with a revocation. It aborts on the unpatched code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…m lookup A CSigShare carries its LLMQ type as an unvalidated uint8_t. CSigSharesManager::ProcessMessage() handed it straight to ProcessMessageSigShare(), which calls CQuorumManager::GetQuorum(). The per-type quorum caches are std::map keyed by LLMQ type and are only seeded by InitQuorumsCache() with the types in the chain's consensus params, so indexing them with std::map::operator[] for an unregistered type inserted a default-constructed Uint256LruHashMap. Its default MaxSize is 0 and unordered_lru_cache's constructor asserts maxSize != 0, aborting the process. Mainnet registers five of ~256 possible values, so almost any value crashed a masternode. Gate the qsigshare branch on Params().GetLLMQ(...).has_value() and ban on failure, matching the check the sibling handlers (QSIGREC, QSIGSESANN, QFCOMMITMENT, QGETDATA) already apply. As defence in depth, look the caches up with find() instead of operator[] in CQuorumBlockProcessor::HasMinedCommitment(), CQuorumManager::GetQuorum() and the QGETDATA handler, so no future caller can insert a zero-capacity cache. Adds regression tests covering both crash sites; they abort on the unpatched code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A quorum commitment naming the genesis block gave GetAllQuorumMembers() a base index whose pprev is null, and that null was passed to ChainstateManager::IsQuorumTypeEnabled()'s gsl::not_null parameter. Its Expects() calls std::terminate(), which is [[noreturn]] noexcept, so none of the catch(const std::exception&) handlers around special-tx processing could contain it: every node connecting such a block died, and died again on restart. The same sink was reachable pre-authentication through an unsolicited qfcommit on a node below dkgInterval height. Take a plain const CBlockIndex* in IsQuorumTypeEnabled() and return false for null. A parentless index has no prior height at which any LLMQ type could have activated, which is what DeploymentActiveAfter(nullptr, ...) already reported before the parameter was tightened to gsl::not_null. No caller relied on the precondition for correctness; a null argument could only ever abort. Also reject a parentless quorum base explicitly in CFinalCommitment::Verify() and VerifySignatureAsync() and in the DKG message handler, so the condition fails validation close to where it is introduced rather than depending on a callee's contract. GetCommitmentsFromBlock() already rejects unregistered LLMQ types before a commitment reaches these paths, and no chain can legitimately reference genesis as a quorum base, so this rejects nothing that previously validated. While here, move the GetAllQuorumMembers() call in VerifySignatureAsync() below the llmq_params validity check and guard the single-member members[0] indexing against an empty member list. Adds a regression test; it terminates on the unpatched code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a Critical fixes section covering the three remotely triggerable crashes, and adjusts the summary to say upgrading is required for masternodes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings the v23.1.8 release and its three remote-crash fixes into develop. All other master-side changes (the v23.1.x security-hardening train, dashpay#7259..dashpay#7465 and dashpay#7398) were dual-merged and already exist on develop as their own merge commits; conflicts they caused resolve to develop's shape. - New code taken from master: the three crash fixes * skip already-removed conflicts when a ProTx key change clears the mempool (txmempool.cpp + regression test) * reject unregistered LLMQ types from qsigshare before quorum lookup (blockprocessor/quorumsman + regression test); the ProcessMessage gate is ported into develop's net_signing.cpp, and the defence-in-depth find() lookup also applied to GetCachedMutableQuorum() * reject parentless quorum base blocks instead of terminating (commitment/utils/validation + regression test); the DKG handler check is ported into develop's net_dkg.cpp - Release bookkeeping taken from master: v23.1.8 release notes, archived 23.1.7 notes, flatpak release entry, regenerated v23.1.8 man pages; configure.ac keeps develop's flags and IS_RELEASE=false, only the version bump to 23.1.8 is taken. - Resolved to develop's side: dkgsessionmgr/signing_shares message paths (develop moved them to net_dkg.cpp/net_signing.cpp), src/active/quorums.cpp (deleted by develop's ActiveContext refactor), qt/guiutil variants of dashpay#7465, and all remaining dual-merged-PR drift.
|
🕓 Ready for review — 3 ahead in queue (commit e15bb64) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (23)
WalkthroughThe client version is updated to 23.1.8, with matching Flatpak metadata and generated manual pages. Release documentation now covers 23.1.8 and archives the 23.1.7 notes. LLMQ handling rejects parentless quorum references and unknown types without unsafe cache access. Mempool conflict cleanup skips transactions already removed recursively. Regression tests cover invalid LLMQ types, genesis quorum handling, and ProTx conflict chains. Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Peer
participant LLMQNetwork
participant QuorumManager
participant Mempool
Peer->>LLMQNetwork: submit quorum message
LLMQNetwork->>QuorumManager: validate quorum type and base block
QuorumManager-->>LLMQNetwork: reject invalid type or parentless base
LLMQNetwork-->>Peer: apply misbehavior handling
Mempool->>Mempool: recheck conflicting transaction presence
Mempool-->>Mempool: remove remaining conflicts recursively
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
LGTM |
Issue being fixed or feature implemented
Merges master (v23.1.8) back into develop so develop carries the v23.1.8 release and, most importantly, the three remotely reachable crash fixes shipped on top of it. Without this, develop-based nodes remain vulnerable to all three crashes.
What was done?
Merged
upstream/masterintodevelop. The genuinely new payload is:txmempool.cpp)qsigsharebefore quorum lookup (llmq/blockprocessor.cpp,llmq/quorumsman.cpp)llmq/commitment.cpp,llmq/utils.cpp,validation.{cpp,h})configure.acversion bump (develop keepsIS_RELEASE=falseand its own configure flags).Everything else on master since the last merge (the v23.1.x security-hardening train, #7259–#7465 and #7398) was dual-merged and already exists on develop as its own merge commits, so all conflicts from those files resolve to develop's side. Deliberate adaptations, itemized in the merge commit message:
qsigshareLLMQ-type gate is ported into develop'snet_signing.cppand the parentless-base DKG check into develop'snet_dkg.cpp(develop moved message processing out ofsigning_shares.cpp/dkgsessionmgr.cpp).find()lookup is additionally applied toCQuorumManager::GetCachedMutableQuorum(), a develop-only method reached with a wire-supplied LLMQ type from theQDATAhandler.CreateProRegTx/CreateProUpServTx/CreateProUpRevTxhelper signatures andMemPoolOptionsForTest.src/active/quorums.cpp(deleted by develop's ActiveContext refactor) stays deleted; develop's fix(net): bound quorum data response vectors #7416 equivalent already covers the new layout.How Has This Been Tested?
Built with
--enable-debugon macOS (arm64). Ran the new/extended suites —evo_deterministicmns_tests,llmq_invalid_type_tests,evo_utils_tests— plus adjacent ones (llmq_signing_tests,llmq_dkg_tests,llmq_blockprocessor_tests,llmq_commitment_tests,llmq_utils_tests,mempool_tests,validation_tests): all pass. The three regression tests abort the process on unpatched code.Breaking Changes
None.
ChainstateManager::IsQuorumTypeEnabled()loosens itspindexPrevparameter fromgsl::not_nullto a plain pointer (null now returns false instead of aborting); all existing callers are unaffected.Checklist: