Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ page. See [DEVELOPMENT_CYCLE.md](DEVELOPMENT_CYCLE.md) for more details.

## [Unreleased]

- Added `wallets --delete` to remove a saved wallet configuration

## [4.0.0]

- Added persistance to existing async payjoin integration
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,12 @@ To view all saved wallet configurations:
cargo run wallets`
```

To delete a saved wallet configuration:

```shell
cargo run wallets --delete <wallet_name>
```

## Adding new features/command

This [guide](./NEW_FEATURE.md) explains how to add a new command/feature to bdk-cli's modular architecture.
6 changes: 3 additions & 3 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#[cfg(feature = "message_signer")]
use crate::handlers::offline::{SignMessageCommand, VerifyMessageCommand};
use crate::handlers::{
config::{ListWalletsCommand, SaveConfigCommand},
config::{SaveConfigCommand, WalletsCommand},
descriptor::DescriptorCommand,
key::{DeriveKeyCommand, GenerateKeyCommand, RestoreKeyCommand},
offline::{
Expand Down Expand Up @@ -141,8 +141,8 @@ pub enum CliSubCommand {
/// This feature is intended for development and testing purposes only.
Descriptor(DescriptorCommand),

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Wallets(WalletsCommand),
/// Generate tab-completion scripts for your shell.
///
/// The completion script is output on stdout, allowing you to redirect
Expand Down
62 changes: 62 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ impl WalletConfig {
.ok_or_else(|| Error::Generic(format!("Wallet {wallet_name} not found in config")))?
.try_into()
}

#[must_use]
pub fn remove_wallet(&mut self, wallet_name: &str) -> Option<WalletConfigInner> {
self.wallets.remove(wallet_name)
}
}

impl TryFrom<&WalletConfigInner> for WalletOpts {
Expand Down Expand Up @@ -346,4 +351,61 @@ mod tests {
let result: Result<WalletOpts, Error> = (&inner).try_into();
assert!(result.is_err());
}

fn test_wallet(name: &str) -> WalletConfigInner {
WalletConfigInner {
wallet: name.to_string(),
network: "testnet".to_string(),
ext_descriptor: EXT_DESCRIPTOR.to_string(),
int_descriptor: Some(INT_DESCRIPTOR.to_string()),
#[cfg(any(feature = "sqlite", feature = "redb"))]
database_type: "sqlite".to_string(),
#[cfg(any(
feature = "electrum",
feature = "esplora",
feature = "rpc",
feature = "cbf"
))]
client_type: Some("rpc".to_string()),
#[cfg(any(feature = "electrum", feature = "esplora", feature = "rpc"))]
server_url: Some("http://localhost:18443".to_string()),
#[cfg(feature = "electrum")]
batch_size: None,
#[cfg(feature = "esplora")]
parallel_requests: None,
#[cfg(feature = "rpc")]
rpc_user: None,
#[cfg(feature = "rpc")]
rpc_password: None,
#[cfg(feature = "rpc")]
cookie: None,
#[cfg(any(feature = "electrum", feature = "esplora"))]
proxy: None,
#[cfg(any(feature = "electrum", feature = "esplora"))]
proxy_auth: None,
#[cfg(any(feature = "electrum", feature = "esplora"))]
proxy_retries: None,
#[cfg(any(feature = "electrum", feature = "esplora"))]
proxy_timeout: None,
#[cfg(feature = "cbf")]
conn_count: None,
}
Comment on lines +356 to +392

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: test_wallet_config_inner_to_opts_conversion and test_invalid_client_type_fails still build WalletConfigInner by hand, repeating the same ~20 cfg-gated fields as the new helper.

Since you are adding a helper anyway, consider turning it into a small builder and reusing it there too - those two only differ in a few values. Fine to leave for a follow-up.

}
#[test]
fn test_remove_wallet_config() {
let mut config = WalletConfig {
wallets: HashMap::from([
("alice".to_string(), test_wallet("alice")),
("bob".to_string(), test_wallet("bob")),
]),
};

let removed = config.remove_wallet("alice");
assert!(removed.is_some());
assert_eq!(removed.unwrap().wallet, "alice");
assert!(!config.wallets.contains_key("alice"));
assert!(config.wallets.contains_key("bob"));

assert!(config.remove_wallet("charlie").is_none());
}
}
23 changes: 17 additions & 6 deletions src/handlers/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,27 @@ impl AppCommand<AppContext<Init>> for SaveConfigCommand {
}

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

#[arg(long = "delete", value_name = "WALLET_NAME")]
pub(crate) delete: Option<String>,
}

impl AppCommand<AppContext<Init>> for ListWalletsCommand {
impl AppCommand<AppContext<Init>> for WalletsCommand {
type Output = WalletsListResult;

fn execute(&self, ctx: &mut AppContext<Init>) -> Result<Self::Output, Error> {
let config = match WalletConfig::load(&ctx.datadir)? {
Some(cfg) => cfg,
None => return Err(Error::Generic("No wallets configured yet.".into())),
};
let mut config = WalletConfig::load(&ctx.datadir)?
.ok_or_else(|| Error::Generic("No wallets configured yet.".into()))?;

if let Some(wallet_name) = &self.delete {
if config.remove_wallet(wallet_name).is_none() {
return Err(Error::Generic(format!(
"Wallet '{wallet_name}' not found in config"
)));
}
config.save(&ctx.datadir)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: deleting the last wallet leaves config.toml with an empty wallets table, a state that was unreachable before --delete. WalletConfig::load returns None only when the file is missing, so wallets then prints {} and exits 0, while the same situation on a fresh datadir fails with "No wallets configured yet." and a non-zero exit (test_list_wallets_empty).

Probably worth folding into the wallets list / wallets delete split you are doing anyway.

}

Ok(WalletsListResult(config.wallets))
}
Expand Down
70 changes: 70 additions & 0 deletions tests/integration/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,36 @@ mod test_config {
use super::*;
use serde_json::Value;

fn save_wallet(cli: &BdkCli, wallet_name: &str) {
let desc = cli
.cmd("descriptor", &["--type", "tr"])
.output()
.expect("Command to generate descriptors failed");

let desc_values: Value =
serde_json::from_slice(&desc.stdout).expect("Invalid JSON from output descriptor");

let pub_desc = &desc_values["public_descriptors"];

cli.build_base_cmd()
.arg("wallet")
.arg("--wallet")
.arg(wallet_name)
.arg("config")
.arg("--ext-descriptor")
.arg(pub_desc["external"].as_str().unwrap())
.arg("--int-descriptor")
.arg(pub_desc["internal"].as_str().unwrap())
.arg("--client-type")
.arg("rpc")
.arg("--database-type")
.arg("sqlite")
.arg("--url")
.arg("http://localhost:18443")
.assert()
.success();
}

#[test]
fn test_save_and_read_wallet_config() {
let temp_dir = TempDir::new().unwrap();
Expand Down Expand Up @@ -291,6 +321,46 @@ mod test_config {
assert_eq!(config["ext_descriptor"].as_str().unwrap(), ext_desc);
assert_eq!(config["int_descriptor"].as_str().unwrap(), int_desc);
}

#[test]
fn test_delete_wallet_config() {
let temp_dir = TempDir::new().unwrap();
let cli = BdkCli::new("regtest", Some(temp_dir.path().to_path_buf()));
let remove_wallet_name = "test_delete_wallet";
let keep_wallet_name = "test_keep_wallet";

save_wallet(&cli, remove_wallet_name);
save_wallet(&cli, keep_wallet_name);

// Delete one config: the output is the remaining wallet map
let output = cli
.build_base_cmd()
.arg("wallets")
.arg("--delete")
.arg(remove_wallet_name)
.output()
.expect("Failed to execute wallets --delete command");
assert!(output.status.success(), "wallets --delete failed");

let list: Value = serde_json::from_slice(&output.stdout).unwrap();
assert!(list.get(remove_wallet_name).is_none());
assert!(list.get(keep_wallet_name).is_some());
}

#[test]
fn test_delete_unknown_wallet_config() {
let temp_dir = TempDir::new().unwrap();
let cli = BdkCli::new("regtest", Some(temp_dir.path().to_path_buf()));
save_wallet(&cli, "existing_wallet");

cli.build_base_cmd()
.arg("wallets")
.arg("--delete")
.arg("ghost_wallet")
.assert()
.failure()
.stderr(predicate::str::contains("not found in config"));
}
}

// SILENT PAYMENTS
Expand Down