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(connectors): lndhub incoming tx order #1625 #1626

Merged
merged 2 commits into from
Oct 17, 2022

Conversation

escapedcat
Copy link
Contributor

@escapedcat escapedcat commented Oct 17, 2022

Describe the changes you have made in this PR

Add reverse to incoming payments to correct wrong order

Link this PR to an issue [optional]

Fixes #1625

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

How has this been tested?

Manually with a bluewallet account

@github-actions
Copy link

github-actions bot commented Oct 17, 2022

🚀 Thanks for the pull request!

Here are the current build files for testing:

Download and unzip the file for your browser. Refer to the readme for detailed install instructions.


This build is brought to you by: Adam Fiscor (who recently dropped 1337 sats):

The future is bright. We just have a lot of work to do.

Want to sponsor the next build? send some sats to ⚡️[email protected] (don't forget to provide your name)

Don't forget: keep stacking sats!

@escapedcat escapedcat marked this pull request as ready for review October 17, 2022 15:42
@escapedcat escapedcat requested review from bumi and reneaaron October 17, 2022 15:42
@bumi
Copy link
Collaborator

bumi commented Oct 17, 2022

I actually think that this breaks other lndhub implementations like the Alby accounts.
those return the invoices in the useful order. Did you test that?

instead of doing the reverse it should be probably ordered by settleDate.

@escapedcat escapedcat force-pushed the fix/1625_lndhub-incoming-order branch from 17b897c to 1fa0b3e Compare October 17, 2022 15:57
@escapedcat
Copy link
Contributor Author

Thanks, yes! Fixed it. Also switched it for LND.

@bumi
Copy link
Collaborator

bumi commented Oct 17, 2022

lnd is at least reliable, because it is specifically that LND. and they have an parameter that we use there for the sorting.

@escapedcat
Copy link
Contributor Author

lnd is at least reliable, because it is specifically that LND. and they have an parameter that we use there for the sorting.

Yeah but I think that's just for the "whole" list in terms of "last page". We still need to "reverse" that last page. afaik that's why we added the reverse there in the first place.

@bumi bumi merged commit 0c7f9ea into master Oct 17, 2022
@bumi bumi deleted the fix/1625_lndhub-incoming-order branch October 17, 2022 19:59
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.

LNDhub connection - Incoming sorting bug (oldest-newest)
2 participants