Write the path hash size on zero hop packets 🤖🤖 - #3293
Open
TeTeHacko wants to merge 1 commit into
Open
Conversation
`sendZeroHop()` set `packet->path_len = 0` to mean "no path segments". That byte also carries the path hash size in its upper 2 bits (Packet.h), so the assignment cleared the size as well and every zero hop packet went out declaring a 1-byte path hash, whatever the node's `path.hash.mode` was. `sendFlood()` already takes a `path_hash_size` and writes it via `setPathHashSizeAndCount()`; this makes `sendZeroHop()` do the same. Routing is unaffected either way: a zero hop packet carries no path and is never repeated (Mesh.cpp checks `getPathHashCount() > 0`). What it breaks is what the node reports about itself, since observers derive a node's hash size from the packets they hear. A node set to 2 bytes sends its flood adverts as 2-byte and its periodic local adverts as 1-byte, so it shows up inconsistently and analyzers have had to special-case zero hop adverts to work around it. Reported in meshcore-dev#2154, where the mechanism was identified but the behaviour was left as-is; also reproduced independently there across firmware 14.0-15.0 on several boards. Receivers already tolerate this: Dispatcher.cpp reads the size with `pkt->path_len >> 6` and the hop count with `pkt->path_len & 63`, and treats the legacy value 00 as 1 byte, so a zero hop packet that now declares 2 or 3 bytes parses correctly on existing firmware. Nothing in the tree compares the whole `path_len` byte against zero to detect a zero hop packet. The parameter defaults to 1, so any caller that does not pass a size keeps today's exact bits on the wire. Examples that have a `path.hash.mode` setting now pass it, matching their neighbouring `sendFlood()` calls. `BaseChatMesh::shareContactZeroHop()` deliberately keeps the default: it rebroadcasts another node's stored advert, so our own setting does not apply there, and the default reproduces the previous byte exactly. `simple_secure_chat` has no such setting at all and also keeps the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
efiten
pushed a commit
to Kpa-clawbot/CoreScope
that referenced
this pull request
Sep 2, 2026
…e route type (#1913) ## Summary `computeNodeHashSizeInfo` skips zero-hop direct adverts by **route type**. It should skip them by the **content of the path byte**, because the two cases are no longer the same thing. A zero-hop direct advert carries no path, so its hop count is 0. Whether the two size bits next to it mean anything depends on the sender: - Firmware that predates [meshcore-dev/MeshCore#3293](meshcore-dev/MeshCore#3293) does `packet->path_len = 0` in `Mesh::sendZeroHop()`, wiping the whole byte including the size bits. `0x00` genuinely says nothing about the node's `path.hash.mode` — skipping it is right, and #649 was right. - A sender that writes the size through `setPathHashSizeAndCount()` emits `0x40` (2 bytes) or `0x80` (3 bytes) with a zero hop count. On a zero-hop packet nothing else can set those bits, so they are a deliberate declaration. #653 landed the skip as `pathByte & 0x3F == 0`, which swallows the second case too. The diagnosis in #649 had actually proposed `pathByte == 0x00`; the review widened it on the reasoning that a zero hop count always implies zeroed size bits. That was true in April, when no firmware wrote them. It is not true now. On the Czech mesh (869.4 MHz), a 24h window of 10k packets holds **54 zero-hop direct adverts: 39 at `0x00` and 15 carrying a declared size** (14× `0x40`, 1× `0x80`). ## Why it matters for display, not just tidiness Measured on one node over a 7-day window. A companion was reconfigured from a 2-byte to a 3-byte path hash. Its first advert under the new setting was a zero-hop direct one on **24 Aug 15:36 UTC** declaring `0x80`. That packet was dropped, so the node kept reading as 2-byte until its next **flood** advert arrived on **25 Aug 10:18 UTC** — 18h42m serving a configuration the analyzer had already been told was stale, confirmed against both an unpatched and a patched instance. With local adverts typically every 2h and flood adverts every 25h, that gap is the normal case rather than a corner one. It bites hardest on an instance whose retention window is shorter than a flood advert interval: there the node has *no* countable advert at all and falls out of `hash_size` entirely (which is what #1912 is about on the rendering side). ## Change `(pathByte & 0x3F) == 0` → `pathByte == 0x00`, in `computeNodeHashSizeInfo` and in `computeAnalyticsHashSizes` so the two views agree. `isZeroHop` renamed to `isUndeclaredZeroHop` in the latter, since that is now what it means. No complexity change — same single byte comparison inside the existing scan. ## Measured A/B Two builds of the **same commit**, one with the change, both run read-only against the same copy of a real 181k-transmission / 973-node database: | | baseline | patched | |---|---|---| | nodes changed | — | **1** | | nodes regressed | — | **0** | | `hash_size_inconsistent` | 6 | **6** | | `multi_byte_status` split | 726 / 161 / 86 | unchanged | The flip-flop flag not moving is the point worth checking: a node that legitimately changes its mode mid-window is still handled by the recency decay from #1788, so reading these packets does not resurrect false "varies". ## Tests `cd cmd/server && go test ./...` → **ok**, 0 failures. Coverage 83.5%, unchanged from master. 5 new cases in `cmd/server/zerohop_hashsize_test.go`, two built from real off-air packets: - zero-hop DIRECT `0x40` → `HashSize 2` (was: dropped) - zero-hop DIRECT `0x80` → `HashSize 3` - zero-hop DIRECT `0x00` → still absent from the map, i.e. #649's behaviour preserved - TRANSPORT_DIRECT at path-byte offset 5, declared vs wiped - the declared size reaching `computeMultiByteCapability` as `confirmed`, which is what the map's multi-byte overlay reads **One existing test changed, flagging it explicitly:** `TestHashSizeTransportDirectZeroHopSkipped` used `0x40` as its "should be skipped" fixture. It now uses `0x00` — the case it was written to cover, since #747 was about the missing `RouteTransportDirect` skip rather than about the size bits. The `0x40` case is covered by the new tests with the opposite expectation. ## Deliberately not touched The decoders (`cmd/server/decoder.go:648`, `cmd/ingestor/decoder.go:1045`) still report `HashSize 0` for these packets, so per-packet views keep showing the size as unknown. Arguably they should follow the same rule, but that changes packet display rather than node attribution and felt like a separate call for you to make. ## Caveat worth stating This attributes a declared size to the pubkey inside the advert. That holds as long as the advert was transmitted by the node that owns it — the same assumption the existing zero-hop **flood** path already makes, so this change does not widen it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
sendZeroHop()setspacket->path_len = 0to mean "no path segments". That byte also carries the path hash size in its upper 2 bits (Packet.h), so the assignment clears the size too, and every zero hop packet goes out declaring a 1-byte path hash regardless of the node'spath.hash.mode.sendFlood()already takes apath_hash_sizeand writes it throughsetPathHashSizeAndCount(); this makessendZeroHop()do the same.Why it matters
Routing is unaffected — a zero hop packet carries no path and is never repeated (
Mesh.cppgates ongetPathHashCount() > 0). What it affects is what a node reports about itself, because observers derive a node's hash size from the packets they hear. A node configured for 2 bytes sends its flood adverts as 2-byte and its periodic local adverts as 1-byte, so it appears inconsistent and downstream tooling has had to special-case zero hop adverts.This was reported in #2154. The mechanism was identified correctly there ("
sendFloodis now correctly using the path hash mode ... howeversendZeroHop... doesn't") but the behaviour was left as is, and it was independently reproduced in that thread across firmware 14.0–15.0 on several boards. The question asked there and not answered was whether changing it would break older firmware — see the next section.Measured on the Czech community mesh, counting only packets that carry at least one hop (where the width is corroborated by the path length rather than just declared): 849 packets from 158 senders at 2 bytes, 107 from 4 senders at 3 bytes, 85 from 32 senders at 1 byte. Zero hop packets have to be excluded from that kind of census entirely, which is exactly the workaround this change removes the need for.
Backwards compatibility
Receivers already tolerate a non-zero size on a zero hop packet.
Dispatcher.cppreadsso the size is masked out of the hop count and the legacy
00is already accounted for. Nothing in the tree compares the wholepath_lenbyte against zero to detect a zero hop packet. A zero hop packet that now declares 2 or 3 bytes parses correctly on existing firmware.Scope
path_hash_sizedefaults to1, so any caller that does not pass a size produces byte-identical output to today. Examples that have apath.hash.modesetting now pass it, mirroring their adjacentsendFlood()calls. Two call sites deliberately keep the default:BaseChatMesh::shareContactZeroHop()rebroadcasts another node's stored advert blob, so this node's setting does not apply.simple_secure_chathas nopath.hash.modein itsNodePrefsat all.Note that the second
sendZeroHop()overload takesuint16_t* transport_codes, so a bare0fordelay_millisbecomes ambiguous once a fourth parameter exists; the affected call sites pass(uint32_t)0.Testing
pio test -e native -e native_kiss_modem— 48/48 pass.Heltec_v3_repeater,Heltec_v3_companion_radio_ble,Heltec_v3_room_server,RAK_4631_repeater,PicoW_repeater,wio-e5-mini_repeater, plusHeltec_t096_sensorandPicoW_terminal_chat— the last two coversimple_sensorandsimple_secure_chat, which the PR build matrix does not include but this change touches.Related to #2154.