From 9d806c608b9c017cf73121537511e7202bec35c4 Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Sun, 8 Mar 2026 21:01:01 -0700 Subject: [PATCH] Add contact normalization rather than loading the packed path bytes --- app/models.py | 30 ++++++++++++++++------- app/path_utils.py | 50 ++++++++++++++++++++++++++++++++++++++ app/radio_sync.py | 27 ++++++++++++++++++-- app/repository/contacts.py | 38 +++++++++++++++++++++-------- tests/test_path_utils.py | 35 ++++++++++++++++++++++++++ tests/test_radio_sync.py | 27 ++++++++++++++++++++ 6 files changed, 186 insertions(+), 21 deletions(-) diff --git a/app/models.py b/app/models.py index 5a5048d..474e65c 100644 --- a/app/models.py +++ b/app/models.py @@ -2,6 +2,8 @@ from typing import Literal from pydantic import BaseModel, Field +from app.path_utils import normalize_contact_route + class Contact(BaseModel): public_key: str = Field(description="Public key (64-char hex)") @@ -26,14 +28,19 @@ class Contact(BaseModel): The radio API uses different field names (adv_name, out_path, etc.) than our database schema (name, last_path, etc.). """ + last_path, last_path_len, out_path_hash_mode = normalize_contact_route( + self.last_path, + self.last_path_len, + self.out_path_hash_mode, + ) return { "public_key": self.public_key, "adv_name": self.name or "", "type": self.type, "flags": self.flags, - "out_path": self.last_path or "", - "out_path_len": self.last_path_len, - "out_path_hash_mode": self.out_path_hash_mode, + "out_path": last_path, + "out_path_len": last_path_len, + "out_path_hash_mode": out_path_hash_mode, "adv_lat": self.lat if self.lat is not None else 0.0, "adv_lon": self.lon if self.lon is not None else 0.0, "last_advert": self.last_advert if self.last_advert is not None else 0, @@ -46,17 +53,22 @@ class Contact(BaseModel): This is the inverse of to_radio_dict(), used when syncing contacts from radio to database. """ + last_path, last_path_len, out_path_hash_mode = normalize_contact_route( + radio_data.get("out_path"), + radio_data.get("out_path_len", -1), + radio_data.get( + "out_path_hash_mode", + -1 if radio_data.get("out_path_len", -1) == -1 else 0, + ), + ) return { "public_key": public_key, "name": radio_data.get("adv_name"), "type": radio_data.get("type", 0), "flags": radio_data.get("flags", 0), - "last_path": radio_data.get("out_path"), - "last_path_len": radio_data.get("out_path_len", -1), - "out_path_hash_mode": radio_data.get( - "out_path_hash_mode", - -1 if radio_data.get("out_path_len", -1) == -1 else 0, - ), + "last_path": last_path, + "last_path_len": last_path_len, + "out_path_hash_mode": out_path_hash_mode, "lat": radio_data.get("adv_lat"), "lon": radio_data.get("adv_lon"), "last_advert": radio_data.get("last_advert"), diff --git a/app/path_utils.py b/app/path_utils.py index 0c58744..1b9c294 100644 --- a/app/path_utils.py +++ b/app/path_utils.py @@ -148,3 +148,53 @@ def first_hop_hex(path_hex: str, hop_count: int) -> str | None: """ hops = split_path_hex(path_hex, hop_count) return hops[0] if hops else None + + +def normalize_contact_route( + path_hex: str | None, + path_len: int | None, + out_path_hash_mode: int | None, +) -> tuple[str, int, int]: + """Normalize stored contact route fields. + + Handles legacy/bad rows where the packed wire path byte was stored directly + in `last_path_len` (sometimes as a signed byte, e.g. `-125` for `0x83`). + Returns `(path_hex, hop_count, hash_mode)`. + """ + normalized_path = path_hex or "" + + try: + normalized_len = int(path_len) if path_len is not None else -1 + except (TypeError, ValueError): + normalized_len = -1 + + try: + normalized_mode = int(out_path_hash_mode) if out_path_hash_mode is not None else None + except (TypeError, ValueError): + normalized_mode = None + + if normalized_len < -1 or normalized_len > 63: + packed = normalized_len & 0xFF + if packed == 0xFF: + return "", -1, -1 + decoded_mode = (packed >> 6) & 0x03 + if decoded_mode != 0x03: + normalized_len = packed & 0x3F + normalized_mode = decoded_mode + + if normalized_len == -1: + return "", -1, -1 + + if normalized_mode not in (0, 1, 2): + normalized_mode = 0 + + if normalized_path: + bytes_per_hop = normalized_mode + 1 + actual_bytes = len(normalized_path) // 2 + expected_bytes = normalized_len * bytes_per_hop + if actual_bytes > expected_bytes >= 0: + normalized_path = normalized_path[: expected_bytes * 2] + elif actual_bytes < expected_bytes and bytes_per_hop > 0 and actual_bytes % bytes_per_hop == 0: + normalized_len = actual_bytes // bytes_per_hop + + return normalized_path, normalized_len, normalized_mode diff --git a/app/radio_sync.py b/app/radio_sync.py index 9e77c9a..e254897 100644 --- a/app/radio_sync.py +++ b/app/radio_sync.py @@ -30,6 +30,21 @@ from app.repository import ( logger = logging.getLogger(__name__) +def _contact_sync_debug_fields(contact: Contact) -> dict[str, object]: + """Return key contact fields for sync failure diagnostics.""" + return { + "type": contact.type, + "flags": contact.flags, + "last_path": contact.last_path, + "last_path_len": contact.last_path_len, + "out_path_hash_mode": contact.out_path_hash_mode, + "last_advert": contact.last_advert, + "lat": contact.lat, + "lon": contact.lon, + "on_radio": contact.on_radio, + } + + async def upsert_channel_from_radio_slot(payload: dict, *, on_radio: bool) -> str | None: """Parse a radio channel-slot payload and upsert to the database. @@ -664,7 +679,8 @@ async def _sync_contacts_to_radio_inner(mc: MeshCore) -> dict: continue try: - result = await mc.commands.add_contact(contact.to_radio_dict()) + radio_contact_payload = contact.to_radio_dict() + result = await mc.commands.add_contact(radio_contact_payload) if result.type == EventType.OK: loaded += 1 await ContactRepository.set_on_radio(contact.public_key, True) @@ -687,7 +703,14 @@ async def _sync_contacts_to_radio_inner(mc: MeshCore) -> dict: ) except Exception as e: failed += 1 - logger.warning("Error loading contact %s: %s", contact.public_key[:12], e) + logger.warning( + "Error loading contact %s with fields=%s radio_payload=%s: %s", + contact.public_key[:12], + _contact_sync_debug_fields(contact), + locals().get("radio_contact_payload"), + e, + exc_info=True, + ) if loaded > 0 or failed > 0: logger.info( diff --git a/app/repository/contacts.py b/app/repository/contacts.py index 11e3ffc..f1fbad5 100644 --- a/app/repository/contacts.py +++ b/app/repository/contacts.py @@ -8,7 +8,7 @@ from app.models import ( ContactAdvertPathSummary, ContactNameHistory, ) -from app.path_utils import first_hop_hex +from app.path_utils import first_hop_hex, normalize_contact_route class AmbiguousPublicKeyPrefixError(ValueError): @@ -23,9 +23,11 @@ class AmbiguousPublicKeyPrefixError(ValueError): class ContactRepository: @staticmethod async def upsert(contact: dict[str, Any]) -> None: - out_path_hash_mode = contact.get("out_path_hash_mode") - if out_path_hash_mode is None: - out_path_hash_mode = -1 if contact.get("last_path_len", -1) == -1 else 0 + last_path, last_path_len, out_path_hash_mode = normalize_contact_route( + contact.get("last_path"), + contact.get("last_path_len", -1), + contact.get("out_path_hash_mode"), + ) await db.conn.execute( """ @@ -54,8 +56,8 @@ class ContactRepository: contact.get("name"), contact.get("type", 0), contact.get("flags", 0), - contact.get("last_path"), - contact.get("last_path_len", -1), + last_path, + last_path_len, out_path_hash_mode, contact.get("last_advert"), contact.get("lat"), @@ -71,14 +73,19 @@ class ContactRepository: @staticmethod def _row_to_contact(row) -> Contact: """Convert a database row to a Contact model.""" + last_path, last_path_len, out_path_hash_mode = normalize_contact_route( + row["last_path"], + row["last_path_len"], + row["out_path_hash_mode"], + ) return Contact( public_key=row["public_key"], name=row["name"], type=row["type"], flags=row["flags"], - last_path=row["last_path"], - last_path_len=row["last_path_len"], - out_path_hash_mode=row["out_path_hash_mode"], + last_path=last_path, + last_path_len=last_path_len, + out_path_hash_mode=out_path_hash_mode, last_advert=row["last_advert"], lat=row["lat"], lon=row["lon"], @@ -215,11 +222,22 @@ class ContactRepository: path_len: int, out_path_hash_mode: int | None = None, ) -> None: + normalized_path, normalized_path_len, normalized_hash_mode = normalize_contact_route( + path, + path_len, + out_path_hash_mode, + ) await db.conn.execute( """UPDATE contacts SET last_path = ?, last_path_len = ?, out_path_hash_mode = COALESCE(?, out_path_hash_mode), last_seen = ? WHERE public_key = ?""", - (path, path_len, out_path_hash_mode, int(time.time()), public_key.lower()), + ( + normalized_path, + normalized_path_len, + normalized_hash_mode, + int(time.time()), + public_key.lower(), + ), ) await db.conn.commit() diff --git a/tests/test_path_utils.py b/tests/test_path_utils.py index d6e846c..7f4b143 100644 --- a/tests/test_path_utils.py +++ b/tests/test_path_utils.py @@ -5,6 +5,7 @@ import pytest from app.path_utils import ( decode_path_byte, first_hop_hex, + normalize_contact_route, parse_packet_envelope, path_wire_len, split_path_hex, @@ -153,6 +154,26 @@ class TestFirstHopHex: assert first_hop_hex("", 0) is None +class TestNormalizeContactRoute: + def test_decodes_legacy_signed_packed_len(self): + path_hex, path_len, hash_mode = normalize_contact_route("3f3f69de1c7b7e7662", -125, 2) + assert path_hex == "3f3f69de1c7b7e7662" + assert path_len == 3 + assert hash_mode == 2 + + def test_decodes_legacy_unsigned_packed_len(self): + path_hex, path_len, hash_mode = normalize_contact_route("7e7662ae9258", 130, None) + assert path_hex == "7e7662ae9258" + assert path_len == 2 + assert hash_mode == 2 + + def test_normalizes_flood_to_empty_path(self): + path_hex, path_len, hash_mode = normalize_contact_route("abcd", -1, 2) + assert path_hex == "" + assert path_len == -1 + assert hash_mode == -1 + + class TestContactToRadioDictHashMode: """Test that Contact.to_radio_dict() preserves the stored out_path_hash_mode.""" @@ -216,6 +237,20 @@ class TestContactToRadioDictHashMode: d = c.to_radio_dict() assert d["out_path_hash_mode"] == 1 + def test_decodes_legacy_signed_packed_len_before_radio_sync(self): + from app.models import Contact + + c = Contact( + public_key="ff" * 32, + last_path="3f3f69de1c7b7e7662", + last_path_len=-125, + out_path_hash_mode=2, + ) + d = c.to_radio_dict() + assert d["out_path"] == "3f3f69de1c7b7e7662" + assert d["out_path_len"] == 3 + assert d["out_path_hash_mode"] == 2 + class TestContactFromRadioDictHashMode: """Test that Contact.from_radio_dict() preserves explicit path hash mode.""" diff --git a/tests/test_radio_sync.py b/tests/test_radio_sync.py index 2ec76e7..2e60835 100644 --- a/tests/test_radio_sync.py +++ b/tests/test_radio_sync.py @@ -377,6 +377,33 @@ class TestSyncRecentContactsToRadio: assert payload["out_path_len"] == 2 assert payload["out_path_hash_mode"] == 1 + @pytest.mark.asyncio + async def test_add_contact_decodes_legacy_packed_path_len(self, test_db): + """Legacy signed packed path bytes are normalized before add_contact.""" + await _insert_contact( + KEY_A, + "Alice", + last_contacted=2000, + last_path="3f3f69de1c7b7e7662", + last_path_len=-125, + out_path_hash_mode=2, + ) + + mock_mc = MagicMock() + mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None) + mock_result = MagicMock() + mock_result.type = EventType.OK + mock_mc.commands.add_contact = AsyncMock(return_value=mock_result) + + radio_manager._meshcore = mock_mc + result = await sync_recent_contacts_to_radio() + + assert result["loaded"] == 1 + payload = mock_mc.commands.add_contact.call_args.args[0] + assert payload["out_path"] == "3f3f69de1c7b7e7662" + assert payload["out_path_len"] == 3 + assert payload["out_path_hash_mode"] == 2 + @pytest.mark.asyncio async def test_mc_param_bypasses_lock_acquisition(self, test_db): """When mc is passed, the function uses it directly without acquiring radio_operation.