Skip to content

Conversation

@DSanich
Copy link

@DSanich DSanich commented Dec 19, 2025

fixed #485

In this pull request, I implemented support for legacy retail QR codes. I thought this was an interesting proposal and, inspired by it GaloyMoney/galoy-client#331.

I also covered the new functionality with tests to ensure the security of the code being implemented.

@prusnak @joelklabo @novascreen @Algooo @aekasitt I would be very glad if this was useful for you :)

Comment on lines +89 to +103
// Skip if it's already a known Lightning format
if (
trimmed.toLowerCase().startsWith("lnbc") ||
trimmed.toLowerCase().startsWith("lightning:") ||
trimmed.toLowerCase().startsWith("lnurl") ||
trimmed.toLowerCase().startsWith("lnurl1") ||
trimmed.startsWith("bitcoin:") ||
trimmed.startsWith("cashuA") ||
trimmed.startsWith("cashuB") ||
trimmed.startsWith("creqA") ||
trimmed.startsWith("http://") ||
trimmed.startsWith("https://")
) {
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this block, because where the function is called, we test all these options before the isLegacyRetailQR is called. Also it seems very odd for this function to recognize other types of payments.

const lightningAddress = convertMerchantQRToLightningAddress(trimmedCode, network);
if (lightningAddress) {
console.log("Converted merchant QR code to Lightning Address:", {
original: trimmedCode.substring(0, 50) + "...",
Copy link
Collaborator

@prusnak prusnak Dec 25, 2025

Choose a reason for hiding this comment

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

why only substring here? it makes debugging harder

Copy link
Collaborator

@prusnak prusnak left a comment

Choose a reason for hiding this comment

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

The PR contains lots of boilerplate for other types of legacy QR codes which are not actually supported.

I would focus on handling only the EMV QR codes that start with "000201" and handle only these.

This would remove 80% of the suggested changes.

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.

FR: support legacy retail qr codes

2 participants