closingd: fail the negotiation when lightningd rejects the peer's closing fee
What the developer wrote
Authored by Ken Sedgwick
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.
(cherry picked from commit a944bebef3a95602b660444828882fc583241401)
What changed, and why it matters
This fix repairs a bug in Core Lightning's channel-closing handshake. Previously, when the node rejected a peer's proposed closing fee, the rejection was not communicated back to the sub-process that negotiates with the peer. That sub-process would still treat the close as agreed and could broadcast the old commitment transaction while the user interface reported a successful mutual close. The patch adds an explicit 'acceptable' flag to the internal reply so the negotiation is aborted and the peer is warned instead of proceeding with a mismatched transaction.
Treat this as a security-relevant bug fix and include it in release notes. Users running mutual-close logic should upgrade. Review other subdaemon wire replies that carry implicit 'continue' semantics to ensure they include explicit verdicts where needed.
Security signals we found
State inconsistency between closingd and lightningd during mutual close
Rejected closing fee could still lead to broadcasting the commitment transaction
User-visible close status ('mutual close') did not match on-chain behavior
Internal wire protocol reply lacked a verdict field, causing a logic bypass
Fix includes regression test removal of xfail marker
Evidence from the diff
closingd’s tell_master_their_offer() previously only received a closing_txid from lightningd in response to closingd_received_signature. It used that txid for the billboard and continued to agree to the peer’s offer, even when lightningd’s closing_control.c had rejected the fee and not updated last_tx. The patch extends closingd_received_signature_reply with a bool acceptable, sets it from closing_fee_is_acceptable() in lightningd, and makes closingd call peer_failed_warn() when acceptable is false. This keeps the channel in CLOSINGD_SIGEXCHANGE so negotiation restarts on reconnect, rather than broadcasting last_tx (still the commitment) as a mutual close. A previously xfail test is now enabled.
Changed components
closingd/closingd.cclosingd/closingd_wire.csvlightningd/closing_control.ctests/test_closing.pyInspect captured patch +33 / −11
### 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;
### 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
### lightningd/closing_control.c
@@ -334,6 +334,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,
@@ -362,17 +363,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)
### tests/test_closing.py
@@ -4523,7 +4523,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.
Why this scored 68/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.