From 216a5f7563ab648ef5f86cbda97f1e8cda9bc394 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 10 Nov 2023 21:31:12 +0700 Subject: [PATCH] refactor: make MNActivationHeight in Params() indeed constant (#5658) ## Issue being fixed or feature implemented Addressed issues and comments from [PR comment](https://github.com/dashpay/dash/pull/5469#discussion_r1317886678) and [PR comment](https://github.com/dashpay/dash/pull/5469#discussion_r1338704082) `Params()` should be const; global variable `CMNHFManager` is a better out-come. ## What was done? The helpers and direct calls of `UpdateMNParams` for each block to update non-constant member in `Params()` is not needed anymore. Instead `CMNHFManager` takes cares about status of Signals for each block, update them dynamically and save in evo db. ## How Has This Been Tested? Run unit/functional tests. ## Breaking Changes Changed rpc `getblockchaininfo`. the field `ehf` changed meaning: it's now only a flag -1/0; but it is introduced a new field `ehf_height` now that a height. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --------- Co-authored-by: UdjinM6 Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Co-authored-by: thephez --- src/chainparams.cpp | 41 +++++----- src/chainparams.h | 6 +- src/consensus/params.h | 12 ++- src/evo/mnhftx.cpp | 75 +++++++++---------- src/evo/mnhftx.h | 27 +++---- src/init.cpp | 3 - src/rpc/blockchain.cpp | 17 +++-- src/test/block_reward_reallocation_tests.cpp | 8 +- .../dynamic_activation_thresholds_tests.cpp | 2 +- src/test/fuzz/versionbits.cpp | 2 +- src/test/versionbits_tests.cpp | 2 +- src/validation.cpp | 4 +- src/versionbits.cpp | 23 ++++-- src/versionbits.h | 33 +++++++- test/functional/feature_asset_locks.py | 1 + test/functional/feature_cltv.py | 2 +- test/functional/feature_dersig.py | 2 +- test/functional/feature_governance.py | 2 +- test/functional/feature_mnehf.py | 4 +- .../feature_new_quorum_type_activation.py | 2 +- test/functional/rpc_blockchain.py | 6 +- .../test_framework/test_framework.py | 9 ++- 22 files changed, 159 insertions(+), 124 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index bec1232f6d..3877ee80cf 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -106,7 +106,7 @@ static CBlock FindDevNetGenesisBlock(const CBlock &prevBlock, const CAmount& rew assert(false); } -bool CChainParams::UpdateMNActivationParam(int nBit, int height, int64_t timePast, bool fJustCheck) const +bool CChainParams::IsValidMNActivation(int nBit, int64_t timePast) const { assert(nBit < VERSIONBITS_NUM_BITS); @@ -114,17 +114,14 @@ bool CChainParams::UpdateMNActivationParam(int nBit, int height, int64_t timePas if (consensus.vDeployments[index].bit == nBit) { auto& deployment = consensus.vDeployments[index]; if (timePast > deployment.nTimeout || timePast < deployment.nStartTime) { - LogPrintf("%s: activation by bit=%d height=%d deployment='%s' is out of time range start=%lld timeout=%lld\n", __func__, nBit, height, VersionBitsDeploymentInfo[Consensus::DeploymentPos(index)].name, deployment.nStartTime, deployment.nTimeout); + LogPrintf("%s: activation by bit=%d deployment='%s' is out of time range start=%lld timeout=%lld\n", __func__, nBit, VersionBitsDeploymentInfo[Consensus::DeploymentPos(index)].name, deployment.nStartTime, deployment.nTimeout); continue; } - if (deployment.nMNActivationHeight < 0) { - LogPrintf("%s: trying to set MnEHF height=%d for non-masternode activation fork bit=%d\n", __func__, height, nBit); + if (!deployment.useEHF) { + LogPrintf("%s: trying to set MnEHF for non-masternode activation fork bit=%d\n", __func__, nBit); return false; } - LogPrintf("%s: set MnEHF height=%d for bit=%d fJustCheck=%d is valid\n", __func__, height, nBit, fJustCheck); - if (!fJustCheck) { - deployment.nMNActivationHeight = height; - } + LogPrintf("%s: set MnEHF for bit=%d is valid\n", __func__, nBit); return true; } } @@ -224,7 +221,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdStart = 3226; // 80% of 4032 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdMin = 2420; // 60% of 4032 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nFalloffCoeff = 5; // this corresponds to 10 periods - consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nMNActivationHeight = 0; // requires EHF activation + consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].useEHF = true; // The best chain should have at least this much work. consensus.nMinimumChainWork = uint256S("0x000000000000000000000000000000000000000000008677827656704520eb39"); // 1889000 @@ -422,7 +419,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdStart = 80; // 80% of 100 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdMin = 60; // 60% of 100 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nFalloffCoeff = 5; // this corresponds to 10 periods - consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nMNActivationHeight = 0; // requires EHF activation + consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].useEHF = true; // The best chain should have at least this much work. consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000002d68d24632e300f"); // 905100 @@ -595,7 +592,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdStart = 80; // 80% of 100 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdMin = 60; // 60% of 100 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nFalloffCoeff = 5; // this corresponds to 10 periods - consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nMNActivationHeight = 0; // requires EHF activation + consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].useEHF = true; // The best chain should have at least this much work. consensus.nMinimumChainWork = uint256S("0x000000000000000000000000000000000000000000000000000000000000000"); @@ -845,7 +842,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdStart = 9; // 80% of 12 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nThresholdMin = 7; // 60% of 7 consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nFalloffCoeff = 5; // this corresponds to 10 periods - consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].nMNActivationHeight = 0; // requires EHF activation + consensus.vDeployments[Consensus::DEPLOYMENT_MN_RR].useEHF = true; // The best chain should have at least this much work. consensus.nMinimumChainWork = uint256S("0x00"); @@ -953,7 +950,7 @@ public: /** * Allows modifying the Version Bits regtest parameters. */ - void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout, int64_t nWindowSize, int64_t nThresholdStart, int64_t nThresholdMin, int64_t nFalloffCoeff, int64_t nMNActivationHeight) + void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout, int64_t nWindowSize, int64_t nThresholdStart, int64_t nThresholdMin, int64_t nFalloffCoeff, int64_t nUseEHF) { consensus.vDeployments[d].nStartTime = nStartTime; consensus.vDeployments[d].nTimeout = nTimeout; @@ -969,8 +966,8 @@ public: if (nFalloffCoeff != -1) { consensus.vDeployments[d].nFalloffCoeff = nFalloffCoeff; } - if (nMNActivationHeight != -1) { - consensus.vDeployments[d].nMNActivationHeight = nMNActivationHeight; + if (nUseEHF != -1) { + consensus.vDeployments[d].useEHF = nUseEHF > 0; } } void UpdateActivationParametersFromArgs(const ArgsManager& args); @@ -1049,9 +1046,9 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) throw std::runtime_error("Version bits parameters malformed, expecting " ":: or " ":::: or " - ":::::::"); + ":::::::"); } - int64_t nStartTime, nTimeout, nWindowSize = -1, nThresholdStart = -1, nThresholdMin = -1, nFalloffCoeff = -1, nMNActivationHeight = -1; + int64_t nStartTime, nTimeout, nWindowSize = -1, nThresholdStart = -1, nThresholdMin = -1, nFalloffCoeff = -1, nUseEHF = -1; if (!ParseInt64(vDeploymentParams[1], &nStartTime)) { throw std::runtime_error(strprintf("Invalid nStartTime (%s)", vDeploymentParams[1])); } @@ -1073,17 +1070,17 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) if (!ParseInt64(vDeploymentParams[6], &nFalloffCoeff)) { throw std::runtime_error(strprintf("Invalid nFalloffCoeff (%s)", vDeploymentParams[6])); } - if (!ParseInt64(vDeploymentParams[7], &nMNActivationHeight)) { - throw std::runtime_error(strprintf("Invalid nMNActivationHeight (%s)", vDeploymentParams[7])); + if (!ParseInt64(vDeploymentParams[7], &nUseEHF)) { + throw std::runtime_error(strprintf("Invalid nUseEHF (%s)", vDeploymentParams[7])); } } bool found = false; for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) { - UpdateVersionBitsParameters(Consensus::DeploymentPos(j), nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nMNActivationHeight); + UpdateVersionBitsParameters(Consensus::DeploymentPos(j), nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); found = true; - LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld, window=%ld, thresholdstart=%ld, thresholdmin=%ld, falloffcoeff=%ld, mnactivationHeight=%ld\n", - vDeploymentParams[0], nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nMNActivationHeight); + LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld, window=%ld, thresholdstart=%ld, thresholdmin=%ld, falloffcoeff=%ld, useehf=%ld\n", + vDeploymentParams[0], nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); break; } } diff --git a/src/chainparams.h b/src/chainparams.h index f5f116a326..bf36b4b851 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -135,15 +135,13 @@ public: void UpdateBudgetParameters(int nMasternodePaymentsStartBlock, int nBudgetPaymentsStartBlock, int nSuperblockStartBlock); void UpdateLLMQInstantSend(Consensus::LLMQType llmqType); /** - * Update params for Masternodes EHF + * Validate params for Masternodes EHF * * @param[in] nBit The version bit to update - * @param[in] height The height of block where that signal is mined * @param[in] timePast The block time to validate if release is already time-outed - * @param[in] fJustCheck If true do not update any internal data, only validate params * @return Whether params are legit and params are updated (if release is known) */ - bool UpdateMNActivationParam(int nBit, int height, int64_t timePast, bool fJustCheck) const; + bool IsValidMNActivation(int nBit, int64_t timePast) const; int PoolMinParticipants() const { return nPoolMinParticipants; } int PoolMaxParticipants() const { return nPoolMaxParticipants; } int FulfilledRequestExpireTime() const { return nFulfilledRequestExpireTime; } diff --git a/src/consensus/params.h b/src/consensus/params.h index 650133c6d2..6c0c25c75d 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -40,6 +40,11 @@ struct BIP9Deployment { int64_t nThresholdMin{0}; /** A coefficient which adjusts the speed a required number of signaling blocks is decreasing from nThresholdStart to nThresholdMin at with each period. */ int64_t nFalloffCoeff{0}; + /** This value is used for forks activated by masternodes. + * false means it is a regular fork, no masternodes confirmation is needed. + * true means that a signalling of masternodes is expected first to determine a height when miners signals are matter. + */ + bool useEHF{false}; /** Constant for nTimeout very far in the future. */ static constexpr int64_t NO_TIMEOUT = std::numeric_limits::max(); @@ -49,13 +54,6 @@ struct BIP9Deployment { * process (which takes at least 3 BIP9 intervals). Only tests that specifically test the * behaviour during activation cannot use this. */ static constexpr int64_t ALWAYS_ACTIVE = -1; - - /** this value is used for forks activated by master nodes. - * negative values means it is regular fork, no masternodes confirmation is needed. - * 0 means that there's no approval from masternodes yet. - * Otherwise it shows minimum height when miner's signals for this block can be assumed - */ - mutable int64_t nMNActivationHeight{-1}; }; /** diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 37a871c072..34a031e23e 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -37,6 +37,19 @@ CMutableTransaction MNHFTxPayload::PrepareTx() const return tx; } +CMNHFManager::CMNHFManager(CEvoDB& evoDb) : + m_evoDb(evoDb) +{ + assert(globalInstance == nullptr); + globalInstance = this; +} + +CMNHFManager::~CMNHFManager() +{ + assert(globalInstance != nullptr); + globalInstance = nullptr; +} + CMNHFManager::Signals CMNHFManager::GetSignalsStage(const CBlockIndex* const pindexPrev) { Signals signals = GetFromCache(pindexPrev); @@ -123,7 +136,7 @@ bool CheckMNHFTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValida return false; } - if (!Params().UpdateMNActivationParam(mnhfTx.signal.versionBit, pindexPrev->nHeight, pindexPrev->GetMedianTimePast(), /* fJustCheck= */ true )) { + if (!Params().IsValidMNActivation(mnhfTx.signal.versionBit, pindexPrev->GetMedianTimePast())) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mnhf-non-ehf"); } @@ -202,7 +215,7 @@ bool CMNHFManager::ProcessBlock(const CBlock& block, const CBlockIndex* const pi return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-duplicate"); } - if (!Params().UpdateMNActivationParam(versionBit, mined_height, pindex->GetMedianTimePast(), true /* fJustCheck */)) { + if (!Params().IsValidMNActivation(versionBit, pindex->GetMedianTimePast())) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-mnhf-non-mn-fork"); } } @@ -211,11 +224,8 @@ bool CMNHFManager::ProcessBlock(const CBlock& block, const CBlockIndex* const pi return true; } for (const auto& versionBit : new_signals) { - signals.insert({versionBit, mined_height}); - - if (!Params().UpdateMNActivationParam(versionBit, mined_height, pindex->GetMedianTimePast(), false /* fJustCheck */)) { - // it should not ever fail - all checks are done above - assert(false); + if (Params().IsValidMNActivation(versionBit, pindex->GetMedianTimePast())) { + signals.insert({versionBit, mined_height}); } } @@ -244,42 +254,22 @@ bool CMNHFManager::UndoBlock(const CBlock& block, const CBlockIndex* const pinde for (const auto& versionBit : excluded_signals) { LogPrintf("%s: exclude mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals.size()); assert(signals.find(versionBit) != signals.end()); - - bool update_ret = Params().UpdateMNActivationParam(versionBit, 0, pindex->GetMedianTimePast(), false /* fJustCheck */); - assert(update_ret); + assert(Params().IsValidMNActivation(versionBit, pindex->GetMedianTimePast())); } return true; } -void CMNHFManager::UpdateChainParams(const CBlockIndex* const pindex, const CBlockIndex* const pindexOld) -{ - LogPrintf("%s: update chain params %s -> %s\n", __func__, pindexOld ? pindexOld->GetBlockHash().ToString() : "", pindex ? pindex->GetBlockHash().ToString() : ""); - Signals signals_old{GetFromCache(pindexOld)}; - for (const auto& signal: signals_old) { - const uint8_t versionBit = signal.first; - - LogPrintf("%s: unload mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals_old.size()); - - bool update_ret = Params().UpdateMNActivationParam(versionBit, 0, pindex->GetMedianTimePast(), /* fJustCheck= */ false); - assert(update_ret); - } - - Signals signals{GetFromCache(pindex)}; - for (const auto& signal: signals) { - const uint8_t versionBit = signal.first; - const int value = signal.second; - - LogPrintf("%s: load mnhf bit=%d block:%s number of known signals:%lld\n", __func__, versionBit, pindex->GetBlockHash().ToString(), signals.size()); - - bool update_ret = Params().UpdateMNActivationParam(versionBit, value, pindex->GetMedianTimePast(), /* fJustCheck= */ false); - assert(update_ret); - } -} - CMNHFManager::Signals CMNHFManager::GetFromCache(const CBlockIndex* const pindex) { if (pindex == nullptr) return {}; + + // TODO: remove this check of phashBlock to nullptr + // This check is needed only because unit test 'versionbits_tests.cpp' + // lets `phashBlock` to be nullptr + if (pindex->phashBlock == nullptr) return {}; + + const uint256& blockHash = pindex->GetBlockHash(); Signals signals{}; { @@ -293,9 +283,8 @@ CMNHFManager::Signals CMNHFManager::GetFromCache(const CBlockIndex* const pindex mnhfCache.insert(blockHash, {}); return {}; } - if (!m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) { - LogPrintf("CMNHFManager::GetFromCache: failure: can't read MnEHF signals from db for %s\n", pindex->GetBlockHash().ToString()); - } + bool ok = m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals); + assert(ok); LOCK(cs_cache); mnhfCache.insert(blockHash, signals); return signals; @@ -308,9 +297,19 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p LOCK(cs_cache); mnhfCache.insert(blockHash, signals); } + if (VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20, versionbitscache) != ThresholdState::ACTIVE) { + return; + } m_evoDb.Write(std::make_pair(DB_SIGNALS, blockHash), signals); } +void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit) +{ + auto signals = GetFromCache(pindex->pprev); + signals.emplace(bit, pindex->nHeight); + AddToCache(signals, pindex); +} + std::string MNHFTx::ToString() const { return strprintf("MNHFTx(versionBit=%d, quorumHash=%s, sig=%s)", diff --git a/src/evo/mnhftx.h b/src/evo/mnhftx.h index 131f050cf4..c130045643 100644 --- a/src/evo/mnhftx.h +++ b/src/evo/mnhftx.h @@ -15,6 +15,7 @@ #include #include #include +#include class BlockValidationState; class CBlock; @@ -91,11 +92,8 @@ public: } }; -class CMNHFManager +class CMNHFManager : public AbstractEHFManager { -public: - using Signals = std::unordered_map; - private: CEvoDB& m_evoDb; @@ -105,9 +103,9 @@ private: unordered_lru_cache mnhfCache GUARDED_BY(cs_cache) {MNHFCacheSize}; public: - explicit CMNHFManager(CEvoDB& evoDb) : - m_evoDb(evoDb) {} - ~CMNHFManager() = default; + explicit CMNHFManager(CEvoDB& evoDb); + ~CMNHFManager(); + explicit CMNHFManager(const CMNHFManager&) = delete; /** * Every new block should be processed when Tip() is updated by calling of CMNHFManager::ProcessBlock @@ -119,19 +117,14 @@ public: */ bool UndoBlock(const CBlock& block, const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** - * Once app is started, need to initialize dictionary will all known signals at the current Tip() - * by calling UpdateChainParams() - */ - void UpdateChainParams(const CBlockIndex* const pindex, const CBlockIndex* const pindexOld) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + // Implements interface + Signals GetSignalsStage(const CBlockIndex* const pindexPrev) override; /** - * This function prepares signals for new block. - * This data is filterd expired signals from previous blocks. - * This member function is not const because it calls non-const GetFromCache() + * Helper that used in Unit Test to forcely setup EHF signal for specific block */ - Signals GetSignalsStage(const CBlockIndex* const pindexPrev); - + void AddSignal(const CBlockIndex* const pindex, int bit) LOCKS_EXCLUDED(cs_cache); private: void AddToCache(const Signals& signals, const CBlockIndex* const pindex); diff --git a/src/init.cpp b/src/init.cpp index 2dfbfc68fa..8a5d30fa27 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2103,9 +2103,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); } - LogPrintf("init.cpp: update chain params right after bls\n"); - node.mnhf_manager->UpdateChainParams(::ChainActive().Tip(), nullptr); - if (!CVerifyDB().VerifyDB( *chainstate, chainparams, chainstate->CoinsDB(), *node.evodb, diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6bc777bbb8..de5a74cccb 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1590,7 +1590,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, const std::unordered_map& signals, UniValue& softforks, const std::string &name, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { // For BIP9 deployments. // Deployments (e.g. testdummy) with timeout value before Jan 1, 2009 are hidden. @@ -1613,7 +1613,10 @@ static void BIP9SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniVal } bip9.pushKV("start_time", consensusParams.vDeployments[id].nStartTime); bip9.pushKV("timeout", consensusParams.vDeployments[id].nTimeout); - bip9.pushKV("ehf", consensusParams.vDeployments[id].nMNActivationHeight); + bip9.pushKV("ehf", consensusParams.vDeployments[id].useEHF); + if (auto it = signals.find(consensusParams.vDeployments[id].bit); it != signals.end()) { + bip9.pushKV("ehf_height", it->second); + } int64_t since_height = VersionBitsStateSinceHeight(active_chain_tip, consensusParams, id, versionbitscache); bip9.pushKV("since", since_height); if (ThresholdState::STARTED == thresholdState) @@ -1674,7 +1677,8 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)"}, {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"}, {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"}, - {RPCResult::Type::NUM, "ehf", "the minimum height when miner's signals for the deployment can be accepted (special values: \"-1\" - any, \"0\" - none)"}, + {RPCResult::Type::BOOL, "ehf", "returns true for EHF activated forks"}, + {RPCResult::Type::NUM, "ehf_height", /* optional */ true, "the minimum height when miner's signals for the deployment matter. Below this height miner signaling cannot trigger hard fork lock-in. Not specified for non-EHF forks"}, {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "activation_height", "expected activation height for this softfork (only for \"locked_in\" status)"}, {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP9 signalling for a softfork", @@ -1706,6 +1710,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) const CBlockIndex* tip = active_chainstate.m_chain.Tip(); CHECK_NONFATAL(tip); const int height = tip->nHeight; + const auto ehfSignals = active_chainstate.GetMNHFSignalsStage(tip); UniValue obj(UniValue::VOBJ); obj.pushKV("chain", strChainName); obj.pushKV("blocks", height); @@ -1750,9 +1755,9 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) BuriedForkDescPushBack(softforks, "dip0024", consensusParams.DIP0024Height, height); BuriedForkDescPushBack(softforks, "realloc", consensusParams.BRRHeight, height); BuriedForkDescPushBack(softforks, "v19", consensusParams.V19Height, height); - BIP9SoftForkDescPushBack(tip, softforks, "v20", consensusParams, Consensus::DEPLOYMENT_V20); - BIP9SoftForkDescPushBack(tip, softforks, "mn_rr", consensusParams, Consensus::DEPLOYMENT_MN_RR); - BIP9SoftForkDescPushBack(tip, softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); + BIP9SoftForkDescPushBack(tip, ehfSignals, softforks, "v20", consensusParams, Consensus::DEPLOYMENT_V20); + BIP9SoftForkDescPushBack(tip, ehfSignals, softforks, "mn_rr", consensusParams, Consensus::DEPLOYMENT_MN_RR); + BIP9SoftForkDescPushBack(tip, ehfSignals, softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY); obj.pushKV("softforks", softforks); diff --git a/src/test/block_reward_reallocation_tests.cpp b/src/test/block_reward_reallocation_tests.cpp index ec4cef54aa..0b1a968613 100644 --- a/src/test/block_reward_reallocation_tests.cpp +++ b/src/test/block_reward_reallocation_tests.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -40,7 +41,7 @@ using SimpleUTXOMap = std::map>; struct TestChainBRRBeforeActivationSetup : public TestChainSetup { // Force fast DIP3 activation - TestChainBRRBeforeActivationSetup() : TestChainSetup(497, {"-dip3params=30:50", "-vbparams=mn_rr:0:999999999999:20:16:12:5:0"}) {} + TestChainBRRBeforeActivationSetup() : TestChainSetup(497, {"-dip3params=30:50", "-vbparams=mn_rr:0:999999999999:20:16:12:5:1"}) {} }; static SimpleUTXOMap BuildSimpleUtxoMap(const std::vector& txs) @@ -269,7 +270,10 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS BOOST_CHECK(!llmq::utils::IsMNRewardReallocationActive(m_node.chainman->ActiveChain().Tip())); // Activate EHF "MN_RR" - Params().UpdateMNActivationParam(Params().GetConsensus().vDeployments[Consensus::DEPLOYMENT_MN_RR].bit, ::ChainActive().Height(), ::ChainActive().Tip()->GetMedianTimePast(), /*fJustCheck=*/ false); + { + LOCK(cs_main); + m_node.mnhf_manager->AddSignal(m_node.chainman->ActiveChain().Tip(), 10); + } // Reward split should stay ~75/25 after reallocation is done, // check 10 next superblocks diff --git a/src/test/dynamic_activation_thresholds_tests.cpp b/src/test/dynamic_activation_thresholds_tests.cpp index a746d86e99..7e32d903d2 100644 --- a/src/test/dynamic_activation_thresholds_tests.cpp +++ b/src/test/dynamic_activation_thresholds_tests.cpp @@ -34,7 +34,7 @@ static constexpr int threshold(int attempt) struct TestChainDATSetup : public TestChainSetup { - TestChainDATSetup() : TestChainSetup(window - 2, {"-vbparams=testdummy:0:999999999999:100:80:60:5:-1"}) {} + TestChainDATSetup() : TestChainSetup(window - 2, {"-vbparams=testdummy:0:999999999999:100:80:60:5:0"}) {} void signal(int num_blocks, bool expected_lockin) { diff --git a/src/test/fuzz/versionbits.cpp b/src/test/fuzz/versionbits.cpp index 1841a7d301..286445b5ff 100644 --- a/src/test/fuzz/versionbits.cpp +++ b/src/test/fuzz/versionbits.cpp @@ -42,7 +42,7 @@ public: bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return Condition(pindex->nVersion); } int64_t BeginTime(const Consensus::Params& params) const override { return m_begin; } - int MasternodeBeginHeight(const Consensus::Params& params) const override { return 0; } + int SignalHeight(const CBlockIndex* const pindex, const Consensus::Params& params) const override { return 0; } int64_t EndTime(const Consensus::Params& params) const override { return m_end; } int Period(const Consensus::Params& params) const override { return m_period; } int Threshold(const Consensus::Params& params, int nAttempt) const override { return m_threshold; } diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index d570ea4367..2ff6379645 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -24,7 +24,7 @@ private: public: int64_t BeginTime(const Consensus::Params& params) const override { return TestTime(10000); } int64_t EndTime(const Consensus::Params& params) const override { return TestTime(20000); } - int MasternodeBeginHeight(const Consensus::Params& params) const override { return 0; } + int SignalHeight(const CBlockIndex* const pindex, const Consensus::Params& params) const override { return 0; } int Period(const Consensus::Params& params) const override { return 1000; } int Threshold(const Consensus::Params& params, int nAttempt) const override { return 900; } bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return (pindex->nVersion & 0x100); } diff --git a/src/validation.cpp b/src/validation.cpp index d7cc2123e0..7b52afe23f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1956,7 +1956,7 @@ public: int64_t BeginTime(const Consensus::Params& params) const override { return 0; } int64_t EndTime(const Consensus::Params& params) const override { return std::numeric_limits::max(); } - int MasternodeBeginHeight(const Consensus::Params& params) const override { return 0; } + int SignalHeight(const CBlockIndex* const pindex, const Consensus::Params& params) const override { return 0; } int Period(const Consensus::Params& params) const override { return params.nMinerConfirmationWindow; } int Threshold(const Consensus::Params& params, int nAttempt) const override { return params.nRuleChangeActivationThreshold; } @@ -2515,7 +2515,7 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState( std::unordered_map CChainState::GetMNHFSignalsStage(const CBlockIndex* const pindexPrev) { - return this->m_mnhfManager.GetSignalsStage(pindexPrev); + return m_mnhfManager.GetSignalsStage(pindexPrev); } bool CChainState::FlushStateToDisk( diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 5ffdb41a3f..3d26e31479 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -4,6 +4,9 @@ #include #include +#include + +#include static int calculateStartHeight(const CBlockIndex* pindexPrev, ThresholdState state, const int nPeriod, const ThresholdConditionCache& cache) { int nStartHeight{std::numeric_limits::max()}; @@ -31,7 +34,7 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* { int nPeriod = Period(params); int64_t nTimeStart = BeginTime(params); - int masternodeStartHeight = MasternodeBeginHeight(params); + int masternodeStartHeight = SignalHeight(pindexPrev, params); int64_t nTimeTimeout = EndTime(params); // Check if this deployment is always active. @@ -205,15 +208,20 @@ private: protected: int64_t BeginTime(const Consensus::Params& params) const override { return params.vDeployments[id].nStartTime; } - int MasternodeBeginHeight(const Consensus::Params& params) const override { + int SignalHeight(const CBlockIndex* const pindexPrev, const Consensus::Params& params) const override { const auto& deployment = params.vDeployments[id]; - if (deployment.nMNActivationHeight == 0) { - return std::numeric_limits::max(); - } - if (deployment.nMNActivationHeight < 0) { + if (!deployment.useEHF) { return 0; } - return deployment.nMNActivationHeight; + // ehfManager should be initialized before first usage of VersionBitsConditionChecker + const auto ehfManagerPtr = gsl::make_not_null(AbstractEHFManager::getInstance()); + const auto signals = ehfManagerPtr->GetSignalsStage(pindexPrev); + const auto it = signals.find(deployment.bit); + if (it == signals.end()) { + return std::numeric_limits::max(); + } + + return it->second; } int64_t EndTime(const Consensus::Params& params) const override { return params.vDeployments[id].nTimeout; } int Period(const Consensus::Params& params) const override { return params.vDeployments[id].nWindowSize ? params.vDeployments[id].nWindowSize : params.nMinerConfirmationWindow; } @@ -267,3 +275,4 @@ void VersionBitsCache::Clear() caches[d].clear(); } } +AbstractEHFManager* AbstractEHFManager::globalInstance{nullptr}; diff --git a/src/versionbits.h b/src/versionbits.h index db57fb5e07..8793d9d971 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -56,7 +56,7 @@ class AbstractThresholdConditionChecker { protected: virtual bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const =0; virtual int64_t BeginTime(const Consensus::Params& params) const =0; - virtual int MasternodeBeginHeight(const Consensus::Params& params) const = 0; + virtual int SignalHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params) const = 0; virtual int64_t EndTime(const Consensus::Params& params) const =0; virtual int Period(const Consensus::Params& params) const =0; virtual int Threshold(const Consensus::Params& params, int nAttempt) const =0; @@ -88,4 +88,35 @@ BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus:: int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos); +class AbstractEHFManager +{ +public: + using Signals = std::unordered_map; + + /** + * getInstance() is used in versionbit because it is non-trivial + * to get access to NodeContext from all usages of VersionBits* methods + * For simplification of interface this methods static/global variable is used + * to get access to EHF data + */ +public: + [[nodiscard]] static AbstractEHFManager* getInstance() { + return globalInstance; + }; + + /** + * `GetSignalsStage' prepares signals for new block. + * The results are diffent with GetFromCache results due to one more + * stage of processing: signals that would be expired in next block + * are excluded from results. + * This member function is not const because it calls non-const GetFromCache() + */ + virtual Signals GetSignalsStage(const CBlockIndex* const pindexPrev) = 0; + + +protected: + static AbstractEHFManager* globalInstance; + +}; + #endif // BITCOIN_VERSIONBITS_H diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index 437adba575..465a363f2a 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -509,6 +509,7 @@ class AssetLocksTest(DashTestFramework): self.check_mempool_size() # activate MN_RR reallocation + self.log.info("Activate mn_rr...") self.activate_mn_rr(expected_activation_height=node.getblockcount() + 12 * 3) self.log.info(f'height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}') assert_equal(new_total, self.get_credit_pool_balance()) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 540b92c151..a5f8550d71 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -60,7 +60,7 @@ class BIP65Test(BitcoinTestFramework): '-dip3params=9000:9000', '-par=1', # Use only one script thread to get the exact reject reason for testing '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard - '-vbparams=v20:0:999999999999:480:384:288:5:-1' # Delay v20 for this test as we don't need it + '-vbparams=v20:0:999999999999:480:384:288:5:0' # Delay v20 for this test as we don't need it ]] self.setup_clean_chain = True self.rpc_timeout = 480 diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 1fa6a52c58..0be42872ca 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -40,7 +40,7 @@ def unDERify(tx): class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1', '-vbparams=v20:0:999999999999:480:384:288:5:-1']] # Use only one script thread to get the exact reject reason for testing + self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1', '-vbparams=v20:0:999999999999:480:384:288:5:0']] # Use only one script thread to get the exact reject reason for testing self.setup_clean_chain = True self.rpc_timeout = 240 diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 976ae9ef0b..3b076aa960 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -14,7 +14,7 @@ class DashGovernanceTest (DashTestFramework): def set_test_params(self): self.v20_start_time = 1417713500 # using adjusted v20 deployment params to test an edge case where superblock maturity window is equal to deployment window size - self.set_dash_test_params(6, 5, [["-budgetparams=10:10:10", f"-vbparams=v20:{self.v20_start_time}:999999999999:10:8:6:5:-1"]] * 6, fast_dip3_enforcement=True) + self.set_dash_test_params(6, 5, [["-budgetparams=10:10:10", f"-vbparams=v20:{self.v20_start_time}:999999999999:10:8:6:5:0"]] * 6, fast_dip3_enforcement=True) def prepare_object(self, object_type, parent_hash, creation_time, revision, name, amount, payment_address): proposal_rev = revision diff --git a/test/functional/feature_mnehf.py b/test/functional/feature_mnehf.py index 2fabdf367a..376d7936ad 100755 --- a/test/functional/feature_mnehf.py +++ b/test/functional/feature_mnehf.py @@ -25,7 +25,7 @@ from test_framework.util import ( class MnehfTest(DashTestFramework): def set_test_params(self): - extra_args = [["-vbparams=testdummy:0:999999999999:12:12:12:5:0", "-persistmempool=0"] for _ in range(4)] + extra_args = [["-vbparams=testdummy:0:999999999999:12:12:12:5:1", "-persistmempool=0"] for _ in range(4)] self.set_dash_test_params(4, 3, fast_dip3_enforcement=True, extra_args=extra_args) def skip_test_if_missing_module(self): @@ -35,7 +35,7 @@ class MnehfTest(DashTestFramework): for inode in range(self.num_nodes): self.log.info(f"Restart node {inode} with {self.extra_args[inode]}") if params is not None: - self.extra_args[inode][0] = f"-vbparams=testdummy:{params[0]}:{params[1]}:12:12:12:5:0" + self.extra_args[inode][0] = f"-vbparams=testdummy:{params[0]}:{params[1]}:12:12:12:5:1" self.log.info(f"Actual restart options: {self.extra_args[inode]}") self.restart_node(0) diff --git a/test/functional/feature_new_quorum_type_activation.py b/test/functional/feature_new_quorum_type_activation.py index a255319b09..47eb0297f8 100755 --- a/test/functional/feature_new_quorum_type_activation.py +++ b/test/functional/feature_new_quorum_type_activation.py @@ -17,7 +17,7 @@ class NewQuorumTypeActivationTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-vbparams=testdummy:0:999999999999:10:8:6:5:-1"]] + self.extra_args = [["-vbparams=testdummy:0:999999999999:10:8:6:5:0"]] def run_test(self): self.log.info(get_bip9_details(self.nodes[0], 'testdummy')) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 1c16d6ce1b..d9d0d58c45 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -147,7 +147,7 @@ class BlockchainTest(BitcoinTestFramework): 'start_time': 0, 'timeout': 9223372036854775807, 'since': 0, - 'ehf': -1, + 'ehf': False, }, 'active': False}, 'mn_rr': { 'type': 'bip9', @@ -156,7 +156,7 @@ class BlockchainTest(BitcoinTestFramework): 'start_time': 0, 'timeout': 9223372036854775807, 'since': 0, - 'ehf': 0, + 'ehf': True, }, 'active': False}, 'testdummy': { @@ -174,7 +174,7 @@ class BlockchainTest(BitcoinTestFramework): 'count': 57, 'possible': True, }, - 'ehf': -1, + 'ehf': False, }, 'active': False}, }) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 3a17660984..db47f98b03 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1128,10 +1128,13 @@ class DashTestFramework(BitcoinTestFramework): def activate_mn_rr(self, expected_activation_height=None): self.nodes[0].sporkupdate("SPORK_24_EHF", 0) self.wait_for_sporks_same() - mn_rr_status = 0 - while mn_rr_status == 0: + mn_rr_height = 0 + while mn_rr_height == 0: time.sleep(1) - mn_rr_status = get_bip9_details(self.nodes[0], 'mn_rr')['ehf'] + try: + mn_rr_height = get_bip9_details(self.nodes[0], 'mn_rr')['ehf_height'] + except KeyError: + pass self.nodes[0].generate(1) self.sync_all() self.activate_by_name('mn_rr', expected_activation_height)