Conversation
fix: remove network cost loading dep from req pay
fix: reown appkit config
fix: close kyc modal on done click
fix: update appkit config
fix: chevron and clear icons overalapping in address input
fix: claim cta changes on navigation b/w intial and confirm view
fix: clear icon visibility
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request refactors multiple components across the codebase by enhancing cross-chain transaction logic, state management, and UI behavior. In the Claim components, new constants and props are introduced to distinguish between same-chain and cross-chain transactions and to better manage route selection. Global components receive improvements in messaging handling and rendering, while Request/Pay and configuration files are updated to streamline loading conditions, imports, and provider settings. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant I as InitialClaimLinkView
participant S as State Handler
participant C as ConfirmClaimLinkView
U->>I: Initiate claim/swap action
I->>S: Evaluate isSameChainAndToken & isCrossChainTransaction
alt Cross-chain transaction
S-->>I: Set cross-chain state
I->>I: Proceed with swap workflow
else Same-chain transaction
S-->>I: Update state for direct claim
I->>I: Proceed with claim workflow
end
U->>C: Click back button
C->>C: Invoke handleGoBack to reset route state
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/Global/ValidatedInput/index.tsx (2)
117-117: Fix redundant double negation.The double negation
!!infoTextis unnecessary as the value will already be coerced to a boolean.- !!infoText ? 'pl-8' : 'pl-4' + infoText ? 'pl-8' : 'pl-4'🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
144-148: Consider adding aria-label for loading state.The loading indicator should have proper accessibility attributes.
- <div className="flex h-full w-12 items-center justify-center dark:bg-n-1"> + <div + className="flex h-full w-12 items-center justify-center dark:bg-n-1" + aria-label="Loading" + role="status" + >src/components/Global/IframeWrapper/index.tsx (1)
119-119: Review polling frequency change.The polling interval has been reduced from 3s to 1.5s. Consider the performance implications of increased polling frequency in production.
Consider implementing exponential backoff or WebSocket connection for real-time updates instead of frequent polling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/Claim/Link/Initial.view.tsx(6 hunks)src/components/Claim/Link/Onchain/Confirm.view.tsx(3 hunks)src/components/Global/IframeWrapper/index.tsx(5 hunks)src/components/Global/ValidatedInput/index.tsx(5 hunks)src/components/Request/Pay/Views/Initial.view.tsx(1 hunks)src/config/peanut.config.tsx(2 hunks)src/config/wagmi.config.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/Request/Pay/Views/Initial.view.tsx (2)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.
🪛 Biome (1.9.4)
src/components/Global/ValidatedInput/index.tsx
[error] 117-117: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (13)
src/config/peanut.config.tsx (2)
19-20:Details
❓ Verification inconclusive
Verify Google Analytics configuration.
Ensure that the Google Analytics key is properly set in the environment variables for production.
🏁 Script executed:
#!/bin/bash # Check if GA key is defined in environment files fd "^\.env" --type f --exec grep -l "NEXT_PUBLIC_GA_KEY"Length of output: 264
Action Required: Update GA Key Verification
The current configuration initializes Google Analytics usinguseEffect(() => { ReactGA.initialize(process.env.NEXT_PUBLIC_GA_KEY ?? '')However, the original verification script didn’t yield results because hidden files (like your .env files) were excluded from the search. Please ensure that the production environment properly sets the
NEXT_PUBLIC_GA_KEY. In addition, use the updated verification command below to search hidden files:#!/bin/bash # Check if GA key is defined in environment files (including hidden files) fd "^\.env" --type f --hidden --exec grep -l "NEXT_PUBLIC_GA_KEY"Verify that this command outputs the file(s) where the GA key is defined. If no results are returned, double-check your environment configuration to confirm that the key is correctly set for production.
29-29:Details
❓ Verification inconclusive
Verify the impact of setting cookies to null.
Setting
cookies={null}explicitly might affect state persistence. This change appears to be related to the updatedContextProvidersignature in wagmi.config.tsx.
🏁 Script executed:
#!/bin/bash # Search for any persistence-related code that might be affected rg -A 5 "cookies|persist|storage" --type ts --type tsxLength of output: 84
Below is a revised review comment. Since the original persistence-related search returned an error due to TSX file type issues, please manually verify the impact of setting
cookies={null}or re-run the enhanced search command (see below) that explicitly registers TSX files. This extra check will confirm that no persistence logic is unexpectedly affected by this change.Next steps:
Ensure TSX files are properly searched:
Run this command to register TSX files with ripgrep and search for persistence-related terms:#!/bin/bash # Add TSX file type, then search for any persistence-related code occurrences. rg --type-add 'tsx:*.tsx' rg -A 5 "cookies|persist|storage" --type ts --type tsxManually verify state persistence:
Check that settingcookies={null}in<config.ContextProvider>(line 29 insrc/config/peanut.config.tsx) is fully compatible with the updated ContextProvider signature inwagmi.config.tsxand does not break any persistence expectations across the codebase.Please confirm these points before merging.
Revised Review Comment
Verify the impact of explicitly settingcookies={null}in<config.ContextProvider>(line 29 insrc/config/peanut.config.tsx). There is a risk that this change could affect state persistence across the app. Please run the above script to search for any persistence-related code in both TS and TSX files. Once the updated search confirms that no storage or persistence logic is inadvertently affected, manually ensure that this configuration change is in line with the updatedContextProvidersignature fromwagmi.config.tsx.
🏁 Script executed:
[run_scripts] #!/bin/bash # Register TSX file type and search for persistence-related references in TypeScript and TSX files. rg --type-add 'tsx:*.tsx' && rg -A 5 "cookies|persist|storage" --type ts --type tsxLength of output: 546
Review Update: Verify Persistence Impact
The updated search using the revised command returned no output, suggesting there are no additional persistence-related logic references (e.g., for cookies, persistence, or storage) in our TS and TSX files. However, setting
cookies={null}in<config.ContextProvider>(src/config/peanut.config.tsx, line 29) remains worth a manual review to ensure that state persistence is not inadvertently affected—especially in relation to the updated ContextProvider signature in wagmi.config.tsx.
- Action Items:
- Run the above script to double-check for any persistence logic that might relate to cookies.
- Manually verify that the explicit setting of
cookies={null}is intentional and does not break state persistence elsewhere.Please confirm these points before merging.
🏁 Script executed:
[run_scripts] #!/bin/bash # Register TSX file type for this search and search for persistence-related references in TypeScript and TSX files. rg --type-add 'tsx:*.tsx' -A 5 "cookies|persist|storage" --type ts --type tsx .Length of output: 14063
Action: Verify Cookies Persistence Setting
The explicit setting of
cookies={null}in<config.ContextProvider>(line 29 insrc/config/peanut.config.tsx) is intended to align with the updated ContextProvider signature inwagmi.config.tsx. However, please verify that passingnulldoes not unintentionally disrupt state persistence. Notably, theContextProviderinwagmi.config.tsxprocesses thecookiesprop viacookieToInitialState, which may rely on real cookie data for persisting state.
- In
src/config/peanut.config.tsx:
- The component sets
<config.ContextProvider cookies={null}>instead of providing an actual cookies string.- In
src/config/wagmi.config.tsx:
- The
ContextProviderconsumes this prop and derives an initial state withcookieToInitialState. Make sure that anullvalue yields the expected fallback behavior.- Across the codebase:
- The search for persistence-related logic (covering keywords like
cookies,persist, andstorage) revealed multiple API route references for cookie handling. Although these references are used in API endpoints rather than in the front-end configuration, confirm that no related persistence logic inadvertently depends on having a valid cookies string.Please manually review and test the behavior of state persistence to ensure that this configuration change meets the intended design.
src/config/wagmi.config.tsx (1)
33-36:Details
❓ Verification inconclusive
Verify network configuration for production.
The networks array defines the supported blockchain networks. Ensure all required networks for production are included and properly configured.
🏁 Script executed:
#!/bin/bash # Search for network usage across the codebase rg -A 5 "chain(Id|Name|s)" --type ts --type tsxLength of output: 77
I'll generate a new shell script that searches for the networks configuration across both TypeScript (.ts) and TSX (.tsx) files without relying on pre-defined file types. This should help us verify that the required networks (e.g., arbitrum, mainnet, optimism, etc.) are indeed referenced in the codebase.
Please run the following script:
#!/bin/bash # Re-run search for the networks configuration to verify usage across .ts and .tsx files rg -A 5 "export const networks" -g "*.ts" -g "*.tsx"Once we have the results from this command, we can finalize the review comment accordingly.
src/components/Global/IframeWrapper/index.tsx (1)
187-187:Details
❓ Verification inconclusive
Verify iframe sandbox permissions.
The iframe has scroll enabled and includes various sandbox permissions. Ensure these are the minimum required permissions for security.
🌐 Web query:
What are the recommended sandbox permissions for iframes in production?💡 Result:
When configuring iframe sandbox permissions for production environments, security best practices emphasize applying maximum restrictions by default and selectively enabling only necessary capabilities. Here's a structured approach based on industry standards:
Default Configuration
Start with no permissions (empty
sandboxattribute) to apply strict security measures:<iframe src="external-content" sandbox></iframe>This blocks:
- Script execution
- Form submissions
- Popup windows
- Same-origin privileges
- Plugin execution
- Top-level navigation[1][6][9]
Selectively Enable Features
Add permissions only when required by functionality:
<!-- Example: Allow forms + scripts --> <iframe src="form-app" sandbox="allow-forms allow-scripts"></iframe>
Common Permissions Use Case Security Consideration allow-formsForm submissions Validate form endpoints[1][9] allow-scriptsJS-dependent content Never combine with allow-same-origin[3][6]allow-popupsControlled new windows Use allow-popups-to-escape-sandboxcautiously[9]allow-presentationScreen sharing Require explicit user consent[9] Critical Restrictions to Maintain
Never combine:
<!-- Dangerous configuration --> <iframe sandbox="allow-scripts allow-same-origin"></iframe>This allows embedded content to bypass security policies[3][6][7]
Block by default:
allow-top-navigation(prevents URL hijacking)allow-pointer-lock(mouse control)allow-storage-access(local data)[1][6][9]Production Best Practices
- Test extensively in staging environments before deployment[1]
- Combine with Content Security Policy:
Content-Security-Policy: frame-src 'self' https://trusted-provider.com;- Regularly audit permission needs - remove unused allowances[1][2]
- For third-party content:
<iframe sandbox="allow-forms" src="https://third-party.com" referrerpolicy="no-referrer"> </iframe>- Use feature policy headers for cross-origin frames[4][10]:
Permissions-Policy: camera=(), microphone=(), geolocation=()Security teams at Adobe and Google recommend maintaining this balance - strict defaults with surgical permission grants - as the most effective defense against clickjacking, XSS attacks, and data exfiltration via iframes[5][7][8].
Citations:
- 1: https://www.dhiwise.com/post/iframe-sandbox-attribute-a-secure-embedded-content-solution
- 2: https://www.reflectiz.com/blog/iframe-security/
- 3: https://cloud.google.com/blog/products/data-analytics/iframe-sandbox-tutorial
- 4: https://www.w3.org/TR/permissions-policy/
- 5: https://developer.adobe.com/express/add-ons/docs/guides/develop/context/
- 6: https://developer.mozilla.org/en-US/docs/Learn_web_development/Core/Structuring_content/General_embedding_technologies
- 7: https://github.com/w3c/webappsec-permissions-policy/blob/master/sandbox.md
- 8: https://www.infoq.com/news/2010/01/HTML-5-Sandbox-IFrame/
- 9: http://www.w3schools.com/Tags/att_iframe_sandbox.asp
- 10: https://developer.mozilla.org/en-US/docs/Web/HTTP/Permissions_Policy
Security Update: Audit & Minimize iframe Sandbox Attributes
Based on industry best practices, please review the iframe sandbox settings in this component. For production, it's recommended to start with no permissions (i.e., an empty sandbox attribute) and then, if needed, to selectively enable only the minimal set of features (such as
allow-formsorallow-scripts). Avoid dangerous combinations—for example, usingallow-scriptstogether withallow-same-origincan considerably weaken your security posture.Action Items:
- Verify that the iframe in the IframeWrapper component is not over-permissive.
- Adjust the sandbox attribute to include only permissions that are strictly required for the functionality.
- Reassess any configuration that might conflict with these recommendations.
src/components/Claim/Link/Onchain/Confirm.view.tsx (3)
28-29: LGTM! Props addition enhances state management.The new props
setSelectedRouteandsetHasFetchedRouteenable proper state reset during navigation.
102-108: LGTM! Clean implementation of back navigation.The
handleGoBackfunction properly resets route selection state before navigating back, preventing stale state issues.
261-265: LGTM! Button handler update maintains consistency.The "Go Back" button now uses
handleGoBackwhile maintaining the same disabled state based onisLoading.src/components/Request/Pay/Views/Initial.view.tsx (1)
447-447: LGTM! Simplified loading state logic.The loading condition has been streamlined to only check if the estimated fee is zero or if the payment hasn't been initiated by the user.
src/components/Claim/Link/Initial.view.tsx (5)
295-298: LGTM! Improved readability with descriptive constant.The
isSameChainAndTokenconstant makes the chain and token comparison logic more readable and maintainable.
299-308: LGTM! Proper state reset on chain/token changes.The effect properly handles state reset when chains or tokens change, preventing stale route data.
398-403: LGTM! Centralized cross-chain transaction check.Using
useMemoforisCrossChainTransactionoptimizes performance and centralizes the cross-chain logic.
406-418: LGTM! Automatic route fetching on cross-chain changes.The effect automatically fetches new routes when cross-chain status changes, improving user experience.
495-533: LGTM! UI updates reflect cross-chain state.The UI now correctly displays route information based on the cross-chain transaction status.
| * @param {Record<string, string>} cookies - An object representing the cookies. | ||
| * @returns {InitialState} The initial state derived from the cookies. | ||
| */ | ||
| const initialState = cookieToInitialState(wagmiAdapter.wagmiConfig as Config, cookies) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify cookie to state conversion.
The cookieToInitialState conversion is crucial for maintaining user state. Ensure proper error handling for invalid cookie data.
Add error handling:
- const initialState = cookieToInitialState(wagmiAdapter.wagmiConfig as Config, cookies)
+ let initialState
+ try {
+ initialState = cookieToInitialState(wagmiAdapter.wagmiConfig as Config, cookies)
+ } catch (error) {
+ console.error('Failed to parse cookie state:', error)
+ initialState = undefined
+ }📝 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 initialState = cookieToInitialState(wagmiAdapter.wagmiConfig as Config, cookies) | |
| let initialState | |
| try { | |
| initialState = cookieToInitialState(wagmiAdapter.wagmiConfig as Config, cookies) | |
| } catch (error) { | |
| console.error('Failed to parse cookie state:', error) | |
| initialState = undefined | |
| } |
| const handleMessage = (event: MessageEvent) => { | ||
| if (event.data?.name === 'complete' && event.data?.metadata?.status === 'completed') { | ||
| onClose() | ||
| } | ||
| } |
There was a problem hiding this comment.
Verify origin of iframe messages.
The message event handler should validate the origin of messages for security.
- const handleMessage = (event: MessageEvent) => {
+ const handleMessage = (event: MessageEvent) => {
+ const allowedOrigins = [process.env.NEXT_PUBLIC_BASE_URL]
+ if (!allowedOrigins.includes(event.origin)) {
+ console.warn(`Received message from unauthorized origin: ${event.origin}`)
+ return
+ }
if (event.data?.name === 'complete' && event.data?.metadata?.status === 'completed') {
onClose()
}
}📝 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 handleMessage = (event: MessageEvent) => { | |
| if (event.data?.name === 'complete' && event.data?.metadata?.status === 'completed') { | |
| onClose() | |
| } | |
| } | |
| const handleMessage = (event: MessageEvent) => { | |
| const allowedOrigins = [process.env.NEXT_PUBLIC_BASE_URL] | |
| if (!allowedOrigins.includes(event.origin)) { | |
| console.warn(`Received message from unauthorized origin: ${event.origin}`) | |
| return | |
| } | |
| if (event.data?.name === 'complete' && event.data?.metadata?.status === 'completed') { | |
| onClose() | |
| } | |
| } |
feat: add maintenance banner with configurable time window
Fix/usdt cashout
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Global/Banner/MaintenanceBanner.tsx (2)
16-18: Consider extracting the time check to a helper function.The logic here is correct, but extracting the condition into a clearly named helper function (e.g.,
isInMaintenanceWindow()) could improve readability and maintainability.
23-28: Unused helper and minimal user context.
formatDateis introduced but never called, and the banner text doesn't include the actual maintenance window. Consider either removingformatDateor leveraging it to display maintenance start/end times for better user clarity.src/config/routesUnderMaintenance.ts (1)
24-27: Clarify the time zone or usage context.Defining the start/end in UTC is a solid choice, but if your user base spans multiple time zones, consider adding disclaimers or converting times locally to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Global/Banner/MaintenanceBanner.tsx(1 hunks)src/config/routesUnderMaintenance.ts(2 hunks)src/constants/cashout.consts.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/constants/cashout.consts.ts
🔇 Additional comments (4)
src/components/Global/Banner/MaintenanceBanner.tsx (2)
11-15: Ensure robust date parsing.If
config.maintenanceTimeis malformed or invalid,new Date(...)could produceNaN, causing the subsequent checks to fail silently and skip displaying a needed banner. Please validate or handle invalid date strings appropriately.
19-19: Good approach to conditionally render the banner.This condition aligns seamlessly with the updated time-based approach for maintenance checks. Nicely done.
src/config/routesUnderMaintenance.ts (2)
3-6: Validate maintenanceTime format.Storing times as a string is fine, but please ensure they conform to a valid format (e.g., ISO8601). Invalid strings could produce
NaNtimestamps in downstream code.
19-22: Expanded routes are consistent.Including all maintainable routes aligns with the final plan. No issues here.
3 participants
Summary by CodeRabbit