This removes a bunch of potentially new error handling in writers
and the checks were kinda useless anyway - in normal operation we
unwrap()ed anyway, and we're gonna want to use the serializtion
framework for ChannelMonitor/ChannelManager serialization, which
may generate things larger than 64KB anyway.
This resolves a spec-compliance bug with BOLT 4 where we simply
failed to deserialize the message and thus could never return an
HTLC failure message. However, note that BOLT 4 incorrectly hints
that a non-malformed message should be used ("...MUST report a
route failure to the origin node") which we cannot do as we cannot
derive a SharedSecret to encrypt a regular update_fail_htlc message
UpdateFailHTLC isn't really an error anymore now that its handled
async after channel commitment (as required by BOLT 2), and since
its unused this is free. To resolve the TODO which intended to use
it for HTLC failure when trying to route forwards, we instead opt
to merge all the HTLC update events into one UpdateHTLCs event
which just contains a CommitmentUpdate object.
Add test for DisconnectPeer event
Update DisconnectPeer with optional ErrorMessage
Manage error for funding_transaction_generated
Add disconnect_socket to SocketDescriptor trait
Summary:
Implementing these, taking inspiration from the surrounding code since i'm a rust n00b.
Test Plan:
```
cargo build
cargo test channel_reestablish
```
output:
```
running 3 tests
test ln::msgs::tests::encoding_channel_reestablish_with_secret ... ok
test ln::msgs::tests::decode_channel_reestablish_bad_length ... ok
test ln::msgs::tests::encoding_channel_reestablish_no_secret ... ok
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 14 filtered out
```
this was a TODO and also briefly discussed in https://github.com/rust-bitcoin/rust-lightning/pull/43#pullrequestreview-135649551
I'm not fully sure how to remove the `Option`, and make it completely required. Would love suggestions. So, have omitted that for now. Plus, better to make smaller, incremental changes.
Test Plan:
`cargo build`
`cargo test`
It obviously was nonsensical prior, though its now way more
complicated...there's refactoring work to be done here, but it
should at least kinda sorta work now.
