From: Matt Corallo <git@bluematt.me>
Date: Thu, 19 Jan 2023 18:24:30 +0000 (+0000)
Subject: Update min-inbound-fee values on `NetworkGraph` load
X-Git-Tag: v0.0.114-beta~45^2~5
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=efdd2217b78b1b5b3cbbcb8f4bce26f1c6a0cf87;p=rust-lightning

Update min-inbound-fee values on `NetworkGraph` load

Historically we've had various bugs in keeping the
`lowest_inbound_channel_fees` field in `NodeInfo` up-to-date as we
go. This leaves the A* routing less efficient as it can't prune
hops as aggressively.

In order to get accurate benchmarks, this commit updates the
minimum-inbound-fees field on load. This is not the most efficient
way of doing so, but suffices for fetching benchmarks and will be
removed in the coming commits.

Note that this is *slower* than the non-updating version in the
previous commit. While I haven't dug into this incredibly deeply,
the graph snapshot in use has min-fee info for only 9,618 of
20,818 nodes. Thus, it is my guess that with the graph snapshot
as-is the branch predictor is able to largely remove the A*
heuristic lookups, but with this change it is forced to wait for
A* heuristic map lookups to complete, causing a performance
regression.

```
test routing::router::benches::generate_mpp_routes_with_probabilistic_scorer ... bench: 182,980,059 ns/iter (+/- 32,662,047)
test routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer  ... bench: 151,170,457 ns/iter (+/- 75,351,011)
test routing::router::benches::generate_routes_with_probabilistic_scorer     ... bench:  58,187,277 ns/iter (+/- 11,606,440)
test routing::router::benches::generate_routes_with_zero_penalty_scorer      ... bench:  41,210,193 ns/iter (+/- 18,103,320)
```
---

diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
index 1a5b97850..065472aa3 100644
--- a/lightning/src/routing/gossip.rs
+++ b/lightning/src/routing/gossip.rs
@@ -1156,14 +1156,14 @@ impl<L: Deref> ReadableArgs<L> for NetworkGraph<L> where L::Target: Logger {
 
 		let genesis_hash: BlockHash = Readable::read(reader)?;
 		let channels_count: u64 = Readable::read(reader)?;
-		let mut channels = BTreeMap::new();
+		let mut channels: BTreeMap<u64, ChannelInfo> = BTreeMap::new();
 		for _ in 0..channels_count {
 			let chan_id: u64 = Readable::read(reader)?;
 			let chan_info = Readable::read(reader)?;
 			channels.insert(chan_id, chan_info);
 		}
 		let nodes_count: u64 = Readable::read(reader)?;
-		let mut nodes = BTreeMap::new();
+		let mut nodes: BTreeMap<NodeId, NodeInfo> = BTreeMap::new();
 		for _ in 0..nodes_count {
 			let node_id = Readable::read(reader)?;
 			let node_info = Readable::read(reader)?;
@@ -1175,6 +1175,22 @@ impl<L: Deref> ReadableArgs<L> for NetworkGraph<L> where L::Target: Logger {
 			(1, last_rapid_gossip_sync_timestamp, option),
 		});
 
+		// Regenerate inbound fees for all channels. The live-updating of these has been broken in
+		// various ways historically, so this ensures that we have up-to-date limits.
+		for (node_id, node) in nodes.iter_mut() {
+			let mut best_fees = RoutingFees { base_msat: u32::MAX, proportional_millionths: u32::MAX };
+			for channel in node.channels.iter() {
+				if let Some(chan) = channels.get(channel) {
+					let dir_opt = if *node_id == chan.node_one { &chan.two_to_one } else { &chan.one_to_two };
+					if let Some(dir) = dir_opt {
+						best_fees.base_msat = cmp::min(best_fees.base_msat, dir.fees.base_msat);
+						best_fees.proportional_millionths = cmp::min(best_fees.proportional_millionths, dir.fees.proportional_millionths);
+					}
+				} else { return Err(DecodeError::InvalidValue); }
+			}
+			node.lowest_inbound_channel_fees = Some(best_fees);
+		}
+
 		Ok(NetworkGraph {
 			secp_ctx: Secp256k1::verification_only(),
 			genesis_hash,