Conversation
Release: v2.16.0
…g in quote parameters
|
💼 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:
WalkthroughBumps many dev/runtime dependency versions across packages, increments package versions in Changes
Sequence Diagram(s)sequenceDiagram
participant Client as App
participant Swap as SwapService
participant Reg as ProviderRegistry
participant Prov as Provider
rect rgba(200,200,255,0.5)
Client->>Swap: init swap(tokenList)
Swap->>Reg: get providers list
loop per provider
Swap->>Prov: await init(tokenList)
alt init resolves
Prov-->>Swap: resolved
else init throws
Prov-->>Swap: rejected
Swap->>Swap: record initFailed[prov.name]=true
end
end
Swap->>Reg: remove providers where initFailed
Swap-->>Client: 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
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
…date-metadata component
There was a problem hiding this comment.
Actionable comments posted: 2
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/polkadot/ui/dot-update-metadata.vue (1)
159-171:⚠️ Potential issue | 🟡 MinorClose the reentrancy gap before
isLoadingis set.Line 170 sets
isLoadingonly after awaitingwindowPromise(Line 160). A quick double-click can enterapprovetwice before the disabled state applies.Proposed fix
const approve = async () => { + if (isLoading.value) return; + isLoading.value = true; const { Resolve, Request } = await windowPromise; if ( !Request.value.params || Request.value.params.length < 1 || !metadata.value || !metadata.value.genesisHash ) { + isLoading.value = false; return Resolve.value({ error: getCustomError('No params') }); } - - isLoading.value = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension/src/providers/polkadot/ui/dot-update-metadata.vue` around lines 159 - 171, The approve handler has a reentrancy gap because isLoading.value is set only after awaiting windowPromise; move the isLoading.value = true assignment to the very start of the approve function (before awaiting windowPromise) so the UI disables immediately, and ensure you clear isLoading.value = false on every early return/error path (including when returning Resolve.value for missing Request.value.params or metadata.value.genesisHash) to avoid leaving the UI locked; update references in the approve function (windowPromise, Resolve, Request, metadata) accordingly.
🤖 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/providers/polkadot/ui/dot-update-metadata.vue`:
- Around line 127-155: The validation function validateMetadataAgainstApi should
be hardened: move the getAllNetworks() call and the targetNetwork lookup inside
the try block, wrap the targetNetwork.api() and any RPC-dependent accesses
(api.genesisHash, api.runtimeVersion.specVersion, targetNetwork.decimals) with a
cancellable/timeout wrapper so RPC calls fail fast, and include the caught error
details in the returned message (e.g., return or throw a string/error that
includes the original exception); specifically update validateMetadataAgainstApi
to perform network discovery and API connection inside try, apply a timeout to
targetNetwork.api() and subsequent ApiPromise usage, and ensure the catch block
returns or rethrows a wrapped error containing the original error message/stack
for easier debugging.
- Around line 178-187: The Promise from
mstorage.addMetadata(metadata.value.genesisHash, JSON.stringify(metadata.value))
currently has no .catch, so failures never resolve the request; add a .catch
handler on that promise to call Resolve.value with an error payload (e.g.,
result: JSON.stringify(false) and/or an error message) and ensure
isLoading.value = false is set in both success and failure paths (or move
isLoading.value = false into a .finally that runs after catch), referencing
mstorage.addMetadata, Resolve.value, metadata.value.genesisHash, and
isLoading.value to locate the code to change.
---
Outside diff comments:
In `@packages/extension/src/providers/polkadot/ui/dot-update-metadata.vue`:
- Around line 159-171: The approve handler has a reentrancy gap because
isLoading.value is set only after awaiting windowPromise; move the
isLoading.value = true assignment to the very start of the approve function
(before awaiting windowPromise) so the UI disables immediately, and ensure you
clear isLoading.value = false on every early return/error path (including when
returning Resolve.value for missing Request.value.params or
metadata.value.genesisHash) to avoid leaving the UI locked; update references in
the approve function (windowPromise, Resolve, Request, metadata) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7096b0b-0607-44c9-9fbd-ddb3b5dca586
📒 Files selected for processing (1)
packages/extension/src/providers/polkadot/ui/dot-update-metadata.vue
Enable NFT tab on Rootstock
… in transaction verification
…nto devop/release-v2-17
gamalielhere
approved these changes
Apr 13, 2026
NickKelly1
approved these changes
Apr 13, 2026
NickKelly1
approved these changes
Apr 13, 2026
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
New Features
Style
Chores