CLOSED: Fix BIP21 support to default to Lightning, enable user to choose onchain

[agbegin]

Background

Currently when scanning a BIP21 unified QR, the Bitcoin Beach Wallet defaults to paying onchain. Defaulting to Lightning was previously implemented (see: GaloyMoney/galoy-client#42 ) but is no longer working as intended.

For context, view the issue raised by Stephen of Bitcoin Design:
GaloyMoney/blink-mobile#411

Task

• Diagnose and fix the issue so that scanning a BIP21 qr will default to Lightning payment
• Enable the user to choose between Lightning and onchain, so if Lightning is not working they can still choose to make an onchain payment when scanning a unified QR

Acceptance criteria

• PR that meets the above requirements is merged, deployed and tested

Payout

• 1M sats

[abhiShandy]

Help me understand this issue. Is the mobile app the only way to scan the QR code?

[abhiShandy]

Nevermind. I have the mobile app running and it is unable to scan BIP21 codes.

[abhiShandy]

My plan is to fix the parsing function in "galoy-client" since that is used in "galoy-mobile" to parse the QR codes.

[abhiShandy]

Follow my notes here:

https://abhishandy.notion.site/Enable-scanning-BIP21-QR-codes-1c50c9d5a7474b8f8c2aea061256722b

[daviroo]

@abhiShandy can you clarify which tasks in your notes you view as part of the bounty and which you view as going over and above what is outlined above?

[daviroo]

@abhiShandy I see in your notes that you have a checkbox for returning both onchain and lightning responses from the client for unified QRs. I actually think it makes sense to make this the default behaviour (even when the lightning invoice is expired). The reason is in the mobile app (or any front end which uses the client) we could provide feedback to the user for why we are doing certain actions. So for instance if the ln invoice is expired we would receive a response which includes some combination of the below:

export type LightningPaymentDestination =
  | {
      paymentType: PaymentType.Lightning
      valid: true
      paymentRequest: string
      amount?: number | undefined
      memo?: string | undefined
      sameNode: boolean
    }
  | {
      paymentType: PaymentType.Lightning
      valid: false
      invalidReason: InvalidLightningDestinationReason
    }

export type OnchainPaymentDestination =
  | {
      paymentType: PaymentType.Onchain
      valid: true
      address: string
      amount?: number | undefined
      memo?: string | undefined
    }
  | {
      paymentType: PaymentType.Onchain
      valid: false
      invalidReason: InvalidOnchainDestinationReason
    }

That way we could explain to the user that the lightning invoice is expired (because we would have a reason for the invalid status) but that we are falling back to onchain. This would mean adding a new return type to the ParsedPaymentDestination type which should have a sensible shape and allow for this front end use case. Let me know if you want to discuss more.

[abhiShandy]

I agree. We should keep the user informed.

[abhiShandy]

the new ParsedPaymentDestination type should accommodate the following cases:

• Valid on-chain & valid LN (most common)
• Valid on-chain & invalid LN (somewhat likely)
• Invalid on-chain & valid LN (rare)
• Invalid on-chain & invalid LN (most rare)

[juscamarena]

I'd make sure it also works opening the URI from a button not just scanning.

[agbegin]

closing.