Repository navigation
Abstraction of ChannelId type #2408
Description
Activity
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.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.
Reacted by dunxen and optoutWork-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)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.)
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.
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.
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:
Proposed change:
a
ChannelIdenum, with values for outpoint-based and temporary IDs (and later revocation-based), all wrapping[u8; 32].Alternatives of increasing complexity / code change needed:
[u8; 32]type ChannelId = [u8; 32]). Can be used interchangebly.struct ChannelId(pub [u8; 32])), similar toPaymentId. Construction and usage sites need slight modification.