Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add Trezor to wallets #545

Merged
merged 3 commits into from Sep 19, 2014

Conversation

Projects
None yet
6 participants
Contributor

saivann commented Sep 8, 2014

This pull request adds TREZOR to the list of wallets, enables the "Hardware" entry in the menus and drops the "soon-to-be-released" list of hardware wallets on the "Secure your wallet" page.

Live previews: (Merged)

Contributor

harding commented Sep 9, 2014

LGTM. Thanks!

providing high level of security

Feels like it's missing an "a"

providing a high level of security

Contributor

saivann commented Sep 10, 2014

@harding You're better than me at evaluating if @christophebiocca's suggestion is better English. Additionally, shouldn't we say "s/a hardware wallet/an hardware wallet/"?

Contributor

harding commented Sep 10, 2014

@saivann the suggestion by @christophebiocca is correct (thanks, @christophebiocca) so I rebase-amended the commit.

"A hardware" is also correct because (in English) the H is pronounced---making the first sound a consonant sound. With another H word where the H isn't pronounced, "an" would be correct. For example: it has been an honor to work with you all.

Contributor

saivann commented Sep 10, 2014

@harding Thanks! I rebased this (again) to fix the merge conflict with breadwallet.

Contributor

saivann commented Sep 10, 2014

In the absence of critical feedback, this pull request will be merged on September 17th.

Contributor

schildbach commented Sep 10, 2014

LGTM

Contributor

slush0 commented Sep 13, 2014

Are there any specific rules for "New app" flag? Firmware is published for half a year already (which is quite significant time in Bitcoin world), trezor-crypto (heart of Trezor) is already used in other applications, everything is covered by unit tests and we already have some peer reviewers checking our code. I think most of applications (especially those with "Centralized" flags) are even more vulnerable to bugs coming from new/untested code because of complete lack of peer reviews, and they don't have "New app" flag...

Contributor

slush0 commented Sep 13, 2014

Otherwise we're okay with this pull request.

Contributor

harding commented Sep 13, 2014

@slush0 the rules are in the "passing score" part of the Transparency section of the site README: https://github.com/bitcoin/bitcoin.org#score

The relevant quote is: "The codebase must be public since at least 6 months and previous commits must remain unchanged."

Although its not stated, I think @saivann has been applying that to mean, "...codebase must be public [and the wallet must be in general public use] since at least 6 months..." (If that's correct, we should make it explicit. The reason we measure from first general public use is that it would be unreasonable to expect volunteers to inspect prototype wallets that may not ever get a general release.)

Looking at the repository, the firmware's first commit is April 29th (4.5 months) and I think Trezors only became generally available about 1 month ago.

I agree that it would be preferable to distinguish between wallets that receive significant peer review and those that probably don't receive any significant reviews---unfortunately, I don't know of any easy and objective way to measure that.

As a footnote, Trezor firmware can be built deterministically, correct? If so, I think you'll be the first wallet besides Bitcoin Core to get a "Good" Transparency score, once the six-month review period expires.

Contributor

slush0 commented Sep 13, 2014

@harding Thanks for explanation. You're right, 4.5 months, my bad. However we published sources when we started shipping preorders, which means hundreds devices in the wild.

You're right that builds are deterministic (since firmware 1.2.0). I'll open pull request abou removing "New app" flag in ~2 months.

Contributor

saivann commented Sep 13, 2014

@harding Making this more explicit makes sense, thanks! How about "codebase and final releases must be public since at least 6 months,"

Maybe this can be discussed when @slush0 opens a pull request in a few months, but hardware devices are different than only software in that (to my knowledge) the device can lie and you have no way to prove if it really uses the right firmware. If I'm not mistaken here, the current "good" score would not be accurate; ("... any developer in the world can ... make sure the final software isn't hiding any secrets"). So maybe a bit more discussions on the topic would make sense.

Contributor

slush0 commented Sep 13, 2014

@saivann Its exact oposite, thanks to deterministic nature of Trezor firmware and deterministic build environment. If you're extra paranoid, you can reflash device with your custom firmware which just blinks the display, then you can reflash the device with proper, signed, deterministically build firmware and you are 100% sure that its using proper firmware.

Contributor

harding commented Sep 13, 2014

@saivann that phrasing sounds good to me.

More discussion on the topic of hardware/firmware determinism does seem warranted. I do think you're right about that being a discussion most appropriate to a future pull request.

saivann added a commit that referenced this pull request Sep 19, 2014

@saivann saivann merged commit 96d7722 into master Sep 19, 2014

@saivann saivann deleted the trezor branch Sep 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment