Add command to delete saved wallet config by yan-pi · Pull Request #312 · bitcoindevkit/bdk-cli

yan-pi · 2026-08-06T17:21:42Z

Description

Adds wallets delete <wallet_name> to remove a saved wallet configuration.
Saved configurations can be listed with wallets list.

Resolves #310.

Notes to the reviewers

List and delete use separate handlers and outputs.

Delete only removes the entry from config.toml.
Wallet database files arenot removed.

Changelog notice

Added wallets delete to remove a saved wallet configuration

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature
I've updated CHANGELOG.md

tvpeter · 2026-08-07T08:59:08Z

Only the config.toml entry is affected, wallet data on disk is not touched (dont know if its the correct path).

So wallet config only writes config.toml. The DB file is created lazily during an actual wallet operation. So deleting a config that was never used leaves nothing on disk. However, if the wallet was used, the DB file remains.

tvpeter

I agree with the approach of the delete operation but the returned type of WalletList is unnecesary. If returning a message is not possible, then it is better to split the wallets into something like wallets list and wallets --delete <name>. So WalletsSubcommand will be the top-level.

tvpeter · 2026-08-07T08:31:09Z

 #[derive(Args, Debug, Clone, PartialEq)]
-pub struct ListWalletsCommand;
+pub struct WalletsCommand {
+    /// Delete the saved configuration for the given wallet instead of listing.


Suggested change

/// Delete the saved configuration for the given wallet instead of listing.

/// Delete the saved configuration for the given wallet.

tvpeter · 2026-08-07T08:53:07Z


-    /// List all saved wallet configurations.
-    Wallets(ListWalletsCommand),
+    /// List all saved wallet configurations, or delete one with `--delete`.


Suggested change

/// List all saved wallet configurations, or delete one with `--delete`.

/// Saved wallet configuration operations.

vadim-anfv

cACK e987877

Thanks for picking this up. I left a couple of non-blocking comments inline, both of which fold naturally into the wallets list / wallets delete restructure @tvpeter asked for.

yan-pi · 2026-08-09T19:48:40Z

@tvpeter, that makes sense. Splitting the operations would also fix the return type issue.

Just to confirm, should this become wallets list and wallets delete <name>, as @vadim-anfv suggested?

tvpeter · 2026-08-10T09:23:04Z

@tvpeter, that makes sense. Splitting the operations would also fix the return type issue.

Just to confirm, should this become wallets list and wallets delete <name>, as @vadim-anfv suggested?

Yes. We both said the same thing.

yan-pi · 2026-08-18T14:20:02Z

Thanks for the first review!
I forgot to request another round of review. Now might be ready for it.

vadim-anfv

tACK f2a7e8f

Both of my earlier comments are addressed. I can't resolve the threads myself - could you close them?

Added two non-blocking notes inline.

vadim-anfv · 2026-08-20T18:58:51Z


 ## [Unreleased]

+- Split saved wallet configuration operations into `wallets list` and `wallets delete <wallet_name>`


This entry and the "Changelog notice" in the PR description are out of sync - could you align them?

While you're here: plain bdk-cli wallets no longer works, and imo two entries would sit closer to https://keepachangelog.com/en/1.0.0/ and make that clearer. Up to you though:

Suggested change

- Split saved wallet configuration operations into `wallets list` and `wallets delete <wallet_name>`

- Replaced plain `wallets` with `wallets list`

- Added `wallets delete <wallet_name>` to remove a saved wallet configuration

vadim-anfv · 2026-08-20T20:01:06Z

+    /// List saved wallet configurations.
+    List(ListWalletsCommand),
+
+    /// Delete a saved wallet configuration.


It would be great to have what you wrote in the PR description in wallets delete --help as well:

Suggested change

/// Delete a saved wallet configuration.

/// Delete a saved wallet configuration.

///

/// Only the entry in `config.toml` is removed. Wallet data on disk is left in place.

codecov · 2026-08-24T09:12:35Z

Codecov Report

❌ Patch coverage is 84.37500% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.66%. Comparing base (e7223e5) to head (f2a7e8f).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/handlers/config.rs	83.33%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
+ Coverage   57.45%   57.66%   +0.20%     
==========================================
  Files          22       22              
  Lines        3695     3725      +30     
==========================================
+ Hits         2123     2148      +25     
- Misses       1572     1577       +5

Flag	Coverage Δ
rust	`57.66% <84.37%> (+0.20%)`	⬆️

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

Thank you @yan-pi for working on this issue.

So having reconsidered the approach I suggested earlier, I think it may not be a good idea to delete saved configurations that have wallet data already persisted. So it will be better to handle it in such a way that, if the wallet data exists, return an error that wallet data exist for that configuration and do not delete the config. While it is possible to recreate the same config, the user might have lost the descriptors to do so. Also, if at all further down the line, we decide to consider deleting, it should also wipe the db file (for sqlite and much harder to even consider for redb) with a --force flag.

Aside that, kindly address the minor failing checks and review earlier by Vadim.

Thank you

github-project-automation Bot added this to BDK-CLI Aug 6, 2026

yan-pi force-pushed the feat/wallets-delete branch from 9c25d3d to e987877 Compare August 6, 2026 18:06

yan-pi changed the title ~~feat(wallet): add wallets --delete to remove saved wallet config~~ Add wallets --delete to remove saved wallet config Aug 6, 2026

tvpeter assigned yan-pi Aug 6, 2026

tvpeter requested changes Aug 7, 2026

View reviewed changes

vadim-anfv reviewed Aug 7, 2026

View reviewed changes

Comment thread src/handlers/config.rs

Comment thread src/config.rs Outdated

yan-pi force-pushed the feat/wallets-delete branch from e987877 to 011390e Compare August 11, 2026 17:23

yan-pi changed the title ~~Add wallets --delete to remove saved wallet config~~ Add command to delete saved wallet config Aug 11, 2026

yan-pi added 2 commits August 11, 2026 16:08

feat(wallet): add saved config list and delete commands

fcd2a19

docs: document saved wallet config commands

f2a7e8f

yan-pi force-pushed the feat/wallets-delete branch from 011390e to f2a7e8f Compare August 11, 2026 19:47

yan-pi requested review from tvpeter and vadim-anfv August 18, 2026 14:16

vadim-anfv approved these changes Aug 20, 2026

View reviewed changes

vadim-anfv mentioned this pull request Aug 21, 2026

feat: add delete subcommand to remove wallet config file #316

Closed

18 tasks

tvpeter requested changes Aug 25, 2026

View reviewed changes

yan-pi mentioned this pull request Aug 27, 2026

CI validation: PR 312 review changes yan-pi/bdk-cli#1

Draft

	/// Delete the saved configuration for the given wallet instead of listing.
	/// Delete the saved configuration for the given wallet.

	/// List all saved wallet configurations, or delete one with `--delete`.
	/// Saved wallet configuration operations.


		## [Unreleased]

		- Split saved wallet configuration operations into `wallets list` and `wallets delete <wallet_name>`

	- Split saved wallet configuration operations into `wallets list` and `wallets delete <wallet_name>`
	- Replaced plain `wallets` with `wallets list`
	- Added `wallets delete <wallet_name>` to remove a saved wallet configuration

Uh oh!

Conversation

yan-pi commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Uh oh!

tvpeter commented Aug 7, 2026

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

tvpeter Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

tvpeter Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

vadim-anfv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yan-pi commented Aug 9, 2026

Uh oh!

tvpeter commented Aug 10, 2026

Uh oh!

yan-pi commented Aug 18, 2026

Uh oh!

vadim-anfv left a comment

Choose a reason for hiding this comment

Uh oh!

vadim-anfv Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

vadim-anfv Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 24, 2026

Codecov Report

Uh oh!

tvpeter left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yan-pi commented Aug 6, 2026 •

edited

Loading