[Jennycj]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.83%. Comparing base (4445a26) to head (bc050c7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #350      +/-   ##
==========================================
+ Coverage   58.74%   58.83%   +0.08%     
==========================================
  Files          25       25              
  Lines        4608     4618      +10     
==========================================
+ Hits         2707     2717      +10     
  Misses       1901     1901              

Flag	Coverage Δ
rust	58.83% <100.00%> (+0.08%)	⬆️

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.

[vadim-anfv]

Thanks for picking this up, @Jennycj.
I left one note inline on the new test. Happy to test once it is rebased.

[vadim-anfv]

I am not sure a test for an absent field is needed at all: the field is gone from WalletConfigInner, so serde cannot emit it either way, and the type already guarantees that.

If you would rather keep the check, maybe move it into test_list_wallets_with_entries just above, which already does the same setup.

[Jennycj]

Thanks for the review @vadim-anfv . I agree, and have removed the test.

[tvpeter]

Thank you @Jennycj for working on this.

A few quick things before we merge:

• Please rebase as #312 introduced updates to the wallets command.
• Address Vadim's feedback regarding the test comment.
• Squash your commits to a single commit.

[tvpeter]

I'd prefer to_wallet_opts name as the method does not implement TryFrom trait and there is no infallible to_wallet_opts here

[Jennycj]

Thanks for pointing this out. I have renamed to to_wallet_opts, and also dropped the leftover let config = self; binding.

[Jennycj]

Thank you @Jennycj for working on this.

A few quick things before we merge:

• Please rebase as Add command to delete saved wallet config #312 introduced updates to the wallets command.
• Address Vadim's feedback regarding the test comment.
• Squash your commits to a single commit.

I have now rebased on master, addressed both review comments, and squashed my commits into a single signed commit.