Conversation
feruzm
changed the title
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds MetaMask + Hive Snap authentication and signing across the web app, migrates wallet/signup/setup flows to MetaMask-first (removing many seed/seed-phrase UI pieces), replaces seed-based master-password derivation across add-keys/permissions flows, adds related hooks/components/tests, and updates broadcast logic to route MetaMask via Hive Snap. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (web app)
participant MetaMask as MetaMask Extension
participant HiveSnap as Hive Snap (inside MetaMask)
participant Backend as Backend (Auth API)
participant HiveChain as Hive Blockchain
Client->>MetaMask: request accounts / connect
MetaMask->>Client: accounts / addresses
Client->>HiveSnap: install/connect & request public posting key
HiveSnap->>Client: posting public key
Client->>Backend: fetch HS challenge / verify on-chain keys
Backend->>Client: HS code (challenge)
Client->>HiveSnap: sign HS code (hive_encrypt)
HiveSnap->>Client: signature
Client->>Backend: submit signature -> authenticate
Backend->>Client: auth token / login success
Client->>HiveChain: broadcast signed transaction (via adapter)
HiveChain->>Client: broadcast result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsx-173-177 (1)
173-177:⚠️ Potential issue | 🟡 MinorLocalize the RC input placeholder.
"Enter amount to delegate (Bn)"is a new user-facing string in TSX. Move it to locale JSON and reference viai18next.t(...).As per coding guidelines:
apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx around lines 173 - 177, The placeholder string for the RC input is hard-coded; add a new key (e.g. "onboard.delegatePlaceholder") to en-US.json with value "Enter amount to delegate (Bn)" and replace the literal in the FormControl placeholder prop with i18next.t('onboard.delegatePlaceholder'); update imports if needed, keeping the existing FormControl, rcAmount and setRcAmount usage intact so only the placeholder is localized.apps/web/src/app/signup/_components/signup-layout-client.tsx-57-61 (1)
57-61:⚠️ Potential issue | 🟡 MinorAvoid inline fallback text for new i18n copy.
defaultValue: "All signup options"introduces a new user-facing string in TSX. Keep the string only in locale JSON and calli18next.t("sign-up.all-options")directly.As per coding guidelines:
apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/_components/signup-layout-client.tsx` around lines 57 - 61, Remove the inline fallback string from the i18n call so new user-facing text lives only in locale JSON: change the component rendering that uses isSubPage and the Link (rendering UilArrowLeft and calling i18next.t("sign-up.all-options", { defaultValue: "All signup options" })) to call i18next.t("sign-up.all-options") with no defaultValue, and add the "sign-up.all-options" entry to en-US.json instead.apps/web/src/app/onboard-friend/[...slugs]/_page.tsx-19-20 (1)
19-20:⚠️ Potential issue | 🟡 MinorEncode hash before interpolating into route path.
Use
encodeURIComponent(hash)to prevent malformed redirects when slug contains reserved URL characters.💡 Suggested fix
- const hash = slugs[slugs.length - 1]; - router.replace(hash ? `/signup/invited/${hash}` : "/signup/invited"); + const hash = slugs[slugs.length - 1]; + router.replace(hash ? `/signup/invited/${encodeURIComponent(hash)}` : "/signup/invited");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_page.tsx around lines 19 - 20, The redirect builds a path using the raw slug value (slugs, hash) which can break when the slug contains reserved URL characters; update the router.replace call that uses hash (router.replace(hash ? `/signup/invited/${hash}` : "/signup/invited")) to encode the hash with encodeURIComponent(hash) before interpolating it into the path so the resulting URL is safe and well-formed.apps/web/src/app/signup/free/_page.tsx-68-81 (1)
68-81:⚠️ Potential issue | 🟡 MinorIgnore stale username lookup results.
The debounced availability check can resolve after the input has already changed, and this callback will still stamp
sign-up.username-existsonto the newer value. That leaves the form disabled for a valid username until the user edits again. Track the queried username and ignore superseded responses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/free/_page.tsx` around lines 68 - 81, The debounced username availability check in useDebounce can apply results to a newer input; fix it by snapshotting the username being queried (e.g., const queried = username) before calling queryClient.fetchQuery(getAccountsQueryOptions([username])) and, in the then handler, compare queried === username (or use a ref like lastQueriedUsername set before the fetch) and only call setUsernameError(i18next.t("sign-up.username-exists")) and setIsDisabled(true) when they match; also ensure you clear the error/disabled state when the current username is different or valid inside the same check flow.apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx-430-467 (1)
430-467:⚠️ Potential issue | 🟡 MinorMove the new status copy into locale keys.
This block introduces raw strings like
"Checking MetaMask multichain addresses...","address:", and"Balance:", so the flow will ship partially untranslated.As per coding guidelines, "All new strings must be added to
en-US.jsononly for internationalization".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx` around lines 430 - 467, The component renders untranslated literals ("Checking MetaMask multichain addresses...", "address:", "Balance:") — add new locale keys to en-US.json (e.g., signup-wallets.metamask.checkingMultichain, signup-wallets.metamask.addressLabel, signup-wallets.balance.label) and replace the raw strings in metamask-connect.tsx with i18next.t(...) calls (use existing i18next import and the surrounding context where selectedCurrency, connectedAddress, tokenAmount, CURRENCIES_META_DATA, Spinner are used) so all new text is pulled from the locale file.apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx-273-281 (1)
273-281:⚠️ Potential issue | 🟡 MinorRetry mechanism may not re-trigger the effect.
The retry button resets
hasInitiatedRef.current = falseandsetStatus("installing-snap"), but theuseEffectdependencies are[username, verifiedWallet, validateAccountIsCreated, recordActivity]. Since none of these change on retry, the effect won't re-run.🐛 Suggested fix - add a retry counter to dependencies
+ const [retryCount, setRetryCount] = useState(0); + useEffect(() => { if (hasInitiatedRef.current) return; hasInitiatedRef.current = true; // ... rest of effect - }, [username, verifiedWallet, validateAccountIsCreated, recordActivity]); + }, [username, verifiedWallet, validateAccountIsCreated, recordActivity, retryCount]); // In the retry button onClick: onClick={() => { hasInitiatedRef.current = false; setStatus("installing-snap"); + setRetryCount(c => c + 1); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 273 - 281, The retry button currently sets hasInitiatedRef.current = false and setStatus("installing-snap") but the effect that performs the install uses dependencies [username, verifiedWallet, validateAccountIsCreated, recordActivity] so it won't re-run; add a retry counter state (e.g., retryCount via useState) and include it in the effect's dependency array, then increment retryCount in the Button onClick (alongside resetting hasInitiatedRef and setStatus) so the useEffect observing retryCount will re-trigger the installation logic in the component (refer to hasInitiatedRef, setStatus, and the useEffect that depends on username, verifiedWallet, validateAccountIsCreated, recordActivity).apps/web/src/app/signup/premium/_page.tsx-244-257 (1)
244-257:⚠️ Potential issue | 🟡 MinorAdd
rel="noopener noreferrer"to external links withtarget="_blank".Opening links in new tabs without
rel="noopener"can expose the originating page towindow.openermanipulation, a potential security vector.🔒 Proposed fix
- <a href="https://ios.ecency.com" className="flex items-center gap-2 bg-gray-100 dark:bg-dark-default hover:bg-gray-200 dark:hover:bg-black rounded-lg px-3 py-2" target="_blank"> + <a href="https://ios.ecency.com" className="flex items-center gap-2 bg-gray-100 dark:bg-dark-default hover:bg-gray-200 dark:hover:bg-black rounded-lg px-3 py-2" target="_blank" rel="noopener noreferrer">- <a href="https://android.ecency.com" className="flex items-center gap-2 bg-gray-100 dark:bg-dark-default hover:bg-gray-200 dark:hover:bg-black rounded-lg px-3 py-2" target="_blank"> + <a href="https://android.ecency.com" className="flex items-center gap-2 bg-gray-100 dark:bg-dark-default hover:bg-gray-200 dark:hover:bg-black rounded-lg px-3 py-2" target="_blank" rel="noopener noreferrer">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/premium/_page.tsx` around lines 244 - 257, Add rel="noopener noreferrer" to both external anchor elements that use target="_blank" (the anchors linking to "https://ios.ecency.com" and "https://android.ecency.com" which wrap {appleSvg} and {googleSvg}) to prevent window.opener access; update those two <a> elements to include rel="noopener noreferrer".apps/web/src/app/signup/premium/_page.tsx-99-104 (1)
99-104:⚠️ Potential issue | 🟡 MinorFix the
.some()callback - it doesn't return a value and the result is unused.The callback never returns a value (no
return), so.some()always returnsfalse. Additionally, the result of.some()is not used. If the intent is to iterate and set errors, use.forEach()or.find()with proper returns.🐛 Proposed fix using forEach
- } else { - referral.split(".").some((item) => { - if (item.length < 3) { - setReferralError(i18next.t("sign-up.referral-min-length-error")); - setIsDisabled(true); - } - }); - } + } else { + const hasShortSegment = referral.split(".").some((item) => item.length < 3); + if (hasShortSegment) { + setReferralError(i18next.t("sign-up.referral-min-length-error")); + setIsDisabled(true); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/premium/_page.tsx` around lines 99 - 104, The callback passed to referral.split(".").some(...) never returns a value and the .some() result is unused; change this to iterate properly — either replace .some(...) with .forEach(...) so each segment is checked and you call setReferralError(...) and setIsDisabled(true) when item.length < 3, or use .find(...) / .some(...) but ensure the callback returns true on the short-item condition and capture the boolean result if needed; update the code around referral.split(".") and keep setReferralError and setIsDisabled calls as-is when the condition is met.apps/web/src/features/shared/login/hooks/use-login-by-metamask.ts-20-25 (1)
20-25:⚠️ Potential issue | 🟡 MinorGuard against
window.ethereumbeing undefined in helper functions.The helper functions use
window.ethereum!with a non-null assertion, but they're defined outside the mutation where the MetaMask check occurs. If these helpers are called before the guard or reused elsewhere, they could throw.🛡️ Proposed defensive check
async function ensureHiveSnap(): Promise<void> { + if (!window.ethereum) { + throw new Error('MetaMask not found'); + } - await window.ethereum!.request({ + await window.ethereum.request({ method: "wallet_requestSnaps", params: { [HIVE_SNAP_ID]: {} } }); }Apply similar guards to
getHivePublicKeysandsignBufferWithSnap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/login/hooks/use-login-by-metamask.ts` around lines 20 - 25, The helpers (ensureHiveSnap, getHivePublicKeys, signBufferWithSnap) currently use window.ethereum! and can throw if called when MetaMask is absent; add a defensive runtime check at the top of each function that verifies window is defined and window.ethereum exists (e.g., if (typeof window === "undefined" || !window.ethereum) throw new Error("MetaMask/ethereum provider not available")), replace the non‑null assertion usage with safe access (window.ethereum.request(...)), and ensure the functions either throw a clear Error or return a rejected Promise when the provider is missing so callers can handle it.apps/web/src/app/signup/invited/[hash]/_page.tsx-93-99 (1)
93-99:⚠️ Potential issue | 🟡 MinorPotential infinite loop in useEffect due to dependency on
rcAmount.The effect depends on
rcAmountand callsrcOperationsCost(), which readsrcAmountand sets multiple state values. While it doesn't directly updatercAmount, addingrcAmountto the dependency array means this effect runs every timercAmountchanges (including on user input), which triggers multiple fetches.Consider debouncing or restructuring to avoid unnecessary
fetchQuerycalls on every keystroke.♻️ Suggested restructure
useEffect(() => { if (!isChecked) { setRcAmount(0); + setCommentAmount(0); + setVoteAmount(0); + setTransferAmount(0); + setCustomJsonAmount(0); + setRcError(""); return; } - rcOperationsCost(); -}, [isChecked, rcAmount]); +}, [isChecked]); + +// Separate debounced effect for RC calculations +useDebounce( + () => { + if (isChecked) { + rcOperationsCost(); + } + }, + 300, + [rcAmount, isChecked] +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx around lines 93 - 99, The useEffect watching isChecked and rcAmount causes rcOperationsCost() to fire on every rcAmount change (keystrokes), leading to repeated fetchQuery calls; update the effect to only depend on isChecked (remove rcAmount from the dependency array) and trigger rcOperationsCost() from a debounced handler or on explicit events (e.g., onBlur or form submit) instead; locate the useEffect and the rcOperationsCost function and replace the direct rcAmount dependency with either a debounced call tied to the rcAmount input or move the fetch logic to an explicit handler to avoid firing fetchQuery on every keystroke.
🧹 Nitpick comments (8)
apps/web/src/app/signup/wallet/_page.tsx (1)
13-17: Consider extracting sharedVerifiedWalletinterface.The
VerifiedWalletinterface is defined here and also used inmetamask-account-creating.tsx(lines 19-26). Consider extracting it to a shared types file to maintain DRY principles.♻️ Suggested approach
Create a shared types file:
// apps/web/src/app/signup/wallet/_types.ts import { EcencyWalletCurrency } from "@ecency/wallets"; export interface VerifiedWallet { currency: EcencyWalletCurrency; address: string; addresses: Partial<Record<EcencyWalletCurrency, string>>; }Then import from both files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_page.tsx` around lines 13 - 17, The VerifiedWallet interface is duplicated (defined here and in metamask-account-creating.tsx); extract VerifiedWallet into a shared types module (e.g., _types.ts) and export it, then replace the local interface declarations in both this file and metamask-account-creating.tsx with an import of the shared VerifiedWallet type to remove duplication and keep types DRY.apps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsx (1)
139-156: Hardcoded string should use i18n.The button label "MetaMask" on line 154 is hardcoded. Per coding guidelines, all new strings must be added to
en-US.jsonfor internationalization.♻️ Suggested fix
> - MetaMask + {i18next.t("key-or-hot.with-metamask", { defaultValue: "MetaMask" })} </Button>Add corresponding entry to
en-US.json:"key-or-hot.with-metamask": "MetaMask"As per coding guidelines: "All new strings must be added to
en-US.jsononly for internationalization"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsx` around lines 139 - 156, Replace the hardcoded "MetaMask" button label with a localized string: use the i18n translation function (e.g., t or useTranslation) where the Button rendered under isMetaMaskUser and onClick={handleMetaMask} reads its label from a key like "key-or-hot.with-metamask" instead of the literal; then add "key-or-hot.with-metamask": "MetaMask" to en-US.json so the locale file contains the new entry.apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx (3)
254-258: Hardcoded UI strings should use i18n.The strings "Back to origin" and "Explore Ecency" are hardcoded. Per coding guidelines, all new strings must be added to
en-US.json.♻️ Suggested fix
<Button size="lg"> - {params?.has("backUri") ? "Back to origin" : "Explore Ecency"} + {params?.has("backUri") + ? i18next.t("g.back-to-origin", { defaultValue: "Back to origin" }) + : i18next.t("g.explore-ecency", { defaultValue: "Explore Ecency" })} </Button>As per coding guidelines: "All new strings must be added to
en-US.jsononly for internationalization"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 254 - 258, Replace the hardcoded button labels in the MetamaskAccountCreating component (the Link/Button block that renders {params?.has("backUri") ? "Back to origin" : "Explore Ecency"}) with i18n lookups; add two new keys (e.g. metamask.backToOrigin and metamask.exploreEcency) to en-US.json and use the app's translation hook/utility (the same pattern used elsewhere in this component) to render t('metamask.backToOrigin') and t('metamask.exploreEcency') instead of the literal strings so the UI reads from the locale file.
111-134: Type safety concern with{} as any.Line 118 uses
{} as anyto bypass TypeScript's type checking forqueryFn. This could hide potential issues if the query function signature changes.♻️ Suggested improvement
- const account = await accountQueryOptions.queryFn!({} as any); + const account = await accountQueryOptions.queryFn!({ + queryKey: accountQueryOptions.queryKey, + signal: new AbortController().signal, + meta: undefined + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 111 - 134, The current use of "{} as any" when invoking accountQueryOptions.queryFn inside validateAccountIsCreated bypasses TypeScript safety; replace this with a properly-typed call by either invoking the query function with the expected QueryFunctionContext (an object containing the queryKey used in getAccountFullQueryOptions) or, simpler and safer, use the typed QueryClient helper (e.g., queryClient.fetchQuery or fetchQuery with accountQueryOptions) so you don't need to cast to any; update validateAccountIsCreated to call accountQueryOptions.queryFn with the correct shape or delegate to queryClient.fetchQuery and keep the existing setQueryData call (queryClient.setQueryData) to cache the result.
63-98: Consider adding error handling for fetch response body.The
createAccountWithWalletsfunction returns the raw Response object, but errors from the backend (e.g., validation failures, duplicate username) may include useful error messages in the response body that aren't surfaced to users.♻️ Suggested improvement
async function createAccountWithWallets( // ... params -): Promise<Response> { +): Promise<{ ok: boolean; error?: string }> { // ... body construction - return fetch(`${ConfigManager.getValidatedBaseUrl()}/private-api/wallets-add`, { + const response = await fetch(`${ConfigManager.getValidatedBaseUrl()}/private-api/wallets-add`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ /* ... */ }) }); + if (!response.ok) { + const errorData = await response.json().catch(() => ({})); + return { ok: false, error: errorData.message || "Account creation failed" }; + } + return { ok: true }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 63 - 98, The createAccountWithWallets function currently returns the raw fetch Response which hides backend error messages; update createAccountWithWallets to check the Response.ok status, parse the response body (preferably JSON, falling back to text) when !ok to extract backend error details, and then throw or return a rejected Error containing the parsed message and status so callers can surface useful validation/duplicate errors; keep the request construction (username, token, meta) the same and perform this response-handling right after the fetch call in createAccountWithWallets.apps/web/src/features/shared/login/login.tsx (1)
206-228: Hardcoded string "MetaMask" should use i18n.The button label on line 226 is hardcoded. For consistency with other login methods and per coding guidelines, use i18next.
♻️ Suggested fix
> - MetaMask + {i18next.t("login.with-metamask", { defaultValue: "MetaMask" })} </Button>As per coding guidelines: "All new strings must be added to
en-US.jsononly for internationalization"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/login/login.tsx` around lines 206 - 228, Replace the hardcoded button label "MetaMask" with an i18n lookup using the project's i18next hook (e.g., useTranslation) and the Button in this component (the JSX inside the conditional rendering where Button with props and icon is rendered); update the component to call t('login.metamask') (or similar key) and add that key/value to en-US.json only. Ensure you import/use useTranslation in this file and keep the button behavior (onClick -> loginByMetaMask, disabled -> !username || isLoginByMetaMaskPending, isLoading -> isLoginByMetaMaskPending) unchanged.apps/web/src/app/signup/premium/_page.tsx (1)
39-39: Consider typing the form ref properly.Using
anybypasses TypeScript's type checking. Consider usingHTMLFormElementfor better type safety.♻️ Suggested improvement
- const form = useRef<any>(); + const form = useRef<HTMLFormElement>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/premium/_page.tsx` at line 39, The form ref is currently untyped (useRef<any>()); change it to a proper HTMLFormElement ref by initializing useRef<HTMLFormElement | null>(null) and update any usages to guard with form.current (e.g., form.current?.submit(), form.current?.reset(), or access elements via form.current.elements) so TypeScript can check DOM form operations; look for the variable named form created via useRef and update its type and initial value accordingly.apps/web/src/app/signup/invited/[hash]/_page.tsx (1)
136-142: Consider awaitingonboardEmailor handling its potential failure.
onboardEmailis called withoutawait, so any failure is silently ignored. If notification failure should be surfaced to the user or logged, consider awaiting and wrapping in try/catch.♻️ Suggested improvement
setModalStep("success"); // Notify new user by email - onboardEmail(newAccountName, decodedInfo.email, activeUser.username); + try { + await onboardEmail(newAccountName, decodedInfo.email, activeUser.username); + } catch (emailErr) { + console.warn("Failed to send onboard email:", emailErr); + } } catch (err: any) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx around lines 136 - 142, The call to onboardEmail(newAccountName, decodedInfo.email, activeUser.username) is un-awaited so failures are ignored; change it to await the promise and handle failures by wrapping the onboardEmail call in a try/catch (inside the same surrounding try or a nested one) and on error log the failure (using error(...) or process logger), setModalStep("failed") and return or otherwise surface the failure so the user is notified; reference the onboardEmail call and the existing catch block that currently calls setModalStep("failed") and error(...formatError(err)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/package.json`:
- Line 67: Add the missing dependency entry for "@wallet-standard/app": "^1.1.0"
to the self-hosted package.json to match the web app manifest; open the
self-hosted package.json and append or merge the dependency under the
"dependencies" object ensuring the exact version string "@wallet-standard/app":
"^1.1.0" is present so both manifests remain in parity.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx:
- Around line 30-32: The fileIsDownloaded boolean is not tied to the current
seed phrase so it can remain true when a new seed is generated; update the
component to reset that flag whenever the seed changes and only set it true
after a successful export of a non-empty seed. Specifically, add an effect that
watches seedPhrase (from useSeedPhrase) and calls setFileIsDownloaded(false)
whenever seedPhrase changes, and in the download/export handler (where you call
the export logic and set secret) only call setFileIsDownloaded(true) after
verifying seedPhrase is non-empty (and after the export completes). Also disable
the download button while seedPhrase is empty to prevent exporting an empty
backup.
- Around line 60-67: The current flow builds an unsigned invite object
(dataToEncode) and places b64uEnc(JSON.stringify(dataToEncode)) into the URL via
setSecret, which only obscures but does not protect or authenticate the payload;
instead, send the invite data (username, email, referral, pubkeys) to the
backend to mint an opaque server-issued token (or return a signed/encrypted
blob) and use that token in setSecret; update the client code around
dataToEncode / b64uEnc / setSecret to POST the payload to an endpoint that
returns a short-lived opaque token (or HMAC-signed/encrypted payload) and have
the server validate/sign the token during account creation so link tampering or
exposure cannot alter the invite fields.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx:
- Around line 87-90: The sendMail function (which calls onboardEmail with
decodedInfo.username, decodedInfo.email, and activeUser.username) returns a
promise but is invoked elsewhere without awaiting or handling rejections; update
call sites to either await sendMail() inside an async context or attach
.catch(...) to handle errors, and optionally wrap the onboardEmail call inside
sendMail with try/catch to log or surface failures (use activeUser/decodedInfo
checks as present before invoking). Ensure all places that call sendMail
(including the other occurrence) handle the returned promise to prevent
unhandled rejections.
- Around line 36-278: Add unit and integration tests for the OnboardCreating
component covering account creation and RC delegation: mount OnboardCreating and
assert UI flows for openSignModal and modalStep transitions (sign →
success/failed), simulate clicks that trigger onCreateAccount and mock
useCreateAccountMutation to test success and error branches, mock
useDelegateRcMutation to verify delegateRc is called when isChecked and rcError
is empty, test rcOperationsCost by mocking queryClient.fetchQuery
(getRcStatsQueryOptions) to assert comment/vote/transfer/customJson amounts and
rcError behavior, and include form/state tests for rcAmount checkbox,
disabled/enabled buttons (create with hive vs credit and accountCredit gating)
plus error handling paths for both createAccount and delegateRc.
- Around line 72-73: Replace the loosly-typed usages with concrete types: change
the fetched rcStats declaration to use RcStats (e.g., const rcStats: RcStats =
await queryClient.fetchQuery(getRcStatsQueryOptions());) to match the imported
type and access ops safely; update the two catch handlers that use err: any to
err: unknown (and pass err to formatError(err) which accepts any); and change
the input handler signature from (e: any) => to (e:
React.ChangeEvent<HTMLInputElement>) => in the onChange prop so the event is
strongly typed. Ensure you update the variable/type names referenced here
(rcStats, getRcStatsQueryOptions, formatError, and the onChange handler) where
they appear in the file.
In `@apps/web/src/app/signup/free/_page.tsx`:
- Around line 98-107: The referral segment check passed to
referral.split(".").some currently doesn't return a boolean, preventing
short-circuiting; update the some() callback so it returns true when an invalid
segment is found (item.length < 3) and false otherwise so the iteration stops on
first error, and keep using setReferralError and setIsDisabled inside that
branch (ensure the callback returns false when segment is valid).
- Around line 262-266: Replace the non-semantic anchor used for toggling login
with a keyboard-focusable button: change the <a> element that calls
toggleUIProp("login") to a <button type="button"> while preserving its className
("pl-1 cursor-pointer") and the onClick handler (toggleUIProp("login")); ensure
any visual styles remain the same and no href is used so semantics and keyboard
access are correct.
- Around line 167-171: The captchaCheck handler leaves isVerified true when the
widget clears/expires because it only sets setIsVerified(true) for non-null
values; update captchaCheck (the onChange handler) to set setIsVerified(false)
when value is null (e.g., setIsVerified(Boolean(value)) or explicitly set false
in the else branch) and also clear any stored captcha token state if you track
one so the submit button is disabled after expiry.
In `@apps/web/src/app/signup/invited/page.tsx`:
- Around line 99-111: The route fails to validate email format: canProceed only
ensures email is non-empty so handleContinue can advance with invalid emails;
update handleContinue to validate the email (e.g., use a regex or HTML5 email
validation via new URLSearchParams or a helper like isValidEmail) before calling
setShowKeys(true) and set an email error (setEmailError) when invalid, or
alternatively wrap the inputs in a form and call checkValidity() prior to
proceeding; ensure the change references canProceed, handleContinue,
setShowKeys, setEmailError, email, and getAccountsQueryOptions (which still runs
the existing username existence check).
- Around line 31-33: Reset fileIsDownloaded whenever the derived seed for the
current username changes: add an effect that watches debouncedUsername (or
seedPhrase if that variable exists) and calls setFileIsDownloaded(false) so the
downloaded flag always reflects the current seed; additionally, prevent
advancing to or rendering the "share key" step until the new seedPhrase has
finished loading (e.g., gate the step transition/visibility on seedPhrase being
non-empty or a loaded flag), updating any step-control logic that uses
fileIsDownloaded/currentStep to check the freshly-loaded seed before allowing
download/share.
- Around line 81-97: The shareLink logic in the useMemo (shareLink, b64uEnc,
accountKeys, username, email) embeds raw email and pubkeys in the URL which
leaks PII and is mutable; change this to request an opaque server-side invite
token: send the invite payload (username, email, pubkeys) to a backend endpoint
(e.g., createInviteToken) that stores or returns a signed/encrypted token, then
build the link using only that token (e.g., /signup/invited/{token}); remove
direct serialization with b64uEnc and ensure server-side validation/signing so
clients cannot tamper with pubkeys.
In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx`:
- Around line 147-528: The PR is missing unit/integration tests for the
MetamaskConnect component covering address discovery, balance validation,
duplicate-address checks and signing; add a new spec file co-located with the
component (MetamaskConnect.spec.tsx) using the test utilities from
src/specs/test-utils.tsx and mock the key interactions: simulate window.ethereum
and eth_requestAccounts to exercise connectMetaMask and
fetchMultichainAddresses, mock useGetExternalWalletBalanceQuery /
getTokenPriceQueryOptions to test the $1 USD MINIMUM_VALIDATION_USD branch and
both valid/invalid balances, stub
EcencyWalletsPrivateApi.useCheckWalletExistence to test addressAlreadyUsed
behavior, and spy on the signAndVerify flow (personal_sign) to assert onVerified
is called with correct args; include tests for UI states (loading spinners,
error messages via i18next keys) and enable/disable of the Sign button.
- Around line 275-291: In signAndVerify: capture the returned signature from
window.ethereum.request("personal_sign") and pass it to onVerified so the server
can validate ownership with a nonce; ensure the signingAddress is the actual
address for the selected currency (use the selected chain address rather than
always evmAddress for BTC/SOL cases), store the signature together with
selectedCurrency and the specific address, and ensure setIsSigning is cleared in
finally; finally update onVerified(selectedCurrency, address,
availableAddresses, signature) (or equivalent payload) so server-side
verification can bind signature → currency → address.
In `@apps/web/src/features/shared/purchase-qr/purchase-qr-builder.tsx`:
- Around line 32-38: Add unit tests for the PurchaseQrBuilder component to cover
the prop→state sync implemented in the useEffect: verify that when propUsername
is provided on initial render the component sets internal username (check
rendered output or exposed state), that updating propUsername causes username to
update (re-rendering accordingly), and that clearing propUsername resets
visibility/state (e.g., hides QR and resets username/related state). Target the
component PurchaseQrBuilder and exercise the useEffect-driven logic tied to
propUsername / username / setUsername by rendering with a test renderer (RTL),
using rerender to change props, and asserting the expected DOM/state
transitions.
- Around line 33-37: The useEffect currently ignores falsy propUsername values
and therefore never syncs cleared usernames; update the effect in
purchase-qr-builder.tsx so it does not gate on truthiness—use a strict
comparison (propUsername !== username) and call setUsername(propUsername) when
they differ, and include both propUsername and username in the effect dependency
array to avoid a stale-closure issue; reference the existing useEffect and the
setUsername state updater to locate and change the logic.
---
Minor comments:
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx:
- Around line 173-177: The placeholder string for the RC input is hard-coded;
add a new key (e.g. "onboard.delegatePlaceholder") to en-US.json with value
"Enter amount to delegate (Bn)" and replace the literal in the FormControl
placeholder prop with i18next.t('onboard.delegatePlaceholder'); update imports
if needed, keeping the existing FormControl, rcAmount and setRcAmount usage
intact so only the placeholder is localized.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_page.tsx:
- Around line 19-20: The redirect builds a path using the raw slug value (slugs,
hash) which can break when the slug contains reserved URL characters; update the
router.replace call that uses hash (router.replace(hash ?
`/signup/invited/${hash}` : "/signup/invited")) to encode the hash with
encodeURIComponent(hash) before interpolating it into the path so the resulting
URL is safe and well-formed.
In `@apps/web/src/app/signup/_components/signup-layout-client.tsx`:
- Around line 57-61: Remove the inline fallback string from the i18n call so new
user-facing text lives only in locale JSON: change the component rendering that
uses isSubPage and the Link (rendering UilArrowLeft and calling
i18next.t("sign-up.all-options", { defaultValue: "All signup options" })) to
call i18next.t("sign-up.all-options") with no defaultValue, and add the
"sign-up.all-options" entry to en-US.json instead.
In `@apps/web/src/app/signup/free/_page.tsx`:
- Around line 68-81: The debounced username availability check in useDebounce
can apply results to a newer input; fix it by snapshotting the username being
queried (e.g., const queried = username) before calling
queryClient.fetchQuery(getAccountsQueryOptions([username])) and, in the then
handler, compare queried === username (or use a ref like lastQueriedUsername set
before the fetch) and only call
setUsernameError(i18next.t("sign-up.username-exists")) and setIsDisabled(true)
when they match; also ensure you clear the error/disabled state when the current
username is different or valid inside the same check flow.
In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx:
- Around line 93-99: The useEffect watching isChecked and rcAmount causes
rcOperationsCost() to fire on every rcAmount change (keystrokes), leading to
repeated fetchQuery calls; update the effect to only depend on isChecked (remove
rcAmount from the dependency array) and trigger rcOperationsCost() from a
debounced handler or on explicit events (e.g., onBlur or form submit) instead;
locate the useEffect and the rcOperationsCost function and replace the direct
rcAmount dependency with either a debounced call tied to the rcAmount input or
move the fetch logic to an explicit handler to avoid firing fetchQuery on every
keystroke.
In `@apps/web/src/app/signup/premium/_page.tsx`:
- Around line 244-257: Add rel="noopener noreferrer" to both external anchor
elements that use target="_blank" (the anchors linking to
"https://ios.ecency.com" and "https://android.ecency.com" which wrap {appleSvg}
and {googleSvg}) to prevent window.opener access; update those two <a> elements
to include rel="noopener noreferrer".
- Around line 99-104: The callback passed to referral.split(".").some(...) never
returns a value and the .some() result is unused; change this to iterate
properly — either replace .some(...) with .forEach(...) so each segment is
checked and you call setReferralError(...) and setIsDisabled(true) when
item.length < 3, or use .find(...) / .some(...) but ensure the callback returns
true on the short-item condition and capture the boolean result if needed;
update the code around referral.split(".") and keep setReferralError and
setIsDisabled calls as-is when the condition is met.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`:
- Around line 273-281: The retry button currently sets hasInitiatedRef.current =
false and setStatus("installing-snap") but the effect that performs the install
uses dependencies [username, verifiedWallet, validateAccountIsCreated,
recordActivity] so it won't re-run; add a retry counter state (e.g., retryCount
via useState) and include it in the effect's dependency array, then increment
retryCount in the Button onClick (alongside resetting hasInitiatedRef and
setStatus) so the useEffect observing retryCount will re-trigger the
installation logic in the component (refer to hasInitiatedRef, setStatus, and
the useEffect that depends on username, verifiedWallet,
validateAccountIsCreated, recordActivity).
In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx`:
- Around line 430-467: The component renders untranslated literals ("Checking
MetaMask multichain addresses...", "address:", "Balance:") — add new locale keys
to en-US.json (e.g., signup-wallets.metamask.checkingMultichain,
signup-wallets.metamask.addressLabel, signup-wallets.balance.label) and replace
the raw strings in metamask-connect.tsx with i18next.t(...) calls (use existing
i18next import and the surrounding context where selectedCurrency,
connectedAddress, tokenAmount, CURRENCIES_META_DATA, Spinner are used) so all
new text is pulled from the locale file.
In `@apps/web/src/features/shared/login/hooks/use-login-by-metamask.ts`:
- Around line 20-25: The helpers (ensureHiveSnap, getHivePublicKeys,
signBufferWithSnap) currently use window.ethereum! and can throw if called when
MetaMask is absent; add a defensive runtime check at the top of each function
that verifies window is defined and window.ethereum exists (e.g., if (typeof
window === "undefined" || !window.ethereum) throw new Error("MetaMask/ethereum
provider not available")), replace the non‑null assertion usage with safe access
(window.ethereum.request(...)), and ensure the functions either throw a clear
Error or return a rejected Promise when the provider is missing so callers can
handle it.
---
Nitpick comments:
In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx:
- Around line 136-142: The call to onboardEmail(newAccountName,
decodedInfo.email, activeUser.username) is un-awaited so failures are ignored;
change it to await the promise and handle failures by wrapping the onboardEmail
call in a try/catch (inside the same surrounding try or a nested one) and on
error log the failure (using error(...) or process logger),
setModalStep("failed") and return or otherwise surface the failure so the user
is notified; reference the onboardEmail call and the existing catch block that
currently calls setModalStep("failed") and error(...formatError(err)).
In `@apps/web/src/app/signup/premium/_page.tsx`:
- Line 39: The form ref is currently untyped (useRef<any>()); change it to a
proper HTMLFormElement ref by initializing useRef<HTMLFormElement | null>(null)
and update any usages to guard with form.current (e.g., form.current?.submit(),
form.current?.reset(), or access elements via form.current.elements) so
TypeScript can check DOM form operations; look for the variable named form
created via useRef and update its type and initial value accordingly.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`:
- Around line 254-258: Replace the hardcoded button labels in the
MetamaskAccountCreating component (the Link/Button block that renders
{params?.has("backUri") ? "Back to origin" : "Explore Ecency"}) with i18n
lookups; add two new keys (e.g. metamask.backToOrigin and
metamask.exploreEcency) to en-US.json and use the app's translation hook/utility
(the same pattern used elsewhere in this component) to render
t('metamask.backToOrigin') and t('metamask.exploreEcency') instead of the
literal strings so the UI reads from the locale file.
- Around line 111-134: The current use of "{} as any" when invoking
accountQueryOptions.queryFn inside validateAccountIsCreated bypasses TypeScript
safety; replace this with a properly-typed call by either invoking the query
function with the expected QueryFunctionContext (an object containing the
queryKey used in getAccountFullQueryOptions) or, simpler and safer, use the
typed QueryClient helper (e.g., queryClient.fetchQuery or fetchQuery with
accountQueryOptions) so you don't need to cast to any; update
validateAccountIsCreated to call accountQueryOptions.queryFn with the correct
shape or delegate to queryClient.fetchQuery and keep the existing setQueryData
call (queryClient.setQueryData) to cache the result.
- Around line 63-98: The createAccountWithWallets function currently returns the
raw fetch Response which hides backend error messages; update
createAccountWithWallets to check the Response.ok status, parse the response
body (preferably JSON, falling back to text) when !ok to extract backend error
details, and then throw or return a rejected Error containing the parsed message
and status so callers can surface useful validation/duplicate errors; keep the
request construction (username, token, meta) the same and perform this
response-handling right after the fetch call in createAccountWithWallets.
In `@apps/web/src/app/signup/wallet/_page.tsx`:
- Around line 13-17: The VerifiedWallet interface is duplicated (defined here
and in metamask-account-creating.tsx); extract VerifiedWallet into a shared
types module (e.g., _types.ts) and export it, then replace the local interface
declarations in both this file and metamask-account-creating.tsx with an import
of the shared VerifiedWallet type to remove duplication and keep types DRY.
In `@apps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsx`:
- Around line 139-156: Replace the hardcoded "MetaMask" button label with a
localized string: use the i18n translation function (e.g., t or useTranslation)
where the Button rendered under isMetaMaskUser and onClick={handleMetaMask}
reads its label from a key like "key-or-hot.with-metamask" instead of the
literal; then add "key-or-hot.with-metamask": "MetaMask" to en-US.json so the
locale file contains the new entry.
In `@apps/web/src/features/shared/login/login.tsx`:
- Around line 206-228: Replace the hardcoded button label "MetaMask" with an
i18n lookup using the project's i18next hook (e.g., useTranslation) and the
Button in this component (the JSX inside the conditional rendering where Button
with props and icon is rendered); update the component to call
t('login.metamask') (or similar key) and add that key/value to en-US.json only.
Ensure you import/use useTranslation in this file and keep the button behavior
(onClick -> loginByMetaMask, disabled -> !username || isLoginByMetaMaskPending,
isLoading -> isLoginByMetaMaskPending) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a52ae43f-d205-4ef4-8ba4-fce92c182866
⛔ Files ignored due to path filters (2)
apps/web/public/assets/metamask-fox.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
apps/web/package.jsonapps/web/public/sw.jsapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsxapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsxapps/web/src/app/onboard-friend/[...slugs]/_page.scssapps/web/src/app/onboard-friend/[...slugs]/_page.tsxapps/web/src/app/purchase/_page.tsxapps/web/src/app/signup/_components/signup-layout-client.tsxapps/web/src/app/signup/email/_page.tsxapps/web/src/app/signup/email/_sign-up.scssapps/web/src/app/signup/email/page.tsxapps/web/src/app/signup/free/_page.tsxapps/web/src/app/signup/free/page.tsxapps/web/src/app/signup/invited/[hash]/_page.tsxapps/web/src/app/signup/invited/[hash]/page.tsxapps/web/src/app/signup/invited/page.tsxapps/web/src/app/signup/page.tsxapps/web/src/app/signup/premium/_page.tsxapps/web/src/app/signup/premium/page.tsxapps/web/src/app/signup/wallet/_components/index.tsapps/web/src/app/signup/wallet/_components/signup-wallet-logo.tsxapps/web/src/app/signup/wallet/_components/signup-wallet-stepper.tsxapps/web/src/app/signup/wallet/_components/steps/index.tsapps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsxapps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-account-creating.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-choose-username.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet-import.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet-item.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-intro.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-private-key-field.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-validate-funds.tsxapps/web/src/app/signup/wallet/_components/steps/signup-wallet-validation-selected.tsxapps/web/src/app/signup/wallet/_page.tsxapps/web/src/app/signup/wallet/page.tsxapps/web/src/app/signup/wallet/types/index.tsapps/web/src/app/signup/wallet/types/signup-external-wallet-information.tsapps/web/src/entities/users.tsapps/web/src/features/i18n/locales/en-US.jsonapps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsxapps/web/src/features/shared/login/hooks/index.tsapps/web/src/features/shared/login/hooks/use-login-by-metamask.tsapps/web/src/features/shared/login/hooks/use-login-in-app.tsapps/web/src/features/shared/login/login.tsxapps/web/src/features/shared/navbar/search/index.tsxapps/web/src/features/shared/purchase-qr/purchase-qr-builder.tsxapps/web/src/providers/sdk/web-broadcast-adapter.ts
💤 Files with no reviewable changes (14)
- apps/web/src/app/signup/wallet/types/index.ts
- apps/web/src/app/onboard-friend/[...slugs]/_page.scss
- apps/web/src/app/signup/wallet/types/signup-external-wallet-information.ts
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet-import.tsx
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet.tsx
- apps/web/src/app/signup/wallet/_components/signup-wallet-logo.tsx
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-connect-wallet-item.tsx
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-private-key-field.tsx
- apps/web/src/app/signup/email/_page.tsx
- apps/web/src/app/signup/wallet/_components/index.ts
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-account-creating.tsx
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-validation-selected.tsx
- apps/web/src/app/signup/wallet/_components/steps/signup-wallet-validate-funds.tsx
- apps/web/src/app/signup/email/_sign-up.scss
| "@tiptap/suggestion": "^2.11.7", | ||
| "@tooni/iconscout-unicons-react": "^1.0.1", | ||
| "@vis.gl/react-google-maps": "^1.5.3", | ||
| "@wallet-standard/app": "^1.1.0", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `@wallet-standard/app` exists in both app manifests and compare ranges.
python - <<'PY'
import json
from pathlib import Path
files = ["apps/web/package.json", "apps/self-hosted/package.json"]
dep = "@wallet-standard/app"
for f in files:
p = Path(f)
if not p.exists():
print(f"{f}: MISSING FILE")
continue
data = json.loads(p.read_text())
val = (data.get("dependencies") or {}).get(dep)
print(f"{f}: {dep} = {val}")
PYRepository: ecency/vision-next
Length of output: 173
Add @wallet-standard/app to apps/self-hosted/package.json to maintain dependency parity (Line 67).
The @wallet-standard/app dependency was added to apps/web/package.json but is missing from apps/self-hosted/package.json. Update the self-hosted app manifest with "@wallet-standard/app": "^1.1.0" to maintain consistency and prevent runtime drift between deployments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/package.json` at line 67, Add the missing dependency entry for
"@wallet-standard/app": "^1.1.0" to the self-hosted package.json to match the
web app manifest; open the self-hosted package.json and append or merge the
dependency under the "dependencies" object ensuring the exact version string
"@wallet-standard/app": "^1.1.0" is present so both manifests remain in parity.
Comment on lines
+30
to
+32
| const { data: seedPhrase = "", refetch: refetchSeed } = useSeedPhrase(decodedInfo.username); | ||
| const [fileIsDownloaded, setFileIsDownloaded] = useState(false); | ||
| const [secret, setSecret] = useState(""); |
Contributor
There was a problem hiding this comment.
Keep the "downloaded" state tied to the current seed phrase.
fileIsDownloaded is set once and never reset when a new seed is generated, so the visible invite link can update to new pubkeys while the downloaded file still contains the old backup. The button is also enabled while seedPhrase can still be empty. Reset the flag whenever the seed changes, and only mark it downloaded after exporting a non-empty current seed.
Also applies to: 38-38, 67-73, 119-123, 140-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx
around lines 30 - 32, The fileIsDownloaded boolean is not tied to the current
seed phrase so it can remain true when a new seed is generated; update the
component to reset that flag whenever the seed changes and only set it true
after a successful export of a non-empty seed. Specifically, add an effect that
watches seedPhrase (from useSeedPhrase) and calls setFileIsDownloaded(false)
whenever seedPhrase changes, and in the download/export handler (where you call
the export logic and set secret) only call setFileIsDownloaded(true) after
verifying seedPhrase is non-empty (and after the export completes). Also disable
the download button while seedPhrase is empty to prevent exporting an empty
backup.
Comment on lines
+60
to
+67
| const dataToEncode = { | ||
| username: decodedInfo.username, | ||
| email: decodedInfo.email, | ||
| referral: decodedInfo.referral ?? "", | ||
| pubkeys | ||
| }; | ||
|
|
||
| setSecret(b64uEnc(JSON.stringify(dataToEncode))); |
Contributor
There was a problem hiding this comment.
Don't put an unsigned invite payload in the URL.
b64uEnc(JSON.stringify(dataToEncode)) only obscures the data. The invitee email is recoverable from logs/history/referrers, and anyone who edits the link can change username, referral, or pubkeys before account creation. Use an opaque server-issued token or a signed/encrypted payload instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx
around lines 60 - 67, The current flow builds an unsigned invite object
(dataToEncode) and places b64uEnc(JSON.stringify(dataToEncode)) into the URL via
setSecret, which only obscures but does not protect or authenticate the payload;
instead, send the invite data (username, email, referral, pubkeys) to the
backend to mint an opaque server-issued token (or return a signed/encrypted
blob) and use that token in setSecret; update the client code around
dataToEncode / b64uEnc / setSecret to POST the payload to an endpoint that
returns a short-lived opaque token (or HMAC-signed/encrypted payload) and have
the server validate/sign the token during account creation so link tampering or
exposure cannot alter the invite fields.
Comment on lines
+36
to
+278
| export function OnboardCreating({ decodedInfo }: Props) { | ||
| const { activeUser, account } = useActiveAccount(); | ||
| const { data: dynamicProps } = useQuery(getDynamicPropsQueryOptions()); | ||
| const queryClient = useQueryClient(); | ||
| const { mutateAsync: createAccount, isPending: isCreatePending } = useCreateAccountMutation(); | ||
| const { mutateAsync: delegateRc } = useDelegateRcMutation(); | ||
|
|
||
| const [modalStep, setModalStep] = useState<ModalStep>(null); | ||
| const [createOption, setCreateOption] = useState<"hive" | "credit">("hive"); | ||
| const [isChecked, setChecked] = useState(false); | ||
| const [rcAmount, setRcAmount] = useState(0); | ||
| const [rcError, setRcError] = useState(""); | ||
| const [commentAmount, setCommentAmount] = useState(0); | ||
| const [voteAmount, setVoteAmount] = useState(0); | ||
| const [transferAmount, setTransferAmount] = useState(0); | ||
| const [customJsonAmount, setCustomJsonAmount] = useState(0); | ||
|
|
||
| const accountCredit = account?.pending_claimed_accounts ?? 0; | ||
|
|
||
| const confirmFields = [ | ||
| { label: i18next.t("onboard.username"), value: decodedInfo.username }, | ||
| { label: i18next.t("onboard.public-owner"), value: decodedInfo.pubkeys.ownerPublicKey }, | ||
| { label: i18next.t("onboard.public-active"), value: decodedInfo.pubkeys.activePublicKey }, | ||
| { label: i18next.t("onboard.public-posting"), value: decodedInfo.pubkeys.postingPublicKey }, | ||
| { label: i18next.t("onboard.public-memo"), value: decodedInfo.pubkeys.memoPublicKey } | ||
| ]; | ||
|
|
||
| useEffect(() => { | ||
| if (!isChecked) { | ||
| setRcAmount(0); | ||
| return; | ||
| } | ||
| rcOperationsCost(); | ||
| }, [isChecked, rcAmount]); | ||
|
|
||
| const rcOperationsCost = async () => { | ||
| const rcStats: any = await queryClient.fetchQuery(getRcStatsQueryOptions()); | ||
| const ops = rcStats.ops; | ||
|
|
||
| if (isNaN(rcAmount) || rcAmount * 1e9 < 5000000000) { | ||
| setRcError(i18next.t("onboard.rc-error")); | ||
| } else { | ||
| setRcError(""); | ||
| } | ||
|
|
||
| setCommentAmount(Math.ceil((rcAmount * 1e9) / ops.comment_operation.avg_cost)); | ||
| setVoteAmount(Math.ceil((rcAmount * 1e9) / ops.vote_operation.avg_cost)); | ||
| setTransferAmount(Math.ceil((rcAmount * 1e9) / ops.transfer_operation.avg_cost)); | ||
| setCustomJsonAmount(Math.ceil((rcAmount * 1e9) / ops.custom_json_operation.avg_cost)); | ||
| }; | ||
|
|
||
| const sendMail = async () => { | ||
| if (activeUser) { | ||
| await onboardEmail(decodedInfo.username, decodedInfo.email, activeUser.username); | ||
| } | ||
| }; | ||
|
|
||
| const onCreateAccount = async (type: "hive" | "credit") => { | ||
| if (!activeUser || !decodedInfo.pubkeys) return; | ||
|
|
||
| const useClaimed = type === "credit"; | ||
| const newAccountName = decodedInfo.username.trim().toLowerCase(); | ||
|
|
||
| try { | ||
| await createAccount({ | ||
| newAccountName, | ||
| keys: decodedInfo.pubkeys, | ||
| fee: (dynamicProps ?? DEFAULT_DYNAMIC_PROPS).accountCreationFee, | ||
| useClaimed | ||
| }); | ||
|
|
||
| setModalStep("success"); | ||
| sendMail(); | ||
| } catch (err: any) { | ||
| setModalStep("failed"); | ||
| error(...formatError(err)); | ||
| return; | ||
| } | ||
|
|
||
| if (isChecked && !rcError) { | ||
| try { | ||
| await delegateRc({ to: newAccountName, maxRc: rcAmount * 1e9 }); | ||
| } catch (err: any) { | ||
| error(...formatError(err)); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const openSignModal = (option: "hive" | "credit") => { | ||
| setCreateOption(option); | ||
| setModalStep("sign"); | ||
| }; | ||
|
|
||
| if (!activeUser) { | ||
| return ( | ||
| <div className="max-w-[600px] mx-auto text-center py-12"> | ||
| <p className="text-lg">{i18next.t("onboard.login-warning")}</p> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="max-w-[600px] w-full mx-auto"> | ||
| <div className="bg-white dark:bg-dark-200 rounded-2xl p-6 md:p-8"> | ||
| <h3 className="text-2xl font-semibold text-blue-dark-sky mb-6"> | ||
| {i18next.t("onboard.confirm-details")} | ||
| </h3> | ||
|
|
||
| {/* Account + key details */} | ||
| <div className="space-y-3 mb-6"> | ||
| {confirmFields.map((field, i) => ( | ||
| <div key={i}> | ||
| <span className="opacity-60 text-sm">{field.label}</span> | ||
| <strong className="block break-all text-sm">{field.value}</strong> | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* RC delegation */} | ||
| <div className="border-t border-gray-200 dark:border-gray-700 pt-4 mb-6"> | ||
| <label className="flex items-start gap-2 cursor-pointer"> | ||
| <input | ||
| type="checkbox" | ||
| className="w-5 h-5 mt-0.5 flex-shrink-0" | ||
| checked={isChecked} | ||
| onChange={() => setChecked(!isChecked)} | ||
| /> | ||
| <span className="text-sm text-orange-600 dark:text-orange-400"> | ||
| {i18next.t("onboard.rc-to-new-acc")} {decodedInfo.username}{" "} | ||
| {i18next.t("onboard.minimum-rc")} | ||
| </span> | ||
| </label> | ||
|
|
||
| {isChecked && ( | ||
| <div className="mt-3 pl-7"> | ||
| {rcError && <p className="text-red text-sm mb-2">{rcError}</p>} | ||
| <InputGroup> | ||
| <FormControl | ||
| type="text" | ||
| placeholder="Enter amount to delegate (Bn)" | ||
| value={rcAmount} | ||
| onChange={(e: any) => setRcAmount(Number(e.target.value))} | ||
| /> | ||
| </InputGroup> | ||
| <div className="flex flex-wrap gap-x-3 gap-y-1 mt-2 text-xs opacity-75"> | ||
| <span>{i18next.t("onboard.posts-comments")} {commentAmount}</span> | ||
| <span>{i18next.t("onboard.votes")} {voteAmount}</span> | ||
| <span>{i18next.t("onboard.transfers")} {transferAmount}</span> | ||
| <span>{i18next.t("onboard.reblogs-follows")} {customJsonAmount}</span> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Payment buttons */} | ||
| <div className="text-center"> | ||
| <p className="text-sm opacity-75 mb-3">{i18next.t("onboard.pay-fee")}</p> | ||
| <div className="flex justify-center gap-3 flex-wrap"> | ||
| <Button | ||
| disabled={isChecked && !!rcError} | ||
| onClick={() => openSignModal("hive")} | ||
| > | ||
| {i18next.t("onboard.create-account-hive")} | ||
| </Button> | ||
| <Button | ||
| disabled={accountCredit <= 0 || (isChecked && !!rcError)} | ||
| onClick={() => openSignModal("credit")} | ||
| > | ||
| {i18next.t("onboard.create-account-credit", { n: accountCredit })} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Sign / Success / Failed Modal */} | ||
| <Modal | ||
| show={!!modalStep} | ||
| centered={true} | ||
| onHide={() => setModalStep(null)} | ||
| size="lg" | ||
| > | ||
| <ModalHeader closeButton={true}> | ||
| <ModalTitle /> | ||
| </ModalHeader> | ||
| <ModalBody> | ||
| {modalStep === "sign" && ( | ||
| <div className="flex flex-col"> | ||
| <div className="border-b border-gray-200 dark:border-gray-700 pb-3 mb-4"> | ||
| <div className="font-semibold">{i18next.t("onboard.sign-header-title")}</div> | ||
| <div className="text-sm opacity-60">{i18next.t("onboard.sign-sub-title")}</div> | ||
| </div> | ||
| {isCreatePending && <LinearProgress />} | ||
| <div className="flex justify-center py-4"> | ||
| <Button | ||
| disabled={isCreatePending} | ||
| icon={isCreatePending ? <Spinner className="mr-1.5 w-3.5 h-3.5" /> : undefined} | ||
| iconPlacement="left" | ||
| onClick={() => onCreateAccount(createOption)} | ||
| > | ||
| {i18next.t("onboard.sign-header-title")} | ||
| </Button> | ||
| </div> | ||
| <p className="text-center"> | ||
| <a href="#" onClick={(e) => { e.preventDefault(); setModalStep(null); }}> | ||
| {i18next.t("g.back")} | ||
| </a> | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {modalStep === "success" && ( | ||
| <div className="text-center py-4"> | ||
| <p className="text-lg font-semibold text-green mb-2"> | ||
| {i18next.t("trx-common.success-title")} | ||
| </p> | ||
| <p> | ||
| {i18next.t("onboard.success-message")}{" "} | ||
| <strong>{decodedInfo.username}</strong> | ||
| </p> | ||
| <Link href={`/@${decodedInfo.username}`}> | ||
| <Button className="mt-4" onClick={() => setModalStep(null)}> | ||
| {i18next.t("g.finish")} | ||
| </Button> | ||
| </Link> | ||
| </div> | ||
| )} | ||
|
|
||
| {modalStep === "failed" && ( | ||
| <div className="text-center py-4"> | ||
| <p className="text-lg font-semibold text-red mb-2"> | ||
| {i18next.t("onboard.failed-title")} | ||
| </p> | ||
| <p className="text-red">{i18next.t("onboard.failed-message")}</p> | ||
| <Button className="mt-4" onClick={() => setModalStep(null)}> | ||
| {i18next.t("onboard.try-again")} | ||
| </Button> | ||
| </div> | ||
| )} | ||
| </ModalBody> | ||
| </Modal> | ||
| </div> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for tests covering onboard creating / invited signup flow
rg -nP --glob '**/*.{test,spec}.{ts,tsx}' 'OnboardCreating|onboard-friend|signup/invited|useCreateAccountMutation|useDelegateRcMutation'Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check for test files in the onboard-friend directory
find apps/web/src/app/onboard-friend -type f \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" \) 2>/dev/nullRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Look for test files in the broader app directory to understand testing patterns
find apps/web/src -type d -name '__tests__' -o -name '__test__' | head -20Repository: ecency/vision-next
Length of output: 237
🏁 Script executed:
# Search for useCreateAccountMutation and useDelegateRcMutation definitions to see if they have tests
rg -l 'useCreateAccountMutation|useDelegateRcMutation' --type ts --type tsx | grep -E '(test|spec|__tests__)' | head -20Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Check if there are any test files at all in the apps/web/src directory structure
find apps/web/src -type f \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" \) | wc -lRepository: ecency/vision-next
Length of output: 65
🏁 Script executed:
# Search all test files for "onboard" references
rg -i 'onboard' --type ts -g '**/*.test.ts' -g '**/*.test.tsx' -lRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check the directory structure of onboard-friend to see where tests should be
find apps/web/src/app/onboard-friend -type f -o -type d | sortRepository: ecency/vision-next
Length of output: 446
Add tests for the OnboardCreating component and related account creation flow.
This substantial feature path handles account creation, RC delegation, and multiple account creation methods but lacks test coverage. Per coding guidelines, all new features in @ecency/web require tests. Consider adding tests for:
- Account creation logic with
useCreateAccountMutation - RC delegation flow with
useDelegateRcMutation - Form validation and state management
- Modal interactions (sign, success, failed states)
- Error handling for both creation and delegation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx
around lines 36 - 278, Add unit and integration tests for the OnboardCreating
component covering account creation and RC delegation: mount OnboardCreating and
assert UI flows for openSignModal and modalStep transitions (sign →
success/failed), simulate clicks that trigger onCreateAccount and mock
useCreateAccountMutation to test success and error branches, mock
useDelegateRcMutation to verify delegateRc is called when isChecked and rcError
is empty, test rcOperationsCost by mocking queryClient.fetchQuery
(getRcStatsQueryOptions) to assert comment/vote/transfer/customJson amounts and
rcError behavior, and include form/state tests for rcAmount checkbox,
disabled/enabled buttons (create with hive vs credit and accountCredit gating)
plus error handling paths for both createAccount and delegateRc.
Comment on lines
+72
to
+73
| const rcStats: any = await queryClient.fetchQuery(getRcStatsQueryOptions()); | ||
| const ops = rcStats.ops; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify any-typed additions in this component
fd 'onboard-creating.tsx' | xargs rg -nP ':\s*any\b|\(e:\s*any\)'Repository: ecency/vision-next
Length of output: 286
🏁 Script executed:
cat -n apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsx | head -180 | tail -120Repository: ecency/vision-next
Length of output: 4740
🏁 Script executed:
head -60 apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsxRepository: ecency/vision-next
Length of output: 2649
🏁 Script executed:
rg 'getRcStatsQueryOptions|formatError' apps/web/src --type ts --type tsx -A 2 | head -50Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
rg 'getRcStatsQueryOptions|formatError' apps/web/src -A 2Repository: ecency/vision-next
Length of output: 40230
🏁 Script executed:
rg 'export.*formatError' apps/web/src -B 2 -A 3Repository: ecency/vision-next
Length of output: 798
🏁 Script executed:
# Check if there are type definitions in node_modules or in the SDK
rg 'getRcStatsQueryOptions' --type ts --type tsx -A 1 2>/dev/null || rg 'getRcStatsQueryOptions' -A 1Repository: ecency/vision-next
Length of output: 50376
🏁 Script executed:
rg 'type RcStats|interface RcStats' packages/sdk -A 5Repository: ecency/vision-next
Length of output: 17570
🏁 Script executed:
# Check React event types used elsewhere in the component file
rg 'React\.' apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsxRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Look for similar error handling patterns in the codebase
rg 'catch \(err: ' apps/web/src -A 1 | head -30Repository: ecency/vision-next
Length of output: 1868
Replace any with concrete types to maintain type safety in strict mode.
Four instances of any weaken type guarantees in a critical onboarding flow:
- Line 72:
rcStats: any→ UseRcStatsfrom@ecency/sdk(already imported) - Lines 109, 118:
err: anyin catch blocks → Useunknownper strict mode convention, thoughformatError()accepts any - Line 177:
onChange={(e: any) =>→ UseReact.ChangeEvent<HTMLInputElement>for the input element
Import RcStats is already available from the existing @ecency/sdk import if not present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx
around lines 72 - 73, Replace the loosly-typed usages with concrete types:
change the fetched rcStats declaration to use RcStats (e.g., const rcStats:
RcStats = await queryClient.fetchQuery(getRcStatsQueryOptions());) to match the
imported type and access ops safely; update the two catch handlers that use err:
any to err: unknown (and pass err to formatError(err) which accepts any); and
change the input handler signature from (e: any) => to (e:
React.ChangeEvent<HTMLInputElement>) => in the onChange prop so the event is
strongly typed. Ensure you update the variable/type names referenced here
(rcStats, getRcStatsQueryOptions, formatError, and the onChange handler) where
they appear in the file.
Comment on lines
+99
to
+111
| const canProceed = !usernameError && !emailError && username && email; | ||
|
|
||
| const handleContinue = useCallback(async () => { | ||
| if (!canProceed) return; | ||
|
|
||
| // Final check: username exists? | ||
| const existing = await queryClient.fetchQuery(getAccountsQueryOptions([username])); | ||
| if (existing.length > 0) { | ||
| setUsernameError(i18next.t("sign-up.username-exists")); | ||
| return; | ||
| } | ||
| setShowKeys(true); | ||
| }, [canProceed, username, queryClient]); |
Contributor
There was a problem hiding this comment.
This route never validates the email format.
canProceed only checks for a non-empty string, and unlike the free signup flow there is no form submission or checkValidity() call here. A value like "abc" can reach the generated invite link unchanged. Wrap these inputs in a form or add an explicit email validity check before setShowKeys(true).
Also applies to: 149-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/signup/invited/page.tsx` around lines 99 - 111, The route
fails to validate email format: canProceed only ensures email is non-empty so
handleContinue can advance with invalid emails; update handleContinue to
validate the email (e.g., use a regex or HTML5 email validation via new
URLSearchParams or a helper like isValidEmail) before calling setShowKeys(true)
and set an email error (setEmailError) when invalid, or alternatively wrap the
inputs in a form and call checkValidity() prior to proceeding; ensure the change
references canProceed, handleContinue, setShowKeys, setEmailError, email, and
getAccountsQueryOptions (which still runs the existing username existence
check).
Comment on lines
+147
to
+528
| export function MetamaskConnect({ username, onVerified, onBack }: Props) { | ||
| const [isConnecting, setIsConnecting] = useState(false); | ||
| const [evmAddress, setEvmAddress] = useState(""); | ||
| const [chainAddresses, setChainAddresses] = useState<WalletAddressMap>({}); | ||
| const [selectedCurrency, setSelectedCurrency] = useState<EcencyWalletCurrency | null>(null); | ||
| const [isSigning, setIsSigning] = useState(false); | ||
| const [isLoadingChainAddresses, setIsLoadingChainAddresses] = useState(false); | ||
| const [addressAlreadyUsed, setAddressAlreadyUsed] = useState(false); | ||
|
|
||
| const { mutateAsync: checkWalletExistence } = | ||
| EcencyWalletsPrivateApi.useCheckWalletExistence(); | ||
|
|
||
| const isEvmChain = selectedCurrency | ||
| ? SUPPORTED_CHAINS.find((c) => c.currency === selectedCurrency)?.isEvm ?? false | ||
| : false; | ||
|
|
||
| // The active address: EVM chains share the same address, non-EVM use fetched multichain address | ||
| const connectedAddress = isEvmChain | ||
| ? evmAddress | ||
| : (selectedCurrency ? chainAddresses[selectedCurrency] ?? "" : ""); | ||
|
|
||
| const availableAddresses = useMemo<WalletAddressMap>(() => { | ||
| const evmAddresses = evmAddress | ||
| ? { | ||
| [EcencyWalletCurrency.ETH]: evmAddress, | ||
| [EcencyWalletCurrency.BNB]: evmAddress | ||
| } | ||
| : {}; | ||
|
|
||
| return { | ||
| ...evmAddresses, | ||
| ...chainAddresses | ||
| }; | ||
| }, [evmAddress, chainAddresses]); | ||
|
|
||
| // Balance query — only enabled when we have both currency and address | ||
| const { | ||
| data: externalWalletBalance, | ||
| refetch: refetchBalance, | ||
| isLoading: isBalanceLoading | ||
| } = useGetExternalWalletBalanceQuery( | ||
| selectedCurrency ?? EcencyWalletCurrency.ETH, | ||
| connectedAddress | ||
| ); | ||
|
|
||
| const { | ||
| data: priceUsd, | ||
| isLoading: isPriceLoading, | ||
| isError: isPriceError | ||
| } = useQuery({ | ||
| ...getTokenPriceQueryOptions(selectedCurrency ?? EcencyWalletCurrency.ETH), | ||
| enabled: !!selectedCurrency && !!connectedAddress, | ||
| staleTime: 30000 | ||
| }); | ||
|
|
||
| // Poll balance every 10s when checking | ||
| useInterval( | ||
| () => { | ||
| if (selectedCurrency && connectedAddress) { | ||
| refetchBalance(); | ||
| } | ||
| }, | ||
| selectedCurrency && connectedAddress ? 10000 : null | ||
| ); | ||
|
|
||
| const decimals = useMemo(() => { | ||
| if (selectedCurrency && DECIMALS_BY_CURRENCY[selectedCurrency] !== undefined) { | ||
| return DECIMALS_BY_CURRENCY[selectedCurrency]!; | ||
| } | ||
| if (externalWalletBalance?.unit) { | ||
| return DECIMALS_BY_UNIT[externalWalletBalance.unit.toLowerCase()] ?? 0; | ||
| } | ||
| return 0; | ||
| }, [selectedCurrency, externalWalletBalance?.unit]); | ||
|
|
||
| const tokenAmount = useMemo(() => { | ||
| const balance = externalWalletBalance?.balanceBigInt ?? BigInt(0); | ||
| const divisor = new Decimal(10).pow(decimals); | ||
| return new Decimal(balance.toString()).div(divisor); | ||
| }, [decimals, externalWalletBalance?.balanceBigInt]); | ||
|
|
||
| const hasPrice = typeof priceUsd === "number" && Number.isFinite(priceUsd); | ||
| const usdValue = hasPrice ? tokenAmount.mul(priceUsd) : new Decimal(0); | ||
| const hasValidBalance = hasPrice && usdValue.greaterThanOrEqualTo(MINIMUM_VALIDATION_USD); | ||
|
|
||
| const connectMetaMask = useCallback(async () => { | ||
| if (!window.ethereum?.isMetaMask) { | ||
| error(i18next.t("signup-wallets.metamask.not-found")); | ||
| return; | ||
| } | ||
|
|
||
| setIsConnecting(true); | ||
| try { | ||
| setChainAddresses({}); | ||
| setAddressAlreadyUsed(false); | ||
|
|
||
| const accounts = await window.ethereum.request({ | ||
| method: "eth_requestAccounts" | ||
| }); | ||
| if (accounts?.[0]) { | ||
| setEvmAddress(accounts[0]); | ||
| setSelectedCurrency(EcencyWalletCurrency.ETH); | ||
|
|
||
| // Also fetch non-EVM addresses (BTC, SOL) via multichain API | ||
| setIsLoadingChainAddresses(true); | ||
| try { | ||
| const multichain = await fetchMultichainAddresses(); | ||
| if (process.env.NODE_ENV === "development") { | ||
| console.log("[MetaMask multichain] fetched addresses:", multichain); | ||
| } | ||
| setChainAddresses(multichain); | ||
| } finally { | ||
| setIsLoadingChainAddresses(false); | ||
| } | ||
| } | ||
| } catch { | ||
| error(i18next.t("signup-wallets.metamask.connect-error")); | ||
| setIsLoadingChainAddresses(false); | ||
| } finally { | ||
| setIsConnecting(false); | ||
| } | ||
| }, []); | ||
|
|
||
| const selectChain = useCallback((currency: EcencyWalletCurrency) => { | ||
| setSelectedCurrency(currency); | ||
| setAddressAlreadyUsed(false); | ||
| }, []); | ||
|
|
||
| const signAndVerify = useCallback(async () => { | ||
| if (!window.ethereum || !connectedAddress || !selectedCurrency) return; | ||
|
|
||
| setIsSigning(true); | ||
| try { | ||
| // For EVM chains, sign with MetaMask directly | ||
| // For non-EVM chains, we still use MetaMask's personal_sign with the EVM address | ||
| // to prove the user controls this MetaMask wallet | ||
| const signingAddress = evmAddress || connectedAddress; | ||
| const message = `Create Hive account: ${username}`; | ||
| await window.ethereum.request({ | ||
| method: "personal_sign", | ||
| params: [message, signingAddress] | ||
| }); | ||
|
|
||
| success(i18next.t("signup-wallets.validate-funds.validation-success")); | ||
| onVerified(selectedCurrency, connectedAddress, availableAddresses); | ||
| } catch { | ||
| error(i18next.t("signup-wallets.metamask.sign-rejected")); | ||
| } finally { | ||
| setIsSigning(false); | ||
| } | ||
| }, [availableAddresses, connectedAddress, evmAddress, selectedCurrency, username, onVerified]); | ||
|
|
||
| // Auto-check balance when address + chain are ready | ||
| useEffect(() => { | ||
| if (connectedAddress && selectedCurrency) { | ||
| refetchBalance(); | ||
| } | ||
| }, [connectedAddress, selectedCurrency, refetchBalance]); | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
|
|
||
| if (!selectedCurrency || !connectedAddress) { | ||
| setAddressAlreadyUsed(false); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| } | ||
|
|
||
| setAddressAlreadyUsed(false); | ||
|
|
||
| void (async () => { | ||
| try { | ||
| const isAvailable = await checkWalletExistence({ | ||
| address: connectedAddress, | ||
| currency: selectedCurrency | ||
| }); | ||
|
|
||
| if (!cancelled && !isAvailable) { | ||
| setAddressAlreadyUsed(true); | ||
| error(i18next.t("signup-wallets.metamask.address-in-use")); | ||
| } | ||
| } catch { | ||
| // If check fails, allow continuing — backend will catch duplicates | ||
| } | ||
| })(); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [checkWalletExistence, connectedAddress, selectedCurrency]); | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-6 w-full"> | ||
| <div> | ||
| <div className="text-lg font-semibold"> | ||
| {i18next.t("signup-wallets.metamask.title")} | ||
| </div> | ||
| <div className="opacity-50"> | ||
| {i18next.t("signup-wallets.metamask.description")} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Step 1: Connect MetaMask */} | ||
| {!evmAddress && ( | ||
| <div className="flex flex-col items-center gap-4 py-8"> | ||
| <Image | ||
| src="/assets/undraw-crypto-wallet.svg" | ||
| alt="" | ||
| width={200} | ||
| height={200} | ||
| className="max-w-[180px] mb-4" | ||
| /> | ||
| <Button | ||
| size="lg" | ||
| onClick={connectMetaMask} | ||
| disabled={isConnecting} | ||
| icon={isConnecting ? <Spinner className="w-4 h-4" /> : undefined} | ||
| > | ||
| {i18next.t("signup-wallets.metamask.connect-button")} | ||
| </Button> | ||
| <p className="text-sm opacity-50 text-center max-w-[400px]"> | ||
| {i18next.t("signup-wallets.metamask.connect-hint")} | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Step 2: Select chain + verify balance */} | ||
| {evmAddress && ( | ||
| <> | ||
| <div className="bg-green-050 dark:bg-green-900/20 rounded-xl p-4 flex items-center gap-3"> | ||
| <UilCheckCircle className="text-green w-5 h-5 flex-shrink-0" /> | ||
| <div> | ||
| <div className="text-sm font-medium"> | ||
| {i18next.t("signup-wallets.metamask.connected")} | ||
| </div> | ||
| <div className="text-xs opacity-75 font-mono truncate"> | ||
| {evmAddress} | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="grid grid-cols-2 sm:grid-cols-3 gap-3"> | ||
| {SUPPORTED_CHAINS.map(({ currency, comingSoon }) => { | ||
| const meta = CURRENCIES_META_DATA[currency]; | ||
| if (!meta) return null; | ||
| const isSelected = selectedCurrency === currency; | ||
|
|
||
| return ( | ||
| <button | ||
| key={currency} | ||
| onClick={() => !comingSoon && selectChain(currency)} | ||
| disabled={comingSoon} | ||
| className={`flex items-center gap-3 p-3 rounded-xl border-2 transition-all relative ${ | ||
| comingSoon | ||
| ? "border-gray-200 dark:border-gray-700 opacity-50 cursor-not-allowed" | ||
| : isSelected | ||
| ? "border-blue-dark-sky bg-blue-dark-sky-040 dark:bg-blue-dark-sky/20 cursor-pointer" | ||
| : "border-gray-200 dark:border-gray-700 hover:border-blue-dark-sky/50 cursor-pointer" | ||
| }`} | ||
| > | ||
| <Image | ||
| src={meta.icon} | ||
| alt={meta.title} | ||
| width={32} | ||
| height={32} | ||
| className="w-8 h-8" | ||
| /> | ||
| <div className="text-left"> | ||
| <div className="font-semibold text-sm">{meta.title}</div> | ||
| <div className="text-xs opacity-60">{meta.name}</div> | ||
| </div> | ||
| {comingSoon && ( | ||
| <span className="absolute top-1 right-1 text-[10px] font-bold uppercase bg-gray-200 dark:bg-gray-700 rounded px-1.5 py-0.5"> | ||
| {i18next.t("g.soon", { defaultValue: "Soon" })} | ||
| </span> | ||
| )} | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
|
|
||
| {/* Non-EVM chain address from MetaMask */} | ||
| {selectedCurrency && !isEvmChain && connectedAddress && ( | ||
| <div className="bg-gray-50 dark:bg-dark-default rounded-xl p-3 text-sm"> | ||
| <span className="opacity-60"> | ||
| {CURRENCIES_META_DATA[selectedCurrency]?.title} address:{" "} | ||
| </span> | ||
| <span className="font-mono break-all">{connectedAddress}</span> | ||
| </div> | ||
| )} | ||
| {selectedCurrency && !isEvmChain && isLoadingChainAddresses && ( | ||
| <div className="bg-gray-50 dark:bg-dark-default rounded-xl p-3 text-sm flex items-center gap-2"> | ||
| <Spinner className="w-4 h-4" /> | ||
| <span className="opacity-75">Checking MetaMask multichain addresses...</span> | ||
| </div> | ||
| )} | ||
| {selectedCurrency && !isEvmChain && !connectedAddress && !isLoadingChainAddresses && ( | ||
| <div className="text-sm text-orange-500"> | ||
| {i18next.t("signup-wallets.metamask.chain-not-available", { | ||
| chain: CURRENCIES_META_DATA[selectedCurrency]?.title ?? "", | ||
| defaultValue: "{{chain}} address not found in your MetaMask wallet. Please enable this chain in MetaMask and reconnect." | ||
| })} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Balance info */} | ||
| {selectedCurrency && connectedAddress && !addressAlreadyUsed && ( | ||
| <div className="bg-gray-50 dark:bg-dark-default rounded-xl p-4 space-y-2"> | ||
| {(isBalanceLoading || isPriceLoading) && ( | ||
| <div className="flex items-center gap-2 text-sm opacity-75"> | ||
| <Spinner className="w-4 h-4" /> | ||
| {i18next.t("signup-wallets.validate-funds.checking-balance")} | ||
| </div> | ||
| )} | ||
|
|
||
| {!isBalanceLoading && tokenAmount.greaterThan(0) && ( | ||
| <div className="text-sm"> | ||
| <span className="opacity-75">Balance: </span> | ||
| <span className="font-semibold"> | ||
| {tokenAmount.toFixed(4)} {CURRENCIES_META_DATA[selectedCurrency]?.name} | ||
| </span> | ||
| </div> | ||
| )} | ||
|
|
||
| {hasPrice && ( | ||
| <div className="text-sm"> | ||
| <span className="opacity-75"> | ||
| {i18next.t("signup-wallets.validate-funds.estimated-value", { | ||
| value: usdValue.toFixed(2) | ||
| })} | ||
| </span> | ||
| </div> | ||
| )} | ||
|
|
||
| {!isBalanceLoading && !isPriceLoading && !hasValidBalance && ( | ||
| <div className="text-sm text-orange-500"> | ||
| {i18next.t("signup-wallets.validate-funds.minimum-required", { | ||
| minimum: "$1.00" | ||
| })} | ||
| </div> | ||
| )} | ||
|
|
||
| {isPriceError && ( | ||
| <div className="text-sm text-orange-500"> | ||
| {i18next.t("signup-wallets.validate-funds.price-unavailable")} | ||
| </div> | ||
| )} | ||
|
|
||
| {hasValidBalance && ( | ||
| <div className="flex items-center gap-2 text-green text-sm font-medium"> | ||
| <UilCheckCircle className="w-4 h-4" /> | ||
| {i18next.t("signup-wallets.validate-funds.validation-success")} | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
| {addressAlreadyUsed && ( | ||
| <div className="bg-red-50 dark:bg-red-900/20 rounded-xl p-4 text-sm text-red"> | ||
| {i18next.t("signup-wallets.metamask.address-in-use")} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Sign & Continue */} | ||
| <div className="flex items-center justify-between"> | ||
| <Button appearance="gray" onClick={onBack} icon={<UilArrowLeft />} iconPlacement="left"> | ||
| {i18next.t("g.back")} | ||
| </Button> | ||
| <Button | ||
| onClick={signAndVerify} | ||
| disabled={!hasValidBalance || addressAlreadyUsed || isSigning} | ||
| icon={isSigning ? <Spinner className="w-4 h-4" /> : undefined} | ||
| > | ||
| {i18next.t("signup-wallets.metamask.sign-and-continue")} | ||
| </Button> | ||
| </div> | ||
| </> | ||
| )} | ||
| </div> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether this signup step already has focused tests in the repo.
fd '(\.test|\.spec)\.(ts|tsx)$' apps/web/src/app/signup/wallet
rg -nP 'MetamaskConnect|fetchMultichainAddresses|signAndVerify|signup-wallets\.metamask' \
apps/web \
-g '*.test.ts' -g '*.test.tsx' -g '*.spec.ts' -g '*.spec.tsx' \
-C 2Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Search for metamask-connect test file in expected locations
find apps/web -name '*metamask-connect*' -type fRepository: ecency/vision-next
Length of output: 131
🏁 Script executed:
# Check if there are any spec files in the signup directory structure
find apps/web/src/app/signup -name '*.spec.tsx' -o -name '*.test.tsx' 2>/dev/nullRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Look for existing signup wallet tests to understand the test structure
find apps/web -path '*/signup*' -name '*.spec.tsx' -o -path '*/signup*' -name '*.test.tsx' 2>/dev/nullRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Verify the exact structure and check for any test-utils or related test configuration
ls -la apps/web/src/specs/ 2>/dev/null | head -20Repository: ecency/vision-next
Length of output: 877
🏁 Script executed:
# Check if there's any test setup or configuration that might indicate where tests should go
cat apps/web/src/specs/test-utils.tsx 2>/dev/null | head -30Repository: ecency/vision-next
Length of output: 1222
Add tests for the MetamaskConnect component to cover all signup branches.
This component implements address discovery (fetchMultichainAddresses), balance validation ($1 USD minimum), duplicate-address checks (checkWalletExistence), and the signing step (signAndVerify), but has no corresponding test coverage. Per coding guidelines, all new features in @ecency/web require tests. Use the testing utilities in src/specs/test-utils.tsx and follow the *.spec.tsx pattern for test co-location.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx` around
lines 147 - 528, The PR is missing unit/integration tests for the
MetamaskConnect component covering address discovery, balance validation,
duplicate-address checks and signing; add a new spec file co-located with the
component (MetamaskConnect.spec.tsx) using the test utilities from
src/specs/test-utils.tsx and mock the key interactions: simulate window.ethereum
and eth_requestAccounts to exercise connectMetaMask and
fetchMultichainAddresses, mock useGetExternalWalletBalanceQuery /
getTokenPriceQueryOptions to test the $1 USD MINIMUM_VALIDATION_USD branch and
both valid/invalid balances, stub
EcencyWalletsPrivateApi.useCheckWalletExistence to test addressAlreadyUsed
behavior, and spy on the signAndVerify flow (personal_sign) to assert onVerified
is called with correct args; include tests for UI states (loading spinners,
error messages via i18next keys) and enable/disable of the Sign button.
Comment on lines
+275
to
+291
| const signAndVerify = useCallback(async () => { | ||
| if (!window.ethereum || !connectedAddress || !selectedCurrency) return; | ||
|
|
||
| setIsSigning(true); | ||
| try { | ||
| // For EVM chains, sign with MetaMask directly | ||
| // For non-EVM chains, we still use MetaMask's personal_sign with the EVM address | ||
| // to prove the user controls this MetaMask wallet | ||
| const signingAddress = evmAddress || connectedAddress; | ||
| const message = `Create Hive account: ${username}`; | ||
| await window.ethereum.request({ | ||
| method: "personal_sign", | ||
| params: [message, signingAddress] | ||
| }); | ||
|
|
||
| success(i18next.t("signup-wallets.validate-funds.validation-success")); | ||
| onVerified(selectedCurrency, connectedAddress, availableAddresses); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx | head -300Repository: ecency/vision-next
Length of output: 12075
🏁 Script executed:
cat -n apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx | sed -n '260,310p'Repository: ecency/vision-next
Length of output: 2100
🏁 Script executed:
rg -A 5 "onVerified" apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsxRepository: ecency/vision-next
Length of output: 1067
🏁 Script executed:
rg "personal_sign\|signature\|verify.*sign" apps/web/src/app/signup/wallet/ -A 3 -B 3Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
rg "selectedCurrency" apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx -B 5 -A 5Repository: ecency/vision-next
Length of output: 7086
🏁 Script executed:
rg "onVerified" apps/web/src/app/signup/wallet/ -B 3 -A 8 | head -100Repository: ecency/vision-next
Length of output: 5074
🏁 Script executed:
find apps/web/src/app/signup/wallet -type f -name "*.tsx" | head -10Repository: ecency/vision-next
Length of output: 597
🏁 Script executed:
rg "MetamaskConnect" apps/web/src/app/signup/wallet -B 2 -A 10Repository: ecency/vision-next
Length of output: 3766
🏁 Script executed:
rg "signature\|personal_sign" apps/web/src/app/signup/ -lRepository: ecency/vision-next
Length of output: 44
Capture and verify the wallet signature to prove ownership of the exact address being registered.
The personal_sign result is discarded (line 285–288), and when BTC/SOL is selected the code signs with evmAddress instead of the selected chain address. This means:
- No proof that the user controls the
currency/addresspair being registered onVerified()receives no signature for server-side verification- Non-EVM address ownership is never cryptographically validated
Store the signature, bind it to the selected currency and address, and validate it server-side with a nonce.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx` around
lines 275 - 291, In signAndVerify: capture the returned signature from
window.ethereum.request("personal_sign") and pass it to onVerified so the server
can validate ownership with a nonce; ensure the signingAddress is the actual
address for the selected currency (use the selected chain address rather than
always evmAddress for BTC/SOL cases), store the signature together with
selectedCurrency and the specific address, and ensure setIsSigning is cleared in
finally; finally update onVerified(selectedCurrency, address,
availableAddresses, signature) (or equivalent payload) so server-side
verification can bind signature → currency → address.
Comment on lines
+32
to
+38
| // Sync prop username when it arrives (e.g. from parent useEffect reading URL params) | ||
| useEffect(() => { | ||
| if (propUsername && propUsername !== username) { | ||
| setUsername(propUsername); | ||
| } | ||
| }, [propUsername]); | ||
|
|
Contributor
There was a problem hiding this comment.
Please add tests for the new prop→state sync behavior.
This introduces new behavior and should be covered (at least: set from prop, prop change, and prop clear resetting visibility/state).
As per coding guidelines, “All new features in @ecency/web require tests”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/shared/purchase-qr/purchase-qr-builder.tsx` around
lines 32 - 38, Add unit tests for the PurchaseQrBuilder component to cover the
prop→state sync implemented in the useEffect: verify that when propUsername is
provided on initial render the component sets internal username (check rendered
output or exposed state), that updating propUsername causes username to update
(re-rendering accordingly), and that clearing propUsername resets
visibility/state (e.g., hides QR and resets username/related state). Target the
component PurchaseQrBuilder and exercise the useEffect-driven logic tied to
propUsername / username / setUsername by rendering with a test renderer (RTL),
using rerender to change props, and asserting the expected DOM/state
transitions.
Comment on lines
+33
to
+37
| useEffect(() => { | ||
| if (propUsername && propUsername !== username) { | ||
| setUsername(propUsername); | ||
| } | ||
| }, [propUsername]); |
Contributor
There was a problem hiding this comment.
Prop sync is incomplete for cleared usernames.
At Line 34, the truthy check skips updates when propUsername is cleared (""/undefined), so local username (and potentially displayed QR) can remain stale.
💡 Suggested fix
useEffect(() => {
- if (propUsername && propUsername !== username) {
- setUsername(propUsername);
- }
+ const nextUsername = propUsername ?? "";
+ setUsername((prev) => (prev === nextUsername ? prev : nextUsername));
+ if (!nextUsername) {
+ setIsQrShow(false);
+ }
}, [propUsername]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/shared/purchase-qr/purchase-qr-builder.tsx` around
lines 33 - 37, The useEffect currently ignores falsy propUsername values and
therefore never syncs cleared usernames; update the effect in
purchase-qr-builder.tsx so it does not gate on truthiness—use a strict
comparison (propUsername !== username) and call setUsername(propUsername) when
they differ, and include both propUsername and username in the effect dependency
array to avoid a stale-closure issue; reference the existing useEffect and the
setUsername state updater to locate and change the logic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsx (1)
22-131:⚠️ Potential issue | 🟠 MajorAdd tests for the MetaMask banner component logic.
Per coding guidelines, all new features in
@ecency/webrequire tests. The MetaMask-specific banner logic including conditional visibility for MetaMask users, banner dismissal, and token presence checks must be covered by tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsx around lines 22 - 131, Add unit tests for the ProfileWalletExternalBanner component covering MetaMask-specific visibility and dismissal/token checks: write tests that mount ProfileWalletExternalBanner and mock useActiveAccount to set activeUser.username to the route username, mock getAccessToken/getLoginType to simulate MetaMask vs non-MetaMask, mock useLocalStorage to observe setDismissedAt/removeDismissedAt behavior, and mock React Query responses for getAccountFullQueryOptions (to control data.profile.tokens) and checkUsernameWalletsPendingQueryOptions (to control walletCheckData.exist). Specifically include tests that (1) show banner for MetaMask users when posting_json_metadata has no chain tokens, (2) hide for non-MetaMask when walletCheckData.exist is true, (3) hide when data.profile.tokens includes EcencyWalletCurrency symbols, and (4) persist dismissal by asserting setDismissedAt(Date.now()) is called and subsequent render respects isDismissed logic (including expiry behavior by faking Date.now()). Use your app's testing utilities (React Testing Library + query client mocks) and assert presence/absence of banner elements and button actions rather than implementation details.apps/web/src/providers/sdk/web-broadcast-adapter.ts (1)
523-539: 🛠️ Refactor suggestion | 🟠 MajorAdd explicit retry handling for active-authority operations in the MetaMask/Keychain path.
This path executes active-authority signing for granting posting authority, but there’s no retry strategy on transient signing/broadcast failures.
As per coding guidelines: “Implement the broadcast adapter pattern with methods:
getLoginType(),showAuthUpgradeUI(), and retry logic for active authority operations”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/providers/sdk/web-broadcast-adapter.ts` around lines 523 - 539, The key/keychain branch lacks retry handling for active-authority signing/broadcast; update the keychain path in getWebBroadcastAdapter (the case 'key' and the case 'keychain' that calls broadcastWithKeychain) to retry transient signing/broadcast failures (e.g., exponential backoff with a small max attempts) when calling CONFIG.hiveClient.broadcast.sendOperations or delegating to broadcastWithKeychain, and ensure getTempActiveKey is cleared only after a successful broadcast (use clearTempActiveKey on success, not before final failure), and on repeated failures invoke showAuthUpgradeUI() or getLoginType() flow to surface an auth upgrade prompt; keep retry limits and surface final error after retries.
♻️ Duplicate comments (4)
apps/web/package.json (1)
68-68:⚠️ Potential issue | 🟠 MajorKeep dependency parity with
apps/self-hosted/package.jsonfor Line 68.
@wallet-standard/appwas added here; please confirm the same package/version is present inapps/self-hosted/package.jsonto avoid deployment drift.#!/bin/bash # Verify `@wallet-standard/app` parity between web and self-hosted manifests. python - <<'PY' import json from pathlib import Path files = ["apps/web/package.json", "apps/self-hosted/package.json"] dep = "@wallet-standard/app" for f in files: p = Path(f) if not p.exists(): print(f"{f}: FILE_NOT_FOUND") continue data = json.loads(p.read_text()) val = (data.get("dependencies") or {}).get(dep) print(f"{f}: {dep} = {val}") PYBased on learnings: In the ecency/vision-next repository, the self-hosted app dependencies are intentionally kept consistent with the web app dependencies to maintain compatibility and architectural parity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` at line 68, Confirm that the dependency "@wallet-standard/app" is present in the self-hosted package.json and that its version specifier matches "^1.1.0"; if it is missing or the version differs, add or update the "@wallet-standard/app" entry in the self-hosted package.json to "^1.1.0" and regenerate/install dependencies (lockfile) so both manifests remain in parity.apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx (1)
275-292:⚠️ Potential issue | 🔴 CriticalCapture and pass the MetaMask signature (ownership proof is currently discarded).
The
personal_signresult is ignored, andonVerifiedis called without signature/nonce binding. That leaves no cryptographic proof tied to the selected wallet address during account creation.#!/bin/bash # Verify whether signature data is captured/passed from MetamaskConnect. rg -nP --type=tsx -C3 'personal_sign|onVerified\(' apps/web/src/app/signup/wallet/_components/stepsExpected: signature is captured from
personal_signand propagated inonVerifiedpayload.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx` around lines 275 - 292, The personal_sign result in signAndVerify is currently discarded; capture the signature returned by window.ethereum.request (using method "personal_sign" with params [message, signingAddress]), and pass that signature (and the signed message or nonce) into onVerified so the caller receives a cryptographic proof tied to signingAddress; update the call site of onVerified to accept the new signature parameter and propagate it, and ensure signAndVerify uses signingAddress (evmAddress || connectedAddress) and message when constructing the signature payload.apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsx (2)
60-67:⚠️ Potential issue | 🟠 MajorDon’t put the invite payload in the URL as a base64 blob.
b64uEnc(JSON.stringify(...))only obscures the payload. The invitee email is recoverable from logs/history/referrers, and anyone who edits the blob can changeusername,referral, orpubkeysbefore signup. Please replace this with an opaque server-issued token or a signed/encrypted payload validated on the server.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx around lines 60 - 67, The code in onboard-asking.tsx is putting sensitive invite data into the URL by calling b64uEnc(JSON.stringify(dataToEncode)) and setSecret; stop shipping raw/obfusced payloads client-side and instead request an opaque server-issued token or a server-signed/encrypted blob that the server validates on use. Replace the client-side encoding flow (the dataToEncode object, b64uEnc call and setSecret usage) with a call to your backend (e.g., an API endpoint you create) that accepts username/email/referral/pubkeys, returns a short opaque token (or a signed JWT/encrypted payload), and use that token in the URL; ensure server-side validation/verification of the token in the signup flow before trusting username/email/referral/pubkeys.
30-32:⚠️ Potential issue | 🟠 MajorReset
fileIsDownloadedwhen the master password changes.Regenerating
masterPasswordrecomputessecret, butfileIsDownloadedstaystrue, so the share link can point to keys the user has never exported.handleDownloadalso flips the flag unconditionally instead of tying it to the current password export.Minimal fix
const [masterPassword, setMasterPassword] = useState(() => generateMasterPassword()); const [fileIsDownloaded, setFileIsDownloaded] = useState(false); const [secret, setSecret] = useState(""); + + useEffect(() => { + setFileIsDownloaded(false); + }, [masterPassword]); ... const handleDownload = useCallback(() => { + if (!masterPassword) return; downloadKeys(); setFileIsDownloaded(true); - }, [downloadKeys]); + }, [downloadKeys, masterPassword]);Also applies to: 70-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx around lines 30 - 32, The fileIsDownloaded flag must be reset whenever the master password (and derived secret) changes and only be set true when the current password export actually completes; update the component so that changing masterPassword (via setMasterPassword or when generateMasterPassword runs) also calls setFileIsDownloaded(false) or add a useEffect that watches masterPassword and resets fileIsDownloaded, and modify handleDownload to mark setFileIsDownloaded(true) only after the download/export for the current masterPassword/secret succeeds (tie the success flag to the specific secret/masterPassword in the download flow).
🟠 Major comments (19)
apps/web/src/app/signup/free/_page.tsx-68-76 (1)
68-76:⚠️ Potential issue | 🟠 MajorPrevent stale username-check responses from overriding current input state.
At Line 71, an older
fetchQueryresponse can resolve after a newer username input and still setusernameError/isDisabled, causing incorrect blocking.💡 Suggested fix
+ const usernameCheckSeq = useRef(0); + useDebounce( () => { if (username?.length >= 3 && username.length <= 16) { - queryClient.fetchQuery(getAccountsQueryOptions([username])).then((r) => { + const seq = ++usernameCheckSeq.current; + queryClient.fetchQuery(getAccountsQueryOptions([username])).then((r) => { + if (seq !== usernameCheckSeq.current) return; if (r.length > 0) { setUsernameError(i18next.t("sign-up.username-exists")); setIsDisabled(true); + } else { + setUsernameError(""); } }); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/free/_page.tsx` around lines 68 - 76, The debounced fetchQuery can resolve out-of-order and overwrite state for a newer input; inside the useDebounce callback (the function that reads username and calls queryClient.fetchQuery), capture the current username into a local const (e.g. const queriedUsername = username) or use a monotonic requestId stored in a ref, then after the fetch completes compare queriedUsername/requestId against the latest value (or ref) before calling setUsernameError and setIsDisabled; only update state when they match to prevent stale responses from overriding the current input.apps/web/src/app/signup/free/_page.tsx-125-141 (1)
125-141:⚠️ Potential issue | 🟠 MajorHandle network failures for pre-submit account/referral checks.
At Line 125 and Line 134,
fetchQueryruns outside thetry/catch; a rejection bypasses your error handling and gives no controlled UI error path.💡 Suggested fix
- const existingAccount = await queryClient.fetchQuery( - getAccountsQueryOptions([username]) - ); - if (existingAccount.length > 0) { - setUsernameError(i18next.t("sign-up.username-exists")); - return; - } - - if (referral) { - const referralIsValid = await queryClient.fetchQuery( - getAccountsQueryOptions([referral]) - ); - if (referralIsValid.length === 0) { - setReferralError(i18next.t("sign-up.referral-invalid")); - return; - } - } - setInProgress(true); try { + const existingAccount = await queryClient.fetchQuery( + getAccountsQueryOptions([username]) + ); + if (existingAccount.length > 0) { + setUsernameError(i18next.t("sign-up.username-exists")); + return; + } + + if (referral) { + const referralIsValid = await queryClient.fetchQuery( + getAccountsQueryOptions([referral]) + ); + if (referralIsValid.length === 0) { + setReferralError(i18next.t("sign-up.referral-invalid")); + return; + } + } + if (!isVerified) { error(i18next.t("login.captcha-check-required")); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/free/_page.tsx` around lines 125 - 141, The pre-submit checks call queryClient.fetchQuery with getAccountsQueryOptions outside of any try/catch, so network failures throw and bypass UI error handling; wrap both calls to queryClient.fetchQuery (the one that checks existingAccount and the one that checks referralIsValid) in try/catch blocks and on catch set an appropriate UI error (e.g., call setUsernameError or setReferralError or a shared form-level error like setSubmitError with a localized "network" message via i18next.t), then return to abort submission; ensure you reference getAccountsQueryOptions, queryClient.fetchQuery, setUsernameError and setReferralError when making the changes so the error path is consistent and user-visible.apps/web/src/app/signup/free/_page.tsx-21-273 (1)
21-273:⚠️ Potential issue | 🟠 MajorAdd test coverage for the FreeSignUp component.
This new feature handles critical user-facing logic: form validation (username, email, referral), username existence checking, referral validation, ReCAPTCHA gating, and registration states. Per coding guidelines, all new features in
@ecency/webrequire tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/free/_page.tsx` around lines 21 - 273, Add unit/integration tests for the FreeSignUp component covering form validation and flows: mount FreeSignUp and assert username/email/referral validation messages driven by getUsernameError and length checks, simulate useQueryClient.fetchQuery responses to test existing username (set usernameError), invalid referral (set referralError) and valid referral path, verify ReCAPTCHA gating by calling captchaCheck (or simulating ReCAPTCHA onChange) disables/enables submit, mock signUp to produce success (sets done and clears lsReferral via useLocalStorage wrapper) and failure (sets registrationError from thrown error.data.message), and assert submit button disabled states (inProgress, isDisabled, errors). Target symbols: FreeSignUp, handleSubmit, captchaCheck, signUp, getAccountsQueryOptions, and the local state interactions (setLsReferral) when stubbing queryClient and signUp.apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx-130-203 (1)
130-203:⚠️ Potential issue | 🟠 MajorRetry currently does not restart the provisioning effect.
The retry button resets state, but the effect in Lines 130-203 does not depend on status, so the workflow typically won’t re-run.
🛠️ Suggested fix
- useEffect(() => { - if (hasInitiatedRef.current) return; + useEffect(() => { + if (status !== "installing-snap" || hasInitiatedRef.current) return; hasInitiatedRef.current = true; @@ - }, [username, verifiedWallet, validateAccountIsCreated, recordActivity]); + }, [status, username, verifiedWallet, validateAccountIsCreated, recordActivity]);Also applies to: 292-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 130 - 203, The effect guarded by hasInitiatedRef (inside the useEffect that runs the MetaMask snap provisioning flow) never restarts on retry because the dependency array lacks any retry/state token; update the logic so retries restart the effect by either adding the relevant status/retry token to the dependency array (e.g., include status or retryCount) or ensure the retry handler resets hasInitiatedRef.current = false before setting status, and then include that status/retry token in the useEffect dependencies; reference hasInitiatedRef, useEffect, setStatus, and the retry handler so the provisioning flow reruns when the user clicks retry.apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx-110-125 (1)
110-125:⚠️ Potential issue | 🟠 MajorBound validation polling to avoid an infinite loading state.
Lines 110-121 can poll forever if the account never appears while the component stays mounted.
🛠️ Suggested fix
- while (!shouldStop?.()) { + const maxAttempts = 24; // ~2 minutes at 5s interval + let attempts = 0; + while (!shouldStop?.() && attempts < maxAttempts) { try { const account = await queryClient.fetchQuery(accountQueryOptions); if (account) { queryClient.setQueryData(accountQueryOptions.queryKey, account); return account as unknown as FullAccountEntity; } } catch { /* Account might not yet exist; retry */ } await delay(5000); + attempts += 1; } + if (!shouldStop?.()) { + const timeoutError = new Error("Account validation timed out"); + timeoutError.name = "TimeoutError"; + throw timeoutError; + } + const abortError = new Error("Account validation aborted"); abortError.name = "AbortError"; throw abortError;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 110 - 125, The polling loop using shouldStop, queryClient.fetchQuery, and delay can run indefinitely if an account never appears; modify the loop (inside metamask-account-creating component) to enforce a bounded retry strategy by tracking attempts or a deadline (e.g., maxAttempts or a timeout timestamp) and break/throw after exceeding it; ensure you still respect shouldStop(), call queryClient.setQueryData when account is found, and replace the unconditional final AbortError throw with a clear timeout/limit error when the bound is reached so the component stops loading instead of polling forever.apps/web/src/app/wallet/setup-external/_components/setup-external-metamask.tsx-1-540 (1)
1-540:⚠️ Potential issue | 🟠 MajorAdd spec file for SetupExternalMetamask component.
No test file exists for this 540-line feature. Create
apps/web/src/specs/app/wallet/setup-external/_components/setup-external-metamask.spec.tsxto test the connect/sign/linking/success flows, MetaMask vs non-MetaMask user branching, error paths, and blockchain operations. Per coding guidelines, all new features require tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/wallet/setup-external/_components/setup-external-metamask.tsx` around lines 1 - 540, Add a new Jest/React Testing Library spec for the SetupExternalMetamask component that exercises the connect/sign/linking/success flows and error branches: render SetupExternalMetamask (and SetupExternalMetamaskInner) and mock window.ethereum and wallet-standard behavior to test connectAndFetch (successful eth_requestAccounts, fetchMultichainAddresses, ensureHiveSnap/getHivePublicKeys), the MetaMask-user shortcut (handleLinkMetaMaskUser), the non-MetaMask signing path (KeyOrHot invoking handleLinkByKey which triggers saveWalletData and hiveClient.broadcast.updateAccount), and failure cases (eth_requestAccounts rejection, snap errors, and private API save failure). Use spies/mocks for getWallets, saveTokens (useSaveWalletInformationToMetadata), EcencyWalletsPrivateApi.useUpdateAccountWithWallets, getQueryClient fetchQuery, and Client.broadcast.updateAccount to assert correct UI step transitions (connect → sign → linking → success), proper calls to fetchMultichainAddresses/ensureHiveSnap/getHivePublicKeys, and that errors show the error path and reset state.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/account-recovery.tsx-196-196 (1)
196-196:⚠️ Potential issue | 🟠 MajorRemove or fix the MetaMask callback on line 196—it incorrectly uses the "keychain" type.
The
useAccountUpdateRecoverymutation only supports"key" | "keychain" | "hivesigner" | "ecency"as valid signing types. The callbackonMetaMask={() => handleSign("keychain")}is semantically incorrect because MetaMask is a distinct wallet that won't work with the keychain broadcaster. Either remove this callback or add proper MetaMask support to the mutation's SignType union and implement its handler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/account-recovery.tsx at line 196, The onMetaMask callback is incorrectly calling handleSign("keychain"); update the code to either remove the onMetaMask handler or implement proper MetaMask support: change the callback to remove onMetaMask or wire it to a new handler that calls handleSign with a correct SignType (e.g., "metamask") and extend the useAccountUpdateRecovery SignType union and its mutation implementation to accept and process "metamask" (or add a separate metamask-specific signer function). Locate handleSign, the onMetaMask prop usage, and the useAccountUpdateRecovery SignType/handler to ensure the signer and broadcaster logic are updated consistently.apps/web/src/app/signup/invited/[hash]/_page.tsx-93-99 (1)
93-99:⚠️ Potential issue | 🟠 MajorRecompute RC validation/costs when the RC amount changes.
Right now RC validation and operation estimates are recalculated only when
isCheckedchanges. EditingrcAmountleavesrcErrorand estimated counts stale.💡 Suggested fix
-useEffect(() => { +useEffect(() => { if (!isChecked) { setRcAmount(0); return; } rcOperationsCost(); -}, [isChecked]); +}, [isChecked, rcAmount]);Also applies to: 225-231
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx around lines 93 - 99, The useEffect that calls rcOperationsCost currently only depends on isChecked, so changes to rcAmount leave rcError and estimated RC counts stale; update that effect (the one that sets rcAmount to 0 when !isChecked and calls rcOperationsCost) to include rcAmount in its dependency array and ensure rcOperationsCost runs whenever either isChecked or rcAmount changes; also apply the same dependency change to the other similar useEffect handling RC validation/estimates (the duplicate block around the rc operations/validation logic) so rcError and estimates are recomputed when rcAmount is edited.apps/web/src/features/i18n/locales/en-US.json-2921-2941 (1)
2921-2941:⚠️ Potential issue | 🟠 MajorAdd all new MetaMask flow strings as explicit locale keys (don’t rely on TSX
defaultValue).The signup MetaMask UI now uses additional text keys (e.g. address label / chain unavailable / multichain checking / balance label) via fallback defaults. Please add them in this file so translations are centralized and consistent.
As per coding guidelines: “All new strings must be added to
en-US.jsononly for internationalization”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/i18n/locales/en-US.json` around lines 2921 - 2941, The MetaMask signup UI added new in-UI string keys that are currently using TSX defaultValue fallbacks; update the en-US.json "metamask" object to include explicit keys for all new flow strings (e.g., add keys such as "address-label"/"enter-address" already present, and the missing ones like "chain-unavailable"/"chain-unsupported" (ensure exact key used in code), "multichain-checking" or "checking-multichain", "balance-label"/"wallet-balance", and any other labels used by the MetaMask signup components) so translations are centralized—find usages in the MetaMask-related components (look for strings referenced in the signup MetaMask TSX files) and add matching keys and English values into en-US.json instead of relying on defaultValue.apps/web/src/app/signup/invited/[hash]/_page.tsx-184-192 (1)
184-192:⚠️ Potential issue | 🟠 MajorMove new sponsor copy into locale JSON instead of
defaultValue.
sponsor-titleandsponsor-descriptionare newly introduced user-facing strings but defined inline viadefaultValue. Add them toen-US.jsonand consume keys directly.As per coding guidelines: “All new strings must be added to
en-US.jsononly for internationalization”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/invited/`[hash]/_page.tsx around lines 184 - 192, Add the new user-facing strings into the locale JSON and stop using inline defaultValue: add "onboard.sponsor-title" and "onboard.sponsor-description" entries to en-US.json with the provided copy, then update the i18next.t calls in the _page.tsx component to call i18next.t("onboard.sponsor-title") and i18next.t("onboard.sponsor-description") without the defaultValue option; ensure the JSON keys match exactly and run a quick i18n build/check to validate they resolve at runtime.apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx-420-447 (1)
420-447:⚠️ Potential issue | 🟠 MajorAvoid inline fallback strings for new copy in this flow.
New user-facing text is added via
defaultValueinstead of locale keys. Please add these strings toen-US.jsonand use keys only.As per coding guidelines: “All new strings must be added to
en-US.jsononly for internationalization”.Also applies to: 464-464
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsx` around lines 420 - 447, The i18next.t calls in the MetaMask connect component are using inline defaultValue fallbacks (e.g., the calls that render "Soon", the "{{chain}} address:" label and the multichain/checking/chain-not-available messages inside the metamask-connect component), which violates the i18n guideline; add new keys and English text to en-US.json (for each string used as defaultValue, create a meaningful key under signup-wallets.metamask or similar), then replace the i18next.t invocations to use only those keys (remove defaultValue arguments) so the component (metamask-connect.tsx) and any uses of CURRENCIES_META_DATA display text via locale keys; also update the other occurrence mentioned (around line ~464) similarly.apps/web/src/features/shared/metamask-sign-button.tsx-25-25 (1)
25-25:⚠️ Potential issue | 🟠 MajorAvoid embedding new strings directly in the component.
Both the
alttext and label fallback string are user-facing strings and should be sourced from locale keys only.As per coding guidelines
apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization.Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/metamask-sign-button.tsx` at line 25, The component apps/web/src/features/shared/metamask-sign-button.tsx contains user-facing strings (the img alt="metamask" and the label fallback string) hard-coded; replace these literals with locale keys from en-US.json and load them via the existing i18n hook/translator used in the component (e.g., useTranslation or t) so the img alt and the fallback label use t('your.key.for.metamaskAlt') and t('your.key.for.metamaskLabel') respectively, and add the corresponding entries to en-US.json.apps/web/src/specs/features/permissions/add-keys-flow.spec.tsx-35-44 (1)
35-44:⚠️ Potential issue | 🟠 MajorAvoid mocking internal
KeyOrHotin this flow test.This replaces internal app behavior with a fake implementation and shifts the test toward implementation details.
As per coding guidelines
apps/web/src/specs/**/*.spec.{ts,tsx}: Mock external dependencies with vi.fn(), not internal functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/features/permissions/add-keys-flow.spec.tsx` around lines 35 - 44, The test currently overrides the internal KeyOrHot component inside the vi.mock call which hides real app behavior; remove the KeyOrHot override from the mock and only mock external functions (e.g., keep success: vi.fn(), error: vi.fn()) so the real KeyOrHot component is exercised. Locate the vi.mock invocation in the spec and delete the KeyOrHot: (...) entry, leaving only mocks for external helpers, or alternatively import the real module instead of mocking it so the KeyOrHot component's actual behavior is used in the add-keys-flow.spec.tsx test.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-3-review-keys.tsx-79-86 (1)
79-86:⚠️ Potential issue | 🟠 MajorMove login-type labels to translation keys.
These labels are new user-facing strings and should come from i18n resources instead of inline literals.
As per coding guidelines
apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-3-review-keys.tsx around lines 79 - 86, Replace the inline user-facing labels in the switch block for login types (cases "metamask", "keychain", "hivesigner", "hiveauth", "privateKey" in step-3-review-keys.tsx) with calls to the translation function used in this component (e.g., t(...) or useTranslations(...)) and add corresponding keys to en-US.json (suggestion: profile.permissions.loginType.metamask/keychain/hivesigner/hiveauth/privateKey). Ensure the returned objects keep the same className values but use the translation keys for label text so all new strings are sourced from i18n resources.apps/web/src/specs/features/permissions/add-keys-flow.spec.tsx-7-11 (1)
7-11:⚠️ Potential issue | 🟠 MajorUse
renderWithQueryClientinstead of mocking React Query internals.This spec bypasses the actual query layer by mocking core hooks. The component tree uses React Query in multiple steps (step-1-authenticate, step-3-review-keys, and step-4-confirm all call
useQuery), so the test should userenderWithQueryClientfromsrc/specs/test-utils.tsxto properly test the integration.Also, remove the mock of the internal
@/features/sharedcomponent (lines 35-44). Per guidelines, mock only external dependencies, not internal application components.Also applies to: 56-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/features/permissions/add-keys-flow.spec.tsx` around lines 7 - 11, Remove the manual vi.mock of "@tanstack/react-query" (the useQuery/useQueryClient/useMutation stubs) and instead render the component under test with the existing test helper renderWithQueryClient from src/specs/test-utils.tsx so the real React Query provider is used for the steps that call useQuery (step-1-authenticate, step-3-review-keys, step-4-confirm). Also remove the vi.mock of the internal "@/features/shared" component (the mock around lines 35-44) — only external libs should be mocked; keep internal components real. Apply the same replacement for the second mocked block referenced in the comment (lines ~56-74).apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx-117-161 (1)
117-161:⚠️ Potential issue | 🟠 MajorKeep revoke available for MetaMask users.
The
!isMetamaskguard currently hides the whole secondary row, including the trash action. That means MetaMask accounts with multiple authorities can no longer revoke old keys from this screen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx around lines 117 - 161, The current !isMetamask guard hides the entire secondary row (including the revoke/trash action), preventing MetaMask users from revoking keys; update the JSX so only the private-key display and reveal/copy controls are hidden for MetaMask while the revoke button remains rendered for both types. Concretely, keep the outer row and the copy/reveal buttons conditional on !isMetamask (using isMetamask, keys[key[0]], copy, setShowReveal), but move the revoke block (the Button that calls setShowRevoke and setRevokingKey and is guarded by accountData?.[keyName].length > 1) outside that !isMetamask conditional so the trash action is available for MetaMask accounts as well.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx-152-155 (1)
152-155:⚠️ Potential issue | 🟠 MajorPass the listed authority key into the revoke dialog.
keys[key[0]]is the revealed private key and is often empty until the user explicitly reveals it. Clicking revoke before that opens the dialog without a stable target, and even when populated it's the wrong identifier to remove.Suggested fix
onClick={() => { setShowRevoke(true); - setRevokingKey(keys[key[0]]); + setRevokingKey(key[0]); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx around lines 152 - 155, The revoke dialog is being given the revealed private key (keys[key[0]]) which is often empty or the wrong identifier; change the onClick to pass the listed authority item itself into setRevokingKey (e.g., setRevokingKey(key)) so the dialog receives the stable key/identifier being displayed; if your list item is an object/array and you need a specific identifier field, pass that explicit field (for example key.id or key.address) instead of keys[key[0]].apps/web/src/features/shared/key-or-hot/index.tsx-27-45 (1)
27-45:⚠️ Potential issue | 🟠 MajorDon’t hard-switch every MetaMask user to the MetaMask button.
This branch fires on login type alone, even when the caller did not pass
onMetaMaskor explicitly needskeyOnly/authority="owner". In those cases the component drops the raw-key path and can degrade into a no-op or block owner-level actions entirely.Possible guard
-export function KeyOrHot({ inProgress, onKey, onHot, onKc, onMetaMask, keyOnly, authority="active" }: Props) { +export function KeyOrHot({ + inProgress, + onKey, + onHot, + onKc, + onMetaMask, + keyOnly, + authority = "active" +}: Props) { const { activeUser } = useActiveAccount(); const isMobileBrowser = useIsMobile(); const useHiveAuth = shouldUseHiveAuth(activeUser?.username); const isMetaMaskUser = activeUser && getLoginType(activeUser.username) === "metamask"; + const canUseMetaMask = isMetaMaskUser && !!onMetaMask && !keyOnly && authority === "active"; const canRenderKeychain = !isMetaMaskUser && onKc && (!isMobileBrowser || useHiveAuth || isKeychainInAppBrowser()); ... - if (isMetaMaskUser) { + if (canUseMetaMask) { return ( <div className="key-or-hot"> <MetaMaskSignButton onClick={() => onMetaMask?.()} /> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/key-or-hot/index.tsx` around lines 27 - 45, The component currently forces MetaMask UI when getLoginType(...) === "metamask" regardless of props; change the early-return branch in KeyOrHot to only switch to the MetaMask button when onMetaMask is provided and the caller allows non-key-only/owner actions: replace the current isMetaMaskUser check with a guard like isMetaMaskUser && onMetaMask && !keyOnly && authority !== "owner" (or equivalent) so that when keyOnly is true, authority is "owner", or no onMetaMask handler is passed the component falls back to the normal key/hot rendering paths.apps/web/src/app/wallet/setup-external/_components/setup-external-import.tsx-66-76 (1)
66-76:⚠️ Potential issue | 🟠 MajorAvoid uncommitted splits across metadata and private API stores.
The flow commits
saveTokens()to the Hive profile first, then callssaveToPrivateApi()with blankhiveKeys. If the second call fails, the first write has already persisted—returning to the entry step masks an inconsistent state across two backends. The private API endpoint stores the emptyhiveKeysdirectly without merge logic; on subsequent calls with real keys, they would overwrite any prior values. Either unify both operations behind a single backend endpoint, or ensure the private API treats empty key fields as no-op and preserves existing values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/wallet/setup-external/_components/setup-external-import.tsx` around lines 66 - 76, The current flow saves to Hive metadata via saveTokens() before calling the private API (saveToPrivateApi()), which can persist blank hiveKeys and leave stores inconsistent; change the flow so both stores are updated atomically by either (a) calling saveToPrivateApi() first with the full merged payload (including hiveKeys) and only on success calling saveTokens(), or (b) have the private API treat empty hiveKeys as a no-op by fetching existing keys and merging before writing (so use EcencyWalletsPrivateApi.useUpdateAccountWithWallets to fetch/merge existing hiveKeys), and ensure onError handlers (used in useSaveWalletInformationToMetadata/saveToPrivateApi) rollback or prevent partial commits (do not call setStep("addresses") until both updates succeed). Include formatError/error logging on failures as before.
🟡 Minor comments (11)
apps/web/src/features/shared/navbar/sidebar/navbar-side-main-logout.tsx-24-27 (1)
24-27:⚠️ Potential issue | 🟡 MinorUse exact profile-route matching instead of substring matching.
Line 26 can misclassify routes like
/@alice2as Alice’s own profile ifusername === "alice", causing an incorrect redirect at Line 35.Suggested fix
- const isOnOwnProfile = username && pathname?.includes(`@${username}`); + const isOnOwnProfile = Boolean( + username && + (pathname === `/@${username}` || pathname.startsWith(`/@${username}/`)) + );Also applies to: 33-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/navbar/sidebar/navbar-side-main-logout.tsx` around lines 24 - 27, The logout logic in handleLogout uses substring matching (isOnOwnProfile via pathname?.includes(`@${username}`)) which misclassifies routes like /@alice2; change isOnOwnProfile to perform exact profile-route matching—e.g., compare the path segment to `@${username}` or use a regex that matches `^/@${username}(/|$)` against pathname—so the redirect logic (around the current Line 35) only triggers when the user is truly on their own profile.apps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsx-98-117 (1)
98-117:⚠️ Potential issue | 🟡 MinorAdd new MetaMask i18n keys to
en-US.json.The component introduces three new i18n keys (
wallet.link-metamask.banner-title,wallet.link-metamask.banner-description,wallet.link-metamask.title) that are missing from the locale file. Per coding guidelines, all new strings must be added toen-US.jsononly. WhiledefaultValuefallbacks prevent runtime errors, the keys should be properly registered in the locale file.Keys to add to `apps/web/src/features/i18n/locales/en-US.json` under the `wallet` section:
"link-metamask": { "banner-title": "Link your MetaMask wallets", "banner-description": "Save your ETH, BNB, and SOL addresses to your Hive profile to view balances and enable transactions.", "title": "Link MetaMask" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsx around lines 98 - 117, The component introduces three new i18n keys (wallet.link-metamask.banner-title, wallet.link-metamask.banner-description, wallet.link-metamask.title) that must be added to the English locale; open en-US.json and under the "wallet" section add a "link-metamask" object containing "banner-title", "banner-description", and "title" with the provided English strings so the keys exist (they are referenced by the ProfileWalletExternalBanner component and used as fallbacks via defaultValue).apps/web/src/app/wallet/setup-external/_components/setup-external-metamask.tsx-125-157 (1)
125-157:⚠️ Potential issue | 🟡 MinorMove new fallback copy out of code and into
en-US.json.These segments add multiple inline
defaultValuestrings; new user-facing strings should be defined in locale files and referenced by key only.As per coding guidelines:
apps/web/**/*.{ts,tsx}: All new strings must be added toen-US.jsononly for internationalization.Also applies to: 405-416, 432-440, 449-457, 463-465, 532-534
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/wallet/setup-external/_components/setup-external-metamask.tsx` around lines 125 - 157, The inline fallback strings in METAMASK_STEPS and METAMASK_STEPS_SHORT (e.g. uses of i18next.t("wallet.link-metamask.step-connect", { defaultValue: "Connect" }), "wallet.link-metamask.step-connect-desc", "wallet.link-metamask.step-sign", "wallet.link-metamask.step-sign-desc", "wallet.link-metamask.step-link", "wallet.link-metamask.step-link-desc", etc.) must be moved into the en-US.json locale file and removed from the code; update en-US.json with keys for each user-facing string referenced in METAMASK_STEPS/METAMASK_STEPS_SHORT and then change the calls in setup-external-metamask.tsx to call i18next.t("...") without providing defaultValue so the component only references keys (repeat same change for the other occurrences noted: lines ~405-416, 432-440, 449-457, 463-465, 532-534).apps/web/src/utils/master-password.ts-4-6 (1)
4-6:⚠️ Potential issue | 🟡 MinorUpdate the doc comment to match the actual entropy source.
The comment says
crypto.getRandomValues, but implementation usescrypto.randomUUID()+Date.now().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/utils/master-password.ts` around lines 4 - 6, Update the function doc comment for the Hive master password generator to accurately describe the entropy source: replace the incorrect mention of crypto.getRandomValues with the actual use of crypto.randomUUID() combined with Date.now() (and any string concatenation or hashing step used) so the comment for the function (e.g., generateMasterPassword or whatever function name appears in master-password.ts) reflects that the implementation derives entropy from crypto.randomUUID() plus the current timestamp before WIF encoding.apps/web/src/specs/utils/master-password.spec.ts-30-31 (1)
30-31:⚠️ Potential issue | 🟡 MinorLength assertion is far below the stated requirement.
The comment says 50+ chars, but the assertion only checks
> 5, which won’t catch bad outputs.🎯 Tighten assertion
- expect(password.length).toBeGreaterThan(5); + expect(password.length).toBeGreaterThan(50);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/utils/master-password.spec.ts` around lines 30 - 31, The test's length assertion for the generated master password is too weak: update the expectation in master-password.spec.ts to match the comment ("50+ chars") by asserting password.length is at least 50 (e.g., password.length >= 50 or toBeGreaterThan(49)) instead of the current toBeGreaterThan(5); locate the assertion that references the password variable in the spec and tighten it accordingly so the test fails for short outputs.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx-47-60 (1)
47-60:⚠️ Potential issue | 🟡 MinorAdd the missing
privateKeybadge case.
getDerivationBadge()falls back togetLoginTypeBadge()when the derivation is"unknown", but this switch returnsnullforprivateKey, so private-key users lose the badge entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx around lines 47 - 60, getLoginTypeBadge currently returns null for the privateKey login type causing private-key users to lose their badge when getDerivationBadge falls back to it; add a case for "privateKey" in getLoginTypeBadge that returns an object with a label like "Private Key" and appropriate CSS classes (mirroring the style pattern used for "metamask", "keychain", etc.) so private-key users render a badge consistently with other login types.apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsx-71-85 (1)
71-85:⚠️ Potential issue | 🟡 MinorGuard the RC math when the amount isn't finite.
You set
rcErrorfor invalid input, but the fourMath.ceilcalls still run withNaN/Infinity, so the helper counters can render nonsense. Reset the counters and return before doing the divisions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx around lines 71 - 85, The rcOperationsCost function currently sets rcError when rcAmount is invalid but still proceeds to compute and set counters, which yields NaN/Infinity; update rcOperationsCost to early-return when rcAmount is not a finite number or when rcAmount * 1e9 < 5000000000 by first calling setRcError (as already done) and then resetting the counters via setCommentAmount(0), setVoteAmount(0), setTransferAmount(0), setCustomJsonAmount(0) and return; otherwise continue with the existing Math.ceil calculations using ops from getRcStatsQueryOptions().apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsx-173-177 (1)
173-177:⚠️ Potential issue | 🟡 MinorMove the new delegate placeholder into i18n.
"Enter amount to delegate (Bn)"is a new user-facing string inapps/web, so it won't be translated with the rest of the onboarding flow. As per coding guidelines, "All new strings must be added toen-US.jsononly for internationalization".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx around lines 173 - 177, The placeholder "Enter amount to delegate (Bn)" is a new user-facing string and must be moved into i18n: add a new key (e.g., "onboard.delegatePlaceholder": "Enter amount to delegate (Bn)") to en-US.json and replace the hard-coded placeholder in the FormControl inside onboard-creating.tsx with the translation lookup (use the existing translation hook/function used elsewhere in this component, e.g., t('onboard.delegatePlaceholder')), keeping the FormControl props (value={rcAmount} and onChange={(e:any)=>setRcAmount(Number(e.target.value))}) unchanged.apps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsx-132-138 (1)
132-138:⚠️ Potential issue | 🟡 MinorUse the same condition for the icon
altas the icon/label.When
showHiveAuthOnMobileflips this button to HiveAuth,srcandkeychainLabelswitch over butaltstill says"keychain". That gives screen readers the wrong signer.Suggested fix
- alt={useHiveAuth ? "hiveauth" : "keychain"} + alt={useHiveAuthIcon ? "hiveauth" : "keychain"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsx` around lines 132 - 138, The Image alt text is not using the same condition as the icon/label, causing screen readers to announce the wrong signer; update the alt prop on the Image in auth-upgrade-dialog (the same component that sets src to keychainIcon) to use the same boolean that controls the icon/label (e.g., showHiveAuthOnMobile or useHiveAuth) and return the same label string (keychainLabel vs hiveAuthLabel) so the alt exactly matches the visible icon/label.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-2-generate-seed.tsx-61-63 (1)
61-63:⚠️ Potential issue | 🟡 MinorAdd accessible names to the icon-only copy/regenerate buttons.
Both controls render as icons only, so assistive tech gets unlabeled actions in a credential-management flow. Please add
aria-label(and ideallytitle) for the copy and regenerate buttons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-2-generate-seed.tsx around lines 61 - 63, The two icon-only Button instances rendering UilCopyAlt and UilSync (the ones wired to handleCopy and handleRegenerate) lack accessible names; add aria-label attributes (e.g., aria-label="Copy seed" and aria-label="Regenerate seed") and matching title attributes to those Button props so screen readers and tooltips convey the actions for the copy and regenerate controls.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-4-confirm.tsx-177-179 (1)
177-179:⚠️ Potential issue | 🟡 MinorAdd translation key to
en-US.jsoninstead of using inlinedefaultValue.The inline
defaultValuebypasses the translation file. The string"permissions.add-keys.step4.new-master-password-keys"should be defined inen-US.json.Suggested fix
- {i18next.t("permissions.add-keys.step4.new-master-password-keys", { - defaultValue: "4 new keys (owner, active, posting, memo) from master password" - })} + {i18next.t("permissions.add-keys.step4.new-master-password-keys")}And add to
en-US.json:"permissions.add-keys.step4.new-master-password-keys": "4 new keys (owner, active, posting, memo) from master password"As per coding guidelines: "All new strings must be added to
en-US.jsononly for internationalization".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-4-confirm.tsx around lines 177 - 179, Replace the inline defaultValue usage in the i18next.t call inside the Step 4 confirm component (step-4-confirm.tsx) by adding the key "permissions.add-keys.step4.new-master-password-keys" to en-US.json with the value "4 new keys (owner, active, posting, memo) from master password" and then call i18next.t("permissions.add-keys.step4.new-master-password-keys") (remove the defaultValue option) so the string is served from the translation file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48cb82ba-6c77-486c-95b7-4c16dd2cc533
⛔ Files ignored due to path filters (5)
apps/web/public/assets/hive-logo.svgis excluded by!**/*.svgpackages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
apps/web/package.jsonapps/web/public/sw.jsapps/web/src/app/(dynamicPages)/entry/_helpers/generate-entry-metadata.tsapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/account-recovery.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-2-generate-seed.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-3-review-keys.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/add-keys-steps/step-4-confirm.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-authorities.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-keys-add-keys.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsxapps/web/src/app/communities/create/_components/community-create-sign-step.tsxapps/web/src/app/layout.tsxapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsxapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsxapps/web/src/app/signup/free/_page.tsxapps/web/src/app/signup/invited/[hash]/_page.tsxapps/web/src/app/signup/invited/page.tsxapps/web/src/app/signup/premium/_page.tsxapps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsxapps/web/src/app/signup/wallet/_components/steps/metamask-connect.tsxapps/web/src/app/wallet/setup-external/_components/index.tsapps/web/src/app/wallet/setup-external/_components/setup-external-import.tsxapps/web/src/app/wallet/setup-external/_components/setup-external-metamask.tsxapps/web/src/app/wallet/setup-external/page.tsxapps/web/src/features/i18n/locales/en-US.jsonapps/web/src/features/shared/auth-upgrade/auth-upgrade-dialog.tsxapps/web/src/features/shared/index.tsapps/web/src/features/shared/key-or-hot-dialog.tsxapps/web/src/features/shared/key-or-hot/index.tsxapps/web/src/features/shared/login/hooks/use-login-by-metamask.tsapps/web/src/features/shared/login/login-user-item.tsxapps/web/src/features/shared/metamask-sign-button.tsxapps/web/src/features/shared/navbar/sidebar/navbar-side-main-logout.tsxapps/web/src/features/shared/transactions/transaction-signer.tsxapps/web/src/features/wallet/hooks/index.tsapps/web/src/features/wallet/hooks/use-download-keys.tsapps/web/src/features/wallet/operations/wallet-operations-sign.tsxapps/web/src/providers/sdk/web-broadcast-adapter.tsapps/web/src/specs/features/permissions/add-keys-flow.spec.tsxapps/web/src/specs/features/permissions/manage-key-metamask.spec.tsxapps/web/src/specs/features/permissions/step3-login-type-badge.spec.tsxapps/web/src/specs/features/shared/key-or-hot-metamask.spec.tsxapps/web/src/specs/features/shared/metamask-sign-button.spec.tsxapps/web/src/specs/features/signup/invited-page.spec.tsxapps/web/src/specs/features/signup/onboard-legacy-redirect.spec.tsxapps/web/src/specs/utils/master-password.spec.tsapps/web/src/utils/master-password.tsapps/web/src/utils/onBoard-helper.tspackages/sdk/src/modules/core/mutations/use-broadcast-mutation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/signup/invited/page.tsx
- apps/web/src/app/signup/premium/_page.tsx
Comment on lines
+52
to
+74
| async function signBufferWithSnap(message: string): Promise<string> { | ||
| // Sign an arbitrary message using the Hive snap's posting key. | ||
| // We use hive_encrypt with a byte array buffer — when given number[] instead of | ||
| // a string, the snap SHA-256 hashes the bytes and calls wallet.signDigest(), | ||
| // producing a standard Hive signature (same as signBuffer). | ||
| const bytes = Array.from(new TextEncoder().encode(message)); | ||
|
|
||
| const result = await requireEthereum().request({ | ||
| method: "wallet_invokeSnap", | ||
| params: { | ||
| snapId: HIVE_SNAP_ID, | ||
| request: { | ||
| method: "hive_encrypt", | ||
| params: { | ||
| buffer: bytes, | ||
| firstKey: { role: "posting", accountIndex: 0 } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| // hive_encrypt returns { buffer: signatureHex } when given a byte array | ||
| return (result as { buffer: string }).buffer; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the @hiveio/metamask-snapAPI officially support generating Hive signatures for arbitrary login challenges viahive_encryptwith a byte-arraybuffer, and what is the documented return shape for that call?
💡 Result:
No. The snap’s documented RPC surface does not describe hive_encrypt as a way to generate Hive signatures for arbitrary login challenges, nor does it accept a byte-array/Uint8Array-style payload. Instead, hive_encrypt is specified as encryption of “string data” (“buffer” is a string), alongside hive_decrypt, while signing is only described for Hive transactions via hive_signTransaction. [1]
For hive_encrypt, the documented return shape is:
{ buffer: string } // “Encrypted data”[1]
Sources:
[1] Hive Wallet MetaMask Snap OpenRPC spec (hive-wallet-api.openrpc.json). (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/openhive-network/metamask-snap/main/hive-wallet-api.openrpc.json
Replace hive_encrypt with the documented signing RPC or add a binding integration test.
This code uses hive_encrypt—which is documented exclusively for string data encryption—to generate Hive signatures for arbitrary login challenges by passing a number[] buffer. The snap's documented signing RPC is hive_signTransaction. The current approach relies on undocumented snap internals (byte arrays triggering signature generation via SHA-256 + wallet.signDigest()); a snap update or stricter spec compliance will silently break MetaMask login. Either switch to the documented signing RPC or add an integration test that validates the actual snap contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/shared/login/hooks/use-login-by-metamask.ts` around
lines 52 - 74, The current signBufferWithSnap function uses the undocumented
hive_encrypt path to get a signature by sending a number[] buffer; replace that
call with the documented signing RPC (use method "hive_signTransaction" on the
snap request via requireEthereum().request with HIVE_SNAP_ID) and adjust the
request.params.request payload to the documented shape for signing a
message/digest so the snap returns the signature in its documented response
format, then update the function to extract and return the signature accordingly
(or alternatively add an integration test that calls signBufferWithSnap against
a real/fixture snap to assert the signature behavior if you intentionally keep
hive_encrypt).
Member
Author
There was a problem hiding this comment.
undocumented doesn't mean it breaks... codebase is here: https://github.com/openhive-network/metamask-snap
| // Build a transaction using dhive | ||
| const props = await CONFIG.hiveClient.database.getDynamicGlobalProperties(); | ||
| const refBlockNum = props.head_block_number & 0xFFFF; | ||
| const refBlockPrefix = Buffer.from(props.head_block_id, 'hex').readUInt32LE(4); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether Buffer is explicitly polyfilled/imported for web runtime.
rg -nP -C3 '\bBuffer\.from\(' apps/web/src/providers/sdk/web-broadcast-adapter.ts
rg -nP -C3 'from ["'\'']buffer["'\'']|globalThis\.Buffer|window\.Buffer|ProvidePlugin.*Buffer' next.config.* apps/web package.jsonRepository: ecency/vision-next
Length of output: 915
🏁 Script executed:
# Check full imports and structure of web-broadcast-adapter.ts
head -30 apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Check if next.config.js exists and has Buffer polyfill
cat apps/web/next.config.js 2>/dev/null || cat next.config.js 2>/dev/null || echo "No next.config found"
# Check for the required adapter methods: getLoginType, showAuthUpgradeUI, and retry logic
rg -n "getLoginType|showAuthUpgradeUI|retry|activeAuthority" apps/web/src/providers/sdk/web-broadcast-adapter.tsRepository: ecency/vision-next
Length of output: 10275
🏁 Script executed:
# Get full context around line 136 to see the method it's in
sed -n '120,145p' apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Check what method contains this line
sed -n '100,140p' apps/web/src/providers/sdk/web-broadcast-adapter.ts | tail -20Repository: ecency/vision-next
Length of output: 1765
🏁 Script executed:
# Check all imports at the top of web-broadcast-adapter.ts to confirm Buffer is NOT imported
head -50 apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Also check if there's a polyfill or setup file that might provide Buffer globally
rg -l "Buffer|polyfill" apps/web/src/providers/ --type ts --type tsx | head -10Repository: ecency/vision-next
Length of output: 1935
🏁 Script executed:
# Check if Buffer is imported anywhere in the file
grep -n "import.*Buffer" apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Verify the complete file structure - check line count first
wc -l apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Check for any Buffer usage without import by looking at the method
sed -n '120,155p' apps/web/src/providers/sdk/web-broadcast-adapter.tsRepository: ecency/vision-next
Length of output: 1294
🏁 Script executed:
# Verify if Buffer from "buffer" package is available in node_modules
grep -r '"buffer"' apps/web/package.json
# Check if there's any global Buffer setup in layouts or providers
rg -n "Buffer|polyfill" apps/web/src/layout* apps/web/src/_app* apps/web/src/app --type ts --type tsx 2>/dev/null | head -10
# Check if the webpack config or any file provides Buffer to browser
rg -n "resolve.fallback.*buffer|ProvidePlugin.*Buffer" apps/web --type js 2>/dev/nullRepository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check dependencies for buffer package
cat apps/web/package.json | grep -A 5 -B 5 "buffer"
# Check if buffer is listed as a dependency anywhere
rg "\"buffer\"" apps/web/package.json package.json
# Check webpack config for any polyfill setup
grep -n "resolve.fallback\|ProvidePlugin" next.config.jsRepository: ecency/vision-next
Length of output: 109
🏁 Script executed:
# Check apps/web package.json for buffer dependency
cat apps/web/package.json | grep -i buffer
# Check root package.json for buffer dependency
cat package.json | grep -i buffer
# Check apps/web/next.config.js for resolve.fallback config
grep -n "resolve.fallback\|ProvidePlugin\|Buffer" apps/web/next.config.jsRepository: ecency/vision-next
Length of output: 156
🏁 Script executed:
# Get the full resolve.fallback configuration in apps/web/next.config.js
sed -n '91,105p' apps/web/next.config.js
# Also check if buffer is available in package.json dependencies
grep -E "\"buffer\"|\"bigint-buffer\"" apps/web/package.json package.jsonRepository: ecency/vision-next
Length of output: 564
🏁 Script executed:
# Check if "buffer" package is in root package.json dependencies
grep -A 200 "dependencies" package.json | grep -i "\"buffer\""
# Check for any global Buffer setup in the app initialization files
rg -n "window.Buffer\|global.Buffer\|globalThis.Buffer" apps/web/src --type ts --type tsx
# Look for the hive-auth.ts import pattern to confirm this is how Buffer should be imported
grep -A 2 "import.*Buffer" apps/web/src/utils/hive-auth.tsRepository: ecency/vision-next
Length of output: 219
🏁 Script executed:
# Confirm that Buffer is not available globally - check if it's provided by any polyfill plugin
grep -n "ProvidePlugin\|Buffer" apps/web/next.config.js
# Final confirmation: check if broadcastWithMetaMaskSnap is actually exported/used
rg -n "broadcastWithMetaMaskSnap" apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Check if this method is actually called anywhere in the codebase
rg -n "broadcastWithMetaMaskSnap" apps/web/src --type ts --type tsxRepository: ecency/vision-next
Length of output: 222
🏁 Script executed:
# Check the context around line 348 where broadcastWithMetaMaskSnap is called
sed -n '340,360p' apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Check for the grantPostingAuthority method and its retry logic
rg -n "grantPostingAuthority" apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Look for the method that contains line 348 to understand the context
sed -n '320,360p' apps/web/src/providers/sdk/web-broadcast-adapter.tsRepository: ecency/vision-next
Length of output: 2378
🏁 Script executed:
# Check the grantPostingAuthority method implementation to see if there's retry logic
sed -n '482,530p' apps/web/src/providers/sdk/web-broadcast-adapter.ts
# Also check if there's any async/await retry pattern in the full method
sed -n '475,540p' apps/web/src/providers/sdk/web-broadcast-adapter.tsRepository: ecency/vision-next
Length of output: 3899
Import Buffer from the "buffer" polyfill package in web-broadcast-adapter.ts.
The broadcastWithMetaMaskSnap() function uses Buffer.from() at line 136 but the file does not import Buffer. This will cause a ReferenceError at runtime when users attempt to sign transactions via MetaMask snap. Add import { Buffer } from "buffer"; at the top of the file, following the pattern already established in hive-auth.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/providers/sdk/web-broadcast-adapter.ts` at line 136, The file
uses Buffer.from() in the broadcastWithMetaMaskSnap flow (see const
refBlockPrefix) but does not import Buffer; add an import for Buffer from the
buffer polyfill at the top of the file (import { Buffer } from "buffer";)
following the same pattern used in hive-auth.ts so Buffer is defined when
broadcastWithMetaMaskSnap executes.
Member
Author
There was a problem hiding this comment.
But Buffer is a Node.js global that's also available in Next.js
Contributor
There was a problem hiding this comment.
@feruzm, you're right that Buffer is a global in the Node.js runtime (which Next.js uses for SSR/API routes). However, broadcastWithMetaMaskSnap runs exclusively in the browser (client-side), since it checks for window.ethereum and interacts with MetaMask — and this is where the concern applies.
Buffer is a Node.js-specific global. Before Webpack v5, build tools polyfilled it automatically; with Webpack 5 (which Next.js uses), this automatic polyfilling was removed.
Buffer is globally available in Node.js without any import, but browsers don't have Buffer — they have their own APIs for handling binary data.
The existing hive-auth.ts in the same codebase already follows the explicit import pattern precisely because of this — it imports Buffer from the "buffer" package for the client-side path.
That said, if the project's next.config.js webpack config includes a ProvidePlugin entry for Buffer (e.g., new webpack.ProvidePlugin({ Buffer: ['buffer', 'Buffer'] })), then the global would be available without an explicit import. Could you confirm whether that's configured? If so, I'll mark this comment as resolved and add a note for future reviews.
✏️ Learnings added
Learnt from: feruzm
Repo: ecency/vision-next PR: 705
File: apps/web/src/providers/sdk/web-broadcast-adapter.ts:136-136
Timestamp: 2026-03-16T16:38:59.352Z
Learning: In ecency/vision-next (apps/web), `Buffer` is NOT automatically polyfilled in the browser environment since the project uses Next.js with Webpack 5. `broadcastWithMetaMaskSnap` in `apps/web/src/providers/sdk/web-broadcast-adapter.ts` uses `Buffer.from()` in a browser-only code path (MetaMask interaction). The project team confirmed that Buffer is available as a Node.js global in server-side Next.js code, but for browser/client-side code paths, explicit import or ProvidePlugin configuration is needed. The existing `hive-auth.ts` explicitly imports Buffer from the "buffer" package.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: ecency/vision-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T09:48:26.155Z
Learning: Applies to apps/web/src/providers/sdk/web-broadcast-adapter.ts : Implement the broadcast adapter pattern with methods: `getLoginType()`, `showAuthUpgradeUI()`, and retry logic for active authority operations
Learnt from: CR
Repo: ecency/vision-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T09:48:26.155Z
Learning: Applies to apps/web/src/providers/sdk/**/*.{ts,tsx} : Use the broadcast adapter pattern in `apps/web/src/providers/sdk/web-broadcast-adapter.ts` to decouple SDK mutations from web auth
Learnt from: CR
Repo: ecency/vision-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T09:48:26.155Z
Learning: Applies to packages/render-helper/**/*.{ts,tsx} : Framework-agnostic utilities in `ecency/render-helper` should have minimal dependencies and work in any JavaScript environment (browser, Node.js, React Native)
Learnt from: feruzm
Repo: ecency/vision-next PR: 673
File: apps/web/src/defaults.ts:33-33
Timestamp: 2026-02-23T10:25:29.108Z
Learning: LocalStorage keys are auto-prefixed by the ls helper (ecency_). When using direct localStorage calls anywhere in the web app, always use the full prefixed key (e.g., 'ecency_*') to ensure consistency with the ls helper. Specifically, avoid reading or writing keys like 'image_proxy' via localStorage.getItem/setItem without the 'ecency_' prefix; instead use the fully prefixed key or route through the ls helper to maintain naming consistency.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx (1)
23-33:⚠️ Potential issue | 🟡 MinorGuard query against undefined
activeUser.The non-null assertion
activeUser?.username!on line 24 is misleading—ifactiveUseris undefined, the optional chaining still yieldsundefined, and the assertion merely silences TypeScript. This could passundefinedtogetAccountFullQueryOptions, potentially causing runtime errors or invalid API requests.Proposed fix: add `enabled` guard
const { data: accountData } = useQuery({ ...getAccountFullQueryOptions(activeUser?.username!), + enabled: !!activeUser?.username, select: (resp) =>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx around lines 23 - 33, The useQuery call that uses getAccountFullQueryOptions(activeUser?.username!) should be guarded so we don't pass undefined username: update the useQuery invocation (the call that defines accountData) to include an enabled flag like enabled: Boolean(activeUser?.username) (or !!activeUser?.username) so the query only runs when activeUser and username exist, keeping the select mapping (posting/owner/active/weight/memo) intact; this prevents calling getAccountFullQueryOptions with undefined and avoids the non-null assertion on activeUser?.username in the useQuery options.
♻️ Duplicate comments (3)
apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsx (1)
72-73:⚠️ Potential issue | 🟠 MajorRemove
anyin strict TypeScript paths (already flagged earlier).On Line 72, Line 109, and Line 118, explicit
anybypasses strict-mode guarantees in this flow.Proposed fix
- const rcStats: any = await queryClient.fetchQuery(getRcStatsQueryOptions()); + const rcStats = await queryClient.fetchQuery(getRcStatsQueryOptions()); @@ - } catch (err: any) { + } catch (err: unknown) { setModalStep("failed"); error(...formatError(err)); return; @@ - } catch (err: any) { + } catch (err: unknown) { error(...formatError(err)); }#!/bin/bash # Verify no explicit `any` remains in this file. fd 'onboard-creating.tsx' --exec rg -nP ':\s*any\b' {}As per coding guidelines, "TypeScript strict mode is enabled; all new code should include proper types".
Also applies to: 109-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx around lines 72 - 73, Replace the explicit any on rcStats with a concrete type: create an interface/type (e.g., RcStats { ops: number; /* other fields returned by getRcStatsQueryOptions() */ }) or use unknown and validate it, then cast the fetch result from queryClient.fetchQuery(getRcStatsQueryOptions()) to that type so rcStats and rcStats.ops are strongly typed; update all occurrences (rcStats declarations at rcStats, the fetchQuery call, and any later uses) to use this new type and add minimal runtime checks if needed to satisfy strict TypeScript.apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsx (2)
65-73:⚠️ Potential issue | 🟠 MajorDo not put invite identity data in an unsigned URL payload (already reported).
On Line 65-Line 73,
b64uEnc(JSON.stringify(...))is only encoding, not protection. It exposes email in browser history/referrer/logs and allows link tampering (username/referral/pubkeys) before creation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx around lines 65 - 73, The code currently builds sensitive invite data in dataToEncode and exposes it via setSecret(b64uEnc(...)) which is only encoding (b64uEnc) and not protected; replace this pattern by avoiding embedding raw identity in the URL: instead create a server-side opaque token or a signed/encrypted payload (e.g., issue a short-lived invite token from the backend that maps to decodedInfo and pubkeys) and setSecret to that token; update references to dataToEncode, setSecret, b64uEnc, accountKeys and decodedInfo so the frontend requests or receives a server-generated token (or cryptographically signs/encrypts the payload) rather than placing username/email/referral/pubkeys in an unsigned base64 URL.
55-57:⚠️ Potential issue | 🟠 MajorPrevent stale invite links and false “downloaded” state (already reported pattern).
On Line 56, when
accountKeysis unavailable the effect exits without clearingsecret; on Line 77,fileIsDownloadedis set totrueeven ifdownloadKeys()is a no-op. This can expose stale/invalid invite links.Proposed fix
useEffect(() => { - if (!accountKeys || !decodedInfo) return; + if (!accountKeys || !decodedInfo) { + setSecret(""); + return; + } @@ const handleDownload = useCallback(() => { + if (!masterPassword || !accountKeys) { + return; + } downloadKeys(); setFileIsDownloaded(true); - }, [downloadKeys]); + }, [downloadKeys, masterPassword, accountKeys]);Also applies to: 75-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx around lines 55 - 57, The effect using useEffect should clear stale state when prerequisites are missing: inside the effect that checks accountKeys and decodedInfo, ensure you reset secret (and any related state) and set fileIsDownloaded to false when accountKeys is falsy so stale invite links are not kept; additionally, update the logic around downloadKeys() so fileIsDownloaded is only set to true when downloadKeys() actually performs work or returns a success/true indicator (do not set fileIsDownloaded to true on a no-op); modify the code paths in the same component around secret, fileIsDownloaded and the downloadKeys() call to reflect these changes.
🧹 Nitpick comments (9)
apps/web/src/specs/features/shared/metamask-sign-button.spec.tsx (2)
6-18: Replaceanyin mocks with typed interfacesLine 8 and Line 12 use
anyfor props in a new TSX test file. Please type these mocks withinterface-based props to satisfy strict typing.Suggested refactor
+interface NextImageMockProps extends React.ImgHTMLAttributes<HTMLImageElement> {} + +interface ButtonMockProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { + icon?: React.ReactNode; + children?: React.ReactNode; +} + vi.mock("next/image", () => ({ __esModule: true, - default: (props: any) => <img {...props} /> + default: (props: NextImageMockProps) => <img {...props} /> })); vi.mock("@ui/button", () => ({ - Button: ({ children, onClick, icon, className, ...rest }: any) => ( + Button: ({ children, onClick, icon, className, ...rest }: ButtonMockProps) => ( <button onClick={onClick} className={className} {...rest}> {icon} {children} </button> ) }));As per coding guidelines: "
**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript" and "TypeScript strict mode is enabled; all new code should include proper types".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/features/shared/metamask-sign-button.spec.tsx` around lines 6 - 18, The mocks use untyped props (any); define proper interfaces and apply them to the mock components: create an ImageProps interface for the next/image mock and a ButtonProps interface for the `@ui/button` mock, then replace the anonymous any parameters in the default export mock and the Button mock with those interfaces (and optionally annotate the mock function as React.FC<ImageProps> and React.FC<ButtonProps>) so the test file adheres to strict TypeScript and the project guideline to prefer interface-based prop shapes.
23-26: Assert button behavior by accessible role/name instead of raw i18n key textLine 25 checks the translation key string directly, which is implementation-coupled. Prefer a role-based assertion for user-visible behavior.
Suggested refactor
it("renders sign with metamask text", () => { render(<MetaMaskSignButton onClick={vi.fn()} />); - expect(screen.getByText("key-or-hot.sign-with-metamask")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /metamask/i })).toBeInTheDocument(); });As per coding guidelines: "
apps/web/src/specs/**/*.spec.{ts,tsx}: Test user-visible behavior, not implementation details; usescreen.getByRoleovergetByTestIdwhen possible`".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/specs/features/shared/metamask-sign-button.spec.tsx` around lines 23 - 26, The test for MetaMaskSignButton is asserting the raw i18n key; update the assertion to check the button's accessible role and visible name instead: render MetaMaskSignButton (the existing render call and vi.fn() can stay) and replace screen.getByText("key-or-hot.sign-with-metamask") with screen.getByRole('button', { name: /sign with metamask/i }) (or an appropriate regex/locale-aware string) to assert user-visible behavior rather than the implementation detail.apps/web/src/features/shared/login/login.tsx (1)
213-213: Add.catch()for consistency with existing login handlers.The
loginByMetaMask()call returns a Promise without a.catch()handler. While React Query'sonErrorhandles the error for display purposes, the Promise itself still rejects. For consistency withhandleKeychainLogin()(line 54) which uses.catch(() => { /* Already handled */ }), consider wrapping this similarly:- onClick={() => !!username && loginByMetaMask()} + onClick={() => !!username && loginByMetaMask().catch(() => { /* Already handled in onError */ })}Alternatively, extract to a
handleMetaMaskLoginfunction similar tohandleKeychainLoginfor better readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/login/login.tsx` at line 213, The onClick currently calls loginByMetaMask() directly which returns a Promise without a .catch(), so add a small wrapper similar to handleKeychainLogin: either create a handleMetaMaskLogin function that calls loginByMetaMask().catch(() => { /* Already handled */ }) and use that in onClick, or append .catch(() => { /* Already handled */ }) to the existing call; reference the loginByMetaMask invocation and mirror the error-swallowing pattern used by handleKeychainLogin for consistency.apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx (1)
39-45: Remove duplicategetLoginTypecall.
getLoginTypeis called twice with the same argument (lines 39 and 45). DeriveisMetamaskfromloginTypeinstead.Proposed fix
- const isMetamask = getLoginType(activeUser?.username ?? "") === "metamask"; - const [showReveal, setShowReveal] = useState(false); const [showRevoke, setShowRevoke] = useState(false); const [revokingKey, setRevokingKey] = useState(""); const loginType = getLoginType(activeUser?.username ?? ""); + const isMetamask = loginType === "metamask";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx around lines 39 - 45, Compute loginType once and reuse it: call getLoginType(activeUser?.username ?? "") to set loginType, then derive isMetamask from that value (e.g., isMetamask = loginType === "metamask"), removing the duplicate getLoginType call so only loginType and isMetamask are defined and used.apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx (1)
18-24: ExtractverifiedWalletinto a dedicated interface.Inline object typing in
Propsmakes reuse and strict typing harder as this flow grows.💡 Proposed refactor
+interface VerifiedWallet { + currency: EcencyWalletCurrency; + address: string; + addresses: Partial<Record<EcencyWalletCurrency, string>>; +} + interface Props { username: string; - verifiedWallet: { - currency: EcencyWalletCurrency; - address: string; - addresses: Partial<Record<EcencyWalletCurrency, string>>; - }; + verifiedWallet: VerifiedWallet; }As per coding guidelines:
**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx` around lines 18 - 24, Props currently in metamask-account-creating.tsx uses an inline object type for verifiedWallet which reduces reuse and strict typing; extract that shape into a dedicated interface (e.g., VerifiedWallet) that describes currency: EcencyWalletCurrency, address: string, addresses: Partial<Record<EcencyWalletCurrency, string>> and replace the inline type in the Props interface with this new VerifiedWallet interface, updating any references to Props or verifiedWallet in the component or related types (e.g., functions or props usage) to use the renamed interface.apps/web/src/features/shared/key-or-hot/index.tsx (2)
51-96: LGTM!The conditional rendering logic correctly shows the alternative signing options section only when at least one option is available. The guard
{onHot && (on line 55 properly prevents rendering the HiveSign button without a handler.Minor inconsistency: line 60 uses
onClick={() => onHot()}while line 80 usesonClick={onKc}directly. Consider using direct references for both for consistency.Optional: Consistent onClick handler style
- onClick={() => onHot()} + onClick={onHot}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/key-or-hot/index.tsx` around lines 51 - 96, Change the inconsistent onClick handler style by using a direct function reference for the HiveSign button instead of wrapping it in an arrow thunk; replace onClick={() => onHot()} with onClick={onHot} so both Button elements use direct handler references (symbols: onHot and onKc, components: the Button rendering the HiveSign and the Button rendering the Keychain).
39-45: MetaMask users bypass the key input option entirely.When a MetaMask user accesses this component (and
onMetaMaskis provided), they only see the MetaMask button with no option to sign with a private key directly. Verify this is the intended UX—if MetaMask users should optionally use key-based signing as a fallback, consider renderingKeyInputabove the MetaMask button similar to the non-MetaMask flow.Also, the arrow function wrapper on line 42 is unnecessary since
onMetaMaskis already verified to exist.Minor cleanup for onClick handler
- <MetaMaskSignButton onClick={() => onMetaMask()} /> + <MetaMaskSignButton onClick={onMetaMask} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/shared/key-or-hot/index.tsx` around lines 39 - 45, Currently when isMetaMaskUser && onMetaMask && !keyOnly the component only returns the MetaMask button, which hides the KeyInput fallback; change the render so that when isMetaMaskUser and onMetaMask are present and keyOnly is false you render KeyInput above the MetaMaskSignButton (matching the non-MetaMask flow) to allow key-based signing as a fallback, and simplify the MetaMask button handler by passing onMetaMask directly (replace onClick={() => onMetaMask()} with onClick={onMetaMask}); update references in this component to KeyInput, MetaMaskSignButton, isMetaMaskUser, onMetaMask, and keyOnly accordingly.apps/web/src/app/publish/_components/publish-success-state.tsx (1)
35-41: Use camelCase for the share-button collection constant.Line 35 uses
SHARE_BUTTONS; rename it toshareButtonsto match repository naming rules.As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use camelCase for variable and function names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/publish/_components/publish-success-state.tsx` around lines 35 - 41, Rename the constant SHARE_BUTTONS to camelCase shareButtons and update all local references to it (e.g., where it's iterated/rendered in the PublishSuccessState component or any helper functions in this file); keep the array contents and the "as const" assertion unchanged and ensure any exported symbol name is updated accordingly if this constant is exported or imported elsewhere; verify usages that reference SHARE_BUTTONS (such as map calls using makeShareUrlTwitter/makeShareUrlReddit/etc.) are updated to use shareButtons.apps/web/src/app/publish/_components/publish-validate-post.tsx (1)
24-24: Extract and reuse a sharedPublishEntryInfointerface foronSuccess.Line 24 inlines a shape that is also repeated in
apps/web/src/app/publish/_page.tsx(Line 37) andapps/web/src/app/publish/drafts/[id]/_page.tsx(Line 30). Centralizing this interface will prevent type drift across the publish flow.As per coding guidelines,
**/*.{ts,tsx}: Prefer interface for defining object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/publish/_components/publish-validate-post.tsx` at line 24, Create and export a shared interface named PublishEntryInfo with the properties { title: string; author: string; permlink: string; category: string } in a central publish types module, then replace the inline object type in the onSuccess signature in publish-validate-post.tsx with entryInfo?: PublishEntryInfo; finally update the other publish flow modules (the publish page and drafts page where the same shape is repeated) to import and use PublishEntryInfo so all onSuccess signatures and related usages share the same type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx:
- Around line 117-162: The grid uses two columns (grid-cols-[1fr_max-content])
but for non-MetaMask users the revoke button (StyledTooltip + Button with
UilTrash that calls setShowRevoke and setRevokingKey) is rendered outside the
fragment that contains the private key display and the copy/reveal button
container, causing it to wrap; move the revoke StyledTooltip/Button into the
same action container div (the <div className="flex gap-2"> that renders
copy/reveal) so all action buttons (copy/reveal/revoke) render together in the
second column and keep the conditional display tied to the non-MetaMask branch
(use the same isMetamask and keys[key[0]] checks as appropriate).
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx:
- Around line 49-50: Change the catch clause in onboard-asking.tsx from catch
(err: any) to catch (err: unknown) and narrow the error before calling
error(...): if err is an instance of Error use err.message, otherwise use the
translation fallback key "g.error" (not "g.unknown-error"); update the call
sites around the error(...) invocation and use any existing translator function
(e.g., t) as needed to produce the fallback message.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx:
- Around line 63-69: The effect is calling rcOperationsCost() without handling
rejections (rcOperationsCost and the fetchQuery it uses can throw), so wrap the
call in an async handler and guard failures: inside the useEffect create and
invoke an async IIFE that try/catches await rcOperationsCost(), and on error
call setRcAmount(0) (or a safe fallback) and optionally set an error state; also
ensure rcOperationsCost itself catches or re-throws with context from fetchQuery
so callers can handle it. Reference: useEffect, rcOperationsCost, fetchQuery,
setRcAmount, isChecked, rcAmount.
- Line 175: The component currently embeds a fallback English string in the
i18next.t call (placeholder={i18next.t("onboard.delegate-placeholder", {
defaultValue: "Enter amount to delegate (Bn)" })}) — remove the inline
defaultValue, add the "onboard.delegate-placeholder" entry with the English text
"Enter amount to delegate (Bn)" to en-US.json, and update the component to call
i18next.t("onboard.delegate-placeholder") (locate this in onboard-creating.tsx
where the placeholder prop and i18next.t are used).
In `@apps/web/src/app/onboard-friend/`[...slugs]/_page.tsx:
- Around line 26-30: The redirect logic in the component wrongly maps
/onboard-friend/creating (when type === "creating" and slugs[1] is missing) to
`/signup/invited/creating`; update the router.replace calls so that when type
=== "creating" and there is no slugs[1] you explicitly redirect to
`/signup/invited` instead of using hash or slugs[1]. Locate the block using
type, slugs, and router.replace (in _page.tsx) and add a branch (or adjust the
existing else-if/else) that checks for type === "creating" && !slugs[1] and
calls router.replace('/signup/invited').
In `@apps/web/src/app/publish/_components/publish-success-state.tsx`:
- Around line 59-66: The anchor elements in publish-success-state.tsx (the
icon-only link rendered with key={id}, href={url}, title={label}) lack an
explicit accessible name for assistive tech; add aria-label={label} to the
anchor (the same element that has key={id} and className="w-10 h-10 ...") so
screen readers receive a clear name for each icon-only share link alongside the
existing title.
- Around line 36-40: Replace hardcoded UI strings for share buttons and the
fallback title with i18n keys: move labels "Twitter", "Reddit", "Facebook",
"LinkedIn", "Discord" and the "Share your post" fallback into en-US.json and
load them via the app's translation hook. In publish-success-state.tsx, replace
the literal label values in the share array entries (the objects with id
"twitter"/"reddit"/"facebook"/"linkedin"/"discord" and makeFn like
makeShareUrlTwitter, makeShareUrlReddit, etc.) with calls to the i18n getter
(e.g., t('publish.share.twitter')) and replace the hardcoded fallback title
("Share your post") with t('publish.share.fallbackTitle'); add corresponding
keys in en-US.json only and ensure keys follow existing naming conventions.
- Around line 43-74: Add unit tests for the ShareBar component and the
conditional render that shows it when step === "published" and entryInfo exists:
test the parent PublishSuccessState (or the component that contains the step
check) to assert ShareBar is rendered only when step === "published" and not
otherwise; mount ShareBar directly to verify each SHARE_BUTTONS entry produces
the correct href by comparing URLs produced by makeShareUrlTwitter (for buttons
that require title) and makeShareUrlFacebook (for those that don't), and assert
each share button is accessible via screen.getByRole('link', { name: /label/i })
or equivalent accessible queries; cover Twitter/Reddit path (makeShareUrlTwitter
signature uses title) and Facebook/LinkedIn/Discord path (makeShareUrlFacebook
signature) and ensure aria/title values match the SHARE_BUTTONS label for
accessibility.
In `@apps/web/src/app/signup/free/_page.tsx`:
- Around line 163-165: The code uses inline casts for the caught error (e) like
(e as { data?: { message?: string } }) which weakens strict typing; define a
named interface (e.g., interface ApiError { data?: { message?: string } }) and
add a type guard function (e.g., function isApiError(val: unknown): val is
ApiError) that checks for the data.message shape, then replace the inline cast
with the type guard (if (e instanceof Error && isApiError(e)) { ... }); apply
the same pattern for the other occurrences mentioned (around the other checks at
the referenced locations) to remove any/inline shapes and satisfy strict-mode
and style rules.
- Around line 68-77: The debounced username check can return out-of-order and
lacks error handling; modify the useDebounce callback that calls
queryClient.fetchQuery(getAccountsQueryOptions([username])) to track the
most-recent username (e.g., a latestUsernameRef set before issuing the query)
and when the promise resolves compare the returned result against
latestUsernameRef.current and only call setUsernameError / setIsDisabled if it
still matches; also wrap the fetchQuery call in try/catch to handle query
failures (clear or set a generic error and setIsDisabled appropriately) so stale
responses and errors are ignored/handled correctly.
- Around line 21-278: Add a new test file (e.g., FreeSignUp.spec.tsx) to cover
the FreeSignUp component: mount FreeSignUp and assert form validation
(username/email/referral error states), async uniqueness checks by mocking
getAccountsQueryOptions responses via the queryClient fetchQuery, submission
flow by mocking signUp success and error responses, CAPTCHA behavior by mocking
ReCAPTCHA onChange to toggle isVerified, and success UI (done true) and
registrationError displays; also stub hooks that interact with
navigation/storage (useSearchParams, useRouter, useLocalStorage, useQueryClient)
to control referral handling and redirection, and ensure the submit button
enable/disable logic (inProgress, isDisabled, usernameError, emailError,
referralError) is asserted.
- Around line 150-156: The signUp call currently only sends {username, email,
referral} and ignores the reCAPTCHA token, making client-side isVerified
bypassable; update the implementation so the reCAPTCHA token is included and
validated server-side: modify the signUp call sites in _page.tsx (the
signUp(username, email, referral) invocations) to pass the token (e.g.,
signUp(username, email, referral, captchaToken)) and update the `@ecency/sdk`
signUp function signature to accept a captchaToken parameter and include it in
the request body so the server can verify it; ensure all other occurrences
(around the other mentioned locations) are updated consistently.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`:
- Around line 176-178: The current if (!response.ok) branches in
metamask-account-creating.tsx throw a generic Error and drop server-side
details; update both occurrences to read the response body (prefer JSON with
await response.json(), falling back to await response.text() if needed), extract
a meaningful message or error field, and include it plus
response.status/statusText in the thrown Error (or pass into your existing
error-handling/logging). Locate the two blocks using the response variable in
this file and replace the generic throw with creating an Error that contains the
backend message and status for better user/log visibility.
- Around line 111-127: The polling loop in validateAccountIsCreated can run
indefinitely; add a cap by tracking either a maxAttempts counter (e.g., const
maxAttempts = 12) or a deadline (const timeoutMs = 60000, const deadline =
Date.now()+timeoutMs) before entering the while, incrementing attempts (or
checking Date.now() > deadline) on each iteration after the catch/delay, and if
exceeded throw a new Error("Account validation timed out") with name =
"TimeoutError"; keep using accountQueryOptions and queryClient.fetchQuery as-is
and ensure the shouldStop check still breaks the loop early.
In `@apps/web/src/features/shared/key-or-hot/index.tsx`:
- Around line 24-27: The authority prop must be made required to force callers
to specify key level: update the Props type by removing the optional marker from
authority (authority: "owner" | "active") and remove the default value from the
KeyOrHot function signature so KeyOrHot({ ..., authority }: Props) expects an
explicit value; then update all call sites (e.g., where KeyOrHot is used in
account-recovery and manage-authorities flows) to pass the correct authority
("owner" or "active") so sensitive operations request the proper key type.
In `@apps/web/src/features/shared/login/login.tsx`:
- Around line 206-228: Replace the direct window check with a client-only state
flag to avoid hydration mismatch: add a state variable (e.g.,
isMetaMaskAvailable via useState(false)) and set it inside useEffect to
!!window.ethereum?.isMetaMask on mount, then change the render conditional that
currently uses typeof window !== "undefined" && window.ethereum?.isMetaMask to
use isMetaMaskAvailable; keep existing handlers/props like loginByMetaMask,
isLoginByMetaMaskPending and username unchanged so button behavior is preserved.
- Line 226: The UI uses the i18n key "key-or-hot.sign-with-metamask" in
login.tsx but that key is missing from the locale file; add "sign-with-metamask"
under the "key-or-hot" namespace in en-US.json (alongside the related keys) with
the English string "Sign with MetaMask" so
i18next.t("key-or-hot.sign-with-metamask") resolves correctly.
---
Outside diff comments:
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx:
- Around line 23-33: The useQuery call that uses
getAccountFullQueryOptions(activeUser?.username!) should be guarded so we don't
pass undefined username: update the useQuery invocation (the call that defines
accountData) to include an enabled flag like enabled:
Boolean(activeUser?.username) (or !!activeUser?.username) so the query only runs
when activeUser and username exist, keeping the select mapping
(posting/owner/active/weight/memo) intact; this prevents calling
getAccountFullQueryOptions with undefined and avoids the non-null assertion on
activeUser?.username in the useQuery options.
---
Duplicate comments:
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx:
- Around line 65-73: The code currently builds sensitive invite data in
dataToEncode and exposes it via setSecret(b64uEnc(...)) which is only encoding
(b64uEnc) and not protected; replace this pattern by avoiding embedding raw
identity in the URL: instead create a server-side opaque token or a
signed/encrypted payload (e.g., issue a short-lived invite token from the
backend that maps to decodedInfo and pubkeys) and setSecret to that token;
update references to dataToEncode, setSecret, b64uEnc, accountKeys and
decodedInfo so the frontend requests or receives a server-generated token (or
cryptographically signs/encrypts the payload) rather than placing
username/email/referral/pubkeys in an unsigned base64 URL.
- Around line 55-57: The effect using useEffect should clear stale state when
prerequisites are missing: inside the effect that checks accountKeys and
decodedInfo, ensure you reset secret (and any related state) and set
fileIsDownloaded to false when accountKeys is falsy so stale invite links are
not kept; additionally, update the logic around downloadKeys() so
fileIsDownloaded is only set to true when downloadKeys() actually performs work
or returns a success/true indicator (do not set fileIsDownloaded to true on a
no-op); modify the code paths in the same component around secret,
fileIsDownloaded and the downloadKeys() call to reflect these changes.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx:
- Around line 72-73: Replace the explicit any on rcStats with a concrete type:
create an interface/type (e.g., RcStats { ops: number; /* other fields returned
by getRcStatsQueryOptions() */ }) or use unknown and validate it, then cast the
fetch result from queryClient.fetchQuery(getRcStatsQueryOptions()) to that type
so rcStats and rcStats.ops are strongly typed; update all occurrences (rcStats
declarations at rcStats, the fetchQuery call, and any later uses) to use this
new type and add minimal runtime checks if needed to satisfy strict TypeScript.
---
Nitpick comments:
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsx:
- Around line 39-45: Compute loginType once and reuse it: call
getLoginType(activeUser?.username ?? "") to set loginType, then derive
isMetamask from that value (e.g., isMetamask = loginType === "metamask"),
removing the duplicate getLoginType call so only loginType and isMetamask are
defined and used.
In `@apps/web/src/app/publish/_components/publish-success-state.tsx`:
- Around line 35-41: Rename the constant SHARE_BUTTONS to camelCase shareButtons
and update all local references to it (e.g., where it's iterated/rendered in the
PublishSuccessState component or any helper functions in this file); keep the
array contents and the "as const" assertion unchanged and ensure any exported
symbol name is updated accordingly if this constant is exported or imported
elsewhere; verify usages that reference SHARE_BUTTONS (such as map calls using
makeShareUrlTwitter/makeShareUrlReddit/etc.) are updated to use shareButtons.
In `@apps/web/src/app/publish/_components/publish-validate-post.tsx`:
- Line 24: Create and export a shared interface named PublishEntryInfo with the
properties { title: string; author: string; permlink: string; category: string }
in a central publish types module, then replace the inline object type in the
onSuccess signature in publish-validate-post.tsx with entryInfo?:
PublishEntryInfo; finally update the other publish flow modules (the publish
page and drafts page where the same shape is repeated) to import and use
PublishEntryInfo so all onSuccess signatures and related usages share the same
type.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`:
- Around line 18-24: Props currently in metamask-account-creating.tsx uses an
inline object type for verifiedWallet which reduces reuse and strict typing;
extract that shape into a dedicated interface (e.g., VerifiedWallet) that
describes currency: EcencyWalletCurrency, address: string, addresses:
Partial<Record<EcencyWalletCurrency, string>> and replace the inline type in the
Props interface with this new VerifiedWallet interface, updating any references
to Props or verifiedWallet in the component or related types (e.g., functions or
props usage) to use the renamed interface.
In `@apps/web/src/features/shared/key-or-hot/index.tsx`:
- Around line 51-96: Change the inconsistent onClick handler style by using a
direct function reference for the HiveSign button instead of wrapping it in an
arrow thunk; replace onClick={() => onHot()} with onClick={onHot} so both Button
elements use direct handler references (symbols: onHot and onKc, components: the
Button rendering the HiveSign and the Button rendering the Keychain).
- Around line 39-45: Currently when isMetaMaskUser && onMetaMask && !keyOnly the
component only returns the MetaMask button, which hides the KeyInput fallback;
change the render so that when isMetaMaskUser and onMetaMask are present and
keyOnly is false you render KeyInput above the MetaMaskSignButton (matching the
non-MetaMask flow) to allow key-based signing as a fallback, and simplify the
MetaMask button handler by passing onMetaMask directly (replace onClick={() =>
onMetaMask()} with onClick={onMetaMask}); update references in this component to
KeyInput, MetaMaskSignButton, isMetaMaskUser, onMetaMask, and keyOnly
accordingly.
In `@apps/web/src/features/shared/login/login.tsx`:
- Line 213: The onClick currently calls loginByMetaMask() directly which returns
a Promise without a .catch(), so add a small wrapper similar to
handleKeychainLogin: either create a handleMetaMaskLogin function that calls
loginByMetaMask().catch(() => { /* Already handled */ }) and use that in
onClick, or append .catch(() => { /* Already handled */ }) to the existing call;
reference the loginByMetaMask invocation and mirror the error-swallowing pattern
used by handleKeychainLogin for consistency.
In `@apps/web/src/specs/features/shared/metamask-sign-button.spec.tsx`:
- Around line 6-18: The mocks use untyped props (any); define proper interfaces
and apply them to the mock components: create an ImageProps interface for the
next/image mock and a ButtonProps interface for the `@ui/button` mock, then
replace the anonymous any parameters in the default export mock and the Button
mock with those interfaces (and optionally annotate the mock function as
React.FC<ImageProps> and React.FC<ButtonProps>) so the test file adheres to
strict TypeScript and the project guideline to prefer interface-based prop
shapes.
- Around line 23-26: The test for MetaMaskSignButton is asserting the raw i18n
key; update the assertion to check the button's accessible role and visible name
instead: render MetaMaskSignButton (the existing render call and vi.fn() can
stay) and replace screen.getByText("key-or-hot.sign-with-metamask") with
screen.getByRole('button', { name: /sign with metamask/i }) (or an appropriate
regex/locale-aware string) to assert user-visible behavior rather than the
implementation detail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0f23d25-7e21-414d-897a-f9d1110286f9
📒 Files selected for processing (22)
apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-key.tsxapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsxapps/web/src/app/onboard-friend/[...slugs]/_components/onboard-creating.tsxapps/web/src/app/onboard-friend/[...slugs]/_page.tsxapps/web/src/app/publish/_components/publish-success-state.tsxapps/web/src/app/publish/_components/publish-validate-post.tsxapps/web/src/app/publish/_page.tsxapps/web/src/app/publish/drafts/[id]/_page.tsxapps/web/src/app/signup/free/_page.tsxapps/web/src/app/signup/invited/[hash]/_page.tsxapps/web/src/app/signup/invited/page.tsxapps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsxapps/web/src/features/shared/key-or-hot/index.tsxapps/web/src/features/shared/login/login.tsxapps/web/src/specs/features/permissions/add-keys-flow.spec.tsxapps/web/src/specs/features/permissions/manage-key-metamask.spec.tsxapps/web/src/specs/features/permissions/step3-login-type-badge.spec.tsxapps/web/src/specs/features/shared/key-or-hot-metamask.spec.tsxapps/web/src/specs/features/shared/metamask-sign-button.spec.tsxapps/web/src/specs/features/signup/invited-page.spec.tsxapps/web/src/specs/features/signup/onboard-legacy-redirect.spec.tsxapps/web/src/specs/utils/master-password.spec.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/src/specs/features/shared/key-or-hot-metamask.spec.tsx
- apps/web/src/specs/features/permissions/add-keys-flow.spec.tsx
- apps/web/src/specs/features/permissions/step3-login-type-badge.spec.tsx
- apps/web/src/app/signup/invited/page.tsx
- apps/web/src/specs/features/signup/invited-page.spec.tsx
- apps/web/src/specs/utils/master-password.spec.ts
- apps/web/src/specs/features/signup/onboard-legacy-redirect.spec.tsx
- apps/web/src/specs/features/permissions/manage-key-metamask.spec.tsx
- apps/web/src/app/signup/invited/[hash]/_page.tsx
Comment on lines
+49
to
+50
| } catch (err: any) { | ||
| error(err?.message); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no explicit `any` remains in this file.
fd 'onboard-asking.tsx' --exec rg -nP ':\s*any\b' {}Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Read the file to check context and imports
cat -n apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsx | head -60Repository: ecency/vision-next
Length of output: 2404
🏁 Script executed:
# Check for all `any` occurrences (not just in catch handlers)
rg -nP '\bany\b' apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsxRepository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Check if i18next is imported or available in the file
rg -n "import.*i18next|import.*useTranslation|from.*i18n" apps/web/src/app/onboard-friend/[...slugs]/_components/onboard-asking.tsxRepository: ecency/vision-next
Length of output: 94
🏁 Script executed:
# Find the translation file and check for "unknown-error" or "error" keys
fd 'en-US.json' --exec grep -i "unknown.error\|error" {} | head -20Repository: ecency/vision-next
Length of output: 1578
🏁 Script executed:
# Check how error messages are typically handled with i18next in similar files
rg "i18next\.t\(" apps/web/src --type ts --type tsx -A 1 | head -30Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Check how error handling with i18next is done in similar catch blocks
rg "catch.*unknown|catch.*err.*unknown" apps/web/src --type ts --type tsx -A 2 | head -30Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Look for i18next usage in error handling contexts
rg "i18next\.t.*error|error.*i18next\.t" apps/web/src -A 1 -B 1 | head -40Repository: ecency/vision-next
Length of output: 3538
🏁 Script executed:
# Check what generic error key is used in the translation
grep -E '"error":|"g\.|"error-' apps/web/src/locales/en-US.json | head -20Repository: ecency/vision-next
Length of output: 126
🏁 Script executed:
# Find the en-US.json file
fd 'en-US.json' -type fRepository: ecency/vision-next
Length of output: 232
🏁 Script executed:
# Check for general error key patterns in translation files
fd 'en-US|en\.json' -type f -exec grep -l '"error"' {} \;Repository: ecency/vision-next
Length of output: 232
🏁 Script executed:
# Find the en-US.json file
find . -name "*en-US*" -o -name "*en.json"Repository: ecency/vision-next
Length of output: 109
🏁 Script executed:
# Look for translation files
find . -path "*/locales/*" -name "*.json"Repository: ecency/vision-next
Length of output: 1215
🏁 Script executed:
# Check for "g." prefixed keys and error-related keys in the translation file
grep -E '"g\.|"error' apps/web/src/features/i18n/locales/en-US.json | head -20Repository: ecency/vision-next
Length of output: 1440
🏁 Script executed:
# Verify if "g.unknown-error" exists as a translation key
grep -r '"g\.unknown-error"' apps/web/src/features/i18n/locales/Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check how error handling with unknown is typically done in the codebase
rg "err instanceof Error" apps/web/src -B 2 -A 2 | head -40Repository: ecency/vision-next
Length of output: 2274
🏁 Script executed:
# Check what "g." namespace contains
grep -A 5 '"g":' apps/web/src/features/i18n/locales/en-US.json | head -30Repository: ecency/vision-next
Length of output: 171
🏁 Script executed:
# Look for "Unknown error" or similar generic error messages
grep -i "unknown\|generic" apps/web/src/features/i18n/locales/en-US.jsonRepository: ecency/vision-next
Length of output: 505
🏁 Script executed:
# Get the full "g" namespace from the translation file
grep -A 100 '"g": {' apps/web/src/features/i18n/locales/en-US.json | grep -B 1 '^ }' | head -80Repository: ecency/vision-next
Length of output: 44
🏁 Script executed:
# Check what pattern is used in similar error handling in the onboard flow
rg "onboard" apps/web/src -A 5 -B 5 | grep -A 10 -B 10 "catch\|error"Repository: ecency/vision-next
Length of output: 46404
🏁 Script executed:
# Check the "g" namespace completely to see all available keys
sed -n '/"g": {/,/^ },/p' apps/web/src/features/i18n/locales/en-US.jsonRepository: ecency/vision-next
Length of output: 2975
Use unknown instead of any in catch handlers.
On Line 49, catch (err: any) weakens strict-mode guarantees. Prefer unknown, then narrow for messaging.
Proposed fix
- } catch (err: any) {
- error(err?.message);
+ } catch (err: unknown) {
+ const message = err instanceof Error ? err.message : i18next.t("g.error");
+ error(message);
return null;
}The translation key g.error (not g.unknown-error) is the appropriate fallback key for generic errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (err: any) { | |
| error(err?.message); | |
| } catch (err: unknown) { | |
| const message = err instanceof Error ? err.message : i18next.t("g.error"); | |
| error(message); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-asking.tsx
around lines 49 - 50, Change the catch clause in onboard-asking.tsx from catch
(err: any) to catch (err: unknown) and narrow the error before calling
error(...): if err is an instance of Error use err.message, otherwise use the
translation fallback key "g.error" (not "g.unknown-error"); update the call
sites around the error(...) invocation and use any existing translator function
(e.g., t) as needed to produce the fallback message.
Comment on lines
+63
to
+69
| useEffect(() => { | ||
| if (!isChecked) { | ||
| setRcAmount(0); | ||
| return; | ||
| } | ||
| rcOperationsCost(); | ||
| }, [isChecked, rcAmount]); |
Contributor
There was a problem hiding this comment.
Guard rcOperationsCost() failures to avoid unhandled promise rejections.
On Line 68, rcOperationsCost() is fired from useEffect without rejection handling, and on Line 72 fetchQuery(...) can reject. This can leave RC state inconsistent and produce unhandled runtime errors.
Proposed fix
useEffect(() => {
if (!isChecked) {
setRcAmount(0);
+ setRcError("");
return;
}
- rcOperationsCost();
+ void rcOperationsCost();
}, [isChecked, rcAmount]);
const rcOperationsCost = async () => {
- const rcStats: any = await queryClient.fetchQuery(getRcStatsQueryOptions());
- const ops = rcStats.ops;
+ try {
+ const rcStats = await queryClient.fetchQuery(getRcStatsQueryOptions());
+ const ops = rcStats.ops;
- if (isNaN(rcAmount) || rcAmount * 1e9 < 5000000000) {
- setRcError(i18next.t("onboard.rc-error"));
- } else {
- setRcError("");
- }
+ if (isNaN(rcAmount) || rcAmount * 1e9 < 5000000000) {
+ setRcError(i18next.t("onboard.rc-error"));
+ } else {
+ setRcError("");
+ }
- setCommentAmount(Math.ceil((rcAmount * 1e9) / ops.comment_operation.avg_cost));
- setVoteAmount(Math.ceil((rcAmount * 1e9) / ops.vote_operation.avg_cost));
- setTransferAmount(Math.ceil((rcAmount * 1e9) / ops.transfer_operation.avg_cost));
- setCustomJsonAmount(Math.ceil((rcAmount * 1e9) / ops.custom_json_operation.avg_cost));
+ setCommentAmount(Math.ceil((rcAmount * 1e9) / ops.comment_operation.avg_cost));
+ setVoteAmount(Math.ceil((rcAmount * 1e9) / ops.vote_operation.avg_cost));
+ setTransferAmount(Math.ceil((rcAmount * 1e9) / ops.transfer_operation.avg_cost));
+ setCustomJsonAmount(Math.ceil((rcAmount * 1e9) / ops.custom_json_operation.avg_cost));
+ } catch (err) {
+ setRcError(i18next.t("onboard.rc-error"));
+ error(...formatError(err));
+ }
};Also applies to: 71-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx
around lines 63 - 69, The effect is calling rcOperationsCost() without handling
rejections (rcOperationsCost and the fetchQuery it uses can throw), so wrap the
call in an async handler and guard failures: inside the useEffect create and
invoke an async IIFE that try/catches await rcOperationsCost(), and on error
call setRcAmount(0) (or a safe fallback) and optionally set an error state; also
ensure rcOperationsCost itself catches or re-throws with context from fetchQuery
so callers can handle it. Reference: useEffect, rcOperationsCost, fetchQuery,
setRcAmount, isChecked, rcAmount.
| <InputGroup> | ||
| <FormControl | ||
| type="text" | ||
| placeholder={i18next.t("onboard.delegate-placeholder", { defaultValue: "Enter amount to delegate (Bn)" })} |
Contributor
There was a problem hiding this comment.
Avoid embedding new fallback English UI text in component code.
On Line 175, defaultValue: "Enter amount to delegate (Bn)" introduces a new UI string in code. Move it to en-US.json and reference only the translation key.
As per coding guidelines, "apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/onboard-friend/`[...slugs]/_components/onboard-creating.tsx
at line 175, The component currently embeds a fallback English string in the
i18next.t call (placeholder={i18next.t("onboard.delegate-placeholder", {
defaultValue: "Enter amount to delegate (Bn)" })}) — remove the inline
defaultValue, add the "onboard.delegate-placeholder" entry with the English text
"Enter amount to delegate (Bn)" to en-US.json, and update the component to call
i18next.t("onboard.delegate-placeholder") (locate this in onboard-creating.tsx
where the placeholder prop and i18next.t are used).
Comment on lines
+111
to
+127
| while (!shouldStop?.()) { | ||
| try { | ||
| const account = await queryClient.fetchQuery(accountQueryOptions); | ||
| if (account) { | ||
| queryClient.setQueryData(accountQueryOptions.queryKey, account); | ||
| return account as unknown as FullAccountEntity; | ||
| } | ||
| } catch { | ||
| /* Account might not yet exist; retry */ | ||
| } | ||
| await delay(5000); | ||
| } | ||
|
|
||
| const abortError = new Error("Account validation aborted"); | ||
| abortError.name = "AbortError"; | ||
| throw abortError; | ||
| }, |
Contributor
There was a problem hiding this comment.
Add a timeout/attempt cap to account validation polling.
validateAccountIsCreated can run forever while mounted (Line 111 onward), which can trap the user in “validating” and continuously hit the backend.
💡 Proposed fix
async function validateAccountIsCreated = useCallback(
async (shouldStop?: () => boolean): Promise<FullAccountEntity> => {
const accountQueryOptions = getAccountFullQueryOptions(username);
const queryClient = getQueryClient();
+ const maxAttempts = 24; // ~2 minutes at 5s interval
- while (!shouldStop?.()) {
+ for (let attempt = 0; attempt < maxAttempts && !shouldStop?.(); attempt++) {
try {
const account = await queryClient.fetchQuery(accountQueryOptions);
if (account) {
queryClient.setQueryData(accountQueryOptions.queryKey, account);
return account as unknown as FullAccountEntity;
}
} catch {
/* Account might not yet exist; retry */
}
await delay(5000);
}
- const abortError = new Error("Account validation aborted");
- abortError.name = "AbortError";
- throw abortError;
+ if (shouldStop?.()) {
+ const abortError = new Error("Account validation aborted");
+ abortError.name = "AbortError";
+ throw abortError;
+ }
+
+ throw new Error("Account creation timed out");
},
[username]
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`
around lines 111 - 127, The polling loop in validateAccountIsCreated can run
indefinitely; add a cap by tracking either a maxAttempts counter (e.g., const
maxAttempts = 12) or a deadline (const timeoutMs = 60000, const deadline =
Date.now()+timeoutMs) before entering the while, incrementing attempts (or
checking Date.now() > deadline) on each iteration after the catch/delay, and if
exceeded throw a new Error("Account validation timed out") with name =
"TimeoutError"; keep using accountQueryOptions and queryClient.fetchQuery as-is
and ensure the shouldStop check still breaks the loop early.
Comment on lines
+176
to
+178
| if (!response.ok) { | ||
| throw new Error("Account creation failed"); | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve backend error details for account-creation failures.
The current path throws a generic error on non-OK responses, so user-facing and logs lose the actual server reason.
💡 Proposed fix
- if (!response.ok) {
- throw new Error("Account creation failed");
- }
+ if (!response.ok) {
+ const message = await response.text().catch(() => "");
+ throw new Error(message || "Account creation failed");
+ }Also applies to: 195-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/signup/wallet/_components/steps/metamask-account-creating.tsx`
around lines 176 - 178, The current if (!response.ok) branches in
metamask-account-creating.tsx throw a generic Error and drop server-side
details; update both occurrences to read the response body (prefer JSON with
await response.json(), falling back to await response.text() if needed), extract
a meaningful message or error field, and include it plus
response.status/statusText in the thrown Error (or pass into your existing
error-handling/logging). Locate the two blocks using the response variable in
this file and replace the generic throw with creating an Error that contains the
backend message and status for better user/log visibility.
Comment on lines
+24
to
+27
| authority?: "owner" | "active"; | ||
| } | ||
|
|
||
| export function KeyOrHot({ inProgress, onKey, onHot, onKc, keyOnly, authority="active" }: Props) { | ||
| export function KeyOrHot({ inProgress, onKey, onHot, onKc, onMetaMask, keyOnly, authority="active" }: Props) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of KeyOrHot that might need explicit authority specification
# Look for callers that don't pass authority prop
rg -n -A10 '<KeyOrHot' --type=tsx --type=ts | rg -v 'authority='Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Find all usages of KeyOrHot component
rg -n '<KeyOrHot' --type ts --type tsx -A 5 | head -100Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Find all usages of KeyOrHot component without file type filters
rg '<KeyOrHot' -A 5 | head -150Repository: ecency/vision-next
Length of output: 7890
🏁 Script executed:
#!/bin/bash
# Get full context for files with KeyOrHot but no authority shown
echo "=== transaction-signer.tsx ==="
rg -B 5 -A 15 '<KeyOrHot' apps/web/src/features/shared/transactions/transaction-signer.tsx
echo -e "\n=== key-or-hot-dialog.tsx ==="
rg -B 5 -A 15 '<KeyOrHot' apps/web/src/features/shared/key-or-hot-dialog.tsx
echo -e "\n=== account-recovery.tsx ==="
rg -B 5 -A 15 '<KeyOrHot' 'apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/account-recovery.tsx'
echo -e "\n=== manage-authorities.tsx ==="
rg -B 5 -A 15 '<KeyOrHot' 'apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-authorities.tsx'
echo -e "\n=== community-create-sign-step.tsx ==="
rg -B 5 -A 15 '<KeyOrHot' apps/web/src/app/communities/create/_components/community-create-sign-step.tsxRepository: ecency/vision-next
Length of output: 3326
🏁 Script executed:
#!/bin/bash
# Find KeyInput component to see if it validates authority
echo "=== KeyInput component ==="
find . -name "*key-input*" -type f | head -20
echo -e "\n=== Check account-recovery handleSign function ==="
rg -B 2 -A 10 'const handleSign' 'apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/account-recovery.tsx'
echo -e "\n=== Check manage-authorities revoke function ==="
rg -B 2 -A 10 'const revoke' 'apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-authorities.tsx'Repository: ecency/vision-next
Length of output: 733
🏁 Script executed:
#!/bin/bash
# Check KeyInput component implementation
echo "=== KeyInput component ==="
cat -n apps/web/src/features/ui/input/key-input.tsx
echo -e "\n=== Find revoke function in manage-authorities.tsx ==="
rg -B 2 -A 15 'revoke' 'apps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/manage-authorities.tsx' | head -60Repository: ecency/vision-next
Length of output: 6933
🏁 Script executed:
#!/bin/bash
# Check the full KeyOrHot implementation to see how it passes keyType to KeyInput
cat -n apps/web/src/features/shared/key-or-hot/index.tsxRepository: ecency/vision-next
Length of output: 4060
🏁 Script executed:
#!/bin/bash
# Check updateRecovery to see what it requires
echo "=== Search for updateRecovery definition ==="
rg -i 'updateRecovery' --type=ts --type=js -B 2 -A 5 | head -80
echo -e "\n=== Search for useAccountRevokePosting ==="
rg 'useAccountRevokePosting' -B 2 -A 5 | head -80Repository: ecency/vision-next
Length of output: 50376
authority should be required, not optional with default "active".
Making authority optional masks missing authority specifications in callers. Operations like account recovery and authority management that require owner-level access silently default to prompting for active keys instead. Callers like account-recovery.tsx and manage-authorities.tsx don't pass the prop, so users are prompted for the wrong key type. With authority required, TypeScript would enforce that callers explicitly specify the needed level for sensitive operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/shared/key-or-hot/index.tsx` around lines 24 - 27, The
authority prop must be made required to force callers to specify key level:
update the Props type by removing the optional marker from authority (authority:
"owner" | "active") and remove the default value from the KeyOrHot function
signature so KeyOrHot({ ..., authority }: Props) expects an explicit value; then
update all call sites (e.g., where KeyOrHot is used in account-recovery and
manage-authorities flows) to pass the correct authority ("owner" or "active") so
sensitive operations request the proper key type.
Comment on lines
+206
to
+228
| {typeof window !== "undefined" && window.ethereum?.isMetaMask && ( | ||
| <Button | ||
| appearance="secondary" | ||
| outline={true} | ||
| full={true} | ||
| size="lg" | ||
| className="col-span-2" | ||
| onClick={() => !!username && loginByMetaMask()} | ||
| disabled={!username || isLoginByMetaMaskPending} | ||
| isLoading={isLoginByMetaMaskPending} | ||
| icon={ | ||
| <Image | ||
| width={100} | ||
| height={100} | ||
| src="/assets/metamask-fox.svg" | ||
| alt="metamask" | ||
| className="w-4 h-4" | ||
| /> | ||
| } | ||
| > | ||
| {i18next.t("key-or-hot.sign-with-metamask", { defaultValue: "Sign with MetaMask" })} | ||
| </Button> | ||
| )} |
Contributor
There was a problem hiding this comment.
Consider hydration mismatch with window check.
The typeof window !== "undefined" && window.ethereum?.isMetaMask check runs on every render, but during SSR the condition is always false, while on the client it may be true. This can cause a hydration mismatch warning in Next.js when the server-rendered HTML differs from the client-rendered output.
Consider using a useEffect + state pattern to defer the MetaMask availability check to client-side only:
const [isMetaMaskAvailable, setIsMetaMaskAvailable] = useState(false);
useEffect(() => {
setIsMetaMaskAvailable(!!window.ethereum?.isMetaMask);
}, []);Then use isMetaMaskAvailable in the conditional render. This ensures server and initial client render match (both false), avoiding hydration warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/shared/login/login.tsx` around lines 206 - 228, Replace
the direct window check with a client-only state flag to avoid hydration
mismatch: add a state variable (e.g., isMetaMaskAvailable via useState(false))
and set it inside useEffect to !!window.ethereum?.isMetaMask on mount, then
change the render conditional that currently uses typeof window !== "undefined"
&& window.ethereum?.isMetaMask to use isMetaMaskAvailable; keep existing
handlers/props like loginByMetaMask, isLoginByMetaMaskPending and username
unchanged so button behavior is preserved.
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 by CodeRabbit
New Features
Refactor
Chores