Skip to content

Commit

Permalink
Merge #13481: doc: Rewrite some validation docs as lock annotations
Browse files Browse the repository at this point in the history 
Summary:
fa324a8b15a4ef4138685b3427c895ec14faf3af doc: Rewrite some validation doc as lock annotations (MarcoFalke)

Pull request description:

  #13402 added some lock annotations in comments. This pull removes them and adds clang-readable locking annotations instead.

Tree-SHA512: 2d392efa8ac4978830a9df08b2009e69d6f1ac031f62be2275ae8d7c7e483331c7f8d458d865443af907a7af27a592421c6cca6b2df3f2877e0f369b9198f383

Backport of Core PR13481
bitcoin/bitcoin#13481

Depends on D4029

Test Plan:
  ../configure CXX=clang++ CC=clang
  make check

And run team city `build-werror`

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4026
  • Loading branch information
@jonspock
Nico Guiton authored and jonspock committed Oct 1, 2020
1 parent af5c634 commit 69520cf
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 30 deletions.
43 changes: 30 additions & 13 deletions src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,21 @@ class CChainState {
CBlockIndex *pindexBestParked = nullptr;

bool LoadBlockIndexDB(const Config &config);
bool LoadBlockIndex(const Config &config, CBlockTreeDB &blocktree);
bool LoadBlockIndex(const Config &config, CBlockTreeDB &blocktree)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

bool ActivateBestChain(
const Config &config, CValidationState &state,
std::shared_ptr<const CBlock> pblock = std::shared_ptr<const CBlock>());

bool AcceptBlockHeader(const Config &config, const CBlockHeader &block,
CValidationState &state, CBlockIndex **ppindex);
CValidationState &state, CBlockIndex **ppindex)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool AcceptBlock(const Config &config,
const std::shared_ptr<const CBlock> &pblock,
CValidationState &state, bool fRequested,
const CDiskBlockPos *dbp, bool *fNewBlock);
const CDiskBlockPos *dbp, bool *fNewBlock)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

// Block (dis)connection on a given view:
DisconnectResult DisconnectBlock(const CBlock &block,
Expand All @@ -167,17 +170,20 @@ class CChainState {

// Manual block validity manipulation:
bool PreciousBlock(const Config &config, CValidationState &state,
CBlockIndex *pindex);
CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main);
bool UnwindBlock(const Config &config, CValidationState &state,
CBlockIndex *pindex, bool invalidate);
bool ResetBlockFailureFlags(CBlockIndex *pindex);
CBlockIndex *pindex, bool invalidate)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool ResetBlockFailureFlags(CBlockIndex *pindex)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
template <typename F>
void UpdateFlagsForBlock(CBlockIndex *pindexBase, CBlockIndex *pindex, F f);
template <typename F, typename C>
void UpdateFlags(CBlockIndex *pindex, F f, C fchild);
template <typename F> void UpdateFlags(CBlockIndex *pindex, F f);
/** Remove parked status from a block and its descendants. */
bool UnparkBlockImpl(CBlockIndex *pindex, bool fClearChildren);
bool UnparkBlockImpl(CBlockIndex *pindex, bool fClearChildren)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

bool ReplayBlocks(const Consensus::Params &params, CCoinsView *view);
bool RewindBlockIndex(const Config &config);
Expand All @@ -201,6 +207,13 @@ class CChainState {
CBlockIndex *AddToBlockIndex(const CBlockHeader &block);
/** Create a new block index entry for a given block hash */
//CBlockIndex *InsertBlockIndex(const BlockHash &hash);
// EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/**
* Make various assertions about the state of the block index.
*
* By default this only executes fully when using the Regtest chain; see:
* fCheckBlockIndex.
*/
void CheckBlockIndex(const Consensus::Params &consensusParams);

void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state);
Expand All @@ -210,7 +223,8 @@ class CChainState {
const CDiskBlockPos &pos);

bool RollforwardBlock(const CBlockIndex *pindex, CCoinsViewCache &inputs,
const Consensus::Params &params);
const Consensus::Params &params)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
} g_chainstate;

/**
Expand Down Expand Up @@ -2504,7 +2518,8 @@ class ConnectTrace {
};

static bool FinalizeBlockInternal(const Config &config, CValidationState &state,
const CBlockIndex *pindex) {
const CBlockIndex *pindex)
EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
AssertLockHeld(cs_main);
if (pindex->nStatus.isInvalid()) {
// We try to finalize an invalid block.
Expand Down Expand Up @@ -2535,7 +2550,8 @@ static bool FinalizeBlockInternal(const Config &config, CValidationState &state,
}

static const CBlockIndex *FindBlockToFinalize(const Config &config,
CBlockIndex *pindexNew) {
CBlockIndex *pindexNew)
EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
AssertLockHeld(cs_main);

const int32_t maxreorgdepth =
Expand Down Expand Up @@ -2986,7 +3002,7 @@ bool CChainState::ActivateBestChainStep(
return true;
}

static void NotifyHeaderTip() {
static void NotifyHeaderTip() LOCKS_EXCLUDED(cs_main) {
bool fNotify = false;
bool fInitialBlockDownload = false;
static CBlockIndex *pindexHeaderOld = nullptr;
Expand Down Expand Up @@ -3354,7 +3370,7 @@ bool CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) {
return true;
}

bool ResetBlockFailureFlags(CBlockIndex *pindex) {
bool ResetBlockFailureFlags(CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
return g_chainstate.ResetBlockFailureFlags(pindex);
}

Expand Down Expand Up @@ -4684,7 +4700,8 @@ bool CChainState::LoadBlockIndexDB(const Config &config) {
return true;
}

bool static LoadBlockIndexDB(const Config &config) {
bool static LoadBlockIndexDB(const Config &config)
EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
if (!g_chainstate.LoadBlockIndex(config, *pblocktree)) {
return false;
}
Expand Down
38 changes: 22 additions & 16 deletions src/validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ class BlockValidationOptions {
* Note that we guarantee that either the proof-of-work is valid on pblock, or
* (and possibly also) BlockChecked will have been called.
*
* Call without cs_main held.
* May not be called in a validationinterface callback.
*
* @param[in] config The global config.
* @param[in] pblock The block we want to process.
Expand All @@ -346,12 +347,13 @@ class BlockValidationOptions {
*/
bool ProcessNewBlock(const Config &config,
const std::shared_ptr<const CBlock> pblock,
bool fForceProcessing, bool *fNewBlock);
bool fForceProcessing, bool *fNewBlock)
LOCKS_EXCLUDED(cs_main);

/**
* Process incoming block headers.
*
* Call without cs_main held.
* May not be called in a validationinterface callback.
*
* @param[in] config The config.
* @param[in] block The block headers themselves.
Expand All @@ -366,7 +368,8 @@ bool ProcessNewBlockHeaders(const Config &config,
const std::vector<CBlockHeader> &block,
CValidationState &state,
const CBlockIndex **ppindex = nullptr,
CBlockHeader *first_invalid = nullptr);
CBlockHeader *first_invalid = nullptr)
LOCKS_EXCLUDED(cs_main);

/**
* Open a block file (blk?????.dat).
Expand Down Expand Up @@ -394,7 +397,7 @@ bool LoadGenesisBlock(const CChainParams &chainparams);
* Load the block tree and coins database from disk, initializing state if we're
* running with -reindex.
*/
bool LoadBlockIndex(const Config &config);
bool LoadBlockIndex(const Config &config) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Update the chain tip based on database information.
Expand Down Expand Up @@ -615,11 +618,12 @@ bool ContextualCheckTransactionForCurrentBlock(const Consensus::Params &params,

/**
* Check a block is completely valid from start to finish (only works on top of
* our current best block, with cs_main held)
* our current best block)
*/
bool TestBlockValidity(const CChainParams &chainparams, CValidationState &state,
const CBlock &block, CBlockIndex *pindexPrev,
BlockValidationOptions validationOptions);
BlockValidationOptions validationOptions)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* When there are blocks in the active chain with missing data, rewind the
Expand Down Expand Up @@ -653,10 +657,10 @@ CBlockIndex *FindForkInGlobalIndex(const CChain &chain,
* PreciousBlock() will override the effects of earlier calls. The effects of
* calls to PreciousBlock() are not retained across restarts.
*
* Returns true if the provided block index successfully became the chain tip.
* May not be called in a validationinterface callback.
*/
bool PreciousBlock(const Config &config, CValidationState &state,
CBlockIndex *pindex);
CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main);

/**
* Mark a block as finalized.
Expand All @@ -668,30 +672,32 @@ bool FinalizeBlockAndInvalidate(const Config &config, CValidationState &state,

/** Mark a block as invalid. */
bool InvalidateBlock(const Config &config, CValidationState &state,
CBlockIndex *pindex);
CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Park a block. */
bool ParkBlock(const Config &config, CValidationState &state,
CBlockIndex *pindex);
CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Remove invalidity status from a block and its descendants. */
bool ResetBlockFailureFlags(CBlockIndex *pindex);
bool ResetBlockFailureFlags(CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Remove parked status from a block and its descendants. */
bool UnparkBlockAndChildren(CBlockIndex *pindex);
bool UnparkBlockAndChildren(CBlockIndex *pindex)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Remove parked status from a block. */
bool UnparkBlock(CBlockIndex *pindex);
bool UnparkBlock(CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Retrieve the topmost finalized block.
*/
const CBlockIndex *GetFinalizedBlock();
const CBlockIndex *GetFinalizedBlock() EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Checks if a block is finalized.
*/
bool IsBlockFinalized(const CBlockIndex *pindex);
bool IsBlockFinalized(const CBlockIndex *pindex)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** The currently-connected chain of blocks (protected by cs_main). */
extern CChain &chainActive;
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ std::string LabelFromValue(const UniValue &value) {
std::string generateOneBlock(const Config &config, CScript& coinbaseScript) {
{
// keep cs_main locked.
LOCK(cs_main);
//LOCK(cs_main);

unsigned int nExtraNonce = 0;
std::unique_ptr<CBlockTemplate> pblocktemplate(
Expand Down

0 comments on commit 69520cf

Please sign in to comment.