From 94b58364d790141542747bf159b21f0044d0be2f Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 15:48:12 +0200 Subject: [PATCH 01/26] fix(explorer): a seeded mainnet URL is not a choice the user made MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found re-validating the spec review of #52-#54. Spec 2026-09-14 section 4 promises "never a silent mainnet query" off mainnet, and the CHANGELOG entry for it says the same. Both were true only of a fresh database. Migration 009 originally seeded `explorer_api_url` with the mainnet explorer. #54 edited that already-shipped migration to seed an empty string instead, so the runtime resolves the explorer per the profile's network. A migration runs once: every installation that had already applied 009 kept the mainnet URL in `settings`, and `explorer_client_from_settings` prefers an explicit setting over the network default. So an upgraded wallet on testnet or regtest went on reading mainnet and getting confident answers about a chain it was not on — the exact cross-network read G2 exists to close. Two layers, because they fail differently. Migration 032 deletes the setting when it still holds the value 009 seeded, and leaves any other URL alone: a private explorer is the user's own and the wallet cannot tell which chain it serves. The factory then refuses the known mainnet explorer off mainnet, which covers a database that reaches the resolver before a newer build's migrations have run. Four tests pinned the migration count and had to move to 32; the comment block in `migration_tests` names the new one like its neighbours. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2572 passed), npm run lint:format. --- CHANGELOG.md | 1 + src-tauri/src/db/migrations.rs | 8 ++- src-tauri/src/providers/mod.rs | 19 ++++- .../sql/032_clear_seeded_mainnet_explorer.sql | 19 +++++ src-tauri/src/tests/connection_tests.rs | 4 +- src-tauri/src/tests/migration_tests.rs | 59 ++++++++++++++- src-tauri/src/tests/provider_hnsfans_tests.rs | 72 +++++++++++++++++++ 7 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 src-tauri/src/sql/032_clear_seeded_mainnet_explorer.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index ba3c2ef2..b8bac1a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and — once a wallet profile exists — whether the node's network matches the wallet's; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **A testnet or regtest wallet no longer reads from the mainnet explorer.** An early migration seeded `explorer_api_url` with the mainnet explorer, and a later change swapped that seed for an empty string so the wallet could pick an explorer per the profile's network. The swap only helped databases created after it: every installation that had already run the original kept the mainnet URL, and an explicit setting outranks the network default, so a testnet or regtest profile went on querying mainnet and getting confident answers about a chain it was not on. A new migration removes exactly the value the old one seeded, leaving an explorer URL you chose yourself alone, and the explorer factory now refuses the known mainnet explorer off mainnet in case a database reaches it before the migration has run. - **Reveal is no longer offered on a name with nothing left to reveal.** A lockup stranded in an auction that lapsed is an unspent BID coin, and the Reveal button was gated on holding one of those anywhere in the wallet rather than in the auction being looked at. So on a fully revealed name the button stayed live for good and failed every time with "no unspent bid coin". It now reads the same set the reveal transaction is built from, so the button and the builder cannot disagree. - **Registering no longer looks as though DNS records are required.** The Register step put a record editor in front of the user and said nothing about it. Records are optional — Handshake accepts an empty resource, and the wallet was already sending one when the editor was untouched — so the panel now says registering claims the name and records can follow with Update, and keeps the editor behind "Add DNS records now (optional)". Register also stopped appearing twice, live in both the guided step and the records section, and the manual auction actions now show only what the stage allows, each with a line saying what pressing it does — Redeem names the amount it reclaims. - **The name status no longer contradicts the modal it opens.** The Owned Names table printed the auction phase while the modal printed the task, so a row reading "Closed" opened a modal headed "Won — Register Now". The table now shows the same summary the auctions list and the modal do, and defers to what is in flight when a transaction of yours is waiting for a block. The phase was nearly a constant down that column anyway: every name you own has a closed auction. diff --git a/src-tauri/src/db/migrations.rs b/src-tauri/src/db/migrations.rs index 43c9e608..2718a40e 100644 --- a/src-tauri/src/db/migrations.rs +++ b/src-tauri/src/db/migrations.rs @@ -50,6 +50,10 @@ const MIGRATIONS: &[(&str, &str)] = &[ ("029", include_str!("../sql/029_bid_auction_scope.sql")), ("030", include_str!("../sql/030_bid_commitment_auction.sql")), ("031", include_str!("../sql/031_rescan_reveal_pairing.sql")), + ( + "032", + include_str!("../sql/032_clear_seeded_mainnet_explorer.sql"), + ), ]; pub fn run(conn: &Connection) -> Result<(), rusqlite::Error> { @@ -91,7 +95,7 @@ mod tests { let count: i64 = conn .query_row("SELECT COUNT(*) FROM schema_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(count, 31, "expected 31 migrations, got {count}"); + assert_eq!(count, 32, "expected 32 migrations, got {count}"); } #[test] @@ -102,7 +106,7 @@ mod tests { let count: i64 = conn .query_row("SELECT COUNT(*) FROM schema_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(count, 31); + assert_eq!(count, 32); } #[test] diff --git a/src-tauri/src/providers/mod.rs b/src-tauri/src/providers/mod.rs index 4ed5e14e..298e25b9 100644 --- a/src-tauri/src/providers/mod.rs +++ b/src-tauri/src/providers/mod.rs @@ -36,11 +36,22 @@ pub use signer::{ /// data. On testnet/regtest/simnet there is no known public explorer, so /// pointing a non-mainnet wallet at `e.hnsfans.com` produced false "no data" /// results. Resolution order: -/// 1. explicit `explorer_api_url` from settings (any network), else +/// 1. explicit `explorer_api_url` from settings, unless it is the known +/// mainnet explorer and the profile is not on mainnet, else /// 2. [`Network::default_explorer_base_url`] (mainnet only), else /// 3. `None` — the explorer fallback is *disabled* and callers must degrade /// to cache or a candid "explorer unavailable" error instead of mainnet. /// +/// Step 1 refuses the mainnet explorer off mainnet because the stored setting +/// is not always something the user chose: migration 009 seeded it with the +/// mainnet URL, and every installation that ran that version carries the value +/// still (032 clears exactly that seeded value, but a database can reach this +/// code before migrations of a newer build have run). An explicit URL outranks +/// the network default, so without this check the seeded mainnet URL silently +/// won on a testnet or regtest profile — the cross-network read the guard is +/// for. Any other URL is the user's own and is honoured on every network: the +/// wallet cannot know which chain a private explorer serves. +/// /// If `explorer_fallback_url` is set in settings, the client will /// automatically fail over to it when the primary explorer is unreachable. pub fn explorer_client_from_settings( @@ -50,7 +61,11 @@ pub fn explorer_client_from_settings( let explicit = settings .get("explorer_api_url") .map(|s| s.trim()) - .filter(|s| !s.is_empty()); + .filter(|s| !s.is_empty()) + .filter(|u| { + network == crate::noncustodial::network::Network::Main + || u.trim_end_matches('/') != hnsfans::DEFAULT_EXPLORER_URL + }); let url = match explicit { Some(u) => u, None => network.default_explorer_base_url()?, diff --git a/src-tauri/src/sql/032_clear_seeded_mainnet_explorer.sql b/src-tauri/src/sql/032_clear_seeded_mainnet_explorer.sql new file mode 100644 index 00000000..94421081 --- /dev/null +++ b/src-tauri/src/sql/032_clear_seeded_mainnet_explorer.sql @@ -0,0 +1,19 @@ +-- Clear the mainnet explorer URL that migration 009 used to seed. +-- +-- 009 originally seeded `explorer_api_url` with the mainnet explorer. It was +-- later changed to seed an empty string, so the runtime resolves the explorer +-- per the active profile's network (`Network::default_explorer_base_url`, G2) +-- and a testnet/regtest profile has no mainnet URL to inherit. That edit only +-- helps a database created after it: on an installation where 009 had already +-- run, the mainnet URL stayed in `settings`, and an explicit `explorer_api_url` +-- outranks the network default — so a testnet or regtest profile kept reading +-- from the mainnet explorer, which is the silent cross-network read the guard +-- exists to prevent. +-- +-- Only the exact value 009 seeded is removed. A URL the user typed themselves +-- is theirs, is not necessarily mainnet, and is left alone; the network guard +-- in `providers::explorer_client_from_settings` refuses it when it disagrees +-- with the profile's network. +DELETE FROM settings + WHERE key = 'explorer_api_url' + AND value = 'https://e.hnsfans.com'; diff --git a/src-tauri/src/tests/connection_tests.rs b/src-tauri/src/tests/connection_tests.rs index a0a8de54..99ab0fd9 100644 --- a/src-tauri/src/tests/connection_tests.rs +++ b/src-tauri/src/tests/connection_tests.rs @@ -39,11 +39,11 @@ fn test_migrations_run_idempotent() { // Running migrations again should be a no-op (idempotent). crate::db::migrations::run(&conn).unwrap(); - // Verify all 31 migrations are recorded. + // Verify all 32 migrations are recorded. let count: i64 = conn .query_row("SELECT COUNT(*) FROM schema_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(count, 31); + assert_eq!(count, 32); } #[test] diff --git a/src-tauri/src/tests/migration_tests.rs b/src-tauri/src/tests/migration_tests.rs index f9b36707..2919ddd2 100644 --- a/src-tauri/src/tests/migration_tests.rs +++ b/src-tauri/src/tests/migration_tests.rs @@ -68,7 +68,9 @@ fn test_schema_version_tracking() { // 029 (bid_auction_scope: bid index keyed by the auction's OPEN height), // 030 (bid_commitment_auction: commitments carry their auction too), // 031 (rescan_reveal_pairing: reveal values may sit on the wrong bid). - assert_eq!(count, 31); + // 032 (clear_seeded_mainnet_explorer: 009 seeded a mainnet URL that + // outranked the network default on every other network). + assert_eq!(count, 32); } #[test] @@ -194,3 +196,58 @@ fn test_connection_open() { let _ = std::fs::remove_dir_all(&dir); } + +// --- 032: the seeded mainnet explorer is cleared, a chosen one is kept --- + +/// Migration 009 seeded this value; 032 removes exactly it. +const SEEDED_MAINNET_EXPLORER: &str = "https://e.hnsfans.com"; + +#[test] +fn migration_032_clears_the_explorer_url_009_seeded() { + let conn = rusqlite::Connection::open_in_memory().unwrap(); + crate::db::migrations::run(&conn).unwrap(); + // Re-seed the way an installation that ran the original 009 looks, then + // replay 032 over it: migrations run once, so this is the state such a + // database is already in when the new build starts. + conn.execute( + "INSERT OR REPLACE INTO settings (key, value) VALUES ('explorer_api_url', ?1)", + [SEEDED_MAINNET_EXPLORER], + ) + .unwrap(); + conn.execute_batch(include_str!("../sql/032_clear_seeded_mainnet_explorer.sql")) + .unwrap(); + + let remaining: i64 = conn + .query_row( + "SELECT COUNT(*) FROM settings WHERE key = 'explorer_api_url'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + remaining, 0, + "the seeded mainnet URL must be gone so the network default applies" + ); +} + +#[test] +fn migration_032_keeps_an_explorer_url_the_user_chose() { + let conn = rusqlite::Connection::open_in_memory().unwrap(); + crate::db::migrations::run(&conn).unwrap(); + conn.execute( + "INSERT OR REPLACE INTO settings (key, value) VALUES ('explorer_api_url', ?1)", + ["https://explorer.example.test"], + ) + .unwrap(); + conn.execute_batch(include_str!("../sql/032_clear_seeded_mainnet_explorer.sql")) + .unwrap(); + + let value: String = conn + .query_row( + "SELECT value FROM settings WHERE key = 'explorer_api_url'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(value, "https://explorer.example.test"); +} diff --git a/src-tauri/src/tests/provider_hnsfans_tests.rs b/src-tauri/src/tests/provider_hnsfans_tests.rs index 199080f1..0ea1935d 100644 --- a/src-tauri/src/tests/provider_hnsfans_tests.rs +++ b/src-tauri/src/tests/provider_hnsfans_tests.rs @@ -863,3 +863,75 @@ async fn test_explorer_provider_trait_delegates_get_address_txids() { // exercises the delegation. let _ = result; } + +// --- The seeded mainnet explorer must not leak onto another network (032) --- + +/// The value migration 009 used to seed into `explorer_api_url`. Installations +/// that ran that version still carry it, and an explicit setting outranks the +/// network default, so it has to be refused rather than trusted. +const SEEDED_MAINNET_EXPLORER: &str = "https://e.hnsfans.com"; + +#[test] +fn seeded_mainnet_explorer_is_refused_off_mainnet() { + let mut settings = std::collections::HashMap::new(); + settings.insert( + "explorer_api_url".to_string(), + SEEDED_MAINNET_EXPLORER.to_string(), + ); + + for network in [ + crate::noncustodial::network::Network::Testnet, + crate::noncustodial::network::Network::Regtest, + crate::noncustodial::network::Network::Simnet, + ] { + assert!( + crate::providers::explorer_client_from_settings(&settings, network).is_none(), + "{network:?} must not fall back to the mainnet explorer" + ); + } +} + +#[test] +fn seeded_mainnet_explorer_is_refused_off_mainnet_with_a_trailing_slash() { + // The same URL with hsd's habitual trailing slash is the same explorer. + let mut settings = std::collections::HashMap::new(); + settings.insert( + "explorer_api_url".to_string(), + format!("{SEEDED_MAINNET_EXPLORER}/"), + ); + assert!(crate::providers::explorer_client_from_settings( + &settings, + crate::noncustodial::network::Network::Regtest, + ) + .is_none()); +} + +#[test] +fn the_mainnet_explorer_is_still_used_on_mainnet() { + let mut settings = std::collections::HashMap::new(); + settings.insert( + "explorer_api_url".to_string(), + SEEDED_MAINNET_EXPLORER.to_string(), + ); + assert!(crate::providers::explorer_client_from_settings( + &settings, + crate::noncustodial::network::Network::Main, + ) + .is_some()); +} + +#[test] +fn a_users_own_explorer_url_is_honoured_off_mainnet() { + // Only the known mainnet explorer is refused. The wallet cannot tell which + // chain a private explorer serves, so it takes the user at their word. + let mut settings = std::collections::HashMap::new(); + settings.insert( + "explorer_api_url".to_string(), + "https://explorer.example.test".to_string(), + ); + assert!(crate::providers::explorer_client_from_settings( + &settings, + crate::noncustodial::network::Network::Regtest, + ) + .is_some()); +} From dd90548062a7e6d0260aa4f476b1a6ad2ad2305d Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 15:55:33 +0200 Subject: [PATCH 02/26] fix(sync): an unreadable profile network is unknown, not mainnet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CODING_STANDARDS says `Option` means "unknown", not "false", and that callers act only on a definite answer. Three sync steps broke that rule the same way: they resolved the profile's network with get_wallet_profile(..).ok().flatten().and_then(..).unwrap_or_default() and `Network` derives `Default = Main`. So a profile row that fails to load resolved to mainnet and the step carried on — picking the mainnet explorer for a wallet that may be on neither. That is the silent cross-network read the G2 gate exists to close, and the CHANGELOG entry for it claims cannot happen. The trigger is a DB read failure or a profile that disappears mid-sync, not an everyday path, so this is the rule being applied rather than a bug anyone has hit. It is still the wrong answer to give. The three copies of the chain are now one helper on `commands/active_profile.rs` — the module the standard names as the home for this, and already the place that turns a profile into a `Network`. It returns `Option`: missing profile, DB error and an unparseable string are one answer, because every caller refuses rather than guesses. The existing `active_profile_network_from_conn` keeps defaulting to mainnet for callers that only need a label. The repair and discover steps now report no explorer through the path they already had, and their message says both reasons it can fire rather than only the configured-URL one. The SPV step returns early and says which profile it could not read. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2575 passed). --- src-tauri/src/commands/active_profile.rs | 19 +++++++++ src-tauri/src/commands/sync.rs | 32 ++++++++------- src-tauri/src/commands/sync_spv.rs | 15 ++++--- src-tauri/src/tests/active_profile_tests.rs | 43 +++++++++++++++++++++ 4 files changed, 89 insertions(+), 20 deletions(-) diff --git a/src-tauri/src/commands/active_profile.rs b/src-tauri/src/commands/active_profile.rs index 0a978f2e..264d9309 100644 --- a/src-tauri/src/commands/active_profile.rs +++ b/src-tauri/src/commands/active_profile.rs @@ -48,3 +48,22 @@ pub(crate) fn active_profile_network_opt(state: &AppState) -> Option { let conn = state.db.lock().ok()?; active_profile_network_opt_from_conn(&conn) } + +/// The `Network` of one *named* profile, or `None` when the profile is +/// missing, the DB errors, or the stored string does not parse. +/// +/// The three failures collapse into one answer on purpose: every caller of +/// this refuses to act rather than guess, and the guess would be mainnet. A +/// sync step that guessed would read another chain's explorer into this +/// profile's cache, which is the cross-network read the network guard exists +/// to prevent. Callers that only need a *label* should keep using +/// [`active_profile_network_from_conn`]. +pub(crate) fn profile_network_opt_from_conn( + conn: &rusqlite::Connection, + profile_id: &str, +) -> Option { + db::queries::get_wallet_profile(conn, profile_id) + .ok() + .flatten() + .and_then(|p| Network::from_str_opt(&p.network)) +} diff --git a/src-tauri/src/commands/sync.rs b/src-tauri/src/commands/sync.rs index a40cda9d..f32a3493 100644 --- a/src-tauri/src/commands/sync.rs +++ b/src-tauri/src/commands/sync.rs @@ -822,12 +822,14 @@ pub async fn repair_step_windowed( Err(_) => return, }; // G2: resolve the profile's network to gate the explorer availability. - let network = queries::get_wallet_profile(&conn, profile_id) - .ok() - .flatten() - .and_then(|p| crate::noncustodial::derivation::network_from_profile(&p.network).ok()) - .unwrap_or_default(); - let explorer_opt = crate::providers::explorer_client_from_settings(&settings, network); + // A network that cannot be read is unknown, not mainnet — guessing + // would point this step at another chain's explorer and write its + // answers into this profile's cache. + let explorer_opt = + crate::commands::active_profile::profile_network_opt_from_conn(&conn, profile_id) + .and_then(|network| { + crate::providers::explorer_client_from_settings(&settings, network) + }); let addrs = queries::get_profile_addresses(&conn, profile_id).unwrap_or_default(); // Total backlog counted once: the stable "/ N" denominator for progress. let total = @@ -842,7 +844,7 @@ pub async fn repair_step_windowed( None => { let mut s = status.lock().await; s.errors.push( - "explorer unavailable for this network — configure explorer_api_url or wait for the local node to sync" + "no explorer for this profile — its network could not be read, or the network has no explorer configured; set explorer_api_url or wait for the local node to sync" .to_string(), ); return; @@ -1100,12 +1102,14 @@ pub async fn discover_step(status: &Arc>, db_path: &str, profi Err(_) => return, }; // G2: resolve the profile's network to gate the explorer availability. - let network = queries::get_wallet_profile(&conn, profile_id) - .ok() - .flatten() - .and_then(|p| crate::noncustodial::derivation::network_from_profile(&p.network).ok()) - .unwrap_or_default(); - let explorer_opt = crate::providers::explorer_client_from_settings(&settings, network); + // A network that cannot be read is unknown, not mainnet — guessing + // would point this step at another chain's explorer and write its + // answers into this profile's cache. + let explorer_opt = + crate::commands::active_profile::profile_network_opt_from_conn(&conn, profile_id) + .and_then(|network| { + crate::providers::explorer_client_from_settings(&settings, network) + }); let addrs = queries::get_profile_addresses(&conn, profile_id).unwrap_or_default(); (explorer_opt, addrs) }; @@ -1117,7 +1121,7 @@ pub async fn discover_step(status: &Arc>, db_path: &str, profi None => { let mut s = status.lock().await; s.errors.push( - "explorer unavailable for this network — configure explorer_api_url or wait for the local node to sync" + "no explorer for this profile — its network could not be read, or the network has no explorer configured; set explorer_api_url or wait for the local node to sync" .to_string(), ); return; diff --git a/src-tauri/src/commands/sync_spv.rs b/src-tauri/src/commands/sync_spv.rs index a5773f1e..1c41e0ee 100644 --- a/src-tauri/src/commands/sync_spv.rs +++ b/src-tauri/src/commands/sync_spv.rs @@ -48,12 +48,15 @@ pub async fn sync_spv_step(db_path: &str, profile_id: &str) -> bool { Err(_) => return false, // lcov-excl-line }; // G2: resolve the profile's network (before dropping the connection) to - // gate the explorer availability below. - let network = queries::get_wallet_profile(&conn, profile_id) - .ok() - .flatten() - .and_then(|p| crate::noncustodial::derivation::network_from_profile(&p.network).ok()) - .unwrap_or_default(); + // gate the explorer availability below. A network that cannot be read is + // unknown, not mainnet — an SPV step that guessed would show the user a + // mainnet balance for a wallet that is not on mainnet. + let Some(network) = + crate::commands::active_profile::profile_network_opt_from_conn(&conn, profile_id) + else { + eprintln!("sync_spv_step: profile {profile_id} has no readable network"); + return false; + }; // Verify the SPV node is reachable. Resolve the profile's effective node // config (per-profile override -> global -> default) rather than reading // global settings directly for the node endpoint. The explorer factory diff --git a/src-tauri/src/tests/active_profile_tests.rs b/src-tauri/src/tests/active_profile_tests.rs index 2e750fa2..d65e4aca 100644 --- a/src-tauri/src/tests/active_profile_tests.rs +++ b/src-tauri/src/tests/active_profile_tests.rs @@ -90,3 +90,46 @@ fn the_optional_form_reads_the_active_profile_network() { Some(Network::Regtest) ); } + +// --- profile_network_opt_from_conn: a named profile, unknown means unknown --- + +#[test] +fn a_named_profiles_network_is_read_without_the_active_profile() { + // The sync steps work on a profile id they were handed, which need not be + // the active one. + let conn = db(); + seed_profile(&conn, "p-regtest", "regtest"); + seed_profile(&conn, "p-main", "mainnet"); + set_active_profile(&conn, "p-main").unwrap(); + assert_eq!( + crate::commands::active_profile::profile_network_opt_from_conn(&conn, "p-regtest"), + Some(Network::Regtest) + ); +} + +#[test] +fn a_missing_profile_reads_as_unknown_not_mainnet() { + let conn = db(); + assert_eq!( + crate::commands::active_profile::profile_network_opt_from_conn(&conn, "nobody"), + None + ); +} + +#[test] +fn an_unparseable_network_string_reads_as_unknown_not_mainnet() { + let conn = db(); + // Same smuggling as the fallback test above: the schema's CHECK keeps + // unknown networks out, so this branch is defensive (an older DB, a future + // network name) rather than a state the app can produce. + conn.pragma_update(None, "ignore_check_constraints", true) + .unwrap(); + seed_profile(&conn, "p1", "weirdnet"); + conn.pragma_update(None, "ignore_check_constraints", false) + .unwrap(); + assert_eq!( + crate::commands::active_profile::profile_network_opt_from_conn(&conn, "p1"), + None, + "unknown must not resolve to mainnet for a step that acts on the answer" + ); +} From ff0f1a76ab1a64bb469dce634715520f25e8c212 Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 16:05:26 +0200 Subject: [PATCH 03/26] fix(names): a failed lookup is not the same as nothing to reveal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `find_name_action_context` gathers the evidence that decides whether Reveal, Redeem and the ownership actions are offered, and five of its lookups degraded a failure into an empty answer. It returns `Result<_, AppError>`, so every one of them could have said so. The worst is the name hash. `hash_name` fails only on a name that is not valid, and the fallback substituted `[0u8; 32]` — a hash that matches no coin. Both coin queries below it are keyed by that hash, so any name that reached the fallback reported no reveal coins and no owner coin, which the capability model reads as "nothing to redeem" on a wallet that may have lockups sitting in losing reveals. `list_bid_commitments` failing the same way withdraws Reveal and Redeem from a wallet that has bid. CODING_STANDARDS: a DB failure in a user-triggered command is returned, not swallowed into a default. The bid-coin search was a `find_map` swallowing each commitment's error in turn; it is now a loop that stops at the first coin and returns the first error. This surfaced a test fixture that the zero hash had been hiding: `batch_capabilities_respects_wallet_profile_isolation` seeded "nameforA" and "nameforB", and Handshake names are lowercase — `verify_name` refuses an uppercase letter, so those rows were never hashable. Renamed to valid names; the assertions are unchanged. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2576 passed). --- src-tauri/src/commands/names.rs | 33 +++++++++++-------- .../src/tests/names_action_context_tests.rs | 22 +++++++++++++ src-tauri/src/tests/names_cmd_tests.rs | 12 +++---- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index a79b5889..c30003a8 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -524,8 +524,10 @@ pub(crate) fn find_name_action_context( auction_start: Option, ) -> Result { // Newest first, so the first match is the most recent bid in this auction. - let for_name: Vec = queries::list_bid_commitments(conn, profile_id) - .unwrap_or_default() + // A DB failure is returned, not read as "this wallet has never bid": the + // capabilities built from it decide whether Reveal and Redeem are offered, + // and an empty list withdraws both from a wallet that has money locked up. + let for_name: Vec = queries::list_bid_commitments(conn, profile_id)? .into_iter() .filter(|b| b.name == name) .collect(); @@ -583,7 +585,11 @@ pub(crate) fn find_name_action_context( // lands back on the bid coin's own address, see `build_reveal_draft`), so // only the covenant type differs between the two queries below. // - let name_hash_hex = hex::encode(names::hash_name(name).unwrap_or([0u8; 32])); + // A zero hash is not a name. `hash_name` only fails on a name that is not + // valid, and the coin lookups below are keyed by this hash — so the + // fallback answered "no reveal coins" for every name, which reads as + // nothing to redeem. + let name_hash_hex = hex::encode(names::hash_name(name)?); // Every bid of THIS auction, not the newest one and not every bid the // profile has ever placed on the name. Both wrong answers were live: // picking one commitment's address was never well defined (`created_at` @@ -595,28 +601,27 @@ pub(crate) fn find_name_action_context( // with "no unspent bid coin". This is the same set `build_reveal_draft` // builds its transaction from, so the button and the builder cannot // disagree. - let bid_coin = commitments.iter().find_map(|b| { - queries::find_unspent_covenant_utxo( + let mut bid_coin = None; + for b in &commitments { + if let Some(coin) = queries::find_unspent_covenant_utxo( conn, profile_id, &b.address, sync::COV_BID as i64, name, &b.name_hash_hex, - ) - .ok() - .flatten() - }); + )? { + bid_coin = Some(coin); + break; + } + } let reveal_coins = queries::find_unspent_covenant_utxos_by_name_hash( conn, profile_id, COV_REVEAL as i64, &name_hash_hex, - ) - .unwrap_or_default(); - let owner_coin = queries::get_name_coin(conn, profile_id, name) - .ok() - .flatten(); + )?; + let owner_coin = queries::get_name_coin(conn, profile_id, name)?; // A reveal coin that is NOT the name's owner is a losing bid this wallet // can still reclaim. Outbidding yourself leaves exactly this: you own the // name AND hold losing reveals on it. diff --git a/src-tauri/src/tests/names_action_context_tests.rs b/src-tauri/src/tests/names_action_context_tests.rs index b0b09ea5..07907549 100644 --- a/src-tauri/src/tests/names_action_context_tests.rs +++ b/src-tauri/src/tests/names_action_context_tests.rs @@ -1188,3 +1188,25 @@ mod rpc_injected_tests { ); } } + +// --- A name that cannot be hashed is an error, not an empty answer --- + +#[test] +fn an_invalid_name_is_refused_rather_than_read_as_nothing_to_reveal() { + // The coin lookups are keyed by the name's hash. Hashing an invalid name + // fails, and the old fallback substituted a zero hash — which matches no + // coin, so every name that reached it reported no reveal coins and no + // owner coin. That is the same answer as "you have nothing to redeem" on + // a wallet that may have money locked up. + let conn = test_db(); + seed_profile(&conn); + seed_derived_address(&conn, ADDRESS, 0, 0); + + let Err(err) = find_name_action_context(&conn, PROFILE, "NotAName", None) else { + panic!("an unhashable name must not resolve to an empty context"); + }; + assert!( + format!("{err:?}").contains("NotAName"), + "the error should name what it refused, got: {err:?}" + ); +} diff --git a/src-tauri/src/tests/names_cmd_tests.rs b/src-tauri/src/tests/names_cmd_tests.rs index 20f86a12..491c5567 100644 --- a/src-tauri/src/tests/names_cmd_tests.rs +++ b/src-tauri/src/tests/names_cmd_tests.rs @@ -1404,12 +1404,12 @@ async fn batch_capabilities_respects_wallet_profile_isolation() { |r| r.get(0), ) .unwrap(); - // Wallet A owns "nameforA"; wallet B owns "nameforB" — each row's + // Wallet A owns "namefora"; wallet B owns "nameforb" — each row's // owner_address only matches its own wallet's derived address. conn.execute( "INSERT INTO tracked_name_states (wallet_profile_id, name, name_hash_hex, state, owner_txid, owner_vout, owner_address, height) - VALUES (?1, 'nameforA', 'aabb', 'CLOSED', ?2, 0, ?3, 100)", + VALUES (?1, 'namefora', 'aabb', 'CLOSED', ?2, 0, ?3, 100)", rusqlite::params![ &a, "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", @@ -1420,7 +1420,7 @@ async fn batch_capabilities_respects_wallet_profile_isolation() { conn.execute( "INSERT INTO tracked_name_states (wallet_profile_id, name, name_hash_hex, state, owner_txid, owner_vout, owner_address, height) - VALUES (?1, 'nameforB', 'ccdd', 'CLOSED', ?2, 0, ?3, 100)", + VALUES (?1, 'nameforb', 'ccdd', 'CLOSED', ?2, 0, ?3, 100)", rusqlite::params![ &b, "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", @@ -1432,7 +1432,7 @@ async fn batch_capabilities_respects_wallet_profile_isolation() { }; let app = mock_app_with(state); - let names = vec!["nameforA".to_string(), "nameforB".to_string()]; + let names = vec!["namefora".to_string(), "nameforb".to_string()]; let as_a = names::get_names_action_capabilities(app.state(), names.clone(), Some(profile_a)) .await @@ -1444,7 +1444,7 @@ async fn batch_capabilities_respects_wallet_profile_isolation() { .collect(); assert_eq!( owned_by_a, - vec!["nameforA"], + vec!["namefora"], "profile A must only see its own name as owned" ); @@ -1458,7 +1458,7 @@ async fn batch_capabilities_respects_wallet_profile_isolation() { .collect(); assert_eq!( owned_by_b, - vec!["nameforB"], + vec!["nameforb"], "profile B must only see its own name as owned" ); } From db2aa8d55ba5ac4be64f1cc95c42019737c5b83f Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 16:13:28 +0200 Subject: [PATCH 04/26] fix(names): the rest of the context builder says when a lookup failed Follows the previous commit through the same function. Five more lookups in `find_name_action_context` turned a DB failure into an answer: - the pending-OPEN coin and draft checks read a failure as `false`, so a broadcast OPEN waiting for a block stopped suppressing the button that would send a second one; - `pending_broadcast_actions_for_name` read a failure as "nothing in flight", which is what every "waiting for a block" label is derived from; - the reveal draft's status read a failure as "no draft", which is the branch that falls back to chain facts; - `get_tracked_name_state` and `estimate_persisted_height` read a failure as "unknown", and unknown deliberately widens the transfer-lockup gate: the gate offers FINALIZE when it cannot see the chain, so a swallowed error offered an action consensus would refuse. `estimate_persisted_height` was already propagated with `?` further down this same file, so the two call sites disagreed about whether its failure mattered. Hashing the name moved out of the `.ok()` chain it was hiding in and is now the same `?` the previous commit introduced, which also flattens the nested closure the fallback needed. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2576 passed). --- src-tauri/src/commands/names.rs | 41 ++++++++++++++------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index c30003a8..fb38901c 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -662,24 +662,16 @@ pub(crate) fn find_name_action_context( // permanently un-openable — including one whose auction had since lapsed // and which the chain now reports as available again. A live auction is // already handled by the phase check in `build_name_action_capabilities`. - let has_pending_open_coin = names::hash_name(name) - .ok() - .map(hex::encode) - .map(|nh_hex| { - queries::has_unconfirmed_covenant_utxo_by_name_hash( - conn, - profile_id, - sync::COV_OPEN as i64, - &nh_hex, - ) - .unwrap_or(false) - }) - .unwrap_or(false); + let has_pending_open_coin = queries::has_unconfirmed_covenant_utxo_by_name_hash( + conn, + profile_id, + sync::COV_OPEN as i64, + &hex::encode(names::hash_name(name)?), + )?; let has_pending_open_draft = - queries::has_pending_draft_for_name(conn, profile_id, "open", name).unwrap_or(false); + queries::has_pending_draft_for_name(conn, profile_id, "open", name)?; let has_pending_open = has_pending_open_coin || has_pending_open_draft; - let pending_actions = - queries::pending_broadcast_actions_for_name(conn, profile_id, name).unwrap_or_default(); + let pending_actions = queries::pending_broadcast_actions_for_name(conn, profile_id, name)?; let pending_broadcast_action = pending_actions.first().cloned(); // `get_name_coin` answers "can we spend it" and returns unspent coins @@ -720,21 +712,22 @@ pub(crate) fn find_name_action_context( // dropped/failed; when there's no draft (restored/cross-device wallet), the // caller falls back to the bid-coin-spent chain fact. let reveal_txid = bid.as_ref().and_then(|b| b.reveal_txid.clone()); - let reveal_draft_status = reveal_txid.as_ref().and_then(|txid| { - queries::get_draft_status_by_txid(conn, profile_id, txid) - .ok() - .flatten() - }); + let reveal_draft_status = match reveal_txid.as_ref() { + Some(txid) => queries::get_draft_status_by_txid(conn, profile_id, txid)?, + None => None, + }; let bid_value_doos = bid.as_ref().map(|b| b.bid_value_doos); let lockup_value_doos = bid.as_ref().map(|b| b.lockup_value_doos); - let tracked_row = queries::get_tracked_name_state(conn, profile_id, name).unwrap_or(None); + let tracked_row = queries::get_tracked_name_state(conn, profile_id, name)?; let transfer_height = tracked_row .as_ref() .and_then(|t| t.transfer_height) .filter(|h| *h > 0); - let current_height = - crate::commands::read::estimate_persisted_height(conn, profile_id).unwrap_or(None); + // The same call is already propagated further down this file; a failure + // here read as "height unknown", which quietly widens every gate that + // compares a height against the tip. + let current_height = crate::commands::read::estimate_persisted_height(conn, profile_id)?; Ok(NameActionContext { has_bid_commitment: bid.is_some(), From 147aef7de36d8b7b3d7a2b66fecd5e5722054841 Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 16:54:12 +0200 Subject: [PATCH 05/26] fix(node): finish routing node config through the profile (ADR-001, Step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec called Step 2 done — "route every site that reads node config from global settings through the resolver; realign now runs only on fallback resolution" — and several sites still read global. `start_hsd` was the plainest: it spawned hsd with the api-key from global settings while every read and broadcast used the profile's, so a profile with its own key started a node it could not then talk to. It also ran `realign_loopback_rpc_url` unconditionally, which is what ADR-001's "Interaction with N11" exists to forbid: an override is an explicit choice, and rewriting it is the silent abandonment ADR-002 refuses. Both now come from `effective_node_config_for_profile`. `EffectiveNodeConfig` was written for exactly this and had no reader outside its own module. `from_override` meant "differs from global", not "the profile chose it", so pinning a profile to the value global happens to hold read as no override at all — and global can change under the user afterwards. It now means what its name says. Realign needs a narrower question than that one, since it rewrites the URL and nothing else, so `url_from_override` answers it: overriding only `chain_source` no longer protects a stale global loopback port from being fixed. Keys outside the node-config tuple no longer count as a node override either. Six paths fell back to the global node when a profile's own config would not resolve, which ADR-001 names as the thing not to do: "a per-profile override that fails is not a fallback — it is a user-facing error". The status probe and the per-profile tip gate now report "cannot tell", and `stop_hsd`, the paid-swap check and the action history return the error. Fee estimation falls through to the built-in rate, because an estimate from another chain's node is worse than no estimate. That rewrite surfaced how those branches actually worked: `get_active_profile_id` returns `Ok("")` when there is no active profile and never `Err`, so the "no profile" case had been arriving through `for_profile("")` failing and being swallowed. It is now an explicit arm, and the `Err` arm means what it says — a DB failure. The background sync gated "is the node authoritative?" on the global node while every step it governs built a per-profile client, so the answer described a node nobody was going to talk to. It now asks the same per-profile gate the daemons use, which #55 added and left unused here. `node_config` ran raw SQL; both statements moved to `db::queries`, and the `get_profile_settings` test moved with the helper. Docs brought in line with the code: the spec carries a per-step status table (Steps 1-2 of 9 done, and nothing writes `profile_settings` outside tests yet, so the override is not reachable by a user), the specs README lists it, and its dead prototype pointer is gone. CONTEXT.md described per-slot read/write overrides and slot-independent resolution as current behaviour; there is no slot column and the resolver returns one tuple, so that is now marked planned. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2581 passed), npx vitest run (890 passed), npm run lint:format. --- CHANGELOG.md | 1 + CONTEXT.md | 9 +- ...5-per-profile-node-banner-and-preflight.md | 19 ++++- docs/specs/README.md | 1 + src-tauri/src/commands/history.rs | 13 +-- src-tauri/src/commands/node.rs | 84 +++++++++++-------- src-tauri/src/commands/paid_swaps.rs | 17 ++-- src-tauri/src/commands/read.rs | 11 ++- src-tauri/src/commands/sync.rs | 30 +++---- src-tauri/src/commands/tx.rs | 48 ++++++----- src-tauri/src/db/queries.rs | 36 ++++++++ src-tauri/src/noncustodial/node_config.rs | 68 +++++++-------- src-tauri/src/noncustodial/rpc.rs | 8 +- src-tauri/src/tests/node_config_tests.rs | 61 +++++++++++++- src-tauri/src/tests/node_status_tests.rs | 34 ++++++++ 15 files changed, 294 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8bac1a9..a1e3f75a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and — once a wallet profile exists — whether the node's network matches the wallet's; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **Each wallet profile now really uses its own node.** Per-profile node configuration was stored and resolved, but several paths still asked the global settings: the node the app started took the global api-key, the background sync decided whether "the node" was caught up by asking the global one while every step it gated talked to the profile's, and a sync failure named the global URL in a message telling you to go fix it. Worse, a profile whose own node configuration would not resolve was quietly served the global node instead of being reported as misconfigured, so a profile pinned to a regtest node could be answered by a mainnet one. Each of those now resolves through the profile, and an unresolvable profile configuration is an error rather than somebody else's node. The automatic repair of a stale loopback port also stops rewriting a URL you set on the profile yourself. - **A testnet or regtest wallet no longer reads from the mainnet explorer.** An early migration seeded `explorer_api_url` with the mainnet explorer, and a later change swapped that seed for an empty string so the wallet could pick an explorer per the profile's network. The swap only helped databases created after it: every installation that had already run the original kept the mainnet URL, and an explicit setting outranks the network default, so a testnet or regtest profile went on querying mainnet and getting confident answers about a chain it was not on. A new migration removes exactly the value the old one seeded, leaving an explorer URL you chose yourself alone, and the explorer factory now refuses the known mainnet explorer off mainnet in case a database reaches it before the migration has run. - **Reveal is no longer offered on a name with nothing left to reveal.** A lockup stranded in an auction that lapsed is an unspent BID coin, and the Reveal button was gated on holding one of those anywhere in the wallet rather than in the auction being looked at. So on a fully revealed name the button stayed live for good and failed every time with "no unspent bid coin". It now reads the same set the reveal transaction is built from, so the button and the builder cannot disagree. - **Registering no longer looks as though DNS records are required.** The Register step put a record editor in front of the user and said nothing about it. Records are optional — Handshake accepts an empty resource, and the wallet was already sending one when the editor was untouched — so the panel now says registering claims the name and records can follow with Update, and keeps the editor behind "Add DNS records now (optional)". Register also stopped appearing twice, live in both the guided step and the records section, and the manual auction actions now show only what the stage allows, each with a line saying what pressing it does — Redeem names the amount it reclaims. diff --git a/CONTEXT.md b/CONTEXT.md index 195572ec..2c41526f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -28,9 +28,10 @@ _Avoid_: Sync mode The tuple `(node_rpc_url, node_rpc_api_key, chain_source)` that tells the wallet how to reach a node. Can be global (applies to all profiles) or per-profile (applies only to that profile). Per-profile overrides the global. _Avoid_: Connection settings (too vague) -**Per-slot per-profile override**: -A per-profile override of a single slot's source that takes precedence over the global default for that slot. A profile resolves two independent slots — the read slot and the write (send) slot — and each carries its own override flag. A profile may override its read slot (e.g. read via explorer) while inheriting the write slot from global, or vice versa. Stored in `profile_settings` table. If an override is set and invalid (unreachable, mismatched network), it is a configuration error for that slot, not a fallback to global. -_Avoid_: Per-profile override (imprecise now that override is per-slot), profile-specific node, profile node setting +**Per-profile override**: +A per-profile choice of node configuration that takes precedence over the global default. Stored in the `profile_settings` table, one row per key. If an override is set and invalid (unreachable, mismatched network), it is a configuration error for that profile, not a fallback to global. +_Planned, not built_: splitting this per *slot*, so a profile could override its read slot (e.g. read via explorer) while inheriting the write slot from global. The table has no slot column and resolution returns one tuple; see step 8 of the [per-profile node spec](./docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md). Until then "per-slot per-profile override" names something the wallet does not do. +_Avoid_: Profile-specific node, profile node setting **Preflight**: A check performed before an operation (sync, read, broadcast) to ensure the node is reachable and on the correct network for the active profile. Returns a status (ready, missing, misconfigured) and optionally a suggested fix. @@ -41,5 +42,5 @@ The node's reported chain (from `getblockchaininfo`) does not match the profile' _Avoid_: Chain mismatch (same thing, but "network" is the profile's term) **Effective node config**: -The resolved sources for a profile after applying the resolution order per slot: per-slot per-profile override (if set) → global settings (if set) → built-in default. Resolution runs independently for the read slot and the write (send) slot, so the effective read source and effective write source can differ (e.g. read via explorer, write via a local node). Each resolved slot is a `(node_rpc_url, node_rpc_api_key, chain_source)` tuple; the write slot may resolve to none when no node is available to send through. +The resolved node configuration for a profile after applying the resolution order, evaluated independently per key: per-profile override (if set and non-empty) → global settings (if set) → built-in default. The result is one `(node_rpc_url, node_rpc_api_key, chain_source)` tuple, plus whether the profile's own override supplied it — which is what keeps the loopback-port realign off a URL the user chose. A profile that does not resolve is an error, never a fallback to global. _Avoid_: Resolved config (same meaning, but "effective" emphasizes the resolution order) diff --git a/docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md b/docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md index 5e7b4b3b..19e44cd5 100644 --- a/docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md +++ b/docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md @@ -8,9 +8,22 @@ on top of [ADR-001 Per-profile node configuration](../adr/0001-per-profile-node- the ADR defines *what is stored and how it resolves*, this spec defines *what the user sees and can do*. -The prototype at `prototypes/per-profile-node-banner/index.html` answered these -questions and is no longer needed — the verdicts below are the durable output -and will be implemented step by step in the real app. +A throwaway prototype answered these questions and was deleted with them; the +verdicts below are its durable output, implemented step by step in the real app. + +Progress against the plan at the bottom of this file: + +| Step | State | +|------|-------| +| 1. Schema + resolver | Done. `profile_settings` (migration 027), `effective_node_config_for_profile`. | +| 2. Migrate call sites | Done. Every node client resolves per profile; realign runs only on fallback resolution; an unresolvable profile config is an error, never a fallback to global. | +| 3. Preflight command | Not started. No `profile_preflight` exists. | +| 4. Banner UI | Not started. | +| 5. Preflight cache | Not started. | +| 6. Override modal | Not started — so nothing writes `profile_settings` outside tests, and the override is not yet reachable by a user. | +| 7. Write-path fresh preflight | Not started. | +| 8. Per-slot read/write overrides | Not started. The table has no slot column, and the resolver returns one tuple. | +| 9. E2E matrix test | Not started. | ## Vocabulary (from CONTEXT.md) diff --git a/docs/specs/README.md b/docs/specs/README.md index c1257220..44e41fcf 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -20,5 +20,6 @@ Each spec has these sections, in this order: |------|--------| | [2026-09-11 Remote-node connection & broadcast guard](./2026-09-11-remote-node-connection-and-broadcast-guard.md) | Implemented on `feat/spv-broadcast-guard-and-remote-node-onboarding` | | [2026-09-14 Network-derived behaviour](./2026-09-14-network-derived-behaviour.md) | Implemented on `feat/batch-transfer` | +| [2026-09-15 Per-profile node banner & preflight](./2026-09-15-per-profile-node-banner-and-preflight.md) | Steps 1-2 of 9 implemented — see the spec's own status table | | [2026-09-20 Multiple bids per name](./2026-09-20-multiple-bids-per-name.md) | Implemented on `feat/batch-reveal-redeem-finalize-ui` | | [2026-09-21 Paid name swaps](./2026-09-21-paid-name-swaps.md) | Not implemented — UI withdrawn, see spec | diff --git a/src-tauri/src/commands/history.rs b/src-tauri/src/commands/history.rs index 816e1de8..77c2c03a 100644 --- a/src-tauri/src/commands/history.rs +++ b/src-tauri/src/commands/history.rs @@ -35,7 +35,6 @@ use serde::Serialize; use tauri::State; use crate::commands::read::resolve_profile; -use crate::db::queries; use crate::error::AppError; use crate::noncustodial::sync::{ COV_BID, COV_CLAIM, COV_FINALIZE, COV_NONE, COV_OPEN, COV_REDEEM, COV_REGISTER, COV_RENEW, @@ -326,17 +325,13 @@ pub async fn read_action_history( }; // Snapshot addresses + build the node client under a short DB lock; drop before .await. - // Per-profile node override routing (ADR-001): if an active profile exists, - // use its effective node config; otherwise fall back to global settings. + // Per-profile node override routing (ADR-001): this profile's effective + // node config, and an error when it will not resolve. Falling back to + // global would list another node's view of this wallet's history. let (addresses, node) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; let addrs = load_wallet_addresses(&conn, &profile_id)?; - let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, &profile_id) - .unwrap_or_else(|_| { - // Fallback to global settings if profile config is missing or misconfigured. - let settings = queries::get_settings(&conn).unwrap_or_default(); - crate::noncustodial::rpc::NodeRpcClient::from_settings(&settings) - }); + let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, &profile_id)?; (addrs, client) }; if addresses.is_empty() { diff --git a/src-tauri/src/commands/node.rs b/src-tauri/src/commands/node.rs index 07b1a5d2..789ef684 100644 --- a/src-tauri/src/commands/node.rs +++ b/src-tauri/src/commands/node.rs @@ -238,19 +238,22 @@ async fn probe_node(state: &AppState) -> Option { // Per-profile node override routing (ADR-001): probe the active profile's // effective node config (per-profile override -> global settings -> // built-in default) so the tray/status reflects the active profile's node. - // Fall back to global settings when there is no active profile or its - // config is missing/misconfigured. + // With no active profile at all there is no override to honour, so global + // is the whole answer; a profile whose own config will not resolve is a + // configuration error, and this probe reports "cannot tell" rather than + // quietly describing a node the profile does not use. let client = { let db = state.db.lock().ok()?; match db::queries::get_active_profile_id(&db) { - Ok(profile_id) => NodeRpcClient::for_profile(&db, &profile_id).unwrap_or_else(|_| { - let settings = db::queries::get_settings(&db).unwrap_or_default(); - NodeRpcClient::from_settings(&settings) - }), - Err(_) => { + // No active profile — the id is stored as an empty string, not an + // error — so there is nothing that could override, and global is + // the whole answer. + Ok(id) if id.is_empty() => { let settings = db::queries::get_settings(&db).ok()?; NodeRpcClient::from_settings(&settings) } + Ok(profile_id) => NodeRpcClient::for_profile(&db, &profile_id).ok()?, + Err(_) => return None, } }; client @@ -469,31 +472,43 @@ pub async fn start_hsd(state: State<'_, AppState>) -> Result global -> hsd.conf) so the node + // we start and the node we talk to agree. Reading global settings here + // handed a profile with its own key a node started with somebody else's. + // `node_mode` stays global: it is not part of the ADR-001 tuple. + // + // The same resolution decides whether realign may touch the URL, so both + // are taken under one lock. let (api_key, node_mode) = { let db = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; + let profile_id = db::queries::get_active_profile_id(&db)?; + let effective = + crate::noncustodial::node_config::effective_node_config_for_profile(&db, &profile_id)?; let settings = db::queries::get_settings(&db)?; - let api_key = crate::noncustodial::rpc::resolve_node_api_key(&settings); let node_mode = crate::noncustodial::rpc::resolve_node_mode(&settings); - (api_key, node_mode) - }; - // hsd will listen on this network's RPC port, but `node_rpc_url` keeps - // whatever was seeded — the mainnet 12037. Left alone, the wallet starts a - // regtest node and then talks to a port nothing is listening on. Realign a - // stale loopback default now; a custom port or a remote host is left as the - // user set it (see `realign_loopback_rpc_url`). - { - let db = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; - let current = db::queries::get_settings(&db)? - .get("node_rpc_url") - .cloned() - .unwrap_or_default(); - if let Some(fixed) = crate::noncustodial::rpc::realign_loopback_rpc_url(¤t, network) { - db::queries::set_setting(&db, "node_rpc_url", &fixed)?; + // hsd will listen on this network's RPC port, but a globally seeded + // `node_rpc_url` keeps whatever it was given — the mainnet 12037. Left + // alone, the wallet starts a regtest node and then talks to a port + // nothing is listening on. Realign a stale loopback default now; a + // custom port or a remote host is left as the user set it (see + // `realign_loopback_rpc_url`). + // + // ADR-001 (Interaction with N11): a profile that supplies its own URL + // has made an explicit choice, and rewriting it — even a "stale + // default" — is the silent abandonment ADR-002 refuses. Realign + // therefore applies only to a URL that came from the global fallback, + // and it writes back to the global setting it came from. + if !effective.url_from_override { + if let Some(fixed) = + crate::noncustodial::rpc::realign_loopback_rpc_url(&effective.node_rpc_url, network) + { + db::queries::set_setting(&db, "node_rpc_url", &fixed)?; + } } - } + (effective.node_rpc_api_key, node_mode) + }; std::fs::create_dir_all(&data_dir) .map_err(|e| AppError::Other(format!("cannot create data dir {data_dir}: {e}")))?; @@ -694,19 +709,20 @@ pub async fn stop_hsd(state: State<'_, AppState>) -> Result<(), AppError> { // answering to stop. Best-effort: if nothing's reachable, that's fine. // Per-profile node override routing (ADR-001): stop the node the active // profile is actually pointed at (per-profile override -> global settings - // -> built-in default). Fall back to global settings when there is no - // active profile or its config is missing/misconfigured. + // -> built-in default). With no active profile there is no override to + // honour; a profile whose own config will not resolve is returned as the + // configuration error it is, because "stop the node" aimed at the wrong + // endpoint can stop somebody else's. let client = { let db = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; - match db::queries::get_active_profile_id(&db) { - Ok(profile_id) => NodeRpcClient::for_profile(&db, &profile_id).unwrap_or_else(|_| { - let settings = db::queries::get_settings(&db).unwrap_or_default(); - NodeRpcClient::from_settings(&settings) - }), - Err(_) => { + match db::queries::get_active_profile_id(&db)? { + // No active profile — the id is stored as an empty string, not an + // error — so there is nothing that could override. + id if id.is_empty() => { let settings = db::queries::get_settings(&db)?; NodeRpcClient::from_settings(&settings) } + profile_id => NodeRpcClient::for_profile(&db, &profile_id)?, } }; let _ = client.stop().await; diff --git a/src-tauri/src/commands/paid_swaps.rs b/src-tauri/src/commands/paid_swaps.rs index c490522d..df517294 100644 --- a/src-tauri/src/commands/paid_swaps.rs +++ b/src-tauri/src/commands/paid_swaps.rs @@ -304,17 +304,18 @@ pub async fn claim_paid_transfer( // Per-profile node override routing (ADR-001): a paid swap is verified // against a node on the active profile's network. Use the active // profile's effective node config (per-profile override -> global - // settings -> built-in default). Fall back to global settings when - // there is no active profile or its config is missing/misconfigured. - let client = match queries::get_active_profile_id(&db) { - Ok(profile_id) => NodeRpcClient::for_profile(&db, &profile_id).unwrap_or_else(|_| { - let settings = queries::get_settings(&db).unwrap_or_default(); - NodeRpcClient::from_settings(&settings) - }), - Err(_) => { + // settings -> built-in default). A profile whose config will not + // resolve is returned as the configuration error it is: this decides + // whether a payment arrived, and asking a node on another chain + // answers a different question. + let client = match queries::get_active_profile_id(&db)? { + // No active profile — the id is stored as an empty string, not an + // error — so there is nothing that could override. + id if id.is_empty() => { let settings = queries::get_settings(&db)?; NodeRpcClient::from_settings(&settings) } + profile_id => NodeRpcClient::for_profile(&db, &profile_id)?, }; (offer, client) }; diff --git a/src-tauri/src/commands/read.rs b/src-tauri/src/commands/read.rs index 9e984eaf..7d7283d9 100644 --- a/src-tauri/src/commands/read.rs +++ b/src-tauri/src/commands/read.rs @@ -182,12 +182,11 @@ pub(crate) async fn node_tip_height_if_synced_from_profile_with_network( Ok(c) => c, Err(_) => return None, }; - let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, profile_id) - .unwrap_or_else(|_| { - // Fallback to global settings if profile config is missing or misconfigured. - let settings = queries::get_settings(&conn).unwrap_or_default(); - crate::noncustodial::rpc::NodeRpcClient::from_settings(&settings) - }); + // ADR-001: a profile whose node config will not resolve is a configuration + // error for that profile, not a fallback to global. This gate answers + // "is this profile's node authoritative?", and global's node is not an + // answer to that question — it may be another chain entirely. + let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, profile_id).ok()?; node_tip_height_if_synced_with_client(&client, expected_network).await } diff --git a/src-tauri/src/commands/sync.rs b/src-tauri/src/commands/sync.rs index f32a3493..a55b087f 100644 --- a/src-tauri/src/commands/sync.rs +++ b/src-tauri/src/commands/sync.rs @@ -458,23 +458,19 @@ pub async fn run_sync_steps( let node_authoritative = if node_mode.is_spv() { false } else { - // The network belongs to the profile being synced, not to whichever - // profile happens to be active: a node on another chain must never be - // authoritative for this one. - let snapshot = open_conn(db_path).ok().and_then(|c| { - let settings = queries::get_settings(&c).ok()?; - let network = queries::get_wallet_profile(&c, profile_id) - .ok() - .flatten() - .map(|p| p.network); - Some((settings, network)) - }); - match snapshot { - Some((s, network)) => { - crate::commands::read::node_ready_from_settings(&s, network.as_deref()).await - } - None => false, - } + // Both the network and the node belong to the profile being synced, + // not to whichever profile happens to be active. The network, because + // a node on another chain must never be authoritative for this one. + // The node, because this profile may point at its own (ADR-001) — and + // the steps this gate governs already build their client per profile, + // so asking the global node whether it is caught up decided what to do + // with a node nobody was going to talk to. + let network = open_conn(db_path) + .ok() + .and_then(|c| queries::get_wallet_profile(&c, profile_id).ok().flatten()) + .map(|p| p.network); + crate::commands::read::node_ready_from_profile(db_path, profile_id, network.as_deref()) + .await }; // Step 2: Repair owned names from inventory + tracked (explorer only). diff --git a/src-tauri/src/commands/tx.rs b/src-tauri/src/commands/tx.rs index 31a89634..0a381026 100644 --- a/src-tauri/src/commands/tx.rs +++ b/src-tauri/src/commands/tx.rs @@ -215,20 +215,25 @@ pub(crate) async fn resolve_fee_rate(state: &State<'_, AppState>, fee_rate: Opti } // 2) Ask the node for an estimate — same behavior as before. // Per-profile node override routing (ADR-001): if an active profile - // exists, use its effective node config; otherwise fall back to global. - let client = if let Some(id) = profile_id { - match state.db.lock() { - Ok(conn) => NodeRpcClient::for_profile(&conn, &id) - .unwrap_or_else(|_| NodeRpcClient::from_settings(&s)), - Err(_) => NodeRpcClient::from_settings(&s), - } - } else { - NodeRpcClient::from_settings(&s) + // exists, use its effective node config; with no profile at all + // there is no override to honour, so global is the whole answer. + // A profile whose config will not resolve falls through to the + // built-in rate rather than to global's node: a fee estimate from + // another chain's node is worse than no estimate. + let client = match profile_id.filter(|id| !id.is_empty()) { + Some(id) => match state.db.lock() { + Ok(conn) => NodeRpcClient::for_profile(&conn, &id).ok(), + Err(_) => None, + }, + None => Some(NodeRpcClient::from_settings(&s)), }; - client - .estimate_smart_fee(6) - .await - .unwrap_or(send::DEFAULT_FEE_RATE_PER_BYTE) + match client { + Some(c) => c + .estimate_smart_fee(6) + .await + .unwrap_or(send::DEFAULT_FEE_RATE_PER_BYTE), + None => send::DEFAULT_FEE_RATE_PER_BYTE, + } } None => send::DEFAULT_FEE_RATE_PER_BYTE, } @@ -246,7 +251,7 @@ pub async fn sync_wallet_state( ) -> Result { // 1. Snapshot addresses + settings under the lock, then release it before // any network I/O. - let (profile_id, profile_network, addresses, settings, client) = { + let (profile_id, profile_network, addresses, client) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; let profile = match wallet_profile_id { Some(id) => db::queries::get_wallet_profile(&conn, &id)? @@ -285,13 +290,12 @@ pub async fn sync_wallet_state( } } } - let settings = db::queries::get_settings(&conn)?; // Per-profile node override routing (ADR-001): the sync must use the // profile's effective node config (override -> global -> default), not // the raw global settings. Build the client while the lock is held so // the effective-config resolver can read `profile_settings`. let client = NodeRpcClient::for_profile(&conn, &profile.id)?; - (profile.id, profile.network, addresses, settings, client) + (profile.id, profile.network, addresses, client) }; // Probe the node first. If it's unreachable, that's expected in explorer / @@ -326,13 +330,13 @@ pub async fn sync_wallet_state( } }; - // 2. Fetch coins per address (network I/O, no lock held). - let node_url = settings - .get("node_rpc_url") - .map(|s| s.as_str()) - .unwrap_or("the configured node"); + // 2. Fetch coins per address (network I/O, no lock held). The URL named in + // any failure is the one this client resolved through the profile, not the + // global setting: they differ whenever the profile overrides it, and the + // global one would send the user to fix a node that was never asked. + let node_url = client.node_url().to_string(); let (all_coins, txs) = - fetch_wallet_coins_and_txs_with_client(&client, &addresses, node_url).await?; + fetch_wallet_coins_and_txs_with_client(&client, &addresses, &node_url).await?; // Balances below are split on coinbase maturity, which is per-network. let sync_network = derivation::network_from_profile(&profile_network)?; diff --git a/src-tauri/src/db/queries.rs b/src-tauri/src/db/queries.rs index fdc09320..cbbf9e0a 100644 --- a/src-tauri/src/db/queries.rs +++ b/src-tauri/src/db/queries.rs @@ -2749,6 +2749,42 @@ pub fn set_bid_reveal_txid( Ok(()) } +/// Whether a wallet profile row exists. +/// +/// Node-config resolution (ADR-001) treats a missing profile as a hard error +/// rather than a fallback to global settings, so it has to ask this before it +/// merges anything. +pub fn wallet_profile_exists( + conn: &rusqlite::Connection, + profile_id: &str, +) -> Result { + Ok(conn.query_row( + "SELECT COUNT(*) > 0 FROM wallet_profiles WHERE id = ?1", + [profile_id], + |row| row.get(0), + )?) +} + +/// All `profile_settings` rows for one profile, as a key/value map. +/// +/// These are the per-profile overrides of ADR-001. An absent key means "no +/// choice made here", which resolution reads as "fall back to global". +pub fn get_profile_settings( + conn: &rusqlite::Connection, + profile_id: &str, +) -> Result, AppError> { + let mut stmt = conn.prepare("SELECT key, value FROM profile_settings WHERE profile_id = ?1")?; + let rows = stmt.query_map([profile_id], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + })?; + let mut map = std::collections::HashMap::new(); + for row in rows { + let (k, v) = row?; + map.insert(k, v); + } + Ok(map) +} + #[cfg(test)] mod noncustodial_query_tests { use super::*; diff --git a/src-tauri/src/noncustodial/node_config.rs b/src-tauri/src/noncustodial/node_config.rs index 59a7b225..7746319f 100644 --- a/src-tauri/src/noncustodial/node_config.rs +++ b/src-tauri/src/noncustodial/node_config.rs @@ -9,9 +9,8 @@ //! //! A per-profile override is an explicit user choice: when present it is used //! verbatim and never silently rewritten (e.g. `realign_loopback_rpc_url` is -//! skipped for an overridden URL — see [`EffectiveNodeConfig::from_override`]). - -use std::collections::HashMap; +//! skipped for an overridden URL — see +//! [`EffectiveNodeConfig::url_from_override`]). use crate::error::AppError; use crate::noncustodial::rpc::{resolve_node_api_key, ChainSource}; @@ -21,6 +20,12 @@ use crate::noncustodial::rpc::{resolve_node_api_key, ChainSource}; /// port for other networks when this comes from the global fallback. pub const DEFAULT_NODE_RPC_URL: &str = "http://127.0.0.1:12037"; +/// The `profile_settings` keys that make up the ADR-001 node-config tuple. +/// A profile may hold other per-profile settings; those say nothing about +/// which node this profile talks to, so they must not make the config read as +/// overridden. +const NODE_CONFIG_KEYS: [&str; 3] = ["node_rpc_url", "node_rpc_api_key", "chain_source"]; + /// The resolved node configuration for one profile. #[derive(Debug, Clone, PartialEq, Eq)] pub struct EffectiveNodeConfig { @@ -30,28 +35,18 @@ pub struct EffectiveNodeConfig { pub node_rpc_api_key: String, /// Resolved chain source. pub chain_source: ChainSource, - /// True when at least one node-config key came from the profile's - /// `profile_settings` override (as opposed to global/default). Callers use - /// this to skip `realign_loopback_rpc_url`, which must only touch a URL that - /// came from the global fallback. + /// True when the profile's own `profile_settings` supplied any key of the + /// node-config tuple — whatever value it holds. "The user chose this here", + /// not "this differs from global": a user who deliberately pins a profile + /// to the value global happens to carry today has still chosen it, and + /// global can change under them afterwards. pub from_override: bool, -} - -/// Read all `profile_settings` rows for a profile into a key/value map. -pub fn get_profile_settings( - conn: &rusqlite::Connection, - profile_id: &str, -) -> Result, AppError> { - let mut stmt = conn.prepare("SELECT key, value FROM profile_settings WHERE profile_id = ?1")?; - let rows = stmt.query_map([profile_id], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) - })?; - let mut map = HashMap::new(); - for row in rows { - let (k, v) = row?; - map.insert(k, v); - } - Ok(map) + /// True when the profile's own `profile_settings` supplied the resolved + /// `node_rpc_url`. This is the one realign must consult: it rewrites the + /// URL and nothing else, so an override of `chain_source` alone is no + /// reason to leave a stale global loopback port pointing at the wrong + /// network (ADR-001, "Interaction with N11"). + pub url_from_override: bool, } /// Resolve the effective node configuration for `profile_id`. @@ -65,33 +60,29 @@ pub fn effective_node_config_for_profile( profile_id: &str, ) -> Result { // A missing profile is a hard error, never a silent default (ADR-001). - let exists: bool = conn.query_row( - "SELECT COUNT(*) > 0 FROM wallet_profiles WHERE id = ?1", - [profile_id], - |row| row.get(0), - )?; - if !exists { + if !crate::db::queries::wallet_profile_exists(conn, profile_id)? { return Err(AppError::NotFound(format!("wallet profile {profile_id}"))); } let global = crate::db::queries::get_settings(conn)?; - let overrides = get_profile_settings(conn, profile_id)?; + let overrides = crate::db::queries::get_profile_settings(conn, profile_id)?; // Build a merged settings view (override -> global) so the existing // resolvers (`resolve_node_api_key`, `ChainSource::from_settings`) apply the // same precedence without re-implementing their rules. A non-empty override // wins; an empty override string is "no meaningful choice" and must not - // blank out a good global value. `from_override` is set when any override - // key actually changes the resolved value away from the global fallback. + // blank out a good global value. + let chosen_here = |key: &str| { + overrides + .get(key) + .map(|v| !v.trim().is_empty()) + .unwrap_or(false) + }; let mut merged = global.clone(); - let mut from_override = false; for (k, v) in &overrides { if v.trim().is_empty() { continue; } - if merged.get(k).map(String::as_str) != Some(v.as_str()) { - from_override = true; - } merged.insert(k.clone(), v.clone()); } @@ -106,6 +97,7 @@ pub fn effective_node_config_for_profile( node_rpc_url, node_rpc_api_key, chain_source, - from_override, + from_override: NODE_CONFIG_KEYS.iter().any(|k| chosen_here(k)), + url_from_override: chosen_here("node_rpc_url"), }) } diff --git a/src-tauri/src/noncustodial/rpc.rs b/src-tauri/src/noncustodial/rpc.rs index 92bcdb7a..83b55bfb 100644 --- a/src-tauri/src/noncustodial/rpc.rs +++ b/src-tauri/src/noncustodial/rpc.rs @@ -396,8 +396,12 @@ impl NodeRpcClient { self.source } - /// Test-only accessor for the resolved node URL (after trailing-slash trim). - #[cfg(test)] + /// The node URL this client actually talks to (after trailing-slash trim). + /// + /// A message that names the node must read it from here rather than from + /// global settings: a profile may resolve to its own endpoint (ADR-001), + /// and naming the global URL then sends the user to fix a node the failing + /// request never touched. pub fn node_url(&self) -> &str { &self.node_url } diff --git a/src-tauri/src/tests/node_config_tests.rs b/src-tauri/src/tests/node_config_tests.rs index aa7fb5bc..3a43c5d7 100644 --- a/src-tauri/src/tests/node_config_tests.rs +++ b/src-tauri/src/tests/node_config_tests.rs @@ -12,10 +12,9 @@ use rusqlite::Connection; +use crate::db::queries::get_profile_settings; use crate::error::AppError; -use crate::noncustodial::node_config::{ - effective_node_config_for_profile, get_profile_settings, DEFAULT_NODE_RPC_URL, -}; +use crate::noncustodial::node_config::{effective_node_config_for_profile, DEFAULT_NODE_RPC_URL}; use crate::noncustodial::rpc::ChainSource; /// A migrated in-memory DB with the migrations the resolver depends on, plus @@ -245,3 +244,59 @@ fn from_effective_config_carries_resolved_fields() { assert_eq!(client.node_url(), "https://node.example:12037"); assert_eq!(client.api_key(), "k"); } + +// --- Which flag realign is allowed to consult (ADR-001, N11) --- + +#[test] +fn an_override_equal_to_global_is_still_the_users_choice() { + // The flag used to mean "differs from global", so pinning a profile to the + // value global happens to hold today read as no override at all — and + // realign would then rewrite a URL the user had deliberately set. Global + // can also change under them afterwards. + let conn = db_with_profile(); + set_global(&conn, "node_rpc_url", "http://127.0.0.1:12037"); + set_override(&conn, "p1", "node_rpc_url", "http://127.0.0.1:12037"); + let cfg = effective_node_config_for_profile(&conn, "p1").unwrap(); + assert!(cfg.from_override, "choosing a value is choosing it"); + assert!(cfg.url_from_override, "realign must leave this URL alone"); +} + +#[test] +fn overriding_the_chain_source_alone_leaves_the_url_realignable() { + // Realign rewrites the URL and nothing else. A profile that only chose its + // chain source has expressed no opinion about the port, so a stale global + // loopback must still be fixable. + let conn = db_with_profile(); + set_global(&conn, "node_rpc_url", "http://127.0.0.1:12037"); + set_override(&conn, "p1", "chain_source", "remote_node"); + let cfg = effective_node_config_for_profile(&conn, "p1").unwrap(); + assert!(cfg.from_override, "the profile did choose something"); + assert!( + !cfg.url_from_override, + "the URL still came from the global fallback" + ); +} + +#[test] +fn a_per_profile_setting_outside_the_node_tuple_is_not_a_node_override() { + // `profile_settings` is a general per-profile store. A key that says + // nothing about which node this profile talks to must not make the node + // config read as overridden — and so must not disable realign. + let conn = db_with_profile(); + set_override(&conn, "p1", "some_unrelated_preference", "yes"); + let cfg = effective_node_config_for_profile(&conn, "p1").unwrap(); + assert!(!cfg.from_override); + assert!(!cfg.url_from_override); +} + +#[test] +fn an_empty_url_override_does_not_protect_the_url_from_realign() { + // An empty override is "no meaningful choice" for resolution, so it must + // be "no meaningful choice" for realign too, or the two disagree about + // whose URL is in play. + let conn = db_with_profile(); + set_global(&conn, "node_rpc_url", "http://127.0.0.1:12037"); + set_override(&conn, "p1", "node_rpc_url", ""); + let cfg = effective_node_config_for_profile(&conn, "p1").unwrap(); + assert!(!cfg.url_from_override); +} diff --git a/src-tauri/src/tests/node_status_tests.rs b/src-tauri/src/tests/node_status_tests.rs index 7d79d80c..7bff6f44 100644 --- a/src-tauri/src/tests/node_status_tests.rs +++ b/src-tauri/src/tests/node_status_tests.rs @@ -1427,6 +1427,40 @@ use crate::commands::node::{ PrefixMigration, }; +#[tokio::test] +async fn probe_refuses_a_profile_whose_node_config_will_not_resolve() { + // ADR-001: "a per-profile override that fails is not a fallback — it is a + // user-facing error". The probe used to swallow the failure and describe + // whatever node global happens to name, which on another chain is a + // confident answer about somebody else's node. + let mut server_global = mockito::Server::new_async().await; + let m = server_global + .mock("POST", "/") + .match_body(mockito::Matcher::Regex("getblockchaininfo".into())) + .with_body( + r#"{"result":{"blocks":99,"headers":99,"verificationprogress":1.0},"error":null,"id":1}"#, + ) + .expect(0) + .create_async() + .await; + + let conn = rusqlite::Connection::open_in_memory().unwrap(); + conn.execute_batch("PRAGMA foreign_keys = ON;").unwrap(); + db::migrations::run(&conn).unwrap(); + db::queries::set_setting(&conn, "node_rpc_url", &server_global.url()).unwrap(); + // An active id pointing at a profile that does not exist: resolution + // returns NotFound, which is exactly the "will not resolve" case. + db::queries::set_setting(&conn, "active_wallet_profile_id", "ghost").unwrap(); + + let app = app_with(conn); + let state = app.state::(); + assert!( + !crate::commands::node::probe_and_update(&state).await, + "an unresolvable profile config must read as no node, not as global's" + ); + m.assert_async().await; +} + /// A unique scratch dir under the OS temp dir, cleaned on drop. struct Scratch(std::path::PathBuf); impl Scratch { From 9dd98b89bd416913564813fcc6a2d55757d8d82a Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 17:08:29 +0200 Subject: [PATCH 06/26] fix(bridge): the TS mirror stops making required fields optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five fields of `NameActionCapabilities` were declared optional on the TS side with the reason written into the doc comment: "Optional so existing fixtures stay valid". The Rust struct always sends them — `auction_bidding_blocks` and `auction_reveal_blocks` as `Option`, `pending_broadcast_action` as `Option`, and `stranded_bid_count` and `stranded_lockup_doos` as plain `i64` that cannot be absent at all. CODING_STANDARDS asks for the opposite trade: "Adding a field means: Rust struct + TS interface + every test fixture that builds that object", and fixtures are to be complete. Optional fields buy a quiet fixture at the cost of every consumer having to handle an `undefined` the backend never sends — and of the next field being added the same way. #61's own review fixed this for `nameIsRegistered` and `transferPending` and left these. Two fixtures were genuinely incomplete and now say what the backend says. Also in the bridge: the browser-QA mock's `get_settings` hand-listed twelve of the twenty-three settings, so `allow_remote_broadcast` among others read as `undefined` in browser QA and nowhere else, and `check_node_connection` had no handler at all — "Test connection" fell through to the unknown-command warning and returned null, which the caller reads as a failure it cannot explain. The mock now spreads the typed `DEFAULT_SETTINGS`, so a new setting cannot be forgotten there, and answers the probe. That import had to move: the settings store imports `invoke`, `invoke` imports the mock, and a mock reaching back into the store closed a cycle that left `DEFAULT_SETTINGS` undefined at module-init time. The defaults now live in a leaf module both sides import; the store re-exports them so existing importers are untouched. Two smaller ones from the same review: the sidebar's About link is an icon whose `title` was also its accessible name, and the tooltip conversion removed it without an `aria-label`; and `Tooltip`'s prop doc promised that empty content renders the children "bare, with no wrapper", while the code always renders the wrapper and explains three lines below why it must. Gates: npx tsc -b, npx vite build, npx vitest run (890 passed), npm run lint:format, lint:secure-imports, lint:native-title. Note: `activity-view`, `send-flow` and `wallet-view` each failed once in a full-suite run and passed alone and on re-run. Pre-existing flakes, not from this change; worth their own fix. --- CHANGELOG.md | 1 + src/components/Layout.tsx | 6 ++- .../__tests__/name-sign-message.test.tsx | 5 +++ src/components/ui/Tooltip.tsx | 5 ++- src/lib/nameSections.test.ts | 5 +++ src/lib/settingsDefaults.ts | 43 +++++++++++++++++++ src/lib/webqa-mock.ts | 36 ++++++++++------ src/stores/settings.ts | 30 +------------ src/types/index.ts | 13 +++--- 9 files changed, 94 insertions(+), 50 deletions(-) create mode 100644 src/lib/settingsDefaults.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a1e3f75a..1816cced 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and — once a wallet profile exists — whether the node's network matches the wallet's; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **The About link in the sidebar has a name again for screen readers.** It is an icon with no text, so its old `title` was also its accessible name; converting the sidebar's hints to tooltips took the `title` away without putting an `aria-label` in its place, leaving the link announced as nothing. The keyboard-shortcuts button beside it already carried one. - **Each wallet profile now really uses its own node.** Per-profile node configuration was stored and resolved, but several paths still asked the global settings: the node the app started took the global api-key, the background sync decided whether "the node" was caught up by asking the global one while every step it gated talked to the profile's, and a sync failure named the global URL in a message telling you to go fix it. Worse, a profile whose own node configuration would not resolve was quietly served the global node instead of being reported as misconfigured, so a profile pinned to a regtest node could be answered by a mainnet one. Each of those now resolves through the profile, and an unresolvable profile configuration is an error rather than somebody else's node. The automatic repair of a stale loopback port also stops rewriting a URL you set on the profile yourself. - **A testnet or regtest wallet no longer reads from the mainnet explorer.** An early migration seeded `explorer_api_url` with the mainnet explorer, and a later change swapped that seed for an empty string so the wallet could pick an explorer per the profile's network. The swap only helped databases created after it: every installation that had already run the original kept the mainnet URL, and an explicit setting outranks the network default, so a testnet or regtest profile went on querying mainnet and getting confident answers about a chain it was not on. A new migration removes exactly the value the old one seeded, leaving an explorer URL you chose yourself alone, and the explorer factory now refuses the known mainnet explorer off mainnet in case a database reaches it before the migration has run. - **Reveal is no longer offered on a name with nothing left to reveal.** A lockup stranded in an auction that lapsed is an unspent BID coin, and the Reveal button was gated on holding one of those anywhere in the wallet rather than in the auction being looked at. So on a fully revealed name the button stayed live for good and failed every time with "no unspent bid coin". It now reads the same set the reveal transaction is built from, so the button and the builder cannot disagree. diff --git a/src/components/Layout.tsx b/src/components/Layout.tsx index 162dbe7a..720a6c91 100644 --- a/src/components/Layout.tsx +++ b/src/components/Layout.tsx @@ -81,7 +81,11 @@ export function Layout() { - + ℹ️ diff --git a/src/components/name-actions/__tests__/name-sign-message.test.tsx b/src/components/name-actions/__tests__/name-sign-message.test.tsx index bf5cb9d4..a81b2586 100644 --- a/src/components/name-actions/__tests__/name-sign-message.test.tsx +++ b/src/components/name-actions/__tests__/name-sign-message.test.tsx @@ -65,6 +65,11 @@ function capsFor(name: string, ownsName: boolean): NameActionCapabilities { countdownLabel: null, countdownBlocks: null, countdownHours: null, + auctionBiddingBlocks: null, + auctionRevealBlocks: null, + pendingBroadcastAction: null, + strandedBidCount: 0, + strandedLockupDoos: 0, }; } diff --git a/src/components/ui/Tooltip.tsx b/src/components/ui/Tooltip.tsx index 776d78db..c9a1590c 100644 --- a/src/components/ui/Tooltip.tsx +++ b/src/components/ui/Tooltip.tsx @@ -30,8 +30,9 @@ export const TOOLTIP_OPEN_DELAY_MS = 300; interface TooltipProps { /** * Tooltip body. Kept short — one or two lines. When empty (`null`, - * `undefined` or `""`) the children render bare, with no wrapper and no - * hover handling, so callers can pass a conditional reason directly. + * `undefined` or `""`) no tooltip is shown and no hover handling applies, + * so callers can pass a conditional reason directly. The trigger wrapper is + * still rendered — see the note on `hasContent` for why it must be. */ content: ReactNode; /** The trigger. Rendered inline; gets the hover/focus reference props. */ diff --git a/src/lib/nameSections.test.ts b/src/lib/nameSections.test.ts index b13d3035..9906628e 100644 --- a/src/lib/nameSections.test.ts +++ b/src/lib/nameSections.test.ts @@ -42,6 +42,11 @@ function caps(over: Partial = {}): NameActionCapabilitie countdownLabel: null, countdownBlocks: null, countdownHours: null, + auctionBiddingBlocks: null, + auctionRevealBlocks: null, + pendingBroadcastAction: null, + strandedBidCount: 0, + strandedLockupDoos: 0, ...over, }; } diff --git a/src/lib/settingsDefaults.ts b/src/lib/settingsDefaults.ts new file mode 100644 index 00000000..c11b1fb5 --- /dev/null +++ b/src/lib/settingsDefaults.ts @@ -0,0 +1,43 @@ +/** + * The default value of every setting, in one typed object. + * + * It lives in a leaf module on purpose. The settings store imports `invoke`, + * and `invoke` imports the browser-QA mock, so a mock that reached back into + * the store for these defaults would close an import cycle and leave the + * object undefined at module-init time, depending on which side loaded first. + * Everything that needs the defaults imports them from here instead. + * + * Typed as `Settings`, so adding a setting is a type error until its default + * is written down — which is what keeps the mock and the real backend + * answering with the same shape. + */ +import type { Settings } from "../types"; + +export const DEFAULT_SETTINGS: Settings = { + // Sending node (hsd RPC); reads come from the explorer below. + node_rpc_url: "http://127.0.0.1:12037", + node_rpc_api_key: "", + hsd_prefix: "", + hsd_path: "", + autostart_hsd: "true", + explorer_api_url: "https://e.hnsfans.com", + address_gap_limit: "20", + signer_session_timeout_seconds: "900", + onboarding_complete: "false", + deadline_notify_enabled: "false", + deadline_notify_reveal_lead_blocks: "144", + deadline_notify_renewal_lead_days: "30", + watchlist_notify_enabled: "false", + watchlist_notify_bidding_soon_lead_blocks: "144", + watchlist_notify_highest_bid_threshold_hns: "", + background_sync_enabled: "1", + node_mode: "full", + explorer_fallback_url: "", + chain_source: "local_node", + close_to_tray: "1", + allow_remote_broadcast: "false", + tray_hint_shown: "0", + launch_at_login: "0", + fee_rate_doos_per_kvb: "", + update_notify_enabled: "false", +}; diff --git a/src/lib/webqa-mock.ts b/src/lib/webqa-mock.ts index a72e9f23..2b9109ad 100644 --- a/src/lib/webqa-mock.ts +++ b/src/lib/webqa-mock.ts @@ -10,6 +10,9 @@ */ /* eslint-disable @typescript-eslint/no-unused-vars */ +import { DEFAULT_SETTINGS } from "./settingsDefaults"; +import type { NodeConnectionCheck, Settings } from "../types"; + type Handler = (args?: Record) => unknown; // ═══════════════════════════════════════════════════════════════════════════ @@ -344,23 +347,32 @@ function auctionPositionNames(): string[] { const handlers: Record = { // ── Settings ────────────────────────────────────────────────────────── - get_settings: () => ({ - node_rpc_url: "http://127.0.0.1:12037", - node_rpc_api_key: "", - hsd_prefix: "", - hsd_path: "", - explorer_api_url: "https://e.hnsfans.com", - address_gap_limit: "20", - signer_session_timeout_seconds: "900", + // Built from the typed defaults rather than hand-listed: this map used to + // omit settings the real backend always returns (`allow_remote_broadcast` + // among them), so a screen that reads one saw `undefined` in browser QA and + // nowhere else. Spreading the typed object means a new setting cannot be + // forgotten here. + get_settings: (): Settings => ({ + ...DEFAULT_SETTINGS, onboarding_complete: "true", - background_sync_enabled: "1", - node_mode: "full", - explorer_fallback_url: "", - chain_source: "local_node", }), update_setting: () => null, + // Browser QA has no node to probe. Answer as a reachable, synced, matching + // node so "Test connection" completes instead of falling through to the + // unknown-command warning and returning null, which the caller reads as a + // failure it cannot explain. + check_node_connection: (): NodeConnectionCheck => ({ + reachable: true, + height: 100_000, + headers: 100_000, + synced: true, + network: "regtest", + networkMatches: true, + error: null, + }), + // ── Daemon control ──────────────────────────────────────────────────── is_background_sync_enabled: () => true, set_background_sync_enabled: () => null, diff --git a/src/stores/settings.ts b/src/stores/settings.ts index 1f5def2e..4d37e5d4 100644 --- a/src/stores/settings.ts +++ b/src/stores/settings.ts @@ -1,5 +1,6 @@ import { create } from "zustand"; import { invoke } from "../lib/invoke"; +import { DEFAULT_SETTINGS } from "../lib/settingsDefaults"; import type { Settings } from "../types"; interface SettingsState { @@ -10,34 +11,7 @@ interface SettingsState { saveAll: (partial: Partial) => Promise; } -export const DEFAULT_SETTINGS: Settings = { - // Sending node (hsd RPC); reads come from the explorer below. - node_rpc_url: "http://127.0.0.1:12037", - node_rpc_api_key: "", - hsd_prefix: "", - hsd_path: "", - autostart_hsd: "true", - explorer_api_url: "https://e.hnsfans.com", - address_gap_limit: "20", - signer_session_timeout_seconds: "900", - onboarding_complete: "false", - deadline_notify_enabled: "false", - deadline_notify_reveal_lead_blocks: "144", - deadline_notify_renewal_lead_days: "30", - watchlist_notify_enabled: "false", - watchlist_notify_bidding_soon_lead_blocks: "144", - watchlist_notify_highest_bid_threshold_hns: "", - background_sync_enabled: "1", - node_mode: "full", - explorer_fallback_url: "", - chain_source: "local_node", - close_to_tray: "1", - allow_remote_broadcast: "false", - tray_hint_shown: "0", - launch_at_login: "0", - fee_rate_doos_per_kvb: "", - update_notify_enabled: "false", -}; +export { DEFAULT_SETTINGS } from "../lib/settingsDefaults"; export const useSettingsStore = create((set, get) => ({ settings: null, diff --git a/src/types/index.ts b/src/types/index.ts index 0d8ca070..3a392416 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -664,25 +664,24 @@ export interface NameActionCapabilities { /** * How long this network's auction phases run, in blocks. Static per network, * but needed before an auction exists — to say what opening one commits to. - * Optional so existing fixtures stay valid; `null` when the backend could not - * determine the network. + * `null` when the backend could not determine the network. */ - auctionBiddingBlocks?: number | null; - auctionRevealBlocks?: number | null; + auctionBiddingBlocks: number | null; + auctionRevealBlocks: number | null; /** * The action this wallet has broadcast for the name that is still waiting * for a block (`"open"`, `"reveal"`, …). Until it is mined the chain reports * the name's previous state, so nothing phase-derived can describe it. */ - pendingBroadcastAction?: string | null; + pendingBroadcastAction: string | null; /** * Bids placed in an EARLIER auction of the same name whose lockup is * stranded — the auction closed without a reveal, so it can be neither * revealed nor redeemed. Scoping the bids panel by auction hid these, which * leaves money missing with nothing on screen to explain it. */ - strandedBidCount?: number; - strandedLockupDoos?: number; + strandedBidCount: number; + strandedLockupDoos: number; /** * Losing reveals a REDEEM would reclaim on this name, and what they are * worth. "Redeem" is a covenant name, not an explanation — the button says From 14c4ba443480ca0614ec001d08a384216e1bd41d Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 17:14:40 +0200 Subject: [PATCH 07/26] test: one lock per piece of global state, and stop excluding testable code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three test-hygiene rules from CODING_STANDARDS, each broken in a way the file argued for in a comment. `cookie_vault`'s own tests serialized on a module-local mutex, while every test in `namebase_cmd_tests` serializes on the `serial_test` key `cookie_vault` — and both mutate the same process-global `TEST_DEK` and test-backend slots through the same setters. Two different locks around one piece of state exclude nothing. The comment explaining the choice said `#[serial]` "isn't a dep here"; `serial_test` is a dev-dependency, and its Cargo.toml entry describes this exact use. Both guards are gone and the six tests that touch those slots carry the shared key, so the two files now exclude each other. `node_status_data_dir_defaults_to_home_dot_hsd_when_prefix_unset` reads `$HOME` without `#[serial(hsd_home)]`, while the writer and every other reader carry it. The standard names this as the failure that made a test fail about one run in twenty, and #61 fixed it everywhere except this file. `#[cfg_attr(coverage_nightly, coverage(off))]` sat on six pure functions, which the standard forbids outright — "Never on a pure function — write a test instead". Three were already under test and the attribute was simply hiding that: `daemon_bin_name`, `provision_addresses` (a `_from_conn` shape) and `fetch_wallet_coins_and_txs_with_client`, whose own doc says "Testable against a mock without an AppState" three lines above the attribute saying it is not. `fetch_all` is the same `&dyn NodeRpc` seam and is driven through the poll path. The remaining two had no tests and now do: `output_names_from_pairs`, which pairs each Ledger output index with its covenant name, and `status_word_message`, which turns an APDU status word into what the user reads when a device refuses. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2586 passed, run twice for the serialization change). --- src-tauri/src/commands/daemon_ctl.rs | 1 - src-tauri/src/commands/secure_wallet.rs | 1 - src-tauri/src/commands/tx.rs | 21 +++++++++- src-tauri/src/daemon/watched_names.rs | 1 - src-tauri/src/noncustodial/cookie_vault.rs | 39 +++++++++--------- .../src/providers/ledger/hid_transport.rs | 1 - .../src/tests/ledger_hid_transport_tests.rs | 40 +++++++++++++++++++ src-tauri/src/tests/mod.rs | 1 + src-tauri/src/tests/node_status_tests.rs | 1 + 9 files changed, 79 insertions(+), 27 deletions(-) create mode 100644 src-tauri/src/tests/ledger_hid_transport_tests.rs diff --git a/src-tauri/src/commands/daemon_ctl.rs b/src-tauri/src/commands/daemon_ctl.rs index 1c69f7f7..2a03e797 100644 --- a/src-tauri/src/commands/daemon_ctl.rs +++ b/src-tauri/src/commands/daemon_ctl.rs @@ -224,7 +224,6 @@ pub(crate) const RESOURCE_REL_DIRS: &[&str] = &[ ]; /// Return the platform-adjusted daemon binary name (appends `.exe` on Windows). -#[cfg_attr(coverage_nightly, coverage(off))] pub(crate) fn daemon_bin_name() -> String { if cfg!(target_os = "windows") { format!("{DAEMON_BIN_NAME}.exe") diff --git a/src-tauri/src/commands/secure_wallet.rs b/src-tauri/src/commands/secure_wallet.rs index c3eed2e0..85b21afd 100644 --- a/src-tauri/src/commands/secure_wallet.rs +++ b/src-tauri/src/commands/secure_wallet.rs @@ -103,7 +103,6 @@ pub(crate) fn account_xpub_from_seed( /// Derive + persist the initial receive/change address windows for a profile. /// Returns the first receive address (the profile's default receive address). -#[cfg_attr(coverage_nightly, coverage(off))] pub(crate) fn provision_addresses( conn: &rusqlite::Connection, profile_id: &str, diff --git a/src-tauri/src/commands/tx.rs b/src-tauri/src/commands/tx.rs index 0a381026..ef9697cf 100644 --- a/src-tauri/src/commands/tx.rs +++ b/src-tauri/src/commands/tx.rs @@ -1272,7 +1272,6 @@ async fn sign_via_ledger( /// Convert pre-resolved `(output_index, name)` pairs into the /// [`OutputName`](crate::providers::ledger::parse_mode::OutputName) entries /// that the parse-mode builder expects. -#[cfg_attr(coverage_nightly, coverage(off))] fn output_names_from_pairs( pairs: &[(usize, String)], ) -> Vec { @@ -1431,7 +1430,6 @@ pub(crate) async fn classify_broadcast_outcome_with_client( /// silently skipped — the tx cache is best-effort, not authoritative. /// /// Testable against a mock without an AppState. -#[cfg_attr(coverage_nightly, coverage(off))] pub(crate) async fn fetch_wallet_coins_and_txs_with_client( client: &dyn crate::noncustodial::node_rpc::NodeRpc, addresses: &[String], @@ -2623,4 +2621,23 @@ mod pure_helper_tests { fn local_txid_from_summary_none_when_json_is_invalid() { assert_eq!(local_txid_from_summary("not { valid json"), None); } + + #[test] + fn output_names_from_pairs_keeps_each_index_with_its_name() { + // The Ledger parse-mode builder is told which output carries which + // name; pairing them by position is the whole job, and getting it + // wrong labels a covenant on the device with another output's name. + let out = + output_names_from_pairs(&[(0, "example".to_string()), (2, "another".to_string())]); + assert_eq!(out.len(), 2); + assert_eq!(out[0].output_index, 0); + assert_eq!(out[0].name, "example"); + assert_eq!(out[1].output_index, 2); + assert_eq!(out[1].name, "another"); + } + + #[test] + fn output_names_from_pairs_maps_an_empty_slice_to_an_empty_vec() { + assert!(output_names_from_pairs(&[]).is_empty()); + } } diff --git a/src-tauri/src/daemon/watched_names.rs b/src-tauri/src/daemon/watched_names.rs index 05ac8b22..c13910b1 100644 --- a/src-tauri/src/daemon/watched_names.rs +++ b/src-tauri/src/daemon/watched_names.rs @@ -582,7 +582,6 @@ async fn try_run_watched_scan(db_path: &str) -> Result<(), AppError> { /// every 60s, so simple sequential polling is preferable to pulling in a /// streaming-concurrency dependency. Names that error out or return /// null/unparsable data are silently dropped; they'll be retried next cycle. -#[cfg_attr(coverage_nightly, coverage(off))] async fn fetch_all( node: &dyn crate::noncustodial::node_rpc::NodeRpc, names: &[String], diff --git a/src-tauri/src/noncustodial/cookie_vault.rs b/src-tauri/src/noncustodial/cookie_vault.rs index bbc5877f..accdc7bb 100644 --- a/src-tauri/src/noncustodial/cookie_vault.rs +++ b/src-tauri/src/noncustodial/cookie_vault.rs @@ -324,10 +324,15 @@ pub fn decrypt_cookie(blob_hex: &str) -> Result, AppError> { #[cfg(test)] mod tests { use super::*; + use serial_test::serial; - /// Serializes all tests that mutate the process-global `TEST_DEK` slot so - /// they don't race each other under cargo's parallel test runner. - static DEK_TEST_GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); + // Tests that mutate the process-global `TEST_DEK` / test-backend slots + // carry `#[serial(cookie_vault)]`. A module-local mutex used to do this + // job, which excluded these tests from each other but not from + // `tests::namebase_cmd_tests`, whose every test already serializes on the + // `cookie_vault` key and calls `set_test_dek` through the same slots. Two + // locks around one piece of global state is no lock at all; one key covers + // both files. // A fixed DEK for pure-crypto tests. These tests exercise the crypto // envelope directly (bypassing the OS keyring), so they never touch the @@ -492,6 +497,7 @@ mod tests { /// the risk; the real guarantee is the `#[cfg(any(test, debug_assertions))]` /// on the item itself, verified to compile-out by the release profile. #[test] + #[serial(cookie_vault)] fn test_dek_slot_present_only_under_debug_or_test() { // Under `cargo test`, cfg(test) is set, so `set_test_dek` is compiled // in and callable — exercised here to keep the bypass path covered. @@ -499,7 +505,6 @@ mod tests { // `#[cfg(any(test, debug_assertions))]` attribute on the item, not by // this test: a `--release` build has `debug_assertions` off, so the // function and its backing slot are not compiled at all. - let _held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); set_test_dek(Some(vec![0u8; DEK_LEN])); set_test_dek(None); } @@ -554,12 +559,12 @@ mod tests { /// branch in `get_or_create_dek` (line 154) and the DEK-zeroize wrappers /// without touching the OS keyring. /// - /// Serialized (not `#[serial]`, which isn't a dep here) via a module mutex - /// so it doesn't race other tests that toggle the shared TEST_DEK slot. + /// Serialized on the `cookie_vault` key so it does not race the other + /// tests — here or in `namebase_cmd_tests` — that toggle the shared + /// TEST_DEK slot. #[test] + #[serial(cookie_vault)] fn public_cookie_roundtrip_with_test_dek() { - let _held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - set_test_dek(Some(test_dek())); let plaintext = b"session=xyz; secure; httponly"; let blob_hex = encrypt_cookie(plaintext).expect("encrypt_cookie"); @@ -572,9 +577,8 @@ mod tests { /// `encrypt_cookie` propagates the empty-plaintext rejection through the /// public API (with a test DEK installed). #[test] + #[serial(cookie_vault)] fn public_encrypt_cookie_rejects_empty() { - let _held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - set_test_dek(Some(test_dek())); let err = encrypt_cookie(b"").unwrap_err(); assert!(matches!(err, AppError::InvalidInput(_)), "got {err:?}"); @@ -585,9 +589,8 @@ mod tests { /// (the `return Ok(dek)` when a fixed DEK is installed) without going /// through the public encrypt/decrypt wrappers. #[test] + #[serial(cookie_vault)] fn get_or_create_dek_returns_installed_test_dek() { - let _held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - let fixed = test_dek(); set_test_dek(Some(fixed.clone())); let got = get_or_create_dek().expect("test DEK should be returned"); @@ -712,15 +715,11 @@ mod tests { ); } - /// Serializes tests that mutate the process-global test-backend slot. - static BACKEND_TEST_GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(()); - #[test] + #[serial(cookie_vault)] fn get_or_create_dek_uses_installed_test_backend_existing_entry() { // Must NOT race with test-DEK-slot tests either: get_or_create_dek - // consults TEST_DEK first. - let _dek_held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - let _held = BACKEND_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); + // consults TEST_DEK first — the shared serial key covers both slots. set_test_dek(None); let fixed = vec![9u8; DEK_LEN]; @@ -734,10 +733,8 @@ mod tests { } #[test] + #[serial(cookie_vault)] fn get_or_create_dek_uses_installed_test_backend_new_entry() { - let _dek_held = DEK_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - let _held = BACKEND_TEST_GUARD.lock().unwrap_or_else(|p| p.into_inner()); - set_test_dek(None); set_test_keyring_backend(Some(Box::new(FakeKeyring::empty()))); diff --git a/src-tauri/src/providers/ledger/hid_transport.rs b/src-tauri/src/providers/ledger/hid_transport.rs index 49f29afb..097a76ea 100644 --- a/src-tauri/src/providers/ledger/hid_transport.rs +++ b/src-tauri/src/providers/ledger/hid_transport.rs @@ -173,7 +173,6 @@ impl Transport { } /// Human-readable message for a non-success status word. -#[cfg_attr(coverage_nightly, coverage(off))] pub fn status_word_message(sw: u16) -> String { let hint = match sw { 0x6985 => " (user rejected on device)", diff --git a/src-tauri/src/tests/ledger_hid_transport_tests.rs b/src-tauri/src/tests/ledger_hid_transport_tests.rs new file mode 100644 index 00000000..905aab5b --- /dev/null +++ b/src-tauri/src/tests/ledger_hid_transport_tests.rs @@ -0,0 +1,40 @@ +//! Tests for `providers::ledger::hid_transport` helpers that need no device. +//! +//! `status_word_message` turns an APDU status word into the sentence the user +//! reads when a Ledger refuses something. It is the only part of this module +//! that runs without hardware. + +use crate::providers::ledger::hid_transport::status_word_message; + +#[test] +fn a_known_status_word_explains_itself() { + let msg = status_word_message(0x6985); + assert!( + msg.contains("0x6985"), + "the raw word stays in the text: {msg}" + ); + assert!( + msg.contains("user rejected on device"), + "the common case must not read as an unexplained code: {msg}" + ); +} + +#[test] +fn every_mapped_status_word_adds_a_hint() { + for (sw, needle) in [ + (0x6985u16, "user rejected"), + (0x6d00, "instruction not supported"), + (0x6e00, "class not supported"), + (0x6a80, "invalid data"), + (0x5515, "device locked"), + ] { + let msg = status_word_message(sw); + assert!(msg.contains(needle), "0x{sw:04x} lost its hint: {msg}"); + } +} + +#[test] +fn an_unknown_status_word_is_reported_without_inventing_a_reason() { + let msg = status_word_message(0x1234); + assert_eq!(msg, "APDU failed with status 0x1234"); +} diff --git a/src-tauri/src/tests/mod.rs b/src-tauri/src/tests/mod.rs index 3b78e328..43d9cae3 100644 --- a/src-tauri/src/tests/mod.rs +++ b/src-tauri/src/tests/mod.rs @@ -36,6 +36,7 @@ mod history_cmd_tests; mod hsd_extra_tests; mod hsd_parity_tests; mod hsd_types_tests; +mod ledger_hid_transport_tests; mod live_node_it; mod migration_tests; mod mock_node_rpc; diff --git a/src-tauri/src/tests/node_status_tests.rs b/src-tauri/src/tests/node_status_tests.rs index 7bff6f44..4118a62e 100644 --- a/src-tauri/src/tests/node_status_tests.rs +++ b/src-tauri/src/tests/node_status_tests.rs @@ -1256,6 +1256,7 @@ async fn node_status_reflects_seeded_profile_network_testnet() { // =========================================================================== #[tokio::test] +#[serial_test::serial(hsd_home)] async fn node_status_data_dir_defaults_to_home_dot_hsd_when_prefix_unset() { let conn = blank_conn(); db::queries::set_setting(&conn, "node_rpc_url", "http://127.0.0.1:1").unwrap(); From 4af5e5888574717173412c04bd3fbb3e7e8cc48c Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 17:54:41 +0200 Subject: [PATCH 08/26] refactor: delete the parts nothing calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BidGate` decided whether the advanced auction section should draw the Bid and Lockup inputs, so that no phase where bidding is impossible could invite one. Its own doc says the predicate is "shared by BOTH call sites (the guided BIDDING panel and the advanced section)"; both are gone. `GuidedAction` renders `BidForm` directly, and #61's `resolveSections` answers the question a different way — a section that cannot apply is not rendered at all, so there is nothing left to gate. The component had no non-test caller and two test files. Its dedicated unit test goes with it: the behaviour it guarded is still pinned by `name-actions-bid-gate.test.tsx`, which drives the modal, and that is where a regression would now appear. Two assertions in that file looked for the absence of `bid-gate-placeholder` — a testid only `BidGate` ever rendered, so they could no longer fail; they are gone and the describe block says what the file actually holds. `build_batch_renew_draft_inner` and `build_batch_transfer_draft_inner` each took a `names` slice they did not read, with `let _ = names; // kept for API parity` to prove it. There is no parity to keep: `per_name` carries the names, and both functions build `batch_names` from it. The parameter and its five now-orphaned test bindings are gone. `derive_auction_task_state` carried `let _ = has_bid_commitment;` inside the BIDDING arm, which reads as though the parameter were unused. It is used four lines below, in the arm right after. Left alone deliberately: `DataTable`'s `onRowClick` and the `fromInteractiveChild` guard behind it still have no screen passing them, but the spec records that as an accepted gap, and reversing a decision the team wrote down is not a cleanup. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2586 passed), npx tsc -b, npx vitest run (884 passed), npm run lint:format. --- src-tauri/src/commands/names.rs | 11 +- .../tests/build_batch_renew_draft_tests.rs | 11 +- .../tests/build_batch_transfer_draft_tests.rs | 7 - .../__tests__/name-actions-bid-gate.test.tsx | 9 +- src/components/name-actions/BidGate.tsx | 81 ----------- .../name-actions/__tests__/bid-gate.test.tsx | 132 ------------------ 6 files changed, 11 insertions(+), 240 deletions(-) delete mode 100644 src/components/name-actions/BidGate.tsx delete mode 100644 src/components/name-actions/__tests__/bid-gate.test.tsx diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index fb38901c..fbe9554b 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -1551,7 +1551,6 @@ pub fn derive_auction_task_state( // header + the distinguished "yours" rows in the bid list convey // that a bid is already placed; we no longer collapse to a // terminal WaitingForBidding state that hides the form. - let _ = has_bid_commitment; AuctionTaskState::ReadyToBid } "REVEAL" => { @@ -2873,7 +2872,7 @@ pub async fn build_batch_renew_draft( } let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; - build_batch_renew_draft_inner(&conn, &ctx, &names, per_name, &rblock, rate) + build_batch_renew_draft_inner(&conn, &ctx, per_name, &rblock, rate) } /// Pure inner logic for `build_batch_renew_draft`, testable without a Tauri @@ -2885,7 +2884,6 @@ pub async fn build_batch_renew_draft( pub(crate) fn build_batch_renew_draft_inner( conn: &rusqlite::Connection, ctx: &Ctx, - names: &[String], per_name: Vec<(String, [u8; 32], queries::NameCoin, NameState)>, rblock: &[u8; 32], rate: u64, @@ -2917,7 +2915,6 @@ pub(crate) fn build_batch_renew_draft_inner( )?; // Persist with first name as primary; the draft plan contains all names. let display_name = names_pure::display_names(&batch_names); - let _ = names; // kept for API parity; batch_names carries the actual list let name_refs: Vec<&str> = batch_names.iter().map(|s| s.as_str()).collect(); persist_with_conn( conn, @@ -2963,9 +2960,7 @@ pub async fn build_batch_transfer_draft( } let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; - build_batch_transfer_draft_inner( - &conn, &ctx, &names, per_name, &recipient, version, &program, rate, - ) + build_batch_transfer_draft_inner(&conn, &ctx, per_name, &recipient, version, &program, rate) } /// Pure inner logic for `build_batch_transfer_draft`, testable without a Tauri @@ -2982,7 +2977,6 @@ pub async fn build_batch_transfer_draft( pub(crate) fn build_batch_transfer_draft_inner( conn: &rusqlite::Connection, ctx: &Ctx, - names: &[String], per_name: Vec<(String, [u8; 32], queries::NameCoin, NameState)>, recipient: &str, version: u8, @@ -3015,7 +3009,6 @@ pub(crate) fn build_batch_transfer_draft_inner( rate, )?; let display_name = names_pure::display_names(&batch_names); - let _ = names; // kept for API parity; batch_names carries the actual list let name_refs: Vec<&str> = batch_names.iter().map(|s| s.as_str()).collect(); persist_with_conn( conn, diff --git a/src-tauri/src/tests/build_batch_renew_draft_tests.rs b/src-tauri/src/tests/build_batch_renew_draft_tests.rs index 053cc5c1..5fd544ba 100644 --- a/src-tauri/src/tests/build_batch_renew_draft_tests.rs +++ b/src-tauri/src/tests/build_batch_renew_draft_tests.rs @@ -179,11 +179,9 @@ fn setup( #[test] fn batch_renew_happy_path_persists_draft() { let (conn, ctx, per_name) = setup(&["alpha", "bravo"]); - let names: Vec = vec!["alpha".into(), "bravo".into()]; let rblock = [0x55u8; 32]; - let summary = - build_batch_renew_draft_inner(&conn, &ctx, &names, per_name, &rblock, 10).unwrap(); + let summary = build_batch_renew_draft_inner(&conn, &ctx, per_name, &rblock, 10).unwrap(); assert_eq!(summary.action, "batch-renew"); let drafts: i64 = conn @@ -208,8 +206,7 @@ fn batch_renew_empty_persists_zero_input_draft() { // The async wrapper rejects empty `names` up front; if the inner is // called with an empty batch, `build_batch_plan` refuses (no outputs). let (conn, ctx, _per_name) = setup(&[]); - let err = - build_batch_renew_draft_inner(&conn, &ctx, &[], Vec::new(), &[0u8; 32], 10).unwrap_err(); + let err = build_batch_renew_draft_inner(&conn, &ctx, Vec::new(), &[0u8; 32], 10).unwrap_err(); match err { AppError::InvalidInput(msg) => assert!(msg.contains("at least one output"), "got {msg}"), other => panic!("expected InvalidInput, got {other:?}"), @@ -219,9 +216,7 @@ fn batch_renew_empty_persists_zero_input_draft() { #[test] fn batch_renew_single_name() { let (conn, ctx, per_name) = setup(&["solo"]); - let names: Vec = vec!["solo".into()]; - let summary = - build_batch_renew_draft_inner(&conn, &ctx, &names, per_name, &[0x11u8; 32], 20).unwrap(); + let summary = build_batch_renew_draft_inner(&conn, &ctx, per_name, &[0x11u8; 32], 20).unwrap(); assert_eq!(summary.action, "batch-renew"); let name_list = summary.summary.get("nameList"); assert!(name_list.is_some(), "batch draft records the name list"); diff --git a/src-tauri/src/tests/build_batch_transfer_draft_tests.rs b/src-tauri/src/tests/build_batch_transfer_draft_tests.rs index 56948e42..3564ac69 100644 --- a/src-tauri/src/tests/build_batch_transfer_draft_tests.rs +++ b/src-tauri/src/tests/build_batch_transfer_draft_tests.rs @@ -198,11 +198,9 @@ fn setup(names_in: &[&str]) -> Fixture { #[test] fn batch_transfer_happy_path_persists_draft() { let f = setup(&["alpha", "bravo"]); - let names: Vec = vec!["alpha".into(), "bravo".into()]; let summary = build_batch_transfer_draft_inner( &f.conn, &f.ctx, - &names, f.per_name, &f.recipient, f.version, @@ -249,7 +247,6 @@ fn batch_transfer_uses_owner_address_for_output() { // the recipient lives only in the covenant items. Guard both by parsing // the persisted plan. let f = setup(&["alpha", "bravo"]); - let names: Vec = vec!["alpha".into(), "bravo".into()]; let owner_addr = f.owner_addr.clone(); let program_hex = hex::encode(&f.program); let version = f.version; @@ -257,7 +254,6 @@ fn batch_transfer_uses_owner_address_for_output() { build_batch_transfer_draft_inner( &f.conn, &f.ctx, - &names, f.per_name, &f.recipient, f.version, @@ -311,11 +307,9 @@ fn batch_transfer_uses_owner_address_for_output() { #[test] fn batch_transfer_single_name() { let f = setup(&["solo"]); - let names: Vec = vec!["solo".into()]; let summary = build_batch_transfer_draft_inner( &f.conn, &f.ctx, - &names, f.per_name, &f.recipient, f.version, @@ -334,7 +328,6 @@ fn batch_transfer_empty_errors() { let err = build_batch_transfer_draft_inner( &f.conn, &f.ctx, - &[], Vec::new(), &f.recipient, f.version, diff --git a/src/components/__tests__/name-actions-bid-gate.test.tsx b/src/components/__tests__/name-actions-bid-gate.test.tsx index a56d1948..6c1d75fc 100644 --- a/src/components/__tests__/name-actions-bid-gate.test.tsx +++ b/src/components/__tests__/name-actions-bid-gate.test.tsx @@ -130,7 +130,12 @@ function wrapper() { beforeEach(() => invokeMock.mockReset()); -describe("NameActionsModal — BidGate hides inputs off the bidding phase", () => { +// `BidGate` is gone: it existed to decide whether the advanced section should +// draw the Bid/Lockup inputs, and `resolveSections` now decides whether that +// section exists at all. These tests keep the behaviour it was written for — +// no phase outside BIDDING may invite a bid — pinned on the modal itself, +// which is where a regression would actually show. +describe("NameActionsModal — no phase outside BIDDING invites a bid", () => { it("offers no advanced section at all during OPENING, so no bid can be invited", async () => { invokeMock.mockImplementation(route("OPENING", false)); render( {}} />, { @@ -145,7 +150,6 @@ describe("NameActionsModal — BidGate hides inputs off the bidding phase", () = expect(screen.queryByTestId("all-actions-toggle")).not.toBeInTheDocument(); expect(screen.queryByLabelText("Bid (HNS)")).not.toBeInTheDocument(); expect(screen.queryByLabelText("Lockup (HNS)")).not.toBeInTheDocument(); - expect(screen.queryByTestId("bid-gate-placeholder")).not.toBeInTheDocument(); }); it("shows the Bid/Lockup inputs during BIDDING when canBid is allowed", async () => { @@ -158,7 +162,6 @@ describe("NameActionsModal — BidGate hides inputs off the bidding phase", () = await waitFor(() => { expect(screen.getAllByLabelText("Bid (HNS)").length).toBeGreaterThan(0); }); - expect(screen.queryByTestId("bid-gate-placeholder")).not.toBeInTheDocument(); }); it("hides the Show-all-actions toggle in BIDDING when every advanced action is disabled", async () => { diff --git a/src/components/name-actions/BidGate.tsx b/src/components/name-actions/BidGate.tsx deleted file mode 100644 index 84bac28b..00000000 --- a/src/components/name-actions/BidGate.tsx +++ /dev/null @@ -1,81 +0,0 @@ -import type { ReactNode } from "react"; -import { BidForm, type BidFormProps } from "./BidForm"; -import { formatCountdown, type PhaseBadge, type PhaseCountdown } from "../../lib/auction"; -import type { NameActionCapability } from "../../types"; - -/** - * The single gate for the bid + lockup inputs (Diagnostic fix: the advanced - * auction section used to render {@link BidForm} unconditionally, so the Bid / - * Lockup fields showed — inviting input — in phases where bidding is - * impossible, e.g. right next to "Open" during OPENING. That made it look as - * though Open would also place a bid. - * - * The gate is a single predicate — `canBid.allowed` — shared by BOTH call - * sites (the guided BIDDING panel and the advanced section): - * - * - `canBid.allowed` true → render the {@link BidForm} (the "dumb" input pair). - * - false + a pre-bidding phase (AVAILABLE / OPENING / a BIDDING phase where - * the user has already bid) → render a contextual placeholder that says - * how long to wait (reusing the existing `countdown` / {@link formatCountdown}) - * and what to do next, falling back to `canBid.reason`. - * - false + any other phase (REVEAL / CLOSED / TRANSFER / REVOKED) → render - * nothing; the advanced section already surfaces those actions elsewhere. - * - * `BidForm` stays "dumb": it only knows how to draw the fields. All phase - * reasoning lives here. - */ -export interface BidGateProps extends BidFormProps { - /** The single visibility predicate: whether bidding is possible right now. */ - canBid: NameActionCapability; - /** Current auction phase — used to pick the placeholder copy. */ - phase: PhaseBadge["phase"]; - /** Existing countdown for this name; drives "opens in …" / "Reveal opens in …". */ - countdown: PhaseCountdown | null; -} - -/** The contextual placeholder shown when the inputs are gated off. Returns - * null for phases where a bid-related hint would be noise. */ -function bidPlaceholder( - phase: PhaseBadge["phase"], - countdown: PhaseCountdown | null, - reason: string | null, -): ReactNode { - const cd = countdown ? formatCountdown(countdown) : null; - - let body: ReactNode = null; - switch (phase) { - case "AVAILABLE": - body = "Open the auction first; bidding starts after the opening period."; - break; - case "OPENING": - body = cd - ? `Bidding opens in ${cd}. You'll set your bid & lockup then.` - : "Bidding opens after the opening period. You'll set your bid & lockup then."; - break; - case "BIDDING": - // canBid is false in BIDDING only once the user has already placed a bid. - body = cd ? `Your bid is placed. Reveal opens in ${cd}.` : "Your bid is placed."; - break; - default: - // REVEAL / CLOSED / TRANSFER / REVOKED / OTHER — no bid hint here. - return null; - } - - return ( -
- {body} - {/* Fall back to the backend-provided reason when our copy is generic. */} - {reason && !cd &&
{reason}
} -
- ); -} - -export function BidGate({ canBid, phase, countdown, ...formProps }: BidGateProps) { - if (canBid.allowed) { - return ; - } - return <>{bidPlaceholder(phase, countdown, canBid.reason)}; -} diff --git a/src/components/name-actions/__tests__/bid-gate.test.tsx b/src/components/name-actions/__tests__/bid-gate.test.tsx deleted file mode 100644 index 56171fd9..00000000 --- a/src/components/name-actions/__tests__/bid-gate.test.tsx +++ /dev/null @@ -1,132 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import "@testing-library/jest-dom"; -import { render, screen } from "@testing-library/react"; -import { BidGate } from "../BidGate"; -import type { PhaseCountdown } from "../../../lib/auction"; -import type { NameActionCapability } from "../../../types"; - -/** - * Regression: the advanced auction section used to render - * unconditionally, so Bid / Lockup inputs were visible in phases where - * bidding is impossible (AVAILABLE / OPENING / BIDDING-after-bid). The - * shared BidGate replaces that: a single `canBid.allowed` predicate decides - * whether the fields appear, with a contextual placeholder (using the - * existing countdown) when they do not. - */ - -const baseFormProps = { - variant: "advanced" as const, - bidHns: "", - onBidChange: vi.fn(), - lockupHns: "", - onLockupChange: vi.fn(), - bidError: null, - lockupError: null, - forfeitLockupText: "0", - disabled: false, - busy: false, - onSubmit: vi.fn(), - idleLabel: "Bid", - busyLabel: "…", -}; - -const allowed: NameActionCapability = { allowed: true, reason: null }; -const denied = (reason: string | null = null): NameActionCapability => ({ - allowed: false, - reason, -}); - -const openingCountdown: PhaseCountdown = { - label: "Bidding opens in", - blocks: 12, - hours: 2, -}; -const revealCountdown: PhaseCountdown = { - label: "Reveal starts in", - blocks: 6, - hours: 1, -}; - -describe("BidGate", () => { - it("renders the BidForm fields when canBid.allowed is true", () => { - render( - , - ); - // BidForm inputs are labelled by /"Lockup (HNS)". - expect(screen.getByLabelText("Bid (HNS)")).toBeInTheDocument(); - expect(screen.getByLabelText("Lockup (HNS)")).toBeInTheDocument(); - expect(screen.queryByTestId("bid-gate-placeholder")).not.toBeInTheDocument(); - }); - - it("hides fields and shows the AVAILABLE placeholder when canBid is denied", () => { - render( - , - ); - expect(screen.queryByLabelText("Bid (HNS)")).not.toBeInTheDocument(); - const ph = screen.getByTestId("bid-gate-placeholder"); - expect(ph).toHaveTextContent(/open the auction first/i); - }); - - it("shows the OPENING placeholder with the formatted countdown", () => { - render( - , - ); - expect(screen.queryByLabelText("Bid (HNS)")).not.toBeInTheDocument(); - // formatCountdown(openingCountdown) → "12 blocks (~2h)" - expect(screen.getByTestId("bid-gate-placeholder")).toHaveTextContent( - /Bidding opens in 12 blocks \(~2h\)/i, - ); - }); - - it("shows the hasBid placeholder in BIDDING when canBid is denied", () => { - // canBid.allowed is false in BIDDING only once the user has already bid. - render( - , - ); - expect(screen.queryByLabelText("Bid (HNS)")).not.toBeInTheDocument(); - expect(screen.getByTestId("bid-gate-placeholder")).toHaveTextContent( - /Your bid is placed\. Reveal opens in 6 blocks \(~1h\)/i, - ); - }); - - it("falls back to canBid.reason when no countdown is available", () => { - render( - , - ); - const ph = screen.getByTestId("bid-gate-placeholder"); - expect(ph).toHaveTextContent(/Bidding opens after the opening period/i); - // Fallback reason is surfaced beneath the generic copy. - expect(ph).toHaveTextContent(/Node not synced/i); - }); - - it("renders nothing outside pre-bidding phases (REVEAL / CLOSED / TRANSFER / REVOKED)", () => { - for (const phase of ["REVEAL", "CLOSED", "TRANSFER", "REVOKED"] as const) { - const { container, unmount } = render( - , - ); - expect(container.textContent ?? "").toBe(""); - expect(screen.queryByLabelText("Bid (HNS)")).not.toBeInTheDocument(); - unmount(); - } - }); -}); From dcb766dbb295c86177d7b43c44053a4dea4de34f Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 18:06:06 +0200 Subject: [PATCH 09/26] docs: make the comments say what the code beside them does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pass over every place the review found a comment arguing for behaviour the code no longer has. Two module headers had a coverage note pasted into the middle of a sentence, so `history.rs` read "...so spend attribution" and then, twelve lines later, "needs no extra roundtrips", and `secure_prompt.rs` cut item 4 of its lifecycle list in half. Both notes moved below the prose they interrupted. The quoted percentages went with them: they were stale the first time anyone touched either file, so what is written down now is the shape of the gap. `chain_synced` had lost its own documentation — two constants were inserted between the doc block and the function, so the 23 lines explaining the sync rule described a float. `pending_broadcast_actions_for_name` had two contradicting doc blocks stacked on it, one saying "the most recent wins" and the next saying to look at all of them; they belong to two different functions, and the singular one had no doc at all. Claims that were simply untrue: `namebase.rs` said the keyring-unavailable branch could not be produced deterministically, next to a test helper that produces it; `read.rs` linked a function deleted some commits ago; `chain_scan.rs` called the reveal-to-bid pairing "a best-effort heuristic; each bidder has one BID per name", directly above the exact-outpoint match that replaced it and the comment explaining why; `names.rs` kept an old paragraph calling the expiry constant "the single source of truth" above a new one saying live code reads something else; `OwnershipActions.tsx` described the "Buy with payment" button withdrawn in #62; and `read.rs` pointed at another file for a rule, which the standard names as a sign the rule should be shared instead. One of them was hiding dead code. `GuidedAction`'s BIDDING case returned a wait-for-reveal panel when the task state was `waitingForBidding`, with copy stating the one-bid-per-wallet rule. The BIDDING arm of `derive_auction_task_state` has returned `ReadyToBid` unconditionally since multi-bid, so that branch could not run against a real node. Its test built the impossible pair by hand; it now pins what actually happens — the bid form stays offered to a wallet that has already bid. The flag `alreadyBidWaiting` survives as `biddingNotOpenYet`, which is what `waitingForBidding` means now that only OPENING and a pending OPEN produce it. `Settings.tsx` carried a second implementation of the sync rule with a comment asking whoever changed one to remember the other. `node_status` already computes the shared verdict to pick its read source; it now reports it, the TS mirror carries it, and the screen reads it. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2586 passed), npx tsc -b, npx vitest run (884 passed), npm run lint:format, lint:native-title. --- CHANGELOG.md | 1 + src-tauri/src/commands/chain_scan.rs | 10 ++--- src-tauri/src/commands/history.rs | 19 ++++---- src-tauri/src/commands/namebase.rs | 8 ++-- src-tauri/src/commands/names.rs | 19 ++++---- src-tauri/src/commands/node.rs | 5 +++ src-tauri/src/commands/read.rs | 10 ++--- src-tauri/src/commands/secure_prompt.rs | 19 ++++---- src-tauri/src/db/queries.rs | 28 +++++++----- src-tauri/src/noncustodial/rpc.rs | 12 ++--- src/components/NameActionsModal.tsx | 23 +++++----- src/components/Settings.tsx | 23 ++-------- .../__tests__/name-acquisition.test.tsx | 45 +++++++------------ src/components/__tests__/node-status.test.tsx | 13 +++++- src/components/name-actions/GuidedAction.tsx | 32 ++++--------- src/components/name-actions/NameBidsPanel.tsx | 8 ++-- .../name-actions/OwnershipActions.tsx | 3 -- src/queries/node.ts | 7 +++ 18 files changed, 133 insertions(+), 152 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1816cced..87c81182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and — once a wallet profile exists — whether the node's network matches the wallet's; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **The guided panel no longer tells you a name allows one bid each.** Bidding a second time on a name has been allowed for a while, but a panel left over from the old rule still said otherwise, and it could only appear in a state the wallet stopped producing at the same time. The bid form now stays offered for the whole bidding window, however many bids you already hold. Settings also reads the node's sync verdict from the backend instead of working it out a second time, so the label and what reads actually do cannot disagree. - **The About link in the sidebar has a name again for screen readers.** It is an icon with no text, so its old `title` was also its accessible name; converting the sidebar's hints to tooltips took the `title` away without putting an `aria-label` in its place, leaving the link announced as nothing. The keyboard-shortcuts button beside it already carried one. - **Each wallet profile now really uses its own node.** Per-profile node configuration was stored and resolved, but several paths still asked the global settings: the node the app started took the global api-key, the background sync decided whether "the node" was caught up by asking the global one while every step it gated talked to the profile's, and a sync failure named the global URL in a message telling you to go fix it. Worse, a profile whose own node configuration would not resolve was quietly served the global node instead of being reported as misconfigured, so a profile pinned to a regtest node could be answered by a mainnet one. Each of those now resolves through the profile, and an unresolvable profile configuration is an error rather than somebody else's node. The automatic repair of a stale loopback port also stops rewriting a URL you set on the profile yourself. - **A testnet or regtest wallet no longer reads from the mainnet explorer.** An early migration seeded `explorer_api_url` with the mainnet explorer, and a later change swapped that seed for an empty string so the wallet could pick an explorer per the profile's network. The swap only helped databases created after it: every installation that had already run the original kept the mainnet URL, and an explicit setting outranks the network default, so a testnet or regtest profile went on querying mainnet and getting confident answers about a chain it was not on. A new migration removes exactly the value the old one seeded, leaving an explorer URL you chose yourself alone, and the explorer factory now refuses the known mainnet explorer off mainnet in case a database reaches it before the migration has run. diff --git a/src-tauri/src/commands/chain_scan.rs b/src-tauri/src/commands/chain_scan.rs index 8eff551e..84babc7f 100644 --- a/src-tauri/src/commands/chain_scan.rs +++ b/src-tauri/src/commands/chain_scan.rs @@ -369,11 +369,11 @@ pub(crate) async fn scan_block( ], )?; } - // Match REVEALs to their BIDs by nameHash (within the same block or earlier - // blocks). A REVEAL output's value IS the true bid value; the BID output's - // value is the lockup (bid + mask). We update the FIRST matching BID that - // doesn't already have a reveal_txid — this is a best-effort heuristic; - // in practice each bidder has one BID per name per auction. + // Match REVEALs to their BIDs. A REVEAL output's value IS the true bid + // value; the BID output's value is the lockup (bid + mask). The pairing is + // exact, not a heuristic: each reveal names the outpoint of the bid it + // spends, which is the only thing that still works once a wallet holds + // several bids on one name. for reveal in &reveals { // Address the exact BID this reveal spends. The previous rule — "the // earliest bid not yet matched" — was indistinguishable from the truth diff --git a/src-tauri/src/commands/history.rs b/src-tauri/src/commands/history.rs index 77c2c03a..9b0cf0d6 100644 --- a/src-tauri/src/commands/history.rs +++ b/src-tauri/src/commands/history.rs @@ -13,21 +13,22 @@ //! (`db/queries.rs:1820-1875`). The critical simplification vs. block scanning //! is that `/tx/address` returns fully-decoded inputs with a resolved //! `coin { value, address, covenant }` (see hsd api-docs), so spend attribution -//! -//! COVERAGE: 97.63% line / 97.51% region — realistic ceiling. Remaining ~9 -//! missed lines are all llvm-cov region-boundary artifacts, not real gaps: -//! closing braces inside `classify_tx`'s covenant loop, the `load_wallet_addresses` -//! `query_map` row-closure, `#[tauri::command]` async-wrapper attribute lines, -//! and one sort-comparator arm the stdlib sort never invokes in a<->b order for -//! the tested inputs. The surrounding logic is all exercised. Test harness in -//! `src/tests/history_cmd_tests.rs` uses `MockNodeRpc` for unit tests and -//! mockito regex-match on `GET /tx/address/:addr` for integration tests. //! needs no extra `getrawtransaction` roundtrips. //! //! Covenant constants come from `noncustodial::sync` (verified against hsd //! `lib/covenants/rules.js`). We rely on the numeric `covenant.type`, NOT the //! symbolic `action` string, because the `POST /tx/address` bulk route omits //! the string (and we may add bulk later); the numeric type is always present. +//! +//! Coverage: what this module does not reach is llvm-cov region boundaries +//! rather than untested logic — closing braces inside `classify_tx`'s covenant +//! loop, the `load_wallet_addresses` `query_map` row closure, +//! `#[tauri::command]` async-wrapper attribute lines, and a sort-comparator arm +//! the stdlib sort never invokes in a<->b order for the tested inputs. The test +//! harness in `src/tests/history_cmd_tests.rs` drives `MockNodeRpc` for unit +//! tests and a mockito regex match on `GET /tx/address/:addr` for integration +//! tests. (A percentage used to be quoted here; it went stale the first time +//! anyone touched the file, so the shape of the gap is written down instead.) use std::collections::{BTreeMap, HashSet}; diff --git a/src-tauri/src/commands/namebase.rs b/src-tauri/src/commands/namebase.rs index 7fd098e1..e72c859c 100644 --- a/src-tauri/src/commands/namebase.rs +++ b/src-tauri/src/commands/namebase.rs @@ -2,10 +2,10 @@ // * #[tauri::command] macro attribute lines are structurally uncoverable — the // macro is expanded at compile time and does not execute at runtime. // * The `read_cookie` keyring-unavailable branch (Err(_e) => ... return legacy) -// fires only when `cookie_vault::encrypt_cookie` fails because the OS keyring -// is unreachable. Tests install a fixed test DEK (so encryption always -// succeeds); forcing this branch would require an OS-level keyring failure -// that the test harness can't produce deterministically. +// fires when `cookie_vault::encrypt_cookie` fails because the OS keyring is +// unreachable. `cookie_vault::set_keyring_unavailable_for_test` installs an +// always-failing backend to drive it; see +// `namebase_cmd_tests::read_cookie_returns_legacy_plaintext_when_keyring_unavailable`. // * The poisoned-Mutex fallback in `commands::active_profile::active_profile_network` // (Err(_) => Network::Main) requires a panicked lock holder — not reachable // through the command surface. diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index fbe9554b..def6bd3a 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -338,14 +338,15 @@ pub enum AuctionTaskState { UnavailableOther, } -/// Days-until-expiry threshold below which an owned name's task state becomes -/// [`AuctionTaskState::ExpiringSoon`] (and the Renewals screen flags the row). -/// A missed renewal on Handshake loses the name forever, so this errs early. -/// (A settings-configurable threshold was considered and skipped for now — -/// the constant is the single source of truth, surfaced to the frontend via -/// `read_renewals.expiringSoonThresholdDays`.) -/// Mainnet's expiry warning threshold, kept as a named constant because tests -/// and the notification default both pin the historical 30-day behaviour. +/// Mainnet's days-until-expiry threshold below which an owned name's task +/// state becomes [`AuctionTaskState::ExpiringSoon`] (and the Renewals screen +/// flags the row). A missed renewal on Handshake loses the name forever, so +/// this errs early. Kept as a named constant because tests and the +/// notification default both pin the historical 30-day behaviour, and +/// surfaced to the frontend via `read_renewals.expiringSoonThresholdDays`. +/// +/// Not configurable: a settings-controlled threshold was considered and +/// skipped. /// Live code reads /// [`crate::noncustodial::network::Network::expiring_soon_threshold_days`] /// instead, which scales this to the network's own renewal window — a flat 30 @@ -1327,7 +1328,7 @@ pub(crate) fn build_name_action_capabilities( // `WaitingForBidding` is reused for two distinct situations: a pending OPEN // that hasn't reached BIDDING yet (default reason "The auction opens for // bidding soon.") and a name already in the on-chain BIDDING phase that - // THIS wallet has already bid on (one bid per wallet per name). For the + // THIS wallet has already bid on. For the // latter the default reason reads wrong — bidding is already open and the // wallet's action is to wait for the reveal window, not for bidding to // start — so refine the reason to match the "your bid is placed" panel the diff --git a/src-tauri/src/commands/node.rs b/src-tauri/src/commands/node.rs index 789ef684..9a58335b 100644 --- a/src-tauri/src/commands/node.rs +++ b/src-tauri/src/commands/node.rs @@ -345,6 +345,11 @@ pub async fn node_status(state: State<'_, AppState>) -> Result) - // different chain (e.g. regtest node vs mainnet wallet). // A DB failure here degrades to "no network to compare" — the read // gate is a routing decision, not a security boundary, and the SPV / - // sync gates below still apply. The connection probe (commands/node.rs) - // propagates the same error instead. + // sync gates below still apply. A caller that must not proceed on an + // unknown network propagates the error instead of degrading. let net = queries::get_active_profile_network(&db).ok().flatten(); (mode, net) }; @@ -144,9 +144,9 @@ pub(crate) async fn node_tip_height_if_synced(state: &State<'_, AppState>) -> Op node_tip_height_if_synced_for_network(state, expected_network.as_deref()).await } -/// Same as [`node_tip_height_if_synced_from_settings`], but additionally -/// rejects (returns `None`) when the node's reported `chain` disagrees with -/// `expected_network`. Set `expected_network` to the active profile's stored +/// Resolve the node's tip height from a settings map, returning `None` unless +/// the node is reachable and fully synced — and additionally rejecting when +/// its reported `chain` disagrees with `expected_network`. Set `expected_network` to the active profile's stored /// network string — the schema allows only `"mainnet"`, `"testnet"` and /// `"regtest"`; `"main"` and `"simnet"` are accepted defensively by the /// comparison. Leave it `None` to skip the network check. diff --git a/src-tauri/src/commands/secure_prompt.rs b/src-tauri/src/commands/secure_prompt.rs index 4297dcfd..f1b0238e 100644 --- a/src-tauri/src/commands/secure_prompt.rs +++ b/src-tauri/src/commands/secure_prompt.rs @@ -13,20 +13,19 @@ //! 3. The secure window calls [`secure_prompt_fetch`] to read its request, then //! [`secure_prompt_submit`] with the user's answer, which fulfils the oneshot. //! 4. The originating flow receives the [`SecurePromptResult`] and the window is -//! -//! COVERAGE: 14% — most functions require a live Tauri window (`secure-prompt-` -//! webview) and IPC dispatch. `prompt_secure`, `secure_prompt_fetch`, -//! `secure_prompt_submit`, and `assert_owning_window` all open/manage/close Tauri -//! windows and await oneshot channels from frontend IPC calls — impossible to -//! exercise from a unit test without a full Tauri desktop runtime. The only -//! testable functions are `random_id` (pure RNG) and `push_test_answer` (test -//! helper). All real logic (secret handling, passphrase validation) is in the -//! callers (`commands::secure_wallet`) which ARE tested via the secure-window -//! flow in integration tests. This module is the plumbing. //! closed by the backend. //! //! The secret value (passphrase or mnemonic) only ever flows window <-> backend. //! It is never the return value of a React-invoked command. +//! +//! Coverage here is low and stays low: `prompt_secure`, `secure_prompt_fetch`, +//! `secure_prompt_submit` and `assert_owning_window` open, manage and close +//! Tauri windows and await oneshot channels fulfilled by frontend IPC, which a +//! unit test cannot drive without a full desktop runtime. Only `random_id` and +//! the `push_test_answer` helper are reachable. The logic worth testing — +//! secret handling and passphrase validation — lives in the callers +//! (`commands::secure_wallet`) and is covered through the secure-window +//! integration flow. This module is the plumbing. use std::collections::HashMap; use std::sync::Mutex; diff --git a/src-tauri/src/db/queries.rs b/src-tauri/src/db/queries.rs index cbbf9e0a..ebbc622b 100644 --- a/src-tauri/src/db/queries.rs +++ b/src-tauri/src/db/queries.rs @@ -1559,8 +1559,8 @@ pub fn has_pending_draft_for_name( Ok(false) } -/// The action of a transaction this wallet has BROADCAST for `name` that the -/// chain has not confirmed yet — `Some("open")`, `Some("reveal")`, and so on. +/// Every action this wallet has BROADCAST for `name` that the chain has not +/// mined yet — `"open"`, `"reveal"` and so on — newest first. /// /// This is the gap the UI has to narrate. Between broadcast and the next block /// the chain still reports the name's previous state, so every phase-derived @@ -1569,15 +1569,13 @@ pub fn has_pending_draft_for_name( /// is indefinite. /// /// Only `broadcast_pending`/`broadcasted` count: a `draft` or `signed` row has -/// not left the device, and `confirmed`/`dropped`/`failed` are settled. When -/// several qualify — a name can legitimately have more than one in flight — the -/// most recent wins, which is the one the user just sent. -/// Every action this wallet has broadcast for `name` and the chain has not -/// mined, newest first. More than one can be in flight at once — a register -/// and a redeem on the same name spend different coins and are independent — -/// so a single answer has to pick, and `created_at` has second resolution: -/// two drafts made in the same second order arbitrarily. Callers that ask -/// "is a transaction of this kind in flight?" must look at all of them. +/// not left the device, and `confirmed`/`dropped`/`failed` are settled. +/// +/// More than one can be in flight at once — a register and a redeem on the +/// same name spend different coins and are independent — so a caller asking +/// "is a transaction of this kind in flight?" must look at all of them rather +/// than at the first. `created_at` has second resolution, so two drafts made +/// in the same second order arbitrarily between themselves. pub fn pending_broadcast_actions_for_name( conn: &rusqlite::Connection, profile_id: &str, @@ -1601,6 +1599,14 @@ pub fn pending_broadcast_actions_for_name( Ok(out) } +/// The most recent of [`pending_broadcast_actions_for_name`], or `None` when +/// nothing this wallet sent for `name` is still waiting for a block. +/// +/// "Most recent" is the one the user just pressed, which is what a single +/// "waiting for a block" label should name. Ordering is by `created_at`, which +/// has second resolution, so two drafts made in the same second pick between +/// themselves arbitrarily — a caller that must not miss one of several in +/// flight wants the plural form instead. pub fn pending_broadcast_action_for_name( conn: &rusqlite::Connection, profile_id: &str, diff --git a/src-tauri/src/noncustodial/rpc.rs b/src-tauri/src/noncustodial/rpc.rs index 83b55bfb..5504b93b 100644 --- a/src-tauri/src/noncustodial/rpc.rs +++ b/src-tauri/src/noncustodial/rpc.rs @@ -869,6 +869,12 @@ impl BlockchainInfo { } } +/// Loose floor below which a `blocks == headers` match is distrusted as +/// "headers not yet at the real tip" (see `chain_synced`'s ~8%-verified case). +const HEADERS_MATCH_PROGRESS_FLOOR: f64 = 0.999; +/// Progress gate used when the node reports no header height to compare against. +const PROGRESS_ONLY_SYNCED_GATE: f64 = 0.9999; + /// The one "is this node synced?" rule, shared by the read/write gates, the /// node-status probe and the remote-node connection check. /// @@ -889,12 +895,6 @@ impl BlockchainInfo { /// answer is `assume_when_unknown`: callers gating spends on a configured node /// pass `true` so regtest keeps working, while a first-contact probe of an /// unknown remote node passes `false`. -/// -/// Loose floor below which a `blocks == headers` match is distrusted as -/// "headers not yet at the real tip" (see the ~8%-verified case above). -const HEADERS_MATCH_PROGRESS_FLOOR: f64 = 0.999; -/// Progress gate used when the node reports no header height to compare against. -const PROGRESS_ONLY_SYNCED_GATE: f64 = 0.9999; pub fn chain_synced( blocks: i64, headers: Option, diff --git a/src/components/NameActionsModal.tsx b/src/components/NameActionsModal.tsx index b4b2f335..2697b91b 100644 --- a/src/components/NameActionsModal.tsx +++ b/src/components/NameActionsModal.tsx @@ -350,19 +350,18 @@ export function NameActionsModal({ // leading an auction does not — the owner coin is still a REVEAL. caps?.nameIsRegistered === true; - // A wallet that has bid and is waiting for the window has nothing left to - // submit. This no longer reaches the advanced toggle — `sections.anyLive` - // decides that — and survives only to keep the "unlock to sign" notice off a - // modal with nothing to sign. - const alreadyBidWaiting = caps?.taskState === "waitingForBidding"; + // The auction exists but its bidding window has not opened: the name is in + // OPENING, or an OPEN this wallet broadcast is still waiting for a block. + // (It once also meant "this wallet has already bid", back when a second bid + // was refused; several bids are allowed now, so BIDDING never reports this.) + const biddingNotOpenYet = caps?.taskState === "waitingForBidding"; // Whether the modal actually offers something to sign/broadcast right now. - // `hasRelevantActions` includes a phase-based fallback that is true during - // BIDDING even after THIS wallet has already bid — in that state every - // action is caps-disabled and there is nothing left to submit, so the - // "unlock to sign" notice would be pointless. Gate signable UI on this - // instead of the looser `hasRelevantActions`. - const hasSignableActions = hasRelevantActions && !alreadyBidWaiting; + // `hasRelevantActions` includes a phase-based fallback that is true before + // the bidding window opens, when every action is caps-disabled and there is + // nothing to submit — the "unlock to sign" notice would be pointless there. + // Gate signable UI on this instead of the looser `hasRelevantActions`. + const hasSignableActions = hasRelevantActions && !biddingNotOpenYet; // One rule for the toggle: open it only when something behind it can be // acted on. Every "is this phase meaningful?" special case this used to @@ -818,7 +817,7 @@ export function NameActionsModal({ name={name} profileId={profile?.id ?? null} phase={badge.phase} - suppressEmptyHint={alreadyBidWaiting} + suppressEmptyHint={biddingNotOpenYet} /> {/* A bid this wallet placed in an EARLIER auction of this name whose diff --git a/src/components/Settings.tsx b/src/components/Settings.tsx index 083959c2..06130b48 100644 --- a/src/components/Settings.tsx +++ b/src/components/Settings.tsx @@ -913,28 +913,13 @@ function NodeControl({ dirty, hsdPathConfigured }: { dirty: boolean; hsdPathConf const connected = status?.connected ?? false; const processAlive = status?.process_alive ?? false; - // "Synced" = chain tip reached (applied blocks caught up to best header). - // verificationProgress can plateau just under 1.0 (e.g. ~0.9997 on regtest), - // so a headers match is the ground truth and progress only corroborates it. - // Mirrors the backend `chain_synced` rule in src-tauri/.../rpc.rs — keep the - // two in sync, or the label will disagree with what reads actually do. const height = status?.height ?? null; const headers = status?.headers ?? null; const progress = status?.verification_progress ?? null; - // Loose floor below which a height == headers match is distrusted as - // "headers not yet at the real tip" (the ~8%-verified case). - const HEADERS_MATCH_PROGRESS_FLOOR = 0.999; - const synced = - headers != null && headers > 0 - ? // Headers known: blocks caught up to the tip, and (when reported) - // progress clears the loose floor so a far-behind node stays unsynced. - height != null && - height >= headers && - (progress == null || progress >= HEADERS_MATCH_PROGRESS_FLOOR) - : // No headers to compare — fall back to the progress-only gate. - progress != null - ? progress >= 0.9999 - : true; + // The backend's own verdict, not a second copy of the rule. This used to be + // re-derived here from height/headers/progress with a comment asking whoever + // changed one to remember the other — which is the drift, written down. + const synced = status?.synced ?? false; const pct = progress != null ? Math.floor(progress * 1000) / 10 diff --git a/src/components/__tests__/name-acquisition.test.tsx b/src/components/__tests__/name-acquisition.test.tsx index b1a16d08..4b4f9c31 100644 --- a/src/components/__tests__/name-acquisition.test.tsx +++ b/src/components/__tests__/name-acquisition.test.tsx @@ -545,14 +545,11 @@ describe("NameActionsModal — guided acquisition flow", () => { expect(screen.getByText("Bid")).toBeInTheDocument(); }); - // Regression: a name whose on-chain phase is still BIDDING but for which - // THIS wallet already placed a bid resolves to taskState `waitingForBidding` - // — but because it IS genuinely bidding (phase BIDDING + a placed bid), the - // badge now unifies with the modal's on-chain phase and reads "Bidding". - // The guided body used to key purely on the raw phase and render a - // "Place a Bid"-flavored panel; it must render the wait-for-reveal panel - // and offer no bid CTA. - it("shows a wait-for-reveal panel (not a bid CTA) for a BIDDING name already bid by this wallet", async () => { + // Regression: a BIDDING name this wallet has already bid on still invites + // another bid. Several independent bids per name are allowed, so the guided + // panel must keep the bid form rather than collapsing to the wait-for-reveal + // panel it used to show once one bid was placed. + it("still offers the bid form for a BIDDING name this wallet has already bid on", async () => { invokeMock.mockImplementation( routeModal( { @@ -574,33 +571,27 @@ describe("NameActionsModal — guided acquisition flow", () => { capabilities: { name: "bidname", phase: "BIDDING", - taskState: "waitingForBidding", + taskState: "readyToBid", ownsName: false, nameIsRegistered: false, transferPending: false, redeemableRevealCount: 0, redeemableValueDoos: 0, hasBidCommitment: true, + myBidCount: 1, hasRevealCoin: false, hasOwnerCoin: false, canOpen: { allowed: false, reason: null }, - canBid: { - allowed: false, - reason: - "you already have a bid commitment for this name (one bid per wallet per name)", - }, + canBid: { allowed: true, reason: null }, canReveal: { allowed: false, reason: "Reveal not open yet" }, canRedeem: { allowed: false, reason: null }, canRegister: { allowed: false, reason: "Phase is BIDDING" }, canUpdate: { allowed: false, reason: null }, canTransfer: { allowed: false, reason: null }, canFinalize: { allowed: false, reason: null }, - nextActionKey: "WAIT", - nextActionLabel: "Wait for Bidding", - // Backend refines this reason for the already-bid BIDDING case - // (see the next_action override in names.rs). The frontend - // no-countdown panel renders the SAME string verbatim. - nextActionReason: "Your bid is placed. Wait for the reveal window to open.", + nextActionKey: "BID", + nextActionLabel: "Place a Bid", + nextActionReason: null, countdownLabel: null, countdownBlocks: null, countdownHours: null, @@ -610,18 +601,12 @@ describe("NameActionsModal — guided acquisition flow", () => { ); render( {}} />, { wrapper: wrapper() }); - // The badge reads "Bidding" (genuine-bidding unification), matching the - // on-chain phase — not "Waiting for Bidding". + // The badge matches the on-chain phase. expect(await screen.findByText("Bidding")).toBeInTheDocument(); expect(screen.queryByText("Waiting for Bidding")).not.toBeInTheDocument(); - // The guided body is the wait-for-reveal panel, not the bid panel. - expect(screen.getByTestId("bidding-waiting")).toBeInTheDocument(); - // Exact unified copy (verbatim match with backend next_action_reason). - expect( - screen.getByText("Your bid is placed. Wait for the reveal window to open."), - ).toBeInTheDocument(); - // No bid-flavored guided copy under the "Bidding" badge. - expect(screen.queryByText("Place a Bid")).not.toBeInTheDocument(); + // The bid form is still offered, holding an existing bid notwithstanding. + expect(screen.getAllByLabelText("Bid (HNS)").length).toBeGreaterThan(0); + expect(screen.getAllByLabelText("Lockup (HNS)").length).toBeGreaterThan(0); }); it("shows Reveal for a REVEAL name", async () => { diff --git a/src/components/__tests__/node-status.test.tsx b/src/components/__tests__/node-status.test.tsx index e6973442..0d5de6dd 100644 --- a/src/components/__tests__/node-status.test.tsx +++ b/src/components/__tests__/node-status.test.tsx @@ -40,6 +40,7 @@ type NodeOver = Partial<{ height: number | null; verification_progress: number | null; headers: number | null; + synced: boolean; last_error: string | null; index_mismatch: boolean; read_source: "local" | "explorer"; @@ -57,6 +58,11 @@ function nodeStatus(over: NodeOver = {}) { height: null, verification_progress: null, headers: null, + // The backend's own `chain_synced` verdict. The rule itself is tested in + // Rust (`chain_synced`); these tests pin what the UI renders when handed + // each answer, so each case states the verdict it is exercising rather + // than re-deriving one here. + synced: false, last_error: null, index_mismatch: false, read_source: "explorer", @@ -105,7 +111,7 @@ beforeEach(() => { describe("Node status (truthful, RPC-based)", () => { it("Settings shows Connected · block N when the RPC answers", async () => { invokeMock.mockImplementation( - route(nodeStatus({ connected: true, process_alive: true, height: 218456 })), + route(nodeStatus({ connected: true, process_alive: true, height: 218456, synced: true })), ); render(, { wrapper: wrapper() }); @@ -123,6 +129,7 @@ describe("Node status (truthful, RPC-based)", () => { height: 40000, headers: 100000, verification_progress: 0.4, + synced: false, }), ), ); @@ -146,6 +153,7 @@ describe("Node status (truthful, RPC-based)", () => { height: 65027, headers: 65027, verification_progress: 0.19, + synced: false, }), ), ); @@ -172,6 +180,7 @@ describe("Node status (truthful, RPC-based)", () => { height: 317, headers: 317, verification_progress: 0.9997, + synced: true, }), ), ); @@ -193,6 +202,7 @@ describe("Node status (truthful, RPC-based)", () => { height: 317, headers: 317, verification_progress: 0.08, + synced: false, }), ), ); @@ -211,6 +221,7 @@ describe("Node status (truthful, RPC-based)", () => { height: 317, headers: 317, verification_progress: 0.9999, + synced: true, }), ), ); diff --git a/src/components/name-actions/GuidedAction.tsx b/src/components/name-actions/GuidedAction.tsx index 53c1e062..a79b4571 100644 --- a/src/components/name-actions/GuidedAction.tsx +++ b/src/components/name-actions/GuidedAction.tsx @@ -180,30 +180,14 @@ export function GuidedAction({ ); case "BIDDING": - // Task-state precedence over raw on-chain phase: once THIS wallet has - // placed its bid the backend reports `waitingForBidding` (the badge - // reads "Waiting for Bidding"). The name is still in the on-chain - // BIDDING phase, but there is no bid action left for this wallet — one - // bid per wallet per name — so render a wait-for-reveal panel instead - // of the bid form. The switch keyed purely on `badge.phase`, so an - // already-bid name showed the "Place a Bid" form under a - // "Waiting for Bidding" badge. - if (caps?.taskState === "waitingForBidding") { - return ( -
- You already placed a bid for this name (one bid per wallet per name). Wait for the - reveal window, then reveal your bid. -
- {countdown - ? `Your bid is placed. Reveal opens in ${formatCountdown(countdown)}.` - : // Verbatim match with the backend `next_action_reason` for this - // already-bid WaitingForBidding case (see next_action override - // in names.rs). Keep the two strings identical. - "Your bid is placed. Wait for the reveal window to open."} -
-
- ); - } + // A wallet that has already bid is still invited to bid again: several + // independent bids on one name are allowed for the rest of the window. + // This case used to collapse to a wait-for-reveal panel once a bid was + // placed, keyed on the backend reporting `waitingForBidding` during + // BIDDING — which it no longer does, and the panel's copy still named + // the one-bid-per-wallet rule that was removed with it. How many bids + // are yours is said by the modal header and the tinted rows in the bid + // list instead. return (
Ownership
diff --git a/src/queries/node.ts b/src/queries/node.ts index 545cf254..e94c652a 100644 --- a/src/queries/node.ts +++ b/src/queries/node.ts @@ -19,6 +19,13 @@ export interface NodeStatus { verification_progress: number | null; /** Peers' best header height (the sync target), when reported. */ headers: number | null; + /** + * Whether the node has reached the chain tip, as the backend's own + * `chain_synced` rule decides it — the same verdict the read and write gates + * act on. Read this rather than comparing height/headers/progress here, or + * the label and what reads actually do can disagree. + */ + synced: boolean; /** Why the last start failed (with log tail), when the RPC isn't answering. */ last_error: string | null; /** The failure is a chain/index mismatch hsd can't fix in place → offer re-sync. */ From 5ecea8344abfb033527d216797fb86c268066598 Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 18:08:32 +0200 Subject: [PATCH 10/26] docs: stop the manual promising what the wallet does not do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UI text, doc comments and `docs/*.md` may only claim what the code enforces. Six places claimed more. Paid name swaps were withdrawn in #62 and the spec explains at length why the shape could not work — but the manual still walked the user through both flows step by step, the README still listed the feature and promised "neither party can renege", the owned-name action table still offered "Buy with payment", and the QA checklist still asked a tester to find the buttons. All four now say the feature is gone, why, and what survives: an offer recorded before the withdrawal can still be claimed. The manual's batch-operations section described hsd's `createbatch` RPC and automatic chunking, followed by a note saying the description was inaccurate and would be rewritten later. Neither exists. The section now states the behaviour once: one transaction, one txid, capped at 100 names, no chunking. `NODE_SETUP.md` told SPV users to open a "Node mode" dropdown that #52 replaced with the Chain source selector, and said the network comparison only starts applying in Settings because onboarding has no wallet yet — which #54 changed, and onboarding now compares against the network picked on the previous step. The CHANGELOG entry for that carried the same stale clause. It also described per-slot read and write overrides and slot-independent resolution as how the wallet stores node config. There is no slot column and resolution returns one tuple; that is step 8 of the per-profile spec, now labelled as planned. And it documented no rule for when a node counts as synced, although that one rule decides reads, writes, the status panel and "Test connection". It does now, including both fallbacks and why a first-contact probe assumes the opposite of a configured node. Two spec pointers had gone stale: N14 named `NameInfoModal`, folded into `NameActionsModal` in #57, and R7 described the probe reading the network from the DB, which since #54 is the fallback for when the caller supplies none. The QA checklist named the deleted modal too. Gates: npm run lint:format. --- CHANGELOG.md | 3 +- README.md | 3 - docs/NODE_SETUP.md | 55 ++++++++++++++----- docs/QA_WORKFLOWS.md | 12 ++-- docs/USER_MANUAL.md | 46 +++++++--------- ...ote-node-connection-and-broadcast-guard.md | 7 ++- .../2026-09-14-network-derived-behaviour.md | 7 ++- 7 files changed, 78 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87c81182..9ab39f17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,10 @@ ### Added - **Several bids on one name.** The wallet allowed a single bid per name per wallet; it now allows as many as you like, each with its own value and lockup, the way Namebase does. Every bid rotates to a fresh receive address, so each gets its own nonce, blind, commitment row and BID coin, and the send-side guards that refused a second one are gone. The name modal's header reads "Latest bid … · lockup … · N of yours", and the bids panel keeps every bid in one list with your own rows tinted, so which are yours is visible without counting. -- **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and — once a wallet profile exists — whether the node's network matches the wallet's; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. +- **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and whether the node's network matches the wallet's — during onboarding, the network you picked on the previous step; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **The manual and README no longer advertise paid name swaps.** The two buttons were withdrawn because the shape they implemented could not be atomic and could only be pressed by the party with nobody to pay, but the manual still walked you through using them, the README still promised that neither party could renege, and the QA checklist still looked for them. All three now say the feature is gone and what remains: an offer recorded earlier can still be claimed. The manual also had a note admitting its own batch-operations section was inaccurate; the section now says what the wallet does instead. - **The guided panel no longer tells you a name allows one bid each.** Bidding a second time on a name has been allowed for a while, but a panel left over from the old rule still said otherwise, and it could only appear in a state the wallet stopped producing at the same time. The bid form now stays offered for the whole bidding window, however many bids you already hold. Settings also reads the node's sync verdict from the backend instead of working it out a second time, so the label and what reads actually do cannot disagree. - **The About link in the sidebar has a name again for screen readers.** It is an icon with no text, so its old `title` was also its accessible name; converting the sidebar's hints to tooltips took the `title` away without putting an `aria-label` in its place, leaving the link announced as nothing. The keyboard-shortcuts button beside it already carried one. - **Each wallet profile now really uses its own node.** Per-profile node configuration was stored and resolved, but several paths still asked the global settings: the node the app started took the global api-key, the background sync decided whether "the node" was caught up by asking the global one while every step it gated talked to the profile's, and a sync failure named the global URL in a message telling you to go fix it. Worse, a profile whose own node configuration would not resolve was quietly served the global node instead of being reported as misconfigured, so a profile pinned to a regtest node could be answered by a mainnet one. Each of those now resolves through the profile, and an unresolvable profile configuration is an error rather than somebody else's node. The automatic repair of a stale loopback port also stops rewriting a URL you set on the profile yourself. diff --git a/README.md b/README.md index b6dc3cbf..01d5c5bd 100644 --- a/README.md +++ b/README.md @@ -59,9 +59,6 @@ Built with Tauri v2, React + TypeScript, Rust, and SQLite. - **Name watchlist** — track names you don't own for monitoring. Watchlist page in the sidebar with add/remove, tags, CSV import/export, and "Add to Watchlist" buttons in name modals. -- **Paid name swaps** — atomic buyer-seller name transfer with payment. The buyer - finalizes a TRANSFER and pays the seller in a single transaction - (finalizeWithPayment), so neither party can renege after the lockup expires. - **Recover lost bids** — if you lose your local bid data (reinstall, seed- restore, import from another wallet), Namehold can brute-force the bid value from your seed and reveal it before the window closes. See diff --git a/docs/NODE_SETUP.md b/docs/NODE_SETUP.md index d8f53658..9387b055 100644 --- a/docs/NODE_SETUP.md +++ b/docs/NODE_SETUP.md @@ -57,13 +57,13 @@ Instead of running hsd locally, you can point the wallet at an existing hsd RPC: back), so "Test connection" reuses your stored key when the URL you probe matches the saved node. A freshly typed URL is probed without the stored key — the secret is never sent to an endpoint you just typed. -- **Network mismatch:** once a wallet profile exists, "Test connection" compares the node's - reported network with your wallet's and flags a mismatch (e.g. a testnet node for a - mainnet wallet). A mismatched node is refused everywhere: the app will not read from it, - will not sync from it, reports it as unable to send, and refuses to broadcast through it — - the signed transaction never leaves your device. Settings will not save one either. - During first-run onboarding there is no wallet yet, so the comparison starts applying in - Settings. +- **Network mismatch:** "Test connection" compares the node's reported network with the + wallet's and flags a mismatch (e.g. a testnet node for a mainnet wallet). A mismatched + node is refused everywhere: the app will not read from it, will not sync from it, reports + it as unable to send, and refuses to broadcast through it — the signed transaction never + leaves your device. Settings will not save one either. This applies during first-run + onboarding too: there is no wallet profile yet, so the comparison uses the network you + picked on the previous step. - **To send:** enable "Allow sending via remote node" (off by default for safety; shown in the onboarding Remote step and in Settings → Connections). This sets the `allow_remote_broadcast` flag, which gates the broadcast path. Your recovery phrase @@ -118,15 +118,38 @@ rows inside the shared database or in the node's own datadir. | Path | What it holds | |------|---------------| -| `~/.namehold/portfolio.db` | The shared SQLite database: wallet profiles, portfolio (UTXOs, name states, transactions), and **all node configuration** (global + per-profile `node_rpc_url`, `node_rpc_api_key`, `chain_source`, per-slot overrides). Used by both the GUI and the `namehold-syncd` daemon. | +| `~/.namehold/portfolio.db` | The shared SQLite database: wallet profiles, portfolio (UTXOs, name states, transactions), and **all node configuration** (global + per-profile `node_rpc_url`, `node_rpc_api_key`, `chain_source`). Used by both the GUI and the `namehold-syncd` daemon. | | `~/.namehold/syncd.pid` | PID file for the background sync daemon (see above). | -**Node RPC config is not stored in files.** The regtest/testnet/mainnet -connection settings you enter under **Settings → Node RPC** are written as rows -in `portfolio.db` (keyed by wallet profile, plus a global default), not as a -per-network config file. Resolution runs independently for the read slot and -the write (send) slot: per-slot per-profile override → global setting → -built-in default. +**Node RPC config is not stored in files.** The connection settings you enter +under **Settings → Connections** are written as rows in `portfolio.db` (a global +default, plus per-profile overrides), not as a per-network config file. Each key +resolves on its own: per-profile override → global setting → built-in default. +Splitting the read and write sides so a profile could read through one source +and send through another is planned, not built — see step 8 of +`docs/specs/2026-09-15-per-profile-node-banner-and-preflight.md`. + +### When the wallet calls a node "synced" + +The chain tip is the test: the node is synced once the blocks it has applied +have caught up to the best header it knows about. `verificationprogress` only +corroborates that, because it can plateau just below 1.0 — around 0.9997 on +regtest — and a node sitting at the tip would otherwise never qualify. To keep +a node that reports `blocks == headers` while barely verified from passing, +progress (when reported at all) must also clear a loose 0.999 floor. + +Two fallbacks matter in practice: + +- **No header height reported.** Older builds and some nodes do not send one. + The wallet falls back to `verificationprogress >= 0.9999`. +- **Neither reported.** A regtest node with a single miner reports no sync + metadata at all. A gate on a node you configured assumes synced, so regtest + keeps working; a first-contact probe of a node you have just typed in assumes + the opposite, so an unknown node is not trusted on no evidence. + +One rule answers this everywhere — the read gate, the write gate, the node +status panel and "Test connection" — so the label in Settings cannot disagree +with what reads actually do. ### Node (hsd) datadirs and default RPC ports @@ -158,7 +181,9 @@ For users who don't need to send transactions or want faster initial setup: explorer. - **Explorer failover** — set a fallback URL in Settings for when the primary explorer is unreachable. -- **To enable:** Settings → Connections → Node mode → select "SPV" → Save. +- **To enable:** Settings → Connections → Chain source → select "Local node (SPV)" + → Save. There is no separate "Node mode" dropdown; the one selector sets both + the source and, for a local node, whether it runs full or SPV. - **Status indicator:** StatusStrip shows "Explorer (SPV)" when SPV mode is active. SPV mode is ideal for: diff --git a/docs/QA_WORKFLOWS.md b/docs/QA_WORKFLOWS.md index 520f8c75..8ba90d3b 100644 --- a/docs/QA_WORKFLOWS.md +++ b/docs/QA_WORKFLOWS.md @@ -208,7 +208,7 @@ hsd-cli rpc generate 1440 # advance through REVEAL phase ### H. Watchlist - [ ] Watchlist page visible in sidebar -- [ ] Add a name to the watchlist from NameActionsModal or NameInfoModal +- [ ] Add a name to the watchlist from the name modal - [ ] Remove a name from the watchlist - [ ] Watchlist table shows Countdown, Highest bid, Expires columns - [ ] Owned names get an inline Owned badge @@ -226,10 +226,12 @@ hsd-cli rpc generate 1440 # advance through REVEAL phase - [ ] "Reveal Selected" / "Redeem Selected" / "Finalize Selected" work similarly - [ ] Deselecting all hides the batch action bar -### J. Paid name swaps -- [ ] Names in TRANSFER state show "Buy with payment" button (buyer side) -- [ ] "Sell with payment" flow available for owned names in TRANSFER -- [ ] Saved offers list shows pending offers +### J. Paid name swaps (withdrawn) +The two entry points were removed on 2026-09-21; see +`docs/specs/2026-09-21-paid-name-swaps.md`. What remains is the claim path for +an offer recorded before that. +- [ ] No "Buy with payment" or "Sell with payment" button appears on any name +- [ ] Saved offers list shows pending offers recorded earlier - [ ] claim_paid_transfer verifies the broadcast tx before marking paid ### K. Bid recovery diff --git a/docs/USER_MANUAL.md b/docs/USER_MANUAL.md index ac0ae9f2..1f2444fd 100644 --- a/docs/USER_MANUAL.md +++ b/docs/USER_MANUAL.md @@ -347,7 +347,6 @@ In the Name Actions modal for an owned name, click **Show all actions**: | **Finalize** | Complete a transfer after the lockup period (mainnet: ~2 days). | | **Cancel** | Revert a pending transfer before it's finalized. | | **Revoke** | Permanently burn the name (irreversible). | -| **Buy with payment** | Finalize a transfer AND pay the seller in a single transaction (atomic swap). | All of these need the signer unlocked and a synced node. @@ -367,34 +366,29 @@ Each batch action opens a **confirmation modal** showing the count, estimated fee, and a collapsible list of the selected names. Cancel closes without broadcasting; Confirm signs + broadcasts the draft in one step. -Batch operations use hsd's `createbatch` RPC, which handles consensus limits -automatically (chunking to stay under block-size limits). - -**Note:** The description above is inaccurate and kept only until the next -manual pass rewrites this section. In reality, batch operations are built -client-side as a single transaction (there is no `createbatch` RPC in hsd). -The client enforces a conservative `MAX_BATCH_SIZE=100` names per batch, -well below hsd's per-transaction covenant limits (300 OPENS, 600 UPDATES, -600 RENEWALS). Per-block limits are identical to per-tx limits, so a -100-item batch will never be rejected on covenant-count grounds. Chunking -is not implemented — each batch is one atomic transaction with one txid. +A batch is built in the wallet as one transaction with one txid, so it either +all lands or none of it does. There is no chunking and no `createbatch` RPC in +hsd. The wallet caps a batch at 100 names, well under hsd's per-transaction +covenant limits (300 OPENs, 600 UPDATEs, 600 RENEWs), and per-block limits are +the same as per-transaction ones — so a full batch is never refused for +carrying too many covenants. ### Paid name swaps -To sell a name for HNS (**Sell with payment** flow): -1. Open the name in **Manage** → **Sell with payment** section. -2. Enter the buyer's address, your price (HNS), and confirm. This transfers - the name to the buyer with a lockup period recorded as a saved offer. -3. Wait for the buyer to broadcast their finalize-with-payment tx. -4. Once the buyer's tx confirms, the app verifies it (checks the payment - output matches your offer) and marks the offer paid — HNS lands in your - wallet atomically. - -To buy a name: -1. Wait for the seller to transfer the name to your address (name shows TRANSFER state). -2. Click **Buy with payment** → enter seller's address + amount. -3. Review the draft → sign → broadcast. -4. The name is finalized and the seller is paid in the same transaction. +Not available. The wallet once offered "Sell with payment" and "Buy with +payment"; both were withdrawn on 2026-09-21 because the shape they implemented +could not do what the names promised. A transfer's coin stays at the seller's +address, so only the seller can finalize — which left "Buy with payment" +pressable only by the party with nobody to pay — and nothing about the +transaction was atomic, so "one transaction" meant one wallet funding both +halves of its own trade. + +Selling a name for HNS therefore means transferring it and being paid +separately, with the trust that implies. If you recorded an offer before the +buttons were withdrawn, its claim panel still appears and still works. + +`docs/specs/2026-09-21-paid-name-swaps.md` has the consensus rules behind this +and what a working implementation would need. --- diff --git a/docs/specs/2026-09-11-remote-node-connection-and-broadcast-guard.md b/docs/specs/2026-09-11-remote-node-connection-and-broadcast-guard.md index 249227ae..2925385a 100644 --- a/docs/specs/2026-09-11-remote-node-connection-and-broadcast-guard.md +++ b/docs/specs/2026-09-11-remote-node-connection-and-broadcast-guard.md @@ -85,9 +85,12 @@ reported as "syncing", not "synced". (The read gate uses `true` for the same call because a regtest miner never reports progress — the two call sites are deliberately different and each says why.) -**R7 — Network comparison in the probe.** `check_node_connection` reads +**R7 — Network comparison in the probe.** `check_node_connection` takes the +network to compare against from its caller when one is supplied — which is how +onboarding compares before any profile exists — and otherwise reads `db::queries::get_active_profile_network(&conn)` (a DB error is returned to -the UI, not swallowed) and passes it to `check_node_connection_with_client`, +the UI, not swallowed). Either way it passes the answer to +`check_node_connection_with_client`, which sets `network_matches = noncustodial::network::network_check(expected, info.chain)`. Semantics: `Some(false)` = mismatch; `Some(true)` = match; `None` = nothing to compare (no active profile, or node reports no `chain`). diff --git a/docs/specs/2026-09-14-network-derived-behaviour.md b/docs/specs/2026-09-14-network-derived-behaviour.md index f07d14eb..4c5d7974 100644 --- a/docs/specs/2026-09-14-network-derived-behaviour.md +++ b/docs/specs/2026-09-14-network-derived-behaviour.md @@ -163,10 +163,11 @@ simnet mine on demand, so the old arithmetic invented six blocks for every idle hour and every renewal countdown drifted. A stale height is reported as stale. Pinned by `test_has_wall_clock_block_timing_all_variants`. -**N14 — Mainnet-only explorer links appear only on mainnet.** `NameInfoModal` -and `NameActionsModal` gate their "View on explorer" link on +**N14 — Mainnet-only explorer links appear only on mainnet.** +`NameActionsModal` gates its "View on explorer" link on `profile.network === "mainnet"`, as `TxInfoModal`, `BlockInfoModal`, -`ReceiveAddressList` and `WalletView` already did. Shakeshift indexes no other +`ReceiveAddressList` and `WalletView` already did. (`NameInfoModal` carried the +same gate and was folded into `NameActionsModal` in #57.) Shakeshift indexes no other chain, so the link 404s elsewhere. **N15 — Starting a node refuses rather than guessing mainnet.** From 1bfbd04c089a8af1abb70baf4f62e1abfdd5d8aa Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 18:12:26 +0200 Subject: [PATCH 11/26] docs(specs): write down the behaviour that shipped without a spec `docs/specs/README.md` says a spec is the source of truth a reviewer checks the code against, and that when behaviour changes the spec changes in the same PR. Two gaps against that. #62 changed the multiple-bids feature in six places and touched only the new paid-swaps spec. Those six are now requirements, each naming the code that enforces it and the test that pins it: - R11b grew from two capabilities to five. Renew was ungated although hsd's RENEW handler ends a transfer exactly as UPDATE does, and Transfer was offered on a name already being transferred; Revoke stays offered because consensus allows it and destroying the name is what the button says. - R11c, the FINALIZE lockup gate, including why it prefers the live tip and why an unknown height leaves the action offered. - R11d, the `batch-` prefix strip, without which a batched transfer read as no transfer at all. - R13 gained the Owned Names State column, the fourth surface reading `pendingBroadcastAction` and the last one that did not. - R13b, TRANSFER as a task rather than a phase hsd never reports, and where it ranks. - R13c, REDEEM classified as spendable, which is what made reclaimed lockups visible in the balance again. - R13d, the confirm dialog summing every non-change output, which replaces a Known gap the same commit had closed and left standing. Batch name operations had no spec at all: six commands, a frontend selection flow, a confirmation modal, a user-manual section and a QA checklist, shipped in #53 across every layer the README says requires one. `docs/specs/2026-09-15-batch-name-operations.md` records what is enforced (one transaction and one txid, the 100-name cap and why that number, abort-before-write on a bad name, atomic reservation, a batch of one taking the same path) and what is not (no chunking, no per-name result, no cross-covenant batching). Every pointer in it resolves. Also in the registry: the new spec is listed, and three rows named the feature branch they shipped on rather than saying whether the behaviour is in the product. Gates: npm run lint:format. No code changed. --- .../specs/2026-09-15-batch-name-operations.md | 146 ++++++++++++++++++ .../2026-09-20-multiple-bids-per-name.md | 103 ++++++++++-- docs/specs/2026-09-21-paid-name-swaps.md | 4 + docs/specs/README.md | 7 +- 4 files changed, 240 insertions(+), 20 deletions(-) create mode 100644 docs/specs/2026-09-15-batch-name-operations.md diff --git a/docs/specs/2026-09-15-batch-name-operations.md b/docs/specs/2026-09-15-batch-name-operations.md new file mode 100644 index 00000000..d9f6df96 --- /dev/null +++ b/docs/specs/2026-09-15-batch-name-operations.md @@ -0,0 +1,146 @@ +# Batch name operations + +Status: implemented. Written after the fact, when a review found the feature +had shipped across backend, frontend and user docs with no spec. + +## 1. Summary + +Acting on one name at a time is the wrong unit of work for a portfolio: a +hundred names come up for renewal together, and a wallet that bid on twenty +names has twenty reveals to make in the same window. The user selects names in +the Owned Names table, presses one button, sees a confirmation naming the count +and the fee, and signs once. The result is a single transaction with a single +txid — it all lands or none of it does. + +## 2. Terms + +- **Batch** — one transaction carrying the same covenant for several names. + Not a queue and not a series: there is one draft, one signature and one txid. +- **Batch size** — how many names one batch carries. Capped at + `MAX_BATCH_SIZE`. +- **Per-name prefetch** — the `(name, name_hash, owner_coin, on_chain_state)` + tuple the command resolves for every name before it builds anything, so a + name that cannot participate stops the batch before any write. +- **Inner builder** — the `build_batch_*_draft_inner` function: the pure half, + taking a `Connection` and the prefetch rather than a Tauri `State`. + +## 3. Requirements + +**B1 — Six covenants can be batched.** `bid`, `renew`, `transfer`, `reveal`, +`redeem` and `finalize`. Nothing else is offered in bulk. +*Enforced:* `commands/names.rs::build_batch_{bid,renew,transfer,reveal,redeem,finalize}_draft`. +*Pinned:* the happy path of each — `build_batch_bid_draft_tests::batch_bid_happy_path_persists_summary_commitments_and_draft`, +`build_batch_renew_draft_tests::batch_renew_happy_path_persists_draft`, +`build_batch_transfer_draft_tests::batch_transfer_happy_path_persists_draft`, +`build_batch_reveal_draft_tests::batch_reveal_happy_path_persists_draft`, +`build_batch_redeem_draft_tests::batch_redeem_happy_path_persists_draft`, +`build_batch_finalize_draft_tests::batch_finalize_happy_path_persists_draft`. + +**B2 — A batch is one transaction, not a chunked series.** There is no +`createbatch` RPC in hsd and no chunking here: every batch builder calls +`actions::build_batch_plan` once and persists one draft. +*Enforced:* `commands/names.rs` (each `*_inner` → `build_batch_plan` → +`persist_with_conn`). +*Pinned:* the happy-path tests above assert a single persisted draft. + +**B3 — At most 100 names per batch.** `MAX_BATCH_SIZE = 100`, refused with the +count and the limit in the message. The number is deliberately well under +hsd's per-transaction covenant limits (300 OPENs, 600 UPDATEs, 600 RENEWs); +per-block limits are the same as per-transaction ones, so a full batch is never +refused for carrying too many covenants. +*Enforced:* `commands/names.rs::MAX_BATCH_SIZE` and the length check in each +batch command. + +**B4 — One bad name aborts the whole batch, before any write.** Every name's +owner coin and on-chain state are resolved up front, and a shared destination +is decoded up front, so a name in the wrong phase or an unparseable address +fails while nothing has been persisted or reserved. A partially applied batch +would be worse than none: the user would have to work out which half went. +*Enforced:* the per-name prefetch loop and the early `address::decode` in each +batch command. +*Pinned:* `build_batch_bid_draft_tests::batch_bid_one_bad_phase_aborts_entire_batch`, +`build_batch_finalize_draft_tests::batch_finalize_rejects_non_transfer_coin`, +`build_batch_reveal_draft_tests::batch_reveal_bad_nonce_length_errors`. + +**B5 — Coin reservation and draft persistence are atomic.** The inner builder +requires the caller to hold the DB mutex for its whole duration, so the coins a +batch reserves and the draft that claims them are written under one guard. A +second batch cannot select a coin the first has taken. +*Enforced:* each `build_batch_*_draft_inner` (documented on the function), with +the wrapper taking `state.db.lock()` immediately before the call. +*Pinned:* `build_batch_transfer_draft_tests::batch_transfer_happy_path_persists_draft` +asserts the reservation count. + +**B6 — A batch transfer names one recipient, and the value stays put.** The +recipient is decoded once and written into every name's TRANSFER covenant; the +output value stays at each name's current owner address until FINALIZE moves +it, as consensus requires. +*Enforced:* `commands/names.rs::build_batch_transfer_draft_inner`. +*Pinned:* `build_batch_transfer_draft_tests::batch_transfer_uses_owner_address_for_output`. + +**B7 — A batch of one is still a batch.** Selecting a single name takes the +same path rather than falling back to the singular command, so the two cannot +drift. +*Pinned:* `batch_transfer_single_name`, `batch_renew_single_name`, +`batch_reveal_single_name`, `batch_redeem_single_name`, +`batch_finalize_single_name`. + +**B8 — An empty selection is refused where it is meaningless.** `transfer` and +`finalize` reject an empty name list outright. `bid`, `renew`, `reveal` and +`redeem` build a zero-input draft instead, which is what their callers expect +when a filter matches nothing. +*Pinned:* `batch_transfer_empty_errors`, `batch_finalize_empty_errors`, +`batch_bid_empty_specs_persists_zero_output_draft`, +`batch_renew_empty_persists_zero_input_draft`, +`batch_reveal_empty_persists_zero_input_draft`, +`batch_redeem_empty_persists_zero_input_draft`. + +**B9 — The confirmation says what will be signed.** `BatchConfirmModal` shows +the count, the estimated fee and a collapsible list of the selected names. +Cancel closes without broadcasting; Confirm signs and broadcasts in one step. +The amount it reports sums every output except change (R13d of the +[multiple-bids spec](./2026-09-20-multiple-bids-per-name.md)), which matters +here because a batch reveal or redeem carries one output per bid. +*Enforced:* `src/components/BatchConfirmModal.tsx`, driven from +`src/components/WalletView.tsx`. +*Pinned:* `src/components/__tests__/batch-transfer.test.tsx`. + +**B10 — A batched action still counts as spending the name.** The draft a +batch builder persists records itself as `batch-`, and the owner-spend +check strips that prefix — see R11d of the +[multiple-bids spec](./2026-09-20-multiple-bids-per-name.md). + +## 4. Explicitly not enforced + +- **No chunking.** A selection over `MAX_BATCH_SIZE` is refused, not split. + The user reduces the selection. +- **No per-name result.** A batch is one transaction, so there is no notion of + "eight succeeded, two failed" — the node accepts it or it does not. +- **No cross-covenant batching.** Renewing some names and revealing others in + one transaction is possible on-chain but not offered; each button carries one + covenant. +- **No fee negotiation per name.** One fee rate applies to the whole batch. + +## 5. Known gaps + +- **The 100-name cap is a round number, not a measured limit.** It is well + under the consensus covenant limits, but the real ceiling is transaction + size, which depends on the inputs coin selection picks. A batch of 100 names + with many small inputs could still be large. +- **`build_batch_*_draft` commands carry `coverage(off)`.** They are IO shells, + which the standard allows, but it means the length and prefetch checks are + covered only through their inner builders and the frontend tests. + +## 6. Pointers + +- Backend: `src-tauri/src/commands/names.rs` (`MAX_BATCH_SIZE`, the six + `build_batch_*_draft` commands and their `*_inner` halves), + `src-tauri/src/noncustodial/actions.rs` (`build_batch_plan`). +- Frontend: `src/components/BatchConfirmModal.tsx`, + `src/components/WalletView.tsx` (selection + action bar), + `src/components/BatchBidModal.tsx`. +- Tests: `src-tauri/src/tests/build_batch_{bid,renew,transfer,reveal,redeem,finalize}_draft_tests.rs`, + `src/components/__tests__/batch-transfer.test.tsx`. +- Docs: `docs/USER_MANUAL.md` ("Batch operations"), `docs/QA_WORKFLOWS.md` + (section I). +- CHANGELOG: the batch-transfer entry under `## [Unreleased]`. diff --git a/docs/specs/2026-09-20-multiple-bids-per-name.md b/docs/specs/2026-09-20-multiple-bids-per-name.md index 375a6eb2..4d19da34 100644 --- a/docs/specs/2026-09-20-multiple-bids-per-name.md +++ b/docs/specs/2026-09-20-multiple-bids-per-name.md @@ -174,15 +174,51 @@ false. `names::tests::ownership_actions_stay_available_once_registered`, `sign_name_message_tests::rejects_a_name_whose_owner_coin_is_still_a_reveal`. -**R11b — The two transfer capabilities answer for the transfer.** Update is -refused while a transfer is pending: hsd lets a TRANSFER coin go to UPDATE, -RENEW, FINALIZE or REVOKE, and that UPDATE branch *is* the cancel, so a button -labelled "edit your DNS records" was a way to lose a transfer in flight. -Cancel transfer requires a transfer to cancel — the condition `can_finalize` -has always carried — instead of building a no-op UPDATE that costs a fee. +**R11b — Every capability answers for a transfer in flight.** hsd lets a +TRANSFER coin go to UPDATE, RENEW, FINALIZE or REVOKE, and consensus decides +which of those a button may offer. + +- **Update** is refused: its UPDATE branch *is* the cancel, so a button + labelled "edit your DNS records" was a way to lose a transfer in flight. +- **Renew** is refused for the same reason. hsd's RENEW handler runs + `ns.setTransfer(0)` exactly as UPDATE does, so "extend my registration" + ended a transfer and said nothing about transfers. +- **Transfer** is refused: a TRANSFER coin may become an UPDATE, RENEW, + FINALIZE or REVOKE and nothing else, so a second one is a transaction the + node rejects. +- **Revoke** stays offered. Consensus allows it from a TRANSFER coin, and + destroying the name is exactly what that button says it does. +- **Cancel transfer** requires a transfer to cancel — the condition + `can_finalize` has always carried — instead of building a no-op UPDATE that + costs a fee. + *Enforced:* `commands/names.rs::build_name_action_capabilities`. *Pinned:* `names::tests::update_is_refused_while_a_transfer_is_pending`, -`names::tests::cancel_transfer_is_refused_when_no_transfer_is_pending`. +`names::tests::cancel_transfer_is_refused_when_no_transfer_is_pending`, +`names::tests::renew_is_refused_while_a_transfer_is_pending`, +`names::tests::transfer_is_refused_while_a_transfer_is_already_pending`. + +**R11c — Finalize waits out the transfer lockup.** hsd refuses a FINALIZE +until `transfer + transferLockup` blocks have passed (`bad-finalize-maturity`), +so offering it the moment a transfer is mined sends the user at a transaction +the node throws away — two days on mainnet, ten blocks on regtest. The gate +compares the transfer's height against the tip and the reason counts the +blocks left rather than only saying no. It prefers the live tip, fetched once +per call, and falls back to the persisted estimate when no synced node answers; +with either height unknown the action stays offered, because refusing one the +node would accept is its own kind of wrong. +*Enforced:* `commands/names.rs::build_name_action_capabilities`, +`commands/names.rs::evaluate_name_action_capabilities` (live tip). +*Pinned:* `names::tests::finalize_waits_out_the_transfer_lockup`. + +**R11d — A batched owner spend is still an owner spend.** The draft a batch +builder persists records itself as `batch-`, so an owner-spend check +matching only the singular action names read a batched transfer as no transfer +at all — collapsing ownership the moment one went out. The match strips a +`batch-` prefix, which leaves `batch-bid` and `batch-redeem` correctly saying +nothing about ownership. +*Enforced:* `commands/names.rs::find_name_action_context`. +*Pinned:* `names_action_context_tests::find_name_action_context_recognises_a_batched_owner_spend`. **R12 — A name whose auction lapsed can be opened again.** Only an *unconfirmed* OPEN coin counts as a pending OPEN. The OPEN output is a @@ -196,15 +232,54 @@ pending" made every name this wallet had ever opened permanently un-openable. ### Saying what is happening **R13 — An action sent but not yet mined says so, everywhere it appears.** The -name modal, the auctions list and the guided panel all read the same -`pendingBroadcastAction` and render "waiting for a block" naming the action, -rather than the unchanged phase. +name modal, the auctions list, the guided panel and the Owned Names table's +State column all read the same `pendingBroadcastAction` and render "waiting for +a block" naming the action, rather than the unchanged phase. The State column +was the last surface that did not, so a row read "Owned" while the modal it +opened read "Redeem · waiting for a block". *Enforced:* `db/queries.rs::pending_broadcast_action_for_name`, -`src/lib/auction.ts` (`pendingBroadcastText`, `pendingBroadcastBadge`). +`src/lib/auction.ts` (`pendingBroadcastText`, `pendingBroadcastBadge`), +`src/components/WalletView.tsx` (State column). *Pinned:* `auction.test.ts :: names the action that is waiting for a block`, +`wallet-view.test.tsx :: defers to what is in flight, like the modal it opens`, `auction-positions.test.tsx :: a broadcasted open the node/explorer hasn't caught up to (waitingForBidding) shows Waiting for Bidding / View`, `name-acquisition.test.tsx :: says the OPEN is waiting to be mined, with no Open button`. +**R13b — A pending transfer is a task, not a phase that never arrives.** hsd +has six name states — OPENING, LOCKED, BIDDING, REVEAL, CLOSED, REVOKED — and +TRANSFER is not among them: a transfer leaves the state at CLOSED and shows +itself through `info.transfer`. A task derivation keyed on a `"TRANSFER"` phase +string therefore never fired, and every name being transferred fell through to +"no urgent action" — on a name whose one remaining action is to finalize it. +The task reads the transfer the node actually reports, and ranks behind the +renewal alarm but ahead of everything quiet: losing the name outranks +completing a transfer of it, and nothing else does. +*Enforced:* `commands/names.rs::derive_auction_task_state`. +*Pinned:* `auction_capabilities_tests::a_recorded_transfer_yields_transfer_pending_finalize`. + +**R13c — A reclaimed lockup is money again.** REDEEM is classified as +spendable, not name-bound. hsd draws the same line in +`Covenant::isNonspendable()`, which returns false for NONE, OPEN and REDEEM: a +redeemed coin spends like any other output. Without this, HNS reclaimed from +losing bids landed in `name_control`, so the balance card did not show it as +spendable and coin selection would not draw on it — the user had just paid a +fee to get it back and it stayed invisible. OPEN stays name-bound on purpose: +hsd would spend it too, but it is a zero-value marker, so counting it as liquid +would add an input and no value. No migration is needed; the sync upsert +rewrites `spend_class` on conflict. +*Enforced:* `noncustodial/sync.rs` (spend-class match). +*Pinned:* `sync::tests::redeemed_value_counts_as_liquid_balance`. + +**R13d — A confirm dialog counts every output the action carries.** A name +action can carry several: revealing a name bid on more than once emits one +REVEAL per bid, and redeeming reclaims one per losing reveal. Reporting +`outputs[0]` offered a live wallet a redeem of three reveals worth 28 HNS with +12 on the dialog — the one figure a user checks before signing, wrong on every +multi-bid action. The total sums every output except change, which the plan +already identifies by index. +*Enforced:* `commands/names.rs` (`send_total_doos` in the draft summary). +*Pinned:* `build_redeem_draft_tests::build_redeem_draft_totals_every_output_it_reclaims`. + **R14 — Our own unmined bid appears in the bids panel, counted apart.** A bid we broadcast but the chain has not indexed is appended to the list as `pending: true`, so placing a second bid does not look like it vanished. Only @@ -306,12 +381,6 @@ capability fields `nameIsRegistered` and `transferPending`. ## 5. Known gaps -- **A multi-reveal draft reports only its primary output in the send total.** - `sendTotalDoos` on a reveal covering several bids shows the first output's - value rather than the sum, so the confirm dialog understates the amount - moved (the fee and the transaction itself are correct). Accepted for now: - it is a display figure on a self-spend, and every output returns to the - wallet. - **`DataTable` still carries an `onRowClick` prop with no caller.** Kept because the guard in R18 is the thing worth keeping, and the next table that wants a row click should get the guarded version. diff --git a/docs/specs/2026-09-21-paid-name-swaps.md b/docs/specs/2026-09-21-paid-name-swaps.md index 69d230cc..c526bb94 100644 --- a/docs/specs/2026-09-21-paid-name-swaps.md +++ b/docs/specs/2026-09-21-paid-name-swaps.md @@ -86,3 +86,7 @@ and broadcasts. There is no separate "finalize with payment" command for them. - `src-tauri/src/noncustodial/actions.rs` — the `sighash::ALL` of W2. - `name-modal-sections.test.tsx :: offers no way to start a paid swap` — pins the withdrawal. +- User-facing copy that described the withdrawn flows, corrected to match: + `docs/USER_MANUAL.md` ("Paid name swaps", and the owned-name action table), + `README.md` (feature list), `docs/QA_WORKFLOWS.md` (section J). Any future + attempt has to update these three in the same PR as the code. diff --git a/docs/specs/README.md b/docs/specs/README.md index 44e41fcf..c459cf05 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -18,8 +18,9 @@ Each spec has these sections, in this order: | Spec | Status | |------|--------| -| [2026-09-11 Remote-node connection & broadcast guard](./2026-09-11-remote-node-connection-and-broadcast-guard.md) | Implemented on `feat/spv-broadcast-guard-and-remote-node-onboarding` | -| [2026-09-14 Network-derived behaviour](./2026-09-14-network-derived-behaviour.md) | Implemented on `feat/batch-transfer` | +| [2026-09-11 Remote-node connection & broadcast guard](./2026-09-11-remote-node-connection-and-broadcast-guard.md) | Implemented | +| [2026-09-14 Network-derived behaviour](./2026-09-14-network-derived-behaviour.md) | Implemented | +| [2026-09-15 Batch name operations](./2026-09-15-batch-name-operations.md) | Implemented; spec written after the fact | | [2026-09-15 Per-profile node banner & preflight](./2026-09-15-per-profile-node-banner-and-preflight.md) | Steps 1-2 of 9 implemented — see the spec's own status table | -| [2026-09-20 Multiple bids per name](./2026-09-20-multiple-bids-per-name.md) | Implemented on `feat/batch-reveal-redeem-finalize-ui` | +| [2026-09-20 Multiple bids per name](./2026-09-20-multiple-bids-per-name.md) | Implemented | | [2026-09-21 Paid name swaps](./2026-09-21-paid-name-swaps.md) | Not implemented — UI withdrawn, see spec | From 145d10e34c0082ebb659e4f790e4f9bef51d6214 Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 24 Sep 2026 18:21:20 +0200 Subject: [PATCH 12/26] refactor: one home per shared thing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layering first. CODING_STANDARDS names `commands::read` a legacy shared layer and says not to add to it; #53 and #55 added three node-readiness helpers, and `chain_scan`, `sync`, `names`, `deadlines` and the watched-name daemon all reach into it for them. They now live in `commands/node_readiness.rs` — the "small dedicated helper module under commands/" the standard points at, of which `active_profile.rs` is its own example. `estimate_persisted_height` moved with them: it answers what to believe about the chain height when no node does, which is the same question with the node taken away, and it was the only other reason `names.rs` imported the legacy layer. Nothing about the functions changed. What remains of `commands::read` in sibling commands is `resolve_profile` and two pre-existing imports. Then three things written down more than once. `node_config::DEFAULT_NODE_RPC_URL` spelled out mainnet's loopback port beside `Network::default_rpc_url`, which builds the same string from the per-network port table. It now calls it, and its doc says why mainnet is the only defensible default in a resolver that deliberately does not consult the profile's network. "Allow sending via remote node" was hand-rolled in both Onboarding and Settings, down to a shared `data-testid` that would match twice if a page rendered both. The two differed only in text size and whether they showed the explanatory line, so those are props on one `AllowRemoteBroadcastToggle`. This is a safety opt-in whose real enforcement is in `broadcast_tx_draft`; the least the two screens can do is ask the same question. The "no explorer available" message existed in four wordings across the read and sync paths, including two I introduced earlier on this branch. It is one constant now, next to the factory that decides there is no explorer, covering both reasons it refuses. Gates: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (2586 passed), npx tsc -b, npx vite build, npx vitest run (884 passed), npm run lint:format, lint:secure-imports, lint:native-title. --- src-tauri/src/commands/chain_scan.rs | 4 +- src-tauri/src/commands/deadlines.rs | 2 +- src-tauri/src/commands/mod.rs | 1 + src-tauri/src/commands/names.rs | 11 +- src-tauri/src/commands/node_readiness.rs | 279 +++++++++++++++++ src-tauri/src/commands/read.rs | 294 ++---------------- src-tauri/src/commands/sync.rs | 20 +- src-tauri/src/daemon/watched_names.rs | 9 +- src-tauri/src/noncustodial/node_config.rs | 15 +- src-tauri/src/providers/mod.rs | 13 + src-tauri/src/tests/node_config_tests.rs | 6 +- .../src/tests/node_rpc_injected_tests.rs | 4 +- src-tauri/src/tests/node_status_tests.rs | 10 +- src-tauri/src/tests/read_cmd_tests.rs | 20 +- src/components/Onboarding.tsx | 15 +- src/components/Settings.tsx | 22 +- .../ui/AllowRemoteBroadcastToggle.tsx | 50 +++ 17 files changed, 429 insertions(+), 346 deletions(-) create mode 100644 src-tauri/src/commands/node_readiness.rs create mode 100644 src/components/ui/AllowRemoteBroadcastToggle.tsx diff --git a/src-tauri/src/commands/chain_scan.rs b/src-tauri/src/commands/chain_scan.rs index 84babc7f..1b1d7940 100644 --- a/src-tauri/src/commands/chain_scan.rs +++ b/src-tauri/src/commands/chain_scan.rs @@ -110,7 +110,7 @@ pub async fn run_chain_scanner(db_path: String) { // Use per-profile probe if active profile exists; otherwise fall back to global. let tip = if let Some(profile_id) = active_profile_id.as_deref() { - match crate::commands::read::node_tip_height_if_synced_from_profile_with_network( + match crate::commands::node_readiness::node_tip_height_if_synced_from_profile_with_network( &db_path, profile_id, expected_network.as_deref(), @@ -124,7 +124,7 @@ pub async fn run_chain_scanner(db_path: String) { } } } else { - match crate::commands::read::node_tip_height_if_synced_from_settings_with_network( + match crate::commands::node_readiness::node_tip_height_if_synced_from_settings_with_network( &settings, expected_network.as_deref(), ) diff --git a/src-tauri/src/commands/deadlines.rs b/src-tauri/src/commands/deadlines.rs index 3f312acf..2b361e63 100644 --- a/src-tauri/src/commands/deadlines.rs +++ b/src-tauri/src/commands/deadlines.rs @@ -396,7 +396,7 @@ pub async fn scan_deadline_notifications( // Probe the node BEFORE taking the DB lock (guard is !Send across await, // same discipline as `read_renewals`). - let live_height = crate::commands::read::node_tip_height_if_synced(&state).await; + let live_height = crate::commands::node_readiness::node_tip_height_if_synced(&state).await; let (previously_notified, reveal, renewal) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index b65a7430..ff0b3f05 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -16,6 +16,7 @@ pub mod namebase_history; pub mod names; pub mod names_pure; pub mod node; +pub mod node_readiness; pub mod paid_swaps; pub mod read; pub mod read_pure; diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index def6bd3a..c38d4e3f 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -728,7 +728,8 @@ pub(crate) fn find_name_action_context( // The same call is already propagated further down this file; a failure // here read as "height unknown", which quietly widens every gate that // compares a height against the tip. - let current_height = crate::commands::read::estimate_persisted_height(conn, profile_id)?; + let current_height = + crate::commands::node_readiness::estimate_persisted_height(conn, profile_id)?; Ok(NameActionContext { has_bid_commitment: bid.is_some(), @@ -789,7 +790,7 @@ pub async fn get_name_action_capabilities( Some(id) => id, None => return Ok(conservative_capabilities(&name, "no active wallet profile")), }; - let live_tip = crate::commands::read::node_tip_height_if_synced(&state).await; + let live_tip = crate::commands::node_readiness::node_tip_height_if_synced(&state).await; evaluate_name_action_capabilities(&state, name, &profile_id, live_tip).await } @@ -835,7 +836,7 @@ pub async fn get_names_action_capabilities( // Fetched once for the whole batch, not once per name: the only thing it // is needed for is the transfer-lockup countdown, and a stale tip there // refuses a FINALIZE the node would accept. - let live_tip = crate::commands::read::node_tip_height_if_synced(&state).await; + let live_tip = crate::commands::node_readiness::node_tip_height_if_synced(&state).await; let mut out = Vec::with_capacity(names.len()); for name in names { out.push(evaluate_name_action_capabilities(&state, name, &profile_id, live_tip).await?); @@ -874,7 +875,7 @@ async fn evaluate_name_action_capabilities( // spuriously false for names we actually own. Treat "reachable but not // synced" exactly like unreachable: fall back to local Sync evidence. // Reuses the same gate as `read_balance`/`read_names` — no duplicate logic. - let name_info = if crate::commands::read::is_node_ready_for_local_reads(state).await { + let name_info = if crate::commands::node_readiness::is_node_ready_for_local_reads(state).await { client.get_name_info(&name).await.ok() } else { None @@ -956,7 +957,7 @@ async fn evaluate_name_action_capabilities( // duplicated — both read the same helpers. let renewal_window = network.name_params().renewal_window as i64; let current_height = - crate::commands::read::estimate_persisted_height(&conn, profile_id)?; + crate::commands::node_readiness::estimate_persisted_height(&conn, profile_id)?; (tracked, action_ctx, addrs, renewal_window, current_height) }; let tracked = match tracked { diff --git a/src-tauri/src/commands/node_readiness.rs b/src-tauri/src/commands/node_readiness.rs new file mode 100644 index 00000000..0aca8153 --- /dev/null +++ b/src-tauri/src/commands/node_readiness.rs @@ -0,0 +1,279 @@ +//! Whether the node is good enough to be believed, and how high the chain is. +//! +//! One question with several callers and several amounts of context: a Tauri +//! `State`, a settings map, a profile id and a DB path, or an injected client. +//! They all end at [`node_tip_height_if_synced_with_client`], so "synced" means +//! the same thing to the read gate, the background sync, the chain scanner and +//! the watched-name daemon — and a node on the wrong chain is refused by all of +//! them rather than by whichever happened to check. +//! +//! [`estimate_persisted_height`] is the other half: what to believe about the +//! chain height when no node answers. It lives here because it is the fallback +//! of the same question, and because the callers that need one usually need +//! both. +//! +//! This is a dedicated helper module rather than more surface on +//! `commands::read`, which CODING_STANDARDS names as a legacy shared layer not +//! to add to. + +use tauri::State; + +use crate::db::queries; +use rusqlite::OptionalExtension; + +use crate::error::AppError; +use crate::AppState; + +/// Check if the local hsd node is connected AND fully synced, making local +/// cached data the preferred read source. Returns `true` when the node RPC +/// answers and the chain is caught up (height ≥ headers, or progress ≥ 0.9999). +/// +/// In SPV mode, always returns `false` — SPV nodes don't have `--index-address` +/// and can't serve UTXO queries, so all reads must go through the explorer. +pub(crate) async fn is_node_ready_for_local_reads(state: &State<'_, AppState>) -> bool { + // SPV mode: node is never authoritative for reads. + let (node_mode, expected_network) = { + let db = match state.db.lock() { + Ok(db) => db, + Err(_) => return false, + }; + let settings = match crate::db::queries::get_settings(&db) { + Ok(s) => s, + Err(_) => return false, + }; + let mode = crate::noncustodial::rpc::resolve_node_mode(&settings); + // Resolve the active profile's network so we can reject a node on a + // different chain (e.g. regtest node vs mainnet wallet). + // A DB failure here degrades to "no network to compare" — the read + // gate is a routing decision, not a security boundary, and the SPV / + // sync gates below still apply. A caller that must not proceed on an + // unknown network propagates the error instead of degrading. + let net = queries::get_active_profile_network(&db).ok().flatten(); + (mode, net) + }; + if node_mode.is_spv() { + return false; + } + node_tip_height_if_synced_for_network(state, expected_network.as_deref()) + .await + .is_some() +} + +/// Like [`node_tip_height_if_synced`] but with an explicitly supplied +/// `expected_network`, for the one caller that has already resolved it +/// ([`is_node_ready_for_local_reads`]). Rejects the node when its reported +/// `chain` disagrees (e.g. a regtest node answering for a mainnet wallet). +/// `None` means "no network to compare" — see +/// [`node_tip_height_if_synced_with_client`] for why that is permissive. +pub(crate) async fn node_tip_height_if_synced_for_network( + state: &State<'_, AppState>, + expected_network: Option<&str>, +) -> Option { + let settings = { + let db = state.db.lock().ok()?; + crate::db::queries::get_settings(&db).ok()? + }; + node_tip_height_if_synced_from_settings_with_network(&settings, expected_network).await +} + +/// The live node tip height, but ONLY when the node is connected, fully synced, +/// AND reporting the same chain as the active profile. `None` when the node is +/// unreachable, catching up, or on another network. +/// +/// The expected network is resolved here rather than taken as an argument: +/// every `State`-based caller wants the active profile's chain, and a helper +/// that could be called without one is exactly how the cross-chain reads this +/// guard exists to prevent got in. Callers outside a `State` context use +/// [`node_tip_height_if_synced_from_settings_with_network`], which makes the +/// expected network an explicit argument they cannot forget. +pub(crate) async fn node_tip_height_if_synced(state: &State<'_, AppState>) -> Option { + let expected_network = { + let db = state.db.lock().ok()?; + queries::get_active_profile_network(&db).ok().flatten() + }; + node_tip_height_if_synced_for_network(state, expected_network.as_deref()).await +} + +/// Resolve the node's tip height from a settings map, returning `None` unless +/// the node is reachable and fully synced — and additionally rejecting when +/// its reported `chain` disagrees with `expected_network`. Set `expected_network` to the active profile's stored +/// network string — the schema allows only `"mainnet"`, `"testnet"` and +/// `"regtest"`; `"main"` and `"simnet"` are accepted defensively by the +/// comparison. Leave it `None` to skip the network check. +/// +/// This is the guard that prevents a regtest node from being treated as +/// authoritative for a mainnet wallet (or any other cross-network mismatch). +/// The comparison normalizes both sides through +/// [`crate::noncustodial::network::network_name_matches`] so +/// `"mainnet"` (profile) and `"main"` (hsd) count as equal. +pub(crate) async fn node_tip_height_if_synced_from_settings_with_network( + settings: &std::collections::HashMap, + expected_network: Option<&str>, +) -> Option { + let client = crate::noncustodial::rpc::NodeRpcClient::from_settings(settings); + node_tip_height_if_synced_with_client(&client, expected_network).await +} + +/// Per-profile readiness probe: the live node tip height, but ONLY when the node +/// is connected, fully synced, AND reporting the same chain as the profile. +/// Returns None when the node is unreachable, catching up, on another network, +/// or the profile doesn't exist. +/// +/// Per-profile node override routing (ADR-001): if the profile has a per-profile +/// override, it takes precedence; otherwise falls back to global settings; otherwise +/// uses the built-in default. This is the readiness probe for background daemons +/// (chain scanner, watched-name daemon) that operate on behalf of a specific profile. +pub(crate) async fn node_tip_height_if_synced_from_profile_with_network( + db_path: &str, + profile_id: &str, + expected_network: Option<&str>, +) -> Option { + let conn = match crate::db::connection::open(std::path::Path::new(db_path)) { + Ok(c) => c, + Err(_) => return None, + }; + // ADR-001: a profile whose node config will not resolve is a configuration + // error for that profile, not a fallback to global. This gate answers + // "is this profile's node authoritative?", and global's node is not an + // answer to that question — it may be another chain entirely. + let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, profile_id).ok()?; + node_tip_height_if_synced_with_client(&client, expected_network).await +} + +/// The client-injected core of [`node_tip_height_if_synced_from_settings_with_network`]. +/// All the sync-progress + network-match logic lives here so it can be unit +/// tested against a `MockNodeRpc` without a live node. The settings-based +/// wrappers construct the real `NodeRpcClient` and delegate here. +pub(crate) async fn node_tip_height_if_synced_with_client( + client: &dyn crate::noncustodial::node_rpc::NodeRpc, + expected_network: Option<&str>, +) -> Option { + let info = client.get_blockchain_info().await.ok()?; + // Reject the node only on a POSITIVE mismatch. `network_check` returns + // `None` when either side is unknown (no profile, or a node that doesn't + // report `chain` — older builds); we conservatively allow that, and the + // SPV gate and other checks still apply. + if crate::noncustodial::network::network_check(expected_network, info.chain.as_deref()) + == Some(false) + { + return None; + } + // Connected — now check if synced. No sync metadata at all (e.g. regtest + // with a single miner) counts as synced. + info.is_synced(/* assume_when_unknown */ true) + .then_some(info.blocks) +} + +/// Settings-based readiness gate: `true` when the local node is connected, fully +/// synced, AND reporting `expected_network`. Mirrors +/// [`is_node_ready_for_local_reads`] for callers that only have settings/a DB +/// connection (the background sync thread, the chain scanner, the watched-name +/// daemon). Pass the active profile's stored network string; `None` skips the +/// comparison and should only be used where no profile exists. +pub async fn node_ready_from_settings( + settings: &std::collections::HashMap, + expected_network: Option<&str>, +) -> bool { + node_tip_height_if_synced_from_settings_with_network(settings, expected_network) + .await + .is_some() +} + +/// Per-profile readiness gate: true when the node is connected, fully synced, +/// AND reporting the profile's network. Mirrors node_ready_from_settings for +/// callers that have a profile ID and a DB path (background daemons). +/// Returns false when the profile doesn't exist or the node is unreachable. +pub async fn node_ready_from_profile( + db_path: &str, + profile_id: &str, + expected_network: Option<&str>, +) -> bool { + node_tip_height_if_synced_from_profile_with_network(db_path, profile_id, expected_network) + .await + .is_some() +} + +/// Best persisted estimate of the current chain height when no synced node is +/// available, extrapolated to "now" by elapsed wall time (~10-minute blocks). +/// Extrapolation matters for safety: a stale snapshot UNDERestimates the +/// height and therefore INFLATES days-until-expiry — the dangerous direction. +/// +/// Candidates (max wins): +/// * per-name explorer/node stats snapshots persisted in +/// `tracked_name_states.raw_json` — `renewalPeriodEnd - blocksUntilExpire` +/// is the chain height the stats were computed at, aged by `updated_at`; +/// * `wallet_profiles.last_synced_height`, aged by `last_synced_at`. +pub(crate) fn estimate_persisted_height( + conn: &rusqlite::Connection, + profile_id: &str, +) -> Result, AppError> { + // Ageing a stored height by wall clock assumes blocks arrive on a schedule. + // They do on main and testnet; on regtest and simnet they are mined on + // demand, so the same arithmetic invents six blocks an idle hour never + // produced and every renewal countdown drifts. There, report the stored + // height as-is: stale but true. + let ages_by_wall_clock = queries::get_wallet_profile(conn, profile_id)? + .and_then(|p| crate::noncustodial::network::Network::from_str_opt(&p.network)) + .unwrap_or_default() + .has_wall_clock_block_timing(); + let age = |elapsed: i64| { + if ages_by_wall_clock { + elapsed.max(0) + } else { + 0 + } + }; + + let mut best: Option = None; + let mut consider = |h: Option| { + if let Some(h) = h { + best = Some(best.map_or(h, |b| b.max(h))); + } + }; + + // Per-name stats snapshots. raw_json is either the explorer HsdName shape + // (stats at the root) or the node getnameinfo result ({"info": {...}}). + let mut stmt = conn.prepare( + "SELECT raw_json, + CAST((strftime('%s','now') - strftime('%s', updated_at)) / 600 AS INTEGER) + FROM tracked_name_states + WHERE wallet_profile_id = ?1 AND raw_json IS NOT NULL", + )?; + let rows = stmt.query_map(rusqlite::params![profile_id], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) + })?; + for row in rows { + let (raw, elapsed_blocks) = row?; + let Ok(v) = serde_json::from_str::(&raw) else { + continue; + }; + let info = match v.get("info") { + Some(i) if !i.is_null() => i, + _ => &v, + }; + let Some(stats) = info.get("stats").filter(|s| !s.is_null()) else { + continue; + }; + let end = stats.get("renewalPeriodEnd").and_then(|x| x.as_i64()); + let until = stats.get("blocksUntilExpire").and_then(|x| x.as_i64()); + if let (Some(end), Some(until)) = (end, until) { + consider(Some(end - until + age(elapsed_blocks))); + } + } + + // Last node-synced height (stale, but still a floor), aged the same way. + let profile_snapshot: Option<(Option, i64)> = conn + .query_row( + "SELECT last_synced_height, + CAST((strftime('%s','now') - strftime('%s', COALESCE(last_synced_at, datetime('now')))) / 600 AS INTEGER) + FROM wallet_profiles WHERE id = ?1", + rusqlite::params![profile_id], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .optional()?; + if let Some((Some(h), elapsed_blocks)) = profile_snapshot { + consider(Some(h + age(elapsed_blocks))); + } + + Ok(best) +} diff --git a/src-tauri/src/commands/read.rs b/src-tauri/src/commands/read.rs index 5100838b..4b277d95 100644 --- a/src-tauri/src/commands/read.rs +++ b/src-tauri/src/commands/read.rs @@ -25,7 +25,6 @@ use crate::db::queries; use crate::error::AppError; use crate::providers::hnsfans::HnsFansClient; use crate::AppState; -use rusqlite::OptionalExtension; use serde::Serialize; use std::collections::HashSet; use std::time::Duration; @@ -74,146 +73,6 @@ pub(crate) fn resolve_profile( active_profile(state) } -/// Check if the local hsd node is connected AND fully synced, making local -/// cached data the preferred read source. Returns `true` when the node RPC -/// answers and the chain is caught up (height ≥ headers, or progress ≥ 0.9999). -/// -/// In SPV mode, always returns `false` — SPV nodes don't have `--index-address` -/// and can't serve UTXO queries, so all reads must go through the explorer. -pub(crate) async fn is_node_ready_for_local_reads(state: &State<'_, AppState>) -> bool { - // SPV mode: node is never authoritative for reads. - let (node_mode, expected_network) = { - let db = match state.db.lock() { - Ok(db) => db, - Err(_) => return false, - }; - let settings = match crate::db::queries::get_settings(&db) { - Ok(s) => s, - Err(_) => return false, - }; - let mode = crate::noncustodial::rpc::resolve_node_mode(&settings); - // Resolve the active profile's network so we can reject a node on a - // different chain (e.g. regtest node vs mainnet wallet). - // A DB failure here degrades to "no network to compare" — the read - // gate is a routing decision, not a security boundary, and the SPV / - // sync gates below still apply. A caller that must not proceed on an - // unknown network propagates the error instead of degrading. - let net = queries::get_active_profile_network(&db).ok().flatten(); - (mode, net) - }; - if node_mode.is_spv() { - return false; - } - node_tip_height_if_synced_for_network(state, expected_network.as_deref()) - .await - .is_some() -} - -/// Like [`node_tip_height_if_synced`] but with an explicitly supplied -/// `expected_network`, for the one caller that has already resolved it -/// ([`is_node_ready_for_local_reads`]). Rejects the node when its reported -/// `chain` disagrees (e.g. a regtest node answering for a mainnet wallet). -/// `None` means "no network to compare" — see -/// [`node_tip_height_if_synced_with_client`] for why that is permissive. -pub(crate) async fn node_tip_height_if_synced_for_network( - state: &State<'_, AppState>, - expected_network: Option<&str>, -) -> Option { - let settings = { - let db = state.db.lock().ok()?; - crate::db::queries::get_settings(&db).ok()? - }; - node_tip_height_if_synced_from_settings_with_network(&settings, expected_network).await -} - -/// The live node tip height, but ONLY when the node is connected, fully synced, -/// AND reporting the same chain as the active profile. `None` when the node is -/// unreachable, catching up, or on another network. -/// -/// The expected network is resolved here rather than taken as an argument: -/// every `State`-based caller wants the active profile's chain, and a helper -/// that could be called without one is exactly how the cross-chain reads this -/// guard exists to prevent got in. Callers outside a `State` context use -/// [`node_tip_height_if_synced_from_settings_with_network`], which makes the -/// expected network an explicit argument they cannot forget. -pub(crate) async fn node_tip_height_if_synced(state: &State<'_, AppState>) -> Option { - let expected_network = { - let db = state.db.lock().ok()?; - queries::get_active_profile_network(&db).ok().flatten() - }; - node_tip_height_if_synced_for_network(state, expected_network.as_deref()).await -} - -/// Resolve the node's tip height from a settings map, returning `None` unless -/// the node is reachable and fully synced — and additionally rejecting when -/// its reported `chain` disagrees with `expected_network`. Set `expected_network` to the active profile's stored -/// network string — the schema allows only `"mainnet"`, `"testnet"` and -/// `"regtest"`; `"main"` and `"simnet"` are accepted defensively by the -/// comparison. Leave it `None` to skip the network check. -/// -/// This is the guard that prevents a regtest node from being treated as -/// authoritative for a mainnet wallet (or any other cross-network mismatch). -/// The comparison normalizes both sides through -/// [`crate::noncustodial::network::network_name_matches`] so -/// `"mainnet"` (profile) and `"main"` (hsd) count as equal. -pub(crate) async fn node_tip_height_if_synced_from_settings_with_network( - settings: &std::collections::HashMap, - expected_network: Option<&str>, -) -> Option { - let client = crate::noncustodial::rpc::NodeRpcClient::from_settings(settings); - node_tip_height_if_synced_with_client(&client, expected_network).await -} - -/// Per-profile readiness probe: the live node tip height, but ONLY when the node -/// is connected, fully synced, AND reporting the same chain as the profile. -/// Returns None when the node is unreachable, catching up, on another network, -/// or the profile doesn't exist. -/// -/// Per-profile node override routing (ADR-001): if the profile has a per-profile -/// override, it takes precedence; otherwise falls back to global settings; otherwise -/// uses the built-in default. This is the readiness probe for background daemons -/// (chain scanner, watched-name daemon) that operate on behalf of a specific profile. -pub(crate) async fn node_tip_height_if_synced_from_profile_with_network( - db_path: &str, - profile_id: &str, - expected_network: Option<&str>, -) -> Option { - let conn = match crate::db::connection::open(std::path::Path::new(db_path)) { - Ok(c) => c, - Err(_) => return None, - }; - // ADR-001: a profile whose node config will not resolve is a configuration - // error for that profile, not a fallback to global. This gate answers - // "is this profile's node authoritative?", and global's node is not an - // answer to that question — it may be another chain entirely. - let client = crate::noncustodial::rpc::NodeRpcClient::for_profile(&conn, profile_id).ok()?; - node_tip_height_if_synced_with_client(&client, expected_network).await -} - -/// The client-injected core of [`node_tip_height_if_synced_from_settings_with_network`]. -/// All the sync-progress + network-match logic lives here so it can be unit -/// tested against a `MockNodeRpc` without a live node. The settings-based -/// wrappers construct the real `NodeRpcClient` and delegate here. -pub(crate) async fn node_tip_height_if_synced_with_client( - client: &dyn crate::noncustodial::node_rpc::NodeRpc, - expected_network: Option<&str>, -) -> Option { - let info = client.get_blockchain_info().await.ok()?; - // Reject the node only on a POSITIVE mismatch. `network_check` returns - // `None` when either side is unknown (no profile, or a node that doesn't - // report `chain` — older builds); we conservatively allow that, and the - // SPV gate and other checks still apply. - if crate::noncustodial::network::network_check(expected_network, info.chain.as_deref()) - == Some(false) - { - return None; - } - // Connected — now check if synced. No sync metadata at all (e.g. regtest - // with a single miner) counts as synced. - info.is_synced(/* assume_when_unknown */ true) - .then_some(info.blocks) -} - /// Client-injected RPC phase of owned-name discovery. Resolves each /// `WalletNameHash` → name via `getnamebyhash` (falling back to the coin's /// `raw_name_hex`), then fetches `getnameinfo` for each resolved name. @@ -326,35 +185,6 @@ pub(crate) async fn resolve_name_ownership_with_client( }) } -/// Settings-based readiness gate: `true` when the local node is connected, fully -/// synced, AND reporting `expected_network`. Mirrors -/// [`is_node_ready_for_local_reads`] for callers that only have settings/a DB -/// connection (the background sync thread, the chain scanner, the watched-name -/// daemon). Pass the active profile's stored network string; `None` skips the -/// comparison and should only be used where no profile exists. -pub async fn node_ready_from_settings( - settings: &std::collections::HashMap, - expected_network: Option<&str>, -) -> bool { - node_tip_height_if_synced_from_settings_with_network(settings, expected_network) - .await - .is_some() -} - -/// Per-profile readiness gate: true when the node is connected, fully synced, -/// AND reporting the profile's network. Mirrors node_ready_from_settings for -/// callers that have a profile ID and a DB path (background daemons). -/// Returns false when the profile doesn't exist or the node is unreachable. -pub async fn node_ready_from_profile( - db_path: &str, - profile_id: &str, - expected_network: Option<&str>, -) -> bool { - node_tip_height_if_synced_from_profile_with_network(db_path, profile_id, expected_network) - .await - .is_some() -} - /// HNSFans explorer client from settings + the active profile's network. /// Thin wrapper kept for call-site brevity — the actual construction and the /// network gate live in [`crate::providers::explorer_client_from_settings`] @@ -373,12 +203,7 @@ fn explorer_client( /// unavailable for the active profile's network (G2). Concrete and actionable: /// it names the missing setting rather than degrading to empty/mainnet data. fn explorer_unavailable_error() -> AppError { - AppError::Other( - "No explorer is available for this network. The node is not synced \ - and no 'explorer_api_url' is configured — set one in Settings, or \ - wait for the local node to finish syncing." - .to_string(), - ) + AppError::Other(crate::providers::EXPLORER_UNAVAILABLE.to_string()) } /// Node-only owned-name discovery for [`discover_owned_names`]. Resolves the @@ -525,7 +350,7 @@ pub async fn read_balance( }; // Prefer local cache when the node is connected and synced. - if is_node_ready_for_local_reads(&state).await { + if crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; return queries::read_cached_balance(&conn, &id, profile_network(&conn, &id)?); } @@ -646,7 +471,7 @@ pub async fn read_names( }; // Check node readiness BEFORE acquiring the DB lock so we don't hold // MutexGuard across the async RPC probe (MutexGuard is !Send). - let local_ready = is_node_ready_for_local_reads(&state).await; + let local_ready = crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await; let out = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; @@ -743,7 +568,7 @@ pub async fn discover_owned_names( // us the nameHash, resolved to a name via `getnamebyhash` (or the paired // OPEN/BID/FINALIZE covenant's rawName). This is the "post-sync workaround // retired" path described in the Feature 3 plan. - if is_node_ready_for_local_reads(&state).await { + if crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { return discover_owned_names_via_node(&state, &id).await; } @@ -899,7 +724,7 @@ pub async fn read_name_info( // `getnameinfo` returns `{ info: { name, state, stats:{…phase…} } }` // (or null `info` for a name that has never been touched on-chain). if let Some(node) = node_opt.as_ref() { - if is_node_ready_for_local_reads(&state).await { + if crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { if let Some(shaped) = read_name_info_node_with_client(node, &name).await { return Ok(serde_json::to_value(&shaped)?); } @@ -1181,7 +1006,7 @@ pub async fn read_name_bids( // indexed past the name's auction height, serve bids from the local // `name_bid_outpoints` table — no HNSFans call. Fall through to the // explorer when the scanner hasn't caught up yet. - if is_node_ready_for_local_reads(&state).await { + if crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { let name_hash_hex = hex::encode(crate::noncustodial::names::hash_name(&name)?); let (indexed_bids, commitments, scanner_height, name_height) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; @@ -1312,7 +1137,8 @@ pub async fn get_resource( }; // Records come from the node only; without a profile there's no node client // and thus no records — treat that like "node not ready". - let node_ready = node_opt.is_some() && is_node_ready_for_local_reads(&state).await; + let node_ready = node_opt.is_some() + && crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await; // Name info: try node first, then explorer. let info: serde_json::Value = if let (true, Some(node)) = (node_ready, node_opt.as_ref()) { @@ -1388,7 +1214,7 @@ pub async fn read_name_records( Some(id) => id, None => return Ok(empty_resource()), }; - if is_node_ready_for_local_reads(&state).await { + if crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { // Build the node client from the resolved profile's *effective* config // (per-profile override -> global -> default; ADR-001) under a short // lock, then drop the guard BEFORE the async RPC call — the same @@ -1458,7 +1284,7 @@ pub async fn read_block_info( ) -> Result { // Node-only: without a synced local node there's nothing to read. Soft- // degrade to null so the modal can show a "requires synced node" hint. - if height < 0 || !is_node_ready_for_local_reads(&state).await { + if height < 0 || !crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await { return Ok(serde_json::Value::Null); } @@ -1538,7 +1364,9 @@ pub async fn read_tx_info( txid: String, ) -> Result { let txid = txid.trim().to_string(); - if txid.is_empty() || !is_node_ready_for_local_reads(&state).await { + if txid.is_empty() + || !crate::commands::node_readiness::is_node_ready_for_local_reads(&state).await + { return Ok(serde_json::Value::Null); } @@ -1858,91 +1686,6 @@ fn empty_renewals() -> RenewalsResponse { } } -/// Best persisted estimate of the current chain height when no synced node is -/// available, extrapolated to "now" by elapsed wall time (~10-minute blocks). -/// Extrapolation matters for safety: a stale snapshot UNDERestimates the -/// height and therefore INFLATES days-until-expiry — the dangerous direction. -/// -/// Candidates (max wins): -/// * per-name explorer/node stats snapshots persisted in -/// `tracked_name_states.raw_json` — `renewalPeriodEnd - blocksUntilExpire` -/// is the chain height the stats were computed at, aged by `updated_at`; -/// * `wallet_profiles.last_synced_height`, aged by `last_synced_at`. -pub(crate) fn estimate_persisted_height( - conn: &rusqlite::Connection, - profile_id: &str, -) -> Result, AppError> { - // Ageing a stored height by wall clock assumes blocks arrive on a schedule. - // They do on main and testnet; on regtest and simnet they are mined on - // demand, so the same arithmetic invents six blocks an idle hour never - // produced and every renewal countdown drifts. There, report the stored - // height as-is: stale but true. - let ages_by_wall_clock = queries::get_wallet_profile(conn, profile_id)? - .and_then(|p| crate::noncustodial::network::Network::from_str_opt(&p.network)) - .unwrap_or_default() - .has_wall_clock_block_timing(); - let age = |elapsed: i64| { - if ages_by_wall_clock { - elapsed.max(0) - } else { - 0 - } - }; - - let mut best: Option = None; - let mut consider = |h: Option| { - if let Some(h) = h { - best = Some(best.map_or(h, |b| b.max(h))); - } - }; - - // Per-name stats snapshots. raw_json is either the explorer HsdName shape - // (stats at the root) or the node getnameinfo result ({"info": {...}}). - let mut stmt = conn.prepare( - "SELECT raw_json, - CAST((strftime('%s','now') - strftime('%s', updated_at)) / 600 AS INTEGER) - FROM tracked_name_states - WHERE wallet_profile_id = ?1 AND raw_json IS NOT NULL", - )?; - let rows = stmt.query_map(rusqlite::params![profile_id], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) - })?; - for row in rows { - let (raw, elapsed_blocks) = row?; - let Ok(v) = serde_json::from_str::(&raw) else { - continue; - }; - let info = match v.get("info") { - Some(i) if !i.is_null() => i, - _ => &v, - }; - let Some(stats) = info.get("stats").filter(|s| !s.is_null()) else { - continue; - }; - let end = stats.get("renewalPeriodEnd").and_then(|x| x.as_i64()); - let until = stats.get("blocksUntilExpire").and_then(|x| x.as_i64()); - if let (Some(end), Some(until)) = (end, until) { - consider(Some(end - until + age(elapsed_blocks))); - } - } - - // Last node-synced height (stale, but still a floor), aged the same way. - let profile_snapshot: Option<(Option, i64)> = conn - .query_row( - "SELECT last_synced_height, - CAST((strftime('%s','now') - strftime('%s', COALESCE(last_synced_at, datetime('now')))) / 600 AS INTEGER) - FROM wallet_profiles WHERE id = ?1", - rusqlite::params![profile_id], - |row| Ok((row.get(0)?, row.get(1)?)), - ) - .optional()?; - if let Some((Some(h), elapsed_blocks)) = profile_snapshot { - consider(Some(h + age(elapsed_blocks))); - } - - Ok(best) -} - /// Pure DB + math core of `read_renewals` (testable without Tauri state). /// /// `live_node_height` is the tip of a connected, fully synced node when one is @@ -1976,7 +1719,8 @@ pub(crate) fn compute_renewals( let (current_height, height_source) = match live_node_height { Some(h) => (Some(h), "node"), - None => match estimate_persisted_height(conn, profile_id)? { + None => match crate::commands::node_readiness::estimate_persisted_height(conn, profile_id)? + { Some(h) => (Some(h), "explorer"), None => (None, "unknown"), }, @@ -2119,7 +1863,7 @@ pub async fn read_renewals( None => return Ok(empty_renewals()), }; // Probe the node BEFORE taking the DB lock (the guard is !Send). - let live_height = node_tip_height_if_synced(&state).await; + let live_height = crate::commands::node_readiness::node_tip_height_if_synced(&state).await; let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; compute_renewals(&conn, &id, live_height) } @@ -2170,7 +1914,7 @@ pub async fn repair_owned_names(state: State<'_, AppState>) -> Result) -> Result e, None => { let mut s = status.lock().await; - s.errors.push( - "no explorer for this profile — its network could not be read, or the network has no explorer configured; set explorer_api_url or wait for the local node to sync" - .to_string(), - ); + s.errors + .push(crate::providers::EXPLORER_UNAVAILABLE.to_string()); return; } }; @@ -1116,10 +1118,8 @@ pub async fn discover_step(status: &Arc>, db_path: &str, profi Some(e) => e, None => { let mut s = status.lock().await; - s.errors.push( - "no explorer for this profile — its network could not be read, or the network has no explorer configured; set explorer_api_url or wait for the local node to sync" - .to_string(), - ); + s.errors + .push(crate::providers::EXPLORER_UNAVAILABLE.to_string()); return; } }; diff --git a/src-tauri/src/daemon/watched_names.rs b/src-tauri/src/daemon/watched_names.rs index c13910b1..37a2e473 100644 --- a/src-tauri/src/daemon/watched_names.rs +++ b/src-tauri/src/daemon/watched_names.rs @@ -519,15 +519,18 @@ async fn try_run_watched_scan(db_path: &str) -> Result<(), AppError> { // `node_ready_from_profile` resolves the effective config identically. // Only the no-active-profile fallback uses global settings. let node_ready = if let Some(profile_id) = active_profile_id.as_deref() { - crate::commands::read::node_ready_from_profile( + crate::commands::node_readiness::node_ready_from_profile( db_path, profile_id, expected_network.as_deref(), ) .await } else { - crate::commands::read::node_ready_from_settings(&settings, expected_network.as_deref()) - .await + crate::commands::node_readiness::node_ready_from_settings( + &settings, + expected_network.as_deref(), + ) + .await }; // 3. Adaptive skip + fetch. Bounded concurrency (4) to avoid hammering hsd. diff --git a/src-tauri/src/noncustodial/node_config.rs b/src-tauri/src/noncustodial/node_config.rs index 7746319f..cd5e69d0 100644 --- a/src-tauri/src/noncustodial/node_config.rs +++ b/src-tauri/src/noncustodial/node_config.rs @@ -16,9 +16,16 @@ use crate::error::AppError; use crate::noncustodial::rpc::{resolve_node_api_key, ChainSource}; /// The built-in default node RPC URL when neither an override nor a global -/// setting supplies one. Mainnet's default hsd node port; realign fixes the -/// port for other networks when this comes from the global fallback. -pub const DEFAULT_NODE_RPC_URL: &str = "http://127.0.0.1:12037"; +/// setting supplies one: mainnet's default hsd port, from +/// [`Network::default_rpc_url`] rather than spelled out again here. Realign +/// fixes the port for other networks when this came from the global fallback. +/// +/// Resolution has no profile network to consult — it resolves the endpoint +/// tuple and deliberately does not touch the profile's network — so mainnet is +/// the only defensible default, and realign is what corrects it. +pub fn default_node_rpc_url() -> String { + crate::noncustodial::network::Network::Main.default_rpc_url() +} /// The `profile_settings` keys that make up the ADR-001 node-config tuple. /// A profile may hold other per-profile settings; those say nothing about @@ -89,7 +96,7 @@ pub fn effective_node_config_for_profile( let node_rpc_url = merged .get("node_rpc_url") .map(String::to_string) - .unwrap_or_else(|| DEFAULT_NODE_RPC_URL.to_string()); + .unwrap_or_else(default_node_rpc_url); let node_rpc_api_key = resolve_node_api_key(&merged); let chain_source = ChainSource::from_settings(&merged); diff --git a/src-tauri/src/providers/mod.rs b/src-tauri/src/providers/mod.rs index 298e25b9..30620ca6 100644 --- a/src-tauri/src/providers/mod.rs +++ b/src-tauri/src/providers/mod.rs @@ -23,6 +23,19 @@ pub use signer::{ WriteCapability, }; +/// What to tell the user when [`explorer_client_from_settings`] returns `None` +/// and the caller needed it. +/// +/// One sentence, in one place, because it was written four different ways +/// across the read and sync paths — and it has to cover both reasons the +/// factory refuses: the profile's network has no explorer configured, or the +/// network could not be read at all. Either way the actionable part is the +/// same, and naming the setting beats degrading to empty or to mainnet data. +pub const EXPLORER_UNAVAILABLE: &str = + "No explorer is available for this wallet's network, and the local node is \ + not synced. Set 'explorer_api_url' in Settings, or wait for the node to \ + finish syncing."; + /// The ONE place settings turn into an explorer client (Task 11 / S1, G2). /// /// Before this, `HnsFansClient::new(...)` was constructed at three separate diff --git a/src-tauri/src/tests/node_config_tests.rs b/src-tauri/src/tests/node_config_tests.rs index 3a43c5d7..18fa8ebe 100644 --- a/src-tauri/src/tests/node_config_tests.rs +++ b/src-tauri/src/tests/node_config_tests.rs @@ -14,7 +14,7 @@ use rusqlite::Connection; use crate::db::queries::get_profile_settings; use crate::error::AppError; -use crate::noncustodial::node_config::{effective_node_config_for_profile, DEFAULT_NODE_RPC_URL}; +use crate::noncustodial::node_config::{default_node_rpc_url, effective_node_config_for_profile}; use crate::noncustodial::rpc::ChainSource; /// A migrated in-memory DB with the migrations the resolver depends on, plus @@ -94,7 +94,7 @@ fn missing_profile_is_not_found() { fn falls_back_to_builtin_default_when_nothing_set() { let conn = db_with_profile(); let cfg = effective_node_config_for_profile(&conn, "p1").unwrap(); - assert_eq!(cfg.node_rpc_url, DEFAULT_NODE_RPC_URL); + assert_eq!(cfg.node_rpc_url, default_node_rpc_url()); assert_eq!(cfg.node_rpc_api_key, ""); assert_eq!(cfg.chain_source, ChainSource::LocalNode); assert!(!cfg.from_override, "pure default must not be an override"); @@ -220,7 +220,7 @@ fn for_profile_falls_through_to_global_then_default() { // No override, no global url -> built-in default; global key present. set_global(&conn, "node_rpc_api_key", "global-key"); let client = NodeRpcClient::for_profile(&conn, "p1").unwrap(); - assert_eq!(client.node_url(), DEFAULT_NODE_RPC_URL); + assert_eq!(client.node_url(), default_node_rpc_url()); assert_eq!(client.api_key(), "global-key"); } diff --git a/src-tauri/src/tests/node_rpc_injected_tests.rs b/src-tauri/src/tests/node_rpc_injected_tests.rs index 761fae46..2cf4e88a 100644 --- a/src-tauri/src/tests/node_rpc_injected_tests.rs +++ b/src-tauri/src/tests/node_rpc_injected_tests.rs @@ -15,9 +15,9 @@ use serde_json::json; +use crate::commands::node_readiness::node_tip_height_if_synced_with_client; use crate::commands::read::{ - node_tip_height_if_synced_with_client, read_block_info_with_client, - read_name_records_with_client, read_tx_info_with_client, + read_block_info_with_client, read_name_records_with_client, read_tx_info_with_client, }; use crate::error::AppError; use crate::noncustodial::node_rpc::NodeRpc; diff --git a/src-tauri/src/tests/node_status_tests.rs b/src-tauri/src/tests/node_status_tests.rs index 4118a62e..07382d18 100644 --- a/src-tauri/src/tests/node_status_tests.rs +++ b/src-tauri/src/tests/node_status_tests.rs @@ -62,7 +62,7 @@ async fn node_status_reports_disconnected_when_no_node() { // --- is_node_ready_for_local_reads ------------------------------------------- -use crate::commands::read::is_node_ready_for_local_reads; +use crate::commands::node_readiness::is_node_ready_for_local_reads; #[tokio::test] async fn local_reads_not_ready_when_not_connected() { @@ -76,7 +76,7 @@ async fn local_reads_not_ready_when_not_connected() { // --- node_ready_from_settings (the settings-based gate used by the background // sync thread, which has no State) -------------------------------- -use crate::commands::read::node_ready_from_settings; +use crate::commands::node_readiness::node_ready_from_settings; /// Build a settings map pointing the node RPC at a mockito server URL. fn settings_for_url(url: &str) -> std::collections::HashMap { @@ -501,7 +501,7 @@ async fn probe_and_update_sets_flag_true_when_node_answers() { // --- node_tip_height_if_synced_from_settings_with_network --------------------- -use crate::commands::read::node_tip_height_if_synced_from_settings_with_network; +use crate::commands::node_readiness::node_tip_height_if_synced_from_settings_with_network; #[tokio::test] async fn synced_with_matching_network_returns_height() { @@ -619,7 +619,7 @@ async fn synced_with_no_chain_in_response_skips_check() { // Per-profile node config resolution for readiness probe (ADR-001) // =========================================================================== -use crate::commands::read::node_tip_height_if_synced_from_profile_with_network; +use crate::commands::node_readiness::node_tip_height_if_synced_from_profile_with_network; /// Helper: create a temp file-backed DB (in-memory won't work because the /// async probe re-opens the connection from a path — see the Send bound @@ -783,7 +783,7 @@ async fn probe_respects_network_mismatch_with_profile_override() { // Per-profile readiness gate (boolean wrapper) // =========================================================================== -use crate::commands::read::node_ready_from_profile; +use crate::commands::node_readiness::node_ready_from_profile; #[tokio::test] async fn node_ready_from_profile_returns_true_when_synced_and_network_matches() { diff --git a/src-tauri/src/tests/read_cmd_tests.rs b/src-tauri/src/tests/read_cmd_tests.rs index b756408f..0b135938 100644 --- a/src-tauri/src/tests/read_cmd_tests.rs +++ b/src-tauri/src/tests/read_cmd_tests.rs @@ -10,6 +10,7 @@ use rusqlite::params; use tauri::test::{mock_builder, mock_context, noop_assets}; use tauri::Manager; +use crate::commands::node_readiness::is_node_ready_for_local_reads; use crate::commands::read::{ discover_owned_names, empty_name_bids_response, merge_name_bids, read_auction_position_names, read_balance, read_name_bids, read_name_info, read_name_records, read_names, read_transactions, @@ -23,9 +24,8 @@ use crate::AppState; // block so it's easy to see the read-tests baseline vs. the new coverage // harness in a single file. use crate::commands::read::{ - get_resource, is_node_ready_for_local_reads, list_receive_addresses, merge_indexed_bids, - read_block_info, read_renewals, read_tx_info, repair_owned_names, resolve_profile, - reveal_next_receive_address, + get_resource, list_receive_addresses, merge_indexed_bids, read_block_info, read_renewals, + read_tx_info, repair_owned_names, resolve_profile, reveal_next_receive_address, }; // --------------------------------------------------------------------------- @@ -2072,7 +2072,7 @@ async fn repair_owned_names_via_node_records_error_on_rpc_failure() { fn estimate_persisted_height_returns_none_when_no_signal() { let conn = empty_db(); add_profile(&conn, "H1", "regtest"); - let h = crate::commands::read::estimate_persisted_height(&conn, "H1").unwrap(); + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "H1").unwrap(); assert!(h.is_none()); } @@ -2088,7 +2088,7 @@ fn estimate_persisted_height_reads_from_profile_last_synced_height() { [], ) .unwrap(); - let h = crate::commands::read::estimate_persisted_height(&conn, "H2") + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "H2") .unwrap() .unwrap(); // Value may be aged slightly (>=12345); the important thing is it was read. @@ -2123,7 +2123,7 @@ fn estimate_persisted_height_prefers_max_across_sources() { [], ) .unwrap(); - let h = crate::commands::read::estimate_persisted_height(&conn, "H3") + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "H3") .unwrap() .unwrap(); // Max of (25000 - 5000) and 15000 is 20000 (plus small aging drift). @@ -2654,7 +2654,7 @@ fn estimate_persisted_height_reads_node_shaped_raw_json_info_field() { rusqlite::params![raw], ) .unwrap(); - let h = crate::commands::read::estimate_persisted_height(&conn, "EN1") + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "EN1") .unwrap() .unwrap(); // 50000 - 10000 == 40000, plus small aging drift (rows aged in blocks). @@ -2703,7 +2703,7 @@ fn estimate_persisted_height_skips_malformed_and_stats_less_rows() { ) .unwrap(); // With no valid signals AND no last_synced_height set, the result is None. - let h = crate::commands::read::estimate_persisted_height(&conn, "EN2").unwrap(); + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "EN2").unwrap(); assert!(h.is_none(), "all rows should be skipped; got {h:?}"); } @@ -2738,7 +2738,7 @@ fn estimate_persisted_height_picks_max_across_multiple_rows() { ) .unwrap(); } - let h = crate::commands::read::estimate_persisted_height(&conn, "EN3") + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "EN3") .unwrap() .unwrap(); // Max implied height is 40000 (row 2). Aging drift only adds — never subtracts. @@ -2761,7 +2761,7 @@ fn estimate_persisted_height_ignores_null_last_synced_height() { [], ) .unwrap(); - let h = crate::commands::read::estimate_persisted_height(&conn, "EN4").unwrap(); + let h = crate::commands::node_readiness::estimate_persisted_height(&conn, "EN4").unwrap(); assert!(h.is_none()); } diff --git a/src/components/Onboarding.tsx b/src/components/Onboarding.tsx index a89256e0..e5cc4912 100644 --- a/src/components/Onboarding.tsx +++ b/src/components/Onboarding.tsx @@ -6,6 +6,7 @@ import { Button } from "./ui/Button"; import { useNodeConnectionCheck } from "../hooks/useNodeConnectionCheck"; import { fromConnectionMode } from "../lib/connectionMode"; import { boolToSetting } from "../lib/settingsBool"; +import { AllowRemoteBroadcastToggle } from "./ui/AllowRemoteBroadcastToggle"; import { RemoteNodeFields } from "./ui/RemoteNodeFields"; import type { WalletNetwork } from "../types"; @@ -164,15 +165,11 @@ function ConnectionChoice({ apiKeyTestId="remote-api-key-input" actionsLayout="stack" /> - +