-
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
Poc microcrate account #972
Conversation
I like it that there are no |
//! By constraining the wrapper to be independent of internal zingolib functionality | ||
//! we expose a shareable component, and refine the definition of zingolib. | ||
|
||
/// The a view-only abstraction that provides a management interface for |
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.
"The a view-only" is a typo? also comment seems to just ...end. I am ok to chalk that up to the TODO if you want
/// TODO: Explain more about our Account abstraction. | ||
pub struct ZingoAccount( | ||
pub zip32::AccountId, | ||
pub zcash_keys::keys::UnifiedFullViewingKey, |
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 dont think we should make fields pub unless there is a really good reason? is there one?
the id
and ufvk
methods are private though which is surprising. Have you looked at how this Account trait is used in LRZ?
I think we should probably have a crate for many "primitive" wallet objects like account. I think one crate for just this one struct will end up with a silly amount of crates. Or we just have the struct in zingolib. |
This is an experiment. I don't have a great idea for what the tradeoffs might be, but all other things being equal, I think more encapsulated is better.