Skip to content

Commit 54faec8

Browse files
ryanofskyryanofsky
committed
refactor: Detach wallet transaction methods (followup for move-only)
Followup to commit "MOVEONLY: CWallet transaction code out of wallet.cpp/.h" that detaches and renames some CWalletTx methods, making into them into standalone functions or CWallet methods instead. There are no changes in behavior and no code changes that aren't purely mechanical. It just gives spend and receive functions more consistent names and removes the circular dependencies added by the earlier MOVEONLY commit. There are also no comment or documentation changes. Removed comments from transaction.h are just migrated to spend.h, receive.h, and wallet.h.
1 parent b784ab1 commit 54faec8

20 files changed

+532
-520
lines changed

‎src/bench/coin_selection.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <interfaces/chain.h>
77
#include <node/context.h>
88
#include <wallet/coinselection.h>
9+
#include <wallet/spend.h>
910
#include <wallet/wallet.h>
1011

1112
#include <set>
@@ -17,7 +18,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<st
1718
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
1819
tx.vout.resize(1);
1920
tx.vout[0].nValue = nValue;
20-
wtxs.push_back(std::make_unique<CWalletTx>(&wallet, MakeTransactionRef(std::move(tx))));
21+
wtxs.push_back(std::make_unique<CWalletTx>(MakeTransactionRef(std::move(tx))));
2122
}
2223

2324
// Simple benchmark for wallet coin selection. Note that it maybe be necessary
@@ -45,7 +46,7 @@ static void CoinSelection(benchmark::Bench& bench)
4546
// Create coins
4647
std::vector<COutput> coins;
4748
for (const auto& wtx : wtxs) {
48-
coins.emplace_back(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */);
49+
coins.emplace_back(wallet, *wtx, 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */);
4950
}
5051

5152
const CoinEligibilityFilter filter_standard(1, 6, 0);
@@ -56,7 +57,7 @@ static void CoinSelection(benchmark::Bench& bench)
5657
bench.run([&] {
5758
std::set<CInputCoin> setCoinsRet;
5859
CAmount nValueRet;
59-
bool success = wallet.AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
60+
bool success = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
6061
assert(success);
6162
assert(nValueRet == 1003 * COIN);
6263
assert(setCoinsRet.size() == 2);
@@ -75,9 +76,9 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>
7576
CMutableTransaction tx;
7677
tx.vout.resize(nInput + 1);
7778
tx.vout[nInput].nValue = nValue;
78-
std::unique_ptr<CWalletTx> wtx = std::make_unique<CWalletTx>(&testWallet, MakeTransactionRef(std::move(tx)));
79+
std::unique_ptr<CWalletTx> wtx = std::make_unique<CWalletTx>(MakeTransactionRef(std::move(tx)));
7980
set.emplace_back();
80-
set.back().Insert(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false);
81+
set.back().Insert(COutput(testWallet, *wtx, nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false);
8182
wtxn.emplace_back(std::move(wtx));
8283
}
8384
// Copied from src/wallet/test/coinselector_tests.cpp

‎src/bench/wallet_balance.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <test/util/setup_common.h>
1010
#include <test/util/wallet.h>
1111
#include <validationinterface.h>
12+
#include <wallet/receive.h>
1213
#include <wallet/wallet.h>
1314

1415
#include <optional>
@@ -35,11 +36,11 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
3536
}
3637
SyncWithValidationInterfaceQueue();
3738

38-
auto bal = wallet.GetBalance(); // Cache
39+
auto bal = GetBalance(wallet); // Cache
3940

4041
bench.run([&] {
4142
if (set_dirty) wallet.MarkDirty();
42-
bal = wallet.GetBalance();
43+
bal = GetBalance(wallet);
4344
if (add_mine) assert(bal.m_mine_trusted > 0);
4445
if (add_watchonly) assert(bal.m_watchonly_trusted > 0);
4546
});

‎src/wallet/feebumper.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include <wallet/coincontrol.h>
1313
#include <wallet/feebumper.h>
1414
#include <wallet/fees.h>
15+
#include <wallet/receive.h>
16+
#include <wallet/spend.h>
1517
#include <wallet/wallet.h>
1618

1719
//! Check whether transaction has descendant in wallet or mempool, or has been
@@ -30,7 +32,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
3032
}
3133
}
3234

33-
if (wtx.GetDepthInMainChain() != 0) {
35+
if (wallet.GetTxDepthInMainChain(wtx) != 0) {
3436
errors.push_back(Untranslated("Transaction has been mined, or is conflicted with a mined transaction"));
3537
return feebumper::Result::WALLET_ERROR;
3638
}
@@ -48,7 +50,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
4850
// check that original tx consists entirely of our inputs
4951
// if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
5052
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
51-
if (!wallet.IsAllFromMe(*wtx.tx, filter)) {
53+
if (!AllInputsMine(wallet, *wtx.tx, filter)) {
5254
errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
5355
return feebumper::Result::WALLET_ERROR;
5456
}
@@ -81,7 +83,7 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt
8183

8284
// Given old total fee and transaction size, calculate the old feeRate
8385
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
84-
CAmount old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
86+
CAmount old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut();
8587
const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
8688
CFeeRate nOldFeeRate(old_fee, txSize);
8789
// Min total fee is old fee + relay fee
@@ -174,7 +176,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
174176
// Fill in recipients(and preserve a single change key if there is one)
175177
std::vector<CRecipient> recipients;
176178
for (const auto& output : wtx.tx->vout) {
177-
if (!wallet.IsChange(output)) {
179+
if (!OutputIsChange(wallet, output)) {
178180
CRecipient recipient = {output.scriptPubKey, output.nValue, false};
179181
recipients.push_back(recipient);
180182
} else {
@@ -185,7 +187,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
185187
}
186188

187189
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
188-
old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
190+
old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut();
189191

190192
if (coin_control.m_feerate) {
191193
// The user provided a feeRate argument.
@@ -220,7 +222,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
220222
int change_pos_in_out = -1; // No requested location for change
221223
bilingual_str fail_reason;
222224
FeeCalculation fee_calc_out;
223-
if (!wallet.CreateTransaction(recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, fee_calc_out, false)) {
225+
if (!CreateTransaction(wallet, recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, fee_calc_out, false)) {
224226
errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason);
225227
return Result::WALLET_ERROR;
226228
}

‎src/wallet/interfaces.cpp‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
#include <wallet/fees.h>
2424
#include <wallet/ismine.h>
2525
#include <wallet/load.h>
26+
#include <wallet/receive.h>
2627
#include <wallet/rpcwallet.h>
28+
#include <wallet/spend.h>
2729
#include <wallet/wallet.h>
2830

2931
#include <memory>
@@ -55,7 +57,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
5557
result.tx = wtx.tx;
5658
result.txin_is_mine.reserve(wtx.tx->vin.size());
5759
for (const auto& txin : wtx.tx->vin) {
58-
result.txin_is_mine.emplace_back(wallet.IsMine(txin));
60+
result.txin_is_mine.emplace_back(InputIsMine(wallet, txin));
5961
}
6062
result.txout_is_mine.reserve(wtx.tx->vout.size());
6163
result.txout_address.reserve(wtx.tx->vout.size());
@@ -67,9 +69,9 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
6769
wallet.IsMine(result.txout_address.back()) :
6870
ISMINE_NO);
6971
}
70-
result.credit = wtx.GetCredit(ISMINE_ALL);
71-
result.debit = wtx.GetDebit(ISMINE_ALL);
72-
result.change = wtx.GetChange();
72+
result.credit = CachedTxGetCredit(wallet, wtx, ISMINE_ALL);
73+
result.debit = CachedTxGetDebit(wallet, wtx, ISMINE_ALL);
74+
result.change = CachedTxGetChange(wallet, wtx);
7375
result.time = wtx.GetTxTime();
7476
result.value_map = wtx.mapValue;
7577
result.is_coinbase = wtx.IsCoinBase();
@@ -81,15 +83,15 @@ WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
8183
{
8284
WalletTxStatus result;
8385
result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits<int>::max();
84-
result.blocks_to_maturity = wtx.GetBlocksToMaturity();
85-
result.depth_in_main_chain = wtx.GetDepthInMainChain();
86+
result.blocks_to_maturity = wallet.GetTxBlocksToMaturity(wtx);
87+
result.depth_in_main_chain = wallet.GetTxDepthInMainChain(wtx);
8688
result.time_received = wtx.nTimeReceived;
8789
result.lock_time = wtx.tx->nLockTime;
8890
result.is_final = wallet.chain().checkFinalTx(*wtx.tx);
89-
result.is_trusted = wtx.IsTrusted();
91+
result.is_trusted = CachedTxIsTrusted(wallet, wtx);
9092
result.is_abandoned = wtx.isAbandoned();
9193
result.is_coinbase = wtx.IsCoinBase();
92-
result.is_in_main_chain = wtx.IsInMainChain();
94+
result.is_in_main_chain = wallet.IsTxInMainChain(wtx);
9395
return result;
9496
}
9597

@@ -242,7 +244,7 @@ class WalletImpl : public Wallet
242244
LOCK(m_wallet->cs_wallet);
243245
CTransactionRef tx;
244246
FeeCalculation fee_calc_out;
245-
if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos,
247+
if (!CreateTransaction(*m_wallet, recipients, tx, fee, change_pos,
246248
fail_reason, coin_control, fee_calc_out, sign)) {
247249
return {};
248250
}
@@ -358,7 +360,7 @@ class WalletImpl : public Wallet
358360
}
359361
WalletBalances getBalances() override
360362
{
361-
const auto bal = m_wallet->GetBalance();
363+
const auto bal = GetBalance(*m_wallet);
362364
WalletBalances result;
363365
result.balance = bal.m_mine_trusted;
364366
result.unconfirmed_balance = bal.m_mine_untrusted_pending;
@@ -381,15 +383,15 @@ class WalletImpl : public Wallet
381383
balances = getBalances();
382384
return true;
383385
}
384-
CAmount getBalance() override { return m_wallet->GetBalance().m_mine_trusted; }
386+
CAmount getBalance() override { return GetBalance(*m_wallet).m_mine_trusted; }
385387
CAmount getAvailableBalance(const CCoinControl& coin_control) override
386388
{
387-
return m_wallet->GetAvailableBalance(&coin_control);
389+
return GetAvailableBalance(*m_wallet, &coin_control);
388390
}
389391
isminetype txinIsMine(const CTxIn& txin) override
390392
{
391393
LOCK(m_wallet->cs_wallet);
392-
return m_wallet->IsMine(txin);
394+
return InputIsMine(*m_wallet, txin);
393395
}
394396
isminetype txoutIsMine(const CTxOut& txout) override
395397
{
@@ -404,13 +406,13 @@ class WalletImpl : public Wallet
404406
CAmount getCredit(const CTxOut& txout, isminefilter filter) override
405407
{
406408
LOCK(m_wallet->cs_wallet);
407-
return m_wallet->GetCredit(txout, filter);
409+
return OutputGetCredit(*m_wallet, txout, filter);
408410
}
409411
CoinsList listCoins() override
410412
{
411413
LOCK(m_wallet->cs_wallet);
412414
CoinsList result;
413-
for (const auto& entry : m_wallet->ListCoins()) {
415+
for (const auto& entry : ListCoins(*m_wallet)) {
414416
auto& group = result[entry.first];
415417
for (const auto& coin : entry.second) {
416418
group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i),
@@ -428,7 +430,7 @@ class WalletImpl : public Wallet
428430
result.emplace_back();
429431
auto it = m_wallet->mapWallet.find(output.hash);
430432
if (it != m_wallet->mapWallet.end()) {
431-
int depth = it->second.GetDepthInMainChain();
433+
int depth = m_wallet->GetTxDepthInMainChain(it->second);
432434
if (depth >= 0) {
433435
result.back() = MakeWalletTxOut(*m_wallet, it->second, output.n, depth);
434436
}

‎src/wallet/load.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <util/system.h>
1313
#include <util/translation.h>
1414
#include <wallet/context.h>
15+
#include <wallet/spend.h>
1516
#include <wallet/wallet.h>
1617
#include <wallet/walletdb.h>
1718

0 commit comments

Comments
 (0)