-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initialize wallet with unified spending key #459
Conversation
f0108a7
to
7c8f879
Compare
7c8f879
to
1de0df1
Compare
let usk = UnifiedSpendingKey::from_bytes(Era::Orchard, usk) | ||
.map_err(|_| "Error decoding unified spending key.")?; | ||
|
||
// Workaround https://github.com/zcash/librustzcash/issues/929 by serializing and deserializing the transparent key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a bit of digging, and it turns out the TransparentPrivKey
type being used here is actually a zingolib type, albeit a redefinition/copy-paste of the hdwallet
type with a bunch of additional methods....I think it would make more sense to use the hdwallet
type (either sacrifice a little bit of method call syntax and define local helpers instead of being method calls, or just define a local trait with the additional functionality in order to keep the method syntax) to avoid the surprising amount of duplicated code I just discovered, but that's not a blocking concern and definitely beyond the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's the hdwallet
type defined? Why do we have to copy paste?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC it was because the struct we might have reused carried private fields, and exposed them only in a limited way.
But as I think @AloeareV was alluding to, I may have even been looking at a similarly named, but incompatible type anyway, so maybe this was all necessary. I'd have to go back and check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks great.
This unlocks the ability for importing a wallet across FFI that has spending keys, but for which the mnemonic is not known, or perhaps the account index is not the 0 presumed by
WalletBase
when given a mnemonic.