Commit Graph

542 Commits

Author SHA1 Message Date
Olaoluwa Osuntokun
5f0fad85be multi: address lingering TODO by no longer wiping out local HTLCs on remote close
In this commit, we fix a lingering TOOD statement in the channel arb.
Before this commitment, we would simply wipe our our local HTLC set of
the HTLC set that was on the remote commitment transaction on force
close. This was incorrect as if our commitment transaction had an HTLC
that the remote commitment didn't, then we would fail to cancel that
back, and cause both channels to time out on chain.

In order to remedy this, we introduce a new `HtlcSetKey` struct to track
all 3 possible in-flight set of HTLCs: ours, theirs, and their pending.

We also we start to tack on additional data to all the unilateral close
messages we send to subscribers. This new data is the CommitSet, or the
set of valid commitments at channel closure time. This new information
will be used by the channel arb in an upcoming commit to ensure it will
cancel back HTLCs in the case of split commitment state.

Finally, we start to thread through an optional *CommitSet to the
advanceState method. This additional information will give the channel
arb addition information it needs to ensure it properly cancels back
HTLCs that are about to time out or may time out depending on which
commitment is played.

Within the htlcswitch pakage, we modify the `SignNextCommitment` method
to return the new set of pending HTLCs for the remote party's commitment
transaction and `ReceiveRevocation` to return the latest set of
commitment transactions on the remote party's commitment as well. This
is a preparatory change which is part of a larger change to address a
lingering TODO in the cnct.

Additionally, rather than just send of the set of HTLCs after the we
revoke, we'll also send of the set of HTLCs after the remote party
revokes, and we create a pending commitment state for it.
2019-05-27 14:34:13 -07:00
Johan T. Halseth
cd02c22977
htlcswitch+router: move deobfuscator creation to GetPaymentResult call
In this commit we move handing the deobfuscator from the router to the
switch from when the payment is initiated, to when the result is
queried.

We do this because only the router can recreate the deobfuscator after a
restart, and we are preparing for being able to handle results across
restarts.

Since the deobfuscator cannot be nil anymore, we can also get rid of
that special case.
2019-05-16 23:56:12 +02:00
Johan T. Halseth
f99d0c4c68
htlcswitch/switch+payment_result: define networkResult, extractResult 2019-05-16 23:56:12 +02:00
Johan T. Halseth
ec087a9f73
htlcswitch+router: define PaymentResult, GetPaymentResult
This lets us distinguish an critical error from a actual payment result
(success or failure). This is important since we know that we can only
attempt another payment when a final result from the previous payment
attempt is received.
2019-05-16 23:56:12 +02:00
Johan T. Halseth
be129eb7c7
htlcswitch/switch: remove numPendingPayments only used in tests 2019-05-16 23:56:12 +02:00
Johan T. Halseth
cff4d3547d
htlcswitch/switch: clarify paymentID uniqueness
With the following commits, it'll become important to not resuse
paymentIDs, since there is no way to tell whether the HTLC in question
has already been forwarded and settled/failed.

We clarify this in the SendHTLC comments, and alter the tests to not
attempt to resend an HTLC with a duplicate payment ID.
2019-05-16 23:56:11 +02:00
Johan T. Halseth
bbbe3a37f5
htlcswitch/switch: make parseFailedPayment not dependent on pkt
We will later persist the fields necessary to decrypt a received error.
2019-05-16 23:56:11 +02:00
Johan T. Halseth
c9e8ff6a34
switch+router+server: move NextPaymentID to router
This commit moves the responsibility of generating a unique payment ID
from the switch to the router. This will make it easier for the router
to keep track of which HTLCs were successfully forwarded onto the
network, as it can query the switch for existing HTLCs as long as the
paymentIDs are kept.

The router is expected to maintain a map from paymentHash->paymentID,
such that they can be replayed on restart. This also lets the router
check the status of a sent payment after a restart, by querying the
switch for the paymentID in question.
2019-05-16 23:56:06 +02:00
Joost Jager
570f9ca57e
htlcswitch/test: hodl invoice restart test
This commit adds a test that covers the hodl invoice behaviour after a
link restart.
2019-05-15 14:42:24 +02:00
Joost Jager
e5ead599cc
htlcswitch/test: use single channel restore function
This commit refactors test code around channel restoration in unit
tests to make it easier to use.
2019-05-15 14:42:21 +02:00
Joost Jager
064e8492de
cnct+htlcswitch+invoices: move invoice parameter check out of link
This commit is the final step in making the link unaware of invoices. It
now purely offers the htlc to the invoice registry and follows
instructions from the invoice registry about how and when to respond to
the htlc.

The change also fixes a bug where upon restart, hodl htlcs were
subjected to the invoice minimum cltv delta requirement again. If the
block height has increased in the mean while, the htlc would be canceled
back.

Furthermore the invoice registry interaction is aligned between link and
contract resolvers.
2019-05-15 14:42:12 +02:00
Joost Jager
d55a8b7b29
channel+cnct: remove preimage from channel and resolution
Now that the success resolver preimage field is always populated by the
incoming contest resolver, preimage lookups earlier in the
process (channel and channel arbitrator) can mostly be removed.
2019-05-15 14:41:49 +02:00
Olaoluwa Osuntokun
a8fa4094ba
Merge pull request #3027 from Roasbeef/new-onion-structs
router+build: update to the latest version of lightning-onion
2019-05-03 13:30:41 -07:00
Joost Jager
f5f6a52ed8
htlcswitch: check channel policy for local htlcs 2019-05-03 12:22:36 +02:00
Joost Jager
e571532d9a
htlcswitch: reorder policy checks
This commit reorders the policies check as a preparation for splitting
the checks in separate sets for the incoming and outgoing htlc.
2019-05-03 12:22:33 +02:00
Olaoluwa Osuntokun
56c969c911
htlcswitch: add new TestUpdateFailMalformedHTLCErrorConversion test
In this commit, we add a new test to ensure that we're able to properly
convert malformed HTLC errors that are sourced from multiple hops away,
or our direct channel peers. In order to test this effectively, we force
the onion decryptors of various peers to always fail which will trigger
the malformed HTLC logic.
2019-04-30 20:13:38 -07:00
Olaoluwa Osuntokun
be63c7d286
htlcswitch: properly convert multi-hop malformed HTLC failures
In this commit, we now properly convert multi-hop malformed HTLC
failures. Before this commit, we wouldn't properly add a layer of
encryption to these errors meaning that the destination would fail to
decrypt the error as it was actually plaintext.

To remedy this, we'll now check if we need to convert an error, and if
so we'll encrypt it as if it we were the source of the error (the true
source is our direct channel peer).
2019-04-30 20:13:37 -07:00
Olaoluwa Osuntokun
cdc4aca40f
htlcswitch: properly handle direct link malformed HTLC failures
In this commit, we fix a bug that caused us to be unable to properly
handle malformed HTLC failures from our direct link. Before this commit,
we would attempt to decrypt it and fail since it wasn't well formed. In
this commit, if its an error for a local payment, and it needed to be
converted, then we'll decode it w/o decrypting since it's already
plaintext.
2019-04-30 20:13:36 -07:00
Olaoluwa Osuntokun
c67ca0a329
htlcswitch: add new EncryptMalformedError method to ErrorEncrypter
In this commit, we add a new method to the ErrorEncrypter interface:
`EncryptMalformedError`. This takes a raw error (no encryption or MAC),
and encrypts it as if we were the originator of this error. This will be
used by the switch to convert malformed fail errors to regular fully
encrypted errors.
2019-04-30 20:13:35 -07:00
Olaoluwa Osuntokun
649e080da6
htlcswitch: for UpdateFailMalformedHTLC packets mark fail as needing conversion
In this commit, we start the first phase of fixing an existing bug
within the switch. As is, we don't properly convert
`UpdateFailMalformedHTLC` to regular `UpdateFailHTLC` messages that are
fully encrypted. When we receive a `UpdateFailMalformedHTLC` today,
we'll convert it into a regular fail message by simply encoding the
failure message raw. This failure message doesn't have  a MAC yet, so if
we sent it backwards, then the destination wouldn't be able to decrypt
it. We can recognize this type of failure as it'll be the same size as
the raw failure message max size, but it has 4 extra bytes for the
encoding. When we come across this message, we'll mark is as needing
conversion so the switch can take care of it.
2019-04-30 20:13:34 -07:00
Olaoluwa Osuntokun
8418dc5530
Merge pull request #2939 from Roasbeef/hodl-htlc-fix
htlcswitch: only perform CLTV checks for HTLCs with open invoices
2019-04-15 12:52:45 -07:00
Joost Jager
40cea6b487
htlcswitch/test: increase test timeout for hodl invoice tests 2019-04-12 09:01:05 +02:00
Olaoluwa Osuntokun
5d68f0f9eb
htlcswitch: only perform CLTV checks for HTLCs with open invoices
In this commit, we fix a bug that would cause a node with a hodl HTLC to
cancel back the HTLC upon restart if the invoice has been settled, but
the HTLC is still present on the commitment transaction. A fix for the
HTLC still being present (not triggering a new commitment) has been
fixed recently. However, for older nodes with a lingering HTLC, on
restart it would be failed back.

In this commit, we make the check stricter by only performing these
checks for HTLCs that are in the open state. This ensures that we'll
only check this constraints the first time around, before the HTLC has
been transitioned to the accepted state.
2019-04-11 17:53:25 -07:00
Conner Fromknecht
057bed2b92
htlcswitch/link: delay restored channel reestablish
This commit adds a brief delay when sending our channel reestablish
message if the link contains a restored channel to ensure we first have
a stable connection. Sending the message will cause the remote peer to
force close the channel, which currently may not be resumed reliably if
the connection is being torn town simultaneously. This delay can be
removed after the force close is reliable, but in the meantime it
improves the reliability of successfully closing out the channel and
allows the `channel_backup_restore/restore_during_creation` to pass
reliably.
2019-04-11 15:42:02 -07:00
Olaoluwa Osuntokun
bc425b1a77
Merge pull request #2918 from joostjager/hodl-drop-fix
htlcswitch: ensure commit tx gets updated after settle hodl invoice
2019-04-11 15:38:56 -07:00
Olaoluwa Osuntokun
5ac9ba6472
htlcswitch: synchronously send the chan sync message to the remote peer
In this commit, we modify the starting link logic to always send the
chan sync message to the remote peer in a synchronous manner. Otherwise,
it's possible that we fail very quickly below this block, and don't ever
send the message to the remote peer.
2019-04-10 18:01:52 -07:00
Joost Jager
72c86fbf5d
htlcswitch/test: test revocation window exhaustion 2019-04-10 21:30:31 +02:00
Joost Jager
6d2dfed03d
htlcswitch: increase batch counter for exit hop settle and fail
The idea of the batch counter is to increase it for commit tx updates,
so that if the commit tx cannot be updated immediately (revocation
window exhausted), the batch ticker makes sure it happens later.

The batch counter was increased for forwarded htlcs, but not for exit hop
resolutions.

This lead to the situation where the commitment tx would not be updated,
even though the htlc was settled locally. When no other changes happen
on the channel, the htlc eventually reaches its expiry and the channel
is force closed.
2019-04-10 21:04:51 +02:00
Joost Jager
038ce342b3
htlcswitch: do not check final cltv for accepted invoices 2019-04-08 18:16:21 +02:00
Joost Jager
206d93d856
htlcswitch/test: test zero value for outbound cltv reject delta 2019-04-05 11:36:18 +02:00
Joost Jager
1b2816006f
htlcswitch/test: align test invoice cltv expiry 2019-04-05 11:36:16 +02:00
Joost Jager
af7d0e5ff5
htlcswitch/test: convert TestChannelLinkSingleHopPayment to two hop network 2019-04-05 11:36:13 +02:00
Joost Jager
037913fd28
link: rewrite height comparisons without subtraction
Prevent the case where a uint32 wrap around could happen.
2019-04-05 11:36:10 +02:00
Joost Jager
ab4da0f53d
cnct: define separate broadcast delta for outgoing htlcs
This commits exposes the various parameters around going to chain and
accepting htlcs in a clear way.

In addition to this, it reverts those parameters to what they were
before the merge of commit d1076271456bdab1625ea6b52b93ca3e1bd9aed9.
2019-04-05 11:36:07 +02:00
Olaoluwa Osuntokun
893cff60c4
Merge pull request #2839 from joostjager/htlc-expiry-check
link: increase expiry grace delta for exit hop and forwarded htlcs
2019-04-01 18:15:56 -07:00
Olaoluwa Osuntokun
1afec1342b
htlcswitch: update syncChanStates for new ChanSyncMsg API 2019-03-28 17:53:56 -07:00
Johan T. Halseth
59e3390f6c
Merge pull request #2750 from cryptagoras/patch-1
htlcswitch/switch: log target node on insufficient cap error
2019-03-27 15:10:08 +01:00
Joost Jager
cd535b9401
link: increase expiry grace delta
This commit increase the expiry grace delta to a value above the
broadcast delta. This prevents htlcs from being accepted that would
immediately trigger a channel force close.

A correct delta is generated in server.go where there is access to
the broadcast delta and passed via the peer to the links.

Co-authored-by: Jim Posen <jim.posen@gmail.com>
2019-03-26 18:42:00 +01:00
Joost Jager
e6fbbaa1dc
link: also check expiry grace delta for forwarded htlcs
Previously there was no minimum remaining blocks requirement on
forwarded htlcs, which may cause channel arbitrator to force
close the channel directly after forwarding the htlc.

Co-authored-by: Jim Posen <jim.posen@gmail.com>
2019-03-26 18:41:41 +01:00
cryptagoras
a50428ca96 htlcswitch/switch: log target node on insufficient cap error 2019-03-25 18:25:58 +02:00
Joost Jager
0823c79e4e
htlcswitch/test: hodl invoice test 2019-03-15 10:09:20 +01:00
Joost Jager
32f2b047e8
htlcswitch: hodl invoice
This commit modifies the invoice registry to handle invoices for which
the preimage is not known yet (hodl invoices). In that case, the
resolution channel passed in from links and resolvers is stored until we
either learn the preimage or want to cancel the htlc.
2019-03-15 10:09:17 +01:00
Joost Jager
1f41a2abce
htlcswitch: abtract invoice from link
This commit detaches signaling the invoice registry that an htlc was
locked in from the actually settling of the htlc.

It is a preparation for hodl invoices.
2019-03-15 10:09:15 +01:00
Joost Jager
aeb35d9898
htlcswitch/test: use real invoice registry with temp db as mock
In further commits the behaviour of invoice registry becomes more
intrinsically connected to the link. This commit prepares for that by
allowing link and registry to be tested as a single unit.
2019-03-15 10:09:12 +01:00
Joost Jager
e464ed18c7
htlcswitch/test: make unknown hash test independent of mock
In the TestChannelLinkMultiHopUnknownPaymentHash test, a preimage was
modified to trigger an unknown payment hash failure. The way the mock is
implemented, it would take the hash of that modified preimage and store
it. It basically stores a completely different invoice. For this test,
it is just as good to store no invoice at all.
2019-03-15 10:09:09 +01:00
Joost Jager
c23bb5b3f1
htlcswitch: extract settle invoice into method 2019-03-15 10:09:03 +01:00
Joost Jager
19f79613df
channeldb: store hold invoice 2019-03-15 10:08:55 +01:00
Joost Jager
4141773e90
htlcswitch: resolve invoice cancelation race condition
Previously it could happen that an invoice was open at the time of the
LookupInvoice call, the htlc was settled because of that, but when the
SettleInvoice call was made eventually, it would fail because the
invoice was canceled in the mean time. The htlc would then be settled,
but the invoice not marked as such.
2019-03-15 10:08:46 +01:00
Joost Jager
eb598ec7a4
htlcswitch: extract exit hop processing to method 2019-03-15 10:08:43 +01:00
Joost Jager
9643b45dbc
htlcswitch/test: move preimage cache to server level
In this commit the preimage cache is instantiated on the mock server
level where it belongs. Previously it was a cache shared across all mock
servers.
2019-03-15 10:08:40 +01:00