Implement marking node in network graph as failed

[dunxen]

I see that we still have TODOs for this functionality and I cannot find any related Issues/PRs when searching.

rust-lightning/lightning/src/routing/gossip.rs

Lines 1525 to 1532 in d8cca98

	/// Marks a node in the graph as failed.
	pub fn node_failed(&self, _node_id: &PublicKey, is_permanent: bool) {
	if is_permanent {
	// TODO: Wholly remove the node
	} else {
	// TODO: downgrade the node
	}
	}

I've already started taking a look at this. Anything I should keep in mind since the time these were originally marked TODO?

[TheBlueMatt]

I think we should just drop that method? We already handle removals, and handling temporary failures is the responsibility of the scorer, not the graph (or, is now).

[dunxen]

I think we should just drop that method? We already handle removals, and handling temporary failures is the responsibility of the scorer, not the graph (or, is now).

Ah, so this is outdated. I'll check again, but I did see some uses of the method when handling failure events.

[dunxen]

Hmm, so I'm very likely not searching well enough, but I can't find where we handle removals other than in the case where that happens with channel_failed() when the node has no channels left in the graph. And I can't find where PERM|NODE errors are actually causing removals, other than in NetworkGraph::handle_event which calls that empty method.

Other than that I can get this method removed so there's no more confusion :)

[TheBlueMatt]

Grrr, I think you're right, we need to implement the is_permanent half here, but remove the non-permanent case, I think.

[dunxen]

Grrr, I think you're right, we need to implement the is_permanent half here, but remove the non-permanent case, I think.

Well if that's the case, I do already have it implemented, must just get some tests down for it. :)

I can get a PR up.