Clean up and fix account adding

As per BIP44, 20 addresses are checked for transactions, not just the
first one.
Show the last account only if used or named.
If all accounts are used, prompt for password to create new one.

Fixes #1128

gui/qt/main_window.py

@@ -167,6 +167,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             self.console.showMessage(self.network.banner)
 
         self.payment_request = None
+        self.checking_accounts = False
         self.qr_window = None
         self.not_enough_funds = False
         self.pluginsdialog = None
@@ -263,9 +264,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         self.need_update.set()
         # Once GUI has been initialized check if we want to announce something since the callback has been called before the GUI was initialized
         self.notify_transactions()
-        self.update_account_selector()
         # update menus
-        self.new_account_menu.setVisible(self.wallet.can_create_accounts())
+        self.update_new_account_menu()
         self.export_menu.setEnabled(not self.wallet.is_watching_only())
         self.password_menu.setEnabled(self.wallet.can_change_password())
         self.seed_menu.setEnabled(self.wallet.has_seed())
@@ -511,8 +511,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
 
     def timer_actions(self):
         if self.need_update.is_set():
-            self.update_wallet()
             self.need_update.clear()
+            self.update_wallet()
         # resolve aliases
         self.payto_e.resolve()
         # update fee
@@ -589,6 +589,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         self.update_status()
         if self.wallet.up_to_date or not self.network or not self.network.is_connected():
             self.update_tabs()
+        if self.wallet.up_to_date:
+            self.check_next_account()
 
     def update_tabs(self):
         self.history_list.update()
@@ -1489,15 +1491,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         menu.addAction(_("Rename"), lambda: self.edit_account_label(k))
         if self.wallet.seed_version > 4:
             menu.addAction(_("View details"), lambda: self.show_account_details(k))
-        if self.wallet.account_is_pending(k):
-            menu.addAction(_("Delete"), lambda: self.delete_pending_account(k))
         menu.exec_(self.address_list.viewport().mapToGlobal(position))
 
-    def delete_pending_account(self, k):
-        self.wallet.delete_pending_account(k)
-        self.address_list.update()
-        self.update_account_selector()
-
     def create_receive_menu(self, position):
         selected = self.address_list.selectedItems()
         multi_select = len(selected) > 1
@@ -1933,30 +1928,47 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         if self.set_contact(unicode(line2.text()), str(line1.text())):
             self.tabs.setCurrentIndex(4)
 
+    def update_new_account_menu(self):
+        self.new_account_menu.setVisible(self.wallet.can_create_accounts())
+        self.new_account_menu.setEnabled(self.wallet.permit_account_naming())
+        self.update_account_selector()
 
-    @protected
-    def new_account_dialog(self, password):
-        dialog = WindowModalDialog(self, _("New Account"))
+    def new_account_dialog(self):
+        dialog = WindowModalDialog(self, _("New Account Name"))
         vbox = QVBoxLayout()
-        vbox.addWidget(QLabel(_('Account name')+':'))
+        msg = _("Enter a name to give the account.  You will not be "
+                "permitted to create further accounts until the new account "
+                "receives at least one transaction.") + "\n"
+        label = QLabel(msg)
+        label.setWordWrap(True)
+        vbox.addWidget(label)
         e = QLineEdit()
         vbox.addWidget(e)
-        msg = _("Note: Newly created accounts are 'pending' until they receive bitcoins.") + " " \
-            + _("You will need to wait for 2 confirmations until the correct balance is displayed and more addresses are created for that account.")
-        l = QLabel(msg)
-        l.setWordWrap(True)
-        vbox.addWidget(l)
         vbox.addLayout(Buttons(CancelButton(dialog), OkButton(dialog)))
         dialog.setLayout(vbox)
-        r = dialog.exec_()
-        if not r:
-            return
-        name = str(e.text())
-        self.wallet.create_pending_account(name, password)
-        self.address_list.update()
-        self.update_account_selector()
-        self.tabs.setCurrentIndex(3)
+        if dialog.exec_():
+            self.wallet.set_label(self.wallet.last_account_id(), str(e.text()))
+            self.address_list.update()
+            self.tabs.setCurrentIndex(3)
+            self.update_new_account_menu()
 
+    def check_next_account(self):
+        if self.wallet.needs_next_account() and not self.checking_accounts:
+            try:
+                self.checking_accounts = True
+                msg = _("All the accounts in your wallet have received "
+                        "transactions.  Electrum must check whether more "
+                        "accounts exist; one will only be shown if "
+                        "it has been used or you give it a name.")
+                self.show_message(msg, title=_("Check Accounts"))
+                self.create_next_account()
+                self.update_new_account_menu()
+            finally:
+                self.checking_accounts = False
+
+    @protected
+    def create_next_account(self, password):
+        self.wallet.create_next_account(password)
 
     def show_master_public_keys(self):
         dialog = WindowModalDialog(self, "Master Public Keys")

lib/account.py

@@ -75,6 +75,10 @@ class Account(object):
     def redeem_script(self, for_change, n):
         return None
 
+    def is_used(self, wallet):
+        addresses = self.get_addresses(False)
+        return any(wallet.address_is_old(a, -1) for a in addresses)
+
     def synchronize_sequence(self, wallet, for_change):
         limit = wallet.gap_limit_for_change if for_change else wallet.gap_limit
         while True:
@@ -94,36 +98,6 @@ class Account(object):
         self.synchronize_sequence(wallet, True)
 
 
-class PendingAccount(Account):
-    def __init__(self, v):
-        self.pending_address = v['address']
-        self.change_pubkeys = []
-        self.receiving_pubkeys = [ v['pubkey'] ]
-
-    def synchronize(self, wallet):
-        return
-
-    def get_addresses(self, is_change):
-        return [] if is_change else [self.pending_address]
-
-    def has_change(self):
-        return False
-
-    def dump(self):
-        return {'pending':True, 'address':self.pending_address, 'pubkey':self.receiving_pubkeys[0] }
-
-    def get_name(self, k):
-        return _('Pending account')
-
-    def get_master_pubkeys(self):
-        return []
-
-    def get_type(self):
-        return _('pending')
-
-    def get_xpubkeys(self, for_change, n):
-        return self.get_pubkeys(for_change, n)
-
 class ImportedAccount(Account):
     def __init__(self, d):
         self.keypairs = d['imported']
@@ -399,5 +373,3 @@ class Multisig_Account(BIP32_Account):
 
     def get_type(self):
         return _('Multisig %d of %d'%(self.m, len(self.xpub_list)))
-
-

lib/synchronizer.py

@@ -178,6 +178,4 @@ class Synchronizer(ThreadJob):
         up_to_date = self.is_up_to_date()
         if up_to_date != self.wallet.is_up_to_date():
             self.wallet.set_up_to_date(up_to_date)
-            if up_to_date:
-                self.wallet.save_transactions(write=True)
             self.network.trigger_callback('updated')

lib/wallet.py

@@ -25,6 +25,7 @@ import time
 import json
 import copy
 from functools import partial
+from i18n import _
 
 from util import NotEnoughFunds, PrintError, profiler
 
@@ -291,6 +292,7 @@ class Abstract_Wallet(PrintError):
     def load_accounts(self):
         self.accounts = {}
         d = self.storage.get('accounts', {})
+        removed = False
         for k, v in d.items():
             if self.wallet_type == 'old' and k in [0, '0']:
                 v['mpk'] = self.storage.get('master_public_key')
@@ -300,12 +302,11 @@ class Abstract_Wallet(PrintError):
             elif v.get('xpub'):
                 self.accounts[k] = BIP32_Account(v)
             elif v.get('pending'):
-                try:
-                    self.accounts[k] = PendingAccount(v)
-                except:
-                    pass
+                removed = True
             else:
                 self.print_error("cannot load account", v)
+        if removed:
+            self.save_accounts()
 
     def synchronize(self):
         pass
@@ -313,8 +314,17 @@ class Abstract_Wallet(PrintError):
     def can_create_accounts(self):
         return False
 
-    def set_up_to_date(self,b):
-        with self.lock: self.up_to_date = b
+    def needs_next_account(self):
+        return self.can_create_accounts() and self.accounts_all_used()
+
+    def permit_account_naming(self):
+        return self.can_create_accounts()
+
+    def set_up_to_date(self, up_to_date):
+        with self.lock:
+            self.up_to_date = up_to_date
+        if up_to_date:
+            self.save_transactions(write=True)
 
     def is_up_to_date(self):
         with self.lock: return self.up_to_date
@@ -645,15 +655,6 @@ class Abstract_Wallet(PrintError):
         amount = max(0, sendable - fee)
         return amount, fee
 
-    def get_account_name(self, k):
-        return self.labels.get(k, self.accounts[k].get_name(k))
-
-    def get_account_names(self):
-        account_names = {}
-        for k in self.accounts.keys():
-            account_names[k] = self.get_account_name(k)
-        return account_names
-
     def get_account_addresses(self, acc_id, include_change=True):
         if acc_id is None:
             addr_list = self.addresses(include_change)
@@ -1101,7 +1102,6 @@ class Abstract_Wallet(PrintError):
         self.storage.write()
 
     def wait_until_synchronized(self, callback=None):
-        from i18n import _
         def wait_for_wallet():
             self.set_up_to_date(False)
             while not self.is_up_to_date():
@@ -1125,8 +1125,19 @@ class Abstract_Wallet(PrintError):
         else:
             self.synchronize()
 
+    def accounts_to_show(self):
+        return self.accounts.keys()
+
     def get_accounts(self):
-        return self.accounts
+        return {a_id: a for a_id, a in self.accounts.items()
+                if a_id in self.accounts_to_show()}
+
+    def get_account_name(self, k):
+        return self.labels.get(k, self.accounts[k].get_name(k))
+
+    def get_account_names(self):
+        ids = self.accounts_to_show()
+        return dict(zip(ids, map(self.get_account_name, ids)))
 
     def add_account(self, account_id, account):
         self.accounts[account_id] = account
@@ -1632,111 +1643,66 @@ class BIP32_Simple_Wallet(BIP32_Wallet):
 class BIP32_HD_Wallet(BIP32_Wallet):
     # wallet that can create accounts
     def __init__(self, storage):
-        self.next_account = storage.get('next_account2', None)
         BIP32_Wallet.__init__(self, storage)
+        # Backwards-compatibility.  Remove legacy "next_account2" and
+        # drop unused master public key to avoid duplicate errors
+        storage.put('next_account2', None)
+        self.master_public_keys.pop(self.next_derivation()[0], None)
+
+    def next_account_number(self):
+        assert (set(self.accounts.keys()) ==
+                set(['%d' % n for n in range(len(self.accounts))]))
+        return len(self.accounts)
+
+    def next_derivation(self):
+        account_id = '%d' % self.next_account_number()
+        return self.root_name + account_id + "'", account_id
+
+    def show_account(self, account_id):
+        return self.account_is_used(account_id) or account_id in self.labels
+
+    def last_account_id(self):
+        return '%d' % (self.next_account_number() - 1)
+
+    def accounts_to_show(self):
+        # The last account is shown only if named or used
+        result = list(self.accounts.keys())
+        last_id = self.last_account_id()
+        if not self.show_account(last_id):
+            result.remove(last_id)
+        return result
 
     def can_create_accounts(self):
         return self.root_name in self.master_private_keys.keys()
 
-    def addresses(self, b=True):
-        l = BIP32_Wallet.addresses(self, b)
-        if self.next_account:
-            _, _, _, next_address = self.next_account
-            if next_address not in l:
-                l.append(next_address)
-        return l
+    def permit_account_naming(self):
+        return (self.can_create_accounts() and
+                not self.show_account(self.last_account_id()))
 
-    def get_address_index(self, address):
-        if self.next_account:
-            next_id, next_xpub, next_pubkey, next_address = self.next_account
-            if address == next_address:
-                return next_id, (0,0)
-        return BIP32_Wallet.get_address_index(self, address)
+    def create_main_account(self, password):
+        # First check the password is valid (this raises if it isn't).
+        self.check_password(password)
+        assert self.next_account_number() == 0
+        self.create_next_account(password, _('Main account'))
+        self.create_next_account(password)
 
-    def num_accounts(self):
-        keys = []
-        for k, v in self.accounts.items():
-            if type(v) != BIP32_Account:
-                continue
-            keys.append(k)
-        i = 0
-        while True:
-            account_id = '%d'%i
-            if account_id not in keys:
-                break
-            i += 1
-        return i
-
-    def get_next_account(self, password):
-        account_id = '%d'%self.num_accounts()
-        derivation = self.root_name + "%d'"%int(account_id)
+    def create_next_account(self, password, label=None):
+        derivation, account_id = self.next_derivation()
         xpub, xprv = self.derive_xkeys(self.root_name, derivation, password)
         self.add_master_public_key(derivation, xpub)
         if xprv:
             self.add_master_private_key(derivation, xprv, password)
         account = BIP32_Account({'xpub':xpub})
-        addr, pubkey = account.first_address()
-        self.add_address(addr)
-        return account_id, xpub, pubkey, addr
-
-    def create_main_account(self, password):
-        # First check the password is valid (this raises if it isn't).
-        self.check_password(password)
-        assert self.num_accounts() == 0
-        self.create_account('Main account', password)
-
-    def create_account(self, name, password):
-        account_id, xpub, _, _ = self.get_next_account(password)
-        account = BIP32_Account({'xpub':xpub})
         self.add_account(account_id, account)
-        self.set_label(account_id, name)
-        # add address of the next account
-        self.next_account = self.get_next_account(password)
-        self.storage.put('next_account2', self.next_account)
-
-    def account_is_pending(self, k):
-        return type(self.accounts.get(k)) == PendingAccount
-
-    def delete_pending_account(self, k):
-        assert type(self.accounts.get(k)) == PendingAccount
-        self.accounts.pop(k)
+        if label:
+            self.set_label(account_id, label)
         self.save_accounts()
 
-    def create_pending_account(self, name, password):
-        if self.next_account is None:
-            self.next_account = self.get_next_account(password)
-            self.storage.put('next_account2', self.next_account)
-        next_id, next_xpub, next_pubkey, next_address = self.next_account
-        if name:
-            self.set_label(next_id, name)
-        self.accounts[next_id] = PendingAccount({'pending':True, 'address':next_address, 'pubkey':next_pubkey})
-        self.save_accounts()
-
-    def synchronize(self):
-        # synchronize existing accounts
-        BIP32_Wallet.synchronize(self)
-
-        if self.next_account is None and not self.use_encryption:
-            try:
-                self.next_account = self.get_next_account(None)
-                self.storage.put('next_account2', self.next_account)
-            except:
-                self.print_error('cannot get next account')
-        # check pending account
-        if self.next_account is not None:
-            next_id, next_xpub, next_pubkey, next_address = self.next_account
-            if self.address_is_old(next_address):
-                self.print_error("creating account", next_id)
-                self.add_account(next_id, BIP32_Account({'xpub':next_xpub}))
-                # here the user should get a notification
-                self.next_account = None
-                self.storage.put('next_account2', self.next_account)
-            elif self.history.get(next_address, []):
-                if next_id not in self.accounts:
-                    self.print_error("create pending account", next_id)
-                    self.accounts[next_id] = PendingAccount({'pending':True, 'address':next_address, 'pubkey':next_pubkey})
-                    self.save_accounts()
+    def account_is_used(self, account_id):
+        return self.accounts[account_id].is_used(self)
 
+    def accounts_all_used(self):
+        return all(self.account_is_used(acc_id) for acc_id in self.accounts)
 
 
 class NewWallet(BIP32_Wallet, Mnemonic):