Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

branch: merge 0.18 staging branch into master #8067

Merged
merged 75 commits into from
Oct 9, 2023
Merged

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Oct 6, 2023

Take 2, supersedes: #8059

Branch has been rebased over master to handle conflicts that arose.

saubyk and others added 30 commits October 6, 2023 16:34
We also update the legacy code to return an error when the payment is
not found.
This commit changes `fetchPaymentStatus` to return an error when the
payment creation info bucket cannot be found. Instead of using the
ambiguous status `StatusUnknown`, we tell the callsite explictly that
there's no such payment. This also means the vague `StatusUnknown` can
now be removed as it has multiple meanings.
This commit adds a new payment status, `StatusInitiated`, to properly
represent the state where a payment is newly created without attempting
any HTLCs. Since the `PaymentStatus` is a memory representation of a
given payment's status, the enum used for the statuses are directly
updated.
This commit adds a new method, `initializable`, to help decide whether
initiating a payment is allowed.
This commit adds a new method, `removable`, to help decide whether
deleting a payment is allowed.
This commit adds a new method, `updatable`, to help decide whether
updating a given payment is allowed.
Renamed `p` to `payment` to since it's used by the method's pointer receiver.
This commit adds a new method, `Registrable`, to help decide whether
adding new HTLCs to a given payment is allowed. A new unit test,
`TestRegistrable` is also added to test it.
This commit introduces more granular statuses to better determine a
payment's current state. Based on whether there are inflight HTLCs, the
state of each of the HTLCs, and whether the payment is failed, a total
of 5 states are derived, which can give a finer control over what action
to take based on a given state.

Also, `fetchPayment` now uses `decidePaymentStatus`. After applying the
new function, the returned payment status would be different,

```
| inflight | settled | failed | reason |       previous  ->   now          |
|:--------:|:-------:|:------:|:------:|:---------------------------------:|
|   true   |   true  |  true  |   yes  |    StatusInFlight(unchanged)      |
|   true   |   true  |  true  |   no   |    StatusInFlight(unchanged)      |
|   true   |   true  |  false |   yes  |    StatusInFlight(unchanged)      |
|   true   |   true  |  false |   no   |    StatusInFlight(unchanged)      |
|   true   |   false |  true  |   yes  |    StatusInFlight(unchanged)      |
|   true   |   false |  true  |   no   |    StatusInFlight(unchanged)      |
|   true   |   false |  false |   yes  |    StatusInFlight(unchanged)      |
|   true   |   false |  false |   no   |    StatusInFlight(unchanged)      |
|   false  |   true  |  true  |   yes  |    StatusSucceeded(unchanged)     |
|   false  |   true  |  true  |   no   |    StatusSucceeded(unchanged)     |
|   false  |   true  |  false |   yes  |    StatusSucceeded(unchanged)     |
|   false  |   true  |  false |   no   |    StatusSucceeded(unchanged)     |
|   false  |   false |  true  |   yes  |     StatusFailed(unchanged)       |
|   false  |   false |  true  |   no   |     StatusInFlight(unchanged)     |
|   false  |   false |  false |   yes  |     StatusFailed(unchanged)       |
|   false  |   false |  false |   no   |  StatusInFlight -> StatusInitiated|
```
…rminal state

This commit applies the new method `Terminated`. A side effect from
using this method is, we can now save one query `fetchPayment` inside
`FetchInFlightPayments`.
This commit removes the `Litecoin`, `LtcMode` and `LitecoindMode`
members from the main LND `Config` struct. Since only the bitcoin
blockchain is now supported, this commit also deprecates the
`cfg.Bitcoin.Active` config option.
This commit removes the Litecoin, LtcMode and LitcoinMode members from
the chainreg `Config` struct.
yyforyongyu and others added 22 commits October 6, 2023 16:38
This commit moves the struct `paymentState` used in `routing` into
`channeldb` and replaces it with `MPPaymentState`. In the following
commit we'd see the benefit, that we don't need to pass variables back
and forth between the two packages. More importantly, this state is put
closer to its origin, and is strictly updated whenever a payment is read
from disk. This approach is less error-prone comparing to the previous
one, which both the `payment` and `paymentState` need to be updated at
the same time to make sure the data stay consistant in a parallel
environment.
This commit adds a new method, `NeedWaitAttempts`, to properly decide
whether we need to wait for the outcome of htlc attempts based on the
payment's current state.
This commit turns `MPPayment` into an interface inside `routing`. Having
this interface gives us the benefit to write more granular unit tests
inside payment lifecycle. As seen from the modified unit tests, several
hacky ways of testing the `SendPayment` method is now replaced by a mock
over `MPPayment`.
This commit adds a new method to properly init a payment lifecycle so we
can easily see the default values used here.
This commit refactors the params used in lifecycle to prefer
`HTLCAttempt` over `HTLCAttemptInfo`. This change is needed as
`HTLCAttempt` also wraps settled and failure info, which is useful in
the following commits.
This commit adds a representation of blinded payments, which include a
blinded path and aggregate routing parameters to be used in payment to
the path.
This commit adds the encrypted_data, blinding_point and total_amt_msat
tlvs to the known set of even tlvs for the onion payload. These TLVs
are added in two places (the onion payload and hop struct) because
lnd uses the same set of TLV types for both structs (and they
inherently represent the same thing).

Note: in some places, unit tests intentionally mimic the style
of older tests, so as to be more consistently readable.
With the addition of blinded routes, we now need to account for the
possibility that intermediate nodes payloads will not have an amount
and expiry set because that information is provided by the recipient
encrypted data blob. This commit updates our payload packing to only
optionally include those fields.
When we introduce blinded routes, some of our hops are expected
to have zero amounts to forward in their hop payload. This commit
updates our hop fee logic to attribute the full blinded route's
fees to the introduction node. We can't actually know where/how
these fees are distributed, so we collect them all at the
introduction node.
This commit introduces a single struct to hold all of the parameters
that are passed to FindRoute. This cleans up an already overloaded
function signature and prepares us for handling requests with blinded
routes where we need to perform some additional processing on our
para (such as extracting the target node from the blinded path).
Add the option to include a blinded route in a route request (exclusive
to including hop hints, because it's incongruous to include both), and
express the route as a chain of hop hints.

Using a chain of hints over a single hint to represent the whole path
allows us to re-use our route construction to fill in a lot of the
path on our behalf.
This commit updates route construction to backfill the fields
required for payment to blinded paths and set amount to forward
and expiry fields to zero for intermediate hops (as is instructed
in the route blinding specification).

We could attempt to do this in the first pass, but that loop
relies on fields like amount to forward and expiry to calculate
each hop backwards, so we keep it simple (stupid) and post
processes the blinded portion, since it's computationally cheap
and more readable.
Blinded routes can now have "hints" that have zero value edges, so we
remove this log to avoid spamming logs.
Note: This commit can be dropped before merge, it's mostly added
to make the PR easier to manually test against other
implementations that have bolt 12 invoices implemented already!
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments
guggero
🥇
13
▀▀▀
1d 18h 38m
14
▀▀▀
yyforyongyu
🥈
12
▀▀▀
17h 52m
15
▀▀▀
Roasbeef
🥉
4
10h 36m
6
bhandras
3
9h 34m
5
Crypt-iQ
3
1d 2h 57m
1
saubyk
3
1d 5h 48m
0
ProofOfKeags
2
2d 17h 2m
10
▀▀
alexbosworth
1
1d 47m
1
dstadulis
1
4h 11m
0
morehouse
1
53m
0
ellemouton
1
15h 56m
1
GeorgeTsagk
1
1d 2h 25m
0
bitromortac
1
8d 13h 39m
▀▀▀▀
1

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Individual commits don't compile, but I think that's okay in this case. LGTM 🎉

Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Roasbeef Roasbeef merged commit 4f34606 into master Oct 9, 2023
23 of 27 checks passed
@guggero guggero deleted the 0-18-staging-rebased branch October 9, 2023 19:59
@guggero guggero restored the 0-18-staging-rebased branch October 9, 2023 20:00
@guggero guggero deleted the 0-18-staging-rebased branch October 9, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants