Wallet simplified#3175
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Peak Vault dApp bridge (injected JS + handler), a Receive QR action sheet, token switching in transfers, wallet header quick-actions redesign, video thumbnail propagation to VideoPlayer, i18n additions, multiple TextInput platform tweaks, and bumps Changes
Sequence Diagram(s)sequenceDiagram
participant WebView
participant InjectedBridge as PeakVaultBridge
participant BridgeHandler as Native Bridge Handler
participant Account as Account State
participant NativeUI as Native UI
WebView->>InjectedBridge: window.peakvault.* invokes API -> dispatch pvRequest detail
InjectedBridge->>BridgeHandler: postMessage { name: 'pvRequest', data }
BridgeHandler->>Account: validate login & account match
alt account valid
BridgeHandler->>NativeUI: show SIGN_CONFIRM sheet
NativeUI->>BridgeHandler: user approve / reject
alt approve & broadcast
BridgeHandler->>BridgeHandler: broadcast transaction
BridgeHandler->>InjectedBridge: postMessage peak-vault-response (success/result)
else approve & sign-only
BridgeHandler->>Account: derive key via PIN, sign payload
BridgeHandler->>InjectedBridge: postMessage peak-vault-response (signature + pubkey)
end
else invalid
BridgeHandler->>InjectedBridge: postMessage peak-vault-response (error)
end
InjectedBridge->>WebView: dispatch peak-vault-response-<resId> event -> resolve Promise
sequenceDiagram
participant User
participant TransferUI as Transfer View
participant TransferContainer
participant BalanceSvc as Balance Fetch
User->>TransferUI: tap fund badge
TransferUI->>TransferUI: open token picker overlay
User->>TransferUI: select token
TransferUI->>TransferContainer: call setFundType(newType)
TransferContainer->>TransferContainer: set fundType, reset balance
TransferContainer->>BalanceSvc: fetchBalance(account.name)
BalanceSvc->>TransferContainer: return balance
TransferUI->>TransferUI: close picker, clear amount if token changed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts (1)
313-329:⚠️ Potential issue | 🟠 MajorGate
peakvault-connectbehind user confirmation.This still exposes the logged-in account to the page without consent. Please require the same confirmation sheet flow before returning
result: [username].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts` around lines 313 - 329, The current peakvault-connect branch in useHiveBridgeHandler sends account info via _sendPvResponse without user consent; change it to invoke the app's confirmation sheet flow (the same confirmation flow used elsewhere in this file) before returning result: [username]: if !username keep the existing _sendPvError behavior, otherwise present the confirmation UI to the user and only call _sendPvResponse({ success: true, error: '', account: username, result: [username] }) when the user explicitly confirms; if the user rejects or times out, return an appropriate _sendPvError and do not expose username.
🧹 Nitpick comments (2)
src/components/postHtmlRenderer/postHtmlRenderer.tsx (2)
329-350: Thumbnail resolution for anchor-rendered videos LGTM.Preferring the inline
video-thumbnail<img src>and falling back to_metadataThumbUrlis a sensible precedence, and forwardingthumbnailUrl={thumbUri}intoVideoPlayeraligns with the new prop threaded throughpostBodyView/postInteractionHandler. One minor readability nit:.indexOf('video-thumbnail') >= 0could be.classes?.includes('video-thumbnail')for consistency with the same check done viaincludeselsewhere in this file (e.g. line 431, 477) and to tolerate any child shape lackingclasses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/postHtmlRenderer/postHtmlRenderer.tsx` around lines 329 - 350, Replace the brittle ".classes.indexOf('video-thumbnail') >= 0" check inside the tnode.children.find call with a safe includes check so children missing classes don't throw: use child.classes?.includes('video-thumbnail') (referencing the tnode.children.find and imgElement symbols) to locate the thumbnail img; keep the existing fallback to _metadataThumbUrl and the rest of the rendering logic unchanged.
140-141: Narrow the memo dependency tometadata?.image.
_metadataThumbUrlonly readsmetadata?.image, but the memo re-runs whenever the wholemetadatareference changes (which in this codebase happens on most re-renders). Narrowing the dep keeps the value stable across unrelated metadata churn and avoids redundant invalidation of_handleOnLinkPress,_anchorRenderer, and_iframeRenderercallbacks that depend on it.♻️ Proposed tweak
- const _metadataThumbUrl = useMemo(() => _getFirstMetaImage(metadata?.image), [metadata]); + const _metadataThumbUrl = useMemo( + () => _getFirstMetaImage(metadata?.image), + [metadata?.image], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/postHtmlRenderer/postHtmlRenderer.tsx` around lines 140 - 141, The useMemo that computes _metadataThumbUrl currently depends on the entire metadata object which causes unnecessary recomputations; change the dependency array on the useMemo that calls _getFirstMetaImage to only depend on metadata?.image (i.e., useMemo(() => _getFirstMetaImage(metadata?.image), [metadata?.image])) so _metadataThumbUrl stays stable across unrelated metadata object changes and prevents needless invalidation of _handleOnLinkPress, _anchorRenderer, and _iframeRenderer callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts`:
- Line 311: The code reads keyRole from pvData (const { resId, account:
requestedAccount, operations, keyRole, broadcast } = pvData) but uses it
directly to pick an authority, allowing malformed/unknown values from the
WebView to silently fall back to different authorities; instead validate keyRole
against an explicit allowed set (e.g., const ALLOWED_KEY_ROLES =
['posting','active','owner']) before using it, and if keyRole is
missing/invalid: fail fast (reject the request or return an error response)
rather than defaulting differently for UI vs broadcast. Update every place that
branches on keyRole (the destructuring site and the similar logic around the
other blocks you noted) to perform the same validation and use the validated
value (or abort) so UI and broadcast behavior remain consistent.
- Around line 505-535: The sign-only unsupported error should be returned
immediately instead of first reading PIN/local keys; in the sign-only branch
call _sendPvError(resId, 'Sign-only mode is not supported in Ecency browser.
Please use requestBroadcast instead.', account) and return before accessing
store.getState(), getDigitPinCode, selectCurrentAccount, getActiveKey or
getPostingKey so key material is not touched when we always reject sign-only
requests.
- Around line 552-555: The pvRequest handling currently trusts pvData.sourceUrl
from the message; update useHiveBridgeHandler to accept the current trusted
WebView URL (e.g., trustedWebViewUrl) and, inside the onMessage/parsed.name ===
'pvRequest' branch before calling _handlePeakVaultMessage or showing
confirmation, validate that the origin/domain extracted from pvData.sourceUrl
strictly matches the trustedWebViewUrl (or its origin) and reject/ignore the
request if it does not match; reference the _handlePeakVaultMessage call, the
onMessage handler that checks parsed.name === 'pvRequest', and the
pvData.sourceUrl value when adding this gate.
In `@src/screens/wallet/children/walletHeader.tsx`:
- Around line 48-77: _onSendPress and _onSwapPress navigate unconditionally even
when no user is signed in; mirror the guard used in
_onReceivePress/_onGetPointsPress by checking currentAccount?.name before
calling navigation.navigate, and if falsy dispatch the same toastNotification
(using intl.formatMessage({ id: 'alert.not_logged_in' })) instead of navigating;
otherwise proceed to call navigation.navigate with the existing params for
ROUTES.SCREENS.TRANSFER and ROUTES.SCREENS.TRADE respectively.
---
Duplicate comments:
In `@src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts`:
- Around line 313-329: The current peakvault-connect branch in
useHiveBridgeHandler sends account info via _sendPvResponse without user
consent; change it to invoke the app's confirmation sheet flow (the same
confirmation flow used elsewhere in this file) before returning result:
[username]: if !username keep the existing _sendPvError behavior, otherwise
present the confirmation UI to the user and only call _sendPvResponse({ success:
true, error: '', account: username, result: [username] }) when the user
explicitly confirms; if the user rejects or times out, return an appropriate
_sendPvError and do not expose username.
---
Nitpick comments:
In `@src/components/postHtmlRenderer/postHtmlRenderer.tsx`:
- Around line 329-350: Replace the brittle ".classes.indexOf('video-thumbnail')
>= 0" check inside the tnode.children.find call with a safe includes check so
children missing classes don't throw: use
child.classes?.includes('video-thumbnail') (referencing the tnode.children.find
and imgElement symbols) to locate the thumbnail img; keep the existing fallback
to _metadataThumbUrl and the rest of the rendering logic unchanged.
- Around line 140-141: The useMemo that computes _metadataThumbUrl currently
depends on the entire metadata object which causes unnecessary recomputations;
change the dependency array on the useMemo that calls _getFirstMetaImage to only
depend on metadata?.image (i.e., useMemo(() =>
_getFirstMetaImage(metadata?.image), [metadata?.image])) so _metadataThumbUrl
stays stable across unrelated metadata object changes and prevents needless
invalidation of _handleOnLinkPress, _anchorRenderer, and _iframeRenderer
callbacks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a136bbbd-4455-45d2-b134-b9284569afdc
📒 Files selected for processing (4)
src/components/postHtmlRenderer/postHtmlRenderer.tsxsrc/hooks/useLinkProcessor.tsxsrc/screens/dappBrowser/hooks/useHiveBridgeHandler.tssrc/screens/wallet/children/walletHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useLinkProcessor.tsx
| // Handle Peak Vault messages | ||
| if (parsed.name === 'pvRequest') { | ||
| _handlePeakVaultMessage((parsed as PeakVaultRequest).data); | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect WebView wiring for Peak Vault injection vs message handling.
# Expectation: PEAK_VAULT_BRIDGE_JS is gated by HTTPS, and onMessage/handleMessage is still registered on the WebView.
fd -i '^dappBrowser\.tsx$' src | xargs rg -n -C3 \
'injectedJavaScriptBeforeContentLoaded|PEAK_VAULT_BRIDGE_JS|onMessage|handleMessage|useHiveBridgeHandler'Repository: ecency/ecency-mobile
Length of output: 1359
🏁 Script executed:
#!/bin/bash
# Read the useHiveBridgeHandler.ts file to inspect the _handlePeakVaultMessage function
# and understand how pvData.sourceUrl is used
# First, find the file
fd -i 'useHiveBridgeHandler.ts' src
# Get file size to determine reading strategy
wc -l src/screens/dappBrowser/hooks/useHiveBridgeHandler.tsRepository: ecency/ecency-mobile
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Read around lines 552-555 and surrounding context to understand the flow
sed -n '540,600p' src/screens/dappBrowser/hooks/useHiveBridgeHandler.tsRepository: ecency/ecency-mobile
Length of output: 1867
🏁 Script executed:
#!/bin/bash
# Find _handlePeakVaultMessage function definition
rg -n '_handlePeakVaultMessage' src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts -A 20Repository: ecency/ecency-mobile
Length of output: 1738
🏁 Script executed:
#!/bin/bash
# Check if there's any URL validation or trusted URL comparison before handling pvRequest
rg -n 'pvRequest|sourceUrl|pvData' src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts -B 2 -A 5Repository: ecency/ecency-mobile
Length of output: 1632
🏁 Script executed:
#!/bin/bash
# Check if activeWebViewRef or tab.url is used anywhere for validation
rg -n 'activeWebViewRef|tab\.url|webViewUrl|currentUrl' src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts -B 2 -A 2Repository: ecency/ecency-mobile
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the beginning of the file to see what's passed to the hook
head -100 src/screens/dappBrowser/hooks/useHiveBridgeHandler.tsRepository: ecency/ecency-mobile
Length of output: 3071
Gate Peak Vault requests with the trusted WebView URL before processing.
The HTTPS-only injection check is bypassed when onMessage receives { name: 'pvRequest' } directly from untrusted page JavaScript. The pvData.sourceUrl is attacker-controlled, and the domain extracted from it (lines 358, 460) is displayed in the confirmation dialog without validating it matches the actual WebView URL. Pass the current trusted WebView URL to useHiveBridgeHandler and verify it matches pvData.sourceUrl before routing the request and showing the confirmation sheet.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/dappBrowser/hooks/useHiveBridgeHandler.ts` around lines 552 -
555, The pvRequest handling currently trusts pvData.sourceUrl from the message;
update useHiveBridgeHandler to accept the current trusted WebView URL (e.g.,
trustedWebViewUrl) and, inside the onMessage/parsed.name === 'pvRequest' branch
before calling _handlePeakVaultMessage or showing confirmation, validate that
the origin/domain extracted from pvData.sourceUrl strictly matches the
trustedWebViewUrl (or its origin) and reject/ignore the request if it does not
match; reference the _handlePeakVaultMessage call, the onMessage handler that
checks parsed.name === 'pvRequest', and the pvData.sourceUrl value when adding
this gate.
Summary by CodeRabbit
New Features
Updates