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

#54 Public account type #65

Merged
merged 9 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Example/DApp/ClientDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ClientDelegate: WalletConnectClientDelegate {
onSessionResponse?(sessionResponse)
}

func didUpdate(sessionTopic: String, accounts: Set<String>) {
func didUpdate(sessionTopic: String, accounts: Set<Account>) {
}

func didUpgrade(sessionTopic: String, permissions: Session.Permissions) {
Expand Down
6 changes: 3 additions & 3 deletions Example/ExampleApp/Responder/ResponderViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ extension ResponderViewController: SessionViewControllerDelegate {
print("[RESPONDER] Approving session...")
let proposal = currentProposal!
currentProposal = nil
let accounts = proposal.permissions.blockchains.map {$0+":\(account)"}
client.approve(proposal: proposal, accounts: Set(accounts))
let accounts = Set(proposal.permissions.blockchains.compactMap { Account($0+":\(account)") })
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 add another convenience init to avoid passing the : , It looks easy to forget

Copy link
Author

Choose a reason for hiding this comment

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

I added two more init to make the type more flexible.

client.approve(proposal: proposal, accounts: accounts)
}

func didRejectSession() {
Expand Down Expand Up @@ -196,7 +196,7 @@ extension ResponderViewController: WalletConnectClientDelegate {

}

func didUpdate(sessionTopic: String, accounts: Set<String>) {
func didUpdate(sessionTopic: String, accounts: Set<Account>) {

}

Expand Down
75 changes: 75 additions & 0 deletions Sources/WalletConnect/Account.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
A value that identifies an account in any given blockchain.

This structure parses account IDs according to [CAIP-10].
Account IDs are prefixed with a [CAIP-2] blockchain ID, delimited by a `':'` character, followed by the account address.

Specifying a blockchain account by using a chain-agnostic identifier is useful to allow interoperability between multiple
chains when using both wallets and decentralized applications.

[CAIP-2]:https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-2.md
[CAIP-10]:https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-10.md
*/
public struct Account: Equatable, Hashable {

/// A blockchain namespace. Usually describes an ecosystem or standard.
public var namespace: String
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use let for stored properties? immutable is better

Copy link
Author

Choose a reason for hiding this comment

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

Just changed to let


/// A reference string that identifies a blockchain within a given namespace.
public var reference: String

/// The account's address specific to the blockchain.
public var address: String

/// The CAIP-2 blockchain identifier of the account.
public var blockchainIdentifier: String {
"\(namespace):\(reference)"
}

/// The CAIP-10 account identifier absolute string.
public var absoluteString: String {
"\(namespace):\(reference):\(address)"
}

/// Returns whether the account conforms to CAIP-10.
public var isCAIP10Conformant: Bool {
String.conformsToCAIP10(absoluteString)
}

/**
Creates an account instance from the provided string.

This initializer returns nil if the string doesn't represent a valid account id in conformance with
[CAIP-10](https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-10.md).
*/
public init?(_ string: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would make sense to create a Chain and add init from a chain and an address.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, but IMHO only if we use the Chain type in more places in the engine. Sounds like an overkill if we add it just to use in an Account

guard String.conformsToCAIP10(string) else { return nil }
let splits = string.split(separator: ":")
self.namespace = String(splits[0])
self.reference = String(splits[1])
self.address = String(splits[2])
}
}

extension Account: LosslessStringConvertible {
public var description: String {
return absoluteString
}
}

extension Account: Codable {

public init(from decoder: Decoder) throws {
let container = try decoder.singleValueContainer()
let absoluteString = try container.decode(String.self)
guard let account = Account(absoluteString) else {
throw DecodingError.dataCorruptedError(in: container, debugDescription: "Malformed CAIP-10 account identifier.")
}
self = account
}

public func encode(to encoder: Encoder) throws {
var container = encoder.singleValueContainer()
try container.encode(absoluteString)
}
}
14 changes: 5 additions & 9 deletions Sources/WalletConnect/WalletConnectClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ public final class WalletConnectClient {
/// - Parameters:
/// - proposal: Session Proposal received from peer client in a WalletConnect delegate function: `didReceive(sessionProposal: Session.Proposal)`
/// - accounts: A Set of accounts that the dapp will be allowed to request methods executions on.
public func approve(proposal: Session.Proposal, accounts: Set<String>) {
sessionEngine.approve(proposal: proposal.proposal, accounts: accounts)
public func approve(proposal: Session.Proposal, accounts: Set<Account>) {
sessionEngine.approve(proposal: proposal.proposal, accounts: Set(accounts.map { $0.absoluteString }))
}

/// For the responder to reject a session proposal.
Expand All @@ -153,12 +153,8 @@ public final class WalletConnectClient {
/// - Parameters:
/// - topic: Topic of the session that is intended to be updated.
/// - accounts: Set of accounts that will be allowed to be used by the session after the update.
public func update(topic: String, accounts: Set<String>) {
do {
try sessionEngine.update(topic: topic, accounts: accounts)
} catch {
print("Error on session update call: \(error)")
}
public func update(topic: String, accounts: Set<Account>) throws {
try sessionEngine.update(topic: topic, accounts: Set(accounts.map { $0.absoluteString }))
}

/// For the responder to upgrade session permissions
Expand Down Expand Up @@ -305,7 +301,7 @@ public final class WalletConnectClient {
delegate?.didUpgrade(sessionTopic: topic, permissions: upgradedPermissions)
}
sessionEngine.onSessionUpdate = { [unowned self] topic, accounts in
delegate?.didUpdate(sessionTopic: topic, accounts: accounts)
delegate?.didUpdate(sessionTopic: topic, accounts: Set(accounts.compactMap { Account($0) }))
}
sessionEngine.onNotificationReceived = { [unowned self] topic, notification in
delegate?.didReceive(notification: notification, sessionTopic: topic)
Expand Down
2 changes: 1 addition & 1 deletion Sources/WalletConnect/WalletConnectClientDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public protocol WalletConnectClientDelegate: AnyObject {
/// Tells the delegate that extra accounts has been included in session sequence
///
/// Function is executed on controller and non-controller client when both communicating peers have successfully included new accounts requested by the controller client.
func didUpdate(sessionTopic: String, accounts: Set<String>)
func didUpdate(sessionTopic: String, accounts: Set<Account>)

/// Tells the delegate that the client has settled a session.
///
Expand Down
4 changes: 2 additions & 2 deletions Tests/IntegrationTests/ClientDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ClientDelegate: WalletConnectClientDelegate {
var onSessionRejected: ((String, Reason)->())?
var onSessionDelete: (()->())?
var onSessionUpgrade: ((String, Session.Permissions)->())?
var onSessionUpdate: ((String, Set<String>)->())?
var onSessionUpdate: ((String, Set<Account>)->())?
var onNotificationReceived: ((Session.Notification, String)->())?
var onPairingUpdate: ((String, AppMetadata)->())?

Expand Down Expand Up @@ -42,7 +42,7 @@ class ClientDelegate: WalletConnectClientDelegate {
func didUpgrade(sessionTopic: String, permissions: Session.Permissions) {
onSessionUpgrade?(sessionTopic, permissions)
}
func didUpdate(sessionTopic: String, accounts: Set<String>) {
func didUpdate(sessionTopic: String, accounts: Set<Account>) {
onSessionUpdate?(sessionTopic, accounts)
}
func didReceive(notification: Session.Notification, sessionTopic: String) {
Expand Down
10 changes: 5 additions & 5 deletions Tests/IntegrationTests/ClientTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ final class ClientTests: XCTestCase {
func testNewSession() {
let proposerSettlesSessionExpectation = expectation(description: "Proposer settles session")
let responderSettlesSessionExpectation = expectation(description: "Responder settles session")
let account = "0x022c0c42a80bd19EA4cF0F94c4F9F96645759716"
let account = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!

let permissions = Session.Permissions.stub()
let uri = try! proposer.client.connect(sessionPermissions: permissions)!
Expand Down Expand Up @@ -244,7 +244,7 @@ final class ClientTests: XCTestCase {
func testSuccessfulSessionUpgrade() {
let proposerSessionUpgradeExpectation = expectation(description: "Proposer upgrades session on responder request")
let responderSessionUpgradeExpectation = expectation(description: "Responder upgrades session on proposer response")
let account = "0x022c0c42a80bd19EA4cF0F94c4F9F96645759716"
let account = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
let permissions = Session.Permissions.stub()
let upgradePermissions = Session.Permissions(blockchains: ["eip155:42"], methods: ["eth_sendTransaction"])
let uri = try! proposer.client.connect(sessionPermissions: permissions)!
Expand All @@ -271,16 +271,16 @@ final class ClientTests: XCTestCase {
func testSuccessfulSessionUpdate() {
let proposerSessionUpdateExpectation = expectation(description: "Proposer updates session on responder request")
let responderSessionUpdateExpectation = expectation(description: "Responder updates session on proposer response")
let account = "eip155:42:0x022c0c42a80bd19EA4cF0F94c4F9F96645759716"
let updateAccounts: Set<String> = ["eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb"]
let account = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
let updateAccounts: Set<Account> = [Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdf")!]
let permissions = Session.Permissions.stub()
let uri = try! proposer.client.connect(sessionPermissions: permissions)!
try! responder.client.pair(uri: uri)
responder.onSessionProposal = { [unowned self] proposal in
self.responder.client.approve(proposal: proposal, accounts: [account])
}
responder.onSessionSettled = { [unowned self] sessionSettled in
responder.client.update(topic: sessionSettled.topic, accounts: updateAccounts)
try? responder.client.update(topic: sessionSettled.topic, accounts: updateAccounts)
}
responder.onSessionUpdate = { _, accounts in
XCTAssertEqual(accounts, updateAccounts)
Expand Down
34 changes: 34 additions & 0 deletions Tests/WalletConnectTests/AccountTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import XCTest
@testable import WalletConnect

final class AccountTests: XCTestCase {

func testInit() {
// Valid accounts
XCTAssertNotNil(Account("std:0:0"))
XCTAssertNotNil(Account("chainstd:8c3444cf8970a9e41a706fab93e7a6c4:6d9b0b4b9994e8a6afbd3dc3ed983cd51c755afb27cd1dc7825ef59c134a39f7"))

// Invalid accounts
XCTAssertNil(Account("std:0:$"))
XCTAssertNil(Account("std:$:0"))
XCTAssertNil(Account("st:0:0"))
}

func testBlockchainIdentifier() {
let account = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
XCTAssertEqual(account.blockchainIdentifier, "eip155:1")
}

func testAbsoluteString() {
let accountString = "eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb"
let account = Account(accountString)!
XCTAssertEqual(account.absoluteString, accountString)
}

func testCodable() throws {
let account = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
let encoded = try JSONEncoder().encode(account)
let decoded = try JSONDecoder().decode(Account.self, from: encoded)
XCTAssertEqual(account, decoded)
}
}