Skip to content

Conversation

@sanity
Copy link
Collaborator

@sanity sanity commented Dec 10, 2025

Problem

The ring module had low test coverage in several key files:

  • connection_manager.rs: 22.1%
  • peer_key_location.rs: 18.0%
  • location.rs: 64.0%
  • score.rs: 0% (not even included in mod.rs)

Additionally, the Score type's Ord implementation had a potential bug where partial_cmp().unwrap_or(Equal) would treat NaN values as equal to everything, violating Ord requirements.

Solution

Bug Fix

  • Fixed Score::Ord to use total_cmp() for consistent NaN handling. The previous implementation would cause undefined behavior in sorting algorithms when NaN values were present.

New Tests (~100 tests added)

score.rs (8 tests):

  • Normal value ordering, negative values, NaN handling, infinity handling
  • Sorting with mixed values (verifies NaN sorts consistently)

location.rs (~35 tests):

  • Distance calculations (wrapping at 0.5, normalization)
  • Location arithmetic (add distance, boundary conditions)
  • IPv4/IPv6 address mapping (Sybil mitigation via subnet masking)
  • Hash consistency for HashMap/HashSet usage
  • Contract key location computation

peer_key_location.rs (~25 tests):

  • PeerAddr variants (Unknown/Known)
  • Ordering (unknown addresses sort last)
  • Display formatting
  • IPv6 support
  • Serialization

connection_manager.rs (~30 tests):

  • Initial state and configuration (gateway vs regular mode)
  • should_accept logic (min/max connections, already connected peers)
  • Transient connection budget enforcement
  • Routing with skip lists
  • Peer identity updates

Test Plan

  • All 148 ring module tests pass
  • Clippy passes with no warnings
  • Code formatted

[AI-assisted - Claude]

@sanity sanity enabled auto-merge December 10, 2025 17:36
sanity and others added 2 commits December 11, 2025 16:15
- Fix Score::Ord to use total_cmp() for consistent NaN handling
  (previous implementation would treat NaN as equal to everything)
- Add score.rs to mod.rs exports
- Add ~100 new tests covering:
  - score.rs: ordering, NaN/infinity handling, sorting
  - location.rs: distance calculations, IPv4/IPv6 mapping, hashing
  - peer_key_location.rs: PeerAddr variants, ordering, serialization
  - connection_manager.rs: should_accept logic, transient connections,
    routing, peer identity updates

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Per review feedback - the Score type wasn't actually used anywhere
in the codebase, so remove it rather than keeping dead code with
allow(dead_code) attributes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sanity sanity added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 9f7b93a Dec 12, 2025
8 checks passed
@sanity sanity deleted the ring-unit-tests branch December 12, 2025 03:25
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.

3 participants