Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Sep 30, 2025

Backport Summary

This backports Bitcoin Core PR bitcoin#27444 (commit cd603ed) to Dash Core.

Original Bitcoin Changes

  • Updates Valgrind CI jobs from Debian Buster to Debian Bookworm
  • Uses Valgrind 3.19 (available in Bookworm)
  • Updates compiler from clang-18 to clang (Bookworm default)
  • Adds new packages: libnatpmp-dev, libsqlite3-dev
  • Updates valgrind.supp suppressions file for Bookworm
  • Removes libstdc++ suppression (no longer needed)
  • Changes CCACHE_MAXSIZE to CCACHE_SIZE

Dash-Specific Adaptations

None required. This is a straightforward CI configuration update that applies cleanly to Dash Core.

Files Modified

  • ci/test/00_setup_env_native_fuzz_with_valgrind.sh
  • ci/test/00_setup_env_native_valgrind.sh
  • contrib/valgrind.supp

Testing

CI configuration changes only - no code changes requiring build/test.


Bitcoin Commit: cd603ed
Bitcoin PR: bitcoin#27444
Backport Version: 0.25
Batch: 412


Note

Updates Valgrind CI to Debian Bookworm, switches to default clang, adjusts packages and ccache var, and refreshes Valgrind suppressions for Bookworm.

  • CI (Valgrind jobs):
    • Use Debian Bookworm base image via CI_IMAGE_NAME_TAG="debian:bookworm" in ci/test/00_setup_env_native_*valgrind*.sh.
    • Switch compiler from clang-18/clang++-18 to clang/clang++; keep DWARF-4 pin with updated comment.
    • Update packages:
      • Remove libclang-rt-dev.
      • Add libnatpmp-dev, libsqlite3-dev (and refine lists).
    • Set CONTAINER_NAME=ci_native_valgrind for native valgrind job; keep fuzz container name.
    • Rename CCACHE_MAXSIZE to CCACHE_SIZE in fuzz-with-valgrind setup.
  • Valgrind suppressions (contrib/valgrind.supp):
    • Update "Tested on" to Bookworm/clang.
    • Remove obsolete libstdc++ and GUI (libgdk-3) suppressions; retain other suppressions.

Written by Cursor Bugbot for commit 0d84ef2. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Chores
    • Updated CI environment to Debian Bookworm with standardized clang toolchain and expanded package set.
    • Renamed ccache size variable and aligned configuration for consistency.
  • Tests
    • Updated Valgrind setup for 3.20+ and refreshed suppressions to match Debian Bookworm.
    • Reduced false positives in memory checks (e.g., libdb/stdlib noise) and stabilized native fuzzing.
    • Preserved DWARF-4 debug flags to maintain Valgrind compatibility.

…grind jobs

e047ae8 valgrind: update supps for Debian Bookworm. (fanquake)
ba29143 ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs (fanquake)

Pull request description:

  Switch to using Debian Bookworm and [valgrind 3.19](https://packages.debian.org/bookworm/valgrind) in the Valgrind jobs. Also update the suppressions file.

  This originally contained a changed to build valgrind 3.20 from source (for improved aarch64 support), but I'll split that into it's own change.

Top commit has no ACKs.

Tree-SHA512: 73ec162d6e07f8a6767d15c0fc298ec6e1a2ba8ec8f9ea902dbfd0a1e3c491411781beec2f6de66fd15006475dbc024bc512f09aa94e2615b713ba873fac14de
@coderabbitai
Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Updates CI native Valgrind and fuzz-with-Valgrind setup scripts to use Debian Bookworm, generic clang/clang++, and adjust ccache variable naming. Package lists are updated. Valgrind suppressions are replaced to match Debian Bookworm, removing old libstdc++ and GUI-related entries and adding libdb-focused suppressions.

Changes

Cohort / File(s) Summary
CI native Valgrind setup scripts
ci/test/00_setup_env_native_fuzz_with_valgrind.sh, ci/test/00_setup_env_native_valgrind.sh
Set CI_IMAGE_NAME_TAG to "debian:bookworm"; switch BITCOIN_CONFIG compilers from clang-18/clang++-18 to clang/clang++; keep -gdwarf-4 flags with updated Valgrind 3.20+ context; rename CCACHE_MAXSIZE to CCACHE_SIZE; add packages (e.g., libnatpmp-dev, libsqlite3-dev); set CONTAINER_NAME for native Valgrind script.
Valgrind suppressions (Bookworm alignment)
contrib/valgrind.supp
Replace prior libstdc++/GUI leak suppressions with Debian Bookworm-oriented rules; add multiple libdb-related Memcheck suppressions (Cond, Param, Leak) and related IO paths; remove obsolete GUI leak suppression.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title directly conveys the primary change—updating the CI to use Debian Bookworm and Valgrind 3.19 for Valgrind jobs—while also indicating the upstream PR reference; it is specific, concise, and clearly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ 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.25-batch-412-pr-27444

📜 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 54e2588 and 0d84ef2.

📒 Files selected for processing (3)
  • ci/test/00_setup_env_native_fuzz_with_valgrind.sh (2 hunks)
  • ci/test/00_setup_env_native_valgrind.sh (1 hunks)
  • contrib/valgrind.supp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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_valgrind.sh
  • ci/test/00_setup_env_native_fuzz_with_valgrind.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_valgrind.sh
  • contrib/valgrind.supp
  • ci/test/00_setup_env_native_fuzz_with_valgrind.sh
contrib/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • contrib/valgrind.supp
🧠 Learnings (1)
📓 Common learnings
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-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-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.
⏰ 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: Cursor Bugbot
🔇 Additional comments (7)
ci/test/00_setup_env_native_valgrind.sh (2)

9-11: CI configuration updated to Debian Bookworm with additional packages.

The changes correctly update the container image to Debian Bookworm, add the container name, and expand the package list with libnatpmp-dev and libsqlite3-dev. These align with the Bitcoin Core backport objectives.


16-17: Approve code changes as-is. Debian Bookworm ships Valgrind 3.19.0, so pinning DWARF-4 until upgrading to ≥3.19 remains correct; switching from clang-18/clang++-18 to generic clang/clang++ matches Bookworm’s defaults, and BITCOIN_CONFIG is consistently used across all CI scripts.

ci/test/00_setup_env_native_fuzz_with_valgrind.sh (3)

9-9: CI image updated to Debian Bookworm.

Correctly adds the CI_IMAGE_NAME_TAG export to specify Debian Bookworm container image, consistent with the backport objectives.


18-19: Compiler specification simplified for Bookworm compatibility.

The change from explicit clang-18/clang++-18 to generic clang/clang++ aligns with using Debian Bookworm's default clang version. Comment correctly notes the dwarf-4 pinning rationale for Valgrind 3.20+ compatibility.


20-20: Ccache variable renamed to CCACHE_SIZE.

The rename from CCACHE_MAXSIZE to CCACHE_SIZE reflects updated ccache configuration naming conventions. Value remains 200M.

contrib/valgrind.supp (2)

16-17: Documentation updated to reflect Debian Bookworm testing environment.

The comment correctly updates the tested platforms from Ubuntu to Debian Bookworm (aarch64 and x86_64), noting the use of system libs, clang compiler, and "without gui" configuration. This aligns with the CI configuration changes in the setup scripts.


18-125: Valgrind suppressions updated for Debian Bookworm environment.

The suppressions have been appropriately updated for the new Debian Bookworm environment:

  • Previous libstdc++ and libgdk GUI suppressions removed (Ubuntu 20.04/18.04-specific)
  • New libdb suppressions added targeting Debian Bookworm's library versions
  • Existing suppressions for leveldb, boost, shutdown leaks, etc. retained

The wholesale replacement of environment-specific suppressions is appropriate when migrating CI infrastructure from Ubuntu to Debian Bookworm, as Valgrind warnings are OS and library-version dependent as noted in line 14.


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 and usage tips.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on October 7

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

export CCACHE_MAXSIZE=200M
# Temporarily pin dwarf 4, until using Valgrind 3.20 or later
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++ CFLAGS='-gdwarf-4' CXXFLAGS='-gdwarf-4'"
export CCACHE_SIZE=200M
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Cache Size Variable Name Change

The CCACHE_MAXSIZE environment variable was changed to CCACHE_SIZE. CCACHE_SIZE is not a recognized ccache environment variable, so the intended cache size limit isn't applied, potentially leading to unbounded cache growth.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants