Channel phase refactor, wrap channel phase, store wrapped phase in map

[optout21]

Rationale:

• Better encapsulate channel phase, shield from ChannelManager
• In channel map, store wrapped channel phase, not phase
• Can change the channel phase without changing the map
• Preparation for easier introduction of new channel phases, for splicing

Changes in this PR:

• New ChannelWrapper struct to wrap a ChannelPhase
• ChannelManager.channel_by_id map holds ChannelWrapper structs, as opposed to ChannelPhase enums
• Transition from pending to funded channel can be done without removing the enrtry from the map and adding again, chaning only inside the wrapper struct around the phase.

Next steps:

• Rename Channel -> FundedChannel and ChannelWrapper -> Channel
• Create more handling method on Channel (channel wrapper), that dispatch into different phases.

[optout21]

Preferably we should get this in before further substantial dual funding or splicing changes

[dunxen]

Thanks!

Discussed this @jkczyz in some calls, makes sense and agree with moving the enum inward effectively. Happy to prioritise this as it will reduce matching hell in ChannelManager where this phase stuff never really belonged, but just need to review implementation first. At first glance it seems to create even more indirection. That's fine if it's temporary and cleans up ChannelManager now already. I'll do some more in-depth review.

[optout21]

At first glance it seems to create even more indirection. That's fine if it's temporary and cleans up ChannelManager now already. I'll do some more in-depth review.

Please note that this is not complete, but only the basic change to the map, further steps are possible (as noted in description). Various handling logic in ChannelManager (of type match channel.phase()) can be moved into Channel, where it can inspect its own phase. Yes, more encapsulation sometimes goes with more indirection.

[dunxen]

Please note that this is not complete, but only the basic change to the map, further steps are possible (as noted in description). Various handling logic in ChannelManager (of type match channel.phase()) can be moved into Channel, where it can inspect its own phase. Yes, more encapsulation sometimes goes with more indirection.

Sounds good! :) I wasn't too worried about the extra wrapper, just wanted to get the bigger picture of its specific use.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 71.03275% with 115 lines in your changes missing coverage. Please review.

Project coverage is 89.21%. Comparing base (0c31021) to head (a8f83dc).

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	68.22%	91 Missing and 18 partials :warning:
lightning/src/ln/channel.rs	86.36%	5 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #3418 +/- ## ========================================== - Coverage 89.22% 89.21% -0.01% ========================================== Files 130 130 Lines 106965 107038 +73 Branches 106965 107038 +73 ========================================== + Hits 95438 95496 +58 - Misses 8734 8748 +14 - Partials 2793 2794 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[optout21]

We may as well do this now. At very least the rename.

The rename of Channel would create a rather large diff, that's why I was a bit hesitant to do it right away. Considering.

[jkczyz]

The rename of Channel would create a rather large diff, that's why I was a bit hesitant to do it right away. Considering.

I don't think Channel is used in that many places. Definitely fewer than ChannelPhase, which you are already changing to ChannelWrapper here. Would be less churn that way.