-
Notifications
You must be signed in to change notification settings - Fork 173
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
[W3W] Add pending proposals #839
Conversation
# Conflicts: # Example/WalletApp/PresentationLayer/Wallet/Wallet/WalletView.swift # Sources/WalletConnectSign/Engine/Common/ApproveEngine.swift # Sources/WalletConnectSign/Sign/SignClientFactory.swift
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are unresolved comments in your previous PR
#744
wouldn't it be easier to reopen the last one so we can continue discussion?
|
||
requestSubscriptionPayloads.forEach { | ||
let proposal = $0.request | ||
proposalPayloadsStore.set($0, forKey: proposal.proposer.publicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where are we reading from that store? Couldn't find
# Conflicts: # Sources/Auth/AuthClientFactory.swift # Sources/Auth/Services/Wallet/WalletRequestSubscriber.swift # Sources/WalletConnectSign/Engine/Common/ApproveEngine.swift # Sources/WalletConnectSign/Engine/Common/SessionEngine.swift # Sources/WalletConnectSign/Sign/SignClientFactory.swift # Tests/AuthTests/WalletRequestSubscriberTests.swift # Tests/WalletConnectSignTests/SessionEngineTests.swift
Sources/Auth/AuthClientFactory.swift
Outdated
@@ -1,15 +1,15 @@ | |||
import Foundation | |||
|
|||
public struct AuthClientFactory { | |||
import WalletConnectVerify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will brake cocoapods
/// - Parameter id: id of a wc_sessionRequest jsonrpc request | ||
/// - Returns: json rpc record object for given id or nil if record for give id does not exits | ||
public func getSessionRequestRecord(id: RPCID) -> Request? { | ||
public func getSessionRequestRecord(id: RPCID) -> (request: Request, context: VerifyContext?)? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about separate getVerifyContext(id: RPCID)
.
To not affect current Client interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss it on sync
@@ -1,5 +1,7 @@ | |||
import Foundation | |||
|
|||
import WalletConnectVerify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will brake cocoapods
# Conflicts: # Sources/Auth/Services/Wallet/WalletRequestSubscriber.swift # Sources/WalletConnectSign/Engine/Common/SessionEngine.swift
Description
Resolves # (issue)
How Has This Been Tested?
Due Dilligence