Conversation
|
💼 Build Files |
WalkthroughRemoves Shiden/ShidenEVM/Bitrock network definitions and related mappings; adds wallet-address restriction checks and UI flows across the extension (App, Restricted view, connect/send components); updates global env typings; small json-tree-view API rename; many dependency version bumps. Changes
Sequence Diagram(s)mermaid App->>WalletCheck: checkAddress(selectedAccount) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/name-resolution/package.json (1)
45-45: Repository URL contains placeholder.Line 45 contains a placeholder URL
<FILL_IT>in the repository field, which should be corrected to the actual repository path.- "url": "git+https://github.com/<FILL_IT>" + "url": "git+https://github.com/enkryptcom/enKrypt/tree/main/packages/name-resolution"
♻️ Duplicate comments (7)
packages/swap/package.json (1)
58-58: Inconsistent version specifier for typescript-eslint (same as packages/request).Line 58 uses
8.49.0(pinned) while surrounding packages use^8.49.0(flexible).packages/extension-bridge/package.json (1)
64-64: Inconsistent version specifier for typescript-eslint.Line 64 uses
8.49.0(pinned) while other packages use^8.49.0(flexible).packages/signers/bitcoin/package.json (1)
49-49: Inconsistent version specifier for typescript-eslint.Line 49 uses
8.49.0(pinned) while other packages use^8.49.0(flexible).packages/hw-wallets/package.json (2)
25-25: @types/node major version mismatch (same issue as packages/signers/bitcoin).This package uses
^24.10.2while the majority of packages use^22.19.2. Verify if this is intentional.
39-39: Inconsistent version specifier for typescript-eslint.Line 39 uses
8.49.0(pinned) while other packages use^8.49.0(flexible).packages/name-resolution/package.json (1)
39-39: Inconsistent version specifier for typescript-eslint.Line 39 uses
8.49.0(pinned) while other packages use^8.49.0(flexible).packages/storage/package.json (1)
44-44: Inconsistent version specifier for typescript-eslint.Line 44 uses
8.49.0(pinned) while other packages use^8.49.0(flexible).
🧹 Nitpick comments (1)
packages/request/package.json (1)
48-48: Use consistent version specifier for typescript-eslint.Line 48 uses
8.49.0(pinned) while lines 35–36 use^8.49.0(flexible). For consistency within this file, add the caret prefix to match the @typescript-eslint packages.
📜 Review details
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 (19)
package.json(1 hunks)packages/extension-bridge/package.json(1 hunks)packages/extension/package.json(6 hunks)packages/extension/src/providers/ethereum/networks/bitrock.ts(0 hunks)packages/extension/src/providers/ethereum/networks/index.ts(0 hunks)packages/hw-wallets/package.json(2 hunks)packages/keyring/package.json(1 hunks)packages/name-resolution/package.json(1 hunks)packages/request/package.json(1 hunks)packages/signers/bitcoin/package.json(1 hunks)packages/signers/ethereum/package.json(1 hunks)packages/signers/kadena/package.json(1 hunks)packages/signers/massa/package.json(1 hunks)packages/signers/polkadot/package.json(1 hunks)packages/storage/package.json(1 hunks)packages/swap/package.json(1 hunks)packages/types/package.json(1 hunks)packages/types/src/networks.ts(0 hunks)packages/utils/package.json(1 hunks)
💤 Files with no reviewable changes (3)
- packages/extension/src/providers/ethereum/networks/bitrock.ts
- packages/types/src/networks.ts
- packages/extension/src/providers/ethereum/networks/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: buildAll
- GitHub Check: test
🔇 Additional comments (15)
package.json (1)
35-37: ✓ Minor devDependency bumps look appropriate.The version updates to commitlint and @swc/core are incremental and low-risk.
packages/signers/bitcoin/package.json (1)
35-35:⚠️ @types/node major version inconsistency across monorepo signers and hw-wallets packages.This package uses
^24.10.2alongside packages/signers/ethereum, packages/signers/kadena, packages/signers/polkadot, and packages/hw-wallets. Most other packages (extension-bridge, extension, keyring, name-resolution, request, signers/massa, storage, swap, types, utils) use^22.19.2. Different major versions of @types/node in a monorepo workspace can cause type conflicts and build failures.Verify if the v24 versions for these five packages are intentional or should be aligned with the rest of the monorepo.
packages/name-resolution/package.json (1)
40-40: No action required. The viem dependency update from 2.39.3 to 2.41.2 contains no breaking changes—both releases are minor/patch updates with only additive features (chain-specific transaction nonce preference in 2.41.2 and chain.prepareTransactionRequest config in 2.41.1). The update is safe within semver constraints and poses no risk to name resolution functionality.packages/hw-wallets/package.json (1)
55-68: Dependency updates appear compatible with current codebase usage patterns.The specified versions do exist and have been analyzed:
@zondax/ledger-substratev2.0.0 removed the legacy API, but the codebase uses only the modern constructor-based API (newAcalaApp,newKusamaApp, etc.), so compatibility is maintained.@ledgerhq/hw-app-btcv10.13.0 and@ledgerhq/hw-app-ethv6.47.1 show no documented breaking changes.@polkadot/typesv16.5.4 usages (TypeRegistry, Metadata, ExtrinsicPayload, SignerPayloadJSON) are all standard stable interfaces with no breaking changes detected.All imports across the codebase follow standard API patterns and should work without issues.
packages/types/package.json (1)
26-42: Dev tooling updates are safe and consistent.Minor and patch version updates to development dependencies (TypeScript ESLint, Prettier, Node types) across all tooling packages. No runtime impact.
packages/signers/massa/package.json (1)
34-53: Dev tooling updates are consistent and safe.All changes are minor/patch version updates to development dependencies, with no runtime impact.
packages/utils/package.json (2)
32-49: Dev tooling updates are safe.Minor/patch version updates to development dependencies with no runtime impact.
27-27: Verify that all cryptography-related tests pass with @polkadot/util-crypto 14.x.This is a major version bump of a critical runtime dependency. The upgrade affects functions actively used in crypto operations (encodeAddress, blake2AsU8a, base64Decode/Encode, u8aWrapBytes). Ensure all signing and hashing tests validate correct output with the new version, especially substrate message signing which uses u8aWrapBytes—any behavioral change in this utility would alter signature values.
packages/signers/kadena/package.json (2)
31-31: Major version upgrade for @polkadot/util-crypto also appears here.This mirrors the upgrade in packages/utils/package.json. Ensure the breaking changes are documented and tested across dependent packages.
29-48: Other dev tooling updates are consistent and safe.Minor/patch version updates to development dependencies with no runtime impact.
packages/keyring/package.json (1)
37-53: Dev tooling updates are safe.Minor/patch version updates to development dependencies with no runtime impact.
packages/signers/polkadot/package.json (3)
25-25: @commitlint/cli is listed as a runtime dependency.This is typically a development-time linting tool and should usually be in
devDependenciesrather thandependencies. Verify this is intentional, or consider moving it todevDependencies.
27-29: Verify @PolkaDot packages major version compatibility.Multiple @PolkaDot packages updated to 14.x versions (util, util-crypto, wasm-crypto). Ensure they're released together and compatible. Check that any code changes required for these major version upgrades have been applied.
32-49: Other dev tooling updates are safe.Minor/patch version updates to development dependencies with no runtime impact.
packages/signers/ethereum/package.json (1)
32-50: Dev tooling updates are consistent and safe.All changes are minor/patch version updates to development dependencies with no runtime impact.
feat: enhance restricted address handling in App.vue and index.vue
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/extension/src/providers/ethereum/ui/eth-connect-dapp.vue (1)
36-52: Consider adding a loading state during the restriction check.There's a race condition where the UI renders with
isRestricted = falsebefore the asyncisWalletRestrictedcheck completes. During this window, restricted users might see the normal "Connect" UI briefly before it switches to the restricted message.Consider adding a loading state:
+const isCheckingRestriction = ref(true); const isRestricted = ref(false); // In onBeforeMount: +isCheckingRestriction.value = true; isWalletRestricted(displayAddress.value) .then(res => { isRestricted.value = res; }) + .finally(() => { + isCheckingRestriction.value = false; + }); // In template: -<div v-if="!isRestricted" class="provider-connect-dapp__info"> +<div v-if="!isCheckingRestriction && !isRestricted" class="provider-connect-dapp__info">packages/extension/src/ui/action/views/restricted/index.vue (1)
10-14: Consider truncating or formatting the restricted address for readability.The full address is displayed inline in the message. For long addresses (like Ethereum addresses), this might make the text hard to read.
Consider truncating the address:
- Your wallet address [{{ restrictedAddress }}] is associated with suspect + Your wallet address [{{ restrictedAddress.slice(0, 6) }}...{{ restrictedAddress.slice(-4) }}] is associated with suspectOr use a monospace font for better readability (add to the
<style>section):.blocked-page p { code { font-family: monospace; background-color: rgba(0, 0, 0, 0.05); padding: 2px 4px; border-radius: 3px; } }Then in the template:
<code>{{ restrictedAddress }}</code>packages/extension/src/ui/action/App.vue (1)
463-491: Consider refactoring the deeply nested logic inonSelectedAddressChanged.The function now has significant nesting with the restriction check gating the rest of the logic. Consider extracting the post-restriction logic into a separate function for better readability.
Example refactor:
+const handleUnrestrictedAddressChange = async (newAccount: EnkryptAccount) => { + const accountStates = { + [ProviderName.ethereum]: EVMAccountState, + [ProviderName.bitcoin]: BTCAccountState, + [ProviderName.solana]: SolAccountState, + }; + if (Object.keys(accountStates).includes(currentNetwork.value.provider)) { + const AccountState = new accountStates[ + currentNetwork.value.provider as keyof typeof accountStates + ](); + const domain = await domainState.getCurrentDomain(); + AccountState.addApprovedAddress(newAccount.address, domain); + } + await domainState.setSelectedAddress(newAccount.address); + await sendToBackgroundFromAction({ + message: JSON.stringify({ + method: InternalMethods.sendToTab, + params: [ + { + method: MessageMethod.changeAddress, + params: [currentNetwork.value.displayAddress(newAccount.address)], + }, + ], + }), + provider: currentNetwork.value.provider, + tabId: await domainState.getCurrentTabId(), + }); +}; + const onSelectedAddressChanged = async (newAccount: EnkryptAccount) => { accountHeaderData.value.selectedAccount = newAccount; await checkAddress(newAccount); if (!isAddressRestricted.value.isRestricted) { - const accountStates = { - [ProviderName.ethereum]: EVMAccountState, - [ProviderName.bitcoin]: BTCAccountState, - [ProviderName.solana]: SolAccountState, - }; - if (Object.keys(accountStates).includes(currentNetwork.value.provider)) { - const AccountState = new accountStates[ - currentNetwork.value.provider as keyof typeof accountStates - ](); - const domain = await domainState.getCurrentDomain(); - AccountState.addApprovedAddress(newAccount.address, domain); - } - await domainState.setSelectedAddress(newAccount.address); - await sendToBackgroundFromAction({ - message: JSON.stringify({ - method: InternalMethods.sendToTab, - params: [ - { - method: MessageMethod.changeAddress, - params: [currentNetwork.value.displayAddress(newAccount.address)], - }, - ], - }), - provider: currentNetwork.value.provider, - tabId: await domainState.getCurrentTabId(), - }); + await handleUnrestrictedAddressChange(newAccount); } };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/extension/env.d.ts(1 hunks)packages/extension/src/libs/json-tree-view/JsonTreeViewItem.vue(1 hunks)packages/extension/src/providers/ethereum/ui/eth-connect-dapp.vue(5 hunks)packages/extension/src/providers/ethereum/ui/send-transaction/index.vue(5 hunks)packages/extension/src/ui/action/App.vue(6 hunks)packages/extension/src/ui/action/views/restricted/index.vue(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
packages/extension/env.d.ts (1)
17-20: LGTM! Good TypeScript hygiene.Adding the Window interface declaration improves type safety for the
__PACKAGE_VERSION__property while maintaining consistency with the existing top-level declaration.packages/extension/src/ui/action/views/restricted/index.vue (1)
100-117: LGTM! Props and events properly defined.The new props and event emitter are well-typed and follow Vue 3 composition API best practices.
packages/extension/src/libs/json-tree-view/JsonTreeViewItem.vue (1)
103-103: Verify the switch-ts library version supports.otherwise().The code currently uses
.otherwise()with switch-ts@^2.0.0 (version 2.0.0 installed). No other usages of.default()exist in the codebase. However, official API documentation for switch-ts 2.0.0 could not be accessed to confirm that this version supports.otherwise()as the replacement method for.default().
4 participants
Summary by CodeRabbit
New Features
Chores
Revert
✏️ Tip: You can customize this high-level summary in your review settings.