-
Notifications
You must be signed in to change notification settings - Fork 104
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
Remove Tor embedded library and use onion addresses #662
Conversation
The Tor proxy status UI has been removed. Phoenix now controls that the Electrum server is an onion address, or shows a message otherwise. The "enable tor" switch and the confirmation dialog have been updated. Some warning about background payments have been amended or removed since using a Tor VPN app should be more reliable. Translation have been removed and need to be fixed.
When tor is enabled, Phoenix will require electrum servers to use an onion address. This way, if the Tor VPN fails, connection will not be established. However, some users may want to opt-out, which is valid if they are connecting to their own server and don't mind connecting to it on clearnet.
555a7c9
to
08f7882
Compare
Also decreased space in release note component
this fixes an issue where peer connection could hang when the handshake timed out. The connectionLoop job would not detect the state change.
Note that (on Android at least) we need to let users access the Settings screen from the initialisation screen, so they can activate Tor/set Electrum before they actually create or restore their wallet. But we'll do that in another PR, since this one is blocking other important PRs. |
This breaks my heart. I maintain the kmp-tor library and would love to see this feature reintegrated into Phoenix. Orbot has a plethora of issues and is unreliable. |
What about Arti? It's official Tor reimplementation in Rust, which aims to be more modular, safe and so on. It's already in version 1.4.0 (but it still isn't 100% alternative of Tor C implementation though). That would be an interesting alternative. Most user's probably will not want to keep starting and shutting off Orbot (for example, if I'm using VPN, I can't use Orbot, because there can be only 1 VPN running on Android, so I need to keep switching between them ans that's annoying. With the current Tor implementation in Phoenix I don't need to do that). |
There are indeed other good options which could replace our library (although note that Arti is officially not ready for production). But embedding Tor directly in the app inherently has drawbacks. The issue is that Lightning is an interactive protocol. You need to respond to payment messages, and do so quickly. But when the app is in the background or stopped (which is often), the OS gives you a limited budget to process notifications in a timely manner (and they tend to shrink that budget in each new version). Having to first start Tor and connect before processing a payment is too much; it means that in most cases, background payments will fail. Delegating the Tor connection to Orbot not only removes "work" from the Phoenix app, it is also persistent and more reliable because it is treated as a VPN connection. |
Yeah I hear ya on the timeliness. It's completely understandable as it relates to the function of the app. Unfortunately, I do not, nor shall I ever, use Orbot as a VPN because you can only run 1 on mobile (among other reasons). What are your thoughts on adding the ability to define a SOCKS5 port for tor connectivity? Otherwise, Phoenix users MUST use a VPN app that forces traffic over tor (currently, only offered by 1 app, Orbot, AFAIK). EDIT: Also, regarding removal of "work" from Phoenix developers. Yes, tor functionality is pretty crazy to implement in a reliable, sound manner. Maintaining that is also trying. And this is exactly why I created |
We instantiated a new `SelectorManager` at each connection attempt, leaking resources even if each socket was properly cleaned up on disconnection. The leak affects both jvm and native. Also removes the `startTls` function that is not used anymore since we moved Tor outside phoenix (ACINQ/phoenix#662).
I totally agree with both @05nelsonm and @MasterixCZ . This is very not UX friendly. Users do not want to switch app to enable Orbot each time they want to process a payment. Yes, the previous implementation was long to start and to connect to tor but I worked (without any click of the user)! In my opinion, remove tor from phoenix is not going in the right direction for the privacy of user, because they will now just not enable tor on their wallet... I already have some complain about this from friend who use Phoenix often. |
First of all, thanks for developing Phoenix. It makes a difference 💟 I just went here because of removal of this feature. As others mentioned, on mobile it's not possible to keep Orbot when running other VPN. Looking at Moon feature requests (muun/apollo#21 (comment)), people are requesting built-in Tor support like in Phoenix:
So this really was a competitive advantage over others. For instance, Trezor Suite has similar useful built-in Tor support. Would be nice to have it reverted or re-implemented. Thanks!
|
This PR removes the tor-mobile-kmp library embedded inside Phoenix. Access to Tor is not done by Phoenix anymore, instead, when Tor is enabled in the application, Phoenix will exclusively use onion addresses when connecting to the Lightning peer and to Electrum.
Access to the Tor network is delegated to a Tor proxy application that needs to be installed by the user (such as Orbot).
This provides several improvements:
Besides, the
tor-mobile-kmp
library was difficult to maintain. For example, it did not contain a recent Tor binary. Orbot will do a much better job at that. Our library also caused issues with the Kotlin 2.0 update, and with M3 chips.