Channled: Add CHANNEL_GOT_ANNOUNCEMENT_REPLY to make sure master stor…

…es the sigs When Channeld sends sigs to Master, it will wait for CHANNEL_GOT_ANNOUNCEMENT_REPLY to make sure master stores the sigs. After that, Channeld won't send sigs to matser in the future.
ElementsProject · May 6, 2019 · 55cca59 · 55cca59
1 parent f730a20
commit 55cca59
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - plugin: the `connected` hook can now send an `error_message` to the rejected peer.
 - Protocol: we now enforce `option_upfront_shutdown_script` if a peer negotiates it.
 - JSON API: `listforwards` now includes the local_failed forwards with failcode (Issue [#2435](https://github.com/ElementsProject/lightning/issues/2435), PR [#2524](https://github.com/ElementsProject/lightning/pull/2524))
+- DB: Store the signatures of channel announcement sent from remote peer into DB, and init channel with signatures from DB directly when reenable the channel.
+(Issue [#2409](https://github.com/ElementsProject/lightning/issues/2409))
 
 ### Changed
 

diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv
@@ -101,6 +101,9 @@ channel_got_announcement,1017
 channel_got_announcement,,remote_ann_node_sig,secp256k1_ecdsa_signature
 channel_got_announcement,,remote_ann_bitcoin_sig,secp256k1_ecdsa_signature
 
+# Wait for reply, to make sure it's on disk.(We store the announcement only once!)
+channel_got_announcement_reply,1117
+
 # When we receive funding_locked.
 channel_got_funding_locked,1019
 channel_got_funding_locked,,next_per_commit_point,struct pubkey

diff --git a/channeld/channeld.c b/channeld/channeld.c
@@ -237,6 +237,59 @@ static const u8 *hsm_req(const tal_t *ctx, const u8 *req TAKES)
 	return msg;
 }
 
+/* This queues other traffic from the fd until we get reply. */
+static u8 *wait_sync_reply(const tal_t *ctx,
+			   const u8 *msg,
+			   int replytype,
+			   int fd,
+			   struct msg_queue *queue,
+			   const char *who)
+{
+	u8 *reply;
+
+	status_trace("Sending %s %u", who, fromwire_peektype(msg));
+
+	if (!wire_sync_write(fd, msg))
+		status_failed(STATUS_FAIL_INTERNAL_ERROR,
+			      "Could not set sync write to %s: %s",
+			      who, strerror(errno));
+
+	status_trace("... , awaiting %u", replytype);
+
+	for (;;) {
+		reply = wire_sync_read(ctx, fd);
+		if (!reply)
+			status_failed(STATUS_FAIL_INTERNAL_ERROR,
+				      "Could not set sync read from %s: %s",
+				      who, strerror(errno));
+		if (fromwire_peektype(reply) == replytype) {
+			status_trace("Got it!");
+			break;
+		}
+
+		status_trace("Nope, got %u instead", fromwire_peektype(reply));
+		msg_enqueue(queue, take(reply));
+	}
+
+	return reply;
+}
+
+static u8 *master_wait_sync_reply(const tal_t *ctx,
+				  struct peer *peer, const u8 *msg,
+				  enum channel_wire_type replytype)
+{
+	return wait_sync_reply(ctx, msg, replytype,
+			       MASTER_FD, peer->from_master, "master");
+}
+
+static u8 *gossipd_wait_sync_reply(const tal_t *ctx,
+				   struct peer *peer, const u8 *msg,
+				   enum gossip_peerd_wire_type replytype)
+{
+	return wait_sync_reply(ctx, msg, replytype,
+			       GOSSIP_FD, peer->from_gossipd, "gossipd");
+}
+
 /*
  * The maximum msat that this node will accept for an htlc.
  * It's flagged as an optional field in `channel_update`.
@@ -461,6 +514,7 @@ static void announce_channel(struct peer *peer)
 
 static void channel_announcement_negotiate(struct peer *peer)
 {
+	u8 *msg;
 	/* Don't do any announcement work if we're shutting down */
 	if (peer->shutdown_sent[LOCAL])
 		return;
@@ -512,14 +566,12 @@ static void channel_announcement_negotiate(struct peer *peer)
 		 * But we won't do this when restart, because we load announcement
 		 * signatures form DB when we reenable Channeld! */
 		if(peer->remote_ann_stored == false){
-			/* Ask Master to save remote announcement into DB.
-			 * We will never waste time on waiting for remote peer announcement
-			 * reply when restart.
-			 */
-			wire_sync_write(MASTER_FD,
-					take(towire_channel_got_announcement(NULL,
-								&peer->announcement_node_sigs[REMOTE],
-								&peer->announcement_bitcoin_sigs[REMOTE])));
+			msg = towire_channel_got_announcement(NULL,
+					       &peer->announcement_node_sigs[REMOTE],
+					       &peer->announcement_bitcoin_sigs[REMOTE]);
+			master_wait_sync_reply(tmpctx, peer,
+					       take(msg),
+					       WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY);
 			peer->remote_ann_stored = true;
 		}
 		announce_channel(peer);
@@ -575,8 +627,8 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg
 	secp256k1_ecdsa_signature *remote_bitcoin_sigs;
 	struct short_channel_id *remote_scid = tal(tmpctx, struct short_channel_id);
 
-	remote_node_sigs = talz(tmpctx, secp256k1_ecdsa_signature);
-	remote_bitcoin_sigs = talz(tmpctx, secp256k1_ecdsa_signature);
+	remote_node_sigs = tal(tmpctx, secp256k1_ecdsa_signature);
+	remote_bitcoin_sigs = tal(tmpctx, secp256k1_ecdsa_signature);
 
 	if (!fromwire_announcement_signatures(msg,
 					      &chanid,
@@ -607,15 +659,26 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg
 		if (!short_channel_id_eq(remote_scid,
 				 	&peer->short_channel_ids[REMOTE]))
 			peer_failed(&peer->cs,
-					&peer->channel_id,
-			    	"Wrong announcement: "
-					"first received short_channel_ids: %s"
-			    	" , now second get %s",
-			    	type_to_string(peer, struct short_channel_id,
+				    &peer->channel_id,
+			    	    "Wrong announcement: "
+				    "first received short_channel_ids: %s"
+			    	    " , now second get %s",
+			    	    type_to_string(peer, struct short_channel_id,
 						   &peer->short_channel_ids[REMOTE]),
-			    	type_to_string(peer, struct short_channel_id,
+			    	    type_to_string(peer, struct short_channel_id,
 						   remote_scid));
 
+		/* Ask master to update the sigs(the new sigs will be send to
+		 * Master in channel_announcement_negotiate). Channeld won't
+		 * check if sigs are valid, and Gossipd will do that.
+		 */
+		if(!memcmp(&peer->announcement_node_sigs[REMOTE], remote_node_sigs,
+					  sizeof(peer->announcement_node_sigs[REMOTE])) ||
+					  !memcmp(&peer->announcement_bitcoin_sigs[REMOTE],
+					  remote_bitcoin_sigs,
+					  sizeof(peer->announcement_bitcoin_sigs[REMOTE])))
+			peer->remote_ann_stored = false;
+
 	} else {
 		peer->have_sigs[REMOTE] = true;
 
@@ -826,59 +889,6 @@ static void maybe_send_shutdown(struct peer *peer)
 	billboard_update(peer);
 }
 
-/* This queues other traffic from the fd until we get reply. */
-static u8 *wait_sync_reply(const tal_t *ctx,
-			   const u8 *msg,
-			   int replytype,
-			   int fd,
-			   struct msg_queue *queue,
-			   const char *who)
-{
-	u8 *reply;
-
-	status_trace("Sending %s %u", who, fromwire_peektype(msg));
-
-	if (!wire_sync_write(fd, msg))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Could not set sync write to %s: %s",
-			      who, strerror(errno));
-
-	status_trace("... , awaiting %u", replytype);
-
-	for (;;) {
-		reply = wire_sync_read(ctx, fd);
-		if (!reply)
-			status_failed(STATUS_FAIL_INTERNAL_ERROR,
-				      "Could not set sync read from %s: %s",
-				      who, strerror(errno));
-		if (fromwire_peektype(reply) == replytype) {
-			status_trace("Got it!");
-			break;
-		}
-
-		status_trace("Nope, got %u instead", fromwire_peektype(reply));
-		msg_enqueue(queue, take(reply));
-	}
-
-	return reply;
-}
-
-static u8 *master_wait_sync_reply(const tal_t *ctx,
-				  struct peer *peer, const u8 *msg,
-				  enum channel_wire_type replytype)
-{
-	return wait_sync_reply(ctx, msg, replytype,
-			       MASTER_FD, peer->from_master, "master");
-}
-
-static u8 *gossipd_wait_sync_reply(const tal_t *ctx,
-				   struct peer *peer, const u8 *msg,
-				   enum gossip_peerd_wire_type replytype)
-{
-	return wait_sync_reply(ctx, msg, replytype,
-			       GOSSIP_FD, peer->from_gossipd, "gossipd");
-}
-
 static u8 *foreign_channel_update(const tal_t *ctx,
 				  struct peer *peer,
 				  const struct short_channel_id *scid)
@@ -2826,6 +2836,7 @@ static void req_in(struct peer *peer, const u8 *msg)
 	case WIRE_CHANNEL_GOT_REVOKE_REPLY:
 	case WIRE_CHANNEL_GOT_FUNDING_LOCKED:
 	case WIRE_CHANNEL_GOT_ANNOUNCEMENT:
+	case WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY:
 	case WIRE_CHANNEL_GOT_SHUTDOWN:
 	case WIRE_CHANNEL_SHUTDOWN_COMPLETE:
 	case WIRE_CHANNEL_DEV_REENABLE_COMMIT_REPLY:

diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c
@@ -139,13 +139,21 @@ static void peer_got_announcement(struct channel *channel, const u8 *msg)
 		return;
 	}
 
+	/* To handle the case that Master was shut down before sends reply.
+	 * In this case, master may receive announcement more than once!
+	 */
+	tal_free(channel->remote_ann_node_sig);
+	tal_free(channel->remote_ann_bitcoin_sig);
+
 	channel->remote_ann_node_sig
 			    = tal_steal(channel, remote_ann_node_sig);
 	channel->remote_ann_bitcoin_sig
 			    = tal_steal(channel, remote_ann_bitcoin_sig);
 
 	/* save remote peer announcement signatures into DB! */
 	wallet_announcement_save(channel->peer->ld->wallet, channel);
+	subd_send_msg(channel->owner,
+		      take(towire_channel_got_announcement_reply(msg)));
 }
 
 static void peer_got_shutdown(struct channel *channel, const u8 *msg)
@@ -281,6 +289,7 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds)
 	case WIRE_CHANNEL_OFFER_HTLC_REPLY:
 	case WIRE_CHANNEL_DEV_REENABLE_COMMIT_REPLY:
 	case WIRE_CHANNEL_DEV_MEMLEAK_REPLY:
+	case WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY:
 		break;
 	}