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

Implement immediate flag for waitanyinvoice. #3438

Closed

Conversation

ZmnSCPxj
Copy link
Collaborator

@ZmnSCPxj ZmnSCPxj commented Jan 24, 2020

An alternative to #3449. Closes: #3449

This adds the immediate flag, equivalent to timeout=0 on #3449

Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Very nice 👍

One thing that comes to mind is that most APIs that have a waitsomething call will have a timeout parameter, and sometimes they'll have a blocking=False argument (which is what your immediate flag compares to). I kind of like the timeout variant since it allows to semi-block a thread and do other work if the timeout expires.

Just as a discussion topic, would a timeout parameter, that may be 0 and return immediately, be a more flexible way to implement this? The downside obviously is that it'd require a timer and a timeout callback similar to the success callback.

contrib/pyln-client/pyln/client/lightning.py Outdated Show resolved Hide resolved
@ZmnSCPxj
Copy link
Collaborator Author

Just as a discussion topic, would a timeout parameter, that may be 0 and return immediately, be a more flexible way to implement this? The downside obviously is that it'd require a timer and a timeout callback similar to the success callback.

I considered this as well --- wallet/invoices.c can certainly handle having the callback context tal-deleted under it --- but then I thought what would be the point of using anything other than 0 or UINT32_MAX for that.

Or maybe it is just better to have a consistent interface, and a good number of our commands do have timeout.

… if no invoice available.

Fixes: ElementsProject#3192

Changelog-Added: `waitanyinvoice` now supports an `immediate` flag, which when set will cause the command to fail immediately instead of blocking when no paid invoice is available.
@ZmnSCPxj
Copy link
Collaborator Author

Changed to use a timeout instead of threading an immediate parameter through to wallet/invoices.c The wallet/invoices.c sub-module is perfectly capable of having its callback cancelled, so this is perfectly fine and thus immediate need not be passed in to the invoices sub-module.

@ZmnSCPxj ZmnSCPxj deleted the waitanyinvoice-immediate branch January 29, 2020 02:54
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.

2 participants