Skip to content

Update static invoice store for invoice requests - #635

Merged
tnull merged 2 commits into
lightningdevkit:developfrom
joostjager:always-online-fwd-inv-req
Sep 18, 2025
Merged

Update static invoice store for invoice requests #635
tnull merged 2 commits into
lightningdevkit:developfrom
joostjager:always-online-fwd-inv-req

Conversation

@joostjager

@joostjagerjoostjager commented Sep 16, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

With the merge of ttps://github.com/lightningdevkit/rust-lightning/pull/4049, it is now possible for a static invoice server to forward the invoice request to the recipient if they are online.

@ldk-reviews-bot

ldk-reviews-bot commented Sep 16, 2025•
edited
Loading

Copy link
Copy Markdown

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull
tnull reviewed Sep 16, 2025
View reviewed changes

@tnulltnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This has the static_invoice_server test failing

@joostjager

joostjager commented Sep 16, 2025

Copy link
Copy Markdown
ContributorAuthor

This has the static_invoice_server test failing

I noticed that on the other PR too. Wasn't sure if it was a general flake or a specific flake. I slow do get the feeling it might be specific.

@joostjager

joostjager commented Sep 16, 2025

Copy link
Copy Markdown
ContributorAuthor

Could repro locally after all. I think this may be caused by non-determinism introduced in lightningdevkit/rust-lightning#4049 where potentially two StaticInvoice onion messages may arrive at the sender in some order.

The flake disappears with lightningdevkit/rust-lightning#4046.

I am going to take the easy way and won't try to debug this intermediate state of the ldk code. Closing this PR and moving the commit to #632

@tnull

tnull commented Sep 17, 2025

Copy link
Copy Markdown
Collaborator

I am going to take the easy way and won't try to debug this intermediate state of the ldk code. Closing this PR and moving the commit to #632

Hmm, it would be appreciated to at least somehow address the API changes on LDK main though. To get #633 to build I have to bump the tracked commit and that would include the static invoice store related API changes.

@joostjagerjoostjager reopened this Sep 17, 2025
@joostjager
joostjagerforce-pushed the always-online-fwd-inv-req branch from 1a2335f to 54adf2fCompareSeptember 17, 2025 12:42
@joostjagerjoostjager mentioned this pull request Sep 17, 2025
Merged
@joostjager
Update static invoice store for invoice requests
8d18d16
With the merge of
lightningdevkit/rust-lightning#4049,
it is now possible for a static invoice server to forward the invoice
request to the recipient if they are online.
@joostjager
joostjagerforce-pushed the always-online-fwd-inv-req branch from 54adf2f to 2a912d2CompareSeptember 18, 2025 08:49
@joostjager

joostjager commented Sep 18, 2025

Copy link
Copy Markdown
ContributorAuthor

LDK issue fixed in lightningdevkit/rust-lightning#4078

tnull
tnull reviewed Sep 18, 2025
View reviewed changes

@tnulltnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM mod one question.

Comment threadtests/integration_tests_rust.rs
@joostjager
Update static invoice test to use unannounced channels
efbef4c
To better align with the expected real life setup.
@joostjager
joostjagerforce-pushed the always-online-fwd-inv-req branch from 2a912d2 to efbef4cCompareSeptember 18, 2025 08:59
tnull
tnull approved these changes Sep 18, 2025
View reviewed changes

@tnulltnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACK

@tnull
tnull merged commit 0f1d66c into lightningdevkit:developSep 18, 2025
14 of 15 checks passed
@joostjagerjoostjager mentioned this pull request Nov 19, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tnulltnulltnull approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@joostjager@ldk-reviews-bot@tnull