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).
2020-05-26 20:07:18 +03:00 · 2020-05-26 20:07:18 +03:00 · 28128f4d2f
parent 0d54073ec9
commit 28128f4d2f
5 changed files with 82 additions and 53 deletions
--- 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 =
--- 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
--- 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
--- a/vendor/nim-serialization
+++ b/vendor/nim-serialization
@ -1 +1 @@
-Subproject commit 8a25451fce5e7207c44a7815496540c631fa1615
+Subproject commit 2baaccd50e0a15939a1d3effc7e07c8a552e8ac1
--- a/vendor/nim-stew
+++ b/vendor/nim-stew
@ -1 +1 @@
-Subproject commit cf837b3fb6108f666c76523688178d3c2dd9ba93
+Subproject commit 4ffd3e1f59c2f12ae57456c6fccf477bc203da78