fix: repair makeseeds.py, getblockchaininfo[softforks] help text, drop extra generates from test, resolve macOS GID issue

[kwvg]

Additional Information

• The help text for getblockchaininfo['softforks'] was introduced in bitcoin#16060 (backported in dash#5255) but the RPCResult help text was introduced in bitcoin#17809, backported before bitcoin#16060. This resulted in the help text for softforks deviating from upstream without the output to match (which is aligned with upstream).

  This has since been resolved as the output structure hasn't changed, only the help text.

• bitcoin#22818 (backported in dash#6214) activated many softforks at height 1, which rendered the block generation calls in wallet_signrawtransactionwithwallet.py (source) no longer necessary, additionally bitcoin#22550 (backported in dash#6189) added assertions to validate CSV/CLTV activation that were not included in the backport.
  This has since been resolved, replacing the generate calls with assertions.

  • The locktime height was dropped from 1000 to 100 (matching with upstream, source) as we are no longer generating enough blocks to reach that height anymore.

• makeseeds.py is currently mangled because dash#6666 updated the script to look for the core_p2p purpose even though purpose code support was spun-off into dash#6674. This meant, in between dash#6666 and dash#6674's merger, makeseeds.py was broken.

  This necessitated a fix that was introduced as e714c06 (dash#6858) but meant that after dash#6674, makeseeds.py is broken again. This has been resolved as part of this PR.

• macOS users who want to utilise the develop container (source) need to set the USER_ID and GROUP_ID args to avoid permissions issues. This creates a slight problem as the GID for staff is 20 on macOS but Linux uses GID 20 for dialout, so creating a group with GID 20 will fail.

  As we only care about the GID, it is safe to skip the group creation step and assign the user to the group anyways as we do it by GID and not name (the user will be presented as a member of the dialout group). This should resolve issues for macOS users trying to build the develop container.

Breaking Changes

None expected.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[kwvg]

@coderabbitai review

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

This pull request modifies several components across infrastructure, data processing, RPC interfaces, and tests. The Dockerfile startup sequence is updated to conditionally apply group modifications only if the target group does not exist. The makeseeds.py script changes its address extraction logic to target a specific core_p2p nested key instead of the first address in a flat list. The RPC schema for getblockchaininfo() is restructured: the softforks field transitions from a static object to a dynamic object with reorganized per-softfork entries that include a "type" indicator, expanded bip9-specific fields, and new top-level "height" and "active" fields. Corresponding test updates replace block generation-based soft-fork activation checks with direct queries to the modified softforks schema.

Sequence Diagram(s)

sequenceDiagram
    participant Client as RPC Client
    participant RPC as getblockchaininfo()
    participant Schema as Softforks Schema

    Client->>RPC: Request blockchain info
    RPC->>Schema: Build softforks response
    rect rgb(240, 248, 255)
        Note over Schema: Old: Static per-deployment keys
        Schema->>Schema: Fixed keys for each deployment
    end
    rect rgb(173, 216, 230)
        Note over Schema: New: Dynamic object structure
        Schema->>Schema: Generic "xxxx" placeholder key
        Schema->>Schema: Per-softfork: type + bip9/details
        Schema->>Schema: Add height and active fields
    end
    RPC->>Client: Return updated softforks structure

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

• src/rpc/blockchain.cpp: RPC schema restructuring for softforks requires careful validation of the new dynamic object structure and field reorganization to ensure compatibility and correct semantic representation.
• contrib/seeds/makeseeds.py: Address extraction logic change from flat indexing to nested dictionary lookup requires verification that the new key path (core_p2p) exists and produces correct IP/onion address results.
• test/functional/wallet_signrawtransactionwithwallet.py: Soft-fork activation check refactoring needs confirmation that the updated schema assertions align with the modified RPC response structure.
• contrib/containers/ci/ci-slim.Dockerfile: Conditional group modification logic requires review to ensure the guard check and subsequent ownership adjustments maintain correct permissions.

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title directly and explicitly maps to the four main fixes in the changeset: repair to makeseeds.py, updates to getblockchaininfo[softforks] help text, removal of extra generate calls from wallet tests, and resolution of the macOS GID issue. Each item listed corresponds to a real, concrete change present in the PR across the modified files. While the title reads somewhat like a summary list rather than a single cohesive statement, it is specific, clear, and avoids vague terminology, allowing a teammate scanning history to understand that this is a multi-fix PR addressing these particular issues. The title successfully conveys the primary changes without ambiguity.
Description Check	✅ Passed	The pull request description is directly related to the changeset and provides substantial context for all four fixes being applied. The author explains the historical context and upstream backport ordering for the getblockchaininfo softforks help text, details the makeseeds.py breakage caused by intermediate backport state, clarifies why test generate calls are no longer needed, and explains the macOS GID conflict with appropriate technical reasoning. The description includes references to specific upstream and internal PRs, breaking changes assessment, and a completed checklist. This level of detail and relevance to the actual changes in the PR far exceeds the lenient pass threshold for this check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4fa575 and 39e847c.

📒 Files selected for processing (4)

• contrib/containers/ci/ci-slim.Dockerfile (1 hunks)
• contrib/seeds/makeseeds.py (1 hunks)
• src/rpc/blockchain.cpp (1 hunks)
• test/functional/wallet_signrawtransactionwithwallet.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be written in Python and placed in test/functional/

Files:

• test/functional/wallet_signrawtransactionwithwallet.py

contrib/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)

Files:

• contrib/seeds/makeseeds.py
• contrib/containers/ci/ci-slim.Dockerfile

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/rpc/blockchain.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build container / Build container

🔇 Additional comments (4)

contrib/containers/ci/ci-slim.Dockerfile (1)

108-114: LGTM! Defensive guard for group creation.

The conditional check correctly prevents group modification failures when the target GID already exists (e.g., GID 20 'dialout' on Linux vs 'staff' on macOS). This maintains backward compatibility while avoiding CI failures.

src/rpc/blockchain.cpp (1)

1465-1492: LGTM! Help text now matches actual RPC output.

The restructured softforks documentation correctly reflects the dynamic object structure with per-fork entries. Key improvements:

• OBJ_DYN properly indicates variable softfork keys
• Placeholder "xxxx" follows RPC documentation conventions
• Expanded bip9 fields (ehf, ehf_height, activation_height, min_activation_height) align with implementation
• Top-level "height" and "active" fields match the actual output structure

test/functional/wallet_signrawtransactionwithwallet.py (2)

127-134: LGTM! Direct activation check is clearer.

Asserting CSV activation via getblockchaininfo()['softforks']['csv']['active'] is more explicit and maintainable than block-generation-based heuristics. This leverages the updated RPC schema from src/rpc/blockchain.cpp.

---

157-166: LGTM! Simplified CLTV test aligns with upstream.

Changes improve the test:

• Direct activation assertion via getblockchaininfo()['softforks']['bip65']['active'] is clearer
• Reduced locktime from 1000 to 100 matches upstream test behavior (test: improve test_signing_with_{csv,cltv} subtests (speed, prevent timeout) bitcoin/bitcoin#22550/dash#6189)

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Overview

Greptile Summary

This PR addresses four independent fixes that resolve issues from previous backports and cross-platform compatibility problems. The changes repair the makeseeds.py script to work with the new ExtAddr structure where addresses are keyed by purpose codes like core_p2p, fix help text documentation for getblockchaininfo RPC's softforks field to accurately reflect the dynamic object structure, streamline the wallet_signrawtransactionwithwallet.py functional test by removing unnecessary block generation now that softforks activate at height 1, and resolve a Docker build failure for macOS users where GID 20 conflicts between macOS's staff group and Linux's dialout group. These are all cleanup and correction changes that don't alter core functionality.

Important Files Changed

Filename	Score	Overview
contrib/seeds/makeseeds.py	5/5	Fixed masternode address access to use new core_p2p purpose code structure
src/rpc/blockchain.cpp	5/5	Corrected help text for softforks field to match actual dynamic object output
test/functional/wallet_signrawtransactionwithwallet.py	5/5	Removed obsolete block generation, added softfork activation assertions, reduced locktime to 100
contrib/containers/ci/ci-slim.Dockerfile	5/5	Added conditional check to skip group modification when GID exists for macOS compatibility

Confidence score: 5/5

• This PR is safe to merge with minimal risk as all changes are well-documented fixes to known issues with clear explanations
• Score reflects straightforward corrections with no complex logic changes, clear backport context, and targeted fixes for documented problems
• No files require special attention; all changes are simple, well-explained, and address specific documented issues with appropriate justification

Context used:

• Context from dashboard - CLAUDE.md (source)

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[PastaPastaPasta]

utACK 39e847c

[knst]

utACK 39e847c

[UdjinM6]

utACK 39e847c