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:
parent
f917e5ecfd
commit
c09670530c
2 changed files with 27 additions and 31 deletions
|
|
@ -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> {
|
||||
|
|
|
|||
54
src/vault.rs
54
src/vault.rs
|
|
@ -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.
|
||||
//
|
||||
// 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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue