ACINQ / phoenix

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

Add "smart" channels flow in PeerManager #338

Closed dpad85 closed 1 year ago

dpad85 commented 1 year ago

The purpose of this PR is to help the UI have a more consistent view of the wallet's channels by providing a single source of truth.

Problem

The Peer instance already exposes flow of the channels in bootFlow and channelsFlow, the first one being the channels in the database when starting up the node, and the second one being the actual, live channels that have been reestablished with the peer, and that second flow is empty while the node is starting (which can take a long time if the connection is bad). In any case this level of details is not useful to the user ; both flows must be combined into one flow. This is what the BalanceManager used to do. The BalanceManager also applied specific rules to compute the usable balance, by taking the channels' state into account.

However there are other places in the app where we need the list of channels: the channels list screen in the settings, the receive screen (to know if we need to display a liquidity disclaimer), the drain wallet screen...

Proposed solution

We add a new channelsFlow in PeerManager that combines Peer.bootFlow and Peer.channelsFlow and publishes a map of channelId -> LocalChannelInfo which is a new object.

data class LocalChannelInfo(
    val channelId: String,
    val state: ChannelState,
    val isBooting: Boolean,
) {
    val isUsable by lazy { state is Normal && !isBooting }
    val localBalance by lazy { state.localBalance() }
    ...
}

This object explicitly declares if the channel data comes from the DB (isBooting: Boolean), and also contains a few helpers methods (e.g. to get the usable balance). This flow is what should now be used, whether it is by other managers/MVI controllers in the shared module or directly by the UI code.

Only commit fda6c17104f40fc3c21eef0db0e9b5802ee876ca needs to be reviewed, the other one only affects Android.

robbiehanson commented 1 year ago

if the wallet starts and fails to connect (let's suppose the ACINQ node is gone) then the user is unable to close any channels! So we should use the new PeerManager.channelsFlow here instead.

I think you're correct. Glancing at the code I see there's an isForceClosable function that returns true for ChannelInfoStatus.Offline, which is what we want in this scenario.

robbiehanson commented 1 year ago

Good idea :+1: