Skip to content

feat(wallet): add delete_wallet RPC#2497

Merged
shamardy merged 9 commits intostagingfrom
feat/delete-wallet
Jun 24, 2025
Merged

feat(wallet): add delete_wallet RPC#2497
shamardy merged 9 commits intostagingfrom
feat/delete-wallet

Conversation

@shamardy
Copy link
Copy Markdown
Collaborator

@shamardy shamardy commented Jun 20, 2025

This PR introduces the delete_wallet RPC endpoint, allowing for the secure deletion of wallets with password confirmation.

To Test:

  • Create two wallets, for example, wallet-to-delete (password: pass1) and active-wallet (password: pass2).
  • Log into active-wallet to make it the active session.
  • Verify wallets exist by calling get_wallet_names.

Attempt to delete the active wallet (should fail):

{
  "userpass": "your_rpc_password",
  "method": "delete_wallet",
  "mmrpc": "2.0",
  "params": {
    "wallet_name": "active-wallet",
    "password": "pass2"
  }
}

Expected Error: Cannot delete wallet 'active-wallet' as it is currently active.

Attempt to delete with an incorrect password (should fail):

{
  "userpass": "your_rpc_password",
  "method": "delete_wallet",
  "mmrpc": "2.0",
  "params": {
    "wallet_name": "wallet-to-delete",
    "password": "wrong_password"
  }
}

Expected Error: Invalid password

Attempt to delete a non-existent wallet (should fail):

{
  "userpass": "your_rpc_password",
  "method": "delete_wallet",
  "mmrpc": "2.0",
  "params": {
    "wallet_name": "non-existent-wallet",
    "password": "any_password"
  }
}

Expected Error: Wallet 'non-existent-wallet' not found.

Successfully delete an inactive wallet

{
  "userpass": "your_rpc_password",
  "method": "delete_wallet",
  "mmrpc": "2.0",
  "params": {
    "wallet_name": "wallet-to-delete",
    "password": "pass1"
  }
}

Expected Response: A successful result ({"result":null}).

Verify deletion:

Call get_wallet_names again. The response should now only list active-wallet.

Test deletion in no-login mode:

Create wallets again and test deleting them in no-login mode when there is no active one, this mimics what will happen in GUIs.

shamardy added 2 commits June 20, 2025 11:01
This commit introduces the delete_wallet RPC endpoint, which allows users to delete a specified wallet after password confirmation.

It also refactors the passphrase loading mechanism for better clarity and explicit behavior. The function read_and_decrypt_passphrase_if_available is now try_load_wallet_passphrase and requires a wallet_name, decoupling storage access from the active wallet context. A new helper, try_load_active_wallet_passphrase, is introduced to handle operations on the current wallet.
This commit introduces comprehensive integration tests for the `delete_wallet` RPC endpoint to ensure its functionality is robust and correct.

Tests have been implemented for both native and WASM environments, covering the following scenarios:
- Successful deletion of an inactive wallet.
- Failure when attempting to delete the currently active wallet.
- Failure when providing an incorrect password.
- Failure when targeting a non-existent wallet.

Additionally, a new test helper function, `delete_wallet`, has been added to `mm2_test_helpers` to streamline the process of calling the RPC endpoint within the tests.
…r handling

The `test_change_mnemonic_password_rpc` integration test was failing due to an incorrect status code assertion. The test previously expected a `500 INTERNAL_SERVER_ERROR` when an invalid `current_password` was supplied.

This failure was introduced by a recent refactoring that improved error propagation for passphrase handling. The system now correctly identifies an invalid password during mnemonic decryption and returns a `MnemonicRpcError::InvalidPassword`, which corresponds to a `400 BAD_REQUEST` HTTP status.

This commit updates the test to assert for `StatusCode::BAD_REQUEST`, ensuring it aligns with the corrected and more precise error-handling behavior of the RPC endpoint.
@shamardy shamardy changed the title feat(wallet): Add delete_wallet RPC feat(wallet): add delete_wallet RPC Jun 20, 2025
shamardy added 3 commits June 20, 2025 13:40
The `get_wallet_names` RPC guarantees an alphabetically sorted list. This commit updates the test assertion to match the expected order, fixing the failure.
The test_delete_wallet_rpc was failing on the wasm32 target due to an incorrect assertion on the HTTP status code. The test expected a 400 BAD_REQUEST, but the wasm test harness does not reliably propagate specific HTTP status codes for RPC errors.

This commit modifies the test to no longer check the StatusCode. Instead, it now deserializes the JSON error response and asserts directly on the error_type and error_data fields. This approach provides more precise validation of the error condition and makes the test more resilient to the specifics of the wasm RPC transport layer.
@shamardy shamardy force-pushed the feat/delete-wallet branch from b8bf52c to 319fbf6 Compare June 21, 2025 21:43
@shamardy shamardy added status: pending review priority: urgent Critical tasks requiring immediate action. 2.5.0-beta and removed status: in progress labels Jun 23, 2025
@shamardy shamardy requested review from CharlVS and mariocynicys June 23, 2025 03:50
No need to continue support for QRC20 anymore
@smk762
Copy link
Copy Markdown

smk762 commented Jun 23, 2025

Docs PR at GLEECBTC/komodo-docs-mdx#529

Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks!
got only a single Q regarding the no-login mode (and could such mode be tested).

p.s. no idea how the no login mode works xD

A seed node (i_am_seed: true) requires a persistent private key to establish its P2P identity. Previously, a seed node could start without a passphrase, leading to a bad state.

This commit adds a pre-check to ensure CryptoCtx is initialized for seed nodes, causing startup to fail with a clear error if no wallet identity is present.

Additionally, this commit includes:

- A new test (test_delete_wallet_in_no_login_mode) to verify wallet deletion works correctly in no login mdoe
- A safeguard in the no_login_node test helper to enforce valid test configurations.
@smk762 smk762 self-requested a review June 24, 2025 05:38
smk762
smk762 previously approved these changes Jun 24, 2025
Copy link
Copy Markdown

@smk762 smk762 left a comment

Choose a reason for hiding this comment

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

LGTM. Docs PR approved, will be merged later this week.
Tests passed:

  • attempt to delete non-existent wallet
  • attempt to delete active wallet
  • attempt to delete with wrong password
  • attempt to delete non-active wallet with correct password.

All responded as expected.

Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

Copy link
Copy Markdown

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM

@shamardy shamardy merged commit d62c20b into staging Jun 24, 2025
19 of 24 checks passed
@shamardy shamardy deleted the feat/delete-wallet branch June 24, 2025 09:11
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Jun 27, 2025
* dev: (30 commits)
  chore(core): replace hash_raw_entry with stable entry() API (GLEECBTC#2473)
  chore(core): adapt `MmError` and usages for compatibility with new rustc versions (GLEECBTC#2443)
  feat(wallet): add `delete_wallet` RPC (GLEECBTC#2497)
  chore(release): add changelog entries for v2.5.0-beta (GLEECBTC#2494)
  chore(release): bump kdf version to 2.5.0-beta (GLEECBTC#2492)
  feat(tests): zcoin unit test to validate dex fee (GLEECBTC#2460)
  fix(zcoin): correctly track unconfirmed z-coin notes (GLEECBTC#2331)
  improvement(orders): remove BTC specific volume from min_trading_vol logic (GLEECBTC#2483)
  feat(ibc-routing-part-2): supporting entire Cosmos network for swaps (GLEECBTC#2476)
  fix(startup): don't initialize WalletConnect during startup (GLEECBTC#2485)
  fix(dns): better ip resolution (GLEECBTC#2487)
  improvement(event-streaming): strong type streamer IDs (GLEECBTC#2441)
  bump timed-map to `1.4.1` (GLEECBTC#2481)
  improvement(RPC): unified interface for legacy and current RPC interfaces (GLEECBTC#2450)
  improvement(tendermint): `tendermint_tx_internal_id` helper (GLEECBTC#2438)
  feat(walletconnect): add WalletConnect v2 support for EVM and Cosmos (GLEECBTC#2223)
  feat(ibc-routing-part-1): supporting entire Cosmos network for swaps (GLEECBTC#2459)
  fix(test): fix HD Wallet message signing tests (GLEECBTC#2474)
  improvement(builds): enable static CRT linking for MSVC builds (GLEECBTC#2464)
  feat(wallet): implement HD multi-address support for message signing (GLEECBTC#2432)
  ...

# Conflicts:
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
dimxy pushed a commit that referenced this pull request Jun 27, 2025
* dev:
  chore(core): replace hash_raw_entry with stable entry() API (#2473)
  chore(core): adapt `MmError` and usages for compatibility with new rustc versions (#2443)
  feat(wallet): add `delete_wallet` RPC (#2497)
  chore(release): add changelog entries for v2.5.0-beta (#2494)
  chore(release): bump kdf version to 2.5.0-beta (#2492)
  feat(tests): zcoin unit test to validate dex fee (#2460)

# Conflicts:
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Jul 16, 2025
* refactor-gas-fee-policy: (30 commits)
  revert change in test_enable_custom_erc20
  fix(test) disable get_swap_gas_fee_policy for tests (no platform coin in tests)
  revert tokio test and use block_on for eth docker test
  Refactor gas fee code: get max_eth_tx_type from platform coin, add gas fee adjust coin params, rename "set_swap_transaction_fee_policy" rpc to "set_swap_gas_fee_policy", force set_swap_gas_fee_policy to store policy in the platform coin
  fix(eth): Propagate structured EIP-1559 fee errors (GLEECBTC#2532)
  fix(eth): Correctly implement ETH max withdrawal logic (GLEECBTC#2531)
  feat(use-clap-for-cli): use clap to parse CLI-Args GLEECBTC#2215 (GLEECBTC#2510)
  feat(orderbook): expirable maker orders (GLEECBTC#2516)
  improvement(eth): drop parity support (GLEECBTC#2527)
  chore(release): finalize changelog for v2.5.0-beta (GLEECBTC#2524)
  chore(toolchain): upgrade toolchain to nightly 1.86.0 (GLEECBTC#2444)
  feat(swap): rpc to find best swap with liquidity routing for ask (GLEECBTC#2362)
  fix(kdf_walletconnect): apply explicit MmError mapping (GLEECBTC#2514)
  fix(walletconnect): centralize connection and retry logic (GLEECBTC#2508)
  fix(hw-wallet): avoid calling `get_enabled_address` in trezor-based coin init (GLEECBTC#2504)
  chore(core): replace hash_raw_entry with stable entry() API (GLEECBTC#2473)
  chore(core): adapt `MmError` and usages for compatibility with new rustc versions (GLEECBTC#2443)
  feat(wallet): add `delete_wallet` RPC (GLEECBTC#2497)
  chore(release): add changelog entries for v2.5.0-beta (GLEECBTC#2494)
  chore(release): bump kdf version to 2.5.0-beta (GLEECBTC#2492)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_utils.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/qrc20.rs
#	mm2src/coins/qrc20/swap.rs
#	mm2src/coins_activation/src/platform_coin_with_tokens.rs
#	mm2src/coins_activation/src/token.rs
#	mm2src/mm2_main/src/lp_swap/check_balance.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/ext_api/ext_api_types.rs
#	mm2src/mm2_main/src/rpc/lp_commands/mod.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/errors.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
#	mm2src/trading_api/src/one_inch_api/classic_swap_types.rs
#	mm2src/trading_api/src/one_inch_api/client.rs
#	mm2src/trading_api/src/one_inch_api/portfolio_types.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: urgent Critical tasks requiring immediate action. status: pending review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants