Stratum v2 via IPC Mining Interface tracking issue #31098

issue Sjors openend this issue on October 16, 2024
  1. Sjors commented at 12:43 pm on October 16, 2024: member

    After much discussion in #29432 it appears there’s currently little support for directly supporting Stratum v2 in Bitcoin Core, with most contributors favouring the creation of a Mining IPC interface that external applications can use.

    The only such application currently is the Template Provider implemented by me in https://github.com/Sjors/bitcoin/pull/48. This also serves to illustrate how the interface is used and that it’s complete. The folks working on SRI should be able to build a Template Provider as well, once the interface is complete and shipped in a release.

    Needed for (hopefully) v29

    To complete the interface:

    For multiprocess release binaries

    Can wait for later releases

    Current Template Provider users

    As a stop-gap measure I plan to maintain https://github.com/Sjors/bitcoin/pull/68 which is a continuation of #29432, since people are currently using that. Once the Mining interface makes it into a release, along with multiprocess enabled Guix binaries, I plan to drop that in favour of https://github.com/Sjors/bitcoin/pull/48.

    Additionally I plan to convert the latter from a patch set on top of Bitcoin Core into a standalone application, to make it easier for others to contribute. That should make it less dependent on my own effort. The Bitcoin Core project could decide in the future to “adopt” this project, but that is entirely uncertain and most contributors seems averse to this idea.

  2. TheBlueMatt commented at 1:35 pm on October 16, 2024: contributor
    I’m really quite confused by this. Creating a “template IPC” interface is not substantially different than creating any other protocol for providing templates. What is the advantage for Bitcoin core to provide something proprietary over a standard that matches what consumers of that interface would want?
  3. TheBlueMatt commented at 2:27 pm on October 16, 2024: contributor

    My understanding of the conversation previously was (a) a desire to avoid a ton of TCP logic (which will largely be addressed by upcoming work to remove libevent’s HTTP server and presumably then unify the Connman logic across any TCP listeners), (b) a desire to use this as a way to get more multiprocess in Core (cool, but this is unrelated to whether Sv2 is supported in Core). Neither of those precludes or leads to the conclusion that “there’s currently little support for directly supporting Stratum v2 in Bitcoin Core”.

    I’d like to better understand the logic for building a new protocol (obligatory XKCD 927 referehce) here and what design considerations are going into that protocol design. Was consideration given to the goals of allowing regulated institutions to use third-party template providers with minimal restrictions? Presumably this new protocol needs a BIP and should go through normal protocol review.

    It’s also my understanding the the IPC logic in Bitcoin Core is not intended as a public API, and thus users of it are expected to exclusively be in Bitcoin Core, I suppose that is changing here?

  4. Sjors commented at 4:03 pm on October 16, 2024: member

    To start with the last point:

    It’s also my understanding the the IPC logic in Bitcoin Core is not intended as a public API, and thus users of it are expected to exclusively be in Bitcoin Core, I suppose that is changing here?

    In general IPC interfaces are not intended to be ~public~ stable, but the mining interface is an exception. @josibake is working on a proof-of-concept Rust demo that uses it, which should be fairly easy to use in SRI.

    Creating a “template IPC” interface is not substantially different than creating any other protocol for providing templates.

    The mining interface in interfaces/mining.h, node/interfaces.cpp and mining.capnp is orders of magnitude less code than what’s currently in https://github.com/Sjors/bitcoin/pull/68. That would be still be true after a refactor of Sv2Connman. It also moves all DoS-resistance responsibility to the user of this trusted interface.

    Neither of those precludes or leads to the conclusion that “there’s currently little support for directly supporting Stratum v2 in Bitcoin Core”.

    There are several concept NACKs on #29432 and earlier versions of it. There were a few concept ACKs from experienced developers on the first version in #29432, but they haven’t reviewed this work. The pace of review on the sub PRs for noise encryption #29346 / https://github.com/Sjors/bitcoin/pull/66, transport #30315 / https://github.com/Sjors/bitcoin/pull/67, connection management https://github.com/Sjors/bitcoin/pull/67 and the template provider application https://github.com/Sjors/bitcoin/pull/49 is glacial, it would take years at this pace to get them merged.

    In contrast review on the multiprocess approach has been fast.

    design considerations are going into that protocol design

    The Mining interface was designed by refactoring https://github.com/Sjors/bitcoin/pull/68 to use it. Additionally there’s a few methods that the RPC uses internally. There’s not much more to it.

    Was consideration given to the goals of allowing regulated institutions to use third-party template providers with minimal restrictions?

    Nothing changes here. Anyone can run bitcoin-node and connect a template provider application to it that listens on the public internet, either the one I wrote or a new one.

    Presumably this new protocol needs a BIP and should go through normal protocol review.

    I think you’re assuming here that this IPC is directly exposed to the untrusted internet? It’s really only supposed to be used by a trusted application that in turn can accept connections from the outside.

  5. TheBlueMatt commented at 5:04 pm on October 16, 2024: contributor

    The mining interface in interfaces/mining.h, node/interfaces.cpp and mining.capnp is orders of magnitude less code than what’s currently in https://github.com/Sjors/bitcoin/pull/68. That would be still be true after a refactor of Sv2Connman.

    I’m curious where the bulk of that is. It should be basically identical in terms of total volume (framing and such all has to happen either way, even if its auto-generated code when using capnproto), the protocol would be ~identical in function, and IPC vs TCP should be fairly similar (I assume its a socket interface either way). That just leaves the encryption on top which IIRC wasn’t a huge volume of code.

    It also moves all DoS-resistance responsibility to the user of this trusted interface.

    I’m curious what the worry is here - the protocol is almost entirely push-only, do we anticipate DoS is a material risk there? If there are material concerns here (eg too-large-coinbase-tx signaling pushing transactions out of the template attacks) they’ll simply not be addressed at all. The nice thing about having things in an active project is that people may actually look at it.

    There are several concept NACKs on #29432 and earlier versions of it. There were a few concept ACKs from experienced developers on the first version in #29432, but they haven’t reviewed this work. The pace of review on the sub PRs for noise encryption #29346 / https://github.com/Sjors/bitcoin/pull/66, transport #30315 / https://github.com/Sjors/bitcoin/pull/67, connection management https://github.com/Sjors/bitcoin/pull/67 and the template provider application https://github.com/Sjors/bitcoin/pull/49 is glacial, it would take years at this pace to get them merged.

    I understand things may be slow, but I’m curious why the conclusion there is that instead we should build a new protocol that accomplishes substantially the same thing but with fewer features. If the concern is that things like the encryption layer are moving too slow and we want to get something out it would be perfectly reasonable to ship a shorter-term patchset that doesn’t support that and consider adding support for it later. Same goes for TCP, even, we could consider adding a named-socket-only version that does the same framing but based on named sockets to get something available and slowly add the full protocol parts at a later time based on use.

    In contrast review on the multiprocess approach has been fast.

    Yes, developers have a tendency to prefer their own NIH thing, but that is fairly weak justification.

    Nothing changes here. Anyone can run bitcoin-node and connect a template provider application to it that listens on the public internet, either the one I wrote or a new one.

    That is a very substantial change - requiring an external application makes the value proposition of a common protocol that miners can connect to third-party nodes for templates much, much weaker. “Just turn this option on in bitcoin.conf” is a very different story from “also figure out how to reliably operate some unmaintained software that someone posted on github five years ago” (which is the current state of much of the solo-mining world).

    The Mining interface was designed by refactoring https://github.com/Sjors/bitcoin/pull/68 to use it. Additionally there’s a few methods that the RPC uses internally. There’s not much more to it.

    Sure, and there’s no much more to the Sv2 mining interface either :). Either way, I’m still kinda confused why it was decided to NIH a new protocol here.

    I think you’re assuming here that this IPC is directly exposed to the untrusted internet? It’s really only supposed to be used by a trusted application that in turn can accept connections from the outside.

    No, I am not. Exposing work coming out of Bitcoin Core is possibly the most important API from Bitcoin Core (outside of P2P and maybe sendrawtransaction) for the broader Bitcoin ecosystem. Defining a new API to do it haphazardly seems incredibly strange to me.

  6. TheBlueMatt commented at 5:06 pm on October 16, 2024: contributor
    Its probably worth pointing out that with new MEV-extraction things coming out these days Bitcoin Core will not be the only server of work, which means we also need to define new mining work-providing protocols in terms of the server-side behavior as well (not just “whatever Bitcoin Core does”, and not just whatever goes in its docs).
  7. Sjors commented at 8:31 am on October 17, 2024: member

    I’m curious where the bulk of that is.

    It’s literally in https://github.com/Sjors/bitcoin/pull/68, split between https://github.com/Sjors/bitcoin/pull/66, https://github.com/Sjors/bitcoin/pull/67, https://github.com/Sjors/bitcoin/pull/50 and https://github.com/Sjors/bitcoin/pull/49 in that order. I’ve already refactored that code to use the interface, just not via IPC, so it’s all additional. The first three PRs don’t use the interface at all.

    framing and such all has to happen either way, even if its auto-generated code when using capnproto

    I think you misunderstand the intended setup: the idea is to have a bitcoin-node application to which you connect via unix socket a bitcoin-stratum Template Provider application (https://github.com/Sjors/bitcoin/pull/68 or something based on SRI). That application then listens on the stratum port, uses noise encryption, etc.

    So it’s not either, or, it’s both. But the Bitcoin Core project would only have to maintain the cap&proto stuff - which it can use for other things, like separating the GUI from the node.

    do we anticipate DoS is a material risk there

    One obvious DoS issue is that as clients receive multiple templates the node holds on to transaction references even as the mempool drops them. I still have to open a followup issue to discuss that. One suggested solution is to have the node refuse to make new templates are some time, or give the client a way to measure memory footprint so it can choose which older templates to drop. That doesn’t have to be fully resolved on the Bitcoin Core end: it sees bitcoin-stratum as a trusted process, it will have to deal with the problem.

    More generally many historical bugs were in p2p code. Adding more p2p functionality adds more unknown unknowns.

    I understand things may be slow, but I’m curious why the conclusion there is that instead we should build a new protocol that accomplishes substantially the same thing but with fewer features.

    Hopefully I clarified above that this doesn’t replace stratum v2, it’s a layer in between. I’m able to reuse the same code for both the integrated and via-IPC Template Provider, so there’s no loss in functionality. The only big difference is division of responsibility for maintaining the functionality.

    The nice thing about having things in an active project is that people may actually look at it.

    That is a very substantial change - requiring an external application makes the value proposition of a common protocol that miners can connect to third-party nodes for templates much, much weaker.

    I agree with both statements, but I haven’t been able to convince others to take on that burden.

    we also need to define new mining work-providing protocols in terms of the server-side behavior as well

    I think Stratum v2 should remain the standard for that.

  8. TheBlueMatt commented at 1:15 pm on October 17, 2024: contributor

    It’s literally in https://github.com/Sjors/bitcoin/pull/68, split between https://github.com/Sjors/bitcoin/pull/66, https://github.com/Sjors/bitcoin/pull/67, https://github.com/Sjors/bitcoin/pull/50 and https://github.com/Sjors/bitcoin/pull/49 in that order. I’ve already refactored that code to use the interface, just not via IPC, so it’s all additional. The first three PRs don’t use the interface at all.

    I guess I was more curious what you think the bulk of the issue is, rather than just the bulk of the code. Do you think the lack of desire for review (compared to reviewing the capnproto protocol instead) comes from the encryption, the framing logic, the connman management, etc?

    I think you misunderstand the intended setup: the idea is to have a bitcoin-node application to which you connect via unix socket a bitcoin-stratum Template Provider application (https://github.com/Sjors/bitcoin/pull/68 or something based on SRI). That application then listens on the stratum port, uses noise encryption, etc.

    I understand that’s the goal, but why on earth would anyone actually implement a pool/miner that way? Bitcoin Core is defining a standard protocol to output work, forcing people to use a translation proxy to convert from that standard protocol to another standard protocol with basically equivalent messages is just latency, complexity, and possibility for failure. It’d be nice to have it listen on TCP, but that’s much better accomplished with netcat than some kind of protocol translation.

    One obvious DoS issue is that as clients receive multiple templates the node holds on to transaction references even as the mempool drops them. I still have to open a followup issue to discuss that. One suggested solution is to have the node refuse to make new templates are some time, or give the client a way to measure memory footprint so it can choose which older templates to drop. That doesn’t have to be fully resolved on the Bitcoin Core end: it sees bitcoin-stratum as a trusted process, it will have to deal with the problem.

    What’s wrong with just having a fixed timeout? As Bitcoin Core is now the thing responsible for deciding when a new template is created it is in full control of the memory used and can keep track of how much memory it is using. It could even use the flush bit to clear out old work if the mempool fee is climbing rapidly.

    More generally many historical bugs were in p2p code. Adding more p2p functionality adds more unknown unknowns.

    Yea, but that’s why this is an almost-push-only protocol, much fewer unknown unknowns :).

    Hopefully I clarified above that this doesn’t replace stratum v2, it’s a layer in between. I’m able to reuse the same code for both the integrated and via-IPC Template Provider, so there’s no loss in functionality. The only big difference is division of responsibility for maintaining the functionality. I think Stratum v2 should remain the standard for that.

    I strongly disagree, there’s a separate discussion on the Sv2 github (which I know you disagree with) on dropping the TP in favor of this new IPC TP, but I see zero reason why we should define a secondary Sv2-specific protocol for TP(ing) when there’s a perfectly fine one in Core. I’m not really a fan of the ecosystem standardizing a protocol around capnproto and everyone being forced to take a dependency on it (or maybe its encoding is simple enough that others can implement it by hand? not sure) and hope that this new protocol eventually gets exposed over TCP with cryptographic authentication, but even with those there’s no reason to have some Sv2-specific nonsense. Please let’s not have two nearly-identical protocols for the same thing, that’s insane.

  9. Sjors commented at 3:26 pm on October 21, 2024: member

    Do you think the lack of desire for review (compared to reviewing the capnproto protocol instead) comes from the encryption, the framing logic, the connman management, etc?

    The “instead” misses the mark: reviewing capnproto was already necessary to achieve all the other use cases of multiprocess. The mining interface just provided an easier opportunity to make progress there.

    Trying to redesign the interface in a way that makes it DoS-safe brings back the need for a lot of the review that not including https://github.com/Sjors/bitcoin/pull/68 is trying to avoid. Which in practice probably means the interface won’t get shipped for ages.


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: 2024-11-23 12:12 UTC

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