From 3dba12fd76278cb9ae914fa9172482c1db84625f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20S=C3=A9chet?= Date: Thu, 24 May 2018 14:10:04 +0200 Subject: [PATCH] Use hasData/hasUndo instead of masking with BLOCK_HAVE_DATA/BLOCK_HAVE_UNDO Summary: As per title. Depends on D1450 Test Plan: make check Reviewers: #bitcoin_abc, schancel Reviewed By: #bitcoin_abc, schancel Subscribers: schancel, teamcity Differential Revision: https://reviews.bitcoinabc.org/D1451 --- src/chain.h | 11 +++++------ src/net_processing.cpp | 14 ++++++-------- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 5 ++--- src/validation.cpp | 33 ++++++++++++++++----------------- src/wallet/wallet.cpp | 3 +-- 6 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/chain.h b/src/chain.h index 0de7e94b2..f35f5678e 100644 --- a/src/chain.h +++ b/src/chain.h @@ -248,7 +248,6 @@ struct BlockStatus { // To transition from this and the plain old intereger. // TODO: delete. uint32_t operator&(uint32_t rhs) const { return status & rhs; } - uint32_t operator|(uint32_t rhs) const { return status | rhs; } BlockStatus &operator&=(uint32_t rhs) { this->status &= rhs; @@ -378,7 +377,7 @@ class CBlockIndex { CDiskBlockPos GetBlockPos() const { CDiskBlockPos ret; - if (nStatus & BLOCK_HAVE_DATA) { + if (nStatus.hasData()) { ret.nFile = nFile; ret.nPos = nDataPos; } @@ -387,7 +386,7 @@ class CBlockIndex { CDiskBlockPos GetUndoPos() const { CDiskBlockPos ret; - if (nStatus & BLOCK_HAVE_UNDO) { + if (nStatus.hasUndo()) { ret.nFile = nFile; ret.nPos = nUndoPos; } @@ -518,13 +517,13 @@ class CDiskBlockIndex : public CBlockIndex { READWRITE(VARINT(nHeight)); READWRITE(nStatus); READWRITE(VARINT(nTx)); - if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) { + if (nStatus.hasData() || nStatus.hasUndo()) { READWRITE(VARINT(nFile)); } - if (nStatus & BLOCK_HAVE_DATA) { + if (nStatus.hasData()) { READWRITE(VARINT(nDataPos)); } - if (nStatus & BLOCK_HAVE_UNDO) { + if (nStatus.hasUndo()) { READWRITE(VARINT(nUndoPos)); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d7b549d40..d3c418a96 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -623,8 +623,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, // We consider the chain that this peer is on invalid. return; } - if (pindex->nStatus & BLOCK_HAVE_DATA || - chainActive.Contains(pindex)) { + if (pindex->nStatus.hasData() || chainActive.Contains(pindex)) { if (pindex->nChainTx) { state->pindexLastCommonBlock = pindex; } @@ -1213,7 +1212,7 @@ static void ProcessGetData(const Config &config, CNode *pfrom, } // Pruned nodes may have deleted the block, so check whether // it's available before trying to send. - if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) { + if (send && (mi->second->nStatus.hasData())) { // Send block from disk CBlock block; if (!ReadBlockFromDisk(block, (*mi).second, config)) { @@ -1891,7 +1890,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom, MIN_BLOCKS_TO_KEEP - 3600 / chainparams.GetConsensus().nPowTargetSpacing; if (fPruneMode && - (!(pindex->nStatus & BLOCK_HAVE_DATA) || + (!pindex->nStatus.hasData() || pindex->nHeight <= chainActive.Tip()->nHeight - nPrunedBlocksLikelyToHave)) { LogPrint( @@ -1932,8 +1931,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom, LOCK(cs_main); BlockMap::iterator it = mapBlockIndex.find(req.blockhash); - if (it == mapBlockIndex.end() || - !(it->second->nStatus & BLOCK_HAVE_DATA)) { + if (it == mapBlockIndex.end() || !it->second->nStatus.hasData()) { LogPrintf("Peer %d sent us a getblocktxn for a block we don't have", pfrom->id); return true; @@ -2307,7 +2305,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom, mapBlocksInFlight.find(pindex->GetBlockHash()); bool fAlreadyInFlight = blockInFlightIt != mapBlocksInFlight.end(); - if (pindex->nStatus & BLOCK_HAVE_DATA) { + if (pindex->nStatus.hasData()) { // Nothing to do here return true; } @@ -2702,7 +2700,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom, // up to a limit. while (pindexWalk && !chainActive.Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { - if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && + if (!pindexWalk->nStatus.hasData() && !mapBlocksInFlight.count(pindexWalk->GetBlockHash())) { // We don't have this block, and it's not yet in flight. vToFetch.push_back(pindexWalk); diff --git a/src/rest.cpp b/src/rest.cpp index 97e1675d3..1c320cd75 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -235,7 +235,7 @@ static bool rest_block(const Config &config, HTTPRequest *req, } pblockindex = mapBlockIndex[hash]; - if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && + if (fHavePruned && !pblockindex->nStatus.hasData() && pblockindex->nTx > 0) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e09b9fffd..710b46e9e 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -826,7 +826,7 @@ UniValue getblock(const Config &config, const JSONRPCRequest &request) { CBlock block; CBlockIndex *pblockindex = mapBlockIndex[hash]; - if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && + if (fHavePruned && !pblockindex->nStatus.hasData() && pblockindex->nTx > 0) { throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); } @@ -1328,8 +1328,7 @@ UniValue getblockchaininfo(const Config &config, if (fPruneMode) { CBlockIndex *block = chainActive.Tip(); - while (block && block->pprev && - (block->pprev->nStatus & BLOCK_HAVE_DATA)) { + while (block && block->pprev && block->pprev->nStatus.hasData()) { block = block->pprev; } diff --git a/src/validation.cpp b/src/validation.cpp index f0643aa6a..cf2b93d3d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2826,7 +2826,7 @@ static CBlockIndex *FindMostWorkChain() { // for the most work chain if we come across them; we can't switch // to a chain unless we have all the non-active-chain parent blocks. bool fFailedChain = pindexTest->nStatus & BLOCK_FAILED_MASK; - bool fMissingData = !(pindexTest->nStatus & BLOCK_HAVE_DATA); + bool fMissingData = !pindexTest->nStatus.hasData(); if (fFailedChain || fMissingData) { // Candidate chain is not usable (either invalid or missing // data) @@ -3847,7 +3847,7 @@ static bool AcceptBlock(const Config &config, // Try to process all requested blocks that we don't have, but only // process an unrequested block if it's new and has enough work to // advance our tip, and isn't too many blocks ahead. - bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA; + bool fAlreadyHave = pindex->nStatus.hasData(); bool fHasMoreWork = (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true); @@ -4344,7 +4344,7 @@ static bool LoadBlockIndexDB(const CChainParams &chainparams) { std::set setBlkDataFiles; for (const std::pair &item : mapBlockIndex) { CBlockIndex *pindex = item.second; - if (pindex->nStatus & BLOCK_HAVE_DATA) { + if (pindex->nStatus.hasData()) { setBlkDataFiles.insert(pindex->nFile); } } @@ -4457,7 +4457,7 @@ bool CVerifyDB::VerifyDB(const Config &config, CCoinsView *coinsview, break; } - if (fPruneMode && !(pindex->nStatus & BLOCK_HAVE_DATA)) { + if (fPruneMode && !pindex->nStatus.hasData()) { // If pruning, only go back as far as we have data. LogPrintf("VerifyDB(): block verification stopping at height %d " "(pruning, no data)\n", @@ -4692,7 +4692,7 @@ bool RewindBlockIndex(const Config &config) { CValidationState state; CBlockIndex *pindex = chainActive.Tip(); while (chainActive.Height() >= nHeight) { - if (fPruneMode && !(chainActive.Tip()->nStatus & BLOCK_HAVE_DATA)) { + if (fPruneMode && !chainActive.Tip()->nStatus.hasData()) { // If pruning, don't try rewinding past the HAVE_DATA point; since // older blocks can't be served anyway, there's no need to walk // further, and trying to DisconnectTip() will fail (and require a @@ -4892,7 +4892,7 @@ bool LoadExternalBlockFile(const Config &config, FILE *fileIn, // process in case the block isn't known yet if (mapBlockIndex.count(hash) == 0 || - (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { + !mapBlockIndex[hash]->nStatus.hasData()) { LOCK(cs_main); CValidationState state; if (AcceptBlock(config, pblock, state, nullptr, true, dbp, @@ -5013,7 +5013,7 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { int nHeight = 0; // Oldest ancestor of pindex which is invalid. CBlockIndex *pindexFirstInvalid = nullptr; - // Oldest ancestor of pindex which does not have BLOCK_HAVE_DATA. + // Oldest ancestor of pindex which does not have data available. CBlockIndex *pindexFirstMissing = nullptr; // Oldest ancestor of pindex for which nTx == 0. CBlockIndex *pindexFirstNeverProcessed = nullptr; @@ -5035,8 +5035,7 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { pindex->nStatus & BLOCK_FAILED_VALID) { pindexFirstInvalid = pindex; } - if (pindexFirstMissing == nullptr && - !(pindex->nStatus & BLOCK_HAVE_DATA)) { + if (pindexFirstMissing == nullptr && !pindex->nStatus.hasData()) { pindexFirstMissing = pindex; } if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) { @@ -5079,15 +5078,15 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { if (!fHavePruned) { // If we've never pruned, then HAVE_DATA should be equivalent to nTx // > 0 - assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0)); + assert(!pindex->nStatus.hasData() == (pindex->nTx == 0)); assert(pindexFirstMissing == pindexFirstNeverProcessed); - } else { + } else if (pindex->nStatus.hasData()) { // If we have pruned, then we can only say that HAVE_DATA implies // nTx > 0 - if (pindex->nStatus & BLOCK_HAVE_DATA) assert(pindex->nTx > 0); + assert(pindex->nTx > 0); } - if (pindex->nStatus & BLOCK_HAVE_UNDO) { - assert(pindex->nStatus & BLOCK_HAVE_DATA); + if (pindex->nStatus.hasUndo()) { + assert(pindex->nStatus.hasData()); } // This is pruning-independent. assert(((pindex->nStatus & BLOCK_VALID_MASK) >= @@ -5164,7 +5163,7 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { } rangeUnlinked.first++; } - if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && + if (pindex->pprev && pindex->nStatus.hasData() && pindexFirstNeverProcessed != nullptr && pindexFirstInvalid == nullptr) { // If this block has block data available, some parent was never @@ -5172,7 +5171,7 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { // mapBlocksUnlinked. assert(foundInUnlinked); } - if (!(pindex->nStatus & BLOCK_HAVE_DATA)) { + if (!pindex->nStatus.hasData()) { // Can't be in mapBlocksUnlinked if we don't HAVE_DATA assert(!foundInUnlinked); } @@ -5181,7 +5180,7 @@ static void CheckBlockIndex(const Consensus::Params &consensusParams) { // mapBlocksUnlinked. assert(!foundInUnlinked); } - if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && + if (pindex->pprev && pindex->nStatus.hasData() && pindexFirstNeverProcessed == nullptr && pindexFirstMissing != nullptr) { // We HAVE_DATA for this block, have received data for all parents diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3fb1a4778..aae25f74d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4235,8 +4235,7 @@ CWallet *CWallet::CreateWalletFromFile(const CChainParams &chainParams, // re-enable. if (fPruneMode) { CBlockIndex *block = chainActive.Tip(); - while (block && block->pprev && - (block->pprev->nStatus & BLOCK_HAVE_DATA) && + while (block && block->pprev && block->pprev->nStatus.hasData() && block->pprev->nTx > 0 && pindexRescan != block) { block = block->pprev; }