From 73d0fc98884e1c49cca1531a3e3e67ccd6a2f3f4 Mon Sep 17 00:00:00 2001 From: Strycher Date: Sun, 19 Jul 2026 22:49:05 -0400 Subject: [PATCH] fix(#309): decode and encode path_len per firmware, not as a byte count The firmware path-len byte is packed and self-describing (src/Packet.h:79-84): high 2 bits = hash size - 1, low 6 bits = hash COUNT (hops), byte length = count * size. The app treated the low 6 bits as a byte count and ignored the width, on the strength of a comment asserting the high bits were "not reliably populated". Both halves of that comment were wrong. Receive: Contact.fromFrame read `count` bytes where firmware meant `count` hops, so at 2-byte width it kept HALF of every path and discarded the rest. DAYTON VA's 2-hop route became a 2-byte fragment, which is why every repeater login in Bandit's capture timed out. Send: buildUpdateContactPathFrame wrote the count raw, leaving mode bits 00. That tells the radio "1-byte hashes" while handing it 2-byte hash data, so it routed to nodes never on the route. This is the likelier cause of the observed "routes via c6 and 5c separately" behaviour. Also fixes a third unit inconsistency: setContactPath wrote customPath.length (bytes) into pathLength (hops) after every path set. Contact now carries pathHashWidth per path, so a stored path can never be re-sliced at a width it was not captured at. realHopCount() is removed rather than adjusted: it divided an already-correct hop count, halving it at 2-byte width, and every call site was compensating for a bug that no longer exists. Migration (Ben's call, option A): records written before this change have no pathHashWidth and hold truncated bytes that cannot be recovered by reinterpretation, so their path is dropped and the contact reverts to flood until the radio re-supplies it. Logged, not silent. Tests asserted the old semantics (expect(realHopCount(2, 2), 1)) and were rewritten against firmware truth rather than adjusted to keep passing. Added send-side encoding coverage, which did not exist. 456 tests pass. Refs #240, #279, #299 Co-Authored-By: Claude Opus 4.8 --- lib/connector/meshcore_connector.dart | 19 +++- lib/connector/meshcore_protocol.dart | 70 ++++++++++---- lib/models/contact.dart | 51 +++++++--- lib/screens/channel_chat_screen.dart | 5 +- lib/screens/channel_message_path_screen.dart | 9 +- lib/screens/chat_screen.dart | 12 +-- lib/storage/contact_store.dart | 23 ++++- .../build_update_contact_path_frame_test.dart | 53 +++++++++++ test/connector/path_hash_test.dart | 95 +++++++++++++++---- test/models/contact_pathlen_test.dart | 39 ++++++-- 10 files changed, 298 insertions(+), 78 deletions(-) diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 90fddc8..596462a 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -3192,11 +3192,15 @@ class MeshCoreConnector extends ChangeNotifier { } } + /// Pushes a path to the device. [hopCount] is a HOP count and [hashWidth] the + /// bytes per hop hash; both are needed because the wire path_len byte packs + /// them together, and sending a bare count mislabels the width. (#309) Future setContactPath( Contact contact, Uint8List customPath, - int pathLen, - ) async { + int hopCount, { + int hashWidth = 1, + }) async { // Serialize path operations to prevent interleaved async calls from // leaving in-memory state inconsistent with the device. final prev = _pathOpLock; @@ -3210,7 +3214,8 @@ class MeshCoreConnector extends ChangeNotifier { buildUpdateContactPathFrame( contact.publicKey, customPath, - pathLen, + hopCount, + hashWidth: hashWidth, type: contact.type, flags: contact.flags, name: contact.name, @@ -3225,8 +3230,12 @@ class MeshCoreConnector extends ChangeNotifier { (c) => c.publicKeyHex == contact.publicKeyHex, ); if (idx != -1) { + // pathLength is a HOP count. This wrote customPath.length (a BYTE + // count), which silently redefined the field's unit after any path + // set and doubled it at 2-byte width. (#309) _contacts[idx] = _contacts[idx].copyWith( - pathLength: customPath.length, + pathLength: hopCount, + pathHashWidth: hashWidth, path: customPath, ); notifyListeners(); @@ -3272,6 +3281,7 @@ class MeshCoreConnector extends ChangeNotifier { latestContact.publicKey, latestContact.path, latestContact.pathLength, + hashWidth: latestContact.pathHashWidth, type: latestContact.type, flags: updatedFlags, name: latestContact.name, @@ -3615,6 +3625,7 @@ class MeshCoreConnector extends ChangeNotifier { contact.publicKey, contact.path, contact.pathLength, + hashWidth: contact.pathHashWidth, type: contact.type, flags: contact.flags, name: contact.name, diff --git a/lib/connector/meshcore_protocol.dart b/lib/connector/meshcore_protocol.dart index 13e3a05..516af02 100644 --- a/lib/connector/meshcore_protocol.dart +++ b/lib/connector/meshcore_protocol.dart @@ -514,24 +514,50 @@ int readInt32LE(Uint8List data, int offset) { return val; } -// Path-length byte from the firmware. Low 6 bits = the path length field -// (a BYTE count of the hop-hash array, 0-63); high 2 bits carry an optional -// hash-size mode hint (0..2 -> 1..3 bytes/hop) that is not reliably populated, -// so the device's configured hash width (MeshCoreConnector.pathHashByteWidth) -// is authoritative for slicing the path. realHopCount() converts the byte -// length to a true hop count at that width. (#112) -// TX counterpart: buildSetPathHashModeFrame (CMD_SET_PATH_HASH_MODE). +// Path-length byte from the firmware. This is a PACKED field, and both halves +// are authoritative — the path is self-describing on the wire: +// +// high 2 bits = hash size - 1 (0..2 -> 1..3 bytes per hop hash) +// low 6 bits = hash COUNT (the number of HOPS, 0-63) +// byte length = count * size +// +// Verified against firmware `src/Packet.h:79-84`: +// getPathHashSize() == (path_len >> 6) + 1 +// getPathHashCount() == path_len & 63 +// getPathByteLen() == getPathHashCount() * getPathHashSize() +// setPathHashSizeAndCount(sz, n) { path_len = ((sz - 1) << 6) | (n & 63); } +// +// The high bits are set deliberately by that setter, so the per-path width +// travels with the path. The companion contact frame carries this same encoded +// byte verbatim: `Packet::copyPath()` returns path_len unchanged +// (`src/Packet.cpp:32-35`) into `ContactInfo.out_path_len` +// (`src/helpers/BaseChatMesh.cpp:319`), which `writeContactRespFrame` emits +// as-is (`examples/companion_radio/MyMesh.cpp:205-212`). +// +// A prior comment here claimed the low 6 bits were a BYTE count and that the +// high bits were "not reliably populated". Both were wrong, and #222 plus the +// Contact decode were built on them: the app read `count` bytes where firmware +// meant `count` hops, keeping half of every path at 2-byte width. (#309) +// +// TX counterparts: buildSetPathHashModeFrame (CMD_SET_PATH_HASH_MODE) sets the +// device-wide default width; encodePathLen() packs a per-path value to send. int pathHopCount(int pathLenRaw) => pathLenRaw & 0x3F; int pathHashSizeBytes(int pathLenRaw) => ((pathLenRaw >> 6) & 0x03) + 1; -/// Real hop count from the firmware path byte-length and the device hash width. -/// `pathHopCount` returns the path BYTE length, so at a 2-byte hash width a -/// single 2-byte hop reports 2 — divide by the width to get the true hop count. -/// Null (unknown) and negative (flood) sentinels pass through unchanged. (#112) -int? realHopCount(int? rawByteLen, int hashWidth) { - if (rawByteLen == null || rawByteLen < 0) return rawByteLen; - final w = hashWidth < 1 ? 1 : hashWidth; - return rawByteLen ~/ w; +/// Byte length of the hop-hash array described by a raw path-length byte. +int pathByteLength(int pathLenRaw) => + pathHopCount(pathLenRaw) * pathHashSizeBytes(pathLenRaw); + +/// Packs a hop count and per-hop hash width into the firmware path-length byte. +/// +/// Mirrors firmware `Packet::setPathHashSizeAndCount`. Sending a bare hop count +/// (mode bits 00) tells the radio "1-byte hashes", so a 2-byte path routed that +/// way is read one byte per hop and goes to nodes that were never on the route. +/// Width is clamped to 1..3; mode 3 is reserved by firmware +/// (`isValidPathLen` rejects hash_size == 4). (#309) +int encodePathLen(int hopCount, int hashWidth) { + final w = hashWidth.clamp(1, 3); + return ((w - 1) << 6) | (hopCount & 0x3F); } /// Readable strings from a RESP_CODE_DEVICE_INFO frame: build date, model, and @@ -824,10 +850,19 @@ Uint8List buildResetPathFrame(Uint8List pubKey) { // Build CMD_ADD_UPDATE_CONTACT frame to set custom path // Format: [cmd][pub_key x32][type][flags][path_len][path x64][name x32][Lat? x4, Lon? x4][timestamp? x4] +// +// [hopCount] is a HOP count and [hashWidth] the bytes per hop hash; the two are +// packed into the single wire path_len byte via encodePathLen(). +// +// This previously wrote the count raw, leaving the mode bits 00 — which tells +// the radio "1-byte hashes". On a 2-byte net that handed the firmware 2-byte +// hash data labelled as 1-byte hops, so it routed to nodes that were never on +// the route. That is the send-side half of #240's misrouting. (#309) Uint8List buildUpdateContactPathFrame( Uint8List pubKey, Uint8List path, - int pathLen, { + int hopCount, { + int hashWidth = 1, int type = 1, // ADV_TYPE_CHAT int flags = 0, String name = '', @@ -840,7 +875,8 @@ Uint8List buildUpdateContactPathFrame( writer.writeBytes(pubKey); writer.writeByte(type); writer.writeByte(flags); - writer.writeByte(pathLen); + // Negative = flood sentinel, passed through as the firmware's 0xFF. + writer.writeByte(hopCount < 0 ? 0xFF : encodePathLen(hopCount, hashWidth)); writer.writeBytesPadded(path, maxPathSize); diff --git a/lib/models/contact.dart b/lib/models/contact.dart index 7c690d3..07e2d1c 100644 --- a/lib/models/contact.dart +++ b/lib/models/contact.dart @@ -8,8 +8,22 @@ class Contact { final String name; final int type; final int flags; - final int pathLength; // -1 = flood, 0+ = direct hops (from device) - final Uint8List path; // Path bytes from device + + /// Hop count for [path]: -1 = flood, 0+ = number of hops. + /// + /// This is the firmware path-len field's low 6 bits, which firmware defines + /// as a hash COUNT, not a byte length (`src/Packet.h:79-84`). It was + /// previously treated as a byte count, which truncated every path at widths + /// above 1. (#309) + final int pathLength; + + /// Bytes per hop hash in [path] (1..3), from the path-len byte's high 2 bits. + /// + /// Carried per-path rather than read from the connector's global width, so a + /// stored path can never be re-sliced at a width it was not captured at. + final int pathHashWidth; + + final Uint8List path; // Path bytes from device (pathLength * pathHashWidth) final int? pathOverride; // User's path override: -1 = force flood, null = auto final Uint8List? pathOverrideBytes; // User's path override bytes @@ -28,6 +42,7 @@ class Contact { required this.type, this.flags = 0, required this.pathLength, + this.pathHashWidth = 1, required this.path, this.pathOverride, this.pathOverrideBytes, @@ -80,6 +95,7 @@ class Contact { int? type, int? flags, int? pathLength, + int? pathHashWidth, Uint8List? path, int? pathOverride, Uint8List? pathOverrideBytes, @@ -98,6 +114,7 @@ class Contact { type: type ?? this.type, flags: flags ?? this.flags, pathLength: pathLength ?? this.pathLength, + pathHashWidth: pathHashWidth ?? this.pathHashWidth, path: path ?? this.path, pathOverride: clearPathOverride ? null @@ -133,8 +150,8 @@ class Contact { return parts.join(','); } - /// Default grouping uses legacy single-byte hop hash width. - String get pathIdList => pathFormattedIdList(pathHashSize); + /// Groups by this path's own captured width, not a global or legacy default. + String get pathIdList => pathFormattedIdList(pathHashWidth); String get shortPubKeyHex { return "<${publicKeyHex.substring(0, 8)}...${publicKeyHex.substring(publicKeyHex.length - 8)}>"; @@ -166,14 +183,20 @@ class Contact { final type = reader.readByte(); final flags = reader.readByte(); final pathLen = reader.readByte(); - // The firmware path-len byte packs a hash-mode hint in the high 2 bits and - // the path BYTE length in the low 6 (#222). Decode before use: a direct - // node at 2-byte mode sends 0x40, whose raw value would otherwise read as - // 64 hops and pull 64 junk bytes into the path. 0xFF stays the flood - // sentinel; the device-configured pathHashByteWidth converts bytes->hops - // at display time. - final byteLen = pathLen == 0xFF ? -1 : pathHopCount(pathLen); - final safePathLen = byteLen > 0 ? byteLen : 0; + // The firmware path-len byte packs hash size in the high 2 bits and hash + // COUNT (hops) in the low 6; the byte length is count * size + // (`src/Packet.h:79-84`). 0xFF stays the flood sentinel. + // + // This previously read `count` BYTES, so at 2-byte width it kept half of + // every path and discarded the rest — the truncation behind #240's failed + // repeater logins. The width is taken from the path itself rather than the + // connector's global width, so a path is always sliced at the width it was + // captured at. (#309) + final isFlood = pathLen == 0xFF; + final hopCount = isFlood ? -1 : pathHopCount(pathLen); + final hashWidth = isFlood ? 1 : pathHashSizeBytes(pathLen); + final byteLen = isFlood ? 0 : (hopCount * hashWidth); + final safePathLen = byteLen.clamp(0, maxPathSize); final pathBytes = reader.readBytes(maxPathSize).sublist(0, safePathLen); final name = reader.readCStringGreedy(maxNameSize); @@ -218,7 +241,9 @@ class Contact { name: name.isEmpty ? 'Unknown' : name, type: type, flags: flags, - pathLength: byteLen, // decoded low-6-bit byte length; -1 = flood (#222) + pathLength: + hopCount, // hop count from the low 6 bits; -1 = flood (#309) + pathHashWidth: hashWidth, // bytes/hop from the high 2 bits (#309) path: pathBytes, latitude: lat, longitude: lon, diff --git a/lib/screens/channel_chat_screen.dart b/lib/screens/channel_chat_screen.dart index fb9ff0d..60f4c87 100644 --- a/lib/screens/channel_chat_screen.dart +++ b/lib/screens/channel_chat_screen.dart @@ -1530,8 +1530,9 @@ class _ChannelChatScreenState extends State { } Widget _buildHopBadge(BuildContext context, ChannelMessage message) { - final hashWidth = context.read().pathHashByteWidth; - final hops = realHopCount(message.hopCount, hashWidth) ?? 0; + // message.hopCount is already a hop count (path-len low 6 bits). It used to + // be divided by the device width, which halved it at 2-byte width. (#309) + final hops = message.hopCount ?? 0; final color = Theme.of(context).brightness == Brightness.dark ? Colors.grey[400] : Colors.grey[600]; diff --git a/lib/screens/channel_message_path_screen.dart b/lib/screens/channel_message_path_screen.dart index b328503..274e004 100644 --- a/lib/screens/channel_message_path_screen.dart +++ b/lib/screens/channel_message_path_screen.dart @@ -43,9 +43,11 @@ class ChannelMessagePathScreen extends StatelessWidget { final hashWidth = connector.pathHashByteWidth; final hops = _buildPathHops(primaryPath, connector, l10n, hashWidth); final hasHopDetails = primaryPath.isNotEmpty; + // Observed = bytes we hold / this path's width. Claimed = the hop count + // the path-len byte already carries; dividing it again halved it. (#309) final observedLabel = _formatObservedHops( - primaryPath.length ~/ hashWidth, - realHopCount(message.hopCount, hashWidth), + primaryPath.length ~/ message.pathHashSize, + message.hopCount, l10n, ); final extraPaths = _otherPaths(primaryPath, message.pathVariants); @@ -121,7 +123,6 @@ class ChannelMessagePathScreen extends StatelessWidget { Widget _buildSummaryCard(BuildContext context, {String? observedLabel}) { final l10n = context.l10n; - final hashWidth = context.read().pathHashByteWidth; return Card( child: Padding( padding: const EdgeInsets.all(12), @@ -145,7 +146,7 @@ class ChannelMessagePathScreen extends StatelessWidget { ), _buildDetailRow( l10n.channelPath_pathLabelTitle, - _formatPathLabel(realHopCount(message.hopCount, hashWidth), l10n), + _formatPathLabel(message.hopCount, l10n), ), if (observedLabel != null) _buildDetailRow(l10n.channelPath_observedLabel, observedLabel), diff --git a/lib/screens/chat_screen.dart b/lib/screens/chat_screen.dart index d8c743a..156378e 100644 --- a/lib/screens/chat_screen.dart +++ b/lib/screens/chat_screen.dart @@ -1274,15 +1274,11 @@ class _ChatScreenState extends State { return context.l10n.chat_hopsForced(contact.pathOverride!); } - // Use device's path. pathLength is a BYTE length; divide by the device's - // configured hash width to get the true hop count (#222). + // Use device's path. pathLength is a HOP count straight from the path-len + // byte's low 6 bits; it used to be divided by the device width, which + // halved it at 2-byte width. (#309) if (contact.pathLength < 0) return context.l10n.chat_floodAuto; - final hops = - realHopCount( - contact.pathLength, - context.read().pathHashByteWidth, - ) ?? - 0; + final hops = contact.pathLength; if (hops == 0) return context.l10n.chat_direct; return context.l10n.chat_hopsCount(hops); } diff --git a/lib/storage/contact_store.dart b/lib/storage/contact_store.dart index 7883d88..5d1805d 100644 --- a/lib/storage/contact_store.dart +++ b/lib/storage/contact_store.dart @@ -67,6 +67,7 @@ class ContactStore { 'type': contact.type, 'flags': contact.flags, 'pathLength': contact.pathLength, + 'pathHashWidth': contact.pathHashWidth, 'path': base64Encode(contact.path), 'pathOverride': contact.pathOverride, 'pathOverrideBytes': contact.pathOverrideBytes != null @@ -88,13 +89,31 @@ class ContactStore { final lastSeenMs = json['lastSeen'] as int? ?? 0; final lastMessageMs = json['lastMessageAt'] as int?; final lastModifiedMs = json['lastModified'] as int?; + final isLegacyPathRecord = + !json.containsKey('pathHashWidth') && + ((json['pathLength'] as int? ?? -1) > 0); + if (isLegacyPathRecord) { + appLogger.info( + 'Dropping pre-#309 path for contact ${json['name']} ' + '(decoded with the old count-as-bytes rule, so truncated); ' + 'reverting to flood until the device re-supplies it', + ); + } return Contact( publicKey: Uint8List.fromList(base64Decode(json['publicKey'] as String)), name: json['name'] as String? ?? 'Unknown', type: json['type'] as int? ?? 0, flags: json['flags'] as int? ?? 0, - pathLength: json['pathLength'] as int? ?? -1, - path: json['path'] != null + // Records written before #309 have no 'pathHashWidth' and their path was + // decoded with the old count-as-bytes rule, so at any width above 1 the + // stored bytes are a truncated fragment. Truncated bytes cannot be + // recovered by reinterpreting them, so a legacy record's path is dropped + // and the contact reverts to flood until the radio re-supplies it (the + // device refreshes contacts routinely, so this self-heals). Ben's call: + // re-fetch rather than keep a path we know is wrong. (#309) + pathLength: isLegacyPathRecord ? -1 : (json['pathLength'] as int? ?? -1), + pathHashWidth: json['pathHashWidth'] as int? ?? 1, + path: (!isLegacyPathRecord && json['path'] != null) ? Uint8List.fromList(base64Decode(json['path'] as String)) : Uint8List(0), pathOverride: json['pathOverride'] as int?, diff --git a/test/connector/build_update_contact_path_frame_test.dart b/test/connector/build_update_contact_path_frame_test.dart index da88f9a..dd110d3 100644 --- a/test/connector/build_update_contact_path_frame_test.dart +++ b/test/connector/build_update_contact_path_frame_test.dart @@ -16,6 +16,59 @@ void main() { final pubKey = Uint8List.fromList(List.generate(32, (i) => i)); final path = Uint8List.fromList([0xAA, 0xBB]); + // Byte offset of path_len in the frame: 1 cmd + 32 pubKey + 1 type + 1 flags. + const int pathLenOffset = 35; + + group('buildUpdateContactPathFrame path_len encoding (#309)', () { + test('packs the hash width into the high 2 bits', () { + // One 2-byte hop must go out as 0x41, not a bare 1. Writing the count + // raw leaves mode bits 00, which tells the radio "1-byte hashes" while + // handing it 2-byte hash data, so it routes to nodes that were never on + // the route. That is the send-side half of #240's misrouting. + final frame = buildUpdateContactPathFrame( + pubKey, + Uint8List.fromList([0xC6, 0x5C]), + 1, + hashWidth: 2, + ); + expect(frame[pathLenOffset], 0x41); + expect(pathHopCount(frame[pathLenOffset]), 1); + expect(pathHashSizeBytes(frame[pathLenOffset]), 2); + }); + + test('two 2-byte hops encode as 0x42', () { + final frame = buildUpdateContactPathFrame( + pubKey, + Uint8List.fromList([0xC6, 0x5C, 0xA1, 0xB2]), + 2, + hashWidth: 2, + ); + expect(frame[pathLenOffset], 0x42); + expect(pathByteLength(frame[pathLenOffset]), 4); + }); + + test('1-byte width is byte-identical to the pre-fix encoding', () { + // Legacy 1-byte nets must see no change on the wire. + final frame = buildUpdateContactPathFrame( + pubKey, + Uint8List.fromList([0xAA, 0xBB, 0xCC]), + 3, + hashWidth: 1, + ); + expect(frame[pathLenOffset], 3); + }); + + test('negative hop count emits the 0xFF flood sentinel', () { + final frame = buildUpdateContactPathFrame( + pubKey, + Uint8List(0), + -1, + hashWidth: 2, + ); + expect(frame[pathLenOffset], 0xFF); + }); + }); + group('buildUpdateContactPathFrame', () { test('omits lat/lon and timestamp tail when neither is provided', () { final frame = buildUpdateContactPathFrame( diff --git a/test/connector/path_hash_test.dart b/test/connector/path_hash_test.dart index fbe3224..fe7c3e5 100644 --- a/test/connector/path_hash_test.dart +++ b/test/connector/path_hash_test.dart @@ -1,32 +1,91 @@ -// Path-hash width / hop-count decode (#112). The firmware path-length byte is a -// BYTE count of the hop-hash array, not a hop count, so at a 2-byte hash width a -// single 2-byte hop reports 2. realHopCount divides by the device width to get -// true hops; the Packet Path screen slices and matches at the device width so a -// 2-byte hash like 6A3D resolves to one repeater instead of two 1-byte hops. +// Path-len byte encode/decode (#309, supersedes #112/#222). +// +// The firmware path-len byte is PACKED and self-describing: +// high 2 bits = hash size - 1 (1..3 bytes per hop hash) +// low 6 bits = hash COUNT, i.e. the number of HOPS +// byte length = count * size +// +// Verified against firmware src/Packet.h:79-84: +// getPathHashSize() == (path_len >> 6) + 1 +// getPathHashCount() == path_len & 63 +// getPathByteLen() == getPathHashCount() * getPathHashSize() +// setPathHashSizeAndCount(sz, n) { path_len = ((sz - 1) << 6) | (n & 63); } +// +// This file previously asserted the opposite (low 6 bits = a BYTE count, and a +// realHopCount() that divided by the device width). Those assertions encoded the +// bug: the app read `count` bytes where firmware meant `count` hops, keeping +// half of every path at 2-byte width, and halved every displayed hop count. import 'package:flutter_test/flutter_test.dart'; import 'package:meshcore_open/connector/meshcore_protocol.dart'; void main() { - group('realHopCount', () { - test('divides path byte-length by hash width', () { - // The bug: one 2-byte hop (6A3D) reported byteLen 2 -> shown as "2 hops". - expect(realHopCount(2, 2), 1); - expect(realHopCount(4, 2), 2); - expect(realHopCount(6, 3), 2); + group('path-len decode', () { + test('low 6 bits are a HOP count, not a byte count', () { + // 0x42 = 2-byte hashes, 2 hops. The old model called this "2 bytes". + expect(pathHopCount(0x42), 2); + expect(pathHashSizeBytes(0x42), 2); + expect(pathByteLength(0x42), 4); // 2 hops * 2 bytes }); - test('is a no-op at 1-byte width (no regression for legacy nets)', () { - expect(realHopCount(3, 1), 3); + test('one 2-byte hop is 1 hop occupying 2 bytes', () { + // Bandit's C65C case: ONE hop through a 2-byte-prefixed repeater. + expect(pathHopCount(0x41), 1); + expect(pathHashSizeBytes(0x41), 2); + expect(pathByteLength(0x41), 2); }); - test('treats width < 1 as 1', () { - expect(realHopCount(3, 0), 3); + test('1-byte width is unchanged (legacy nets)', () { + expect(pathHopCount(0x03), 3); + expect(pathHashSizeBytes(0x03), 1); + expect(pathByteLength(0x03), 3); }); - test('passes null (unknown) and negative (flood) sentinels through', () { - expect(realHopCount(null, 2), isNull); - expect(realHopCount(-1, 2), -1); + test('3-byte width', () { + // 0x82 = size 3, 2 hops -> 6 bytes. + expect(pathHashSizeBytes(0x82), 3); + expect(pathHopCount(0x82), 2); + expect(pathByteLength(0x82), 6); + }); + + test('direct (zero-hop) at 2-byte mode carries no path bytes', () { + // 0x40 = size 2, 0 hops. Reading the raw byte as a length would have + // pulled 64 junk bytes into the path. + expect(pathHopCount(0x40), 0); + expect(pathByteLength(0x40), 0); + }); + }); + + group('encodePathLen', () { + test('packs width and hop count the way firmware does', () { + expect(encodePathLen(2, 2), 0x42); + expect(encodePathLen(1, 2), 0x41); + expect(encodePathLen(3, 1), 0x03); + expect(encodePathLen(2, 3), 0x82); + expect(encodePathLen(0, 2), 0x40); + }); + + test('round-trips through the decoders', () { + for (final width in [1, 2, 3]) { + for (final hops in [0, 1, 5, 63]) { + final encoded = encodePathLen(hops, width); + expect(pathHopCount(encoded), hops, reason: 'hops w=$width'); + expect(pathHashSizeBytes(encoded), width, reason: 'width w=$width'); + } + } + }); + + test('clamps width to the 1..3 firmware range (mode 3 is reserved)', () { + expect(pathHashSizeBytes(encodePathLen(1, 0)), 1); + expect(pathHashSizeBytes(encodePathLen(1, 9)), 3); + }); + + test('a bare hop count would mislabel the width as 1 byte', () { + // The send-side bug: writing the count raw leaves mode bits 00, telling + // the radio "1-byte hashes" while handing it 2-byte hash data. + const rawCount = 2; + expect(pathHashSizeBytes(rawCount), 1); // what the radio would have read + expect(pathHashSizeBytes(encodePathLen(2, 2)), 2); // what it must read }); }); } diff --git a/test/models/contact_pathlen_test.dart b/test/models/contact_pathlen_test.dart index c89af0c..a6323c6 100644 --- a/test/models/contact_pathlen_test.dart +++ b/test/models/contact_pathlen_test.dart @@ -46,18 +46,37 @@ void main() { expect(c.path, [0xAA, 0xBB, 0xCC]); }); - test('2-byte-mode multi-hop (0x44) decodes to 4 bytes, not flood', () { - // High bits = mode-1 hint, low bits = 4 bytes = a 2-hop path at 2-byte - // width. Pre-#222 the raw 68 (> maxPathSize) was mis-flagged as flood. + test('2-byte-mode 2-hop (0x42) keeps all FOUR bytes (#309)', () { + // 0x42 = size 2, count 2 -> 2 hops occupying 2*2 = 4 bytes. + // + // Pre-#309 the low 6 bits were read as a byte count, so this kept only + // the first 2 bytes and silently discarded the second hop. That is the + // truncation behind #240's failed repeater logins. final c = Contact.fromFrame( - _frame(pathLen: 0x44, pathBytes: [1, 2, 3, 4]), + _frame(pathLen: 0x42, pathBytes: [0xC6, 0x5C, 0xA1, 0xB2]), )!; - expect(c.pathLength, 4); - expect(c.path, [1, 2, 3, 4]); - expect( - realHopCount(c.pathLength, 2), - 2, - ); // 4 bytes / 2-byte width = 2 hops + expect(c.pathLength, 2); // HOPS, not bytes + expect(c.pathHashWidth, 2); + expect(c.path, [0xC6, 0x5C, 0xA1, 0xB2]); // all 4 bytes retained + }); + + test('2-byte-mode single hop (0x41) is 1 hop of 2 bytes (#309)', () { + // Bandit's C65C case: ONE hop, previously surfaced as "2 hops". + final c = Contact.fromFrame( + _frame(pathLen: 0x41, pathBytes: [0xC6, 0x5C]), + )!; + expect(c.pathLength, 1); + expect(c.pathHashWidth, 2); + expect(c.path, [0xC6, 0x5C]); + }); + + test('3-byte-mode 2-hop (0x82) keeps six bytes (#309)', () { + final c = Contact.fromFrame( + _frame(pathLen: 0x82, pathBytes: [1, 2, 3, 4, 5, 6]), + )!; + expect(c.pathLength, 2); + expect(c.pathHashWidth, 3); + expect(c.path, [1, 2, 3, 4, 5, 6]); }); test('flood sentinel (0xFF) stays -1, empty path', () {