New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[peer] Refactor peer code into its own package #439

Closed
davecgh opened this Issue May 27, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@davecgh
Copy link
Member

davecgh commented May 27, 2015

The current peer code is a bit more tightly integrated than it should be. Ideally, it should be a separate sub-package named peer, akin to wire, txscript, addrmgr, and so on.

While the wire package handles all of the protocol level bits, this new package should handle the fundamental functions of a Bitcoin peer such as the initial version negotiation, pings, pongs, tracking of how many bytes have been sent and received, queuing of messages, etc. It should further provide the ability for the caller to register and unregister one or more listeners for each Bitcoin p2p message. This will pave the way for the package to be used in btcd, btcwallet for SPV support, and other cases where it could be useful.

The vast majority of this work will be a simple refactoring the current code into a separate package, however some new code will need to be added to deal with the ability to register listeners and for the message handlers themselves to invoke the registered listeners.

Once the peer package provides a fully functional peer with the fundamental functionality, btcd should be refactored to make use of the package by registering listeners for all of the appropriate messages and handling them as it currently does.

Here are a few guidelines for the new package:

  • The current peer type should move to the peer package and be exported as peer.Peer
  • The code needs to have test coverage added, but the new Peer type should not be an interface, rather the test code should use a fake net.Conn so the peer code believes it is talking to other real peers
  • The current architecture of 3 goroutines per peer, inHandler, queueHandler, and outHandler should be retained
  • The Peer type should continue to expose most of the current exported functions. In particular, String, AddKnownInventory, VersionKnown, ProtocolVersion, PushGetBlocksMsg, PushGetHeadersMsg, PushRejectMsg, QueueMessage, QueueInventory, Connected, Disconnect, Start, and Shutdown
  • Most of the handleXMsg functions, with the exception of the fundamental ones such as handleVersionMsg, handlePingMsg, and handlePongMsg, should be be removed from the peer type since those cases will now need to be handled via listeners
  • The code removed from the handlXMsgfunctions will need to be refactored into btcd proper via registered listeners
  • There will likely need to be some additional callbacks that aren't directly related to messages in order to deal with the melding of the peer providing the fundamental functionality along with allowing the caller to customize what happens. For example, the handleVersionMsg function has some code after the initial version negotiation phase that will need to be refactored into btcd proper
  • The services flag needs to made configurable since the intent of this package is to ultimately be usable by btcd (a full node), btcwallet (an SPV node), and other cases where it could be useful (where it won't be a full node either).

@tuxcanfly will be working on this.

@davecgh

This comment has been minimized.

Copy link
Member

davecgh commented Oct 28, 2015

This was implemented by #445.

@davecgh davecgh closed this Oct 28, 2015

dajohi pushed a commit to dajohi/btcd that referenced this issue Nov 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment