This commit removes the overflowQueue from the link. We do so in order
to promote better UX for senders, so that HTLCs are failed faster when
the commitment is full. This gives the sender the opportunity to try
another, more open path, rather than perceive the HTLC as being stuck.
At the same time, we remove the total number of active goroutines in lnd
by a factor of N where N is the number of active channels.
This commit is preparation for the test added in the subsequent commit.
We modify makeHoldPayment to return any failures direectly when trying
to add an HTLC to the switch. This lets us know that the HTLC was indeed
sent without failure when the method returns.
A following commits will move/modify callsites of AckPacket, Start, and
Stop, none of which use the return value and ultimately cause the linter
to complain. However, none of these in-memory operations can fail so we
just remove the returned errors altogether.
This is mainly motivated by a now fixed bug in the wallet in which
change addresses could at times be created outside of the default key
scopes. Recovery only used to be performed on the default key scopes, so
ideally this test case would've caught the bug earlier.
Update the PendingChannel message from a bool to an enum to
differentiate between the case where the remote party initiated the
channel and we have no record of the channel initiator. The previous
field has not been included in a release of lnd, so we can replace the
field directly.
Move enum out of CloseSummary struct for more general use. This does
not change the encoding of the enum, and will only cause compile time
errors for existing clients. This enum has not been included in a
release yet, so we can make this move without much disruption.
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