diff --git a/electrum/bitcoin.py b/electrum/bitcoin.py index 562b53ee4..7afde6e54 100644 --- a/electrum/bitcoin.py +++ b/electrum/bitcoin.py @@ -439,7 +439,7 @@ def script_to_address(script: bytes, *, net=None) -> Optional[str]: def address_to_script(addr: str, *, net=None) -> bytes: if net is None: net = constants.net if not is_address(addr, net=net): - raise BitcoinException(f"invalid bitcoin address: {addr}") + raise BitcoinException(f"invalid bitcoin address: {neuter_bitcoin_address(addr)}") witver, witprog = segwit_addr.decode_segwit_address(net.SEGWIT_HRP, addr) if witprog is not None: if not (0 <= witver <= 16): @@ -455,6 +455,17 @@ def address_to_script(addr: str, *, net=None) -> bytes: return script +def neuter_bitcoin_address(addr: str) -> str: + """Truncate a bitcoin address, for display in errors that might get sent to the crash reporter, + to reduce harm to the user's privacy. + """ + assert isinstance(addr, str), type(addr) + if len(addr) <= 7: + return addr + neutered_addr = addr[:5] + '..' + addr[-2:] + return f"{neutered_addr!r} (len={len(addr)})" + + class OnchainOutputType(Enum): """Opaque types of scriptPubKeys. In case of p2sh, p2wsh and similar, no knowledge of redeem script, etc. @@ -470,7 +481,7 @@ def address_to_payload(addr: str, *, net=None) -> Tuple[OnchainOutputType, bytes """Return (type, pubkey hash / witness program) for an address.""" if net is None: net = constants.net if not is_address(addr, net=net): - raise BitcoinException(f"invalid bitcoin address: {addr}") + raise BitcoinException(f"invalid bitcoin address: {neuter_bitcoin_address(addr)}") witver, witprog = segwit_addr.decode_segwit_address(net.SEGWIT_HRP, addr) if witprog is not None: if witver == 0: diff --git a/electrum/synchronizer.py b/electrum/synchronizer.py index c231eab12..ed7369d19 100644 --- a/electrum/synchronizer.py +++ b/electrum/synchronizer.py @@ -33,7 +33,7 @@ from aiorpcx import run_in_thread, RPCError from . import util from .transaction import Transaction, PartialTransaction from .util import make_aiohttp_session, NetworkJobOnDefaultServer, random_shuffled_copy, OldTaskGroup -from .bitcoin import address_to_scripthash, is_address +from .bitcoin import address_to_scripthash, is_address, neuter_bitcoin_address from .logging import Logger from .interface import GracefulDisconnect, NetworkTimeout @@ -84,12 +84,12 @@ class SynchronizerBase(NetworkJobOnDefaultServer): self.session.unsubscribe(self.status_queue) def add(self, addr: str) -> None: - if not is_address(addr): raise ValueError(f"invalid bitcoin address {addr}") + if not is_address(addr): raise ValueError(f"invalid bitcoin address {neuter_bitcoin_address(addr)}") self._adding_addrs.add(addr) # this lets is_up_to_date already know about addr async def _add_address(self, addr: str): try: - if not is_address(addr): raise ValueError(f"invalid bitcoin address {addr}") + if not is_address(addr): raise ValueError(f"invalid bitcoin address {neuter_bitcoin_address(addr)}") if addr in self.requested_addrs: return self.requested_addrs.add(addr) await self.taskgroup.spawn(self._subscribe_to_address, addr) diff --git a/electrum/wallet.py b/electrum/wallet.py index 29e2e52e2..4e03b26d2 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -46,6 +46,7 @@ import electrum_ecc as ecc from aiorpcx import ignore_after, run_in_thread from . import util, keystore, transaction, bitcoin, coinchooser, bip32, descriptor +from . import constants from .i18n import _ from .bip32 import BIP32Node, convert_bip32_intpath_to_strpath, convert_bip32_strpath_to_intpath from .logging import get_logger, Logger @@ -456,7 +457,7 @@ class Abstract_Wallet(ABC, Logger, EventListener): self._up_to_date = False self.up_to_date_changed_event = asyncio.Event() - self.test_addresses_sanity() + assert self.db.get('genesis_blockhash') == constants.net.GENESIS, self.db.get('genesis_blockhash') if self.storage and self.has_storage_encryption(): if (se := self.storage.get_encryption_version()) not in (ae := self.get_available_storage_encryption_versions()): raise WalletFileException(f"unexpected storage encryption type. found: {se!r}. allowed: {ae!r}") @@ -695,15 +696,6 @@ class Abstract_Wallet(ABC, Logger, EventListener): def basename(self) -> str: return self.storage.basename() if self.storage else 'no_name' - def test_addresses_sanity(self) -> None: - addrs = self.get_receiving_addresses() - if len(addrs) > 0: - addr = str(addrs[0]) - if not bitcoin.is_address(addr): - neutered_addr = addr[:5] + '..' + addr[-2:] - raise WalletFileException(f'The addresses in this wallet are not bitcoin addresses.\n' - f'e.g. {neutered_addr} (length: {len(addr)})') - def check_returned_address_for_corruption(func): def wrapper(self, *args, **kwargs): addr = func(self, *args, **kwargs) diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py index be55c405b..a5d146a00 100644 --- a/electrum/wallet_db.py +++ b/electrum/wallet_db.py @@ -34,6 +34,7 @@ from functools import partial import attr from . import bitcoin +from . import constants from .util import profiler, WalletFileException, multisig_type, TxMinedInfo, MyEncoder from .keystore import bip44_derivation from .transaction import Transaction, TxOutpoint, tx_from_any, PartialTransaction, PartialTxOutput, BadHeaderMagic @@ -69,7 +70,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 = 70 # electrum >= 2.7 will set this to prevent +FINAL_SEED_VERSION = 71 # electrum >= 2.7 will set this to prevent # old versions from overwriting new format @@ -245,6 +246,7 @@ class WalletDBUpgrader(Logger): self._convert_version_68() self._convert_version_69() self._convert_version_70() + self._convert_version_71() self.put('seed_version', FINAL_SEED_VERSION) # just to be sure def _convert_wallet_type(self): @@ -1400,6 +1402,27 @@ class WalletDBUpgrader(Logger): connection['budget_spends'] = new_budget_spends self.data['seed_version'] = 70 + def _convert_version_71(self): + """Save 'genesis_blockhash' in DB.""" + if not self._is_upgrade_method_needed(70, 70): + return + # first, check we are trying to open this DB on the correct chain (mainnet vs testnet) + addresses = self.data.get("addresses", {}) + if self.data['wallet_type'] == 'imported': + recv_addrs = list(addresses.keys()) + else: + recv_addrs = addresses.get("receiving", []) + if len(recv_addrs) > 0: + first_address = recv_addrs[0] + if not bitcoin.is_address(first_address): + neutered_addr = first_address[:5] + '..' + first_address[-2:] + raise WalletFileException( + f"The addresses in this wallet are not bitcoin addresses. " + f"e.g. {neutered_addr} (len={len(first_address)})") + # if so, save genesis hash + self.data['genesis_blockhash'] = constants.net.GENESIS + self.data['seed_version'] = 71 + def _convert_imported(self): if not self._is_upgrade_method_needed(0, 13): return @@ -1503,6 +1526,7 @@ def upgrade_wallet_db(data: dict, do_upgrade: bool) -> Tuple[dict, bool]: if len(data) == 0: # create new DB data['seed_version'] = FINAL_SEED_VERSION + data["genesis_blockhash"] = constants.net.GENESIS # store this for debugging purposes v = DBMetadata( creation_timestamp=int(time.time()), @@ -1511,6 +1535,13 @@ def upgrade_wallet_db(data: dict, do_upgrade: bool) -> Tuple[dict, bool]: assert data.get("db_metadata", None) is None data["db_metadata"] = v.to_json() was_upgraded = True + # Test mainnet/testnet mixup. Do this before DB upgrades, as those might assume + # network magic bytes (e.g. if they parse an address or an xpub). + if data.get("genesis_blockhash", None) not in (constants.net.GENESIS, None): + raise WalletFileException( + _("This wallet file was created for a different network/chain.\n" + "Current chain: {}").format(constants.net.NET_NAME) + ) dbu = WalletDBUpgrader(data) if dbu.requires_split(): diff --git a/tests/test_daemon.py b/tests/test_daemon.py index 6da78076a..c6b2b2eb7 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -2,6 +2,7 @@ import asyncio from collections import defaultdict import os from typing import Optional, Iterable +from unittest import mock from electrum.commands import Commands from electrum.daemon import Daemon @@ -11,6 +12,7 @@ from electrum.lnworker import LNWallet, LNPeerManager from electrum.lnwatcher import LNWatcher from electrum import util from electrum.utils.memory_leak import count_objects_in_memory +from electrum import constants from . import ElectrumTestCase, as_testnet, restore_wallet_from_text__for_unittest @@ -347,3 +349,27 @@ class TestLoadWallet(DaemonTestCase): wallet1 = self.daemon.load_wallet(path1, password="garbage") with self.assertRaises(util.InvalidPassword): wallet1 = self.daemon.load_wallet(path1, password="garbage", force_check_password=True) + + async def test_mainnet_testnet_mixup(self): + """version bytes in addresses, xpubs, etc. differ between mainnet and testnet. + If the user tries to open a wallet for a different chain, try to show a reasonable error message. + """ + # we are on mainnet, and will try to open testnet wallets: + assert constants.net.TESTNET is False + + # case 1: fresh wallet created on wrong network + with mock.patch("electrum.constants.net", constants.BitcoinTestnet): + path = self._restore_wallet_from_text("9dk", password=None) + with self.assertRaises(util.WalletFileException): + wallet = self.daemon.load_wallet(path, password=None, upgrade=True) + + # case 2: existing older wallet (db v57) that gets populated with 'genesis_blockhash' in convert_version_71 + path = self.get_wallet_file_path("client_4_5_2_9dk_with_ln") + with self.assertRaises(util.WalletFileException): + wallet = self.daemon.load_wallet(path, password=None, upgrade=True) + + # case 3: existing older wallet (db v18) that gets populated with 'genesis_blockhash' in convert_version_71 + # // this test does not work: convert_version_20 raises InvalidMasterKeyVersionBytes + # path = self.get_wallet_file_path("client_3_3_8_xpub_with_realistic_history") + # with self.assertRaises(util.WalletFileException): + # wallet = self.daemon.load_wallet(path, password=None, upgrade=True)