From 31290b6b3579c89963013d762b8e56889b0d01e6 Mon Sep 17 00:00:00 2001 From: kdeme Date: Thu, 18 Apr 2019 12:25:41 +0200 Subject: [PATCH 1/3] Fix to only allow sync for peer with eth support --- eth/p2p/blockchain_sync.nim | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/eth/p2p/blockchain_sync.nim b/eth/p2p/blockchain_sync.nim index 706e761..803e3d1 100644 --- a/eth/p2p/blockchain_sync.nim +++ b/eth/p2p/blockchain_sync.nim @@ -331,10 +331,12 @@ proc onPeerDisconnected(ctx: SyncContext, p: Peer) = proc startSync(ctx: SyncContext) = var po: PeerObserver po.onPeerConnected = proc(p: Peer) {.gcsafe.} = - ctx.onPeerConnected(p) + if p.supports(eth): + ctx.onPeerConnected(p) po.onPeerDisconnected = proc(p: Peer) {.gcsafe.} = - ctx.onPeerDisconnected(p) + if p.supports(eth): + ctx.onPeerDisconnected(p) ctx.peerPool.addObserver(ctx, po) From 7cdd31e98257178a05dcbffc737542d4af10b34a Mon Sep 17 00:00:00 2001 From: kdeme Date: Thu, 18 Apr 2019 14:22:45 +0200 Subject: [PATCH 2/3] Add option to set specific protocol for observer --- eth/p2p/blockchain_sync.nim | 7 +++---- eth/p2p/peer_pool.nim | 9 +++++++-- eth/p2p/private/p2p_types.nim | 1 + eth/p2p/rlpx.nim | 11 ++++++----- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/eth/p2p/blockchain_sync.nim b/eth/p2p/blockchain_sync.nim index 803e3d1..be14e21 100644 --- a/eth/p2p/blockchain_sync.nim +++ b/eth/p2p/blockchain_sync.nim @@ -331,13 +331,12 @@ proc onPeerDisconnected(ctx: SyncContext, p: Peer) = proc startSync(ctx: SyncContext) = var po: PeerObserver po.onPeerConnected = proc(p: Peer) {.gcsafe.} = - if p.supports(eth): - ctx.onPeerConnected(p) + ctx.onPeerConnected(p) po.onPeerDisconnected = proc(p: Peer) {.gcsafe.} = - if p.supports(eth): - ctx.onPeerDisconnected(p) + ctx.onPeerDisconnected(p) + po.setProtocol eth ctx.peerPool.addObserver(ctx, po) proc findBestPeer(node: EthereumNode): (Peer, DifficultyInt) = diff --git a/eth/p2p/peer_pool.nim b/eth/p2p/peer_pool.nim index 4bb479e..328fdab 100644 --- a/eth/p2p/peer_pool.nim +++ b/eth/p2p/peer_pool.nim @@ -35,7 +35,8 @@ proc addObserver(p: PeerPool, observerId: int, observer: PeerObserver) = p.observers[observerId] = observer if not observer.onPeerConnected.isNil: for peer in p.connectedNodes.values: - observer.onPeerConnected(peer) + if observer.protocol.isNil or peer.supports(observer.protocol): + observer.onPeerConnected(peer) proc delObserver(p: PeerPool, observerId: int) = p.observers.del(observerId) @@ -46,6 +47,9 @@ proc addObserver*(p: PeerPool, observerId: ref, observer: PeerObserver) {.inline proc delObserver*(p: PeerPool, observerId: ref) {.inline.} = p.delObserver(cast[int](observerId)) +template setProtocol*(observer: PeerObserver, Protocol: type) = + observer.protocol = Protocol.protocolInfo + proc stopAllPeers(p: PeerPool) {.async.} = debug "Stopping all peers ..." # TODO: ... @@ -108,7 +112,8 @@ proc addPeer*(pool: PeerPool, peer: Peer): bool = pool.connectedNodes[peer.remote] = peer for o in pool.observers.values: if not o.onPeerConnected.isNil: - o.onPeerConnected(peer) + if o.protocol.isNil or peer.supports(o.protocol): + o.onPeerConnected(peer) return true else: return false diff --git a/eth/p2p/private/p2p_types.nim b/eth/p2p/private/p2p_types.nim index cb0e125..cc06fc2 100644 --- a/eth/p2p/private/p2p_types.nim +++ b/eth/p2p/private/p2p_types.nim @@ -59,6 +59,7 @@ type PeerObserver* = object onPeerConnected*: proc(p: Peer) {.gcsafe.} onPeerDisconnected*: proc(p: Peer) {.gcsafe.} + protocol*: ProtocolInfo Capability* = object name*: string diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index feebf75..43e2d2e 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -227,9 +227,6 @@ proc registerProtocol(protocol: ProtocolInfo) = # Message composition and encryption # -template protocolOffset(peer: Peer, Protocol: type): int = - peer.dispatcher.protocolOffsets[Protocol.protocolInfo.index] - proc perPeerMsgIdImpl(peer: Peer, proto: ProtocolInfo, msgId: int): int {.inline.} = result = msgId if not peer.dispatcher.isNil: @@ -239,9 +236,12 @@ template getPeer(peer: Peer): auto = peer template getPeer(response: Response): auto = Peer(response) template getPeer(response: ResponseWithId): auto = response.peer +proc supports*(peer: Peer, proto: ProtocolInfo): bool {.inline.} = + peer.dispatcher.protocolOffsets[proto.index] != -1 + proc supports*(peer: Peer, Protocol: type): bool {.inline.} = ## Checks whether a Peer supports a particular protocol - peer.protocolOffset(Protocol) != -1 + peer.supports(Protocol.protocolInfo) template perPeerMsgId(peer: Peer, MsgType: type): int = perPeerMsgIdImpl(peer, MsgType.msgProtocol.protocolInfo, MsgType.msgId) @@ -1124,7 +1124,8 @@ proc removePeer(network: EthereumNode, peer: Peer) = for observer in network.peerPool.observers.values: if not observer.onPeerDisconnected.isNil: - observer.onPeerDisconnected(peer) + if observer.protocol.isNil or peer.supports(observer.protocol): + observer.onPeerDisconnected(peer) proc callDisconnectHandlers(peer: Peer, reason: DisconnectionReason): Future[void] = var futures = newSeqOfCap[Future[void]](allProtocols.len) From 5d9239ef17087ce7cc240b2843ee75567d569f78 Mon Sep 17 00:00:00 2001 From: kdeme Date: Thu, 18 Apr 2019 14:33:38 +0200 Subject: [PATCH 3/3] Quick fix for when removePeer gets called before dispatcher is initialized --- eth/p2p/rlpx.nim | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/eth/p2p/rlpx.nim b/eth/p2p/rlpx.nim index 43e2d2e..5b572c6 100644 --- a/eth/p2p/rlpx.nim +++ b/eth/p2p/rlpx.nim @@ -1122,10 +1122,13 @@ proc removePeer(network: EthereumNode, peer: Peer) = if network.peerPool != nil and not peer.remote.isNil: network.peerPool.connectedNodes.del(peer.remote) - for observer in network.peerPool.observers.values: - if not observer.onPeerDisconnected.isNil: - if observer.protocol.isNil or peer.supports(observer.protocol): - observer.onPeerDisconnected(peer) + # Note: we need to do this check as disconnect (and thus removePeer) + # currently can get called before the dispatcher is initialized. + if not peer.dispatcher.isNil: + for observer in network.peerPool.observers.values: + if not observer.onPeerDisconnected.isNil: + if observer.protocol.isNil or peer.supports(observer.protocol): + observer.onPeerDisconnected(peer) proc callDisconnectHandlers(peer: Peer, reason: DisconnectionReason): Future[void] = var futures = newSeqOfCap[Future[void]](allProtocols.len)