-
Notifications
You must be signed in to change notification settings - Fork 159
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
ZIP 400: Update wallet schema #968
base: main
Are you sure you want to change the base?
Conversation
zips/zip-0400.rst
Outdated
|
||
"acc*", "Account data", "1. ``string strAccount``", "1. ``CAccount account``" | ||
"acentry*", "Track internal transfers between accounts in the same wallet", "1. ``string strAccount`` | ||
"3..4", "acc*", "Account data", "1. ``string strAccount``", "1. ``CAccount account``" |
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.
If the end version is less than 6:
- Check this is actually correct.
- If it is, describe how later versions handle/migrate the 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.
Looks like both acc
and acentry
, which were part of the 'account' API, were disabled since the Zcash launch, and removed in v4.5.0: https://github.com/zcash/zcash/releases/tag/v4.5.0.
Here is also the Bitcoin Core release it references: https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.17.0.md#label-and-account-apis-for-wallet.
Should we add the fact that both were deprecated/disabled, and that they have no replacement?
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.
utACK with comments.
Co-authored-by: Daira-Emma Hopwood <[email protected]>
…r `chdseed` & `hdseed` and add instancing details.
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.
Kudos @dorianvp this looks very good.
"3..4", "acc*", "Account data. This entry is used in Bitcoin and not in Zcash.", "1. ``string strAccount``", "1. ``CAccount account``" | ||
"3..4", "acentry*", "Track internal transfers between accounts in the same wallet. This entry is used in Bitcoin and not in Zcash.", "1. ``string strAccount`` | ||
2. ``uint64_t nNumber``", "1. ``CAccountingEntry acentry``" |
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.
Does this mean that these fields will never be present in a Zcash walletdb? In that case just delete them.
"3..4", "acc*", "Account data. This entry is used in Bitcoin and not in Zcash.", "1. ``string strAccount``", "1. ``CAccount account``" | |
"3..4", "acentry*", "Track internal transfers between accounts in the same wallet. This entry is used in Bitcoin and not in Zcash.", "1. ``string strAccount`` | |
2. ``uint64_t nNumber``", "1. ``CAccountingEntry acentry``" |
"3..6", "vkey*", "Sprout Viewing Keys.", "1. ``libzcash::SproutViewingKey vk``", "1. ``char fYes``" | ||
"3..6", "watchs*", "Watch-only t-addresses.", "1. ``CScript script``", "1. ``char fYes``" | ||
"3..6", "**witnesscachesize**", "Shielded Note Witness cache size.", "", "1. ``int64_t nWitnessCacheSize``" | ||
"3..6", "wkey*", "Wallet 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.
What is the type of this?
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.
Lines 12-17: MUST and MAY are not used (and don't appear to be needed). Just delete the Terminology section if there are no other terms to define. Also delete [#BCP14]
from the References section.
zips/zip-0400.rst
Outdated
"3..6", "pool*", "", "1. ``int64_t nIndex``", "1. ``CKeyPool keypool``" | ||
"3..6", "purpose*", "Short description or identifier of an address.", "1. ``string strAddress``", "1. ``string strPurpose``" | ||
"3..6", "sapzaddr*", "Sapling z-addr Incoming Viewing key and address.", "1. ``libzcash::SaplingPaymentAddress addr``", "1. ``libzcash::SaplingIncomingViewingKey ivk``" | ||
"3..6", "sapextfvk*", "Sapling Extended Full Viewing 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.
What is the type of this?
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.
Updated! However, due to the length of the type, a horizontal scrollbar appeared as the table became wider.
zips/zip-0400.rst
Outdated
"3..4", "hdseed*", "Hierarchical Deterministic seed. [#zip-0032]_ Replaced by ``mnemonicphrase`` in later versions.", "1. ``uint256 seedFp``", "1. ``RawHDSeed rawSeed``" | ||
"3..6", "key*", "Transparent pubkey and privkey.", "1. ``CPubKey vchPubKey``", "1. ``CPrivKey pkey``" | ||
"3..6", "keymeta*", "Transparent key metadata.", "1. ``CPubKey vchPubKey``", "1. ``CKeyMetadata keyMeta``" | ||
"3..6", "**minversion**", "Wallet required minimal version.", "", "" |
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.
What is the type of this?
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.
ACK with suggestions.
Co-authored-by: Daira-Emma Hopwood <[email protected]>
Co-authored-by: Daira-Emma Hopwood <[email protected]>
Co-authored-by: Daira-Emma Hopwood <[email protected]>
Co-authored-by: Daira-Emma Hopwood <[email protected]>
Ref: #964. This PR updates the wallet schema to incorporate all changes across different
zcashd
versions.