Skip to content

Commit 026e249

Browse files
MacOMNIxlc
MacOMNI
and
xlc
authored
improve dedup connection logic (#213)
* improve dedup connection logic * Update Networking/Tests/NetworkingTests/PeerTests.swift Co-authored-by: Xiliang Chen <[email protected]> * Update Networking/Tests/NetworkingTests/PeerTests.swift Co-authored-by: Xiliang Chen <[email protected]> * Update Networking/Tests/NetworkingTests/PeerTests.swift Co-authored-by: Xiliang Chen <[email protected]> * update test --------- Co-authored-by: Xiliang Chen <[email protected]>
1 parent 0bd14b7 commit 026e249

File tree

2 files changed

+77
-36
lines changed

2 files changed

+77
-36
lines changed

‎Networking/Sources/Networking/Peer.swift‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -303,49 +303,49 @@ private struct PeerEventHandler<Handler: StreamHandler>: QuicEventHandler {
303303
connections.byId[connection.id]
304304
}
305305
guard let conn else {
306-
logger.warning("Trying to open connection but connection is gone?", metadata: ["connectionId": "\(connection.id)"])
306+
logger.warning("Attempt to open but connection is absent", metadata: ["connectionId": "\(connection.id)"])
307307
return .code(.connectionRefused)
308308
}
309+
309310
do {
310311
let (publicKey, alternativeName) = try parseCertificate(data: certificate, type: .x509)
311312
logger.trace("Certificate parsed", metadata: [
312313
"connectionId": "\(connection.id)",
313314
"publicKey": "\(publicKey.toHexString())",
314315
"alternativeName": "\(alternativeName)",
315316
])
316-
if alternativeName != generateSubjectAlternativeName(pubkey: publicKey) {
317-
return .code(.badCert)
318-
}
319-
320317
if publicKey == impl.publicKey {
321-
// self connection
322-
logger.trace("self connection rejected", metadata: [
323-
"connectionId": "\(connection.id)",
324-
"publicKey": "\(publicKey.toHexString())",
325-
])
318+
// Self connection detected
319+
logger.trace("Rejecting self-connection", metadata: ["connectionId": "\(connection.id)"])
326320
return .code(.connectionRefused)
327321
}
328-
322+
if alternativeName != generateSubjectAlternativeName(pubkey: publicKey) {
323+
return .code(.badCert)
324+
}
329325
// TODO: verify if it is current or next validator
330326

327+
// Check for an existing connection by public key
331328
return try impl.connections.write { connections in
332329
if connections.byPublicKey.keys.contains(publicKey) {
333-
// duplicated connection
334-
logger.debug("duplicated connection rejected", metadata: [
335-
"connectionId": "\(connection.id)",
336-
"publicKey": "\(publicKey.toHexString())",
337-
])
338-
// TODO: write a test for this
339-
return .code(.connectionRefused)
330+
// Deterministically decide based on public key comparison
331+
if !publicKey.lexicographicallyPrecedes(impl.publicKey) {
332+
connections.byPublicKey[publicKey] = conn
333+
try conn.opened(publicKey: publicKey)
334+
return .code(.success)
335+
} else {
336+
logger.debug("Rejecting duplicate connection by rule", metadata: [
337+
"connectionId": "\(connection.id)", "publicKey": "\(publicKey.toHexString())",
338+
])
339+
return .code(.connectionRefused)
340+
}
341+
} else {
342+
connections.byPublicKey[publicKey] = conn
343+
try conn.opened(publicKey: publicKey)
344+
return .code(.success)
340345
}
341-
connections.byPublicKey[publicKey] = conn
342-
try conn.opened(publicKey: publicKey)
343-
return .code(.success)
344346
}
345347
} catch {
346-
logger.warning("Failed to parse certificate", metadata: [
347-
"connectionId": "\(connection.id)",
348-
"error": "\(error)"])
348+
logger.warning("Certificate parsing failed", metadata: ["connectionId": "\(connection.id)", "error": "\(error)"])
349349
return .code(.badCert)
350350
}
351351
}

‎Networking/Tests/NetworkingTests/PeerTests.swift‎

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,46 @@ struct PeerTests {
143143
typealias EphemeralHandler = MockEphemeralStreamHandler
144144
}
145145

146+
@Test
147+
func concurrentPeerConnection() async throws {
148+
let peer1 = try Peer(
149+
options: PeerOptions<MockStreamHandler>(
150+
role: .validator,
151+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
152+
genesisHeader: Data32(),
153+
secretKey: Ed25519.SecretKey(from: Data32.random()),
154+
presistentStreamHandler: MockPresentStreamHandler(),
155+
ephemeralStreamHandler: MockEphemeralStreamHandler(),
156+
serverSettings: .defaultSettings,
157+
clientSettings: .defaultSettings
158+
)
159+
)
160+
let peer2 = try Peer(
161+
options: PeerOptions<MockStreamHandler>(
162+
role: .validator,
163+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
164+
genesisHeader: Data32(),
165+
secretKey: Ed25519.SecretKey(from: Data32.random()),
166+
presistentStreamHandler: MockPresentStreamHandler(),
167+
ephemeralStreamHandler: MockEphemeralStreamHandler(),
168+
serverSettings: .defaultSettings,
169+
clientSettings: .defaultSettings
170+
)
171+
)
172+
173+
let connection1 = try peer1.connect(to: peer2.listenAddress(), role: .validator)
174+
let connection2 = try peer2.connect(to: peer1.listenAddress(), role: .validator)
175+
try? await Task.sleep(for: .milliseconds(50))
176+
if !connection1.isClosed {
177+
let data = try await connection1.request(MockRequest(kind: .typeA, data: Data("hello world".utf8)))
178+
#expect(data == Data("hello world response".utf8))
179+
}
180+
if !connection2.isClosed {
181+
let data = try await connection2.request(MockRequest(kind: .typeA, data: Data("hello world".utf8)))
182+
#expect(data == Data("hello world response".utf8))
183+
}
184+
}
185+
146186
@Test
147187
func largeDataRequest() async throws {
148188
let handler1 = MockPresentStreamHandler()
@@ -159,7 +199,7 @@ struct PeerTests {
159199
let peer1 = try Peer(
160200
options: PeerOptions<MockStreamHandler>(
161201
role: .validator,
162-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 9085)!,
202+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
163203
genesisHeader: Data32(),
164204
secretKey: Ed25519.SecretKey(from: Data32.random()),
165205
presistentStreamHandler: handler1,
@@ -172,7 +212,7 @@ struct PeerTests {
172212
let peer2 = try Peer(
173213
options: PeerOptions<MockStreamHandler>(
174214
role: .validator,
175-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 9086)!,
215+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
176216
genesisHeader: Data32(),
177217
secretKey: Ed25519.SecretKey(from: Data32.random()),
178218
presistentStreamHandler: handler2,
@@ -185,7 +225,7 @@ struct PeerTests {
185225
try? await Task.sleep(for: .milliseconds(50))
186226

187227
let connection1 = try peer1.connect(
188-
to: NetAddr(ipAddress: "127.0.0.1", port: 9086)!, role: .validator
228+
to: peer2.listenAddress(), role: .validator
189229
)
190230
try? await Task.sleep(for: .milliseconds(50))
191231

@@ -218,7 +258,7 @@ struct PeerTests {
218258
let peer1 = try Peer(
219259
options: PeerOptions<MockStreamHandler>(
220260
role: .validator,
221-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 185)!,
261+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
222262
genesisHeader: Data32(),
223263
secretKey: Ed25519.SecretKey(from: Data32.random()),
224264
presistentStreamHandler: MockPresentStreamHandler(),
@@ -231,7 +271,7 @@ struct PeerTests {
231271
let peer2 = try Peer(
232272
options: PeerOptions<MockStreamHandler>(
233273
role: .validator,
234-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 186)!,
274+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
235275
genesisHeader: Data32(),
236276
secretKey: Ed25519.SecretKey(from: Data32.random()),
237277
presistentStreamHandler: handler2,
@@ -244,7 +284,7 @@ struct PeerTests {
244284
let peer3 = try Peer(
245285
options: PeerOptions<MockStreamHandler>(
246286
role: .validator,
247-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 187)!,
287+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
248288
genesisHeader: Data32(),
249289
secretKey: Ed25519.SecretKey(from: Data32.random()),
250290
presistentStreamHandler: handler2,
@@ -257,7 +297,7 @@ struct PeerTests {
257297
try? await Task.sleep(for: .milliseconds(50))
258298

259299
let connection = try peer1.connect(
260-
to: NetAddr(ipAddress: "127.0.0.1", port: 186)!, role: .validator
300+
to: peer2.listenAddress(), role: .validator
261301
)
262302
try? await Task.sleep(for: .milliseconds(50))
263303

@@ -308,7 +348,7 @@ struct PeerTests {
308348
let peer1 = try Peer(
309349
options: PeerOptions<MockStreamHandler>(
310350
role: .validator,
311-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 8081)!,
351+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
312352
genesisHeader: Data32(),
313353
secretKey: Ed25519.SecretKey(from: Data32.random()),
314354
presistentStreamHandler: handler1,
@@ -320,7 +360,7 @@ struct PeerTests {
320360
let peer2 = try Peer(
321361
options: PeerOptions<MockStreamHandler>(
322362
role: .validator,
323-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 8082)!,
363+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
324364
genesisHeader: Data32(),
325365
secretKey: Ed25519.SecretKey(from: Data32.random()),
326366
presistentStreamHandler: handler2,
@@ -331,7 +371,7 @@ struct PeerTests {
331371
)
332372

333373
_ = try peer1.connect(
334-
to: NetAddr(ipAddress: "127.0.0.1", port: 8082)!, role: .validator
374+
to: peer2.listenAddress(), role: .validator
335375
)
336376

337377
try? await Task.sleep(for: .milliseconds(50))
@@ -344,7 +384,7 @@ struct PeerTests {
344384
kind: .uniqueB, message: .init(kind: .uniqueB, data: Data("I am jam".utf8))
345385
)
346386
// Verify last received data
347-
try? await Task.sleep(for: .milliseconds(50))
387+
try? await Task.sleep(for: .milliseconds(100))
348388
await #expect(handler2.lastReceivedData == Data("hello world".utf8))
349389
await #expect(handler1.lastReceivedData == Data("I am jam".utf8))
350390
}
@@ -383,6 +423,7 @@ struct PeerTests {
383423
let dataList1 = try await connection1.request(
384424
MockRequest(kind: .typeA, data: Data("hello world".utf8))
385425
)
426+
try? await Task.sleep(for: .milliseconds(100))
386427
#expect(dataList1 == Data("hello world response".utf8))
387428
}
388429

@@ -452,7 +493,7 @@ struct PeerTests {
452493
let peer = try Peer(
453494
options: PeerOptions<MockStreamHandler>(
454495
role: .builder,
455-
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: UInt16(7081 + i))!,
496+
listenAddress: NetAddr(ipAddress: "127.0.0.1", port: 0)!,
456497
genesisHeader: Data32(),
457498
secretKey: Ed25519.SecretKey(from: Data32.random()),
458499
presistentStreamHandler: handler,

0 commit comments

Comments
 (0)