Add support for amount and memo values when scanning QR code #13

Closed
opened 1 year ago by fekt · 4 comments
fekt commented 1 year ago
Collaborator

I tested scanning QR codes generated by the WordPress plugin the other day and noting here that the Android app detects the codes as invalid. I believe this is due to the amt and memo params that are passed. Existing Android code may only support an address with no params but this needs to be verified.

While a memo during checkout is optional, the amount is always passed and the Android app should support these params and populate the values in appropriate fields when scanning.

I tested scanning QR codes generated by the WordPress plugin the other day and noting here that the Android app detects the codes as invalid. I believe this is due to the `amt` and `memo` params that are passed. Existing Android code may only support an address with no params but this needs to be verified. While a memo during checkout is optional, the amount is always passed and the Android app should support these params and populate the values in appropriate fields when scanning.
duke commented 1 year ago
Owner

@fekt yes, those should be supported. IIRC, both SD + SDL support amt and memo params in payment URLS

@fekt yes, those should be supported. IIRC, both SD + SDL support `amt` and `memo` params in payment URLS
fekt commented 1 year ago
Poster
Collaborator

It looks like part of the problem is actually the QR code in the WordPress plugin. This was never really tested with the old pairing app. The pay URI is formatted incorrectly like this: hush://zs1a3xrq8k4sl2z4mu3y5mcp803frzna5hlht5g629u362grcsdny999ewzhyxf5wl28453ymf8r60?amt=515.89539089

It should use a colon only like this:
hush:zs1a3xrq8m4sl2z4mu3y5mcp103frzna5hlht5a542u362grcsdny999ewzhyxf5wl28453ymf8e50?amt=515.89539089

The app might populate the values once fixed in WP plugin (trying that now). Unsure as comment in the code says ZIP 321 is not actually implemented.

It looks like part of the problem is actually the QR code in the WordPress plugin. This was never really tested with the old pairing app. The pay URI is formatted incorrectly like this: `hush://zs1a3xrq8k4sl2z4mu3y5mcp803frzna5hlht5g629u362grcsdny999ewzhyxf5wl28453ymf8r60?amt=515.89539089` It should use a colon only like this: `hush:zs1a3xrq8m4sl2z4mu3y5mcp103frzna5hlht5a542u362grcsdny999ewzhyxf5wl28453ymf8e50?amt=515.89539089` The app might populate the values once fixed in WP plugin (trying that now). Unsure as comment in the code says [ZIP 321](https://zips.z.cash/zip-0321) is not actually implemented.
fekt commented 1 year ago
Poster
Collaborator

Plugin now uses correct format. App had another bug where it was checking substring in pos 6 instead of 5 for hush:. With both fixes, the QR in WP plugin now validates in the app and populates the address but does not populate amount or memo.

The amount is specified before getting to the send screen to scan QR code and not sure if that will be easy to change. I'll try populating the memo first another day since there's a field on the same screen for it with the address.

Plugin now uses correct format. App had another bug where it was checking substring in pos 6 instead of 5 for `hush:`. With both fixes, the QR in WP plugin now validates in the app and populates the address but does not populate amount or memo. The amount is specified before getting to the send screen to scan QR code and not sure if that will be easy to change. I'll try populating the memo first another day since there's a field on the same screen for it with the address.
fekt commented 1 year ago
Poster
Collaborator

Seems I have this working now with the amount and memo. Fuck Zatoshi and praise Satoshi.

Seems I have this working now with the amount and memo. Fuck Zatoshi and praise Satoshi.
fekt closed this issue 1 year ago
Sign in to join this conversation.
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.