feat: evm interact form #1202
feat: evm interact form #1202
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis pull request introduces a new EVM (Ethereum Virtual Machine) contract interaction form, adding a comprehensive set of components and utilities for managing dynamic form inputs related to Ethereum ABI (Application Binary Interface). The changes include the creation of multiple React components for various field types, enhanced validation functions, and utility functions for handling JSON fragment types and dimensions. Additionally, updates to import paths reflect a reorganization of the codebase. Changes
Sequence DiagramsequenceDiagram
participant User
participant EvmAbiForm
participant FormFields
participant Field
participant BaseField
participant Validation
User->>EvmAbiForm: Interact with form
EvmAbiForm->>FormFields: Render form components
FormFields->>Field: Determine field type
Field->>BaseField: Render input field
BaseField->>Validation: Validate input
Validation-->>BaseField: Return validation result
BaseField-->>User: Display field with validation feedback
Possibly related PRs
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (11)
src/lib/utils/validate.ts (1)
Line range hint
50-57: LGTM! The function export aligns with the EVM form requirements.The implementation correctly validates fixed-length hex byte strings with exact length matching.
Consider adding input validation.
The function assumes valid inputs but could benefit from parameter validation:
export const isHexFixedBytes = (address: string, length: number): boolean => { + if (!address || length <= 0) return false; const regex = new RegExp(`^0x[a-fA-F0-9]{${length}}$`); if (!regex.test(address)) { return false; }Consider consolidating hex validation functions.
Since
isHexFixedBytesandisHexAddressshare similar logic, consider combining them:const validateHexString = ( address: string, length: number, exact: boolean = false ): boolean => { if (!address || length <= 0) return false; const pattern = exact ? `^0x[a-fA-F0-9]{${length}}$` : `^0x[a-fA-F0-9]{1,${length}}$`; const regex = new RegExp(pattern); if (!regex.test(address)) { return false; } const strip = padHexAddress(address as HexAddr, length).slice(2); return isHex(strip); }; export const isHexFixedBytes = (address: string, length: number): boolean => validateHexString(address, length, true); const isHexAddress = (address: string, length: number): boolean => validateHexString(address, length, false);src/lib/components/evm-abi/fields/types.ts (1)
4-9: LGTM! Consider adding JSDoc comments.The
FieldPropsinterface is well-designed with proper type safety and good integration with both ethers and react-hook-form libraries.Consider adding JSDoc comments to document the interface and its properties, especially explaining the relationship with
JsonFragmentTypeand the purpose of omitting the name property. Example:+/** + * Props for form field components that handle Ethereum ABI types + * @template T - The type of form values + */ export interface FieldProps<T extends FieldValues> extends Omit<JsonFragmentType, "name"> { + /** Form control from react-hook-form */ control: Control<T>; + /** Field path in the form values */ name: FieldPath<T>; + /** Optional label to display above the field */ label?: string; }src/lib/components/evm-abi/fields/Field.tsx (1)
7-16: Add error boundary and prop validation.The component could benefit from better error handling and prop validation:
- Missing error boundary for child component failures
- Uncontrolled prop spreading could lead to prop pollution
Consider wrapping the component with an error boundary and validating props:
import { ErrorBoundary } from 'react-error-boundary'; interface ValidatedFieldProps<T extends FieldValues> extends FieldProps<T> { // Add only the additional props that are actually needed className?: string; style?: React.CSSProperties; } export const Field = <T extends FieldValues>({ type, components, className, style, ...rest }: ValidatedFieldProps<T>) => { return ( <ErrorBoundary fallback={<div>Something went wrong</div>}> {/* Your existing conditional rendering */} </ErrorBoundary> ); };src/lib/components/evm-abi/fields/BoolField.tsx (1)
5-14: Use type-safe boolean value mapping.The current string-based boolean mapping could be improved:
- Magic strings ("1"/"0") are used for values
- No TypeScript type safety for option values
Consider this type-safe implementation:
+const enum BooleanValue { + True = "1", + False = "0" +} + -const BOOL_FIELD_OPTIONS: SelectInputOption<string>[] = [ +const BOOL_FIELD_OPTIONS: SelectInputOption<BooleanValue>[] = [ { label: "True", - value: "1", + value: BooleanValue.True, }, { label: "False", - value: "0", + value: BooleanValue.False, }, ];src/lib/components/evm-abi/fields/BaseField.tsx (2)
16-24: Consider memoizing form controller setup.The
useControllerhook setup could benefit from memoization to prevent unnecessary re-renders, especially when handling complex form structures.+ const rules = useMemo(() => getRules<T>(type, isRequired), [type, isRequired]); const { field: { value, onBlur, onChange, ...fieldProps }, fieldState: { isTouched, isDirty, error }, } = useController({ name, control, - rules: getRules<T>(type, isRequired), + rules, });
26-31: Consider adding aria-label for accessibility.The Input component should have an aria-label for better accessibility.
<FormControl isInvalid={isError} {...fieldProps}> - <Input value={value} onBlur={onBlur} onChange={onChange} /> + <Input + value={value} + onBlur={onBlur} + onChange={onChange} + aria-label={name} + /> {isError && <FormErrorMessage>{error.message}</FormErrorMessage>} </FormControl>src/lib/components/evm-abi/EvmAbiForm.tsx (2)
42-45: Consider debouncing the onChange callback.The
propsOnChangecallback with deep cloning could be expensive for large data structures. Consider debouncing the updates.+ import { useMemo, useCallback } from 'react'; + import debounce from 'lodash/debounce'; + const debouncedOnChange = useCallback( + debounce((data: JsonDataType[]) => { + propsOnChange?.(cloneDeep(data)); + }, 300), + [propsOnChange] + ); useEffect(() => { - propsOnChange?.(cloneDeep(inputs)); + debouncedOnChange(inputs); // eslint-disable-next-line react-hooks/exhaustive-deps }, [JSON.stringify(inputs)]);
28-34: Consider adding form validation mode configuration.The form's validation mode is hardcoded to "all". Consider making it configurable through props for different use cases.
+ interface EvmAbiFormProps { + // ... existing props + validationMode?: "onBlur" | "onChange" | "onSubmit" | "all"; + } const { control, reset, watch } = useForm<{ inputs: JsonDataType[]; payableAmount: string; }>({ defaultValues: { inputs: defaultValues, payableAmount: "" }, - mode: "all", + mode: validationMode ?? "all", });src/lib/components/evm-abi/fields/utils.ts (2)
8-17: Enhance uint validation with additional checks.The current implementation might allow invalid inputs like decimals or scientific notation. Consider adding checks for these cases.
const validateUint = (uintType: string) => (v: string) => { try { + if (v.includes('.') || v.toLowerCase().includes('e')) { + return `Input must be a whole number`; + } const value = big(v); const maxValue = big(2).pow(parseInt(uintType.slice(4))); if (value.lt(0) || value.gte(maxValue)) throw new Error(); return undefined; } catch { return `Input must be '${uintType}'`; } };
54-55: Improve regex pattern for base type extraction.The current regex pattern
/(^.[^[]*)/gmight be too permissive. Consider using a more specific pattern that matches EVM types.- const baseTypeRegExp: RegExp = /(^.[^[]*)/g; + const baseTypeRegExp: RegExp = /^(u?int\d*|address|bytes\d*|bool|string)(?:\[\d*\])*$/;src/lib/components/evm-abi/fields/FieldTemplate.tsx (1)
82-87: Consider memoizing the onChange handler.The handler function is recreated on every render, which could impact performance for deeply nested arrays.
+ const handleAddItem = useCallback(() => { + onChange([ + ...value, + getDefaultValueFromDimensions(restDimensions, components), + ]); + }, [value, onChange, restDimensions, components]); // In JSX: - onClick={() => - onChange([ - ...value, - getDefaultValueFromDimensions(restDimensions, components), - ]) - } + onClick={handleAddItem}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
CHANGELOG.md(1 hunks)src/lib/components/evm-abi/EvmAbiForm.tsx(1 hunks)src/lib/components/evm-abi/fields/BaseField.tsx(1 hunks)src/lib/components/evm-abi/fields/BoolField.tsx(1 hunks)src/lib/components/evm-abi/fields/Field.tsx(1 hunks)src/lib/components/evm-abi/fields/FieldTemplate.tsx(1 hunks)src/lib/components/evm-abi/fields/TupleField.tsx(1 hunks)src/lib/components/evm-abi/fields/TypeLabel.tsx(1 hunks)src/lib/components/evm-abi/fields/index.tsx(1 hunks)src/lib/components/evm-abi/fields/types.ts(1 hunks)src/lib/components/evm-abi/fields/utils.ts(1 hunks)src/lib/components/evm-abi/index.ts(1 hunks)src/lib/components/evm-abi/utils.ts(1 hunks)src/lib/pages/deploy-script/components/ScriptInput.tsx(1 hunks)src/lib/pages/interact/component/form/ExecuteArea.tsx(1 hunks)src/lib/pages/interact/component/form/ViewArea.tsx(1 hunks)src/lib/utils/evm.ts(2 hunks)src/lib/utils/validate.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/lib/pages/interact/component/form/ViewArea.tsx
- src/lib/components/evm-abi/index.ts
- src/lib/pages/deploy-script/components/ScriptInput.tsx
🔇 Additional comments (6)
src/lib/pages/interact/component/form/ExecuteArea.tsx (2)
14-14: Verify the import path change impact.The import path change from "lib/components/abi" to "lib/components/move-abi" suggests a refactoring of ABI components. Since this component handles Move module execution, we should verify that this change aligns with the PR's objective of adding EVM interaction support.
Let's verify the import changes and their impact:
✅ Verification successful
Import path change is correct and properly structured
The change from "lib/components/abi" to "lib/components/move-abi" is part of a well-structured separation between Move and EVM components. The ExecuteArea component correctly uses the Move ABI form as it handles Move module execution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the structure and usage of ABI components echo "Checking ABI component structure..." fd -e tsx -e ts "abi|move-abi" src/lib/components echo "Checking usage of ABI components..." rg -l "from ['\"]lib/components/(abi|move-abi)['\"]" src/Length of output: 519
Script:
#!/bin/bash # Check the content of relevant files echo "=== ExecuteArea.tsx ===" rg -A 5 "AbiForm" src/lib/pages/interact/component/form/ExecuteArea.tsx echo -e "\n=== ViewArea.tsx ===" rg -A 5 "AbiForm" src/lib/pages/interact/component/form/ViewArea.tsx echo -e "\n=== ScriptInput.tsx ===" rg -A 5 "AbiForm" src/lib/pages/deploy-script/components/ScriptInput.tsx echo -e "\n=== Checking for EvmAbiForm usage ===" rg -A 5 "EvmAbiForm" src/Length of output: 4152
Line range hint
1-186: Clarify the relationship between Move and EVM interactions.This component is specifically designed for Move module interactions (using
MsgExecuteModule), but the PR's objective is to add EVM contract interaction support. Please clarify:
- Is this file part of the EVM interaction implementation?
- If yes, should it be refactored to handle both Move and EVM interactions?
- If no, why is it being modified as part of this PR?
Let's check for EVM-related changes:
src/lib/components/evm-abi/fields/BaseField.tsx (1)
6-8: LGTM! Well-structured generic interface.The
BaseFieldPropsinterface properly extendsFieldPropswith generic type constraints, allowing type-safe form field implementations.src/lib/components/evm-abi/fields/TupleField.tsx (1)
51-51: Verify field path type safety.The type assertion
as FieldPath<T>might not be safe if the constructed path doesn't match the form's schema.✅ Verification successful
Type assertion is safe in this context
The
as FieldPath<T>assertion is safe because the field path is constructed within a tuple field that strictly follows the EVM ABI schema. The index is bounded by the components array length, and the parent name is already a valid field path, ensuring type safety at runtime.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar field path constructions in the codebase rg -A 2 "as FieldPath<" --type typescriptLength of output: 78
Script:
#!/bin/bash # Search for FieldPath type usage and field path constructions rg --type-add 'ts:*.{ts,tsx}' -t ts -A 3 'FieldPath|name\.\$\{index\}|name\s*\+\s*index' # Search for form schema definitions in the component's directory fd -e ts -e tsx . src/lib/components/evm-abi | xargs cat # Look for tuple field type definitions ast-grep --pattern 'type $_ = { $$$ tuple: $_ $$$ }'Length of output: 20405
CHANGELOG.md (2)
42-42: LGTM! The changelog entry follows the established format.The entry is properly placed under the "Features" section and includes the PR number with a clear description of the changes.
Line range hint
1-40: LGTM! Well-structured changelog documentation.The file includes comprehensive guidelines for maintaining the changelog, follows established standards (Keep a Changelog and Semantic Versioning), and maintains consistent formatting throughout.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/lib/components/evm-abi/fields/TupleField.tsx (1)
21-24:⚠️ Potential issueAdd type safety check for watched values.
The direct usage of watched values without type checking could lead to runtime errors.
- const values = useWatch<T>({ + const values = useWatch<T>({ control, name, }); + const safeValues = Array.isArray(values) ? values : [];
🧹 Nitpick comments (3)
src/lib/components/evm-abi/fields/BoolField.tsx (1)
21-26: Consider handling undefined values.The form controller doesn't set a default value, which could lead to undefined value issues. Consider setting a default value to ensure consistent behavior.
} = useController({ control, name, + defaultValue: "0", // Default to false });src/lib/pages/evm-contract-details/dummy.ts (2)
1-2: Track temporary code with issue.The TODO comment indicates this is temporary code. To ensure it's not forgotten, this should be tracked properly.
Would you like me to create an issue to track the removal of this temporary file?
5-127: Consider moving test data to dedicated test fixtures.The
EVM_ABIconstant contains complex test data that would be better maintained in dedicated test fixtures.Consider:
- Moving this data to a
__fixtures__directory- Breaking it down into smaller, focused test cases
- Adding type validation using zod or io-ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/lib/components/evm-abi/EvmAbiForm.tsx(1 hunks)src/lib/components/evm-abi/fields/BaseField.tsx(1 hunks)src/lib/components/evm-abi/fields/BoolField.tsx(1 hunks)src/lib/components/evm-abi/fields/TupleField.tsx(1 hunks)src/lib/components/evm-abi/fields/index.tsx(1 hunks)src/lib/components/evm-abi/fields/types.ts(1 hunks)src/lib/pages/evm-contract-details/dummy.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/components/evm-abi/fields/types.ts
🔇 Additional comments (11)
src/lib/components/evm-abi/fields/BoolField.tsx (3)
1-14: LGTM! Well-structured constants and imports.The boolean options are correctly defined with "1" and "0" values, which align with EVM ABI boolean encoding.
16-20: LGTM! Well-typed component definition.The component is properly typed with generics and includes all necessary props for form integration.
28-41: Handle SSR and improve value handling.The component still has potential issues with SSR compatibility and value handling that were previously identified.
The previous review suggested:
- Using useState and useEffect for SSR-safe portal target
- Adding validation for empty/invalid values
- Providing a default value for undefined cases
src/lib/components/evm-abi/EvmAbiForm.tsx (3)
24-28: Optimize dependency tracking for useMemo.Using
JSON.stringifyin dependencies can be expensive and might cause unnecessary recalculations.const defaultValues = useMemo( () => initialData ?? getComponentsDefaultValues(types), - // eslint-disable-next-line react-hooks/exhaustive-deps - [JSON.stringify(initialData)] + [initialData, types] );
39-43: 🛠️ Refactor suggestionOptimize useEffect dependency array.
Similar to the useMemo optimization, using
JSON.stringifyin dependencies is inefficient.useEffect(() => { reset({ inputs: defaultValues }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(defaultValues), reset]); + }, [defaultValues, reset]);Likely invalid or redundant comment.
44-48: 🛠️ Refactor suggestionOptimize form data change handling.
Using
cloneDeepon every form change is expensive. Consider using a more efficient approach.useEffect(() => { - propsOnChange?.(cloneDeep(inputs)); + // Only clone when the callback is actually called + if (propsOnChange) { + const clonedInputs = cloneDeep(inputs); + propsOnChange(clonedInputs); + } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(inputs), propsOnChange]); + }, [inputs, propsOnChange]);Likely invalid or redundant comment.
src/lib/components/evm-abi/fields/index.tsx (2)
23-31: Add error handling for data fetching.The component should handle loading and error states for
useEvmParamsanduseAssetInfoshooks.
37-37: Avoid type casting in name prop.The type casting of the name prop to
Path<T>could be unsafe.Also applies to: 49-49
src/lib/components/evm-abi/fields/TupleField.tsx (3)
1-12: LGTM! Well-structured type definitions and imports.The imports are appropriate, and the interface is well-defined with proper TypeScript constraints.
14-20: LGTM! Well-structured component definition.The component follows React best practices with proper TypeScript generics and prop typing.
51-59: Add type safety for field path construction.The type assertion for the field path might be unsafe if the path doesn't exist in the form values type.
Consider using a type guard or validation utility to ensure the constructed path is valid:
// Add this utility to your codebase function isValidFieldPath<T>(path: string): path is FieldPath<T> { // Implement validation logic based on your form structure return true; // Replace with actual validation } // Then use it in the component const fieldPath = `${name}.${index}`; if (!isValidFieldPath<T>(fieldPath)) { console.warn(`Invalid field path: ${fieldPath}`); return null; }
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/lib/components/evm-abi/fields/index.tsx (3)
23-26:⚠️ Potential issueAdd error handling for data fetching hooks.
The hooks
useEvmParamsanduseAssetInfosshould handle loading and error states to prevent potential runtime issues.- const { data: evmParamsData } = useEvmParams(); - const { data: assetInfos } = useAssetInfos({ + const { data: evmParamsData, error: evmError, isLoading: isEvmLoading } = useEvmParams(); + const { data: assetInfos, error: assetError, isLoading: isAssetLoading } = useAssetInfos({ withPrices: true, }); + if (isEvmLoading || isAssetLoading) { + return <Spinner />; + } + if (evmError || assetError) { + return <Alert status="error">Failed to load required data</Alert>; + }
37-37:⚠️ Potential issueRemove unsafe type casting.
Type casting with
as Path<T>could lead to runtime errors. Consider using proper type definitions.- name={"inputs" as Path<T>} + name="inputs" - name={"payableAmount" as Path<T>} + name="payableAmount"Also applies to: 49-49
28-31:⚠️ Potential issueEnhance null safety for nested object access.
The current implementation could lead to runtime errors when accessing nested properties.
- const feeDenom = evmParamsData?.params.feeDenom; + const feeDenom = evmParamsData?.params?.feeDenom; const feeLabel = feeDenom - ? getTokenLabel(feeDenom, assetInfos?.[feeDenom]?.symbol) + ? getTokenLabel(feeDenom, assetInfos?.[feeDenom]?.symbol ?? '') : undefined;src/lib/components/evm-abi/EvmAbiForm.tsx (1)
24-28: 🛠️ Refactor suggestionOptimize useMemo dependency tracking.
Using
JSON.stringifyin dependencies is expensive and can cause unnecessary recalculations.const defaultValues = useMemo( () => initialData ?? getComponentsDefaultValues(types), - // eslint-disable-next-line react-hooks/exhaustive-deps - [JSON.stringify(initialData), types] + [initialData, types] );
🧹 Nitpick comments (2)
src/lib/components/evm-abi/fields/index.tsx (1)
10-15: Consider making components prop more specific.The
componentsprop type could be more specific to better reflect the expected structure.interface FormFieldsProps<T extends FieldValues> { control: Control<T>; - components: ReadonlyArray<JsonFragmentType>; + components: ReadonlyArray<JsonFragmentType & { + name: string; + type: string; + components?: JsonFragmentType[]; + }>; isPayable: boolean; isDisabled?: boolean; }src/lib/components/evm-abi/EvmAbiForm.tsx (1)
30-36: Consider adding form validation.The form is set up with
mode: "all"but lacks validation rules. Consider adding validation for the inputs and payable amount.const { control, reset, watch } = useForm<{ inputs: JsonDataType[]; payableAmount: string; }>({ defaultValues: { inputs: defaultValues, payableAmount: "" }, mode: "all", + resolver: zodResolver(schema), });Would you like me to help create a validation schema for the form?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/components/evm-abi/EvmAbiForm.tsx(1 hunks)src/lib/components/evm-abi/fields/index.tsx(1 hunks)
Summary by CodeRabbit
Release Notes
New Features
EVM_ABIfor mapping Ethereum function signatures.Improvements
Documentation