merge bitcoin#25218: introduce generic 'Result' class and connect it to CreateTransaction and GetNewDestination

excludes:
- 7a45c33

continuation of 9f5845c from dash#6733

Author: kwvg

src/bench/wallet_loading.cpp

@@ -47,12 +47,11 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
 
 static void AddTx(CWallet& wallet)
 {
-    bilingual_str error;
-    CTxDestination dest;
-    wallet.GetNewDestination("", dest, error);
+    const auto& dest = wallet.GetNewDestination("");
+    assert(dest.HasRes());
 
     CMutableTransaction mtx;
-    mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
+    mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())});
     mtx.vin.push_back(CTxIn());
 
     wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});

src/coinjoin/util.cpp

@@ -282,12 +282,13 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult)
     CTransactionRef tx;
     {
         LOCK2(m_wallet.cs_wallet, ::cs_main);
-        FeeCalculation fee_calc_out;
-        if (auto txr = wallet::CreateTransaction(m_wallet, vecSend, nChangePosRet, strResult, coinControl, fee_calc_out)) {
-            tx = txr->tx;
-            nFeeRet = txr->fee;
-            nChangePosRet = txr->change_pos;
+        auto ret = wallet::CreateTransaction(m_wallet, vecSend, nChangePosRet, coinControl);
+        if (ret) {
+            tx = ret.GetObj().tx;
+            nFeeRet = ret.GetObj().fee;
+            nChangePosRet = ret.GetObj().change_pos;
         } else {
+            strResult = ret.GetError();
             return false;
         }
     }

src/interfaces/wallet.h

@@ -112,7 +112,7 @@ class Wallet
     virtual std::string getWalletName() = 0;
 
     // Get a new address.
-    virtual bool getNewDestination(const std::string label, CTxDestination& dest) = 0;
+    virtual BResult<CTxDestination> getNewDestination(const std::string label) = 0;
 
     //! Get public key.
     virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
@@ -167,12 +167,11 @@ class Wallet
     virtual std::vector<COutPoint> listProTxCoins() = 0;
 
     //! Create transaction.
-    virtual CTransactionRef createTransaction(const std::vector<wallet::CRecipient>& recipients,
+    virtual BResult<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
         const wallet::CCoinControl& coin_control,
         bool sign,
         int& change_pos,
-        CAmount& fee,
-        bilingual_str& fail_reason) = 0;
+        CAmount& fee) = 0;
 
     //! Commit transaction.
     virtual void commitTransaction(CTransactionRef tx,

src/qt/addresstablemodel.cpp

@@ -366,23 +366,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
     else if(type == Receive)
     {
         // Generate a new address to associate with given label
-        CTxDestination dest;
-        if(!walletModel->wallet().getNewDestination(strLabel, dest))
-        {
+        auto op_dest = walletModel->wallet().getNewDestination(strLabel);
+        if (!op_dest) {
             WalletModel::UnlockContext ctx(walletModel->requestUnlock());
-            if(!ctx.isValid())
-            {
+            if (!ctx.isValid()) {
                 // Unlock wallet failed or was cancelled
                 editStatus = WALLET_UNLOCK_FAILURE;
                 return QString();
             }
-            if(!walletModel->wallet().getNewDestination(strLabel, dest))
-            {
+            op_dest = walletModel->wallet().getNewDestination(strLabel);
+            if (!op_dest) {
                 editStatus = KEY_GENERATION_FAILURE;
                 return QString();
             }
         }
-        strAddress = EncodeDestination(dest);
+        strAddress = EncodeDestination(op_dest.GetObj());
     }
     else
     {

src/qt/walletmodel.cpp

@@ -251,11 +251,11 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
     }
 
     CAmount nFeeRequired = 0;
-    bilingual_str error;
     int nChangePosRet = -1;
 
     auto& newTx = transaction.getWtx();
-    newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
+    const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired);
+    newTx = res ? res.GetObj() : nullptr;
     transaction.setTransactionFee(nFeeRequired);
     if (fSubtractFeeFromAmount && newTx)
         transaction.reassignAmounts(nChangePosRet);
@@ -266,7 +266,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
         {
             return SendCoinsReturn(AmountWithFeeExceedsBalance);
         }
-        Q_EMIT message(tr("Send Coins"), QString::fromStdString(error.translated),
+        Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated),
                      CClientUIInterface::MSG_ERROR);
         return TransactionCreationFailed;
     }

src/rpc/evo.cpp

@@ -281,15 +281,12 @@ static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const Specia
         throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("No funds at specified address %s", EncodeDestination(fundDest)));
     }
 
-    bilingual_str strFailReason;
-    FeeCalculation fee_calc_out;
-    auto txr = CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, strFailReason, coinControl, fee_calc_out,
-                                 true, tx.vExtraPayload.size());
-    if (!txr) {
-        throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason.original);
+    auto res = CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, coinControl, /*sign=*/true, tx.vExtraPayload.size());
+    if (!res) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, res.GetError().original);
     }
-    CTransactionRef newTx = txr->tx;
 
+    const CTransactionRef& newTx = res.GetObj().tx;
     tx.vin = newTx->vin;
     tx.vout = newTx->vout;
 

src/test/util/wallet.cpp

@@ -24,11 +24,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
 #ifdef ENABLE_WALLET
 std::string getnewaddress(CWallet& w)
 {
-    CTxDestination dest;
-    bilingual_str error;
-    if (!w.GetNewDestination("", dest, error)) assert(false);
+    auto op_dest = w.GetNewDestination("");
+    assert(op_dest.HasRes());
 
-    return EncodeDestination(dest);
+    return EncodeDestination(op_dest.GetObj());
 }
 
 // void importaddress(CWallet& wallet, const std::string& address)

src/wallet/interfaces.cpp

@@ -175,11 +175,10 @@ class WalletImpl : public Wallet
     }
     int64_t getKeysLeftSinceAutoBackup() override { return m_wallet->nKeysLeftSinceAutoBackup; }
     std::string getWalletName() override { return m_wallet->GetName(); }
-    bool getNewDestination(const std::string label, CTxDestination& dest) override
+    BResult<CTxDestination> getNewDestination(const std::string label) override
     {
         LOCK(m_wallet->cs_wallet);
-        bilingual_str error;
-        return m_wallet->GetNewDestination(label, dest, error);
+        return m_wallet->GetNewDestination(label);
     }
     bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
     {
@@ -288,22 +287,20 @@ class WalletImpl : public Wallet
         LOCK(m_wallet->cs_wallet);
         return m_wallet->ListProTxCoins();
     }
-    CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,
+    BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
         const CCoinControl& coin_control,
         bool sign,
         int& change_pos,
-        CAmount& fee,
-        bilingual_str& fail_reason) override
+        CAmount& fee) override
     {
         LOCK(m_wallet->cs_wallet);
-        FeeCalculation fee_calc_out;
-        std::optional<CreatedTransactionResult> txr = CreateTransaction(*m_wallet, recipients, change_pos,
-                fail_reason, coin_control, fee_calc_out, sign);
-        if (!txr) return {};
-        fee = txr->fee;
-        change_pos = txr->change_pos;
+        const auto& res = CreateTransaction(*m_wallet, recipients, change_pos, coin_control, sign);
+        if (!res) return res.GetError();
+        const auto& txr = res.GetObj();
+        fee = txr.fee;
+        change_pos = txr.change_pos;
 
-        return txr->tx;
+        return txr.tx;
     }
     void commitTransaction(CTransactionRef tx,
         WalletValueMap value_map,

src/wallet/rpc/addresses.cpp

@@ -46,12 +46,11 @@ RPCHelpMan getnewaddress()
     if (!request.params[0].isNull())
         label = LabelFromValue(request.params[0]);
 
-    CTxDestination dest;
-    bilingual_str error;
-    if (!pwallet->GetNewDestination(label, dest, error)) {
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
+    auto op_dest = pwallet->GetNewDestination(label);
+    if (!op_dest) {
+        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
     }
-    return EncodeDestination(dest);
+    return EncodeDestination(op_dest.GetObj());
 },
     };
 }
@@ -80,12 +79,11 @@ RPCHelpMan getrawchangeaddress()
         throw JSONRPCError(RPC_WALLET_ERROR, "Error: This wallet has no available keys");
     }
 
-    CTxDestination dest;
-    bilingual_str error;
-    if (!pwallet->GetNewChangeDestination(dest, error)) {
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
+    auto op_dest = pwallet->GetNewChangeDestination();
+    if (!op_dest) {
+        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
     }
-    return EncodeDestination(dest);
+    return EncodeDestination(op_dest.GetObj());
 },
     };
 }

src/wallet/rpc/spend.cpp

@@ -65,18 +65,16 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
     }
 
     // Send
-    bilingual_str error;
-    FeeCalculation fee_calc_out;
-    std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true);
-    if (!txr) {
-        throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
+    auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, /*sign=*/true);
+    if (!res) {
+        throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original);
     }
-    CTransactionRef tx = txr->tx;
+    const CTransactionRef& tx = res.GetObj().tx;
     wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */);
     if (verbose) {
         UniValue entry(UniValue::VOBJ);
         entry.pushKV("txid", tx->GetHash().GetHex());
-        entry.pushKV("fee_reason", StringForFeeReason(fee_calc_out.reason));
+        entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason));
         return entry;
     }
     return tx->GetHash().GetHex();