wallet_db: put 'genesis_blockhash' in DB, detect mainnet/testnet mixup
If the user tries to open a wallet for a different chain (mainnet vs testnet), try to show a reasonable error message. See previous attempt at this: https://github.com/spesmilo/electrum/commit/c13e05770150c5210783c3d42d3d2b1a683f18b4, which added `wallet.test_addresses_sanity()`. However there are many codepaths where "random" exceptions might get raised before the Wallet object is even instantiated. See [discussion there](https://github.com/spesmilo/electrum/commit/c13e05770150c5210783c3d42d3d2b1a683f18b4#commitcomment-28017341): > should we actually fix that? > if yes, it would be better to write the network type in storage Indeed now I think we should do that. At the time I was concerned it would not help against altcoin forks if we put "mainnet" or "testnet" in the DB. Now I realise we should just put the genesis block hash in the DB instead. Many of the reports in https://github.com/spesmilo/electrum/issues/6526 are likely due to users trying to open a mainnet wallet in testnet mode or vice-versa. fixes https://github.com/spesmilo/electrum/issues/9134 same issue in wizard 2fa two-step wallet-creation flow
This commit is contained in:
+2
-10
@@ -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)
|
||||
|
||||
+31
-1
@@ -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,26 @@ 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):
|
||||
raise WalletFileException(
|
||||
f"The addresses in this wallet are not bitcoin addresses. "
|
||||
f"e.g. {first_address!r}")
|
||||
# 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 +1525,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 +1534,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():
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user