From daccd7eda8e0c1671c2322cd269fe84c5210e4df Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 30 Sep 2026 22:43:34 +0300 Subject: [PATCH 01/11] fix(mempool): evict a pending ProRegTx once a block spends its collateral removeProTxSpentCollateralConflicts() found the pending ProRegTx that references a spent external collateral but only removed the transactions referring to it. The ProRegTx itself stayed in the mempool although it can never be mined anymore (bad-protx-collateral). InstantSend-locked transactions never expire from the mempool, and a lock is only dropped when its transaction leaves the mempool, so such a ProRegTx kept its lock forever and its fee inputs could not be spent again. Remove it as a conflict, which also drops its lock via TransactionRemovedFromMempool. Co-Authored-By: Claude Opus 5.5 --- src/txmempool.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 37c1eb10dedc..2a9521815268 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1052,8 +1052,12 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) for (const auto& in : tx.vin) { auto collateralIt = mapProTxCollaterals.find(in.prevout); if (collateralIt != mapProTxCollaterals.end()) { - // These are not yet mined ProRegTxs - removeProTxReferences(collateralIt->second); + // A not yet mined ProRegTx whose collateral is now spent, and TXs referring to it + const uint256 proRegTxHash{collateralIt->second}; + if (auto it = mapTx.find(proRegTxHash); it != mapTx.end()) { + removeRecursive(it->GetTx(), MemPoolRemovalReason::CONFLICT); + } + removeProTxReferences(proRegTxHash); } auto dmn = mnList.GetMNByCollateral(in.prevout); if (dmn) { From 870ddeb778cd0781c06e687d78ac188d741a7857 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 1 Oct 2026 07:38:36 +0300 Subject: [PATCH 02/11] test: a pending ProRegTx leaves the mempool once a block spends its collateral A pending ProRegTx with an external collateral survives a block spending an unrelated output, and is evicted once a block spends its collateral. Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 62 +++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 625a0c9eabf3..ec3431637b3e 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1540,6 +1540,62 @@ void FuncTestMempoolProRegReplacementUpdateConflict(TestChainSetup& setup) } } +// A pending ProRegTx that references an external collateral can never be mined once a block +// spends that collateral. It must leave the mempool then: nothing else evicts it, and an +// InstantSend-locked transaction does not expire, which would leave its inputs frozen. +void FuncTestMempoolProRegSpentCollateral(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + + CKey ownerKey; + CKey payoutKey; + CKey collateralKey; + CBLSSecretKey operatorKey; + ownerKey.MakeNewKey(true); + payoutKey.MakeNewKey(true); + collateralKey.MakeNewKey(true); + operatorKey.MakeNewKey(); + + auto scriptPayout = GetScriptForDestination(PKHash(payoutKey.GetPubKey())); + auto scriptCollateral = GetScriptForDestination(PKHash(collateralKey.GetPubKey())); + + auto tx_collateral = CreateSpendTx(chainman, utxos, scriptCollateral, dmn_types::Regular.collat_amount, + setup.coinbaseKey); + auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + setup.m_node.dmnman->UpdatedBlockTip(tip_index()); + + const auto collateralOutpoint = GetCollateralOutpoint(tx_collateral); + auto tx_reg = CreateProRegTxExternalCollateral(chainman, utxos, /*port=*/1, collateralOutpoint, scriptPayout, + ownerKey, operatorKey, collateralKey, setup.coinbaseKey); + + CMutableTransaction tx_unrelated; + tx_unrelated.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); + tx_unrelated.vout.emplace_back(0, CScript() << OP_RETURN); + + CMutableTransaction tx_spend; + tx_spend.vin.emplace_back(collateralOutpoint); + tx_spend.vout.emplace_back(0, CScript() << OP_RETURN); + + CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; + TestMemPoolEntryHelper entry; + LOCK2(cs_main, testPool.cs); + + testPool.addUnchecked(entry.FromTx(tx_reg)); + BOOST_CHECK_EQUAL(testPool.size(), 1U); + + testPool.removeForBlock({MakeTransactionRef(tx_unrelated)}, tip_height() + 1); + BOOST_CHECK(testPool.exists(tx_reg.GetHash())); + + testPool.removeForBlock({MakeTransactionRef(tx_spend)}, tip_height() + 1); + BOOST_CHECK(!testPool.exists(tx_reg.GetHash())); +} + void FuncVerifyDB(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); @@ -3489,6 +3545,12 @@ BOOST_AUTO_TEST_CASE(test_mempool_proreg_replacement_update_conflict) FuncTestMempoolProRegReplacementUpdateConflict(setup); } +BOOST_AUTO_TEST_CASE(test_mempool_proreg_spent_collateral) +{ + TestChainV19Setup setup; + FuncTestMempoolProRegSpentCollateral(setup); +} + //This one can be started only with legacy scheme, since inside undo block will switch it back to legacy resulting into an inconsistency BOOST_AUTO_TEST_CASE(verify_db_legacy) { From a0b562ca1ee4a6ab252e564b6ae4a9210762286f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 30 Sep 2026 23:05:40 +0300 Subject: [PATCH 03/11] fix(mempool): keep a pending ProUpServTx across a registrar update that keeps the operator key A ProUpServTx is signed by the operator key, so a confirmed ProUpRegTx only invalidates it when it changes that key. removeProTxKeyChangedConflicts() decides this by comparing each pending update's validForProTxKey with the new key, but addUncheckedProTx() only set validForProTxKey for ProUpRegTx and ProUpRevTx. A ProUpServTx kept the null default and was evicted by every confirmed ProUpRegTx of its masternode, dropping a valid transaction and its InstantSend lock. Record the operator key a ProUpServTx is valid for when it enters the mempool, as for the other updates. Co-Authored-By: Claude Opus 5.5 --- src/txmempool.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 2a9521815268..415938038e92 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -643,6 +643,10 @@ void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, con } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx_hash); + // Without a masternode to take the key from, any registrar update evicts it + if (auto dmn = m_dmnman.GetListAtChainTip().GetMN(proTx.proTxHash)) { + newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator); + } for (const auto& entry : proTx.netInfo->GetEntries()) { mapProTxAddresses.emplace(entry, tx_hash); } From 7005c5b3a79c79521670432f40c6ff867c6d002f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 1 Oct 2026 07:38:37 +0300 Subject: [PATCH 04/11] test: a pending ProUpServTx survives a registrar update that keeps the operator key A confirmed ProUpRegTx that keeps the operator key leaves a pending ProUpServTx in the mempool, and one that changes the key evicts it. Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 51 +++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index ec3431637b3e..82d008b92a59 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1596,6 +1596,51 @@ void FuncTestMempoolProRegSpentCollateral(TestChainSetup& setup) BOOST_CHECK(!testPool.exists(tx_reg.GetHash())); } +// A pending ProUpServTx is signed by the operator key and stays valid across a registrar update +// that keeps that key, so only a registrar update that changes the key may evict it. +void FuncTestMempoolProUpServOperatorKeyChange(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript scriptPayout = GenerateRandomAddress(); + + CKey ownerKey; + CBLSSecretKey operatorKey; + auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); + const uint256 proTxHash = tx_reg.GetHash(); + auto block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(proTxHash)); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + + auto tx_up_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey); + auto tx_same_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + auto tx_new_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + + CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; + TestMemPoolEntryHelper entry; + LOCK2(cs_main, testPool.cs); + + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + BOOST_CHECK_EQUAL(testPool.size(), 1U); + + testPool.removeForBlock({MakeTransactionRef(tx_same_key)}, tip_height() + 1); + BOOST_CHECK(testPool.exists(tx_up_serv.GetHash())); + + testPool.removeForBlock({MakeTransactionRef(tx_new_key)}, tip_height() + 1); + BOOST_CHECK(!testPool.exists(tx_up_serv.GetHash())); +} + void FuncVerifyDB(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); @@ -3551,6 +3596,12 @@ BOOST_AUTO_TEST_CASE(test_mempool_proreg_spent_collateral) FuncTestMempoolProRegSpentCollateral(setup); } +BOOST_AUTO_TEST_CASE(test_mempool_proupserv_operator_key_change) +{ + TestChainV19Setup setup; + FuncTestMempoolProUpServOperatorKeyChange(setup); +} + //This one can be started only with legacy scheme, since inside undo block will switch it back to legacy resulting into an inconsistency BOOST_AUTO_TEST_CASE(verify_db_legacy) { From 9731e9117bbf5626a92da47e6845a95ee7a715c6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 30 Sep 2026 22:53:59 +0300 Subject: [PATCH 05/11] test: pending updates leave the mempool once a block spends the masternode collateral Cover the registered-masternode branch of removeProTxSpentCollateralConflicts(): a pending ProUpServTx and ProUpRegTx survive a block spending an unrelated output, and both are evicted once a block spends the masternode collateral. Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 74 +++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 82d008b92a59..b133b2902f61 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1596,6 +1596,74 @@ void FuncTestMempoolProRegSpentCollateral(TestChainSetup& setup) BOOST_CHECK(!testPool.exists(tx_reg.GetHash())); } +// Pending updates of a registered masternode can never be mined once a block spends its +// collateral, since the masternode is gone. They must leave the mempool then. +void FuncTestMempoolProUpdatesSpentCollateral(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + + CKey ownerKey; + CKey payoutKey; + CKey collateralKey; + CBLSSecretKey operatorKey; + ownerKey.MakeNewKey(true); + payoutKey.MakeNewKey(true); + collateralKey.MakeNewKey(true); + operatorKey.MakeNewKey(); + + auto scriptPayout = GetScriptForDestination(PKHash(payoutKey.GetPubKey())); + auto scriptCollateral = GetScriptForDestination(PKHash(collateralKey.GetPubKey())); + + auto tx_collateral = CreateSpendTx(chainman, utxos, scriptCollateral, dmn_types::Regular.collat_amount, + setup.coinbaseKey); + auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + const auto collateralOutpoint = GetCollateralOutpoint(tx_collateral); + auto tx_reg = CreateProRegTxExternalCollateral(chainman, utxos, /*port=*/1, collateralOutpoint, scriptPayout, + ownerKey, operatorKey, collateralKey, setup.coinbaseKey); + const uint256 proTxHash = tx_reg.GetHash(); + block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(proTxHash)); + + auto tx_up_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey); + auto tx_up_reg = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + + CMutableTransaction tx_unrelated; + tx_unrelated.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); + tx_unrelated.vout.emplace_back(0, CScript() << OP_RETURN); + + CMutableTransaction tx_spend; + tx_spend.vin.emplace_back(collateralOutpoint); + tx_spend.vout.emplace_back(0, CScript() << OP_RETURN); + + CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; + TestMemPoolEntryHelper entry; + LOCK2(cs_main, testPool.cs); + + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + testPool.addUnchecked(entry.FromTx(tx_up_reg)); + BOOST_CHECK_EQUAL(testPool.size(), 2U); + + testPool.removeForBlock({MakeTransactionRef(tx_unrelated)}, tip_height() + 1); + BOOST_CHECK(testPool.exists(tx_up_serv.GetHash())); + BOOST_CHECK(testPool.exists(tx_up_reg.GetHash())); + + testPool.removeForBlock({MakeTransactionRef(tx_spend)}, tip_height() + 1); + BOOST_CHECK(!testPool.exists(tx_up_serv.GetHash())); + BOOST_CHECK(!testPool.exists(tx_up_reg.GetHash())); +} + // A pending ProUpServTx is signed by the operator key and stays valid across a registrar update // that keeps that key, so only a registrar update that changes the key may evict it. void FuncTestMempoolProUpServOperatorKeyChange(TestChainSetup& setup) @@ -3596,6 +3664,12 @@ BOOST_AUTO_TEST_CASE(test_mempool_proreg_spent_collateral) FuncTestMempoolProRegSpentCollateral(setup); } +BOOST_AUTO_TEST_CASE(test_mempool_proupdates_spent_collateral) +{ + TestChainV19Setup setup; + FuncTestMempoolProUpdatesSpentCollateral(setup); +} + BOOST_AUTO_TEST_CASE(test_mempool_proupserv_operator_key_change) { TestChainV19Setup setup; From e1e97039dbd401a937fc705490abeeecdcbb0c20 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 30 Sep 2026 23:05:28 +0300 Subject: [PATCH 06/11] test: pending ProUpRevTx also leaves the mempool once the collateral is spent Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index b133b2902f61..f5f05144e7ed 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1638,6 +1638,7 @@ void FuncTestMempoolProUpdatesSpentCollateral(TestChainSetup& setup) auto tx_up_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey); auto tx_up_reg = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + auto tx_up_rev = CreateProUpRevTx(chainman, utxos, proTxHash, operatorKey, setup.coinbaseKey); CMutableTransaction tx_unrelated; tx_unrelated.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); @@ -1653,15 +1654,18 @@ void FuncTestMempoolProUpdatesSpentCollateral(TestChainSetup& setup) testPool.addUnchecked(entry.FromTx(tx_up_serv)); testPool.addUnchecked(entry.FromTx(tx_up_reg)); - BOOST_CHECK_EQUAL(testPool.size(), 2U); + testPool.addUnchecked(entry.FromTx(tx_up_rev)); + BOOST_CHECK_EQUAL(testPool.size(), 3U); testPool.removeForBlock({MakeTransactionRef(tx_unrelated)}, tip_height() + 1); BOOST_CHECK(testPool.exists(tx_up_serv.GetHash())); BOOST_CHECK(testPool.exists(tx_up_reg.GetHash())); + BOOST_CHECK(testPool.exists(tx_up_rev.GetHash())); testPool.removeForBlock({MakeTransactionRef(tx_spend)}, tip_height() + 1); BOOST_CHECK(!testPool.exists(tx_up_serv.GetHash())); BOOST_CHECK(!testPool.exists(tx_up_reg.GetHash())); + BOOST_CHECK(!testPool.exists(tx_up_rev.GetHash())); } // A pending ProUpServTx is signed by the operator key and stays valid across a registrar update From 4767667746d8d64c57c82ddc6381631bb107241e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 30 Sep 2026 23:18:47 +0300 Subject: [PATCH 07/11] test: a confirmed dissolution evicts pending updates of the dissolved masternode A node holds a pending ProUpSharedRegTx and ProUpShareTx of a shared masternode while another miner, which never saw them, confirms a ProDisTx for it. Once the node syncs that block, the dissolution has spent the collateral and removed the masternode, so both updates must be gone from its mempool. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_masternode_shares.py | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/functional/feature_masternode_shares.py b/test/functional/feature_masternode_shares.py index 4acfdd83c1fa..378ddd80acbe 100755 --- a/test/functional/feature_masternode_shares.py +++ b/test/functional/feature_masternode_shares.py @@ -342,6 +342,40 @@ def test_voting_payee_conflict_eviction(self, node, protx_hash): assert_equal(confirmed_address, voting) assert pending_txid not in node.getrawmempool() + def test_dissolution_evicts_pending_updates(self, node): + self.log.info("A confirmed dissolution evicts pending updates of the dissolved masternode") + shares = [ + {"amount": 600 * COIN, "refundAddress": node.getnewaddress(), "ownerAddress": node.getnewaddress()}, + {"amount": 400 * COIN, "refundAddress": node.getnewaddress(), "ownerAddress": node.getnewaddress()}, + ] + protx_hash, _ = self.register_shared(node, shares, port_offset=8) + registrar_fee, share_fee = node.getnewaddress(), node.getnewaddress() + funding_txid = node.sendmany("", {registrar_fee: 1, share_fee: 1}) + node.syncwithvalidationinterfacequeue() + self.bump_mocktime(10 * 60 + 1) + funding_block = self.generate(node, 1, sync_fun=self.no_op)[0] + assert funding_txid in node.getblock(funding_block)["tx"] + + prepared = node.protx("shared_update_registrar_prepare", protx_hash, "", node.getnewaddress(), registrar_fee) + registrar_sigs = node.protx("shared_sign", prepared["tx"])["signatures"] + registrar = node.protx("shared_combine", prepared["tx"], registrar_sigs) + share_update = node.protx("shared_update_share", protx_hash, 0, node.getnewaddress(), share_fee, False) + dissolve = node.protx("shared_dissolve", protx_hash, 0, DISSOLVE_FEE, False) + self.sync_all() + self.disconnect_nodes(0, 1) + pending_txids = {node.sendrawtransaction(registrar), node.sendrawtransaction(share_update)} + assert pending_txids.issubset(node.getrawmempool()) + # Simulate another miner that never saw the pending updates confirming the dissolution. + other = self.nodes[1] + dissolve_txid = other.sendrawtransaction(dissolve) + self.bump_mocktime(10 * 60 + 1) + dissolve_block = self.generate(other, 1, sync_fun=self.no_op)[0] + assert dissolve_txid in other.getblock(dissolve_block)["tx"] + self.connect_nodes(0, 1) + self.sync_blocks() + assert_raises_rpc_error(None, None, node.protx, "info", protx_hash) + assert pending_txids.isdisjoint(node.getrawmempool()) + def test_separate_participant_wallets(self): self.log.info("Eight separate wallets fund and authorize a shared masternode") node = self.nodes[0] @@ -1164,6 +1198,7 @@ def submit_with_sigs(sigs): self.test_pending_registrar_update(node, protx_hash4) self.test_voting_payee_conflict_eviction(node, protx_hash4) + self.test_dissolution_evicts_pending_updates(node) self.log.info("Shared state survives a node restart") state_before_restart = node.protx("info", protx_hash4)["state"] From 51296891520be61b18d1fb8fb98b9b455f70cd77 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 6 Oct 2026 22:32:10 +0700 Subject: [PATCH 08/11] test: the eviction tests each rebuilt a fixture that an existing test already has: FuncTestMempoolProRegSpentCollateral repeated the setup of FuncTestMempoolReorg, FuncTestMempoolProUpdatesSpentCollateral and FuncTestMempoolProUpServOperatorKeyChange repeated the registered external-collateral masternode of FuncTestMempoolProRegReplacementUpdateConflict, and test_dissolution_evicts_pending_updates registered one more shared masternode and replayed the two-node "other miner" sequence. Check the same outcomes where the fixtures already are: - FuncTestMempoolReorg: a block spending the collateral of the pending ProRegTx evicts it together with the ProUpServTx referring to it; a block spending another output of the collateral tx evicts neither. This now runs for both BLS schemes. - FuncTestMempoolProRegReplacementUpdateConflict: pending ProUpServTx and ProUpRevTx survive a confirmed ProUpRegTx that keeps the operator key and are evicted by one that changes it; pending ProUpServTx, ProUpRegTx and ProUpRevTx are evicted by a spend of the collateral. - feature_masternode_shares.py: the dissolution reorg scenario submits a ProUpSharedRegTx and a ProUpShareTx while the dissolution block is disconnected, and expects both gone once it is reconnected. Also correct the comment in FuncTestMempoolProTxKeyChangedConflictChain: a ProUpServTx entering the mempool now looks its masternode up. --- src/test/evo_deterministicmns_tests.cpp | 231 ++++--------------- test/functional/feature_masternode_shares.py | 48 ++-- 2 files changed, 61 insertions(+), 218 deletions(-) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index f5f05144e7ed..8ca0e4a6c900 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1321,6 +1321,25 @@ void FuncTestMempoolReorg(TestChainSetup& setup) block_reorg.emplace_back(std::make_shared(tx_reg_ds)); testPool.removeForBlock(block_reorg, nHeight + 2); BOOST_CHECK_EQUAL(testPool.size(), 0U); + + // Check mempool as if the new block spent the collateral of ProRegTx instead: ProRegTx can never + // be mined then and has to go together with ProUpServ referring to it. Spending another output + // of the collateral tx invalidates neither + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + testPool.addUnchecked(entry.FromTx(tx_reg)); + BOOST_CHECK_EQUAL(testPool.size(), 2U); + + CMutableTransaction tx_spend_other; + tx_spend_other.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); + tx_spend_other.vout.emplace_back(0, CScript() << OP_RETURN); + testPool.removeForBlock({MakeTransactionRef(tx_spend_other)}, nHeight + 2); + BOOST_CHECK_EQUAL(testPool.size(), 2U); + + CMutableTransaction tx_spend_collateral; + tx_spend_collateral.vin.emplace_back(collateralOutpoint); + tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); + testPool.removeForBlock({MakeTransactionRef(tx_spend_collateral)}, nHeight + 2); + BOOST_CHECK_EQUAL(testPool.size(), 0U); } // Regression test: a ProUpRev/ProUpReg invalidates every pending ProTx of the same masternode, and @@ -1340,7 +1359,7 @@ void FuncTestMempoolProTxKeyChangedConflictChain(TestChainSetup& setup) CKey ownerKey; CBLSSecretKey operatorKey; // Only the resulting proTxHash matters here; the registration never has to be mined because - // none of the paths under test consult the masternode list for a ProUpServ payload. + // a pending ProUpServ of a masternode missing from the list is evicted by any operator key change. auto tx_reg = CreateProRegTx(chainman, utxos, 1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); const uint256 proTxHash = tx_reg.GetHash(); @@ -1538,179 +1557,43 @@ void FuncTestMempoolProRegReplacementUpdateConflict(TestChainSetup& setup) testPool.removeForBlock(connected, tip_height() + 1); BOOST_CHECK_EQUAL(testPool.size(), 0U); } -} - -// A pending ProRegTx that references an external collateral can never be mined once a block -// spends that collateral. It must leave the mempool then: nothing else evicts it, and an -// InstantSend-locked transaction does not expire, which would leave its inputs frozen. -void FuncTestMempoolProRegSpentCollateral(TestChainSetup& setup) -{ - auto& chainman = *Assert(setup.m_node.chainman.get()); - auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; - auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; - - const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); - - CKey ownerKey; - CKey payoutKey; - CKey collateralKey; - CBLSSecretKey operatorKey; - ownerKey.MakeNewKey(true); - payoutKey.MakeNewKey(true); - collateralKey.MakeNewKey(true); - operatorKey.MakeNewKey(); - - auto scriptPayout = GetScriptForDestination(PKHash(payoutKey.GetPubKey())); - auto scriptCollateral = GetScriptForDestination(PKHash(collateralKey.GetPubKey())); - auto tx_collateral = CreateSpendTx(chainman, utxos, scriptCollateral, dmn_types::Regular.collat_amount, - setup.coinbaseKey); - auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); - BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - setup.m_node.dmnman->UpdatedBlockTip(tip_index()); - - const auto collateralOutpoint = GetCollateralOutpoint(tx_collateral); - auto tx_reg = CreateProRegTxExternalCollateral(chainman, utxos, /*port=*/1, collateralOutpoint, scriptPayout, - ownerKey, operatorKey, collateralKey, setup.coinbaseKey); - - CMutableTransaction tx_unrelated; - tx_unrelated.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); - tx_unrelated.vout.emplace_back(0, CScript() << OP_RETURN); - - CMutableTransaction tx_spend; - tx_spend.vin.emplace_back(collateralOutpoint); - tx_spend.vout.emplace_back(0, CScript() << OP_RETURN); - - CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; - TestMemPoolEntryHelper entry; - LOCK2(cs_main, testPool.cs); - - testPool.addUnchecked(entry.FromTx(tx_reg)); - BOOST_CHECK_EQUAL(testPool.size(), 1U); - - testPool.removeForBlock({MakeTransactionRef(tx_unrelated)}, tip_height() + 1); - BOOST_CHECK(testPool.exists(tx_reg.GetHash())); - - testPool.removeForBlock({MakeTransactionRef(tx_spend)}, tip_height() + 1); - BOOST_CHECK(!testPool.exists(tx_reg.GetHash())); -} - -// Pending updates of a registered masternode can never be mined once a block spends its -// collateral, since the masternode is gone. They must leave the mempool then. -void FuncTestMempoolProUpdatesSpentCollateral(TestChainSetup& setup) -{ - auto& chainman = *Assert(setup.m_node.chainman.get()); - auto& dmnman = *Assert(setup.m_node.dmnman); - auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; - auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; - - const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); - - CKey ownerKey; - CKey payoutKey; - CKey collateralKey; - CBLSSecretKey operatorKey; - ownerKey.MakeNewKey(true); - payoutKey.MakeNewKey(true); - collateralKey.MakeNewKey(true); - operatorKey.MakeNewKey(); - - auto scriptPayout = GetScriptForDestination(PKHash(payoutKey.GetPubKey())); - auto scriptCollateral = GetScriptForDestination(PKHash(collateralKey.GetPubKey())); - - auto tx_collateral = CreateSpendTx(chainman, utxos, scriptCollateral, dmn_types::Regular.collat_amount, - setup.coinbaseKey); - auto block = std::make_shared(setup.CreateBlock({tx_collateral}, coinbase_pk, chainman.ActiveChainstate())); - BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(tip_index()); - - const auto collateralOutpoint = GetCollateralOutpoint(tx_collateral); - auto tx_reg = CreateProRegTxExternalCollateral(chainman, utxos, /*port=*/1, collateralOutpoint, scriptPayout, - ownerKey, operatorKey, collateralKey, setup.coinbaseKey); - const uint256 proTxHash = tx_reg.GetHash(); - block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); - BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(tip_index()); - BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(proTxHash)); - - auto tx_up_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey); + // The replacement is not the only confirmed transaction that can leave updates of the MN + // unmineable: so does a ProUpReg changing the operator key, and a spend of the collateral. auto tx_up_reg = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); + auto tx_up_reg_new_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey2.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); auto tx_up_rev = CreateProUpRevTx(chainman, utxos, proTxHash, operatorKey, setup.coinbaseKey); - CMutableTransaction tx_unrelated; - tx_unrelated.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); - tx_unrelated.vout.emplace_back(0, CScript() << OP_RETURN); - - CMutableTransaction tx_spend; - tx_spend.vin.emplace_back(collateralOutpoint); - tx_spend.vout.emplace_back(0, CScript() << OP_RETURN); - - CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; - TestMemPoolEntryHelper entry; - LOCK2(cs_main, testPool.cs); - - testPool.addUnchecked(entry.FromTx(tx_up_serv)); - testPool.addUnchecked(entry.FromTx(tx_up_reg)); - testPool.addUnchecked(entry.FromTx(tx_up_rev)); - BOOST_CHECK_EQUAL(testPool.size(), 3U); - - testPool.removeForBlock({MakeTransactionRef(tx_unrelated)}, tip_height() + 1); - BOOST_CHECK(testPool.exists(tx_up_serv.GetHash())); - BOOST_CHECK(testPool.exists(tx_up_reg.GetHash())); - BOOST_CHECK(testPool.exists(tx_up_rev.GetHash())); - - testPool.removeForBlock({MakeTransactionRef(tx_spend)}, tip_height() + 1); - BOOST_CHECK(!testPool.exists(tx_up_serv.GetHash())); - BOOST_CHECK(!testPool.exists(tx_up_reg.GetHash())); - BOOST_CHECK(!testPool.exists(tx_up_rev.GetHash())); -} - -// A pending ProUpServTx is signed by the operator key and stays valid across a registrar update -// that keeps that key, so only a registrar update that changes the key may evict it. -void FuncTestMempoolProUpServOperatorKeyChange(TestChainSetup& setup) -{ - auto& chainman = *Assert(setup.m_node.chainman.get()); - auto& dmnman = *Assert(setup.m_node.dmnman); - auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; - auto tip_height = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Height()); }; - - const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); - auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); - const CScript scriptPayout = GenerateRandomAddress(); - - CKey ownerKey; - CBLSSecretKey operatorKey; - auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); - const uint256 proTxHash = tx_reg.GetHash(); - auto block = std::make_shared(setup.CreateBlock({tx_reg}, coinbase_pk, chainman.ActiveChainstate())); - BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); - dmnman.UpdatedBlockTip(tip_index()); - BOOST_REQUIRE(dmnman.GetListAtChainTip().HasMN(proTxHash)); - - CBLSSecretKey newOperatorKey; - newOperatorKey.MakeNewKey(); + CMutableTransaction tx_spend_other; + tx_spend_other.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); + tx_spend_other.vout.emplace_back(0, CScript() << OP_RETURN); - auto tx_up_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey); - auto tx_same_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), - ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); - auto tx_new_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), - ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey); - - CTxMemPool testPool{MemPoolOptionsForTest(setup.m_node)}; - TestMemPoolEntryHelper entry; - LOCK2(cs_main, testPool.cs); + CMutableTransaction tx_spend_collateral; + tx_spend_collateral.vin.emplace_back(collateralOutpoint); + tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); - testPool.addUnchecked(entry.FromTx(tx_up_serv)); - BOOST_CHECK_EQUAL(testPool.size(), 1U); + { + LOCK2(cs_main, testPool.cs); + // ProUpServ and ProUpRev are signed by the operator key, so they stay valid across a + // confirmed ProUpReg that keeps it, and across a spend of anything but the collateral. + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + testPool.addUnchecked(entry.FromTx(tx_up_rev)); + testPool.removeForBlock({MakeTransactionRef(tx_up_reg), MakeTransactionRef(tx_spend_other)}, tip_height() + 1); + BOOST_CHECK_EQUAL(testPool.size(), 2U); - testPool.removeForBlock({MakeTransactionRef(tx_same_key)}, tip_height() + 1); - BOOST_CHECK(testPool.exists(tx_up_serv.GetHash())); + testPool.removeForBlock({MakeTransactionRef(tx_up_reg_new_key)}, tip_height() + 1); + BOOST_CHECK_EQUAL(testPool.size(), 0U); - testPool.removeForBlock({MakeTransactionRef(tx_new_key)}, tip_height() + 1); - BOOST_CHECK(!testPool.exists(tx_up_serv.GetHash())); + // Spending the collateral removes the MN, and no update of any kind can be mined after that. + testPool.addUnchecked(entry.FromTx(tx_up_serv)); + testPool.addUnchecked(entry.FromTx(tx_up_reg)); + testPool.addUnchecked(entry.FromTx(tx_up_rev)); + BOOST_CHECK_EQUAL(testPool.size(), 3U); + testPool.removeForBlock({MakeTransactionRef(tx_spend_collateral)}, tip_height() + 1); + BOOST_CHECK_EQUAL(testPool.size(), 0U); + } } void FuncVerifyDB(TestChainSetup& setup) @@ -3662,24 +3545,6 @@ BOOST_AUTO_TEST_CASE(test_mempool_proreg_replacement_update_conflict) FuncTestMempoolProRegReplacementUpdateConflict(setup); } -BOOST_AUTO_TEST_CASE(test_mempool_proreg_spent_collateral) -{ - TestChainV19Setup setup; - FuncTestMempoolProRegSpentCollateral(setup); -} - -BOOST_AUTO_TEST_CASE(test_mempool_proupdates_spent_collateral) -{ - TestChainV19Setup setup; - FuncTestMempoolProUpdatesSpentCollateral(setup); -} - -BOOST_AUTO_TEST_CASE(test_mempool_proupserv_operator_key_change) -{ - TestChainV19Setup setup; - FuncTestMempoolProUpServOperatorKeyChange(setup); -} - //This one can be started only with legacy scheme, since inside undo block will switch it back to legacy resulting into an inconsistency BOOST_AUTO_TEST_CASE(verify_db_legacy) { diff --git a/test/functional/feature_masternode_shares.py b/test/functional/feature_masternode_shares.py index 378ddd80acbe..29a7622a63ee 100755 --- a/test/functional/feature_masternode_shares.py +++ b/test/functional/feature_masternode_shares.py @@ -342,40 +342,6 @@ def test_voting_payee_conflict_eviction(self, node, protx_hash): assert_equal(confirmed_address, voting) assert pending_txid not in node.getrawmempool() - def test_dissolution_evicts_pending_updates(self, node): - self.log.info("A confirmed dissolution evicts pending updates of the dissolved masternode") - shares = [ - {"amount": 600 * COIN, "refundAddress": node.getnewaddress(), "ownerAddress": node.getnewaddress()}, - {"amount": 400 * COIN, "refundAddress": node.getnewaddress(), "ownerAddress": node.getnewaddress()}, - ] - protx_hash, _ = self.register_shared(node, shares, port_offset=8) - registrar_fee, share_fee = node.getnewaddress(), node.getnewaddress() - funding_txid = node.sendmany("", {registrar_fee: 1, share_fee: 1}) - node.syncwithvalidationinterfacequeue() - self.bump_mocktime(10 * 60 + 1) - funding_block = self.generate(node, 1, sync_fun=self.no_op)[0] - assert funding_txid in node.getblock(funding_block)["tx"] - - prepared = node.protx("shared_update_registrar_prepare", protx_hash, "", node.getnewaddress(), registrar_fee) - registrar_sigs = node.protx("shared_sign", prepared["tx"])["signatures"] - registrar = node.protx("shared_combine", prepared["tx"], registrar_sigs) - share_update = node.protx("shared_update_share", protx_hash, 0, node.getnewaddress(), share_fee, False) - dissolve = node.protx("shared_dissolve", protx_hash, 0, DISSOLVE_FEE, False) - self.sync_all() - self.disconnect_nodes(0, 1) - pending_txids = {node.sendrawtransaction(registrar), node.sendrawtransaction(share_update)} - assert pending_txids.issubset(node.getrawmempool()) - # Simulate another miner that never saw the pending updates confirming the dissolution. - other = self.nodes[1] - dissolve_txid = other.sendrawtransaction(dissolve) - self.bump_mocktime(10 * 60 + 1) - dissolve_block = self.generate(other, 1, sync_fun=self.no_op)[0] - assert dissolve_txid in other.getblock(dissolve_block)["tx"] - self.connect_nodes(0, 1) - self.sync_blocks() - assert_raises_rpc_error(None, None, node.protx, "info", protx_hash) - assert pending_txids.isdisjoint(node.getrawmempool()) - def test_separate_participant_wallets(self): self.log.info("Eight separate wallets fund and authorize a shared masternode") node = self.nodes[0] @@ -1198,7 +1164,6 @@ def submit_with_sigs(sigs): self.test_pending_registrar_update(node, protx_hash4) self.test_voting_payee_conflict_eviction(node, protx_hash4) - self.test_dissolution_evicts_pending_updates(node) self.log.info("Shared state survives a node restart") state_before_restart = node.protx("info", protx_hash4)["state"] @@ -1233,6 +1198,10 @@ def submit_with_sigs(sigs): assert_equal(node.protx("info", protx_hash4, node.getbestblockhash())["state"], state_at_snapshot) self.log.info("A reorg across a dissolution restores the masternode and its shares") + registrar_fee, share_fee = node.getnewaddress(), node.getnewaddress() + node.sendmany("", {registrar_fee: 1, share_fee: 1}) + self.bump_mocktime(10 * 60 + 1) + self.generate(node, 1, sync_fun=self.no_op) info_before_dissolve = node.protx("info", protx_hash4) dissolve_txid4 = node.protx("shared_dissolve", protx_hash4, 0, DISSOLVE_FEE) self.bump_mocktime(10 * 60 + 1) @@ -1245,9 +1214,18 @@ def submit_with_sigs(sigs): # the disconnected dissolution returns to the mempool: the restored masternode makes it # valid again, so it is not lost by the reorg assert dissolve_txid4 in node.getrawmempool() + # the restored masternode can be updated again, but the dissolution block was mined without + # these updates: once it is reconnected they can never be mined and must not stay behind + prepared = node.protx("shared_update_registrar_prepare", protx_hash4, "", node.getnewaddress(), registrar_fee) + registrar_sigs = node.protx("shared_sign", prepared["tx"])["signatures"] + registrar = node.protx("shared_combine", prepared["tx"], registrar_sigs) + pending_txids = {node.sendrawtransaction(registrar), + node.protx("shared_update_share", protx_hash4, 0, node.getnewaddress(), share_fee)} + assert pending_txids.issubset(node.getrawmempool()) node.reconsiderblock(dissolve_block) assert_raises_rpc_error(None, None, node.protx, "info", protx_hash4) assert dissolve_txid4 not in node.getrawmempool() + assert pending_txids.isdisjoint(node.getrawmempool()) assert_equal(node.masternodelist(), {}) self.log.info("Every participant's principal was refunded by the dissolutions") From 9895ee7af36b370a75be703e98a2308e1bb899a0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 6 Oct 2026 22:56:48 +0700 Subject: [PATCH 09/11] fix(mempool): keep a pending ProRegTx when its own collateral output is spent removeProTxSpentCollateralConflicts() now evicts a pending ProRegTx whose collateral is spent. A ProRegTx that funds its collateral itself is tracked in mapProTxCollaterals under its own output, and the only transaction able to spend that output is its descendant, which is mined after it. The ProRegTx is still mineable, so this is not a conflict. Blocks do not reach this case, but mempool acceptance does: for a transaction whose InstantSend lock arrived first, MemPoolAccept calls removeProTxConflicts() before looking up the inputs. A locked child spending the collateral then evicted its parent ProRegTx as CONFLICT, the parent lost its InstantSend lock, and the child was rejected for missing inputs. Evict only when the spent collateral is not an output of the ProRegTx itself. Transactions referring to the ProRegTx are still removed, as before. --- src/txmempool.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 415938038e92..b571b0a089a9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1056,9 +1056,11 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) for (const auto& in : tx.vin) { auto collateralIt = mapProTxCollaterals.find(in.prevout); if (collateralIt != mapProTxCollaterals.end()) { - // A not yet mined ProRegTx whose collateral is now spent, and TXs referring to it + // A not yet mined ProRegTx whose collateral is now spent, and TXs referring to it. + // A collateral that is an output of the ProRegTx itself can only be spent by its own + // descendant, which is mined after it and so does not make it unmineable const uint256 proRegTxHash{collateralIt->second}; - if (auto it = mapTx.find(proRegTxHash); it != mapTx.end()) { + if (auto it = mapTx.find(proRegTxHash); it != mapTx.end() && in.prevout.hash != proRegTxHash) { removeRecursive(it->GetTx(), MemPoolRemovalReason::CONFLICT); } removeProTxReferences(proRegTxHash); From 612de7098ed8f36c82e42a03d9df89ece940a010 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 6 Oct 2026 22:56:48 +0700 Subject: [PATCH 10/11] test: a pending ProRegTx survives a spend of the collateral it carries FuncTestMempoolDualProregtx already holds a pending ProRegTx with an internal collateral. Run removeProTxConflicts() for a transaction spending that collateral output, as mempool acceptance does for a transaction whose InstantSend lock arrived first, and expect the ProRegTx to stay. --- src/test/evo_deterministicmns_tests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 8ca0e4a6c900..0555ee10f0f5 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1450,6 +1450,15 @@ void FuncTestMempoolDualProregtx(TestChainSetup& setup) testPool.addUnchecked(entry.FromTx(tx_reg1)); BOOST_CHECK_EQUAL(testPool.size(), 1U); BOOST_CHECK(testPool.existsProviderTxConflict(CTransaction(tx_reg2))); + + // A TX spending the collateral tx_reg1 carries itself is its child. When its InstantSend lock + // arrives first, mempool acceptance drops ProTx conflicts of the child before looking up its + // inputs, and that must not take the parent away + CMutableTransaction tx_spend_collateral; + tx_spend_collateral.vin.emplace_back(collateralOutpoint); + tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); + testPool.removeProTxConflicts(CTransaction(tx_spend_collateral)); + BOOST_CHECK_EQUAL(testPool.size(), 1U); } // A ProRegTx that reuses a confirmed external collateral replaces the live MN at block From 606efac0e48217f8fb66dd413cd1ee51bff620fd Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 6 Oct 2026 21:14:21 +0300 Subject: [PATCH 11/11] revert: drop the pending ProRegTx eviction on collateral spend for now Restore removeProTxSpentCollateralConflicts() to its previous behaviour, which also removes the exemption for a ProRegTx whose own collateral output is spent, and drop the tests added for both in FuncTestMempoolReorg and FuncTestMempoolDualProregtx. The eviction has to be coordinated with a related InstantSend policy change and will be proposed again together with it. The ProUpServTx fix and the coverage of the existing eviction paths stay. Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 28 ------------------------- src/txmempool.cpp | 10 ++------- 2 files changed, 2 insertions(+), 36 deletions(-) diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 0555ee10f0f5..cc5e0eb3e2ab 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1321,25 +1321,6 @@ void FuncTestMempoolReorg(TestChainSetup& setup) block_reorg.emplace_back(std::make_shared(tx_reg_ds)); testPool.removeForBlock(block_reorg, nHeight + 2); BOOST_CHECK_EQUAL(testPool.size(), 0U); - - // Check mempool as if the new block spent the collateral of ProRegTx instead: ProRegTx can never - // be mined then and has to go together with ProUpServ referring to it. Spending another output - // of the collateral tx invalidates neither - testPool.addUnchecked(entry.FromTx(tx_up_serv)); - testPool.addUnchecked(entry.FromTx(tx_reg)); - BOOST_CHECK_EQUAL(testPool.size(), 2U); - - CMutableTransaction tx_spend_other; - tx_spend_other.vin.emplace_back(COutPoint(tx_collateral.GetHash(), collateralOutpoint.n + 1)); - tx_spend_other.vout.emplace_back(0, CScript() << OP_RETURN); - testPool.removeForBlock({MakeTransactionRef(tx_spend_other)}, nHeight + 2); - BOOST_CHECK_EQUAL(testPool.size(), 2U); - - CMutableTransaction tx_spend_collateral; - tx_spend_collateral.vin.emplace_back(collateralOutpoint); - tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); - testPool.removeForBlock({MakeTransactionRef(tx_spend_collateral)}, nHeight + 2); - BOOST_CHECK_EQUAL(testPool.size(), 0U); } // Regression test: a ProUpRev/ProUpReg invalidates every pending ProTx of the same masternode, and @@ -1450,15 +1431,6 @@ void FuncTestMempoolDualProregtx(TestChainSetup& setup) testPool.addUnchecked(entry.FromTx(tx_reg1)); BOOST_CHECK_EQUAL(testPool.size(), 1U); BOOST_CHECK(testPool.existsProviderTxConflict(CTransaction(tx_reg2))); - - // A TX spending the collateral tx_reg1 carries itself is its child. When its InstantSend lock - // arrives first, mempool acceptance drops ProTx conflicts of the child before looking up its - // inputs, and that must not take the parent away - CMutableTransaction tx_spend_collateral; - tx_spend_collateral.vin.emplace_back(collateralOutpoint); - tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); - testPool.removeProTxConflicts(CTransaction(tx_spend_collateral)); - BOOST_CHECK_EQUAL(testPool.size(), 1U); } // A ProRegTx that reuses a confirmed external collateral replaces the live MN at block diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b571b0a089a9..29d08f8d67a6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1056,14 +1056,8 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) for (const auto& in : tx.vin) { auto collateralIt = mapProTxCollaterals.find(in.prevout); if (collateralIt != mapProTxCollaterals.end()) { - // A not yet mined ProRegTx whose collateral is now spent, and TXs referring to it. - // A collateral that is an output of the ProRegTx itself can only be spent by its own - // descendant, which is mined after it and so does not make it unmineable - const uint256 proRegTxHash{collateralIt->second}; - if (auto it = mapTx.find(proRegTxHash); it != mapTx.end() && in.prevout.hash != proRegTxHash) { - removeRecursive(it->GetTx(), MemPoolRemovalReason::CONFLICT); - } - removeProTxReferences(proRegTxHash); + // These are not yet mined ProRegTxs + removeProTxReferences(collateralIt->second); } auto dmn = mnList.GetMNByCollateral(in.prevout); if (dmn) {