twallet: make labels private, and access to need lock - electrum - Electrum Bitcoin wallet
 (HTM) git clone https://git.parazyd.org/electrum
 (DIR) Log
 (DIR) Files
 (DIR) Refs
 (DIR) Submodules
       ---
 (DIR) commit 4b6c86ecbecb445483a5dcf52d7cfea0e268afe9
 (DIR) parent da4f11dbd3650e0912db778ee645592719cea7b7
 (HTM) Author: SomberNight <somber.night@protonmail.com>
       Date:   Tue, 13 Oct 2020 18:30:24 +0200
       
       wallet: make labels private, and access to need lock
       
       e.g. labels plugin iterated over wallet.labels on asyncio thread while user could trigger an edit from Qt thread
       
       Diffstat:
         M electrum/commands.py                |       2 +-
         M electrum/gui/kivy/uix/dialogs/addr… |       2 +-
         M electrum/gui/kivy/uix/dialogs/tx_d… |       2 +-
         M electrum/gui/qt/address_list.py     |       2 +-
         M electrum/gui/qt/history_list.py     |       4 ++--
         M electrum/gui/qt/main_window.py      |       2 +-
         M electrum/gui/qt/utxo_list.py        |       4 ++--
         M electrum/gui/stdio.py               |       6 +++---
         M electrum/gui/text.py                |       8 ++++----
         M electrum/lnworker.py                |       6 +++---
         M electrum/plugins/labels/labels.py   |      28 ++++++++++++++--------------
         M electrum/wallet.py                  |      57 ++++++++++++++++++++-----------
         M electrum/wallet_db.py               |       2 +-
       
       13 files changed, 71 insertions(+), 54 deletions(-)
       ---
 (DIR) diff --git a/electrum/commands.py b/electrum/commands.py
       t@@ -723,7 +723,7 @@ class Commands:
                    if balance:
                        item += (format_satoshis(sum(wallet.get_addr_balance(addr))),)
                    if labels:
       -                item += (repr(wallet.labels.get(addr, '')),)
       +                item += (repr(wallet.get_label(addr)),)
                    out.append(item)
                return out
        
 (DIR) diff --git a/electrum/gui/kivy/uix/dialogs/addresses.py b/electrum/gui/kivy/uix/dialogs/addresses.py
       t@@ -240,7 +240,7 @@ class AddressesDialog(Factory.Popup):
                n = 0
                cards = []
                for address in _list:
       -            label = wallet.labels.get(address, '')
       +            label = wallet.get_label(address)
                    balance = sum(wallet.get_addr_balance(address))
                    is_used_and_empty = wallet.is_used(address) and balance == 0
                    if self.show_used == 1 and (balance or is_used_and_empty):
 (DIR) diff --git a/electrum/gui/kivy/uix/dialogs/tx_dialog.py b/electrum/gui/kivy/uix/dialogs/tx_dialog.py
       t@@ -297,7 +297,7 @@ class TxDialog(Factory.Popup):
            def label_dialog(self):
                from .label_dialog import LabelDialog
                key = self.tx.txid()
       -        text = self.app.wallet.get_label(key)
       +        text = self.app.wallet.get_label_for_txid(key)
                def callback(text):
                    self.app.wallet.set_label(key, text)
                    self.update()
 (DIR) diff --git a/electrum/gui/qt/address_list.py b/electrum/gui/qt/address_list.py
       t@@ -160,7 +160,7 @@ class AddressList(MyTreeView):
                addresses_beyond_gap_limit = self.wallet.get_all_known_addresses_beyond_gap_limit()
                for address in addr_list:
                    num = self.wallet.get_address_history_len(address)
       -            label = self.wallet.labels.get(address, '')
       +            label = self.wallet.get_label(address)
                    c, u, x = self.wallet.get_addr_balance(address)
                    balance = c + u + x
                    is_used_and_empty = self.wallet.is_used(address) and balance == 0
 (DIR) diff --git a/electrum/gui/qt/history_list.py b/electrum/gui/qt/history_list.py
       t@@ -237,7 +237,7 @@ class HistoryModel(CustomModel, Logger):
        
            def update_label(self, index):
                tx_item = index.internalPointer().get_data()
       -        tx_item['label'] = self.parent.wallet.get_label(get_item_key(tx_item))
       +        tx_item['label'] = self.parent.wallet.get_label_for_txid(get_item_key(tx_item))
                topLeft = bottomRight = self.createIndex(index.row(), HistoryColumns.DESCRIPTION)
                self.dataChanged.emit(topLeft, bottomRight, [Qt.DisplayRole])
                self.parent.utxo_list.update()
       t@@ -633,7 +633,7 @@ class HistoryList(MyTreeView, AcceptFileDragDrop):
        
            def show_transaction(self, tx_item, tx):
                tx_hash = tx_item['txid']
       -        label = self.wallet.get_label(tx_hash) or None # prefer 'None' if not defined (force tx dialog to hide Description field if missing)
       +        label = self.wallet.get_label_for_txid(tx_hash) or None # prefer 'None' if not defined (force tx dialog to hide Description field if missing)
                self.parent.show_transaction(tx, tx_desc=label)
        
            def add_copy_menu(self, menu, idx):
 (DIR) diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py
       t@@ -3188,7 +3188,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
                if fee is None:
                    self.show_error(_("Can't bump fee: unknown fee for original transaction."))
                    return
       -        tx_label = self.wallet.get_label(txid)
       +        tx_label = self.wallet.get_label_for_txid(txid)
                tx_size = tx.estimated_size()
                old_fee_rate = fee / tx_size  # sat/vbyte
                d = WindowModalDialog(self, _('Bump Fee'))
 (DIR) diff --git a/electrum/gui/qt/utxo_list.py b/electrum/gui/qt/utxo_list.py
       t@@ -98,7 +98,7 @@ class UTXOList(MyTreeView):
                name = utxo.prevout.to_str()
                name_short = utxo.prevout.txid.hex()[:16] + '...' + ":%d" % utxo.prevout.out_idx
                self._utxo_dict[name] = utxo
       -        label = self.wallet.get_label(utxo.prevout.txid.hex())
       +        label = self.wallet.get_label_for_txid(utxo.prevout.txid.hex())
                amount = self.parent.format_amount(utxo.value_sats(), whitespaces=True)
                labels = [name_short, address, label, amount, '%d'%height]
                utxo_item = [QStandardItem(x) for x in labels]
       t@@ -178,7 +178,7 @@ class UTXOList(MyTreeView):
                    # "Details"
                    tx = self.wallet.db.get_transaction(txid)
                    if tx:
       -                label = self.wallet.get_label(txid) or None # Prefer None if empty (None hides the Description: field in the window)
       +                label = self.wallet.get_label_for_txid(txid) or None # Prefer None if empty (None hides the Description: field in the window)
                        menu.addAction(_("Details"), lambda: self.parent.show_transaction(tx, tx_desc=label))
                    # "Copy ..."
                    idx = self.indexAt(position)
 (DIR) diff --git a/electrum/gui/stdio.py b/electrum/gui/stdio.py
       t@@ -108,7 +108,7 @@ class ElectrumGui:
                    else:
                        time_str = 'unconfirmed'
        
       -            label = self.wallet.get_label(hist_item.txid)
       +            label = self.wallet.get_label_for_txid(hist_item.txid)
                    messages.append(format_str % (time_str, label, format_satoshis(delta, whitespaces=True),
                                                  format_satoshis(hist_item.balance, whitespaces=True)))
        
       t@@ -140,7 +140,7 @@ class ElectrumGui:
                self.print_list(messages, "%19s  %25s "%("Key", "Value"))
        
            def print_addresses(self):
       -        messages = map(lambda addr: "%30s    %30s       "%(addr, self.wallet.labels.get(addr,"")), self.wallet.get_addresses())
       +        messages = map(lambda addr: "%30s    %30s       "%(addr, self.wallet.get_label(addr)), self.wallet.get_addresses())
                self.print_list(messages, "%19s  %25s "%("Address", "Label"))
        
            def print_order(self):
       t@@ -209,7 +209,7 @@ class ElectrumGui:
                    return
        
                if self.str_description:
       -            self.wallet.labels[tx.txid()] = self.str_description
       +            self.wallet.set_label(tx.txid(), self.str_description)
        
                print(_("Please wait..."))
                try:
 (DIR) diff --git a/electrum/gui/text.py b/electrum/gui/text.py
       t@@ -136,7 +136,7 @@ class ElectrumGui:
                    else:
                        time_str = 'unconfirmed'
        
       -            label = self.wallet.get_label(hist_item.txid)
       +            label = self.wallet.get_label_for_txid(hist_item.txid)
                    if len(label) > 40:
                        label = label[0:37] + '...'
                    self.history.append(format_str % (time_str, label, format_satoshis(hist_item.delta, whitespaces=True),
       t@@ -177,7 +177,7 @@ class ElectrumGui:
        
            def print_addresses(self):
                fmt = "%-35s  %-30s"
       -        messages = map(lambda addr: fmt % (addr, self.wallet.labels.get(addr,"")), self.wallet.get_addresses())
       +        messages = map(lambda addr: fmt % (addr, self.wallet.get_label(addr)), self.wallet.get_addresses())
                self.print_list(messages,   fmt % ("Address", "Label"))
        
            def print_edit_line(self, y, label, text, index, size):
       t@@ -314,7 +314,7 @@ class ElectrumGui:
                    elif out == "Edit label":
                        s = self.get_string(6 + self.pos, 18)
                        if s:
       -                    self.wallet.labels[key] = s
       +                    self.wallet.set_label(key, s)
        
            def run_banner_tab(self, c):
                self.show_message(repr(c))
       t@@ -379,7 +379,7 @@ class ElectrumGui:
                    return
        
                if self.str_description:
       -            self.wallet.labels[tx.txid()] = self.str_description
       +            self.wallet.set_label(tx.txid(), self.str_description)
        
                self.show_message(_("Please wait..."), getchar=False)
                try:
 (DIR) diff --git a/electrum/lnworker.py b/electrum/lnworker.py
       t@@ -678,7 +678,7 @@ class LNWallet(LNWorker):
                    item = {
                        'channel_id': bh2u(chan.channel_id),
                        'type': 'channel_opening',
       -                'label': self.wallet.get_label(funding_txid) or (_('Open channel') + ' ' + chan.get_id_for_log()),
       +                'label': self.wallet.get_label_for_txid(funding_txid) or (_('Open channel') + ' ' + chan.get_id_for_log()),
                        'txid': funding_txid,
                        'amount_msat': chan.balance(LOCAL, ctn=0),
                        'direction': 'received',
       t@@ -693,7 +693,7 @@ class LNWallet(LNWorker):
                    item = {
                        'channel_id': bh2u(chan.channel_id),
                        'txid': closing_txid,
       -                'label': self.wallet.get_label(closing_txid) or (_('Close channel') + ' ' + chan.get_id_for_log()),
       +                'label': self.wallet.get_label_for_txid(closing_txid) or (_('Close channel') + ' ' + chan.get_id_for_log()),
                        'type': 'channel_closure',
                        'amount_msat': -chan.balance_minus_outgoing_htlcs(LOCAL),
                        'direction': 'sent',
       t@@ -724,7 +724,7 @@ class LNWallet(LNWorker):
                        'amount_msat': 0,
                        #'amount_msat': amount_msat, # must not be added
                        'type': 'swap',
       -                'label': self.wallet.get_label(txid) or label,
       +                'label': self.wallet.get_label_for_txid(txid) or label,
                    }
                return out
        
 (DIR) diff --git a/electrum/plugins/labels/labels.py b/electrum/plugins/labels/labels.py
       t@@ -36,18 +36,18 @@ class LabelsPlugin(BasePlugin):
                self.target_host = 'labels.electrum.org'
                self.wallets = {}
        
       -    def encode(self, wallet, msg):
       +    def encode(self, wallet: 'Abstract_Wallet', msg: str) -> str:
                password, iv, wallet_id = self.wallets[wallet]
                encrypted = aes_encrypt_with_iv(password, iv, msg.encode('utf8'))
                return base64.b64encode(encrypted).decode()
        
       -    def decode(self, wallet, message):
       +    def decode(self, wallet: 'Abstract_Wallet', message: str) -> str:
                password, iv, wallet_id = self.wallets[wallet]
                decoded = base64.b64decode(message)
                decrypted = aes_decrypt_with_iv(password, iv, decoded)
                return decrypted.decode('utf8')
        
       -    def get_nonce(self, wallet):
       +    def get_nonce(self, wallet: 'Abstract_Wallet'):
                # nonce is the nonce to be used with the next change
                nonce = wallet.db.get('wallet_nonce')
                if nonce is None:
       t@@ -55,12 +55,12 @@ class LabelsPlugin(BasePlugin):
                    self.set_nonce(wallet, nonce)
                return nonce
        
       -    def set_nonce(self, wallet, nonce):
       +    def set_nonce(self, wallet: 'Abstract_Wallet', nonce):
                self.logger.info(f"set {wallet.basename()} nonce to {nonce}")
                wallet.db.put("wallet_nonce", nonce)
        
            @hook
       -    def set_label(self, wallet, item, label):
       +    def set_label(self, wallet: 'Abstract_Wallet', item, label):
                if wallet not in self.wallets:
                    return
                if not item:
       t@@ -99,7 +99,7 @@ class LabelsPlugin(BasePlugin):
                        except Exception as e:
                            raise Exception('Could not decode: ' + await result.text()) from e
        
       -    async def push_thread(self, wallet):
       +    async def push_thread(self, wallet: 'Abstract_Wallet'):
                wallet_data = self.wallets.get(wallet, None)
                if not wallet_data:
                    raise Exception('Wallet {} not loaded'.format(wallet))
       t@@ -107,7 +107,7 @@ class LabelsPlugin(BasePlugin):
                bundle = {"labels": [],
                          "walletId": wallet_id,
                          "walletNonce": self.get_nonce(wallet)}
       -        for key, value in wallet.labels.items():
       +        for key, value in wallet.get_all_labels().items():
                    try:
                        encoded_key = self.encode(wallet, key)
                        encoded_value = self.encode(wallet, value)
       t@@ -118,7 +118,7 @@ class LabelsPlugin(BasePlugin):
                                             'externalId': encoded_key})
                await self.do_post("/labels", bundle)
        
       -    async def pull_thread(self, wallet, force):
       +    async def pull_thread(self, wallet: 'Abstract_Wallet', force: bool):
                wallet_data = self.wallets.get(wallet, None)
                if not wallet_data:
                    raise Exception('Wallet {} not loaded'.format(wallet))
       t@@ -148,8 +148,8 @@ class LabelsPlugin(BasePlugin):
                    result[key] = value
        
                for key, value in result.items():
       -            if force or not wallet.labels.get(key):
       -                wallet.labels[key] = value
       +            if force or not wallet.get_label(key):
       +                wallet._set_label(key, value)
        
                self.logger.info(f"received {len(response)} labels")
                self.set_nonce(wallet, response["nonce"] + 1)
       t@@ -160,21 +160,21 @@ class LabelsPlugin(BasePlugin):
        
            @ignore_exceptions
            @log_exceptions
       -    async def pull_safe_thread(self, wallet, force):
       +    async def pull_safe_thread(self, wallet: 'Abstract_Wallet', force: bool):
                try:
                    await self.pull_thread(wallet, force)
                except ErrorConnectingServer as e:
                    self.logger.info(repr(e))
        
       -    def pull(self, wallet, force):
       +    def pull(self, wallet: 'Abstract_Wallet', force: bool):
                if not wallet.network: raise Exception(_('You are offline.'))
                return asyncio.run_coroutine_threadsafe(self.pull_thread(wallet, force), wallet.network.asyncio_loop).result()
        
       -    def push(self, wallet):
       +    def push(self, wallet: 'Abstract_Wallet'):
                if not wallet.network: raise Exception(_('You are offline.'))
                return asyncio.run_coroutine_threadsafe(self.push_thread(wallet), wallet.network.asyncio_loop).result()
        
       -    def start_wallet(self, wallet):
       +    def start_wallet(self, wallet: 'Abstract_Wallet'):
                if not wallet.network: return  # 'offline' mode
                nonce = self.get_nonce(wallet)
                self.logger.info(f"wallet {wallet.basename()} nonce is {nonce}")
 (DIR) diff --git a/electrum/wallet.py b/electrum/wallet.py
       t@@ -263,7 +263,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                # saved fields
                self.use_change            = db.get('use_change', True)
                self.multiple_change       = db.get('multiple_change', False)
       -        self.labels                = db.get_dict('labels')
       +        self._labels                = db.get_dict('labels')
                self.frozen_addresses      = set(db.get('frozen_addresses', []))
                self.frozen_coins          = set(db.get('frozen_coins', []))  # set of txid:vout strings
                self.fiat_value            = db.get_dict('fiat_value')
       t@@ -423,20 +423,28 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
            def is_deterministic(self) -> bool:
                return self.keystore.is_deterministic()
        
       +    def _set_label(self, key: str, value: Optional[str]) -> None:
       +        with self.lock:
       +            if value is None:
       +                self._labels.pop(key, None)
       +            else:
       +                self._labels[key] = value
       +
            def set_label(self, name: str, text: str = None) -> bool:
                if not name:
                    return False
                changed = False
       -        old_text = self.labels.get(name)
       -        if text:
       -            text = text.replace("\n", " ")
       -            if old_text != text:
       -                self.labels[name] = text
       -                changed = True
       -        else:
       -            if old_text is not None:
       -                self.labels.pop(name)
       -                changed = True
       +        with self.lock:
       +            old_text = self._labels.get(name)
       +            if text:
       +                text = text.replace("\n", " ")
       +                if old_text != text:
       +                    self._labels[name] = text
       +                    changed = True
       +            else:
       +                if old_text is not None:
       +                    self._labels.pop(name)
       +                    changed = True
                if changed:
                    run_hook('set_label', self, name, text)
                return changed
       t@@ -447,7 +455,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                    self.set_label(key, value)
        
            def export_labels(self, path):
       -        write_json_file(path, self.labels)
       +        write_json_file(path, self.get_all_labels())
        
            def set_fiat_value(self, txid, ccy, text, fx, value_sat):
                if not self.db.get_transaction(txid):
       t@@ -568,7 +576,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                              and bool(tx_we_already_have_in_db))
                if tx.is_complete():
                    if tx_we_already_have_in_db:
       -                label = self.get_label(tx_hash)
       +                label = self.get_label_for_txid(tx_hash)
                        if tx_mined_status.height > 0:
                            if tx_mined_status.conf:
                                status = _("{} confirmations").format(tx_mined_status.conf)
       t@@ -680,7 +688,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                        'bc_value': Satoshis(hist_item.delta),
                        'bc_balance': Satoshis(hist_item.balance),
                        'date': timestamp_to_datetime(hist_item.tx_mined_status.timestamp),
       -                'label': self.get_label(hist_item.txid),
       +                'label': self.get_label_for_txid(hist_item.txid),
                        'txpos_in_block': hist_item.tx_mined_status.txpos,
                    }
        
       t@@ -823,7 +831,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                for invoice in self.get_relevant_invoices_for_tx(tx):
                    if invoice.message:
                        labels.append(invoice.message)
       -        if labels and not self.labels.get(tx_hash, ''):
       +        if labels and not self._labels.get(tx_hash, ''):
                    self.set_label(tx_hash, "; ".join(labels))
                return bool(labels)
        
       t@@ -994,19 +1002,28 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
                    item['capital_gain'] = Fiat(cg, fx.ccy)
                return item
        
       -    def get_label(self, tx_hash: str) -> str:
       -        return self.labels.get(tx_hash, '') or self.get_default_label(tx_hash)
       +    def get_label(self, key: str) -> str:
       +        # key is typically: address / txid / LN-payment-hash-hex
       +        return self._labels.get(key) or ''
       +
       +    def get_label_for_txid(self, tx_hash: str) -> str:
       +        return self._labels.get(tx_hash) or self._get_default_label_for_txid(tx_hash)
        
       -    def get_default_label(self, tx_hash) -> str:
       +    def _get_default_label_for_txid(self, tx_hash: str) -> str:
       +        # if no inputs are ismine, concat labels of output addresses
                if not self.db.get_txi_addresses(tx_hash):
                    labels = []
                    for addr in self.db.get_txo_addresses(tx_hash):
       -                label = self.labels.get(addr)
       +                label = self._labels.get(addr)
                        if label:
                            labels.append(label)
                    return ', '.join(labels)
                return ''
        
       +    def get_all_labels(self) -> Dict[str, str]:
       +        with self.lock:
       +            return copy.copy(self._labels)
       +
            def get_tx_status(self, tx_hash, tx_mined_info: TxMinedInfo):
                extra = []
                height = tx_mined_info.height
       t@@ -1672,7 +1689,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
        
            def get_request_URI(self, req: OnchainInvoice) -> str:
                addr = req.get_address()
       -        message = self.labels.get(addr, '')
       +        message = self.get_label(addr)
                amount = req.amount_sat
                extra_query_params = {}
                if req.time:
 (DIR) diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py
       t@@ -1051,7 +1051,7 @@ class WalletDB(JsonDB):
                self.tx_fees.pop(txid, None)
        
            @locked
       -    def get_dict(self, name):
       +    def get_dict(self, name) -> dict:
                # Warning: interacts un-intuitively with 'put': certain parts
                # of 'data' will have pointers saved as separate variables.
                if name not in self.data: