Abstraction of ChannelId type

[optout21]

Channel IDs are 32-byte IDs, and are represented simply as [u8; 32] in LDK.
A new type could be introduced, as an abstraction of the channel ID type. This would be a refactoring.

Rationale:

• better representation of a type representing a channel ID (abstraction)
• potential for indicating when a channel ID is only temporary
• separation of anticipated 'new' channel IDs that are not based on outpoint, but on revocation points, coming with Dual funded / Splicing features (see Dual-funded channels and Splicing Project Tracking #1621 ).

Proposed change:
a ChannelId enum, with values for outpoint-based and temporary IDs (and later revocation-based), all wrapping [u8; 32].

Alternatives of increasing complexity / code change needed:

0. (current state) use native type [u8; 32]
1. a named type alias (type ChannelId = [u8; 32]). Can be used interchangebly.
2. a struct wrapping the native type, as 1-element tuple, struct ChannelId(pub [u8; 32])), similar to PaymentId. Construction and usage sites need slight modification.
3. a struct wrapping the data in a private field, with accessor and special construction methods.
4. an enum, with values for outpoint-based, temporary (and later revocation-based), all wrapping 32-byte array.
5. (not recommended) a trait and different implementations for different variants.

[tnull]

I agree this would be nice to have dedicated types for many of the parameters used in the public API. Note that in LDK Node we have a ChannelId type already, mainly as [u8; 32] isn't easily exposable in the bindings there: https://github.com/lightningdevkit/ldk-node/blob/77acd3b49366c95fc6e13caaa910cf640b55dbd2/src/types.rs#L90-L96

I think we'd eventually like to have similar types for ChannelId, UserChannelId, Amount, etc.

[TheBlueMatt]

Thanks for writing this up! I think we should try to write the enum variant. I'm not sure that it'll be practical, but my first guess is at least most cases it'll be trivial, but its always possible we'll go through it and then find that there's some places where its just not practical and we have to stick with one of the non-enum versions.

[optout21]

Work-in-progress status:
1: Alias: Is relatively straightforward (branch https://github.com/optout21/rust-lightning/tree/channel-id-1alias2)
2: Struct/tuple: more mechanical change (#2449 , branch https://github.com/optout21/rust-lightning/tree/channel-id-2wrapper4)
*3: Struct/field: branch https://github.com/optout21/rust-lightning/tree/channel-id-4struct0
4: Enum: (#2456 branch https://github.com/optout21/rust-lightning/tree/channel-id-3enum2)

[optout21]

I do not recommend an enum, or storing the channel ID 'type' information, on the grounds:

• In the spec and in the wire communication channel ID is just 32 bytes, without information on its interpretation. In some cases it is possible to infer the type, but not in all cases.
• Retrieving information from the channel ID -- funding TX ID, etc. -- is generally not possible, and should not be needed.

My recommendation:

• a separate ChannelId type
• specific construction methods for creating funding-based and temporary IDs from input data. Also a generic constructor with data.
• 32-byte data stored as private field (not as a tuple, to be able to prevent direct access)
• accessor for data
• equality/ordering/hashing based on the data alone

(Updated description, this alternative is number 3.)

[TheBlueMatt]

I do not recommend an enum, or storing the channel ID 'type' information, on the grounds:

Fair enough. Kinda wish we had some way to get that info, but you're right it doesn't make sense.

[optout21]

Fair enough. Kinda wish we had some way to get that info, but you're right it doesn't make sense.

The possibility is open to later change the struct to an enum or extend with a enum type field, if it is needed or makes sense. Introducing a type is an improvement in any case.