diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-12-16 17:43:15 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-12-16 17:43:20 +0100 |
commit | 69f1ee1922e31a94fba043a2278ca857e9d72c2a (patch) | |
tree | 3c94ae5c5f356197a6ec85b3d9952732cf1b8dee | |
parent | 3f205808a5f20c9398d8e74b7e83220a129acc7d (diff) | |
parent | 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 (diff) | |
download | bitcoin-69f1ee1922e31a94fba043a2278ca857e9d72c2a.tar.xz |
Merge #20365: wallettool: add parameter to create descriptors wallet
173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 test: walettool create descriptors (Ivan Metlushko)
345e88eecf1b28607d5da3af38e19794a8a115ce wallettool: add param to create descriptors wallet (Ivan Metlushko)
6d3af3ab627096a824cb6a7ca1ebeddc7530361c wallettool: pass in DatabaseOptions into MakeWallet (Ivan Metlushko)
Pull request description:
Rationale: expose and promote descriptor wallets in more places; make cli tool more consistent with `createwallet` rpc.
Add `-descriptors` parameter which is off by default. When specified it will create a new descriptors wallet with sqlite backend, which is consistent with `createwallet` rpc.
This PR is based on a suggestion from **ryanofsky** https://github.com/bitcoin/bitcoin/pull/19137#discussion_r516779603
Example:
```
$ ./src/bitcoin-wallet -wallet=fewty -descriptors create
Topping up keypool...
Wallet info
===========
Name: fewty
Format: sqlite
Descriptors: yes
Encrypted: no
HD (hd seed available): yes
Keypool Size: 6000
Transactions: 0
Address Book: 0
```
```
$ ./src/bitcoin-wallet -wallet=fewty create
Topping up keypool...
Wallet info
===========
Name: fewty
Format: bdb
Descriptors: no
Encrypted: no
HD (hd seed available): yes
Keypool Size: 2000
Transactions: 0
Address Book: 0
```
ACKs for top commit:
achow101:
ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12
ryanofsky:
Code review ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12. This seems pretty nicely implemented now, with opportunities to clean up more and dedup later
MarcoFalke:
Concept ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 ðŸŒ
Tree-SHA512: cc32ba336ff709de2707ee15f495b4617908e8700ede8401a58e894f44cda485c544d644023c9a6604d88a62db9d92152383ee2e8abf691688c25cf6e222c622
-rw-r--r-- | src/bitcoin-wallet.cpp | 1 | ||||
-rw-r--r-- | src/wallet/wallettool.cpp | 36 | ||||
-rwxr-xr-x | test/functional/tool_wallet.py | 122 |
3 files changed, 69 insertions, 90 deletions
diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index d258f9f933..68890fda2d 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -28,6 +28,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-descriptors", "Create descriptors wallet. Only for create", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("info", "Get wallet info", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index dad1232e10..fda9025588 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -21,30 +21,27 @@ static void WalletToolReleaseWallet(CWallet* wallet) delete wallet; } -static void WalletCreate(CWallet* wallet_instance) +static void WalletCreate(CWallet* wallet_instance, uint64_t wallet_creation_flags) { LOCK(wallet_instance->cs_wallet); wallet_instance->SetMinVersion(FEATURE_HD_SPLIT); + wallet_instance->AddWalletFlags(wallet_creation_flags); - // generate a new HD seed - auto spk_man = wallet_instance->GetOrCreateLegacyScriptPubKeyMan(); - CPubKey seed = spk_man->GenerateNewSeed(); - spk_man->SetHDSeed(seed); + if (!wallet_instance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + auto spk_man = wallet_instance->GetOrCreateLegacyScriptPubKeyMan(); + spk_man->SetupGeneration(false); + } else { + wallet_instance->SetupDescriptorScriptPubKeyMans(); + } tfm::format(std::cout, "Topping up keypool...\n"); wallet_instance->TopUpKeyPool(); } -static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, bool create) +static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, DatabaseOptions options) { - DatabaseOptions options; DatabaseStatus status; - if (create) { - options.require_create = true; - } else { - options.require_existing = true; - } bilingual_str error; std::unique_ptr<WalletDatabase> database = MakeDatabase(path, options, status, error); if (!database) { @@ -85,7 +82,7 @@ static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::pa } } - if (create) WalletCreate(wallet_instance.get()); + if (options.require_create) WalletCreate(wallet_instance.get(), options.create_flags); return wallet_instance; } @@ -110,14 +107,23 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) fs::path path = fs::absolute(name, GetWalletDir()); if (command == "create") { - std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ true); + DatabaseOptions options; + options.require_create = true; + if (gArgs.GetBoolArg("-descriptors", false)) { + options.create_flags |= WALLET_FLAG_DESCRIPTORS; + options.require_format = DatabaseFormat::SQLITE; + } + + std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options); if (wallet_instance) { WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); } } else if (command == "info" || command == "salvage") { if (command == "info") { - std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ false); + DatabaseOptions options; + options.require_existing = true; + std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options); if (!wallet_instance) return false; WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 615b772dc8..35576f00ea 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -28,8 +28,11 @@ class ToolWalletTest(BitcoinTestFramework): def bitcoin_wallet_process(self, *args): binary = self.config["environment"]["BUILDDIR"] + '/src/bitcoin-wallet' + self.config["environment"]["EXEEXT"] - args = ['-datadir={}'.format(self.nodes[0].datadir), '-chain=%s' % self.chain] + list(args) - return subprocess.Popen([binary] + args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + default_args = ['-datadir={}'.format(self.nodes[0].datadir), '-chain=%s' % self.chain] + if self.options.descriptors: + default_args.append('-descriptors') + + return subprocess.Popen([binary] + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) def assert_raises_tool_error(self, error, *args): p = self.bitcoin_wallet_process(*args) @@ -63,6 +66,36 @@ class ToolWalletTest(BitcoinTestFramework): result = 'unchanged' if new == old else 'increased!' self.log.debug('Wallet file timestamp {}'.format(result)) + def get_expected_info_output(self, name="", transactions=0, keypool=2, address=0): + wallet_name = self.default_wallet_name if name == "" else name + output_types = 3 # p2pkh, p2sh, segwit + if self.options.descriptors: + return textwrap.dedent('''\ + Wallet info + =========== + Name: %s + Format: sqlite + Descriptors: yes + Encrypted: no + HD (hd seed available): yes + Keypool Size: %d + Transactions: %d + Address Book: %d + ''' % (wallet_name, keypool * output_types, transactions, address)) + else: + return textwrap.dedent('''\ + Wallet info + =========== + Name: %s + Format: bdb + Descriptors: no + Encrypted: no + HD (hd seed available): yes + Keypool Size: %d + Transactions: %d + Address Book: %d + ''' % (wallet_name, keypool, transactions, address * output_types)) + def test_invalid_tool_commands_and_args(self): self.log.info('Testing that various invalid commands raise with specific error messages') self.assert_raises_tool_error('Invalid command: foo', 'foo') @@ -98,33 +131,7 @@ class ToolWalletTest(BitcoinTestFramework): # shasum_before = self.wallet_shasum() timestamp_before = self.wallet_timestamp() self.log.debug('Wallet file timestamp before calling info: {}'.format(timestamp_before)) - if self.options.descriptors: - out = textwrap.dedent('''\ - Wallet info - =========== - Name: default_wallet - Format: sqlite - Descriptors: yes - Encrypted: no - HD (hd seed available): yes - Keypool Size: 6 - Transactions: 0 - Address Book: 1 - ''') - else: - out = textwrap.dedent('''\ - Wallet info - =========== - Name: \ - - Format: bdb - Descriptors: no - Encrypted: no - HD (hd seed available): yes - Keypool Size: 2 - Transactions: 0 - Address Book: 3 - ''') + out = self.get_expected_info_output(address=1) self.assert_tool_output(out, '-wallet=' + self.default_wallet_name, 'info') timestamp_after = self.wallet_timestamp() self.log.debug('Wallet file timestamp after calling info: {}'.format(timestamp_after)) @@ -155,33 +162,7 @@ class ToolWalletTest(BitcoinTestFramework): shasum_before = self.wallet_shasum() timestamp_before = self.wallet_timestamp() self.log.debug('Wallet file timestamp before calling info: {}'.format(timestamp_before)) - if self.options.descriptors: - out = textwrap.dedent('''\ - Wallet info - =========== - Name: default_wallet - Format: sqlite - Descriptors: yes - Encrypted: no - HD (hd seed available): yes - Keypool Size: 6 - Transactions: 1 - Address Book: 1 - ''') - else: - out = textwrap.dedent('''\ - Wallet info - =========== - Name: \ - - Format: bdb - Descriptors: no - Encrypted: no - HD (hd seed available): yes - Keypool Size: 2 - Transactions: 1 - Address Book: 3 - ''') + out = self.get_expected_info_output(transactions=1, address=1) self.assert_tool_output(out, '-wallet=' + self.default_wallet_name, 'info') shasum_after = self.wallet_shasum() timestamp_after = self.wallet_timestamp() @@ -199,19 +180,7 @@ class ToolWalletTest(BitcoinTestFramework): shasum_before = self.wallet_shasum() timestamp_before = self.wallet_timestamp() self.log.debug('Wallet file timestamp before calling create: {}'.format(timestamp_before)) - out = textwrap.dedent('''\ - Topping up keypool... - Wallet info - =========== - Name: foo - Format: bdb - Descriptors: no - Encrypted: no - HD (hd seed available): yes - Keypool Size: 2000 - Transactions: 0 - Address Book: 0 - ''') + out = "Topping up keypool...\n" + self.get_expected_info_output(name="foo", keypool=2000) self.assert_tool_output(out, '-wallet=foo', 'create') shasum_after = self.wallet_shasum() timestamp_after = self.wallet_timestamp() @@ -237,9 +206,13 @@ class ToolWalletTest(BitcoinTestFramework): self.log.debug('Wallet file timestamp after calling getwalletinfo: {}'.format(timestamp_after)) assert_equal(0, out['txcount']) - assert_equal(1000, out['keypoolsize']) - assert_equal(1000, out['keypoolsize_hd_internal']) - assert_equal(True, 'hdseedid' in out) + if not self.options.descriptors: + assert_equal(1000, out['keypoolsize']) + assert_equal(1000, out['keypoolsize_hd_internal']) + assert_equal(True, 'hdseedid' in out) + else: + assert_equal(3000, out['keypoolsize']) + assert_equal(3000, out['keypoolsize_hd_internal']) self.log_wallet_timestamp_comparison(timestamp_before, timestamp_after) assert_equal(timestamp_before, timestamp_after) @@ -261,10 +234,9 @@ class ToolWalletTest(BitcoinTestFramework): # Warning: The following tests are order-dependent. self.test_tool_wallet_info() self.test_tool_wallet_info_after_transaction() + self.test_tool_wallet_create_on_existing_wallet() + self.test_getwalletinfo_on_different_wallet() if not self.options.descriptors: - # TODO: Wallet tool needs more create options at which point these can be enabled. - self.test_tool_wallet_create_on_existing_wallet() - self.test_getwalletinfo_on_different_wallet() # Salvage is a legacy wallet only thing self.test_salvage() |