fix: migrate session keys from starknet_* to stellar_* (Close #235)#261
fix: migrate session keys from starknet_* to stellar_* (Close #235)#261davedumto merged 3 commits intoStreamFi-x:devfrom
Conversation
|
@Agbasimere is attempting to deploy a commit to the chibuikemmichaelilonze's projects Team on Vercel. A member of the Team first needs to authorize it. |
davedumto
left a comment
There was a problem hiding this comment.
Review — Changes Requested
The core key migration (starknet_* → stellar_*) is correctly implemented in auth-provider.tsx and ProtectedRoute.tsx — nice work on that part. However, this PR includes several changes that were not requested in issue #235. Please revert those and keep the PR focused on what the issue asks for.
Out-of-scope changes to revert
1. app/api/streams/live/route.ts — Not mentioned in #235. Swallowing all errors as 200 { streams: [] } hides real production bugs. Please revert this file entirely.
2. Test files (chat-section.test.tsx, useChat.test.ts) & @testing-library/dom in package.json — Not mentioned in #235. screen and waitFor are already re-exported from @testing-library/react — splitting them into @testing-library/dom imports is unnecessary and adds an unneeded dependency. Please revert these files and remove the added dependency.
3. package.json — tsc → npx tsc — Not mentioned in #235. tsc already works in npm scripts via the typescript devDependency. Please revert.
Cleanup logic is over-engineered
4. StarknetKeyCleanup in providers.tsx — The issue suggests a simple cleanup:
if (localStorage.getItem("starknet_last_wallet")) {
localStorage.removeItem("starknet_last_wallet");
localStorage.removeItem("starknet_auto_connect");
}The current implementation runs cleanup 7+ times in 2 seconds (4 setTimeouts + a setInterval every 300ms + a synchronous call during render). This is overkill. A single useEffect with one call + one delayed retry is more than enough:
useEffect(() => {
removeAllStarknetKeys();
const timer = setTimeout(removeAllStarknetKeys, 500);
return () => clearTimeout(timer);
}, []);Also, the synchronous removeAllStarknetKeys() call outside the useEffect is a side effect during render — this should be removed.
5. Duplicate cleanup in auth-provider.tsx — StarknetKeyCleanup in providers.tsx already wraps AuthProvider, so the cleanup useEffect in auth-provider.tsx is redundant. Pick one location.
Formatting noise
6. ProtectedRoute.tsx — The semicolons and line-break reformatting mixed into this PR make the diff harder to review. The actual key rename is ~4 lines — the rest is formatting. Let Prettier handle formatting separately.
TL;DR: The key rename is correct. Please strip out the unrelated changes (items 1–3), simplify the cleanup (items 4–5), and separate formatting (item 6). Once those are addressed this should be good to merge.
|
Addressed the review: reverted route.ts, test imports, and package.json changes; simplified StarknetKeyCleanup to a single useEffect + one 500ms retry and removed the duplicate cleanup from auth-provider. ProtectedRoute only had the key renames. Kindly review. |
Closes #235
Summary:
Updates the localStorage keys used for wallet persistence as part of the Stellar migration. The old starknet_last_wallet / starknet_auto_connect keys are replaced with stellar_last_wallet / stellar_auto_connect, with a one-time cleanup to ensure returning users don’t get stuck with stale data.
Changes
auth-provider.tsx
Switches to the new stellar_* storage keys and adds a small fallback in useEffect to clean up legacy keys for users who still have them.
ProtectedRoute.tsx
Updated to read from stellar_auto_connect and stellar_last_wallet.
providers.tsx
Introduces StarknetKeyCleanup, which clears all starknet_* entries on load. It runs once synchronously and again after a short delay to cover cases where a wallet library re-inserts the old keys.
Misc
Minor type and test adjustments for local dev.
/api/streams/live now returns a 200 with an empty list during local development when no DB is connected (improves DX).
Loom
Part 1 — global search confirming no starknet_* usages left:
https://www.loom.com/share/b67be8d8a45d4ff4a70eeb91c2253c34
Part 2 — new stellar_* keys visible in DevTools localStorage:
https://www.loom.com/share/e7b2c1642bdd4e6abf1d2b13cec6378b
Part 3 — legacy starknet_* keys removed on app load (cleanup in providers.tsx).