Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Auth, Sign] #485 remove pairing #486

Merged
merged 14 commits into from
Sep 5, 2022
Merged

[Auth, Sign] #485 remove pairing #486

merged 14 commits into from
Sep 5, 2022

Conversation

llbartekll
Copy link
Contributor

@llbartekll llbartekll commented Sep 5, 2022

Description

  • Add disconnect() method to auth client api. Removes pairing
  • Add disconnect service for Sign

Resolves #485 #447

How Has This Been Tested?

Due Dilligence

  • Breaking change
  • Requires a documentation update

@llbartekll llbartekll changed the title [Auth] #485 remove pairing [Auth, Sign] #485 remove pairing Sep 5, 2022
import WalletConnectUtils
import WalletConnectPairing

class DeletePairingService {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move it in WalletConnectPairing package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see. Whey using different NetworkInteractors..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah exactly, even the protocol method is the same

if pairingStorage.hasPairing(forTopic: topic) {
try await deletePairingService.delete(topic: topic)
} else if sessionStorage.hasSession(forTopic: topic) {
try await deleteSessionService.delete(topic: topic)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pairing and a session at the same time? I mean: do we need to clear pairing and session together on disconnect call?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if so, I think is better to have 1 DeleteSessionService, which clean pairing and session storage

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one topic corresponds to one pairing or one session and we have a common public method for deletion.
So we either delete a pairing or a session.

…V2 into #447-remove-pairing

# Conflicts:
#	Package.swift
#	Sources/Auth/Services/Common/NetworkingInteractor.swift
#	Sources/Chat/ChatClient.swift
#	Sources/Chat/ChatClientFactory.swift
#	Sources/Chat/ProtocolServices/Common/MessagingService.swift
#	Sources/Chat/ProtocolServices/Invitee/InvitationHandlingService.swift
#	Sources/Chat/ProtocolServices/Inviter/InviteService.swift
#	Sources/Chat/Types/Message.swift
#	Sources/WalletConnectNetworking/NetworkInteractor.swift
#	Sources/WalletConnectNetworking/NetworkingInteractor.swift
#	Sources/WalletConnectNetworking/RequestSubscriptionPayload.swift
#	Sources/WalletConnectNetworking/ResponseSubscriptionPayload.swift
#	Tests/TestingUtils/NetworkingInteractorMock.swift
@llbartekll llbartekll requested a review from flypaper0 September 5, 2022 11:35
@llbartekll llbartekll merged commit 918c8e1 into develop Sep 5, 2022
@llbartekll llbartekll deleted the #447-remove-pairing branch September 5, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants