qt history view custom fiat input fixes

previously, when you submitted a fiat value with thousands separator,
it would be discarded.

electrum/exchange_rate.py

@@ -464,9 +464,13 @@ class FxThread(ThreadJob):
         d = get_exchanges_by_ccy(history)
         return d.get(ccy, [])
 
+    @staticmethod
+    def remove_thousands_separator(text):
+        return text.replace(',', '') # FIXME use THOUSAND_SEPARATOR in util
+
     def ccy_amount_str(self, amount, commas):
         prec = CCY_PRECISIONS.get(self.ccy, 2)
-        fmt_str = "{:%s.%df}" % ("," if commas else "", max(0, prec))
+        fmt_str = "{:%s.%df}" % ("," if commas else "", max(0, prec)) # FIXME use util.THOUSAND_SEPARATOR and util.DECIMAL_POINT
         try:
             rounded_amount = round(amount, prec)
         except decimal.InvalidOperation:

electrum/gui/qt/history_list.py

@@ -275,10 +275,11 @@ class HistoryList(MyTreeWidget, AcceptFileDragDrop):
             if value and value < 0:
                 item.setForeground(3, red_brush)
                 item.setForeground(4, red_brush)
-            if fiat_value and not tx_item['fiat_default']:
+            if fiat_value is not None and not tx_item['fiat_default']:
                 item.setForeground(6, blue_brush)
             if tx_hash:
                 item.setData(0, Qt.UserRole, tx_hash)
+                item.setData(0, Qt.UserRole+1, value)
             self.insertTopLevelItem(0, item)
             if current_tx == tx_hash:
                 self.setCurrentItem(item)
@@ -286,6 +287,7 @@ class HistoryList(MyTreeWidget, AcceptFileDragDrop):
     def on_edited(self, item, column, prior):
         '''Called only when the text actually changes'''
         key = item.data(0, Qt.UserRole)
+        value = item.data(0, Qt.UserRole+1)
         text = item.text(column)
         # fixme
         if column == 3:
@@ -293,7 +295,7 @@ class HistoryList(MyTreeWidget, AcceptFileDragDrop):
             self.update_labels()
             self.parent.update_completions()
         elif column == 6:
-            self.parent.wallet.set_fiat_value(key, self.parent.fx.ccy, text)
+            self.parent.wallet.set_fiat_value(key, self.parent.fx.ccy, text, self.parent.fx, value)
             self.on_update()
 
     def on_doubleclick(self, item, column):

electrum/tests/test_wallet.py

@@ -3,9 +3,16 @@ import tempfile
 import sys
 import os
 import json
+from decimal import Decimal
+from unittest import TestCase
+import time
 
 from io import StringIO
 from electrum.storage import WalletStorage, FINAL_SEED_VERSION
+from electrum.wallet import Abstract_Wallet
+from electrum.exchange_rate import ExchangeBase, FxThread
+from electrum.util import TxMinedStatus
+from electrum.bitcoin import COIN
 
 from . import SequentialTestCase
 
@@ -68,3 +75,67 @@ class TestWalletStorage(WalletTestCase):
         with open(self.wallet_path, "r") as f:
             contents = f.read()
         self.assertEqual(some_dict, json.loads(contents))
+
+class FakeExchange(ExchangeBase):
+    def __init__(self, rate):
+        super().__init__(lambda self: None, lambda self: None)
+        self.quotes = {'TEST': rate}
+
+class FakeFxThread:
+    def __init__(self, exchange):
+        self.exchange = exchange
+        self.ccy = 'TEST'
+
+    remove_thousands_separator = staticmethod(FxThread.remove_thousands_separator)
+    timestamp_rate = FxThread.timestamp_rate
+    ccy_amount_str = FxThread.ccy_amount_str
+    history_rate = FxThread.history_rate
+
+class FakeWallet:
+    def __init__(self, fiat_value):
+        super().__init__()
+        self.fiat_value = fiat_value
+        self.transactions = self.verified_tx = {'abc': 'Tx'}
+
+    def get_tx_height(self, txid):
+        # because we use a current timestamp, and history is empty,
+        # FxThread.history_rate will use spot prices
+        return TxMinedStatus(height=10, conf=10, timestamp=time.time(), header_hash='def')
+
+    default_fiat_value = Abstract_Wallet.default_fiat_value
+    price_at_timestamp = Abstract_Wallet.price_at_timestamp
+    class storage:
+        put = lambda self, x: None
+
+txid = 'abc'
+ccy = 'TEST'
+
+class TestFiat(TestCase):
+    def setUp(self):
+        self.value_sat = COIN
+        self.fiat_value = {}
+        self.wallet = FakeWallet(fiat_value=self.fiat_value)
+        self.fx = FakeFxThread(FakeExchange(Decimal('1000.001')))
+        default_fiat = Abstract_Wallet.default_fiat_value(self.wallet, txid, self.fx, self.value_sat)
+        self.assertEqual(Decimal('1000.001'), default_fiat)
+        self.assertEqual('1,000.00', self.fx.ccy_amount_str(default_fiat, commas=True))
+
+    def test_save_fiat_and_reset(self):
+        self.assertEqual(False, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1000.01', self.fx, self.value_sat))
+        saved = self.fiat_value[ccy][txid]
+        self.assertEqual('1,000.01', self.fx.ccy_amount_str(Decimal(saved), commas=True))
+        self.assertEqual(True,       Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '', self.fx, self.value_sat))
+        self.assertNotIn(txid, self.fiat_value[ccy])
+        # even though we are not setting it to the exact fiat value according to the exchange rate, precision is truncated away
+        self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1,000.002', self.fx, self.value_sat))
+
+    def test_too_high_precision_value_resets_with_no_saved_value(self):
+        self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1,000.001', self.fx, self.value_sat))
+
+    def test_empty_resets(self):
+        self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '', self.fx, self.value_sat))
+        self.assertNotIn(ccy, self.fiat_value)
+
+    def test_save_garbage(self):
+        self.assertEqual(False, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, 'garbage', self.fx, self.value_sat))
+        self.assertNotIn(ccy, self.fiat_value)

electrum/util.py

@@ -39,6 +39,7 @@ import urllib.request, urllib.parse, urllib.error
 import builtins
 import json
 import time
+from typing import NamedTuple, Optional
 
 import aiohttp
 from aiohttp_socks import SocksConnector, SocksVer
@@ -129,31 +130,15 @@ class UserCancelled(Exception):
     '''An exception that is suppressed from the user'''
     pass
 
-class Satoshis(object):
-    __slots__ = ('value',)
-
-    def __new__(cls, value):
-        self = super(Satoshis, cls).__new__(cls)
-        self.value = value
-        return self
-
-    def __repr__(self):
-        return 'Satoshis(%d)'%self.value
+class Satoshis(NamedTuple):
+    value: int
 
     def __str__(self):
         return format_satoshis(self.value) + " BTC"
 
-class Fiat(object):
-    __slots__ = ('value', 'ccy')
-
-    def __new__(cls, value, ccy):
-        self = super(Fiat, cls).__new__(cls)
-        self.ccy = ccy
-        self.value = value
-        return self
-
-    def __repr__(self):
-        return 'Fiat(%s)'% self.__str__()
+class Fiat(NamedTuple):
+    value: Optional[Decimal]
+    ccy: str
 
     def __str__(self):
         if self.value is None or self.value.is_nan():

electrum/wallet.py

@@ -247,24 +247,37 @@ class Abstract_Wallet(AddressSynchronizer):
             self.storage.put('labels', self.labels)
         return changed
 
-    def set_fiat_value(self, txid, ccy, text):
+    def set_fiat_value(self, txid, ccy, text, fx, value):
         if txid not in self.transactions:
             return
-        if not text:
+        # since fx is inserting the thousands separator,
+        # and not util, also have fx remove it
+        text = fx.remove_thousands_separator(text)
+        def_fiat = self.default_fiat_value(txid, fx, value)
+        formatted = fx.ccy_amount_str(def_fiat, commas=False)
+        def_fiat_rounded = Decimal(formatted)
+        reset = not text
+        if not reset:
+            try:
+                text_dec = Decimal(text)
+                text_dec_rounded = Decimal(fx.ccy_amount_str(text_dec, commas=False))
+                reset = text_dec_rounded == def_fiat_rounded
+            except:
+                # garbage. not resetting, but not saving either
+                return False
+        if reset:
             d = self.fiat_value.get(ccy, {})
             if d and txid in d:
                 d.pop(txid)
             else:
-                return
-        else:
-            try:
-                Decimal(text)
-            except:
-                return
+                # avoid saving empty dict
+                return True
         if ccy not in self.fiat_value:
             self.fiat_value[ccy] = {}
-        self.fiat_value[ccy][txid] = text
+        if not reset:
+            self.fiat_value[ccy][txid] = text
         self.storage.put('fiat_value', self.fiat_value)
+        return reset
 
     def get_fiat_value(self, txid, ccy):
         fiat_value = self.fiat_value.get(ccy, {}).get(txid)
@@ -423,21 +436,11 @@ class Abstract_Wallet(AddressSynchronizer):
                 income += value
             # fiat computations
             if fx and fx.is_enabled() and fx.get_history_config():
-                fiat_value = self.get_fiat_value(tx_hash, fx.ccy)
-                fiat_default = fiat_value is None
-                fiat_rate = self.price_at_timestamp(tx_hash, fx.timestamp_rate)
-                fiat_value = fiat_value if fiat_value is not None else value / Decimal(COIN) * fiat_rate
-                fiat_fee = tx_fee / Decimal(COIN) * fiat_rate if tx_fee is not None else None
-                item['fiat_value'] = Fiat(fiat_value, fx.ccy)
-                item['fiat_fee'] = Fiat(fiat_fee, fx.ccy) if fiat_fee else None
-                item['fiat_default'] = fiat_default
+                fiat_fields = self.get_tx_item_fiat(tx_hash, value, fx, tx_fee)
+                fiat_value = fiat_fields['fiat_value'].value
+                item.update(fiat_fields)
                 if value < 0:
-                    acquisition_price = - value / Decimal(COIN) * self.average_price(tx_hash, fx.timestamp_rate, fx.ccy)
-                    liquidation_price = - fiat_value
-                    item['acquisition_price'] = Fiat(acquisition_price, fx.ccy)
-                    cg = liquidation_price - acquisition_price
-                    item['capital_gain'] = Fiat(cg, fx.ccy)
-                    capital_gains += cg
+                    capital_gains += fiat_fields['capital_gain'].value
                     fiat_expenditures += -fiat_value
                 else:
                     fiat_income += fiat_value
@@ -478,6 +481,27 @@ class Abstract_Wallet(AddressSynchronizer):
             'summary': summary
         }
 
+    def default_fiat_value(self, tx_hash, fx, value):
+        return value / Decimal(COIN) * self.price_at_timestamp(tx_hash, fx.timestamp_rate)
+
+    def get_tx_item_fiat(self, tx_hash, value, fx, tx_fee):
+        item = {}
+        fiat_value = self.get_fiat_value(tx_hash, fx.ccy)
+        fiat_default = fiat_value is None
+        fiat_rate = self.price_at_timestamp(tx_hash, fx.timestamp_rate)
+        fiat_value = fiat_value if fiat_value is not None else self.default_fiat_value(tx_hash, fx, value)
+        fiat_fee = tx_fee / Decimal(COIN) * fiat_rate if tx_fee is not None else None
+        item['fiat_value'] = Fiat(fiat_value, fx.ccy)
+        item['fiat_fee'] = Fiat(fiat_fee, fx.ccy) if fiat_fee else None
+        item['fiat_default'] = fiat_default
+        if value < 0:
+            acquisition_price = - value / Decimal(COIN) * self.average_price(tx_hash, fx.timestamp_rate, fx.ccy)
+            liquidation_price = - fiat_value
+            item['acquisition_price'] = Fiat(acquisition_price, fx.ccy)
+            cg = liquidation_price - acquisition_price
+            item['capital_gain'] = Fiat(cg, fx.ccy)
+        return item
+
     def get_label(self, tx_hash):
         label = self.labels.get(tx_hash, '')
         if label is '':