Apply multi-hop settings into tunnel

[mojganii]

This PR enables multi-hop tunneling support in Swift.

---

This change is 

[rablador]

Reviewed 18 of 18 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @mojganii)

---

ios/MullvadREST/Relay/RelaySelectorWrapper.swift line 56 at r1 (raw file):

    private func addObserver() {
        self.observer = MultihopObserverBlock(didUpdateMultihop: { [weak self] _, multihopState in
            // Discard applying multi-hop settings until it's under development

We already have a debug check for the settings switch, right? Should be enough.

---

ios/MullvadREST/Transport/Shadowsocks/ShadowsocksLoader.swift line 56 at r1 (raw file):

        // The multihop state gets updated a lot when observing the tunnel, clear the cache if the multihop settings have changed.
        self.observer = MultihopObserverBlock(didUpdateMultihop: { [weak self] _, newMultihopState in
            // Discard applying multi-hop settings until it's under development

We already have a debug check for the settings switch, right? Should be enough.

---

ios/MullvadVPN/TunnelManager/TunnelState+UI.swift line 187 at r1 (raw file):

                        "TUNNEL_STATE_PQ_CONNECTED_ACCESSIBILITY_LABEL",
                        tableName: "Main",
                        value: "Quantum secure connection. Connected to %@, %@",

Aren't we changing these too?

---

ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorWrapperTests.swift line 37 at r1 (raw file):

            multihopUpdater: multihopUpdater
        )
        multihopStateListener.onNewMultihop?(.off)

Duplicate

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

            newSettings: updatedSettings
        ))
    }

Perhaps we should add a test case where there should be no reconnect as well.

[mojganii]

Reviewable status: 0 of 278 files reviewed, 5 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPN/TunnelManager/TunnelState+UI.swift line 187 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Aren't we changing these too?

As I've seen in desktop app, this always shows exit country and city.

---

ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorWrapperTests.swift line 37 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Duplicate

Done.

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Perhaps we should add a test case where there should be no reconnect as well.

The test covers the scenario where any changes in settings occur. Based on the TunnelSettingsStrategy, it decides whether to reconnect to new relays or not. For example, if the change is related to relayConstraints or multihop, it triggers a reconnect to the new relays. However, if the change happens to obfuscater, it retains the current relays. The test covers all properties.

---

ios/MullvadREST/Relay/RelaySelectorWrapper.swift line 56 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

We already have a debug check for the settings switch, right? Should be enough.

now we are importing the commit which contains go side.I remove that to make the test easier.

---

ios/MullvadREST/Transport/Shadowsocks/ShadowsocksLoader.swift line 56 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

We already have a debug check for the settings switch, right? Should be enough.

Removed

[rablador]

Reviewed 33 of 53 files at r3, 245 of 245 files at r4, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @mojganii)

---

ios/build-wireguard-go.sh line 15 at r4 (raw file):

ACTION=$1

export PATH="/opt/homebrew/opt/[email protected]/bin:$PATH"

This should not be added in this PR I think.

---

ios/MullvadVPN/TunnelManager/Tunnel+Settings.swift line 26 at r4 (raw file):

        switch (oldSettings, newSettings) {
        case let (old, new) where old.relayConstraints != new.relayConstraints:
            logger?.debug("Relay address changed from \(old.relayConstraints) to \(new.relayConstraints)")

Do we need these logs? They're good when developing to see that it works, but we also have tests for this.

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 489 at r4 (raw file):

		7A3353972AAA0F8600F0A71C /* OperationBlockObserverSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3353962AAA0F8600F0A71C /* OperationBlockObserverSupport.swift */; };
		7A3AD5012C1068A800E9AD90 /* RelayPicking.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3AD5002C1068A800E9AD90 /* RelayPicking.swift */; };
		7A3EFAAB2BDFDAE800318736 /* (null) in Sources */ = {isa = PBXBuildFile; };

These nulls seem weird.

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 5936 at r4 (raw file):

				58B26E242943520C00D5980C /* NotificationProviderProtocol.swift in Sources */,
				5877F94E2A0A59AA0052D9E9 /* NotificationResponse.swift in Sources */,
				7AE2414A2C20682B0076CE33 /* (null) in Sources */,

null

---

ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved line 2 at r4 (raw file):

{
  "originHash" : "c15149b2d59d9e9c72375f65339c04f41a19943e1117e682df27fc9f943fdc56",

This file should not change.

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

Previously, mojganii wrote…

The test covers the scenario where any changes in settings occur. Based on the TunnelSettingsStrategy, it decides whether to reconnect to new relays or not. For example, if the change is related to relayConstraints or multihop, it triggers a reconnect to the new relays. However, if the change happens to obfuscater, it retains the current relays. The test covers all properties.

Yes, I was only thinking that an inverted test could be good as well. To make sure that the strategy does not trigger a reconnect if nothing changes.

[mojganii]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @rablador)

---

ios/build-wireguard-go.sh line 15 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

This should not be added in this PR I think.

the app doesn't get built without this line.

---

ios/MullvadVPN/TunnelManager/Tunnel+Settings.swift line 26 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

Do we need these logs? They're good when developing to see that it works, but we also have tests for this.

I doubt wether it's needed or not.but it gives information to us when reconnecting is using the new relays or not.

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 489 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

These nulls seem weird.

I've not seen that! Xcode made it. good catch!

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 5936 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

null

Done.

---

ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved line 2 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

This file should not change.

It shouldn't have been updated.

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Yes, I was only thinking that an inverted test could be good as well. To make sure that the strategy does not trigger a reconnect if nothing changes.

When settings changes occur, reconnection happens regardless of whether oldSettings and newSettings are the same. If we decide to reconnect only when the old and new settings are different, then we need to cover the test case you mentioned. However, the current behavior of the app is to reconnect whenever settings are updated, even if the values are repetitive.

[mojganii]

Reviewable status: 276 of 278 files reviewed, 6 unresolved discussions (waiting on @rablador)

---

ios/build-wireguard-go.sh line 15 at r4 (raw file):

Previously, mojganii wrote…

the app doesn't get built without this line.

let's remove that when main was updated.

[rablador]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mojganii)

---

ios/build-wireguard-go.sh line 15 at r4 (raw file):

Previously, mojganii wrote…

let's remove that when main was updated.

Are you sure? Main doesn't need go 1.20 now. I had problems on main earlier today, but that was because I had caches from when we tried to get the new wireguardkit updates to work. Cleaning derived data fixed it.

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

Previously, mojganii wrote…

When settings changes occur, reconnection happens regardless of whether oldSettings and newSettings are the same. If we decide to reconnect only when the old and new settings are different, then we need to cover the test case you mentioned. However, the current behavior of the app is to reconnect whenever settings are updated, even if the values are repetitive.

The reconnection happens, but not selecting a new relay, every time, right? Isn't that what the tunnel strategy tries to determine? Let's say there's something wrong in the code in shouldReconnectToNewRelay, then we could end up with selecting a new relay every time even though neither multihop nor relay constraints have changed.

[mojganii]

Reviewable status: 274 of 278 files reviewed, 1 unresolved discussion (waiting on @rablador)

---

ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelSettingsStrategyTests.swift line 89 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

The reconnection happens, but not selecting a new relay, every time, right? Isn't that what the tunnel strategy tries to determine? Let's say there's something wrong in the code in shouldReconnectToNewRelay, then we could end up with selecting a new relay every time even though neither multihop nor relay constraints have changed.

Ok

---

ios/build-wireguard-go.sh line 15 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

Are you sure? Main doesn't need go 1.20 now. I had problems on main earlier today, but that was because I had caches from when we tried to get the new wireguardkit updates to work. Cleaning derived data fixed it.

Done.

[rablador]

Reviewed 4 of 4 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved