Skip to content

Add migrate_zcashd_wallet command.#152

Merged
nuttycom merged 15 commits into
zcash:mainfrom
nuttycom:feature/zcashd_wallet_import
Sep 15, 2025
Merged

Add migrate_zcashd_wallet command.#152
nuttycom merged 15 commits into
zcash:mainfrom
nuttycom:feature/zcashd_wallet_import

Conversation

@nuttycom
Copy link
Copy Markdown
Contributor

@nuttycom nuttycom commented May 22, 2025

This adds support for importing the secret key material from a zcashd wallet.dat file into zallet.

It does not at present support import of wallet history; a wallet initialized using this command will perform recovery via scanning the chain.

Depends on zcash/librustzcash#1768

Comment thread zallet/src/components/json_rpc/methods/list_addresses.rs
@nuttycom nuttycom marked this pull request as draft May 22, 2025 23:19
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch 7 times, most recently from 099475d to 86d747e Compare May 23, 2025 00:58
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch 4 times, most recently from e2d4ef6 to 17df195 Compare June 6, 2025 21:44
@nuttycom nuttycom marked this pull request as ready for review June 6, 2025 22:00
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from 2b4d0c7 to 7b2b817 Compare June 9, 2025 20:42
@nuttycom nuttycom marked this pull request as draft June 20, 2025 19:58
@nuttycom nuttycom mentioned this pull request Jun 20, 2025
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch 5 times, most recently from b55f231 to b565c41 Compare June 27, 2025 18:41
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from b565c41 to fa843a2 Compare July 31, 2025 01:38
@oxarbitrage oxarbitrage mentioned this pull request Aug 19, 2025
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from fa843a2 to 619d100 Compare August 22, 2025 22:08
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch 3 times, most recently from 7df8b76 to 4c547c3 Compare September 6, 2025 04:33
nuttycom and others added 6 commits September 12, 2025 16:16
Co-authored-by: Jack Grigg <thestr4d@gmail.com>
The primary purpose of the asynchronous operation of `z_sendmany` is to
avoid long proving times in a synchronous call. We don't have any reason
to believe that proposal construction should be a long-running process,
and it's better to report errors to the user synchronously rather than
async, so this change moves all of proposal construction into the
synchronous phase of the RPC call.
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from 91040f5 to c013767 Compare September 12, 2025 22:16
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from 1da8505 to cb94bf3 Compare September 12, 2025 23:11
@nuttycom
Copy link
Copy Markdown
Contributor Author

diff from prior review

@nuttycom nuttycom requested a review from str4d September 12, 2025 23:14
Copy link
Copy Markdown
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK cb94bf3 with nits, but blocked on knowing what dependency version we are depending on.

Comment thread Cargo.toml
zcash_protocol = { git = "https://github.com/zcash/librustzcash.git", rev = "10caf455e3f52744b5392af226a408b05721f70f" }

zewif = { git = "https://github.com/zcash/zewif.git", rev = "f84f80612813ba00a0a8a9a5f060bd217fa981cc" }
zewif-zcashd = { git = "https://github.com/zcash/zewif-zcashd.git", rev = "02a98d6236e24819904e084180da9ba0f5c9b5d0" }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this commit from? I can't find it anywhere.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zewif commit is the tip of zcash/zewif#1.

The zewif-zcashd commit does not correspond to either the tip of https://github.com/zcash/zewif-zcashd or any PR opened on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zcash/zewif#1 for zewif
https://github.com/zcash/zewif-zcashd/tree/fix/wallettx_sapling_parsing for zewif-zcashd. It's on PR #1 from my fork; I've also now pushed it to the zcash org repo.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not on zcash/zewif-zcashd#1 which is very confusing to me, because I do now see it on that branch.

Comment thread zallet/Cargo.toml Outdated
Comment thread zallet/src/cli.rs Outdated
Comment thread zallet/src/cli.rs Outdated
Comment thread zallet/src/cli.rs Outdated
Comment thread zallet/src/commands.rs Outdated
Comment thread zallet/src/commands.rs Outdated
Comment thread zallet/src/components/json_rpc/methods/view_transaction.rs Outdated
let config = APP.config();

// Start monitoring the chain.
let (chain_view, _chain_indexer_task_handle) = ChainView::new(&config).await?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: if we start any component that fires off a background task, we need to shut it down as well. This particular callsite will be altered by #237 so I will fix this then.


if !account_exists {
db_data.import_account_ufvk(
&format!("zcashd legacy sapling {}", idx),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, but for the record this will be incorrect for imported Sapling keys (which are not associated with any legacy account) until #263 is done.

Co-authored-by: Jack Grigg <thestr4d@gmail.com>
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from f9e3faf to f53923b Compare September 13, 2025 01:05
@nuttycom nuttycom requested a review from str4d September 13, 2025 01:06
Copy link
Copy Markdown
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK f53923b however CI is still failing. The tests now need to have the new features enabled; something like this in zallet/Cargo.toml should work:

[[test]]
name = "cli_tests"
required-features = ["zcashd-import"]

Place this between [package.metadata.deb] and [dependencies] (I always try to follow the section order in The Manifest Format for consistency).

Copy link
Copy Markdown
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK e46fb92

`CmdRunner` uses `cargo run` under the hood, which can rebuild the main
binary. If this runs before `cli_tests` it can alter the features that
the latter test expects to be present.

We also fix the acceptance tests to tolerate the stderr output maybe
containing a rebuild.

Co-authored-by: Jack Grigg <jack@electriccoin.co>
@nuttycom nuttycom force-pushed the feature/zcashd_wallet_import branch from 61700db to bb3e630 Compare September 15, 2025 23:11
@nuttycom nuttycom enabled auto-merge September 15, 2025 23:12
Copy link
Copy Markdown
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-utACK be4d76f

@nuttycom nuttycom merged commit f8f1f6c into zcash:main Sep 15, 2025
20 of 21 checks passed
@nuttycom nuttycom deleted the feature/zcashd_wallet_import branch September 15, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants