gossipd: cleanups due to feedback from cdecker.

1. make queue_peer_msg() use both if branches, as both equally likely.
2. Remove redundant *scid = NULL in handle_channel_announcement.
3. Log failing pending channel_updates.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2018-03-13 09:36:00 +10:30 committed by Christian Decker
parent 1dccbb30f9
commit 640ff4b4b9
2 changed files with 44 additions and 30 deletions

View file

@ -293,18 +293,15 @@ static void reached_peer(struct daemon *daemon, const struct pubkey *id,
static void queue_peer_msg(struct peer *peer, const u8 *msg TAKES) static void queue_peer_msg(struct peer *peer, const u8 *msg TAKES)
{ {
const u8 *send;
if (peer->local) { if (peer->local) {
msg_enqueue(&peer->local->peer_out, msg); msg_enqueue(&peer->local->peer_out, msg);
return; } else {
}
/* Use gossip_index 0 meaning don't update index */ /* Use gossip_index 0 meaning don't update index */
send = towire_gossip_send_gossip(NULL, 0, msg); const u8 *send = towire_gossip_send_gossip(NULL, 0, msg);
if (taken(msg)) if (taken(msg))
tal_free(msg); tal_free(msg);
daemon_conn_send(peer->remote, take(send)); daemon_conn_send(peer->remote, take(send));
}
} }
static void peer_error(struct peer *peer, const char *fmt, ...) static void peer_error(struct peer *peer, const char *fmt, ...)

View file

@ -627,9 +627,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
err = towire_errorfmt(rstate, NULL, err = towire_errorfmt(rstate, NULL,
"Malformed channel_announcement %s", "Malformed channel_announcement %s",
tal_hex(pending, pending->announce)); tal_hex(pending, pending->announce));
tal_free(pending); goto malformed;
*scid = NULL;
return err;
} }
/* Check if we know the channel already (no matter in what /* Check if we know the channel already (no matter in what
@ -640,9 +638,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
__func__, __func__,
type_to_string(trc, struct short_channel_id, type_to_string(trc, struct short_channel_id,
&pending->short_channel_id)); &pending->short_channel_id));
tal_free(pending); goto ignored;
*scid = NULL;
return NULL;
} }
/* We don't replace previous ones, since we might validate that and /* We don't replace previous ones, since we might validate that and
@ -652,8 +648,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
__func__, __func__,
type_to_string(trc, struct short_channel_id, type_to_string(trc, struct short_channel_id,
&pending->short_channel_id)); &pending->short_channel_id));
*scid = NULL; goto ignored;
return tal_free(pending);
} }
/* FIXME: Handle duplicates as per BOLT #7 */ /* FIXME: Handle duplicates as per BOLT #7 */
@ -668,9 +663,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
if (unsupported_features(features, NULL)) { if (unsupported_features(features, NULL)) {
status_trace("Ignoring channel announcement, unsupported features %s.", status_trace("Ignoring channel announcement, unsupported features %s.",
tal_hex(pending, features)); tal_hex(pending, features));
tal_free(pending); goto ignored;
*scid = NULL;
return NULL;
} }
/* BOLT #7: /* BOLT #7:
@ -684,9 +677,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
type_to_string(pending, struct short_channel_id, type_to_string(pending, struct short_channel_id,
&pending->short_channel_id), &pending->short_channel_id),
type_to_string(pending, struct bitcoin_blkid, &chain_hash)); type_to_string(pending, struct bitcoin_blkid, &chain_hash));
tal_free(pending); goto ignored;
*scid = NULL;
return NULL;
} }
err = check_channel_announcement(rstate, err = check_channel_announcement(rstate,
@ -707,8 +698,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
* correct: * correct:
* - SHOULD fail the connection. * - SHOULD fail the connection.
*/ */
tal_free(pending); goto malformed;
return err;
} }
status_trace("Received channel_announcement for channel %s", status_trace("Received channel_announcement for channel %s",
@ -723,8 +713,38 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
list_add_tail(&rstate->pending_cannouncement, &pending->list); list_add_tail(&rstate->pending_cannouncement, &pending->list);
tal_add_destructor2(pending, destroy_pending_cannouncement, rstate); tal_add_destructor2(pending, destroy_pending_cannouncement, rstate);
/* Success */
*scid = &pending->short_channel_id; *scid = &pending->short_channel_id;
return NULL; return NULL;
malformed:
tal_free(pending);
*scid = NULL;
return err;
ignored:
tal_free(pending);
*scid = NULL;
return NULL;
}
static void process_pending_channel_update(struct routing_state *rstate,
const struct short_channel_id *scid,
const u8 *cupdate)
{
u8 *err;
if (!cupdate)
return;
/* FIXME: We don't remember who sent us updates, so can't error them */
err = handle_channel_update(rstate, cupdate);
if (err) {
status_trace("Pending channel_update for %s: %s",
type_to_string(trc, struct short_channel_id, scid),
sanitize_error(trc, err, NULL));
tal_free(err);
}
} }
bool handle_pending_cannouncement(struct routing_state *rstate, bool handle_pending_cannouncement(struct routing_state *rstate,
@ -808,11 +828,8 @@ bool handle_pending_cannouncement(struct routing_state *rstate,
pubkey_eq(&pending->node_id_2, &rstate->local_id); pubkey_eq(&pending->node_id_2, &rstate->local_id);
/* Did we have an update waiting? If so, apply now. */ /* Did we have an update waiting? If so, apply now. */
/* FIXME: We don't remember who sent us updates, so we can't error them */ process_pending_channel_update(rstate, scid, pending->updates[0]);
if (pending->updates[0]) process_pending_channel_update(rstate, scid, pending->updates[1]);
tal_free(handle_channel_update(rstate, pending->updates[0]));
if (pending->updates[1])
tal_free(handle_channel_update(rstate, pending->updates[1]));
process_pending_node_announcement(rstate, &pending->node_id_1); process_pending_node_announcement(rstate, &pending->node_id_1);
process_pending_node_announcement(rstate, &pending->node_id_2); process_pending_node_announcement(rstate, &pending->node_id_2);