[#665] Recover from corrupted sapling-spend.params and sapling-output.params files

* [#665] Recover from corrupted sapling-spend.params and sapling-output.params * Make parameters on exception internal * Make SaplingParameters internal * Fix for changed parameters visibility in tests Co-authored-by: Carter Jernigan <git@carterjernigan.com>
2022-09-27 07:03:36 +02:00 · 2022-09-27 07:03:36 +02:00 · e62ca0de70
parent 2d2e838fed
commit e62ca0de70
4 changed files with 164 additions and 37 deletions
--- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolBasicTest.kt
+++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolBasicTest.kt
@ -90,11 +90,20 @@ class SaplingParamToolBasicTest {
        // we need to use modified array of sapling parameters to pass through the SHA1 hashes validation
        val destDir = SaplingParamTool.initAndGetParamsDestinationDir(
            SaplingParamToolFixture.new(
-                saplingParamsFiles = SaplingParamToolFixture.SAPLING_PARAMS_FILES
-                    .also {
-                        it[0].fileHash = spendFile.getSha1Hash()
-                        it[1].fileHash = outputFile.getSha1Hash()
-                    }
+                saplingParamsFiles = listOf(
+                    SaplingParameters(
+                        SaplingParamToolFixture.PARAMS_DIRECTORY,
+                        SaplingParamTool.SPEND_PARAM_FILE_NAME,
+                        SaplingParamTool.SPEND_PARAM_FILE_MAX_BYTES_SIZE,
+                        spendFile.getSha1Hash()
+                    ),
+                    SaplingParameters(
+                        SaplingParamToolFixture.PARAMS_DIRECTORY,
+                        SaplingParamTool.OUTPUT_PARAM_FILE_NAME,
+                        SaplingParamTool.OUTPUT_PARAM_FILE_MAX_BYTES_SIZE,
+                        outputFile.getSha1Hash()
+                    )
+                )
            )
        )

@ -118,25 +127,34 @@ class SaplingParamToolBasicTest {
    fun ensure_params_exception_thrown_test() = runTest {
        val saplingParamTool = SaplingParamTool(
            SaplingParamToolFixture.new(
-                saplingParamsFiles = SaplingParamToolFixture.SAPLING_PARAMS_FILES
-                    .also {
-                        it[0].fileName = "test_file_0"
-                        it[1].fileName = "test_file_1"
-                    }
+                saplingParamsFiles = listOf(
+                    SaplingParameters(
+                        SaplingParamToolFixture.PARAMS_DIRECTORY,
+                        "test_file_1",
+                        SaplingParamTool.SPEND_PARAM_FILE_MAX_BYTES_SIZE,
+                        SaplingParamTool.SPEND_PARAM_FILE_SHA1_HASH
+                    ),
+                    SaplingParameters(
+                        SaplingParamToolFixture.PARAMS_DIRECTORY,
+                        "test_file_0",
+                        SaplingParamTool.OUTPUT_PARAM_FILE_MAX_BYTES_SIZE,
+                        SaplingParamTool.OUTPUT_PARAM_FILE_SHA1_HASH
+                    )
+                )
            )
        )

        // now we inject params files to the preferred location to pass through the check missing files phase
        SaplingParamsFixture.createFile(
            File(
-                SaplingParamToolFixture.SAPLING_PARAMS_FILES[0].destinationDirectory,
-                SaplingParamToolFixture.SAPLING_PARAMS_FILES[0].fileName
+                saplingParamTool.properties.saplingParams[0].destinationDirectory,
+                saplingParamTool.properties.saplingParams[0].fileName
            )
        )
        SaplingParamsFixture.createFile(
            File(
-                SaplingParamToolFixture.SAPLING_PARAMS_FILES[1].destinationDirectory,
-                SaplingParamToolFixture.SAPLING_PARAMS_FILES[1].fileName
+                saplingParamTool.properties.saplingParams[1].destinationDirectory,
+                saplingParamTool.properties.saplingParams[1].fileName
            )
        )

--- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolIntegrationTest.kt
+++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/internal/SaplingParamToolIntegrationTest.kt
@ -14,6 +14,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import java.io.File
 import kotlin.test.assertContains
+import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
@ -162,7 +163,7 @@ class SaplingParamToolIntegrationTest {
    fun fetch_params_incorrect_hash_test() = runTest {
        val saplingParamTool = SaplingParamTool.new(getAppContext())

-        assertFailsWith<TransactionEncoderException.FetchParamsException> {
+        assertFailsWith<TransactionEncoderException.ValidateParamsException> {
            saplingParamTool.fetchParams(
                SaplingParamsFixture.new(
                    fileName = SaplingParamsFixture.OUTPUT_FILE_NAME,
@ -180,7 +181,7 @@ class SaplingParamToolIntegrationTest {
    fun fetch_params_incorrect_max_file_size_test() = runTest {
        val saplingParamTool = SaplingParamTool.new(getAppContext())

-        assertFailsWith<TransactionEncoderException.FetchParamsException> {
+        assertFailsWith<TransactionEncoderException.ValidateParamsException> {
            saplingParamTool.fetchParams(
                SaplingParamsFixture.new(
                    fileName = SaplingParamsFixture.OUTPUT_FILE_NAME,
@ -192,4 +193,69 @@ class SaplingParamToolIntegrationTest {

        assertFalse(saplingParamTool.validate(SaplingParamsFixture.DESTINATION_DIRECTORY))
    }
+
+    @Test
+    @LargeTest
+    fun fetch_param_manual_recover_test_from_fetch_params_exception() = runTest {
+        val saplingParamTool = SaplingParamTool.new(getAppContext())
+
+        SaplingParamsFixture.DESTINATION_DIRECTORY.delete() // will cause the FetchParamsException
+
+        val exception = assertFailsWith<TransactionEncoderException.FetchParamsException> {
+            saplingParamTool.fetchParams(outputSaplingParams)
+        }
+
+        assertEquals(outputSaplingParams.fileName, exception.parameters.fileName)
+
+        val expectedOutputFile = File(
+            SaplingParamsFixture.DESTINATION_DIRECTORY,
+            SaplingParamsFixture.OUTPUT_FILE_NAME
+        )
+
+        assertFalse(expectedOutputFile.exists())
+
+        // to set up the missing deleted folder
+        SaplingParamTool.initAndGetParamsDestinationDir(saplingParamTool.properties)
+
+        // re-try with parameters returned by the exception
+        saplingParamTool.fetchParams(exception.parameters)
+
+        assertTrue(expectedOutputFile.exists())
+    }
+
+    @Test
+    @LargeTest
+    fun fetch_param_manual_recover_test_from_validate_params_exception() = runTest {
+        val saplingParamTool = SaplingParamTool.new(getAppContext())
+
+        val expectedOutputFile = File(
+            SaplingParamsFixture.DESTINATION_DIRECTORY,
+            SaplingParamsFixture.OUTPUT_FILE_NAME
+        )
+
+        val outputSaplingParams = SaplingParamsFixture.new(
+            SaplingParamsFixture.DESTINATION_DIRECTORY,
+            SaplingParamsFixture.OUTPUT_FILE_NAME,
+            SaplingParamsFixture.OUTPUT_FILE_MAX_SIZE,
+            SaplingParamsFixture.SPEND_FILE_HASH // will cause the ValidateParamsException
+        )
+
+        val exception = assertFailsWith<TransactionEncoderException.ValidateParamsException> {
+            saplingParamTool.fetchParams(outputSaplingParams)
+        }
+
+        assertFalse(expectedOutputFile.exists())
+
+        val fixedOutputSaplingParams = SaplingParamsFixture.new(
+            destinationDirectoryPath = exception.parameters.destinationDirectory,
+            fileName = exception.parameters.fileName,
+            fileMaxSize = exception.parameters.fileMaxSizeBytes,
+            fileHash = SaplingParamsFixture.OUTPUT_FILE_HASH // fixed file hash
+        )
+
+        // re-try with fixed parameters
+        saplingParamTool.fetchParams(fixedOutputSaplingParams)
+
+        assertTrue(expectedOutputFile.exists())
+    }
 }
--- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/exception/Exceptions.kt
+++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/exception/Exceptions.kt
@ -1,5 +1,6 @@
 package cash.z.ecc.android.sdk.exception

+import cash.z.ecc.android.sdk.internal.SaplingParameters
 import cash.z.ecc.android.sdk.internal.model.Checkpoint
 import cash.z.ecc.android.sdk.model.BlockHeight
 import cash.z.ecc.android.sdk.model.ZcashNetwork
@ -266,8 +267,18 @@ sealed class LightWalletException(message: String, cause: Throwable? = null) : S
 /**
 * Potentially user-facing exceptions thrown while encoding transactions.
 */
-sealed class TransactionEncoderException(message: String, cause: Throwable? = null) : SdkException(message, cause) {
-    class FetchParamsException(message: String) : TransactionEncoderException("Failed to fetch params due to: $message")
+sealed class TransactionEncoderException(
+    message: String,
+    cause: Throwable? = null
+) : SdkException(message, cause) {
+    class FetchParamsException internal constructor(
+        internal val parameters: SaplingParameters,
+        message: String
+    ) : TransactionEncoderException("Failed to fetch params: $parameters, due to: $message")
+    class ValidateParamsException internal constructor(
+        internal val parameters: SaplingParameters,
+        message: String
+    ) : TransactionEncoderException("Failed to validate fetched params: $parameters, due to:$message")
    object MissingParamsException : TransactionEncoderException(
        "Cannot send funds due to missing spend or output params and attempting to download them failed."
    )
--- 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
@ -10,6 +10,7 @@ import cash.z.ecc.android.sdk.internal.ext.getSha1Hash
 import cash.z.ecc.android.sdk.internal.ext.mkdirsSuspend
 import cash.z.ecc.android.sdk.internal.ext.renameToSuspend
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
@ -17,6 +18,7 @@ import java.io.File
 import java.io.IOException
 import java.net.URL
 import java.nio.channels.Channels
+import kotlin.time.Duration.Companion.milliseconds

 internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
    companion object {
@ -178,9 +180,15 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
     * @param destinationDir the directory where the params should be stored.
     *
     * @throws TransactionEncoderException.MissingParamsException in case of failure while checking sapling params
+     * @throws TransactionEncoderException.FetchParamsException
+     * @throws TransactionEncoderException.ValidateParamsException
     * files
     */
-    @Throws(TransactionEncoderException.MissingParamsException::class)
+    @Throws(
+        TransactionEncoderException.ValidateParamsException::class,
+        TransactionEncoderException.FetchParamsException::class,
+        TransactionEncoderException.MissingParamsException::class
+    )
    internal suspend fun ensureParams(destinationDir: File) {
        properties.saplingParams.filter {
            !File(it.destinationDirectory, it.fileName).existsSuspend()
@ -189,8 +197,22 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
                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
+                twig(
+                    "Failed to fetch param file ${it.fileName} due to: $e. The second attempt is starting with a " +
+                        "little delay."
+                )
+                // Re-run the fetch with a little delay, if it failed previously (as it can be caused by network
+                // conditions). We do it only once, the next failure is delivered to the caller of this method.
+                delay(200.milliseconds)
+                fetchParams(it)
+            } catch (e: TransactionEncoderException.ValidateParamsException) {
+                twig(
+                    "Failed to validate fetched param file ${it.fileName} due to: $e. The second attempt is starting" +
+                        " now."
+                )
+                // Re-run the fetch for invalid param file immediately, if it failed previously. We do it again only
+                // once, the next failure is delivered to the caller of this method.
+                fetchParams(it)
            }
        }

@ -207,8 +229,12 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
     * @param paramsToFetch parameters wrapper class, which holds information about it
     *
     * @throws TransactionEncoderException.FetchParamsException if any error while downloading the params file occurs
+     * @throws TransactionEncoderException.ValidateParamsException if a failure in validation of fetched file occurs
     */
-    @Throws(TransactionEncoderException.FetchParamsException::class)
+    @Throws(
+        TransactionEncoderException.ValidateParamsException::class,
+        TransactionEncoderException.FetchParamsException::class
+    )
    internal suspend fun fetchParams(paramsToFetch: SaplingParameters) {
        val url = URL("$CLOUD_PARAM_DIR_URL/${paramsToFetch.fileName}")
        val temporaryFile = File(
@ -239,19 +265,25 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
                // 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)
-                }
+                finalizeAndReportError(
+                    temporaryFile,
+                    exception = TransactionEncoderException.FetchParamsException(
+                        paramsToFetch,
+                        "Error while fetching ${paramsToFetch.fileName}, caused by $exception."
+                    )
+                )
            }.onSuccess {
                twig(
                    "Fetch and write of the temporary ${temporaryFile.name} succeeded. Validating and moving it to " +
-                        "the final destination"
+                        "the final destination."
                )
                if (!isFileHashValid(temporaryFile, paramsToFetch.fileHash)) {
                    finalizeAndReportError(
                        temporaryFile,
-                        message = "Failed while validating fetched params file: ${paramsToFetch.fileName}"
+                        exception = TransactionEncoderException.ValidateParamsException(
+                            paramsToFetch,
+                            "Failed while validating fetched param file: ${paramsToFetch.fileName}."
+                        )
                    )
                }
                val resultFile = File(paramsToFetch.destinationDirectory, paramsToFetch.fileName)
@ -259,24 +291,24 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
                    finalizeAndReportError(
                        temporaryFile,
                        resultFile,
-                        message = "Failed while renaming result params file: ${paramsToFetch.fileName}"
+                        exception = TransactionEncoderException.ValidateParamsException(
+                            paramsToFetch,
+                            "Failed while renaming result param file: ${paramsToFetch.fileName}."
+                        )
                    )
                }
-
-                // TODO [#665]: https://github.com/zcash/zcash-android-wallet-sdk/issues/665
-                // TODO [#665]: Recover from corrupted sapling-spend.params and sapling-output.params
            }
        }
    }

    @Throws(TransactionEncoderException.FetchParamsException::class)
-    private suspend fun finalizeAndReportError(vararg files: File, message: String) {
+    private suspend fun finalizeAndReportError(vararg files: File, exception: TransactionEncoderException) {
        files.forEach {
            it.deleteSuspend()
        }
-        message.also {
+        exception.also {
            twig(it)
-            throw TransactionEncoderException.FetchParamsException(it)
+            throw it
        }
    }

@ -297,9 +329,9 @@ internal class SaplingParamTool(val properties: SaplingParamToolProperties) {
 */
 internal data class SaplingParameters(
    val destinationDirectory: File,
-    var fileName: String,
+    val fileName: String,
    val fileMaxSizeBytes: Long,
-    var fileHash: String
+    val fileHash: String
 )

 /**