From 1364e7538acf37d1b649d2ca8ef647858540b227 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Sat, 19 Mar 2022 09:36:50 +0100 Subject: [PATCH 1/4] bump fee of swap claim transactions Note: This adds a new field, spent_txid, to PartialTxOutput --- electrum/address_synchronizer.py | 14 +++++++--- electrum/submarine_swaps.py | 44 ++++++++++++++++++++------------ electrum/transaction.py | 1 + electrum/wallet.py | 15 +++++++++-- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py index 8d213dac5..aaed906a7 100644 --- a/electrum/address_synchronizer.py +++ b/electrum/address_synchronizer.py @@ -761,10 +761,9 @@ class AddressSynchronizer(Logger): for tx_hash, height in h: l = self.db.get_txi_addr(tx_hash, address) for txi, v in l: - sent[txi] = height + sent[txi] = tx_hash, height return received, sent - def get_addr_outputs(self, address: str) -> Dict[TxOutpoint, PartialTxInput]: coins, spent = self.get_addr_io(address) out = {} @@ -775,7 +774,13 @@ class AddressSynchronizer(Logger): utxo._trusted_address = address utxo._trusted_value_sats = value utxo.block_height = tx_height - utxo.spent_height = spent.get(prevout_str, None) + if prevout_str in spent: + txid, height = spent[prevout_str] + utxo.spent_txid = txid + utxo.spent_height = height + else: + utxo.spent_txid = None + utxo.spent_height = None out[prevout] = utxo return out @@ -816,7 +821,8 @@ class AddressSynchronizer(Logger): else: u += v if txo in sent: - if sent[txo] > 0: + sent_txid, sent_height = sent[txo] + if sent_height > 0: c -= v else: u -= v diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index dd7d38327..405e2e491 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -99,7 +99,6 @@ def create_claim_tx( txin: PartialTxInput, witness_script: bytes, preimage: Union[bytes, int], # 0 if timing out forward-swap - privkey: bytes, address: str, amount_sat: int, locktime: int, @@ -117,10 +116,7 @@ def create_claim_tx( txin.witness_script = witness_script txout = PartialTxOutput.from_address_and_value(address, amount_sat) tx = PartialTransaction.from_io([txin], [txout], version=2, locktime=locktime) - #tx.set_rbf(True) - sig = bytes.fromhex(tx.sign_txin(0, privkey)) - witness = [sig, preimage, witness_script] - txin.witness = bytes.fromhex(construct_witness(witness)) + tx.set_rbf(True) return tx @@ -169,21 +165,23 @@ class SwapManager(Logger): return current_height = self.network.get_local_height() delta = current_height - swap.locktime - if not swap.is_reverse and delta < 0: - # too early for refund - return txos = self.lnwatcher.get_addr_outputs(swap.lockup_address) for txin in txos.values(): if swap.is_reverse and txin.value_sats() < swap.onchain_amount: self.logger.info('amount too low, we should not reveal the preimage') continue + swap.funding_txid = txin.prevout.txid.hex() spent_height = txin.spent_height if spent_height is not None: + swap.spending_txid = txin.spent_txid if spent_height > 0 and current_height - spent_height > REDEEM_AFTER_DOUBLE_SPENT_DELAY: self.logger.info(f'stop watching swap {swap.lockup_address}') self.lnwatcher.remove_callback(swap.lockup_address) swap.is_redeemed = True continue + if not swap.is_reverse and delta < 0: + # too early for refund + return # FIXME the mining fee should depend on swap.is_reverse. # the txs are not the same size... amount_sat = txin.value_sats() - self.get_claim_fee() @@ -201,17 +199,12 @@ class SwapManager(Logger): txin=txin, witness_script=swap.redeem_script, preimage=preimage, - privkey=swap.privkey, address=address, amount_sat=amount_sat, locktime=locktime, ) + self.sign_tx(tx, swap) await self.network.broadcast_transaction(tx) - # save txid - if swap.is_reverse: - swap.spending_txid = tx.txid() - else: - self.wallet.set_label(tx.txid(), 'Swap refund') def get_claim_fee(self): return self.wallet.config.estimate_fee(136, allow_fallback_to_static_rates=True) @@ -307,7 +300,7 @@ class SwapManager(Logger): dummy_output = PartialTxOutput.from_address_and_value(ln_dummy_address(), expected_onchain_amount_sat) tx.outputs().remove(dummy_output) tx.add_outputs([funding_output]) - tx.set_rbf(False) + tx.set_rbf(True) # rbf must not decrease payment self.wallet.sign_transaction(tx, password) # save swap data in wallet in case we need a refund swap = SwapData( @@ -321,7 +314,7 @@ class SwapManager(Logger): lightning_amount = lightning_amount_sat, is_reverse = False, is_redeemed = False, - funding_txid = tx.txid(), + funding_txid = None, spending_txid = None, ) self.swaps[payment_hash.hex()] = swap @@ -541,3 +534,22 @@ class SwapManager(Logger): if is_reverse and send_amount is not None: send_amount += self.get_claim_fee() return send_amount + + def get_swap_by_tx(self, tx): + # determine if tx is spending from a swap + txin = tx.inputs()[0] + for key, swap in self.swaps.items(): + if txin.prevout.txid.hex() == swap.funding_txid: + return swap + return None + + def sign_tx(self, tx, swap): + preimage = swap.preimage if swap.is_reverse else 0 + witness_script = swap.redeem_script + txin = tx.inputs()[0] + txin.script_type = 'p2wsh' + txin.script_sig = b'' + txin.witness_script = witness_script + sig = bytes.fromhex(tx.sign_txin(0, swap.privkey)) + witness = [sig, preimage, witness_script] + txin.witness = bytes.fromhex(construct_witness(witness)) diff --git a/electrum/transaction.py b/electrum/transaction.py index aa071fbfb..57cb89995 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -1210,6 +1210,7 @@ class PartialTxInput(TxInput, PSBTSection): self._trusted_address = None # type: Optional[str] self.block_height = None # type: Optional[int] # height at which the TXO is mined; None means unknown self.spent_height = None # type: Optional[int] # height at which the TXO got spent + self.spent_txid = None # type: Optional[str] # txid of the spender self._is_p2sh_segwit = None # type: Optional[bool] # None means unknown self._is_native_segwit = None # type: Optional[bool] # None means unknown diff --git a/electrum/wallet.py b/electrum/wallet.py index 5a7d1dd07..f056762fd 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -591,10 +591,14 @@ class Abstract_Wallet(AddressSynchronizer, ABC): return any([chan.funding_outpoint.txid == txid for chan in self.lnworker.channels.values()]) + def is_swap_tx(self, tx: Transaction) -> bool: + return bool(self.lnworker.swap_manager.get_swap_by_tx(tx)) if self.lnworker else False + def get_tx_info(self, tx: Transaction) -> TxWalletDetails: tx_wallet_delta = self.get_wallet_delta(tx) is_relevant = tx_wallet_delta.is_relevant is_any_input_ismine = tx_wallet_delta.is_any_input_ismine + is_swap = self.is_swap_tx(tx) fee = tx_wallet_delta.fee exp_n = None can_broadcast = False @@ -629,7 +633,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC): size = tx.estimated_size() fee_per_byte = fee / size exp_n = self.config.fee_to_depth(fee_per_byte) - can_bump = is_any_input_ismine and not tx.is_final() + can_bump = (is_any_input_ismine or is_swap) and not tx.is_final() can_dscancel = (is_any_input_ismine and not tx.is_final() and not all([self.is_mine(txout.address) for txout in tx.outputs()])) try: @@ -640,7 +644,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC): else: status = _('Local') can_broadcast = self.network is not None - can_bump = is_any_input_ismine and not tx.is_final() + can_bump = (is_any_input_ismine or is_swap) and not tx.is_final() else: status = _("Signed") can_broadcast = self.network is not None @@ -1961,6 +1965,8 @@ class Abstract_Wallet(AddressSynchronizer, ABC): for k in self.get_keystores(): if k.can_sign_txin(txin): return True + if self.is_swap_tx(tx): + return True return False def get_input_tx(self, tx_hash: str, *, ignore_network_issues=False) -> Optional[Transaction]: @@ -2013,6 +2019,11 @@ class Abstract_Wallet(AddressSynchronizer, ABC): return if not isinstance(tx, PartialTransaction): return + # note: swap signing does not require the password + swap = self.lnworker.swap_manager.get_swap_by_tx(tx) if self.lnworker else None + if swap: + self.lnworker.swap_manager.sign_tx(tx, swap) + return # add info to a temporary tx copy; including xpubs # and full derivation paths as hw keystores might want them tmp_tx = copy.deepcopy(tx) From 357aaff582836a755f4e4bfc9c5c829197061b72 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Wed, 30 Mar 2022 19:01:57 +0200 Subject: [PATCH 2/4] swaps: when RBF-ing a forward swap tx, payment amt must not decrease --- electrum/submarine_swaps.py | 8 +++++++- electrum/wallet.py | 21 +++++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index 405e2e491..30a07ec29 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -300,7 +300,7 @@ class SwapManager(Logger): dummy_output = PartialTxOutput.from_address_and_value(ln_dummy_address(), expected_onchain_amount_sat) tx.outputs().remove(dummy_output) tx.add_outputs([funding_output]) - tx.set_rbf(True) # rbf must not decrease payment + tx.set_rbf(True) # note: rbf must not decrease payment self.wallet.sign_transaction(tx, password) # save swap data in wallet in case we need a refund swap = SwapData( @@ -543,6 +543,12 @@ class SwapManager(Logger): return swap return None + def is_lockup_address_for_a_swap(self, addr: str) -> bool: + for key, swap in self.swaps.items(): # TODO take lock? or add index to avoid looping + if addr == swap.lockup_address: + return True + return False + def sign_tx(self, tx, swap): preimage = swap.preimage if swap.is_reverse else 0 witness_script = swap.redeem_script diff --git a/electrum/wallet.py b/electrum/wallet.py index f056762fd..5c7278e17 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -1713,11 +1713,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC): s = list(filter(lambda o: self.is_mine(o.address), outputs)) # ... unless there is none if not s: - s = outputs - x_fee = run_hook('get_tx_extra_fee', self, tx) - if x_fee: - x_fee_address, x_fee_amount = x_fee - s = list(filter(lambda o: o.address != x_fee_address, s)) + s = [out for out in outputs if self._is_rbf_allowed_to_touch_tx_output(out)] if not s: raise CannotBumpFee('No outputs at all??') @@ -1764,11 +1760,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC): # select non-ismine outputs s = [(idx, out) for (idx, out) in enumerate(outputs) if not self.is_mine(out.address)] - # exempt 2fa fee output if present - x_fee = run_hook('get_tx_extra_fee', self, tx) - if x_fee: - x_fee_address, x_fee_amount = x_fee - s = [(idx, out) for (idx, out) in s if out.address != x_fee_address] + s = [(idx, out) for (idx, out) in s if self._is_rbf_allowed_to_touch_tx_output(out)] if not s: raise CannotBumpFee("Cannot find payment output") @@ -1803,6 +1795,15 @@ class Abstract_Wallet(AddressSynchronizer, ABC): outputs = [out for (idx, out) in enumerate(outputs) if idx not in del_out_idxs] return PartialTransaction.from_io(inputs, outputs) + def _is_rbf_allowed_to_touch_tx_output(self, txout: TxOutput) -> bool: + # 2fa fee outputs if present, should not be removed or have their value decreased + if self.is_billing_address(txout.address): + return False + # submarine swap funding outputs must not be decreased + if self.lnworker and self.lnworker.swap_manager.is_lockup_address_for_a_swap(txout.address): + return False + return True + def cpfp(self, tx: Transaction, fee: int) -> Optional[PartialTransaction]: txid = tx.txid() for i, o in enumerate(tx.outputs()): From e36d7fed7da25ef4d4ef147d7c8c919ddafa04ea Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 31 Mar 2022 20:45:05 +0200 Subject: [PATCH 3/4] swaps: more precise tx size estimation for claim tx when RBF-ing --- electrum/submarine_swaps.py | 30 ++++++++++++++++++++++++++---- electrum/transaction.py | 3 +++ electrum/wallet.py | 9 ++++++--- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index 30a07ec29..dbe2fc99d 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -11,7 +11,7 @@ from .crypto import sha256, hash_160 from .ecc import ECPrivkey from .bitcoin import (script_to_p2wsh, opcodes, p2wsh_nested_script, push_script, is_segwit_address, construct_witness) -from .transaction import PartialTxInput, PartialTxOutput, PartialTransaction +from .transaction import PartialTxInput, PartialTxOutput, PartialTransaction, Transaction, TxInput from .transaction import script_GetOp, match_script_against_template, OPPushDataGeneric, OPPushDataPubkey from .util import log_exceptions from .lnutil import REDEEM_AFTER_DOUBLE_SPENT_DELAY, ln_dummy_address @@ -246,7 +246,7 @@ class SwapManager(Logger): assert self.lnwatcher privkey = os.urandom(32) pubkey = ECPrivkey(privkey).get_public_key_bytes(compressed=True) - lnaddr, invoice = await self.lnworker.create_invoice( + lnaddr, invoice = self.lnworker.create_invoice( amount_msat=lightning_amount_sat * 1000, message='swap', expiry=3600 * 24, @@ -535,9 +535,12 @@ class SwapManager(Logger): send_amount += self.get_claim_fee() return send_amount - def get_swap_by_tx(self, tx): + def get_swap_by_tx(self, tx: Transaction) -> Optional[SwapData]: # determine if tx is spending from a swap txin = tx.inputs()[0] + return self.get_swap_by_claim_txin(txin) + + def get_swap_by_claim_txin(self, txin: TxInput) -> Optional[SwapData]: for key, swap in self.swaps.items(): if txin.prevout.txid.hex() == swap.funding_txid: return swap @@ -549,10 +552,29 @@ class SwapManager(Logger): return True return False - def sign_tx(self, tx, swap): + def add_txin_info(self, txin: PartialTxInput) -> None: + """Add some info to a claim txin. + note: even without signing, this is useful for tx size estimation. + """ + swap = self.get_swap_by_claim_txin(txin) + if not swap: + return + preimage = swap.preimage if swap.is_reverse else 0 + witness_script = swap.redeem_script + txin.script_type = 'p2wsh' + txin.num_sig = 1 # hack so that txin not considered "is_complete" + txin.script_sig = b'' + txin.witness_script = witness_script + sig_dummy = b'\x00' * 71 # DER-encoded ECDSA sig, with low S and low R + witness = [sig_dummy, preimage, witness_script] + txin.witness_sizehint = len(bytes.fromhex(construct_witness(witness))) + + def sign_tx(self, tx: PartialTransaction, swap: SwapData) -> None: preimage = swap.preimage if swap.is_reverse else 0 witness_script = swap.redeem_script txin = tx.inputs()[0] + assert len(tx.inputs()) == 1, f"expected 1 input for swap claim tx. found {len(tx.inputs())}" + assert txin.prevout.txid.hex() == swap.funding_txid txin.script_type = 'p2wsh' txin.script_sig = b'' txin.witness_script = witness_script diff --git a/electrum/transaction.py b/electrum/transaction.py index 57cb89995..7e4d206ce 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -712,6 +712,8 @@ class Transaction: if not txin.is_segwit(): return construct_witness([]) + if estimate_size and txin.witness_sizehint is not None: + return '00' * txin.witness_sizehint if _type in ('address', 'unknown') and estimate_size: _type = cls.guess_txintype_from_address(txin.address) pubkeys, sig_list = cls.get_siglist(txin, estimate_size=estimate_size) @@ -1213,6 +1215,7 @@ class PartialTxInput(TxInput, PSBTSection): self.spent_txid = None # type: Optional[str] # txid of the spender self._is_p2sh_segwit = None # type: Optional[bool] # None means unknown self._is_native_segwit = None # type: Optional[bool] # None means unknown + self.witness_sizehint = None # type: Optional[int] # byte size of serialized complete witness, for tx size est @property def utxo(self): diff --git a/electrum/wallet.py b/electrum/wallet.py index 5c7278e17..88f4eb676 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -1929,10 +1929,13 @@ class Abstract_Wallet(AddressSynchronizer, ABC): address = self.get_txin_address(txin) # note: we add input utxos regardless of is_mine self._add_input_utxo_info(txin, ignore_network_issues=ignore_network_issues, address=address) - if not self.is_mine(address): + is_mine = self.is_mine(address) + if not is_mine: is_mine = self._learn_derivation_path_for_address_from_txinout(txin, address) - if not is_mine: - return + if not is_mine: + if self.lnworker: + self.lnworker.swap_manager.add_txin_info(txin) + return # set script_type first, as later checks might rely on it: txin.script_type = self.get_txin_type(address) txin.num_sig = self.m if isinstance(self, Multisig_Wallet) else 1 From 670f8dbe422f0da9463771a3c4b0e991c05738e9 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Fri, 1 Apr 2022 13:55:36 +0200 Subject: [PATCH 4/4] submarine_swaps: use prevout to determine if a txin is claiming a swap --- electrum/submarine_swaps.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index dbe2fc99d..540102de2 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -92,6 +92,7 @@ class SwapData(StoredObject): funding_txid = attr.ib(type=Optional[str]) spending_txid = attr.ib(type=Optional[str]) is_redeemed = attr.ib(type=bool) + _funding_prevout = None # for RBF def create_claim_tx( @@ -171,6 +172,7 @@ class SwapManager(Logger): self.logger.info('amount too low, we should not reveal the preimage') continue swap.funding_txid = txin.prevout.txid.hex() + swap._funding_prevout = txin.prevout spent_height = txin.spent_height if spent_height is not None: swap.spending_txid = txin.spent_txid @@ -542,7 +544,7 @@ class SwapManager(Logger): def get_swap_by_claim_txin(self, txin: TxInput) -> Optional[SwapData]: for key, swap in self.swaps.items(): - if txin.prevout.txid.hex() == swap.funding_txid: + if txin.prevout == swap._funding_prevout: return swap return None