diff --git a/Bot_Usage.md b/Bot_Usage.md index 8bcae34..1d5a0b3 100644 --- a/Bot_Usage.md +++ b/Bot_Usage.md @@ -887,8 +887,14 @@ in one reply. Arguments: pin is still generated: `dist unavailable (1/total), `. - Zero located hops → `dist/map unavailable (0/total)`; a direct (no-hop) message → `direct (no path)`. -- Reliability note: 1-byte path hashes collide (many repeaters share a first - byte), so results are most reliable on 2- and 3-byte paths. +- Reliability note: path hashes can collide (several repeaters share the same + leading byte(s)), so results are most reliable on 2- and 3-byte paths. Hops + resolve only against repeater/room contacts. When a hash matches more than + one repeater, the hop is placed only if a candidate is clearly the closest + fit to a known anchor (the bot's own location, the sender, or an + unambiguous neighbouring hop); otherwise it is left unlocated rather than + risk using a far repeater that merely shares the hash. Setting the bot's own + location on the radio improves this disambiguation. - 10s per-user cooldown; no auth; works in DM and any allowed channel. - Makes a da.gd shortener call per invocation (when ≥2 hops are located). diff --git a/commands/path.py b/commands/path.py index 99c7c8f..351deb4 100644 --- a/commands/path.py +++ b/commands/path.py @@ -50,18 +50,98 @@ def _haversine(lat1, lon1, lat2, lon2, unit): return 2 * r * math.asin(math.sqrt(a)) -async def _resolve_hop_named(ctx, hop_hex): - # "map a path-hop hex prefix to (lat, lon, name), or None when it - # doesn't resolve to exactly one contact carrying a location. +def _num(v): + # device_info values are JSON-encoded; decode to float (None if missing). + try: + return float(json.loads(v)) + except (TypeError, ValueError): + return None + + +async def _bot_location(ctx): + # the bot's own advertised location (degrees), or None if unset (0/0). rows = await ctx.bot.db.fetchall( - "SELECT adv_lat, adv_lon, adv_name FROM contacts " - "WHERE substr(public_key,1,?)=? " + "SELECT key, value FROM device_info " + "WHERE key IN ('self_info.adv_lat','self_info.adv_lon')" + ) + d = {r["key"]: r["value"] for r in rows} + lat = _num(d.get("self_info.adv_lat")) + lon = _num(d.get("self_info.adv_lon")) + if lat is None or lon is None or (lat == 0 and lon == 0): + return None + return (lat, lon) + + +async def _sender_location(ctx): + # best-effort far-end anchor: the message sender's advertised location. + pk = ctx.sender_pubkey + if not pk: + return None + row = await ctx.bot.db.fetchone( + "SELECT adv_lat, adv_lon FROM contacts WHERE public_key=? " "AND adv_lat IS NOT NULL AND adv_lat!=0 AND adv_lon!=0", - (len(hop_hex), hop_hex), + (pk.lower(),), ) - if len(rows) == 1: - return (rows[0]["adv_lat"], rows[0]["adv_lon"], rows[0]["adv_name"]) - return None + return (row["adv_lat"], row["adv_lon"]) if row else None + + +async def _resolve_hops(ctx, hops): + """Resolve each path hop to (lat, lon, name) or None. + + Path hops are repeater/room hashes (the leading bytes of the repeater's + pubkey), which can collide across contacts. The old resolver matched any + contact type and trusted "exactly one located contact" — which silently + picked the WRONG repeater whenever only the wrong one (e.g. a far node + pulled in by tropospheric ducting) carried a location while the correct + local hop had none. Now: + - only repeater/room contacts are candidates (clients never repeat); + - a hop whose hash matches a single contact is trusted, even if distant + (a unique hash is unambiguous); + - a colliding hop is placed only when >= 2 of its candidates carry a + location and one is the clear nearest to a trusted anchor (the bot's + own location, the sender, or an already-unambiguous hop). A lone + located candidate among a collision is ambiguous and left unlocated, + so a wrong distance is never shown. + """ + per_hop = [] + for h in hops: + rows = await ctx.bot.db.fetchall( + "SELECT adv_lat, adv_lon, adv_name FROM contacts " + "WHERE type IN (2, 3) AND substr(public_key,1,?)=?", + (len(h), h), + ) + located = [ + (r["adv_lat"], r["adv_lon"], r["adv_name"]) + for r in rows + if r["adv_lat"] not in (None, 0) and r["adv_lon"] not in (None, 0) + ] + per_hop.append((len(rows), located)) + + anchors = [] + for loc in (await _bot_location(ctx), await _sender_location(ctx)): + if loc: + anchors.append(loc) + + resolved = [None] * len(hops) + # pass 1: unambiguous hops (single matching contact) are trusted and seed + # the anchor set used to disambiguate any collisions. + for i, (n, located) in enumerate(per_hop): + if n == 1 and located: + resolved[i] = located[0] + anchors.append((located[0][0], located[0][1])) + + # pass 2: collisions — choose the located candidate nearest a trusted + # anchor, but only when at least two candidates are located (otherwise the + # right one can't be told apart from the impostor → leave unlocated). + for i, (n, located) in enumerate(per_hop): + if n > 1 and len(located) >= 2 and anchors: + resolved[i] = min( + located, + key=lambda p: min( + _haversine(p[0], p[1], a[0], a[1], "mi") for a in anchors + ), + ) + return resolved def _pin_symbol(i): @@ -193,8 +273,9 @@ async def handle(ctx): path_str = ",".join(hops) nh = len(hops) - # resolve hops with names for the map markers - named = [await _resolve_hop_named(ctx, h) for h in hops] + # resolve hops with names for the map markers (collision-aware; see + # _resolve_hops — a far repeater sharing a path hash no longer wins) + named = await _resolve_hops(ctx, hops) loc = [p for p in named if p] n_located = len(loc) diff --git a/tests/test_path_resolve.py b/tests/test_path_resolve.py new file mode 100644 index 0000000..dffba2c --- /dev/null +++ b/tests/test_path_resolve.py @@ -0,0 +1,170 @@ +#!/usr/bin/env python3 +"""Collision-aware path-hop resolution for !path. + +A path hop is the leading bytes of a repeater's pubkey and can collide across +contacts. The resolver must never let a far repeater that merely shares the +hash supply a bogus location (the tropospheric-ducting bug), while still +trusting unambiguous hops and disambiguating multi-located collisions by +proximity to the bot. + +Run: /home/steve/dev/meshcore/meshcore-bot/venv/bin/python tests/test_path_resolve.py +""" + +import asyncio +import importlib.util +import json +import sys +from pathlib import Path +from types import SimpleNamespace + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT)) + +import mcbot # noqa: E402 + +# commands/ is not a package (the bot loads plugins dynamically), so load the +# module straight from its file. +_spec = importlib.util.spec_from_file_location( + "pathcmd", ROOT / "commands" / "path.py" +) +pathcmd = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(pathcmd) + +_failures = 0 + + +def check(cond, msg): + global _failures + print(f" {'ok' if cond else 'FAIL'}: {msg}") + if not cond: + _failures += 1 + + +def make_bot(): + cfg = mcbot.Config() + cfg.db_path = Path(":memory:") + log = mcbot.logging.getLogger("test-path") + log.addHandler(mcbot.logging.NullHandler()) + log.propagate = False + return mcbot.MCBot(cfg, log) + + +def pk(prefix): + # full 64-hex pubkey whose leading bytes are `prefix` + return prefix + "0" * (64 - len(prefix)) + + +async def add_contact(bot, public_key, *, type_=2, lat=None, lon=None, name=""): + await bot.db.execute( + "INSERT INTO contacts (public_key, adv_name, type, adv_lat, adv_lon) " + "VALUES (?,?,?,?,?)", + (public_key, name, type_, lat, lon), + ) + + +async def set_bot_location(bot, lat, lon): + # device_info values are JSON-encoded, mirroring _upsert_device_info + for k, v in (("self_info.adv_lat", lat), ("self_info.adv_lon", lon)): + await bot.db.execute( + "INSERT INTO device_info(key,value,last_updated) VALUES(?,?,0)", + (k, json.dumps(v)), + ) + + +def ctx_for(bot, sender_pubkey=None): + return SimpleNamespace(bot=bot, sender_pubkey=sender_pubkey) + + +async def test_collision_only_far_has_geo_is_unlocated(): + print("test_collision_only_far_has_geo_is_unlocated") + bot = make_bot() + await set_bot_location(bot, 32.0, -96.0) # Texas + # the reported bug: local hop (correct) has no geo; a far repeater sharing + # the 2-byte hash has geo. The far one must NOT be used. + await add_contact(bot, pk("abcd0"), name="local", lat=None, lon=None) + await add_contact(bot, pk("abcd1"), name="louisiana", lat=30.0, lon=-92.0) + res = await pathcmd._resolve_hops(ctx_for(bot), ["abcd"]) + check(res == [None], f"colliding hop with one geo'd impostor -> unlocated (got {res})") + bot.db.close() + + +async def test_collision_two_geo_picks_nearest_to_bot(): + print("test_collision_two_geo_picks_nearest_to_bot") + bot = make_bot() + await set_bot_location(bot, 32.0, -96.0) + await add_contact(bot, pk("abcd0"), name="near", lat=32.1, lon=-96.1) + await add_contact(bot, pk("abcd1"), name="far", lat=30.0, lon=-92.0) + res = await pathcmd._resolve_hops(ctx_for(bot), ["abcd"]) + check(res[0] is not None and res[0][2] == "near", + f"nearest-to-bot candidate wins (got {res[0]})") + bot.db.close() + + +async def test_unique_hop_trusted_even_if_far(): + print("test_unique_hop_trusted_even_if_far") + bot = make_bot() + await set_bot_location(bot, 32.0, -96.0) + # a single repeater matches the hash -> unambiguous, trust it even if far + await add_contact(bot, pk("ef01"), name="solo", lat=12.3, lon=45.6) + res = await pathcmd._resolve_hops(ctx_for(bot), ["ef01"]) + check(res[0] is not None and res[0][2] == "solo", "unique hop resolved") + bot.db.close() + + +async def test_non_repeater_excluded(): + print("test_non_repeater_excluded") + bot = make_bot() + await set_bot_location(bot, 32.0, -96.0) + # a companion (type 1) with geo must not satisfy a repeater hop + await add_contact(bot, pk("1234"), type_=1, name="client", lat=31.0, lon=-95.0) + res = await pathcmd._resolve_hops(ctx_for(bot), ["1234"]) + check(res == [None], "companion-type contact not used as a hop") + # add a real repeater sharing the prefix -> now it (not the client) wins + await add_contact(bot, pk("12349"), type_=2, name="rep", lat=31.5, lon=-95.5) + res = await pathcmd._resolve_hops(ctx_for(bot), ["1234"]) + check(res[0] is not None and res[0][2] == "rep", "repeater chosen over client") + bot.db.close() + + +async def test_unique_geoless_hop_is_unlocated(): + print("test_unique_geoless_hop_is_unlocated") + bot = make_bot() + await add_contact(bot, pk("5678"), name="noloc", lat=None, lon=None) + res = await pathcmd._resolve_hops(ctx_for(bot), ["5678"]) + check(res == [None], "unique repeater with no geo -> unlocated") + bot.db.close() + + +async def test_sender_anchor_used_when_no_bot_location(): + print("test_sender_anchor_used_when_no_bot_location") + bot = make_bot() # no bot location set + sender = pk("9999") + await add_contact(bot, sender, type_=1, name="sender", lat=30.0, lon=-92.0) + # collision with two geo'd candidates; only the sender anchors the choice + await add_contact(bot, pk("abcd0"), name="near-sender", lat=30.2, lon=-92.2) + await add_contact(bot, pk("abcd1"), name="far", lat=40.0, lon=-110.0) + res = await pathcmd._resolve_hops(ctx_for(bot, sender_pubkey=sender), ["abcd"]) + check(res[0] is not None and res[0][2] == "near-sender", + f"sender anchors disambiguation when bot loc absent (got {res[0]})") + bot.db.close() + + +async def main(): + for t in ( + test_collision_only_far_has_geo_is_unlocated, + test_collision_two_geo_picks_nearest_to_bot, + test_unique_hop_trusted_even_if_far, + test_non_repeater_excluded, + test_unique_geoless_hop_is_unlocated, + test_sender_anchor_used_when_no_bot_location, + ): + await t() + print() + if _failures: + print(f"FAILED: {_failures} check(s)") + sys.exit(1) + print("ALL TESTS PASSED") + + +if __name__ == "__main__": + asyncio.run(main())