Skip to content

Commit

Permalink
GH-3262 rewrote approval function (#3392)
Browse files Browse the repository at this point in the history
Changes proposed in this pull request:
- Interaction with the wc library extracted to a `ApproverImpl` class behind a testable interface
- Approving logic handles `requiredNamespaces` and `optionalNamespaces` correctly
- Added template-based tests for the approving function
  • Loading branch information
DmitryBespalov authored Feb 2, 2024
1 parent 22dade7 commit 994b3ac
Show file tree
Hide file tree
Showing 17 changed files with 1,281 additions and 45 deletions.
8 changes: 8 additions & 0 deletions Multisig.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,8 @@
B367AED22B0CB89600F06B86 /* TransactionValidationTestCase1.json in Resources */ = {isa = PBXBuildFile; fileRef = B367AED12B0CB89600F06B86 /* TransactionValidationTestCase1.json */; };
B3710B3D2AD584BE002E503B /* SecureConfig in Frameworks */ = {isa = PBXBuildFile; productRef = B3710B3C2AD584BE002E503B /* SecureConfig */; };
B3710B442AD5AAD7002E503B /* config.bundle in Resources */ = {isa = PBXBuildFile; fileRef = B3710B432AD5AAD7002E503B /* config.bundle */; };
B37C73AB2B6D0ADC00E85E7F /* WalletConnectManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B37C73AA2B6D0ADC00E85E7F /* WalletConnectManagerTests.swift */; };
B37C73AF2B6D2FC100E85E7F /* WalletConnectManagerTestData.bundle in Resources */ = {isa = PBXBuildFile; fileRef = B37C73AE2B6D2FC100E85E7F /* WalletConnectManagerTestData.bundle */; };
B380756B2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756A2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json */; };
B380756D2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756C2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json */; };
B380756F2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756E2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json */; };
Expand Down Expand Up @@ -2096,6 +2098,8 @@
B3710B432AD5AAD7002E503B /* config.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = config.bundle; sourceTree = "<group>"; };
B3710B452AD5AE9D002E503B /* apis-prod.example.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "apis-prod.example.json"; sourceTree = "<group>"; };
B3710B462AD5AE9D002E503B /* apis-staging.example.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "apis-staging.example.json"; sourceTree = "<group>"; };
B37C73AA2B6D0ADC00E85E7F /* WalletConnectManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WalletConnectManagerTests.swift; sourceTree = "<group>"; };
B37C73AE2B6D2FC100E85E7F /* WalletConnectManagerTestData.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = WalletConnectManagerTestData.bundle; sourceTree = "<group>"; };
B380756A2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = TransactionValidationTestCase_InvalidSafeTxHash.json; sourceTree = "<group>"; };
B380756C2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_NoConfirmations.json; sourceTree = "<group>"; };
B380756E2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_ConfirmationsNotFromOwners.json; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2779,6 +2783,8 @@
0A9366BC279090CC001918A9 /* OwnerKeySelectionPolicyTests.swift */,
0AADCDBF2858CE2600A0139B /* AddOwnerRequestValidatorTests.swift */,
B35BFD0E2A937DC000A9FB15 /* NavigationRouterTests.swift */,
B37C73AA2B6D0ADC00E85E7F /* WalletConnectManagerTests.swift */,
B37C73AE2B6D2FC100E85E7F /* WalletConnectManagerTestData.bundle */,
);
path = Logic;
sourceTree = "<group>";
Expand Down Expand Up @@ -5223,6 +5229,7 @@
isa = PBXResourcesBuildPhase;
buildActionMask = 2147483647;
files = (
B37C73AF2B6D2FC100E85E7F /* WalletConnectManagerTestData.bundle in Resources */,
0A19D43B249A5BA500A316B6 /* TransferTransaction.json in Resources */,
AD30377B2E867EBF8647725B /* DelegateWarningMultiSend.json in Resources */,
AD303C6514DB164B8DE3BC73 /* DelegateWarningTopLevel.json in Resources */,
Expand Down Expand Up @@ -5981,6 +5988,7 @@
0A9BC3482460539F00EB9C5D /* ContractTests.swift in Sources */,
555312F2250673AD0008206B /* BIP39Tests.swift in Sources */,
0AADCDC02858CE2600A0139B /* AddOwnerRequestValidatorTests.swift in Sources */,
B37C73AB2B6D0ADC00E85E7F /* WalletConnectManagerTests.swift in Sources */,
0AF78A3027E0AAD3001BAFE0 /* WCAppRegistryEntryTests.swift in Sources */,
D86A68DF274E692500D7F5C5 /* TestingAppDelegate.swift in Sources */,
5532D4CA2449A1980067505A /* CrashlyticsLoggerTests.swift in Sources */,
Expand Down
159 changes: 114 additions & 45 deletions Multisig/UI/WalletConnectV2/WalletConnectManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ class WalletConnectManager {
icons: ["https://app.safe.global/favicons/mstile-150x150.png",
"https://app.safe.global/favicons/logo_120x120.png"],
redirect: AppMetadata.Redirect(native: "", universal: "https://app.safe.global/"))


// Testable interface for approving a session
var approver: Approver = ApproverImpl()

private init() { }

func config() {
Expand Down Expand Up @@ -162,59 +165,131 @@ class WalletConnectManager {
}
}
}

// Approves a single chain where selected safe resides.

// Approves required chains and a chain where selected safe resides.
//
// For reference:
// - https://chainagnostic.org/CAIPs/caip-10
// - https://chainagnostic.org/CAIPs/caip-2
//
// Requires:
// - proposal with the chain that is same as the currently selected safe
// - session proposal
// Guarantees:
// - connection approved
// - connection approved if safe's chain is in the proposal
// - if no such chain found in proposal, connection will fail
func approveSession(proposal: Session.Proposal) {
Task { @MainActor in
guard let safe = try? Safe.getSelected() else { return }

// Step 1: find a compatible namespace with safe's chain
var resultNamespaces = [String: SessionNamespace]()
let NAMESPACE_ID = "eip155"

guard
let address = approver.safe?.address,
let chainId = approver.safe?.chain,
let blockchain = Blockchain(namespace: NAMESPACE_ID, reference: chainId)
else {
approver.reject(.preconditionsNotSatisfied(id: proposal.id))
return
}

var chains = [Blockchain]()
var methods = [String]()
var events = [String]()

// approved proposal must contain all of the required namespaces
if let ns = proposal.requiredNamespaces[NAMESPACE_ID] {
chains.append(contentsOf: ns.chains ?? [])
methods.append(contentsOf: ns.methods)
events.append(contentsOf: ns.events)
}

// depending on the dApp, the chain we're looking for will be in the optional namespaces
if let ns = proposal.optionalNamespaces?[NAMESPACE_ID], ns.chains?.contains(blockchain) == true {
chains.append(blockchain)
methods.append(contentsOf: ns.methods)
events.append(contentsOf: ns.events)
}

guard chains.contains(blockchain) else {
approver.reject(.chainNotFound(id: proposal.id))
return
}

// WalletConnect requires to support accounts at least for all required chains
let accounts = chains.compactMap { Account(blockchain: $0, address: address) }

let creme = SessionNamespace(
chains: Set(chains),
accounts: Set(accounts),
methods: Set(methods),
events: Set(events)
)

let treat: [String: SessionNamespace] = [
NAMESPACE_ID: creme
]

let EVM_COMPATIBLE_NETWORK = "eip155"

for (caip2Namespace, namespace) in proposal.allNamespaces {
guard
caip2Namespace == EVM_COMPATIBLE_NETWORK,
let chains = namespace.chains,
let validChain = chains.first(where: { $0.reference == safe.chain?.id }),
let account = Account(validChain.absoluteString + ":\(safe.addressValue)")
else { continue }

// Step 2: share the account address
resultNamespaces[caip2Namespace] = SessionNamespace(
accounts: Set([account]),
methods: namespace.methods,
events: namespace.events)

break
approver.approve(proposalId: proposal.id, namespaces: treat)
}

class Approver {
enum ApprovalFailure: Equatable {
case preconditionsNotSatisfied(id: String)
case chainNotFound(id: String)
}

struct Account {
var address: String
var chain: String
}

var safe: Account? {
nil
}

func reject(_ failure: ApprovalFailure) {
}

func approve(proposalId: String, namespaces: [String: SessionNamespace]) {
}
}

class ApproverImpl: Approver {

override var safe: Account? {
guard let safe = try? Safe.getSelected(),
let address = safe.address,
let chain = safe.chain,
let chainId = chain.id
else {
return nil
}

// Step 1.2: fail if no valid chains found
let hasFoundNamespace = !resultNamespaces.isEmpty
guard hasFoundNamespace else {
Task { @MainActor in
return Account(address: address, chain: chainId)
}

override func reject(_ failure: ApprovalFailure) {
Task { @MainActor in
switch failure {
case .preconditionsNotSatisfied(let id):
App.shared.snackbar.show(error: GSError.WC2SessionApprovalFailed())
try? await Web3Wallet.instance.reject(proposalId: id, reason: .userRejected)

case .chainNotFound(let id):
App.shared.snackbar.show(error: GSError.WC2SessionApprovalFailedWrongChain())
try? await Web3Wallet.instance.reject(proposalId: id, reason: .userRejectedChains)
}
return
}

// Step 3: continue with connection
do {
try await Web3Wallet.instance.approve(proposalId: proposal.id, namespaces: resultNamespaces)
} catch {
Task { @MainActor in
}

override func approve(proposalId: String, namespaces: [String: SessionNamespace]) {
Task { @MainActor in
do {
try await Web3Wallet.instance.approve(proposalId: proposalId, namespaces: namespaces)
} catch {
LogService.shared.error("Approval failed: \(error)")
App.shared.snackbar.show(error: GSError.WC2SessionApprovalFailed())
}
}
}
}


/// By default, session lifetime is set for 7 days and after that time user's session will expire.
/// This method will extend the session for 7 days
Expand Down Expand Up @@ -357,9 +432,3 @@ extension Bundle {
return object(forInfoDictionaryKey: "CFBundleDisplayName") as? String ?? "Safe Multisig"
}
}

extension Session.Proposal {
var allNamespaces: [String: ProposalNamespace] {
requiredNamespaces.merging(optionalNamespaces ?? [:], uniquingKeysWith: { (req, _) in req })
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"proposer": {
"description": "Explore DeFi liquidity pools or create your own. Provide liquidity to accumulate yield from swap fees while retaining your token exposure as prices move",
"url": "https://app.balancer.fi",
"name": "Balancer",
"icons": [
"https://app.balancer.fi/favicon.ico"
]
},
"requiredNamespaces": {
},
"optionalNamespaces": {
},
"pairingTopic": "45bfa73feb1dcd24142a6603a735d2f47463e3662cdacc99c55213a5f7b79794",
"proposal": {
"requiredNamespaces": {
},
"relays": [
{
"protocol": "irn"
}
],
"proposer": {
"metadata": {
"url": "https://app.balancer.fi",
"description": "Explore DeFi liquidity pools or create your own. Provide liquidity to accumulate yield from swap fees while retaining your token exposure as prices move",
"name": "Balancer",
"icons": [
"https://app.balancer.fi/favicon.ico"
]
},
"publicKey": "49dda9830d2e7139d8580c44d4a10be9e209c38442f542e5ca176d34867a5279"
},
"optionalNamespaces": {
}
},
"id": "49dda9830d2e7139d8580c44d4a10be9e209c38442f542e5ca176d34867a5279"
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{
"proposer": {
"description": "Explore DeFi liquidity pools or create your own. Provide liquidity to accumulate yield from swap fees while retaining your token exposure as prices move",
"url": "https://app.balancer.fi",
"name": "Balancer",
"icons": [
"https://app.balancer.fi/favicon.ico"
]
},
"requiredNamespaces": {
},
"optionalNamespaces": {
"eip155": {
"chains": [
"eip155:2"
],
"methods": [
"personal_sign",
"eth_sendTransaction"
],
"events": [
"chainChanged",
"accountsChanged"
]
}
},
"pairingTopic": "45bfa73feb1dcd24142a6603a735d2f47463e3662cdacc99c55213a5f7b79794",
"proposal": {
"requiredNamespaces": {
},
"relays": [
{
"protocol": "irn"
}
],
"proposer": {
"metadata": {
"url": "https://app.balancer.fi",
"description": "Explore DeFi liquidity pools or create your own. Provide liquidity to accumulate yield from swap fees while retaining your token exposure as prices move",
"name": "Balancer",
"icons": [
"https://app.balancer.fi/favicon.ico"
]
},
"publicKey": "49dda9830d2e7139d8580c44d4a10be9e209c38442f542e5ca176d34867a5279"
},
"optionalNamespaces": {
"eip155": {
"methods": [
"personal_sign",
"eth_sendTransaction"
],
"events": [
"chainChanged",
"accountsChanged"
],
"chains": [
"eip155:2"
]
}
}
},
"id": "49dda9830d2e7139d8580c44d4a10be9e209c38442f542e5ca176d34867a5279"
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"eip155": {
"chains": [
"eip155:42161"
],
"accounts": [
"eip155:42161:0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE"
],
"methods": [
"eth_accounts",
"wallet_requestPermissions",
"wallet_getPermissions",
"personal_sign",
"eth_sign",
"wallet_scanQRCode",
"wallet_addEthereumChain",
"eth_signTypedData_v3",
"eth_signTypedData_v4",
"eth_sendRawTransaction",
"eth_sendTransaction",
"wallet_registerOnboarding",
"eth_signTypedData",
"eth_requestAccounts",
"eth_signTransaction",
"wallet_watchAsset",
"wallet_switchEthereumChain"
],
"events": [
"connect",
"message",
"chainChanged",
"accountsChanged",
"disconnect"
]
}
}

Loading

0 comments on commit 994b3ac

Please sign in to comment.