Skip to content

Commit

Permalink
show bip39 warning and add info about checksum disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
ecdsa committed Aug 16, 2017
1 parent 322587e commit 8b194cd
Showing 1 changed file with 19 additions and 16 deletions.
35 changes: 19 additions & 16 deletions gui/qt/seed_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,21 @@ def seed_options(self):
vbox.addWidget(cb_ext)
if 'bip39' in self.options:
def f(b):
self.is_seed = (lambda x: bool(x)) if b else self.saved_is_seed
self.on_edit()
if b:
msg = ' '.join([
'<b>' + _('Warning') + '</b>' + ': ',
_('BIP39 seeds may not be supported in the future.'),
'<b>' + _('Warning') + ': BIP39 seeds are dangerous!' + '</b><br/><br/>',
_('BIP39 seeds can be imported in Electrum so that users can access funds locked in other wallets.'),
_('However, BIP39 seeds do not include a version number, which compromises compatibility with future wallet software.'),
'<br/><br/>',
_('As technology matures, Bitcoin address generation may change.'),
_('However, BIP39 seeds do not include a version number.'),
_('As a result, it is not possible to infer your wallet type from a BIP39 seed.'),
'<br/><br/>',
_('We do not guarantee that BIP39 seeds will be supported in future versions of Electrum.'),
_('We recommend to use seeds generated by Electrum or compatible wallets.'),
_('We do not guarantee that BIP39 imports will always be supported in Electrum.'),
_('In addition, Electrum does not verify the checksum of BIP39 seeds; make sure you type your seed correctly.'),
])
#self.parent.show_warning(msg)
self.seed_type_label.setVisible(not b)
self.is_seed = (lambda x: bool(x)) if b else self.saved_is_seed
self.on_edit()
else:
msg = ''
self.seed_warning.setText(msg)

cb_bip39 = QCheckBox(_('BIP39 seed'))
cb_bip39.toggled.connect(f)
cb_bip39.setChecked(self.is_bip39)
Expand Down Expand Up @@ -130,9 +129,10 @@ def __init__(self, seed=None, title=None, icon=True, msg=None, options=None, is_
hbox.addWidget(passphrase_e)
self.addLayout(hbox)
self.addStretch(1)
self.seed_warning = WWLabel('')
self.addWidget(self.seed_warning)
if msg:
msg = seed_warning_msg(seed)
self.addWidget(WWLabel(msg))
self.seed_warning.setText(seed_warning_msg(seed))

def get_seed(self):
text = unicode(self.seed_e.text())
Expand All @@ -142,8 +142,11 @@ def on_edit(self):
from electrum.bitcoin import seed_type
s = self.get_seed()
b = self.is_seed(s)
t = seed_type(s)
label = _('Seed Type') + ': ' + t if t else ''
if not self.is_bip39:
t = seed_type(s)
label = _('Seed Type') + ': ' + t if t else ''
else:
label = 'BIP39 (checksum disabled)'
self.seed_type_label.setText(label)
self.parent.next_button.setEnabled(b)

Expand Down

3 comments on commit 8b194cd

@fyookball
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly disagree with disabling the checksum. Some checksum is better than none. It's only a matter of time before someone loses money because of a typo. The scenario is this: A user has an existing bip39 seed that he uses for cold storage... he inputs it on a cold storage machine to get an address, makes a typo, sends money to the wrong address, and then removes the wallet (because he knows the seed).

The chances of something like this happening are much higher than someone getting a false positive from a weak checksum. I don't understand the reasoning here... seems like a terrible decision!

@dabura667
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a "RECOVERY" method.

Your concern is only valid if:

  1. Someone generates a BIP39 phrase somewhere.
  2. They never look at any addresses and never use it once in the wallet app that generated it.
  3. They then for some reason decide to "Recover" it in Electrum.

Electrum's BIP39 is there SOLELY to assist Trezor (etc) users in case they lose their device. Which is recovery. Which means if they don't see money they'll just try again until they see their money.

@ecdsa
Copy link
Member Author

@ecdsa ecdsa commented on 8b194cd Aug 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fyookball you are commenting on a commit that was later superseded.

Please sign in to comment.