Account APIs needed for the mobile SDKs #1267
Conversation
|
The changes to |
| /// An incoming viewing key. | ||
| /// | ||
| /// Accounts that have this kind of viewing key cannot be used in wallet contexts, | ||
| /// because they are unable to maintain an accurate balance. | ||
| Incoming(Uivk), |
There was a problem hiding this comment.
Note that this will change in #1245
There was a problem hiding this comment.
Yep that's fine; this is still a private enum.
zcash_client_sqlite/src/wallet.rs
Outdated
| kind: AccountType, | ||
| viewing_key: ViewingKey, |
There was a problem hiding this comment.
Hm, doing it this way means that the (Derived, IVK) combination is supported, which doesn't make much sense.
There was a problem hiding this comment.
It's not an invalid combination: one could import a UIVK for which you know its derivation pathway, in order to make future identification of the corresponding UFVK and USK easier.
There was a problem hiding this comment.
Yeah, I decided that that would be a reasonable use case, hence "this is fine."
There was a problem hiding this comment.
What this boils down to is that the thing that makes an account "usable as a wallet" is that it has a UFVK (and can track balance), not whether we know the derivation path; as long as the caller can provide the corresponding USK when needed, that's all that matters.
zcash_client_sqlite/src/wallet.rs
Outdated
| SqliteClientError::CorruptedData("Unrecognized account_type".to_string()) | ||
| })?; | ||
| let kind = parse_account_kind( | ||
| row.get("account_type")?, |
There was a problem hiding this comment.
I didn't realize, somehow, that named column access was supported.
There was a problem hiding this comment.
That was introduced in #1235 and I don't quite get how it is implemented (or when support was added to rusqlite). It might be nice to do a pass over the crate to use that instead of column indexing, if it is not inefficient, but for now I've stuck with whatever style the local code used in each place.
zcash_client_sqlite/src/wallet.rs
Outdated
| kind: AccountType, | ||
| viewing_key: ViewingKey, |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1267 +/- ##
==========================================
- Coverage 63.55% 63.54% -0.01%
==========================================
Files 121 121
Lines 13652 13661 +9
==========================================
+ Hits 8676 8681 +5
- Misses 4976 4980 +4 ☔ View full report in Codecov by Sentry. |
|
Force-pushed to address review comments. |
This exposes the kind of account (derived or imported, from which the ZIP 32 account index can be obtained), and enables the
WalletRead::Accounttype to be looked up viaWalletRead::AccountIdinstead of solely via the derivation parameters or UFVK.