Conversation
Release v2.15
…loading states and improved layout
… and permit details display
…d security notice
… encryption key request UI
…nd responsive design
… responsive design
- Updated styles for send-contacts-list, send-fee-select, send-from-contacts-list, send-header, send-input-amount, send-nft-select, send-address-input, send-alert, and send-token-select components. - Improved layout responsiveness and visual consistency with new padding, margin, and border adjustments. - Enhanced hover and active states for better user interaction feedback. - Applied scoped styles to prevent style leakage and ensure component encapsulation. - Introduced animations for dropdowns and alerts to improve user experience. - Adjusted font sizes, weights, and colors for better readability and aesthetics.
…and background effects
…ed layout, styling, and animations
…yling across network activity views
… price, price change percentage, and sparkline data
…on components for a cleaner UI
…nents to enhance UI consistency
fix(signers): rename test suite from Ethereum to Bitcoin signing
|
💼 Build Files |
|
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:
WalkthroughThis PR performs a major dependency upgrade across the monorepo, introduces market data display features (token price, price change, sparkline charts), adds RWA (Real World Asset) filtering support, implements loading states for asset selection, refactors multiple blockchain provider UI components with scoped styling and hover/focus states, and updates window type definitions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 12
Note
Due to the large number of review comments, Critical, Major 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 (13)
packages/extension/src/ui/action/views/network-activity/components/transaction-timer.vue (1)
27-31:⚠️ Potential issue | 🟡 Minor
setIntervalis never cleared — potential memory leak.
updateTimer()creates an interval that is never stored or cleaned up on component unmount. If this component is conditionally rendered (e.g., only while a transaction is pending), repeated mount/unmount cycles will stack leaked intervals.🛡️ Proposed fix using `onBeforeUnmount`
-import { onMounted, ref } from 'vue'; +import { onMounted, onBeforeUnmount, ref } from 'vue'; import moment from 'moment';+let intervalId: ReturnType<typeof setInterval> | null = null; + const updateTimer = () => { - setInterval(() => { + intervalId = setInterval(() => { timer.value = moment.duration(moment(props.date).diff(Date.now())); }, 1000); }; + +onBeforeUnmount(() => { + if (intervalId) { + clearInterval(intervalId); + } +});packages/extension/src/ui/provider-pages/enkrypt/unlock-keyring.vue (1)
97-114:⚠️ Potential issue | 🟡 MinorMissing
.catch()onsendToBackground— UI can get stuck in processing state.If
sendToBackgroundrejects (e.g., extension context invalidated, message port closed),isProcessingremainstrueand the user is stuck with no way to retry or see an error.Proposed fix
sendToBackground({ method: InternalMethods.unlock, params: [password.value, false], }).then(res => { if (res.error) { errorMsg.value = res.error.message; isError.value = true; isProcessing.value = false; } else { Resolve.value({ result: JSON.stringify(res.result), }); } - }); + }).catch(() => { + isError.value = true; + isProcessing.value = false; + });packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue (1)
139-140:⚠️ Potential issue | 🟠 Major
JSON.parsecan throw onundefined,null, or malformed sparkline data.If
token.sparklineisundefinedornull, the check!== ''passes andJSON.parse(undefined)throws aSyntaxError, crashing the component. Malformed JSON strings would also crash here.🛡️ Proposed fix: add a safe parse
data: - props.token.sparkline !== '' ? JSON.parse(props.token.sparkline) : [], + (() => { + try { + return props.token.sparkline ? JSON.parse(props.token.sparkline) : []; + } catch { + return []; + } + })(),packages/extension/src/providers/common/ui/send-transaction/send-header.vue (1)
3-9:⚠️ Potential issue | 🟡 MinorClickable area mismatch: cursor suggests interactivity on the full selector, but only the arrow icon toggles the dropdown.
The
<a class="send-transaction__selector">hascursor: pointer(line 118), but@click="toggleSelector"is only on<switch-arrow>(line 8). Users clicking the "Send token" / "Send NFT" text will see a pointer cursor but get no response. Consider moving the@clickto the parent<a>element, or conditionally applyingcursor: pointeronly whenisNftAvailableis true.Proposed fix
- <a class="send-transaction__selector"> + <a class="send-transaction__selector" :class="{ clickable: isNftAvailable }" `@click`="isNftAvailable && toggleSelector()"> <span v-if="isSendToken">Send token</span> <span v-else>Send NFT</span> <switch-arrow v-if="isNftAvailable" - `@click`="toggleSelector" ></switch-arrow>And in the styles:
&__selector { - cursor: pointer; + cursor: default; + + &.clickable { + cursor: pointer; + }packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (1)
267-271:⚠️ Potential issue | 🟠 MajorPre-existing bug:
selectedFeedefault is alwaysREGULARregardless of network.The condition
props.network.name === NetworkNames.Ethereum || NetworkNames.Binancealways evaluates totruebecauseNetworkNames.Binanceis a non-empty string (truthy). The intended logic is likely:props.network.name === NetworkNames.Ethereum || props.network.name === NetworkNames.BinanceAs a result,
ECONOMYis never selected as the default fee tier for any network. This is a pre-existing issue but worth fixing while you're in this file.🐛 Proposed fix
const selectedFee = ref<GasPriceTypes>( - props.network.name === NetworkNames.Ethereum || NetworkNames.Binance + props.network.name === NetworkNames.Ethereum || props.network.name === NetworkNames.Binance ? GasPriceTypes.REGULAR : GasPriceTypes.ECONOMY, );packages/name-resolution/package.json (1)
44-46:⚠️ Potential issue | 🟡 MinorRepository URL contains a
<FILL_IT>placeholder.While this appears to be pre-existing, the repository URL on Line 45 is still a placeholder. Since this file is being updated, it would be a good opportunity to fix it (likely
enkryptcom/enKrypt/tree/main/packages/name-resolution).packages/extension/src/providers/ethereum/ui/eth-verify-transaction.vue (4)
290-293:⚠️ Potential issue | 🔴 CriticalBug: Operator precedence causes condition to always be truthy.
NetworkNames.Binanceis a non-empty string, so(network.value.name === NetworkNames.Ethereum) || NetworkNames.Binanceis always truthy.selectedFeewill always beGasPriceTypes.REGULARregardless of the actual network.🐛 Proposed fix
selectedFee.value = - network.value.name === NetworkNames.Ethereum || NetworkNames.Binance + network.value.name === NetworkNames.Ethereum || network.value.name === NetworkNames.Binance ? GasPriceTypes.REGULAR : GasPriceTypes.ECONOMY;
302-387:⚠️ Potential issue | 🔴 Critical
isLoadingnever clears ifdecodeTxrejects.
isLoading = falseis only set in the.finally()oftx.getGasCosts()(line 385–386). IfdecodeTx(line 302) rejects, theawaitthrows and execution never reaches the gas-cost chain, leaving the user stuck on an infinite loading spinner with no error message.Wrap the entire loading sequence in
try/catch/finally, or moveisLoading = falseand error handling to the outer scope.🐛 Sketch of fix
+ try { await decodeTx( Request.value.params![0] as EthereumTransaction, network.value as EvmNetwork, ).then(decoded => { // ... existing .then() logic ... }); const web3 = new Web3Eth(network.value.node); const tx = new Transaction( Request.value.params![0] as EthereumTransaction, web3, ); await tx .getGasCosts() .then(async gasvals => { // ... existing .then() logic ... }) .catch(e => { errorMsg.value = e.message; - }) - .finally(() => { - isLoading.value = false; }); + } catch (e: any) { + errorMsg.value = e.message || 'Failed to load transaction'; + } finally { + isLoading.value = false; + }
401-494:⚠️ Potential issue | 🟠 MajorMissing
awaitontx.getFinalizedTransaction(...)— unhandled rejection risk.
getFinalizedTransaction(...).then(...)is not awaited, so if the promise rejects before the.thenruns (or if.then's callback throws without being caught), the rejection is unhandled. Also,isProcessingwill never be reset tofalsein error paths.Consider
awaiting the chain and wrapping intry/finallyto ensureisProcessingis always reset:- tx.getFinalizedTransaction({ gasPriceType: selectedFee.value }).then( + await tx.getFinalizedTransaction({ gasPriceType: selectedFee.value }).then(
428-428:⚠️ Potential issue | 🟡 MinorTypo:
'UKNWN'→'UNKNWN'or'UNKNOWN'.Minor typo in the fallback token symbol.
packages/extension/src/providers/ethereum/ui/eth-decrypt.vue (1)
152-170:⚠️ Potential issue | 🟠 Major
isProcessingis never reset on failure — button stays permanently disabled.If
sendToBackgroundrejects (or the.thencallback throws), there's no.catchto resetisProcessing. The user would be stuck with a disabled "Decrypting..." button and no way to retry or decline (decline still works, but the primary action is dead).Compare with
eth-sign-typedata.vue(lines 281-299) which wraps the flow intry/catchand resetsisProcessingin the catch block.Proposed fix: add error handling
const approve = async () => { if (isProcessing.value) return; isProcessing.value = true; - const { Request, sendToBackground, Resolve } = await windowPromise; - const encryptedMessage = Request.value.params![0] as string; - sendToBackground({ - method: InternalMethods.ethereumDecrypt, - params: [encryptedMessage, account.value], - }).then(res => { - if (res.error) { - Resolve.value(res); - } else { - Resolve.value({ - result: JSON.stringify(res.result), - }); - } - }); + try { + const { Request, sendToBackground, Resolve } = await windowPromise; + const encryptedMessage = Request.value.params![0] as string; + sendToBackground({ + method: InternalMethods.ethereumDecrypt, + params: [encryptedMessage, account.value], + }).then(res => { + if (res.error) { + Resolve.value(res); + } else { + Resolve.value({ + result: JSON.stringify(res.result), + }); + } + }).catch(e => { + console.error(e); + isProcessing.value = false; + }); + } catch { + isProcessing.value = false; + } };packages/extension/src/providers/ethereum/ui/eth-get-encryption-key.vue (1)
153-170:⚠️ Potential issue | 🟠 Major
isProcessingis never reset on failure — same issue aseth-decrypt.vue.
sendToBackground().then()at Line 161 has no.catch(). If it rejects, the button stays permanently disabled at "Providing...".Proposed fix
const approve = async () => { if (isProcessing.value) return; isProcessing.value = true; - const { sendToBackground, Resolve } = await windowPromise; - sendToBackground({ - method: InternalMethods.getEthereumEncryptionPublicKey, - params: [account.value], - }).then(res => { - if (res.error) { - Resolve.value(res); - } else { - Resolve.value({ - result: JSON.stringify(res.result), - }); - } - }); + try { + const { sendToBackground, Resolve } = await windowPromise; + sendToBackground({ + method: InternalMethods.getEthereumEncryptionPublicKey, + params: [account.value], + }).then(res => { + if (res.error) { + Resolve.value(res); + } else { + Resolve.value({ + result: JSON.stringify(res.result), + }); + } + }).catch(e => { + console.error(e); + isProcessing.value = false; + }); + } catch { + isProcessing.value = false; + } };packages/extension/src/providers/ethereum/ui/eth-sign-message.vue (1)
153-165:⚠️ Potential issue | 🟡 Minor
isProcessingis set after theawait— race window for double-submission.The guard at Line 154 checks
isProcessing, but the flag isn't set until Line 157, afterawait windowPromiseat Line 155. A second click arriving during theawaityield would pass the guard.Move
isProcessing.value = trueimmediately after the guard, consistent with the other components in this PR.Proposed fix
const approve = async () => { if (isProcessing.value) return; + isProcessing.value = true; const { Request, Resolve } = await windowPromise; const msg = Request.value.params![0] as `0x{string}`; - isProcessing.value = true; MessageSigner({
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 30: The publish script in package.json currently hardcodes an OTP ("--otp
151351") in the "publish" script (the yarn workspaces foreach ... exec 'yarn npm
publish --access public --otp 151351' command) which must be removed; update the
"publish" script to omit the literal OTP and instead pass the OTP from
invocation or environment (e.g. use npm_config_otp / NPM_CONFIG_OTP or allow
callers to supply --otp at runtime so the executed yarn npm publish uses the
live OTP), ensuring the changed script still runs the same workspaces foreach
exec flow but reads the OTP dynamically rather than embedding 151351.
In `@packages/extension/src/providers/ethereum/ui/eth-decrypt.vue`:
- Around line 405-411: The scoped <style> in eth-decrypt.vue currently targets
.common-popup__content but will not match elements inside the child CommonPopup
component; update the component styles so the override applies by either
removing the scoped attribute from the <style lang="less"> block or by changing
the selector to use Vue’s deep combinator (e.g. :deep(.common-popup__content))
so the rule pierces child component scope; apply the same change to
eth-get-encryption-key.vue and eth-sign-message.vue where .common-popup
overrides are used.
In `@packages/extension/src/providers/ethereum/ui/eth-get-encryption-key.vue`:
- Around line 401-407: The scoped CSS override for .common-popup__content in
eth-get-encryption-key.vue won’t affect the CommonPopup child because the style
block is scoped; update the selector to use the deep combinator (e.g.
:deep(.common-popup__content) { ... }) or remove the scoped attribute from the
style block so the override applies globally; locate the style block in
eth-get-encryption-key.vue and change the .common-popup { &__content { ... } }
rule to use :deep() targeting .common-popup__content (or un-scope the style) to
ensure the CommonPopup child receives the override.
In `@packages/extension/src/providers/ethereum/ui/eth-sign-message.vue`:
- Around line 435-441: The scoped CSS selector for .common-popup__content in
eth-sign-message.vue won’t reach the child component because scoped styles are
isolated; update the style to use the Vue deep selector (e.g., ::v-deep or >>>)
or move the rule to a non-scoped/global stylesheet so the override applies to
the child .common-popup__content; search for the .common-popup block in
eth-sign-message.vue (and mirror the same fix you applied in eth-decrypt.vue and
eth-get-encryption-key.vue) and replace the scoped rule with a deep/global rule
that sets padding and max-width.
In `@packages/extension/src/providers/ethereum/ui/eth-sign-typedata.vue`:
- Around line 622-628: The scoped <style> in eth-sign-typedata.vue can't
override the global .common-popup__content rule as written; change the selector
to use Vue's deep selector so the rule targets the global popup class (for
example use ::v-deep .common-popup__content { padding: 0 20px !important;
max-width: 100% !important; } or :deep(.common-popup__content) { ... }), or move
the override into a non-scoped/global stylesheet; update the CSS that currently
uses .common-popup { &__content { ... } } to one of these approaches so
.common-popup__content is actually overridden.
- Around line 218-228: The computed logic that checks for an "Unlimited" amount
is calling BigInt(amount || 0) which will throw on non-numeric strings; wrap the
conversion in a safe check or try/catch inside the computed so malformed
msg.value/msg.amount doesn't break rendering: verify amount is a numeric string
(e.g., /^\d+$/) before using BigInt, or perform BigInt conversion inside a
try/catch and fall back to leaving amount unchanged and isUnlimited=false on
error; update the block referencing amount and isUnlimited accordingly so
non-numeric values are handled safely.
In `@packages/extension/src/providers/ethereum/ui/eth-verify-transaction.vue`:
- Around line 946-1105: The CSS in eth-verify-transaction.vue uses global
selectors (.common-popup, .popup, .transaction-fee, .transaction-fee-item) with
many !important rules which will leak across the extension; scope these rules
either by adding scoped to the <style> tag and wrapping component-child
overrides with :deep(...) where needed (so rules like .common-popup, .popup,
.transaction-fee, .transaction-fee-item become :deep(.common-popup) etc.), or
prefix every selector with a unique parent class for this view (e.g.,
.eth-verify-transaction .common-popup, .eth-verify-transaction .popup,
.eth-verify-transaction .transaction-fee, .eth-verify-transaction
.transaction-fee-item) and remove unnecessary !important flags where possible;
update selectors in the block containing &__header, &__content, &__logo,
&__network, &__buttons and all transaction-fee / transaction-fee-item nested
rules accordingly.
In
`@packages/extension/src/providers/ethereum/ui/send-transaction/verify-transaction/index.vue`:
- Around line 335-345: The h3 selector in verify-transaction/index.vue currently
uses -webkit-background-clip: text and -webkit-text-fill-color: transparent
without defining a background, making the "Verify Transaction" title invisible;
to fix, either add a suitable background/background-image (e.g., a
linear-gradient) to the same h3 rule so the clipped gradient shows through, or
remove the background-clip and -webkit-text-fill-color properties (and leave
color: `@primaryLabel`) so the title renders normally—update the h3 rule
accordingly.
In `@packages/extension/src/ui/action/composables/account-info.ts`:
- Around line 46-50: The block that sets
tokenPrice.value/priceChangePercentage.value/sparkline.value reads
market[0].sparkline_in_24h.price without guarding sparkline_in_24h; update the
assignment for sparkline.value in the composable (where market is used and
tokenPrice.value, priceChangePercentage.value, sparkline.value are set) to use
optional chaining on sparkline_in_24h (e.g., sparkline_in_24h?.price) and fall
back to an empty array or default JSON string when undefined so accessing price
cannot throw; keep the other assignments unchanged.
In `@packages/hw-wallets/package.json`:
- Around line 55-72: The package bump touches multiple Ledger/Trezor libs that
your code accesses via deep/internal imports; verify that imports like
`@ledgerhq/hw-app-btc/lib/createTransaction`, /lib/serializeTransaction,
/lib/bip32, `@ledgerhq/hw-app-eth/lib/services/ledger`,
`@ledgerhq/live-common/lib/hw/`{getDeviceInfo,openApp,getAppAndVersion}, and
`@trezor/connect/lib/utils/pathUtils` still resolve and expose the same APIs in
the new versions, updating import paths or using public SDK APIs if internals
changed; run end-to-end Ledger and Trezor signing flows in hw-wallets (covering
functions that call createTransaction/serializeTransaction, the ledger service
usage, live-common hw helpers, and trezor path utilities) and fix any runtime
errors by switching to supported public exports or pinning compatible versions.
In `@packages/swap/src/index.ts`:
- Around line 139-147: Normalize addresses and use a Set for O(n) filtering:
create a Set from rwaFilterList with all addresses lowercased, then update the
three filters (this.tokenList.all, this.tokenList.top, this.tokenList.trending)
to test membership by comparing t.address.toLowerCase() against that Set instead
of using Array.includes; reference the existing symbols this.rwaFilterList,
this.tokenList.all, this.tokenList.top, this.tokenList.trending and ensure
t.address is normalized before checking.
- Around line 127-147: The RWA compliance fetches (when this.walletId ===
WalletIdentifier.enkrypt) can throw and currently bubble up from init(),
breaking initPromise; wrap the whole fetch logic that assigns this.rwaFilterList
in a try/catch so any network/JSON errors result in this.rwaFilterList = []
instead of throwing. Specifically, in the block referencing IP_COMPLY_URL and
RWA_FILTER_LIST, surround the await fetch(IP_COMPLY_URL) and the conditional
fetch(RWA_FILTER_LIST) with error handling, default to an empty array on any
failure, and ensure subsequent filters on tokenList.all/top/trending use that
safe fallback so init() and initPromise never reject due to compliance lookup
failures.
🟡 Minor comments (10)
packages/hw-wallets/src/trezor/trezorConnect.ts-22-26 (1)
22-26:⚠️ Potential issue | 🟡 MinorPre-existing:
appUrlreferencesmyetherwallet.cominstead ofenkrypt.com.The webextension branch (Line 9) correctly uses
https://www.enkrypt.com, but this branch still points tohttp://www.myetherwallet.com. Consider fixing this while you're touching the function.Proposed fix
await initFunc.init({ lazyLoad: true, manifest: { email: "info@enkrypt.com", - appUrl: "http://www.myetherwallet.com", + appUrl: "https://www.enkrypt.com", }, });packages/extension/src/providers/common/ui/verify-transaction/verify-transaction-account.vue-79-83 (1)
79-83:⚠️ Potential issue | 🟡 MinorHover effect on a non-interactive element may mislead users.
This
divhas no click handler or link behavior, so the hover background transition could signal interactivity where there is none. If this is intentional (e.g., a parent adds click behavior), disregard — otherwise consider removing it to avoid confusing users.packages/extension/src/providers/ethereum/ui/send-transaction/components/send-address-input.vue-175-178 (1)
175-178:⚠️ Potential issue | 🟡 MinorCSS
:disabledselector will never match — input usesreadonlynow.Since Line 17 changed from
:disabledto:readonly, this&:disabledCSS block is dead code. WhendisableDirectInputis true, the input gets thereadonlyattribute, notdisabled, socursor: pointerandopacity: 1won't apply.🔧 Proposed fix
- &:disabled { + &:read-only { cursor: pointer; opacity: 1; }packages/signers/kadena/package.json-33-33 (1)
33-33:⚠️ Potential issue | 🟡 Minor
@types/nodeversion inconsistent across signers packages.This package uses
^25.2.2, but most packages use^22.19.10. The mismatch affects five packages: kadena, polkadot, bitcoin, ethereum, and hw-wallets. A major-version difference in Node type definitions across the monorepo can introduce subtle type incompatibilities. Align these to^22.19.10unless there's a specific reason for the higher version.packages/extension-bridge/package.json-42-42 (1)
42-42:⚠️ Potential issue | 🟡 Minorserialize-error v13 requires Node.js 20 — verify your deployment targets support it.
The major version bump includes one breaking change: Node.js 20 is now required (Node 18 and earlier are no longer supported). The
serializeError()API itself remains compatible. Confirm that your build and runtime environments support Node.js 20.packages/signers/polkadot/package.json-25-25 (1)
25-25:⚠️ Potential issue | 🟡 Minor
@commitlint/cliis listed underdependenciesinstead ofdevDependencies.
@commitlint/cliis a commit-message linting tool and should not be a production dependency. It will be bundled into the published package unnecessarily. Move it todevDependencies.Proposed fix
"dependencies": { - "@commitlint/cli": "^20.4.1", "@enkryptcom/utils": "workspace:^", ... }, "devDependencies": { + "@commitlint/cli": "^20.4.1", "@enkryptcom/types": "workspace:^", ... },packages/extension/src/ui/action/views/send-process/components/send-process-amount.vue-9-9 (1)
9-9:⚠️ Potential issue | 🟡 Minor
token.symbol.toUpperCase()will throw iftokendefaults to{}.The
tokenprop defaults to an empty object, sotoken.symbolwould beundefined. Calling.toUpperCase()onundefinedproduces aTypeError. Add optional chaining or a fallback.🐛 Proposed fix
- <span>{{ token.symbol.toUpperCase() }}</span> + <span>{{ token.symbol?.toUpperCase() }}</span>packages/extension/src/ui/action/views/network-activity/components/network-activity-total.vue-27-27 (1)
27-27:⚠️ Potential issue | 🟡 MinorProp default
'0'doesn't match the conditional check'0.00', risking an unwanted market block render.Line 27 checks
tokenPrice !== '0.00', but the prop default on line 80 is'0'. If this component is ever rendered without the composable providing the value (or during the initial render before the composable sets it),'0' !== '0.00'evaluates totrue, and the market block would render with a$0.00price display.Align the prop default with the sentinel value used in the composable and the template guard.
Proposed fix
tokenPrice: { type: String, - default: '0', + default: '0.00', },Also applies to: 78-81
packages/extension/src/providers/ethereum/ui/eth-decrypt.vue-131-134 (1)
131-134:⚠️ Potential issue | 🟡 MinorFavicon error handler could loop if the fallback icon also fails to load.
If
network.value.iconis an invalid URL (e.g., during the loading error state before network is initialized), settingimg.srcto it will trigger@erroragain, creating an infinite loop of error events.Proposed fix: guard against repeated fallback
const handleFaviconError = (e: Event) => { const img = e.target as HTMLImageElement; - img.src = network.value.icon; + if (img.src !== network.value.icon) { + img.src = network.value.icon; + } };packages/extension/src/providers/ethereum/ui/eth-connect-dapp.vue-20-29 (1)
20-29:⚠️ Potential issue | 🟡 MinorMissing favicon error handler — broken image if URL is invalid.
All other refactored components in this PR (
eth-decrypt,eth-get-encryption-key,eth-sign-message,eth-sign-typedata) include ahandleFaviconErrorfallback tonetwork.value.icon. This component displays the favicon at Line 23 but lacks the@errorhandler, so a broken favicon URL will render a broken image icon.Proposed fix
In the template:
- <img :src="Options.faviconURL" /> + <img :src="Options.faviconURL" `@error`="handleFaviconError" />In the script:
+const handleFaviconError = (e: Event) => { + const img = e.target as HTMLImageElement; + img.src = network.value.icon; +};
🧹 Nitpick comments (37)
packages/extension/src/providers/ethereum/ui/send-transaction/components/send-alert.vue (1)
22-28: Hardcoded border color diverges from@errortheme variable.Line 23 uses
rgba(239, 68, 68, 0.2)while the text and icon reference@error. If the theme's error color ever changes, this border won't follow. Consider deriving it from the variable for consistency:Suggested fix
- border: 1.5px solid rgba(239, 68, 68, 0.2); + border: 1.5px solid fade(`@error`, 20%);packages/signers/bitcoin/tests/sign.test.ts (1)
9-15: Consider renamingethreumSignertobitcoinSignerfor consistency.While the test description was corrected, the variable name
ethreumSignerstill references "Ethereum" despite instantiating aBitcoinSigner. Renaming it would improve code readability and maintain consistency with the corrected test description.♻️ Proposed refactor for naming consistency
- const ethreumSigner = new BitcoinSigner(); + const bitcoinSigner = new BitcoinSigner(); const promises = fixtures.valid.map((f) => { const ecpair = { publicKey: bufferToHex(getPublicKey(hexToBuffer(f.d))), privateKey: `0x${f.d}`, }; - return ethreumSigner.sign(f.m, ecpair).then((sig) => { + return bitcoinSigner.sign(f.m, ecpair).then((sig) => { expect(sig).equals(packages/extension/src/ui/action/views/network-activity/components/transaction-timer.vue (1)
37-44: Unused@keyframes spinblock.This keyframe animation is newly added but never referenced by any
animationproperty in this file. If the spinner component (send-spinner-animation.vue) handles its own animation internally, this block is dead code and should be removed.🧹 Remove unused keyframes
-@keyframes spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } -} -packages/extension/src/providers/ethereum/ui/send-transaction/components/send-token-select.vue (2)
53-70: Consider extracting the repeated brand color into a Less variable.The color
rgb(98, 126, 234)is used at various opacities across ~6 places in this block (border, hover, box-shadow, arrow background). If this accent color ever changes, all instances must be updated manually.A shared Less variable (e.g.,
@accentBlue) would improve maintainability:Example
+@accentBlue: 98, 126, 234; + .send-token-select { - border: 1.5px solid rgba(98, 126, 234, 0.15); + border: 1.5px solid rgba(`@accentBlue`, 0.15); ... - box-shadow: 0 2px 8px rgba(98, 126, 234, 0.06); + box-shadow: 0 2px 8px rgba(`@accentBlue`, 0.06);
64-64: Prefer explicit transition properties overall.
transition: allcan trigger transitions on unintended properties (e.g.,width,height,padding) and may cause unnecessary compositing work. Specifying the targeted properties is a minor best-practice improvement:Suggested change
- transition: all 200ms ease-in-out; + transition: border-color 200ms ease-in-out, background 200ms ease-in-out, box-shadow 200ms ease-in-out, transform 200ms ease-in-out;packages/extension/src/providers/common/ui/verify-transaction/verify-transaction-amount.vue (1)
87-87: Hardcoded color value instead of theme variable.
#16a34ais hardcoded here while other color values in this component (and sibling components) use theme variables like@primaryLabel/@secondaryLabel. Consider extracting this to a theme variable (e.g.,@successLabelor@positiveColor) for consistency and easier theme maintenance.packages/extension/src/ui/provider-pages/enkrypt/unlock-keyring.vue (1)
204-295: Excessive!importantoverrides on child component styles.There are 25+
!importantdeclarations overriding.common-popup,.button,.lock-screen-password-input, and.modal-forgotinternals. While nesting under.unlock-keyringlimits the blast radius, this creates a fragile coupling to child component class names and makes future style changes difficult to reason about.Consider using Vue's
:deep()with ascopedstyle block instead, which would provide the same override capability with proper encapsulation:<style lang="less" scoped> :deep(.common-popup) { padding: 0; background: `@white`; } </style>packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue (2)
97-97:use()is called inside<script setup>, re-registering ECharts components on every instance.ECharts
use()is idempotent but still does unnecessary work per component mount. Move it to module scope (outside<script setup>) or to a shared initialization file.
360-372: Legacy utility classes (.chart,.down,.up) appear unused in this template.The comment says "Legacy class overrides for compatibility," but if no other component relies on these being defined in this file's (unscoped) style block, they can be removed to avoid style leakage.
#!/bin/bash # Check if .chart, .down, .up classes are referenced from other components that might rely on this file's styles rg -n "class=\"chart\"|class=\"down\"|class=\"up\"" --type=vue -C 2packages/extension/src/providers/common/ui/send-transaction/send-fee-select.vue (1)
156-167: Hardcoded color values instead of theme variables.Lines 161 and 165 use hardcoded
#16a34afor the time indicator text and icon color, while the rest of the component consistently uses theme variables (@primaryLabel,@secondaryLabel, etc.). Consider extracting this to a theme variable (e.g.,@successor@green) for consistency and easier theming.packages/extension/src/providers/common/ui/send-transaction/send-header.vue (1)
161-170:@keyframesnested inside a selector block.While Less compiles this correctly, nesting
@keyframesinside&__dropdownis unconventional and may confuse future maintainers. Consider hoisting it to the top level of the.send-transactionblock or outside it entirely.packages/extension/src/providers/common/ui/send-transaction/send-input-amount.vue (2)
115-119: Minor layout shift on focus due to border/padding compensation mismatch.Border increases from
1.5pxto2px(+0.5px per side), but padding decreases by 1px per side (16→15, 20→19) instead of 0.5px. This causes a subtle 0.5px inward content shift on focus. To eliminate the shift entirely, use15.5px/19.5pxpadding — or keep as-is since it's nearly imperceptible.
146-156: Spin-button removal rules targettype="number"but the input istype="text".The
input[type='number']selectors on lines 146–156 won't match since the<input>on line 6 hastype="text". This is dead CSS. Consider removing it to avoid confusion, or keep it if you anticipate the input type may change.packages/extension/src/providers/ethereum/ui/send-transaction/components/send-address-input.vue (1)
19-21: Hardcoded inlinecolor: 'red'/'black'bypasses theme variables.The CSS on Line 163 sets
color:@primaryLabel``, but the inline style will always override it. Consider using@primaryLabel(or a CSS variable) for the valid state and a theme-aware error color for the invalid state, so this component respects any future theming.packages/extension/src/providers/ethereum/ui/send-transaction/verify-transaction/index.vue (1)
291-300:::beforepseudo-element has no visible styling.This pseudo-element is positioned and sized (180px tall) but defines no
background,background-color, orbackground-image. It won't render anything visible. If it's a placeholder for a decorative gradient, please add the intended background; otherwise, remove it to avoid dead CSS.packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (3)
483-490: NFT validation guard is sound, but relies on fragile string matching.The added checks for
'No NFTs found'and'Loading'correctly suppress the "Invalid NFT selected" error during transient states. However, these are magic strings that must stay in sync with the placeholder values set inselectedNft(Line 283) and the NFT list component. If either display string changes, this guard silently breaks.Consider extracting these into named constants shared between the placeholder initialization and this validation.
♻️ Example
+const NFT_PLACEHOLDER_LOADING = 'Loading'; +const NFT_PLACEHOLDER_EMPTY = 'No NFTs found'; + const selectedNft = ref<NFTItemWithCollectionName>({ id: '', contract: '', image: '', - name: 'Loading', + name: NFT_PLACEHOLDER_LOADING, url: '', collectionName: '', type: NFTType.ERC721, });Then in the validation:
if ( !isSendToken.value && !selectedNft.value.id && - selectedNft.value.name !== 'No NFTs found' && - selectedNft.value.name !== 'Loading' + selectedNft.value.name !== NFT_PLACEHOLDER_EMPTY && + selectedNft.value.name !== NFT_PLACEHOLDER_LOADING ) {
872-881:::beforepseudo-element has no visible styling — same issue as in verify-transaction.No
backgroundorbackground-imageis defined, so this 200px-tall positioned element renders nothing. Either add the intended decorative background or remove it.
1019-1057: Staggered child animations — consideranimation-fill-modeinteraction.The parent
.send-transactionhas its ownfadeInUpat 300ms, while children runelementFadeIn(250ms) with staggered delays (0–210ms). Because ofbackwardsfill mode, children beyond the first will initially be invisible (opacity: 0), which is correct. However, child 1 (delay 0ms) finishes its fade-in at 250ms while the parent is still mid-fade atopacity ≈ 0.83. The visual overlap is subtle but the stagger effect may appear muted for early children.This is a design judgment call, not a bug — just flagging for awareness.
packages/swap/src/configs.ts (2)
165-166: Inconsistent URL pattern and missing semicolons.
RWA_FILTER_LISTusesrefs/heads/mainwhile every other raw GitHub URL in this file uses justmain(e.g., Line 144). Both resolve, but prefer the shorter form for consistency.- Both lines are missing trailing semicolons, unlike the rest of the file.
Proposed fix
-const RWA_FILTER_LIST = "https://raw.githubusercontent.com/enkryptcom/dynamic-data/refs/heads/main/configs/filtered-rwa-addresses.json" -const IP_COMPLY_URL = "https://partners.mewapi.io/o/ipcomply" +const RWA_FILTER_LIST = "https://raw.githubusercontent.com/enkryptcom/dynamic-data/main/configs/filtered-rwa-addresses.json"; +const IP_COMPLY_URL = "https://partners.mewapi.io/o/ipcomply";
203-204: Missing trailing comma afterIP_COMPLY_URLin export list.Minor style nit — add a trailing comma for consistency with the rest of the export block and cleaner diffs.
Proposed fix
RWA_FILTER_LIST, - IP_COMPLY_URL + IP_COMPLY_URL,packages/swap/src/index.ts (1)
132-132: Variableresshadows the outerresfrom line 129.The inner
.then((res) => ...)shadows the outer response variable. This is addressed by the refactor proposed above (using separate variable names likecomplyRes/filterRes), but flagging independently in case the larger refactor isn't adopted.packages/extension/src/providers/ethereum/ui/eth-verify-transaction.vue (3)
12-16: Loading spinner lacks accessibility hints.Screen readers won't announce the loading state. Consider adding
role="status"andaria-live="polite"to the container, or anaria-labelon the spinner.- <div v-if="isLoading" class="provider-verify-transaction__loading"> + <div v-if="isLoading" class="provider-verify-transaction__loading" role="status" aria-live="polite"> <div class="provider-verify-transaction__loading-spinner"></div> <p>Loading transaction...</p> </div>
108-115: Prefer strict equality (===) over loose equality (==).Line 110 uses
==for string comparison. While functionally equivalent here,===is idiomatic in TypeScript and consistent with the rest of the file (e.g., line 182 uses!=—which has the same issue).
621-629: Hardcoded color values bypass the theme system.The amount card gradient, box-shadow, approval badge colors (line 720), flow indicator colors (lines 771, 776), and approval-amount color (line 735) all use hardcoded hex/rgba values instead of Less theme variables (e.g.,
@primary). These won't adapt if the extension supports dark mode or theme customization.packages/extension/src/ui/action/views/send-process/components/send-process-nft.vue (1)
80-89: Unusedpselector — no<p>element in this component's template.The template only contains
<h4>and<h6>inside__info, so thisprule is dead CSS. Consider removing it to avoid confusion, or if it's intended for future use, add a comment.packages/extension/src/ui/action/views/send-process/index.vue (1)
92-111: Consider respectingprefers-reduced-motionfor the infinite pulse animation.The
pulseanimation runs indefinitely on the loading indicator. Users who prefer reduced motion (e.g., vestibular disorder) may find this distracting. A simple media query override would improve accessibility.♻️ Suggested addition
&__loading { width: 120px; height: 120px; animation: pulse 2s ease-in-out infinite; } + +@media (prefers-reduced-motion: reduce) { + .send-process__loading { + animation: none; + } +}Also applies to: 208-212
packages/extension/src/ui/action/views/send-process/components/send-process-account.vue (1)
101-109:word-break: break-allmay break human-readable names mid-word.Line 17 shows
h4renders eithername(human-readable) ordisplayAddress. For addresses,break-allis appropriate, but for names like "Alexander" it could split mid-word. Consideroverflow-wrap: break-wordwhich only breaks when necessary and prefers natural break points.♻️ Suggested change
- word-break: break-all; + overflow-wrap: break-word;packages/extension/src/ui/action/views/network-activity/components/network-activity-transaction.vue (1)
377-380: Hardcoded error color#dc2626instead of a theme variable.The
.droppedclass on line 382 uses the theme variable@tertiaryLabel, but.erroruses a hardcoded hex color. For consistency and easier theme maintenance, consider using a theme variable (e.g.,@erroror similar) if one exists.packages/extension/src/ui/action/views/network-activity/index.vue (1)
339-417: Styling updates look cohesive.The
fadeInUpanimation, gradient backgrounds, and::beforedecorative layer withpointer-events: noneare well-structured. Minor note: several hardcoded hex/rgba values (e.g.,#f8fafc,rgba(98, 126, 234, 0.06)) are used instead of theme variables, but this is understandable for fine-grained gradient control.packages/extension/src/ui/action/views/network-activity/components/network-activity-total.vue (1)
95-131:JSON.parseon sparkline data could throw on malformed input.Line 128 calls
JSON.parse(props.sparkline)inside the computed. While the composable always produces the value viaJSON.stringify, a defensive try/catch or a fallback would prevent a runtime error from crashing the chart if the data is ever corrupted or the prop is set from a different source.Proposed fix — safe parse helper
+const parsedSparkline = computed(() => { + if (!props.sparkline) return []; + try { + return JSON.parse(props.sparkline); + } catch { + return []; + } +}); + const chartOption = computed(() => ({ // ... series: [ { // ... - data: props.sparkline !== '' ? JSON.parse(props.sparkline) : [], + data: parsedSparkline.value, }, ], }));packages/extension/src/ui/action/views/asset-detail-view/index.vue (3)
127-171: Chartoptionis aref— it won't react totokenprop changes.Unlike
network-activity-total.vuewhich usescomputedfor itschartOption, this file usesref. The color (line 130), area style color (line 161), and sparkline data (line 168) are all derived fromprops.tokenand evaluated only once at setup. If the same component instance is reused for a different token (e.g., the dialog stays mounted whiletokenchanges), the chart will display stale data.Consider converting to
computedfor consistency withnetwork-activity-total.vueand correct reactivity:Proposed fix
-const option = ref({ +const option = computed(() => ({ width: 140, height: 80, // ... (same content) data: props.token.sparkline !== '' ? JSON.parse(props.token.sparkline) : [], // ... -}); +}));
78-92: Custom token delete section is well-implemented.The warning with a distinct visual treatment (amber background/border, icon) before a destructive action is good UX. The inline SVG is small enough to be acceptable, though extracting it into a reusable icon component would be more consistent with the rest of the codebase (e.g.,
SparklineUp,SparklineDown).
189-404: Styles are not scoped — pre-existing but worth noting with all the new classes.The BEM naming (
asset-detail-view__*) provides reasonable scoping by convention. However, since this PR adds many new class names and other components in the PR usescoped, consider aligning this file in a follow-up.packages/extension/src/ui/action/composables/account-info.ts (1)
39-51: Eliminate redundant API call —getTokenValueinternally callsgetMarketData.Line 40 calls
getMarketDatafor the coingeckoID, and line 41-45 callsgetTokenValuewhich internally callsgetMarketDataagain for the same ID (market-data/index.ts:32). This results in two separate GraphQL API requests for identical data. Reuse themarketresult from line 40 to computefiatAmountdirectly asprice × balance × fiat_exchange_rate, eliminating the second call.packages/extension/src/providers/ethereum/ui/eth-connect-dapp.vue (2)
179-189: No double-click guard onconnect().Other action handlers in this PR (e.g.,
approve()in eth-decrypt, eth-sign-message, eth-sign-typedata) include anisProcessingguard to prevent concurrent submissions.connect()callsResolve.value()directly, and calling it twice could lead to unexpected behavior if the user double-clicks.Proposed fix
+const isProcessing = ref(false); + const connect = async () => { + if (isProcessing.value) return; + isProcessing.value = true; + const { Resolve } = await windowPromise; const accountState = new AccountState(); await accountState.addApprovedAddress(
205-475: Unscoped styles leak.common-popupoverrides globally.The
<style>at Line 205 is not scoped. The.common-popupoverrides (Lines 415-475) apply globally, including heavy!importantusage. If another popup component is rendered in the same context, these overrides will bleed through. In a single-popup extension context this may be tolerable, but it's fragile.Consider scoping this block or namespacing the overrides under a component-specific wrapper class.
packages/extension/src/providers/ethereum/ui/eth-sign-typedata.vue (1)
312-620: ~300 lines of CSS duplicated across 4 components — consider extracting shared styles.The loading spinner,
fadeInanimation, DApp header, signer/account card, and security notice styles are nearly identical acrosseth-decrypt.vue,eth-get-encryption-key.vue,eth-sign-message.vue, and this file. This adds up to ~500+ lines of duplicated CSS.Consider extracting these into a shared Less mixin or partial (e.g.,
dapp-popup-common.less) and importing it, similar to howcommon-popup.lessis already shared.
| "upgrade": "yarn upgrade-interactive", | ||
| "version:bump:patch": "yarn workspaces foreach --worktree -pv --exclude @enkryptcom/extension exec 'yarn version --immediate patch'", | ||
| "publish": "yarn workspaces foreach --worktree -pv --exclude @enkryptcom/extension exec 'yarn npm publish --access public --otp 312003'", | ||
| "publish": "yarn workspaces foreach --worktree -pv --exclude @enkryptcom/extension exec 'yarn npm publish --access public --otp 151351'", |
There was a problem hiding this comment.
Hardcoded OTP in the publish script is a security anti-pattern.
The --otp 151351 value is committed to version control. While npm OTPs expire quickly, this pattern is problematic:
- Error-prone workflow: Developers must remember to edit, commit, and push a new OTP before each publish — and the OTP may expire before the command runs.
- Git history pollution: Expired secrets accumulate in history.
- Misleading: Other contributors may attempt to use the stale OTP.
Instead, pass the OTP at invocation time:
Suggested approach
- "publish": "yarn workspaces foreach --worktree -pv --exclude `@enkryptcom/extension` exec 'yarn npm publish --access public --otp 151351'",
+ "publish": "yarn workspaces foreach --worktree -pv --exclude `@enkryptcom/extension` exec 'yarn npm publish --access public'",Then publish with: yarn publish --otp <live-otp>, or use npm_config_otp environment variable, or configure an automation token that doesn't require OTP.
📝 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.
| "publish": "yarn workspaces foreach --worktree -pv --exclude @enkryptcom/extension exec 'yarn npm publish --access public --otp 151351'", | |
| "publish": "yarn workspaces foreach --worktree -pv --exclude `@enkryptcom/extension` exec 'yarn npm publish --access public'", |
🤖 Prompt for AI Agents
In `@package.json` at line 30, The publish script in package.json currently
hardcodes an OTP ("--otp 151351") in the "publish" script (the yarn workspaces
foreach ... exec 'yarn npm publish --access public --otp 151351' command) which
must be removed; update the "publish" script to omit the literal OTP and instead
pass the OTP from invocation or environment (e.g. use npm_config_otp /
NPM_CONFIG_OTP or allow callers to supply --otp at runtime so the executed yarn
npm publish uses the live OTP), ensuring the changed script still runs the same
workspaces foreach exec flow but reads the OTP dynamically rather than embedding
151351.
| // Override common-popup for this component | ||
| .common-popup { | ||
| &__content { | ||
| padding: 0 20px !important; | ||
| max-width: 100% !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
Scoped style override on .common-popup__content will not take effect.
This <style> block is scoped (Line 180), so Vue scopes all selectors to elements rendered directly in this component's template. The .common-popup__content element is rendered inside the CommonPopup child component's own template (not in a slot), so the scoped selector won't match it.
Compare with eth-connect-dapp.vue (Line 205) which correctly uses <style lang="less"> without scoped, allowing its .common-popup overrides to apply.
Either remove scoped from this component's <style> block, or use Vue's :deep() combinator:
Option A: Use :deep() to pierce child component scope
// Override common-popup for this component
-.common-popup {
- &__content {
- padding: 0 20px !important;
- max-width: 100% !important;
+:deep(.common-popup) {
+ .common-popup__content {
+ padding: 0 20px !important;
+ max-width: 100% !important;
}
}Option B: Remove scoped attribute
-<style lang="less" scoped>
+<style lang="less">Note: The same issue affects eth-get-encryption-key.vue and eth-sign-message.vue which also use scoped with .common-popup overrides.
📝 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.
| // Override common-popup for this component | |
| .common-popup { | |
| &__content { | |
| padding: 0 20px !important; | |
| max-width: 100% !important; | |
| } | |
| } | |
| // Override common-popup for this component | |
| :deep(.common-popup) { | |
| .common-popup__content { | |
| padding: 0 20px !important; | |
| max-width: 100% !important; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/extension/src/providers/ethereum/ui/eth-decrypt.vue` around lines
405 - 411, The scoped <style> in eth-decrypt.vue currently targets
.common-popup__content but will not match elements inside the child CommonPopup
component; update the component styles so the override applies by either
removing the scoped attribute from the <style lang="less"> block or by changing
the selector to use Vue’s deep combinator (e.g. :deep(.common-popup__content))
so the rule pierces child component scope; apply the same change to
eth-get-encryption-key.vue and eth-sign-message.vue where .common-popup
overrides are used.
| // Override common-popup for this component | ||
| .common-popup { | ||
| &__content { | ||
| padding: 0 20px !important; | ||
| max-width: 100% !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
Scoped style override on .common-popup__content won't apply — same issue as eth-decrypt.vue.
Style is scoped (Line 180), so the .common-popup__content override won't reach inside the CommonPopup child component. Use :deep() or remove scoped.
🤖 Prompt for AI Agents
In `@packages/extension/src/providers/ethereum/ui/eth-get-encryption-key.vue`
around lines 401 - 407, The scoped CSS override for .common-popup__content in
eth-get-encryption-key.vue won’t affect the CommonPopup child because the style
block is scoped; update the selector to use the deep combinator (e.g.
:deep(.common-popup__content) { ... }) or remove the scoped attribute from the
style block so the override applies globally; locate the style block in
eth-get-encryption-key.vue and change the .common-popup { &__content { ... } }
rule to use :deep() targeting .common-popup__content (or un-scope the style) to
ensure the CommonPopup child receives the override.
| // Override common-popup for this component | ||
| .common-popup { | ||
| &__content { | ||
| padding: 0 20px !important; | ||
| max-width: 100% !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
Scoped style override on .common-popup__content won't apply — same issue as eth-decrypt.vue and eth-get-encryption-key.vue.
🤖 Prompt for AI Agents
In `@packages/extension/src/providers/ethereum/ui/eth-sign-message.vue` around
lines 435 - 441, The scoped CSS selector for .common-popup__content in
eth-sign-message.vue won’t reach the child component because scoped styles are
isolated; update the style to use the Vue deep selector (e.g., ::v-deep or >>>)
or move the rule to a non-scoped/global stylesheet so the override applies to
the child .common-popup__content; search for the .common-popup block in
eth-sign-message.vue (and mirror the same fix you applied in eth-decrypt.vue and
eth-get-encryption-key.vue) and replace the scoped rule with a deep/global rule
that sets padding and max-width.
| if ( | ||
| amount === | ||
| '115792089237316195423570985008687907853269984665640564039457584007913129639935' || | ||
| amount === | ||
| '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' || | ||
| BigInt(amount || 0) >= | ||
| BigInt('0xffffffffffffffffffffffffffffffffffffffffffff') | ||
| ) { | ||
| amount = 'Unlimited'; | ||
| isUnlimited = true; | ||
| } |
There was a problem hiding this comment.
BigInt(amount || 0) will throw on non-numeric amount strings, crashing the computed property.
amount is derived from arbitrary typed-data message fields (msg.value || msg.amount). If it contains a non-numeric string (e.g., a token symbol was mistakenly placed there, or the data is malformed), BigInt() throws a SyntaxError. Since this runs inside a computed, the unhandled exception will break rendering.
Proposed fix: wrap in try/catch
// Check for unlimited approval (max uint256)
- if (
- amount ===
- '115792089237316195423570985008687907853269984665640564039457584007913129639935' ||
- amount ===
- '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' ||
- BigInt(amount || 0) >=
- BigInt('0xffffffffffffffffffffffffffffffffffffffffffff')
- ) {
- amount = 'Unlimited';
- isUnlimited = true;
+ try {
+ if (
+ amount ===
+ '115792089237316195423570985008687907853269984665640564039457584007913129639935' ||
+ amount ===
+ '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' ||
+ BigInt(amount || 0) >=
+ BigInt('0xffffffffffffffffffffffffffffffffffffffffffff')
+ ) {
+ amount = 'Unlimited';
+ isUnlimited = true;
+ }
+ } catch {
+ // amount is not a valid BigInt-compatible string; leave as-is
}📝 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.
| if ( | |
| amount === | |
| '115792089237316195423570985008687907853269984665640564039457584007913129639935' || | |
| amount === | |
| '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' || | |
| BigInt(amount || 0) >= | |
| BigInt('0xffffffffffffffffffffffffffffffffffffffffffff') | |
| ) { | |
| amount = 'Unlimited'; | |
| isUnlimited = true; | |
| } | |
| try { | |
| if ( | |
| amount === | |
| '115792089237316195423570985008687907853269984665640564039457584007913129639935' || | |
| amount === | |
| '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff' || | |
| BigInt(amount || 0) >= | |
| BigInt('0xffffffffffffffffffffffffffffffffffffffffffff') | |
| ) { | |
| amount = 'Unlimited'; | |
| isUnlimited = true; | |
| } | |
| } catch { | |
| // amount is not a valid BigInt-compatible string; leave as-is | |
| } |
🤖 Prompt for AI Agents
In `@packages/extension/src/providers/ethereum/ui/eth-sign-typedata.vue` around
lines 218 - 228, The computed logic that checks for an "Unlimited" amount is
calling BigInt(amount || 0) which will throw on non-numeric strings; wrap the
conversion in a safe check or try/catch inside the computed so malformed
msg.value/msg.amount doesn't break rendering: verify amount is a numeric string
(e.g., /^\d+$/) before using BigInt, or perform BigInt conversion inside a
try/catch and fall back to leaving amount unchanged and isUnlimited=false on
error; update the block referencing amount and isUnlimited accordingly so
non-numeric values are handled safely.
| h3 { | ||
| font-style: normal; | ||
| font-weight: 700; | ||
| font-size: 24px; | ||
| line-height: 32px; | ||
| font-size: 22px; | ||
| line-height: 28px; | ||
| color: @primaryLabel; | ||
| margin: 0; | ||
| -webkit-background-clip: text; | ||
| -webkit-text-fill-color: transparent; | ||
| background-clip: text; | ||
| } |
There was a problem hiding this comment.
Title text will be invisible — background-clip: text without a background.
Lines 342–344 set -webkit-background-clip: text, background-clip: text, and -webkit-text-fill-color: transparent, which renders the text color as transparent and clips a background to the text shape. However, no background or background-image (e.g., a gradient) is defined here. The color: @primaryLabel`` on Line 340 is overridden by -webkit-text-fill-color: transparent.
The "Verify Transaction" heading will be invisible to the user.
🐛 Proposed fix — either add a gradient or remove the clip properties
Option A: Add a gradient background (if the intent is a gradient title)
h3 {
font-style: normal;
font-weight: 700;
font-size: 22px;
line-height: 28px;
color: `@primaryLabel`;
margin: 0;
+ background: linear-gradient(135deg, `#627eea`, `#8b5cf6`);
-webkit-background-clip: text;
-webkit-text-fill-color: transparent;
background-clip: text;
}Option B: Remove the clip properties (if gradient was unintended)
h3 {
font-style: normal;
font-weight: 700;
font-size: 22px;
line-height: 28px;
color: `@primaryLabel`;
margin: 0;
- -webkit-background-clip: text;
- -webkit-text-fill-color: transparent;
- background-clip: text;
}📝 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.
| h3 { | |
| font-style: normal; | |
| font-weight: 700; | |
| font-size: 24px; | |
| line-height: 32px; | |
| font-size: 22px; | |
| line-height: 28px; | |
| color: @primaryLabel; | |
| margin: 0; | |
| -webkit-background-clip: text; | |
| -webkit-text-fill-color: transparent; | |
| background-clip: text; | |
| } | |
| h3 { | |
| font-style: normal; | |
| font-weight: 700; | |
| font-size: 22px; | |
| line-height: 28px; | |
| color: `@primaryLabel`; | |
| margin: 0; | |
| background: linear-gradient(135deg, `#627eea`, `#8b5cf6`); | |
| -webkit-background-clip: text; | |
| -webkit-text-fill-color: transparent; | |
| background-clip: text; | |
| } |
| h3 { | |
| font-style: normal; | |
| font-weight: 700; | |
| font-size: 24px; | |
| line-height: 32px; | |
| font-size: 22px; | |
| line-height: 28px; | |
| color: @primaryLabel; | |
| margin: 0; | |
| -webkit-background-clip: text; | |
| -webkit-text-fill-color: transparent; | |
| background-clip: text; | |
| } | |
| h3 { | |
| font-style: normal; | |
| font-weight: 700; | |
| font-size: 22px; | |
| line-height: 28px; | |
| color: `@primaryLabel`; | |
| margin: 0; | |
| } |
🤖 Prompt for AI Agents
In
`@packages/extension/src/providers/ethereum/ui/send-transaction/verify-transaction/index.vue`
around lines 335 - 345, The h3 selector in verify-transaction/index.vue
currently uses -webkit-background-clip: text and -webkit-text-fill-color:
transparent without defining a background, making the "Verify Transaction" title
invisible; to fix, either add a suitable background/background-image (e.g., a
linear-gradient) to the same h3 rule so the clipped gradient shows through, or
remove the background-clip and -webkit-text-fill-color properties (and leave
color: `@primaryLabel`) so the title renders normally—update the h3 rule
accordingly.
| if (market && market[0]) { | ||
| tokenPrice.value = market[0].current_price?.toString() || defaultFiatVal; | ||
| priceChangePercentage.value = market[0].price_change_percentage_24h || 0; | ||
| sparkline.value = JSON.stringify(market[0].sparkline_in_24h.price); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the MarketData class and getMarketData method
find . -type f -name "*.ts" | xargs grep -l "getMarketData" | head -20Repository: enkryptcom/enKrypt
Length of output: 862
🏁 Script executed:
# Search for the CoinGeckoTokenMarket type definition
rg -n "sparkline_in_24h" --type=ts -B5 -A5 | head -100Repository: enkryptcom/enKrypt
Length of output: 10312
🏁 Script executed:
# Check the account-info.ts file around line 46-50
cat -n packages/extension/src/ui/action/composables/account-info.ts | sed -n '40,55p'Repository: enkryptcom/enKrypt
Length of output: 784
🏁 Script executed:
# Look for the marketData service and its implementation
find . -type f -name "*market*" -o -name "*Market*" | grep -E "\.(ts|js)$"Repository: enkryptcom/enKrypt
Length of output: 44
🏁 Script executed:
# Find and view the market-data implementation
cat -n packages/extension/src/libs/market-data/index.ts | head -100Repository: enkryptcom/enKrypt
Length of output: 4135
🏁 Script executed:
# Search for CoinGeckoTokenMarket type definition
rg -n "interface CoinGeckoTokenMarket|type CoinGeckoTokenMarket|class CoinGeckoTokenMarket" --type=ts -A10Repository: enkryptcom/enKrypt
Length of output: 886
🏁 Script executed:
# Check if getMarketData uses any query parameters that might control sparkline inclusion
rg -n "getMarketData" --type=ts -B2 -A8 | grep -A10 "getMarketData\("Repository: enkryptcom/enKrypt
Length of output: 85
🏁 Script executed:
# Get the full CoinGeckoTokenMarket type definition
cat -n packages/extension/src/libs/market-data/types.ts | sed -n '23,60p'Repository: enkryptcom/enKrypt
Length of output: 662
🏁 Script executed:
# Check getMarketInfoByIDs implementation and sparkline parameter
cat -n packages/extension/src/libs/market-data/ethvm.ts | head -150Repository: enkryptcom/enKrypt
Length of output: 3362
Add null guard for sparkline_in_24h before accessing its price property.
Line 49 accesses market[0].sparkline_in_24h.price without checking if sparkline_in_24h exists. Although the CoinGeckoTokenMarket type interface declares sparkline_in_24h as required, defensive coding patterns elsewhere in the codebase (e.g., massa-base.ts line 69) guard against it being undefined, suggesting API responses can be incomplete. Add optional chaining to prevent a potential TypeError:
Proposed fix
if (market && market[0]) {
tokenPrice.value = market[0].current_price?.toString() || defaultFiatVal;
priceChangePercentage.value = market[0].price_change_percentage_24h || 0;
- sparkline.value = JSON.stringify(market[0].sparkline_in_24h.price);
+ sparkline.value = market[0].sparkline_in_24h?.price
+ ? JSON.stringify(market[0].sparkline_in_24h.price)
+ : '';
}📝 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.
| if (market && market[0]) { | |
| tokenPrice.value = market[0].current_price?.toString() || defaultFiatVal; | |
| priceChangePercentage.value = market[0].price_change_percentage_24h || 0; | |
| sparkline.value = JSON.stringify(market[0].sparkline_in_24h.price); | |
| } | |
| if (market && market[0]) { | |
| tokenPrice.value = market[0].current_price?.toString() || defaultFiatVal; | |
| priceChangePercentage.value = market[0].price_change_percentage_24h || 0; | |
| sparkline.value = market[0].sparkline_in_24h?.price | |
| ? JSON.stringify(market[0].sparkline_in_24h.price) | |
| : ''; | |
| } |
🤖 Prompt for AI Agents
In `@packages/extension/src/ui/action/composables/account-info.ts` around lines 46
- 50, The block that sets
tokenPrice.value/priceChangePercentage.value/sparkline.value reads
market[0].sparkline_in_24h.price without guarding sparkline_in_24h; update the
assignment for sparkline.value in the composable (where market is used and
tokenPrice.value, priceChangePercentage.value, sparkline.value are set) to use
optional chaining on sparkline_in_24h (e.g., sparkline_in_24h?.price) and fall
back to an empty array or default JSON string when undefined so accessing price
cannot throw; keep the other assignments unchanged.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
310-318:⚠️ Potential issue | 🟠 MajorMissing error handling in
fetchAssets— user can get stuck on the loading screen.If
getAllTokensrejects (e.g., network error),isLoadingAssetsremainstrueindefinitely and the user sees only a spinner with no way to retry or navigate back. Add a.catchto resetisLoadingAssetsand surface the error.🐛 Suggested fix
const fetchAssets = () => { selectedAsset.value = loadingAsset; isLoadingAssets.value = true; return props.network.getAllTokens(addressFrom.value).then(allAssets => { accountAssets.value = allAssets as BTCToken[]; selectedAsset.value = allAssets[0] as BTCToken; isLoadingAssets.value = false; + }).catch(() => { + isLoadingAssets.value = false; + accountAssets.value = []; }); };
🧹 Nitpick comments (4)
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-address-input.vue (1)
175-178: Dead CSS:&:disabledrules will never apply.The input now uses
:readonlyinstead of:disabled, so these:disabledstyles are unreachable. Consider removing them or replacing with&:read-onlyif you still want to style the readonly state.♻️ Suggested fix
- &:disabled { - cursor: pointer; - opacity: 1; - } + &:read-only { + cursor: pointer; + opacity: 1; + }packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-token-select.vue (1)
2-2: Use<button>instead of<a>for a non-navigating click target.An
<a>withouthrefis not keyboard-focusable and won't respond to Enter/Space key presses. Since this element triggers a JavaScript action (opening the token selector) rather than navigating, a<button>is the correct semantic element and provides accessibility for free.♻️ Suggested fix
- <a class="send-token-select" `@click`="emit('update:toggleTokenSelect')"> + <button class="send-token-select" `@click`="emit('update:toggleTokenSelect')">- </a> + </button>And add a reset in the styles:
.send-token-select { + appearance: none; + border: none; + font: inherit; + text-align: left; height: 60px;packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (2)
248-250:showMaxcomputed always returnstrue— consider simplifying.This computed property adds indirection for no runtime benefit. If future logic is planned, a
TODOcomment would clarify intent; otherwise a plain constant is clearer.♻️ Suggested simplification
-const showMax = computed(() => { - return true; -}); +const showMax = true;
719-747: Staggered animation delays may shift when conditional children change.The
nth-childselectors assign fixed delays, butv-ifelements (e.g.,send-token-selectvssend-nft-select) alter which DOM children are present. This means the animation stagger order can change depending on whether the user is sending tokens or NFTs. The visual effect is purely cosmetic, so it's not a functional concern — just worth noting if you want consistent stagger ordering.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
310-318:⚠️ Potential issue | 🟠 MajorMissing error handling leaves the UI stuck on the loading spinner.
If
getAllTokensrejects (network error, timeout, etc.), the.thencallback never executes, soisLoadingAssetsremainstrueand the user sees the spinner indefinitely with no recovery path. Add a.catch(orfinally) to reset the loading state and surface the error.Proposed fix
const fetchAssets = () => { selectedAsset.value = loadingAsset; isLoadingAssets.value = true; - return props.network.getAllTokens(addressFrom.value).then(allAssets => { - accountAssets.value = allAssets as BTCToken[]; - selectedAsset.value = allAssets[0] as BTCToken; - isLoadingAssets.value = false; - }); + return props.network + .getAllTokens(addressFrom.value) + .then(allAssets => { + accountAssets.value = allAssets as BTCToken[]; + selectedAsset.value = allAssets[0] as BTCToken; + }) + .catch(err => { + console.error('Failed to load assets', err); + }) + .finally(() => { + isLoadingAssets.value = false; + }); };packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
455-475:⚠️ Potential issue | 🟠 MajorMissing error handling in
fetchAssets— loading spinner can hang forever.If
props.network.getAllTokens(...)rejects, the.then()callback never runs, soisLoadingAssetsstaystrueand the user is stuck on the loading spinner with no recourse.Proposed fix — add a `.catch()` handler
return props.network.getAllTokens(addressFrom.value).then(allAssets => { accountAssets.value = allAssets as SOLToken[]; const hasParamData = isSendToken.value && tokenParamData && tokenParamData.contract; if (hasParamData) { const selectedToken = accountAssets.value.find( asset => asset.contract === tokenParamData.contract, ); if (selectedToken) { selectedAsset.value = selectedToken as SOLToken; } } else { selectedAsset.value = allAssets[0] as SOLToken; } isLoadingAssets.value = false; + }).catch(() => { + isLoadingAssets.value = false; });
🤖 Fix all issues with AI agents
In
`@packages/extension/src/providers/solana/ui/send-transaction/components/send-address-input.vue`:
- Line 17: The template binds readonly via :readonly="disableDirectInput" but
the component stylesheet targets :disabled, so the styles never apply; update
the CSS selector(s) that currently use :disabled (in this file's scoped styles
for the address input) to use the correct pseudoclass :read-only (or change to a
class-based approach and toggle a class from the component when
disableDirectInput is true), ensuring the rules that set cursor:pointer and
opacity:1 are applied when the input is readonly; locate the selectors
referencing :disabled and replace them with :read-only or the new class tied to
disableDirectInput.
🧹 Nitpick comments (5)
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (2)
248-250:showMaxis a constant disguised as a computed.This always returns
true. If it's not expected to change based on future conditions, simplify toconst showMax = trueor passtruedirectly in the template.
742-781: Staggered fade-in applies to all direct children, including conditionally rendered ones.Elements like
send-alert,transaction-fee-view, andnft-select-listthat toggle visibility will replay the fade-in animation each time they appear. If that's not desired, scope the animation to a specific class or usev-showinstead ofv-iffor those elements.packages/extension/src/providers/solana/ui/send-transaction/components/send-address-input.vue (1)
19-21: Hardcoded inline colors bypass theme tokens.
'red'and'black'won't adapt to dark mode or the design system. Consider using CSS classes toggled by the validity check instead of inline styles.packages/extension/src/providers/solana/ui/send-transaction/components/send-fee-select.vue (1)
120-136: Hardcoded#16a34acolor values — consider using a theme token.Lines 130 and 134 use raw hex for the time badge color. If the design system has a success/green token, using it would improve maintainability and dark-mode readiness.
packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
1015-1054: Staggered animations only cover 8 children.The
nth-childdelays go up to 8, but the send-transaction form can have more direct children (header, from-input, from-contacts, to-input, to-contacts, token-select, assets-list, nft-select, nft-list, input-amount, fee-select, alert, buttons = up to ~13). Children beyond the 8th won't get a stagger delay, though they'll still fade in at 0ms delay due to the generic& > *rule — so the visual impact is minor.
| v-model="address" | ||
| type="text" | ||
| :disabled="disableDirectInput" | ||
| :readonly="disableDirectInput" |
There was a problem hiding this comment.
:disabled CSS selector is now dead code — input uses readonly instead.
Line 17 binds :readonly="disableDirectInput" but Lines 172–175 style :disabled. Since the input is never disabled, the cursor: pointer and opacity: 1 rules won't apply. Update the selector to :read-only or add a class-based approach.
Proposed fix
- &:disabled {
+ &:read-only {
cursor: pointer;
opacity: 1;
}Also applies to: 172-175
🤖 Prompt for AI Agents
In
`@packages/extension/src/providers/solana/ui/send-transaction/components/send-address-input.vue`
at line 17, The template binds readonly via :readonly="disableDirectInput" but
the component stylesheet targets :disabled, so the styles never apply; update
the CSS selector(s) that currently use :disabled (in this file's scoped styles
for the address input) to use the correct pseudoclass :read-only (or change to a
class-based approach and toggle a class from the component when
disableDirectInput is true), ensuring the rules that set cursor:pointer and
opacity:1 are applied when the input is readonly; locate the selectors
referencing :disabled and replace them with :read-only or the new class tied to
disableDirectInput.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/extension/src/providers/ethereum/inject.ts`:
- Around line 71-80: Update the error handling around the sendMessageHandler
call in inject.ts so you don't always rewrite every error to code 4001 and a raw
non-string message; instead, when catching the error preserve its numeric error
code if present (e.g., use e.code when typeof e.code === 'number'), and
otherwise default to a generic JSON-RPC/internal error code like -32000; also
ensure the message is a string (use e.message if e is an Error, else String(e)).
Apply this change in the block that calls this.sendMessageHandler(this.name,
JSON.stringify(request)) so returned/rejected errors propagate accurate code and
message values.
🧹 Nitpick comments (2)
packages/extension/env.d.ts (1)
20-20: Consider using a stricter type instead ofany.The
EnkryptWindowinterface inpackages/extension/src/types/globals.tsalready definesenkryptwith a structured type ({ providers: { [key: string]: any }; settings: SettingsType }). Usinganyhere bypasses that contract onwindow.enkryptaccesses. Consider reusing or referencing the same shape to get type safety on directwindow.enkryptusage.packages/extension/src/providers/ethereum/inject.ts (1)
180-180: Usenew CustomEvent(...)instead of casting to(window as any).CustomEvent.The codebase has proper TypeScript DOM types configured (tsconfig.app.json extends
@vue/tsconfig/tsconfig.dom.jsonwith "dom" in lib), soCustomEventis available as a standard global. The(window as any).CustomEventcast is unnecessary andnew CustomEvent(EIP6963Events.announce, {...})would be cleaner and type-safe.
| const resp = await this.sendMessageHandler( | ||
| this.name, | ||
| JSON.stringify(request), | ||
| ).catch(e => { | ||
| return Promise.reject({ | ||
| code: 4001, | ||
| message: e, | ||
| }); | ||
| }); | ||
| return resp; |
There was a problem hiding this comment.
Hardcoding error code 4001 for all failures is semantically incorrect.
EIP-1193 defines code 4001 specifically as "User Rejected Request." By catching all errors and re-wrapping them with 4001, legitimate provider errors (network failures, invalid params, internal errors, etc.) will be misidentified as user rejections. Downstream dApps that branch on error codes (e.g., to distinguish user cancellation from a network error) will behave incorrectly.
Additionally, message: e assigns the raw caught value (possibly an Error object) to message, which EIP-1193 expects to be a string.
Consider preserving the original error's code when available and only defaulting to 4001 if appropriate:
Suggested approach
const resp = await this.sendMessageHandler(
this.name,
JSON.stringify(request),
).catch(e => {
return Promise.reject({
- code: 4001,
- message: e,
+ code: e?.code ?? 4001,
+ message: typeof e === 'string' ? e : e?.message ?? 'Unknown error',
});
});
return resp;🤖 Prompt for AI Agents
In `@packages/extension/src/providers/ethereum/inject.ts` around lines 71 - 80,
Update the error handling around the sendMessageHandler call in inject.ts so you
don't always rewrite every error to code 4001 and a raw non-string message;
instead, when catching the error preserve its numeric error code if present
(e.g., use e.code when typeof e.code === 'number'), and otherwise default to a
generic JSON-RPC/internal error code like -32000; also ensure the message is a
string (use e.message if e is an Error, else String(e)). Apply this change in
the block that calls this.sendMessageHandler(this.name, JSON.stringify(request))
so returned/rejected errors propagate accurate code and message values.
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Bug Fixes
Chores