Sjors
commented at 3:34 PM on November 21, 2025:
member
Implements a way to track the memory footprint of all non-mempool transactions that are still being referenced by block templates, see discussion in #33899. It does not impose a limit.
IPC clients can query this footprint (total, across all clients) using the getMemoryLoad() IPC method. Its client-side usage is demonstrated here:
Additionally, the functional test in interface_ipc.py is expanded to demonstrate how template memory management works: templates are not released until the client drops references to them, or calls the template destroy method, or disconnects. The destroy method is called automatically by clients using libmultiprocess, as sv2-tp does. In the Python tests it also happens when references are destroyed or go out of scope.
mining: track non-mempool memory usage: add TxTemplateMap to BlockTemplateManager to track how many templates contain any given transaction. This map is updated by the BlockTemplate constructor and destructor.
mining: add GetTemplateMemoryUsage() - loops over this map and sums up the memory footprint for transactions outside the mempool (includes a fuzzer)
ipc: add getMemoryLoad() expose this information to IPC clients and add test coverage
DrahtBot added the label Mining on Nov 21, 2025
DrahtBot
commented at 3:34 PM on November 21, 2025:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
#35847 (test: move more tests to baseindex_tests and run them for all indexes by mzumsande)
#35680 (private broadcast: bound memory use of broadcast attempts by instagibbs)
#35675 (mining: add block template manager by ismaelsadeeq)
#35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
#35569 (Encapsulation for CTransaction by purpleKarrot)
#35482 (fuzz: exercise the transaction-handling path in process_message(s) by HowHsu)
#35300 (mining: add precious option to IPC block submission by w0xlt)
#34995 (iwyu: Fix warnings in src/common and treat them as errors by hebasto)
#34803 (mempool: asynchronous mempool fee rate diagram updates via validation interface by ismaelsadeeq)
#34565 (refactor: extract BlockDownloadManager from PeerManagerImpl by w0xlt)
#34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation by ismaelsadeeq)
#32468 (rpc: generateblock to allow multiple outputs by polespinasa)
#31117 (miner: Reorg Testnet4 minimum difficulty blocks by fjahr)
#30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Sjors
commented at 3:36 PM on November 21, 2025:
member
I haven't benchmarked this yet on mainnet, so I'm not sure if checking every (unique) transaction for mempool presence is unacceptably expensive.
If people prefer, I could also add a way for the getblocktemplate RPC to opt-out of the memory bookkeeping, since it holds on to one template max and no longer than a minute.
Sjors force-pushed on Nov 21, 2025
DrahtBot added the label CI failed on Nov 21, 2025
DrahtBot
commented at 4:05 PM on November 21, 2025:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
š§ At least one of the CI tasks failed.
<sub>Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/19575422916/job/56059300316</sub>
<sub>LLM reason (āØ experimental): clang-tidy flagged fatal errors (loop variable copied for range-based for causing a warnings-as-errors failure) in interfaces.cpp, breaking the CI run.</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
in
src/node/interfaces.cpp:882
in
f22413f31foutdated
882 | {
883 | assert(m_block_template);
884 | + 885 | + TxTemplateMap& tx_refs{*Assert(m_tx_template_refs)}; 886 | + // Don't track the dummy coinbase, because it can be modified in-place 887 | + // by submitSolution()
ismaelsadeeq
commented at 4:22 PM on November 21, 2025:
member
Concept ACK
I think it would be better if we have internal memory management for the mining interface IPC, since we hold on to the block templates.
I would suggest the following approach:
Add memory budget for the mining interface.
Introduce a tracking list of recently built block templates and total memory usage.
Add templates to the list and increment the memory usage after every createnewblock or waitnext return.
Whenever the memory budget is exhausted, we should release templates in FIFO order.
I think since we create a new template after a time interval elapses even if fees increase and that interval is usually enough for the client to receive and distribute the template to miners, this mechanism should be safe as the miners have long switch to most recent template when the budget elapsed because of the time interval being used in between returns of waitnext.
Mining interface clients should also handle their own memory internally.
Currently, I donāt see much use for the exposed getMemoryLoad method. In my opinion, we should not rely on the IPC client to manage our memory.
Sjors
commented at 4:34 PM on November 21, 2025:
member
In my opinion, we should not rely on the IPC client to manage our memory.
Whenever the memory budget is exhausted, we should release templates in FIFO order
It seems counter intuitive, but from a memory management perspective IPC clients are treated no different than our own code. And if we started FIFO deleting templates that are used by our own code, we'd crash.
So I think FIFO deletion should be a last resort (not implemented here).
There's another reason why we should give clients an opportunity to gracefully release templates in whatever order they prefer. Maybe there's 100 downstream ASIC's, one of which is very slow at loading templates, so it's only given a new template when the tip changes, not when there's a fee change. In that scenario you have a specific template that the client wants to "defend" at all cost.
In practice I'm hoping none of this matters and we can pick and recommend defaults that make it unlikely to get close to a memory limit, other than during some weird token launch.
DrahtBot removed the label CI failed on Nov 21, 2025
ismaelsadeeq
commented at 5:38 PM on November 21, 2025:
member
It seems counter intuitive, but from a memory management perspective IPC clients are treated no different than our own code. And if we started FIFO deleting templates that are used by our own code, we'd crash.
IMHO I think we should separate that, and treat clients differently from our own code, because they are different codebases and separate applications with their own memory.
Maybe there are 100 downstream ASICs, one of which is very slow at loading templates, so itās only given a new template when the tip changes, not when thereās a fee change. In that scenario you have a specific template that the client wants to ādefendā at all costs.
I see your point but I donāt think thatās a realistic scenario, and I think we shouldnāt design software to be one-size-fits-all.
If you want to use only single block templates, then use createnewblock and create a new block template and mine that continuously until the chain tip changes or you mine a block.
waitNext returning indicates that we assume your miners are switching from the block they are currently mining to the new one they receive.
Depending on the budget (which I assume is large), many templates would need to be returned before we exhaust it.
Delegating template eviction responsibility to the client can put us in a situation where they handle it poorly and cause us to OOM (but I guess your argument is that we rather take that chance than being in a situation where we make miners potentially lose on rewards).
However I think if there is a clean separation of concerns between the Bitcoin Core node and its clients and clear interface definition and expectations that should not happen, and I believe the mining interface should not differ in that respect.
Otherwise, if we do want a one-size-fits-all solution capable of handling the scenario you described, we should rethink the design entirely and revert to an approach where we do not retain block templates.
Sjors
commented at 10:49 AM on November 24, 2025:
member
Delegating template eviction responsibility to the client can put us in a situation where they handle it poorly and cause us to OOM
Note that it's already the clients responsibility, that's inherent to how multiprocess works.
In the scenario where they handle it poorly, we can use FIFO deletion. All getMemoryLoad() does is give clients an opportunity to handle it better. If they're fine with FIFO, then they never have to call this method.
treat clients differently from our own code
We currently don't track whether any given CBlockTemplate is owned by an IPC client or by our internal code. Once we introduce FIFO deletion all call sites will have to check if it's been deleted since, or we need to exempt them from the memory accounting.
an approach where we do not retain block templates.
Afaik that means revalidating the block from scratch, removing one advantage the submitBlock() approach has over the submitblock RPC (I haven't benchmarked this though).
Sjors
commented at 4:56 PM on November 24, 2025:
member
I tracked the non-mempool transaction memory footprint for half a day on mainnet, using fairly aggressive template update criteria (minimum fee delta 1 sat and no more than once per second). So far the footprint is minuscule, but of course this depends on the mempool weather:
The memory spike after each new block is because sv2-tp holds on to templates from previous blocks for 10 seconds. Those ~3 MB spikes may look impressive, but keep in mind that the default mempool is 300 MB.
Sjors force-pushed on Nov 25, 2025
Sjors
commented at 3:56 PM on November 25, 2025:
member
I restructured the implementation and commits a bit.
The TxTemplateMap now lives on the NodeContext rather than MinerImpl (interface). This reflects the fact that we want to track the global memory footprint instead of per client. It's a lightweight member template_tx_refs which should be easy to fold into a block template manager later.
It's also less code churn because I don't have to touch the BlockTemplateImpl constructor.
It also made it easier to move GetTemplateMemoryUsage from interface.cpp to miner.cpp, where it's more reusable.
This in turn let me split out a separate commit that introduces the actual getMemoryLoad() interface method. So even if we decide against including that method, the rest of the PR should be useful. However I do think it's worth keeping, it's already been a helpful debugging and monitoring tool.
I added some comments to point out that we don't hold a mempool.cs lock during the calculation because we don't need an accurate result (mempool drift) and we don't want to bog down transaction relay with a potentially long lock (1-3ms in my testing so far).
Sjors force-pushed on Nov 25, 2025
Sjors
commented at 5:22 PM on November 25, 2025:
member
mining_getblocktemplate_longpoll.py triggered a stack-use-after-return, due to block_template being static (to allow template reuse between RPC calls). I added a commit d752dccaa56b663001d1bb29ab8b9a50628602a9 to move this longpoll template to the node context. This seems more appropriate anyway since BlockTemplate has a m_node member, so it shouldn't be able to outlive the node.
One caveat is that gbt_template has to be cleared before template_tx_refs, so I swapped them and added a comment (cde248a6613b6e37f7f7e35c1aabeb75347ffe95 -> 9c667c362a1639b48113a3657882b751f475082c.
Expanded the PR description.
DrahtBot added the label CI failed on Nov 25, 2025
Sjors force-pushed on Nov 25, 2025
DrahtBot removed the label CI failed on Nov 25, 2025
in
src/node/interfaces.cpp:883
in
ac1e97a592outdated
@brunoerg that's useful. So far interface_ipc.py never had a transaction appear in multiple templates, so the destructor would always remove the last reference.
I adjusted the test so that it does. Now your mutation causes a crash during this test.
brunoerg
commented at 6:06 AM on December 4, 2025:
Nice, thank you!
Sjors
commented at 8:19 AM on December 3, 2025:
member
Here's a slightly more realistic plot from last night on a well connected node running on an Intel i5-8400:
It's connected to DMND pool, declaring custom templates and getting them approved, but not actually mining. Due to their rate limiting I set -sv2interval=20, so if fees go up, it waits at least 20 seconds before generating a new template. It does not wait when the tip changes.
The machine also runs a lightning node and BTCPay so the moment block comes in the system is quite busy.
Sjors force-pushed on Dec 3, 2025
ryanofsky
commented at 9:36 PM on December 3, 2025:
contributor
Concept ACKe8f8f7f677bcde0179526be3ed9a657c44998b93. All the changes here seem good and mostly straightforward. The getMemoryLoad() function seems useful by itself and the underlying tracking would seem to provide almost everything needed to limit memory used by block templates.
I am a little concerned about the idea of proactively deleting block templates in FIFO order on behalf of clients, since it seems like this could increase complexity server-side, and client-side if clients have to deal with templates disappearing without being notified. Just not returning new templates after a certain amount of memory has been used would like a simpler approach.
Additionally, the functional test in interface_ipc.py is expanded to demonstrate how template memory management works: templates are not released until the client disconnects or calls the destroy() method.
Would be good if this said templates are also released if the python references are destroyed or go out of scope. (This stood out because I tested this yesterday in #33940 (comment).)
Sjors
commented at 10:46 AM on December 4, 2025:
member
Just not returning new templates after a certain amount of memory has been used would like a simpler approach.
It is, but refusing to make new templates doesn't stop the footprint of existing templates from growing. The worst case extra memory footprint for existing templates is the full size of the mempool.
This is rather unlikely though, it would only happen if between two blocks the entire mempool was gradually RBF'd in such a way that each transaction was at the top of the mempool briefly, and thus made it into a template.
Would be good if this said templates are also released
ryanofsky
commented at 5:37 PM on December 4, 2025:
In commit "rpc: move static block_template to node context" (a5eee29fd7d177f57c78da9773cb656a129de839)
I think it would actually be nice to move all these static variables to a struct or class like @ismaelsadeeq's BlockTemplateCache from #33421. But this could be a followup, and doesn't need to complicate this PR.
in
src/node/context.h:74
in
7c4d03d7b2outdated
68 | @@ -67,7 +69,11 @@ struct NodeContext {
69 | std::unique_ptr<AddrMan> addrman;
70 | std::unique_ptr<CConnman> connman;
71 | std::unique_ptr<CTxMemPool> mempool;
72 | - //! Cache latest getblocktemplate result for BIP 22 long polling 73 | + //! Track how many templates (which we hold on to on behalf of connected IPC 74 | + //! clients) are referencing each transaction. 75 | + TxTemplateMap template_tx_refs;
ryanofsky
commented at 5:55 PM on December 4, 2025:
In commit "mining: track non-mempool memory usage" (7c4d03d7b23417612fbca2f22e5bb1a198c9e5a2)
This map can updated from multiple threads, so it needs a mutex to be used safely. I think I'd suggest combining template_tx_refs and gbt_template variables and a mutex into single struct called something like BlockTemplateState and adding a unique_ptr to that struct as a member here. The struct could be replaced with a cache class in #33421.
To limit the scope of this PR, I only added the mutex, but called it template_state_mutex in anticipation.
in
test/functional/interface_ipc.py:218
in
e8f8f7f677outdated
216 | self.log.debug("Wait for another, but time out, since the fee threshold is set now")
217 | template7 = await template6.result.waitNext(ctx, waitoptions)
218 | assert_equal(template7.to_dict(), {})
219 |
220 | + self.log.debug("Memory load should be zero because there was no mempool churn") 221 | + with self.nodes[0].assert_debug_log(["Calculate template transaction reference memory footprint"]):
ryanofsky
commented at 6:11 PM on December 4, 2025:
In commit "ipc: add getMemoryLoad()" (e8f8f7f677bcde0179526be3ed9a657c44998b93)
Seems ok to assert this log message is logged, but I'm wondering if there was a particular reason for doing this. Was the idea to pair the LOG_TIME_MILLIS_WITH_CATEGORY and assert_debug_log calls together?
Would be good if this said templates are also released
Added a sentence to the PR description.
Sorry, I should have made a more specific suggestion. The problem is is that this sentence is not accurate: "templates are not released until the client disconnects or calls the destroy() method." Templates will be released if the client drops references to them, even if it never disconnects or calls destroy. I would just change it to "templates are not released until the client drops references to them, or calls the template destroy method, or disconnects"
Sjors force-pushed on Dec 5, 2025
Sjors force-pushed on Dec 5, 2025
DrahtBot added the label CI failed on Dec 5, 2025
Sjors force-pushed on Dec 5, 2025
DrahtBot removed the label CI failed on Dec 5, 2025
DrahtBot added the label Needs rebase on Dec 16, 2025
Sjors force-pushed on Dec 19, 2025
Sjors
commented at 10:25 AM on December 19, 2025:
member
Rebased after #34003. Dropped c548d6f0e8ecc0da6e29256c7085d48d10e10216test: destroy templates more carefully. That commit also added coverage for feeThreshold == MAX_MONEY, so I moved that into a new commit - not really related to this PR though.
DrahtBot removed the label Needs rebase on Dec 19, 2025
DrahtBot added the label Needs rebase on Jan 13, 2026
Sjors
commented at 9:54 AM on January 14, 2026:
member
DrahtBot removed the label CI failed on Feb 24, 2026
in
src/node/context.h:79
in
c37d715c1d
74 | + //! Track how many templates (which we hold on to on behalf of connected IPC 75 | + //! clients) are referencing each transaction. 76 | + TxTemplateMap template_tx_refs GUARDED_BY(template_state_mutex); 77 | + //! Cache latest getblocktemplate result for BIP 22 long polling. Must be cleared 78 | + //! before template_tx_refs. 79 | + std::unique_ptr<interfaces::BlockTemplate> gbt_template;
I think the comment warrants an elaboration. gbt_template is not explicitly cleared anywhere, so that happens at the destructor of NodeContext. struct members are destroyed in reverse order of their declaration. Is this comment intended to prevent swapping the declaration order of template_tx_refs and gbt_template?
Maybe:
//! Cache latest getblocktemplate result for BIP 22 long polling. Must be cleared
//! before template_tx_refs because the destructor of this decrements the count
//! in `template_tx_refs` of each transaction in the template. If it does not find
//! some of its transactions in `template_tx_refs` then it will abort.
Itās not only about declaration order. The intent is to destroy all BlockTemplate instances, including the one in gbt_result, while template_tx_refs is still alive so destructors can decrement reference counts. Once all templates are gone (and the map is empty), template_tx_refs can be destroyed.
in
src/node/interfaces.cpp:67
in
c37d715c1doutdated
How many entries will be supposedly stored in this map? std::map has lookup O(log(size)) whereas std::unordered_map has O(1). Here we do not need the entries to be ordered.
nit: start the comment with /** to make doxygen recognize it and attach it to the following code in the documentation.
Just mentioning - it is the same for map and unordered_map when used with shared_ptr (or CTransactionRef) - they compare pointers. So, two distinct objects that have the same values for all their members will be considered different. I am not sure if this is a problem in our code. Grepping the code for (map|set).*CTransactionRef, it looks like this will be the first case in non-test code where we use CTransactionRef as a key without providing custom comparator/hasher.
To illustrate with an explicit example:
CMutableTransaction mutable_tx;
// mutable_tx.vin = ...
// mutable_tx.vout = ...
CTransaction tx1{mutable_tx};
CTransaction tx2 = tx1; // tx2 is a copy of tx1, same transaction _logically_
CTransactionRef tx1_ref{MakeTransactionRef(tx1)};
CTransactionRef tx2_ref{MakeTransactionRef(tx2)};
std::map<CTransactionRef, int> m;
assert(m.emplace(tx1_ref, 5).second); // inserted
assert(m.emplace(tx2_ref, 6).second); // inserted
assert(m.size() == 2); // has 2 elements
Using a salted hash instead of barely using the first bytes of the transaction id as a hash because somebody may craft a pile of transactions with such ids as to make unordered_map lookup time deteriorate from O(1) to O(size). Not sure if that is an overkill in the current use case. In PrivateBroadcast::m_transactions I used the simpler:
because there 1. the transactions are originating locally (do not come from untrusted sources) and 2. the expectation is to store a small number of transactions, so even O(size) will not hog the machine.
I ended up using CTransactionRefSaltedHash. Unless performance is actually a problem, it's better to err on the side of caution.
It seems unlikely that someone is going to spend a fortune on grinding 64 bit mempool collisions to marginally slow this function down. But the transactions are not generated locally and with a combination bad mempool weather and a long block interval, the collection can be large.
Also, since we're exposing this function in src/util/hasher.h, where others might reuse it, it seems better to use a safe version.
Sjors referenced this in commit 33d0af5d71 on Mar 5, 2026
vasild approved
vasild
commented at 8:57 AM on March 6, 2026:
contributor
ACK04553cd6612f08d757fb20287fe111007bfb6db1
Would be good to figure out if #33922 (review) needs addressing.
Sjors referenced this in commit fa3eb29207 on Mar 6, 2026
Sjors referenced this in commit 202285bae3 on Mar 11, 2026
enirox001
commented at 10:52 AM on April 6, 2026:
contributor
ACK04553cd
Went through each commit:
6f12988 - forward declaration of BlockTemplate in context.h is necessary, mining.h is not in the include chain
b22e535 - destruction ordering of gbt_result before template_tx_refs looks correct. break in ~BlockTemplateImpl makes sense, and no lock inversion between template_state_mutex and cs_main
1d7497f - mempool.cs is intentionally not held for the full loop to avoid blocking relay, which is well documented
9f4b744 - locking looks consistent with commit 3, template_state_mutex guards the map while mempool.cs is acquired per exists() call
04553cd - MAX_MONEY is a good boundary case to have covered
Ran interface_ipc_mining.py and all tests passed successfully.
Sjors force-pushed on Apr 8, 2026
Sjors
commented at 9:32 AM on April 8, 2026:
member
<variant> was added in the last commit 10acf818ed70d4cd2af864278a35a6b64de0bb16refactor: disable default std::hash for CTransactionRef, but I think that it is not needed for the newly added code:
/** Disable default std::hash for CTransactionRef to prevent accidentally
* comparing by pointer. Use CTransactionRefSaltedHash or provide a custom
* hasher. */
template<>
struct std::hash<CTransactionRef> {
size_t operator()(const CTransactionRef&) const = delete;
};
My suggestion would be either commit 10acf818ed70d4cd2af864278a35a6b64de0bb16 (CI passed IIUC, so I think that counts as "tried"?) An alternative would be #35073 (review), which I presume was tried by the author that posted it. I am happy to push that suggestion to CI, if you want me to try it as well.
Unfortunately that leads to a reference to 'hash' is ambiguous (std::__1::hash vs std::hash). Let's see if b89942d29f1e71429070b2d1140d2792eb33bf47 fixes that.
10acf818ed70d4cd2af864278a35a6b64de0bb16 is of course a lot simpler, but also brittle because slightly different builds of IWYU will demand different includes (<variant> or <string_view>).
10acf81 is of course a lot simpler, but also brittle because slightly different builds of IWYU will demand different includes (<variant> or <string_view>).
Correct, but I think this is already true on current master: I think on current master a different build of IWYU will already yield a different result. I know it is not ideal, but I think the current approach is to let the CI spit out the diff when in doubt, then look if the diff has a backdoor or some nasty/wrong stuff, if not, just take the diff.
The problem with 10acf818ed70d4cd2af864278a35a6b64de0bb16 is that it only works on our CI. On e.g. my Ubuntu 25.10 with include-what-you-use 0.26, based clang version 22.1.1, that commit fails because it insists on <string_view>.
Ideally we shouldn't end up in the same situation as our linter job that can't be run outside a container.
(this code was changed, and also moved to #35101, so no longer relevant here)
DrahtBot removed the label CI failed on Apr 17, 2026
Sjors force-pushed on Apr 17, 2026
Sjors marked this as a draft on Apr 17, 2026
DrahtBot added the label CI failed on Apr 17, 2026
DrahtBot
commented at 3:25 PM on April 17, 2026:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
š§ At least one of the CI tasks failed.
<sub>Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/24570973945/job/71843744324</sub>
<sub>LLM reason (āØ experimental): CI failed due to a C++ build error: reference to 'hash' is ambiguous in primitives/transaction.h (conflict with std::hash).</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
Sjors force-pushed on Apr 17, 2026
Sjors
commented at 4:01 PM on April 17, 2026:
member
Waiting for CI to pass. I'm probably going to move 92fd54b259fadce8c14b8207d40431aef60a8c8d and b89942d29f1e71429070b2d1140d2792eb33bf47 in a separate PR. Need to address this too: #33922 (review)
DrahtBot removed the label CI failed on Apr 17, 2026
Sjors force-pushed on Apr 17, 2026
Sjors
commented at 7:49 PM on April 17, 2026:
member
The safety commit for CTransactionRef and IWYU changes are now in #35101 and dropped from this PR.
Sjors marked this as ready for review on Apr 17, 2026
vasild approved
vasild
commented at 10:04 AM on April 27, 2026:
contributor
Meanwhile #33421 was superseded by #35581, which I need to study in order to decide if I want to build on top of it or continue with the existing approach.
DrahtBot removed the label Needs rebase on Jun 24, 2026
Sjors force-pushed on Jun 24, 2026
Sjors
commented at 7:38 PM on June 24, 2026:
member
I opened #35598 and dropped 3cb6c4779e20f183347c3c1d63d8abb9d2e4c4eetest: cover feeThreshold = MAX_MONEY from this PR since it's unrelated and distracting.
I implemented the same functionality on top of #35581 in https://github.com/Sjors/bitcoin/pull/120. It fits well, although I still haven't studied PR 35581 itself. It also appears that I can drop 22f69a10df4afdc4c7b7d97844d8c245158f1ca0rpc: move static block_template to node context in that design.
If #35581 gets enough support from reviewers, I'll switch this PR over to it. That's preferable to merging this and then having to move things to the BlockTemplateManager later, especially since this change isn't urgent.
Kino1994 referenced this in commit 7abd0f1771 on Jun 28, 2026
DrahtBot added the label Needs rebase on Jul 7, 2026
Sjors force-pushed on Jul 8, 2026
Sjors
commented at 7:58 AM on July 8, 2026:
member
Expanded the new template manager fuzzer to cover GetTemplateMemoryUsage().
DrahtBot removed the label Needs rebase on Jul 8, 2026
Sjors force-pushed on Jul 8, 2026
Sjors force-pushed on Jul 16, 2026
DrahtBot added the label CI failed on Jul 16, 2026
DrahtBot
commented at 3:11 PM on July 16, 2026:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
š§ At least one of the CI tasks failed.
<sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/29504258847/job/87640797555</sub>
<sub>LLM reason (āØ experimental): CI failed because IWYU detected required include changes (failure generated from IWYU, causing the test script to exit non-zero).</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
Sjors force-pushed on Jul 16, 2026
DrahtBot removed the label CI failed on Jul 16, 2026
DrahtBot added the label Needs rebase on Jul 23, 2026
node: introduce BlockTemplateManager
Add BlockTemplateManager, a wrapper around
BlockAssembler::CreateNewBlock(), and store it in NodeContext.
Wire it into node init and test setup so it is reset before its
mempool/chainman dependencies, and update tests and fuzz setups that
rebuild chainman or mempool. Add a unit test that verifies a block
template can be created through the manager.
081dce6669
node: move mining_args to BlockTemplateManager
Pass the parsed mining args to BlockTemplateManager at construction
and expose them via BlockCreateArgs(), so the manager owns the
init-time block create options instead of NodeContext.
13d8264bfc
Sjors force-pushed on Jul 23, 2026
Sjors
commented at 5:37 PM on July 23, 2026:
member
DrahtBot added the label CI failed on Jul 23, 2026
DrahtBot
commented at 6:05 PM on July 23, 2026:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
š§ At least one of the CI tasks failed.
<sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/30030208529/job/89284620089</sub>
<sub>LLM reason (āØ experimental): CI failed because IWYU detected missing/incorrect includes (e.g., required txmempool.h) and aborted with āFailure generated from IWYU.ā</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
DrahtBot removed the label Needs rebase on Jul 23, 2026
DrahtBot removed the label CI failed on Jul 23, 2026
miner: move SubmitBlock into BlockTemplateManager
Move SubmitBlockStateCatcher and SubmitBlock from miner.cpp into
BlockTemplateManager as a member function. This groups block submission
with block creation in the same class. The function uses m_chainman
directly instead of taking it as a parameter.
24e410eb5e
node: move tip and wait helpers into BlockTemplateManager
Move the mining tip lookup and block-template waiting helpers (GetTip,
WaitTipChanged, WaitAndCreateNewBlock, InterruptWait,
CooldownIfHeadersAhead) into BlockTemplateManager so the manager owns
the template waiting flow. The manager now takes KernelNotifications
at construction.
5a6e2e6f57
interfaces: create block template via BlockTemplateManager
Route the Mining interface's createNewBlock() through
BlockTemplateManager::CreateNewTemplate() instead of constructing a
BlockAssembler directly. Merging the init-time defaults into unset
options now happens inside CreateNewTemplate(), so every caller gets
them applied.
96119f4f6b
rpc: route getblocktemplate internals through node
Add EnsureBlockTemplateManager() and use it in getblocktemplate for tip
lookup and longpoll waiting, while using ChainstateManager directly for
test-chain and IBD checks.
c7a67fb9ad
rpc: build getblocktemplate via BlockTemplateManagera742f21ccb
rpc: do not copy template data in getblocktemplate
The transactions, fees and sigops costs, and the coinbase outputs
are only read, so reference them from the template instead of copying
them out of it.
9795150b5a
rpc: only copy the header in getblocktemplate
The block copy exists solely to apply header adjustments (time, nonce,
version bits) that must not mutate the cached template. Now that the
transactions are read directly from the template, copy only the header
instead of the entire block.
2de5797788
rpc: build generation templates via BlockTemplateManager
generateblock is no longer safe for fuzzing: it previously threw at
EnsureMining (node.mining is never set in the fuzz setup), but with
EnsureBlockTemplateManager it now executes, mining and submitting a
real block. Submission mutates the chain state shared across fuzz
iterations and writes the block to disk.
99c7b8f9c7
rpc: wait for tips via BlockTemplateManager
The waitforblock, waitforblockheight and waitfornewblock RPCs are no
longer safe for fuzzing: they previously threw at EnsureMining
(node.mining is never set in the fuzz setup), but with
EnsureBlockTemplateManager they now really wait. The tip never changes
during fuzzing and nothing interrupts the wait, so a call without a
timeout blocks forever.
da99cab9bb
test: create templates via BlockTemplateManagerdabdf3db4d
node: remove NodeContext::mining and EnsureMiningd87c9e2eff
ci: enforce iwyu for block template manager83a6f49610
test: fuzz BlockTemplateManagera87d81548b
refactor: move CTransactionRefComp to util/hasher2fa2c9a512
mining: track non-mempool memory usage
IPC clients can hold on to block templates indefinately, which has the
same impact as when the node holds a shared pointer to the
CBlockTemplate. Because each template in turn tracks CTransactionRefs,
transactions that are removed from the mempool will not have
their memory cleared.
This commit adds bookkeeping to the block template constructor and
destructor that will let us track the resulting memory footprint.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
bf0d400356
mining: add GetTemplateMemoryUsage()
Calculate the non-mempool memory footprint for template transaction
references.
Add bench logging to collect data on whether caching or simplified
heuristics are needed, such as not checking for mempool presence.
Check the calculation in the block template manager fuzz test.
e169516461
ipc: add getMemoryLoad()
Allow IPC clients to inspect the amount of memory consumed by
non-mempool transactions in blocks.
Returns a MemoryLoad struct which can later be expanded to e.g.
include a limit.
Expand the interface_ipc.py test to demonstrate the behavior and
to illustrate how clients can call destroy() to reduce memory
pressure.
de2c7a8f97
Sjors force-pushed on Jul 24, 2026
Sjors referenced this in commit 0bec5d692f on Jul 24, 2026
DrahtBot added the label Needs rebase on Aug 4, 2026
DrahtBot
commented at 9:35 AM on August 4, 2026:
contributor
<!--cf906140f33d8803c4a75a2196329ecb-->
š This pull request conflicts with the target branch and needs rebase.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-08-11 10:51 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
