From 28128f4d2f65865ffec3cb5877ea5a396d1068fc Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Tue, 26 May 2020 20:07:18 +0300 Subject: [PATCH] Add a handler for the Goodbye message The lack of body of `goodbye` in sync_protocol.nim was preventing the respective LibP2P protocol to be mounted and advertised on the network. Adding a body fixes that, but I've also made some changes in the P2P protocol codegen that will prevent the issue from happening again (no body is now considered the equivalent of having an empty body). --- beacon_chain/eth2_network.nim | 73 ++++++++++---------- beacon_chain/sync_protocol.nim | 5 +- beacon_chain/sync_protocol.nim.generated.nim | 53 ++++++++++---- vendor/nim-serialization | 2 +- vendor/nim-stew | 2 +- 5 files changed, 82 insertions(+), 53 deletions(-) diff --git a/beacon_chain/eth2_network.nim b/beacon_chain/eth2_network.nim index 12bc329f8..2c97eaef6 100644 --- a/beacon_chain/eth2_network.nim +++ b/beacon_chain/eth2_network.nim @@ -572,8 +572,7 @@ proc handleIncomingStream(network: Eth2Node, try: logReceivedMsg(peer, MsgType(msg.get)) - let userHandlerFut = callUserHandler(MsgType, peer, conn, noSnappy, msg.get) - await userHandlerFut + await callUserHandler(MsgType, peer, conn, noSnappy, msg.get) except CatchableError as err: await sendErrorResponse(peer, conn, noSnappy, ServerError, ErrorMsg err.msg.toBytes) @@ -809,6 +808,7 @@ proc p2pProtocolBackendImpl*(p: P2PProtocol): Backend = MsgRecName = msg.recName MsgStrongRecName = msg.strongRecName codecNameLit = getRequestProtoName(msg.procDef) + protocolMounterName = ident(msgName & "Mounter") ## ## Implement the Thunk: @@ -823,18 +823,19 @@ proc p2pProtocolBackendImpl*(p: P2PProtocol): Backend = ## initialize the network object by creating handlers bound to the ## specific network. ## - var mounter: NimNode + var userHandlerCall = newTree(nnkDiscardStmt) + if msg.userHandler != nil: - var - protocolMounterName = ident(msgName & "_mounter") - userHandlerCall: NimNode - OutputParamType = msg.outputParamType + var OutputParamType = if msg.kind == msgRequest: msg.outputParamType + else: nil if OutputParamType == nil: - userHandlerCall = newCall(ident"sendUserHandlerResultAsChunkImpl", - streamVar, - noSnappyVar, - msg.genUserHandlerCall(msgVar, [peerVar])) + userHandlerCall = msg.genUserHandlerCall(msgVar, [peerVar]) + if msg.kind == msgRequest: + userHandlerCall = newCall(ident"sendUserHandlerResultAsChunkImpl", + streamVar, + noSnappyVar, + userHandlerCall) else: if OutputParamType.kind == nnkVarTy: OutputParamType = OutputParamType[0] @@ -852,36 +853,32 @@ proc p2pProtocolBackendImpl*(p: P2PProtocol): Backend = peerVar, streamVar, noSnappyVar)), msg.genUserHandlerCall(msgVar, [peerVar], outputParam = responseVar)) - protocol.outRecvProcs.add quote do: - template `callUserHandler`(`MSG`: type `MsgStrongRecName`, - `peerVar`: `Peer`, - `streamVar`: `Connection`, - `noSnappyVar`: bool, - `msgVar`: `MsgRecName`): untyped = - `userHandlerCall` + protocol.outRecvProcs.add quote do: + template `callUserHandler`(`MSG`: type `MsgStrongRecName`, + `peerVar`: `Peer`, + `streamVar`: `Connection`, + `noSnappyVar`: bool, + `msgVar`: `MsgRecName`): untyped = + `userHandlerCall` - proc `protocolMounterName`(`networkVar`: `Eth2Node`) = - proc sszThunk(`streamVar`: `Connection`, - `protocolVar`: string): Future[void] {.gcsafe.} = - return handleIncomingStream(`networkVar`, `streamVar`, true, - `MsgStrongRecName`) + proc `protocolMounterName`(`networkVar`: `Eth2Node`) = + proc sszThunk(`streamVar`: `Connection`, + `protocolVar`: string): Future[void] {.gcsafe.} = + return handleIncomingStream(`networkVar`, `streamVar`, true, + `MsgStrongRecName`) - mount `networkVar`.switch, - LPProtocol(codec: `codecNameLit` & "ssz", - handler: sszThunk) + mount `networkVar`.switch, + LPProtocol(codec: `codecNameLit` & "ssz", + handler: sszThunk) - proc snappyThunk(`streamVar`: `Connection`, - `protocolVar`: string): Future[void] {.gcsafe.} = - return handleIncomingStream(`networkVar`, `streamVar`, false, - `MsgStrongRecName`) + proc snappyThunk(`streamVar`: `Connection`, + `protocolVar`: string): Future[void] {.gcsafe.} = + return handleIncomingStream(`networkVar`, `streamVar`, false, + `MsgStrongRecName`) - mount `networkVar`.switch, - LPProtocol(codec: `codecNameLit` & "ssz_snappy", - handler: snappyThunk) - - mounter = protocolMounterName - else: - mounter = newNilLit() + mount `networkVar`.switch, + LPProtocol(codec: `codecNameLit` & "ssz_snappy", + handler: snappyThunk) ## ## Implement Senders and Handshake @@ -896,7 +893,7 @@ proc p2pProtocolBackendImpl*(p: P2PProtocol): Backend = newCall(registerMsg, protocol.protocolInfoVar, msgNameLit, - mounter, + protocolMounterName, codecNameLit)) result.implementProtocolInit = proc (p: P2PProtocol): NimNode = diff --git a/beacon_chain/sync_protocol.nim b/beacon_chain/sync_protocol.nim index dd24e22f7..ea0207d3b 100644 --- a/beacon_chain/sync_protocol.nim +++ b/beacon_chain/sync_protocol.nim @@ -166,7 +166,10 @@ p2pProtocol BeaconSync(version = 1, debug "Block root request done", peer, roots = blockRoots.len, count, found - proc goodbye(peer: Peer, reason: DisconnectionReason) {.libp2pProtocol("goodbye", 1).} + proc goodbye(peer: Peer, + reason: DisconnectionReason) + {.async, libp2pProtocol("goodbye", 1).} = + debug "Received Goodbye message", reason proc setStatusMsg(peer: Peer, statusMsg: StatusMsg) = debug "Peer status", peer, statusMsg diff --git a/beacon_chain/sync_protocol.nim.generated.nim b/beacon_chain/sync_protocol.nim.generated.nim index 276dc49c2..2a21d61d1 100644 --- a/beacon_chain/sync_protocol.nim.generated.nim +++ b/beacon_chain/sync_protocol.nim.generated.nim @@ -238,12 +238,25 @@ proc beaconBlocksByRootUserHandler(peer: Peer; blockRoots: BlockRootsList; respo inc found debug "Block root request done", peer, roots = blockRoots.len, count, found +proc goodbyeUserHandler(peer: Peer; reason: DisconnectionReason) {.async, + libp2pProtocol("goodbye", 1), gcsafe.} = + type + CurrentProtocol = BeaconSync + template state(peer: Peer): ref[BeaconSyncPeerState:ObjectType] = + cast[ref[BeaconSyncPeerState:ObjectType]](getState(peer, BeaconSyncProtocol)) + + template networkState(peer: Peer): ref[BeaconSyncNetworkState:ObjectType] = + cast[ref[BeaconSyncNetworkState:ObjectType]](getNetworkState(peer.network, + BeaconSyncProtocol)) + + debug "Received Goodbye message", reason + template callUserHandler(MSG: type statusObj; peer: Peer; stream: Connection; noSnappy: bool; msg: StatusMsg): untyped = var response = init(SingleChunkResponse[StatusMsg], peer, stream, noSnappy) statusUserHandler(peer, msg, response) -proc status_mounter(network: Eth2Node) = +proc statusMounter(network: Eth2Node) = proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = return handleIncomingStream(network, stream, true, statusObj) @@ -259,7 +272,7 @@ template callUserHandler(MSG: type pingObj; peer: Peer; stream: Connection; noSnappy: bool; msg: uint64): untyped = sendUserHandlerResultAsChunkImpl(stream, noSnappy, pingUserHandler(peer, msg)) -proc ping_mounter(network: Eth2Node) = +proc pingMounter(network: Eth2Node) = proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = return handleIncomingStream(network, stream, true, pingObj) @@ -275,7 +288,7 @@ template callUserHandler(MSG: type getMetadataObj; peer: Peer; stream: Connectio noSnappy: bool; msg: getMetadataObj): untyped = sendUserHandlerResultAsChunkImpl(stream, noSnappy, getMetadataUserHandler(peer)) -proc getMetadata_mounter(network: Eth2Node) = +proc getMetadataMounter(network: Eth2Node) = proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = return handleIncomingStream(network, stream, true, getMetadataObj) @@ -294,7 +307,7 @@ template callUserHandler(MSG: type beaconBlocksByRangeObj; peer: Peer; noSnappy) beaconBlocksByRangeUserHandler(peer, msg.startSlot, msg.count, msg.step, response) -proc beaconBlocksByRange_mounter(network: Eth2Node) = +proc beaconBlocksByRangeMounter(network: Eth2Node) = proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = return handleIncomingStream(network, stream, true, beaconBlocksByRangeObj) @@ -312,7 +325,7 @@ template callUserHandler(MSG: type beaconBlocksByRootObj; peer: Peer; noSnappy) beaconBlocksByRootUserHandler(peer, msg, response) -proc beaconBlocksByRoot_mounter(network: Eth2Node) = +proc beaconBlocksByRootMounter(network: Eth2Node) = proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = return handleIncomingStream(network, stream, true, beaconBlocksByRootObj) @@ -324,18 +337,34 @@ proc beaconBlocksByRoot_mounter(network: Eth2Node) = mount network.switch, LPProtocol(codec: "/eth2/beacon_chain/req/beacon_blocks_by_root/1/" & "ssz_snappy", handler: snappyThunk) -registerMsg(BeaconSyncProtocol, "status", status_mounter, +template callUserHandler(MSG: type goodbyeObj; peer: Peer; stream: Connection; + noSnappy: bool; msg: DisconnectionReason): untyped = + goodbyeUserHandler(peer, msg) + +proc goodbyeMounter(network: Eth2Node) = + proc sszThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = + return handleIncomingStream(network, stream, true, goodbyeObj) + + mount network.switch, LPProtocol(codec: "/eth2/beacon_chain/req/goodbye/1/" & + "ssz", handler: sszThunk) + proc snappyThunk(stream: Connection; protocol: string): Future[void] {.gcsafe.} = + return handleIncomingStream(network, stream, false, goodbyeObj) + + mount network.switch, LPProtocol(codec: "/eth2/beacon_chain/req/goodbye/1/" & + "ssz_snappy", handler: snappyThunk) + +registerMsg(BeaconSyncProtocol, "status", statusMounter, "/eth2/beacon_chain/req/status/1/") -registerMsg(BeaconSyncProtocol, "ping", ping_mounter, +registerMsg(BeaconSyncProtocol, "ping", pingMounter, "/eth2/beacon_chain/req/ping/1/") -registerMsg(BeaconSyncProtocol, "getMetadata", getMetadata_mounter, +registerMsg(BeaconSyncProtocol, "getMetadata", getMetadataMounter, "/eth2/beacon_chain/req/metadata/1/") -registerMsg(BeaconSyncProtocol, "beaconBlocksByRange", - beaconBlocksByRange_mounter, +registerMsg(BeaconSyncProtocol, "beaconBlocksByRange", beaconBlocksByRangeMounter, "/eth2/beacon_chain/req/beacon_blocks_by_range/1/") -registerMsg(BeaconSyncProtocol, "beaconBlocksByRoot", beaconBlocksByRoot_mounter, +registerMsg(BeaconSyncProtocol, "beaconBlocksByRoot", beaconBlocksByRootMounter, "/eth2/beacon_chain/req/beacon_blocks_by_root/1/") -registerMsg(BeaconSyncProtocol, "goodbye", nil, "/eth2/beacon_chain/req/goodbye/1/") +registerMsg(BeaconSyncProtocol, "goodbye", goodbyeMounter, + "/eth2/beacon_chain/req/goodbye/1/") proc BeaconSyncPeerConnected(peer: Peer; stream: Connection) {.async, gcsafe.} = type CurrentProtocol = BeaconSync diff --git a/vendor/nim-serialization b/vendor/nim-serialization index 8a25451fc..2baaccd50 160000 --- a/vendor/nim-serialization +++ b/vendor/nim-serialization @@ -1 +1 @@ -Subproject commit 8a25451fce5e7207c44a7815496540c631fa1615 +Subproject commit 2baaccd50e0a15939a1d3effc7e07c8a552e8ac1 diff --git a/vendor/nim-stew b/vendor/nim-stew index cf837b3fb..4ffd3e1f5 160000 --- a/vendor/nim-stew +++ b/vendor/nim-stew @@ -1 +1 @@ -Subproject commit cf837b3fb6108f666c76523688178d3c2dd9ba93 +Subproject commit 4ffd3e1f59c2f12ae57456c6fccf477bc203da78