From 89c40ecd13645ddc7234583ad9901e241e19cb73 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 7 Oct 2026 15:24:55 +0300 Subject: [PATCH 1/5] fix: move the masternode list tip before a reorg re-adds transactions to the mempool MaybeUpdateMempoolForReorg() re-accepts the transactions of disconnected blocks before the block tip notifications move the deterministic masternode list. The mempool checks provider transactions against the list at the tip, so meanwhile it sees the list of the disconnected block. A key change that block confirmed already shows its new operator key there and is no longer recorded as a key change when it returns to the mempool; validForProTxKey of returning updates records a key that is not current. Move the list tip to the new chain tip before re-adding transactions. It never moves ahead of the chain; the later tip notification moves it on to the final tip. The change was first proposed in #7728. Co-Authored-By: Claude Opus 5.5 --- src/evo/specialtxman.cpp | 2 ++ src/evo/specialtxman.h | 3 +++ src/validation.cpp | 6 ++++++ 3 files changed, 11 insertions(+) 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/validation.cpp b/src/validation.cpp index 4ad368546a03..9729ccfb70bc 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 From 0295a516b5f2d9648b2c33f0b73424bd950d6f3f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 7 Oct 2026 15:25:03 +0300 Subject: [PATCH 2/5] fix(mining): mine a pending ProUpServTx before a key change of its masternode CheckProUpServTx() verifies the operator signature against the masternode list of the previous block, and block connection applies provider transactions in block order without checking again who signed them. A ProUpServTx signed with the operator key that an earlier transaction in the same block changes or revokes still applies: it restores the previous operator's service fields and, after a key change, revives the masternode the change just PoSe-banned (#7582). The mempool accepts both transactions together and block assembly orders them by feerate, so a displaced operator can wait with a low-feerate service update for the key change to be mined. Mined in the other order both transactions are fine: the key change resets the operator fields the service update set. So instead of keeping either transaction out of the mempool, block assembly now mines the masternode's pending ProUpServTxs before the key change, together with their ancestors. One that cannot be in the block now, that cannot reliably be put first (see below), or whose package has a ProUpServTx of a masternode whose key already changed in the block, is left out; once the key change is in the block, no later package with a ProUpServTx of that masternode is taken, as whether one passes the package checks can change while the template is built (the ChainLock safety check depends on the clock). If those pulled in do not fit, they are all left out and the key change goes in without them: the mempool admits any number of independently funded service updates of a masternode, so requiring them to fit would let the operator being replaced hold the key change back indefinitely. A ProUpServTx cannot reliably be put ahead of a key change if a pending operator key change is among its ancestors, directly or through other pending transactions, as it can only be mined after it (for its own masternode it would restore the previous operator's service fields; for another masternode that change would have to come first), or a pending asset lock, asset unlock or MNHF signal, whose own checks in block assembly could keep the update, and the key change with it, out of the block. The mempool rejects such a ProUpServTx with "protx-dup"; it can only arrive after the transaction it spends, so every node decides the same way. A reorg can still return such an ancestor below a ProUpServTx already in the pool, and package acceptance (regtest-only) checks package members against the pool before adding any of them, so block assembly applies the same test to what it pulls in. Whether a transaction changes or revokes a masternode's operator key is decided against the masternode list at the tip rather than the flag recorded at admission, which goes stale when a reorg changes the key under a transaction staying in the pool. Co-Authored-By: Claude Opus 5.5 --- src/node/miner.cpp | 83 ++++++++++++++++++++++++++++++++++++++++++++-- src/txmempool.cpp | 65 ++++++++++++++++++++++++++++++++++++ src/txmempool.h | 13 ++++++++ src/validation.cpp | 2 +- 4 files changed, 160 insertions(+), 3 deletions(-) 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/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 9729ccfb70bc..f0a49ba580ff 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1112,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"); } From f2c84a89dd7b9ff048c1ff38c792b786a4d6dcdf Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 9 Oct 2026 13:45:25 +0300 Subject: [PATCH 3/5] test: block assembly mines a pending ProUpServTx before a key change of its masternode - A ProUpServTx goes in before a key change or a revocation of its masternode although they pay more; a registrar update keeping the operator key is ordered by fee as usual; a ProUpServTx that cannot be mined does not hold the key change back. - The mempool rejects a ProUpServTx spending an output of a pending key change, directly, through another transaction, or of a key change of another masternode, or an output of a pending asset lock, and accepts one independent of any of them. - A key change returned to the mempool by InvalidateBlock still counts as the masternode's pending key change, so a second one is rejected, and still puts a pending ProUpServTx before it. - A registrar update that kept the operator key when it entered the mempool, and became a key change when a reorg disconnected the block that set the key, still has a pending ProUpServTx placed before it. - A ProUpServTx that follows a key change of its masternode in the pool is never mined after it; one following a registrar update that keeps the key still is. - A reorg returning a zero-fee key change of another masternode below a pending ProUpServTx does not hold back a key change of the ProUpServTx's masternode; the ProUpServTx is left out of the block instead. - A ProUpServTx funded by an ordinary pending transaction goes in with it, parent first, before the key change. With room in the block for either the two of them or the key change but not all three, the key change goes in without the ProUpServTx and the template stays within its size limit. - Independently funded ProUpServTxs that fit ahead of a key change only one at a time do not hold it back; the key change goes in without them. - A key change of masternode A that goes in without A's ProUpServTxs does not hold back a key change of masternode B through a ProUpServTx of B funded by one of A's; that ProUpServTx is left out instead. CreateProUpServTx() and CreateProUpRevTx() take a fee, as CreateProUpRegTx() already does, so the templates hold transactions with real fees. Co-Authored-By: Claude Opus 5.5 --- src/test/evo_deterministicmns_tests.cpp | 663 +++++++++++++++++++++++- 1 file changed, 661 insertions(+), 2 deletions(-) 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) { From 7645a26dc559be6fb9521214351cb5416868fb65 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 7 Oct 2026 15:25:16 +0300 Subject: [PATCH 4/5] docs: add release note for mining service updates before key changes Co-Authored-By: Claude Opus 5.5 --- doc/release-notes-7857.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 doc/release-notes-7857.md 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) From 64b828d559464a929986c95a283460b0d6b84109 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 13 Oct 2026 12:50:48 -0500 Subject: [PATCH 5/5] test: mine a pending service update before a shared masternode's key change The unit tests cover ProUpRegTx and ProUpRevTx key changes, but not the ProUpSharedRegTx branch of CTxMemPool::GetKeyChangeTarget(). Run the ordering check for a shared masternode on a generated block: a service update signed with the operator key that a pending shared registrar update replaces is mined first, even when the rotation is prioritised above it, and the masternode ends the block banned under the new key. Without the block assembly change the rotation is mined first and the test fails on the ordering check. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_masternode_shares.py | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) 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"]