Feat/add GridPlus Lattice1 Hardware Wallet #1346

mrcnk · 2024-02-15T17:15:30Z

Fixes BX-####
Figma link (if any):

What changed (plus any additional context for devs)

Added GridPlus as a hardware wallet option.
Added the onboarding process flow for GridPlus.
Added gridplus-sdk dependency and allowed it in lavamoat.
Added handlers to sign legacy transaction, typed data, and personal messages with GridPlus.
Added handler to broadcast transactions with GridPlus.
Added semi-automatic E2E test for the GridPlus onboarding.
Added an option to derive an address with custom address index with GridPlus.
Added effects to check for removed permissions of GridPlus (in Lattice1).

Screen recordings / screenshots

What to test

All new signing handlers: legacy, EIP-712, message.
Broadcasting transactions signed by Lattice1.
dApp interactions with https://metamask.github.io/test-dapp/.

…omGridPlus

socket-security · 2024-02-15T17:18:49Z

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert	Package	Note	Source
Install scripts	npm/[email protected]	Install script: postinstall Source: `npx patch-package`	`package.json`

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@SocketSecurity ignore npm/[email protected]

brunobar79

REALLY GOOD STUFF!!

Added some comments but nothing major.
LMK if you have any questions.

src/design-system/components/TextOverflow/TextOverflow.tsx

src/entries/popup/Routes.tsx

src/entries/popup/components/Checkbox/Checkbox.tsx

src/entries/popup/handlers/gridplus.ts

src/entries/popup/handlers/wallet.ts

src/entries/popup/pages/hw/gridplus/addressChoice.tsx

static/manifest.json

static/vendor/trezor-connect.js

mrcnk · 2024-02-28T15:08:53Z

@brunobar79 the PR feedback improvements are implemented now.

socket-security · 2024-02-29T14:29:37Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@ethereumjs/[email protected]	Transitive: network	`+3`	1.41 MB	holgerd77
npm/[email protected]	environment, network	`+14`	4.53 MB	asmiller1989

View full report↗︎

github-actions · 2024-02-29T19:33:19Z

Here's the packed extension for this build:
node_modules.tar.gz

BrodyHughes

did a quick lil onboarding test using mock data / IS_TESTING. Only issues I see:

Wallets & Keys page uses the generic Ledger logo instead of the Grid Lattice + logo
The first two pages of the flow don’t seem to match our other designs. mainly font size / spacing. this is just me nit picking tho. may need designer input.

e2e/helpers.ts

package.json

BrodyHughes

Finally got to test this with a physical GridPlus Lattice today:

Import flow worked well
Eth mainnet swaps, sends + dapp interactions are the only thing that seemed to work for me. No L2s or sidechains. When attempting, the GridPlus screen would flash, then the browser extension would say ‘make sure your GridPlus is unlocked’
Importing a duplicate wallet just caused the gridPlus import flow to get stuck on this screen:

Also when you import a wallet with the GridPlus, then import a sub wallet under the same mnemonic, they appear as separate wallet groups in our Wallets & Keys:

Additionally, it would be nice if the flow was keyboard navigable. I have suggestions on how to make the first page of the flow work with our keyboard nav. You can add tabIndex={0} on most things to make it work, or in some cases, you can also use our Lens component.

src/entries/popup/pages/hw/gridplus/walletCredentials.tsx

…-extension into feat/add-gridplus-lattice1

mrcnk · 2024-03-11T14:16:47Z

@BrodyHughes @brunobar79 the new feedback is addressed now. Pushed the improvements.

BrodyHughes · 2024-03-15T17:28:25Z

Do you have a more square sized asset for this screen so it matches the designs of the others?

Also still seeing these issues:

Eth mainnet swaps, sends + dapp interactions are the only thing that seemed to work for me. No L2s or sidechains. When attempting, the GridPlus screen would flash, then the browser extension would say ‘make sure your GridPlus is unlocked’

Importing a duplicate wallet just caused the gridPlus import flow to get stuck on this screen:

* Also when you import a wallet with the GridPlus, then import a sub wallet under the same mnemonic, they appear as separate wallet groups in our Wallets & Keys:

mrcnk · 2024-04-15T13:55:34Z

Hi @BrodyHughes @brunobar79, I refactored the tx signing part and L2 transactions should now be working fine. Also the logo in Wallets & Keys is updated to match the rest.

greg-schrammel · 2024-04-20T15:50:48Z

src/entries/popup/pages/hw/gridplus/addressChoice.tsx

+      const nonExistingAddresses = fetchedAddresses
+        .map((address) => getAddress(address))
+        .filter(
+          (address) =>
+            !sortedAccounts.map((account) => account.address).includes(address),
+        );


is the idea here to not let the user select accounts that he already have in the extension?
lets say he had the pk in the extension AND in the grid+ lattice?
this should prevent him from importing that account from the gridplus?

Yes, precisely. It resulted in a thrown error when importing the same address another time.

greg-schrammel · 2024-04-20T17:43:47Z

src/entries/popup/pages/hw/gridplus/walletCredentials.tsx

+          deviceId: formData.deviceId,
+          password: formData.password,
+          name: appName,
+          getStoredClient: () => useGridPlusClientStore.getState().client,


why do we have a useGridPlusClientStore and a getStoredGridPlusClient?

Hi @greg-schrammel! Our SDK currently only supports synchronous storages (it's Web Storage API compliant), so in order to upgrade from it, to async Chrome Storage API, we had to utilize a simple Zustand store to make it synchronous.

greg-schrammel · 2024-04-22T13:38:18Z

src/entries/popup/pages/hw/gridplus/addressChoice.tsx

+          (account) => account.address,
+        );
+      } else {
+        fetchedAddresses = (await fetchAddresses()) as Address[];


does this fetchAddresses get the client from some global? how does it know? couldn't find it in the sdk docs

fetchAddresses calls our API. Now, if user permitted Rainbow, it will return n derived addresses of user's wallet. The detailed implementation:
https://github.com/GridPlus/gridplus-sdk/blob/32b5389a768b2efd989a8af892a2b14e837110a8/src/api/addresses.ts#L18

greg-schrammel · 2024-04-22T13:40:31Z

src/entries/popup/pages/hw/gridplus/addressChoice.tsx

+    event.preventDefault();
+    const accountsToImport = formData.selectedAddresses.map((address, i) => ({
+      address,
+      index: i,


is there a way to "add by index"? this index here should be the index of the wallet in the derivation path, not it's index in the array

The index in this view actually relates to the derivation path in Lattice. If there's no override for fetchAddresses starting path, then it starts from address index 0.

github-actions · 2024-05-01T08:20:25Z

Here's the packed extension for this build:
node_modules.tar.gz

DanielSinclair · 2024-05-07T21:20:39Z

Closing in favor of #1538 to run CI and merge this week

mrcnk added 19 commits November 21, 2023 14:57

feat(hw): add gridplus connect option

06bb15b

feat(gridplus): add a gridplus connector route

0e91d8d

feat(gridplus): add nested routes for gridplus onboarding

d840b7e

feat(gridplus): connect gridplus routes

a9969b0

feat(gridplus): unstub onboarding

1bf3bb9

fix(conflicts): resolve

2055931

feat(gridplus): wrap up gridplus onboarding

bf6c40d

feat(gridplus): add styles to addresses choice

cc5f025

feat(gridplus): add signTransactionFromGridPlus and sendTransactionFr…

8ff215d

…omGridPlus

feat(gridplus): add a handler for personal signature

0633f1f

feat(gridplus): wrap up typed data signing

fc486e3

fix(gridplus): fix personal sign

6c0ba38

chore(e2e): add semi-automatic testing for GridPlus

cace361

chore(gridplus): update image assets

44b9a96

feat(gridplus): implement add by index

052c74d

chore(gridplus): improve onboarding ui

36dd231

feat(gridplus): add informative labels for the onboarding

f5c1b8d

chore(gridplus): handle case of removed permissions in Lattice

7e64b4a

chore(gridplus): sync up with latest origin

e2bcecc

brunobar79 reviewed Feb 20, 2024

View reviewed changes

mrcnk added 4 commits February 26, 2024 14:14

Merge branch 'master' into feat/add-gridplus-lattice1

589f968

fix(gridplus): pr improvements

520ce66

fix(gridplus): revert trezor vendor lib linting changes

2c8759c

fix(gridplus): revert checkbox changes

c0a489d

chore(gridplus): add onboarding e2e flow mocks

0ebd883

Merge branch 'master' into feat/add-gridplus-lattice1

25bc5f4

BrodyHughes reviewed Mar 1, 2024

View reviewed changes

e2e/helpers.ts Outdated Show resolved Hide resolved

brunobar79 reviewed Mar 1, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

BrodyHughes reviewed Mar 7, 2024

View reviewed changes

src/entries/popup/pages/hw/gridplus/walletCredentials.tsx Show resolved Hide resolved

src/entries/popup/pages/hw/gridplus/walletCredentials.tsx Outdated Show resolved Hide resolved

src/entries/popup/pages/hw/gridplus/walletCredentials.tsx Show resolved Hide resolved

mrcnk added 2 commits March 11, 2024 15:14

fix(gridplus): apply pr feedback

fe5d6e7

Merge branch 'feat/add-gridplus-lattice1' of github.com:mrcnk/browser…

e20520e

…-extension into feat/add-gridplus-lattice1

Merge branch 'master' into feat/add-gridplus-lattice1

21b75f8

mrcnk added 3 commits April 15, 2024 15:37

fix(gridplus): fix L2 transactions

ade3086

fix(conflicts): resolve

ad91af6

chore(gridplus): update logo in wallets & keys

7b47ed3

greg-schrammel reviewed Apr 20, 2024

View reviewed changes

greg-schrammel reviewed Apr 22, 2024

View reviewed changes

Merge branch 'master' into feat/add-gridplus-lattice1

0d2cca3

greg-schrammel added 3 commits May 7, 2024 17:17

refactor AddressChoice

03accfd

a

b309e86

generate password (?)

52179a6

DanielSinclair mentioned this pull request May 7, 2024

feat: GridPlus Lattice1 support #1538

Draft

DanielSinclair closed this May 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/add GridPlus Lattice1 Hardware Wallet #1346

Feat/add GridPlus Lattice1 Hardware Wallet #1346

mrcnk commented Feb 15, 2024

socket-security bot commented Feb 15, 2024 •

edited

Loading

brunobar79 left a comment

mrcnk commented Feb 28, 2024

socket-security bot commented Feb 29, 2024 •

edited

Loading

github-actions bot commented Feb 29, 2024

BrodyHughes left a comment

BrodyHughes left a comment •

edited

Loading

mrcnk commented Mar 11, 2024

BrodyHughes commented Mar 15, 2024

mrcnk commented Apr 15, 2024

greg-schrammel Apr 20, 2024

mrcnk Apr 22, 2024

greg-schrammel Apr 20, 2024 •

edited

Loading

mrcnk Apr 22, 2024

greg-schrammel Apr 22, 2024

mrcnk Apr 25, 2024

greg-schrammel Apr 22, 2024

mrcnk Apr 25, 2024

github-actions bot commented May 1, 2024

DanielSinclair commented May 7, 2024

Feat/add GridPlus Lattice1 Hardware Wallet #1346

Feat/add GridPlus Lattice1 Hardware Wallet #1346

Conversation

mrcnk commented Feb 15, 2024

What changed (plus any additional context for devs)

Screen recordings / screenshots

What to test

socket-security bot commented Feb 15, 2024 • edited Loading

Next steps

brunobar79 left a comment

Choose a reason for hiding this comment

mrcnk commented Feb 28, 2024

socket-security bot commented Feb 29, 2024 • edited Loading

github-actions bot commented Feb 29, 2024

BrodyHughes left a comment

Choose a reason for hiding this comment

BrodyHughes left a comment • edited Loading

Choose a reason for hiding this comment

mrcnk commented Mar 11, 2024

BrodyHughes commented Mar 15, 2024

mrcnk commented Apr 15, 2024

greg-schrammel Apr 20, 2024

Choose a reason for hiding this comment

mrcnk Apr 22, 2024

Choose a reason for hiding this comment

greg-schrammel Apr 20, 2024 • edited Loading

Choose a reason for hiding this comment

mrcnk Apr 22, 2024

Choose a reason for hiding this comment

greg-schrammel Apr 22, 2024

Choose a reason for hiding this comment

mrcnk Apr 25, 2024

Choose a reason for hiding this comment

greg-schrammel Apr 22, 2024

Choose a reason for hiding this comment

mrcnk Apr 25, 2024

Choose a reason for hiding this comment

github-actions bot commented May 1, 2024

DanielSinclair commented May 7, 2024

socket-security bot commented Feb 15, 2024 •

edited

Loading

socket-security bot commented Feb 29, 2024 •

edited

Loading

BrodyHughes left a comment •

edited

Loading

greg-schrammel Apr 20, 2024 •

edited

Loading