rpc: send: support external signer

10 files changed, 180 insertions, 5 deletions

diff --git a/src/util/error.cpp b/src/util/error.cpp
index 76fac4d391..48c81693f3 100644
--- a/src/util/error.cpp
+++ b/src/util/error.cpp
@@ -31,6 +31,10 @@ bilingual_str TransactionErrorString(const TransactionError err)
             return Untranslated("Specified sighash value does not match value stored in PSBT");
         case TransactionError::MAX_FEE_EXCEEDED:
             return Untranslated("Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)");
+        case TransactionError::EXTERNAL_SIGNER_NOT_FOUND:
+            return Untranslated("External signer not found");
+        case TransactionError::EXTERNAL_SIGNER_FAILED:
+            return Untranslated("External signer failed to sign");
         // no default case, so the compiler can warn about missing cases
     }
     assert(false);
diff --git a/src/util/error.h b/src/util/error.h
index 6633498d2b..4cc35eb1fd 100644
--- a/src/util/error.h
+++ b/src/util/error.h
@@ -30,6 +30,8 @@ enum class TransactionError {
     PSBT_MISMATCH,
     SIGHASH_MISMATCH,
     MAX_FEE_EXCEEDED,
+    EXTERNAL_SIGNER_NOT_FOUND,
+    EXTERNAL_SIGNER_FAILED,
 };
 
 bilingual_str TransactionErrorString(const TransactionError error);
diff --git a/src/wallet/external_signer.cpp b/src/wallet/external_signer.cpp
index baf97700e7..3396111760 100644
--- a/src/wallet/external_signer.cpp
+++ b/src/wallet/external_signer.cpp
@@ -3,6 +3,10 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <chainparams.h>
+#include <core_io.h>
+#include <psbt.h>
+#include <util/strencodings.h>
+#include <util/system.h>
 #include <wallet/external_signer.h>
 
 ExternalSigner::ExternalSigner(const std::string& command, const std::string& fingerprint, std::string chain, std::string name): m_command(command), m_fingerprint(fingerprint), m_chain(chain), m_name(name) {}
@@ -65,4 +69,51 @@ UniValue ExternalSigner::GetDescriptors(int account)
     return RunCommandParseJSON(m_command + " --fingerprint \"" + m_fingerprint + "\"" + NetworkArg() + " getdescriptors --account " + strprintf("%d", account));
 }
 
+bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::string& error)
+{
+    // Serialize the PSBT
+    CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
+    ssTx << psbtx;
+
+    // Check if signer fingerprint matches any input master key fingerprint
+    bool match = false;
+    for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) {
+        const PSBTInput& input = psbtx.inputs[i];
+        for (auto entry : input.hd_keypaths) {
+            if (m_fingerprint == strprintf("%08x", ReadBE32(entry.second.fingerprint))) match = true;
+        }
+    }
+
+    if (!match) {
+        error = "Signer fingerprint " + m_fingerprint + " does not match any of the inputs:\n" + EncodeBase64(ssTx.str());
+        return false;
+    }
+
+    std::string command = m_command + " --stdin --fingerprint \"" + m_fingerprint + "\"" + NetworkArg();
+    std::string stdinStr = "signtx \"" + EncodeBase64(ssTx.str()) + "\"";
+
+    const UniValue signer_result = RunCommandParseJSON(command, stdinStr);
+
+    if (find_value(signer_result, "error").isStr()) {
+        error = find_value(signer_result, "error").get_str();
+        return false;
+    }
+
+    if (!find_value(signer_result, "psbt").isStr()) {
+        error = "Unexpected result from signer";
+        return false;
+    }
+
+    PartiallySignedTransaction signer_psbtx;
+    std::string signer_psbt_error;
+    if (!DecodeBase64PSBT(signer_psbtx, find_value(signer_result, "psbt").get_str(), signer_psbt_error)) {
+        error = strprintf("TX decode failed %s", signer_psbt_error);
+        return false;
+    }
+
+    psbtx = signer_psbtx;
+
+    return true;
+}
+
 #endif
diff --git a/src/wallet/external_signer.h b/src/wallet/external_signer.h
index 198a939d3d..4b9711107b 100644
--- a/src/wallet/external_signer.h
+++ b/src/wallet/external_signer.h
@@ -10,6 +10,8 @@
 #include <univalue.h>
 #include <util/system.h>
 
+struct PartiallySignedTransaction;
+
 class ExternalSignerException : public std::runtime_error {
 public:
     using std::runtime_error::runtime_error;
@@ -60,6 +62,11 @@ public:
     //! @param[out] UniValue see doc/external-signer.md
     UniValue GetDescriptors(int account);
 
+    //! Sign PartiallySignedTransaction on the device.
+    //! Calls `<command> signtransaction` and passes the PSBT via stdin.
+    //! @param[in,out] psbt  PartiallySignedTransaction to be signed
+    bool SignTransaction(PartiallySignedTransaction& psbt, std::string& error);
+
 #endif
 };
 
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
index 1cde143ef9..e60d7b8004 100644
--- a/src/wallet/external_signer_scriptpubkeyman.h
+++ b/src/wallet/external_signer_scriptpubkeyman.h
@@ -26,6 +26,8 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
   static ExternalSigner GetExternalSigner();
 
   bool DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const;
+
+  TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override;
 };
 #endif
 
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 9d61f6fbe2..bfc42ac1b0 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -4195,8 +4195,10 @@ static RPCHelpMan send()
             // Make a blank psbt
             PartiallySignedTransaction psbtx(rawTx);
 
-            // Fill transaction with our data and sign
-            bool complete = true;
+            // First fill transaction with our data without signing,
+            // so external signers are not asked sign more than once.
+            bool complete;
+            pwallet->FillPSBT(psbtx, complete, SIGHASH_ALL, false, true);
             const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_ALL, true, false);
             if (err != TransactionError::OK) {
                 throw JSONRPCTransactionError(err);
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 4630603f8e..efb408c163 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -13,6 +13,7 @@
 #include <util/system.h>
 #include <util/time.h>
 #include <util/translation.h>
+#include <wallet/external_signer.h>
 #include <wallet/scriptpubkeyman.h>
 
 //! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details.
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7c15438067..08e480225d 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -19,6 +19,7 @@
 #include <policy/policy.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
+#include <psbt.h>
 #include <script/descriptor.h>
 #include <script/script.h>
 #include <script/signingprovider.h>
diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py
index 4036c785b3..676d0a0a4d 100755
--- a/test/functional/mocks/signer.py
+++ b/test/functional/mocks/signer.py
@@ -51,9 +51,25 @@ def displayaddress(args):
 
     return sys.stdout.write(json.dumps({"address": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g"}))
 
+def signtx(args):
+    if args.fingerprint != "00000001":
+        return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint}))
+
+    with open(os.path.join(os.getcwd(), "mock_psbt"), "r", encoding="utf8") as f:
+        mock_psbt = f.read()
+
+    if args.fingerprint == "00000001" :
+        sys.stdout.write(json.dumps({
+            "psbt": mock_psbt,
+            "complete": True
+        }))
+    else:
+        sys.stdout.write(json.dumps({"psbt": args.psbt}))
+
 parser = argparse.ArgumentParser(prog='./signer.py', description='External signer mock')
 parser.add_argument('--fingerprint')
 parser.add_argument('--chain', default='main')
+parser.add_argument('--stdin', action='store_true')
 
 subparsers = parser.add_subparsers(description='Commands', dest='command')
 subparsers.required = True
@@ -69,6 +85,16 @@ parser_displayaddress = subparsers.add_parser('displayaddress', help='display ad
 parser_displayaddress.add_argument('--desc', metavar='desc')
 parser_displayaddress.set_defaults(func=displayaddress)
 
+parser_signtx = subparsers.add_parser('signtx')
+parser_signtx.add_argument('psbt', metavar='psbt')
+
+parser_signtx.set_defaults(func=signtx)
+
+if not sys.stdin.isatty():
+    buffer = sys.stdin.read()
+    if buffer and buffer.rstrip() != "":
+       sys.argv.extend(buffer.rstrip().split(" "))
+
 args = parser.parse_args()
 
 perform_pre_checks()
diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
index b39f1b4d9b..9dd080dca9 100755
--- a/test/functional/wallet_signer.py
+++ b/test/functional/wallet_signer.py
@@ -25,7 +25,7 @@ class SignerTest(BitcoinTestFramework):
             return path
 
     def set_test_params(self):
-        self.num_nodes = 3
+        self.num_nodes = 4
 
         self.extra_args = [
             [],
@@ -54,7 +54,7 @@ class SignerTest(BitcoinTestFramework):
 
         # Handle script missing:
         assert_raises_rpc_error(-1, 'execve failed: No such file or directory',
-            self.nodes[2].enumeratesigners
+            self.nodes[3].enumeratesigners
         )
 
         # Handle error thrown by script
@@ -100,7 +100,7 @@ class SignerTest(BitcoinTestFramework):
         # )
         # self.clear_mock_result(self.nodes[1])
 
-        assert_equal(hww.getwalletinfo()["keypoolsize"], 3)
+        assert_equal(hww.getwalletinfo()["keypoolsize"], 30)
 
         address1 = hww.getnewaddress(address_type="bech32")
         assert_equal(address1, "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g")
@@ -134,5 +134,84 @@ class SignerTest(BitcoinTestFramework):
         )
         self.clear_mock_result(self.nodes[1])
 
+        self.log.info('Prepare mock PSBT')
+        self.nodes[0].sendtoaddress(address1, 1)
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Load private key into wallet to generate a signed PSBT for the mock
+        self.nodes[1].createwallet(wallet_name="mock", disable_private_keys=False, blank=True, descriptors=True)
+        mock_wallet = self.nodes[1].get_wallet_rpc("mock")
+        assert mock_wallet.getwalletinfo()['private_keys_enabled']
+
+        result = mock_wallet.importdescriptors([{
+            "desc": "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0/*)#rweraev0",
+            "timestamp": 0,
+            "range": [0,1],
+            "internal": False,
+            "active": True
+        },
+        {
+            "desc": "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/*)#j6uzqvuh",
+            "timestamp": 0,
+            "range": [0, 0],
+            "internal": True,
+            "active": True
+        }])
+        assert_equal(result[0], {'success': True})
+        assert_equal(result[1], {'success': True})
+        assert_equal(mock_wallet.getwalletinfo()["txcount"], 1)
+        dest = self.nodes[0].getnewaddress(address_type='bech32')
+        mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {}, True)['psbt']
+        mock_psbt_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt, sign=True, sighashtype="ALL", bip32derivs=True)
+        mock_psbt_final = mock_wallet.finalizepsbt(mock_psbt_signed["psbt"])
+        mock_tx = mock_psbt_final["hex"]
+        assert(mock_wallet.testmempoolaccept([mock_tx])[0]["allowed"])
+
+        # # Create a new wallet and populate with specific public keys, in order
+        # # to work with the mock signed PSBT.
+        # self.nodes[1].createwallet(wallet_name="hww4", disable_private_keys=True, descriptors=True, external_signer=True)
+        # hww4 = self.nodes[1].get_wallet_rpc("hww4")
+        #
+        # descriptors = [{
+        #     "desc": "wpkh([00000001/84'/1'/0']tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/*)#x30uthjs",
+        #     "timestamp": "now",
+        #     "range": [0, 1],
+        #     "internal": False,
+        #     "watchonly": True,
+        #     "active": True
+        # },
+        # {
+        #     "desc": "wpkh([00000001/84'/1'/0']tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/*)#h92akzzg",
+        #     "timestamp": "now",
+        #     "range": [0, 0],
+        #     "internal": True,
+        #     "watchonly": True,
+        #     "active": True
+        # }]
+
+        # result = hww4.importdescriptors(descriptors)
+        # assert_equal(result[0], {'success': True})
+        # assert_equal(result[1], {'success': True})
+        assert_equal(hww.getwalletinfo()["txcount"], 1)
+
+        assert(hww.testmempoolaccept([mock_tx])[0]["allowed"])
+
+        with open(os.path.join(self.nodes[1].cwd, "mock_psbt"), "w", encoding="utf8") as f:
+            f.write(mock_psbt_signed["psbt"])
+
+        self.log.info('Test send using hww1')
+
+        res = hww.send(outputs={dest:0.5},options={"add_to_wallet": False})
+        assert(res["complete"])
+        assert_equal(res["hex"], mock_tx)
+
+        # # Handle error thrown by script
+        # self.set_mock_result(self.nodes[4], "2")
+        # assert_raises_rpc_error(-1, 'Unable to parse JSON',
+        #     hww4.signerprocesspsbt, psbt_orig, "00000001"
+        # )
+        # self.clear_mock_result(self.nodes[4])
+
 if __name__ == '__main__':
     SignerTest().main()