Skip to content

Fix localStorage quota error in asset metadata cache - #479

Merged
pietro909 merged 3 commits into
masterfrom
fix/localstorage-quota-asset-cache
Mar 26, 2026
Merged

pietro909 merged 3 commits into
masterfrom
fix/localstorage-quota-asset-cache

Conversation

@pietro909

@pietro909 pietro909 commented Mar 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Evict expired entries (older than 24h TTL) from the cache on save, preventing unbounded growth
  • Extract ASSET_METADATA_TTL_MS into a shared constant in storage.ts

Fixes: Failed to execute 'setItem' on 'Storage': Setting the value of 'config' exceeded the quota.

The config key itself is small, but assetMetadataCache was filling up the ~5MB localStorage quota with large icon strings and never-evicted entries, causing subsequent writes (including config) to fail.

Test plan

  • Unit tests added for expired entry eviction
  • Manually verify asset icons still display correctly after page reload (re-fetched from server)
  • Verify that after 24h, stale asset metadata is refreshed rather than served from localStorage

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Asset metadata cache now properly expires outdated entries, preventing stale data from consuming storage space.
    • Optimized data storage by removing unnecessary icon information from asset metadata records while preserving other asset details.
  • Tests

    • Added comprehensive test coverage for asset metadata storage persistence and cache expiration validation.

pietro909 and others added 2 commits March 26, 2026 12:08
Asset metadata icons are base64-encoded image strings that can be tens
or hundreds of KB each. Persisting them fills up the ~5MB localStorage
quota unnecessarily since they are re-fetched on demand. The hasIcon
flag is preserved so the UI still knows an icon exists.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entries older than 24h (ASSET_METADATA_TTL_MS) are now dropped when
persisting the cache to localStorage, preventing unbounded growth.
The TTL constant is shared between storage.ts and wallet.tsx.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 26, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

These changes refactor the asset metadata cache TTL constant from wallet.tsx to the shared storage.ts module, add TTL-based expiration filtering to prevent stale data persistence, and strip the icon field from persisted metadata records to reduce storage overhead. New test cases validate both the TTL eviction behavior and icon field removal.

Changes

Cohort / File(s) Summary
Core Storage Logic
src/lib/storage.ts
Introduced ASSET_METADATA_TTL_MS constant and enhanced saveAssetMetadataToStorage to filter out expired cache entries before persistence and remove metadata.icon from stored CachedAssetDetails records.
Wallet Provider
src/providers/wallet.tsx
Removed local ASSET_METADATA_TTL_MS definition and imported it from src/lib/storage instead.
Test Coverage
src/test/lib/storage.test.ts
Added tests verifying that saveAssetMetadataToStorage strips the icon field during persistence and evicts cache entries exceeding the TTL threshold.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main purpose of the PR: fixing a localStorage quota error by optimizing the asset metadata cache. It directly relates to the core changes (stripping icons, evicting expired entries) that address the quota issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/localstorage-quota-asset-cache

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 527e249
Status: ✅  Deploy successful!
Preview URL: https://3db22699.wallet-bitcoin.pages.dev
Branch Preview URL: https://fix-localstorage-quota-asset.wallet-bitcoin.pages.dev

View logs

@ghost

ghost commented Mar 26, 2026

Copy link
Copy Markdown

🔍 Arkana Review — wallet#479

Solid fix. localStorage quota exhaustion from unbounded icon caching is a real user-facing bug.

What works well

  • Root cause addressed correctly — stripping base64 icon strings from persisted metadata while keeping hasIcon flag. Icons are re-fetched on demand.
  • TTL eviction on save — expired entries are pruned during saveAssetMetadataToStorage, preventing unbounded growth.
  • Shared constant — ASSET_METADATA_TTL_MS extracted to storage.ts and imported in wallet.tsx, eliminating duplication.
  • Tests — both icon stripping and expired entry eviction have dedicated unit tests.

Minor observations

  1. metadata destructure safety — const { metadata, ...rest } = v assumes metadata always exists. The ?? {} on metadata handles the nullish case for icon extraction, but worth confirming that the type guarantees metadata is always present on CachedAssetDetails, otherwise rest will silently omit it.

  2. Eviction timing — eviction only occurs during save. If the cache is read without a subsequent save (e.g., app only reads cached data), stale entries persist in localStorage until the next save cycle. This is fine for now since reads are typically followed by saves, but worth noting.

  3. No migration for existing caches — existing bloated caches will be cleaned on the next save, which is good. No explicit migration needed.

Looks good to merge. 👍

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 527e249
Status: ✅  Deploy successful!
Preview URL: https://476b861e.arkade-wallet.pages.dev
Branch Preview URL: https://fix-localstorage-quota-asset.arkade-wallet.pages.dev

View logs

Keep icons persisted in localStorage; only rely on TTL-based eviction
to control cache size.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@pietro909
pietro909 requested a review from bordalix March 26, 2026 11:23
@ghost

ghost commented Mar 26, 2026

Copy link
Copy Markdown

🔍 Review — arkade-os/wallet#479

Scope: Fix localStorage quota error by evicting expired entries from asset metadata cache on save.

Analysis

Clean, focused fix. The root cause is well-identified: assetMetadataCache accumulated large icon strings with no eviction, eventually blowing the ~5MB localStorage quota and breaking subsequent setItem calls (including config).

What changed:

  • ASSET_METADATA_TTL_MS extracted to shared constant in storage.ts (was duplicated in wallet.tsx)
  • saveAssetMetadataToStorage now filters out entries older than TTL before serializing
  • Unit test covers the eviction path

Correctness: ✅

  • Eviction logic is straightforward: now - cachedAt >= TTL_MS → skip. Boundary condition (>=) is correct — entries exactly at TTL are evicted, which is fine since they would be stale on the next read anyway.
  • The constant extraction removes the duplication between storage.ts and wallet.tsx.

One consideration: This only evicts on save. If the cache is read-heavy with few writes, stale entries persist in localStorage until the next save. In practice this is fine since any new asset fetch triggers a save, but worth noting.

Security: No concerns — no key material, no protocol logic.

LGTM. Small, well-scoped fix with test coverage.

@pietro909
pietro909 merged commit 7ed29fd into master Mar 26, 2026
5 checks passed
@bordalix
bordalix deleted the fix/localstorage-quota-asset-cache branch May 18, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant