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

Use received amount to account for overpayment #1038

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

dangeross
Copy link
Collaborator

@dangeross dangeross commented Jul 10, 2024

Changes Payment mapping to use the received amount instead of invoice amount to account for overpayments.

Example when sending in Phoenix 1500 instead of 1000 msats:

Received payment: amount_received_msat: Some(Amount { msat: 1500 }) amount_msat: Some(Amount { msat: 1000 })

Fixes #1026

Copy link
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

LGTM

@JssDWt
Copy link
Contributor

JssDWt commented Jul 11, 2024

Looks good.

What about payments that were already inserted with the amount_msat amount? Should we update those somehow?

@dangeross
Copy link
Collaborator Author

What about payments that were already inserted with the amount_msat amount? Should we update those somehow?

@JssDWt I think if drop the last_backup_time cached_item in the migration, then we would re-sync all the payments (paid invoices) from Greenlight. What do you think @roeierez?

@dangeross dangeross marked this pull request as ready for review July 11, 2024 13:01
@JssDWt
Copy link
Contributor

JssDWt commented Jul 11, 2024

I think if drop the last_backup_time cached_item in the migration, then we would re-sync all the payments (paid invoices) from Greenlight

I think it's the last_sync_time we would have to drop there.

@dangeross
Copy link
Collaborator Author

I think it's the last_sync_time we would have to drop there.

Oops, yeah

@dangeross dangeross requested a review from roeierez July 12, 2024 06:00
Copy link
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@JssDWt JssDWt left a comment

Choose a reason for hiding this comment

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

LGTM

@dangeross dangeross merged commit d193902 into main Jul 12, 2024
8 of 9 checks passed
@dangeross dangeross deleted the savage-account-for-overpayment branch July 12, 2024 15:47
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.

Bug: Incorrect payment amount_msat for Bolt11 overpayments
3 participants