Skip to content

Commit b66c429

Browse files
author
Antoine Riard
committed
Remove locked_chain from GetDepthInMainChain and its callers
We don't remove yet Chain locks as we need to preserve lock order with CWallet one until swapping at once to avoid deadlock failures (spotted by --enable-debug)
1 parent 0ff0387 commit b66c429

File tree

7 files changed

+91
-93
lines changed

7 files changed

+91
-93
lines changed

‎src/interfaces/wallet.cpp

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ namespace interfaces {
3131
namespace {
3232

3333
//! Construct wallet tx struct.
34-
WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx)
34+
WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
3535
{
3636
WalletTx result;
3737
result.tx = wtx.tx;
@@ -49,7 +49,7 @@ WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, co
4949
wallet.IsMine(result.txout_address.back()) :
5050
ISMINE_NO);
5151
}
52-
result.credit = wtx.GetCredit(locked_chain, ISMINE_ALL);
52+
result.credit = wtx.GetCredit(ISMINE_ALL);
5353
result.debit = wtx.GetDebit(ISMINE_ALL);
5454
result.change = wtx.GetChange();
5555
result.time = wtx.GetTxTime();
@@ -63,21 +63,20 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C
6363
{
6464
WalletTxStatus result;
6565
result.block_height = locked_chain.getBlockHeight(wtx.m_confirm.hashBlock).get_value_or(std::numeric_limits<int>::max());
66-
result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain);
67-
result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
66+
result.blocks_to_maturity = wtx.GetBlocksToMaturity();
67+
result.depth_in_main_chain = wtx.GetDepthInMainChain();
6868
result.time_received = wtx.nTimeReceived;
6969
result.lock_time = wtx.tx->nLockTime;
7070
result.is_final = locked_chain.checkFinalTx(*wtx.tx);
7171
result.is_trusted = wtx.IsTrusted(locked_chain);
7272
result.is_abandoned = wtx.isAbandoned();
7373
result.is_coinbase = wtx.IsCoinBase();
74-
result.is_in_main_chain = wtx.IsInMainChain(locked_chain);
74+
result.is_in_main_chain = wtx.IsInMainChain();
7575
return result;
7676
}
7777

7878
//! Construct wallet TxOut struct.
79-
WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain,
80-
CWallet& wallet,
79+
WalletTxOut MakeWalletTxOut(CWallet& wallet,
8180
const CWalletTx& wtx,
8281
int n,
8382
int depth) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
@@ -86,7 +85,7 @@ WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain,
8685
result.txout = wtx.tx->vout[n];
8786
result.time = wtx.GetTxTime();
8887
result.depth_in_main_chain = depth;
89-
result.is_spent = wallet.IsSpent(locked_chain, wtx.GetHash(), n);
88+
result.is_spent = wallet.IsSpent(wtx.GetHash(), n);
9089
return result;
9190
}
9291

@@ -235,7 +234,7 @@ class WalletImpl : public Wallet
235234
{
236235
auto locked_chain = m_wallet->chain().lock();
237236
LOCK(m_wallet->cs_wallet);
238-
return m_wallet->AbandonTransaction(*locked_chain, txid);
237+
return m_wallet->AbandonTransaction(txid);
239238
}
240239
bool transactionCanBeBumped(const uint256& txid) override
241240
{
@@ -282,7 +281,7 @@ class WalletImpl : public Wallet
282281
LOCK(m_wallet->cs_wallet);
283282
auto mi = m_wallet->mapWallet.find(txid);
284283
if (mi != m_wallet->mapWallet.end()) {
285-
return MakeWalletTx(*locked_chain, *m_wallet, mi->second);
284+
return MakeWalletTx(*m_wallet, mi->second);
286285
}
287286
return {};
288287
}
@@ -293,7 +292,7 @@ class WalletImpl : public Wallet
293292
std::vector<WalletTx> result;
294293
result.reserve(m_wallet->mapWallet.size());
295294
for (const auto& entry : m_wallet->mapWallet) {
296-
result.emplace_back(MakeWalletTx(*locked_chain, *m_wallet, entry.second));
295+
result.emplace_back(MakeWalletTx(*m_wallet, entry.second));
297296
}
298297
return result;
299298
}
@@ -338,7 +337,7 @@ class WalletImpl : public Wallet
338337
in_mempool = mi->second.InMempool();
339338
order_form = mi->second.vOrderForm;
340339
tx_status = MakeWalletTxStatus(*locked_chain, mi->second);
341-
return MakeWalletTx(*locked_chain, *m_wallet, mi->second);
340+
return MakeWalletTx(*m_wallet, mi->second);
342341
}
343342
return {};
344343
}
@@ -407,7 +406,7 @@ class WalletImpl : public Wallet
407406
auto& group = result[entry.first];
408407
for (const auto& coin : entry.second) {
409408
group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i),
410-
MakeWalletTxOut(*locked_chain, *m_wallet, *coin.tx, coin.i, coin.nDepth));
409+
MakeWalletTxOut(*m_wallet, *coin.tx, coin.i, coin.nDepth));
411410
}
412411
}
413412
return result;
@@ -422,9 +421,9 @@ class WalletImpl : public Wallet
422421
result.emplace_back();
423422
auto it = m_wallet->mapWallet.find(output.hash);
424423
if (it != m_wallet->mapWallet.end()) {
425-
int depth = it->second.GetDepthInMainChain(*locked_chain);
424+
int depth = it->second.GetDepthInMainChain();
426425
if (depth >= 0) {
427-
result.back() = MakeWalletTxOut(*locked_chain, *m_wallet, it->second, output.n, depth);
426+
result.back() = MakeWalletTxOut(*m_wallet, it->second, output.n, depth);
428427
}
429428
}
430429
}

‎src/wallet/feebumper.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
//! Check whether transaction has descendant in wallet or mempool, or has been
1818
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
19-
static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet& wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
19+
static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
2020
{
2121
if (wallet.HasWalletSpend(wtx.GetHash())) {
2222
errors.push_back("Transaction has descendants in the wallet");
@@ -30,7 +30,7 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai
3030
}
3131
}
3232

33-
if (wtx.GetDepthInMainChain(locked_chain) != 0) {
33+
if (wtx.GetDepthInMainChain() != 0) {
3434
errors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
3535
return feebumper::Result::WALLET_ERROR;
3636
}
@@ -146,7 +146,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
146146
if (wtx == nullptr) return false;
147147

148148
std::vector<std::string> errors_dummy;
149-
feebumper::Result res = PreconditionChecks(*locked_chain, wallet, *wtx, errors_dummy);
149+
feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
150150
return res == feebumper::Result::OK;
151151
}
152152

@@ -165,7 +165,7 @@ Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, co
165165
}
166166
const CWalletTx& wtx = it->second;
167167

168-
Result result = PreconditionChecks(*locked_chain, *wallet, wtx, errors);
168+
Result result = PreconditionChecks(*wallet, wtx, errors);
169169
if (result != Result::OK) {
170170
return result;
171171
}
@@ -291,7 +291,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
291291
}
292292
const CWalletTx& wtx = it->second;
293293

294-
Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors);
294+
Result result = PreconditionChecks(wallet, wtx, errors);
295295
if (result != Result::OK) {
296296
return result;
297297
}
@@ -382,7 +382,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti
382382
CWalletTx& oldWtx = it->second;
383383

384384
// make sure the transaction still has no descendants and hasn't been mined in the meantime
385-
Result result = PreconditionChecks(*locked_chain, wallet, oldWtx, errors);
385+
Result result = PreconditionChecks(wallet, oldWtx, errors);
386386
if (result != Result::OK) {
387387
return result;
388388
}

‎src/wallet/rpcdump.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ UniValue importaddress(const JSONRPCRequest& request)
325325
{
326326
auto locked_chain = pwallet->chain().lock();
327327
LOCK(pwallet->cs_wallet);
328-
pwallet->ReacceptWalletTransactions(*locked_chain);
328+
pwallet->ReacceptWalletTransactions();
329329
}
330330
}
331331

@@ -514,7 +514,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
514514
{
515515
auto locked_chain = pwallet->chain().lock();
516516
LOCK(pwallet->cs_wallet);
517-
pwallet->ReacceptWalletTransactions(*locked_chain);
517+
pwallet->ReacceptWalletTransactions();
518518
}
519519
}
520520

@@ -1413,7 +1413,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
14131413
{
14141414
auto locked_chain = pwallet->chain().lock();
14151415
LOCK(pwallet->cs_wallet);
1416-
pwallet->ReacceptWalletTransactions(*locked_chain);
1416+
pwallet->ReacceptWalletTransactions();
14171417
}
14181418

14191419
if (pwallet->IsAbortingRescan()) {

‎src/wallet/rpcwallet.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ void EnsureWalletIsUnlocked(const CWallet* pwallet)
126126

127127
static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry)
128128
{
129-
int confirms = wtx.GetDepthInMainChain(locked_chain);
129+
int confirms = wtx.GetDepthInMainChain();
130130
entry.pushKV("confirmations", confirms);
131131
if (wtx.IsCoinBase())
132132
entry.pushKV("generated", true);
@@ -631,7 +631,7 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request)
631631

632632
for (const CTxOut& txout : wtx.tx->vout)
633633
if (txout.scriptPubKey == scriptPubKey)
634-
if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth)
634+
if (wtx.GetDepthInMainChain() >= nMinDepth)
635635
nAmount += txout.nValue;
636636
}
637637

@@ -697,7 +697,7 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request)
697697
{
698698
CTxDestination address;
699699
if (ExtractDestination(txout.scriptPubKey, address) && pwallet->IsMine(address) && setAddress.count(address)) {
700-
if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth)
700+
if (wtx.GetDepthInMainChain() >= nMinDepth)
701701
nAmount += txout.nValue;
702702
}
703703
}
@@ -1057,7 +1057,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co
10571057
continue;
10581058
}
10591059

1060-
int nDepth = wtx.GetDepthInMainChain(locked_chain);
1060+
int nDepth = wtx.GetDepthInMainChain();
10611061
if (nDepth < nMinDepth)
10621062
continue;
10631063

@@ -1314,8 +1314,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
13141314
}
13151315

13161316
// Received
1317-
if (listReceived.size() > 0 && wtx.GetDepthInMainChain(locked_chain) >= nMinDepth)
1318-
{
1317+
if (listReceived.size() > 0 && wtx.GetDepthInMainChain() >= nMinDepth) {
13191318
for (const COutputEntry& r : listReceived)
13201319
{
13211320
std::string label;
@@ -1332,9 +1331,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con
13321331
MaybePushAddress(entry, r.destination);
13331332
if (wtx.IsCoinBase())
13341333
{
1335-
if (wtx.GetDepthInMainChain(locked_chain) < 1)
1334+
if (wtx.GetDepthInMainChain() < 1)
13361335
entry.pushKV("category", "orphan");
1337-
else if (wtx.IsImmatureCoinBase(locked_chain))
1336+
else if (wtx.IsImmatureCoinBase())
13381337
entry.pushKV("category", "immature");
13391338
else
13401339
entry.pushKV("category", "generate");
@@ -1598,7 +1597,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
15981597
for (const std::pair<const uint256, CWalletTx>& pairWtx : pwallet->mapWallet) {
15991598
CWalletTx tx = pairWtx.second;
16001599

1601-
if (depth == -1 || abs(tx.GetDepthInMainChain(*locked_chain)) < depth) {
1600+
if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) {
16021601
ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */);
16031602
}
16041603
}
@@ -1715,7 +1714,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
17151714
}
17161715
const CWalletTx& wtx = it->second;
17171716

1718-
CAmount nCredit = wtx.GetCredit(*locked_chain, filter);
1717+
CAmount nCredit = wtx.GetCredit(filter);
17191718
CAmount nDebit = wtx.GetDebit(filter);
17201719
CAmount nNet = nCredit - nDebit;
17211720
CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0);
@@ -1779,7 +1778,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request)
17791778
if (!pwallet->mapWallet.count(hash)) {
17801779
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
17811780
}
1782-
if (!pwallet->AbandonTransaction(*locked_chain, hash)) {
1781+
if (!pwallet->AbandonTransaction(hash)) {
17831782
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not eligible for abandonment");
17841783
}
17851784

@@ -2210,7 +2209,7 @@ static UniValue lockunspent(const JSONRPCRequest& request)
22102209
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout index out of bounds");
22112210
}
22122211

2213-
if (pwallet->IsSpent(*locked_chain, outpt.hash, outpt.n)) {
2212+
if (pwallet->IsSpent(outpt.hash, outpt.n)) {
22142213
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected unspent output");
22152214
}
22162215

‎src/wallet/test/wallet_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,13 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
281281

282282
// Call GetImmatureCredit() once before adding the key to the wallet to
283283
// cache the current immature credit amount, which is 0.
284-
BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 0);
284+
BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 0);
285285

286286
// Invalidate the cached vanue, add the key, and make sure a new immature
287287
// credit amount is calculated.
288288
wtx.MarkDirty();
289289
BOOST_CHECK(spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()));
290-
BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 50*COIN);
290+
BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 50*COIN);
291291
}
292292

293293
static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)

0 commit comments

Comments
 (0)