Closed
Conversation
When an MV3 service worker wakes up and calls set() with an updater (e.g. requestStorage.addEvent spreading prev into a new array), the in-memory cache may still be null because _getDataFromStorage() is async. The updater then runs with prev = null and [...null] throws "TypeError: a is not iterable", aborting addEvent and leaving the approval flow in a bad state (popup opens for a request that was never persisted). Load the cache synchronously-ish (await from chrome.storage) in set() if it's still null, so every updater receives a real value. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
* fix: harden popup lifecycle against hangs and duplicates
Addresses three symptoms: popup hanging open after completion, multiple
popup windows opening per request, and the approval promise never
resolving when the user X's the popup.
methods.ts
- Replace in-memory isPopupOpen flag with chrome.windows.getAll lookup
that survives service worker restarts. Focuses an existing popup
(matched by URL) instead of opening a second one.
- Register chrome.windows.onRemoved exactly once at module load and
fan out to per-request subscribers, instead of adding a new listener
on every openPopup() call (previously leaked forever).
- requireApproval now resolves {success:false} when the popup closes
without an eth_sign_response, so chain handlers no longer hang and
the dapp's RPC call terminates.
chain handlers
- Every transaction_complete / signature_complete / transaction_error
message now carries eventId so the popup can match it to the right
in-flight request. ethereum threads the id through signMessage /
signTypedData; solana's buildEvent mutates requestInfo.id so the
match works end-to-end.
Transaction.tsx
- Ignores completion/error messages whose eventId doesn't match the
current event. Previously any signature_complete would close the
popup, slamming the window on unrelated queued requests.
ethereumHandler.ts
- Removes dead duplicate openPopup / requireUnlock (never called,
would have bypassed dedup if it had been).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
* fix: tag outer handleWalletRequest error with eventId
The outer catch in handleWalletRequest sends transaction_error to the
popup but was the one remaining site missing the eventId scope tag.
Combined with Transaction.tsx's backward-compat rule that accepts
unscoped messages, a thrown chain handler would surface its error on
the wrong event if two events were queued — exactly the
cross-contamination this PR is fixing everywhere else.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
* fix: harden popup empty state UX and make it self-healing Addresses the "popup open but says 'no events' with no recovery" failure mode, plus a latent crash if requestStorage returns null. Events.tsx - Null-guard requestStorage.getEvents() — previously a null return would throw on `for...of null` and white-screen the popup. - Wrap the fetch in try/catch and render a dedicated error state instead of crashing. - Subscribe to requestStorage changes so a second request arriving while the popup is open becomes visible immediately (no need to resolve the current request first). - Auto-close the window 3s after landing in an empty state. Covers the case where a request was cancelled upstream, where cleanup ran before the window closed itself, or where the popup was opened with no pending request. - Better copy in the empty / loading / error states so the user knows what is happening and that the window will close itself. - Keep currentIndex in bounds if the event list shrinks beneath it. Popup.tsx - Replace the placeholder "Error Occur" / "Loading ..." fallbacks with a proper Chakra-styled error panel that includes a Close button, so a rendering crash doesn't leave the user with no way out other than clicking the OS window close button. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix: clamp Events currentIndex inline to avoid one-render crash The useEffect-based bounds check snapped currentIndex back only AFTER the render that caused it to go out of bounds. When requestStorage's subscribe fired and the event list shrank beneath currentIndex, the first render after the shrink still had the stale index — passing events[currentIndex] = undefined into <Transaction />, which reads event.id immediately and error-boundary crashes. Compute a clamped safeIndex during render instead. currentIndex as state is preserved for user-driven navigation; only the array access is guarded. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix: auto-close popup on fetch error, not just empty state The error-state UI copy already said the window would close itself, but the auto-close effect bailed on fetchError, leaving the user stuck on a dead-end error screen — the exact failure mode this PR is trying to remove. Collapse the two conditions into a single shouldAutoClose predicate so any non-actionable state (empty OR fetch failure) triggers the timer. Cleanup still runs correctly on recovery (error clears → new events arrive → previous cleanup cancels the timer, new effect short-circuits). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
* fix: replace broken remote logo refs with local assets
The client referenced api.keepkey.info/coins/{keepkey,pioneerMan,ethereum}.png
shortnames that never existed on the CDN (all 403). The CDN only serves
CAIP-base64-encoded filenames, which the dynamic asset-icon code already
generates correctly. This change fixes every hardcoded shortname usage:
- KeepKey brand marks across popup + side-panel switch to bundled /kk-logo.png
(and chrome.runtime.getURL for the EIP-6963 announce icon and manifest
web_accessible_resources so dapps can load it).
- AddDappModal default icon now uses caipToIcon('eip155:1/slip44:60'), which
the CDN does serve.
- NetworkDropdown hides the avatar entirely when no network is selected
instead of rendering Chakra's default silhouette from a 403 fallback.
- headerUtils.getIconUrl last-resort fallback no longer builds
btoa(chainSymbol) URLs (always 403); returns '' so Chakra Avatar falls
back to an initial letter.
- wallet.ts serviceImageUrl points at pioneers.dev (chrome-extension:// URLs
can't load in the vault desktop UI).
Third-party image hosts also replaced with bundled assets:
- Animated kk.gif (5.6MB from i.ibb.co) converted to kk.webp (286KB, 240px,
15fps) — 95% smaller, same animation.
- MetaMask fox and Keplr logos downloaded from their official repos.
- Xfi src dropped (Coming Soon overlay covers the avatar; letter fallback).
Net: zero remote image hosts remain outside the working CAIP CDN pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
* fix: move new branded assets to chrome-extension/public root
Absolute src="/kk.webp" and /brand/... paths resolve to the extension
origin root (chrome-extension://<id>/kk.webp). Vite's base:'' does not
rewrite these, so emitting them under pages/side-panel/public/ (→
dist/side-panel/) was a 404. Moving to chrome-extension/public/ lands
them at the dist root and matches the already-working /kk-logo.png
convention.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
SidePanel.tsx default-state welcome screen uses <img src="/logo_vertical.svg"> as an opacity-0.25 watermark. The file was in pages/side-panel/public/, so Vite emitted it to dist/side-panel/logo_vertical.svg while the absolute path requested chrome-extension://<id>/logo_vertical.svg — 404. Same class of bug that #41 fixed for kk.webp and brand icons. Moving both vertical logos to chrome-extension/public/ lands them at the dist root and matches the working /kk-logo.png / /kk.webp convention. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Previous encode (q=50, 15fps) produced visible "tracing" — residual inter-frame prediction errors that looked like motion trails. Bumping quality to 90 and frame rate to 20fps eliminates the smear while still landing at 560KB (90% smaller than the original 5.6MB GIF). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The ffmpeg-produced animated WebP had broken frame disposal — every frame composited on top of the previous, leaving "tracing" ghosts. Falling back to a properly-optimized GIF (240px, 192 colors, coalesced + layered optimize) lands at 690KB and animates correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Prefetch Solana pubkey during onStart so the network shows up in the dropdown without waiting for a dapp-initiated request. Previously the Solana pubkey was only derived lazily on first dapp interaction, so the network list silently skipped it on fresh sessions. - Hide the total balance + Send/Receive action row during the initial balance fetch. Showing $0.00 + disabled buttons while loading caused a jittery shift once balances arrived. - Center the Balances loading spinner full-height with a caption and smoother spin. Replaces the top-anchored inline spinner that looked cramped above the card list. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
* feat: spice up balance loading screen with hero spinner and skeleton rows Replace the plain centered spinner + "Loading balances…" text with a multi-layer animated hero spinner (triple counter-rotating rings, pulsing glow, breathing center dot) above skeleton cards that mirror the real asset row layout. Adds a subtle kk-logo watermark behind everything and a teal shimmer sweep that travels across each skeleton card staggered by 150ms. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix: show Solana balance and SPL tokens in side panel Three layered bugs were each sufficient to zero out Solana balance display; all three are fixed together because fixing any one alone leaves the chain still broken: 1. Wrong CAIP in shortListSymbolToCaip['SOL'] / shortListNameToCaip.solana. Previously pointed at wrapped-SOL SPL token (solana:.../solana:so111…, all lowercase). Now points at native SOL (solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501), matching pioneer-caip's ChainToCaip and vault-v11's config. 2. Wrong Pioneer endpoint for Solana. /charts/portfolio returns empty {balances:[], tokens:[]} for any Solana pubkey (verified via direct curl). Vault-v11 uses /portfolio via pioneer.GetPortfolioBalances, which returns natives + SPL tokens in one flat array. Route Solana pubkeys to a third batch hitting /portfolio with the required key:public-* Authorization header; EVM/UTXO still go through /charts/portfolio for its richer Zapper/Unchained token data. 3. Response case mismatch. Pioneer echoes CAIP/networkId back in lowercase regardless of request casing. The side-panel asset list uses canonical mixed-case network IDs from ChainToNetworkId, so strict b.networkId === asset.networkId comparisons in Balances.tsx silently dropped every Solana entry. Rewrite Solana entries to canonical casing before they enter the merged balances array. Also eliminates a first-run race: the initial fetchBalancesFromPioneer() fired before prefetchSolanaPubkey() persisted the Solana pubkey, so run 1 never included Solana at all. Chain a forced refetch on prefetch resolution so the Solana entry lands in cachedBalances before the UI mounts. Verified against the live Pioneer API: for the exact address the client derives from the device at m/44'/501'/0'/0', /portfolio returns {native SOL + 3 SPL tokens}. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(review): prevent stale-fetch cache clobber and push balance updates to UI Addresses two PR review findings: 1. HIGH: stale earlier fetch could clobber a later fetch's result. The first cold-start fetchBalancesFromPioneer() starts before the Solana pubkey is persisted, and the chained forceRefresh fetch runs in parallel. If the forced fetch finishes first (correctly populating cachedBalances with SOL + SPL tokens) and the original slower fetch finishes later, the original unconditionally overwrote cachedBalances back to the pre-Solana snapshot. Fix: tag each fetch with a monotonic latestFetchId bumped only when real work starts (not for dedup-return paths), and only commit to cachedBalances if this fetch is still the most recent (myFetchId === latestFetchId). Superseded fetches log their discard and return their result to direct callers without touching the cache. 2. MEDIUM: UI never observed late cache updates. Balances.tsx and SidePanel.tsx each fetched GET_APP_BALANCES once and then stopped listening, so the cold-start forced refetch that lands Solana after the panel mounts was invisible to users. Fix: background now emits BALANCES_UPDATED via chrome.runtime.sendMessage every time cachedBalances is successfully written. Balances.tsx, SidePanel.tsx, and Tokens.tsx subscribe to that message and re-fetch so the UI reflects the latest cache without the user having to manually refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
…back networkIdToIcon() was mapping Solana to the wrapped-SOL SPL CAIP (solana:.../solana:So111…), which returns 403 on keepkey.info/coins/<base64-caip>.png. The Avatar component fell back to rendering the first letter of the network name — a green "S" badge where the logo should be. Native SOL's slip44:501 CAIP has a real icon (200), matching pioneer-caip's ChainToCaip convention used elsewhere in the codebase after PR #42. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds thin Makefile over pnpm scripts so the stack-wide "use make for everything" convention applies here too. Also commits the current minified build of the Solana/EVM injected script. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Port the approval UI into the side panel and remove the popup entirely.
Side panel is now the sole surface for both portfolio and dApp approvals.
Three-phase merge plus iterative review fixes across five rounds:
Phase A: port popup approval components to side-panel/src/approval and
subscribe SidePanel to requestStorage.
Phase B: background.methods.ts opens the side panel via
chrome.sidePanel.open; setPanelBehavior({openPanelOnActionClick:true})
+ action badge as user-gesture fallback; 10-minute approval timeout
replaces the old popup-closed escape hatch.
Phase C: delete pages/popup, its e2e specs, and the OPEN_SIDEBAR
background handler. Chrome-only; Firefox build hard-gated until a
replacement UI ships (task #5).
Iterative fixes touched: approval routing / id mismatch, PersonalSignTx
wire-up, bridge same-origin + falsy RPC handling, provider coexistence
(stop stomping window.ethereum), GET_PUBKEY_CONTEXT asset scoping,
Receive dedup by CAIP, ETH account removal runtime state, custom
network mirror into provider stores, GET_CHARTS networkId filter,
Clear-all covers every user-written storage, approval window routing,
accountIndex preservation end-to-end, full asset-context pass-through,
native-preference for default global send/receive, scalar balance
fallback in Transfer. New side-panel approval e2e smoke covers the
reject path.
Lint check is red on develop (470 errors pre-existing); this PR's
lint delta is ~0 after ignoring the ported approval subtree.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bump every workspace to 0.0.27 via ./update_version.sh for the release cut. Also restore chrome-extension/public/injected.js to its esbuild-minified form — prettier had un-minified it on a prior lint-staged run, bloating the shipped bundle and making every build produce a noisy diff. Add the file to .prettierignore so this stays fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The "KeepKey Vault Required" card rendered without a background on top of the side panel's dark theme, so the title (no color) and the \`gray.500\` / \`gray.400\` helper text were effectively black on near- black. Users couldn't read the "vault must be running" instruction. Give the card an explicit dark-but-distinct background (\`bg="gray.800"\` over the \`gray.900\` panel) with a faint border for separation, and switch every Text to the white/whiteAlpha scale the rest of the sidebar uses. Link color bumped to \`teal.300\` so it's visible against gray.800. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Collaborator
Author
|
Closing — holding the 0.0.27 cut while TON + Tron support and related fixes land on develop first. Will reopen a new release PR once the next batch is ready to ship. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cuts 0.0.27 from develop. Thirteen commits since master; headline is the popup → side-panel refactor.
Headline change: popup → side-panel (`80be566`)
The separate approval popup window is gone. The side panel is now the sole surface for both portfolio and dApp approvals:
Landed with ~30 review-fix commits covering approval routing correctness, content-bridge security + falsy RPC handling, window.ethereum coexistence, asset-context account-index preservation end-to-end, Receive CAIP-based dedup, custom-network lifecycle, clear-all completeness, and default native-asset selection for global Send/Receive.
Other fixes from prior PRs in this batch
Known limitations
Test plan
🤖 Generated with Claude Code