From a03a89707c3dfb8c0773d09d53de02e1024aacf6 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 25 Jul 2022 10:11:09 -0600 Subject: [PATCH 01/24] Clarify instructions for Android NDK installation. --- docs/Setup.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Setup.md b/docs/Setup.md index 0b6141de..db8db3ed 100644 --- a/docs/Setup.md +++ b/docs/Setup.md @@ -39,7 +39,7 @@ Start by making sure the command line with Gradle works first, because **all the export PATH=${PATH}:${ANDROID_HOME}/platform-tools ``` 1. Install the Android NDK - 1. Go to the Android SDK Manager inside Android Studio + 1. Go to the Android SDK Manager inside Android Studio. Select the "SDK Tools" tab. 1. Click the checkbox for "Show Package Details" 1. Install the exact NDK version listed in [gradle.properties](../gradle.properties) under `ANDROID_NDK_VERSION` 1. Configure a device for development and testing From d15f58142aa56a486ec786f8a34fc0094477a22d Mon Sep 17 00:00:00 2001 From: Honza Rychnovsky Date: Wed, 24 Aug 2022 08:30:18 +0200 Subject: [PATCH 02/24] [#691] Increase Deploy action Upload artifacts timeout --- .github/workflows/deploy-release.yml | 2 +- .github/workflows/deploy-snapshot.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy-release.yml b/.github/workflows/deploy-release.yml index 3e997e3d..13734f67 100644 --- a/.github/workflows/deploy-release.yml +++ b/.github/workflows/deploy-release.yml @@ -87,7 +87,7 @@ jobs: - name: Upload Artifacts if: ${{ always() }} uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 - timeout-minutes: 1 + timeout-minutes: 2 with: name: Release binaries path: ~/artifacts diff --git a/.github/workflows/deploy-snapshot.yml b/.github/workflows/deploy-snapshot.yml index c69de193..c3e8dc25 100644 --- a/.github/workflows/deploy-snapshot.yml +++ b/.github/workflows/deploy-snapshot.yml @@ -94,7 +94,7 @@ jobs: - name: Upload Artifacts if: ${{ always() }} uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 - timeout-minutes: 1 + timeout-minutes: 2 with: name: Snapshot binaries path: ~/artifacts From 1fae5beab6b3537b7ea31d92f9308d0c7b71afa1 Mon Sep 17 00:00:00 2001 From: Honza Rychnovsky Date: Thu, 25 Aug 2022 14:36:57 +0200 Subject: [PATCH 03/24] [#509] Remove OkHttp and Okio dependencies * [#509] Remove OkHttp and Okio dependencies - Okio -> Java NIO anf OkHttp -> HttpURLConnection in SaplingParamTool class - Both replaced also in the test packages - Removed dependencies to the replaced components - Again switched a few assertions from JUnit to kotlin.test - Created readLinesInFlow() function which is now reused in few places and runs without OkHttp - Added subsequent TODOs to code base - Wrapped to withContext to move it to an IO thread. - Added use function on the stream and close() at the end of the channels use. Co-authored-by: Carter Jernigan --- gradle.properties | 2 - sdk-lib/build.gradle.kts | 8 -- .../z/ecc/android/sdk/ext/TestExtensions.kt | 18 ++-- .../sdk/internal/SaplingParamToolTest.kt | 12 +-- .../cash/z/ecc/android/sdk/test/Global.kt | 16 +++- .../android/sdk/util/AddressGeneratorUtil.kt | 22 +---- .../android/sdk/util/BalancePrinterUtil.kt | 19 +--- .../cash/z/ecc/android/sdk/ext/ZcashSdk.kt | 2 +- .../android/sdk/internal/SaplingParamTool.kt | 95 ++++++++++--------- settings.gradle.kts | 4 - 10 files changed, 84 insertions(+), 114 deletions(-) diff --git a/gradle.properties b/gradle.properties index 1dac21e3..3afd162b 100644 --- a/gradle.properties +++ b/gradle.properties @@ -102,8 +102,6 @@ KOTLINX_COROUTINES_VERSION=1.6.4 KOTLIN_VERSION=1.7.10 MOCKITO_KOTLIN_VERSION=2.2.0 MOCKITO_VERSION=4.6.1 -OKHTTP_VERSION=4.10.0 -OKIO_VERSION=3.2.0 PROTOC_VERSION=3.21.4 ZCASH_WALLET_PLUGINS_VERSION=1.0.1 diff --git a/sdk-lib/build.gradle.kts b/sdk-lib/build.gradle.kts index 4e3f49ad..a5e02bb4 100644 --- a/sdk-lib/build.gradle.kts +++ b/sdk-lib/build.gradle.kts @@ -278,11 +278,6 @@ dependencies { // (com.google.guava:listenablefuture:1.0) per this recommendation from Chris Povirk, given guava's decision to // split ListenableFuture away from Guava: https://groups.google.com/d/msg/guava-discuss/GghaKwusjcY/bCIAKfzOEwAJ implementation(libs.guava) - // OKIO is a transitive dependency used when writing param files to disk. Like GSON, this can be - // replaced if needed. For compatibility, we match the library version used in grpc-okhttp: - // https://github.com/grpc/grpc-java/blob/v1.37.x/build.gradle#L159 - implementation(libs.okio) - implementation(libs.okhttp) // Tests testImplementation(libs.kotlin.reflect) @@ -305,9 +300,6 @@ dependencies { androidTestImplementation(libs.coroutines.okhttp) androidTestImplementation(libs.kotlin.test) androidTestImplementation(libs.kotlinx.coroutines.test) - // used by 'ru.gildor.corutines.okhttp.await' (to make simple suspended requests) and breaks on versions higher - // than 3.8.0 - androidTestImplementation(libs.okhttp) // sample mnemonic plugin androidTestImplementation(libs.zcashwalletplgn) diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/ext/TestExtensions.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/ext/TestExtensions.kt index 4bb1340f..7c5c8d9c 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/ext/TestExtensions.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/ext/TestExtensions.kt @@ -3,11 +3,12 @@ package cash.z.ecc.android.sdk.ext import cash.z.ecc.android.sdk.Initializer import cash.z.ecc.android.sdk.model.ZcashNetwork import cash.z.ecc.android.sdk.util.SimpleMnemonics +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking -import okhttp3.OkHttpClient -import okhttp3.Request +import kotlinx.coroutines.withContext import org.json.JSONObject -import ru.gildor.coroutines.okhttp.await +import java.net.HttpURLConnection +import java.net.URL import kotlin.test.assertNotNull fun Initializer.Config.seedPhrase(seedPhrase: String, network: ZcashNetwork) { @@ -16,12 +17,11 @@ fun Initializer.Config.seedPhrase(seedPhrase: String, network: ZcashNetwork) { object BlockExplorer { suspend fun fetchLatestHeight(): Long { - val client = OkHttpClient() - val request = Request.Builder() - .url("https://api.blockchair.com/zcash/blocks?limit=1") - .build() - val result = client.newCall(request).await() - val body = result.body?.string() + val url = URL("https://api.blockchair.com/zcash/blocks?limit=1") + val connection = withContext(Dispatchers.IO) { + url.openConnection() + } as HttpURLConnection + val body = connection.inputStream.bufferedReader().readText() assertNotNull(body, "Body can not be null.") return JSONObject(body).getJSONArray("data").getJSONObject(0).getLong("id") } diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt index d3d479ef..ba72ab72 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt @@ -5,12 +5,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import cash.z.ecc.android.sdk.ext.ZcashSdk import kotlinx.coroutines.runBlocking -import org.junit.Assert import org.junit.Before import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import java.io.File +import kotlin.test.assertFalse @RunWith(AndroidJUnit4::class) @Ignore( @@ -39,7 +39,7 @@ class SaplingParamToolTest { val result = SaplingParamTool.validate(cacheDir) // Then - Assert.assertFalse(result) + assertFalse(result) } @Test @@ -52,7 +52,7 @@ class SaplingParamToolTest { val result = SaplingParamTool.validate(cacheDir) // Then - Assert.assertFalse("Validation should fail when the spend params are missing", result) + assertFalse(result, "Validation should fail when the spend params are missing") } @Test @@ -65,7 +65,7 @@ class SaplingParamToolTest { val result = SaplingParamTool.validate(cacheDir) // Then - Assert.assertFalse("Validation should fail when the spend params are missing", result) + assertFalse(result, "Validation should fail when the spend params are missing") } @Test @@ -73,13 +73,13 @@ class SaplingParamToolTest { // Given SaplingParamTool.fetchParams(cacheDir) - Assert.assertFalse("insufficient storage", false) + assertFalse(false, "insufficient storage") } @Test fun testSufficientDeviceStorageForOnlyOneFile() = runBlocking { SaplingParamTool.fetchParams(cacheDir) - Assert.assertFalse("insufficient storage", false) + assertFalse(false, "insufficient storage") } } diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/test/Global.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/test/Global.kt index cd859de7..10ba6fd1 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/test/Global.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/test/Global.kt @@ -3,9 +3,23 @@ package cash.z.ecc.android.sdk.test import android.content.Context import androidx.annotation.StringRes import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.asFlow +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn +import kotlin.test.assertNotNull fun getAppContext(): Context = ApplicationProvider.getApplicationContext() fun getStringResource(@StringRes resId: Int) = getAppContext().getString(resId) -fun getStringResourceWithArgs(@StringRes resId: Int, vararg formatArgs: String) = getAppContext().getString(resId, *formatArgs) +fun getStringResourceWithArgs(@StringRes resId: Int, vararg formatArgs: String) = + getAppContext().getString(resId, *formatArgs) + +fun readFileLinesInFlow(filePathName: String) = flow { + val testFile = javaClass.getResourceAsStream(filePathName) + assertNotNull(testFile, "Test file read failure.") + + emitAll(testFile.bufferedReader().lineSequence().asFlow()) +}.flowOn(Dispatchers.IO) diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/AddressGeneratorUtil.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/AddressGeneratorUtil.kt index faba2d7c..61e970dd 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/AddressGeneratorUtil.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/AddressGeneratorUtil.kt @@ -1,24 +1,18 @@ package cash.z.ecc.android.sdk.util -import androidx.test.platform.app.InstrumentationRegistry import cash.z.ecc.android.sdk.model.ZcashNetwork +import cash.z.ecc.android.sdk.test.readFileLinesInFlow import cash.z.ecc.android.sdk.tool.DerivationTool import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.runBlocking -import okio.buffer -import okio.source import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test -import java.io.IOException @ExperimentalCoroutinesApi class AddressGeneratorUtil { - private val context = InstrumentationRegistry.getInstrumentation().context - private val mnemonics = SimpleMnemonics() @Test @@ -32,7 +26,7 @@ class AddressGeneratorUtil { @Test fun generateAddresses() = runBlocking { - readLines() + readFileLinesInFlow("/utils/seeds.txt") .map { seedPhrase -> mnemonics.toSeed(seedPhrase.toCharArray()) }.map { seed -> @@ -42,16 +36,4 @@ class AddressGeneratorUtil { assertTrue(address.startsWith("zs1")) } } - - @Throws(IOException::class) - fun readLines() = flow { - val seedFile = javaClass.getResourceAsStream("/utils/seeds.txt")!! - seedFile.source().buffer().use { source -> - var line: String? = source.readUtf8Line() - while (line != null) { - emit(line) - line = source.readUtf8Line() - } - } - } } diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/BalancePrinterUtil.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/BalancePrinterUtil.kt index 782bdc38..40a84ec4 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/BalancePrinterUtil.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/BalancePrinterUtil.kt @@ -12,17 +12,14 @@ import cash.z.ecc.android.sdk.model.BlockHeight import cash.z.ecc.android.sdk.model.LightWalletEndpoint import cash.z.ecc.android.sdk.model.ZcashNetwork import cash.z.ecc.android.sdk.model.defaultForNetwork +import cash.z.ecc.android.sdk.test.readFileLinesInFlow import cash.z.ecc.android.sdk.tool.CheckpointTool import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.runBlocking -import okio.buffer -import okio.source import org.junit.Before import org.junit.Ignore import org.junit.Test -import java.io.IOException /** * A tool for checking transactions since the given birthday and printing balances. This was useful for the Zcon1 app to @@ -77,7 +74,7 @@ class BalancePrinterUtil { @Test @Ignore("This test is broken") fun printBalances() = runBlocking { - readLines() + readFileLinesInFlow("/utils/seeds.txt") .map { seedPhrase -> twig("checking balance for: $seedPhrase") mnemonics.toSeed(seedPhrase.toCharArray()) @@ -141,18 +138,6 @@ class BalancePrinterUtil { // assertEquals("foo", "bar") // } - @Throws(IOException::class) - fun readLines() = flow { - val seedFile = javaClass.getResourceAsStream("/utils/seeds.txt")!! - seedFile.source().buffer().use { source -> - var line: String? = source.readUtf8Line() - while (line != null) { - emit(line) - line = source.readUtf8Line() - } - } - } - // private fun initWallet(seed: String): Wallet { // val spendingKeyProvider = Delegates.notNull() // return Wallet( diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/ext/ZcashSdk.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/ext/ZcashSdk.kt index 3def355c..ffb2883b 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/ext/ZcashSdk.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/ext/ZcashSdk.kt @@ -75,7 +75,7 @@ object ZcashSdk { const val REWIND_DISTANCE = 10 /** - * File name for the sappling spend params + * File name for the sapling spend params */ const val SPEND_PARAM_FILE_NAME = "sapling-spend.params" diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt index dcd82204..20cde51b 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt @@ -7,11 +7,21 @@ import cash.z.ecc.android.sdk.internal.ext.existsSuspend import cash.z.ecc.android.sdk.internal.ext.mkdirsSuspend import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext -import okhttp3.OkHttpClient -import okhttp3.Request -import okio.buffer -import okio.sink import java.io.File +import java.net.URL +import java.nio.channels.Channels + +// TODO [#666]: https://github.com/zcash/zcash-android-wallet-sdk/issues/666 +// TODO [#666]: Download sapling-spend.params and sapling-output.params atomically + +// TODO [#665]: https://github.com/zcash/zcash-android-wallet-sdk/issues/665 +// TODO [#665]: Recover from corrupted sapling-spend.params and sapling-output.params + +// TODO [#664]: https://github.com/zcash/zcash-android-wallet-sdk/issues/664 +// TODO [#664]: Check size of download for sapling-spend.params and sapling-output.params + +// TODO [#611]: https://github.com/zcash/zcash-android-wallet-sdk/issues/611 +// TODO [#611]: Move Params Directory to No Backup Directory @Suppress("UtilityClassWithPublicConstructor") class SaplingParamTool { @@ -56,44 +66,52 @@ class SaplingParamTool { * on-demand. */ suspend fun fetchParams(destinationDir: String) { - val client = createHttpClient() var failureMessage = "" arrayOf( ZcashSdk.SPEND_PARAM_FILE_NAME, ZcashSdk.OUTPUT_PARAM_FILE_NAME ).forEach { paramFileName -> - val url = "${ZcashSdk.CLOUD_PARAM_DIR_URL}/$paramFileName" - val request = Request.Builder().url(url).build() - val response = withContext(Dispatchers.IO) { client.newCall(request).execute() } - if (response.isSuccessful) { - twig("fetch succeeded", -1) - val file = File(destinationDir, paramFileName) - if (file.parentFile?.existsSuspend() == true) { - twig("directory exists!", -1) - } else { - twig("directory did not exist attempting to make it") - file.parentFile?.mkdirsSuspend() - } - withContext(Dispatchers.IO) { - response.body?.let { body -> - body.source().use { source -> - file.sink().buffer().use { sink -> - twig("writing to $file") - sink.writeAll(source) + val url = URL("${ZcashSdk.CLOUD_PARAM_DIR_URL}/$paramFileName") + + val file = File(destinationDir, paramFileName) + if (file.parentFile?.existsSuspend() == true) { + twig("Directory ${file.parentFile?.name} exists!") + } else { + twig("Directory did not exist attempting to make it.") + file.parentFile?.mkdirsSuspend() + } + + withContext(Dispatchers.IO) { + runCatching { + Channels.newChannel(url.openStream()).use { readableByteChannel -> + file.outputStream().use { fileOutputStream -> + fileOutputStream.channel.use { fileChannel -> + // transfers bytes from stream to file (position 0 to the end position) + fileChannel.transferFrom(readableByteChannel, 0, Long.MAX_VALUE) } } } + }.onFailure { exception -> + // IllegalArgumentException - If the preconditions on the parameters do not hold + // NonReadableChannelException - If the source channel was not opened for reading + // NonWritableChannelException - If this channel was not opened for writing + // ClosedChannelException - If either this channel or the source channel is closed + // AsynchronousCloseException - If another thread closes either channel while the transfer is + // in progress + // ClosedByInterruptException - If another thread interrupts the current thread while the + // transfer is in progress, thereby closing both channels and setting the current thread's + // interrupt status + // IOException - If some other I/O error occurs + failureMessage += "Error while fetching $paramFileName, caused by $exception\n" + twig(failureMessage) + }.onSuccess { + twig("Fetch and write of $paramFileName succeeded.") } - } else { - failureMessage += "Error while fetching $paramFileName : $response\n" - twig(failureMessage) } - - twig("fetch succeeded, done writing $paramFileName") } - if (failureMessage.isNotEmpty()) throw TransactionEncoderException.FetchParamsException( - failureMessage - ) + if (failureMessage.isNotEmpty()) { + throw TransactionEncoderException.FetchParamsException(failureMessage) + } } suspend fun clear(destinationDir: String) { @@ -122,20 +140,5 @@ class SaplingParamTool { println("Param files${if (!it) "did not" else ""} both exist!") } } - - // - // Helpers - // - /** - * Http client is only used for downloading sapling spend and output params data, which are - * necessary for the wallet to scan blocks. - * - * @return an http client suitable for downloading params data. - */ - private fun createHttpClient(): OkHttpClient { - // TODO [#686]: add logging and timeouts - // TODO [#686]: https://github.com/zcash/zcash-android-wallet-sdk/issues/686 - return OkHttpClient() - } } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 1c9046c7..c3bd3bc0 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -92,8 +92,6 @@ dependencyResolutionManagement { val kotlinxCoroutinesVersion = extra["KOTLINX_COROUTINES_VERSION"].toString() val mockitoKotlinVersion = extra["MOCKITO_KOTLIN_VERSION"].toString() val mockitoVersion = extra["MOCKITO_VERSION"].toString() - val okhttpVersion = extra["OKHTTP_VERSION"].toString() - val okioVersion = extra["OKIO_VERSION"].toString() val protocVersion = extra["PROTOC_VERSION"].toString() val rustGradlePluginVersion = extra["RUST_GRADLE_PLUGIN_VERSION"].toString() val zcashWalletPluginVersion = extra["ZCASH_WALLET_PLUGINS_VERSION"].toString() @@ -141,8 +139,6 @@ dependencyResolutionManagement { library("kotlinx-coroutines-android", "org.jetbrains.kotlinx:kotlinx-coroutines-android:$kotlinxCoroutinesVersion") library("kotlinx-coroutines-core", "org.jetbrains.kotlinx:kotlinx-coroutines-core:$kotlinxCoroutinesVersion") library("material", "com.google.android.material:material:$googleMaterialVersion") - library("okhttp", "com.squareup.okhttp3:okhttp:$okhttpVersion") - library("okio", "com.squareup.okio:okio:$okioVersion") library("zcashwalletplgn", "com.github.zcash:zcash-android-wallet-plugins:$zcashWalletPluginVersion") // Test libraries From 12c23dd054c687431aaf51bfc5f67d5dbc08625b Mon Sep 17 00:00:00 2001 From: Honza Rychnovsky Date: Fri, 26 Aug 2022 15:31:56 +0200 Subject: [PATCH 04/24] [#664] Check sapling files size * [#664] Transfer maximum bytes from remote file * Tests for limiting sapling files max size * Address comments from review * Optimization of sapling files download * Add units to data class Co-authored-by: Carter Jernigan --- .../sdk/internal/SaplingParamToolTest.kt | 96 ++++++-- .../z/ecc/fixture/SaplingParamsFixture.kt | 28 +++ .../android/sdk/internal/SaplingParamTool.kt | 232 ++++++++++-------- 3 files changed, 227 insertions(+), 129 deletions(-) create mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/fixture/SaplingParamsFixture.kt diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt index ba72ab72..edcdbc06 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt @@ -1,85 +1,137 @@ package cash.z.ecc.android.sdk.internal -import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 -import androidx.test.platform.app.InstrumentationRegistry -import cash.z.ecc.android.sdk.ext.ZcashSdk +import cash.z.ecc.fixture.SaplingParamsFixture import kotlinx.coroutines.runBlocking import org.junit.Before import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import java.io.File +import kotlin.test.assertContains import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue -@RunWith(AndroidJUnit4::class) @Ignore( "These tests need to be refactored to a separate test module. They cause SSLHandshakeException: Chain " + "validation failed on CI" ) +@RunWith(AndroidJUnit4::class) class SaplingParamToolTest { - val context: Context = InstrumentationRegistry.getInstrumentation().context + private val spendSaplingParams = SaplingParamsFixture.newFile() - val cacheDir = "${context.cacheDir.absolutePath}/params" + private val outputSaplingParams = SaplingParamsFixture.newFile( + SaplingParamsFixture.DESTINATION_DIRECTORY, + SaplingParamsFixture.OUTPUT_FILE_NAME, + SaplingParamsFixture.OUTPUT_FILE_MAX_SIZE + ) @Before fun setup() { // clear the param files - runBlocking { SaplingParamTool.clear(cacheDir) } + runBlocking { SaplingParamTool.clear(spendSaplingParams.destinationDirectoryPath) } } @Test - @Ignore("This test is broken") - fun testFilesExists() = runBlocking { + fun test_files_exists() = runBlocking { // Given - SaplingParamTool.fetchParams(cacheDir) + SaplingParamTool.fetchParams(spendSaplingParams) + SaplingParamTool.fetchParams(outputSaplingParams) // When - val result = SaplingParamTool.validate(cacheDir) + val result = SaplingParamTool.validate(SaplingParamsFixture.DESTINATION_DIRECTORY) // Then - assertFalse(result) + assertTrue(result) } @Test fun output_file_exists() = runBlocking { // Given - SaplingParamTool.fetchParams(cacheDir) - File(cacheDir, ZcashSdk.OUTPUT_PARAM_FILE_NAME).delete() + SaplingParamTool.fetchParams(spendSaplingParams) + File(spendSaplingParams.destinationDirectoryPath, spendSaplingParams.fileName).delete() // When - val result = SaplingParamTool.validate(cacheDir) + val result = SaplingParamTool.validate(spendSaplingParams.destinationDirectoryPath) // Then assertFalse(result, "Validation should fail when the spend params are missing") } @Test - fun param_file_exists() = runBlocking { + fun spend_file_exists() = runBlocking { // Given - SaplingParamTool.fetchParams(cacheDir) - File(cacheDir, ZcashSdk.SPEND_PARAM_FILE_NAME).delete() + SaplingParamTool.fetchParams(outputSaplingParams) + File(outputSaplingParams.destinationDirectoryPath, outputSaplingParams.fileName).delete() // When - val result = SaplingParamTool.validate(cacheDir) + val result = SaplingParamTool.validate(outputSaplingParams.destinationDirectoryPath) // Then - assertFalse(result, "Validation should fail when the spend params are missing") + assertFalse(result, "Validation should fail when the output params are missing") } @Test fun testInsufficientDeviceStorage() = runBlocking { // Given - SaplingParamTool.fetchParams(cacheDir) + SaplingParamTool.fetchParams(spendSaplingParams) assertFalse(false, "insufficient storage") } @Test fun testSufficientDeviceStorageForOnlyOneFile() = runBlocking { - SaplingParamTool.fetchParams(cacheDir) + SaplingParamTool.fetchParams(spendSaplingParams) assertFalse(false, "insufficient storage") } + + @Test + fun check_all_files_fetched() = runBlocking { + val expectedSpendFile = File( + SaplingParamsFixture.DESTINATION_DIRECTORY, + SaplingParamsFixture.SPEND_FILE_NAME + ) + val expectedOutputFile = File( + SaplingParamsFixture.DESTINATION_DIRECTORY, + SaplingParamsFixture.OUTPUT_FILE_NAME + ) + + SaplingParamTool.ensureParams(SaplingParamsFixture.DESTINATION_DIRECTORY) + + val actualFiles = File(SaplingParamsFixture.DESTINATION_DIRECTORY).listFiles() + assertNotNull(actualFiles) + + assertContains(actualFiles, expectedSpendFile) + + assertContains(actualFiles, expectedOutputFile) + } + + @Test + fun check_correct_spend_param_file_size() = runBlocking { + SaplingParamTool.fetchParams(spendSaplingParams) + + val expectedSpendFile = File( + SaplingParamsFixture.DESTINATION_DIRECTORY, + SaplingParamsFixture.SPEND_FILE_NAME + ) + + assertTrue(expectedSpendFile.length() < SaplingParamsFixture.SPEND_FILE_MAX_SIZE) + assertFalse(expectedSpendFile.length() < SaplingParamsFixture.OUTPUT_FILE_MAX_SIZE) + } + + @Test + fun check_correct_output_param_file_size() = runBlocking { + SaplingParamTool.fetchParams(outputSaplingParams) + + val expectedOutputFile = File( + SaplingParamsFixture.DESTINATION_DIRECTORY, + SaplingParamsFixture.OUTPUT_FILE_NAME + ) + + assertTrue(expectedOutputFile.length() < SaplingParamsFixture.OUTPUT_FILE_MAX_SIZE) + assertFalse(expectedOutputFile.length() > SaplingParamsFixture.SPEND_FILE_MAX_SIZE) + } } diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/SaplingParamsFixture.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/SaplingParamsFixture.kt new file mode 100644 index 00000000..bbac723a --- /dev/null +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/SaplingParamsFixture.kt @@ -0,0 +1,28 @@ +package cash.z.ecc.fixture + +import cash.z.ecc.android.sdk.ext.ZcashSdk +import cash.z.ecc.android.sdk.internal.SaplingFileParameters +import cash.z.ecc.android.sdk.internal.SaplingParamTool +import cash.z.ecc.android.sdk.test.getAppContext +import java.io.File + +object SaplingParamsFixture { + + val DESTINATION_DIRECTORY: String = File(getAppContext().cacheDir, "params").absolutePath + + const val SPEND_FILE_NAME = ZcashSdk.SPEND_PARAM_FILE_NAME + const val SPEND_FILE_MAX_SIZE = SaplingParamTool.SPEND_PARAM_FILE_MAX_BYTES_SIZE + + const val OUTPUT_FILE_NAME = ZcashSdk.OUTPUT_PARAM_FILE_NAME + const val OUTPUT_FILE_MAX_SIZE = SaplingParamTool.OUTPUT_PARAM_FILE_MAX_BYTES_SIZE + + internal fun newFile( + destinationDirectoryPath: String = DESTINATION_DIRECTORY, + fileName: String = SPEND_FILE_NAME, + fileMaxSize: Long = SPEND_FILE_MAX_SIZE + ) = SaplingFileParameters( + destinationDirectoryPath = destinationDirectoryPath, + fileName = fileName, + fileMaxSizeBytes = fileMaxSize + ) +} diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt index 20cde51b..3856f1af 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/SaplingParamTool.kt @@ -17,128 +17,146 @@ import java.nio.channels.Channels // TODO [#665]: https://github.com/zcash/zcash-android-wallet-sdk/issues/665 // TODO [#665]: Recover from corrupted sapling-spend.params and sapling-output.params -// TODO [#664]: https://github.com/zcash/zcash-android-wallet-sdk/issues/664 -// TODO [#664]: Check size of download for sapling-spend.params and sapling-output.params - // TODO [#611]: https://github.com/zcash/zcash-android-wallet-sdk/issues/611 // TODO [#611]: Move Params Directory to No Backup Directory -@Suppress("UtilityClassWithPublicConstructor") -class SaplingParamTool { +object SaplingParamTool { + /** + * Maximum file size for the sapling spend params - 50MB + */ + internal const val SPEND_PARAM_FILE_MAX_BYTES_SIZE = 50L * 1024L * 1024L - companion object { - /** - * Checks the given directory for the output and spending params and calls [fetchParams] if - * they're missing. - * - * @param destinationDir the directory where the params should be stored. - */ - suspend fun ensureParams(destinationDir: String) { - var hadError = false - arrayOf( + /** + * Maximum file size for the sapling spend params - 5MB + */ + internal const val OUTPUT_PARAM_FILE_MAX_BYTES_SIZE = 5L * 1024L * 1024L + + /** + * Checks the given directory for the output and spending params and calls [fetchParams] for those, which are + * missing. + * + * @param destinationDir the directory where the params should be stored. + * + * @throws TransactionEncoderException.MissingParamsException in case of failure while checking sapling params + * files + */ + @Throws(TransactionEncoderException.MissingParamsException::class) + suspend fun ensureParams(destinationDir: String) { + arrayOf( + SaplingFileParameters( + destinationDir, ZcashSdk.SPEND_PARAM_FILE_NAME, - ZcashSdk.OUTPUT_PARAM_FILE_NAME - ).forEach { paramFileName -> - if (!File(destinationDir, paramFileName).existsSuspend()) { - twig("WARNING: $paramFileName not found at location: $destinationDir") - hadError = true - } - } - if (hadError) { - @Suppress("TooGenericExceptionCaught") - try { - Bush.trunk.twigTask("attempting to download missing params") { - fetchParams(destinationDir) - } - } catch (e: Throwable) { - twig("failed to fetch params due to: $e") - throw TransactionEncoderException.MissingParamsException - } + SPEND_PARAM_FILE_MAX_BYTES_SIZE + ), + SaplingFileParameters( + destinationDir, + ZcashSdk.OUTPUT_PARAM_FILE_NAME, + OUTPUT_PARAM_FILE_MAX_BYTES_SIZE + ) + ).filter { + !File(it.destinationDirectoryPath, it.fileName).existsSuspend() + }.forEach { + try { + twig("Attempting to download missing params: ${it.fileName}.") + fetchParams(it) + } catch (e: TransactionEncoderException.FetchParamsException) { + twig("Failed to fetch params ${it.fileName} due to: $e") + throw TransactionEncoderException.MissingParamsException } } - /** - * Download and store the params into the given directory. - * - * @param destinationDir the directory where the params will be stored. It's assumed that we - * have write access to this directory. Typically, this should be the app's cache directory - * because it is not harmful if these files are cleared by the user since they are downloaded - * on-demand. - */ - suspend fun fetchParams(destinationDir: String) { - var failureMessage = "" - arrayOf( - ZcashSdk.SPEND_PARAM_FILE_NAME, - ZcashSdk.OUTPUT_PARAM_FILE_NAME - ).forEach { paramFileName -> - val url = URL("${ZcashSdk.CLOUD_PARAM_DIR_URL}/$paramFileName") + if (!validate(destinationDir)) { + twig("Fetching sapling params files failed.") + throw TransactionEncoderException.MissingParamsException + } + } - val file = File(destinationDir, paramFileName) - if (file.parentFile?.existsSuspend() == true) { - twig("Directory ${file.parentFile?.name} exists!") - } else { - twig("Directory did not exist attempting to make it.") - file.parentFile?.mkdirsSuspend() - } + /** + * Download and store the params file into the given directory. It also checks the file size to eliminate the + * risk of downloading potentially large file from a malicious server. + * + * @param paramsToFetch parameters wrapper class, which holds information about it + * + * @throws TransactionEncoderException.FetchParamsException if any error while downloading the params file occurs + */ + @Throws(TransactionEncoderException.FetchParamsException::class) + internal suspend fun fetchParams(paramsToFetch: SaplingFileParameters) { + val url = URL("${ZcashSdk.CLOUD_PARAM_DIR_URL}/${paramsToFetch.fileName}") - withContext(Dispatchers.IO) { - runCatching { - Channels.newChannel(url.openStream()).use { readableByteChannel -> - file.outputStream().use { fileOutputStream -> - fileOutputStream.channel.use { fileChannel -> - // transfers bytes from stream to file (position 0 to the end position) - fileChannel.transferFrom(readableByteChannel, 0, Long.MAX_VALUE) - } - } + val file = File(paramsToFetch.destinationDirectoryPath, paramsToFetch.fileName) + if (file.parentFile?.existsSuspend() == true) { + twig("Directory ${file.parentFile?.name} exists!") + } else { + twig("Directory did not exist attempting to make it.") + file.parentFile?.mkdirsSuspend() + } + + withContext(Dispatchers.IO) { + runCatching { + Channels.newChannel(url.openStream()).use { readableByteChannel -> + file.outputStream().use { fileOutputStream -> + fileOutputStream.channel.use { fileChannel -> + // Transfers bytes from stream to file from position 0 to end position or to max + // file size limit. This eliminates the risk of downloading potentially large files + // from a malicious server. We need to make a check of the file hash then. + fileChannel.transferFrom(readableByteChannel, 0, paramsToFetch.fileMaxSizeBytes) } - }.onFailure { exception -> - // IllegalArgumentException - If the preconditions on the parameters do not hold - // NonReadableChannelException - If the source channel was not opened for reading - // NonWritableChannelException - If this channel was not opened for writing - // ClosedChannelException - If either this channel or the source channel is closed - // AsynchronousCloseException - If another thread closes either channel while the transfer is - // in progress - // ClosedByInterruptException - If another thread interrupts the current thread while the - // transfer is in progress, thereby closing both channels and setting the current thread's - // interrupt status - // IOException - If some other I/O error occurs - failureMessage += "Error while fetching $paramFileName, caused by $exception\n" - twig(failureMessage) - }.onSuccess { - twig("Fetch and write of $paramFileName succeeded.") } } - } - if (failureMessage.isNotEmpty()) { - throw TransactionEncoderException.FetchParamsException(failureMessage) - } - } - - suspend fun clear(destinationDir: String) { - if (validate(destinationDir)) { - arrayOf( - ZcashSdk.SPEND_PARAM_FILE_NAME, - ZcashSdk.OUTPUT_PARAM_FILE_NAME - ).forEach { paramFileName -> - val file = File(destinationDir, paramFileName) - if (file.deleteRecursivelySuspend()) { - twig("Files deleted successfully") - } else { - twig("Error: Files not able to be deleted!") - } + }.onFailure { exception -> + // IllegalArgumentException - If the preconditions on the parameters do not hold + // NonReadableChannelException - If the source channel was not opened for reading + // NonWritableChannelException - If this channel was not opened for writing + // ClosedChannelException - If either this channel or the source channel is closed + // AsynchronousCloseException - If another thread closes either channel while the transfer is + // in progress + // ClosedByInterruptException - If another thread interrupts the current thread while the + // transfer is in progress, thereby closing both channels and setting the current thread's + // interrupt status + // IOException - If some other I/O error occurs + "Error while fetching ${paramsToFetch.fileName}, caused by $exception".also { + twig(it) + throw TransactionEncoderException.FetchParamsException(it) } - } - } - - suspend fun validate(destinationDir: String): Boolean { - return arrayOf( - ZcashSdk.SPEND_PARAM_FILE_NAME, - ZcashSdk.OUTPUT_PARAM_FILE_NAME - ).all { paramFileName -> - File(destinationDir, paramFileName).existsSuspend() - }.also { - println("Param files${if (!it) "did not" else ""} both exist!") + }.onSuccess { + twig("Fetch and write of ${paramsToFetch.fileName} succeeded.") } } } + + suspend fun clear(destinationDir: String) { + if (validate(destinationDir)) { + arrayOf( + ZcashSdk.SPEND_PARAM_FILE_NAME, + ZcashSdk.OUTPUT_PARAM_FILE_NAME + ).forEach { paramFileName -> + val file = File(destinationDir, paramFileName) + if (file.deleteRecursivelySuspend()) { + twig("Files deleted successfully") + } else { + twig("Error: Files not able to be deleted!") + } + } + } + } + + suspend fun validate(destinationDir: String): Boolean { + return arrayOf( + ZcashSdk.SPEND_PARAM_FILE_NAME, + ZcashSdk.OUTPUT_PARAM_FILE_NAME + ).all { paramFileName -> + File(destinationDir, paramFileName).existsSuspend() + }.also { + println("Param files ${if (!it) "did not" else ""} both exist!") + } + } } + +/** + * Sapling file parameters class to hold each sapling file attributes. + */ +internal data class SaplingFileParameters( + val destinationDirectoryPath: String, + val fileName: String, + val fileMaxSizeBytes: Long +) From ec6b714f925fcfabe2982cd9901ef649e7a523e2 Mon Sep 17 00:00:00 2001 From: Bruno Wieczorek Date: Sat, 27 Aug 2022 14:25:54 +0200 Subject: [PATCH 05/24] [#288] Replace Deprecated Usage of `ConflatedBroadcastChannel` Co-authored-by: Carter Jernigan --- .../cash/z/ecc/android/sdk/SdkSynchronizer.kt | 18 +++----- .../sdk/block/CompactBlockProcessor.kt | 43 ++++++------------- .../internal/ext/android/ComputableFlow.kt | 22 +++------- .../ext/android/ComputableFlowTest.kt | 35 +++++++++++++++ 4 files changed, 61 insertions(+), 57 deletions(-) create mode 100644 sdk-lib/src/test/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlowTest.kt diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt index fe07fcaf..4aa6ac93 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt @@ -66,11 +66,9 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel -import kotlinx.coroutines.channels.ConflatedBroadcastChannel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first @@ -95,7 +93,6 @@ import kotlin.coroutines.EmptyCoroutineContext * @property processor saves the downloaded compact blocks to the cache and then scans those blocks for * data related to this wallet. */ -@OptIn(kotlinx.coroutines.ObsoleteCoroutinesApi::class) @FlowPreview @Suppress("TooManyFunctions") class SdkSynchronizer internal constructor( @@ -109,9 +106,7 @@ class SdkSynchronizer internal constructor( private val _saplingBalances = MutableStateFlow(null) private val _transparentBalances = MutableStateFlow(null) - // TODO [#288]: Remove Deprecated Usage of ConflatedBroadcastChannel - // TODO [#288]: https://github.com/zcash/zcash-android-wallet-sdk/issues/288 - private val _status = ConflatedBroadcastChannel(DISCONNECTED) + private val _status = MutableStateFlow(DISCONNECTED) /** * The lifespan of this Synchronizer. This scope is initialized once the Synchronizer starts @@ -173,10 +168,7 @@ class SdkSynchronizer internal constructor( * processor is finished scanning, the synchronizer updates transaction and balance info and * then emits a [SYNCED] status. */ - // TODO [#658] Replace ComputableFlow and asFlow() obsolete Coroutine usage - // TODO [#658] https://github.com/zcash/zcash-android-wallet-sdk/issues/658 - @Suppress("DEPRECATION") - override val status = _status.asFlow() + override val status = _status.asStateFlow() /** * Indicates the download progress of the Synchronizer. When progress reaches 100, that @@ -301,8 +293,8 @@ class SdkSynchronizer internal constructor( processor.stop() twig("Synchronizer::stop: coroutineScope.cancel()") coroutineScope.cancel() - twig("Synchronizer::stop: _status.cancel()") - _status.cancel() + twig("Synchronizer::stop: _status.value = STOPPED") + _status.value = STOPPED twig("Synchronizer::stop: COMPLETE") } } @@ -412,7 +404,7 @@ class SdkSynchronizer internal constructor( // ignore enhancing status for now // TODO [#682]: clean this up and handle enhancing gracefully // TODO [#682]: https://github.com/zcash/zcash-android-wallet-sdk/issues/682 - if (synchronizerStatus != ENHANCING) _status.send(synchronizerStatus) + if (synchronizerStatus != ENHANCING) _status.value = synchronizerStatus } }.launchIn(this) processor.start() 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 25df0a16..0ab35267 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,10 +46,8 @@ import cash.z.wallet.sdk.rpc.Service import io.grpc.StatusRuntimeException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.channels.ConflatedBroadcastChannel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.isActive @@ -77,7 +75,6 @@ import kotlin.time.Duration.Companion.days * in when considering initial range to download. In most cases, this should be the birthday height * of the current wallet--the height before which we do not need to scan for transactions. */ -@OptIn(kotlinx.coroutines.ObsoleteCoroutinesApi::class) @OpenForTesting @Suppress("TooManyFunctions", "LargeClass") class CompactBlockProcessor internal constructor( @@ -129,12 +126,9 @@ class CompactBlockProcessor internal constructor( ) ) - // TODO [#288]: Remove Deprecated Usage of ConflatedBroadcastChannel - // TODO [#288]: https://github.com/zcash/zcash-android-wallet-sdk/issues/288 - private val _state: ConflatedBroadcastChannel = ConflatedBroadcastChannel(Initialized) - private val _progress = ConflatedBroadcastChannel(0) - private val _processorInfo = - ConflatedBroadcastChannel(ProcessorInfo(null, null, null, null, null)) + private val _state: MutableStateFlow = MutableStateFlow(Initialized) + private val _progress = MutableStateFlow(0) + private val _processorInfo = MutableStateFlow(ProcessorInfo(null, null, null, null, null)) private val _networkHeight = MutableStateFlow(null) private val processingMutex = Mutex() @@ -166,28 +160,19 @@ class CompactBlockProcessor internal constructor( * The flow of state values so that a wallet can monitor the state of this class without needing * to poll. */ - // TODO [#658] Replace ComputableFlow and asFlow() obsolete Coroutine usage - // TODO [#658] https://github.com/zcash/zcash-android-wallet-sdk/issues/658 - @Suppress("DEPRECATION") - val state = _state.asFlow() + val state = _state.asStateFlow() /** * The flow of progress values so that a wallet can monitor how much downloading remains * without needing to poll. */ - // TODO [#658] Replace ComputableFlow and asFlow() obsolete Coroutine usage - // TODO [#658] https://github.com/zcash/zcash-android-wallet-sdk/issues/658 - @Suppress("DEPRECATION") - val progress = _progress.asFlow() + val progress = _progress.asStateFlow() /** * The flow of detailed processorInfo like the range of blocks that shall be downloaded and * scanned. This gives the wallet a lot of insight into the work of this processor. */ - // TODO [#658] Replace ComputableFlow and asFlow() obsolete Coroutine usage - // TODO [#658] https://github.com/zcash/zcash-android-wallet-sdk/issues/658 - @Suppress("DEPRECATION") - val processorInfo = _processorInfo.asFlow() + val processorInfo = _processorInfo.asStateFlow() /** * The flow of network height. This value is updated at the same time that [currentInfo] is @@ -260,7 +245,7 @@ class CompactBlockProcessor internal constructor( } } } - } while (isActive && !_state.isClosedForSend && _state.value !is Stopped) + } while (isActive && _state.value !is Stopped) twig("processor complete") stop() } @@ -600,7 +585,7 @@ class CompactBlockProcessor internal constructor( if (null == range || range.isEmpty()) { twig("no blocks to download") } else { - _state.send(Downloading) + _state.value = Downloading Twig.sprout("downloading") twig("downloading blocks in range $range", -1) @@ -633,7 +618,7 @@ class CompactBlockProcessor internal constructor( } twig("downloaded $count blocks!") progress = (i / batches.toFloat() * 100).roundToInt() - _progress.send(progress) + _progress.value = progress val lastDownloadedHeight = downloader.getLastDownloadedHeight() updateProgress(lastDownloadedHeight = lastDownloadedHeight) downloadedBlockHeight = end + 1 @@ -641,7 +626,7 @@ class CompactBlockProcessor internal constructor( } Twig.clip("downloading") } - _progress.send(100) + _progress.value = 100 } /** @@ -756,7 +741,7 @@ class CompactBlockProcessor internal constructor( withContext(IO) { _networkHeight.value = networkBlockHeight - _processorInfo.send(currentInfo) + _processorInfo.value = currentInfo } } @@ -859,7 +844,7 @@ class CompactBlockProcessor internal constructor( lastDownloadRange = (targetHeight + 1)..currentNetworkBlockHeight ) } - _progress.send(0) + _progress.value = 0 } else { if (null == currentNetworkBlockHeight) { updateProgress( @@ -873,7 +858,7 @@ class CompactBlockProcessor internal constructor( ) } - _progress.send(0) + _progress.value = 0 if (null != lastScannedHeight) { val range = (targetHeight + 1)..lastScannedHeight @@ -1058,7 +1043,7 @@ class CompactBlockProcessor internal constructor( * Transmits the given state for this processor. */ private suspend fun setState(newState: State) { - _state.send(newState) + _state.value = newState } /** diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlow.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlow.kt index b9829e45..c4550b84 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlow.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlow.kt @@ -3,37 +3,29 @@ package cash.z.ecc.android.sdk.internal.ext.android import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ObsoleteCoroutinesApi -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel -import kotlinx.coroutines.channels.ConflatedBroadcastChannel -import kotlinx.coroutines.flow.asFlow -import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch /* Adapted from ComputableLiveData */ -// TODO [#658] https://github.com/zcash/zcash-android-wallet-sdk/issues/658 -@Suppress("DEPRECATION") -@OptIn(ObsoleteCoroutinesApi::class) abstract class ComputableFlow(dispatcher: CoroutineDispatcher = Dispatchers.IO) { - private val computationScope: CoroutineScope = CoroutineScope(dispatcher + SupervisorJob()) - private val computationChannel: ConflatedBroadcastChannel = ConflatedBroadcastChannel() - internal val flow = computationChannel.asFlow().flowOn(dispatcher).onStart { - invalidate() - } + private val computationScope: CoroutineScope = CoroutineScope(dispatcher) + private val computationFlow: MutableSharedFlow = MutableSharedFlow(replay = 1) + internal val flow = computationFlow.asSharedFlow().onStart { invalidate() } /** * Invalidates the flow. * This will trigger a call to [.compute]. */ fun invalidate() { - computationScope.launch { computationChannel.send(compute()) } + computationScope.launch { computationFlow.emit(compute()) } } fun cancel() { computationScope.cancel() - computationChannel.cancel() + computationFlow.resetReplayCache() } protected abstract fun compute(): T diff --git a/sdk-lib/src/test/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlowTest.kt b/sdk-lib/src/test/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlowTest.kt new file mode 100644 index 00000000..4f3d10b5 --- /dev/null +++ b/sdk-lib/src/test/java/cash/z/ecc/android/sdk/internal/ext/android/ComputableFlowTest.kt @@ -0,0 +1,35 @@ +package cash.z.ecc.android.sdk.internal.ext.android + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.Test +import kotlin.test.assertEquals + +class ComputableFlowTest { + + private class TestComputableFlow : ComputableFlow(dispatcher = Dispatchers.Unconfined) { + var computationCounter: Int = 0 + private set + + override fun compute() = ++computationCounter + } + + @Test + fun shouldComputeOnInvalidation() { + val testComputableFlow = TestComputableFlow() + + testComputableFlow.invalidate() + testComputableFlow.invalidate() + + assertEquals(2, testComputableFlow.computationCounter) + } + + @Test + fun shouldInvalidateOnCollection() = runBlocking { + val testComputableFlow = TestComputableFlow() + testComputableFlow.flow.first() + + assertEquals(1, testComputableFlow.computationCounter) + } +} From 251bb4dbfcb6b528bc0084e2740d7ff78e5736bf Mon Sep 17 00:00:00 2001 From: Carter Jernigan Date: Wed, 31 Aug 2022 11:17:47 -0400 Subject: [PATCH 06/24] [#697] Fix running robo test (#698) --- .github/workflows/pull-request.yml | 5 ++++- build.gradle.kts | 8 +++---- demo-app/build.gradle.kts | 35 ++++++++++++++++++++++++++---- gradle.properties | 7 +++++- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 4e09e985..19b56229 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -163,6 +163,7 @@ jobs: run: | ./gradlew :sdk-lib:lintRelease :demo-app:lintZcashmainnetRelease - name: Collect Artifacts + if: ${{ always() }} timeout-minutes: 1 env: ARTIFACTS_DIR_PATH: ${{ format('{0}/artifacts', env.home) }} @@ -171,6 +172,7 @@ jobs: mkdir ${ARTIFACTS_DIR_PATH} zip -r ${LINT_ZIP_PATH} . -i \*build/reports/\* - name: Upload Artifacts + if: ${{ always() }} uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 timeout-minutes: 1 with: @@ -361,6 +363,7 @@ jobs: permissions: packages: read contents: read + id-token: write steps: - name: Checkout timeout-minutes: 1 @@ -381,7 +384,7 @@ jobs: - name: Download a single artifact uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 with: - name: Release binaries + name: Demo app release binaries - name: Robo test timeout-minutes: 15 env: diff --git a/build.gradle.kts b/build.gradle.kts index 6f51cbe9..d35f6463 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -60,10 +60,10 @@ fladle { // Firebase Test Lab has min and max values that might differ from our project's // These are determined by `gcloud firebase test android models list` @Suppress("MagicNumber", "PropertyName", "VariableNaming") - val FIREBASE_TEST_LAB_MIN_API = 23 + val FIREBASE_TEST_LAB_MIN_API = 19 @Suppress("MagicNumber", "PropertyName", "VariableNaming") - val FIREBASE_TEST_LAB_MAX_API = 30 + val FIREBASE_TEST_LAB_MAX_API = 33 val minSdkVersion = run { val buildMinSdk = project.properties["ANDROID_MIN_SDK_VERSION"].toString().toInt() @@ -84,8 +84,8 @@ fladle { } devices.addAll( - mapOf("model" to "Pixel2", "version" to minSdkVersion), - mapOf("model" to "Pixel2", "version" to targetSdkVersion) + mapOf("model" to "Nexus5", "version" to minSdkVersion), + mapOf("model" to "Pixel2.arm", "version" to targetSdkVersion) ) @Suppress("MagicNumber") diff --git a/demo-app/build.gradle.kts b/demo-app/build.gradle.kts index 0a3e795c..6914ddd5 100644 --- a/demo-app/build.gradle.kts +++ b/demo-app/build.gradle.kts @@ -20,6 +20,30 @@ android { viewBinding = true } + val releaseKeystorePath = project.property("ZCASH_RELEASE_KEYSTORE_PATH").toString() + val releaseKeystorePassword = project.property("ZCASH_RELEASE_KEYSTORE_PASSWORD").toString() + val releaseKeyAlias = project.property("ZCASH_RELEASE_KEY_ALIAS").toString() + val releaseKeyAliasPassword = + project.property("ZCASH_RELEASE_KEY_ALIAS_PASSWORD").toString() + val isReleaseSigningConfigured = listOf( + releaseKeystorePath, + releaseKeystorePassword, + releaseKeyAlias, + releaseKeyAliasPassword + ).all { it.isNotBlank() } + + signingConfigs { + if (isReleaseSigningConfigured) { + // If this block doesn't execute, the output will be unsigned + create("release").apply { + storeFile = File(releaseKeystorePath) + storePassword = releaseKeystorePassword + keyAlias = releaseKeyAlias + keyPassword = releaseKeyAliasPassword + } + } + } + flavorDimensions.add("network") productFlavors { @@ -46,6 +70,9 @@ android { File("proguard-project.txt") ) ) + if (isReleaseSigningConfigured) { + signingConfig = signingConfigs.getByName("release") + } } } @@ -78,10 +105,10 @@ fladle { // Firebase Test Lab has min and max values that might differ from our project's // These are determined by `gcloud firebase test android models list` @Suppress("MagicNumber", "PropertyName", "VariableNaming") - val FIREBASE_TEST_LAB_MIN_API = 23 + val FIREBASE_TEST_LAB_MIN_API = 19 @Suppress("MagicNumber", "PropertyName", "VariableNaming") - val FIREBASE_TEST_LAB_MAX_API = 30 + val FIREBASE_TEST_LAB_MAX_API = 33 val minSdkVersion = run { val buildMinSdk = @@ -116,8 +143,8 @@ fladle { testTimeout.set("5m") devices.addAll( - mapOf("model" to "Pixel2", "version" to minSdkVersion), - mapOf("model" to "Pixel2", "version" to targetSdkVersion) + mapOf("model" to "Nexus5", "version" to minSdkVersion), + mapOf("model" to "Pixel2.arm", "version" to targetSdkVersion) ) flankVersion.set(libs.versions.flank.get()) diff --git a/gradle.properties b/gradle.properties index 3afd162b..384637d9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -52,11 +52,16 @@ IS_MINIFY_APP_ENABLED=true # are overridden via ~/.gradle/gradle.properties to allow secure injection. # Debug keystore is useful if using Google Maps or Firebase, which require API keys to be linked # to a signing key. Without a debug keystore, the default Android debug keystore will be used. +# Without a release signing configuration, the release output will not be signed. ZCASH_DEBUG_KEYSTORE_PATH= +ZCASH_RELEASE_KEYSTORE_PATH= +ZCASH_RELEASE_KEYSTORE_PASSWORD= +ZCASH_RELEASE_KEY_ALIAS= +ZCASH_RELEASE_KEY_ALIAS_PASSWORD= # Versions ANDROID_MIN_SDK_VERSION=19 -ANDROID_TARGET_SDK_VERSION=31 +ANDROID_TARGET_SDK_VERSION=33 ANDROID_COMPILE_SDK_VERSION=33 # TODO[#317]: https://github.com/zcash/zcash-android-wallet-sdk/issues/317 From cf5b762fd1f1a8254047eed48eabd04ce598bb3e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 06:36:25 -0400 Subject: [PATCH 07/24] Bump actions/cache from 3.0.7 to 3.0.8 in /.github/actions/setup (#690) Bumps [actions/cache](https://github.com/actions/cache) from 3.0.7 to 3.0.8. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/a7c34adf76222e77931dedbf4a45b2e4648ced19...fd5de65bc895cf536527842281bea11763fefd77) --- updated-dependencies: - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/actions/setup/action.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index 719c6585..12c6f69d 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -31,13 +31,13 @@ runs: echo "org.gradle.daemon=false" >> ~/.gradle/gradle.properties - name: Gradle Wrapper Cache id: gradle-wrapper-cache - uses: actions/cache@a7c34adf76222e77931dedbf4a45b2e4648ced19 + uses: actions/cache@fd5de65bc895cf536527842281bea11763fefd77 with: path: ~/.gradle/wrapper key: ${{ runner.os }}-gradle-wrapper-${{ hashFiles(format('{0}{1}', github.workspace, '/gradle/wrapper/gradle-wrapper.properties')) }} - name: Gradle Dependency Cache id: gradle-dependency-cache - uses: actions/cache@a7c34adf76222e77931dedbf4a45b2e4648ced19 + uses: actions/cache@fd5de65bc895cf536527842281bea11763fefd77 with: path: ~/.gradle/caches/modules-2 key: ${{ runner.os }}-gradle-deps-${{ hashFiles(format('{0}{1}', github.workspace, '/gradle.properties')) }} @@ -48,7 +48,7 @@ runs: # 2. Relying on the sha for an exact match so that the prime_cache job is re-used by all dependent jobs in a single workflow run - name: Gradle Build Cache id: gradle-build-cache - uses: actions/cache@a7c34adf76222e77931dedbf4a45b2e4648ced19 + uses: actions/cache@fd5de65bc895cf536527842281bea11763fefd77 with: path: | ~/.gradle/caches/build-cache-1 @@ -59,7 +59,7 @@ runs: ${{ runner.os }}-gradle-build- - name: Rust Cache id: rust-cache - uses: actions/cache@a7c34adf76222e77931dedbf4a45b2e4648ced19 + uses: actions/cache@fd5de65bc895cf536527842281bea11763fefd77 with: path: | sdk-lib/target From b7df1836345dace983708a9cf935186f2e42cce5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 06:36:54 -0400 Subject: [PATCH 08/24] Bump google-github-actions/auth from 0.8.0 to 0.8.1 (#709) Bumps [google-github-actions/auth](https://github.com/google-github-actions/auth) from 0.8.0 to 0.8.1. - [Release notes](https://github.com/google-github-actions/auth/releases) - [Changelog](https://github.com/google-github-actions/auth/blob/main/CHANGELOG.md) - [Commits](https://github.com/google-github-actions/auth/compare/ceee102ec2387dd9e844e01b530ccd4ec87ce955...dac4e13deb3640f22e3ffe758fd3f95e6e89f712) --- updated-dependencies: - dependency-name: google-github-actions/auth dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/pull-request.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 19b56229..c3b6be71 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -236,7 +236,7 @@ jobs: ./gradlew assembleDebug assembleAndroidTest - name: Authenticate to Google Cloud for Firebase Test Lab id: auth_test_lab - uses: google-github-actions/auth@ceee102ec2387dd9e844e01b530ccd4ec87ce955 + uses: google-github-actions/auth@dac4e13deb3640f22e3ffe758fd3f95e6e89f712 with: create_credentials_file: true project_id: ${{ secrets.FIREBASE_TEST_LAB_PROJECT }} @@ -374,7 +374,7 @@ jobs: uses: ./.github/actions/setup - name: Authenticate to Google Cloud for Firebase Test Lab id: auth_test_lab - uses: google-github-actions/auth@ceee102ec2387dd9e844e01b530ccd4ec87ce955 + uses: google-github-actions/auth@dac4e13deb3640f22e3ffe758fd3f95e6e89f712 with: create_credentials_file: true project_id: ${{ secrets.FIREBASE_TEST_LAB_PROJECT }} From 35e38ddb1922d09e091beec20ff9063d4fa0e22c Mon Sep 17 00:00:00 2001 From: Honza Rychnovsky Date: Tue, 6 Sep 2022 12:44:33 +0200 Subject: [PATCH 09/24] [#666] Download sapling files atomically (#707) - Enhanced implementation of SaplingParamTool component. It got sapling files move from legacy folder to the preferred one functionality. It downloads the sapling files atomically now (through the temporary file names). It contains all related constants now. It works with SaplingParamToolProperties, which allows us to easily test the SaplingParamTool functionalities. It also now checks file hashes. Removed unnecessary clear function from the component. Changed valid function. - Moved related constants from ZcashSdk class to SaplingParamTool class - Changed Initializer, WalletTransactionEncoder and RustBackend classes to work with File instead of path String - Minor changes in comments in other classes - Added getSha1Hash() extension function to the FileExt class * Related tests - Two new test fixtures to simplify our tests - Test for getSha1Hash() extension function - Minor changes in existing tests - Created new SaplingParamToolBasicTest, which covers non-integration functionality of SaplingParamTool - Moved integration tests of the SaplingParamTool to the new SaplingParamToolIntegrationTest and added some new * Related manual tests - Created Download sapling files manual test - Created Move sapling files to no_backup manual test - Update existing Move database files to no_bakcup manual test Co-authored-by: Carter Jernigan --- .idea/runConfigurations/detektAll.xml | 6 +- .run/assembleAndroidTest.run.xml | 11 +- .run/ktlint.run.xml | 6 +- CHANGELOG.md | 6 + docs/tests/Download sapling files.md | 32 ++ .../tests/Move database files to no_backup.md | 54 ++-- docs/tests/Move sapling files to no_backup.md | 47 +++ .../android/sdk/db/DatabaseCoordinatorTest.kt | 48 +-- .../cash/z/ecc/android/sdk/ext/FileExtTest.kt | 55 ++++ .../ecc/android/sdk/integration/SanityTest.kt | 9 +- .../ecc/android/sdk/integration/SmokeTest.kt | 4 +- .../sdk/internal/SaplingParamToolBasicTest.kt | 148 +++++++++ .../SaplingParamToolIntegrationTest.kt | 195 ++++++++++++ .../sdk/internal/SaplingParamToolTest.kt | 137 --------- .../z/ecc/android/sdk/jni/BranchIdTest.kt | 21 +- .../cash/z/ecc/fixture/DatabasePathFixture.kt | 3 +- .../z/ecc/fixture/SaplingParamToolFixture.kt | 36 +++ .../z/ecc/fixture/SaplingParamsFixture.kt | 66 ++-- .../cash/z/ecc/android/sdk/Initializer.kt | 23 +- .../cash/z/ecc/android/sdk/SdkSynchronizer.kt | 6 +- .../cash/z/ecc/android/sdk/Synchronizer.kt | 4 +- .../ecc/android/sdk/db/DatabaseCoordinator.kt | 10 +- .../cash/z/ecc/android/sdk/ext/ZcashSdk.kt | 17 -- .../android/sdk/internal/SaplingParamTool.kt | 282 ++++++++++++++---- .../z/ecc/android/sdk/internal/ext/FileExt.kt | 37 +++ .../transaction/WalletTransactionEncoder.kt | 39 +-- .../cash/z/ecc/android/sdk/jni/RustBackend.kt | 25 +- .../ecc/android/sdk/jni/RustBackendWelding.kt | 3 + 28 files changed, 988 insertions(+), 342 deletions(-) create mode 100644 docs/tests/Download sapling files.md create mode 100644 docs/tests/Move sapling files to no_backup.md create mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/ext/FileExtTest.kt create mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolBasicTest.kt create mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolIntegrationTest.kt delete mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolTest.kt create mode 100644 sdk-lib/src/androidTest/java/cash/z/ecc/fixture/SaplingParamToolFixture.kt diff --git a/.idea/runConfigurations/detektAll.xml b/.idea/runConfigurations/detektAll.xml index bc83a0ba..5d5cdac2 100644 --- a/.idea/runConfigurations/detektAll.xml +++ b/.idea/runConfigurations/detektAll.xml @@ -4,12 +4,14 @@ -