pay: make interface idempotent.

We stopped automatically retransmitting locally-generated add/removes
after a reconnect, but this breaks the "pay" interface as it stands.

The correct solution to this is to make the pay interface idempotent:
you can trigger it as many times as you want and it will only succeed
once.

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

daemon/lightningd.c

@@ -246,6 +246,7 @@ static struct lightningd_state *lightningd_state(void)
 				   "lightningd(%u):", (int)getpid());
 
 	list_head_init(&dstate->peers);
+	list_head_init(&dstate->pay_commands);
 	timers_init(&dstate->timers, controlled_time());
 	txwatch_hash_init(&dstate->txwatches);
 	txowatch_hash_init(&dstate->txowatches);

daemon/lightningd.h

@@ -87,6 +87,9 @@ struct lightningd_state {
 	/* Addresses to contact peers. */
 	struct list_head addresses;
 
+	/* Any outstanding "pay" commands. */
+	struct list_head pay_commands;
+	
 	/* Crypto tables for global use. */
 	secp256k1_context *secpctx;
 

daemon/pay.c

@@ -8,67 +8,106 @@
 #include "peer.h"
 #include "routing.h"
 #include <ccan/str/hex/hex.h>
+#include <ccan/structeq/structeq.h>
 #include <inttypes.h>
 
 /* Outstanding "pay" commands. */
 struct pay_command {
 	struct list_node list;
+	struct sha256 rhash;
+	u64 msatoshis, fee;
+	struct pubkey id;
+	/* Set if this is in progress. */
 	struct htlc *htlc;
+	/* Preimage if this succeeded. */
+	struct rval *rval;
 	struct command *cmd;
 };
 
-void complete_pay_command(struct peer *peer, struct htlc *htlc)
+static void json_pay_success(struct command *cmd, const struct rval *rval)
+{
+	struct json_result *response;
+
+	response = new_json_result(cmd);
+	json_object_start(response, NULL);
+	json_add_hex(response, "preimage", rval, sizeof(*rval));
+	json_object_end(response);
+	command_success(cmd, response);
+}
+
+static void handle_json(struct command *cmd, const struct htlc *htlc)
+{
+	FailInfo *f;
+	struct pubkey id;
+	const char *idstr = "INVALID";
+
+	if (htlc->r) {
+		json_pay_success(cmd, htlc->r);
+		return;
+	}
+
+	f = failinfo_unwrap(cmd, htlc->fail, tal_count(htlc->fail));
+	if (!f) {
+		command_fail(cmd, "failed (bad message)");
+		return;
+	}
+
+	if (proto_to_pubkey(cmd->dstate->secpctx, f->id, &id))
+		idstr = pubkey_to_hexstr(cmd, cmd->dstate->secpctx, &id);
+
+	command_fail(cmd,
+		     "failed: error code %u node %s reason %s",
+		     f->error_code, idstr, f->reason ? f->reason : "unknown");
+}
+
+void complete_pay_command(struct lightningd_state *dstate,
+			  const struct htlc *htlc)
 {
 	struct pay_command *i;
 
-	list_for_each(&peer->pay_commands, i, list) {
+	list_for_each(&dstate->pay_commands, i, list) {
 		if (i->htlc == htlc) {
-			if (htlc->r) {
-				struct json_result *response;
-
-				response = new_json_result(i->cmd);	
-				json_object_start(response, NULL);
-				json_add_hex(response, "preimage",
-					     htlc->r, sizeof(*htlc->r));
-				json_object_end(response);
-				command_success(i->cmd, response);
-			} else {
-				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");
-				}
-			}
+			if (htlc->r)
+				i->rval = tal_dup(i, struct rval, htlc->r);
+			i->htlc = NULL;
+
+			/* Can be NULL if JSON RPC goes away. */
+			if (i->cmd)
+				handle_json(i->cmd, htlc);
 			return;
 		}
 	}
 
 	/* Can happen if RPC connection goes away. */
-	log_unusual(peer->log, "No command for HTLC %"PRIu64" %s",
+	log_unusual(dstate->base_log, "No command for HTLC %"PRIu64" %s",
 		    htlc->id, htlc->r ? "fulfill" : "fail");
 }
 
-static void remove_from_list(struct pay_command *pc)
+/* When JSON RPC goes away, cmd is freed: detach from any running paycommand */
+static void remove_cmd_from_pc(struct command *cmd)
 {
-	list_del(&pc->list);
+	struct pay_command *pc;
+
+	list_for_each(&cmd->dstate->pay_commands, pc, list) {
+		if (pc->cmd == cmd) {
+			pc->cmd = NULL;
+			return;
+		}
+	}
+	/* We can reach here, in the case where another pay command
+	 * re-uses the pc->cmd before we get around to cleaning up. */
+}
+
+static struct pay_command *find_pay_command(struct lightningd_state *dstate,
+					    const struct sha256 *rhash)
+{
+	struct pay_command *pc;
+
+	list_for_each(&dstate->pay_commands, pc, list) {
+		if (structeq(rhash, &pc->rhash))
+			return pc;
+	}
+	return NULL;
 }
 
 static void json_pay(struct command *cmd,
@@ -120,6 +159,39 @@ static void json_pay(struct command *cmd,
 		return;
 	}
 
+	pc = find_pay_command(cmd->dstate, &rhash);
+	if (pc) {
+		log_debug(cmd->dstate->base_log, "json_pay: found previous");
+		if (pc->htlc) {
+			log_add(cmd->dstate->base_log, "... still in progress");
+			command_fail(cmd, "still in progress");
+			return;
+		}
+		if (pc->rval) {
+			log_add(cmd->dstate->base_log, "... succeeded");
+			/* Must match successful payment parameters. */
+			if (pc->msatoshis != msatoshis) {
+				command_fail(cmd,
+					     "already succeeded with amount %"
+					     PRIu64, pc->msatoshis);
+				return;
+			}
+			if (!structeq(&pc->id, &id)) {
+				char *previd;
+				previd = pubkey_to_hexstr(cmd,
+							  cmd->dstate->secpctx,
+							  &pc->id);
+				command_fail(cmd,
+					     "already succeeded to %s",
+					     previd);
+				return;
+			}
+			json_pay_success(cmd, pc->rval);
+			return;
+		}
+		log_add(cmd->dstate->base_log, "... retrying");
+	}
+
 	/* FIXME: Add fee param, check for excessive fee. */
 	peer = find_route(cmd->dstate, &id, msatoshis, &fee, &route);
 	if (!peer) {
@@ -141,8 +213,16 @@ static void json_pay(struct command *cmd,
 	expiry += get_block_height(cmd->dstate) + 1;
 
 	onion = onion_create(cmd, cmd->dstate->secpctx, route, msatoshis, fee);
-	pc = tal(cmd, struct pay_command);
+
+	if (!pc)
+		pc = tal(cmd->dstate, struct pay_command);
 	pc->cmd = cmd;
+	pc->rhash = rhash;
+	pc->rval = NULL;
+	pc->id = id;
+	pc->msatoshis = msatoshis;
+	pc->fee = fee;
+
 	err = command_htlc_add(peer, msatoshis + fee, expiry, &rhash, NULL,
 			       onion, &error_code, &pc->htlc);
 	if (err) {
@@ -151,13 +231,13 @@ static void json_pay(struct command *cmd,
 	}
 
 	/* Wait until we get response. */
-	list_add_tail(&peer->pay_commands, &pc->list);
-	tal_add_destructor(pc, remove_from_list);
+	list_add_tail(&cmd->dstate->pay_commands, &pc->list);
+	tal_add_destructor(cmd, remove_cmd_from_pc);
 }
 
 const struct json_command pay_command = {
 	"pay",
 	json_pay,
 	"Send {id} {msatoshis} in return for preimage of {rhash}",
-	"Returns an empty result on success"
+	"Returns the {preimage} on success"
 };

daemon/pay.h

@@ -2,10 +2,10 @@
 #define LIGHTNING_DAEMON_PAY_H
 #include "config.h"
 
-struct peer;
+struct lightningd_state;
 struct htlc;
-struct rval;
 
-void complete_pay_command(struct peer *peer, struct htlc *htlc);
+void complete_pay_command(struct lightningd_state *dstate,
+			  const struct htlc *htlc);
 
 #endif /* LIGHTNING_DAEMON_PAY_H */

daemon/peer.c

@@ -594,7 +594,7 @@ static void our_htlc_failed(struct peer *peer, struct htlc *htlc)
 			      htlc->fail, tal_count(htlc->fail));
 		command_htlc_fail(htlc->src->peer, htlc->src);
 	} else
-		complete_pay_command(peer, htlc);
+		complete_pay_command(peer->dstate, htlc);
 }
 
 static void our_htlc_fulfilled(struct peer *peer, struct htlc *htlc)
@@ -603,7 +603,7 @@ static void our_htlc_fulfilled(struct peer *peer, struct htlc *htlc)
 		set_htlc_rval(htlc->src->peer, htlc->src, htlc->r);
 		command_htlc_fulfill(htlc->src->peer, htlc->src);
 	} else {
-		complete_pay_command(peer, htlc);
+		complete_pay_command(peer->dstate, htlc);
 	}
 }
 
@@ -2369,7 +2369,6 @@ struct peer *new_peer(struct lightningd_state *dstate,
 	peer->outpkt = tal_arr(peer, Pkt *, 0);
 	peer->commit_jsoncmd = NULL;
 	list_head_init(&peer->outgoing_txs);
-	list_head_init(&peer->pay_commands);
 	list_head_init(&peer->their_commits);
 	peer->anchor.ok_depth = -1;
 	peer->order_counter = 0;

daemon/peer.h

@@ -99,9 +99,6 @@ struct peer {
 	/* If we're doing a commit, this is the command which triggered it */
 	struct command *commit_jsoncmd;
 
-	/* Any outstanding "pay" commands. */
-	struct list_head pay_commands;
-	
 	/* Global state. */
 	struct lightningd_state *dstate;
 

daemon/test/test.sh

@@ -1008,6 +1008,9 @@ if [ ! -n "$MANUALCOMMIT" ]; then
     lcli1 add-route $ID2 $ID3 546000 10 36 36
     RHASH5=`lcli3 accept-payment $HTLC_AMOUNT | sed 's/.*"\([0-9a-f]*\)".*/\1/'`
 
+    # FIXME: We don't save payments in db yet!
+    DO_RECONNECT=""
+
     # Try wrong hash.
     if lcli1 pay $ID3 $HTLC_AMOUNT $RHASH4; then
 	echo Paid with wrong hash? >&2
@@ -1028,6 +1031,15 @@ if [ ! -n "$MANUALCOMMIT" ]; then
     # starts.
     check lcli3 "getpeers | $FGREP \"\\\"our_amount\\\" : $(($HTLC_AMOUNT - $NO_HTLCS_FEE / 2))\""
     lcli3 close $ID2
+
+    # Re-send should be a noop (doesn't matter that node3 is down!)
+    lcli1 pay $ID3 $HTLC_AMOUNT $RHASH5
+
+    # Re-send to different id or amount should complain.
+    lcli1 pay $ID2 $HTLC_AMOUNT $RHASH5 | $FGREP "already succeeded to $ID3"
+    lcli1 pay $ID2 $(($HTLC_AMOUNT + 1)) $RHASH5 | $FGREP "already succeeded with amount $HTLC_AMOUNT"
+
+    DO_RECONNECT=$RECONNECT
 fi
 
 lcli1 close $ID2