Dropped funding transactions should be reattempted

[whitslack]

Issue and Steps to Reproduce

C-Lightning creates a zombie channel that will remain in the CHANNELD_AWAITING_LOCKIN state indefinitely if BitcoinD rejected the initial attempt to broadcast the channel's funding transaction because the transaction paid a fee rate lower than BitcoinD's mempoolminfee.

1. Configure BitcoinD with a small maxmempool.
2. Start BitcoinD and wait for the mempool to fill up.
3. Examine mempoolminfee in the output of bitcoin-cli getmempoolinfo.
4. Execute lightning-cli fundchannel with a feerate less than mempoolminfee.
5. BitcoinD will immediately drop the funding transaction from its mempool without broadcasting it to the network. C-Lightning puts the channel in CHANNELD_AWAITING_LOCKIN state, where it will remain forever.

Even though mempoolminfee may eventually decay below the funding transaction's fee rate, C-Lightning will never reattempt to broadcast the funding transaction.

Attempted Workarounds

I have unsuccessfully tried several strategies to rebroadcast the dropped funding transactions:

• Manually disconnecting the peers (with lightning-cli disconnect) doesn't work. They automatically reconnect, but C-Lightning does not resubmit their channels' funding transactions to BitcoinD.
• The raw transactions returned by lightning-cli listtransactions are unsigned and thus cannot be submitted to bitcoin-cli sendrawtransaction.

  $ lightning-cli listtransactions |
      jq -r '.transactions[] | .hash + " " + .rawtx' |
      fgrep -f <(lightning-cli listpeers | jq -r '.peers[] | .channels[] | select(.state == "CHANNELD_AWAITING_LOCKIN") | .funding_txid') |
      cut -d' ' -f2 |
      xargs -d'\n' -n1 bitcoin-cli sendrawtransaction
  error code: -26
  error message:
  non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)
  ⋮
  

• Querying the database directly also yields only unsigned raw transactions.

  lightning=> SELECT * FROM transactions WHERE id IN (SELECT funding_tx_id FROM channels WHERE state = 2);
                                   id                                 | blockheight | txindex |                                                                                                                            rawtx                                                                                                                             | type | channel_id
  --------------------------------------------------------------------+-------------+---------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+------+------------
   \xd04ce56c1b6ce90e8a24098b04c5fcd2a8c87d33f4bb391f099b188ceceeac6b |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f00000000002200201a3d1cdca8894e60cbf0bc384e68817713551892b7e4fd557797dbc216d81521b4d7380000000000160014baa198a7332115dfa5bcb536c72c0a3206b289ec00000000 |    0 |
   \x58b7ad19eb7372a14473c3ddde4888b66f2950b7ef6645a65be1205449753aec |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f00000000002200208df905e88a0e41b50572576563f43a57e7537ff6b699dca33071d17ec3a1dd0db4d7380000000000160014b6322e9bed6d99a2361c20dec01ea15260562bc600000000 |    0 |
   \x071315b96b455472208dabe89fc78619d61453c78155d7f658cb7643dda687ef |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f0000000000220020e4910cde8a601a30823179df263fa6cae50ea8ee2dc9c050c84ad787bb7a16d3b4d7380000000000160014e4d9e98b4f722880695c85a8e60f940a6397f72300000000 |    0 |
   \x408cca21d986780e15b9cf9f67af3bb4d9b0d9a9cde8d1dc82822a6b6ede45b3 |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f0000000000220020f542cdb9c44c98e249aeb94a743997d357f21a290f3bd8cd74c7296c9a057bd9b4d7380000000000160014df1991b5805cdf5c93b3811cd42763922b677e1300000000 |    0 |
   \xe9e12feea55c673cb9a90a2fa469700b4005ab54919cf851ce14fbcaf90b18cd |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f000000000022002087ac25cced60f92e6442984bf76bdbc36d59b54c6d29250aefbbeaca9850a752b4d73800000000001600140f91a76103252285e57ffc2971ec5e67a463833800000000 |    0 |
   \x1575804269cd3980201ae49b2af373e7b612abfa7b83c009cc16a0a4ac04b2c0 |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f000000000022002036999730aff3abd15d2aabb0477370d7d5b547d7c7d6b2df2cfc0535c513c6aab4d73800000000001600143922745e651b0dfdfd022a48a1fcdf12b994ef8500000000 |    0 |
   \xefcf21f7a0f6a4eb902c1ad56cc203ab4ccc193ef517753f867a99d02e2bfac8 |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f0000000000220020418ff5587dd9e6576f4c742ccab0184d3476591ed751b6146dd163e4695a8a2db4d7380000000000160014bd9e1db36d574b4b30fb8d59b3bac182bd4b97aa00000000 |    0 |
   \x96e288c1c76d49fc838e6b75f84dc97abe39dccc9e061e68881b570984a02e8a |             |         | \x0200000001d8bfcd1b21a1336864a805f6eeb1e8b967b2ff875c3b3f1dea92dd09f0c175230100000000feffffff0240420f00000000002200201320a4829ba6dd6b4e5e44927bbac76bf9ce331c1cc49984e40572621cb25143b4d7380000000000160014cfb4ae62797f0ba659d6153836dbb1bbffee280d00000000 |    0 |
  (8 rows)
  

  Note: In my case, they all attempt to spend the same UTxO. I guess C-Lightning was aware that the broadcast failed in each case, and so it didn't mark the UTxO as spent. But if it was aware that the broadcasts failed, then why did it leave the channels in the CHANNELD_AWAITING_LOCKIN state?

Possible Resolutions

• C-Lightning should periodically reattempt to broadcast any funding transaction that was rejected due to having a fee rate below mempoolminfee. It must internally mark the funding transaction's inputs as spent so that it doesn't try to spend them elsewhere.
• Alternatively, C-Lightning may precede its call to sendrawtransaction with a call to prioritisetransaction so that BitcoinD will not drop the transaction from its mempool (and will broadcast it), even if its fee rate is lower than mempoolminfee.

[darosior]

I like the prioritisetransaction trick, but I wonder what could be a good value. We could :

• call getmempoolinfo, get the mempoolminfee.
• call prioritisetransaction with fee_delta being strlen(tx_hex) * mempoolminfee (note that mempoolminfee is actually a feerate and that this effectively adds two times the minimum fee, as 1 byte = 2 hex chars).
• call sendrawtransaction, finally.

That's at the expense of two new RPC call each time we send a transaction (which we do pretty loosely, it really isnt necessary to prioritise all tx we pass to bcli as a large amount of them will fail).

[whitslack]

@darosior: Remember, not all bytes in a SegWit transaction count as a full virtual byte. If you add strlen(tx_hex) * mempoolminfee, you'll actually be adding strictly more than two times the minimum fee. It would be better to use the actual vsize of the transaction, which is the size of the non-witness data plus one quarter the size of the witness data. But realistically, all you need is a lower bound on an acceptable value, and your simple proposal achieves that just fine.

[darosior]

Remember, not all bytes in a SegWit transaction count as a full virtual byte

Yeah obviously :-) This was precised for the less obvious (to me?) absence of the * 2. What I wanted to precise is just not to insert it at the bottom of the mempool.