From 4319f3ac70aea9022eb58cd2bd348f37d447cb22 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Aug 2016 14:23:46 +0930 Subject: [PATCH] peer: explicitly store the previous revocation hash when sending new update. We want to stop keeping old commitment information (except the minimal txid to commitment-number mapping). One place we currently use it is after sending a commitment signature, and before we've received the revocation for the old commitment. For this duration, there are two valid commitment transactions. So we store "their_prev_revocation_hash" explicitly for this duration. Signed-off-by: Rusty Russell --- daemon/packets.c | 26 ++++---- daemon/packets.h | 3 +- daemon/peer.c | 164 +++++++++++++++++++++++++++-------------------- daemon/peer.h | 5 +- 4 files changed, 114 insertions(+), 84 deletions(-) diff --git a/daemon/packets.c b/daemon/packets.c index 1274b8da4..0fffd25ac 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -200,8 +200,7 @@ void queue_pkt_revocation(struct peer *peer, update_revocation__init(u); - u->revocation_preimage - = sha256_to_proto(u, peer->local.commit->prev->revocation_preimage); + u->revocation_preimage = sha256_to_proto(u, preimage); u->next_revocation_hash = sha256_to_proto(u, &peer->local.next_revocation_hash); @@ -468,15 +467,13 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt, return NULL; } -Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt, - struct commit_info *ci) +Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt) { const UpdateRevocation *r = pkt->update_revocation; - struct sha256 h; + struct sha256 h, preimage; - assert(!ci->revocation_preimage); - ci->revocation_preimage = tal(ci, struct sha256); - proto_to_sha256(r->revocation_preimage, ci->revocation_preimage); + assert(peer->their_prev_revocation_hash); + proto_to_sha256(r->revocation_preimage, &preimage); /* BOLT #2: * @@ -484,16 +481,21 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt, * SHA256 hash of `revocation_preimage` matches the previous commitment * transaction, and MUST fail if it does not. */ - sha256(&h, ci->revocation_preimage, sizeof(*ci->revocation_preimage)); - if (!structeq(&h, &ci->revocation_hash)) + sha256(&h, &preimage, sizeof(preimage)); + if (!structeq(&h, peer->their_prev_revocation_hash)) return pkt_err(peer, "complete preimage incorrect"); // save revocation preimages in shachain if (!shachain_add_hash(&peer->their_preimages, - 0xFFFFFFFFFFFFFFFFL - ci->commit_num, - ci->revocation_preimage)) + 0xFFFFFFFFFFFFFFFFL + - (peer->remote.commit->commit_num - 1), + &preimage)) return pkt_err(peer, "preimage not next in shachain"); + /* Clear the previous revocation hash. */ + peer->their_prev_revocation_hash + = tal_free(peer->their_prev_revocation_hash); + /* Save next revocation hash. */ proto_to_sha256(r->next_revocation_hash, &peer->remote.next_revocation_hash); diff --git a/daemon/packets.h b/daemon/packets.h index 140325839..e8add8384 100644 --- a/daemon/packets.h +++ b/daemon/packets.h @@ -51,8 +51,7 @@ Pkt *accept_pkt_update_accept(struct peer *peer, const Pkt *pkt); Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt, struct bitcoin_signature *sig); -Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt, - struct commit_info *ci); +Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt); Pkt *accept_pkt_close_clearing(struct peer *peer, const Pkt *pkt); diff --git a/daemon/peer.c b/daemon/peer.c index c3eb73ddd..b1cc5c359 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -740,6 +740,7 @@ static bool closing_pkt_in(struct peer *peer, const Pkt *pkt) static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) { Pkt *err; + struct sha256 preimage; struct commit_info *ci = new_commit_info(peer); /* FIXME: We can actually merge these two... */ static const struct state_table commit_changes[] = { @@ -805,27 +806,23 @@ static Pkt *handle_pkt_commit(struct peer *peer, const Pkt *pkt) peer->their_commitsigs++; /* Now, send the revocation. */ - ci = peer->local.commit->prev; - assert(ci); - assert(!ci->revocation_preimage); /* We have their signature on the current one, right? */ assert(peer->local.commit->sig); + assert(peer->local.commit->commit_num > 0); if (!htlcs_changestate(peer, revocation_changes, ARRAY_SIZE(revocation_changes))) fatal("sent revoke with no changes"); - ci->revocation_preimage = tal(ci, struct sha256); - peer_get_revocation_preimage(peer, ci->commit_num, - ci->revocation_preimage); + peer_get_revocation_preimage(peer, peer->local.commit->commit_num - 1, + &preimage); /* Fire off timer if this ack caused new changes */ if (peer_uncommitted_changes(peer)) remote_changes_pending(peer); - queue_pkt_revocation(peer, ci->revocation_preimage, - &peer->local.next_revocation_hash); + queue_pkt_revocation(peer, &preimage, &peer->local.next_revocation_hash); return NULL; } @@ -899,7 +896,6 @@ static Pkt *handle_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt) static Pkt *handle_pkt_revocation(struct peer *peer, const Pkt *pkt) { - struct commit_info *ci = peer->remote.commit->prev; Pkt *err; static const struct state_table changes[] = { { SENT_ADD_COMMIT, RCVD_ADD_REVOCATION }, @@ -908,7 +904,7 @@ static Pkt *handle_pkt_revocation(struct peer *peer, const Pkt *pkt) { SENT_REMOVE_COMMIT, RCVD_REMOVE_REVOCATION } }; - err = accept_pkt_revocation(peer, pkt, ci); + err = accept_pkt_revocation(peer, pkt); if (err) return err; @@ -1546,6 +1542,11 @@ static void do_commit(struct peer *peer, struct command *jsoncmd) peer->commit_jsoncmd = jsoncmd; ci = new_commit_info(peer); + assert(!peer->their_prev_revocation_hash); + peer->their_prev_revocation_hash + = tal_dup(peer, struct sha256, + &peer->remote.commit->revocation_hash); + /* BOLT #2: * * A node MUST NOT send an `update_commit` message which does @@ -1653,6 +1654,7 @@ static struct peer *new_peer(struct lightningd_state *dstate, peer->closing_onchain.ci = NULL; peer->commit_timer = NULL; peer->nc = NULL; + peer->their_prev_revocation_hash = NULL; /* Make it different from other node (to catch bugs!), but a * round number for simple eyeballing. */ peer->htlc_id_counter = pseudorand(1ULL << 32) * 1000; @@ -2632,11 +2634,22 @@ static void resolve_our_unilateral(struct peer *peer, * Similarly, when node A sees a *commitment tx* from B: */ static void resolve_their_unilateral(struct peer *peer, - const struct bitcoin_tx *tx) + const struct bitcoin_tx *tx, + const struct sha256_double *txid, + struct htlc **htlcs, + const u8 **wscripts, + int to_us_idx, int to_them_idx) { - const struct commit_info *ci = peer->closing_onchain.ci; + const struct commit_info *ci; size_t num_ours, num_theirs; + /* FIXME: Remove closing_onchain.ci */ + if (structeq(&peer->remote.commit->txid, txid)) + peer->closing_onchain.ci = peer->remote.commit; + else + peer->closing_onchain.ci = peer->remote.commit->prev; + + ci = peer->closing_onchain.ci; peer->closing_onchain.resolved = tal_arrz(tx, const struct bitcoin_tx *, tal_count(ci->map)); @@ -2761,44 +2774,20 @@ static bool find_their_old_tx(struct peer *peer, return false; } -static bool resolve_their_steal(struct peer *peer, - const struct bitcoin_tx *tx) +static void resolve_their_steal(struct peer *peer, + const struct bitcoin_tx *tx, + const struct sha256_double *txid, + const struct sha256 *revocation_preimage, + struct htlc **htlcs, + const u8 **wscripts, + int to_us_idx, int to_them_idx) { - u64 commit_num; - struct sha256_double txid; - struct sha256 revocation_preimage, rhash; - int to_us_idx, to_them_idx, i, n; - struct htlc **htlcs; - const u8 **wscripts; + int i, n; struct bitcoin_tx *steal_tx; size_t wsize = 0; u64 input_total = 0, fee; - bitcoin_txid(tx, &txid); - if (!find_their_old_tx(peer, &txid, &commit_num)) { - log_broken_struct(peer->log, "Could not find revoked txid %s", - struct sha256_double, &txid); - return false; - } - - if (!shachain_get_hash(&peer->their_preimages, - 0xFFFFFFFFFFFFFFFFL - commit_num, - &revocation_preimage)) { - log_broken(peer->log, "Could not get shachain for %"PRIu64, - commit_num); - return false; - } - sha256(&rhash, &revocation_preimage, sizeof(revocation_preimage)); - - htlcs = tx_map_outputs(peer, &rhash, tx, REMOTE, commit_num, - &to_us_idx, &to_them_idx, &wscripts); - if (!htlcs) { - log_broken(peer->log, "Could not map_htlcs for %"PRIu64, - commit_num); - return false; - } - - /* FIXME: Caller should allocate this. */ + /* FIXME: Caller should allocate this. */ peer->closing_onchain.resolved = tal_arr(tx, const struct bitcoin_tx *, tx->output_count); @@ -2836,7 +2825,7 @@ static bool resolve_their_steal(struct peer *peer, peer->closing_onchain.resolved[i] = steal_tx; /* Connect it up. */ - steal_tx->input[n].txid = txid; + steal_tx->input[n].txid = *txid; steal_tx->input[n].index = i; steal_tx->input[n].amount = tal_dup(steal_tx, u64, &tx->output[i].amount); @@ -2858,7 +2847,7 @@ static bool resolve_their_steal(struct peer *peer, for (i = 0; i < tal_count(peer->closing_onchain.resolved); i++) peer->closing_onchain.resolved[i] = tx; tal_free(steal_tx); - return true; + return; } steal_tx->output[0].amount = input_total - fee; steal_tx->output[0].script = scriptpubkey_p2sh(steal_tx, @@ -2877,14 +2866,38 @@ static bool resolve_their_steal(struct peer *peer, steal_tx->input[i].witness = bitcoin_witness_secret(steal_tx, peer->dstate->secpctx, - &revocation_preimage, - sizeof(revocation_preimage), + revocation_preimage, + sizeof(*revocation_preimage), &sig, wscripts[i]); } broadcast_tx(peer, steal_tx); - return true; +} + +static struct sha256 *get_rhash(struct peer *peer, u64 commit_num, + struct sha256 *rhash) +{ + struct sha256 preimage; + + /* Previous revoked tx? */ + if (shachain_get_hash(&peer->their_preimages, + 0xFFFFFFFFFFFFFFFFL - commit_num, + &preimage)) { + sha256(rhash, &preimage, sizeof(preimage)); + return tal_dup(peer, struct sha256, &preimage); + } + + /* Current tx? */ + if (commit_num == peer->remote.commit->commit_num) { + *rhash = peer->remote.commit->revocation_hash; + return NULL; + } + + /* Last tx, but we haven't got revoke for it yet? */ + assert(commit_num == peer->remote.commit->commit_num-1); + *rhash = *peer->their_prev_revocation_hash; + return NULL; } /* We assume the tx is valid! Don't do a blockchain.info and feed this @@ -2899,6 +2912,7 @@ static enum watch_result anchor_spent(struct peer *peer, enum state newstate; struct htlc_map_iter it; struct htlc *h; + u64 commit_num; assert(input_num < tx->input_count); @@ -2935,26 +2949,40 @@ static enum watch_result anchor_spent(struct peer *peer, err = pkt_err(peer, "Our own unilateral close tx seen"); peer->closing_onchain.ci = peer->local.commit; resolve_our_unilateral(peer, tx); - /* Their unilateral */ - } else if (structeq(&peer->remote.commit->txid, &txid)) { - newstate = STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL; - peer->closing_onchain.ci = peer->remote.commit; - err = pkt_err(peer, "Unilateral close tx seen"); - resolve_their_unilateral(peer, tx); - /* Their previous, unrevoked unilateral */ - } else if (peer->remote.commit->prev - && !peer->remote.commit->prev->revocation_preimage - && structeq(&peer->remote.commit->prev->txid, &txid)) { - newstate = STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL; - peer->closing_onchain.ci = peer->remote.commit->prev; - err = pkt_err(peer, "Unilateral close tx seen"); - resolve_their_unilateral(peer, tx); - } else if (resolve_their_steal(peer, tx)) { - newstate = STATE_CLOSE_ONCHAIN_CHEATED; - err = pkt_err(peer, "Revoked transaction seen"); + /* Must be their unilateral */ + } else if (find_their_old_tx(peer, &txid, &commit_num)) { + struct sha256 *preimage, rhash; + int to_us_idx, to_them_idx; + struct htlc **htlcs; + const u8 **wscripts; + + preimage = get_rhash(peer, commit_num, &rhash); + htlcs = tx_map_outputs(peer, &rhash, tx, REMOTE, commit_num, + &to_us_idx, &to_them_idx, &wscripts); + if (!htlcs) { + /* Should not happen */ + log_broken(peer->log, + "Can't resolve known anchor spend %"PRIu64"!", + commit_num); + goto unknown_spend; + } + if (preimage) { + newstate = STATE_CLOSE_ONCHAIN_CHEATED; + err = pkt_err(peer, "Revoked transaction seen"); + resolve_their_steal(peer, tx, &txid, preimage, + htlcs, wscripts, + to_us_idx, to_them_idx); + } else { + newstate = STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL; + err = pkt_err(peer, "Unilateral close tx seen"); + resolve_their_unilateral(peer, tx, &txid, + htlcs, wscripts, + to_us_idx, to_them_idx); + } } else { /* FIXME: Log harder! */ - log_broken(peer->log, "Unknown anchor spend! Funds may be lost!"); + log_broken(peer->log, + "Unknown anchor spend! Funds may be lost!"); goto unknown_spend; } diff --git a/daemon/peer.h b/daemon/peer.h index 3ee12174c..313375d45 100644 --- a/daemon/peer.h +++ b/daemon/peer.h @@ -54,8 +54,6 @@ struct commit_info { struct bitcoin_signature *sig; /* Map for permutation: see commit_tx.c */ int *map; - /* Revocation preimage (if known). */ - struct sha256 *revocation_preimage; }; struct peer_visible_state { @@ -202,6 +200,9 @@ struct peer { /* Stuff we have in common. */ struct peer_visible_state local, remote; + /* If we have sent a new commit tx, but not received their revocation */ + struct sha256 *their_prev_revocation_hash; + /* this is where we will store their revocation preimages*/ struct shachain their_preimages; };