Conversation
WalkthroughThe changes introduce WalletConnect integration using the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginPage
participant Wagmi
participant WalletConnect
participant PillarWalletApp
User->>LoginPage: Clicks "Connect Pillar Wallet"
LoginPage->>Wagmi: Initiate WalletConnect connection
Wagmi->>WalletConnect: Generate connection URI
WalletConnect-->>LoginPage: Emit URI event
LoginPage->>PillarWalletApp: Redirect with wc?uri=...
PillarWalletApp-->>Wagmi: Complete wallet connection
Wagmi-->>LoginPage: Provide account address
LoginPage->>LoginPage: Update state and reload if needed
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 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 (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying x with
|
| Latest commit: |
5f5e1c0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0d5f54b2.x-e62.pages.dev |
| Branch Preview URL: | https://improvement-pillar-wallet-co.x-e62.pages.dev |
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
![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
🧹 Nitpick comments (1)
src/pages/Login.tsx (1)
37-37: Consider adding fallback handling for deep link failures.The deep link to
pillarwallet://wc?uri=assumes the protocol handler is registered on the user's device. If it's not, the user will see an error or nothing will happen.Consider implementing a fallback mechanism:
- Add a timeout to detect if the deep link succeeded
- Show a fallback UI with the WalletConnect QR code if the deep link fails
- Document the requirement for the Pillar Wallet app to be installed
Would you like me to suggest an implementation with fallback handling?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(3 hunks)src/components/BottomMenuModal/AccountModal.tsx(3 hunks)src/containers/Main.tsx(4 hunks)src/pages/Login.tsx(4 hunks)src/translations/en.json(1 hunks)
🔇 Additional comments (7)
src/translations/en.json (1)
26-26: LGTM! Translation key follows established patterns.The new translation key is appropriately placed in the "action" section and follows the existing naming conventions for UI elements.
package.json (1)
42-42:Details
❓ Verification inconclusive
Verify package versions for compatibility and security.
Please ensure the new package versions are compatible with existing dependencies and free from known vulnerabilities.
Also applies to: 48-48, 105-105, 153-153
🏁 Script executed:
#!/bin/bash # Check for security vulnerabilities in the new packages npm audit --audit-level moderate --json | jq '.vulnerabilities | to_entries[] | select(.key | test("@tanstack/react-query|@wagmi/core|wagmi|typescript")) | {package: .key, severity: .value.severity, title: .value.title}' # Check wagmi and @wagmi/core compatibility echo "Checking wagmi version compatibility..." npm view @wagmi/core@^2.16.7 peerDependencies npm view wagmi@2.14.16 dependencies | grep "@wagmi/core"Length of output: 1240
Security & Dependency Compatibility Check
File: package.json
Lines: 42, 48, 105, 153Snippet:
"@tanstack/react-query": "^5.77.1", "@wagmi/core": "^2.16.7", "wagmi": "2.14.16", "typescript": "^5.x"Please perform the following verifications:
- Generate or update your lockfile before auditing:
npm install --package-lock-only- Run a full security audit at moderate level:
npm audit --audit-level=moderate- Ensure peerDependencies for
@wagmi/core@^2.16.7are satisfied:
• typescript ≥ 5.0.4
• @tanstack/query-core ≥ 5.0.0
• viem 2.x- Confirm that
wagmi@2.14.16pulls in@wagmi/core@2.16.7as expected:npm ls @wagmi/core npm ls @tanstack/query-core npm ls typescriptsrc/components/BottomMenuModal/AccountModal.tsx (2)
27-27: LGTM! Proper wagmi integration.The
useDisconnecthook is correctly imported and used following wagmi best practices.Also applies to: 70-70
144-161: Excellent implementation of async logout with wallet disconnect.The logout flow properly:
- Makes the callback async to handle the disconnect operation
- Calls
await disconnect()before other cleanup operations- Updates the dependency array to include
disconnectThis ensures proper wallet disconnection during the logout process.
src/containers/Main.tsx (1)
79-82: LGTM! Proper provider setup and session management.The useEffect for session storage management and the provider wrapping with QueryClient and Wagmi are correctly implemented. The provider order ensures proper context availability throughout the application.
Also applies to: 373-379
src/pages/Login.tsx (2)
74-81: UI implementation looks good!The new
InsideWrappercomponent properly centers the buttons and maintains consistent styling. The button layout with full width and proper spacing provides a good user experience.Also applies to: 98-105
49-53:Details
❓ Verification inconclusive
Verify the auto-connection behavior is intended.
The code automatically triggers
loginOrLink()on the first wallet without user interaction. This could lead to unexpected behavior and conflicts with the manual connection buttons.Additionally, the
eslint-disable-next-line no-useless-returncomment suggests there might be a missing dependency or condition that should be checked.
🏁 Script executed:
#!/bin/bash # Description: Check if auto-connection is used elsewhere in the codebase # Search for other instances of automatic wallet connection rg -A 5 "loginOrLink\(\)" --type tsx --type ts # Check for any configuration or feature flags related to auto-connection rg -i "auto.*connect|automatic.*wallet" --type tsx --type ts --type jsonLength of output: 179
I’m not finding any matches due to the
rgtype filters. Let’s search without relying on predefined types:
🏁 Script executed:
#!/bin/bash # Find all occurrences of loginOrLink in .ts/.tsx files rg -A 5 "loginOrLink\(" --glob "*.ts" --glob "*.tsx" # Look for any “auto-connect” or similar flags in code or config rg -i "auto.*connect|automatic.*wallet" --glob "*.ts" --glob "*.tsx" --glob "*.json"Length of output: 1202
Confirm intentional auto-login behavior on wallet connection
Currently in
src/pages/Login.tsx(lines 49–53), the effect will immediately invokeif (!wallets.length) return; wallets[0].loginOrLink();as soon as any wallet appears—without any user click. In contrast, elsewhere (e.g.
WalletConnectDropdown) the user must explicitly trigger a connect.• Is this automatic login/link flow on page load intended?
• If so, please document this UX assumption.
• Otherwise, consider requiring a button press or explicit user action.
• Also add error handling aroundloginOrLink()and remove the unnecessaryeslint-disable-next-line no-useless-return.
package.json
Outdated
| "@uniswap/sdk-core": "^4.2.0", | ||
| "@uniswap/smart-order-router": "3.26.1", | ||
| "@uniswap/v3-sdk": "^3.11.0", | ||
| "@wagmi/core": "^2.16.7", |
There was a problem hiding this comment.
Fix version mismatch between wagmi dependencies.
There's a potential version compatibility issue:
@wagmi/coreuses^2.16.7(allowing newer minor versions)wagmiuses2.14.16(fixed version)
This mismatch could lead to runtime errors or type conflicts.
Consider aligning the versions:
- "@wagmi/core": "^2.16.7",
+ "@wagmi/core": "^2.14.16",Or update wagmi to use a caret range as well:
- "wagmi": "2.14.16",
+ "wagmi": "^2.14.16",Also applies to: 105-105
🤖 Prompt for AI Agents
In package.json at lines 48 and 105, there is a version mismatch between
@wagmi/core (^2.16.7) and wagmi (2.14.16). To fix this, align both dependencies
to use the same versioning scheme, either by setting both to a fixed version
like 2.16.7 or by using caret ranges (e.g., ^2.16.7) for both. This ensures
compatibility and prevents runtime or type conflicts.
| export const config = createConfig({ | ||
| chains: [mainnet], | ||
| connectors: [ | ||
| walletConnect({ | ||
| projectId: process.env.REACT_APP_WC_ID ?? '', | ||
| showQrModal: !isMobile, | ||
| isNewChainsStale: true, | ||
| methods: ['eth_sendTransaction', 'personal_sign'], | ||
| events: ['chainChanged', 'accountsChanged'], | ||
| metadata: { | ||
| name: 'PillarX', | ||
| description: 'PillarX', | ||
| url: 'https://pillarx.app/', | ||
| icons: ['https://pillarx.app/favicon.ico'], | ||
| }, | ||
| }), | ||
| ], | ||
| transports: { | ||
| [mainnet.id]: http(), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider supporting multiple chains in wagmi config.
The wagmi configuration only includes mainnet, but the application appears to support multiple chains based on the visibleChains usage elsewhere in the codebase. This might limit wallet connection functionality.
Consider updating the config to support all visible chains:
export const config = createConfig({
- chains: [mainnet],
+ chains: visibleChains,
connectors: [
walletConnect({
// ... connector config
}),
],
transports: {
- [mainnet.id]: http(),
+ ...visibleChains.reduce((acc, chain) => ({
+ ...acc,
+ [chain.id]: http(),
+ }), {}),
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const config = createConfig({ | |
| chains: [mainnet], | |
| connectors: [ | |
| walletConnect({ | |
| projectId: process.env.REACT_APP_WC_ID ?? '', | |
| showQrModal: !isMobile, | |
| isNewChainsStale: true, | |
| methods: ['eth_sendTransaction', 'personal_sign'], | |
| events: ['chainChanged', 'accountsChanged'], | |
| metadata: { | |
| name: 'PillarX', | |
| description: 'PillarX', | |
| url: 'https://pillarx.app/', | |
| icons: ['https://pillarx.app/favicon.ico'], | |
| }, | |
| }), | |
| ], | |
| transports: { | |
| [mainnet.id]: http(), | |
| }, | |
| }); | |
| export const config = createConfig({ | |
| chains: visibleChains, | |
| connectors: [ | |
| walletConnect({ | |
| projectId: process.env.REACT_APP_WC_ID ?? '', | |
| showQrModal: !isMobile, | |
| isNewChainsStale: true, | |
| methods: ['eth_sendTransaction', 'personal_sign'], | |
| events: ['chainChanged', 'accountsChanged'], | |
| metadata: { | |
| name: 'PillarX', | |
| description: 'PillarX', | |
| url: 'https://pillarx.app/', | |
| icons: ['https://pillarx.app/favicon.ico'], | |
| }, | |
| }), | |
| ], | |
| transports: { | |
| ...visibleChains.reduce((acc, chain) => ({ | |
| ...acc, | |
| [chain.id]: http(), | |
| }), {}), | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In src/containers/Main.tsx around lines 332 to 352, the wagmi config currently
only includes the mainnet chain, which limits wallet connection functionality.
Update the chains array in the config to include all chains referenced by
visibleChains instead of just mainnet. This will enable support for multiple
chains as used elsewhere in the application.
| chains: [mainnet], | ||
| connectors: [ | ||
| walletConnect({ | ||
| projectId: process.env.REACT_APP_WC_ID ?? '', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify WalletConnect project ID is configured.
The projectId uses an environment variable that might not be set, which could cause wallet connection failures.
🏁 Script executed:
#!/bin/bash
# Check if REACT_APP_WC_ID is mentioned in environment files
echo "Checking for WalletConnect project ID configuration..."
fd -t f "\.(env|example)" --exec grep -l "REACT_APP_WC_ID" {} \;
# Check if it's documented in README
fd -t f "README" --exec grep -l "REACT_APP_WC_ID" {} \;Length of output: 243
Document and enforce REACT_APP_WC_ID presence
The projectId at src/containers/Main.tsx (line 336) is pulled from process.env.REACT_APP_WC_ID, but that variable isn’t declared in any .env* file or in the README. Without it, WalletConnect will silently fail.
Please:
- Add a placeholder to your environment example:
# .env.example +REACT_APP_WC_ID=<your WalletConnect project ID>
- Update README.md to include setup steps for
REACT_APP_WC_ID. - (Optional) Add a runtime check in Main.tsx to log or throw if
process.env.REACT_APP_WC_IDis empty, so missing configs are caught early.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/containers/Main.tsx at line 336, the projectId uses
process.env.REACT_APP_WC_ID which is not declared in any environment files or
documented. Fix this by adding a placeholder for REACT_APP_WC_ID in the
environment example files, update README.md to include instructions for setting
REACT_APP_WC_ID, and optionally add a runtime check in Main.tsx to log an error
or throw if REACT_APP_WC_ID is missing to catch configuration issues early.
| return <Loading type="wait" />; | ||
| }; | ||
|
|
||
| const isMobile = /iPhone|iPad|iPod|Android/i.test(navigator.userAgent); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move mobile detection inside component to avoid SSR issues.
Checking navigator.userAgent at the module level can cause hydration mismatches in SSR environments and makes the code harder to test.
Consider moving the mobile detection inside the component or using a safer approach:
-const isMobile = /iPhone|iPad|iPod|Android/i.test(navigator.userAgent);
-
export const config = createConfig({
chains: [mainnet],
connectors: [
walletConnect({
projectId: process.env.REACT_APP_WC_ID ?? '',
- showQrModal: !isMobile,
+ showQrModal: typeof window !== 'undefined' ? !/iPhone|iPad|iPod|Android/i.test(navigator.userAgent) : true,
// ... rest of config
}),
],
// ... rest of config
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isMobile = /iPhone|iPad|iPod|Android/i.test(navigator.userAgent); | |
| export const config = createConfig({ | |
| chains: [mainnet], | |
| connectors: [ | |
| walletConnect({ | |
| projectId: process.env.REACT_APP_WC_ID ?? '', | |
| showQrModal: typeof window !== 'undefined' | |
| ? !/iPhone|iPad|iPod|Android/i.test(navigator.userAgent) | |
| : true, | |
| // ... rest of config | |
| }), | |
| ], | |
| // ... rest of config | |
| }); |
🤖 Prompt for AI Agents
In src/containers/Main.tsx at line 330, the mobile detection using
navigator.userAgent is done at the module level, which can cause SSR hydration
mismatches. Move the mobile detection logic inside the React component function
or use a useEffect hook to set a state variable for isMobile after the component
mounts, ensuring it only runs on the client side and avoids SSR issues.
src/pages/Login.tsx
Outdated
| const provider = await walletConnectConnector.getProvider(); | ||
| provider.once('display_uri', (uri: string) => { | ||
| const encodedURI = encodeURIComponent(uri); | ||
| window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add cleanup and validation for the WalletConnect provider.
The current implementation has potential issues:
- No cleanup if the component unmounts before the event fires
- No validation that
uriis a valid string - No loading state while waiting for the URI
Consider implementing proper cleanup and validation:
const listenForWalletConnectUri = async () => {
if (!walletConnectConnector) {
throw new Error('WalletConnect connector not found');
}
connect({ connector: walletConnectConnector });
const provider = await walletConnectConnector.getProvider();
- provider.once('display_uri', (uri: string) => {
- const encodedURI = encodeURIComponent(uri);
- window.location.href = `pillarwallet://wc?uri=${encodedURI}`;
- });
+ provider.once('display_uri', (uri: string) => {
+ if (typeof uri !== 'string' || !uri) {
+ console.error('Invalid WalletConnect URI received');
+ return;
+ }
+ try {
+ const encodedURI = encodeURIComponent(uri);
+ window.location.href = `pillarwallet://wc?uri=${encodedURI}`;
+ } catch (error) {
+ console.error('Failed to process WalletConnect URI:', error);
+ }
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const provider = await walletConnectConnector.getProvider(); | |
| provider.once('display_uri', (uri: string) => { | |
| const encodedURI = encodeURIComponent(uri); | |
| window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | |
| }); | |
| const provider = await walletConnectConnector.getProvider(); | |
| - provider.once('display_uri', (uri: string) => { | |
| - const encodedURI = encodeURIComponent(uri); | |
| - window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | |
| - }); | |
| + provider.once('display_uri', (uri: string) => { | |
| + if (typeof uri !== 'string' || !uri) { | |
| + console.error('Invalid WalletConnect URI received'); | |
| + return; | |
| + } | |
| + try { | |
| + const encodedURI = encodeURIComponent(uri); | |
| + window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | |
| + } catch (error) { | |
| + console.error('Failed to process WalletConnect URI:', error); | |
| + } | |
| + }); |
🤖 Prompt for AI Agents
In src/pages/Login.tsx around lines 34 to 38, the WalletConnect provider event
listener lacks cleanup on component unmount, does not validate that the uri is a
valid string, and does not handle a loading state while waiting for the URI. To
fix this, add a cleanup function to remove the 'display_uri' event listener when
the component unmounts, validate that the uri parameter is a non-empty string
before using it, and implement a loading state to indicate waiting for the URI
before redirecting.
| const listenForWalletConnectUri = async () => { | ||
| if (!walletConnectConnector) { | ||
| throw new Error('WalletConnect connector not found'); | ||
| } | ||
|
|
||
| connect({ connector: walletConnectConnector }); | ||
|
|
||
| const provider = await walletConnectConnector.getProvider(); | ||
| provider.once('display_uri', (uri: string) => { | ||
| const encodedURI = encodeURIComponent(uri); | ||
| window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Add error handling for the WalletConnect connection flow.
The listenForWalletConnectUri function throws an error but it's called directly in the onClick handler without any error handling. This could lead to unhandled promise rejections.
Apply this diff to add proper error handling:
- <Button
- onClick={listenForWalletConnectUri}
- $last
- $fullWidth
- >{t`action.connectPillarWallet`}</Button>
+ <Button
+ onClick={async () => {
+ try {
+ await listenForWalletConnectUri();
+ } catch (error) {
+ console.error('Failed to connect wallet:', error);
+ // Consider showing a user-friendly error message
+ }
+ }}
+ $last
+ $fullWidth
+ >{t`action.connectPillarWallet`}</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const listenForWalletConnectUri = async () => { | |
| if (!walletConnectConnector) { | |
| throw new Error('WalletConnect connector not found'); | |
| } | |
| connect({ connector: walletConnectConnector }); | |
| const provider = await walletConnectConnector.getProvider(); | |
| provider.once('display_uri', (uri: string) => { | |
| const encodedURI = encodeURIComponent(uri); | |
| window.location.href = `pillarwallet://wc?uri=${encodedURI}`; | |
| }); | |
| }; | |
| <Button | |
| onClick={async () => { | |
| try { | |
| await listenForWalletConnectUri(); | |
| } catch (error) { | |
| console.error('Failed to connect wallet:', error); | |
| // Consider showing a user-friendly error message | |
| } | |
| }} | |
| $last | |
| $fullWidth | |
| >{t`action.connectPillarWallet`}</Button> |
🤖 Prompt for AI Agents
In src/pages/Login.tsx around lines 27 to 39, the listenForWalletConnectUri
function can throw errors that are not caught when called directly in the
onClick handler, leading to unhandled promise rejections. To fix this, wrap the
call to listenForWalletConnectUri in a try-catch block or handle the promise
rejection properly where it is invoked, ensuring any errors are caught and
handled gracefully, such as by showing an error message or logging the error.
|
Updated UI 
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/pages/Login.tsx (3)
37-41: Add cleanup and validation for the WalletConnect provider.The implementation still lacks cleanup if the component unmounts before the event fires, validation that
uriis a valid string, and loading state handling.
30-42: Add error handling for the WalletConnect connection flow.The
listenForWalletConnectUrifunction can throw errors but is called directly in the onClick handler without error handling, leading to potential unhandled promise rejections.
44-50:⚠️ Potential issueAvoid page reload for state synchronization.
The current implementation still reloads the page when an address is detected, which provides poor user experience and indicates potential state management issues. Additionally, there's a logic error:
sessionStorage.setItem('loginPageReloaded', 'true')is called afterwindow.location.reload(), so it will never execute.Apply this diff to fix the logic error:
useEffect(() => { const reloaded = sessionStorage.getItem('loginPageReloaded'); if (!address || reloaded === 'true') return; - window.location.reload(); - sessionStorage.setItem('loginPageReloaded', 'true'); + window.location.reload(); }, [address]);However, consider avoiding page reloads entirely by using proper state management.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)src/pages/Login.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.example
🔇 Additional comments (3)
src/pages/Login.tsx (3)
2-3: Import additions look good.The new imports for
useEffect,useWallets,useConnect, anduseAccountare properly structured and necessary for the wallet integration functionality.Also applies to: 7-7
77-83: UI layout improvements enhance user experience.The new
InsideWrappercontainer and structured button layout improve the visual organization of the login options. The addition of the Pillar Wallet icon enhances user recognition.
99-106: Well-structured styled component.The
InsideWrapperstyled component provides appropriate spacing and layout for the button container with clear flexbox styling.
| logout(); | ||
| } | ||
|
|
||
| await disconnect(); |
There was a problem hiding this comment.
What if the user is not connected via Wagmi? Should it not be an if condition here to check if the user has connected with Wagmi before disconnecting via Wagmi?
There was a problem hiding this comment.
Added condition here.
src/containers/Main.tsx
Outdated
| defaultChain: isTestnet ? sepolia : mainnet, | ||
| embeddedWallets: { | ||
| createOnLogin: 'users-without-wallets', | ||
| showWalletUIs: false, |
There was a problem hiding this comment.
Removed it.
| const [t] = useTranslation(); | ||
|
|
||
| // Get WalletConnect connector | ||
| const walletConnectConnector = connectors.find( |
There was a problem hiding this comment.
What if the user connects via WalletConnect using another app and not the Pillar Wallet?
There was a problem hiding this comment.
In mobile, Directly open Pillar App. Desktop user can connect with different app.
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
|
Need to resolve the checks that were not successful before I can approve please |
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
jest.config.json (1)
17-17: Regex looks fine, but consider matching the full@wagmi/*scope explicitly.
"node_modules/(?!(mime|wagmi|@wagmi)/)"works, yet@wagmiis only the scope – real paths are@wagmi/core,@wagmi/connectors, etc.
For tighter intent (and to avoid accidentally transforming an unrelated future package also starting with@wagmi), you can anchor the pattern to the scope:- "transformIgnorePatterns": ["node_modules/(?!(mime|wagmi|@wagmi)/)"] + "transformIgnorePatterns": ["node_modules/(?!(mime|wagmi|@wagmi/.*)/)"]No functional change today, but it prevents surprises if other scoped packages appear.
src/test-utils/setupJest.ts (1)
122-124: Align connector mock signature with actualwalletConnectexport.The real API is:
import { walletConnect } from 'wagmi/connectors' walletConnect({ projectId, chains })If your components supply arguments, returning a plain
jest.fn()is OK, but add a default resolver to avoidundefined is not a functionwhen they attempt to call methods on the connector instance.-jest.mock('wagmi/connectors', () => ({ - walletConnect: jest.fn(), +jest.mock('wagmi/connectors', () => ({ + walletConnect: jest.fn(() => ({ id: 'walletConnect', name: 'WalletConnect', type: 'walletConnect' })), }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
jest.config.json(1 hunks)src/containers/Main.test.tsx(1 hunks)src/test-utils/setupJest.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: unit-tests
🔇 Additional comments (1)
src/test-utils/setupJest.ts (1)
110-120: Exportingmainnetwith onlyidmay break chain-aware logic.Your app sometimes calls
mainnet.name/nativeCurrency(see e.g.NetworkIconcomponent).
Expose at least the commonly used fields:- mainnet: { id: 1, name: 'mainnet' }, + mainnet: { id: 1, name: 'Mainnet', nativeCurrency: { symbol: 'ETH', decimals: 18 } },
| jest.mock('wagmi', () => { | ||
| const actualWagmi = jest.requireActual('wagmi'); | ||
| return { | ||
| ...actualWagmi, | ||
| createConfig: jest.fn(() => ({})), | ||
| WagmiProvider: ({ children }: any) => <div>{children}</div>, | ||
| http: jest.fn(), | ||
| mainnet: { id: 1 }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicated & incomplete wagmi mock – consolidate into setupJest.ts.
A project-wide mock for wagmi already exists in src/test-utils/setupJest.ts.
Re-declaring it here
- risks the two mocks diverging,
- hides
WagmiProvideronly for this test file, and - introduces JSX (
<div>{children}</div>) without importingReact– this is fine only if the new JSX transform is enabled intsconfig& Babel/Jest.
Consider deleting these lines and, if the wrapper provider is required everywhere, extending the central mock once:
- jest.mock('wagmi', () => {
- const actualWagmi = jest.requireActual('wagmi');
- return {
- ...actualWagmi,
- createConfig: jest.fn(() => ({})),
- WagmiProvider: ({ children }: any) => <div>{children}</div>,
- http: jest.fn(),
- mainnet: { id: 1 },
- };
- });
+ // remove – use the mock defined in setupJest.tsIf you really need a custom wrapper only for this test, import React and compose with jest.requireActual('wagmi').WagmiProvider instead of fully shadowing it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mock('wagmi', () => { | |
| const actualWagmi = jest.requireActual('wagmi'); | |
| return { | |
| ...actualWagmi, | |
| createConfig: jest.fn(() => ({})), | |
| WagmiProvider: ({ children }: any) => <div>{children}</div>, | |
| http: jest.fn(), | |
| mainnet: { id: 1 }, | |
| }; | |
| }); | |
| // remove – use the mock defined in setupJest.ts |
🤖 Prompt for AI Agents
In src/containers/Main.test.tsx lines 6 to 15, remove the duplicated and
incomplete wagmi mock since a project-wide mock already exists in
src/test-utils/setupJest.ts. If you need a custom WagmiProvider wrapper for this
test, import React and compose it with the actual WagmiProvider from the central
mock instead of fully shadowing it. This avoids divergence, ensures consistent
mocking, and prevents JSX usage without importing React.
| jest.mock('wagmi', () => ({ | ||
| createConfig: jest.fn(), | ||
| http: jest.fn(), | ||
| mainnet: { id: 1, name: 'mainnet' }, | ||
| useAccount: jest.fn().mockReturnValue({ | ||
| address: '0x', | ||
| }), | ||
| useDisconnect: jest.fn().mockReturnValue({ | ||
| disconnect: jest.fn(), | ||
| }), | ||
| })); |
There was a problem hiding this comment.
Provide a minimal but valid return value for createConfig.
Components that call createConfig() may access fields such as publicClient or connectors.
Returning undefined (current behaviour) can cause hard-to-trace Cannot read property '...' of undefined in tests that import other components.
-jest.mock('wagmi', () => ({
- createConfig: jest.fn(),
+jest.mock('wagmi', () => ({
+ // return an object with the keys your code expects
+ createConfig: jest.fn(() => ({ connectors: [], publicClient: {} })),Even a stubbed shape prevents runtime errors and keeps the test surface realistic.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mock('wagmi', () => ({ | |
| createConfig: jest.fn(), | |
| http: jest.fn(), | |
| mainnet: { id: 1, name: 'mainnet' }, | |
| useAccount: jest.fn().mockReturnValue({ | |
| address: '0x', | |
| }), | |
| useDisconnect: jest.fn().mockReturnValue({ | |
| disconnect: jest.fn(), | |
| }), | |
| })); | |
| jest.mock('wagmi', () => ({ | |
| // return an object with the keys your code expects | |
| createConfig: jest.fn(() => ({ connectors: [], publicClient: {} })), | |
| http: jest.fn(), | |
| mainnet: { id: 1, name: 'mainnet' }, | |
| useAccount: jest.fn().mockReturnValue({ | |
| address: '0x', | |
| }), | |
| useDisconnect: jest.fn().mockReturnValue({ | |
| disconnect: jest.fn(), | |
| }), | |
| })); |
🤖 Prompt for AI Agents
In src/test-utils/setupJest.ts around lines 110 to 120, the jest.mock for
'wagmi' returns undefined for createConfig, causing runtime errors when
components access properties like publicClient or connectors on its result.
Update the mock to return a minimal but valid object from createConfig,
including stubbed fields such as publicClient and connectors, to prevent
undefined property access errors and keep the test environment realistic.
|
Fixed lint and test cases. |
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
github-actions
bot
requested a deployment
to
Preview (improvement/pillar_wallet_connect_button)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/pages/Login.tsx (3)
28-41: Critical issues remain unresolved in WalletConnect implementation.The
listenForWalletConnectUrifunction still has the same issues identified in previous reviews:
- No cleanup for event listeners on component unmount
- No validation that
uriis a valid string- No error handling when called from onClick
- Uses
anytype for provider
43-49: Page reload approach remains problematic.This useEffect still uses
window.location.reload()for state synchronization, which was previously flagged as providing poor user experience and indicating potential state management issues.
51-55: Automatic wallet login without user consent persists.The automatic execution of
wallets[0].loginOrLink()when wallets become available was previously flagged as potentially violating user expectations and wallet security best practices.
🧹 Nitpick comments (2)
src/components/Form/Select/index.tsx (1)
29-31: Consider optimizing chain name detection.The current logic performs a linear search through all chain names for each option. Consider memoizing the chain names set for better performance.
+const chainNames = useMemo( + () => new Set(CompatibleChains.map((chains) => chains.chainName)), + [] +); + -const isChainName = CompatibleChains.map((chains) => chains.chainName).some( - (chain) => option.title.includes(chain) -); +const isChainName = Array.from(chainNames).some((chain) => + option.title.includes(chain) +);src/components/Form/AssetSelect/index.tsx (1)
150-157: Optimize effect dependencies to prevent unnecessary re-renders.The
assetsarray in the dependency list will cause the effect to re-run whenever the array reference changes, even if the content is the same. Sinceassetsis already derived fromwalletPortfolioDatawhich is tracked viaisWalletPortfolioDataSuccess, includingassetsis redundant.- }, [ - walletAddress, - chainId, - isWalletPortfolioDataSuccess, - isWalletPortfolioDataLoading, - isWalletPortfolioDataFetching, - assets, - ]); + }, [ + walletAddress, + chainId, + isWalletPortfolioDataSuccess, + isWalletPortfolioDataLoading, + isWalletPortfolioDataFetching, + walletPortfolioData, + nfts, + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pillarx-app/components/PortfolioOverview/test/__snapshots__/PortfolioOverview.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokensSlider/test/__snapshots__/TokenSlider.test.tsx.snapis excluded by!**/*.snapsrc/components/BottomMenu/__snapshots__/BottomMenu.test.tsx.snapis excluded by!**/*.snapsrc/components/Button/__snapshots__/Button.test.tsx.snapis excluded by!**/*.snapsrc/components/Form/Select/__snapshots__/Select.test.tsx.snapis excluded by!**/*.snapsrc/components/Text/Paragraph/__snapshots__/Paragraph.test.tsx.snapis excluded by!**/*.snapsrc/components/Text/Title/__snapshots__/Title.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (24)
src/apps/pillarx-app/components/RandomAvatar/RandomAvatar.tsx(2 hunks)src/components/BottomMenu/BottomMenu.test.tsx(1 hunks)src/components/BottomMenuModal/AccountModal.tsx(12 hunks)src/components/BottomMenuModal/SendModal/SendModal.test.skip.tsx(1 hunks)src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx(1 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(10 hunks)src/components/BottomMenuModal/index.tsx(1 hunks)src/components/Form/AssetSelect/index.tsx(8 hunks)src/components/Form/Select/Select.test.tsx(3 hunks)src/components/Form/Select/index.tsx(4 hunks)src/containers/Authorized.tsx(1 hunks)src/hooks/__tests__/useAccountBalance.test.tsx(0 hunks)src/hooks/useAccountBalances.tsx(0 hunks)src/hooks/useAssets.tsx(0 hunks)src/pages/Login.tsx(3 hunks)src/providers/AccountBalancesProvider.tsx(0 hunks)src/providers/AccountNftsProvider.tsx(1 hunks)src/providers/AccountTransactionHistoryProvider.tsx(2 hunks)src/providers/AssetsProvider.tsx(0 hunks)src/providers/__tests__/AccountBalancesProvider.test.tsx(0 hunks)src/providers/__tests__/AssetsProvider.test.tsx(0 hunks)src/services/tokensData.ts(2 hunks)src/types/index.ts(2 hunks)src/utils/common.ts(1 hunks)
💤 Files with no reviewable changes (7)
- src/hooks/tests/useAccountBalance.test.tsx
- src/hooks/useAssets.tsx
- src/providers/tests/AssetsProvider.test.tsx
- src/providers/AssetsProvider.tsx
- src/providers/tests/AccountBalancesProvider.test.tsx
- src/hooks/useAccountBalances.tsx
- src/providers/AccountBalancesProvider.tsx
✅ Files skipped from review due to trivial changes (3)
- src/providers/AccountNftsProvider.tsx
- src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx
- src/providers/AccountTransactionHistoryProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/BottomMenuModal/AccountModal.tsx
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/containers/Authorized.tsx (2)
src/providers/WalletConnectToastProvider.tsx (1)
WalletConnectToastProvider(28-100)src/providers/WalletConnectModalProvider.tsx (1)
WalletConnectModalProvider(23-84)
src/components/Form/Select/index.tsx (1)
src/utils/blockchain.ts (1)
CompatibleChains(260-289)
src/components/Form/AssetSelect/index.tsx (1)
src/services/tokensData.ts (2)
convertPortfolioAPIResponseToToken(97-121)chainNameToChainIdTokensData(234-255)
src/types/index.ts (1)
src/services/tokensData.ts (1)
Token(19-29)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
src/apps/the-exchange/utils/wrappedTokens.ts (1)
isNativeToken(29-30)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (24)
src/pages/Login.tsx (5)
2-3: LGTM! Clean import additions for wagmi integration.The new imports for wagmi hooks and the Pillar wallet icon are appropriate for the WalletConnect functionality being added.
Also applies to: 7-7, 14-14
18-20: LGTM! Proper hook initialization.The wagmi and Privy hooks are correctly initialized for wallet connection functionality.
23-26: LGTM! WalletConnect connector identification.The logic to find the WalletConnect connector from available connectors is straightforward and appropriate.
76-82: LGTM! Clean button implementation with proper styling.The new button structure with the wallet icon and proper event handler is well-implemented. The InsideWrapper provides good organization for the buttons.
98-105: LGTM! Appropriate styling for button container.The InsideWrapper styling provides proper vertical centering and layout for the login buttons.
src/utils/common.ts (1)
3-8: LGTM! Improved import organization.Adding comment sections for "types" and "utils" improves code readability and organization without affecting functionality.
src/components/BottomMenu/BottomMenu.test.tsx (1)
59-73: LGTM! Test setup aligns with provider refactoring.The removal of
AssetsProviderandAccountBalancesProviderfrom the test setup correctly reflects the broader refactoring that eliminates these providers from the codebase. The simplified provider hierarchy maintains test coverage while removing deprecated dependencies.src/components/BottomMenuModal/index.tsx (1)
40-40: LGTM! Dependency array correctly updated.The removal of
AccountBalancesContextfrom the dependency array aligns with the elimination of balance context usage in the useEffect hook. This correctly reflects the simplified update logic that now only manages NFT data updates.src/components/BottomMenuModal/SendModal/SendModal.test.skip.tsx (1)
54-60: LGTM! Test provider hierarchy simplified appropriately.The removal of
AccountBalancesProviderfrom the test setup is consistent with the broader refactoring that eliminates this provider from the codebase. The test maintains its coverage while reflecting the updated application structure.src/apps/pillarx-app/components/RandomAvatar/RandomAvatar.tsx (1)
8-8: LGTM! Clean enhancement for avatar shape control.The
isRoundprop addition is well-implemented with appropriate default value and correct logic mapping to the underlyingsquareattribute.Also applies to: 15-15, 44-44
src/components/Form/Select/Select.test.tsx (3)
6-12: LGTM! Improved import organization.Separating the
SelectOptiontype import aligns with the shared types refactoring.
104-105: LGTM! Simplified test assertion.Replacing complex DOM traversal with
getByTextmakes the test more maintainable and less brittle.
152-158: LGTM! Better test targeting with data-testid.Using
data-testidattribute for test targeting is a best practice that makes tests more reliable and maintainable.src/containers/Authorized.tsx (1)
58-74: ```shell
#!/bin/bashRe-run search without relying on built-in tsx/ts types
rg -n -C 3 -e "AssetsProvider|AccountBalancesProvider|useAssets|useAccountBalances" -g '*.{ts,tsx}'
</details> <details> <summary>src/components/Form/Select/index.tsx (3)</summary> `7-8`: **LGTM! Consistent with shared types refactoring.** Moving `SelectOption` to shared types improves maintainability and consistency across the codebase. --- `48-52`: **LGTM! Good fallback UI for options without images.** The RandomAvatar fallback for non-chain options without images improves the user experience by providing consistent visual representation. --- `61-65`: **LGTM! Enhanced testability with data-testid.** Adding the `data-testid` attribute aligns with testing best practices and supports the improved test assertions. </details> <details> <summary>src/types/index.ts (4)</summary> `1-3`: **LGTM! Appropriate imports for NFT types.** The imports from `@etherspot/data-utils` and local `tokensData` service are correctly typed and necessary for the new interfaces. --- `41-46`: **LGTM! Well-structured TokenAssetSelectOption interface.** The interface properly extends `SelectOption` and includes all necessary properties for token asset handling including balance and chain information. --- `48-53`: **LGTM! Comprehensive NftAssetSelectOption interface.** The interface appropriately handles NFT-specific properties including the NFT object, collection, and chain information. --- `55-63`: **LGTM! Clean type organization.** The `AssetSelectOption` union type and base `SelectOption` interface provide a solid foundation for asset selection functionality across the application. </details> <details> <summary>src/components/Form/AssetSelect/index.tsx (1)</summary> `172-173`: **Add null safety for optional balance field.** The balance is retrieved from an optional field but used without null checking, which could result in undefined behavior. ```diff - const balance = assetOption.asset?.balance ?? 0; + const balance = assetOption.asset?.balance ?? 0;Note: While the nullish coalescing operator is already used here correctly, ensure this pattern is consistently applied throughout the codebase where balance is accessed.
Likely an incorrect or invalid review comment.
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
254-254: LGTM! Proper null handling for balance field.The nullish coalescing operator correctly handles the optional balance field.
src/services/tokensData.ts (1)
178-188: Well-implemented PnL calculation with proper edge case handling.The calculation correctly handles the -100% price change scenario and prevents division by zero errors.
| (selectedAsset?.type !== 'token' || isValidAmount(amount)) && | ||
| (selectedAsset?.type !== 'token' || | ||
| +getAmountLeft(selectedAsset, amount, selectedAssetBalance) >= 0); | ||
| +getAmountLeft(selectedAsset, amount, selectedAsset.balance) >= 0); |
There was a problem hiding this comment.
Add null check for balance in transaction validation.
The selectedAsset.balance is passed to getAmountLeft without checking if it exists, which could cause the validation to fail incorrectly.
- +getAmountLeft(selectedAsset, amount, selectedAsset.balance) >= 0);
+ +getAmountLeft(selectedAsset, amount, selectedAsset.balance ?? 0) >= 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +getAmountLeft(selectedAsset, amount, selectedAsset.balance) >= 0); | |
| +getAmountLeft(selectedAsset, amount, selectedAsset.balance ?? 0) >= 0); |
🤖 Prompt for AI Agents
In src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx at line
467, the code passes selectedAsset.balance directly to getAmountLeft without
verifying if balance is null or undefined. Add a null check for
selectedAsset.balance before passing it to getAmountLeft to ensure the
validation does not fail incorrectly when balance is missing.
| if (selectedAsset?.type !== 'token' || !selectedAsset.balance) return 0; | ||
|
|
||
| const adjustedBalance = isNativeToken(selectedAsset.asset.contract) | ||
| ? selectedAssetBalance - deploymentCost | ||
| : selectedAssetBalance; | ||
| ? selectedAsset.balance - deploymentCost | ||
| : selectedAsset.balance; | ||
|
|
||
| return isAmountInputAsFiat | ||
| ? selectedAssetPrice * adjustedBalance | ||
| : adjustedBalance; | ||
| }, [ | ||
| selectedAsset, | ||
| deploymentCost, | ||
| isAmountInputAsFiat, | ||
| selectedAssetPrice, | ||
| selectedAssetBalance, | ||
| ]); | ||
| }, [selectedAsset, deploymentCost, isAmountInputAsFiat, selectedAssetPrice]); |
There was a problem hiding this comment.
Ensure null safety when accessing optional balance field.
The component accesses selectedAsset.balance which could be undefined. While there's a truthy check, the mathematical operations could produce NaN if balance is undefined.
- if (selectedAsset?.type !== 'token' || !selectedAsset.balance) return 0;
+ if (selectedAsset?.type !== 'token' || selectedAsset.balance == null) return 0;
const adjustedBalance = isNativeToken(selectedAsset.asset.contract)
- ? selectedAsset.balance - deploymentCost
- : selectedAsset.balance;
+ ? Math.max(0, selectedAsset.balance - deploymentCost)
+ : selectedAsset.balance;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (selectedAsset?.type !== 'token' || !selectedAsset.balance) return 0; | |
| const adjustedBalance = isNativeToken(selectedAsset.asset.contract) | |
| ? selectedAssetBalance - deploymentCost | |
| : selectedAssetBalance; | |
| ? selectedAsset.balance - deploymentCost | |
| : selectedAsset.balance; | |
| return isAmountInputAsFiat | |
| ? selectedAssetPrice * adjustedBalance | |
| : adjustedBalance; | |
| }, [ | |
| selectedAsset, | |
| deploymentCost, | |
| isAmountInputAsFiat, | |
| selectedAssetPrice, | |
| selectedAssetBalance, | |
| ]); | |
| }, [selectedAsset, deploymentCost, isAmountInputAsFiat, selectedAssetPrice]); | |
| if (selectedAsset?.type !== 'token' || selectedAsset.balance == null) return 0; | |
| const adjustedBalance = isNativeToken(selectedAsset.asset.contract) | |
| ? Math.max(0, selectedAsset.balance - deploymentCost) | |
| : selectedAsset.balance; | |
| return isAmountInputAsFiat | |
| ? selectedAssetPrice * adjustedBalance | |
| : adjustedBalance; | |
| }, [selectedAsset, deploymentCost, isAmountInputAsFiat, selectedAssetPrice]); |
🤖 Prompt for AI Agents
In src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx around
lines 441 to 450, the code accesses selectedAsset.balance without fully ensuring
it is defined, which can cause NaN in calculations. To fix this, explicitly
check that selectedAsset.balance is a valid number before performing arithmetic
operations, and provide a default value or return 0 if it is undefined or not a
number to ensure null safety.
| blockchain: chainIdToChainNameTokensData( | ||
| Number(contract.chainId.split(':')[1]) | ||
| ), |
There was a problem hiding this comment.
Add validation when parsing chainId from string.
The chainId is extracted by splitting a string and parsing it to a number without validation. This could fail if the format is unexpected.
- blockchain: chainIdToChainNameTokensData(
- Number(contract.chainId.split(':')[1])
- ),
+ blockchain: (() => {
+ const parts = contract.chainId.split(':');
+ const chainIdNum = parts.length > 1 ? Number(parts[1]) : undefined;
+ return chainIdToChainNameTokensData(chainIdNum);
+ })(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| blockchain: chainIdToChainNameTokensData( | |
| Number(contract.chainId.split(':')[1]) | |
| ), | |
| blockchain: (() => { | |
| const parts = contract.chainId.split(':'); | |
| const chainIdNum = parts.length > 1 ? Number(parts[1]) : undefined; | |
| return chainIdToChainNameTokensData(chainIdNum); | |
| })(), |
🤖 Prompt for AI Agents
In src/services/tokensData.ts around lines 110 to 112, the code extracts chainId
by splitting a string and converting the second part to a number without
validation, which may cause errors if the format is unexpected. Add validation
to check that contract.chainId contains a colon and that the part after the
colon is a valid number before parsing. Handle cases where the format is invalid
by providing a fallback or throwing a clear error.
github-actions
bot
temporarily deployed
to
Preview (fix/wc_login_request)
3 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2025-06-03.at.11.46.16.mp4
Types of changes
Summary by CodeRabbit