Fix panic in router given to bogus last-hop hints

[TheBlueMatt]

See new comments and test cases for more info

Found by the router fuzz target.

[codecov]

Codecov Report

Merging #958 (7e1ba11) into main (2940099) will increase coverage by 1.49%.
The diff coverage is 100.00%.

❗ Current head 7e1ba11 differs from pull request most recent head 2825d65. Consider uploading reports for the commit 2825d65 to get more accurate results

@@            Coverage Diff             @@
##             main     #958      +/-   ##
==========================================
+ Coverage   90.59%   92.08%   +1.49%     
==========================================
  Files          60       60              
  Lines       30423    39616    +9193     
==========================================
+ Hits        27561    36480    +8919     
- Misses       2862     3136     +274     

Impacted Files	Coverage Δ
lightning/src/routing/router.rs	97.72% <100.00%> (+1.76%)	⬆️
lightning/src/ln/wire.rs	60.52% <0.00%> (-3.58%)	⬇️
lightning/src/util/macro_logger.rs	87.87% <0.00%> (-1.19%)	⬇️
lightning/src/ln/functional_test_utils.rs	94.79% <0.00%> (-0.03%)	⬇️
lightning/src/ln/reorg_tests.rs	100.00% <0.00%> (+0.37%)	⬆️
lightning/src/chain/chainmonitor.rs	96.21% <0.00%> (+0.41%)	⬆️
lightning/src/util/ser.rs	91.70% <0.00%> (+0.43%)	⬆️
lightning/src/ln/chanmon_update_fail_tests.rs	98.25% <0.00%> (+0.47%)	⬆️
lightning/src/ln/msgs.rs	88.82% <0.00%> (+0.48%)	⬆️
lightning/src/util/ser_macros.rs	97.34% <0.00%> (+0.59%)	⬆️
... and 18 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2940099...2825d65. Read the comment docs.

[ariard]

Maybe of your interest @naumenkogs :) ?

[TheBlueMatt]

If I had to guess this is actually a bug in the modifications I made to @naumenkogs' PR, not his bug, though my memory is a bit fuzzy.

[jkczyz]

Did these values need to be updated from what was used in the original test to exercise the same behavior?

[TheBlueMatt]

I don't see why they would?

[TheBlueMatt]

Squashed without diff:

$ git diff-tree -U1 7e1ba11a ee755d3b
$

[valentinewallace]

Just nits pretty much. After a bit of staring, solution makes sense to me! Double checked and tests do panic without the patch in ways that align with the comments.

[valentinewallace]

Hmm I don't know if it's clear what "value-overflowing fee" is on first read. Is there a way to rephrase/clarify? 🤔 Also, should it be s/value-overflowing/value-underflowing?

[TheBlueMatt]

No, the fee overflowed the storage space. I rephrased slightly.

[valentinewallace]

Does this mean did_add_or_update_path_to_src_node? (i.e. the "or" is implied?)

[TheBlueMatt]

I figure its implied - the block can only either add or update, but even if it did both the meaning would still hold.

[TheBlueMatt]

Very slightly tweaked comments and stray newlines, will merge after CI:

$ git diff-tree -U1 ee755d3b 2825d65c
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 9c821d7d..1d8bf268 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -2346,2 +2346,3 @@ mod tests {
 	}
+
 	#[test]
@@ -2370,3 +2371,2 @@ mod tests {
 		assert_eq!(route.paths[0][1].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly
-
 	}
@@ -2376,3 +2376,3 @@ mod tests {
 		// Previously, when we had a last-hop hint connected directly to a first-hop channel, where
-		// the last-hop had a value-overflowing fee, we'd panic.
+		// the last-hop had a fee which overflowed a u64, we'd panic.
 		// This was due to us adding the first-hop from us unconditionally, causing us to think
@@ -2380,3 +2380,3 @@ mod tests {
 		// In this test, we previously hit a subtraction underflow due to having less available
-		// liquidity at a hop than 0.
+		// liquidity at the last hop than 0.
 		assert!(do_unannounced_path_test(Some(21_000_000_0000_0000_000), 0, 21_000_000_0000_0000_000, 21_000_000_0000_0000_000).is_err());
@@ -2391,3 +2391,2 @@ mod tests {
 
-
 	#[test]