wallet.get_full_history: more consistent sort order

before:
```
>>> [print(wallet.get_tx_status(txid, wallet.adb.get_tx_height(txid))) for txid in list(wallet.get_full_history())[-10:]]
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(0, 'Unconfirmed [20. sat/b, 0.00 MB]')
(2, 'in 2 blocks')
(3, 'Local [180.4 sat/b]')
(3, 'Local [180.2 sat/b]')
(2, 'in 2016 blocks')
(0, 'Unconfirmed [180. sat/b, 0.00 MB]')
```

after:
```
>>> [print(wallet.get_tx_status(txid, wallet.adb.get_tx_height(txid))) for txid in list(wallet.get_full_history())[-10:]]
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(7, '2023-04-04 16:13')
(0, 'Unconfirmed [20. sat/b, 0.00 MB]')
(0, 'Unconfirmed [180. sat/b, 0.00 MB]')
(2, 'in 2016 blocks')
(2, 'in 2 blocks')
(3, 'Local [180.4 sat/b]')
(3, 'Local [180.2 sat/b]')
```

electrum/address_synchronizer.py

@@ -50,6 +50,9 @@ TX_HEIGHT_LOCAL = -2
 TX_HEIGHT_UNCONF_PARENT = -1
 TX_HEIGHT_UNCONFIRMED = 0
 
+TX_TIMESTAMP_INF = 999_999_999_999
+TX_HEIGHT_INF = 10 ** 9
+
 
 class HistoryItem(NamedTuple):
     txid: str
@@ -476,28 +479,29 @@ class AddressSynchronizer(Logger, EventListener):
                 self._history_local.clear()
                 self._get_balance_cache.clear()  # invalidate cache
 
-    def _get_txpos(self, tx_hash: str) -> Tuple[int, int]:
+    def _get_tx_sort_key(self, tx_hash: str) -> Tuple[int, int]:
-        """Returns (height, txpos) tuple, even if the tx is unverified.
+        """Returns a key to be used for sorting txs."""
-        If txpos is -1, height should only be used for sorting purposes.
-        """
         with self.lock:
-            verified_tx_mined_info = self.db.get_verified_tx(tx_hash)
+            tx_mined_info = self.get_tx_height(tx_hash)
-            if verified_tx_mined_info:
+            height = self.tx_height_to_sort_height(tx_mined_info.height)
-                height = verified_tx_mined_info.height
+            txpos = tx_mined_info.txpos or -1
-                txpos = verified_tx_mined_info.txpos
+            return height, txpos
-                assert height > 0, height
+
-                assert txpos is not None
+    @classmethod
-                return height, txpos
+    def tx_height_to_sort_height(cls, height: int = None):
-            elif tx_hash in self.unverified_tx:
+        """Return a height-like value to be used for sorting txs."""
-                height = self.unverified_tx[tx_hash]
+        if height is not None:
-                assert height > 0, height
+            if height > 0:
-                return height, -1
+                return height
-            elif tx_hash in self.unconfirmed_tx:
+            if height == TX_HEIGHT_UNCONFIRMED:
-                height = self.unconfirmed_tx[tx_hash]
+                return TX_HEIGHT_INF
-                assert height <= 0, height
+            if height == TX_HEIGHT_UNCONF_PARENT:
-                return (10**9 - height), -1
+                return TX_HEIGHT_INF + 1
-            else:
+            if height == TX_HEIGHT_FUTURE:
-                return (10**9 + 1), -1
+                return TX_HEIGHT_INF + 2
+            if height == TX_HEIGHT_LOCAL:
+                return TX_HEIGHT_INF + 3
+        return TX_HEIGHT_INF + 100
 
     def with_local_height_cached(func):
         # get local height only once, as it's relatively expensive.
@@ -530,7 +534,7 @@ class AddressSynchronizer(Logger, EventListener):
             tx_mined_status = self.get_tx_height(tx_hash)
             fee = self.get_tx_fee(tx_hash)
             history.append((tx_hash, tx_mined_status, delta, fee))
-        history.sort(key = lambda x: self._get_txpos(x[0]))
+        history.sort(key = lambda x: self._get_tx_sort_key(x[0]))
         # 3. add balance
         h2 = []
         balance = 0

electrum/gui/qml/qetransactionlistmodel.py

@@ -219,6 +219,7 @@ class QETransactionListModel(QAbstractListModel, QtEventListener):
         txinfo = self.wallet.get_tx_info(tx)
         status, status_str = self.wallet.get_tx_status(txid, txinfo.tx_mined_status)
         tx_item['date'] = status_str
+        # note: if the height changes, that might affect the history order, but we won't re-sort now.
         tx_item['height'] = self.wallet.adb.get_tx_height(txid).height
         index = self.index(tx_item_idx, 0)
         roles = [self._ROLE_RMAP[x] for x in ['height', 'date']]

electrum/lnworker.py

@@ -73,7 +73,7 @@ from .lnmsg import decode_msg
 from .i18n import _
 from .lnrouter import (RouteEdge, LNPaymentRoute, LNPaymentPath, is_route_sane_to_use,
                        NoChannelPolicy, LNPathInconsistent)
-from .address_synchronizer import TX_HEIGHT_LOCAL
+from .address_synchronizer import TX_HEIGHT_LOCAL, TX_TIMESTAMP_INF
 from . import lnsweep
 from .lnwatcher import LNWalletWatcher
 from .crypto import pw_encode_with_version_and_mac, pw_decode_with_version_and_mac
@@ -900,6 +900,7 @@ class LNWallet(LNWorker):
                 'amount_msat': chan.balance(LOCAL, ctn=0),
                 'direction': PaymentDirection.RECEIVED,
                 'timestamp': tx_height.timestamp,
+                'monotonic_timestamp': tx_height.timestamp or TX_TIMESTAMP_INF,
                 'date': timestamp_to_datetime(tx_height.timestamp),
                 'fee_sat': None,
                 'fee_msat': None,
@@ -922,6 +923,7 @@ class LNWallet(LNWorker):
                 'amount_msat': -chan.balance_minus_outgoing_htlcs(LOCAL),
                 'direction': PaymentDirection.SENT,
                 'timestamp': tx_height.timestamp,
+                'monotonic_timestamp': tx_height.timestamp or TX_TIMESTAMP_INF,
                 'date': timestamp_to_datetime(tx_height.timestamp),
                 'fee_sat': None,
                 'fee_msat': None,

electrum/wallet.py

@@ -73,7 +73,7 @@ from .transaction import (Transaction, TxInput, UnknownTxinType, TxOutput,
                           PartialTransaction, PartialTxInput, PartialTxOutput, TxOutpoint)
 from .plugin import run_hook
 from .address_synchronizer import (AddressSynchronizer, TX_HEIGHT_LOCAL,
-                                   TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_FUTURE)
+                                   TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_FUTURE, TX_TIMESTAMP_INF)
 from .invoices import BaseInvoice, Invoice, Request
 from .invoices import PR_PAID, PR_UNPAID, PR_UNKNOWN, PR_EXPIRED, PR_UNCONFIRMED, PR_INFLIGHT
 from .contacts import Contacts
@@ -1011,7 +1011,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
             domain = self.get_addresses()
         monotonic_timestamp = 0
         for hist_item in self.adb.get_history(domain=domain):
-            monotonic_timestamp = max(monotonic_timestamp, (hist_item.tx_mined_status.timestamp or 999_999_999_999))
+            monotonic_timestamp = max(monotonic_timestamp, (hist_item.tx_mined_status.timestamp or TX_TIMESTAMP_INF))
             d = {
                 'txid': hist_item.txid,
                 'fee_sat': hist_item.fee,
@@ -1241,9 +1241,13 @@ class Abstract_Wallet(ABC, Logger, EventListener):
             transactions_tmp[key] = tx_item
         # sort on-chain and LN stuff into new dict, by timestamp
         # (we rely on this being a *stable* sort)
+        def sort_key(x):
+            txid, tx_item = x
+            ts = tx_item.get('monotonic_timestamp') or tx_item.get('timestamp') or float('inf')
+            height = self.adb.tx_height_to_sort_height(tx_item.get('height'))
+            return ts, height
         transactions = OrderedDictWithIndex()
-        for k, v in sorted(list(transactions_tmp.items()),
+        for k, v in sorted(list(transactions_tmp.items()), key=sort_key):
-                           key=lambda x: x[1].get('monotonic_timestamp') or x[1].get('timestamp') or float('inf')):
             transactions[k] = v
         now = time.time()
         balance = 0