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

Fix automated retries retrying too much #2037

Merged
merged 1 commit into from
Mar 28, 2025
Merged

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Mar 28, 2025

Description

We are automatically retrying things that we would not have shown in notifications because this filter is applied to failed invoices for notifications but was missing for failed invoices for automated retries.

#2036 is related because the error happened when the receiver was trying to pay themselves (?)

Additional context

  1. Maybe there's a solution that involves the bigger picture (keeping failed invoices for automated retries vs notifications in sync), but I guess this fix will do for now.

  2. The way we set userCancel is also an issue: we only ever set it to true if the user clicks on X for a QR code to close it. In every other case, it is set to false which means we will automatically retry the invoice. However, there are cases where a user did not click on X but still might want to NOT retry the invoice, like if they simply navigated away or closed the tab. We should NOT assume they want to retry.

  3. RECEIVE sets userId of the wrapped invoice (= invoice shown to the sender) to the receiver so the receiver will retry the wrapped invoice to themselves, paying the 10% proxy fee for nothing. Setting the userId of the sender's invoice isn't necessarily a bug, but that the receiver would automatically retry a wrapped invoice that will result in them paying themselves is DEFINITELY a bug.

Checklist

Are your changes backwards compatible? Please answer below:

yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

0.

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

@ekzyis ekzyis requested a review from huumn March 28, 2025 00:48
@huumn huumn merged commit a2faa31 into master Mar 28, 2025
6 checks passed
@huumn huumn deleted the fix-automated-retries branch March 28, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants