Introduce new ChannelId struct

[optout21]

Fixes #2408 . Introduces a new ChannelId struct, enclosing the 32-byte data. It has specific constructors, for normal funding-tx-based and for temporary channel ID cases.

ChannelId is exposed as lightning::ln::ChannelId.

This is a breaking change. The type of channel_id parameter has been changed from [u8; 32] to ChannelId in several ChannelManager APIs, namely:

• create_chan_between_nodes()
• create_chan_between_nodes_with_value()
• create_funding_transaction()
• sign_funding_transaction()
• open_zero_conf_channel()
• create_chan_between_nodes_with_value_confirm_second()
• create_chan_between_nodes_with_value_confirm()
• create_chan_between_nodes_with_value_a()
• create_announced_chan_between_nodes()
• create_announced_chan_between_nodes_with_value()
• close_channel()
• test_txn_broadcast()

The type has been changed in several events as well (FundingGenerationReady, PaymentClaimable, PaymentForwarded, ChannelPending, etc.).

Review hints:

• The new ChannelId struct is in channel.rs https://github.com/lightningdevkit/rust-lightning/pull/2485/files#diff-8d28e96b6f27edb9a9739b2dce334f8906ec906155d421a7b5b02aa8e6c96057
• Affected APIs are in channelmanager.rs https://github.com/lightningdevkit/rust-lightning/pull/2485/files#diff-2423bfc3e50222fe4bfbd77f65cb8bb78625558ace3df2ef80619bc712b634af
• Many changes involve logging (log_bytes!(channel_id)), those are straightforward but numerous

TODO:

• Determine how it affects APIs, uses, backward-compatibility
• Tests for ChannelId serialization/deserialization
• New macro log_channel_id!()
• Review hints

[codecov-commenter]

Codecov Report

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

Comparison is base (3dffe54) 90.56% compared to head (e99e6ab) 90.59%.
Report is 2 commits behind head on main.

❗ 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    #2485      +/-   ##
==========================================
+ Coverage   90.56%   90.59%   +0.02%     
==========================================
  Files         109      110       +1     
  Lines       57304    57410     +106     
  Branches    57304    57410     +106     
==========================================
+ Hits        51899    52008     +109     
+ Misses       5405     5402       -3     

Files Changed	Coverage Δ
lightning/src/ln/mod.rs	96.15% <ø> (ø)
lightning/src/routing/gossip.rs	89.94% <0.00%> (-0.13%)	⬇️
lightning/src/util/test_utils.rs	73.61% <ø> (ø)
lightning/src/ln/peer_handler.rs	61.45% <13.79%> (+0.40%)	⬆️
lightning/src/util/macro_logger.rs	93.61% <50.00%> (+4.03%)	⬆️
lightning/src/events/mod.rs	48.33% <78.94%> (-0.10%)	⬇️
lightning/src/ln/msgs.rs	85.82% <81.66%> (+0.21%)	⬆️
lightning/src/ln/channelmanager.rs	87.19% <88.23%> (-0.02%)	⬇️
lightning/src/ln/channel.rs	89.83% <95.83%> (+<0.01%)	⬆️
lightning-invoice/src/utils.rs	97.67% <100.00%> (ø)
... and 7 more

... and 5 files with indirect coverage changes

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

[dunxen]

This overall looks like an improvement, but I'd like to maybe see what others had to say about types for different IDs like

• temporary channel IDs for V1 unfunded channels
• channel IDs for V1 funded channels with known funding txids
• temporary channel IDs (revocation basepoint XORed with zero) for V2 channels for open_channel2 and accept_channel2
• channel IDs (both party revocation basepoints XORed) for V2 channels after accept_channel2.

For me currently I don't see a huge benefit in splitting all these cases into distinct types or even enum variants. So I think this PR is all we need so that we at least have type safety for channel ID bytes vs hash bytes for example.

[TheBlueMatt]

Mostly some minor process nits on commit construction, but LGTM.

[optout21]

Review comments addressed. commits regrouped (into 2 commits) as suggested.
Only outstanding issue is where to have the ChannelId declaration -- leave in channel.rs, in mod.rs, or in new, own mod.

[dunxen]

LGTM with moving the ChannelId stuff to its own mod.

[optout21]

Moved ChannelId to its own new module (channel_id.rs); but need to clean up the commits

[dunxen]

Oops sorry, yeah looks good for squash. Also missing module docs for the new module

[optout21]

Per-commit check failed, fixed (moved last fix in first commit)

[optout21]

This PR touches many logging lines, so it frequently gets in conflict with current main... I keep rebasing+resolving conflicts, but it would be nice to get more approvals ;)
Rebased again.

[TheBlueMatt]

Heh, sorry, been a super busy merge week. Will review now and hopefully @dunxen is still around to ack.

[TheBlueMatt]

Two quick comments, didnt look at the second commit but i assume its fine.

[optout21]

Simplified as suggested (@TheBlueMatt): changed struct into a pub tuple; removed .bytes() (use .0 instead).
Also found some instances in tests where temp ID is generated, but from_bytes() was used.
Also some rustdoc formatting.

[TheBlueMatt]

why did you add brackets?

[optout21]

Hmm, typo, after subsequent add and delete.

[TheBlueMatt]

Good to keep the blame layer clean (doesn't matter much for use statements), but makes it easier to read stuff back later :)

[optout21]

2 CI builds fail, likely due to #2527 . 2 approvals.