feat: support change passphrasae pin #374
Conversation
WalkthroughThis update adds new functions for session ID retrieval and passphrase PIN changes. It refines session and PIN management, updates interrupt handling for hardware security operations, and improves user interface flows for PIN and passphrase actions. Multiple new translation strings are added. Debug-only error handling is removed in favor of always raising exceptions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
core/src/trezor/lvglui/i18n/locales/de.py (1)
1024-1024: German phrasing off.core/src/trezor/lvglui/i18n/locales/fr.py (1)
1032-1032: Wrong French form.core/src/apps/common/request_pin.py (1)
365-377: Remove the commented-out code.Dead code clutters the file.
core/src/apps/base.py (1)
270-279: Combine if branches.These branches can be merged using
or.- elif has_attach and session_id_in_msg is not None and passphrase_state is None: - session_id = storage.cache.start_session() - elif device_is_unlocked() and storage.device.is_passphrase_pin_enabled(): - session_id = storage.cache.start_session() + elif (has_attach and session_id_in_msg is not None and passphrase_state is None) or (device_is_unlocked() and storage.device.is_passphrase_pin_enabled()): + session_id = storage.cache.start_session()core/src/trezor/lvglui/i18n/locales/ko.py (1)
1032-1032: Wrong verb form
"이해하다" is not a complete prompt.core/src/trezor/lvglui/i18n/locales/pt_br.py (1)
1032-1032: Verbo isolado
"Entender" soa inacabado.core/src/trezor/lvglui/i18n/locales/ja.py (1)
1026-1026: 表現ぎこちない
"ネットワークで委任" が不自然。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
core/mocks/generated/trezorcrypto/se_thd89.pyiis excluded by!**/generated/**
📒 Files selected for processing (34)
core/embed/extmod/modtrezorcrypto/modtrezorcrypto-se-thd89.h(4 hunks)core/embed/trezorhal/se_thd89.c(4 hunks)core/embed/trezorhal/se_thd89.h(3 hunks)core/embed/trezorhal/thd89.c(2 hunks)core/embed/trezorhal/thd89.h(1 hunks)core/src/apps/base.py(5 hunks)core/src/apps/common/passphrase.py(1 hunks)core/src/apps/common/pin_constants.py(1 hunks)core/src/apps/common/request_pin.py(5 hunks)core/src/apps/management/apply_settings.py(1 hunks)core/src/apps/management/change_pin.py(2 hunks)core/src/apps/ur_registry/chains/bitcoin/transaction.py(3 hunks)core/src/apps/ur_registry/chains/ethereum/eth_sign_request.py(1 hunks)core/src/apps/ur_registry/chains/hardware_requests/hardware_call.py(1 hunks)core/src/apps/ur_registry/chains/solana/sol_sign_request.py(1 hunks)core/src/storage/cache.py(2 hunks)core/src/storage/device.py(2 hunks)core/src/trezor/lvglui/i18n/keys.py(3 hunks)core/src/trezor/lvglui/i18n/locales/de.py(1 hunks)core/src/trezor/lvglui/i18n/locales/en.py(2 hunks)core/src/trezor/lvglui/i18n/locales/es.py(1 hunks)core/src/trezor/lvglui/i18n/locales/fr.py(1 hunks)core/src/trezor/lvglui/i18n/locales/it.py(1 hunks)core/src/trezor/lvglui/i18n/locales/ja.py(1 hunks)core/src/trezor/lvglui/i18n/locales/ko.py(1 hunks)core/src/trezor/lvglui/i18n/locales/pt_br.py(1 hunks)core/src/trezor/lvglui/i18n/locales/ru.py(1 hunks)core/src/trezor/lvglui/i18n/locales/zh_cn.py(1 hunks)core/src/trezor/lvglui/i18n/locales/zh_hk.py(1 hunks)core/src/trezor/lvglui/scrs/components/keyboard.py(1 hunks)core/src/trezor/lvglui/scrs/homescreen.py(1 hunks)core/src/trezor/lvglui/scrs/lockscreen.py(0 hunks)core/src/trezor/lvglui/scrs/pinscreen.py(1 hunks)core/src/trezor/uart.py(1 hunks)
💤 Files with no reviewable changes (1)
- core/src/trezor/lvglui/scrs/lockscreen.py
🧰 Additional context used
🧬 Code Graph Analysis (8)
core/src/apps/ur_registry/chains/ethereum/eth_sign_request.py (2)
core/src/apps/ur_registry/chains/__init__.py (1)
MismatchError(7-9)core/src/apps/common/keychain.py (1)
root_fingerprint(147-156)
core/src/apps/management/apply_settings.py (3)
core/src/storage/device.py (2)
is_passphrase_pin_enabled(1423-1431)set_passphrase_pin_enabled(1434-1448)core/src/apps/common/passphrase.py (1)
is_passphrase_pin_enabled(15-16)core/src/apps/base.py (1)
lock_device(544-562)
core/src/apps/ur_registry/chains/bitcoin/transaction.py (1)
core/src/apps/ur_registry/chains/__init__.py (1)
MismatchError(7-9)
core/src/apps/ur_registry/chains/hardware_requests/hardware_call.py (2)
core/src/apps/ur_registry/chains/__init__.py (1)
MismatchError(7-9)core/src/apps/common/keychain.py (1)
root_fingerprint(147-156)
core/src/apps/common/passphrase.py (1)
core/src/apps/common/pin_constants.py (1)
PinType(17-24)
core/src/apps/ur_registry/chains/solana/sol_sign_request.py (2)
core/src/apps/ur_registry/chains/__init__.py (1)
MismatchError(7-9)core/src/apps/common/keychain.py (1)
root_fingerprint(147-156)
core/src/trezor/uart.py (2)
core/src/storage/device.py (2)
is_passphrase_pin_enabled(1423-1431)set_passphrase_pin_enabled(1434-1448)core/src/apps/common/passphrase.py (1)
is_passphrase_pin_enabled(15-16)
core/embed/trezorhal/thd89.c (1)
core/embed/trezorhal/irq.h (2)
disable_irq(64-68)enable_irq(60-62)
🪛 Ruff (0.12.2)
core/src/apps/base.py
270-273: Combine if branches using logical or operator
Combine if branches
(SIM114)
276-279: Combine if branches using logical or operator
Combine if branches
(SIM114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Defs check
- GitHub Check: Style check
- GitHub Check: Gen check
🔇 Additional comments (32)
core/src/apps/ur_registry/chains/ethereum/eth_sign_request.py (1)
217-219: LGTM: Clean error handling standardization.Removes debug-conditional behavior. Always raises exceptions now.
core/src/apps/ur_registry/chains/solana/sol_sign_request.py (1)
201-203: LGTM: Consistent with Ethereum changes.Same error handling pattern. Good consistency.
core/src/apps/ur_registry/chains/hardware_requests/hardware_call.py (1)
78-80: LGTM: Hex string format appropriate for hardware calls.Consistent error handling. Different format makes sense here.
core/src/apps/ur_registry/chains/bitcoin/transaction.py (3)
358-360: LGTM: ECDSA script type validation standardized.Consistent error handling improvement.
376-378: LGTM: Schnorr script type validation standardized.Same pattern as ECDSA handling above.
427-429: LGTM: Output processing validation standardized.Completes the fingerprint validation standardization.
core/embed/trezorhal/thd89.h (1)
14-14: LGTM!Proper external variable declaration for interrupt nesting tracking.
core/src/apps/common/pin_constants.py (1)
23-24: LGTM!New PIN type constant properly added with correct MAX update.
core/src/apps/common/passphrase.py (1)
83-88: LGTM!Explicit fingerprint disabling improves authentication flow control.
core/src/trezor/uart.py (1)
152-153: LGTM!Proper passphrase PIN state management on fingerprint match.
core/src/storage/cache.py (2)
36-36: LGTM!New sessionless cache key follows established pattern.
148-148: LGTM!Correct field size allocation for the new cache key.
core/src/trezor/lvglui/i18n/locales/zh_cn.py (1)
1022-1032: LGTM!New translation strings are properly formatted and consistently added.
core/src/apps/management/apply_settings.py (1)
78-82: LGTM!Proper security logic - disabling passphrase PIN when passphrase is disabled and immediately locking device.
core/src/trezor/lvglui/scrs/pinscreen.py (1)
556-568: LGTM!Standard PIN confirmation flow correctly implemented with proper cancellation handling.
core/src/trezor/lvglui/scrs/components/keyboard.py (1)
555-555: LGTM!Good improvement - using configurable minimum length instead of hardcoded value.
core/src/trezor/lvglui/scrs/homescreen.py (1)
4318-4318: LGTM!The PIN use type change aligns with the enhanced passphrase PIN management mentioned in the PR objectives.
core/src/trezor/lvglui/i18n/locales/it.py (1)
1022-1032: Translation strings added correctly.The new Italian translation strings are properly formatted and integrated into the translations array.
core/src/trezor/lvglui/i18n/locales/en.py (2)
922-922: Good punctuation fix.Replacing the typographic apostrophe with a standard apostrophe improves consistency.
1022-1032: New translation strings added correctly.The English translation strings are properly formatted and match the Italian translations in structure.
core/embed/trezorhal/se_thd89.h (3)
43-43: New PIN type enum added correctly.The new
PIN_TYPE_USER_AND_PASSPHRASE_PIN_CHECKfollows the existing naming pattern and fits logically in the enum sequence.
124-124: Function declaration added correctly.The
se_change_pin_passphrasefunction signature follows existing conventions with proper parameter types and return type.
142-142: Function declaration added correctly.The
se_session_get_current_idfunction signature is consistent with other SE session functions.core/embed/trezorhal/thd89.c (1)
581-591: Add error handling for interrupt nesting.If
_thd89_transmit_exfails,thd89_irq_nestwon't decrement. Interrupts stay disabled.Apply this fix:
secbool thd89_transmit_ex(uint8_t addr, uint8_t *cmd, uint16_t len, uint8_t *resp, uint16_t *resp_len) { uint32_t irq = disable_irq(); thd89_irq_nest++; secbool result = _thd89_transmit_ex(addr, cmd, len, resp, resp_len); thd89_irq_nest--; - if (thd89_irq_nest == 0) { + if (thd89_irq_nest <= 0) { + thd89_irq_nest = 0; // Reset to 0 in case of underflow enable_irq(irq); } return result; }Likely an incorrect or invalid review comment.
core/src/trezor/lvglui/i18n/keys.py (1)
2018-2020: Fix typo: "youll" should be "you'll"Line 2019 contains a typo. Add the apostrophe back.
-# youll need the PIN of the standard wallet. +# you'll need the PIN of the standard wallet.Likely an incorrect or invalid review comment.
core/embed/trezorhal/se_thd89.c (5)
6-6: LGTM!IRQ nesting implementation is correct and follows standard patterns.
Also applies to: 267-276
281-291: LGTM!Consistent IRQ management implementation.
1497-1522: Verify PIN_MAX_LENGTH constant usage.Function logic is sound but confirm PIN_MAX_LENGTH applies to passphrase PINs.
1524-1536: LGTM!Follows established dual-address pattern correctly.
2019-2025: LGTM!Simple session ID retrieval implementation is correct.
core/src/storage/device.py (2)
1428-1431: LGTM!Clean cache-based implementation. Logic is correct.
1447-1448: LGTM!Proper cache storage with correct byte encoding.
4 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Other