wallet: RBF batch payments manager

[ecdsa]

note: this is not "reorg safe", meaning that there is no guarantee that all payments will end up in in the blockchain.
However, I believe this is "double send safe", meaning that we will never send a payment twice.

[f321x]

I get this exception and electrum crashes when batching is enabled and trying to fully spend an unconfirmed utxo from the "Coins" tab with right click -> fully spend -> send to address in clipboard.

Traceback (most recent call last):
  File "/home/user/code/electrum-fork/electrum/gui/qt/confirm_tx_dialog.py", line 116, in timer_actions
    self.update()
    ~~~~~~~~~~~^^
  File "/home/user/code/electrum-fork/electrum/gui/qt/confirm_tx_dialog.py", line 120, in update
    self.update_tx()
    ~~~~~~~~~~~~~~^^
  File "/home/user/code/electrum-fork/electrum/gui/qt/confirm_tx_dialog.py", line 641, in update_tx
    self.tx = self.make_tx(fee_estimator, confirmed_only=confirmed_only)
              ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/code/electrum-fork/electrum/gui/qt/send_tab.py", line 320, in <lambda>
    make_tx = lambda fee_est, *, confirmed_only=False: self.wallet.make_unsigned_transaction(
                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        coins=get_coins(nonlocal_only=nonlocal_only, confirmed_only=confirmed_only),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        outputs=outputs,
        ^^^^^^^^^^^^^^^^
        fee=fee_est,
        ^^^^^^^^^^^^
        is_sweep=is_sweep)
        ^^^^^^^^^^^^^^^^^^
  File "/home/user/code/electrum-fork/electrum/util.py", line 492, in do_profile
    o = func(*args, **kw_args)
  File "/home/user/code/electrum-fork/electrum/wallet.py", line 1879, in make_unsigned_transaction
    base_tx = self.get_unconfirmed_base_tx_for_batching(outputs, coins)
  File "/home/user/code/electrum-fork/electrum/wallet.py", line 1774, in get_unconfirmed_base_tx_for_batching
    output_amount = sum(o.value for o in outputs)
TypeError: unsupported operand type(s) for +: 'int' and 'str'

[ecdsa]

File "/home/user/code/electrum-fork/electrum/wallet.py", line 1879, in make_unsigned_transaction
base_tx = self.get_unconfirmed_base_tx_for_batching(outputs, coins)

This does not correspond to the code in the batch_payment_manager branch.
get_unconfirmed_base_tx_for_batching is not used anymore (and should probably be removed)

[f321x]

Can confirm it doesn't happen on this branch, only on master.

[ecdsa]

Note: This PR does not work well with fee settings.

When we change the fee settings in ConfirmTxDialog, this side effects the fee settings in config, which is in turn used by TxBatcher. I think we should use a FeeEstimator object that holds fee settings and provides fee estimates, without committing the settings to config.

[SomberNight]

TxBatcher needs to be taught the 100 KB max tx size standardness rule. It must not try creating txs that are too large.

[SomberNight]

In general it is not safe to batch all lightning sweeps together, due to (1) pinning attacks and (2) split views of mempools due to distributed nature of network.

---

Perhaps the scariest problem is to do with revocations:

• counterparty force-closes using old state
• ctx has some htlcs
• counterparty creates a 2nd stage htlc tx ("tx1"), that spends one or more of the ctx's htlc outputs, and they add a lot of inputs/outputs to this tx (which they can do when using anchor channels due to sighash_single). They ensure tx1 is large (in vbytes), pays a low feerate so it does not confirm, but it pays a high absolute fee (as it is large).
• the local client then tries to punish the counterparty: we want to sweep the relevant htlc_tx outputs but we also want to sweep to_local
• if we batch the sweeping of those two utxos, to create "tx2", as "tx1" might ~never confirm, "tx2" won't get mined either.
• and in the meantime, the relative timelock (to_self_delay) on to_local is ticking. If that expires, the remote can just sweep it for themselves.

(see lightning/bolts#803)

The easiest workaround I believe would be to avoid batching all revocations. That is, any sweep where we use the revocation path, should be a completely separate 1-in-1-out tx.

---

However, it's not just about revocations. In general, I believe it is not safe to batch the sweeping of txos where some txos are spendable by both us and the remote, and some only by us. Perhaps we should have a separate "safe" batch for sweeping txos that are only spendable by us, and another for the risky txos.

One complication is that some utxos transition from "only spendable by us" to "spendable by either party" over time (~already when a timelock gets close to expiry, but especially after it expires).

I am not yet sure exactly how to handle this.

---

Also, btw, to simplify reasoning, I think we should consider strictly separating the batching of user-initiated txs (e.g. simple sends), and lnwatcher/swap_manager stuff.

[SomberNight]

(2) split views of mempools due to distributed nature of network

Example:

• at block 0, remote counterparty force-closes small-value chan1, the to_self_delay is 1000
• we are offline
• at block 1000, remote force-closes high-value chan2, the to_self_delay is 1000
• at block 1100, we come online - and the remote somehow learns of this instantly
  • they try to partition the mempool
  • "at the same time":
    • remote broadcasts tx1, that sweeps the to_local output of their chan1
      • they try to pin this tx to the mempool: it is large in size, low feerate, but highish absolute fee
    • we broadcast tx2, that sweeps both to_locals (from chan1 and chan2) in a single batched tx
• if the remote is successful, the mempool is now partitioned so that most nodes and miners have tx1, but our electrum server has tx2
  • the attacker's goal here is that neither tx1 nor tx2 gets mined
• now the attacker waits.
• at block 2000, the CSV for "high-value" chan2 has expired, and the remote broadcasts a 1-in-1-out tx that spends just that to_local