-
Notifications
You must be signed in to change notification settings - Fork 451
feat: create qr wallet on btc only OK-47055 OK-47064 OK-47071 #9261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: x
Are you sure you want to change the base?
Conversation
WalkthroughThis PR attaches an app-level firmware type to device features, adds helpers to compute and embed it, and propagates that data through device storage/update flows, QR-wallet network selection, firmware-aware UI badges, and related service logic. Changes
sequenceDiagram
participant UI as App UI
participant Service as Service (QR / Hardware)
participant DeviceUtils as deviceUtils
participant DB as LocalDb
UI->>Service: Request prepare/create (QR or HW)
Service->>DeviceUtils: attachAppParamsToFeatures / getFirmwareType
DeviceUtils-->>Service: featuresInfo (includes $app_firmware_type)
alt QR network selection
Service->>Service: choose networks based on firmwareType
end
Service->>DB: updateDevice / createHwWallet with featuresInfo
DB-->>Service: ack stored device (features include $app_firmware_type)
Service-->>UI: return prepared data / networks / device info
UI->>UI: render BTC badge/avatar when firmwareType == BitcoinOnly
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/kit-bg/src/services/ServiceQrWallet/ServiceQrWallet.ts (1)
238-256: Use cached firmware type instead of async SDK call for QR walletsThe new
isBtcOnlyFirmwarebranch is useful for keeping BTC‑only QR wallets on BTC networks. However,deviceUtils.getFirmwareTypewill hit the hardware SDK and expects full device features, whilebyDevice?.featuresInfofor QR wallets is a small app-augmented object.Since you already persist
$app_firmware_typeand havegetFirmwareTypeByCachedFeatures, you can simplify and avoid unnecessary SDK usage:- const firmwareType = await deviceUtils.getFirmwareType({ - features: byDevice?.featuresInfo, - }); - const isBtcOnlyFirmware = firmwareType === EFirmwareType.BitcoinOnly; + const firmwareType = deviceUtils.getFirmwareTypeByCachedFeatures({ + features: byDevice?.featuresInfo, + }); + const isBtcOnlyFirmware = firmwareType === EFirmwareType.BitcoinOnly;This keeps behavior the same for Bitcoin‑only (where
$app_firmware_typeis set) and treats missing/Universal types as “not BTC‑only” without involving the SDK.packages/kit-bg/src/dbs/local/LocalDbBase.ts (1)
2359-2475: QR wallet firmware parsing is reasonable; consider documenting expected name format
createQrWalletnow derivesfirmwareTypefromqrDevice.name(split on-for passphrase hash, then:forserialNoandfirmwareStr) and stores it as$app_firmware_typeinsidefeaturesInfo. This is a neat, self-contained way to mark BTC-only QR devices.Two small robustness points:
- The code assumes a
name:serial:fwTypelayout and treats only a third segment equal to'btc'(case-insensitive) as Bitcoin-only. If the QR firmware ever tweaks this format, behavior will silently change.- For existing devices with non-empty
featuresbut no$app_firmware_type, theexistingDevicebranch only setsitem.featureswhen it was previously empty, so older QR devices won’t gain the new flag automatically.If this format is stable, current code is fine. Otherwise, consider:
- Adding a short comment about the expected
qrDevice.nameformat, and/or- A small migration path that backfills
$app_firmware_typefor existing QR devices when you next see afirmwareStr.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/kit-bg/src/dbs/local/LocalDbBase.ts(3 hunks)packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts(3 hunks)packages/kit-bg/src/services/ServiceQrWallet/ServiceQrWallet.ts(3 hunks)packages/kit/src/components/AccountAvatar/AccountAvatar.tsx(4 hunks)packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx(3 hunks)packages/shared/src/utils/deviceUtils.ts(4 hunks)packages/shared/types/device.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx (3)
packages/shared/types/account.ts (1)
IHwQrWalletWithDevice(56-59)packages/kit/src/hooks/usePromiseResult.ts (1)
usePromiseResult(69-346)packages/kit/src/components/WalletAvatar/WalletAvatar.tsx (1)
IWalletAvatarProps(22-26)
packages/kit/src/components/AccountAvatar/AccountAvatar.tsx (2)
packages/kit/src/components/NetworkAvatar/NetworkAvatar.tsx (1)
NetworkAvatar(82-127)packages/shared/src/config/presetNetworks.ts (1)
presetNetworksMap(2897-3016)
packages/shared/src/utils/deviceUtils.ts (1)
packages/shared/types/device.ts (2)
IOneKeyDeviceFeatures(26-26)IOneKeyDeviceFeaturesWithAppParams(28-30)
⏰ 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). (4)
- GitHub Check: lint (20.x)
- GitHub Check: unittest (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (3)
packages/shared/types/device.ts (1)
28-30: New features-with-app-params type looks consistentExtending
IOneKeyDeviceFeatureswith optional$app_firmware_typematches how you serialize features inLocalDbBaseand consume them indeviceUtils. No issues from a typing or usage standpoint.packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1)
1225-1279: Propagating$app_firmware_typeon BTC-only transitions is correctSetting
$app_firmware_typewhen flipping between Universal and Bitcoin‑only keeps the cachedfeaturesInfoin sync with the actual firmware type, and matches howupdateDeviceVersionInfomergesbitcoinOnlyFlaginto features. The logic for capabilities and vendor remains unchanged.packages/kit-bg/src/dbs/local/LocalDbBase.ts (1)
2145-2156: Good centralization of app firmware type into storedfeaturesUsing
deviceUtils.attachAppParamsToFeaturesin bothupdateDeviceandcreateHwWallet, then serializing viastableStringify, gives you a single, consistent place to derive and persist$app_firmware_type. ExtendingbitcoinOnlyFlagto carry$app_firmware_typealso keeps version updates in sync with firmware transitions.This wiring matches how
deviceUtils.getFirmwareTypeByCachedFeaturesand the various “is BTC-only” checks read fromfeaturesInfo. No functional issues here.Also applies to: 2224-2231, 2849-2853
| import type { ReactElement } from 'react'; | ||
| import { isValidElement, useMemo } from 'react'; | ||
|
|
||
| import { EFirmwareType } from '@onekeyfe/hd-shared'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Tighten useMemo dependencies for firmwareType
firmwareType’s useMemo reads wallet but only depends on wallet?.associatedDeviceInfo?.featuresInfo. This can trip hook lint rules and, in edge cases, return stale values.
You can depend on wallet directly (or pre‑extract featuresInfo) to keep things in sync:
- const firmwareType = useMemo(() => {
+ const firmwareType = useMemo(() => {
if (!wallet) {
return undefined;
}
return deviceUtils.getFirmwareTypeByCachedFeatures({
features: wallet?.associatedDeviceInfo?.featuresInfo,
});
- }, [wallet?.associatedDeviceInfo?.featuresInfo]);
+ }, [wallet, wallet?.associatedDeviceInfo?.featuresInfo]);Also applies to: 27-30, 150-157
| {firmwareType === EFirmwareType.BitcoinOnly ? ( | ||
| <Stack | ||
| position="absolute" | ||
| h="$4" | ||
| px="$0.5" | ||
| justifyContent="center" | ||
| top={-4} | ||
| left={-4} | ||
| borderRadius="$full" | ||
| zIndex="$1" | ||
| > | ||
| <NetworkAvatar networkId={presetNetworksMap.btc.id} size={14} /> | ||
| </Stack> | ||
| ) : null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Clarify when the BTC badge should render
The BTC NetworkAvatar badge always renders for Bitcoin‑only firmware, even if networkId is already BTC. This can lead to two BTC icons in the same corner.
If you want the badge to signal “BTC‑only device”, not duplicate the current network icon, guard on networkId:
- {firmwareType === EFirmwareType.BitcoinOnly ? (
+ {firmwareType === EFirmwareType.BitcoinOnly &&
+ networkId !== presetNetworksMap.btc.id ? (
<Stack
position="absolute"
h="$4"
px="$0.5"
justifyContent="center"
top={-4}
left={-4}
borderRadius="$full"
zIndex="$1"
>
<NetworkAvatar networkId={presetNetworksMap.btc.id} size={14} />
</Stack>
) : null}🤖 Prompt for AI Agents
In packages/kit/src/components/AccountAvatar/AccountAvatar.tsx around lines 356
to 369, the BTC NetworkAvatar badge is rendered for all Bitcoin‑only firmware
devices which can duplicate the visible network icon; change the render
condition to require both firmwareType === EFirmwareType.BitcoinOnly and
networkId !== presetNetworksMap.btc.id so the BTC badge only appears when the
current networkId is not already BTC (use the existing presetNetworksMap.btc.id
constant for the comparison).
| import type { EFirmwareType } from '@onekeyfe/hd-shared'; | ||
|
|
||
| export type IDeviceManagementListModalItem = IHwQrWalletWithDevice & { | ||
| firmwareTypeBadge?: EFirmwareType; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Leverage cached firmware type to avoid async per-item SDK calls
Enriching each list item with firmwareTypeBadge is a good fit for DeviceManagementListModal. Right now you call deviceUtils.getFirmwareType in a for..of loop, which invokes the hardware SDK even though device.device?.featuresInfo already carries the app-level firmware type.
You can switch to the synchronous cache helper and drop the awaits:
- const devices: Array<IDeviceManagementListModalItem> = Object.values(r)
+ const devices: Array<IDeviceManagementListModalItem> = Object.values(r)
.filter(
(item): item is IHwQrWalletWithDevice =>
Boolean(item.device) && !item.wallet.deprecated,
)
.sort((a, b) => {
// Sort by walletOrder or fallback to walletNo
const orderA = a.wallet.walletOrder || a.wallet.walletNo;
const orderB = b.wallet.walletOrder || b.wallet.walletNo;
return orderA - orderB;
});
- for (const item of devices) {
- const firmwareTypeBadge = await deviceUtils.getFirmwareType({
- features: item.device?.featuresInfo,
- });
- item.firmwareTypeBadge = firmwareTypeBadge;
- }
+ for (const item of devices) {
+ item.firmwareTypeBadge =
+ deviceUtils.getFirmwareTypeByCachedFeatures({
+ features: item.device?.featuresInfo,
+ });
+ }This keeps UI behavior the same, avoids unnecessary async work in the render data path, and uses the same cached field you already store in featuresInfo.
Also applies to: 46-47, 53-71, 120-125
🤖 Prompt for AI Agents
In
packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx
around lines 36-40 (and also apply the same change at 46-47, 53-71, 120-125),
the code calls the async SDK helper to compute firmwareTypeBadge for each item;
instead, read the firmware type synchronously from device.device?.featuresInfo
(or use the synchronous cached helper provided in deviceUtils), remove the
awaits/async SDK calls inside the per-item loop and other locations, and set
firmwareTypeBadge directly from that cached value so the UI remains unchanged
while avoiding unnecessary async SDK calls.
| function getFirmwareTypeByCachedFeatures({ | ||
| features, | ||
| }: { | ||
| features: | ||
| | (IOneKeyDeviceFeatures & { $app_firmware_type?: EFirmwareType }) | ||
| | undefined; | ||
| }) { | ||
| if (!features) { | ||
| return EFirmwareType.Universal; | ||
| } | ||
|
|
||
| return features.$app_firmware_type; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use cached $app_firmware_type more aggressively to avoid extra SDK calls
The new helpers are a solid addition:
getFirmwareTypeByCachedFeaturescleanly reads$app_firmware_typefrom stored features for UI and services.attachAppParamsToFeaturescentralizes augmenting features before persisting them.
Two refinements would reduce redundant SDK work and keep behavior simpler:
- Short-circuit
getFirmwareTypeon any cached value, not just Bitcoin-only
Right now you only skip the SDK when $app_firmware_type === BitcoinOnly. For features that already carry Universal, you still call into sdkGetFirmwareType:
async function getFirmwareType({
features,
}: {
features:
| (IOneKeyDeviceFeatures & { $app_firmware_type?: EFirmwareType })
| undefined;
}) {
if (!features) {
return EFirmwareType.Universal;
}
- if (
- features.$app_firmware_type &&
- features.$app_firmware_type === EFirmwareType.BitcoinOnly
- ) {
- return EFirmwareType.BitcoinOnly;
- }
+ if (features.$app_firmware_type) {
+ return features.$app_firmware_type;
+ }
const { getFirmwareType: sdkGetFirmwareType } = await CoreSDKLoader();
return sdkGetFirmwareType(features);
}- Align
getFirmwareTypeByCachedFeatures’s param type withIOneKeyDeviceFeaturesWithAppParams
You already declared IOneKeyDeviceFeaturesWithAppParams. Using it here makes the intent clearer and avoids duplicating the inline intersection:
-function getFirmwareTypeByCachedFeatures({
- features,
-}: {
- features:
- | (IOneKeyDeviceFeatures & { $app_firmware_type?: EFirmwareType })
- | undefined;
-}) {
+function getFirmwareTypeByCachedFeatures({
+ features,
+}: {
+ features: IOneKeyDeviceFeaturesWithAppParams | undefined;
+}) {Both changes keep the external behavior the same but simplify the code path and reduce dependency on the hardware SDK for already-augmented feature blobs.
Also applies to: 576-596, 676-685, 687-722
🤖 Prompt for AI Agents
In packages/shared/src/utils/deviceUtils.ts around lines 562-574 (and similarly
apply the same changes to ranges 576-596, 676-685, 687-722), update
getFirmwareTypeByCachedFeatures to accept IOneKeyDeviceFeaturesWithAppParams as
its parameter type and to short-circuit whenever a cached $app_firmware_type
exists (return it directly) instead of only short-circuiting on BitcoinOnly;
then adjust callers that currently only bypass sdkGetFirmwareType for
BitcoinOnly so they use this helper to skip the SDK call whenever
$app_firmware_type is present, reducing duplicate SDK requests while preserving
existing behavior.
2d2548e to
502d734
Compare
502d734 to
4d11a00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (5)
packages/shared/src/utils/deviceUtils.ts (2)
576-596: Short-circuit on any cached firmware type.Currently you only skip the SDK call when
$app_firmware_type === BitcoinOnly. For features withUniversalcached, you still hit the SDK.Return any cached value directly:
async function getFirmwareType({ features, }: { features: | (IOneKeyDeviceFeatures & { $app_firmware_type?: EFirmwareType }) | undefined; }) { if (!features) { return EFirmwareType.Universal; } - if ( - features.$app_firmware_type && - features.$app_firmware_type === EFirmwareType.BitcoinOnly - ) { - return EFirmwareType.BitcoinOnly; + if (features.$app_firmware_type) { + return features.$app_firmware_type; } const { getFirmwareType: sdkGetFirmwareType } = await CoreSDKLoader(); return sdkGetFirmwareType(features); }This reduces redundant SDK calls for already-augmented features.
562-574: Use the defined type instead of inline intersection.You already declared
IOneKeyDeviceFeaturesWithAppParamsbut duplicate the intersection inline.Use the shared type for clarity:
function getFirmwareTypeByCachedFeatures({ features, }: { - features: - | (IOneKeyDeviceFeatures & { $app_firmware_type?: EFirmwareType }) - | undefined; + features: IOneKeyDeviceFeaturesWithAppParams | undefined; }) {Apply the same change to
getFirmwareType(lines 576-596) andattachAppParamsToFeaturesinput (lines 676-685) for consistency.packages/kit/src/components/AccountAvatar/AccountAvatar.tsx (2)
150-157: TightenuseMemodependencies.The hook depends on
wallet?.associatedDeviceInfobut only readswallet?.associatedDeviceInfo?.featuresInfo. This can cause stale values or unnecessary recalculations.Depend on the specific nested field or the entire wallet:
const firmwareType = useMemo(() => { if (!wallet?.associatedDeviceInfo) { return undefined; } return deviceUtils.getFirmwareTypeByCachedFeatures({ features: wallet.associatedDeviceInfo?.featuresInfo, }); - }, [wallet?.associatedDeviceInfo]); + }, [wallet?.associatedDeviceInfo?.featuresInfo]);
356-369: Guard against duplicate BTC badge.The badge renders for all Bitcoin-only firmware, even when
networkIdis already BTC. This can show two BTC icons in the same corner.Only show the badge when the current network isn't BTC:
- {firmwareType === EFirmwareType.BitcoinOnly ? ( + {firmwareType === EFirmwareType.BitcoinOnly && + networkId !== presetNetworksMap.btc.id ? ( <Stack position="absolute" h="$4" px="$0.5" justifyContent="center" top={-4} left={-4} borderRadius="$full" zIndex="$1" > <NetworkAvatar networkId={presetNetworksMap.btc.id} size={14} /> </Stack> ) : null}packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx (1)
65-71: Use cached firmware type to avoid async SDK calls.The loop calls
deviceUtils.getFirmwareTypefor each item, which hits the hardware SDK even thoughdevice.device?.featuresInfoalready carries the app-level firmware type.Switch to the synchronous cache helper:
for (const item of devices) { - const firmwareTypeBadge = await deviceUtils.getFirmwareType({ + item.firmwareTypeBadge = + deviceUtils.getFirmwareTypeByCachedFeatures({ features: item.device?.featuresInfo, }); - item.firmwareTypeBadge = firmwareTypeBadge; }This avoids unnecessary async work in the render data path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
packages/kit-bg/src/dbs/local/LocalDbBase.ts(3 hunks)packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts(3 hunks)packages/kit-bg/src/services/ServiceQrWallet/ServiceQrWallet.ts(3 hunks)packages/kit/src/components/AccountAvatar/AccountAvatar.tsx(4 hunks)packages/kit/src/views/AccountManagerStacks/pages/AccountSelectorStack/WalletDetails/WalletDetailsHeader/index.tsx(3 hunks)packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx(3 hunks)packages/shared/src/utils/deviceUtils.ts(4 hunks)packages/shared/types/device.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/kit/src/views/AccountManagerStacks/pages/AccountSelectorStack/WalletDetails/WalletDetailsHeader/index.tsx (1)
packages/kit/src/components/WalletAvatar/WalletAvatar.tsx (1)
WalletAvatar(63-120)
packages/kit/src/components/AccountAvatar/AccountAvatar.tsx (2)
packages/kit/src/components/NetworkAvatar/NetworkAvatar.tsx (1)
NetworkAvatar(82-127)packages/shared/src/config/presetNetworks.ts (1)
presetNetworksMap(2897-3016)
packages/shared/src/utils/deviceUtils.ts (1)
packages/shared/types/device.ts (2)
IOneKeyDeviceFeatures(26-26)IOneKeyDeviceFeaturesWithAppParams(28-30)
packages/kit/src/views/DeviceManagement/pages/DeviceManagementListModal/index.tsx (3)
packages/shared/types/account.ts (1)
IHwQrWalletWithDevice(56-59)packages/kit/src/hooks/usePromiseResult.ts (1)
usePromiseResult(69-346)packages/kit/src/components/WalletAvatar/WalletAvatar.tsx (1)
IWalletAvatarProps(22-26)
⏰ 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). (4)
- GitHub Check: unittest (20.x)
- GitHub Check: lint (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (5)
packages/shared/types/device.ts (1)
28-30: LGTM!The type extension cleanly adds app-level firmware type tracking to device features.
packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1)
1225-1279: LGTM!The firmware type propagation through upgrade flows is correct. The logic correctly assigns
$app_firmware_typefor both upgrade directions.packages/kit-bg/src/services/ServiceQrWallet/ServiceQrWallet.ts (1)
252-256: LGTM!The firmware-aware network selection correctly restricts to all default networks for BTC-only firmware, aligning with the PR objectives.
packages/kit-bg/src/dbs/local/LocalDbBase.ts (2)
2164-2177: Attaching app params before persisting features is a solid moveYou now normalize features through
attachAppParamsToFeaturesand persist viastableStringify, which keeps device feature JSON consistent and makes change detection reliable. I don’t see functional issues here.
2246-2251: ExtendingbitcoinOnlyFlagwith app firmware type matches the merge logicAdding
$app_firmware_typeto thebitcoinOnlyFlagshape lines up with the spread intoitem.featuresand lets version updates override or inject firmware type cleanly. No problems spotted in this flow.
| const featuresInfo = await deviceUtils.attachAppParamsToFeatures({ | ||
| features, | ||
| }); | ||
| const featuresStr = JSON.stringify(featuresInfo); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using stableStringify for HW featuresStr for consistency
Here you serialize featuresInfo with JSON.stringify, while updateDevice uses stringUtils.stableStringify. Using stableStringify here too would keep the stored features string canonical from first insert and avoid a one-time diff on the first updateDevice call.
- const featuresInfo = await deviceUtils.attachAppParamsToFeatures({
- features,
- });
- const featuresStr = JSON.stringify(featuresInfo);
+ const featuresInfo = await deviceUtils.attachAppParamsToFeatures({
+ features,
+ });
+ const featuresStr = stringUtils.stableStringify(featuresInfo);🤖 Prompt for AI Agents
In packages/kit-bg/src/dbs/local/LocalDbBase.ts around lines 2868 to 2872, the
code uses JSON.stringify to serialize featuresInfo which causes a non-canonical
representation compared to updateDevice that uses stringUtils.stableStringify;
replace JSON.stringify(featuresInfo) with
stringUtils.stableStringify(featuresInfo) and ensure stringUtils is imported in
this file (or the correct stable stringify helper is referenced) so the stored
featuresStr is canonical from initial insert and avoids a one-time diff on first
updateDevice call.
| const firmwareType = useMemo(() => { | ||
| return deviceUtils.getFirmwareTypeByCachedFeatures({ | ||
| features: device?.featuresInfo, | ||
| }); | ||
| }, [device]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Tighten useMemo dependencies.
The hook depends on device but only reads device?.featuresInfo. This can cause stale closures or unnecessary recalculations.
Extract the nested value or depend on both for clarity:
const firmwareType = useMemo(() => {
+ const features = device?.featuresInfo;
return deviceUtils.getFirmwareTypeByCachedFeatures({
- features: device?.featuresInfo,
+ features,
});
- }, [device]);
+ }, [device?.featuresInfo]);🤖 Prompt for AI Agents
In
packages/kit/src/views/AccountManagerStacks/pages/AccountSelectorStack/WalletDetails/WalletDetailsHeader/index.tsx
around lines 63 to 67, the useMemo currently lists [device] but only reads
device?.featuresInfo which can cause stale closures or extra recalcs; extract
the nested value (e.g. const features = device?.featuresInfo) and change the
useMemo dependency to that extracted value (or directly to device?.featuresInfo)
so the memo only reruns when features actually change.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.