ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
616 stars 93 forks source link

(ios) Background receive bug #531

Closed robbiehanson closed 3 months ago

robbiehanson commented 3 months ago

(ios) Backgroud receive bug

@dpad85 recently discovered a bug that affect's Phoenix's ability to receive payments via the background process (on iOS).

Steps to reproduce:

  1. Using iOS 15 or 16 (not 17+)
  2. Open the Phoenix app, create an invoice, and then force-quit the app
  3. Pay the invoice with another wallet
  4. Note that the payment succeeds
  5. Open the Phoenix app again, create another invoice, and then force-quit the app
  6. Attempt to pay the invoice with another wallet
  7. Note that this time the payment fails immediately, and a Phoenix notification appears ("Missed incoming payment")

Analysis

On iOS we are forced to use a notification-service-extension to handle incoming payments when the Phoenix app is in the background, or not running. (See #280 for details.) Since this is a completely separate process, there are various edge cases we have to protect against.

One of the edge cases is that the server only allows a single client to be connected at-a-time (for the same wallet). Which means:

As part of the architecture to implement these safety measures, we have a simple IPC channel where each process can send flags to the other (such as ping, pong, or offline).

The simple fix

On iOS 17, when you force-quit the app, the SceneDelegate.sceneDidEnterBackground() method is called. However, this does NOT occur on iOS 15 or 16. And because of this, the offline flag was not sent via the IPC channel. So what was happening:

So the simple fix is that the main Phoenix app should send the offline_from_mainApp when force-quit. I've implemented this change in the PR.

The harder fix

Even with the simple fix, there's still the edge-case where the main Phoenix app crashes. In which case it won't send the offline_from_mainApp message. So the background app still needs to be smarter about how it handles IPC messages. Especially since iOS may keep the background process open.

So now, when the background process isn't actively processing a notification, it closes the IPC channel. And everytime it opens a new IPC channel, it always ignores any existing message on the channel.

This change has also been implemented in the PR.