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

Adding NDEF capability #239

Merged
merged 28 commits into from
Dec 15, 2024
Merged

Adding NDEF capability #239

merged 28 commits into from
Dec 15, 2024

Conversation

lollerfirst
Copy link
Contributor

Read and write cashu tokens to a NFC card through WebNFC amazing NDEFReader.

Lmk if you want to move the relevant code to a store to make it a bit tidier.

@prusnak
Copy link
Contributor

prusnak commented Oct 14, 2024

Amazing! Love it. I saw the demo in Berlin.

@lollerfirst
Copy link
Contributor Author

Amazing! Love it

Were you there? I haven't seen you around! Wish I met you!

@prusnak
Copy link
Contributor

prusnak commented Oct 15, 2024

Might it be worth considering adding an option which writes URL https://wallet.cashu.me/?token=cashuB... instead of raw text cashuB... to an NFC card? And even make it the default?

This would be perfect onboarding experience, because this works even if a user has no idea what a cashu token is and how to redeem it.

src/pages/WalletPage.vue Outdated Show resolved Hide resolved
@lollerfirst
Copy link
Contributor Author

Might it be worth considering adding an option which writes URL https://wallet.cashu.me/?token=cashuB... instead of raw text cashuB... to an NFC card? And even make it the default?

This would be perfect onboarding experience, because this works even if a user has no idea what a cashu token is and how to redeem it.

@prusnak
I have added this feature. It's amazing. Try it out.
Also it seems like NDEFRecord constructor in Chrome is broken and i needs to be bypassed. Luckly I found an example here: https://developer.chrome.com/docs/capabilities/nfc

src/pages/WalletPage.vue Outdated Show resolved Hide resolved
this.scanningCard = false;
try {
const tokenURL =
window.location.toString() +
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether we should use this or just hardcode wallet.cashu.me here

Both approaches have their advantages and disadvantages. What do you think @callebtc?

@prusnak
Copy link
Contributor

prusnak commented Oct 21, 2024

Since this got raised on Telegram. Maybe we can use this PR to also fix ?token=... -> #token=...:

diff --git a/src-pwa/manifest.json b/src-pwa/manifest.json
index 7bca548..c5e6761 100644
--- a/src-pwa/manifest.json
+++ b/src-pwa/manifest.json
@@ -9,11 +9,11 @@
   "protocol_handlers": [
     {
       "protocol": "web+cashu",
-      "url": "/?token=%s"
+      "url": "/#token=%s"
     },
     {
       "protocol": "web+lightning",
-      "url": "/?lightning=%s"
+      "url": "/#lightning=%s"
     }
   ],
   "icons": [
diff --git a/src/components/SendTokenDialog.vue b/src/components/SendTokenDialog.vue
index 1983838..c3f375c 100644
--- a/src/components/SendTokenDialog.vue
+++ b/src/components/SendTokenDialog.vue
@@ -280,7 +280,7 @@
                 size="md"
                 icon="link"
                 flat
-                @click="copyText(baseURL + '?token=' + sendData.tokensBase64)"
+                @click="copyText(baseURL + '#token=' + sendData.tokensBase64)"
                 ><q-tooltip>Copy link</q-tooltip></q-btn
               >
               <q-btn
@@ -676,7 +676,7 @@ export default defineComponent({
                 try {
                   const tokenURL =
                     window.location.toString() +
-                    "?token=" +
+                    "#token=" +
                     this.sendData.tokensBase64;
                   this.ndef
                     .write(
diff --git a/src/pages/WalletPage.vue b/src/pages/WalletPage.vue
index ea3eb8f..d57a0a0 100644
--- a/src/pages/WalletPage.vue
+++ b/src/pages/WalletPage.vue
@@ -410,7 +410,7 @@ export default {
                     const decodedTokenLink = new TextDecoder().decode(
                       message.records[0].data
                     );
-                    const cashuIndex = decodedTokenLink.indexOf("?token=cashu");
+                    const cashuIndex = decodedTokenLink.indexOf("#token=cashu");
                     if (cashuIndex === -1) {
                       throw new Error("not a cashu token");
                     }
diff --git a/src/stores/wallet.ts b/src/stores/wallet.ts
index 2afd88a..dfca241 100644
--- a/src/stores/wallet.ts
+++ b/src/stores/wallet.ts
@@ -1104,7 +1104,7 @@ export const useWalletStore = defineStore("wallet", {
         this.payInvoiceData.input.request = req
         await this.lnurlPayFirst(this.payInvoiceData.input.request);
       } else if (req.indexOf("cashuA") !== -1) {
-        // very dirty way of parsing cashu tokens from either a pasted token or a URL like https://host.com?token=eyJwcm
+        // very dirty way of parsing cashu tokens from either a pasted token or a URL like https://host.com#token=eyJwcm
         receiveStore.receiveData.tokensBase64 = req.slice(req.indexOf("cashuA"));
         this.handleCashuToken()
       } else if (req.indexOf("cashuB") !== -1) {

@lollerfirst
Copy link
Contributor Author

@prusnak

Unfortunately doesn't seem to be enough. Tested it right now and when opening the link Chrome inserts a "/" between the "#" and the token.

https://192.168.1.130:8080/#/token=cashuB...

@prusnak
Copy link
Contributor

prusnak commented Oct 21, 2024

Unfortunately doesn't seem to be enough.

It seems this is some Vue magic. Not Chrome specific.

@lollerfirst
Copy link
Contributor Author

#244

@ok300
Copy link

ok300 commented Oct 21, 2024

Might it be worth considering adding an option which writes URL ... instead of raw text cashuB... to an NFC card?

This could bring extra limitations on the UX. AFAIK URLs have max ~2000 chars, so a token has to fit in that.

A V4 token can be, depending on the proofs (inputs) :

  • 1 proof => ~500 chars
  • 2 proofs => ~910 chars
  • 3 proofs => ~1340 chars
  • 4 proofs => ~1760 chars

Maybe this can be validated in the "write NFC" phase?

@prusnak
Copy link
Contributor

prusnak commented Oct 21, 2024

This could bring extra limitations on the UX. AFAIK URLs have max ~2000 chars, so a token has to fit in that.

Not really. Since only Chrome on Android can read NFC via WebNFC and this browser supports 2MB payload in GET, I don't see how an UX can be limited when compared to the original proposal.

@lollerfirst
Copy link
Contributor Author

Do we want to merge this?

@prusnak
Copy link
Contributor

prusnak commented Nov 3, 2024

Do we want to merge this?

I am in favor, but cannot do that :)

@callebtc
Copy link
Collaborator

callebtc commented Nov 4, 2024

The functionality is very cool, I would love to merge it but the UI needs work.

@prusnak
Copy link
Contributor

prusnak commented Nov 4, 2024

but the UI needs work.

Do you have anything particular in mind?

@prusnak
Copy link
Contributor

prusnak commented Dec 1, 2024

@lollerfirst I think we should add a switch which allows you to choose between the text/URL/binary representation of the token.

It does not have to be a part of this PR, but we should think about the ideal UX for this option chooser.

Options:

@lollerfirst
Copy link
Contributor Author

This can be part of some "NFC settings" in the settings I suppose.

@lollerfirst
Copy link
Contributor Author

Also @prusnak don't know if it interests you but lately I've been focusing on cashu-kvac (the Signal anonymous credentials scheme, but for cashu) which would give the option for making tokens of constant size (~100-200 bytes)

src/pages/WalletPage.vue Outdated Show resolved Hide resolved
@lollerfirst
Copy link
Contributor Author

Finally managed to fix the "toggle" behaviour for the scanner.

src/components/ReceiveTokenDialog.vue Outdated Show resolved Hide resolved
@lollerfirst
Copy link
Contributor Author

I will be using the encoding choice in the scanner once cashu-ts supports binary tokens

prusnak and others added 4 commits December 13, 2024 13:29
better UI for NFC errors
everything is also prepared for reading/writing binary tokens
and we will enable the functionality once it arrives to cashu-ts
nfc: implement reading/writing of text/weburl tokens
@lollerfirst
Copy link
Contributor Author

lollerfirst commented Dec 13, 2024

This works really nice. Although when receiving with NFC it automatically redeems, whereas the other methods only fill the receiveData.tokensBase64 property and then it's the user who decides to redeem by pressing "receive".

Should we conform to that? @prusnak

@prusnak
Copy link
Contributor

prusnak commented Dec 13, 2024

Yeah, maybe we should not automatically redeem and just prefill the receive form like other methods do.

Copy link
Contributor

@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.

tested ACK

I think we should merge this one now @callebtc

We will follow up with another PR which adds binary encoding of tokens once the encoding/decoding is supported in cashu-ts

@prusnak
Copy link
Contributor

prusnak commented Dec 15, 2024

the edits commit 8649b5c broke the receive dialog.

When NFC is enabled and Payment Requests are disabled:

  1. the Payment Requests button is still shown
  2. there is no Close Button

When NFC is disabled or Payment Requests are enabled, everything is shown like it should.

@callebtc callebtc merged commit e2dd0aa into cashubtc:main Dec 15, 2024
1 check failed
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.

4 participants