From 537ef62a107198d47640ffaf853ced0a853884ab Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 23 Aug 2026 12:01:12 -0500 Subject: [PATCH] fix(mempool): exempt a MN's own ProRegTx from collateral-reuse conflicts The collateral-reuse conflict checks added in dash#7489 compare only map presence, not identity. While a reorg is being processed, dmnman's tip list is not yet rolled back (SynchronousUpdatedBlockTip fires after MaybeUpdateMempoolForReorg), so a disconnected ProRegTx being resubmitted still resolves via GetMNByCollateral() to the masternode it itself created. With an update for that MN pending in the mempool, the resubmission was rejected as protx-dup and dropped for good, even though registration and update are mineable together in that order. Treat the masternode's own registration (same hash) as not-a-replacement in both directions: the ProRegTx branch skips the mapProTxRefs lookup when the resolved MN's proTxHash equals the incoming tx hash, and the three update branches now share a collateralReusedInMempool() helper that requires the pending ProRegTx to be a different transaction, mirroring the it->second != proTxHash form the neighbouring checks already use. Co-Authored-By: Claude Fable 5 --- src/test/evo_deterministicmns_tests.cpp | 28 +++++++++++++++++++++ src/txmempool.cpp | 33 +++++++++++++++---------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index e76b07e6059c..641942bde046 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1521,6 +1521,34 @@ void FuncTestMempoolProRegReplacementUpdateConflict(TestChainSetup& setup) MakeTransactionRef(tx_reg_replace)}; testPool.removeForBlock(connected, tip_height() + 1); BOOST_CHECK_EQUAL(testPool.size(), 0U); + + // ProUpReg and ProUpRev for the replaced MN conflict with a pending replacement too. + CKey votingKey; + votingKey.MakeNewKey(true); + CBLSSecretKey operatorKey3; + operatorKey3.MakeNewKey(); + auto tx_up_reg = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey3.GetPublicKey(), + votingKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + auto tx_up_rev = CreateProUpRevTx(chainman, utxos, proTxHash, operatorKey, setup.coinbaseKey); + + testPool.addUnchecked(entry.FromTx(tx_reg_replace)); + BOOST_CHECK(testPool.existsProviderTxConflict(CTransaction(tx_up_reg))); + BOOST_CHECK(testPool.existsProviderTxConflict(CTransaction(tx_up_rev))); + testPool.removeRecursive(CTransaction(tx_reg_replace), MemPoolRemovalReason::MANUAL); + BOOST_CHECK_EQUAL(testPool.size(), 0U); + + // The MN's own ProRegTx is not a replacement. While a reorg is being processed the + // tip list can still contain the MN whose registration is being resubmitted; if + // either direction were treated as a conflict, the resubmitted registration (or its + // updates) would be dropped even though the pair is mineable together. + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + BOOST_CHECK(!testPool.existsProviderTxConflict(CTransaction(tx_reg))); + testPool.removeRecursive(CTransaction(tx_up_serv), MemPoolRemovalReason::MANUAL); + + testPool.addUnchecked(entry.FromTx(tx_reg)); + BOOST_CHECK(!testPool.existsProviderTxConflict(CTransaction(tx_up_serv))); + BOOST_CHECK(!testPool.existsProviderTxConflict(CTransaction(tx_up_reg))); + BOOST_CHECK(!testPool.existsProviderTxConflict(CTransaction(tx_up_rev))); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index a7754ea05052..d515b187cfa7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1431,6 +1431,16 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { return false; }; + // A pending ProRegTx claiming a live MN's collateral replaces (deletes) that MN when + // mined, so an update targeting the MN could never be mined afterwards. The MN's own + // ProRegTx (same hash, e.g. resubmitted while a reorg is being processed) replaces + // nothing and is not a conflict. + auto collateralReusedInMempool = [&](const CDeterministicMN& dmn) EXCLUSIVE_LOCKS_REQUIRED(cs) { + AssertLockHeld(cs); + auto it = mapProTxCollaterals.find(dmn.collateralOutpoint); + return it != mapProTxCollaterals.end() && it->second != dmn.proTxHash; + }; + const uint256 tx_hash{tx.GetHash()}; if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { const auto opt_proTx = GetTxPayload(tx); @@ -1462,10 +1472,12 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { // A replacement ProRegTx deletes the live MN backed by this collateral. Any // in-mempool update that still targets that MN's proTxHash would then fail // BuildNewListFromBlock with bad-protx-hash if both were mined in one block. - if (auto dmn = m_dmnman.GetListAtChainTip().GetMNByCollateral(proTx.collateralOutpoint)) { - if (mapProTxRefs.find(dmn->proTxHash) != mapProTxRefs.end()) { - return true; - } + // Exempt the MN's own registration: while a reorg is being processed the tip + // list can still contain the MN whose ProRegTx is being resubmitted, and + // re-registering the same MN replaces nothing. + if (auto dmn = m_dmnman.GetListAtChainTip().GetMNByCollateral(proTx.collateralOutpoint); + dmn && dmn->proTxHash != tx_hash && mapProTxRefs.count(dmn->proTxHash)) { + return true; } } return false; @@ -1487,11 +1499,8 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { return true; } } - // Conflict with a replacement ProRegTx that reuses this MN's external collateral. - if (auto dmn = m_dmnman.GetListAtChainTip().GetMN(opt_proTx->proTxHash)) { - if (mapProTxCollaterals.count(dmn->collateralOutpoint)) { - return true; - } + if (auto dmn = m_dmnman.GetListAtChainTip().GetMN(opt_proTx->proTxHash); dmn && collateralReusedInMempool(*dmn)) { + return true; } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { const auto opt_proTx = GetTxPayload(tx); @@ -1507,8 +1516,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } - // Conflict with a replacement ProRegTx that reuses this MN's external collateral. - if (mapProTxCollaterals.count(dmn->collateralOutpoint)) { + if (collateralReusedInMempool(*dmn)) { return true; } // only allow one operator key change in the mempool @@ -1533,8 +1541,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } - // Conflict with a replacement ProRegTx that reuses this MN's external collateral. - if (mapProTxCollaterals.count(dmn->collateralOutpoint)) { + if (collateralReusedInMempool(*dmn)) { return true; } // only allow one operator key change in the mempool