Add derivation methods for ZIP 325 private use metadata keys

[str4d]

Note
This code review checklist is intended to serve as a starting point for the author and reviewer, although it may not be appropriate for all types of changes (e.g. fixing a spelling typo in documentation). For more in-depth discussion of how we think about code review, please see Code Review Guidelines.

Author

• Self-review your own code in GitHub's web interface¹
• Add automated tests as appropriate
• Update the manual tests² as appropriate
• Check the code coverage³ report for the automated tests
• Update documentation as appropriate (e.g README.md, Architecture.md, etc.)
• Run the demo app and try the changes
• Pull in the latest changes from the main branch and squash your commits before assigning a reviewer⁴

Reviewer

• Check the code with the Code Review Guidelines checklist
• Perform an ad hoc review⁵
• Review the automated tests
• Review the manual tests
• Review the documentation, README.md, Architecture.md, etc. as appropriate
• Run the demo app and try the changes⁶

Footnotes

1. Code often looks different when reviewing the diff in a browser, making it easier to spot potential bugs. ↩

2. While we aim for automated testing of the SDK, some aspects require manual testing. If you had to manually test
   something during development of this pull request, write those steps down. ↩

3. While we are not looking for perfect coverage, the tool can point out potential cases that have been missed. Code coverage can be generated with: ./gradlew check for Kotlin modules and ./gradlew connectedCheck -PIS_ANDROID_INSTRUMENTATION_TEST_COVERAGE_ENABLED=true for Android modules. ↩

4. Having your code up to date and squashed will make it easier for others to review. Use best judgement when squashing commits, as some changes (such as refactoring) might be easier to review as a separate commit. ↩

5. In addition to a first pass using the code review guidelines, do a second pass using your best judgement and experience which may identify additional questions or comments. Research shows that code review is most effective when done in multiple passes, where reviewers look for different things through each pass. ↩

6. While the CI server runs the demo app to look for build failures or crashes, humans running the demo app are
   more likely to notice unexpected log messages, UI inconsistencies, or bad output data. Perform this step last, after verifying the code changes are safe to run locally. ↩

[daira]

utACK with non-blocking comments.

[str4d]

Force-pushed to remove the compilation hotfix commit that created another JNI mismatch. Instead I'm going to fix #1684 and #1657 in a separate PR, then rebase this on top of it.

[str4d]

Rebased on 1687 to fix JNI issues and address comments.

[daira]

fn registered_secret_key_from_jni(
    env: &JNIEnv,
    key_sk: JByteArray,
    key_c: JByteArray,
) -> anyhow::Result<zip32::registered::SecretKey> {
    let sk = utils::java_bytes_to_rust(env, &key_sk)?
        .as_slice()
        .try_into()
        .map_err(|_| anyhow!("Incorrect length for key_sk"))?;
    let c = utils::java_bytes_to_rust(env, &key_c)?
        .as_slice()
        .try_into()
        .map_err(|_| anyhow!("Incorrect length for key_c"))?;

    Ok(zip32::registered::SecretKey::from_parts(sk, ChainCode::new(c)))
}

Suggested change

	let account_metadata_key_sk = utils::java_bytes_to_rust(env, &account_metadata_key_sk)?;
	let account_metadata_key_c = utils::java_bytes_to_rust(env, &account_metadata_key_c)?;
	let ufvk_string = utils::java_nullable_string_to_rust(env, &ufvk_string)?;
	let private_use_subject = utils::java_bytes_to_rust(env, &private_use_subject)?;
	let network = parse_network(network_id as u32)?;
	let account_metadata_key = {
	let sk = account_metadata_key_sk
	.as_slice()
	.try_into()
	.map_err(|_| anyhow!("Incorrect length for account_metadata_key_sk"))?;
	let chain_code = ChainCode::new(
	account_metadata_key_c
	.as_slice()
	.try_into()
	.map_err(|_| anyhow!("Incorrect length for account_metadata_key_c"))?,
	);
	zip32::registered::SecretKey::from_parts(sk, chain_code)
	};
	let account_metadata_key = registered_secret_key_from_jni(env, account_metadata_key_sk, account_metadata_key_c)?;
	let ufvk_string = utils::java_nullable_string_to_rust(env, &ufvk_string)?;
	let private_use_subject = utils::java_bytes_to_rust(env, &private_use_subject)?;
	let network = parse_network(network_id as u32)?;

[daira]

This file is useless cruft. The typesafe mapping is already done in DerivationToolExt.

In a follow-up PR, delete Typesafe from the method names in Derivation, then delete this file and the only use of it in DerivationTool, which can then just use:

private val instance = SuspendingLazy<Unit, DerivationTool> { RustDerivationTool.new() }

[daira]

Suggested change

	*
	*
	* This method should only be used via `model.AccountMetadataKey` (#1685).
	*

[daira]

utACK with comments.

[str4d]

Rebased on rust-updates after crate releases.

[str4d]

The rest of the review comments I'm not addressing (either because they aren't relevant to this PR, or because I think the current abstraction is fine).

[daira]

utACK modulo ktlint failures, which should be fixed. (Why don't they block merges?)

[str4d]

I cannot fix the ktlint failure, as doing so creates a detekt lint failure.

[HonzaR]

Rebased on the rust-updates branch to fix the ktlint warning.

[HonzaR]

Left one non-blocking comment inline.

[HonzaR]

Should this page exist already, or is it a pre-existing ZIP? I get 404 on this URL.

[str4d]

That URL will exist once zcash/zips#979 is finished and merged.