From d5358215160de8b6417746d81520b540ccde3ac8 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Mon, 15 Nov 2021 14:23:33 +0100 Subject: [PATCH] htlctx: deal with possible peer htlctx batching Due to anchor channel's sighash.SINGLE and sighash.ANYONECANPAY, several HTLC-transactions can be combined. This means we must watch for revoked outputs in the HTLC transaction not only at index 0 but at any index. --- electrum/lnchannel.py | 14 +++---- electrum/lnsweep.py | 96 +++++++++++++++++++++++++++---------------- electrum/lnwatcher.py | 12 +++--- 3 files changed, 73 insertions(+), 49 deletions(-) diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py index b63054fdd..7851f6bd1 100644 --- a/electrum/lnchannel.py +++ b/electrum/lnchannel.py @@ -58,7 +58,7 @@ from .lnutil import (Outpoint, LocalConfig, RemoteConfig, Keypair, OnlyPubkeyKey received_htlc_trim_threshold_sat, make_commitment_output_to_remote_address, FIXED_ANCHOR_SAT, ChannelType, LNProtocolWarning, ctx_has_anchors) from .lnsweep import txs_our_ctx, txs_their_ctx -from .lnsweep import tx_their_htlctx_justice, SweepInfo +from .lnsweep import txs_their_htlctx_justice, SweepInfo from .lnsweep import tx_their_ctx_to_remote_backup from .lnhtlc import HTLCManager from .lnmsg import encode_msg, decode_msg @@ -284,10 +284,10 @@ class AbstractChannel(Logger, ABC): def delete_closing_height(self): self.storage.pop('closing_height', None) - def create_sweeptxs_for_our_ctx(self, ctx): + def create_sweeptxs_for_our_ctx(self, ctx: Transaction) -> Optional[Dict[str, SweepInfo]]: return txs_our_ctx(chan=self, ctx=ctx, sweep_address=self.get_sweep_address()) - def create_sweeptxs_for_their_ctx(self, ctx): + def create_sweeptxs_for_their_ctx(self, ctx: Transaction) -> Optional[Dict[str, SweepInfo]]: return txs_their_ctx(chan=self, ctx=ctx, sweep_address=self.get_sweep_address()) def is_backup(self) -> bool: @@ -603,8 +603,8 @@ class ChannelBackup(AbstractChannel): else: return - def maybe_sweep_revoked_htlc(self, ctx: Transaction, htlc_tx: Transaction) -> Optional[SweepInfo]: - return None + def maybe_sweep_revoked_htlcs(self, ctx: Transaction, htlc_tx: Transaction) -> Dict[int, SweepInfo]: + return {} def extract_preimage_from_htlc_txin(self, txin: TxInput) -> None: return None @@ -1726,9 +1726,9 @@ class Channel(AbstractChannel): assert not (self.get_state() == ChannelState.WE_ARE_TOXIC and ChanCloseOption.LOCAL_FCLOSE in ret), "local force-close unsafe if we are toxic" return ret - def maybe_sweep_revoked_htlc(self, ctx: Transaction, htlc_tx: Transaction) -> Optional[SweepInfo]: + def maybe_sweep_revoked_htlcs(self, ctx: Transaction, htlc_tx: Transaction) -> Dict[int, SweepInfo]: # look at the output address, check if it matches - return tx_their_htlctx_justice(self, ctx, htlc_tx, self.get_sweep_address()) + return txs_their_htlctx_justice(self, ctx, htlc_tx, self.get_sweep_address()) def has_pending_changes(self, subject: HTLCOwner) -> bool: next_htlcs = self.hm.get_htlcs_in_next_ctx(subject) diff --git a/electrum/lnsweep.py b/electrum/lnsweep.py index ec5864823..43182928c 100644 --- a/electrum/lnsweep.py +++ b/electrum/lnsweep.py @@ -4,6 +4,7 @@ from typing import Optional, Dict, List, Tuple, TYPE_CHECKING, NamedTuple, Callable from enum import Enum, auto +from functools import partial import electrum_ecc as ecc @@ -36,6 +37,7 @@ _logger = get_logger(__name__) HTLC_TRANSACTION_DEADLINE_FRACTION = 4 HTLC_TRANSACTION_SWEEP_TARGET = 10 +HTLCTX_INPUT_OUTPUT_INDEX = 0 class SweepInfo(NamedTuple): @@ -90,8 +92,9 @@ def txs_their_ctx_watchtower(chan: 'Channel', ctx: Transaction, per_commitment_s commit=ctx, htlc=htlc, ctx_output_idx=ctx_output_idx) - return tx_sweep_our_htlctx( + return tx_sweep_htlctx_output( htlc_tx=htlc_tx, + output_idx=HTLCTX_INPUT_OUTPUT_INDEX, htlctx_witness_script=htlc_tx_witness_script, sweep_address=sweep_address, privkey=other_revocation_privkey, @@ -147,19 +150,23 @@ def tx_their_ctx_justice( return None -def tx_their_htlctx_justice( +def txs_their_htlctx_justice( chan: 'Channel', ctx: Transaction, htlc_tx: Transaction, - sweep_address: str) -> Optional[SweepInfo]: - + sweep_address: str) -> Dict[int, SweepInfo]: + """Creates justice transactions for every output in the HTLC transaction. + Due to anchor type channels it can happen that a remote party batches HTLC transactions, + which is why this method can return multiple SweepInfos. + """ x = extract_ctx_secrets(chan, ctx) if not x: - return + return {} ctn, their_pcp, is_revocation, per_commitment_secret = x if not is_revocation: - return - # prep + return {} + + # get HTLC constraints (secrets and locktime) pcp = ecc.ECPrivkey(per_commitment_secret).get_public_key_bytes(compressed=True) this_conf, other_conf = get_ordered_channel_configs(chan=chan, for_us=False) other_revocation_privkey = derive_blinded_privkey( @@ -167,26 +174,38 @@ def tx_their_htlctx_justice( per_commitment_secret) to_self_delay = other_conf.to_self_delay this_delayed_pubkey = derive_pubkey(this_conf.delayed_basepoint.pubkey, pcp) - # same witness script as to_local revocation_pubkey = ecc.ECPrivkey(other_revocation_privkey).get_public_key_bytes(compressed=True) + # uses the same witness script as to_local witness_script = make_commitment_output_to_local_witness_script( revocation_pubkey, to_self_delay, this_delayed_pubkey) htlc_address = redeem_script_to_address('p2wsh', witness_script) - # check that htlc_tx is a htlc - if htlc_tx.outputs()[0].address != htlc_address: - return - gen_tx = lambda: tx_sweep_our_htlctx( - sweep_address=sweep_address, - htlc_tx=htlc_tx, - htlctx_witness_script=witness_script, - privkey=other_revocation_privkey, - is_revocation=True, - config=chan.lnworker.config) - return SweepInfo( - name='redeem_htlc2', - csv_delay=0, - cltv_abs=0, - gen_tx=gen_tx) + # check that htlc transaction contains at least an output that is supposed to be + # spent via a second stage htlc transaction + htlc_outputs_idxs = [idx for idx, output in enumerate(htlc_tx.outputs()) if output.address == htlc_address] + if not htlc_outputs_idxs: + return {} + + # generate justice transactions + def justice_tx(output_idx): + return tx_sweep_htlctx_output( + sweep_address=sweep_address, + output_idx=output_idx, + htlc_tx=htlc_tx, + htlctx_witness_script=witness_script, + privkey=other_revocation_privkey, + is_revocation=True, + config=chan.lnworker.config + ) + index_to_sweepinfo = {} + for output_idx in htlc_outputs_idxs: + index_to_sweepinfo[output_idx] = SweepInfo( + name='redeem_htlc2', + csv_delay=0, + cltv_abs=0, + gen_tx=partial(justice_tx, output_idx) + ) + + return index_to_sweepinfo def txs_our_ctx( @@ -281,21 +300,25 @@ def txs_our_ctx( htlc_direction=htlc_direction, ctx_output_idx=ctx_output_idx, htlc_relative_idx=htlc_relative_idx) - sweep_tx = lambda: tx_sweep_our_htlctx( + # we sweep our ctx with HTLC transactions individually, therefore the CSV-locked output is always at + # index TIMELOCKED_HTLCTX_OUTPUT_INDEX + assert True + sweep_tx = lambda: tx_sweep_htlctx_output( to_self_delay=to_self_delay, htlc_tx=htlc_tx, + output_idx=HTLCTX_INPUT_OUTPUT_INDEX, htlctx_witness_script=htlctx_witness_script, sweep_address=sweep_address, privkey=our_localdelayed_privkey.get_secret_bytes(), is_revocation=False, config=chan.lnworker.config) # side effect - txs[htlc_tx.inputs()[0].prevout.to_str()] = SweepInfo( + txs[htlc_tx.inputs()[HTLCTX_INPUT_OUTPUT_INDEX].prevout.to_str()] = SweepInfo( name='first-stage-htlc', csv_delay=0, cltv_abs=htlc_tx.locktime, gen_tx=lambda: htlc_tx) - txs[htlc_tx.txid() + ':0'] = SweepInfo( + txs[htlc_tx.txid() + f':{HTLCTX_INPUT_OUTPUT_INDEX}'] = SweepInfo( name='second-stage-htlc', csv_delay=to_self_delay, cltv_abs=0, @@ -645,16 +668,16 @@ def tx_our_ctx_htlctx( htlc_outpoint = TxOutpoint(txid=bfh(ctx.txid()), out_idx=ctx_output_idx) htlc_input_idx = funded_htlc_tx.get_input_idx_that_spent_prevout(htlc_outpoint) - htlc_out_address = maybe_zero_fee_htlc_tx.outputs()[0].address + htlc_out_address = maybe_zero_fee_htlc_tx.outputs()[HTLCTX_INPUT_OUTPUT_INDEX].address htlc_output_idx = funded_htlc_tx.get_output_idxs_from_address(htlc_out_address).pop() inputs = funded_htlc_tx.inputs() outputs = funded_htlc_tx.outputs() - if htlc_input_idx != 0: + if htlc_input_idx != HTLCTX_INPUT_OUTPUT_INDEX: htlc_txin = inputs.pop(htlc_input_idx) - inputs.insert(0, htlc_txin) - if htlc_output_idx != 0: + inputs.insert(HTLCTX_INPUT_OUTPUT_INDEX, htlc_txin) + if htlc_output_idx != HTLCTX_INPUT_OUTPUT_INDEX: htlc_txout = outputs.pop(htlc_output_idx) - outputs.insert(0, htlc_txout) + outputs.insert(HTLCTX_INPUT_OUTPUT_INDEX, htlc_txout) final_htlc_tx = PartialTransaction.from_io( inputs, outputs, @@ -677,8 +700,8 @@ def tx_our_ctx_htlctx( # sign HTLC output remote_htlc_sig = chan.get_remote_htlc_sig_for_htlc(htlc_relative_idx=htlc_relative_idx) - local_htlc_sig = final_htlc_tx.sign_txin(0, local_htlc_privkey) - txin = final_htlc_tx.inputs()[0] + local_htlc_sig = final_htlc_tx.sign_txin(HTLCTX_INPUT_OUTPUT_INDEX, local_htlc_privkey) + txin = final_htlc_tx.inputs()[HTLCTX_INPUT_OUTPUT_INDEX] witness_script_in = txin.witness_script assert witness_script_in txin.witness = make_htlc_tx_witness(remote_htlc_sig, local_htlc_sig, preimage, witness_script_in) @@ -693,6 +716,7 @@ def tx_their_ctx_htlc( config: SimpleConfig, has_anchors: bool, ) -> Optional[PartialTransaction]: + """Deals with normal (non-CSV timelocked) HTLC output sweeps.""" assert type(cltv_abs) is int preimage = preimage or b'' # preimage is required iff (not is_revocation and htlc is offered) val = ctx.outputs()[output_idx].value @@ -801,8 +825,8 @@ def tx_ctx_to_local( -def tx_sweep_our_htlctx( - *, htlc_tx: Transaction, htlctx_witness_script: bytes, sweep_address: str, +def tx_sweep_htlctx_output( + *, htlc_tx: Transaction, output_idx: int, htlctx_witness_script: bytes, sweep_address: str, privkey: bytes, is_revocation: bool, to_self_delay: int = None, config: SimpleConfig) -> Optional[PartialTransaction]: """Create a txn that sweeps the output of a first stage htlc tx @@ -813,7 +837,7 @@ def tx_sweep_our_htlctx( return tx_ctx_to_local( sweep_address=sweep_address, ctx=htlc_tx, - output_idx=0, + output_idx=output_idx, witness_script=htlctx_witness_script, privkey=privkey, is_revocation=is_revocation, diff --git a/electrum/lnwatcher.py b/electrum/lnwatcher.py index 82d6289b0..b6370ab63 100644 --- a/electrum/lnwatcher.py +++ b/electrum/lnwatcher.py @@ -463,14 +463,14 @@ class LNWalletWatcher(LNWatcher): spender_tx = self.adb.get_transaction(spender_txid) if spender_txid else None if spender_tx: # the spender might be the remote, revoked or not - e_htlc_tx = chan.maybe_sweep_revoked_htlc(closing_tx, spender_tx) - if e_htlc_tx: - spender2 = spenders.get(spender_txid+':0') - if spender2: - keep_watching |= not self.is_deeply_mined(spender2) + htlc_idx_to_sweepinfo = chan.maybe_sweep_revoked_htlcs(closing_tx, spender_tx) + for idx, htlc_revocation_sweep_info in htlc_idx_to_sweepinfo.items(): + htlc_tx_spender = spenders.get(spender_txid+f':{idx}') + if htlc_tx_spender: + keep_watching |= not self.is_deeply_mined(htlc_tx_spender) else: keep_watching = True - await self.maybe_redeem(spenders, spender_txid+':0', e_htlc_tx, name) + await self.maybe_redeem(spenders, spender_txid+f':{idx}', htlc_revocation_sweep_info, name) else: keep_watching |= not self.is_deeply_mined(spender_txid) txin_idx = spender_tx.get_input_idx_that_spent_prevout(TxOutpoint.from_str(prevout))