RFC: Cancelling waitNext calls in the IPC mining interface #33575

issue ryanofsky openend this issue on October 8, 2025
  1. ryanofsky commented at 4:20 pm on October 8, 2025: contributor

    It seems that the IPC mining interface needs to provide some way to cancel BlockTemplate::waitNext calls. This should not be hard to implement, but there are a few approaches that could be taken and it would be useful to know which would work best for callers. Context:


    Originally posted by @plebhash in #33554:

    the thing is, I still need to cater for incoming Sv2 CoinbaseOutputConstraints messages, and whenever that arrives, I need to make sure the BlockTemplate (or TemplateIpcClient as I’m aliasing on the rust code) that’s used to kickstart the next waitNext loops has the correct block_reserved_weight and max_additional_sigops as defined on the Sv2 message.


    Different ways I could think of doing this:

    1. Have BlockTemplate destructor cancel the waitNext call and have it return null right away.
    2. Add a new BlockTemplate::interruptWait() to be able to interrupt waitNext() without destroying the template.
    3. Make Mining::createNewBlock() cancel any pending waitNext() calls.
    4. Use Cap’n Proto’s cancellation mechanism. Rust code could just drop the waitNext() promise. C++ waitNext() method could have a new interfaces::Cancel argument that would let it ask if the request was cancelled and/or register a cancellation callback.

    If main use-cases for cancelling waitNext methods are when needing to specify changed BlockCreateOptions, and when shutting down, approach (1) could be sufficient, because in both of these cases the client will be discarding the BlockTemplate anyway.

    But maybe a new BlockTemplate::interruptWait() method (2) could be useful for other reasons and we should add that method too?

    Approach (3) making other IPC calls implicitly cancel pending waitNext() calls, seems like it could make mining interface more annoying to use, so I think it is probably not a good idea, but wanted to mention it as an alternative.

    Approach (4) could be interesting, and would be the most general approach but would be more work than other approaches to implement. (The implementation would need to modify the mp.Context PassField implementation to call Promise::attach on the promise it returns to be notified when the promise is discarded, as described https://capnproto.org/cxxrpc.html#cancellation and https://github.com/capnproto/capnproto/blob/v2/kjdoc/tour.md#cancellation)

  2. plebhash commented at 4:36 pm on October 8, 2025: none

    number 1 would be a bit prohibitive on the Sv2 implementations we’re working on

    we need to stop waitNext every time the coinbase output constratints change, but that doesn’t mean that a solution associatated with past constraints couldn’t still arrive

    so we still need to keep a reference to every “live” BlockTemplate in memory, at least until a chain tip update

    if we’re forced to call destroy just to stop waitNext, that would lose context and forbid us from submitting this potentially valid solution

  3. ryanofsky commented at 4:57 pm on October 8, 2025: contributor

    if we’re forced to call destroy just to stop waitNext, that would lose context and forbid us from submitting this potentially valid solution

    Makes sense. So it sounds like you would want a BlockTemplate::interruptWait() method?

  4. plebhash commented at 7:51 pm on October 8, 2025: none

    tbh option 3 feels like the lowest hanging fruit on our side, because it would require basically no changes from the current implementation (aside from the instantiation of new server threads, which is only a temporary workaround for the issues we’re trying to mitigate here).

    basically the arrival of CoinbaseOutputConstraints triggers the following:

    • breaking the loop that monitors for waitNext responses
    • instantiating a BlockTemplate via Mining::createNewBlock() under new coinbase constraints
    • instantiating a new dedicated server thread (which as stated above, is only a temporary workaround)
    • spawning a new loop to monitor for waitNext responses

    for option 2, I’d have to add some extra steps before breaking the loop, and perhaps some synchronization logic to make sure we do Mining::createNewBlock() and spawn the new loop only after BlockTemplate::interruptWait() has finished

    with option 3, the flow is pretty much the same to what we already have, and whenever the new waitNext loop is spawned, the dedicated thread is already going to be free


    but I do notice that Mining::createNewBlock() doesn’t take :Proxy.Context as a parameter, wouldn’t that be needed in order to implement option 3?

    (this is a very naive quesiton, perhaps it makes no sense at all)

  5. plebhash commented at 8:38 pm on October 8, 2025: none

    oh sorry, just saw the remark saying that option 3 could have undesired consequences, I overlooked that on my first reading!

    so yeah option 3 is not necessarily my go-to

    option 2 would slightly increase complexity on rust side, but it seems perfectly doable

    and also after contemplating option 4, it also seems like a low hanging fruit on our side (even though it seems more complex to implement on C++ side)

    that’s because we already drop the waitNext promise once a CoinbaseOutputConstraints arrive… so if that led to the server also cancelling execution and freeing up the thread for new calls, that’d make everything smooth

  6. ryanofsky commented at 9:24 pm on October 8, 2025: contributor

    Good catch: createNewBlock (and checkBlock below it) really should take an mp.Context parameter. Without it, it’s impossible to specify a worker thread for the calls to run on, so the calls wiill block the event loop thread while they execute, potentially affecting other calls and clients. This looks like another thing that should be fixed. But fixing it is more-or-less independent from implementing option 3. The createNewBlock implementation could be changed to cancel prior waitNext calls without needing to add a context parameter.

    I was a little surprised that from your perspective there might be significant difference between options 2 and 3. But maybe from your second comment the difference is not too significant. Option 2 should just require an extra block.interruptWait() call before the mining.createNewBlock() call. I should probably try to understand the rust client code and your description above a little better though. I’d think if the rust code is asynchronous, there should only be a single loop, and no need to break and spawn a new loop to handle waitNext responses. I would expect something like following very rough pseudocode:

     0block = None
     1next_block = mining.createNewBlock(create_options)
     2sv2_msg = sv2_recv()
     3while True:
     4    done = await wait([next_block, sv2_msg])
     5
     6    if next_block in done:
     7        block = (await next_block).result
     8        sv2_send(block)
     9        next_block = block.waitNext(threadmap.makeThread().result)
    10
    11    if sv2_msg in done:
    12        msg = await sv2_msg
    13        sv2_msg = sv2_recv()
    14
    15        if msg.type == "coinbase_constraints":
    16            create_options.sigops = msg.sigops
    17            if block: block.interruptWait()
    18            next_block = mining.createNewBlock(create_options)
    19        elif msg.type == "found_solution":
    20             mining.submitSolution(...)
    

    Hopefully this is not too far off base.

    I guess next steps here would be to implement options 1 and 2. It’s good to know option 4 has some appeal too, because that could be a followup, but it’s would be relatively complicated to implement on the server side at the moment.

  7. plebhash commented at 3:54 pm on October 9, 2025: none

    the current implementation has two concurrent loops:

    I guess in theory they could be unified into a single loop, however interruptWait would have to be called whenever some incoming Sv2 message arrived, not only if that was a CoinbaseOutputConstraints.

    that’s because under a single unified loop, the futures for incoming messages now would be “competing” with the waitNext promises, so we would drop the promise for waitNext whenever the Sv2 message arrived (for more details, see tokio::select!)

    so basically, every iteration of the loop would need to instantiate a brand new waitNext request (regardless of whether it’s under different coinbase constraints or not)… and since it cannot conflict with the previous one, we’d have to always call interruptWait before the loop restarts

    it would require some refactoring, but at least in theory, under my current mental model, should be doable!

  8. plebhash commented at 4:03 pm on October 9, 2025: none

    although I wonder about potentially undesirable round-trips that could be avoided by having separate loops?

    for example, every time there’s a new template, we send a NewTemplate message

    and for most Sv2 implementations that are on the receiving and of this message, we expect to get a Sv2 RequestTransactionData message right afterwards

    so every time there’s a new template, we’re essentially creating one “useless” waitNext request that’s bound to be discarded right afterwards (with the extra round-trip of interruptWait)


    by having two separate loops, we leave those concerns independent from eachother (with the penalty of having to coordinate the interruptWait with the re-spawns, on the relatively rare occasions in which it’s actually needed)

  9. ryanofsky commented at 4:51 pm on October 9, 2025: contributor

    re: #33575 (comment)

    I guess in theory they could be unified into a single loop, however interruptWait would have to be called whenever some incoming Sv2 message arrived, not only if that was a CoinbaseOutputConstraints.

    This is surprising to me, but it sounds like this justifies using two loops to avoid pointless interruptWait and waitNext churn.

    In python pseudocode above, waitNext promise is stored in the next_block variable and isn’t updated until the the promise resolves or there is a CoinbaseOutputConstraints message. But I could imagine rust code not letting the promise lifetime be managed as flexibly. Also I could image there being benefits to having two loops like maybe making it easier to write code that doesn’t await unnecessarily. I’m sure using two loops make sense, it’s just not what I might have expected naively.

  10. plebhash commented at 7:49 pm on October 9, 2025: none

    hmm thinking about it deeper, I guess I could simply save the waitNext future into a mut variable so it’s still available across the unified loop iterations, and then replace it whenever it either returns or is cancelled (via interruptWait) due to CoinbaseOutputConstraints

    so I wouldn’t say your original gut feeling is totally wrong!

    although I still appreciate the benefits of separating concerns into different loops… I guess I’ll experiment with both approaches whenever interruptWait becomes available and see which one leads to the best overall result vs tradeoffs

    but bottom line is that options 2, 3 and 4 are all perfectly viable solutions for our needs!


github-metadata-mirror

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: 2025-10-10 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me