mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-11 03:05:38 +00:00
Follows #1841. Moves the pathTrust default from 2 back to 1. ## Why #1784's first acceptance criterion is **"Default behaviour remains backward-compatible"**, and its example config shows `minHashBytesForMapping: 1`. What shipped is 2. The problem is not the value. It is that **there is no way to undo it from the UI.** #1841 adds no control for the threshold: it is `config.json` only, and changing it needs a restart. The customizer gains a hint that says exactly that. So an instance that upgrades without touching config switches to the stricter rule, and the only visible symptom is that the neighbour graph and the resolved paths quietly get smaller. The existing "Hide 1-byte path hops" toggle (#1633) is a *display* filter and does not change what counts as evidence, so it is not an escape hatch either. The two are easy to confuse. ## How much this actually moves Measured on a live instance via `/api/analytics/hash-sizes`, not estimated: | path-hop observations | count | share | |---|---|---| | 1-byte prefix | 116,923 | **56.0%** | | 2-byte prefix | 86,031 | 41.2% | | 3-byte prefix | 5,753 | 2.8% | | repeaters by observed hash size | count | |---|---| | 1-byte | **645 (41%)** | | 2-byte | 865 | | 3-byte | 63 | At threshold 2 the 1-byte column stops counting as mapping evidence. `MeetsPathTrust` also drops the legacy bucket-0 observations with it (pre-#1638 persisted neighbor edges that carry no per-mode breakdown), so already-stored edges lose their evidence status on upgrade too. ## What this does not change The knob works and is untouched. Operators who want the stricter behaviour set `minHashBytesForMapping` to 2 or 3, which is the opt-in #1784 describes. Only the default moves. Nothing about storage changes; packets and paths were never affected either way. ## Also fixes an inconsistency inside #1841 Five frontend consumers already fall back to **1** when `MC_getPathTrustThreshold()` is unavailable: `analytics.js`, `live.js`, `map.js`, `nodes.js`, `route-view.js`. Two fell back to **2**: `hop-filter.js` (the getter itself) and `customize-v2.js`. They now all agree. ## Tests - `internal/packetpath`: `TestMeetsPathTrust_ZeroValueOptIn` was asserting the old default *through behaviour*, so it would need rewriting on any future default change. It now asserts the property instead: an absent JSON field resolves to `DefaultMinHashBytesForMapping`, behaves identically to naming that value outright, and an explicit stricter setting still wins. Package tests pass. - `test-issue-1633-hide-1byte-hops.js`: the case pinning the getter's default is updated, with the reasoning in a comment so the next person sees why it is 1. **37 passed, 0 failed** (master baseline: 37 passed, 0 failed). - `cmd/server` config tests pass. ## One thing I want to flag rather than paper over The test I changed was named `default is 2 (operator-confirmed)`. I am overriding something that was confirmed with an operator, and I am not claiming that confirmation was wrong. My reading is that it was about the threshold being a *useful* value, which it is, rather than about it being the default in a build with no UI to change it. If the intent really was "2 out of the box for everyone", say so and I will close this. @Bjorkan as the issue author, @nullrouten0 and @Saarlandpower since you have touched adjacent code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Wzwr3eXseyNM7Xj598djjE Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+2
-2
@@ -224,8 +224,8 @@
|
||||
"_comment_observerThresholds": "Observer health classification. Online: last_seen < observerOnlineMinutes ago. Stale: between Online and observerStaleMinutes. Offline: beyond observerStaleMinutes. Defaults 60 / 1440 (1h / 24h) match the node thresholds for consistency and eliminate flap on low-traffic / CDN-fronted instances (#1552). Operators who want the old aggressive 10-min Online threshold can set observerOnlineMinutes: 10."
|
||||
},
|
||||
"pathTrust": {
|
||||
"minHashBytesForMapping": 2,
|
||||
"_comment_pathTrust": "Minimum path-hash prefix length, in bytes, trusted as mapping/topology evidence (issue #1784). MeshCore path hops are hashed pubkey prefixes of 1, 2, or 3 bytes (firmware hash_size = (pathByte>>6)+1); valid range is 1-3. Default 2 excludes 1-byte prefixes (256-value collision space) from mapping evidence — operator-confirmed to reduce false positives on denser meshes. Set to 1 for backward-compatible trust-all behavior. Set to 3 for the strictest mode (only 3-byte evidence trusted). This does not affect raw storage — packets/paths are always stored as received."
|
||||
"minHashBytesForMapping": 1,
|
||||
"_comment_pathTrust": "Minimum path-hash prefix length, in bytes, trusted as mapping/topology evidence (issue #1784). MeshCore path hops are hashed pubkey prefixes of 1, 2 or 3 bytes (firmware hash_size = (pathByte>>6)+1); valid range is 1-3. Default 1 keeps the pre-#1784 behaviour, where every prefix length counts. Set 2 to exclude 1-byte prefixes (256-value collision space, so a currently-unique match can still be a false positive on a dense mesh), or 3 to require the strongest evidence. NOTE: there is no UI control for this - it is config-only and needs a restart, and raising it can remove a large share of your neighbour-graph edges and resolved paths with nothing in the UI explaining why. This does not affect raw storage: packets and paths are always stored as received."
|
||||
},
|
||||
"defaultRegion": "SJC",
|
||||
"mapDefaults": {
|
||||
|
||||
@@ -20,7 +20,19 @@ type TrustConfig struct {
|
||||
MinHashBytesForMapping int `json:"minHashBytesForMapping,omitempty"`
|
||||
}
|
||||
|
||||
const DefaultMinHashBytesForMapping = 2
|
||||
// DefaultMinHashBytesForMapping is the backward-compatible default: every
|
||||
// prefix length counts as mapping evidence, exactly as before #1784.
|
||||
//
|
||||
// It is deliberately 1 rather than the stricter 2. There is no UI control for
|
||||
// this threshold, it can only be changed in config.json and that needs a
|
||||
// restart, so shipping 2 would tighten every instance on upgrade with nothing
|
||||
// in the UI explaining why the neighbour graph shrank. Measured on a live
|
||||
// network, 56% of path-hop observations carry a 1-byte prefix and 41% of
|
||||
// repeaters use a 1-byte hash, so that is not a marginal change. Issue #1784's
|
||||
// own first acceptance criterion is that the default stays backward compatible.
|
||||
//
|
||||
// Operators who want the stricter behaviour set minHashBytesForMapping: 2 or 3.
|
||||
const DefaultMinHashBytesForMapping = 1
|
||||
|
||||
const MaxHashBytes = 3
|
||||
|
||||
|
||||
@@ -3,13 +3,16 @@ package packetpath
|
||||
import "testing"
|
||||
|
||||
func TestMeetsPathTrust_DefaultThreshold(t *testing.T) {
|
||||
// nil cfg → DefaultMinHashBytesForMapping (2): 0-byte and 1-byte excluded.
|
||||
// nil cfg → DefaultMinHashBytesForMapping (1): everything is trusted, which
|
||||
// is the pre-#1784 behaviour. This test is the guard on that promise: if the
|
||||
// default is ever raised, upgrading instances silently lose a large share of
|
||||
// their mapping evidence with no UI control to opt back out.
|
||||
cases := []struct {
|
||||
prefixBytes int
|
||||
want bool
|
||||
}{
|
||||
{0, false},
|
||||
{1, false},
|
||||
{0, true},
|
||||
{1, true},
|
||||
{2, true},
|
||||
{3, true},
|
||||
}
|
||||
@@ -49,17 +52,25 @@ func TestMeetsPathTrust_Exclude1Byte(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestMeetsPathTrust_ZeroValueOptIn(t *testing.T) {
|
||||
// A zero-value MinHashBytesForMapping (unset) must fall back to default (2),
|
||||
// so bucket-0 and 1-byte prefixes fail; 2-byte and above pass.
|
||||
cfg := &TrustConfig{}
|
||||
if MeetsPathTrust(1, cfg) {
|
||||
t.Errorf("MeetsPathTrust(1, %+v) = true, want false (unset uses default=2, 1-byte excluded)", cfg)
|
||||
// A zero-value MinHashBytesForMapping (the JSON field absent) must resolve to
|
||||
// DefaultMinHashBytesForMapping, not be read as an explicit opt-in to some
|
||||
// other number. Asserted against the constant rather than against a literal
|
||||
// so this keeps testing the property if the default is ever changed again.
|
||||
unset := &TrustConfig{}
|
||||
if got := unset.MinHashBytesOrDefault(); got != DefaultMinHashBytesForMapping {
|
||||
t.Errorf("unset.MinHashBytesOrDefault() = %d, want %d", got, DefaultMinHashBytesForMapping)
|
||||
}
|
||||
if MeetsPathTrust(0, cfg) {
|
||||
t.Errorf("MeetsPathTrust(0, %+v) = true, want false (bucket-0 excluded at default threshold)", cfg)
|
||||
// And it must behave identically to a config that names the default outright.
|
||||
explicit := &TrustConfig{MinHashBytesForMapping: DefaultMinHashBytesForMapping}
|
||||
for _, prefixBytes := range []int{0, 1, 2, 3} {
|
||||
if MeetsPathTrust(prefixBytes, unset) != MeetsPathTrust(prefixBytes, explicit) {
|
||||
t.Errorf("prefixBytes=%d: unset and explicit-default disagree", prefixBytes)
|
||||
}
|
||||
}
|
||||
if !MeetsPathTrust(2, cfg) {
|
||||
t.Errorf("MeetsPathTrust(2, %+v) = false, want true (2-byte passes at default threshold)", cfg)
|
||||
// An explicit stricter setting must still be honoured over the default.
|
||||
strict := &TrustConfig{MinHashBytesForMapping: 2}
|
||||
if MeetsPathTrust(1, strict) {
|
||||
t.Error("MeetsPathTrust(1, minBytes=2) = true, want false — an explicit setting must win")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1604,7 +1604,7 @@
|
||||
try { on = localStorage.getItem('meshcore-hide-1byte-hops') === 'true'; } catch (_e) {}
|
||||
var trustThreshold = (typeof window.MC_getPathTrustThreshold === 'function')
|
||||
? window.MC_getPathTrustThreshold()
|
||||
: 2;
|
||||
: 1;
|
||||
var trustDesc = trustThreshold >= 2
|
||||
? '1-byte path-hash prefixes collide ~8-way at ~2k relays and are excluded from topology/mapping evidence (minHashBytesForMapping: ' + trustThreshold + '). Routes below this threshold show as speculative or are excluded. Change via pathTrust.minHashBytesForMapping in config.json.'
|
||||
: '1-byte path-hash prefixes collide ~8-way at ~2k relays — many polylines and rows they produce are visual noise. Hide them here without changing what\'s stored. Set pathTrust.minHashBytesForMapping in config.json to 2 or 3 for stricter server-side evidence requirements.';
|
||||
|
||||
@@ -56,7 +56,7 @@
|
||||
if (typeof window !== 'undefined' && typeof window.PATH_TRUST === 'number') {
|
||||
return window.PATH_TRUST;
|
||||
}
|
||||
return 2;
|
||||
return 1;
|
||||
}
|
||||
|
||||
// #1784 — whether a hop meets the server-side path trust threshold.
|
||||
|
||||
@@ -451,12 +451,18 @@ test('[kb #1] anti-tautology: tests reference the actual production files (not i
|
||||
|
||||
console.log('\n=== #1784: path trust threshold ===');
|
||||
|
||||
test('#1784: MC_getPathTrustThreshold default is 2 (operator-confirmed)', () => {
|
||||
test('#1784: MC_getPathTrustThreshold defaults to 1 (backward compatible)', () => {
|
||||
// Deliberately 1, matching DefaultMinHashBytesForMapping in
|
||||
// internal/packetpath/trust.go. There is no UI control for this threshold —
|
||||
// it is config.json only and needs a restart — so a stricter default would
|
||||
// silently tighten every instance on upgrade with nothing in the UI saying
|
||||
// why the neighbour graph shrank. Operators opt in with
|
||||
// pathTrust.minHashBytesForMapping: 2 or 3.
|
||||
const ctx = makeSandbox();
|
||||
load(ctx, 'public/hop-filter.js');
|
||||
delete ctx.window.PATH_TRUST;
|
||||
assert.strictEqual(ctx.window.MC_getPathTrustThreshold(), 2,
|
||||
'default path trust threshold must be 2 (operator-confirmed, excludes 1-byte)');
|
||||
assert.strictEqual(ctx.window.MC_getPathTrustThreshold(), 1,
|
||||
'default path trust threshold must be 1 so an upgrade changes nothing');
|
||||
});
|
||||
|
||||
test('#1784: MC_getPathTrustThreshold reads window.PATH_TRUST', () => {
|
||||
|
||||
Reference in New Issue
Block a user