From 34df4ccbb1abe0cb485416ab7aec87ba636f3b78 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Mon, 29 May 2023 09:01:06 -0500 Subject: [PATCH] Make `ControlMessageHandler` take `PeerManager` rather than `Node` as a param (#5081) * Make ControlMessageHandler take PeerManager rather than Node as a param * refactor PeerData to not take a reference to Node * Move ControlMessageHandler out of {Node,NeutrinoNode} --- .../node/networking/P2PClientActorTest.scala | 10 ++++---- .../org/bitcoins/node/NeutrinoNode.scala | 7 +----- .../main/scala/org/bitcoins/node/Node.scala | 4 ---- .../scala/org/bitcoins/node/PeerData.scala | 18 ++++++++------- .../scala/org/bitcoins/node/PeerFinder.scala | 12 +++++++--- .../scala/org/bitcoins/node/PeerManager.scala | 3 ++- .../peer/ControlMessageHandler.scala | 23 +++++++++++-------- .../bitcoins/testkit/node/NodeUnitTest.scala | 9 ++++++-- 8 files changed, 48 insertions(+), 38 deletions(-) diff --git a/node-test/src/test/scala/org/bitcoins/node/networking/P2PClientActorTest.scala b/node-test/src/test/scala/org/bitcoins/node/networking/P2PClientActorTest.scala index ed9b0a1f41..4b40ee861b 100644 --- a/node-test/src/test/scala/org/bitcoins/node/networking/P2PClientActorTest.scala +++ b/node-test/src/test/scala/org/bitcoins/node/networking/P2PClientActorTest.scala @@ -6,6 +6,7 @@ import org.bitcoins.node.config.NodeAppConfig import org.bitcoins.node.models.Peer import org.bitcoins.node.networking.P2PClient.ConnectCommand import org.bitcoins.node.networking.peer.{ + ControlMessageHandler, PeerMessageReceiver, PeerMessageReceiverState } @@ -103,10 +104,11 @@ class P2PClientActorTest //piggy back off of node infra to setup p2p clients, but don't actually use //the node itself so stop it here an clean up resources allocated by it _ <- node.stop() - } yield PeerMessageReceiver( - controlMessageHandler = node.controlMessageHandler, - dataMessageHandler = node.peerManager.getDataMessageHandler, - peer = peer) + controlMessageHandler = ControlMessageHandler(node.peerManager) + } yield PeerMessageReceiver(controlMessageHandler = controlMessageHandler, + dataMessageHandler = + node.peerManager.getDataMessageHandler, + peer = peer) val clientActorF: Future[TestActorRef[P2PClientActor]] = peerMessageReceiverF.map { peerMsgRecv => diff --git a/node/src/main/scala/org/bitcoins/node/NeutrinoNode.scala b/node/src/main/scala/org/bitcoins/node/NeutrinoNode.scala index be46830b06..6c2fd8d2ac 100644 --- a/node/src/main/scala/org/bitcoins/node/NeutrinoNode.scala +++ b/node/src/main/scala/org/bitcoins/node/NeutrinoNode.scala @@ -15,10 +15,7 @@ import org.bitcoins.core.p2p.ServiceIdentifier import org.bitcoins.core.protocol.BlockStamp import org.bitcoins.node.config.NodeAppConfig import org.bitcoins.node.models.Peer -import org.bitcoins.node.networking.peer.{ - ControlMessageHandler, - DataMessageHandlerState -} +import org.bitcoins.node.networking.peer.{DataMessageHandlerState} import java.time.Instant import scala.concurrent.Future @@ -41,8 +38,6 @@ case class NeutrinoNode( implicit override def chainAppConfig: ChainAppConfig = chainConfig - val controlMessageHandler: ControlMessageHandler = ControlMessageHandler(this) - override lazy val peerManager: PeerManager = PeerManager(paramPeers, this, walletCreationTimeOpt) diff --git a/node/src/main/scala/org/bitcoins/node/Node.scala b/node/src/main/scala/org/bitcoins/node/Node.scala index 5f801968c8..1a2090463e 100644 --- a/node/src/main/scala/org/bitcoins/node/Node.scala +++ b/node/src/main/scala/org/bitcoins/node/Node.scala @@ -21,7 +21,6 @@ import org.bitcoins.node.networking.peer.DataMessageHandlerState.{ MisbehavingPeer } import org.bitcoins.node.networking.peer.{ - ControlMessageHandler, PeerMessageSender, SyncDataMessageHandlerState } @@ -42,9 +41,6 @@ trait Node extends NodeApi with ChainQueryApi with P2PLogger { implicit def executionContext: ExecutionContext = system.dispatcher def peerManager: PeerManager - - def controlMessageHandler: ControlMessageHandler - def nodeCallbacks: NodeCallbacks = nodeAppConfig.callBacks lazy val txDAO: BroadcastAbleTransactionDAO = BroadcastAbleTransactionDAO() diff --git a/node/src/main/scala/org/bitcoins/node/PeerData.scala b/node/src/main/scala/org/bitcoins/node/PeerData.scala index dcd3f40388..be98fdcf57 100644 --- a/node/src/main/scala/org/bitcoins/node/PeerData.scala +++ b/node/src/main/scala/org/bitcoins/node/PeerData.scala @@ -7,6 +7,7 @@ import org.bitcoins.node.config.NodeAppConfig import org.bitcoins.node.models.Peer import org.bitcoins.node.networking.P2PClient import org.bitcoins.node.networking.peer.{ + ControlMessageHandler, PeerMessageReceiver, PeerMessageReceiverState, PeerMessageSender @@ -19,7 +20,8 @@ import scala.concurrent.duration.DurationInt */ case class PeerData( peer: Peer, - node: Node, + controlMessageHandler: ControlMessageHandler, + peerManager: PeerManager, supervisor: ActorRef )(implicit system: ActorSystem, @@ -33,18 +35,18 @@ case class PeerData( private lazy val client: Future[P2PClient] = { val peerMessageReceiver = - PeerMessageReceiver(node.controlMessageHandler, - node.peerManager.getDataMessageHandler, + PeerMessageReceiver(controlMessageHandler, + peerManager.getDataMessageHandler, peer) P2PClient( peer = peer, peerMessageReceiver = peerMessageReceiver, peerMsgRecvState = PeerMessageReceiverState.fresh(), - onReconnect = node.peerManager.onReconnect, - onStop = node.peerManager.onP2PClientStopped, - onInitializationTimeout = node.peerManager.onInitializationTimeout, - node.peerManager.onQueryTimeout, - node.peerManager.sendResponseTimeout, + onReconnect = peerManager.onReconnect, + onStop = peerManager.onP2PClientStopped, + onInitializationTimeout = peerManager.onInitializationTimeout, + onQueryTimeout = peerManager.onQueryTimeout, + sendResponseTimeout = peerManager.sendResponseTimeout, maxReconnectionTries = 4, supervisor = supervisor ) diff --git a/node/src/main/scala/org/bitcoins/node/PeerFinder.scala b/node/src/main/scala/org/bitcoins/node/PeerFinder.scala index d91071c22a..0d3454cba8 100644 --- a/node/src/main/scala/org/bitcoins/node/PeerFinder.scala +++ b/node/src/main/scala/org/bitcoins/node/PeerFinder.scala @@ -7,6 +7,7 @@ import org.bitcoins.core.p2p.ServiceIdentifier import org.bitcoins.core.util.{NetworkUtil, StartStopAsync} import org.bitcoins.node.config.NodeAppConfig import org.bitcoins.node.models.{Peer, PeerDAO, PeerDb} +import org.bitcoins.node.networking.peer.ControlMessageHandler import java.net.{InetAddress, UnknownHostException} import java.util.concurrent.atomic.AtomicBoolean @@ -18,7 +19,8 @@ import scala.util.{Failure, Random, Success} case class PeerFinder( paramPeers: Vector[Peer], - node: NeutrinoNode, + controlMessageHandler: ControlMessageHandler, + peerManager: PeerManager, skipPeers: () => Vector[Peer], supervisor: ActorRef)(implicit ec: ExecutionContext, @@ -216,12 +218,16 @@ case class PeerFinder( /** creates and initialises a new test peer */ private def tryPeer(peer: Peer): Future[Unit] = { - _peerData.put(peer, PeerData(peer, node, supervisor)) + _peerData.put( + peer, + PeerData(peer, controlMessageHandler, peerManager, supervisor)) _peerData(peer).peerMessageSender.map(_.connect()) } private def tryToReconnectPeer(peer: Peer): Future[Unit] = { - _peerData.put(peer, PeerData(peer, node, supervisor)) + _peerData.put( + peer, + PeerData(peer, controlMessageHandler, peerManager, supervisor)) _peerData(peer).peerMessageSender.map(_.reconnect()) } diff --git a/node/src/main/scala/org/bitcoins/node/PeerManager.scala b/node/src/main/scala/org/bitcoins/node/PeerManager.scala index ba83b830da..eea6eaa58b 100644 --- a/node/src/main/scala/org/bitcoins/node/PeerManager.scala +++ b/node/src/main/scala/org/bitcoins/node/PeerManager.scala @@ -65,7 +65,8 @@ case class PeerManager( private val finder: PeerFinder = PeerFinder(paramPeers = paramPeers, - node = node, + controlMessageHandler = ControlMessageHandler(this), + peerManager = this, skipPeers = () => peers, supervisor = supervisor) diff --git a/node/src/main/scala/org/bitcoins/node/networking/peer/ControlMessageHandler.scala b/node/src/main/scala/org/bitcoins/node/networking/peer/ControlMessageHandler.scala index 027ee35c0a..62c9a24a43 100644 --- a/node/src/main/scala/org/bitcoins/node/networking/peer/ControlMessageHandler.scala +++ b/node/src/main/scala/org/bitcoins/node/networking/peer/ControlMessageHandler.scala @@ -2,14 +2,17 @@ package org.bitcoins.node.networking.peer import org.bitcoins.core.p2p._ import org.bitcoins.core.util.NetworkUtil +import org.bitcoins.node.config.NodeAppConfig import org.bitcoins.node.models.Peer import org.bitcoins.node.networking.peer.PeerMessageReceiverState._ -import org.bitcoins.node.{Node, P2PLogger} +import org.bitcoins.node.{P2PLogger, PeerManager} import scala.concurrent.{ExecutionContext, Future} import scala.util.{Failure, Success, Try} -case class ControlMessageHandler(node: Node)(implicit ec: ExecutionContext) +case class ControlMessageHandler(peerManager: PeerManager)(implicit + ec: ExecutionContext, + nodeAppConfig: NodeAppConfig) extends P2PLogger { def handleControlPayload( @@ -33,7 +36,7 @@ case class ControlMessageHandler(node: Node)(implicit ec: ExecutionContext) case good: Initializing => val newState = good.withVersionMsg(versionMsg) - node.peerManager.onVersionMessage(peer, versionMsg) + peerManager.onVersionMessage(peer, versionMsg) sender.sendVerackMessage().map(_ => newState) } @@ -50,7 +53,7 @@ case class ControlMessageHandler(node: Node)(implicit ec: ExecutionContext) case good: Initializing => val newState = good.toNormal(VerAckMessage) - node.peerManager.onInitialization(peer).map(_ => newState) + peerManager.onInitialization(peer).map(_ => newState) } case ping: PingMessage => @@ -92,8 +95,8 @@ case class ControlMessageHandler(node: Node)(implicit ec: ExecutionContext) NetworkUtil.parseInetSocketAddress(bytes, networkAddress.port) val peer = Peer.fromSocket(socket = inetAddress, socks5ProxyParams = - node.nodeAppConfig.socks5ProxyParams) - node.peerManager.addPeerToTry(Vector(peer), 0) + nodeAppConfig.socks5ProxyParams) + peerManager.addPeerToTry(Vector(peer), 0) } case addr: AddrV2Message => val bytes = addr.bytes @@ -103,14 +106,14 @@ case class ControlMessageHandler(node: Node)(implicit ec: ExecutionContext) NetworkUtil.parseInetSocketAddress(bytes, port) val peer = Peer.fromSocket(socket = inetAddress, socks5ProxyParams = - node.nodeAppConfig.socks5ProxyParams) + nodeAppConfig.socks5ProxyParams) val priority = if (services.nodeCompactFilters) 1 else 0 addr match { case IPv4AddrV2Message(_, _, _, _) | IPv6AddrV2Message(_, _, _, _) => - node.peerManager.addPeerToTry(Vector(peer), priority = priority) + peerManager.addPeerToTry(Vector(peer), priority = priority) case TorV3AddrV2Message(_, _, _, _) => - if (node.nodeAppConfig.torConf.enabled) - node.peerManager.addPeerToTry(Vector(peer), priority) + if (nodeAppConfig.torConf.enabled) + peerManager.addPeerToTry(Vector(peer), priority) case _ => logger.debug(s"Unsupported network. Skipping.") } } diff --git a/testkit/src/main/scala/org/bitcoins/testkit/node/NodeUnitTest.scala b/testkit/src/main/scala/org/bitcoins/testkit/node/NodeUnitTest.scala index 6c0f079c79..1cc2618eeb 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/node/NodeUnitTest.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/node/NodeUnitTest.scala @@ -211,8 +211,11 @@ object NodeUnitTest extends P2PLogger { appConfig.chainConf, appConfig.nodeConf, system) + val controlMessageHandler = ControlMessageHandler(node.peerManager)( + system.dispatcher, + appConfig.nodeConf) val receiver = - PeerMessageReceiver(controlMessageHandler = node.controlMessageHandler, + PeerMessageReceiver(controlMessageHandler = controlMessageHandler, dataMessageHandler = node.peerManager.getDataMessageHandler, peer = peer)(system, appConfig.nodeConf) @@ -368,8 +371,10 @@ object NodeUnitTest extends P2PLogger { chainAppConfig: ChainAppConfig, system: ActorSystem): Future[PeerMessageReceiver] = { val node = buildNode(peer, chainApi, walletCreationTimeOpt) + val controlMessageHandler = + ControlMessageHandler(node.peerManager)(system.dispatcher, nodeAppConfig) val receiver = - PeerMessageReceiver(controlMessageHandler = node.controlMessageHandler, + PeerMessageReceiver(controlMessageHandler = controlMessageHandler, dataMessageHandler = node.peerManager.getDataMessageHandler, peer = peer)