Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

@DashCoreAutoGuix DashCoreAutoGuix commented Sep 30, 2025

Backport of Bitcoin PR bitcoin#26733

This is a backport of Bitcoin Core PR bitcoin#26733

Original Bitcoin Commit

  • Commit: 5394522
  • Author: Andrew Chow
  • Date: Mon May 1 09:21:28 2023 -0400

Changes

This PR adds a test for the sendmany RPC command that uses the subtractfeefrom parameter to send to multiple addresses, verifying that fees are correctly subtracted from the specified addresses.

Dash Adaptations

  • Changed "BTC" references to "DASH" in test comments
  • Maintained compatibility with Dash's test framework patterns
  • No changes to core functionality - test-only backport

Testing

  • Modified file: test/functional/wallet_basic.py
  • This test will be validated by CI functional tests

Backported-by: Claude (Agentic Backporting System)
Bitcoin-PR: bitcoin#26733
Bitcoin-Commit: 5394522

Summary by CodeRabbit

  • Tests
    • Added functional coverage for wallet sends with fees subtracted from multiple outputs, including sendmany scenarios.
    • Verifies accurate recipient amounts after fee distribution and correct sender balance adjustments.
    • Cross-checks computed received amounts against transaction fees to ensure consistency.
    • Enhances confidence in wallet fee calculations and transaction accounting through expanded test assertions.

…tractfeefrom` parameter

057057a Add test for `sendmany` rpc that uses `subtractfeefrom` parameter (Yusuf Sahin HAMZA)

Pull request description:

  This PR adds test that uses `sendmany` rpc to send **BTC** to multiple addresses using `subtractfeefrom` parameter, then checks receiver addresses balances to make sure fees are subtracted correctly.

ACKs for top commit:
  achow101:
    ACK 057057a

Tree-SHA512: 51167120d489f0ff7b8b9855424d07cb55a8965984f904643cddf45e7a08c350eaded498c350ec9c660edf72c2f128ec142347c9c79d5043d9f6cd481b15cd7e
@coderabbitai
Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Adds a new test case in wallet_basic functional tests to perform a sendmany with subtract-fee-from-outputs for two recipients, then verifies recipient received amounts, sender balance reduction, and fee distribution consistency with prior balance assertions.

Changes

Cohort / File(s) Summary of changes
Functional wallet tests
test/functional/wallet_basic.py
Added a test block for sendmany with subtract-fee-from-outputs to two new addresses; verifies recipient amounts, sender balance update, and fee allocation correctness.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the core change, namely adding a new test for the sendmany RPC using the subtractfeefrom parameter, and it aligns with the primary modification introduced by the pull request.
✨ 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-415-pr-26733

📜 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 ac6847e.

📒 Files selected for processing (1)
  • test/functional/wallet_basic.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • test/functional/wallet_basic.py
**

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

  • test/functional/wallet_basic.py
🧠 Learnings (1)
📓 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-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-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.
🧬 Code graph analysis (1)
test/functional/wallet_basic.py (2)
test/functional/test_framework/test_framework.py (1)
  • sync_all (876-878)
test/functional/test_framework/util.py (1)
  • assert_equal (69-74)
🔇 Additional comments (1)
test/functional/wallet_basic.py (1)

279-291: LGTM! Test correctly validates sendmany with subtractfeefrom for multiple recipients.

This backport faithfully adds the test from Bitcoin Core PR bitcoin#26733. The test logic is sound:

  • Sends 5 DASH to two addresses with fees subtracted from both
  • Verifies sender's balance decreases by the nominal amount (10 DASH)
  • Confirms recipient's total increase accounts for the fee
  • Validates equal fee distribution between the two recipient addresses

The Dash-specific adaptation ("5 DASH" in comment) is minimal and appropriate. The use of named parameters (dummy='', amounts=, subtractfeefrom=) improves readability compared to positional arguments used elsewhere in the file.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Ruff (0.13.1)
test/functional/wallet_basic.py

�[1;31mruff failed�[0m
�[1mCause:�[0m Failed to load configuration /ruff.toml
�[1mCause:�[0m Failed to parse /ruff.toml
�[1mCause:�[0m TOML parse error at line 26, column 3
|
26 | "RSE100", # Use of assert detected
| ^^^^^^^^
Unknown rule selector: RSE100


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.

3 participants