mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-07-24 22:45:20 +00:00
fix: Ensure empty neighbor lists in newer announcements clear mesh edges (#620)
* fix: Ensure empty neighbor lists in newer announcements clear mesh edges - Refactored MeshGraphService.updateFromAnnouncement to prioritize timestamp checks. - Treated null neighbor lists (omitted TLV) as empty lists to allow peer disconnection/isolation updates to propagate. - Added MeshGraphServiceTest to verify timestamp logic and edge eviction. * fix(test): Use TestOnly API to reset singleton instead of reflection - Added MeshGraphService.resetForTesting() - Updated MeshGraphServiceTest to use the new API, avoiding fragile reflection on companion object fields.
This commit is contained in:
callebtc
@@ -33,23 +33,20 @@ class MeshGraphService private constructor() {
|
||||
// Always update nickname if provided
|
||||
if (originNickname != null) nicknames[originPeerID] = originNickname
|
||||
|
||||
// If no neighbors TLV present, do not modify edges or timestamps
|
||||
if (neighborsOrNull == null) {
|
||||
publishSnapshot()
|
||||
return
|
||||
}
|
||||
|
||||
// Newer-only replacement per origin (based on TLV-bearing announcements only)
|
||||
// 1. Check timestamp first to ensure this is the latest word from the peer
|
||||
val prevTs = lastUpdate[originPeerID]
|
||||
if (prevTs != null && prevTs >= timestamp) {
|
||||
// Older or equal TLV-bearing update: ignore
|
||||
// Older or equal update: ignore
|
||||
return
|
||||
}
|
||||
lastUpdate[originPeerID] = timestamp
|
||||
|
||||
// Update what originPeerID announces
|
||||
// 2. Latest announcement determines state.
|
||||
// If neighborsOrNull is null (TLV omitted), it means the peer is not reporting any neighbors (empty list).
|
||||
val neighbors = neighborsOrNull ?: emptyList()
|
||||
|
||||
// Filter out self-loops just in case
|
||||
val newSet = neighborsOrNull.distinct().take(10).filter { it != originPeerID }.toSet()
|
||||
val newSet = neighbors.distinct().take(10).filter { it != originPeerID }.toSet()
|
||||
announcements[originPeerID] = newSet
|
||||
|
||||
publishSnapshot()
|
||||
@@ -119,5 +116,12 @@ class MeshGraphService private constructor() {
|
||||
fun getInstance(): MeshGraphService = INSTANCE ?: synchronized(this) {
|
||||
INSTANCE ?: MeshGraphService().also { INSTANCE = it }
|
||||
}
|
||||
|
||||
@org.jetbrains.annotations.TestOnly
|
||||
fun resetForTesting() {
|
||||
synchronized(this) {
|
||||
INSTANCE = null
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
package com.bitchat.android.services.meshgraph
|
||||
|
||||
import org.junit.Assert.*
|
||||
import org.junit.Test
|
||||
import org.junit.Before
|
||||
|
||||
class MeshGraphServiceTest {
|
||||
|
||||
private lateinit var service: MeshGraphService
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
// Use the test-only API to reset the singleton state safely
|
||||
MeshGraphService.resetForTesting()
|
||||
service = MeshGraphService.getInstance()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testUpdateFromAnnouncement_AddsNeighbors() {
|
||||
val origin = "PeerA"
|
||||
val neighbors = listOf("PeerB", "PeerC")
|
||||
val timestamp = 100UL
|
||||
|
||||
service.updateFromAnnouncement(origin, "Alice", neighbors, timestamp)
|
||||
|
||||
val snapshot = service.graphState.value
|
||||
// Verify nodes
|
||||
assertTrue(snapshot.nodes.any { it.peerID == "PeerA" })
|
||||
assertTrue(snapshot.nodes.any { it.peerID == "PeerB" })
|
||||
assertTrue(snapshot.nodes.any { it.peerID == "PeerC" })
|
||||
|
||||
// Verify edges (unconfirmed because B and C haven't announced A)
|
||||
// A -> B
|
||||
val edgeAB = snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerB") || (it.a == "PeerB" && it.b == "PeerA") }
|
||||
assertNotNull(edgeAB)
|
||||
assertFalse(edgeAB!!.isConfirmed)
|
||||
assertEquals("PeerA", edgeAB.confirmedBy)
|
||||
|
||||
// A -> C
|
||||
val edgeAC = snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerC") || (it.a == "PeerC" && it.b == "PeerA") }
|
||||
assertNotNull(edgeAC)
|
||||
assertFalse(edgeAC!!.isConfirmed)
|
||||
assertEquals("PeerA", edgeAC.confirmedBy)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testUpdateFromAnnouncement_NewerTimestampReplacesNeighbors() {
|
||||
val origin = "PeerA"
|
||||
|
||||
// Initial state: A -> {B, C}
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerB", "PeerC"), 100UL)
|
||||
|
||||
// Update: A -> {B, D} (newer timestamp)
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerB", "PeerD"), 200UL)
|
||||
|
||||
val snapshot = service.graphState.value
|
||||
|
||||
// Verify Edge A-B exists
|
||||
assertNotNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerB") || (it.a == "PeerB" && it.b == "PeerA") })
|
||||
|
||||
// Verify Edge A-D exists
|
||||
assertNotNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerD") || (it.a == "PeerD" && it.b == "PeerA") })
|
||||
|
||||
// Verify Edge A-C does NOT exist
|
||||
assertNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerC") || (it.a == "PeerC" && it.b == "PeerA") })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testUpdateFromAnnouncement_OlderTimestampIsIgnored() {
|
||||
val origin = "PeerA"
|
||||
|
||||
// Initial state: A -> {B, C} at ts=200
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerB", "PeerC"), 200UL)
|
||||
|
||||
// Old Update: A -> {D} at ts=100
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerD"), 100UL)
|
||||
|
||||
val snapshot = service.graphState.value
|
||||
|
||||
// Should still be {B, C}
|
||||
assertNotNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerB") || (it.a == "PeerB" && it.b == "PeerA") })
|
||||
assertNotNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerC") || (it.a == "PeerC" && it.b == "PeerA") })
|
||||
assertNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerD") || (it.a == "PeerD" && it.b == "PeerA") })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testUpdateFromAnnouncement_NullNeighborsClearsList_TheFix() {
|
||||
val origin = "PeerA"
|
||||
|
||||
// Initial state: A -> {B, C} at ts=100
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerB", "PeerC"), 100UL)
|
||||
|
||||
// Update with NULL neighbors (omitted TLV) at ts=200
|
||||
service.updateFromAnnouncement(origin, "Alice", null, 200UL)
|
||||
|
||||
val snapshot = service.graphState.value
|
||||
|
||||
// All edges from A should be gone
|
||||
val edgesFromA = snapshot.edges.filter { it.a == "PeerA" || it.b == "PeerA" }
|
||||
assertTrue("Edges from PeerA should be empty after null update", edgesFromA.isEmpty())
|
||||
|
||||
// Nodes B and C might still exist if they were added to the node list, but connected edges are gone.
|
||||
// Actually, publishSnapshot collects nodes from nicknames and announcements.
|
||||
// Since we provided nicknames for PeerA, it should be there.
|
||||
// PeerB and PeerC were only in announcements. Since A's announcement is cleared, and B/C never announced,
|
||||
// they might disappear from the node list if 'nicknames' doesn't contain them.
|
||||
// Let's check edges primarily as that's what routing cares about.
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testUpdateFromAnnouncement_NullNeighborsWithOlderTimestampIsIgnored() {
|
||||
val origin = "PeerA"
|
||||
|
||||
// Initial state: A -> {B, C} at ts=200
|
||||
service.updateFromAnnouncement(origin, "Alice", listOf("PeerB", "PeerC"), 200UL)
|
||||
|
||||
// Old Update with NULL neighbors at ts=100
|
||||
service.updateFromAnnouncement(origin, "Alice", null, 100UL)
|
||||
|
||||
val snapshot = service.graphState.value
|
||||
|
||||
// Should still be {B, C} because the null update was older
|
||||
assertFalse(snapshot.edges.isEmpty())
|
||||
assertNotNull(snapshot.edges.find { (it.a == "PeerA" && it.b == "PeerB") || (it.a == "PeerB" && it.b == "PeerA") })
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user