Skip to content
This repository has been archived by the owner on Nov 21, 2019. It is now read-only.

Add "Display address on TREZOR" #829

Merged
merged 3 commits into from
Aug 9, 2017
Merged

Add "Display address on TREZOR" #829

merged 3 commits into from
Aug 9, 2017

Conversation

karelbilek
Copy link
Contributor

Finally got around to this

#651

The actual change is fairly small, most of the PR is the new trezorconnect script, plus the translations.

I have added the link to the balance directive, since that is where the address is.

@tayvano tayvano merged commit b027525 into MyEtherWallet:mercury Aug 9, 2017
@tayvano
Copy link
Contributor

tayvano commented Aug 9, 2017

Kickass! Can you let me know when signing is enabled via connect? We are adding more and more support to https://www.myetherwallet.com/signmsg.html

@karelbilek
Copy link
Contributor Author

We are now adding it :) however, there are some issues with compatibility with other wallets, we will discuss that

@tayvano
Copy link
Contributor

tayvano commented Aug 9, 2017

You are amazing. Let us know if we can do anything to assist.

@karelbilek
Copy link
Contributor Author

Here is the issue

ethereum/go-ethereum#14794

We are using length as bitcoin varint. So our signatures are not compatible with geth signatures (which are incompatible with myetherwallet signatures).

@tayvano
Copy link
Contributor

tayvano commented Aug 9, 2017

I was talking to Aaron @ MetaMask earlier regarding this. There is no real formal signing spec (that isn't inherently flawed?).

We will do our best to match whatever standard emerges, so please do not worry about doing something differently to work for us specifically—it's far easier for us to adapt than you / geth. 😉

Aaron and Dan at MetaMask have some opinions on the matter. I'll point them to that thread to chime in as necessary.

@tayvano
Copy link
Contributor

tayvano commented Aug 9, 2017

@Runn1ng For some reason when I compile for production (gulp prep) it throws errors. Any ideas (I'm not operating at full capacity atm, but can look again in the AM)

screen shot 2017-08-09 at 5 32 34 am

@karelbilek
Copy link
Contributor Author

I will look at it

@karelbilek
Copy link
Contributor Author

#835

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants