htlc: save fail message in HTLC.

It's not currently encrypted, but at least you get some idea now why
an HTLC failed.  We (ab)use HTTP error codes for the moment.

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

daemon/Makefile

@@ -22,6 +22,7 @@ DAEMON_SRC :=					\
 	daemon/cryptopkt.c			\
 	daemon/db.c				\
 	daemon/dns.c				\
+	daemon/failure.c			\
 	daemon/feechange.c			\
 	daemon/htlc.c				\
 	daemon/jsonrpc.c			\
@@ -64,6 +65,7 @@ DAEMON_HEADERS :=				\
 	daemon/cryptopkt.h			\
 	daemon/db.h				\
 	daemon/dns.h				\
+	daemon/failure.h			\
 	daemon/feechange.h			\
 	daemon/feechange_state.h		\
 	daemon/htlc.h				\

daemon/db.c

@@ -503,10 +503,10 @@ static void load_peer_htlcs(struct peer *peer)
 			fatal("load_peer_htlcs:step gave %s:%s",
 			      sqlite3_errstr(err), sqlite3_errmsg(sql));
 
-		if (sqlite3_column_count(stmt) != 10)
-			fatal("load_peer_htlcs:step gave %i cols, not 10",
+		if (sqlite3_column_count(stmt) != 11)
+			fatal("load_peer_htlcs:step gave %i cols, not 11",
 			      sqlite3_column_count(stmt));
-		/* CREATE TABLE htlcs (peer "SQL_PUBKEY", id INT, state TEXT, msatoshis INT, expiry INT, rhash "SQL_RHASH", r "SQL_R", routing "SQL_ROUTING", src_peer "SQL_PUBKEY", src_id INT, PRIMARY KEY(peer, id, state)); */
+		/* CREATE TABLE htlcs (peer "SQL_PUBKEY", id INT, state TEXT, msatoshis INT, expiry INT, rhash "SQL_RHASH", r "SQL_R", routing "SQL_ROUTING", src_peer "SQL_PUBKEY", src_id INT, fail BLOB, PRIMARY KEY(peer, id, state)); */
 		sha256_from_sql(stmt, 5, &rhash);
 
 		hstate = htlc_state_from_name(sqlite3_column_str(stmt, 2));
@@ -527,6 +527,14 @@ static void load_peer_htlcs(struct peer *peer)
 			htlc->r = tal(htlc, struct rval);
 			from_sql_blob(stmt, 6, htlc->r, sizeof(*htlc->r));
 		}
+		if (sqlite3_column_type(stmt, 10) != SQLITE_NULL) {
+			htlc->fail = tal_sql_blob(htlc, stmt, 10);
+		}
+
+		if (htlc->r && htlc->fail)
+			fatal("%s HTLC %"PRIu64" has failed and fulfilled?",
+			      htlc_owner(htlc) == LOCAL ? "local" : "remote",
+			      htlc->id);
 
 		log_debug(peer->log, "Loaded %s HTLC %"PRIu64" (%s)",
 			  htlc_owner(htlc) == LOCAL ? "local" : "remote",
@@ -1023,7 +1031,7 @@ void db_init(struct lightningd_state *dstate)
 			 "CREATE TABLE wallet (privkey "SQL_PRIVKEY");"
 			 "CREATE TABLE anchors (peer "SQL_PUBKEY", txid "SQL_TXID", idx INT, amount INT, ok_depth INT, min_depth INT, bool ours, PRIMARY KEY(peer));"
 			 /* FIXME: state in primary key is overkill: just need side */
-			 "CREATE TABLE htlcs (peer "SQL_PUBKEY", id INT, state TEXT, msatoshis INT, expiry INT, rhash "SQL_RHASH", r "SQL_R", routing "SQL_ROUTING", src_peer "SQL_PUBKEY", src_id INT, PRIMARY KEY(peer, id, state));"
+			 "CREATE TABLE htlcs (peer "SQL_PUBKEY", id INT, state TEXT, msatoshis INT, expiry INT, rhash "SQL_RHASH", r "SQL_R", routing "SQL_ROUTING", src_peer "SQL_PUBKEY", src_id INT, fail BLOB, PRIMARY KEY(peer, id, state));"
 			 "CREATE TABLE feechanges (peer "SQL_PUBKEY", state TEXT, fee_rate INT, PRIMARY KEY(peer,state));"
 			 "CREATE TABLE commit_info (peer "SQL_PUBKEY", side TEXT, commit_num INT, revocation_hash "SQL_SHA256", xmit_order INT, sig "SQL_SIGNATURE", prev_revocation_hash "SQL_SHA256", PRIMARY KEY(peer, side));"
 			 "CREATE TABLE shachain (peer "SQL_PUBKEY", shachain BINARY(%zu), PRIMARY KEY(peer));"
@@ -1253,7 +1261,7 @@ bool db_new_htlc(struct peer *peer, const struct htlc *htlc)
 	if (htlc->src) {
 		errmsg = db_exec(ctx, peer->dstate, 
 				 "INSERT INTO htlcs VALUES"
-				 " (x'%s', %"PRIu64", '%s', %"PRIu64", %u, x'%s', NULL, x'%s', x'%s', %"PRIu64");",
+				 " (x'%s', %"PRIu64", '%s', %"PRIu64", %u, x'%s', NULL, x'%s', x'%s', %"PRIu64", NULL);",
 				 pubkey_to_hexstr(ctx, peer->dstate->secpctx, peer->id),
 				 htlc->id,
 				 htlc_state_name(htlc->state),
@@ -1266,7 +1274,7 @@ bool db_new_htlc(struct peer *peer, const struct htlc *htlc)
 	} else {
 		errmsg = db_exec(ctx, peer->dstate, 
 				 "INSERT INTO htlcs VALUES"
-				 " (x'%s', %"PRIu64", '%s', %"PRIu64", %u, x'%s', NULL, x'%s', NULL, NULL);",
+				 " (x'%s', %"PRIu64", '%s', %"PRIu64", %u, x'%s', NULL, x'%s', NULL, NULL, NULL);",
 				 peerid,
 				 htlc->id,
 				 htlc_state_name(htlc->state),
@@ -1388,6 +1396,28 @@ bool db_htlc_fulfilled(struct peer *peer, const struct htlc *htlc)
 	return !errmsg;
 }
 
+bool db_htlc_failed(struct peer *peer, const struct htlc *htlc)
+{
+	const char *errmsg, *ctx = tal(peer, char);
+	const char *peerid = pubkey_to_hexstr(ctx, peer->dstate->secpctx, peer->id);
+
+	log_debug(peer->log, "%s(%s)", __func__, peerid);
+
+	/* When called from their_htlc_added() we're routing a failure,
+	 * we are in a transaction.  Otherwise, not. */
+	errmsg = db_exec(ctx, peer->dstate, 
+			 "UPDATE htlcs SET fail=x'%s' WHERE peer=x'%s' AND id=%"PRIu64" AND state='%s';",
+			 tal_hexstr(ctx, htlc->fail, sizeof(*htlc->fail)),
+			 peerid,
+			 htlc->id,
+			 htlc_state_name(htlc->state));
+
+	if (errmsg)
+		log_broken(peer->log, "%s:%s", __func__, errmsg);
+	tal_free(ctx);
+	return !errmsg;
+}
+
 bool db_new_commit_info(struct peer *peer, enum channel_side side,
 			const struct sha256 *prev_rhash)
 {

daemon/db.h

@@ -22,6 +22,7 @@ bool db_add_peer_address(struct lightningd_state *dstate,
 
 /* Must NOT be inside transaction. */
 bool db_htlc_fulfilled(struct peer *peer, const struct htlc *htlc);
+bool db_htlc_failed(struct peer *peer, const struct htlc *htlc);
 bool db_set_our_closing_script(struct peer *peer);
 bool db_set_their_closing_script(struct peer *peer);
 bool db_update_our_closing(struct peer *peer);

daemon/failure.c

@@ -0,0 +1,41 @@
+#include "failure.h"
+#include "protobuf_convert.h"
+#include <ccan/tal/str/str.h>
+
+/* FIXME: Crypto! */
+const u8 *failinfo_create(const tal_t *ctx,
+			  secp256k1_context *secpctx,
+			  const struct pubkey *id,
+			  u32 error_code,
+			  const char *reason)
+{
+	FailInfo *f = tal(ctx, FailInfo);
+	u8 *arr;
+
+	fail_info__init(f);
+	f->id = pubkey_to_proto(f, secpctx, id);
+	f->error_code = error_code;
+	if (reason)
+		f->reason = tal_strdup(f, reason);
+	else
+		f->reason = NULL;
+
+	arr = tal_arr(ctx, u8, fail_info__get_packed_size(f));
+	fail_info__pack(f, arr);
+	tal_free(f);
+	return arr;
+}
+
+FailInfo *failinfo_unwrap(const tal_t *ctx, const void *data, size_t len)
+{
+	struct ProtobufCAllocator *prototal = make_prototal(ctx);
+	FailInfo *f;
+
+	f = fail_info__unpack(prototal, len, data);
+	if (f)
+		steal_from_prototal(ctx, prototal, f);
+	else
+		tal_free(prototal);
+
+	return f;
+}

daemon/failure.h

@@ -0,0 +1,37 @@
+#ifndef LIGHTNING_DAEMON_FAILURE_H
+#define LIGHTNING_DAEMON_FAILURE_H
+#include "config.h"
+#include "lightning.pb-c.h"
+#include <ccan/short_types/short_types.h>
+#include <ccan/tal/tal.h>
+#include <secp256k1.h>
+
+struct pubkey;
+
+enum fail_error {
+	BAD_REQUEST_400 = 400,
+	UNAUTHORIZED_401 = 401,
+	PAYMENT_REQUIRED_402 = 402,
+	FORBIDDEN_403 = 403,
+	NOT_FOUND_404 = 404,
+	METHOD_NOT_ALLOWED_405 = 405,
+	REQUEST_TIMEOUT_408 = 408,
+	GONE_410 = 410,
+	IM_A_TEAPOT_418 = 418,
+	INTERNAL_SERVER_ERROR_500 = 500,
+	NOT_IMPLEMENTED_501 = 501,
+	BAD_GATEWAY_502 = 502,
+	SERVICE_UNAVAILABLE_503 = 503,
+	GATEWAY_TIMEOUT_504 = 504,
+	VERSION_NOT_SUPPORTED_505 = 505
+};
+
+const u8 *failinfo_create(const tal_t *ctx,
+			  secp256k1_context *secpctx,
+			  const struct pubkey *id,
+			  enum fail_error error_code,
+			  const char *reason);
+
+FailInfo *failinfo_unwrap(const tal_t *ctx, const void *data, size_t len);
+
+#endif /* LIGHTNING_DAEMON_FAILURE_H */

daemon/htlc.h

@@ -69,6 +69,7 @@ struct htlc {
 	const u8 *routing;
 	/* Previous HTLC (if any) which made us offer this (OURS only) */
 	struct htlc *src;
+	const u8 *fail;
 };
 
 const char *htlc_state_name(enum htlc_state s);

daemon/packets.c

@@ -171,9 +171,11 @@ void queue_pkt_htlc_fail(struct peer *peer, struct htlc *htlc)
 	update_fail_htlc__init(f);
 	f->id = htlc->id;
 
-	/* FIXME: reason! */
 	f->reason = tal(f, FailReason);
 	fail_reason__init(f->reason);
+	f->reason->info.len = tal_count(htlc->fail);
+	f->reason->info.data = tal_dup_arr(f->reason, u8,
+					   htlc->fail, f->reason->info.len, 0);
 
 	queue_pkt(peer, PKT__PKT_UPDATE_FAIL_HTLC, f);
 }
@@ -451,7 +453,16 @@ Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h)
 	if (err)
 		return err;
 
-	/* FIXME: Save reason. */
+	if ((*h)->r)
+		return pkt_err(peer, "HTLC %"PRIu64" already fulfilled",
+			       (*h)->id);
+
+	/* This can happen with re-transmissions; simply note it. */
+	if ((*h)->fail) {
+		log_debug(peer->log, "HTLC %"PRIu64" failed twice", (*h)->id);
+		(*h)->fail = tal_free((*h)->fail);
+	}
+	set_htlc_fail(peer, *h, f->reason->info.data, f->reason->info.len);
 	return NULL;
 }
 

daemon/pay.c

@@ -1,4 +1,5 @@
 #include "chaintopology.h"
+#include "failure.h"
 #include "jsonrpc.h"
 #include "lightningd.h"
 #include "log.h"
@@ -32,7 +33,29 @@ void complete_pay_command(struct peer *peer, struct htlc *htlc)
 				json_object_end(response);
 				command_success(i->cmd, response);
 			} else {
-				command_fail(i->cmd, "htlc failed");
+				FailInfo *f;
+				f = failinfo_unwrap(i->cmd, htlc->fail,
+						    tal_count(htlc->fail));
+				if (!f) {
+					command_fail(i->cmd,
+						     "htlc failed (bad message)");
+				} else {
+					struct pubkey id;
+					secp256k1_context *secpctx;
+					const char *idstr = "INVALID";
+
+					secpctx = i->cmd->dstate->secpctx;
+					if (proto_to_pubkey(secpctx,
+							    f->id, &id))
+						idstr = pubkey_to_hexstr(i->cmd,
+							 secpctx, &id);
+					command_fail(i->cmd,
+						     "htlc failed: error code %u"
+						     " node %s, reason %s",
+						     f->error_code, idstr,
+						     f->reason ? f->reason
+						     : "unknown");
+				}
 			}
 			return;
 		}
@@ -62,6 +85,8 @@ static void json_pay(struct command *cmd,
 	struct peer *peer;
 	struct pay_command *pc;
 	const u8 *onion;
+	enum fail_error error_code;
+	const char *err;
 
 	if (!json_get_params(buffer, params,
 			     "id", &idtok,
@@ -118,10 +143,10 @@ static void json_pay(struct command *cmd,
 	onion = onion_create(cmd, cmd->dstate->secpctx, route, msatoshis, fee);
 	pc = tal(cmd, struct pay_command);
 	pc->cmd = cmd;
-	pc->htlc = command_htlc_add(peer, msatoshis + fee, expiry, &rhash, NULL,
-				    onion);
-	if (!pc->htlc) {
-		command_fail(cmd, "could not add htlc");
+	err = command_htlc_add(peer, msatoshis + fee, expiry, &rhash, NULL,
+			       onion, &error_code, &pc->htlc);
+	if (err) {
+		command_fail(cmd, "could not add htlc: %u: %s", error_code, err);
 		return;
 	}
 

daemon/peer.c

@@ -57,6 +57,8 @@ struct json_connecting {
 	struct anchor_input *input;
 };
 
+static bool command_htlc_set_fail(struct peer *peer, struct htlc *htlc,
+				  enum fail_error error_code, const char *why);
 static bool command_htlc_fail(struct peer *peer, struct htlc *htlc);
 static bool command_htlc_fulfill(struct peer *peer, struct htlc *htlc);
 static void try_commit(struct peer *peer);
@@ -409,10 +411,20 @@ void set_htlc_rval(struct peer *peer,
 		   struct htlc *htlc, const struct rval *rval)
 {
 	assert(!htlc->r);
+	assert(!htlc->fail);
 	htlc->r = tal_dup(htlc, struct rval, rval);
 	db_htlc_fulfilled(peer, htlc);
 }
 
+void set_htlc_fail(struct peer *peer,
+		   struct htlc *htlc, const void *fail, size_t len)
+{
+	assert(!htlc->r);
+	assert(!htlc->fail);
+	htlc->fail = tal_dup_arr(htlc, u8, fail, len, 0);
+	db_htlc_failed(peer, htlc);
+}
+
 static void route_htlc_onwards(struct peer *peer,
 			       struct htlc *htlc,
 			       u64 msatoshis,
@@ -422,6 +434,9 @@ static void route_htlc_onwards(struct peer *peer,
 {
 	struct pubkey id;
 	struct peer *next;
+	struct htlc *newhtlc;
+	enum fail_error error_code;
+	const char *err;
 
 	if (!only_dest) {
 		log_debug_struct(peer->log, "Forwarding HTLC %s",
@@ -432,7 +447,8 @@ static void route_htlc_onwards(struct peer *peer,
 	if (!proto_to_pubkey(peer->dstate->secpctx, pb_id, &id)) {
 		log_unusual(peer->log,
 			    "Malformed pubkey for HTLC %"PRIu64, htlc->id);
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
+				      "Malformed pubkey");
 		return;
 	}
 
@@ -442,7 +458,8 @@ static void route_htlc_onwards(struct peer *peer,
 			    htlc->id, next ? "ready " : "");
 		log_add_struct(peer->log, "%s", struct pubkey, &id);
 		if (!peer->dstate->dev_never_routefail)
-			command_htlc_fail(peer, htlc);
+			command_htlc_set_fail(peer, htlc, NOT_FOUND_404,
+					      "Unknown peer");
 		return;
 	}
 
@@ -456,7 +473,8 @@ static void route_htlc_onwards(struct peer *peer,
 			    ": %"PRIi64" on %"PRIu64,
 			    htlc->id, htlc->msatoshis - msatoshis,
 			    msatoshis);
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, PAYMENT_REQUIRED_402,
+				      "Insufficent fee");
 		return;
 	}
 
@@ -464,13 +482,13 @@ static void route_htlc_onwards(struct peer *peer,
 			 struct pubkey, next->id);
 
 	/* This checks the HTLC itself is possible. */
-	if (!command_htlc_add(next, msatoshis,
-			      abs_locktime_to_blocks(&htlc->expiry)
-			      - next->nc->delay,
-			      &htlc->rhash, htlc, rest_of_route)) {
-		command_htlc_fail(peer, htlc);
-		return;
-	}
+	err = command_htlc_add(next, msatoshis,
+			       abs_locktime_to_blocks(&htlc->expiry)
+			       - next->nc->delay,
+			       &htlc->rhash, htlc, rest_of_route,
+			       &error_code, &newhtlc);
+	if (err)
+		command_htlc_set_fail(peer, htlc, error_code, err);
 }
 
 static void their_htlc_added(struct peer *peer, struct htlc *htlc,
@@ -482,7 +500,8 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 
 	if (abs_locktime_is_seconds(&htlc->expiry)) {
 		log_unusual(peer->log, "HTLC %"PRIu64" is in seconds", htlc->id);
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
+				      "bad locktime");
 		return;
 	}
 
@@ -491,7 +510,8 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 		log_unusual(peer->log, "HTLC %"PRIu64" expires too soon:"
 			    " block %u",
 			    htlc->id, abs_locktime_to_blocks(&htlc->expiry));
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
+				      "expiry too soon");
 		return;
 	}
 
@@ -500,7 +520,8 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 		log_unusual(peer->log, "HTLC %"PRIu64" expires too far:"
 			    " block %u",
 			    htlc->id, abs_locktime_to_blocks(&htlc->expiry));
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
+				      "expiry too far");
 		return;
 	}
 
@@ -509,7 +530,8 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 	if (!step) {
 		log_unusual(peer->log, "Bad onion, failing HTLC %"PRIu64,
 			    htlc->id);
-		command_htlc_fail(peer,	htlc);
+		command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
+				      "invalid onion");
 		return;
 	}
 
@@ -524,7 +546,9 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 			log_add_struct(peer->log, " rhash=%s",
 				       struct sha256, &htlc->rhash);
 			if (unlikely(!peer->dstate->dev_never_routefail))
-				command_htlc_fail(peer,	htlc);
+				command_htlc_set_fail(peer, htlc,
+						      UNAUTHORIZED_401,
+						      "unknown rhash");
 			goto free_rest;
 		}
 			
@@ -534,7 +558,9 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 				    htlc->id,
 				    htlc->msatoshis,
 				    payment->msatoshis);
-			command_htlc_fail(peer, htlc);
+			command_htlc_set_fail(peer, htlc,
+					      UNAUTHORIZED_401,
+					      "incorrect amount");
 			return;
 		}
 
@@ -551,7 +577,8 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
 		goto free_rest;
 	default:
 		log_info(peer->log, "Unknown step type %u", step->next_case);
-		command_htlc_fail(peer, htlc);
+		command_htlc_set_fail(peer, htlc, VERSION_NOT_SUPPORTED_505,
+				      "unknown step type");
 		goto free_rest;
 	}
 
@@ -561,9 +588,12 @@ free_rest:
 
 static void our_htlc_failed(struct peer *peer, struct htlc *htlc)
 {
-	if (htlc->src)
+	assert(htlc_owner(htlc) == LOCAL);
+	if (htlc->src) {
+		set_htlc_fail(htlc->src->peer, htlc->src,
+			      htlc->fail, tal_count(htlc->fail));
 		command_htlc_fail(htlc->src->peer, htlc->src);
-	else
+	} else
 		complete_pay_command(peer, htlc);
 }
 
@@ -709,7 +739,7 @@ static void check_both_committed(struct peer *peer, struct htlc *h)
 	switch (h->state) {
 	case RCVD_REMOVE_ACK_REVOCATION:
 		/* If it was fulfilled, we handled it immediately. */
-		if (!h->r)
+		if (h->fail)
 			our_htlc_failed(peer, h);
 		break;
 	case RCVD_ADD_ACK_REVOCATION:
@@ -1599,7 +1629,17 @@ static const struct bitcoin_tx *htlc_fulfill_tx(const struct peer *peer,
 	return tx;
 }
 
-/* FIXME: Reason! */
+static bool command_htlc_set_fail(struct peer *peer, struct htlc *htlc,
+				  enum fail_error error_code, const char *why)
+{
+	const u8 *fail = failinfo_create(htlc, peer->dstate->secpctx,
+					 &peer->dstate->id, error_code, why);
+
+	set_htlc_fail(peer, htlc, fail, tal_count(fail));
+	tal_free(fail);
+	return command_htlc_fail(peer, htlc);
+}
+
 static bool command_htlc_fail(struct peer *peer, struct htlc *htlc)
 {
 	/* If onchain, nothing we can do. */
@@ -1670,31 +1710,35 @@ static bool command_htlc_fulfill(struct peer *peer, struct htlc *htlc)
 	return true;
 }
 
-struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
-			      unsigned int expiry,
-			      const struct sha256 *rhash,
-			      struct htlc *src,
-			      const u8 *route)
+const char *command_htlc_add(struct peer *peer, u64 msatoshis,
+			     unsigned int expiry,
+			     const struct sha256 *rhash,
+			     struct htlc *src,
+			     const u8 *route,
+			     u32 *error_code,
+			     struct htlc **htlc)
 {
 	struct channel_state *cstate;
 	struct abs_locktime locktime;
-	struct htlc *htlc;
 
 	if (!blocks_to_abs_locktime(expiry, &locktime)) {
 		log_unusual(peer->log, "add_htlc: fail: bad expiry %u", expiry);
-		return NULL;
+		*error_code = BAD_REQUEST_400;
+		return "bad expiry";
 	}
 
 	if (expiry < get_block_height(peer->dstate) + peer->dstate->config.min_htlc_expiry) {
 		log_unusual(peer->log, "add_htlc: fail: expiry %u is too soon",
 			    expiry);
-		return NULL;
+		*error_code = BAD_REQUEST_400;
+		return "expiry too soon";
 	}
 
 	if (expiry > get_block_height(peer->dstate) + peer->dstate->config.max_htlc_expiry) {
 		log_unusual(peer->log, "add_htlc: fail: expiry %u is too far",
 			    expiry);
-		return NULL;
+		*error_code = BAD_REQUEST_400;
+		return "expiry too far";
 	}
 
 	/* BOLT #2:
@@ -1704,18 +1748,20 @@ struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
 	 */
 	if (peer->remote.staging_cstate->side[OURS].num_htlcs == 300) {
 		log_unusual(peer->log, "add_htlc: fail: already at limit");
-		return NULL;
+		*error_code = SERVICE_UNAVAILABLE_503;
+		return "channel full";
 	}
 
 	if (!state_can_add_htlc(peer->state)) {
 		log_unusual(peer->log, "add_htlc: fail: peer state %s",
 			    state_name(peer->state));
-		return NULL;
+		*error_code = NOT_FOUND_404;
+		return "peer not available";
 	}
 
-	htlc = peer_new_htlc(peer, peer->htlc_id_counter,
-			     msatoshis, rhash, expiry, route, tal_count(route),
-			     src, SENT_ADD_HTLC);
+	*htlc = peer_new_htlc(peer, peer->htlc_id_counter,
+			      msatoshis, rhash, expiry, route, tal_count(route),
+			      src, SENT_ADD_HTLC);
 
 	/* FIXME: BOLT is not correct here: we should say IFF we cannot
 	 * afford it in remote at its own current proposed fee-rate. */
@@ -1725,20 +1771,24 @@ struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
 	 * the remote commitment transaction at the current `fee_rate`
 	 */
 	cstate = copy_cstate(peer, peer->remote.staging_cstate);
-	if (!cstate_add_htlc(cstate, htlc)) {
+	if (!cstate_add_htlc(cstate, *htlc)) {
 		log_unusual(peer->log, "add_htlc: fail: Cannot afford %"PRIu64
 			    " milli-satoshis in their commit tx",
 			    msatoshis);
-		return tal_free(htlc);
+		*htlc = tal_free(*htlc);
+		*error_code = SERVICE_UNAVAILABLE_503;
+		return "cannot afford htlc";
 	}
 	tal_free(cstate);
 
 	cstate = copy_cstate(peer, peer->local.staging_cstate);
-	if (!cstate_add_htlc(cstate, htlc)) {
+	if (!cstate_add_htlc(cstate, *htlc)) {
 		log_unusual(peer->log, "add_htlc: fail: Cannot afford %"PRIu64
 			    " milli-satoshis in our commit tx",
 			    msatoshis);
-		return tal_free(htlc);
+		*htlc = tal_free(*htlc);
+		*error_code = SERVICE_UNAVAILABLE_503;
+		return "cannot afford htlc";
 	}
 	tal_free(cstate);
 
@@ -1747,17 +1797,17 @@ struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
 	 * The sending node MUST add the HTLC addition to the unacked
 	 * changeset for its remote commitment
 	 */
-	if (!cstate_add_htlc(peer->remote.staging_cstate, htlc))
+	if (!cstate_add_htlc(peer->remote.staging_cstate, *htlc))
 		fatal("Could not add HTLC?");
 
 	remote_changes_pending(peer);
 
-	queue_pkt_htlc_add(peer, htlc);
+	queue_pkt_htlc_add(peer, *htlc);
 
 	/* Make sure we never offer the same one twice. */
 	peer->htlc_id_counter++;
 
-	return htlc;
+	return NULL;
 }
 
 static struct io_plan *pkt_out(struct io_conn *conn, struct peer *peer)
@@ -2463,6 +2513,7 @@ struct htlc *peer_new_htlc(struct peer *peer,
 	h->msatoshis = msatoshis;
 	h->rhash = *rhash;
 	h->r = NULL;
+	h->fail = NULL;
 	if (!blocks_to_abs_locktime(expiry, &h->expiry))
 		fatal("Invalid HTLC expiry %u", expiry);
 	h->routing = tal_dup_arr(h, u8, route, routelen, 0);
@@ -2891,7 +2942,8 @@ static void check_htlc_expiry(struct peer *peer)
 			continue;
 
 		/* This can fail only if we're in an error state. */
-		if (!command_htlc_fail(peer, h))
+		if (!command_htlc_set_fail(peer, h,
+					   REQUEST_TIMEOUT_408, "timed out"))
 			return;
 	}
 
@@ -3132,6 +3184,14 @@ static const struct bitcoin_tx *irrevocably_resolved(struct peer *peer)
 	return peer->onchain.tx;
 }
 
+/* We usually don't fail HTLCs we offered, but if the peer breaks down
+ * before we've confirmed it, this is exactly what happens. */
+static void fail_own_htlc(struct peer *peer, struct htlc *htlc)
+{
+	set_htlc_fail(peer, htlc, "peer closed", strlen("peer closed"));
+	our_htlc_failed(peer, htlc);
+}
+
 /* We've spent an HTLC output to get our funds back.  There's still a 
  * chance that they could also spend the HTLC output (using the preimage),
  * so we need to wait for some confirms.
@@ -3149,7 +3209,7 @@ static enum watch_result our_htlc_timeout_depth(struct peer *peer,
 		return KEEP_WATCHING;
 	if (depth + 1 < peer->dstate->config.min_htlc_expiry)
 		return KEEP_WATCHING;
-	our_htlc_failed(peer, htlc);
+	fail_own_htlc(peer, htlc);
 	return DELETE_WATCH;
 }
 
@@ -3291,7 +3351,7 @@ static enum watch_result our_unilateral_depth(struct peer *peer,
 			log_debug(peer->log,
 				  "%s:failing uncommitted htlc %"PRIu64,
 				  __func__, h->id);
-			our_htlc_failed(peer, h);
+			fail_own_htlc(peer, h);
 		}
 	}
 	return DELETE_WATCH;
@@ -3728,7 +3788,7 @@ static enum watch_result anchor_spent(struct peer *peer,
 	     h;
 	     h = htlc_map_next(&peer->htlcs, &it)) {
 		if (h->state == SENT_ADD_HTLC) {
-			our_htlc_failed(peer, h);
+			fail_own_htlc(peer, h);
 		}
 	}
 
@@ -4302,6 +4362,8 @@ static void json_newhtlc(struct command *cmd,
 	struct sha256 rhash;
 	struct json_result *response = new_json_result(cmd);
 	struct htlc *htlc;
+	const char *err;
+	enum fail_error error_code;
 
 	if (!json_get_params(buffer, params,
 			     "peerid", &peeridtok,
@@ -4352,10 +4414,11 @@ static void json_newhtlc(struct command *cmd,
 	}
 
 	log_debug(peer->log, "JSON command to add new HTLC");
-	htlc = command_htlc_add(peer, msatoshis, expiry, &rhash, NULL,
-				dummy_single_route(cmd, peer, msatoshis));
-	if (!htlc) {
-		command_fail(cmd, "could not add htlc");
+	err = command_htlc_add(peer, msatoshis, expiry, &rhash, NULL,
+			       dummy_single_route(cmd, peer, msatoshis),
+			       &error_code, &htlc);
+	if (err) {
+		command_fail(cmd, "could not add htlc: %u:%s", error_code, err);
 		return;
 	}
 	log_debug(peer->log, "JSON new HTLC is %"PRIu64, htlc->id);
@@ -4467,15 +4530,16 @@ static void json_failhtlc(struct command *cmd,
 			  const char *buffer, const jsmntok_t *params)
 {
 	struct peer *peer;
-	jsmntok_t *peeridtok, *idtok;
+	jsmntok_t *peeridtok, *idtok, *reasontok;
 	u64 id;
 	struct htlc *htlc;
 
 	if (!json_get_params(buffer, params,
 			     "peerid", &peeridtok,
 			     "id", &idtok,
+			     "reason", &reasontok,
 			     NULL)) {
-		command_fail(cmd, "Need peerid and id");
+		command_fail(cmd, "Need peerid, id and reason");
 		return;
 	}
 
@@ -4514,6 +4578,8 @@ static void json_failhtlc(struct command *cmd,
 		return;
 	}
 
+	set_htlc_fail(peer, htlc, buffer + reasontok->start,
+		      reasontok->end - reasontok->start);
 	if (command_htlc_fail(peer, htlc))
 		command_success(cmd, null_response(cmd));
 	else
@@ -4525,7 +4591,7 @@ static void json_failhtlc(struct command *cmd,
 const struct json_command failhtlc_command = {
 	"failhtlc",
 	json_failhtlc,
-	"Fail htlc proposed by {peerid} which has {id}",
+	"Fail htlc proposed by {peerid} which has {id}, using {reason}",
 	"Returns an empty result on success"
 };
 

daemon/peer.h

@@ -7,6 +7,7 @@
 #include "bitcoin/script.h"
 #include "bitcoin/shadouble.h"
 #include "channel.h"
+#include "failure.h"
 #include "feechange.h"
 #include "htlc.h"
 #include "lightning.pb-c.h"
@@ -247,6 +248,9 @@ struct peer *new_peer(struct lightningd_state *dstate,
 void set_htlc_rval(struct peer *peer,
 		   struct htlc *htlc, const struct rval *rval);
 
+void set_htlc_fail(struct peer *peer,
+		   struct htlc *htlc, const void *fail, size_t fail_len);
+
 /* Populates very first peer->{local,remote}.commit->{tx,cstate} */
 bool setup_first_commit(struct peer *peer);
 
@@ -268,11 +272,13 @@ struct htlc *peer_new_htlc(struct peer *peer,
 			   struct htlc *src,
 			   enum htlc_state state);
 
-struct htlc *command_htlc_add(struct peer *peer, u64 msatoshis,
-			      unsigned int expiry,
-			      const struct sha256 *rhash,
-			      struct htlc *src,
-			      const u8 *route);
+const char *command_htlc_add(struct peer *peer, u64 msatoshis,
+			     unsigned int expiry,
+			     const struct sha256 *rhash,
+			     struct htlc *src,
+			     const u8 *route,
+			     enum fail_error *error_code,
+			     struct htlc **htlc);
 
 void peer_unexpected_pkt(struct peer *peer, const Pkt *pkt, const char *where);
 

daemon/test/test.sh

@@ -714,7 +714,7 @@ A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
 A_FEE=$(($A_FEE + $EXTRA_FEE))
 check_status $A_AMOUNT $A_FEE "{ msatoshis : $HTLC_AMOUNT, expiry : { block : $EXPIRY }, rhash : $RHASH , state : SENT_ADD_ACK_REVOCATION } " $B_AMOUNT $B_FEE ""
 
-lcli2 failhtlc $ID1 $HTLCID
+lcli2 failhtlc $ID1 $HTLCID 695
 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
 
@@ -819,7 +819,7 @@ if [ -n "$CLOSE_WITH_HTLCS" ]; then
     check_peerstate lcli2 STATE_SHUTDOWN
 
     # Fail one, still waiting.
-    lcli2 failhtlc $ID1 $HTLCID
+    lcli2 failhtlc $ID1 $HTLCID 800
     check_peerstate lcli1 STATE_SHUTDOWN
     check_peerstate lcli2 STATE_SHUTDOWN
 
@@ -846,7 +846,7 @@ if [ -n "$CLOSE_WITH_HTLCS" ]; then
 fi
 
 lcli1 fulfillhtlc $ID2 $HTLCID2 $SECRET2
-lcli2 failhtlc $ID1 $HTLCID
+lcli2 failhtlc $ID1 $HTLCID 849
 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
 [ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
 [ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1

lightning.pb-c.c

@@ -738,6 +738,49 @@ void   update_fulfill_htlc__free_unpacked
   assert(message->base.descriptor == &update_fulfill_htlc__descriptor);
   protobuf_c_message_free_unpacked ((ProtobufCMessage*)message, allocator);
 }
+void   fail_info__init
+                     (FailInfo         *message)
+{
+  static FailInfo init_value = FAIL_INFO__INIT;
+  *message = init_value;
+}
+size_t fail_info__get_packed_size
+                     (const FailInfo *message)
+{
+  assert(message->base.descriptor == &fail_info__descriptor);
+  return protobuf_c_message_get_packed_size ((const ProtobufCMessage*)(message));
+}
+size_t fail_info__pack
+                     (const FailInfo *message,
+                      uint8_t       *out)
+{
+  assert(message->base.descriptor == &fail_info__descriptor);
+  return protobuf_c_message_pack ((const ProtobufCMessage*)message, out);
+}
+size_t fail_info__pack_to_buffer
+                     (const FailInfo *message,
+                      ProtobufCBuffer *buffer)
+{
+  assert(message->base.descriptor == &fail_info__descriptor);
+  return protobuf_c_message_pack_to_buffer ((const ProtobufCMessage*)message, buffer);
+}
+FailInfo *
+       fail_info__unpack
+                     (ProtobufCAllocator  *allocator,
+                      size_t               len,
+                      const uint8_t       *data)
+{
+  return (FailInfo *)
+     protobuf_c_message_unpack (&fail_info__descriptor,
+                                allocator, len, data);
+}
+void   fail_info__free_unpacked
+                     (FailInfo *message,
+                      ProtobufCAllocator *allocator)
+{
+  assert(message->base.descriptor == &fail_info__descriptor);
+  protobuf_c_message_free_unpacked ((ProtobufCMessage*)message, allocator);
+}
 void   fail_reason__init
                      (FailReason         *message)
 {
@@ -2219,6 +2262,70 @@ const ProtobufCMessageDescriptor update_fulfill_htlc__descriptor =
   (ProtobufCMessageInit) update_fulfill_htlc__init,
   NULL,NULL,NULL    /* reserved[123] */
 };
+static const ProtobufCFieldDescriptor fail_info__field_descriptors[3] =
+{
+  {
+    "id",
+    1,
+    PROTOBUF_C_LABEL_REQUIRED,
+    PROTOBUF_C_TYPE_MESSAGE,
+    0,   /* quantifier_offset */
+    offsetof(FailInfo, id),
+    &bitcoin_pubkey__descriptor,
+    NULL,
+    0,             /* flags */
+    0,NULL,NULL    /* reserved1,reserved2, etc */
+  },
+  {
+    "error_code",
+    2,
+    PROTOBUF_C_LABEL_REQUIRED,
+    PROTOBUF_C_TYPE_UINT32,
+    0,   /* quantifier_offset */
+    offsetof(FailInfo, error_code),
+    NULL,
+    NULL,
+    0,             /* flags */
+    0,NULL,NULL    /* reserved1,reserved2, etc */
+  },
+  {
+    "reason",
+    3,
+    PROTOBUF_C_LABEL_OPTIONAL,
+    PROTOBUF_C_TYPE_STRING,
+    0,   /* quantifier_offset */
+    offsetof(FailInfo, reason),
+    NULL,
+    NULL,
+    0,             /* flags */
+    0,NULL,NULL    /* reserved1,reserved2, etc */
+  },
+};
+static const unsigned fail_info__field_indices_by_name[] = {
+  1,   /* field[1] = error_code */
+  0,   /* field[0] = id */
+  2,   /* field[2] = reason */
+};
+static const ProtobufCIntRange fail_info__number_ranges[1 + 1] =
+{
+  { 1, 0 },
+  { 0, 3 }
+};
+const ProtobufCMessageDescriptor fail_info__descriptor =
+{
+  PROTOBUF_C__MESSAGE_DESCRIPTOR_MAGIC,
+  "fail_info",
+  "FailInfo",
+  "FailInfo",
+  "",
+  sizeof(FailInfo),
+  3,
+  fail_info__field_descriptors,
+  fail_info__field_indices_by_name,
+  1,  fail_info__number_ranges,
+  (ProtobufCMessageInit) fail_info__init,
+  NULL,NULL,NULL    /* reserved[123] */
+};
 static const ProtobufCFieldDescriptor fail_reason__field_descriptors[1] =
 {
   {

lightning.pb-c.h

@@ -32,6 +32,7 @@ typedef struct _Route Route;
 typedef struct _Routing Routing;
 typedef struct _UpdateAddHtlc UpdateAddHtlc;
 typedef struct _UpdateFulfillHtlc UpdateFulfillHtlc;
+typedef struct _FailInfo FailInfo;
 typedef struct _FailReason FailReason;
 typedef struct _UpdateFailHtlc UpdateFailHtlc;
 typedef struct _UpdateFee UpdateFee;
@@ -404,8 +405,20 @@ struct  _UpdateFulfillHtlc
 
 
 /*
- * FIXME: Failure information.
+ * This is encrypted in fail_reason.
  */
+struct  _FailInfo
+{
+  ProtobufCMessage base;
+  BitcoinPubkey *id;
+  uint32_t error_code;
+  char *reason;
+};
+#define FAIL_INFO__INIT \
+ { PROTOBUF_C_MESSAGE_INIT (&fail_info__descriptor) \
+    , NULL, 0, NULL }
+
+
 struct  _FailReason
 {
   ProtobufCMessage base;
@@ -915,6 +928,25 @@ UpdateFulfillHtlc *
 void   update_fulfill_htlc__free_unpacked
                      (UpdateFulfillHtlc *message,
                       ProtobufCAllocator *allocator);
+/* FailInfo methods */
+void   fail_info__init
+                     (FailInfo         *message);
+size_t fail_info__get_packed_size
+                     (const FailInfo   *message);
+size_t fail_info__pack
+                     (const FailInfo   *message,
+                      uint8_t             *out);
+size_t fail_info__pack_to_buffer
+                     (const FailInfo   *message,
+                      ProtobufCBuffer     *buffer);
+FailInfo *
+       fail_info__unpack
+                     (ProtobufCAllocator  *allocator,
+                      size_t               len,
+                      const uint8_t       *data);
+void   fail_info__free_unpacked
+                     (FailInfo *message,
+                      ProtobufCAllocator *allocator);
 /* FailReason methods */
 void   fail_reason__init
                      (FailReason         *message);
@@ -1139,6 +1171,9 @@ typedef void (*UpdateAddHtlc_Closure)
 typedef void (*UpdateFulfillHtlc_Closure)
                  (const UpdateFulfillHtlc *message,
                   void *closure_data);
+typedef void (*FailInfo_Closure)
+                 (const FailInfo *message,
+                  void *closure_data);
 typedef void (*FailReason_Closure)
                  (const FailReason *message,
                   void *closure_data);
@@ -1190,6 +1225,7 @@ extern const ProtobufCMessageDescriptor route__descriptor;
 extern const ProtobufCMessageDescriptor routing__descriptor;
 extern const ProtobufCMessageDescriptor update_add_htlc__descriptor;
 extern const ProtobufCMessageDescriptor update_fulfill_htlc__descriptor;
+extern const ProtobufCMessageDescriptor fail_info__descriptor;
 extern const ProtobufCMessageDescriptor fail_reason__descriptor;
 extern const ProtobufCMessageDescriptor update_fail_htlc__descriptor;
 extern const ProtobufCMessageDescriptor update_fee__descriptor;

lightning.proto

@@ -166,7 +166,13 @@ message update_fulfill_htlc {
   required rval r = 2;
 }
 
-// FIXME: Failure information.
+// This is encrypted in fail_reason.
+message fail_info {
+  required bitcoin_pubkey id = 1;
+  required uint32 error_code = 2;
+  optional string reason = 3;
+}
+
 message fail_reason {
   required bytes info = 1;
 }