CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react #2498

raulduartep · 2023-06-12T19:21:08Z

What current issue(s) from Trello/Github does this address?

What problem does this PR solve?

How did you solve this problem?

How did you make sure your solution works?

Are there any special changes in the code that we should be aware of?

Is there anything else we should know?

Unit tests written?

lllwvlvwlll · 2023-06-12T19:21:10Z

Task linked: CU-864efmq36 Change NWD to use wallet-connect-sdk-wallet-react

__tests__/actions/blockHeightActions.test.js

comountainclimber

We should not combine the scope of this task with the renaming of the import of neon-js

melanke · 2023-06-16T19:45:15Z

~~Max, just a warning, let's not merge this, Raul needs to fix something related to ledger first~~

Done

comountainclimber · 2023-06-23T16:24:52Z

app/components/ConnectDapp/ApproveConnection/ApproveConnection.jsx

+    const { network: proposalNetwork } = getNetworkFromProposal(proposal)
+
+    if (proposalNetwork !== net) {
+      showModal(MODAL_TYPES.NETWORK_SWITCH, {


Suggested change

showModal(MODAL_TYPES.NETWORK_SWITCH, {

return showModal(MODAL_TYPES.NETWORK_SWITCH, {

dAppName: metadata.name,

onSwitch: () => approveSession(proposalNetwork),

onCancel: rejectSession,

proposalNetwork,

})

nitpick suggestion ☝️

comountainclimber · 2023-06-23T16:26:08Z

app/components/ConnectDapp/ConnectionUrlForm/ConnectionUrlForm.jsx

@@ -0,0 +1,73 @@
+// @flow
+


Suggested change

comountainclimber · 2023-06-23T16:31:23Z

app/components/DappRequest/InvokeFunction/InvokeFunction.jsx

@@ -0,0 +1,250 @@
+/* eslint-disable no-nested-ternary */
+// @flow
+


Suggested change

comountainclimber · 2023-06-23T16:32:04Z

app/components/DappRequest/InvokeFunction/InvokeResult.jsx

@@ -0,0 +1,61 @@
+/* eslint-disable no-nested-ternary */
+// @flow
+


Suggested change

comountainclimber · 2023-06-23T16:32:46Z

app/components/DappRequest/InvokeFunction/index.js

@@ -0,0 +1,28 @@
+import { withRouter } from 'react-router-dom'


Suggested change

import { withRouter } from 'react-router-dom'

// @flow

import { withRouter } from 'react-router-dom'

comountainclimber · 2023-06-23T16:32:54Z

app/components/DappRequest/MessageSuccess.jsx

@@ -0,0 +1,39 @@
+// @flow
+


Suggested change

comountainclimber · 2023-06-23T16:33:08Z

app/components/DappRequest/SignMessage/SignMessage.jsx

@@ -0,0 +1,144 @@
+/* eslint-disable no-nested-ternary */
+// @flow
+


Suggested change

comountainclimber · 2023-06-23T16:33:39Z

app/components/DappRequest/SignMessage/SignMessage.jsx

@@ -0,0 +1,144 @@
+/* eslint-disable no-nested-ternary */


Suggested change

/* eslint-disable no-nested-ternary */

comountainclimber · 2023-06-23T16:34:01Z

app/components/DappRequest/SignMessage/SignMessage.jsx

+      setLoading(false)
+    }
+  }
+


Suggested change

if (loading) return <ConnectionLoader />

comountainclimber · 2023-06-23T16:35:06Z

app/components/DappRequest/SignMessage/SignMessage.jsx

+    }
+  }
+
+  return loading ? (


Suggested change

return loading ? (

return success ? (

<MessageSuccess isVerify={false} />

) :

comountainclimber · 2023-06-23T16:36:57Z

app/components/DappRequest/InvokeFunction/InvokeFunction.jsx

+    async () => {
+      try {
+        setLoading(true)
+        const account = new n3Wallet.Account(isHardwareLogin ? publicKey : wif)


I could be wrong but I dont think we ever need to instantiate the account with wif when performing invokeFunction 👀

comountainclimber · 2023-06-23T16:37:38Z

app/components/DappRequest/VerifyMessage/VerifyMessage.jsx

@@ -0,0 +1,162 @@
+/* eslint-disable no-nested-ternary */


Lets follow the pattern in the above file and keep the rule in place

comountainclimber · 2023-06-23T16:43:36Z

app/containers/DappRequest/DappRequest.jsx

+      })()
+    },
+    [rejectRequest, Component, request, showErrorNotification, history],
+  )


This is some pretty weird looking syntax, I think something like this would be cleaner:

useEffect(() => { async function checkUnsupportedMethod() { if (!Component) { showErrorNotification({ message: 'unsupportedMethod', }); await rejectRequest(request, { code: 1, message: 'unsupportedMethod', }); history.push(ROUTES.DASHBOARD); } }; checkUnsupportedMethod(); }, [rejectRequest, Component, request, showErrorNotification, history]);

Yea, I would even put the checkUnsupportedMethod on a separated useCallback

comountainclimber · 2023-06-23T16:43:59Z

app/containers/DappRequest/DappRequest.jsx

+  const { method } = request.params.request
+  const { rejectRequest } = useWalletConnectWallet()
+
+  const Component = useMemo(() => componentsByMethod[method], [method])


I like this 💪 nice work @raulduartep

comountainclimber · 2023-06-23T16:46:09Z

app/ledger/n3NeonLedger.js

+  const id = showInfoNotification({
+    message: 'Please sign the transaction on your hardware device',
+    autoDismiss: 0,
+  })(store.dispatch)


Have you tested this change?? We already dispatch this notification from other places in the application so adding it here is probably going to cause it to render twice

comountainclimber · 2023-06-23T16:49:42Z

app/util/walletConnect.js

+export const walletConnectOptions: TOptions = {
+  clientOptions: {
+    metadata: {
+      name: 'CoZ Wallet Prototype',


Shouldnt this be Neon Walletnot CoZ Wallet Prototype

comountainclimber reviewed Jun 13, 2023

View reviewed changes

__tests__/actions/blockHeightActions.test.js Show resolved Hide resolved

comountainclimber suggested changes Jun 13, 2023

View reviewed changes

raulduartep force-pushed the CU-864efmq36 branch 4 times, most recently from 61bd289 to 409dbe6 Compare June 21, 2023 21:56

melanke approved these changes Jun 22, 2023

View reviewed changes

melanke requested a review from comountainclimber June 22, 2023 13:14

raulduartep force-pushed the CU-864efmq36 branch 3 times, most recently from 481e3fd to 13edd57 Compare June 22, 2023 19:44

comountainclimber reviewed Jun 23, 2023

View reviewed changes

app/components/ConnectDapp/ConnectionUrlForm/ConnectionUrlForm.jsx Outdated

@@ -0,0 +1,73 @@

// @flow

Copy link

Contributor

comountainclimber Jun 23, 2023

Choose a reason for hiding this comment

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

Suggested change

comountainclimber reviewed Jun 23, 2023

View reviewed changes

app/components/DappRequest/MessageSuccess.jsx Outdated

@@ -0,0 +1,39 @@

// @flow

Copy link

Contributor

comountainclimber Jun 23, 2023

Choose a reason for hiding this comment

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

Suggested change

comountainclimber reviewed Jun 23, 2023

View reviewed changes

raulduartep force-pushed the CU-864efmq36 branch from 13edd57 to 83760f9 Compare June 23, 2023 17:08

CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react

8503de4

raulduartep force-pushed the CU-864efmq36 branch from 83760f9 to 8503de4 Compare June 26, 2023 18:24

comountainclimber approved these changes Jun 26, 2023

View reviewed changes

melanke merged commit 2800340 into dev Jun 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react #2498

CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react #2498

raulduartep commented Jun 12, 2023

lllwvlvwlll commented Jun 12, 2023

comountainclimber left a comment

melanke commented Jun 16, 2023 •

edited

Loading

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

melanke Jun 26, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

comountainclimber Jun 23, 2023

-      showModal(MODAL_TYPES.NETWORK_SWITCH, {
+      return showModal(MODAL_TYPES.NETWORK_SWITCH, {
+        dAppName: metadata.name,
+        onSwitch: () => approveSession(proposalNetwork),
+        onCancel: rejectSession,
+        proposalNetwork,
+      })

		@@ -0,0 +1,250 @@
		/* eslint-disable no-nested-ternary */
		// @flow

		@@ -0,0 +1,61 @@
		/* eslint-disable no-nested-ternary */
		// @flow

		@@ -0,0 +1,28 @@
		import { withRouter } from 'react-router-dom'

		@@ -0,0 +1,144 @@
		/* eslint-disable no-nested-ternary */
		// @flow

-  return loading ? (
+  return success ? (
+     <MessageSuccess isVerify={false} />
+  ) :

CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react #2498

CU-864efmq36 - Change NWD to use wallet-connect-sdk-wallet-react #2498

Conversation

raulduartep commented Jun 12, 2023

lllwvlvwlll commented Jun 12, 2023

comountainclimber left a comment

Choose a reason for hiding this comment

melanke commented Jun 16, 2023 • 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

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

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

melanke commented Jun 16, 2023 •

edited

Loading