refactor: Move node and wallet code out of src/interfaces #20494

pull ryanofsky wants to merge 3 commits into bitcoin:master from ryanofsky:pr/ipc-mv changing 4 files +302 −317
  1. ryanofsky commented at 2:39 PM on November 25, 2020: contributor

    This PR is part of the process separation project.

    Move NodeImpl from interfaces/node.cpp to node/interfaces.cpp Move ChainImpl from interfaces/chain.cpp to node/interfaces.cpp Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cpp

    No changes to any classes (can review with git diff --color-moved=dimmed_zebra)

    Motivation for this change is to move node and wallet code to respective directories where it might fit in better than src/interfaces/, but also to remove all unnecessary code from src/interfaces/ to unblock #19160 review, which has been hung up partially because of code organization. Building on top of this PR, #19160 should now be able to organize interface implementations more understandably in src/node/ src/wallet/ src/ipc/ and src/init/ directories instead of having so much functionality all in src/interfaces/

  2. Move NodeImpl from interfaces/node.cpp to node/interfaces.cpp 12bd0fc9d7
  3. Move ChainImpl from interfaces/chain.cpp to node/interfaces.cpp
    No changes to ChainImpl or any related classes (review with `git diff --color-moved=dimmed_zebra`)
    2a26771d81
  4. Move WalletImpl from interfaces/wallet.cpp to wallet/interfaces.cpp 629a9299b2
  5. fanquake added the label Refactoring on Nov 25, 2020
  6. ryanofsky added this to the "In progress" column in a project
  7. ryanofsky cross-referenced this on Nov 25, 2020 from issue multiprocess: Add basic spawn and IPC support by ryanofsky
  8. DrahtBot commented at 3:42 PM on November 25, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #20530 (lint, refactor: Update cppcheck linter to c++17 and improve explicit usage by fjahr)
    • #20172 (rpc, net: Expose connections_onion_only in getnetworkinfo RPC output by hebasto)
    • #19910 (net processing: Move peer_map to PeerManager by jnewbery)
    • #19901 (Avoid locking CTxMemPool::cs recursively in CTxMemPool::DynamicMemoryUsage() by hebasto)
    • #19771 (net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection by luke-jr)
    • #18766 (Disable fee estimation in blocksonly mode (by removing the fee estimates global) by darosior)
    • #17167 (Allow whitelisting outgoing connections by luke-jr)

    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.

  9. ryanofsky cross-referenced this on Nov 25, 2020 from issue Multiprocess bitcoin by ryanofsky
  10. ryanofsky cross-referenced this on Nov 25, 2020 from issue multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky
  11. ryanofsky cross-referenced this on Nov 25, 2020 from issue multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky
  12. DrahtBot cross-referenced this on Nov 25, 2020 from issue rpc, net: Expose connections_onion_only in getnetworkinfo RPC output by hebasto
  13. DrahtBot cross-referenced this on Nov 25, 2020 from issue net processing: Move peer_map to PeerManager by jnewbery
  14. DrahtBot cross-referenced this on Nov 25, 2020 from issue Avoid locking CTxMemPool::cs recursively in CTxMemPool::DynamicMemoryUsage() by hebasto
  15. DrahtBot cross-referenced this on Nov 25, 2020 from issue net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection by luke-jr
  16. DrahtBot cross-referenced this on Nov 25, 2020 from issue Disable fee estimation in blocksonly mode (by removing the fee estimates global) by darosior
  17. DrahtBot cross-referenced this on Nov 25, 2020 from issue Allow whitelisting outgoing connections by luke-jr
  18. promag commented at 12:02 AM on November 30, 2020: member

    Code review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255.

    This change is reasonable, interface implementations are moved to their "home".

  19. DrahtBot cross-referenced this on Nov 30, 2020 from issue lint, refactor: Update cppcheck linter to c++17 and improve explicit usage by fjahr
  20. in src/node/interfaces.cpp:7 in 2a26771d81 outdated 
       1 | @@ -2,47 +2,58 @@
   2 |  // Distributed under the MIT software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | -#include <interfaces/node.h>
   6 | -
   7 |  #include <addrdb.h>
   8 |  #include <banman.h>
   9 | +#include <boost/signals2/signal.hpp>
    MarcoFalke commented at 8:35 AM on December 1, 2020:

    why is this moved?

    ryanofsky commented at 2:23 PM on December 1, 2020:

    re: #20494 (review)

    why is this moved?

    Not sure. This came from #19160 where I was using IWYU a lot. I tried to manually sort system includes below project includes after running it, but I wasn't very diligent. It'd be more convenient if there was just a single include section, or an automatic include sorter

  21. MarcoFalke approved
  22. MarcoFalke commented at 8:39 AM on December 1, 2020: member

    review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255 🔺

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

review ACK 629a9299b2a7241a3fa7d597cb34abcbe1af9255 🔺
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgpKgv9FXDr3pKOUjSzUhQP8v6tY0NTOf/V0UP6izQZuoOkAxjFJq/FgXNAgnU7
NyX7VHnqx/1gf+GAOpKrcxvWderBde6fDPNHS+jcZo4aNsv15Vsqv/Fia9VaKw0h
kBkEzWm8N3hEktVdqTsZrGee26qD0jlJM64eW/Se9Kq3D6uQgyusEOJ5uiu6cmG4
ycXeRnTCs1PCneNlt/6hypLs9iSe5RUHhM8rmEFh6HLBmW0t+mpca7cOdGmZAGXp
R+rMTh8LeljiEL10I/v1pg0OYDOIJ7IQpERwrajiB4eWVgHbHmxqr1AVGikQDm36
StCw6QRCV0GzRPE963zpP5SvI0JxdOTTrTBzlKZqCMO5b1hoZJDq/9NEUIZQWIVq
4nYDDtyrQBqSunNAqEi1QAmEbt+IhGZR4v0yWGZHLLOuAXOAs+X5GbOwRzgPQeAC
rx9EawhmAxgH1tkSUScC3xyYgeN5GGosF9yX8xrocoQNKbILu1E8y1u1tt1Izjkx
SAS0x21D
=i4RY
-----END PGP SIGNATURE-----

    Timestamp of file with hash bfed3f19ad97ec6b521680bc88144a3f4a28fae8cda34c721b3183fddf8c449c -

    </details>

  23. MarcoFalke merged this on Dec 1, 2020
  24. MarcoFalke closed this on Dec 1, 2020
  25. sidhujag referenced this in commit ac0b2e1a3f on Dec 1, 2020
  26. hebasto cross-referenced this on Dec 7, 2020 from issue net: Add NAT-PMP port forwarding support by hebasto
  27. ryanofsky moved this from the "In progress" to the "Done" column in a project
  28. ryanofsky cross-referenced this on Apr 10, 2021 from issue interfaces: Expose settings.json methods to GUI by ryanofsky
  29. bitcoin locked this on Feb 15, 2022
Contributors
ryanofskyDrahtBotpromagMarcoFalke
Labels
Refactoring
Linked (view graph)
#10102 Multiprocess bitcoin#15936 interfaces: Expose settings.json methods to GUI#18077 net: Add NAT-PMP port forwarding support#19160 multiprocess: Add basic spawn and IPC support#19460 multiprocess: Add bitcoin-wallet -ipcconnect option#19461 multiprocess: Add bitcoin-gui -ipcconnect option#28722 Multiprocess tracking issue
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-05-20 06:54 UTC