Merge #15508: Refactor analyzepsbt for use outside RPC code

892eff05f1 Add documentation of struct PSBTAnalysis et al (Glenn Willen)
ef22fe8c1f Refactor analyzepsbt for use outside RPC code (Glenn Willen)
afd20a25f2 Move PSBT decoding functions from core_io to psbt.cpp (Glenn Willen)

Pull request description:

  Refactor the analyzepsbt RPC into (1) an AnalyzePSBT function, which returns
  its output as a new strongly-typed PSBTAnalysis struct, and (2) a thin wrapper
  which converts the struct into a UniValue for RPC use.

  ----

  As with my previous refactoring PR, I need this because I am creating a dependency on this code from the GUI. Per discussion in #bitcoin-core-dev on IRC, since we don't want to create a dependency on UniValue in anything outside RPC, I introduced some new structs to hold the info we get when analyzing a PSBT. For the field types, I used whatever types are already used internally for this data (e.g. CAmount, CFeeRate, CKeyID), and only convert to int/string etc. in the wrapper.

  @achow101, maybe take the first look? :-)

ACKs for commit 892eff:
  sipa:
    utACK 892eff05f1
  achow101:
    utACK 892eff05f1
  ryanofsky:
    utACK 892eff05f1. Just small cleanups since the last review: removing unneeded include, forward decl, adding const ref

Tree-SHA512: eb278b0a82717ebc3eb0c08dc5bb4eefb996a317a6a3a8ecf51cd88110ddbb188ad3482cdd9563e557995e73aca5a282c1f6e352bc598155f1203b7b46fe5dee

src/Makefile.am

@@ -434,8 +434,8 @@ libbitcoin_common_a_SOURCES = \
   netaddress.cpp \
   netbase.cpp \
   policy/feerate.cpp \
-  psbt.cpp \
   protocol.cpp \
+  psbt.cpp \
   scheduler.cpp \
   script/descriptor.cpp \
   script/ismine.cpp \

src/core_io.h

@@ -16,7 +16,6 @@ class CBlockHeader;
 class CScript;
 class CTransaction;
 struct CMutableTransaction;
-struct PartiallySignedTransaction;
 class uint256;
 class UniValue;
 
@@ -37,11 +36,6 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header);
  */
 bool ParseHashStr(const std::string& strHex, uint256& result);
 std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName);
-
-//! Decode a base64ed PSBT into a PartiallySignedTransaction
-NODISCARD bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error);
-//! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction
-NODISCARD bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error);
 int ParseSighashString(const UniValue& sighash);
 
 // core_write.cpp

src/core_read.cpp

@@ -4,7 +4,6 @@
 
 #include <core_io.h>
 
-#include <psbt.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
@@ -177,33 +176,6 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk)
     return true;
 }
 
-bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error)
-{
-    bool invalid;
-    std::string tx_data = DecodeBase64(base64_tx, &invalid);
-    if (invalid) {
-        error = "invalid base64";
-        return false;
-    }
-    return DecodeRawPSBT(psbt, tx_data, error);
-}
-
-bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)
-{
-    CDataStream ss_data(tx_data.data(), tx_data.data() + tx_data.size(), SER_NETWORK, PROTOCOL_VERSION);
-    try {
-        ss_data >> psbt;
-        if (!ss_data.empty()) {
-            error = "extra data after PSBT";
-            return false;
-        }
-    } catch (const std::exception& e) {
-        error = e.what();
-        return false;
-    }
-    return true;
-}
-
 bool ParseHashStr(const std::string& strHex, uint256& result)
 {
     if ((strHex.size() != 64) || !IsHex(strHex))

src/psbt.cpp

@@ -2,9 +2,14 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <coins.h>
+#include <consensus/tx_verify.h>
+#include <policy/policy.h>
 #include <psbt.h>
 #include <util/strencodings.h>
 
+#include <numeric>
+
 PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx) : tx(tx)
 {
     inputs.resize(tx.vin.size());
@@ -205,7 +210,7 @@ void PSBTOutput::Merge(const PSBTOutput& output)
     if (redeem_script.empty() && !output.redeem_script.empty()) redeem_script = output.redeem_script;
     if (witness_script.empty() && !output.witness_script.empty()) witness_script = output.witness_script;
 }
-bool PSBTInputSigned(PSBTInput& input)
+bool PSBTInputSigned(const PSBTInput& input)
 {
     return !input.final_script_sig.empty() || !input.final_script_witness.IsNull();
 }
@@ -325,3 +330,162 @@ TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector
 
     return TransactionError::OK;
 }
+
+std::string PSBTRoleName(PSBTRole role) {
+    switch (role) {
+    case PSBTRole::UPDATER: return "updater";
+    case PSBTRole::SIGNER: return "signer";
+    case PSBTRole::FINALIZER: return "finalizer";
+    case PSBTRole::EXTRACTOR: return "extractor";
+    }
+}
+
+PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
+{
+    // Go through each input and build status
+    PSBTAnalysis result;
+
+    bool calc_fee = true;
+    bool all_final = true;
+    bool only_missing_sigs = true;
+    bool only_missing_final = false;
+    CAmount in_amt = 0;
+
+    result.inputs.resize(psbtx.tx->vin.size());
+
+    for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
+        PSBTInput& input = psbtx.inputs[i];
+        PSBTInputAnalysis& input_analysis = result.inputs[i];
+
+        // Check for a UTXO
+        CTxOut utxo;
+        if (psbtx.GetInputUTXO(utxo, i)) {
+            in_amt += utxo.nValue;
+            input_analysis.has_utxo = true;
+        } else {
+            input_analysis.has_utxo = false;
+            input_analysis.is_final = false;
+            input_analysis.next = PSBTRole::UPDATER;
+            calc_fee = false;
+        }
+
+        // Check if it is final
+        if (!utxo.IsNull() && !PSBTInputSigned(input)) {
+            input_analysis.is_final = false;
+            all_final = false;
+
+            // Figure out what is missing
+            SignatureData outdata;
+            bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, &outdata);
+
+            // Things are missing
+            if (!complete) {
+                input_analysis.missing_pubkeys = outdata.missing_pubkeys;
+                input_analysis.missing_redeem_script = outdata.missing_redeem_script;
+                input_analysis.missing_witness_script = outdata.missing_witness_script;
+                input_analysis.missing_sigs = outdata.missing_sigs;
+
+                // If we are only missing signatures and nothing else, then next is signer
+                if (outdata.missing_pubkeys.empty() && outdata.missing_redeem_script.IsNull() && outdata.missing_witness_script.IsNull() && !outdata.missing_sigs.empty()) {
+                    input_analysis.next = PSBTRole::SIGNER;
+                } else {
+                    only_missing_sigs = false;
+                    input_analysis.next = PSBTRole::UPDATER;
+                }
+            } else {
+                only_missing_final = true;
+                input_analysis.next = PSBTRole::FINALIZER;
+            }
+        } else if (!utxo.IsNull()){
+            input_analysis.is_final = true;
+        }
+    }
+
+    if (all_final) {
+        only_missing_sigs = false;
+        result.next = PSBTRole::EXTRACTOR;
+    }
+    if (calc_fee) {
+        // Get the output amount
+        CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0),
+            [](CAmount a, const CTxOut& b) {
+                return a += b.nValue;
+            }
+        );
+
+        // Get the fee
+        CAmount fee = in_amt - out_amt;
+        result.fee = fee;
+
+        // Estimate the size
+        CMutableTransaction mtx(*psbtx.tx);
+        CCoinsView view_dummy;
+        CCoinsViewCache view(&view_dummy);
+        bool success = true;
+
+        for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
+            PSBTInput& input = psbtx.inputs[i];
+            Coin newcoin;
+
+            if (!SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, nullptr, true) || !psbtx.GetInputUTXO(newcoin.out, i)) {
+                success = false;
+                break;
+            } else {
+                mtx.vin[i].scriptSig = input.final_script_sig;
+                mtx.vin[i].scriptWitness = input.final_script_witness;
+                newcoin.nHeight = 1;
+                view.AddCoin(psbtx.tx->vin[i].prevout, std::move(newcoin), true);
+            }
+        }
+
+        if (success) {
+            CTransaction ctx = CTransaction(mtx);
+            size_t size = GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS));
+            result.estimated_vsize = size;
+            // Estimate fee rate
+            CFeeRate feerate(fee, size);
+            result.estimated_feerate = feerate;
+        }
+
+        if (only_missing_sigs) {
+            result.next = PSBTRole::SIGNER;
+        } else if (only_missing_final) {
+            result.next = PSBTRole::FINALIZER;
+        } else if (all_final) {
+            result.next = PSBTRole::EXTRACTOR;
+        } else {
+            result.next = PSBTRole::UPDATER;
+        }
+    } else {
+        result.next = PSBTRole::UPDATER;
+    }
+
+    return result;
+}
+
+bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error)
+{
+    bool invalid;
+    std::string tx_data = DecodeBase64(base64_tx, &invalid);
+    if (invalid) {
+        error = "invalid base64";
+        return false;
+    }
+    return DecodeRawPSBT(psbt, tx_data, error);
+}
+
+bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)
+{
+    CDataStream ss_data(tx_data.data(), tx_data.data() + tx_data.size(), SER_NETWORK, PROTOCOL_VERSION);
+    try {
+        ss_data >> psbt;
+        if (!ss_data.empty()) {
+            error = "extra data after PSBT";
+            return false;
+        }
+    } catch (const std::exception& e) {
+        error = e.what();
+        return false;
+    }
+    return true;
+}

src/psbt.h

@@ -7,6 +7,8 @@
 
 #include <attributes.h>
 #include <node/transaction.h>
+#include <optional.h>
+#include <policy/feerate.h>
 #include <primitives/transaction.h>
 #include <pubkey.h>
 #include <script/sign.h>
@@ -548,8 +550,42 @@ struct PartiallySignedTransaction
     }
 };
 
+enum class PSBTRole {
+    UPDATER,
+    SIGNER,
+    FINALIZER,
+    EXTRACTOR
+};
+
+/**
+ * Holds an analysis of one input from a PSBT
+ */
+struct PSBTInputAnalysis {
+    bool has_utxo; //!< Whether we have UTXO information for this input
+    bool is_final; //!< Whether the input has all required information including signatures
+    PSBTRole next; //!< Which of the BIP 174 roles needs to handle this input next
+
+    std::vector<CKeyID> missing_pubkeys; //!< Pubkeys whose BIP32 derivation path is missing
+    std::vector<CKeyID> missing_sigs;    //!< Pubkeys whose signatures are missing
+    uint160 missing_redeem_script;       //!< Hash160 of redeem script, if missing
+    uint256 missing_witness_script;      //!< SHA256 of witness script, if missing
+};
+
+/**
+ * Holds the results of AnalyzePSBT (miscellaneous information about a PSBT)
+ */
+struct PSBTAnalysis {
+    Optional<size_t> estimated_vsize;      //!< Estimated weight of the transaction
+    Optional<CFeeRate> estimated_feerate;  //!< Estimated feerate (fee / weight) of the transaction
+    Optional<CAmount> fee;                 //!< Amount of fee being paid by the transaction
+    std::vector<PSBTInputAnalysis> inputs; //!< More information about the individual inputs of the transaction
+    PSBTRole next;                         //!< Which of the BIP 174 roles needs to handle the transaction next
+};
+
+std::string PSBTRoleName(PSBTRole role);
+
 /** Checks whether a PSBTInput is already signed. */
-bool PSBTInputSigned(PSBTInput& input);
+bool PSBTInputSigned(const PSBTInput& input);
 
 /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */
 bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false);
@@ -580,4 +616,17 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
  */
 NODISCARD TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs);
 
+/**
+ * Provides helpful miscellaneous information about where a PSBT is in the signing workflow.
+ *
+ * @param[in] psbtx the PSBT to analyze
+ * @return A PSBTAnalysis with information about the provided PSBT.
+ */
+PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx);
+
+//! Decode a base64ed PSBT into a PartiallySignedTransaction
+NODISCARD bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error);
+//! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction
+NODISCARD bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error);
+
 #endif // BITCOIN_PSBT_H

src/rpc/rawtransaction.cpp

@@ -1943,148 +1943,56 @@ UniValue analyzepsbt(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error));
     }
 
-    // Go through each input and build status
+    PSBTAnalysis psbta = AnalyzePSBT(psbtx);
+
     UniValue result(UniValue::VOBJ);
     UniValue inputs_result(UniValue::VARR);
-    bool calc_fee = true;
-    bool all_final = true;
-    bool only_missing_sigs = true;
-    bool only_missing_final = false;
-    CAmount in_amt = 0;
-    for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
-        PSBTInput& input = psbtx.inputs[i];
+    for (const auto& input : psbta.inputs) {
         UniValue input_univ(UniValue::VOBJ);
         UniValue missing(UniValue::VOBJ);
 
-        // Check for a UTXO
-        CTxOut utxo;
-        if (psbtx.GetInputUTXO(utxo, i)) {
-            in_amt += utxo.nValue;
-            input_univ.pushKV("has_utxo", true);
-        } else {
-            input_univ.pushKV("has_utxo", false);
-            input_univ.pushKV("is_final", false);
-            input_univ.pushKV("next", "updater");
-            calc_fee = false;
-        }
+        input_univ.pushKV("has_utxo", input.has_utxo);
+        input_univ.pushKV("is_final", input.is_final);
+        input_univ.pushKV("next", PSBTRoleName(input.next));
 
-        // Check if it is final
-        if (!utxo.IsNull() && !PSBTInputSigned(input)) {
-            input_univ.pushKV("is_final", false);
-            all_final = false;
-
-            // Figure out what is missing
-            SignatureData outdata;
-            bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, &outdata);
-
-            // Things are missing
-            if (!complete) {
-                if (!outdata.missing_pubkeys.empty()) {
-                    // Missing pubkeys
-                    UniValue missing_pubkeys_univ(UniValue::VARR);
-                    for (const CKeyID& pubkey : outdata.missing_pubkeys) {
-                        missing_pubkeys_univ.push_back(HexStr(pubkey));
-                    }
-                    missing.pushKV("pubkeys", missing_pubkeys_univ);
-                }
-                if (!outdata.missing_redeem_script.IsNull()) {
-                    // Missing redeemScript
-                    missing.pushKV("redeemscript", HexStr(outdata.missing_redeem_script));
-                }
-                if (!outdata.missing_witness_script.IsNull()) {
-                    // Missing witnessScript
-                    missing.pushKV("witnessscript", HexStr(outdata.missing_witness_script));
-                }
-                if (!outdata.missing_sigs.empty()) {
-                    // Missing sigs
-                    UniValue missing_sigs_univ(UniValue::VARR);
-                    for (const CKeyID& pubkey : outdata.missing_sigs) {
-                        missing_sigs_univ.push_back(HexStr(pubkey));
-                    }
-                    missing.pushKV("signatures", missing_sigs_univ);
-                }
-                input_univ.pushKV("missing", missing);
-
-                // If we are only missing signatures and nothing else, then next is signer
-                if (outdata.missing_pubkeys.empty() && outdata.missing_redeem_script.IsNull() && outdata.missing_witness_script.IsNull() && !outdata.missing_sigs.empty()) {
-                    input_univ.pushKV("next", "signer");
-                } else {
-                    only_missing_sigs = false;
-                    input_univ.pushKV("next", "updater");
-                }
-            } else {
-                only_missing_final = true;
-                input_univ.pushKV("next", "finalizer");
+        if (!input.missing_pubkeys.empty()) {
+            UniValue missing_pubkeys_univ(UniValue::VARR);
+            for (const CKeyID& pubkey : input.missing_pubkeys) {
+                missing_pubkeys_univ.push_back(HexStr(pubkey));
             }
-        } else if (!utxo.IsNull()){
-            input_univ.pushKV("is_final", true);
+            missing.pushKV("pubkeys", missing_pubkeys_univ);
+        }
+        if (!input.missing_redeem_script.IsNull()) {
+            missing.pushKV("redeemscript", HexStr(input.missing_redeem_script));
+        }
+        if (!input.missing_witness_script.IsNull()) {
+            missing.pushKV("witnessscript", HexStr(input.missing_witness_script));
+        }
+        if (!input.missing_sigs.empty()) {
+            UniValue missing_sigs_univ(UniValue::VARR);
+            for (const CKeyID& pubkey : input.missing_sigs) {
+                missing_sigs_univ.push_back(HexStr(pubkey));
+            }
+            missing.pushKV("signatures", missing_sigs_univ);
+        }
+        if (!missing.getKeys().empty()) {
+            input_univ.pushKV("missing", missing);
         }
         inputs_result.push_back(input_univ);
     }
     result.pushKV("inputs", inputs_result);
 
-    if (all_final) {
-        only_missing_sigs = false;
-        result.pushKV("next", "extractor");
+    if (psbta.estimated_vsize != nullopt) {
+        result.pushKV("estimated_vsize", (int)*psbta.estimated_vsize);
     }
-    if (calc_fee) {
-        // Get the output amount
-        CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0),
-            [](CAmount a, const CTxOut& b) {
-                return a += b.nValue;
-            }
-        );
-
-        // Get the fee
-        CAmount fee = in_amt - out_amt;
-
-        // Estimate the size
-        CMutableTransaction mtx(*psbtx.tx);
-        CCoinsView view_dummy;
-        CCoinsViewCache view(&view_dummy);
-        bool success = true;
-
-        for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
-            PSBTInput& input = psbtx.inputs[i];
-            if (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, nullptr, true)) {
-                mtx.vin[i].scriptSig = input.final_script_sig;
-                mtx.vin[i].scriptWitness = input.final_script_witness;
-
-                Coin newcoin;
-                if (!psbtx.GetInputUTXO(newcoin.out, i)) {
-                    success = false;
-                    break;
-                }
-                newcoin.nHeight = 1;
-                view.AddCoin(psbtx.tx->vin[i].prevout, std::move(newcoin), true);
-            } else {
-                success = false;
-                break;
-            }
-        }
-
-        if (success) {
-            CTransaction ctx = CTransaction(mtx);
-            size_t size = GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS));
-            result.pushKV("estimated_vsize", (int)size);
-            // Estimate fee rate
-            CFeeRate feerate(fee, size);
-            result.pushKV("estimated_feerate", ValueFromAmount(feerate.GetFeePerK()));
-        }
-        result.pushKV("fee", ValueFromAmount(fee));
-
-        if (only_missing_sigs) {
-            result.pushKV("next", "signer");
-        } else if (only_missing_final) {
-            result.pushKV("next", "finalizer");
-        } else if (all_final) {
-            result.pushKV("next", "extractor");
-        } else {
-            result.pushKV("next", "updater");
-        }
-    } else {
-        result.pushKV("next", "updater");
+    if (psbta.estimated_feerate != nullopt) {
+        result.pushKV("estimated_feerate", ValueFromAmount(psbta.estimated_feerate->GetFeePerK()));
     }
+    if (psbta.fee != nullopt) {
+        result.pushKV("fee", ValueFromAmount(*psbta.fee));
+    }
+    result.pushKV("next", PSBTRoleName(psbta.next));
+
     return result;
 }