Skip to content

Commit 0671d66

Browse files
marcofleonmarcofleon
committed
wallet, refactor: Convert uint256 to Txid in wallet
Switch all instances of transactions from uint256 to Txid in the wallet and relevant tests.
1 parent c8ed51e commit 0671d66

18 files changed

+106
-106
lines changed

‎src/qt/test/wallettests.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_ty
7575
}
7676

7777
//! Send coins to address and return txid.
78-
uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
78+
Txid SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
7979
QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
8080
{
8181
QVBoxLayout* entries = sendCoinsDialog.findChild<QVBoxLayout*>("entries");
@@ -86,8 +86,8 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe
8686
->findChild<QFrame*>("frameFeeSelection")
8787
->findChild<QCheckBox*>("optInRBF")
8888
->setCheckState(rbf ? Qt::Checked : Qt::Unchecked);
89-
uint256 txid;
90-
boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const uint256& hash, ChangeType status) {
89+
Txid txid;
90+
boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const Txid& hash, ChangeType status) {
9191
if (status == CT_NEW) txid = hash;
9292
}));
9393
ConfirmSend(/*text=*/nullptr, confirm_type);
@@ -97,7 +97,7 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe
9797
}
9898

9999
//! Find index of txid in transaction list.
100-
QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
100+
QModelIndex FindTx(const QAbstractItemModel& model, const Txid& txid)
101101
{
102102
QString hash = QString::fromStdString(txid.ToString());
103103
int rows = model.rowCount({});
@@ -111,7 +111,7 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
111111
}
112112

113113
//! Invoke bumpfee on txid and check results.
114-
void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, std::string expectError, bool cancel)
114+
void BumpFee(TransactionView& view, const Txid& txid, bool expectDisabled, std::string expectError, bool cancel)
115115
{
116116
QTableView* table = view.findChild<QTableView*>("transactionView");
117117
QModelIndex index = FindTx(*table->selectionModel()->model(), txid);
@@ -285,8 +285,8 @@ void TestGUI(interfaces::Node& node, const std::shared_ptr<CWallet>& wallet)
285285
// Send two transactions, and verify they are added to transaction list.
286286
TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
287287
QCOMPARE(transactionTableModel->rowCount({}), 105);
288-
uint256 txid1 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 5 * COIN, /*rbf=*/false);
289-
uint256 txid2 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 10 * COIN, /*rbf=*/true);
288+
Txid txid1 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 5 * COIN, /*rbf=*/false);
289+
Txid txid2 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 10 * COIN, /*rbf=*/true);
290290
// Transaction table model updates on a QueuedConnection, so process events to ensure it's updated.
291291
qApp->processEvents();
292292
QCOMPARE(transactionTableModel->rowCount({}), 107);

‎src/wallet/feebumper.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, con
146146

147147
namespace feebumper {
148148

149-
bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
149+
bool TransactionCanBeBumped(const CWallet& wallet, const Txid& txid)
150150
{
151151
LOCK(wallet.cs_wallet);
152152
const CWalletTx* wtx = wallet.GetWalletTx(txid);
@@ -157,7 +157,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
157157
return res == feebumper::Result::OK;
158158
}
159159

160-
Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
160+
Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
161161
CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> original_change_index)
162162
{
163163
// For now, cannot specify both new outputs to use and an output index to send change
@@ -348,7 +348,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
348348
}
349349
}
350350

351-
Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid)
351+
Result CommitTransaction(CWallet& wallet, const Txid& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid)
352352
{
353353
LOCK(wallet.cs_wallet);
354354
if (!errors.empty()) {

‎src/wallet/feebumper.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ enum class Result
3131
};
3232

3333
//! Return whether transaction can be bumped.
34-
bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
34+
bool TransactionCanBeBumped(const CWallet& wallet, const Txid& txid);
3535

3636
/** Create bumpfee transaction based on feerate estimates.
3737
*
@@ -47,7 +47,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
4747
* @param[in] original_change_index The position of the change output to deduct the fee from in the transaction being bumped
4848
*/
4949
Result CreateRateBumpTransaction(CWallet& wallet,
50-
const uint256& txid,
50+
const Txid& txid,
5151
const CCoinControl& coin_control,
5252
std::vector<bilingual_str>& errors,
5353
CAmount& old_fee,
@@ -67,7 +67,7 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx);
6767
//! but sets errors if the tx could not be added to the mempool (will try later)
6868
//! or if the old transaction could not be marked as replaced.
6969
Result CommitTransaction(CWallet& wallet,
70-
const uint256& txid,
70+
const Txid& txid,
7171
CMutableTransaction&& mtx,
7272
std::vector<bilingual_str>& errors,
7373
Txid& bumped_txid);

‎src/wallet/interfaces.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ class WalletImpl : public Wallet
533533
std::unique_ptr<Handler> handleTransactionChanged(TransactionChangedFn fn) override
534534
{
535535
return MakeSignalHandler(m_wallet->NotifyTransactionChanged.connect(
536-
[fn](const uint256& txid, ChangeType status) { fn(Txid::FromUint256(txid), status); }));
536+
[fn](const Txid& txid, ChangeType status) { fn(txid, status); }));
537537
}
538538
std::unique_ptr<Handler> handleWatchOnlyChanged(WatchOnlyChangedFn fn) override
539539
{

‎src/wallet/receive.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminef
254254
}
255255

256256
// NOLINTNEXTLINE(misc-no-recursion)
257-
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents)
257+
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<Txid>& trusted_parents)
258258
{
259259
AssertLockHeld(wallet.cs_wallet);
260260
if (wtx.isConfirmed()) return true;
@@ -285,7 +285,7 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uin
285285

286286
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx)
287287
{
288-
std::set<uint256> trusted_parents;
288+
std::set<Txid> trusted_parents;
289289
LOCK(wallet.cs_wallet);
290290
return CachedTxIsTrusted(wallet, wtx, trusted_parents);
291291
}
@@ -296,7 +296,7 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
296296
isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED;
297297
{
298298
LOCK(wallet.cs_wallet);
299-
std::set<uint256> trusted_parents;
299+
std::set<Txid> trusted_parents;
300300
for (const auto& entry : wallet.mapWallet)
301301
{
302302
const CWalletTx& wtx = entry.second;
@@ -325,7 +325,7 @@ std::map<CTxDestination, CAmount> GetAddressBalances(const CWallet& wallet)
325325

326326
{
327327
LOCK(wallet.cs_wallet);
328-
std::set<uint256> trusted_parents;
328+
std::set<Txid> trusted_parents;
329329
for (const auto& walletEntry : wallet.mapWallet)
330330
{
331331
const CWalletTx& wtx = walletEntry.second;
@@ -348,7 +348,7 @@ std::map<CTxDestination, CAmount> GetAddressBalances(const CWallet& wallet)
348348
if(!ExtractDestination(output.scriptPubKey, addr))
349349
continue;
350350

351-
CAmount n = wallet.IsSpent(COutPoint(Txid::FromUint256(walletEntry.first), i)) ? 0 : output.nValue;
351+
CAmount n = wallet.IsSpent(COutPoint(walletEntry.first, i)) ? 0 : output.nValue;
352352
balances[addr] += n;
353353
}
354354
}

‎src/wallet/receive.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define BITCOIN_WALLET_RECEIVE_H
77

88
#include <consensus/amount.h>
9+
#include <util/transaction_identifier.h>
910
#include <wallet/transaction.h>
1011
#include <wallet/types.h>
1112
#include <wallet/wallet.h>
@@ -45,7 +46,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
4546
CAmount& nFee, const isminefilter& filter,
4647
bool include_change);
4748
bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
48-
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
49+
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<Txid>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
4950
bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx);
5051

5152
struct Balance {

‎src/wallet/rpc/backup.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ RPCHelpMan importprunedfunds()
5353
if (!DecodeHexTx(tx, request.params[0].get_str())) {
5454
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
5555
}
56-
uint256 hashTx = tx.GetHash();
5756

5857
DataStream ssMB{ParseHexV(request.params[1], "proof")};
5958
CMerkleBlock merkleBlock;
@@ -73,7 +72,7 @@ RPCHelpMan importprunedfunds()
7372
}
7473

7574
std::vector<uint256>::const_iterator it;
76-
if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx)) == vMatch.end()) {
75+
if ((it = std::find(vMatch.begin(), vMatch.end(), tx.GetHash())) == vMatch.end()) {
7776
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction given doesn't exist in proof");
7877
}
7978

@@ -110,8 +109,8 @@ RPCHelpMan removeprunedfunds()
110109

111110
LOCK(pwallet->cs_wallet);
112111

113-
uint256 hash(ParseHashV(request.params[0], "txid"));
114-
std::vector<uint256> vHash;
112+
Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))};
113+
std::vector<Txid> vHash;
115114
vHash.push_back(hash);
116115
if (auto res = pwallet->RemoveTxs(vHash); !res) {
117116
throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(res).original);

‎src/wallet/rpc/coins.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
5656

5757
// Tally
5858
CAmount amount = 0;
59-
for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) {
60-
const CWalletTx& wtx = wtx_pair.second;
59+
for (const auto& [_, wtx] : wallet.mapWallet) {
6160
int depth{wallet.GetTxDepthInMainChain(wtx)};
6261
if (depth < min_depth
6362
// Coinbase with less than 1 confirmation is no longer in the main chain

‎src/wallet/rpc/spend.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
10681068
throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead.");
10691069
}
10701070

1071-
uint256 hash(ParseHashV(request.params[0], "txid"));
1071+
Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))};
10721072

10731073
CCoinControl coin_control;
10741074
coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);

‎src/wallet/rpc/transactions.cpp

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <policy/rbf.h>
88
#include <rpc/util.h>
99
#include <rpc/blockchain.h>
10+
#include <util/transaction_identifier.h>
1011
#include <util/vector.h>
1112
#include <wallet/receive.h>
1213
#include <wallet/rpc/util.h>
@@ -34,11 +35,10 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue
3435
} else {
3536
entry.pushKV("trusted", CachedTxIsTrusted(wallet, wtx));
3637
}
37-
uint256 hash = wtx.GetHash();
38-
entry.pushKV("txid", hash.GetHex());
38+
entry.pushKV("txid", wtx.GetHash().GetHex());
3939
entry.pushKV("wtxid", wtx.GetWitnessHash().GetHex());
4040
UniValue conflicts(UniValue::VARR);
41-
for (const uint256& conflict : wallet.GetTxConflicts(wtx))
41+
for (const Txid& conflict : wallet.GetTxConflicts(wtx))
4242
conflicts.push_back(conflict.GetHex());
4343
entry.pushKV("walletconflicts", std::move(conflicts));
4444
UniValue mempool_conflicts(UniValue::VARR);
@@ -67,7 +67,7 @@ struct tallyitem
6767
{
6868
CAmount nAmount{0};
6969
int nConf{std::numeric_limits<int>::max()};
70-
std::vector<uint256> txids;
70+
std::vector<Txid> txids;
7171
bool fIsWatchonly{false};
7272
tallyitem() = default;
7373
};
@@ -100,8 +100,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
100100

101101
// Tally
102102
std::map<CTxDestination, tallyitem> mapTally;
103-
for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
104-
const CWalletTx& wtx = pairWtx.second;
103+
for (const auto& [_, wtx] : wallet.mapWallet) {
105104

106105
int nDepth = wallet.GetTxDepthInMainChain(wtx);
107106
if (nDepth < nMinDepth)
@@ -169,7 +168,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
169168
obj.pushKV("label", label);
170169
UniValue transactions(UniValue::VARR);
171170
if (it != mapTally.end()) {
172-
for (const uint256& _item : (*it).second.txids) {
171+
for (const Txid& _item : (*it).second.txids) {
173172
transactions.push_back(_item.GetHex());
174173
}
175174
}
@@ -650,8 +649,7 @@ RPCHelpMan listsinceblock()
650649

651650
UniValue transactions(UniValue::VARR);
652651

653-
for (const std::pair<const uint256, CWalletTx>& pairWtx : wallet.mapWallet) {
654-
const CWalletTx& tx = pairWtx.second;
652+
for (const auto& [_, tx] : wallet.mapWallet) {
655653

656654
if (depth == -1 || abs(wallet.GetTxDepthInMainChain(tx)) < depth) {
657655
ListTransactions(wallet, tx, 0, true, transactions, filter, filter_label, include_change);
@@ -760,7 +758,7 @@ RPCHelpMan gettransaction()
760758

761759
LOCK(pwallet->cs_wallet);
762760

763-
uint256 hash(ParseHashV(request.params[0], "txid"));
761+
Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))};
764762

765763
isminefilter filter = ISMINE_SPENDABLE;
766764

@@ -833,7 +831,7 @@ RPCHelpMan abandontransaction()
833831

834832
LOCK(pwallet->cs_wallet);
835833

836-
uint256 hash(ParseHashV(request.params[0], "txid"));
834+
Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))};
837835

838836
if (!pwallet->mapWallet.count(hash)) {
839837
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");

0 commit comments

Comments
 (0)