Refactor loop detection and path validation logic with fixes#146
Draft
agessaman wants to merge 2 commits intorightup:feat/companionfrom
Draft
Refactor loop detection and path validation logic with fixes#146agessaman wants to merge 2 commits intorightup:feat/companionfrom
agessaman wants to merge 2 commits intorightup:feat/companionfrom
Conversation
- Introduced new LOOP_DETECT_THRESHOLDS structure for multi-byte hash handling in loop detection. - Enhanced path validation to check for encoded path length consistency and enforce flood hop limits based on configuration. - Updated flood forwarding logic to ensure proper handling of path lengths and hash sizes. - Added tests for firmware parity and path validation scenarios, ensuring robustness in loop detection and path processing.
…match wire out_path_len is the encoded path-length byte (wire format), not the path byte count. The code was slicing with out_path[: out_path_len], so the slice length was the raw encoded value (e.g. 0x41). When out_path was shorter than that (e.g. 2 bytes) but out_path_len was 0x41 (meaning 4 path bytes), the packet had path_len=0x41 and only 2 path bytes, so serialization produced a truncated path on the wire. Fix: use PathUtils.get_path_byte_len() for the slice length, cap to len(out_path) when truncated, and set path_len via PathUtils.encode_path_len() so the encoded length matches the number of path bytes actually sent. Applied in room_server.py (push to client) and text.py (CLI reply).
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
1 participant
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.
I think these are areas that need updates, but whether these are built properly I'm not 100% sure.
The loop detection logic had some limitations around multi-byte paths, like my repeater's prefix 010101 would trigger loop detection if the preceding or following multi-byte path hash included an 01 because it interpreted it as single bytes so xx01,010101 was a loop.
refactor: update loop detection and path validation logic was an attempt to match the logic in the firmware that considers the path hash length when identifying loops.
The direct-routing path length fix seems like an encoding problem.