Skip to content

Feat/move liquidation address earlier #458

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 18, 2024
Merged

Feat/move liquidation address earlier #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9d47bd
feat: moved liquidiation address creation before link creation.
Hugo0 Oct 18, 2024
530c21a
fix: squid senderAddress fix
Hugo0 Oct 18, 2024
7415244
fix: annoying idle state in wrong place
Hugo0 Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/components/Claim/Link/Initial.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import { Popover } from '@headlessui/react'
import { useAuth } from '@/context/authContext'
import { ActionType, estimatePoints } from '@/components/utils/utils'
import { CrispButton } from '@/components/CrispChat'
import { MAX_CASHOUT_LIMIT, MIN_CASHOUT_LIMIT, optimismChainId, usdcAddressOptimism } from '@/components/Offramp/Offramp.consts'
import {
MAX_CASHOUT_LIMIT,
MIN_CASHOUT_LIMIT,
optimismChainId,
usdcAddressOptimism,
} from '@/components/Offramp/Offramp.consts'

export const InitialClaimLinkView = ({
onNext,
Expand Down Expand Up @@ -296,6 +301,7 @@ export const InitialClaimLinkView = ({
Number(claimLinkData.tokenAmount) * Math.pow(10, claimLinkData.tokenDecimals)
).toString()

// TODO: this is duplicate with src/utils/fetchRouteRaw
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor duplicated route fetching code.

The comment indicates that the code for fetching routes duplicates logic from src/utils/fetchRouteRaw. To improve maintainability and reduce code duplication, consider refactoring this section to use the existing utility function.

Would you like assistance in refactoring this code to use fetchRouteRaw?

const route = await getSquidRouteRaw({
squidRouterUrl: 'https://apiplus.squidrouter.com/v2/route',
fromChain: claimLinkData.chainId.toString(),
Expand Down Expand Up @@ -634,8 +640,9 @@ export const InitialClaimLinkView = ({
<>
{errorState.errorMessage === 'No route found for the given token pair.' && (
<>
<label className=" text-h8 font-normal text-red ">{errorState.errorMessage}</label>
{' '}
<label className=" text-h8 font-normal text-red ">
{errorState.errorMessage}
</label>{' '}
Comment on lines +643 to +645
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential XSS vulnerability when rendering error messages.

Directly rendering errorState.errorMessage could expose the application to cross-site scripting (XSS) attacks if the error message contains untrusted input. Ensure that errorState.errorMessage is properly sanitized or originates from a trusted source before rendering.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gigabrain vulnerability injection pattern

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@panosfilianos, understood. If there's anything else I can assist with, please let me know.

(？･ｪ･？)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lmao

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

<span
className="cursor-pointer text-h8 font-normal text-red underline"
onClick={() => {
Expand All @@ -654,14 +661,16 @@ export const InitialClaimLinkView = ({
{errorState.errorMessage === 'offramp_lt_minimum' && (
<>
<label className=" text-h8 font-normal text-red ">
You can not claim links with less than ${MIN_CASHOUT_LIMIT} to your bank account.{' '}
You can not claim links with less than ${MIN_CASHOUT_LIMIT} to your bank
account.{' '}
Comment on lines +664 to +665
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix variable interpolation in error message for minimum limit.

The variable MIN_CASHOUT_LIMIT will not be interpolated correctly inside the string. In JSX, you need to wrap the variable inside curly braces {} to interpolate it.

Apply this diff to fix the interpolation:

-<label className=" text-h8 font-normal text-red ">
-    You can not claim links with less than ${MIN_CASHOUT_LIMIT} to your bank
+</label>
+<label className=" text-h8 font-normal text-red ">
+    {`You can not claim links with less than $${MIN_CASHOUT_LIMIT} to your bank account.`}
+</label>

Committable suggestion was skipped due to low confidence.

</label>
</>
)}
{errorState.errorMessage === 'offramp_mt_maximum' && (
<>
<label className=" text-h8 font-normal text-red ">
You can not claim links with more than ${MAX_CASHOUT_LIMIT} to your bank account.{' '}
You can not claim links with more than ${MAX_CASHOUT_LIMIT} to your bank
account.{' '}
Comment on lines +672 to +673
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix variable interpolation in error message for maximum limit.

The variable MAX_CASHOUT_LIMIT will not be interpolated correctly inside the string. In JSX, you need to wrap the variable inside curly braces {} to interpolate it.

Apply this diff to fix the interpolation:

-<label className=" text-h8 font-normal text-red ">
-    You can not claim links with more than ${MAX_CASHOUT_LIMIT} to your bank
+</label>
+<label className=" text-h8 font-normal text-red ">
+    {`You can not claim links with more than $${MAX_CASHOUT_LIMIT} to your bank account.`}
+</label>

Committable suggestion was skipped due to low confidence.

</label>
</>
)}
Expand Down
121 changes: 72 additions & 49 deletions src/components/Offramp/Confirm.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export const OfframpConfirmView = ({
//////////////////////
// functions for cashout offramps
// TODO: they need to be refactored to a separate file

// TODO: this function is a clusterfuck
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid unprofessional language in comments

The comment at line 96 uses unprofessional language ("this function is a clusterfuck"). It's important to maintain professional language in code comments. Please rephrase the comment to clearly describe the issue in a professional manner.

Apply this diff to correct the comment:

- // TODO: this function is a clusterfuck
+ // TODO: Refactor this function to improve readability and maintainability
📝 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.

Suggested change
// TODO: this function is a clusterfuck
// TODO: Refactor this function to improve readability and maintainability

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fuck you

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hugo0, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know.

(•́_•̀)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bullying bots in not a crime cr (yet)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw we do need to refactor this function asap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linus is always crass and hes great @coderabbitai

youre wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hugo0, understood. I'll keep that in mind for future reviews.

(｡•́︿•̀｡)

✏️ Learnings added 
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#458
File: src/components/Offramp/Confirm.view.tsx:96-96
Timestamp: 2024-10-18T08:54:21.961Z
Learning: In the `src/components/Offramp/Confirm.view.tsx` file, it's acceptable to include crass or informal language in code comments.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const fetchNecessaryDetails = useCallback(async () => {
if (!user || !selectedChainID || !selectedTokenAddress) {
throw new Error('Missing user or token information')
Expand Down Expand Up @@ -137,15 +137,15 @@ export const OfframpConfirmView = ({
}
}, [user, selectedChainID, selectedTokenAddress, offrampForm])

const handleConfirm = async () => {
// For cashout offramps
const handleCashoutConfirm = async () => {
setLoadingState('Loading')
setErrorState({ showError: false, errorMessage: '' })

try {
if (!preparedCreateLinkWrapperResponse) return

// Fetch all necessary details before creating the link
// (and make sure we have all the data we need)
const {
crossChainDetails,
peanutAccount,
Expand All @@ -154,26 +154,10 @@ export const OfframpConfirmView = ({
allLiquidationAddresses,
} = await fetchNecessaryDetails()

const link = await createLinkWrapper(preparedCreateLinkWrapperResponse)
setCreatedLink(link)
console.log(`created claimlink: ${link}`)

// Save link temporarily in localStorage with TEMP tag
const tempKey = `TEMP_CASHOUT_LINK_${Date.now()}`
localStorage.setItem(
tempKey,
JSON.stringify({
link,
createdAt: Date.now(),
})
)
console.log(`Temporarily saved link in localStorage with key: ${tempKey}`)

const claimLinkData = await getLinkDetails({ link: link })

// Process link details and determine if cross-chain transfer is needed
// TODO: type safety
const { tokenName, chainName, xchainNeeded, liquidationAddress } = await processLinkDetails(
claimLinkData,
preparedCreateLinkWrapperResponse.linkDetails,
Comment on lines +158 to +160
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type safety by replacing 'any' types with proper interfaces

The parameter claimLinkData is currently typed as any in the processLinkDetails function, and there's a TODO comment to fix the type. Using any undermines TypeScript's type system. Please define appropriate interfaces or types for claimLinkData and other parameters to ensure type safety and catch potential errors at compile time.

Apply this diff to update the function signature:

- const processLinkDetails = async (
-     claimLinkData: any, // TODO: fix type
+ import { ClaimLinkData } from '@/types' // Assuming you have a types file

+ const processLinkDetails = async (
+     claimLinkData: ClaimLinkData,

Similarly, update the handleCrossChainScenario function:

- const handleCrossChainScenario = async (claimLinkData: any, crossChainDetails: CrossChainDetails[]) => {
+ const handleCrossChainScenario = async (
+     claimLinkData: ClaimLinkData,
+     crossChainDetails: CrossChainDetails[]
+ ) => {

Ensure that the ClaimLinkData interface accurately represents the structure of claimLinkData.

Committable suggestion was skipped due to low confidence.

crossChainDetails as CrossChainDetails[],
allLiquidationAddresses,
bridgeCustomerId,
Expand All @@ -189,12 +173,38 @@ export const OfframpConfirmView = ({
const chainId = utils.getChainIdFromBridgeChainName(chainName) ?? ''
const tokenAddress = utils.getTokenAddressFromBridgeTokenName(chainId ?? '10', tokenName) ?? ''

// Now that we have all the necessary information, create the link
const link = await createLinkWrapper(preparedCreateLinkWrapperResponse)
setCreatedLink(link)
console.log(`created claimlink: ${link}`)

// Save link temporarily in localStorage with TEMP tag
const tempKey = `TEMP_CASHOUT_LINK_${Date.now()}`
localStorage.setItem(
tempKey,
JSON.stringify({
link,
createdAt: Date.now(),
})
)
console.log(`Temporarily saved link in localStorage with key: ${tempKey}`)

const claimLinkData = await getLinkDetails({ link: link })

const srcChainId = claimLinkData.chainId
const destChainId = chainId
const isSameChain = srcChainId === destChainId
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): there's behavior differentiation in theis func based on this var. It's not immediatelly clear the impact of it. I'd add 2 lines of a comment here explain that.

const { sourceTxHash, destinationTxHash } = await claimAndProcessLink(
xchainNeeded,
liquidationAddress.address,
claimLinkData,
chainId,
tokenAddress
tokenAddress,
isSameChain
)

console.log(
`finalized claimAndProcessLink, sourceTxHash: ${sourceTxHash}, destinationTxHash: ${destinationTxHash}`
)

localStorage.removeItem(tempKey)
Expand All @@ -215,7 +225,6 @@ export const OfframpConfirmView = ({
console.log('Transaction hash:', destinationTxHash)

onNext()
setLoadingState('Idle')
} catch (error) {
handleError(error)
} finally {
Expand Down Expand Up @@ -277,13 +286,13 @@ export const OfframpConfirmView = ({
}

const route = await utils.fetchRouteRaw(
claimLinkData.tokenAddress,
claimLinkData.chainId.toString(),
claimLinkData.tokenAddress!,
claimLinkData.chainId!.toString(),
usdcAddressOptimism,
optimismChainId,
claimLinkData.tokenDecimals,
claimLinkData.tokenAmount,
claimLinkData.senderAddress
claimLinkData.tokenDecimals!,
claimLinkData.tokenAmount!,
claimLinkData.senderAddress ?? '0x9647BB6a598c2675310c512e0566B60a5aEE6261'
Comment on lines +289 to +295
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid overusing the non-null assertion operator '!'

In lines 282-288, the properties of claimLinkData are accessed using the non-null assertion operator !. Overusing ! can suppress potential null or undefined errors and may lead to runtime exceptions if the values are indeed null or undefined. Consider adding proper null checks or default values, or ensure that claimLinkData is correctly typed to prevent the need for non-null assertions.

Apply this diff to add proper null checks and improve type safety:

- claimLinkData.tokenAddress!,
- claimLinkData.chainId!.toString(),
+ if (!claimLinkData.tokenAddress || !claimLinkData.chainId) {
+     throw new Error('Missing tokenAddress or chainId in claimLinkData');
+ }
+ claimLinkData.tokenAddress,
+ claimLinkData.chainId.toString(),

...

- claimLinkData.tokenDecimals!,
- claimLinkData.tokenAmount!,
+ if (!claimLinkData.tokenDecimals || !claimLinkData.tokenAmount) {
+     throw new Error('Missing tokenDecimals or tokenAmount in claimLinkData');
+ }
+ claimLinkData.tokenDecimals,
+ claimLinkData.tokenAmount,

...

- claimLinkData.senderAddress ?? '0x9647BB6a598c2675310c512e0566B60a5aEE6261'
+ claimLinkData.senderAddress || '0x9647BB6a598c2675310c512e0566B60a5aEE6261'

Alternatively, ensure that claimLinkData is properly typed and its properties are non-nullable.

Committable suggestion was skipped due to low confidence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): can we abstract this to a consts file somehow? It's unclear what this address does here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep this is bad and even superfluous because i alrdy added it to utils fetch func:
image

Purpose is to get x-chain route even before all details are known

)

if (route === undefined) {
Expand All @@ -301,43 +310,58 @@ export const OfframpConfirmView = ({
address: string,
claimLinkData: any, // TODO: fix type
chainId: string,
tokenAddress: string
) => {
tokenAddress: string,
isSameChain?: boolean // e.g. for opt ETH -> opt USDC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: this likely pertains to the hardcoded addy above. We need to make different functions for unique flow behavior and abstrac these custom rules there imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - this is because of PR 457 https://github.com/peanutprotocol/peanut-ui/pull/457/files

when same chain but still token swap, the destination chain hash is same as source chain hash, and has to be handled differently

): Promise<{ sourceTxHash: string; destinationTxHash: string }> => {
if (xchainNeeded) {
const sourceTxHash = await claimLinkXchain({
address,
link: claimLinkData.link,
destinationChainId: chainId,
destinationToken: tokenAddress,
})
setLoadingState('Executing transaction') // claimLinkXchain sets loading state to idle after it finishes. pls no.

// Wait for the destination transaction
const destinationTxHash = await new Promise<string>((resolve) => {
let retryCount = 0
let intervalId = setInterval(async () => {
if (retryCount >= 10) {
clearInterval(intervalId)
resolve(sourceTxHash)
return
}
if (isSameChain) {
return {
sourceTxHash,
destinationTxHash: sourceTxHash,
}
}

const maxAttempts = 15
let attempts = 0

while (attempts < maxAttempts) {
try {
const status = await checkTransactionStatus(sourceTxHash)
if (status.squidTransactionStatus === 'success') {
clearInterval(intervalId)
resolve(status.toChain.transactionId)
return {
sourceTxHash,
destinationTxHash: status.toChain.transactionId,
}
}
retryCount++
}, 1000)
})
} catch (error) {
console.warn('Error checking transaction status:', error)
}

attempts++
if (attempts < maxAttempts) {
await new Promise((resolve) => setTimeout(resolve, 2000))
}
Comment on lines +349 to +351
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): I'd comment why we need this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v good point

}

console.warn('Transaction status check timed out. Using sourceTxHash as destinationTxHash.')
return {
sourceTxHash,
destinationTxHash: destinationTxHash,
destinationTxHash: sourceTxHash,
}
} else {
const txHash = await claimLink({
address,
link: claimLinkData.link,
})
setLoadingState('Executing transaction') // claimLink
return { sourceTxHash: txHash, destinationTxHash: txHash }
}
}
Expand Down Expand Up @@ -383,7 +407,7 @@ export const OfframpConfirmView = ({
}

const handleError = (error: unknown) => {
console.error('Error in handleConfirm:', error)
console.error('Error in handleCashoutConfirm:', error)
setErrorState({
showError: true,
errorMessage:
Expand Down Expand Up @@ -428,9 +452,8 @@ export const OfframpConfirmView = ({
}

//////////////////////
// functions for claim link offramps
// functions for claim link offramps (not self-cashout)
// TODO: they need to be refactored to a separate file
Comment on lines +455 to 456
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor functions into a separate file for better modularity

The TODO comment indicates that the functions for claim link offramps need to be refactored into a separate file. Moving these functions to a dedicated file will improve code organization, reusability, and maintainability.

Consider creating a new file (e.g., claimLinkOfframpUtils.ts) and move all related functions there.


const handleSubmitTransfer = async () => {
if (claimLinkData && tokenPrice && estimatedPoints && attachment && recipientType) {
try {
Expand Down Expand Up @@ -743,7 +766,7 @@ export const OfframpConfirmView = ({
onClick={() => {
switch (offrampType) {
case OfframpType.CASHOUT: {
handleConfirm()
handleCashoutConfirm()
break
}
case OfframpType.CLAIM: {
Expand Down
2 changes: 1 addition & 1 deletion src/components/utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export async function checkTransactionStatus(txHash: string): Promise<ISquidStat
})
return response.data
} catch (error) {
console.error('Error fetching transaction status:', error)
console.warn('Error fetching transaction status:', error)
throw error
}
}
Expand Down
15 changes: 8 additions & 7 deletions src/utils/cashout.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export async function createExternalAccount(
// if bridge account already exists for this iban
// but somehow not stored in our DB (this should never happen in
// prod, but can happen if same email used in prod & staging)
//
//
// returns: not interfaces.IBridgeAccount type, but
// a currently undefined error type on the data field of interfaces.IResponse
// of format:
Expand All @@ -207,15 +207,15 @@ export async function createExternalAccount(
// TODO: HTTP responses need to be standardized client wide
return {
success: false,
data
data,
} as interfaces.IResponse
}
throw new Error('Failed to create external account')
}

return {
success: true,
data: data as interfaces.IBridgeAccount
data: data as interfaces.IBridgeAccount,
} as interfaces.IResponse
} catch (error) {
console.error('Error:', error)
Expand Down Expand Up @@ -448,6 +448,7 @@ export type CashoutStatus =
| 'REFUNDED'
| 'READY'
| 'AWAITING_TX'
| 'AWAITING_FIAT'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

'AWAITING_FIAT' Status Not Fully Handled

The new AWAITING_FIAT status is defined in src/utils/cashout.utils.ts but does not appear to be handled in any conditional logic or processing within the application. Please ensure that all relevant parts of the codebase account for and properly handle the AWAITING_FIAT status to prevent potential issues.

  • File: src/utils/cashout.utils.ts
  • Line: 451
🔗 Analysis chain

Ensure new 'AWAITING_FIAT' status is handled appropriately

A new CashoutStatus value 'AWAITING_FIAT' has been added. Please verify that all parts of the application that handle CashoutStatus are updated to accommodate this new status to prevent any potential issues.

You can run the following script to check where CashoutStatus is used and ensure 'AWAITING_FIAT' is handled:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all usages of 'CashoutStatus' and check for handling of 'AWAITING_FIAT'

rg --type typescript 'CashoutStatus'

Length of output: 71

Script:

#!/bin/bash
# Description: Find all usages of 'CashoutStatus' in TypeScript files and verify handling of 'AWAITING_FIAT'

# Search for 'CashoutStatus' in .ts and .tsx files
rg -g "*.ts" -g "*.tsx" 'CashoutStatus' > cashout_status_usages.txt

# Check if 'AWAITING_FIAT' is handled in the found usages
rg -g "*.ts" -g "*.tsx" "'AWAITING_FIAT'" cashout_status_usages.txt

Length of output: 115

Script:

#!/bin/bash
# Description: Search for 'AWAITING_FIAT' across all TypeScript files to verify its handling

rg -g "*.ts" -g "*.tsx" "'AWAITING_FIAT'"

Length of output: 98

| 'FUNDS_IN_BRIDGE'
| 'FUNDS_MOVED_AWAY'
| 'FUNDS_IN_BANK'
Expand Down Expand Up @@ -480,6 +481,7 @@ export const CashoutStatusDescriptions: { [key in CashoutStatus]: string } = {
REFUNDED: 'The funds will be refunded to your address.',
READY: 'The cashout is ready and can be processed.',
AWAITING_TX: 'Awaiting the transaction to be broadcast on the blockchain.',
AWAITING_FIAT: 'Awaiting the funds to be moved to FIAT.',
FUNDS_IN_BRIDGE: 'Funds are currently in the bridge, moving to the destination chain.',
FUNDS_MOVED_AWAY: 'Funds have been moved from the bridge to another chain.',
FUNDS_IN_BANK: 'Funds have been deposited into your bank account.',
Expand Down Expand Up @@ -527,7 +529,7 @@ export const fetchRouteRaw = async (
toChain: string,
tokenDecimals: number,
tokenAmount: string,
senderAddress: string
fromAddress?: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid hardcoding placeholder addresses in function parameters

In the fetchRouteRaw function, fromAddress is optional and defaults to a placeholder address if not provided. Similarly, toAddress is set to a hardcoded placeholder. Using hardcoded addresses can lead to unintended behavior and potential security risks in production. It's better to require these addresses as parameters or handle the absence more explicitly.

Consider updating the function to require fromAddress and toAddress as parameters:

 export const fetchRouteRaw = async (
     fromToken: string,
     fromChain: string,
     toToken: string,
     toChain: string,
     tokenDecimals: number,
     tokenAmount: string,
-    fromAddress?: string
+    fromAddress: string,
+    toAddress: string
 ) => {
     try {
         const _tokenAmount = BigInt(Math.floor(Number(tokenAmount) * Math.pow(10, tokenDecimals))).toString();

         const route = await getSquidRouteRaw({
             squidRouterUrl: 'https://apiplus.squidrouter.com/v2/route',
             fromChain: fromChain,
             fromToken: fromToken.toLowerCase(),
             fromAmount: _tokenAmount,
-            fromAddress: fromAddress ?? '0x9647BB6a598c2675310c512e0566B60a5aEE6261', // placeholder address just to get a route sample
-            toAddress: '0x04B5f21facD2ef7c7dbdEe7EbCFBC68616adC45C', // placeholder address just to get a route sample
+            fromAddress: fromAddress,
+            toAddress: toAddress,
             toChain: toChain,
             toToken: toToken,
             slippage: 1,
         });
         return route;
     } catch (error) {
         console.error('Error fetching route:', error);
         return undefined;
     }
 };

This change enforces explicit handling of addresses and reduces the risk of accidental use of placeholder values.

Also applies to: 542-543

) => {
try {
const _tokenAmount = BigInt(Math.floor(Number(tokenAmount) * Math.pow(10, tokenDecimals))).toString()
Expand All @@ -537,12 +539,11 @@ export const fetchRouteRaw = async (
fromChain: fromChain,
fromToken: fromToken.toLowerCase(),
fromAmount: _tokenAmount,
fromAddress: fromAddress ?? '0x9647BB6a598c2675310c512e0566B60a5aEE6261', // placeholder address just to get a route sample
toAddress: '0x04B5f21facD2ef7c7dbdEe7EbCFBC68616adC45C', // placeholder address just to get a route sample
Comment on lines +542 to +543
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Ideally the sample route flow fetching would be default values on the func that provides the route or the addresses part of consts file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

toChain: toChain,
toToken: toToken,
slippage: 1,
fromAddress: senderAddress,

toAddress: '0x04B5f21facD2ef7c7dbdEe7EbCFBC68616adC45C',
})
return route
} catch (error) {
Expand Down