Skip to content

Commit

Permalink
Add compile time checking for all cs_main runtime locking assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
practicalswift committed Aug 25, 2018
1 parent f6eb85d commit 9e0a514
Show file tree
Hide file tree
Showing 13 changed files with 54 additions and 52 deletions.
6 changes: 3 additions & 3 deletions src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class PendingWalletTxImpl : public PendingWalletTx
};

//! Construct wallet tx struct.
WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
WalletTx result;
result.tx = wtx.tx;
Expand Down Expand Up @@ -85,7 +85,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
}

//! Construct wallet tx status struct.
WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx)
static WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
WalletTxStatus result;
auto mi = ::mapBlockIndex.find(wtx.hashBlock);
Expand All @@ -104,7 +104,7 @@ WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx)
}

//! Construct wallet TxOut struct.
WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int depth)
static WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int depth) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
WalletTxOut result;
result.txout = wtx.tx->vout[n];
Expand Down
8 changes: 4 additions & 4 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) EXCLUSIV
* lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by
* removing the first element if necessary.
*/
static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman)
static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
CNodeState* nodestate = State(nodeid);
Expand Down Expand Up @@ -831,7 +831,7 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
// active chain if they are no more than a month older (both in time, and in
// best equivalent proof of work) than the best header chain we know about and
// we fully-validated them at some point.
static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams)
static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
if (chainActive.Contains(pindex)) return true;
Expand Down Expand Up @@ -1260,7 +1260,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
}
}

void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc)
void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc) LOCKS_EXCLUDED(cs_main)
{
AssertLockNotHeld(cs_main);

Expand Down Expand Up @@ -2925,7 +2925,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true;
}

static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool enable_bip61)
static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
CNodeState &state = *State(pnode->GetId());
Expand Down
2 changes: 1 addition & 1 deletion src/net_processing.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI
bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing);

/** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
void ConsiderEviction(CNode *pto, int64_t time_in_seconds);
void ConsiderEviction(CNode *pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound */
void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams);
/** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */
Expand Down
2 changes: 1 addition & 1 deletion src/test/miner_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static CBlockIndex CreateBlockIndex(int nHeight)
return index;
}

static bool TestSequenceLocks(const CTransaction &tx, int flags)
static bool TestSequenceLocks(const CTransaction &tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
LOCK(mempool.cs);
return CheckSequenceLocks(tx, flags);
Expand Down
2 changes: 1 addition & 1 deletion src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso
}
}

void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags)
void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
// Remove transactions spending a coinbase which are now immature and no-longer-final transactions
LOCK(cs);
Expand Down
29 changes: 15 additions & 14 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class CChainState {
// Block (dis)connection on a given view:
DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view);
bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex,
CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false);
CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

// Block disconnection on our pcoinsTip:
bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool);
Expand All @@ -189,8 +189,8 @@ class CChainState {
void UnloadBlockIndex();

private:
bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace);
bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool);
bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Create a new block index entry for a given block hash */
Expand All @@ -202,7 +202,7 @@ class CChainState {
*/
void CheckBlockIndex(const Consensus::Params& consensusParams);

void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state);
void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const CDiskBlockPos& pos, const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

Expand Down Expand Up @@ -457,7 +457,7 @@ std::string FormatStateMessage(const CValidationState &state)
state.GetRejectCode());
}

static bool IsCurrentForFeeEstimation()
static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
if (IsInitialBlockDownload())
Expand All @@ -482,7 +482,7 @@ static bool IsCurrentForFeeEstimation()
* and instead just erase from the mempool as needed.
*/

static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool)
static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
std::vector<uint256> vHashUpdate;
Expand Down Expand Up @@ -524,7 +524,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool,
// Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool
// were somehow broken and returning the wrong scriptPubKeys
static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool,
unsigned int flags, bool cacheSigStore, PrecomputedTransactionData& txdata) {
unsigned int flags, bool cacheSigStore, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
AssertLockHeld(cs_main);

// pool.cs should be locked already, but go ahead and re-take the lock here
Expand Down Expand Up @@ -559,7 +559,7 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt

static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx,
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache, bool test_accept)
bool bypass_limits, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
const CTransaction& tx = *ptx;
const uint256 hash = tx.GetHash();
Expand Down Expand Up @@ -977,7 +977,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
/** (try to) add transaction to memory pool with a specified acceptance time **/
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx,
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept)
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
std::vector<COutPoint> coins_to_uncache;
bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept);
Expand Down Expand Up @@ -1216,7 +1216,7 @@ static void AlertNotify(const std::string& strMessage)
t.detach(); // thread runs free
}

static void CheckForkWarningConditions()
static void CheckForkWarningConditions() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
// Before we get past initial download, we cannot reliably alert about forks
Expand Down Expand Up @@ -1257,7 +1257,7 @@ static void CheckForkWarningConditions()
}
}

static void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip)
static void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
// If we are on a fork that is sufficiently large, set a warning flag
Expand Down Expand Up @@ -1290,7 +1290,7 @@ static void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip)
CheckForkWarningConditions();
}

void static InvalidChainFound(CBlockIndex* pindexNew)
void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
if (!pindexBestInvalid || pindexNew->nChainWork > pindexBestInvalid->nChainWork)
pindexBestInvalid = pindexNew;
Expand Down Expand Up @@ -1377,7 +1377,7 @@ void InitScriptExecutionCache() {
*
* Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp
*/
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks)
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
if (!tx.IsCoinBase())
{
Expand Down Expand Up @@ -1743,7 +1743,7 @@ static bool IsScriptWitnessEnabled(const Consensus::Params& params)
return params.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nTimeout != 0;
}

static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) {
static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
AssertLockHeld(cs_main);

unsigned int flags = SCRIPT_VERIFY_NONE;
Expand Down Expand Up @@ -2863,6 +2863,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c
}
return true;
}

bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) {
return g_chainstate.InvalidateBlock(state, chainparams, pindex);
}
Expand Down
8 changes: 4 additions & 4 deletions src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ void PruneBlockFilesManual(int nManualPruneHeight);
* plTxnReplaced will be appended to with all transactions replaced from mempool **/
bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx,
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false);
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Convert CValidationState to a human-readable message for logging */
std::string FormatStateMessage(const CValidationState &state);
Expand All @@ -329,12 +329,12 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
*
* See consensus/consensus.h for flag definitions.
*/
bool CheckFinalTx(const CTransaction &tx, int flags = -1);
bool CheckFinalTx(const CTransaction &tx, int flags = -1) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Test whether the LockPoints height and time are still valid on the current chain
*/
bool TestLockPointValidity(const LockPoints* lp);
bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Check if transaction will be BIP 68 final in the next block to be created.
Expand All @@ -347,7 +347,7 @@ bool TestLockPointValidity(const LockPoints* lp);
*
* See consensus/consensus.h for flag definitions.
*/
bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false);
bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Closure representing one script verification
Expand Down
1 change: 0 additions & 1 deletion src/validationinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <primitives/block.h>
#include <scheduler.h>
#include <sync.h>
#include <txmempool.h>
#include <util.h>
#include <validation.h>
Expand Down
4 changes: 3 additions & 1 deletion src/validationinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
#define BITCOIN_VALIDATIONINTERFACE_H

#include <primitives/transaction.h> // CTransaction(Ref)
#include <sync.h>

#include <functional>
#include <memory>

extern CCriticalSection cs_main;
class CBlock;
class CBlockIndex;
struct CBlockLocator;
Expand Down Expand Up @@ -51,7 +53,7 @@ void CallFunctionInValidationInterfaceQueue(std::function<void ()> func);
* });
* promise.get_future().wait();
*/
void SyncWithValidationInterfaceQueue();
void SyncWithValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main);

/**
* Implement this to subscribe to events generated in validation
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/feebumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

//! Check whether transaction has descendant in wallet or mempool, or has been
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet)
static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(cs_main, wallet->cs_wallet)
{
if (wallet->HasWalletSpend(wtx.GetHash())) {
errors.push_back("Transaction has descendants in the wallet");
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void EnsureWalletIsUnlocked(CWallet * const pwallet)
}
}

static void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry)
static void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
int confirms = wtx.GetDepthInMainChain();
entry.pushKV("confirmations", confirms);
Expand Down Expand Up @@ -1526,7 +1526,7 @@ struct tallyitem
}
};

static UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool by_label)
static UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
// Minimum confirmations
int nMinDepth = 1;
Expand Down Expand Up @@ -1793,7 +1793,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
* @param ret The UniValue into which the result is stored.
* @param filter The "is mine" filter bool.
*/
static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter)
static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
CAmount nFee;
std::string strSentAccount;
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4410,7 +4410,7 @@ bool CMerkleTx::IsImmatureCoinBase() const
return GetBlocksToMaturity() > 0;
}

bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state)
bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
// We must set fInMempool here - while it will be re-set to true by the
// entered-mempool callback, if we did not there would be a race where a
Expand Down
Loading

0 comments on commit 9e0a514

Please sign in to comment.