diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 00bf77bbb..81c596801 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -289,9 +289,6 @@ static void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage) if (peer_state_on_chain(hin->key.peer->state)) { msg = towire_onchain_known_preimage(hin, preimage); } else { - /* FIXME: fail the peer if it doesn't tell us that htlc - * fulfill is committed before deadline. - */ msg = towire_channel_fulfill_htlc(hin, hin->key.id, preimage); } subd_send_msg(hin->key.peer->owner, take(msg)); @@ -1437,6 +1434,21 @@ static u32 htlc_out_deadline(const struct htlc_out *hout) return hout->cltv_expiry + 1; } +/* BOLT #2: + * + * For HTLCs we accept and have a preimage: the fulfillment deadline when we + * have to fail the channel and fulfill the HTLC onchain before its + * `cltv_expiry`. This is steps 4-7 above, which means a deadline of `2R+G+S` + * blocks before `cltv_expiry`; 7 blocks is reasonable. + */ +/* We approximate this, by using half the cltv_expiry_delta (3R+2G+2S), + * rounded up. */ +static u32 htlc_in_deadline(const struct lightningd *ld, + const struct htlc_in *hin) +{ + return hin->cltv_expiry - (ld->config.cltv_expiry_delta + 1)/2; +} + void notify_new_block(struct lightningd *ld, u32 height) { bool removed; @@ -1480,4 +1492,49 @@ void notify_new_block(struct lightningd *ld, u32 height) } /* Iteration while removing is safe, but can skip entries! */ } while (removed); + + + /* BOLT #2: + * + * A node MUST estimate a fulfillment deadline for each HTLC it is + * attempting to fulfill. A node ... MUST fail the connection if a + * HTLC it has fulfilled is in either node's current commitment + * transaction past this fulfillment deadline. + */ + do { + struct htlc_in *hin; + struct htlc_in_map_iter ini; + + removed = false; + + for (hin = htlc_in_map_first(&ld->htlcs_in, &ini); + hin; + hin = htlc_in_map_next(&ld->htlcs_in, &ini)) { + /* Not fulfilled? If overdue, that's their problem... */ + if (!hin->preimage) + continue; + + /* Not timed out yet? */ + if (height < htlc_in_deadline(ld, hin)) + continue; + + /* Peer on chain already? */ + if (peer_on_chain(hin->key.peer)) + continue; + + /* Peer already failed, or we hit it? */ + if (hin->key.peer->error) + continue; + + peer_fail_permanent_str(hin->key.peer, + take(tal_fmt(hin, + "Fulfilled HTLC %"PRIu64 + " %s cltv %u hit deadline", + hin->key.id, + htlc_state_name(hin->hstate), + hin->cltv_expiry))); + removed = true; + } + /* Iteration while removing is safe, but can skip entries! */ + } while (removed); } diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 6f11c415b..6b8d39163 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -1635,6 +1635,46 @@ class LightningDTests(BaseLightningDTests): l1.daemon.wait_for_log('-> ONCHAIND_OUR_UNILATERAL') l2.daemon.wait_for_log('-> ONCHAIND_THEIR_UNILATERAL') + @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") + def test_htlc_in_timeout(self): + """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 + disconnects = ['-WIRE_REVOKE_AND_ACK*2'] + l1 = self.node_factory.get_node(disconnect=disconnects, + options=['--no-reconnect']) + l2 = self.node_factory.get_node() + + l1.rpc.connect(l2.info['id'], 'localhost:{}'.format(l2.info['port'])) + chanid = self.fund_channel(l1, l2, 10**6) + + # Wait for route propagation. + bitcoind.rpc.generate(5) + l1.daemon.wait_for_logs(['Received channel_update for channel {}\(0\)' + .format(chanid), + 'Received channel_update for channel {}\(1\)' + .format(chanid)]) + + amt = 200000000 + inv = l2.rpc.invoice(amt, 'test_htlc_in_timeout', 'desc')['bolt11'] + assert l2.rpc.listinvoice('test_htlc_in_timeout')[0]['complete'] == False + + payfuture = self.executor.submit(l1.rpc.pay, inv); + + # l1 will drop to chain, not reconnect. + l1.daemon.wait_for_log('dev_disconnect: -WIRE_REVOKE_AND_ACK') + + # Deadline HTLC expity minus 1/2 cltv-expiry delta (rounded up) (== cltv - 3). ctlv is 5+1. + bitcoind.rpc.generate(2) + assert not l2.daemon.is_in_log('hit deadline') + bitcoind.rpc.generate(1) + + l2.daemon.wait_for_log('Fulfilled HTLC 0 SENT_REMOVE_COMMIT cltv {} hit deadline'.format(bitcoind.rpc.getblockcount()+3)) + l2.daemon.wait_for_log('sendrawtx exit 0') + l2.bitcoin.rpc.generate(1) + l2.daemon.wait_for_log('-> ONCHAIND_OUR_UNILATERAL') + l1.daemon.wait_for_log('-> ONCHAIND_THEIR_UNILATERAL') + @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_disconnect(self): # These should all make us fail, and retry.