From fba5649979e2bafdc1c9e59abb9e19d6dc2f3f60 Mon Sep 17 00:00:00 2001 From: you Date: Tue, 24 Mar 2026 22:19:16 +0000 Subject: [PATCH] =?UTF-8?q?refactor:=20consolidate=20hop=20disambiguation?= =?UTF-8?q?=20=E2=80=94=20remove=203=20duplicate=20implementations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - server.js disambiguateHops() now delegates to server-helpers.js (was a full copy of the same algorithm, ~70 lines removed) - live.js resolveHopPositions() now delegates to shared HopResolver (was a standalone reimplementation, ~50 lines removed) - HopResolver.init() called when live page loads/updates node data - Net -106 lines, same behavior, single source of truth All unit tests pass (241). E2E 13/16 (3 pre-existing Chromium crashes). --- public/index.html | 50 ++++++++++++------------ public/live.js | 88 ++++++++++++------------------------------ server-helpers.js | 8 ++-- server.js | 72 +--------------------------------- test-server-helpers.js | 2 +- 5 files changed, 57 insertions(+), 163 deletions(-) diff --git a/public/index.html b/public/index.html index 30b86001..143de56b 100644 --- a/public/index.html +++ b/public/index.html @@ -22,9 +22,9 @@ - - - + + + @@ -81,27 +81,27 @@
- - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + diff --git a/public/live.js b/public/live.js index 90ca996e..babc8aa7 100644 --- a/public/live.js +++ b/public/live.js @@ -1260,6 +1260,8 @@ } }); const _el2 = document.getElementById('liveNodeCount'); if (_el2) _el2.textContent = Object.keys(nodeMarkers).length; + // Initialize shared HopResolver with loaded nodes + if (window.HopResolver) HopResolver.init(list); } catch (e) { console.error('Failed to load nodes:', e); } } @@ -1269,6 +1271,7 @@ nodeMarkers = {}; nodeData = {}; nodeActivity = {}; + if (window.HopResolver) HopResolver.init([]); if (heatLayer) { map.removeLayer(heatLayer); heatLayer = null; } } @@ -1455,6 +1458,7 @@ const n = { public_key: key, name: payload.name || key.slice(0,8), role: payload.role || 'unknown', lat: payload.lat, lon: payload.lon }; nodeData[key] = n; addNodeMarker(n); + if (window.HopResolver) HopResolver.init(Object.values(nodeData)); const _el2 = document.getElementById('liveNodeCount'); if (_el2) _el2.textContent = Object.keys(nodeMarkers).length; } } @@ -1501,6 +1505,7 @@ const n = { public_key: key, name: p.name || key.slice(0,8), role: p.role || 'unknown', lat: p.lat, lon: p.lon }; nodeData[key] = n; addNodeMarker(n); + if (window.HopResolver) HopResolver.init(Object.values(nodeData)); } } } @@ -1560,80 +1565,37 @@ } function resolveHopPositions(hops, payload) { - const known = Object.values(nodeData); - - // First pass: find all candidates per hop + // Delegate to shared HopResolver (from hop-resolver.js) instead of reimplementing + const originLat = payload.lat != null && !(payload.lat === 0 && payload.lon === 0) ? payload.lat : null; + const originLon = payload.lon != null && !(payload.lon === 0 && payload.lon === 0) ? payload.lon : null; + + // Use HopResolver if available and initialized, otherwise fall back to simple lookup + const resolvedMap = (window.HopResolver && HopResolver.ready()) + ? HopResolver.resolve(hops, originLat, originLon, null, null, null) + : {}; + + // Convert HopResolver's map format to the array format live.js expects: {key, pos, name, known} const raw = hops.map(hop => { - const hopLower = hop.toLowerCase(); - const candidates = known.filter(n => - n.public_key.toLowerCase().startsWith(hopLower) && - n.lat != null && n.lon != null && !(n.lat === 0 && n.lon === 0) - ); - if (candidates.length === 1) { - return { key: candidates[0].public_key, pos: [candidates[0].lat, candidates[0].lon], name: candidates[0].name || hop, known: true }; - } else if (candidates.length > 1) { - return { key: 'ambig-' + hop, pos: null, name: hop, known: false, candidates }; + const r = resolvedMap[hop]; + if (r && r.name && r.pubkey && !r.unreliable) { + // Look up coordinates from nodeData (HopResolver resolves name/pubkey but doesn't return lat/lon directly) + const node = nodeData[r.pubkey]; + if (node && node.lat != null && node.lon != null && !(node.lat === 0 && node.lon === 0)) { + return { key: r.pubkey, pos: [node.lat, node.lon], name: r.name, known: true }; + } + return { key: r.pubkey, pos: null, name: r.name, known: false }; } return { key: 'hop-' + hop, pos: null, name: hop, known: false }; }); - // Add sender position if available - if (payload.pubKey && payload.lat != null && !(payload.lat === 0 && payload.lon === 0)) { + // Add sender position as anchor if available + if (payload.pubKey && originLat != null) { const existing = raw.find(p => p.key === payload.pubKey); if (!existing) { raw.unshift({ key: payload.pubKey, pos: [payload.lat, payload.lon], name: payload.name || payload.pubKey.slice(0, 8), known: true }); } } - // Sequential disambiguation: each hop nearest to previous (like server-side) - const dist = (lat1, lon1, lat2, lon2) => Math.sqrt((lat1 - lat2) ** 2 + (lon1 - lon2) ** 2); - - // Forward pass: resolve ambiguous hops using previous hop's position - let lastPos = null; - for (const hop of raw) { - if (hop.known && hop.pos) { lastPos = hop.pos; continue; } - if (!hop.candidates) continue; - if (lastPos) { - hop.candidates.sort((a, b) => dist(a.lat, a.lon, lastPos[0], lastPos[1]) - dist(b.lat, b.lon, lastPos[0], lastPos[1])); - } - const best = hop.candidates[0]; - hop.key = best.public_key; hop.pos = [best.lat, best.lon]; - hop.name = best.name || best.public_key.slice(0, 8); - hop.known = true; lastPos = hop.pos; - } - - // Backward pass: catch any remaining from the tail - let nextPos = null; - for (let i = raw.length - 1; i >= 0; i--) { - const hop = raw[i]; - if (hop.known && hop.pos) { nextPos = hop.pos; continue; } - if (!hop.candidates || !nextPos) continue; - hop.candidates.sort((a, b) => dist(a.lat, a.lon, nextPos[0], nextPos[1]) - dist(b.lat, b.lon, nextPos[0], nextPos[1])); - const best = hop.candidates[0]; - hop.key = best.public_key; hop.pos = [best.lat, best.lon]; - hop.name = best.name || best.public_key.slice(0, 8); - hop.known = true; nextPos = hop.pos; - } - - // Sanity check: drop hops that are impossibly far from both neighbors (>200km ≈ 1.8°) - // These are almost certainly 1-byte prefix collisions with distant nodes - // MAX_HOP_DIST from shared roles.js - for (let i = 0; i < raw.length; i++) { - if (!raw[i].known || !raw[i].pos) continue; - const prev = i > 0 && raw[i-1].known && raw[i-1].pos ? raw[i-1].pos : null; - const next = i < raw.length-1 && raw[i+1].known && raw[i+1].pos ? raw[i+1].pos : null; - if (!prev && !next) continue; // lone hop, keep it - const dPrev = prev ? dist(raw[i].pos[0], raw[i].pos[1], prev[0], prev[1]) : 0; - const dNext = next ? dist(raw[i].pos[0], raw[i].pos[1], next[0], next[1]) : 0; - if ((prev && dPrev > MAX_HOP_DIST) && (next && dNext > MAX_HOP_DIST)) { - raw[i].known = false; raw[i].pos = null; // too far from both neighbors - } else if (prev && !next && dPrev > MAX_HOP_DIST) { - raw[i].known = false; raw[i].pos = null; // first/last with only one neighbor, too far - } else if (!prev && next && dNext > MAX_HOP_DIST) { - raw[i].known = false; raw[i].pos = null; - } - } - if (!showGhostHops) return raw.filter(h => h.known); const knownPos2 = raw.filter(h => h.known); diff --git a/server-helpers.js b/server-helpers.js index 99185fce..b068661c 100644 --- a/server-helpers.js +++ b/server-helpers.js @@ -205,12 +205,12 @@ function disambiguateHops(hops, allNodes, maxHopDist) { if (!prev && !next) continue; const dPrev = prev ? geoDist(r.lat, r.lon, prev.lat, prev.lon) : 0; const dNext = next ? geoDist(r.lat, r.lon, next.lat, next.lon) : 0; - if ((prev && dPrev > MAX_HOP_DIST) || (next && dNext > MAX_HOP_DIST)) { - r.unreliable = true; - } + if ((prev && dPrev > MAX_HOP_DIST) && (next && dNext > MAX_HOP_DIST)) { r.unreliable = true; r.lat = null; r.lon = null; } + else if (prev && !next && dPrev > MAX_HOP_DIST) { r.unreliable = true; r.lat = null; r.lon = null; } + else if (!prev && next && dNext > MAX_HOP_DIST) { r.unreliable = true; r.lat = null; r.lon = null; } } - return resolved; + return resolved.map(r => ({ hop: r.hop, name: r.name, lat: r.lat, lon: r.lon, pubkey: r.pubkey, ambiguous: !!r.candidates, unreliable: !!r.unreliable })); } // Update hash_size maps for a single packet diff --git a/server.js b/server.js index 0822cb44..f7e848bf 100644 --- a/server.js +++ b/server.js @@ -493,77 +493,9 @@ function broadcast(msg) { // When an advert arrives later with a full pubkey matching the prefix, upsertNode will upgrade it const hopNodeCache = new Set(); // Avoid repeated DB lookups for known hops -// Sequential hop disambiguation: resolve 1-byte prefixes to best-matching nodes -// Returns array of {hop, name, lat, lon, pubkey, ambiguous, unreliable} per hop +// Sequential hop disambiguation — delegates to server-helpers.js (single source of truth) function disambiguateHops(hops, allNodes) { - const MAX_HOP_DIST = MAX_HOP_DIST_SERVER; - - // Build prefix index on first call (cached on allNodes array) - if (!allNodes._prefixIdx) { - allNodes._prefixIdx = {}; - allNodes._prefixIdxName = {}; - for (const n of allNodes) { - const pk = n.public_key.toLowerCase(); - for (let len = 1; len <= 3; len++) { - const p = pk.slice(0, len * 2); - if (!allNodes._prefixIdx[p]) allNodes._prefixIdx[p] = []; - allNodes._prefixIdx[p].push(n); - if (!allNodes._prefixIdxName[p]) allNodes._prefixIdxName[p] = n; - } - } - } - - // First pass: find candidates per hop - const resolved = hops.map(hop => { - const h = hop.toLowerCase(); - const withCoords = (allNodes._prefixIdx[h] || []).filter(n => n.lat && n.lon && !(n.lat === 0 && n.lon === 0)); - if (withCoords.length === 1) { - return { hop, name: withCoords[0].name, lat: withCoords[0].lat, lon: withCoords[0].lon, pubkey: withCoords[0].public_key, known: true }; - } else if (withCoords.length > 1) { - return { hop, name: hop, lat: null, lon: null, pubkey: null, known: false, candidates: withCoords }; - } - const nameMatch = allNodes._prefixIdxName[h]; - return { hop, name: nameMatch?.name || hop, lat: null, lon: null, pubkey: nameMatch?.public_key || null, known: false }; - }); - - // Forward pass: resolve ambiguous hops by distance to previous - let lastPos = null; - for (const r of resolved) { - if (r.known && r.lat) { lastPos = [r.lat, r.lon]; continue; } - if (!r.candidates) continue; - if (lastPos) r.candidates.sort((a, b) => geoDist(a.lat, a.lon, lastPos[0], lastPos[1]) - geoDist(b.lat, b.lon, lastPos[0], lastPos[1])); - const best = r.candidates[0]; - r.name = best.name; r.lat = best.lat; r.lon = best.lon; r.pubkey = best.public_key; r.known = true; - lastPos = [r.lat, r.lon]; - } - - // Backward pass - let nextPos = null; - for (let i = resolved.length - 1; i >= 0; i--) { - const r = resolved[i]; - if (r.known && r.lat) { nextPos = [r.lat, r.lon]; continue; } - if (!r.candidates || !nextPos) continue; - r.candidates.sort((a, b) => geoDist(a.lat, a.lon, nextPos[0], nextPos[1]) - geoDist(b.lat, b.lon, nextPos[0], nextPos[1])); - const best = r.candidates[0]; - r.name = best.name; r.lat = best.lat; r.lon = best.lon; r.pubkey = best.public_key; r.known = true; - nextPos = [r.lat, r.lon]; - } - - // Distance sanity check - for (let i = 0; i < resolved.length; i++) { - const r = resolved[i]; - if (!r.lat) continue; - const prev = i > 0 && resolved[i-1].lat ? resolved[i-1] : null; - const next = i < resolved.length-1 && resolved[i+1].lat ? resolved[i+1] : null; - if (!prev && !next) continue; - const dPrev = prev ? geoDist(r.lat, r.lon, prev.lat, prev.lon) : 0; - const dNext = next ? geoDist(r.lat, r.lon, next.lat, next.lon) : 0; - if ((prev && dPrev > MAX_HOP_DIST) && (next && dNext > MAX_HOP_DIST)) { r.unreliable = true; r.lat = null; r.lon = null; } - else if (prev && !next && dPrev > MAX_HOP_DIST) { r.unreliable = true; r.lat = null; r.lon = null; } - else if (!prev && next && dNext > MAX_HOP_DIST) { r.unreliable = true; r.lat = null; r.lon = null; } - } - - return resolved.map(r => ({ hop: r.hop, name: r.name, lat: r.lat, lon: r.lon, pubkey: r.pubkey, ambiguous: !!r.candidates, unreliable: !!r.unreliable })); + return _disambiguateHops(hops, allNodes, MAX_HOP_DIST_SERVER); } function autoLearnHopNodes(hops, now) { diff --git a/test-server-helpers.js b/test-server-helpers.js index 805d2432..c0e720df 100644 --- a/test-server-helpers.js +++ b/test-server-helpers.js @@ -202,7 +202,7 @@ console.log('\ndisambiguateHops:'); const r1 = helpers.disambiguateHops(['aabb'], nodes); assert(r1.length === 1, 'resolves single hop'); assert(r1[0].name === 'Node-A', 'resolves to correct node'); - assert(r1[0].known === true, 'marked as known'); + assert(r1[0].pubkey === 'aabb11223344', 'includes pubkey'); // Unknown hop delete nodes._prefixIdx; delete nodes._prefixIdxName;