diff --git a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt index 7293e27..9a17f79 100644 --- a/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/DriverRoadflareRepository.kt @@ -440,6 +440,23 @@ class DriverRoadflareRepository(context: Context) { return _state.value?.followers?.any { it.pubkey == pubkey && it.mutedAt != null } ?: false } + /** + * True if the rider is muted via either path (issue #82). Single helper so receive-side + * filters across the codebase use the same check and can't drift. Callers: + * - `DriverViewModel.processIncomingOffer` — Kind 3173 direct/RoadFlare ride offers + * - `DriverViewModel.subscribeToBroadcastRequests` — Kind 3173 broadcast offers + * - `RoadflareListenerService.subscribeToRoadflareRequests` — Kind 3173 RoadFlare offers + * reaching the foreground service + * - `RoadflareListenerService.processPingEvent` — Kind 3189 driver pings + * - `MainActivity` Kind 3188 ack handler — refuses key re-delivery to muted riders + * + * Kind 3187 (`RoadflareKeyManager.handleFollowNotification`) deliberately calls + * [isMuted] and [isFollowerMuted] separately because the two outcomes map to + * distinct return values (`AlreadyMuted` vs `AlreadyLightMuted`) — collapsing + * to `isAnyMuted` there would lose the semantic distinction the caller depends on. + */ + fun isAnyMuted(pubkey: String): Boolean = isMuted(pubkey) || isFollowerMuted(pubkey) + // In-memory flag (resets across process restarts) tracking whether the lightweight-mute // reconciliation against Kind 30177 has run at least once this session. // diff --git a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt index d040e4c..27d1cc0 100644 --- a/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt +++ b/common/src/main/java/com/ridestr/common/data/FollowedDriversRepository.kt @@ -7,6 +7,7 @@ import com.ridestr.common.nostr.events.RoadflareKey import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import org.json.JSONArray import org.json.JSONObject @@ -25,6 +26,27 @@ data class CachedDriverLocation( val keyVersion: Int = 0 ) +/** + * Cached driver presence — read from the **public** `status` tag on Kind 30014 events + * (no decryption required). Issue #82: drives the rider's "this driver is available + * even though I can't see their location" UX so a stale or missing RoadFlare key never + * blocks ride-on-demand. + * + * Distinct from [CachedDriverLocation] which carries the decrypted lat/lon/timestamp + * from the event content. Presence is a strict subset that any follower can read + * regardless of key state — the protocol exposes `status` publicly via the event tags + * (see `RoadflareLocationEvent.create()` line 86 + `getStatus()` helper line 163). + * + * @param status `"online"` / `"on_ride"` / `"offline"` from the public tag + * @param timestamp `event.createdAt` of the latest 30014 event for this driver + * @param keyVersion Driver's current RoadFlare key version, also from a public tag + */ +data class CachedDriverPresence( + val status: String, + val timestamp: Long, + val keyVersion: Int = 0 +) + /** * Repository for managing rider's followed drivers list for RoadFlare. * Stores drivers and their RoadFlare decryption keys in SharedPreferences. @@ -76,6 +98,48 @@ class FollowedDriversRepository(context: Context) { _driverLocations.value = emptyMap() } + /** + * Cached driver presence — populated from the public `status` tag on Kind 30014 + * regardless of whether the encrypted content can be decrypted. Issue #82. + */ + private val _driverPresence = MutableStateFlow>(emptyMap()) + val driverPresence: StateFlow> = _driverPresence.asStateFlow() + + /** + * Update a driver's cached presence from a Kind 30014 event's PUBLIC tags. + * Skips the update if an existing entry has a newer-or-equal timestamp (out-of-order + * delivery + same-event multi-relay dedup). + * + * Issue #82: uses [kotlinx.coroutines.flow.MutableStateFlow.update] so the read / + * timestamp-compare / write executes atomically under CAS. Direct + * `_driverPresence.value =` would race when multiple relay callbacks (`Dispatchers.IO`) + * update the same pubkey concurrently — both reading the same `existing` before either + * writes — and the lower-timestamp write could win, defeating the guard. The other + * in-memory caches in this file use the simpler unprotected pattern; this one is held + * to a stricter standard because the PR's KDoc explicitly advertises the out-of-order + * guard as a correctness property. + */ + fun updateDriverPresence(pubkey: String, status: String, timestamp: Long, keyVersion: Int = 0) { + _driverPresence.update { current -> + val existing = current[pubkey] + if (existing != null && existing.timestamp >= timestamp) { + current + } else { + current + (pubkey to CachedDriverPresence(status, timestamp, keyVersion)) + } + } + } + + /** Remove a driver's cached presence. */ + fun removeDriverPresence(pubkey: String) { + _driverPresence.value = _driverPresence.value - pubkey + } + + /** Clear all cached driver presence. */ + fun clearDriverPresence() { + _driverPresence.value = emptyMap() + } + /** * Load cached driver names from SharedPreferences. */ @@ -191,6 +255,11 @@ class FollowedDriversRepository(context: Context) { } // Remove from location cache (in-memory only, no persist needed) _driverLocations.value = _driverLocations.value - pubkey + // Issue #82: also remove the presence entry so a re-add of the same driver + // in the same session doesn't see a stale `(timestamp >= timestamp)` guard + // hit and silently render the re-added driver as offline until their next + // broadcast tick. + _driverPresence.value = _driverPresence.value - pubkey } /** @@ -268,7 +337,7 @@ class FollowedDriversRepository(context: Context) { } /** - * Clear all followed drivers and cached names/locations (for logout). + * Clear all followed drivers and cached names/locations/presence (for logout). */ fun clearAll() { prefs.edit() @@ -278,6 +347,10 @@ class FollowedDriversRepository(context: Context) { _drivers.value = emptyList() _driverNames.value = emptyMap() _driverLocations.value = emptyMap() + // Issue #82: presence has the same in-memory-only semantic as locations and + // belongs in the logout reset path so the next user's session doesn't inherit + // stale presence data from the previous identity. + _driverPresence.value = emptyMap() } companion object { diff --git a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt index d98a197..559b0d3 100644 --- a/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt +++ b/common/src/main/java/com/ridestr/common/roadflare/RoadflareDriverPresenceCoordinator.kt @@ -48,22 +48,62 @@ class RoadflareDriverPresenceCoordinator( locationSubId?.let { nostrService.closeRoadflareSubscription(it) } locationSubId = null - val withKeys = drivers.filter { it.roadflareKey != null } - if (withKeys.isEmpty()) { - Log.d(TAG, "No drivers with keys to subscribe to (total: ${drivers.size})") + // Issue #82: prune lastLocationCreatedAt for drivers no longer in the list. Without + // this, removing + re-adding the same driver in a single session leaves the prior + // session's `lastLocationCreatedAt[pubkey]` intact, which then gates the same Kind + // 30014 event (createdAt unchanged within the 5-min TTL window) at the + // `eventCreatedAt < lastSeen` boundary — driver shows as offline until the next + // broadcast tick, defeating the stale-key UX in a real scenario. + val currentPubkeys = drivers.map { it.pubkey }.toSet() + lastLocationCreatedAt.keys.retainAll(currentPubkeys) + + // Issue #82: subscribe to ALL followed drivers, not just those with a current key. + // The PUBLIC `status` tag on Kind 30014 lets us track availability for stale-key / + // missing-key drivers without ever decrypting the encrypted lat/lon content. The + // decryption attempt below is best-effort — if it fails we still surface the driver + // as available via `updateDriverPresence`, so the rider can fall back to the + // rider-route fare and request the ride anyway. + if (drivers.isEmpty()) { + Log.d(TAG, "No followed drivers to subscribe to") return } - val driverPubkeys = withKeys.map { it.pubkey } - Log.d(TAG, "Subscribing to ${driverPubkeys.size} driver locations") + val driverPubkeys = drivers.map { it.pubkey } + val withKeysCount = drivers.count { it.roadflareKey != null } + Log.d(TAG, "Subscribing to ${driverPubkeys.size} driver locations ($withKeysCount with keys)") locationSubId = nostrService.subscribeToRoadflareLocations(driverPubkeys) { event, relayUrl -> val driverPubKey = event.pubKey - val driver = withKeys.find { it.pubkey == driverPubKey } - val roadflareKey = driver?.roadflareKey + val eventCreatedAt = event.createdAt + val isExpired = RoadflareLocationEvent.isExpired(event) + val lastSeen = lastLocationCreatedAt[driverPubKey] ?: 0L + val isOutOfOrder = eventCreatedAt < lastSeen + if (isExpired || isOutOfOrder) { + Log.d(TAG, "Rejected stale/out-of-order 30014 from ${driverPubKey.take(8)}: expired=$isExpired, outOfOrder=$isOutOfOrder") + return@subscribeToRoadflareLocations + } + + // Mark this event as the latest seen for the driver, regardless of decryption + // outcome — the out-of-order guard at the top now applies to presence-only + // events too, so a stale Kind 30014 can't overwrite a fresher presence update. + lastLocationCreatedAt[driverPubKey] = eventCreatedAt + + // Always update presence from the PUBLIC tags — works regardless of key state. + val publicStatus = RoadflareLocationEvent.getStatus(event) + val publicKeyVersion = RoadflareLocationEvent.getKeyVersion(event) + followedDriversRepository.updateDriverPresence( + pubkey = driverPubKey, + status = publicStatus, + timestamp = eventCreatedAt, + keyVersion = publicKeyVersion + ) + + // Best-effort location decryption — only succeeds if we have the current key. + val driver = drivers.find { it.pubkey == driverPubKey } + val roadflareKey = driver?.roadflareKey if (roadflareKey == null) { - Log.w(TAG, "No RoadFlare key for driver ${driverPubKey.take(8)}") + Log.d(TAG, "No RoadFlare key for ${driverPubKey.take(8)} — presence-only update") return@subscribeToRoadflareLocations } @@ -74,27 +114,19 @@ class RoadflareDriverPresenceCoordinator( ) if (locationData != null) { - val eventCreatedAt = event.createdAt - - val isExpired = RoadflareLocationEvent.isExpired(event) - val lastSeen = lastLocationCreatedAt[driverPubKey] ?: 0L - val isOutOfOrder = eventCreatedAt < lastSeen - - if (!isExpired && !isOutOfOrder) { - lastLocationCreatedAt[driverPubKey] = eventCreatedAt - followedDriversRepository.updateDriverLocation( - pubkey = driverPubKey, - lat = locationData.location.lat, - lon = locationData.location.lon, - status = locationData.tagStatus, - timestamp = eventCreatedAt, - keyVersion = locationData.keyVersion - ) - } else { - Log.d(TAG, "Rejected stale/out-of-order 30014 from ${driverPubKey.take(8)}: expired=$isExpired, outOfOrder=$isOutOfOrder") - } + followedDriversRepository.updateDriverLocation( + pubkey = driverPubKey, + lat = locationData.location.lat, + lon = locationData.location.lon, + status = locationData.tagStatus, + timestamp = eventCreatedAt, + keyVersion = locationData.keyVersion + ) } else { - Log.w(TAG, "Failed to decrypt location from ${driverPubKey.take(8)}") + // Decryption failed despite having a key — likely stale (key was rotated). + // Presence already updated above; rider will see the driver as available + // and fall through to the rider-route fare path on offer-send. + Log.w(TAG, "Failed to decrypt location from ${driverPubKey.take(8)} (presence-only, likely stale key)") } } } @@ -121,6 +153,9 @@ class RoadflareDriverPresenceCoordinator( locationSubId?.let { nostrService.closeRoadflareSubscription(it) } locationSubId = null lastLocationCreatedAt.clear() + // Don't clear presence/locations from the repository here — other ViewModel + // observers may briefly read them across configuration changes. The repository + // owns its own clear lifecycle (e.g., on logout). } companion object { diff --git a/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt new file mode 100644 index 0000000..0746dc0 --- /dev/null +++ b/common/src/test/java/com/ridestr/common/data/FollowedDriversRepositoryPresenceTest.kt @@ -0,0 +1,178 @@ +package com.ridestr.common.data + +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Tests for issue #82's lightweight presence channel — the public-tag-only path on + * Kind 30014 that lets the rider know a driver is available even when their stored + * RoadFlare key can't decrypt the encrypted lat/lon content. + * + * Robolectric provides Android Context for SharedPreferences. Presence itself is + * in-memory only (matches `CachedDriverLocation`'s ephemeral semantic) but the repo + * needs Context to construct. + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [28]) +class FollowedDriversRepositoryPresenceTest { + + private lateinit var repo: FollowedDriversRepository + + @Before + fun setUp() { + repo = FollowedDriversRepository(ApplicationProvider.getApplicationContext()) + } + + @Test + fun `driverPresence starts empty`() { + assertTrue(repo.driverPresence.value.isEmpty()) + } + + @Test + fun `updateDriverPresence stores entry keyed by pubkey`() { + repo.updateDriverPresence(pubkey = "rider-A", status = "online", timestamp = 1_000L, keyVersion = 5) + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("online", entry?.status) + assertEquals(1_000L, entry?.timestamp) + assertEquals(5, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence overwrites with newer timestamp`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 6) + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("on_ride", entry?.status) + assertEquals(2_000L, entry?.timestamp) + assertEquals(6, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence skips out-of-order updates`() { + // Issue #82: protocol guarantee: Kind 30014 events have monotonic createdAt per driver + // when delivered in order, but relay reordering can deliver an older event after a newer + // one. The repository must reject the older one to keep presence consistent. + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 6) + repo.updateDriverPresence("rider-A", "online", 1_500L, 5) // older — must be skipped + + val entry = repo.driverPresence.value["rider-A"] + assertEquals("on_ride", entry?.status) + assertEquals(2_000L, entry?.timestamp) + assertEquals(6, entry?.keyVersion) + } + + @Test + fun `updateDriverPresence skips equal-timestamp duplicate`() { + // Same event arriving from two relays must not overwrite the first. + repo.updateDriverPresence("rider-A", "online", 2_000L, 5) + repo.updateDriverPresence("rider-A", "on_ride", 2_000L, 5) + + // The first wins because the second is not strictly newer. + assertEquals("online", repo.driverPresence.value["rider-A"]?.status) + } + + @Test + fun `removeDriverPresence drops entry`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.removeDriverPresence("rider-A") + assertNull(repo.driverPresence.value["rider-A"]) + } + + @Test + fun `clearDriverPresence wipes all entries`() { + repo.updateDriverPresence("rider-A", "online", 1_000L) + repo.updateDriverPresence("rider-B", "on_ride", 1_500L) + repo.clearDriverPresence() + assertTrue(repo.driverPresence.value.isEmpty()) + } + + @Test + fun `driverPresence and driverLocations are independent state flows`() { + repo.updateDriverPresence("rider-A", "online", 1_000L, 5) + repo.updateDriverLocation("rider-B", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + assertTrue("rider-A appears in presence", repo.driverPresence.value.containsKey("rider-A")) + assertFalse("rider-A NOT in locations", repo.driverLocations.value.containsKey("rider-A")) + assertTrue("rider-B appears in locations", repo.driverLocations.value.containsKey("rider-B")) + assertFalse("rider-B NOT in presence (writes are independent)", repo.driverPresence.value.containsKey("rider-B")) + } + + @Test + fun `clearAll wipes the presence channel along with locations`() { + // Issue #82 pass-1 fix: presence has the same in-memory-only semantic as locations + // and MUST be cleared on logout so the next user's session doesn't inherit stale + // data from the previous identity. The test pins this so a future refactor that + // adds another in-memory state can't silently regress the cleanup. + repo.updateDriverPresence("driver-A", "online", 1_000L, 5) + repo.updateDriverPresence("driver-B", "on_ride", 1_500L, 6) + repo.updateDriverLocation("driver-C", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + repo.clearAll() + + assertTrue("presence map must be empty after clearAll", repo.driverPresence.value.isEmpty()) + assertTrue("locations map must be empty after clearAll", repo.driverLocations.value.isEmpty()) + } + + @Test + fun `removeDriver clears the presence entry for that pubkey`() { + // Issue #82 pass-3 fix: removeDriver was clearing names + locations but not presence. + // The leak meant a re-add of the same driver in a single session could see the + // `(timestamp >= timestamp)` guard hit on the same Kind 30014 event (still within + // its 5-min relay TTL), rendering the re-added driver as offline until the next + // broadcast tick. Pin the cleanup symmetric with locations. + // (Note: this repo doesn't expose addDriver as a unit-test seam — `_drivers` is only + // mutated via the wider Kind 30011 sync path. The presence clear is unconditional on + // pubkey match, so test it directly without going through the drivers list.) + repo.updateDriverPresence("driver-A", "online", 1_000L, 5) + repo.updateDriverPresence("driver-B", "online", 1_000L, 5) + repo.updateDriverLocation("driver-A", lat = 36.0, lon = -115.0, status = "online", timestamp = 1_000L) + + repo.removeDriver("driver-A") + + assertNull("driver-A presence must be cleared", repo.driverPresence.value["driver-A"]) + assertNull("driver-A location must be cleared", repo.driverLocations.value["driver-A"]) + assertNotNull("driver-B presence must NOT be touched", repo.driverPresence.value["driver-B"]) + } + + @Test + fun `updateDriverPresence converges to highest timestamp under concurrent updates`() { + // Issue #82 pass-3 fix: updateDriverPresence used direct `_driverPresence.value =` + // assignment with a non-atomic read-then-check-then-write guard. Two relay threads + // calling updateDriverPresence for the same pubkey with different timestamps could + // both pass the `existing.timestamp >= timestamp` guard (both reading the same + // `existing` before either writes), and the lower-timestamp write could win the + // StateFlow assignment. The fix uses StateFlow.update {} for CAS-retried + // read-modify-write. + // + // Sanity check: launch many concurrent updates with strictly-ascending timestamps + // (in shuffled order to avoid happens-before ordering by accident) and confirm the + // highest timestamp wins. Uses plain JVM threads to avoid coroutine-scope plumbing + // in a Robolectric test. + val concurrency = 64 + val timestamps = (1..concurrency).toList().shuffled() + val threads = timestamps.map { ts -> + Thread { repo.updateDriverPresence("driver-X", "online", ts.toLong(), 0) } + } + threads.forEach { it.start() } + threads.forEach { it.join() } + + val finalEntry = repo.driverPresence.value["driver-X"] + assertNotNull("driver-X presence must be set", finalEntry) + assertEquals( + "highest timestamp must win after concurrent updates", + concurrency.toLong(), + finalEntry?.timestamp + ) + } +} diff --git a/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt b/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt index 9a7c94e..d36fa32 100644 --- a/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt +++ b/common/src/test/java/com/ridestr/common/roadflare/RoadflareMuteTest.kt @@ -275,6 +275,32 @@ class RoadflareMuteTest { assertEquals(setOf("rider-A"), needing) } + @Test + fun `isAnyMuted returns true for either heavyweight or lightweight mute`() { + // Issue #82 helper — single check used by all four receive-side handlers + // (Kind 3173 / 3187 / 3188 / 3189). Must catch both mute paths. + seedFollower("rider-A", mutedAt = null) // unmuted baseline + seedFollower("rider-B", mutedAt = 100L) // lightweight only + seedFollower("rider-C", mutedAt = null) // will become heavyweight + seedFollower("rider-D", mutedAt = 100L) // both paths + + val current = repository.state.value!! + repository.restoreFromBackup( + current.copy( + muted = listOf( + MutedRider(pubkey = "rider-C", mutedAt = 50L), + MutedRider(pubkey = "rider-D", mutedAt = 50L) + ) + ) + ) + + assertFalse("unmuted rider must NOT be flagged", repository.isAnyMuted("rider-A")) + assertTrue("lightweight-muted rider must be flagged", repository.isAnyMuted("rider-B")) + assertTrue("heavyweight-muted rider must be flagged", repository.isAnyMuted("rider-C")) + assertTrue("rider muted via both paths must be flagged", repository.isAnyMuted("rider-D")) + assertFalse("unknown pubkey must NOT be flagged", repository.isAnyMuted("ghost-rider")) + } + @Test fun `lightweight mute is independent of heavyweight MutedRider`() { seedFollower("rider-A", mutedAt = 1L) diff --git a/drivestr/src/main/java/com/drivestr/app/MainActivity.kt b/drivestr/src/main/java/com/drivestr/app/MainActivity.kt index 46c690b..6e5ea96 100644 --- a/drivestr/src/main/java/com/drivestr/app/MainActivity.kt +++ b/drivestr/src/main/java/com/drivestr/app/MainActivity.kt @@ -478,13 +478,15 @@ fun DrivestrApp(settingsRepository: SettingsRepository) { android.util.Log.w("MainActivity", "Key ack pubkey mismatch - ignoring (claimed=${ackData.riderPubKey.take(8)}, signer=${event.pubKey.take(8)})") } else { // Verify authorized follower (approved + not muted via either path). - // - Heavyweight mute (`MutedRider` / "Remove" UX) excludes the rider entirely. - // - Lightweight mute (issue #80, `RoadflareFollower.mutedAt`) suppresses - // key delivery — re-sends in response to acks would defeat the mute. + // `isAnyMuted` covers BOTH heavyweight (`MutedRider` / "Remove" UX, + // which excludes the rider entirely) AND lightweight (issue #80, + // `RoadflareFollower.mutedAt`, which suppresses key delivery — re-sends + // in response to acks would defeat the mute). Single helper so the + // four receive-side handlers (Kind 3173 broadcast/direct, 3188, 3189) + // can't drift on which paths they cover. val follower = driverRoadflareRepo.getFollowers().find { it.pubkey == ackData.riderPubKey } - val isMuted = driverRoadflareRepo.getMutedPubkeys().contains(ackData.riderPubKey) - val isLightMuted = follower?.mutedAt != null - val isAuthorized = follower != null && follower.approved && !isMuted && !isLightMuted + val isAuthorized = follower != null && follower.approved && + !driverRoadflareRepo.isAnyMuted(ackData.riderPubKey) if (!isAuthorized) { android.util.Log.w("MainActivity", "Key ack from unauthorized follower - ignoring") diff --git a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt index 3d4081f..8d55b52 100644 --- a/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt +++ b/drivestr/src/main/java/com/drivestr/app/service/RoadflareListenerService.kt @@ -185,9 +185,6 @@ class RoadflareListenerService : Service() { } private fun subscribeToRoadflareRequests(driverPubKey: String) { - // Get muted pubkeys to filter - val mutedPubkeys = driverRoadflareRepo?.getMutedPubkeys()?.toSet() ?: emptySet() - subscriptionId = nostrService?.relayManager?.subscribe( kinds = listOf(RideshareEventKinds.RIDE_OFFER), tags = mapOf( @@ -198,8 +195,13 @@ class RoadflareListenerService : Service() { // Skip if already seen (atomic check-and-add) if (!seenRequests.add(event.id)) return@subscribe - // Skip if from muted rider - if (event.pubKey in mutedPubkeys) { + // Issue #82: live mute check via the unified `isAnyMuted` helper. + // Pre-fix this captured a `getMutedPubkeys()` snapshot at subscribe-open + // time (heavyweight only) — which had two compounding gaps: mutes applied + // after subscribe were invisible until service restart, AND lightweight-muted + // riders (`RoadflareFollower.mutedAt`) bypassed the check entirely. The + // live `isAnyMuted` call covers both paths and refreshes per event. + if (driverRoadflareRepo?.isAnyMuted(event.pubKey) == true) { Log.d(TAG, "Ignoring RoadFlare from muted rider ${event.pubKey.take(8)}") return@subscribe } @@ -264,8 +266,12 @@ class RoadflareListenerService : Service() { // requiring a service restart (unlike the Kind 3173 snapshot approach). // O(n) in muted-count per event; acceptable up to ~200 muted pubkeys. // Revisit with a cached snapshot if drivers report >1000 followers or high muted counts. - val mutedPubkeys = driverRoadflareRepo?.getMutedPubkeys() ?: emptySet() - if (event.pubKey in mutedPubkeys) { + // + // Issue #82: now checks BOTH mute paths (heavyweight MutedRider + lightweight + // RoadflareFollower.mutedAt) via the unified `isAnyMuted` helper. Pre-#82 only + // the heavyweight list was checked, so a lightweight-muted rider could still wake + // the driver up with a Kind 3189 ping. + if (driverRoadflareRepo?.isAnyMuted(event.pubKey) == true) { Log.d(TAG, "Discarding authenticated ping from muted rider ${event.pubKey.take(8)}") return } diff --git a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt index 7976cac..4a1c34d 100644 --- a/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt +++ b/drivestr/src/main/java/com/drivestr/app/viewmodels/DriverViewModel.kt @@ -3085,6 +3085,15 @@ class DriverViewModel @Inject constructor( val offerType = if (offer.isRoadflare) "RoadFlare" else "Direct" Log.d(TAG, "Received $offerType offer from ${offer.riderPubKey.take(8)}...") + // Issue #82 Scope A — silent mute filter. Drop offers from heavyweight-muted + // ("Removed") AND lightweight-muted riders. The rider sees no response (matches + // standard "soft block" UX from Twitter/Instagram) and can fall back to another + // app — explicit product decision: muted means muted, downgraded UX is acceptable. + if (driverRoadflareRepository.isAnyMuted(offer.riderPubKey)) { + Log.d(TAG, "Filtering offer from muted rider ${offer.riderPubKey.take(8)}...") + return + } + // Filter out offers we've already accepted (prevents duplicates after ride completion) if (offer.eventId in acceptedOfferEventIds) { Log.d(TAG, "Ignoring already-accepted offer: ${offer.eventId.take(8)}") @@ -3416,6 +3425,14 @@ class DriverViewModel @Inject constructor( Log.d(TAG, "Received broadcast ride request from ${request.riderPubKey.take(8)}, fare=${request.fareEstimate}") + // Issue #82 Scope A — silent mute filter for the broadcast Kind 3173 path. + // Mirrors the check in `processIncomingOffer` for the direct/RoadFlare path. + // Without this, a muted rider's broadcast request still lands in the inbox. + if (driverRoadflareRepository.isAnyMuted(request.riderPubKey)) { + Log.d(TAG, "Filtering broadcast request from muted rider ${request.riderPubKey.take(8)}...") + return@subscribeToBroadcastRideRequests + } + // Filter out requests we've already accepted if (request.eventId in acceptedOfferEventIds) { Log.d(TAG, "Ignoring already-accepted request: ${request.eventId.take(8)}") diff --git a/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt b/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt index 73efb9b..a0c8e38 100644 --- a/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt +++ b/rider-app/src/main/java/com/ridestr/rider/ui/screens/RiderModeScreen.kt @@ -2939,6 +2939,10 @@ private fun RoadflareDriverSelectionSheet( // Use repository cache for names and locations (survives sheet close/reopen) val cachedLocations by followedDriversRepository.driverLocations.collectAsState() + // Issue #82: presence channel — drives availability for drivers whose Kind 30014 + // location can't be decrypted (stale key, missing key). Read from the public + // `status` tag, no decryption required. + val cachedPresence by followedDriversRepository.driverPresence.collectAsState() val driverNames by followedDriversRepository.driverNames.collectAsState() // Location subscriptions and profile fetching are now managed by @@ -3047,14 +3051,27 @@ private fun RoadflareDriverSelectionSheet( val driverEntries = drivers.map { driver -> val cachedState = cachedLocations[driver.pubkey] + val cachedPres = cachedPresence[driver.pubkey] val driverLocation = cachedState?.let { Location(lat = it.lat, lon = it.lon) } val driverName = driverNames[driver.pubkey] ?: driver.pubkey.take(8) + "..." val hasKey = driver.roadflareKey != null - val isOnline = cachedState != null && + // Issue #82: a driver is "online" if EITHER the decrypted location says so + // (cachedState path — drivers with a fresh key) OR the public `status` tag + // on Kind 30014 says so (cachedPres path — drivers with stale or missing + // key). The presence-only path drives the stale-key offer flow: rider can + // still request the ride even though they can't see the driver's lat/lon. + val locationOnline = cachedState != null && cachedState.status == com.ridestr.common.nostr.events.RoadflareLocationEvent.Status.ONLINE && (nowSec - cachedState.timestamp) < staleThresholdSec - + val presenceOnline = cachedPres != null && + cachedPres.status == com.ridestr.common.nostr.events.RoadflareLocationEvent.Status.ONLINE && + (nowSec - cachedPres.timestamp) < staleThresholdSec + val isOnline = locationOnline || presenceOnline + + // Per-driver fare quote needs the actual location. When location is + // unavailable (stale key) we omit the quote — the offer-send path falls + // back to the rider-route fare via `state.fareEstimate` automatically. val quote = if (driverLocation != null && pickupLocation != null) { RoadflareFarePolicy.quoteDriver( pickupLat = pickupLocation.lat, @@ -3097,7 +3114,14 @@ private fun RoadflareDriverSelectionSheet( pickupMiles = quote?.pickupMiles, fareState = quote?.fareState ?: FareState.ESTIMATED, isTooFar = isTooFar, - isBroadcastEligible = hasKey && isOnline && !isTooFar, + // Broadcast requires a known location for the geographic too-far cap + // — `sendRoadflareToAll` filters out null-location drivers anyway. + // Use the strict `locationOnline` here so the count + button match + // what the batch will actually attempt. + isBroadcastEligible = hasKey && locationOnline && !isTooFar, + // Direct selection works on the looser `isOnline` (location OR + // presence). Sending an offer with `driverLocation = null` falls + // back to the rider-route fare via `state.fareEstimate`. isDirectSelectable = hasKey && !isSending ) ) diff --git a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt index 461d787..5d98301 100644 --- a/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt +++ b/rider-app/src/main/java/com/ridestr/rider/viewmodels/RiderViewModel.kt @@ -1826,14 +1826,58 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer to ${driverPubKey.take(16)}: $eventId") + // Acceptance subscription MUST be armed before any further round-trip + // so a fast-responding driver's Kind 3174 isn't missed. setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) applyOfferSuccessState(params, eventId) + // Issue #82: stale-key piggyback. If we sent an offer to a driver we + // can't see the location of (no key OR stale key), fire a Kind 3188 + // status="stale" alongside so the driver re-delivers the current key in + // the same beat. By the time they accept, we have the new key and can + // decrypt the in-ride Kind 30014 updates. Fire-and-forget — runs AFTER + // the acceptance subscription is live so its publish round-trip can't + // race the driver's response. + if (driverLocation == null) { + requestKeyRefreshAlongsideOffer(driverPubKey) + } } else { applyOfferFailureState("Failed to send RoadFlare offer") } } } + /** + * Fire a Kind 3188 with `status="stale"` to a driver alongside the just-sent ride + * offer. Issue #82: the driver receives both events together; the existing Kind 3188 + * ack handler re-delivers the current Kind 3186 key share in response. Best-effort — + * any failure is logged and otherwise ignored. + */ + private suspend fun requestKeyRefreshAlongsideOffer(driverPubKey: String) { + try { + val storedKey = followedDriversRepository.drivers.value + .find { it.pubkey == driverPubKey } + ?.roadflareKey + val storedKeyUpdatedAt = storedKey?.keyUpdatedAt ?: 0L + val storedVersion = storedKey?.version ?: 0 + + val ackEventId = nostrService.publishRoadflareKeyAck( + driverPubKey = driverPubKey, + keyVersion = storedVersion, + keyUpdatedAt = storedKeyUpdatedAt, + status = "stale" + ) + if (ackEventId != null) { + Log.d(TAG, "Stale-signal piggyback: requested key refresh from ${driverPubKey.take(8)}... (offer sent without location)") + } else { + Log.w(TAG, "Stale-signal piggyback failed for ${driverPubKey.take(8)}... (offer still went through)") + } + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: Exception) { + Log.w(TAG, "Stale-signal piggyback threw for ${driverPubKey.take(8)}...", e) + } + } + /** * Send a RoadFlare offer with an alternate (non-bitcoin) payment method. * Called when rider chooses "Continue with Alternate Payment" from insufficient funds dialog. @@ -1892,8 +1936,13 @@ class RiderViewModel @Inject constructor( val eventId = sendOfferToNostr(params, pickupRoute) if (eventId != null) { Log.d(TAG, "Sent RoadFlare offer with $paymentMethod to ${driverPubKey.take(16)}: $eventId") + // Acceptance subscription armed BEFORE the piggyback round-trip — see + // sendRoadflareOffer() for the ordering rationale. setupOfferSubscriptions(eventId, driverPubKey, isBroadcast = false) applyOfferSuccessState(params, eventId) + if (driverLocation == null) { + requestKeyRefreshAlongsideOffer(driverPubKey) + } } else { applyOfferFailureState("Failed to send RoadFlare offer") }