feat: collapsable address/tx in auth signer #183
Conversation
WalkthroughThe pull request introduces enhancements to multiple components, focusing on improving flexibility, user interaction, and data presentation. The changes include adding a size prop to the Changes
Sequence DiagramsequenceDiagram
participant User
participant AuthSignerModal
participant MessageDecoder
participant SyntaxHighlighter
User->>AuthSignerModal: Interact with modal
AuthSignerModal->>MessageDecoder: Decode message
MessageDecoder-->>AuthSignerModal: Decoded message
AuthSignerModal->>SyntaxHighlighter: Format decoded message
SyntaxHighlighter-->>AuthSignerModal: Formatted message
AuthSignerModal->>User: Display expandable message
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
fixes #176 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
==========================================
- Coverage 55.04% 54.73% -0.31%
==========================================
Files 153 153
Lines 15515 15645 +130
==========================================
+ Hits 8540 8564 +24
- Misses 6975 7081 +106 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
components/react/authSignerModal.tsx (1)
Add test coverage for critical transaction signing components
The review comment is valid. The codebase has a robust testing infrastructure, but the
authSignerModal.tsxcomponent lacks test coverage despite containing security-critical functionality:
- Message decoders for multiple Cosmos SDK transaction types
- Transaction signing UI components
- Error handling for decoding failures
- Theme-aware display components
Given the security-sensitive nature of transaction signing and message decoding, comprehensive test coverage is essential to prevent potential vulnerabilities and ensure reliable operation.
🔗 Analysis chain
Line range hint
39-301: Add comprehensive test coverage.The static analysis shows significant gaps in test coverage. Please add tests for:
- Message decoders
- Value formatting
- UI interactions
- Theme handling
Run this script to identify critical paths requiring test coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all exported functions and components that need test coverage # Find all exported entities rg -l "export (const|function|class|interface)" | while read -r file; do echo "=== $file ===" rg "export (const|function|class|interface)" "$file" done # Find React components with props ast-grep --pattern 'interface $name Props'Length of output: 29392
Script:
#!/bin/bash # Search for test files related to authSignerModal and its components echo "=== Searching for test files ===" fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts echo -e "\n=== Searching for test utilities ===" rg -l "render.*Provider|test-utils|jest.setup|vitest" echo -e "\n=== Checking imports of testing libraries ===" rg -l "import.*[@]testing-library|import.*[@]jest|import.*vitest" echo -e "\n=== Looking for message decoder tests ===" rg -l "describe.*message.*decoder|test.*message.*decoder|it.*message.*decoder"Length of output: 5902
Script:
#!/bin/bash # Examine test utilities and decoder-related tests echo "=== Examining test utilities ===" cat tests/render.tsx echo -e "\n=== Searching for decoder-related tests ===" rg -l "decode|TxBody|AuthInfo|MsgSend" echo -e "\n=== Searching for indirect tests ===" rg -l "SignModal|DisplayDataToSign|messageDecoders" echo -e "\n=== Looking for Cosmos SDK message type tests ===" rg -l "cosmos.bank.v1beta1|cosmos.group.v1|cosmos.upgrade.v1beta1"Length of output: 2962
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 41-42: components/react/authSignerModal.tsx#L41-L42
Added lines #L41 - L42 were not covered by tests
[warning] 45-46: components/react/authSignerModal.tsx#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 49-50: components/react/authSignerModal.tsx#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 53-54: components/react/authSignerModal.tsx#L53-L54
Added lines #L53 - L54 were not covered by tests
[warning] 57-58: components/react/authSignerModal.tsx#L57-L58
Added lines #L57 - L58 were not covered by tests
[warning] 61-62: components/react/authSignerModal.tsx#L61-L62
Added lines #L61 - L62 were not covered by tests
[warning] 65-66: components/react/authSignerModal.tsx#L65-L66
Added lines #L65 - L66 were not covered by tests
[warning] 69-70: components/react/authSignerModal.tsx#L69-L70
Added lines #L69 - L70 were not covered by tests
[warning] 73-74: components/react/authSignerModal.tsx#L73-L74
Added lines #L73 - L74 were not covered by tests
[warning] 77-78: components/react/authSignerModal.tsx#L77-L78
Added lines #L77 - L78 were not covered by tests
[warning] 81-82: components/react/authSignerModal.tsx#L81-L82
Added lines #L81 - L82 were not covered by tests
[warning] 85-86: components/react/authSignerModal.tsx#L85-L86
Added lines #L85 - L86 were not covered by tests
[warning] 89-90: components/react/authSignerModal.tsx#L89-L90
Added lines #L89 - L90 were not covered by tests
[warning] 93-94: components/react/authSignerModal.tsx#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 105-105: components/react/authSignerModal.tsx#L105
Added line #L105 was not covered by tests
[warning] 110-110: components/react/authSignerModal.tsx#L110
Added line #L110 was not covered by tests
[warning] 112-114: components/react/authSignerModal.tsx#L112-L114
Added lines #L112 - L114 were not covered by tests
[warning] 119-151: components/react/authSignerModal.tsx#L119-L151
Added lines #L119 - L151 were not covered by tests
🧹 Nitpick comments (3)
components/messageSyntax.tsx (1)
35-53: Consider refactoring to reduce code duplication.The
prettyPrintJSONfunction and styling configuration are duplicated betweenmessageSyntaxandobjectSyntax. Consider extracting these into shared utilities.+const prettyPrintJSON = (obj: Record<string, any>): string => { + return JSON.stringify(obj, null, 2); +}; + +const getSyntaxHighlighterProps = (theme: string) => ({ + language: "json", + style: theme === 'dark' ? oneDark : oneLight, + customStyle: { + backgroundColor: 'transparent', + padding: '1rem', + borderRadius: '0.5rem', + } +}); + export function messageSyntax(fieldsToShow: string[], message: MessageType, theme: string) { - const prettyPrintJSON = (obj: Record<string, any>): string => { - return JSON.stringify(obj, null, 2); - }; - return ( <SyntaxHighlighter - language="json" - style={theme === 'dark' ? oneDark : oneLight} - customStyle={{ - backgroundColor: 'transparent', - padding: '1rem', - borderRadius: '0.5rem', - }} + {...getSyntaxHighlighterProps(theme)} > {prettyPrintJSON( fieldsToShow.reduce((acc: Record<string, any>, field: string) => { acc[field] = message[field]; return acc; }, {}) )} </SyntaxHighlighter> ); } export function objectSyntax(object: Record<string, any>, theme: string) { - const prettyPrintJSON = (obj: Record<string, any>): string => { - return JSON.stringify(obj, null, 2); - }; - return ( <SyntaxHighlighter - language="json" - style={theme === 'dark' ? oneDark : oneLight} - customStyle={{ - backgroundColor: 'transparent', - padding: '1rem', - borderRadius: '0.5rem', - }} + {...getSyntaxHighlighterProps(theme)} > {prettyPrintJSON(object)} </SyntaxHighlighter> ); }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 35-50: components/messageSyntax.tsx#L35-L50
Added lines #L35 - L50 were not covered by testscomponents/react/authSignerModal.tsx (2)
39-97: Consider implementing a more maintainable message decoder registry.The current implementation of messageDecoders has repetitive code patterns. Consider using a factory pattern or a more declarative approach.
+type MessageDecoder<T> = { + type: string; + decoder: typeof T; +}; + +const createMessageDecoder = <T>({ type, decoder }: MessageDecoder<T>) => ({ + [type]: (value: Uint8Array) => { + const decoded = decoder.decode(value); + return { ...decoded }; + } +}); + +const messageDecoders = { + ...createMessageDecoder({ type: '/cosmos.bank.v1beta1.MsgSend', decoder: MsgSend }), + ...createMessageDecoder({ type: '/cosmos.group.v1.MsgCreateGroupWithPolicy', decoder: MsgCreateGroupWithPolicy }), + // ... other decoders +};🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 41-42: components/react/authSignerModal.tsx#L41-L42
Added lines #L41 - L42 were not covered by tests
[warning] 45-46: components/react/authSignerModal.tsx#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 49-50: components/react/authSignerModal.tsx#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 53-54: components/react/authSignerModal.tsx#L53-L54
Added lines #L53 - L54 were not covered by tests
[warning] 57-58: components/react/authSignerModal.tsx#L57-L58
Added lines #L57 - L58 were not covered by tests
[warning] 61-62: components/react/authSignerModal.tsx#L61-L62
Added lines #L61 - L62 were not covered by tests
[warning] 65-66: components/react/authSignerModal.tsx#L65-L66
Added lines #L65 - L66 were not covered by tests
[warning] 69-70: components/react/authSignerModal.tsx#L69-L70
Added lines #L69 - L70 were not covered by tests
[warning] 73-74: components/react/authSignerModal.tsx#L73-L74
Added lines #L73 - L74 were not covered by tests
[warning] 77-78: components/react/authSignerModal.tsx#L77-L78
Added lines #L77 - L78 were not covered by tests
[warning] 81-82: components/react/authSignerModal.tsx#L81-L82
Added lines #L81 - L82 were not covered by tests
[warning] 85-86: components/react/authSignerModal.tsx#L85-L86
Added lines #L85 - L86 were not covered by tests
[warning] 89-90: components/react/authSignerModal.tsx#L89-L90
Added lines #L89 - L90 were not covered by tests
[warning] 93-94: components/react/authSignerModal.tsx#L93-L94
Added lines #L93 - L94 were not covered by tests
119-151: Enhance error handling in message decoding.The current implementation catches errors but only logs them. Consider:
- Adding structured error types
- Providing more context in error messages
- Implementing retry logic for transient failures
+type DecodingError = { + type: 'DECODE_ERROR' | 'PARSE_ERROR'; + messageType: string; + error: Error; +}; + +const handleDecodingError = (error: Error, messageType: string): DecodingError => ({ + type: 'DECODE_ERROR', + messageType, + error, +}); + try { // ... decoding logic } catch (error) { - console.error(`Failed to decode message of type ${msg.typeUrl}:`, error); + const decodingError = handleDecodingError(error as Error, msg.typeUrl); + console.error('Message decoding failed:', decodingError); return { typeUrl: msg.typeUrl, value: base64Value, + error: decodingError, }; }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 119-151: components/react/authSignerModal.tsx#L119-L151
Added lines #L119 - L151 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/icons/ArrowRightIcon.tsx(1 hunks)components/messageSyntax.tsx(1 hunks)components/react/authSignerModal.tsx(4 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
components/messageSyntax.tsx
[warning] 35-50: components/messageSyntax.tsx#L35-L50
Added lines #L35 - L50 were not covered by tests
components/react/authSignerModal.tsx
[warning] 41-42: components/react/authSignerModal.tsx#L41-L42
Added lines #L41 - L42 were not covered by tests
[warning] 45-46: components/react/authSignerModal.tsx#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 49-50: components/react/authSignerModal.tsx#L49-L50
Added lines #L49 - L50 were not covered by tests
[warning] 53-54: components/react/authSignerModal.tsx#L53-L54
Added lines #L53 - L54 were not covered by tests
[warning] 57-58: components/react/authSignerModal.tsx#L57-L58
Added lines #L57 - L58 were not covered by tests
[warning] 61-62: components/react/authSignerModal.tsx#L61-L62
Added lines #L61 - L62 were not covered by tests
[warning] 65-66: components/react/authSignerModal.tsx#L65-L66
Added lines #L65 - L66 were not covered by tests
[warning] 69-70: components/react/authSignerModal.tsx#L69-L70
Added lines #L69 - L70 were not covered by tests
[warning] 73-74: components/react/authSignerModal.tsx#L73-L74
Added lines #L73 - L74 were not covered by tests
[warning] 77-78: components/react/authSignerModal.tsx#L77-L78
Added lines #L77 - L78 were not covered by tests
[warning] 81-82: components/react/authSignerModal.tsx#L81-L82
Added lines #L81 - L82 were not covered by tests
[warning] 85-86: components/react/authSignerModal.tsx#L85-L86
Added lines #L85 - L86 were not covered by tests
[warning] 89-90: components/react/authSignerModal.tsx#L89-L90
Added lines #L89 - L90 were not covered by tests
[warning] 93-94: components/react/authSignerModal.tsx#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 105-105: components/react/authSignerModal.tsx#L105
Added line #L105 was not covered by tests
[warning] 110-110: components/react/authSignerModal.tsx#L110
Added line #L110 was not covered by tests
[warning] 112-114: components/react/authSignerModal.tsx#L112-L114
Added lines #L112 - L114 were not covered by tests
[warning] 119-151: components/react/authSignerModal.tsx#L119-L151
Added lines #L119 - L151 were not covered by tests
[warning] 185-185: components/react/authSignerModal.tsx#L185
Added line #L185 was not covered by tests
[warning] 196-200: components/react/authSignerModal.tsx#L196-L200
Added lines #L196 - L200 were not covered by tests
[warning] 203-206: components/react/authSignerModal.tsx#L203-L206
Added lines #L203 - L206 were not covered by tests
[warning] 214-227: components/react/authSignerModal.tsx#L214-L227
Added lines #L214 - L227 were not covered by tests
[warning] 230-245: components/react/authSignerModal.tsx#L230-L245
Added lines #L230 - L245 were not covered by tests
[warning] 301-301: components/react/authSignerModal.tsx#L301
Added line #L301 was not covered by tests
🔇 Additional comments (3)
components/icons/ArrowRightIcon.tsx (1)
4-11: LGTM! Well-structured prop interface and component signature.The addition of the
sizeprop with a default value enhances the component's reusability while maintaining backward compatibility.components/messageSyntax.tsx (1)
35-53: Add unit tests for the new objectSyntax function.The static analysis indicates missing test coverage for the new function. Please add tests to verify:
- Correct JSON formatting
- Theme-based styling
- Handling of different object types
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 35-50: components/messageSyntax.tsx#L35-L50
Added lines #L35 - L50 were not covered by testscomponents/react/authSignerModal.tsx (1)
214-245: LGTM! Well-implemented collapsible UI.The implementation of collapsible sections with state management is clean and follows React best practices.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 214-227: components/react/authSignerModal.tsx#L214-L227
Added lines #L214 - L227 were not covered by tests
[warning] 230-245: components/react/authSignerModal.tsx#L230-L245
Added lines #L230 - L245 were not covered by tests
This pr:
Summary by CodeRabbit