diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 625a0c9eabf3..cc5e0eb3e2ab 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1340,7 +1340,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,6 +1538,43 @@ void FuncTestMempoolProRegReplacementUpdateConflict(TestChainSetup& setup) testPool.removeForBlock(connected, tip_height() + 1); BOOST_CHECK_EQUAL(testPool.size(), 0U); } + + // 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_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); + + CMutableTransaction tx_spend_collateral; + tx_spend_collateral.vin.emplace_back(collateralOutpoint); + tx_spend_collateral.vout.emplace_back(0, CScript() << OP_RETURN); + + { + 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_up_reg_new_key)}, tip_height() + 1); + BOOST_CHECK_EQUAL(testPool.size(), 0U); + + // 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) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 37c1eb10dedc..29d08f8d67a6 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); } diff --git a/test/functional/feature_masternode_shares.py b/test/functional/feature_masternode_shares.py index 4acfdd83c1fa..29a7622a63ee 100755 --- a/test/functional/feature_masternode_shares.py +++ b/test/functional/feature_masternode_shares.py @@ -1198,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) @@ -1210,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")