From f2eefeb07ed352f884b6246da1915b54e9286df2 Mon Sep 17 00:00:00 2001 From: Andrey Scherbovich Date: Fri, 9 Jul 2021 14:36:32 +0200 Subject: [PATCH] improve UpdateSessionHandler --- Sources/Internal/Communicator.swift | 10 +++--- Sources/Internal/HandshakeHandler.swift | 2 +- Sources/Internal/UpdateSessionHandler.swift | 11 ++++-- Sources/PublicInterface/Client.swift | 34 +++++++++---------- Sources/PublicInterface/Server.swift | 23 ++++++++++--- Sources/PublicInterface/Session.swift | 2 +- Sources/PublicInterface/WalletConnect.swift | 4 +-- Tests/ClientTests.swift | 10 +++--- Tests/Helpers.swift | 2 +- Tests/WalletConnectTests.swift | 2 +- WalletConnectSwift.xcodeproj/project.pbxproj | 4 ++- .../xcschemes/WalletConnectSwift.xcscheme | 28 +++++++-------- 12 files changed, 74 insertions(+), 58 deletions(-) diff --git a/Sources/Internal/Communicator.swift b/Sources/Internal/Communicator.swift index b948d3f7a..513a20196 100644 --- a/Sources/Internal/Communicator.swift +++ b/Sources/Internal/Communicator.swift @@ -29,8 +29,8 @@ class Communicator { return sessions.find(url: url) } - func addSession(_ session: Session) { - sessions.add(session) + func addOrUpdateSession(_ session: Session) { + sessions.addOrUpdate(session) } func removeSession(by url: WCURL) { @@ -53,8 +53,8 @@ class Communicator { return pendingDisconnectSessions.find(url: url) } - func addPendingDisconnectSession(_ session: Session) { - pendingDisconnectSessions.add(session) + func addOrUpdatePendingDisconnectSession(_ session: Session) { + pendingDisconnectSessions.addOrUpdate(session) } func removePendingDisconnectSession(by url: WCURL) { @@ -107,7 +107,7 @@ class Communicator { self.queue = queue } - func add(_ session: Session) { + func addOrUpdate(_ session: Session) { dispatchPrecondition(condition: .notOnQueue(queue)) queue.sync { [unowned self] in self.sessions[session.url] = session diff --git a/Sources/Internal/HandshakeHandler.swift b/Sources/Internal/HandshakeHandler.swift index 817c7d5c3..a0db0a0df 100644 --- a/Sources/Internal/HandshakeHandler.swift +++ b/Sources/Internal/HandshakeHandler.swift @@ -4,7 +4,7 @@ import Foundation -protocol HandshakeHandlerDelegate: class { +protocol HandshakeHandlerDelegate: AnyObject { // TODO: instead of IDType, use RequestID func handler(_ handler: HandshakeHandler, didReceiveRequestToCreateSession: Session, requestId: RequestID) } diff --git a/Sources/Internal/UpdateSessionHandler.swift b/Sources/Internal/UpdateSessionHandler.swift index 9775eb889..57859c356 100644 --- a/Sources/Internal/UpdateSessionHandler.swift +++ b/Sources/Internal/UpdateSessionHandler.swift @@ -5,7 +5,7 @@ import Foundation protocol UpdateSessionHandlerDelegate: AnyObject { - func handler(_ handler: UpdateSessionHandler, didUpdateSessionByURL: WCURL, approved: Bool) + func handler(_ handler: UpdateSessionHandler, didUpdateSessionByURL: WCURL, sessionInfo: SessionInfo) } class UpdateSessionHandler: RequestHandler { @@ -22,7 +22,7 @@ class UpdateSessionHandler: RequestHandler { func handle(request: Request) { do { let sessionInfo = try request.parameter(of: SessionInfo.self, at: 0) - delegate?.handler(self, didUpdateSessionByURL: request.url, approved: sessionInfo.approved) + delegate?.handler(self, didUpdateSessionByURL: request.url, sessionInfo: sessionInfo) } catch { LogService.shared.log("WC: wrong format of wc_sessionUpdate request: \(error)") // TODO: send error response @@ -30,8 +30,13 @@ class UpdateSessionHandler: RequestHandler { } } +/// https://docs.walletconnect.org/tech-spec#session-update struct SessionInfo: Decodable { var approved: Bool var accounts: [String]? - var chainId: Int + var chainId: Int? +} + +enum ChainID { + static let mainnet = 1 } diff --git a/Sources/PublicInterface/Client.swift b/Sources/PublicInterface/Client.swift index b014f14ae..09ad07854 100644 --- a/Sources/PublicInterface/Client.swift +++ b/Sources/PublicInterface/Client.swift @@ -4,7 +4,7 @@ import Foundation -public protocol ClientDelegate: class { +public protocol ClientDelegate: AnyObject { func client(_ client: Client, didFailToConnect url: WCURL) func client(_ client: Client, didConnect url: WCURL) func client(_ client: Client, didConnect session: Session) @@ -208,7 +208,7 @@ public class Client: WalletConnect { return } - communicator.addSession(session) + communicator.addOrUpdateSession(session) delegate?.client(self, didConnect: session) } catch { // TODO: handle error @@ -236,7 +236,9 @@ public class Client: WalletConnect { try! send(Response(request: request, error: .invalidJSON)) return } + guard let session = communicator.session(by: request.url) else { return } + if !info.approved { do { try disconnect(from: session) @@ -244,21 +246,19 @@ public class Client: WalletConnect { delegate?.client(self, didDisconnect: session) } } else { - if let walletInfo = session.walletInfo { - let updatedInfo = Session.WalletInfo( - approved: info.approved, - accounts: info.accounts ?? [], - chainId: info.chainId, - peerId: walletInfo.peerId, - peerMeta: walletInfo.peerMeta - ) - var updatedSesson = session - updatedSesson.walletInfo = updatedInfo - communicator.addSession(updatedSesson) - delegate?.client(self, didUpdate: updatedSesson) - } else { - delegate?.client(self, didUpdate: session) - } + // we do not add sessions without walletInfo + let walletInfo = session.walletInfo! + let updatedInfo = Session.WalletInfo( + approved: info.approved, + accounts: info.accounts ?? [], + chainId: info.chainId ?? ChainID.mainnet, + peerId: walletInfo.peerId, + peerMeta: walletInfo.peerMeta + ) + var updatedSesson = session + updatedSesson.walletInfo = updatedInfo + communicator.addOrUpdateSession(updatedSesson) + delegate?.client(self, didUpdate: updatedSesson) } } else { // TODO: error handling diff --git a/Sources/PublicInterface/Server.swift b/Sources/PublicInterface/Server.swift index a03c3d21b..f35b7d7ed 100644 --- a/Sources/PublicInterface/Server.swift +++ b/Sources/PublicInterface/Server.swift @@ -4,12 +4,12 @@ import Foundation -public protocol RequestHandler: class { +public protocol RequestHandler: AnyObject { func canHandle(request: Request) -> Bool func handle(request: Request) } -public protocol ServerDelegate: class { +public protocol ServerDelegate: AnyObject { /// Websocket connection was dropped during handshake. The connectoin process should be initiated again. func server(_ server: Server, didFailToConnect url: WCURL) @@ -172,7 +172,7 @@ extension Server: HandshakeHandlerDelegate { self.communicator.send(response, topic: session.dAppInfo.peerId) if walletInfo.approved { let updatedSession = Session(url: session.url, dAppInfo: session.dAppInfo, walletInfo: walletInfo) - self.communicator.addSession(updatedSession) + self.communicator.addOrUpdateSession(updatedSession) self.communicator.subscribe(on: walletInfo.peerId, url: updatedSession.url) self.delegate?.server(self, didConnect: updatedSession) } @@ -181,14 +181,27 @@ extension Server: HandshakeHandlerDelegate { } extension Server: UpdateSessionHandlerDelegate { - func handler(_ handler: UpdateSessionHandler, didUpdateSessionByURL url: WCURL, approved: Bool) { + func handler(_ handler: UpdateSessionHandler, didUpdateSessionByURL url: WCURL, sessionInfo: SessionInfo) { guard let session = communicator.session(by: url) else { return } - if !approved { + if !sessionInfo.approved { do { try disconnect(from: session) } catch { // session already disconnected delegate?.server(self, didDisconnect: session) } + } else { + // we do not add sessions without walletInfo + let walletInfo = session.walletInfo! + let updatedInfo = Session.WalletInfo( + approved: sessionInfo.approved, + accounts: sessionInfo.accounts ?? [], + chainId: sessionInfo.chainId ?? ChainID.mainnet, + peerId: walletInfo.peerId, + peerMeta: walletInfo.peerMeta + ) + var updatedSesson = session + updatedSesson.walletInfo = updatedInfo + communicator.addOrUpdateSession(updatedSesson) } } } diff --git a/Sources/PublicInterface/Session.swift b/Sources/PublicInterface/Session.swift index 5d1b07ec0..946353c5b 100644 --- a/Sources/PublicInterface/Session.swift +++ b/Sources/PublicInterface/Session.swift @@ -69,7 +69,7 @@ public struct Session: Codable { self.peerMeta = peerMeta } - func with(approved: Bool) -> WalletInfo { + public func with(approved: Bool) -> WalletInfo { return WalletInfo(approved: approved, accounts: self.accounts, chainId: self.chainId, diff --git a/Sources/PublicInterface/WalletConnect.swift b/Sources/PublicInterface/WalletConnect.swift index 79719b167..6873047d6 100644 --- a/Sources/PublicInterface/WalletConnect.swift +++ b/Sources/PublicInterface/WalletConnect.swift @@ -33,7 +33,7 @@ open class WalletConnect { guard session.walletInfo != nil else { throw WalletConnectError.missingWalletInfoInSession } - communicator.addSession(session) + communicator.addOrUpdateSession(session) listen(on: session.url) } @@ -46,7 +46,7 @@ open class WalletConnect { throw WalletConnectError.tryingToDisconnectInactiveSession } try sendDisconnectSessionRequest(for: session) - communicator.addPendingDisconnectSession(session) + communicator.addOrUpdatePendingDisconnectSession(session) communicator.disconnect(from: session.url) } diff --git a/Tests/ClientTests.swift b/Tests/ClientTests.swift index fc4cb0415..7f082e67e 100644 --- a/Tests/ClientTests.swift +++ b/Tests/ClientTests.swift @@ -23,7 +23,7 @@ class ClientTests: XCTestCase { } func test_sendRequest_whenNoWalletInfo_thenThrows() { - communicator.addSession(Session.testSessionWithoutWalletInfo) + communicator.addOrUpdateSession(Session.testSessionWithoutWalletInfo) XCTAssertThrowsError(try client.send(Request.testRequest, completion: nil), "missingWalletInfoInSession") { error in XCTAssertEqual(error as? Client.ClientError, .missingWalletInfoInSession) @@ -31,7 +31,7 @@ class ClientTests: XCTestCase { } func test_sendRequest_callsCommunicator() { - communicator.addSession(Session.testSession) + communicator.addOrUpdateSession(Session.testSession) try? client.send(Request.testRequest, completion: nil) XCTAssertNotNil(communicator.sentRequest) } @@ -68,12 +68,12 @@ class ClientTests: XCTestCase { @discardableResult private func prepareAccountWithTestSession() -> String { - communicator.addSession(Session.testSession) + communicator.addOrUpdateSession(Session.testSession) return Session.testSession.walletInfo!.accounts[0] } func test_onConnect_whenSessionExists_thenSubscribesOnDappPeerIdTopic() { - communicator.addSession(Session.testSession) + communicator.addOrUpdateSession(Session.testSession) client.onConnect(to: WCURL.testURL) XCTAssertEqual(communicator.subscribedOn?.topic, Session.testSession.dAppInfo.peerId) XCTAssertEqual(communicator.subscribedOn?.url, Session.testSession.url) @@ -86,7 +86,7 @@ class ClientTests: XCTestCase { } func test_onConnect_whenSessionExists_thenCallsDelegate() { - communicator.addSession(Session.testSession) + communicator.addOrUpdateSession(Session.testSession) XCTAssertNil(delegate.connectedSession) client.onConnect(to: WCURL.testURL) XCTAssertEqual(delegate.connectedSession, Session.testSession) diff --git a/Tests/Helpers.swift b/Tests/Helpers.swift index 5da1f78c5..2ac61e89c 100644 --- a/Tests/Helpers.swift +++ b/Tests/Helpers.swift @@ -16,7 +16,7 @@ class MockCommunicator: Communicator { didListen = true } - override func addSession(_ session: Session) { + override func addOrUpdateSession(_ session: Session) { sessions.append(session) } diff --git a/Tests/WalletConnectTests.swift b/Tests/WalletConnectTests.swift index 7a6f3d50b..88f3f444c 100644 --- a/Tests/WalletConnectTests.swift +++ b/Tests/WalletConnectTests.swift @@ -21,7 +21,7 @@ class WalletConnectTests: XCTestCase { } func test_whenConnectingToExistingURL_thenThrows() { - mockCommunicator.addSession(Session.testSession) + mockCommunicator.addOrUpdateSession(Session.testSession) XCTAssertThrowsError(try wc.connect(to: WCURL.testURL)) } diff --git a/WalletConnectSwift.xcodeproj/project.pbxproj b/WalletConnectSwift.xcodeproj/project.pbxproj index 650c82d34..91b5d649d 100644 --- a/WalletConnectSwift.xcodeproj/project.pbxproj +++ b/WalletConnectSwift.xcodeproj/project.pbxproj @@ -265,7 +265,7 @@ isa = PBXProject; attributes = { LastSwiftUpdateCheck = 1030; - LastUpgradeCheck = 1030; + LastUpgradeCheck = 1250; ORGANIZATIONNAME = "Gnosis Ltd."; TargetAttributes = { 0A92784F230BFB2600FDCC0D = { @@ -487,6 +487,7 @@ CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF = YES; CLANG_WARN_OBJC_LITERAL_CONVERSION = YES; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; + CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES; CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; CLANG_WARN_STRICT_PROTOTYPES = YES; CLANG_WARN_SUSPICIOUS_MOVE = YES; @@ -548,6 +549,7 @@ CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF = YES; CLANG_WARN_OBJC_LITERAL_CONVERSION = YES; CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; + CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES; CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; CLANG_WARN_STRICT_PROTOTYPES = YES; CLANG_WARN_SUSPICIOUS_MOVE = YES; diff --git a/WalletConnectSwift.xcodeproj/xcshareddata/xcschemes/WalletConnectSwift.xcscheme b/WalletConnectSwift.xcodeproj/xcshareddata/xcschemes/WalletConnectSwift.xcscheme index 163c4f4d0..e8b81a6ad 100644 --- a/WalletConnectSwift.xcodeproj/xcshareddata/xcschemes/WalletConnectSwift.xcscheme +++ b/WalletConnectSwift.xcodeproj/xcshareddata/xcschemes/WalletConnectSwift.xcscheme @@ -1,6 +1,6 @@ + onlyGenerateCoverageForSpecifiedTargets = "YES"> + + + + - - - - - - - -