fix(vault): stop rewriting the vault on every load; clippy cleanup

- migrate_vault_signer_modes now reports a change only when the vault
  version actually moves. The unconditional 'changed = true' made
  App::load re-save the vault on every start (harmless, idempotent,
  but wasteful). Per-profile signer_mode normalisation was already a
  no-op: the serde default fills missing fields at parse time and the
  current version serialises it explicitly.
- idempotency test tightened to assert changed == false for a
  current-version vault (previously ducked the question).
- nip46_client.rs: drop clone-on-Copy in get_public_key (clippy).
This commit is contained in:
Avi 2026-09-11 15:11:00 -05:00
commit c09670530c
2 changed files with 27 additions and 31 deletions

View file

@ -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<Event, SigningError> {

View file

@ -356,35 +356,28 @@ pub fn parse_vault(content: &str) -> Result<Vault, AppError> {
/// 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.
// 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.
//
// 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;
}
// 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);
}