-
Notifications
You must be signed in to change notification settings - Fork 102
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
Support taproot addresses and rotation #516
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note that this PR will have to be rebased on kotlin 1.9. |
The swap-in wallet now exposes a flow containing the current address, and a flow of addresses that have been used. Those addresses use taproot and txs should be lighter and more private. Note that the wallet does not expose future unused addreses, only past addresses that have already received funds, or the most recent (index-wise) address that thas not been used. The legacy address is still available and will still work, however usage should be discouraged. There have been some api changes in a few methods. The swap-in model have been removed from the ReceiveController since it's not used anymore, and we want to move away from the MVI pattern.
By default the app should use the last available address exposed by the swap-in wallet. The user can switch to the legacy address if need be, for compatibility purposes with older services. Previously used addresses can be displayed in the Receive screen for now. Since the swap-in wallet may take time to synchronize all used addresses at startup, the app stores the last known unused index in the preferences, and uses that index when the app starts. This way, the user does not have to wait for an address to be shown before he can deposit funds to Phoenix. There's a risk of address reuse but it's small enough and the UX benefits are significant if the wallet must check a lot of addresses.
The standalone business started by the channels-watcher should be terminated to avoid having another electrum watcher running in the background.
The receive view is back to its original state.
Also add `script` to BitcoinUri object for ease of use.
dpad85
force-pushed
the
taproot-rotate
branch
from
February 15, 2024 12:18
35071b6
to
c56b6d9
Compare
side-note: The phoenix-legacy app cannot compute swap-in addresses for the new taproot-based protocol, but it should be easy to implement if needed:
This will make the old version of |
- Either, Try, Chain are now provided by bitcoin-kmp - NodeParams has been simplified - Added a new Bolt12Invoice type; PaymentRequest is an interface. See: ACINQ/lightning-kmp#590 ACINQ/lightning-kmp#603 ACINQ/lightning-kmp#605
Prevents rare issue with db lock
robbiehanson
approved these changes
Feb 22, 2024
This is downgrade avoids the new animation effects added in v2.7.0 which are distracting and hard to remove.
This fixes an issue with trampoline payments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for taproot addresses
Phoenix now shows a taproot address by default for swap-ins (on-chain deposits to Phoenix). Taproot addresses are more private (observers won't know funds sent there are for LN channels) and are cheaper (~15% less fee, and once channels also support taproot, it could be up to 30% less expensive in total).
Backward compat with legacy addresses
Phoenix will still be able to swap funds sent on the legacy address, even if it's not the preferred way. User can also switch back to the legacy address for deposits from wallets/services that don't support taproot.
Address rotation
Taproot addresses are rotated, which also improves privacy besides the taproot features - see ACINQ/lightning-kmp#584 for details.
It means that Phoenix will synchronise its view of the swap-in wallet at startup to find the last unused address, and avoid address reuse. This synchronisation can take time (dozens of seconds if many addresses), during which the UI should still display an address (for good UX).
@robbiehanson To do so, the last known unused index should be stored in the preferences, and used at startup to provide an address to the UI:
Then listen to
Peer.swapInWallet.swapInAddressFlow
and use that address at collect (which should be the same address as calculated before, unless a payment was made while the app was off).Show used addresses
The user can check his addresses in Settings > Wallet info > Swap addresses.
These addresses are exposed in
Peer.swapInWallet.wallet.walletStateFlow
. Note that the legacy address is contained in that flow (using aAddressMeta.Single
meta).Also, this flow only exposes the adresses that have been used, + the first unused address. It does not list other unused addresses.