From f150696a261ec7cdb185b293fa730ff8a117e3ab Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 29 Aug 2026 19:34:36 +0200 Subject: [PATCH] fix: wait out the second instead of stamping created_at in the future MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the PR: created_at has second resolution, so nothing on nostr can replace an address more than once per second — and a client that keeps out-stamping the previous version drifts a second further into the future per republish, which relays may reject. That is right, and it points at a better guard than `+ 1`. One second is the real floor on how often an address can be replaced, so a client that replaces one faster should wait for the clock rather than invent a timestamp. awaitCreatedAtToSupersede suspends until the second the previous version claimed has passed, then stamps the real time — the new version still wins, and no event is ever dated ahead of the clock. The wait is bounded (MAX_SUPERSEDE_WAIT_SECONDS). A version further ahead than that came from another device's skewed clock rather than this client's own burst, and sleeping it out could take hours, so past the bound out-stamping is still the only way to supersede. Applied to the two paths that can accumulate drift across repeated edits and were already suspending under a mutex: the NIP-78 settings blob and the per-d-tag app recommendations. RoomParticipantActions keeps the non-suspending form — it is reached from Compose click handlers, and its stamp derives from the single event being acted on, so it sits at most one second ahead and cannot drift. Note the debounce added earlier already keeps the settings pickers from publishing sub-second at all (measured on device: 23 rapid toggles → 3 events, each stamped at the true wall-clock second, the `+ 1` never firing). This makes that a guarantee rather than a consequence of timing, and extends it to the settings paths that are deliberately not debounced. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Voa2KcknNffhvPqsRG92hx --- .../nip78AppSpecific/AppSpecificState.kt | 9 +- .../AppRecommendationsState.kt | 14 ++- .../participants/RoomParticipantActions.kt | 5 + .../nip01Core/core/ReplaceableSupersession.kt | 40 ++++++++ .../core/AwaitCreatedAtToSupersedeTest.kt | 92 +++++++++++++++++++ 5 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/core/AwaitCreatedAtToSupersedeTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip78AppSpecific/AppSpecificState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip78AppSpecific/AppSpecificState.kt index 8564f2433e..453b1a07a5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip78AppSpecific/AppSpecificState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip78AppSpecific/AppSpecificState.kt @@ -25,11 +25,10 @@ import com.vitorpamplona.amethyst.model.AccountSyncedSettingsInternal import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.NoteState import com.vitorpamplona.quartz.nip01Core.core.JsonMapper -import com.vitorpamplona.quartz.nip01Core.core.nextCreatedAtToSupersede +import com.vitorpamplona.quartz.nip01Core.core.awaitCreatedAtToSupersede import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import com.vitorpamplona.quartz.nip78AppData.AppSpecificDataEvent import com.vitorpamplona.quartz.utils.Log -import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers @@ -75,11 +74,7 @@ class AppSpecificState( val (toInternal, createdAt) = stampOrder.withLock { val snapshot = settings.syncedSettings.toInternal(settings.mutedPublicChats.value) - val stamp = - nextCreatedAtToSupersede( - newestKnown = maxOf(lastPublishedAt, amethystSettingsNote.event?.createdAt ?: 0L), - now = TimeUtils.now(), - ) + val stamp = awaitCreatedAtToSupersede(maxOf(lastPublishedAt, amethystSettingsNote.event?.createdAt ?: 0L)) lastPublishedAt = stamp snapshot to stamp } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip89AppHandlers/AppRecommendationsState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip89AppHandlers/AppRecommendationsState.kt index 68775c9f8b..e5c9d81cfd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip89AppHandlers/AppRecommendationsState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip89AppHandlers/AppRecommendationsState.kt @@ -24,7 +24,7 @@ import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.filterIntoSet import com.vitorpamplona.quartz.nip01Core.core.Address -import com.vitorpamplona.quartz.nip01Core.core.nextCreatedAtToSupersede +import com.vitorpamplona.quartz.nip01Core.core.awaitCreatedAtToSupersede import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner @@ -32,7 +32,6 @@ import com.vitorpamplona.quartz.nip89AppHandlers.PlatformType import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent import com.vitorpamplona.quartz.nip89AppHandlers.recommendation.AppRecommendationEvent import com.vitorpamplona.quartz.nip89AppHandlers.recommendation.tags.RecommendationTag -import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.SharingStarted @@ -85,11 +84,16 @@ class AppRecommendationsState( */ private val publishMutex = Mutex() - /** The createdAt this d-tag's next version needs to supersede whatever is in cache for it. */ - private fun nextCreatedAt(supportedKind: String): Long { + /** + * The createdAt this d-tag's next version needs to supersede whatever is in cache for it. Waits + * out the second rather than stamping the future, so repeatedly toggling one recommendation + * cannot drift its `created_at` ahead of the clock. Runs under [publishMutex], which is what + * keeps two waits from racing each other onto the same second. + */ + private suspend fun nextCreatedAt(supportedKind: String): Long { val address = Address(AppRecommendationEvent.KIND, signer.pubKey, supportedKind) val latest = cache.getAddressableNoteIfExists(address)?.event?.createdAt ?: 0L - return nextCreatedAtToSupersede(latest, TimeUtils.now()) + return awaitCreatedAtToSupersede(latest) } private fun currentRecommendations(supportedKind: String): List { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/participants/RoomParticipantActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/participants/RoomParticipantActions.kt index 54f90a9cc7..0afb5bbeaa 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/participants/RoomParticipantActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/participants/RoomParticipantActions.kt @@ -137,6 +137,11 @@ internal object RoomParticipantActions { // Creating a room then immediately promoting in the same wall-clock // second produced the dreaded silent-no-op: both versions carried the // same createdAt and the tie-break picked one at random. + // + // The out-stamping form rather than awaitCreatedAtToSupersede: this is + // reached from non-suspending Compose click handlers, and the stamp is + // derived from the one event being acted on, so it runs at most a second + // ahead of the clock and cannot drift the way a repeated republish does. val nextCreatedAt = nextCreatedAtToSupersede(original.createdAt, TimeUtils.now()) return MeetingSpaceEvent.build( diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/ReplaceableSupersession.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/ReplaceableSupersession.kt index 48748517e4..b775be70b6 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/ReplaceableSupersession.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/core/ReplaceableSupersession.kt @@ -20,6 +20,9 @@ */ package com.vitorpamplona.quartz.nip01Core.core +import com.vitorpamplona.quartz.utils.TimeUtils +import kotlinx.coroutines.delay + /** * The NIP-01 replaceable/addressable supersession rule: the winner of an * address is the highest `created_at`, with ties broken by the LEXICALLY @@ -51,3 +54,40 @@ fun nextCreatedAtToSupersede( newestKnown: Long, now: Long, ): Long = (newestKnown + 1).coerceAtLeast(now) + +/** + * How far ahead of this device's clock [awaitCreatedAtToSupersede] is still willing to wait. A + * version further ahead than this came from another device's skewed clock rather than from this + * client's own burst, and waiting it out could mean sleeping for hours. + */ +const val MAX_SUPERSEDE_WAIT_SECONDS = 5L + +/** + * The `created_at` for the next version of an address, waiting for the clock rather than running + * ahead of it: if [newestKnown] already claims the current second, this suspends until that second + * has passed and then stamps the real time. + * + * Prefer this to [nextCreatedAtToSupersede] wherever the caller can suspend. Both make the new + * version win, but this one never puts a `created_at` in the future — one second of second-resolution + * `created_at` is genuinely the floor on how often an address can be replaced, so a client that + * replaces one faster has to wait, not invent a timestamp. Repeatedly out-stamping instead would + * drift a second further ahead per republish, and relays reject events too far in the future. + * + * The wait is bounded by [MAX_SUPERSEDE_WAIT_SECONDS]; past that the only way to supersede is still + * to out-stamp, so it falls back to [nextCreatedAtToSupersede]. + * + * [now] is injectable so the rule can be tested against a virtual clock. + */ +suspend fun awaitCreatedAtToSupersede( + newestKnown: Long, + now: () -> Long = TimeUtils::now, +): Long { + val startedAt = now() + if (newestKnown < startedAt) return startedAt + + val secondsToWait = newestKnown - startedAt + 1 + if (secondsToWait > MAX_SUPERSEDE_WAIT_SECONDS) return nextCreatedAtToSupersede(newestKnown, startedAt) + + delay(secondsToWait * 1000) + return maxOf(now(), newestKnown + 1) +} diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/core/AwaitCreatedAtToSupersedeTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/core/AwaitCreatedAtToSupersedeTest.kt new file mode 100644 index 0000000000..af9ddf30e8 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/core/AwaitCreatedAtToSupersedeTest.kt @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.core + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * The point of waiting rather than out-stamping: `created_at` is whole seconds, so one second is the + * real floor on how often an address can be replaced. A client that replaces one faster has to wait + * for the clock — inventing a timestamp instead drifts a second further into the future per + * republish, and relays reject events too far ahead. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class AwaitCreatedAtToSupersedeTest { + @Test + fun stampsTheClockWhenItHasAlreadyMovedPastTheNewestVersion() = + runTest { + advanceTimeBy(10_000) + val clock = { testScheduler.currentTime / 1000 } + + val stamp = awaitCreatedAtToSupersede(newestKnown = 5, now = clock) + + assertEquals(10L, stamp) + assertEquals(10_000L, testScheduler.currentTime, "must not wait when there is nothing to wait for") + } + + @Test + fun waitsOutTheSecondTheNewestVersionClaimedInsteadOfStampingTheFuture() = + runTest { + advanceTimeBy(10_000) + val clock = { testScheduler.currentTime / 1000 } + + val stamp = awaitCreatedAtToSupersede(newestKnown = 10, now = clock) + + assertEquals(11L, stamp) + assertEquals(11_000L, testScheduler.currentTime, "should have waited exactly the one second out") + assertTrue(stamp <= clock(), "a stamp must never be ahead of the clock") + } + + @Test + fun aBurstNeverRunsAheadOfTheClock() = + runTest { + advanceTimeBy(10_000) + val clock = { testScheduler.currentTime / 1000 } + + // Five back-to-back republishes, each superseding the one before it. + var newest = 10L + repeat(5) { + newest = awaitCreatedAtToSupersede(newestKnown = newest, now = clock) + assertTrue(newest <= clock(), "republish $it stamped the future") + } + + assertEquals(15L, newest) + } + + @Test + fun outStampsAVersionTooFarAheadToWaitOut() = + runTest { + advanceTimeBy(10_000) + val clock = { testScheduler.currentTime / 1000 } + + // Another device's skewed clock. Waiting it out would mean sleeping for hours, so the + // only way to supersede it is still to out-stamp it. + val stamp = awaitCreatedAtToSupersede(newestKnown = 10 + MAX_SUPERSEDE_WAIT_SECONDS + 1, now = clock) + + assertEquals(17L, stamp) + assertEquals(10_000L, testScheduler.currentTime, "must not wait out an implausible skew") + } +}