Pin support (LUD-21) #33

X-Hades-X · 2024-05-20T08:40:30Z

This PR implements the LUD-21 as specified here https://github.com/bitcoin-ring/luds/blob/withdraw-pin/21.md
This LUD has not been merged to as official LNURL LUD

Adds a pin pad that appears inside a bottom drawer to enter a 4 digit pin if the amount that has to be paid is bigger then the pinLimit value of the LNURLw response.

X-Hades-X · 2024-05-20T08:42:07Z

src/hooks/useNfc.ts

@@ -49,7 +70,7 @@ export const useNfc = () => {
  }, []);

  const readingNfcLoop = useCallback(
-    async (pr: string) => {
+    async (pr: string, amount: number) => {


I had to pass the amount into this function to check if pin is required.

It is also needed for the commented out part, the lnurlp request as far as I understand. Was there a reason why it is commented out? Are there any bugs or is it a bad idea to add this parameter to the function?

The LNURLp was commented as it require some additional work. We could do that in another PR :)
Adding the amount is okay.

X-Hades-X · 2024-05-20T08:44:51Z

src/hooks/useNfc.ts

        if (debitCardData?.status === "OK" && !error) {
          setIsNfcActionSuccess(true);
        } else {
+          if (debitCardData?.status === "ERROR" && !error) {


If the pin is wrong the response is in the debitCardData object not the error object. Is that a wrong implementation on my side? Should the server send another status then 200? I haven't seen a specification for that in the LUD-3.

That's probably a mis-implementation on our side. You can keep the code like this for the moment.

SwissBitcoinPay

Additionally, there is quite a few of lint issues.
Please run npm run lint and fix the errors you see.

SwissBitcoinPay · 2024-05-20T09:17:45Z

package.json

@@ -47,6 +47,7 @@
    "react-dom": "18.2.0",
    "react-i18next": "13.2.2",
    "react-native": "0.73.2",
+    "react-native-animated-bottom-drawer": "^0.0.23",


For security reasons, we keep the version fixed like this:

Suggested change

"react-native-animated-bottom-drawer": "^0.0.23",

"react-native-animated-bottom-drawer": "0.0.23",

Please do npm install again after changing

SwissBitcoinPay · 2024-05-20T09:17:55Z

package.json

@@ -58,6 +59,7 @@
    "react-native-dotenv": "3.4.9",
    "react-native-linear-gradient": "2.8.3",
    "react-native-nfc-manager": "3.14.11",
+    "react-native-pin-view": "^3.0.3",


For security reasons, we keep the version fixed like this:

Suggested change

"react-native-pin-view": "^3.0.3",

"react-native-pin-view": "3.0.3",

Please do npm install again after changing

SwissBitcoinPay · 2024-05-20T10:00:03Z

src/hooks/useNfc.ts

@@ -49,7 +70,7 @@ export const useNfc = () => {
  }, []);

  const readingNfcLoop = useCallback(
-    async (pr: string) => {
+    async (pr: string, amount: number) => {


The LNURLp was commented as it require some additional work. We could do that in another PR :)
Adding the amount is okay.

SwissBitcoinPay · 2024-05-20T10:01:03Z

src/hooks/useNfc.ts

@@ -114,15 +135,31 @@ export const useNfc = () => {
              tag: "withdrawRequest";
              callback: string;
              k1: string;
+              pinLimit: number;


As pinLimit is not always defined, you need to add a ? as "possibly undefined"

Suggested change

pinLimit: number;

pinLimit?: number;

SwissBitcoinPay · 2024-05-20T10:01:21Z

src/hooks/useNfc.ts

            }>(cardData);

+            let pin = "";
+            if (cardDataResponse.pinLimit !== undefined) {


Simplify it with:

Suggested change

if (cardDataResponse.pinLimit !== undefined) {

if (cardDataResponse.pinLimit) {

I was thinking that the pinLimit could be set to 0. But then one could argue that means it is "disabled" since the number in the pinLimit is the min amounted needed before asking the pin and the smallest possible number for that would be 1.
How should we implement it?

Interesting. The doc says value must be a positive integer (including 0), but also to withdraw an amount equal to or greater [...].
But as we can't really withdraw 0 sat, I agree with you that 0 could means "disabled".
As this is not clear, you can keep it as cardDataResponse.pinLimit !== undefined and ignore my initial comment.

SwissBitcoinPay · 2024-05-20T10:52:50Z

src/hooks/useNfc.ts

        if (debitCardData?.status === "OK" && !error) {
          setIsNfcActionSuccess(true);
        } else {
+          if (debitCardData?.status === "ERROR" && !error) {


That's probably a mis-implementation on our side. You can keep the code like this for the moment.

SwissBitcoinPay · 2024-05-20T10:55:06Z

src/screens/Invoice/Invoice.tsx

@@ -32,6 +32,8 @@ import { faBitcoin } from "@fortawesome/free-brands-svg-icons";
 import axios from "axios";
 import * as S from "./styled";

+import { PinPad } from "../PinPad";


Please do the following:

Move the PinPad component (folder) to src/components

Add export { PinPad } from "./PinPad"; to src/components/index.ts

Change this line to:

Suggested change

import { PinPad } from "../PinPad";

import { PinPad } from "@components";

SwissBitcoinPay · 2024-05-20T11:00:29Z

src/screens/PinPad/PinPad.tsx

+import { faDeleteLeft, faUnlock } from "@fortawesome/free-solid-svg-icons";
+
+export const PinPad = (props) => {
+  const pinView = useRef(null);


Above the component, you can add the following:

type PinViewFunctions = { clearAll: () => void; };

And change this line to

Suggested change

const pinView = useRef(null);

const pinView = useRef<PinViewFunctions>(null);

SwissBitcoinPay · 2024-05-20T11:01:24Z

src/screens/PinPad/PinPad.tsx

+import { Text, Icon } from "@components";
+import { faDeleteLeft, faUnlock } from "@fortawesome/free-solid-svg-icons";
+
+export const PinPad = (props) => {


You need to type the props, like we do on other components.

SwissBitcoinPay · 2024-05-20T11:03:22Z

src/screens/PinPad/PinPad.tsx

+  const styles = StyleSheet.create({
+    container: {
+        backgroundColor:  "rgba(0,0,0,0.5)",
+    },
+    handleContainer: {
+      backgroundColor:  "transparent",
+    },
+    pinPad: {
+      backgroundColor: "transparent",
+      alignItems: "center",
+      flex: 1,
+    },
+    pinPadTitle: {
+      paddingTop: 48,
+      paddingBottom: 24,
+      color: "rgba(255,255,255,1)",
+      fontSize: 48,
+    },
+    whiteBorder: {
+      borderWidth: 1,
+      borderColor: "#FFF",
+    },
+  });


We usually we styled-components.
Can you try to migrate this to styled.ts. Check other styled.ts files in the repo to see how it works. It's pretty simple!

And it help to keep the component's logic files clean.

X-Hades-X · 2024-05-24T13:30:28Z

Additionally, there is quite a few of lint issues. Please run npm run lint and fix the errors you see.

I can't get rid of these whatever I do. I don't understand why it is telling me this because the code clearly works. What can I do about this?

X-Hades-X · 2024-05-24T14:18:45Z

This I also can't solve and don't understand what it needs

jimmydjabali · 2024-05-25T12:43:51Z

src/hooks/useNfc.ts

+  const [ pinResolver, setPinResolver ] = useState({ resolve: undefined })
+
+  const setPin = (pin: string) => {
+    if(pinResolver.resolve) {
+      pinResolver.resolve(pin);
+    }
+  }
+
+  const getPin = useCallback(async () => {
+    const pinInputPromise = () => {
+      let _pinResolver;
+      return [ new Promise(( resolve ) => {
+          _pinResolver = resolve
+      }), _pinResolver]
+    }
+
+    const [ promise, resolve ] = pinInputPromise()
+    setPinResolver({ resolve })


You can fix TS issues here by committing this

Suggested change

const [ pinResolver, setPinResolver ] = useState({ resolve: undefined })

const setPin = (pin: string) => {

if(pinResolver.resolve) {

pinResolver.resolve(pin);

}

}

const getPin = useCallback(async () => {

const pinInputPromise = () => {

let _pinResolver;

return [ new Promise(( resolve ) => {

_pinResolver = resolve

}), _pinResolver]

}

const [ promise, resolve ] = pinInputPromise()

setPinResolver({ resolve })

const [pinResolver, setPinResolver] = useState<{

resolve: (v: string) => void;

}>();

const setPin = (pin: string) => {

if (pinResolver?.resolve) {

pinResolver.resolve(pin);

}

};

const getPin = useCallback(async () => {

const pinInputPromise = () => {

let _pinResolver;

return [

new Promise<string>((resolve) => {

_pinResolver = resolve;

}),

_pinResolver

];

};

const [promise, resolve] = pinInputPromise();

setPinResolver({ resolve } as unknown as { resolve: (v: string) => void });

I had something like that but wasn't thinkin about casting first to unknown. Thanks 👍

jimmydjabali · 2024-05-25T12:45:30Z

src/components/PinPad/PinPad.tsx

+                buttonViewStyle={S.PinViewStyles.whiteBorder}
+                buttonTextStyle={{ color: "#FFF" }}
+                onButtonPress={key => setButtonPressed(key)}
+                customLeftButton={showRemoveButton ? <Icon icon={faDeleteLeft} size={36} color={"#FFF"} /> : undefined}


To fix the TS issue here, you need to replace this lin with:

Suggested change

customLeftButton={showRemoveButton ? <Icon icon={faDeleteLeft} size={36} color={"#FFF"} /> : undefined}

customLeftButton={showRemoveButton ? LeftButton : undefined}

And add a LeftButton component below:

const LeftButton = () => <Icon icon={faDeleteLeft} size={36} color={"#FFF"} />;

I had <LeftButton />, so that's how you do it

If I do it without <> it never appears

jimmydjabali · 2024-05-25T12:53:14Z

src/components/PinPad/PinPad.tsx

+              </S.PinPadTitle>
+              <ReactNativePinView
+                inputSize={32}
+                ref={pinView}


To avoid a new TS issue here, add a // @ts-ignore :

Suggested change

ref={pinView}

// @ts-ignore

ref={pinView}

X-Hades-X added 6 commits April 22, 2024 11:44

Pin support

dc829ff

handle everything in useNfc hook for now

c163052

move pinpad into own component

5c2baff

add bottom drawer

d28605b

cleanup

d634623

fix failed pin state

a988fc1

X-Hades-X commented May 20, 2024

View reviewed changes

check undefined since it could be 0

fb42dc2

SwissBitcoinPay requested changes May 20, 2024

View reviewed changes

changes from review

a83a0df

jimmydjabali requested changes May 25, 2024

View reviewed changes

solve more lint issues

6302cf5

jimmydjabali approved these changes May 26, 2024

View reviewed changes

SwissBitcoinPay merged commit ec62cc6 into SwissBitcoinPay:main May 26, 2024
1 check passed

SwissBitcoinPay mentioned this pull request Sep 6, 2024

LUD-21: pinLimit for withdrawRequest - resolves #201 lnurl/luds#200

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pin support (LUD-21) #33

Pin support (LUD-21) #33

X-Hades-X commented May 20, 2024

X-Hades-X May 20, 2024

SwissBitcoinPay May 20, 2024

X-Hades-X May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay left a comment

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

X-Hades-X May 24, 2024

jimmydjabali May 24, 2024 •

edited

Loading

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

SwissBitcoinPay May 20, 2024

X-Hades-X commented May 24, 2024

X-Hades-X commented May 24, 2024

jimmydjabali May 25, 2024

X-Hades-X May 26, 2024

jimmydjabali May 25, 2024

X-Hades-X May 26, 2024 •

edited

Loading

X-Hades-X May 26, 2024

jimmydjabali May 25, 2024

	"react-native-animated-bottom-drawer": "^0.0.23",
	"react-native-animated-bottom-drawer": "0.0.23",

	"react-native-pin-view": "^3.0.3",
	"react-native-pin-view": "3.0.3",

	if (cardDataResponse.pinLimit !== undefined) {
	if (cardDataResponse.pinLimit) {

	import { PinPad } from "../PinPad";
	import { PinPad } from "@components";

	const pinView = useRef(null);
	const pinView = useRef<PinViewFunctions>(null);

	customLeftButton={showRemoveButton ? <Icon icon={faDeleteLeft} size={36} color={"#FFF"} /> : undefined}
	customLeftButton={showRemoveButton ? LeftButton : undefined}

Pin support (LUD-21) #33

Pin support (LUD-21) #33

Conversation

X-Hades-X commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SwissBitcoinPay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmydjabali May 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

X-Hades-X commented May 24, 2024

X-Hades-X commented May 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

X-Hades-X May 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmydjabali May 24, 2024 •

edited

Loading

X-Hades-X May 26, 2024 •

edited

Loading