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

[#1239] Post-v2.0.0-rc1 cleanup 

* Add JniScanProgressTest

* Remove unnecessary testnet workaround

As this was fixed with the last rust changes.

* Simplify batchCount calculating

* Fix ratio typo

* Docummentation comments changes
This commit is contained in:
Honza Rychnovský 2023-09-18 16:51:26 +02:00 committed by GitHub
parent 62fc5d7cfa
commit 9869fd4300
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 86 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
backend-lib/src/main/rust/lib.rs
View File

@ -1149,6 +1149,9 @@ pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_internal_jni_RustBackend_ge
/// Returns a `JniScanProgress` object, provided that numerator is nonnegative, denominator
/// is positive, and the represented ratio is in the range 0.0 to 1.0 inclusive.
///
/// If these conditions are not met, this fails and leaves an `IllegalArgumentException`
/// pending.
fn encode_scan_progress(env: &JNIEnv<'_>, progress: Ratio<u64>) -> Result<jobject, failure::Error> {
let output = env.new_object(
"cash/z/ecc/android/sdk/internal/model/JniScanProgress",

3
backend-lib/src/main/rust/utils.rs
View File

@ -22,7 +22,8 @@ pub(crate) fn rust_bytes_to_java(
env: &JNIEnv<'_>,
data: &[u8],
) -> Result<jbyteArray, failure::Error> {
// SAFETY: jbyte (i8) has the same size and alignment as u8.
// SAFETY: jbyte (i8) has the same size and alignment as u8, and a well-defined
// twos-complement representation with no "trap representations".
let buf = unsafe { slice::from_raw_parts(data.as_ptr().cast(), data.len()) };
let jret = env.new_byte_array(data.len() as jsize)?;
env.set_byte_array_region(jret, 0, buf)?;

46
backend-lib/src/test/java/cash/z/ecc/android/sdk/internal/model/JniScanProgressTest.kt Normal file
View File

@ -0,0 +1,46 @@
package cash.z.ecc.android.sdk.internal.model
import kotlin.test.Test
import kotlin.test.assertFailsWith
import kotlin.test.assertIs
class JniScanProgressTest {
@Test
fun both_attribute_within_constraints() {
val instance = JniScanProgress(
numerator = 1L,
denominator = 100L
)
assertIs<JniScanProgress>(instance)
}
@Test
fun numerator_attribute_not_in_constraints() {
assertFailsWith(IllegalArgumentException::class) {
JniScanProgress(
numerator = -1L,
denominator = 100L
)
}
}
@Test
fun denominator_attribute_not_in_constraints() {
assertFailsWith(IllegalArgumentException::class) {
JniScanProgress(
numerator = 1L,
denominator = 0L
)
}
}
@Test
fun ratio_not_in_constraints() {
assertFailsWith(IllegalArgumentException::class) {
JniScanProgress(
numerator = 100L,
denominator = 1L
)
}
}
}

71
sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/processor/CompactBlockProcessor.kt
View File

@ -203,7 +203,8 @@ class CompactBlockProcessor internal constructor(
updateBirthdayHeight()
// Clear any undeleted left over block files from previous sync attempts
// Clear any undeleted left over block files from previous sync attempts.
// lastKnownHeight is only used for error reporting.
deleteAllBlockFiles(
downloader = downloader,
lastKnownHeight = when (val result = getMaxScannedHeight(backend)) {
@ -240,7 +241,7 @@ class CompactBlockProcessor internal constructor(
)
) {
PutSaplingSubtreeRootsResult.Success -> {
// Lets continue with the next step
// Let's continue with the next step
}
is PutSaplingSubtreeRootsResult.Failure -> {
BlockProcessingResult.SyncFailure(result.failedAtHeight, result.exception)
@ -255,22 +256,10 @@ class CompactBlockProcessor internal constructor(
firstUnenhancedHeight = _processorInfo.value.firstUnenhancedHeight
)
}
GetSubtreeRootsResult.Linear -> {
// This is caused by an empty response result. Although the spend-before-sync
// synchronization algorithm is not supported, we can get the entire block range as we
// previously did for the linear sync type.
processNewBlocksInSbSOrder(
backend = backend,
downloader = downloader,
repository = repository,
network = network,
lastValidHeight = lowerBoundHeight,
firstUnenhancedHeight = _processorInfo.value.firstUnenhancedHeight
)
}
is GetSubtreeRootsResult.OtherFailure -> {
// The server possibly replied with some unsupported error. We still approach
// spend-before-sync synchronization.
is GetSubtreeRootsResult.OtherFailure, GetSubtreeRootsResult.Linear -> {
// This is caused by an empty response result or another unsupported error.
// Although the spend-before-sync synchronization algorithm is not supported, we can get
// the entire block range as we previously did for the linear sync type.
processNewBlocksInSbSOrder(
backend = backend,
downloader = downloader,
@ -625,7 +614,7 @@ class CompactBlockProcessor internal constructor(
lastValidHeight = lastValidHeight
)
) {
is UpdateChainTipResult.Success -> { /* Lets continue to the next step */ }
is UpdateChainTipResult.Success -> { /* Let's continue to the next step */ }
is UpdateChainTipResult.Failure -> {
return SbSPreparationResult.ProcessFailure(
result.failedAtHeight,
@ -900,8 +889,7 @@ class CompactBlockProcessor internal constructor(
}
} else {
Twig.debug {
"Warning: gave up on fetching UTXOs for this session. It seems to unavailable on " +
"lightwalletd."
"Warning: gave up on fetching UTXOs for this session. It is unavailable on lightwalletd."
}
}
@ -978,9 +966,9 @@ class CompactBlockProcessor internal constructor(
/**
* Default size of batches of blocks to request from the compact block service. Then it's also used as a default
* size of batches of blocks to scan via librustzcash. For scanning action applies this - the smaller this
* number the more granular information can be provided about scan state. Unfortunately, it may also lead to
* a lot of overhead during scanning.
* size of batches of blocks to scan via librustzcash. The smaller the batch size for scanning, the more
* granular information can be provided about scan state. Unfortunately, small batches may also lead to a lot
* of overhead during scanning.
*/
internal const val SYNC_BATCH_SIZE = 100
@ -996,7 +984,7 @@ class CompactBlockProcessor internal constructor(
internal const val REWIND_DISTANCE = 10
/**
* Limit millis value for restarting currently running block synchronization.
* Timeout in milliseconds for restarting the currently running block synchronization.
*/
internal val SYNCHRONIZATION_RESTART_TIMEOUT = 10.minutes.inWholeMilliseconds
@ -1049,8 +1037,8 @@ class CompactBlockProcessor internal constructor(
}
/**
* This operation downloads note commitment tree data from the lightwalletd server to decide if we communicate
* with linear or spend-before-sync node
* This operation downloads note commitment tree data from the lightwalletd server, to decide whether or not
* the lightwalletd and its backing node support spend-before-sync.
*
* @return GetSubtreeRootsResult as a wrapper for the lightwalletd response result
*/
@ -1066,17 +1054,13 @@ class CompactBlockProcessor internal constructor(
retryUpToAndContinue(GET_SUBTREE_ROOTS_RETRIES) {
downloader.getSubtreeRoots(
startIndex = 0,
maxEntries = if (network.isTestnet()) {
65536
} else {
0
},
maxEntries = 0,
shieldedProtocol = ShieldedProtocolEnum.SAPLING
).onEach { response ->
when (response) {
is Response.Success -> {
Twig.verbose {
"SubtreeRoot got successfully: it's completingHeight: ${response.result
"SubtreeRoot fetched successfully: its completingHeight is: ${response.result
.completingBlockHeight}"
}
}
@ -1372,8 +1356,8 @@ class CompactBlockProcessor internal constructor(
// Increment and compare the range for triggering the enhancing
enhancingRange = enhancingRange.start..continuousResult.batch.range.endInclusive
// Enhancing is run in case of the range is on or over its limit, or in case of any failure
// state comes from the previous stages, or if the end of the sync range is reached
// Enhancing is run when the range is on or over its limit, or there is any failure
// from previous stages, or if the end of the sync range is reached.
if (enhancingRange.length() >= ENHANCE_BATCH_SIZE ||
resultState != SyncingResult.AllSuccess ||
continuousResult.batch.order == batches.size.toLong()
@ -1435,10 +1419,7 @@ class CompactBlockProcessor internal constructor(
syncRanges.forEach { range ->
val missingBlockCount = range.endInclusive.value - range.start.value + 1
val batchCount = (
missingBlockCount / SYNC_BATCH_SIZE +
(if (missingBlockCount.rem(SYNC_BATCH_SIZE) == 0L) 0 else 1)
)
val batchCount = (missingBlockCount + SYNC_BATCH_SIZE - 1) / SYNC_BATCH_SIZE
allMissingBlocksCount += missingBlockCount
allRangesBatchCount += batchCount
}
@ -1457,7 +1438,7 @@ class CompactBlockProcessor internal constructor(
* @param syncRange Current range to be processed
* @param network The network we are operating on
*
* @return List of [BlockBatch] to for synchronization
* @return List of [BlockBatch] to prepare for synchronization
*/
private fun getBatchedBlockList(
syncRange: ClosedRange<BlockHeight>,
@ -1506,7 +1487,6 @@ class CompactBlockProcessor internal constructor(
downloadException!!
}
) { failedAttempts ->
@Suppress("MagicNumber")
if (failedAttempts == 0) {
Twig.verbose { "Starting to download batch $batch" }
} else {
@ -1787,7 +1767,8 @@ class CompactBlockProcessor internal constructor(
}
/**
* Emit an instance of processorInfo, corresponding to the provided data.
* Sets new values of ProcessorInfo and network height corresponding to the provided data for this
* [CompactBlockProcessor].
*
* @param networkBlockHeight the latest block available to lightwalletd that may or may not be
* downloaded by this wallet yet.
@ -1812,7 +1793,7 @@ class CompactBlockProcessor internal constructor(
}
/**
* Emit an instance of progress.
* Sets the progress for this [CompactBlockProcessor].
*
* @param progress the block syncing progress of type [PercentDecimal] in the range of [0, 1]
*/
@ -1821,7 +1802,7 @@ class CompactBlockProcessor internal constructor(
}
/**
* Transmits the given state for this processor.
* Sets the state of this [CompactBlockProcessor].
*/
private suspend fun setState(newState: State) {
_state.value = newState
@ -1854,7 +1835,7 @@ class CompactBlockProcessor internal constructor(
}
/**
* Rewind back at least two weeks worth of blocks.
* Rewind back two weeks worth of blocks. The final amount of rewinded blocks depends on [rewindToNearestHeight].
*/
suspend fun quickRewind(): Boolean {
val height = when (val result = getMaxScannedHeight(backend)) {

2
sdk-lib/src/main/java/cash/z/ecc/android/sdk/block/processor/model/GetScanProgressResult.kt
View File

@ -8,7 +8,7 @@ import cash.z.ecc.android.sdk.model.PercentDecimal
*/
internal sealed class GetScanProgressResult {
data class Success(val scanProgress: ScanProgress) : GetScanProgressResult() {
fun toPercentDecimal() = PercentDecimal(scanProgress.getSafeRation())
fun toPercentDecimal() = PercentDecimal(scanProgress.getSafeRatio())
}
data object None : GetScanProgressResult()

4
sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/ext/WalletService.kt
View File

@ -46,8 +46,8 @@ suspend inline fun retryUpToAndThrow(
* retries succeed, then leave the block execution unfinished and continue.
*
* @param retries the number of times to retry the block after the first attempt fails.
* @param exceptionWrapper a function that can wrap the final failure to add more useful information
* * or context. Default behavior is to just return the final exception.
* @param exceptionWrapper a function that can wrap the final failure to add more useful information or context.
* Default behavior is to just return the final exception.
* @param initialDelayMillis the initial amount of time to wait before the first retry.
* @param block the code to execute, which will be wrapped in a try/catch and retried whenever an
* exception is thrown up to [retries] attempts.

4
sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/model/ScanProgress.kt
View File

@ -4,12 +4,12 @@ internal data class ScanProgress(
private val numerator: Long,
private val denominator: Long
) {
override fun toString() = "ScanProgress($numerator/$denominator) -> ${getSafeRation()}"
override fun toString() = "ScanProgress($numerator/$denominator) -> ${getSafeRatio()}"
/**
* Returns progress ratio in [0, 1] range. Any out-of-range value is treated as 0.
*/
fun getSafeRation() = numerator.toFloat().div(denominator).let { ration ->
fun getSafeRatio() = numerator.toFloat().div(denominator).let { ration ->
if (ration < 0f || ration > 1f) {
0f
} else {

4
sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/BlockHeight.kt
View File

@ -43,7 +43,7 @@ data class BlockHeight internal constructor(val value: Long) : Comparable<BlockH
operator fun minus(other: Int): BlockHeight {
require(other >= 0) {
"Cannot subtract negative value $other to BlockHeight"
"Cannot subtract negative value $other from BlockHeight"
}
return BlockHeight(value - other.toLong())
@ -51,7 +51,7 @@ data class BlockHeight internal constructor(val value: Long) : Comparable<BlockH
operator fun minus(other: Long): BlockHeight {
require(other >= 0) {
"Cannot subtract negative value $other to BlockHeight"
"Cannot subtract negative value $other from BlockHeight"
}
return BlockHeight(value - other)

4
sdk-lib/src/test/java/cash/z/ecc/android/sdk/internal/model/ScanProgressTest.kt
View File

@ -9,7 +9,7 @@ class ScanProgressTest {
fun get_valid_ratio_test() {
val scanProgress = ScanProgressFixture.new()
assertEquals(
scanProgress.getSafeRation(),
scanProgress.getSafeRatio(),
ScanProgressFixture.DEFAULT_NUMERATOR.toFloat().div(ScanProgressFixture.DEFAULT_DENOMINATOR)
)
}
@ -19,6 +19,6 @@ class ScanProgressTest {
val scanProgress = ScanProgressFixture.new(
denominator = 0
)
assertEquals(0f, scanProgress.getSafeRation())
assertEquals(0f, scanProgress.getSafeRatio())
}
}