diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs index 4221df8..dcab6de 100644 --- a/src/signer/nip46_client.rs +++ b/src/signer/nip46_client.rs @@ -1158,7 +1158,7 @@ impl Signer for Nip46ClientSigner { // The identity learned from the signer during the handshake — NOT // the URI key, which for bunker:// flows is a per-connection comms // key. Until the handshake resolves it the session is not usable. - inner.identity.clone().ok_or(SigningError::NotConnected) + inner.identity.ok_or(SigningError::NotConnected) } async fn sign_event(&self, event: UnsignedEvent) -> Result { diff --git a/src/vault.rs b/src/vault.rs index 24d0702..d46ce04 100644 --- a/src/vault.rs +++ b/src/vault.rs @@ -356,35 +356,28 @@ pub fn parse_vault(content: &str) -> Result { /// left untouched. Only profiles with the legacy `None` value (or /// missing the field entirely) are assigned `Embedded`. /// -/// Returns `true` if any profiles were migrated (i.e. the vault should -/// be re-saved). +/// Missing `signer_mode` fields are assigned `Embedded` by the serde +/// default during deserialization, and every vault at the current +/// `VAULT_VERSION` serialises `signer_mode` explicitly — so once the +/// version bump below has been saved, re-saving adds nothing. A change +/// is therefore reported only when the version actually moves, instead +/// of on every load (the old unconditional `changed = true` made +/// `App::load` rewrite the vault on each start). +/// +/// Returns `true` if the vault was migrated (i.e. it should be re-saved). pub fn migrate_vault_signer_modes(vault: &mut Vault) -> bool { - let mut changed = false; - for _profile in &mut vault.profiles { - // The serde default already handles missing fields during - // deserialization, but once loaded, profiles that were stored - // before signer_mode was introduced will have the default value. - // We write it explicitly so the on-disk format is canonical. - // - // After the first save, every profile will have an explicit - // signer_mode and this becomes a no-op. - // - // We cannot distinguish "user explicitly set Embedded" from - // "serde defaulted to Embedded", so we always write it — this is - // safe because Embedded is the correct default and the write is - // idempotent. - changed = true; - } - // Also ensure the nip46_connections vector exists (serde default - // handles this during deserialization, but we normalise here too). - if vault.version < VAULT_VERSION { - vault.version = VAULT_VERSION; - changed = true; - } + // Profiles need no per-field work: the serde default already filled + // any missing `signer_mode` at parse time and serialization at the + // current version writes it explicitly. + // // Legacy connections without profile_npub (None) are left as-is. // Ownership cannot be reliably inferred from active_profile, so these // connections remain unusable until the user re-creates them. - changed + if vault.version < VAULT_VERSION { + vault.version = VAULT_VERSION; + return true; + } + false } /// Store (or replace) a NIP-46 connection secret in the vault, keyed by an @@ -852,11 +845,14 @@ mod tests { }); let changed1 = migrate_vault_signer_modes(&mut vault); - assert!(changed1, "first migration should report change"); + // Vault::empty() is already at the current version with an + // explicit signer_mode, so migration must report no change — + // the old always-true return made App::load rewrite the vault + // on every start. + assert!(!changed1, "current-version vault should not report change"); - let _changed2 = migrate_vault_signer_modes(&mut vault); - // The function always returns true because it normalises the version. - // The important thing is that running it twice doesn't corrupt data. + let changed2 = migrate_vault_signer_modes(&mut vault); + assert!(!changed2, "re-running migration stays a no-op"); assert_eq!(vault.profiles[0].signer_mode, SignerMode::Embedded); assert_eq!(vault.version, VAULT_VERSION); }