fundchannel: cancel channel on transaction broadcast error

[darosior]

As minimum mempool fee rose, i encountered some expected errors from bitcoind when funding channels (eg: Error broadcasting transaction: error code: -26\\nerror message:\\nmempool min fee not met, 154 < 200.), however i had to cancel half-started channel by hand. This makes fundchannel call fundchannel_cancel on transaction broadcast error.

[ZmnSCPxj]

I believe there is an edge-case possibility below:

• The bitcoind is remote, not local.
• The bitcoin-cli command sends the request to add the funding tx to the mempool.to the remote bitcoind.
• The bitcoind successfully adds the transaction to its mempool.
• The network connection between us and the bitcoind is interrupted.
• We receive an error from bitcoin-cli due to the network interruption (bitcoin-cli never received confirmation that the command succeeded).
• We fundchannel_cancel the channel.
• The bitcoind propagates the funding transaction, which eventually hits the miners.
• Miners mine the funding transaction.
• We already forgot the channel and all the necessary data to recover our funds from the channel.

Even if we do mempool monitoring, if the bitcoind ever sent out the transaction, there is a small chance still that it was propagated outwards, potentially all the way out to miners, before getting evicted by the local node mempool.

So I think it is dangerous to assume that failure of the sendrawtransaction is automatically something we should fundchannel_cancel. We can only safely fundchannel_cancel if we double-spend the funding transaction, which is more complicated.

In order to implement double-spending the funding transaction we should create an alternative RBF of the funding tx that pays to ourself only (call it the "cancellation tx") and spends the same set of inputs. Then if sendpsbt fails, enter into an infinite loop where we generate the cancellation tx, and keep spamming sendpsbt on the cancellation tx until either the cancellation transaction or the funding tx is deeply confirmed, and only fundchannel_cancel if the cancellation transaction is deeply confirmed.

That also means we need to suppress the sendpsbt behavior of automatically adding change outputs as unconfirmed utxos we own.

@rustyrussell I think we need to separate the "gather change outputs of psbt" from the "send out a psbt" commands, which is one reason I proposed gatherpsbt. So the logic would be:

• If the sendpsbt on the funding tx succeeds:
  • gatherpsbt the funding tx, and return success.
  • Queue up some background thread to keep reposting the funding transaction.
• If the sendpsbt on the funding tx fails:
  • Generate a new cancellation tx with higher feerate and signpsbt it.
  • Queue it up on some backgorund thread to keep reposting the cancellation transaction.
  • If the cancellation tx is deeply confirmed:
    • fundchannel_cancel the channel.
    • (no need to gatherpsbt, the normal onchain scanning should see it)\
    • return failure
  • If the funding tx is deeply confirmed:
    • do nothing and return success
    • (gatherpsbt also not needed for similar reasons)

This also means that fundchannel should save this information in persistent storage as well..... sigh.

[niftynei]

Generally speaking, I think this issue points to a good question, namely: should a broadcast error be automatically cleaned up, or should we allow the user some discretion in resolving the error?

The corner case that @ZmnSCPxj pointed out leads me to think that perhaps alerting the user and allowing them to take action is perhaps the best course in this situation.

In this case, we should consider improving our error messages that we report back to the user, perhaps with the suggestion of ways to fix it. This is probably enough of a corner case that expecting manual intervention is warranted?

otoh, if it's just a case of confirming that the transaction has not in fact been broadcast, unbeknownst to us, i believe that fundchannel_cancel now checks for the broadcast status of a tx before completing the cancellation. As long 1) we don't execute a cancellation of the channel if bitcoind is non-responsive and 2) we're using the same bitcoind backend to broadcast + to confirm the broadcast, then automatically calling fundchannel_cancel seems like the reasonable action to take.

[niftynei]

It seems like the real solution to this specific problem (feerates rising faster than your ability to publish txs) however, is to allow RBF for channel opens, which will be possible with the v2 of channel opens.

Our updated PSBT RPC interface will allow us to theoretically to offer this for v1, but will require some non-standard erroring out of existing/established channels and basically the same as canceling a channel and restarting the fundchannel call... which is bascially the blueprint of what this PR is starting down...

[niftynei]

Oh, one more thing. If we wanted to take specific action for how to handle specific bitcoin backend error codes (i.e. this transaction failed to broadcast because of fees), that might be a more robust / safe way to automatically cancel channel opens? Just a thought. I don't love the idea of integrating bitcoind error codes, but maybe that's ok.

[ZmnSCPxj]

@niftynei I described how to do it with funding v1 a long time ago:https://lists.linuxfoundation.org/pipermail/lightning-dev/2018-January/000889.html

Unfortunately that requires support for multiple channels per peer on both sides, and we have a policy of allowing only one channel per peer.

Note that it is not safe to cancel any version of a channel until you have deeply confirmed a transaction that double-spends the funding tx of that version of the channel!

The best we in lightningd can do with funding v1 is #475, which requires us to have a change output (i.e. anchor output), even if we would otherwise not need a change output.

Also, note that the specific problem @darosior is trying to fix is having a feerate lower than the minimum relay rate of your peers, RBF might not help there.

[ZmnSCPxj]

Oh, one more thing. If we wanted to take specific action for how to handle specific bitcoin backend error codes (i.e. this transaction failed to broadcast because of fees), that might be a more robust / safe way to automatically cancel channel opens? Just a thought. I don't love the idea of integrating bitcoind error codes, but maybe that's ok.

Please do not make #3858 harder than it already is!

[darosior]

I like the idea of specific errors from the backend, but of course not specific to bitcoind. This can be made backward compatible, i think.

[rustyrussell]

Firstly, yes, there is a danger here in cancelling on any bitcoind failure: good catch @ZmnSCPxj !

We can, however, and should cancel in the specific case of known errors.

---

In general, we need to shepherd our transactions properly: @ZmnSCPxj has pointed this our before. This is not just for funding a transaction, but includes RBF. outgoing payments to others, penalty transactions, HTLC transactions, commitment transactions, etc. It also includes noticing when a transaction is no longer possible (an input is spent by another tx, to sufficient depth).

Fundamentally, we care about deadlines, unspendability and outputs.

1. Funding outputs. Deadline: forget, unilateral close or doublespend (maybe negotiate RBF in future). Unspendable: call the channel cleanup.
2. Withdrawl outputs. Deadline: loose (I suggest: hour, day, week), RBF when reached. Unspendable: re-create the output using a new tx if replacement doesn't have same address.
   (Variant: to-self withdrawls, like to cold storage, can be replaced by updates with greater/lesser amounts, or even a new addr).
3. Commitment transaction outputs. Deadline: depends on timeout, CPFP (option_anchor_outputs) when reached. Unspendable: tell onchaind about the replacing tx.
4. HTLC transaction outputs. Deadline: per-HTLC, RBF if option_anchor_outputs. Unspendable: tell onchaind.
5. Penalty transaction outputs. Deadline: depends on timeout, RBF when reached. Unspendable: tell onchaind.

Note that various transactions can be usefully combined, particularly the SIGHASH_SINGLE HTLC transactions with option_anchor_inputs, and potentially we could do the same with penalty transactions (which are currently one-tx-per-output for simplicity).

Thoughts?

[ZmnSCPxj]

Funding outputs. Deadline: forget, unilateral close or doublespend (maybe negotiate RBF in future). Unspendable: call the channel cleanup.

If we double-spend, then any change outputs on the funding transaction become invalidated, and if we later use that change output to anchor a different tx (such as a withdraw or another funding tx) then double-spending to self will also invalidate the subsequent tx.

This is particularly saddening if you send a single coin to lightningd, then open a channel with partial funds (so a change output is generated), then some time later, with the first channel not yet confirmed, open another channel. If the first channel reaches the deadline that leads us to RBF-double-spend it, the second transaction is also invalidated due to the change output disappearing, so the deadline of the second channel is actually the nearer of the deadline of the first and second channels.

multifundchannel would help slightly, but it requires that you decide on all channels you are funding right now; if you want to open one channel now, and reserve the option to open another channel to an arbitrary node later, multifundchannel does not help.

We can safely forget channel fundings where we have no money in (i.e. acceptor in fundchannelv1, or fundchannelv2 if we do not add any of our own money).

We already have an implicit deadline for fundings, sort of. If the channel remains unconfirmed for a long time, if we are the acceptor, we forget it. If the initiator then mentions it after we forgot, we error at them, which causes the initiator to drop it unilaterally.

---

If we are going to maintain deadlines, we want data about those deadlines to be stored in permanent storage, in case of a restart in the middle of a deadline. If we are managing our fundings and withdrawals in a separate plugin, then those plugins need to store the information about those transactions on-disk. That probably means putting a separate database for each plugin, since a database would give us some assurance of the atomicity and reliability of the information storage, which we would have to reimplement if we used our own storage mechanism.

---

A lot of the complexity here can be simplified by separating the onchain-related transactions (funding channels, withdrawing) from the Lightning-related ones (commitment, HTLC, penalty).

---

Penalty transaction outputs. Deadline: depends on timeout, RBF when reached. Unspendable: tell onchaind.

Whut. Deadline is when the thief can steal from you; if you reach that, you should prefer to burn the fund (i.e. send to an OP_RETURN with all the money as mining fees), since at least a random miner might not be in cahoots with the thief, whereas letting the thief get even 1 satoshi is risky since it invites attack. You should RBF well before then. xref #3870, which really needs more reviews.

[cdecker]

I have to agree with the other reviewers that this is too simplistic in its current form, even without the complexity of @ZmnSCPxj remote bitcoind, the return codes from bitcoin-cli sendrawtransaction are far from easy to interpret. Consider for example that it'll return "missing inputs" if we submit the same transaction twice, and "already in blockchain" once it gets confirmed (and at least one outpoint is still unspent, otherwise it reverts to "missing inputs").

Shepherding transactions we send, and overriding with RBF if necessary, before considering the attempt cancelled is really the only option I see.

[cdecker]

Suggested change

	json_add_string(req->js, "id", node_id_to_hexstr(tmpctx, fr->id));
	json_add_node_id(req->js, "id", fr->id);

[darosior]

i completely agree as well and was keeping it open to not stop discussions (and to evaluate a possible move to standard errors on the Bitcoin plugin API).

I'm going to close this and rather open an issue as i'm not going to implement any to the proposed solutions soon.

[darosior]

#3970

[3nprob]

Is it considered safe calling this for a channel-opening attempt that had the fee low enough that it now has fallen out of the mempool? Channel is still in CHANNELD_AWAITING_LOCKIN

[cdecker]

Is it considered safe calling this for a channel-opening attempt that had the fee low enough that it now has fallen out of the mempool? Channel is still in CHANNELD_AWAITING_LOCKIN

There is no real way to know whether a transaction has been dropped from the mempool of all nodes. Node operators may adjust their mempool size, and potentially rebroadcast low-fee transactions at any time. We know some mining pools do this to stuff their blocks in low-fee periods.