wallet: ExternalSigner: add GetDescriptors method

Sjors · Sjors · commit 7ebc7c021597 · 2021-02-23T14:34:31.000+01:00
diff --git a/src/wallet/external_signer.cpp b/src/wallet/external_signer.cpp
@@ -55,4 +55,9 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS
     return true;
 }
 
+UniValue ExternalSigner::GetDescriptors(int account)
+{
+    return RunCommandParseJSON(m_command + " --fingerprint \"" + m_fingerprint + "\"" + NetworkArg() + " getdescriptors --account " + strprintf("%d", account));
+}
+
 #endif
diff --git a/src/wallet/external_signer.h b/src/wallet/external_signer.h
@@ -49,6 +49,12 @@ class ExternalSigner
     //! @param[out] success     Boolean
     static bool Enumerate(const std::string& command, std::vector<ExternalSigner>& signers, std::string chain, bool ignore_errors = false);
 
+    //! Get receive and change Descriptor(s) from device for a given account.
+    //! Calls `<command> getdescriptors --account <account>`
+    //! @param[in] account  which BIP32 account to use (e.g. `m/44'/0'/account'`)
+    //! @param[out] UniValue see doc/external-signer.md
+    UniValue GetDescriptors(int account);
+
 #endif
 };
 
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
@@ -22,6 +22,9 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
   * Returns false if already setup or setup fails, true if setup is successful
   */
   bool SetupDescriptor(std::unique_ptr<Descriptor>desc);
+
+  static ExternalSigner GetExternalSigner();
+
 };
 #endif
 
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -582,6 +582,11 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     //! Setup descriptors based on the given CExtkey
     bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type);
 
+    /** Provide a descriptor at setup time
+    * Returns false if already setup or setup fails, true if setup is successful
+    */
+    bool SetupDescriptor(std::unique_ptr<Descriptor>desc);
+
     bool HavePrivateKeys() const override;
 
     int64_t GetOldestKeyPoolTime() const override;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -14,6 +14,7 @@
 #include <key.h>
 #include <key_io.h>
 #include <optional.h>
+#include <outputtype.h>
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <primitives/block.h>
@@ -3864,7 +3865,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st
             walletInstance->SetupLegacyScriptPubKeyMan();
         }
 
-        if (!(wallet_creation_flags & (WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET))) {
+        if ((wallet_creation_flags & WALLET_FLAG_EXTERNAL_SIGNER) || !(wallet_creation_flags & (WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET))) {
             LOCK(walletInstance->cs_wallet);
             if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
                 walletInstance->SetupDescriptorScriptPubKeyMans();
@@ -4488,32 +4489,65 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
 {
     AssertLockHeld(cs_wallet);
 
-    // Make a seed
-    CKey seed_key;
-    seed_key.MakeNewKey(true);
-    CPubKey seed = seed_key.GetPubKey();
-    assert(seed_key.VerifyPubKey(seed));
+    if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) {
+        // Make a seed
+        CKey seed_key;
+        seed_key.MakeNewKey(true);
+        CPubKey seed = seed_key.GetPubKey();
+        assert(seed_key.VerifyPubKey(seed));
 
-    // Get the extended key
-    CExtKey master_key;
-    master_key.SetSeed(seed_key.begin(), seed_key.size());
+        // Get the extended key
+        CExtKey master_key;
+        master_key.SetSeed(seed_key.begin(), seed_key.size());
 
-    for (bool internal : {false, true}) {
-        for (OutputType t : OUTPUT_TYPES) {
-            auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, internal));
-            if (IsCrypted()) {
-                if (IsLocked()) {
-                    throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors");
+        for (bool internal : {false, true}) {
+            for (OutputType t : OUTPUT_TYPES) {
+                auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, internal));
+                if (IsCrypted()) {
+                    if (IsLocked()) {
+                        throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors");
+                    }
+                    if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) {
+                        throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors");
+                    }
                 }
-                if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) {
-                    throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors");
+                spk_manager->SetupDescriptorGeneration(master_key, t);
+                uint256 id = spk_manager->GetID();
+                m_spk_managers[id] = std::move(spk_manager);
+                AddActiveScriptPubKeyMan(id, t, internal);
+            }
+        }
+    } else {
+#ifdef ENABLE_EXTERNAL_SIGNER
+        ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
+
+        // TODO: add account parameter
+        int account = 0;
+        UniValue signer_res = signer.GetDescriptors(account);
+
+        if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result");
+        for (bool internal : {false, true}) {
+            const UniValue& descriptor_vals = find_value(signer_res, internal ? "internal" : "receive");
+            if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result");
+            for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) {
+                std::string desc_str = desc_val.getValStr();
+                FlatSigningProvider keys;
+                std::string dummy_error;
+                std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, dummy_error, false);
+                if (!desc->GetOutputType()) {
+                    continue;
                 }
+                OutputType t =  *desc->GetOutputType();
+                auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, internal));
+                spk_manager->SetupDescriptor(std::move(desc));
+                uint256 id = spk_manager->GetID();
+                m_spk_managers[id] = std::move(spk_manager);
+                AddActiveScriptPubKeyMan(id, t, internal);
             }
-            spk_manager->SetupDescriptorGeneration(master_key, t);
-            uint256 id = spk_manager->GetID();
-            m_spk_managers[id] = std::move(spk_manager);
-            AddActiveScriptPubKeyMan(id, t, internal);
         }
+#else
+        throw std::runtime_error(std::string(__func__) + ": Wallets with external signers require Boost::Process library.");
+#endif
     }
 }
 
diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py
@@ -20,13 +20,37 @@ def perform_pre_checks():
 def enumerate(args):
   sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}]))
 
+def getdescriptors(args):
+    xpub = "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B"
+
+    sys.stdout.write(json.dumps({
+        "receive": [
+            "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j",
+            "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/0/*))#r0grqw5x",
+            "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/0/*)#x30uthjs"
+        ],
+        "internal": [
+            "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2",
+            "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/1/*))#kwx4c3pe",
+            "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/1/*)#h92akzzg"
+        ]
+    }))
+
+
 parser = argparse.ArgumentParser(prog='./signer.py', description='External signer mock')
+parser.add_argument('--fingerprint')
+parser.add_argument('--chain', default='main')
+
 subparsers = parser.add_subparsers(description='Commands', dest='command')
 subparsers.required = True
 
 parser_enumerate = subparsers.add_parser('enumerate', help='list available signers')
 parser_enumerate.set_defaults(func=enumerate)
 
+parser_getdescriptors = subparsers.add_parser('getdescriptors')
+parser_getdescriptors.set_defaults(func=getdescriptors)
+parser_getdescriptors.add_argument('--account', metavar='account')
+
 args = parser.parse_args()
 
 perform_pre_checks()
diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
@@ -91,5 +91,37 @@ def run_test(self):
         not_hww = self.nodes[1].get_wallet_rpc('not_hww')
         assert_raises_rpc_error(-8, "Wallet flag is immutable: external_signer", not_hww.setwalletflag, "external_signer", True)
 
+        # assert_raises_rpc_error(-4, "Multiple signers found, please specify which to use", wallet_name='not_hww', disable_private_keys=True, descriptors=True, external_signer=True)
+
+        # TODO: Handle error thrown by script
+        # self.set_mock_result(self.nodes[1], "2")
+        # assert_raises_rpc_error(-1, 'Unable to parse JSON',
+        #     self.nodes[1].createwallet, wallet_name='not_hww2', disable_private_keys=True, descriptors=True, external_signer=False
+        # )
+        # self.clear_mock_result(self.nodes[1])
+
+        assert_equal(hww.getwalletinfo()["keypoolsize"], 3)
+
+        address1 = hww.getnewaddress(address_type="bech32")
+        assert_equal(address1, "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g")
+        address_info = hww.getaddressinfo(address1)
+        assert_equal(address_info['solvable'], True)
+        assert_equal(address_info['ismine'], True)
+        assert_equal(address_info['hdkeypath'], "m/84'/1'/0'/0/0")
+
+        address2 = hww.getnewaddress(address_type="p2sh-segwit")
+        assert_equal(address2, "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp")
+        address_info = hww.getaddressinfo(address2)
+        assert_equal(address_info['solvable'], True)
+        assert_equal(address_info['ismine'], True)
+        assert_equal(address_info['hdkeypath'], "m/49'/1'/0'/0/0")
+
+        address3 = hww.getnewaddress(address_type="legacy")
+        assert_equal(address3, "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9")
+        address_info = hww.getaddressinfo(address3)
+        assert_equal(address_info['solvable'], True)
+        assert_equal(address_info['ismine'], True)
+        assert_equal(address_info['hdkeypath'], "m/44'/1'/0'/0/0")
+
 if __name__ == '__main__':
     SignerTest().main()

Original file line number	Diff line number	Diff line change
`@@ -55,4 +55,9 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS`
`55`	`55`	`return true;`
`56`	`56`	`}`
`57`	`57`
	`58`	`+UniValue ExternalSigner::GetDescriptors(int account)`
	`59`	`+{`
	`60`	`+ return RunCommandParseJSON(m_command + " --fingerprint \"" + m_fingerprint + "\"" + NetworkArg() + " getdescriptors --account " + strprintf("%d", account));`
	`61`	`+}`
	`62`	`+`
`58`	`63`	`#endif`