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: add lookup invoice to nwc webln provider #73

Merged
merged 5 commits into from
Aug 29, 2023

Conversation

rolznz
Copy link
Contributor

@rolznz rolznz commented Aug 21, 2023

this is not actually a webln method. Not sure what to do here.

@rolznz rolznz requested review from bumi and reneaaron August 21, 2023 12:25
@@ -214,7 +224,7 @@ export class NostrWebLNProvider implements WebLNProvider, Nip07Provider {
// TODO: use NIP-47 get_info call
async getInfo(): Promise<GetInfoResponse> {
return {
methods: ["getInfo", "sendPayment", "addinvoice", "getBalance"],
methods: ["getInfo", "sendPayment", "addinvoice", "getBalance", "lookupinvoice"],
Copy link
Contributor

Choose a reason for hiding this comment

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

we have sendPayment, so this should probably be lookupInvoice

also addinvoice should probably be makeInvoice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bumi thanks for confirming, I got a bit confused here.

The ones I put are LND methods, but then we also have the official WebLN methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bumi I have updated webln-types and fixed those inconsistencies. Thanks!


return this.executeNip47Request<LookupInvoiceResponse>(
"lookup_invoice",
"lookupinvoice",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we actually need to pass the two here? or can this be some mapping?
this is only needed for the events, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bumi the webln request method is needed for the notification function. WebLN and NWC event names are inconsistent, I think we need to provide both.

Unless we can guarantee the only change is in casing, then we could have a mapping function from camelCase to lower_case_with_underscores

Copy link
Contributor

Choose a reason for hiding this comment

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

but can't we do a mapping from

a = { 
  "lookup_invoice" : "lookupinvoice" 
}
a["lookup_invoice"]

that then can be checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bumi added, thanks

@rolznz rolznz requested a review from bumi August 22, 2023 15:59
@rolznz rolznz enabled auto-merge August 29, 2023 07:40
@rolznz rolznz merged commit c2e4762 into master Aug 29, 2023
1 check passed
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.

2 participants