Merge #6503: backport: merge bitcoin#23106, bitcoin#22067, bitcoin#23303

, bitcoin#23403, bitcoin#22513, bitcoin#17034, bitcoin#23718, partial bitcoin#21330, bitcoin#21365, bitcoin#22514 (psbt backports)

aba9370 test: strip `vin.scriptWitness` from more PSBT test vectors (Kittywhiskers Van Gogh)
e4530e6 merge bitcoin#23718: hash preimages fields (Kittywhiskers Van Gogh)
dc9342b test: strip `vin.scriptWitness` from PSBT test vectors (Kittywhiskers Van Gogh)
c076d02 merge bitcoin#17034: PSBT version, proprietary, and xpub fields (BIP 174) (Kittywhiskers Van Gogh)
f1ba319 partial bitcoin#22514: Actually use SIGHASH_DEFAULT for PSBT signing (Kittywhiskers Van Gogh)
0c52bfe merge bitcoin#22513: Allow walletprocesspsbt to sign without finalizing (Kittywhiskers Van Gogh)
193765b merge bitcoin#23403: Fix segfault in the psbt_wallet_tests/psbt_updater_test (Kittywhiskers Van Gogh)
ba85b4c merge bitcoin#23303: Fix wallet_multisig_descriptor_psbt.py (Kittywhiskers Van Gogh)
c7a69cc merge bitcoin#22067: Test and document a basic M-of-N multisig using descriptor wallets and PSBTs (Kittywhiskers Van Gogh)
3219855 merge bitcoin#23106: Ensure wallet is unlocked before signing PSBT with walletprocesspsbt and GUI (Kittywhiskers Van Gogh)
b7c7c7b partial bitcoin#21365: Basic Taproot signing support for descriptor wallets (Kittywhiskers Van Gogh)
4a08920 partial bitcoin#21330: Deal with missing data in signature hashes more consistently (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Even though Dash Core does not implement Taproot, [bitcoin#21365](bitcoin#21365) has been partially backported due to changes related to transaction precomputation, required by later PSBT backports.

  * Some test vectors introduced in [bitcoin#17034](bitcoin#17034) and [bitcoin#23718](bitcoin#23718) have `psbtx.tx->vin[i].scriptWitness` populated and as Dash Core does not support SegWit, attempting to read the raw data results in serialization errors.

    The offending field was stripped out by forking Bitcoin Core v24 and modifying `decodepsbt` to strip out the field, reencode the modified data and return it. These changes were made in separate commits.

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK aba9370
  PastaPastaPasta:
    utACK aba9370

Tree-SHA512: 439e009da032dffaa7b7048984ed27fefb7ce0854240e280a898d20c50132197d5575d8a289391d50f2527ab4f26326481ce78200c3014b6223d9c661e5c9377

doc/descriptors.md

@@ -117,6 +117,47 @@ Key order does not matter for `sortedmulti()`. `sortedmulti()` behaves in the sa
 as `multi()` does but the keys are reordered in the resulting script such that they
 are lexicographically ordered as described in BIP67.
 
+#### Basic multisig example
+
+For a good example of a basic M-of-N multisig between multiple participants using descriptor
+wallets and PSBTs, as well as a signing flow, see [this functional test](/test/functional/wallet_multisig_descriptor_psbt.py).
+
+Disclaimers: It is important to note that this example serves as a quick-start and is kept basic for readability. A downside of the approach
+outlined here is that each participant must maintain (and backup) two separate wallets: a signer and the corresponding multisig.
+It should also be noted that privacy best-practices are not "by default" here - participants should take care to only use the signer to sign
+transactions related to the multisig. Lastly, it is not recommended to use anything other than a Bitcoin Core descriptor wallet to serve as your
+signer(s). Other wallets, whether hardware or software, likely impose additional checks and safeguards to prevent users from signing transactions that
+could lead to loss of funds, or are deemed security hazards. Conforming to various 3rd-party checks and verifications is not in the scope of this example.
+
+The basic steps are:
+
+  1. Every participant generates an xpub. The most straightforward way is to create a new descriptor wallet which we will refer to as
+     the participant's signer wallet. Avoid reusing this wallet for any purpose other than signing transactions from the
+     corresponding multisig we are about to create. Hint: extract the wallet's xpubs using `listdescriptors` and pick the one from the
+     `pkh` descriptor since it's least likely to be accidentally reused (legacy addresses)
+  2. Create a watch-only descriptor wallet (blank, private keys disabled). Now the multisig is created by importing the two descriptors:
+     `wsh(sortedmulti(<M>,XPUB1/0/*,XPUB2/0/*,…,XPUBN/0/*))` and `wsh(sortedmulti(<M>,XPUB1/1/*,XPUB2/1/*,…,XPUBN/1/*))`
+     (one descriptor w/ `0` for receiving addresses and another w/ `1` for change). Every participant does this
+  3. A receiving address is generated for the multisig. As a check to ensure step 2 was done correctly, every participant
+     should verify they get the same addresses
+  4. Funds are sent to the resulting address
+  5. A sending transaction from the multisig is created using `walletcreatefundedpsbt` (anyone can initiate this). It is simple to do
+     this in the GUI by going to the `Send` tab in the multisig wallet and creating an unsigned transaction (PSBT)
+  6. At least `M` participants check the PSBT with their multisig using `decodepsbt` to verify the transaction is OK before signing it.
+  7. (If OK) the participant signs the PSBT with their signer wallet using `walletprocesspsbt`. It is simple to do this in the GUI by
+     loading the PSBT from file and signing it
+  8. The signed PSBTs are collected with `combinepsbt`, finalized w/ `finalizepsbt`, and then the resulting transaction is broadcasted
+     to the network. Note that any wallet (eg one of the signers or multisig) is capable of doing this.
+  9. Checks that balances are correct after the transaction has been included in a block
+
+You may prefer a daisy chained signing flow where each participant signs the PSBT one after another until
+the PSBT has been signed `M` times and is "complete." For the most part, the steps above remain the same, except (6, 7)
+change slightly from signing the original PSBT in parallel to signing it in series. `combinepsbt` is not necessary with
+this signing flow and the last (`m`th) signer can just broadcast the PSBT after signing. Note that a parallel signing flow may be
+preferable in cases where there are more signers. This signing flow is also included in the test / Python example.
+[The test](/test/functional/wallet_multisig_descriptor_psbt.py) is meant to be documentation as much as it is a functional test, so
+it is kept as simple and readable as possible.
+
 ### BIP32 derived keys and chains
 
 Most modern wallet software and hardware uses keys that are derived using

doc/psbt.md

@@ -92,6 +92,9 @@ hardware implementations will typically implement multiple roles simultaneously.
 
 #### Multisig with multiple Bitcoin Core instances
 
+For a quick start see [Basic M-of-N multisig example using descriptor wallets and PSBTs](./descriptors.md#basic-multisig-example).
+If you are using legacy wallets feel free to continue with the example provided here.
+
 Alice, Bob, and Carol want to create a 2-of-3 multisig address. They're all using
 Bitcoin Core. We assume their wallets only contain the multisig funds. In case
 they also have a personal wallet, this can be accomplished through the

src/coinjoin/server.cpp

@@ -556,7 +556,7 @@ bool CCoinJoinServer::IsInputScriptSigValid(const CTxIn& txin) const
         txNew.vin[nTxInIndex].scriptSig = txin.scriptSig;
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- verifying scriptSig %s\n", ScriptToAsmStr(txin.scriptSig).substr(0, 24));
         // TODO we're using amount=0 here but we should use the correct amount. This works because Dash ignores the amount while signing/verifying (only used in Bitcoin/Segwit)
-        if (!VerifyScript(txNew.vin[nTxInIndex].scriptSig, sigPubKey, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, MutableTransactionSignatureChecker(&txNew, nTxInIndex, 0))) {
+        if (!VerifyScript(txNew.vin[nTxInIndex].scriptSig, sigPubKey, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, MutableTransactionSignatureChecker(&txNew, nTxInIndex, 0, MissingDataBehavior::ASSERT_FAIL))) {
             LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- VerifyScript() failed on input %d\n", nTxInIndex);
             return false;
         }

src/node/psbt.cpp

@@ -22,6 +22,8 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
 
     result.inputs.resize(psbtx.tx->vin.size());
 
+    const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx);
+
     for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
         PSBTInput& input = psbtx.inputs[i];
         PSBTInputAnalysis& input_analysis = result.inputs[i];
@@ -60,7 +62,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
 
             // Figure out what is missing
             SignatureData outdata;
-            bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, &outdata);
+            bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, 1, &outdata);
 
             // Things are missing
             if (!complete) {
@@ -119,7 +121,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
             PSBTInput& input = psbtx.inputs[i];
             Coin newcoin;
 
-            if (!SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, nullptr, true) || !psbtx.GetInputUTXO(newcoin.out, i)) {
+            if (!SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, 1) || !psbtx.GetInputUTXO(newcoin.out, i)) {
                 success = false;
                 break;
             } else {

src/psbt.cpp

@@ -37,6 +37,13 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
     for (unsigned int i = 0; i < outputs.size(); ++i) {
         outputs[i].Merge(psbt.outputs[i]);
     }
+    for (auto& xpub_pair : psbt.m_xpubs) {
+        if (m_xpubs.count(xpub_pair.first) == 0) {
+            m_xpubs[xpub_pair.first] = xpub_pair.second;
+        } else {
+            m_xpubs[xpub_pair.first].insert(xpub_pair.second.begin(), xpub_pair.second.end());
+        }
+    }
     unknown.insert(psbt.unknown.begin(), psbt.unknown.end());
 
     return true;
@@ -63,12 +70,15 @@ bool PartiallySignedTransaction::AddOutput(const CTxOut& txout, const PSBTOutput
 
 bool PartiallySignedTransaction::GetInputUTXO(CTxOut& utxo, int input_index) const
 {
-    PSBTInput input = inputs[input_index];
+    const PSBTInput& input = inputs[input_index];
     uint32_t prevout_index = tx->vin[input_index].prevout.n;
     if (input.non_witness_utxo) {
         if (prevout_index >= input.non_witness_utxo->vout.size()) {
             return false;
         }
+        if (input.non_witness_utxo->GetHash() != tx->vin[input_index].prevout.hash) {
+            return false;
+        }
         utxo = input.non_witness_utxo->vout[prevout_index];
     } else {
         return false;
@@ -127,6 +137,10 @@ void PSBTInput::Merge(const PSBTInput& input)
     if (!non_witness_utxo && input.non_witness_utxo) non_witness_utxo = input.non_witness_utxo;
 
     partial_sigs.insert(input.partial_sigs.begin(), input.partial_sigs.end());
+    ripemd160_preimages.insert(input.ripemd160_preimages.begin(), input.ripemd160_preimages.end());
+    sha256_preimages.insert(input.sha256_preimages.begin(), input.sha256_preimages.end());
+    hash160_preimages.insert(input.hash160_preimages.begin(), input.hash160_preimages.end());
+    hash256_preimages.insert(input.hash256_preimages.begin(), input.hash256_preimages.end());
     hd_keypaths.insert(input.hd_keypaths.begin(), input.hd_keypaths.end());
     unknown.insert(input.unknown.begin(), input.unknown.end());
 
@@ -202,7 +216,24 @@ bool PSBTInputSigned(const PSBTInput& input)
     return !input.final_script_sig.empty();
 }
 
-bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash, SignatureData* out_sigdata, bool use_dummy)
+PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& psbt)
+{
+    const CMutableTransaction& tx = *psbt.tx;
+    bool have_all_spent_outputs = true;
+    std::vector<CTxOut> utxos(tx.vin.size());
+    for (size_t idx = 0; idx < tx.vin.size(); ++idx) {
+        if (!psbt.GetInputUTXO(utxos[idx], idx)) have_all_spent_outputs = false;
+    }
+    PrecomputedTransactionData txdata;
+    if (have_all_spent_outputs) {
+        txdata.Init(tx, std::move(utxos), true);
+    } else {
+        txdata.Init(tx, {}, true);
+    }
+    return txdata;
+}
+
+bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, int sighash,  SignatureData* out_sigdata, bool finalize)
 {
     PSBTInput& input = psbt.inputs.at(index);
     const CMutableTransaction& tx = *psbt.tx;
@@ -233,12 +264,16 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction&
     }
 
     bool sig_complete;
-    if (use_dummy) {
+    if (txdata == nullptr) {
         sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata);
     } else {
-        MutableTransactionSignatureCreator creator(&tx, index, utxo.nValue, sighash);
+        MutableTransactionSignatureCreator creator(&tx, index, utxo.nValue, txdata, sighash);
         sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata);
     }
+
+    // If we are not finalizing, set sigdata.complete to false to not set the scriptSig
+    if (!finalize && sigdata.complete) sigdata.complete = false;
+
     input.FromSignatureData(sigdata);
 
     // Fill in the missing info
@@ -258,8 +293,9 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx)
     //   PartiallySignedTransaction did not understand them), this will combine them into a final
     //   script.
     bool complete = true;
+    const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx);
     for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
-        complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, SIGHASH_ALL);
+        complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, SIGHASH_ALL, nullptr, true);
     }
 
     return complete;
@@ -331,3 +367,11 @@ bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data,
     }
     return true;
 }
+
+uint32_t PartiallySignedTransaction::GetVersion() const
+{
+    if (m_version != std::nullopt) {
+        return *m_version;
+    }
+    return 0;
+}