Conversation
feat: export private key
WalkthroughRepository-wide updates include Node.js/runtime version bumps, dependency upgrades, and multiple network registry changes (additions/removals/renames). A new private-key export flow is added end-to-end (UI → background → keyring). Name-resolution disables SID paths. Tests/settings updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Accounts View / Export Modal
participant BG as Background Internal Handler
participant KR as KeyRingBase / KeyRing
participant Store as Key Storage / Signer
rect rgb(240,248,255)
note over User,UI: Export private key flow (new)
User->>UI: Click "Export" on account
UI->>UI: Prompt for extension password
UI->>BG: InternalMethods.getPrivateKey([account, password])
BG->>KR: getPrivateKey(options, password)
KR->>Store: Retrieve/derive private key
Store-->>KR: privateKey (string)
KR-->>BG: privateKey
BG-->>UI: { result: JSON.stringify(privateKey) }
UI->>UI: Show private key in modal
end
rect rgb(255,240,240)
alt Error
Store-->>KR: throw Error
KR-->>BG: Error
BG-->>UI: { error }
UI->>User: Display error message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/signers/polkadot/src/index.ts (2)
115-146: Async verify is not awaited — assertions always pass.
assert(this.verify(...))passes a Promise (truthy), so failures won’t be caught. This affects ECDSA, Ed25519, and Sr25519 branches.Apply:
- assert( - this.verify( - bufferToHex(msgHashBuffer), - bufferToHex(sig), - bufferToHex(pair.publicKey), - ), - Errors.SigningErrors.UnableToVerify, - ); + { + const ok = await this.verify( + bufferToHex(msgHashBuffer), + bufferToHex(sig), + bufferToHex(pair.publicKey), + ); + assert(ok, Errors.SigningErrors.UnableToVerify); + } ... - assert( - this.verify( - bufferToHex(msgHashBuffer), - bufferToHex(sig), - bufferToHex(pair.publicKey), - ), - Errors.SigningErrors.UnableToVerify, - ); + { + const ok = await this.verify( + bufferToHex(msgHashBuffer), + bufferToHex(sig), + bufferToHex(pair.publicKey), + ); + assert(ok, Errors.SigningErrors.UnableToVerify); + } ... - assert( - this.verify( - bufferToHex(msgHashBuffer), - bufferToHex(sig), - bufferToHex(pair.publicKey), - ), - Errors.SigningErrors.UnableToVerify, - ); + { + const ok = await this.verify( + bufferToHex(msgHashBuffer), + bufferToHex(sig), + bufferToHex(pair.publicKey), + ); + assert(ok, Errors.SigningErrors.UnableToVerify); + }Add regression tests that flip a bit in the signature and expect an error.
49-55: Remove unsupportedroundsargument from mnemonicToMiniSecret call
mnemonicToMiniSecret only accepts (mnemonic, password?, wordlist?, onlyJs?), so the fifth parameter (2048) in packages/signers/polkadot/src/index.ts lines 49–55 is invalid and should be removed.
🧹 Nitpick comments (28)
packages/name-resolution/tests/resolver.test.ts (1)
23-24: SID path disabled → assert null; clean up the commented expectationGood change. Drop the commented line and use a null-specific assertion for clarity.
- // expect(address).to.be.eq("0xb5932a6B7d50A966AEC6C74C97385412Fb497540"); - expect(address).to.be.eq(null); + expect(address).to.be.null;packages/name-resolution/tests/sid.test.ts (1)
4-4: Make the skip explicit in the suite title (tracks intent in CI output)This keeps intent visible without hunting PR context.
-describe.skip("SID Name resolving", () => { +describe.skip("[temporarily disabled] SID Name resolving", () => {packages/name-resolution/src/index.ts (3)
27-28: Feature-flag SID initialization instead of commenting it outKeeps the path dormant by default but easy to re-enable without code churn.
- // this.sid.init(), + // Feature-flag SID init; disabled by default. + (process.env.ENABLE_SID === "true" ? this.sid.init() : Promise.resolve()),
35-35: Gate SID reverse resolution behind a flag (no-op by default)Avoids long-term commented code while preserving easy toggling.
- // this.sid.resolveReverseName(address), + // Feature-flag SID reverse resolution. + ...(process.env.ENABLE_SID === "true" ? [this.sid.resolveReverseName(address)] : []),
58-60: Keep the disabled SID path self-consistent and flag-gatedUpdates the stale variable name in comments and shows how to re-enable safely.
- // if (this.sid.isSupportedName(name)) - // return this.sid.resolveAddress(name, paymentIdChain); + // Feature-flag SID address resolution. + // if (this.sid.isSupportedName(name) && process.env.ENABLE_SID === "true") + // return this.sid.resolveAddress(name, _paymentIdChain);packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
145-149: Drop empty tbName for consistencyOther
bsEndpointentries omittbName. Suggest removing it instead of setting to an empty string.Apply:
[NetworkNames.Sanko]: { - tbName: '', cgPlatform: CoingeckoPlatform.Sanko, bsEndpoint: true, },package.json (1)
37-38: Unify dev tool versions across workspace
- @types/node is pinned to ^22.18.1 in most packages but ^24.3.1 in hw-wallets and all signers—align all to ^24.3.1
- typescript is specified as ~5.9.2 in packages/extension and ^5.9.2 elsewhere—choose one semver spec (recommend ^5.9.2)
Run a workspace-wide dedupe (e.g.pnpm dedupeoryarn dedupe) to collapse duplicates.Dockerfile (1)
1-1: Pin base image digest and enable Corepack for Yarn 4 determinism.
Current tag-only pin risks silent base image drift; also ensure Yarn 4 is activated explicitly.Apply:
-FROM node:22.18-bookworm -RUN apt-get update -RUN apt-get install build-essential zip -y +FROM node:22.18-bookworm@sha256:<digest-here> +RUN apt-get update && \ + apt-get install -y --no-install-recommends build-essential zip && \ + rm -rf /var/lib/apt/lists/* ENV HOME /home ENV NODE_OPTIONS --max-old-space-size=8192 -RUN node -v && npm -v && yarn -v +RUN corepack enable && corepack prepare yarn@4.5.1 --activate && node -v && npm -v && yarn -v WORKDIR /home(Replace with the current digest.)
.github/workflows/test-all.yml (1)
12-12: Avoid Node version drift by sourcing from .nvmrc.
Keeps CI and local dev in lockstep as versions change.- node-version: "22.18.0" + node-version-file: ".nvmrc" cache: "yarn".github/workflows/test-swap.yml (1)
12-12: Use .nvmrc in CI for consistency.
Same rationale as the main workflow.- node-version: "22.18.0" + node-version-file: ".nvmrc" cache: "yarn"packages/utils/package.json (1)
27-27: Confirm @PolkaDot dependency alignment
All @polkadot/util and @polkadot/util-crypto versions are ^13.5.6 and @polkadot/wasm-crypto is ^7.5.1 across every package.json—no version skew detected. Optional: bumpengines.nodeto ≥18 (or ≥22) in the workspace root to match CI/runtime.packages/extension-bridge/package.json (1)
47-53: Align @types/node versions across all packages
Detected both^22.18.1and^24.3.1in workspace packages—standardize on a single version to avoid type-surface drift.packages/swap/package.json (1)
27-29: Solana deps consistent
Bothpackages/extension/package.jsonandpackages/swap/package.jsonuse@solana/web3.js@^1.98.4and@solana/spl-token@^0.4.14, with no version mismatches detected. Optional: hoist these entries to the rootpackage.json(or a shared config) for easier maintenance.packages/signers/ethereum/package.json (1)
35-38: @types/node version mismatch: use ^22.18.1 or verify no Node-24-specific types
packages/signers/ethereum/package.json uses ^24.3.1, while .nvmrc pins v22.18.0 and most packages use ^22.18.1. Either downgrade or confirm compatibility.packages/name-resolution/package.json (1)
25-28: Deps upgrade OK; fix repository URL placeholder (nit)
Library bumps look good; no code-level imports of “viem” detected, so the bump is safe. Update therepositoryfield to match other packages:- "repository": { - "type": "git", - "url": "git+https://github.com/<FILL_IT>" - }, + "repository": { + "type": "git", + "url": "git+https://github.com/enkryptcom/enKrypt/tree/main/packages/name-resolution" + },packages/keyring/src/index.ts (4)
318-340: Add session guard and timer reset for parity with other sensitive ops.Other secrets-touching methods assert unlocked and/or reset the auto-lock timer. For consistency and UX (prevent mid-flow re-lock), at least reset the timer when already unlocked.
Apply:
async getPrivateKey( options: SignOptions, keyringPassword: string, ): Promise<string> { + // Keep UX consistent with sign/encrypt flows + if (!this.#isLocked && this.autoLockTime !== 0) { + this.#resetTimeout(); + }
318-340: Ensure the requested account actually exists before exporting its key.Currently, mnemonic branch derives any path; and privkey branch trusts presence in ENCRYPTED_PRIVKEYS. Add a check against stored accounts to avoid exporting keys for non-existent records or mismatched signer/basePath combinations.
): Promise<string> { + const exists = (await this.getKeysArray()).some( + (a) => + a.basePath === options.basePath && + a.pathIndex === options.pathIndex && + a.signerType === options.signerType && + a.walletType === options.walletType && + !a.isHardware + ); + assert(exists, Errors.KeyringErrors.AddressDoesntExists);
328-332: Minor: reuse in-memory privkeys when unlocked to avoid extra decrypt.If already unlocked, prefer this.#privkeys to reduce storage I/O and crypto ops. Fallback to storage when locked.
- if (options.walletType === WalletType.privkey) { - const privkeys = await this.#getPrivateKeys(keyringPassword); - assert(privkeys[options.pathIndex.toString()], Errors.KeyringErrors.AddressDoesntExists); - return privkeys[options.pathIndex.toString()]; + if (options.walletType === WalletType.privkey) { + const map = !this.#isLocked && this.#privkeys + ? this.#privkeys + : await this.#getPrivateKeys(keyringPassword); + const key = map[options.pathIndex.toString()]; + assert(key, Errors.KeyringErrors.AddressDoesntExists); + return key;
333-339: Style: terminate statement; keep codebase conventions consistent.Add a semicolon after the await call.
- const mnemonic = await this.#getMnemonic(keyringPassword) + const mnemonic = await this.#getMnemonic(keyringPassword);packages/types/src/networks.ts (1)
89-90: Guard rename of CotiDevnet → CotiTestnet
Renaming this enum breaks persisted settings—add a migration to remap stored"CotiDevnet"→"CotiTestnet"on load.packages/extension/src/providers/ethereum/networks/zksepolia.ts (1)
6-23: Rename vars from zkgoerli → zksepolia for clarityNames still say “goerli”; rename to avoid confusion.
-const zkgoerliOptions: EvmNetworkOptions = { +const zksepoliaOptions: EvmNetworkOptions = { name: NetworkNames.zkSyncSepolia, name_long: 'zkSync Sepolia', @@ -const zkgoerli = new EvmNetwork(zkgoerliOptions); -export default zkgoerli; +const zksepolia = new EvmNetwork(zksepoliaOptions); +export default zksepolia;packages/extension/src/libs/keyring/keyring.ts (1)
100-102: Wrapper method: LGTM; consider documenting return formatIf callers expect 0x-prefixed hex, document/normalize here to avoid UI edge cases.
packages/extension/src/ui/action/views/accounts/components/accounts-list-item.vue (1)
82-83: Accessibility nit: add aria-label to the More buttonSmall a11y win for keyboard/screen readers.
- <div + <div v-if="showEdit" ref="toggle" class="accounts-item__more" + aria-label="Account actions menu" @click.stop="toggleEdit" >packages/extension/src/ui/action/views/accounts/components/accounts-list-item-menu.vue (1)
13-19: Make Export action accessible (keyboard/semantics).Use button semantics so keyboard users can trigger Export. Minimal change: add role and key handlers.
- <a + <a v-if="exportable" class="accounts-item__edit-item" - @click.stop="$emit('action:export')" + role="button" + tabindex="0" + @click.stop="$emit('action:export')" + @keyup.enter.stop="$emit('action:export')" + @keyup.space.stop="$emit('action:export')" >packages/extension/src/providers/ethereum/networks/index.ts (1)
58-59: Align naming: usecotiTestnet(noNodesuffix) for consistency.Most keys are network names without the
Nodesuffix. Consider renaming for consistency.-import cotiTestnetNode from './coti-testnet'; +import cotiTestnet from './coti-testnet'; @@ - cotiTestnetNode: cotiTestnetNode, + cotiTestnet: cotiTestnet,Also applies to: 144-145
packages/extension/src/ui/action/views/accounts/index.vue (1)
14-31: Prefer stable keys.Using
indexas:keycan cause rendering issues on mutations. If feasible, key byaccount.address.- :key="index" + :key="account.address"packages/extension/src/ui/action/views/accounts/components/export-account-form.vue (2)
18-27: Show actionable error text and reset errors before retry.Display a generic but accurate message and clear previous errors when the user retries.
- <p v-show="keyringError" class="export-account-form__error"> - Invalid Keyring password - </p> + <p v-show="keyringError" class="export-account-form__error"> + {{ errorText || 'Invalid password or export not allowed for this account.' }} + </p>
30-36: Optional: add copy-to-clipboard with auto-hide.For usability and safety, consider a “Copy” button and auto-hide the key after a short interval (e.g., 30s). I can provide a small patch if desired.
📜 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 (53)
.github/workflows/test-all.yml(1 hunks).github/workflows/test-swap.yml(1 hunks).nvmrc(1 hunks)Dockerfile(1 hunks)package.json(1 hunks)packages/extension-bridge/package.json(2 hunks)packages/extension/configs/vitest.config.mts(1 hunks)packages/extension/package.json(6 hunks)packages/extension/src/libs/background/index.ts(2 hunks)packages/extension/src/libs/background/internal/get-private-key.ts(1 hunks)packages/extension/src/libs/background/internal/index.ts(2 hunks)packages/extension/src/libs/keyring/keyring.ts(1 hunks)packages/extension/src/providers/common/libs/new-features.ts(1 hunks)packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts(1 hunks)packages/extension/src/providers/ethereum/networks/cagaAnkara.ts(0 hunks)packages/extension/src/providers/ethereum/networks/coti-devnet.ts(0 hunks)packages/extension/src/providers/ethereum/networks/coti-testnet.ts(1 hunks)packages/extension/src/providers/ethereum/networks/form-testnet.ts(0 hunks)packages/extension/src/providers/ethereum/networks/form.ts(0 hunks)packages/extension/src/providers/ethereum/networks/index.ts(6 hunks)packages/extension/src/providers/ethereum/networks/skale/chaos.ts(0 hunks)packages/extension/src/providers/ethereum/networks/skale/index.ts(0 hunks)packages/extension/src/providers/ethereum/networks/tac.ts(1 hunks)packages/extension/src/providers/ethereum/networks/zkgoerli.ts(0 hunks)packages/extension/src/providers/ethereum/networks/zksepolia.ts(1 hunks)packages/extension/src/providers/polkadot/networks/index.ts(0 hunks)packages/extension/src/providers/polkadot/networks/unique/opal.ts(0 hunks)packages/extension/src/types/messenger.ts(1 hunks)packages/extension/src/ui/action/views/accounts/components/accounts-list-item-menu.vue(2 hunks)packages/extension/src/ui/action/views/accounts/components/accounts-list-item.vue(2 hunks)packages/extension/src/ui/action/views/accounts/components/export-account-form.vue(1 hunks)packages/extension/src/ui/action/views/accounts/index.vue(6 hunks)packages/hw-wallets/package.json(3 hunks)packages/keyring/package.json(2 hunks)packages/keyring/src/index.ts(1 hunks)packages/name-resolution/package.json(3 hunks)packages/name-resolution/src/index.ts(2 hunks)packages/name-resolution/tests/resolver.test.ts(1 hunks)packages/name-resolution/tests/sid.test.ts(1 hunks)packages/request/package.json(2 hunks)packages/signers/bitcoin/package.json(2 hunks)packages/signers/ethereum/package.json(2 hunks)packages/signers/kadena/package.json(2 hunks)packages/signers/polkadot/package.json(2 hunks)packages/signers/polkadot/src/index.ts(1 hunks)packages/storage/package.json(2 hunks)packages/swap/package.json(2 hunks)packages/swap/src/common/estimateGasList.ts(0 hunks)packages/swap/src/index.ts(1 hunks)packages/types/package.json(2 hunks)packages/types/src/networks.ts(3 hunks)packages/utils/package.json(2 hunks)
💤 Files with no reviewable changes (10)
- packages/extension/src/providers/ethereum/networks/cagaAnkara.ts
- packages/extension/src/providers/polkadot/networks/index.ts
- packages/extension/src/providers/ethereum/networks/form.ts
- packages/extension/src/providers/ethereum/networks/skale/chaos.ts
- packages/swap/src/common/estimateGasList.ts
- packages/extension/src/providers/ethereum/networks/form-testnet.ts
- packages/extension/src/providers/ethereum/networks/coti-devnet.ts
- packages/extension/src/providers/ethereum/networks/zkgoerli.ts
- packages/extension/src/providers/ethereum/networks/skale/index.ts
- packages/extension/src/providers/polkadot/networks/unique/opal.ts
🧰 Additional context used
🧬 Code graph analysis (6)
packages/extension/src/libs/background/internal/get-private-key.ts (5)
packages/extension/src/libs/background/internal/index.ts (1)
getPrivateKey(19-19)packages/extension/src/libs/keyring/keyring.ts (2)
getPrivateKey(100-102)KeyRingBase(14-103)packages/keyring/src/index.ts (2)
getPrivateKey(318-340)password(111-131)packages/extension/src/types/messenger.ts (1)
InternalOnMessageResponse(59-62)packages/extension/src/libs/error/index.ts (1)
getCustomError(27-33)
packages/extension/src/providers/ethereum/networks/tac.ts (2)
packages/extension/src/providers/ethereum/types/evm-network.ts (2)
EvmNetworkOptions(22-53)EvmNetwork(55-286)packages/extension/src/providers/ethereum/libs/activity-handlers/index.ts (1)
EtherscanActivity(10-10)
packages/extension/src/providers/ethereum/networks/zksepolia.ts (1)
packages/extension/src/providers/ethereum/types/evm-network.ts (2)
EvmNetworkOptions(22-53)EvmNetwork(55-286)
packages/extension/src/providers/ethereum/networks/coti-testnet.ts (1)
packages/extension/src/providers/ethereum/types/evm-network.ts (2)
EvmNetworkOptions(22-53)EvmNetwork(55-286)
packages/keyring/src/index.ts (1)
packages/keyring/src/utils.ts (1)
pathParser(3-18)
packages/extension/src/libs/background/index.ts (3)
packages/extension/src/libs/background/internal/index.ts (1)
getPrivateKey(19-19)packages/extension/src/libs/keyring/keyring.ts (1)
getPrivateKey(100-102)packages/keyring/src/index.ts (1)
getPrivateKey(318-340)
⏰ 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 (30)
packages/name-resolution/src/index.ts (2)
51-51: Param rename to underscore-prefixed is finePrevents unused param warnings without breaking the public signature.
23-28: Verify no callers rely on SID or the thirdresolveAddressparameter
Automated grep failed – please manually confirm no remaining code depends on SID resolution or a 3-arg call toresolveAddressby checking:
- calls to
resolver.resolveAddresspassing three parameters- any direct references to
SIDResolver- resolution of
.arbor.bnbdomainspackages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (2)
145-149: Sanko routed to Blockscout — LGTMSwitching Sanko to
bsEndpoint: truelooks correct and consistent with other Blockscout-backed networks.
145-149: Blockscout support and native-token injection verified for Sanko
TheNetworkEndpointsconfig includes[NetworkNames.Sanko]: 'https://explorer.sanko.xyz/', andgetBlockscoutBalancesalways prepends a{ token: NATIVE_TOKEN_ADDRESS, … }entry before merging ERC-20 balances, so the native coin is correctly handled onbsEndpoint..nvmrc (1)
1-1: Node version pin aligned — looks good.
Matches Docker and CI updates to 22.18.0.packages/request/package.json (1)
21-23: ```shell
#!/bin/bashPrint root package.json engines section if present
echo "==== root package.json ===="
if grep -q '"engines"' package.json; then
sed -n '1,200p' package.json | rg -nP '"engines"\s*:\s*{[^}]*' -C2
else
echo "No engines field in root package.json"
fiLocate and print packages/request/package.json snippet around engines
REQUEST_PKG=$(find . -type f -path '*packages/request/package.json' | head -n1)
echo
echo "==== packages/request/package.json ===="
if [ -n "$REQUEST_PKG" ]; then
sed -n '1,200p' "$REQUEST_PKG"
else
echo "packages/request/package.json not found"
fiList all engines.node entries across all package.json files
echo
echo "==== All engines.node entries ===="
find . -type f -name package.json -exec grep -Hn '"engines"' -C1 {} ; | grep -B1 '"node"\s*:' || echo "No other engines.node entries found"</blockquote></details> <details> <summary>packages/swap/src/index.ts (1)</summary><blockquote> `270-271`: **No verbatimModuleSyntax usage; explicit type-only re-export is optional** Search across tsconfig*.json shows no instances of `verbatimModuleSyntax: true`, so no change is needed now. Convert to `export type { ToTokenType }` only if that flag is enabled in the future. </blockquote></details> <details> <summary>packages/storage/package.json (1)</summary><blockquote> `30-33`: **Tooling bumps look good** No runtime impact; matches workspace upgrades. Also applies to: 43-44 </blockquote></details> <details> <summary>packages/signers/bitcoin/package.json (1)</summary><blockquote> `35-38`: **Resolved:** No Node built-in imports detected in packages/signers/bitcoin; keeping @types/node at ^24.3.1 is safe. </blockquote></details> <details> <summary>packages/signers/polkadot/package.json (2)</summary><blockquote> `34-37`: **Verify Node.js toolchain alignment across monorepo** No Node.js engine/version detected in CI configs, .nvmrc, or Dockerfiles; ensure a Node.js version compatible with @types/node@24.x is used to prevent type lib/runtime mismatches. --- `27-29`: **Polkadot dependencies aligned across all packages.** Verified @polkadot/util and @polkadot/util-crypto are pinned to ^13.5.6, and @polkadot/wasm-crypto to ^7.5.1 in every package.json; no mismatches found. </blockquote></details> <details> <summary>packages/signers/kadena/package.json (1)</summary><blockquote> `31-31`: **DevDependency bumps look good.** No functional/runtime impact expected for the signer; safe to merge. Also applies to: 33-37, 46-47 </blockquote></details> <details> <summary>packages/keyring/package.json (2)</summary><blockquote> `32-32`: **@polkadot/util patch bump LGTM.** Aligned with other packages; no breaking API changes expected. --- `37-41`: **Confirm Node engine and @types/node parity across packages** - All packages specify `"engines.node": ">=14.15.0"`, but devDependencies list mixed `@types/node` majors (^22.18.1 vs ^24.3.1). - Standardize on a single `@types/node` major or tighten `engines.node` (e.g. `>=24.x`) to match the highest types version; tsconfig lib targets (ESNext, dom) are already consistent. </blockquote></details> <details> <summary>packages/types/package.json (1)</summary><blockquote> `27-31`: **Toolchain refresh LGTM.** Pure tooling changes; minimal risk. Also applies to: 40-41 </blockquote></details> <details> <summary>packages/keyring/src/index.ts (1)</summary><blockquote> `318-340`: **No duplicate getPrivateKey implementations.** The only logic lives in packages/keyring/src/index.ts; the method in packages/extension/src/libs/keyring/keyring.ts is merely a proxy to this core implementation. </blockquote></details> <details> <summary>packages/hw-wallets/package.json (1)</summary><blockquote> `55-63`: ```shell #!/bin/bash # Verify @ledgerhq dependencies across Yarn workspaces to detect duplicate transports set -euo pipefail # Install dependencies quietly yarn install --silent # Extract workspace names workspace_names=$(yarn workspaces list --json | jq -r .name) # Iterate each workspace and list @ledgerhq packages at depth 0 for ws in $workspace_names; do echo "=== Workspace: $ws ===" yarn workspace "$ws" list --pattern "@ledgerhq" --depth=0 || true donepackages/types/src/networks.ts (2)
110-110: Extension: Missing TAC provider/config and registration
No ‘TAC’ references found underpackages/extension/src—please add TAC’s RPC endpoint, chainId, currency, and logo in the extension’s provider/config files and register it in all NetworkNames usages (UI filters, network switcher, RPC registry).
31-31: No ZkSyncGoerli references detected; no downstream updates required.packages/extension/src/providers/common/libs/new-features.ts (1)
3-3: TAC in newNetworks: LGTMLooks consistent with the TAC additions elsewhere.
packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/configs.ts (1)
71-72: Verify TAC explorer API compatibilityConfirm that https://explorer.tac.build/ supports the Etherscan-compatible API endpoints your handler uses (it constructs URLs by appending
api?module=account&action=txlist&address=…to the base URL in packages/extension/src/providers/ethereum/libs/activity-handlers/providers/etherscan/index.ts (lines 23–26)). Verify the exact path, any required authentication and rate limits.packages/extension/src/providers/ethereum/networks/zksepolia.ts (1)
16-17: No action: WS RPC already supported
Multiple existing network configs (e.g. glmr.ts, sepolia.ts, zksync.ts) usewss://endpoints, confirming wrapper support.packages/extension/package.json (2)
3-3: Extension manifest(s) not detected: confirm their location and update version to 2.11.0 for Chrome/Firefox submission.
92-142: Smoke-test Vite 7 & ECharts integration
- vue-echarts@7.x still targets ECharts 5 (ECharts 6 support arrives in vue-echarts v8) – either stay on ECharts 5 or upgrade to vue-echarts v8.
- @crxjs/vite-plugin@2.2.0 supports Vite 3+, including Vite 7, but Vite 7 raises Node requirements (>= 20.19 or 22.12+). Verify your Node version and test dev/build flows (HMR, svg icons, build.rollupOptions.input).
- Run full end-to-end builds/tests in both Chrome and Firefox before merging.
packages/extension/src/libs/background/index.ts (1)
171-173: Access control and telemetry for getPrivateKey verified
getPrivateKey is only handled in internalHandler (not reachable via externalHandler) and no persistence or telemetry code touches its returned value.packages/extension/src/libs/background/internal/index.ts (1)
9-20: Export wiring: LGTMInternal export is consistent with the new handler.
packages/extension/src/ui/action/views/accounts/components/accounts-list-item.vue (1)
34-35: Export event forwarding is correctly wired
accounts/index.vue binds@action:export="exportAccount(index)",accounts-list-item.vueforwards$attrs, andaccounts-list-item-menu.vueemitsaction:export.packages/extension/src/providers/ethereum/networks/tac.ts (1)
7-21: Network config validated—ensure RPC CORS/keyless support.
explorer.tac.buildis already mapped in the EtherscanActivity provider and is Etherscan-compatible.coingeckoID: 'tac'matches CoinGecko’s TAC asset slug.- Verify that
https://rpc.ankr.com/tacrequires no API key and returns the necessary CORS headers for the extension.packages/extension/src/providers/ethereum/networks/coti-testnet.ts (1)
16-19: No changes required — EvmNetwork already supports WSS RPC URLs (numerous networks in packages/extension/src/providers/ethereum/networks usewss://).packages/extension/src/ui/action/views/accounts/index.vue (1)
26-30: Make Export button conditional on walletType
Replace the always-true binding with a check againstWalletType, for example:- :exportable="true" + :exportable=" + account.walletType === WalletType.mnemonic || + account.walletType === WalletType.privkey + "Ensure
WalletTypeis imported into this component’s script. Add a unit test verifying that hardware-type accounts do not render the Export action.
| include: ['crypto'], | ||
| }), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm crypto-only polyfill is sufficient.
Restricting to ['crypto'] may break tests using Buffer/stream/process. If failures appear, include missing shims explicitly.
Quick scan:
Add needed shims, e.g.:
- include: ['crypto'],
+ include: ['crypto', 'buffer'],🏁 Script executed:
#!/bin/bash
rg -nP '\b(Buffer|process|stream|Readable|Writable)\b' packages/extension/src packages/extension/tests || trueLength of output: 16275
Include ‘buffer’ in Vitest polyfills
Tests throughout the extension reference Buffer (e.g. in swap, send-transactions, import-account, providers, etc.), so replace:
– include: ['crypto'],
+ include: ['crypto', 'buffer'],Add ‘stream’ or ‘process’ to the include array only if you hit further undefined‐shim errors.
📝 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.
| include: ['crypto'], | |
| }), | |
| include: ['crypto', 'buffer'], | |
| }), |
🤖 Prompt for AI Agents
In packages/extension/configs/vitest.config.mts around lines 9 to 10, the Vitest
polyfills currently include only 'crypto' but tests reference Buffer; update the
include array to add 'buffer' (so Buffer is polyfilled) and run the tests; only
add 'stream' or 'process' to the include array if you encounter additional
undefined-shim errors for those globals.
| find: /^(vite-plugin-node-polyfills\/shims\/.+)/, | ||
| replacement: '$1', | ||
| customResolver(source) { | ||
| return import.meta.resolve(source).replace(/^file:\/\//, ''); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make customResolver path-safe across OSes and URLs.
import.meta.resolve returns a file:// URL; string replace is brittle (Windows, percent-encoding). Use fileURLToPath.
+import { fileURLToPath } from 'node:url';
...
customResolver(source) {
- return import.meta.resolve(source).replace(/^file:\/\//, '');
+ return fileURLToPath(import.meta.resolve(source));
},Confirm Node ≥20 runs Vitest in CI/local, since import.meta.resolve isn’t supported on older Node:
🏁 Script executed:
#!/bin/bash
node -p "process.version"Length of output: 7
Use fileURLToPath instead of string replace in customResolver
Replace brittle URL replacement with fileURLToPath so paths are OS-safe:
+import { fileURLToPath } from 'node:url';
…
customResolver(source) {
- return import.meta.resolve(source).replace(/^file:\/\//, '');
+ return fileURLToPath(import.meta.resolve(source));
},📝 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.
| find: /^(vite-plugin-node-polyfills\/shims\/.+)/, | |
| replacement: '$1', | |
| customResolver(source) { | |
| return import.meta.resolve(source).replace(/^file:\/\//, ''); | |
| }, | |
| }, | |
| // at the top of packages/extension/configs/vitest.config.mts | |
| import { fileURLToPath } from 'node:url'; | |
| … | |
| export default defineConfig({ | |
| plugins: [ | |
| // … | |
| { | |
| find: /^(vite-plugin-node-polyfills\/shims\/.+)/, | |
| replacement: '$1', | |
| customResolver(source) { | |
| return fileURLToPath(import.meta.resolve(source)); | |
| }, | |
| }, | |
| // … | |
| ], | |
| }); |
🤖 Prompt for AI Agents
In packages/extension/configs/vitest.config.mts around lines 23 to 28, the
customResolver currently strips the file URL via string replace which is OS- and
edge-case brittle; import fileURLToPath from 'url' at the top of the file and
replace the replace call with a fileURLToPath conversion (e.g. const resolved =
import.meta.resolve(source); return fileURLToPath(new URL(resolved));), ensuring
the resolver returns an OS-safe file path; add the import line: import {
fileURLToPath } from 'url'.
| const account = message.params[0] as EnkryptAccount | ||
| const password = message.params[1] as string | ||
| try { | ||
| const privKey = await keyring.getPrivateKey(account, password) | ||
| return { | ||
| result: JSON.stringify(privKey), | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use SignOptions, return plain string, and preserve ProviderError.
KeyRingBase.getPrivateKeyexpectsSignOptions, notEnkryptAccount. Cast/validate accordingly to avoid type drift.- Return the private key as a plain string; JSON-wrapping forces callers to JSON.parse.
- Preserve
code/messageif the thrown error is already aProviderError.
-import { EnkryptAccount, RPCRequestType } from '@enkryptcom/types'
+import { RPCRequestType, SignOptions } from '@enkryptcom/types'
@@
- const account = message.params[0] as EnkryptAccount
+ const options = message.params[0] as SignOptions
const password = message.params[1] as string
try {
- const privKey = await keyring.getPrivateKey(account, password)
+ const privKey = await keyring.getPrivateKey(options, password)
return {
- result: JSON.stringify(privKey),
+ result: privKey,
}
} catch (e: any) {
- return {
- error: getCustomError(e.message),
- }
+ // If it's already a ProviderError, forward it; else wrap it.
+ if (e && typeof e === 'object' && 'code' in e && 'message' in e) {
+ return { error: e }
+ }
+ return { error: getCustomError(e?.message ?? 'unknown error') }
}I can also add runtime shape checks to coerce EnkryptAccount into SignOptions if you prefer keeping the UI payload unchanged.
📝 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.
| const account = message.params[0] as EnkryptAccount | |
| const password = message.params[1] as string | |
| try { | |
| const privKey = await keyring.getPrivateKey(account, password) | |
| return { | |
| result: JSON.stringify(privKey), | |
| } | |
| import { RPCRequestType, SignOptions } from '@enkryptcom/types' | |
| // …other imports… | |
| const options = message.params[0] as SignOptions | |
| const password = message.params[1] as string | |
| try { | |
| const privKey = await keyring.getPrivateKey(options, password) | |
| return { | |
| result: privKey, | |
| } | |
| } catch (e: any) { | |
| // If it's already a ProviderError, forward it; else wrap it. | |
| if (e && typeof e === 'object' && 'code' in e && 'message' in e) { | |
| return { error: e } | |
| } | |
| return { error: getCustomError(e?.message ?? 'unknown error') } | |
| } |
🤖 Prompt for AI Agents
In packages/extension/src/libs/background/internal/get-private-key.ts around
lines 14 to 20, the handler currently passes an EnkryptAccount and JSON-wraps
the private key while losing ProviderError details; change it to
convert/validate the incoming payload into SignOptions (or coerce EnkryptAccount
-> SignOptions with runtime checks), call keyring.getPrivateKey(signOptions,
password) with the correct type, return the private key as a raw string in
result (not JSON.stringify), and when catching errors rethrow/preserve existing
ProviderError instances (preserve their code/message) instead of wrapping or
discarding them.
| getPrivateKey = 'enkrypt_getPrivateKey', | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Introducing getPrivateKey: confirm strict gating and scope.
Ensure this internal method is only callable from trusted endpoints, requires an unlocked keyring + fresh user confirmation, and never responds to content scripts.
Suggested checks (background handler):
- Validate sender.origin against allowlist.
- Require re-auth with password/PIN per invocation.
- Rate-limit and audit-log export attempts.
- Return ephemeral data (avoid storing in Redux/state), disable clipboard auto-copy, and scrub from memory/UI ASAP.
You can locate uses with:
Optional typing tweak:
- Consider typing result as a branded Hex type (e.g., HexString) instead of plain string in InternalOnMessageResponse.
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'InternalMethods\.(getPrivateKey)|["'\'']enkrypt_getPrivateKey["'\'']'Length of output: 2027
Enforce strict access controls for getPrivateKey
- In
packages/extension/src/libs/background/index.ts, don’t callgetPrivateKeyunconditionally—validatesender.originagainst your allowlist and require a fresh password/PIN unlock of the keyring per request. - Rate-limit and audit-log each export; return the key as ephemeral data (no Redux/UI storage), disable any auto-copy, and scrub it from memory/UI immediately.
- (Optional) Brand the return type as
HexStringinstead of a rawstringinInternalOnMessageResponse.
🤖 Prompt for AI Agents
In packages/extension/src/types/messenger.ts around lines 38-39, the
getPrivateKey message is defined but background handlers currently call it
unconditionally; update the background handler in
packages/extension/src/libs/background/index.ts to enforce strict controls:
validate sender.origin against a configured allowlist before proceeding, require
an explicit per-request keyring unlock (password/PIN prompt) and fail if not
provided, apply rate-limiting and write an audit log entry for each export
attempt, return the key as ephemeral data (preferably branded as HexString in
InternalOnMessageResponse) without persisting to Redux/UI or auto-copying, and
immediately scrub the key from memory/UI after use.
| const model = defineModel<boolean>(); | ||
| const closeWindow = () => { | ||
| model.value = false; | ||
| }; | ||
| const keyringError = ref(false); | ||
| const isDone = ref(false); | ||
| const isLoading = ref(false); | ||
| const keyringPassword = ref(__PREFILL_PASSWORD__!); | ||
| const privKey = ref(''); | ||
| const keyringBase = new KeyRingBase(); | ||
|
|
||
| const props = defineProps({ | ||
| account: { | ||
| type: Object as PropType<EnkryptAccount>, | ||
| default: () => ({}), | ||
| }, | ||
| }); | ||
|
|
||
| const showAction = async () => { | ||
| try { | ||
| isLoading.value = true; | ||
| const res = await sendToBackgroundFromAction({ | ||
| message: JSON.stringify({ | ||
| method: InternalMethods.getPrivateKey, | ||
| params: [props.account, keyringPassword.value], | ||
| }), | ||
| }); | ||
| if (res.error) { | ||
| throw res.error; | ||
| } else { | ||
| privKey.value = JSON.parse(res.result!); | ||
| } | ||
| isDone.value = true; | ||
| } catch (err) { | ||
| keyringError.value = true; | ||
| } | ||
| isLoading.value = false; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid JSON round-trip and improve error handling UX.
- Use the plain string result (after backend change).
- Reset error state before attempting.
- Provide a generic error message instead of always “Invalid Keyring password”.
const showAction = async () => {
- try {
- isLoading.value = true;
+ try {
+ isLoading.value = true;
+ keyringError.value = false;
+ errorText.value = '';
const res = await sendToBackgroundFromAction({
message: JSON.stringify({
method: InternalMethods.getPrivateKey,
params: [props.account, keyringPassword.value],
}),
});
if (res.error) {
throw res.error;
} else {
- privKey.value = JSON.parse(res.result!);
+ privKey.value = res.result!;
}
isDone.value = true;
} catch (err) {
- keyringError.value = true;
+ keyringError.value = true;
+ errorText.value = 'Invalid password or export not allowed for this account.';
}
isLoading.value = false;
};Add this state in <script setup>:
-const keyringError = ref(false);
+const keyringError = ref(false);
+const errorText = ref('');📝 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.
| const model = defineModel<boolean>(); | |
| const closeWindow = () => { | |
| model.value = false; | |
| }; | |
| const keyringError = ref(false); | |
| const isDone = ref(false); | |
| const isLoading = ref(false); | |
| const keyringPassword = ref(__PREFILL_PASSWORD__!); | |
| const privKey = ref(''); | |
| const keyringBase = new KeyRingBase(); | |
| const props = defineProps({ | |
| account: { | |
| type: Object as PropType<EnkryptAccount>, | |
| default: () => ({}), | |
| }, | |
| }); | |
| const showAction = async () => { | |
| try { | |
| isLoading.value = true; | |
| const res = await sendToBackgroundFromAction({ | |
| message: JSON.stringify({ | |
| method: InternalMethods.getPrivateKey, | |
| params: [props.account, keyringPassword.value], | |
| }), | |
| }); | |
| if (res.error) { | |
| throw res.error; | |
| } else { | |
| privKey.value = JSON.parse(res.result!); | |
| } | |
| isDone.value = true; | |
| } catch (err) { | |
| keyringError.value = true; | |
| } | |
| isLoading.value = false; | |
| }; | |
| // In <script setup>, add new errorText state next to keyringError | |
| const keyringError = ref(false); | |
| const errorText = ref(''); | |
| const isDone = ref(false); | |
| const isLoading = ref(false); | |
| const keyringPassword = ref(__PREFILL_PASSWORD__!); | |
| const privKey = ref(''); | |
| const keyringBase = new KeyRingBase(); | |
| const showAction = async () => { | |
| try { | |
| isLoading.value = true; | |
| keyringError.value = false; | |
| errorText.value = ''; | |
| const res = await sendToBackgroundFromAction({ | |
| message: JSON.stringify({ | |
| method: InternalMethods.getPrivateKey, | |
| params: [props.account, keyringPassword.value], | |
| }), | |
| }); | |
| if (res.error) { | |
| throw res.error; | |
| } else { | |
| // backend now returns a plain string | |
| privKey.value = res.result!; | |
| } | |
| isDone.value = true; | |
| } catch (err) { | |
| keyringError.value = true; | |
| errorText.value = 'Invalid password or export not allowed for this account.'; | |
| } finally { | |
| isLoading.value = false; | |
| } | |
| }; |
packages/hw-wallets/package.json
Outdated
| "@types/node": "^24.3.1", | ||
| "@typescript-eslint/eslint-plugin": "^8.43.0", | ||
| "@typescript-eslint/parser": "^8.43.0", | ||
| "eslint": "^9.35.0", | ||
| "eslint-config-airbnb-base": "^15.0.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bump Node engine to match upgraded toolchain (ESLint 9, Vitest 3, TS 5.9).
These deps require Node ≥18 (ESLint 9 specifically ≥18.18). Current "engines.node": ">=14.15.0" is misleading and can break installs/CI.
Apply:
"engines": {
- "node": ">=14.15.0"
+ "node": ">=18.18.0"
},Run to confirm consistency repo-wide:
🏁 Script executed:
#!/bin/bash
fd package.json -t f | xargs -I{} jq -r '.name + " => " + (.engines.node // "N/A")' {}Length of output: 508
Bump Node engine to ≥18.18.0 in all packages
Update every engines.node entry currently set to >=14.15.0 to >=18.18.0 in each package.json (e.g. @enkryptcom/hw-wallets, keyring, name-resolution, request, signer-*, storage, swap, types, utils). Root packages (enkrypt, extension, extension-bridge) currently lack an engines field—consider adding one for consistency.
🤖 Prompt for AI Agents
In packages/hw-wallets/package.json around lines 25 to 29, the project needs its
Node engine requirement updated for compatibility; change or add an "engines"
field to require "node": ">=18.18.0" (and update the same "engines.node" value
across all package.json files mentioned: @enkryptcom/hw-wallets, keyring,
name-resolution, request, signer-*, storage, swap, types, utils), and for root
packages (enkrypt, extension, extension-bridge) add a consistent "engines": {
"node": ">=18.18.0" } entry if missing; ensure package.json formatting and
version constraints remain valid and run a quick npm/yarn install or lint to
verify no schema errors.
2 participants
Summary by CodeRabbit