Remove `DerivationTool` APIs for deriving transparent keys and addresses

Users should use the UnifiedAddress etc. APIs instead.

CHANGELOG.md

@@ -20,8 +20,6 @@ Change Log
 - `cash.z.ecc.android.sdk.tool`:
   - `DerivationTool.deriveUnifiedSpendingKey`
   - `DerivationTool.deriveUnifiedFullViewingKey`
-  - `DerivationTool.deriveTransparentAccountPrivateKey`
-  - `DerivationTool.deriveTransparentAddressFromAccountPrivateKey`
   - `DerivationTool.deriveUnifiedAddress`
   - `DerivationTool.deriveUnifiedFullViewingKeys`
   - `DerivationTool.validateUnifiedFullViewingKey`
@@ -60,18 +58,13 @@ Change Log
     public key, and not the extended public key as intended. This made it incompatible
     with ZIP 316.
 - `cash.z.ecc.android.sdk.tool`:
-  - `DerivationTool.deriveSpendingKeys` (use
-    `DerivationTool.deriveUnifiedSpendingKey` instead).
-  - `DerivationTool.deriveViewingKey` (use
-  - `DerivationTool.deriveUnifiedFullViewingKey` instead).
-  - `DerivationTool.deriveTransparentAddressFromPrivateKey` (use
-    `DerivationTool.deriveTransparentAddressFromAccountPrivateKey` instead).
-  - `DerivationTool.deriveTransparentSecretKey` (use
-    `DerivationTool.deriveTransparentAccountPrivateKey` instead).
+  - `DerivationTool.deriveSpendingKeys` (use `DerivationTool.deriveUnifiedSpendingKey` instead).
+  - `DerivationTool.deriveViewingKey` (use `DerivationTool.deriveUnifiedFullViewingKey` instead).
+  - `DerivationTool.deriveTransparentAddress` (use `Synchronizer.getLegacyTransparentAddress` instead).
+  - `DerivationTool.deriveTransparentAddressFromPrivateKey` (use `Synchronizer.getLegacyTransparentAddress` instead).
+  - `DerivationTool.deriveTransparentAddressFromPublicKey` (use `Synchronizer.getLegacyTransparentAddress` instead).
+  - `DerivationTool.deriveTransparentSecretKey` (use `DerivationTool.deriveUnifiedSpendingKey` instead).
   - `DerivationTool.deriveShieldedAddress`
-    - TODO: Do we still need to be able to derive Sapling shielded addresses for legacy
-      support? Currently removed because `UnifiedFullViewingKey` doesn't expose the
-      Sapling FVK on the Kotlin side (unlike the previous `UnifiedViewingKey`).
   - `DerivationTool.deriveUnifiedViewingKeys`
   - `DerivationTool.validateUnifiedViewingKey`
 

darkside-test-lib/src/androidTest/java/cash/z/ecc/android/sdk/darkside/test/TestWallet.kt

@@ -65,8 +65,6 @@ class TestWallet(
     private val seed: ByteArray = Mnemonics.MnemonicCode(seedPhrase).toSeed()
     private val shieldedSpendingKey =
         runBlocking { DerivationTool.deriveUnifiedSpendingKey(seed, network = network, Account.DEFAULT) }
-    private val transparentAccountPrivateKey =
-        runBlocking { DerivationTool.deriveTransparentAccountPrivateKey(seed, network = network, Account.DEFAULT) }
     val synchronizer: SdkSynchronizer = Synchronizer.newBlocking(
         context,
         network,
@@ -81,9 +79,9 @@ class TestWallet(
 
     val available get() = synchronizer.saplingBalances.value?.available
     val unifiedAddress =
-        runBlocking { DerivationTool.deriveUnifiedAddress(seed, network = network, Account.DEFAULT) }
+        runBlocking { synchronizer.getCurrentAddress(Account.DEFAULT) }
     val transparentAddress =
-        runBlocking { DerivationTool.deriveTransparentAddress(seed, network = network, Account.DEFAULT) }
+        runBlocking { synchronizer.getLegacyTransparentAddress(Account.DEFAULT) }
     val birthdayHeight get() = synchronizer.latestBirthdayHeight
     val networkName get() = synchronizer.network.networkName
 
@@ -139,7 +137,7 @@ class TestWallet(
             twig("FOUND utxo balance of total: ${walletBalance.total}  available: ${walletBalance.available}")
 
             if (walletBalance.available.value > 0L) {
-                synchronizer.shieldFunds(shieldedSpendingKey, transparentAccountPrivateKey)
+                synchronizer.shieldFunds(shieldedSpendingKey)
                     .onCompletion { twig("done shielding funds") }
                     .catch { twig("Failed with $it") }
                     .collect()

demo-app/src/main/java/cash/z/ecc/android/sdk/demoapp/demos/listutxos/ListUtxosFragment.kt

@@ -24,7 +24,6 @@ import cash.z.ecc.android.sdk.model.LightWalletEndpoint
 import cash.z.ecc.android.sdk.model.TransactionOverview
 import cash.z.ecc.android.sdk.model.ZcashNetwork
 import cash.z.ecc.android.sdk.model.defaultForNetwork
-import cash.z.ecc.android.sdk.tool.DerivationTool
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.first
@@ -173,14 +172,9 @@ class ListUtxosFragment : BaseDemoFragment<FragmentListUtxosBinding>() {
     override fun onResume() {
         super.onResume()
         resetInBackground()
-        val seed = Mnemonics.MnemonicCode(sharedViewModel.seedPhrase.value).toSeed()
         viewLifecycleOwner.lifecycleScope.launchWhenStarted {
             binding.inputAddress.setText(
-                DerivationTool.deriveTransparentAddress(
-                    seed,
-                    ZcashNetwork.fromResources(requireApplicationContext()),
-                    Account.DEFAULT
-                )
+                synchronizer.getLegacyTransparentAddress(Account.DEFAULT)
             )
         }
     }

sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/jni/TransparentTest.kt

@@ -7,7 +7,6 @@ import cash.z.ecc.android.sdk.annotation.MaintainedTest
 import cash.z.ecc.android.sdk.annotation.TestPurpose
 import cash.z.ecc.android.sdk.internal.TroubleshootingTwig
 import cash.z.ecc.android.sdk.internal.Twig
-import cash.z.ecc.android.sdk.model.Account
 import cash.z.ecc.android.sdk.model.ZcashNetwork
 import cash.z.ecc.android.sdk.tool.DerivationTool
 import kotlinx.coroutines.runBlocking
@@ -22,29 +21,6 @@ import org.junit.runners.Parameterized
 @SmallTest
 class TransparentTest(val expected: Expected, val network: ZcashNetwork) {
 
-    @Test
-    fun deriveTransparentAccountPrivateKeyTest() = runBlocking {
-        assertEquals(
-            expected.tAccountPrivKey,
-            DerivationTool.deriveTransparentAccountPrivateKey(
-                SEED,
-                network = network,
-                Account.DEFAULT
-            )
-        )
-    }
-
-    @Test
-    fun deriveTransparentAddressTest() = runBlocking {
-        assertEquals(expected.tAddr, DerivationTool.deriveTransparentAddress(SEED, network = network, Account.DEFAULT))
-    }
-
-    @Test
-    fun deriveTransparentAddressFromAccountPrivateKeyTest() = runBlocking {
-        val pk = DerivationTool.deriveTransparentAccountPrivateKey(SEED, network = network, Account.DEFAULT)
-        assertEquals(expected.tAddr, DerivationTool.deriveTransparentAddressFromAccountPrivateKey(pk, network = network))
-    }
-
     @Test
     fun deriveUnifiedFullViewingKeysFromSeedTest() = runBlocking {
         val ufvks = DerivationTool.deriveUnifiedFullViewingKeys(SEED, network = network)

sdk-lib/src/androidTest/java/cash/z/ecc/android/sdk/util/TestWallet.kt

@@ -65,8 +65,6 @@ class TestWallet(
     private val seed: ByteArray = Mnemonics.MnemonicCode(seedPhrase).toSeed()
     private val spendingKey =
         runBlocking { DerivationTool.deriveUnifiedSpendingKey(seed, network = network, Account.DEFAULT) }
-    private val transparentAccountPrivateKey =
-        runBlocking { DerivationTool.deriveTransparentAccountPrivateKey(seed, network = network, Account.DEFAULT) }
     val synchronizer: SdkSynchronizer = Synchronizer.newBlocking(
         context,
         network,
@@ -79,9 +77,9 @@ class TestWallet(
 
     val available get() = synchronizer.saplingBalances.value?.available
     val unifiedAddress =
-        runBlocking { DerivationTool.deriveUnifiedAddress(seed, network = network, Account.DEFAULT) }
+        runBlocking { synchronizer.getCurrentAddress(Account.DEFAULT) }
     val transparentAddress =
-        runBlocking { DerivationTool.deriveTransparentAddress(seed, network = network, Account.DEFAULT) }
+        runBlocking { synchronizer.getLegacyTransparentAddress(Account.DEFAULT) }
     val birthdayHeight get() = synchronizer.latestBirthdayHeight
     val networkName get() = synchronizer.network.networkName
 
@@ -141,7 +139,7 @@ class TestWallet(
             twig("FOUND utxo balance of total: ${walletBalance.total}  available: ${walletBalance.available}")
 
             if (walletBalance.available.value > 0L) {
-                synchronizer.shieldFunds(spendingKey, transparentAccountPrivateKey)
+                synchronizer.shieldFunds(spendingKey)
                     .onCompletion { twig("done shielding funds") }
                     .catch { twig("Failed with $it") }
                     .collect()

sdk-lib/src/main/java/cash/z/ecc/android/sdk/jni/RustBackendWelding.kt

@@ -114,30 +114,6 @@ internal interface RustBackendWelding {
             account: Account
         ): UnifiedSpendingKey
 
-        suspend fun deriveTransparentAddress(
-            seed: ByteArray,
-            network: ZcashNetwork,
-            account: Account,
-            index: Int = 0
-        ): String
-
-        suspend fun deriveTransparentAddressFromPublicKey(
-            publicKey: String,
-            network: ZcashNetwork
-        ): String
-
-        suspend fun deriveTransparentAddressFromAccountPrivateKey(
-            privateKey: String,
-            network: ZcashNetwork,
-            index: Int = 0
-        ): String
-
-        suspend fun deriveTransparentAccountPrivateKey(
-            seed: ByteArray,
-            network: ZcashNetwork,
-            account: Account
-        ): String
-
         suspend fun deriveUnifiedFullViewingKey(
             usk: UnifiedSpendingKey,
             network: ZcashNetwork

sdk-lib/src/main/java/cash/z/ecc/android/sdk/tool/DerivationTool.kt

@@ -97,41 +97,6 @@ class DerivationTool {
             deriveUnifiedAddressFromViewingKey(viewingKey, networkId = network.id)
         }
 
-        // WIP probably shouldn't be used just yet. Why?
-        //  - because we need the private key associated with this seed and this function doesn't return it.
-        //  - the underlying implementation needs to be split out into a few lower-level calls
-        override suspend fun deriveTransparentAddress(
-            seed: ByteArray,
-            network: ZcashNetwork,
-            account: Account,
-            index: Int
-        ): String = withRustBackendLoaded {
-            deriveTransparentAddressFromSeed(seed, account.value, index, networkId = network.id)
-        }
-
-        override suspend fun deriveTransparentAddressFromPublicKey(
-            publicKey: String,
-            network: ZcashNetwork
-        ): String = withRustBackendLoaded {
-            deriveTransparentAddressFromPubKey(pk = publicKey, networkId = network.id)
-        }
-
-        override suspend fun deriveTransparentAddressFromAccountPrivateKey(
-            privateKey: String,
-            network: ZcashNetwork,
-            index: Int
-        ): String = withRustBackendLoaded {
-            deriveTransparentAddressFromAccountPrivKey(sk = privateKey, index = index, networkId = network.id)
-        }
-
-        override suspend fun deriveTransparentAccountPrivateKey(
-            seed: ByteArray,
-            network: ZcashNetwork,
-            account: Account
-        ): String = withRustBackendLoaded {
-            deriveTransparentAccountPrivKeyFromSeed(seed, account.value, networkId = network.id)
-        }
-
         @Suppress("UNUSED_PARAMETER")
         fun validateUnifiedFullViewingKey(viewingKey: UnifiedFullViewingKey, networkId: Int = ZcashNetwork.Mainnet.id) {
             // TODO [#654] https://github.com/zcash/zcash-android-wallet-sdk/issues/654
@@ -177,26 +142,5 @@ class DerivationTool {
 
         @JvmStatic
         private external fun deriveUnifiedAddressFromViewingKey(key: String, networkId: Int): String
-
-        @JvmStatic
-        private external fun deriveTransparentAddressFromSeed(
-            seed: ByteArray,
-            account: Int,
-            index: Int,
-            networkId: Int
-        ): String
-
-        @JvmStatic
-        private external fun deriveTransparentAddressFromPubKey(pk: String, networkId: Int): String
-
-        @JvmStatic
-        private external fun deriveTransparentAddressFromAccountPrivKey(sk: String, index: Int, networkId: Int): String
-
-        @JvmStatic
-        private external fun deriveTransparentAccountPrivKeyFromSeed(
-            seed: ByteArray,
-            account: Int,
-            networkId: Int
-        ): String
     }
 }

sdk-lib/src/main/rust/lib.rs

@@ -6,11 +6,9 @@ use std::convert::{TryFrom, TryInto};
 use std::panic;
 use std::path::Path;
 use std::ptr;
-use std::str::FromStr;
 
 use android_logger::Config;
 use failure::format_err;
-use hdwallet::traits::{Deserialize, Serialize};
 use jni::objects::{JObject, JValue};
 use jni::{
     objects::{JClass, JString},
@@ -19,7 +17,6 @@ use jni::{
 };
 use log::Level;
 use schemer::MigratorError;
-use secp256k1::PublicKey;
 use secrecy::{ExposeSecret, SecretVec};
 use zcash_address::{ToAddress, ZcashAddress};
 use zcash_client_backend::keys::{DecodingError, UnifiedSpendingKey};
@@ -45,8 +42,6 @@ use zcash_client_sqlite::{
     BlockDb, NoteId, WalletDb,
 };
 use zcash_primitives::consensus::Network::{MainNetwork, TestNetwork};
-#[allow(deprecated)]
-use zcash_primitives::legacy::keys::{pubkey_to_address, AccountPrivKey};
 use zcash_primitives::{
     block::BlockHash,
     consensus::{BlockHeight, BranchId, Network, Parameters},
@@ -1019,139 +1014,6 @@ pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_jni_RustBackend_putUtxo(
     unwrap_exc_or(&env, res, JNI_FALSE)
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_tool_DerivationTool_deriveTransparentAccountPrivKeyFromSeed(
-    env: JNIEnv<'_>,
-    _: JClass<'_>,
-    seed: jbyteArray,
-    account: jint,
-    network_id: jint,
-) -> jstring {
-    let res = panic::catch_unwind(|| {
-        let network = parse_network(network_id as u32)?;
-        let seed = env.convert_byte_array(seed).unwrap();
-        let account = if account >= 0 {
-            AccountId::from(account as u32)
-        } else {
-            return Err(format_err!("account argument must be nonnegative"));
-        };
-        // Derive the USK to ensure it exists, and fetch its transparent component.
-        let usk = UnifiedSpendingKey::from_seed(&network, &seed, account)
-            .map_err(|e| format_err!("error generating unified spending key from seed: {:?}", e))?;
-        // Derive the corresponding BIP 32 extended privkey.
-        let xprv = utils::p2pkh_xprv(&network, &seed, account)
-            .expect("USK derivation should ensure this exists");
-        // Verify that we did derive the same privkey.
-        assert_eq!(
-            usk.transparent().to_account_pubkey().serialize(),
-            AccountPrivKey::from_extended_privkey(xprv.extended_key.clone())
-                .to_account_pubkey()
-                .serialize(),
-        );
-        // Encode using the BIP 32 xprv serialization format.
-        let xprv_str: String = xprv.serialize();
-        let output = env
-            .new_string(xprv_str)
-            .expect("Couldn't create Java string for private key!");
-        Ok(output.into_raw())
-    });
-    unwrap_exc_or(&env, res, ptr::null_mut())
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_tool_DerivationTool_deriveTransparentAddressFromSeed(
-    env: JNIEnv<'_>,
-    _: JClass<'_>,
-    seed: jbyteArray,
-    account: jint,
-    index: jint,
-    network_id: jint,
-) -> jstring {
-    let res = panic::catch_unwind(|| {
-        let network = parse_network(network_id as u32)?;
-        let seed = env.convert_byte_array(seed).unwrap();
-        let account = if account >= 0 {
-            account as u32
-        } else {
-            return Err(format_err!("account argument must be nonnegative"));
-        };
-        let index = if index >= 0 {
-            index as u32
-        } else {
-            return Err(format_err!("index argument must be nonnegative"));
-        };
-        let tfvk = UnifiedSpendingKey::from_seed(&network, &seed, AccountId::from(account))
-            .map_err(|e| format_err!("error generating unified spending key from seed: {:?}", e))
-            .map(|usk| usk.transparent().to_account_pubkey())?;
-        let taddr = match utils::p2pkh_addr(tfvk, index) {
-            Ok(taddr) => taddr,
-            Err(e) => return Err(format_err!("Couldn't derive transparent address: {:?}", e)),
-        };
-        let taddr = taddr.encode(&network);
-
-        let output = env
-            .new_string(taddr)
-            .expect("Couldn't create Java string for taddr!");
-        Ok(output.into_raw())
-    });
-    unwrap_exc_or(&env, res, ptr::null_mut())
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_tool_DerivationTool_deriveTransparentAddressFromAccountPrivKey(
-    env: JNIEnv<'_>,
-    _: JClass<'_>,
-    secret_key: JString<'_>,
-    index: jint,
-    network_id: jint,
-) -> jstring {
-    let res = panic::catch_unwind(|| {
-        let network = parse_network(network_id as u32)?;
-        let index = if index >= 0 {
-            index as u32
-        } else {
-            return Err(format_err!("index argument must be nonnegative"));
-        };
-        let xprv_str = utils::java_string_to_rust(&env, secret_key);
-        let xprv = match hdwallet_bitcoin::PrivKey::deserialize(xprv_str) {
-            Ok(xprv) => xprv,
-            Err(e) => return Err(format_err!("Invalid transparent extended privkey: {:?}", e)),
-        };
-        let tfvk = AccountPrivKey::from_extended_privkey(xprv.extended_key).to_account_pubkey();
-        let taddr = match utils::p2pkh_addr(tfvk, index) {
-            Ok(taddr) => taddr,
-            Err(e) => return Err(format_err!("Couldn't derive transparent address: {:?}", e)),
-        };
-        let taddr = taddr.encode(&network);
-
-        let output = env.new_string(taddr).expect("Couldn't create Java string!");
-
-        Ok(output.into_raw())
-    });
-    unwrap_exc_or(&env, res, ptr::null_mut())
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_tool_DerivationTool_deriveTransparentAddressFromPubKey(
-    env: JNIEnv<'_>,
-    _: JClass<'_>,
-    public_key: JString<'_>,
-    network_id: jint,
-) -> jstring {
-    #[allow(deprecated)]
-    let res = panic::catch_unwind(|| {
-        let network = parse_network(network_id as u32)?;
-        let public_key_str = utils::java_string_to_rust(&env, public_key);
-        let pk = PublicKey::from_str(&public_key_str)?;
-        let taddr = pubkey_to_address(&pk).encode(&network);
-
-        let output = env.new_string(taddr).expect("Couldn't create Java string!");
-
-        Ok(output.into_raw())
-    });
-    unwrap_exc_or(&env, res, ptr::null_mut())
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn Java_cash_z_ecc_android_sdk_jni_RustBackend_decryptAndStoreTransaction(
     env: JNIEnv<'_>,

sdk-lib/src/main/rust/utils.rs

@@ -1,4 +1,3 @@
-use hdwallet::{ExtendedPrivKey, KeyChain};
 use jni::{
     descriptors::Desc,
     errors::Result as JNIResult,
@@ -6,14 +5,6 @@ use jni::{
     sys::{jobjectArray, jsize},
     JNIEnv,
 };
-use zcash_primitives::{
-    consensus,
-    legacy::{
-        keys::{AccountPubKey, IncomingViewingKey},
-        TransparentAddress,
-    },
-    zip32::AccountId,
-};
 
 use std::ops::Deref;
 
@@ -87,27 +78,3 @@ where
 //    }
 //    outer
 //}
-
-pub(crate) fn p2pkh_xprv<P: consensus::Parameters>(
-    params: &P,
-    seed: &[u8],
-    account: AccountId,
-) -> Result<hdwallet_bitcoin::PrivKey, hdwallet::error::Error> {
-    let master_key = ExtendedPrivKey::with_seed(&seed)?;
-    let key_chain = hdwallet::DefaultKeyChain::new(master_key);
-    let chain_path = format!("m/44H/{}H/{}H", params.coin_type(), u32::from(account)).into();
-    let (extended_key, derivation) = key_chain.derive_private_key(chain_path)?;
-    Ok(hdwallet_bitcoin::PrivKey {
-        network: hdwallet_bitcoin::Network::MainNet,
-        derivation,
-        extended_key,
-    })
-}
-
-pub(crate) fn p2pkh_addr(
-    tfvk: AccountPubKey,
-    index: u32,
-) -> Result<TransparentAddress, hdwallet::error::Error> {
-    tfvk.derive_external_ivk()
-        .and_then(|tivk| tivk.derive_address(index))
-}