From c797ed21c51d2aeb18b54cb0b35da859ba0cb84e Mon Sep 17 00:00:00 2001 From: zancas Date: Tue, 7 Jul 2026 20:16:30 -0700 Subject: [PATCH] feat!: define the Wallet abstraction the harness actuates generically Rename the client layer to the Wallet abstraction: the client module becomes wallet, the Client and ClientConfig traits become Wallet and WalletConfig, and ClientError becomes WalletError, whose messages now name the operation rather than one binary. The trait is the interface through which the harness drives any wallet implementation, and implementations live with their binaries: the zcash-devtool one remains in-tree for now, and a zingo-cli implementation lands in the zingolib repository against zingolabs/zingolib#2419 (the request spec is added as zingolib-wallet-impl-spec.md; nothing implements the second wallet here). Add LocalNet::launch_wallet::(make_config), the generic actuation path: it mints the WalletNetwork from the running Validator, wires the Indexer connection, and launches whichever implementation the caller names. The integration tests' helper now routes through it. Add ValidatorHeights::activation_heights(), a read-only accessor, because a foreign implementation must serialize the reported schedule into its own binary's configuration; construction remains private, so the ADR 0003 provenance guarantee is untouched. CONTEXT.md gains the Wallet glossary term. Co-Authored-By: Claude Fable 5 --- CONTEXT.md | 8 ++ zcash_local_net/CHANGELOG.md | 18 ++- zcash_local_net/src/error.rs | 13 ++- zcash_local_net/src/lib.rs | 21 +++- zcash_local_net/src/{client.rs => wallet.rs} | 103 ++++++++++-------- .../src/{client => wallet}/zcash_devtool.rs | 64 +++++------ zcash_local_net/tests/integration.rs | 33 +++--- zingolib-wallet-impl-spec.md | 95 ++++++++++++++++ 8 files changed, 251 insertions(+), 104 deletions(-) rename zcash_local_net/src/{client.rs => wallet.rs} (70%) rename zcash_local_net/src/{client => wallet}/zcash_devtool.rs (95%) create mode 100644 zingolib-wallet-impl-spec.md diff --git a/CONTEXT.md b/CONTEXT.md index c947141..f4eeba1 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -15,6 +15,14 @@ A process that consumes a Validator's data and serves the light-client gRPC protocol (zainod, or legacy lightwalletd). _Avoid_: proxy, server +**Wallet**: +A light-client process the harness manages alongside the Validator and +Indexer: restored from a seed, synced through an Indexer, driven to +send and shield, and queried for balances and addresses. The harness +actuates every Wallet through one generic interface; each +implementation lives with its own binary (zcash-devtool, zingo-cli). +_Avoid_: client (the superseded name), lightclient + **Legacy stack**: zcashd (Validator) and lightwalletd (Indexer). Opt-in only, scheduled for simultaneous removal; neither is part of the harness's future. diff --git a/zcash_local_net/CHANGELOG.md b/zcash_local_net/CHANGELOG.md index f117734..e001b61 100644 --- a/zcash_local_net/CHANGELOG.md +++ b/zcash_local_net/CHANGELOG.md @@ -50,6 +50,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 configured NU7 height instead of silently dropping it (the devtool TOML gates `nu7` behind `zcash_unstable`), matching the zebrad writer's no-silent-drop policy below. +- **Breaking** — the client layer is now the **Wallet abstraction**: the + `client` module is renamed to `wallet`, the `Client`/`ClientConfig` + traits to `Wallet`/`WalletConfig`, and `ClientError` to `WalletError`, + whose messages now name the operation rather than one binary. The + trait is the interface through which the harness actuates any wallet + implementation; implementations live with their binaries (the + zcash-devtool one remains in-tree for now, and a zingo-cli + implementation lands in the zingolib repository — see + `zingolib-wallet-impl-spec.md`). - **Breaking** — the zebrad regtest config writer now **rejects activation heights it cannot express** instead of silently dropping or rewriting them: upgrades through Canopy must be `Some(1)` (the emitted config @@ -87,7 +96,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `zingo_consensus::NetworkKind`: network identity without activation heights, with `From`/`From<&NetworkType>` conversions — the config shape for components that must not be told heights. -- `client::WalletNetwork` and `client::ValidatorHeights`: the network a +- `LocalNet::launch_wallet::(make_config)`: generic wallet + actuation — mints the `WalletNetwork` from the running Validator, + wires the Indexer connection, and launches any `Wallet` + implementation. `ValidatorHeights::activation_heights()` exposes the + reported schedule read-only, because foreign implementations must + serialize it into their own binaries' configs; construction remains + private to preserve the ADR 0003 provenance guarantee. +- `wallet::WalletNetwork` and `wallet::ValidatorHeights`: the network a wallet is launched against, and regtest activation heights whose provenance is a Validator query. `WalletNetwork::from_validator()` is the only public constructor of `ValidatorHeights`, which makes ADR diff --git a/zcash_local_net/src/error.rs b/zcash_local_net/src/error.rs index e9141fd..c91f2f5 100644 --- a/zcash_local_net/src/error.rs +++ b/zcash_local_net/src/error.rs @@ -118,12 +118,13 @@ pub enum LaunchError { } /// Errors associated with driving wallet client operations -/// (run-to-completion subprocess invocations, see [`crate::client`]). +/// (see [`crate::wallet`]). Shared by every [`crate::wallet::Wallet`] +/// implementation, so messages name the operation, not one binary. #[derive(thiserror::Error, Debug, Clone)] -pub enum ClientError { +pub enum WalletError { /// The client binary could not be spawned at all /// (PATH/`TEST_BINARIES_DIR`/permission problems). - #[error("zcash-devtool {operation} failed to spawn: {io_error}")] + #[error("wallet {operation} failed to spawn: {io_error}")] SpawnFailed { /// The wallet operation being attempted operation: &'static str, @@ -132,7 +133,7 @@ pub enum ClientError { }, /// Writing to the child's stdin failed (used by `init`, which /// receives the mnemonic on stdin). - #[error("zcash-devtool {operation}: writing to child stdin failed: {io_error}")] + #[error("wallet {operation}: writing to child stdin failed: {io_error}")] StdinWriteFailed { /// The wallet operation being attempted operation: &'static str, @@ -141,7 +142,7 @@ pub enum ClientError { }, /// The operation subprocess exited non-zero. #[error( - "zcash-devtool {operation} failed.\nExit status: {exit_status}\nStdout: {stdout}\nStderr: {stderr}" + "wallet {operation} failed.\nExit status: {exit_status}\nStdout: {stdout}\nStderr: {stderr}" )] OperationFailed { /// The wallet operation being attempted @@ -158,7 +159,7 @@ pub enum ClientError { /// the contract-drift tripwire: it fires when the client binary's /// output format changes out from under the harness's parsers. #[error( - "zcash-devtool {operation} succeeded but its output could not be parsed: {reason}\nStdout: {stdout}" + "wallet {operation} succeeded but its output could not be parsed: {reason}\nStdout: {stdout}" )] UnexpectedOutput { /// The wallet operation being attempted diff --git a/zcash_local_net/src/lib.rs b/zcash_local_net/src/lib.rs index f94f899..61eb941 100644 --- a/zcash_local_net/src/lib.rs +++ b/zcash_local_net/src/lib.rs @@ -12,7 +12,7 @@ //! # List of Managed Processes //! - Zebrad //! - Zainod -//! - zcash-devtool (wallet client; per-operation subprocess, see [`crate::client`]) +//! - zcash-devtool (wallet; per-operation subprocess, see [`crate::wallet`]) //! //! # Prerequisites //! @@ -42,7 +42,6 @@ //! See [`crate::LocalNet`]. //! -pub mod client; pub mod config; pub mod error; pub mod indexer; @@ -52,6 +51,7 @@ pub mod process; pub mod rpc_client; pub mod utils; pub mod validator; +pub mod wallet; pub mod zebra_rpc; mod launch; @@ -169,6 +169,23 @@ where }) .await } + + /// Launch a wallet against this network, generically over the + /// wallet implementation `W`. The wallet's network is minted from + /// the running Validator ([`wallet::WalletNetwork::from_validator`], + /// the only source of regtest heights for a wallet config — ADR + /// 0003) and its server connection is wired to the Indexer. + /// `make_config` is one of the implementation's constructors, e.g. + /// `ZcashDevtoolConfig::faucet`. + pub async fn launch_wallet( + &self, + make_config: impl FnOnce(wallet::WalletNetwork) -> W::Config, + ) -> Result { + let network = wallet::WalletNetwork::from_validator(self.validator()).await; + let mut config = make_config(network); + wallet::WalletConfig::setup_indexer_connection(&mut config, self.indexer()); + W::launch(config).await + } } impl LocalNet diff --git a/zcash_local_net/src/client.rs b/zcash_local_net/src/wallet.rs similarity index 70% rename from zcash_local_net/src/client.rs rename to zcash_local_net/src/wallet.rs index cf87e7d..67f8452 100644 --- a/zcash_local_net/src/client.rs +++ b/zcash_local_net/src/wallet.rs @@ -1,20 +1,21 @@ -//! Module for the structs that represent and manage wallet client processes i.e. zcash-devtool. +//! The Wallet abstraction: the interface through which the harness +//! actuates wallets, generically over their implementations. //! -//! Clients are the third kind of process this crate manages, alongside +//! Wallets are the third kind of process this crate manages, alongside //! validators ([`crate::validator`]) and indexers ([`crate::indexer`]). -//! They differ structurally from both: the managed binary is not a -//! daemon. Each wallet operation (`init`, `sync`, `send`, …) is a -//! separate run-to-completion subprocess invocation against a persistent -//! wallet directory. There is no long-lived child handle to stop or to -//! probe for readiness; the managed state is the wallet directory -//! itself, created in a tempdir owned by the client struct and removed -//! when it drops. +//! This module defines only the contract — the [`Wallet`] and +//! [`WalletConfig`] traits and their supporting types. Implementations +//! live with their binaries: the zcash-devtool wallet is in +//! [`zcash_devtool`] (in-tree for now), and the zingo-cli wallet is +//! implemented in the zingolib repository against this trait. The +//! harness drives whichever implementation the caller names, e.g. +//! through `LocalNet::launch_wallet::`. //! -//! Clients speak the lightwalletd protocol (gRPC) to an indexer — they +//! Wallets speak the lightwalletd protocol (gRPC) to an indexer — they //! never talk to the validator directly. Launch order is therefore -//! validator → indexer → client; [`ClientConfig::setup_indexer_connection`] +//! validator → indexer → wallet; [`WalletConfig::setup_indexer_connection`] //! mirrors [`crate::indexer::IndexerConfig::setup_validator_connection`] -//! for wiring the client to a running indexer. +//! for wiring the wallet to a running indexer. //! //! Activation heights are the one exception to "never talk to the //! validator": a regtest wallet needs the chain's schedule, the @@ -23,20 +24,30 @@ //! on the wallet's behalf, and the type system enforces it — see //! [`WalletNetwork`] and [`ValidatorHeights`]. -use crate::{error::ClientError, indexer::Indexer}; +use crate::{error::WalletError, indexer::Indexer}; /// Regtest activation heights whose provenance is a query of a running -/// Validator. The inner value has no public constructor and no public -/// accessor; the only way to obtain one is -/// [`WalletNetwork::from_validator`]. A wallet configured with these -/// heights is therefore guaranteed, at compile time, to have derived -/// them from the Validator (ADR 0003: the Validator is the single -/// source of truth for activation heights). Crate-internal tests may -/// construct the value directly to pin serialization offline, where no -/// chain exists for the heights to disagree with. +/// Validator. The inner value has no public constructor; the only way +/// to obtain one is [`WalletNetwork::from_validator`]. A wallet +/// configured with these heights is therefore guaranteed, at compile +/// time, to have derived them from the Validator (ADR 0003: the +/// Validator is the single source of truth for activation heights). +/// Crate-internal tests may construct the value directly to pin +/// serialization offline, where no chain exists for the heights to +/// disagree with. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct ValidatorHeights(pub(crate) zingo_consensus::ActivationHeights); +impl ValidatorHeights { + /// The schedule the Validator reported. Reading is public because + /// every [`Wallet`] implementation must serialize the heights into + /// its own binary's configuration; only *construction* is + /// restricted, since provenance — not secrecy — is the invariant. + pub fn activation_heights(&self) -> zingo_consensus::ActivationHeights { + self.0 + } +} + /// The network a wallet client is launched against. Unlike /// [`zingo_consensus::NetworkType`], the regtest variant cannot carry /// caller-supplied heights: it demands a [`ValidatorHeights`], which @@ -64,15 +75,15 @@ impl WalletNetwork { } } -/// Can offer specific functionality shared across configuration for all clients. -pub trait ClientConfig: std::fmt::Debug { - /// To receive the connection details of the indexer this client's +/// Configuration behavior every wallet implementation shares. +pub trait WalletConfig: std::fmt::Debug { + /// To receive the connection details of the indexer this /// wallet will sync from and broadcast through. fn setup_indexer_connection(&mut self, indexer: &I); } /// Which receiver of the wallet's unified address to emit from -/// [`Client::address`]. +/// [`Wallet::address`]. /// /// A dedicated enum rather than [`zingo_consensus::MinerPool`], which /// has no `Unified` variant — the wrong shape for "give me this @@ -90,46 +101,46 @@ pub enum AddressReceiver { Orchard, } -/// Functionality for wallet client processes. +/// The interface through which the harness actuates a wallet. /// /// The operation set mirrors what wallet integration suites (zaino's in /// particular) drive between asserts: sync to tip, send, shield, /// per-pool balance, address derivation, and rescan-from-scratch. All /// operations run to completion before returning — callers can sequence /// `act → mine → wait → assert` without additional synchronization. -pub trait Client: Sized { - /// A config struct for the client. - type Config: ClientConfig; +pub trait Wallet: Sized { + /// The configuration for this wallet implementation. + type Config: WalletConfig; /// Create the wallet (restoring from the configured mnemonic and - /// birthday) and return the managed client. The configured indexer + /// birthday) and return the managed wallet. The configured indexer /// must already be serving: wallet initialization fetches the chain /// tip and the birthday tree state from it. fn launch(config: Self::Config) - -> impl std::future::Future>; + -> impl std::future::Future>; /// Scan the chain and sync the wallet to the indexer's tip. - fn sync(&self) -> impl std::future::Future>; + fn sync(&self) -> impl std::future::Future>; /// Send `value_zats` zatoshis to `address` (transparent, sapling or /// unified). Returns the txid of the broadcast transaction as a hex /// string. The transaction is broadcast but NOT mined; mine a block - /// and [`Client::sync`] to confirm it. + /// and [`Wallet::sync`] to confirm it. fn send( &self, address: &str, value_zats: u64, - ) -> impl std::future::Future>; + ) -> impl std::future::Future>; /// Shield transparent funds (including mature transparent coinbase) /// into the orchard pool. Returns the txid of the broadcast /// transaction as a hex string. - fn shield(&self) -> impl std::future::Future>; + fn shield(&self) -> impl std::future::Future>; - /// The wallet's view of its balance. Run [`Client::sync`] first; + /// The wallet's view of its balance. Run [`Wallet::sync`] first; /// this reads the local wallet database without contacting the /// indexer. - fn balance(&self) -> impl std::future::Future>; + fn balance(&self) -> impl std::future::Future>; /// The requested `receiver` of the wallet's unified address, as an /// encoded address string. Reads the local wallet database without @@ -137,29 +148,29 @@ pub trait Client: Sized { fn address( &self, receiver: AddressReceiver, - ) -> impl std::future::Future>; + ) -> impl std::future::Future>; /// The wallet's default unified address. Convenience for - /// [`Client::address`] with [`AddressReceiver::Unified`]. - fn default_address(&self) -> impl std::future::Future> { + /// [`Wallet::address`] with [`AddressReceiver::Unified`]. + fn default_address(&self) -> impl std::future::Future> { self.address(AddressReceiver::Unified) } /// Node/indexer information reported by the configured server. /// Contacts the indexer (the analogue of zingolib's `do_info`); a /// smoke check that the wallet can reach and talk to its server. - fn get_info(&self) -> impl std::future::Future>; + fn get_info(&self) -> impl std::future::Future>; /// Wipe the wallet state and re-restore from the stored mnemonic /// and birthday, preserving account metadata. Equivalent to a - /// rescan from scratch; [`Client::sync`] afterwards to rebuild. - fn rescan(&self) -> impl std::future::Future>; + /// rescan from scratch; [`Wallet::sync`] afterwards to rebuild. + fn rescan(&self) -> impl std::future::Future>; } -/// Node/indexer information from [`Client::get_info`]. +/// Node/indexer information from [`Wallet::get_info`]. /// /// The field set is a frozen contract with the wallet binary: see -/// `client::zcash_devtool`'s get-info parser. `chain_tip_height` is the +/// `wallet::zcash_devtool`'s get-info parser. `chain_tip_height` is the /// **server/node tip** the indexer reports, never the wallet's /// locally-synced height (which, if ever surfaced, gets its own /// explicitly-named field). @@ -177,7 +188,7 @@ pub struct GetInfo { /// A wallet balance snapshot, in zatoshis. /// -/// Spendable values are as reported by the client's configured +/// Spendable values are as reported by the wallet's configured /// confirmations policy; immature or unconfirmed funds are included in /// `total` but not in the per-pool spendable fields. #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/zcash_local_net/src/client/zcash_devtool.rs b/zcash_local_net/src/wallet/zcash_devtool.rs similarity index 95% rename from zcash_local_net/src/client/zcash_devtool.rs rename to zcash_local_net/src/wallet/zcash_devtool.rs index 8f36f56..b8d7ec7 100644 --- a/zcash_local_net/src/client/zcash_devtool.rs +++ b/zcash_local_net/src/wallet/zcash_devtool.rs @@ -4,7 +4,7 @@ //! as a managed subprocess. The binary must be built with //! `--features regtest_support` for regtest wallets — stock builds //! reject `-n regtest` (the operation fails with "Unsupported network" -//! captured in [`crate::error::ClientError::OperationFailed`]). +//! captured in [`crate::error::WalletError::OperationFailed`]). //! //! Output-shape contract: the parsers in this module (final-line txid, //! `balance --json` object, `Default Address:` / `Receiver():` @@ -22,14 +22,14 @@ use tempfile::TempDir; use zingo_test_vectors::seeds::{ABANDON_ART_SEED, HOSPITAL_MUSEUM_SEED}; use crate::{ - client::{ - AddressReceiver, Client, ClientConfig, GetInfo, ValidatorHeights, WalletBalance, - WalletNetwork, - }, - error::ClientError, + error::WalletError, indexer::Indexer, logs::LogsToDir, utils::executable_finder::pick_command, + wallet::{ + AddressReceiver, GetInfo, ValidatorHeights, Wallet, WalletBalance, WalletConfig, + WalletNetwork, + }, }; const EXECUTABLE_NAME: &str = "zcash-devtool"; @@ -84,7 +84,7 @@ pub fn supported_regtest_activation_heights() -> zingo_consensus::ActivationHeig /// The wallet is restored from `mnemonic` at `birthday` and synced /// against a lightwalletd-protocol (gRPC) server at /// `127.0.0.1:indexer_port` — wire it to a running indexer with -/// [`ClientConfig::setup_indexer_connection`] or set the port directly. +/// [`WalletConfig::setup_indexer_connection`] or set the port directly. /// /// `network` must name the network of the indexer's validator; for /// regtest it can only be built from that validator, so agreement is @@ -148,7 +148,7 @@ impl ZcashDevtoolConfig { /// index 1 of this seed as the recipient; `init` restores account /// index 0, so addresses differ from the zingolib recipient's. /// Tests should obtain addresses from - /// [`Client::default_address`] rather than from constants recorded + /// [`Wallet::default_address`] rather than from constants recorded /// against account 1. pub fn recipient(network: WalletNetwork) -> Self { Self { @@ -162,7 +162,7 @@ impl ZcashDevtoolConfig { } } -impl ClientConfig for ZcashDevtoolConfig { +impl WalletConfig for ZcashDevtoolConfig { fn setup_indexer_connection(&mut self, indexer: &I) { self.indexer_port = indexer.listen_port(); } @@ -232,7 +232,7 @@ impl ZcashDevtool { /// `nu7` is intentionally omitted — the devtool's TOML gates that /// field behind `zcash_unstable`, so emitting it would trip /// `deny_unknown_fields` on release builds. - fn write_activation_heights_toml(&self) -> Result, ClientError> { + fn write_activation_heights_toml(&self) -> Result, WalletError> { let WalletNetwork::Regtest(ValidatorHeights(heights)) = self.config.network else { return Ok(None); }; @@ -265,7 +265,7 @@ impl ZcashDevtool { } } let path = self.wallet_dir.path().join("activation-heights.toml"); - std::fs::write(&path, body).map_err(|io_error| ClientError::SpawnFailed { + std::fs::write(&path, body).map_err(|io_error| WalletError::SpawnFailed { operation: "init", io_error: format!("writing activation-heights file: {io_error}"), })?; @@ -306,7 +306,7 @@ impl ZcashDevtool { operation: &'static str, args: &[&str], stdin_line: Option<&str>, - ) -> Result { + ) -> Result { let mut command = pick_command(EXECUTABLE_NAME, false); command .arg("wallet") @@ -323,7 +323,7 @@ impl ZcashDevtool { let mut handle = command .spawn() - .map_err(|io_error| ClientError::SpawnFailed { + .map_err(|io_error| WalletError::SpawnFailed { operation, io_error: io_error.to_string(), })?; @@ -333,7 +333,7 @@ impl ZcashDevtool { let output = handle .wait_with_output() - .map_err(|io_error| ClientError::SpawnFailed { + .map_err(|io_error| WalletError::SpawnFailed { operation, io_error: io_error.to_string(), })?; @@ -342,7 +342,7 @@ impl ZcashDevtool { if output.status.success() { Ok(output) } else { - Err(ClientError::OperationFailed { + Err(WalletError::OperationFailed { operation, exit_status: output.status, stdout: String::from_utf8_lossy(&output.stdout).into_owned(), @@ -357,10 +357,10 @@ impl ZcashDevtool { &self, operation: &'static str, args: &[&str], - ) -> Result { + ) -> Result { let output = self.run_wallet_op(operation, args, None).await?; let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - parse_final_txid(&stdout).map_err(|reason| ClientError::UnexpectedOutput { + parse_final_txid(&stdout).map_err(|reason| WalletError::UnexpectedOutput { operation, reason, stdout, @@ -368,10 +368,10 @@ impl ZcashDevtool { } } -impl Client for ZcashDevtool { +impl Wallet for ZcashDevtool { type Config = ZcashDevtoolConfig; - async fn launch(config: Self::Config) -> Result { + async fn launch(config: Self::Config) -> Result { crate::utils::executable_finder::trace_version_and_location(EXECUTABLE_NAME, "--help"); // tempfile failures here are environment errors (no tmpfs @@ -430,7 +430,7 @@ impl Client for ZcashDevtool { Ok(client) } - async fn sync(&self) -> Result<(), ClientError> { + async fn sync(&self) -> Result<(), WalletError> { self.run_wallet_op( "sync", &["sync", "-s", &self.server(), "--connection", "direct"], @@ -440,7 +440,7 @@ impl Client for ZcashDevtool { Ok(()) } - async fn send(&self, address: &str, value_zats: u64) -> Result { + async fn send(&self, address: &str, value_zats: u64) -> Result { let identity_file = self.identity_file(); let identity = identity_file.to_str().expect("tempdir paths are UTF-8"); let value = value_zats.to_string(); @@ -466,7 +466,7 @@ impl Client for ZcashDevtool { .await } - async fn shield(&self) -> Result { + async fn shield(&self) -> Result { let identity_file = self.identity_file(); let identity = identity_file.to_str().expect("tempdir paths are UTF-8"); self.run_txid_op( @@ -484,7 +484,7 @@ impl Client for ZcashDevtool { .await } - async fn balance(&self) -> Result { + async fn balance(&self) -> Result { let min_confirmations = self.config.min_confirmations.to_string(); let output = self .run_wallet_op( @@ -499,14 +499,14 @@ impl Client for ZcashDevtool { ) .await?; let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - parse_balance_json(&stdout).map_err(|reason| ClientError::UnexpectedOutput { + parse_balance_json(&stdout).map_err(|reason| WalletError::UnexpectedOutput { operation: "balance", reason, stdout, }) } - async fn address(&self, receiver: AddressReceiver) -> Result { + async fn address(&self, receiver: AddressReceiver) -> Result { let flag = match receiver { AddressReceiver::Unified => "unified", AddressReceiver::Transparent => "transparent", @@ -529,14 +529,14 @@ impl Client for ZcashDevtool { AddressReceiver::Sapling => parse_receiver(&stdout, "sapling"), AddressReceiver::Orchard => parse_receiver(&stdout, "orchard"), }; - parsed.map_err(|reason| ClientError::UnexpectedOutput { + parsed.map_err(|reason| WalletError::UnexpectedOutput { operation: "list-addresses", reason, stdout, }) } - async fn get_info(&self) -> Result { + async fn get_info(&self) -> Result { let output = self .run_wallet_op( "get-info", @@ -545,14 +545,14 @@ impl Client for ZcashDevtool { ) .await?; let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - parse_getinfo_json(&stdout).map_err(|reason| ClientError::UnexpectedOutput { + parse_getinfo_json(&stdout).map_err(|reason| WalletError::UnexpectedOutput { operation: "get-info", reason, stdout, }) } - async fn rescan(&self) -> Result<(), ClientError> { + async fn rescan(&self) -> Result<(), WalletError> { let identity_file = self.identity_file(); let identity = identity_file.to_str().expect("tempdir paths are UTF-8"); self.run_wallet_op( @@ -578,18 +578,18 @@ fn write_stdin_line( handle: &mut Child, operation: &'static str, line: &str, -) -> Result<(), ClientError> { +) -> Result<(), WalletError> { let mut stdin = handle .stdin .take() - .ok_or_else(|| ClientError::StdinWriteFailed { + .ok_or_else(|| WalletError::StdinWriteFailed { operation, io_error: "child stdin was not captured".to_string(), })?; stdin .write_all(line.as_bytes()) .and_then(|()| stdin.write_all(b"\n")) - .map_err(|io_error| ClientError::StdinWriteFailed { + .map_err(|io_error| WalletError::StdinWriteFailed { operation, io_error: io_error.to_string(), }) diff --git a/zcash_local_net/tests/integration.rs b/zcash_local_net/tests/integration.rs index 1aa8bb7..8e15474 100644 --- a/zcash_local_net/tests/integration.rs +++ b/zcash_local_net/tests/integration.rs @@ -939,18 +939,18 @@ async fn zebrad_regtest_skips_seed_peer_dns() { /// zcash-devtool client management: pins the devtool CLI contract /// (flags, stdout shapes, regtest activation-height alignment) against /// the real binary, per the "behaviour drift from a contract this code -/// mirrors" rule — the parsers in `client::zcash_devtool` are only +/// mirrors" rule — the parsers in `wallet::zcash_devtool` are only /// trusted because these tests exercise them live. /// /// Requires `zcash-devtool` (built with `--features regtest_support`) /// in `TEST_BINARIES_DIR` or on `PATH`. mod devtool_client { - use zcash_local_net::client::zcash_devtool::{ - ZcashDevtool, ZcashDevtoolConfig, supported_regtest_activation_heights, - }; - use zcash_local_net::client::{AddressReceiver, Client, ClientConfig as _, WalletNetwork}; use zcash_local_net::indexer::zainod::ZainodConfig; use zcash_local_net::validator::Validator as _; + use zcash_local_net::wallet::zcash_devtool::{ + ZcashDevtool, ZcashDevtoolConfig, supported_regtest_activation_heights, + }; + use zcash_local_net::wallet::{AddressReceiver, Wallet, WalletNetwork}; use zingo_test_vectors::{ REG_O_ADDR_FROM_ABANDONART, REG_T_ADDR_FROM_ABANDONART, REG_Z_ADDR_FROM_ABANDONART, }; @@ -1000,20 +1000,19 @@ mod devtool_client { .unwrap() } - /// Launch a devtool wallet wired to the local net's indexer. The - /// wallet's network is minted from the running validator - /// ([`WalletNetwork::from_validator`]), which is the only way to - /// obtain regtest heights for a wallet config (ADR 0003). - /// `make_config` is one of the [`ZcashDevtoolConfig`] constructors, - /// e.g. `ZcashDevtoolConfig::faucet`. + /// Launch a devtool wallet through the harness's generic actuation + /// path: `LocalNet::launch_wallet` mints the wallet's network from + /// the running validator (ADR 0003) and wires the indexer + /// connection, for any `Wallet` implementation — the devtool one + /// here. `make_config` is one of the [`ZcashDevtoolConfig`] + /// constructors, e.g. `ZcashDevtoolConfig::faucet`. async fn launch_client( net: &LocalNet, make_config: impl FnOnce(WalletNetwork) -> ZcashDevtoolConfig, ) -> ZcashDevtool { - let network = WalletNetwork::from_validator(net.validator()).await; - let mut config = make_config(network); - config.setup_indexer_connection(net.indexer()); - ZcashDevtool::launch(config).await.unwrap() + net.launch_wallet::(make_config) + .await + .unwrap() } /// Sync the wallet until its view of the chain tip reaches @@ -1023,7 +1022,7 @@ mod devtool_client { async fn sync_to_height( client: &ZcashDevtool, target_height: u32, - ) -> zcash_local_net::client::WalletBalance { + ) -> zcash_local_net::wallet::WalletBalance { const ATTEMPTS: u32 = 120; for _ in 0..ATTEMPTS { client.sync().await.unwrap(); @@ -1039,7 +1038,7 @@ mod devtool_client { /// The faucet linchpin: devtool's account-0 derivation of the /// abandon-art seed must yield the same addresses the validators /// mine to, otherwise the "faucet" never sees a reward. Pins every - /// receiver of [`Client::address`] against the `zingo_test_vectors` + /// receiver of [`Wallet::address`] against the `zingo_test_vectors` /// constants — the unified address (== the orchard miner address) /// and the bare transparent/sapling receivers — proving the /// abandon-art wallet owns the addresses the harness pays. diff --git a/zingolib-wallet-impl-spec.md b/zingolib-wallet-impl-spec.md new file mode 100644 index 0000000..5f397ed --- /dev/null +++ b/zingolib-wallet-impl-spec.md @@ -0,0 +1,95 @@ +# Spec: implement the harness Wallet trait for zingo-cli, in zingolib + +Requested by infras for the zingolib Ironwood integration branch +(zingolabs/zingolib#2419, `feat/ironwood`). The harness +(`zcash_local_net`, this repository, branch `bump_to_NU6.3`) defines a +generic Wallet abstraction and actuates any implementation through it; +the zcash-devtool implementation lives in-tree for now, and the +zingo-cli implementation lives in zingolib. Implementations never live +in this repository going forward. + +## The contract zingolib implements + +`zcash_local_net::wallet` defines the interface: + +- **`trait Wallet`** with an associated `Config: WalletConfig` and the + operations `launch`, `sync`, `send`, `shield`, `balance`, `address` + (plus the provided `default_address`), `get_info`, and `rescan`. + Every operation runs to completion before returning, so callers can + sequence act → mine → wait → assert without extra synchronization. +- **`trait WalletConfig`** with `setup_indexer_connection`, which + receives the Indexer's gRPC listen port. Wallets speak only the + lightwalletd protocol to an Indexer; they never contact the + Validator directly. +- **`WalletNetwork` / `ValidatorHeights`** (ADR 0003): the regtest + variant of a wallet's network can only be built by + `WalletNetwork::from_validator(&validator)`, which queries the + running Validator's schedule. Read the schedule with + `ValidatorHeights::activation_heights()` and serialize it into + zingo-cli's own configuration. There must be no other source of + regtest activation heights in the implementation — no compiled-in + defaults, no hand-typed vectors, no zingolib-side constants. +- **`WalletError`**: the shared error type. Use `OperationFailed` for + a non-zero child exit (carrying captured output), and + `UnexpectedOutput` as the contract-drift tripwire when zingo-cli's + output no longer parses. Do not add blanket conversions that + collapse distinct failures into one variant. +- **`WalletBalance`** (zatoshis: `total`, `sapling_spendable`, + `orchard_spendable`, `ironwood_spendable`, `transparent_spendable`, + and `chain_tip_height`, the wallet's synced height) and **`GetInfo`** + (`server_uri`, `chain_name`, `chain_tip_height`). `GetInfo`'s + `chain_tip_height` is the server tip the Indexer reports, never the + wallet's synced height — that distinction is a frozen contract. + +The harness actuates implementations generically: + +```rust +let faucet: ZingoCliWallet = net + .launch_wallet(ZingoCliWalletConfig::faucet) + .await?; +``` + +`LocalNet::launch_wallet::` mints the `WalletNetwork` from the +running Validator and wires the Indexer connection before calling +`W::launch`, so the implementation's constructors must have the shape +`fn(WalletNetwork) -> Config` (a faucet constructor restoring the +shared "abandon … art" mnemonic at birthday zero, and a recipient +constructor, mirror the devtool implementation's pair; the faucet seed +is what the harness's validators mine to). + +## Requested change + +1. In zingolib, add a module or crate (zingolib's choice of location) + that depends on `zcash_local_net` — pinned to the `bump_to_NU6.3` + tip or the 0.8.0 release that follows it — and implements `Wallet` + and `WalletConfig` for a zingo-cli-driven wallet. +2. Pin the implementation's parsing of zingo-cli output against the + real binary with tests, in the same way the devtool implementation + pins its parsers: recorded shapes for unit tests, live integration + runs for the end-to-end contract. Do not derive behavior from + documentation alone. +3. Prove the implementation with the harness's scenario shape: on a + `LocalNet`, launch a faucet through + `launch_wallet`, mine, sync, and assert miner rewards appear in the + balance; send to a recipient wallet and assert receipt; shield + transparent funds and assert the value lands in the ironwood pool + under NU6.3; rescan and assert the balance survives. These mirror + the devtool suite in `zcash_local_net/tests/integration.rs`, which + is the parity reference. + +## Out of scope + +- Changing the trait itself. If zingo-cli cannot honestly satisfy a + method's semantics, propose the interface change upstream in infras + rather than bending the implementation's semantics to fit. +- Mainnet and Testnet wallets. +- The zcash-devtool implementation, which remains in infras for now. + +## Delivery + +An implementation on the #2419 integration branch or a follow-up +zingolib branch, pinning `zcash_local_net` at a rev containing the +Wallet abstraction. When both implementations exist, infras can lift +its devtool scenario tests into generic fixtures parameterized over +`W: Wallet`, so the identical suite runs against both wallets in their +respective repositories.