Skip to content

Consolidate retry attempts in a single location #6474

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

Merged
merged 1 commit into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions ios/MullvadMockData/MullvadREST/RelaySelectorStub.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ extension RelaySelectorStub {
cityCode: "got",
latitude: 0,
longitude: 0
), retryAttempts: 0
)
)

return SelectedRelays(
entry: cityRelay,
exit: cityRelay
exit: cityRelay,
retryAttempt: 0
)
}
}
Expand Down
8 changes: 4 additions & 4 deletions ios/MullvadREST/Relay/MultihopDecisionFlow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct OneToOne: MultihopDecisionFlow {

let entryMatch = try relayPicker.findBestMatch(from: entryCandidates)
let exitMatch = try relayPicker.findBestMatch(from: exitCandidates)
return SelectedRelays(entry: entryMatch, exit: exitMatch)
return SelectedRelays(entry: entryMatch, exit: exitMatch, retryAttempt: relayPicker.connectionAttemptCount)
}

func canHandle(entryCandidates: [RelayCandidate], exitCandidates: [RelayCandidate]) -> Bool {
Expand Down Expand Up @@ -70,11 +70,11 @@ struct OneToMany: MultihopDecisionFlow {
case let (1, count) where count > 1:
let entryMatch = try multihopPicker.findBestMatch(from: entryCandidates)
let exitMatch = try multihopPicker.exclude(relay: entryMatch, from: exitCandidates)
return SelectedRelays(entry: entryMatch, exit: exitMatch)
return SelectedRelays(entry: entryMatch, exit: exitMatch, retryAttempt: relayPicker.connectionAttemptCount)
default:
let exitMatch = try multihopPicker.findBestMatch(from: exitCandidates)
let entryMatch = try multihopPicker.exclude(relay: exitMatch, from: entryCandidates)
return SelectedRelays(entry: entryMatch, exit: exitMatch)
return SelectedRelays(entry: entryMatch, exit: exitMatch, retryAttempt: relayPicker.connectionAttemptCount)
}
}

Expand Down Expand Up @@ -107,7 +107,7 @@ struct ManyToMany: MultihopDecisionFlow {

let exitMatch = try multihopPicker.findBestMatch(from: exitCandidates)
let entryMatch = try multihopPicker.exclude(relay: exitMatch, from: entryCandidates)
return SelectedRelays(entry: entryMatch, exit: exitMatch)
return SelectedRelays(entry: entryMatch, exit: exitMatch, retryAttempt: relayPicker.connectionAttemptCount)
}

func canHandle(entryCandidates: [RelayCandidate], exitCandidates: [RelayCandidate]) -> Bool {
Expand Down
5 changes: 2 additions & 3 deletions ios/MullvadREST/Relay/RelayPicking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ extension RelayPicking {
return SelectedRelay(
endpoint: match.endpoint,
hostname: match.relay.hostname,
location: match.location,
retryAttempts: connectionAttemptCount
location: match.location
)
}
}
Expand All @@ -50,7 +49,7 @@ struct SinglehopPicker: RelayPicking {

let match = try findBestMatch(from: candidates)

return SelectedRelays(entry: nil, exit: match)
return SelectedRelays(entry: nil, exit: match, retryAttempt: connectionAttemptCount)
}
}

Expand Down
10 changes: 4 additions & 6 deletions ios/MullvadREST/Relay/RelaySelectorProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,11 @@ public struct SelectedRelay: Equatable, Codable {
/// Relay geo location.
public let location: Location

/// Number of retried attempts to connect to a relay.
public let retryAttempts: UInt

/// Designated initializer.
public init(endpoint: MullvadEndpoint, hostname: String, location: Location, retryAttempts: UInt) {
public init(endpoint: MullvadEndpoint, hostname: String, location: Location) {
self.endpoint = endpoint
self.hostname = hostname
self.location = location
self.retryAttempts = retryAttempts
}
}

Expand All @@ -46,10 +42,12 @@ extension SelectedRelay: CustomDebugStringConvertible {
public struct SelectedRelays: Equatable, Codable {
public let entry: SelectedRelay?
public let exit: SelectedRelay
public let retryAttempt: UInt

public init(entry: SelectedRelay?, exit: SelectedRelay) {
public init(entry: SelectedRelay?, exit: SelectedRelay, retryAttempt: UInt) {
self.entry = entry
self.exit = exit
self.retryAttempt = retryAttempt
}
}

Expand Down
34 changes: 22 additions & 12 deletions ios/MullvadVPN.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions ios/MullvadVPN/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,19 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
ipOverrideRepository: ipOverrideRepository
)

let constraintsUpdater = RelayConstraintsUpdater()
let multihopListener = MultihopStateListener()
let multihopUpdater = MultihopUpdater(listener: multihopListener)

relayCacheTracker = RelayCacheTracker(
relayCache: ipOverrideWrapper,
application: application,
apiProxy: apiProxy
)

addressCacheTracker = AddressCacheTracker(application: application, apiProxy: apiProxy, store: addressCache)
tunnelStore = TunnelStore(application: application)

let constraintsUpdater = RelayConstraintsUpdater()
let multihopListener = MultihopStateListener()
let multihopUpdater = MultihopUpdater(listener: multihopListener)
tunnelStore = TunnelStore(application: application)

let relaySelector = RelaySelectorWrapper(
relayCache: ipOverrideWrapper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
networkReachability: .reachable,
connectionAttemptCount: 0,
transportLayer: .udp,
remotePort: selectedRelays.exit.endpoint.ipv4Relay.port, // TODO: Multihop
remotePort: selectedRelays.entry?.endpoint.ipv4Relay.port ?? selectedRelays.exit.endpoint.ipv4Relay
.port,
isPostQuantum: settings.tunnelQuantumResistance.isEnabled
)
)
Expand Down
26 changes: 26 additions & 0 deletions ios/MullvadVPN/TunnelManager/Tunnel+Settings.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// Tunnel+Settings.swift
// MullvadVPN
//
// Created by Mojgan on 2024-06-19.
// Copyright © 2024 Mullvad VPN AB. All rights reserved.
//

import MullvadLogging
import MullvadSettings

protocol TunnelSettingsStrategyProtocol {
func shouldReconnectToNewRelay(oldSettings: LatestTunnelSettings, newSettings: LatestTunnelSettings) -> Bool
}

struct TunnelSettingsStrategy: TunnelSettingsStrategyProtocol {
func shouldReconnectToNewRelay(oldSettings: LatestTunnelSettings, newSettings: LatestTunnelSettings) -> Bool {
switch (oldSettings, newSettings) {
case let (old, new) where old.relayConstraints != new.relayConstraints,
let (old, new) where old.tunnelMultihopState != new.tunnelMultihopState:
true
default:
false
}
}
}
40 changes: 20 additions & 20 deletions ios/MullvadVPN/TunnelManager/TunnelManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -939,16 +939,16 @@ final class TunnelManager: StorePaymentObserver {
let operation = AsyncBlockOperation(dispatchQueue: internalQueue) {
let currentSettings = self._tunnelSettings
var updatedSettings = self._tunnelSettings
let settingsStrategy = TunnelSettingsStrategy()

modificationBlock(&updatedSettings)

// Select new relay only when relay constraints change.
let currentConstraints = currentSettings.relayConstraints
let updatedConstraints = updatedSettings.relayConstraints
let selectNewRelay = currentConstraints != updatedConstraints

self.setSettings(updatedSettings, persist: true)
self.reconnectTunnel(selectNewRelay: selectNewRelay, completionHandler: nil)
self.reconnectTunnel(
selectNewRelay: settingsStrategy
.shouldReconnectToNewRelay(oldSettings: currentSettings, newSettings: updatedSettings),
completionHandler: nil
)
}

operation.completionBlock = {
Expand Down Expand Up @@ -1146,27 +1146,27 @@ extension TunnelManager {

```
func delay(seconds: UInt) async throws {
try await Task.sleep(nanoseconds: UInt64(seconds) * 1_000_000_000)
try await Task.sleep(nanoseconds: UInt64(seconds) * 1_000_000_000)
}

Task {
print("Wait 5 seconds")
try await delay(seconds: 5)
print("Wait 5 seconds")
try await delay(seconds: 5)

print("Simulate active account")
self.tunnelManager.simulateAccountExpiration(option: .active)
try await delay(seconds: 5)
print("Simulate active account")
self.tunnelManager.simulateAccountExpiration(option: .active)
try await delay(seconds: 5)

print("Simulate close to expiry")
self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry)
try await delay(seconds: 10)
print("Simulate close to expiry")
self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry)
try await delay(seconds: 10)

print("Simulate expired account")
self.tunnelManager.simulateAccountExpiration(option: .expired)
try await delay(seconds: 5)
print("Simulate expired account")
self.tunnelManager.simulateAccountExpiration(option: .expired)
try await delay(seconds: 5)

print("Simulate active account")
self.tunnelManager.simulateAccountExpiration(option: .active)
print("Simulate active account")
self.tunnelManager.simulateAccountExpiration(option: .active)
}
```

Expand Down
12 changes: 6 additions & 6 deletions ios/MullvadVPN/TunnelManager/TunnelState+UI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ extension TunnelState {
value: "Quantum secure connection. Connected to %@, %@",
comment: ""
),
tunnelInfo.exit.location.city, // TODO: Multihop
tunnelInfo.exit.location.country // TODO: Multihop
tunnelInfo.exit.location.city,
tunnelInfo.exit.location.country
)
} else {
String(
Expand All @@ -198,8 +198,8 @@ extension TunnelState {
value: "Secure connection. Connected to %@, %@",
comment: ""
),
tunnelInfo.exit.location.city, // TODO: Multihop
tunnelInfo.exit.location.country // TODO: Multihop
tunnelInfo.exit.location.city,
tunnelInfo.exit.location.country
)
}

Expand All @@ -219,8 +219,8 @@ extension TunnelState {
value: "Reconnecting to %@, %@",
comment: ""
),
tunnelInfo.exit.location.city, // TODO: Multihop
tunnelInfo.exit.location.country // TODO: Multihop
tunnelInfo.exit.location.city,
tunnelInfo.exit.location.country
)

case .waitingForConnectivity(.noConnection), .error:
Expand Down
23 changes: 19 additions & 4 deletions ios/MullvadVPN/TunnelManager/TunnelState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,39 @@ enum TunnelState: Equatable, CustomStringConvertible {
"pending reconnect after disconnect"
case let .connecting(tunnelRelays, isPostQuantum):
if let tunnelRelays {
"connecting \(isPostQuantum ? "(PQ) " : "")to \(tunnelRelays.exit.hostname)" // TODO: Multihop
"""
connecting \(isPostQuantum ? "(PQ) " : "")\
to \(tunnelRelays.exit.hostname)\
\(tunnelRelays.entry.flatMap { " via \($0.hostname)" } ?? "")
"""
} else {
"connecting\(isPostQuantum ? " (PQ)" : ""), fetching relay"
}
case let .connected(tunnelRelays, isPostQuantum):
"connected \(isPostQuantum ? "(PQ) " : "")to \(tunnelRelays.exit.hostname)" // TODO: Multihop
"""
connected \(isPostQuantum ? "(PQ) " : "")\
to \(tunnelRelays.exit.hostname)\
\(tunnelRelays.entry.flatMap { " via \($0.hostname)" } ?? "")
"""
case let .disconnecting(actionAfterDisconnect):
"disconnecting and then \(actionAfterDisconnect)"
case .disconnected:
"disconnected"
case let .reconnecting(tunnelRelays, isPostQuantum):
"reconnecting \(isPostQuantum ? "(PQ) " : "")to \(tunnelRelays.exit.hostname)" // TODO: Multihop
"""
reconnecting \(isPostQuantum ? "(PQ) " : "")\
to \(tunnelRelays.exit.hostname)\
\(tunnelRelays.entry.flatMap { " via \($0.hostname)" } ?? "")
"""
case .waitingForConnectivity:
"waiting for connectivity"
case let .error(blockedStateReason):
"error state: \(blockedStateReason)"
case let .negotiatingPostQuantumKey(tunnelRelays, _):
"negotiating key with \(tunnelRelays.exit.hostname)" // TODO: Multihop
"""
negotiating key with exit relay: \(tunnelRelays.exit.hostname)\
\(tunnelRelays.entry.flatMap { " via \($0.hostname)" } ?? "")
"""
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ final class TunnelControlView: UIView {
connectButtonBlurView.isEnabled = model.enableButtons
cityLabel.attributedText = attributedStringForLocation(string: model.city)
countryLabel.attributedText = attributedStringForLocation(string: model.country)
connectionPanel.connectedRelayName = model.connectedRelayName
connectionPanel.connectedRelayName = model.connectedRelaysName
connectionPanel.dataSource = model.connectionPanel

updateSecureLabel(tunnelState: tunnelState)
Expand Down Expand Up @@ -227,14 +227,15 @@ final class TunnelControlView: UIView {
private func updateTunnelRelays(tunnelRelays: SelectedRelays?) {
if let tunnelRelays {
cityLabel.attributedText = attributedStringForLocation(
string: tunnelRelays.exit.location.city // TODO: Multihop
string: tunnelRelays.exit.location.city
)
countryLabel.attributedText = attributedStringForLocation(
string: tunnelRelays.exit.location.country // TODO: Multihop
string: tunnelRelays.exit.location.country
)

connectionPanel.isHidden = false
connectionPanel.connectedRelayName = tunnelRelays.exit.hostname // TODO: Multihop
connectionPanel.connectedRelayName = tunnelRelays.exit
.hostname + "\(tunnelRelays.entry.flatMap { " via \($0.hostname)" } ?? "")"
} else {
countryLabel.attributedText = attributedStringForLocation(string: " ")
cityLabel.attributedText = attributedStringForLocation(string: " ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ struct TunnelControlViewModel {
let enableButtons: Bool
let city: String
let country: String
let connectedRelayName: String
let connectedRelaysName: String
let outgoingConnectionInfo: OutgoingConnectionInfo?

var connectionPanel: ConnectionPanelData? {
Expand All @@ -29,7 +29,7 @@ struct TunnelControlViewModel {
}

return ConnectionPanelData(
inAddress: "\(tunnelRelays.exit.endpoint.ipv4Relay.ip)\(portAndTransport)", // TODO: Multihop
inAddress: "\(tunnelRelays.entry?.endpoint.ipv4Relay.ip ?? tunnelRelays.exit.endpoint.ipv4Relay.ip)\(portAndTransport)",
outAddress: outgoingConnectionInfo?.outAddress
)
}
Expand All @@ -41,7 +41,7 @@ struct TunnelControlViewModel {
enableButtons: true,
city: "",
country: "",
connectedRelayName: "",
connectedRelaysName: "",
outgoingConnectionInfo: nil
)
}
Expand All @@ -53,7 +53,7 @@ struct TunnelControlViewModel {
enableButtons: enableButtons,
city: city,
country: country,
connectedRelayName: connectedRelayName,
connectedRelaysName: connectedRelaysName,
outgoingConnectionInfo: nil
)
}
Expand All @@ -65,7 +65,7 @@ struct TunnelControlViewModel {
enableButtons: enableButtons,
city: city,
country: country,
connectedRelayName: connectedRelayName,
connectedRelaysName: connectedRelaysName,
outgoingConnectionInfo: outgoingConnectionInfo
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ class TunnelViewController: UIViewController, RootContainment {
case let .connecting(tunnelRelays, _):
mapViewController.removeLocationMarker()
contentView.setAnimatingActivity(true)
mapViewController.setCenter(tunnelRelays?.exit.location.geoCoordinate, animated: animated) // TODO: Multihop
mapViewController.setCenter(tunnelRelays?.exit.location.geoCoordinate, animated: animated)

case let .reconnecting(tunnelRelays, _), let .negotiatingPostQuantumKey(tunnelRelays, _):
mapViewController.removeLocationMarker()
contentView.setAnimatingActivity(true)
mapViewController.setCenter(tunnelRelays.exit.location.geoCoordinate, animated: animated) // TODO: Multihop
mapViewController.setCenter(tunnelRelays.exit.location.geoCoordinate, animated: animated)

case let .connected(tunnelRelays, _):
let center = tunnelRelays.exit.location.geoCoordinate // TODO: Multihop
let center = tunnelRelays.exit.location.geoCoordinate
mapViewController.setCenter(center, animated: animated) {
self.contentView.setAnimatingActivity(false)

Expand Down
Loading
Loading