Matt Corallo [Sun, 6 Jan 2019 20:14:43 +0000 (15:14 -0500)]
Fix multi-remote-HTLC-claim preimage learning
When our counterparty claims multiple HTLCs from offered outputs in
one transaction we should still be able to learn the preimages.
Sadly, due to two bugs we were not previously doing so.
This tests a case we previously didn't handle correctly where we
returned early if there was no to_remote output to claim and thus
failed to fail-backwards HTLCs which were present.
Matt Corallo [Fri, 21 Dec 2018 19:32:44 +0000 (14:32 -0500)]
Move fail-backwards up for no to-remote output claims
This fixes HTLC fail-backwards in case we haven't yet sent enough
to have a to_remote output to claim, plus some edge cases where it
could be removed due to a fee update, though hopefully that goes
away with simplified_commitment.
Matt Corallo [Thu, 13 Dec 2018 21:23:22 +0000 (16:23 -0500)]
Use bitcoin_hashes' Ripemd160/Hash160 from rust-crypto+rust-bitcoin
Note that this requires rewriting full_stack_target tests, which
sucks, but it does let the fuzzer hit more stuff since the real
ripemd160 isn't ever actually called anymore.
Matt Corallo [Tue, 18 Dec 2018 03:43:05 +0000 (22:43 -0500)]
Update incorrect_payment_amount generation/handling for BOLT uptd
ie dont generate them as they're a really obvious privacy leak.
Luckily we were already handling them the same aside from log
printing so don't have to touch anything there. I was lazy in
updating tests but it only effects log printing, so whatever.
Matt Corallo [Tue, 11 Dec 2018 03:47:21 +0000 (22:47 -0500)]
Move monitor-generated HTLC event handling to manager event-getters
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH #80.
This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
Antoine Riard [Tue, 11 Dec 2018 04:56:02 +0000 (23:56 -0500)]
Add is_resolving_output in ChannelMonitor
Called in ChannelMonitor block_connected, returning
HTLCUpdate upstream via ManyChannelMonitor to
link htlcs between monitors. Used by ChannelManager to
fulfill/fail htlcs backwards accordingly
If spurrious HTLCUpdate are generated due to block re-scan
and htlc are already LocalRemoved, discard them in
channel get_update_*_htlc
Antoine Riard [Fri, 30 Nov 2018 15:58:44 +0000 (10:58 -0500)]
Track HTLCSource in ChannelMonitor
Insert it in current_local_signed_tx, prev_local_signed_tx,
remote_claimable_outpoints. For so get it provided by
Channel calls to provide_latest_{local,remote}_tx
Matt Corallo [Tue, 27 Nov 2018 02:54:14 +0000 (21:54 -0500)]
Handle monitor update failures in msg-recv functions
This adds a few TODOs around further message rebroadcasting which
needs to be implemented as well as some loss of tracking of HTLCs
on permanent channel failure which needs to get transferred over to
the appropriate in-memory ChannelMonitor.
Matt Corallo [Mon, 26 Nov 2018 21:40:15 +0000 (16:40 -0500)]
Swap handle_monitor_update_fail for a macro ala try_chan_entry
This resolves an API bug where send_payment may return a
MonitorUpdateFailed Err both when the payment will not be sent and
when the HTLC will be retried automatically when monitor updating
is restored. This makes it impossible for a client to know when
they should retry a payment and when they should not.
Matt Corallo [Mon, 10 Dec 2018 20:02:50 +0000 (15:02 -0500)]
Check tx output matches monitor output data (and is sufficient len)
Fixes a panic found by fuzzer in case the monitor per-commitment
data is garbage. We had a similar check for revoked commitment tx
but didn't copy it down to non-revoked commitment tx, so do that
now.
Matt Corallo [Tue, 27 Nov 2018 03:21:28 +0000 (22:21 -0500)]
Remove check which makes us sometimes never send closing_signed
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.
Matt Corallo [Fri, 23 Nov 2018 03:45:51 +0000 (22:45 -0500)]
Simplify + document the ChannelManager Err flow a bit
This removes all the channel-closure stuff from handle_error!() and
MsgHandleErrInternal, making all the Err handling consistent by
closing the channel before releasing the channel_state lock and
then calling handle_error!() outside of the lock.
Matt Corallo [Fri, 23 Nov 2018 00:38:28 +0000 (19:38 -0500)]
Remove MsgHandleErrInternal::from_chan_maybe_close as it's useless
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.
Matt Corallo [Mon, 19 Nov 2018 03:01:32 +0000 (22:01 -0500)]
Close channels on Err returns inside the same channel_state lock
If we never accessed channels for a peer outside of a message
handler for that peer then this wouldn't be a problem since message
handlers are required to be serialized per-peer. However, that
isn't the world we live in - we may want to forward payments or we
may get a send_payment call.