Update trezor connect to enable passphrase via device

ghost commented 3 years ago

Both trezor wallet models allow for an optional passphrase. The model T has the advantage that it allows entering the passphrase via the touch screen on the device itself instead of inside the browser. It seems like we are using trezor connect version 7.0.5 in this repo which does not support typing passphrases on the device but just the browser which is not ideal from a security perspective. It has another disadvantage, which is that the passphrase also needs to be typed in for making a transaction.

I propose updating trezor connect to the latest stable version (https://www.npmjs.com/package/trezor-connect/v/8.1.14) which should resolve both mentioned issues.

Here a comparison of what can be seen in the browser when connecting with a trezor model t and passphrase enabled on yearn.finance (which uses version 7 of trezor connect):

vs myetherwallet which uses version 8 of trezor connect:

saltyfacu commented 3 years ago

How does it work when you use it connected through Metamask? Does it have the expected behaviour?

ghost commented 3 years ago

Hi fameal. Same issue on metamask (though that is a problem on their side cause they also haven't updated to trezor connect 8 yet: https://github.com/MetaMask/metamask-extension/issues/8349)

Take a look at myetherwallet, they use a connect version >= 8 and there it works fine. Same on synthetix.

saltyfacu commented 3 years ago

Yes, but we are using web3-react and they have not updated the trezor module.

ghost commented 3 years ago

I am not familiar with web-dev (doing backend only). tbh, I am not sure if I get this, can you explain further pls? The web3-react trezor connector (6.1.6: https://www.npmjs.com/package/@web3-react/trezor-connector) has trezor connect as a dependency. Our package-lock.json file has "@web3-react/trezor-connector" and trezor-connect version 7.0.5 hardcoded. When I install the app via npm install, it installs whatever is specified in package-lock.json right?

So why can't we just update the trezor-connect dependency to lets say 8.1.14 (the latest version)? I actually tested this and changed this:

    "trezor-connect": {
      "version": "7.0.5",
      "resolved": "https://registry.npmjs.org/trezor-connect/-/trezor-connect-7.0.5.tgz",
      "integrity": "sha512-cGHcNuO/kGVF6b1mp5VB/RwXcXwqZJDPLp3opx7vM+BQ8xB4oDAUdL+T8aCKRbDv6HwP/wvGwoaok/+9kYOPfA==",
      "requires": {
        "@babel/runtime": "^7.3.1",
        "events": "^3.0.0",
        "whatwg-fetch": "^3.0.0"
      }

to (cp'd from here: https://github.com/MyEtherWallet/MyEtherWallet/blob/master/package-lock.json)

    "trezor-connect": {
      "version": "8.1.14",
      "resolved": "https://registry.npmjs.org/trezor-connect/-/trezor-connect-8.1.14.tgz",
      "integrity": "sha512-Y0yY7Mc8iN/feLct2bnJNEIbFbbMC+bCddQ7vMf22ygpcl/plNnvnze03mi6+7hEk3FmiR66gi12INSOr0wBHw==",
      "requires": {
        "@babel/runtime": "^7.11.0",
        "events": "^3.2.0",
        "whatwg-fetch": "^3.3.1"
      },

After installing it (I removed the previous installed node_modules) it is using trezor.io/8 instead of 7 and allows me to type in the passphrase via the device, which is exactly what I wanted.

Is there anything that I miss here? Thanks for looking into this! :)

saltyfacu commented 3 years ago

I'm testing that, but installing the package with npm. I will test it in a short while.

ghost commented 3 years ago

Just tried it on the mirrored y.finance site and works as expected :) Thanks for the upgrade guys! I am closing the issue.

yearn / iearn-finance

Update trezor connect to enable passphrase via device #97