[RFC] Post-0.9 network/protocol/main refactor #3465

Just writing this idea down, to get comments on it and let it materialize before we actually have time to implement it.

Currently, there is very weak separation of concerns between net.cpp, protocol.cpp and main.cpp. I believe there are 4 separate ones distinguishable:

• Management of I/O between sockets and buffers, and peer management (mostly net.cpp).
• Definition of the actual P2P messages, their (de)serialization, and perhaps some basic sanity checking on them (protocol.cpp, net.cpp and main.cpp).
• Processing and interaction of P2P messages with the validation engine (datastructures mostly in net, code mostly in main).
• Actual validation (main).

I'd very much like to see these as separate modules with well-defined responsibilities:

1. The "peer interaction" layer, which manages connections and network buffers, and does not expose any internal CNode data structure. It only allows writing to and reading from NodeId's, and management of the set of peers.
2. The bitcoin P2P layer, which uses serialize.h and core.h to serialize/deserialize p2p messages. It probably has methods for sending any type of P2P message to peers, and signals for processing every incoming message type.
3. The core validation engine (which manages the block tree, chain state, ..., but nothing peer-related). This has probably methods for processing a block or a transaction or a header (ProcessBlock, AcceptToMemoryPool, ...), and signals for notifying higher layers about blocks being connected/disconnected (probably with a distinction between internal changes, and public ones - the latter only trigger after a full succesful reorganization).
4. The core-p2p glue, which depends both on main and the p2p layer, and maintains per-peer (indexed by NodeId) data relevant to interaction (almost all of CNode's current fields).

For example:

• The set of banned IPs is in 1.
• The definition of the message block and computation of the checksum is in 2.
• Logic for connecting and disconnecting blocks is in 3.
• Per-peer DoS scores are in 4.
• IsInitialBlockDownload is in 4.

Open questions:

• Join 1 and 2? In Bitcoin's context they are unlikely to be swapped without swapping the other. However:
  • There could be different implementations of 1 for deeply different types of communication backend (from a local file, over freenet, from a satellite receiver, ...).
  • There could be a bitcoin protocol v2, mostly rewritten from scratch, that can still be relayed over different types of networks.
• Thread management. Where is the "controller" thread? If it is in 4, it means that other modules can not as easily plug into 2, as they become dependent on having a 4 present. On the other hand, making 2 the controller itself removes the knowledge about locking requirements in 4 from it.

Good points, agreed on the concept.

Especially separating out (3) and making it self-contained is important, as the validation part is what makes Bitcoin, and having it as a library would encourage diversity in clients. Other clients will want to have their own implementation of P2P network handling and the glue parts.

Re: questions

• Joining 1 and 2 makes sense. They could later be split if it becomes necessary, but planning forward too much for cases that aren't completely thought through is seldom constructive.
• Seems safe to assume that that 4 (the glue) is always present if 1+2 are present. No matter how the other modules change, there will always be some glue needed to connect the P2P network to the validation engine.

Indeed, clearly separating validation is a very nice goal on itself.

Agree on keeping 1 and 2 together initially.

Regarding 4 being present... what if I wanted to port bitcoin-seeder to run as a module using the Bitcoin reference codebase? It would use 1/2, but not 3/4.

3 and 4 have been separated in #9260.