fix(hash-size): key the zero-hop advert skip on the path byte, not the route type

[TeTeHacko]

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 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 Direct/zero-hop adverts make node always appear as using 1 byte hash size #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. Direct/zero-hop adverts make node always appear as using 1 byte hash size #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.

[efiten]

Review from the queue triage (#1922). This is the one PR in the queue I had not reviewed yet, so this is written fresh today rather than posted from the backlog.

Verdict: approve. The reasoning is right and I could not fault it. Two things worth putting on the record, both of which make it safer than it looks.

The firmware premise is correct, but the PR it depends on has not shipped

The change trusts a non-zero path byte with a zero hop count as a deliberate size declaration. That behaviour comes from setPathHashSizeAndCount(), and the PR correctly attributes it to meshcore-dev/MeshCore#3293.

That firmware PR is still open, not merged. So essentially no node in the field emits such a byte yet. This is forward-looking preparation, which is fine, but it is worth stating in the PR body so nobody reads it as fixing something visible today.

Measured on a live network, the change moves exactly one packet

The only bytes where old and new behaviour differ are those whose low six bits (the hop count) are zero while the byte itself is not: 0x40, 0x80, 0xC0. Everything else is treated identically, because the old (pathByte & 0x3F) == 0 test already declined to skip a byte with a non-zero hop count.

Across 54,452 direct and transport-direct ADVERTs on a live instance:

path byte	count
0x00 (route 2)	54,441
0x00 (route 3)	1
0x40	1
0x93, 0x52	2 each
0x07, 0x09, 0x34, 0x43, 0x33, 0x8B	1 each

Only 0x40 changes behaviour: one packet in 54,452. Every other non-zero byte carries a non-zero hop count (0x93 → count 19, 0x52 → 18, 0x8B → 11, 0x43 → 3), so none of them was ever being skipped and none is newly trusted. The "what if this starts trusting noise" worry does not survive contact with the data.

Also good

Making computeAnalyticsHashSizes and computeNodeHashSizeInfo agree on the same rule is worth having on its own. The two were keyed differently and that is the kind of drift that produces two views of the same node disagreeing.

One interaction to be aware of, not a change request

Three open PRs now all move hash-size semantics: this one, #1912 (render an unobserved size as unknown rather than 1 byte) and the #1784 pathTrust chain (stop counting 1-byte prefixes as mapping evidence by default). They do not conflict textually, but merged together they change what the hash-size views report in three different ways at once. Worth a single line in the release notes covering all three rather than three separate surprises.

[efiten]

Recycling this again, and the reason is my mistake rather than anything about your PR.

Earlier today I approved the pending workflow run on this PR. That was the wrong order: approving an action_required run makes it execute against the merge commit from when the run was created, not against current master. This one was created weeks ago, so it tested a base that predates the #1923 fix and five merges that have landed since. The result it produced says nothing useful.

Closing and reopening now gets a fresh merge commit against current master, which is what the run should have been all along. No action needed from you, and apologies for the second round of noise.

[efiten]

Merging in the #1922 queue run. My review above stands: approve, and the two things I put on the record there are worth keeping in view rather than acting on now.

Short version for anyone reading later: the firmware behaviour this relies on (setPathHashSizeAndCount writing a size with a zero hop count) comes from meshcore-dev/MeshCore#3293, which is still open. So this is forward-looking preparation. Measured across 54,452 direct and transport-direct ADVERTs on a live network, the change moves exactly one packet today, because 0x40/0x80/0xC0 are the only bytes where old and new behaviour differ and only 0x40 occurs at all. Every other non-zero path byte carries a non-zero hop count and was never being skipped.

Making computeAnalyticsHashSizes and computeNodeHashSizeInfo agree on one rule is worth having on its own merits.