diff --git a/docs/testing/manual_testing/Switch cache to store blocks on disk.md b/docs/testing/manual_testing/Switch cache to store blocks on disk.md index 20a5e550..590ca8c7 100644 --- a/docs/testing/manual_testing/Switch cache to store blocks on disk.md +++ b/docs/testing/manual_testing/Switch cache to store blocks on disk.md @@ -36,7 +36,7 @@ Observed result of this manual test should be: 1. Switch to the latest commit on the **Main** branch in your git client 1. Update dependencies lock (if needed) and sync Gradle files 1. Run the demo-app on the same emulator device as previously -1. Once the app is opened go through the same steps as previously to let the SDK apply the new cache storing +1. Once the app is opened go through the same steps as previously to let the SDK apply the new cache storing mechanisms 1. Open the Device File Explorer in the Android Studio again 1. Go to `/data/data/cash.z.ecc.android.sdk.demoapp.mainnet/no_backup/co.electricoin.zcash/` @@ -46,4 +46,7 @@ Observed result of this manual test should be: 1. Verify there is no `cache.sqlite3` database file, and check that no rollback files (suffixed with `journal` or `wal`) are present. The file names can vary, depending on the current build variant. 1. Inspect older legacy database folder `/data/data/cash.z.ecc.android.sdk.demoapp.mainnet/databases/`, which - also should not contain `cache.sqlite3` or rollback files. \ No newline at end of file + also should not contain `cache.sqlite3` or rollback files. +1. Once the whole sync process is done, verify that the temporary `blocks` directory is removed from the device + storage with all its block files, or is empty, as it's automatically created by the new blocks polling mechanism. +1. Verify also that the `blockmeta.sqlite` still preserves diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepositoryTest.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepositoryTest.kt index 2c55f9ab..a7d51865 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepositoryTest.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepositoryTest.kt @@ -2,6 +2,7 @@ package cash.z.ecc.android.sdk.internal.storage.block import cash.z.ecc.android.sdk.internal.ext.deleteRecursivelySuspend import cash.z.ecc.android.sdk.internal.ext.existsSuspend +import cash.z.ecc.android.sdk.internal.ext.listSuspend import cash.z.ecc.android.sdk.internal.ext.mkdirsSuspend import cash.z.ecc.android.sdk.jni.RustBackendWelding import cash.z.ecc.android.sdk.model.BlockHeight @@ -27,13 +28,12 @@ class FileCompactBlockRepositoryTest { @OptIn(ExperimentalCoroutinesApi::class) @Before fun setup() = runTest { - val rootDirectory = FilePathFixture.newBlocksDir() - if (rootDirectory.existsSuspend()) { - rootDirectory.deleteRecursivelySuspend() + val blocksDirectory = FilePathFixture.newBlocksDir() + if (blocksDirectory.existsSuspend()) { + blocksDirectory.deleteRecursivelySuspend() } - val blocksDir = FilePathFixture.newBlocksDir() - blocksDir.mkdirsSuspend() + blocksDirectory.mkdirsSuspend() } @OptIn(ExperimentalCoroutinesApi::class) @@ -148,6 +148,40 @@ class FileCompactBlockRepositoryTest { assertEquals(blocks.count(), rootBlocksDirectory.list()!!.size) } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun deleteCompactBlockFilesTest() = runTest { + val rustBackend = FakeRustBackendFixture().new() + val blocksDirectory = FilePathFixture.newBlocksDir() + val parentDirectory = blocksDirectory.parentFile!! + + val blockRepository = getMockedFileCompactBlockRepository(rustBackend, blocksDirectory) + + val testedBlocksRange = ListOfCompactBlocksFixture.DEFAULT_FILE_BLOCK_RANGE + val blocks = ListOfCompactBlocksFixture.new(testedBlocksRange) + + val persistedBlocksCount = blockRepository.write(blocks) + + parentDirectory.also { + assertTrue(it.existsSuspend()) + assertTrue(it.listSuspend()!!.contains(FilePathFixture.DEFAULT_BLOCKS_DIR_NAME)) + } + blocksDirectory.also { + assertTrue(it.existsSuspend()) + assertEquals(blocks.count(), persistedBlocksCount) + } + + blockRepository.deleteCompactBlockFiles() + + parentDirectory.also { + assertTrue(it.existsSuspend()) + assertFalse(it.listSuspend()!!.contains(FilePathFixture.DEFAULT_BLOCKS_DIR_NAME)) + } + blocksDirectory.also { blocksDir -> + assertFalse(blocksDir.existsSuspend()) + } + } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun rewindToTest() = runTest { diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FilePathFixture.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FilePathFixture.kt index d983ba49..69d93c2a 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FilePathFixture.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FilePathFixture.kt @@ -5,7 +5,7 @@ import java.io.File object FilePathFixture { private val DEFAULT_ROOT_DIR_PATH = DatabasePathFixture.new() - private const val DEFAULT_BLOCKS_DIR_NAME = FileCompactBlockRepository.BLOCKS_DOWNLOAD_DIRECTORY + internal const val DEFAULT_BLOCKS_DIR_NAME = FileCompactBlockRepository.BLOCKS_DOWNLOAD_DIRECTORY internal fun newBlocksDir( rootDirectoryPath: String = DEFAULT_ROOT_DIR_PATH, diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/CompactBlockProcessor.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/CompactBlockProcessor.kt index 21baec75..e171c46f 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/CompactBlockProcessor.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/CompactBlockProcessor.kt @@ -46,7 +46,6 @@ import co.electriccoin.lightwallet.client.fixture.BenchmarkingBlockRangeFixture import co.electriccoin.lightwallet.client.model.BlockHeightUnsafe import co.electriccoin.lightwallet.client.model.LightWalletEndpointInfoUnsafe import co.electriccoin.lightwallet.client.model.Response -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow @@ -238,6 +237,13 @@ class CompactBlockProcessor internal constructor( } delay(napTime) } + is BlockProcessingResult.FailedDeleteBlocks -> { + Twig.debug { + "Failed to delete temporary blocks files from the device disk. It will be retried on the" + + " next time, while downloading new blocks." + } + // Do nothing. The other phases went correctly. + } is BlockProcessingResult.Error -> { if (consecutiveChainErrors.get() >= RETRIES) { val errorMessage = "ERROR: unable to resolve reorg at height $result after " + @@ -289,40 +295,74 @@ class CompactBlockProcessor internal constructor( setState(State.Scanned(currentInfo.lastScanRange)) BlockProcessingResult.NoBlocksToProcess } else { - if (BenchmarkingExt.isBenchmarking()) { - // We inject a benchmark test blocks range at this point to process only a restricted range of blocks - // for a more reliable benchmark results. - val benchmarkBlockRange = BenchmarkingBlockRangeFixture.new().let { - // Convert range of Longs to range of BlockHeights - BlockHeight.new(ZcashNetwork.Mainnet, it.start)..( - BlockHeight.new(ZcashNetwork.Mainnet, it.endInclusive) - ) - } - downloadNewBlocks(benchmarkBlockRange) - val error = validateAndScanNewBlocks(benchmarkBlockRange) - if (error != BlockProcessingResult.Success) { - error + val processRanges = ProcessBlocksRanges().apply { + if (BenchmarkingExt.isBenchmarking()) { + // We inject a benchmark test blocks range at this point to process only a restricted range of + // blocks for a more reliable benchmark results. + val benchmarkBlockRange = BenchmarkingBlockRangeFixture.new().let { + // Convert range of Longs to range of BlockHeights + BlockHeight.new(ZcashNetwork.Mainnet, it.start)..( + BlockHeight.new(ZcashNetwork.Mainnet, it.endInclusive) + ) + } + downloadRange = benchmarkBlockRange + scanRange = benchmarkBlockRange } else { - enhanceTransactionDetails(benchmarkBlockRange) - } - } else { - downloadNewBlocks(currentInfo.lastDownloadRange) - val error = validateAndScanNewBlocks(currentInfo.lastScanRange) - if (error != BlockProcessingResult.Success) { - error - } else { - currentInfo.lastScanRange?.let { enhanceTransactionDetails(it) } - ?: BlockProcessingResult.NoBlocksToProcess + downloadRange = currentInfo.lastDownloadRange + scanRange = currentInfo.lastScanRange } } + + downloadNewBlocks(processRanges.downloadRange) + + val validationResult = validateAndScanNewBlocks(processRanges.scanRange) + if (validationResult != BlockProcessingResult.Success) { + deleteAllBlockFiles() + return@withContext validationResult + } + + val deleteBlocksResult = deleteAllBlockFiles() + + val enhanceResult = enhanceTransactionDetails(processRanges.scanRange) + + // Return enhance or delete operations result in case of any failure happened + if (enhanceResult != BlockProcessingResult.Success || + enhanceResult != BlockProcessingResult.NoBlocksToProcess + ) { + return@withContext enhanceResult + } else if (deleteBlocksResult != BlockProcessingResult.Success) { + return@withContext BlockProcessingResult.FailedDeleteBlocks + } + + return@withContext BlockProcessingResult.Success } } + private suspend fun deleteAllBlockFiles(): BlockProcessingResult { + Twig.debug { "Deleting all temporary blocks files now." } + // Now we approach to delete all the temporary blocks files from the device disk. Note that we'd + // like to ideally do this continuously while syncing a next group of blocks in the future. + return if (downloader.compactBlockRepository.deleteCompactBlockFiles()) { + BlockProcessingResult.Success + } else { + BlockProcessingResult.FailedDeleteBlocks + } + } + + /** + * Helper wrapping class to provide more clarity about ranges used in the processing new blocks mechanism. + */ + private data class ProcessBlocksRanges( + var downloadRange: ClosedRange? = null, + var scanRange: ClosedRange? = null + ) + sealed class BlockProcessingResult { object NoBlocksToProcess : BlockProcessingResult() object Success : BlockProcessingResult() object Reconnecting : BlockProcessingResult() object FailedEnhance : BlockProcessingResult() + object FailedDeleteBlocks : BlockProcessingResult() data class Error(val failedAtHeight: BlockHeight) : BlockProcessingResult() } @@ -421,7 +461,11 @@ class CompactBlockProcessor internal constructor( result } - private suspend fun enhanceTransactionDetails(lastScanRange: ClosedRange): BlockProcessingResult { + private suspend fun enhanceTransactionDetails(lastScanRange: ClosedRange?): BlockProcessingResult { + if (lastScanRange == null) { + return BlockProcessingResult.NoBlocksToProcess + } + Twig.debug { "Enhancing transaction details for blocks $lastScanRange" } setState(Enhancing) @Suppress("TooGenericExceptionCaught") @@ -448,10 +492,10 @@ class CompactBlockProcessor internal constructor( BlockProcessingResult.FailedEnhance } } -// TODO [#683]: we still need a way to identify those transactions that failed to be enhanced -// TODO [#683]: https://github.com/zcash/zcash-android-wallet-sdk/issues/683 + // TODO [#683]: we still need a way to identify those transactions that failed to be enhanced + // TODO [#683]: https://github.com/zcash/zcash-android-wallet-sdk/issues/683 - private suspend fun enhance(transaction: TransactionOverview) = withContext(Dispatchers.IO) { + private suspend fun enhance(transaction: TransactionOverview) = withContext(IO) { transaction.minedHeight?.let { minedHeight -> enhanceHelper(transaction.id, transaction.rawId.byteArray, minedHeight) } @@ -480,7 +524,7 @@ class CompactBlockProcessor internal constructor( /** * Confirm that the wallet data is properly setup for use. */ -// Need to refactor this to be less ugly and more testable + // Need to refactor this to be less ugly and more testable @Suppress("NestedBlockDepth") private suspend fun verifySetup() { // verify that the data is initialized @@ -642,7 +686,7 @@ class CompactBlockProcessor internal constructor( * @param range the range of blocks to download. */ @VisibleForTesting -// allow mocks to verify how this is called, rather than the downloader, which is more complex + // allow mocks to verify how this is called, rather than the downloader, which is more complex @Suppress("MagicNumber") internal suspend fun downloadNewBlocks(range: ClosedRange?) = withContext(IO) { @@ -1048,7 +1092,7 @@ class CompactBlockProcessor internal constructor( repository.lastScannedHeight() // CompactBlockProcessor is the wrong place for this, but it's where all the other APIs that need -// access to the RustBackend live. This should be refactored. + // access to the RustBackend live. This should be refactored. internal suspend fun createAccount(seed: ByteArray): UnifiedSpendingKey = rustBackend.createAccount(seed) @@ -1266,9 +1310,9 @@ class CompactBlockProcessor internal constructor( val hash: String? ) -// -// Helper Extensions -// + // + // Helper Extensions + // /** * Log the mutex in great detail just in case we need it for troubleshooting deadlock. diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/db/DatabaseCoordinator.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/db/DatabaseCoordinator.kt index 7a0330d1..aa9ca379 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/db/DatabaseCoordinator.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/db/DatabaseCoordinator.kt @@ -104,7 +104,7 @@ internal class DatabaseCoordinator private constructor(context: Context) { * Returns the file of the Data database that would correspond to the given alias * and network attributes. * - * @param network the network associated with the data in the database. + * @param network the network associated with the data in the database * @param alias the alias to convert into a database path * * @return the Data database file @@ -135,7 +135,7 @@ internal class DatabaseCoordinator private constructor(context: Context) { * PendingTransactions.db, we choose slightly different approach, but it also leads to * original database files migration with additional renaming too. * - * @param network the network associated with the data in the database. + * @param network the network associated with the data in the database * @param alias the alias to convert into a database path * * @return the PendingTransaction database file @@ -169,7 +169,7 @@ internal class DatabaseCoordinator private constructor(context: Context) { * Function for common deletion of Data and Cache database files. It also checks and deletes * additional journal and wal files, if they exist. * - * @param network the network associated with the data in the database. + * @param network the network associated with the data in the database * @param alias the alias to convert into a database path * * @return true only if any database deleted, false otherwise @@ -229,7 +229,7 @@ internal class DatabaseCoordinator private constructor(context: Context) { * as the preferred (i.e. newly created) file for subsequent use (and eventually move). * * @param appContext the application context - * @param network the network associated with the data in the database. + * @param network the network associated with the data in the database * @param alias the alias to convert into a database path * @param databaseName the name of the new database file */ @@ -425,8 +425,8 @@ internal class DatabaseCoordinator private constructor(context: Context) { * instantiation. * * @param context - * @param klass The database class. - * @param databaseFile The database file. + * @param klass the database class + * @param databaseFile the database file * @return A {@code RoomDatabaseBuilder} which you can use to create the database. */ internal fun commonDatabaseBuilder( diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/FileExt.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/FileExt.kt index 85e71796..a57b1735 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/FileExt.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/FileExt.kt @@ -23,6 +23,8 @@ suspend fun File.deleteRecursivelySuspend() = withContext(Dispatchers.IO) { dele suspend fun File.listFilesSuspend(): Array? = withContext(Dispatchers.IO) { listFiles() } +suspend fun File.listSuspend(): Array? = withContext(Dispatchers.IO) { list() } + suspend fun File.inputStreamSuspend(): FileInputStream = withContext(Dispatchers.IO) { inputStream() } suspend fun File.createNewFileSuspend() = withContext(Dispatchers.IO) { createNewFile() } diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/repository/CompactBlockRepository.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/repository/CompactBlockRepository.kt index 0ad06bb5..7aff143b 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/repository/CompactBlockRepository.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/repository/CompactBlockRepository.kt @@ -22,6 +22,14 @@ interface CompactBlockRepository { */ suspend fun findCompactBlock(height: BlockHeight): JniBlockMeta? + /** + * This function is supposed to be used once the whole blocks sync process done. It removes all the temporary + * block files from the device disk together with theirs parent directory. + * + * @return true when all block files are deleted, false only if the deletion fails + */ + suspend fun deleteCompactBlockFiles(): Boolean + /** * Write the given blocks to this store, which may be anything from an in-memory cache to a DB. * diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepository.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepository.kt index 4f88b3d2..5dd141cd 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepository.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/storage/block/FileCompactBlockRepository.kt @@ -3,6 +3,7 @@ package cash.z.ecc.android.sdk.internal.storage.block import androidx.annotation.VisibleForTesting import cash.z.ecc.android.sdk.internal.Twig import cash.z.ecc.android.sdk.internal.ext.createNewFileSuspend +import cash.z.ecc.android.sdk.internal.ext.deleteRecursivelySuspend import cash.z.ecc.android.sdk.internal.ext.deleteSuspend import cash.z.ecc.android.sdk.internal.ext.existsSuspend import cash.z.ecc.android.sdk.internal.ext.mkdirsSuspend @@ -63,6 +64,16 @@ internal class FileCompactBlockRepository( override suspend fun rewindTo(height: BlockHeight) = rustBackend.rewindBlockMetadataToHeight(height) + override suspend fun deleteCompactBlockFiles(): Boolean { + Twig.debug { "Removing blocks directory ${blocksDirectory.path} with all its children." } + + if (blocksDirectory.existsSuspend()) { + return blocksDirectory.deleteRecursivelySuspend() + } + + return true + } + companion object { /** * The name of the directory for downloading blocks