Skip to content

fix: defer Platform revival until block connection succeeds #7835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PastaPastaPasta wants to merge 2 commits into
base: develop
Choose a base branch
from
+151 −23
Open

fix: defer Platform revival until block connection succeeds #7835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82c7db7
fix: defer Platform revival until block connection succeeds
PastaPastaPasta Oct 7, 2026
99f5e80
fix: roll back a rejected block's EvoDB writes before evicting its ca…
PastaPastaPasta Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 27 additions & 20 deletions src/evo/deterministicmns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,9 +678,9 @@ CDeterministicMNManager::CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaM

CDeterministicMNManager::~CDeterministicMNManager() = default;

bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex,
BlockValidationState& state, const CDeterministicMNList& oldList,
const CDeterministicMNList& newList, MNListUpdates& updatesRet)
bool CDeterministicMNManager::ProcessBlock(gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,
const CDeterministicMNList& oldList, const CDeterministicMNList& newList,
MNListUpdates& updatesRet)
{
AssertLockHeld(::cs_main);

Expand Down Expand Up @@ -721,23 +721,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
__func__, nHeight, newList.GetCounts().total());
}

// apply platform unban for platform revive too, after all persistent
// payload checks have succeeded
for (int i = 1; i < static_cast<int>(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<CProUpServTx>(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);
Expand Down Expand Up @@ -773,6 +756,30 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
return true;
}

void CDeterministicMNManager::ResetPlatformBans(const CBlock& block, const CBlockIndex& index)
{
AssertLockHeld(::cs_main);
if (!DeploymentActiveAt(index, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003)) return;

for (size_t i = 1; i < block.vtx.size(); ++i) {
const CTransaction& tx = *block.vtx[i];
if (!tx.IsSpecialTxVersion() || tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) continue;
const auto pro_tx = GetTxPayload<CProUpServTx>(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<const CBlockIndex*> pindex, MNListUpdates& updatesRet)
{
int nHeight = pindex->nHeight;
Expand Down
7 changes: 6 additions & 1 deletion src/evo/deterministicmns.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,11 +788,16 @@ class CDeterministicMNManager
explicit CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaMan& mn_metaman);
~CDeterministicMNManager();

bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,
bool ProcessBlock(gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,
const CDeterministicMNList& oldList, const CDeterministicMNList& newList, MNListUpdates& updatesRet)
EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
bool UndoBlock(gsl::not_null<const CBlockIndex*> 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<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs);

CDeterministicMNList GetListForBlock(gsl::not_null<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs) {
Expand Down
10 changes: 9 additions & 1 deletion src/evo/specialtxman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,14 @@ uint256 CSpecialTxProcessor::GetDeterministicMNListHash(gsl::not_null<const CBlo

void CSpecialTxProcessor::UpdatedMNListTip(gsl::not_null<const CBlockIndex*> 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,
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions src/evo/specialtxman.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ class CSpecialTxProcessor
/** Passthrough to CDeterministicMNManager::UpdatedBlockTip */
void UpdatedMNListTip(gsl::not_null<const CBlockIndex*> pindex);

void ResetPlatformBans(const CBlock& block, const CBlockIndex& index) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
void DiscardDeterministicMNBlock(const uint256& block_hash);

private:
friend class CMNListBuilder;

Expand Down
100 changes: 99 additions & 1 deletion src/test/evo_deterministicmns_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <consensus/validation.h>
#include <deploymentstatus.h>
#include <evo/assetlocktx.h>
#include <evo/cbtx.h>
#include <evo/deterministicmns.h>
#include <evo/evodb.h>
#include <evo/providertx.h>
Expand All @@ -20,9 +21,11 @@
#include <evo/specialtxman.h>
#include <interfaces/node.h>
#include <llmq/context.h>
#include <node/mempool_args.h>
#include <masternode/meta.h>
#include <messagesigner.h>
#include <netbase.h>
#include <node/blockstorage.h>
#include <node/mempool_args.h>
#include <node/miner.h>
#include <policy/policy.h>
#include <pow.h>
Expand Down Expand Up @@ -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<CCbTx>(*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<CBlock>(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<CBlock>(good), /*force_processing=*/true, nullptr));
Comment on lines +4910 to +4919

@thepastaclaw thepastaclaw Oct 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved at 99f5e80d; see the replies below.

🟡 Suggestion: Cover rejected-candidate state through the public list lookup

The fixture checks rejection and unchanged Platform metadata, then connects the valid counterpart under a different block hash. It never calls GetListForBlock(rejected_index), so removing DiscardDeterministicMNBlock() would leave the rejected candidate's list and diff cached without any current assertion observing that stale state. Before connecting the valid counterpart, add a public list lookup that verifies the rejected service update is not returned and compares the result with a fresh CDeterministicMNManager backed by the same rolled-back EvoDB. This directly covers the cache-consistency behavior introduced by this PR without accessing private database keys or requiring a concurrent reproduction, and restores the comparison currently claimed in the PR description.

source: gpt-6.1-sol (phase2-reviewer: general)

@PastaPastaPasta PastaPastaPasta Oct 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in the second commit (fix: roll back a rejected block's EvoDB writes before evicting its caches). Right after the rejected block is marked BLOCK_FAILED_VALID, and before the valid counterpart is connected, the fixture now:

  • calls dmnman.GetListForBlock(rejected_index) and checks the returned list does not contain the candidate's service update (the MN is either absent or its primary port is not the updated :2);
  • builds a fresh CDeterministicMNManager on the same rolled-back EvoDB and checks GetListForBlock(rejected_index) returns an identical list (operator==).

It only goes through public API: no private DB keys and no new test case.

Mutation check: with m_special_tx.DiscardDeterministicMNBlock(...) temporarily removed from ConnectTip, both new assertions fail, because the live manager keeps returning the cached candidate list with the service update:

evo_deterministicmns_tests.cpp:4915: error: check !rejected_dmn || ...GetPrimary().GetPort() != 2 has failed
evo_deterministicmns_tests.cpp:4917: error: check rejected_list == fresh_dmnman.GetListForBlock(rejected_index) has failed
*** 2 failures are detected

With the eviction restored, the case passes.

🤖 Posted autonomously by Claude on behalf of pasta.

@thepastaclaw thepastaclaw Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved (re-reviewed at 99f5e80d): The fixture now performs the public rejected-block lookup before connecting the valid counterpart, verifies the service update is absent, and compares the result with a fresh manager using the same rolled-back EvoDB.

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()
7 changes: 7 additions & 0 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

const auto time_6{SteadyClock::now()};
time_post_connect += time_6 - time_5;
Expand Down
Loading