Chain interface: drying up and splitting it in multiple ones #17668

issue ariard opened this issue on December 4, 2019
  1. ariard commented at 7:38 PM on December 4, 2019: member

    Once #16426 merged, we should keep drying up the interface to make it fully asynchronously, few ideas to brainstorm on:

    • all getHeight, getBlockHeight and findFirstBlockWithTimeAndHeight-style methods should be removed once rescan logic moved in its own thread behind the interface (I have already a functional branch for this)
    • all mempool related methods (mempoolMinFee) could be moved in its own Mempool::interface, client could care only about fee estimation, make it asynchronous too
    • initMessage, initError and others could be moved in Node::interface
    • handleRpc and others, wallet should have its own RPC server

    I think last two issues could be started today, they don't rely on any other PRs.

    We may also have a Broadcast interface, tracking the chain state and broadcasting a tx are really 2 different logics. @ryanofsky pretty sure you have thoughts on it too.

  2. ariard added the label Feature on Dec 4, 2019
  3. fanquake added the label Brainstorming on Dec 4, 2019
  4. ryanofsky commented at 8:01 PM on December 4, 2019: contributor

    Things suggested above sound good to me. There's a TODO comment with similar suggestions.

    • initMessage, initError and others could be moved in Node::interface

    The interfaces::Node class is intended more for the gui and would give the wallet more functionality than it needs but I've previously thought these could go in a small interfaces::Ui or interfaces::WalletClient class along with the loadWallet notification (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253322273)

  5. MarcoFalke commented at 11:58 AM on August 3, 2022: member

    Is this still an issue?

  6. ariard commented at 2:15 PM on August 3, 2022: member

    @MarcoFalke

    From a quick read, I would say the suggestion about initMessage and initError is still relevant against today's code (a). The other one about isolating the rescan logic on the wallet-side is still relevant (b). About a "Mempool::interface" I think some information could be relevant in the recent discussions about mempool-code reorganizations (c). Wallet should have its own RPC server is already documented as a TODO in doc/multiprocess.md iirc (d). A new Broadcast or extended Chain interface would be relevant for AltNet (e).

    I don't have interest in doing a) or b) or d) in the coming future. I do have strong interest in c) and e) soon.

    From an issue-management standpoint, feel free to close the issue or re-dispatch the relevant informations elsewhere.

  7. MarcoFalke added the label Refactoring on Aug 3, 2022
  8. ryanofsky commented at 2:52 AM on August 6, 2022: contributor

    I think a lot of what's discussed here is still relevant, but there's not really a need to keep the issue open. I think any new ideas that we don't want to forget about could be added to the TODO comment. Otherwise I don't think there's any decisions that need to be discussed or ongoing development that needs to be tracked. The chain interface is already a lot simpler than it used to be after #16426, and it could just be broken up a little more.

  9. ryanofsky closed this on Aug 6, 2022

  10. bitcoin locked this on Aug 6, 2023

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: 2026-04-21 18:14 UTC

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