Feat/attach pin - #501
Conversation
WalkthroughThis update adds new features and refactors device passphrase and PIN handling across the codebase. It introduces new device unlock methods, enhances passphrase state management, and updates UI components for better user interaction. Several package versions are bumped, and new localization strings are added. State management shifts to Jotai atoms, and new test tools support PIN attachment workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant DeviceProvider
participant JotaiAtoms
participant HardwareInputPinProvider
participant HardwarePassphraseProvider
participant CoreAPI
participant Device
UI->>DeviceProvider: Render DeviceList
DeviceProvider->>JotaiAtoms: Get selected device
UI->>HardwareInputPinProvider: Trigger PIN dialog on event
HardwareInputPinProvider->>UI: Show ReceivePin modal
UI->>HardwarePassphraseProvider: Trigger passphrase dialog on event
HardwarePassphraseProvider->>UI: Show EnterPhase modal
UI->>CoreAPI: Call deviceUnlock/connect
CoreAPI->>Device: Send command (GetAddress/GetFeatures/LockDevice)
Device->>CoreAPI: Return features/passphrase state
CoreAPI->>UI: Return result
sequenceDiagram
participant SDK
participant Device
participant UI
participant JotaiAtoms
SDK->>Device: Request Passphrase or PIN
Device->>SDK: Emit event with payload (existsAttachPinUser, etc.)
SDK->>UI: Show dialog (PIN or Passphrase)
UI->>JotaiAtoms: Update state (PIN value, passphrase, etc.)
UI->>SDK: Submit response (with attachPinOnDevice if applicable)
SDK->>Device: Send PassphraseAck/response
Device->>SDK: Continue operation
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
fc62fbf to
ea6f05c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
🔭 Outside diff range comments (1)
packages/connect-examples/expo-example/src/components/ui/Header.tsx (1)
40-48: Use keyExtractor instead of inline map for better React perf.Mapping over
visibleItemscreates a new function each render. Minor, but easy to optimise:- {visibleItems.map(item => ( - <Group.Item key={item.route}> + {visibleItems.map(({ route, labelId }) => ( + <Group.Item key={route}> <Button - variant={currentRoute === item.route ? 'primary' : 'secondary'} - onPress={() => navigate(item.route)} + variant={currentRoute === route ? 'primary' : 'secondary'} + onPress={() => navigate(route)} > - {intl.formatMessage({ id: item.labelId })} + {intl.formatMessage({ id: labelId })} </Button>
📜 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
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
packages/connect-examples/electron-example/package.json(1 hunks)packages/connect-examples/expo-example/App.tsx(4 hunks)packages/connect-examples/expo-example/locale/en-US.json(3 hunks)packages/connect-examples/expo-example/locale/zh-CN.json(3 hunks)packages/connect-examples/expo-example/package.json(3 hunks)packages/connect-examples/expo-example/src/atoms/deviceAtoms.ts(1 hunks)packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts(1 hunks)packages/connect-examples/expo-example/src/components/CommonParamsView.tsx(1 hunks)packages/connect-examples/expo-example/src/components/DeviceList.tsx(5 hunks)packages/connect-examples/expo-example/src/components/EnterPhase.tsx(1 hunks)packages/connect-examples/expo-example/src/components/HandleSDKEvents.tsx(1 hunks)packages/connect-examples/expo-example/src/components/ReceivePin.tsx(1 hunks)packages/connect-examples/expo-example/src/components/ui/Header.tsx(1 hunks)packages/connect-examples/expo-example/src/data/device.ts(1 hunks)packages/connect-examples/expo-example/src/provider/CommonParamsProvider.tsx(1 hunks)packages/connect-examples/expo-example/src/provider/DeviceProvider.tsx(1 hunks)packages/connect-examples/expo-example/src/provider/HardwareInputPinProvider.tsx(3 hunks)packages/connect-examples/expo-example/src/provider/HardwarePassphraseProvider.tsx(1 hunks)packages/connect-examples/expo-example/src/route.ts(1 hunks)packages/connect-examples/expo-example/src/testTools/attachToPinTest/atoms.ts(1 hunks)packages/connect-examples/expo-example/src/testTools/attachToPinTest/index.tsx(1 hunks)packages/connect-examples/expo-example/src/views/AttachToPinTestingScreen.tsx(1 hunks)packages/connect-examples/expo-example/src/views/FirmwareScreen/index.tsx(4 hunks)packages/connect-examples/expo-example/tamagui.config.ts(2 hunks)packages/core/package.json(2 hunks)packages/core/src/api/FirmwareUpdate.ts(1 hunks)packages/core/src/api/GetPassphraseState.ts(1 hunks)packages/core/src/api/allnetwork/AllNetworkGetAddress.ts(6 hunks)packages/core/src/api/device/DeviceUnlock.ts(1 hunks)packages/core/src/api/index.ts(1 hunks)packages/core/src/api/stellar/StellarSignTransaction.ts(0 hunks)packages/core/src/core/index.ts(5 hunks)packages/core/src/data/messages/messages.json(7 hunks)packages/core/src/device/Device.ts(7 hunks)packages/core/src/device/DeviceCommands.ts(4 hunks)packages/core/src/events/device.ts(1 hunks)packages/core/src/events/ui-request.ts(2 hunks)packages/core/src/events/ui-response.ts(1 hunks)packages/core/src/inject.ts(1 hunks)packages/core/src/types/api/deviceUnlock.ts(1 hunks)packages/core/src/types/api/index.ts(2 hunks)packages/core/src/types/params.ts(1 hunks)packages/core/src/utils/deviceFeaturesUtils.ts(3 hunks)packages/hd-ble-sdk/package.json(2 hunks)packages/hd-common-connect-sdk/package.json(2 hunks)packages/hd-transport-http/package.json(2 hunks)packages/hd-transport-lowlevel/package.json(2 hunks)packages/hd-transport-react-native/package.json(2 hunks)packages/hd-transport-webusb/package.json(2 hunks)packages/hd-transport/package.json(1 hunks)packages/hd-transport/src/types/messages.ts(6 hunks)packages/hd-web-sdk/package.json(2 hunks)packages/shared/package.json(1 hunks)packages/shared/src/HardwareError.ts(2 hunks)submodules/firmware(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/api/stellar/StellarSignTransaction.ts
🧰 Additional context used
🧬 Code Graph Analysis (13)
packages/connect-examples/expo-example/src/components/CommonParamsView.tsx (1)
packages/connect-examples/expo-example/src/components/SwitchInput.tsx (1)
SwitchInput(12-37)
packages/core/src/api/GetPassphraseState.ts (1)
packages/core/src/utils/deviceFeaturesUtils.ts (1)
getPassphraseStateWithRefreshDeviceInfo(76-121)
packages/core/src/types/api/deviceUnlock.ts (1)
packages/core/src/types/params.ts (2)
CommonParams(1-45)Response(59-59)
packages/connect-examples/expo-example/src/views/AttachToPinTestingScreen.tsx (4)
packages/connect-examples/expo-example/src/provider/DeviceProvider.tsx (1)
DeviceProvider(24-24)packages/connect-examples/expo-example/src/provider/HardwareInputPinProvider.tsx (1)
HardwareInputPinDialogProvider(21-84)packages/connect-examples/expo-example/src/provider/HardwarePassphraseProvider.tsx (1)
HardwarePassphraseDialogProvider(23-110)packages/connect-examples/expo-example/src/testTools/attachToPinTest/index.tsx (1)
AttachToPinTestProvider(186-188)
packages/core/src/events/ui-request.ts (1)
packages/hd-transport/src/types/messages.ts (1)
PinMatrixRequestType(1219-1219)
packages/connect-examples/expo-example/src/provider/HardwarePassphraseProvider.tsx (2)
packages/core/src/events/ui-response.ts (1)
UI_RESPONSE(4-9)packages/connect-examples/expo-example/src/components/EnterPhase.tsx (1)
EnterPhase(20-102)
packages/core/src/core/index.ts (4)
packages/core/src/events/ui-request.ts (1)
UI_REQUEST(8-44)packages/core/src/types/device.ts (1)
KnownDevice(22-37)packages/core/src/device/Device.ts (1)
DeviceEvents(65-79)packages/core/src/events/ui-response.ts (1)
UI_RESPONSE(4-9)
packages/connect-examples/expo-example/src/atoms/deviceAtoms.ts (1)
packages/connect-examples/expo-example/src/components/DeviceList.tsx (1)
Device(24-31)
packages/core/src/device/DeviceCommands.ts (1)
packages/core/src/events/device.ts (1)
PassphraseRequestPayload(47-49)
packages/core/src/api/device/DeviceUnlock.ts (3)
packages/hd-transport/src/types/messages.ts (1)
LockDevice(2390-2390)packages/core/src/api/helpers/pathUtils.ts (1)
toHardened(7-7)packages/shared/src/HardwareError.ts (1)
HardwareErrorCode(46-415)
packages/connect-examples/expo-example/src/components/EnterPhase.tsx (2)
packages/connect-examples/expo-example/src/provider/MediaProvider.tsx (1)
useMedia(20-20)packages/connect-examples/expo-example/src/components/ui/Button.tsx (1)
Button(204-204)
packages/connect-examples/expo-example/src/provider/DeviceProvider.tsx (2)
packages/connect-examples/expo-example/src/atoms/deviceAtoms.ts (1)
selectDeviceAtom(4-4)packages/connect-examples/expo-example/src/components/DeviceList.tsx (1)
DeviceList(228-228)
packages/hd-transport/src/types/messages.ts (1)
packages/core/src/api/GetPassphraseState.ts (1)
GetPassphraseState(6-27)
🔇 Additional comments (80)
submodules/firmware (1)
1-1: Submodule firmware commit not found on remoteI ran:
git ls-remote https://github.com/OneKeyHQ/firmware.git | grep a8996f9b3f66815f1cd6e8f145b99856a69d782aNo match. Please manually verify that this commit hash exists on the public firmware repo and is merged or tagged on the expected branch. Also, don’t forget to:
- Add a short note in the PR body or commit message summarizing key firmware changes (new protobuf messages, bug-fixes, breaking changes).
- Confirm the remote URL and branch are correct and that the commit is reachable.
- Ensure CI jobs use
git submodule update --init --depth 1to avoid shallow-clone issues.packages/hd-transport/package.json (1)
3-3: Ensure all downstream packages consume the alpha tag.Transport is now on
1.0.39-alpha.1. Double-check that every consumer in the monorepo points to the same alpha or you’ll get mismatched dependency graphs during install.packages/connect-examples/electron-example/package.json (1)
5-5: Confirm auto-update channels handle alpha versions.Electron Builder may treat
1.0.39-alpha.1as a prerelease. Verify your update server’s semver rules so users don’t miss stable updates later.packages/core/package.json (1)
3-3: Core’s version bump looks good.Nothing further here.
packages/core/src/api/FirmwareUpdate.ts (1)
169-169: Good logging standardization!Using
Log.loginstead ofconsole.logmaintains consistency with the rest of the file and provides better observability.packages/hd-transport-webusb/package.json (1)
3-3: Version synchronization looks good.The alpha version bump and dependency alignment are consistent with the coordinated release strategy.
Also applies to: 23-24
packages/core/src/types/params.ts (1)
41-44: Clean parameter addition.The
skipPassphraseCheckproperty is well-documented and properly typed as optional. Nice clear naming.packages/hd-transport-http/package.json (1)
3-3: Consistent versioning across transports.The version updates match the coordinated release pattern and dependencies are properly aligned.
Also applies to: 27-28
packages/connect-examples/expo-example/src/provider/CommonParamsProvider.tsx (1)
13-13: Safe default value for the new parameter.Setting
skipPassphraseCheck: falseas the default ensures the passphrase check runs unless explicitly disabled. Good defensive programming.packages/core/src/events/ui-response.ts (1)
18-23: Document the new flag and check downstream usage
attachPinOnDevicelooks fine, but add a short JSDoc so future devs know when to set it. Also grep the repo to be sure all handlers look for this flag; missing checks will break flows silently.packages/hd-web-sdk/package.json (1)
3-4: Caret + pre-release can fetch wrong builds
^1.0.39-alpha.1lets npm jump to1.0.39(stable) once it appears, skipping later alphas or RCs. If you need the exact alpha, lock it with"1.0.39-alpha.1"or switch to~.Also applies to: 24-28
packages/connect-examples/expo-example/src/data/device.ts (1)
73-77: Clean implementation follows established patterns.The new
deviceUnlockAPI entry is properly structured and positioned logically next todeviceLock. ThenoDeviceIdReq: truesetting is appropriate for device unlock operations.packages/hd-transport-react-native/package.json (1)
3-3: Version bump looks clean and consistent.The package version and dependencies are properly synchronized at
1.0.39-alpha.1. The alpha version indicates this is a pre-release version, which is appropriate for new features.Also applies to: 22-23
packages/hd-common-connect-sdk/package.json (1)
3-3: Coordinated version bump maintains package consistency.All dependencies are properly updated to match the new package version. The synchronized versioning ensures compatibility across the SDK ecosystem.
Also applies to: 23-27
packages/core/src/types/api/index.ts (1)
38-38: Type definitions properly integrated.The
deviceUnlockimport and type definition are correctly added and positioned logically next todeviceLock. The implementation follows established patterns in the codebase.Also applies to: 218-218
packages/connect-examples/expo-example/src/components/HandleSDKEvents.tsx (1)
75-77: Clean logging removal approved.Good cleanup! Removing the console.log noise while keeping the event emission intact makes the code cleaner.
packages/hd-ble-sdk/package.json (2)
3-3: Version bump looks good.Clean alpha version update aligns with the broader SDK changes.
23-25: Dependency versions match perfectly.All three dependency versions updated consistently to the same alpha version.
packages/core/src/api/index.ts (1)
30-30: deviceUnlock API implementation verifiedThe new export in
packages/core/src/api/index.tsmatches theDeviceUnlockclass atpackages/core/src/api/device/DeviceUnlock.ts. No issues found—everything’s wired up correctly.packages/core/src/inject.ts (1)
141-142: DeviceUnlock method implemented correctly.The implementation follows the established pattern. The hardcoded
useEmptyPassphrase: truesuggests this is specifically for empty passphrase unlocks, which makes sense for a general unlock function.packages/connect-examples/expo-example/package.json (3)
3-3: Version bump matches the ecosystem.Clean alpha version update aligns with other packages in the PR.
22-25: OneKey dependencies updated consistently.All four OneKey SDK dependencies bumped to the same alpha version. Good consistency.
39-39: Toast dependency added for UI improvements.The new @tamagui/toast dependency supports the enhanced user feedback features mentioned in the PR.
packages/connect-examples/expo-example/src/views/AttachToPinTestingScreen.tsx (1)
1-21: LGTM! Clean component structure with proper provider nesting.The component follows React best practices with well-organized imports and a clear provider hierarchy. The nested context pattern correctly establishes the required hardware interaction environment.
packages/connect-examples/expo-example/tamagui.config.ts (1)
45-45: Performance improvement: Animation feels smoother now.Reducing stiffness from 250 to 150 makes the "quick" animation less aggressive and more user-friendly.
packages/connect-examples/expo-example/locale/zh-CN.json (1)
8-8: Localization additions look good and follow naming conventions.The new Chinese translations properly support the PIN attachment feature with consistent key naming patterns.
Also applies to: 11-11, 27-27, 51-51
packages/shared/src/HardwareError.ts (2)
139-142: New error code follows established patterns correctly.The error code numbering is sequential and the JSDoc comment format is consistent with existing entries.
440-440: Error message is clear and descriptive.The message accurately describes the unlock type mismatch scenario.
packages/core/src/types/api/deviceUnlock.ts (1)
1-4: Clean TypeScript declaration with proper typing.The function signature follows project conventions with correct imports and type annotations. The mandatory
connectIdand optionalparamsstructure is consistent with other API methods.packages/core/src/api/GetPassphraseState.ts (2)
16-16: Clean destructuring change.The destructuring correctly extracts only the needed
passphraseStatefrom the returned object.
20-20: Comment simplified effectively.The generic comment "refresh device info" is clearer than the previous specific wording.
packages/core/src/events/ui-request.ts (2)
69-69: New button request type follows pattern.The
'ButtonRequest_AttachPin'addition is consistent with existing button request types.
83-83: Clean optional property addition.The
existsAttachPinUserboolean field is properly optional and well-named.packages/connect-examples/expo-example/src/provider/HardwareInputPinProvider.tsx (2)
36-38: Clean payload propagation.The payload parameter flows correctly through the dialog state. Good implementation.
74-74: Props passed correctly.The payload prop is correctly passed to the ReceivePin component.
packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts (2)
1-1: Good import addition.The
atomWithStorageimport enables persistence for the connection type.
6-9: Smart persistence upgrade.Changing to
atomWithStoragewith the 'onekey-connectionType' key preserves user preferences across sessions. Good UX improvement.packages/connect-examples/expo-example/locale/en-US.json (1)
8-8: LGTM! Clean localization additions.The new localization keys follow the existing naming convention and provide clear, descriptive labels for the attach-to-pin feature.
Also applies to: 11-11, 27-27, 51-51
packages/connect-examples/expo-example/App.tsx (2)
134-160: Well-structured toast implementation.The ToastView component properly handles toast state and provides clean UI feedback. The positioning and styling choices are appropriate.
56-59: Good integration of the new screen.The AttachToPinTestingScreen registration follows the existing navigation pattern correctly.
packages/connect-examples/expo-example/src/testTools/attachToPinTest/index.tsx (5)
21-51: Clean account management component.The AccountLists component properly handles account display and address checking with good error handling and user feedback via toast notifications.
53-78: Well-structured wallet list display.The WalletLists component provides a clean interface for viewing wallet details and deletion functionality.
80-122: Good loading state management.The LoadingView component properly handles different request states with appropriate UI feedback.
133-150: Proper event handling setup.The hardware UI event listener is correctly implemented with proper cleanup in the useEffect hook.
136-138: Confirm or remove the 100 ms delay before opening the passphrase dialogI didn’t find other instances of delaying dialog opens in the repo. Are these 100 ms waits essential for UI stability or state updates?
- If not, call
openPassphraseDialog()directly.- If yes, please add a comment explaining why the timeout is needed.
Locations to check:
- packages/connect-examples/expo-example/src/testTools/attachToPinTest/index.tsx: lines 136–138 and 140–142
packages/connect-examples/expo-example/src/provider/HardwarePassphraseProvider.tsx (3)
42-56: Clean passphrase confirmation logic.The onConfirmPassphrase callback properly handles the UI response with correct payload structure.
58-82: Well-structured device input modes.The callbacks for different input modes (device-only and device-with-pin) are properly implemented with correct payload flags.
112-120: Proper context hook implementation.The useHardwarePassphraseDialog hook correctly enforces provider usage with clear error messaging.
packages/connect-examples/expo-example/src/atoms/deviceAtoms.ts (2)
4-6: Clean atom definitions.The selectDeviceAtom and deviceListAtom are properly typed and follow Jotai conventions.
8-29: Well-structured action atom.The deviceActionsAtom properly handles different device actions with clear type discrimination and appropriate state updates.
packages/connect-examples/expo-example/src/provider/DeviceProvider.tsx (2)
1-11: Clean migration to Jotai state management.The refactor from local React context to global Jotai atoms is well-executed. The
useDevicehook is properly simplified and the import statements are clean.
17-21: DeviceList integration aligns with centralized state.Removing the
onSelectedprop from DeviceList is correct since device selection is now managed through the globalselectDeviceAtom. This maintains consistency with the state management refactor.packages/core/src/device/DeviceCommands.ts (3)
13-13: New attachPinOnDevice property enhances passphrase response.The addition of the optional
attachPinOnDeviceboolean toPassphrasePromptResponseproperly extends the type to support the new attach pin functionality.
441-455: Attach pin logic implementation is sound.The extraction of
existsAttachPinUserfrom the message and the conditional logic for handling attach pin on device is correctly implemented. The flow properly checks both flags before sending the appropriate acknowledgment.
513-539: PassphraseRequestPayload parameter properly integrated.The updated
_promptPassphrasemethod signature correctly accepts the new payload parameter and passes it through to the device event emission. This maintains the existing error handling patterns.packages/core/src/api/allnetwork/AllNetworkGetAddress.ts (3)
239-239: Original index tracking improves batch request handling.Adding
_originalIndexto the MethodParams type enables proper order preservation in batch operations. This is a clean solution for maintaining request-response order alignment.
271-294: Index propagation through generateMethodName is correct.The method signature update and index assignment properly propagate the original request position through the processing pipeline. This ensures each method call retains its original context.
378-407: Response mapping preserves original order effectively.The refactored loop using
for...ofand the index-based result mapping ensure responses are returned in the same order as input requests. This eliminates potential ordering issues in batch operations.packages/connect-examples/expo-example/src/views/FirmwareScreen/index.tsx (3)
10-26: Jotai imports and atom usage are correct.The import statements properly include the necessary Jotai hooks and the selectDeviceAtom. The migration from local state to global atoms is well-implemented.
389-392: Device state access simplified with atoms.Replacing the selectDevice prop with
useAtomValue(selectDeviceAtom)eliminates prop drilling and centralizes device state management. This follows the established migration pattern.
772-778: Global state updates properly implemented.Using
useSetAtom(selectDeviceAtom)for device state updates and removing the onSelected prop from DeviceList maintains consistency with the centralized state management approach.packages/connect-examples/expo-example/src/components/EnterPhase.tsx (4)
8-18: Interface definition is clear and well-typed.The
EnterPhasePropsinterface properly defines all callback functions and payload types. The optionalrequestPayloadwith its nested structure provides good type safety.
33-37: Submit handler implementation is efficient.The
useCallbackhook properly memoizes the submit function and handles empty passphrase gracefully with the fallback to empty string.
39-41: Responsive design and memoization are well-implemented.The responsive width calculation and title text memoization optimize performance while providing good UX across different screen sizes.
104-126: CloseButton memoization follows best practices.The memoized CloseButton component with proper displayName and clean prop interface demonstrates good React patterns. The positioning and styling are appropriate for a modal close button.
packages/connect-examples/expo-example/src/components/ReceivePin.tsx (1)
305-305: Consider security implications of displaying payload.Displaying the raw payload as JSON might expose sensitive information. Ensure the payload doesn't contain PII or sensitive data before rendering.
packages/core/src/core/index.ts (4)
344-362: Well-structured passphrase state handling.The code properly passes the new parameters to
checkPassphraseStateSafetyand closes the UI window after verification. Good error handling flow.
779-786: Logical enhancement to passphrase validation.The addition of
skipPassphraseCheckprovides flexibility while maintaining security. The condition correctly handles all cases.
844-851: Proper handling of new attach PIN flow.The code correctly handles both PIN entry and attach PIN button requests. Passing the specific request code as the UI message type enables proper UI differentiation.
862-884: All passphrase handlers are in sync.Both
onDevicePassphraseHandlerandonEmptyPassphraseHandlernow match theDeviceEvents['passphrase']signature.
onDevicePassphraseHandlerhandlesexistsAttachPinUserand returnsattachPinOnDevice.onEmptyPassphraseHandleraccepts the new payload shape and returns an empty passphrase.No further changes needed.
packages/hd-transport/src/types/messages.ts (5)
1235-1235: Good addition of PIN attachment flags.The optional boolean fields
exists_attach_pin_userandon_device_attach_pinfollow the existing pattern. The naming is consistent with other optional flags in these types.Also applies to: 1243-1243
2337-2338: Feature flags follow established pattern.The new
attach_to_pin_userandunlocked_attach_pinboolean flags match the existing Features type structure. Good consistency with other optional device capability flags.
4587-4588: MessageType integration looks correct.The new message types are properly added to the
MessageTypeinterface. This ensures type safety for the new PIN attachment functionality.
1197-1197: ButtonRequest_AttachPin (8000) is unique and matches the protobuf schema
No otherButtonRequestvalues use 8000, and it aligns with the IDs inmessages.json. No conflicts detected.
2202-2202: passphrase_state in Initialize is correctly aligned
All transport types usepassphrase_state?: string:
- Initialize (packages/hd-transport/src/types/messages.ts:2198–2203)
- GetPassphraseState (…/messages.ts:2794–2796)
- PassphraseState (…/messages.ts:2798–2803)
In
Device.ts(line 407),options.passphraseStatemaps topayload.passphrase_state. No mismatch found. No changes required.packages/core/src/data/messages/messages.json (3)
3024-3025: Confirm enum-ID uniqueness and consumer support
"ButtonRequest_AttachPin": 8000jumps from 20 to 8000. That is legal, but please double-check:
- No other
ButtonRequestTypeentry already owns 8000.- UI/event code maps 8000 to the new button label, otherwise users will see an “unknown button request” dialog.
If both are fine, no action needed.
7557-7580: New messages look good
GetPassphraseStateandPassphraseStatehave clear, minimal schemas and sequential field IDs. 👍
3075-3079: No ID collisions in PassphraseRequest
Our scan shows only IDs 1 and 8000 are used undernested.PassphraseRequest.fields. IDs 4–7999 are free, so 8000 won’t collide.If you value denser numbering and smaller tags on the wire, you can pick the next free ID (e.g. 2 or 3) instead of jumping to 8000. Otherwise, this change is safe.
packages/core/src/utils/deviceFeaturesUtils.ts (1)
142-145: Add null check for passphrase stateThe
onlyMainPincondition should handle the case where bothonlyMainPinandexpectPassphraseStateare provided.const { message, type } = await commands.typedCall('GetPassphraseState', 'PassphraseState', { - passphrase_state: options?.onlyMainPin ? undefined : options?.expectPassphraseState, + passphrase_state: options?.onlyMainPin ? undefined : (options?.expectPassphraseState ?? undefined), });Likely an incorrect or invalid review comment.
packages/core/src/device/Device.ts (1)
409-414: LGTM! Helpful debug loggingThe debug logs provide useful context for troubleshooting device initialization.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/core/src/utils/deviceFeaturesUtils.ts (1)
146-148: Replace@ts-expect-errorwith proper error handlingpackages/core/src/device/Device.ts (2)
390-390: Remove commented code
751-761: Add documentation for complex passphrase logic
📜 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
📒 Files selected for processing (2)
packages/core/src/device/Device.ts(8 hunks)packages/core/src/utils/deviceFeaturesUtils.ts(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: Analyze (javascript-typescript)
- GitHub Check: lint (18.x)
- GitHub Check: build (18.x)
🔇 Additional comments (6)
packages/core/src/utils/deviceFeaturesUtils.ts (3)
86-92: Good enhancement of passphrase state retrievalThe updated function signature and destructuring pattern properly handle the new response fields (
newSession,unlockedAttachPin). This aligns well with the enhanced passphrase management features.
141-156: Good feature detection for Pro deviceThe version check for Pro device (>= 4.15.0) properly gates the new passphrase state functionality. The structured response handling is clean.
110-118: Device.features is guaranteed before invocationWe verified that all callers check for
device.featuresbefore callinggetPassphraseStateWithRefreshDeviceInfo(e.g. inGetPassphraseState.tsandDevice.ts). The existingfeatures?.device_idguard is sufficient. No further changes needed.packages/core/src/device/Device.ts (3)
70-74: Good API design for passphrase eventsThe updated event signature properly includes
PassphraseRequestPayloadto carry additional context. This improves the passphrase handling flow.
409-414: Good debug logging for troubleshootingThe detailed debug log helps track initialization parameters. This will be valuable for debugging session-related issues.
762-770: Good error handling for attach pin misuseThe logic properly handles invalid attach pin scenarios. Locking the device and clearing state prevents security issues.
| async lockDevice() { | ||
| const res = await this.commands.typedCall('LockDevice', 'Success', {}); | ||
| return res.message; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for lockDevice
The method should handle potential errors from the device command.
async lockDevice() {
- const res = await this.commands.typedCall('LockDevice', 'Success', {});
- return res.message;
+ try {
+ const res = await this.commands.typedCall('LockDevice', 'Success', {});
+ return res.message;
+ } catch (error) {
+ throw ERRORS.TypedError(
+ HardwareErrorCode.RuntimeError,
+ `Failed to lock device: ${error.message}`
+ );
+ }
}📝 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.
| async lockDevice() { | |
| const res = await this.commands.typedCall('LockDevice', 'Success', {}); | |
| return res.message; | |
| } | |
| async lockDevice() { | |
| try { | |
| const res = await this.commands.typedCall('LockDevice', 'Success', {}); | |
| return res.message; | |
| } catch (error) { | |
| throw ERRORS.TypedError( | |
| HardwareErrorCode.RuntimeError, | |
| `Failed to lock device: ${error.message}` | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/device/Device.ts around lines 729 to 733, the lockDevice
method lacks error handling for the asynchronous device command call. Wrap the
await call to this.commands.typedCall in a try-catch block, catch any errors
thrown during the call, and handle them appropriately, such as logging the error
or rethrowing with additional context, to ensure the method gracefully manages
potential failures.
| updateInternalState( | ||
| enablePassphrase: boolean, | ||
| passphraseState: string | undefined, | ||
| deviceId: string, | ||
| sessionId: string | null = null, | ||
| featuresSessionId: string | null = null | ||
| ) { | ||
| Log.debug( | ||
| 'tryFixInternalState session param: ', | ||
| 'updateInternalState session param: ', | ||
| `device_id: ${deviceId}`, | ||
| `passphraseState: ${state}`, | ||
| `sessionId: ${sessionId}` | ||
| `enablePassphrase: ${enablePassphrase}`, | ||
| `passphraseState: ${passphraseState}`, | ||
| `sessionId: ${sessionId}`, | ||
| `featuresSessionId: ${featuresSessionId}` | ||
| ); | ||
|
|
||
| const key = `${deviceId}`; | ||
| const session = deviceSessionCache[key]; | ||
| if (session) { | ||
| deviceSessionCache[this.generateStateKey(deviceId, state)] = session; | ||
| delete deviceSessionCache[key]; | ||
| } else if (sessionId) { | ||
| deviceSessionCache[this.generateStateKey(deviceId, state)] = sessionId; | ||
| if (enablePassphrase) { | ||
| // update the sessionId | ||
| if (sessionId) { | ||
| deviceSessionCache[this.generateStateKey(deviceId, passphraseState)] = sessionId; | ||
| } else if (featuresSessionId) { | ||
| deviceSessionCache[this.generateStateKey(deviceId, passphraseState)] = featuresSessionId; | ||
| } | ||
| } | ||
| Log.debug('tryFixInternalState session cache: ', deviceSessionCache); | ||
|
|
||
| // delete the old sessionId | ||
| const oldKey = `${deviceId}`; | ||
| if (deviceSessionCache[oldKey]) { | ||
| delete deviceSessionCache[oldKey]; | ||
| } | ||
|
|
||
| Log.debug('updateInternalState session cache: ', deviceSessionCache); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clear parameter naming in updateInternalState
The refactored method with explicit parameters is much clearer than the previous tryFixInternalState. Good job making the intent explicit.
Consider adding JSDoc comments to document each parameter:
- // attach to pin to fix internal state
- updateInternalState(
+ /**
+ * Updates internal session state for passphrase management
+ * @param enablePassphrase - Whether passphrase is enabled
+ * @param passphraseState - Current passphrase state
+ * @param deviceId - Device identifier
+ * @param sessionId - New session ID to set
+ * @param featuresSessionId - Fallback session ID from features
+ */
+ updateInternalState(Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/core/src/device/Device.ts around lines 318 to 350, the
updateInternalState method has clear parameters but lacks JSDoc comments. Add
JSDoc comments above the method to document each parameter's purpose and types,
explaining enablePassphrase, passphraseState, deviceId, sessionId, and
featuresSessionId to improve code readability and maintainability.
* feat: attach to pin * chore: attach example * chore: update protobuf * fix: all network error * feat: device unlock * chore: optimize error * chore: change limit * feat: add skipPassphraseCheck params * chore: optimize log * chore: update protobuf * chore: release 1.0.39-alpha.1 * chore: optimize session cache * chore: optimize session cache
* feat: emulator v1 * fix: fix build target * fix: fix build target * fix: version * fix: app * fix: app * fix: build * fix: build * fix: build * fix: delete cname * fix: route * fix: deploy * fix: deploy * fix: upgrade version * fix: postinstall * fix: webpack * fix: build * fix: index.html * fix: index.html * fix: index.html * fix: index.html * fix: fix workflow * fix: fix workflow * fix: fix workflow * fix: fix workflow * fix: delte cname * fix: delte cname * fix: delte cname * fix: delte cname * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: buffer * fix: router * feat: optimize routing with webpack historyApiFallback * fix: router * fix: animation * fix: cnmae * fix: cnmae * fix: 404 index.html * fix: 404 index.html * fix: 404 index.html * fix: url * Fix root path redirect: redirect to /expo-example/api-payload instead of /expo-example/ * Fix expo-example routing: ensure all routes include /expo-example/ prefix by updating React Navigation linking config * fix: url * Move version info to header: display commit SHA and date in YYYYMMDD format next to Hardware Example title * fix: icon * fix: output * fix: fix env * chore: optimize UI * fix: build error * fix: optimize bundle * fix: fix build * fix: fix build * fix: fix cjs * fix: delete unuse shim plugin * fix: allow sourcemap * fix: fix bundle * fix: build * fix: build * fix: base * fix: rollup * fix: use webpack & udpate route * fix: build * fix: fix env * fix: fix icon * fix: fix emulator UI * fix: add kabr * fix: png * fix: optimize UI * fix: fix search * fix: fix short curt * fix: fix short cut url * fix: optimize data * fix: optimize chain * fix: optimize log * fix: optimize UI * fix: optimize UI * fix: optimize UI * fix: optimize i18n * fix: optimize i18n * fix: optimize config * fix: optimize method registor * fix: fix parameter * fix: xrp algo * fix: fix chain param * fix: fix i18n * fix: optimize md * fix: update version * fix: CommandPalette * fix: fixdHeight * fix: store * Revert "fix: store" This reverts commit e40c181. * Reapply "fix: store" This reverts commit db12f7d. * Revert "Reapply "fix: store"" This reverts commit 44a349d. * fix: store * fix: fix store * fix: fix ui * fix: fix ui * fix: ui * fix: ui * feat: add entry * fix: optimize md * fix: ui * fix: optimize md * fix: ui * fix: brige status * fix: passphraseState * fix: passphraseState * fix: delete unuse md * fix: rename example v2 * fix: build error * fix: rename playground * fix: package assets * fix: build error * fix: delete unuse webpack * fix: optimize log * fix: optimize emulator * fix: constant * fix: optimize UI * fix: jump * fix: fix firmware update error * fix: delete claude local config * fix: git ignore * fix: add ignore coderabbit * fix: add ignore coderabbit * fix: lint * fix: delete grep ; * fix: prefixes * fix: node 20 * fix: eslint * Feat/attach pin (#501) * feat: attach to pin * chore: attach example * chore: update protobuf * fix: all network error * feat: device unlock * chore: optimize error * chore: change limit * feat: add skipPassphraseCheck params * chore: optimize log * chore: update protobuf * chore: release 1.0.39-alpha.1 * chore: optimize session cache * chore: optimize session cache * fix: yarn lock * fix: eslint --------- Co-authored-by: ByteZhang <ByteZhang@protonmail.com>
4 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores