From 0391948e295ef6bad14af8c73e4fcddf54fe0cd5 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 15 May 2024 12:14:30 +0300 Subject: [PATCH 1/4] Add invalid requests sanitiser --- .../Engine/Common/SessionEngine.swift | 14 +++- .../Services/HistoryService.swift | 18 ++++- .../Services/InvalidRequestsSanitiser.swift | 20 ++++++ .../Sign/SignClientFactory.swift | 3 +- .../RPCHistory/RPCHistory.swift | 16 ++++- .../InvalidRequestsSanitiserTests.swift | 69 +++++++++++++++++++ .../SessionEngineTests.swift | 32 ++++----- 7 files changed, 149 insertions(+), 23 deletions(-) create mode 100644 Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift create mode 100644 Tests/WalletConnectSignTests/InvalidRequestsSanitiserTests.swift diff --git a/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift b/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift index 435f34f98..5477f86a9 100644 --- a/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift +++ b/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift @@ -26,6 +26,7 @@ final class SessionEngine { private var publishers = [AnyCancellable]() private let logger: ConsoleLogging private let sessionRequestsProvider: SessionRequestsProvider + private let invalidRequestsSanitiser: InvalidRequestsSanitiser init( networkingInteractor: NetworkInteracting, @@ -35,7 +36,8 @@ final class SessionEngine { kms: KeyManagementServiceProtocol, sessionStore: WCSessionStorage, logger: ConsoleLogging, - sessionRequestsProvider: SessionRequestsProvider + sessionRequestsProvider: SessionRequestsProvider, + invalidRequestsSanitiser: InvalidRequestsSanitiser ) { self.networkingInteractor = networkingInteractor self.historyService = historyService @@ -45,6 +47,7 @@ final class SessionEngine { self.sessionStore = sessionStore self.logger = logger self.sessionRequestsProvider = sessionRequestsProvider + self.invalidRequestsSanitiser = invalidRequestsSanitiser setupConnectionSubscriptions() setupRequestSubscriptions() @@ -52,8 +55,15 @@ final class SessionEngine { setupUpdateSubscriptions() setupExpirationSubscriptions() DispatchQueue.main.asyncAfter(deadline: .now() + 1) { [weak self] in - sessionRequestsProvider.emitRequestIfPending() + self?.sessionRequestsProvider.emitRequestIfPending() } + + removeInvalidSessionRequests() + } + + private func removeInvalidSessionRequests() { + let sessionTopics = Set(sessionStore.getAll().map {$0.topic}) + invalidRequestsSanitiser.removeInvalidSessionRequests(validSessionTopics: sessionTopics) } func hasSession(for topic: String) -> Bool { diff --git a/Sources/WalletConnectSign/Services/HistoryService.swift b/Sources/WalletConnectSign/Services/HistoryService.swift index 6a3b8a01b..ddb44c1ab 100644 --- a/Sources/WalletConnectSign/Services/HistoryService.swift +++ b/Sources/WalletConnectSign/Services/HistoryService.swift @@ -1,6 +1,10 @@ import Foundation -final class HistoryService { +protocol HistoryServiceProtocol { + func getPendingRequests() -> [(request: Request, context: VerifyContext?)] +} + +final class HistoryService: HistoryServiceProtocol { private let history: RPCHistory private let verifyContextStore: CodableStore @@ -25,6 +29,8 @@ final class HistoryService { getPendingRequestsSortedByTimestamp() } + + func getPendingRequestsSortedByTimestamp() -> [(request: Request, context: VerifyContext?)] { let requests = history.getPending() .compactMap { mapRequestRecord($0) } @@ -80,3 +86,13 @@ private extension HistoryService { return (mappedRequest, record.id, record.timestamp) } } + +#if DEBUG +class MockHistoryService: HistoryServiceProtocol { + var pendingRequests: [(request: Request, context: VerifyContext?)] = [] + + func getPendingRequests() -> [(request: Request, context: VerifyContext?)] { + return pendingRequests + } +} +#endif diff --git a/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift new file mode 100644 index 000000000..1ffc40dd7 --- /dev/null +++ b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift @@ -0,0 +1,20 @@ + +import Foundation + +class InvalidRequestsSanitiser { + let historyService: HistoryServiceProtocol + private let history: RPCHistoryProtocol + + init(historyService: HistoryServiceProtocol, history: RPCHistoryProtocol) { + self.historyService = historyService + self.history = history + } + + func removeInvalidSessionRequests(validSessionTopics: Set) { + let pendingRequests = historyService.getPendingRequests() + let invalidTopics = Set(pendingRequests.map { $0.request.topic }).subtracting(validSessionTopics) + if !invalidTopics.isEmpty { + history.deleteAll(forTopics: Array(invalidTopics)) + } + } +} diff --git a/Sources/WalletConnectSign/Sign/SignClientFactory.swift b/Sources/WalletConnectSign/Sign/SignClientFactory.swift index bfbdcc6b9..33143fb43 100644 --- a/Sources/WalletConnectSign/Sign/SignClientFactory.swift +++ b/Sources/WalletConnectSign/Sign/SignClientFactory.swift @@ -61,7 +61,8 @@ public struct SignClientFactory { let historyService = HistoryService(history: rpcHistory, verifyContextStore: verifyContextStore) let verifyClient = VerifyClientFactory.create() let sessionRequestsProvider = SessionRequestsProvider(historyService: historyService) - let sessionEngine = SessionEngine(networkingInteractor: networkingClient, historyService: historyService, verifyContextStore: verifyContextStore, verifyClient: verifyClient, kms: kms, sessionStore: sessionStore, logger: logger, sessionRequestsProvider: sessionRequestsProvider) + let invalidRequestsSanitiser = InvalidRequestsSanitiser(historyService: historyService, history: rpcHistory) + let sessionEngine = SessionEngine(networkingInteractor: networkingClient, historyService: historyService, verifyContextStore: verifyContextStore, verifyClient: verifyClient, kms: kms, sessionStore: sessionStore, logger: logger, sessionRequestsProvider: sessionRequestsProvider, invalidRequestsSanitiser: invalidRequestsSanitiser) let nonControllerSessionStateMachine = NonControllerSessionStateMachine(networkingInteractor: networkingClient, kms: kms, sessionStore: sessionStore, logger: logger) let controllerSessionStateMachine = ControllerSessionStateMachine(networkingInteractor: networkingClient, kms: kms, sessionStore: sessionStore, logger: logger) let sessionExtendRequester = SessionExtendRequester(sessionStore: sessionStore, networkingInteractor: networkingClient) diff --git a/Sources/WalletConnectUtils/RPCHistory/RPCHistory.swift b/Sources/WalletConnectUtils/RPCHistory/RPCHistory.swift index ff6bda280..7060c89ae 100644 --- a/Sources/WalletConnectUtils/RPCHistory/RPCHistory.swift +++ b/Sources/WalletConnectUtils/RPCHistory/RPCHistory.swift @@ -1,6 +1,10 @@ import Foundation -public final class RPCHistory { +public protocol RPCHistoryProtocol { + func deleteAll(forTopics topics: [String]) +} + +public final class RPCHistory: RPCHistoryProtocol { public struct Record: Codable { public enum Origin: String, Codable { @@ -144,3 +148,13 @@ extension RPCHistory { } } } + +#if DEBUG +class MockRPCHistory: RPCHistoryProtocol { + var deletedTopics: [String] = [] + + func deleteAll(forTopics topics: [String]) { + deletedTopics.append(contentsOf: topics) + } +} +#endif diff --git a/Tests/WalletConnectSignTests/InvalidRequestsSanitiserTests.swift b/Tests/WalletConnectSignTests/InvalidRequestsSanitiserTests.swift new file mode 100644 index 000000000..dda89b064 --- /dev/null +++ b/Tests/WalletConnectSignTests/InvalidRequestsSanitiserTests.swift @@ -0,0 +1,69 @@ +import XCTest +@testable import WalletConnectSign +@testable import WalletConnectUtils + +class InvalidRequestsSanitiserTests: XCTestCase { + var sanitiser: InvalidRequestsSanitiser! + var mockHistoryService: MockHistoryService! + var mockRPCHistory: MockRPCHistory! + + override func setUp() { + super.setUp() + mockHistoryService = MockHistoryService() + mockRPCHistory = MockRPCHistory() + sanitiser = InvalidRequestsSanitiser(historyService: mockHistoryService, history: mockRPCHistory) + } + + override func tearDown() { + sanitiser = nil + mockHistoryService = nil + mockRPCHistory = nil + super.tearDown() + } + + func testRemoveInvalidSessionRequests_noPendingRequests() { + let validSessionTopics: Set = ["validTopic1", "validTopic2"] + + sanitiser.removeInvalidSessionRequests(validSessionTopics: validSessionTopics) + + XCTAssertTrue(mockRPCHistory.deletedTopics.isEmpty) + } + + func testRemoveInvalidSessionRequests_allRequestsValid() { + let validSessionTopics: Set = ["validTopic1", "validTopic2"] + mockHistoryService.pendingRequests = [ + (request: try! Request(topic: "validTopic1", method: "method1", params: AnyCodable("params1"), chainId: Blockchain("eip155:1")!), context: nil), + (request: try! Request(topic: "validTopic2", method: "method2", params: AnyCodable("params2"), chainId: Blockchain("eip155:1")!), context: nil) + ] + + sanitiser.removeInvalidSessionRequests(validSessionTopics: validSessionTopics) + + XCTAssertTrue(mockRPCHistory.deletedTopics.isEmpty) + } + + func testRemoveInvalidSessionRequests_someRequestsInvalid() { + let validSessionTopics: Set = ["validTopic1", "validTopic2"] + mockHistoryService.pendingRequests = [ + (request: try! Request(topic: "validTopic1", method: "method1", params: AnyCodable("params1"), chainId: Blockchain("eip155:1")!), context: nil), + (request: try! Request(topic: "invalidTopic1", method: "method2", params: AnyCodable("params2"), chainId: Blockchain("eip155:1")!), context: nil), + (request: try! Request(topic: "invalidTopic2", method: "method3", params: AnyCodable("params3"), chainId: Blockchain("eip155:1")!), context: nil) + ] + + sanitiser.removeInvalidSessionRequests(validSessionTopics: validSessionTopics) + + XCTAssertEqual(mockRPCHistory.deletedTopics.sorted(), ["invalidTopic1", "invalidTopic2"]) + } + + func testRemoveInvalidSessionRequests_withEmptyValidSessionTopics() { + let validSessionTopics: Set = [] + + mockHistoryService.pendingRequests = [ + (request: try! Request(topic: "invalidTopic1", method: "method1", params: AnyCodable("params1"), chainId: Blockchain("eip155:1")!), context: nil), + (request: try! Request(topic: "invalidTopic2", method: "method2", params: AnyCodable("params2"), chainId: Blockchain("eip155:1")!), context: nil) + ] + + sanitiser.removeInvalidSessionRequests(validSessionTopics: validSessionTopics) + + XCTAssertEqual(mockRPCHistory.deletedTopics.sorted(), ["invalidTopic1", "invalidTopic2"]) + } +} diff --git a/Tests/WalletConnectSignTests/SessionEngineTests.swift b/Tests/WalletConnectSignTests/SessionEngineTests.swift index e3a42232e..a6854c5bc 100644 --- a/Tests/WalletConnectSignTests/SessionEngineTests.swift +++ b/Tests/WalletConnectSignTests/SessionEngineTests.swift @@ -13,33 +13,29 @@ final class SessionEngineTests: XCTestCase { override func setUp() { networkingInteractor = NetworkingInteractorMock() sessionStorage = WCSessionStorageMock() + let defaults = RuntimeKeyValueStorage() + let rpcHistory = RPCHistory( + keyValueStore: .init( + defaults: defaults, + identifier: "" + ) + ) verifyContextStore = CodableStore(defaults: RuntimeKeyValueStorage(), identifier: "") + let historyService = HistoryService( + history: rpcHistory, + verifyContextStore: verifyContextStore + ) engine = SessionEngine( networkingInteractor: networkingInteractor, - historyService: HistoryService( - history: RPCHistory( - keyValueStore: .init( - defaults: RuntimeKeyValueStorage(), - identifier: "" - ) - ), - verifyContextStore: verifyContextStore - ), + historyService: historyService, verifyContextStore: verifyContextStore, verifyClient: VerifyClientMock(), kms: KeyManagementServiceMock(), sessionStore: sessionStorage, logger: ConsoleLoggerMock(), sessionRequestsProvider: SessionRequestsProvider( - historyService: HistoryService( - history: RPCHistory( - keyValueStore: .init( - defaults: RuntimeKeyValueStorage(), - identifier: "" - ) - ), - verifyContextStore: verifyContextStore - )) + historyService: historyService), + invalidRequestsSanitiser: InvalidRequestsSanitiser(historyService: historyService, history: rpcHistory) ) } From e4812ac720924688baa6a9580e5c4190672a660f Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 15 May 2024 14:26:12 +0300 Subject: [PATCH 2/4] Update Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift Co-authored-by: Jack Pooley <169029079+jackpooleywc@users.noreply.github.com> --- .../WalletConnectSign/Services/InvalidRequestsSanitiser.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift index 1ffc40dd7..f68800f0e 100644 --- a/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift +++ b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift @@ -1,7 +1,7 @@ import Foundation -class InvalidRequestsSanitiser { +final class InvalidRequestsSanitiser { let historyService: HistoryServiceProtocol private let history: RPCHistoryProtocol From a07eb760d6618131fe5f6083e3bee569d643ecb7 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 15 May 2024 14:26:23 +0300 Subject: [PATCH 3/4] Update Sources/WalletConnectSign/Engine/Common/SessionEngine.swift Co-authored-by: Jack Pooley <169029079+jackpooleywc@users.noreply.github.com> --- Sources/WalletConnectSign/Engine/Common/SessionEngine.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift b/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift index 5477f86a9..08cd1d0f6 100644 --- a/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift +++ b/Sources/WalletConnectSign/Engine/Common/SessionEngine.swift @@ -62,7 +62,7 @@ final class SessionEngine { } private func removeInvalidSessionRequests() { - let sessionTopics = Set(sessionStore.getAll().map {$0.topic}) + let sessionTopics = Set(sessionStore.getAll().map(\.topic)) invalidRequestsSanitiser.removeInvalidSessionRequests(validSessionTopics: sessionTopics) } From bc231c8d9fb541c4ef1ec96c8832acef48ec39d7 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Wed, 15 May 2024 14:29:09 +0300 Subject: [PATCH 4/4] update --- .../WalletConnectSign/Services/InvalidRequestsSanitiser.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift index f68800f0e..31793769b 100644 --- a/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift +++ b/Sources/WalletConnectSign/Services/InvalidRequestsSanitiser.swift @@ -2,7 +2,7 @@ import Foundation final class InvalidRequestsSanitiser { - let historyService: HistoryServiceProtocol + private let historyService: HistoryServiceProtocol private let history: RPCHistoryProtocol init(historyService: HistoryServiceProtocol, history: RPCHistoryProtocol) {