This is the case pointed out by nayuta-gondo at
https://github.com/lightningnetwork/lightning-rfc/issues/499#issuecomment-438623208
though this doesn't actually solve the issue of ensuring we have a
consistent fee view when we start shutdown processing. There isn't
a clear solution to that however without adding additional state
tracking in Channel.
This also removes an associated test that tests for the correct
behavior (but didn't consider the bug) as we no longer behave
correctly. This should be fine as we'll be removing all the
update_fee garbage with option_simplified_commitment anyway.
Technically funding_transaction_generated was fine using it, but
calling force_shutdown on an empty Channel inside the channel_state
lock isn't a big deal and almost any other use of it would be
unsafe.
This converts block_connected failures to returning the
ErrorMessage that needs to be sent directly, since it always
results in channel closure and never results in needing to call
force_shutdown. It also converts update_add_htlc and closing_signed
handlers to ChannelError as the rest of the message handlers.
This extends 1b33064554 by
re-simplifying the ChannelMonitor <-> Channel interface a bit as we
never have any use for the latest remote commitment point until we
have knowledge of a remote transaction generated using it.
This is a big patch, but its all very mechanical, everything here
should be pretty obvious, and it all has to happen at once due to a
few common utility functions all having the same return type.
Note that this exposes a race in channel closure where we may
access a channel via some non-peer-specific mechanism like
forwarding an HTLC or sending a payment during the time between
the channel gave us a Close error and expected us to never call it
again and the time we actually removed it from the channel_state
set outside of the internal_* handler.
See https://github.com/lightningnetwork/lightning-rfc/issues/499
for a bit more about the ambiguity we're addressing here.
Also drop holding cell update_fee the same way we drop holding
cell update_add_htlcs when sending shutdown, resolving a bug.
We always require that any changes to Channel state be committed
immediately (within the same lock) so we should never have
uncommitted changes which would prevent us from sending a Shutdown
response.
This is both required by the protocol and also makes sense - if
we're the funder we don't mind accepting payment on the channel
after one confirmation because we assume we won't double-spend
ourselves.
