rpc: Implement z_importviewingkey for Sapling#431
Conversation
mnm458
changed the title
mnm458
changed the title
Add support for importing Sapling extended full viewing keys via the z_importviewingkey JSON-RPC method. The wallet will track incoming and outgoing transactions for addresses derived from imported keys but will not have spending authority. Supports the "whenkeyisnew", "yes", and "no" rescan options with a configurable start height, matching zcashd's interface. Closes zcash#80
mnm458
force-pushed
the
z_importviewingkey
branch
from
0cd39a8 to
dca4cbe
Compare
nullcopy
left a comment
nullcopy
left a comment
There was a problem hiding this comment.
Thanks for taking this on! I've left a few requests before this can merge.
Also, I think we need an integration test, before we merge this. I've created an issue (zcash/integration-tests#99). It'd be helpful if you wanted to take that on too, but no obligation ofc 😅
Reviewed as of dca4cbe
| #[derive(Clone, Debug, Serialize, Documented, JsonSchema)] | ||
| pub(crate) struct ResultType { | ||
| /// The type of the imported address (always "sapling"). | ||
| address_type: String, |
There was a problem hiding this comment.
| address_type: String, | |
| type: String, |
Match zcashd naming
| /// Validates the `rescan` parameter. | ||
| /// | ||
| /// Returns the validated rescan value, or an RPC error if the value is invalid. | ||
| fn validate_rescan(rescan: Option<&str>) -> RpcResult<&str> { |
There was a problem hiding this comment.
Please create an enum for the rescan value, so that we don't rely on stringly-typed data
| /// commitment trees are empty. For non-zero heights, fetches the real treestate from | ||
| /// the chain indexer so the sync engine can validate note commitment tree continuity. | ||
| #[cfg(zallet_build = "wallet")] | ||
| pub(super) async fn fetch_account_birthday( |
There was a problem hiding this comment.
This is a useful helper. There's a few places in the codebase that should use this helper now:
- get_new_account.rs:58-92
- recover_accounts.rs:85-125
|
|
||
| wallet | ||
| .import_account_ufvk( | ||
| &format!("Imported Sapling viewing key {}", &address[..16]), |
There was a problem hiding this comment.
| &format!("Imported Sapling viewing key {}", &address[..16]), | |
| &format!("Imported Sapling viewing key {}", &address), |
| .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.
This check should only apply if rescan="yes"
| // | ||
| // TODO: When rescan is "yes" and the key already exists, zcashd would force a | ||
| // rescan from start_height. We currently skip this because zcash_client_sqlite | ||
| // does not expose a way to reset scan ranges for an existing account. |
There was a problem hiding this comment.
This TODO is out of date. zcash_client_backend recently added WalletWrite::rewind_to_chain_state which does the rescan you need here. You'll need to update the crate patch in Cargo.toml for zcash_client_backend to point to latest main.
| match existing_account { | ||
| Some(account) => { | ||
| if matches!(account.purpose(), AccountPurpose::Spending { .. }) { | ||
| return Err(LegacyCode::Wallet.with_message(format!( |
There was a problem hiding this comment.
| return Err(LegacyCode::Wallet.with_message(format!( | |
| "The wallet already contains the private key for this viewing key (address: {address})", |
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// - `vkey` (string, required) The viewing key (see `z_exportviewingkey`). |
There was a problem hiding this comment.
| /// - `vkey` (string, required) The viewing key (see `z_exportviewingkey`). | |
| /// - `vkey` (string, required) The viewing key. |
z_exportviewingkey hasn't been implemented yet, so this reference doesn't make sense. IMO just drop the reference for now
2 participants
Summary
Implements
z_importviewingkeyRPC method for Sapling extended full viewing keys (issue #80).Imports a Sapling extfvk (
zxviews/zxviewtestsapling) so the wallet can track transactions without holding spending authority. Accounts are stored withAccountPurpose::ViewOnly.fetch_account_birthdaytojson_rpc::utilsas a shared helper (also needed by Implementz_importkeyandz_exportkeyfor Sapling #400 — whichever merges first provides it).Why implement this despite zero usage in the survey? Watch-only wallets are a fundamental capability for exchanges, auditors, and operators who need to monitor balances without holding spending keys. As zcashd is deprecated, this becomes the only path for users to import existing viewing keys into the new stack. It also supports the separation-of-concerns model where spending keys stay on cold storage while
a hot node tracks balances.
Test plan
cargo build— cleancargo test -p zallet import_viewing_key— 13/13 passingzcashdand Zallet output #16)Close rpc: Implement
z_importviewingkey#80