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..327e1dec353f 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,99 @@ 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)); + 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()); + 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..32e8ce9a75d4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3648,6 +3648,10 @@ 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); LogError("%s: ConnectBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString()); @@ -3693,6 +3697,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;