Skip to content

Commit 99471e6

Browse files
kwvgkwvg
committed
wallet, interfaces: replace LockAnnotations as implicit lock no longer exists, inverse lock order
1 parent a104670 commit 99471e6

File tree

12 files changed

+38
-47
lines changed

12 files changed

+38
-47
lines changed

‎src/coinjoin/client.cpp‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -523,8 +523,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
523523
if (fMasternodeMode || pnode == nullptr) return false;
524524
if (!mixingMasternode) return false;
525525

526-
LOCK(cs_main);
527-
LOCK(mixingWallet.cs_wallet);
526+
LOCK2(mixingWallet.cs_wallet, cs_main);
528527
LOCK(cs_coinjoin);
529528

530529
finalMutableTransaction = CMutableTransaction{finalTransactionNew};
@@ -749,7 +748,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(CConnman& connman, bool fDr
749748
CAmount nBalanceNeedsAnonymized;
750749

751750
{
752-
LOCK2(cs_main, mixingWallet.cs_wallet);
751+
LOCK2(mixingWallet.cs_wallet, cs_main);
753752

754753
if (!fDryRun && mixingWallet.IsLocked(true)) {
755754
strAutoDenomResult = _("Wallet is locked.");
@@ -900,7 +899,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(CConnman& connman, bool fDr
900899
mixingWallet.LockCoin(txin.prevout);
901900
vecOutPointLocked.push_back(txin.prevout);
902901
}
903-
} // LOCK2(cs_main, mixingWallet.cs_wallet);
902+
} // LOCK2(mixingWallet.cs_wallet, cs_main);
904903

905904
// Always attempt to join an existing queue
906905
if (JoinExistingQueue(nBalanceNeedsAnonymized, connman)) {
@@ -1211,7 +1210,7 @@ bool CCoinJoinClientManager::MarkAlreadyJoinedQueueAsTried(CCoinJoinQueue& dsq)
12111210

12121211
bool CCoinJoinClientSession::SubmitDenominate(CConnman& connman)
12131212
{
1214-
LOCK2(cs_main, mixingWallet.cs_wallet);
1213+
LOCK2(mixingWallet.cs_wallet, cs_main);
12151214

12161215
std::string strError;
12171216
std::vector<CTxDSIn> vecTxDSIn;
@@ -1354,7 +1353,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts()
13541353
{
13551354
if (!CCoinJoinClientOptions::IsEnabled()) return false;
13561355

1357-
LOCK2(cs_main, mixingWallet.cs_wallet);
1356+
LOCK2(mixingWallet.cs_wallet, cs_main);
13581357

13591358
// NOTE: We do not allow txes larger than 100 kB, so we have to limit number of inputs here.
13601359
// We still want to consume a lot of inputs to avoid creating only smaller denoms though.
@@ -1536,7 +1535,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate)
15361535
{
15371536
if (!CCoinJoinClientOptions::IsEnabled()) return false;
15381537

1539-
LOCK2(cs_main, mixingWallet.cs_wallet);
1538+
LOCK2(mixingWallet.cs_wallet, cs_main);
15401539

15411540
// NOTE: We do not allow txes larger than 100 kB, so we have to limit number of inputs here.
15421541
// We still want to consume a lot of inputs to avoid creating only smaller denoms though.

‎src/interfaces/chain.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class ChainImpl : public Chain
132132
explicit ChainImpl(NodeContext& node) : m_node(node) {}
133133
Optional<int> getHeight() override
134134
{
135-
LockAnnotation lock(::cs_main);
135+
LOCK(cs_main);
136136
int height = ::ChainActive().Height();
137137
if (height >= 0) {
138138
return height;
@@ -141,7 +141,7 @@ class ChainImpl : public Chain
141141
}
142142
Optional<int> getBlockHeight(const uint256& hash) override
143143
{
144-
LockAnnotation lock(::cs_main);
144+
LOCK(cs_main);
145145
CBlockIndex* block = LookupBlockIndex(hash);
146146
if (block && ::ChainActive().Contains(block)) {
147147
return block->nHeight;
@@ -150,34 +150,34 @@ class ChainImpl : public Chain
150150
}
151151
uint256 getBlockHash(int height) override
152152
{
153-
LockAnnotation lock(::cs_main);
153+
LOCK(cs_main);
154154
CBlockIndex* block = ::ChainActive()[height];
155155
assert(block != nullptr);
156156
return block->GetBlockHash();
157157
}
158158
int64_t getBlockTime(int height) override
159159
{
160-
LockAnnotation lock(::cs_main);
160+
LOCK(cs_main);
161161
CBlockIndex* block = ::ChainActive()[height];
162162
assert(block != nullptr);
163163
return block->GetBlockTime();
164164
}
165165
int64_t getBlockMedianTimePast(int height) override
166166
{
167-
LockAnnotation lock(::cs_main);
167+
LOCK(cs_main);
168168
CBlockIndex* block = ::ChainActive()[height];
169169
assert(block != nullptr);
170170
return block->GetMedianTimePast();
171171
}
172172
bool haveBlockOnDisk(int height) override
173173
{
174-
LockAnnotation lock(::cs_main);
174+
LOCK(cs_main);
175175
CBlockIndex* block = ::ChainActive()[height];
176176
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
177177
}
178178
Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override
179179
{
180-
LockAnnotation lock(::cs_main);
180+
LOCK(cs_main);
181181
CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time, height);
182182
if (block) {
183183
if (hash) *hash = block->GetBlockHash();
@@ -187,7 +187,7 @@ class ChainImpl : public Chain
187187
}
188188
Optional<int> findPruned(int start_height, Optional<int> stop_height) override
189189
{
190-
LockAnnotation lock(::cs_main);
190+
LOCK(cs_main);
191191
if (::fPruneMode) {
192192
CBlockIndex* block = stop_height ? ::ChainActive()[*stop_height] : ::ChainActive().Tip();
193193
while (block && block->nHeight >= start_height) {
@@ -201,7 +201,7 @@ class ChainImpl : public Chain
201201
}
202202
Optional<int> findFork(const uint256& hash, Optional<int>* height) override
203203
{
204-
LockAnnotation lock(::cs_main);
204+
LOCK(cs_main);
205205
const CBlockIndex* block = LookupBlockIndex(hash);
206206
const CBlockIndex* fork = block ? ::ChainActive().FindFork(block) : nullptr;
207207
if (height) {
@@ -218,20 +218,20 @@ class ChainImpl : public Chain
218218
}
219219
CBlockLocator getTipLocator() override
220220
{
221-
LockAnnotation lock(::cs_main);
221+
LOCK(cs_main);
222222
return ::ChainActive().GetLocator();
223223
}
224224
Optional<int> findLocatorFork(const CBlockLocator& locator) override
225225
{
226-
LockAnnotation lock(::cs_main);
226+
LOCK(cs_main);
227227
if (CBlockIndex* fork = FindForkInGlobalIndex(::ChainActive(), locator)) {
228228
return fork->nHeight;
229229
}
230230
return nullopt;
231231
}
232232
bool checkFinalTx(const CTransaction& tx) override
233233
{
234-
LockAnnotation lock(::cs_main);
234+
LOCK(cs_main);
235235
return CheckFinalTx(tx);
236236
}
237237
bool findBlock(const uint256& hash, CBlock* block, int64_t* time, int64_t* time_max) override

‎src/interfaces/wallet.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class WalletImpl : public Wallet
266266
}
267267
void listProTxCoins(std::vector<COutPoint>& outputs) override
268268
{
269-
LOCK2(cs_main, m_wallet->cs_wallet);
269+
LOCK2(m_wallet->cs_wallet, cs_main);
270270
return m_wallet->ListProTxCoins(outputs);
271271
}
272272
CTransactionRef createTransaction(const std::vector<CRecipient>& recipients,

‎src/qt/governancelist.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ QString Proposal::url() const { return m_url; }
6464

6565
bool Proposal::isActive() const
6666
{
67-
std::string strError;
6867
LOCK(cs_main);
68+
std::string strError;
6969
return govObj.IsValidLocally(strError, false);
7070
}
7171

‎src/qt/test/wallettests.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,7 @@ void TestGUI(interfaces::Node& node)
121121
wallet->SetLastBlockProcessed(105, ::ChainActive().Tip()->GetBlockHash());
122122
}
123123
{
124-
LockAnnotation lock(::cs_main);
125-
124+
LOCK(cs_main);
126125
WalletRescanReserver reserver(wallet.get());
127126
reserver.reserve();
128127
CWallet::ScanResult result = wallet->ScanForWalletTransactions(wallet->chain().getBlockHash(0), {} /* stop_block */, reserver, true /* fUpdate */);

‎src/rpc/rpcevo.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,7 @@ static UniValue protx_list(const JSONRPCRequest& request)
10211021
throw std::runtime_error("\"protx list wallet\" not supported when wallet is disabled");
10221022
}
10231023
#ifdef ENABLE_WALLET
1024-
LOCK2(cs_main, pwallet->cs_wallet);
1024+
LOCK2(pwallet->cs_wallet, cs_main);
10251025

10261026
if (request.params.size() > 4) {
10271027
protx_list_help(request);

‎src/wallet/rpcdump.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)
927927
if (!wallet) return NullUniValue;
928928
CWallet* const pwallet = wallet.get();
929929

930-
LockAnnotation lock(::cs_main);
931-
LOCK(pwallet->cs_wallet);
930+
LOCK2(pwallet->cs_wallet, cs_main);
932931

933932
EnsureWalletIsUnlocked(pwallet);
934933

‎src/wallet/rpcwallet.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,8 +1567,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
15671567
// the user could have gotten from another RPC command prior to now
15681568
pwallet->BlockUntilSyncedToCurrentChain();
15691569

1570-
LockAnnotation lock(::cs_main);
1571-
LOCK(pwallet->cs_wallet);
1570+
LOCK2(pwallet->cs_wallet, cs_main);
15721571

15731572
// The way the 'height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0.
15741573
Optional<int> height = MakeOptional(false, int()); // Height of the specified block or the common ancestor, if the block provided was in a deactivated chain.

‎src/wallet/test/coinselector_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins)
125125
BOOST_AUTO_TEST_CASE(bnb_search_test)
126126
{
127127

128-
LOCK2(cs_main, testWallet.cs_wallet);
128+
LOCK2(testWallet.cs_wallet, cs_main);
129129

130130
// Setup
131131
std::vector<CInputCoin> utxo_pool;
@@ -284,7 +284,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
284284
CAmount nValueRet;
285285
bool bnb_used;
286286

287-
LOCK2(cs_main, testWallet.cs_wallet);
287+
LOCK2(testWallet.cs_wallet, cs_main);
288288

289289
// test multiple times to allow for differences in the shuffle order
290290
for (int i = 0; i < RUN_TESTS; i++)
@@ -563,7 +563,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
563563
CAmount nValueRet;
564564
bool bnb_used;
565565

566-
LOCK2(cs_main, testWallet.cs_wallet);
566+
LOCK2(testWallet.cs_wallet, cs_main);
567567

568568
empty_wallet();
569569

@@ -587,7 +587,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
587587
std::exponential_distribution<double> distribution (100);
588588
FastRandomContext rand;
589589

590-
LOCK2(cs_main, testWallet.cs_wallet);
590+
LOCK2(testWallet.cs_wallet, cs_main);
591591

592592
// Run this test 100 times
593593
for (int i = 0; i < 100; ++i)

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
270270
CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
271271
CWalletTx wtx(&wallet, m_coinbase_txns.back());
272272

273-
LockAnnotation lock(::cs_main);
274-
LOCK(wallet.cs_wallet);
273+
LOCK2(wallet.cs_wallet, cs_main);
275274
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
276275

277276
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 0);
@@ -295,7 +294,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
295294
SetMockTime(mockTime);
296295
CBlockIndex* block = nullptr;
297296
if (blockTime > 0) {
298-
LockAnnotation lock(::cs_main); // for mapBlockIndex
297+
LOCK(cs_main);
299298
auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex);
300299
assert(inserted.second);
301300
const uint256& hash = inserted.first->first;
@@ -305,8 +304,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
305304
}
306305

307306
CWalletTx wtx(&wallet, MakeTransactionRef(tx));
308-
LOCK(cs_main);
309-
LOCK(wallet.cs_wallet);
307+
LOCK2(wallet.cs_wallet, cs_main);
310308
// If transaction is already in map, to avoid inconsistencies, unconfirmation
311309
// is needed before confirm again with different block.
312310
std::map<uint256, CWalletTx>::iterator it = wallet.mapWallet.find(wtx.GetHash());
@@ -410,8 +408,7 @@ class ListCoinsTestingSetup : public TestChain100Setup
410408
}
411409
CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
412410

413-
LOCK(cs_main);
414-
LOCK(wallet->cs_wallet);
411+
LOCK2(wallet->cs_wallet, cs_main);
415412
wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash());
416413
auto it = wallet->mapWallet.find(tx->GetHash());
417414
BOOST_CHECK(it != wallet->mapWallet.end());
@@ -621,7 +618,7 @@ class CreateTransactionTestSetup : public TestChain100Setup
621618
blocktx = CMutableTransaction(*tx);
622619
}
623620
CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
624-
LOCK2(cs_main, wallet->cs_wallet);
621+
LOCK2(wallet->cs_wallet, cs_main);
625622
auto it = wallet->mapWallet.find(tx->GetHash());
626623
BOOST_CHECK(it != wallet->mapWallet.end());
627624
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@@ -959,7 +956,7 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup
959956
CreateAndProcessBlock({CMutableTransaction(*tx2)}, GetScriptForRawPubKey({}));
960957

961958
{
962-
LOCK2(cs_main, wallet->cs_wallet);
959+
LOCK2(wallet->cs_wallet, cs_main);
963960
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
964961
}
965962

0 commit comments

Comments
 (0)