Merge pull request #641 from WalletConnect/bugfix/pairing-metadata-#640

[Sign] PeerMetadata for Pairing
WalletConnect · Dec 22, 2022 · 6f3d392 · 6f3d392
2 parents f21eb29 + 18cd7bb
commit 6f3d392
Show file tree

Hide file tree

Showing 13 changed files with 82 additions and 66 deletions.
diff --git a/Sources/Auth/Services/App/AppRespondSubscriber.swift b/Sources/Auth/Services/App/AppRespondSubscriber.swift
@@ -37,7 +37,8 @@ class AppRespondSubscriber {
         networkingInteractor.responseSubscription(on: AuthRequestProtocolMethod())
             .sink { [unowned self] (payload: ResponseSubscriptionPayload<AuthRequestParams, Cacao>)  in
 
-                pairingRegisterer.activate(pairingTopic: payload.topic)
+                pairingRegisterer.activate(pairingTopic: payload.topic, peerMetadata: nil)
+
                 networkingInteractor.unsubscribe(topic: payload.topic)
 
                 let requestId = payload.id

diff --git a/Sources/Auth/Services/Wallet/WalletRequestSubscriber.swift b/Sources/Auth/Services/Wallet/WalletRequestSubscriber.swift
@@ -28,7 +28,11 @@ class WalletRequestSubscriber {
         pairingRegisterer.register(method: AuthRequestProtocolMethod())
             .sink { [unowned self] (payload: RequestSubscriptionPayload<AuthRequestParams>) in
                 logger.debug("WalletRequestSubscriber: Received request")
-                pairingRegisterer.activate(pairingTopic: payload.topic)
+
+                pairingRegisterer.activate(
+                    pairingTopic: payload.topic,
+                    peerMetadata: payload.request.requester.metadata
+                )
 
                 let request = AuthRequest(id: payload.id, payload: payload.request.payloadParams)
                 onRequest?(request)

diff --git a/Sources/WalletConnectPairing/PairingClient.swift b/Sources/WalletConnectPairing/PairingClient.swift
@@ -10,7 +10,6 @@ public class PairingClient: PairingRegisterer, PairingInteracting {
     private let walletPairService: WalletPairService
     private let appPairService: AppPairService
     private let appPairActivateService: AppPairActivationService
-    private let appUpdateMetadataService: AppUpdateMetadataService
     private var pingResponsePublisherSubject = PassthroughSubject<String, Never>()
     private let logger: ConsoleLogging
     private let pingService: PairingPingService
@@ -32,7 +31,6 @@ public class PairingClient: PairingRegisterer, PairingInteracting {
          expirationService: ExpirationService,
          pairingRequestsSubscriber: PairingRequestsSubscriber,
          appPairActivateService: AppPairActivationService,
-         appUpdateMetadataService: AppUpdateMetadataService,
          cleanupService: PairingCleanupService,
          pingService: PairingPingService,
          socketConnectionStatusPublisher: AnyPublisher<SocketConnectionStatus, Never>,
@@ -45,7 +43,6 @@ public class PairingClient: PairingRegisterer, PairingInteracting {
         self.logger = logger
         self.deletePairingService = deletePairingService
         self.appPairActivateService = appPairActivateService
-        self.appUpdateMetadataService = appUpdateMetadataService
         self.resubscribeService = resubscribeService
         self.expirationService = expirationService
         self.cleanupService = cleanupService
@@ -81,12 +78,8 @@ public class PairingClient: PairingRegisterer, PairingInteracting {
         return try await appPairService.create()
     }
 
-    public func activate(pairingTopic: String) {
-        appPairActivateService.activate(for: pairingTopic)
-    }
-
-    public func updateMetadata(_ topic: String, metadata: AppMetadata) {
-        appUpdateMetadataService.updatePairingMetadata(topic: topic, metadata: metadata)
+    public func activate(pairingTopic: String, peerMetadata: AppMetadata?) {
+        appPairActivateService.activate(for: pairingTopic, peerMetadata: peerMetadata)
     }
 
     public func getPairings() -> [Pairing] {

diff --git a/Sources/WalletConnectPairing/PairingClientFactory.swift b/Sources/WalletConnectPairing/PairingClientFactory.swift
@@ -20,7 +20,6 @@ public struct PairingClientFactory {
         let deletePairingService = DeletePairingService(networkingInteractor: networkingClient, kms: kms, pairingStorage: pairingStore, logger: logger)
         let pingService = PairingPingService(pairingStorage: pairingStore, networkingInteractor: networkingClient, logger: logger)
         let appPairActivateService = AppPairActivationService(pairingStorage: pairingStore, logger: logger)
-        let appUpdateMetadataService = AppUpdateMetadataService(pairingStore: pairingStore)
         let expirationService = ExpirationService(pairingStorage: pairingStore, networkInteractor: networkingClient, kms: kms)
         let resubscribeService = ResubscribeService(networkInteractor: networkingClient, pairingStorage: pairingStore)
 
@@ -34,7 +33,6 @@ public struct PairingClientFactory {
             expirationService: expirationService,
             pairingRequestsSubscriber: pairingRequestsSubscriber,
             appPairActivateService: appPairActivateService,
-            appUpdateMetadataService: appUpdateMetadataService,
             cleanupService: cleanupService,
             pingService: pingService,
             socketConnectionStatusPublisher: networkingClient.socketConnectionStatusPublisher,

diff --git a/Sources/WalletConnectPairing/PairingRegisterer.swift b/Sources/WalletConnectPairing/PairingRegisterer.swift
@@ -6,7 +6,6 @@ public protocol PairingRegisterer {
         method: ProtocolMethod
     ) -> AnyPublisher<RequestSubscriptionPayload<RequestParams>, Never>
 
-    func activate(pairingTopic: String)
+    func activate(pairingTopic: String, peerMetadata: AppMetadata?)
     func validatePairingExistance(_ topic: String) throws
-    func updateMetadata(_ topic: String, metadata: AppMetadata)
 }
diff --git a/Sources/WalletConnectPairing/Services/App/AppPairActivationService.swift b/Sources/WalletConnectPairing/Services/App/AppPairActivationService.swift
@@ -2,23 +2,30 @@ import Foundation
 import Combine
 
 final class AppPairActivationService {
-    private let pairingStorage: PairingStorage
+    enum Errors: Error {
+        case pairingNotFound
+    }
+
+    private let pairingStorage: WCPairingStorage
     private let logger: ConsoleLogging
 
-    init(pairingStorage: PairingStorage, logger: ConsoleLogging) {
+    init(pairingStorage: WCPairingStorage, logger: ConsoleLogging) {
         self.pairingStorage = pairingStorage
         self.logger = logger
     }
 
-    func activate(for topic: String) {
+    func activate(for topic: String, peerMetadata: AppMetadata?) {
         guard var pairing = pairingStorage.getPairing(forTopic: topic) else {
             return logger.error("Pairing not found for topic: \(topic)")
         }
+
         if !pairing.active {
             pairing.activate()
         } else {
             try? pairing.updateExpiry()
         }
+
+        pairing.updatePeerMetadata(peerMetadata)
         pairingStorage.setPairing(pairing)
     }
 }
diff --git a/Sources/WalletConnectPairing/Services/App/AppUpdateMetadataService.swift b/Sources/WalletConnectPairing/Services/App/AppUpdateMetadataService.swift
diff --git a/Sources/WalletConnectPairing/Types/WCPairing.swift b/Sources/WalletConnectPairing/Types/WCPairing.swift
@@ -7,8 +7,8 @@ public struct WCPairing: SequenceObject {
 
     public let topic: String
     public let relay: RelayProtocolOptions
-    public var peerMetadata: AppMetadata?
 
+    public private (set) var peerMetadata: AppMetadata?
     public private (set) var expiryDate: Date
     public private (set) var active: Bool
 
@@ -53,6 +53,10 @@ public struct WCPairing: SequenceObject {
         try? updateExpiry()
     }
 
+    public mutating func updatePeerMetadata(_ metadata: AppMetadata?) {
+        peerMetadata = metadata
+    }
+
     public mutating func updateExpiry(_ ttl: TimeInterval = WCPairing.timeToLiveActive) throws {
         let now = Self.dateInitializer()
         let newExpiryDate = now.advanced(by: ttl)

diff --git a/Sources/WalletConnectSign/Engine/Common/ApproveEngine.swift b/Sources/WalletConnectSign/Engine/Common/ApproveEngine.swift
@@ -56,6 +56,7 @@ final class ApproveEngine {
     }
 
     func approveProposal(proposerPubKey: String, validating sessionNamespaces: [String: SessionNamespace]) async throws {
+
         guard let payload = try proposalPayloadsStore.get(key: proposerPubKey) else {
             throw Errors.wrongRequestParams
         }
@@ -76,15 +77,12 @@ final class ApproveEngine {
 
         let sessionTopic = agreementKey.derivedTopic()
         try kms.setAgreementSecret(agreementKey, topic: sessionTopic)
+        sessionToPairingTopic.set(payload.topic, forKey: sessionTopic)
 
         guard let relay = proposal.relays.first else {
             throw Errors.relayNotFound
         }
 
-        guard var pairing = pairingStore.getPairing(forTopic: payload.topic) else {
-            throw Errors.pairingNotFound
-        }
-
         let result = SessionType.ProposeResponse(relay: relay, responderPublicKey: selfPublicKey.hexRepresentation)
         let response = RPCResponse(id: payload.id, result: result)
 
@@ -94,8 +92,10 @@ final class ApproveEngine {
 
         _ = try await [proposeResponse, settleRequest]
 
-        try pairing.updateExpiry()
-        pairingStore.setPairing(pairing)
+        pairingRegisterer.activate(
+            pairingTopic: payload.topic,
+            peerMetadata: payload.request.proposer.metadata
+        )
     }
 
     func reject(proposerPubKey: String, reason: SignReasonCode) async throws {
@@ -209,29 +209,14 @@ private extension ApproveEngine {
     // TODO: Move to Non-Controller SettleEngine
     func handleSessionProposeResponse(payload: ResponseSubscriptionPayload<SessionType.ProposeParams, SessionType.ProposeResponse>) {
         do {
-            let pairingTopic = payload.topic
-
-            guard var pairing = pairingStore.getPairing(forTopic: pairingTopic) else {
-                throw Errors.pairingNotFound
-            }
-
-            // Activate the pairing
-            if !pairing.active {
-                pairing.activate()
-            } else {
-                try pairing.updateExpiry()
-            }
-
-            pairingStore.setPairing(pairing)
-
             let selfPublicKey = try AgreementPublicKey(hex: payload.request.proposer.publicKey)
             let agreementKeys = try kms.performKeyAgreement(selfPublicKey: selfPublicKey, peerPublicKey: payload.response.responderPublicKey)
 
             let sessionTopic = agreementKeys.derivedTopic()
             logger.debug("Received Session Proposal response")
 
             try kms.setAgreementSecret(agreementKeys, topic: sessionTopic)
-            sessionToPairingTopic.set(pairingTopic, forKey: sessionTopic)
+            sessionToPairingTopic.set(payload.topic, forKey: sessionTopic)
 
             settlingProposal = payload.request
 
@@ -329,8 +314,12 @@ private extension ApproveEngine {
             publicKey: agreementKeys.publicKey.hexRepresentation,
             metadata: metadata
         )
+
         if let pairingTopic = try? sessionToPairingTopic.get(key: topic) {
-            pairingRegisterer.updateMetadata(pairingTopic, metadata: params.controller.metadata)
+            pairingRegisterer.activate(
+                pairingTopic: pairingTopic,
+                peerMetadata: params.controller.metadata
+            )
         }
 
         let session = WCSession(
@@ -343,6 +332,7 @@ private extension ApproveEngine {
             acknowledged: true
         )
         sessionStore.setSession(session)
+
         Task(priority: .high) {
             try await networkingInteractor.respondSuccess(topic: payload.topic, requestId: payload.id, protocolMethod: protocolMethod)
         }

diff --git a/Tests/TestingUtils/Mocks/PairingRegistererMock.swift b/Tests/TestingUtils/Mocks/PairingRegistererMock.swift
@@ -13,15 +13,11 @@ public class PairingRegistererMock<RequestParams>: PairingRegisterer where Reque
         subject.eraseToAnyPublisher() as! AnyPublisher<RequestSubscriptionPayload<RequestParams>, Never>
     }
 
-    public func activate(pairingTopic: String) {
+    public func activate(pairingTopic: String, peerMetadata: WalletConnectPairing.AppMetadata?) {
         isActivateCalled = true
     }
 
     public func validatePairingExistance(_ topic: String) throws {
 
     }
-
-    public func updateMetadata(_ topic: String, metadata: AppMetadata) {
-
-    }
 }
diff --git a/Tests/WalletConnectPairingTests/AppPairActivationServiceTests.swift b/Tests/WalletConnectPairingTests/AppPairActivationServiceTests.swift
@@ -0,0 +1,42 @@
+import XCTest
+@testable import WalletConnectPairing
+@testable import TestingUtils
+import WalletConnectUtils
+
+final class AppPairActivationServiceTests: XCTestCase {
+
+    var service: AppPairActivationService!
+    var storageMock: WCPairingStorage!
+    var logger: ConsoleLogger!
+
+    override func setUp() {
+        storageMock = WCPairingStorageMock()
+        logger = ConsoleLogger()
+        service = AppPairActivationService(pairingStorage: storageMock, logger: logger)
+    }
+
+    override func tearDown() {
+        storageMock = nil
+        logger = nil
+        service = nil
+    }
+
+    func testActivate() {
+        let topic = "topic"
+        let pairing = WCPairing(topic: topic)
+        let date = pairing.expiryDate
+
+        storageMock.setPairing(pairing)
+
+        XCTAssertFalse(pairing.active)
+        XCTAssertNil(pairing.peerMetadata)
+
+        service.activate(for: topic, peerMetadata: .stub())
+
+        let activated = storageMock.getPairing(forTopic: topic)!
+
+        XCTAssertTrue(activated.active)
+        XCTAssertNotNil(activated.peerMetadata)
+        XCTAssertTrue(activated.expiryDate > date)
+    }
+}
diff --git a/Tests/WalletConnectSignTests/AppProposalServiceTests.swift b/Tests/WalletConnectSignTests/AppProposalServiceTests.swift
@@ -124,7 +124,6 @@ final class AppProposalServiceTests: XCTestCase {
         let sessionTopic = networkingInteractor.subscriptions.last!
 
         XCTAssertTrue(networkingInteractor.didCallSubscribe)
-        XCTAssert(storedPairing.active)
         XCTAssertEqual(topicB, sessionTopic, "Responder engine calls back with session topic")
     }
 

diff --git a/Tests/WalletConnectSignTests/ApproveEngineTests.swift b/Tests/WalletConnectSignTests/ApproveEngineTests.swift
@@ -64,11 +64,10 @@ final class ApproveEngineTests: XCTestCase {
 
         let topicB = networkingInteractor.subscriptions.last!
 
-        let extendedPairing = pairingStorageMock.getPairing(forTopic: topicA)!
         XCTAssertTrue(networkingInteractor.didCallSubscribe)
         XCTAssert(cryptoMock.hasAgreementSecret(for: topicB), "Responder must store agreement key for topic B")
         XCTAssertEqual(networkingInteractor.didRespondOnTopic!, topicA, "Responder must respond on topic A")
-        XCTAssertEqual(extendedPairing.expiryDate.timeIntervalSince1970, Date(timeIntervalSinceNow: 2_592_000).timeIntervalSince1970, accuracy: 1, "pairing expiry has been extended by 30 days")
+        XCTAssertTrue(pairingRegisterer.isActivateCalled)
     }
 
     func testReceiveProposal() {