[Wallet] Sapling keys management final round. #1884
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All of these maps are created from scratch on wallet load, so we can alter their contents without compatibility concerns. With this change, we can use the existing maps to implement viewing key support. The downside is that the wallet will take more space in memory, but that can easily be improved in future by storing ExtFVK fingerprints in some of the maps (which would improve memory usage even compared to the original layout). Coming from zcash@d74fdd76eb1707012beed1eba1e9ab83cf6ea5b5
Now that we store SaplingExtendedFullViewingKey internally, we have access to the default address everywhere we require it. Coming from zcash@b62bb98087a83d8bb9c1ff7669d34b9ca9adbaf4
Coming from zcash@7de06244e6a874cd468bad021b7537d5909c1b71
furszy
force-pushed
the
2020_sapling_wallet_keys
branch
2 times, most recently
from
fd0033b to
de5872d
Compare
Fuzzbawls
changed the title
Author
|
travis job solved, ready for review. |
furszy
requested review from
Fuzzbawls and
random-zebra
and removed request for
Fuzzbawls and
random-zebra
Fuzzbawls
reviewed
Oct 8, 2020
Collaborator
Fuzzbawls
left a comment
Fuzzbawls
left a comment
There was a problem hiding this comment.
noticed some duplicate file output in dumpwallet, but otherwise looking good
furszy
force-pushed
the
2020_sapling_wallet_keys
branch
from
de5872d to
1b63091
Compare
Author
|
updated per feedback. |
Fuzzbawls
previously approved these changes
Oct 8, 2020
Collaborator
Fuzzbawls
left a comment
Fuzzbawls
left a comment
There was a problem hiding this comment.
ACK 1b630912ee5e8311d5f73fb1978ec5b2a2cc0d49
random-zebra
reviewed
Oct 9, 2020
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
Pretty good 🥃
Just few minor points
furszy
force-pushed
the
2020_sapling_wallet_keys
branch
from
1b63091 to
5cb4144
Compare
Author
|
Updated per feedback. |
random-zebra
reviewed
Oct 9, 2020
furszy
force-pushed
the
2020_sapling_wallet_keys
branch
from
5cb4144 to
9bfa1d6
Compare
Author
|
updated per feedback. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Decoupled from #1798, purely focused on the wallet's Sapling keys area. On four main aspects:
Sapling keys management refactored to support viewing key functionality.
New RPC commands implemented:
exportsaplingkey,importsaplingkey,importsaplingviewingkeyandexportsaplingviewingkey.RPC commands expanded adding sapling keys support capabilities:
dumpwalletandimportwallet.Implemented unit test coverage for:
getnewshieldedaddresson an encrypted and unencrypted wallet,listsaplingaddresses,importsaplingkey,exportsaplingkeyvia rpc calls.Extra data:
Further down 1798 road, we will get even more testing coverage for this features like #1798/c5d4ffe3, plus a test coverage expansion for Sapling keys in
importwalletanddumpwallet. (cannot add them here because they are depending on the shielded_spend functionality that will treat in another decouplement PR round).The list of decoupled commits from #1798 is the following one:
SaplingFullViewingKey -> SaplingExtendedFullViewingKey in keystore maps —> a98a0bee9dd592ad6ed95ca2f1afd96bb096de81
Remove default address parameter from Sapling keystore methods —> 96d30a7948b9a92c603fe43a755c045be992691a
test: Add test for CBasicKeyStore::AddSaplingFullViewingKey —> 21f683c1c189ab749d5627894dedb4259eca3a3d
Add encoding and decoding for Sapling extended full viewing keys. —> 7da9de34d51c552f2e5205b3953defa7555f34ec
[Wallet] Do not add a sapling key if the sspkm is not enabled. —> 9ad9572c84c37a7b7945a12dc1fd667051dd59e8
GetSpendingKeyForPaymentAddress method implemented —> f1d664609ee3b507ea067ec4ce05e1716cd51be7
RPC: Implement exportsaplingkey —> 2eb2a3a0202a4182e75ab687f0f40b9dea351228
wallet: AddSpendingKeyToWallet method implemented. —> e66acfd9c5db5f92f00b4bdb4278e4f4164a81fd
adding exportsaplingkey command to the rpcwallet table —> 368c0d063d6f121a0a4d8000b5f238164836f366
RPC: importsaplingkey command implemented. —> 3e4b1baf63545cc89766c9a2b48db19ae484a14e
dumpwallet including sapling keys —> 30336c14ec161e41be3a944fdbaa0c57f67edcb5
importwallet including sapling keys —> 519b78c866663df099e8621ae6b9d165af7805d1
importsaplingviewingkey RPC command implemented. —> 4323b2d1f38ecd40bcff98e2351b642019abb131
RPC: exportsaplingviewingkey command back ported —> a38da6aa6d4c2d2f79fe06ecb9153fc3e8acc269
Sapling logging type added. —> b301ff667c30511ef9fc8957b1b66acbaa8f2b77
rpc_wallet_getnewsaplingaddress test implemented. —> 102c760639f814e0bc0bb5b3fbcc838e694ef72b
rpc_wallet_encrypted_wallet_sapzkeys test included. —> d46183666f524fb69f0d359c52266a75e46b3e6d
Test: sapling wallet import/export keys implemented. —> 8792587447b2e9bdf4a302b0e8647edccc305444
importsaplingkey RPC command unit test —> 67f79937bb4ddb2f04e2924ad7a0d996cf9c2732