diff --git a/doc/developers-guide/plugin-development/event-notifications.md b/doc/developers-guide/plugin-development/event-notifications.md index 4ee034a8548d..ef0779bfd2c5 100644 --- a/doc/developers-guide/plugin-development/event-notifications.md +++ b/doc/developers-guide/plugin-development/event-notifications.md @@ -181,6 +181,19 @@ A notification for topic `invoice_creation` is sent every time an invoice is cre } ``` +If the invoice is associated with a BOLT 12 offer, an `offer_id` field will be present: + +```json +{ + "invoice_creation": { + "label": "unique-label-for-invoice", + "preimage": "0000000000000000000000000000000000000000000000000000000000000000", + "msat": 10000, + "offer_id": "b1d1062abc09790a68be83e4c257614a57978e4053a954bfee5909ed71e23e03" + } +} +``` + Before version `23.11` the `msat` field was a string with msat-suffix, e.g: `"10000msat"`. ### `warning` diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 6ddecd4dad78..596c3e9feacb 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -872,7 +872,7 @@ invoice_complete(struct invoice_info *info, json_add_u64(response, "created_index", details->created_index); notify_invoice_creation(info->cmd->ld, info->b11->msat, - &info->payment_preimage, info->label); + &info->payment_preimage, info->label, NULL); if (warning_no_listincoming) json_add_string(response, "warning_listincoming", @@ -1686,7 +1686,7 @@ static struct command_result *json_createinvoice(struct command *cmd, NULL)) return fail_exists(cmd, label); - notify_invoice_creation(cmd->ld, b11->msat, preimage, label); + notify_invoice_creation(cmd->ld, b11->msat, preimage, label, NULL); } else { struct tlv_invoice *inv; struct sha256 offer_id, *local_offer_id; @@ -1783,7 +1783,7 @@ static struct command_result *json_createinvoice(struct command *cmd, local_offer_id)) return fail_exists(cmd, label); - notify_invoice_creation(cmd->ld, &msat, preimage, label); + notify_invoice_creation(cmd->ld, &msat, preimage, label, local_offer_id); } response = json_stream_success(cmd); diff --git a/lightningd/notification.c b/lightningd/notification.c index 755a606b17cd..f7a7ebbd0bfd 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -228,13 +228,16 @@ void notify_invoice_payment(struct lightningd *ld, static void invoice_creation_notification_serialize(struct json_stream *stream, const struct amount_msat *amount, const struct preimage *preimage, - const struct json_escape *label) + const struct json_escape *label, + const struct sha256 *offer_id) { if (amount != NULL) json_add_amount_msat(stream, "msat", *amount); json_add_preimage(stream, "preimage", preimage); json_add_escaped_string(stream, "label", label); + if (offer_id) + json_add_sha256(stream, "offer_id", offer_id); } REGISTER_NOTIFICATION(invoice_creation) @@ -242,12 +245,13 @@ REGISTER_NOTIFICATION(invoice_creation) void notify_invoice_creation(struct lightningd *ld, const struct amount_msat *amount, const struct preimage *preimage, - const struct json_escape *label) + const struct json_escape *label, + const struct sha256 *offer_id) { struct jsonrpc_notification *n = notify_start(ld, "invoice_creation"); if (!n) return; - invoice_creation_notification_serialize(n->stream, amount, preimage, label); + invoice_creation_notification_serialize(n->stream, amount, preimage, label, offer_id); notify_send(ld, n); } diff --git a/lightningd/notification.h b/lightningd/notification.h index 7e4c94111695..37f8d3f68574 100644 --- a/lightningd/notification.h +++ b/lightningd/notification.h @@ -46,7 +46,8 @@ void notify_invoice_payment(struct lightningd *ld, void notify_invoice_creation(struct lightningd *ld, const struct amount_msat *amount, const struct preimage *preimage, - const struct json_escape *label); + const struct json_escape *label, + const struct sha256 *offer_id); void notify_channel_opened(struct lightningd *ld, const struct node_id *node_id, diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index b64337505a73..394dbcbfe261 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -2975,6 +2975,23 @@ void htlcs_notify_new_block(struct lightningd *ld) if (channel->error) continue; + /* Fulfill is in progress: we have the preimage + * and have already told channeld (or will on + * reconnect). Don't force-close; if peer goes + * on-chain, onchaind will claim using the + * preimage. + * See https://github.com/ElementsProject/lightning/issues/8899 */ + if (hin->hstate >= SENT_REMOVE_HTLC) { + log_unusual(channel->log, + "Fulfilled HTLC %"PRIu64 + " %s cltv %u hit deadline," + " but removal already in progress", + hin->key.id, + htlc_state_name(hin->hstate), + hin->cltv_expiry); + continue; + } + channel_fail_permanent(channel, REASON_PROTOCOL, "Fulfilled HTLC %"PRIu64 diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index fc00a7c9c25e..6934417d66d7 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -530,7 +530,8 @@ void notify_disconnect(struct lightningd *ld UNNEEDED, const struct node_id *nod void notify_invoice_creation(struct lightningd *ld UNNEEDED, const struct amount_msat *amount UNNEEDED, const struct preimage *preimage UNNEEDED, - const struct json_escape *label UNNEEDED) + const struct json_escape *label UNNEEDED, + const struct sha256 *offer_id UNNEEDED) { fprintf(stderr, "notify_invoice_creation called!\n"); abort(); } /* Generated stub for notify_invoice_payment */ void notify_invoice_payment(struct lightningd *ld UNNEEDED, diff --git a/tests/test_closing.py b/tests/test_closing.py index 0cc1c2b68a85..29cc156ccb89 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3887,7 +3887,16 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): @pytest.mark.parametrize("anchors", [False, True]) def test_htlc_no_force_close(node_factory, bitcoind, anchors): """l2<->l3 force closes while an HTLC is in flight from l1, but l2 can't timeout because the feerate has spiked. It should do so anyway.""" - opts = [{}, {}, {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC']}] + # l3 disconnects before sending update_fulfill_htlc to l2, and + # dev-no-reconnect prevents automatic reconnection. This leaves + # l3's incoming HTLC in SENT_REMOVE_HTLC: l3 has the preimage but + # can't deliver it. l3 no longer force-closes for this (the + # preimage is safe in the DB and onchaind will claim on-chain), + # so l2 force-closes first for the offered HTLC timeout. + opts = [{'dev-no-reconnect': None}, + {'dev-no-reconnect': None}, + {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], + 'dev-no-reconnect': None}] if anchors is False: for opt in opts: opt['dev-force-features'] = "-23" @@ -3911,17 +3920,20 @@ def test_htlc_no_force_close(node_factory, bitcoind, anchors): htlc_txs = [] - # l3 drops to chain, holding htlc (but we stop it xmitting txs) + # l3 won't force-close (removal in progress), so censor its + # onchaind txs for when l2 goes on-chain and l3 sees the + # commitment. def censoring_sendrawtx(r): htlc_txs.append(r['params'][0]) return {'id': r['id'], 'result': {}} l3.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx) - # l3 gets upset, drops to chain when there are < 4 blocks remaining. - # But tx doesn't get mined... + # l3 should NOT force-close: it has the preimage and the removal is + # already in progress. It just logs a warning. bitcoind.generate_block(8) - l3.daemon.wait_for_log("Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC 0 SENT_REMOVE_.* cltv 119 hit deadline") + l3.daemon.wait_for_log(r'but removal already in progress') + assert not l3.daemon.is_in_log(r'Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC 0 SENT_REMOVE') # l2 closes drops the commitment tx at block 115 (one block after timeout) bitcoind.generate_block(4) @@ -3957,6 +3969,89 @@ def censoring_sendrawtx(r): # FIXME: l2 should complain! +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported') +def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind): + """Test that l2 does not force-close when fulfilled HTLC is in + SENT_REMOVE_HTLC state (preimage known, fulfill queued to channeld + but not yet sent to upstream peer). + + Reproduces https://github.com/ElementsProject/lightning/issues/8899: + CLN force-closed with 'Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline' + without attempting to send update_fulfill_htlc upstream first. + """ + # l1 -> l2 -> l3 topology. + # l2 disconnects from l1 right before sending update_fulfill_htlc, + # so the incoming HTLC on l1-l2 stays in SENT_REMOVE_HTLC. + # l2 cannot reconnect (dev-no-reconnect), simulating the scenario where + # the upstream peer appears connected but isn't processing messages. + # + # Use identical feerates to avoid gratuitous commits to update them. + opts = [{'dev-no-reconnect': None, + 'feerates': (7500, 7500, 7500, 7500)}, + {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], + 'dev-no-reconnect': None, + 'feerates': (7500, 7500, 7500, 7500)}, + {'feerates': (7500, 7500, 7500, 7500)}] + + l1, l2, l3 = node_factory.line_graph(3, opts=opts, wait_for_announce=True) + + amt = 12300000 + inv = l3.rpc.invoice(amt, 'test_fulfilled_deadline', 'desc') + + # Use explicit route with known delays to have predictable cltv_expiry. + # delay=16 for first hop (cltv_delta=6 + cltv_final=10), + # delay=10 for second hop (cltv_final=10). + route = [{'amount_msat': amt + 1 + amt * 10 // 1000000, + 'id': l2.info['id'], + 'delay': 16, + 'channel': first_scid(l1, l2)}, + {'amount_msat': amt, + 'id': l3.info['id'], + 'delay': 10, + 'channel': first_scid(l2, l3)}] + l1.rpc.sendpay(route, inv['payment_hash'], + payment_secret=inv['payment_secret']) + + # l3 fulfills the HTLC, preimage flows back to l2. + # l2 transitions the incoming HTLC (from l1) to SENT_REMOVE_HTLC, + # then tries to send update_fulfill_htlc to l1 but disconnects. + # Wait for channeld on the l1-l2 channel to die after the disconnect. + # Note: "Peer transient failure" appears just before the dev_disconnect + # log in connectd, so we must wait for it first to avoid advancing the + # log search position past it. + l2.daemon.wait_for_log(r'chan#1: Peer transient failure in CHANNELD_NORMAL') + + # Get the incoming HTLC's cltv_expiry from listpeerchannels. + htlc = only_one(only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['htlcs']) + # Under Valgrind, the state may advance to SENT_REMOVE_COMMIT before + # the disconnect fully takes effect. Both states reproduce the bug. + assert htlc['state'] in ('SENT_REMOVE_HTLC', 'SENT_REMOVE_COMMIT'), \ + f"Expected SENT_REMOVE_HTLC or SENT_REMOVE_COMMIT, got {htlc['state']}" + cltv_expiry = htlc['expiry'] + + # Compute the deadline dynamically from the actual HTLC cltv_expiry. + # htlc_in_deadline = cltv_expiry - (cltv_expiry_delta + 1)/2 + # With regtest cltv_expiry_delta=6: deadline = cltv_expiry - 3 + deadline = cltv_expiry - (6 + 1) // 2 + current_height = bitcoind.rpc.getblockcount() + + # Mine up to one block before the deadline — should NOT trigger force-close. + blocks_to_deadline = deadline - current_height + assert blocks_to_deadline > 1, f"Not enough room: deadline={deadline}, height={current_height}" + bitcoind.generate_block(blocks_to_deadline - 1) + sync_blockheight(bitcoind, [l2]) + assert not l2.daemon.is_in_log('hit deadline') + + # Mine one more block to hit the deadline. + # l2 should NOT force-close: it has the preimage and the removal is + # already in progress (SENT_REMOVE_HTLC or SENT_REMOVE_COMMIT). + # It just needs to reconnect to send update_fulfill_htlc upstream. + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [l2]) + l2.daemon.wait_for_log(r'but removal already in progress') + assert not l2.daemon.is_in_log(r'Fulfilled HTLC 0 SENT_REMOVE_(HTLC|COMMIT) cltv .* hit deadline[^,]') + + def test_closing_tx_valid(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True, 'dev-no-reconnect': None}) diff --git a/tests/test_misc.py b/tests/test_misc.py index 5baebbcdce42..34dd9a5aeab7 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -521,13 +521,18 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor): def test_htlc_in_timeout(node_factory, bitcoind, executor): """Test that we drop onchain if the peer doesn't accept fulfilled HTLC""" - # HTLC 1->2, 1 fails after 2 has sent committed the fulfill + # HTLC 1->2, 1 fails after 2 has sent committed the fulfill. + # l2 has the preimage but can't deliver it (l1 disconnected and + # won't reconnect). l2 no longer force-closes for this (removal + # is in progress, preimage safe in DB). Instead l1 force-closes + # when the offered HTLC hits its deadline, and l2 claims on-chain + # using the preimage via onchaind. disconnects = ['-WIRE_REVOKE_AND_ACK*2'] # Feerates identical so we don't get gratuitous commit to update them l1 = node_factory.get_node(disconnect=disconnects, options={'dev-no-reconnect': None}, feerates=(7500, 7500, 7500, 7500)) - l2 = node_factory.get_node() + l2 = node_factory.get_node(options={'dev-no-reconnect': None}) # Give it some sats for anchor spend! l2.fundwallet(25000, mine_block=False) @@ -556,15 +561,23 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor): assert not l2.daemon.is_in_log('hit deadline') bitcoind.generate_block(1) - l2.daemon.wait_for_log('Fulfilled HTLC 0 SENT_REMOVE_COMMIT cltv .* hit deadline') - l2.daemon.wait_for_log('sendrawtx exit 0') - l2.bitcoin.generate_block(1) - l2.daemon.wait_for_log(' to ONCHAIN') + # l2 has the preimage but removal is in progress — it should NOT + # force-close. It logs a warning instead. + l2.daemon.wait_for_log(r'but removal already in progress') + assert not l2.daemon.is_in_log(r'Peer permanent failure in CHANNELD_NORMAL: Fulfilled HTLC') + + # l1's offered HTLC hits deadline (cltv_expiry + 1), which is a + # few blocks after l2's fulfilled deadline. l1 force-closes. + bitcoind.generate_block(4 + shadowlen) + l1.daemon.wait_for_log('Offered HTLC 0 SENT_ADD_ACK_REVOCATION cltv .* hit deadline') + l1.daemon.wait_for_log('sendrawtx exit 0') + l1.bitcoin.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') + l2.daemon.wait_for_log(' to ONCHAIN') - # L2 will collect HTLC (iff no shadow route) + # L2 will collect HTLC using the preimage (from l1's unilateral) _, txid, blocks = l2.wait_for_onchaind_tx('OUR_HTLC_SUCCESS_TX', - 'OUR_UNILATERAL/THEIR_HTLC') + 'THEIR_UNILATERAL/THEIR_HTLC') assert blocks == 0 # If we try to reuse the same output as we used for the anchor spend, then