Skip to content

fix: make trust scores effective in find_closest_nodes#4

Merged
dirvine merged 1 commit intosaorsa-labs:mainfrom
maqi:trust_weighted_find_closest
Jan 28, 2026
Merged

fix: make trust scores effective in find_closest_nodes#4
dirvine merged 1 commit intosaorsa-labs:mainfrom
maqi:trust_weighted_find_closest

Conversation

@maqi
Copy link
Contributor

@maqi maqi commented Jan 28, 2026

Summary

  • Fixed a bug where trust scores were never actually used in find_closest_nodes()
  • The previous implementation sorted by full XOR distance first, but since peer IDs are unique, distances are always unique - making trust scores dead code
  • Introduced distance bucketing using leading zero bits as 'distance magnitude' to group similarly-close nodes
  • Within each magnitude bucket, nodes are now sorted by trust (desc) then RTT (asc)

Problem

The original code:

a_distance.cmp(&b_distance)
    .then_with(|| /* trust comparison */)
    .then_with(|| /* RTT comparison */)

Since XOR distances between unique peer IDs and any target are always unique, the then_with closures for trust and RTT were never executed.

Solution

Use distance magnitude (256 - leading_zeros) as a coarse grouping:

  • Nodes with the same magnitude are within a factor of 2 in actual distance
  • This preserves Kademlia's convergence properties
  • Trust now meaningfully influences selection among similarly-close nodes

Test plan

  • Verify existing tests pass
  • Manual verification that trust scores affect node selection within same distance magnitude

The previous implementation sorted candidates by full XOR distance first,
then by trust score as a tiebreaker. Since peer IDs are unique, XOR
distances are always unique, meaning trust scores were never actually
used in selection.

This fix introduces distance bucketing using leading zero bits as
"distance magnitude". Nodes within the same magnitude (same number of
leading zeros) are within a factor of 2 in actual distance, making them
effectively equivalent from a Kademlia routing perspective.

Within each magnitude bucket, nodes are now sorted by trust (descending)
then RTT (ascending), allowing trust to meaningfully influence which
nodes are selected while preserving Kademlia's convergence properties.
dirvine added a commit that referenced this pull request Jan 28, 2026
…ghted routing

This commit adds extensive test coverage and documentation improvements on top
of PR #4's fix for trust score usage in find_closest_nodes().

## Test Coverage Added (5 new tests):

1. **test_trust_weighted_find_closest_nodes**: Verifies trust system integration
   and that trust scores influence routing decisions within distance magnitude buckets

2. **test_distance_magnitude_bucketing**: Tests XOR distance calculation and
   magnitude bucketing properties (factor of 2 grouping)

3. **test_find_closest_self_lookup**: Edge case testing for self-lookup
   (all-zero distance)

4. **test_distance_overrides_trust**: Security test verifying that distance
   always takes precedence over trust scores, preventing routing manipulation

5. **test_find_closest_nodes_performance**: Performance regression test ensuring
   trust calculation doesn't impact lookup speed

## Documentation Improvements:

### Security Properties (module-level docs):
- Distance-first routing guarantees
- Trust as tiebreaker within magnitude buckets
- Sybil resistance through distance precedence
- No routing centralization risk

### Edge Case Documentation:
- Self-lookup behavior (distance = 0)
- Maximum distance behavior (distance = 256)
- Saturating arithmetic to prevent underflow

### Implementation Notes:
- Factor of 2 granularity preserves Kademlia's O(log n) convergence
- Trust manipulation cannot override distance ordering

All new tests pass. Two pre-existing test failures remain (unrelated to this PR).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
dirvine added a commit that referenced this pull request Jan 28, 2026
…ests

Merges PR #4 from @maqi which fixes the bug where trust scores were never
actually used in find_closest_nodes() due to unique XOR distances.

Original contribution introduced distance magnitude bucketing using leading
zero bits, allowing trust to influence selection within similarly-distant nodes.

Additional improvements committed on top:
- 5 comprehensive tests verifying trust integration, edge cases, and security
- Module-level security documentation
- Performance regression testing
- Edge case documentation (self-lookup, saturation arithmetic)

All quality checks pass (fmt, clippy, new tests).

Thanks to @maqi for identifying and fixing this subtle but important routing bug!

Co-Authored-By: qima <qi.ma@maidsafe.net>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@dirvine dirvine merged commit 7251ed9 into saorsa-labs:main Jan 28, 2026
@dirvine
Copy link
Collaborator

dirvine commented Jan 28, 2026

Thank you @maqi for this excellent bug fix! 🎉

You correctly identified a subtle but important issue where trust scores were dead code due to unique XOR distances. The distance magnitude bucketing solution is elegant and preserves Kademlia's routing properties while enabling trust-weighted selection.

What we merged:

Your contribution:

  • Distance magnitude bucketing using leading zero bits
  • Trust as tiebreaker within magnitude buckets
  • Clear problem analysis and solution rationale

Additional improvements we added on top:

  • 5 comprehensive tests covering:
    • Trust system integration verification
    • Distance magnitude bucketing properties
    • Edge cases (self-lookup, all-zero/all-ones distances)
    • Security: distance always overrides trust
    • Performance regression testing
  • Security documentation explaining threat model and Sybil resistance
  • Edge case documentation for distance_magnitude() function
  • Saturating arithmetic to prevent underflow edge cases

Verification:

  • ✅ All new tests pass (5/5)
  • ✅ Zero clippy warnings
  • ✅ Code formatting passes
  • ✅ No performance regression detected

Your fix is now merged to main! The trust system will now meaningfully reduce timeouts and improve reliability by preferring trusted nodes among similarly-distant peers.

Thanks again for the contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants