Implement z_importkey and z_exportkey for Sapling#400
Conversation
Adds two new RPC methods:
- `z_importkey`: Imports a Sapling extended spending key into the wallet.
The key is encrypted with age and stored in the keystore. A UFVK is
derived and imported into the wallet database so the sync engine can
track transactions. The rescan parameter controls whether historical
blocks are scanned ("yes", "no", or "whenkeyisnew"), and start_height
sets the birthday for the imported account. Real treestates are fetched
from the chain indexer for non-zero start heights.
- `z_exportkey`: Exports the spending key for a Sapling payment address.
Requires the wallet to be unlocked. Looks up the key by iterating
standalone sapling DFVKs and using decrypt_diversifier to match the
address.
Also adds `fetch_account_birthday` to `json_rpc::utils` as a shared
helper for building an AccountBirthday from a chain treestate.
Includes unit tests for parameter validation, key encoding/decoding
roundtrips, and address parsing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
oxarbitrage
force-pushed
the
z_importexportkey
branch
from
67e0b34 to
532371e
Compare
|
I'm not sure that it makes sense to implement the legacy versions of these methods; their utility is extremely limited and I would prefer that we avoid introducing API surface that we then have to maintain. Both of these methods got zero points in our survey of what people are using: https://docs.google.com/spreadsheets/d/1UJxH1cowexGqadU32Uei5Qak6jGhXjb18-T_QBPmDAA/edit?gid=0#gid=0 |
|
I guess that I can see implementing |
|
Thanks for the feedback.
I wasn’t aware of the survey, thanks for sharing it. I’ll consider it for future work. It might also make sense to close or re-evaluate issues in the repository that correspond to methods with zero usage. That said, I do think import-related APIs may become more valuable as zcashd is deprecated and users look for ways to migrate into the new stack.
Not only for paper wallets. If you have a legacy Sapling key (with funds) and want to import it into the new stack without relying on
That makes sense. The export functionality was mainly added for completeness. I’m happy to remove it, or alternatively keep it with clearer documentation highlighting the risks. Let me know how you’d prefer to proceed. Happy to adjust the PR accordingly. |
|
By the way, I should have provided more context earlier. The main motivation here was recovering legacy funds that were otherwise inaccessible. I had the mnemonic, but none of the available tools worked for recovery (zeccavator, zodl/zashi, and ywallet all failed in this case). The only viable path was to derive the Sapling key from the mnemonic (I wrote a small tool for that: https://github.com/oxarbitrage/zcash-sapling-derive) and then import it into zcashd using That worked for me, but it made me realize that this recovery path would no longer be available once zcashd is deprecated. That’s what motivated porting these calls into zallet. |
|
I suggest we add the import key logic here, so zcashd folks can migrate |
| .get_treestate(treestate_height.to_string()) | ||
| .await | ||
| .map_err(|e| { | ||
| LegacyCode::InvalidParameter.with_message(format!( |
There was a problem hiding this comment.
s/LegacyCode::InvalidParameter/RpcErrorCode::InternalError/
| .map_err(|e| LegacyCode::Database.with_message(e.to_string()))?; | ||
|
|
||
| if let Some(tip) = chain_tip { | ||
| if start_height > tip { |
There was a problem hiding this comment.
(not blocking) might be nice to skip this check if rescan="no".
| // - "no" → use the current chain tip so the sync engine only tracks new | ||
| // transactions going forward. | ||
| // | ||
| // TODO: When rescan is "yes" and the key already exists, zcashd would force a |
There was a problem hiding this comment.
The recently added rewind_to_height() method should do this for you now
| // does not expose a way to reset scan ranges for an existing account. | ||
| let effective_height = match rescan { | ||
| "yes" | "whenkeyisnew" => start_height, | ||
| "no" => chain_tip.unwrap_or(BlockHeight::from_u32(0)), |
There was a problem hiding this comment.
Not sure how I feel about silently failing over to 0 here
There was a problem hiding this comment.
Thinking on it some more, I'm actually ok with it. I would support logging a warning here, though (not blocking)
| ) -> RpcResult<AccountBirthday> { | ||
| if height == BlockHeight::from_u32(0) { | ||
| return Ok(AccountBirthday::from_parts( | ||
| ChainState::empty(BlockHeight::from_u32(0), BlockHash([0; 32])), |
There was a problem hiding this comment.
Should this at least include the real genesis hash?
4 participants
Adds two new RPC methods:
z_importkey: Imports a Sapling extended spending key into the wallet. The key is encrypted with age and stored in the keystore. A UFVK is derived and imported into the wallet database so the sync engine can track transactions. The rescan parameter controls whether historical blocks are scanned ("yes", "no", or "whenkeyisnew"), and start_height sets the birthday for the imported account. Real treestates are fetched from the chain indexer for non-zero start heights.z_exportkey: Exports the spending key for a Sapling payment address. Requires the wallet to be unlocked. Looks up the key by iterating standalone sapling DFVKs and using decrypt_diversifier to match the address.Also adds
fetch_account_birthdaytojson_rpc::utilsas a shared helper for building an AccountBirthday from a chain treestate.Includes unit tests for parameter validation, key encoding/decoding roundtrips, and address parsing.
Close #69
Close #79