Uh oh!
There was an error while loading. Please reload this page.
Fix/chinese phone hardwarecheck - #986
Conversation
📝 WalkthroughWalkthroughThis PR bumps the ChangesVersion and Build Metadata Updates
Key Generation Failure Recovery and Demotion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/lib/global/controllers/key.ts (1)
173-176: 💤 Low valueVerify factory returns the same manager instance.
#demoteToSoftware()assignsthis.#manager = KeyManagerFactory.get("software"), but line 175 callsKeyManagerFactory.getSoftware().regenerate(). If the factory returns new instances rather than a singleton, these could be different objects, though both would operate on the samelocalStoragekey so it would still work correctly.For clarity and consistency, consider calling
regenerate()on the manager returned by#demoteToSoftware():Suggested refactor
async `#rotateToFreshKey`(): Promise<void> { - await this.#demoteToSoftware();- await KeyManagerFactory.getSoftware().regenerate();+ const manager = await this.#demoteToSoftware();+ await (manager as SoftwareKeyManager).regenerate(); }Note: This would require importing
SoftwareKeyManagertype or addingregenerate()to theKeyManagerinterface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/lib/global/controllers/key.ts` around lines 173 - 176, The method `#rotateToFreshKey` uses KeyManagerFactory.getSoftware().regenerate() which may return a different instance than the one assigned in `#demoteToSoftware`; change `#rotateToFreshKey` to call regenerate() on the manager set by `#demoteToSoftware` (this.#manager) instead of calling KeyManagerFactory.getSoftware() again, and ensure the manager type exposes regenerate() by importing SoftwareKeyManager or adding regenerate() to the KeyManager interface so the code typechecks.infrastructure/eid-wallet/src-tauri/Cargo.toml (1)
3-3: ⚡ Quick winClarify semver intent for the eid-wallet version bump (looks like app release alignment)
infrastructure/eid-wallet/src-tauri/Cargo.tomlsets the crate[package] versionto1.0.1, and the same1.0.1value is also used in the app-level manifests/metadata (infrastructure/eid-wallet/package.json,src-tauri/tauri.conf.json, and generated iOSInfo.plist/xcodeprojmarketing/current versions). The remaining0.1.0references insrc-tauriare for a dependency (tauri-plugin-crypto-hw = "0.1.0") /Cargo.lock, not theeid-walletcrate version. Document whether this Rust version is intended purely as app release numbering (to avoid readers interpreting0.x -> 1.xas a Rust API semver/breaking-change signal).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src-tauri/Cargo.toml` at line 3, Add a clarifying comment and/or crate metadata near the Cargo.toml package version to state that the package version "1.0.1" is aligned with the app release/marketing version rather than indicating a Rust API semver/breaking-change intent; update the `[package] version` entry (the "version" field in infrastructure/eid-wallet/src-tauri/Cargo.toml) by inserting a short TOML comment above it (or add a `package.metadata` key) that explicitly documents this convention so readers know the bump is for app release alignment and not a crate API semver change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@infrastructure/eid-wallet/src-tauri/Cargo.toml`:
- Line 3: Add a clarifying comment and/or crate metadata near the Cargo.toml
package version to state that the package version "1.0.1" is aligned with the
app release/marketing version rather than indicating a Rust API
semver/breaking-change intent; update the `[package] version` entry (the
"version" field in infrastructure/eid-wallet/src-tauri/Cargo.toml) by inserting
a short TOML comment above it (or add a `package.metadata` key) that explicitly
documents this convention so readers know the bump is for app release alignment
and not a crate API semver change.
In `@infrastructure/eid-wallet/src/lib/global/controllers/key.ts`:
- Around line 173-176: The method `#rotateToFreshKey` uses
KeyManagerFactory.getSoftware().regenerate() which may return a different
instance than the one assigned in `#demoteToSoftware`; change `#rotateToFreshKey` to
call regenerate() on the manager set by `#demoteToSoftware` (this.#manager)
instead of calling KeyManagerFactory.getSoftware() again, and ensure the manager
type exposes regenerate() by importing SoftwareKeyManager or adding regenerate()
to the KeyManager interface so the code typechecks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f35588fd-019c-4d7f-adf1-c3b0e1354146
⛔ Files ignored due to path filters (4)
infrastructure/eid-wallet/src-tauri/Cargo.lockis excluded by!**/*.lockinfrastructure/eid-wallet/src-tauri/gen/apple/eid-wallet.xcodeproj/project.pbxprojis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/apple/eid-wallet_iOS/Info.plistis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/apple/project.ymlis excluded by!**/gen/**
📒 Files selected for processing (5)
infrastructure/eid-wallet/package.jsoninfrastructure/eid-wallet/src-tauri/Cargo.tomlinfrastructure/eid-wallet/src-tauri/tauri.conf.jsoninfrastructure/eid-wallet/src/lib/crypto/SoftwareKeyManager.tsinfrastructure/eid-wallet/src/lib/global/controllers/key.ts
Uh oh!
There was an error while loading. Please reload this page.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Release Notes
Chores
New Features
Bug Fixes