From 6d3af3ab627096a824cb6a7ca1ebeddc7530361c Mon Sep 17 00:00:00 2001 From: Ivan Metlushko Date: Wed, 11 Nov 2020 11:40:02 +0700 Subject: [PATCH 1/3] wallettool: pass in DatabaseOptions into MakeWallet --- src/wallet/wallettool.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 0e18d6a740b..4e8bfd615cb 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -36,15 +36,9 @@ static void WalletCreate(CWallet* wallet_instance) wallet_instance->TopUpKeyPool(); } -static std::shared_ptr MakeWallet(const std::string& name, const fs::path& path, bool create) +static std::shared_ptr 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 database = MakeDatabase(path, options, status, error); if (!database) { @@ -85,7 +79,7 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa } } - if (create) WalletCreate(wallet_instance.get()); + if (options.require_create) WalletCreate(wallet_instance.get()); return wallet_instance; } @@ -110,14 +104,18 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) fs::path path = fs::absolute(name, GetWalletDir()); if (command == "create") { - std::shared_ptr wallet_instance = MakeWallet(name, path, /* create= */ true); + DatabaseOptions options; + options.require_create = true; + std::shared_ptr 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 wallet_instance = MakeWallet(name, path, /* create= */ false); + DatabaseOptions options; + options.require_existing = true; + std::shared_ptr wallet_instance = MakeWallet(name, path, options); if (!wallet_instance) return false; WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); From 345e88eecf1b28607d5da3af38e19794a8a115ce Mon Sep 17 00:00:00 2001 From: Ivan Metlushko Date: Wed, 11 Nov 2020 11:41:53 +0700 Subject: [PATCH 2/3] wallettool: add param to create descriptors wallet --- src/bitcoin-wallet.cpp | 1 + src/wallet/wallettool.cpp | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index b9c2fe2d345..6671838104e 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -27,6 +27,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-wallet=", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-debug=", "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 4e8bfd615cb..538389e4c03 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -21,16 +21,19 @@ 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(); @@ -79,7 +82,7 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa } } - if (options.require_create) WalletCreate(wallet_instance.get()); + if (options.require_create) WalletCreate(wallet_instance.get(), options.create_flags); return wallet_instance; } @@ -106,6 +109,11 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) if (command == "create") { 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 wallet_instance = MakeWallet(name, path, options); if (wallet_instance) { WalletShowInfo(wallet_instance.get()); From 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 Mon Sep 17 00:00:00 2001 From: Ivan Metlushko Date: Fri, 13 Nov 2020 17:52:28 +0700 Subject: [PATCH 3/3] test: walettool create descriptors --- test/functional/tool_wallet.py | 122 +++++++++++++-------------------- 1 file changed, 47 insertions(+), 75 deletions(-) diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 615b772dc8b..35576f00ea3 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()