MuSig2 power

doc/multisig-tutorial.md

@@ -20,15 +20,27 @@ This tutorial also uses the default PKH derivation path to get the xpubs and doe
 
 For a 2-of-3 multisig, create 3 wallets. These wallets contain HD seed and private keys, which will be used to sign the PSBTs and derive the xpub.
 
-These three wallets should not be used directly for privacy reasons (public key reuse). They should only be used to sign transactions for the (watch-only) multisig wallet.
+These three wallets should not be used directly for privacy reasons (public key reuse). They should only be used to sign transactions for the (watch-only) multisig wallet. To make that less likely to happen by accident, participant wallets only have singlesig legacy addresses, and no singlesig Bech32 addresses.
+
+`addhdkey` adds an HD key to the blank wallet, which `createwalletdescriptor` then uses:
 
 ```bash
 for ((n=1;n<=3;n++))
 do
- ./build/bin/bitcoin rpc -signet createwallet "participant_${n}"
+ ./build/bin/bitcoin rpc -signet -named createwallet wallet_name="participant_${n}" blank=true
+ ./build/bin/bitcoin rpc -signet -rpcwallet="participant_${n}" addhdkey
+ ./build/bin/bitcoin rpc -signet -rpcwallet="participant_${n}" createwalletdescriptor legacy
 done
 ```
 
+The `legacy` descriptor is only there to hold the private keys: it derives from `m/44h/1h/0h/<0;1>/*`, the same paths as the multisig descriptor defined below, and a wallet can only sign for a derivation path that one of its descriptors uses.
+
+A later step spends from the multisig wallet, so create one more wallet to receive that payment. This one is an ordinary singlesig wallet, standing in for whoever is being paid:
+
+```bash
+./build/bin/bitcoin rpc -signet createwallet "recipient"
+```
+
 Extract the xpub of each wallet. To do this, the `derivehdkey` RPC is used.
 
 Note that previously at least two descriptors were usually used, one for external derivation paths and one for internal ones. Since https://github.com/bitcoin/bitcoin/pull/22838 this redundancy has been eliminated by a multipath descriptor with <code><0;1></code> at the [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#change) change level expanding to external and internal descriptors when imported.
@@ -100,6 +112,7 @@ Once the wallets have already been created and this tutorial needs to be repeate
 ```bash
 for ((n=1;n<=3;n++)); do ./build/bin/bitcoin rpc -signet loadwallet "participant_${n}"; done
 ./build/bin/bitcoin rpc -signet loadwallet "multisig_wallet_01"
+./build/bin/bitcoin rpc -signet loadwallet "recipient"
 ```
 
 ### 1.4 Fund the wallet
@@ -138,7 +151,7 @@ PSBT is a data format that allows wallets and other tools to exchange informatio
 
 The current PSBT version (v0) is defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki).
 
-For simplicity, the destination address is taken from the `participant_1` wallet in the code above, but it can be any valid bitcoin address.
+The destination address is taken from the `recipient` wallet.
 
 The `walletcreatefundedpsbt` RPC is used to create and fund a transaction in the PSBT format. It is the first step in creating the PSBT.
 
@@ -147,7 +160,7 @@ balance=$(./build/bin/bitcoin rpc -signet -rpcwallet="multisig_wallet_01" getbal
 
 amount=$(echo "$balance * 0.8" | bc -l | sed -e 's/^\./0./' -e 's/^-\./-0./')
 
-destination_addr=$(./build/bin/bitcoin rpc -signet -rpcwallet="participant_1" getnewaddress)
+destination_addr=$(./build/bin/bitcoin rpc -signet -rpcwallet="recipient" getnewaddress "" bech32m)
 
 funded_psbt=$(./build/bin/bitcoin rpc -signet -rpcwallet="multisig_wallet_01" walletcreatefundedpsbt outputs="{\"$destination_addr\": $amount}" | jq -r '.psbt')
 ```

doc/release-notes-32857.md

@@ -0,0 +1,8 @@
+Updated RPCs
+------------
+
+- The `send`, `sendall`, `walletprocesspsbt`, `walletcreatefundedpsbt`, and
+  `descriptorprocesspsbt` RPCs now accept a `keypath_only` option. When enabled,
+  they do not add new Taproot script-path data, sign script paths, or finalize
+  script-path spends. Existing script-path data and signatures remain in a
+  supplied PSBT. (#32857)

doc/release-notes-32861.md

@@ -0,0 +1,9 @@
+Updated RPCs
+------------
+
+- `createwalletdescriptor` no longer requires the `hdkey` argument when the
+  wallet has no active descriptors but does have exactly one HD key added with
+  `addhdkey`. Previously the user had to look the key up with `listdescriptors`
+  and pass it in. This makes it easier to set up a blank wallet that only has
+  the descriptors it actually needs, which `doc/multisig-tutorial.md` now
+  demonstrates. (#32861)

src/common/types.h

@@ -48,6 +48,12 @@ struct PSBTFillOptions {
      * Whether to fill in bip32 derivation information if available.
      */
     bool bip32_derivs{true};
+
+    /**
+     * Only add new Taproot key-path data, and only sign and finalize Taproot
+     * inputs using the key path. Existing script-path data is left intact.
+     */
+    bool taproot_keypath_only{false};
 };
 
 } // namespace common

src/core_io.cpp

@@ -338,9 +338,13 @@ const std::map<unsigned char, std::string> mapSigHashTypes = {
     {static_cast<unsigned char>(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY), std::string("SINGLE|ANYONECANPAY")},
 };
 
-std::string SighashToStr(unsigned char sighash_type)
+std::string SighashToStr(int32_t sighash_type)
 {
-    const auto& it = mapSigHashTypes.find(sighash_type);
+    // Signatures encode the sighash type in a single byte, but the PSBT field
+    // for it is a 32 bit unsigned integer in BIP 174 (signed in PSBTInput)
+    if (sighash_type < 0 || sighash_type > 0xff) return "";
+    const uint8_t sighash_byte(sighash_type);
+    const auto& it = mapSigHashTypes.find(sighash_byte);
     if (it == mapSigHashTypes.end()) return "";
     return it->second;
 }

src/core_io.h

@@ -8,6 +8,7 @@
 #include <consensus/amount.h>
 #include <util/result.h>
 
+#include <cstdint>
 #include <functional>
 #include <string>
 
@@ -42,7 +43,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
 UniValue ValueFromAmount(CAmount amount);
 std::string FormatScript(const CScript& script);
 std::string EncodeHexTx(const CTransaction& tx);
-std::string SighashToStr(unsigned char sighash_type);
+std::string SighashToStr(int32_t sighash_type);
 void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false, const SigningProvider* provider = nullptr);
 void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex = true, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS, std::function<bool(const CTxOut&)> is_change_func = {});
 

src/external_signer.cpp

@@ -8,10 +8,13 @@
 #include <common/run_command.h>
 #include <core_io.h>
 #include <psbt.h>
+#include <script/interpreter.h>
 #include <util/strencodings.h>
 #include <util/subprocess.h>
 
 #include <algorithm>
+#include <cstdint>
+#include <optional>
 #include <stdexcept>
 #include <string>
 #include <vector>
@@ -77,6 +80,49 @@ UniValue ExternalSigner::GetDescriptors(const int account)
     return RunCommandParseJSON(Cat(m_command, Cat(Cat({"--fingerprint", m_fingerprint}, NetworkArg()), {"getdescriptors", "--account", strprintf("%d", account)})), "");
 }
 
+UniValue ExternalSigner::RegisterDescriptor(const std::string& name, const std::string& descriptor) const
+{
+    return RunCommandParseJSON(Cat(m_command, Cat(Cat({"--fingerprint", m_fingerprint}, NetworkArg()), {"registerdescriptor", name, descriptor})), "");
+}
+
+UniValue ExternalSigner::DisplayAddressRegistered(const std::string& registration, bool change, uint32_t index) const
+{
+    // The registration descriptor combines the chains as <receive;change>,
+    // so the change chain is multipath index 1.
+    return RunCommandParseJSON(Cat(m_command, Cat(Cat({"--fingerprint", m_fingerprint}, NetworkArg()),
+        {"displayaddress",
+         "--registration", registration,
+         "--index", strprintf("%u", index),
+         "--multipath-index", change ? "1" : "0"})), "");
+}
+
+//! Find a sighash type used by the signer that doesn't commit to all outputs;
+//! a signature with SIGHASH_NONE or SIGHASH_SINGLE would let anyone alter
+//! them. SIGHASH_ANYONECANPAY is acceptable, since it only permits adding
+//! inputs, which does not affect us.
+static std::optional<int32_t> FindUnsafeSighashType(const PartiallySignedTransaction& psbtx)
+{
+    std::vector<int32_t> sighash_types;
+    for (const PSBTInput& input : psbtx.inputs) {
+        if (input.sighash_type) sighash_types.push_back(*input.sighash_type);
+        for (const auto& [_, sig] : input.partial_sigs) {
+            if (!sig.second.empty()) sighash_types.push_back(sig.second.back());
+        }
+        // A 64 byte taproot signature implies SIGHASH_DEFAULT, 65 bytes
+        // carries an explicit sighash type in the last byte
+        if (input.m_tap_key_sig.size() == 65) sighash_types.push_back(input.m_tap_key_sig.back());
+        for (const auto& [_, sig] : input.m_tap_script_sigs) {
+            if (sig.size() == 65) sighash_types.push_back(sig.back());
+        }
+    }
+
+    for (const int32_t sighash_type : sighash_types) {
+        const int32_t base_sighash_type{sighash_type & ~SIGHASH_ANYONECANPAY};
+        if (base_sighash_type != SIGHASH_DEFAULT && base_sighash_type != SIGHASH_ALL) return sighash_type;
+    }
+    return std::nullopt;
+}
+
 bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::string& error)
 {
     // Serialize the PSBT
@@ -121,7 +167,51 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str
         return false;
     }
 
-    psbtx = *signer_psbtx;
+    if (const std::optional<int32_t> sighash{FindUnsafeSighashType(*signer_psbtx)}) {
+        const std::string sighash_str{SighashToStr(*sighash)};
+        error = strprintf("Signer used an unsafe sighash type: %s", sighash_str.empty() ? "unknown" : sighash_str);
+        return false;
+    }
+
+    if (!psbtx.Merge(*signer_psbtx)) {
+        error = "Signer returned a PSBT for a different transaction";
+        return false;
+    }
+
+    return true;
+}
 
+bool ExternalSigner::SignTransactionRegistered(PartiallySignedTransaction& psbtx,
+                                               const std::string& registration,
+                                               std::string& error)
+{
+    // Serialize the PSBT
+    DataStream ssTx{};
+    ssTx << psbtx;
+
+    const std::vector<std::string> command = Cat(m_command, Cat({"--stdin", "--fingerprint", m_fingerprint}, NetworkArg()));
+
+    // The registration is base64 encoded by HWI and therefore contains no
+    // whitespace for shlex to split when the command is read from stdin.
+    const std::string stdinStr = "signtx " + EncodeBase64(ssTx.str()) + " --registration " + registration;
+
+    const UniValue signer_result = RunCommandParseJSON(command, stdinStr);
+
+    if (signer_result.find_value("error").isStr()) {
+        error = signer_result.find_value("error").get_str();
+        return false;
+    }
+    if (!signer_result.find_value("psbt").isStr()) {
+        error = "Unexpected result from signer";
+        return false;
+    }
+
+    util::Result<PartiallySignedTransaction> signer_psbtx = DecodeBase64PSBT(signer_result.find_value("psbt").get_str());
+    if (!signer_psbtx) {
+        error = strprintf("TX decode failed %s", util::ErrorString(signer_psbtx).original);
+        return false;
+    }
+
+    psbtx = *signer_psbtx;
     return true;
 }

src/external_signer.h

@@ -8,6 +8,7 @@
 #include <common/system.h>
 #include <univalue.h>
 
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -52,18 +53,48 @@ class ExternalSigner
     //!            Must include a public key or xpub, as well as key origin.
     UniValue DisplayAddress(const std::string& descriptor) const;
 
+    //! Display an address from a previously registered descriptor.
+    //! Calls `<command> displayaddress --registration <registration>
+    //! --index <index> --multipath-index <0|1>`.
+    //! @param[in] registration opaque value returned by `registerdescriptor`
+    //! @param[in] change       whether to show the change-chain address
+    //! @param[in] index        address index within the chain
+    //! @returns                signer reply, expected to contain `address`
+    UniValue DisplayAddressRegistered(const std::string& registration, bool change, uint32_t index) const;
+
     //! Get receive and change Descriptor(s) from device for a given account.
     //! Calls `<command> --fingerprint <fingerprint> --chain <chain> getdescriptors
     //! --account <account>`.
     //! @param[in] account  which BIP32 account to use (e.g. `m/44'/0'/account'`)
     //! @returns see doc/external-signer.md
     UniValue GetDescriptors(int account);
 
+    //! Register a descriptor on the device.
+    //! Calls `<command> registerdescriptor <name> <descriptor>`.
+    //! @param[in] name       descriptor name to display on the signer
+    //! @param[in] descriptor combined multipath descriptor
+    //! @returns opaque registration provided by the signer
+    UniValue RegisterDescriptor(const std::string& name, const std::string& descriptor) const;
+
     //! Sign PartiallySignedTransaction on the device.
     //! Calls `<command> --stdin --fingerprint <fingerprint> --chain <chain>` and passes the
     //! `signtx` command and PSBT via stdin.
     //! @param[in,out] psbt  PartiallySignedTransaction to be signed
     bool SignTransaction(PartiallySignedTransaction& psbt, std::string& error);
+
+    //! Sign a PartiallySignedTransaction using a previously registered
+    //! descriptor. Used for descriptors (e.g. MuSig2) that require
+    //! on-device registration before signing.
+    //!
+    //! Pipes `signtx <base64> --registration <registration>` to the
+    //! signer's stdin.
+    //!
+    //! @param[in,out] psbt         PSBT to be signed
+    //! @param[in] registration     opaque value returned by `registerdescriptor`
+    //! @param[out] error           populated on failure
+    bool SignTransactionRegistered(PartiallySignedTransaction& psbt,
+                                   const std::string& registration,
+                                   std::string& error);
 };
 
 #endif // BITCOIN_EXTERNAL_SIGNER_H

src/key_io.cpp

@@ -81,6 +81,7 @@ class DestinationEncoder
     std::string operator()(const CNoDestination& no) const { return {}; }
     std::string operator()(const PubKeyDestination& pk) const { return {}; }
 };
+} // namespace
 
 CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector<int>* error_locations)
 {
@@ -209,7 +210,6 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
     if (error_locations) *error_locations = std::move(res.second);
     return CNoDestination();
 }
-} // namespace
 
 CKey DecodeSecret(const std::string& str)
 {

src/key_io.h

@@ -24,6 +24,7 @@ std::string EncodeExtPubKey(const CExtPubKey& extpubkey);
 std::string EncodeDestination(const CTxDestination& dest);
 CTxDestination DecodeDestination(const std::string& str);
 CTxDestination DecodeDestination(const std::string& str, std::string& error_msg, std::vector<int>* error_locations = nullptr);
+CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector<int>* error_locations = nullptr);
 bool IsValidDestinationString(const std::string& str);
 bool IsValidDestinationString(const std::string& str, const CChainParams& params);
 

src/psbt.cpp

@@ -464,6 +464,7 @@ bool PSBTInput::Merge(const PSBTInput& input)
     for (const auto& [agg_key_lh, psigs] : input.m_musig2_partial_sigs) {
         m_musig2_partial_sigs[agg_key_lh].insert(psigs.begin(), psigs.end());
     }
+    if (sighash_type == std::nullopt && input.sighash_type != std::nullopt) sighash_type = input.sighash_type;
     if (sequence == std::nullopt && input.sequence != std::nullopt) sequence = input.sequence;
     if (time_locktime == std::nullopt && input.time_locktime != std::nullopt) time_locktime = input.time_locktime;
     if (height_locktime == std::nullopt && input.height_locktime != std::nullopt) height_locktime = input.height_locktime;
@@ -733,7 +734,7 @@ util::Expected<void, PSBTError> SignPSBTInput(const SigningProvider& provider, P
     if (txdata == nullptr) {
         sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata);
     } else {
-        MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, {.sighash_type = sighash});
+        MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, {.sighash_type = sighash, .taproot_keypath_only = options.taproot_keypath_only});
         sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata);
     }
     // Verify that a witness signature was produced in case one was required.

src/qt/createwalletdialog.cpp

@@ -26,34 +26,25 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) :
     });
 
     connect(ui->encrypt_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) {
-        // Disable the disable_privkeys_checkbox and external_signer_checkbox when isEncryptWalletChecked is
+        // Disable the disable_privkeys_checkbox when isEncryptWalletChecked is
         // set to true, enable it when isEncryptWalletChecked is false.
         ui->disable_privkeys_checkbox->setEnabled(!checked);
-#ifdef ENABLE_EXTERNAL_SIGNER
-        ui->external_signer_checkbox->setEnabled(m_has_signers && !checked);
-#endif
+
         // When the disable_privkeys_checkbox is disabled, uncheck it.
         if (!ui->disable_privkeys_checkbox->isEnabled()) {
             ui->disable_privkeys_checkbox->setChecked(false);
         }
-
-        // When the external_signer_checkbox box is disabled, uncheck it.
-        if (!ui->external_signer_checkbox->isEnabled()) {
-            ui->external_signer_checkbox->setChecked(false);
-        }
-
     });
 
     connect(ui->external_signer_checkbox, &QCheckBox::toggled, [this](bool checked) {
-        ui->encrypt_wallet_checkbox->setEnabled(!checked);
-        ui->blank_wallet_checkbox->setEnabled(!checked);
-        ui->disable_privkeys_checkbox->setEnabled(!checked);
+        // In the basic use case all keys will be on the external signer
+        // device and the wallet should be watch-only. Makes this the
+        // default suggestion.
+        ui->disable_privkeys_checkbox->setChecked(checked);
 
-        // The external signer checkbox is only enabled when a device is detected.
-        // In that case it is checked by default. Toggling it restores the other
-        // options to their default.
+        // The external signer box is checked by default when a device is
+        // detected. Toggling it restores the other options to their default.
         ui->encrypt_wallet_checkbox->setChecked(false);
-        ui->disable_privkeys_checkbox->setChecked(checked);
         ui->blank_wallet_checkbox->setChecked(false);
     });
 
@@ -103,12 +94,9 @@ void CreateWalletDialog::setSigners(const std::vector<std::unique_ptr<interfaces
     if (m_has_signers) {
         ui->external_signer_checkbox->setEnabled(true);
         ui->external_signer_checkbox->setChecked(true);
-        ui->encrypt_wallet_checkbox->setEnabled(false);
         ui->encrypt_wallet_checkbox->setChecked(false);
         // The order matters, because connect() is called when toggling a checkbox:
-        ui->blank_wallet_checkbox->setEnabled(false);
         ui->blank_wallet_checkbox->setChecked(false);
-        ui->disable_privkeys_checkbox->setEnabled(false);
         ui->disable_privkeys_checkbox->setChecked(true);
         const std::string label = signers[0]->getName();
         ui->wallet_name_line_edit->setText(QString::fromStdString(label));