diff --git a/doc/release-notes-7857.md b/doc/release-notes-7857.md new file mode 100644 index 000000000000..d6781a125242 --- /dev/null +++ b/doc/release-notes-7857.md @@ -0,0 +1,13 @@ +Mining and mempool policy +------------------------- + +- When a block template includes an operator key change or revocation for a masternode, it now + places the masternode's pending `ProUpServTx` signed with the key being replaced before it. Such + a service update could otherwise confirm after the change in the same block and restore the + previous operator's service and payout details. A service update that cannot be placed before + the change, or that does not fit in the block with it, is left out of that block. (#7857) +- The mempool now rejects with `protx-dup` a `ProUpServTx` that spends, directly or through other + pending transactions, an output of a pending operator key change or revocation, which it can only + confirm after, or of a pending asset lock, asset unlock or MNHF signal, which could keep it out of + a block. A service update funded from such outputs can be sent once they confirm, signed with the + operator key that is current then. (#7857) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index aef331c99d8d..ce3ab0cdb5c3 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -819,6 +819,8 @@ uint256 CSpecialTxProcessor::GetDeterministicMNListHash(gsl::not_null pindex) { m_dmnman.UpdatedBlockTip(pindex); } + bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const CChain& chain, const CBlock& block, const CBlockIndex* pindex, SpecialTxRules rules, const CCoinsViewCache& view, CAmount blockSubsidy, bool fJustCheck, diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 1481ad01683a..18f48598a2a6 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -107,6 +107,9 @@ class CSpecialTxProcessor /** Return a canonical hash of the deterministic MN list derived at a block. */ uint256 GetDeterministicMNListHash(gsl::not_null pindex) const; + /** Passthrough to CDeterministicMNManager::UpdatedBlockTip */ + void UpdatedMNListTip(gsl::not_null pindex); + private: bool CheckSpecialTxInner(const CChain* chain, const CTransaction& tx, const CBlockIndex* pindexPrev, SpecialTxRules rules, const CCoinsViewCache& view, const std::optional& indexes, diff --git a/src/node/miner.cpp b/src/node/miner.cpp index b3a32bad66d3..5667865a9ed4 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -553,6 +554,16 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // This map with signals is used only to find duplicates auto signals = m_chainstate.m_special_tx.m_mnhfman->GetSignalsStage(pindexPrev); const SpecialTxRules special_tx_rules{GetSpecialTxRules(pindexPrev, m_chainstate.m_chainman)}; + // Masternodes whose operator key a transaction already in the block changes or revokes. A + // ProUpServTx of theirs must not follow: one left out of the key change's package, whether it + // could not be put first, did not fit or did not pass TestPackageTransactions() then (which can + // change while the template is built), may come up later. + std::set keyChangedMNs; + const auto serviceUpdateTarget = [](const CTransaction& tx) -> std::optional { + if (tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) return std::nullopt; + const auto opt_proTx = GetTxPayload(tx); + return opt_proTx ? std::make_optional(opt_proTx->proTxHash) : std::nullopt; + }; // mapModifiedTx will store sorted packages after they are modified // because some of their txs are already in the block @@ -659,6 +670,50 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele onlyUnconfirmed(ancestors); ancestors.insert(iter); + // A pending ProUpServTx signed with the operator key that a key change in this package + // replaces must not follow the key change in the block: it would restore the previous + // operator's service fields. Mine such updates first. One that cannot be in this block now, + // whose package block assembly could not reliably put first (the mempool rejects those on + // entry, but a reorg can return such ancestors later), or whose package has an update of a + // masternode whose key already changed in the block, is left out: keyChangedMNs keeps it out + // of the rest of the block. If those pulled in do not fit, they are all left out instead of + // holding the key change back, which pending updates could otherwise do forever. + CTxMemPool::setEntries serviceUpdates; + for (const auto& entry : ancestors) { + for (const auto& update : mempool.GetServiceUpdatesBeforeKeyChange(entry)) { + if (inBlock.count(update) || serviceUpdates.count(update)) { + continue; + } + auto updatePackage{mempool.AssumeCalculateMemPoolAncestors(__func__, *update, + CTxMemPool::Limits::NoLimits(), + /*fSearchForParents=*/false)}; + onlyUnconfirmed(updatePackage); + updatePackage.insert(update); + const bool followsKeyChange{std::any_of(updatePackage.begin(), updatePackage.end(), + [&](const CTxMemPool::txiter& it) { + const auto target{serviceUpdateTarget(it->GetTx())}; + return target && keyChangedMNs.count(*target); + })}; + if (!followsKeyChange && TestPackageTransactions(updatePackage) && + !mempool.IsUnorderableServiceUpdate(update->GetTx(), updatePackage)) { + serviceUpdates.insert(updatePackage.begin(), updatePackage.end()); + } + } + } + if (!serviceUpdates.empty()) { + uint64_t extraSize{0}; + unsigned int extraSigOps{0}; + for (const auto& entry : serviceUpdates) { + if (!ancestors.count(entry)) { + extraSize += entry->GetTxSize(); + extraSigOps += entry->GetSigOpCount(); + } + } + if (!TestPackage(packageSize + extraSize, packageSigOps + extraSigOps)) { + serviceUpdates.clear(); + } + } + // Test if all tx's are Final and safe if (!TestPackageTransactions(ancestors)) { if (fUsingModified) { @@ -668,15 +723,38 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - // Package can be added. Sort the entries in a valid order. + // Package can be added. Sort the entries in a valid order, the service updates first. std::vector sortedEntries; - SortForBlock(ancestors, sortedEntries); + SortForBlock(serviceUpdates, sortedEntries); + CTxMemPool::setEntries rest; + for (const auto& entry : ancestors) { + if (!serviceUpdates.count(entry)) { + rest.insert(entry); + } + } + std::vector sortedRest; + SortForBlock(rest, sortedRest); + sortedEntries.insert(sortedEntries.end(), sortedRest.begin(), sortedRest.end()); + ancestors.insert(serviceUpdates.begin(), serviceUpdates.end()); auto packageSignals = signals; + std::vector packageKeyChangedMNs; std::vector creditPoolTransactions; bool validPackage{true}; for (const auto& entry : sortedEntries) { const auto& tx = entry->GetTx(); + if (const auto target = serviceUpdateTarget(tx); + target && (keyChangedMNs.count(*target) || + std::find(packageKeyChangedMNs.begin(), packageKeyChangedMNs.end(), *target) != + packageKeyChangedMNs.end())) { + LogPrintf("%s: package tx %s skipped, it would follow a key change of masternode %s\n", __func__, + tx.GetHash().ToString(), target->ToString()); + validPackage = false; + break; + } + if (const auto target = mempool.GetKeyChangeTarget(*entry)) { + packageKeyChangedMNs.push_back(*target); + } if (std::optional signal = extractEHFSignal(tx); signal != std::nullopt) { if (!packageSignals.emplace(*signal, 0).second) { LogPrintf("%s: package tx %s skipped due to duplicate EHF signal %d\n", __func__, @@ -723,6 +801,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // This transaction will make it in; reset the failed counter. nConsecutiveFailed = 0; signals = std::move(packageSignals); + keyChangedMNs.insert(packageKeyChangedMNs.begin(), packageKeyChangedMNs.end()); for (size_t i = 0; i < sortedEntries.size(); ++i) { AddToBlock(sortedEntries[i]); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 1d307803eb9f..c6333308bc24 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -97,7 +98,7 @@ static CMutableTransaction CreateProRegTxExternalCollateral(const ChainstateMana } static CMutableTransaction CreateProUpServTx(const ChainstateManager& chainman, SimpleUTXOMap& utxos, const uint256& proTxHash, const CBLSSecretKey& operatorKey, int port, const CScript& scriptOperatorPayout, const CKey& coinbaseKey, - uint16_t version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false)) + uint16_t version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false), CAmount fee = 0) { CProUpServTx proTx; proTx.nVersion = version; @@ -111,6 +112,7 @@ static CMutableTransaction CreateProUpServTx(const ChainstateManager& chainman, tx.nVersion = 3; tx.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE; const auto spent = FundTransaction(chainman, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN); + tx.vout.back().nValue -= fee; proTx.inputsHash = CalcTxInputsHash(CTransaction(tx)); proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme); SetTxPayload(tx, proTx); @@ -145,7 +147,8 @@ static CMutableTransaction CreateProUpRegTx(const ChainstateManager& chainman, S return tx; } -static CMutableTransaction CreateProUpRevTx(const ChainstateManager& chainman, SimpleUTXOMap& utxos, const uint256& proTxHash, const CBLSSecretKey& operatorKey, const CKey& coinbaseKey) +static CMutableTransaction CreateProUpRevTx(const ChainstateManager& chainman, SimpleUTXOMap& utxos, const uint256& proTxHash, const CBLSSecretKey& operatorKey, const CKey& coinbaseKey, + CAmount fee = 0) { CProUpRevTx proTx; proTx.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); @@ -155,6 +158,7 @@ static CMutableTransaction CreateProUpRevTx(const ChainstateManager& chainman, S tx.nVersion = 3; tx.nType = TRANSACTION_PROVIDER_UPDATE_REVOKE; const auto spent = FundTransaction(chainman, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN); + tx.vout.back().nValue -= fee; proTx.inputsHash = CalcTxInputsHash(CTransaction(tx)); proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme); SetTxPayload(tx, proTx); @@ -1583,6 +1587,601 @@ void FuncTestMempoolProRegReplacementUpdateConflict(TestChainSetup& setup) } } +// Builds a block template from the given mempool transactions and returns the position of each +// transaction in it. +static std::map TemplatePositions(TestChainSetup& setup, + const std::vector>& txs, + size_t block_max_size = DEFAULT_BLOCK_MAX_SIZE) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& mempool = *Assert(setup.m_node.mempool.get()); + TestMemPoolEntryHelper entry; + { + LOCK2(cs_main, mempool.cs); + for (const auto& [tx, fee] : txs) { + mempool.addUnchecked(entry.Fee(fee).FromTx(tx)); + } + } + node::BlockAssembler::Options options; + options.nBlockMaxSize = block_max_size; + const auto block_template = node::BlockAssembler{chainman.ActiveChainstate(), setup.m_node, &mempool, options}.CreateNewBlock( + GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey())); + BOOST_REQUIRE(block_template); + std::map positions; + for (size_t i = 0; i < block_template->block.vtx.size(); ++i) { + positions.emplace(block_template->block.vtx[i]->GetHash(), i); + } + { + LOCK2(cs_main, mempool.cs); + for (const auto& [tx, fee] : txs) { + mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::MANUAL); + } + } + return positions; +} + +// A pending ProUpServTx is signed with the operator key a key change replaces. Mined after the key +// change in the same block, it would restore the previous operator's service fields, so block +// assembly mines it first whatever the fees say. +void FuncTestMinerServiceUpdateBeforeKeyChange(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()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript scriptPayout = GenerateRandomAddress(); + + CKey ownerKey; + CKey otherOwnerKey; + CBLSSecretKey operatorKey; + CBLSSecretKey otherOperatorKey; + auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); + auto tx_other_reg = CreateProRegTx(chainman, utxos, /*port=*/2, scriptPayout, setup.coinbaseKey, otherOwnerKey, + otherOperatorKey); + const uint256 proTxHash = tx_reg.GetHash(); + auto block = std::make_shared( + setup.CreateBlock({tx_reg, tx_other_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(); + const CAmount low_fee{1000}; + const CAmount high_fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/3, CScript(), setup.coinbaseKey, + version, low_fee); + // Takes the address of the other masternode, so it cannot be mined at all + auto tx_serv_unminable = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), + setup.coinbaseKey, version, low_fee); + // The revocation spends what the key change spends; the two are never in the pool together + auto revoke_utxos{utxos}; + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + auto tx_revoke = CreateProUpRevTx(chainman, revoke_utxos, proTxHash, operatorKey, setup.coinbaseKey, high_fee); + auto tx_same_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, high_fee); + + // The key change or revocation pays more, but the service update goes in first + for (const auto& tx_change : {tx_key_change, tx_revoke}) { + const auto positions = TemplatePositions(setup, {{tx_serv, low_fee}, {tx_change, high_fee}}); + BOOST_REQUIRE(positions.count(tx_serv.GetHash()) && positions.count(tx_change.GetHash())); + BOOST_CHECK_LT(positions.at(tx_serv.GetHash()), positions.at(tx_change.GetHash())); + } + std::map positions; + + // A registrar update that keeps the operator key is ordered by fee as usual + positions = TemplatePositions(setup, {{tx_serv, low_fee}, {tx_same_key, high_fee}}); + BOOST_REQUIRE(positions.count(tx_serv.GetHash()) && positions.count(tx_same_key.GetHash())); + BOOST_CHECK_LT(positions.at(tx_same_key.GetHash()), positions.at(tx_serv.GetHash())); + + // A service update that cannot be mined does not hold the key change back + positions = TemplatePositions(setup, {{tx_serv_unminable, low_fee}, {tx_key_change, high_fee}}); + BOOST_CHECK(!positions.count(tx_serv_unminable.GetHash())); + BOOST_CHECK(positions.count(tx_key_change.GetHash())); +} + +// A ProUpServTx with a pending operator key change among its ancestors can only be mined after that +// change. For its own masternode it would then restore the previous operator's service fields; for +// another one it would hold back a key change of its own masternode in block assembly, as would a +// pending asset lock or unlock that block assembly turns down. The mempool refuses it whether it +// spends such a transaction directly or through another one, and accepts an independent ProUpServTx. +void FuncTestMempoolRejectsUnorderableServiceUpdate(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& mempool = *Assert(setup.m_node.mempool.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto submit = [&](const CMutableTransaction& tx) { + return WITH_LOCK(::cs_main, return chainman.ProcessTransaction(MakeTransactionRef(tx))); + }; + auto output_coins = [](const CMutableTransaction& tx) { + return SimpleUTXOMap{{COutPoint(tx.GetHash(), 0), Coin(tx.vout[0], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + const CScript coinbase_pkh = GetScriptForDestination(PKHash(setup.coinbaseKey.GetPubKey())); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript scriptPayout = GenerateRandomAddress(); + + CKey ownerKey; + CKey otherOwnerKey; + CBLSSecretKey operatorKey; + CBLSSecretKey otherOperatorKey; + auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); + auto tx_other_reg = CreateProRegTx(chainman, utxos, /*port=*/2, scriptPayout, setup.coinbaseKey, otherOwnerKey, + otherOperatorKey); + const uint256 proTxHash = tx_reg.GetHash(); + auto block = std::make_shared( + setup.CreateBlock({tx_reg, tx_other_reg}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + CBLSSecretKey newOperatorKey; + CBLSSecretKey otherNewOperatorKey; + newOperatorKey.MakeNewKey(); + otherNewOperatorKey.MakeNewKey(); + const CAmount fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, fee); + auto tx_other_key_change = CreateProUpRegTx(chainman, utxos, tx_other_reg.GetHash(), otherOwnerKey, + otherNewOperatorKey.GetPublicKey(), otherOwnerKey.GetPubKey().GetID(), + scriptPayout, setup.coinbaseKey, version, fee); + BOOST_REQUIRE(submit(tx_key_change).m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_REQUIRE(submit(tx_other_key_change).m_result_type == MempoolAcceptResult::ResultType::VALID); + + // An ordinary transaction spending the change of the key change, for a service update to spend in turn + BOOST_REQUIRE_EQUAL(tx_key_change.vout.size(), 2U); + const COutPoint key_change_change{tx_key_change.GetHash(), 1}; + CMutableTransaction tx_spend_key_change; + tx_spend_key_change.vin.emplace_back(key_change_change); + tx_spend_key_change.vout.emplace_back(tx_key_change.vout[1].nValue - fee, coinbase_pkh); + const SimpleUTXOMap spend_coins{ + {key_change_change, Coin(tx_key_change.vout[1], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + SignTransaction(tx_spend_key_change, spend_coins, setup.coinbaseKey); + BOOST_REQUIRE(submit(tx_spend_key_change).m_result_type == MempoolAcceptResult::ResultType::VALID); + + // Parked in the mempool without validation, only to be an ancestor + CMutableTransaction tx_asset_lock; + tx_asset_lock.nVersion = 3; + tx_asset_lock.nType = TRANSACTION_ASSET_LOCK; + const auto lock_coins = FundTransaction(chainman, tx_asset_lock, utxos, coinbase_pkh, 1 * COIN); + SetTxPayload(tx_asset_lock, CAssetLockPayload{}); + SignTransaction(tx_asset_lock, lock_coins, setup.coinbaseKey); + { + LOCK2(cs_main, mempool.cs); + mempool.addUnchecked(TestMemPoolEntryHelper{}.Fee(fee).Time(Now()).FromTx(tx_asset_lock)); + } + + for (const auto& [funding, port] : std::vector>{ + {tx_key_change, 3}, {tx_spend_key_change, 4}, {tx_other_key_change, 5}, {tx_asset_lock, 7}}) { + auto coins = output_coins(funding); + auto tx_serv = CreateProUpServTx(chainman, coins, proTxHash, operatorKey, port, CScript(), setup.coinbaseKey, + version, fee); + const auto result = submit(tx_serv); + BOOST_CHECK(result.m_result_type == MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "protx-dup"); + } + + auto tx_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/6, CScript(), setup.coinbaseKey, + version, fee); + BOOST_CHECK(submit(tx_serv).m_result_type == MempoolAcceptResult::ResultType::VALID); +} + +// A reorg returns a mined key change to the mempool before the block tip notifications move the +// masternode list. Recorded against the list of the disconnected block, which already has the new +// key, it would not count as a pending key change, and the mempool would admit a second one for the +// masternode. A service update still goes in before it. +void FuncTestMinerServiceUpdateBeforeReorgedKeyChange(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& mempool = *Assert(setup.m_node.mempool.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + + 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()); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + const CAmount low_fee{1000}; + const CAmount high_fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + block = std::make_shared(setup.CreateBlock({tx_key_change}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + BlockValidationState state; + BOOST_REQUIRE(chainman.ActiveChainstate().InvalidateBlock(state, tip_index())); + BOOST_REQUIRE(WITH_LOCK(mempool.cs, return mempool.exists(tx_key_change.GetHash()))); + dmnman.UpdatedBlockTip(tip_index()); + + CBLSSecretKey otherOperatorKey; + otherOperatorKey.MakeNewKey(); + auto tx_other_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, otherOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + const auto result{WITH_LOCK(::cs_main, return chainman.ProcessTransaction(MakeTransactionRef(tx_other_key_change)))}; + BOOST_CHECK(result.m_result_type == MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "protx-dup"); + + auto tx_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey, + version, low_fee); + const auto positions = TemplatePositions(setup, {{tx_serv, low_fee}}); + BOOST_REQUIRE(positions.count(tx_serv.GetHash()) && positions.count(tx_key_change.GetHash())); + BOOST_CHECK_LT(positions.at(tx_serv.GetHash()), positions.at(tx_key_change.GetHash())); +} + +// A registrar update that kept the operator key when it entered the mempool becomes a key change +// once a reorg disconnects the block that set that key. A service update signed with the key +// restored by the reorg still goes in before it. +void FuncTestMinerServiceUpdateBeforeKeyChangeSurvivingReorg(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& mempool = *Assert(setup.m_node.mempool.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + + 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()); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + const CAmount low_fee{1000}; + const CAmount high_fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + block = std::make_shared(setup.CreateBlock({tx_key_change}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + // Keeps the key the mined change set, so it enters the mempool as no key change + auto tx_same_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, high_fee); + BOOST_REQUIRE(WITH_LOCK(::cs_main, return chainman.ProcessTransaction(MakeTransactionRef(tx_same_key))) + .m_result_type == MempoolAcceptResult::ResultType::VALID); + + BlockValidationState state; + BOOST_REQUIRE(chainman.ActiveChainstate().InvalidateBlock(state, tip_index())); + dmnman.UpdatedBlockTip(tip_index()); + BOOST_REQUIRE(WITH_LOCK(mempool.cs, return mempool.exists(tx_same_key.GetHash()))); + + auto tx_serv = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey, + version, low_fee); + const auto positions = TemplatePositions(setup, {{tx_serv, low_fee}}); + BOOST_REQUIRE(positions.count(tx_serv.GetHash()) && positions.count(tx_same_key.GetHash())); + BOOST_CHECK_LT(positions.at(tx_serv.GetHash()), positions.at(tx_same_key.GetHash())); +} + +// However a ProUpServTx comes to follow a key change of its masternode in the pool (it spends the key +// change here), block assembly never mines it after the key change, but still mines one following a +// registrar update that keeps the operator key. +void FuncTestMinerServiceUpdateNeverAfterKeyChange(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 output_coins = [](const CMutableTransaction& tx) { + return SimpleUTXOMap{{COutPoint(tx.GetHash(), 0), Coin(tx.vout[0], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + }; + + 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()); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + const CAmount fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, fee); + auto tx_same_key = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, operatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, fee); + + auto key_change_coins = output_coins(tx_key_change); + auto tx_serv = CreateProUpServTx(chainman, key_change_coins, proTxHash, operatorKey, /*port=*/2, CScript(), + setup.coinbaseKey, version, fee); + auto positions = TemplatePositions(setup, {{tx_key_change, fee}, {tx_serv, fee}}); + BOOST_CHECK(positions.count(tx_key_change.GetHash())); + BOOST_CHECK(!positions.count(tx_serv.GetHash())); + + auto same_key_coins = output_coins(tx_same_key); + auto tx_serv_after_same_key = CreateProUpServTx(chainman, same_key_coins, proTxHash, operatorKey, /*port=*/3, + CScript(), setup.coinbaseKey, version, fee); + positions = TemplatePositions(setup, {{tx_same_key, fee}, {tx_serv_after_same_key, fee}}); + BOOST_REQUIRE(positions.count(tx_same_key.GetHash()) && positions.count(tx_serv_after_same_key.GetHash())); + BOOST_CHECK_LT(positions.at(tx_same_key.GetHash()), positions.at(tx_serv_after_same_key.GetHash())); +} + +// A reorg can return, below a pending ProUpServTx, an ancestor the mempool would have refused it +// for: here a key change of another masternode that pays nothing, so nothing else gets it mined. +// Block assembly leaves such a ProUpServTx out instead of holding back a key change of its +// masternode, and does not mine it after that key change either. +void FuncTestMinerKeyChangeNotHeldBackAfterReorg(TestChainSetup& setup) +{ + auto& chainman = *Assert(setup.m_node.chainman.get()); + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& mempool = *Assert(setup.m_node.mempool.get()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + auto submit = [&](const CMutableTransaction& tx) { + return WITH_LOCK(::cs_main, return chainman.ProcessTransaction(MakeTransactionRef(tx))); + }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript scriptPayout = GenerateRandomAddress(); + + CKey ownerKey; + CKey otherOwnerKey; + CBLSSecretKey operatorKey; + CBLSSecretKey otherOperatorKey; + auto tx_reg = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKey, operatorKey); + auto tx_other_reg = CreateProRegTx(chainman, utxos, /*port=*/2, scriptPayout, setup.coinbaseKey, otherOwnerKey, + otherOperatorKey); + const uint256 proTxHash = tx_reg.GetHash(); + auto block = std::make_shared( + setup.CreateBlock({tx_reg, tx_other_reg}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + CBLSSecretKey newOperatorKey; + CBLSSecretKey otherNewOperatorKey; + newOperatorKey.MakeNewKey(); + otherNewOperatorKey.MakeNewKey(); + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_other_key_change = CreateProUpRegTx(chainman, utxos, tx_other_reg.GetHash(), otherOwnerKey, + otherNewOperatorKey.GetPublicKey(), otherOwnerKey.GetPubKey().GetID(), + scriptPayout, setup.coinbaseKey, version, /*fee=*/0); + block = std::make_shared(setup.CreateBlock({tx_other_key_change}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + dmnman.UpdatedBlockTip(tip_index()); + + // Spends the confirmed key change, so the mempool admits it + SimpleUTXOMap other_key_change_coins{{COutPoint(tx_other_key_change.GetHash(), 0), + Coin(tx_other_key_change.vout[0], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + auto tx_serv = CreateProUpServTx(chainman, other_key_change_coins, proTxHash, operatorKey, /*port=*/3, CScript(), + setup.coinbaseKey, version, /*fee=*/500); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + /*fee=*/100000); + BOOST_REQUIRE(submit(tx_serv).m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_REQUIRE(submit(tx_key_change).m_result_type == MempoolAcceptResult::ResultType::VALID); + + BlockValidationState state; + BOOST_REQUIRE(chainman.ActiveChainstate().InvalidateBlock(state, tip_index())); + dmnman.UpdatedBlockTip(tip_index()); + { + LOCK(mempool.cs); + BOOST_REQUIRE(mempool.exists(tx_other_key_change.GetHash())); + BOOST_REQUIRE(mempool.exists(tx_serv.GetHash())); + BOOST_REQUIRE(mempool.exists(tx_key_change.GetHash())); + } + + const auto positions = TemplatePositions(setup, {}); + BOOST_CHECK(positions.count(tx_key_change.GetHash())); + BOOST_CHECK(!positions.count(tx_serv.GetHash())); +} + +// A ProUpServTx funded by an ordinary pending transaction is pulled in ahead of a key change of its +// masternode together with that parent. With room for all of them they go in as parent, service +// update, key change. With room for either the parent and the update or the key change, but not for +// all three, the key change goes in without the update, and the template stays within its size limit. +void FuncTestMinerServiceUpdateWithParentBeforeKeyChange(TestChainSetup& setup, bool limit_block_size) +{ + 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()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + const CScript coinbase_pkh = GetScriptForDestination(PKHash(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()); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + const CAmount low_fee{1000}; + const CAmount high_fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + CMutableTransaction tx_parent; + const auto parent_coins = FundTransaction(chainman, tx_parent, utxos, coinbase_pkh, 1 * COIN); + tx_parent.vout.back().nValue -= low_fee; + SignTransaction(tx_parent, parent_coins, setup.coinbaseKey); + SimpleUTXOMap service_coins{ + {COutPoint(tx_parent.GetHash(), 0), Coin(tx_parent.vout[0], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + auto tx_serv = CreateProUpServTx(chainman, service_coins, proTxHash, operatorKey, /*port=*/2, CScript(), + setup.coinbaseKey, version, low_fee); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + const std::vector> txs{{tx_parent, low_fee}, + {tx_serv, low_fee}, + {tx_key_change, high_fee}}; + + if (!limit_block_size) { + const auto positions = TemplatePositions(setup, txs); + BOOST_REQUIRE(positions.count(tx_parent.GetHash()) && positions.count(tx_serv.GetHash()) && + positions.count(tx_key_change.GetHash())); + BOOST_CHECK_LT(positions.at(tx_parent.GetHash()), positions.at(tx_serv.GetHash())); + BOOST_CHECK_LT(positions.at(tx_serv.GetHash()), positions.at(tx_key_change.GetHash())); + return; + } + + TemplatePositions(setup, {}); + const auto base_size{static_cast(*Assert(node::BlockAssembler::m_last_block_size))}; + const size_t service_size{::GetSerializeSize(CTransaction(tx_parent), PROTOCOL_VERSION) + + ::GetSerializeSize(CTransaction(tx_serv), PROTOCOL_VERSION)}; + const size_t key_change_size{::GetSerializeSize(CTransaction(tx_key_change), PROTOCOL_VERSION)}; + const size_t block_max_size{base_size + std::max(service_size, key_change_size) + 1}; + const auto positions = TemplatePositions(setup, txs, block_max_size); + BOOST_CHECK(positions.count(tx_key_change.GetHash())); + BOOST_CHECK(!positions.count(tx_serv.GetHash())); + BOOST_CHECK_LT(static_cast(*Assert(node::BlockAssembler::m_last_block_size)), block_max_size); +} + +// The operator being replaced can keep several independently funded service updates of its masternode +// pending, together too large to go in ahead of a key change. Block assembly then mines the key change +// without them instead of holding it back, and does not mine them after it. +void FuncTestMinerKeyChangeNotHeldBackByServiceUpdates(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()); }; + + 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()); + + CBLSSecretKey newOperatorKey; + newOperatorKey.MakeNewKey(); + const CAmount low_fee{1000}; + const CAmount high_fee{100000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + auto tx_serv1 = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/2, CScript(), setup.coinbaseKey, + version, low_fee); + auto tx_serv2 = CreateProUpServTx(chainman, utxos, proTxHash, operatorKey, /*port=*/3, CScript(), setup.coinbaseKey, + version, low_fee); + auto tx_key_change = CreateProUpRegTx(chainman, utxos, proTxHash, ownerKey, newOperatorKey.GetPublicKey(), + ownerKey.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + high_fee); + + // Room for the key change and either update, but not both + TemplatePositions(setup, {}); + const auto base_size{static_cast(*Assert(node::BlockAssembler::m_last_block_size))}; + const size_t service_size{std::max(::GetSerializeSize(CTransaction(tx_serv1), PROTOCOL_VERSION), + ::GetSerializeSize(CTransaction(tx_serv2), PROTOCOL_VERSION))}; + const size_t key_change_size{::GetSerializeSize(CTransaction(tx_key_change), PROTOCOL_VERSION)}; + const size_t block_max_size{base_size + key_change_size + service_size + 1}; + const auto positions = TemplatePositions(setup, {{tx_serv1, low_fee}, {tx_serv2, low_fee}, {tx_key_change, high_fee}}, + block_max_size); + BOOST_CHECK(positions.count(tx_key_change.GetHash())); + BOOST_CHECK(!positions.count(tx_serv1.GetHash()) && !positions.count(tx_serv2.GetHash())); + BOOST_CHECK_LT(static_cast(*Assert(node::BlockAssembler::m_last_block_size)), block_max_size); +} + +// A key change goes in without the service updates of its masternode A when they do not fit. A +// pending service update of another masternode B, funded by one of A's, cannot go in after that +// either, and must not hold back B's key change. +void FuncTestMinerKeyChangeNotHeldBackByOtherKeyChange(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()); }; + + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto utxos = BuildSimpleUtxoMap(setup.m_coinbase_txns); + const CScript scriptPayout = GenerateRandomAddress(); + + CKey ownerKeyA; + CKey ownerKeyB; + CBLSSecretKey operatorKeyA; + CBLSSecretKey operatorKeyB; + auto tx_reg_a = CreateProRegTx(chainman, utxos, /*port=*/1, scriptPayout, setup.coinbaseKey, ownerKeyA, operatorKeyA); + auto tx_reg_b = CreateProRegTx(chainman, utxos, /*port=*/2, scriptPayout, setup.coinbaseKey, ownerKeyB, operatorKeyB); + const uint256 proTxHashA = tx_reg_a.GetHash(); + const uint256 proTxHashB = tx_reg_b.GetHash(); + auto block = std::make_shared(setup.CreateBlock({tx_reg_a, tx_reg_b}, coinbase_pk, chainman.ActiveChainstate())); + BOOST_REQUIRE(chainman.ProcessNewBlock(block, true, nullptr)); + // Matures enough coinbase outputs to fund the transactions below + for (int i = 0; i < 5; ++i) { + setup.CreateAndProcessBlock({}, coinbase_pk); + } + dmnman.UpdatedBlockTip(tip_index()); + + CBLSSecretKey newOperatorKeyA; + CBLSSecretKey newOperatorKeyB; + newOperatorKeyA.MakeNewKey(); + newOperatorKeyB.MakeNewKey(); + const CAmount low_fee{1000}; + const auto version = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + std::vector serv_a; + for (uint16_t port = 3; port < 7; ++port) { + serv_a.push_back(CreateProUpServTx(chainman, utxos, proTxHashA, operatorKeyA, port, CScript(), + setup.coinbaseKey, version, low_fee)); + } + SimpleUTXOMap serv_a_coins{ + {COutPoint(serv_a[0].GetHash(), 0), Coin(serv_a[0].vout[0], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}}; + auto tx_serv_b = CreateProUpServTx(chainman, serv_a_coins, proTxHashB, operatorKeyB, /*port=*/7, CScript(), + setup.coinbaseKey, version, low_fee); + auto tx_key_change_a = CreateProUpRegTx(chainman, utxos, proTxHashA, ownerKeyA, newOperatorKeyA.GetPublicKey(), + ownerKeyA.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + /*fee=*/200000); + auto tx_key_change_b = CreateProUpRegTx(chainman, utxos, proTxHashB, ownerKeyB, newOperatorKeyB.GetPublicKey(), + ownerKeyB.GetPubKey().GetID(), scriptPayout, setup.coinbaseKey, version, + /*fee=*/100000); + + // Room for both key changes with B's update and its parent, but not for A's key change with all + // of A's updates + auto size = [](const CMutableTransaction& tx) { return ::GetSerializeSize(CTransaction(tx), PROTOCOL_VERSION); }; + TemplatePositions(setup, {}); + const auto base_size{static_cast(*Assert(node::BlockAssembler::m_last_block_size))}; + const size_t block_max_size{base_size + size(tx_key_change_a) + size(tx_key_change_b) + size(serv_a[0]) + + size(tx_serv_b) + 1}; + BOOST_REQUIRE_GT(size(serv_a[1]) + size(serv_a[2]) + size(serv_a[3]), size(tx_key_change_b) + size(tx_serv_b) + 1); + std::vector> txs; + for (const auto& tx : serv_a) { + txs.emplace_back(tx, low_fee); + } + txs.insert(txs.end(), {{tx_serv_b, low_fee}, {tx_key_change_a, 200000}, {tx_key_change_b, 100000}}); + const auto positions = TemplatePositions(setup, txs, block_max_size); + BOOST_CHECK(positions.count(tx_key_change_a.GetHash()) && positions.count(tx_key_change_b.GetHash())); + BOOST_CHECK(!positions.count(tx_serv_b.GetHash())); + for (const auto& tx : serv_a) { + BOOST_CHECK(!positions.count(tx.GetHash())); + } +} + void FuncVerifyDB(TestChainSetup& setup) { auto& chainman = *Assert(setup.m_node.chainman.get()); @@ -3679,6 +4278,66 @@ BOOST_AUTO_TEST_CASE(test_mempool_proreg_replacement_update_conflict) FuncTestMempoolProRegReplacementUpdateConflict(setup); } +BOOST_AUTO_TEST_CASE(test_miner_service_update_before_key_change) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateBeforeKeyChange(setup); +} + +BOOST_AUTO_TEST_CASE(test_mempool_rejects_unorderable_service_update) +{ + TestChainV19Setup setup; + FuncTestMempoolRejectsUnorderableServiceUpdate(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_service_update_before_reorged_key_change) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateBeforeReorgedKeyChange(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_service_update_before_key_change_surviving_reorg) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateBeforeKeyChangeSurvivingReorg(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_service_update_never_after_key_change) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateNeverAfterKeyChange(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_key_change_not_held_back_after_reorg) +{ + TestChainV19Setup setup; + FuncTestMinerKeyChangeNotHeldBackAfterReorg(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_service_update_with_parent_before_key_change) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateWithParentBeforeKeyChange(setup, /*limit_block_size=*/false); +} + +BOOST_AUTO_TEST_CASE(test_miner_key_change_without_room_for_service_update) +{ + TestChainV19Setup setup; + FuncTestMinerServiceUpdateWithParentBeforeKeyChange(setup, /*limit_block_size=*/true); +} + +BOOST_AUTO_TEST_CASE(test_miner_key_change_not_held_back_by_service_updates) +{ + TestChainV19Setup setup; + FuncTestMinerKeyChangeNotHeldBackByServiceUpdates(setup); +} + +BOOST_AUTO_TEST_CASE(test_miner_key_change_not_held_back_by_other_key_change) +{ + TestChainV19Setup setup; + FuncTestMinerKeyChangeNotHeldBackByOtherKeyChange(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/src/txmempool.cpp b/src/txmempool.cpp index 453a180f8444..6256810299dd 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1099,6 +1099,71 @@ void CTxMemPool::removeProTxKeyChangedConflicts(const CTransaction &tx, const ui } } +std::optional CTxMemPool::GetKeyChangeTarget(const CTxMemPoolEntry& entry) const +{ + AssertLockHeld(cs); + const CTransaction& tx{entry.GetTx()}; + uint256 proTxHash; + std::optional new_key; + if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) return std::nullopt; + proTxHash = opt_proTx->proTxHash; + new_key = opt_proTx->pubKeyOperator; + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) return std::nullopt; + proTxHash = opt_proTx->proTxHash; + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SHARED_REGISTRAR) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) return std::nullopt; + proTxHash = opt_proTx->proTxHash; + new_key = opt_proTx->pubKeyOperator; + } else { + return std::nullopt; + } + const auto dmn = m_dmnman.GetListAtChainTip().GetMN(proTxHash); + if (!dmn) { + return std::nullopt; + } + const bool changes_key{new_key ? dmn->pdmnState->pubKeyOperator != *new_key + : dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()}; + return changes_key ? std::make_optional(proTxHash) : std::nullopt; +} + +bool CTxMemPool::IsUnorderableServiceUpdate(const CTransaction& tx, const setEntries& ancestors) const +{ + AssertLockHeld(cs); + return tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE && + std::any_of(ancestors.begin(), ancestors.end(), [&](const txiter& it) EXCLUSIVE_LOCKS_REQUIRED(cs) { + AssertLockHeld(cs); + const CTransaction& ancestor{it->GetTx()}; + return GetKeyChangeTarget(*it).has_value() || + (ancestor.IsSpecialTxVersion() && + (ancestor.nType == TRANSACTION_ASSET_LOCK || ancestor.nType == TRANSACTION_ASSET_UNLOCK || + ancestor.nType == TRANSACTION_MNHF_SIGNAL)); + }); +} + +std::vector CTxMemPool::GetServiceUpdatesBeforeKeyChange(txiter key_change) const +{ + AssertLockHeld(cs); + const auto proTxHash = GetKeyChangeTarget(*key_change); + if (!proTxHash) { + return {}; + } + setEntries descendants; + CalculateDescendants(key_change, descendants); + std::vector updates; + for (auto its = mapProTxRefs.equal_range(*proTxHash); its.first != its.second; ++its.first) { + const auto it = mapTx.find(its.first->second); + if (it != mapTx.end() && it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_SERVICE && !descendants.count(it)) { + updates.push_back(it); + } + } + return updates; +} + void CTxMemPool::removeProTxVotingPayeeConflicts(const uint256& proTxHash, const CKeyID& keyIDVoting, uint16_t refType) { const CTxDestination voting_dest{PKHash(keyIDVoting)}; diff --git a/src/txmempool.h b/src/txmempool.h index 08f94c60320e..c2fea79b0c7f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -542,6 +542,19 @@ class CTxMemPool void removeProTxConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeExpiredAssetUnlock(int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); + /** If entry changes or revokes the operator key a masternode has at the chain tip, return that + * masternode. Decided against the current list rather than isKeyChangeProTx, which records the list + * at admission and goes stale when a reorg changes the key under a transaction staying in the pool. */ + std::optional GetKeyChangeTarget(const CTxMemPoolEntry& entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** If key_change changes or revokes a masternode's operator key, return that masternode's pending + * ProUpServTxs that do not descend from it. They are signed with the key being replaced, and mined + * after key_change in the same block they would restore the previous operator's service fields. */ + std::vector GetServiceUpdatesBeforeKeyChange(txiter key_change) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Is tx a ProUpServTx that block assembly could not reliably mine ahead of a key change of its + * masternode? That is one with a pending operator key change among its ancestors, which it can only + * be mined after, or with a pending asset lock or unlock or MNHF signal, whose own checks in block + * assembly could keep it, and the key change it was to precede, out of the block. */ + bool IsUnorderableServiceUpdate(const CTransaction& tx, const setEntries& ancestors) const EXCLUSIVE_LOCKS_REQUIRED(cs); bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); bool isSpent(const COutPoint& outpoint) const; diff --git a/src/validation.cpp b/src/validation.cpp index 4ad368546a03..f0a49ba580ff 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -427,6 +427,12 @@ void Chainstate::MaybeUpdateMempoolForReorg( AssertLockHeld(cs_main); AssertLockHeld(m_mempool->cs); + // The mempool checks provider transactions against the masternode list at the tip, which the + // block tip notifications move only after this. Move it now, so a transaction re-added below is + // not checked against the list of a disconnected block. + if (fAddToMempool && m_chain.Tip()) { + m_special_tx.UpdatedMNListTip(m_chain.Tip()); + } std::vector vHashUpdate; { // disconnectpool is ordered so that the front is the most recently-confirmed @@ -1106,7 +1112,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) const CBlockIndex* tip{m_active_chainstate.m_chain.Tip()}; if (!CheckSpecialTxForMempool(tx, GetSpecialTxRules(tip, m_active_chainstate.m_chainman), state)) return false; - if (m_pool.existsProviderTxConflict(tx)) { + if (m_pool.existsProviderTxConflict(tx) || m_pool.IsUnorderableServiceUpdate(tx, *ancestors)) { return state.Invalid(TxValidationResult::TX_CONFLICT, "protx-dup"); } diff --git a/test/functional/feature_masternode_shares.py b/test/functional/feature_masternode_shares.py index cbf4dc05ae5c..eb1d8b919e00 100755 --- a/test/functional/feature_masternode_shares.py +++ b/test/functional/feature_masternode_shares.py @@ -348,6 +348,38 @@ def test_voting_payee_conflict_eviction(self, node, protx_hash): assert_equal(confirmed_address, voting) assert pending_txid not in node.getrawmempool() + def test_service_update_before_key_change(self, node, protx_hash, port): + self.log.info("A pending service update is mined before an operator key change of its masternode") + + def mine(): + self.bump_mocktime(10 * 60 + 1) + return self.generate(node, 1, sync_fun=self.no_op)[0] + + def rotate(operator, fee): + prepared = node.protx("shared_update_registrar_prepare", protx_hash, operator, "", fee) + sigs = node.protx("shared_sign", prepared["tx"])["signatures"] + return node.protx("shared_combine", prepared["tx"], sigs, True) + + fees = [node.getnewaddress() for _ in range(3)] + node.sendmany("", {address: 1 for address in fees}) + mine() + old_key, new_key = node.bls("generate"), node.bls("generate") + # Start from an operator key this test controls + rotate(old_key["public"], fees.pop()) + mine() + + serv_txid = node.protx("update_service", protx_hash, [f"127.0.0.1:{p2p_port(port)}"], old_key["secret"], "", + fees.pop()) + rotation_txid = rotate(new_key["public"], fees.pop()) + # By fee alone the rotation would go first, and the service update after it would restore the + # previous operator's service and revive the masternode the rotation bans + node.prioritisetransaction(rotation_txid, COIN) + block_txs = node.getblock(mine())["tx"] + assert_greater_than(block_txs.index(rotation_txid), block_txs.index(serv_txid)) + state = node.protx("info", protx_hash)["state"] + assert_equal(state["pubKeyOperator"], new_key["public"]) + assert_greater_than(state["PoSeBanHeight"], 0) + def test_separate_participant_wallets(self): self.log.info("Eight separate wallets fund and authorize a shared masternode") node = self.nodes[0] @@ -1170,6 +1202,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_service_update_before_key_change(node, protx_hash4, port=5) self.log.info("Shared state survives a node restart") state_before_restart = node.protx("info", protx_hash4)["state"]