Skip to content

Commit 6e8a420

Browse files
codeofalltradescodeofalltrades
committed
Merge #952: [Wallet][Consensus] Resolve misc clang lock annotation warnings.
8d58da5 Resolve misc clang lock annotation warnings. (Zannick) Pull request description: ### Problem Building with clang results in lots of warnings about lock annotations not being obeyed. ### Solution Follow instructions on warnings: - Replace double lock of cs_main with an assertion. - Reorganize code to release cs_main before calling blockheaderToJSON. - Lock cs_main before calling any of: GetBlocksToMaturity, GetAddressBalances, IsSpent, TestBlockValidity. ### Tested Configured with --with-sanitizers=thread, built with clang, run on regtest. Tree-SHA512: af09c9c9ec0264dffd3914f8d278604e8d22e8ecc815a1b1586057f48131fe82550a7badf86cfcb6d411509aca003944eda66b6473a32a14d319db9fea57806d
2 parents 8736fc7 + 8d58da5 commit 6e8a420

5 files changed

Lines changed: 55 additions & 30 deletions

File tree

‎src/interfaces/wallet.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ WalletTx MakeWalletTx(CHDWallet& wallet, MapRecords_t::const_iterator irtx)
235235
//! Construct wallet tx status struct.
236236
WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx)
237237
{
238+
LOCK(cs_main);
238239
WalletTxStatus result;
239240
auto mi = ::mapBlockIndex.find(wtx.hashBlock);
240241
CBlockIndex* block = mi != ::mapBlockIndex.end() ? mi->second : nullptr;

‎src/miner.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
499499
if(fProofOfFullNode && !fProofOfStake)
500500
LogPrint(BCLog::BLOCKCREATION, "%s: A block can not be proof of full node and proof of work.\n", __func__);
501501
else if(fProofOfFullNode && fProofOfStake) {
502-
LOCK(cs_main);
502+
AssertLockHeld(cs_main);
503503
pblock->hashPoFN = veil::GetFullNodeHash(*pblock, pindexPrev);
504504
}
505505

‎src/rpc/blockchain.cpp‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -960,29 +960,35 @@ static UniValue getblockheader(const JSONRPCRequest& request)
960960
+ HelpExampleRpc("getblockheader", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\"")
961961
);
962962

963-
LOCK(cs_main);
963+
const CBlockIndex* pChainTip;
964+
const CBlockIndex* pblockindex;
965+
{
966+
LOCK(cs_main);
964967

965-
std::string strHash = request.params[0].get_str();
966-
uint256 hash(uint256S(strHash));
968+
std::string strHash = request.params[0].get_str();
969+
uint256 hash(uint256S(strHash));
967970

968-
bool fVerbose = true;
969-
if (!request.params[1].isNull())
970-
fVerbose = request.params[1].get_bool();
971+
bool fVerbose = true;
972+
if (!request.params[1].isNull())
973+
fVerbose = request.params[1].get_bool();
971974

972-
const CBlockIndex* pblockindex = LookupBlockIndex(hash);
973-
if (!pblockindex) {
974-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
975-
}
975+
pblockindex = LookupBlockIndex(hash);
976+
if (!pblockindex) {
977+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
978+
}
976979

977-
if (!fVerbose)
978-
{
979-
CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION);
980-
ssBlock << pblockindex->GetBlockHeader();
981-
std::string strHex = HexStr(ssBlock.begin(), ssBlock.end());
982-
return strHex;
980+
if (!fVerbose)
981+
{
982+
CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION);
983+
ssBlock << pblockindex->GetBlockHeader();
984+
std::string strHex = HexStr(ssBlock.begin(), ssBlock.end());
985+
return strHex;
986+
}
987+
988+
pChainTip = chainActive.Tip();
983989
}
984990

985-
return blockheaderToJSON(chainActive.Tip(), pblockindex);
991+
return blockheaderToJSON(pChainTip, pblockindex);
986992
}
987993

988994
static CBlock GetBlockChecked(const CBlockIndex* pblockindex)

‎src/wallet/rpcwallet.cpp‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ static UniValue listaddresses(const JSONRPCRequest& request)
721721
throw JSONRPCError(RPC_INVALID_PARAMETER, type + " is not a valid address type");
722722
}
723723
}
724-
LOCK(pwallet->cs_wallet);
724+
LOCK2(cs_main, pwallet->cs_wallet);
725725

726726
UniValue results(UniValue::VARR);
727727
AnonWallet* pAnonWallet = wallet->GetAnonWallet();
@@ -2110,10 +2110,18 @@ static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const
21102110
{
21112111
if (wtx.GetDepthInMainChain() < 1)
21122112
entry.pushKV("category", "orphan");
2113-
else if (wtx.GetBlocksToMaturity() > 0)
2114-
entry.pushKV("category", "immature");
21152113
else
2116-
entry.pushKV("category", "generate");
2114+
{
2115+
int nBlocksToMaturity;
2116+
{
2117+
LOCK(cs_main);
2118+
nBlocksToMaturity = wtx.GetBlocksToMaturity();
2119+
}
2120+
if (nBlocksToMaturity > 0)
2121+
entry.pushKV("category", "immature");
2122+
else
2123+
entry.pushKV("category", "generate");
2124+
}
21172125
}
21182126
else
21192127
{

‎src/wallet/wallet.cpp‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -771,8 +771,10 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const
771771
}
772772
}
773773

774-
if (setAnonTx.count(hash))
774+
if (setAnonTx.count(hash)) {
775+
LOCK(cs_main);
775776
return pAnonWalletMain->IsSpent(hash, n);
777+
}
776778

777779
return false;
778780
}
@@ -2310,9 +2312,12 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
23102312

23112313
CAmount CWalletTx::GetCredit(const isminefilter& filter, bool fResetCache) const
23122314
{
2313-
// Must wait until coinbase is safely deep enough in the chain before valuing it
2314-
if (IsCoinBase() && GetBlocksToMaturity() > 0)
2315-
return 0;
2315+
{
2316+
LOCK(cs_main);
2317+
// Must wait until coinbase is safely deep enough in the chain before valuing it
2318+
if (IsCoinBase() && GetBlocksToMaturity() > 0)
2319+
return 0;
2320+
}
23162321

23172322
CAmount credit = 0;
23182323
if (filter & ISMINE_SPENDABLE)
@@ -2345,6 +2350,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter, bool fResetCache) const
23452350

23462351
CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const
23472352
{
2353+
LOCK(cs_main);
23482354
if (IsCoinBase() && GetBlocksToMaturity() > 0 && IsInMainChain())
23492355
{
23502356
if (fUseCache && fImmatureCreditCached)
@@ -2362,9 +2368,12 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter
23622368
if (pwallet == nullptr)
23632369
return 0;
23642370

2365-
// Must wait until coinbase is safely deep enough in the chain before valuing it
2366-
if (IsCoinBase() && GetBlocksToMaturity() > 0)
2367-
return 0;
2371+
{
2372+
LOCK(cs_main);
2373+
// Must wait until coinbase is safely deep enough in the chain before valuing it
2374+
if (IsCoinBase() && GetBlocksToMaturity() > 0)
2375+
return 0;
2376+
}
23682377

23692378
CAmount* cache = nullptr;
23702379
bool* cache_used = nullptr;
@@ -2408,6 +2417,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter
24082417

24092418
CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const
24102419
{
2420+
LOCK(cs_main);
24112421
if (IsCoinBase() && GetBlocksToMaturity() > 0 && IsInMainChain())
24122422
{
24132423
if (fUseCache && fImmatureWatchCreditCached)
@@ -4496,7 +4506,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances()
44964506
std::map<CTxDestination, CAmount> balances;
44974507

44984508
{
4499-
LOCK(cs_wallet);
4509+
LOCK2(cs_main, cs_wallet);
45004510
for (const auto& walletEntry : mapWallet)
45014511
{
45024512
const CWalletTx *pcoin = &walletEntry.second;

0 commit comments

Comments
 (0)