From b47fbfead05e237c30976febf4a87053f1d183be Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 2 Sep 2016 12:02:18 +0930 Subject: [PATCH] db: Always fail 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 --- daemon/db.c | 3 +-- daemon/packets.c | 10 +++------- daemon/packets.h | 3 ++- daemon/peer.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- daemon/peer.h | 3 --- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/daemon/db.c b/daemon/db.c index 6448d3d61..03dafa2ea 100644 --- a/daemon/db.c +++ b/daemon/db.c @@ -1407,8 +1407,7 @@ bool db_htlc_failed(struct peer *peer, const struct htlc *htlc) 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. */ + assert(peer->dstate->db->in_transaction); 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)), diff --git a/daemon/packets.c b/daemon/packets.c index beb02f994..ba55f6e97 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -444,7 +444,8 @@ static Pkt *find_commited_htlc(struct peer *peer, uint64_t id, return NULL; } -Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h) +Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h, + u8 **fail) { const UpdateFailHtlc *f = pkt->update_fail_htlc; Pkt *err; @@ -457,12 +458,7 @@ Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt, struct htlc **h) 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); + *fail = tal_dup_arr(*h, u8, f->reason->info.data, f->reason->info.len,0); return NULL; } diff --git a/daemon/packets.h b/daemon/packets.h index 4f866e926..ae99395b9 100644 --- a/daemon/packets.h +++ b/daemon/packets.h @@ -44,7 +44,8 @@ Pkt *accept_pkt_open_complete(struct peer *peer, const Pkt *pkt); 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_fail(struct peer *peer, const Pkt *pkt, struct htlc **h, + u8 **fail); Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt, struct htlc **h, struct rval *r); diff --git a/daemon/peer.c b/daemon/peer.c index 2ba61d5f0..275967780 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -411,8 +411,8 @@ static void set_htlc_rval(struct peer *peer, db_htlc_fulfilled(peer, htlc); } -void set_htlc_fail(struct peer *peer, - struct htlc *htlc, const void *fail, size_t len) +static void set_htlc_fail(struct peer *peer, + struct htlc *htlc, const void *fail, size_t len) { assert(!htlc->r); assert(!htlc->fail); @@ -1112,12 +1112,28 @@ static Pkt *handle_pkt_htlc_add(struct peer *peer, const Pkt *pkt) static Pkt *handle_pkt_htlc_fail(struct peer *peer, const Pkt *pkt) { struct htlc *htlc; + u8 *fail; Pkt *err; - err = accept_pkt_htlc_fail(peer, pkt, &htlc); + err = accept_pkt_htlc_fail(peer, pkt, &htlc, &fail); if (err) return err; + /* This can happen with re-transmissions; simply note it. */ + if (htlc->fail) { + log_debug(peer->log, "HTLC %"PRIu64" failed twice", htlc->id); + htlc->fail = tal_free(htlc->fail); + } + + if (!db_start_transaction(peer)) + return pkt_err(peer, "database error"); + + set_htlc_fail(peer, htlc, fail, tal_count(fail)); + tal_free(fail); + + if (!db_commit_transaction(peer)) + return pkt_err(peer, "database error"); + cstate_fail_htlc(peer->local.staging_cstate, htlc); /* BOLT #2: @@ -2951,10 +2967,17 @@ static void check_htlc_expiry(struct peer *peer) if (height <= abs_locktime_to_blocks(&h->expiry)) continue; + if (!db_start_transaction(peer)) { + peer_fail(peer, __func__); + return; + } /* This can fail only if we're in an error state. */ - if (!command_htlc_set_fail(peer, h, - REQUEST_TIMEOUT_408, "timed out")) + command_htlc_set_fail(peer, h, + REQUEST_TIMEOUT_408, "timed out"); + if (!db_commit_transaction(peer)) { + peer_fail(peer, __func__); return; + } } /* BOLT #2: @@ -3196,8 +3219,11 @@ static const struct bitcoin_tx *irrevocably_resolved(struct peer *peer) * before we've confirmed it, this is exactly what happens. */ static void fail_own_htlc(struct peer *peer, struct htlc *htlc) { + /* We can't do anything about db failures; peer already closed. */ + db_start_transaction(peer); set_htlc_fail(peer, htlc, "peer closed", strlen("peer closed")); our_htlc_failed(peer, htlc); + db_commit_transaction(peer); } /* We've spent an HTLC output to get our funds back. There's still a @@ -4580,8 +4606,19 @@ static void json_failhtlc(struct command *cmd, return; } + if (!db_start_transaction(peer)) { + command_fail(cmd, "database error"); + return; + } + set_htlc_fail(peer, htlc, buffer + reasontok->start, reasontok->end - reasontok->start); + + if (!db_commit_transaction(peer)) { + command_fail(cmd, "database error"); + return; + } + if (command_htlc_fail(peer, htlc)) command_success(cmd, null_response(cmd)); else diff --git a/daemon/peer.h b/daemon/peer.h index 07ce1168d..5c0bc0b0f 100644 --- a/daemon/peer.h +++ b/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_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);