From 49616ba8349c8de845ea0a6f03b297f0c0fa202d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Honza=20Rychnovsk=C3=BD?= Date: Fri, 13 Dec 2024 12:03:42 +0100 Subject: [PATCH] Resolve [#1640] review comments * `Account` documentation fix * Make `AccountPurpose.Spending` args not null As they now live under `AccountPurpose.Spending` where we need them not null + `AccountImportSetup` doc fix * Replace `bytes` with `usk` in `AccountUsk` * Change test name --- .../sdk/internal/TypesafeBackendImpl.kt | 2 +- .../cash/z/ecc/android/sdk/model/Account.kt | 2 +- .../android/sdk/model/AccountImportSetup.kt | 3 +- .../z/ecc/android/sdk/model/AccountPurpose.kt | 4 +- .../z/ecc/android/sdk/model/AccountUsk.kt | 51 ++----------------- .../z/ecc/android/sdk/model/AccountTest.kt | 2 +- 6 files changed, 11 insertions(+), 53 deletions(-) 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 cd13e4719..a5082c6a4 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 @@ -69,7 +69,7 @@ internal class TypesafeBackendImpl(private val backend: Backend) : TypesafeBacke treeState = treeState.encoded, ufvk = setup.ufvk.encoding, seedFingerprint = setup.purpose.seedFingerprint, - zip32AccountIndex = setup.purpose.zip32AccountIndex?.index, + zip32AccountIndex = setup.purpose.zip32AccountIndex.index, ) AccountPurpose.ViewOnly -> backend.importAccountUfvk( diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/Account.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/Account.kt index 6b3466c0a..5b085935b 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/Account.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/Account.kt @@ -28,7 +28,7 @@ import cash.z.ecc.android.sdk.internal.model.JniAccount * @param keySource A string identifier or other metadata describing the location of the spending * key corresponding to the account's UFVK. This is set internally by the wallet app based on * its private enumeration of spending methods it supports. - * @param seedFingerprint The seed fingerprint. Must be length 16. + * @param seedFingerprint The seed fingerprint. Must be length 32. * @param hdAccountIndex ZIP 32 account index */ data class Account internal constructor( diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountImportSetup.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountImportSetup.kt index 5709152d3..7470e3fc1 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountImportSetup.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountImportSetup.kt @@ -1,8 +1,7 @@ package cash.z.ecc.android.sdk.model /** - * Wrapper for the import account API based on viewing key. Set both [seedFingerprint] and [zip32AccountIndex] null - * when using [AccountPurpose.ViewOnly]. + * Wrapper for the import account API based on viewing key. * * @param accountName A human-readable name for the account. This will be visible to the wallet * user, and the wallet app may obtain it from them. diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountPurpose.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountPurpose.kt index e0611679d..3f9a71704 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountPurpose.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountPurpose.kt @@ -16,8 +16,8 @@ sealed class AccountPurpose { * @param zip32AccountIndex The ZIP 32 account-level component of the HD derivation path at which to derive the */ data class Spending( - val seedFingerprint: ByteArray?, - val zip32AccountIndex: Zip32AccountIndex?, + val seedFingerprint: ByteArray, + val zip32AccountIndex: Zip32AccountIndex, ) : AccountPurpose() { override val value = 0 } diff --git a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountUsk.kt b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountUsk.kt index c00d03c95..d4e7558bb 100644 --- a/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountUsk.kt +++ b/sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountUsk.kt @@ -1,6 +1,5 @@ package cash.z.ecc.android.sdk.model -import cash.z.ecc.android.sdk.internal.jni.RustBackend import cash.z.ecc.android.sdk.internal.model.JniAccountUsk /** @@ -18,57 +17,17 @@ class AccountUsk private constructor( */ val accountUuid: AccountUuid, /** - * The binary encoding of the [ZIP 316](https://zips.z.cash/zip-0316) Unified Spending - * Key for [accountUuid]. - * - * This encoding **MUST NOT** be exposed to users. It is an internal encoding that is - * inherently unstable, and only intended to be passed between the SDK and the storage - * backend. Wallets **MUST NOT** allow this encoding to be exported or imported. + * The [ZIP 316](https://zips.z.cash/zip-0316) Unified Spending Key. */ - private val bytes: FirstClassByteArray + private val usk: UnifiedSpendingKey ) { - internal constructor(uskJni: JniAccountUsk) : this( - AccountUuid.new(uskJni.accountUuid), - FirstClassByteArray(uskJni.bytes.copyOf()) - ) - - /** - * The binary encoding of the [ZIP 316](https://zips.z.cash/zip-0316) Unified Spending - * Key for [accountUuid]. - * - * This encoding **MUST NOT** be exposed to users. It is an internal encoding that is - * inherently unstable, and only intended to be passed between the SDK and the storage - * backend. Wallets **MUST NOT** allow this encoding to be exported or imported. - */ - fun copyBytes() = bytes.byteArray.copyOf() - - // Override to prevent leaking key to logs - override fun toString() = "AccountUsk(account=$accountUuid, bytes=***)" + override fun toString() = "AccountUsk(account=$accountUuid, usk=$usk)" companion object { - /** - * This method may fail if the [bytes] no longer represent a valid key. A key could become invalid due to - * network upgrades or other internal changes. If a non-successful result is returned, clients are expected - * to use [DerivationTool.deriveUnifiedSpendingKey] to regenerate the key from the seed. - * - * @return A validated AccountUsk. - */ - suspend fun new( - accountUuid: AccountUuid, - bytes: ByteArray - ): Result { - val bytesCopy = bytes.copyOf() - RustBackend.loadLibrary() - return runCatching { - require(RustBackend.validateUnifiedSpendingKey(bytesCopy)) - AccountUsk(accountUuid, FirstClassByteArray(bytesCopy)) - } - } - - fun new(jniAccountUsk: JniAccountUsk): AccountUsk = + suspend fun new(jniAccountUsk: JniAccountUsk): AccountUsk = AccountUsk( accountUuid = AccountUuid.new(jniAccountUsk.accountUuid), - bytes = FirstClassByteArray(jniAccountUsk.bytes) + usk = UnifiedSpendingKey.new(jniAccountUsk.bytes) ) } } diff --git a/sdk-lib/src/test/java/cash/z/ecc/android/sdk/model/AccountTest.kt b/sdk-lib/src/test/java/cash/z/ecc/android/sdk/model/AccountTest.kt index ff171796e..5feb163c3 100644 --- a/sdk-lib/src/test/java/cash/z/ecc/android/sdk/model/AccountTest.kt +++ b/sdk-lib/src/test/java/cash/z/ecc/android/sdk/model/AccountTest.kt @@ -7,7 +7,7 @@ import kotlin.test.assertFailsWith class AccountTest { @Test - fun out_of_bounds() { + fun uuid_wrong_length() { assertFailsWith(IllegalArgumentException::class) { AccountFixture.new(accountUuid = UUID.fromString("random")) }