fix(bloom): restrict outgoing filter merge to tree peers only - #88
Merged
Conversation
fipsd computed each node's outgoing bloom filter by OR-merging the stored view of EVERY active neighbor (full-mesh). FIPS mandates a TREE-ONLY merge: only spanning-tree peers (parent + children) may be folded into an outgoing filter. Non-tree mesh peers still receive/store FilterAnnounce for routing queries, but their filters must not be propagated transitively. Folding mesh shortcuts into outgoing filters saturates every node's filter toward the full network and destroys the upward=subtree / downward=complement directional asymmetry along tree edges. Ground truth: fips/docs/design/fips-bloom-filters.md lines 133-144 (tree-only merge rule), 147-154 (split-horizon still applies), 188-194 (per-peer table: out->A = Self+B, excludes mesh C). The all-peer OR-union remains correct and untouched for the separate mesh-size cardinality estimator (lines 340-359). Change: - bloom_snapshot() now filters active_neighbors down to tree peers before merging, reusing a new bloom_is_tree_peer() helper derived from the exact same parent/child classification used by bloom_peer_role(). Split-horizon exclusion of the destination peer is preserved; the send-to-all-neighbors path (who receives filters) is unchanged. - docs/graph-native-bloom.md corrected from "every peer view" to tree-only, citing the FIPS rule and noting the estimator path is intentionally distinct. Regenerated derived golden fixtures (m1/m2/m3/m4) whose snapshots shift under the corrected merge scope, via the blessed example generators. The pinned codec fixture is unaffected (merge scope, not wire sizes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbLLiWJfyxPXjxy2u6JxKd
These derived fixtures are only exercised by scripts/check.sh (not the cargo test suite): the m6 analysis index is computed from the m2 artifact, and the m8 release audit records artifact/analysis sha256 hashes. Both shift mechanically under the corrected tree-only bloom merge. Regenerated via the blessed CLI paths (analyze index / release audit); only the derived hashes changed (atlas_sha256 unchanged, confirming scope). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbLLiWJfyxPXjxy2u6JxKd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
bloom_snapshot()incrates/fips-engine/src/engine_bloom_events.rscomputed each node's OUTGOING bloom filter by OR-merging the stored filter view of every active neighbor (full-mesh) — iteratinggraph.active_neighbors(from)andunion_assign-ing every peer view except the destination.Real FIPS mandates a TREE-ONLY merge: only spanning-tree peers (parent + children) may be folded into an outgoing filter. Non-tree mesh peers' filters are stored/consulted for routing but MUST NOT be propagated transitively.
Why it matters
Folding mesh shortcuts into outgoing filters saturates every node's filter toward the full network (bloom saturation) and destroys the directional asymmetry along tree edges:
The full-mesh merge blurred both directions toward "everything", inflating FPR and cardinality and undermining the routing model.
Ground truth
fips/docs/design/fips-bloom-filters.md:The fix
bloom_snapshot()now filtersactive_neighbors(from)down to tree peers before merging, via a newbloom_is_tree_peer()helper derived from the exact same parent/child classificationbloom_peer_role()uses (no duplicated/subtly-different rule).tois still excluded from the merge.bloom_modelin the recovery path) is untouched.docs/graph-native-bloom.mdcorrected from "every peer view" to tree-only, citing the FIPS rule.Tests
cargo test --workspace: 152 passed, 0 failed.cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings: clean.scripts/check-fips-codecs.sh: clean — pinnedfixtures/codecs/fips-80c956a.jsonunaffected (this fix is merge scope, not wire sizes).Regenerated goldens
The corrected merge scope shifts bloom snapshots, which ripple into derived artifacts. Regenerated via the blessed example generators (
generate_m1_fixtures,generate_m2_fixtures,regenerate_m3_fixtures,generate_m4_fixtures) — no hand-edited bytes:fixtures/m1/root-ratchet-12-artifact.json,-reproduction.jsonfixtures/m2/root-ratchet-recovery-artifact.json,-reproduction.jsonfixtures/m3/search-result.json,shrink-result.json,fixtures/corpus/m3-root-ratchet/reproduction.jsonfixtures/m4/billion-demo.json,hybrid-anomaly.jsonReproducibility/logic assertions (e.g.
assert_eq!(first, second), schema validation, all-assertions-pass) remained green; only the derived bit-for-bit golden comparisons required regeneration.🤖 Generated with Claude Code
https://claude.ai/code/session_01MbLLiWJfyxPXjxy2u6JxKd