Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@beescuit
Copy link

Really simple but functional. I noticed that starblocks's payment invoice uses lightning: and not lightning:// (the only one specified by the function) so I added support for that prefix too by using regex.


export const sanitizePaymentRequest = (pr) => {
return pr.replace(prefix, '')
return pr.replace(/lightning:(\/\/)?/g, '')

Choose a reason for hiding this comment

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

Can you add comment explaining the possible prefix matches?

Copy link
Author

Choose a reason for hiding this comment

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

I just found the two above (lightning: and lightning://) so they are the only possible matches

Choose a reason for hiding this comment

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

I meant to add it as a comment in the code above that line, something like:
// Payment requests should start with either lightning: or lightning://

Copy link
Author

Choose a reason for hiding this comment

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

oh sorry for my misunderstanding. I'll do it rn

Copy link

Choose a reason for hiding this comment

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

Is there any documentation for the lightning:// prefix? I could only find https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md which says not to use lightning://

Copy link
Member

Choose a reason for hiding this comment

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

Our usage of the lightning:// prefix actually predates BOLT 11 itself.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

A more direct change is to modify the definition of prefix here:

No regex required!

@bryanvu
Copy link
Contributor

bryanvu commented Dec 22, 2017

Thanks @BeeCodiing and @bulenttastan! This was completed as a followup to PR #107.

@bryanvu bryanvu closed this Dec 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants