From 82c7db7db6ce59c5a268029c4c3160160388fe06 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 02:16:31 -0500 Subject: [PATCH 1/2] fix: defer Platform revival until block connection succeeds Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evo/deterministicmns.cpp | 47 ++++++------ src/evo/deterministicmns.h | 7 +- src/evo/specialtxman.cpp | 10 ++- src/evo/specialtxman.h | 3 + src/test/evo_deterministicmns_tests.cpp | 95 ++++++++++++++++++++++++- src/validation.cpp | 4 ++ 6 files changed, 143 insertions(+), 23 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 2d07f290ae6b..282b02a722f8 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -678,9 +678,9 @@ CDeterministicMNManager::CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaM CDeterministicMNManager::~CDeterministicMNManager() = default; -bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null pindex, - BlockValidationState& state, const CDeterministicMNList& oldList, - const CDeterministicMNList& newList, MNListUpdates& updatesRet) +bool CDeterministicMNManager::ProcessBlock(gsl::not_null pindex, BlockValidationState& state, + const CDeterministicMNList& oldList, const CDeterministicMNList& newList, + MNListUpdates& updatesRet) { AssertLockHeld(::cs_main); @@ -721,23 +721,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null(block.vtx.size()); i++) { - const CTransaction& tx = *block.vtx[i]; - if (!tx.IsSpecialTxVersion() || tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) { - // only interested in revive transactions - continue; - } - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) continue; // should not happen but does not matter - - if (!m_mn_metaman.ResetPlatformBan(opt_proTx->proTxHash, nHeight)) { - LogPrint(BCLog::LLMQ, "%s -- MN %s is failed to Platform revived at height %d\n", __func__, - opt_proTx->proTxHash.ToString(), nHeight); - } - } - diff.nHeight = pindex->nHeight; mnListDiffsCache.emplace(pindex->GetBlockHash(), diff); mnListsCache.insert_or_assign(newList.GetBlockHash(), newList); @@ -773,6 +756,30 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null(tx); + if (!pro_tx) continue; + if (!m_mn_metaman.ResetPlatformBan(pro_tx->proTxHash, index.nHeight)) { + LogPrint(BCLog::LLMQ, "%s -- MN %s is failed to Platform revived at height %d\n", __func__, + pro_tx->proTxHash.ToString(), index.nHeight); + } + } +} + +void CDeterministicMNManager::DiscardBlock(const uint256& block_hash) +{ + LOCK(cs); + mnListsCache.erase(block_hash); + mnListDiffsCache.erase(block_hash); +} + bool CDeterministicMNManager::UndoBlock(gsl::not_null pindex, MNListUpdates& updatesRet) { int nHeight = pindex->nHeight; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index b1e1104f0cdf..4646aa172a4d 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -788,11 +788,16 @@ class CDeterministicMNManager explicit CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaMan& mn_metaman); ~CDeterministicMNManager(); - bool ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, + bool ProcessBlock(gsl::not_null pindex, BlockValidationState& state, const CDeterministicMNList& oldList, const CDeterministicMNList& newList, MNListUpdates& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main); bool UndoBlock(gsl::not_null pindex, MNListUpdates& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs); + /** Apply local metadata updates only after successful active-chain connection. */ + void ResetPlatformBans(const CBlock& block, const CBlockIndex& index) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** Drop candidate state whose EvoDB transaction was rolled back. */ + void DiscardBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(!cs); + void UpdatedBlockTip(gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs); CDeterministicMNList GetListForBlock(gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs) { diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index d4f6e03e08d9..6d522b4e4413 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -901,6 +901,14 @@ uint256 CSpecialTxProcessor::GetDeterministicMNListHash(gsl::not_null pindex) { m_dmnman.UpdatedBlockTip(pindex); } +void CSpecialTxProcessor::ResetPlatformBans(const CBlock& block, const CBlockIndex& index) +{ + AssertLockHeld(::cs_main); + m_dmnman.ResetPlatformBans(block, index); +} + +void CSpecialTxProcessor::DiscardDeterministicMNBlock(const uint256& block_hash) { m_dmnman.DiscardBlock(block_hash); } + bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const CChain& chain, const CBlock& block, const CBlockIndex* pindex, SpecialTxRules rules, const CCoinsViewCache& view, CAmount blockSubsidy, bool fJustCheck, @@ -1021,7 +1029,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const } mn_list.SetBlockHash(pindex->GetBlockHash()); - if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, updatesRet.old_list, mn_list, updatesRet)) { + if (!fJustCheck && !m_dmnman.ProcessBlock(pindex, state, updatesRet.old_list, mn_list, updatesRet)) { // pass the state returned by the function above return false; } diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 05c66ce753df..3d904bf9e342 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -112,6 +112,9 @@ class CSpecialTxProcessor /** Passthrough to CDeterministicMNManager::UpdatedBlockTip */ void UpdatedMNListTip(gsl::not_null pindex); + void ResetPlatformBans(const CBlock& block, const CBlockIndex& index) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void DiscardDeterministicMNBlock(const uint256& block_hash); + private: friend class CMNListBuilder; diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 9cc1f4bf6dac..3453245cf620 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -20,9 +21,11 @@ #include #include #include -#include +#include #include #include +#include +#include #include #include #include @@ -4824,4 +4827,94 @@ BOOST_AUTO_TEST_CASE(migration_logic_validation) BOOST_CHECK_EQUAL(convertedDiff.state.pubKeyOperator.ToString(), legacyDiff.state.pubKeyOperator.ToString()); } +BOOST_AUTO_TEST_CASE(platform_ban_reset_requires_successful_connection) +{ + TestMNChainSetup setup{100, {"-dip3params=109:500"}}; + for (int i = 0; i < 20; ++i) + setup.ProcessBlock(); + auto& chainman = setup.chainman; + auto& dmnman = setup.dmnman; + auto& metaman = *setup.m_node.mn_metaman; + + CKey owner_key; + owner_key.MakeNewKey(true); + CBLSSecretKey operator_key; + operator_key.MakeNewKey(); + CProRegTx pro_reg; + pro_reg.nVersion = ProTxVersion::BasicBLS; + pro_reg.nType = MnType::Evo; + pro_reg.netInfo = NetInfoInterface::MakeNetInfo(pro_reg.nVersion); + BOOST_REQUIRE_EQUAL(pro_reg.netInfo->AddEntry(NetInfoPurpose::CORE_P2P, "1.1.1.1:1"), NetInfoStatus::Success); + pro_reg.keyIDOwner = owner_key.GetPubKey().GetID(); + pro_reg.keyIDVoting = pro_reg.keyIDOwner; + pro_reg.pubKeyOperator.Set(operator_key.GetPublicKey(), /*specificLegacyScheme=*/false); + pro_reg.scriptPayout = GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())); + pro_reg.collateralOutpoint.n = 0; + pro_reg.platformNodeID.SetHex("00112233445566778899aabbccddeeff00112233"); + pro_reg.platformP2PPort = 20001; + pro_reg.platformHTTPPort = 20002; + CMutableTransaction tx_reg; + tx_reg.nVersion = 3; + tx_reg.nType = TRANSACTION_PROVIDER_REGISTER; + const auto reg_spent = FundTransaction(chainman, tx_reg, setup.utxos, pro_reg.scriptPayout, + dmn_types::Evo.collat_amount); + pro_reg.inputsHash = CalcTxInputsHash(CTransaction(tx_reg)); + SetTxPayload(tx_reg, pro_reg); + SignTransaction(tx_reg, reg_spent, setup.coinbaseKey); + const uint256 protx_hash = tx_reg.GetHash(); + setup.ProcessBlock({tx_reg}); + BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(protx_hash)); + + PlatformBanMessage ban; + ban.m_protx_hash = protx_hash; + ban.m_requested_height = setup.Tip()->nHeight; + const auto ban_hash = ban.GetHash(); + BOOST_REQUIRE(metaman.SetPlatformBan(ban_hash, std::move(ban))); + const auto metadata_before = metaman.GetInfo(protx_hash); + + CProUpServTx pro_serv; + pro_serv.nVersion = pro_reg.nVersion; + pro_serv.nType = MnType::Evo; + pro_serv.proTxHash = protx_hash; + pro_serv.netInfo = NetInfoInterface::MakeNetInfo(pro_serv.nVersion); + BOOST_REQUIRE_EQUAL(pro_serv.netInfo->AddEntry(NetInfoPurpose::CORE_P2P, "1.1.1.1:2"), NetInfoStatus::Success); + pro_serv.platformNodeID = pro_reg.platformNodeID; + pro_serv.platformP2PPort = pro_reg.platformP2PPort; + pro_serv.platformHTTPPort = pro_reg.platformHTTPPort; + CMutableTransaction tx_serv; + tx_serv.nVersion = 3; + tx_serv.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE; + const auto serv_spent = FundTransaction(chainman, tx_serv, setup.utxos, pro_reg.scriptPayout, COIN); + pro_serv.inputsHash = CalcTxInputsHash(CTransaction(tx_serv)); + pro_serv.sig = operator_key.Sign(::SerializeHash(pro_serv), /*specificLegacyScheme=*/false); + SetTxPayload(tx_serv, pro_serv); + SignTransaction(tx_serv, serv_spent, setup.coinbaseKey); + const CBlock good = setup.CreateBlock({tx_serv}, setup.coinbase_pk, chainman.ActiveChainstate()); + CBlock rejected = good; + auto cbtx = GetTxPayload(*rejected.vtx[0]); + BOOST_REQUIRE(cbtx.has_value()); + BOOST_REQUIRE(cbtx->merkleRootMNList != uint256::ONE); + cbtx->merkleRootMNList = uint256::ONE; + CMutableTransaction coinbase{*rejected.vtx[0]}; + SetTxPayload(coinbase, *cbtx); + rejected.vtx[0] = MakeTransactionRef(coinbase); + rejected.hashMerkleRoot = BlockMerkleRoot(rejected); + while (!CheckProofOfWork(rejected.GetHash(), rejected.nBits, chainman.GetConsensus())) + ++rejected.nNonce; + + const auto tip_before = setup.Tip(); + chainman.ProcessNewBlock(std::make_shared(rejected), /*force_processing=*/true, nullptr); + BOOST_CHECK(setup.Tip() == tip_before); + BOOST_CHECK(metaman.IsPlatformBanned(protx_hash)); + BOOST_CHECK_EQUAL(metaman.GetInfo(protx_hash).m_platform_ban_updated, metadata_before.m_platform_ban_updated); + const auto rejected_index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(rejected.GetHash())); + BOOST_REQUIRE(rejected_index); + BOOST_REQUIRE(WITH_LOCK(cs_main, return rejected_index->nStatus & BLOCK_FAILED_VALID)); + + BOOST_REQUIRE(chainman.ProcessNewBlock(std::make_shared(good), /*force_processing=*/true, nullptr)); + BOOST_CHECK(setup.Tip()->GetBlockHash() == good.GetHash()); + BOOST_CHECK(!metaman.IsPlatformBanned(protx_hash)); + BOOST_CHECK_EQUAL(metaman.GetInfo(protx_hash).m_platform_ban_updated, setup.Tip()->nHeight); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index c1ddccaca04b..832bebb81d8b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3648,6 +3648,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, bool rv = ConnectBlock(blockConnecting, state, pindexNew, view); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { + m_special_tx.DiscardDeterministicMNBlock(pindexNew->GetBlockHash()); if (state.IsInvalid()) InvalidBlockFound(pindexNew, state); LogError("%s: ConnectBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString()); @@ -3693,6 +3694,9 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, // on legacy. if (this == &m_chainman.ActiveChainstate()) bls_scheme_guard.Commit(); UpdateTip(pindexNew); + if (this == &m_chainman.ActiveChainstate()) { + m_special_tx.ResetPlatformBans(blockConnecting, *pindexNew); + } const auto time_6{SteadyClock::now()}; time_post_connect += time_6 - time_5; From 99f5e80d5c1cbe1708097b260afb5ab9ea936a1d Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 03:43:44 -0500 Subject: [PATCH 2/2] fix: roll back a rejected block's EvoDB writes before evicting its caches Another thread can read the open EvoDB transaction. Between evicting a rejected candidate's masternode-list caches and the transaction rollback in the destructor, a concurrent lookup such as protx info could read the candidate's diff and cache it again, leaving rejected state cached after the rollback. Roll back first, then evict under the manager lock. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/test/evo_deterministicmns_tests.cpp | 5 +++++ src/validation.cpp | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 3453245cf620..327e1dec353f 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -4910,6 +4910,11 @@ BOOST_AUTO_TEST_CASE(platform_ban_reset_requires_successful_connection) const auto rejected_index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(rejected.GetHash())); BOOST_REQUIRE(rejected_index); BOOST_REQUIRE(WITH_LOCK(cs_main, return rejected_index->nStatus & BLOCK_FAILED_VALID)); + const auto rejected_list = dmnman.GetListForBlock(rejected_index); + const auto rejected_dmn = rejected_list.GetMN(protx_hash); + BOOST_CHECK(!rejected_dmn || rejected_dmn->pdmnState->netInfo->GetPrimary().GetPort() != 2); + CDeterministicMNManager fresh_dmnman{*Assert(setup.m_node.evodb), metaman}; + BOOST_CHECK(rejected_list == fresh_dmnman.GetListForBlock(rejected_index)); BOOST_REQUIRE(chainman.ProcessNewBlock(std::make_shared(good), /*force_processing=*/true, nullptr)); BOOST_CHECK(setup.Tip()->GetBlockHash() == good.GetHash()); diff --git a/src/validation.cpp b/src/validation.cpp index 832bebb81d8b..32e8ce9a75d4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3648,6 +3648,9 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, bool rv = ConnectBlock(blockConnecting, state, pindexNew, view); GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { + // Other threads read the open transaction, so roll it back before evicting the + // candidate or a concurrent lookup could cache it again. + dbTx->Rollback(); m_special_tx.DiscardDeterministicMNBlock(pindexNew->GetBlockHash()); if (state.IsInvalid()) InvalidBlockFound(pindexNew, state);