chore(quic): add tests from common interop (#1662)

.pinned

@@ -8,7 +8,7 @@ json_serialization;https://github.com/status-im/nim-json-serialization@#2b1c5eb1
 metrics;https://github.com/status-im/nim-metrics@#6142e433fc8ea9b73379770a788017ac528d46ff
 ngtcp2;https://github.com/status-im/nim-ngtcp2@#9456daa178c655bccd4a3c78ad3b8cce1f0add73
 nimcrypto;https://github.com/cheatfate/nimcrypto@#19c41d6be4c00b4a2c8000583bd30cf8ceb5f4b1
-quic;https://github.com/vacp2p/nim-quic@#cae13c2d22ba2730c979486cf89b88927045c3ae
+quic;https://github.com/vacp2p/nim-quic@#9370190ded18d78a5a9990f57aa8cbbf947f3891
 results;https://github.com/arnetheduck/nim-results@#df8113dda4c2d74d460a8fa98252b0b771bf1f27
 secp256k1;https://github.com/status-im/nim-secp256k1@#f808ed5e7a7bfc42204ec7830f14b7a42b63c284
 serialization;https://github.com/status-im/nim-serialization@#548d0adc9797a10b2db7f788b804330306293088

libp2p.nimble

@@ -10,7 +10,7 @@ skipDirs = @["tests", "examples", "Nim", "tools", "scripts", "docs"]
 requires "nim >= 2.0.0",
   "nimcrypto >= 0.6.0 & < 0.7.0", "dnsclient >= 0.3.0 & < 0.4.0", "bearssl >= 0.2.5",
   "chronicles >= 0.11.0 & < 0.12.0", "chronos >= 4.0.4", "metrics", "secp256k1",
-  "stew >= 0.4.0", "websock >= 0.2.0", "unittest2", "results", "quic >= 0.2.15",
+  "stew >= 0.4.0", "websock >= 0.2.0", "unittest2", "results", "quic >= 0.2.16",
   "https://github.com/vacp2p/nim-jwt.git#18f8378de52b241f321c1f9ea905456e89b95c6f"
 
 let nimc = getEnv("NIMC", "nim") # Which nim compiler to use

libp2p/transports/quictransport.nim

@@ -195,7 +195,7 @@ type CertGenerator =
 
 type QuicTransport* = ref object of Transport
   listener: Listener
-  client: QuicClient
+  client: Opt[QuicClient]
   privateKey: PrivateKey
   connections: seq[P2PConnection]
   rng: ref HmacDrbgContext
@@ -248,27 +248,33 @@ method handles*(transport: QuicTransport, address: MultiAddress): bool {.raises:
     return false
   QUIC_V1.match(address)
 
-method start*(
+proc makeConfig(self: QuicTransport): TLSConfig =
-    self: QuicTransport, addrs: seq[MultiAddress]
-) {.async: (raises: [LPError, transport.TransportError, CancelledError]).} =
-  doAssert self.listener.isNil, "start() already called"
-  #TODO handle multiple addr
-
   let pubkey = self.privateKey.getPublicKey().valueOr:
     doAssert false, "could not obtain public key"
     return
 
-  try:
+  let cert = self.certGenerator(KeyPair(seckey: self.privateKey, pubkey: pubkey))
-    if self.rng.isNil:
+  let tlsConfig = TLSConfig.init(
-      self.rng = newRng()
+    cert.certificate, cert.privateKey, @[alpn], Opt.some(makeCertificateVerifier())
+  )
+  return tlsConfig
 
-    let cert = self.certGenerator(KeyPair(seckey: self.privateKey, pubkey: pubkey))
+proc getRng(self: QuicTransport): ref HmacDrbgContext =
-    let tlsConfig = TLSConfig.init(
+  if self.rng.isNil:
-      cert.certificate, cert.privateKey, @[alpn], Opt.some(makeCertificateVerifier())
+    self.rng = newRng()
-    )
+
-    self.client = QuicClient.init(tlsConfig, rng = self.rng)
+  return self.rng
-    self.listener =
+
-      QuicServer.init(tlsConfig, rng = self.rng).listen(initTAddress(addrs[0]).tryGet)
+method start*(
+    self: QuicTransport, addrs: seq[MultiAddress]
+) {.async: (raises: [LPError, transport.TransportError, CancelledError]).} =
+  doAssert self.listener.isNil, "start() already called"
+  # TODO(#1663): handle multiple addr
+
+  try:
+    self.listener = QuicServer.init(self.makeConfig(), rng = self.getRng()).listen(
+        initTAddress(addrs[0]).tryGet
+      )
     await procCall Transport(self).start(addrs)
     self.addrs[0] =
       MultiAddress.init(self.listener.localAddress(), IPPROTO_UDP).tryGet() &
@@ -289,19 +295,21 @@ method start*(
   self.running = true
 
 method stop*(transport: QuicTransport) {.async: (raises: []).} =
-  if transport.running:
+  let conns = transport.connections[0 .. ^1]
-    let conns = transport.connections[0 .. ^1]
+  for c in conns:
-    for c in conns:
+    await c.close()
-      await c.close()
+
-    await procCall Transport(transport).stop()
+  if not transport.listener.isNil:
     try:
       await transport.listener.stop()
     except CatchableError as exc:
       trace "Error shutting down Quic transport", description = exc.msg
     transport.listener.destroy()
-    transport.running = false
     transport.listener = nil
 
+  transport.client = Opt.none(QuicClient)
+  await procCall Transport(transport).stop()
+
 proc wrapConnection(
     transport: QuicTransport, connection: QuicConnection
 ): QuicSession {.raises: [TransportOsError, MaError].} =
@@ -365,7 +373,11 @@ method dial*(
     async: (raises: [transport.TransportError, CancelledError])
 .} =
   try:
-    let quicConnection = await self.client.dial(initTAddress(address).tryGet)
+    if not self.client.isSome:
+      self.client = Opt.some(QuicClient.init(self.makeConfig(), rng = self.getRng()))
+
+    let client = self.client.get()
+    let quicConnection = await client.dial(initTAddress(address).tryGet)
     return self.wrapConnection(quicConnection)
   except CancelledError as e:
     raise e

tests/testquic.nim

@@ -60,15 +60,19 @@ proc invalidCertGenerator(
   except ResultError[crypto.CryptoError]:
     raiseAssert "private key should be set"
 
-proc createTransport(withInvalidCert: bool = false): Future[QuicTransport] {.async.} =
+proc createTransport(
-  let ma = @[MultiAddress.init("/ip4/127.0.0.1/udp/0/quic-v1").tryGet()]
+    isServer: bool = false, withInvalidCert: bool = false
+): Future[QuicTransport] {.async.} =
   let privateKey = PrivateKey.random(ECDSA, (newRng())[]).tryGet()
   let trans =
     if withInvalidCert:
       QuicTransport.new(Upgrade(), privateKey, invalidCertGenerator)
     else:
       QuicTransport.new(Upgrade(), privateKey)
-  await trans.start(ma)
+
+  if isServer: # servers are started because they need to listen
+    let ma = @[MultiAddress.init("/ip4/127.0.0.1/udp/0/quic-v1").tryGet()]
+    await trans.start(ma)
 
   return trans
 
@@ -77,12 +81,46 @@ suite "Quic transport":
     checkTrackers()
 
   asyncTest "can handle local address":
-    let trans = await createTransport()
+    let server = await createTransport(isServer = true)
-    check trans.handles(trans.addrs[0])
+    check server.handles(server.addrs[0])
-    await trans.stop()
+    await server.stop()
+
+  asyncTest "handle accept cancellation":
+    let server = await createTransport(isServer = true)
+
+    let acceptFut = server.accept()
+    await acceptFut.cancelAndWait()
+    check acceptFut.cancelled
+
+    await server.stop()
+
+  asyncTest "handle dial cancellation":
+    let server = await createTransport(isServer = true)
+    let client = await createTransport(isServer = false)
+
+    let connFut = client.dial(server.addrs[0])
+    await connFut.cancelAndWait()
+    check connFut.cancelled
+
+    await client.stop()
+    await server.stop()
+
+  asyncTest "stopping transport kills connections":
+    let server = await createTransport(isServer = true)
+    let client = await createTransport(isServer = false)
+
+    let acceptFut = server.accept()
+    let conn = await client.dial(server.addrs[0])
+    let serverConn = await acceptFut
+
+    await client.stop()
+    await server.stop()
+
+    check serverConn.closed()
+    check conn.closed()
 
   asyncTest "transport e2e":
-    let server = await createTransport()
+    let server = await createTransport(isServer = true)
     asyncSpawn createServerAcceptConn(server)()
     defer:
       await server.stop()
@@ -101,7 +139,7 @@ suite "Quic transport":
     await runClient()
 
   asyncTest "transport e2e - invalid cert - server":
-    let server = await createTransport(true)
+    let server = await createTransport(isServer = true, withInvalidCert = true)
     asyncSpawn createServerAcceptConn(server)()
     defer:
       await server.stop()
@@ -115,22 +153,27 @@ suite "Quic transport":
     await runClient()
 
   asyncTest "transport e2e - invalid cert - client":
-    let server = await createTransport()
+    let server = await createTransport(isServer = true)
     asyncSpawn createServerAcceptConn(server)()
     defer:
       await server.stop()
 
     proc runClient() {.async.} =
-      let client = await createTransport(true)
+      let client = await createTransport(withInvalidCert = true)
       expect QuicTransportDialError:
         discard await client.dial("", server.addrs[0])
       await client.stop()
 
     await runClient()
 
+  asyncTest "should allow multiple local addresses":
+    # TODO(#1663): handle multiple addr
+    # See test example in commonTransportTest
+    return
+
   asyncTest "server not accepting":
-    let server = await createTransport()
+    let server = await createTransport(isServer = true)
-    # itentionally not calling createServerAcceptConn as server should not accept
+    # intentionally not calling createServerAcceptConn as server should not accept
     defer:
       await server.stop()
 
@@ -145,7 +188,7 @@ suite "Quic transport":
     await runClient()
 
   asyncTest "closing session should close all streams":
-    let server = await createTransport()
+    let server = await createTransport(isServer = true)
     # because some clients will not write full message, 
     # it is expected for server to receive eof
     asyncSpawn createServerAcceptConn(server, true)()
@@ -201,7 +244,7 @@ suite "Quic transport":
         check (await stream.readLp(100)) == fromHex("5678")
         await client.stop()
 
-      let server = await createTransport()
+      let server = await createTransport(isServer = true)
       asyncSpawn serverHandler(server)
       defer:
         await server.stop()