qml: adapt wallet transaction creation API

Author: johnny9

qml/models/psbtqmlmodel.cpp

@@ -18,6 +18,7 @@
 #include <script/script.h>
 #include <script/solver.h>
 #include <streams.h>
+#include <util/result.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
 
@@ -76,19 +77,14 @@ std::optional<std::pair<int, int>> ExtractMultisigSigInfo(const CScript& script)
     return std::nullopt;
 }
 
-bool IsMultisigScript(const CScript& script)
-{
-    return ExtractMultisigSigInfo(script).has_value();
-}
-
-bool DecodePsbtFromBytes(const QByteArray& bytes, PartiallySignedTransaction& psbt, std::string& error)
+util::Result<PartiallySignedTransaction> DecodePsbtFromBytes(const QByteArray& bytes)
 {
     std::vector<std::byte> raw;
     raw.reserve(bytes.size());
     for (const char ch : bytes) {
         raw.push_back(static_cast<std::byte>(static_cast<unsigned char>(ch)));
     }
-    return DecodeRawPSBT(psbt, std::span<const std::byte>{raw.data(), raw.size()}, error);
+    return DecodeRawPSBT(std::span<const std::byte>{raw.data(), raw.size()});
 }
 
 } // namespace
@@ -113,19 +109,22 @@ QString PsbtQmlModel::LoadPsbtFromFile(const QString& path, PartiallySignedTrans
     }
 
     const QByteArray bytes{file.readAll()};
-    std::string error;
-    if (DecodePsbtFromBytes(bytes, psbt, error)) {
+    const auto raw_result{DecodePsbtFromBytes(bytes)};
+    if (raw_result) {
+        psbt = *raw_result;
         return {};
     }
 
-    psbt = PartiallySignedTransaction{};
-    std::string base64_error;
-    if (DecodeBase64PSBT(psbt, QString::fromUtf8(bytes).trimmed().toStdString(), base64_error)) {
+    const auto base64_result{DecodeBase64PSBT(QString::fromUtf8(bytes).trimmed().toStdString())};
+    if (base64_result) {
+        psbt = *base64_result;
         return {};
     }
 
-    const std::string& decode_error{base64_error == "invalid base64" ? error : base64_error};
-    return tr("Could not decode PSBT: %1").arg(QString::fromStdString(decode_error));
+    const bilingual_str raw_error{util::ErrorString(raw_result)};
+    const bilingual_str base64_error{util::ErrorString(base64_result)};
+    const bilingual_str& decode_error{base64_error.original == "invalid base64" ? raw_error : base64_error};
+    return tr("Could not decode PSBT: %1").arg(QString::fromStdString(decode_error.translated));
 }
 
 QByteArray PsbtQmlModel::SerializePsbtRaw(const PartiallySignedTransaction& psbt)
@@ -171,7 +170,7 @@ std::optional<std::pair<int, int>> PsbtQmlModel::MultisigPsbtInputSigInfo(const
     if (auto info{ExtractMultisigSigInfo(input.witness_script)}) return info;
 
     CTxOut utxo;
-    if (psbt.GetInputUTXO(utxo, index)) {
+    if (input.GetUTXO(utxo)) {
         if (auto info{ExtractMultisigSigInfo(utxo.scriptPubKey)}) return info;
     }
     return std::nullopt;
@@ -222,7 +221,8 @@ void PsbtQmlModel::setMatchedTxid(const QString& txid)
 
 QString PsbtQmlModel::loadFromFile(const QString& path)
 {
-    PartiallySignedTransaction psbt;
+    CMutableTransaction empty_tx;
+    PartiallySignedTransaction psbt{empty_tx};
     const QString error{LoadPsbtFromFile(path, psbt)};
     if (!error.isEmpty()) {
         setError(error);
@@ -248,7 +248,7 @@ void PsbtQmlModel::sign()
     bool complete{false};
     size_t signed_inputs{0};
     const std::optional<common::PSBTError> error{
-        m_wallet->fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &signed_inputs, *m_psbt, complete)};
+        m_wallet->fillPSBT({.sign = true, .bip32_derivs = true}, &signed_inputs, *m_psbt, complete)};
     if (error) {
         refreshState(tr("Could not sign PSBT: %1").arg(PsbtErrorText(*error)));
         return;
@@ -328,7 +328,7 @@ void PsbtQmlModel::refreshState(const QString& status_override)
     size_t could_sign{0};
     std::optional<common::PSBTError> fill_error;
     if (m_wallet) {
-        fill_error = m_wallet->fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &could_sign, *m_psbt, complete);
+        fill_error = m_wallet->fillPSBT({.sign = false, .bip32_derivs = true}, &could_sign, *m_psbt, complete);
     }
 
     m_error = fill_error ? PsbtErrorText(*fill_error) : QString();
@@ -361,13 +361,14 @@ void PsbtQmlModel::refreshState(const QString& status_override)
 QStringList PsbtQmlModel::buildSummary(const PartiallySignedTransaction& psbt) const
 {
     QStringList lines;
-    if (!psbt.tx) {
+    const auto unsigned_tx{psbt.GetUnsignedTx()};
+    if (!unsigned_tx) {
         lines << tr("PSBT does not contain an unsigned transaction.");
         return lines;
     }
 
     CAmount total{0};
-    for (const CTxOut& output : psbt.tx->vout) {
+    for (const CTxOut& output : unsigned_tx->vout) {
         total += output.nValue;
         CTxDestination destination;
         const QString address{ExtractDestination(output.scriptPubKey, destination) ? QString::fromStdString(EncodeDestination(destination)) : tr("unknown destination")};

qml/models/walletqmlmodel.cpp

@@ -192,14 +192,12 @@ std::optional<CAmount> TryPreviewFee(interfaces::Wallet& wallet,
                                      const std::vector<wallet::CRecipient>& recipients,
                                      const wallet::CCoinControl& coin_control)
 {
-    int change_position{-1};
-    CAmount fee{0};
-    const auto result = wallet.createTransaction(recipients, coin_control, /*sign=*/false, change_position, fee);
+    const auto result = wallet.createTransaction(recipients, coin_control, /*sign=*/false, /*change_pos=*/std::nullopt);
     if (!result) {
         return std::nullopt;
     }
 
-    return fee;
+    return result->fee;
 }
 
 std::optional<std::vector<wallet::CRecipient>> WithLargestRecipientPayingFee(const std::vector<wallet::CRecipient>& recipients)
@@ -1597,25 +1595,24 @@ bool WalletQmlModel::prepareTransactionInternal(std::optional<SecureString> pass
         return false;
     }
 
-    int nChangePosRet = -1;
-    CAmount nFeeRequired = 0;
     const bool sign = !m_wallet->privateKeysDisabled();
-    const auto& result = m_wallet->createTransaction(*vec_send, coin_control, sign, nChangePosRet, nFeeRequired);
+    const auto& result = m_wallet->createTransaction(*vec_send, coin_control, sign, /*change_pos=*/std::nullopt);
     if (result) {
         if (m_current_transaction) {
             delete m_current_transaction;
         }
-        const CTransactionRef& newTx = *result;
+        const CTransactionRef& newTx = result->tx;
         m_current_transaction = new WalletQmlModelTransaction(m_send_recipients, this);
         m_current_psbt.reset();
         m_current_transaction_source = CurrentTransactionSource::SendDraft;
         m_current_transaction_can_send = true;
         m_current_transaction_can_broadcast = false;
         m_current_transaction_review_message.clear();
         m_current_transaction->setWtx(newTx);
-        m_current_transaction->setTransactionFee(nFeeRequired);
+        m_current_transaction->setTransactionFee(result->fee);
         if (subtract_fee_from_amount) {
-            m_current_transaction->reassignAmounts(nChangePosRet);
+            m_current_transaction->reassignAmounts(
+                result->change_pos ? static_cast<int>(*result->change_pos) : -1);
         }
         m_current_transaction->setDisplayUnit(m_display_unit);
         relock_guard.relock();
@@ -1642,7 +1639,8 @@ void WalletQmlModel::approveExternalSignerTransaction()
     }
 
     try {
-        PartiallySignedTransaction psbtx;
+        CMutableTransaction empty_tx;
+        PartiallySignedTransaction psbtx{empty_tx};
         if (m_current_psbt) {
             psbtx = *m_current_psbt;
         } else {
@@ -1652,7 +1650,7 @@ void WalletQmlModel::approveExternalSignerTransaction()
         }
 
         bool complete{false};
-        const auto draft_err = m_wallet->fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true,
+        const auto draft_err = m_wallet->fillPSBT({.sign = false, .bip32_derivs = true},
             /*n_signed=*/nullptr, psbtx, complete);
         if (draft_err) {
             Q_EMIT externalSignerApprovalFailed(
@@ -1662,7 +1660,7 @@ void WalletQmlModel::approveExternalSignerTransaction()
         }
 
         if (!complete) {
-            const auto sign_err = m_wallet->fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true,
+            const auto sign_err = m_wallet->fillPSBT({.sign = true, .bip32_derivs = true},
                 /*n_signed=*/nullptr, psbtx, complete);
             if (sign_err) {
                 const bool signer_not_found = *sign_err == common::PSBTError::EXTERNAL_SIGNER_NOT_FOUND;
@@ -1813,7 +1811,7 @@ bool WalletQmlModel::sendTransactionInternal(std::optional<SecureString> passphr
 
             size_t signed_inputs{0};
             const std::optional<common::PSBTError> fill_error{
-                m_wallet->fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &signed_inputs, psbt, complete)};
+                m_wallet->fillPSBT({.sign = true, .bip32_derivs = true}, &signed_inputs, psbt, complete)};
             if (fill_error) {
                 setTransactionStatus(PsbtQmlModel::PsbtErrorText(*fill_error));
                 return false;
@@ -1876,15 +1874,17 @@ WalletQmlModel::PsbtImportResult WalletQmlModel::importPsbtFromFile(const QStrin
         return PsbtImportResult::PsbtUnsupported;
     }
 
-    PartiallySignedTransaction psbt;
+    CMutableTransaction empty_tx;
+    PartiallySignedTransaction psbt{empty_tx};
     const QString load_err{PsbtQmlModel::LoadPsbtFromFile(path, psbt)};
     if (!load_err.isEmpty()) {
         m_imported_psbt_model->setError(load_err);
         return PsbtImportResult::PsbtUnsupported;
     }
 
-    if (m_wallet && psbt.tx) {
-        const Txid psbt_txid{psbt.tx->GetHash()};
+    const auto unsigned_tx{psbt.GetUnsignedTx()};
+    if (m_wallet && unsigned_tx) {
+        const Txid psbt_txid{unsigned_tx->GetHash()};
         interfaces::WalletTxStatus tx_status;
         int num_blocks{0};
         int64_t block_time{0};
@@ -1915,7 +1915,8 @@ QString WalletQmlModel::saveCurrentTransactionAsPsbt(const QString& path)
         return tr("No transaction is prepared.");
     }
 
-    PartiallySignedTransaction psbtx;
+    CMutableTransaction empty_tx;
+    PartiallySignedTransaction psbtx{empty_tx};
     try {
         if (m_current_psbt) {
             psbtx = *m_current_psbt;
@@ -1932,7 +1933,7 @@ QString WalletQmlModel::saveCurrentTransactionAsPsbt(const QString& path)
         }
 
         bool complete{false};
-        const auto err{m_wallet->fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true,
+        const auto err{m_wallet->fillPSBT({.sign = false, .bip32_derivs = true},
                                           /*n_signed=*/nullptr, psbtx, complete)};
         if (err) {
             return PsbtQmlModel::PsbtErrorText(*err);
@@ -1950,33 +1951,39 @@ bool WalletQmlModel::tryImportPsbtToReview(const PartiallySignedTransaction& psb
         reason = tr("No wallet is loaded.");
         return false;
     }
-    if (!psbt.tx) {
+    const auto unsigned_tx{psbt.GetUnsignedTx()};
+    if (!unsigned_tx) {
         reason = tr("The PSBT does not contain an unsigned transaction.");
         return false;
     }
-    if (psbt.tx->vin.empty() || psbt.tx->vout.empty()) {
+    if (unsigned_tx->vin.empty() || unsigned_tx->vout.empty()) {
         reason = tr("The PSBT has no inputs or outputs.");
         return false;
     }
-    if (psbt.inputs.size() != psbt.tx->vin.size() || psbt.outputs.size() != psbt.tx->vout.size()) {
+    if (psbt.inputs.size() != unsigned_tx->vin.size() || psbt.outputs.size() != unsigned_tx->vout.size()) {
         reason = tr("The PSBT is malformed.");
         return false;
     }
 
-    const bool spends_only_wallet_inputs{std::all_of(psbt.tx->vin.begin(), psbt.tx->vin.end(), [this](const CTxIn& input) {
+    const bool spends_only_wallet_inputs{std::all_of(unsigned_tx->vin.begin(), unsigned_tx->vin.end(), [this](const CTxIn& input) {
         return m_wallet->txinIsMine(input);
     })};
 
     PartiallySignedTransaction analysis_psbt{psbt};
     bool complete{FinalizePSBT(analysis_psbt)};
     size_t could_sign{0};
     const std::optional<common::PSBTError> fill_error{
-        m_wallet->fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &could_sign, analysis_psbt, complete)};
+        m_wallet->fillPSBT({.sign = false, .bip32_derivs = false}, &could_sign, analysis_psbt, complete)};
     if (fill_error) {
         reason = PsbtQmlModel::PsbtErrorText(*fill_error);
         return false;
     }
     complete = FinalizePSBT(analysis_psbt);
+    const auto analysis_tx{analysis_psbt.GetUnsignedTx()};
+    if (!analysis_tx) {
+        reason = tr("The PSBT does not contain an unsigned transaction.");
+        return false;
+    }
     std::optional<std::pair<int, int>> multisig_sig_info;
     if (!complete) {
         for (size_t i{0}; i < analysis_psbt.inputs.size(); ++i) {
@@ -2009,7 +2016,7 @@ bool WalletQmlModel::tryImportPsbtToReview(const PartiallySignedTransaction& psb
     };
     std::vector<DraftRecipient> draft_recipients;
     CAmount recipient_total{0};
-    for (const CTxOut& output : analysis_psbt.tx->vout) {
+    for (const CTxOut& output : analysis_tx->vout) {
         CTxDestination destination;
         if (!ExtractDestination(output.scriptPubKey, destination)) {
             if (output.nValue == 0 && output.scriptPubKey.IsUnspendable()) {
@@ -2056,7 +2063,7 @@ bool WalletQmlModel::tryImportPsbtToReview(const PartiallySignedTransaction& psb
         delete m_current_transaction;
     }
     m_current_transaction = new WalletQmlModelTransaction(m_send_recipients, this);
-    m_current_transaction->setWtx(MakeTransactionRef(*analysis_psbt.tx));
+    m_current_transaction->setWtx(MakeTransactionRef(*analysis_tx));
     if (analysis.fee) {
         m_current_transaction->setTransactionFee(*analysis.fee);
     }

test/mocks/mockwallet.h

@@ -53,7 +53,7 @@ class StubWallet : public interfaces::Wallet
     bool unlockCoin(const COutPoint&) override { return false; }
     bool isLockedCoin(const COutPoint&) override { return false; }
     void listLockedCoins(std::vector<COutPoint>&) override {}
-    util::Result<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>&, const wallet::CCoinControl&, bool, int&, CAmount&) override { return util::Error{Untranslated("not implemented")}; }
+    util::Result<wallet::CreatedTransactionResult> createTransaction(const std::vector<wallet::CRecipient>&, const wallet::CCoinControl&, bool, std::optional<unsigned int>) override { return util::Error{Untranslated("not implemented")}; }
     void commitTransaction(CTransactionRef, interfaces::WalletValueMap, interfaces::WalletOrderForm) override {}
     bool transactionCanBeAbandoned(const Txid&) override { return false; }
     bool abandonTransaction(const Txid&) override { return false; }
@@ -66,7 +66,7 @@ class StubWallet : public interfaces::Wallet
     std::set<interfaces::WalletTx> getWalletTxs() override { return {}; }
     bool tryGetTxStatus(const Txid&, interfaces::WalletTxStatus&, int&, int64_t&) override { return false; }
     interfaces::WalletTx getWalletTxDetails(const Txid&, interfaces::WalletTxStatus&, interfaces::WalletOrderForm&, bool&, int&) override { return {}; }
-    std::optional<common::PSBTError> fillPSBT(std::optional<int>, bool, bool, size_t*, PartiallySignedTransaction&, bool&) override { return std::nullopt; }
+    std::optional<common::PSBTError> fillPSBT(const common::PSBTFillOptions&, size_t*, PartiallySignedTransaction&, bool&) override { return std::nullopt; }
     interfaces::WalletBalances getBalances() override { return {}; }
     bool tryGetBalances(interfaces::WalletBalances&, uint256&) override { return false; }
     CAmount getBalance() override { return 0; }
@@ -106,14 +106,23 @@ class MockWallet : public StubWallet
         return getNewDestinationValue(type, label);
     }
 
-    util::Result<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
+    util::Result<wallet::CreatedTransactionResult> createTransaction(const std::vector<wallet::CRecipient>& recipients,
         const wallet::CCoinControl& coin_control,
         bool sign,
-        int& change_pos,
-        CAmount& fee) override
+        std::optional<unsigned int>) override
     {
         if (createTransactionHandler) {
-            return createTransactionHandler(recipients, coin_control, sign, change_pos, fee);
+            int change_pos{-1};
+            CAmount fee{0};
+            auto result = createTransactionHandler(recipients, coin_control, sign, change_pos, fee);
+            if (!result) {
+                return util::Error{util::ErrorString(result)};
+            }
+            return wallet::CreatedTransactionResult{
+                *result,
+                fee,
+                change_pos >= 0 ? std::optional<unsigned int>{static_cast<unsigned int>(change_pos)} : std::nullopt,
+                FeeCalculation{}};
         }
         return util::Error{Untranslated("no createTransactionHandler installed")};
     }