Skip to content

fix: drop a queued contact save when the pubky sign-in ends #1417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jasonvdb wants to merge 4 commits into
base: master
Choose a base branch
from
+784 −49
Open

fix: drop a queued contact save when the pubky sign-in ends #1417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c5b5d3
fix: drop a queued contact save when the pubky sign-in ends
Jasonvdb Oct 2, 2026
d460134
docs: describe contacts refresh batching and freshness on both platforms
Jasonvdb Oct 2, 2026
ee573ac
chore: add changelog fragment for 1417
Jasonvdb Oct 3, 2026
ddb21e6
fix: end a contact save's sign-in when another sign-in starts
Jasonvdb Oct 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 67 additions & 14 deletions app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,17 @@ sealed class PubkyContactError(message: String) : AppError(message) {
data object CannotAddSelf : PubkyContactError("Cannot add your own pubky as a contact")
data object InvalidFormat : PubkyContactError("Invalid pubky key format")
data object ActiveSubscription : PubkyContactError("Contact has an active subscription")
data object SignInChanged : PubkyContactError("Pubky sign-in changed while saving the contact")
}

/**
* One sign-in of a Pubky identity. Work the user starts in it, such as a contact edit that first waits for a profile
* lookup, checks [PubkyRepo.isCurrent] before it writes, so it stops once that identity signs out or the next sign-in
* starts. Every sign-in starts a new one, adopting a Ring identity included, even when it signs in the same identity
* again; restoring or refreshing the session of the identity already signed in does not. It holds no secret.
*/
class PubkySignIn internal constructor(val publicKey: String, internal val generation: Long)

private fun Throwable.containsActiveSubscriptionError(): Boolean =
generateSequence(this) { it.cause }.any { it is PubkyContactError.ActiveSubscription }

Expand Down Expand Up @@ -152,6 +161,7 @@ class PubkyRepo @Inject constructor(
private val adoptedSourceCheckMutex = Mutex()
private val adoptionMutex = Mutex()
private val profileWriteGeneration = AtomicLong(0L)
private val signInGeneration = AtomicLong(0L)
private var isServiceInitialized = false

private val _profile = MutableStateFlow<PubkyProfile?>(null)
Expand Down Expand Up @@ -394,7 +404,7 @@ class PubkyRepo @Inject constructor(
}
is InitResult.Restored -> {
_sessionRestorationFailed.update { false }
_publicKey.update { result.publicKey }
continueSignIn(result.publicKey)
Logger.info("Restored paykit session for '${redacted(result.publicKey)}'", context = TAG)
}
is InitResult.RestorationFailed -> {
Expand Down Expand Up @@ -540,7 +550,7 @@ class PubkyRepo @Inject constructor(

val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix()
clearProfileIfIdentityChanged(prefixedPublicKey)
_publicKey.update { prefixedPublicKey }
startSignIn(prefixedPublicKey)
notifyBackupStateChanged()
Logger.info("Adopted ring identity for '${redacted(rawPublicKey)}'", context = TAG)
prefixedPublicKey
Expand Down Expand Up @@ -648,6 +658,18 @@ class PubkyRepo @Inject constructor(
}
}

/** The current sign-in, for work that must stop once it ends, or null when no identity is signed in. */
fun currentSignIn(): PubkySignIn? {
// The generation is read first, so a sign-out between the two reads leaves a sign-in that is already over.
val generation = signInGeneration.get()
val publicKey = _publicKey.value ?: return null
return PubkySignIn(publicKey, generation)
}

/** Whether [signIn] has not ended: its identity has not signed out and no other sign-in has started since. */
fun isCurrent(signIn: PubkySignIn): Boolean =
signInGeneration.get() == signIn.generation && _publicKey.value == signIn.publicKey
Comment on lines +670 to +671

@greptile-apps greptile-apps Bot Oct 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Old sign-in becomes current again

If a user adopts Ring identity A, switches to B, then adopts A again, a contact edit started during the first A sign-in can still save. Ring adoption changes the public key without advancing the sign-in generation, so the old token matches both checks when A returns. The delayed edit can then write into A’s later sign-in instead of being dropped.

@Jasonvdb Jasonvdb Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ddb21e6. You were right: adoptRingIdentity published the new key without starting a new sign-in. A sign-in taken under A was current again after A → B → A, and re-adopting A while A was signed in kept it too.

  • Every sign-in starts a new one. Ring adoption, identity creation, signup approval and backup restore now call startSignIn, which advances the generation before publishing the key. iOS feat: one-time stale channel monitor recovery #854 already renews its session revision on each of these.
  • A refresh keeps the current sign-in. Restoring the stored session at start-up and refreshSessionIfPossible use continueSignIn. They keep the current sign-in when the key is unchanged, so a session refresh doesn't silently drop an edit that is still saving. When the key changes, including from signed out, they start a new one.
  • A second gap is closed too. clearAuthenticatedState advances the generation, then suspends before it clears the key. A sign-in taken in that window could become current again after the same identity was restored.

Tests added to PubkyRepoTest:

  • After A → B → A, and after re-adopting A while signed in, the old edit fails with SignInChanged, saves nothing through the SDK and writes no override. Both failed on ee573ac with expected:<SignInChanged> but was:<null>.
  • A sign-in taken while sign-out resets the store ends once the identity is restored. This also failed on ee573ac.
  • An edit that is saving while its identity's session is refreshed still saves. This one passes on both. It fails if the refresh path starts a new sign-in.

Full unit suite: 3215 tests, 0 failures. detekt is unchanged.


// endregion

// region Profile loading
Expand Down Expand Up @@ -814,7 +836,7 @@ class PubkyRepo @Inject constructor(
tags = tags,
status = null,
)
_publicKey.update { publicKey }
startSignIn(publicKey)
setProfile(createdProfile)
cacheMetadata(createdProfile)
settingsStore.setPubkyProfileSetupPending(false)
Expand Down Expand Up @@ -1117,8 +1139,16 @@ class PubkyRepo @Inject constructor(
}
}

/**
* Saves a contact's label and keeps the rest of its profile as the contact's local override. The edit belongs to
* [signIn]: once that identity signs out or the next sign-in starts, it fails with
* [PubkyContactError.SignInChanged] before the save, or once the save has run, before the override and the contact
* row are written. Sign-out clears the overrides, so a save that lands after it must not write one back, least of
* all for the next identity, which may have saved a contact with the same key.
*/
@Suppress("LongParameterList")
suspend fun updateContact(
signIn: PubkySignIn,
publicKey: String,
name: String,
bio: String,
Expand All @@ -1137,11 +1167,16 @@ class PubkyRepo @Inject constructor(
tags = tags,
status = null,
)
pubkyService.saveContact(prefixedKey, name)
upsertContactProfileOverride(updatedProfile)
updateContacts { current ->
current.map { if (it.publicKey == prefixedKey) updatedProfile else it }
.sortedBy { it.name.lowercase() }
requireCurrent(signIn)
// The SDK checks the identity under the same lock as the save, so no other identity can slip in between.
pubkyService.saveContact(prefixedKey, name, expectedIdentity = signIn.publicKey)
upsertContactProfileOverride(updatedProfile, signIn)
synchronized(contactsLock) {
requireCurrent(signIn)
updateContacts { current ->
current.map { if (it.publicKey == prefixedKey) updatedProfile else it }
.sortedBy { it.name.lowercase() }
}
}
markContactsLoaded()
Logger.info("Updated contact '${redacted(prefixedKey)}'", context = TAG)
Expand Down Expand Up @@ -1372,7 +1407,7 @@ class PubkyRepo @Inject constructor(
}
}

_publicKey.update { publicKey }
startSignIn(publicKey)
var pendingSaved = false
try {
settingsStore.setPubkyProfileSetupPending(true)
Expand Down Expand Up @@ -1473,7 +1508,7 @@ class PubkyRepo @Inject constructor(
val secretKeyHex = deriveLocalSecretKeyFromWalletSeed()
keychain.upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secretKeyHex)
pubkyService.signIn(secretKeyHex)
_publicKey.update { pubkyService.publicKeyFromSecret(secretKeyHex).ensurePubkyPrefix() }
startSignIn(pubkyService.publicKeyFromSecret(secretKeyHex).ensurePubkyPrefix())
}

PubkySessionBackupKind.ExternalSession -> Unit
Expand Down Expand Up @@ -1507,7 +1542,7 @@ class PubkyRepo @Inject constructor(
val publicKey = pubkyService.publicKeyFromSecret(storedSecretKeyHex).ensurePubkyPrefix()

notifyBackupStateChanged()
_publicKey.update { publicKey }
continueSignIn(publicKey)

true
}
Expand Down Expand Up @@ -1765,18 +1800,23 @@ class PubkyRepo @Inject constructor(
}.getOrNull()
?: listOf(PaykitReceiverPaths.WALLET)

private suspend fun upsertContactProfileOverride(profile: PubkyProfile) {
private suspend fun upsertContactProfileOverride(profile: PubkyProfile, signIn: PubkySignIn) {
val prefixedKey = profile.publicKey.ensurePubkyPrefix()
val ownerPublicKey = requireNotNull(_publicKey.value) { "Pubky identity unavailable" }
pubkyStore.update { data ->
// Checked as the store applies the write: sign-out ends the sign-in before it resets the store.
if (!isCurrent(signIn)) return@update data
data.copy(
ownerPublicKey = ownerPublicKey,
ownerPublicKey = signIn.publicKey,
contactProfileOverrides = data.contactProfileOverrides + (prefixedKey to profile.toProfileData()),
)
}
notifyBackupStateChanged()
}

private fun requireCurrent(signIn: PubkySignIn) {
if (!isCurrent(signIn)) throw PubkyContactError.SignInChanged
}

private suspend fun removeContactProfileOverride(publicKey: String) {
val prefixedKey = publicKey.ensurePubkyPrefix()
val ownerPublicKey = requireNotNull(_publicKey.value) { "Pubky identity unavailable" }
Expand Down Expand Up @@ -1860,10 +1900,23 @@ class PubkyRepo @Inject constructor(
_profile.update { profile }
}

private fun startSignIn(publicKey: String) {
// First, so the sign-in it replaces has ended before the key is published, even when it is the same identity.
signInGeneration.incrementAndGet()
_publicKey.update { publicKey }
}

private fun continueSignIn(publicKey: String) {
// Restoring or refreshing the session already signed in keeps its sign-in, so an edit under way still saves.
if (_publicKey.value != publicKey) startSignIn(publicKey)
}

private suspend fun clearAuthenticatedState(
clearCachedProfile: Boolean = true,
clearRestorationFailure: Boolean = true,
) = withContext(ioDispatcher) {
// First, so work of the ending sign-in stops before the store reset below, and cannot write after it.
signInGeneration.incrementAndGet()
if (clearCachedProfile) {
evictPubkyImages()
profileWriteGeneration.incrementAndGet()
Expand Down
14 changes: 14 additions & 0 deletions app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import to.bitkit.repositories.PrivatePaykitPaymentContext
import to.bitkit.repositories.PrivatePaykitRepo
import to.bitkit.repositories.PubkyContactError
import to.bitkit.repositories.PubkyRepo
import to.bitkit.repositories.PubkySignIn
import to.bitkit.repositories.PublicPaykitPaymentResult
import to.bitkit.ui.shared.toast.ToastEventBus
import to.bitkit.utils.Logger
Expand Down Expand Up @@ -347,10 +348,13 @@ class ContactDetailViewModel @Inject constructor(
transform: (ImmutableList<String>) -> ImmutableList<String>,
onSuccess: () -> Unit = {},
) {
val signIn = pubkyRepo.currentSignIn() ?: return
viewModelScope.launch {
tagPersistenceMutex.withLock {
contactLoad?.join()
if (!isTagChangeCurrent(signIn)) return@withLock
pubkyRepo.resolvePendingContactProfile(publicKey)
if (!isTagChangeCurrent(signIn)) return@withLock
val state = _uiState.value
val profile = pubkyRepo.contacts.value.find { it.publicKey == publicKey }
?: state.profile
Expand All @@ -361,6 +365,7 @@ class ContactDetailViewModel @Inject constructor(
return@withLock
}
pubkyRepo.updateContact(
signIn = signIn,
publicKey = publicKey,
name = profile.name,
bio = profile.bio,
Expand All @@ -376,6 +381,7 @@ class ContactDetailViewModel @Inject constructor(
}
onSuccess()
}.onFailure {
if (!isTagChangeCurrent(signIn)) return@onFailure
Logger.error("Failed to update tags for contact '$redactedPublicKey'", it, context = TAG)
ToastEventBus.send(
type = Toast.ToastType.ERROR,
Expand All @@ -386,6 +392,14 @@ class ContactDetailViewModel @Inject constructor(
}
}
}

private fun isTagChangeCurrent(signIn: PubkySignIn): Boolean {
val isCurrent = pubkyRepo.isCurrent(signIn)
if (!isCurrent) {
Logger.info("Dropped a tag change for '$redactedPublicKey' after the Pubky sign-in ended", context = TAG)
}
return isCurrent
}
}

@Stable
Expand Down
9 changes: 8 additions & 1 deletion app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,14 @@ class EditContactViewModel @Inject constructor(
_uiState.update { it.copy(showDeleteDialog = false) }
}

/** The save belongs to the Pubky sign-in it was made in, and stops quietly once that sign-in has ended. */
fun save() {
val state = _uiState.value
val signIn = pubkyRepo.currentSignIn() ?: return
viewModelScope.launch {
_uiState.update { it.copy(isSaving = true) }
pubkyRepo.updateContact(
signIn = signIn,
publicKey = publicKey,
name = state.name,
bio = state.bio,
Expand All @@ -223,7 +226,11 @@ class EditContactViewModel @Inject constructor(
)
_effects.emit(EditContactEffect.SaveSuccess)
}.onFailure {
Logger.error("Failed to save contact '$publicKey'", it, context = TAG)
if (pubkyRepo.isCurrent(signIn)) {
Logger.error("Failed to save contact '$publicKey'", it, context = TAG)
} else {
Logger.info("Dropped a contact edit after the Pubky sign-in ended", context = TAG)
}
_uiState.update { it.copy(isSaving = false) }
}
}
Expand Down
Loading
Loading