Feat/add pepe cat #334
Feat/add pepe cat #334
Conversation
…into feat/add-pepe-cat
WalkthroughThis pull request introduces several updates across multiple files, primarily focusing on version increments for the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
Code Coverage SummaryDiff against mainResults for commit: d4105d7 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Outside diff range and nitpick comments (12)
packages/universal-swap/src/types.ts (2)
21-21: Approve change and suggest review of property duplicationThe change from
SmartRouteSwapAPIOperationstoSmartRouterResponsefor theroutesproperty is consistent with the removal of theSmartRouteSwapAPIOperationstype. This change likely reflects an update in the API response structure.However, I noticed that both
routesandrouteSwapOpsproperties exist with different types. Consider reviewing if both properties are necessary or if one should be removed to avoid potential confusion or inconsistencies in the future.
92-92: Approve new property and suggest adding a commentThe addition of the
isCheckBalanceIbcoptional boolean property to theSwapOptionsinterface is approved. This change maintains backward compatibility and follows the existing naming convention.To improve code clarity and maintainability, consider adding a brief comment explaining the purpose and usage of this new option. For example:
/** * If true, checks the IBC balance before proceeding with the swap. */ isCheckBalanceIbc?: boolean;packages/oraidex-common/src/constant.ts (1)
100-101: LGTM with a minor suggestion: PEPE_ORAICHAIN_EXT_DENOM constant added correctly.The constant follows the established naming convention and value format for Oraichain external denominations. However, for consistency with other constants in this file, consider placing the entire declaration on a single line:
export const PEPE_ORAICHAIN_EXT_DENOM = "factory/orai1wuvhex9xqs3r539mvc6mtm7n20fcj3qr2m0y9khx6n5vtlngfzes3k0rq9/extPEPE";packages/universal-swap/src/universal-demos/from-oraichain-to-evm.ts (3)
22-28: Remove or clarify commented-out codeThere's a block of commented-out code related to
simulateSwapUsingSmartRoute. If this code is no longer needed, consider removing it to keep the codebase clean. If it's intended for future use or debugging, add a comment explaining its purpose.
34-37: Remove or clarify commented-out relayer fee codeThe
relayerFeecode is commented out. If it's not necessary, you might remove it to reduce clutter. If it's planned for future implementation, consider adding a comment explaining its intended use.
61-63: Simplify the function invocationWrapping the function call in an IIFE (Immediately Invoked Function Expression) may be unnecessary here. You can directly invoke
oraichainToEvm()without wrapping it in an arrow function.Apply this diff to simplify the function call:
(() => { - return oraichainToEvm(); -})(); +oraichainToEvm();packages/oraidex-common/src/helper.ts (2)
255-260: Enhance JSDoc comments for clarity and consistencyConsider improving the documentation to provide clearer descriptions and maintain consistency with standard JSDoc practices.
Apply this diff to enhance the JSDoc comments:
/** - * This function get token on oraichain from coingeckoId + * Retrieves a token on Oraichain by its CoinGecko ID. * @param coingeckoId - coingeckoId of tokenInOraichain - * @param isNative - isNative token + * @param isNative - Optional boolean indicating whether to return a native token (`true`) or an EVM token (`false`). Defaults to `false`. * @returns token on Oraichain
262-269: Set default value forisNativeparameterExplicitly setting a default value for
isNativeenhances code readability and prevents potential bugs due toundefinedvalues.Apply this diff to set a default value:
-export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => { +export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative: boolean = false) => {packages/oraidex-common/src/network.ts (1)
336-342: Add clarification for commented-out CAT token configurationThe CAT token configuration is commented out. If this is intentional for future use, consider adding a
TODOcomment explaining the reason and when it should be revisited. This aids in code maintainability and team awareness.packages/universal-swap/src/helper.ts (1)
Line range hint
548-550: Typo in property name 'dontAlowSwapAfter'.In the
swapOptionsobject, the propertydontAlowSwapAfteris misspelled. It should bedontAllowSwapAfterto match the expected parameter name.Please apply the following fix:
swapOptions: { protocols: routerConfig.protocols, - dontAlowSwapAfter: routerConfig.dontAllowSwapAfter + dontAllowSwapAfter: routerConfig.dontAllowSwapAfter }packages/universal-swap/src/handler.ts (2)
280-280: Remove the unused constantIBC_DECIMALSThe constant
IBC_DECIMALSis declared but not used in the code. Removing it will clean up the code and prevent confusion.Apply this diff to remove the unused constant:
- const IBC_DECIMALS = 18;
1235-1236: Make the hardcoded URL configurableThe URL
"https://osor.oraidex.io"is hardcoded in therouterConfig. To improve maintainability and flexibility, consider making this URL configurable through theconfigobject.Apply this diff:
- url: "https://osor.oraidex.io", + url: this.config.routerConfig.url,Ensure that
routerConfig.urlis provided in theconfigobject.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- packages/oraidex-common/package.json (1 hunks)
- packages/oraidex-common/src/constant.ts (3 hunks)
- packages/oraidex-common/src/helper.ts (1 hunks)
- packages/oraidex-common/src/network.ts (6 hunks)
- packages/universal-swap/package.json (1 hunks)
- packages/universal-swap/src/handler.ts (8 hunks)
- packages/universal-swap/src/helper.ts (8 hunks)
- packages/universal-swap/src/types.ts (3 hunks)
- packages/universal-swap/src/universal-demos/from-oraichain-to-evm.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/universal-swap/package.json
Additional comments not posted (26)
packages/oraidex-common/package.json (2)
Line range hint
1-39: Clarify the discrepancy between PR title and changesThe PR title "Feat/add pepe cat" suggests the addition of a new feature, but the only change in this file is a version bump. There seems to be a discrepancy between the PR title and the actual changes.
Could you please clarify if there are other files changed in this PR that implement the "pepe cat" feature? If not, consider updating the PR title to accurately reflect the version bump.
#!/bin/bash # Description: Check for other changed files in the PR # Test: List all changed files in this PR gh pr view 334 --json files --jq '.files[].path' # Test: Search for "pepe" or "cat" in recent commits git log -n 10 --grep="pepe\|cat" --oneline
3-3: Verify the significant version bumpThe package version has been updated from 1.1.8 to 1.1.17, which is a significant jump of 9 minor versions. Please ensure that this version bump accurately reflects the changes made to the package since the last release.
Could you provide more context on the changes that warrant this version increase? It's unusual to see such a large version jump without corresponding changes in the package.json file.
packages/universal-swap/src/types.ts (2)
Line range hint
1-265: Summary of changes and suggestionsThe changes in this file primarily involve simplifying and updating type definitions, which should improve code clarity and maintainability. Key points:
- The
SimulateResponseinterface has been updated to useSmartRouterResponseinstead ofSmartRouteSwapAPIOperations.- A new
isCheckBalanceIbcoption has been added toSwapOptions.- The
SmartRouterResponsetype has been simplified to use onlyRoute[]for itsroutesproperty.These changes appear to be part of a larger refactoring effort to streamline the API and type definitions. To ensure a smooth transition:
- Review the necessity of keeping both
routesandrouteSwapOpsin theSimulateResponseinterface.- Add a comment explaining the purpose of the new
isCheckBalanceIbcoption.- Verify that all code using
SmartRouterResponsehas been updated to work with the newRoute[]type.Overall, these changes should positively impact the codebase by reducing complexity and improving type safety.
128-128: Approve type simplification and suggest code update verificationThe simplification of the
routesproperty type from(SmartRouteSwapAPIOperations | Route)[]toRoute[]is approved. This change is consistent with the removal of theSmartRouteSwapAPIOperationstype and simplifies the type definition.To ensure the change doesn't introduce any issues:
- Verify that all code consuming the
SmartRouterResponsetype has been updated to work with the newRoute[]type.- Update any documentation or comments referencing the old type structure.
packages/oraidex-common/src/constant.ts (5)
56-56: LGTM: PEPE_BSC_CONTRACT constant added correctly.The constant follows the established naming convention for BSC contracts and contains a valid Ethereum address format.
57-57: LGTM: CAT_BSC_CONTRACT constant added correctly.The constant adheres to the established naming convention for BSC contracts and contains a valid Ethereum address format.
72-72: LGTM: PEPE_ETH_CONTRACT constant added correctly.The constant follows the established naming convention for ETH contracts, contains a valid Ethereum address format, and is appropriately placed among other ETH contract constants.
102-102: LGTM: CAT_ORAICHAIN_EXT_DENOM constant added correctly.The constant adheres to the established naming convention and value format for Oraichain external denominations, and its declaration style is consistent with other constants in the file.
56-57: Summary: New constants for PEPE and CAT tokens added successfully.The changes introduce constants for PEPE and CAT tokens across BSC, ETH, and Oraichain networks. These additions are consistent with the existing structure and naming conventions in the file.
To ensure these changes are fully integrated:
Please run the following script to check for any references to these new token constants in the codebase:
This will help identify any areas in the codebase where these new constants should be used or where they might need to be added.
Also applies to: 72-72, 100-102
Verification successful
Verified: New PEPE and CAT token constants are properly integrated.
All new constants for PEPE and CAT tokens are correctly referenced within the codebase, and no hardcoded addresses were found outside of
constant.ts. The additions maintain consistency and align with existing patterns.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to new PEPE and CAT token constants echo "Checking for references to new token constants:" rg --type typescript "PEPE_BSC_CONTRACT|CAT_BSC_CONTRACT|PEPE_ETH_CONTRACT|PEPE_ORAICHAIN_EXT_DENOM|CAT_ORAICHAIN_EXT_DENOM" -g '!constant.ts' echo "Checking for hardcoded addresses that might need to use these constants:" rg --type typescript "0x25d887Ce7a35172C62FeBFD67a1856F20FaEbB00|0x6894CDe390a3f51155ea41Ed24a33A4827d3063D|0x6982508145454Ce325dDbE47a25d4ec3d2311933" -g '!constant.ts' echo "Checking for potential places to use these new constants:" rg --type typescript "BSC_CONTRACT|ETH_CONTRACT|ORAICHAIN_EXT_DENOM" -g '!constant.ts'Length of output: 895
Script:
#!/bin/bash # Description: Check for references to new PEPE and CAT token constants echo "Checking for references to new token constants:" rg "PEPE_BSC_CONTRACT|CAT_BSC_CONTRACT|PEPE_ETH_CONTRACT|PEPE_ORAICHAIN_EXT_DENOM|CAT_ORAICHAIN_EXT_DENOM" -g '*.ts' echo "Checking for hardcoded addresses that might need to use these constants:" rg "0x25d887Ce7a35172C62FeBFD67a1856F20FaEbB00|0x6894CDe390a3f51155ea41Ed24a33A4827d3063D|0x6982508145454Ce325dDbE47a25d4ec3d2311933" -g '*.ts' echo "Checking for potential places to use these new constants:" rg "BSC_CONTRACT|ETH_CONTRACT|ORAICHAIN_EXT_DENOM" -g '*.ts'Length of output: 15415
packages/universal-swap/src/universal-demos/from-oraichain-to-evm.ts (1)
41-41: Verify the user slippage valueThe
userSlippageis set to1. Depending on how slippage is calculated in your application, this might represent1%or1. Ensure that this value aligns with the expected slippage tolerance in your swap operations.Please confirm that
1is the intended slippage value. Adjust if necessary.packages/oraidex-common/src/helper.ts (1)
262-269:⚠️ Potential issueAddress potential breaking change due to function signature update
The function
getTokenOnOraichainhas changed its signature by replacing the optionaldecimalsparameter withisNative. This modification may introduce breaking changes in parts of the codebase where this function is called with thedecimalsparameter.Run the following script to identify all usages of
getTokenOnOraichainthat may need updating:Verification successful
No Breaking Changes Detected
All usages of
getTokenOnOraichainhave been updated to use theisNativeparameter. No outdated calls with thedecimalsparameter were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all calls to `getTokenOnOraichain` with two arguments, indicating potential outdated usage. # Search for function calls with two arguments ast-grep --lang typescript --pattern 'getTokenOnOraichain($_, $_)' # Expected Result: All occurrences where `getTokenOnOraichain` is called with two arguments. Verify that these calls have been updated to use `isNative` instead of `decimals`.Length of output: 182
packages/oraidex-common/src/network.ts (5)
58-62: Verify imported constants are defined correctlyThe constants
PEPE_ORAICHAIN_EXT_DENOM,CAT_ORAICHAIN_EXT_DENOM,PEPE_ETH_CONTRACT,PEPE_BSC_CONTRACT, andCAT_BSC_CONTRACTare being imported from"./constant". Please ensure that these constants are correctly defined and exportable in the"./constant"module to prevent any import errors.
122-123: Confirm accuracy of added CoinGecko IDsThe CoinGecko IDs
"pepe"and"simon-s-cat"have been added to theCoinGeckoIdtype. Verify that these IDs exactly match the identifiers used by CoinGecko to ensure accurate price retrieval.
989-996: Ensure consistentprefixTokenusage in EVM token configurationsIn the Ethereum PEPE token configuration, the
prefixTokenfield is included, but verify that it aligns with other EVM token configurations for consistency.
675-681:⚠️ Potential issueMissing
prefixTokenin PEPE Ethereum token configurationThe PEPE token entry for Ethereum does not include the
prefixTokenfield, which may lead to inconsistent token handling. IncludingprefixTokenensures proper denomination prefixes are applied.Include
prefixTokenin the configuration:{ coinDenom: "PEPE", coinMinimalDenom: ORAI_BRIDGE_EVM_ETH_DENOM_PREFIX + PEPE_ETH_CONTRACT, bridgeNetworkIdentifier: "0x01", coinDecimals: 18, coinGeckoId: "pepe", + prefixToken: ORAI_BRIDGE_EVM_ETH_DENOM_PREFIX, coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" }Likely invalid or redundant comment.
663-672: 🛠️ Refactor suggestionConsistent use of
prefixTokenacross token configurationsIn the PEPE token configurations, the
prefixTokenfield is used inconsistently. Ensure thatprefixTokenis included where necessary to maintain consistency and avoid potential issues with token denomination prefixes.Add
prefixTokento maintain consistency:{ coinDenom: "PEPE", coinMinimalDenom: ORAI_BRIDGE_EVM_DENOM_PREFIX + PEPE_BSC_CONTRACT, bridgeNetworkIdentifier: "0x38", coinDecimals: 18, coinGeckoId: "pepe", + prefixToken: ORAI_BRIDGE_EVM_DENOM_PREFIX, coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" },Likely invalid or redundant comment.
packages/universal-swap/src/helper.ts (7)
330-330: Usage of optional chaining is correct.The addition of
swapOption?.isSourceReceiverTestsafely accesses the property and prevents runtime errors ifswapOptionis undefined.
468-470: Deprecation notice for 'generateSwapRoute' method added appropriately.The method
generateSwapRouteis correctly marked as deprecated with a reference to useUniversalSwapHelper.generateMsgsSmartRouterV2withV3instead.
505-507: Deprecation notice for 'generateSwapOperationMsgs' method added appropriately.The method
generateSwapOperationMsgsis correctly marked as deprecated, advising to useUniversalSwapHelper.generateMsgsSmartRouterV2withV3instead.
593-595: Deprecation notice for 'simulateSwap' method added appropriately.The method
simulateSwapis correctly marked as deprecated, suggesting to useUniversalSwapHelper.handleSimulateSwapinstead.
747-752: Initialization of default router configuration is correct.The
routerConfigDefaultobject is properly set up using nullish coalescing to provide default values when necessary.
765-771: Function call to 'simulateSwapUsingSmartRoute' is correctly updated.The parameters passed to
simulateSwapUsingSmartRouteare appropriate, and the use ofrouterConfigDefaultensures consistency in configuration.
772-777: 'displayAmount' is calculated using the correct decimals.The
displayAmountis accurately computed usingtoInfo.decimals, which ensures the return value has the correct precision.packages/universal-swap/src/handler.ts (3)
140-140: Confirm theisNativeparameter usage for special chainsIn this line,
getTokenOnOraichainis called withisNativeset totrue. Ensure that this aligns with the intended logic for handling special chains and CoinGecko IDs, and that it correctly fetches native tokens when required.
957-967: Repeated logical error in balance checking conditionSimilar to the previous comment, the condition
if (!this.config?.swapOptions?.isCheckBalanceIbc)is used here. Verify if the negation is needed or if it should be removed to align with the intended logic.
1036-1058: Repeated logical error in balance checking conditionThis condition
if (!this.config?.swapOptions?.isCheckBalanceIbc)appears again. Please ensure that the balance checking logic is consistent throughout the codebase and correct the condition if necessary.
|
|
||
| const oraichainToEvm = async () => { | ||
| const chainId = "Oraichain"; | ||
| const wallet = new CosmosWalletImpl(process.env.MNEMONIC); |
There was a problem hiding this comment.
Add error handling for undefined mnemonic
The process.env.MNEMONIC might be undefined or not properly set. It's advisable to add a check to ensure that the mnemonic is available before initializing the wallet to prevent runtime errors.
Apply this diff to add error handling:
const wallet = new CosmosWalletImpl(process.env.MNEMONIC);
+if (!process.env.MNEMONIC) {
+ throw new Error("MNEMONIC environment variable is not defined");
+}Committable suggestion was skipped due to low confidence.
|
|
||
| const sender = await wallet.getKeplrAddr(chainId); | ||
| const fromAmount = 100; | ||
| let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === "pepe"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding token identifiers
Hardcoding coinGeckoId as "pepe" reduces the flexibility of your function. Consider parameterizing the token identifiers or retrieving them from a configuration to enhance reusability and maintainability.
For example, modify the code to accept token identifiers as parameters:
const fromAmount = 100;
-let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === "pepe");
+const fromCoinGeckoId = "pepe"; // This could be a parameter or from config
+let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === fromCoinGeckoId);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.
| let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === "pepe"); | |
| const fromAmount = 100; | |
| const fromCoinGeckoId = "pepe"; // This could be a parameter or from config | |
| let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === fromCoinGeckoId); |
| { | ||
| originalFromToken, | ||
| originalToToken, | ||
| sender: { cosmos: sender, evm: "0x8c7E0A841269a01c0Ab389Ce8Fb3Cf150A94E797" }, |
There was a problem hiding this comment.
Avoid hardcoding EVM addresses
Hardcoding the EVM address "0x8c7E0A841269a01c0Ab389Ce8Fb3Cf150A94E797" can lead to security issues and reduces flexibility. Consider retrieving the address dynamically or passing it as a parameter.
Example modification:
sender: { cosmos: sender, evm: "0x8c7E0A841269a01c0Ab389Ce8Fb3Cf150A94E797" },
+// Replace hardcoded address with a variable or input parameter
+const evmAddress = process.env.EVM_ADDRESS; // Ensure this is securely managed
+sender: { cosmos: sender, evm: evmAddress },Also, include error handling for cases where evmAddress may be undefined.
Committable suggestion was skipped due to low confidence.
| const fromAmount = 100; | ||
| let originalFromToken = flattenTokens.find((t) => t.chainId === chainId && t.coinGeckoId === "pepe"); | ||
|
|
||
| let originalToToken = flattenTokens.find((t) => t.chainId === "0x38" && t.coinGeckoId === "pepe"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding chain IDs and token identifiers
The chain ID "0x38" (which represents BSC) and coinGeckoId "pepe" are hardcoded. This can make future maintenance difficult if these values change. Consider using constants, configurations, or function parameters to make the code more adaptable.
Example adjustment:
const chainId = "Oraichain";
+const toChainId = "0x38"; // This could be a parameter or from config
+const toCoinGeckoId = "pepe"; // This could be a parameter or from config
-let originalToToken = flattenTokens.find((t) => t.chainId === "0x38" && t.coinGeckoId === "pepe");
+let originalToToken = flattenTokens.find((t) => t.chainId === toChainId && t.coinGeckoId === toCoinGeckoId);Committable suggestion was skipped due to low confidence.
| export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => { | ||
| const filterOraichainToken = oraichainTokens.filter((orai) => orai.coinGeckoId === coingeckoId); | ||
| if (!filterOraichainToken.length) return undefined; | ||
| if (filterOraichainToken.length === 1) return filterOraichainToken[0]; | ||
|
|
||
| const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms)); | ||
| return oraichainToken; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize the token filtering logic
Refactor the conditional logic to improve readability and efficiency by directly using the filter method instead of find.
Apply this diff to simplify the code:
-export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => {
- const filterOraichainToken = oraichainTokens.filter((orai) => orai.coinGeckoId === coingeckoId);
- if (!filterOraichainToken.length) return undefined;
- if (filterOraichainToken.length === 1) return filterOraichainToken[0];
- const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms));
- return oraichainToken;
+export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative: boolean = false) => {
+ const tokens = oraichainTokens.filter(
+ (token) =>
+ token.coinGeckoId === coingeckoId &&
+ (isNative ? !token.evmDenoms : token.evmDenoms)
+ );
+ return tokens.length > 0 ? tokens[0] : undefined;
};Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => { | |
| const filterOraichainToken = oraichainTokens.filter((orai) => orai.coinGeckoId === coingeckoId); | |
| if (!filterOraichainToken.length) return undefined; | |
| if (filterOraichainToken.length === 1) return filterOraichainToken[0]; | |
| const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms)); | |
| return oraichainToken; | |
| }; | |
| export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative: boolean = false) => { | |
| const tokens = oraichainTokens.filter( | |
| (token) => | |
| token.coinGeckoId === coingeckoId && | |
| (isNative ? !token.evmDenoms : token.evmDenoms) | |
| ); | |
| return tokens.length > 0 ? tokens[0] : undefined; | |
| }; |
Ensure correct logic in token selection based on isNative flag
The current logic may not correctly handle cases where multiple tokens share the same coingeckoId. If no token matches the condition in the find method, oraichainToken will be undefined.
Consider providing a default behavior or handling the case when oraichainToken is undefined. Apply this diff to add a fallback:
const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms));
+if (!oraichainToken) {
+ // Fallback to the first token or handle the undefined case appropriately
+ return filterOraichainToken[0];
+}
return oraichainToken;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 getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => { | |
| const filterOraichainToken = oraichainTokens.filter((orai) => orai.coinGeckoId === coingeckoId); | |
| if (!filterOraichainToken.length) return undefined; | |
| if (filterOraichainToken.length === 1) return filterOraichainToken[0]; | |
| const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms)); | |
| return oraichainToken; | |
| }; | |
| export const getTokenOnOraichain = (coingeckoId: CoinGeckoId, isNative?: boolean) => { | |
| const filterOraichainToken = oraichainTokens.filter((orai) => orai.coinGeckoId === coingeckoId); | |
| if (!filterOraichainToken.length) return undefined; | |
| if (filterOraichainToken.length === 1) return filterOraichainToken[0]; | |
| const oraichainToken = filterOraichainToken.find((token) => (isNative ? !token.evmDenoms : token.evmDenoms)); | |
| if (!oraichainToken) { | |
| // Fallback to the first token or handle the undefined case appropriately | |
| return filterOraichainToken[0]; | |
| } | |
| return oraichainToken; | |
| }; |
| coinDenom: "PEPE", | ||
| coinMinimalDenom: "bep20_pepe", | ||
| contractAddress: PEPE_BSC_CONTRACT, | ||
| coinDecimals: 18, | ||
| coinGeckoId: "pepe", | ||
| bridgeTo: ["Oraichain"], | ||
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" |
There was a problem hiding this comment.
Include prefixToken in PEPE BSC token configuration
The PEPE token entry for BSC lacks the prefixToken field, which could cause inconsistencies in token identification. Adding this field will harmonize the configuration with other tokens.
Modify the entry to include prefixToken:
{
coinDenom: "PEPE",
coinMinimalDenom: "bep20_pepe",
contractAddress: PEPE_BSC_CONTRACT,
coinDecimals: 18,
coinGeckoId: "pepe",
bridgeTo: ["Oraichain"],
+ prefixToken: ORAI_BRIDGE_EVM_DENOM_PREFIX,
coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776"
}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.
| coinDenom: "PEPE", | |
| coinMinimalDenom: "bep20_pepe", | |
| contractAddress: PEPE_BSC_CONTRACT, | |
| coinDecimals: 18, | |
| coinGeckoId: "pepe", | |
| bridgeTo: ["Oraichain"], | |
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" | |
| coinDenom: "PEPE", | |
| coinMinimalDenom: "bep20_pepe", | |
| contractAddress: PEPE_BSC_CONTRACT, | |
| coinDecimals: 18, | |
| coinGeckoId: "pepe", | |
| bridgeTo: ["Oraichain"], | |
| prefixToken: ORAI_BRIDGE_EVM_DENOM_PREFIX, | |
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" |
| coinDenom: "PEPE", | ||
| coinGeckoId: "pepe", | ||
| coinMinimalDenom: PEPE_ORAICHAIN_DENOM, | ||
| bridgeTo: ["0x38", "0x01"], | ||
| coinDecimals: 6, | ||
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" | ||
| }, |
There was a problem hiding this comment.
Potential mismatch in coinDecimals for PEPE token
The PEPE token is set with coinDecimals: 6. Typically, PEPE token operates with 18 decimals on Ethereum and BSC networks. Using an incorrect decimal value may lead to misrepresentation of token balances and transaction amounts.
Update the coinDecimals to 18 to align with the standard:
{
coinDenom: "PEPE",
coinGeckoId: "pepe",
coinMinimalDenom: PEPE_ORAICHAIN_DENOM,
bridgeTo: ["0x38", "0x01"],
- coinDecimals: 6,
+ coinDecimals: 18,
coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776"
},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.
| coinDenom: "PEPE", | |
| coinGeckoId: "pepe", | |
| coinMinimalDenom: PEPE_ORAICHAIN_DENOM, | |
| bridgeTo: ["0x38", "0x01"], | |
| coinDecimals: 6, | |
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" | |
| }, | |
| coinDenom: "PEPE", | |
| coinGeckoId: "pepe", | |
| coinMinimalDenom: PEPE_ORAICHAIN_DENOM, | |
| bridgeTo: ["0x38", "0x01"], | |
| coinDecimals: 18, | |
| coinImageUrl: "https://assets.coingecko.com/coins/images/29850/standard/pepe-token.jpeg?1696528776" | |
| }, |
| if (!fromInfo || !toInfo) | ||
| throw new Error(`Cannot find token on Oraichain for token ${fromInfo.coinGeckoId} and ${toInfo.coinGeckoId}`); | ||
|
|
There was a problem hiding this comment.
Potential runtime error when accessing properties of undefined variables.
If fromInfo or toInfo is undefined, accessing fromInfo.coinGeckoId or toInfo.coinGeckoId will result in a runtime error.
Please modify the error handling to prevent this:
if (!fromInfo || !toInfo)
- throw new Error(`Cannot find token on Oraichain for token ${fromInfo.coinGeckoId} and ${toInfo.coinGeckoId}`);
+ throw new Error(`Cannot find token on Oraichain for tokens: fromInfo=${fromInfo?.coinGeckoId}, toInfo=${toInfo?.coinGeckoId}`);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 (!fromInfo || !toInfo) | |
| throw new Error(`Cannot find token on Oraichain for token ${fromInfo.coinGeckoId} and ${toInfo.coinGeckoId}`); | |
| if (!fromInfo || !toInfo) | |
| throw new Error(`Cannot find token on Oraichain for tokens: fromInfo=${fromInfo?.coinGeckoId}, toInfo=${toInfo?.coinGeckoId}`); |
| if (!this.config?.swapOptions?.isCheckBalanceIbc) { | ||
| const routerClient = new OraiswapRouterQueryClient(client, network.mixer_router); | ||
| const isSufficient = await UniversalSwapHelper.checkFeeRelayer({ | ||
| originalFromToken: this.swapData.originalFromToken, | ||
| fromAmount: this.swapData.fromAmount, | ||
| relayerFee: this.swapData.relayerFee, | ||
| routerClient | ||
| }); | ||
| if (!isSufficient) | ||
| throw generateError( | ||
| `Your swap amount ${this.swapData.fromAmount} cannot cover the fees for this transaction. Please try again with a higher swap amount` | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential logical error in balance checking condition
The condition if (!this.config?.swapOptions?.isCheckBalanceIbc) may be incorrect. Currently, balance checks are performed when isCheckBalanceIbc is false or undefined. If the intention is to perform balance checks only when isCheckBalanceIbc is true, consider removing the negation.
Apply this diff to correct the condition:
- if (!this.config?.swapOptions?.isCheckBalanceIbc) {
+ if (this.config?.swapOptions?.isCheckBalanceIbc) {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 (!this.config?.swapOptions?.isCheckBalanceIbc) { | |
| const routerClient = new OraiswapRouterQueryClient(client, network.mixer_router); | |
| const isSufficient = await UniversalSwapHelper.checkFeeRelayer({ | |
| originalFromToken: this.swapData.originalFromToken, | |
| fromAmount: this.swapData.fromAmount, | |
| relayerFee: this.swapData.relayerFee, | |
| routerClient | |
| }); | |
| if (!isSufficient) | |
| throw generateError( | |
| `Your swap amount ${this.swapData.fromAmount} cannot cover the fees for this transaction. Please try again with a higher swap amount` | |
| ); | |
| } | |
| if (this.config?.swapOptions?.isCheckBalanceIbc) { | |
| const routerClient = new OraiswapRouterQueryClient(client, network.mixer_router); | |
| const isSufficient = await UniversalSwapHelper.checkFeeRelayer({ | |
| originalFromToken: this.swapData.originalFromToken, | |
| fromAmount: this.swapData.fromAmount, | |
| relayerFee: this.swapData.relayerFee, | |
| routerClient | |
| }); | |
| if (!isSufficient) | |
| throw generateError( | |
| `Your swap amount ${this.swapData.fromAmount} cannot cover the fees for this transaction. Please try again with a higher swap amount` | |
| ); | |
| } |
| private getTokenOnOraichain(coinGeckoId: CoinGeckoId, isNative?: boolean): TokenItemType { | ||
| const fromTokenOnOrai = getTokenOnOraichain(coinGeckoId, isNative); |
There was a problem hiding this comment.
💡 Codebase verification
Some calls to getTokenOnOraichain are not updated to the new signature with the isNative parameter. Please update these usages accordingly:
packages/universal-swap/tests/helper.spec.ts- Multiple instances without the
isNativeparameter.
- Multiple instances without the
packages/oraidex-common/tests/helper.spec.ts- Calls missing the
isNativeparameter.
- Calls missing the
packages/universal-swap/src/helper.ts- Several occurrences without the updated parameter.
packages/universal-swap/src/handler.ts- Multiple lines need to include the
isNativeparameter.
- Multiple lines need to include the
packages/universal-swap/src/universal-demos/ibc-hooks-demo.ts- Missing
isNativeparameter in calls.
- Missing
Analysis chain
Ensure all calls to getTokenOnOraichain are updated to the new signature
The method getTokenOnOraichain now accepts isNative?: boolean instead of decimals?: number. Please verify that all usages of this method have been updated accordingly and that the isNative parameter is correctly provided where necessary.
Run the following script to find all usages of getTokenOnOraichain:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all calls to 'getTokenOnOraichain' and display the lines with context
rg --type typescript "getTokenOnOraichain\(" -A 1
Length of output: 86
Script:
#!/bin/bash
# Description: Find all calls to 'getTokenOnOraichain' within TypeScript files and display the lines with context
rg "getTokenOnOraichain\(" --glob "*.ts" --glob "*.tsx" -A 1
Length of output: 6000
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores