From 012574790d9dfce43288f98d4af207f383363884 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 31 Aug 2016 16:04:59 +0930 Subject: [PATCH] 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 --- daemon/lightningd.c | 1 + daemon/lightningd.h | 3 + daemon/pay.c | 166 ++++++++++++++++++++++++++++++++------------ daemon/pay.h | 6 +- daemon/peer.c | 5 +- daemon/peer.h | 3 - daemon/test/test.sh | 12 ++++ 7 files changed, 144 insertions(+), 52 deletions(-) diff --git a/daemon/lightningd.c b/daemon/lightningd.c index 6487d6909..4f00d3b48 100644 --- a/daemon/lightningd.c +++ b/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); diff --git a/daemon/lightningd.h b/daemon/lightningd.h index 3923ccee0..8d1c5c171 100644 --- a/daemon/lightningd.h +++ b/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; diff --git a/daemon/pay.c b/daemon/pay.c index 74db2c03c..524b8c1c0 100644 --- a/daemon/pay.c +++ b/daemon/pay.c @@ -8,67 +8,106 @@ #include "peer.h" #include "routing.h" #include +#include #include /* 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" }; diff --git a/daemon/pay.h b/daemon/pay.h index 2b117d6e3..75a90b70f 100644 --- a/daemon/pay.h +++ b/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 */ diff --git a/daemon/peer.c b/daemon/peer.c index 560ea73ac..449a1a61d 100644 --- a/daemon/peer.c +++ b/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; diff --git a/daemon/peer.h b/daemon/peer.h index de9ba317a..954ed1b00 100644 --- a/daemon/peer.h +++ b/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; diff --git a/daemon/test/test.sh b/daemon/test/test.sh index d421f992c..99fd5bd43 100755 --- a/daemon/test/test.sh +++ b/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