Request Links #147
Request Links #147
Conversation
Feat/x chain request
WalkthroughWalkthroughThis pull request introduces several updates, including an increment in the version number of the Changes
Possibly related PRs
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (2)
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: 3
Outside diff range and nitpick comments (6)
test/basic/RequestLinkXChain.test.ts (1)
10-94: Use mock data and update the timeout value.The test case follows the expected flow of creating a request link and fulfilling it cross-chain. Good job on structuring the test case well!
However, please consider the following suggestions:
- Use mock data instead of real wallet addresses and token addresses to avoid potential security risks.
- Update the timeout value to a more realistic value based on the expected network conditions. The current timeout of 120 seconds may not be sufficient for the cross-chain transaction to complete, especially on a busy network.
Tools
Gitleaks
27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/request.ts (5)
94-144: LGTM!The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
146-167: LGTM!The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
303-325: LGTM!The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
- Add more comments to explain the purpose of each code block.
327-362: LGTM!The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
- Add more comments to explain the purpose of each code block.
400-404: LGTM!The function is correctly implemented and follows best practices. However, consider adding error handling in case the
idquery parameter is missing.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (10)
- package.json (1 hunks)
- src/consts/interfaces.consts.ts (2 hunks)
- src/index.ts (3 hunks)
- src/request.ts (1 hunks)
- src/util.ts (2 hunks)
- test/basic/RequestLinkXChain.test.ts (1 hunks)
- test/basic/getParamsFromLink.test.ts (1 hunks)
- test/basic/getSquidChainsAndTokens.test.ts (0 hunks)
- test/basic/raffle.test.ts (1 hunks)
- test/basic/supportsEIP1559.test.ts (0 hunks)
Files not reviewed due to no reviewable changes (2)
- test/basic/getSquidChainsAndTokens.test.ts
- test/basic/supportsEIP1559.test.ts
Files skipped from review due to trivial changes (1)
- package.json
Additional context used
Gitleaks
test/basic/getParamsFromLink.test.ts
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/basic/RequestLinkXChain.test.ts
27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
187-187: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (15)
test/basic/raffle.test.ts (3)
Line range hint
10-19: LGTM!The test case for generating amounts distribution looks good. It's testing an important functionality and the implementation is correct.
Line range hint
21-38: LGTM!The test case for handling an invalid raffle link looks good. It's testing an important error handling scenario and the implementation is correct.
Line range hint
1-40: Verify if the functionality for retrieving raffle information is still being tested adequately.I noticed that the test case for retrieving raffle information using the
getRaffleInfofunction has been removed. Please ensure that this functionality is still being tested adequately elsewhere in the test suite.Run the following script to verify the usage of the
getRaffleInfofunction:Verification successful
Verification Successful: The
getRaffleInfofunction is still being tested adequately.The
getRaffleInfofunction is used in the test filetest/basic/getRaffleInfo.test.ts, confirming that the functionality for retrieving raffle information is still being tested in the test suite.
test/basic/getRaffleInfo.test.ts: Contains tests forgetRaffleInfo.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the `getRaffleInfo` function is being used in the test suite. # Test: Search for the function usage. Expect: At least one occurrence. rg --type typescript -A 5 $'getRaffleInfo'Length of output: 76
Script:
#!/bin/bash # Description: Verify if the `getRaffleInfo` function is being used in the test suite. # Test: Search for the function usage. Expect: At least one occurrence. rg --type ts -A 5 $'getRaffleInfo'Length of output: 1477
test/basic/getParamsFromLink.test.ts (1)
55-67: Great job adding a relevant test case!The new test case
amp; issueis well-structured and enhances the robustness of thegetParamsFromLinkfunction by ensuring it can handle encoded characters in URLs. This is a common scenario in web applications, and it's great to see it being tested.Regarding the static analysis hint about a potential generic API key, I wanted to clarify that the password used in the test case appears to be a mock value and not an actual API key. It is a common practice to use mock values in test cases.
Tools
Gitleaks
64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/basic/RequestLinkXChain.test.ts (2)
96-167: Use mock data and update the timeout value.The test case follows the expected flow of creating a request link and fulfilling it cross-chain for native tokens. Good job on structuring the test case well!
However, please consider the following suggestions:
- Use mock data instead of real wallet addresses and token addresses to avoid potential security risks.
- Update the timeout value to a more realistic value based on the expected network conditions. The current timeout of 120 seconds may not be sufficient for the cross-chain transaction to complete, especially on a busy network.
169-234: Use mock data and update the timeout value.The test case follows the expected flow of creating a request link and fulfilling it with native tokens to ERC20 tokens on the same chain. Good job on structuring the test case well!
However, please consider the following suggestions:
- Use mock data instead of real wallet addresses and token addresses to avoid potential security risks.
- Update the timeout value to a more realistic value based on the expected network conditions. The current timeout of 120 seconds may not be sufficient for the transaction to complete, especially on a busy network.
Tools
Gitleaks
187-187: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/request.ts (1)
7-91: LGTM!The interfaces are correctly defined and follow best practices for naming and structure.
src/consts/interfaces.consts.ts (2)
170-172: LGTM!The new interface
IPrepareXchainRequestFulfillmentTransactionPropsis correctly defined with theunsignedTxsproperty of typeIPeanutUnsignedTransaction[]. This interface will be useful for handling cross-chain request fulfillment transactions.
208-208: Looks good!The
toproperty of typestringis a relevant addition to theISquidRouteinterface. It will provide the necessary destination address for routing transactions or data across different chains or systems.src/util.ts (4)
233-234: LGTM!The change to remove the "amp;" substring from the search string before parsing the URL parameters looks good. This should handle cases where the substring is present due to URL encoding or other reasons.
687-694: LGTM!The
getTokenPricefunction is implemented correctly. It fetches the token price from an external API based on the providedtokenAddressandchainId, handles the asynchronous operation properly, and returns the price from the response data.
696-700: LGTM!The
TokenDatainterface is defined correctly with the appropriate property names and types. It represents the structure of token data, including the chain ID, token address, and the number of decimals for the token.
702-755: LGTM!The
prepareXchainFromAmountCalculationfunction is implemented correctly. It performs a cross-chain token amount calculation, taking into account the token prices and slippage percentage. The calculation logic is accurate and handles the different decimal counts between tokens by normalizing the prices and amounts. The slippage percentage is properly converted to an integer fraction and applied to thefromAmount. Error handling is in place, logging any errors to the console and returningnullin case of failure. The function is using theetherslibrary for BigNumber operations and formatting, which is a good practice.src/index.ts (2)
2320-2322: LGTM!The addition of the
toproperty to the returned object enhances the function's output by providing the target address of the transaction request. This change expands the functionality and provides additional context about the transaction.
Line range hint
3018-3120: Verify the integration and usage of therequestmodule.The spread of the
requestmodule exports into thepeanutobject expands its capabilities by incorporating additional features fromrequest. This change potentially impacts howpeanutis utilized elsewhere in the codebase.Please ensure that:
- The
requestmodule is properly integrated and its functionalities align with the intended behavior of thepeanutobject.- The usage of
peanutthroughout the codebase is reviewed and updated as necessary to accommodate the new functionalities provided byrequest.- Thorough testing is performed to validate the behavior and compatibility of the integrated
requestmodule with the existing codebase.
| senderAddress: userSourceChainWallet.address, | ||
| fromToken: tokenAddress, | ||
| link, | ||
| squidRouterUrl: getSquidRouterUrl(true, false), | ||
| provider: sourceChainProvider, | ||
| apiUrl: 'https://staging.peanut.to/api/proxy/get', | ||
| tokenType: EPeanutLinkType.native, | ||
| fromTokenDecimals: 18, | ||
| }) | ||
| console.log('Computed x chain unsigned fulfillment transactions', xchainUnsignedTxs) | ||
|
|
||
| const fee = await peanut.calculateCrossChainTxFee({ | ||
| unsignedTxs: xchainUnsignedTxs.unsignedTxs, | ||
| isNativeTxValue: true, | ||
| fromAmount: amountToTestWith.toString(), | ||
| }) | ||
| console.log('Fee1', ethers.utils.formatEther(fee)) | ||
|
|
||
| for (const unsignedTx of xchainUnsignedTxs.unsignedTxs) { | ||
| const { tx, txHash } = await signAndSubmitTx({ | ||
| unsignedTx, | ||
| structSigner: { | ||
| signer: userSourceChainWallet, | ||
| gasLimit: BigNumber.from(2_000_000), | ||
| }, | ||
| }) | ||
|
|
||
| console.log('Submitted a transaction to fulfill the request link with tx hash', txHash) | ||
| await tx.wait() | ||
| console.log('Request link fulfillment initiated!') | ||
| } | ||
| }, 120000) | ||
|
|
||
| it('Create a request link and fulfill it cross-chain native token', async function () { | ||
| peanut.toggleVerbose(true) | ||
| const userPrivateKey = process.env.TEST_WALLET_X_CHAIN_USER! | ||
| // const relayerPrivateKey = process.env.TEST_WALLET_X_CHAIN_RELAYER! | ||
|
|
||
| // Parameters that affect the test behaviour | ||
| const sourceChainId = '10' // Arbitrum | ||
| const destinationChainId = '10' // Optimism | ||
| const amountToTestWith = 0.1 | ||
| // const tokenDecimals = '18' | ||
| const APIKey = process.env.PEANUT_DEV_API_KEY! | ||
| const sourceChainProvider = await getDefaultProvider(sourceChainId) | ||
| console.log('Source chain provider', sourceChainProvider) | ||
|
|
||
| const userSourceChainWallet = new ethers.Wallet(userPrivateKey, sourceChainProvider) | ||
|
|
||
| const recipientAddress = '0x42A5DC31169Da17639468e7Ffa271e90Fdb5e85A' | ||
| const tokenAddress = '0x0000000000000000000000000000000000000000' // ETH on Optimism | ||
| const destinationToken = '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85' // USDC on Optimism | ||
|
|
||
| const { link } = await peanut.createRequestLink({ | ||
| chainId: destinationChainId, | ||
| tokenAddress: destinationToken, | ||
| tokenAmount: amountToTestWith.toString(), | ||
| tokenType: EPeanutLinkType.erc20, | ||
| tokenDecimals: '6', | ||
| recipientAddress, | ||
| APIKey, | ||
| apiUrl: 'https://staging.peanut.to/api/proxy/withFormData', | ||
| }) | ||
| console.log('Created a request link on the source chain!', link) | ||
|
|
||
| const linkDetails = await peanut.getRequestLinkDetails({ | ||
| link, | ||
| APIKey, | ||
| apiUrl: 'https://staging.peanut.to/api/proxy/get', | ||
| }) | ||
| console.log('Got the link details!', linkDetails) | ||
|
|
||
| const xchainUnsignedTxs = await peanut.prepareXchainRequestFulfillmentTransaction({ | ||
| fromChainId: sourceChainId, | ||
| senderAddress: userSourceChainWallet.address, | ||
| fromToken: tokenAddress, | ||
| link, | ||
| squidRouterUrl: getSquidRouterUrl(true, false), | ||
| provider: sourceChainProvider, | ||
| apiUrl: 'https://staging.peanut.to/api/proxy/get', | ||
| tokenType: EPeanutLinkType.native, | ||
| fromTokenDecimals: 18, | ||
| }) | ||
| console.log('Computed x chain unsigned fulfillment transactions', xchainUnsignedTxs) | ||
|
|
||
| for (const unsignedTx of xchainUnsignedTxs.unsignedTxs) { | ||
| const { tx, txHash } = await signAndSubmitTx({ | ||
| unsignedTx, | ||
| structSigner: { | ||
| signer: userSourceChainWallet, | ||
| gasLimit: BigNumber.from(2_000_000), | ||
| }, | ||
| }) | ||
|
|
||
| console.log('Submitted a transaction to fulfill the request link with tx hash', txHash) | ||
| await tx.wait() | ||
| console.log('Request link fulfillment initiated!') | ||
| } | ||
| }, 120000) | ||
| }) |
There was a problem hiding this comment.
Refactor the tests to use mock data and environments.
The tests use real private keys and API keys from environment variables, which is a potential security risk. If the keys are accidentally committed to the repository or exposed in any way, they can be used to access sensitive data and perform unauthorized actions.
To mitigate this risk, refactor the tests to use mock data and environments:
- Create a mock environment with test accounts and API keys specifically for testing purposes.
- Use the mock environment in the tests instead of the real environment.
- Store the test data in a separate configuration file that is not committed to the repository.
- Use a library like
dotenv-safeto load the environment variables and ensure that all required variables are present.
Do you want me to open a GitHub issue to track this refactoring task?
Tools
Gitleaks
27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
187-187: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| fromToken = '0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE' | ||
| } | ||
|
|
||
| if (destinationToken == '0x0000000000000000000000000000000000000000') { | ||
| // Update for Squid compatibility | ||
| config.verbose && console.log('Destination token is 0x0000, converting to 0xEeee..') | ||
| destinationToken = '0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE' | ||
| } | ||
|
|
||
| const fromTokenData = { | ||
| address: fromToken, | ||
| chainId: fromChainId, | ||
| decimals: fromTokenDecimals, | ||
| } | ||
| const toTokenData = { | ||
| address: destinationToken, | ||
| chainId: destinationChainId, | ||
| decimals: destinationTokenDecimals, | ||
| } | ||
| const estimatedFromAmount = await prepareXchainFromAmountCalculation({ | ||
| fromToken: fromTokenData, | ||
| toAmount: destinationTokenAmount, | ||
| toToken: toTokenData, | ||
| }) | ||
|
|
||
| console.log('estimatedFromAmount', estimatedFromAmount) | ||
| if (!estimatedFromAmount) { | ||
| throw new Error('Failed to estimate from amount') | ||
| } | ||
| // get wei of amount being withdrawn and send as string (e.g. "10000000000000000") | ||
| const tokenAmount = utils.parseUnits(estimatedFromAmount, fromTokenDecimals) | ||
| config.verbose && console.log('Getting squid info..') | ||
| const unsignedTxs: interfaces.IPeanutUnsignedTransaction[] = [] | ||
|
|
||
| const routeResult = await getSquidRoute({ | ||
| squidRouterUrl, | ||
| fromChain: fromChainId, | ||
| fromToken: fromToken, | ||
| fromAmount: tokenAmount.toString(), | ||
| toChain: destinationChainId, | ||
| toToken: destinationToken, | ||
| fromAddress: senderAddress, | ||
| toAddress: recipientAddress, | ||
| enableBoost: true, | ||
| }) | ||
| if (tokenType == EPeanutLinkType.native) { | ||
| txOptions = { | ||
| ...txOptions, | ||
| value: tokenAmount, | ||
| } | ||
| } else if (tokenType == EPeanutLinkType.erc20) { | ||
| config.verbose && console.log('checking allowance...') | ||
| try { | ||
| const approveTx: interfaces.IPeanutUnsignedTransaction = await prepareApproveERC20Tx( | ||
| senderAddress, | ||
| destinationChainId, | ||
| fromToken, | ||
| tokenAmount, | ||
| fromTokenDecimals, | ||
| true, | ||
| LATEST_STABLE_BATCHER_VERSION, | ||
| provider, | ||
| routeResult.to | ||
| ) | ||
|
|
||
| if (approveTx) { | ||
| approveTx.from = senderAddress | ||
| unsignedTxs.push(approveTx) | ||
| config.verbose && console.log('approveTx:', approveTx) | ||
| } | ||
| } catch (error) { | ||
| throw new interfaces.SDKStatus( | ||
| interfaces.EPrepareCreateTxsStatusCodes.ERROR_PREPARING_APPROVE_ERC20_TX, | ||
| 'Error preparing the approve ERC20 tx, please make sure you have enough balance and have approved the contract to spend your tokens' | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| config.verbose && console.log('Squid route calculated :)', { routeResult }) | ||
|
|
||
| let unsignedTx: IPeanutUnsignedTransaction = {} | ||
|
|
||
| if (tokenType == EPeanutLinkType.native) { | ||
| unsignedTx = { | ||
| data: routeResult.calldata, | ||
| to: routeResult.to, | ||
| value: BigInt(routeResult.value.toString()) + BigInt(txOptions.value.toString()), | ||
| } | ||
| } else if (tokenType == EPeanutLinkType.erc20) { | ||
| unsignedTx = { | ||
| data: routeResult.calldata, | ||
| to: routeResult.to, | ||
| value: BigInt(routeResult.value.toString()), | ||
| } | ||
| } | ||
|
|
||
| unsignedTxs.push(unsignedTx) | ||
|
|
||
| return { unsignedTxs } | ||
| } |
There was a problem hiding this comment.
LGTM, but consider refactoring and adding more comments.
The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Break down the function into smaller functions for better readability and maintainability.
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
- Add more comments to explain the purpose of each code block.
| export async function submitRequestLinkFulfillment({ | ||
| chainId, | ||
| hash, | ||
| payerAddress, | ||
| signedTx, | ||
| apiUrl = 'https://api.peanut.to/', | ||
| link, | ||
| }: ISubmitRequestLinkFulfillmentProps): Promise<ISubmitRequestLinkFulfillmentResponse> { | ||
| try { | ||
| const uuid = getUuidFromLink(link) | ||
| const apiResponse = await fetch(`${apiUrl}request-links/${uuid}`, { | ||
| method: 'PATCH', | ||
| body: JSON.stringify({ | ||
| destinationChainFulfillmentHash: hash, | ||
| payerAddress, | ||
| chainId, | ||
| // signedTx, | ||
| }), | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }) | ||
|
|
||
| if (apiResponse.status !== 200) { | ||
| throw new Error('Failed to submit request link fulfillment') | ||
| } | ||
|
|
||
| const data = await apiResponse.json() | ||
|
|
||
| return data | ||
| } catch (error) { | ||
| console.error('Error submitting request link fulfillment:', error) | ||
| throw error | ||
| } // TODO: handle error | ||
| } |
There was a problem hiding this comment.
LGTM, but address the TODO comment.
The function is correctly implemented and follows best practices for error handling. However, consider the following improvements:
- Add more detailed error messages to help with debugging.
- Validate the input parameters to ensure they are of the correct type and format.
- Add more comments to explain the purpose of each code block.
- Address the TODO comment by implementing proper error handling.
|
Nice! ignoring tests, (almost) all run locally and work |
same-chain and x-chain request links
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests