From 44d1d5520152d53f573f8df28095fbdf3506a2f2 Mon Sep 17 00:00:00 2001 From: Kevin Gorham Date: Fri, 7 May 2021 03:59:47 -0400 Subject: [PATCH] Improvement: Refactor the initializer and move all DB creation code. Move the code to create, migrate and populate data from the initializer over to the repository. Both classes are now much simpler. --- .../cash/z/ecc/android/sdk/Initializer.kt | 74 +---- .../cash/z/ecc/android/sdk/SdkSynchronizer.kt | 10 +- .../transaction/PagedTransactionRepository.kt | 263 +++++++++++++----- 3 files changed, 200 insertions(+), 147 deletions(-) diff --git a/src/main/java/cash/z/ecc/android/sdk/Initializer.kt b/src/main/java/cash/z/ecc/android/sdk/Initializer.kt index e673c512..cfea3a79 100644 --- a/src/main/java/cash/z/ecc/android/sdk/Initializer.kt +++ b/src/main/java/cash/z/ecc/android/sdk/Initializer.kt @@ -3,7 +3,6 @@ package cash.z.ecc.android.sdk import android.content.Context import cash.z.ecc.android.sdk.exception.InitializerException import cash.z.ecc.android.sdk.ext.ZcashSdk -import cash.z.ecc.android.sdk.ext.tryWarn import cash.z.ecc.android.sdk.ext.twig import cash.z.ecc.android.sdk.jni.RustBackend import cash.z.ecc.android.sdk.tool.DerivationTool @@ -24,6 +23,7 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr val host: String val port: Int val viewingKeys: List + val overwriteVks: Boolean val birthday: WalletBirthday /** @@ -51,13 +51,11 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr val loadedBirthday = WalletBirthdayTool.loadNearest(context, network, heightToUse) birthday = loadedBirthday viewingKeys = config.viewingKeys + overwriteVks = config.overwriteVks alias = config.alias host = config.host port = config.port rustBackend = initRustBackend(network, birthday) - if (config.resetAccounts) resetAccounts() - // TODO: get rid of this by first answering the question: why is this necessary? - initMissingDatabases(birthday, *viewingKeys.toTypedArray()) } catch (t: Throwable) { onCriticalError(t) throw t @@ -79,68 +77,6 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr ) } - private fun initMissingDatabases( - birthday: WalletBirthday, - vararg viewingKeys: UnifiedViewingKey - ) { - maybeCreateDataDb() - maybeInitBlocksTable(birthday) - maybeInitAccountsTable(*viewingKeys) - } - - private fun resetAccounts() { - // Short-term fix: drop and recreate accounts for key migration - tryWarn("Warning: did not drop the accounts table. It probably did not yet exist.") { - rustBackend.dropAccountsTable() - twig("Reset accounts table to allow for key migration") - } - } - - /** - * Create the dataDb and its table, if it doesn't exist. - */ - private fun maybeCreateDataDb() { - tryWarn("Warning: did not create dataDb. It probably already exists.") { - rustBackend.initDataDb() - twig("Initialized wallet for first run") - } - } - - /** - * 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.", - ifContains = "table is not empty" - ) { - rustBackend.initBlocksTable( - birthday.height, - birthday.hash, - birthday.time, - birthday.tree - ) - twig("seeded the database with sapling tree at height ${birthday.height}") - } - twig("database file: ${rustBackend.pathDataDb}") - } - - /** - * Initialize the accounts table with the given viewing keys. - */ - 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.", - ifContains = "table is not empty" - ) { - rustBackend.initAccountsTable(*viewingKeys) - accountsCreated = true - twig("Initialized the accounts table with ${viewingKeys.size} viewingKey(s)") - } - } - private fun onCriticalError(error: Throwable) { twig("********") twig("******** INITIALIZER ERROR: $error") @@ -192,7 +128,7 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr var defaultToOldestHeight: Boolean? = null private set - var resetAccounts: Boolean = false + var overwriteVks: Boolean = false private set constructor(block: (Config) -> Unit) : this() { @@ -250,7 +186,7 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr * probably has serious bugs. */ fun setViewingKeys(vararg unifiedViewingKeys: UnifiedViewingKey, overwrite: Boolean = false): Config = apply { - resetAccounts = overwrite + overwriteVks = overwrite viewingKeys.apply { clear() addAll(unifiedViewingKeys) @@ -258,7 +194,7 @@ class Initializer constructor(appContext: Context, onCriticalErrorHandler: ((Thr } fun setOverwriteKeys(isOverwrite: Boolean) { - resetAccounts = isOverwrite + overwriteVks = isOverwrite } /** diff --git a/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt b/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt index 565667cf..115fa5ca 100644 --- a/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt +++ b/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt @@ -38,7 +38,6 @@ import cash.z.ecc.android.sdk.ext.toHexReversed import cash.z.ecc.android.sdk.ext.tryNull import cash.z.ecc.android.sdk.ext.twig import cash.z.ecc.android.sdk.ext.twigTask -import cash.z.ecc.android.sdk.jni.RustBackend import cash.z.ecc.android.sdk.service.LightWalletGrpcService import cash.z.ecc.android.sdk.service.LightWalletService import cash.z.ecc.android.sdk.tool.DerivationTool @@ -138,10 +137,10 @@ class SdkSynchronizer internal constructor( // override val balances: Flow = _balances.asFlow() - override val clearedTransactions = storage.allTransactions + override val clearedTransactions get() = storage.allTransactions override val pendingTransactions = txManager.getAll() - override val sentTransactions = storage.sentTransactions - override val receivedTransactions = storage.receivedTransactions + override val sentTransactions get() = storage.sentTransactions + override val receivedTransactions get() = storage.receivedTransactions // // Status @@ -402,6 +401,7 @@ class SdkSynchronizer internal constructor( private fun onCriticalError(unused: CoroutineContext?, error: Throwable) { twig("********") twig("******** ERROR: $error") + twig(error) if (error.cause != null) twig("******** caused by ${error.cause}") if (error.cause?.cause != null) twig("******** caused by ${error.cause?.cause}") twig("********") @@ -759,7 +759,7 @@ class SdkSynchronizer internal constructor( fun Synchronizer( initializer: Initializer, repository: TransactionRepository = - PagedTransactionRepository(initializer.context, 1000, initializer.rustBackend.pathDataDb), // TODO: fix this pagesize bug, small pages should not crash the app. It crashes with: Uncaught Exception: android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views. and is probably related to FlowPagedList + PagedTransactionRepository(initializer.context, 1000, initializer.rustBackend, initializer.birthday, initializer.viewingKeys, initializer.overwriteVks), // TODO: fix this pagesize bug, small pages should not crash the app. It crashes with: Uncaught Exception: android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views. and is probably related to FlowPagedList blockStore: CompactBlockStore = CompactBlockDbStore(initializer.context, initializer.rustBackend.pathCacheDb), service: LightWalletService = LightWalletGrpcService(initializer.context, initializer.host, initializer.port), encoder: TransactionEncoder = WalletTransactionEncoder(initializer.rustBackend, repository), diff --git a/src/main/java/cash/z/ecc/android/sdk/transaction/PagedTransactionRepository.kt b/src/main/java/cash/z/ecc/android/sdk/transaction/PagedTransactionRepository.kt index 680183dc..4fbaca81 100644 --- a/src/main/java/cash/z/ecc/android/sdk/transaction/PagedTransactionRepository.kt +++ b/src/main/java/cash/z/ecc/android/sdk/transaction/PagedTransactionRepository.kt @@ -9,12 +9,21 @@ import cash.z.ecc.android.sdk.db.BlockDao import cash.z.ecc.android.sdk.db.DerivedDataDb import cash.z.ecc.android.sdk.db.TransactionDao import cash.z.ecc.android.sdk.db.entity.ConfirmedTransaction +import cash.z.ecc.android.sdk.exception.RepositoryException import cash.z.ecc.android.sdk.ext.ZcashSdk +import cash.z.ecc.android.sdk.ext.android.RefreshableDataSourceFactory import cash.z.ecc.android.sdk.ext.android.toFlowPagedList import cash.z.ecc.android.sdk.ext.android.toRefreshable +import cash.z.ecc.android.sdk.ext.tryWarn +import cash.z.ecc.android.sdk.ext.twig +import cash.z.ecc.android.sdk.jni.RustBackend import cash.z.ecc.android.sdk.type.UnifiedAddressAccount +import cash.z.ecc.android.sdk.type.UnifiedViewingKey +import cash.z.ecc.android.sdk.type.WalletBirthday import kotlinx.coroutines.Dispatchers.IO +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.withContext +import java.util.concurrent.atomic.AtomicBoolean /** * Example of a repository that leverages the Room paging library to return a [PagedList] of @@ -23,100 +32,208 @@ import kotlinx.coroutines.withContext * * @param pageSize transactions per page. This influences pre-fetch and memory configuration. */ -open class PagedTransactionRepository( - open val derivedDataDb: DerivedDataDb, - open val pageSize: Int = 10 +class PagedTransactionRepository( + val appContext: Context, + val pageSize: Int = 10, + val rustBackend: RustBackend, + val birthday: WalletBirthday, + val viewingKeys: List, + val overwriteVks: Boolean = false, ) : TransactionRepository { + private val lazy = LazyPropertyHolder() + + override val receivedTransactions get() = lazy.receivedTransactions + override val sentTransactions get() = lazy.sentTransactions + override val allTransactions get() = lazy.allTransactions + + // + // TransactionRepository API + // + + override fun invalidate() = lazy.allTransactionsFactory.refresh() + + override fun lastScannedHeight(): Int { + return lazy.blocks.lastScannedHeight() + } + + override fun firstScannedHeight(): Int { + return lazy.blocks.firstScannedHeight() + } + + override fun isInitialized(): Boolean { + return lazy.blocks.count() > 0 + } + + override suspend fun findEncodedTransactionById(txId: Long) = withContext(IO) { + lazy.transactions.findEncodedTransactionById(txId) + } + + override suspend fun findNewTransactions(blockHeightRange: IntRange): List = + lazy.transactions.findAllTransactionsByRange(blockHeightRange.first, blockHeightRange.last) + + override suspend fun findMinedHeight(rawTransactionId: ByteArray) = withContext(IO) { + lazy.transactions.findMinedHeight(rawTransactionId) + } + + override suspend fun findMatchingTransactionId(rawTransactionId: ByteArray): Long? = + lazy.transactions.findMatchingTransactionId(rawTransactionId) + + override suspend fun cleanupCancelledTx(rawTransactionId: ByteArray) = lazy.transactions.cleanupCancelledTx(rawTransactionId) + override suspend fun deleteExpired(lastScannedHeight: Int): Int { + // let expired transactions linger in the UI for a little while + return lazy.transactions.deleteExpired(lastScannedHeight - (ZcashSdk.EXPIRY_OFFSET / 2)) + } + override suspend fun count(): Int = withContext(IO) { + lazy.transactions.count() + } + + override suspend fun getAccount(accountId: Int): UnifiedAddressAccount? = lazy.accounts.findAccountById(accountId) + + override suspend fun getAccountCount(): Int = lazy.accounts.count() + + override fun prepare() { + twig("Preparing repository for use...") + initMissingDatabases() + // provide the database to all the lazy properties that are waiting for it to exist + lazy.db = buildDatabase() + applyKeyMigrations() + } + /** - * Constructor that creates the database. + * Create any databases that don't already exist via Rust. Originally, this was done on the Rust + * side because Rust was intended to own the "dataDb" and Kotlin just reads from it. Since then, + * it has been more clear that Kotlin should own the data and just let Rust use it. */ - constructor( - context: Context, - pageSize: Int = 10, - dataDbName: String = ZcashSdk.DB_DATA_NAME - ) : this( - Room.databaseBuilder(context, DerivedDataDb::class.java, dataDbName) + private fun initMissingDatabases() { + maybeCreateDataDb() + maybeInitBlocksTable(birthday) + maybeInitAccountsTable(viewingKeys) + } + + /** + * Create the dataDb and its table, if it doesn't exist. + */ + private fun maybeCreateDataDb() { + tryWarn("Warning: did not create dataDb. It probably already exists.") { + rustBackend.initDataDb() + twig("Initialized wallet for first run file: ${rustBackend.pathDataDb}") + } + } + + /** + * 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.", + ifContains = "table is not empty" + ) { + rustBackend.initBlocksTable( + birthday.height, + birthday.hash, + birthday.time, + birthday.tree + ) + twig("seeded the database with sapling tree at height ${birthday.height}") + } + twig("database file: ${rustBackend.pathDataDb}") + } + + /** + * Initialize the accounts table with the given viewing keys. + */ + private fun maybeInitAccountsTable(viewingKeys: List) { + // TODO: consider converting these to typed exceptions in the welding layer + tryWarn( + "Warning: did not initialize the accounts table. It probably was already initialized.", + ifContains = "table is not empty" + ) { + rustBackend.initAccountsTable(*viewingKeys.toTypedArray()) + twig("Initialized the accounts table with ${viewingKeys.size} viewingKey(s)") + } + } + + /** + * Build the database and apply migrations. + */ + private fun buildDatabase(): DerivedDataDb { + twig("Building dataDb and applying migrations") + return Room.databaseBuilder(appContext, DerivedDataDb::class.java, rustBackend.pathDataDb) .setJournalMode(RoomDatabase.JournalMode.TRUNCATE) .addMigrations(DerivedDataDb.MIGRATION_3_4) .addMigrations(DerivedDataDb.MIGRATION_4_3) .addMigrations(DerivedDataDb.MIGRATION_4_5) .addMigrations(DerivedDataDb.MIGRATION_5_6) .addMigrations(DerivedDataDb.MIGRATION_6_7) - .build(), - pageSize - ) - init { - derivedDataDb.openHelper.writableDatabase.beginTransaction() - derivedDataDb.openHelper.writableDatabase.endTransaction() + .build().also { + // TODO: document why we do this. My guess is to catch database issues early or to trigger migrations--I forget why it was added but there was a good reason? + it.openHelper.writableDatabase.beginTransaction() + it.openHelper.writableDatabase.endTransaction() + } } - private val blocks: BlockDao = derivedDataDb.blockDao() - private val accounts: AccountDao = derivedDataDb.accountDao() - private val transactions: TransactionDao = derivedDataDb.transactionDao() - private val receivedTxDataSourceFactory = transactions.getReceivedTransactions().toRefreshable() - private val sentTxDataSourceFactory = transactions.getSentTransactions().toRefreshable() - private val allTxDataSourceFactory = transactions.getAllTransactions().toRefreshable() - - // - // TransactionRepository API - // - - override val receivedTransactions = receivedTxDataSourceFactory.toFlowPagedList(pageSize) - override val sentTransactions = sentTxDataSourceFactory.toFlowPagedList(pageSize) - override val allTransactions = allTxDataSourceFactory.toFlowPagedList(pageSize) - - override fun invalidate() = allTxDataSourceFactory.refresh() - - override fun lastScannedHeight(): Int { - return blocks.lastScannedHeight() + private fun applyKeyMigrations() { + if (overwriteVks) { + twig("applying key migrations . . .") + maybeInitAccountsTable(viewingKeys) + } } - override fun firstScannedHeight(): Int { - return blocks.firstScannedHeight() - } - - override fun isInitialized(): Boolean { - return blocks.count() > 0 - } - - override suspend fun findEncodedTransactionById(txId: Long) = withContext(IO) { - transactions.findEncodedTransactionById(txId) - } - - override suspend fun findNewTransactions(blockHeightRange: IntRange): List = - transactions.findAllTransactionsByRange(blockHeightRange.first, blockHeightRange.last) - - override suspend fun findMinedHeight(rawTransactionId: ByteArray) = withContext(IO) { - transactions.findMinedHeight(rawTransactionId) - } - - override suspend fun findMatchingTransactionId(rawTransactionId: ByteArray): Long? = - transactions.findMatchingTransactionId(rawTransactionId) - - override suspend fun cleanupCancelledTx(rawTransactionId: ByteArray) = transactions.cleanupCancelledTx(rawTransactionId) - override suspend fun deleteExpired(lastScannedHeight: Int): Int { - // let expired transactions linger in the UI for a little while - return transactions.deleteExpired(lastScannedHeight - (ZcashSdk.EXPIRY_OFFSET / 2)) - } - override suspend fun count(): Int = withContext(IO) { - transactions.count() - } - - override suspend fun getAccount(accountId: Int): UnifiedAddressAccount? = accounts.findAccountById(accountId) - - override suspend fun getAccountCount(): Int = accounts.count() - /** * Close the underlying database. */ fun close() { - derivedDataDb.close() + lazy.db?.close() } // TODO: begin converting these into Data Access API. For now, just collect the desired operations and iterate/refactor, later - fun findBlockHash(height: Int): ByteArray? = blocks.findHashByHeight(height) - fun getTransactionCount(): Int = transactions.count() + fun findBlockHash(height: Int): ByteArray? = lazy.blocks.findHashByHeight(height) + fun getTransactionCount(): Int = lazy.transactions.count() // TODO: convert this into a wallet repository rather than "transaction repository" + + /** + * Helper class that holds all the properties that depend on the database being prepared. If any + * properties are accessed before then, it results in an Unprepared Exception. + */ + inner class LazyPropertyHolder { + var isPrepared = AtomicBoolean(false) + var db: DerivedDataDb? = null + set(value) { + field = value + if (value != null) isPrepared.set(true) + } + + // DAOs + val blocks: BlockDao by lazyDb { db!!.blockDao() } + val accounts: AccountDao by lazyDb { db!!.accountDao() } + val transactions: TransactionDao by lazyDb { db!!.transactionDao() } + + // Transaction Flows + val allTransactionsFactory: RefreshableDataSourceFactory by lazyDb { + transactions.getAllTransactions().toRefreshable() + } + val allTransactions: Flow> by lazyDb { + allTransactionsFactory.toFlowPagedList(pageSize) + } + val receivedTransactions: Flow> by lazyDb { + transactions.getReceivedTransactions().toRefreshable().toFlowPagedList(pageSize) + } + val sentTransactions: Flow> by lazyDb { + transactions.getSentTransactions().toRefreshable().toFlowPagedList(pageSize) + } + + /** + * If isPrepared is true, execute the given block and cache the value, always returning it + * to future requests. Otherwise, throw an Unprepared exception. + */ + inline fun lazyDb(crossinline block: () -> T) = object : Lazy { + val cached: T? = null + override val value: T + get() = cached ?: if (isPrepared.get()) block() else throw RepositoryException.Unprepared + override fun isInitialized(): Boolean = cached != null + } + } }