Skip to content

Consider using an enum for channel phase in single PeerState::channel_by_id map #2422

Closed
@dunxen

Description

@dunxen

In a very early iteration of #2077, an enum for different channel structs was considered for the channel_by_id map. However, that early iteration still dealt with the ChannelInterface trait which made it clunky. We should consider it again now that we have a simplified ChannelContext. This is completely internal to ChannelManager and so does not affect the public API.

We'll also propose calling this enum ChannelPhase to not overload the "kind", "type", or "state" nomenclature. Its variants will contain the new channel structs.

It would be worth considering an enum with a single map for a few good reasons:

  • Adding new ChannelPhase variants with new channel structs does not mean adding a new map to PeerState.
  • Channel counting logic would not need to be updated with each additional map
  • An enum can ensure that we handle the cases
  • Often reduces the number of map lookups

One con I can see:

  • Some handling of the enum might be a little clunky in some places, but this might mean those areas could do with some light refactoring.

This depends on the completion of #2382 which is required for the 116 release.

(h/t @jkczyz for bringing this up in review of #2382)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions