Encode & test channel_update message type in failure messages.

[tnull]

Following the clarification of the spec (lightning/bolts#979) and the ability to decode type-prefixed channel updates attached to failure messages (#1455), this PR implements also the encoding of said channel updates.

It also aims to increase test coverage, in particular to check whether we still allow decoding of channel updates without the type prefix be be backwards-compatible.

Based off #1455.

Closes #1450.

[tnull]

First steps towards encoding are done, which however feel somewhat hacky. Optimally, there should be a neater solution to
serialize/deserialize ChannelUpdates with and without the type prefix. However, I'm afraid that this would not be an easy fix without changing how we handle deserialization/serialization generally?

Currently two tests are still failing and the new one(s) are still work in progress.

[TheBlueMatt]

Yea, I don't think it makes sense to go rewrite the serialization stuff, this is fine. That said, lets get the constant from ChannelUpdate::TYPE, which may require importing the ln::wire::Encode trait.

[codecov-commenter]

Codecov Report

Merging #1465 (11768fb) into main (6592081) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 11768fb differs from pull request most recent head 6d8be70. Consider uploading reports for the commit 6d8be70 to get more accurate results

@@            Coverage Diff             @@
##             main    #1465      +/-   ##
==========================================
- Coverage   90.92%   90.91%   -0.01%     
==========================================
  Files          75       75              
  Lines       41842    41869      +27     
  Branches    41842    41869      +27     
==========================================
+ Hits        38043    38066      +23     
- Misses       3799     3803       +4     

Impacted Files	Coverage Δ
lightning/src/ln/channelmanager.rs	84.69% <100.00%> (+<0.01%)	⬆️
lightning/src/ln/onion_route_tests.rs	97.51% <100.00%> (+0.05%)	⬆️
lightning/src/ln/onion_utils.rs	95.37% <100.00%> (+0.03%)	⬆️
lightning/src/ln/priv_short_conf_tests.rs	97.95% <100.00%> (+0.03%)	⬆️
lightning/src/util/events.rs	33.21% <0.00%> (-0.35%)	⬇️
lightning-net-tokio/src/lib.rs	75.15% <0.00%> (-0.32%)	⬇️
lightning/src/ln/functional_tests.rs	97.13% <0.00%> (-0.04%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6592081...6d8be70. Read the comment docs.

[tnull]

Rebased and added test to confirm we still can handle failure messages including channel updates without the type prefix.

[tnull]

Squashed.

[vincenzopalazzo]

I need to review the tests, and I will do it in a couple of hours, but I have considered to how to remove the prefix

[vincenzopalazzo]

I would like to have a very tiny comment that documents what this sum will do!

This make it harder to read for people that switch implementation during the day

[tnull]

I would like to have a very tiny comment that documents what this sum will do!

I'm not sure that a comment would provide much more information that is not self-explanatory? Maybe chan_update.serialized_length() + mem::size_of<u16>() + mem::size_of<u64> + mem::size_of<u16>() would be more expressive, but would also render the line unnecessarily long? Also, this only reserves a certain amount of vector capacity, nothing would really break if the calculation would be off by a byte or two.

This make it harder to read for people that switch implementation during the day

Mh, how does this relate to other implementations?

[vincenzopalazzo]

Mh? I mean a comment to explain the meaning of the number // to the serialization length we sum up: 2 (flag) + 8 (whatever) + 2(whatever)

BTW, it is not relevent!

[valentinewallace]

Looks good! Tried to hunt for other places we encode channel updates in errors but they seem to be covered, and the testing hits the two cases.

[vincenzopalazzo]

LGTM