From abffb3f9ee096345135bef88dd77aced69322194 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 28 Feb 2024 01:44:12 +0000 Subject: [PATCH] Adjust `Synchronizer.proposeShielding` API - Returns `null` when there are no funds to shield or the shielding threshold is not met. - Throws an exception if there are funds to shield in more than one transparent receiver within the account. - Has an optional parameter for specifying which transparent receiver to shield funds from. Part of Electric-Coin-Company/zcash-android-wallet-sdk#680. --- .../z/ecc/android/sdk/internal/Backend.kt | 5 +- .../android/sdk/internal/jni/RustBackend.kt | 29 ++++++----- backend-lib/src/main/rust/lib.rs | 49 +++++++++++++++++-- backend-lib/src/main/rust/utils.rs | 4 ++ .../android/sdk/darkside/test/TestWallet.kt | 10 ++-- .../demos/getbalance/GetBalanceFragment.kt | 10 ++-- .../screen/home/viewmodel/WalletViewModel.kt | 12 +++-- .../cash/z/ecc/android/sdk/util/TestWallet.kt | 10 ++-- .../cash/z/ecc/fixture/FakeRustBackend.kt | 5 +- .../cash/z/ecc/android/sdk/SdkSynchronizer.kt | 7 +-- .../cash/z/ecc/android/sdk/Synchronizer.kt | 17 +++++-- .../android/sdk/internal/TypesafeBackend.kt | 5 +- .../sdk/internal/TypesafeBackendImpl.kt | 20 +++++--- .../transaction/OutboundTransactionManager.kt | 15 +++++- .../OutboundTransactionManagerImpl.kt | 5 +- .../transaction/TransactionEncoder.kt | 15 +++++- .../transaction/TransactionEncoderImpl.kt | 12 +++-- 17 files changed, 167 insertions(+), 63 deletions(-) diff --git a/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/Backend.kt b/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/Backend.kt index d4dbf199..6ba1375b 100644 --- a/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/Backend.kt +++ b/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/Backend.kt @@ -31,8 +31,9 @@ interface Backend { suspend fun proposeShielding( account: Int, shieldingThreshold: Long, - memo: ByteArray? = byteArrayOf() - ): ProposalUnsafe + memo: ByteArray? = byteArrayOf(), + transparentReceiver: String? = null + ): ProposalUnsafe? suspend fun createProposedTransaction( proposal: ProposalUnsafe, diff --git a/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/jni/RustBackend.kt b/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/jni/RustBackend.kt index ec74a843..e0cfffde 100644 --- a/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/jni/RustBackend.kt +++ b/backend-lib/src/main/java/cash/z/ecc/android/sdk/internal/jni/RustBackend.kt @@ -308,19 +308,23 @@ class RustBackend private constructor( override suspend fun proposeShielding( account: Int, shieldingThreshold: Long, - memo: ByteArray? - ): ProposalUnsafe { + memo: ByteArray?, + transparentReceiver: String? + ): ProposalUnsafe? { return withContext(SdkDispatchers.DATABASE_IO) { - ProposalUnsafe.parse( - proposeShielding( - dataDbFile.absolutePath, - account, - shieldingThreshold, - memo ?: ByteArray(0), - networkId = networkId, - useZip317Fees = IS_USE_ZIP_317_FEES + proposeShielding( + dataDbFile.absolutePath, + account, + shieldingThreshold, + memo ?: ByteArray(0), + transparentReceiver, + networkId = networkId, + useZip317Fees = IS_USE_ZIP_317_FEES + )?.let { + ProposalUnsafe.parse( + it ) - ) + } } } @@ -588,9 +592,10 @@ class RustBackend private constructor( account: Int, shieldingThreshold: Long, memo: ByteArray, + transparentReceiver: String?, networkId: Int, useZip317Fees: Boolean - ): ByteArray + ): ByteArray? @JvmStatic @Suppress("LongParameterList") diff --git a/backend-lib/src/main/rust/lib.rs b/backend-lib/src/main/rust/lib.rs index b721f04e..2890400c 100644 --- a/backend-lib/src/main/rust/lib.rs +++ b/backend-lib/src/main/rust/lib.rs @@ -1482,6 +1482,7 @@ pub extern "C" fn Java_cash_z_ecc_android_sdk_internal_jni_RustBackend_proposeSh account: jint, shielding_threshold: jlong, memo: JByteArray<'local>, + transparent_receiver: JString<'local>, network_id: jint, use_zip317_fees: jboolean, ) -> jbyteArray { @@ -1494,10 +1495,36 @@ pub extern "C" fn Java_cash_z_ecc_android_sdk_internal_jni_RustBackend_proposeSh .map_err(|()| format_err!("Invalid shielding threshold, out of range"))?; let memo_bytes = env.convert_byte_array(memo).unwrap(); + let transparent_receiver = + match utils::java_nullable_string_to_rust(env, &transparent_receiver) { + None => Ok(None), + Some(addr) => match Address::decode(&network, &addr) { + None => Err(format_err!("Transparent receiver is for the wrong network")), + Some(addr) => match addr { + Address::Sapling(_) | Address::Unified(_) => Err(format_err!( + "Transparent receiver is not a transparent address" + )), + Address::Transparent(addr) => { + if db_data + .get_transparent_receivers(account)? + .contains_key(&addr) + { + Ok(Some(addr)) + } else { + Err(format_err!( + "Transparent receiver does not belong to account {}", + u32::from(account), + )) + } + } + }, + }, + }?; + let min_confirmations = 0; let min_confirmations_for_heights = NonZeroU32::new(1).unwrap(); - let from_addrs: Vec = db_data + let account_receivers = db_data .get_target_and_anchor_heights(min_confirmations_for_heights) .map_err(|e| format_err!("Error while fetching anchor height: {}", e)) .and_then(|opt_anchor| { @@ -1515,9 +1542,23 @@ pub extern "C" fn Java_cash_z_ecc_android_sdk_internal_jni_RustBackend_proposeSh e ) }) - })? - .into_keys() - .collect(); + })?; + + let from_addrs = if let Some((addr, _)) = transparent_receiver.map_or_else(|| + if account_receivers.len() > 1 { + Err(format_err!( + "Account has more than one transparent receiver with funds to shield; this is not yet supported by the SDK. Provide a specific transparent receiver to shield funds from." + )) + } else { + Ok(account_receivers.iter().next().map(|(a, v)| (*a, *v))) + }, + |addr| Ok(account_receivers.get(&addr).map(|value| (addr, *value))) + )?.filter(|(_, value)| *value >= shielding_threshold.into()) { + [addr] + } else { + // There are no transparent funds to shield; don't create a proposal. + return Ok(ptr::null_mut()); + }; let memo = Memo::from_bytes(&memo_bytes).unwrap(); diff --git a/backend-lib/src/main/rust/utils.rs b/backend-lib/src/main/rust/utils.rs index da7b7aac..ef1ed9aa 100644 --- a/backend-lib/src/main/rust/utils.rs +++ b/backend-lib/src/main/rust/utils.rs @@ -38,6 +38,10 @@ pub(crate) fn java_string_to_rust(env: &mut JNIEnv, jstring: &JString) -> String .into() } +pub(crate) fn java_nullable_string_to_rust(env: &mut JNIEnv, jstring: &JString) -> Option { + (!jstring.is_null()).then(|| java_string_to_rust(env, jstring)) +} + pub(crate) fn rust_bytes_to_java<'a>( env: &JNIEnv<'a>, data: &[u8], diff --git a/darkside-test-lib/src/androidTest/java/cash/z/ecc/android/sdk/darkside/test/TestWallet.kt b/darkside-test-lib/src/androidTest/java/cash/z/ecc/android/sdk/darkside/test/TestWallet.kt index 65dd0f66..2bbd69aa 100644 --- a/darkside-test-lib/src/androidTest/java/cash/z/ecc/android/sdk/darkside/test/TestWallet.kt +++ b/darkside-test-lib/src/androidTest/java/cash/z/ecc/android/sdk/darkside/test/TestWallet.kt @@ -129,10 +129,12 @@ class TestWallet( synchronizer.getTransparentBalance(transparentAddress).let { walletBalance -> if (walletBalance.value > 0L) { - synchronizer.createProposedTransactions( - synchronizer.proposeShielding(shieldedSpendingKey.account, Zatoshi(100000)), - shieldedSpendingKey - ) + synchronizer.proposeShielding(shieldedSpendingKey.account, Zatoshi(100000))?.let { + synchronizer.createProposedTransactions( + it, + shieldedSpendingKey + ) + } } } diff --git a/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/demos/getbalance/GetBalanceFragment.kt b/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/demos/getbalance/GetBalanceFragment.kt index 9145ca9a..d221e9ad 100644 --- a/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/demos/getbalance/GetBalanceFragment.kt +++ b/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/demos/getbalance/GetBalanceFragment.kt @@ -68,10 +68,12 @@ class GetBalanceFragment : BaseDemoFragment() { Account.DEFAULT ) sharedViewModel.synchronizerFlow.value?.let { synchronizer -> - synchronizer.createProposedTransactions( - synchronizer.proposeShielding(usk.account, Zatoshi(100000)), - usk - ) + synchronizer.proposeShielding(usk.account, Zatoshi(100000))?.let { it1 -> + synchronizer.createProposedTransactions( + it1, + usk + ) + } } } } diff --git a/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/ui/screen/home/viewmodel/WalletViewModel.kt b/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/ui/screen/home/viewmodel/WalletViewModel.kt index 417bd083..df66cb47 100644 --- a/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/ui/screen/home/viewmodel/WalletViewModel.kt +++ b/demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/ui/screen/home/viewmodel/WalletViewModel.kt @@ -230,12 +230,14 @@ class WalletViewModel(application: Application) : AndroidViewModel(application) viewModelScope.launch { val spendingKey = spendingKey.filterNotNull().first() kotlin.runCatching { - synchronizer.createProposedTransactions( - synchronizer.proposeShielding(spendingKey.account, Zatoshi(100000)), - spendingKey - ) + synchronizer.proposeShielding(spendingKey.account, Zatoshi(100000))?.let { + synchronizer.createProposedTransactions( + it, + spendingKey + ) + } } - .onSuccess { mutableSendState.value = SendState.Sent(it.toList()) } + .onSuccess { it?.let { mutableSendState.value = SendState.Sent(it.toList()) } } .onFailure { mutableSendState.value = SendState.Error(it) } } } else { diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/TestWallet.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/TestWallet.kt index a6ef3e45..1edf516a 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/TestWallet.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/TestWallet.kt @@ -132,10 +132,12 @@ class TestWallet( Twig.debug { "FOUND utxo balance of total: $walletBalance" } if (walletBalance.value > 0L) { - synchronizer.createProposedTransactions( - synchronizer.proposeShielding(spendingKey.account, Zatoshi(100000)), - spendingKey - ) + synchronizer.proposeShielding(spendingKey.account, Zatoshi(100000))?.let { + synchronizer.createProposedTransactions( + it, + spendingKey + ) + } } } diff --git a/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FakeRustBackend.kt b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FakeRustBackend.kt index 90efd678..53872421 100644 --- a/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FakeRustBackend.kt +++ b/sdk-lib/src/androidTest/java/cash/z/ecc/fixture/FakeRustBackend.kt @@ -89,8 +89,9 @@ internal class FakeRustBackend( override suspend fun proposeShielding( account: Int, shieldingThreshold: Long, - memo: ByteArray? - ): ProposalUnsafe { + memo: ByteArray?, + transparentReceiver: String? + ): ProposalUnsafe? { TODO("Not yet implemented") } diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt index 91d4eac0..af0183ef 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/SdkSynchronizer.kt @@ -570,8 +570,9 @@ class SdkSynchronizer private constructor( override suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: String - ): Proposal = txManager.proposeShielding(account, shieldingThreshold, memo) + memo: String, + transparentReceiver: String? + ): Proposal? = txManager.proposeShielding(account, shieldingThreshold, memo, transparentReceiver) @Throws(TransactionEncoderException::class) override suspend fun createProposedTransactions( @@ -636,7 +637,7 @@ class SdkSynchronizer private constructor( message = "Upcoming SDK 2.1 will create multiple transactions at once for some recipients.", replaceWith = ReplaceWith( - "createProposedTransactions(proposeShielding(usk.account, shieldingThreshold, memo), usk)" + "proposeShielding(usk.account, shieldingThreshold, memo)?.let { createProposedTransactions(it, usk) }" ) ) @Throws(TransactionEncoderException::class, TransactionSubmitException::class) diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/Synchronizer.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/Synchronizer.kt index 914b7ecc..4bc740f7 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/Synchronizer.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/Synchronizer.kt @@ -193,12 +193,23 @@ interface Synchronizer { * @param shieldingThreshold the minimum transparent balance required before a * proposal will be created. * @param memo the optional memo to include as part of the proposal's transactions. + * @param transparentReceiver a specific transparent receiver within the account that + * should be the source of transparent funds. Default is + * null which will select whichever of the account's + * transparent receivers has funds to shield. + * + * @return the proposal, or null if the transparent balance that would be shielded is + * zero or below `shieldingThreshold`. + * + * @throws Exception if `transparentReceiver` is null and there are transparent funds + * in more than one of the account's transparent receivers. */ suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: String = ZcashSdk.DEFAULT_SHIELD_FUNDS_MEMO_PREFIX - ): Proposal + memo: String = ZcashSdk.DEFAULT_SHIELD_FUNDS_MEMO_PREFIX, + transparentReceiver: String? = null + ): Proposal? /** * Creates the transactions in the given proposal. @@ -247,7 +258,7 @@ interface Synchronizer { message = "Upcoming SDK 2.1 will create multiple transactions at once for some recipients.", replaceWith = ReplaceWith( - "createProposedTransactions(proposeShielding(usk.account, shieldingThreshold, memo), usk)" + "proposeShielding(usk.account, shieldingThreshold, memo)?.let { createProposedTransactions(it, usk) }" ) ) suspend fun shieldFunds( diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackend.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackend.kt index a8a3a272..69a7c0b6 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackend.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackend.kt @@ -35,8 +35,9 @@ internal interface TypesafeBackend { suspend fun proposeShielding( account: Account, shieldingThreshold: Long, - memo: ByteArray? = byteArrayOf() - ): Proposal + memo: ByteArray? = byteArrayOf(), + transparentReceiver: String? = null + ): Proposal? suspend fun createProposedTransaction( proposal: Proposal, diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackendImpl.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackendImpl.kt index 58afb413..5d66a329 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackendImpl.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/TypesafeBackendImpl.kt @@ -54,15 +54,19 @@ internal class TypesafeBackendImpl(private val backend: Backend) : TypesafeBacke override suspend fun proposeShielding( account: Account, shieldingThreshold: Long, - memo: ByteArray? - ): Proposal = - Proposal.fromUnsafe( - backend.proposeShielding( - account.value, - shieldingThreshold, - memo + memo: ByteArray?, + transparentReceiver: String? + ): Proposal? = + backend.proposeShielding( + account.value, + shieldingThreshold, + memo, + transparentReceiver + )?.let { + Proposal.fromUnsafe( + it ) - ) + } override suspend fun createProposedTransaction( proposal: Proposal, diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManager.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManager.kt index 0e4ca96b..fcc9b3e2 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManager.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManager.kt @@ -59,12 +59,23 @@ internal interface OutboundTransactionManager { * @param shieldingThreshold the minimum transparent balance required before a * proposal will be created. * @param memo the optional memo to include as part of the proposal's transactions. + * @param transparentReceiver a specific transparent receiver within the account that + * should be the source of transparent funds. Default is + * null which will select whichever of the account's + * transparent receivers has funds to shield. + * + * @return the proposal, or null if the transparent balance that would be shielded is + * zero or below `shieldingThreshold`. + * + * @throws Exception if `transparentReceiver` is null and there are transparent funds + * in more than one of the account's transparent receivers. */ suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: String - ): Proposal + memo: String, + transparentReceiver: String? + ): Proposal? /** * Creates the transactions in the given proposal. diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManagerImpl.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManagerImpl.kt index 3d5143b3..45fd421a 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManagerImpl.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/OutboundTransactionManagerImpl.kt @@ -53,8 +53,9 @@ internal class OutboundTransactionManagerImpl( override suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: String - ): Proposal = encoder.proposeShielding(account, shieldingThreshold, memo.toByteArray()) + memo: String, + transparentReceiver: String? + ): Proposal? = encoder.proposeShielding(account, shieldingThreshold, memo.toByteArray(), transparentReceiver) override suspend fun createProposedTransactions( proposal: Proposal, diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoder.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoder.kt index cd1845d4..c0aa8cea 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoder.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoder.kt @@ -64,12 +64,23 @@ internal interface TransactionEncoder { * @param shieldingThreshold the minimum transparent balance required before a * proposal will be created. * @param memo the optional memo to include as part of the proposal's transactions. + * @param transparentReceiver a specific transparent receiver within the account that + * should be the source of transparent funds. Default is + * null which will select whichever of the account's + * transparent receivers has funds to shield. + * + * @return the proposal, or null if the transparent balance that would be shielded is + * zero or below `shieldingThreshold`. + * + * @throws Exception if `transparentReceiver` is null and there are transparent funds + * in more than one of the account's transparent receivers. */ suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: ByteArray? = byteArrayOf() - ): Proposal + memo: ByteArray? = byteArrayOf(), + transparentReceiver: String? = null + ): Proposal? /** * Creates the transactions in the given proposal. diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoderImpl.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoderImpl.kt index 9509dca3..671c089a 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoderImpl.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/transaction/TransactionEncoderImpl.kt @@ -1,5 +1,6 @@ package cash.z.ecc.android.sdk.internal.transaction +import cash.z.ecc.android.sdk.exception.SdkException import cash.z.ecc.android.sdk.exception.TransactionEncoderException import cash.z.ecc.android.sdk.ext.masked import cash.z.ecc.android.sdk.internal.SaplingParamTool @@ -97,11 +98,12 @@ internal class TransactionEncoderImpl( override suspend fun proposeShielding( account: Account, shieldingThreshold: Zatoshi, - memo: ByteArray? - ): Proposal { + memo: ByteArray?, + transparentReceiver: String? + ): Proposal? { @Suppress("TooGenericExceptionCaught") return try { - backend.proposeShielding(account, shieldingThreshold.value, memo) + backend.proposeShielding(account, shieldingThreshold.value, memo, transparentReceiver) } catch (t: Throwable) { // TODO [#680]: if this error matches: Insufficient balance (have 0, need 1000 including fee) // then consider custom error that says no UTXOs existed to shield @@ -239,7 +241,9 @@ internal class TransactionEncoderImpl( return try { saplingParamTool.ensureParams(saplingParamTool.properties.paramsDirectory) Twig.debug { "params exist! attempting to shield..." } - val proposal = backend.proposeShielding(usk.account, 100000, memo) + val proposal = + backend.proposeShielding(usk.account, 100000, memo) + ?: throw SdkException("Insufficient balance (have 0, need 100000 including fee)", null) backend.createProposedTransaction(proposal, usk) } catch (t: Throwable) { // TODO [#680]: if this error matches: Insufficient balance (have 0, need 1000 including fee)