Skip to content

fix(create_tx): enforce single recipient for send_all#291

Merged
tvpeter merged 1 commit into
bitcoindevkit:masterfrom
1estart:fix/send-all-recipient-validation
Jun 29, 2026
Merged

fix(create_tx): enforce single recipient for send_all#291
tvpeter merged 1 commit into
bitcoindevkit:masterfrom
1estart:fix/send-all-recipient-validation

Conversation

@1estart

@1estart 1estart commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Closes #290

Description

Previously, passing multiple --to arguments with --send_all would silently drain the wallet to only the first recipient, ignoring the rest. This was inconsistent with the silent-payments create_tx, which already validates the recipient count.

Now returns an error unless exactly one recipient is provided.

Notes to the reviewers

  • The fix mirrors the validation logic already present in the silent-payments create_tx branch of the same file, so behavior is now consistent between the two code paths.

Changelog notice

  • Fixed create_tx --send_all to reject multiple recipients instead of silently using only the first one

Checklists

All Submissions:

  • I've signed all my commits

  • I followed the contribution guidelines

  • I ran cargo fmt and cargo clippy before committing

  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@va-an va-an left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The handlers.rs fix looks good.

As discussed in #225 (comment) we're moving away from this style of testing (driving the binary as a subprocess), so I'd suggest not adding tests in this PR and dropping the new test file. We can add proper coverage as a follow up after #278 and #289 are merged.

Previously, passing multiple --to arguments with --send_all would
silently drain the wallet to only the first recipient, ignoring the
rest. This was inconsistent with the silent-payments create_tx, which
already validates the recipient count.

Now returns an error unless exactly one recipient is provided
@1estart 1estart force-pushed the fix/send-all-recipient-validation branch from 00d042e to 50d248c Compare June 26, 2026 05:19
@1estart

1estart commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

The handlers.rs fix looks good.

As discussed in #225 (comment) we're moving away from this style of testing (driving the binary as a subprocess), so I'd suggest not adding tests in this PR and dropping the new test file. We can add proper coverage as a follow up after #278 and #289 are merged.

Understood. I've kept the changes minimal and dropped the test file as suggested. I can add the tests later once the related PRs are merged.

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 10.21%. Comparing base (c579854) to head (50d248c).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
src/handlers.rs 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
- Coverage   10.22%   10.21%   -0.02%     
==========================================
  Files           8        8              
  Lines        2709     2713       +4     
==========================================
  Hits          277      277              
- Misses       2432     2436       +4     
Flag Coverage Δ
rust 10.21% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tvpeter tvpeter added this to the CLI 4.0.0 milestone Jun 29, 2026
@tvpeter tvpeter added the good first issue Good for newcomers label Jun 29, 2026
@tvpeter

tvpeter commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

ACK 50d248c

@va-an va-an left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK 50d248c

@tvpeter tvpeter merged commit 0a771ca into bitcoindevkit:master Jun 29, 2026
7 of 9 checks passed
@github-project-automation github-project-automation Bot moved this to Done in BDK-CLI Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issue Good for newcomers

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

create_tx --send_all silently ignores all recipients except the first

3 participants