Skip to content

fix:master_wallet can't be deleted or duplicated#74

Merged
Keshoid merged 3 commits intorelease/nodectl/v0.4.0from
feature/sma-48-protect-master-wallet-from-conflicts
Apr 9, 2026
Merged

fix:master_wallet can't be deleted or duplicated#74
Keshoid merged 3 commits intorelease/nodectl/v0.4.0from
feature/sma-48-protect-master-wallet-from-conflicts

Conversation

@mrnkslv
Copy link
Copy Markdown

@mrnkslv mrnkslv commented Apr 9, 2026

Summary

Reserve the logical wallet name master_wallet in nodectl: operators cannot config wallet add a regular wallet with that name (it remains the master’s slot in config wallet ls), and config wallet rm fails immediately with a clear error when that name is used—without loading the config or suggesting “not found”. The name is defined once as MASTER_WALLET_RESERVED_NAME in utils.rs and reused by get_wallet_config, WalletAddCmd, WalletLsCmd, and WalletRmCmd so resolution and CLI checks stay aligned.

Changes

  • utils.rs
    Add public MASTER_WALLET_RESERVED_NAME ("master_wallet") with a short comment: CLI / get_wallet_config / config wallet ls.
    get_wallet_config: resolve the master wallet when name == MASTER_WALLET_RESERVED_NAME instead of a hardcoded literal.
  • config_wallet_cmd.rs
    Import MASTER_WALLET_RESERVED_NAME from utils (no duplicate local constant).
    WalletAddCmd: reject --name equal to MASTER_WALLET_RESERVED_NAME; error text uses the constant (e.g. '{}' is a reserved name).
    WalletLsCmd: append the master row with MASTER_WALLET_RESERVED_NAME.
    WalletRmCmd: before AppConfig::load, if the target equals MASTER_WALLET_RESERVED_NAME, bail with The master wallet cannot be removed (no config mutation).

Closes SMA-48

Copilot AI review requested due to automatic review settings April 9, 2026 05:59
@linear
Copy link
Copy Markdown

linear bot commented Apr 9, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where master_wallet could be deleted or duplicated by reserving it as a special wallet name. The fix centralizes the wallet name string in a constant MASTER_WALLET_RESERVED_NAME and uses it consistently across add, remove, and list operations. The remove operation also adds an early error check to prevent deletion of the master wallet with a clear error message.

Changes:

  • Introduce MASTER_WALLET_RESERVED_NAME constant to define the reserved wallet name once
  • Update WalletAddCmd to check against the constant instead of a hardcoded string
  • Update WalletLsCmd to use the constant for displaying the master wallet
  • Add early validation in WalletRmCmd to prevent removal of the master wallet

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +44 to +45
/// Name shown for `master_wallet` in `config wallet ls` and accepted by `get_wallet_config`.
const MASTER_WALLET_RESERVED_NAME: &str = "master_wallet";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The constant MASTER_WALLET_RESERVED_NAME is defined with the note that it's accepted by get_wallet_config, but the function in utils.rs (line 139) still uses a hardcoded string literal "master_wallet" instead of this constant. For consistency and maintainability as stated in the PR description ("The string is defined once as MASTER_WALLET_RESERVED_NAME and reused for add, rm, and ls so the behavior stays aligned"), this should be updated to use the constant.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

@mrnkslv mrnkslv requested review from ITBear, Keshoid and Lapo4kaKek April 9, 2026 06:25
@Keshoid Keshoid merged commit 64820fd into release/nodectl/v0.4.0 Apr 9, 2026
5 checks passed
@Keshoid Keshoid deleted the feature/sma-48-protect-master-wallet-from-conflicts branch April 9, 2026 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants