Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Dec 9, 2025

Partial backport of bitcoin#26508

Changes

Only the scantxoutset changes are applicable as Dash does not have the scanblocks RPC function.

This PR improves the error message for invalid action in scantxoutset:

  • Old: "Invalid command"
  • New: "Invalid action 'X'" (includes the invalid action name for better debugging)

Bitcoin PR Summary

Original Bitcoin PR added:

  1. scanblocks: Improved error message (not in Dash)
  2. scanblocks: Documentation improvements (not in Dash)
  3. scantxoutset: Improved error message ✓

Testing

  • Build: ✓ Passed
  • Unit tests (rpc_tests): ✓ Passed
  • Updated test/functional/rpc_scantxoutset.py to match new error format

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error messages for invalid scantxoutset command actions to display the specific invalid action value provided, improving debugging clarity.

✏️ Tip: You can customize this high-level summary in your review settings.

Partial backport of bitcoin#26508

Only the scantxoutset changes are applicable as Dash does not have
the scanblocks RPC function.

Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Walkthrough

Error message for invalid scantxoutset RPC actions made more specific. The thrown JSONRPCError now includes the invalid action value in the message format, changing from "Invalid command" to "Invalid action ''". The corresponding test is updated to expect the new error message format.

Changes

Cohort / File(s) Summary
Error message improvement for scantxoutset
src/rpc/blockchain.cpp, test/functional/rpc_scantxoutset.py
Updated error message to include the actual invalid action value for clarity. Test assertion updated to match new error message format "Invalid action 'invalid_command'" instead of "Invalid command".

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 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 accurately reflects the main change: improving error messages for scantxoutset RPC by making them more specific with the invalid action value included.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.25-batch-488-pr-26508

📜 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 e3d61f2 and 73df875.

📒 Files selected for processing (2)
  • src/rpc/blockchain.cpp (1 hunks)
  • test/functional/rpc_scantxoutset.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

C++20 codebase should be placed under src/

Files:

  • src/rpc/blockchain.cpp
**

⚙️ 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:

  • src/rpc/blockchain.cpp
  • test/functional/rpc_scantxoutset.py
test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • test/functional/rpc_scantxoutset.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 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
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: In Dash backports from Bitcoin Core, when the DIFFICULTY_ADJUSTMENT_INTERVAL constant is missing, it should be defined as 24 for Dash (different from Bitcoin's value), as seen in the getnetworkhashps RPC backport fix.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 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.
🧬 Code graph analysis (1)
src/rpc/blockchain.cpp (2)
src/rpc/request.cpp (3)
  • JSONRPCError (56-62)
  • JSONRPCError (56-56)
  • request (31-31)
src/rpc/server.cpp (1)
  • request (552-552)
🪛 Cppcheck (2.18.0)
src/rpc/blockchain.cpp

[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.

(toomanyconfigs)

⏰ 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). (6)
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: Lint / Run linters
🔇 Additional comments (2)
src/rpc/blockchain.cpp (1)

2587-2587: LGTM! Error message improvement faithfully backported.

The error message now includes the invalid action value, improving debuggability. The terminology "action" correctly matches the parameter name defined in the RPC help text.

test/functional/rpc_scantxoutset.py (1)

131-132: LGTM! Test correctly validates the new error message format.

The test now verifies that the invalid action name is included in the error message, ensuring the improvement is properly tested.


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.

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.

2 participants