fix v1 and legacy decryption, fix status command, add key conversion helpers#1
fix v1 and legacy decryption, fix status command, add key conversion helpers#1AustinKelsay merged 2 commits intomasterfrom
Conversation
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds a new top-level "keys" CLI route with interactive conversion (npub/nsec/hex) components and help text; documents the new command and README updates. Expands peer diagnostics with a compatibility fallback path. Updates crypto/share handling to support multiple IV/salt lengths and returns ivLength; renames exported salt-length constant. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant CLI as App router (src/App.tsx)
participant KH as KeyHelp
participant KC as KeyConvert
participant IC as igloo-core utils
U->>CLI: igloo-cli keys [convert] [flags/args]
alt subcommand=convert or key flags present
CLI->>KC: render KeyConvert(flags,args)
KC->>KC: detect input / resolve conflicts
KC->>IC: validate/convert (npub ↔ nsec ↔ hex)
IC-->>KC: converted values
KC-->>U: display conversion rows
else no subcommand and no inputs
CLI->>KH: render KeyHelp()
KH-->>U: usage hints
end
sequenceDiagram
actor U as User
participant KS as KeysetStatus.startDiagnostics
participant COMP as checkPeerStatusCompat
participant STD as checkPeerStatus
participant DEC as decodeGroup/decodeShare
participant DER as deriveSelfPubkeyFromPackages
participant CVT as convert_pubkey
participant PING as pingPeer
U->>KS: Start diagnostics
KS->>COMP: checkPeerStatusCompat()
COMP->>STD: try standard checkPeerStatus()
alt standard success
STD-->>COMP: status
COMP-->>KS: status
else standard failure
COMP->>DEC: decode local packages
DEC-->>COMP: group/share packages
COMP->>DER: derive signer pubkey
DER-->>COMP: signer pubkey
COMP->>CVT: convert to BIP340 ping key
CVT-->>COMP: bip340 key
loop for each peer candidate
COMP->>PING: pingPeer(bip340 key, relays)
PING-->>COMP: result/error
end
alt any ping success
COMP-->>KS: compatibility status
else all fail
COMP-->>KS: combined error
end
end
KS-->>U: results/errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/App.tsx (1)
48-68: Consider refactoring to use a Set for faster lookups.The
hasKeyInputsfunction iterates through an array of key names on every invocation. For better performance, convertstringKeysto aSetoutside the function and reuse it.Apply this diff:
+const KEY_INPUT_FLAGS = new Set([ + 'npub', + 'nsec', + 'hex', + 'hex-public', + 'hex-private', + 'public-hex', + 'private-hex', + 'hexPublic', + 'hexPrivate', + 'publicHex', + 'privateHex', + 'value' +]); + function hasKeyInputs(flags: Record<string, string | boolean>) { - const stringKeys = [ - 'npub', - 'nsec', - 'hex', - 'hex-public', - 'hex-private', - 'public-hex', - 'private-hex', - 'hexPublic', - 'hexPrivate', - 'publicHex', - 'privateHex', - 'value' - ]; - - return stringKeys.some(key => { + return Array.from(KEY_INPUT_FLAGS).some(key => { const value = flags[key]; return typeof value === 'string' && value.trim().length > 0; }); }src/components/keys/KeyConvert.tsx (1)
134-271: Verify the conflict detection logic handles all edge cases.The
detectInputfunction implements comprehensive input detection with conflict resolution. However, the conflict detection at lines 147-150 only triggers when the same type has different values from different sources.Consider this scenario:
- User provides
--npub npub1abc...- User also provides positional
npub npub1xyz...Both would add candidates of type 'npub' with different values, triggering the conflict at line 149. This is correct.
However, verify that the function correctly handles:
- Multiple positional arguments (checked at lines 230-232, 241-243)
- Conflicting type specifications (e.g.,
--from nsec --value <npub-format-value>)The type mismatch between
--fromand actual value format is not explicitly validated. For example:
--from nsec --value npub1abc...would calladdCandidate('nsec', 'npub1abc...', '--from/--value')- This would later fail in
performConversionwhenvalidateNostrKey(rawValue, 'nsec')is calledWhile this eventually produces an error, it would be clearer to validate the format matches the declared type earlier in
detectInput. Consider adding format validation when--fromis used:const fromFlag = normalizeInputType(getStringFlag(flags, 'from')); if (fromFlag) { const valueFlag = getStringFlag(flags, 'value'); if (!valueFlag) { return {error: 'Provide --value when using --from.'}; } // Add format validation here const inferredType = inferInputType(valueFlag); if (inferredType && inferredType !== fromFlag) { return {error: `Value format (${formatTypeLabel(inferredType)}) doesn't match --from type (${formatTypeLabel(fromFlag)}).`}; } addCandidate(fromFlag, valueFlag, '--from/--value'); }This would provide earlier, clearer error messages for format mismatches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.md(2 hunks)llm/context/cli/cli-routing-and-prompts.md(2 hunks)llm/context/cli/igloo-cli-automation-overview.md(1 hunks)llm/context/cli/peer-diagnostics-status.md(1 hunks)llm/context/cli/share-storage-format.md(1 hunks)llm/context/share-file-v1-spec.md(1 hunks)src/App.tsx(4 hunks)src/components/Help.tsx(2 hunks)src/components/Intro.tsx(1 hunks)src/components/keys/KeyConvert.tsx(1 hunks)src/components/keys/KeyHelp.tsx(1 hunks)src/components/keyset/KeysetStatus.tsx(3 hunks)src/components/keyset/ShareSaver.tsx(3 hunks)src/components/share/ShareAdd.tsx(2 hunks)src/keyset/crypto.ts(5 hunks)
🔇 Additional comments (26)
llm/context/share-file-v1-spec.md (1)
75-76: LGTM! Clear compatibility documentation.The note appropriately documents the salt expansion behavior and backward compatibility strategy, helping implementers understand the Desktop/CLI alignment and fallback logic.
llm/context/cli/share-storage-format.md (1)
43-45: LGTM! Comprehensive compatibility documentation.The documentation clearly explains the salt padding strategy and the multi-dimensional retry logic (password encoding, PBKDF2 iterations, IV lengths, salt lengths) that ensures cross-compatibility between CLI and Desktop file formats.
src/components/Intro.tsx (1)
26-26: LGTM! Consistent UI addition.The new command entry follows the existing formatting pattern and clearly describes the key conversion functionality.
llm/context/cli/igloo-cli-automation-overview.md (1)
10-10: LGTM! Clear automation documentation.The documentation effectively explains both the shorthand flags for interactive use and the --from/--value pattern for scripting scenarios.
llm/context/cli/cli-routing-and-prompts.md (2)
7-7: LGTM! Routing documentation addition.The command matrix entry appropriately documents the new keys command and its relationship to KeyConvert/KeyHelp components.
16-16: LGTM! Clear router behavior description.The renderKeys documentation clearly explains the routing logic, shorthand flag handling, and fallback behavior when inputs are missing.
src/components/Help.tsx (2)
22-22: LGTM! Consistent command documentation.The keys command entry follows the existing format and clearly describes its purpose.
39-46: LGTM! Comprehensive option documentation.The flag descriptions clearly explain the key conversion options, covering both shorthand flags (--npub, --nsec, etc.) and the generic --hex/--kind pattern. The text is consistent with the existing help style.
src/components/keys/KeyHelp.tsx (1)
1-20: LGTM! Well-structured help component.The KeyHelp component provides clear, concise examples for key conversion scenarios. The usage of color coding (cyanBright title, gray note) and the layout structure are consistent with the existing CLI UI patterns.
llm/context/cli/peer-diagnostics-status.md (2)
20-20: LGTM! Detailed compatibility fallback documentation.The documentation clearly explains the compatibility strategy: trying checkPeerStatus first, then falling back to local decoding, pubkey derivation, and manual peer pinging when older shares lack pubkey fields.
27-27: LGTM! Comprehensive error handling documentation.The updated error handling description appropriately documents how failures are handled in both the primary and fallback paths.
src/App.tsx (5)
7-8: LGTM!The new imports for
KeyConvertandKeyHelpcomponents are properly structured and follow the existing import pattern in the file.
70-80: LGTM!The
KEY_TYPE_SUBCOMMANDSSet provides an efficient lookup mechanism for key-related subcommands and correctly includes all supported key type aliases.
144-145: LGTM!The new 'keys' case properly integrates into the existing command routing structure and delegates to the
renderKeysfunction.
155-156: LGTM!The updated command examples appropriately showcase both the existing
signercommand and the newkeys convertfunctionality.
82-99: Ignore args slicing inconsistency. Slicing off"convert"but passing full args for type subcommands is intentional—detectInputexpects the first element to be a valid type when invoked via a type subcommand and handles both patterns correctly.Likely an incorrect or invalid review comment.
src/components/keys/KeyConvert.tsx (10)
1-11: LGTM!The imports are well-structured and properly utilize the key validation and conversion utilities from
@frostr/igloo-core.
13-42: LGTM!The type definitions are comprehensive and clearly document the data structures used throughout the conversion flow.
44-57: LGTM!The
TYPE_ALIASESmapping provides good coverage of common key type aliases and variations, improving user experience by accepting multiple input formats.
59-65: LGTM!The
normalizeInputTypefunction correctly handles undefined inputs and performs case-insensitive alias resolution.
67-73: LGTM!The
stripHexPrefixfunction properly handles both lowercase and uppercase '0x' prefixes while preserving the original value when no prefix exists.
75-91: LGTM!The flag extraction helper functions (
getStringFlagandgetFlagValue) provide clean abstractions for safely retrieving string values from the flags object.
119-132: LGTM!The
formatTypeLabelfunction provides user-friendly labels for each input type with appropriate default handling.
273-332: LGTM with one observation.The conversion logic correctly handles all four input types and performs appropriate validations. The use of
derivePublicKeyfor nsec and hex-private conversions properly generates all related key formats.One observation: The function calls
stripHexPrefixfor hex inputs (lines 299, 310) but this has already been done in the inference logic. While this double-stripping is harmless (the second call is idempotent), it's redundant sincerawValueshould already be normalized at this point.
334-341: LGTM!The error rendering function provides clear feedback with usage guidance. The usage message accurately reflects the supported command syntax.
343-377: LGTM!The main component properly uses
useMemoto avoid redundant detection on every render, implements comprehensive error handling, and provides clear, user-friendly output formatting.
…HexKey triggers a clearer error that tells users to add --from hex-public|hex-private or pair --hex with --kind public|private (src/components/ keys/KeyConvert.tsx:64, src/components/keys/KeyConvert.tsx:132). - Updated automation docs so downstream agents know hex inputs must declare their kind (llm/context/cli/ igloo-cli-automation-overview.md:6).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/keys/KeyConvert.tsx (1)
337-344: Consider expanding the usage hint.The current usage hint only shows
--from/--valuepattern. Consider mentioning other supported input methods for better user guidance:function renderError(message: string) { return ( <Box flexDirection="column"> <Text color="red">{message}</Text> - <Text color="gray">Usage: igloo-cli keys convert --from nsec|npub|hex-public|hex-private --value <key></Text> + <Text color="gray">Usage: igloo-cli keys convert [OPTIONS] [TYPE] [KEY]</Text> + <Text color="gray">Options: --npub, --nsec, --hex-public, --hex-private, --hex/--kind, --from/--value</Text> </Box> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
llm/context/cli/igloo-cli-automation-overview.md(1 hunks)src/components/keys/KeyConvert.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- llm/context/cli/igloo-cli-automation-overview.md
🔇 Additional comments (15)
src/components/keys/KeyConvert.tsx (15)
1-11: LGTM!The imports are necessary and appropriately scoped. All imported functions are used throughout the component.
13-42: LGTM!The type definitions are well-structured and support the component's input detection, conflict tracking, and conversion logic.
44-57: LGTM!The alias mapping is comprehensive and user-friendly, supporting multiple common variations for each key type.
93-106: Past review concern addressed.The inference fallback for 64-character hex strings has been removed as recommended.
inferInputTypenow only recognizesnpub1andnsec1prefixes, returningundefinedfor raw hex inputs. This eliminates the unreliable trial-conversion logic that could misclassify keys.The callers (lines 165-177, 232-243) correctly handle
undefinedby checkingisRawHexKeyand prompting the user to specify the key type explicitly via--fromor--hex/--kindflags.
154-161: LGTM!The
--from/--valueflag validation is correct. It ensures both flags are provided together and adds the candidate appropriately.
163-177: LGTM!The
--valuealone logic correctly infers type from npub/nsec prefixes and provides clear, actionable error messages for raw hex inputs or unrecognized formats.
179-207: LGTM!The direct flag handling for
--nsec,--npub,--hex-private, and--hex-publicis correct. Support for multiple flag name variations (kebab-case and camelCase) enhances usability.
209-218: LGTM!The
--hex/--kindvalidation is correct. It ensures both flags are provided together and restricts--kindtopublicorprivate(which normalize tohex-publicorhex-private), rejecting invalid combinations like--kind npub.
220-248: LGTM!The positional argument handling correctly supports both
[type, value]and[value]patterns, with clear error messages for raw hex inputs and appropriate argument count validation.
250-274: LGTM!The conflict detection and final validation logic is robust. It ensures exactly one key input per invocation and provides clear, detailed error messages for conflicts or missing inputs.
278-286: LGTM!The npub conversion logic is correct: validates the key and converts to public hex.
301-311: LGTM!The hex-public conversion correctly strips the optional 0x prefix, validates the key, and converts to npub. The note for prefix removal enhances transparency.
312-327: LGTM!The hex-private conversion correctly strips the optional 0x prefix, validates the key, converts to nsec, and derives the public key from the hex private key.
346-380: LGTM!The
KeyConvertcomponent is well-structured with appropriate memoization, comprehensive error handling for both detection and conversion phases, and clear rendering logic.The try-catch block (lines 357-379) correctly handles validation and conversion errors that may be thrown by
@frostr/igloo-corefunctions.
287-300: Ignore derivePublicKey input format concern derivePublicKey accepts both hex and nsec private keys per @frostr/igloo-core documentation, so no change is needed.Likely an incorrect or invalid review comment.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation