-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Part 1|3] Introduce SQL Payment schema into LND #9147
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
[Part 1|3] Introduce SQL Payment schema into LND #9147
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Some questions I already had:
I think this is ok and we just include an interface for the |
As in a composite foreign key? That should be fine, though if there's a way we can avoid it in practice to simplify queries or instead add a |
|
@ziggie1984 I think the image upload on that last comment failed. |
|
I used Claude to create an mermaid diagram of the schema so far (zero shot, and first pass, might have some errors). This is nice, as we can check it into the repo, and the markdown updates will be shown in the git diff: erDiagram
mpp_payment ||--o{ first_hop_custom_records : has
mpp_payment ||--|| payment_status_types : has
mpp_payment ||--|| mpp_state : has
mpp_payment ||--o{ htlc_attempt : has
htlc_attempt ||--o| htlc_settle_info : has
htlc_attempt ||--o| htlc_fail_info : has
htlc_fail_info ||--|| htlc_fail_reason_types : has
htlc_fail_info ||--o| failure_msg_types : has
htlc_attempt ||--o| route : has
route ||--o{ hop : contains
hop ||--o| blinded_data : has
hop ||--o| mpp_record : has
hop ||--o| amp_record : has
hop ||--o{ hop_custom_records : has
mpp_payment {
integer id PK
integer payment_status FK
blob payment_hash
bigint amount_msat
blob payment_request
timestamp created_at
}
first_hop_custom_records {
bigint key FK
blob value
text payment_id
}
payment_status_types {
integer id PK
text description
}
mpp_state {
integer payment_id FK
integer num_attempts_in_flight
bigint remaining_amt
bigint FeesPaid
boolean has_settled_htlc
boolean payment_failed
}
htlc_attempt {
integer id PK,FK
blob session_key
timestamp attempt_time
integer payment_id FK
}
htlc_settle_info {
integer id PK
integer attempt_id FK
blob preimage
timestamp settle_time
}
htlc_fail_info {
integer id PK
integer attempt_id FK
integer failure_source_index
integer htlc_fail_reason FK
integer failure_msg FK
}
htlc_fail_reason_types {
integer id PK
text description
}
failure_msg_types {
integer id PK
text description
text error_msg
}
route {
integer id PK
integer htlc_attempt_id FK
integer total_timeLock
bigint total_amount
blob source_key
bigint first_hop_amount
}
hop {
integer id PK
integer route_id FK
blob pub_key
text chan_id
integer outgoing_time_lock
bigint amt_to_forward
blob meta_data
}
blinded_data {
integer hop_id FK
blob encrypted_data
blob blinding_point
bigint total_amt
}
mpp_record {
integer hop_id FK
blob payment_addr
bigint total_msat
}
amp_record {
integer hop_id FK
blob root_share
blob set_id
integer child_index
}
hop_custom_records {
bigint key FK
blob value
integer hop_id FK
}
|
| amount_msat BIGINT NOT NULL, | ||
|
|
||
| -- payment_request is the payment request of the payment. | ||
| payment_request BLOB UNIQUE NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should split this out, as in the future we'll have more than one type of payment request encoding. I think it's also possible to actually have duplicates here: someone pays a zero value invoice multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be considered, current version does not separate it out.
| CREATE TABLE legacy_payments ( | ||
| payment_id INTEGER PRIMARY KEY REFERENCES payments(id) ON DELETE CASCADE, | ||
|
|
||
| payment_hash BLOB UNIQUE NOT NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there were instances of duplicate payment hashes in the past?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this will be handled separately to keep the unique constraint requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes duplicate payments are handled separately in a followup PR.
|
|
||
| value BLOB NOT NULL, | ||
|
|
||
| payment_id INTEGER REFERENCES payments(id) ON DELETE CASCADE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we could further denormalize this with another layer of indirection. So we store a table of TLV KV pairs, then an associative table that relates a payment_id to an entry in the TLV KV table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm can be considered current version creates a TLV table for all the related parts separately
| CREATE INDEX IF NOT EXISTS amp_record_set_id_idx ON amp_record(set_id); | ||
|
|
||
| CREATE TABLE "hop_custom_records" ( | ||
| key BIGINT NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah so if we go with the suggestion above, then this would become another associative table (dedup the KV pairs, then one table to map a given hop_id to the set of KV pairs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's see what the other reviewers think.
|
Updated version: erDiagram
payments {
BIGINT id PK
INTEGER payment_status FK
INTEGER payment_type FK
BIGINT amount_msat
BLOB payment_request
TIMESTAMP created_at
}
payment_status_types {
INTEGER id PK
TEXT description
}
payment_types {
INTEGER id PK
TEXT description
}
legacy_payments {
INTEGER payment_id PK,FK
BLOB payment_hash
}
mpp_payments {
INTEGER payment_id PK,FK
INTEGER mpp_record_id FK
}
amp_payments {
INTEGER payment_id PK,FK
INTEGER amp_record_id FK
}
mpp_state {
INTEGER payment_id PK,FK
INTEGER num_attempts_in_flight
BIGINT remaining_amt
BIGINT FeesPaid
BOOLEAN has_settled_htlc
BOOLEAN payment_failed
}
first_hop_custom_records {
BIGINT key
BLOB value
INTEGER payment_id FK
}
htlc_attempt {
INTEGER id PK
BLOB session_key
TIMESTAMP attempt_time
INTEGER payment_id FK
}
route {
INTEGER htlc_attempt_id PK,FK
INTEGER total_timeLock
BIGINT total_amount
BLOB source_key
BIGINT first_hop_amount
}
hop {
INTEGER id PK
INTEGER route_id FK
BLOB pub_key
TEXT chan_id
INTEGER outgoing_time_lock
BIGINT amt_to_forward
BLOB meta_data
}
mpp_record {
INTEGER hop_id PK,FK
BLOB payment_addr
BIGINT total_msat
}
amp_record {
INTEGER hop_id PK,FK
BLOB root_share
BLOB set_id
INTEGER child_index
}
hop_custom_records {
BIGINT key
BLOB value
BIGINT hop_id FK
}
blinded_data {
INTEGER hop_id PK,FK
BLOB encrypted_data
BLOB blinding_point
BIGINT total_amt
}
htlc_settle_info {
INTEGER htlc_attempt_id PK,FK
BLOB preimage
TIMESTAMP settle_time
}
htlc_fail_info {
INTEGER htlc_attempt_id PK,FK
INTEGER htlc_fail_reason FK
TEXT failure_msg
}
htlc_fail_reason_types {
INTEGER id PK
TEXT description
}
payments ||--o{ legacy_payments : has
payments ||--o{ mpp_payments : has
payments ||--o{ amp_payments : has
payments ||--o{ mpp_state : has
payments ||--o{ first_hop_custom_records : has
payments ||--o{ htlc_attempt : has
payments }|--|| payment_status_types : references
payments }|--|| payment_types : references
mpp_payments }|--|| mpp_record : references
amp_payments }|--|| amp_record : references
htlc_attempt ||--o| route : has
route ||--o{ hop : contains
hop ||--o| mpp_record : has
hop ||--o| amp_record : has
hop ||--o{ hop_custom_records : has
hop ||--o| blinded_data : has
htlc_attempt ||--o| htlc_settle_info : has
htlc_attempt ||--o| htlc_fail_info : has
htlc_fail_info }|--|| htlc_fail_reason_types : references
|
|
Updated graph: erDiagram
payments {
BIGINT id PK
INTEGER payment_status FK
INTEGER payment_type FK
BIGINT amount_msat
TIMESTAMP created_at
}
payment_requests {
INTEGER id PK
INTEGER payment_id FK
BLOB payment_request
}
payment_status_types {
INTEGER id PK
TEXT description
}
payment_types {
INTEGER id PK
TEXT description
}
legacy_payments {
INTEGER payment_id PK,FK
BLOB payment_hash
}
mpp_payments {
INTEGER payment_id PK,FK
INTEGER mpp_record_id FK
}
amp_payments {
INTEGER payment_id PK,FK
INTEGER amp_record_id FK
}
payment_state {
INTEGER payment_id PK,FK
INTEGER num_attempts_in_flight
BIGINT remaining_amt
BIGINT fees_paid
BOOLEAN has_settled_htlc
BOOLEAN payment_failed
}
htlc_attempt {
INTEGER id PK
BLOB session_key
TIMESTAMP attempt_time
INTEGER payment_id FK
}
route {
INTEGER htlc_attempt_id PK,FK
INTEGER total_timeLock
BIGINT total_amount
BLOB source_key
BIGINT first_hop_amount
}
hop {
INTEGER id PK
INTEGER route_id FK
BLOB pub_key
TEXT chan_id
INTEGER outgoing_time_lock
BIGINT amt_to_forward
BLOB meta_data
}
mpp_record {
INTEGER hop_id PK,FK
BLOB payment_addr
BIGINT total_msat
}
amp_record {
INTEGER hop_id PK,FK
BLOB root_share
BLOB set_id
INTEGER child_index
}
tlv_records {
INTEGER id PK
BIGINT key
BLOB value
}
custom_records {
INTEGER tlv_record_id PK,FK
INTEGER hop_id FK
}
first_hop_custom_records {
INTEGER tlv_record_id PK,FK
INTEGER payment_id FK
}
blinded_data {
INTEGER hop_id PK,FK
BLOB encrypted_data
BLOB blinding_point
BIGINT total_amt
}
htlc_settle_info {
INTEGER htlc_attempt_id PK,FK
BLOB preimage
TIMESTAMP settle_time
}
htlc_fail_info {
INTEGER htlc_attempt_id PK,FK
INTEGER htlc_fail_reason FK
TEXT failure_msg
}
htlc_fail_reason_types {
INTEGER id PK
TEXT description
}
payments ||--o{ payment_requests : has
payments ||--o| legacy_payments : has
payments ||--o| mpp_payments : has
payments ||--o| amp_payments : has
payments ||--o| payment_state : has
payments ||--o{ htlc_attempt : has
payments }o--|| payment_status_types : has
payments }o--|| payment_types : has
htlc_attempt ||--o| route : has
route ||--o{ hop : has
hop ||--o| mpp_record : has
hop ||--o| amp_record : has
hop ||--o{ custom_records : has
hop ||--o| blinded_data : has
htlc_attempt ||--o| htlc_settle_info : has
htlc_attempt ||--o| htlc_fail_info : has
htlc_fail_info }o--|| htlc_fail_reason_types : has
mpp_record ||--|| mpp_payments : references
amp_record ||--|| amp_payments : references
tlv_records ||--o{ custom_records : has
tlv_records ||--o{ first_hop_custom_records : has
payments ||--o{ first_hop_custom_records : has
|
616b851 to
9817290
Compare
|
Currently creating the whole change into the one draft PR and then if it all works together will split all the different parts into separate PRs. |
|
List of ideas to include during refactor (in progress):
|
|
fix the nil hash patch properly via SQL: #9703 (comment) |
|
New updated Diagram: |
|
I created a refined schema which is more optimzed: https://dbdiagram.io/d/Payment-Schema-66fbb44df9b1444815012417 |
7bf09e0 to
e9d0a4f
Compare
e9d0a4f to
3d9090b
Compare
|
OK so I broke the whole PR into pieces and will use a side branch instead to merge those PRs until the whole feature is ready. The tests which use the nativesql-experiment will fail currently because the backend is not implemented yet. Couple of words regarding the new schema and the design choices: Differences to the old design:
|
3d9090b to
7e3b327
Compare
7e3b327 to
ca3b10b
Compare
ellemouton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
|
@yyforyongyu: review reminder |
ca3b10b to
af52e5d
Compare
We prepare the code for the sql payment backend. However no payment db interface method for the sql backend is implemented yet. This will be done in the following commits. They currently use the embedded KVStore to satify the build environment.
28f9815 to
752af81
Compare
This does not include duplicate payments yet. They will be added when the migration code is introduced for payments.
We allow the migration of the payment schema to be applied when the test_native_sql is active to make sure the tables are properly contructed.
752af81 to
75b3a35
Compare
yyforyongyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still doing a final round review, and found this panic from the CI,
2025-10-13 07:07:15.700 [DBG] CRTR router.go:1419: Scanning for inflight payments
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x1bed372]
goroutine 23608 [running]:
github.com/lightningnetwork/lnd/kvdb.View(...)
/home/runner/go/pkg/mod/github.com/lightningnetwork/lnd/[email protected]/interface.go:27
github.com/lightningnetwork/lnd/payments/db.(*KVStore).FetchInFlightPayments(0xc0006437c0)
/home/runner/work/lnd/lnd/payments/db/kv_store.go:750 +0x172
github.com/lightningnetwork/lnd/routing.(*controlTower).FetchInFlightPayments(0x7f51ee812c28?)
/home/runner/work/lnd/lnd/routing/control_tower.go:287 +0x1b
github.com/lightningnetwork/lnd/routing.(*ChannelRouter).resumePayments(0xc001e7bd70)
/home/runner/work/lnd/lnd/routing/router.go:1420 +0x66
github.com/lightningnetwork/lnd/routing.(*ChannelRouter).Start(0xc001d69998?)
/home/runner/work/lnd/lnd/routing/router.go:359 +0x85
github.com/lightningnetwork/lnd.(*server).Start.func1()
/home/runner/work/lnd/lnd/server.go:2320 +0x1ec5
sync.(*Once).doSlow(0xc000651508?, 0xf?)
/opt/hostedtoolcache/go/1.24.6/x64/src/sync/once.go:78 +0xab
sync.(*Once).Do(...)
/opt/hostedtoolcache/go/1.24.6/x64/src/sync/once.go:69
github.com/lightningnetwork/lnd.(*server).Start(0xc000651508, {0x29be1b0, 0xc000123a90})
/home/runner/work/lnd/lnd/server.go:2170 +0xb6
github.com/lightningnetwork/lnd.Main.func12()
/home/runner/work/lnd/lnd/lnd.go:761 +0x28
created by github.com/lightningnetwork/lnd.Main in goroutine 1
/home/runner/work/lnd/lnd/lnd.go:760 +0x4a25
yes that's expected, we embed the kvstore for the sqlstore for now so that we can compile it, but for the sql tests they will fail because I never initialize this value because it would not work anyways if we partly implement each function because then parts will live in the kvstore and parts in the sql store, thats why for now these itests will fail for the test_native_sql run. |
yyforyongyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
a0044ba
into
lightningnetwork:elle-payment-sql-series-new

This implements the SQL backend for the payments subsystem of LND.
The following parts will focus on: