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