channel: reserve a bip32 index as soon as channel is opened.

This simplifies things, and means it's always in the database. Our previous approach to creating it on the fly had holes when it was created for onchaind, causing us to use another every time we restarted. Signed-off-by: Rusty Russell <[email protected]>
vswee · Mar 7, 2018 · 6c1233d · 6c1233d
1 parent 242a934
commit 6c1233d
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 162 deletions.
diff --git a/channeld/channel.c b/channeld/channel.c
@@ -146,9 +146,12 @@ struct peer {
 	/* We save calculated commit sigs while waiting for master approval */
 	struct commit_sigs *next_commit_sigs;
 
-	/* If master told us to shut down, this contains scriptpubkey until
-	 * we're ready to send it. */
-	u8 *unsent_shutdown_scriptpubkey;
+	/* The scriptpubkey to use for shutting down. */
+	u8 *final_scriptpubkey;
+
+	/* If master told us to shut down */
+	bool send_shutdown;
+	/* Has shutdown been sent by each side? */
 	bool shutdown_sent[NUM_SIDES];
 
 	/* Information used for reestablishment. */
@@ -769,7 +772,7 @@ static void maybe_send_shutdown(struct peer *peer)
 {
 	u8 *msg;
 
-	if (!peer->unsent_shutdown_scriptpubkey)
+	if (!peer->send_shutdown)
 		return;
 
 	/* Send a disable channel_update so others don't try to route
@@ -778,11 +781,9 @@ static void maybe_send_shutdown(struct peer *peer)
 	wire_sync_write(GOSSIP_FD, msg);
 	enqueue_peer_msg(peer, take(msg));
 
-	msg = towire_shutdown(peer, &peer->channel_id,
-			      peer->unsent_shutdown_scriptpubkey);
+	msg = towire_shutdown(peer, &peer->channel_id, peer->final_scriptpubkey);
 	enqueue_peer_msg(peer, take(msg));
-	peer->unsent_shutdown_scriptpubkey
-		= tal_free(peer->unsent_shutdown_scriptpubkey);
+	peer->send_shutdown = false;
 	peer->shutdown_sent[LOCAL] = true;
 	billboard_update(peer);
 }
@@ -1589,11 +1590,24 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown)
 			    &peer->channel_id,
 			    "Bad shutdown %s", tal_hex(peer, shutdown));
 
-	/* Tell master, it will tell us what to send (if any). */
+	/* Tell master: we don't have to wait because on reconnect other end
+	 * will re-send anyway. */
 	wire_sync_write(MASTER_FD,
 			take(towire_channel_got_shutdown(peer, scriptpubkey)));
 
 	peer->shutdown_sent[REMOTE] = true;
+	/* BOLT #2:
+	 *
+	 * A receiving node:
+	 * ...
+	 * - once there are no outstanding updates on the peer, UNLESS
+	 *   it has already sent a `shutdown`:
+	 *    - MUST reply to a `shutdown` message with a `shutdown`
+	 */
+	if (!peer->shutdown_sent[LOCAL]) {
+		peer->send_shutdown = true;
+		start_commit_timer(peer);
+	}
 	billboard_update(peer);
 }
 
@@ -2377,13 +2391,11 @@ static void handle_ping_cmd(struct peer *peer, const u8 *inmsg)
 
 static void handle_shutdown_cmd(struct peer *peer, const u8 *inmsg)
 {
-	u8 *scriptpubkey;
-
-	if (!fromwire_channel_send_shutdown(peer, inmsg, &scriptpubkey))
+	if (!fromwire_channel_send_shutdown(inmsg))
 		master_badmsg(WIRE_CHANNEL_SEND_SHUTDOWN, inmsg);
 
-	/* We can't send this until commit (if any) is done, so start timer<. */
-	peer->unsent_shutdown_scriptpubkey = scriptpubkey;
+	/* We can't send this until commit (if any) is done, so start timer. */
+	peer->send_shutdown = true;
 	start_commit_timer(peer);
 }
 
@@ -2537,8 +2549,9 @@ static void init_channel(struct peer *peer)
 				   &peer->funding_locked[REMOTE],
 				   &peer->short_channel_ids[LOCAL],
 				   &reconnected,
-				   &peer->unsent_shutdown_scriptpubkey,
+				   &peer->send_shutdown,
 				   &peer->shutdown_sent[REMOTE],
+				   &peer->final_scriptpubkey,
 				   &peer->channel_flags,
 				   &funding_signed))
 		master_badmsg(WIRE_CHANNEL_INIT, msg);

diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv
@@ -55,9 +55,10 @@ channel_init,,local_funding_locked,bool
 channel_init,,remote_funding_locked,bool
 channel_init,,funding_short_id,struct short_channel_id
 channel_init,,reestablish,bool
-channel_init,,shutdown_scriptpubkey_len,u16
-channel_init,,shutdown_scriptpubkey,shutdown_scriptpubkey_len*u8
+channel_init,,send_shutdown,bool
 channel_init,,remote_shutdown_received,bool
+channel_init,,final_scriptpubkey_len,u16
+channel_init,,final_scriptpubkey,final_scriptpubkey_len*u8
 channel_init,,flags,u8
 channel_init,,init_peer_pkt_len,u16
 channel_init,,init_peer_pkt,init_peer_pkt_len*u8
@@ -164,10 +165,8 @@ channel_got_revoke,,changed,num_changed*struct changed_htlc
 # (eg. if we sent another commitment_signed, that would implicitly ack).
 channel_got_revoke_reply,1122
 
-# Tell peer that channel is shutting down
+# Tell peer to shut down channel.
 channel_send_shutdown,1023
-channel_send_shutdown,,scriptpubkey_len,u16
-channel_send_shutdown,,scriptpubkey,scriptpubkey_len*u8
 
 # Peer told us that channel is shutting down
 channel_got_shutdown,1024

diff --git a/lightningd/channel.c b/lightningd/channel.c
@@ -1,3 +1,4 @@
+#include <bitcoin/script.h>
 #include <ccan/crypto/hkdf_sha256/hkdf_sha256.h>
 #include <ccan/tal/str/str.h>
 #include <gossipd/gen_gossip_wire.h>
@@ -145,8 +146,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    const struct channel_info *channel_info,
 			    /* NULL or stolen */
 			    u8 *remote_shutdown_scriptpubkey,
-			    /* (-1 if not chosen yet) */
-			    s64 local_shutdown_idx,
+			    u64 final_key_idx,
 			    bool last_was_revoke,
 			    /* NULL or stolen */
 			    struct changed_htlc *last_sent_commit,
@@ -199,7 +199,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 	channel->channel_info = *channel_info;
 	channel->remote_shutdown_scriptpubkey
 		= tal_steal(channel, remote_shutdown_scriptpubkey);
-	channel->local_shutdown_idx = local_shutdown_idx;
+	channel->final_key_idx = final_key_idx;
 	channel->last_was_revoke = last_was_revoke;
 	channel->last_sent_commit = tal_steal(channel, last_sent_commit);
 	channel->first_blocknum = first_blocknum;
@@ -208,6 +208,11 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 	list_add_tail(&peer->channels, &channel->list);
 	tal_add_destructor(channel, destroy_channel);
 
+	/* Make sure we see any spends using this key */
+	txfilter_add_scriptpubkey(peer->ld->owned_txfilter,
+				  take(p2wpkh_for_keyidx(NULL, peer->ld,
+							 channel->final_key_idx)));
+
 	return channel;
 }
 

diff --git a/lightningd/channel.h b/lightningd/channel.h
@@ -80,8 +80,8 @@ struct channel {
 
 	/* Their scriptpubkey if they sent shutdown. */
 	u8 *remote_shutdown_scriptpubkey;
-	/* Our key for shutdown (-1 if not chosen yet) */
-	s64 local_shutdown_idx;
+	/* Address for any final outputs */
+	u64 final_key_idx;
 
 	/* Reestablishment stuff: last sent commit and revocation details. */
 	bool last_was_revoke;
@@ -122,8 +122,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    const struct channel_info *channel_info,
 			    /* NULL or stolen */
 			    u8 *remote_shutdown_scriptpubkey,
-			    /* (-1 if not chosen yet) */
-			    s64 local_shutdown_idx,
+			    u64 final_key_idx,
 			    bool last_was_revoke,
 			    /* NULL or stolen */
 			    struct changed_htlc *last_sent_commit,

diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c
@@ -76,48 +76,11 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg)
 		return;
 	}
 
-	if (channel->local_shutdown_idx == -1) {
-		u8 *scriptpubkey;
-
-		channel->local_shutdown_idx = wallet_get_newindex(ld);
-		if (channel->local_shutdown_idx == -1) {
-			channel_internal_error(channel,
-					    "Can't get local shutdown index");
-			return;
-		}
-
+	/* If we weren't already shutting down, we are now */
+	if (channel->state == CHANNELD_NORMAL)
 		channel_set_state(channel,
 				  CHANNELD_NORMAL, CHANNELD_SHUTTING_DOWN);
 
-		/* BOLT #2:
-		 *
-		 * A sending node MUST set `scriptpubkey` to one of the
-		 * following forms:
-		 *
-		 * ...3. `OP_0` `20` 20-bytes (version 0 pay to witness pubkey),
-		 */
-		scriptpubkey = p2wpkh_for_keyidx(msg, ld,
-						 channel->local_shutdown_idx);
-		if (!scriptpubkey) {
-			channel_internal_error(channel,
-					    "Can't get shutdown script %"PRIu64,
-					    channel->local_shutdown_idx);
-			return;
-		}
-
-		txfilter_add_scriptpubkey(ld->owned_txfilter, scriptpubkey);
-
-		/* BOLT #2:
-		 *
-		 * A receiving node MUST reply to a `shutdown` message with a
-		 * `shutdown` once there are no outstanding updates on the
-		 * peer, unless it has already sent a `shutdown`.
-		 */
-		subd_send_msg(channel->owner,
-			      take(towire_channel_send_shutdown(channel,
-								scriptpubkey)));
-	}
-
 	/* TODO(cdecker) Selectively save updated fields to DB */
 	wallet_channel_save(ld->wallet, channel);
 }
@@ -215,7 +178,6 @@ bool peer_start_channeld(struct channel *channel,
 	const struct failed_htlc **failed_htlcs;
 	enum side *failed_sides;
 	struct short_channel_id funding_channel_id;
-	const u8 *shutdown_scriptpubkey;
 	u64 num_revocations;
 	struct lightningd *ld = channel->peer->ld;
 	const struct config *cfg = &ld->config;
@@ -262,13 +224,6 @@ bool peer_start_channeld(struct channel *channel,
 		memset(&funding_channel_id, 0, sizeof(funding_channel_id));
 	}
 
-	if (channel->local_shutdown_idx != -1) {
-		shutdown_scriptpubkey
-			= p2wpkh_for_keyidx(tmpctx, ld,
-					    channel->local_shutdown_idx);
-	} else
-		shutdown_scriptpubkey = NULL;
-
 	num_revocations = revocations_received(&channel->their_shachain.chain);
 
 	/* Warn once. */
@@ -316,8 +271,10 @@ bool peer_start_channeld(struct channel *channel,
 				      channel->remote_funding_locked,
 				      &funding_channel_id,
 				      reconnected,
-				      shutdown_scriptpubkey,
+				      channel->state == CHANNELD_SHUTTING_DOWN,
 				      channel->remote_shutdown_scriptpubkey != NULL,
+				      p2wpkh_for_keyidx(tmpctx, ld,
+							channel->final_key_idx),
 				      channel->channel_flags,
 				      funding_signed);
 

diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c
@@ -1,3 +1,4 @@
+#include <bitcoin/script.h>
 #include <closingd/gen_closing_wire.h>
 #include <common/close_tx.h>
 #include <common/initial_commit_tx.h>
@@ -140,20 +141,15 @@ void peer_start_closingd(struct channel *channel,
 			 bool reconnected)
 {
 	const tal_t *tmpctx = tal_tmpctx(channel);
-	u8 *initmsg, *local_scriptpubkey;
+	u8 *initmsg;
 	u64 minfee, startfee, feelimit;
 	u64 num_revocations;
 	u64 funding_msatoshi, our_msatoshi, their_msatoshi;
 	struct lightningd *ld = channel->peer->ld;
 
-	if (channel->local_shutdown_idx == -1
-	    || !channel->remote_shutdown_scriptpubkey) {
+	if (!channel->remote_shutdown_scriptpubkey) {
 		channel_internal_error(channel,
-				    "Can't start closing: local %s remote %s",
-				    channel->local_shutdown_idx == -1
-				    ? "not shutdown" : "shutdown",
-				    channel->remote_shutdown_scriptpubkey
-				    ? "shutdown" : "not shutdown");
+				       "Can't start closing: no remote info");
 		tal_free(tmpctx);
 		return;
 	}
@@ -174,15 +170,6 @@ void peer_start_closingd(struct channel *channel,
 		return;
 	}
 
-	local_scriptpubkey = p2wpkh_for_keyidx(tmpctx, ld,
-					       channel->local_shutdown_idx);
-	if (!local_scriptpubkey) {
-		channel_internal_error(channel,
-				    "Can't generate local shutdown scriptpubkey");
-		tal_free(tmpctx);
-		return;
-	}
-
 	/* BOLT #2:
 	 *
 	 * A sending node MUST set `fee_satoshis` lower than or equal
@@ -227,7 +214,8 @@ void peer_start_closingd(struct channel *channel,
 				      their_msatoshi / 1000, /* Rounds down */
 				      channel->our_config.dust_limit_satoshis,
 				      minfee, feelimit, startfee,
-				      local_scriptpubkey,
+				      p2wpkh_for_keyidx(tmpctx, ld,
+							channel->final_key_idx),
 				      channel->remote_shutdown_scriptpubkey,
 				      reconnected,
 				      channel->next_index[LOCAL],

diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c
@@ -1,3 +1,4 @@
+#include <bitcoin/script.h>
 #include <common/key_derive.h>
 #include <errno.h>
 #include <inttypes.h>
@@ -343,13 +344,12 @@ enum watch_result funding_spent(struct channel *channel,
 				size_t input_num,
 				const struct block *block)
 {
-	u8 *msg, *scriptpubkey;
+	u8 *msg;
 	struct bitcoin_txid our_last_txid;
-	s64 keyindex;
-	struct pubkey ourkey;
 	struct htlc_stub *stubs;
 	const tal_t *tmpctx = tal_tmpctx(channel);
 	struct lightningd *ld = channel->peer->ld;
+	struct pubkey final_key;
 
 	channel_fail_permanent(channel, "Funding transaction spent");
 
@@ -380,35 +380,13 @@ enum watch_result funding_spent(struct channel *channel,
 		return KEEP_WATCHING;
 	}
 
-	/* We re-use this key to send other outputs to. */
-	if (channel->local_shutdown_idx >= 0)
-		keyindex = channel->local_shutdown_idx;
-	else {
-		keyindex = wallet_get_newindex(ld);
-		if (keyindex < 0) {
-			log_broken(channel->log, "Could not get keyindex");
-			tal_free(tmpctx);
-			return KEEP_WATCHING;
-		}
-	}
-	scriptpubkey = p2wpkh_for_keyidx(tmpctx, ld, keyindex);
-	if (!scriptpubkey) {
-		channel_internal_error(channel,
-				    "Can't get shutdown script %"PRIu64,
-				    keyindex);
+	if (!bip32_pubkey(ld->wallet->bip32_base, &final_key,
+			  channel->final_key_idx)) {
+		log_broken(channel->log, "Could not derive onchain key %"PRIu64,
+			   channel->final_key_idx);
 		tal_free(tmpctx);
-		return DELETE_WATCH;
-	}
-	txfilter_add_scriptpubkey(ld->owned_txfilter, scriptpubkey);
-
-	if (!bip32_pubkey(ld->wallet->bip32_base, &ourkey, keyindex)) {
-		channel_internal_error(channel,
-				    "Can't get shutdown key %"PRIu64,
-				    keyindex);
-		tal_free(tmpctx);
-		return DELETE_WATCH;
+		return KEEP_WATCHING;
 	}
-
 	/* This could be a mutual close, but it doesn't matter. */
 	bitcoin_txid(channel->last_tx, &our_last_txid);
 
@@ -429,9 +407,10 @@ enum watch_result funding_spent(struct channel *channel,
 				  channel->our_config.dust_limit_satoshis,
 				  &channel->channel_info.theirbase.revocation,
 				  &our_last_txid,
-				  scriptpubkey,
+				  p2wpkh_for_keyidx(tmpctx, ld,
+						    channel->final_key_idx),
 				  channel->remote_shutdown_scriptpubkey,
-				  &ourkey,
+				  &final_key,
 				  channel->funder,
 				  &channel->channel_info.theirbase.payment,
 				  &channel->channel_info.theirbase.htlc,