onchaind: update bolt #5, and implement failure of timed-out onchain HTLCs.

We re-use the value for reasonable_depth given by the master, and we
tell it when our timeout transactions reach that depth.

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

lightningd/peer_control.c

@@ -1282,6 +1282,26 @@ static int handle_missing_htlc_output(struct peer *peer, const u8 *msg)
 	return 0;
 }
 
+static int handle_onchain_htlc_timeout(struct peer *peer, const u8 *msg)
+{
+	struct htlc_stub htlc;
+
+	if (!fromwire_onchain_htlc_timeout(msg, NULL, &htlc)) {
+		log_broken(peer->log, "Invalid onchain_htlc_timeout");
+		return -1;
+	}
+
+	/* BOLT #5:
+	 *
+	 * If the HTLC output has *timed out* and not been *resolved*, the node
+	 * MUST *resolve* the output and MUST fail the corresponding incoming
+	 * HTLC (if any) once the resolving transaction has reached reasonable
+	 * depth.
+	 */
+	onchain_failed_our_htlc(peer, &htlc, "timed out");
+	return 0;
+}
+
 static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds)
 {
 	enum onchain_wire_type t = fromwire_peektype(msg);
@@ -1302,6 +1322,9 @@ static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds)
 	case WIRE_ONCHAIN_MISSING_HTLC_OUTPUT:
 		return handle_missing_htlc_output(sd->peer, msg);
 
+	case WIRE_ONCHAIN_HTLC_TIMEOUT:
+		return handle_onchain_htlc_timeout(sd->peer, msg);
+
 	/* We send these, not receive them */
 	case WIRE_ONCHAIN_INIT:
 	case WIRE_ONCHAIN_SPENT:

onchaind/onchain.c

@@ -51,8 +51,8 @@ static struct privkey *revocation_privkey;
 /* one value is useful for a few witness scripts */
 static const u8 ONE = 0x1;
 
-/* When to tell master about HTLCs which aren't in commitment tx */
-static u32 htlc_missing_depth;
+/* When to tell master about HTLCs which are missing/timed out */
+static u32 reasonable_depth;
 
 /* The messages to send at that depth. */
 static u8 **missing_htlc_msgs;
@@ -691,7 +691,6 @@ static void output_spent(struct tracked_output ***outs,
 			    || out->resolved->tx_type == OUR_HTLC_TIMEOUT_TX)
 				resolve_htlc_tx(outs, i, tx, &txid,
 						tx_blockheight);
-			/* FIXME: Fail timed out htlc after reasonable depth. */
 			return;
 		}
 
@@ -749,6 +748,36 @@ static void output_spent(struct tracked_output ***outs,
 	unwatch_tx(tx);
 }
 
+static void update_resolution_depth(struct tracked_output *out, u32 depth)
+{
+	status_trace("%s/%s->%s depth %u",
+		     tx_type_name(out->tx_type),
+		     output_type_name(out->output_type),
+		     tx_type_name(out->resolved->tx_type),
+		     depth);
+
+	/* BOLT #5:
+	 *
+	 * If the HTLC output has *timed out* and not been *resolved*,
+	 * the node MUST *resolve* the output and MUST fail the
+	 * corresponding incoming HTLC (if any) once the resolving
+	 * transaction has reached reasonable depth. */
+	if (out->resolved->tx_type == OUR_HTLC_TIMEOUT_TX
+	    || out->resolved->tx_type == OUR_HTLC_TIMEOUT_TO_US) {
+		if (out->resolved->depth < reasonable_depth
+		    && depth >= reasonable_depth) {
+			u8 *msg;
+			status_trace("%s/%s reached reasonable depth %u",
+				     tx_type_name(out->tx_type),
+				     output_type_name(out->output_type),
+				     depth);
+			msg = towire_onchain_htlc_timeout(out, out->htlc);
+			wire_sync_write(REQ_FD, take(msg));
+		}
+	}
+	out->resolved->depth = depth;
+}
+
 static void tx_new_depth(struct tracked_output **outs,
 			 const struct sha256_double *txid, u32 depth)
 {
@@ -756,7 +785,7 @@ static void tx_new_depth(struct tracked_output **outs,
 
 	/* Special handling for commitment tx reaching depth */
 	if (structeq(&outs[0]->resolved->txid, txid)
-	    && depth >= htlc_missing_depth
+	    && depth >= reasonable_depth
 	    && missing_htlc_msgs) {
 		status_trace("Sending %zu missing htlc messages",
 			     tal_count(missing_htlc_msgs));
@@ -770,12 +799,7 @@ static void tx_new_depth(struct tracked_output **outs,
 		/* Is this tx resolving an output? */
 		if (outs[i]->resolved) {
 			if (structeq(&outs[i]->resolved->txid, txid)) {
-				status_trace("%s/%s->%s depth %u",
-					     tx_type_name(outs[i]->tx_type),
-					     output_type_name(outs[i]->output_type),
-					     tx_type_name(outs[i]->resolved->tx_type),
-					     depth);
-				outs[i]->resolved->depth = depth;
+				update_resolution_depth(outs[i], depth);
 			}
 			continue;
 		}
@@ -969,7 +993,9 @@ static void resolve_our_htlc_ourcommit(struct tracked_output *out)
 	 * ...
 	 *
 	 * If the HTLC output has *timed out* and not been *resolved*, the
-	 * node MUST *resolve* the output.  If the transaction is the node's
+	 * node MUST *resolve* the output and MUST fail the corresponding
+	 * incoming HTLC (if any) once the resolving transaction has reached
+	 * reasonable depth.  If the transaction is the node's
 	 * own commitment transaction, it MUST *resolve* the output by
 	 * spending it using the HTLC-timeout transaction, and the
 	 * HTLC-timeout transaction output MUST be *resolved* as described in
@@ -1016,9 +1042,11 @@ static void resolve_our_htlc_theircommit(struct tracked_output *out)
 	 * ...
 	 *
 	 * If the HTLC output has *timed out* and not been *resolved*, the
-	 * node MUST *resolve* the output.  If the transaction is the node's
-	 * own commitment transaction, .... Otherwise it MUST resolve the
-	 * output by spending it to a convenient address.
+	 * node MUST *resolve* the output and MUST fail the corresponding
+	 * incoming HTLC (if any) once the resolving transaction has reached
+	 * reasonable depth.  If the transaction is the node's own commitment
+	 * transaction, .... Otherwise it MUST resolve the output by spending
+	 * it to a convenient address.
 	 */
 	tx = tx_to_us(out, out, 0, out->htlc->cltv_expiry, NULL, 0,
 		      out->wscript,
@@ -1884,7 +1912,7 @@ int main(int argc, char *argv[])
 				   &remote_delayed_payment_basepoint,
 				   tx,
 				   &tx_blockheight,
-				   &htlc_missing_depth,
+				   &reasonable_depth,
 				   &remote_htlc_sigs,
 				   &num_htlcs)) {
 		master_badmsg(WIRE_ONCHAIN_INIT, msg);

onchaind/onchain_wire.csv

@@ -26,7 +26,7 @@ onchain_init,,remote_payment_basepoint,struct pubkey
 onchain_init,,remote_delayed_payment_basepoint,struct pubkey
 onchain_init,,tx,struct bitcoin_tx
 onchain_init,,tx_blockheight,u32
-onchain_init,,htlc_missing_depth,u32
+onchain_init,,reasonable_depth,u32
 onchain_init,,num_htlc_sigs,u16
 onchain_init,,htlc_signature,num_htlc_sigs*secp256k1_ecdsa_signature
 onchain_init,,num_htlcs,u64
@@ -74,3 +74,7 @@ onchain_extracted_preimage,,preimage,struct preimage
 # onchaind->master: this HTLC was missing from commit tx.
 onchain_missing_htlc_output,9
 onchain_missing_htlc_output,,htlc,struct htlc_stub
+
+# onchaind->master: this HTLC has timed out (after reasonable_depth)
+onchain_htlc_timeout,10
+onchain_htlc_timeout,,htlc,struct htlc_stub

tests/test_lightningd.py

@@ -513,6 +513,75 @@ class LightningDTests(BaseLightningDTests):
         # Payment failed, BTW
         assert l2.rpc.listinvoice('onchain_dust_out')[0]['complete'] == False
 
+    def test_onchain_timeout(self):
+        """Onchain handling of outgoing failed htlcs"""
+        # HTLC 1->2, 1 fails just after it's irrevocably committed
+        disconnects = ['+WIRE_REVOKE_AND_ACK', 'permfail']
+        l1 = self.node_factory.get_node(disconnect=disconnects)
+        l2 = self.node_factory.get_node()
+
+        l1.rpc.connect('localhost', l2.info['port'], l2.info['id'])
+        self.fund_channel(l1, l2, 10**6)
+
+        rhash = l2.rpc.invoice(10**8, 'onchain_timeout')['rhash']
+        # We underpay, so it fails.
+        routestep = {
+            'msatoshi' : 10**8 - 1,
+            'id' : l2.info['id'],
+            'delay' : 5,
+            'channel': '1:1:1'
+        }
+
+        q = queue.Queue()
+
+        def try_pay():
+            try:
+                l1.rpc.sendpay(to_json([routestep]), rhash, async=False)
+                q.put(None)
+            except Exception as err:
+                q.put(err)
+
+        t = threading.Thread(target=try_pay)
+        t.daemon = True
+        t.start()
+
+        # l1 will drop to chain.
+        l1.daemon.wait_for_log('permfail')
+        l1.daemon.wait_for_log('sendrawtx exit 0')
+        l1.bitcoin.rpc.generate(1)
+        l1.daemon.wait_for_log('-> ONCHAIND_OUR_UNILATERAL')
+        l2.daemon.wait_for_log('-> ONCHAIND_THEIR_UNILATERAL')
+
+        # Wait for timeout.
+        l1.daemon.wait_for_log('Propose handling OUR_UNILATERAL/DELAYED_OUTPUT_TO_US by OUR_DELAYED_RETURN_TO_WALLET .* in 6 blocks')
+        bitcoind.rpc.generate(6)
+
+        # (l1 will also collect its to-self payment.)
+        l1.daemon.wait_for_log('sendrawtx exit 0')
+        l1.daemon.wait_for_log('sendrawtx exit 0')
+        
+        # We use 3 blocks for "reasonable depth"
+        bitcoind.rpc.generate(3)
+
+        # It should fail.
+        err = q.get(timeout = 5)
+        assert type(err) is ValueError
+        t.join(timeout=1)
+        assert not t.isAlive()
+
+        l1.daemon.wait_for_log('WIRE_PERMANENT_CHANNEL_FAILURE: timed out')
+
+        # 91 later, l2 is done.
+        bitcoind.rpc.generate(91)
+        l2.daemon.wait_for_log('onchaind complete, forgetting peer')
+
+        # Now, 100 blocks and l1 should be done.
+        bitcoind.rpc.generate(6)
+        l1.daemon.wait_for_log('onchaind complete, forgetting peer')
+
+        # Payment failed, BTW
+        assert l2.rpc.listinvoice('onchain_timeout')[0]['complete'] == False
+
     def test_onchain_middleman(self):
         # HTLC 1->2->3, 1->2 goes down after 2 gets preimage from 3.
         disconnects = ['-WIRE_UPDATE_FULFILL_HTLC', 'permfail']