Use a single peer state map for all channel phases in peer state (channel, ChannelPhase, ChannelManager, PeerState)

https://github.com/lightningdevkit/rust-lightning/pull/2495

Host: vladimirfomene  -  PR author: dunxen

Notes

  • PR 2495 introduces the ChannelPhase enum with variants containing each existing channel struct so that they can be placed in a single map in PeerState.
  • This was one possible design considered during the conceptual review of the Channel split and refactoring work, but seemed to introduce too much complexity at the time when combined with the ChannelInterface type that was considered earlier but later dropped. At this stage it makes sense to take another look at using an enum map going forward to address some short-comings of the existing (however simple) multi-map approach for PeerState channels - one issue being higher order map lookups in a few cases.

Questions

  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?
  2. What are the advantages of having a single map for unfunded and funded channel types per peer? What are the disadvantages?
  3. We still want different types to differentiate between different unfunded and funded channels, how do we manage to have three different channel types in a single HashMap?
  4. We’d like to introduce a proper type for channel IDs in LDK, as seen in PR 2485. If we further differentiate between channel types, such as having separate types for random V1 temporary_channel_id’s, txid-based V1 channel_ids, zero-XORed V2 temporary_channel_ids, and XORed V2 channel_ids, how would we handle this for keys, or is it best to stick to just using plain [u8; 32]s in the channel_by_id map?
  5. What is the current phase of the moon? Please only answer with an emoji.