lightningd: fail htlcs we fulfill if peer unresponsive after deadline.

Closes: #241
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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);
 }

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.