diff --git a/electrum/bitcoin.py b/electrum/bitcoin.py index 94fcd95bb..43ada289a 100644 --- a/electrum/bitcoin.py +++ b/electrum/bitcoin.py @@ -28,7 +28,7 @@ from typing import List, Tuple, TYPE_CHECKING, Optional, Union, Sequence import enum from enum import IntEnum, Enum -from .util import bfh, BitcoinException, assert_bytes, to_bytes, inv_dict, is_hex_str +from .util import bfh, BitcoinException, assert_bytes, to_bytes, inv_dict, is_hex_str, classproperty from . import version from . import segwit_addr from . import constants @@ -754,6 +754,29 @@ def is_minikey(text: str) -> bool: def minikey_to_private_key(text: str) -> bytes: return sha256(text) -# dummy address for fee estimation of funding tx -def get_dummy_address(purpose): + +def _get_dummy_address(purpose: str) -> str: return redeem_script_to_address('p2wsh', sha256(bytes(purpose, "utf8")).hex()) + +_dummy_addr_funcs = set() +class DummyAddress: + """dummy address for fee estimation of funding tx + Use e.g. as: DummyAddress.CHANNEL + """ + def purpose(func): + _dummy_addr_funcs.add(func) + return classproperty(func) + + @purpose + def CHANNEL(self) -> str: + return _get_dummy_address("channel") + @purpose + def SWAP(self) -> str: + return _get_dummy_address("swap") + + @classmethod + def is_dummy_address(cls, addr: str) -> bool: + return addr in (f(cls) for f in _dummy_addr_funcs) + + +class DummyAddressUsedInTxException(Exception): pass diff --git a/electrum/gui/qml/qechannelopener.py b/electrum/gui/qml/qechannelopener.py index bc89bfced..e5189ca39 100644 --- a/electrum/gui/qml/qechannelopener.py +++ b/electrum/gui/qml/qechannelopener.py @@ -9,7 +9,7 @@ from electrum.i18n import _ from electrum.gui import messages from electrum.util import bfh from electrum.lnutil import extract_nodeid, ConnStringFormatError -from electrum.bitcoin import get_dummy_address +from electrum.bitcoin import DummyAddress from electrum.lnworker import hardcoded_trampoline_nodes from electrum.logging import get_logger @@ -182,7 +182,7 @@ class QEChannelOpener(QObject, AuthMixin): """ self._logger.debug('opening channel') # read funding_sat from tx; converts '!' to int value - funding_sat = funding_tx.output_value_for_address(get_dummy_address('channel')) + funding_sat = funding_tx.output_value_for_address(DummyAddress.CHANNEL) lnworker = self._wallet.wallet.lnworker def open_thread(): diff --git a/electrum/gui/qml/qeswaphelper.py b/electrum/gui/qml/qeswaphelper.py index 07cd12267..f5bd90911 100644 --- a/electrum/gui/qml/qeswaphelper.py +++ b/electrum/gui/qml/qeswaphelper.py @@ -6,7 +6,7 @@ from typing import Union from PyQt5.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject, QTimer, Q_ENUMS from electrum.i18n import _ -from electrum.bitcoin import get_dummy_address +from electrum.bitcoin import DummyAddress from electrum.logging import get_logger from electrum.transaction import PartialTxOutput from electrum.util import NotEnoughFunds, NoDynamicFeeEstimates, profiler, get_asyncio_loop @@ -245,7 +245,7 @@ class QESwapHelper(AuthMixin, QObject, QtEventListener): # this is just to estimate the maximal spendable onchain amount for HTLC self.update_tx('!') try: - max_onchain_spend = self._tx.output_value_for_address(get_dummy_address('swap')) + max_onchain_spend = self._tx.output_value_for_address(DummyAddress.SWAP) except AttributeError: # happens if there are no utxos max_onchain_spend = 0 reverse = int(min(lnworker.num_sats_can_send(), @@ -283,7 +283,7 @@ class QESwapHelper(AuthMixin, QObject, QtEventListener): self._tx = None self.valid = False return - outputs = [PartialTxOutput.from_address_and_value(get_dummy_address('swap'), onchain_amount)] + outputs = [PartialTxOutput.from_address_and_value(DummyAddress.SWAP, onchain_amount)] coins = self._wallet.wallet.get_spendable_coins(None) try: self._tx = self._wallet.wallet.make_unsigned_transaction( diff --git a/electrum/gui/qt/confirm_tx_dialog.py b/electrum/gui/qt/confirm_tx_dialog.py index a21c1606a..f951b3412 100644 --- a/electrum/gui/qt/confirm_tx_dialog.py +++ b/electrum/gui/qt/confirm_tx_dialog.py @@ -39,6 +39,7 @@ from electrum.plugin import run_hook from electrum.transaction import Transaction, PartialTransaction from electrum.wallet import InternalAddressCorruption from electrum.simple_config import SimpleConfig +from electrum.bitcoin import DummyAddress from .util import (WindowModalDialog, ColorScheme, HelpLabel, Buttons, CancelButton, BlockingWaitingDialog, PasswordLineEdit, WWLabel, read_QIcon) @@ -574,7 +575,7 @@ class TxEditor(WindowModalDialog): self.error = long_warning else: messages.append(long_warning) - if self.tx.has_dummy_output('swap'): + if self.tx.has_dummy_output(DummyAddress.SWAP): messages.append(_('This transaction will send funds to a submarine swap.')) # warn if spending unconf if any((txin.block_height is not None and txin.block_height<=0) for txin in self.tx.inputs()): diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index b2f42dcea..16e1ebcf0 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -51,7 +51,7 @@ import electrum from electrum.gui import messages from electrum import (keystore, ecc, constants, util, bitcoin, commands, paymentrequest, lnutil) -from electrum.bitcoin import COIN, is_address, get_dummy_address +from electrum.bitcoin import COIN, is_address, DummyAddress from electrum.plugin import run_hook, BasePlugin from electrum.i18n import _ from electrum.util import (format_time, UserCancelled, profiler, bfh, InvalidPassword, @@ -1303,7 +1303,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): @protected def _open_channel(self, connect_str, funding_sat, push_amt, funding_tx, password): # read funding_sat from tx; converts '!' to int value - funding_sat = funding_tx.output_value_for_address(get_dummy_address('channel')) + funding_sat = funding_tx.output_value_for_address(DummyAddress.CHANNEL) def task(): return self.wallet.lnworker.open_channel( connect_str=connect_str, diff --git a/electrum/gui/qt/send_tab.py b/electrum/gui/qt/send_tab.py index fc0aca14e..fcd2753b0 100644 --- a/electrum/gui/qt/send_tab.py +++ b/electrum/gui/qt/send_tab.py @@ -11,7 +11,7 @@ from PyQt5.QtGui import QMovie, QColor from electrum.i18n import _ from electrum.logging import Logger - +from electrum.bitcoin import DummyAddress from electrum.plugin import run_hook from electrum.util import NotEnoughFunds, NoDynamicFeeEstimates, parse_max_spend from electrum.invoices import PR_PAID, Invoice, PR_BROADCASTING, PR_BROADCAST @@ -326,11 +326,11 @@ class SendTab(QWidget, MessageBoxMixin, Logger): return is_preview = conf_dlg.is_preview - if tx.has_dummy_output('swap'): + if tx.has_dummy_output(DummyAddress.SWAP): sm = self.wallet.lnworker.swap_manager coro = sm.request_swap_for_tx(tx) swap, invoice, tx = self.network.run_from_another_thread(coro) - assert not tx.has_dummy_output('swap') + assert not tx.has_dummy_output(DummyAddress.SWAP) tx.swap_invoice = invoice tx.swap_payment_hash = swap.payment_hash diff --git a/electrum/gui/qt/swap_dialog.py b/electrum/gui/qt/swap_dialog.py index 945ffc0d9..0e9145369 100644 --- a/electrum/gui/qt/swap_dialog.py +++ b/electrum/gui/qt/swap_dialog.py @@ -5,7 +5,7 @@ from PyQt5.QtWidgets import QLabel, QVBoxLayout, QGridLayout, QPushButton from electrum.i18n import _ from electrum.util import NotEnoughFunds, NoDynamicFeeEstimates -from electrum.bitcoin import get_dummy_address +from electrum.bitcoin import DummyAddress from electrum.transaction import PartialTxOutput, PartialTransaction from electrum.gui import messages @@ -173,7 +173,7 @@ class SwapDialog(WindowModalDialog, QtEventListener): def _spend_max_forward_swap(self, tx: Optional[PartialTransaction]) -> None: if tx: - amount = tx.output_value_for_address(get_dummy_address('swap')) + amount = tx.output_value_for_address(DummyAddress.SWAP) self.send_amount_e.setAmount(amount) else: self.send_amount_e.setAmount(None) @@ -295,7 +295,7 @@ class SwapDialog(WindowModalDialog, QtEventListener): if max_amount > max_swap_amount: onchain_amount = max_swap_amount self.config.WALLET_SEND_CHANGE_TO_LIGHTNING = False - outputs = [PartialTxOutput.from_address_and_value(get_dummy_address('swap'), onchain_amount)] + outputs = [PartialTxOutput.from_address_and_value(DummyAddress.SWAP, onchain_amount)] try: tx = self.window.wallet.make_unsigned_transaction( coins=coins, diff --git a/electrum/gui/qt/transaction_dialog.py b/electrum/gui/qt/transaction_dialog.py index 28479b038..556381c33 100644 --- a/electrum/gui/qt/transaction_dialog.py +++ b/electrum/gui/qt/transaction_dialog.py @@ -46,7 +46,7 @@ from electrum.simple_config import SimpleConfig from electrum.util import quantize_feerate from electrum import bitcoin -from electrum.bitcoin import base_encode, NLOCKTIME_BLOCKHEIGHT_MAX, get_dummy_address +from electrum.bitcoin import base_encode, NLOCKTIME_BLOCKHEIGHT_MAX, DummyAddress from electrum.i18n import _ from electrum.plugin import run_hook from electrum import simple_config @@ -183,7 +183,7 @@ class TxInOutWidget(QWidget): fmt.setAnchor(True) fmt.setUnderlineStyle(QTextCharFormat.SingleUnderline) return fmt - elif sm and sm.is_lockup_address_for_a_swap(addr) or addr==get_dummy_address('swap'): + elif sm and sm.is_lockup_address_for_a_swap(addr) or addr == DummyAddress.SWAP: tf_used_swap = True return self.txo_color_swap.text_char_format elif self.wallet.is_billing_address(addr): diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 70d0ddcfe..62471d643 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -24,7 +24,7 @@ from . import constants from .util import (bfh, log_exceptions, ignore_exceptions, chunks, OldTaskGroup, UnrelatedTransactionException, error_text_bytes_to_safe_str) from . import transaction -from .bitcoin import make_op_return, get_dummy_address +from .bitcoin import make_op_return, DummyAddress from .transaction import PartialTxOutput, match_script_against_template, Sighash from .logging import Logger from .lnonion import (new_onion_packet, OnionFailureCode, calc_hops_data_for_payment, @@ -812,7 +812,7 @@ class Peer(Logger): redeem_script = funding_output_script(local_config, remote_config) funding_address = bitcoin.redeem_script_to_address('p2wsh', redeem_script) funding_output = PartialTxOutput.from_address_and_value(funding_address, funding_sat) - funding_tx.replace_dummy_output('channel', funding_address) + funding_tx.replace_output_address(DummyAddress.CHANNEL, funding_address) # find and encrypt op_return data associated to funding_address has_onchain_backup = self.lnworker and self.lnworker.has_recoverable_channels() if has_onchain_backup: diff --git a/electrum/lnworker.py b/electrum/lnworker.py index 2c150d5b4..0ba2d9d8a 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -56,7 +56,7 @@ from .lnchannel import ChannelState, PeerState, HTLCWithStatus from .lnrater import LNRater from . import lnutil from .lnutil import funding_output_script -from .bitcoin import redeem_script_to_address, get_dummy_address +from .bitcoin import redeem_script_to_address, DummyAddress from .lnutil import (Outpoint, LNPeerAddr, get_compressed_pubkey_from_bech32, extract_nodeid, PaymentFailure, split_host_port, ConnStringFormatError, @@ -1274,7 +1274,7 @@ class LNWallet(LNWorker): funding_sat: int, node_id: bytes, fee_est=None) -> PartialTransaction: - outputs = [PartialTxOutput.from_address_and_value(get_dummy_address('channel'), funding_sat)] + outputs = [PartialTxOutput.from_address_and_value(DummyAddress.CHANNEL, funding_sat)] if self.has_recoverable_channels(): dummy_scriptpubkey = make_op_return(self.cb_data(node_id)) outputs.append(PartialTxOutput(scriptpubkey=dummy_scriptpubkey, value=0)) @@ -2555,7 +2555,7 @@ class LNWallet(LNWorker): # check that we can send onchain swap_server_mining_fee = 10000 # guessing, because we have not called get_pairs yet swap_funding_sat = swap_recv_amount + swap_server_mining_fee - swap_output = PartialTxOutput.from_address_and_value(get_dummy_address('swap'), int(swap_funding_sat)) + swap_output = PartialTxOutput.from_address_and_value(DummyAddress.SWAP, int(swap_funding_sat)) if not self.wallet.can_pay_onchain([swap_output], coins=coins): continue return (chan, swap_recv_amount) diff --git a/electrum/network.py b/electrum/network.py index e59e17aca..3429414df 100644 --- a/electrum/network.py +++ b/electrum/network.py @@ -48,7 +48,7 @@ from .util import (log_exceptions, ignore_exceptions, OldTaskGroup, bfh, make_aiohttp_session, send_exception_to_crash_reporter, is_hash256_str, is_non_negative_integer, MyEncoder, NetworkRetryManager, nullcontext, error_text_str_to_safe_str) -from .bitcoin import COIN +from .bitcoin import COIN, DummyAddress, DummyAddressUsedInTxException from . import constants from . import blockchain from . import bitcoin @@ -923,6 +923,8 @@ class Network(Logger, NetworkRetryManager[ServerAddr]): raise RequestTimedOut() if timeout is None: timeout = self.get_network_timeout_seconds(NetworkTimeout.Urgent) + if any(DummyAddress.is_dummy_address(txout.address) for txout in tx.outputs()): + raise DummyAddressUsedInTxException("tried to broadcast tx with dummy address!") try: out = await self.interface.session.send_request('blockchain.transaction.broadcast', [tx.serialize()], timeout=timeout) # note: both 'out' and exception messages are untrusted input from the server diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index a622ef05d..c7e108b32 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -17,7 +17,7 @@ from .transaction import PartialTxInput, PartialTxOutput, PartialTransaction, Tr from .transaction import script_GetOp, match_script_against_template, OPPushDataGeneric, OPPushDataPubkey from .util import log_exceptions, BelowDustLimit, OldTaskGroup from .lnutil import REDEEM_AFTER_DOUBLE_SPENT_DELAY -from .bitcoin import dust_threshold, get_dummy_address +from .bitcoin import dust_threshold, DummyAddress from .logging import Logger from .lnutil import hex_to_bytes from .lnaddr import lndecode @@ -190,7 +190,7 @@ class SwapManager(Logger): self.wallet = wallet self.lnworker = lnworker self.taskgroup = None - self.dummy_address = get_dummy_address('swap') + self.dummy_address = DummyAddress.SWAP self.swaps = self.wallet.db.get_dict('submarine_swaps') # type: Dict[str, SwapData] self._swaps_by_funding_outpoint = {} # type: Dict[TxOutpoint, SwapData] @@ -706,13 +706,13 @@ class SwapManager(Logger): funding_output = PartialTxOutput.from_address_and_value(swap.lockup_address, swap.onchain_amount) tx = self.wallet.create_transaction(outputs=[funding_output], rbf=True, password=password) else: - tx.replace_dummy_output('swap', swap.lockup_address) + tx.replace_output_address(DummyAddress.SWAP, swap.lockup_address) tx.set_rbf(True) self.wallet.sign_transaction(tx, password) return tx @log_exceptions - async def request_swap_for_tx(self, tx): + async def request_swap_for_tx(self, tx: 'PartialTransaction'): for o in tx.outputs(): if o.address == self.dummy_address: change_amount = o.value @@ -725,7 +725,7 @@ class SwapManager(Logger): swap, invoice = await self.request_normal_swap( lightning_amount_sat = lightning_amount_sat, expected_onchain_amount_sat=change_amount) - tx.replace_dummy_output('swap', swap.lockup_address) + tx.replace_output_address(DummyAddress.SWAP, swap.lockup_address) return swap, invoice, tx @log_exceptions diff --git a/electrum/tests/test_wallet_vertical.py b/electrum/tests/test_wallet_vertical.py index 12ab201b6..e2b18f4fe 100644 --- a/electrum/tests/test_wallet_vertical.py +++ b/electrum/tests/test_wallet_vertical.py @@ -2891,6 +2891,27 @@ class TestWalletSending(ElectrumTestCase): self.assertEqual('d6823918ff82ed240995e9e6f02e0d2f3f15e0b942616ab34481ce8a3399dc72', tx.txid()) self.assertEqual('d6823918ff82ed240995e9e6f02e0d2f3f15e0b942616ab34481ce8a3399dc72', tx.wtxid()) + @mock.patch.object(wallet.Abstract_Wallet, 'save_db') + async def test_we_dont_sign_tx_including_dummy_address(self, mock_save_db): + wallet1 = self.create_standard_wallet_from_seed('bitter grass shiver impose acquire brush forget axis eager alone wine silver') + + # bootstrap wallet1 + funding_tx = Transaction('01000000014576dacce264c24d81887642b726f5d64aa7825b21b350c7b75a57f337da6845010000006b483045022100a3f8b6155c71a98ad9986edd6161b20d24fad99b6463c23b463856c0ee54826d02200f606017fd987696ebbe5200daedde922eee264325a184d5bbda965ba5160821012102e5c473c051dae31043c335266d0ef89c1daab2f34d885cc7706b267f3269c609ffffffff0240420f00000000001600148a28bddb7f61864bdcf58b2ad13d5aeb3abc3c42a2ddb90e000000001976a914c384950342cb6f8df55175b48586838b03130fad88ac00000000') + funding_txid = funding_tx.txid() + self.assertEqual('add2535aedcbb5ba79cc2260868bb9e57f328738ca192937f2c92e0e94c19203', funding_txid) + wallet1.adb.receive_tx_callback(funding_txid, funding_tx, TX_HEIGHT_UNCONFIRMED) + + # wallet1 -> dummy address + outputs = [PartialTxOutput.from_address_and_value(bitcoin.DummyAddress.CHANNEL, 250000)] + + with self.assertRaises(bitcoin.DummyAddressUsedInTxException): + tx = wallet1.mktx(outputs=outputs, password=None, fee=5000, tx_version=1, rbf=False) + + coins = wallet1.get_spendable_coins(domain=None) + tx = wallet1.make_unsigned_transaction(coins=coins, outputs=outputs, fee=5000) + with self.assertRaises(bitcoin.DummyAddressUsedInTxException): + wallet1.sign_transaction(tx, password=None) + class TestWalletOfflineSigning(ElectrumTestCase): TESTNET = True diff --git a/electrum/transaction.py b/electrum/transaction.py index eccc87a3e..fc25a70bc 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -52,7 +52,7 @@ from .bitcoin import (TYPE_ADDRESS, TYPE_SCRIPT, hash_160, from .crypto import sha256d from .logging import get_logger from .util import ShortID, OldTaskGroup -from .bitcoin import get_dummy_address +from .bitcoin import DummyAddress from .descriptor import Descriptor, MissingSolutionPiece, create_dummy_descriptor_from_address from .json_db import stored_in @@ -1156,7 +1156,7 @@ class Transaction: script = bitcoin.address_to_script(addr) return self.get_output_idxs_from_scriptpubkey(script) - def replace_output_address(self, old_address, new_address): + def replace_output_address(self, old_address: str, new_address: str) -> None: idx = list(self.get_output_idxs_from_address(old_address)) assert len(idx) == 1 amount = self._outputs[idx[0]].value @@ -1169,13 +1169,8 @@ class Transaction: def get_change_outputs(self): return [o for o in self._outputs if o.is_change] - def replace_dummy_output(self, purpose, new_address): - dummy_addr = get_dummy_address(purpose) - self.replace_output_address(dummy_addr, new_address) - - def has_dummy_output(self, purpose): - addr = get_dummy_address(purpose) - return len(self.get_output_idxs_from_address(addr)) == 1 + def has_dummy_output(self, dummy_addr: str) -> bool: + return len(self.get_output_idxs_from_address(dummy_addr)) == 1 def output_value_for_address(self, addr): # assumes exactly one output has that address diff --git a/electrum/util.py b/electrum/util.py index 678aa9d9e..c12d271de 100644 --- a/electrum/util.py +++ b/electrum/util.py @@ -2003,6 +2003,14 @@ class nullcontext: pass +class classproperty(property): + """~read-only class-level @property + from https://stackoverflow.com/a/13624858 by denis-ryzhkov + """ + def __get__(self, owner_self, owner_cls): + return self.fget(owner_cls) + + def get_running_loop() -> Optional[asyncio.AbstractEventLoop]: """Returns the asyncio event loop that is *running in this thread*, if any.""" try: diff --git a/electrum/wallet.py b/electrum/wallet.py index 0c14a2c08..53db51f6c 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -60,7 +60,8 @@ from .util import (NotEnoughFunds, UserCancelled, profiler, OldTaskGroup, ignore Fiat, bfh, TxMinedInfo, quantize_feerate, OrderedDictWithIndex) from .simple_config import SimpleConfig, FEE_RATIO_HIGH_WARNING, FEERATE_WARNING_HIGH_FEE from .bitcoin import COIN, TYPE_ADDRESS -from .bitcoin import is_address, address_to_script, is_minikey, relayfee, dust_threshold, get_dummy_address +from .bitcoin import is_address, address_to_script, is_minikey, relayfee, dust_threshold +from .bitcoin import DummyAddress, DummyAddressUsedInTxException from .crypto import sha256d from . import keystore from .keystore import (load_keystore, Hardware_KeyStore, KeyStore, KeyStoreWithMPK, @@ -1767,7 +1768,7 @@ class Abstract_Wallet(ABC, Logger, EventListener): amount = change[0].value ln_amount = self.lnworker.swap_manager.get_recv_amount(amount, is_reverse=False) if ln_amount and ln_amount <= self.lnworker.num_sats_can_receive(): - tx.replace_output_address(change[0].address, get_dummy_address('swap')) + tx.replace_output_address(change[0].address, DummyAddress.SWAP) else: # "spend max" branch # note: This *will* spend inputs with negative effective value (if there are any). @@ -2385,6 +2386,8 @@ class Abstract_Wallet(ABC, Logger, EventListener): return if not isinstance(tx, PartialTransaction): return + if any(DummyAddress.is_dummy_address(txout.address) for txout in tx.outputs()): + raise DummyAddressUsedInTxException("tried to sign tx with dummy address!") # note: swap signing does not require the password swap = self.get_swap_by_claim_tx(tx) if swap: