fix: zerohop also zeros hop_start to prevent traceroute ghost hops (#46)#47
Open
eric-becker wants to merge 6 commits into
Open
fix: zerohop also zeros hop_start to prevent traceroute ghost hops (#46)#47eric-becker wants to merge 6 commits into
eric-becker wants to merge 6 commits into
Conversation
Receiving firmware computes hopsTaken = hop_start - hop_limit and pads RouteDiscovery.route[] with 0xFFFFFFFF sentinels for the difference, which apps render as 'Meshtastic ffff (ffff)' ghost hops in traceroutes. Setting hop_start = 0 hits a documented special case in getHopsAway (returns -1 'unknown'), so the padding loop never fires. Refs #46 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mirrors the zerohop_protobuf fix for JSON-encoded packets. Consumers computing hops-taken from hop_start (or hop_start - hops_away) would otherwise see a non-zero value and could render the same ghost hops that motivated the protobuf fix. Refs #46
The synthetic envelope round-trip already verified hop_limit was zeroed; extend it to also verify hop_start ends at 0, and pass an explicit hop_start=3 in construction so the assertion has bite. Refs #46
End-to-end verification through ExHook + EMQX that floodgate zeros hop_start alongside hop_limit. Local Docker verification is constrained on this host (k3s networking); CI is authoritative. Refs #46
Updates the lead description, before/after examples, config table entry, and JSON-mirror note to reflect that both hop_limit and hop_start are now zeroed. Per-message log examples are unchanged because they show original pre-modification sender values for observability. Refs #46
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.
Summary
Closes #46.
Floodgate previously set only
MeshPacket.hop_limit = 0while leavinghop_startuntouched. Receiving Meshtastic firmware computeshopsTaken = hop_start - hop_limitand padsRouteDiscovery.route[]with0xFFFFFFFFsentinels for the difference, which the phone apps render asMeshtastic ffff (ffff)ghost hops in traceroutes (6 entries between the real endpoints, hop count inflated to ~7).This PR also zeros
hop_start(and JSONhops_away). Settinghop_start = 0triggers a documented special case in firmware (NodeDB.cpp:1684):getHopsAwayreturns-1("unknown"), andinsertUnknownHops(TraceRouteModule.cpp:362) guards onhopsTaken >= 0so the padding loop doesn't fire. Rebroadcast suppression behaviour is unchanged (NextHopRouter.cpp:144 still gates onhop_limit > 0only).Per-message log records continue to show the original sender values for observability — metadata is extracted before mutation.
Changes
src/floodgate/zerohop.py— zerohop_startinzerohop_protobuf; zerohop_startandhops_away(when present) inzerohop_json.tests/test_zerohop.py— newTestZerohopProtobufHopFieldsclass; new JSON parity tests; tightened existing synthetic round-trip to asserthop_start == 0.tests/test_portnum.py— backward-compatible:_build_encrypted_envelopegains optionalhop_startparam (defaultNone, no behaviour change for existing callers).tests/integration/test-driver/run.py—case_zerohopnow asserts deliveredhop_start == 0end-to-end through ExHook + EMQX.README.md,src/floodgate/__main__.py— describe both fields are zeroed.Test plan
pytest tests/ --ignore=tests/test_container_smoke.py -q./scripts/run-integration.sh) — verified in CI. (Local Docker is constrained on the maintainer's host due to k3s networking.)Meshtastic ffff (ffff)ghost hops in the phone apps. To verify post-merge on the production deployment using the screenshots in fix: zerohop inflates traceroute hop count and shows 'Meshtastic ffff (ffff)' ghost hops #46 as the baseline.🤖 Generated with Claude Code