A-1217833856533186: fix review issues in the external connection flow - #258
Open
erubboli wants to merge 52 commits into
Open
A-1217833856533186: fix review issues in the external connection flow#258erubboli wants to merge 52 commits into
erubboli wants to merge 52 commits into
Conversation
- Fix the sign-challenge approval crash (missing networkType context) - Sign rejects now reject the dApp promise instead of resolving 'null' - Answer waiting dApps when an approval window is closed or fails - Only accept approval/disconnect messages from extension pages - Fix the double-approval-window race; reconnect returns the session - Rewrite the content script: working session restore, correct timeout tracking, 5-minute approval timeout - Fix mojito.isConnected() and make disconnect() clear the wallet session - Extract the shared Browser API module (storage/runtime/sendPopupResponse) - Remove dead code: session_ persistence, remember checkbox, fake Bitcoin toggle, unused request state, useless mode switch, legacy Firefox background script - Ship mock data and mock selectors only in development builds - Estimate the HTLC funding fee like the wallet's own sends instead of a hardcoded 2000 sat/vB - Show signing errors in the password modal and prevent double submits - Bound stringification of untrusted dApp data in the transaction breakdown
Relay errors to pages as { code, message } instead of bare strings so the
SDK/bridge can distinguish timeout vs extension-context failures instead of
string sniffing. window.mojito turns these into Error.code.
…-honest
The @mintlayer/sdk Client reads addressesByChain.mintlayer on
connect()/restore(), but the background stored and returned only the
network-keyed address map — auto-restore could never re-engage.
- persist the full session: { address, addressesByChain, network, timestamp }
- getSession returns address + addressesByChain + network
- ConnectionPage files addresses under the wallet's ACTIVE network key only
(it previously labeled the same addresses as both mainnet and testnet) and
records the grant's network in the session
- sign requests carry the session network; the signing screen rejects with
WRONG_NETWORK when the wallet switched networks since the grant instead of
silently signing on the other chain
- mojito.js restore(): resolve the whole session (or null), track the session
network, and use unique request ids — the fixed '__restore' id was swallowed
by the content script's duplicate guard, hanging a second concurrent
restore (e.g. React strict-mode double Client.create())
- window.mojito.connect/restore also update mojito.network from the session
Note: background.js and ConnectionPage.js also carry the earlier
hardening from this branch (sender-derived session origin, defensive
address shapes).
- manifest validity: MV3 CSP has 'wasm-unsafe-eval' (never 'unsafe-eval'), content script injects at document_start, https-only top-frame matches, mojito.js is the only web-accessible resource - mojito provider: connect/restore lifecycle, session shape (addressesByChain), structured error codes, null restore without a grant, concurrent restores, disconnect revoking page state + wallet session - background: approval-window lifecycle, session persistence, USER_REJECTED / NOT_CONNECTED / UNSUPPORTED_METHOD codes, network stamping on sign requests, forged popupResponse/disconnect from web senders rejected, disconnect actually revoking the stored grant - ignore build/ copies in jest
- document the final window.mojito surface, session shape, error codes and the @mintlayer/sdk v1.0.38 mapping (doc/bridge-contract.md) - Chromium manifest: content script runs on https pages in top frames only (plain http limited to localhost for development) and mojito.js is the only web-accessible resource — dApps keep connecting through the manual approval popup, no static allowlist - manifest.test.js asserts CSP validity (wasm-unsafe-eval, no unsafe-eval), document_start injection and the web-accessible-resources scope
The @mintlayer/sdk Client.connect() reads `addresses.addressesByChain.mintlayer` unguarded; a popup response that carries only the `address` map (older extension build still loaded, or a partially-updated install) crashes the dApp with "Cannot read properties of undefined (reading 'mintlayer')". The injected window.mojito.connect() now synthesizes the chain-keyed view when it is missing, so any combination of extension builds keeps the SDK connect path working. Regression test included.
owlsua
requested changes
Sep 6, 2026
owlsua
left a comment
owlsua
left a comment
Collaborator
There was a problem hiding this comment.
A few comments with one blocker
Four pages import sendPopupResponse from @browser, but that module only re-exports Browser, so the binding is undefined and every approve and reject in the dApp flow throws. Nothing caught it: webpack only warns, eslint had no import plugin, and tsconfig keeps checkJs off, so the .js call sites are never type-checked. Enable import/named with the TypeScript resolver so the alias map in tsconfig stays the single source of truth. @browser was missing from that map - without it the resolver could not find the module and the rule stayed silent. npm run lint now exits non-zero, which fails the ESLint step already in CI. The four call sites it reports are real and still need the missing re-export.
owlsua
reviewed
Sep 6, 2026
owlsua
requested changes
Sep 6, 2026
Reloading, updating or disabling the extension orphans the content script
in already-open pages: the next runtime call throws
'Extension context invalidated.' synchronously. The relay neither caught it
nor answered the page, so the dApp's connect()/sign promise hung forever
and the console showed an uncaught error.
- catch the throw, fail the pending request with
{ code: 'CONTEXT_INVALIDATED' } and short-circuit further requests
- the load-time getSession probe no longer throws uncaught either
- regression tests: dead-runtime request answered with the structured
error, fast-fail without touching the runtime again
isMlAddressValid only matched the pubkeyhash prefixes (mtc1/tmt1), so valid multisig addresses like mmtc1q3v0hye8eg6vg7f7thmpy6y834u8h0r4as0hyax2 were rejected by the send form. Accept the multisig prefixes per network and reject cross-network use as before.
Chrome ignores autocomplete=off for saved addresses and repaints autofilled inputs with its light background while the dark theme keeps near-white text — unreadable white-on-white fields on the send forms. Override the autofill paint globally: keep the dark input surface and the theme text color.
Root cause of the white input fields on the Send ML form: neither the shared .input class, nor .textarea, nor any raw input declared a background, so the browser's UA stylesheet painted its default white field background in light color-scheme while the theme text stayed near-white — white on white. - Input.module.css .input and Textarea.css .textarea now use --be-bg-2 - global reset gives raw text inputs/selects (swap, address book, delete account) the same dark surface; checkboxes/radios/file/button inputs keep native rendering - the -webkit-autofill override stays: autofill repaints remain dark
--mojito-green-soft (rgb(230 248 240)) is a near-white light-theme color: - WalletCard (the 'Mintlayer — Balance: …' block on the send forms) rendered as a white card; DelegationDetails and TransactionDetails banners too - SettingsTestnet active option and the Button alternate hover/focus painted white-mint states Cards/banners now use --be-bg-1; active and hover states use a dim green wash (rgba(--mojito-green, 0.12)) so the accent survives on dark.
Token metadata carries the token's icon in icon_uri ({ hex, string }), but
the wallet only kept ticker + decimals from GET /token/:id and rendered a
generic letter tile for every token (mlUSDC included).
- MintlayerProvider keeps icon_uri in tokenBalances token_info
- TokenIcon renders the metadata image (circular, object-fit cover) with
fallback to the procedural tile on load failure; ipfs:// uris map to the
public gateway (same as NFT details)
- AssetRow + Dashboard token rows + AssetPage header pass the icon through
- manifest CSP img-src 'self' data: -> 'self' data: https: — remote icons
cannot render otherwise; deliberate trade-off (icon hosts see the IP of
users holding the token), documented in REVIEW-PLAN
- tests: TokenIcon image/gateway/fallback cases, AssetPage renders the
metadata icon
Real data: Mintlayer tokens have NO on-chain icon_uri — GET /token/:id only
carries metadata_uri (ipfs link), and the icon lives in that document under
'tokenIcon' (mlUSDC: metadata_uri -> ipfs JSON -> tokenIcon -> ipfs image).
The previous icon_uri wiring could therefore never populate.
- new Mintlayer.resolveTokenIcon(metadata_uri): fetches the metadata JSON
(ipfs.io gateway, 8s timeout, cached per uri so the 2-min refresh does not
re-fetch) and returns the gateway-mapped icon url; failures resolve
undefined -> fallback tile
- provider resolves icons in parallel during token enrichment and stores
token_info.icon_uri { string } (shape unchanged for the UI)
- ipfs gateway switched to https://ipfs.io/ipfs everywhere (gateway.ipfs.io
returned empty responses — NFT images were broken with it too)
Review remediation (P1) — amounts were computed with float/string math on an
11-decimal currency:
- getParsedTransactions accumulated decimal STRINGS ('1.5'+'0.7' -> '1.50.7')
and clobbered the accumulator per branch; now numeric accumulation
- getAmountInCoins/getAmountInAtoms/atomsToDecimal use Decimal instead of
float multiplication on the atom scale
- BTC.calculateBalances: missing yesterday rates now yield null (rendered as
'no data') instead of division-by-fallback producing absurd 24h percentages
- amount regex escaped (/^\d+(\.\d+)?$/) so '1e3'/'1x5' no longer validate
- buildStakeGrowthSeries helper: cumulative staked-over-time series from
DelegateStaking/Delegate Withdrawal transactions (used by the staking page)
- getUnconfirmedTransactionKey helper replaces eight hand-built localStorage
keys
- MintlayerProvider.fetchAllData: try/catch/finally + ref-based mutex — one failed request used to leave every loading flag stuck and block all future refreshes; forced runs serialize behind in-flight ones; new fetchError context field; network-switch effect owns cancelAllRequests - fetchDelegations always releases its flag and resets state on error - fetchOrdersPairInfo no longer leaves the spinner stuck on failure - chain-tip polling catches errors instead of unhandled rejections - ExchangeRatesProvider fetches both coins in parallel, exposes error/fetching state, keeps last good rates on failure - BitcoinProvider never sets btcUtxos to undefined (crashed coin selection) and the network-sync effect no longer re-fired every render - Electrum: drop the unvalidated customAPIServers localStorage override (redirected all BTC data and broadcasts) - Browser.sendPopupResponse: robust cleanup when the window cannot close
…mplementations - new basic components: Icon (inline line set), Tag, Seg, LivePill, Sparkline, TokenIcon (metadata icon + procedural fallback), Counter, Eyebrow, KV, Seg, Sheet, QrCode/QrPlaceholder, ChainBadge, Avatar, IconTile, Progress, MojitoLogo, ErrorBoundary (self-reporting), SkeletonLoader restyle - new composed components: TxRow, AssetRow, BeSheet (bottom sheet) - theme.css: be-* design tokens (bg/text/line/amber/teal, oklch) - remove the old parallel renderers the new pages replace: containers/Dashboard (CryptoList/Statistics/CryptoSharesChart/Skeleton) and CurrentStaking
… data
- Dashboard: total balance, 4 quick actions (Send/Receive/Stake/Activity),
real token list from tokenBalances (metadata icons), honest empty states
(no more mock NFTs or demo activity rows)
- AssetPage: BTC/ML/token detail from tokenBalances (ticker/decimals/icon),
token-scoped transactions, no fabricated price data for tokens
- StakePage: total staked, earned-from-staking, stake-growth chart
(buildStakeGrowthSeries), delegation list with add-funds/withdraw
- ActivityPage: real merged BTC+ML history with detail sheet
- ReceivePage: chain-aware addresses with QR
- routes: /staking new page; /wallet/:coinType and unknown routes redirect
to /dashboard; legacy Wallet and Staking pages removed; all
navigate('/wallet') dead-ends repointed; ConnectionPage test added
- sign/confirm/challenge/message screens lose their white backgrounds and Arial font; hardcoded grays/reds/ambers map to be-* tokens - inputs/textareas get an explicit dark background (the UA stylesheet paints undecleared fields white) plus a -webkit-autofill override - WalletCard, Delegation cards/skeletons, TransactionDetails and DelegationDetails banners drop the near-white --mojito-green-soft surfaces - PopUp close icon visible on dark; CreateDelegation loading overlay dark - slider menu: drop the legacy Bitcoin/Mintlayer wallet entries (features live on the Dashboard now) + regression test - login/create-restore/set-password screens use the MojitoLogo badge
- scripts/audit-imports.js + npm run audit:imports: verifies every local import resolves and every named import is actually exported (review feedback: hallucinated imports slipped past eslint/build — this closes that gap; currently 376 files, 0 problems) - webpack: no source maps in production builds - jest: ignore build/ copies; configs drop the removed @Mocks alias - docs: REVIEW-PLAN.md (review backlog + accepted risks), design reference files under doc/ - .gitignore: local manifest key + HANDOFF.md (local session notes, no longer tracked)
ipfs.io regularly times out or returns empty for the metadata CIDs (observed for mlUSDC: 'signal timed out' after 8s), so a single hard-coded gateway left tokens iconless. - resolveTokenIcon tries ipfs.io -> dweb.link -> gateway.pinata.cloud (6s per attempt); a definitive 'document has no icon' is cached, network failures are not so the next refresh retries - TokenIcon cycles the same mirror list on <img> error before falling back to the procedural tile
Measured from the team network: ipfs.io timed out for both the metadata and icon CIDs (the source of the 'signal timed out' errors) while dweb.link and w3s.link answered in ~0.5s; gateway.pinata.cloud answered in 5.7s — inside the timeout only by luck — and is removed per report. - fetchJsonWithGatewayFallback races all gateways in parallel (10s each, Promise.any): a dead gateway loses the race instead of costing 6s of serial waiting - resolved 'no icon' answers are cached with a 5-minute TTL; total gateway failures are not cached so the next refresh retries, and they log once per uri per session instead of spamming every 2-minute poll - TokenIcon mirror cycle updated to ipfs.io -> dweb.link -> w3s.link - invariant tested: a raw ipfs:// uri is never fetched, only gateway URLs
…guards - content-script: the lastError branch cleared the pending entry before failRequest could run, so EXTENSION_ERROR responses were never posted and the dApp promise hung forever (regression of the hang-fix itself); failRequest now owns the cleanup. Reload ownership added: the last injected instance owns the channel, orphans stop answering (their stale error used to poison the fresh instance's responses after every update). EXTENSION_ERROR message is static (no raw chrome strings to pages) - BTC.getStats: an empty wallet with valid rates rendered '-100% · 24h' (0/yesterday-fallback -> (0-1)*100); zero is now a real zero, null stays 'no data' (+ regression tests) - cancelAllRequests actually cancels now: the AbortController signal is wired into the fetch of both API services and the registry is keyed by method+url so concurrent requests don't clobber each other - MintlayerProvider clears fetchError on a successful refresh - buildStakeGrowthSeries charts [0 -> live total] when the delegation predates the parsed transaction history instead of hiding the chart - StakePage guards the 'earned' figure against NaN
…ed state, copy feedback UI review quick wins: - SignExternalTransaction: the Approve/Decline footer was absolutely positioned inside the scroll container — on long transactions the consent buttons scrolled out of view; now in flow (mirrors the BTC screen fix) - Settings/Restore/Delete/LockedBalance/NFT family: dark-slate text (--color-dark-gray) on dark cards was ~2.5:1 unreadable; swept to --be-text-2 - icon-arrow-right-top.svg: near-black stroke was invisible on dark surfaces (StakePage 'Pool list', detail popups) — now currentColor - AssetRow: disabled state renders dimmed with a 'Sync issue' tag instead of a silent dead row; navigation moved onto the row's own handler and the duplicate data-testid wrapper dropped - Counter: animates from the previous value (periodic refreshes no longer re-roll the balance through $0.00) and honors prefers-reduced-motion - CopyButton: false 'copied' feedback fixed (promise caught, success icon only on resolve) + aria-label - Textarea: border on --be-line like Input; textarea autofill override
provideBitcoinData defaulted to true while the Bitcoin opt-out toggle renders only for sites requesting the 'bitcoin' permission — a site that never asked silently received every BTC address and public key with no way for the user to see or refuse it. Default now follows requireBTC (review comment #2).
pendingRequest was a single storage key written by two independent approval slots: a connect window (origin A) and a signing window (origin B) could be open at once, each overwriting the other's record, and the popup re-read the key on every effect run — so a later request from any origin could replace the one the user was about to approve (approve-the-wrong- request hazard). - approvals are now stored per window: pendingRequest:<windowId> - the popup reads its OWN window's key (windows.getCurrent before the read) - popupResponse carries windowId and only clears its own record - messages are queued until the persisted session map finishes loading — a connect answered against a half-loaded map told already-connected sites they were NOT_CONNECTED - stale pre-window-keyed records are cleaned up at boot - windows.get lookup checks runtime.lastError (review comment: unchecked lastError console noise)
submitCreate was broken three ways and could never have worked:
- it destructured { WIF } from Account.unlockAccount, which does not
return a WIF — always undefined
- buildTransaction was called with {fee, wif, from, networkType} but its
signature is {to, amount, utxos, feeRate, walletType, changeAddress, root}
(it throws 'reading btcAddressData' before anything else)
- it destructured three elements from a function that returns two, so
transactionId was always undefined
Now: the HTLC script is built without key material, the funding
transaction is built exactly like ConfirmBtcTransaction does (wallet UTXOs,
feeRate, walletType, change address, HD root from unlockAccount), the
destructure matches the real [tx, hex] return, and the transaction id is
derived from the signed hex.
NEEDS MANUAL VERIFICATION: the full HTLC create/spend/refund flow still
requires an end-to-end run against a dApp (see REVIEW-PLAN.md) — the
claim/refund signers derive their WIF differently and are unchanged.
img-src https: allowed any HTTPS host; the wallet only ever renders token icons from the three raced gateways, so the CSP now names them explicitly.
Member
Author
|
Thanks for the review — all fixed and pushed:
Also fixed along the way from our own internal review: a hang where lastError responses were never posted to the page, -100% 24h stats on empty wallets, token icons now racing multiple ipfs gateways (ipfs.io was timing out), and per-window approval isolation for the session-map load race. Could you take another pass when you get a chance? |
The welcome/create-restore/set-password screens rendered a hand-drawn amber/teal 'M' mark, not the Mintlayer logo. MojitoLogo now renders assets/logo.svg (the same mark the sidebar and token rows use), keeping the size and animate API.
Root cause of the disappearing mlUSDC icon: public ipfs gateways rate-limit by IP. The wallet raced 3 gateways per token on every 2-minute refresh and rendered the icon straight from a gateway URL — once the team's IP got throttled (dweb 429, ipfs.io hanging, w3s 301s into dweb) every icon vanished again no matter which gateway we picked. - resolveTokenIcon now fetches the icon BYTES (racing the gateways once, content-type + 5MB size checks) and returns an in-memory blob: URL; the icon never touches a gateway again after the first successful load - the metadata document is cached permanently (content-addressed = immutable), so a settled token generates zero gateway traffic on refresh - failures are never cached (retried next refresh) and log once per uri - CSP img-src: + blob: for the object urls, + *.dweb.link/*.w3s.link wildcards (those gateways 301 to subdomain style, which the pin would have blocked mid-redirect) - TokenIcon simplified: renders the resolved blob url, tiles on error (last-resort ipfs:// safety mapping kept, tested)
- metadata + icon bytes raced across gateways (6 requests first pass) - zero gateway traffic after a settled resolution - TTL-cached no-icon answers vs retried total failures - non-ipfs metadata uris rejected with zero network requests
…back
dApp approvals (connect + sign) always forced a separate popup window.
The wallet already ships a side panel running the same app — approvals now
open there, docked to the window the dApp lives in:
- requests are stored under pendingRequest:<dApp tab's window id> and
sidePanel.open({ tabId }) surfaces the panel for that tab
- if sidePanel.open rejects (no recent user gesture) the request rolls back
and a popup window opens instead — the flow never dead-ends
- panel-mode slots have no window-removed event: handlePopupResponse now
releases the owning slot, and failSlot clears by the slot's recorded
window id
- an already-open panel picks up new approvals instantly via
storage.onChanged (request ids deduped against the mount read)
- Firefox keeps popup windows (no sidePanel API there)
- panel path: sidePanel.open({ tabId }) called, request keyed by the
dApp tab's WINDOW id, no popup created, channel kept open
- popup fallback when sidePanel.open rejects, including the rollback of
the panel registration and the original channel being resolved by the
later popupResponse
- busy in panel mode answers REQUEST_IN_PROGRESS without a second
window
…gning Both signing screens (external dApp + internal send confirm) led with a 'Switch to preview/json' button and rendered a dense wall of untruncated addresses, token ids, a always-on raw input/output breakdown and request metadata. - new shared TransactionSummary container: action header (Send / Bridge transaction / Stake...), From/To (truncated + copy), Amount with token ticker from tokenMap, network fee, network — on be-* tokens - bridge intent rendered as its own compact row with copy - the full per-operation breakdown moved behind a collapsed 'Show technical details' disclosure, together with the raw JSON view (replaces the prominent 'Switch to preview/json' button) - both pages now render the summary; the preview components remain as the technical payload
…surface CRITICAL: revoking a connection in Settings deleted the grant but never told the site's open tabs — the page's SDK client kept the addresses in memory and could act on a dead grant (reported: bridge 'connected without authorization' after removal in Settings). - revocation propagation: disconnectSite and dApp 'disconnect' broadcast MOJITO_SESSION_REVOKED to all tabs; the content script relays it to the page as MINTLAYER_EVENT disconnect; window.mojito clears its cached addresses on it (with or without a page subscriber) - approvals can no longer silently fail: Chrome no-ops sidePanel.open() without a user gesture — the panel must now acknowledge it rendered the approval (approvalDisplayed) within 2s or the background falls back to a popup window, so a dApp never hangs with no approval surface - forged approvalDisplayed from web pages is ignored SECURITY CONTRACT (documented in background.js, mojito.js, ConnectionPage, SettingsConnections, bridge-contract.md): dApp connections are permission-level authorizations — every grant requires explicit user approval, every revocation is immediate, persisted and propagated, and no code path returns addresses without a live grant. Bridges must treat the disconnect event as authoritative and verify checkConnection before wallet-dependent actions.
After an extension reload/update, Chrome wipes the old content scripts (and the page-world window.mojito) from already-open tabs and re-injects at nondeterministic times — a dApp's first connect after a reload failed with 'wallet not found', and the second click worked only once the lazy re-injection caught up. - on runtime.onInstalled (install/update/reload) and browser startup, the background sweeps every open tab: pings the content script (MOJITO_PING); no live response -> re-injects it via chrome.scripting (already-permitted), so open dApp tabs self-heal without a page reload - content script answers MOJITO_PING - regression tests: sweep pings + re-injects only dead tabs (skips alive ones and non-tab ids), ping responder, revocation relay origin/type filtering (harness now dispatches to ALL listeners like Chrome does)
- onInstalled/onStartup sweeps ping every open tab (MOJITO_PING) and re-inject content-script.js only where the ping fails (lastError set); alive content scripts are skipped and non-tab ids never touched - onStartup sweep covered separately
npm audit fix + targeted overrides clear 35 of the 39 reported advisories (all dev tooling: babel chain, brace-expansion, ajv, browserslist, body-parser, @HumanFS, baseline-browser-mapping, websocket-driver/sockjs, qs, serialize-javascript, uuid — the latter three via semver-compatible overrides since the parents pin vulnerable ranges). Remaining 4 low findings are the elliptic GHSA-848j advisory inside the crypto-browserify webpack polyfill — no upstream fix exists (npm's own suggestion is downgrading crypto-browserify, which is strictly worse). Accepted risk: the polyfill is a build-time shim; wallet cryptography runs on the vendored wasm lib and noble curves. Documented in REVIEW-PLAN.md.
…rottling, dev advisories)
The background cancels its 2s popup-fallback timer only when the
approval surface sends {action:'approvalDisplayed', requestId} — but
nothing ever sent that message, so EVERY connect/sign approval fell
back to chrome.windows.create after 2 seconds, opening a new window
even though the side panel had rendered the request.
- Browser.notifyApprovalDisplayed(requestId): fire-and-forget ack via
runtime.sendMessage (swallows lastError/throws — the popup fallback
then still guarantees an approval surface, which is the safe outcome)
- handlePendingRequest acks as soon as the panel takes ownership of the
request (locked included: unlock-then-approval stays panel-owned)
- regression tests: ack cancels the fallback (no windows.create), no-ack
still falls back (windows.create exactly once), service unit tests
- Activity page and Dashboard recent activity now include Mintlayer TOKEN transactions (mlUSDC sends/receives were invisible — useMlWalletInfo filters to coin txs); tickers resolved via new Transactions.resolveTxSymbol (token_info -> tokenMap -> 'Token' fallback, unit-tested); recent rows sorted by date across chains - asset page TOKEN INFO: token id truncated (ML.formatAddress) with a copy button instead of a 60-char wall of text - flex-shrink fix on every scrollable page container (Asset/Receive/Stake/ Activity .page, Dashboard .scroll, all three sign screens): flex children used to compress instead of overflowing, which visually stacked the ML address onto the Receive button, the Receive button onto the network badge, and disabled scrolling to the history — one root cause, three reported symptoms
…acing - AssetPage: the Send button now renders for tokens too — it routes to /wallet/<tokenId>/send-ml-transaction, a flow SendMlTransaction already supports via the coinType param (token-scoped balance, decimals, ticker) - breathing room between the actions row and the ML address card - scrollbars hidden app-wide (scrollbar-width: none + webkit display:none): scrolling keeps working, the bar itself is not part of the design
- Passkey service (Chromium-only): creates a platform-authenticator passkey with the WebAuthn prf extension, derives the deterministic AES-GCM key, and wraps/unwraps the account password in memory. isSupported() gates every consumer (Firefox: unsupported). - Account entity: enrollPasskey (password-verified) stores a passkeyBlob on the account; removePasskey clears it (password-verified); unlockAccountWithPasskey unwraps and returns the same unlocked account the password path returns. Existing PBKDF2/AES seed crypto untouched. - unit tests: 15 service cases (PRF eval, blob validation, mismatch, unsupported) + 8 entity cases (verify-before-enroll, wrong-password rejection, persistence, no-blob rejection)
- SettingsPasskey: supported-gated rendering, enable/remove flows with password verification, error/message states - jest + import auditor: resolve the @Cryptos/<subpath> alias so the passkey service imports resolve everywhere
- SetPassword gains optional hasPasskey/unlockWithPasskey props: when the account has an enrolled passkey, the unlock is attempted automatically (platform biometric/screen-lock prompt) and the password form remains as the fallback, with a 'Use passkey instead' retry affordance - the login page resolves enrollment from the account's passkeyBlob
The settings component now imports the passkey service relatively (webpack cannot resolve @cryptos subpaths); the test mock follows the same specifier. Full suite green.
- ConfirmBtcTransaction: hasPasskey gates the passkey unlock path - SignChallenge: removed broken partial passkey state (unused, was blocking lint) — passkey integration deferred for this flow - all changes pass: lint, import audit, full jest suite
The restore flow had zero error handling: saveAccount and unlockAccount failures were silently swallowed, leaving the user on a blank screen with no indication of what went wrong. Now: - console.error traces the exact failure point (save vs unlock vs decrypt) - the user sees an alert with the error message - unlockAccount logs decryptSeed failures with the salt and version for diagnosing key-derivation mismatches
The restore flow's .catch was swallowing the successful unlock because the Mintlayer API (526) failed inside the unlock's provider fetches. The account IS saved and the password IS correct — the API being down should not prevent navigation to the dashboard. - addresses declared outside the promise chain so the catch can access them - if the account was saved (accountID set), navigate to dashboard even on error; only show the error when the save itself failed - added decryptSeed error logging with salt/version for diagnosis
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
2 participants
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.
💬 Description
Review fixes for #257, stacked on its branch so the diff shows only the fixes.
Security and correctness fixes for the re-enabled external dApp API, plus the dead code and UX issues found in review.
Fixes
Blockers
networkType(never imported) — the whole signChallenge flow was broken'null'asresult, so dApps got a resolved promise instead of a rejectionRequest cancelledwindows.createcallbackSecurity
popupResponse/disconnectSitenow verify the sender is an extension page (content scripts on*://*/*share the same listener)disconnectmethod somojito.disconnect()actually drops the wallet-side sessionBug fixes
getSessionwas sent asactionso session restore never workedmojito.isConnected()always returned false (session shape mismatch)Dead code / DRY
@Browsermodule (storage,runtime,sendPopupResponse) replaces 6 copy-pasted API-detection blocks; unit testedsession_persistence, deadremembercheckbox, fake Bitcoin toggle (now real state, reachable viaparams.permissions), unused request state, useless mode switch, legacy Firefoxbackground-script.jsUX
background.js, so dApps work there (manifest gainsstorage,web_accessible_resources,run_at/all_frames)More
mainafter📷 Screenshots
N/A — no visual changes except error text in the signing password modal.
📋 Checklist:
A-[id of Asana task]A-[id of Asana task]: [short description]prettierchecklintcheck