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

Trezor wallet is not supported. #4911

Closed
aeyakovenko opened this issue Jul 2, 2019 · 25 comments
Closed

Trezor wallet is not supported. #4911

aeyakovenko opened this issue Jul 2, 2019 · 25 comments
Labels
good first issue Good for newcomers
Milestone

Comments

@aeyakovenko
Copy link
Member

Problem

Trezor wallet is not supported.

Proposed Solution

Add support for the cli to use the wallet.

@mvines
Copy link
Member

mvines commented Oct 17, 2019

A potential starting point: https://github.com/paritytech/parity-ethereum/pull/6403/files

@vkomenda
Copy link
Contributor

vkomenda commented Oct 17, 2019

To continue the analogy, can it somehow be built on top of ledger-tool? No, I confused it with Ledger support.

@mvines
Copy link
Member

mvines commented Oct 17, 2019

Yeah I'd say either a stand-alone tool to begin with or potentially a subcommand of the cli/

@vkomenda
Copy link
Contributor

We should keep in mind that more recently Parity have completely removed hardware wallet support. The point being that supporting it was not possible in a non-interactive environment. It looks like the required interactivity had been removed prior to that in a process of decoupling account management from the blockchain engine. It's probably better to try keeping anything that requires interaction inside cli or maybe introduce another subcrate? hw?

@aeyakovenko
Copy link
Member Author

@vkomenda can v0 simply proxy over from then cli to the HSM?

@vkomenda
Copy link
Contributor

@aeyakovenko what is HSM?

Do we need to add Solana support to Trezor firmware?

@aeyakovenko
Copy link
Member Author

@vkomenda HSM is just an acronym - Hardware Security Module. For v0, I think we can just use Trezor to store the keys and provide signatures for message that the CLI requests.

@vkomenda
Copy link
Contributor

Is it OK to limit Trezor support to firmware version 1.7.0 and above? Reference Parity code might have worked up to versions 1.6.x but it's definitely not compatible with versions starting 1.7.0 because USB IDs and some magic numbers changed. I think we might expect users to have upgraded their device firmware to the latest version.

@aeyakovenko
Copy link
Member Author

@vkomenda that works for us.

@stale
Copy link

stale bot commented Oct 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Oct 22, 2020
@stale
Copy link

stale bot commented Oct 29, 2020

This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Oct 29, 2020
@jcoffland
Copy link

I'd like to see this issue reopened. Support for Trezor One from the command line tools would be great. Limiting support to a more recent version of the Trezor firmware, as suggested by @vkomenda, would be fine. I see that Solana uses ed25519 signatures and it looks like the Trezor firmware supports this.

I think it would be ideal if the solana command line tools called the trezord HTTP API on the default port. This would allow you to use trezord for both web apps and Solana CLI at the same time. Trezord has several endpoints listed here: https://github.com/trezor/trezord-go#api-documentation These endpoints exchange data using protobufs defined here: https://github.com/trezor/trezor-common/blob/master/protob/messages.proto

This documentation looks particularly useful: https://wiki.trezor.io/Developers_guide:Message_Workflows

@jcoffland
Copy link

It looks like modifications to the Trezor firmware will be needed. Here is some info on contributing support for new coins from the Trezor site: https://wiki.trezor.io/Developers_guide:Contributing

@mvines mvines reopened this Feb 26, 2021
@stale stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Feb 26, 2021
@jcoffland
Copy link

I created a PR for the trezor-firmware that adds Solana support. Note, recent trezor firmwares only work with the model T. This is because the model 1 does not have enough FLASH space.

@t-nelson
Copy link
Contributor

AFAIK Trezor isn't accepting new coin support ATM. Since Solana didn't fork any previous network's transaction format, that's what we'd need to do to add support.

Other

At the moment, we do not have the capacity to add new coins that do not fit the aforementioned
category. Our current product goal is to unite what we support in firmware and in Trezor Suite
and since firmware is way ahead of Suite we want to pause for a bit, implement the remaining
coins into Suite and then consider adding new coins. This effectively means that our team will
not be accepting any requests to add new cryptocurrencies. 

@Arrowana
Copy link
Contributor

Arrowana commented Jul 1, 2021

What about routing signing through some sort of mechanism which gets a serialized transaction and call a signTransaction which can be anything.

The idea is to offload any custom signing code from the cli itself, a bit like how a Dapp signs, it doesn't care about what web wallet is used and only issues a signTransaction call and gets the signed transaction back

Scenario:

  • Anything that doesn't exist yet in a front-end can be signed using a web/hardware wallet through cli.
  • The problem of multisig not supported for all cli tools, just offload to this sign process?

For the trezor firmware being "frozen" to new chains that is annoying, is it worth having a fork?

@jcoffland
Copy link

A fork is a good idea.

@t-nelson
Copy link
Contributor

t-nelson commented Jul 1, 2021

I'm not sure what good a fork would do? You still need Trezor signed bins to install them on a device AFAIK

@jcoffland
Copy link

jcoffland commented Jul 1, 2021

@Arrowana The problem with having the device sign transactions it doesn't understand is that malicious software on the connected device could steal all your coin by modifying the transaction. If the signing device understands the transaction it can check what it does and ask the user to confirm that they really want to do that thing.

One solution to this is to display some representation of the transaction (a hash or word phrase) that can be verified through a side channel. Maybe with some sort of two factor authentication.

Ultimately, as the user you have to trust some software outside of the signing device. How do you know the contract you're committing funds to does what you think it's going to do unless you've picked the code apart yourself? How do you know the receive addresses you're using are correct? Maybe your browser was hacked and is modifying them.

Hardware wallets can really only do so much.

@Arrowana
Copy link
Contributor

Arrowana commented Jul 1, 2021

@jcoffland I never said do not sign what you don't understand/can't see. However, that is pretty much standard now since the ledger cannot decode much.

I think the only sane way to verify the transaction is to have the wallet decode it, which is really up to the wallet. Anything else seems like a borderline compromise

Imagine if we had a solana hardware wallet in embedded rust, we could just use the solana-labs crates for decoding and format the output, done.

@jcoffland
Copy link

So you trust the web wallet?

@Arrowana
Copy link
Contributor

Arrowana commented Jul 1, 2021

So you trust the web wallet?

"the [hardware] wallet decodes it"

On the web wallet side, I easily trust a web wallet like sollet for reasonable amount of funds, like defi tinkering, since the code is open source and I spent some time adding features to it.

@jcoffland
Copy link

To trust the web wallet requires more than just trusting the JavaScript code. You also need to trust your browser. And to trust your browser you need to trust your OS and your hardware and the chips on your mother board, etc.

Anyway, I think we're really on the same page. I actually suggested to the Trezor guys that Trezor keep it simple and just sign a transaction hash without the hardware wallet knowing what it was. They didn't like this idea at all. My reasoning is that it's too difficult to create code that can understand all the possible Solana transactions on a simple piece of hardware like Trezor. Ultimately, you're trusting some defi website to create the correct transaction anyway. The next step is to trust that the transaction that website created made it to your hardware wallet unaltered. Verifying a hash on the Trezor screen can help with this. To make it more secure you should also communicate the transaction hash through a side channel. Say by sending it in an SMS to your phone. So if the website showed me a transaction hash and sent me an SMS of the hash and both matched what I saw on my Trezor, I think I could be reasonably happy to approve the transaction without the Trezor understanding the transaction. If you can accept this, implementing support for new coins on Trezor is pretty easy.

@jcrsilva
Copy link

Seems like Trezor added official Solana support back in Dec 2023.

Maybe adding support for Trezor would be simpler now.

@t-nelson
Copy link
Contributor

there's nothing to "add" here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants