Merge #17524: psbt: handle unspendable psbts

773d4572a4 Mark PSBTs spending unspendable outputs as invalid in analysis (Andrew Chow)
638e40cb60 Have a PSBTAnalysis state that indicates invalid PSBT (Andrew Chow)

Pull request description:

  When analyzing an unspendable PSBT, report that it is unspendable and exit analysis early.

ACKs for top commit:
  Sjors:
    ACK 773d457
  instagibbs:
    After some thought ACK 773d4572a4

Tree-SHA512: 99b0cb2fa1ea37593fc65a20effe881639d69ddeeecf5197bc87bc7f2220cbeb40f1d429d517e4d27f2e9fb563a00cd845d2b4b1ce05246a75a6cb56fb9b0ba5

src/node/psbt.cpp

@@ -7,6 +7,7 @@
 #include <node/psbt.h>
 #include <policy/policy.h>
 #include <policy/settings.h>
+#include <tinyformat.h>
 
 #include <numeric>
 
@@ -39,6 +40,11 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
             calc_fee = false;
         }
 
+        if (!utxo.IsNull() && utxo.scriptPubKey.IsUnspendable()) {
+            result.SetInvalid(strprintf("PSBT is not valid. Input %u spends unspendable output", i));
+            return result;
+        }
+
         // Check if it is final
         if (!utxo.IsNull() && !PSBTInputSigned(input)) {
             input_analysis.is_final = false;

src/node/psbt.h

@@ -30,6 +30,17 @@ struct PSBTAnalysis {
     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 error;                     //!< Error message
+
+    void SetInvalid(std::string err_msg)
+    {
+        estimated_vsize = nullopt;
+        estimated_feerate = nullopt;
+        fee = nullopt;
+        inputs.clear();
+        next = PSBTRole::CREATOR;
+        error = err_msg;
+    }
 };
 
 /**

src/psbt.cpp

@@ -348,6 +348,7 @@ TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector
 
 std::string PSBTRoleName(PSBTRole role) {
     switch (role) {
+    case PSBTRole::CREATOR: return "creator";
     case PSBTRole::UPDATER: return "updater";
     case PSBTRole::SIGNER: return "signer";
     case PSBTRole::FINALIZER: return "finalizer";

src/psbt.h

@@ -560,6 +560,7 @@ struct PartiallySignedTransaction
 };
 
 enum class PSBTRole {
+    CREATOR,
     UPDATER,
     SIGNER,
     FINALIZER,

src/rpc/rawtransaction.cpp

@@ -1674,6 +1674,7 @@ UniValue analyzepsbt(const JSONRPCRequest& request)
                 "  \"estimated_feerate\" : feerate   (numeric, optional) Estimated feerate of the final signed transaction in " + CURRENCY_UNIT + "/kB. Shown only if all UTXO slots in the PSBT have been filled.\n"
                 "  \"fee\" : fee                     (numeric, optional) The transaction fee paid. Shown only if all UTXO slots in the PSBT have been filled.\n"
                 "  \"next\" : \"role\"                 (string) Role of the next person that this psbt needs to go to\n"
+                "  \"error\" : \"error\"               (string) Error message if there is one"
                 "}\n"
             },
             RPCExamples {
@@ -1726,7 +1727,7 @@ UniValue analyzepsbt(const JSONRPCRequest& request)
         }
         inputs_result.push_back(input_univ);
     }
-    result.pushKV("inputs", inputs_result);
+    if (!inputs_result.empty()) result.pushKV("inputs", inputs_result);
 
     if (psbta.estimated_vsize != nullopt) {
         result.pushKV("estimated_vsize", (int)*psbta.estimated_vsize);
@@ -1738,6 +1739,9 @@ UniValue analyzepsbt(const JSONRPCRequest& request)
         result.pushKV("fee", ValueFromAmount(*psbta.fee));
     }
     result.pushKV("next", PSBTRoleName(psbta.next));
+    if (!psbta.error.empty()) {
+        result.pushKV("error", psbta.error);
+    }
 
     return result;
 }

test/functional/rpc_psbt.py

@@ -417,5 +417,10 @@ class PSBTTest(BitcoinTestFramework):
         analyzed = self.nodes[0].analyzepsbt(signed)
         assert analyzed['inputs'][0]['has_utxo'] and analyzed['inputs'][0]['is_final'] and analyzed['next'] == 'extractor'
 
+        self.log.info("PSBT spending unspendable outputs should have error message and Creator as next")
+        analysis = self.nodes[0].analyzepsbt('cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWAEHYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFv8/wADXYP/7//////8JxOh0LR2HAI8AAAAAAAEBIADC6wsAAAAAF2oUt/X69ELjeX2nTof+fZ10l+OyAokDAQcJAwEHEAABAACAAAEBIADC6wsAAAAAF2oUt/X69ELjeX2nTof+fZ10l+OyAokDAQcJAwEHENkMak8AAAAA')
+        assert_equal(analysis['next'], 'creator')
+        assert_equal(analysis['error'], 'PSBT is not valid. Input 0 spends unspendable output')
+
 if __name__ == '__main__':
     PSBTTest().main()