From 4b28bfc323edd235d0803b44c1e32f46a70de351 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 23 Jan 2024 01:33:42 +0000 Subject: [PATCH 1/3] tests: walletdb: add failing test case for persistence with jsonpatch add reproducer for https://github.com/spesmilo/electrum/issues/8842 --- electrum/tests/test_wallet.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/electrum/tests/test_wallet.py b/electrum/tests/test_wallet.py index 832035b81..f7a538edf 100644 --- a/electrum/tests/test_wallet.py +++ b/electrum/tests/test_wallet.py @@ -17,7 +17,11 @@ from electrum.util import TxMinedInfo, InvalidPassword from electrum.bitcoin import COIN from electrum.wallet_db import WalletDB, JsonDB from electrum.simple_config import SimpleConfig +from electrum import util from electrum.daemon import Daemon +from electrum.invoices import PR_UNPAID, PR_PAID, PR_UNCONFIRMED +from electrum.transaction import tx_from_any +from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED from . import ElectrumTestCase @@ -112,6 +116,32 @@ class TestWalletStorage(WalletTestCase): self.assertTrue('030dac677b9484e23db6f9255eddf433f4f12c02f9b35e0100f2f103ffbccf540f' in wallet.keystore.keypairs) self.assertTrue('02f11d5f222a728fd08226cb5a1e85a74d58fc257bd3764bf1234346f91defed72' in wallet.keystore.keypairs) + async def test_storage_prevouts_by_scripthash_persistence(self): + text = 'cycle rocket west magnet parrot shuffle foot correct salt library feed song' + d = restore_wallet_from_text(text, path=self.wallet_path, gap_limit=2, config=self.config) + wallet1 = d['wallet'] # type: Standard_Wallet + # create payreq + addr = wallet1.get_unused_address() + self.assertEqual("1NNkttn1YvVGdqBW4PR6zvc3Zx3H5owKRf", addr) + pr_key = wallet1.create_request(amount_sat=10000, message="msg", address=addr, exp_delay=86400) + pr = wallet1.get_request(pr_key) + self.assertIsNotNone(pr) + self.assertEqual(PR_UNPAID, wallet1.get_invoice_status(pr)) + await wallet1.stop() + + # open the wallet anew again, and get paid onchain + del wallet1 + wallet1 = Daemon._load_wallet(self.wallet_path, password=None, config=self.config) + tx = tx_from_any("02000000000101a97a9ae7fb1a9220fdd170a974987ac24631dcff89b60fa4907c78c3639994db0000000000fdffffff0210270000000000001976a914ea7804a2c266063572cc009a63dc25dcc0e9d9b588ac20491e0000000000160014b8e4fdc91593b67de2bf214694ef47e38dc2ee8e02473044022005326882904906cfa9c1de75333ace1019596f2ab25d21118220d037dfc0e48b02207d0b3f075cfe5e1e0247ff3cdd7155dc05e7459daf1bfa0ea02e9112b9151ec90121026cc6a74c2b0e38661d341ffae48fe7dde5196ca4afe95d28b496673fa4cf646700000000") + wallet1.adb.receive_tx_callback(tx, TX_HEIGHT_UNCONFIRMED) + self.assertEqual(PR_UNCONFIRMED, wallet1.get_invoice_status(pr)) + await wallet1.stop() + + # open the wallet anew again, and verify payreq is still paid + del wallet1 + wallet1 = Daemon._load_wallet(self.wallet_path, password=None, config=self.config) + self.assertEqual(PR_UNCONFIRMED, wallet1.get_invoice_status(pr)) + class FakeExchange(ExchangeBase): def __init__(self, rate): From 79aba5364cca4b2926037ee5985fd0b573ddf4c8 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 23 Jan 2024 01:38:17 +0000 Subject: [PATCH 2/3] walletdb: change structure of prevouts_by_scripthash, and db upgrade this fixes the test case added in prev --- electrum/wallet_db.py | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py index f22cf4e66..1604b9e93 100644 --- a/electrum/wallet_db.py +++ b/electrum/wallet_db.py @@ -69,7 +69,7 @@ class WalletUnfinished(WalletFileException): # seed_version is now used for the version of the wallet file OLD_SEED_VERSION = 4 # electrum versions < 2.0 NEW_SEED_VERSION = 11 # electrum versions >= 2.0 -FINAL_SEED_VERSION = 57 # electrum >= 2.7 will set this to prevent +FINAL_SEED_VERSION = 58 # electrum >= 2.7 will set this to prevent # old versions from overwriting new format @@ -103,7 +103,6 @@ class WalletFileExceptionVersion51(WalletFileException): pass # register dicts that require value conversions not handled by constructor json_db.register_dict('transactions', lambda x: tx_from_any(x, deserialize=False), None) -json_db.register_dict('prevouts_by_scripthash', lambda x: set(tuple(k) for k in x), None) json_db.register_dict('data_loss_protect_remote_pcp', lambda x: bytes.fromhex(x), None) json_db.register_dict('contacts', tuple, None) # register dicts that require key conversion @@ -230,6 +229,7 @@ class WalletDBUpgrader(Logger): self._convert_version_55() self._convert_version_56() self._convert_version_57() + self._convert_version_58() self.put('seed_version', FINAL_SEED_VERSION) # just to be sure def _convert_wallet_type(self): @@ -1107,6 +1107,25 @@ class WalletDBUpgrader(Logger): self.data.pop('seed_type', None) self.data['seed_version'] = 57 + def _convert_version_58(self): + # re-construct prevouts_by_scripthash + # new structure: scripthash -> outpoint -> value + if not self._is_upgrade_method_needed(57, 57): + return + from .bitcoin import script_to_scripthash + transactions = self.get('transactions', {}) # txid -> raw_tx + prevouts_by_scripthash = {} + for txid, raw_tx in transactions.items(): + tx = Transaction(raw_tx) + for idx, txout in enumerate(tx.outputs()): + outpoint = f"{txid}:{idx}" + scripthash = script_to_scripthash(txout.scriptpubkey.hex()) + if scripthash not in prevouts_by_scripthash: + prevouts_by_scripthash[scripthash] = {} + prevouts_by_scripthash[scripthash][outpoint] = txout.value + self.put('prevouts_by_scripthash', prevouts_by_scripthash) + self.data['seed_version'] = 58 + def _convert_imported(self): if not self._is_upgrade_method_needed(0, 13): return @@ -1362,23 +1381,23 @@ class WalletDB(JsonDB): assert isinstance(prevout, TxOutpoint) assert isinstance(value, int) if scripthash not in self._prevouts_by_scripthash: - self._prevouts_by_scripthash[scripthash] = set() - self._prevouts_by_scripthash[scripthash].add((prevout.to_str(), value)) + self._prevouts_by_scripthash[scripthash] = dict() + self._prevouts_by_scripthash[scripthash][prevout.to_str()] = value @modifier def remove_prevout_by_scripthash(self, scripthash: str, *, prevout: TxOutpoint, value: int) -> None: assert isinstance(scripthash, str) assert isinstance(prevout, TxOutpoint) assert isinstance(value, int) - self._prevouts_by_scripthash[scripthash].discard((prevout.to_str(), value)) + self._prevouts_by_scripthash[scripthash].pop(prevout.to_str(), None) if not self._prevouts_by_scripthash[scripthash]: self._prevouts_by_scripthash.pop(scripthash) @locked def get_prevouts_by_scripthash(self, scripthash: str) -> Set[Tuple[TxOutpoint, int]]: assert isinstance(scripthash, str) - prevouts_and_values = self._prevouts_by_scripthash.get(scripthash, set()) - return {(TxOutpoint.from_str(prevout), value) for prevout, value in prevouts_and_values} + prevouts_and_values = self._prevouts_by_scripthash.get(scripthash, {}) + return {(TxOutpoint.from_str(prevout), value) for prevout, value in prevouts_and_values.items()} @modifier def add_transaction(self, tx_hash: str, tx: Transaction) -> None: @@ -1615,8 +1634,8 @@ class WalletDB(JsonDB): self.history = self.get_dict('addr_history') # address -> list of (txid, height) self.verified_tx = self.get_dict('verified_tx3') # txid -> (height, timestamp, txpos, header_hash) self.tx_fees = self.get_dict('tx_fees') # type: Dict[str, TxFeesValue] - # scripthash -> set of (outpoint, value) - self._prevouts_by_scripthash = self.get_dict('prevouts_by_scripthash') # type: Dict[str, Set[Tuple[str, int]]] + # scripthash -> outpoint -> value + self._prevouts_by_scripthash = self.get_dict('prevouts_by_scripthash') # type: Dict[str, Dict[str, int]] # remove unreferenced tx for tx_hash in list(self.transactions.keys()): if not self.get_txi_addresses(tx_hash) and not self.get_txo_addresses(tx_hash): From 173404229381d580b7ed601897087cba0a2e9b6e Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 23 Jan 2024 01:49:04 +0000 Subject: [PATCH 3/3] jsondb: raise when trying to store sets in the db Some generic protection to prevent bugs similar to what was just fixed in prev commits. related https://github.com/spesmilo/electrum/issues/8842 --- electrum/json_db.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/electrum/json_db.py b/electrum/json_db.py index 1c1841bd8..b88ba4f56 100644 --- a/electrum/json_db.py +++ b/electrum/json_db.py @@ -159,6 +159,9 @@ class StoredDict(dict): # convert lists if isinstance(v, list): v = StoredList(v, self.db, self.path + [key]) + # reject sets. they do not work well with jsonpatch + if isinstance(v, set): + raise Exception(f"Do not store sets inside jsondb. path={self.path!r}") # set item dict.__setitem__(self, key, v) if self.db and patch: