zec
/
zcash-android-wallet-sdk
mirror of https://github.com/zcash/zcash-android-wallet-sdk.git

Fix: Improve error handling and surface critical Initialization errors.

This commit is contained in:
Kevin Gorham 2021-04-09 21:19:33 -04:00
parent dffb6f257f
commit 1eb1a6aa8e
No known key found for this signature in database
GPG Key ID: CCA55602DF49FC38
12 changed files with 120 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/main/java/cash/z/ecc/android/sdk/Initializer.kt
View File

@ -16,7 +16,7 @@ import java.io.File
/**
* Simplified Initializer focused on starting from a ViewingKey.
*/
class Initializer constructor(appContext: Context, config: Config) : SdkSynchronizer.SdkInitializer {
class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Throwable?) -> Boolean)? = null, config: Config) : SdkSynchronizer.SdkInitializer {
override val context = appContext.applicationContext
override val rustBackend: RustBackend
override val network: ZcashNetwork
@ -26,6 +26,14 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
val viewingKeys: List<UnifiedViewingKey>
val birthday: WalletBirthday
/**
* A callback to invoke whenever an uncaught error is encountered. By definition, the return
* value of the function is ignored because this error is unrecoverable. The only reason the
* function has a return value is so that all error handlers work with the same signature which
* allows one function to handle all errors in simple apps.
*/
override var onCriticalErrorHandler: ((Throwable?) -> Boolean)? = onCriticalErrorHandler
/**
* True when accounts have been created by this initializer.
*
@ -55,7 +63,8 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
}
}
constructor(appContext: Context, block: (Config) -> Unit) : this(appContext, Config(block))
constructor(appContext: Context, config: Config) : this(appContext, null, config)
constructor(appContext: Context, onCriticalErrorHandler: ((Throwable?) -> Boolean)? = null, block: (Config) -> Unit) : this(appContext, onCriticalErrorHandler, Config(block))
fun erase() = erase(context, network, alias)
@ -92,9 +101,10 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
* Initialize the blocks table with the given birthday, if needed.
*/
private fun maybeInitBlocksTable(birthday: WalletBirthday) {
// TODO: consider converting these to typed exceptions in the welding layer
tryWarn(
"Warning: did not initialize the blocks table. It probably was already initialized.",
unlessContains = "constraint failed"
ifContains = "table is not empty"
) {
rustBackend.initBlocksTable(
birthday.height,
@ -111,9 +121,10 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
* Initialize the accounts table with the given viewing keys, if needed.
*/
private fun maybeInitAccountsTable(vararg viewingKeys: UnifiedViewingKey) {
// TODO: consider converting these to typed exceptions in the welding layer
tryWarn(
"Warning: did not initialize the accounts table. It probably was already initialized.",
unlessContains = "constraint failed"
ifContains = "table is not empty"
) {
rustBackend.initAccountsTable(*viewingKeys)
accountsCreated = true
@ -121,25 +132,27 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
}
}
/**
* Validate that the alias doesn't contain malicious characters by enforcing simple rules which
* permit the alias to be used as part of a file name for the preferences and databases. This
* enables multiple wallets to exist on one device, which is also helpful for sweeping funds.
*
* @param alias the alias to validate.
*
* @throws IllegalArgumentException whenever the alias is not less than 100 characters or
* contains something other than alphanumeric characters. Underscores are allowed but aliases
* must start with a letter.
*/
internal fun validateAlias(alias: String) {
require(
alias.length in 1..99 && alias[0].isLetter() &&
alias.all { it.isLetterOrDigit() || it == '_' }
) {
"ERROR: Invalid alias ($alias). For security, the alias must be shorter than 100 " +
"characters and only contain letters, digits or underscores and start with a letter"
private fun onCriticalError(error: Throwable) {
twig("********")
twig("******** INITIALIZER ERROR: $error")
if (error.cause != null) twig("******** caused by ${error.cause}")
if (error.cause?.cause != null) twig("******** caused by ${error.cause?.cause}")
twig("********")
twig(error)
if (onCriticalErrorHandler == null) {
twig(
"WARNING: a critical error occurred on the Initializer but no callback is " +
"registered to be notified of critical errors! THIS IS PROBABLY A MISTAKE. To " +
"respond to these errors (perhaps to update the UI or alert the user) set " +
"initializer.onCriticalErrorHandler to a non-null value or use the secondary " +
"constructor: Initializer(context, handler) { ... }. Note that the synchronizer " +
"and initializer BOTH have error handlers and since the initializer exists " +
"before the synchronizer, it needs its error handler set separately."
)
}
onCriticalErrorHandler?.invoke(error)
}
class Config private constructor (
@ -188,7 +201,7 @@ class Initializer constructor(appContext: Context, config: Config) : SdkSynchron
* @param defaultToOldestHeight determines how a null birthday height will be
* interpreted. Typically, `false` for new wallets and `true` for restored wallets because
* new wallets want to load quickly but restored wallets want to find all possible
* transactions.
* transactions. Again, this value is only considered when [height] is null.
*
*/
fun setBirthdayHeight(height: Int?, defaultToOldestHeight: Boolean = false): Config =

8
src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt
View File

@ -347,6 +347,13 @@ class SdkSynchronizer internal constructor(
_balances.send(processor.getBalanceInfo())
}
suspend fun isValidAddress(address: String): Boolean {
try {
return !validateAddress(address).isNotValid
} catch (t: Throwable) { }
return false
}
private fun CoroutineScope.onReady() = launch(CoroutineExceptionHandler(::onCriticalError)) {
twig("Synchronizer (${this@SdkSynchronizer}) Ready. Starting processor!")
var lastScanTime = 0L
@ -684,6 +691,7 @@ class SdkSynchronizer internal constructor(
val host: String
val port: Int
val alias: String
val onCriticalErrorHandler: ((Throwable?) -> Boolean)?
}
interface Erasable {

3
src/main/java/cash/z/ecc/android/sdk/block/CompactBlockDownloader.kt
View File

@ -47,7 +47,8 @@ open class CompactBlockDownloader private constructor(val compactBlockStore: Com
}
/**
* Rewind the storage to the given height, usually to handle reorgs.
* Rewind the storage to the given height, usually to handle reorgs. Deletes all blocks above
* the given height.
*
* @param height the height to which the data will rewind.
*/

8
src/main/java/cash/z/ecc/android/sdk/block/CompactBlockProcessor.kt
View File

@ -49,6 +49,7 @@ import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.isActive
import kotlinx.coroutines.withContext
import java.util.Locale
import java.util.concurrent.atomic.AtomicInteger
import kotlin.math.max
import kotlin.math.min
@ -105,6 +106,8 @@ class CompactBlockProcessor(
* Callback for apps to report scan times. As blocks are scanned in batches, this listener is
* invoked at the end of every batch and the second parameter is only true when all batches are
* complete. The first parameter contains useful information on the blocks scanned per second.
*
* The Boolean param (isComplete) is true when this event represents the completion of a scan
*/
var onScanMetricCompleteListener: ((BatchMetrics, Boolean) -> Unit)? = null
@ -524,7 +527,8 @@ class CompactBlockProcessor(
result = rustBackend.scanBlocks(SCAN_BATCH_SIZE)
metrics.endBatch()
val lastScannedHeight = range.start + metrics.cumulativeItems - 1
val percent = "%.0f".format((lastScannedHeight - range.first) / (range.last - range.first).toFloat() * 100.0f)
val percentValue = (lastScannedHeight - range.first) / (range.last - range.first + 1).toFloat() * 100.0f
val percent = "%.0f".format(percentValue.coerceAtMost(100f).coerceAtLeast(0f))
twig("batch scanned ($percent%): $lastScannedHeight/${range.last} | ${metrics.batchTime}ms, ${metrics.batchItems}blks, ${metrics.batchIps.format()}bps")
if (currentInfo.lastScannedHeight != lastScannedHeight) {
scannedNewBlocks = true
@ -917,7 +921,7 @@ class CompactBlockProcessor(
}
private fun Service.LightdInfo.matchingNetwork(network: String): Boolean {
fun String.toId() = toLowerCase().run {
fun String.toId() = toLowerCase(Locale.US).run {
when {
contains("main") -> "mainnet"
contains("test") -> "testnet"

5
src/main/java/cash/z/ecc/android/sdk/block/CompactBlockStore.kt
View File

@ -28,10 +28,7 @@ interface CompactBlockStore {
suspend fun write(result: List<CompactFormats.CompactBlock>)
/**
* Remove every block above and including the given height.
*
* After this operation, the data store will look the same as one that has not yet stored the given block height.
* Meaning, if max height is 100 block and rewindTo(50) is called, then the highest block remaining will be 49.
* Remove every block above the given height.
*
* @param height the target height to which to rewind.
*/

8
src/main/java/cash/z/ecc/android/sdk/exception/Exceptions.kt
View File

@ -142,10 +142,10 @@ sealed class InitializerException(message: String, cause: Throwable? = null) : S
"not to mask this error because the root issue should be addressed."
)
object MissingViewingKeyException : InitializerException(
"Expected a viewingKey for this wallet but failed to find one. This usually means that " +
"wallet setup happened incorrectly. A workaround might be to derive the " +
"viewingKey from the seed or seedPhrase, if they exist, but it is probably " +
"better not to mask this error because the root issue should be addressed."
"Expected a unified viewingKey for this wallet but failed to find one. This usually means" +
" that wallet setup happened incorrectly. A workaround might be to derive the" +
" unified viewingKey from the seed or seedPhrase, if they exist, but it is probably" +
" better not to mask this error because the root issue should be addressed."
)
object DatabasePathException :
InitializerException(

4
src/main/java/cash/z/ecc/android/sdk/ext/ConsensusBranchId.kt
View File

@ -1,5 +1,7 @@
package cash.z.ecc.android.sdk.ext
import java.util.Locale
/**
* Helper class for converting/displaying consensus branch ids. Activation height is intentionally
* omitted since this is not the source of truth for branch information but rather a tool for
@ -22,7 +24,7 @@ enum class ConsensusBranchId(val displayName: String, val id: Long, val hexId: S
fun fromId(id: Long): ConsensusBranchId? = values().firstOrNull { it.id == id }
fun fromHex(hex: String): ConsensusBranchId? = values().firstOrNull { branch ->
hex.toLowerCase().replace("_", "").replaceFirst("0x", "").let { sanitized ->
hex.toLowerCase(Locale.US).replace("_", "").replaceFirst("0x", "").let { sanitized ->
branch.hexId.equals(sanitized, true)
}
}

20
src/main/java/cash/z/ecc/android/sdk/ext/Ext.kt
View File

@ -8,17 +8,31 @@ internal inline fun <R> tryNull(block: () -> R): R? {
}
}
/**
* Execute the given block, converting exceptions into warnings. This can be further controlled by
* filters so that only exceptions matching the given constraints are converted into warnings.
*
* @param ifContains only convert an exception into a warning if it contains the given text
* @param unlessContains convert all exceptions into warnings unless they contain the given text
*/
internal inline fun <R> tryWarn(
message: String,
ifContains: String? = null,
unlessContains: String? = null,
block: () -> R
): R? {
return try {
block()
} catch (t: Throwable) {
if (unlessContains != null &&
(t.message?.toLowerCase()?.contains(unlessContains.toLowerCase()) == true)
) {
val shouldThrowAnyway = (
unlessContains != null &&
(t.message?.toLowerCase()?.contains(unlessContains.toLowerCase()) == true)
) ||
(
ifContains != null &&
(t.message?.toLowerCase()?.contains(ifContains.toLowerCase()) == false)
)
if (shouldThrowAnyway) {
throw t
} else {
twig("$message due to: $t")

6
src/main/java/cash/z/ecc/android/sdk/ext/Metrics.kt
View File

@ -45,7 +45,7 @@ class Sma(val window: Int = 3) {
fun format(places: Int = 0) = "%.${places}f".format(average)
}
class BatchMetrics(val range: IntRange, val batchSize: Int, val onMetricComplete: ((BatchMetrics, Boolean) -> Unit)? = null) {
class BatchMetrics(val range: IntRange, val batchSize: Int, private val onMetricComplete: ((BatchMetrics, Boolean) -> Unit)? = null) {
private var completedBatches = 0
private var rangeStartTime = 0L
private var batchStartTime = 0L
@ -54,8 +54,8 @@ class BatchMetrics(val range: IntRange, val batchSize: Int, val onMetricComplete
private inline fun now() = System.currentTimeMillis()
private inline fun ips(blocks: Int, time: Long) = 1000.0f * blocks / time
val isComplete = completedBatches * batchSize >= rangeSize
val isBatchComplete = batchEndTime > batchStartTime
val isComplete get() = completedBatches * batchSize >= rangeSize
val isBatchComplete get() = batchEndTime > batchStartTime
val cumulativeItems get() = min(completedBatches * batchSize, rangeSize)
val cumulativeTime get() = (if (isComplete) batchEndTime else now()) - rangeStartTime
val batchTime get() = max(batchEndTime - batchStartTime, now() - batchStartTime)

49
src/main/java/cash/z/ecc/android/sdk/ext/Twig.kt
View File

@ -31,15 +31,25 @@ interface Twig {
val trunk get() = Bush.trunk
/**
* Plants the twig, making it the one and only bush. Twigs can be bundled together to create the appearance of
* multiple bushes (i.e `Twig.plant(twigA + twigB + twigC)`) even though there's only ever one bush.
* Convenience function to just turn this thing on. Twigs are silent by default so this is
* most useful to enable developer logging at the right time.
*/
fun enabled(isEnabled: Boolean) {
if (isEnabled) plant(TroubleshootingTwig()) else plant(SilentTwig())
}
/**
* Plants the twig, making it the one and only bush. Twigs can be bundled together to create
* the appearance of multiple bushes (i.e `Twig.plant(twigA + twigB + twigC)`) even though
* there's only ever one bush.
*/
fun plant(rootTwig: Twig) {
Bush.trunk = rootTwig
}
/**
* Generate a leaf on the bush. Leaves show up in every log message as tags until they are clipped.
* Generate a leaf on the bush. Leaves show up in every log message as tags until they are
* clipped.
*/
fun sprout(leaf: Leaf) = Bush.leaves.add(leaf)
@ -56,9 +66,9 @@ interface Twig {
}
/**
* A collection of tiny logs (twigs) consisting of one trunk and maybe some leaves. There can only ever be one trunk.
* Trunks are created by planting a twig. Whenever a leaf sprouts, it will appear as a tag on every log message
* until clipped.
* A collection of tiny logs (twigs) consisting of one trunk and maybe some leaves. There can only
* ever be one trunk. Trunks are created by planting a twig. Whenever a leaf sprouts, it will appear
* as a tag on every log message until clipped.
*
* @see [Twig.plant]
* @see [Twig.sprout]
@ -74,6 +84,13 @@ object Bush {
*/
inline fun twig(message: String) = Bush.trunk.twig(message)
/**
* Makes an exception.
*/
inline fun twig(t: Throwable) = t.stackTraceToString().lines().forEach {
twig(it)
}
/**
* Times a tiny log.
*/
@ -123,7 +140,8 @@ open class TroubleshootingTwig(
open class CompositeTwig(open val twigBundle: MutableList<Twig>) :
Twig {
override operator fun plus(twig: Twig): Twig {
if (twig is CompositeTwig) twigBundle.addAll(twig.twigBundle) else twigBundle.add(twig); return this
if (twig is CompositeTwig) twigBundle.addAll(twig.twigBundle) else twigBundle.add(twig)
return this
}
override fun twig(logMessage: String) {
@ -145,27 +163,28 @@ inline fun <R> Twig.twig(logMessage: String, block: () -> R): R {
}
/**
* A tiny log task. Execute the block of code with some twigging around the outside. For silent twigs, this adds a small
* amount of overhead at the call site but still avoids logging.
* A tiny log task. Execute the block of code with some twigging around the outside. For silent
* twigs, this adds a small amount of overhead at the call site but still avoids logging.
*
* note: being an extension function (i.e. static rather than a member of the Twig interface) allows this function to be
* inlined and simplifies its use with suspend functions
* note: being an extension function (i.e. static rather than a member of the Twig interface) allows
* this function to be inlined and simplifies its use with suspend functions
* (otherwise the function and its "block" param would have to suspend)
*/
inline fun <R> Twig.twigTask(logMessage: String, block: () -> R): R {
twig("$logMessage - started | on thread ${Thread.currentThread().name}")
twig("$logMessage - started | on thread ${Thread.currentThread().name}")
val start = System.nanoTime()
val result = block()
val elapsed = ((System.nanoTime() - start) / 1e5).roundToLong() / 10L
twig("$logMessage - completed | in $elapsed ms" + " on thread ${Thread.currentThread().name}")
twig("$logMessage - completed | in $elapsed ms" + " on thread ${Thread.currentThread().name}")
return result
}
/**
* A tiny log formatter that makes twigs pretty spiffy.
*
* @param stackFrame the stack frame from which we try to derive the class. This can vary depending on how the code is
* called so we expose it for flexibility. Jiggle the handle on this whenever the line numbers appear incorrect.
* @param stackFrame the stack frame from which we try to derive the class. This can vary depending
* on how the code is called so we expose it for flexibility. Jiggle the handle on this whenever the
* line numbers appear incorrect.
*/
inline fun spiffy(stackFrame: Int = 4, tag: String = "@TWIG"): (String) -> String = { logMessage: String ->
val stack = Thread.currentThread().stackTrace[stackFrame]

1
src/main/java/cash/z/ecc/android/sdk/ext/WalletService.kt
View File

@ -87,6 +87,7 @@ suspend inline fun retryWithBackoff(noinline onErrorListener: ((Throwable) -> Bo
sequence /= 2
}
twig("Failed due to $t caused by ${t.cause} backing off and retrying in ${duration}ms...")
twig(t)
delay(duration)
}
}

6
src/main/java/cash/z/ecc/android/sdk/service/LightWalletGrpcService.kt
View File

@ -186,7 +186,11 @@ class LightWalletGrpcService private constructor(
val host: String,
val port: Int,
val usePlaintext: Boolean
)
) {
override fun toString(): String {
return "$host:$port?usePlaintext=$usePlaintext"
}
}
companion object {
/**