db: Always fulfill HTLC inside a transaction.

This is important when we put payments in the database: they need to be
updated atomically as the HTLC is.

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

daemon/db.c

@@ -1386,9 +1386,7 @@ bool db_htlc_fulfilled(struct peer *peer, const struct htlc *htlc)
 
 	log_debug(peer->log, "%s(%s)", __func__, peerid);
 
-	/* When called from their_htlc_added() and it's a payment to
-	 * us, we are in a transaction.  When called due to
-	 * PKT_UPDATE_FULFILL_HTLC we are not. */
+	assert(peer->dstate->db->in_transaction);
 	errmsg = db_exec(ctx, peer->dstate, 
 			 "UPDATE htlcs SET r=x'%s' WHERE peer=x'%s' AND id=%"PRIu64" AND state='%s';",
 			 tal_hexstr(ctx, htlc->r, sizeof(*htlc->r)),

daemon/packets.c

@@ -467,11 +467,10 @@ Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h)
 }
 
 Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt, struct htlc **h,
-			     bool *was_already_fulfilled)
+			     struct rval *r)
 {
 	const UpdateFulfillHtlc *f = pkt->update_fulfill_htlc;
 	struct sha256 rhash;
-	struct rval r;
 	Pkt *err;
 
 	err = find_commited_htlc(peer, f->id, h);
@@ -479,18 +478,12 @@ Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt, struct htlc **h,
 		return err;
 
 	/* Now, it must solve the HTLC rhash puzzle. */
-	proto_to_rval(f->r, &r);
-	sha256(&rhash, &r, sizeof(r));
+	proto_to_rval(f->r, r);
+	sha256(&rhash, r, sizeof(*r));
 
 	if (!structeq(&rhash, &(*h)->rhash))
 		return pkt_err(peer, "Invalid r for %"PRIu64, f->id);
 
-	if ((*h)->r) {
-		*was_already_fulfilled = true;
-	} else {
-		*was_already_fulfilled = false;
-		set_htlc_rval(peer, *h, &r);
-	}
 	return NULL;
 }
 

daemon/packets.h

@@ -47,7 +47,7 @@ Pkt *accept_pkt_htlc_add(struct peer *peer, const Pkt *pkt, struct htlc **h);
 Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h);
 
 Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt, struct htlc **h,
-			     bool *was_already_fulfilled);
+			     struct rval *r);
 
 Pkt *accept_pkt_update_fee(struct peer *peer, const Pkt *pkt, u64 *feerate);
 

daemon/peer.c

@@ -402,8 +402,8 @@ static bool peer_received_unexpected_pkt(struct peer *peer, const Pkt *pkt,
 	return peer_comms_err(peer, pkt_err_unexpected(peer, pkt));
 }
 
-void set_htlc_rval(struct peer *peer,
-		   struct htlc *htlc, const struct rval *rval)
+static void set_htlc_rval(struct peer *peer,
+			  struct htlc *htlc, const struct rval *rval)
 {
 	assert(!htlc->r);
 	assert(!htlc->fail);
@@ -1133,16 +1133,24 @@ static Pkt *handle_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt)
 {
 	struct htlc *htlc;
 	Pkt *err;
-	bool was_already_fulfilled;
+	struct rval r;
 
-	/* Reconnect may mean HTLC was already fulfilled.  That's OK. */
-	err = accept_pkt_htlc_fulfill(peer, pkt, &htlc, &was_already_fulfilled);
+	err = accept_pkt_htlc_fulfill(peer, pkt, &htlc, &r);
 	if (err)
 		return err;
 	
-	/* We can relay this upstream immediately. */
-	if (!was_already_fulfilled)
+	/* Reconnect may mean HTLC was already fulfilled.  That's OK. */
+	if (!htlc->r) {
+		if (!db_start_transaction(peer))
+			return pkt_err(peer, "database error");
+
+		set_htlc_rval(peer, htlc, &r);
+
+		/* We can relay this upstream immediately. */
 		our_htlc_fulfilled(peer, htlc);
+		if (!db_commit_transaction(peer))
+			return pkt_err(peer, "database error");
+	}
 
 	/* BOLT #2:
 	 *
@@ -4409,6 +4417,7 @@ static void json_newhtlc(struct command *cmd,
 	command_success(cmd, response);
 }
 
+/* FIXME: rename to dev- */
 const struct json_command newhtlc_command = {
 	"newhtlc",
 	json_newhtlc,
@@ -4416,6 +4425,7 @@ const struct json_command newhtlc_command = {
 	"Returns { id: u64 } result on success"
 };
 
+/* FIXME: rename to dev- */
 static void json_fulfillhtlc(struct command *cmd,
 			     const char *buffer, const jsmntok_t *params)
 {
@@ -4487,9 +4497,22 @@ static void json_fulfillhtlc(struct command *cmd,
 
 	/* This can happen if we're disconnected, and thus haven't sent
 	 * fulfill yet; we stored r in database immediately. */
-	if (!htlc->r)
+	if (!htlc->r) {
+		if (!db_start_transaction(peer)) {
+			command_fail(cmd, "database error");
+			return;
+		}
+
 		set_htlc_rval(peer, htlc, &r);
 
+		/* We can relay this upstream immediately. */
+		our_htlc_fulfilled(peer, htlc);
+		if (!db_commit_transaction(peer)) {
+			command_fail(cmd, "database error");
+			return;
+		}
+	}
+
 	if (command_htlc_fulfill(peer, htlc))
 		command_success(cmd, null_response(cmd));
 	else

daemon/peer.h

@@ -243,9 +243,6 @@ struct peer *new_peer(struct lightningd_state *dstate,
 		      enum state state,
 		      enum state_input offer_anchor);
 
-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);