[WIP] New ChannelId type, wrapper for [u8; 32]

[optout21]

Partial solution for #2408 .

[ariard]

"A unique 32-byte identifier for a channel.”

I think the uniqueness derivation guarantees can be documented (both for legacy channels and new channel id dual funding). Our internal ChannelId struct could be added a per-context (legacy/ dual-funding) prefix to commit to the channel type, and this prefix could be consumed by something like the Validating Lightning Signer to enforce channel policy based on the type. As a practical application, I think this is a bit of a premature though policy downgrade based on channel type confusion has been a real security issue for VLS in the past.

[codecov-commenter]

Codecov Report

Patch coverage: 80.85% and project coverage change: +0.02% 🎉

Comparison is base (e13ff10) 90.24% compared to head (7d6eed2) 90.26%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2449      +/-   ##
==========================================
+ Coverage   90.24%   90.26%   +0.02%     
==========================================
  Files         106      106              
  Lines       55817    55830      +13     
  Branches    55817    55830      +13     
==========================================
+ Hits        50370    50394      +24     
+ Misses       5447     5436      -11     

Files Changed	Coverage Δ
lightning/src/routing/gossip.rs	89.61% <ø> (ø)
lightning/src/routing/router.rs	93.54% <ø> (ø)
lightning/src/ln/peer_handler.rs	61.60% <7.69%> (ø)
lightning/src/util/macro_logger.rs	89.58% <50.00%> (ø)
lightning/src/ln/msgs.rs	85.14% <75.55%> (+0.40%)	⬆️
lightning/src/events/mod.rs	42.78% <76.47%> (+1.18%)	⬆️
lightning/src/ln/channelmanager.rs	85.47% <90.27%> (+0.02%)	⬆️
lightning/src/ln/channel.rs	89.44% <96.55%> (+0.03%)	⬆️
lightning-invoice/src/utils.rs	97.69% <100.00%> (ø)
lightning/src/chain/channelmonitor.rs	94.67% <100.00%> (ø)
... and 5 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[optout21]

Checks OK except per-commit check; squashing the commits would probably solve it.
Not intended for merging in this form, closing.

[optout21]

I think the uniqueness derivation guarantees can be documented

I haven't given much though about the exact meaning of uniqueness here, is it for the whole network, for one node, or for one node-pair.

Our internal ChannelId struct could be added a per-context (legacy/ dual-funding) prefix to commit to the channel type

I was thinking the same, but then I realized that this does not make much sense, as often the channel ID is received from the network in messages, and no context information is available there. This could be done only with a change in the protocol...
In general most of the time it's not possible to know if a channel ID is temporary or funding-based, so I decided that storing this info in the few cases when it is known does not make much sense.

(BTW, latest non-draft PR is #2485 )

[TheBlueMatt]

In general most of the time it's not possible to know if a channel ID is temporary or funding-based, so I decided that storing this info in the few cases when it is known does not make much sense.

I'm not sure about this - it may be the case that it's often not possible to know if a channel is v1 or v2 without context, but we should almost always know if a channel id is pre-funding or not because most messages are only allowed in a specific context.

[optout21]

An example: we receive a closing_signed message. It contains a channel ID, which is deserialized into a field of ChannelId type. At this point we don't even know if the channel exists. Then we look up this channel, and check if it's in the right state. Could it be a temporary channel ID? Maybe in this particular case it cannot, but I imagine there are scenarios where both are possible.
The point is that that variety of the channel ID is often can be inferred from external context, and it may be messy to try to capture this in an enum. Or it's possible to have a generic/unknown value of the enum, but that makes the whole point weaker.