feat: tx evm #1188
feat: tx evm #1188
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis pull request introduces comprehensive changes across multiple files, focusing on enhancing EVM (Ethereum Virtual Machine) transaction handling, improving type safety, and refining linting configurations. The modifications span various aspects of the codebase, including transaction simulation, fee estimation, JSON-RPC interactions, and ESLint rule configurations. Key additions include new hooks for EVM transactions, expanded type definitions, and utility functions for EVM-related operations. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts (1)
6-19: Consider enhancing error handling and input validation.While the implementation is functional, consider these improvements:
- Add input validation for the transaction request
- Handle potential errors from
requestEthereumTx- Return loading/error states for better UX
Example enhancement:
export const useSignAndBroadcastEvm = () => { const { walletProvider, chainId } = useCurrentChain(); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<Error | null>(null); return useCallback( async (request: TransactionRequest): Promise<string> => { + setIsLoading(true); + setError(null); + + // Validate request + if (!request.to) { + throw new Error("Transaction must specify 'to' address"); + } + try { if (walletProvider.type === "initia-widget") { const { requestEthereumTx } = walletProvider.context; return requestEthereumTx(request, { chainId }); } throw new Error("Unsupported wallet provider"); + } catch (err) { + setError(err instanceof Error ? err : new Error('Unknown error')); + throw err; + } finally { + setIsLoading(false); } }, [chainId, walletProvider.context, walletProvider.type] ); };src/lib/app-provider/hooks/useSimulateFeeEvm.ts (2)
15-16: Remove redundant comment.The comment "If the user has connected with Initia Widget" is redundant with the if condition below it.
- // If the user has connected with Initia Widget if (walletProvider.type === "initia-widget") {
10-25: Consider enhancing error handling and input validation.Similar to useSignAndBroadcastEvm, consider adding:
- Input validation for the transaction request
- Better error handling with specific error types
- Loading states for better UX
Example enhancement:
+import { useState } from "react"; import { useCallback } from "react"; export const useSimulateFeeEvm = () => { const { chainId, walletProvider } = useCurrentChain(); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<Error | null>(null); return useCallback( async (request: TransactionRequest): Promise<Gas<Big>> => { + setIsLoading(true); + setError(null); + + // Validate request + if (!request.to) { + throw new Error("Transaction must specify 'to' address"); + } + try { if (walletProvider.type === "initia-widget") { const { estimateEthereumTx } = walletProvider.context; return estimateEthereumTx(request, chainId).then(zGas(zBig).parse); } throw new Error("Unsupported wallet provider"); + } catch (err) { + const error = err instanceof Error ? err : new Error('Unknown error'); + setError(error); + throw error; + } finally { + setIsLoading(false); } }, [chainId, walletProvider.context, walletProvider.type] ); };src/lib/services/tx/simulateFeeEvm.ts (1)
19-45: Consider enhancing the query configuration.The hook implementation follows React Query best practices, but consider these improvements:
- Add retry configuration for specific error types.
- Configure
staleTimeto prevent unnecessary refetches.- Add JSDoc documentation for the hook's purpose and usage.
Example implementation:
+/** + * Hook to simulate EVM transaction fees. + * @param params - Query parameters including the transaction request + * @returns Query result containing the simulated gas fees + */ export const useSimulateFeeEvmQuery = ({ enabled, request, retry = 2, extraQueryKey = [], onSuccess, onError, }: SimulateQueryEvmParams) => { const { address } = useCurrentChain(); const simulateFeeEvm = useSimulateFeeEvm(); return useQuery({ queryKey: [ CELATONE_QUERY_KEYS.SIMULATE_FEE_EVM, request, address, ...extraQueryKey, ], queryFn: () => simulateFeeEvm(request), enabled, - retry, + retry: (failureCount, error) => { + // Retry only for network errors + if (error instanceof NetworkError) return failureCount < retry; + return false; + }, refetchOnReconnect: false, refetchOnWindowFocus: false, + staleTime: 30 * 1000, // Consider data fresh for 30 seconds onSuccess, onError, }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.eslintrc.json(1 hunks)CHANGELOG.md(1 hunks)package.json(3 hunks)src/lib/amplitude/Amplitude.ts(1 hunks)src/lib/app-provider/env.ts(1 hunks)src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts(1 hunks)src/lib/app-provider/hooks/useSimulateFeeEvm.ts(1 hunks)src/lib/components/json-schema/UploadTemplate.tsx(1 hunks)src/lib/components/json-schema/form/templates/BaseInputTemplate.tsx(2 hunks)src/lib/components/json-schema/form/templates/button-templates/icon-buttons/ChakraIconButton.tsx(1 hunks)src/lib/pages/account-details/components/AccountTitle.tsx(1 hunks)src/lib/pages/interact-contract/components/execute-area/schema-execute/index.tsx(1 hunks)src/lib/pages/interact-contract/components/query-area/schema-query/index.tsx(1 hunks)src/lib/pages/publish-module/index.tsx(1 hunks)src/lib/services/tx/simulateFee.ts(3 hunks)src/lib/services/tx/simulateFeeEvm.ts(1 hunks)src/lib/types/contractState.ts(1 hunks)src/lib/types/move/abi.ts(2 hunks)src/lib/utils/abi.ts(2 hunks)src/lib/utils/formatter/token.ts(1 hunks)src/lib/utils/json.ts(1 hunks)src/lib/utils/mergeRefs.ts(1 hunks)src/lib/utils/validate.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- src/lib/utils/formatter/token.ts
- src/lib/amplitude/Amplitude.ts
- src/lib/pages/interact-contract/components/query-area/schema-query/index.tsx
- src/lib/utils/abi.ts
- src/lib/components/json-schema/form/templates/button-templates/icon-buttons/ChakraIconButton.tsx
🔇 Additional comments (15)
src/lib/types/contractState.ts (1)
Line range hint
16-20: LGTM! Good schema design.The exported schema provides proper type validation for contract state with well-structured fields.
src/lib/utils/mergeRefs.ts (1)
13-13: LGTM! Improved type safety.Good change to use strict equality check (
!==) instead of loose equality (!=). This prevents potential type coercion issues.src/lib/types/move/abi.ts (1)
6-8: LGTM! Well-structured type definitions.The interfaces are well-defined with clear property types and relationships. The
ModuleAbiinterface provides a comprehensive structure for module information, whileGenericTypeParamandFieldinterfaces are appropriately used withinExposedFunctionandStructinterfaces.Also applies to: 10-12, 33-38
src/lib/pages/publish-module/index.tsx (1)
11-14: LGTM! Better organization of type definitions.Moving the
PublishCompleteStateinterface above its usage inDEFAULT_STATEfollows TypeScript best practices.src/lib/services/tx/simulateFeeEvm.ts (1)
10-17: LGTM! Well-structured query parameters interface.The
SimulateQueryEvmParamsinterface provides a comprehensive set of options for controlling the query behavior.src/lib/utils/json.ts (1)
33-33: LGTM! Simplified error handling.Removing the unused error parameter in the catch block follows ESLint best practices without affecting functionality.
src/lib/pages/account-details/components/AccountTitle.tsx (1)
9-11: LGTM! Well-defined interface for Initia username data.The interface is properly typed and follows TypeScript best practices.
src/lib/services/tx/simulateFee.ts (1)
34-34: LGTM! Improved query key stability.Using
chainIdinstead ofrpcEndpointin query keys is a better practice as it provides a more stable identifier for caching.Also applies to: 48-48
src/lib/app-provider/env.ts (1)
6-6: LGTM! The new query key follows conventions.The addition of
SIMULATE_FEE_EVMis well-placed in the SIMULATE section and follows the established naming pattern for EVM-related keys..eslintrc.json (1)
13-27: Consider the impact of strict ESLint rules.The new rules will enforce better code quality, but some might be too restrictive:
no-consoleas error might hinder debuggingno-param-reassignwithprops: truemight be too strict for reducerseqeqeqas error is good for type safetyLet's check the impact of these rules:
✅ Verification successful
ESLint rules are well-suited for the codebase with minor adjustments
The analysis shows:
no-consoleas error is appropriate since current console usage is intentional and properly handled with eslint-disable comments for error loggingno-param-reassignwithprops: trueis suitable as reducer patterns follow best practiceseqeqeqas error is good for enforcing type safetyRecommendations:
- Consider allowing
console.errorby modifying the rule:"no-console": ["error", { "allow": ["error"] }]- Keep other rules as they currently enforce good practices
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for console usage and parameter reassignment echo "Checking for console usage..." rg "console\." -l echo "Checking for parameter reassignment..." rg "\b(\w+)\s*=\s*" --type ts --type tsxLength of output: 783
Script:
#!/bin/bash # Check console usage context echo "Console usage context:" rg "console\." -A 2 -B 2 # Fix parameter reassignment check for TS files echo -e "\nParameter reassignment in TS files:" rg "\b(\w+)\s*=\s*" --type ts # Look for reducer patterns echo -e "\nReducer patterns:" rg "reduce\(" -A 3 -B 1Length of output: 72485
Script:
#!/bin/bash # Check for reducer patterns and parameter reassignment echo "Checking reducer patterns and accumulator usage:" rg "\.reduce\(" -A 5 -B 2 echo -e "\nChecking parameter reassignment in TS/TSX files:" rg "(\w+)\s*=\s*" --type-add "typescript:*.{ts,tsx}" --type typescriptLength of output: 69311
src/lib/utils/validate.ts (1)
22-24: LGTM! Simplified error handling.Removing unused error parameters from catch blocks improves code cleanliness while maintaining the same functionality.
package.json (2)
75-77: Verify compatibility with updated @initia packages.The updates to @initia packages might have breaking changes:
- @initia/initia.js: 0.2.24 → 0.2.26
- @initia/react-wallet-widget: ^0.191.0 → 0.193.0
106-106: Verify ethers integration.The addition of ethers v6.13.5 is appropriate for EVM functionality. However, ensure it's properly integrated with existing code.
Let's check the usage:
✅ Verification successful
ethers integration verified successfully ✅
The ethers v6.13.5 package is properly integrated, being used exclusively for TypeScript type definitions (TransactionRequest) in EVM-related functionality across hooks and services.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check ethers import usage echo "Checking ethers usage..." rg "from ['\"]ethers['\"]" --type ts --type tsxLength of output: 140
Script:
#!/bin/bash # Comprehensive check for ethers usage echo "Checking for ethers imports and usage..." rg "(?i)(?:import.*from.*['\"]ethers['\"]|require.*['\"]ethers['\"]|import.*['\"]ethers['\"]|from\s+['\"]ethers['\"])" echo -e "\nChecking for ethers. usage..." rg "ethers\." -A 2 echo -e "\nChecking for ethers configuration..." rg "ethers" package.jsonLength of output: 736
Script:
#!/bin/bash # Check implementation of files using TransactionRequest echo "Checking useSimulateFeeEvm.ts implementation..." cat src/lib/app-provider/hooks/useSimulateFeeEvm.ts echo -e "\nChecking useSignAndBroadcastEvm.ts implementation..." cat src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts echo -e "\nChecking simulateFeeEvm.ts implementation..." cat src/lib/services/tx/simulateFeeEvm.tsLength of output: 3130
src/lib/components/json-schema/form/templates/BaseInputTemplate.tsx (1)
154-154: LGTM! Good improvement in type safety.The change from
==to===for null comparison is a good practice as it prevents potential type coercion issues and makes the code more predictable.CHANGELOG.md (1)
42-42: LGTM! Well-formatted changelog entry.The new entry follows the changelog guidelines with proper PR reference and clear description.
| } catch { | ||
| // | ||
| } |
There was a problem hiding this comment.
Add error handling in catch block.
Empty catch blocks are an anti-pattern as they silently swallow errors, making debugging difficult. Consider logging the error or showing a user-friendly message.
Apply this diff to improve error handling:
- } catch {
- //
+ } catch (error) {
+ console.error('Failed to parse initial message:', error);
+ // Optionally show a user-friendly message
}📝 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.
| } catch { | |
| // | |
| } | |
| } catch (error) { | |
| console.error('Failed to parse initial message:', error); | |
| // Optionally show a user-friendly message | |
| } |
| } catch { | ||
| // | ||
| } |
There was a problem hiding this comment.
Add error handling for file reading failures.
Empty catch blocks hide file reading errors from users, providing no feedback when uploads fail. Consider showing an error message to help users understand and resolve the issue.
Apply this diff to improve error handling:
- } catch {
- //
+ } catch (error) {
+ console.error('Failed to read file:', error);
+ dispatch({
+ type: ActionType.SET_ERROR,
+ method,
+ error: 'Failed to read file. Please try again.',
+ });
}📝 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.
| } catch { | |
| // | |
| } | |
| } catch (error) { | |
| console.error('Failed to read file:', error); | |
| dispatch({ | |
| type: ActionType.SET_ERROR, | |
| method, | |
| error: 'Failed to read file. Please try again.', | |
| }); | |
| } |
| export interface InitiaUsernameDataResponse { | ||
| username: string | null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/app-provider/hooks/index.ts(1 hunks)src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts(1 hunks)src/lib/app-provider/hooks/useSimulateFeeEvm.ts(1 hunks)src/lib/pages/account-details/components/AccountTitle.tsx(1 hunks)src/lib/services/tx/simulateFeeEvm.ts(1 hunks)src/lib/types/big.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/services/tx/simulateFeeEvm.ts
🔇 Additional comments (4)
src/lib/types/big.ts (1)
8-8: LGTM! Good addition of bigint support.The addition of
z.bigint()with string transformation is a good practice for handling EVM transaction values which commonly use bigint. The transformation ensures type safety while maintaining compatibility with Big.js.src/lib/app-provider/hooks/index.ts (1)
22-24: LGTM! Exports are properly organized.The new EVM-related hook exports maintain the established pattern and ordering.
src/lib/pages/account-details/components/AccountTitle.tsx (2)
7-7: LGTM! Clean import statement update.The addition of the
Nullabletype import is consistent with its usage in the new interface.
9-11: Well-structured interface definition!The
InitiaUsernameDataResponseinterface is well-defined and properly integrated with the component's logic. The use ofNullable<string>is handled correctly throughout the component with appropriate null checks.
| } catch { | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling.
Returning an empty string on error silently masks issues. Consider:
- Propagating the error with meaningful context
- Logging the error for debugging
- Returning a Result/Either type to handle success/failure cases explicitly
| if (walletProvider.type === "initia-widget") { | ||
| const { requestEthereumTx } = walletProvider.context; | ||
| const result = await requestEthereumTx(request, { chainId }); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type narrowing for wallet provider context.
The code assumes requestEthereumTx exists on the context without type checking. Consider adding a type guard:
if (walletProvider.type === "initia-widget") {
+ if (!('requestEthereumTx' in walletProvider.context)) {
+ throw new Error("Wallet provider context missing requestEthereumTx");
+ }
const { requestEthereumTx } = walletProvider.context;
const result = await requestEthereumTx(request, { chainId });
return result;📝 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 (walletProvider.type === "initia-widget") { | |
| const { requestEthereumTx } = walletProvider.context; | |
| const result = await requestEthereumTx(request, { chainId }); | |
| return result; | |
| } | |
| if (walletProvider.type === "initia-widget") { | |
| if (!('requestEthereumTx' in walletProvider.context)) { | |
| throw new Error("Wallet provider context missing requestEthereumTx"); | |
| } | |
| const { requestEthereumTx } = walletProvider.context; | |
| const result = await requestEthereumTx(request, { chainId }); | |
| return result; | |
| } |
| } catch { | ||
| return zGas(zBig).parse(0); | ||
| } |
There was a problem hiding this comment.
Improve error handling for gas estimation.
Returning 0 for gas on error is dangerous as it could lead to failed transactions. Consider:
- Propagating the error with context
- Implementing a fallback estimation strategy
- Using a Result/Either type to handle success/failure explicitly
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
src/lib/components/table/evm-transactions/EvmTransactionsTable.tsx (1)
54-55: Consider adding type safety for evmDenom.The
evmDenomprop is passed as an optional value (evmParams?.params.feeDenom). Consider adding runtime validation or TypeScript type guards to handle cases whereevmDenommight be undefined.+ const evmDenom = evmParams?.params.feeDenom; + if (!evmDenom) { + console.warn('EVM denom is not available'); + return <Loading />; + }Also applies to: 71-72
src/lib/services/types/evm/params.ts (1)
4-14: Add JSDoc comments to document the schema fields.While the schema is well-structured, adding documentation for each field would improve maintainability and help other developers understand the purpose of each parameter.
Example documentation:
/** * Zod schema for EVM parameters * @property params.extra_eips - List of additional EIPs to enable * @property params.allowed_publishers - List of authorized publishing addresses * @property params.allow_custom_erc20 - Flag to enable custom ERC20 tokens * @property params.allowed_custom_erc20s - List of allowed custom ERC20 token addresses * @property params.fee_denom - Denomination for transaction fees */ export const zEvmParams = z .object({ // ... rest of the code })src/lib/app-provider/tx/evm/requestEvm.ts (2)
28-28: Add type narrowing for estimatedFee check.The current check
if (!estimatedFee)could be improved with explicit type narrowing.- if (!estimatedFee) return null; + if (estimatedFee === undefined) return null;
35-38: Consider adding error tracking.Success tracking is implemented, but error tracking is missing. Consider adding
trackTxFailedfor consistency.onTxSucceed: () => { trackTxSucceed(); onTxSucceed?.(); }, - onTxFailed, + onTxFailed: () => { + trackTxFailed(); + onTxFailed?.(); + },src/lib/components/EstimatedEvmFeeRender.tsx (3)
8-16: Define explicit interface for component props.Extract props into a named interface for better maintainability and documentation.
+interface EstimatedEvmFeeRenderProps { + effectiveGasPrice: Big; + gasUsed: Big; + loading: boolean; +} + -export const EstimatedEvmFeeRender = ({ - effectiveGasPrice, - gasUsed, - loading, -}: { - effectiveGasPrice: Big; - gasUsed: Big; - loading: boolean; -}) => { +export const EstimatedEvmFeeRender = ({ + effectiveGasPrice, + gasUsed, + loading, +}: EstimatedEvmFeeRenderProps) => {
22-29: Extract loading states into a reusable component.Consider consolidating the loading indicators into a separate component for better reusability and consistency.
+const LoadingIndicator = ({ text }: { text?: string }) => ( + <> + <Spinner size="sm" mx={1} /> + {text} + </> +); + if (isAssetInfoLoading || isEvmParamsLoading) - return <Spinner size="sm" mx={1} />; + return <LoadingIndicator />; if (loading) - return ( - <> - <Spinner size="sm" mx={1} /> Estimating ... - </> - ); + return <LoadingIndicator text="Estimating ..." />;
34-40: Consider wrapping calculations in error boundary.The fee calculations could potentially throw errors (e.g., from Big.js operations). Consider adding error handling.
+ try { const feeAmount = effectiveGasPrice.mul(gasUsed); const feeToken = coinToTokenWithValue( feeDenom, feeAmount.toFixed(), assetInfos ); return <>{formatTokenWithValue(feeToken)}</>; + } catch (error) { + console.error('Error calculating fee:', error); + return <>Error calculating fee</>; + }src/lib/app-fns/tx/common/postEvm.tsx (1)
32-32: Address the TODO comment about event/rawlog attribute picker.The TODO comment suggests missing functionality for event/rawlog attribute selection.
Would you like me to help implement the event/rawlog attribute picker or create a GitHub issue to track this task?
src/lib/app-fns/tx/common/post.tsx (1)
21-23: Consider adding JSDoc documentation.Adding documentation would help explain the purpose and usage of the generic type parameter.
+/** + * Creates a transaction posting function that handles DeliverTxResponse results. + * @template T Type extending DeliverTxResponse + * @param postFn Function that posts the transaction and returns a Promise<T> + * @returns Function that processes and formats the transaction result + */ export const postTx = <T extends DeliverTxResponse>({ postFn, }: PostTxParams<T>) => {src/lib/services/tx/jsonRpc.ts (1)
88-93: Add input validation and documentation.The
getEthCallfunction would benefit from input validation and JSDoc documentation.+/** + * Makes an eth_call JSON-RPC request. + * @param endpoint The JSON-RPC endpoint URL + * @param from The sender address (optional) + * @param to The target contract address + * @param data The call data + * @returns Promise resolving to the call result + */ export const getEthCall = ( endpoint: string, from: Nullable<HexAddr>, to: HexAddr, data: string -) => requestJsonRpc(endpoint, "eth_call", [{ from, to, data }, "latest"]); +) => { + if (!endpoint) throw new Error("Endpoint is required"); + if (!to) throw new Error("Target address is required"); + if (!data) throw new Error("Call data is required"); + return requestJsonRpc(endpoint, "eth_call", [{ from, to, data }, "latest"]); +};src/lib/app-fns/tx/evm/requestEvm.tsx (1)
24-78: Consider adding transaction timeout handling.The function effectively handles transaction success and error cases. However, it might benefit from having a timeout mechanism for long-running transactions.
Consider adding a timeout parameter and implementing timeout handling:
interface RequestEvmTxParams { to: HexAddr; data: string; fee: StdFee; signAndBroadcastEvm: SignAndBroadcastEvm; onTxSucceed?: () => void; onTxFailed?: () => void; + timeoutMs?: number; } export const requestEvmTx = ({ to, data, fee, signAndBroadcastEvm, onTxSucceed, onTxFailed, + timeoutMs = 30000, }: RequestEvmTxParams): Observable<TxResultRendering> => { return pipe( sendingTx(fee), postEvmTx<TxReceiptJsonRpc>({ postFn: () => signAndBroadcastEvm({ to, data }), + timeoutMs, }),src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts (1)
10-50: Consider making timeout values configurable.The timeout values are hardcoded. Consider making them configurable through parameters or environment variables for flexibility.
+interface EvmTxResponseConfig { + timeoutMs?: number; + pollIntervalMs?: number; +} + -const getEvmTxResponse = async (jsonRpcEndpoint: string, txHash: string) => { +const getEvmTxResponse = async ( + jsonRpcEndpoint: string, + txHash: string, + config?: EvmTxResponseConfig +) => { - const TIME_OUT_MS = 3000; - const POLL_INTERVAL_MS = 1000; + const TIME_OUT_MS = config?.timeoutMs ?? 3000; + const POLL_INTERVAL_MS = config?.pollIntervalMs ?? 1000;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/lib/app-fns/tx/common/index.ts(1 hunks)src/lib/app-fns/tx/common/post.tsx(3 hunks)src/lib/app-fns/tx/common/postEvm.tsx(1 hunks)src/lib/app-fns/tx/evm/requestEvm.tsx(1 hunks)src/lib/app-provider/env.ts(2 hunks)src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts(1 hunks)src/lib/app-provider/tx/evm/requestEvm.ts(1 hunks)src/lib/components/EstimatedEvmFeeRender.tsx(1 hunks)src/lib/components/table/evm-transactions/EvmTransactionsTable.tsx(2 hunks)src/lib/pages/evm-tx-details/data.ts(1 hunks)src/lib/services/evm/jsonRpc.ts(1 hunks)src/lib/services/tx/index.ts(3 hunks)src/lib/services/tx/jsonRpc.ts(2 hunks)src/lib/services/types/evm/params.ts(1 hunks)src/lib/services/types/tx.ts(1 hunks)src/lib/utils/evm.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/lib/pages/evm-tx-details/data.ts
🔇 Additional comments (14)
src/lib/components/table/evm-transactions/EvmTransactionsTable.tsx (1)
54-54: LGTM! Property naming convention update.The change from
fee_denomtofeeDenomaligns with JavaScript/TypeScript camelCase naming conventions.Also applies to: 71-71
src/lib/app-fns/tx/common/index.ts (1)
3-3: LGTM!The new export follows the established pattern and maintains consistency with other exports.
src/lib/app-fns/tx/common/postEvm.tsx (1)
7-9: LGTM! Well-structured interface definition.The
PostEvmTxParamsinterface with generic type constraint is well-defined and provides good type safety.src/lib/services/evm/jsonRpc.ts (1)
8-8: LGTM! Type expansion improves flexibility.Adding
objecttoJsonRpcParamstype is appropriate for EVM transactions that require structured parameters.Let's verify that the expanded type is used correctly:
✅ Verification successful
Type expansion is correctly used throughout the codebase
The expanded
JsonRpcParamstype accommodates all current usage patterns, including string parameters for cosmos transactions and object parameters for ETH calls.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for JsonRpcParams usage to ensure compatibility rg -l "JsonRpcParams" | xargs rg "JsonRpcParams.*=.*" -A 2Length of output: 242
Script:
#!/bin/bash # Get complete implementation context ast-grep --pattern 'export const requestJsonRpc = ($$$) { $$$ }' # Find all usages of requestJsonRpc rg "requestJsonRpc\(" -A 2Length of output: 954
src/lib/app-fns/tx/common/post.tsx (1)
10-12: LGTM! Type constraint simplification.Simplifying to
DeliverTxResponsemakes the code more focused and maintainable.src/lib/app-fns/tx/evm/requestEvm.tsx (1)
15-22: LGTM! Well-structured interface definition.The interface clearly defines the required parameters and optional callbacks for transaction handling.
src/lib/services/types/tx.ts (1)
469-469: LGTM! Clean type export.The type is correctly inferred from the Zod schema and properly exported.
src/lib/services/tx/index.ts (1)
783-808: LGTM! Well-implemented hook with proper error handling and caching.The hook effectively:
- Handles EVM configuration
- Includes proper error handling
- Uses appropriate query caching
- Maintains type safety
src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts (2)
52-54: LGTM! Clean type definition.The type is well-defined and properly exported.
56-82: Add type narrowing for wallet provider context.The code assumes
requestEthereumTxexists on the context without type checking.if (evm.enabled && walletProvider.type === "initia-widget") { + if (!('requestEthereumTx' in walletProvider.context)) { + throw new Error("Wallet provider context missing requestEthereumTx"); + } const { requestEthereumTx } = walletProvider.context; const txHash = await requestEthereumTx(request, { chainId }); return getEvmTxResponse(evm.jsonRpc, txHash); }src/lib/app-provider/env.ts (2)
6-6: LGTM! Well-organized simulation key addition.The new
SIMULATE_FEE_EVMkey follows the existing naming pattern and is correctly placed in the simulation section.
147-147: LGTM! Properly placed EVM query key.The new
EVM_ETH_CALLkey is logically placed in the EVM section and follows the established naming convention.src/lib/utils/evm.ts (2)
7-7: LGTM!The import statement follows the existing pattern and imports the necessary types from the ethers library.
95-101: Verify integration with EVM transaction hooks.Let's verify the usage of this function in the codebase:
✅ Verification successful
Proper integration with EVM transaction flow verified
The
encodeFunctionDatautility is correctly integrated into the EVM transaction flow:
- Used by
requestEvmTxfor transaction preparation- Properly interfaces with ethers.js for ABI encoding
- Follows the standard EVM transaction pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of encodeFunctionData echo "Searching for encodeFunctionData usage..." rg "encodeFunctionData" --type ts # Check the implementation of related hooks echo -e "\nChecking related hook implementations..." rg "useSignAndBroadcastEvm|useSimulateFeeEvm" --type ts # Look for similar encoding patterns echo -e "\nChecking for similar encoding patterns..." ast-grep --pattern 'new Interface($$$)'Length of output: 1713
Script:
#!/bin/bash # Check hook implementations echo "Content of useSignAndBroadcastEvm.ts:" cat src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts echo -e "\nContent of useSimulateFeeEvm.ts:" cat src/lib/app-provider/hooks/useSimulateFeeEvm.ts # Look for transaction preparation functions echo -e "\nSearching for transaction preparation patterns:" rg "prepareTx|prepareTransaction" --type ts -A 5Length of output: 3932
Script:
#!/bin/bash # Check where hooks are used and how transactions are prepared echo "Searching for TransactionRequest usage:" rg "TransactionRequest" --type ts -B 2 -A 2 echo -e "\nChecking files that use the hooks:" rg "useSignAndBroadcastEvm|useSimulateFeeEvm" --type ts -B 2 -A 2 echo -e "\nLooking for transaction creation patterns:" ast-grep --pattern 'const $_ = { to: $_, data: $_ }'Length of output: 7182
Script:
#!/bin/bash # Check transaction preparation implementation echo "Content of requestEvmTx implementation:" cat src/lib/app-fns/tx/evm/requestEvm.ts # Look for any other files that might be preparing transactions echo -e "\nSearching for files with 'evm' and 'tx' in path:" fd -t f -p ".*evm.*tx.*" -x cat {}Length of output: 54308
Script:
#!/bin/bash # Search for files in app-fns directory echo "Checking app-fns directory:" fd -t f . src/lib/app-fns -x cat {} # Search for transaction preparation patterns echo -e "\nSearching for transaction preparation and encoding patterns:" rg "prepareTx|prepareTransaction|encodeFunctionData" --type ts -B 2 -A 5Length of output: 37101
| export const postEvmTx = <T extends TxReceiptJsonRpc>({ | ||
| postFn, | ||
| }: PostEvmTxParams<T>) => { | ||
| return () => | ||
| postFn().then((txResult) => { | ||
| return { | ||
| value: txResult, | ||
| phase: TxStreamPhase.BROADCAST, | ||
| receipts: [ | ||
| { | ||
| title: "Tx Hash", | ||
| html: ( | ||
| <ExplorerLink | ||
| type="tx_hash" | ||
| value={txResult.transactionHash} | ||
| openNewTab | ||
| /> | ||
| ), | ||
| }, | ||
| { | ||
| title: "Tx Fee", | ||
| // TODO: Implement event/rawlog attribute picker | ||
| html: ( | ||
| <EstimatedEvmFeeRender | ||
| effectiveGasPrice={txResult.effectiveGasPrice} | ||
| gasUsed={txResult.gasUsed} | ||
| loading={false} | ||
| /> | ||
| ), | ||
| }, | ||
| ], | ||
| receiptInfo: { | ||
| header: "Sending Transaction", | ||
| }, | ||
| actionVariant: "sending", | ||
| } as TxResultRendering<T>; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for failed transactions.
The current implementation doesn't handle transaction failures explicitly. Consider adding error handling similar to how postTx handles isDeliverTxFailure.
Here's a suggested implementation:
export const postEvmTx = <T extends TxReceiptJsonRpc>({
postFn,
}: PostEvmTxParams<T>) => {
return () =>
postFn().then((txResult) => {
+ // Check for transaction failure
+ if (txResult.status === '0x0') {
+ throw new Error(`Transaction failed: ${txResult.transactionHash}`);
+ }
return {
value: txResult,
phase: TxStreamPhase.BROADCAST,📝 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 postEvmTx = <T extends TxReceiptJsonRpc>({ | |
| postFn, | |
| }: PostEvmTxParams<T>) => { | |
| return () => | |
| postFn().then((txResult) => { | |
| return { | |
| value: txResult, | |
| phase: TxStreamPhase.BROADCAST, | |
| receipts: [ | |
| { | |
| title: "Tx Hash", | |
| html: ( | |
| <ExplorerLink | |
| type="tx_hash" | |
| value={txResult.transactionHash} | |
| openNewTab | |
| /> | |
| ), | |
| }, | |
| { | |
| title: "Tx Fee", | |
| // TODO: Implement event/rawlog attribute picker | |
| html: ( | |
| <EstimatedEvmFeeRender | |
| effectiveGasPrice={txResult.effectiveGasPrice} | |
| gasUsed={txResult.gasUsed} | |
| loading={false} | |
| /> | |
| ), | |
| }, | |
| ], | |
| receiptInfo: { | |
| header: "Sending Transaction", | |
| }, | |
| actionVariant: "sending", | |
| } as TxResultRendering<T>; | |
| }); | |
| }; | |
| export const postEvmTx = <T extends TxReceiptJsonRpc>({ | |
| postFn, | |
| }: PostEvmTxParams<T>) => { | |
| return () => | |
| postFn().then((txResult) => { | |
| // Check for transaction failure | |
| if (txResult.status === '0x0') { | |
| throw new Error(`Transaction failed: ${txResult.transactionHash}`); | |
| } | |
| return { | |
| value: txResult, | |
| phase: TxStreamPhase.BROADCAST, | |
| receipts: [ | |
| { | |
| title: "Tx Hash", | |
| html: ( | |
| <ExplorerLink | |
| type="tx_hash" | |
| value={txResult.transactionHash} | |
| openNewTab | |
| /> | |
| ), | |
| }, | |
| { | |
| title: "Tx Fee", | |
| // TODO: Implement event/rawlog attribute picker | |
| html: ( | |
| <EstimatedEvmFeeRender | |
| effectiveGasPrice={txResult.effectiveGasPrice} | |
| gasUsed={txResult.gasUsed} | |
| loading={false} | |
| /> | |
| ), | |
| }, | |
| ], | |
| receiptInfo: { | |
| header: "Sending Transaction", | |
| }, | |
| actionVariant: "sending", | |
| } as TxResultRendering<T>; | |
| }); | |
| }; |
src/lib/utils/evm.ts
Outdated
| export const encodeFunctionData = ( | ||
| abiSection: JsonFragment, | ||
| values: unknown[] | ||
| ) => { | ||
| const iface = new Interface([abiSection]); | ||
| return iface.encodeFunctionData(abiSection.name ?? "", values); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety, error handling, and documentation.
The function implementation could be improved in several areas:
+/**
+ * Encodes function data using ethers Interface for EVM transactions
+ * @param abiSection - The ABI fragment containing function details
+ * @param values - Array of parameter values matching the function signature
+ * @returns Encoded function data as a hex string
+ * @throws {Error} If ABI is invalid or encoding fails
+ */
export const encodeFunctionData = (
abiSection: JsonFragment,
- values: unknown[]
+ values: Array<string | number | boolean>
) => {
+ if (!abiSection.name || !abiSection.type || abiSection.type !== "function") {
+ throw new Error("Invalid ABI: missing function name or type");
+ }
+
+ try {
const iface = new Interface([abiSection]);
- return iface.encodeFunctionData(abiSection.name ?? "", values);
+ return iface.encodeFunctionData(abiSection.name, values);
+ } catch (error) {
+ throw new Error(`Failed to encode function data: ${(error as Error).message}`);
+ }
};Changes:
- Added JSDoc documentation
- Improved type safety by replacing
unknown[]with specific types - Added validation for required ABI properties
- Added error handling with descriptive messages
📝 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 encodeFunctionData = ( | |
| abiSection: JsonFragment, | |
| values: unknown[] | |
| ) => { | |
| const iface = new Interface([abiSection]); | |
| return iface.encodeFunctionData(abiSection.name ?? "", values); | |
| }; | |
| /** | |
| * Encodes function data using ethers Interface for EVM transactions | |
| * @param abiSection - The ABI fragment containing function details | |
| * @param values - Array of parameter values matching the function signature | |
| * @returns Encoded function data as a hex string | |
| * @throws {Error} If ABI is invalid or encoding fails | |
| */ | |
| export const encodeFunctionData = ( | |
| abiSection: JsonFragment, | |
| values: Array<string | number | boolean> | |
| ) => { | |
| if (!abiSection.name || !abiSection.type || abiSection.type !== "function") { | |
| throw new Error("Invalid ABI: missing function name or type"); | |
| } | |
| try { | |
| const iface = new Interface([abiSection]); | |
| return iface.encodeFunctionData(abiSection.name, values); | |
| } catch (error) { | |
| throw new Error(`Failed to encode function data: ${(error as Error).message}`); | |
| } | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/lib/app-fns/tx/common/postEvm.tsx (1)
14-47:⚠️ Potential issueAdd error handling for failed transactions.
The current implementation doesn't handle transaction failures.
return () => postFn().then((txResult) => { + if (txResult.status === '0x0') { + throw new Error(`Transaction failed: ${txResult.transactionHash}`); + } return { value: txResult, phase: TxStreamPhase.BROADCAST,src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts (1)
70-83:⚠️ Potential issueAdd type narrowing for wallet provider context.
The code assumes
requestEthereumTxexists on the context without type checking.if (evm.enabled && walletProvider.type === "initia-widget") { + if (!('requestEthereumTx' in walletProvider.context) || !('ethereum' in walletProvider.context)) { + throw new Error("Wallet provider context missing required methods"); + } const { requestEthereumTx, ethereum } = walletProvider.context;
🧹 Nitpick comments (3)
src/lib/app-fns/tx/common/sendingEvm.tsx (2)
12-13: Remove unnecessary eslint-disable comment and improve type safety.The eslint-disable comment and void parameter can be removed since they're not needed.
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - return (_: void) => { + return () => {🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
8-36: Consider memoizing the return function.Since this function returns another function that creates an object with React components, consider memoizing it to prevent unnecessary re-renders.
+import { useMemo } from "react"; -export const sendingEvmTx = ({ +export const sendingEvmTx = useMemo(({ gasPrice, simulatedGasUsed, }: SimulatedFeeEvm) => { return () => { return { // ... existing implementation } as TxResultRendering; }; -}; +}, [gasPrice, simulatedGasUsed]);🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/lib/components/EstimatedFeeEvmRender.tsx (1)
8-41: Optimize component performance with useMemo.The component performs calculations on every render. Consider memoizing the fee calculation.
+import { useMemo } from "react"; export const EstimatedFeeEvmRender = ({ gasPrice, gasUsed, loading, }: { gasPrice: Big; gasUsed: Big; loading: boolean; }) => { const { data: assetInfos, isLoading: isAssetInfoLoading } = useAssetInfos({ withPrices: false, }); const { data: evmParams, isLoading: isEvmParamsLoading } = useEvmParams(); + const feeToken = useMemo(() => { + if (!evmParams?.params.feeDenom || !assetInfos) return null; + try { + const feeAmount = gasPrice.mul(gasUsed); + return coinToTokenWithValue( + evmParams.params.feeDenom, + feeAmount.toFixed(), + assetInfos + ); + } catch (error) { + console.error('Error calculating fee:', error); + return null; + } + }, [gasPrice, gasUsed, evmParams?.params.feeDenom, assetInfos]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/lib/app-fns/tx/common/postEvm.tsx(1 hunks)src/lib/app-fns/tx/common/sendingEvm.tsx(1 hunks)src/lib/app-fns/tx/evm/requestEvm.tsx(1 hunks)src/lib/app-provider/hooks/index.ts(1 hunks)src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts(1 hunks)src/lib/app-provider/tx/evm/requestEvm.ts(1 hunks)src/lib/components/EstimatedFeeEvmRender.tsx(1 hunks)src/lib/components/token/TokenImageWithAmount.tsx(2 hunks)src/lib/services/evm/index.ts(2 hunks)src/lib/services/evm/jsonRpc.ts(3 hunks)src/lib/services/tx/index.ts(2 hunks)src/lib/services/tx/jsonRpc.ts(2 hunks)src/lib/services/tx/simulateFeeEvm.ts(1 hunks)src/lib/services/types/tx.ts(4 hunks)src/lib/utils/evm.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/app-provider/hooks/index.ts
- src/lib/services/tx/index.ts
- src/lib/services/types/tx.ts
- src/lib/app-fns/tx/evm/requestEvm.tsx
- src/lib/services/tx/simulateFeeEvm.ts
- src/lib/services/tx/jsonRpc.ts
- src/lib/app-provider/tx/evm/requestEvm.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/app-fns/tx/common/sendingEvm.tsx
[error] 13-13: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (8)
src/lib/components/token/TokenImageWithAmount.tsx (2)
4-4: LGTM!The import statement is correctly placed and properly aligned with the component's dependencies.
20-20: Verify the token formatting behavior across the codebase.The refactoring from using two separate functions to a single
formatTokenWithValuefunction looks good and simplifies the implementation. However, let's verify that this change maintains consistent behavior across all token formatting use cases.Let's check other usages of token formatting in the codebase:
✅ Verification successful
Token formatting behavior is consistent across the codebase
The refactoring maintains backward compatibility while providing a more convenient combined function. The implementation is well-tested and shows consistent usage patterns across components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usages of the old formatting functions and verify consistent usage of the new function # Check for any remaining usage of old functions echo "Checking for remaining usage of formatUTokenWithPrecision..." rg "formatUTokenWithPrecision" echo "Checking for remaining usage of getTokenLabel..." rg "getTokenLabel" # Check usage patterns of new function echo "Checking usage patterns of formatTokenWithValue..." rg "formatTokenWithValue" -A 2Length of output: 27914
src/lib/app-fns/tx/common/postEvm.tsx (1)
32-32: Address TODO comment about event/rawlog attribute picker.The TODO comment indicates missing functionality for event/rawlog attribute picking.
Would you like me to help implement the event/rawlog attribute picker or create a GitHub issue to track this task?
src/lib/utils/evm.ts (2)
105-111: Previous review comments still apply.The suggested improvements from the previous review for type safety, error handling, and documentation have not been addressed.
113-119: 🛠️ Refactor suggestionAdd type safety, error handling, and documentation.
The function implementation needs improvements:
+/** + * Decodes function result data using ethers Interface for EVM transactions + * @param abiSection - The ABI fragment containing function details + * @param data - The hex string data to decode + * @returns Decoded function result + * @throws {Error} If ABI is invalid or decoding fails + */ export const decodeEvmFunctionResult = ( abiSection: JsonFragment, data: string -) => { - const iface = new Interface([abiSection]); - return iface.decodeFunctionResult(abiSection.name ?? "", data); +): unknown => { + if (!abiSection.name || !abiSection.type || abiSection.type !== "function") { + throw new Error("Invalid ABI: missing function name or type"); + } + + try { + const iface = new Interface([abiSection]); + return iface.decodeFunctionResult(abiSection.name, data); + } catch (error) { + throw new Error(`Failed to decode function result: ${(error as Error).message}`); + } };Likely invalid or redundant comment.
src/lib/services/evm/jsonRpc.ts (2)
37-37: Ensure changing the return value ofrequestJsonRpcdoes not introduce breaking changesBy returning
parsed.resultinstead of the entireparsedobject, this change may affect functions that rely on the previous return structure ofrequestJsonRpc. Please verify that all consumers of this function handle the new return value correctly.
76-84:getEthCallfunction implementation is correctThe
getEthCallfunction correctly constructs aneth_callJSON-RPC request and properly parses the response usingparseWithError(zHex, result).src/lib/services/evm/index.ts (1)
74-100:useEthCallhook is properly implementedThe
useEthCallhook appropriately sets up a query for making Ethereum calls, handling EVM configuration and user address conversion correctly.
| const feeAmount = gasPrice.mul(gasUsed); | ||
| const feeToken = coinToTokenWithValue( | ||
| feeDenom, | ||
| feeAmount.toFixed(), | ||
| assetInfos | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for invalid fee calculations.
The multiplication of gasPrice and gasUsed could throw if the values are invalid. Consider adding try-catch.
- const feeAmount = gasPrice.mul(gasUsed);
- const feeToken = coinToTokenWithValue(
- feeDenom,
- feeAmount.toFixed(),
- assetInfos
- );
+ try {
+ const feeAmount = gasPrice.mul(gasUsed);
+ const feeToken = coinToTokenWithValue(
+ feeDenom,
+ feeAmount.toFixed(),
+ assetInfos
+ );
+ return <>{formatTokenWithValue(feeToken, undefined, false)}</>;
+ } catch (error) {
+ console.error('Error calculating fee:', error);
+ return <>Error calculating fee</>;
+ }
- return <>{formatTokenWithValue(feeToken, undefined, false)}</>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const feeAmount = gasPrice.mul(gasUsed); | |
| const feeToken = coinToTokenWithValue( | |
| feeDenom, | |
| feeAmount.toFixed(), | |
| assetInfos | |
| ); | |
| try { | |
| const feeAmount = gasPrice.mul(gasUsed); | |
| const feeToken = coinToTokenWithValue( | |
| feeDenom, | |
| feeAmount.toFixed(), | |
| assetInfos | |
| ); | |
| return <>{formatTokenWithValue(feeToken, undefined, false)}</>; | |
| } catch (error) { | |
| console.error('Error calculating fee:', error); | |
| return <>Error calculating fee</>; | |
| } |
| const getEvmTxResponse = async (jsonRpcEndpoint: string, txHash: string) => { | ||
| const TIME_OUT_MS = 3000; | ||
| const POLL_INTERVAL_MS = 1000; | ||
|
|
||
| let timedOut = false; | ||
| const txPollTimeout = setTimeout(() => { | ||
| timedOut = true; | ||
| }, TIME_OUT_MS); | ||
|
|
||
| const pollForEvmTx = async (_txHash: string): Promise<TxReceiptJsonRpc> => { | ||
| if (timedOut) { | ||
| throw new Error( | ||
| `Transaction with hash ${_txHash} was submitted but was not yet found on the chain. You might want to check later. There was a wait of ${ | ||
| TIME_OUT_MS / 1000 | ||
| } seconds.` | ||
| ); | ||
| } | ||
| await sleep(POLL_INTERVAL_MS); | ||
| const result = await requestJsonRpc( | ||
| jsonRpcEndpoint, | ||
| "eth_getTransactionReceipt", | ||
| [txHash] | ||
| ) | ||
| .then((result) => parseWithError(zTxReceiptJsonRpc, result)) | ||
| .catch(() => undefined); | ||
| return result ? result : pollForEvmTx(_txHash); | ||
| }; | ||
|
|
||
| return new Promise<TxReceiptJsonRpc>((resolve, reject) => { | ||
| pollForEvmTx(txHash).then( | ||
| (value) => { | ||
| clearTimeout(txPollTimeout); | ||
| resolve(value); | ||
| }, | ||
| (error) => { | ||
| clearTimeout(txPollTimeout); | ||
| reject(error); | ||
| } | ||
| ); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract timeout constants and improve error handling in polling mechanism.
The timeout and polling interval values should be configurable constants, and error handling could be improved.
+const EVM_TX_TIMEOUT_MS = 3000;
+const EVM_TX_POLL_INTERVAL_MS = 1000;
+
const getEvmTxResponse = async (jsonRpcEndpoint: string, txHash: string) => {
- const TIME_OUT_MS = 3000;
- const POLL_INTERVAL_MS = 1000;
+ const timeoutMessage = (hash: string, timeout: number) =>
+ `Transaction ${hash} was not found on chain after ${timeout / 1000}s`;
let timedOut = false;
const txPollTimeout = setTimeout(() => {
timedOut = true;
- }, TIME_OUT_MS);
+ }, EVM_TX_TIMEOUT_MS);
const pollForEvmTx = async (_txHash: string): Promise<TxReceiptJsonRpc> => {
if (timedOut) {
- throw new Error(
- `Transaction with hash ${_txHash} was submitted but was not yet found on the chain. You might want to check later. There was a wait of ${
- TIME_OUT_MS / 1000
- } seconds.`
- );
+ throw new Error(timeoutMessage(_txHash, EVM_TX_TIMEOUT_MS));
}
- await sleep(POLL_INTERVAL_MS);
+ await sleep(EVM_TX_POLL_INTERVAL_MS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getEvmTxResponse = async (jsonRpcEndpoint: string, txHash: string) => { | |
| const TIME_OUT_MS = 3000; | |
| const POLL_INTERVAL_MS = 1000; | |
| let timedOut = false; | |
| const txPollTimeout = setTimeout(() => { | |
| timedOut = true; | |
| }, TIME_OUT_MS); | |
| const pollForEvmTx = async (_txHash: string): Promise<TxReceiptJsonRpc> => { | |
| if (timedOut) { | |
| throw new Error( | |
| `Transaction with hash ${_txHash} was submitted but was not yet found on the chain. You might want to check later. There was a wait of ${ | |
| TIME_OUT_MS / 1000 | |
| } seconds.` | |
| ); | |
| } | |
| await sleep(POLL_INTERVAL_MS); | |
| const result = await requestJsonRpc( | |
| jsonRpcEndpoint, | |
| "eth_getTransactionReceipt", | |
| [txHash] | |
| ) | |
| .then((result) => parseWithError(zTxReceiptJsonRpc, result)) | |
| .catch(() => undefined); | |
| return result ? result : pollForEvmTx(_txHash); | |
| }; | |
| return new Promise<TxReceiptJsonRpc>((resolve, reject) => { | |
| pollForEvmTx(txHash).then( | |
| (value) => { | |
| clearTimeout(txPollTimeout); | |
| resolve(value); | |
| }, | |
| (error) => { | |
| clearTimeout(txPollTimeout); | |
| reject(error); | |
| } | |
| ); | |
| }); | |
| }; | |
| const EVM_TX_TIMEOUT_MS = 3000; | |
| const EVM_TX_POLL_INTERVAL_MS = 1000; | |
| const getEvmTxResponse = async (jsonRpcEndpoint: string, txHash: string) => { | |
| const timeoutMessage = (hash: string, timeout: number) => | |
| `Transaction ${hash} was not found on chain after ${timeout / 1000}s`; | |
| let timedOut = false; | |
| const txPollTimeout = setTimeout(() => { | |
| timedOut = true; | |
| }, EVM_TX_TIMEOUT_MS); | |
| const pollForEvmTx = async (_txHash: string): Promise<TxReceiptJsonRpc> => { | |
| if (timedOut) { | |
| throw new Error(timeoutMessage(_txHash, EVM_TX_TIMEOUT_MS)); | |
| } | |
| await sleep(EVM_TX_POLL_INTERVAL_MS); | |
| const result = await requestJsonRpc( | |
| jsonRpcEndpoint, | |
| "eth_getTransactionReceipt", | |
| [txHash] | |
| ) | |
| .then((result) => parseWithError(zTxReceiptJsonRpc, result)) | |
| .catch(() => undefined); | |
| return result ? result : pollForEvmTx(_txHash); | |
| }; | |
| return new Promise<TxReceiptJsonRpc>((resolve, reject) => { | |
| pollForEvmTx(txHash).then( | |
| (value) => { | |
| clearTimeout(txPollTimeout); | |
| resolve(value); | |
| }, | |
| (error) => { | |
| clearTimeout(txPollTimeout); | |
| reject(error); | |
| } | |
| ); | |
| }); | |
| }; |
| export const convertCosmosChainIdToEvmChainId = (chainId: string) => { | ||
| // metamask max | ||
| const METAMASK_MAX = "4503599627370476"; | ||
|
|
||
| const hash = keccak256(Buffer.from(chainId)); | ||
| const rawEvmChainId = Buffer.from(hash).readBigUInt64BE(); | ||
| return big(rawEvmChainId.toString()).mod(METAMASK_MAX).toFixed(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation, input validation, and error handling.
The chain ID conversion function needs improvements in several areas:
+/**
+ * Converts a Cosmos chain ID to an EVM-compatible chain ID
+ * @param chainId - The Cosmos chain ID string
+ * @returns A string representing the EVM chain ID, compatible with MetaMask's maximum value
+ * @throws {Error} If the chain ID is invalid or conversion fails
+ */
export const convertCosmosChainIdToEvmChainId = (chainId: string) => {
+ if (!chainId) {
+ throw new Error("Chain ID is required");
+ }
+
// MetaMask's maximum safe chain ID (2^52 - 1)
const METAMASK_MAX = "4503599627370476";
- const hash = keccak256(Buffer.from(chainId));
- const rawEvmChainId = Buffer.from(hash).readBigUInt64BE();
- return big(rawEvmChainId.toString()).mod(METAMASK_MAX).toFixed();
+ try {
+ const hash = keccak256(Buffer.from(chainId));
+ const rawEvmChainId = Buffer.from(hash).readBigUInt64BE();
+ return big(rawEvmChainId.toString()).mod(METAMASK_MAX).toFixed();
+ } catch (error) {
+ throw new Error(`Failed to convert chain ID: ${(error as Error).message}`);
+ }
};📝 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 convertCosmosChainIdToEvmChainId = (chainId: string) => { | |
| // metamask max | |
| const METAMASK_MAX = "4503599627370476"; | |
| const hash = keccak256(Buffer.from(chainId)); | |
| const rawEvmChainId = Buffer.from(hash).readBigUInt64BE(); | |
| return big(rawEvmChainId.toString()).mod(METAMASK_MAX).toFixed(); | |
| }; | |
| /** | |
| * Converts a Cosmos chain ID to an EVM-compatible chain ID | |
| * @param chainId - The Cosmos chain ID string | |
| * @returns A string representing the EVM chain ID, compatible with MetaMask's maximum value | |
| * @throws {Error} If the chain ID is invalid or conversion fails | |
| */ | |
| export const convertCosmosChainIdToEvmChainId = (chainId: string) => { | |
| if (!chainId) { | |
| throw new Error("Chain ID is required"); | |
| } | |
| // MetaMask's maximum safe chain ID (2^52 - 1) | |
| const METAMASK_MAX = "4503599627370476"; | |
| try { | |
| const hash = keccak256(Buffer.from(chainId)); | |
| const rawEvmChainId = Buffer.from(hash).readBigUInt64BE(); | |
| return big(rawEvmChainId.toString()).mod(METAMASK_MAX).toFixed(); | |
| } catch (error) { | |
| throw new Error(`Failed to convert chain ID: ${(error as Error).message}`); | |
| } | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
42-42: Add release dates for version entries.Each version section should include its release date to help track when changes were made available.
-## v1.9.3 +## v1.9.3 - 2024-01-10
Line range hint
42-1124: Consider standardizing entry descriptions.While most entries are clear, some could be more descriptive. Consider standardizing the format to always:
- Start with a verb (Add, Fix, Update, etc.)
- Clearly state what was changed
- Include the impact or reason when relevant
Example of improving an entry:
-[#1188](https://github.com/alleslabs/celatone-frontend/pull/1188) Add request and simulate evm tx +[#1188](https://github.com/alleslabs/celatone-frontend/pull/1188) Add request and simulate EVM transaction functionality to support Ethereum-compatible transactions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/lib/utils/validate.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/utils/validate.ts
🔇 Additional comments (2)
CHANGELOG.md (2)
Line range hint
1-24: LGTM! Well-structured changelog header.The header section properly explains the guiding principles and usage instructions for maintaining the changelog.
Line range hint
26-40: LGTM! Follows Keep a Changelog format.The file structure correctly follows the Keep a Changelog format and clearly states adherence to Semantic Versioning.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx (1)
Line range hint
24-26: Add validation for constructor arguments format.The placeholder suggests a specific hex format, but there's no validation to ensure the input matches this format. Consider adding validation to prevent invalid submissions.
// Add this validation function const isValidConstructorArgs = (value: string) => /^(0x)?[0-9a-fA-F]+$/.test(value); // Add validation to the onChange handler onChange={(e) => { const value = e.target.value; if (value === '' || isValidConstructorArgs(value)) { field.onChange(value); } }}
🧹 Nitpick comments (2)
src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx (2)
Line range hint
28-33: Improve accessibility for the constructor arguments input.The textarea lacks proper ARIA attributes and error handling for better accessibility.
<Textarea visibility={field.value === undefined ? "hidden" : "visible"} placeholder="ex.000000000000000000000000c005dc82818d67af737725bd4bf75435d065d239" value={field.value} onChange={(e) => field.onChange(e.target.value)} + aria-label="Constructor Arguments" + aria-describedby="constructor-args-help" /> +<Text id="constructor-args-help" fontSize="sm" color="text.dark"> + Enter the constructor arguments in hexadecimal format +</Text>
4-6: Consider using a more specific type for the form control.The
EvmContractVerifyFormtype might be too broad. Consider creating a more specific type for the constructor arguments field.interface ConstructorArgsField { verifyForm: { form: { constructorArgs?: string; }; }; } interface ConstructorArgsProps { control: Control<ConstructorArgsField>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/components/forms/SelectInput.tsx(1 hunks)src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/lib/components/forms/SelectInput.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
Summary by CodeRabbit
Based on the comprehensive pull request summary, here are the updated release notes:
New Features
Improvements
Bug Fixes
Chores