wallet: remove std::optional when PSBTResult is returned

kevkevinpal · kevkevinpal · commit 748ac3b42c55 · 2025-07-13T10:19:03.000-04:00
PSBTResult has an enum value of OK, so it makes sense to return OK
insead of a {}. This change makes it so we check for an OK value instead
of null/{}
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -205,7 +205,7 @@ class Wallet
         int& num_blocks) = 0;
 
     //! Fill PSBT.
-    virtual std::optional<common::PSBTResult> fillPSBT(std::optional<int> sighash_type,
+    virtual common::PSBTResult fillPSBT(std::optional<int> sighash_type,
         bool sign,
         bool bip32derivs,
         size_t* n_signed,
diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp
@@ -60,9 +60,9 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx)
     if (m_wallet_model) {
         size_t n_could_sign;
         const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
-        if (err) {
+        if (err != PSBTResult::OK) {
             showStatus(tr("Failed to load transaction: %1")
-                           .arg(QString::fromStdString(PSBTResultString(*err).translated)),
+                           .arg(QString::fromStdString(PSBTResultString(err).translated)),
                        StatusLevel::ERR);
             return;
         }
@@ -85,9 +85,9 @@ void PSBTOperationsDialog::signTransaction()
 
     const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
 
-    if (err) {
+    if (err != PSBTResult::OK) {
         showStatus(tr("Failed to sign transaction: %1")
-            .arg(QString::fromStdString(PSBTResultString(*err).translated)), StatusLevel::ERR);
+            .arg(QString::fromStdString(PSBTResultString(err).translated)), StatusLevel::ERR);
         return;
     }
 
@@ -253,7 +253,7 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p
     bool complete;
     const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
 
-    if (err) {
+    if (err != PSBTResult::OK) {
         return 0;
     }
     return n_signed;
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
@@ -448,7 +448,7 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
 }
 
 bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) {
-    std::optional<PSBTResult> result;
+    PSBTResult result;
     try {
         result = model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
     } catch (const std::runtime_error& e) {
@@ -467,7 +467,7 @@ bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx,
         QMessageBox::critical(nullptr, msg, msg);
         return false;
     }
-    if (result) {
+    if (result != PSBTResult::OK) {
         qWarning() << "Failed to sign PSBT";
         processSendCoinsReturn(WalletModel::TransactionCreationFailed);
         return false;
diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp
@@ -79,7 +79,7 @@ util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin
 }
 
 // If sign is true, transaction must previously have been filled
-std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
+PSBTResult ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
 {
     if (!sign) {
         return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize);
@@ -91,7 +91,7 @@ std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigne
         // TODO: for multisig wallets, we should only care if all _our_ inputs are signed
         complete &= PSBTInputSigned(input);
     }
-    if (complete) return {};
+    if (complete) return PSBTResult::OK;
 
     auto signer{GetExternalSigner()};
     if (!signer) {
@@ -105,6 +105,6 @@ std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigne
         return PSBTResult::EXTERNAL_SIGNER_FAILED;
     }
     if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup
-    return {};
+    return PSBTResult::OK;
 }
 } // namespace wallet
diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h
@@ -36,7 +36,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
   */
  util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const;
 
-  std::optional<common::PSBTResult> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
+  common::PSBTResult FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
 };
 } // namespace wallet
 #endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -340,7 +340,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
         bool complete;
         wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true);
         auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
-        if (err) return false;
+        if (err != PSBTResult::OK) return false;
         complete = FinalizeAndExtractPSBT(psbtx, mtx);
         return complete;
     } else {
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
@@ -383,7 +383,7 @@ class WalletImpl : public Wallet
         }
         return {};
     }
-    std::optional<PSBTResult> fillPSBT(std::optional<int> sighash_type,
+    PSBTResult fillPSBT(std::optional<int> sighash_type,
         bool sign,
         bool bip32derivs,
         size_t* n_signed,
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
@@ -102,8 +102,8 @@ static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const
     bool complete;
     pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true);
     const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
-    if (err) {
-        throw JSONRPCPSBTError(*err);
+    if (err != PSBTResult::OK) {
+        throw JSONRPCPSBTError(err);
     }
 
     CMutableTransaction mtx;
@@ -1171,7 +1171,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
         PartiallySignedTransaction psbtx(mtx);
         bool complete = false;
         const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true)};
-        CHECK_NONFATAL(!err);
+        CHECK_NONFATAL(err == PSBTResult::OK);
         CHECK_NONFATAL(!complete);
         DataStream ssTx{};
         ssTx << psbtx;
@@ -1631,8 +1631,8 @@ RPCHelpMan walletprocesspsbt()
     if (sign) EnsureWalletIsUnlocked(*pwallet);
 
     const auto err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs, nullptr, finalize)};
-    if (err) {
-        throw JSONRPCPSBTError(*err);
+    if (err != PSBTResult::OK) {
+        throw JSONRPCPSBTError(err);
     }
 
     UniValue result(UniValue::VOBJ);
@@ -1770,8 +1770,8 @@ RPCHelpMan walletcreatefundedpsbt()
     bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
     bool complete = true;
     const auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
-    if (err) {
-        throw JSONRPCPSBTError(*err);
+    if (err != PSBTResult::OK) {
+        throw JSONRPCPSBTError(err);
     }
 
     // Serialize the PSBT
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
@@ -1303,7 +1303,7 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
     return SigningResult::OK;
 }
 
-std::optional<PSBTResult> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
+PSBTResult DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
 {
     if (n_signed) {
         *n_signed = 0;
@@ -1396,7 +1396,7 @@ std::optional<PSBTResult> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTra
         UpdatePSBTOutput(HidingSigningProvider(keys.get(), /*hide_secret=*/true, /*hide_origin=*/!bip32derivs), psbtx, i);
     }
 
-    return {};
+    return PSBTResult::OK;
 }
 
 std::unique_ptr<CKeyMetadata> DescriptorScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -145,7 +145,7 @@ class ScriptPubKeyMan
     /** Sign a message with the given script */
     virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; };
     /** Adds script and derivation path information to a PSBT, and optionally signs it. */
-    virtual std::optional<common::PSBTResult> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTResult::UNSUPPORTED; }
+    virtual common::PSBTResult FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTResult::UNSUPPORTED; }
 
     virtual uint256 GetID() const { return uint256(); }
 
@@ -373,7 +373,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
 
     bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
     SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
-    std::optional<common::PSBTResult> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
+    common::PSBTResult FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
 
     uint256 GetID() const override;
 
diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp
@@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
 
     // Fill transaction with our data
     bool complete = true;
-    BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, std::nullopt, false, true));
+    BOOST_REQUIRE(m_wallet.FillPSBT(psbtx, complete, std::nullopt, false, true) == PSBTResult::OK);
 
     // Get the final tx
     DataStream ssTx{};
@@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
 
     // Try to sign the mutated input
     SignatureData sigdata;
-    BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, std::nullopt, true, true));
+    BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, std::nullopt, true, true) != PSBTResult::OK);
 }
 
 BOOST_AUTO_TEST_CASE(parse_hd_keypath)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -2090,7 +2090,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint,
     return false;
 }
 
-std::optional<PSBTResult> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, std::optional<int> sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
+PSBTResult CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, std::optional<int> sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
 {
     if (n_signed) {
         *n_signed = 0;
@@ -2124,7 +2124,7 @@ std::optional<PSBTResult> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, b
     for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) {
         int n_signed_this_spkm = 0;
         const auto error{spk_man->FillPSBT(psbtx, txdata, sighash_type, sign, bip32derivs, &n_signed_this_spkm, finalize)};
-        if (error) {
+        if (error != PSBTResult::OK) {
             return error;
         }
 
@@ -2141,7 +2141,7 @@ std::optional<PSBTResult> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, b
         complete &= PSBTInputSignedAndVerified(psbtx, i, &txdata);
     }
 
-    return {};
+    return PSBTResult::OK;
 }
 
 SigningResult CWallet::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -681,7 +681,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      * @param[in] finalize whether to create the final scriptSig or scriptWitness if possible
      * return error
      */
-    std::optional<common::PSBTResult> FillPSBT(PartiallySignedTransaction& psbtx,
+    common::PSBTResult FillPSBT(PartiallySignedTransaction& psbtx,
                   bool& complete,
                   std::optional<int> sighash_type = std::nullopt,
                   bool sign = true,

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`// If sign is true, transaction must previously have been filled`
`82`		`-std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const`
	`82`	`+PSBTResult ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const`
`83`	`83`	`{`
`84`	`84`	`if (!sign) {`
`85`	`85`	`return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize);`
`@@ -91,7 +91,7 @@ std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigne`
`91`	`91`	`// TODO: for multisig wallets, we should only care if all _our_ inputs are signed`
`92`	`92`	`complete &= PSBTInputSigned(input);`
`93`	`93`	`}`
`94`		`- if (complete) return {};`
	`94`	`+ if (complete) return PSBTResult::OK;`
`95`	`95`
`96`	`96`	`auto signer{GetExternalSigner()};`
`97`	`97`	`if (!signer) {`
`@@ -105,6 +105,6 @@ std::optional<PSBTResult> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySigne`
`105`	`105`	`return PSBTResult::EXTERNAL_SIGNER_FAILED;`
`106`	`106`	`}`
`107`	`107`	`if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup`
`108`		`- return {};`
	`108`	`+ return PSBTResult::OK;`
`109`	`109`	`}`
`110`	`110`	`} // namespace wallet`
Original file line number	Diff line number	Diff line change
`@@ -383,7 +383,7 @@ class WalletImpl : public Wallet`
`383`	`383`	`}`
`384`	`384`	`return {};`
`385`	`385`	`}`
`386`		`- std::optional<PSBTResult> fillPSBT(std::optional<int> sighash_type,`
	`386`	`+ PSBTResult fillPSBT(std::optional<int> sighash_type,`
`387`	`387`	`bool sign,`
`388`	`388`	`bool bip32derivs,`
`389`	`389`	`size_t* n_signed,`
Original file line number	Diff line number	Diff line change
`@@ -1303,7 +1303,7 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,`
`1303`	`1303`	`return SigningResult::OK;`
`1304`	`1304`	`}`
`1305`	`1305`
`1306`		`-std::optional<PSBTResult> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const`
	`1306`	`+PSBTResult DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const`
`1307`	`1307`	`{`
`1308`	`1308`	`if (n_signed) {`
`1309`	`1309`	`*n_signed = 0;`
`@@ -1396,7 +1396,7 @@ std::optional<PSBTResult> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTra`
`1396`	`1396`	`UpdatePSBTOutput(HidingSigningProvider(keys.get(), /hide_secret=/true, /hide_origin=/!bip32derivs), psbtx, i);`
`1397`	`1397`	`}`
`1398`	`1398`
`1399`		`- return {};`
	`1399`	`+ return PSBTResult::OK;`
`1400`	`1400`	`}`
`1401`	`1401`
`1402`	`1402`	`std::unique_ptr<CKeyMetadata> DescriptorScriptPubKeyMan::GetMetadata(const CTxDestination& dest) const`