Skip to content
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 29 commits into from
Feb 22, 2024
Merged

Support taproot addresses and rotation #516

merged 29 commits into from
Feb 22, 2024

Conversation

dpad85
Copy link
Member

@dpad85 dpad85 commented Feb 9, 2024

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).

image

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.

image image image

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:

keyManager.swapInOnChainWallet.getSwapInProtocol(index).address(chain)

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.

image image

These addresses are exposed in Peer.swapInWallet.wallet.walletStateFlow. Note that the legacy address is contained in that flow (using a AddressMeta.Single meta).

Also, this flow only exposes the adresses that have been used, + the first unused address. It does not list other unused addresses.

@dpad85
Copy link
Member Author

dpad85 commented Feb 9, 2024

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.
@sstone
Copy link
Member

sstone commented Feb 20, 2024

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:

  • updatebitcoin-lib 0.18.3 and add bitcoin-kmp 0.17.0 and secp256k1-kmp 0.14.0
  • update eclair-core v0.4.23-android-phoenix and add a method to compute new swap-in addresses

This will make the old version of eclair-core used by the legacy app depend on kotlin 1.9 but I understand that it is not a problem as the legacy app is already using kotlin 1.9.

@dpad85 dpad85 marked this pull request as ready for review February 21, 2024 17:27
@dpad85 dpad85 requested a review from robbiehanson February 21, 2024 17:27
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.
@dpad85 dpad85 merged commit d38a83f into master Feb 22, 2024
@dpad85 dpad85 deleted the taproot-rotate branch February 22, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants