Specifically, there really should be no Errs, but in case there is
some case where duplicate HTLC removes are possible, return
IgnoreError and debug_assert to see if fuzzing can find them.
Matt Corallo [Fri, 14 Sep 2018 17:35:56 +0000 (13:35 -0400)]
Do not fail-backwards LocalAnnounced HTLCs upon force-close.
This is completely unsafe as we have provided the remote side with
a commitment_signed which they can broadcast, including the HTLC
transaction, and then could claim it on-chain after we've failed it
backwards!
Matt Corallo [Thu, 13 Sep 2018 15:34:26 +0000 (11:34 -0400)]
Optimize check_spend_remote HTLC a tad by avoiding indirections
Instead of hopping a pointer, we're only ever going to return one
Transaction at max, so skip the Vec. Also avoid
re-pubkey-converting the revocation key.
Antoine Riard [Tue, 11 Sep 2018 01:40:53 +0000 (01:40 +0000)]
Implement claiming of revoked HTLC transactions by ChannelMonitor
Refactor check_spend_remote_transaction in part to check_spend_remote_htlc to
avoid lock mess in block_connected. We need
remote_commitment_txn_on_chain to match remote HTLC tx
Matt Corallo [Wed, 12 Sep 2018 17:21:13 +0000 (13:21 -0400)]
Correct MAX_BUF_SIZE in serialization
I think this might have been my fault due to faulty review
feedback, but fuzzer caught trivial crash here where you try to
send a ping message larger than 16KB (but smaller than the
max-length 64KB) and you crash as message serialization is unwrap()
Matt Corallo [Tue, 11 Sep 2018 18:20:40 +0000 (14:20 -0400)]
Allow duplicate-payment_hash HTLCs for HTLC forwards
This is required by BOLT 2 to ensure that no attacker can simply
relay every public node a duplicate-payment_hash HTLC for each HTLC
it receives to deduce where an HTLC came from.
Note that this makes the claim logic much less incentive-compatible
as we will not claim all available HTLCs with the same payment_hash
even if we know the preimage! This is OK because, most likely, any
attackers trying to map the network will use small-value payments
and, hopefully, we will move away from constant hashes across an
entire payment at some point in the near future.
This further simplifies the payment transition state a bit, so
hopefully at least we got some readability out of all of this
Matt Corallo [Thu, 6 Sep 2018 23:12:32 +0000 (19:12 -0400)]
Sync get_update_fail_htlc, get_update_fulfill_htlc state err result
Both get_update_fail_htlc and get_update_fulfill_htlc should never
be called before any HTLC could have been accepted (ie
pre-ChannelFunded) nor should they ever be called
post-ShutdownComplete as the Channel object should be destroyed at
that point. Previously get_update_fulfill_htlc would panic, but
get_update_fail_htlc would return an Err. For now make them both
panic but we can revisit this if we want to have fewer panics in
the future.
Matt Corallo [Sat, 8 Sep 2018 14:32:39 +0000 (10:32 -0400)]
Update add_update_monitor docs to indicate registration req.
It wasn't entirely clear from the existing docs that it is the
responsibility of the implementor of ManyChannelMonitor to
register the relevant outpoint.
Matt Corallo [Fri, 7 Sep 2018 15:56:41 +0000 (11:56 -0400)]
Refactor/dont re-enter block_conencted on duplicate watch calls
Previously we'd hit an infinite loop if a block_connected call
always resulted in the same ChainWatchInterface registrations.
While we're at it, we also split ChainWatchUtil in two to make
things a bit more flexible for users, though not sure if that
actually matters, and make the matching more aggressive in testing,
even if we pick the more performant option at runtime.
Matt Corallo [Wed, 5 Sep 2018 18:47:43 +0000 (14:47 -0400)]
Drop HTLCState::LocalRemovedAwaitingCommitment
This was redundant and was included because the HTLC still needed
to be monitored, but that happens in ChannelMonitor, so there is no
need for it in Channel itself.
Matt Corallo [Wed, 5 Sep 2018 18:08:38 +0000 (14:08 -0400)]
Document+check commitment_signed generation success on send_htlc
Because we don't have an HTLCState for
update_add_htlc-generated-but-not-yet-commitment_signed to simplify
the mess of HTLCState match arms, any time a Channel::send_htlc
call returns Ok(Some(_)) we MUST call commitment_signed and it MUST
return success (or close the channel). We mention this in the docs
and panic if its not met in ChannelManager (which lets the fuzz
tester check this).
Matt Corallo [Wed, 5 Sep 2018 00:07:29 +0000 (20:07 -0400)]
Move announcement_signatures handling into new force-close macro
Because we've separated out channel closure from ErrorMessage
returning we can return error messages in a few additional cases,
like if the peer sent us a message for a channel they didn't own.
projects / rust-lightning / log