Add Concurrency #206
Add Concurrency #206
Conversation
…V2 into async-pair # Conflicts: # Sources/WalletConnect/Engine/Common/SessionEngine.swift # Sources/WalletConnect/WalletConnectClient.swift # Tests/IntegrationTests/ClientTest.swift # Tests/WalletConnectTests/SessionEngineTests.swift
| @@ -0,0 +1,33 @@ | |||
|
|
|||
There was a problem hiding this comment.
Are we going to migrate PairingEngines code in here over time?
There was a problem hiding this comment.
I do not think so, I would rather split PairingEngine for more classes in the future.
like Controller/ProposeEngine NonController/ProposeEngine`. one class for each protocol method segregated by controller/nonController. Probably we could agree on better naming.
| func testPairMultipleTimesOnSameURIThrows() async { | ||
| let uri = WalletConnectURI.stub() | ||
| for i in 1...10 { | ||
| usleep(100) | ||
| if i == 1 { | ||
| XCTAssertNoThrow(Task{try await engine.pair(uri)}) | ||
| } else { | ||
| await XCTAssertThrowsErrorAsync(try await engine.pair(uri)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think that we could throttle the pair(_:) call (with something like a timer) instead of throwing an error. It's natural for the QR scanner to make multiple calls, the "error" is expected, so it might lead to developer confusion when using the SDK. But this can be done later.
There was a problem hiding this comment.
I think throttle should be implemented on the app side instead of sdk.
| try? engine.settle(topic: topicB, proposal: proposal, accounts: [], namespaces: [Namespace.stub()]) | ||
|
|
||
| usleep(100) |
There was a problem hiding this comment.
Why are these usleep() calls needed?
There was a problem hiding this comment.
settle method has an async Task in it's body and it is not async by itself yet. Assertions are executed synchronously after settle call and concurrently to networkingInteractor.subscribe()
I plan to make settle asyn as well in the future so we can remove usleet()
closes #168
closes #205