diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index af286a51f148..b251b9272f19 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -990,6 +990,45 @@ BOOST_AUTO_TEST_CASE(v19_activation_legacy) FuncV19Activation(setup); } +BOOST_AUTO_TEST_CASE(v19_boundary_validation_failure_restores_bls_scheme) +{ + TestChainV19Setup setup; + auto& chainman = *Assert(setup.m_node.chainman.get()); + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + + BOOST_REQUIRE(!DeploymentActiveAt(*chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + BOOST_REQUIRE(DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)); + struct ScopedBLSLegacySchemeRestore { + explicit ScopedBLSLegacySchemeRestore(bool saved_scheme) : m_saved_scheme(saved_scheme) {} + ~ScopedBLSLegacySchemeRestore() { bls::bls_legacy_scheme.store(m_saved_scheme); } + bool m_saved_scheme; + } bls_scheme_restore{bls::bls_legacy_scheme.load()}; + + CMutableTransaction bad_tx; + bad_tx.nVersion = 1; + bad_tx.vin.emplace_back(COutPoint(uint256::ONE, 0)); + bad_tx.vout.emplace_back(1 * COIN, CScript{} << OP_TRUE); + + bls::bls_legacy_scheme.store(true); + + CBlock proposal_block = setup.CreateBlock({bad_tx}, coinbase_pk, chainman.ActiveChainstate()); + { + LOCK(cs_main); + BlockValidationState state; + BOOST_CHECK(!TestBlockValidity(state, *Assert(setup.m_node.chainlocks), *Assert(setup.m_node.evodb), Params(), + chainman.ActiveChainstate(), proposal_block, chainman.ActiveChain().Tip(), + /*fCheckPOW=*/true, /*fCheckMerkleRoot=*/true)); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-txns-inputs-missingorspent"); + } + BOOST_CHECK(bls::bls_legacy_scheme.load()); + + CBlock connect_block = setup.CreateBlock({bad_tx}, coinbase_pk, chainman.ActiveChainstate()); + const int height_before_invalid_block{chainman.ActiveChain().Height()}; + (void)chainman.ProcessNewBlock(std::make_shared(connect_block), /*force_processing=*/true, /*new_block=*/nullptr); + BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), height_before_invalid_block); + BOOST_CHECK(bls::bls_legacy_scheme.load()); +} + BOOST_AUTO_TEST_CASE(dip3_protx_legacy) { TestChainDIP3Setup setup; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 1525d645c196..e82f488603aa 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -362,4 +362,5 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) rpc_thread.join(); } } + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 9b99d0168343..c2e364b0f16b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -140,6 +141,40 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe } // namespace node using node::GetTransaction; +namespace { + +void SetBLSLegacyScheme(bool legacy_scheme, const char* logging_context) noexcept +{ + if (legacy_scheme != bls::bls_legacy_scheme.load()) { + bls::bls_legacy_scheme.store(legacy_scheme); + LogPrintf("%s: bls_legacy_scheme=%d\n", logging_context, bls::bls_legacy_scheme.load()); + } +} + +class ScopedBLSLegacyScheme +{ +public: + explicit ScopedBLSLegacyScheme(const char* logging_context) noexcept : + m_logging_context(logging_context), + m_saved_scheme(bls::bls_legacy_scheme.load()) + { + } + + ScopedBLSLegacyScheme(const ScopedBLSLegacyScheme&) = delete; + ScopedBLSLegacyScheme& operator=(const ScopedBLSLegacyScheme&) = delete; + + ~ScopedBLSLegacyScheme() noexcept + { + SetBLSLegacyScheme(m_saved_scheme, m_logging_context); + } + +private: + const char* const m_logging_context; + const bool m_saved_scheme; +}; + +} // namespace + const CBlockIndex* CChainState::FindForkInGlobalIndex(const CBlockLocator& locator) const { AssertLockHeld(cs_main); @@ -2181,7 +2216,7 @@ static int64_t nBlocksTotal = 0; * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, - CCoinsViewCache& view, bool fJustCheck) + CCoinsViewCache& view, bool fJustCheck, bool* effective_bls_legacy_scheme) { int64_t nTimeStart = GetTimeMicros(); @@ -2193,6 +2228,8 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, assert(m_chain_helper); + ScopedBLSLegacyScheme bls_scheme_guard{__func__}; + // Check it again in case a previous version let a bad block in // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or // ContextualCheckBlockHeader() here. This means that if we add a new @@ -2551,6 +2588,9 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, nTime8 - nTimeStart // in microseconds (µs) ); + if (effective_bls_legacy_scheme) { + *effective_bls_legacy_scheme = bls::bls_legacy_scheme.load(); + } return true; } @@ -3011,11 +3051,12 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew // When adding aggregate statistics in the future, keep in mind that // nBlocksTotal may be zero until the ConnectBlock() call below. LogPrint(BCLog::BENCHMARK, " - Load block from disk: %.2fms\n", (nTime2 - nTime1) * MILLI); + bool effective_bls_legacy_scheme{bls::bls_legacy_scheme.load()}; { auto dbTx = m_evoDb.BeginTransaction(); CCoinsViewCache view(&CoinsTip()); - bool rv = ConnectBlock(blockConnecting, state, pindexNew, view); + bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, /*fJustCheck=*/false, &effective_bls_legacy_scheme); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { if (state.IsInvalid()) @@ -3045,6 +3086,7 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew } // Update m_chain & related variables. m_chain.SetTip(*pindexNew); + SetBLSLegacyScheme(effective_bls_legacy_scheme, __func__); UpdateTip(pindexNew); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; @@ -4338,10 +4380,7 @@ bool TestBlockValidity(BlockValidationState& state, AssertLockHeld(cs_main); assert(pindexPrev && pindexPrev == chainstate.m_chain.Tip()); - // TODO: instead restoring bls_legacy_scheme better to keep it unchanged - // Moreover, current implementation is working incorrect if current function - // will return value too early due to error: old value won't be restored - auto bls_legacy_scheme = bls::bls_legacy_scheme.load(); + ScopedBLSLegacyScheme bls_scheme_guard{__func__}; uint256 hash = block.GetHash(); if (chainlocks.HasConflictingChainLock(pindexPrev->nHeight + 1, hash)) { @@ -4371,12 +4410,6 @@ bool TestBlockValidity(BlockValidationState& state, assert(state.IsValid()); - // we could switch to another scheme while testing, switch back to the original one - if (bls_legacy_scheme != bls::bls_legacy_scheme.load()) { - bls::bls_legacy_scheme.store(bls_legacy_scheme); - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); - } - return true; } @@ -4449,6 +4482,8 @@ bool CVerifyDB::VerifyDB( return true; } + ScopedBLSLegacyScheme bls_scheme_guard{__func__}; + // begin tx and let it rollback auto dbTx = evoDb.BeginTransaction(); @@ -4538,6 +4573,7 @@ bool CVerifyDB::VerifyDB( // check level 4: try reconnecting blocks if (nCheckLevel >= 4 && !skipped_l3_checks) { + bool effective_bls_legacy_scheme{bls::bls_legacy_scheme.load()}; while (pindex != chainstate.m_chain.Tip()) { const int percentageDone = std::max(1, std::min(99, 100 - (int)(((double)(chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * 50))); if (reportDone < percentageDone / 10) { @@ -4550,8 +4586,9 @@ bool CVerifyDB::VerifyDB( CBlock block; if (!ReadBlockFromDisk(block, pindex, consensus_params)) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); - if (!chainstate.ConnectBlock(block, state, pindex, coins)) + if (!chainstate.ConnectBlock(block, state, pindex, coins, /*fJustCheck=*/false, &effective_bls_legacy_scheme)) return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + SetBLSLegacyScheme(effective_bls_legacy_scheme, __func__); if (ShutdownRequested()) return true; } } diff --git a/src/validation.h b/src/validation.h index 5d9d09061d6c..314c9b2f7b6e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -709,7 +709,7 @@ class CChainState // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck = false, bool* effective_bls_legacy_scheme = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);