From a8b9f8a78daf1d47b58e3dfb314af046b5d9b415 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 12:19:43 -0700 Subject: [PATCH 1/9] tests: reproduce a mutual close at the fee ceiling falling back to the commitment With every fee estimate at the floor, the opener's closing fee range is a single value. Each output is rounded down to whole satoshis, so the msat remainders end up in the fee and the closing transaction pays one satoshi more than the fee both sides agreed on. lightningd rejects that transaction as above its maximum, keeps the commitment as last_tx, and the close proceeds to broadcast the commitment while reporting a mutual close. test_closing_fee_rounding_at_ceiling pins both nodes at the floor, leaves remainders of 999 and 1 msat, closes from the opener, and checks that the transaction returned by close is a two-output closing transaction paying the agreed fee plus one satoshi, and that both nodes see the output after one block. Marked xfail until the following commits. Changelog-None. --- tests/test_closing.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_closing.py b/tests/test_closing.py index c9d0b4512a0c..356e745ad2a6 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4321,6 +4321,47 @@ def test_closing_minfee(node_factory, bitcoind): bitcoind.generate_block(1, wait_for_mempool=txid) +@pytest.mark.xfail(strict=True, reason="lightningd rejects the rounded fee and broadcasts the commitment") +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') +def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): + """A close pinned at the fee ceiling stays a mutual close. + + With every estimate at the floor, the opener's closing fee range is + a single value. Each output is rounded down to whole satoshis, so + the msat remainders end up in the fee and the transaction pays one + satoshi more than the agreed fee. lightningd must still accept it + rather than fall back to broadcasting the commitment. + """ + l1, l2 = node_factory.line_graph(2, opts={'feerates': (253, 253, 253, 253)}) + chan = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + funding = int(Millisatoshi(chan['total_msat']).to_satoshi()) + + # Leave remainders which sum to exactly 1000msat: l1 keeps ...999msat, + # l2 gets ...001msat. Rounding both down costs one satoshi of fee. + l1.pay(l2, 100000001) + wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == []) + + fee = closing_fee(253, 2) + res = l1.rpc.close(l2.info['id']) + assert res['type'] == 'mutual' + tx = bitcoind.rpc.decoderawtransaction(only_one(res['txs'])) + + # A closing transaction, not the commitment. + assert len(tx['vout']) == 2 + assert tx['locktime'] == 0 + + # The agreed fee plus the rounded-off remainders. + paid = funding - sum(int(round(o['value'] * 10**8)) for o in tx['vout']) + assert paid == fee + 1 + billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'] + assert billboard == ['CLOSINGD_SIGEXCHANGE:We agreed on a closing fee of {} satoshi for tx:{}'.format(fee, tx['txid'])] + + bitcoind.generate_block(1, wait_for_mempool=tx['txid']) + wait_for(lambda: 'ONCHAIN:Tracking mutual close transaction' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status']) + assert tx['txid'] in [o['txid'] for o in l1.rpc.listfunds()['outputs']] + assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): """Test that we use anchor on peer's commit to CPFP tx""" From b88a0c5bf86a0a48e87715f98e959836350574c4 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 14:24:53 -0700 Subject: [PATCH 2/9] lightningd: bound the negotiated closing fee, not the transaction's fee closing_fee_is_acceptable checked the fee the closing transaction pays against the maximum derived from the unilateral feerate. That fee is larger than the fee_satoshis both sides agreed on whenever the outputs' msat remainders were rounded away or an output was trimmed as dust. With every estimate at the floor, closingd's minimum and maximum are the same value, so any rounding put the transaction one satoshi over the maximum and lightningd rejected it. The rejection left the commitment as last_tx and the close broadcast it while reporting a mutual close. Derive the negotiated fee from the transaction: our rounded-down balance minus the output paying our shutdown script. closingd subtracts fee_satoshis from the opener's output, and the maximum only applies when we are the opener, so that difference is exactly the fee we agreed to pay. Bound that instead. The minimum is still checked against the fee the transaction actually pays, which is what relay depends on. If our output was trimmed the whole fee is ours and is bounded as before. Changelog-Fixed: lightningd: a mutual close at the fee ceiling no longer falls back to broadcasting the commitment because of satoshi rounding. Fixes: #9495 --- lightningd/closing_control.c | 49 +++++++++++++++++++++++++++++++++--- tests/test_closing.py | 1 - 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 47806d21ec44..4d0980910066 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -211,12 +211,48 @@ static u32 calc_max_close_feerate(struct lightningd *ld, return max_feerate; } +/* The fee closingd negotiated is what it took off our output (we only + * bound the fee when we are the opener, and the opener pays it). The + * transaction itself pays more than that whenever the outputs' msat + * remainders were rounded away or an output was trimmed as dust: neither + * is a fee we chose, so neither counts against our maximum. */ +static bool negotiated_close_fee(const struct channel *channel, + const struct bitcoin_tx *tx, + struct amount_sat *fee) +{ + struct amount_sat ours = amount_msat_to_sat_round_down(channel->our_msat); + struct amount_sat out_amt; + + for (size_t i = 0; i < tx->wtx->num_outputs; i++) { + const struct wally_tx_output *out = &tx->wtx->outputs[i]; + const u8 *script = tal_dup_arr(tmpctx, u8, + out->script, out->script_len, 0); + if (!scripteq(script, channel->shutdown_scriptpubkey[LOCAL])) + continue; + out_amt = bitcoin_tx_output_get_amount_sat(tx, i); + if (!amount_sat_sub(fee, ours, out_amt)) { + /* closingd built the tx from this same balance, so + * this cannot underflow; count the whole fee if it + * does. */ + log_broken(channel->log, + "Closing tx output %zu pays us %s," + " more than our balance %s", + i, fmt_amount_sat(tmpctx, out_amt), + fmt_amount_sat(tmpctx, ours)); + return false; + } + return true; + } + /* Our output was trimmed: the fee is everything. */ + return false; +} + /* Assess whether a proposed closing fee is acceptable. */ static bool closing_fee_is_acceptable(struct lightningd *ld, struct channel *channel, const struct bitcoin_tx *tx) { - struct amount_sat fee, last_fee; + struct amount_sat fee, last_fee, negotiated; u64 weight; /* Calculate actual fee (adds in eliminated outputs) */ @@ -251,9 +287,14 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, return false; } max_fee = amount_tx_fee(max_feerate, weight); - if (channel->opener == LOCAL && amount_sat_less(max_fee, fee)) { - log_debug(channel->log, "... That's above our max %s" - " for weight %"PRIu64" at feerate %u", + if (!negotiated_close_fee(channel, tx, &negotiated)) + negotiated = fee; + if (channel->opener == LOCAL + && amount_sat_less(max_fee, negotiated)) { + log_debug(channel->log, "... Negotiated fee %s is above" + " our max %s for weight %"PRIu64 + " at feerate %u", + fmt_amount_sat(tmpctx, negotiated), fmt_amount_sat(tmpctx, max_fee), weight, max_feerate); return false; diff --git a/tests/test_closing.py b/tests/test_closing.py index 356e745ad2a6..14233d17a823 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4321,7 +4321,6 @@ def test_closing_minfee(node_factory, bitcoind): bitcoind.generate_block(1, wait_for_mempool=txid) -@pytest.mark.xfail(strict=True, reason="lightningd rejects the rounded fee and broadcasts the commitment") @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): """A close pinned at the fee ceiling stays a mutual close. From 3da571ef073f2888a25ca638f86a25d32f5c9b52 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 15:06:08 -0700 Subject: [PATCH 3/9] tests: reproduce a close with a feerange below the estimate floor falling back to the commitment closingd negotiates within the feerange given to `close`, and lightningd hands it the range minimum as its floor. lightningd's own acceptance check does not use that minimum: it checks the agreed fee against the floor derived from its fee estimates. When the estimates sit above the range, the agreed fee is rejected as too low, the commitment stays as last_tx, and the close broadcasts it while reporting a mutual close. test_closing_feerange_below_estimates opens at the floor, raises the opener's estimates, closes with a feerange pinned at the floor, and checks that the transaction returned by close is a two-output closing transaction at the agreed fee. Marked xfail until the following commit. Changelog-None. --- tests/test_closing.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_closing.py b/tests/test_closing.py index 14233d17a823..2ad7d9805a0d 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4361,6 +4361,42 @@ def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] +@pytest.mark.xfail(strict=True, reason="lightningd rejects the fee below its estimate floor and broadcasts the commitment") +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') +def test_closing_feerange_below_estimates(node_factory, bitcoind): + """A close with a feerange below the estimate floor stays a mutual close. + + closingd negotiates within the feerange given to `close`, but + lightningd checked the agreed fee against the floor derived from its + fee estimates. With estimates above the range, the agreed fee was + rejected as too low and the commitment was broadcast instead. + """ + l1, l2 = node_factory.line_graph(2, opts={'feerates': (253, 253, 253, 253), + 'may_reconnect': True}) + l1.pay(l2, 100000000) + wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == []) + + # l1's estimate floor becomes 1000perkw, well above the range. + l1.force_feerates(2000) + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + + fee = closing_fee(253, 2) + res = l1.rpc.close(l2.info['id'], feerange=['253perkw', '253perkw']) + assert res['type'] == 'mutual' + tx = bitcoind.rpc.decoderawtransaction(only_one(res['txs'])) + + # A closing transaction at the agreed fee, not the commitment. + assert len(tx['vout']) == 2 + assert tx['locktime'] == 0 + billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'] + assert 'CLOSINGD_SIGEXCHANGE:We agreed on a closing fee of {} satoshi for tx:{}'.format(fee, tx['txid']) in billboard + + bitcoind.generate_block(1, wait_for_mempool=tx['txid']) + wait_for(lambda: 'ONCHAIN:Tracking mutual close transaction' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status']) + assert tx['txid'] in [o['txid'] for o in l1.rpc.listfunds()['outputs']] + assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): """Test that we use anchor on peer's commit to CPFP tx""" From 27e0858aac24e88ced7a4465958ec270389049b7 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 15:34:07 -0700 Subject: [PATCH 4/9] lightningd: honour the close feerange minimum when accepting a closing fee peer_start_closingd hands closingd the minimum of the feerange given to `close` as its floor, so closingd negotiates down to it. The acceptance check in closing_fee_is_acceptable kept using the floor derived from the fee estimates, so with estimates above the range the agreed fee was rejected as too low, the commitment stayed as last_tx, and the close broadcast it while reporting a mutual close. Use the range minimum as the floor there too, as calc_max_close_feerate already does for the maximum. Both ends of the check now match the bounds closingd negotiated within. Changelog-Fixed: lightningd: `close` with a feerange below the fee estimates no longer falls back to broadcasting the commitment. --- lightningd/closing_control.c | 4 ++++ tests/test_closing.py | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 4d0980910066..01b111e2c389 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -276,6 +276,10 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, /* If we don't have a feerate estimate, this gives feerate_floor */ min_feerate = feerate_min(ld, NULL); + /* A feerange given to `close` is what closingd negotiated + * within; its minimum is our floor too. */ + if (channel->closing_feerate_range) + min_feerate = channel->closing_feerate_range[0]; max_feerate = calc_max_close_feerate(ld, channel); min_fee = amount_tx_fee(min_feerate, weight); diff --git a/tests/test_closing.py b/tests/test_closing.py index 2ad7d9805a0d..0902b464b98f 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4361,7 +4361,6 @@ def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] -@pytest.mark.xfail(strict=True, reason="lightningd rejects the fee below its estimate floor and broadcasts the commitment") @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_closing_feerange_below_estimates(node_factory, bitcoind): """A close with a feerange below the estimate floor stays a mutual close. From c2d6f07d7811f3d82e1dc9b2d88bf8324c6b0e3b Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 15:44:32 -0700 Subject: [PATCH 5/9] tests: reproduce closingd completing a close on a fee lightningd rejected When lightningd rejects a closing fee, closingd never learns it: the reply only carries a txid, which closingd uses for its billboard. It agrees to the offer, reports the close complete, and lightningd broadcasts last_tx, still the commitment, as if it were the mutual close. The previous commits removed the known reasons for a rejection, so add --dev-reject-closing-fee, which makes lightningd reject every closing fee the peer offers. test_closing_rejected_fee_fails_negotiation runs it on the non-opener: the opener's close must end in a unilateral close at its timeout, and the rejecting node must never agree to a fee or broadcast anything. Marked xfail until the following commit. Changelog-None. --- lightningd/closing_control.c | 5 +++++ lightningd/lightningd.c | 1 + lightningd/lightningd.h | 3 +++ lightningd/options.c | 4 ++++ tests/test_closing.py | 25 +++++++++++++++++++++++++ 5 files changed, 38 insertions(+) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 01b111e2c389..a67adfafc31e 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -270,6 +270,11 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, fmt_amount_sat(tmpctx, last_fee), weight); + if (ld->dev_reject_closing_fee) { + log_debug(channel->log, "... dev-reject-closing-fee"); + return false; + } + if (!channel->ignore_fee_limits && !ld->config.ignore_fee_limits) { struct amount_sat min_fee, max_fee; u32 min_feerate, max_feerate; diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 44ed3fa66425..6dbc2a0473b1 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -131,6 +131,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->dev_throttle_gossip = false; ld->dev_suppress_gossip = false; ld->dev_fast_reconnect = false; + ld->dev_reject_closing_fee = false; ld->dev_force_privkey = NULL; ld->dev_force_bip32_seed = NULL; ld->dev_force_channel_secrets = NULL; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 6d778c929e95..87a2851ceff4 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -322,6 +322,9 @@ struct lightningd { /* Speedup reconnect delay, for testing. */ bool dev_fast_reconnect; + /* Reject every closing fee the peer offers. */ + bool dev_reject_closing_fee; + /* This is the forced private key for the node. */ struct privkey *dev_force_privkey; diff --git a/lightningd/options.c b/lightningd/options.c index 42ee3a6e43f3..6e3979b46850 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -801,6 +801,10 @@ static void dev_register_opts(struct lightningd *ld) opt_set_bool, &ld->dev_fast_reconnect, "Make max default reconnect delay 3 (not 300) seconds"); + clnopt_noarg("--dev-reject-closing-fee", OPT_DEV, + opt_set_bool, + &ld->dev_reject_closing_fee, + "Reject every closing fee the peer offers, as if outside our limits"); clnopt_noarg("--dev-fail-on-subdaemon-fail", OPT_DEV, opt_set_bool, diff --git a/tests/test_closing.py b/tests/test_closing.py index 0902b464b98f..cf9782eba631 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4396,6 +4396,31 @@ def test_closing_feerange_below_estimates(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] +@pytest.mark.xfail(strict=True, reason="closingd completes on a rejected fee and lightningd broadcasts the commitment") +def test_closing_rejected_fee_fails_negotiation(node_factory, bitcoind): + """A closing fee lightningd rejects ends the negotiation. + + closingd only learns the txid from lightningd's reply, never the + verdict, so it agrees to a rejected offer and lightningd broadcasts + the commitment as if it were the mutual close. The known reasons for + a rejection are fixed, so --dev-reject-closing-fee forces one. + """ + l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-reject-closing-fee': None}]) + l1.pay(l2, 100000000) + wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == []) + + # l2 refuses l1's offer, so nobody completes the negotiation and l1 + # closes unilaterally when its timeout expires. + res = l1.rpc.close(l2.info['id'], unilateraltimeout=10) + assert res['type'] == 'unilateral' + assert not l2.daemon.is_in_log('We agreed on a closing fee') + l2.daemon.wait_for_log('outside our fee limits') + + # The only transaction on the wire is that unilateral close. + txid = only_one(res['txids']) + wait_for(lambda: bitcoind.rpc.getrawmempool() == [txid]) + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): """Test that we use anchor on peer's commit to CPFP tx""" From c7a325efbe07d592d188afba2429dd82422553af Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Mon, 14 Sep 2026 15:51:42 -0700 Subject: [PATCH 6/9] closingd: fail the negotiation when lightningd rejects the peer's closing fee lightningd's reply to closingd_received_signature carried only a txid. closingd used it for the billboard and went on to agree to the offer, so when lightningd had rejected the fee the close still completed and drop_to_chain broadcast last_tx, which was still the commitment, while the close command reported a mutual close. Add the verdict to the reply. On a rejection lightningd logs it at UNUSUAL, and closingd sends the peer a warning and exits instead of agreeing, the same way it handles a fee range with no overlap. The channel stays in CLOSINGD_SIGEXCHANGE: negotiation restarts on reconnect with lightningd's current bounds, and the close command's timeout decides when to close unilaterally. Changelog-Fixed: lightningd: a closing fee lightningd rejects fails the negotiation instead of broadcasting the commitment as a mutual close. --- closingd/closingd.c | 20 +++++++++++++++++--- closingd/closingd_wire.csv | 2 ++ lightningd/closing_control.c | 21 ++++++++++++++------- tests/test_closing.py | 1 - 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/closingd/closingd.c b/closingd/closingd.c index 3648a914cede..bd4e575b90ed 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -215,10 +215,12 @@ static void send_offer(struct per_peer_state *pps, peer_write(pps, take(msg)); } -static void tell_master_their_offer(const struct bitcoin_signature *their_sig, +/* Returns false if master says we must not agree to this offer. */ +static bool tell_master_their_offer(const struct bitcoin_signature *their_sig, const struct bitcoin_tx *tx, struct bitcoin_txid *tx_id) { + bool acceptable; u8 *msg = towire_closingd_received_signature(NULL, their_sig, tx); if (!wire_sync_write(REQ_FD, take(msg))) status_failed(STATUS_FAIL_MASTER_IO, @@ -227,9 +229,11 @@ static void tell_master_their_offer(const struct bitcoin_signature *their_sig, /* Wait for master to ack, to make sure it's in db. */ msg = wire_sync_read(NULL, REQ_FD); - if (!fromwire_closingd_received_signature_reply(msg, tx_id)) + if (!fromwire_closingd_received_signature_reply(msg, tx_id, + &acceptable)) master_badmsg(WIRE_CLOSINGD_RECEIVED_SIGNATURE_REPLY, msg); tal_free(msg); + return acceptable; } /* Returns fee they offered. */ @@ -384,7 +388,17 @@ receive_offer(struct per_peer_state *pps, /* Master sorts out what is best offer, we just tell it any above min */ if (amount_sat_greater_eq(received_fee, min_fee_to_accept)) { status_debug("...offer is reasonable"); - tell_master_their_offer(&their_sig, tx, closing_txid); + /* Our own closing_signed for this round has usually gone + * out by now (the opener sends first), so if their fee + * matched ours they hold both signatures and can broadcast + * the close whatever we do here. Refusing only keeps us + * from recording the close as agreed. lightningd checks + * the fee against the same bounds we negotiate within, so + * this is not expected to fire. */ + if (!tell_master_their_offer(&their_sig, tx, closing_txid)) + peer_failed_warn(pps, channel_id, + "Closing fee %s is outside our fee limits", + fmt_amount_sat(tmpctx, received_fee)); } return received_fee; diff --git a/closingd/closingd_wire.csv b/closingd/closingd_wire.csv index 55ca558979d0..a0f3ddeee7e0 100644 --- a/closingd/closingd_wire.csv +++ b/closingd/closingd_wire.csv @@ -44,6 +44,8 @@ msgdata,closingd_received_signature,tx,bitcoin_tx, msgtype,closingd_received_signature_reply,2102 msgdata,closingd_received_signature_reply,closing_txid,bitcoin_txid, +# Whether we may agree to this offer at all. +msgdata,closingd_received_signature_reply,acceptable,bool, # Negotiations complete, we're exiting. msgtype,closingd_complete,2004 diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index a67adfafc31e..d291049ddae6 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -324,6 +324,7 @@ static void peer_received_closing_signature(struct channel *channel, struct bitcoin_txid tx_id; struct lightningd *ld = channel->peer->ld; u8 *funding_wscript; + bool acceptable; if (!fromwire_closingd_received_signature(msg, msg, &sig, &tx)) { channel_internal_error(channel, @@ -352,17 +353,23 @@ static void peer_received_closing_signature(struct channel *channel, return; } - if (closing_fee_is_acceptable(ld, channel, tx)) { + acceptable = closing_fee_is_acceptable(ld, channel, tx); + if (acceptable) { channel_set_last_tx(channel, tx, &sig); wallet_channel_save(ld->wallet, channel); - } - - - // Send back the txid so we can update the billboard on selection. + } else + log_unusual(channel->log, + "Rejecting peer's closing fee offer:" + " closingd must not agree to it"); + + /* Send back the txid so closingd can update the billboard, and + * whether it may agree to this offer at all. Without the verdict + * a rejected offer would still complete the close, and last_tx, + * still the commitment, would be broadcast as the mutual close. */ bitcoin_txid(channel->last_tx, &tx_id); - /* OK, you can continue now. */ subd_send_msg(channel->owner, - take(towire_closingd_received_signature_reply(channel, &tx_id))); + take(towire_closingd_received_signature_reply(channel, &tx_id, + acceptable))); } static void peer_closing_complete(struct channel *channel, const u8 *msg) diff --git a/tests/test_closing.py b/tests/test_closing.py index cf9782eba631..e089a6739c51 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4396,7 +4396,6 @@ def test_closing_feerange_below_estimates(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] -@pytest.mark.xfail(strict=True, reason="closingd completes on a rejected fee and lightningd broadcasts the commitment") def test_closing_rejected_fee_fails_negotiation(node_factory, bitcoind): """A closing fee lightningd rejects ends the negotiation. From fa8b9dd29828c81e3fdb7d7ce487597e4b2763f5 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 17 Sep 2026 07:57:59 -0700 Subject: [PATCH 7/9] fixup! tests: reproduce a mutual close at the fee ceiling falling back to the commitment --- tests/test_closing.py | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/test_closing.py b/tests/test_closing.py index e089a6739c51..c319edc089b8 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4361,6 +4361,58 @@ def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] +@pytest.mark.xfail(strict=True, reason="lightningd bounds the trimmed close's whole fee at the one-output weight and closes unilaterally") +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') +def test_closing_fee_trims_opener_output(node_factory, bitcoind): + """A close whose fee leaves the opener's output below dust stays mutual. + + closingd bounds the fee it agrees to at the weight of a closing + transaction with both outputs, and drops the opener's output once the + fee leaves it below the dust limit. lightningd then sees a one-output + transaction paying the opener's whole balance as fee. It must bound + the fee closingd agreed to, at the weight closingd used, rather than + reject the close and fall back to the commitment. + """ + l1, l2 = node_factory.line_graph(2, opts={'feerates': (253, 253, 253, 253)}) + chan = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + funding = int(Millisatoshi(chan['total_msat']).to_satoshi()) + dust = int(Millisatoshi(chan['dust_limit_msat']).to_satoshi()) + + # Drain the opener to what it has to keep, in whole satoshis so no + # msat remainder reaches the fee. + spendable = int(chan['spendable_msat']) + l1.pay(l2, spendable - spendable % 1000) + wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == []) + chan = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels']) + ours = int(Millisatoshi(chan['to_us_msat']).to_satoshi()) + + # A feerate whose two-output closing fee leaves l1 about half the + # dust limit, so its output is trimmed. + feerate = (ours - dust // 2) * 1000 // closing_fee(1000, 2) + fee = closing_fee(feerate, 2) + assert ours - dust < fee <= ours + + res = l1.rpc.close(l2.info['id'], unilateraltimeout=10, + feerange=['{}perkw'.format(feerate)] * 2) + assert res['type'] == 'mutual' + tx = bitcoind.rpc.decoderawtransaction(only_one(res['txs'])) + + # A closing transaction with only l2's output, not the commitment. + assert len(tx['vout']) == 1 + assert tx['locktime'] == 0 + + # l1's whole balance is fee: the agreed fee plus the trimmed rest. + paid = funding - int(round(only_one(tx['vout'])['value'] * 10**8)) + assert paid == ours + billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'] + assert billboard == ['CLOSINGD_SIGEXCHANGE:We agreed on a closing fee of {} satoshi for tx:{}'.format(fee, tx['txid'])] + + bitcoind.generate_block(1, wait_for_mempool=tx['txid']) + wait_for(lambda: 'ONCHAIN:Tracking mutual close transaction' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status']) + assert tx['txid'] not in [o['txid'] for o in l1.rpc.listfunds()['outputs']] + assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_closing_feerange_below_estimates(node_factory, bitcoind): """A close with a feerange below the estimate floor stays a mutual close. From 170c9f641e6431276e081c23d5eaa4f039b527b1 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 17 Sep 2026 07:59:17 -0700 Subject: [PATCH 8/9] fixup! lightningd: bound the negotiated closing fee, not the transaction's fee --- lightningd/closing_control.c | 39 +++++++++++++++++++++++++----------- tests/test_closing.py | 1 - 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index d291049ddae6..e7acf0179383 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -212,22 +212,28 @@ static u32 calc_max_close_feerate(struct lightningd *ld, } /* The fee closingd negotiated is what it took off our output (we only - * bound the fee when we are the opener, and the opener pays it). The - * transaction itself pays more than that whenever the outputs' msat - * remainders were rounded away or an output was trimmed as dust: neither - * is a fee we chose, so neither counts against our maximum. */ + * bound the fee when we are the opener, and the opener pays it), and + * closingd bounded it at the weight of the closing transaction before + * any fee came off, with our output still present. The transaction + * itself pays more than that whenever the outputs' msat remainders were + * rounded away or the fee trimmed our output as dust: neither is a fee + * we chose, so neither counts against our maximum. *weight is the + * transaction's weight on entry and the weight closingd bounded the fee + * at on return. */ static bool negotiated_close_fee(const struct channel *channel, const struct bitcoin_tx *tx, - struct amount_sat *fee) + struct amount_sat *fee, + u64 *weight) { struct amount_sat ours = amount_msat_to_sat_round_down(channel->our_msat); + const u8 *our_script = channel->shutdown_scriptpubkey[LOCAL]; struct amount_sat out_amt; for (size_t i = 0; i < tx->wtx->num_outputs; i++) { const struct wally_tx_output *out = &tx->wtx->outputs[i]; const u8 *script = tal_dup_arr(tmpctx, u8, out->script, out->script_len, 0); - if (!scripteq(script, channel->shutdown_scriptpubkey[LOCAL])) + if (!scripteq(script, our_script)) continue; out_amt = bitcoin_tx_output_get_amount_sat(tx, i); if (!amount_sat_sub(fee, ours, out_amt)) { @@ -243,8 +249,15 @@ static bool negotiated_close_fee(const struct channel *channel, } return true; } - /* Our output was trimmed: the fee is everything. */ - return false; + + /* Our output was trimmed. closingd drops it once the fee leaves + * it below the dust limit, so the fee is at least our balance less + * that, and the weight closingd bounded it at included the + * output. */ + if (!amount_sat_sub(fee, ours, channel->our_config.dust_limit)) + *fee = AMOUNT_SAT(0); + *weight += bitcoin_tx_output_weight(tal_bytelen(our_script)); + return true; } /* Assess whether a proposed closing fee is acceptable. */ @@ -253,7 +266,7 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, const struct bitcoin_tx *tx) { struct amount_sat fee, last_fee, negotiated; - u64 weight; + u64 weight, negotiated_weight; /* Calculate actual fee (adds in eliminated outputs) */ fee = calc_tx_fee(channel->funding_sats, tx); @@ -295,9 +308,11 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, weight, min_feerate); return false; } - max_fee = amount_tx_fee(max_feerate, weight); - if (!negotiated_close_fee(channel, tx, &negotiated)) + negotiated_weight = weight; + if (!negotiated_close_fee(channel, tx, &negotiated, + &negotiated_weight)) negotiated = fee; + max_fee = amount_tx_fee(max_feerate, negotiated_weight); if (channel->opener == LOCAL && amount_sat_less(max_fee, negotiated)) { log_debug(channel->log, "... Negotiated fee %s is above" @@ -305,7 +320,7 @@ static bool closing_fee_is_acceptable(struct lightningd *ld, " at feerate %u", fmt_amount_sat(tmpctx, negotiated), fmt_amount_sat(tmpctx, max_fee), - weight, max_feerate); + negotiated_weight, max_feerate); return false; } } diff --git a/tests/test_closing.py b/tests/test_closing.py index c319edc089b8..fed3511faa7f 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4361,7 +4361,6 @@ def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind): assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']] -@pytest.mark.xfail(strict=True, reason="lightningd bounds the trimmed close's whole fee at the one-output weight and closes unilaterally") @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd') def test_closing_fee_trims_opener_output(node_factory, bitcoind): """A close whose fee leaves the opener's output below dust stays mutual. From 1375c8452967fa83c98eb0a37ee1f9b8cdc20d66 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 17 Sep 2026 08:02:32 -0700 Subject: [PATCH 9/9] fixup! tests: reproduce closingd completing a close on a fee lightningd rejected --- tests/test_closing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index fed3511faa7f..9e2cc41176cf 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -4466,7 +4466,8 @@ def test_closing_rejected_fee_fails_negotiation(node_factory, bitcoind): assert not l2.daemon.is_in_log('We agreed on a closing fee') l2.daemon.wait_for_log('outside our fee limits') - # The only transaction on the wire is that unilateral close. + # The only transaction on the wire is that unilateral close: with no + # HTLCs at stake, nothing spends its anchor to hurry it along. txid = only_one(res['txids']) wait_for(lambda: bitcoind.rpc.getrawmempool() == [txid])