Stratum v2 Template Provider common functionality #30475

pull Sjors wants to merge 26 commits into bitcoin:master from Sjors:2024/07/sv2-tp-common changing 34 files +5815 −111
  1. Sjors commented at 3:07 pm on July 17, 2024: member

    Parent PR #29432.

    Based on:

    1. Noise encryption #29346
    2. Messages and transport layer #30315
    3. Sv2Connman #30332

    And the following interface changes:

    This contains all Template Provider functionality that can be used by both #29432 and the IPC based sidecar alternative in https://github.com/Sjors/bitcoin/pull/48.

  2. DrahtBot commented at 3:07 pm on July 17, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30443 (Introduce waitFeesChanged() mining interface by Sjors)
    • #30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
    • #30409 (Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals by Sjors)
    • #30205 (test: add mocked Sock that can read/write custom data and/or CNetMessages by vasild)
    • #30051 (crypto, refactor: add new KeyPair class by josibake)
    • #29775 (Testnet4 including PoW difficulty adjustment fix by fjahr)
    • #28241 (Silent payment index (for light wallets and consistency check) by Sjors)
    • #28201 (Silent Payments: sending by josibake)
    • #28122 (Silent Payments: Implement BIP352 by josibake)
    • #27433 (getblocktemplate improvements for segwit and sigops by Sjors)
    • #26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)
    • #26697 (logging: use bitset for categories by LarryRuane)

    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.

  3. DrahtBot added the label CI failed on Jul 17, 2024
  4. Sjors referenced this in commit 8c593f3ec4 on Jul 17, 2024
  5. Sjors force-pushed on Jul 18, 2024
  6. Sjors force-pushed on Jul 18, 2024
  7. Sjors referenced this in commit 97f3230c30 on Jul 18, 2024
  8. Sjors force-pushed on Jul 18, 2024
  9. Sjors referenced this in commit b5044653b2 on Jul 18, 2024
  10. DrahtBot added the label Needs rebase on Jul 18, 2024
  11. Add sv2 log category for Stratum v2 59eacd21cb
  12. Add sv2 noise protocol
    Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>
    f69fd623ad
  13. Add sv2 message CoinbaseOutputDataSize
    This commit adds the simplest stratum v2 message. The remaining messages are introduced in later commits.
    
    Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>
    176795aa1f
  14. Convert between Sv2NetMsg and CSerializedNetMsg
    This allows us to subclass Transport.
    c4f2ef9c74
  15. Introduce Sv2Transport
    Implemented starting from a copy of V2Transport and the V2TransportTester, modifying it to fit Stratum v2 and Noise Protocol requirements.
    
    Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>
    Co-Authored-By: Fi3
    938570a7ac
  16. Add sv2 SETUP_CONNECTION messages
    Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>
    750f75a227
  17. Add strings for Sv2MsgType fc81013ed0
  18. test: put the generic parts from StaticContentsSock into a separate class
    This allows reusing them in other mocked implementations.
    
    Also move the implementation (method definitions) to
    `test/util/net.cpp` to make the header `test/util/net.h` easier to follow.
    88e7fbdff4
  19. test: add a mocked Sock that allows inspecting what has been Send() to it
    And also allows gradually providing the data to be returned by `Recv()`
    and sending and receiving net messages (`CNetMessage`).
    d1e3e3bdaa
  20. Add Sv2Connman
    Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
    3cf779b0c0
  21. Chainparams: add default sv2 port
    Co-authored-by: Christopher Coverdale <chris.coverdale24@gmail.com>
    a1c8e3c6e3
  22. Stratum v2 template provider scaffold
    The template provider will listen for a Job Declarator client.
    It can establish a connection and detect various protocol errors.
    
    Co-Authored-By: Christopher Coverdale <chris.coverdale24@gmail.com>
    Co-Authored-By: Fi3
    fa33ea5073
  23. Add remaining sv2 messages for TemplateProvider 0ba8584d07
  24. Have createNewBlock return BlockTemplate interface
    An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data.
    
    This would be the case for a Stratum v2 Template Provider which needs to send a NewTemplate message (which doesn't include transactions) as quickly as possible.
    fb31d44606
  25. Add GetCoinBaseMerklePath helper 7e2a12feb6
  26. Add getCoinbaseMerklePath() to Mining interface 6489500b06
  27. Add submitSolution to BlockTemplate interface 6c36412cf5
  28. Add waitTipChanged to Mining interface 3fa41d133b
  29. Sv2: construct and submit block templates 2a9571f3df
  30. Handle REQUEST_TRANSACTION_DATA 0e1370c660
  31. Reject RequestTransactionData for stale templates 630cf57408
  32. Handle SUBMIT_SOLUTION 9328d23222
  33. Sjors force-pushed on Jul 19, 2024
  34. Introduce waitFeesChanged() mining interface bdfee73e1b
  35. Incrementally update sv2 block template e02477f624
  36. CKey: add Serialize and Unserialize
    Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
    2bb2838f40
  37. Persist static key for Template Provider dabc32cbe6
  38. Sjors force-pushed on Jul 19, 2024
  39. fanquake commented at 9:33 am on July 19, 2024: member
    Can you clarify what the point of PRs like this is, given you have a number of them open? They aren’t reviewable, or mergable (given they are all based on various PRs, which them selves are in various stages of draft/WIP), and the continual pushes keep taking up all the CI resources in the repo (I thought we’d adjusted the CI so that you can use your own repo?).
  40. Sjors referenced this in commit f951af069e on Jul 19, 2024
  41. DrahtBot removed the label Needs rebase on Jul 19, 2024
  42. Sjors commented at 10:15 am on July 19, 2024: member

    @fanquake there’s ongoing discussion in #29432 as to whether the Template Provider should be part of Bitcoin Core or be its own “sidecar” application. And the latter case there’s still discussion about how to do that, e.g. using ZMQ/RPC, p2p or IPC.

    I built a proof of concept for the IPC sidecar approach in https://github.com/Sjors/bitcoin/pull/48.

    It took me a while to iron out the interfaces, but with that (mostly) out of the way, I expect to need far fewer pushes than in the last few days.

    Although the PoC works, which was easier than I expected, at the moment the sidecar approach is not shippable. E.g. I can’t make reproducible binaries for it. It’ll be at least 8 months until we can potentially ship a multiprocess capable Bitcoin Core release.

    The other suggested approaches (RPC,ZMQ) don’t even have a proof-of-concept demo yet.

    So still consider #29432 to be plan A.

    Every PR I’ve opened is reviewable. They focus on different aspects of the Template Provider. Having them separate should aid in review, since someone interested in the Noise Protocol #29346 may know nothing about connman #30332.

    All interface PR’s should be mergeable. I needed to make sure they’re complete, which I think they are at this point. So I plan to mark #30443 and #30440 as ready for review soon.

    I thought we’d adjusted the CI so that you can use your own repo?

    That was intended for contributors to be able to build on top of #29432. I’ll also use my own CI for anything more experimental than the Template Provider.

    If the discussion in #29432 eventually settles on the sidecar approach (I don’t think it has yet), then it makes sense that I move this PR, #29346, #30315 and #30332 over to my own repo. The other PRs, e.g. #30443, would stay here in that scenario as well.

  43. fanquake commented at 10:51 am on July 19, 2024: member

    Every PR I’ve opened is reviewable.

    Sure, but it’s unlikely anyone is going to review something like this, that is itself based on 7 other PRs, 4 of which are also marked as draft. Even just due to the fact that any review is likely wasted, due to that same review already being done in any of the base PRs.

    If the discussion in #29432 eventually settles on the sidecar approach (I don’t think it has yet), then it makes sense that I move this PR, #29346, #30315 and #30332 over to my own repo.

    I think it would be good to let the discussion settle first, before opening all these PRs, just to (potentially) have to close / move them all again later.

  44. Sjors commented at 11:05 am on July 19, 2024: member

    I opened #29346, #30315 and #30332 months before the renewed discussion started. My approach in #29432 is no different from that of #27854 which has had multiple concept ACK’s from experienced developers for more than a year now. So I don’t think it was premature to open these sub PRs.

    Even just due to the fact that any review is likely wasted, due to that same review already being done in any of the base PRs.

    That’s always a risk when stacking PRs. So far it hasn’t happened. The description in each PR points to the base and parent. This method of splitting larger projects into stacked pull requests is not unique to Sv2.

    Whether the code here eventually ends up in Bitcoin Core or in another project, it’s still useful to get review on it, especially since people use it, including on mainnet (at tiny scale). That includes the TemplateProvider class that this PR introduces.

    I wrote:

    E.g. I can’t make reproducible binaries for it

    Looks like that is possible, see #30483. I’ll give that a try and see if people are able to figure it out.

  45. Sjors commented at 3:18 pm on July 19, 2024: member

    I’m looking into moving #30332 and this PR to my fork so reduce the stack a bit. However I keep getting seemingly spurious MSAN / TSAN failures which I’ve never seen here. So I’ll need to iron out some bugs in self-hosted CI first.

    See e.g. https://cirrus-ci.com/task/5819158305177600 for https://github.com/Sjors/bitcoin/pull/50 which is identical to #30332 (which passed CI).

    Trying to reproduce in isolation in Sjors/#51.

  46. Sjors commented at 7:12 pm on July 19, 2024: member

    Managed to fix the CI issue.

    Moving this PR to https://github.com/Sjors/bitcoin/pull/49 to reduce the PR stack here a bit and not overwhelm CI.

  47. Sjors closed this on Jul 19, 2024


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-10-18 04:12 UTC

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