dashpay / dash

Dash - Reinventing Cryptocurrency
https://www.dash.org
MIT License
1.49k stars 1.2k forks source link

fix: createwallet to require 'load_on_startup' for descriptor wallets #6095

Closed knst closed 2 months ago

knst commented 3 months ago

Issue being fixed or feature implemented

RPC createwallet has changed list of arguments: createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup ) since https://github.com/dashpay/dash/pull/5965 load_on_startup used to be an argument 5 but now it has a number 6. Both arguments 5 and 6 are boolean and it can confuse an user: they may even not notice that something wrong when it meant to be "load on startup" but got "descriptor wallet" which is not expected.

See also previous attempt to resolve issue: https://github.com/dashpay/dash/pull/6029

What was done?

To prevent confusion if user is not aware about this breaking changes, the RPC createwallet throws an exception if user trying to create descriptor wallet but has not mentioned load_on_startup. This requirement can be removed when major amount of users updated to v21.

How Has This Been Tested?

Run unit/functional tests

Tested with CLI:

$ createwallet "tmp-create" true true "" false true
RPC createwallet would not accept creating descriptor wallets without specifying 'load_on_startup' flag. It is required explicitly in v21 due to breaking changes in createwallet RPC (code -8)
$ createwallet "tmp-create" true true "" false true true
{
  "name": "tmp-create",
  "warning": "Empty string given as passphrase, wallet will not be encrypted.\nWallet is an experimental descriptor wallet"
}

Breaking Changes

You can't more pass 'descriptor=NN' without https://github.com/dashpay/dash/pull/5965 which has not been released yet.

Checklist:

PastaPastaPasta commented 2 months ago

back ported in #6115