From 156f49d682ef025fb942c997a6c5475e18eef9cf Mon Sep 17 00:00:00 2001
From: Ryan Ofsky <ryan@ofsky.org>
Date: Mon, 12 Sep 2022 11:09:06 -0400
Subject: [PATCH] interfaces: Change getUnspentOutput return type to avoid
 multiprocess segfault

Coin serialize method segfaults if IsSpent condition is true. This caused
multiprocess code to segfault when serializing the Coin& output argument to of
the Node::getUnspentOutput method if the coin was not found. Segfault could be
triggered by double clicking and viewing transaction details in the GUI
transaction list.

Fix this by replacing Coin& output argument with optional<Coin> return value to
avoid trying to serializing spent coins.
---
 src/interfaces/node.h      | 4 ++--
 src/node/interfaces.cpp    | 6 ++++--
 src/qt/transactiondesc.cpp | 6 ++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 3f8df571249..aeb2612c078 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -204,8 +204,8 @@ public:
     //! Unset RPC timer interface.
     virtual void rpcUnsetTimerInterface(RPCTimerInterface* iface) = 0;
 
-    //! Get unspent outputs associated with a transaction.
-    virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0;
+    //! Get unspent output associated with a transaction.
+    virtual std::optional<Coin> getUnspentOutput(const COutPoint& output) = 0;
 
     //! Broadcast transaction.
     virtual TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) = 0;
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f6dbe4f0087..944c26f00a8 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -328,10 +328,12 @@ public:
     std::vector<std::string> listRpcCommands() override { return ::tableRPC.listCommands(); }
     void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); }
     void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); }
-    bool getUnspentOutput(const COutPoint& output, Coin& coin) override
+    std::optional<Coin> getUnspentOutput(const COutPoint& output) override
     {
         LOCK(::cs_main);
-        return chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin);
+        Coin coin;
+        if (chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin)) return coin;
+        return {};
     }
     TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) override
     {
diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
index dae6a2dea97..34daa8493a9 100644
--- a/src/qt/transactiondesc.cpp
+++ b/src/qt/transactiondesc.cpp
@@ -360,12 +360,10 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
         {
             COutPoint prevout = txin.prevout;
 
-            Coin prev;
-            if(node.getUnspentOutput(prevout, prev))
-            {
+            if (auto prev{node.getUnspentOutput(prevout)}) {
                 {
                     strHTML += "<li>";
-                    const CTxOut &vout = prev.out;
+                    const CTxOut& vout = prev->out;
                     CTxDestination address;
                     if (ExtractDestination(vout.scriptPubKey, address))
                     {