-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
routing: check max routing info size #3841
routing: check max routing info size #3841
Conversation
13f9029
to
6df01de
Compare
0689a2c
to
9063c3d
Compare
f80d87e
to
810dfbc
Compare
0d9a287
to
c3fa681
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, only nits.
This prevents inefficient key conversions in a follow up commit that change the inner pathfinding loop.
5452295
to
6d5d415
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid fix, LGTM 👍
Itest failing, need to investigate |
To allow execution within an existing tx.
Also prepares for payload size tracking during pathfinding
6d5d415
to
790e61f
Compare
Itest fixed, db transaction problem for which I added a commit |
4a254f4
to
1a86460
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now that deadlock is fixed. Nice that we can get rid of the arbitrary limit in favor of a physical constraint! 💯
Enable the test again and use a programmatically built network.
Also the max hop count check can be removed, because the real bound is the payload size. By moving the check inside the search loop, we now also backtrack when we hit the limit.
1a86460
to
c84e57a
Compare
This PR modifies pathfinding to check the accumulated total payload size during search. This ensures that we will never find a route that exceeds the limit of 1300 bytes.
In order to execute this check, it is necessary to take into account whether nodes along the route support tlv (this influences the payload size). The size of custom records that are possibly present is added as well.