fix: run of circular-dependencies with python3.15 by knst · Pull Request #7396 · dashpay/dash

knst · 2026-07-01T07:44:45Z

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:

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
I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

thepastaclaw · 2026-07-01T07:45:05Z

✅ Review complete (commit 7cb0c29)

github-actions · 2026-07-01T07:45:10Z

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

backport: Merge bitcoin#28725, 28771, 28814, 28546, 28076, 28741, 28727, 22764 #7280 - backport: Merge bitcoin#28725, 28771, 28814, 28546, 28076, 28741, 28727, 22764

Please coordinate with the authors of these PRs to avoid merge conflicts.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fe8e0553b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-01T07:47:49Z

-            with Pool(8) as pool:
+            # Use fork so workers inherit the populated `deps` global without
+            # having to pickle it for every task.
+            with multiprocessing.get_context("fork").Pool(8) as pool:


Avoid requiring the fork start method

When this devtool is run with a Windows Python or any other platform that lacks the fork start method, this line raises before any dependency checking runs, which also breaks test/lint/lint-circular-dependencies.py for those environments. The previous multiprocess.Pool path could serialize the nested closure under spawn, so this Python 3.15 fix regresses non-fork platforms; consider passing/initializing deps for workers instead of hard-coding fork.

Useful? React with 👍 / 👎.

coderabbitai · 2026-07-01T07:48:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e0ec1644-82cf-4db3-a366-b173898e282c

📥 Commits

Reviewing files that changed from the base of the PR and between fa97d7ab1979817cdf37e0d3415295f5d0edd575 and 7cb0c29.

📒 Files selected for processing (2)

contrib/containers/ci/ci-slim.Dockerfile
contrib/devtools/circular-dependencies.py

💤 Files with no reviewable changes (1)

contrib/containers/ci/ci-slim.Dockerfile

🚧 Files skipped from review as they are similar to previous changes (1)

contrib/devtools/circular-dependencies.py

Walkthrough

contrib/devtools/circular-dependencies.py now uses the standard library multiprocessing module, with files, deps, and handle_module2 moved to module scope. The cycle search uses a fork-based process pool when available, and falls back to serial mapping otherwise. The CI slim Dockerfile also removes the pinned multiprocess package from its install list.

Estimated code review effort: 2 (Simple) | ~10 minutes

Related PRs: None identified.

Suggested labels: contrib, devtools, refactor

Suggested reviewers: None identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly points to the circular-dependencies Python 3.15 compatibility fix.
Description check	✅ Passed	The description matches the change by explaining the multiprocess/dill issue and the move to global scope.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

thepastaclaw

Code Review

Small dev-tool fix replacing third-party multiprocess (broken on Python 3.15 due to dill/co_lnotab) with stdlib multiprocessing using an explicit fork context, hoisting handle_module2/files/deps to module scope. Behavior is preserved — the pool is recreated inside the outer loop, so each forked worker set inherits an up-to-date deps. Affects only contrib/devtools/circular-dependencies.py; no consensus or runtime code touched.

UdjinM6 · 2026-07-03T09:23:04Z

pls consider adeb8dd

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). Review hint: use `git show -w --color-moved=dimmed-zebra`

…ess dep get_context("fork") raises ValueError on platforms without fork (Windows), so guard it and run handle_module2 serially there. Also remove the now-unused multiprocess pip package, whose only consumer was this script. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

thepastaclaw

Code Review

Small, well-scoped dev-tool fix replacing the third-party multiprocess package with stdlib multiprocessing to unblock Python 3.15 (dill's 3.15 support is broken due to co_lnotab removal). Second commit adds a serial-map fallback for platforms without fork (Windows) and drops the now-unused pip package from the slim CI image. Both changes are correct: handle_module2 and its deps/files globals are hoisted to module scope so fork workers inherit the populated dependency graph, and the serial path runs in the same process where the globals are already populated. No carried-forward findings from the prior 0fe8e05 review (which was clean) and no new blocking or suggestion-level issues in the latest delta.

Source: reviewers opus/opus, sonnet5/claude-sonnet-5, codex/gpt-5.5[high] (failed: initialize stall) for general + dash-core-commit-history; verifier opus/opus.

thepastaclaw · 2026-07-03T11:59:04Z

Thanks. The current PR head 7cb0c292f3325a619e1dc04d63237c993f74a025 already contains the same patch as adeb8dda470fbf3c48d099068800052d71cf0cf5 (same diff/message); I re-reviewed and approved that head.

UdjinM6

utACK 7cb0c29

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

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)

@knst

24920a0 chore: prepare v23.1.8 release (pasta) 2194248 Merge #7348: fix: penalize oversized notfound messages (pasta) f5c72c3 Merge #7347: fix: punish invalid dstx messages (pasta) 550caf7 Merge #7465: fix(qt): handle pixel-sized fonts when scaling widgets (pasta) e203710 Merge #7419: fix(net): bound CoinJoin message vector intake (pasta) 5f5b960 Merge #7418: fix(net): bound signing message vector intake (Pasta) 7cc2cca Merge #7450: test: make governance vote fixtures wire-valid (Pasta) f011c80 Merge #7440: fix(net): bound governance vote signature deserialization (Pasta) 4b4d96a Merge #7442: fix(net): authorize governance inv responses via the net-layer per-peer request tracker (Pasta) f855b13 Merge #7444: fix(net): bound bloom message vectors before allocation (Pasta) 5b5c6fb Merge #7415: fix: bound pending sig share queue (Pasta) 9bbe808 Merge #7416: fix(net): bound quorum data response vectors (Pasta) da42f50 Merge #7424: fix: bound ChainLock seen cache (Pasta) 5b310df Merge #7438: fix: bound SPORK signature deserialization (Pasta) e118d0c Merge #7259: fix: dangling point to cj client (Pasta) 9921621 Merge #7439: refactor: add bounded vector deserialization (Pasta) 89bdf7c Merge #7414: fix(net): throttle per-object governance vote sync requests (Pasta) 44c396d Merge #7402: fix: bound pending recovered sig queue to prevent remote OOM (Pasta) 05cfe27 Merge #7351: fix: limit signing share sessions per peer (pasta) 3ef3a5b Merge #7408: fix: bound DKG contribution blob intake (pasta) 0ea6532 Merge #7387: test: migrate governance inv cache coverage to unit tests (Pasta) 8ffdf7f Merge #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 #7396: fix: run of circular-dependencies with python3.15 (Pasta) b003cdc Merge #7395: ci: update GitHub Actions pins for Node 24 (pasta) 97c3dd1 Merge #7394: fix: stabilize par help text in manpages (pasta) 8f8616b Merge #7372: backport: bitcoin#32693: depends: fix cmake compatibility error for freetype (pasta) 48f72be Merge #7360: fix: empty platformP2PPort deprecated field in protx listdiff results (pasta) a8cccff Merge #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`: `#7259` `#7347` `#7348` `#7351` `#7298` `#7360` `#7372` `#7387` `#7394` `#7395` `#7396` `#7398` `#7402` `#7408` `#7414` `#7415` `#7416` `#7418` `#7419` `#7424` `#7438` `#7439` `#7440` `#7442` `#7444` `#7450` `#7465` Plus `backport: bitcoin#27608`, a single commit taken from Dash #7237 because #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 #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 #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 #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 #7418 and #7438 patch `signing_shares.cpp` / `spork.cpp` where upstream patches `net_signing.cpp` / `net_processing.cpp`. ## Dropped from this branch - **#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: #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 - **#7351** (`fix: limit signing share sessions per peer`) — cherry-picked as a single commit and placed before #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`. - **#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: - **#7347** (`fix: punish invalid dstx messages`) - **#7348** (`fix: penalize oversized notfound messages`) ## Adaptations worth flagging - **#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 #7360 — reading the live port from `netInfo` instead of the always-zero scalar — is retained. - **#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 #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 #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

knst added this to the 24 milestone Jul 1, 2026

knst requested review from PastaPastaPasta and UdjinM6 July 1, 2026 07:45

chatgpt-codex-connector Bot reviewed Jul 1, 2026

View reviewed changes

thepastaclaw approved these changes Jul 1, 2026

View reviewed changes

knst and others added 2 commits July 3, 2026 17:46

fix: run of circular-dependencies with python3.15

de1cc2c

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). Review hint: use `git show -w --color-moved=dimmed-zebra`

knst force-pushed the fix-multiprocess-3.15 branch from fa97d7a to 7cb0c29 Compare July 3, 2026 10:47

thepastaclaw approved these changes Jul 3, 2026

View reviewed changes

UdjinM6 approved these changes Jul 3, 2026

View reviewed changes

knst added the backport-candidate-23.1.x label Jul 5, 2026

This was referenced Jul 5, 2026

fix!: use the same version along all protx special transactions #7302

Merged

backport: bitcoin#24322, #25064, #25065, #25168, #25281, #25290, #25307, #25487, #25607, #26003 (kernel backports) #7361

Merged

PastaPastaPasta merged commit 87ac140 into dashpay:develop Jul 7, 2026
63 of 85 checks passed

thepastaclaw mentioned this pull request Jul 7, 2026

feat(rpc): expose upcoming DKG participation #7404

Closed

5 tasks

knst deleted the fix-multiprocess-3.15 branch July 9, 2026 11:50

thepastaclaw mentioned this pull request Jul 22, 2026

chore: prepare v23.1.8 release #7477

Closed

PastaPastaPasta mentioned this pull request Jul 28, 2026

release: prepare v23.1.8 #7493

Merged

UdjinM6 modified the milestones: 24, 23.1.8 Jul 30, 2026

UdjinM6 removed the backport-candidate-23.1.x label Jul 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: run of circular-dependencies with python3.15 - #7396

fix: run of circular-dependencies with python3.15#7396
PastaPastaPasta merged 2 commits into
dashpay:developfrom
knst:fix-multiprocess-3.15

knst commented Jul 1, 2026 •

edited

Loading

Uh oh!

thepastaclaw commented Jul 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 1, 2026

Uh oh!

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

thepastaclaw left a comment

Uh oh!

UdjinM6 commented Jul 3, 2026

Uh oh!

thepastaclaw left a comment •

edited

Loading

Uh oh!

thepastaclaw commented Jul 3, 2026

Uh oh!

UdjinM6 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

knst commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

thepastaclaw commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 1, 2026

⚠️ Potential Merge Conflicts Detected

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

UdjinM6 commented Jul 3, 2026

Uh oh!

thepastaclaw left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review

Uh oh!

thepastaclaw commented Jul 3, 2026

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

knst commented Jul 1, 2026 •

edited

Loading

thepastaclaw commented Jul 1, 2026 •

edited

Loading

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading

thepastaclaw left a comment •

edited

Loading