Repository navigation
Abstraction of ChannelId type #2408
Description
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:
- (current state) use native type
[u8; 32] - a named type alias (
type ChannelId = [u8; 32]). Can be used interchangebly. - a struct wrapping the native type, as 1-element tuple,
struct ChannelId(pub [u8; 32])), similar toPaymentId. Construction and usage sites need slight modification. - a struct wrapping the data in a private field, with accessor and special construction methods.
- an enum, with values for outpoint-based, temporary (and later revocation-based), all wrapping 32-byte array.
- (not recommended) a trait and different implementations for different variants.
Activity
tnull commentedon Jul 13, 2023
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
ChannelIdtype 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-L96I think we'd eventually like to have similar types for
ChannelId,UserChannelId,Amount, etc.TheBlueMatt commentedon Jul 14, 2023
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 commentedon Jul 26, 2023
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 commentedon Jul 27, 2023
I do not recommend an enum, or storing the channel ID 'type' information, on the grounds:
My recommendation:
(Updated description, this alternative is number 3.)
TheBlueMatt commentedon Aug 13, 2023
Fair enough. Kinda wish we had some way to get that info, but you're right it doesn't make sense.
optout21 commentedon Aug 14, 2023
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.