-
Notifications
You must be signed in to change notification settings - Fork 13
[TASK-13082] Sprint 100 prod release #989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Stop using the skd and use the squid API directly, this give us more control and access to all the data that returns squid (for example, we now have access to the fees and don't have to recalculate them ourselves)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Use coral through squid to get the cross-chain route for the different flows. This enables xchain withdraw for peanut wallet
…ss-chain-action-card
[TASK-11579] feat: add cross-chain action card
Coral integration to staging
* feat: show sponsored by peanut message This message is shown when we are making a transaction from the peanut wallet. If the amount is more than one cent we also show the fee that the user is saving by using peanut. * fix(fees): show correct ammounts for externalwallet * refactor: rename estimatedGasCostUsd to estimatedGasCostUsdUsd
Two things here: 1. We were using the peanut address for approval checking on deposits, this didn't affect because if the approval call failed we just continued with the deposit 2: The base rpc url was the sepolia one, not the mainnet. Big oversight there
* feat: add alachemy as fallback rpc url * fix: remove commented line * fix: address pr review comments
* fix: small fixes around crosschain ui * fix: avoid negative expiry time
… 2 txs in the history (#943) * PATCH implemented in frontend * comment input state updating correctly (onBlur) * freezing amount after updating comments/files input * debounce attachment options and update requests on file change (avoiding race conditions when attaching files!) * style: Apply prettier formatting * removed malicious code Signed-off-by: facundobozzi <[email protected]> * PATCH method using withFormData * better onBlur logic * bug fixes * blur logic fixed * nit pick comments * code rabbit suggestion * replaced useEffect setting state with derived state and debouncing * disabling amount input after request creation * linting --------- Signed-off-by: facundobozzi <[email protected]>
* fix: update desktop navigation with add/withdraw paths * fix: support page
* refactor: fetch external wallet balances from mobula (#956) * fix: update desktop navigation (#970) * fix: support page prod (#971) * fix: update desktop navigation * fix: support page * [TASK-12746] feat: beta changes (#975) * feat: beta changes - Banner that redirects to support page - Funds warning modal * refactor(banner): remove from landing and add hand thumbs up --------- Co-authored-by: Kushagra Sarathe <[email protected]>
* ENS/eth address network error being displayed on the frontend * linting * fixed andre QA observations * deleting console.log * formatting * logic fixed * fixed testst --------- Signed-off-by: facundobozzi <[email protected]>
* fix: slider tap bug logic * fix: tap bug on slider
* removed legacy button and changed try now button colors * try now button color and hero description * YourMoney component * NoFees component first part * stars done * scribble in zero fees * no hidden fees section done * securityBuiltIn done * sendInSeconds progress * sendInSeconds done * updated try now button to be a link * business integration initiation * businessIntegrate done * order and marquee reusability * order fixed * background animations in Landing Page compoejnnts * changed image for text in hero section * figma details * landing page responsiveness improvements * landing page responsiveness done * coderabbit suggestion * business integration button * fixed iphone screenshots * CTA button animation * added debugging for CTA button animation * changed animation since react gave errors * arrows in hero section * NoFees new Zero * no fees responsiveness fixed * sendInSeconds details done * coderabbit suggestions * formatting * hand-token wasnt pushed? forgot to stage or sum? anyways fixed * build issues fixed * coderabbit optimization * code formatting * arrows and ZERO thickness * shade in try now button * progress in button animation * undoing button changes * small detail fixes * added asset * changed peanut business to svg * integrate peanut component done * added pay-zero-fees.svg * added new illustrations * uout money anywhere svg * securitybuiltin component SVGs * adding YourMoney new SVGs * your money anywhere component * instantly send & receive svg * arrows fixed * button and arrows done * desktop sendinseconds button done * removed arrows disappearing effect * MOBILE: hero button done * added mobile svg * yourMoney responsive component done * added mobile-zero-fees.svg * added no-hidden-fees.svg * noFees mobile svg * noFees desktop + mobile improvements * noFees done * mobile security built in SVG * securityprivacy mobile done * mobile-send-in-seconds.svg * sendInSeconds mobile done * business integrate mobile * business integrate button and final details * removed footer * formatting * removed pino-pretty * fixed button instead of sticky/floating --------- Signed-off-by: facundobozzi <[email protected]>
* feat: allow uppercase in url * fix: allow uppercase in recipient
- Slider instead of button - Expiry for local storage instead of session storage - Warning icon color
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces major enhancements and refactors across the application. It adds new landing page sections with animated and interactive elements, a cross-chain swap service, and a balance warning modal. It refactors payment, withdrawal, and request flows for improved state management, removes legacy fee estimation logic, and updates environment configurations. Numerous UI components receive improved styling, copy-to-clipboard support, and context-driven state. New utility hooks, a slider component, and expanded token and route handling are also included. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
🔭 Outside diff range comments (1)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
71-81
: Rename function to reflect its broader purposeThe function
getBicAndRoutingNumber
now also handles CLABE identifiers, making the name misleading.Consider renaming to better reflect its purpose:
-const getBicAndRoutingNumber = () => { +const getBankIdentifier = () => { if (bankAccount && bankAccount.type === AccountType.IBAN) { return bankAccount.bic?.toUpperCase() ?? 'N/A' } else if (bankAccount && bankAccount.type === AccountType.US) { return bankAccount.routingNumber?.toUpperCase() ?? 'N/A' } else if (bankAccount && bankAccount.type === AccountType.CLABE) { return bankAccount.identifier?.toUpperCase() ?? 'N/A' } return 'N/A' }Then update the usage:
-<PaymentInfoRow label="BIC" value={getBicAndRoutingNumber()} /> +<PaymentInfoRow label="BIC" value={getBankIdentifier()} />
🧹 Nitpick comments (27)
src/config/justaname.config.tsx (1)
8-17
: Consider adding error handling for empty RPC URL arrays.The current implementation assumes
rpcUrls[mainnet.id]
contains at least one URL when it exists. However, if the array is empty after filtering,mainnetRpcUrl
would beundefined
, leading to an emptynetworks
array.Consider adding a check to ensure the RPC URL array is not empty:
-const mainnetRpcUrl = rpcUrls[mainnet.id]?.[0] +const mainnetRpcUrls = rpcUrls[mainnet.id] +const mainnetRpcUrl = mainnetRpcUrls?.length > 0 ? mainnetRpcUrls[0] : undefinedThis would make the fallback behavior more explicit and robust.
src/components/Global/ScreenOrientationLocker.tsx (1)
9-11
: Consider improving type safety for screen orientation API.The current implementation uses
as any
type casting which bypasses TypeScript's type checking. This could lead to runtime errors if the API changes.Consider adding proper type definitions or using feature detection:
-if (screen.orientation && (screen.orientation as any).lock) { +if (screen.orientation && 'lock' in screen.orientation && typeof (screen.orientation as any).lock === 'function') {Or better yet, define proper TypeScript interfaces for the Screen Orientation API to improve type safety across the codebase.
src/utils/sdk.utils.ts (3)
15-15
: Consider using number type for chainId parameter.The function accepts
chainId
as a string but immediately converts it to a number on line 20. Consider accepting it as a number directly to match therpcUrls
mapping and avoid unnecessary conversion.-export async function getSDKProvider({ chainId }: { chainId: string }): Promise<providers.Provider | undefined> { +export async function getSDKProvider({ chainId }: { chainId: number }): Promise<providers.Provider | undefined> {Then update the cache key logic:
- if (providerCache.has(chainId)) { - return providerCache.get(chainId) + if (providerCache.has(chainId.toString())) { + return providerCache.get(chainId.toString()) } - const urls = rpcUrls[Number(chainId)] + const urls = rpcUrls[chainId]
22-22
: Fix typo in comment.There's a typo in the comment: "f we have" should be "If we have".
- // f we have specific RPC URLs, use them with a FallbackProvider for resiliency. + // If we have specific RPC URLs, use them with a FallbackProvider for resiliency.
35-36
: Consider using numeric chainId for cache key consistency.The cache key uses the string
chainId
but the function logic would be cleaner with numeric keys throughout. This aligns with the suggestion in the function signature.src/components/LandingPage/yourMoney.tsx (2)
15-15
: Use more specific type for titleSvg property.The
titleSvg
property usesany
type which reduces type safety. Consider using a more specific type likeStaticImageData
from Next.js or a union type of the imported SVGs.interface Feature { id: number title: string - titleSvg: any + titleSvg: StaticImageData description: string - imageSrc: any + imageSrc: StaticImageData imageAlt: string }
99-99
: Consider extracting inline styles to CSS classes.The inline
letterSpacing
style could be extracted to a CSS class for better maintainability.- <p - className="w-full max-w-[360px] text-left font-roboto text-lg font-normal leading-relaxed md:text-lg" - style={{ letterSpacing: '-0.5px' }} - > + <p className="w-full max-w-[360px] text-left font-roboto text-lg font-normal leading-relaxed tracking-tight md:text-lg">Note: You may need to add a custom tracking class if
-0.5px
doesn't match existing Tailwind tracking utilities.src/components/Global/Banner/index.tsx (1)
29-29
: Consider using a route constantThe route
/support
is hardcoded. Consider using a constant for better maintainability.You could define routes in a constants file and import them:
+import { ROUTES } from '@/constants/routes' const handleClick = () => { - router.push('/support') + router.push(ROUTES.SUPPORT) }src/components/LandingPage/sendInSeconds.tsx (1)
24-24
: Extract magic numbers as constantsThe default viewport width and other numeric values should be defined as constants.
Consider extracting these values:
+const DEFAULT_VIEWPORT_WIDTH = 1080 +const CLOUD_SPEED_SLOW = 30 +const CLOUD_SPEED_MEDIUM = 35 +const CLOUD_SPEED_FAST = 45 const createCloudAnimation = (side: 'left' | 'right', width: number, speed: number) => { - const vpWidth = screenWidth || 1080 + const vpWidth = screenWidth || DEFAULT_VIEWPORT_WIDTHsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
48-49
: Consider making the default IBAN country configurableHardcoding Germany ('DE') as the default country for IBAN accounts may not be appropriate for all users.
Consider deriving the country from the IBAN itself or making it configurable:
case AccountType.IBAN: - // Default to a European country that uses EUR/SEPA - countryId = 'DE' // Germany as default EU country + // Try to extract country from IBAN (first 2 chars) or use configurable default + countryId = account.identifier?.substring(0, 2).toUpperCase() || 'DE' breaksrc/components/Global/RouteExpiryTimer/index.tsx (2)
48-48
: Specify radix parameter for parseIntWhile ES5+ defaults to base 10, it's best practice to be explicit.
-const expiryTime = parseInt(expiry) * 1000 +const expiryTime = parseInt(expiry, 10) * 1000
68-117
: Consider using document visibility API to pause timerThe timer continues running even when the page is not visible, which wastes resources.
You could pause the timer when the document is hidden:
useEffect(() => { const handleVisibilityChange = () => { if (document.hidden) { // Pause the timer } else { // Resume and recalculate } } document.addEventListener('visibilitychange', handleVisibilityChange) return () => document.removeEventListener('visibilitychange', handleVisibilityChange) }, [])src/components/LandingPage/hero.tsx (2)
126-136
: Consider removing unused code.The
arrowOpacity
is hardcoded to 1 and the sparkle rendering is commented out. Consider removing these if they're no longer needed.const renderCTAButton = (cta: CTAButton, variant: 'primary' | 'secondary') => { - const arrowOpacity = 1 // Always visible - return ( <motion.div className={getButtonContainerClasses(variant)} initial={getInitialAnimation(variant)} animate={getAnimateAnimation(variant, buttonVisible, buttonScale)} whileHover={getHoverAnimation(variant)} transition={transitionConfig} > - {/* {renderSparkle(variant)} */} -
183-189
: Consider using CSS classes instead of inline styles.For consistency with the rest of the codebase, consider moving inline styles to Tailwind classes or CSS modules.
-<span - className="mt-2 block text-xl leading-tight text-n-1 md:mt-4 md:text-5xl" - style={{ fontWeight: 500, letterSpacing: '-0.5px' }} -> +<span className="mt-2 block text-xl font-medium leading-tight tracking-tight text-n-1 md:mt-4 md:text-5xl">src/components/Global/PeanutActionDetailsCard/index.tsx (2)
35-42
: Consider grouping timer-related props in a separate interface.The component now has many timer-related props that could be better organized for maintainability.
+interface TimerConfig { + show: boolean + expiry?: string + isLoading?: boolean + onNearExpiry?: () => void + onExpired?: () => void + disableRefetch?: boolean + error?: string | null +} interface PeanutActionDetailsCardProps { // ... existing props - showTimer?: boolean - timerExpiry?: string - isTimerLoading?: boolean - onTimerNearExpiry?: () => void - onTimerExpired?: () => void - disableTimerRefetch?: boolean - timerError?: string | null + timer?: TimerConfig }Then update the usage:
-{showTimer && ( +{timer?.show && ( <RouteExpiryTimer - expiry={timerExpiry} - isLoading={isTimerLoading} - onNearExpiry={onTimerNearExpiry} - onExpired={onTimerExpired} - disableRefetch={disableTimerRefetch} - error={timerError} + expiry={timer.expiry} + isLoading={timer.isLoading ?? false} + onNearExpiry={timer.onNearExpiry} + onExpired={timer.onExpired} + disableRefetch={timer.disableRefetch ?? false} + error={timer.error ?? null} /> )}
138-139
: Simplify boolean logic.The condition can be simplified using the existing variables.
-if (isWithdrawBankAccount || isAddBankAccount) +const isBankAccountTransaction = isWithdrawBankAccount || isAddBankAccount +if (isBankAccountTransaction)index.ts (1)
11-11
: Consider aligning the export name with the file name.The export
HandToken
points tonew-hand-token.svg
, which suggests a file rename. For consistency, consider either renaming the export toNewHandToken
or updating the file name tohand-token.svg
.-export { default as HandToken } from './new-hand-token.svg' +export { default as NewHandToken } from './new-hand-token.svg'src/components/Withdraw/views/Confirm.withdraw.view.tsx (2)
25-37
: Consider adding JSDoc comments for the new props.The new cross-chain related props would benefit from documentation explaining their purpose and expected values.
Add JSDoc comments:
interface WithdrawConfirmViewProps { amount: string token: ITokenPriceData chain: interfaces.ISquidChain & { tokens: interfaces.ISquidToken[] } toAddress: string networkFee?: number peanutFee?: string onConfirm: () => void onBack: () => void isProcessing?: boolean error?: string | null - // Timer props for cross-chain withdrawals + /** Whether this is a cross-chain withdrawal requiring route tracking */ isCrossChain?: boolean + /** ISO string timestamp when the cross-chain route expires */ routeExpiry?: string + /** Whether the cross-chain route is currently being calculated */ isRouteLoading?: boolean + /** Callback to refresh the cross-chain route when nearing expiry */ onRouteRefresh?: () => void + /** The cross-chain route details from Squid API */ xChainRoute?: PeanutCrossChainRoute }
63-68
: Consider extracting stable coin check to a shared utility.The stable coin formatting logic
isStableCoin(resolvedTokenSymbol) ? $ ${amount} : ${amount} ${resolvedTokenSymbol}
appears in multiple components. Consider creating a shared formatting utility.Create a utility function:
export function formatTokenAmount(amount: string, tokenSymbol: string): string { return isStableCoin(tokenSymbol) ? `$ ${amount}` : `${amount} ${tokenSymbol}` }Then use it:
- return isStableCoin(resolvedTokenSymbol) ? `$ ${amount}` : `${amount} ${resolvedTokenSymbol}` + return formatTokenAmount(amount, resolvedTokenSymbol)src/components/Request/link/views/Create.request.link.view.tsx (1)
262-268
: Consider adding null check for attachment properties.While the current implementation works, it would be more robust to handle potential undefined values in the attachment comparison.
Add null-safe comparison:
const hasUnsavedChanges = useMemo(() => { if (!requestId) return false const lastSaved = lastSavedAttachmentRef.current - return lastSaved.message !== attachmentOptions.message || lastSaved.rawFile !== attachmentOptions.rawFile + return (lastSaved.message ?? '') !== (attachmentOptions.message ?? '') || + lastSaved.rawFile !== attachmentOptions.rawFile }, [requestId, attachmentOptions.message, attachmentOptions.rawFile])src/app/actions/tokens.ts (1)
194-204
: Consider adding error context to cached functions.The cached gas price function should include error context for debugging when fetching fails.
Add error logging:
const getCachedGasPrice = unstable_cache( async (chainId: string) => { - const client = await getPublicClient(Number(chainId) as ChainId) - const gasPrice = await client.getGasPrice() - return gasPrice.toString() + try { + const client = await getPublicClient(Number(chainId) as ChainId) + const gasPrice = await client.getGasPrice() + return gasPrice.toString() + } catch (error) { + console.error(`Failed to fetch gas price for chain ${chainId}:`, error) + throw error + } }, ['getGasPrice'], { revalidate: 2 * 60, // 2 minutes } )src/app/(mobile-ui)/withdraw/crypto/page.tsx (3)
96-98
: Remove debug console.log statements.Debug logging should be removed or replaced with proper logging that can be controlled via environment variables.
Remove or conditionally log:
- console.log('Preparing withdraw transaction details...') - console.dir(activeChargeDetailsFromStore) + if (process.env.NODE_ENV === 'development') { + console.log('Preparing withdraw transaction details...', activeChargeDetailsFromStore) + }
238-239
: Remove debug console.log statements.Debug statements should be removed from production code.
Remove the debug logs:
- console.log('Refreshing withdraw route due to expiry...') - console.log('About to call prepareTransactionDetails with:', activeChargeDetailsFromStore)
267-273
: Consider removing verbose debug logging.The cross-chain check debug log contains sensitive information and should be removed or made conditional.
Make logging conditional:
- console.log('Cross-chain check:', { - fromChainId, - toChainId, - isPeanutWallet, - isCrossChain: fromChainId !== toChainId, - }) + if (process.env.NODE_ENV === 'development') { + console.log('Cross-chain check:', { fromChainId, toChainId, isCrossChain: fromChainId !== toChainId }) + }src/components/Payment/Views/Confirm.payment.view.tsx (2)
178-181
: Consider validating USD amount for direct payments.When
isDirectUsdPayment
is true but the currency is not USD, the behavior might be unexpected. Consider adding validation or documentation about this edge case.Add validation or clarify the logic:
const usdAmount = isDirectUsdPayment && chargeDetails.currencyCode.toLowerCase() === 'usd' ? chargeDetails.currencyAmount : undefined + // Log warning if direct USD payment requested but currency is not USD + if (isDirectUsdPayment && chargeDetails.currencyCode.toLowerCase() !== 'usd') { + console.warn('Direct USD payment requested but charge currency is not USD') + }
402-410
: Good code reuse pattern!The min received calculation logic is identical to the one in the withdraw confirmation view. This is a good candidate for extraction into a shared utility as suggested earlier.
src/hooks/usePaymentInitiator.ts (1)
646-654
: Consider extracting complex conditional logic for readabilityThe condition for transitioning to the Confirm view is quite complex. Consider extracting it into a well-named helper function for better readability and maintainability.
+ const shouldShowConfirmView = ( + chargeCreated: boolean, + payload: InitiatePaymentPayload, + isPeanutWallet: boolean, + chargeDetails: TRequestChargeResponse + ) => { + if (!chargeCreated) return false; + if (payload.isPintaReq || payload.isAddMoneyFlow) return true; + if (!isPeanutWallet) return true; + // For Peanut Wallet, show confirm if not using native USDC on Peanut chain + return !areEvmAddressesEqual(chargeDetails.tokenAddress, PEANUT_WALLET_TOKEN) || + chargeDetails.chainId !== PEANUT_WALLET_CHAIN.id.toString(); + }; + if ( - chargeCreated && - (payload.isPintaReq || - payload.isAddMoneyFlow || - !isPeanutWallet || - (isPeanutWallet && - (!areEvmAddressesEqual(determinedChargeDetails.tokenAddress, PEANUT_WALLET_TOKEN) || - determinedChargeDetails.chainId !== PEANUT_WALLET_CHAIN.id.toString()))) + shouldShowConfirmView(chargeCreated, payload, isPeanutWallet, determinedChargeDetails) ) {
} | ||
|
||
// some browsers might not support addEventListener on screen.orientation | ||
if (screen.orientation && screen.orientation.addEventListener) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Apply static analysis suggestion: Use optional chaining for safer property access.
The current code checks for screen.orientation
existence but then accesses screen.orientation.addEventListener
directly, which could cause issues if the property exists but addEventListener
doesn't.
Apply the static analysis suggestion to use optional chaining:
-if (screen.orientation && screen.orientation.addEventListener) {
+if (screen.orientation?.addEventListener) {
This is more concise and handles the case where screen.orientation
exists but addEventListener
might not be available.
📝 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 (screen.orientation && screen.orientation.addEventListener) { | |
if (screen.orientation?.addEventListener) { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In src/components/Global/ScreenOrientationLocker.tsx at line 29, replace the
current conditional check that uses separate existence checks for
screen.orientation and screen.orientation.addEventListener with a single
optional chaining expression. This means changing the condition to use
screen.orientation?.addEventListener to safely access addEventListener only if
screen.orientation is defined, making the code more concise and preventing
potential runtime errors.
|
||
const fallbackProvider = new providers.FallbackProvider(providerConfigs, 1) // Quorum of 1, we only need one to work. | ||
|
||
await fallbackProvider.getNetwork() // this checks if at least one provider is responsive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify provider responsiveness check effectiveness.
The getNetwork()
call is a good way to verify provider responsiveness, but consider if additional validation might be needed for specific chain compatibility.
🏁 Script executed:
#!/bin/bash
# Verify if getNetwork() is sufficient for all supported chains
ast-grep --pattern $'getNetwork()'
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Verify presence of getNetwork() call and inspect FallbackProvider setup in sdk.utils.ts
rg -n 'getNetwork' src/utils/sdk.utils.ts
rg -n 'new .*FallbackProvider' src/utils/sdk.utils.ts
Length of output: 423
Ensure chain ID matches expected network after responsiveness check
While fallbackProvider.getNetwork()
confirms at least one RPC endpoint is responsive, it doesn’t guarantee it’s for the intended chain. After calling getNetwork()
, explicitly validate the returned chainId
(or network.name
) against your expected network to catch misconfigured RPC URLs.
• File: src/utils/sdk.utils.ts around line 33
• Replace the one-liner with a network check:
- await fallbackProvider.getNetwork() // this checks if at least one provider is responsive.
+ const network = await fallbackProvider.getNetwork() // verifies provider and returns network info
+ if (network.chainId !== EXPECTED_CHAIN_ID) {
+ throw new Error(
+ `Unexpected network: expected ${EXPECTED_CHAIN_ID}, got ${network.chainId}`
+ )
+ }
Make sure EXPECTED_CHAIN_ID
comes from your config/constants.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/sdk.utils.ts at line 33, after calling
fallbackProvider.getNetwork(), add a check to verify that the returned network's
chainId matches the EXPECTED_CHAIN_ID from your config or constants. If the
chainId does not match, handle the error appropriately to prevent using a
misconfigured RPC URL. Replace the current one-liner with this validation logic
to ensure the provider is connected to the intended network.
whileHover={getHoverAnimation()} | ||
transition={transitionConfig} | ||
> | ||
<a href="/send" className={getButtonClasses()} style={{ fontWeight: 900 }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Next.js Link component for internal navigation
Using a standard anchor tag prevents Next.js from optimizing the navigation.
Replace with Next.js Link component:
+import Link from 'next/link'
-<a href="https://github.com/send" className={getButtonClasses()} style={{ fontWeight: 900 }}>
+<Link href="https://github.com/send" className={getButtonClasses()} style={{ fontWeight: 900 }}>
TRY NOW
-</a>
+</Link>
🤖 Prompt for AI Agents
In src/components/LandingPage/sendInSeconds.tsx at line 232, replace the
standard anchor tag used for internal navigation with Next.js's Link component
to enable client-side navigation and optimization. Import Link from 'next/link'
if not already imported, then wrap the anchor content inside the Link component,
passing the href to Link and removing the href from the anchor tag itself. This
change will improve navigation performance and maintain Next.js best practices.
const [screenWidth, setScreenWidth] = useState(typeof window !== 'undefined' ? window.innerWidth : 1200) | ||
|
||
useEffect(() => { | ||
const handleResize = () => { | ||
setScreenWidth(window.innerWidth) | ||
} | ||
|
||
handleResize() | ||
window.addEventListener('resize', handleResize) | ||
return () => window.removeEventListener('resize', handleResize) | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve SSR safety and performance
The initial window check could cause SSR issues, and the resize handler could benefit from debouncing.
Consider this improved implementation:
-const [screenWidth, setScreenWidth] = useState(typeof window !== 'undefined' ? window.innerWidth : 1200)
+const [screenWidth, setScreenWidth] = useState(1200)
useEffect(() => {
+ let timeoutId: NodeJS.Timeout
const handleResize = () => {
- setScreenWidth(window.innerWidth)
+ clearTimeout(timeoutId)
+ timeoutId = setTimeout(() => {
+ setScreenWidth(window.innerWidth)
+ }, 150) // Debounce resize events
}
handleResize()
window.addEventListener('resize', handleResize)
- return () => window.removeEventListener('resize', handleResize)
+ return () => {
+ window.removeEventListener('resize', handleResize)
+ clearTimeout(timeoutId)
+ }
}, [])
📝 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 [screenWidth, setScreenWidth] = useState(typeof window !== 'undefined' ? window.innerWidth : 1200) | |
useEffect(() => { | |
const handleResize = () => { | |
setScreenWidth(window.innerWidth) | |
} | |
handleResize() | |
window.addEventListener('resize', handleResize) | |
return () => window.removeEventListener('resize', handleResize) | |
}, []) | |
const [screenWidth, setScreenWidth] = useState(1200) | |
useEffect(() => { | |
let timeoutId: NodeJS.Timeout | |
const handleResize = () => { | |
clearTimeout(timeoutId) | |
timeoutId = setTimeout(() => { | |
setScreenWidth(window.innerWidth) | |
}, 150) // Debounce resize events | |
} | |
handleResize() | |
window.addEventListener('resize', handleResize) | |
return () => { | |
window.removeEventListener('resize', handleResize) | |
clearTimeout(timeoutId) | |
} | |
}, []) |
🤖 Prompt for AI Agents
In src/components/LandingPage/sendInSeconds.tsx around lines 11 to 21, improve
SSR safety by initializing screenWidth state without directly accessing window
during render, for example by setting a default value and updating it inside
useEffect. Also, enhance performance by debouncing the resize event handler to
limit how often setScreenWidth is called during rapid window resizing. Implement
a debounce mechanism inside useEffect for the resize handler and clean it up
properly on unmount.
|
||
export const SQUID_INTEGRATOR_ID = '11CBA45B-5EE9-4331-B146-48CCD7ED4C7C' | ||
export const SQUID_API_URL = 'https://apiplus.squidrouter.com/v2' | ||
export const SQUID_INTEGRATOR_ID = process.env.SQUID_INTEGRATOR_ID! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid non-null assertion on environment variable
Using !
on process.env.SQUID_INTEGRATOR_ID
will cause a runtime error if the environment variable is not set.
Add proper validation:
-export const SQUID_INTEGRATOR_ID = process.env.SQUID_INTEGRATOR_ID!
+export const SQUID_INTEGRATOR_ID = process.env.SQUID_INTEGRATOR_ID
+
+if (!SQUID_INTEGRATOR_ID) {
+ throw new Error('SQUID_INTEGRATOR_ID environment variable is required')
+}
📝 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 SQUID_INTEGRATOR_ID = process.env.SQUID_INTEGRATOR_ID! | |
export const SQUID_INTEGRATOR_ID = process.env.SQUID_INTEGRATOR_ID | |
if (!SQUID_INTEGRATOR_ID) { | |
throw new Error('SQUID_INTEGRATOR_ID environment variable is required') | |
} |
🤖 Prompt for AI Agents
In src/constants/general.consts.ts at line 10, avoid using the non-null
assertion operator (!) on process.env.SQUID_INTEGRATOR_ID as it can cause
runtime errors if the variable is undefined. Instead, add validation to check if
the environment variable is set and handle the case when it is missing, such as
throwing a clear error or providing a fallback value.
fetchTokenPrice(params.fromToken, params.fromChain), | ||
]) | ||
if (!toTokenPrice) throw new Error('Could not fetch to token price') | ||
if (!fromTokenPrice) throw new Error('Could not fetch from token price') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle edge case where token prices are zero.
The code fetches token prices but doesn't validate that they're non-zero before using them in calculations, which could lead to division by zero or invalid amounts.
const [toTokenPrice, fromTokenPrice] = await Promise.all([
_toTokenPrice ? Promise.resolve(_toTokenPrice) : fetchTokenPrice(params.toToken, params.toChain),
fetchTokenPrice(params.fromToken, params.fromChain),
])
if (!toTokenPrice) throw new Error('Could not fetch to token price')
if (!fromTokenPrice) throw new Error('Could not fetch from token price')
+
+// Validate prices are valid numbers
+if (!toTokenPrice.price || toTokenPrice.price <= 0) {
+ throw new Error('Invalid to token price')
+}
+if (!fromTokenPrice.price || fromTokenPrice.price <= 0) {
+ throw new Error('Invalid from token price')
+}
📝 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.
fetchTokenPrice(params.fromToken, params.fromChain), | |
]) | |
if (!toTokenPrice) throw new Error('Could not fetch to token price') | |
if (!fromTokenPrice) throw new Error('Could not fetch from token price') | |
const [toTokenPrice, fromTokenPrice] = await Promise.all([ | |
_toTokenPrice | |
? Promise.resolve(_toTokenPrice) | |
: fetchTokenPrice(params.toToken, params.toChain), | |
fetchTokenPrice(params.fromToken, params.fromChain), | |
]) | |
if (!toTokenPrice) throw new Error('Could not fetch to token price') | |
if (!fromTokenPrice) throw new Error('Could not fetch from token price') | |
// Validate prices are valid numbers | |
if (!toTokenPrice.price || toTokenPrice.price <= 0) { | |
throw new Error('Invalid to token price') | |
} | |
if (!fromTokenPrice.price || fromTokenPrice.price <= 0) { | |
throw new Error('Invalid from token price') | |
} |
🤖 Prompt for AI Agents
In src/services/swap.ts around lines 276 to 279, after fetching the token
prices, add checks to ensure that both toTokenPrice and fromTokenPrice are not
zero. If either price is zero, throw an error to prevent division by zero or
invalid calculations. This validation should be done immediately after
confirming the prices are fetched successfully.
while (iterations < maxIterations && highBound > lowBound) { | ||
const midPoint = (lowBound + highBound) / 2n | ||
const testParams = { ...params, fromAmount: midPoint.toString() } | ||
|
||
try { | ||
const response = await getSquidRouteRaw(testParams) | ||
const receivedAmount = BigInt(response.route.estimate.toAmountMin) | ||
console.log('fromAmount', midPoint) | ||
console.log('receivedAmount', receivedAmount) | ||
console.log('targetToAmount', targetToAmount) | ||
if (receivedAmount >= targetToAmount) { | ||
iterations++ | ||
const diff = receivedAmount - targetToAmount | ||
const target = targetToAmount | ||
|
||
let overage: number | ||
if (diff <= Number.MAX_SAFE_INTEGER && target <= Number.MAX_SAFE_INTEGER) { | ||
overage = Number(diff) / Number(target) | ||
} else { | ||
// Handle very large numbers with careful scaling | ||
overage = Number(diff / (target / 1_000_000_000_000_000_000n)) / 1e18 | ||
} | ||
|
||
if (overage <= maxOverage) { | ||
return response | ||
} | ||
|
||
bestResult = { response, overage } | ||
highBound = midPoint - 1n | ||
} else { | ||
lowBound = midPoint + 1n | ||
} | ||
} catch (error) { | ||
console.warn('Error fetching route:', error) | ||
lowBound = midPoint + 1n | ||
iterations++ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add safeguards to prevent infinite loops in binary search.
The binary search could potentially run indefinitely if the bounds don't converge properly, and the current iteration limit of 3 might be too low for accurate results.
let bestResult: { response: SquidRouteResponse; overage: number } | null = null
let iterations = 0
-const maxIterations = 3 // Avoid too many calls to squid API!
+const maxIterations = 5 // Balance between accuracy and API calls
+const minSearchRange = 1000n // Stop if range is too small
// Binary search to find the optimal fromAmount
-while (iterations < maxIterations && highBound > lowBound) {
+while (iterations < maxIterations && highBound > lowBound && (highBound - lowBound) > minSearchRange) {
const midPoint = (lowBound + highBound) / 2n
+
+ // Prevent duplicate searches
+ if (midPoint === lowBound || midPoint === highBound) {
+ break
+ }
📝 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.
while (iterations < maxIterations && highBound > lowBound) { | |
const midPoint = (lowBound + highBound) / 2n | |
const testParams = { ...params, fromAmount: midPoint.toString() } | |
try { | |
const response = await getSquidRouteRaw(testParams) | |
const receivedAmount = BigInt(response.route.estimate.toAmountMin) | |
console.log('fromAmount', midPoint) | |
console.log('receivedAmount', receivedAmount) | |
console.log('targetToAmount', targetToAmount) | |
if (receivedAmount >= targetToAmount) { | |
iterations++ | |
const diff = receivedAmount - targetToAmount | |
const target = targetToAmount | |
let overage: number | |
if (diff <= Number.MAX_SAFE_INTEGER && target <= Number.MAX_SAFE_INTEGER) { | |
overage = Number(diff) / Number(target) | |
} else { | |
// Handle very large numbers with careful scaling | |
overage = Number(diff / (target / 1_000_000_000_000_000_000n)) / 1e18 | |
} | |
if (overage <= maxOverage) { | |
return response | |
} | |
bestResult = { response, overage } | |
highBound = midPoint - 1n | |
} else { | |
lowBound = midPoint + 1n | |
} | |
} catch (error) { | |
console.warn('Error fetching route:', error) | |
lowBound = midPoint + 1n | |
iterations++ | |
} | |
let bestResult: { response: SquidRouteResponse; overage: number } | null = null | |
let iterations = 0 | |
const maxIterations = 5 // Balance between accuracy and API calls | |
const minSearchRange = 1000n // Stop if range is too small | |
// Binary search to find the optimal fromAmount | |
while (iterations < maxIterations && highBound > lowBound && (highBound - lowBound) > minSearchRange) { | |
const midPoint = (lowBound + highBound) / 2n | |
// Prevent duplicate searches | |
if (midPoint === lowBound || midPoint === highBound) { | |
break | |
} | |
const testParams = { ...params, fromAmount: midPoint.toString() } | |
try { | |
const response = await getSquidRouteRaw(testParams) | |
const receivedAmount = BigInt(response.route.estimate.toAmountMin) | |
console.log('fromAmount', midPoint) | |
console.log('receivedAmount', receivedAmount) | |
console.log('targetToAmount', targetToAmount) | |
if (receivedAmount >= targetToAmount) { | |
iterations++ | |
const diff = receivedAmount - targetToAmount | |
const target = targetToAmount | |
let overage: number | |
if (diff <= Number.MAX_SAFE_INTEGER && target <= Number.MAX_SAFE_INTEGER) { | |
overage = Number(diff) / Number(target) | |
} else { | |
// Handle very large numbers with careful scaling | |
overage = Number(diff / (target / 1_000_000_000_000_000_000n)) / 1e18 | |
} | |
if (overage <= maxOverage) { | |
return response | |
} | |
bestResult = { response, overage } | |
highBound = midPoint - 1n | |
} else { | |
lowBound = midPoint + 1n | |
} | |
} catch (error) { | |
console.warn('Error fetching route:', error) | |
lowBound = midPoint + 1n | |
iterations++ | |
} | |
} |
🤖 Prompt for AI Agents
In src/services/swap.ts around lines 316 to 352, the binary search loop risks
running indefinitely if the bounds do not converge properly, and the iteration
limit of 3 is too low for accuracy. Increase the maxIterations to a higher value
to allow sufficient convergence attempts, and add additional safeguards such as
checking if the bounds have changed between iterations or breaking the loop if
no progress is made to prevent infinite looping.
useMemo(() => { | ||
if (isConnected && address) { | ||
setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString()) | ||
setSelectedTokenAddress(PEANUT_WALLET_TOKEN) | ||
} | ||
}, [ | ||
debouncedAttachmentOptions, | ||
debouncedTokenValue, | ||
isValidRecipient, | ||
isCreatingLink, | ||
generatedLink, | ||
_tokenValue, | ||
recipientAddress, | ||
]) | ||
|
||
// check for token value debouncing | ||
const isDebouncing = | ||
(hasAttachment && | ||
attachmentOptions?.message && | ||
(!debouncedAttachmentOptions?.message || | ||
debouncedAttachmentOptions.message !== attachmentOptions.message)) || | ||
(hasAttachment && _tokenValue !== debouncedTokenValue) | ||
}, [isConnected, address, setSelectedChainID, setSelectedTokenAddress]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect use of useMemo
- should use useEffect
for side effects.
useMemo
is for computing derived values, not for performing side effects like setting state. This code performs side effects (calling setters) which should be in useEffect
.
Replace with useEffect
:
- // Set wallet defaults when connected
- useMemo(() => {
- if (isConnected && address) {
- setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString())
- setSelectedTokenAddress(PEANUT_WALLET_TOKEN)
- }
- }, [isConnected, address, setSelectedChainID, setSelectedTokenAddress])
+ // Set wallet defaults when connected
+ useEffect(() => {
+ if (isConnected && address) {
+ setSelectedChainID(PEANUT_WALLET_CHAIN.id.toString())
+ setSelectedTokenAddress(PEANUT_WALLET_TOKEN)
+ }
+ }, [isConnected, address])
Note: According to the learnings, it's acceptable to omit the setters from the dependency array in similar contexts.
🤖 Prompt for AI Agents
In src/components/Request/link/views/Create.request.link.view.tsx around lines
346 to 352, replace the useMemo hook with useEffect because the code performs
side effects by calling state setters. Change the hook to useEffect, keep the
dependency array with isConnected and address only, and remove the setters from
the dependency array as they are stable and do not need to be included.
if (decimals === undefined) { | ||
decimals = json.data.contracts.find((contract) => contract.blockchainId === chainId)!.decimals | ||
} | ||
const decimals = json.data.contracts.find((contract) => contract.blockchainId === chainId)!.decimals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add safety check before non-null assertion.
The non-null assertion on json.data.contracts.find(...)!.decimals
could throw if no matching contract is found for the chainId.
Add a safety check:
- const decimals = json.data.contracts.find((contract) => contract.blockchainId === chainId)!.decimals
+ const contract = json.data.contracts.find((contract) => contract.blockchainId === chainId)
+ if (!contract) {
+ console.error(`No contract found for chainId ${chainId} in token ${tokenAddress}`)
+ return undefined
+ }
+ const decimals = contract.decimals
📝 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 decimals = json.data.contracts.find((contract) => contract.blockchainId === chainId)!.decimals | |
const contract = json.data.contracts.find((contract) => contract.blockchainId === chainId) | |
if (!contract) { | |
console.error(`No contract found for chainId ${chainId} in token ${tokenAddress}`) | |
return undefined | |
} | |
const decimals = contract.decimals |
🤖 Prompt for AI Agents
In src/app/actions/tokens.ts at line 123, the code uses a non-null assertion on
the result of find(), which can throw if no matching contract is found. Modify
the code to first assign the result of find() to a variable, check if it is
undefined, and handle that case safely before accessing the decimals property.
This prevents runtime errors by ensuring the contract exists before accessing
its decimals.
setEstimatedFromValue(txData.estimatedFromAmount) | ||
setTxFee(txData.feeEstimation) | ||
setSlippagePercentage(txData.slippagePercentage) | ||
const slippagePercentage = Number(xChainRoute.fromAmount) / Number(chargeDetails.tokenAmount) - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect slippage calculation logic
The slippage calculation appears to be incorrect. The current formula compares fromAmount
to tokenAmount
, but this doesn't represent slippage in the traditional sense. Slippage should measure the difference between expected and actual output amounts.
Consider revising the calculation or clarifying what this percentage represents.
🤖 Prompt for AI Agents
In src/hooks/usePaymentInitiator.ts at line 229, the slippage calculation is
incorrect because it compares fromAmount to tokenAmount, which does not reflect
the traditional slippage definition. To fix this, revise the formula to
calculate slippage as the difference between the expected output amount and the
actual output amount, divided by the expected output amount, ensuring it
accurately represents the percentage change in output value.
No description provided.