MagicMoney Wallet — Comprehensive Audit

1. Executive Summary

MagicMoney is in very good shape for a pre-1.0 wallet. The security architecture — the part of a wallet you cannot retrofit — is genuinely strong: keys never leave the Electron main process, every window is sandboxed with context isolation, secrets are double-wrapped (PBKDF2-600k → AES-256-GCM → OS keychain) with a KDF upgrade path, provider API keys live only as Cloudflare Worker secrets, the proxy host is pinned to a code-level allowlist, and Supabase writes require a fresh single-use EIP-191 ownership signature. Typecheck is clean, all 37 unit tests and all 3 Playwright e2e tests pass, and a full live walkthrough of the app produced zero JavaScript errors.

The gaps are at the product-hardening layer, not the foundation: no automatic scam-token filtering (phishing airdrops render prominently in the Tokens tab — confirmed on screen), silent dApp chain-switching without a user prompt, thin send-form validation (any string >10 chars is accepted as an address), a duplicated wallet API across the Electron and extension runtimes (~2,900 lines that must be kept in sync by hand), and minimal e2e coverage (3 tests, extension-only). Nothing found is critical; three issues are High.

2. Architecture Assessment

Structure

Clean four-layer split with explicit trust boundaries:

Strengths

• Proxy-first provider routing (api-proxy.ts) with capability gates and graceful self-hoster fallback — a textbook seam.
• Multi-provider fallback ladders for every chain read; provider failure never renders as "you own zero" (last-known-good caches for token balances and NFT floors).
• Chain support is data-driven (chain-config.ts) with testnet-mode selectors rather than forked code paths.
• Deliberate, documented trade-offs — the code comments explain why (e.g. keyless providers stay client-side to preserve per-IP quotas).

Weaknesses

• Dual-runtime duplication: ipc-handlers.ts (1,460 lines) and extension/background.ts (1,421 lines) implement the same wallet API twice against different stores. Comments already show mirror-fixes ("Mirrors the Electron handler"). This is the #1 structural risk — see Refactors.
• God modules: token-fetcher.ts (1,737 lines) mixes per-chain token fetching, NFT fetching, spam-adjacent logic, pricing, and caching in one file; DashboardPage.tsx (1,163 lines) mixes data orchestration, caching, filtering, and five sub-UIs.
• EVM chain definitions exist in two shapes (chain-config.ts and the EVM_CHAINS map in tx-sender.ts) — the BSC "quoted but couldn't send" regression (fixed as M-1 in code comments) came exactly from this split.

3. UI/UX Assessment

Screenshots below are from a live scripted walkthrough of the built app (isolated profile, public test mnemonic). Overall: a distinctive, consistent dark identity; onboarding is genuinely good (masked seed with explicit reveal, strong warnings, password gate with no-recovery notice).

Findings

• HIGH Scam tokens displayed as legitimate holdings. The test wallet's Tokens tab shows "!Ads BTC Casino www.MaticSlot.io", "$ CLAiM : ethena-v2.com", and a 900,000,000-unit "halving-btc.net" token with phishing URLs fully readable. Manual hide/spam buttons exist, but the default presentation lends the wallet's credibility to phishing. (Issue H-1.)
• LOW Long scam-token names break the row layout (vertical character wrap, overlapping the price column) — needs text-overflow: ellipsis + min-width: 0 on the name column. (L-9.)
• HIGH Send form accepts any >10-char string as an address and any positive amount (no balance cap, no Max button); errors surface only at fee-estimation/broadcast as raw provider text. (H-3.)
• MED Password policy is length ≥ 8 only — no strength meter or common-password check for the credential protecting funds. (M-5.)
• LOW Send modal closes on Escape/overlay-click even during the Broadcasting… step — the tx still lands but the user loses the hash/success confirmation. (L-3.)
• LOW Accessibility: icon-only controls (titlebar min/close, hide/spam buttons) rely on title only — add aria-label; audit focus-visible styles. Muted-on-dark contrast is mostly fine. (L-7.)
• GOOD Six-theme system via CSS custom-property token overrides; onboarding safety copy; approval dialogs use danger-tone for risky signatures; App Hub and Market are polished.

4. Code Quality Assessment

• PASS npm run typecheck clean across both tsconfig projects; strict typing throughout; only ~40 any escapes in 25k lines, mostly at WalletConnect/chrome-API boundaries.
• PASS Zero TODO/FIXME/HACK markers. Comment quality is exceptional — modules document their security contracts and the why of workarounds.
• PASS 37/37 unit tests pass (wallet-core derivation vectors, crypto-vault, secure-store, tx-describe, swap-executor, api-proxy).
• MED Test coverage is concentrated: nothing on balance-fetcher, token-fetcher, chain-config invariants, or the swap/senders chain-parity (the BSC regression class). (M-8.)
• LOW Inline-style-heavy JSX (hundreds of style={{…}} objects in DashboardPage/SendModal/modals) alongside a real CSS token system — inconsistent, bloats render trees, invites theme drift. (L-5.)
• LOW Hardcoded hex colors (#22c55e App.tsx:273, #1FCE92, #ef4444) bypass the theme tokens the project's own convention mandates. (L-4.)

5. Performance Analysis

• GOOD Hot-path hygiene is unusually thoughtful: addresses/config are memory-cached because dApps poll many times per second (previously stalled window dragging); the CSP webRequest listener is URL-filtered to file:// to keep dApp traffic off the main process; market data comes from a cron-refreshed Worker KV cache instead of per-user CoinGecko calls; GPU rasterization is force-enabled for canvas-heavy dApps.
• MED Extension service worker is 3.76 MB (792 kB gzip). MV3 workers are torn down and re-parsed on every wake — this taxes every popup open and background event. The build log shows why: @solana/web3.js, WalletConnect, and blake2b are dynamically imported in background.ts but also statically imported by shared modules, so Rollup can't split them. (M-3.)
• MED 189 image-404s in one short session — dead token-logo URLs are re-fetched on every refresh cycle with no negative cache. Wasted network + console noise that will mask real errors. (M-6.)
• LOW Startup fans out to ~20 chains at once; acceptable now because failures degrade to cached values, but a staggered/priority fetch (visible chains first) would improve perceived cold-start.

6. Security Review

What's done right (a long list, deliberately)

• Mnemonic exists decrypted only in main-process memory (secure-store.ts); renderer gets addresses/booleans only; every signing path goes through one accessor with a 15-min genuine-idle auto-lock.
• At rest: PBKDF2-SHA256 600k → AES-256-GCM, wrapped again by OS safeStorage; per-blob KDF params with automatic upgrade-on-unlock; legacy migration path; kdfIterations() rejects downgraded params.
• Biometric unlock is additive (separate wallet.hello.enc), TPM-signature-derived on Windows, self-heals when the platform key is evicted, and never weakens the password path.
• All windows/views: sandbox: true, contextIsolation: true, nodeIntegration: false; strict response-header CSP for the packaged renderer; phishing guard on dApp navigation; popup allowlist; external URLs protocol-checked.
• Provider keys are Worker secrets, not bundle contents; the proxy origin is pinned to a code-level host allowlist so a tampered config.json cannot redirect signing-adjacent traffic (H-3 fix in code).
• Approval windows gate connect/sign/send; EIP-712 payloads are decoded with an explicit UNLIMITED-approval warning and danger tone; eth_sign gets a danger prompt; seed reveal is password-re-verified in both runtimes.
• Worker: prod fail-closed config guard, per-IP rate limiting, single-use replay-guarded EIP-191 ownership proofs (±10 min window + KV single-use) for all Supabase writes.

Gaps

• HIGH H-2: silent chain switching. wallet_switchEthereumChain / wallet_addEthereumChain (ipc-handlers.ts:887-920) change the active network with no user prompt. A malicious dApp can steer context before a tx request; the tx approval does name the chain (good mitigation), but users don't read chain names reliably. MetaMask prompts here for a reason.
• MED M-4: eth_sign is non-standard. It's implemented via signMessage (EIP-191 prefixed) — so dApps expecting raw-digest eth_sign receive signatures that won't verify. Since the prefixed behavior makes it redundant with personal_sign, reject eth_sign outright (industry norm) instead of returning a wrong-scheme signature.
• MED M-9: signing without connection. personal_sign/eth_signTypedData/eth_sendTransaction don't check the origin is approved first — any page in the dApp browser can pop a signing window. The user prompt still gates it, but requiring connection first reduces prompt-fatigue attacks.
• LOW L-2: mnemonic normalization inconsistency. deriveAddresses collapses inner whitespace; the signing helpers (wallet-core.ts:232+) only trim().toLowerCase(). A phrase persisted with irregular internal spacing would derive different keys at signing time than at address time. Currently unreachable via the UI (import joins words with single spaces), but it's a landmine — normalize once, in one shared helper.
• LOW L-8: self-hoster API keys pasted in Settings persist in plaintext config.json. Consider routing them through safeStorage too.

7. Database Review

• Local persistence is flat JSON in userData (addresses, config, approved origins, floor/token caches) with exact in-memory cache invalidation. Appropriate for the data volume; no SQLite needed. Secrets are never in plain files.
• Remote (Supabase) is exemplary for this threat model: the service-role key exists only as a Worker secret; the client proves address ownership with a canonical, byte-matched EIP-191 message binding action + address + timestamp; the Worker recovers the signer with @noble/curves (no heavyweight web3 lib), enforces a ±10-minute freshness window, and burns each signature in KV (single-use). Writes without a proxy degrade to a clean no-op rather than shipping a key.
• LOW The cache files (floor-cache.json, token-balance-cache.json) grow unboundedly keyed per address/collection — add a simple LRU/size cap eventually.

8. State Management Review

• No Redux/Zustand — plain useState + prop drilling, with App.tsx as a simple page router. This is the right call at current scale; don't add a state library.
• Smart mount strategy: Dashboard and Swap stay mounted (hidden) across tab switches so balances, scroll position, and in-flight swap pollers survive — deliberate and effective.
• The dApp chain id lives in main (dapp-chain.ts) with an origin-change reset — correct ownership, prevents cross-dApp chain leakage.
• MED DashboardPage is the de-facto state hub (balances, tokens, NFTs, history, spam sets, account switching, refresh timers in one 1,163-line component). Extract the data layer into hooks (usePortfolio, useSpamFilter) — same architecture, smaller blast radius. (Part of M-2.)
• LOW The hidden-mount strategy means bare text selectors match invisible nodes (bit this audit's own driver; documented in project memory) — a testing footgun worth a code comment.

9. Dependency Review

10. Playwright Testing Findings

• PASS Existing suite (e2e/extension-page-boundary.spec.ts): 3/3 green — verifies pages can't reach internal wallet methods, provider RPC routes stay on the approved path, and the EIP-1193 provider doesn't leak internals. Well-chosen security assertions.
• PASS This audit additionally drove the built Electron app end-to-end (isolated profile): Welcome → Create (seed reveal) → Import (12-word fill) → password → Dashboard (Networks/Tokens/Collectibles) → Market → Swap → Apps → Settings. Every flow completed; zero page errors; real balances rendered for the test mnemonic across Tron/Solana/Abstract/etc.
• MED M-7: coverage gap. Nothing exercises the Electron runtime, onboarding, send-flow validation, lock/unlock, settings, or regressions in tab navigation. The driver pattern used for this audit (wrapper-main + isolated userData + visible-filtered selectors) is proven and documented in project memory — commit it as e2e/electron-smoke.spec.ts.
• LOW Onboarding driver gotchas that should become code comments/fixtures: DevTools window can be Playwright's firstWindow(); hidden-mounted Dashboard nodes shadow text selectors.

11. Bugs & Issues Found

12. Technical Debt

13. Recommended Refactors

1. Shared wallet-API core (addresses M-1 debt). Extract handler bodies into src/shared/wallet-api.ts: pure functions taking a Store interface (already nearly identical between secure-store and chrome-store) + request context. Electron IPC and the extension message router become thin adapters. Do it incrementally — start with the web3 request switch, which is the highest-drift-risk surface. Complexity: High (spread over releases); payoff: every future feature/security fix lands once.
2. Split token-fetcher.ts into tokens/evm.ts, tokens/solana.ts, tokens/cardano.ts, tokens/bitcoin.ts, nft-floors.ts, spam-filter.ts (new, hosts H-1 heuristics). Complexity: Medium — mechanical, tests first.
3. Extract Dashboard data hooks (usePortfolio, useSpamSets, useAutoRefresh) leaving the page as composition. Complexity: Medium.
4. Unify EVM chain metadata: generate tx-sender's viem chain entries from chain-config.ts (single source of truth incl. explorers/symbols); add a unit test asserting swap-supported ⊆ send-supported. Complexity: Low-Medium.
5. Extension bundle diet: make the heavy libs consistently dynamic (or accept static and drop the pretense); target < 1.5 MB worker. Complexity: Medium.

Not recommended: a state-management library, a router, a CSS framework, or migrating to @solana/web3.js v2 — all cost with no current payoff.

14. Priority Rankings

15. Action Plan