diff --git a/AGENTS.md b/AGENTS.md index 0e6e8a7fb..a8659a16a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -147,9 +147,9 @@ fun updateState(action: Action) { ```kotlin suspend fun getData(): Result = withContext(Dispatchers.IO) { runCatching { - Result.success(apiService.fetchData()) - }.onFailure { e -> - Logger.error("Failed", e = e, context = TAG) + apiService.fetchData() + }.onFailure { + Logger.error("Failed", it, context = TAG) } } ``` @@ -176,6 +176,7 @@ suspend fun getData(): Result = withContext(Dispatchers.IO) { - ALWAYS acknowledge datastore async operations run synchronously in a suspend context - NEVER use `runBlocking` in suspend functions - ALWAYS pass the TAG as context to `Logger` calls, e.g. `Logger.debug("message", context = TAG)` +- NEVER add `e = ` named parameter to Logger calls - ALWAYS log errors at the final handling layer where the error is acted upon, not in intermediate layers that just propagate it - ALWAYS use the Result API instead of try-catch - NEVER wrap methods returning `Result` in try-catch diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt index 2550958f1..4d6c1151b 100644 --- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt +++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt @@ -10,17 +10,21 @@ import com.synonym.vssclient.vssNewClientWithLnurlAuth import com.synonym.vssclient.vssStore import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.delay +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import to.bitkit.data.keychain.Keychain import to.bitkit.di.IoDispatcher import to.bitkit.env.Env import to.bitkit.utils.Logger -import to.bitkit.utils.ServiceError import javax.inject.Inject import javax.inject.Singleton import kotlin.time.Duration.Companion.seconds +class MnemonicNotAvailableException : Exception("Mnemonic not available") + @Singleton class VssBackupClient @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, @@ -28,41 +32,83 @@ class VssBackupClient @Inject constructor( private val keychain: Keychain, ) { private var isSetup = CompletableDeferred() + private val setupMutex = Mutex() - suspend fun setup(walletIndex: Int = 0) = withContext(ioDispatcher) { - runCatching { - withTimeout(30.seconds) { - Logger.debug("VSS client setting up…", context = TAG) - val vssUrl = Env.vssServerUrl - val lnurlAuthServerUrl = Env.lnurlAuthServerUrl - val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex) - Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG) - Logger.verbose("Building VSS client with lnurlAuthServerUrl: '$lnurlAuthServerUrl'", context = TAG) - if (lnurlAuthServerUrl.isNotEmpty()) { - val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) - ?: throw ServiceError.MnemonicNotFound() - val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name) + suspend fun setup(walletIndex: Int = 0): Result = withContext(ioDispatcher) { + setupMutex.withLock { + runCatching { + if (isSetup.isCompleted && !isSetup.isCancelled) { + runCatching { isSetup.await() }.onSuccess { return@runCatching } + } + + val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) + ?: throw MnemonicNotAvailableException() - vssNewClientWithLnurlAuth( - baseUrl = vssUrl, - storeId = vssStoreId, - mnemonic = mnemonic, - passphrase = passphrase, - lnurlAuthServerUrl = lnurlAuthServerUrl, - ) - } else { - vssNewClient( - baseUrl = vssUrl, - storeId = vssStoreId, - ) + withTimeout(30.seconds) { + Logger.debug("VSS client setting up…", context = TAG) + val vssUrl = Env.vssServerUrl + val lnurlAuthServerUrl = Env.lnurlAuthServerUrl + val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex) + Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG) + Logger.verbose("Building VSS client with lnurlAuthServerUrl: '$lnurlAuthServerUrl'", context = TAG) + if (lnurlAuthServerUrl.isNotEmpty()) { + val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name) + + vssNewClientWithLnurlAuth( + baseUrl = vssUrl, + storeId = vssStoreId, + mnemonic = mnemonic, + passphrase = passphrase, + lnurlAuthServerUrl = lnurlAuthServerUrl, + ) + } else { + vssNewClient( + baseUrl = vssUrl, + storeId = vssStoreId, + ) + } + isSetup.complete(Unit) + Logger.info("VSS client setup with server: '$vssUrl'", context = TAG) } - isSetup.complete(Unit) - Logger.info("VSS client setup with server: '$vssUrl'", context = TAG) + }.onFailure { + isSetup.completeExceptionally(it) + Logger.error("VSS client setup error", it, context = TAG) + } + } + } + + class SetupRetryLogger { + var onSuccess: (attempt: Int) -> Unit = {} + var onRetry: (attempt: Int, maxAttempts: Int, delayMs: Long) -> Unit = { _, _, _ -> } + var onExhausted: (maxAttempts: Int) -> Unit = {} + } + + suspend fun setupWithRetry( + maxAttempts: Int = 10, + baseDelayMs: Long = 1000L, + logger: SetupRetryLogger.() -> Unit, + ): Result = withContext(ioDispatcher) { + val log = SetupRetryLogger().apply(logger) + var attempt = 0 + while (attempt < maxAttempts) { + val result = setup() + if (result.isSuccess) { + log.onSuccess(attempt + 1) + return@withContext Result.success(Unit) + } + val exception = result.exceptionOrNull() + if (exception != null && exception !is MnemonicNotAvailableException) { + return@withContext result + } + attempt++ + if (attempt < maxAttempts) { + val delayMs = baseDelayMs * attempt + log.onRetry(attempt, maxAttempts, delayMs) + delay(delayMs) } - }.onFailure { - isSetup.completeExceptionally(it) - Logger.error("VSS client setup error", e = it, context = TAG) } + log.onExhausted(maxAttempts) + Result.failure(MnemonicNotAvailableException()) } fun reset() { diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index 9088a0e88..9a58ef65a 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -120,7 +120,22 @@ class BackupRepo @Inject constructor( isObserving = true Logger.debug("Start observing backup statuses and data store changes", context = TAG) - scope.launch { vssBackupClient.setup() } + scope.launch { + vssBackupClient.setupWithRetry { + onSuccess = { attempt -> + Logger.debug("VSS client setup succeeded on attempt $attempt", context = TAG) + } + onRetry = { attempt, maxAttempts, delayMs -> + Logger.debug( + "VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)", + context = TAG, + ) + } + onExhausted = { maxAttempts -> + Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG) + } + } + } scope.launch { BackupCategory.entries.forEach { category -> @@ -543,7 +558,7 @@ class BackupRepo @Inject constructor( suspend fun getLatestBackupTime(): ULong? = withContext(ioDispatcher) { runCatching { withTimeout(VSS_TIMESTAMP_TIMEOUT) { - vssBackupClient.setup() + vssBackupClient.setup().getOrThrow() coroutineScope { BackupCategory.entries .filter { it != BackupCategory.LIGHTNING_CONNECTIONS } diff --git a/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt b/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt new file mode 100644 index 000000000..2c4d00216 --- /dev/null +++ b/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt @@ -0,0 +1,73 @@ +package to.bitkit.data.backup + +import kotlinx.coroutines.runBlocking +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import to.bitkit.data.keychain.Keychain +import to.bitkit.test.BaseUnitTest +import kotlin.test.assertIs +import kotlin.test.assertTrue + +class VssBackupClientTest : BaseUnitTest() { + + private lateinit var sut: VssBackupClient + + private val vssStoreIdProvider = mock() + private val keychain = mock() + + @Before + fun setUp() = runBlocking { + sut = VssBackupClient( + ioDispatcher = testDispatcher, + vssStoreIdProvider = vssStoreIdProvider, + keychain = keychain, + ) + } + + @Test + fun `setup fails with MnemonicNotAvailableException when mnemonic is not available`() = test { + whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null) + + val result = sut.setup() + + assertTrue(result.isFailure) + assertIs(result.exceptionOrNull()) + } + + @Test + fun `setup does not call vssStoreIdProvider when mnemonic is not available`() = test { + whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null) + + sut.setup() + + verify(vssStoreIdProvider, never()).getVssStoreId(any()) + } + + @Test + fun `setup checks mnemonic before proceeding with vss initialization`() = test { + val testMnemonic = "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about" + whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(testMnemonic) + whenever(vssStoreIdProvider.getVssStoreId(any())).thenReturn("test-store-id") + + // Setup will fail on native VSS calls, but we verify we passed the mnemonic check + runCatching { sut.setup() } + + verify(vssStoreIdProvider).getVssStoreId(any()) + } + + @Test + fun `setup can be called multiple times when mnemonic not available`() = test { + whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null) + + // Multiple calls should all fail with MnemonicNotAvailableException without crashing + assertIs(sut.setup().exceptionOrNull()) + assertIs(sut.setup().exceptionOrNull()) + assertIs(sut.setup().exceptionOrNull()) + } +}