Merge pull request #7724 from spesmilo/rbf_swap

bump fee of swap claim transactions

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

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
@@ -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(
@@ -99,7 +100,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 +117,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 +166,24 @@ 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()
+            swap._funding_prevout = txin.prevout
             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 +201,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 +302,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)  # 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(
@@ -321,7 +316,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 +536,50 @@ 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: 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 == swap._funding_prevout:
+                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 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
+        sig = bytes.fromhex(tx.sign_txin(0, swap.privkey))
+        witness = [sig, preimage, witness_script]
+        txin.witness = bytes.fromhex(construct_witness(witness))

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)
@@ -1210,8 +1212,10 @@ 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
+        self.witness_sizehint = None  # type: Optional[int]  # byte size of serialized complete witness, for tx size est
 
     @property
     def utxo(self):

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
@@ -1709,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??')
 
@@ -1760,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")
 
@@ -1799,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()):
@@ -1924,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
@@ -1961,6 +1969,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 +2023,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)