chore(monorepo): remove dependency on connect in components and produ… #17153
Conversation
WalkthroughThis pull request refactors the project’s dependency and import structure across multiple packages and modules. The changes primarily replace imports of entities like ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx (2)
23-23: Simplify the grid layout configuration.The ternary operator in
minmax(${$length > 1 ? '1px' : '6px'}, 6px)is redundant since this CSS block only executes when$length > 1.- grid-template-columns: repeat(auto-fit, minmax(${$length > 1 ? '1px' : '6px'}, 6px)); + grid-template-columns: repeat(auto-fit, minmax(1px, 6px));
55-63: Add error handling and type narrowing.Consider adding error handling for invalid inputs and type narrowing for optional fields:
{visibleTokens.map(token => ( + // Ensure contract address exists + token.contract ? ( <AssetLogo key={token.contract} size={20} coingeckoId={coingeckoId ?? ''} contractAddress={getContractAddressForNetworkSymbol(symbol, token.contract)} - placeholder={token.symbol?.toUpperCase() ?? ''} + placeholder={token.symbol ? token.symbol.toUpperCase() : ''} placeholderWithTooltip={false} /> + ) : null ))}packages/device-utils/src/types.ts (2)
3-11: Consider adding JSDoc comments for device models.While the enum values are self-explanatory for those familiar with Trezor devices, adding documentation would help new developers understand what each model represents.
export enum DeviceModelInternal { + /** Trezor One with Bitcoin-only firmware */ T1B1 = 'T1B1', + /** Trezor T with regular firmware */ T2T1 = 'T2T1', + /** Trezor T with Bitcoin-only firmware */ T2B1 = 'T2B1', + /** Trezor Safe 3 with Bitcoin-only firmware */ T3B1 = 'T3B1', + /** Trezor Safe 3 with regular firmware */ T3T1 = 'T3T1', + /** Trezor Safe 3 wireless variant */ T3W1 = 'T3W1', + /** Unknown device model */ UNKNOWN = 'UNKNOWN', }
20-41: Consider grouping related version fields.The features object has multiple version-related fields that could be grouped for better organization.
export type PartialDevice = { features?: { bootloader_hash?: string; - major_version?: number; - minor_version?: number; - patch_version?: number; - - fw_major?: number; - fw_minor?: number; - fw_patch?: number; + version?: { + major?: number; + minor?: number; + patch?: number; + }; + firmware?: { + major?: number; + minor?: number; + patch?: number; + }; firmwareType?: FirmwareType; revision?: string; bootloader_mode?: boolean; initialize?: boolean; no_backup?: boolean; unit_btconly?: boolean; }; };packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx (1)
67-67: Consider using a more specific type for deviceBackup.The type change from
Features['backup_type'] | nulltostring | nullmakes the type less specific. Since this prop is used to check for specific values (e.g.,deviceBackup === 'Bip39'), consider using a union type to ensure type safety:- deviceBackup?: string | null; + deviceBackup?: 'Bip39' | null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(0 hunks)packages/device-utils/src/firmwareUtils.ts(1 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/src/messages-schema.ts(1 hunks)packages/suite-desktop-core/tsconfig.lib.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/package.json
✅ Files skipped from review due to trivial changes (10)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: build
- GitHub Check: transport-e2e-test
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (17)
packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx (1)
5-5: LGTM! Import change aligns with PR objectives.The change successfully removes the dependency on
@trezor/connectby importing theTokenInfotype from@trezor/blockchain-link-types. The explicittypeimport is a good TypeScript practice.packages/device-utils/src/index.ts (1)
5-5: LGTM! Export statement aligns with PR objectives.The addition of
export * from './types';supports the centralization of device-related types in the device-utils package, which is a key part of removing the connect dependency.packages/device-utils/src/types.ts (2)
1-1: LGTM! Well-defined version string type.The template literal type ensures version strings follow the correct format of three dot-separated numbers.
13-16: LGTM! Clear firmware type enum.The enum values accurately represent the two firmware variants.
packages/connect/src/types/firmware.ts (1)
1-1: LGTM! Import aligns with type centralization.Importing
VersionArrayfrom device-utils instead of defining it locally reduces duplication and centralizes type definitions.packages/connect/src/types/device.ts (1)
1-1: LGTM! Import aligns with type centralization.Importing
FirmwareTypefrom device-utils instead of defining it locally reduces duplication and centralizes type definitions.packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx (1)
8-8: LGTM!The import of
DeviceModelInternalfrom@trezor/device-utilsaligns with the PR objective of removing dependency on@trezor/connect.packages/protobuf/src/messages-schema.ts (1)
1-1: LGTM!The import of
DeviceModelInternalfrom@trezor/device-utilsand removal of the local enum aligns with the PR objective of centralizing device-related types.packages/device-utils/tsconfig.json (1)
4-4: LGTM!Removing the reference to
@trezor/connectis correct as it prevents circular dependencies and aligns with the architectural goal of makingdevice-utilsindependent.packages/connect/tsconfig.json (1)
11-11: LGTM!Adding a reference to
@trezor/device-utilscorrectly establishes the new dependency relationship and maintains proper dependency direction.packages/product-components/tsconfig.json (1)
15-34: Updated References Array Aligns with PR ObjectivesThe references array has been updated to include the new dependency reference
"../blockchain-link-types"(line 25) and no longer includes the"../connect"reference. This change correctly aligns the tsconfig with the intended removal of the obsolete connect dependency in product-components.packages/components/tsconfig.json (1)
29-30: TSConfig Path Reference UpdatedThe reference for
"device-utils"(line 30) has been added as a replacement for the removed"connect"dependency. This update improves the modularity of the components package and is in line with the PR objectives.packages/connect/tsconfig.lib.json (1)
27-29: Added Device-Utils Reference in ConnectA new reference to
"../device-utils"(lines 27–29) has been introduced. This ensures that the connect package now leverages device-related utilities from the updated module. Please confirm that all device-related types and functionality now originate from@trezor/device-utils.packages/suite-desktop-core/tsconfig.lib.json (1)
8-74: Extensive References Array Update – Verify Connect DependencyThe references array has been significantly extended to include many new modules (e.g. message-system, sentry, suite-types, etc.), which should enhance cross-package type-checking and integration. One point to note: the reference to
"../connect"(present around lines 29–32) remains in the configuration. Given that the PR’s main focus was on removing the connect dependency in components and product-components, please verify that its retention in suite-desktop-core is intentional and that no unintended dependencies persist.packages/product-components/package.json (1)
22-22: Dependencies Updated CorrectlyThe dependency on
@trezor/blockchain-link-typeshas been added (line 22) and the obsolete@trezor/connectdependency has been removed. This update is consistent with the broader effort to eliminate the connect dependency and streamline the module imports. Ensure all affected import statements in the codebase have been updated accordingly.packages/components/package.json (1)
29-29: New Device Utils Dependency Added
The new dependency on@trezor/device-utilshas been correctly added to replace the removed@trezor/connectdependency. This update aligns with the PR objectives by reducing unnecessary coupling and increasing modularity.packages/connect/package.json (1)
89-89: Added Device Utils Dependency in Connect Package
The addition of@trezor/device-utilsin the dependencies ensures that device-related functionalities and type definitions are centralized. This change supports the broader effort to shift from@trezor/connectand enhance consistency across the codebase.
|
🚀 Expo preview is ready!
|
692bf4b to
f6b52f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/device-utils/src/customTypes.ts (1)
1-9: LGTM! Consider adding JSDoc comments.The
DeviceModelInternalenum is well-structured with clear naming. Consider adding JSDoc comments to document:
- The purpose of this enum
- What each device model represents (T1B1, T2T1, etc.)
- When UNKNOWN is used
packages/device-utils/package.json (1)
9-9: New "build:lib" Script AdditionThe new
"build:lib"script successfully chains the commands to clean thelibdirectory, build the TypeScript project withtsconfig.lib.json, and update import statements using the customreplace-imports.shscript. Verify that the relative path (../../scripts/replace-imports.sh) is correct within the monorepo structure and that the script has the necessary execution permissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (45)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(2 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/data/models.ts(1 hunks)packages/connect/src/device/__tests__/checkFirmwareRevision.test.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(2 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/customTypes.ts(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/modelUtils.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/__fixtures__/modelUtils.ts(1 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/messages.json(2 hunks)packages/protobuf/scripts/protobuf-types.ts(1 hunks)packages/protobuf/src/index.ts(0 hunks)packages/protobuf/src/messages-schema.ts(1 hunks)packages/protobuf/src/messages.ts(5 hunks)packages/suite-desktop-core/tsconfig.lib.json(1 hunks)submodules/trezor-common(1 hunks)
💤 Files with no reviewable changes (1)
- packages/protobuf/src/index.ts
✅ Files skipped from review due to trivial changes (9)
- packages/connect/src/data/models.ts
- packages/connect/src/device/tests/checkFirmwareRevision.test.ts
- packages/device-utils/tests/fixtures/modelUtils.ts
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/device-utils/tsconfig.lib.json
- packages/connect/src/events/device.ts
- packages/device-utils/src/modelUtils.ts
- submodules/trezor-common
- packages/connect/src/data/firmwareInfo.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- packages/device-utils/tsconfig.json
- packages/connect/tsconfig.lib.json
- packages/connect/tsconfig.json
- packages/components/package.json
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/connect/package.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/device-utils/src/firmwareUtils.ts
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/components/tsconfig.json
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/src/types/firmware.ts
- packages/product-components/tsconfig.json
- packages/product-components/package.json
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/protobuf/src/messages-schema.ts
- packages/suite-desktop-core/tsconfig.lib.json
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: prepare_android_test_app
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: transport-e2e-test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: test
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (13)
packages/protobuf/src/messages.ts (1)
1781-1783:❓ Verification inconclusive
Approve the type change but verify impact.
The change makes the
macproperty required inUnlockedPathRequestresponses, which improves type safety by ensuring the property is always present. However, this is a breaking change that could affect code expectingmacto be optional.Run the following script to find any code that might be affected by this change:
🏁 Script executed:
#!/bin/bash # Description: Find code that might be affected by the UnlockedPathRequest.mac change # Search for UnlockedPathRequest usage rg -A 5 "UnlockedPathRequest" # Search for potential destructuring of the mac property rg "(?:\{[^}]*mac[\s,}]|\.mac\b)"Length of output: 4112
Type Change Approval – Please Verify Downstream Usage
The change to make the
macproperty inUnlockedPathRequestrequired improves type safety since responses will now always include this field. However, since some parts of the code (e.g., inpackages/connect/src/device/DeviceCommands.tsand related usage inpackages/protobuf/messages.jsonandpackages/protobuf/src/messages-schema.ts) may have been written with the assumption thatmaccould be optional, this constitutes a breaking change. Please verify that all consumers ofUnlockedPathRequestcorrectly handle the now-mandatorymacfield.
- File Affected:
packages/protobuf/src/messages.ts(line 1782)- Impact: The response type now enforces the presence of
mac; ensure that downstream code (e.g., inDeviceCommands.tsandunlockPath.ts) does not assume its absence.- Action: Double-check how
UnlockedPathRequestis used across the codebase to prevent any runtime issues due to the required property.packages/protobuf/messages.json (2)
2964-2971: New Field Added: "return_empty_state" in DebugLinkGetStateThis change introduces a new boolean field
"return_empty_state"with ID 4 and a default value offalse. It provides additional control for state retrieval. Please verify that the documentation and corresponding TypeScript schema (in messages-schema.ts) have been updated accordingly so that downstream consumers can correctly interpret and use this field.
5127-5135: Updated Field Requirement: "mac" Field in UnlockedPathRequestThe
"mac"field in theUnlockedPathRequestmessage now explicitly includes"rule": "required", enforcing that a MAC value must be provided. Ensure that all clients sending this message have been updated to provide a valid MAC. It’s also recommended to double-check that the change is consistently reflected in the TypeScript schema and that any related tests are updated.packages/device-utils/src/index.ts (1)
5-6: LGTM! The new exports align with the PR objective.The addition of these exports from
typesandcustomTypesmodules supports the goal of moving type definitions from@trezor/connectto@trezor/device-utils.packages/connect/src/device/calculateRevisionForDevice.ts (1)
1-1: LGTM! Import change aligns with PR objective.The change to import
VersionArrayfrom@trezor/device-utilssupports the goal of centralizing type definitions while preserving the existing functionality.packages/connect/src/types/api/firmwareUpdate.ts (1)
1-1: LGTM! Import change aligns with PR objective.The change to import
VersionArrayfrom@trezor/device-utilssupports the goal of centralizing type definitions while maintaining the existing type structure.packages/connect/src/types/index.ts (1)
46-46: LGTM! Export statement aligns with PR objectives.The export of
FirmwareTypeandVersionArrayfrom@trezor/device-utilsaligns with the PR's goal of reducing dependency on connect by centralizing these types in the device-utils package.packages/device-utils/src/types.ts (3)
1-8: LGTM! Well-structured type definitions.The template literal type
FirmwareVersionString, enumFirmwareType, and tuple typeVersionArrayprovide strong type safety and clear semantics for version handling.
10-60: LGTM! Comprehensive version and bootloader state handling.The
FeaturesNarrowingtype union provides thorough type safety for different device states and version combinations.
62-62:❓ Verification inconclusive
Address the TODO comment regarding code duplication.
The comment indicates potential code duplication with the protobuf package. Consider either:
- Moving these types to the protobuf package
- Creating a shared package for common types
- Documenting why the duplication is necessary
🏁 Script executed:
#!/bin/bash # Check if these types exist in the protobuf package rg -A 5 "type PartialDevice" packages/protobuf/src/messagesLength of output: 132
Attention: Code Duplication in Type Definitions
The TODO comment notes that these types were copy-pasted from
packages/protobuf/src/messages, but our search suggests that the expected file path doesn’t exist. Please verify whether duplicate types (such asPartialDevice) actually reside in the protobuf package—or if the reference is outdated. Based on your findings, consider the following actions:
- If the duplicate types are maintained in an updated location within the protobuf package, refactor by moving these types there.
- Alternatively, create a shared package for common types.
- If the duplication is intentional, document the rationale clearly.
packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts (1)
1-1: LGTM! Import statement updated correctly.The import of
VersionArrayfrom@trezor/device-utilsaligns with the PR's goal of centralizing types while maintaining the function's robust version comparison and error handling.packages/connect/src/types/device.ts (1)
1-1: LGTM! Type imports and exports updated correctly.The imports of
FeaturesNarrowingandFirmwareType, and the export ofDeviceModelInternalfrom@trezor/device-utilsalign with the PR's goal of centralizing device-related types.Also applies to: 151-151
packages/device-utils/package.json (1)
7-8: Dependency Section Clean-up ValidatedThe absence of a
"dependencies"section indicates that the dependency on@trezor/connecthas been removed as per the PR objectives. Please double-check that no module in the package still relies on it.
224f1da to
ad1dbc8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/suite-desktop-core/tsconfig.scripts.json (1)
27-34: Revisit 'connect' DependenciesThe references for
"path": "../connect"and"path": "../connect-common"are still present. Given the PR objective of removing the dependency on the "connect" module from components, please confirm whether these references should be removed or updated (for example, replacing them with references todevice-utils) to align with the updated dependency policy.packages/device-utils/src/types.ts (1)
62-62: Address TODO comment in future work.The TODO indicates this code was copied from the protobuf package. Consider creating a follow-up task to address this duplication in a future PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/data/models.ts(1 hunks)packages/connect/src/device/__tests__/checkFirmwareRevision.test.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(2 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/customTypes.ts(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/modelUtils.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/__fixtures__/modelUtils.ts(1 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/scripts/protobuf-types.ts(1 hunks)packages/protobuf/src/index.ts(0 hunks)packages/suite-desktop-core/tsconfig.scripts.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/protobuf/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/device-utils/tsconfig.lib.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/connect/src/data/firmwareInfo.ts
- packages/connect/tsconfig.json
- packages/connect/src/device/tests/checkFirmwareRevision.test.ts
- packages/connect/tsconfig.lib.json
- packages/device-utils/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/connect/package.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/device-utils/src/customTypes.ts
- packages/connect/src/data/models.ts
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/components/tsconfig.json
- packages/connect/src/events/device.ts
- packages/product-components/tsconfig.json
- packages/connect/src/types/index.ts
- packages/protobuf/scripts/protobuf-types.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/connect/src/types/firmware.ts
- packages/device-utils/src/modelUtils.ts
- packages/device-utils/tests/fixtures/modelUtils.ts
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/package.json
- packages/device-utils/package.json
- packages/components/package.json
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/device-utils/src/index.ts
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (8)
packages/suite-desktop-core/tsconfig.scripts.json (2)
8-25: New Suite-Common References AddedThe newly added references to various suite-common modules (message-system, sentry, suite-constants, suite-types, suite-utils, and wallet-config) enhance module resolution for the scripts. Verify that each added module is indeed needed by the scripts in this directory.
35-73: Overall References VerificationThe remaining references (env-utils, eslint, ipc-proxy, node-utils, request-manager, suite, suite-desktop-api, transport, transport-bridge, urls, utils, trezor-user-env-link, and type-utils) look consistent with the intended modular architecture. Please ensure that each of these modules is correctly configured in your monorepo and that no obsolete or redundant dependency remains.
packages/connect/src/types/device.ts (3)
1-1: Well-organized dependency migration.Nice job importing
FeaturesNarrowingandFirmwareTypefrom the dedicated@trezor/device-utilspackage instead of defining them locally. This aligns with the PR objective of removing dependencies on connect.
158-158: Good package reference update.Updating the import source for
DeviceModelInternalfrom '@trezor/protobuf' to '@trezor/device-utils' maintains consistency with the other device-related type imports.
162-162:✅ Verification successful
Verify type compatibility.
Line 162 uses the imported
FeaturesNarrowingtype. Make sure the implementation indevice-utilsis fully compatible with how it's being used here.
🏁 Script executed:
#!/bin/bash # Find all usages of StrictFeatures to ensure compatibility rg "StrictFeatures" --type tsLength of output: 1555
Type Consistency Verified
The type alias in
packages/connect/src/types/device.ts—declared asexport type StrictFeatures = Features & FeaturesNarrowing;—is used throughout the codebase (e.g., infirmwareUtils.tsvia theisStrictFeaturestype guard and several tests). The shell script output confirms that the usages ofStrictFeaturesalign with the expected pattern, and there’s no evidence of incompatibility with theFeaturesNarrowingimplementation. The integration looks sound, and no further adjustments are needed.packages/device-utils/src/types.ts (3)
1-8: Good type definitions with proper constraints.The new type definitions for
FirmwareVersionString,FirmwareTypeenum, andVersionArrayare well-structured and provide strong typing:
- The template literal type for version strings ensures proper format
- The enum provides type safety for firmware types
- The tuple type for version arrays ensures fixed length
10-60: Comprehensive type definition for device features.The
FeaturesNarrowingtype is well-structured with union types that handle different device states and versions. This provides strong type safety when working with device features across various states.
63-80: Enhanced PartialDevice type with better structured features.The modifications to
PartialDeviceimprove type safety by:
- Using the
FirmwareTypeenum instead of string literals- Expanding the
featuresproperty with comprehensive type informationThis provides better IntelliSense support and compile-time checking.
ad1dbc8 to
ab35037
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/connect/src/types/device.ts (1)
158-158: 💡 Verification agent🧩 Analysis chain
Import source for DeviceModelInternal changed.
The export source has been updated from
@trezor/protobufto@trezor/device-utils, which aligns with the PR objective of restructuring dependencies.
🏁 Script executed:
#!/bin/bash # Verify that DeviceModelInternal is now exported properly from device-utils # and that all imports in the codebase have been updated # Check if DeviceModelInternal is exported from device-utils echo "Checking if DeviceModelInternal is exported from device-utils:" rg "export .* DeviceModelInternal" packages/device-utils/ # Check for any remaining imports from protobuf that might need updating echo "Checking for any remaining imports of DeviceModelInternal from protobuf:" rg "import .* DeviceModelInternal .* from .*protobuf" --type tsLength of output: 1185
Action Required: Update DeviceModelInternal Import/Export References
It appears that the change to use
@trezor/device-utilshas not been fully applied. Specifically:
- File Update Needed: In
packages/connect/src/types/device.ts, the export still reads from@trezor/protobufinstead of the intended@trezor/device-utils.- Additional References: Several files are still importing
DeviceModelInternalfrom@trezor/protobuf:
packages/components/src/components/animations/DeviceAnimation.tsxpackages/components/src/components/animations/LottieAnimation.tsxpackages/connect/src/data/models.tspackages/connect/src/data/firmwareInfo.tspackages/connect/src/device/__tests__/checkFirmwareRevision.test.tspackages/connect/src/types/firmware.tspackages/suite/src/utils/device/__tests__/modelUtils.test.tsPlease update the export in
packages/connect/src/types/device.tsto:export { DeviceModelInternal } from '@trezor/device-utils';and refactor all the above import statements accordingly. This will ensure consistency with the PR's objective of restructuring dependencies.
🧹 Nitpick comments (1)
packages/suite/package.json (1)
74-74: New Dependency Addition: @trezor/protobuf
The addition of"@trezor/protobuf": "workspace:*"aligns well with the PR objective of transitioning away from@trezor/connect. This new dependency should support the updated imports in components (such as for device model definitions) as outlined in the PR summary.Please verify that all usage in the suite and its sub-packages has been updated accordingly so that there is no residual reliance on
@trezor/connect(e.g., on lines 66-67). If@trezor/connectand@trezor/connect-webare no longer needed, consider cleaning them up to further reduce unnecessary dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite-desktop-core/tsconfig.scripts.json(1 hunks)packages/suite/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (32)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/connect/package.json
- packages/connect/tsconfig.lib.json
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/device-utils/tsconfig.json
- packages/components/package.json
- packages/device-utils/src/index.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/connect/src/types/index.ts
- packages/device-utils/tsconfig.lib.json
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/connect/tsconfig.json
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/connect/src/types/firmware.ts
- packages/components/tsconfig.json
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/connect/src/data/firmwareInfo.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/package.json
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/connect/src/events/device.ts
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/device-utils/package.json
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/product-components/tsconfig.json
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/suite-desktop-core/tsconfig.scripts.json
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: prepare_android_test_app
- GitHub Check: transport-e2e-test
- GitHub Check: test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/connect/src/types/device.ts (2)
1-1: Import path modified correctly.The imports are now sourced from
@trezor/device-utils, which aligns with the PR's objective of removing dependencies on connect in components and moving shared types to a more appropriate package.
162-162: Type reference updated successfully.The
StrictFeaturestype now properly references the importedFeaturesNarrowingtype instead of a local definition.packages/device-utils/src/types.ts (3)
1-8: Well-structured type definitions for firmware versioning.The
FirmwareVersionString,FirmwareTypeenum, andVersionArraytype provide a clear, strongly-typed foundation for handling firmware information across the codebase.
10-60: Comprehensive type narrowing for device features.The
FeaturesNarrowingtype definition provides precise discrimination between different device states based on version and bootloader combinations. This enhances type safety throughout the application when handling different device states.
63-80: Enhanced PartialDevice type with detailed feature properties.The
PartialDevicetype has been improved with:
- More specific typing using the
FirmwareTypeenum- Expanded
featuresobject with comprehensive version information- Clear boolean flags for device states like
initializedandbootloader_modeThis enhances type safety when working with device objects.
|
|
||
| // todo: this is copy-pasted from packages/protobuf/src/messages |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address TODO comment regarding copied code.
There's a TODO indicating the code was copy-pasted from another location. This should be addressed to prevent code duplication and potential maintenance issues.
🏁 Script executed:
#!/bin/bash
# Check to see if this code is duplicated from the source mentioned in the TODO
echo "Checking for the original source of the copied code:"
rg --type ts "PartialDevice" packages/protobuf/src/messages
echo "Checking for other instances of similar device types:"
rg --type ts "firmwareType.*FirmwareType" packages/Length of output: 2665
Action Required: Update or Remove Outdated TODO Comment
- The TODO in
packages/device-utils/src/types.tsclaims the code was copied frompackages/protobuf/src/messages, but our search indicates that the referenced directory/file no longer exists. - Please verify if the intended original source has been relocated. If so, update the comment and leverage an import. Alternatively, if the duplication is intentional and the referenced file is obsolete, remove the TODO and document the rationale for keeping the code copy.
ab35037 to
c275510
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/device-utils/src/types.ts (1)
10-60: Consider adding documentation for the complex union typeThis is a detailed type definition that handles various device states and version combinations. While the structure is good, adding JSDoc comments would help other developers understand:
- The purpose of this type
- When each variation is used
- The relationships between bootloader mode, firmware presence, and version numbers
Example documentation:
+/** + * Represents various device states with their corresponding version information. + * - Major version 2 with bootloader mode and no firmware + * - Major version 2 with null bootloader mode and firmware info + * - Major version 2 with bootloader mode and firmware present + * - Major version 1 with bootloader mode and no firmware + * - Major version 1 with bootloader mode and firmware present + */ export type FeaturesNarrowing = | { // ...packages/suite-desktop-core/tsconfig.scripts.json (1)
8-73: Review New and Existing Reference EntriesThe references array has been significantly expanded to include many modules (e.g., message-system, sentry, suite-constants, suite-types, suite-utils, and more). This improves module resolution and integration across the project. However, given the PR’s aim to remove dependency on connect in components and product-components, please verify that the entries for
"../connect"and"../connect-common"are still required within this configuration. Additionally, consider whether organizing these paths alphabetically or grouping them by related functionality might improve maintainability and readability in future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite-desktop-core/tsconfig.scripts.json(1 hunks)packages/suite/package.json(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/suite/src/views/recovery/index.tsx
🚧 Files skipped from review as they are similar to previous changes (33)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/suite/package.json
- packages/connect/tsconfig.json
- packages/connect/tsconfig.lib.json
- packages/device-utils/tsconfig.lib.json
- packages/device-utils/tsconfig.json
- packages/device-utils/src/index.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/connect/src/types/firmware.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/components/tsconfig.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/connect/src/events/device.ts
- packages/connect/src/types/index.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/product-components/package.json
- packages/connect/src/data/firmwareInfo.ts
- packages/device-utils/package.json
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/connect/package.json
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/product-components/tsconfig.json
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/connect/src/types/device.ts
- packages/components/package.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: build-web
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (11)
packages/suite/src/utils/device/__tests__/modelUtils.test.ts (5)
1-1: Good refactoring: Correctly updated import source for dependency reduction.This import change aligns with the PR objective of removing dependency on the
connectmodule by now sourcingDeviceModelInternalfrom@trezor/protobufinstead of@trezor/connect.
3-4: Clean implementation of utility import for testing.The import of
pickByDeviceModelis properly referenced from the local module path, maintaining good encapsulation.
5-9: Good practice: Converted exported constant to local scope.Removing the
exportkeyword from theOPTIONSconstant improves encapsulation by keeping test-specific variables local to the test file where they're used.
11-32: Good practice: Converted exported fixtures to local scope.Removing the
exportkeyword from thefixturesarray improves encapsulation by keeping test-specific data local to the test file. The test cases provide comprehensive coverage of various scenarios for thepickByDeviceModelfunction.
34-40: Well-structured test suite with comprehensive test coverage.The test suite for
pickByDeviceModelis properly implemented:
- Uses a parameterized approach to iterate through test fixtures
- Tests all important scenarios (successful matching, null values, undefined input, non-matching models)
- Clear and descriptive test cases with specific expectations
This ensures the utility function works correctly across different device models and edge cases.
packages/device-utils/src/types.ts (5)
1-1: Well-defined type using template literalGood use of TypeScript's template literal type to enforce a specific format for firmware version strings.
3-6: Proper enum definition for firmware typesThe enum provides clear, descriptive values for the two firmware types, which will improve type safety across the codebase.
8-8: Well-typed version array representationGood decision to explicitly define this tuple type for version arrays, which will ensure consistency when handling version information.
62-62: Address TODO comment regarding copied codeThe TODO comment indicates that this code was copy-pasted from another location. Based on previous review comments, the referenced source might no longer exist.
#!/bin/bash # Check if the referenced path exists and if this code is still duplicated echo "Checking if the referenced path exists:" if [ -d "packages/protobuf/src/messages" ]; then echo "Directory exists, checking for similar type definitions:" rg --type ts "PartialDevice" packages/protobuf/src/messages else echo "Directory does not exist, checking where PartialDevice might be defined now:" rg --type ts "type\s+PartialDevice" packages/ fi echo "Checking for other imports of this type:" rg --type ts "import.*PartialDevice" packages/
63-80:✅ Verification successful
Type definition updates for better integration
The
PartialDevicetype has been properly updated to:
- Use the new
FirmwareTypeenum for improved type safety- Include detailed feature properties for better type checking
This aligns with the PR objective of removing dependencies on the connect package while maintaining necessary type information.
🏁 Script executed:
#!/bin/bash # Check if this type is used consistently across the codebase echo "Checking usage of PartialDevice:" rg --type ts "PartialDevice" packages/ | grep -v "packages/device-utils/src/types.ts" echo "Checking for imports of FirmwareType:" rg --type ts "import.*FirmwareType" packages/ | grep -v "packages/device-utils/src/types.ts"Length of output: 3368
Device-Utils Type Definition Update Verified
The updated
PartialDevicetype inpackages/device-utils/src/types.tsis correctly updated and consistently used across relevant files (e.g.,bootloaderUtils.ts,modeUtils.ts, andfirmwareUtils.ts). The new use of theFirmwareTypeenum and detailed feature properties has been verified through our search, and it aligns with the PR goal of removing the dependency on the connect package within device-utils. Note that while some suite files still importFirmwareTypefrom@trezor/connect, this review focuses solely on thedevice-utilspackage changes.packages/suite/tsconfig.json (1)
105-105: Added New Protobuf ReferenceThe new reference
{ "path": "../protobuf" }is added to facilitate the use of the@trezor/protobufmodule, which aligns with the strategic shift away from the legacy connect dependency in certain parts of the codebase.
c275510 to
61696b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/device-utils/src/types.ts (2)
10-60: Comprehensive type definition for device features.The
FeaturesNarrowingtype provides detailed type safety by explicitly modeling the different possible states of device features. The union types effectively capture various combinations of version numbers and bootloader states.Consider adding documentation comments to explain the purpose of each union variant, as the different combinations of properties and their values might not be immediately obvious to developers unfamiliar with the device firmware architecture.
export type FeaturesNarrowing = + /** + * Device in bootloader mode with major version 2, no firmware installed + */ | { major_version: 2; minor_version: number;
66-79: Comprehensive device features definition.The
featuresproperty now includes a detailed structure that covers all essential device characteristics, including version information, bootloader state, and device flags.Consider adding JSDoc comments to document the meaning and possible values of some of the less obvious properties like
revision,bootloader_hash, andno_backupto improve maintainability.features?: { + /** Device's major version number */ major_version: number; minor_version: number; patch_version: number; + /** Whether the device is in bootloader mode */ bootloader_mode: boolean | null; + /** Whether the device has been initialized with a seed */ initialized: boolean | null; + /** Revision of the hardware */ revision: string | null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite-desktop-core/tsconfig.scripts.json(1 hunks)packages/suite/package.json(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (38)
- packages/suite/tsconfig.json
- packages/device-utils/tsconfig.lib.json
- packages/device-utils/tsconfig.json
- packages/connect/tsconfig.lib.json
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/suite/package.json
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/connect/src/events/device.ts
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/connect/package.json
- packages/components/tsconfig.json
- packages/suite/src/views/recovery/index.tsx
- packages/device-utils/package.json
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/connect/src/types/index.ts
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/components/package.json
- packages/device-utils/src/index.ts
- packages/product-components/package.json
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/product-components/tsconfig.json
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/connect/src/data/firmwareInfo.ts
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/connect/src/types/firmware.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/connect/src/types/device.ts
- packages/suite-desktop-core/tsconfig.scripts.json
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-web
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: transport-e2e-test
- GitHub Check: test
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/device-utils/src/types.ts (5)
1-1: Well-defined string type for firmware versioning.Using a template literal type for
FirmwareVersionStringprovides strong type safety for version strings, ensuring they follow the standardmajor.minor.patchformat.
3-6: Good enumeration of firmware types.The
FirmwareTypeenum clearly defines the two possible firmware variants with descriptive string values. This adds clarity and type safety when working with firmware types.
8-8: Well-typed version tuple.The
VersionArraytype properly ensures that version arrays contain exactly three numeric elements, corresponding to major, minor, and patch versions.
62-62: Action Required: Update or Remove Outdated TODO CommentBased on past review comments, the TODO refers to a file path that may no longer exist.
#!/bin/bash # Check if the referenced source file/directory still exists echo "Checking if the referenced directory exists:" ls -la packages/protobuf/src/messages 2>/dev/null || echo "Directory doesn't exist" echo "Searching for potential new location of this code:" rg --type ts "PartialDevice.*firmwareType.*features" --no-filename packages/
64-64: Type consistency with firmware enumeration.Using the new
FirmwareTypeenum for thefirmwareTypeproperty ensures type consistency throughout the codebase.
61696b1 to
b1bb4e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/device-utils/src/types.ts (2)
10-60: Add documentation to explain the purpose of each union variant.The
FeaturesNarrowingtype is well-structured but lacks documentation explaining what each variant represents. Given its complexity, adding JSDoc comments would significantly improve maintainability.+/** + * Represents different possible configurations of device features based on firmware version and bootloader state. + * Used to narrow down device types for more specific type checking. + */ export type FeaturesNarrowing = + /** + * Represents a device in bootloader mode with firmware v2.x.x without firmware present + */ | { major_version: 2; minor_version: number; patch_version: number; fw_major: null; fw_minor: null; fw_patch: null; bootloader_mode: true; firmware_present: false; } + /** + * Represents a device with firmware v2.x.x in an unknown bootloader state + */ | { major_version: 2; minor_version: number; patch_version: number; fw_major: null; fw_minor: null; fw_patch: null; bootloader_mode: null; firmware_present: null; }
66-79: Consider adding validation constraints for version numbers.The version numbers (major, minor, patch) currently accept any number. Consider adding runtime validation or more specific types to ensure these are non-negative integers within expected ranges.
features?: { - major_version: number; - minor_version: number; - patch_version: number; + major_version: number & { __brand: 'NonNegativeInteger' }; + minor_version: number & { __brand: 'NonNegativeInteger' }; + patch_version: number & { __brand: 'NonNegativeInteger' }; bootloader_mode: boolean | null; initialized: boolean | null; revision: string | null; bootloader_hash: string | null; - fw_major: number | null; - fw_minor: number | null; - fw_patch: number | null; + fw_major: (number & { __brand: 'NonNegativeInteger' }) | null; + fw_minor: (number & { __brand: 'NonNegativeInteger' }) | null; + fw_patch: (number & { __brand: 'NonNegativeInteger' }) | null; no_backup: boolean | null; unit_btconly?: boolean; };Additionally, add a type helper to enforce this constraint:
// Add this elsewhere in your codebase: export type NonNegativeInteger = number & { __brand: 'NonNegativeInteger' }; export function asNonNegativeInteger(n: number): NonNegativeInteger { if (!Number.isInteger(n) || n < 0) { throw new Error(`Expected non-negative integer, got ${n}`); } return n as NonNegativeInteger; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite-desktop-core/tsconfig.scripts.json(1 hunks)packages/suite/package.json(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (38)
- packages/connect/tsconfig.lib.json
- packages/suite/package.json
- packages/device-utils/tsconfig.lib.json
- packages/suite/tsconfig.json
- packages/components/package.json
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/connect/tsconfig.json
- packages/product-components/package.json
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/product-components/tsconfig.json
- packages/device-utils/src/index.ts
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/components/tsconfig.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/device-utils/package.json
- packages/device-utils/tsconfig.json
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/src/events/device.ts
- packages/connect/src/types/firmware.ts
- packages/suite/src/views/recovery/index.tsx
- packages/connect/src/data/firmwareInfo.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/connect/src/types/index.ts
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/connect/package.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/connect/src/types/device.ts
- packages/suite-desktop-core/tsconfig.scripts.json
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: build-web
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/device-utils/src/types.ts (5)
1-1: Well-defined type for firmware versioning.This template literal type creates a strongly-typed string representation of semantic versioning. Great use of TypeScript's template literal types to enforce the X.Y.Z format.
3-6: Clear enum definition for firmware types.The enum provides good clarity with descriptive string values that align with business domain terminology.
8-8: Concise tuple type for version components.This type definition elegantly represents semantic versioning components as a fixed-length array.
62-62: Address TODO comment regarding copied code.The TODO indicates that this code was copy-pasted from another location. This should be addressed to prevent code duplication and potential maintenance issues.
#!/bin/bash # Check if the original source exists and if there are better ways to import/reference these types # Check if the mentioned source file/directory exists echo "Checking if the source directory exists:" ls -la packages/protobuf/src/messages 2>/dev/null || echo "Source directory does not exist" # Look for similar type definitions that might be the new location echo "Searching for similar type definitions:" rg --type ts "type\s+PartialDevice" packages/
64-64: Good usage of newly defined enum type.Using the
FirmwareTypeenum for thefirmwareTypeproperty ensures type safety and consistency when working with firmware types.
b1bb4e3 to
c22f77e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/device-utils/src/types.ts (1)
10-60: Consider adding documentation for the complex union type.The
FeaturesNarrowingtype is a complex union with multiple variants representing different device states. While the implementation is technically correct, it would benefit from documentation explaining the purpose of each variant and when they occur.For example:
- When is a device in bootloader mode with firmware present vs. not present?
- What's the significance of the different major versions?
+/** + * Represents various combinations of device feature states for different firmware versions and bootloader modes. + * - Major version 2 with bootloader mode and no firmware present: Device is in bootloader without firmware + * - Major version 2 with no bootloader mode: Standard model 2 device state + * - Major version 2 with bootloader mode and firmware present: Device is in bootloader with firmware installed + * - Major version 1 with bootloader mode and no firmware present: Legacy device in bootloader without firmware + * - Major version 1 with bootloader mode and firmware present: Legacy device in bootloader with firmware installed + */ export type FeaturesNarrowing =
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite/package.json(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/device-utils/tsconfig.lib.json
- packages/suite/tsconfig.json
- packages/connect/tsconfig.lib.json
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/suite/src/views/recovery/index.tsx
- packages/connect/package.json
- packages/components/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/device-utils/tsconfig.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/components/package.json
- packages/connect/src/types/firmware.ts
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/connect/src/events/device.ts
- packages/device-utils/src/index.ts
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/connect/src/data/firmwareInfo.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/device-utils/package.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/connect/src/types/index.ts
- packages/product-components/tsconfig.json
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- packages/connect/src/types/device.ts
- packages/suite/package.json
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/product-components/package.json
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: prepare_android_test_app
- GitHub Check: EAS Update
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: test
- GitHub Check: build-web
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: transport-e2e-test
🔇 Additional comments (6)
packages/device-utils/src/types.ts (6)
1-1: Clear type definition with appropriate constraints.This template literal type provides strong typing for firmware version strings in the standard format. It ensures that version strings will always follow the
x.y.zpattern.
3-6: Well-defined enum for firmware types.The enum clearly categorizes the two possible firmware types: Bitcoin-only and Regular. This will help maintain consistency across the codebase when referencing firmware types.
8-8: Properly typed version array.This tuple type correctly represents the standard [major, minor, patch] version format, providing strong typing for version arrays throughout the application.
62-62: Action Required: Update or Remove Outdated TODO Comment.The TODO indicates this code was copied from
packages/protobuf/src/messages, but this location appears to be outdated or no longer exists.Please either:
- Update the comment with the current source location if the code is still duplicated from somewhere
- Remove the TODO if the duplication is now intentional or if the source has been fully migrated
- Consider importing the type if it exists elsewhere to avoid duplication
63-65: Good refactoring to use the new FirmwareType enum.The
firmwareTypeproperty now uses the newFirmwareTypeenum defined earlier in this file, which improves type safety and consistency.
66-79: Enhanced type definitions for device features.The
featuresproperty has been expanded with more detailed property definitions, including proper nullability for optional values. This provides better type safety when working with device features.The addition of
unit_btconlywith an optional flag is good for backward compatibility.
c22f77e to
eea4313
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/device-utils/src/types.ts (2)
10-60: Comprehensive device feature state mappingThe
FeaturesNarrowingtype effectively models the various states a device can be in based on firmware version and bootloader status. This union type should help with precise type narrowing in conditional code.However, I recommend adding a brief comment explaining the purpose of this type and the differences between each variant to improve maintainability.
+/** + * Type for narrowing device features based on firmware version and bootloader state. + * Used to provide precise typing for different device states: + * - T2 in bootloader mode without firmware + * - T2 with unspecified bootloader state + * - T2 in bootloader mode with firmware present + * - T1 in bootloader mode without firmware + * - T1 in bootloader mode with firmware present + */ export type FeaturesNarrowing =
66-79: Detailed features object structureThe detailed specification of the device features object ensures consistent typing across the codebase. This structure captures all the necessary properties for device identification and state management.
Consider adding JSDoc comments for each property to document their purpose and possible values, which would improve maintainability and developer experience.
features?: { + /** Major version of the device */ major_version: number; + /** Minor version of the device */ minor_version: number; + /** Patch version of the device */ patch_version: number; + /** Whether the device is in bootloader mode */ bootloader_mode: boolean | null; + /** Whether the device is initialized with a seed */ initialized: boolean | null; + /** Device revision string */ revision: string | null; + /** Hash of the bootloader */ bootloader_hash: string | null; + /** Firmware major version */ fw_major: number | null; + /** Firmware minor version */ fw_minor: number | null; + /** Firmware patch version */ fw_patch: number | null; + /** Whether the device has backup disabled */ no_backup: boolean | null; + /** Whether the device is Bitcoin-only */ unit_btconly?: boolean; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/package.json(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/suite/package.json(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/device-utils/tsconfig.json
- packages/suite/package.json
- packages/connect/tsconfig.lib.json
- packages/device-utils/src/index.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/connect/src/types/index.ts
- packages/connect/tsconfig.json
- packages/components/package.json
- packages/suite/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/device-utils/tsconfig.lib.json
- packages/connect/package.json
- packages/suite/src/views/recovery/index.tsx
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/product-components/package.json
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/connect/src/data/firmwareInfo.ts
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/connect/src/events/device.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/components/tsconfig.json
- packages/device-utils/package.json
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/connect/src/types/firmware.ts
- packages/connect/src/types/device.ts
- packages/product-components/tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: prepare_android_test_app
- GitHub Check: EAS Update
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: transport-e2e-test
- GitHub Check: test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/device-utils/src/types.ts (5)
1-1: Strong type definition for firmware versioningThe
FirmwareVersionStringtype using template literals provides a clear contract for firmware version formatting, ensuring consistency throughout the codebase. This is a good practice for type safety when working with semantic versioning.
3-6: Well-defined enum for firmware typesThe
FirmwareTypeenum provides strong typing and clear documentation of the available firmware variants. This helps with type checking and prevents possible typos when referencing these values elsewhere in the codebase.
8-8: Explicit version array typeThe
VersionArraytype clearly defines the structure for version numbers as a tuple of exactly three numbers. This enhances type safety and ensures consistent version handling across the codebase.
62-62: Action Required: Update or Remove Outdated TODO CommentThe TODO comment indicates the code was copied from
packages/protobuf/src/messages, but previous verification shows that the referenced location may no longer exist.#!/bin/bash # Check if the original source still exists and if this is indeed duplicate code echo "Checking if original source exists:" find packages/protobuf/src -type d -name "messages" | grep . echo "\nSearching for similar definitions in the codebase:" rg "type\s+(?:Partial)?Device\s*=\s*\{" --type ts
64-64: Using enum type for firmwareType improves type safetyChanging
firmwareTypeto use theFirmwareTypeenum instead of string literals improves type safety and aligns with the PR objective of making components more independent by centralizing these definitions in device-utils.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/protobuf/messages.json (1)
5592-5600: Review of NostrPubkey Message Definition
TheNostrPubkeymessage introduces a required fieldpubkeyof typebytes, which appropriately represents a public key. In some parts of the codebase, similar fields are namedpublic_key. It may be worthwhile to ensure that the naming is consistent across the project or intentionally distinct for Nostr.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (28)
packages/components/package.json(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/data/models.ts(1 hunks)packages/connect/src/device/__tests__/checkFirmwareRevision.test.ts(1 hunks)packages/connect/src/types/coinInfo.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/src/utils/__tests__/deviceFeaturesUtils.test.ts(1 hunks)packages/connect/src/utils/assetUtils.ts(1 hunks)packages/device-utils/src/index.ts(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/messages.json(2 hunks)packages/protobuf/scripts/protobuf-types.ts(1 hunks)packages/protobuf/src/index.ts(0 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/protobuf/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- packages/connect/src/data/models.ts
- packages/connect/src/types/coinInfo.ts
- packages/connect/src/device/tests/checkFirmwareRevision.test.ts
- packages/connect/src/utils/assetUtils.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/device-utils/src/index.ts
- packages/connect/src/types/firmware.ts
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/components/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/components/package.json
- packages/product-components/tsconfig.json
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/product-components/package.json
- packages/connect/src/data/firmwareInfo.ts
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- packages/connect/src/types/device.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: transport-e2e-test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (7)
packages/connect/src/utils/__tests__/deviceFeaturesUtils.test.ts (1)
3-3: Good refactoring of DeviceModelInternal importThe change to import
DeviceModelInternalfrom@trezor/device-utilsinstead of from the local types module aligns well with the PR objective of removing connect dependencies. This centralization of device-related types into a dedicated utility package improves code organization and maintainability.Also applies to: 6-6
packages/connect/src/types/index.ts (1)
46-46: Added export for device-utils types - Good change!This addition exports key types from
@trezor/device-utils, allowing them to be used throughout the codebase without direct dependencies on the connect module. This change directly supports the PR objective of removing dependencies on connect in components and products.packages/protobuf/scripts/protobuf-types.ts (1)
187-187: Switch to package import rather than using relative pathsThe relative path
../../device-utils/src/deviceModelInternal.tsis fragile and may break if directory structures change. Consider importing directly from the package instead.- lines.push(readPatch('../../device-utils/src/deviceModelInternal.ts'), ''); + lines.push(readPatch('@trezor/device-utils/lib/deviceModelInternal'), '');Also verify that
@trezor/device-utilsis properly declared as a dependency inpackages/protobuf/package.json.packages/protobuf/messages.json (4)
5580-5591: Review of NostrGetPubkey Message Definition
The newNostrGetPubkeymessage is defined with a single fieldaddress_nthat is marked as repeated of typeuint32with the option"packed": false. This design is consistent with similar message definitions for representing HD node paths. Please verify that the usage ofaddress_nmeets the Nostr protocol requirements.
5601-5618: Review of NostrTag Message Definition
TheNostrTagmessage is structured with a requiredkey(string), an optionalvalue(string), and a repeatedextrafield (strings) to capture multiple metadata entries. This flexible structure is suitable for diverse tagging needs. Please confirm that the intended semantics for theextrafield align with the Nostr protocol specifications.
5619-5650: Review of NostrSignEvent Message Definition
TheNostrSignEventmessage comprises the following fields:
address_n: a repeateduint32(with"packed": false) representing the derivation path,created_at: a requireduint32timestamp,kind: a requireduint32for specifying event type,tags: a repeated field of typeNostrTag, andcontent: a requiredstringfor the actual event content.This design appears to correctly capture the essential elements of a Nostr event. Please double-check that using
uint32for the timestamp and event kind is sufficient for your use case.
5651-5669: Review of NostrEventSignature Message Definition
TheNostrEventSignaturemessage defines three required fields—pubkey,id, andsignature—all of which are of typebytes. This structure is in line with expected cryptographic signature formats in the Nostr protocol and should facilitate proper validation of signed events.
b3a21ee to
585c6c6
Compare
8801e61 to
6b8de11
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
suite-native/react-native-graph/tsconfig.json (1)
7-10: Updated Reference Path in TSConfigThe reference in the
"references"section has been updated from the old path (pointing to../../suite-common/suite-utils) to the new centralized utility package at../../packages/utils. This change aligns with the PR objective of enhancing modularity and maintainability by unifying dependencies and streamlining the monorepo architecture.suite-native/navigation/package.json (1)
28-28: Ensure Consistency with New Utility DependencyThe addition of
"@trezor/utils": "workspace:*"correctly aligns with the broader effort to consolidate utility functions and remove legacy dependencies. Please verify that this new dependency is fully integrated, and consider whether"@trezor/connect"(line 25) is still required in this package in light of the removal objective.suite-native/navigation/tsconfig.json (1)
17-20: Review the References for Dependency ConsistencyA new reference to
"../../packages/utils"has been added (line 20) immediately after the unchanged"../../packages/theme"(line 19), which supports the goal of pulling in new utility functionalities. However, note that the reference to"../../packages/connect"(line 17) still exists even though the overall PR objective is to remove the dependency on connect. Please verify whether"../../packages/connect"is still required for thesuite-native/navigationmodule or if it should be removed to fully align with the intended dependency elimination.packages/device-utils/src/deviceModelInternalUtils.ts (1)
8-11: Consider removing theanytype cast in favor of more precise typing.The function uses a TypeScript type assertion
as anywhich bypasses type checking. This could potentially lead to runtime type errors if not handled carefully.Refactor by using a more precise type assertion or conditional typing:
export const getNarrowedDeviceModelInternal = <T extends DeviceModelInternal>( model: T, ): T extends DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : T => - (model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model) as any; + (model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model) as + (T extends DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : T);packages/device-utils/src/index.ts (1)
4-6: Document the public API changes.These new exports change the public API of the device-utils package. Consider documenting these changes in a CHANGELOG file to help other developers understand the API evolution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (37)
packages/components/src/components/VirtualizedList/VirtualizedList.tsx(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/src/components/buttons/buttonStyleUtils.ts(1 hunks)packages/device-utils/src/deviceModelInternalUtils.ts(1 hunks)packages/device-utils/src/index.ts(1 hunks)packages/suite/src/actions/wallet/selectedAccountActions.ts(2 hunks)packages/suite/src/components/suite/HomescreenGallery.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts(1 hunks)packages/suite/src/hooks/wallet/useSendFormCompose.ts(1 hunks)packages/suite/src/hooks/wallet/useStakeEthForm.ts(1 hunks)packages/suite/src/hooks/wallet/useUnstakeEthForm.ts(1 hunks)packages/suite/src/views/recovery/index.tsx(2 hunks)packages/utils/src/index.ts(2 hunks)packages/utils/tests/comparison.test.ts(2 hunks)suite-common/suite-utils/src/__tests__/comparison.test.ts(0 hunks)suite-common/suite-utils/src/device.ts(1 hunks)suite-common/suite-utils/src/index.ts(0 hunks)suite-common/wallet-core/src/device/deviceThunks.ts(1 hunks)suite-native/module-onboarding/package.json(1 hunks)suite-native/module-onboarding/src/screens/WelcomeScreen.tsx(1 hunks)suite-native/module-onboarding/tsconfig.json(1 hunks)suite-native/module-send/package.json(0 hunks)suite-native/module-send/src/components/SlidingFooterOverlay.tsx(1 hunks)suite-native/module-send/tsconfig.json(0 hunks)suite-native/module-trading/package.json(1 hunks)suite-native/module-trading/src/components/general/TradeableAssetButton.tsx(1 hunks)suite-native/module-trading/tsconfig.json(1 hunks)suite-native/navigation/package.json(1 hunks)suite-native/navigation/src/components/ScreenFooterGradient.tsx(1 hunks)suite-native/navigation/tsconfig.json(1 hunks)suite-native/react-native-graph/package.json(1 hunks)suite-native/react-native-graph/src/AnimatedLineGraph.tsx(1 hunks)suite-native/react-native-graph/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (4)
- suite-native/module-send/tsconfig.json
- suite-native/module-send/package.json
- suite-common/suite-utils/src/index.ts
- suite-common/suite-utils/src/tests/comparison.test.ts
✅ Files skipped from review due to trivial changes (14)
- suite-native/react-native-graph/src/AnimatedLineGraph.tsx
- suite-native/module-send/src/components/SlidingFooterOverlay.tsx
- packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts
- suite-common/wallet-core/src/device/deviceThunks.ts
- suite-native/module-trading/src/components/general/TradeableAssetButton.tsx
- packages/suite/src/hooks/wallet/useSendFormCompose.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx
- suite-native/module-onboarding/src/screens/WelcomeScreen.tsx
- packages/suite/src/hooks/wallet/useUnstakeEthForm.ts
- packages/suite/src/hooks/wallet/useStakeEthForm.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts
- suite-native/navigation/src/components/ScreenFooterGradient.tsx
- packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts
- packages/components/src/components/buttons/buttonStyleUtils.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/components/src/components/animations/DeviceAnimation.tsx
- suite-native/module-onboarding/tsconfig.json
- packages/suite/src/components/suite/HomescreenGallery.tsx
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/suite/src/views/recovery/index.tsx
- suite-common/suite-utils/src/device.ts
🔇 Additional comments (15)
suite-native/module-trading/package.json (1)
19-19: New Dependency Added: @trezor/utilsThe addition of
"@trezor/utils": "workspace:*"aligns with the broader architectural goal of reducing the dependency on the legacy@trezor/connectmodule. By adding this dependency, the module now leverages shared utility functions which improves overall modularity across the monorepo. Please ensure that its usage is consistent in the subsequent component changes.suite-native/react-native-graph/package.json (1)
15-15: Dependency Update to @trezor/utilsThis update replaces the previous utility dependency with "@trezor/utils": "workspace:*". It's aligned with the PR objective to remove reliance on older dependencies and centralize utilities for better modularity. Please ensure that all references in the codebase have been updated accordingly to reflect this change.
suite-native/module-onboarding/package.json (3)
21-21: Introduce@trezor/device-utilsDependencyThe addition of
"@trezor/device-utils": "workspace:*",meets the PR objective of eliminating the dependency on@trezor/connectand aligning the module with the new device-related utilities. Please verify that all code references for connect-related functionality are updated accordingly.
24-24: Add@trezor/utilsDependencyThe introduction of
"@trezor/utils": "workspace:*",supports the move towards a more modular and decoupled architecture. Ensure that all necessary utilities previously tied to other dependencies are now correctly imported from this new package.
1-33: Overall Package Configuration CheckThe overall JSON structure and dependency listings are well-formed and consistent with the monorepo’s workspace strategy. The removal of the
@trezor/connectdependency (implied by its absence) and the addition of updated dependencies contribute to the cleaner, more maintainable architecture targeted by this PR.packages/utils/src/index.ts (2)
16-16: Export addition for comparison utilities looks good.This new export follows the alphabetical ordering pattern of the file and is consistent with the export style used throughout. It aligns with the PR objective of removing dependency on connect by centralizing utility functions.
33-33: Export addition for hexToRgba utility looks good.This new export follows the alphabetical ordering pattern of the file and is consistent with the export style used throughout. It aligns with the PR objective of removing dependency on connect by centralizing utility functions.
suite-native/module-trading/tsconfig.json (1)
8-10: New Reference Added: Utility Package InclusionThe new entry
{ "path": "../../packages/utils" }in thereferencesarray aligns well with our objectives to shift dependencies toward the new utility package. Please verify that the relative path is correct within the monorepo structure and that dependent modules needing these types are updated accordingly.packages/components/src/components/VirtualizedList/VirtualizedList.tsx (1)
6-6: Import source updated appropriately.The import for
isChangedhas been changed from@suite-common/suite-utilsto@trezor/utils, aligning with the PR objective to remove connect dependencies and improve modularity.packages/utils/tests/comparison.test.ts (3)
1-2: Added import for comparison utilities.The new import correctly references the implementation that was moved to the utils package.
3-3: Variable declaration updated correctly.Converted from exported constant to local constant, as the implementation has been moved to the utils package.
168-175: Added test suite to verify the moved utility function.New test suite properly verifies that the moved implementation of
isChangedin the utils package works as expected with all the existing test cases.packages/suite/src/actions/wallet/selectedAccountActions.ts (2)
12-12: Import source updated appropriately.The import for
isChangedhas been changed from a namespace import to a direct import from@trezor/utils, aligning with the PR objective to remove connect dependencies.
200-200: Updated function reference correctly.The function reference has been updated to call the directly imported
isChangedfunction instead of using it through the previous namespace import. The functionality remains unchanged.packages/device-utils/src/index.ts (1)
4-6:❓ Verification inconclusive
Verify impact of removing
modelUtilsexport.The change removes
modelUtilsexport and introduces new exports. Ensure all components have been updated to use the new exports instead of the removed one to maintain compatibility.
🏁 Script executed:
#!/bin/bash # Search for imports of modelUtils from device-utils package to ensure there are no remaining references echo "Checking for imports of modelUtils from @trezor/device-utils" rg "import.*from\s+['\"]@trezor/device-utils['\"]" -A 5 | rg "modelUtils" # Search for any reference to modelUtils to ensure nothing is broken by its removal echo "Checking for any references to modelUtils" rg "modelUtils" --type ts --type tsxLength of output: 1413
Action: Manually Verify All References to "modelUtils" Are Updated
The removal of the
modelUtilsexport from the device-utils package appears not to affect external consumers—no imports ofmodelUtilsfrom@trezor/device-utilswere found. However, note the following:
- Test files in the suite (e.g.,
packages/suite/src/utils/device/modelUtils.test.tsandpackages/suite/src/utils/device/modelUtils.ts) still referencemodelUtilsvia relative imports. Confirm these usages are correct or have been updated according to the new export structure.- The search for references in TSX files reported a file type error. Please manually inspect any TSX components to ensure there are no lingering unintended imports of
modelUtilsfrom the device-utils package.Please verify these points to ensure full compatibility across all components.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/device-utils/src/types.ts (1)
62-62:⚠️ Potential issueAction Required: Update or Remove Outdated TODO Comment
The TODO comment indicates code was copied from
packages/protobuf/src/messages, but this source appears to no longer exist based on previous review feedback.Since this PR is removing dependency on connect in components, consider if these types should be the canonical source rather than a copy. Either update the comment with the current source location or remove it if this is now the primary definition.
🧹 Nitpick comments (3)
packages/device-utils/src/deviceModelInternalUtils.ts (1)
11-11: Consider using a type assertion with the specific type instead ofany.While the
as anycast works, it bypasses TypeScript's type checking. For better type safety, consider using a more specific type assertion:- (model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model) as any; + (model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model) as T extends DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : T;packages/device-utils/src/types.ts (1)
10-60: Consider simplifying the complexFeaturesNarrowingunion type.The
FeaturesNarrowingtype contains significant repetition across its union members. Consider extracting common properties into base types to improve readability and maintainability:type BaseFeatureNarrowing = { minor_version: number; patch_version: number; fw_major: null; fw_minor: null; fw_patch: null; }; type V1BootloaderBase = BaseFeatureNarrowing & { major_version: 1; bootloader_mode: true; }; type V2BootloaderBase = BaseFeatureNarrowing & { major_version: 2; bootloader_mode: true; }; // Then define the union with these base types export type FeaturesNarrowing = | (V2BootloaderBase & { firmware_present: false }) | (V2BootloaderBase & { fw_major: 2; fw_minor: number; fw_patch: number; firmware_present: true; }) // ... and so onpackages/protobuf/messages.json (1)
5601-5618: NostrTag Message StructureThe
"NostrTag"message is structured to include:
- A required
"key"field (string),- An optional
"value"field (string),- A repeated
"extra"field (string).This design supports flexible tagging for events. It may be beneficial to add inline documentation (or comments within the source) explaining the intended semantics of
"value"and"extra"fields to aid future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (164)
packages/components/package.json(1 hunks)packages/components/src/components/VirtualizedList/VirtualizedList.tsx(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/src/components/buttons/buttonStyleUtils.ts(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/e2e/__fixtures__/getFeatures.ts(1 hunks)packages/connect/package.json(1 hunks)packages/connect/setupJest.ts(1 hunks)packages/connect/src/api/common/__fixtures__/paramsValidator.ts(1 hunks)packages/connect/src/api/common/paramsValidator.ts(1 hunks)packages/connect/src/api/ethereum/api/ethereumSignTypedData.ts(1 hunks)packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts(1 hunks)packages/connect/src/api/firmware/parseFirmwareHeaders.ts(1 hunks)packages/connect/src/data/DataManager.ts(1 hunks)packages/connect/src/data/__tests__/firmwareInfo.test.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/data/models.ts(1 hunks)packages/connect/src/device/Device.ts(1 hunks)packages/connect/src/device/__tests__/checkFirmwareRevision.test.ts(1 hunks)packages/connect/src/device/calculateRevisionForDevice.ts(1 hunks)packages/connect/src/events/device.ts(1 hunks)packages/connect/src/types/api/firmwareUpdate.ts(1 hunks)packages/connect/src/types/coinInfo.ts(1 hunks)packages/connect/src/types/device.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/types/index.ts(1 hunks)packages/connect/src/utils/__tests__/deviceFeaturesUtils.test.ts(1 hunks)packages/connect/src/utils/assetUtils.ts(1 hunks)packages/connect/src/utils/deviceFeaturesUtils.ts(1 hunks)packages/connect/tsconfig.json(1 hunks)packages/connect/tsconfig.lib.json(1 hunks)packages/device-utils/package.json(1 hunks)packages/device-utils/src/deviceModelInternalUtils.ts(1 hunks)packages/device-utils/src/firmwareUtils.ts(2 hunks)packages/device-utils/src/index.ts(1 hunks)packages/device-utils/src/types.ts(1 hunks)packages/device-utils/tests/modelUtils.test.ts(0 hunks)packages/device-utils/tsconfig.json(1 hunks)packages/device-utils/tsconfig.lib.json(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/messages.json(2 hunks)packages/protobuf/scripts/protobuf-types.ts(1 hunks)packages/protobuf/src/index.ts(0 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/backup/__tests__/backupActions.test.ts(1 hunks)packages/suite/src/actions/firmware/__fixtures__/firmwareActions.ts(1 hunks)packages/suite/src/actions/firmware/__tests__/firmwareActions.test.ts(1 hunks)packages/suite/src/actions/recovery/__tests__/recoveryActions.test.ts(1 hunks)packages/suite/src/actions/recovery/recoveryActions.ts(1 hunks)packages/suite/src/actions/wallet/selectedAccountActions.ts(2 hunks)packages/suite/src/components/firmware/ReconnectDevicePrompt.tsx(1 hunks)packages/suite/src/components/onboarding/Hologram.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/DeviceDisplay.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/DisplayPaginatedText.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/__tests__/DeviceDisplay.test.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/__tests__/parseTextToPagesAndLines.test.ts(1 hunks)packages/suite/src/components/suite/DeviceDisplay/parseTextToPagesAndLines.ts(1 hunks)packages/suite/src/components/suite/DeviceMatrixExplanation.tsx(1 hunks)packages/suite/src/components/suite/HomescreenGallery.tsx(1 hunks)packages/suite/src/components/suite/PinMatrix/PinMatrix.tsx(1 hunks)packages/suite/src/components/suite/SecurityCheck/SecurityCheckLayout.tsx(1 hunks)packages/suite/src/components/suite/WordInputAdvanced.tsx(1 hunks)packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceStatus.tsx(1 hunks)packages/suite/src/config/onboarding/steps.ts(1 hunks)packages/suite/src/constants/suite/device.ts(1 hunks)packages/suite/src/constants/suite/homescreens.ts(1 hunks)packages/suite/src/hooks/settings/useNetworkSupport.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts(1 hunks)packages/suite/src/hooks/wallet/useSendFormCompose.ts(1 hunks)packages/suite/src/hooks/wallet/useStakeEthForm.ts(1 hunks)packages/suite/src/hooks/wallet/useUnstakeEthForm.ts(1 hunks)packages/suite/src/middlewares/suite/suiteMiddleware.ts(1 hunks)packages/suite/src/storage/migrations/index.ts(1 hunks)packages/suite/src/types/onboarding/index.ts(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/utils/device/modelUtils.ts(1 hunks)packages/suite/src/utils/firmware/__tests__/firmware.test.ts(1 hunks)packages/suite/src/utils/firmware/index.ts(1 hunks)packages/suite/src/utils/onboarding/__tests__/steps.test.ts(1 hunks)packages/suite/src/utils/suite/__fixtures__/device.ts(1 hunks)packages/suite/src/utils/suite/__fixtures__/homescreen.ts(1 hunks)packages/suite/src/utils/suite/__tests__/homescreen.test.ts(1 hunks)packages/suite/src/utils/suite/homescreen.ts(1 hunks)packages/suite/src/views/onboarding/steps/Recovery/RecoveryStepBox.tsx(1 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/onboarding/steps/ResetDevice.tsx(1 hunks)packages/suite/src/views/onboarding/steps/SelectBackupType/SelectBackupType.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(2 hunks)packages/suite/src/views/settings/SettingsDevice/DisplayRotation.tsx(1 hunks)packages/suite/src/views/settings/SettingsDevice/Homescreen.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoRedelegate.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoRewards.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoStake.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoStakingDashboard.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/DeviceButton.tsx(1 hunks)packages/suite/tsconfig.json(1 hunks)packages/utils/src/index.ts(2 hunks)packages/utils/tests/comparison.test.ts(2 hunks)suite-common/firmware/src/hooks/useFirmwareInstallation.ts(1 hunks)suite-common/message-system/src/__fixtures__/messageSystemUtils.ts(1 hunks)suite-common/suite-constants/src/device.ts(1 hunks)suite-common/suite-types/src/guide.ts(1 hunks)suite-common/suite-utils/src/__tests__/comparison.test.ts(0 hunks)suite-common/suite-utils/src/__tests__/device.test.ts(1 hunks)suite-common/suite-utils/src/device.ts(1 hunks)suite-common/suite-utils/src/index.ts(0 hunks)suite-common/test-utils/src/mocks.ts(1 hunks)suite-common/wallet-config/package.json(1 hunks)suite-common/wallet-config/src/networksConfig.ts(1 hunks)suite-common/wallet-config/src/types.ts(1 hunks)suite-common/wallet-config/tsconfig.json(1 hunks)suite-common/wallet-core/src/device/deviceConstants.ts(1 hunks)suite-common/wallet-core/src/device/deviceThunks.ts(1 hunks)suite-common/wallet-core/src/send/sendFormReducer.ts(1 hunks)suite-native/analytics/src/events.ts(1 hunks)suite-native/analytics/src/types.ts(1 hunks)suite-native/device/src/components/DeviceImage.tsx(1 hunks)suite-native/device/src/types.ts(1 hunks)suite-native/device/src/utils.ts(1 hunks)suite-native/firmware/src/hooks/useFirmwareAnalytics.ts(1 hunks)suite-native/icons/package.json(1 hunks)suite-native/icons/src/DeviceModelIcon.tsx(1 hunks)suite-native/icons/tsconfig.json(1 hunks)suite-native/module-authorize-device/src/components/connect/PinOnDevice.tsx(1 hunks)suite-native/module-authorize-device/src/constants/deviceImageConstants.ts(1 hunks)suite-native/module-authorize-device/src/screens/connect/PinScreen.tsx(1 hunks)suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx(1 hunks)suite-native/module-onboarding/package.json(1 hunks)suite-native/module-onboarding/src/components/SecuritySealDescription.tsx(1 hunks)suite-native/module-onboarding/src/components/SecuritySealImages.tsx(1 hunks)suite-native/module-onboarding/src/screens/UninitializedDeviceLandingScreen.tsx(1 hunks)suite-native/module-onboarding/src/screens/WelcomeScreen.tsx(1 hunks)suite-native/module-onboarding/tsconfig.json(1 hunks)suite-native/module-send/package.json(0 hunks)suite-native/module-send/src/components/SlidingFooterOverlay.tsx(1 hunks)suite-native/module-send/tsconfig.json(0 hunks)suite-native/module-trading/package.json(1 hunks)suite-native/module-trading/src/components/general/TradeableAssetButton.tsx(1 hunks)suite-native/module-trading/tsconfig.json(1 hunks)suite-native/navigation/package.json(1 hunks)suite-native/navigation/src/components/ScreenFooterGradient.tsx(1 hunks)suite-native/navigation/tsconfig.json(1 hunks)suite-native/react-native-graph/package.json(1 hunks)suite-native/react-native-graph/src/AnimatedLineGraph.tsx(1 hunks)suite-native/react-native-graph/tsconfig.json(1 hunks)suite-native/receive/src/components/DevicePaginationButton.tsx(1 hunks)suite-native/receive/src/components/DeviceScreenContent.tsx(1 hunks)suite-native/receive/src/components/DeviceScreenPagination.tsx(1 hunks)suite-native/receive/src/types.ts(1 hunks)suite-native/receive/src/utils.ts(1 hunks)
💤 Files with no reviewable changes (6)
- suite-native/module-send/tsconfig.json
- suite-native/module-send/package.json
- suite-common/suite-utils/src/index.ts
- suite-common/suite-utils/src/tests/comparison.test.ts
- packages/protobuf/src/index.ts
- packages/device-utils/tests/modelUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (155)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- packages/suite/package.json
- suite-native/module-onboarding/src/components/SecuritySealDescription.tsx
- packages/suite/src/utils/suite/tests/homescreen.test.ts
- packages/suite/src/components/suite/HomescreenGallery.tsx
- packages/connect/tsconfig.lib.json
- suite-native/module-authorize-device/src/constants/deviceImageConstants.ts
- suite-native/module-trading/tsconfig.json
- packages/suite/src/utils/firmware/index.ts
- suite-native/module-onboarding/src/screens/WelcomeScreen.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceStatus.tsx
- packages/suite/src/views/wallet/staking/components/CardanoRedelegate.tsx
- packages/connect/src/events/device.ts
- packages/suite/src/hooks/wallet/useSendFormCompose.ts
- suite-common/wallet-config/package.json
- packages/suite/src/views/onboarding/steps/SelectBackupType/SelectBackupType.tsx
- suite-native/device/src/types.ts
- packages/suite/src/components/suite/DeviceDisplay/tests/DeviceDisplay.test.tsx
- packages/suite/src/utils/device/modelUtils.ts
- packages/suite/tsconfig.json
- packages/suite/src/types/onboarding/index.ts
- suite-native/receive/src/components/DeviceScreenContent.tsx
- packages/suite/src/constants/suite/homescreens.ts
- suite-common/wallet-config/src/networksConfig.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- suite-common/suite-utils/src/tests/device.test.ts
- packages/utils/tests/comparison.test.ts
- packages/connect/src/types/coinInfo.ts
- packages/components/tsconfig.json
- suite-native/icons/src/DeviceModelIcon.tsx
- packages/suite/src/config/onboarding/steps.ts
- packages/suite/src/views/onboarding/steps/ResetDevice.tsx
- suite-native/module-onboarding/tsconfig.json
- packages/connect/package.json
- packages/utils/src/index.ts
- packages/suite/src/views/wallet/staking/components/CardanoRewards.tsx
- suite-native/module-onboarding/src/components/SecuritySealImages.tsx
- packages/suite/src/actions/firmware/fixtures/firmwareActions.ts
- packages/device-utils/tsconfig.lib.json
- suite-native/device/src/components/DeviceImage.tsx
- suite-native/receive/src/components/DeviceScreenPagination.tsx
- packages/product-components/tsconfig.json
- suite-native/react-native-graph/tsconfig.json
- suite-native/module-trading/src/components/general/TradeableAssetButton.tsx
- packages/suite/src/utils/suite/homescreen.ts
- packages/connect/src/device/calculateRevisionForDevice.ts
- packages/suite/src/components/suite/SecurityCheck/SecurityCheckLayout.tsx
- packages/suite/src/views/onboarding/steps/Recovery/RecoveryStepBox.tsx
- suite-native/navigation/tsconfig.json
- packages/components/src/components/buttons/buttonStyleUtils.ts
- packages/suite/src/views/wallet/staking/components/CardanoStake.tsx
- packages/connect/src/data/DataManager.ts
- suite-native/module-onboarding/src/screens/UninitializedDeviceLandingScreen.tsx
- suite-common/suite-constants/src/device.ts
- packages/device-utils/tsconfig.json
- suite-native/receive/src/components/DevicePaginationButton.tsx
- suite-common/wallet-core/src/device/deviceThunks.ts
- packages/connect/src/data/models.ts
- packages/suite/src/views/wallet/staking/components/DeviceButton.tsx
- packages/suite/src/components/onboarding/Hologram.tsx
- packages/suite/src/views/settings/SettingsDevice/Homescreen.tsx
- packages/connect/tsconfig.json
- suite-common/wallet-config/src/types.ts
- packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts
- packages/connect/src/types/api/firmwareUpdate.ts
- packages/protobuf/scripts/protobuf-types.ts
- packages/connect/src/utils/assetUtils.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx
- suite-native/navigation/src/components/ScreenFooterGradient.tsx
- suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/suite/src/views/wallet/staking/components/CardanoStakingDashboard.tsx
- packages/connect/src/api/firmware/getBinaryForFirmwareUpgrade.ts
- suite-native/module-onboarding/package.json
- packages/device-utils/src/index.ts
- packages/components/src/components/VirtualizedList/VirtualizedList.tsx
- packages/suite/src/constants/suite/device.ts
- packages/suite/src/utils/firmware/tests/firmware.test.ts
- suite-common/wallet-core/src/device/deviceConstants.ts
- packages/components/package.json
- packages/connect/src/api/common/fixtures/paramsValidator.ts
- packages/suite/src/utils/onboarding/tests/steps.test.ts
- suite-native/analytics/src/events.ts
- packages/product-components/src/components/TokenIconSet/TokenIconSet.tsx
- packages/suite/src/components/suite/DeviceDisplay/DeviceDisplay.tsx
- suite-common/message-system/src/fixtures/messageSystemUtils.ts
- suite-native/analytics/src/types.ts
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- packages/suite/src/storage/migrations/index.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- packages/suite/src/hooks/wallet/useStakeEthForm.ts
- packages/suite/src/components/suite/DeviceDisplay/DisplayPaginatedText.tsx
- suite-native/module-authorize-device/src/screens/connect/PinScreen.tsx
- packages/suite/src/components/suite/DeviceMatrixExplanation.tsx
- packages/components/src/components/animations/DeviceAnimation.tsx
- suite-native/module-send/src/components/SlidingFooterOverlay.tsx
- packages/suite/src/actions/recovery/tests/recoveryActions.test.ts
- packages/connect/src/types/index.ts
- packages/device-utils/src/firmwareUtils.ts
- packages/suite/src/actions/wallet/selectedAccountActions.ts
- packages/suite/src/actions/recovery/recoveryActions.ts
- suite-common/firmware/src/hooks/useFirmwareInstallation.ts
- suite-native/module-authorize-device/src/components/connect/PinOnDevice.tsx
- packages/connect/src/api/firmware/parseFirmwareHeaders.ts
- packages/suite/src/components/suite/PinMatrix/PinMatrix.tsx
- suite-native/device/src/utils.ts
- packages/suite/src/components/suite/DeviceDisplay/tests/parseTextToPagesAndLines.test.ts
- suite-native/module-trading/package.json
- suite-common/wallet-core/src/send/sendFormReducer.ts
- suite-native/receive/src/types.ts
- suite-native/icons/package.json
- packages/suite/src/utils/suite/fixtures/device.ts
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/suite/src/components/suite/WordInputAdvanced.tsx
- suite-common/suite-utils/src/device.ts
- packages/suite/src/components/suite/DeviceDisplay/parseTextToPagesAndLines.ts
- packages/connect/src/api/ethereum/api/ethereumSignTypedData.ts
- suite-native/receive/src/utils.ts
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts
- packages/connect/src/device/Device.ts
- packages/connect/e2e/fixtures/getFeatures.ts
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- suite-common/wallet-config/tsconfig.json
- packages/product-components/package.json
- packages/connect/src/data/firmwareInfo.ts
- packages/connect/src/types/device.ts
- packages/suite/src/views/recovery/index.tsx
- packages/suite/src/utils/suite/fixtures/homescreen.ts
- packages/connect/src/utils/tests/deviceFeaturesUtils.test.ts
- packages/connect/src/device/tests/checkFirmwareRevision.test.ts
- suite-native/react-native-graph/package.json
- suite-native/icons/tsconfig.json
- suite-common/suite-types/src/guide.ts
- suite-native/navigation/package.json
- suite-common/test-utils/src/mocks.ts
- suite-native/react-native-graph/src/AnimatedLineGraph.tsx
- packages/connect/src/api/common/paramsValidator.ts
- packages/connect/src/utils/deviceFeaturesUtils.ts
- packages/device-utils/package.json
- packages/connect/src/types/firmware.ts
- packages/connect/setupJest.ts
- packages/suite/src/hooks/settings/useNetworkSupport.ts
- packages/suite/src/views/settings/SettingsDevice/DisplayRotation.tsx
- packages/suite/src/actions/backup/tests/backupActions.test.ts
- packages/suite/src/hooks/wallet/useUnstakeEthForm.ts
- packages/suite/src/middlewares/suite/suiteMiddleware.ts
- packages/suite/src/components/firmware/ReconnectDevicePrompt.tsx
- packages/connect/src/data/tests/firmwareInfo.test.ts
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/suite/src/actions/firmware/tests/firmwareActions.test.ts
- suite-native/firmware/src/hooks/useFirmwareAnalytics.ts
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Unit Tests
- GitHub Check: Releases revision Checks
- GitHub Check: Build libs for publishing
- GitHub Check: Other Checks
- GitHub Check: Linting and formatting
- GitHub Check: Type Checking
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: test
- GitHub Check: build-web
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (7)
packages/device-utils/src/deviceModelInternalUtils.ts (1)
8-11: LGTM: Well-designed utility function to handle device model mapping.The implementation effectively maps T2B1 to T3B1 while preserving the original model type in all other cases. The generic typing with conditional return type provides good type safety for consumers.
packages/device-utils/src/types.ts (2)
1-8: LGTM: Well-defined types for firmware versioning.The
FirmwareVersionStringtemplate literal type andVersionArraytuple type provide strong typing for version information. TheFirmwareTypeenum clearly defines the two possible firmware variants.
63-80: LGTM: UpdatedPartialDevicetype with more specific features structure.The type now uses the newly defined
FirmwareTypeenum and provides a more detailed structure for device features, which should help with type safety throughout the codebase.packages/protobuf/messages.json (4)
5580-5591: NostrGetPubkey Message Type AdditionThe new
"NostrGetPubkey"message type is defined similarly to other address request messages in the file. The use of the repeated field"address_n"with an explicit"packed": falseoption is consistent with the project's existing patterns. Please verify that this option is intentional (e.g. for compatibility with expected client behavior) and that this message integrates correctly with the handling logic in downstream components.
5592-5600: NostrPubkey Message Type AdditionThe
"NostrPubkey"message correctly defines a required"pubkey"field of typebytes. This follows the established format seen in other public key messages, ensuring consistency across the protocol messages.
5619-5650: NostrSignEvent Message DefinitionThe
"NostrSignEvent"message aggregates multiple critical properties for event signing:
"address_n"is defined as a repeateduint32with the"packed": falseoption."created_at","kind", and"content"are marked as required.- The
"tags"field correctly uses the newly defined"NostrTag"repeated message.This structure aligns with patterns for similar event-protocol messages. One point to review is the use of
uint32for"created_at". If this represents a Unix timestamp, ensure that its range (up to year 2106) meets the long-term requirements of the application.
5651-5669: NostrEventSignature Message StructureThe
"NostrEventSignature"message correctly specifies three required fields:
"pubkey","id", and"signature",all defined as
bytes. This structure is consistent with other signature-related messages in the codebase and should integrate smoothly with the event-signing functionality.
6b8de11 to
bb31b97
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/device-utils/src/deviceModelInternalUtils.ts (2)
8-11: Consider improving type safety by avoidingas anycasting.The function logic is sound and well-documented, but the use of
as anybypasses TypeScript's type system, which could lead to potential type errors at runtime. A safer alternative would be to use a more specific type assertion or a type guard pattern.export const getNarrowedDeviceModelInternal = <T extends DeviceModelInternal>( model: T, ): T extends DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : T => - (model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model) as any; + (model === DeviceModelInternal.T2B1 + ? DeviceModelInternal.T3B1 as (T extends DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : never) + : model) as (T extends DeviceModelInternal.T2B1 ? never : T);
8-11: Functional name might be clearer than "Narrowed".The function name suggests "narrowing" a type, but it's actually mapping specific types to others. Consider a more descriptive name like
getNormalizedDeviceModelInternalorgetCompatibleDeviceModelInternalthat better reflects its purpose of treating functionally similar device models uniformly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
packages/components/src/components/VirtualizedList/VirtualizedList.tsx(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/src/components/buttons/buttonStyleUtils.ts(1 hunks)packages/device-utils/src/deviceModelInternalUtils.ts(1 hunks)packages/device-utils/src/index.ts(1 hunks)packages/suite/src/actions/wallet/selectedAccountActions.ts(2 hunks)packages/suite/src/components/suite/HomescreenGallery.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts(1 hunks)packages/suite/src/hooks/wallet/useSendFormCompose.ts(1 hunks)packages/suite/src/hooks/wallet/useStakeEthForm.ts(1 hunks)packages/suite/src/hooks/wallet/useUnstakeEthForm.ts(1 hunks)packages/suite/src/views/recovery/index.tsx(2 hunks)packages/utils/src/index.ts(2 hunks)packages/utils/tests/comparison.test.ts(2 hunks)suite-common/suite-utils/src/__tests__/comparison.test.ts(0 hunks)suite-common/suite-utils/src/device.ts(1 hunks)suite-common/suite-utils/src/index.ts(0 hunks)suite-common/wallet-config/src/types.ts(1 hunks)suite-common/wallet-core/src/device/deviceThunks.ts(1 hunks)suite-native/module-onboarding/package.json(1 hunks)suite-native/module-onboarding/src/screens/WelcomeScreen.tsx(1 hunks)suite-native/module-onboarding/tsconfig.json(1 hunks)suite-native/module-send/package.json(0 hunks)suite-native/module-send/src/components/SlidingFooterOverlay.tsx(1 hunks)suite-native/module-send/tsconfig.json(0 hunks)suite-native/module-trading/package.json(1 hunks)suite-native/module-trading/src/components/general/TradeableAssetButton.tsx(1 hunks)suite-native/module-trading/tsconfig.json(1 hunks)suite-native/navigation/package.json(1 hunks)suite-native/navigation/src/components/ScreenFooterGradient.tsx(1 hunks)suite-native/navigation/tsconfig.json(1 hunks)suite-native/react-native-graph/package.json(1 hunks)suite-native/react-native-graph/src/AnimatedLineGraph.tsx(1 hunks)suite-native/react-native-graph/tsconfig.json(1 hunks)suite-native/test-utils/package.json(1 hunks)suite-native/test-utils/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (4)
- suite-native/module-send/tsconfig.json
- suite-common/suite-utils/src/tests/comparison.test.ts
- suite-native/module-send/package.json
- suite-common/suite-utils/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (33)
- suite-native/navigation/package.json
- packages/suite/src/hooks/wallet/useSendFormCompose.ts
- suite-native/module-trading/tsconfig.json
- suite-native/module-trading/package.json
- packages/components/src/components/buttons/buttonStyleUtils.ts
- suite-common/wallet-config/src/types.ts
- suite-native/module-send/src/components/SlidingFooterOverlay.tsx
- suite-native/navigation/src/components/ScreenFooterGradient.tsx
- suite-native/react-native-graph/package.json
- suite-native/react-native-graph/src/AnimatedLineGraph.tsx
- packages/utils/tests/comparison.test.ts
- packages/utils/src/index.ts
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts
- packages/components/src/components/VirtualizedList/VirtualizedList.tsx
- packages/suite/src/components/suite/HomescreenGallery.tsx
- suite-native/react-native-graph/tsconfig.json
- packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts
- suite-native/module-trading/src/components/general/TradeableAssetButton.tsx
- packages/suite/src/hooks/wallet/useStakeEthForm.ts
- suite-native/navigation/tsconfig.json
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/suite/src/views/recovery/index.tsx
- suite-native/module-onboarding/tsconfig.json
- packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx
- packages/suite/src/hooks/wallet/useUnstakeEthForm.ts
- suite-common/suite-utils/src/device.ts
- suite-native/module-onboarding/src/screens/WelcomeScreen.tsx
- suite-native/module-onboarding/package.json
- packages/suite/src/actions/wallet/selectedAccountActions.ts
- packages/device-utils/src/index.ts
- suite-common/wallet-core/src/device/deviceThunks.ts
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Releases revision Checks
- GitHub Check: Other Checks
- GitHub Check: Build libs for publishing
- GitHub Check: Linting and formatting
- GitHub Check: Type Checking
- GitHub Check: Unit Tests
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: test
- GitHub Check: build-web
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
suite-native/test-utils/tsconfig.json (2)
5-7: New Reference to "suite-common/formatters" Added
The addition of this reference integrates shared formatting utilities into the test configuration. Please verify that the relative path ("../../suite-common/formatters") is correct for all environments in your monorepo.
8-8: New Reference to "intl" Module Added
Integrating the"../intl"reference supports internationalization features for tests. Ensure that this path correctly resolves within your project structure so that the fallback or new intl implementations are properly included.suite-native/test-utils/package.json (1)
16-17: New Workspace Dependencies for Enhanced ModularityThe addition of
"@suite-common/formatters": "workspace:*"and"@suite-native/intl": "workspace:*"aligns well with the PR’s objective to reduce unnecessary dependencies. These changes support the move toward a cleaner, more independent module structure by leveraging workspace resolution. Please ensure that these packages are correctly linked in the workspace context to avoid any unexpected build issues.packages/device-utils/src/deviceModelInternalUtils.ts (2)
3-7: Great documentation!The JSDoc comment clearly explains the purpose and rationale behind this utility function, which is crucial for maintainability. This documentation helps other developers understand why T2B1 and T3B1 models should be treated the same in most contexts.
1-2: LGTM - Clean import.The import statement correctly references the local module containing the
DeviceModelInternalenum. This aligns with the PR objective of restructuring dependencies.
bb31b97 to
fb6752d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/device-utils/src/deviceModelInternalUtils.ts (1)
8-11: Consider a more type-safe alternative toas any.While the function implementation correctly matches its documentation, the use of
as anytype assertion bypasses TypeScript's type checking system, which could potentially hide errors.An alternative approach could be to use function overloads for better type safety:
export function getNarrowedDeviceModelInternal(model: DeviceModelInternal.T2B1): DeviceModelInternal.T3B1; export function getNarrowedDeviceModelInternal<T extends DeviceModelInternal>(model: T): T; export function getNarrowedDeviceModelInternal<T extends DeviceModelInternal>(model: T): DeviceModelInternal { return model === DeviceModelInternal.T2B1 ? DeviceModelInternal.T3B1 : model; }This approach would maintain full type safety without needing type assertions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (145)
packages/components/package.json(1 hunks)packages/components/src/components/VirtualizedList/VirtualizedList.tsx(1 hunks)packages/components/src/components/animations/DeviceAnimation.tsx(1 hunks)packages/components/src/components/animations/LottieAnimation.tsx(1 hunks)packages/components/src/components/buttons/buttonStyleUtils.ts(1 hunks)packages/components/tsconfig.json(1 hunks)packages/connect/e2e/__fixtures__/getFeatures.ts(1 hunks)packages/connect/src/api/common/__fixtures__/paramsValidator.ts(1 hunks)packages/connect/src/api/common/paramsValidator.ts(1 hunks)packages/connect/src/api/ethereum/api/ethereumSignTypedData.ts(1 hunks)packages/connect/src/data/DataManager.ts(1 hunks)packages/connect/src/data/__tests__/firmwareInfo.test.ts(1 hunks)packages/connect/src/data/firmwareInfo.ts(1 hunks)packages/connect/src/data/models.ts(1 hunks)packages/connect/src/device/Device.ts(1 hunks)packages/connect/src/device/__tests__/checkFirmwareRevision.test.ts(1 hunks)packages/connect/src/types/coinInfo.ts(1 hunks)packages/connect/src/types/firmware.ts(1 hunks)packages/connect/src/utils/__tests__/deviceFeaturesUtils.test.ts(1 hunks)packages/connect/src/utils/assetUtils.ts(1 hunks)packages/connect/src/utils/deviceFeaturesUtils.ts(1 hunks)packages/device-utils/src/deviceModelInternalUtils.ts(1 hunks)packages/device-utils/src/index.ts(1 hunks)packages/product-components/package.json(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx(1 hunks)packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx(1 hunks)packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx(2 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx(1 hunks)packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx(1 hunks)packages/product-components/src/utils/mapTrezorModelToIcon.ts(1 hunks)packages/product-components/tsconfig.json(1 hunks)packages/protobuf/scripts/protobuf-types.ts(1 hunks)packages/protobuf/src/index.ts(0 hunks)packages/suite/src/actions/backup/__tests__/backupActions.test.ts(1 hunks)packages/suite/src/actions/firmware/__fixtures__/firmwareActions.ts(1 hunks)packages/suite/src/actions/firmware/__tests__/firmwareActions.test.ts(1 hunks)packages/suite/src/actions/recovery/__tests__/recoveryActions.test.ts(1 hunks)packages/suite/src/actions/recovery/recoveryActions.ts(1 hunks)packages/suite/src/actions/wallet/selectedAccountActions.ts(2 hunks)packages/suite/src/components/firmware/ReconnectDevicePrompt.tsx(1 hunks)packages/suite/src/components/onboarding/Hologram.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/DeviceDisplay.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/DisplayPaginatedText.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/__tests__/DeviceDisplay.test.tsx(1 hunks)packages/suite/src/components/suite/DeviceDisplay/__tests__/parseTextToPagesAndLines.test.ts(1 hunks)packages/suite/src/components/suite/DeviceDisplay/parseTextToPagesAndLines.ts(1 hunks)packages/suite/src/components/suite/DeviceMatrixExplanation.tsx(1 hunks)packages/suite/src/components/suite/HomescreenGallery.tsx(1 hunks)packages/suite/src/components/suite/PinMatrix/PinMatrix.tsx(1 hunks)packages/suite/src/components/suite/SecurityCheck/SecurityCheckLayout.tsx(1 hunks)packages/suite/src/components/suite/WordInputAdvanced.tsx(1 hunks)packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceStatus.tsx(1 hunks)packages/suite/src/config/onboarding/steps.ts(1 hunks)packages/suite/src/constants/suite/device.ts(1 hunks)packages/suite/src/constants/suite/homescreens.ts(1 hunks)packages/suite/src/hooks/settings/useNetworkSupport.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts(1 hunks)packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts(1 hunks)packages/suite/src/hooks/wallet/useSendFormCompose.ts(1 hunks)packages/suite/src/hooks/wallet/useStakeEthForm.ts(1 hunks)packages/suite/src/hooks/wallet/useUnstakeEthForm.ts(1 hunks)packages/suite/src/middlewares/suite/suiteMiddleware.ts(1 hunks)packages/suite/src/storage/migrations/index.ts(1 hunks)packages/suite/src/types/onboarding/index.ts(1 hunks)packages/suite/src/utils/device/__tests__/modelUtils.test.ts(2 hunks)packages/suite/src/utils/device/modelUtils.ts(1 hunks)packages/suite/src/utils/firmware/__tests__/firmware.test.ts(1 hunks)packages/suite/src/utils/firmware/index.ts(1 hunks)packages/suite/src/utils/onboarding/__tests__/steps.test.ts(1 hunks)packages/suite/src/utils/suite/__fixtures__/device.ts(1 hunks)packages/suite/src/utils/suite/__fixtures__/homescreen.ts(1 hunks)packages/suite/src/utils/suite/__tests__/homescreen.test.ts(1 hunks)packages/suite/src/utils/suite/homescreen.ts(1 hunks)packages/suite/src/views/onboarding/steps/Recovery/RecoveryStepBox.tsx(1 hunks)packages/suite/src/views/onboarding/steps/Recovery/index.tsx(1 hunks)packages/suite/src/views/onboarding/steps/ResetDevice.tsx(1 hunks)packages/suite/src/views/onboarding/steps/SelectBackupType/SelectBackupType.tsx(1 hunks)packages/suite/src/views/recovery/index.tsx(2 hunks)packages/suite/src/views/settings/SettingsDevice/DisplayRotation.tsx(1 hunks)packages/suite/src/views/settings/SettingsDevice/Homescreen.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoRedelegate.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoRewards.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoStake.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/CardanoStakingDashboard.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/DeviceButton.tsx(1 hunks)packages/utils/src/index.ts(2 hunks)packages/utils/tests/comparison.test.ts(2 hunks)suite-common/firmware/src/hooks/useFirmwareInstallation.ts(1 hunks)suite-common/message-system/src/__fixtures__/messageSystemUtils.ts(1 hunks)suite-common/suite-constants/src/device.ts(1 hunks)suite-common/suite-types/src/guide.ts(1 hunks)suite-common/suite-utils/src/__tests__/comparison.test.ts(0 hunks)suite-common/suite-utils/src/__tests__/device.test.ts(1 hunks)suite-common/suite-utils/src/device.ts(1 hunks)suite-common/suite-utils/src/index.ts(0 hunks)suite-common/test-utils/src/mocks.ts(1 hunks)suite-common/wallet-config/package.json(1 hunks)suite-common/wallet-config/src/networksConfig.ts(1 hunks)suite-common/wallet-config/src/types.ts(1 hunks)suite-common/wallet-config/tsconfig.json(1 hunks)suite-common/wallet-core/src/device/deviceConstants.ts(1 hunks)suite-common/wallet-core/src/device/deviceThunks.ts(1 hunks)suite-common/wallet-core/src/send/sendFormReducer.ts(1 hunks)suite-native/analytics/src/events.ts(1 hunks)suite-native/analytics/src/types.ts(1 hunks)suite-native/device/src/components/DeviceImage.tsx(1 hunks)suite-native/device/src/types.ts(1 hunks)suite-native/device/src/utils.ts(1 hunks)suite-native/firmware/src/hooks/useFirmwareAnalytics.ts(1 hunks)suite-native/icons/package.json(1 hunks)suite-native/icons/src/DeviceModelIcon.tsx(1 hunks)suite-native/icons/tsconfig.json(1 hunks)suite-native/module-authorize-device/src/components/connect/PinOnDevice.tsx(1 hunks)suite-native/module-authorize-device/src/constants/deviceImageConstants.ts(1 hunks)suite-native/module-authorize-device/src/screens/connect/PinScreen.tsx(1 hunks)suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx(1 hunks)suite-native/module-onboarding/package.json(1 hunks)suite-native/module-onboarding/src/components/SecuritySealDescription.tsx(1 hunks)suite-native/module-onboarding/src/components/SecuritySealImages.tsx(1 hunks)suite-native/module-onboarding/src/screens/UninitializedDeviceLandingScreen.tsx(1 hunks)suite-native/module-onboarding/src/screens/WelcomeScreen.tsx(1 hunks)suite-native/module-onboarding/tsconfig.json(1 hunks)suite-native/module-send/package.json(0 hunks)suite-native/module-send/src/components/SlidingFooterOverlay.tsx(1 hunks)suite-native/module-send/tsconfig.json(0 hunks)suite-native/module-trading/package.json(1 hunks)suite-native/module-trading/src/components/general/TradeableAssetButton.tsx(1 hunks)suite-native/module-trading/tsconfig.json(1 hunks)suite-native/navigation/package.json(1 hunks)suite-native/navigation/src/components/ScreenFooterGradient.tsx(1 hunks)suite-native/navigation/tsconfig.json(1 hunks)suite-native/react-native-graph/package.json(1 hunks)suite-native/react-native-graph/src/AnimatedLineGraph.tsx(1 hunks)suite-native/react-native-graph/tsconfig.json(1 hunks)suite-native/receive/src/components/DevicePaginationButton.tsx(1 hunks)suite-native/receive/src/components/DeviceScreenContent.tsx(1 hunks)suite-native/receive/src/components/DeviceScreenPagination.tsx(1 hunks)suite-native/receive/src/types.ts(1 hunks)suite-native/receive/src/utils.ts(1 hunks)suite-native/test-utils/package.json(1 hunks)suite-native/test-utils/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (5)
- suite-native/module-send/tsconfig.json
- suite-common/suite-utils/src/tests/comparison.test.ts
- suite-native/module-send/package.json
- packages/protobuf/src/index.ts
- suite-common/suite-utils/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (139)
- packages/product-components/src/components/PassphraseTypeCard/EnterOnTrezorButton.tsx
- suite-native/module-trading/tsconfig.json
- suite-native/module-send/src/components/SlidingFooterOverlay.tsx
- packages/suite/src/components/suite/DeviceDisplay/parseTextToPagesAndLines.ts
- packages/suite/src/actions/recovery/tests/recoveryActions.test.ts
- packages/suite/src/utils/suite/homescreen.ts
- packages/suite/src/components/onboarding/Hologram.tsx
- packages/suite/src/views/onboarding/steps/Recovery/RecoveryStepBox.tsx
- packages/suite/src/types/onboarding/index.ts
- suite-native/module-trading/package.json
- suite-native/receive/src/components/DeviceScreenContent.tsx
- suite-common/wallet-core/src/device/deviceConstants.ts
- suite-native/navigation/package.json
- packages/suite/src/utils/device/modelUtils.ts
- suite-native/module-onboarding/src/components/SecuritySealDescription.tsx
- suite-native/module-onboarding/src/screens/WelcomeScreen.tsx
- suite-native/icons/tsconfig.json
- packages/suite/src/hooks/wallet/useSendFormCompose.ts
- suite-native/receive/src/types.ts
- packages/suite/src/views/wallet/staking/components/CardanoStakingDashboard.tsx
- packages/suite/src/config/onboarding/steps.ts
- packages/suite/src/actions/firmware/tests/firmwareActions.test.ts
- packages/suite/src/views/wallet/staking/components/DeviceButton.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.tsx
- suite-native/module-onboarding/src/components/SecuritySealImages.tsx
- packages/suite/src/actions/backup/tests/backupActions.test.ts
- suite-common/suite-types/src/guide.ts
- packages/suite/src/components/suite/SecurityCheck/SecurityCheckLayout.tsx
- packages/suite/src/components/firmware/ReconnectDevicePrompt.tsx
- packages/suite/src/views/onboarding/steps/SelectBackupType/SelectBackupType.tsx
- packages/suite/src/hooks/wallet/trading/form/common/useTradingFormActions.ts
- packages/suite/src/utils/suite/fixtures/homescreen.ts
- packages/product-components/package.json
- packages/suite/src/components/suite/DeviceDisplay/tests/parseTextToPagesAndLines.test.ts
- packages/components/src/components/animations/DeviceAnimation.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.tsx
- suite-native/module-trading/src/components/general/TradeableAssetButton.tsx
- packages/suite/src/views/wallet/staking/components/CardanoRewards.tsx
- packages/connect/src/device/tests/checkFirmwareRevision.test.ts
- packages/connect/src/data/models.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingBuyForm.tsx
- packages/suite/src/utils/suite/fixtures/device.ts
- packages/components/src/components/buttons/buttonStyleUtils.ts
- suite-common/suite-utils/src/tests/device.test.ts
- suite-common/wallet-config/tsconfig.json
- packages/suite/src/views/wallet/staking/components/CardanoStake.tsx
- packages/product-components/tsconfig.json
- packages/suite/src/components/suite/DeviceDisplay/DisplayPaginatedText.tsx
- suite-common/firmware/src/hooks/useFirmwareInstallation.ts
- packages/suite/src/components/suite/DeviceMatrixExplanation.tsx
- packages/suite/src/utils/firmware/index.ts
- packages/suite/src/views/onboarding/steps/ResetDevice.tsx
- suite-common/message-system/src/fixtures/messageSystemUtils.ts
- packages/suite/src/utils/firmware/tests/firmware.test.ts
- packages/connect/src/utils/assetUtils.ts
- suite-native/test-utils/tsconfig.json
- packages/suite/src/views/wallet/staking/components/CardanoRedelegate.tsx
- suite-native/react-native-graph/tsconfig.json
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDevice.stories.tsx
- packages/product-components/src/components/PassphraseTypeCard/PassphraseTypeCard.tsx
- packages/suite/src/constants/suite/homescreens.ts
- suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx
- suite-native/device/src/types.ts
- suite-native/icons/src/DeviceModelIcon.tsx
- suite-native/firmware/src/hooks/useFirmwareAnalytics.ts
- packages/suite/src/components/suite/HomescreenGallery.tsx
- suite-common/test-utils/src/mocks.ts
- packages/suite/src/components/suite/WordInputAdvanced.tsx
- suite-native/module-authorize-device/src/screens/connect/PinScreen.tsx
- suite-native/icons/package.json
- packages/components/src/components/animations/LottieAnimation.tsx
- packages/product-components/src/components/ConfirmOnDevice/ConfirmOnDeviceContent.tsx
- packages/product-components/src/components/RotateDeviceImage/RotateDeviceImage.stories.tsx
- suite-native/device/src/utils.ts
- packages/connect/src/device/Device.ts
- suite-native/receive/src/components/DevicePaginationButton.tsx
- packages/connect/src/types/firmware.ts
- suite-native/test-utils/package.json
- suite-native/receive/src/components/DeviceScreenPagination.tsx
- suite-common/suite-constants/src/device.ts
- suite-native/module-authorize-device/src/components/connect/PinOnDevice.tsx
- suite-native/react-native-graph/package.json
- packages/suite/src/hooks/settings/useNetworkSupport.ts
- suite-common/wallet-config/package.json
- packages/suite/src/views/settings/SettingsDevice/Homescreen.tsx
- packages/product-components/src/utils/mapTrezorModelToIcon.ts
- packages/connect/src/api/ethereum/api/ethereumSignTypedData.ts
- packages/suite/src/storage/migrations/index.ts
- packages/components/tsconfig.json
- suite-native/module-authorize-device/src/constants/deviceImageConstants.ts
- packages/suite/src/constants/suite/device.ts
- packages/connect/src/data/tests/firmwareInfo.test.ts
- packages/suite/src/components/suite/DeviceDisplay/DeviceDisplay.tsx
- packages/suite/src/actions/recovery/recoveryActions.ts
- packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceStatus.tsx
- packages/suite/src/utils/device/tests/modelUtils.test.ts
- suite-common/wallet-config/src/types.ts
- packages/suite/src/hooks/wallet/useStakeEthForm.ts
- packages/components/src/components/VirtualizedList/VirtualizedList.tsx
- suite-native/module-onboarding/tsconfig.json
- packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts
- suite-native/receive/src/utils.ts
- packages/suite/src/hooks/wallet/useUnstakeEthForm.ts
- packages/suite/src/utils/onboarding/tests/steps.test.ts
- packages/components/package.json
- packages/suite/src/actions/wallet/selectedAccountActions.ts
- packages/connect/src/api/common/paramsValidator.ts
- packages/connect/src/api/common/fixtures/paramsValidator.ts
- packages/device-utils/src/index.ts
- packages/suite/src/middlewares/suite/suiteMiddleware.ts
- packages/suite/src/views/recovery/index.tsx
- packages/connect/src/types/coinInfo.ts
- suite-native/device/src/components/DeviceImage.tsx
- suite-native/module-onboarding/src/screens/UninitializedDeviceLandingScreen.tsx
- packages/suite/src/components/suite/DeviceDisplay/tests/DeviceDisplay.test.tsx
- packages/suite/src/components/suite/PinMatrix/PinMatrix.tsx
- packages/protobuf/scripts/protobuf-types.ts
- packages/utils/src/index.ts
- packages/suite/src/actions/firmware/fixtures/firmwareActions.ts
- packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts
- suite-common/wallet-core/src/send/sendFormReducer.ts
- suite-common/wallet-config/src/networksConfig.ts
- packages/connect/src/data/firmwareInfo.ts
- suite-native/navigation/src/components/ScreenFooterGradient.tsx
- suite-native/react-native-graph/src/AnimatedLineGraph.tsx
- suite-native/analytics/src/types.ts
- packages/suite/src/views/settings/SettingsDevice/DisplayRotation.tsx
- packages/connect/e2e/fixtures/getFeatures.ts
- suite-native/module-onboarding/package.json
- packages/connect/src/data/DataManager.ts
- suite-native/analytics/src/events.ts
- suite-native/navigation/tsconfig.json
- packages/suite/src/utils/suite/tests/homescreen.test.ts
- packages/connect/src/utils/deviceFeaturesUtils.ts
- suite-common/wallet-core/src/device/deviceThunks.ts
- packages/connect/src/utils/tests/deviceFeaturesUtils.test.ts
- packages/utils/tests/comparison.test.ts
- packages/suite/src/views/onboarding/steps/Recovery/index.tsx
- suite-common/suite-utils/src/device.ts
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: prepare_android_test_app
- GitHub Check: EAS Update
- GitHub Check: transport-e2e-test
- GitHub Check: build-web
- GitHub Check: test
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (2)
packages/device-utils/src/deviceModelInternalUtils.ts (2)
1-2: Well-structured import.Clean import of
DeviceModelInternalfrom a local module follows good practices and supports the PR goal of centralizing device-related utilities.
3-7: Excellent documentation.The JSDoc comment clearly explains the function's purpose and rationale. It provides valuable context about why T2B1 and T3B1 models should be treated the same in most situations, which helps future maintainers understand this design decision.
…ct-components
I'd say that components and probably also product-components should not depend on connect.