feat: add stellar script #6
Conversation
|
""" WalkthroughThis update restructures chain configuration and gas estimation logic, moving and refactoring related modules into Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StellarScript as scripts/stellar.ts
participant Wallet as src/stellar/wallet.ts
participant Utils as src/stellar/utils.ts
participant Chains as src/common/chains.ts
participant Gas as src/common/gasEstimation.ts
participant StellarSDK
User->>StellarScript: Run script with params
StellarScript->>Wallet: getWallet()
Wallet-->>StellarScript: Keypair
StellarScript->>Utils: getBalances(address)
Utils-->>StellarScript: Balances
StellarScript->>Chains: getChainConfig("stellar")
Chains-->>StellarScript: Stellar Chain Config
StellarScript->>Chains: getChainConfig(destinationChain)
Chains-->>StellarScript: Destination Chain Config
StellarScript->>Gas: calculateEstimatedFee("stellar", destinationChainConfig)
Gas-->>StellarScript: Estimated Fee
StellarScript->>StellarSDK: Prepare and send transaction
StellarSDK-->>StellarScript: Transaction hash
StellarScript->>Utils: waitForTransaction(server, hash)
Utils-->>StellarScript: Transaction confirmation
StellarScript-->>User: Output transaction result
Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
scripts/xrpl-trust-line.ts (1)
8-8:⚠️ Potential issueValidate XRPL seed before use
Defaulting to an empty string may lead to confusing runtime failures. Add a guard to error out ifXRPL_SEEDis missing:if (!xrplWalletSeed) { throw new Error("Environment variable XRPL_SEED must be set"); }
🧹 Nitpick comments (10)
.env.example (1)
3-3: Add Stellar private key placeholder
IncludingSTELLAR_PRIVATE_KEY=aligns with the new Stellar wallet integration.Consider updating the project README or contributing guide to document this new environment variable and its expected format.
scripts/xrpl-trust-line.ts (1)
18-18: Avoid hardcoded RPC URL
Rather than embedding the testnet URL, consider using the chain config fromgetXrplChainConfig()to derive the correct endpoint. This centralizes configuration and eases mainnet/testnet switches.src/stellar/wallet.ts (1)
1-12: Good implementation of Stellar wallet accessThe function correctly retrieves the Stellar private key from environment variables and creates a keypair. This follows secure practices by not hardcoding the key.
I have a few suggestions to improve the robustness:
Consider adding error handling for malformed private keys:
export function getWallet() { const privateKey = process.env.STELLAR_PRIVATE_KEY; if (!privateKey) { throw new Error("STELLAR_PRIVATE_KEY is not set"); } + try { const keypair = Keypair.fromSecret(privateKey); return keypair; + } catch (error) { + throw new Error(`Invalid Stellar private key: ${error.message}`); + } }Adding documentation about the expected format of the private key would be helpful:
/** + * Retrieves the Stellar wallet keypair from environment variables. + * @returns {Keypair} The Stellar keypair object + * @throws {Error} If STELLAR_PRIVATE_KEY is not set or is invalid + */ export function getWallet() {src/common/types.ts (1)
38-44: Added new chain configuration typesThe addition of
XrplChainConfigandStellarChainConfigtypes provides type safety for these chains.Consider adding JSDoc comments to document these types:
/** + * Configuration for XRPL blockchain + */ export type XrplChainConfig = BaseChainConfig & { chainType: "xrpl"; }; /** + * Configuration for Stellar blockchain + */ export type StellarChainConfig = BaseChainConfig & { chainType: "stellar"; };src/common/gasEstimation.ts (2)
11-38: Consider refactoring duplicated gas limit values.The function has good documentation referencing the source of the gas values. However, there's duplication between the testnet and non-testnet environments for some chain types.
Consider refactoring to reduce duplication and improve maintainability:
export function getGasLimit(chainType: string): string { const gasLimits: Record<string, Record<string, string>> = { testnet: { sui: "2750", xrpl: "165000", stellar: "8000000", default: "500000" }, default: { sui: "70000", xrpl: "165000", stellar: "8000000", default: "500000" } }; const envLimits = gasLimits[environment === "testnet" ? "testnet" : "default"]; return envLimits[chainType] || envLimits.default; }
51-63: Correct typo in comment.The hop parameters structure looks good, but there's a typo in the comment.
- // this value will be overrided by the axelarscan api + // this value will be overridden by the axelarscan apisrc/stellar/utils.ts (2)
1-5: Consistent import structure.The imports are properly organized, but there's a small inconsistency with the
common/envimport path.-import { environment } from "common/env"; +import { environment } from "../common/env";This would make it consistent with other import paths in the codebase.
51-68: Enhance error handling in getBalances function.The function has good error handling for 404 errors, but could be improved for other error types.
export async function getBalances(address: string) { const horizonRpcUrl = getHorizonRpcUrl(); const horizonServer = new Horizon.Server(horizonRpcUrl); const response = await horizonServer .accounts() .accountId(address) .call() .catch((error) => { if (error?.response?.status === 404) { return { balances: [] }; } + // Log the error for debugging purposes + console.error(`Error fetching balances for ${address}:`, error); throw error; }); return response.balances; }scripts/stellar.ts (2)
1-22: Organize imports and remove empty import.The import structure has some issues that should be addressed.
-import {} from "@stellar/stellar-sdk"; +import { + TransactionBuilder, + Contract, + nativeToScVal, + rpc, + BASE_FEE, + Networks, +} from "@stellar/stellar-sdk"; import { getChainConfig } from "common/chains"; import { formatUnits, parseUnits } from "ethers"; import { getWallet } from "stellar/wallet"; -import { - TransactionBuilder, - Contract, - nativeToScVal, - rpc, - BASE_FEE, - Networks, -} from "@stellar/stellar-sdk";
23-37: Clean up commented code and document hard-coded values.There's commented out code and some hard-coded values without explanation.
// --- Constants --- const DESTINATION_CHAIN: string = process.argv[2] || "sui"; const DESTINATION_ADDRESS = process.argv[3] || "0xba353a510d8a1174b37c31e6eab6e2d6d93cdb31cd093efdd30c177853533ab0"; const UNIT_AMOUNT = parseUnits(process.argv[4] || "1", 9); console.log("Environment:", environment); -// SQD token -// const TOKEN_ADDRESS = -// "CDBBR6BVVZO32NMFCJJWRJFTKV3QQ2ZLITKKGINP2ONUQCTK7PCT4M3W"; +// Interchain Token Service token ID const ITS_TOKEN_ID = "0x42e69c5a9903ba193f3e9214d41b1ad495faace3ca712fb0c9d0c44cc4d31a0c";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (15)
.env.example(1 hunks)common/chains.ts(0 hunks)common/gasEstimation.ts(0 hunks)package.json(2 hunks)scripts/gen-xrpl-wallet.ts(1 hunks)scripts/stellar.ts(1 hunks)scripts/sui.ts(5 hunks)scripts/xrpl-trust-line.ts(1 hunks)scripts/xrpl.ts(6 hunks)src/common/chains.ts(1 hunks)src/common/gasEstimation.ts(1 hunks)src/common/types.ts(1 hunks)src/stellar/utils.ts(1 hunks)src/stellar/wallet.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (2)
- common/gasEstimation.ts
- common/chains.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/common/gasEstimation.ts (2)
src/common/env.ts (1)
environment(3-4)src/common/types.ts (1)
BaseChainConfig(20-28)
src/stellar/utils.ts (1)
src/common/env.ts (1)
environment(3-4)
src/common/chains.ts (1)
src/common/env.ts (1)
environment(3-4)
🔇 Additional comments (29)
tsconfig.json (2)
4-7: Formatting improvement for thelibarray
Switching to a multi-line array enhances readability and maintainability without altering functionality.
25-26: Enable absolute imports viabaseUrl
Adding"baseUrl": "src"correctly supports the new non-relative import patterns (e.g.,import {…} from "xrpl/wallet").scripts/gen-xrpl-wallet.ts (1)
1-1: Standardize import path forgenerateWallet
Switching to an absolute import ("xrpl/wallet") aligns with the updated module resolution (baseUrl: "src") and keeps imports concise.scripts/xrpl-trust-line.ts (1)
1-3: Normalize script imports using absolute paths
The updated imports ("xrpl/tx","common/chains","xrpl/wallet") correctly follow the new project structure and thebaseUrlcompiler option.package.json (2)
9-13: Update NPM script paths toscripts/folder
Prefixing all script commands withscripts/ensures they execute the correct files in the new directory structure. Bun’s native TS support will handle the.tsextensions.
22-22: Add Stellar SDK dependency
Pinning@stellar/stellar-sdk@^13.3.0integrates the official Stellar library for the new interchain transfer script.scripts/sui.ts (6)
3-11: Import paths updated correctlyThe imports have been standardized to use non-relative paths, which is consistent with the code reorganization.
46-46: RPC URL retrieval from chain configThe Sui client initialization now correctly uses the RPC URL from the chain configuration.
65-65: Updated fee estimation function callThe fee calculation now correctly passes the full destination chain config object instead of just the chain name.
68-73: Formatted console log outputFormatting changes to improve readability of the console output.
149-151: Updated block explorer URL usageThe transaction hash output now correctly references the Sui chain config's block explorer URL.
42-45:Details
✅ Verification successful
Chain configuration retrieval updated
The script now correctly fetches separate chain configurations for Sui and the destination chain.
Let's verify that these configuration objects are always available:
🏁 Script executed:
#!/bin/bash # Check how the getChainConfig function is implemented and whether it has error handling ast-grep --pattern $'export function getChainConfig($name) { $$$ }'Length of output: 70
To locate and inspect the
getChainConfigimplementation (and any error handling around missing configs), let’s search for all its usages and the definition:
🏁 Script executed:
#!/bin/bash # Find every occurrence of getChainConfig and its definition rg -n "getChainConfig" -C 3Length of output: 3263
Let’s inspect the full
getChainConfigimplementation and any error handling around missing entries:
🏁 Script executed:
#!/bin/bash # Show the body of getChainConfig for review rg -n "export async function getChainConfig" -A 40 src/common/chains.tsLength of output: 1065
getChainConfig fails fast on missing chains
The
getChainConfigimplementation throws a descriptive error (Chain ${chainName} not found) if no matching configuration is found, so calls toawait getChainConfig("sui") await getChainConfig(DESTINATION_CHAIN)either return a valid config or abort execution. No additional presence checks are needed unless you want to handle missing configs more gracefully.
scripts/xrpl.ts (4)
1-8: Import paths and organization improvedThe imports have been standardized to use non-relative paths and consolidated where appropriate.
35-38: Chain configuration retrieval updatedThe script now correctly fetches separate chain configurations for XRPL and the destination chain.
137-141: Removed trailing comma in string concatenationMinor formatting improvement by removing trailing comma.
113-122:Details
❌ Incorrect review comment
Fee calculation logic updated
The estimated fee calculation now correctly passes chain config objects instead of just chain IDs.
There's a potential issue in the fee calculation logic. Line 120 passes
xrplChainConfig.idas the first parameter tocalculateEstimatedFee, but based on the usage in the Sui script (line 65), it might expect the full chain config object:- fee = await calculateEstimatedFee(xrplChainConfig.id, destinationChainConfig); + fee = await calculateEstimatedFee(xrplChainConfig, destinationChainConfig);Let's verify the signature of the
calculateEstimatedFeefunction:
🏁 Script executed:
#!/bin/bash # Check the signature of the calculateEstimatedFee function ast-grep --pattern $'export function calculateEstimatedFee($first, $second) { $$$ }'Length of output: 87
Let’s locate the actual definition of
calculateEstimatedFeeto confirm its parameter types:
🏁 Script executed:
#!/bin/bash # Search for the calculateEstimatedFee definition with context rg -n "calculateEstimatedFee" -C3Length of output: 2712
calculateEstimatedFee invocation parameters are correct
The
calculateEstimatedFeefunction is defined to accept astringas its first argument and aBaseChainConfigas its second. PassingxrplChainConfig.id(a string) aligns with its signature, so no change is needed.
- Signature in
src/common/gasEstimation.ts(lines 40–43):
export async function calculateEstimatedFee(sourceChain: string, destinationChainConfig: BaseChainConfig): Promise<string>- Callsite in
scripts/xrpl.ts:120:
fee = await calculateEstimatedFee(xrplChainConfig.id, destinationChainConfig);Likely an incorrect or invalid review comment.
src/common/types.ts (2)
20-28: Well-designed base chain configuration typeThe
BaseChainConfigtype provides a good foundation for different chain implementations with common properties.
30-36: Updated Sui chain configuration typeThe
SuiChainConfignow properly extends the base type with Sui-specific properties.src/common/gasEstimation.ts (3)
1-4: Good import structure and type references.The imports are clear and properly organized, importing the necessary dependencies for the gas estimation functionality.
5-9: Well-defined type for hop parameters.The
HopParamstype clearly defines the structure needed for multi-hop transfers with Axelar.
40-49: Well-structured fee calculation function signature.The function signature is clear and the log statement provides useful debugging information.
src/stellar/utils.ts (3)
6-12: Good utility functions for Stellar value conversions.These utility functions provide clean abstractions for converting different types to Stellar's ScVal format.
14-29: Well-structured tokenToScVal function.The function handles the null case appropriately and provides proper typing for the complex structure.
43-49: Good environment-based URL selection.The function correctly handles different environments for the Horizon RPC URL.
src/common/chains.ts (2)
1-7: Good type imports and organization.The imports are well-structured, importing the necessary types for the chain configuration functionality.
19-33: Good implementation of partial chain name matching.The chain configuration retrieval function is well implemented with a clear comment explaining the partial matching feature.
However, the function could benefit from a more robust type definition for the return value.
-export async function getChainConfig(chainName: string) { +export async function getChainConfig(chainName: string): Promise<BaseChainConfig> {scripts/stellar.ts (3)
38-45: LGTM: Wallet initialization and balance retrieval.The wallet initialization and balance fetching look correct.
46-56: LGTM: Chain configuration and gas fee calculation.The chain configuration retrieval and gas fee calculation are properly implemented.
68-77: LGTM: Well-structured contract call setup.The contract call setup is clear and properly structured with all necessary parameters.
| return (await sdk.estimateMultihopFee(hopParams)) as string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the fee calculation.
The function doesn't handle potential errors from the SDK call.
- return (await sdk.estimateMultihopFee(hopParams)) as string;
+ try {
+ return (await sdk.estimateMultihopFee(hopParams)) as string;
+ } catch (error) {
+ console.error("Error estimating multihop fee:", error);
+ throw new Error(`Failed to estimate fee: ${error instanceof Error ? error.message : String(error)}`);
+ }📝 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.
| return (await sdk.estimateMultihopFee(hopParams)) as string; | |
| } | |
| try { | |
| return (await sdk.estimateMultihopFee(hopParams)) as string; | |
| } catch (error) { | |
| console.error("Error estimating multihop fee:", error); | |
| throw new Error(`Failed to estimate fee: ${error instanceof Error ? error.message : String(error)}`); | |
| } | |
| } |
🤖 Prompt for AI Agents (early access)
In src/common/gasEstimation.ts around lines 65 to 66, the call to sdk.estimateMultihopFee lacks error handling. Wrap the await sdk.estimateMultihopFee(hopParams) call in a try-catch block to catch any errors thrown by the SDK. In the catch block, handle the error appropriately, such as logging it or rethrowing a custom error, to ensure the function gracefully manages failures during fee estimation.
| export async function waitForTransaction(server: rpc.Server, hash: string) { | ||
| let pendingTx = await server.getTransaction(hash); | ||
|
|
||
| while (pendingTx.status === "NOT_FOUND") { | ||
| pendingTx = await server.getTransaction(hash); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| console.log("Waiting for transaction to be included in a ledger..."); | ||
| } | ||
|
|
||
| return pendingTx; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve waitForTransaction with timeout and max retries.
The transaction waiting function could potentially run indefinitely without proper bounds.
-export async function waitForTransaction(server: rpc.Server, hash: string) {
+export async function waitForTransaction(
+ server: rpc.Server,
+ hash: string,
+ maxRetries = 60,
+ delayMs = 1000
+) {
let pendingTx = await server.getTransaction(hash);
+ let attempts = 0;
- while (pendingTx.status === "NOT_FOUND") {
+ while (pendingTx.status === "NOT_FOUND" && attempts < maxRetries) {
pendingTx = await server.getTransaction(hash);
- await new Promise((resolve) => setTimeout(resolve, 1000));
+ await new Promise((resolve) => setTimeout(resolve, delayMs));
console.log("Waiting for transaction to be included in a ledger...");
+ attempts++;
}
+ if (attempts >= maxRetries) {
+ throw new Error(`Transaction ${hash} not confirmed after ${maxRetries} attempts`);
+ }
return pendingTx;
}📝 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 async function waitForTransaction(server: rpc.Server, hash: string) { | |
| let pendingTx = await server.getTransaction(hash); | |
| while (pendingTx.status === "NOT_FOUND") { | |
| pendingTx = await server.getTransaction(hash); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| console.log("Waiting for transaction to be included in a ledger..."); | |
| } | |
| return pendingTx; | |
| } | |
| export async function waitForTransaction( | |
| server: rpc.Server, | |
| hash: string, | |
| maxRetries = 60, | |
| delayMs = 1000 | |
| ) { | |
| let pendingTx = await server.getTransaction(hash); | |
| let attempts = 0; | |
| while (pendingTx.status === "NOT_FOUND" && attempts < maxRetries) { | |
| pendingTx = await server.getTransaction(hash); | |
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | |
| console.log("Waiting for transaction to be included in a ledger..."); | |
| attempts++; | |
| } | |
| if (attempts >= maxRetries) { | |
| throw new Error(`Transaction ${hash} not confirmed after ${maxRetries} attempts`); | |
| } | |
| return pendingTx; | |
| } |
🤖 Prompt for AI Agents (early access)
In src/stellar/utils.ts around lines 31 to 41, the waitForTransaction function currently loops indefinitely if the transaction is never found. To fix this, add a timeout mechanism or a maximum retry count to limit how long the function waits. Implement a counter that increments each loop iteration and break the loop or throw an error if the max retries or timeout duration is exceeded, ensuring the function does not run indefinitely.
| export async function getChainConfigs() { | ||
| let s3ConfigUrl; | ||
| if (environment === "devnet-amplifier") { | ||
| s3ConfigUrl = `https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json`; | ||
| } else { | ||
| s3ConfigUrl = `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`; | ||
| } | ||
|
|
||
| return fetch(s3ConfigUrl).then((res) => res.json()); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add error handling and consider configuration caching.
The function lacks error handling for the fetch operation and doesn't implement caching for performance.
export async function getChainConfigs() {
let s3ConfigUrl;
if (environment === "devnet-amplifier") {
s3ConfigUrl = `https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json`;
} else {
s3ConfigUrl = `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`;
}
- return fetch(s3ConfigUrl).then((res) => res.json());
+ try {
+ const response = await fetch(s3ConfigUrl);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch chain configs: ${response.status} ${response.statusText}`);
+ }
+ return response.json();
+ } catch (error) {
+ console.error("Error fetching chain configs:", error);
+ throw new Error(`Failed to load chain configurations: ${error instanceof Error ? error.message : String(error)}`);
+ }
}Also, consider implementing a caching mechanism to avoid redundant fetches. Let's check if this function is called multiple times in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for getChainConfigs function calls
rg "getChainConfigs\(\)" -A 2 -B 2Length of output: 654
Ensure robust fetching and add in-memory caching in getChainConfigs
To prevent unhandled fetch errors and redundant network calls, update the function in src/common/chains.ts as follows:
• File: src/common/chains.ts (lines ~8–17)
• Wrap the fetch call in try/catch and verify response.ok
• Store the parsed JSON in a module‐scoped cache so subsequent calls return the same data
Proposed diff:
export async function getChainConfigs() {
- let s3ConfigUrl;
- if (environment === "devnet-amplifier") {
- s3ConfigUrl = `https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json`;
- } else {
- s3ConfigUrl = `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`;
- }
-
- return fetch(s3ConfigUrl).then((res) => res.json());
+ // simple in-memory cache
+ if (cachedConfigs) {
+ return cachedConfigs;
+ }
+ const s3ConfigUrl =
+ environment === "devnet-amplifier"
+ ? "https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json"
+ : `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`;
+
+ try {
+ const response = await fetch(s3ConfigUrl);
+ if (!response.ok) {
+ throw new Error(`HTTP ${response.status}: ${response.statusText}`);
+ }
+ const data = await response.json();
+ cachedConfigs = data;
+ return data;
+ } catch (err) {
+ console.error("Failed to load chain configs:", err);
+ throw new Error(`Unable to fetch chain configurations: ${err instanceof Error ? err.message : String(err)}`);
+ }
}
+// declare module‐level cache
+let cachedConfigs: ReturnType<typeof getChainConfigs> | null = null;📝 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 async function getChainConfigs() { | |
| let s3ConfigUrl; | |
| if (environment === "devnet-amplifier") { | |
| s3ConfigUrl = `https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json`; | |
| } else { | |
| s3ConfigUrl = `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`; | |
| } | |
| return fetch(s3ConfigUrl).then((res) => res.json()); | |
| } | |
| export async function getChainConfigs() { | |
| // simple in-memory cache | |
| if (cachedConfigs) { | |
| return cachedConfigs; | |
| } | |
| const s3ConfigUrl = | |
| environment === "devnet-amplifier" | |
| ? "https://axelar-devnets.s3.us-east-2.amazonaws.com/configs/devnet-amplifier-config-1.0.x.json" | |
| : `https://axelar-${environment}.s3.us-east-2.amazonaws.com/configs/${environment}-config-1.x.json`; | |
| try { | |
| const response = await fetch(s3ConfigUrl); | |
| if (!response.ok) { | |
| throw new Error(`HTTP ${response.status}: ${response.statusText}`); | |
| } | |
| const data = await response.json(); | |
| cachedConfigs = data; | |
| return data; | |
| } catch (err) { | |
| console.error("Failed to load chain configs:", err); | |
| throw new Error( | |
| `Unable to fetch chain configurations: ${ | |
| err instanceof Error ? err.message : String(err) | |
| }` | |
| ); | |
| } | |
| } | |
| // declare module‐level cache | |
| let cachedConfigs: ReturnType<typeof getChainConfigs> | null = null; |
🤖 Prompt for AI Agents (early access)
In src/common/chains.ts around lines 8 to 17, the getChainConfigs function lacks error handling for the fetch call and does not cache the fetched configuration. To fix this, wrap the fetch call in a try/catch block, check if the response is ok before parsing JSON, and throw an error if not. Additionally, implement a module-scoped variable to cache the parsed configuration so that subsequent calls return the cached data instead of making redundant network requests.
| const builtTransaction = new TransactionBuilder(account, { | ||
| fee: BASE_FEE, | ||
| networkPassphrase: | ||
| environment === "mainnet" ? Networks.PUBLIC : Networks.TESTNET, | ||
| }) | ||
| .addOperation(operation) | ||
| .setTimeout(30) | ||
| .build(); | ||
|
|
||
| const prepareTransaction = await server.prepareTransaction(builtTransaction); | ||
|
|
||
| prepareTransaction.sign(wallet); | ||
|
|
||
| const txReceipt = await server.sendTransaction(prepareTransaction); | ||
|
|
||
| const tx = await waitForTransaction(server, txReceipt.hash); | ||
|
|
||
| console.log("Transaction Hash:", tx.txHash); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive error handling.
The transaction building, signing, and submission lack error handling.
-const builtTransaction = new TransactionBuilder(account, {
- fee: BASE_FEE,
- networkPassphrase:
- environment === "mainnet" ? Networks.PUBLIC : Networks.TESTNET,
-})
- .addOperation(operation)
- .setTimeout(30)
- .build();
-
-const prepareTransaction = await server.prepareTransaction(builtTransaction);
-
-prepareTransaction.sign(wallet);
-
-const txReceipt = await server.sendTransaction(prepareTransaction);
-
-const tx = await waitForTransaction(server, txReceipt.hash);
-
-console.log("Transaction Hash:", tx.txHash);
+try {
+ const builtTransaction = new TransactionBuilder(account, {
+ fee: BASE_FEE,
+ networkPassphrase:
+ environment === "mainnet" ? Networks.PUBLIC : Networks.TESTNET,
+ })
+ .addOperation(operation)
+ .setTimeout(30)
+ .build();
+
+ console.log("Transaction built successfully");
+
+ const prepareTransaction = await server.prepareTransaction(builtTransaction);
+ console.log("Transaction prepared successfully");
+
+ prepareTransaction.sign(wallet);
+ console.log("Transaction signed successfully");
+
+ const txReceipt = await server.sendTransaction(prepareTransaction);
+ console.log("Transaction submitted successfully");
+
+ const tx = await waitForTransaction(server, txReceipt.hash);
+
+ console.log("Transaction Hash:", tx.txHash);
+ console.log("Transaction completed successfully");
+} catch (error) {
+ console.error("Error executing Stellar transaction:", error);
+ process.exit(1);
+}🤖 Prompt for AI Agents (early access)
In scripts/stellar.ts around lines 79 to 96, the transaction building, signing, and submission process lacks error handling. Wrap the entire sequence in a try-catch block to catch any exceptions during building, signing, preparing, sending, or waiting for the transaction. In the catch block, log or handle the error appropriately to ensure failures are detected and managed gracefully.
scripts/stellar.ts
Outdated
| const server = new rpc.Server(stellarChainConfig.config.rpc[0]); | ||
| const contractId = | ||
| stellarChainConfig.config.contracts.InterchainTokenService.address; | ||
| const contract = new Contract(contractId); | ||
| const caller = addressToScVal(wallet.publicKey()); | ||
| const account = await server.getAccount(walletAddress); | ||
|
|
||
| // TODO: retrieve the gas token dynamically | ||
| const gasTokenAddress = | ||
| "CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC"; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address the TODO comment for dynamic gas token retrieval.
The code has a TODO comment that should be addressed.
The gas token address is hardcoded. This should be dynamically retrieved as mentioned in the TODO comment. Let's check if there's a standard method for retrieving gas tokens in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for gas token retrieval patterns in other scripts
rg -A 3 -B 3 "gasToken" --type tsLength of output: 747
Implement Dynamic Gas Token Retrieval
The hardcoded gas token address remains in scripts/stellar.ts (lines 61–67) despite the TODO. Since no existing helper was found in the codebase, please replace the literal string with a dynamic lookup:
- Remove the hardcoded
const gasTokenAddress = "CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC";
- Introduce a utility (or extend stellarChainConfig) to fetch the native asset’s token address from the RPC endpoint or configuration.
- Update the call site to use this helper:
const gasTokenAddress = await getNativeAssetTokenAddress(server);
This will satisfy the TODO and ensure the gas token is always fetched dynamically.
🤖 Prompt for AI Agents (early access)
In scripts/stellar.ts around lines 57 to 67, replace the hardcoded gas token address with a dynamic retrieval method to address the TODO comment. Remove the literal gas token string and implement or use a utility function, such as getNativeAssetTokenAddress, that fetches the native asset token address from the RPC server or configuration. Then update the code to call this function asynchronously to assign gasTokenAddress, ensuring the gas token is retrieved dynamically rather than hardcoded.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/common/axelarscan.ts (1)
3-15: Add documentation for the getAxelarscanLink functionConsider adding JSDoc comments to document the purpose, parameters, and return value of this utility function, which will improve code maintainability and developer experience.
+/** + * Generates a URL to view a transaction on Axelarscan explorer + * @param txHash - The transaction hash + * @param txIndex - Optional transaction index for multi-action transactions + * @returns URL to the transaction on Axelarscan (mainnet or testnet based on environment) + */ export function getAxelarscanLink(txHash: string, txIndex?: number) { // rest of the function remains unchanged }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/stellar.ts(1 hunks)src/common/axelarscan.ts(1 hunks)src/common/gasEstimation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/common/gasEstimation.ts
- scripts/stellar.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/common/axelarscan.ts (1)
src/common/env.ts (1)
environment(3-4)
🔇 Additional comments (1)
src/common/axelarscan.ts (1)
10-12:Details
✅ Verification successful
URL format for transaction indexes
The current implementation appends transaction indexes with a hyphen (
-) to the base URL. Verify this matches Axelarscan's URL format requirements for transactions with indexes.
🏁 Script executed:
#!/bin/bash # Check if Axelarscan URLs with transaction indexes use the format tx/{hash}-{index} echo "Verifying Axelarscan URL format with transaction indexes..." # Try both potential formats to see which one redirects correctly curl -I -s "https://testnet.axelarscan.io/tx/0x1234-0" | grep -i "location:" curl -I -s "https://testnet.axelarscan.io/tx/0x1234/0" | grep -i "location:"Length of output: 278
To get definitive HTTP status and redirect behavior for each URL format, let’s capture both the status code and the final effective URL:
🏁 Script executed:
#!/bin/bash echo "Checking HTTP status codes and effective URLs for both Axelarscan tx formats..." urls=( "https://testnet.axelarscan.io/tx/0x1234-0" "https://testnet.axelarscan.io/tx/0x1234/0" ) for url in "${urls[@]}"; do code=$(curl -o /dev/null -s -w "%{http_code}" "$url") eff=$(curl -Ls -o /dev/null -w "%{url_effective}" "$url") echo "$url → HTTP $code, effective URL: $eff" doneLength of output: 1243
Correct URL format confirmed
Verified that Axelarscan uses the hyphenated transaction‐index format (
/tx/{hash}-{index}), which returns HTTP 200 (while the slash format returns 404). No changes needed.
| if (environment === "mainnet") { | ||
| axelarscanTxLink = `https://axelarscan.io/tx/${txHash}`; | ||
| } | ||
| axelarscanTxLink = `https://testnet.axelarscan.io/tx/${txHash}`; |
There was a problem hiding this comment.
Fix conditional URL logic
There's a logical error in the environment condition handling. The testnet URL assignment on line 8 will always execute, overriding the mainnet URL even when environment === "mainnet". This means the function will always return a testnet URL regardless of the environment setting.
Apply this fix:
if (environment === "mainnet") {
axelarscanTxLink = `https://axelarscan.io/tx/${txHash}`;
+ } else {
+ axelarscanTxLink = `https://testnet.axelarscan.io/tx/${txHash}`;
}
- axelarscanTxLink = `https://testnet.axelarscan.io/tx/${txHash}`;📝 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 (environment === "mainnet") { | |
| axelarscanTxLink = `https://axelarscan.io/tx/${txHash}`; | |
| } | |
| axelarscanTxLink = `https://testnet.axelarscan.io/tx/${txHash}`; | |
| if (environment === "mainnet") { | |
| axelarscanTxLink = `https://axelarscan.io/tx/${txHash}`; | |
| } else { | |
| axelarscanTxLink = `https://testnet.axelarscan.io/tx/${txHash}`; | |
| } |
🤖 Prompt for AI Agents (early access)
In src/common/axelarscan.ts between lines 5 and 8, the assignment of the testnet
URL is outside the if block, causing it to override the mainnet URL
unconditionally. To fix this, place the testnet URL assignment inside an else
block so that the URL is set to mainnet only when environment is "mainnet" and
to testnet otherwise.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
scripts/stellar.ts (1)
114-128: 🛠️ Refactor suggestionAdd comprehensive error handling
The transaction building, signing, and submission lack error handling, which may lead to unhandled exceptions.
-const builtTransaction = new TransactionBuilder(account, { - fee: BASE_FEE, - networkPassphrase, -}) - .addOperation(operation) - .setTimeout(30) - .build(); - -const prepareTransaction = await server.prepareTransaction(builtTransaction); - -prepareTransaction.sign(wallet); - -const txReceipt = await server.sendTransaction(prepareTransaction); - -const tx = await waitForTransaction(server, txReceipt.hash); +try { + const builtTransaction = new TransactionBuilder(account, { + fee: BASE_FEE, + networkPassphrase, + }) + .addOperation(operation) + .setTimeout(30) + .build(); + + console.log("Transaction built successfully"); + + const prepareTransaction = await server.prepareTransaction(builtTransaction); + console.log("Transaction prepared successfully"); + + prepareTransaction.sign(wallet); + console.log("Transaction signed successfully"); + + const txReceipt = await server.sendTransaction(prepareTransaction); + console.log("Transaction submitted successfully"); + + const tx = await waitForTransaction(server, txReceipt.hash); + + console.log("Transaction Hash:", tx.txHash); + console.log("Transaction completed successfully"); + + // the transaction index for ITS transfer is 4. this can be verified by checking the index of "contract_called" event from the block explorer. + console.log("Axelarscan Link:", getAxelarscanLink(`0x${tx.txHash}`, 4)); +} catch (error) { + console.error("Error executing Stellar transaction:", error); + process.exit(1); +}
🧹 Nitpick comments (6)
src/common/squid.ts (2)
8-9: Update JSDoc return type to match actual return typeThe JSDoc comment specifies a return type of
Uint8Array, but the function actually returns astring. Update the comment to reflect the actual return type.* @param destinationAddress The EVM destination address. -* @returns The generated payload as a Uint8Array. +* @returns The generated payload as a string. */
10-19: Enhance robustness with input validation and deterministic testing supportConsider adding input validation for the destination address and an optional parameter for testing purposes.
-export function generateSquidEvmPayload(destinationAddress: string): string { +export function generateSquidEvmPayload(destinationAddress: string, randomValue?: Uint8Array): string { + // Validate destination address format + if (!destinationAddress.match(/^0x[0-9a-fA-F]{40}$/)) { + throw new Error("Invalid EVM address format"); + } + // encode the payload for squid contract const squidPayload = AbiCoder.defaultAbiCoder().encode( ["address", "bytes32"], - [destinationAddress, hexlify(randomBytes(32))], + [destinationAddress, hexlify(randomValue || randomBytes(32))], ); const metadataVersionBytes = hexlify("0x"); return concat([getBytes(metadataVersionBytes), getBytes(squidPayload)]); }scripts/stellar.ts (4)
33-34: Extract magic numbers as named constantsThe script contains several magic numbers (9, 7, 30, etc.) that would benefit from being defined as named constants.
+// Token decimals +const SQD_DECIMALS = 9; +const XLM_DECIMALS = 7; +const TRANSACTION_TIMEOUT_SECONDS = 30; -const UNIT_AMOUNT = parseUnits(process.argv[4] || "1", 9); +const UNIT_AMOUNT = parseUnits(process.argv[4] || "1", SQD_DECIMALS);And update all other occurrences accordingly:
-console.log("SQD Balance:", formatUnits(balance, 9)); +console.log("SQD Balance:", formatUnits(balance, SQD_DECIMALS)); -console.log("Gas Fee:", formatUnits(gasFee, 7), "XLM"); +console.log("Gas Fee:", formatUnits(gasFee, XLM_DECIMALS), "XLM"); -.setTimeout(30) +.setTimeout(TRANSACTION_TIMEOUT_SECONDS)
1-23: Organize imports for better readabilityThe imports could be better organized to group related packages and internal modules.
-import { getChainConfig } from "common/chains"; -import { formatUnits, parseUnits } from "ethers"; -import { getWallet } from "stellar/wallet"; -import { - TransactionBuilder, - Contract, - nativeToScVal, - rpc, - BASE_FEE, - Networks, -} from "@stellar/stellar-sdk"; -import { - addressToScVal, - getBalances, - hexToScVal, - tokenToScVal, - waitForTransaction, -} from "stellar/utils"; -import { calculateEstimatedFee } from "common/gasEstimation"; -import { environment } from "common/env"; -import { getAxelarscanLink } from "common/axelarscan"; -import { Client } from "@stellar/stellar-sdk/minimal/contract"; -import { generateSquidEvmPayload } from "common/squid"; +// External dependencies +import { formatUnits, parseUnits } from "ethers"; +import { + TransactionBuilder, + Contract, + nativeToScVal, + rpc, + BASE_FEE, + Networks, +} from "@stellar/stellar-sdk"; +import { Client } from "@stellar/stellar-sdk/minimal/contract"; + +// Internal utilities - common +import { getChainConfig } from "common/chains"; +import { calculateEstimatedFee } from "common/gasEstimation"; +import { environment } from "common/env"; +import { getAxelarscanLink } from "common/axelarscan"; +import { generateSquidEvmPayload } from "common/squid"; + +// Internal utilities - stellar specific +import { getWallet } from "stellar/wallet"; +import { + addressToScVal, + getBalances, + hexToScVal, + tokenToScVal, + waitForTransaction, +} from "stellar/utils";
73-76: Simplify conditional assignmentThe conditional logic for determining the payload can be simplified for better readability.
-const payload = - destinationChainType === "evm" - ? generateSquidEvmPayload(DESTINATION_ADDRESS) - : undefined; +const payload = destinationChainType === "evm" + ? generateSquidEvmPayload(DESTINATION_ADDRESS) + : undefined;
133-133: Add a summary output at the end of the scriptConsider adding a summary at the end of the script to clearly indicate successful completion.
console.log("Axelarscan Link:", getAxelarscanLink(`0x${tx.txHash}`, 4)); +console.log("\n✅ Interchain transfer completed successfully!"); +console.log(`Summary: +- From: Stellar (${walletAddress}) +- To: ${DESTINATION_CHAIN} (${DESTINATION_ADDRESS}) +- Amount: ${formatUnits(UNIT_AMOUNT, SQD_DECIMALS)} SQD +- Gas Fee: ${formatUnits(gasFee, XLM_DECIMALS)} XLM +- Transaction: ${tx.txHash} +`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/stellar.ts(1 hunks)scripts/xrpl.ts(6 hunks)src/common/squid.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/xrpl.ts
🧰 Additional context used
🪛 Gitleaks (8.21.2)
scripts/stellar.ts
41-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
43-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
scripts/stellar.ts (1)
94-97:Details
❌ Incorrect review comment
Implement dynamic gas token retrieval
The gas token address is still hardcoded, which is better than before with environment-specific values, but should be retrieved dynamically.
Let's verify if there's a way to retrieve the gas token address dynamically:
Based on the results, consider implementing a utility function to retrieve the gas token address dynamically:
-const gasTokenAddress = - environment === "testnet" - ? "CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC" - : "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"; +// Function to get the native token address from the network +async function getNativeTokenAddress(server: rpc.Server): Promise<string> { + try { + // Query for the native asset contract address + // This is a placeholder - implement the actual logic based on Stellar SDK capabilities + const nativeAssetInfo = await server.getAssetInfo("native"); + return nativeAssetInfo.contract_id; + } catch (error) { + console.error("Error fetching native token address:", error); + // Fallback to known addresses if dynamic retrieval fails + return environment === "testnet" + ? "CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC" + : "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"; + } +} + +const gasTokenAddress = await getNativeTokenAddress(server);
🏁 Script executed:
#!/bin/bash # Look for any utility functions or patterns for retrieving token addresses dynamically rg "tokenAddress|gasToken|getNative" --type tsLength of output: 329
🏁 Script executed:
#!/bin/bash # Display the contents of src/stellar/utils.ts to inspect existing utilities sed -n '1,200p' src/stellar/utils.ts || cat src/stellar/utils.tsLength of output: 1921
🌐 Web query:
In @stellar/stellar-sdk, does rpc.Server have a method getNetworkConfig() that returns the native asset (gas token) contract ID?💡 Result:
In the
@stellar/stellar-sdklibrary, therpc.Serverclass does not have a method namedgetNetworkConfig()that returns the native asset (gas token) contract ID. Therpc.Serverclass primarily provides methods for interacting with the Stellar network, such as submitting transactions and retrieving ledger information.The native asset on the Stellar network is the Lumens (XLM), which is the primary currency used for transaction fees and as a base asset in the Stellar ecosystem. If you're looking to retrieve information about the native asset, you can use the
Serverclass to access the network's details. Here's an example of how you can do this:const StellarSdk = require('@stellar/stellar-sdk'); const server = new StellarSdk.Server('https://horizon.stellar.org'); async function getNetworkDetails() { try { const networkDetails = await server.fetchBaseFee(); console.log('Base fee:', networkDetails); } catch (error) { console.error('Error fetching network details:', error); } } getNetworkDetails();In this example, the
fetchBaseFee()method retrieves the current base fee for transactions on the Stellar network, which is denominated in Lumens (XLM). While this doesn't provide the contract ID of the native asset, it gives you information about the native asset's usage in the network.If you're working with Stellar's smart contracts (Soroban), the native asset is also Lumens (XLM). However, as of my knowledge cutoff in June 2024, the
@stellar/stellar-sdkdoes not provide direct methods to interact with Soroban smart contracts. For interacting with Soroban, you would need to use the@stellar/soroban-clientlibrary, which offers functionalities for deploying and interacting with smart contracts on the Stellar network.In summary, while the
rpc.Serverclass in@stellar/stellar-sdkdoesn't have agetNetworkConfig()method to retrieve the native asset's contract ID, you can use thefetchBaseFee()method to get information about the native asset's usage in the network. For interacting with Soroban smart contracts, consider using the@stellar/soroban-clientlibrary.
Gas token address can’t be fetched dynamically with
@stellar/stellar-sdkThe
rpc.Serverclass in@stellar/stellar-sdkdoes not provide a method to query the native asset’s contract ID at runtime. You have two practical options:
- Keep the testnet/mainnet Lumens (XLM) contract IDs in a config or environment file rather than hardcoding them inline.
- If you really need dynamic lookup, integrate the
@stellar/soroban-clientlibrary and query the Soroban network for the native asset contract ID.Until you adopt one of these approaches, retaining the hardcoded addresses is acceptable.
Likely an incorrect or invalid review comment.
| * prefixed with a 4-byte version indicator (currently 0x00000000). | ||
| * @param destinationAddress The EVM destination address. | ||
| * @returns The generated payload as a Uint8Array. | ||
| */ | ||
| export function generateSquidEvmPayload(destinationAddress: string): string { | ||
| // encode the payload for squid contract | ||
| const squidPayload = AbiCoder.defaultAbiCoder().encode( | ||
| ["address", "bytes32"], | ||
| [destinationAddress, hexlify(randomBytes(32))], | ||
| ); | ||
| const metadataVersionBytes = hexlify("0x"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct 4-byte version implementation
The JSDoc comment mentions a "4-byte version indicator", but the implementation uses an empty byte array with hexlify("0x"). This discrepancy should be addressed for consistency.
- const metadataVersionBytes = hexlify("0x");
+ // 4-byte version indicator (0x00000000)
+ const metadataVersionBytes = hexlify("0x00000000");📝 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.
| * prefixed with a 4-byte version indicator (currently 0x00000000). | |
| * @param destinationAddress The EVM destination address. | |
| * @returns The generated payload as a Uint8Array. | |
| */ | |
| export function generateSquidEvmPayload(destinationAddress: string): string { | |
| // encode the payload for squid contract | |
| const squidPayload = AbiCoder.defaultAbiCoder().encode( | |
| ["address", "bytes32"], | |
| [destinationAddress, hexlify(randomBytes(32))], | |
| ); | |
| const metadataVersionBytes = hexlify("0x"); | |
| export function generateSquidEvmPayload(destinationAddress: string): string { | |
| // encode the payload for squid contract | |
| const squidPayload = AbiCoder.defaultAbiCoder().encode( | |
| ["address", "bytes32"], | |
| [destinationAddress, hexlify(randomBytes(32))], | |
| ); | |
| // 4-byte version indicator (0x00000000) | |
| const metadataVersionBytes = hexlify("0x00000000"); |
🤖 Prompt for AI Agents (early access)
In src/common/squid.ts between lines 6 and 16, the code incorrectly uses
hexlify("0x") to represent the 4-byte version indicator, which is inconsistent
with the JSDoc comment stating it should be 4 bytes (0x00000000). Replace
hexlify("0x") with a proper 4-byte zero value, such as hexlify(new
Uint8Array(4)) or an equivalent method to generate four zero bytes, ensuring the
version indicator matches the documented specification.
| contractId: TOKEN_ADDRESS, | ||
| networkPassphrase, | ||
| rpcUrl: stellarChainConfig.config.rpc[0], | ||
| })) as any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid type assertion to 'any'
Using as any bypasses TypeScript's type checking, which could lead to runtime errors.
-})) as any;
+})) as Contract;If the correct type is not Contract, consider creating an interface that accurately describes the expected structure.
🤖 Prompt for AI Agents (early access)
In scripts/stellar.ts at line 64, avoid using the type assertion 'as any'
because it disables TypeScript's type checking. Instead, determine the correct
type for the object being cast—if it is not 'Contract', define a new interface
that accurately represents its structure and use that type assertion. This will
ensure type safety and prevent potential runtime errors.
| ? hexToScVal(payload as string) | ||
| : nativeToScVal(null, { type: "null" }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety when handling optional payload
The current code casts payload to string even when it might be undefined, which could lead to runtime errors.
- payload
- ? hexToScVal(payload as string)
- : nativeToScVal(null, { type: "null" }),
+ payload !== undefined
+ ? hexToScVal(payload)
+ : nativeToScVal(null, { type: "null" }),📝 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.
| ? hexToScVal(payload as string) | |
| : nativeToScVal(null, { type: "null" }), | |
| payload !== undefined | |
| ? hexToScVal(payload) | |
| : nativeToScVal(null, { type: "null" }), |
🤖 Prompt for AI Agents (early access)
In scripts/stellar.ts around lines 109 to 110, the code casts payload to string
without checking if it is defined, risking runtime errors. Modify the code to
first verify that payload is a string before passing it to hexToScVal, ensuring
type safety by handling the case when payload is undefined or not a string
without casting.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/common/gasEstimation.ts (1)
68-69: Add error handling to the fee calculation.The function doesn't handle potential errors from the SDK call.
- return (await sdk.estimateMultihopFee(hopParams)) as string; + try { + return (await sdk.estimateMultihopFee(hopParams)) as string; + } catch (error) { + console.error("Error estimating multihop fee:", error); + throw new Error(`Failed to estimate fee: ${error instanceof Error ? error.message : String(error)}`); + }
🧹 Nitpick comments (5)
src/common/gasEstimation.ts (5)
22-23: Simplify the switch statement by removing unused case.The
"xrpl-evm"case is empty and falls through to the default case, making it redundant.- case "xrpl-evm": // set average gas used for evm chains to 500k for now. this is not accurate but it's good enough for now as we are focusing on amplifier chains here default: // evm chain type fall into this case🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
23-23: Update the comment style for consistency and formality.The current comment is somewhat informal and implies temporary code. Consider rephrasing it to be more professional and explanatory.
- // set average gas used for evm chains to 500k for now. this is not accurate but it's good enough for now as we are focusing on amplifier chains here + // Set average gas limit for EVM chains to 500k. This is an approximation prioritized for amplifier chains.
13-42: RefactorgetGasLimitto reduce code duplication.The function has duplicate logic between testnet and non-testnet environments with only some values being different. Consider refactoring to reduce duplication.
export function getGasLimit(chainConfig: BaseChainConfig): string { - if (environment === "testnet") { - switch (chainConfig.chainType) { - case "sui": - return "2750"; - case "xrpl": - return "165000"; - case "stellar": - return "8000000"; - case "xrpl-evm": - // set average gas used for evm chains to 500k for now. this is not accurate but it's good enough for now as we are focusing on amplifier chains here - default: - // evm chain type fall into this case - if (chainConfig.id.includes("xrpl-evm")) return "3500000"; - else return "500000"; - } - } else { - switch (chainConfig.chainType) { - case "sui": - return "70000"; - case "xrpl": - return "165000"; - case "stellar": - return "8000000"; - default: - if (chainConfig.id.includes("xrpl-evm")) return "3500000"; - else return "500000"; - } - } + const gasLimits: Record<string, Record<string, string>> = { + testnet: { + sui: "2750", + xrpl: "165000", + stellar: "8000000", + }, + mainnet: { + sui: "70000", + xrpl: "165000", + stellar: "8000000", + } + }; + + const networkType = environment === "testnet" ? "testnet" : "mainnet"; + const chainType = chainConfig.chainType; + + // Special case for XRPL-EVM chains + if (chainConfig.id.includes("xrpl-evm")) { + return "3500000"; + } + + // Return the gas limit for the specific chain type if it exists, otherwise use the default EVM gas limit + return gasLimits[networkType][chainType] || "500000"; }🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
59-59: Fix typo in the comment.The word "overrided" should be "overridden".
- // this value will be overrided by the axelarscan api + // this value will be overridden by the axelarscan api
44-69: Consider adding TypeScript documentation comments.Adding JSDoc comments to the exported functions would improve code documentation and provide better IntelliSense for developers using this function.
+/** + * Calculates the estimated fee for a cross-chain transaction. + * + * @param sourceChain - The ID of the source chain + * @param destinationChainConfig - The full configuration of the destination chain + * @returns A Promise that resolves to the estimated fee as a string + */ export async function calculateEstimatedFee( sourceChain: string, destinationChainConfig: BaseChainConfig, ): Promise<string> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/xrpl.ts(6 hunks)src/common/gasEstimation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/xrpl.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/common/gasEstimation.ts (2)
src/common/types.ts (1)
BaseChainConfig(20-28)src/common/env.ts (1)
environment(3-4)
🪛 Biome (1.9.4)
src/common/gasEstimation.ts
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (1)
src/common/gasEstimation.ts (1)
1-69: LGTM overall, with minor recommendations for improvements.The implementation for the gas estimation logic is well-structured and provides clear separation of concerns. It handles different chain types appropriately and integrates well with the Axelar SDK. Just a few small improvements suggested above.
🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Summary by CodeRabbit
New Features
Improvements
Chores
Refactor