From 36a4ba0aaaa9b35185d7178994e36bc02cca9887 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 5 Mar 2021 02:01:36 +1000 Subject: [PATCH 01/12] versionbits: correct doxygen comments --- src/versionbits.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/versionbits.h b/src/versionbits.h index 634a848ef58..dce39412884 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -80,11 +80,11 @@ struct VersionBitsCache void Clear(); }; -/** Get the BIP9 state for a given deployment at the current tip. */ +/** Get the BIP9 state for a given deployment for the block after pindexPrev. */ ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); -/** Get the numerical statistics for the BIP9 state for a given deployment at the current tip. */ +/** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */ BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); -/** Get the block height at which the BIP9 deployment switched into the state for the block building on the current tip. */ +/** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos); From eccd736f3dc231ac0306ca763c3b72cf8247230a Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 29 Dec 2020 22:43:18 +1000 Subject: [PATCH 02/12] versionbits: Use dedicated lock instead of cs_main --- src/rpc/blockchain.cpp | 2 +- src/validation.cpp | 8 +++----- src/versionbits.cpp | 3 +++ src/versionbits.h | 5 ++++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 02f8cec54c2..22600c53403 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1361,7 +1361,7 @@ static void BuriedForkDescPushBack(UniValue& softforks, const std::string &name, softforks.pushKV(name, rv); } -static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const std::string &name, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const std::string &name, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) { // For BIP9 deployments. // Deployments that are never active are hidden. diff --git a/src/validation.cpp b/src/validation.cpp index df0ec3bd4ff..1b0f881d148 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1606,11 +1606,10 @@ void StopScriptCheckWorkerThreads() scriptcheckqueue.StopWorkerThreads(); } -VersionBitsCache versionbitscache GUARDED_BY(cs_main); +VersionBitsCache versionbitscache; int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) { - LOCK(cs_main); int32_t nVersion = VERSIONBITS_TOP_BITS; for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { @@ -1659,9 +1658,8 @@ static bool IsScriptWitnessEnabled(const Consensus::Params& params) return params.SegwitHeight != std::numeric_limits::max(); } -static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - AssertLockHeld(cs_main); - +static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) +{ unsigned int flags = SCRIPT_VERIFY_NONE; // BIP16 didn't become active until Apr 1 2012 (on mainnet, and diff --git a/src/versionbits.cpp b/src/versionbits.cpp index df2ec4e056e..07ecc93c939 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -192,6 +192,7 @@ public: ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache) { + LOCK(cache.mutex); return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, cache.caches[pos]); } @@ -202,6 +203,7 @@ BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus:: int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache) { + LOCK(cache.mutex); return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, cache.caches[pos]); } @@ -212,6 +214,7 @@ uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentP void VersionBitsCache::Clear() { + LOCK(mutex); for (unsigned int d = 0; d < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; d++) { caches[d].clear(); } diff --git a/src/versionbits.h b/src/versionbits.h index dce39412884..24279a0de4f 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -6,6 +6,8 @@ #define BITCOIN_VERSIONBITS_H #include +#include + #include /** What block version to use for new blocks (pre versionbits) */ @@ -75,7 +77,8 @@ public: * keyed by the bit position used to signal support. */ struct VersionBitsCache { - ThresholdConditionCache caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS]; + Mutex mutex; + ThresholdConditionCache caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(mutex); void Clear(); }; From 2b0d291da8f479739ff394dd92801da8c40b9f8e Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 16 Jun 2020 18:58:56 +1000 Subject: [PATCH 03/12] [refactor] Add deploymentstatus.h Provides DeploymentEnabled, DeploymentActiveAt, and DeploymentActiveAfter helpers for checking the status of buried deployments. Can be overloaded so the same syntax works for non-buried deployments, allowing future soft forks to be changed from signalled to buried deployments without having to touch the implementation code. Replaces IsWitnessEnabled and IsScriptWitnessEnabled. --- src/Makefile.am | 1 + src/consensus/params.h | 29 ++++++++++++++++++ src/deploymentstatus.h | 33 +++++++++++++++++++++ src/init.cpp | 3 +- src/miner.cpp | 5 ++-- src/net_processing.cpp | 9 +++--- src/rpc/mining.cpp | 3 +- src/validation.cpp | 67 ++++++++++++++++-------------------------- src/validation.h | 4 --- 9 files changed, 101 insertions(+), 53 deletions(-) create mode 100644 src/deploymentstatus.h diff --git a/src/Makefile.am b/src/Makefile.am index e2ed70556d9..a3248f3eab5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -145,6 +145,7 @@ BITCOIN_CORE_H = \ core_memusage.h \ cuckoocache.h \ dbwrapper.h \ + deploymentstatus.h \ external_signer.h \ flatfile.h \ fs.h \ diff --git a/src/consensus/params.h b/src/consensus/params.h index 28c95e08841..9b4139d76cc 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -11,6 +11,17 @@ namespace Consensus { +enum BuriedDeployment : int16_t +{ + // buried deployments get negative values to avoid overlap with DeploymentPos + DEPLOYMENT_HEIGHTINCB = std::numeric_limits::min(), + DEPLOYMENT_CLTV, + DEPLOYMENT_DERSIG, + DEPLOYMENT_CSV, + DEPLOYMENT_SEGWIT, +}; +constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_SEGWIT; } + enum DeploymentPos { DEPLOYMENT_TESTDUMMY, @@ -100,7 +111,25 @@ struct Params { */ bool signet_blocks{false}; std::vector signet_challenge; + + int DeploymentHeight(BuriedDeployment dep) const + { + switch (dep) { + case DEPLOYMENT_HEIGHTINCB: + return BIP34Height; + case DEPLOYMENT_CLTV: + return BIP65Height; + case DEPLOYMENT_DERSIG: + return BIP66Height; + case DEPLOYMENT_CSV: + return CSVHeight; + case DEPLOYMENT_SEGWIT: + return SegwitHeight; + } // no default case, so the compiler can warn about missing cases + return std::numeric_limits::max(); + } }; + } // namespace Consensus #endif // BITCOIN_CONSENSUS_PARAMS_H diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h new file mode 100644 index 00000000000..32190e369d5 --- /dev/null +++ b/src/deploymentstatus.h @@ -0,0 +1,33 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_DEPLOYMENTSTATUS_H +#define BITCOIN_DEPLOYMENTSTATUS_H + +#include + +#include + +/** Determine if a deployment is active for the next block */ +inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep) +{ + assert(Consensus::ValidDeployment(dep)); + return (pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1) >= params.DeploymentHeight(dep); +} + +/** Determine if a deployment is active for this block */ +inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::BuriedDeployment dep) +{ + assert(Consensus::ValidDeployment(dep)); + return index.nHeight >= params.DeploymentHeight(dep); +} + +/** Determine if a deployment is enabled (can ever be active) */ +inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::BuriedDeployment dep) +{ + assert(Consensus::ValidDeployment(dep)); + return params.DeploymentHeight(dep) != std::numeric_limits::max(); +} + +#endif // BITCOIN_DEPLOYMENTSTATUS_H diff --git a/src/init.cpp b/src/init.cpp index a88adbaa21d..ae96f510bcb 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -1587,7 +1588,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - if (chainparams.GetConsensus().SegwitHeight != std::numeric_limits::max()) { + if (DeploymentEnabled(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { // Advertise witness capabilities. // The option to not set NODE_WITNESS is only used in the tests and should be removed. nLocalServices = ServiceFlags(nLocalServices | NODE_WITNESS); diff --git a/src/miner.cpp b/src/miner.cpp index 0cf303eb3c4..c1a992b9e9c 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -137,12 +138,12 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc // This is only needed in case the witness softfork activation is reverted // (which would require a very deep reorganization). // Note that the mempool would accept transactions with witness data before - // IsWitnessEnabled, but we would only ever mine blocks after IsWitnessEnabled + // the deployment is active, but we would only ever mine blocks after activation // unless there is a massive block reorganization with the witness softfork // not activated. // TODO: replace this with a call to main to assess validity of a mempool // transaction (which in most cases can be a no-op). - fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()); + fIncludeWitness = DeploymentActiveAfter(pindexPrev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT); int nPackagesSelected = 0; int nDescendantsUpdated = 0; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ab9c41ca2bd..315d2ac5cd8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -997,7 +998,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // We consider the chain that this peer is on invalid. return; } - if (!State(nodeid)->fHaveWitness && IsWitnessEnabled(pindex->pprev, consensusParams)) { + if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { // We wouldn't download this block or its descendants from this peer. return; } @@ -1467,7 +1468,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha return; nHighestFastAnnounce = pindex->nHeight; - bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus()); + bool fWitnessEnabled = DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT); uint256 hashBlock(pblock->GetHash()); { @@ -2082,7 +2083,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && - (!IsWitnessEnabled(pindexWalk->pprev, m_chainparams.GetConsensus()) || State(pfrom.GetId())->fHaveWitness)) { + (!DeploymentActiveAt(*pindexWalk, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) { // We don't have this block, and it's not yet in flight. vToFetch.push_back(pindexWalk); } @@ -3397,7 +3398,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) { + if (DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT) && !nodestate->fSupportsDesiredCmpctVersion) { // Don't bother trying to process compact blocks from v1 peers // after segwit activates. return; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 327f9611960..f054c8b1450 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -774,7 +775,7 @@ static RPCHelpMan getblocktemplate() pblock->nNonce = 0; // NOTE: If at some point we support pre-segwit miners post-segwit-activation, this needs to take segwit support into consideration - const bool fPreSegWit = (pindexPrev->nHeight + 1 < consensusParams.SegwitHeight); + const bool fPreSegWit = !DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT); UniValue aCaps(UniValue::VARR); aCaps.push_back("proposal"); diff --git a/src/validation.cpp b/src/validation.cpp index 1b0f881d148..b69faa54eb8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1649,15 +1650,6 @@ public: static ThresholdConditionCache warningcache[VERSIONBITS_NUM_BITS] GUARDED_BY(cs_main); -// 0.13.0 was shipped with a segwit deployment defined for testnet, but not for -// mainnet. We no longer need to support disabling the segwit deployment -// except for testing purposes, due to limitations of the functional test -// environment. See test/functional/p2p-segwit.py. -static bool IsScriptWitnessEnabled(const Consensus::Params& params) -{ - return params.SegwitHeight != std::numeric_limits::max(); -} - static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) { unsigned int flags = SCRIPT_VERIFY_NONE; @@ -1676,22 +1668,22 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consens // Enforce WITNESS rules whenever P2SH is in effect (and the segwit // deployment is defined). - if (flags & SCRIPT_VERIFY_P2SH && IsScriptWitnessEnabled(consensusparams)) { + if (flags & SCRIPT_VERIFY_P2SH && DeploymentEnabled(consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { flags |= SCRIPT_VERIFY_WITNESS; } - // Start enforcing the DERSIG (BIP66) rule - if (pindex->nHeight >= consensusparams.BIP66Height) { + // Enforce the DERSIG (BIP66) rule + if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_DERSIG)) { flags |= SCRIPT_VERIFY_DERSIG; } - // Start enforcing CHECKLOCKTIMEVERIFY (BIP65) rule - if (pindex->nHeight >= consensusparams.BIP65Height) { + // Enforce CHECKLOCKTIMEVERIFY (BIP65) + if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_CLTV)) { flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; } - // Start enforcing BIP112 (CHECKSEQUENCEVERIFY) - if (pindex->nHeight >= consensusparams.CSVHeight) { + // Enforce CHECKSEQUENCEVERIFY (BIP112) + if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_CSV)) { flags |= SCRIPT_VERIFY_CHECKSEQUENCEVERIFY; } @@ -1700,8 +1692,8 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consens flags |= SCRIPT_VERIFY_TAPROOT; } - // Start enforcing BIP147 NULLDUMMY (activated simultaneously with segwit) - if (IsWitnessEnabled(pindex->pprev, consensusparams)) { + // Enforce BIP147 NULLDUMMY (activated simultaneously with segwit) + if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { flags |= SCRIPT_VERIFY_NULLDUMMY; } @@ -1891,9 +1883,9 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, } } - // Start enforcing BIP68 (sequence locks) + // Enforce BIP68 (sequence locks) int nLockTimeFlags = 0; - if (pindex->nHeight >= m_params.GetConsensus().CSVHeight) { + if (DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_CSV)) { nLockTimeFlags |= LOCKTIME_VERIFY_SEQUENCE; } @@ -2986,7 +2978,7 @@ void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pi pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; pindexNew->nStatus |= BLOCK_HAVE_DATA; - if (IsWitnessEnabled(pindexNew->pprev, m_params.GetConsensus())) { + if (DeploymentActiveAt(*pindexNew, m_params.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { pindexNew->nStatus |= BLOCK_OPT_WITNESS; } pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); @@ -3107,17 +3099,11 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu return true; } -bool IsWitnessEnabled(const CBlockIndex* pindexPrev, const Consensus::Params& params) -{ - int height = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; - return (height >= params.SegwitHeight); -} - void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams) { int commitpos = GetWitnessCommitmentIndex(block); static const std::vector nonce(32, 0x00); - if (commitpos != NO_WITNESS_COMMITMENT && IsWitnessEnabled(pindexPrev, consensusParams) && !block.vtx[0]->HasWitness()) { + if (commitpos != NO_WITNESS_COMMITMENT && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT) && !block.vtx[0]->HasWitness()) { CMutableTransaction tx(*block.vtx[0]); tx.vin[0].scriptWitness.stack.resize(1); tx.vin[0].scriptWitness.stack[0] = nonce; @@ -3130,7 +3116,7 @@ std::vector GenerateCoinbaseCommitment(CBlock& block, const CBloc std::vector commitment; int commitpos = GetWitnessCommitmentIndex(block); std::vector ret(32, 0x00); - if (consensusParams.SegwitHeight != std::numeric_limits::max()) { + if (DeploymentEnabled(consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { if (commitpos == NO_WITNESS_COMMITMENT) { uint256 witnessroot = BlockWitnessMerkleRoot(block, nullptr); CHash256().Write(witnessroot).Write(ret).Finalize(witnessroot); @@ -3208,13 +3194,13 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", "block timestamp too far in the future"); - // Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded: - // check for version 2, 3 and 4 upgrades - if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) || - (block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) || - (block.nVersion < 4 && nHeight >= consensusParams.BIP65Height)) + // Reject blocks with outdated version + if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB)) || + (block.nVersion < 3 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_DERSIG)) || + (block.nVersion < 4 && DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_CLTV))) { return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); + } return true; } @@ -3229,9 +3215,9 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat { const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; - // Start enforcing BIP113 (Median Time Past). + // Enforce BIP113 (Median Time Past). int nLockTimeFlags = 0; - if (nHeight >= consensusParams.CSVHeight) { + if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_CSV)) { assert(pindexPrev != nullptr); nLockTimeFlags |= LOCKTIME_MEDIAN_TIME_PAST; } @@ -3248,7 +3234,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat } // Enforce rule that the coinbase starts with serialized block height - if (nHeight >= consensusParams.BIP34Height) + if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB)) { CScript expect = CScript() << nHeight; if (block.vtx[0]->vin[0].scriptSig.size() < expect.size() || @@ -3266,7 +3252,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat // {0xaa, 0x21, 0xa9, 0xed}, and the following 32 bytes are SHA256^2(witness root, witness reserved value). In case there are // multiple, the last one is used. bool fHaveWitness = false; - if (nHeight >= consensusParams.SegwitHeight) { + if (DeploymentActiveAfter(pindexPrev, consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { int commitpos = GetWitnessCommitmentIndex(block); if (commitpos != NO_WITNESS_COMMITMENT) { bool malleated = false; @@ -4096,9 +4082,8 @@ bool CChainState::NeedsRedownload() const // At and above m_params.SegwitHeight, segwit consensus rules must be validated CBlockIndex* block{m_chain.Tip()}; - const int segwit_height{m_params.GetConsensus().SegwitHeight}; - while (block != nullptr && block->nHeight >= segwit_height) { + while (block != nullptr && DeploymentActiveAt(*block, m_params.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { if (!(block->nStatus & BLOCK_OPT_WITNESS)) { // block is insufficiently validated for a segwit client return true; @@ -5000,7 +4985,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake BLOCK_OPT_WITNESS so that CChainState::NeedsRedownload() // won't ask to rewind the entire assumed-valid chain on startup. - if (index->pprev && ::IsWitnessEnabled(index->pprev, ::Params().GetConsensus())) { + if (index->pprev && DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { index->nStatus |= BLOCK_OPT_WITNESS; } } diff --git a/src/validation.h b/src/validation.h index fc702b71830..50a8d7e5757 100644 --- a/src/validation.h +++ b/src/validation.h @@ -345,10 +345,6 @@ bool TestBlockValidity(BlockValidationState& state, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Check whether witness commitments are required for a block, and whether to enforce NULLDUMMY (BIP 147) rules. - * Note that transaction witness validation rules are always enforced when P2SH is enforced. */ -bool IsWitnessEnabled(const CBlockIndex* pindexPrev, const Consensus::Params& params); - /** Update uncommitted block structures (currently: only the witness reserved value). This is safe for submitted blocks. */ void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams); From de55304f6e7a8b607e6b3fc7436de50910747b0c Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 3 Jul 2020 04:01:23 +1000 Subject: [PATCH 04/12] [refactor] Add versionbits deployments to deploymentstatus.h Adds support for versionbits deployments to DeploymentEnabled, DeploymentActiveAfter and DeploymentActiveAt. Also moves versionbitscache from validation to deploymentstatus. --- src/Makefile.am | 1 + src/consensus/params.h | 3 ++- src/deploymentstatus.cpp | 17 +++++++++++++++++ src/deploymentstatus.h | 22 ++++++++++++++++++++++ src/node/interfaces.cpp | 3 ++- src/rpc/blockchain.cpp | 1 + src/test/versionbits_tests.cpp | 1 + src/validation.cpp | 11 ++++------- src/validation.h | 3 --- 9 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 src/deploymentstatus.cpp diff --git a/src/Makefile.am b/src/Makefile.am index a3248f3eab5..a8e1b7bc1ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -329,6 +329,7 @@ libbitcoin_server_a_SOURCES = \ chain.cpp \ consensus/tx_verify.cpp \ dbwrapper.cpp \ + deploymentstatus.cpp \ flatfile.cpp \ httprpc.cpp \ httpserver.cpp \ diff --git a/src/consensus/params.h b/src/consensus/params.h index 9b4139d76cc..7d5fe1a7340 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -22,13 +22,14 @@ enum BuriedDeployment : int16_t }; constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_SEGWIT; } -enum DeploymentPos +enum DeploymentPos : uint16_t { DEPLOYMENT_TESTDUMMY, DEPLOYMENT_TAPROOT, // Deployment of Schnorr/Taproot (BIPs 340-342) // NOTE: Also add new deployments to VersionBitsDeploymentInfo in versionbits.cpp MAX_VERSION_BITS_DEPLOYMENTS }; +constexpr bool ValidDeployment(DeploymentPos dep) { return DEPLOYMENT_TESTDUMMY <= dep && dep <= DEPLOYMENT_TAPROOT; } /** * Struct for each individual consensus rule change using BIP9. diff --git a/src/deploymentstatus.cpp b/src/deploymentstatus.cpp new file mode 100644 index 00000000000..4e13fd7e0af --- /dev/null +++ b/src/deploymentstatus.cpp @@ -0,0 +1,17 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +VersionBitsCache versionbitscache; + +/* Basic sanity checking for BuriedDeployment/DeploymentPos enums and + * ValidDeployment check */ + +static_assert(ValidDeployment(Consensus::DEPLOYMENT_TESTDUMMY), "sanity check of DeploymentPos failed (TESTDUMMY not valid)"); +static_assert(!ValidDeployment(Consensus::MAX_VERSION_BITS_DEPLOYMENTS), "sanity check of DeploymentPos failed (MAX value considered valid)"); +static_assert(!ValidDeployment(static_cast(Consensus::DEPLOYMENT_TESTDUMMY)), "sanity check of BuriedDeployment failed (overlaps with DeploymentPos)"); diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h index 32190e369d5..327a5f78927 100644 --- a/src/deploymentstatus.h +++ b/src/deploymentstatus.h @@ -6,9 +6,13 @@ #define BITCOIN_DEPLOYMENTSTATUS_H #include +#include #include +/** Global cache for versionbits deployment status */ +extern VersionBitsCache versionbitscache; + /** Determine if a deployment is active for the next block */ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep) { @@ -16,6 +20,12 @@ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus return (pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1) >= params.DeploymentHeight(dep); } +inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep) +{ + assert(Consensus::ValidDeployment(dep)); + return ThresholdState::ACTIVE == VersionBitsState(pindexPrev, params, dep, versionbitscache); +} + /** Determine if a deployment is active for this block */ inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::BuriedDeployment dep) { @@ -23,6 +33,12 @@ inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params return index.nHeight >= params.DeploymentHeight(dep); } +inline bool DeploymentActiveAt(const CBlockIndex& index, const Consensus::Params& params, Consensus::DeploymentPos dep) +{ + assert(Consensus::ValidDeployment(dep)); + return DeploymentActiveAfter(index.pprev, params, dep); +} + /** Determine if a deployment is enabled (can ever be active) */ inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::BuriedDeployment dep) { @@ -30,4 +46,10 @@ inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::Buried return params.DeploymentHeight(dep) != std::numeric_limits::max(); } +inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::DeploymentPos dep) +{ + assert(Consensus::ValidDeployment(dep)); + return params.vDeployments[dep].nTimeout != 0; +} + #endif // BITCOIN_DEPLOYMENTSTATUS_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 807b0143a67..183b5a5d912 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -692,7 +693,7 @@ public: { LOCK(::cs_main); const CBlockIndex* tip = Assert(m_node.chainman)->ActiveChain().Tip(); - return VersionBitsState(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT, versionbitscache) == ThresholdState::ACTIVE; + return DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT); } NodeContext& m_node; }; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 22600c53403..7ba6e13142f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 304cd8feb07..a89447732e2 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include diff --git a/src/validation.cpp b/src/validation.cpp index b69faa54eb8..782a6cd56db 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -684,9 +684,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } // Check for non-standard pay-to-script-hash in inputs - const auto& params = args.m_chainparams.GetConsensus(); - auto taproot_state = VersionBitsState(m_active_chainstate.m_chain.Tip(), params, Consensus::DEPLOYMENT_TAPROOT, versionbitscache); - if (fRequireStandard && !AreInputsStandard(tx, m_view, taproot_state == ThresholdState::ACTIVE)) { + const bool taproot_active = DeploymentActiveAfter(m_active_chainstate.m_chain.Tip(), args.m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_TAPROOT); + if (fRequireStandard && !AreInputsStandard(tx, m_view, taproot_active)) { return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); } @@ -1607,8 +1606,6 @@ void StopScriptCheckWorkerThreads() scriptcheckqueue.StopWorkerThreads(); } -VersionBitsCache versionbitscache; - int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) { int32_t nVersion = VERSIONBITS_TOP_BITS; @@ -1687,8 +1684,8 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consens flags |= SCRIPT_VERIFY_CHECKSEQUENCEVERIFY; } - // Start enforcing Taproot using versionbits logic. - if (VersionBitsState(pindex->pprev, consensusparams, Consensus::DEPLOYMENT_TAPROOT, versionbitscache) == ThresholdState::ACTIVE) { + // Enforce Taproot (BIP340-BIP342) + if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_TAPROOT)) { flags |= SCRIPT_VERIFY_TAPROOT; } diff --git a/src/validation.h b/src/validation.h index 50a8d7e5757..0f00556053a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -24,7 +24,6 @@ #include #include // For CTxMemPool::cs #include -#include #include #include #include @@ -1020,8 +1019,6 @@ public: /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr pblocktree; -extern VersionBitsCache versionbitscache; - /** * Determine what nVersion a new block should use. */ From c64b2c6a0f79369624ae96b2e3d579d50aae4de6 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 11 Jun 2021 06:59:53 +1000 Subject: [PATCH 05/12] scripted-diff: rename versionbitscache -BEGIN VERIFY SCRIPT- sed -i -e 's/versionbitscache/g_versionbitscache/g' $(git grep -l versionbitscache) -END VERIFY SCRIPT- --- src/deploymentstatus.cpp | 2 +- src/deploymentstatus.h | 4 ++-- src/rpc/blockchain.cpp | 4 ++-- src/rpc/mining.cpp | 2 +- src/test/versionbits_tests.cpp | 4 ++-- src/validation.cpp | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/deploymentstatus.cpp b/src/deploymentstatus.cpp index 4e13fd7e0af..90078004218 100644 --- a/src/deploymentstatus.cpp +++ b/src/deploymentstatus.cpp @@ -7,7 +7,7 @@ #include #include -VersionBitsCache versionbitscache; +VersionBitsCache g_versionbitscache; /* Basic sanity checking for BuriedDeployment/DeploymentPos enums and * ValidDeployment check */ diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h index 327a5f78927..3c4f1895a1b 100644 --- a/src/deploymentstatus.h +++ b/src/deploymentstatus.h @@ -11,7 +11,7 @@ #include /** Global cache for versionbits deployment status */ -extern VersionBitsCache versionbitscache; +extern VersionBitsCache g_versionbitscache; /** Determine if a deployment is active for the next block */ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::BuriedDeployment dep) @@ -23,7 +23,7 @@ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep) { assert(Consensus::ValidDeployment(dep)); - return ThresholdState::ACTIVE == VersionBitsState(pindexPrev, params, dep, versionbitscache); + return ThresholdState::ACTIVE == VersionBitsState(pindexPrev, params, dep, g_versionbitscache); } /** Determine if a deployment is active for this block */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7ba6e13142f..7ceff17d0b4 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1369,7 +1369,7 @@ static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniVal if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; UniValue bip9(UniValue::VOBJ); - const ThresholdState thresholdState = VersionBitsState(active_chain_tip, consensusParams, id, versionbitscache); + const ThresholdState thresholdState = VersionBitsState(active_chain_tip, consensusParams, id, g_versionbitscache); switch (thresholdState) { case ThresholdState::DEFINED: bip9.pushKV("status", "defined"); break; case ThresholdState::STARTED: bip9.pushKV("status", "started"); break; @@ -1383,7 +1383,7 @@ static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniVal } bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime); bip9.pushKV("timeout", consensusParams.vDeployments[id].nTimeout); - int64_t since_height = VersionBitsStateSinceHeight(active_chain_tip, consensusParams, id, versionbitscache); + int64_t since_height = VersionBitsStateSinceHeight(active_chain_tip, consensusParams, id, g_versionbitscache); bip9.pushKV("since", since_height); if (ThresholdState::STARTED == thresholdState) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f054c8b1450..d6ca0959f23 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -841,7 +841,7 @@ static RPCHelpMan getblocktemplate() UniValue vbavailable(UniValue::VOBJ); for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { Consensus::DeploymentPos pos = Consensus::DeploymentPos(j); - ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, versionbitscache); + ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, g_versionbitscache); switch (state) { case ThresholdState::DEFINED: case ThresholdState::FAILED: diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index a89447732e2..49c61f5f2ff 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -259,8 +259,8 @@ BOOST_AUTO_TEST_CASE(versionbits_test) /** Check that ComputeBlockVersion will set the appropriate bit correctly */ static void check_computeblockversion(const Consensus::Params& params, Consensus::DeploymentPos dep) { - // This implicitly uses versionbitscache, so clear it every time - versionbitscache.Clear(); + // This implicitly uses g_versionbitscache, so clear it every time + g_versionbitscache.Clear(); int64_t bit = params.vDeployments[dep].bit; int64_t nStartTime = params.vDeployments[dep].nStartTime; diff --git a/src/validation.cpp b/src/validation.cpp index 782a6cd56db..d2baa92dde3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1611,7 +1611,7 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para int32_t nVersion = VERSIONBITS_TOP_BITS; for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { - ThresholdState state = VersionBitsState(pindexPrev, params, static_cast(i), versionbitscache); + ThresholdState state = VersionBitsState(pindexPrev, params, static_cast(i), g_versionbitscache); if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) { nVersion |= VersionBitsMask(params, static_cast(i)); } @@ -4110,7 +4110,7 @@ void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman) nLastBlockFile = 0; setDirtyBlockIndex.clear(); setDirtyFileInfo.clear(); - versionbitscache.Clear(); + g_versionbitscache.Clear(); for (int b = 0; b < VERSIONBITS_NUM_BITS; b++) { warningcache[b].clear(); } From ea68b3a5729f5d240e968388c4f88acffeb27228 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 11 Mar 2021 12:17:22 +1000 Subject: [PATCH 06/12] [move-only] Rename versionbitsinfo to deploymentinfo --- src/Makefile.am | 4 ++-- src/chainparams.cpp | 2 +- src/consensus/params.h | 2 +- src/{versionbitsinfo.cpp => deploymentinfo.cpp} | 2 +- src/{versionbitsinfo.h => deploymentinfo.h} | 6 +++--- src/rpc/mining.cpp | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) rename src/{versionbitsinfo.cpp => deploymentinfo.cpp} (94%) rename src/{versionbitsinfo.h => deploymentinfo.h} (80%) diff --git a/src/Makefile.am b/src/Makefile.am index a8e1b7bc1ed..37ba5ad75b5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -145,6 +145,7 @@ BITCOIN_CORE_H = \ core_memusage.h \ cuckoocache.h \ dbwrapper.h \ + deploymentinfo.h \ deploymentstatus.h \ external_signer.h \ flatfile.h \ @@ -273,7 +274,6 @@ BITCOIN_CORE_H = \ validation.h \ validationinterface.h \ versionbits.h \ - versionbitsinfo.h \ wallet/bdb.h \ wallet/coincontrol.h \ wallet/coinselection.h \ @@ -542,6 +542,7 @@ libbitcoin_common_a_SOURCES = \ compressor.cpp \ core_read.cpp \ core_write.cpp \ + deploymentinfo.cpp \ external_signer.cpp \ init/common.cpp \ key.cpp \ @@ -563,7 +564,6 @@ libbitcoin_common_a_SOURCES = \ script/sign.cpp \ script/signingprovider.cpp \ script/standard.cpp \ - versionbitsinfo.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index fdaadeed4a9..58a27e053b9 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -7,9 +7,9 @@ #include #include +#include #include // for signet block challenge hash #include -#include #include diff --git a/src/consensus/params.h b/src/consensus/params.h index 7d5fe1a7340..174f4677fa0 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -26,7 +26,7 @@ enum DeploymentPos : uint16_t { DEPLOYMENT_TESTDUMMY, DEPLOYMENT_TAPROOT, // Deployment of Schnorr/Taproot (BIPs 340-342) - // NOTE: Also add new deployments to VersionBitsDeploymentInfo in versionbits.cpp + // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp MAX_VERSION_BITS_DEPLOYMENTS }; constexpr bool ValidDeployment(DeploymentPos dep) { return DEPLOYMENT_TESTDUMMY <= dep && dep <= DEPLOYMENT_TAPROOT; } diff --git a/src/versionbitsinfo.cpp b/src/deploymentinfo.cpp similarity index 94% rename from src/versionbitsinfo.cpp rename to src/deploymentinfo.cpp index fa41bad46d9..70d99611330 100644 --- a/src/versionbitsinfo.cpp +++ b/src/deploymentinfo.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include diff --git a/src/versionbitsinfo.h b/src/deploymentinfo.h similarity index 80% rename from src/versionbitsinfo.h rename to src/deploymentinfo.h index a7822bc747a..4c68856eaca 100644 --- a/src/versionbitsinfo.h +++ b/src/deploymentinfo.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_VERSIONBITSINFO_H -#define BITCOIN_VERSIONBITSINFO_H +#ifndef BITCOIN_DEPLOYMENTINFO_H +#define BITCOIN_DEPLOYMENTINFO_H struct VBDeploymentInfo { /** Deployment name */ @@ -14,4 +14,4 @@ struct VBDeploymentInfo { extern const struct VBDeploymentInfo VersionBitsDeploymentInfo[]; -#endif // BITCOIN_VERSIONBITSINFO_H +#endif // BITCOIN_DEPLOYMENTINFO_H diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d6ca0959f23..04aa459e11f 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -35,7 +36,6 @@ #include #include #include -#include #include #include From 92f48f360da5f425428b761219301f509826bec4 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 11 Mar 2021 12:47:24 +1000 Subject: [PATCH 07/12] deploymentinfo: Add DeploymentName() --- src/deploymentinfo.cpp | 18 ++++++++++++++++++ src/deploymentinfo.h | 14 +++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/deploymentinfo.cpp b/src/deploymentinfo.cpp index 70d99611330..030a7806dec 100644 --- a/src/deploymentinfo.cpp +++ b/src/deploymentinfo.cpp @@ -16,3 +16,21 @@ const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_B /*.gbt_force =*/ true, }, }; + +std::string DeploymentName(Consensus::BuriedDeployment dep) +{ + assert(ValidDeployment(dep)); + switch (dep) { + case Consensus::DEPLOYMENT_HEIGHTINCB: + return "bip34"; + case Consensus::DEPLOYMENT_CLTV: + return "bip65"; + case Consensus::DEPLOYMENT_DERSIG: + return "bip66"; + case Consensus::DEPLOYMENT_CSV: + return "csv"; + case Consensus::DEPLOYMENT_SEGWIT: + return "segwit"; + } // no default case, so the compiler can warn about missing cases + return ""; +} diff --git a/src/deploymentinfo.h b/src/deploymentinfo.h index 4c68856eaca..63d58a7da25 100644 --- a/src/deploymentinfo.h +++ b/src/deploymentinfo.h @@ -5,6 +5,10 @@ #ifndef BITCOIN_DEPLOYMENTINFO_H #define BITCOIN_DEPLOYMENTINFO_H +#include + +#include + struct VBDeploymentInfo { /** Deployment name */ const char *name; @@ -12,6 +16,14 @@ struct VBDeploymentInfo { bool gbt_force; }; -extern const struct VBDeploymentInfo VersionBitsDeploymentInfo[]; +extern const VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS]; + +std::string DeploymentName(Consensus::BuriedDeployment dep); + +inline std::string DeploymentName(Consensus::DeploymentPos pos) +{ + assert(Consensus::ValidDeployment(pos)); + return VersionBitsDeploymentInfo[pos].name; +} #endif // BITCOIN_DEPLOYMENTINFO_H From 8ee3e0bed5bf2cd3c7a68ca6ba6c65f7b9a72cca Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 11 Mar 2021 12:17:52 +1000 Subject: [PATCH 08/12] [refactor] rpc/blockchain.cpp: SoftForkPushBack Rename BIP9SoftForkPushBack and BuriedSoftForkPushBack to SoftForkPushBack and have the compiler figure out which one to use based on the deployment type. Avoids the need to update the file when burying a deployment. --- src/rpc/blockchain.cpp | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7ceff17d0b4..9c9a909d669 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -10,8 +10,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -38,6 +40,7 @@ #include #include #include +#include #include #include @@ -1344,25 +1347,25 @@ static RPCHelpMan verifychain() }; } -static void BuriedForkDescPushBack(UniValue& softforks, const std::string &name, int softfork_height, int tip_height) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const Consensus::Params& params, Consensus::BuriedDeployment dep) { // For buried deployments. // A buried deployment is one where the height of the activation has been hardcoded into // the client implementation long after the consensus change has activated. See BIP 90. // Buried deployments with activation height value of // std::numeric_limits::max() are disabled and thus hidden. - if (softfork_height == std::numeric_limits::max()) return; + if (!DeploymentEnabled(params, dep)) return; UniValue rv(UniValue::VOBJ); rv.pushKV("type", "buried"); // getblockchaininfo reports the softfork as active from when the chain height is // one below the activation height - rv.pushKV("active", tip_height + 1 >= softfork_height); - rv.pushKV("height", softfork_height); - softforks.pushKV(name, rv); + rv.pushKV("active", DeploymentActiveAfter(active_chain_tip, params, dep)); + rv.pushKV("height", params.DeploymentHeight(dep)); + softforks.pushKV(DeploymentName(dep), rv); } -static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const std::string &name, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) +static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) { // For BIP9 deployments. // Deployments that are never active are hidden. @@ -1406,7 +1409,7 @@ static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniVal } rv.pushKV("active", ThresholdState::ACTIVE == thresholdState); - softforks.pushKV(name, rv); + softforks.pushKV(DeploymentName(id), rv); } RPCHelpMan getblockchaininfo() @@ -1503,14 +1506,14 @@ RPCHelpMan getblockchaininfo() const Consensus::Params& consensusParams = Params().GetConsensus(); UniValue softforks(UniValue::VOBJ); - BuriedForkDescPushBack(softforks, "bip34", consensusParams.BIP34Height, height); - BuriedForkDescPushBack(softforks, "bip66", consensusParams.BIP66Height, height); - BuriedForkDescPushBack(softforks, "bip65", consensusParams.BIP65Height, height); - BuriedForkDescPushBack(softforks, "csv", consensusParams.CSVHeight, height); - BuriedForkDescPushBack(softforks, "segwit", consensusParams.SegwitHeight, height); - BIP9SoftForkDescPushBack(tip, softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); - BIP9SoftForkDescPushBack(tip, softforks, "taproot", consensusParams, Consensus::DEPLOYMENT_TAPROOT); - obj.pushKV("softforks", softforks); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_HEIGHTINCB); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_DERSIG); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_CLTV); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_CSV); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_SEGWIT); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); + SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_TAPROOT); + obj.pushKV("softforks", softforks); obj.pushKV("warnings", GetWarnings(false).original); return obj; From 0cfd6c6a8f929d5567ac41f95c21548f115efee5 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 29 Dec 2020 11:19:06 +1000 Subject: [PATCH 09/12] [refactor] versionbits: make VersionBitsCache a full class Moves the VersionBits* functions to be methods of the cache class, and makes the cache and its lock private to the class. --- src/deploymentstatus.h | 2 +- src/rpc/blockchain.cpp | 6 +++--- src/rpc/mining.cpp | 6 +++--- src/test/versionbits_tests.cpp | 4 ++-- src/validation.cpp | 4 ++-- src/versionbits.cpp | 20 ++++++++++---------- src/versionbits.h | 31 ++++++++++++++++++------------- 7 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h index 3c4f1895a1b..84c5e54698c 100644 --- a/src/deploymentstatus.h +++ b/src/deploymentstatus.h @@ -23,7 +23,7 @@ inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus inline bool DeploymentActiveAfter(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos dep) { assert(Consensus::ValidDeployment(dep)); - return ThresholdState::ACTIVE == VersionBitsState(pindexPrev, params, dep, g_versionbitscache); + return ThresholdState::ACTIVE == g_versionbitscache.State(pindexPrev, params, dep); } /** Determine if a deployment is active for this block */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9c9a909d669..b630458f234 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1372,7 +1372,7 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; UniValue bip9(UniValue::VOBJ); - const ThresholdState thresholdState = VersionBitsState(active_chain_tip, consensusParams, id, g_versionbitscache); + const ThresholdState thresholdState = g_versionbitscache.State(active_chain_tip, consensusParams, id); switch (thresholdState) { case ThresholdState::DEFINED: bip9.pushKV("status", "defined"); break; case ThresholdState::STARTED: bip9.pushKV("status", "started"); break; @@ -1386,12 +1386,12 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& } bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime); bip9.pushKV("timeout", consensusParams.vDeployments[id].nTimeout); - int64_t since_height = VersionBitsStateSinceHeight(active_chain_tip, consensusParams, id, g_versionbitscache); + int64_t since_height = g_versionbitscache.StateSinceHeight(active_chain_tip, consensusParams, id); bip9.pushKV("since", since_height); if (ThresholdState::STARTED == thresholdState) { UniValue statsUV(UniValue::VOBJ); - BIP9Stats statsStruct = VersionBitsStatistics(active_chain_tip, consensusParams, id); + BIP9Stats statsStruct = g_versionbitscache.Statistics(active_chain_tip, consensusParams, id); statsUV.pushKV("period", statsStruct.period); statsUV.pushKV("threshold", statsStruct.threshold); statsUV.pushKV("elapsed", statsStruct.elapsed); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 04aa459e11f..2762d78493b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -841,7 +841,7 @@ static RPCHelpMan getblocktemplate() UniValue vbavailable(UniValue::VOBJ); for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { Consensus::DeploymentPos pos = Consensus::DeploymentPos(j); - ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, g_versionbitscache); + ThresholdState state = g_versionbitscache.State(pindexPrev, consensusParams, pos); switch (state) { case ThresholdState::DEFINED: case ThresholdState::FAILED: @@ -849,7 +849,7 @@ static RPCHelpMan getblocktemplate() break; case ThresholdState::LOCKED_IN: // Ensure bit is set in block version - pblock->nVersion |= VersionBitsMask(consensusParams, pos); + pblock->nVersion |= g_versionbitscache.Mask(consensusParams, pos); // FALL THROUGH to get vbavailable set... case ThresholdState::STARTED: { @@ -858,7 +858,7 @@ static RPCHelpMan getblocktemplate() if (setClientRules.find(vbinfo.name) == setClientRules.end()) { if (!vbinfo.gbt_force) { // If the client doesn't support this, don't indicate it in the [default] version - pblock->nVersion &= ~VersionBitsMask(consensusParams, pos); + pblock->nVersion &= ~g_versionbitscache.Mask(consensusParams, pos); } } break; diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 49c61f5f2ff..946b35f3b93 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -288,7 +288,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // Check min_activation_height is on a retarget boundary BOOST_REQUIRE_EQUAL(min_activation_height % params.nMinerConfirmationWindow, 0U); - const uint32_t bitmask{VersionBitsMask(params, dep)}; + const uint32_t bitmask{g_versionbitscache.Mask(params, dep)}; BOOST_CHECK_EQUAL(bitmask, uint32_t{1} << bit); // In the first chain, test that the bit is set by CBV until it has failed. @@ -426,7 +426,7 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) // not take precedence over STARTED/LOCKED_IN. So all softforks on // the same bit might overlap, even when non-overlapping start-end // times are picked. - const uint32_t dep_mask{VersionBitsMask(chainParams->GetConsensus(), dep)}; + const uint32_t dep_mask{g_versionbitscache.Mask(chainParams->GetConsensus(), dep)}; BOOST_CHECK(!(chain_all_vbits & dep_mask)); chain_all_vbits |= dep_mask; check_computeblockversion(chainParams->GetConsensus(), dep); diff --git a/src/validation.cpp b/src/validation.cpp index d2baa92dde3..024f643be5c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1611,9 +1611,9 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para int32_t nVersion = VERSIONBITS_TOP_BITS; for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { - ThresholdState state = VersionBitsState(pindexPrev, params, static_cast(i), g_versionbitscache); + ThresholdState state = g_versionbitscache.State(pindexPrev, params, static_cast(i)); if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) { - nVersion |= VersionBitsMask(params, static_cast(i)); + nVersion |= g_versionbitscache.Mask(params, static_cast(i)); } } diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 07ecc93c939..0fbad0a64eb 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -190,32 +190,32 @@ public: } // namespace -ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache) +ThresholdState VersionBitsCache::State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) { - LOCK(cache.mutex); - return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, cache.caches[pos]); + LOCK(m_mutex); + return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, m_caches[pos]); } -BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) +BIP9Stats VersionBitsCache::Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) { return VersionBitsConditionChecker(pos).GetStateStatisticsFor(pindexPrev, params); } -int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache) +int VersionBitsCache::StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) { - LOCK(cache.mutex); - return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, cache.caches[pos]); + LOCK(m_mutex); + return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, m_caches[pos]); } -uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos) +uint32_t VersionBitsCache::Mask(const Consensus::Params& params, Consensus::DeploymentPos pos) { return VersionBitsConditionChecker(pos).Mask(params); } void VersionBitsCache::Clear() { - LOCK(mutex); + LOCK(m_mutex); for (unsigned int d = 0; d < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; d++) { - caches[d].clear(); + m_caches[d].clear(); } } diff --git a/src/versionbits.h b/src/versionbits.h index 24279a0de4f..4ede9208037 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -73,22 +73,27 @@ public: int GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const; }; -/** BIP 9 allows multiple softforks to be deployed in parallel. We cache per-period state for every one of them - * keyed by the bit position used to signal support. */ -struct VersionBitsCache +/** BIP 9 allows multiple softforks to be deployed in parallel. We cache + * per-period state for every one of them. */ +class VersionBitsCache { - Mutex mutex; - ThresholdConditionCache caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(mutex); +private: + Mutex m_mutex; + ThresholdConditionCache m_caches[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] GUARDED_BY(m_mutex); + +public: + /** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */ + static BIP9Stats Statistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + + static uint32_t Mask(const Consensus::Params& params, Consensus::DeploymentPos pos); + + /** Get the BIP9 state for a given deployment for the block after pindexPrev. */ + ThresholdState State(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + + /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ + int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); void Clear(); }; -/** Get the BIP9 state for a given deployment for the block after pindexPrev. */ -ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); -/** Get the numerical statistics for a given deployment for the signalling period that includes the block after pindexPrev. */ -BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); -/** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ -int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); -uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos); - #endif // BITCOIN_VERSIONBITS_H From 4a69b4dbe0d7f504811b67c399da7e6d11e4f805 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 16 Apr 2021 18:33:02 +1000 Subject: [PATCH 10/12] [move-only] Move ComputeBlockVersion from validation to versionbits --- src/validation.cpp | 14 -------------- src/validation.h | 5 ----- src/versionbits.cpp | 16 ++++++++++++++++ src/versionbits.h | 5 +++++ 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 024f643be5c..ee10e047bd1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1606,20 +1606,6 @@ void StopScriptCheckWorkerThreads() scriptcheckqueue.StopWorkerThreads(); } -int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) -{ - int32_t nVersion = VERSIONBITS_TOP_BITS; - - for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { - ThresholdState state = g_versionbitscache.State(pindexPrev, params, static_cast(i)); - if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) { - nVersion |= g_versionbitscache.Mask(params, static_cast(i)); - } - } - - return nVersion; -} - /** * Threshold condition checker that triggers when unknown versionbits are seen on the network. */ diff --git a/src/validation.h b/src/validation.h index 0f00556053a..3d66e3161dd 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1019,11 +1019,6 @@ public: /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr pblocktree; -/** - * Determine what nVersion a new block should use. - */ -int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); - using FopenFn = std::function; /** Dump the mempool to disk. */ diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 0fbad0a64eb..3497fc049e6 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -212,6 +212,22 @@ uint32_t VersionBitsCache::Mask(const Consensus::Params& params, Consensus::Depl return VersionBitsConditionChecker(pos).Mask(params); } +extern VersionBitsCache g_versionbitscache; // removed in next commit + +int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) +{ + int32_t nVersion = VERSIONBITS_TOP_BITS; + + for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { + ThresholdState state = g_versionbitscache.State(pindexPrev, params, static_cast(i)); + if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) { + nVersion |= g_versionbitscache.Mask(params, static_cast(i)); + } + } + + return nVersion; +} + void VersionBitsCache::Clear() { LOCK(m_mutex); diff --git a/src/versionbits.h b/src/versionbits.h index 4ede9208037..c18a8d1176a 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -96,4 +96,9 @@ public: void Clear(); }; +/** + * Determine what nVersion a new block should use. + */ +int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); + #endif // BITCOIN_VERSIONBITS_H From c5f36725e877d8eb492383844f8ef7535466b366 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 16 Apr 2021 18:34:34 +1000 Subject: [PATCH 11/12] [refactor] Move ComputeBlockVersion into VersionBitsCache This also changes ComputeBlockVersion to take the versionbits cache mutex once, rather than once for each versionbits deployment. --- src/miner.cpp | 2 +- src/test/versionbits_tests.cpp | 5 +++++ src/validation.cpp | 2 +- src/versionbits.cpp | 10 +++++----- src/versionbits.h | 9 ++++----- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index c1a992b9e9c..d9186a5d6dc 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -121,7 +121,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc assert(pindexPrev != nullptr); nHeight = pindexPrev->nHeight + 1; - pblock->nVersion = ComputeBlockVersion(pindexPrev, chainparams.GetConsensus()); + pblock->nVersion = g_versionbitscache.ComputeBlockVersion(pindexPrev, chainparams.GetConsensus()); // -regtest only: allow overriding block.nVersion with // -blockversion=N to test forking scenarios if (chainparams.MineBlocksOnDemand()) diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 946b35f3b93..60a8373cf42 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -15,6 +15,11 @@ /* Define a virtual block time, one block per 10 minutes after Nov 14 2014, 0:55:36am */ static int32_t TestTime(int nHeight) { return 1415926536 + 600 * nHeight; } +static int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) +{ + return g_versionbitscache.ComputeBlockVersion(pindexPrev, params); +} + static const std::string StateName(ThresholdState state) { switch (state) { diff --git a/src/validation.cpp b/src/validation.cpp index ee10e047bd1..65d2dfa3b7a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1627,7 +1627,7 @@ public: return pindex->nHeight >= params.MinBIP9WarningHeight && ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) && ((pindex->nVersion >> bit) & 1) != 0 && - ((ComputeBlockVersion(pindex->pprev, params) >> bit) & 1) == 0; + ((g_versionbitscache.ComputeBlockVersion(pindex->pprev, params) >> bit) & 1) == 0; } }; diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 3497fc049e6..94c3c9559f6 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -212,16 +212,16 @@ uint32_t VersionBitsCache::Mask(const Consensus::Params& params, Consensus::Depl return VersionBitsConditionChecker(pos).Mask(params); } -extern VersionBitsCache g_versionbitscache; // removed in next commit - -int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) +int32_t VersionBitsCache::ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) { + LOCK(m_mutex); int32_t nVersion = VERSIONBITS_TOP_BITS; for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { - ThresholdState state = g_versionbitscache.State(pindexPrev, params, static_cast(i)); + Consensus::DeploymentPos pos = static_cast(i); + ThresholdState state = VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, m_caches[pos]); if (state == ThresholdState::LOCKED_IN || state == ThresholdState::STARTED) { - nVersion |= g_versionbitscache.Mask(params, static_cast(i)); + nVersion |= Mask(params, pos); } } diff --git a/src/versionbits.h b/src/versionbits.h index c18a8d1176a..0b2f4a0258a 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -93,12 +93,11 @@ public: /** Get the block height at which the BIP9 deployment switched into the state for the block after pindexPrev. */ int StateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); + /** Determine what nVersion a new block should use + */ + int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); + void Clear(); }; -/** - * Determine what nVersion a new block should use. - */ -int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); - #endif // BITCOIN_VERSIONBITS_H From e48826ad87b4f92261f7433e84f48dac9bd9e5c3 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Mon, 7 Jun 2021 14:58:54 +1000 Subject: [PATCH 12/12] tests: remove ComputeBlockVersion shortcut from versionbits tests --- src/test/versionbits_tests.cpp | 41 +++++++++++++++------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 60a8373cf42..690031cdc17 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -15,11 +15,6 @@ /* Define a virtual block time, one block per 10 minutes after Nov 14 2014, 0:55:36am */ static int32_t TestTime(int nHeight) { return 1415926536 + 600 * nHeight; } -static int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params) -{ - return g_versionbitscache.ComputeBlockVersion(pindexPrev, params); -} - static const std::string StateName(ThresholdState state) { switch (state) { @@ -273,7 +268,7 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus int min_activation_height = params.vDeployments[dep].min_activation_height; // should not be any signalling for first block - BOOST_CHECK_EQUAL(ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); + BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); // always/never active deployments shouldn't need to be tested further if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || @@ -312,9 +307,9 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus // earlier time, so will transition from DEFINED to STARTED at the // end of the first period by mining blocks at nTime == 0 lastBlock = firstChain.Mine(params.nMinerConfirmationWindow - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(ComputeBlockVersion(lastBlock, params) & (1< 0) { lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((ComputeBlockVersion(lastBlock, params) & (1<nHeight + 1 < min_activation_height) { // check signalling continues while min_activation_height is not reached lastBlock = secondChain.Mine(min_activation_height - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); // then reach min_activation_height, which was already REQUIRE'd to start a new period lastBlock = secondChain.Mine(min_activation_height, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); } // Check that we don't signal after activation - BOOST_CHECK_EQUAL(ComputeBlockVersion(lastBlock, params) & (1<