Skip to content
Closed
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 86 additions & 30 deletions walletkit-core/src/storage/credential_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ impl CredentialStore {
/// not exist.
pub fn delete_credential(&self, credential_id: u64) -> StorageResult<()> {
self.lock_inner()?.delete_credential(credential_id)?;
self.notify_vault_changed();
Ok(())
self.notify_vault_changed()
}

/// Stores a credential and optional associated data.
Expand All @@ -214,7 +213,7 @@ impl CredentialStore {
associated_data,
now,
)?;
self.notify_vault_changed();
self.notify_vault_changed()?;
Ok(id)
}

Expand Down Expand Up @@ -284,15 +283,15 @@ impl CredentialStore {
let count = inner.danger_delete_all_credentials()?;
drop(inner);
if count > 0 {
self.notify_vault_changed();
self.notify_vault_changed()?;
}
Ok(count)
}

/// Registers a backup manager that will be notified after vault mutations
/// ([`store_credential`](Self::store_credential),
/// [`delete_credential`](Self::delete_credential),
/// [`danger_delete_all_credentials`](Self::danger_delete_all_credentials)).
/// Backup failures are logged but do not affect the mutation result.
///
/// # Errors
///
Expand All @@ -306,6 +305,21 @@ impl CredentialStore {
})? = manager;
Ok(())
}

/// Triggers a one-off backup sync without mutating the vault.
///
/// Call this at initial backup creation to catch up any credentials
/// that were stored before the backup was set up. The registered
/// [`WalletKitBackupManager`] receives the same `on_vault_changed`
/// callback as after a normal vault mutation.
///
/// # Errors
///
/// Returns an error if no backup manager is configured, if the vault
/// export fails, or if the backup manager callback fails.
pub fn sync_backup(&self) -> StorageResult<()> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this is creating a cyclical pattern of Foreign -> Rust -> Foreign (and potentially more bindings after). Is this initial sync necessary? If there are no credentials there should be nothing to backup right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear - it's the same case with regular credential store calls - they're also Foregin -> Rust -> Foregin

Copy link
Copy Markdown
Contributor Author

@thomas-waite thomas-waite Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, the existing backup solution in Oxide is already doing this. I'm following the same pattern. The initial sync is necessary to backup whatever v4 credentials have already been put into the credential vault when the user enables backups (the callback based backup logic is only triggered when the vault is mutated)

self.notify_vault_changed()
}
}

/// Implementation not exposed to foreign bindings
Expand All @@ -318,40 +332,30 @@ impl CredentialStore {
.map_err(|_| StorageError::Lock("storage mutex poisoned".to_string()))
}

/// Best-effort export + notification to the backup manager, if one is set.
/// Exports a plaintext vault snapshot and notifies the registered backup
/// manager via [`on_vault_changed`](WalletKitBackupManager::on_vault_changed).
///
/// Called after any vault mutation (store, delete) so the host app can
/// sync the updated vault to its backup. Failures are logged but never
/// propagated — the vault mutation has already succeeded and callers
/// should not see an error from a backup side-effect.
fn notify_vault_changed(&self) {
/// Called after vault mutations and by [`sync_backup`](Self::sync_backup).
/// Returns `Ok(())` if no backup manager is configured (noop).
fn notify_vault_changed(&self) -> StorageResult<()> {
// Hold the backup lock for the entire export+callback path. This
// serializes concurrent notifications so backups are delivered in
// mutation order. Recover the guard on poison — the manager is
// still valid after a prior panic.
let guard = self
.backup
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
// mutation order.
let guard = self.backup.lock().map_err(|_| {
StorageError::Lock("backup config mutex poisoned".to_string())
})?;

let dest_dir = guard.dest_dir();
if dest_dir.is_empty() {
return; // NoopBackupManager — nothing to do.
return Ok(()); // NoopBackupManager — nothing to do.
}

// Export a plaintext snapshot of the vault. The file is sensitive
// (unencrypted), so we wrap it in a guard that deletes it on drop —
// no matter how we exit (normal return, early return, or panic).
let vault_path = match self
let vault_path = self
.lock_inner()
.and_then(|inner| inner.export_vault_for_backup(&dest_dir))
{
Ok(path) => path,
Err(e) => {
tracing::error!("Failed to export vault for backup: {e}");
return;
}
};
.and_then(|inner| inner.export_vault_for_backup(&dest_dir))?;

let _cleanup = {
struct CleanupFile(String);
Expand All @@ -371,9 +375,7 @@ impl CredentialStore {
// Hand the path to the host app (e.g. iOS) so it can copy/upload
// the vault to Bedrock. The host must finish with the file during
// this synchronous call — the guard deletes it on return.
if let Err(e) = guard.on_vault_changed(vault_path) {
tracing::error!("Backup manager on_vault_changed failed: {e}");
}
guard.on_vault_changed(vault_path)
}

/// Retrieves a full credential including raw bytes by issuer schema ID.
Expand Down Expand Up @@ -1508,4 +1510,58 @@ mod tests {
cleanup_test_storage(&root);
cleanup_test_storage(&export_dir);
}

#[test]
fn test_sync_backup_triggers_notification_without_mutation() {
use world_id_core::Credential as CoreCredential;

let (store, manager, root, export_dir) = setup_store_with_backup();

// Store a credential so the vault is non-empty.
let cred: Credential = CoreCredential::new()
.issuer_schema_id(100)
.genesis_issued_at(1000)
.into();
store
.store_credential(&cred, &FieldElement::from(7u64), 9999, None, 1000)
.expect("store credential");
assert_eq!(manager.call_count(), 1);

// sync_backup triggers a notification without mutating the vault.
store.sync_backup().expect("sync_backup");
assert_eq!(manager.call_count(), 2);
assert!(
manager.last_file_existed(),
"exported vault file should exist during the callback"
);
let path = manager.last_path().unwrap();
assert!(
!std::path::Path::new(&path).exists(),
"exported vault file should be cleaned up after the callback"
);

// Vault contents are unchanged — still the same credential.
let credentials = store.list_credentials(None, 1000).expect("list");
assert_eq!(credentials.len(), 1);
assert_eq!(credentials[0].issuer_schema_id, 100);

cleanup_test_storage(&root);
cleanup_test_storage(&export_dir);
}

#[test]
fn test_sync_backup_on_empty_vault() {
let (store, manager, root, export_dir) = setup_store_with_backup();

// sync_backup on an empty vault should still trigger a notification.
store.sync_backup().expect("sync_backup");
assert_eq!(manager.call_count(), 1);
assert!(
manager.last_file_existed(),
"exported vault file should exist during the callback"
);

cleanup_test_storage(&root);
cleanup_test_storage(&export_dir);
}
}
Loading