Skip to content

fix: clear additional cartridge keys on disconnect #96

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tarrencev merged 2 commits into from
Feb 13, 2026
Merged

fix: clear additional cartridge keys on disconnect #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dab411
fix: clear additional cartridge storage keys on disconnect
tarrencev Feb 13, 2026
9aa52e0
fix: clear all storage on controller disconnect
tarrencev Feb 13, 2026
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
25 changes: 3 additions & 22 deletions account_sdk/src/storage/localstorage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use web_sys::window;
#[derive(Debug, Clone, Default)]
pub struct LocalStorage;

const CARTRIDGE_STORAGE_PREFIX: &str = "@cartridge/";

#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
#[cfg_attr(target_arch = "wasm32", async_trait)]
impl StorageBackend for LocalStorage {
Expand Down Expand Up @@ -44,27 +42,10 @@ impl StorageBackend for LocalStorage {
}

fn clear(&mut self) -> Result<(), StorageError> {
// Never clear the entire origin localStorage. Only clear Cartridge's namespace.
let local_storage = Self::local_storage()?;
let length = local_storage
.length()
.map_err(|_| StorageError::OperationFailed("getting localStorage length".into()))?;

// Collect keys up front; mutating localStorage while iterating by index is error-prone.
let mut keys = Vec::new();
for i in 0..length {
if let Ok(Some(key)) = local_storage.key(i) {
keys.push(key);
}
}

for key in keys {
if key.starts_with(CARTRIDGE_STORAGE_PREFIX) {
local_storage.remove_item(&key).map_err(|_| {
StorageError::OperationFailed("removing item from localStorage".into())
})?;
}
}
local_storage
.clear()
.map_err(|_| StorageError::OperationFailed("clearing localStorage".into()))?;
Ok(())
}

Expand Down
92 changes: 4 additions & 88 deletions account_sdk/src/storage/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,95 +362,11 @@ pub type Storage = localstorage::LocalStorage;
#[cfg(all(not(target_arch = "wasm32"), feature = "filestorage"))]
pub type Storage = filestorage::FileSystemBackend;

#[derive(Debug, Clone, Serialize, Deserialize)]
struct StoredMultiChainMetadata {
username: String,
chains: Vec<StoredChainInfo>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
struct StoredChainInfo {
chain_id: Felt,
address: Felt,
}

/// Clears only the storage entries associated with a specific address.
///
/// This is intentionally scoped to the given `address` so multiple controllers (addresses)
/// can coexist in the same storage backend, while disconnect clears all chains for the address.
/// Clears all persisted storage entries for the backend implementation.
/// This is used by disconnect and intentionally performs a full clear in the active storage context.
pub(crate) fn clear_controller_storage(
storage: &mut impl StorageBackend,
address: &Felt,
_address: &Felt,
) -> Result<(), StorageError> {
let mut first_err: Option<StorageError> = None;

let mut record = |res: Result<(), StorageError>| {
if let Err(e) = res {
if first_err.is_none() {
first_err = Some(e);
}
}
};

// Remove all chain-scoped keys for this address by prefix (across all chain_ids).
match storage.keys() {
Ok(keys) => {
let account_prefix = format!("@cartridge/account/0x{address:x}/");
let session_prefix = format!("@cartridge/session/0x{address:x}/");
let deployment_prefix = format!("@cartridge/deployment/0x{address:x}/");
let admin_prefix = format!("@cartridge/admin/0x{address:x}/");

for key in keys {
if key.starts_with(&account_prefix)
|| key.starts_with(&session_prefix)
|| key.starts_with(&deployment_prefix)
|| key.starts_with(&admin_prefix)
{
record(storage.remove(&key));
}
}
}
Err(e) => record(Err(e)),
}

// Remove "active" only if it points to this address.
match storage.get(&selectors::Selectors::active()) {
Ok(Some(StorageValue::Active(active))) if &active.address == address => {
record(storage.remove(&selectors::Selectors::active()));
}
Ok(_) => {}
Err(e) => record(Err(e)),
}

// If a multi-chain config exists, remove just this address from it.
// This keeps other controllers intact and avoids noisy load warnings.
let config_key = selectors::Selectors::multi_chain_config();
match storage.get(&config_key) {
Ok(Some(StorageValue::String(config_json))) => {
if let Ok(mut config) = serde_json::from_str::<StoredMultiChainMetadata>(&config_json) {
let before = config.chains.len();
config.chains.retain(|c| c.address != *address);

if config.chains.len() != before {
if config.chains.is_empty() {
record(storage.remove(&config_key));
} else {
match serde_json::to_string(&config) {
Ok(updated_json) => record(
storage.set(&config_key, &StorageValue::String(updated_json)),
),
Err(e) => record(Err(StorageError::Serialization(e))),
}
}
}
}
}
Ok(_) => {}
Err(e) => record(Err(e)),
}

if let Some(e) = first_err {
return Err(e);
}
Ok(())
storage.clear()
}
104 changes: 33 additions & 71 deletions account_sdk/src/storage/storage_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mod tests {
}

#[test]
fn test_clear_controller_storage_is_scoped_and_updates_multi_chain_config() {
fn test_clear_controller_storage_removes_everything() {
let mut storage = InMemoryBackend::new();

let address_a = felt!("0x111");
Expand Down Expand Up @@ -77,6 +77,30 @@ mod tests {
&StorageValue::String("account_a2".to_string()),
)
.unwrap();
storage
.set(
"@cartridge/policies/0x111/0x1",
&StorageValue::String("policies_a_chain_a".to_string()),
)
.unwrap();
storage
.set(
"@cartridge/policies/0x222/0x3",
&StorageValue::String("policies_b_chain_b".to_string()),
)
.unwrap();
storage
.set(
"@cartridge/features",
&StorageValue::String("features".to_string()),
)
.unwrap();
storage
.set(
"@cartridge/https://x.cartridge.gg/active",
&StorageValue::String("active-domain".to_string()),
)
.unwrap();
storage
.set(
&Selectors::session(&address_a, &chain_a2),
Expand Down Expand Up @@ -148,63 +172,17 @@ mod tests {

clear_controller_storage(&mut storage, &address_a).unwrap();

// A's entries are removed across all chains.
assert!(storage
.get(&Selectors::account(&address_a, &chain_a))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::session(&address_a, &chain_a))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::deployment(&address_a, &chain_a))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::account(&address_a, &chain_a2))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::session(&address_a, &chain_a2))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::deployment(&address_a, &chain_a2))
.unwrap()
.is_none());

// B's entries remain.
assert!(storage
.get(&Selectors::account(&address_b, &chain_b))
.unwrap()
.is_some());
assert!(storage
.get(&Selectors::session(&address_b, &chain_b))
.unwrap()
.is_some());
assert!(storage
.get(&Selectors::deployment(&address_b, &chain_b))
.unwrap()
.is_some());

// Active is removed because it pointed at A.
assert!(storage.get(&Selectors::active()).unwrap().is_none());
// All stored data is removed.
assert_eq!(storage.keys().unwrap().len(), 0);

// Multi-chain config no longer includes A.
match storage.get(&Selectors::multi_chain_config()).unwrap() {
Some(StorageValue::String(cfg_json)) => {
let cfg: MultiChainMetadata = serde_json::from_str(&cfg_json).unwrap();
assert_eq!(cfg.chains.len(), 1);
assert_eq!(cfg.chains[0].address, address_b);
assert_eq!(cfg.chains[0].chain_id, chain_b);
}
other => panic!("unexpected multi-chain config storage value: {other:?}"),
match storage.get(&Selectors::active()).unwrap() {
None => {}
other => panic!("unexpected active storage value: {other:?}"),
}
}

#[test]
fn test_clear_controller_storage_does_not_remove_other_active_controller() {
fn test_clear_controller_storage_does_clear_everything_even_for_other_controller() {
let mut storage = InMemoryBackend::new();

let address_a = felt!("0x111");
Expand Down Expand Up @@ -232,7 +210,7 @@ mod tests {
)
.unwrap();

// Active points at B, so clearing A should not change it.
// Active points at B, but full clear removes all keys anyway.
storage
.set(
&Selectors::active(),
Expand All @@ -245,22 +223,6 @@ mod tests {

clear_controller_storage(&mut storage, &address_a).unwrap();

// A's entries are removed across all chains.
assert!(storage
.get(&Selectors::account(&address_a, &chain_a))
.unwrap()
.is_none());
assert!(storage
.get(&Selectors::account(&address_a, &chain_a2))
.unwrap()
.is_none());

match storage.get(&Selectors::active()).unwrap() {
Some(StorageValue::Active(active)) => {
assert_eq!(active.address, address_b);
assert_eq!(active.chain_id, chain_b);
}
other => panic!("unexpected active storage value: {other:?}"),
}
assert_eq!(storage.keys().unwrap().len(), 0);
}
}
Loading