Skip to content

refactor(config): remove redundant wallet field from WalletConfigInner - #350

Open
Jennycj wants to merge 2 commits into
bitcoindevkit:masterfrom
Jennycj:refactor/remove-wallet-config-field
Open

Jennycj wants to merge 2 commits into
bitcoindevkit:masterfrom
Jennycj:refactor/remove-wallet-config-field

Conversation

@Jennycj

@Jennycj Jennycj commented Oct 12, 2026

Copy link
Copy Markdown

Description

Removes the redundant wallet field from WalletConfigInner. The wallet name was being stored twice: once as the key in WalletConfig::wallets, and once inside each entry as wallet: String. The wallets command's JSON/TOML output showed both.
For example:

{
"my_wallet": {
"wallet": "my_wallet",
"network": "testnet",
...
}
}

The "wallet" field inside the entry is gone and the name is still available as the map key.

Fixes #332

Notes to the reviewers

  • TryFrom<&WalletConfigInner> for WalletOpts needed to be removed because it only receives the struct. It is now replaced with a private try_to_wallet_opts(&self, wallet_name: &str) that takes the name from the caller instead.

  • Backward compatible: old config.toml files with the stray wallet = "..." line still load (serde ignores unknown fields). This is covered bytest_load_tolerates_legacy_wallet_field.

  • CHANGELOG updated under [Unreleased].

Checklists

All Submissions:

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

`WalletConfigInner` duplicated its own wallet name in a `wallet` field,
even though it's already the key in `WalletConfig::wallets` - this
showed up as a redundant `"wallet"` entry in `wallets` output.

`TryFrom<&WalletConfigInner> for WalletOpts` relied on that field, so
it's replaced with a private `try_to_wallet_opts(&self, wallet_name: &str)`
that takes the name from the caller's map lookup instead. Old config.toml
files still load fine; serde ignores the now-unknown `wallet` key.

Fixes bitcoindevkit#332
@codecov

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.54%. Comparing base (eadbdc7) to head (553a3eb).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #350      +/-   ##
==========================================
+ Coverage   61.43%   61.54%   +0.11%     
==========================================
  Files          23       23              
  Lines        3993     4005      +12     
==========================================
+ Hits         2453     2465      +12     
  Misses       1540     1540              
Flag Coverage Δ
rust 61.54% <100.00%> (+0.11%) ⬆️

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Remove "wallet" field from WalletConfigInner

2 participants