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): lnbits incoming payments shows outgoing ones as well #1635

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

escapedcat
Copy link
Contributor

Describe the changes you have made in this PR

lnbits payments api shows outgoing and incoming payments. Negative ones are outgoing and need to be filtered out for our incoming txs.

Link this PR to an issue [optional]

Fixes #1631

Type of change

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

Screenshots of the changes [optional]

image

How has this been tested?

Manually by sendign sats back and forth

@github-actions
Copy link

🚀 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 19, 2022 09:24
@bumi
Copy link
Collaborator

bumi commented Oct 19, 2022

did you test it (potentially with different lnbits installations)?
is the ordering correct (maybe also add the sorting there?)?

@escapedcat
Copy link
Contributor Author

did you test it (potentially with different lnbits installations)?

Isn't the API behaving the same among lnbits installations? Where else could I test it?

is the ordering correct (maybe also add the sorting there?)?

Sorting looks good, no? See screenshot above.

@bumi
Copy link
Collaborator

bumi commented Oct 19, 2022

if you tested it and everything works, then merge it.
I'd still add the sorting (which you added also for lnd)?

@escapedcat
Copy link
Contributor Author

if you tested it and everything works, then merge it.

Tested with our lnbits account form the e2e tests.

I'd still add the sorting (which you added also for lnd)?

The sort takes care of an API flaw which doesn't exist in the lnbits API. So I would not add it.

@escapedcat escapedcat merged commit 2c11348 into master Oct 19, 2022
@escapedcat escapedcat deleted the fix/1631_lnbits-incoming-payments branch October 19, 2022 17:22
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.

Sent transaction appear on received tab as +-
3 participants