feat: walletconnect dApps new signing modals #10461
Conversation
📝 WalkthroughWalkthroughAdds Tenderly simulation support (env, CSP, client, types, hook) and a comprehensive WalletConnect dApp signing refactor: new form-driven signing modal, simulation-driven UI components (structured EIP‑712 rendering, gas selection, simulation view), translations, and multiple API/type adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as WalletConnect Signing UI
participant Form as react-hook-form
participant Hook as useSimulateEvmTransaction
participant Adapter as Chain Adapter
participant Tenderly as Tenderly API
participant Store as Asset/Market Store
User->>UI: Open WalletConnect confirmation
UI->>Form: init form (speed, nonce, gas)
UI->>Hook: request simulation(tx, chainId, speed)
Hook->>Adapter: fetch gas fee data
Adapter-->>Hook: gas data
alt tx present & gasPrice available
Hook->>Tenderly: simulateTransaction(request)
Tenderly-->>Hook: simulation response
Hook->>Store: read fee asset & market price
Store-->>Hook: asset & price
Hook-->>UI: simulationQuery + fee
UI->>UI: render TransactionSimulation, StructuredMessage, GasSelection
else
Hook-->>UI: no simulation
end
User->>UI: Confirm
UI->>Form: handleSubmit -> onConfirm(formData?)
Form-->>UI: formData
UI-->>WalletConnect: send response (with optional CustomTransactionData)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
💤 Files with no reviewable changes (4)
🚧 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)
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: 0
♻️ Duplicate comments (1)
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx (1)
27-31: Gas value validation was previously discussedPrior suggestion to guard convertHexToNumber was declined (“trust Tenderly”); leaving as-is for consistency.
🧹 Nitpick comments (3)
src/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx (1)
19-24: Inline the pass-through submit handlerhandleFormSubmit simply forwards to onConfirm. Remove it and pass onConfirm directly.
- const handleFormSubmit = useCallback( - async (formData?: CustomTransactionData) => { - await onConfirm(formData) - }, - [onConfirm], - ) ... - <WalletConnectSigningModal - onConfirm={handleFormSubmit} + <WalletConnectSigningModal + onConfirm={onConfirm} onReject={onReject} state={state} topic={topic} >Also applies to: 29-36
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx (2)
36-39: Inline the pass-through submit handlerYou can pass handleConfirm directly and drop handleFormSubmit.
- const handleFormSubmit = useCallback( - (formData?: CustomTransactionData) => handleConfirm(formData), - [handleConfirm], - ) ... - <WalletConnectSigningModal - onConfirm={handleFormSubmit} + <WalletConnectSigningModal + onConfirm={handleConfirm} onReject={handleReject} state={state} topic={topic} transaction={transaction} formContext={form}Also applies to: 53-59
41-49: Localize the error toast and include contextPass a translation key and options to showErrorToast to meet i18n/error-toast guidelines.
- if (!transaction || !chainId) { - showErrorToast({ - message: 'unable to handle tx due to invalid params', - params: state.modalData.requestEvent?.params, - }) + if (!transaction || !chainId) { + showErrorToast( + new Error('WalletConnect invalid params'), + 'plugins.walletConnectToDapps.errors.invalidParams', + { method: state.modalData.requestEvent?.params?.request?.method, chainId }, + ) handleReject() return null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx(1 hunks)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectSigningModal.tsx(1 hunks)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx(1 hunks)src/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx(1 hunks)src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx(2 hunks)src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx(1 hunks)src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx
- src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx
- src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectSigningModal.tsx
- src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx
- src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.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/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.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/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.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/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
🧠 Learnings (25)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:26.962Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.200Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
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#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:91-95
Timestamp: 2025-09-12T13:43:50.683Z
Learning: gomesalexandre dismissed gas calculation overflow validation in useSimulateEvmTransaction hook as "stale", preferring to trust Tenderly's simulation data without defensive validation checks, consistent with his established pattern of relying on external service quality over defensive programming in WalletConnect flows.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a sophisticated gasLimit management system in WalletConnect dApps using Tenderly simulation. The GasSelectionMenu component automatically adjusts gasLimit via setValue when simulation shows higher gas usage than currently set, handling edge cases like dApps that enforce low gas limits (e.g., 21000) when actual usage is higher (e.g., 23322). This makes customTransactionData.gasLimit highly reliable as the primary source.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a reliable gasLimit flow in WalletConnect dApps where Tenderly simulation provides gas estimates that get written to the form via setValue in GasSelectionMenu.tsx, making customTransactionData.gasLimit the primary reliable source. The sendTransaction.gasLimit fallback is kept as "paranoia" but may rarely be hit in practice due to this simulation-based architecture.
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#10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
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.699Z
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-09-12T11:52:39.200Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.200Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
📚 Learning: 2025-09-12T12:08:15.793Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/WalletConnectModalManager.tsx:226-233
Timestamp: 2025-09-12T12:08:15.793Z
Learning: In WalletConnect dApps integration, EthSignTransactionCallRequest and EthSendTransactionCallRequest have identical transaction parameter structures (from, to, data, etc.) per JSON-RPC spec - only the method field differs. This makes it safe to type-cast between them when passing transaction parameters to components like EIP155TransactionConfirmation that process the underlying transaction data regardless of the originating method.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T12:00:33.878Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
📚 Learning: 2025-09-12T12:00:33.878Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
📚 Learning: 2025-09-12T09:58:57.369Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:0-0
Timestamp: 2025-09-12T09:58:57.369Z
Learning: gomesalexandre prefers letting errors throw and propagate up the call stack in form submit handlers rather than adding local try/catch error handling, consistent with his fail-fast approach in WalletConnect transaction confirmation flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T10:44:46.696Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx:0-0
Timestamp: 2025-09-12T10:44:46.696Z
Learning: gomesalexandre dismissed a clipboard error handling suggestion in PR #10461 for SendTransactionContent.tsx, demonstrating that the current navigator.clipboard.writeText implementation works as expected and preferring to keep it simple without additional try/catch error handling.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS handle different error types appropriately in error toasts
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-11T22:53:19.837Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:27-31
Timestamp: 2025-09-11T22:53:19.837Z
Learning: gomesalexandre trusts Tenderly's data quality and doesn't want defensive validation for gas values (transaction?.gasLimit ?? transaction?.gas) in WalletConnect flows, preferring to rely on the external service providing valid hex values.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T13:43:19.748Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a reliable gasLimit flow in WalletConnect dApps where Tenderly simulation provides gas estimates that get written to the form via setValue in GasSelectionMenu.tsx, making customTransactionData.gasLimit the primary reliable source. The sendTransaction.gasLimit fallback is kept as "paranoia" but may rarely be hit in practice due to this simulation-based architecture.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T13:43:19.748Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a sophisticated gasLimit management system in WalletConnect dApps using Tenderly simulation. The GasSelectionMenu component automatically adjusts gasLimit via setValue when simulation shows higher gas usage than currently set, handling edge cases like dApps that enforce low gas limits (e.g., 21000) when actual usage is higher (e.g., 23322). This makes customTransactionData.gasLimit highly reliable as the primary source.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T13:43:50.683Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:91-95
Timestamp: 2025-09-12T13:43:50.683Z
Learning: gomesalexandre dismissed gas calculation overflow validation in useSimulateEvmTransaction hook as "stale", preferring to trust Tenderly's simulation data without defensive validation checks, consistent with his established pattern of relying on external service quality over defensive programming in WalletConnect flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T12:04:59.511Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx:0-0
Timestamp: 2025-09-12T12:04:59.511Z
Learning: The ShapeShift codebase's fromBaseUnit function correctly handles hex strings (like WalletConnect transaction.value) without manual conversion because bnOrZero -> bn -> new BigNumber() automatically detects and parses hex strings starting with "0x". gomesalexandre confirmed this with concrete evidence showing hex value 0x176d1c49189db correctly converts to 0.000412118294825435 ETH.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-10T15:34:54.593Z
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:46-59
Timestamp: 2025-09-10T15:34:54.593Z
Learning: After extensive testing by gomesalexandre in PR #10458, dApps do not send EIP-712 domain.chainId as hex or bigint values in practice. The simple String(domain.chainId) conversion is sufficient for real-world usage in WalletConnect dApps structured signing.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : Missing try-catch blocks for async operations is an anti-pattern
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-10T15:35:46.223Z
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-08T22:00:48.005Z
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
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.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T10:35:51.606Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:33-45
Timestamp: 2025-09-12T10:35:51.606Z
Learning: gomesalexandre consistently dismisses CodeRabbit suggestions about replacing console.error/console.warn with structured logging in API integration code, preferring simple console logging for debugging Tenderly transaction simulation APIs in WalletConnect flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-10T15:35:36.547Z
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:69-74
Timestamp: 2025-09-10T15:35:36.547Z
Learning: gomesalexandre dismissed alt text accessibility suggestion with "meh" in PR #10458 for EIP155SignTypedDataConfirmation.tsx Image component, consistent with team pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T11:56:19.399Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.399Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T13:16:26.962Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:26.962Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T12:04:24.531Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx:40-140
Timestamp: 2025-09-12T12:04:24.531Z
Learning: gomesalexandre prefers to defer error handling improvements in new/preparatory code until real-world error cases are encountered, rather than implementing speculative error handling without concrete failure scenarios to test against. This aligns with keeping PR scope focused and evidence-based development practices.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-09-12T09:58:50.645Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:14-15
Timestamp: 2025-09-12T09:58:50.645Z
Learning: In EIP155SignTypedDataConfirmation.tsx, gomesalexandre is comfortable with null returns for missing message data as TypeScript type safety guards, viewing them as compile-time protections rather than runtime scenarios that need explicit error handling and user feedback. He considers these TypeScript-level safeguards sufficient and prefers not to add defensive runtime error handling for what should be type-guaranteed scenarios.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsxsrc/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx
📚 Learning: 2025-09-12T13:44:16.978Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:16.978Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
📚 Learning: 2025-08-27T09:47:06.275Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx
🧬 Code graph analysis (2)
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx (8)
src/plugins/walletConnectToDapps/WalletConnectModalManager.tsx (1)
WalletConnectRequestModalProps(49-55)src/plugins/walletConnectToDapps/types.ts (2)
EthSignTransactionCallRequest(150-153)CustomTransactionData(109-114)src/plugins/walletConnectToDapps/hooks/useWalletConnectState.ts (1)
useWalletConnectState(23-77)src/hooks/useErrorToast/useErrorToast.tsx (1)
useErrorToast(14-63)src/plugins/walletConnectToDapps/utils.ts (1)
convertHexToNumber(41-41)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectSigningModal.tsx (1)
WalletConnectSigningModal(26-85)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx (1)
TransactionContent(40-139)src/plugins/walletConnectToDapps/components/modals/TransactionAdvancedParameters.tsx (1)
TransactionAdvancedParameters(22-117)
src/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx (5)
src/plugins/walletConnectToDapps/WalletConnectModalManager.tsx (1)
WalletConnectRequestModalProps(49-55)src/plugins/walletConnectToDapps/types.ts (1)
CustomTransactionData(109-114)src/plugins/walletConnectToDapps/hooks/useWalletConnectState.ts (1)
useWalletConnectState(23-77)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectSigningModal.tsx (1)
WalletConnectSigningModal(26-85)src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/MessageContent.tsx (1)
MessageContent(11-23)
⏰ 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 (4)
src/plugins/walletConnectToDapps/components/modals/EIP155SignMessageConfirmation.tsx (2)
26-26: Verify upstream routing guarantees message presenceIf message can ever be falsy for this modal, consider rejecting the request for consistency; otherwise this early return is fine.
29-36: LGTM: unified SigningModal + MessageContent integrationClean composition and correct prop threading.
src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx (2)
18-21: Types align with intended routingLimiting to EthSignTransactionCallRequest matches the sign vs send split.
53-63: LGTM: SigningModal + TransactionContent + AdvancedParametersGood composition and RHF FormProvider wiring.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/plugins/walletConnectToDapps/WalletConnectModalManager.tsx(4 hunks)src/plugins/walletConnectToDapps/components/modals/SessionProposal.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/plugins/walletConnectToDapps/WalletConnectModalManager.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/components/modals/SessionProposal.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/plugins/walletConnectToDapps/components/modals/SessionProposal.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/plugins/walletConnectToDapps/components/modals/SessionProposal.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/plugins/walletConnectToDapps/components/modals/SessionProposal.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/components/modals/SessionProposal.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:26.962Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.200Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
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#10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.878Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:46-59
Timestamp: 2025-09-10T15:34:54.593Z
Learning: After extensive testing by gomesalexandre in PR #10458, dApps do not send EIP-712 domain.chainId as hex or bigint values in practice. The simple String(domain.chainId) conversion is sufficient for real-world usage in WalletConnect dApps structured signing.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:91-95
Timestamp: 2025-09-12T13:43:50.683Z
Learning: gomesalexandre dismissed gas calculation overflow validation in useSimulateEvmTransaction hook as "stale", preferring to trust Tenderly's simulation data without defensive validation checks, consistent with his established pattern of relying on external service quality over defensive programming in WalletConnect flows.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a sophisticated gasLimit management system in WalletConnect dApps using Tenderly simulation. The GasSelectionMenu component automatically adjusts gasLimit via setValue when simulation shows higher gas usage than currently set, handling edge cases like dApps that enforce low gas limits (e.g., 21000) when actual usage is higher (e.g., 23322). This makes customTransactionData.gasLimit highly reliable as the primary source.
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/EIP155RequestHandlerUtil.ts:94-103
Timestamp: 2025-09-12T13:43:19.748Z
Learning: gomesalexandre has implemented a reliable gasLimit flow in WalletConnect dApps where Tenderly simulation provides gas estimates that get written to the form via setValue in GasSelectionMenu.tsx, making customTransactionData.gasLimit the primary reliable source. The sendTransaction.gasLimit fallback is kept as "paranoia" but may rarely be hit in practice due to this simulation-based architecture.
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#10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
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.699Z
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-09-12T11:52:39.200Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.200Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Applied to files:
src/plugins/walletConnectToDapps/components/modals/SessionProposal.tsx
⏰ 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 (1)
src/plugins/walletConnectToDapps/components/modals/SessionProposal.tsx (1)
22-22: Import looks correct and scoped.Bringing
EIP155_SigningMethodinto this module is appropriate for the namespace approval logic.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Overall what a PR ser!
I did a few stylish improvement in 6b0c847
Some of them are fixing my own comments from the review re:
- Remove cardBg styles as a card has a background with opacity by default we don't need that
- Leverage
InlineCopyButtonfrom our global components and remove theCopyButtonunder the wallet connect umbrella - Fix some style discrepencies on text colors, sizes, spacings...
I tested everything but didn't record, here are the feedbacks:
- Mobile style should use instead of to make it polish (can be done as a follow up, basically switch
ModaltoDialoginWalletConnectModalManager) - Footer on desktop is like on mobile and it feels weird to me but that's more of a @shapeshift/product thing
- I had some weird parsing issues with relay USDC on arb to eth on arb, there was a copy button with no values on the left
- Cowswap => USDC on arb to ETH on arb "nonce not supported" but it was probably because of weird connection issue from my side so...
- Idk why but cowswap "connect all accounts" didn't work I had to select my arb accounts to make it detect my balances
Other than that, everything was super sexy, simulation is crazy!
src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/GasSelectionMenu.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/GasSelectionMenu.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/GasSelectionMenu.tsx
Show resolved
Hide resolved
...lletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/ExpandableCell.tsx
Outdated
Show resolved
Hide resolved
...walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx
Outdated
Show resolved
Hide resolved
...ins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx
Outdated
Show resolved
Hide resolved
src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx
Show resolved
Hide resolved
premiumjibles
left a comment
premiumjibles
left a comment
There was a problem hiding this comment.
Testing sesh:
https://jam.dev/c/538a9bff-2bed-436c-85e5-c75992107b74
Got into a state where I wasn't getting any confirmation pop ups. Not really sure what happened here https://jam.dev/c/c306573d-da6e-4545-bf8c-f6cad0b9db8c. Worked after a refresh so probably just got into a weird state locally.
Could we run some kind of camel case to title case with spaces transformation over the row labels. We're getting some labels all lower case and some camel case.
Also tested with Relay. All looks good. Only the tiny nitpick above.
|
tyvm for the testing ser @premiumjibles! ping for viz:
Yeah this one is already happening in develop unfortunately. From the extensive testing done, that mostly happens in case of hot reloads, though the wc disconnecting is definitely a possibility, although not an us thing. Though what we could do as a nice-to-have is to have some kind of connection status (@shapeshift/product?), think that's technically possible, worst case scenario we can ping at intervals i.e this guy in https://react-app.walletconnect.com/: 
From the point of view of displaying things purely, I'd say we shouldn't do that as this is the data from the message and we should display it as-is. But since the goal is to follow-up with a "View Raw", perhaps a little bit of transformation to make fields look nicer for humans wouldn't hurt? WDYT @shapeshift/product ? |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/plugins/walletConnectToDapps/components/modals/CosmosSignMessageConfirmation.tsx (3)
44-55: Prevent loading-state lock with try/finally.If
onConfirm()/onReject()throws,isLoadingstays true and the modal becomes stuck. Wrap in try/finally.- 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])
82-85: Messages render as “[object Object]”; render structured content.Amino
msgsare objects/arrays — directly interpolating them misleads users about what they’re signing.- <RawText fontWeight='medium' color='text.subtle'> - {messages.length > 0 - ? messages - : translate('plugins.walletConnectToDapps.modal.signMessage.noMessages')} - </RawText> + <RawText fontWeight='medium' color='text.subtle' whiteSpace='pre-wrap'> + {messages.length > 0 + ? JSON.stringify(messages, null, 2) + : translate('plugins.walletConnectToDapps.modal.signMessage.noMessages')} + </RawText>
124-133: authInfo/body are bytes; current render likely shows “[object Object]”.Show a hex/base64 string (and keep copy-friendly). This avoids misrepresenting the payload.
- <RawText fontWeight='medium' color='text.subtle'> - {authInfo} - </RawText> + <RawText fontWeight='medium' color='text.subtle' fontFamily='mono' wordBreak='break-all'> + {authInfo instanceof Uint8Array + ? `0x${Array.from(authInfo).map(b => b.toString(16).padStart(2, '0')).join('')}` + : String(authInfo)} + </RawText> @@ - <RawText fontWeight='medium' color='text.subtle'> - {body} - </RawText> + <RawText fontWeight='medium' color='text.subtle' fontFamily='mono' wordBreak='break-all'> + {body instanceof Uint8Array + ? `0x${Array.from(body).map(b => b.toString(16).padStart(2, '0')).join('')}` + : String(body)} + </RawText>
♻️ Duplicate comments (4)
src/plugins/walletConnectToDapps/utils/tenderly/index.ts (4)
33-37: Console warnings vs structured logs (not rehashing)Previous review already covered replacing console.* with the project logger; acknowledging your stated preference to keep console here.
119-146: Use of any in tuple parsing (not rehashing)Prior review already discussed any vs unknown for dynamic Tenderly payloads; noting your decision to keep any here.
220-229: Timeouts, Result monad, and structured logs (not rehashing)Earlier comments suggested adding a request timeout and adopting Ok/Err + custom errors; acknowledging you opted to keep KISS here.
220-229: Client-sent X-Access-Key (not rehashing)Security note previously raised about exposing the Tenderly API key from the browser; acknowledging your stance for this phase.
🧹 Nitpick comments (20)
src/plugins/walletConnectToDapps/components/modals/TransactionAdvancedParameters.tsx (5)
25-27: Stabilize toggle callback with functional state updateAvoids capturing
isExpandedand re-creating the callback unnecessarily.- const [isExpanded, setIsExpanded] = useState(false) - const hoverStyle = useMemo(() => ({ bg: 'transparent' }), []) - const handleToggle = useCallback(() => setIsExpanded(!isExpanded), [isExpanded]) + const [isExpanded, setIsExpanded] = useState(false) + const hoverStyle = useMemo(() => ({ bg: 'transparent' }), []) + const handleToggle = useCallback(() => setIsExpanded(prev => !prev), [])
14-14: Add a11y: link header button to region and expose expansion stateImprove screen-reader UX by wiring
aria-expanded,aria-controls, and labeling the region. Also mark chevrons as decorative.- import { useCallback, useMemo, useState } from 'react' + import { useCallback, useMemo, useState, useId } from 'react' @@ - const [isExpanded, setIsExpanded] = useState(false) + const [isExpanded, setIsExpanded] = useState(false) + const panelId = useId() @@ - <Button + <Button variant='ghost' size='sm' p={0} h='auto' fontWeight='medium' justifyContent='space-between' onClick={handleToggle} _hover={hoverStyle} w='full' mb={isExpanded ? 3 : 0} + aria-expanded={isExpanded} + aria-controls={panelId} + id={`${panelId}-header`} > @@ - {isExpanded ? <ChevronUpIcon /> : <ChevronDownIcon />} + {isExpanded ? <ChevronUpIcon aria-hidden /> : <ChevronDownIcon aria-hidden />} </Button> @@ -{isExpanded && ( - <VStack spacing={3} alignItems='stretch'> +{isExpanded && ( + <VStack + spacing={3} + alignItems='stretch' + id={panelId} + role='region' + aria-labelledby={`${panelId}-header`} + >Also applies to: 25-27, 36-55, 56-58
66-87: Associate label with input and tighten numeric behavior for nonceLink the label via
htmlFor, moveborderColorto the input field, and guide numeric-only input (mobile keypad, clamping).- <FormControl> - <FormLabel display='flex' columnGap={1}> + <FormControl> + <FormLabel display='flex' columnGap={1} htmlFor='nonce'> <Text color='text.subtle' fontWeight='medium' fontSize='sm' translation='plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.nonce.title' /> <HelperTooltip label={translate( 'plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.nonce.tooltip', )} /> </FormLabel> - <NumberInput borderColor={borderColor} mt={2}> + <NumberInput mt={2} min={0} step={1} precision={0} clampValueOnBlur keepWithinRange> <NumberInputField + id='nonce' + borderColor={borderColor} + inputMode='numeric' + pattern='[0-9]*' + autoComplete='off' placeholder={translate( 'plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.nonce.placeholder', )} {...register('nonce')} /> </NumberInput> </FormControl>
90-112: Associate label with input and tighten numeric behavior for gasLimitSame as above; also prevents accidental decimals and enforces non-negative integers.
- <FormControl> - <FormLabel display='flex' columnGap={1}> + <FormControl> + <FormLabel display='flex' columnGap={1} htmlFor='gasLimit'> <Text color='text.subtle' fontWeight='medium' fontSize='sm' translation='plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.gasLimit.title' /> <HelperTooltip label={translate( 'plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.gasLimit.tooltip', )} /> </FormLabel> - <NumberInput borderColor={borderColor} mt={2}> + <NumberInput mt={2} min={0} step={1} precision={0} clampValueOnBlur keepWithinRange> <NumberInputField + id='gasLimit' + borderColor={borderColor} + inputMode='numeric' + pattern='[0-9]*' + autoComplete='off' placeholder={translate( 'plugins.walletConnectToDapps.modal.sendTransaction.advancedParameters.gasLimit.placeholder', )} {...register('gasLimit')} /> </NumberInput> </FormControl>
22-118: Error boundaries: ensure parent modal wraps childrenPer guidelines, components should be within an error boundary. Given prior PR scope preferences, confirming the parent modal (manager) wraps this subtree is sufficient for this PR.
If not wrapped, I can propose a minimal
<ModalErrorBoundary>around the WalletConnect modal stack.src/plugins/walletConnectToDapps/utils/tenderly/index.ts (5)
22-24: Clarify env error and list the exact missing varsImprove the message so operators immediately know which envs are required.
-if (!TENDERLY_ACCOUNT_SLUG || !TENDERLY_PROJECT_SLUG || !TENDERLY_API_KEY) { - throw new Error('Missing Tenderly account/project env vars') -} +if (!TENDERLY_ACCOUNT_SLUG || !TENDERLY_PROJECT_SLUG || !TENDERLY_API_KEY) { + throw new Error( + 'Missing Tenderly config: VITE_TENDERLY_ACCOUNT_SLUG, VITE_TENDERLY_PROJECT_SLUG, VITE_TENDERLY_API_KEY', + ) +}
31-32: Support both locations for asset changes (compat fallback)Tenderly responses may include asset_changes at the root. Add a fallback to avoid silently dropping results.
- const assetChanges = simulation.transaction.transaction_info?.asset_changes || [] + const assetChanges = + simulation.transaction.transaction_info?.asset_changes ?? + simulation.asset_changes ?? + []
58-64: Avoid double entries on self-transfers (optional)A tx where from === to === user currently yields both send and receive entries. If that’s noisy in the UI, skip the receive in this case.
- if (toAddress && isAddressEqual(toAddress, from)) { + if ( + toAddress && + isAddressEqual(toAddress, from) && + !isAddressEqual(fromAddress, toAddress) + ) { changes.push(makeAssetChange('receive')) }
70-82: Dry up the return shape with a local alias (and tighten children type)Removes repeated inline shapes and replaces any[] with a recursive type alias for readability.
+type StructuredField = { + key: string + value: any + type: string + children?: StructuredField[] +} + export const convertToStructuredFields = ( args: ParsedArgument[], -): { - key: string - value: any - type: string - children?: { - key: string - value: any - type: string - children?: any[] - }[] -}[] => { +): StructuredField[] => {And update convertComponents’ return type accordingly:
- ): - | { - key: string - value: any - type: string - children?: any[] - }[] - | undefined => { + ): StructuredField[] | undefined => {
112-115: Add fallback to simulation.simulation.decoded_inputSome Tenderly variants surface decoded_input under simulation.simulation. Adding a fallback makes parsing more robust.
- const decodedInput = simulation.transaction.transaction_info?.call_trace?.decoded_input + const decodedInput = + simulation.transaction.transaction_info?.call_trace?.decoded_input ?? + simulation.simulation.decoded_inputsrc/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/ExpandableCell.tsx (3)
62-77: Add keyboard support and ARIA to the expandable rowMake the clickable HStack operable via keyboard and expose state to assistive tech.
- <HStack + <HStack spacing={2} align='center' borderWidth={1} borderColor={borderColor} borderRadius='md' px={2} py={1} cursor='pointer' onClick={toggleExpanded} + role='button' + tabIndex={0} + aria-expanded={false} + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + toggleExpanded() + } + }} _hover={hoverSx} > @@ - <HStack + <HStack spacing={2} align='center' borderWidth={1} borderColor={borderColor} borderRadius='md' px={2} py={1} cursor='pointer' onClick={toggleExpanded} + role='button' + tabIndex={0} + aria-expanded={true} + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + toggleExpanded() + } + }} _hover={hoverSx} >Also applies to: 86-101
49-53: Hide copy button for empty stringsPassing an empty value (e.g., from upstream null/undefined normalization) shouldn’t render a copy affordance.
- <InlineCopyButton value={value}> + <InlineCopyButton value={value} isDisabled={!value}> @@ - <InlineCopyButton value={value}> + <InlineCopyButton value={value} isDisabled={!value}> @@ - <InlineCopyButton value={value}> + <InlineCopyButton value={value} isDisabled={!value}>Also applies to: 61-77, 85-101
28-44: Memoize style objects for consistencyMinor: keep style object creation consistent with hoverSx/containerProps to avoid re-creates on each render.
- const chevronIconSx = { boxSize: '10px', color: chevronColor } - const expandedChevronSx = { - boxSize: '10px', - transform: 'rotate(90deg)', - color: chevronColor, - } - const expandedTextSx = { - fontSize: 'xs', - fontFamily: 'mono', - wordBreak: 'break-all' as const, - color: expandedTextColor, - pl: 4, - py: 2, - mt: 1, - maxW: '100%', - textAlign: 'right' as const, - } + const chevronIconSx = useMemo(() => ({ boxSize: '10px', color: chevronColor }), [chevronColor]) + const expandedChevronSx = useMemo( + () => ({ boxSize: '10px', transform: 'rotate(90deg)', color: chevronColor }), + [chevronColor], + ) + const expandedTextSx = useMemo( + () => ({ + fontSize: 'xs', + fontFamily: 'mono', + wordBreak: 'break-all' as const, + color: expandedTextColor, + pl: 4, + py: 2, + mt: 1, + maxW: '100%', + textAlign: 'right' as const, + }), + [expandedTextColor], + )src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx (3)
63-81: Expose expanded state via ARIA on the section toggleSmall a11y boost for assistive tech.
<Button variant='ghost' size='sm' pl={paddingLeft} pr={2} py={1} h='auto' fontWeight='normal' justifyContent='space-between' onClick={toggleExpanded} _hover={hoverStyle} w='full' + aria-expanded={isExpanded} >
124-129: Add alt text and lazy loading to asset iconImproves a11y and performance.
- <Image boxSize='20px' src={asset.icon} borderRadius='full' /> + <Image + boxSize='20px' + src={asset.icon} + alt={`${asset.symbol} icon`} + loading='lazy' + borderRadius='full' + />
37-46: Optional: log assetId resolution failures at debug levelSilent catch can hinder debugging. Consider structured debug logging (behind a flag) without spamming Sentry/UI.
If you have a logger, something like:
// logger.debug('toAssetId failed', { chainId, value, err })src/plugins/walletConnectToDapps/components/modals/CosmosSignMessageConfirmation.tsx (1)
153-157: Add alt text and a safe fallback for dApp icon.Improves a11y and avoids broken-image jitter when
icons[0]is missing.- <Image borderRadius='full' boxSize='24px' src={peerMetadata.icons?.[0]} /> + <Image + borderRadius='full' + boxSize='24px' + src={peerMetadata.icons?.[0]} + alt={peerMetadata.name} + fallbackStrategy='beforeLoadOrError' + />src/plugins/walletConnectToDapps/components/modals/AddressSummaryCard.tsx (2)
30-45: Guard copy affordance when address is empty.Hide/disable the copy button if
addressis falsy to avoid copying an empty string.- <InlineCopyButton value={address}> + <InlineCopyButton value={address} isDisabled={!address}>
45-45: Clarify external link aria label.A long hex string isn’t descriptive for screen readers.
- <ExternalLinkButton href={`${explorerAddressLink}${address}`} ariaLabel={address} /> + <ExternalLinkButton + href={`${explorerAddressLink}${address}`} + ariaLabel='View address on explorer' + />src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx (1)
54-56: bytes[] inputs: show per-item sizes instead of a joined string.
transaction.args[index].toString()joins arrays, producing an aggregate string and misleading “bytes” count. Render each element.- case 'bytes[]': - return <EncodedText value={inputValue} /> + case 'bytes[]': { + const arr = transaction?.args[index] as unknown as Array<string | Uint8Array> + return ( + <Flex direction='column' align='flex-end' gap={1}> + {Array.isArray(arr) + ? arr.map((v, i) => ( + <EncodedText + key={i} + value={typeof v === 'string' ? v : `0x${Array.from(v).map(b => b.toString(16).padStart(2, '0')).join('')}`} + /> + )) + : <EncodedText value={inputValue} />} + </Flex> + ) + }
Description
This PR:
Note, this is p1 out of 2 of the new wc modals work and doesn't bring the new connection flow just yet i.e this guy
Issue (if applicable)
Risk
Medium/High - high risk of borked wc dApps things, even though the guts of this is view layer, parsing being wrong means we could display absolutely wrong data
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Refactor
Localization
Style
Chores