gossipd: do all malformdness checks on node_announcement before queueing.

If the channel is pending, we queue the node_announcment and if the channel
is OK we re-call process_node_announcement.  Make sure that second call
won't fail if the first succeeded.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

gossipd/routing.c

@@ -501,11 +501,18 @@ static void process_pending_node_announcement(struct routing_state *rstate,
 		return;
 
 	if (pna->node_announcement) {
+		u8 *err;
 		SUPERVERBOSE(
 		    "Processing deferred node_announcement for node %s",
 		    type_to_string(pna, struct pubkey, nodeid));
-		/* FIXME: Do something if this is invalid */
-		handle_node_announcement(rstate, pna->node_announcement);
+
+		/* Should not error, since we processed it before */
+		err = handle_node_announcement(rstate, pna->node_announcement);
+		if (err)
+			status_failed(STATUS_FAIL_INTERNAL_ERROR,
+				      "pending node_announcement %s malformed %s?",
+				      tal_hex(trc, pna->node_announcement),
+				      sanitize_error(trc, err, NULL));
 	}
 	pending_node_map_del(rstate->pending_node_map, pna);
 	tal_free(pna);
@@ -1010,6 +1017,24 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann)
 		return err;
 	}
 
+	wireaddrs = read_addresses(tmpctx, addresses);
+	if (!wireaddrs) {
+		/* BOLT #7:
+		 *
+		 * - if `addrlen` is insufficient to hold the address
+		 *  descriptors of the known types:
+		 *    - SHOULD fail the connection.
+		 */
+		u8 *err = towire_errorfmt(rstate, NULL,
+					  "Malformed wireaddrs %s in %s.",
+					  tal_hex(tmpctx, wireaddrs),
+					  tal_hex(tmpctx, node_ann));
+		tal_free(tmpctx);
+		return err;
+	}
+
+	/* Beyond this point it's not malformed, so safe if we make it
+	 * pending and requeue later. */
 	node = get_node(rstate, &node_id);
 
 	/* Check if we are currently verifying the txout for a
@@ -1051,22 +1076,6 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann)
 	status_trace("Received node_announcement for node %s",
 		     type_to_string(tmpctx, struct pubkey, &node_id));
 
-	wireaddrs = read_addresses(tmpctx, addresses);
-	if (!wireaddrs) {
-		/* BOLT #7:
-		 *
-		 * - if `addrlen` is insufficient to hold the address
-		 *  descriptors of the known types:
-		 *    - SHOULD fail the connection.
-		 */
-		u8 *err = towire_errorfmt(rstate, NULL,
-					  "Malformed wireaddrs %s in %s.",
-					  tal_hex(tmpctx, wireaddrs),
-					  tal_hex(tmpctx, node_ann));
-		tal_free(serialized);
-		tal_free(tmpctx);
-		return err;
-	}
 	tal_free(node->addresses);
 	node->addresses = tal_steal(node, wireaddrs);
 

gossipd/test/run-bench-find_route.c

@@ -83,6 +83,10 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 		       const u8 *tag UNNEEDED,
 		       const u8 *payload UNNEEDED)
 { fprintf(stderr, "replace_broadcast called!\n"); abort(); }
+/* Generated stub for sanitize_error */
+char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
+		     struct channel_id *channel_id UNNEEDED)
+{ fprintf(stderr, "sanitize_error called!\n"); abort(); }
 /* Generated stub for status_failed */
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)

gossipd/test/run-find_route-specific.c

@@ -47,6 +47,10 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 		       const u8 *tag UNNEEDED,
 		       const u8 *payload UNNEEDED)
 { fprintf(stderr, "replace_broadcast called!\n"); abort(); }
+/* Generated stub for sanitize_error */
+char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
+		     struct channel_id *channel_id UNNEEDED)
+{ fprintf(stderr, "sanitize_error called!\n"); abort(); }
 /* Generated stub for status_failed */
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)

gossipd/test/run-find_route.c

@@ -45,6 +45,10 @@ bool replace_broadcast(struct broadcast_state *bstate UNNEEDED,
 		       const u8 *tag UNNEEDED,
 		       const u8 *payload UNNEEDED)
 { fprintf(stderr, "replace_broadcast called!\n"); abort(); }
+/* Generated stub for sanitize_error */
+char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
+		     struct channel_id *channel_id UNNEEDED)
+{ fprintf(stderr, "sanitize_error called!\n"); abort(); }
 /* Generated stub for status_failed */
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)