Skip to content

Commit

Permalink
[#1500] Transaction Resubmission
Browse files Browse the repository at this point in the history
* [#1500] Trx resubmission

- The wallet now periodically resubmits unmined sent transactions to lightwalletd that are still within their expiry window
- The Compact block processor's state machine has been extended to check this state periodically at the beginning of every new sync cycle and after processing every block batch, which is currently 1000 blocks
- This also brings changes in Exception definition. TransactionSubmitException now belongs under LightWalletException.
- These changes thus partially address #1484 as well
- Closes #1500
- Changelog update

* Bump library version

- So we can adopt the snapshot version for testing within wallets

* Add constant to query
  • Loading branch information
HonzaR authored Sep 9, 2024
1 parent 6b2bb61 commit a639394
Show file tree
Hide file tree
Showing 10 changed files with 170 additions and 36 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ and this library adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- `Double?.convertUsdToZec` has been added as we are moving away from `BigDecimal` in favor of primitive types
- `Locale.getDefault()` has been added
- Transaction resubmission feature has been added to the CompactBlockProcessor's regular actions. This new action
periodically checks unmined sent transactions that are still within their expiry window and resubmits them if
there are any.

## [2.2.2] - 2024-09-03

Expand Down
20 changes: 10 additions & 10 deletions sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import cash.z.ecc.android.sdk.block.processor.CompactBlockProcessor.State.Synced
import cash.z.ecc.android.sdk.block.processor.CompactBlockProcessor.State.Syncing
import cash.z.ecc.android.sdk.exception.CompactBlockProcessorException
import cash.z.ecc.android.sdk.exception.InitializeException
import cash.z.ecc.android.sdk.exception.LightWalletException
import cash.z.ecc.android.sdk.exception.TransactionEncoderException
import cash.z.ecc.android.sdk.exception.TransactionSubmitException
import cash.z.ecc.android.sdk.ext.ConsensusBranchId
import cash.z.ecc.android.sdk.ext.ZcashSdk
import cash.z.ecc.android.sdk.internal.FastestServerFetcher
Expand Down Expand Up @@ -722,7 +722,7 @@ class SdkSynchronizer private constructor(
"createProposedTransactions(proposeTransfer(usk.account, toAddress, amount, memo), usk)"
)
)
@Throws(TransactionEncoderException::class, TransactionSubmitException::class)
@Throws(TransactionEncoderException::class, LightWalletException.TransactionSubmitException::class)
override suspend fun sendToAddress(
usk: UnifiedSpendingKey,
amount: Zatoshi,
Expand All @@ -742,9 +742,8 @@ class SdkSynchronizer private constructor(
is TransactionSubmitResult.Success -> {
return storage.findMatchingTransactionId(encodedTx.txId.byteArray)!!
}

else -> {
throw TransactionSubmitException()
throw LightWalletException.TransactionSubmitException()
}
}
}
Expand All @@ -756,7 +755,7 @@ class SdkSynchronizer private constructor(
"proposeShielding(usk.account, shieldingThreshold, memo)?.let { createProposedTransactions(it, usk) }"
)
)
@Throws(TransactionEncoderException::class, TransactionSubmitException::class)
@Throws(TransactionEncoderException::class, LightWalletException.TransactionSubmitException::class)
override suspend fun shieldFunds(
usk: UnifiedSpendingKey,
memo: String
Expand All @@ -778,9 +777,8 @@ class SdkSynchronizer private constructor(
is TransactionSubmitResult.Success -> {
return storage.findMatchingTransactionId(encodedTx.txId.byteArray)!!
}

else -> {
throw TransactionSubmitException()
throw LightWalletException.TransactionSubmitException()
}
}
}
Expand Down Expand Up @@ -1046,13 +1044,15 @@ internal object DefaultSynchronizerFactory {
backend: TypesafeBackend,
downloader: CompactBlockDownloader,
repository: DerivedDataRepository,
birthdayHeight: BlockHeight
birthdayHeight: BlockHeight,
txManager: OutboundTransactionManager
): CompactBlockProcessor =
CompactBlockProcessor(
backend = backend,
downloader = downloader,
minimumHeight = birthdayHeight,
repository = repository,
backend = backend,
minimumHeight = birthdayHeight
txManager = txManager,
)
}

Expand Down
3 changes: 2 additions & 1 deletion sdk-lib/src/main/java/cash/z/ecc/android/sdk/Synchronizer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -686,9 +686,10 @@ interface Synchronizer {
val processor =
DefaultSynchronizerFactory.defaultProcessor(
backend = backend,
birthdayHeight = birthday ?: zcashNetwork.saplingActivationHeight,
downloader = downloader,
repository = repository,
birthdayHeight = birthday ?: zcashNetwork.saplingActivationHeight
txManager = txManager,
)

return SdkSynchronizer.new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import cash.z.ecc.android.sdk.exception.CompactBlockProcessorException.Mismatche
import cash.z.ecc.android.sdk.exception.CompactBlockProcessorException.MismatchedNetwork
import cash.z.ecc.android.sdk.exception.InitializeException
import cash.z.ecc.android.sdk.exception.LightWalletException
import cash.z.ecc.android.sdk.exception.TransactionEncoderException
import cash.z.ecc.android.sdk.ext.ZcashSdk
import cash.z.ecc.android.sdk.ext.ZcashSdk.MAX_BACKOFF_INTERVAL
import cash.z.ecc.android.sdk.ext.ZcashSdk.POLL_INTERVAL
Expand Down Expand Up @@ -52,10 +53,12 @@ import cash.z.ecc.android.sdk.internal.model.ext.from
import cash.z.ecc.android.sdk.internal.model.ext.toBlockHeight
import cash.z.ecc.android.sdk.internal.model.ext.toTransactionStatus
import cash.z.ecc.android.sdk.internal.repository.DerivedDataRepository
import cash.z.ecc.android.sdk.internal.transaction.OutboundTransactionManager
import cash.z.ecc.android.sdk.model.Account
import cash.z.ecc.android.sdk.model.BlockHeight
import cash.z.ecc.android.sdk.model.PercentDecimal
import cash.z.ecc.android.sdk.model.RawTransaction
import cash.z.ecc.android.sdk.model.TransactionSubmitResult
import cash.z.ecc.android.sdk.model.WalletBalance
import cash.z.ecc.android.sdk.model.Zatoshi
import cash.z.ecc.android.sdk.model.ZcashNetwork
Expand Down Expand Up @@ -108,10 +111,11 @@ import kotlin.time.toDuration
@OpenForTesting
@Suppress("TooManyFunctions", "LargeClass")
class CompactBlockProcessor internal constructor(
private val backend: TypesafeBackend,
val downloader: CompactBlockDownloader,
minimumHeight: BlockHeight,
private val repository: DerivedDataRepository,
private val backend: TypesafeBackend,
minimumHeight: BlockHeight
private val txManager: OutboundTransactionManager
) {
/**
* Callback for any non-trivial errors that occur while processing compact blocks.
Expand Down Expand Up @@ -457,6 +461,10 @@ class CompactBlockProcessor internal constructor(
network = network,
lastValidHeight = lastValidHeight
)

// Running the unsubmitted transactions check action at the beginning of every sync loop
resubmitUnminedTransactions(networkHeight.value)

when (preparationResult) {
is SbSPreparationResult.ProcessFailure -> {
return preparationResult.toBlockProcessingResult()
Expand Down Expand Up @@ -493,6 +501,9 @@ class CompactBlockProcessor internal constructor(
// Update sync progress and wallet balance
refreshWalletSummary()

// Running the unsubmitted transactions check action at the end of processing every block batch
resubmitUnminedTransactions(networkHeight.value)

when (batchSyncProgress.resultState) {
SyncingResult.UpdateBirthday -> {
updateBirthdayHeight()
Expand Down Expand Up @@ -599,6 +610,9 @@ class CompactBlockProcessor internal constructor(
// Update sync progress and wallet balances
refreshWalletSummary()

// Running the unsubmitted transactions check action at the end of processing every block batch
resubmitUnminedTransactions(networkHeight.value)

when (batchSyncProgress.resultState) {
SyncingResult.UpdateBirthday -> {
updateBirthdayHeight()
Expand Down Expand Up @@ -1022,6 +1036,11 @@ class CompactBlockProcessor internal constructor(
*/
internal const val TRANSACTION_FETCH_RETRIES = 1

/**
* Transaction resubmit retry attempts
*/
internal const val TRANSACTION_RESUBMIT_RETRIES = 1

/**
* UTXOs fetching default attempts at retrying.
*/
Expand Down Expand Up @@ -2199,16 +2218,18 @@ class CompactBlockProcessor internal constructor(
* or repository is empty
*/
@VisibleForTesting
@Suppress("MaxLineLength")
internal suspend fun getFirstUnenhancedHeight(repository: DerivedDataRepository) = repository.firstUnenhancedHeight()
internal suspend fun getFirstUnenhancedHeight(repository: DerivedDataRepository): BlockHeight? {
return repository.firstUnenhancedHeight()
}

/**
* Get the height of the last block that was downloaded by this processor.
*
* @return the last downloaded height reported by the downloader.
*/
@Suppress("MaxLineLength")
internal suspend fun getLastDownloadedHeight(downloader: CompactBlockDownloader) = downloader.getLastDownloadedHeight()
internal suspend fun getLastDownloadedHeight(downloader: CompactBlockDownloader): BlockHeight? {
return downloader.getLastDownloadedHeight()
}

/**
* Get the current unified address for the given wallet account.
Expand Down Expand Up @@ -2483,6 +2504,56 @@ class CompactBlockProcessor internal constructor(
} ?: lowerBoundHeight
}

/**
* This function resubmits the unmined sent transactions that are still within the expiry window. It can produce
* [TransactionEncoderException.TransactionNotFoundException] in case the transaction in not found in the database,
* but networking issues are not reported, it is retried in the next sync cycle instead.
*
* @param blockHeight The block height to which transactions should be compared (usually the current chain tip)
*
* @throws TransactionEncoderException.TransactionNotFoundException in case the encoded transaction is not found
*/
@Throws(TransactionEncoderException.TransactionNotFoundException::class)
private suspend fun resubmitUnminedTransactions(blockHeight: BlockHeight?) {
// Run the check only in case we have already obtained the current chain tip
if (blockHeight == null) {
return
}
val list = repository.findUnminedTransactionsWithinExpiry(blockHeight)

Twig.debug { "Trx resubmission: ${list.size}, ${list.joinToString(separator = ", ") { it.txIdString() }}" }

if (list.isNotEmpty()) {
list.forEach {
val trxForResubmission =
repository.findEncodedTransactionByTxId(it.rawId)
?: throw TransactionEncoderException.TransactionNotFoundException(it.rawId)

Twig.debug { "Trx resubmission: Found: ${trxForResubmission.txIdString()}" }

retryUpToAndContinue(TRANSACTION_RESUBMIT_RETRIES) {
when (val response = txManager.submit(trxForResubmission)) {
is TransactionSubmitResult.Success -> {
Twig.info { "Trx resubmission success: ${response.txIdString()}" }
}
is TransactionSubmitResult.Failure -> {
Twig.error { "Trx resubmission failure: ${response.description}" }
throw LightWalletException.TransactionSubmitException(
response.code,
response.description,
)
}
is TransactionSubmitResult.NotAttempted -> {
Twig.warn { "Trx resubmission not attempted: ${response.txIdString()}" }
}
}
}
}
} else {
Twig.debug { "Trx resubmission: No trx for resubmission found" }
}
}

suspend fun getUtxoCacheBalance(address: String): Zatoshi = backend.getDownloadedUtxoBalance(address)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,30 +271,38 @@ sealed class InitializeException(message: String, cause: Throwable? = null) : Sd
* Exceptions thrown while interacting with lightwalletd.
*/
sealed class LightWalletException(message: String, cause: Throwable? = null) : SdkException(message, cause) {
class DownloadBlockException(code: Int, description: String?, cause: Throwable) : SdkException(
"Failed to download block with code: $code due to: ${description ?: "-"}",
cause
class DownloadBlockException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message = "Failed to download block with code: $code due to: ${description ?: "-"}",
cause = cause
)

class GetSubtreeRootsException(code: Int, description: String?, cause: Throwable) : SdkException(
"Failed to get subtree roots with code: $code due to: ${description ?: "-"}",
cause
class GetSubtreeRootsException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message = "Failed to get subtree roots with code: $code due to: ${description ?: "-"}",
cause = cause
)

class FetchUtxosException(code: Int, description: String?, cause: Throwable) : SdkException(
"Failed to fetch UTXOs with code: $code due to: ${description ?: "-"}",
cause
class FetchUtxosException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message = "Failed to fetch UTXOs with code: $code due to: ${description ?: "-"}",
cause = cause
)

class GetLatestBlockHeightException(code: Int, description: String?, cause: Throwable) : SdkException(
"Failed to fetch latest block height with code: $code due to: ${description ?: "-"}",
cause
class GetLatestBlockHeightException(code: Int, description: String?, cause: Throwable) : LightWalletException(
message = "Failed to fetch latest block height with code: $code due to: ${description ?: "-"}",
cause = cause
)

class GetTAddressTransactionsException(code: Int, description: String?, cause: Throwable) : SdkException(
"Failed to get transactions belonging to the given transparent address with code: $code due" +
" to: ${description ?: "-"}",
cause
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
)

class TransactionSubmitException(code: Int? = null, description: String? = null) : LightWalletException(
message =
"Failed to submit transaction to the lightwalletd server with code: ${code ?: "-"} due" +
" to: ${description ?: "-"}",
cause = null
)
}

Expand Down Expand Up @@ -340,5 +348,3 @@ sealed class TransactionEncoderException(
" height was $lastScannedHeight."
)
}

class TransactionSubmitException : Exception()
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ internal class AllTransactionView(

private const val QUERY_LIMIT = "1" // $NON-NLS

private const val SENT_TRANSACTION_RECOGNITION_VALUE = "0" // $NON-NLS

private val COLUMNS =
arrayOf(
// $NON-NLS
Expand Down Expand Up @@ -57,6 +59,23 @@ internal class AllTransactionView(
AllTransactionViewDefinition.COLUMN_INTEGER_MINED_HEIGHT
)

/**
* Get all sent, unmined transactions that are still within the expiry window
*
* Requested selection should look like this:
* mined_height IS NULL AND expiry_height > ? AND account_balance_delta < 0
*/
private val SELECTION_TRX_RESUBMISSION =
String.format(
Locale.ROOT,
// $NON-NLS
"%s IS NULL AND %s > ? AND %s < %s",
AllTransactionViewDefinition.COLUMN_INTEGER_MINED_HEIGHT,
AllTransactionViewDefinition.COLUMN_INTEGER_EXPIRY_HEIGHT,
AllTransactionViewDefinition.COLUMN_LONG_ACCOUNT_BALANCE_DELTA,
SENT_TRANSACTION_RECOGNITION_VALUE
)

private val SELECTION_RAW_IS_NULL =
String.format(
Locale.ROOT,
Expand Down Expand Up @@ -139,6 +158,16 @@ internal class AllTransactionView(
cursorParser = cursorParser
)

fun getUnminedUnexpiredTransactions(blockHeight: BlockHeight) =
sqliteDatabase.queryAndMap(
table = AllTransactionViewDefinition.VIEW_NAME,
columns = COLUMNS,
orderBy = ORDER_BY,
selection = SELECTION_TRX_RESUBMISSION,
selectionArgs = arrayOf(blockHeight.value),
cursorParser = cursorParser
)

suspend fun getOldestTransaction() =
sqliteDatabase.queryAndMap(
table = AllTransactionViewDefinition.VIEW_NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ internal class DbDerivedDataRepository(
return derivedDataDb.transactionTable.findEncodedTransactionByTxId(txId)
}

override suspend fun findUnminedTransactionsWithinExpiry(blockHeight: BlockHeight): List<DbTransactionOverview> =
derivedDataDb.allTransactionView.getUnminedUnexpiredTransactions(blockHeight).toList()

override suspend fun getOldestTransaction() = derivedDataDb.allTransactionView.getOldestTransaction()

override suspend fun findMinedHeight(rawTransactionId: ByteArray) =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package cash.z.ecc.android.sdk.internal.model

import cash.z.ecc.android.sdk.internal.ext.toHexReversed
import cash.z.ecc.android.sdk.model.BlockHeight
import cash.z.ecc.android.sdk.model.FirstClassByteArray

internal data class EncodedTransaction(
val txId: FirstClassByteArray,
val raw: FirstClassByteArray,
val expiryHeight: BlockHeight?
)
) {
override fun toString() = "EncodedTransaction"

fun txIdString(): String {
return txId.byteArray.toHexReversed()
}
}
Loading

0 comments on commit a639394

Please sign in to comment.