Open
Conversation
Release: v2.16.0
…g in quote parameters
|
💼 Build Files |
WalkthroughBumps many dev/runtime dependency versions across packages, increments two package versions, reduces ParaSwap MEW wallet fee, adjusts oneInchFusion quote source for MEW, makes several provider init methods async and filters out providers that fail initialization, and applies multiple UI styling changes in the extension. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client
participant Swap as SwapService
participant ProvReg as ProviderRegistry
participant Prov as Provider
rect rgba(200,200,255,0.5)
App->>Swap: request swap init(tokenList)
Swap->>ProvReg: get providers list
loop for each provider
Swap->>Prov: call init(tokenList)
alt init succeeds
Prov-->>Swap: resolved
else init fails
Prov-->>Swap: rejected (error)
Swap->>Swap: record provider in initFailed
end
end
Swap->>ProvReg: remove providers in initFailed
Swap-->>App: return initialized providers
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/signers/polkadot/package.json (1)
24-30:@commitlint/cliappears misplaced in runtime dependencies.This is a commit linting tool that should typically be in
devDependenciesrather thandependencies. While this is a pre-existing issue and not introduced by this PR, consider moving it in a follow-up to avoid bundling unnecessary tooling in production builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/signers/polkadot/package.json` around lines 24 - 30, Move the tooling package entry "@commitlint/cli" out of the "dependencies" block and place it into "devDependencies" in package.json so it isn't bundled into runtime builds; update the JSON by removing "@commitlint/cli": "^20.4.2" from the "dependencies" object and adding the same entry under "devDependencies" (preserve the version) and run a quick install/test to ensure lockfile updates.packages/signers/ethereum/package.json (1)
49-49: Inconsistent version specifier fortypescript-eslint.Same as in
packages/request/package.json- this package is pinned to an exact version (8.56.1) while the related typescript-eslint packages on lines 36-37 use caret prefixes. Consider adding the caret for consistency.Suggested fix
- "typescript-eslint": "8.56.1", + "typescript-eslint": "^8.56.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/signers/ethereum/package.json` at line 49, The typescript-eslint dependency in packages/signers/ethereum/package.json is pinned to "8.56.1" while related typescript-eslint packages use caret ranges; update the dependency string for "typescript-eslint" to use a caret range (e.g., "^8.56.1") for consistency with the other typescript-eslint entries and with packages/request/package.json.packages/request/package.json (1)
48-48: Inconsistent version specifier fortypescript-eslint.This package is pinned to an exact version (
8.56.1) while the related@typescript-eslint/eslint-pluginand@typescript-eslint/parseron lines 35-36 use caret prefixes (^8.56.1). Consider adding the caret for consistency with the rest of the devDependencies.Suggested fix
- "typescript-eslint": "8.56.1", + "typescript-eslint": "^8.56.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/request/package.json` at line 48, The devDependency "typescript-eslint" is pinned to an exact version while related packages "@typescript-eslint/eslint-plugin" and "@typescript-eslint/parser" use caret ranges; update the "typescript-eslint" entry in package.json to use a caret-prefixed version (e.g., change "8.56.1" to "^8.56.1") so its version specifier is consistent with the other `@typescript-eslint` packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/signers/ethereum/package.json`:
- Line 35: The package.json dependency for "@types/node" is set to "^25.3.2"
here while other packages use v22.x causing inconsistent type definitions across
the monorepo; either (A) align all packages to a single `@types/node` version by
updating the dependency entry for "@types/node" in signers—ethereum (and the
other signers and hw-wallets packages) to match the chosen monorepo version, or
(B) if different versions are intentional, add a short rationale in each
package’s package.json or repo documentation explaining the Node target
differences and why distinct `@types/node` versions are required so maintainers
understand the split.
---
Nitpick comments:
In `@packages/request/package.json`:
- Line 48: The devDependency "typescript-eslint" is pinned to an exact version
while related packages "@typescript-eslint/eslint-plugin" and
"@typescript-eslint/parser" use caret ranges; update the "typescript-eslint"
entry in package.json to use a caret-prefixed version (e.g., change "8.56.1" to
"^8.56.1") so its version specifier is consistent with the other
`@typescript-eslint` packages.
In `@packages/signers/ethereum/package.json`:
- Line 49: The typescript-eslint dependency in
packages/signers/ethereum/package.json is pinned to "8.56.1" while related
typescript-eslint packages use caret ranges; update the dependency string for
"typescript-eslint" to use a caret range (e.g., "^8.56.1") for consistency with
the other typescript-eslint entries and with packages/request/package.json.
In `@packages/signers/polkadot/package.json`:
- Around line 24-30: Move the tooling package entry "@commitlint/cli" out of the
"dependencies" block and place it into "devDependencies" in package.json so it
isn't bundled into runtime builds; update the JSON by removing
"@commitlint/cli": "^20.4.2" from the "dependencies" object and adding the same
entry under "devDependencies" (preserve the version) and run a quick
install/test to ensure lockfile updates.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
package.jsonpackages/extension-bridge/package.jsonpackages/extension/package.jsonpackages/hw-wallets/package.jsonpackages/keyring/package.jsonpackages/name-resolution/package.jsonpackages/request/package.jsonpackages/signers/bitcoin/package.jsonpackages/signers/ethereum/package.jsonpackages/signers/kadena/package.jsonpackages/signers/massa/package.jsonpackages/signers/polkadot/package.jsonpackages/storage/package.jsonpackages/swap/package.jsonpackages/swap/src/configs.tspackages/swap/src/providers/oneInchFusion/index.tspackages/types/package.jsonpackages/utils/package.json
| "@typescript-eslint/eslint-plugin": "^8.56.0", | ||
| "@typescript-eslint/parser": "^8.56.0", | ||
| "eslint": "^9.39.2", | ||
| "@types/node": "^25.3.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check `@types/node` versions across all package.json files in the monorepo
echo "=== `@types/node` versions across packages ==="
fd 'package.json' --type f --exec grep -l '@types/node' {} \; | xargs -I {} sh -c 'echo "File: {}"; grep "@types/node" "{}" || true; echo ""'Repository: enkryptcom/enKrypt
Length of output: 1215
Address inconsistent @types/node versions across the monorepo.
The monorepo uses two major versions of @types/node: v25.x in some packages (signers—ethereum, bitcoin, kadena, polkadot—and hw-wallets) and v22.x in others. If this split is intentional (e.g., due to different Node.js targets), document the rationale. Otherwise, align all packages to a single version to avoid potential type definition conflicts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/signers/ethereum/package.json` at line 35, The package.json
dependency for "@types/node" is set to "^25.3.2" here while other packages use
v22.x causing inconsistent type definitions across the monorepo; either (A)
align all packages to a single `@types/node` version by updating the dependency
entry for "@types/node" in signers—ethereum (and the other signers and
hw-wallets packages) to match the chosen monorepo version, or (B) if different
versions are intentional, add a short rationale in each package’s package.json
or repo documentation explaining the Node target differences and why distinct
`@types/node` versions are required so maintainers understand the split.
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 (2)
packages/extension/package.json (2)
26-82:⚠️ Potential issue | 🔴 Critical@amplitude/analytics-browser@2.35.3 cannot be verified as a published version and should be investigated.
The official Amplitude TypeScript repository changelog does not include entries for 2.35.x versions, and public package registries (UNPKG) show 2.33.5 as the most recent available version. This suggests the specified version may not exist or may not be properly published. This must be resolved before merging.
vue 3.5.29 is a safe patch release containing only bug fixes (instance leak prevention, className escaping, transition race condition fix) with no breaking changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/package.json` around lines 26 - 82, The dependency entry "@amplitude/analytics-browser": "^2.35.3" in package.json cannot be verified as a published version; update or revert it to a valid released version (e.g., replace the version with a confirmed published release such as "^2.33.5" or the latest valid version), or remove the dependency if unused, then run yarn/npm install and lockfile update to ensure consistency; verify by checking the npm registry for "@amplitude/analytics-browser" before pushing.
103-142:⚠️ Potential issue | 🔴 CriticalCorrection needed:
@types/chrome0.1.37 does not exist, and rollup 4.59.0 introduces a breaking change that requires verification.The version
@types/chrome@0.1.37does not exist in npm; published versions are0.0.xand0.1.0–0.1.4only. Additionally,rollup@4.59.0introduces a breaking change: it now throws an error if generated output paths would escapeoutput.dir(via../traversal or absolute paths). This is a security fix for CVE-2026-27606 and requires verifying that the build configuration does not rely on path escaping viaentryFileNames,chunkFileNames,assetFileNames, or plugin-emitted files.The other packages (
eslint@9.39.3,vue-tsc@3.2.5) are patch releases with no breaking changes.@types/lodash@4.17.24and@types/node@22.19.13are patch updates without documented breaking changes, though@types/nodeshould be verified for TypeScript version compatibility and undici-types changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/package.json` around lines 103 - 142, The package.json lists a non-existent `@types/chrome`@0.1.37 and a potentially breaking rollup@4.59.0; change "@types/chrome" to a published version (e.g., 0.1.4 or the latest 0.0.x) and either pin rollup to a known-safe v4 release or audit/adjust your Rollup config: inspect and fix any usage of entryFileNames, chunkFileNames, assetFileNames or plugin-emitted file names that produce "../" or absolute paths so outputs never escape output.dir, and run a full build to confirm no errors; while here also verify `@types/node` (and undici-types) compatibility with your TypeScript version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/extension/package.json`:
- Around line 26-82: The dependency entry "@amplitude/analytics-browser":
"^2.35.3" in package.json cannot be verified as a published version; update or
revert it to a valid released version (e.g., replace the version with a
confirmed published release such as "^2.33.5" or the latest valid version), or
remove the dependency if unused, then run yarn/npm install and lockfile update
to ensure consistency; verify by checking the npm registry for
"@amplitude/analytics-browser" before pushing.
- Around line 103-142: The package.json lists a non-existent
`@types/chrome`@0.1.37 and a potentially breaking rollup@4.59.0; change
"@types/chrome" to a published version (e.g., 0.1.4 or the latest 0.0.x) and
either pin rollup to a known-safe v4 release or audit/adjust your Rollup config:
inspect and fix any usage of entryFileNames, chunkFileNames, assetFileNames or
plugin-emitted file names that produce "../" or absolute paths so outputs never
escape output.dir, and run a full build to confirm no errors; while here also
verify `@types/node` (and undici-types) compatibility with your TypeScript
version.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue (1)
292-293: Add overflow guards for fixed-width columns to avoid row misalignmentWith fixed widths at Line 292 and Line 333, long currency/localized values can wrap or overflow and make row heights inconsistent. Add truncation in the value block to keep layout stable.
💡 Suggested CSS adjustment
&__value { text-align: right; width: 90px; flex-shrink: 0; + overflow: hidden; } &__usd { font-size: 15px; font-weight: 600; color: `@primaryLabel`; margin: 0 0 2px 0; line-height: 1.3; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } &__price { font-size: 12px; font-weight: 400; color: `@tertiaryLabel`; margin: 0; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; &::before { content: '@'; opacity: 0.7; } }Also applies to: 333-334
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue` around lines 292 - 293, The fixed-width column blocks that set "width: 80px; flex-shrink: 0;" (and the similar block at the later pair of lines) need overflow guards so long currency/localized values don't wrap and change row height; update the CSS for the cell/value container referenced in network-assets-item.vue to include "overflow: hidden; text-overflow: ellipsis; white-space: nowrap;" (or apply these rules to the direct child that renders the value) so text is truncated with an ellipsis and layout remains stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue`:
- Around line 292-293: The fixed-width column blocks that set "width: 80px;
flex-shrink: 0;" (and the similar block at the later pair of lines) need
overflow guards so long currency/localized values don't wrap and change row
height; update the CSS for the cell/value container referenced in
network-assets-item.vue to include "overflow: hidden; text-overflow: ellipsis;
white-space: nowrap;" (or apply these rules to the direct child that renders the
value) so text is truncated with an ellipsis and layout remains stable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b791262f-bb00-426d-ab57-a1d9d9b3cd32
📒 Files selected for processing (2)
packages/extension/src/ui/action/views/network-assets/components/network-assets-header.vuepackages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/extension/src/ui/action/views/swap/index.vue (1)
905-914: Empty::beforepseudo-element has no visual effect.The pseudo-element defines positioning and dimensions but lacks any visual properties (background, color, border, etc.). This appears to be unused code or an incomplete implementation.
💡 Consider removing or completing the pseudo-element
If this was intended as a decorative element, add visual properties:
&::before { content: ''; position: absolute; top: 0; left: 0; right: 0; height: 200px; pointer-events: none; z-index: 0; + background: linear-gradient(135deg, rgba(98, 126, 234, 0.05) 0%, transparent 100%); }Or remove it if not needed:
overflow: hidden; - - &::before { - content: ''; - position: absolute; - top: 0; - left: 0; - right: 0; - height: 200px; - pointer-events: none; - z-index: 0; - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/ui/action/views/swap/index.vue` around lines 905 - 914, The ::before pseudo-element in the CSS block inside the swap component is empty and has no visual effect; either remove the entire &::before rule or complete it by adding intended visual properties (e.g., background, gradient, opacity, border, or box-shadow) and ensure pointer-events and z-index remain correct; update the selector (&::before) in the swap/index.vue styles where height: 200px and positioning are defined so the intended decorative effect is actually visible or the dead rule is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/extension/src/ui/action/views/swap/components/swap-token-to-amount/index.vue`:
- Around line 93-96: The .focus styles never apply because isFocus is never
changed; either add a reactive isFocus boolean on the component and toggle it on
the amount input's focus and blur handlers (set isFocus = true on `@focus` and
isFocus = false on `@blur`, then bind the container class so it gets "focus" when
isFocus is true), or drop the JS and change the selector to use :focus-within on
the container so the focus ring appears whenever the amount input is focused;
update the template to ensure the amount input triggers the chosen approach.
In `@packages/swap/src/index.ts`:
- Around line 171-180: Filtering out failed providers after initialization
(using initFailed and Provider.init) leaves this.providers missing entries that
public methods expect; update the public entry points (getSwap, getStatus,
submitRFQOrder) to defensively handle missing providers by checking the result
of this.providers.find(...) and returning a clean failure/error when not found
instead of dereferencing it; keep the existing initFailed logic but ensure each
method performs a null-check on the provider (by provider id/name from
quote.provider or options.provider) and returns a clear error or throws a
well-defined exception when the provider is absent.
---
Nitpick comments:
In `@packages/extension/src/ui/action/views/swap/index.vue`:
- Around line 905-914: The ::before pseudo-element in the CSS block inside the
swap component is empty and has no visual effect; either remove the entire
&::before rule or complete it by adding intended visual properties (e.g.,
background, gradient, opacity, border, or box-shadow) and ensure pointer-events
and z-index remain correct; update the selector (&::before) in the
swap/index.vue styles where height: 200px and positioning are defined so the
intended decorative effect is actually visible or the dead rule is eliminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21093c06-d0e0-42e2-a524-ff9ed5987a4d
📒 Files selected for processing (12)
packages/extension/src/ui/action/views/swap/components/send-address-input.vuepackages/extension/src/ui/action/views/swap/components/swap-network-select/index.vuepackages/extension/src/ui/action/views/swap/components/swap-token-amount-input/index.vuepackages/extension/src/ui/action/views/swap/components/swap-token-select/index.vuepackages/extension/src/ui/action/views/swap/components/swap-token-to-amount/index.vuepackages/extension/src/ui/action/views/swap/index.vuepackages/swap/package.jsonpackages/swap/src/index.tspackages/swap/src/providers/oneInch/index.tspackages/swap/src/providers/oneInchFusion/index.tspackages/swap/src/providers/paraswap/index.tspackages/swap/src/providers/zerox/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/swap/package.json
Comment on lines
93
to
96
| &.focus { | ||
| border: 1px solid @primary; | ||
| box-shadow: 0 0 0 3px rgba(98, 126, 234, 0.12); | ||
| } |
There was a problem hiding this comment.
The new focus ring never becomes active.
Line 93 styles a .focus state, but this component never mutates isFocus, so the container never receives that class. The new focus treatment won't appear when the amount input is focused.
💡 Suggested fix
- <div class="swap-token-input" :class="{ focus: isFocus }">
+ <div class="swap-token-input">-import { computed, ref } from 'vue';
+import { computed } from 'vue';
...
-const isFocus = ref(false);- &.focus {
+ &:focus-within {
border: 1px solid `@primary`;
box-shadow: 0 0 0 3px rgba(98, 126, 234, 0.12);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/extension/src/ui/action/views/swap/components/swap-token-to-amount/index.vue`
around lines 93 - 96, The .focus styles never apply because isFocus is never
changed; either add a reactive isFocus boolean on the component and toggle it on
the amount input's focus and blur handlers (set isFocus = true on `@focus` and
isFocus = false on `@blur`, then bind the container class so it gets "focus" when
isFocus is true), or drop the JS and change the selector to use :focus-within on
the container so the focus ring appears whenever the amount input is focused;
update the template to ensure the amount input triggers the chosen approach.
Comment on lines
+171
to
+180
| const initFailed: string[] = []; | ||
| await Promise.all( | ||
| this.providers.map((Provider) => Provider.init(this.tokenList.all)), | ||
| this.providers.map((Provider) => { | ||
| const initedProv = Provider.init(this.tokenList.all).catch(() => { | ||
| initFailed.push(Provider.name); | ||
| }); | ||
| return initedProv; | ||
| }), | ||
| ); | ||
| this.providers = this.providers.filter((p) => !initFailed.includes(p.name)); |
There was a problem hiding this comment.
Filtering failed providers here makes later find() calls unsafe.
After Line 180, this.providers is no longer guaranteed to contain every quote.provider / options.provider value the public methods receive. getSwap, getStatus, and submitRFQOrder still dereference the find() result unconditionally, so a stale quote or status object now turns into a runtime exception instead of a clean failure.
🛡️ Suggested guard for the public entry points
getSwap(
quote: SwapQuote,
context?: { signal?: AbortSignal },
): Promise<ProviderSwapResponse | null> {
const provider = this.providers.find((p) => p.name === quote.provider);
- return provider.getSwap(quote, context);
+ if (!provider) return Promise.resolve(null);
+ return provider.getSwap(quote, context);
}
getStatus(options: StatusOptionsResponse): Promise<TransactionStatus | null> {
const provider = this.providers.find((p) => p.name === options.provider);
- return provider.getStatus(options.options);
+ if (!provider) return Promise.resolve(null);
+ return provider.getStatus(options.options);
}
submitRFQOrder(options: RFQOptionsResponse): Promise<string> {
const provider = this.providers.find((p) => p.name === options.provider);
- return (provider as ProviderWithRFQ).submitRFQOrder(options.options);
+ if (!provider) {
+ return Promise.reject(
+ new Error(`Provider ${options.provider} is unavailable`),
+ );
+ }
+ return (provider as ProviderWithRFQ).submitRFQOrder(options.options);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/swap/src/index.ts` around lines 171 - 180, Filtering out failed
providers after initialization (using initFailed and Provider.init) leaves
this.providers missing entries that public methods expect; update the public
entry points (getSwap, getStatus, submitRFQOrder) to defensively handle missing
providers by checking the result of this.providers.find(...) and returning a
clean failure/error when not found instead of dereferencing it; keep the
existing initFailed logic but ensure each method performs a null-check on the
provider (by provider id/name from quote.provider or options.provider) and
returns a clear error or throws a well-defined exception when the provider is
absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Style
Chores