From 4f666b964083186588621e75716807da4a124e1b Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Aug 2026 00:11:19 -0500 Subject: [PATCH 1/4] fix(miner): validate special tx packages atomically --- src/evo/creditpool.cpp | 34 ++++++++++- src/evo/creditpool.h | 13 ++++- src/node/miner.cpp | 81 ++++++++++++-------------- src/node/miner.h | 4 -- src/test/evo_assetlocks_tests.cpp | 48 +++++++++++++++ test/functional/feature_asset_locks.py | 43 +++++++++++++- 6 files changed, 168 insertions(+), 55 deletions(-) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 5839b890305f..239b951a27d0 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -281,7 +281,7 @@ bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-lock-invalid"); } -bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state) +bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, std::optional* inserted_index) { uint64_t index{0}; CAmount toUnlock{0}; @@ -299,11 +299,13 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state) } newIndexes.insert(index); + if (inserted_index) *inserted_index = index; sessionUnlocked += toUnlock; return true; } -bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state) +bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state, + std::optional* inserted_index) { if (!tx.IsSpecialTxVersion()) return true; @@ -312,7 +314,7 @@ bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxVal case TRANSACTION_ASSET_LOCK: return Lock(tx, state); case TRANSACTION_ASSET_UNLOCK: - return Unlock(tx, state); + return Unlock(tx, state, inserted_index); default: return true; } @@ -322,6 +324,32 @@ bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxVal } } +bool CCreditPoolDiff::ProcessLockUnlockTransactions(const std::vector& txs, TxValidationState& state) +{ + const auto initialLocked = sessionLocked; + const auto initialUnlocked = sessionUnlocked; + std::vector insertedIndexes; + + for (const auto& tx : txs) { + std::optional inserted_index; + if (ProcessLockUnlockTransaction(*tx, state, &inserted_index)) { + if (inserted_index) insertedIndexes.push_back(*inserted_index); + continue; + } + + // Roll back exactly what this invocation changed: amounts are scalar + // snapshots, and only the indexes inserted above are erased so that + // state committed by earlier packages is left untouched. + for (const uint64_t index : insertedIndexes) { + newIndexes.erase(index); + } + sessionLocked = initialLocked; + sessionUnlocked = initialUnlocked; + return false; + } + return true; +} + std::optional GetCreditPoolDiffForBlock(CCreditPoolManager& cpoolman, const CBlock& block, const CBlockIndex* pindexPrev, const Consensus::Params& consensusParams, const CAmount blockSubsidy, BlockValidationState& state) diff --git a/src/evo/creditpool.h b/src/evo/creditpool.h index fec44ab680cb..610c193eac74 100644 --- a/src/evo/creditpool.h +++ b/src/evo/creditpool.h @@ -20,6 +20,7 @@ #include #include +#include class BlockValidationState; class CBlock; @@ -84,9 +85,17 @@ class CCreditPoolDiff { /** * This function should be called for each Asset Lock/Unlock tx * to change amount of credit pool + * @param inserted_index if non-null, receives the unlock index recorded for this tx (if any) * @return true if transaction can be included in this block */ - bool ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state); + bool ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state, + std::optional* inserted_index = nullptr); + + /** + * Process a package of Asset Lock/Unlock transactions atomically. + * @return true if all transactions can be included in this block + */ + bool ProcessLockUnlockTransactions(const std::vector& txs, TxValidationState& state); /** * this function returns total amount of credits for the next block @@ -101,7 +110,7 @@ class CCreditPoolDiff { private: bool Lock(const CTransaction& tx, TxValidationState& state); - bool Unlock(const CTransaction& tx, TxValidationState& state); + bool Unlock(const CTransaction& tx, TxValidationState& state, std::optional* inserted_index = nullptr); }; class CCreditPoolManager diff --git a/src/node/miner.cpp b/src/node/miner.cpp index c4ad5451d76b..f277240aa988 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -69,7 +69,6 @@ BlockAssembler::Options::Options() } BlockAssembler::BlockAssembler(Chainstate& chainstate, const NodeContext& node, const CTxMemPool* mempool, const Options& options) : - m_blockman(chainstate.m_blockman), m_chain_helper(chainstate.ChainHelper()), m_chainstate(chainstate), m_evoDb(*Assert(node.evodb)), @@ -77,8 +76,7 @@ BlockAssembler::BlockAssembler(Chainstate& chainstate, const NodeContext& node, m_clhandler(*Assert(node.clhandler)), chainparams(chainstate.m_chainman.GetParams()), m_mempool(mempool), - m_quorum_block_processor(*Assert(Assert(node.llmq_ctx)->quorum_block_processor)), - m_qman(*Assert(Assert(node.llmq_ctx)->qman)) + m_quorum_block_processor(*Assert(Assert(node.llmq_ctx)->quorum_block_processor)) { blockMinFeeRate = options.blockMinFeeRate; nBlockMaxSize = options.nBlockMaxSize; @@ -554,48 +552,6 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } } - if (creditPoolDiff != std::nullopt) { - // If one transaction is skipped due to limits, it is not a reason to interrupt - // whole process of adding transactions. - // `state` is local here because used only to log info about this specific tx - TxValidationState state; - - if (iter->GetTx().IsSpecialTxVersion() && iter->GetTx().nType == TRANSACTION_ASSET_UNLOCK) { - // ASSET_UNLOCK transactions may expire after being added to mempool - // They should not be included to the block - if (!CheckAssetUnlockTx(m_blockman, m_qman, iter->GetTx(), pindexPrev, creditPoolDiff->pool.indexes, state)) { - if (fUsingModified) { - mapModifiedTx.get().erase(modit); - failedTx.insert(iter); - } - LogPrintf("%s: asset unlock tx %s is skipped due %s\n", - __func__, iter->GetTx().GetHash().ToString(), state.ToString()); - continue; - } - } - if (!creditPoolDiff->ProcessLockUnlockTransaction(iter->GetTx(), state)) { - if (fUsingModified) { - mapModifiedTx.get().erase(modit); - failedTx.insert(iter); - } - LogPrintf("%s: asset-locks tx %s skipped due %s\n", - __func__, iter->GetTx().GetHash().ToString(), state.ToString()); - continue; - } - } - if (std::optional signal = extractEHFSignal(iter->GetTx()); signal != std::nullopt) { - if (signals.find(*signal) != signals.end()) { - if (fUsingModified) { - mapModifiedTx.get().erase(modit); - failedTx.insert(iter); - } - LogPrintf("%s: ehf signal tx %s skipped due to duplicate %d\n", - __func__, iter->GetTx().GetHash().ToString(), *signal); - continue; - } - signals.insert({*signal, 0}); - } - // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't // contain anything that is inBlock. assert(!inBlock.count(iter)); @@ -655,6 +611,41 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele std::vector sortedEntries; SortForBlock(ancestors, sortedEntries); + auto packageSignals = signals; + std::vector creditPoolTransactions; + bool validPackage{true}; + for (const auto& entry : sortedEntries) { + const auto& tx = entry->GetTx(); + 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__, + tx.GetHash().ToString(), *signal); + validPackage = false; + break; + } + } + if (tx.IsSpecialTxVersion() && (tx.nType == TRANSACTION_ASSET_LOCK || tx.nType == TRANSACTION_ASSET_UNLOCK)) { + creditPoolTransactions.emplace_back(entry->GetSharedTx()); + } + } + + if (validPackage && creditPoolDiff != std::nullopt && !creditPoolTransactions.empty()) { + TxValidationState state; + if (!creditPoolDiff->ProcessLockUnlockTransactions(creditPoolTransactions, state)) { + LogPrintf("%s: package tx %s skipped due to credit pool state: %s\n", __func__, + iter->GetTx().GetHash().ToString(), state.ToString()); + validPackage = false; + } + } + if (!validPackage) { + if (fUsingModified) { + mapModifiedTx.get().erase(modit); + failedTx.insert(iter); + } + continue; + } + signals = std::move(packageSignals); + for (size_t i = 0; i < sortedEntries.size(); ++i) { AddToBlock(sortedEntries[i]); // Erase from the modified set, if present diff --git a/src/node/miner.h b/src/node/miner.h index dfa24cfcb55d..dc64782c19bc 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -35,11 +35,9 @@ class ChainlockHandler; namespace Consensus { struct Params; }; namespace llmq { class CQuorumBlockProcessor; -class CQuorumManager; } // namespace llmq namespace node { -class BlockManager; struct NodeContext; static const bool DEFAULT_PRINTPRIORITY = false; @@ -167,7 +165,6 @@ class BlockAssembler int nHeight; int64_t m_lock_time_cutoff; - BlockManager& m_blockman; CChainstateHelper& m_chain_helper; Chainstate& m_chainstate; CEvoDB& m_evoDb; @@ -176,7 +173,6 @@ class BlockAssembler const CChainParams& chainparams; const CTxMemPool* const m_mempool; const llmq::CQuorumBlockProcessor& m_quorum_block_processor; - const llmq::CQuorumManager& m_qman; public: struct Options { diff --git a/src/test/evo_assetlocks_tests.cpp b/src/test/evo_assetlocks_tests.cpp index dd4144648acf..d029907256e0 100644 --- a/src/test/evo_assetlocks_tests.cpp +++ b/src/test/evo_assetlocks_tests.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -124,6 +125,16 @@ static CMutableTransaction CreateAssetUnlockTx(FillableSigningProvider& keystore return tx; } +static CTransactionRef CreateCreditPoolUnlockTx(uint64_t index, CAmount amount) +{ + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_ASSET_UNLOCK; + tx.vout.emplace_back(amount, CScript{}); + SetTxPayload(tx, CAssetUnlockPayload{1, index, 0, 0, {}, {}}); + return MakeTransactionRef(std::move(tx)); +} + BOOST_FIXTURE_TEST_SUITE(evo_assetlocks_tests, TestChain100Setup) static void CheckAssetLockCommon(uint8_t version, bool is_v24_active) @@ -507,4 +518,41 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup) } +BOOST_FIXTURE_TEST_CASE(credit_pool_package_atomicity, TestChain100Setup) +{ + LOCK(cs_main); + const auto unlock_seven = CreateCreditPoolUnlockTx(1, 7 * COIN); + const auto unlock_four = CreateCreditPoolUnlockTx(2, 4 * COIN); + const auto* tip = m_node.chainman->ActiveChain().Tip(); + CCreditPoolDiff diff{CCreditPool{100 * COIN, 10 * COIN}, tip, Params().GetConsensus(), 0}; + + TxValidationState package_state; + BOOST_CHECK(!diff.ProcessLockUnlockTransactions({unlock_seven, unlock_four}, package_state)); + BOOST_CHECK_EQUAL(package_state.GetRejectReason(), "failed-creditpool-unlock-too-much"); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 100 * COIN); + + TxValidationState retry_state; + BOOST_CHECK(diff.ProcessLockUnlockTransaction(*unlock_seven, retry_state)); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 93 * COIN); + + // A later package's rollback must erase only its own indexes: index 1 was + // committed above and must survive the failed package below. + const auto unlock_two = CreateCreditPoolUnlockTx(3, 2 * COIN); + const auto unlock_dup = CreateCreditPoolUnlockTx(1, COIN); + TxValidationState dup_state; + BOOST_CHECK(!diff.ProcessLockUnlockTransactions({unlock_two, unlock_dup}, dup_state)); + BOOST_CHECK_EQUAL(dup_state.GetRejectReason(), "failed-creditpool-unlock-duplicated-index"); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 93 * COIN); + + // index 1 must still be known as used... + TxValidationState still_dup_state; + BOOST_CHECK(!diff.ProcessLockUnlockTransactions({unlock_dup}, still_dup_state)); + BOOST_CHECK_EQUAL(still_dup_state.GetRejectReason(), "failed-creditpool-unlock-duplicated-index"); + + // ...while index 3 was rolled back and is usable again + TxValidationState reuse_state; + BOOST_CHECK(diff.ProcessLockUnlockTransactions({unlock_two}, reuse_state)); + BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 91 * COIN); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index 529b072b73a6..ae1821e4e481 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -748,13 +748,15 @@ def test_v24_fork(self, node_wallet, node, pubkey): self.mine_quorum_2_nodes() self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': False, 'reject-reason': 'bad-assetunlock-too-old-quorum'}) + self.test_admissible_asset_unlock_ancestor_package(node_wallet, pubkey) + asset_unlock_tx = self.create_assetunlock(620, 4000 * COIN + 1, pubkey) txid_in_block = self.send_tx(asset_unlock_tx) self.log.info(f"{txid_in_block} should not be mined") tip_hash = self.generate(node, 1)[0] assert txid_in_block not in node.getblock(tip_hash)['tx'] - asset_unlock_tx = self.create_assetunlock(621, 4000 * COIN, pubkey) + asset_unlock_tx = self.create_assetunlock(621, 3999 * COIN, pubkey) txid_in_block = self.send_tx(asset_unlock_tx) self.log.info(f"{txid_in_block} should be mined") tip_hash = self.generate(node, 1)[0] @@ -766,6 +768,45 @@ def test_v24_fork(self, node_wallet, node, pubkey): tip_hash = self.generate(node, 1)[0] assert txid_in_block not in node.getblock(tip_hash)['tx'] + self.test_asset_unlock_ancestor_package(node_wallet, asset_unlock_tx, txid_in_block) + + def create_asset_unlock_child(self, node_wallet, asset_unlock_tx, asset_unlock_txid): + child_value = Decimal(asset_unlock_tx.vout[0].nValue - tiny_amount) / COIN + child_hex = node_wallet.createrawtransaction( + [{'txid': asset_unlock_txid, 'vout': 0}], + {node_wallet.getnewaddress(): child_value}) + signed_child = node_wallet.signrawtransactionwithwallet(child_hex) + assert signed_child['complete'] + child_txid = node_wallet.sendrawtransaction(signed_child['hex']) + + # Ensure package selection considers the child before its Asset Unlock + # ancestor is considered on its own. + node_wallet.prioritisetransaction(child_txid, COIN) + return child_txid + + def test_admissible_asset_unlock_ancestor_package(self, node_wallet, pubkey): + self.log.info("Test an admissible Asset Unlock ancestor package") + asset_unlock_tx = self.create_assetunlock(619, COIN, pubkey) + asset_unlock_txid = self.send_tx(asset_unlock_tx) + child_txid = self.create_asset_unlock_child(node_wallet, asset_unlock_tx, asset_unlock_txid) + + template_txids = {tx_from_hex(tx['data']).rehash() for tx in node_wallet.getblocktemplate()['transactions']} + assert asset_unlock_txid in template_txids + assert child_txid in template_txids + + tip_hash = self.generate(node_wallet, 1)[0] + mined_txids = node_wallet.getblock(tip_hash)['tx'] + assert asset_unlock_txid in mined_txids + assert child_txid in mined_txids + + def test_asset_unlock_ancestor_package(self, node_wallet, asset_unlock_tx, asset_unlock_txid): + self.log.info("Test an Asset Unlock that exceeds the current limit as an ancestor package") + child_txid = self.create_asset_unlock_child(node_wallet, asset_unlock_tx, asset_unlock_txid) + + template_txids = {tx_from_hex(tx['data']).rehash() for tx in node_wallet.getblocktemplate()['transactions']} + assert asset_unlock_txid not in template_txids + assert child_txid not in template_txids + def test_asset_locks_v2_pre_v24(self, node_wallet, node, pubkey): self.log.info("Testing asset lock v2 rejection before v24 activation...") assert not softfork_active(node_wallet, 'v24') From 109c8f28a375dc78eaa0125dfade3f62d5eb8b5a Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Aug 2026 00:12:39 -0500 Subject: [PATCH 2/4] docs: add release note for #7570 --- doc/release-notes-7570.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 doc/release-notes-7570.md diff --git a/doc/release-notes-7570.md b/doc/release-notes-7570.md new file mode 100644 index 000000000000..463569f2be6b --- /dev/null +++ b/doc/release-notes-7570.md @@ -0,0 +1,5 @@ +Bug Fixes +--------- + +- Block template creation now skips Asset Unlock transaction packages that + exceed credit pool limits instead of failing to create a template. (#7570) From 90b8a3b448afc340fb431fdc3cc534f48934ffb2 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Aug 2026 13:47:07 -0500 Subject: [PATCH 3/4] fix(miner): preserve failed package streak --- src/node/miner.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index f277240aa988..1a2450de8e53 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -604,9 +604,6 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - // This transaction will make it in; reset the failed counter. - nConsecutiveFailed = 0; - // Package can be added. Sort the entries in a valid order. std::vector sortedEntries; SortForBlock(ancestors, sortedEntries); @@ -644,6 +641,9 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } continue; } + + // This transaction will make it in; reset the failed counter. + nConsecutiveFailed = 0; signals = std::move(packageSignals); for (size_t i = 0; i < sortedEntries.size(); ++i) { From 4886b9f7ef6d8a8f69e2842826b3e7ba8a9b1492 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Aug 2026 14:17:32 -0500 Subject: [PATCH 4/4] chore: address review feedback --- doc/release-notes-7570.md | 7 +++++-- src/test/evo_assetlocks_tests.cpp | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/release-notes-7570.md b/doc/release-notes-7570.md index 463569f2be6b..4147781d3abc 100644 --- a/doc/release-notes-7570.md +++ b/doc/release-notes-7570.md @@ -1,5 +1,8 @@ Bug Fixes --------- -- Block template creation now skips Asset Unlock transaction packages that - exceed credit pool limits instead of failing to create a template. (#7570) +- Block template creation now checks credit pool limits across complete + transaction packages. Because Asset Unlock limits are cumulative, a package + may exceed the block's limit even though its transactions were accepted + individually. Such packages are now skipped so miners can continue building + a template instead of template creation failing. (#7570) diff --git a/src/test/evo_assetlocks_tests.cpp b/src/test/evo_assetlocks_tests.cpp index d029907256e0..d3f7e9f41d9c 100644 --- a/src/test/evo_assetlocks_tests.cpp +++ b/src/test/evo_assetlocks_tests.cpp @@ -524,7 +524,7 @@ BOOST_FIXTURE_TEST_CASE(credit_pool_package_atomicity, TestChain100Setup) const auto unlock_seven = CreateCreditPoolUnlockTx(1, 7 * COIN); const auto unlock_four = CreateCreditPoolUnlockTx(2, 4 * COIN); const auto* tip = m_node.chainman->ActiveChain().Tip(); - CCreditPoolDiff diff{CCreditPool{100 * COIN, 10 * COIN}, tip, Params().GetConsensus(), 0}; + CCreditPoolDiff diff{CCreditPool{100 * COIN, 10 * COIN}, tip, m_node.chainman->GetConsensus(), 0}; TxValidationState package_state; BOOST_CHECK(!diff.ProcessLockUnlockTransactions({unlock_seven, unlock_four}, package_state));