multistream select make sure to not report NA (#235)

* multistream select make sure to not report NA but rather empty string if all fails

Also re-enable tests

* avoid using bad constructs, make multistream.select flow crystal clear

libp2p/multistream.nim

@@ -58,40 +58,45 @@ proc select*(m: MultistreamSelect,
     trace "selecting proto", proto = proto[0]
     await conn.writeLp((proto[0] & "\n")) # select proto
 
-  result = string.fromBytes((await conn.readLp(1024))) # read ms header
-  result.removeSuffix("\n")
-  if result != Codec:
-    notice "handshake failed", codec = result.toHex()
+  var s = string.fromBytes((await conn.readLp(1024))) # read ms header
+  s.removeSuffix("\n")
+  if s != Codec:
+    notice "handshake failed", codec = s.toHex()
     raise newMultistreamHandshakeException()
 
   if proto.len() == 0: # no protocols, must be a handshake call
-    return
-
-  result = string.fromBytes(await conn.readLp(1024)) # read the first proto
-  trace "reading first requested proto"
-  result.removeSuffix("\n")
-  if result == proto[0]:
-    trace "successfully selected ", proto = proto[0]
-    return
-
-  let protos = proto[1..<proto.len()]
-  trace "selecting one of several protos", protos = protos
-  for p in protos:
-    trace "selecting proto", proto = p
-    await conn.writeLp((p & "\n")) # select proto
-    result = string.fromBytes(await conn.readLp(1024)) # read the first proto
-    result.removeSuffix("\n")
-    if result == p:
-      trace "selected protocol", protocol = result
-      break
+    return Codec
+  else:
+    s = string.fromBytes(await conn.readLp(1024)) # read the first proto
+    trace "reading first requested proto"
+    s.removeSuffix("\n")
+    if s == proto[0]:
+      trace "successfully selected ", proto = proto[0]
+      return proto[0]
+    elif proto.len > 1:
+      # Try to negotiate alternatives
+      let protos = proto[1..<proto.len()]
+      trace "selecting one of several protos", protos = protos
+      for p in protos:
+        trace "selecting proto", proto = p
+        await conn.writeLp((p & "\n")) # select proto
+        s = string.fromBytes(await conn.readLp(1024)) # read the first proto
+        s.removeSuffix("\n")
+        if s == p:
+          trace "selected protocol", protocol = s
+          return s
+      return ""
+    else:
+      # No alternatives, fail
+      return ""
 
 proc select*(m: MultistreamSelect,
              conn: Connection,
              proto: string): Future[bool] {.async.} =
   if proto.len > 0:
-    result = (await m.select(conn, @[proto])) == proto
+    return (await m.select(conn, @[proto])) == proto
   else:
-    result = (await m.select(conn, @[])) == Codec
+    return (await m.select(conn, @[])) == Codec
 
 proc select*(m: MultistreamSelect, conn: Connection): Future[bool] =
   m.select(conn, "")

libp2p/switch.nim

@@ -63,7 +63,7 @@ proc secure(s: Switch, conn: Connection): Future[Connection] {.async, gcsafe.} =
     raise newException(CatchableError, "No secure managers registered!")
 
   let manager = await s.ms.select(conn, s.secureManagers.mapIt(it.codec))
-  if manager.len == 0 or manager == "na":
+  if manager.len == 0:
     raise newException(CatchableError, "Unable to negotiate a secure channel!")
 
   trace "securing connection", codec=manager

tests/testmultistream.nim

@@ -57,7 +57,7 @@ proc newTestSelectStream(): TestSelectStream =
 type
   LsHandler = proc(procs: seq[byte]): Future[void] {.gcsafe.}
 
-  TestLsStream = ref object of LPStream
+  TestLsStream = ref object of Connection
     step*: int
     ls*: LsHandler
 
@@ -103,7 +103,7 @@ proc newTestLsStream(ls: LsHandler): TestLsStream {.gcsafe.} =
 type
   NaHandler = proc(procs: string): Future[void] {.gcsafe.}
 
-  TestNaStream = ref object of LPStream
+  TestNaStream = ref object of Connection
     step*: int
     na*: NaHandler
 

tests/testnative.nim

@@ -14,7 +14,7 @@ import testmultibase,
        testpeer
 
 import testtransport,
-      #  testmultistream,
+       testmultistream,
        testbufferstream,
        testidentify,
        testswitch,