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

feat: replace invoices by bolt11 #830

Merged
merged 3 commits into from
Apr 30, 2022
Merged

feat: replace invoices by bolt11 #830

merged 3 commits into from
Apr 30, 2022

Conversation

dylancom
Copy link
Contributor

@dylancom dylancom commented Apr 26, 2022

Link this PR to an issue

Fixes #809

Type of change (Remove other not matching type)

  • New feature (non-breaking change which adds functionality)

Describe the changes you have made in this PR -)

Replaced the invoices lib by bolt11

PS: needs to be tested thoroughly

@github-actions
Copy link

Build files:

@dylancom dylancom marked this pull request as ready for review April 26, 2022 17:53
Copy link
Collaborator

@bumi bumi left a comment

Choose a reason for hiding this comment

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

utACK

description={invoiceRef.current?.description}
amount={invoiceRef.current?.satoshis}
description={
invoiceRef.current?.tags.find(
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is what I did not like about the bolt11 package. There is no easy way to get the description (which probably most will need)

maybe we should make a PR to get a proper accessor for that? invoiceRef.current.description ? This should do the lookup then in the bolt11 package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, this feels rather cumbersome. Let's try a PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a PR: bitcoinjs/bolt11#60

@dylancom
Copy link
Contributor Author

bolt11 decode got updated :)

Copy link
Contributor

@escapedcat escapedcat left a comment

Choose a reason for hiding this comment

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

🚀

@bumi bumi merged commit 7c18bab into master Apr 30, 2022
@bumi bumi deleted the feature/replace-invoices branch April 30, 2022 16:51
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.

Replace npm package to parse lightning invoices
3 participants