From 1fae5beab6b3537b7ea31d92f9308d0c7b71afa1 Mon Sep 17 00:00:00 2001 From: Honza Rychnovsky Date: Thu, 25 Aug 2022 14:36:57 +0200 Subject: [PATCH] [#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