wip(feat): add trezor support

[rixcian]

Hey @hazae41, I would like to add support for Trezor hardware wallets. The feature is almost fully done (the basic import as you can see on the video down below), I just need to handle all corner cases and tweak the UI.

I also changed the UX flow of "importing a wallet" from a hardware wallet. I know you're currently doing it through the "Seeds" page, but I think that from the user's point of view it's quite complicated, personally I didn't get it for the first time why there's the "Seeds" page at all. Now, I understand that the reason is to be able to generate different wallets from the seed that is saved on the hardware wallet.

So I decided to add the option of importing a hardware wallet directly from the "Wallets" page (as you can see on the video). Would like to know your opinion on this change.

https://github.com/brumewallet/wallet/assets/14358496/c7ef008c-61d1-4c22-9258-be4676eba901

Right now I'm struggling with one UX thing. When the list of addresses (derived from seed on the hardware wallet) appears in the UI, I want to disable those, which are already imported in Brume.

To check which address is already imported into Brume, I need to get all Brume wallet addresses from the storage. I can use the useWallets hook, but it returns me just UUID for each wallet (I need the address of the wallet).

This is a part of the code where I want to check if the address is already imported (src/mods/foreground/entities/wallets/all/create/hardware.tsx).

So, how could I query all user wallets from the IndexedDB storage?

Thanks in advance for any advice! ❤️

[rixcian]

What I'm planning to do next:

• [ ] Check that the user doesn't import already imported wallet
• [ ] Add option to import wallet with on certain index (like it's done under the "Seeds" page)
• [ ] Display correctly the wallet balances in the list (currently it's displaying just "0 ETH")
• [ ] Refactor code & tweak the UI

[hazae41]

Hey thanks for the help

To resolve a wallet you have to use useWallet(uuid)

For the Trezor feature, for supply-chain security reasons we can't add 141 dependencies to our graph, someone needs to reimplement it with a better supply-chain

I would have gladly reimplemented it but I can't since I do not own a Trezor (yet) :(

The other stuff seems good I will check this out today

Thanks for your contribution 🫡

[hazae41]

Also I do not currently check if the address is duplicated when importing from Ledger or generating from seed or importing from private key, it's best to mimic that behavior for now

[hazae41]

"Display correctly the wallet balances in the list (currently it's displaying just "0 ETH")"

You have to take extra care not to fetch all wallets at once, to avoid the bad guys from linking wallets together

[hazae41]

I partially merged

[rixcian]

To resolve a wallet you have to use useWallet(uuid)

That's a problem as well. I'm gonna get all the wallet UUIDs via useWallets then I need to loop through those UUIDs and call useWallet, but I cannot call hooks inside functions. This is a good way when you want to render a component for each wallet, but not when you just want to get the information.

For the Trezor feature, for supply-chain security reasons we can't add 141 dependencies to our graph, someone needs to reimplement it with a better supply-chain

I would have gladly reimplemented it but I can't since I do not own a Trezor (yet)

I'll look at it by myself and try to re-implement it.

"Display correctly the wallet balances in the list (currently it's displaying just "0 ETH")"

You have to take extra care not to fetch all wallets at once, to avoid the bad guys from linking wallets together

Good point 👍

[hazae41]

I'll look at it by myself and try to re-implement it.

Nice, thanks, you MAY start implementing it in libs just like I did with Ledger

That's a problem as well. I'm gonna get all the wallet UUIDs via useWallets then I need to loop through those UUIDs and call useWallet, but I cannot call hooks inside functions. This is a good way when you want to render a component for each wallet, but not when you just want to get the information.

You can use imperative style with getWallet(uuid) (Glacier 2.0 is still undocumented for now so feel free to ask questions)

Also, I just finished writing CONTRIBUTING.md let me know what you think

[rixcian]

You can use imperative style with getWallet(uuid) (Glacier 2.0 is still undocumented for now so feel free to ask questions)

If I'm understanding it correctly, you have to still use the getWallet function inside a useQuery func, which is also a react hook (screenshot down below). So it doesn't fix the problem or am I missing something?

Also, I just finished writing CONTRIBUTING.md let me know what you think

I don't have any problems with the contribution guidelines, I've got it that this is a privacy/security aligned project.

[hazae41]

If I'm understanding it correctly, you have to still use the getWallet function inside a useQuery func, which is also a react hook (screenshot down below). So it doesn't fix the problem or am I missing something?

The getWallet returns a query object, you can use that object directly

[hazae41]

I have good news: I just bought a Trezor so I will be able to help you

[rixcian]

This is the lib where the communication with the Trezor is implemented: https://github.com/trezor/trezor-suite/tree/f5286f61368a8970bf45fc78ccb6cd7a8712ab0c/packages/transport

The communication is described there (in README.md) from a high-level point of view. I guess that's something that we're looking for and what we want to re-implement.

They're using protocol buffers for message standardization. Here's the example test, they have (for sending/receiving msgs): https://github.com/trezor/trezor-suite/blob/f5286f61368a8970bf45fc78ccb6cd7a8712ab0c/packages/transport/tests/abstractUsb.test.ts#L323

Here are the proto definitions for the messages (the link will redirect you directly to proto definition that we gonna use for getting eth address): https://github.com/trezor/trezor-suite/blob/f5286f61368a8970bf45fc78ccb6cd7a8712ab0c/packages/protobuf/messages.json#L3943

I hope you'll find this information helpful for further research/implementation.

[rixcian]

Here are the constants (primarly VENDOR_ID) needed for filtering the devices (via navigator.usb.requestDevice()): https://github.com/trezor/trezor-suite/blob/f5286f61368a8970bf45fc78ccb6cd7a8712ab0c/packages/transport/src/constants.ts#L6

[hazae41]

Nice, we can install https://github.com/protobufjs/protobuf.js for protocol buffers

[Wjxfi]

Any news ?

[hazae41]

Any news ?

I will add soon :)