This commit adds an additional santity check that rejects zero-value
HTLCs, preventing them from being added to the channel state even if the
channel config's minhtlc value is zero.
In #4130, OpenChannel was changed to assert that the wallet is fully
synced before allowing a channel open. This introduced flakes on travis,
which are resolved here by using a wait predicate when calling
OpenChannel.
Note there is one existing call that was not converted, because it is
interested in the returned error. This call does not have a wait
predicate surrounding it, but this shouldn't cause a flake because other
channels are opened earlier in the test that will have already waited
for the wallet to sync up.
This commit adds a test to exercise that HTLC signatures are sent in the
correct order, i.e. they match the sorting of the HTLC outputs on the
commitment after applying BOLT 3's BIP69+CLTV sort.
This commit fixes#4118 by properly sorting the HTLC signatures sent
over the wire to match the BOLT3 BIP69+CLTV sorting of the commitment
outputs.
To do so, we expose the slice of cltv deltas for HTLCs on the unsigned
commitment after applying the commitment sorting. This will be used to
locate the proper output index, as the CLTV serves as a tie breaker
between HTLCs that otherwise have the same payment hash and amount.
Note that #3412 fixed the issue partially by ensuring the commitment was
constructed properly (and the second-level prev outpoint's txid was
correct), but failed to address that the HTLC signatures were still sent
out in the incorrect order. With this, we pass the test case introduce
in the next commit.
We currently write each HTLCs OutputIndex to disk, but we don't use it
when restoring. The restoration is modified to use these directly, since
we will have lost access to the sorting of CLTVs after the initial
signing process.
This commit fixes a recent issue from #4081 that would prevent a frozen
channel from being force closed via the rpc. We correct this, so that
only the co-op path is inhibited.
The linter complains about not checking the return value from
WipeChannel in certain places. Instead of checking we simply remove the
returned error because the in-memory modifications cannot fail.
In this commit, we add some additional logging of the commitments at
play when we detect a channel closure on-chain. This should help to
debug things more in the future as we don't log the full commitments
anywhere. We also now also print the type of commitment as well, as a
follow up from the recent anchor outputs work. In the near future, as we
implement a dynamic commitments update protocol, always logging the
commitment type as well will likely be useful for debugging purposes.
We whitelist a set of "expected" errors that can be returned from
RequestRoute, by converting them into a new type noRouteError. For any
other error returned by RequestRoute, we'll now exit immediately.
testSendToRouteMultiPath tests that we are able to successfully route a
payment using multiple shards across different paths, by using SendToRoute.
Co-authored-by: Joost Jager <joost.jager@gmail.com>
We add validation making sure we are not trying to register MPP shards
for non-MPP payments, and vice versa. We also add validtion of total
sent amount against payment value, and matching MPP options.
We also add methods for copying Route/Hop, since it is useful to use
for modifying the route amount in the test.
This commit enables MPP sends for SendToRoute, by allowing launching
another payment attempt if the hash is already registered with the
ControlTower.
We also set the total payment amount of of the payment from mpp record,
to indicate that the shard value might be different from the total
payment value.
We only mark non-MPP payments as failed in the database after
encountering a failure, since we might want to try more shards for MPP.
For now this means that MPP sendToRoute payments will be failed
only after a restart has happened.
This commit finally enables MP payments within the payment lifecycle
(used for SendPayment). This is done by letting the loop launch shards
as long as there is value remaining to send, inspecting the outcomes for
the sent shards when the full payment amount has been filled.
The method channeldb.MPPayment.SentAmt() is added to easily look up how
much value we have sent for the payment.
(almost) PURE CODE MOVE
The only code change is to change a few select cases from
case _ <- channel:
to
case <- channel:
to please the linter.
The test is testing the payment lifecycle, so move it to
payment_lifecycle_test.go
This commit redefines how the control tower handles shard and payment
level settles and failures. We now consider the payment in flight as
long it has active shards, or it has no active shards but has not
reached a terminal condition (settle of one of the shards, or a payment
level failure has been encountered).
We also make it possible to settle/fail shards regardless of the payment
level status (since we must allow late shards recording their status
even though we have already settled/failed the payment).
Finally, we make it possible to Fail the payment when it is already
failed. This is to allow multiple concurrent shards that reach terminal
errors to mark the payment failed, without havinng to synchronize.
We'll only do so when we have at least one confirmed advertised channel
with the remote peer. This ensures that our node announcement can
propagate throughout the network even for nodes that were offline when
we initially broadcast it.
In preparation for MPP we return the terminal errors recorded with the
control tower. The reason is that we cannot return immediately when a
shard fails for MPP, since there might be more shards in flight that we
must wait for. For that reason we instead mark the payment failed in the
control tower, then return this error when we inspect the payment,
seeing it has been failed and there are no shards in flight.
To move towards how we will handle existing attempt in case of MPP
(collecting their outcome will be done in separate goroutines separate
from the payment loop), we move to collect their outcome first.
To easily fetch HTLCs that are still not resolved, we add the utility
method InFlightHTLCs to channeldb.MPPayment.
Now that SendToRoute is no longer using the payment lifecycle, we move
the max hop check out of the payment shard's launch() method, and return
the error directly, such that it can be handled in SendToRoute.
Now that SendToRoute is no longer using the payment lifecycle, we
remove the error structs and vars used to cache the last encountered
error. For SendToRoute this will now be returned directly after a shard
has failed.
For SendPayment this means that the last error encountered durinng
pathfinding no longer will be returned. All errors encounterd can
instead be inspected from the HTLC list.
Instead of having SendToRoute pull routes from the payment session in
the payment lifecycle, we utilize the new methods on the paymentShard to
launch and collect the result for this single route.
This also let us remove the check for noRouteError, as we will always
have the result from the tried attempt returned. A result of this is
that we can finally remove lastError from the payment lifecycle (see
next commits).
Fetching the final shard result will also be done for calls to
SendToRoute, so we extract this code into a new method.
We move the call to the ControlTower to set the payment level failure
out into the payment loop, as this must be handled differently when
multiple shards are in flight, and for SendToRoute.
Define shardHandler which is a struct holding what is needed to send
attempts along given routes. The reason we define the logic on this
struct instead of the paymentLifecycle is that we later will make
SendToRoute calls not go through the payment lifecycle, but only using
this struct.
The launch shard is responsible for registering the attempt with the
control tower, failing it if the launch fails. Note that it is NOT
responsible for marking the _payment_ failed in case a terminal error is
encountered. This is important since we will later reuse this method for
SendToRoute, where whether to fail the payment cannot be decided on the
shard level.
We replace the cached attempt, and instead use the control tower
(database) to fetch any in-flight attempt. This is done as a
preparation for having multiple attempts in flight.
In addition we remove the cached circuit, as it won't be applicable when
multiple shards are in flight.
Instead of tracking the attemp we consult the database on every
iteration, and pick up any existing attempt. This also let us avoid
having to pass in the existing attempts from the payment loop, as we
just fetch them direclty.
This method is used to fetch a payment and all HTLC attempt that have
been made for that payment. It will both be used to resume inflight
attempts, and to fetch the final outcome of previous attempts.
We also update the the mock control tower to mimic the real control
tower, by letting it track multiple HTLC attempts for a given payment
hash, laying the groundwork for later enabling it for MPP.
The test case's preimage was (mistakenly) overwritten after crafting the
lightning payment, causing the parts of the testcases use the same
preimage causing problems when we are using the payment hash and
preimage in the mock control tower to distinguish paymennts.
In our quest to move calls to the ControlTower into the main payment
lifecycle loop, we move the edge case of a too long route out of
createNewPaymentAttempt.
loop
To prepare for multiple in flight payment attempts, we move
checkpointing the payment attempt out of createNewPaymentAttempt and
into the main payment lifecycle loop.
We'll attempt to move all calls to the DB via the ControlTower into this
loop, so we can more easily handle them in sequence.
active shards
In preparation for doing pathfinding for routes sending a value less
than the total payment amount, we let the payment session take the max
amount to send and the fee limit as arguments to RequestRoute.
This commit moves supplying of the information in the LightningPayment
to the initialization of the paymentSession, away from every call to
RequestRoute.
Instead the paymentSession will store this information internally, as it
doesn't change between payment attempts.
This is done to rid the RequestRoute call of the LightingPayment
argument, as for SendToRoute calls, it is not needed to supply the next
route.
