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.

Sign up for GitHub

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

Jump to bottom

xpay maxdelay parameter #7969

Merged
merged 2 commits into from
Jan 22, 2025
Merged

xpay maxdelay parameter #7969

merged 2 commits into from
Jan 22, 2025
+221 −97

Conversation

michael1011
Copy link
Contributor

@michael1011 michael1011 commented Dec 25, 2024 •
edited
Loading

I would like some feedback on my approach before I start writing tests and update the changelog

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@michael1011 michael1011 mentioned this pull request Dec 25, 2024
Merged
@rustyrussell
Copy link
Contributor

Why do we need this, though?

@michael1011
Copy link
Contributor Author

@rustyrussell you need maxdelay for use cases like servicing trampoline payments or submarine swaps (timeout of the lightning payment has to be shorter than the one onchain)

@rustyrussell
Copy link
Contributor

Right! Well, this patch looks like the correct thing then.

rustyrussell
rustyrussell approved these changes Dec 27, 2024
View reviewed changes
plugins/xpay/xpay.c Show resolved Hide resolved
doc/schemas/lightning-askrene-listlayers.json Show resolved Hide resolved
@michael1011 michael1011 force-pushed the xpay-maxdelay branch 2 times, most recently from 0168268 to a71097c Compare December 27, 2024 10:07
@michael1011 michael1011 marked this pull request as ready for review December 27, 2024 10:14
@michael1011 michael1011 requested a review from cdecker as a code owner December 27, 2024 10:14
@michael1011 michael1011 force-pushed the xpay-maxdelay branch 2 times, most recently from 3abf416 to 5ad5fbc Compare December 27, 2024 10:32
Lagrang3
Lagrang3 approved these changes Dec 28, 2024
View reviewed changes
Copy link
Collaborator

@Lagrang3 Lagrang3 left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@michael1011
xpay: add maxdelay parameter
e973815 
Changelog-Added: Plugins: `xpay` now supports a `maxdelay` parameter for better `xpay-handle-pay` compatibility.
@michael1011
xpay: fix description of description of bias
b6be7fa
@michael1011
Copy link
Contributor Author

Rebased against latest master

@ShahanaFarooqui ShahanaFarooqui added this to the v25.02 milestone Jan 22, 2025
@ShahanaFarooqui ShahanaFarooqui merged commit 54ee08b into ElementsProject:master Jan 22, 2025
39 checks passed
@ShahanaFarooqui
Copy link
Collaborator

ACK b6be7fa.

@michael1011 michael1011 deleted the xpay-maxdelay branch January 22, 2025 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lagrang3 Lagrang3 Lagrang3 approved these changes

@rustyrussell rustyrussell Awaiting requested review from rustyrussell

@cdecker cdecker Awaiting requested review from cdecker cdecker is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v25.02
Development

Successfully merging this pull request may close these issues.
4 participants
@michael1011 @rustyrussell @ShahanaFarooqui @Lagrang3