Skip to content

Commit

Permalink
[#1127] Refactor CompactBlockProcessor.verifySetup
Browse files Browse the repository at this point in the history
- These changes also move the verifySetup trigger before refreshUtxos at the beginning of the Synchronizer start
- Closes #1127
  • Loading branch information
HonzaR authored Oct 17, 2024
1 parent d133140 commit 912117c
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,11 @@ class SdkSynchronizer private constructor(
private fun CoroutineScope.onReady() {
Twig.debug { "Starting synchronizer…" }

// Verify processor setup as soon as possible
launch(CoroutineExceptionHandler(::onCriticalError)) {
processor.verifySetup()
}

// Triggering UTXOs and transactions fetching at the beginning of the block synchronization right after the
// app starts makes the transparent transactions appear faster.
launch(CoroutineExceptionHandler(::onCriticalError)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import cash.z.ecc.android.sdk.model.WalletBalance
import cash.z.ecc.android.sdk.model.Zatoshi
import co.electriccoin.lightwallet.client.model.BlockHeightUnsafe
import co.electriccoin.lightwallet.client.model.GetAddressUtxosReplyUnsafe
import co.electriccoin.lightwallet.client.model.LightWalletEndpointInfoUnsafe
import co.electriccoin.lightwallet.client.model.RawTransactionUnsafe
import co.electriccoin.lightwallet.client.model.Response
import co.electriccoin.lightwallet.client.model.ShieldedProtocolEnum
Expand Down Expand Up @@ -232,8 +233,6 @@ class CompactBlockProcessor internal constructor(

val (saplingStartIndex, orchardStartIndex) = refreshWalletSummary()

verifySetup()

updateBirthdayHeight()

// Clear any undeleted left over block files from previous sync attempts.
Expand Down Expand Up @@ -873,65 +872,70 @@ class CompactBlockProcessor internal constructor(
return true
}

// TODO [#1127]: Refactor CompactBlockProcessor.verifySetup
// TODO [#1127]: Need to refactor this to be less ugly and more testable
// TODO [#1127]: https://github.com/zcash/zcash-android-wallet-sdk/issues/1127

/**
* Confirm that the wallet data is properly setup for use.
*/
@Suppress("NestedBlockDepth")
private suspend fun verifySetup() {
val error =
if (repository.getAccountCount() == 0) {
CompactBlockProcessorException.NoAccount
} else {
// verify that the server is correct

// How do we handle network connection issues?
internal suspend fun verifySetup() {
if (repository.getAccountCount() == 0) {
reportSetupException(CompactBlockProcessorException.NoAccount)
} else {
// Reach out to the server to obtain the current server info
val serverInfo =
runCatching {
downloader.getServerInfo()
}.onFailure {
Twig.error { "Unable to obtain server info due to: ${it.message}" }
}.getOrElse {
reportSetupException(it as CompactBlockProcessorException)
setState(State.Disconnected)
return
}.let { it as LightWalletEndpointInfoUnsafe }

downloader.getServerInfo()?.let { info ->
val serverBlockHeight =
runCatching { info.blockHeightUnsafe.toBlockHeight() }.getOrNull()
// Validate server block height
val serverBlockHeight =
runCatching {
serverInfo.blockHeightUnsafe.toBlockHeight()
}.onFailure {
Twig.error { "Failed to parse server block height with: ${it.message}" }
}.getOrElse {
reportSetupException(CompactBlockProcessorException.BadBlockHeight(serverInfo.blockHeightUnsafe))
return
}

if (null == serverBlockHeight) {
// Note: we could better signal network connection issue
CompactBlockProcessorException.BadBlockHeight(info.blockHeightUnsafe)
} else {
val clientBranchId =
"%x".format(
Locale.ROOT,
backend.getBranchIdForHeight(serverBlockHeight)
)
val network = backend.network.networkName
val clientBranchId =
"%x".format(
Locale.ROOT,
backend.getBranchIdForHeight(serverBlockHeight)
)
val network = backend.network.networkName

if (!clientBranchId.equals(info.consensusBranchId, true)) {
MismatchedConsensusBranch(
clientBranchId = clientBranchId,
serverBranchId = info.consensusBranchId
)
} else if (!info.matchingNetwork(network)) {
MismatchedNetwork(
clientNetwork = network,
serverNetwork = info.chainName
)
} else {
null
}
}
}
if (!clientBranchId.equals(serverInfo.consensusBranchId, true)) {
reportSetupException(
MismatchedConsensusBranch(
clientBranchId = clientBranchId,
serverBranchId = serverInfo.consensusBranchId
)
)
} else if (!serverInfo.matchingNetwork(network)) {
reportSetupException(
MismatchedNetwork(
clientNetwork = network,
serverNetwork = serverInfo.chainName
)
)
}
}
}

if (error != null) {
Twig.debug { "Validating setup prior to scanning . . . ISSUE FOUND! - ${error.javaClass.simpleName}" }
// give listener a chance to override
if (onSetupErrorListener?.invoke(error) != true) {
throw error
} else {
Twig.debug {
"Warning: An ${error::class.java.simpleName} was encountered while verifying setup but " +
"it was ignored by the onSetupErrorHandler. Ignoring message: ${error.message}"
}
private fun reportSetupException(error: CompactBlockProcessorException) {
Twig.warn { "Validating setup prior to scanning . . . ISSUE FOUND! - ${error.message}" }
// Give listener a chance to override
if (onSetupErrorListener?.invoke(error) != true) {
throw error
} else {
Twig.warn {
"Warning: An was encountered while verifying setup but it was ignored by the onSetupErrorHandler. " +
"Ignoring message: ${error.message}"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,10 @@ sealed class LightWalletException(message: String, cause: Throwable? = null) : S
cause = cause
)

class GetTAddressTransactionsException(code: Int, description: String?, cause: Throwable) : LightWalletException(
class GetServerInfoException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message =
"Failed to get transactions belonging to the given transparent address with code: $code due" +
" to: ${description ?: "-"}",
"Failed to get data about currently connected lightwalletd server with code: $code due to: ${description
?: "-"}",
cause = cause
)

Expand All @@ -304,6 +304,13 @@ sealed class LightWalletException(message: String, cause: Throwable? = null) : S
" to: ${description ?: "-"}",
cause = null
)

class GetTAddressTransactionsException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message =
"Failed to get transactions belonging to the given transparent address with code: $code due" +
" to: ${description ?: "-"}",
cause = cause
)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,20 @@ open class CompactBlockDownloader private constructor(val compactBlockRepository
*/
suspend fun getLastDownloadedHeight() = compactBlockRepository.getLatestHeight()

@Throws(LightWalletException.GetServerInfoException::class)
suspend fun getServerInfo(): LightWalletEndpointInfoUnsafe? =
withContext(IO) {
retryUpToAndThrow(GET_SERVER_INFO_RETRIES) {
when (val response = lightWalletClient.getServerInfo()) {
is Response.Success -> return@withContext response.result
else -> {
is Response.Failure -> {
lightWalletClient.reconnect()
Twig.warn { "WARNING: reconnecting to server in response to failure (retry #${it + 1})" }
throw LightWalletException.GetServerInfoException(
response.code,
response.description,
response.toThrowable()
)
}
}
}
Expand Down

0 comments on commit 912117c

Please sign in to comment.