Fix debug panic in the case where a first hop has a channel with an introduction node#2503
Merged
tnull merged 1 commit intolightningdevkit:mainfrom Aug 25, 2023
Conversation
TheBlueMatt
previously approved these changes
Aug 17, 2023
Collaborator
|
Fuzzer disagrees with you, it definitely knows how to get this :). |
Contributor
Author
|
Well you're running the main fuzzers, so I won't know if it gets it unless you tell me lol. Lmk if it got anything else. |
Collaborator
|
I mean you can run it too, there's nothing special about me :) |
valentinewallace
force-pushed
the
2023-08-fix-router-debug-panic
branch
from
c305cda to
24b11ff
Compare
Contributor
Author
|
Added another fix and test for this failing fuzz input: https://pastebin.com/GtCUpwEd |
This previously led to a debug panic in the router because we wouldn't account for the blinded path fee when calculating first_hop<>intro_node hop's available liquidity and construct an invalid path that forwarded more over said hop than was actually available. This also led to us hitting unreachable code, see direct_to_matching_intro_nodes test description.
valentinewallace
force-pushed
the
2023-08-fix-router-debug-panic
branch
from
24b11ff to
c9f5a75
Compare
TheBlueMatt
approved these changes
Aug 24, 2023
tnull
approved these changes
Aug 25, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, we wouldn't account for the blinded path fee when calculating a
first_hop<>intro_nodehop's available liquidity, and construct an invalid path that forwarded more over said hop than was actually available, leading to a debug panic.I'm not sure why the fuzzer didn't catch this. I don't see anything stopping it from generating routes where we have a direct channel between a first hop and an intro node? But I also don't have the strongest understanding of the art of our fuzzing framework. Seems like a candidate for the post-BOLT12 "let's focus on paying down tech debt" quarter.<- the fuzzer did catch it actually