From de28bbd792e029faf5b3deeedcb11ecbd4f6a45f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 26 Jan 2022 12:42:34 +1030 Subject: [PATCH] closingd, lightningd: use bitcoin_tx_2of2_input_witness_weight This fixes lightningd's chronic weight underestimate. Signed-off-by: Rusty Russell Changelog-Fixed: closingd: more accurate weight estimation helps mutual closing near min/max feerates. --- closingd/closingd.c | 17 ++--------------- lightningd/closing_control.c | 4 +++- tests/test_closing.py | 1 - 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/closingd/closingd.c b/closingd/closingd.c index 2d228a462..ffcac22e3 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -579,10 +579,6 @@ static size_t closing_tx_weight_estimate(u8 *scriptpubkey[NUM_SIDES], /* We create a dummy close */ struct bitcoin_tx *tx; struct bitcoin_outpoint dummy_funding; - struct bitcoin_signature dummy_sig; - struct privkey dummy_privkey; - struct pubkey dummy_pubkey; - u8 **witness; memset(&dummy_funding, 0, sizeof(dummy_funding)); tx = create_close_tx(tmpctx, chainparams, @@ -594,17 +590,8 @@ static size_t closing_tx_weight_estimate(u8 *scriptpubkey[NUM_SIDES], out[REMOTE], dust_limit); - /* Create a signature, any signature, so we can weigh fully "signed" - * tx. */ - dummy_sig.sighash_type = SIGHASH_ALL; - memset(&dummy_privkey, 1, sizeof(dummy_privkey)); - sign_hash(&dummy_privkey, &dummy_funding.txid.shad, &dummy_sig.s); - pubkey_from_privkey(&dummy_privkey, &dummy_pubkey); - witness = bitcoin_witness_2of2(NULL, &dummy_sig, &dummy_sig, - &dummy_pubkey, &dummy_pubkey); - bitcoin_tx_input_set_witness(tx, 0, take(witness)); - - return bitcoin_tx_weight(tx); + /* We will have to append the witness */ + return bitcoin_tx_weight(tx) + bitcoin_tx_2of2_input_witness_weight(); } /* Get the minimum and desired fees */ diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 5f0aa53d1..f7d88f2fd 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -202,7 +202,9 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, last_fee = calc_tx_fee(channel->funding_sats, channel->last_tx); /* Weight once we add in sigs. */ - weight = bitcoin_tx_weight(tx) + bitcoin_tx_input_sig_weight() * 2; + assert(!tx->wtx->inputs[0].witness + || tx->wtx->inputs[0].witness->num_items == 0); + weight = bitcoin_tx_weight(tx) + bitcoin_tx_2of2_input_witness_weight(); log_debug(channel->log, "Their actual closing tx fee is %s" " vs previous %s: weight is %"PRIu64, diff --git a/tests/test_closing.py b/tests/test_closing.py index 80dc81df0..e362c8856 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3655,7 +3655,6 @@ def test_close_twice(node_factory, executor): assert fut2.result(TIMEOUT)['type'] == 'mutual' -@pytest.mark.xfail(strict=True) def test_close_weight_estimate(node_factory, bitcoind): """closingd uses the expected closing tx weight to constrain fees; make sure that lightningd agrees once it has the actual agreed tx"""