Skip to content

Commit

Permalink
Resolve [#1640] review comments
Browse files Browse the repository at this point in the history
* `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
  • Loading branch information
HonzaR committed Dec 13, 2024
1 parent 62456ac commit 49616ba
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
51 changes: 5 additions & 46 deletions sdk-lib/src/main/java/cash/z/ecc/android/sdk/model/AccountUsk.kt
Original file line number Diff line number Diff line change
@@ -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

/**
Expand All @@ -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<AccountUsk> {
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)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down

0 comments on commit 49616ba

Please sign in to comment.