Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@cursor review |
| let fingerprint = u32::from_be_bytes(self.rng.r#gen::<[u8; 4]>()); | ||
|
|
||
| if self.contains(fingerprint) { | ||
| return Err(KeychainError::KeyExists); |
There was a problem hiding this comment.
Random fingerprint collision returns error instead of retrying
Low Severity
add_watch_p2_puzzle_hashes generates a random u32 fingerprint and returns KeychainError::KeyExists if it collides with an existing fingerprint. Since the fingerprint is random and the collision check doesn't retry, users could get an unexplained "Key already exists" error when importing a watch-only wallet, even though the addresses are unique. The add_vault method has the same issue deriving a fingerprint from launcher_id bytes.
| } | ||
| } | ||
| } else { | ||
| info!("Wallet is a vault, skipping automatic key derivation"); |
There was a problem hiding this comment.
Log message says "vault" for watch wallets too
Low Severity
The else branch on the matches!(wallet.info, WalletInfo::Bls { .. }) check covers both Vault and Watch wallet types, but the log message says "Wallet is a vault." Watch wallets would produce a misleading log entry, making debugging harder.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| const handleCreate = () => { | ||
| toast.success(t`Vault creation is not yet implemented.`); | ||
| }; |
There was a problem hiding this comment.
Success toast used for unimplemented vault features
Medium Severity
toast.success is used to display "not yet implemented" messages in both MintVault and RecoverVault. After completing a multi-step wizard (generating and potentially saving mnemonics), users see a green success notification saying the feature doesn't work. This is actively misleading — toast.info or toast.warning would be appropriate for stub functionality.
Additional Locations (1)
| </h3> | ||
| <p className='break-all text-sm text-muted-foreground'> | ||
| {info.public_key} | ||
| {info.type === 'bls' && info.public_key} |
There was a problem hiding this comment.
Details dialog shows empty Public Key for non-BLS
Low Severity
The "Wallet Details" dialog unconditionally renders the "Public Key" heading for all wallet types. For vault and watch wallets, info.type === 'bls' && info.public_key evaluates to false, leaving the heading visible with no content below it. The entire block needs to be conditional on info.type === 'bls'.
1 participant
Note
High Risk
High risk because it introduces new wallet kinds (vault/watch), adds a new database migration and puzzle-kind handling, and refactors core wallet/key APIs across Rust backend, RPC/OpenAPI, Tauri commands, and the React app.
Overview
Adds multi-wallet types by replacing the prior key-centric API model with
WalletRecord/WalletKind(bls,vault,watch), renaming endpoints (import_wallet,get_wallets, etc.) and updating OpenAPI/TypeScript bindings accordingly.Introduces initial vault plumbing end-to-end: new
AssetKind::Vault/CoinKind::Vault, new P2 puzzle kinds (Vault,External) with DB support (including migration0006_vaults.sqland new insert/query paths), and guards to skip derivation logic for non-BLS wallets while preventing selection/spending of vault coins.Updates the frontend to match the new wallet model and adds new routes/pages for Mint Vault, Recover Vault (stubbed), Watch Address (imports read-only addresses), plus a Keys management page; also refactors shared UI with new
DurationInputandMnemonicDisplaycomponents and bumpslucide-react.Written by Cursor Bugbot for commit 200ea89. This will update automatically on new commits. Configure here.