Skip to content

Commit a764f9c

Browse files
ksedgwicnGoline
ksedgwic
authored and
nGoline
committed
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 a944beb)
1 parent a8a1032 commit a764f9c

4 files changed

Lines changed: 33 additions & 11 deletions

File tree

‎closingd/closingd.c‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,12 @@ static void send_offer(struct per_peer_state *pps,
215215
peer_write(pps, take(msg));
216216
}
217217

218-
static void tell_master_their_offer(const struct bitcoin_signature *their_sig,
218+
/* Returns false if master says we must not agree to this offer. */
219+
static bool tell_master_their_offer(const struct bitcoin_signature *their_sig,
219220
const struct bitcoin_tx *tx,
220221
struct bitcoin_txid *tx_id)
221222
{
223+
bool acceptable;
222224
u8 *msg = towire_closingd_received_signature(NULL, their_sig, tx);
223225
if (!wire_sync_write(REQ_FD, take(msg)))
224226
status_failed(STATUS_FAIL_MASTER_IO,
@@ -227,9 +229,11 @@ static void tell_master_their_offer(const struct bitcoin_signature *their_sig,
227229

228230
/* Wait for master to ack, to make sure it's in db. */
229231
msg = wire_sync_read(NULL, REQ_FD);
230-
if (!fromwire_closingd_received_signature_reply(msg, tx_id))
232+
if (!fromwire_closingd_received_signature_reply(msg, tx_id,
233+
&acceptable))
231234
master_badmsg(WIRE_CLOSINGD_RECEIVED_SIGNATURE_REPLY, msg);
232235
tal_free(msg);
236+
return acceptable;
233237
}
234238

235239
/* Returns fee they offered. */
@@ -384,7 +388,17 @@ receive_offer(struct per_peer_state *pps,
384388
/* Master sorts out what is best offer, we just tell it any above min */
385389
if (amount_sat_greater_eq(received_fee, min_fee_to_accept)) {
386390
status_debug("...offer is reasonable");
387-
tell_master_their_offer(&their_sig, tx, closing_txid);
391+
/* Our own closing_signed for this round has usually gone
392+
* out by now (the opener sends first), so if their fee
393+
* matched ours they hold both signatures and can broadcast
394+
* the close whatever we do here. Refusing only keeps us
395+
* from recording the close as agreed. lightningd checks
396+
* the fee against the same bounds we negotiate within, so
397+
* this is not expected to fire. */
398+
if (!tell_master_their_offer(&their_sig, tx, closing_txid))
399+
peer_failed_warn(pps, channel_id,
400+
"Closing fee %s is outside our fee limits",
401+
fmt_amount_sat(tmpctx, received_fee));
388402
}
389403

390404
return received_fee;

‎closingd/closingd_wire.csv‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ msgdata,closingd_received_signature,tx,bitcoin_tx,
4444

4545
msgtype,closingd_received_signature_reply,2102
4646
msgdata,closingd_received_signature_reply,closing_txid,bitcoin_txid,
47+
# Whether we may agree to this offer at all.
48+
msgdata,closingd_received_signature_reply,acceptable,bool,
4749

4850
# Negotiations complete, we're exiting.
4951
msgtype,closingd_complete,2004

‎lightningd/closing_control.c‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ static void peer_received_closing_signature(struct channel *channel,
334334
struct bitcoin_txid tx_id;
335335
struct lightningd *ld = channel->peer->ld;
336336
u8 *funding_wscript;
337+
bool acceptable;
337338

338339
if (!fromwire_closingd_received_signature(msg, msg, &sig, &tx)) {
339340
channel_internal_error(channel,
@@ -362,17 +363,23 @@ static void peer_received_closing_signature(struct channel *channel,
362363
return;
363364
}
364365

365-
if (closing_fee_is_acceptable(ld, channel, tx)) {
366+
acceptable = closing_fee_is_acceptable(ld, channel, tx);
367+
if (acceptable) {
366368
channel_set_last_tx(channel, tx, &sig);
367369
wallet_channel_save(ld->wallet, channel);
368-
}
369-
370-
371-
// Send back the txid so we can update the billboard on selection.
370+
} else
371+
log_unusual(channel->log,
372+
"Rejecting peer's closing fee offer:"
373+
" closingd must not agree to it");
374+
375+
/* Send back the txid so closingd can update the billboard, and
376+
* whether it may agree to this offer at all. Without the verdict
377+
* a rejected offer would still complete the close, and last_tx,
378+
* still the commitment, would be broadcast as the mutual close. */
372379
bitcoin_txid(channel->last_tx, &tx_id);
373-
/* OK, you can continue now. */
374380
subd_send_msg(channel->owner,
375-
take(towire_closingd_received_signature_reply(channel, &tx_id)));
381+
take(towire_closingd_received_signature_reply(channel, &tx_id,
382+
acceptable)));
376383
}
377384

378385
static void peer_closing_complete(struct channel *channel, const u8 *msg)

‎tests/test_closing.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4523,7 +4523,6 @@ def test_closing_feerange_below_estimates(node_factory, bitcoind):
45234523
assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']]
45244524

45254525

4526-
@pytest.mark.xfail(strict=True, reason="closingd completes on a rejected fee and lightningd broadcasts the commitment")
45274526
def test_closing_rejected_fee_fails_negotiation(node_factory, bitcoind):
45284527
"""A closing fee lightningd rejects ends the negotiation.
45294528

0 commit comments

Comments
 (0)