feat: wc dApps structured signing #10458
Conversation
📝 WalkthroughWalkthroughUpdates translations and a filter header key. Replaces the old typed-data renderer with a new EIP‑712 structured display component, updates the WalletConnect EIP‑155 typed-data confirmation modal to use peer metadata and the new renderer (early-return if peerMetadata missing), adjusts the events handler to open the new modal, and adds EIP‑712 types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dapp
participant WC as WalletConnect Events Handler
participant UI as SignEIP155TypedDataConfirmation
participant Display as EIP712MessageDisplay
Dapp->>WC: ETH_SIGN_TYPED_DATA (v1/v3/v4)
WC->>UI: Open modal (state, topic)
UI->>UI: Resolve peerMetadata from state.sessionsByTopic[topic]
alt peerMetadata missing
UI-->>WC: Render null (no modal)
else peerMetadata present
UI->>UI: Render peer card (icon, name, link)
UI->>Display: Provide typedData string
Display->>Display: Parse JSON & validate typed data
Display-->>UI: Render Domain (contract, network) and Message (primaryType, fields)
UI-->>WC: User Confirm / Reject
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ Finishing Touches🧪 Generate unit tests
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 |
src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx (1)
43-53: Ensure loading state resets on failure.Wrap in try/finally so
isLoadingis always cleared. Consider showing an error toast on failure.Apply:
- const handleConfirm = useCallback(async () => { - setIsLoading(true) - await onConfirm() - setIsLoading(false) - }, [onConfirm]) + const handleConfirm = useCallback(async () => { + setIsLoading(true) + try { + await onConfirm() + } finally { + setIsLoading(false) + } + }, [onConfirm]) - const handleReject = useCallback(async () => { - setIsLoading(true) - await onReject() - setIsLoading(false) - }, [onReject]) + const handleReject = useCallback(async () => { + setIsLoading(true) + try { + await onReject() + } finally { + setIsLoading(false) + } + }, [onReject])
♻️ Duplicate comments (1)
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx (1)
84-89: Localize the explorer aria-labelConsider translating the aria label for consistency with the rest of the UI text.
🧹 Nitpick comments (3)
src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts (1)
49-51: Prefer structured logging over console.error.Swap
console.errorfor the project’s structured logger and includetopicandrequest.methodas fields.src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx (2)
100-103: Add alt text for the network iconImprove a11y with an
alt. Harmless if the icon fails to load.- <HStack w='24px' h='24px' justify='center' align='center'> - <Image boxSize='16px' src={domainFeeAsset.networkIcon ?? domainFeeAsset.icon} /> - </HStack> + <HStack w='24px' h='24px' justify='center' align='center'> + <Image + boxSize='16px' + src={domainFeeAsset.networkIcon ?? domainFeeAsset.icon} + alt={`${domainFeeAsset.networkName} icon`} + /> + </HStack>
140-160: Memoize message entries and guard against empty messageSaves minor work on re-render and avoids accidental undefined access.
const { primaryType, domain, message } = parsedData return ( <> {domain && Object.keys(domain).length > 0 && ( <ModalSection title='plugins.walletConnectToDapps.modal.signMessage.domain'> <DomainSection domain={domain} /> </ModalSection> )} <ModalSection title='plugins.walletConnectToDapps.modal.signMessage.message'> <VStack align='stretch' spacing={2}> <MessageField name={translate('plugins.walletConnectToDapps.modal.signMessage.primaryType')} value={primaryType} /> - {Object.entries(message).map(([key, value]) => ( + {useMemo(() => Object.entries(message ?? {}), [message]).map(([key, value]) => ( <MessageField key={key} name={key} value={value} /> ))} </VStack> </ModalSection> </> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/assets/translations/en/main.json(2 hunks)src/components/AssetListFiltersDialog/ChainOptionGroup.tsx(1 hunks)src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx(5 hunks)src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx(1 hunks)src/plugins/walletConnectToDapps/components/modals/TypedMessageInfo.tsx(0 hunks)src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts(1 hunks)src/plugins/walletConnectToDapps/types.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/plugins/walletConnectToDapps/components/modals/TypedMessageInfo.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/plugins/walletConnectToDapps/types.tssrc/components/AssetListFiltersDialog/ChainOptionGroup.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsxsrc/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.tssrc/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/plugins/walletConnectToDapps/types.tssrc/components/AssetListFiltersDialog/ChainOptionGroup.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsxsrc/assets/translations/en/main.jsonsrc/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.tssrc/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/plugins/walletConnectToDapps/types.tssrc/components/AssetListFiltersDialog/ChainOptionGroup.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsxsrc/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.tssrc/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/AssetListFiltersDialog/ChainOptionGroup.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/AssetListFiltersDialog/ChainOptionGroup.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:47.970Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
PR: shapeshift/web#10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
PR: shapeshift/web#10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/Routes/RoutesCommon.tsx:231-267
Timestamp: 2025-09-03T21:17:27.681Z
Learning: gomesalexandre prefers to keep PR diffs focused and reasonable in size, deferring tangential improvements (like Mixpanel privacy enhancements) to separate efforts rather than expanding the scope of feature PRs.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx:46-53
Timestamp: 2025-08-14T19:21:45.426Z
Learning: gomesalexandre does not like code patterns being labeled as "technical debt" and prefers neutral language when discussing existing code patterns that were intentionally moved/maintained for consistency.
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/components/AssetListFiltersDialog/ChainOptionGroup.tsx
📚 Learning: 2025-08-03T22:11:30.833Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/typescript-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:11:30.833Z
Learning: Applies to **/*.{ts,tsx} : Manually creating types instead of using utility types
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : EXTRACT complex logic into custom hooks
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
🧬 Code graph analysis (3)
src/components/AssetListFiltersDialog/ChainOptionGroup.tsx (1)
src/components/Text/Text.tsx (1)
Text(19-83)
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx (5)
src/plugins/walletConnectToDapps/types.ts (2)
EIP712Value(261-261)EIP712TypedData(263-268)src/components/Text/Text.tsx (1)
RawText(15-17)packages/caip/src/constants.ts (1)
CHAIN_NAMESPACE(77-82)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/assetsSlice/selectors.ts (1)
selectFeeAssetByChainId(74-78)
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx (3)
src/plugins/walletConnectToDapps/hooks/useWalletConnectState.ts (1)
useWalletConnectState(24-86)src/components/Text/Text.tsx (1)
RawText(15-17)src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx (1)
EIP712MessageDisplay(116-163)
⏰ 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). (1)
- GitHub Check: Install and Cache
🔇 Additional comments (7)
src/assets/translations/en/main.json (1)
218-220: LGTM: “Network” moved to common namespace; no staletransactionRow.networkreferences detected.src/components/AssetListFiltersDialog/ChainOptionGroup.tsx (1)
57-57: LGTM: translation key swap to common.network.Change is correct and aligns with the new key.
src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts (1)
71-73: No changes needed —WalletConnectModalManageralready passestopicintoEIP155SignTypedDataConfirmation.src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx (1)
24-24: topic prop correctly provided
The WalletConnectModalManager guards ontopicand passestopic={topic}toEIP155SignTypedDataConfirmation, matching the other flows.src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx (3)
120-128: Good fallback strategy on invalid typed dataValidating with
viemand showing raw JSON on failure is a sensible UX fallback.
71-109: Domain section UX looks solidContract + explorer link and network display are clear and align with the PR’s goals.
112-118: Ensure parent wraps with an error boundaryPer guidelines, confirm this component is used inside an error boundary and that the parent modal surfaces errors via a translated toast if needed.
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx
Show resolved
Hide resolved
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/63157506-62f8-49fc-b979-9143a0831bfb
Beautiful, I wonder why fees were 0 on cowswap, maybe the amount were too small!
Description
This PR:
Issue (if applicable)
closes #8228
Risk
Low - though risk of borked EIP-712 modals in wc dApps in case we do this wrong
Testing
Engineering
Operations
Screenshots (if applicable)
1inch
CoW
Summary by CodeRabbit
New Features
Chores