wallet, refactor: Convert uint256 to Txid in wallet interfaces

marcofleon · marcofleon · commit c8ed51e62be9 · 2025-05-07T15:59:02.000+01:00
In most cases throughout the wallet, the implicit conversion from `Txid` to
`const uint256&amp;` works. However, `commitBumpTransaction` requires a `uint256&amp;`
out parameter, so `bumped_txid` in `feebumper::CommitTransaction` is also
updated here to use `Txid`.
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
@@ -157,16 +157,16 @@ class Wallet
         WalletOrderForm order_form) = 0;
 
     //! Return whether transaction can be abandoned.
-    virtual bool transactionCanBeAbandoned(const uint256& txid) = 0;
+    virtual bool transactionCanBeAbandoned(const Txid& txid) = 0;
 
     //! Abandon transaction.
-    virtual bool abandonTransaction(const uint256& txid) = 0;
+    virtual bool abandonTransaction(const Txid& txid) = 0;
 
     //! Return whether transaction can be bumped.
-    virtual bool transactionCanBeBumped(const uint256& txid) = 0;
+    virtual bool transactionCanBeBumped(const Txid& txid) = 0;
 
     //! Create bump transaction.
-    virtual bool createBumpTransaction(const uint256& txid,
+    virtual bool createBumpTransaction(const Txid& txid,
         const wallet::CCoinControl& coin_control,
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
@@ -177,28 +177,28 @@ class Wallet
     virtual bool signBumpTransaction(CMutableTransaction& mtx) = 0;
 
     //! Commit bump transaction.
-    virtual bool commitBumpTransaction(const uint256& txid,
+    virtual bool commitBumpTransaction(const Txid& txid,
         CMutableTransaction&& mtx,
         std::vector<bilingual_str>& errors,
-        uint256& bumped_txid) = 0;
+        Txid& bumped_txid) = 0;
 
     //! Get a transaction.
-    virtual CTransactionRef getTx(const uint256& txid) = 0;
+    virtual CTransactionRef getTx(const Txid& txid) = 0;
 
     //! Get transaction information.
-    virtual WalletTx getWalletTx(const uint256& txid) = 0;
+    virtual WalletTx getWalletTx(const Txid& txid) = 0;
 
     //! Get list of all wallet transactions.
     virtual std::set<WalletTx> getWalletTxs() = 0;
 
     //! Try to get updated status for a particular transaction, if possible without blocking.
-    virtual bool tryGetTxStatus(const uint256& txid,
+    virtual bool tryGetTxStatus(const Txid& txid,
         WalletTxStatus& tx_status,
         int& num_blocks,
         int64_t& block_time) = 0;
 
     //! Get transaction details.
-    virtual WalletTx getWalletTxDetails(const uint256& txid,
+    virtual WalletTx getWalletTxDetails(const Txid& txid,
         WalletTxStatus& tx_status,
         WalletOrderForm& order_form,
         bool& in_mempool,
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
@@ -561,15 +561,11 @@ bool WalletModel::bumpFee(Txid hash, Txid& new_hash)
         return false;
     }
     // commit the bumped transaction
-    // Temporary uint256 variable needed for commitBumpTransaction out parameter.
-    uint256 bumped_txid_result;
-    if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, bumped_txid_result)) {
+    if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, new_hash)) {
         QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Could not commit transaction") + "<br />(" +
             QString::fromStdString(errors[0].translated)+")");
         return false;
     }
-    // Assign the received txid back to the new_hash.
-    new_hash = Txid::FromUint256(bumped_txid_result);
     return true;
 }
 
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -348,7 +348,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
     }
 }
 
-Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, uint256& bumped_txid)
+Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid)
 {
     LOCK(wallet.cs_wallet);
     if (!errors.empty()) {
diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h
@@ -70,7 +70,7 @@ Result CommitTransaction(CWallet& wallet,
     const uint256& txid,
     CMutableTransaction&& mtx,
     std::vector<bilingual_str>& errors,
-    uint256& bumped_txid);
+    Txid& bumped_txid);
 
 struct SignatureWeights
 {
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
@@ -291,17 +291,17 @@ class WalletImpl : public Wallet
         LOCK(m_wallet->cs_wallet);
         m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));
     }
-    bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
-    bool abandonTransaction(const uint256& txid) override
+    bool transactionCanBeAbandoned(const Txid& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
+    bool abandonTransaction(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         return m_wallet->AbandonTransaction(txid);
     }
-    bool transactionCanBeBumped(const uint256& txid) override
+    bool transactionCanBeBumped(const Txid& txid) override
     {
         return feebumper::TransactionCanBeBumped(*m_wallet.get(), txid);
     }
-    bool createBumpTransaction(const uint256& txid,
+    bool createBumpTransaction(const Txid& txid,
         const CCoinControl& coin_control,
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
@@ -312,15 +312,15 @@ class WalletImpl : public Wallet
         return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs) == feebumper::Result::OK;
     }
     bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
-    bool commitBumpTransaction(const uint256& txid,
+    bool commitBumpTransaction(const Txid& txid,
         CMutableTransaction&& mtx,
         std::vector<bilingual_str>& errors,
-        uint256& bumped_txid) override
+        Txid& bumped_txid) override
     {
         return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==
                feebumper::Result::OK;
     }
-    CTransactionRef getTx(const uint256& txid) override
+    CTransactionRef getTx(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         auto mi = m_wallet->mapWallet.find(txid);
@@ -329,7 +329,7 @@ class WalletImpl : public Wallet
         }
         return {};
     }
-    WalletTx getWalletTx(const uint256& txid) override
+    WalletTx getWalletTx(const Txid& txid) override
     {
         LOCK(m_wallet->cs_wallet);
         auto mi = m_wallet->mapWallet.find(txid);
@@ -347,7 +347,7 @@ class WalletImpl : public Wallet
         }
         return result;
     }
-    bool tryGetTxStatus(const uint256& txid,
+    bool tryGetTxStatus(const Txid& txid,
         interfaces::WalletTxStatus& tx_status,
         int& num_blocks,
         int64_t& block_time) override
@@ -366,7 +366,7 @@ class WalletImpl : public Wallet
         tx_status = MakeWalletTxStatus(*m_wallet, mi->second);
         return true;
     }
-    WalletTx getWalletTxDetails(const uint256& txid,
+    WalletTx getWalletTxDetails(const Txid& txid,
         WalletTxStatus& tx_status,
         WalletOrderForm& order_form,
         bool& in_mempool,
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
@@ -1166,7 +1166,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
             throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");
         }
 
-        uint256 txid;
+        Txid txid;
         if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
             throw JSONRPCError(RPC_WALLET_ERROR, errors[0].original);
         }

Original file line number	Diff line number	Diff line change
`@@ -561,15 +561,11 @@ bool WalletModel::bumpFee(Txid hash, Txid& new_hash)`
`561`	`561`	`return false;`
`562`	`562`	`}`
`563`	`563`	`// commit the bumped transaction`
`564`		`- // Temporary uint256 variable needed for commitBumpTransaction out parameter.`
`565`		`- uint256 bumped_txid_result;`
`566`		`- if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, bumped_txid_result)) {`
	`564`	`+ if(!m_wallet->commitBumpTransaction(hash, std::move(mtx), errors, new_hash)) {`
`567`	`565`	`QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Could not commit transaction") + "<br />(" +`
`568`	`566`	`QString::fromStdString(errors[0].translated)+")");`
`569`	`567`	`return false;`
`570`	`568`	`}`
`571`		`- // Assign the received txid back to the new_hash.`
`572`		`- new_hash = Txid::FromUint256(bumped_txid_result);`
`573`	`569`	`return true;`
`574`	`570`	`}`
`575`	`571`
Original file line number	Diff line number	Diff line change
`@@ -348,7 +348,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {`
`348`	`348`	`}`
`349`	`349`	`}`
`350`	`350`
`351`		`-Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, uint256& bumped_txid)`
	`351`	`+Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid)`
`352`	`352`	`{`
`353`	`353`	`LOCK(wallet.cs_wallet);`
`354`	`354`	`if (!errors.empty()) {`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ Result CommitTransaction(CWallet& wallet,`
`70`	`70`	`const uint256& txid,`
`71`	`71`	`CMutableTransaction&& mtx,`
`72`	`72`	`std::vector<bilingual_str>& errors,`
`73`		`- uint256& bumped_txid);`
	`73`	`+ Txid& bumped_txid);`
`74`	`74`
`75`	`75`	`struct SignatureWeights`
`76`	`76`	`{`
Original file line number	Diff line number	Diff line change
`@@ -291,17 +291,17 @@ class WalletImpl : public Wallet`
`291`	`291`	`LOCK(m_wallet->cs_wallet);`
`292`	`292`	`m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));`
`293`	`293`	`}`
`294`		`- bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }`
`295`		`- bool abandonTransaction(const uint256& txid) override`
	`294`	`+ bool transactionCanBeAbandoned(const Txid& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }`
	`295`	`+ bool abandonTransaction(const Txid& txid) override`
`296`	`296`	`{`
`297`	`297`	`LOCK(m_wallet->cs_wallet);`
`298`	`298`	`return m_wallet->AbandonTransaction(txid);`
`299`	`299`	`}`
`300`		`- bool transactionCanBeBumped(const uint256& txid) override`
	`300`	`+ bool transactionCanBeBumped(const Txid& txid) override`
`301`	`301`	`{`
`302`	`302`	`return feebumper::TransactionCanBeBumped(*m_wallet.get(), txid);`
`303`	`303`	`}`
`304`		`- bool createBumpTransaction(const uint256& txid,`
	`304`	`+ bool createBumpTransaction(const Txid& txid,`
`305`	`305`	`const CCoinControl& coin_control,`
`306`	`306`	`std::vector<bilingual_str>& errors,`
`307`	`307`	`CAmount& old_fee,`
`@@ -312,15 +312,15 @@ class WalletImpl : public Wallet`
`312`	`312`	`return feebumper::CreateRateBumpTransaction(m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, / require_mine= */ true, outputs) == feebumper::Result::OK;`
`313`	`313`	`}`
`314`	`314`	`bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }`
`315`		`- bool commitBumpTransaction(const uint256& txid,`
	`315`	`+ bool commitBumpTransaction(const Txid& txid,`
`316`	`316`	`CMutableTransaction&& mtx,`
`317`	`317`	`std::vector<bilingual_str>& errors,`
`318`		`- uint256& bumped_txid) override`
	`318`	`+ Txid& bumped_txid) override`
`319`	`319`	`{`
`320`	`320`	`return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==`
`321`	`321`	`feebumper::Result::OK;`
`322`	`322`	`}`
`323`		`- CTransactionRef getTx(const uint256& txid) override`
	`323`	`+ CTransactionRef getTx(const Txid& txid) override`
`324`	`324`	`{`
`325`	`325`	`LOCK(m_wallet->cs_wallet);`
`326`	`326`	`auto mi = m_wallet->mapWallet.find(txid);`
`@@ -329,7 +329,7 @@ class WalletImpl : public Wallet`
`329`	`329`	`}`
`330`	`330`	`return {};`
`331`	`331`	`}`
`332`		`- WalletTx getWalletTx(const uint256& txid) override`
	`332`	`+ WalletTx getWalletTx(const Txid& txid) override`
`333`	`333`	`{`
`334`	`334`	`LOCK(m_wallet->cs_wallet);`
`335`	`335`	`auto mi = m_wallet->mapWallet.find(txid);`
`@@ -347,7 +347,7 @@ class WalletImpl : public Wallet`
`347`	`347`	`}`
`348`	`348`	`return result;`
`349`	`349`	`}`
`350`		`- bool tryGetTxStatus(const uint256& txid,`
	`350`	`+ bool tryGetTxStatus(const Txid& txid,`
`351`	`351`	`interfaces::WalletTxStatus& tx_status,`
`352`	`352`	`int& num_blocks,`
`353`	`353`	`int64_t& block_time) override`
`@@ -366,7 +366,7 @@ class WalletImpl : public Wallet`
`366`	`366`	`tx_status = MakeWalletTxStatus(*m_wallet, mi->second);`
`367`	`367`	`return true;`
`368`	`368`	`}`
`369`		`- WalletTx getWalletTxDetails(const uint256& txid,`
	`369`	`+ WalletTx getWalletTxDetails(const Txid& txid,`
`370`	`370`	`WalletTxStatus& tx_status,`
`371`	`371`	`WalletOrderForm& order_form,`
`372`	`372`	`bool& in_mempool,`
Original file line number	Diff line number	Diff line change
`@@ -1166,7 +1166,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)`
`1166`	`1166`	`throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");`
`1167`	`1167`	`}`
`1168`	`1168`
`1169`		`- uint256 txid;`
	`1169`	`+ Txid txid;`
`1170`	`1170`	`if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {`
`1171`	`1171`	`throw JSONRPCError(RPC_WALLET_ERROR, errors[0].original);`
`1172`	`1172`	`}`