Skip to content

Merge bitcoin/bitcoin#29218: ci: Rename tasks (previous releases, macOS cross)#1057

Closed
DashCoreAutoGuix wants to merge 2 commits into
backport-0.23-batch-720from
backport-0.23-batch-720-pr-29218
Closed

Merge bitcoin/bitcoin#29218: ci: Rename tasks (previous releases, macOS cross)#1057
DashCoreAutoGuix wants to merge 2 commits into
backport-0.23-batch-720from
backport-0.23-batch-720-pr-29218

Conversation

@DashCoreAutoGuix
Copy link
Copy Markdown
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Aug 7, 2025

Backports bitcoin#29218

Original commit: 8c0d1c6

Backported from Bitcoin Core v0.27

Summary by CodeRabbit

  • Chores
    • Updated workflow configuration to improve cache management during release testing.
    • Changed environment variable naming for consistency in test setup scripts.

fa0c594 ci: Rename tasks (previous releases, macOS cross) (MarcoFalke)

Pull request description:

  The previous releases task no longer uses the qt5 dev package, but the depends package, so fix that in the name.

  Also, remove a detail from the macOS cross task name, because anyone can look it up in the source, if they really want to. Otherwise, it may go out of date in the name.

  Also, rename the two tasks' config file to reflect the same.

ACKs for top commit:
  fanquake:
    ACK fa0c594

Tree-SHA512: e6f1d04128d35462c49367c98a0227988695d75add88c569804551e3fd30c22292d22b88fa19c54f02fab0c9784c77a078447de0280553a3fc7162dcf992d7ae
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 7, 2025

Walkthrough

The changes update a GitHub Actions workflow cache key to reference a different setup script and modify an environment variable in that script. Specifically, the workflow now uses ci/test/00_setup_env_native_previous_releases.sh for cache key generation, and the script updates the CONTAINER_NAME variable to a new value.

Changes

Cohort / File(s) Change Summary
Workflow Cache Key Update
.github/workflows/test-src.yml
The releases cache key now uses ci/test/00_setup_env_native_previous_releases.sh instead of ci/test/00_setup_env_native_qt5.sh for hash computation, potentially affecting cache hits.
Environment Variable Update
ci/test/00_setup_env_native_previous_releases.sh
The CONTAINER_NAME environment variable is changed from ci_native_qt5 to ci_native_previous_releases. No other changes made.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4202b5 and bdff3aa.

📒 Files selected for processing (2)
  • .github/workflows/test-src.yml (1 hunks)
  • ci/test/00_setup_env_native_previous_releases.sh (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
.github/**

📄 CodeRabbit Inference Engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to .github directory (GitHub workflows and configs)

Files:

  • .github/workflows/test-src.yml
ci/**

📄 CodeRabbit Inference Engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the ci directory (continuous integration)

Files:

  • ci/test/00_setup_env_native_previous_releases.sh
**

⚙️ CodeRabbit Configuration File

**: # CodeRabbit AI Review Instructions for Dash Backports

Your Role

You are reviewing Bitcoin Core backports to Dash Core. Your ONLY job is to validate that the Dash commit faithfully represents the original Bitcoin commit with minimal, necessary adaptations.

Critical Validation Rules

1. File Operations Must Match (AUTO-REJECT if violated)

  • If Bitcoin modifies an existing file → Dash MUST modify (not create new)
  • If Bitcoin creates a new file → Dash creates
  • If Bitcoin deletes a file → Dash deletes
  • Common failure: Bitcoin modifies keys.txt, Dash creates new file with 58 keys

2. Size Ratio Check (80-150% of Bitcoin)

  • Count functional lines changed (exclude comments/whitespace)
  • Dash changes should be 80-150% of Bitcoin's size
  • Red flag: 2-line Bitcoin fix becoming 150+ lines in Dash

3. No Scope Creep

  • Reject if you see: "TODO:", "FIXME:", "while we're here", "also fix"
  • No unrelated refactoring or style changes
  • Only Bitcoin's intended changes + minimal Dash adaptations

4. Bitcoin-Specific Code Detection

  • Auto-reject witness/segwit code: msg_wtxidrelay, MSG_WTX, witness imports
  • Auto-reject RBF (replace-by-fee) functionality
  • Note: PSBT is supported in Dash (don't flag)

5. Mandatory Adaptations Only

  • bitcoindash in strings/paths
  • BitcoinDash in user-facing text
  • Port numbers: 8332→9998 (RPC), 8333→9999 (P2P)
  • Hardcoded test values specific to Dash
  • No other changes unless absolutely required

6. Completeness Check

  • All files changed in Bitcoin must be present
  • Extra files need clear justification (Dash-specific compatibility)
  • Missing files = incomplete backport

Review Process

  1. First: Check file operations match exactly
  2. Second: Calculate size ratio
  3. Third: Scan for scope creep patterns
  4. Fourth: Detect Bitcoin-specific code
  5. Fifth: Verify all changes are minimal adaptations

Output Format

VALIDATION: [PASS/FAIL]

File Operatio...

Files:

  • ci/test/00_setup_env_native_previous_releases.sh
🧠 Learnings (4)
📓 Common learnings
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T19:54:21.426Z
Learning: In Dash backports from Bitcoin Core, including necessary compilation fixes (such as API compatibility changes like UniValue get_int() → getInt<int>()) alongside the core backport is standard and expected practice. These compatibility fixes ensure the backported code compiles in Dash's evolved codebase while preserving Bitcoin's original functionality and intent.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-27T22:35:10.176Z
Learning: In Dash backports, src/dashbls files are vendored dependencies that should not be modified during Bitcoin Core backports unless there is specific justification. Unauthorized modifications to vendored dependencies should be removed to maintain code integrity.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: When backporting Bitcoin Core commits that use Python's textwrap.dedent() function in test files, the textwrap import statement needs to be explicitly added if it's missing in the Dash test file.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: In Dash backports from Bitcoin Core test files, the `address_to_scriptpubkey` function should be imported from `test_framework.wallet` instead of `test_framework.address` as the import location differs between Bitcoin and Dash test frameworks.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.
Learnt from: knst
PR: DashCoreAutoGuix/dash#436
File: src/univalue/include/univalue.h:86-86
Timestamp: 2025-08-04T18:16:45.145Z
Learning: UniValue (src/univalue) is no longer a vendored dependency in Dash as of PR #6775, so modifications to UniValue files are permitted when needed for Bitcoin Core backports and other legitimate changes.
📚 Learning: during multiple verification attempts of bitcoin core commit 06d469c backport to dash pr #566, da...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.

Applied to files:

  • .github/workflows/test-src.yml
📚 Learning: during dash backport verification of bitcoin core commit 06d469c, scope creep was detected when a...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.

Applied to files:

  • ci/test/00_setup_env_native_previous_releases.sh
📚 Learning: dashcoreautoguix successfully fixed scope creep in bitcoin core commit fcdb39d backport by removi...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.

Applied to files:

  • ci/test/00_setup_env_native_previous_releases.sh
🔇 Additional comments (3)
ci/test/00_setup_env_native_previous_releases.sh (2)

9-9: Container name consistently updated – looks good

Renaming CONTAINER_NAME to ci_native_previous_releases aligns this script with the file-rename backport and avoids cache collisions with the former ci_native_qt5 container. No further action required.


18-18: Confirm continued use of BITCOIN_CONFIG in Dash CI

The BITCOIN_CONFIG variable remains unchanged. Dash CI scripts historically still expect this exact variable, so leaving it intact is correct. Please just double-check that no Dash-specific overrides elsewhere rely on a different name.

.github/workflows/test-src.yml (1)

45-47: Cache-key path updated to new script – change is correct

Updating the hashFiles path to ci/test/00_setup_env_native_previous_releases.sh keeps the cache key in sync with the renamed setup script. This matches upstream Bitcoin Core commit 8c0d1c6 and prevents stale cache usage.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.23-batch-720-pr-29218

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

- Update cache key from '00_setup_env_native_qt5.sh' to '00_setup_env_native_previous_releases.sh'
- This matches the file rename from bitcoin#29218
- Ensures cache invalidation works correctly after file rename

Resolves validation issues while preserving Bitcoin intent.
@DashCoreAutoGuix
Copy link
Copy Markdown
Owner Author

✅ Backport Verification - Issues Fixed

I found 1 issue and automatically applied fixes.

Original Bitcoin commit: 8c0d1c6cc26
Changes pushed to branch: backport-0.23-batch-720-pr-29218
Fix commit: bdff3aa0996
Reviewed commit hash: 91fc1845d38-verify-1754564836

Issues Found and Fixed:

  1. Missing GitHub Actions cache key update - The workflow file .github/workflows/test-src.yml still referenced the old filename ci/test/00_setup_env_native_qt5.sh in its cache key. Updated to reference the renamed file ci/test/00_setup_env_native_previous_releases.sh.

Validation Results:

  • Witness/Segwit code: None detected (safe for Dash)
  • CI Status: All 38 checks passing (normal status)
  • File operations: Properly implements Bitcoin's file renames
  • Size validation: Now complete after adding missing reference update
  • Intent preservation: Maintains Bitcoin's CI task renaming purpose

Reviewer Feedback Analysis:

PastaPastaPasta comments reviewed: 0
Other human comments reviewed: 0

No reviewer feedback found - all automated validation passed.

Bitcoin Changes Analysis:

The Bitcoin commit renamed two CI setup files and updated their references:

  1. 00_setup_env_mac.sh00_setup_env_mac_cross.sh
  2. 00_setup_env_native_qt5.sh00_setup_env_native_previous_releases.sh

The Dash PR correctly applied these renames but missed updating the GitHub Actions cache key reference. This has now been fixed.

The backport should now be complete and faithful to the original.

@DashCoreAutoGuix DashCoreAutoGuix added the fixes-applied Issues found and fixes automatically applied label Aug 7, 2025
@DashCoreAutoGuix
Copy link
Copy Markdown
Owner Author

✅ Backport Verification Successful

No issues found! This backport is complete and faithful to the original Bitcoin change.

Original Bitcoin commit: 8c0d1c6cc26 (Merge bitcoin#29218: ci: Rename tasks (previous releases, macOS cross))
Backport PR: #1057
Reviewed commit hash: bdff3aa0996-verify-1754565142

Verification Details:

  • File operations match Bitcoin's intent (CI system adaptation)
  • No missing changes detected (perfect Dash adaptation)
  • No scope creep or dependency explosions
  • CI checks passing (0 failures)
  • PR comments reviewed and addressed (0 human comments)

Analysis Summary:

This is an excellent example of proper Dash adaptation:

Bitcoin Intent: Update CI cache keys after renaming files from qt5previous_releases

  • Bitcoin: Updated .cirrus.yml cache references (Cirrus CI)
  • Dash: Updated .github/workflows/test-src.yml cache references (GitHub Actions)

Perfect Faithfulness: Same functional change (update cache key), correctly adapted to Dash's CI system.

The MCP tool flagged this as "missing files" due to CI system differences, but manual analysis confirms this is a perfect backport with proper adaptation.

Reviewer Feedback Analysis:

PastaPastaPasta comments reviewed: 0
Other human comments reviewed: 0

No reviewer feedback to address - all automated checks pass.

This PR is ready for merge. ✅

@DashCoreAutoGuix DashCoreAutoGuix added the verified Backport verification passed - ready for merge label Aug 7, 2025
@PastaPastaPasta PastaPastaPasta added ci-failed PR has one or more failing CI jobs and removed verified Backport verification passed - ready for merge labels Aug 21, 2025
@DashCoreAutoGuix
Copy link
Copy Markdown
Owner Author

⚠️ Validation requires manual intervention

Reason: CI has more than 1 failing job (3 CI jobs failing)

🚫 CI Check: 3 jobs failing (threshold: max 1 allowed)
The agent is prohibited from approving PRs with >1 failing CI job.

Failing Jobs:

  • x86_64-pc-linux-gnu / Build depends: FAILURE
  • x86_64-apple-darwin / Build depends: FAILURE
  • Lint / Run linters: FAILURE

Backport Quality Assessment:
✅ The backport itself is technically CORRECT:

  • Properly renames CI test files per Bitcoin commit fa0c594
  • Correctly excludes .cirrus.yml changes (not applicable to Dash)
  • Includes smart GitHub Actions cache key adaptation
  • Faithful backporting with appropriate Dash-specific adaptations

Manual Action Required:
Please investigate and resolve the CI failures before this PR can be approved. The backport changes appear sound, but CI must pass for approval.

@DashCoreAutoGuix DashCoreAutoGuix added the needs-fixes Requires manual fixes to address verification issues label Aug 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-failed PR has one or more failing CI jobs fixes-applied Issues found and fixes automatically applied needs-fixes Requires manual fixes to address verification issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants