-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: validate evm verify form options #1225
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis PR introduces multiple enhancements to the contract verification workflow. A new changelog entry for EVM verification form option validation is added. The contract library components are refactored by simplifying state management and introducing a new dedicated component, while related components in contract details are renamed. Additionally, the optimizer configuration and overall EVM verification form have their validation logic improved with additional state and Zod schema refinements. Several schema definitions are enhanced to enforce stricter validation rules. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/lib/pages/evm-contract-verify/index.tsx (2)
131-185
: Consider consolidating partial schema validation logic.In
isFormDisabled
, multiple Zod schemas are selectively parsed depending on the chosen verification option, which can lead to verbose and repetitive checks. It might be more maintainable to build a consolidated schema (or a more composable approach) that accounts for each option’s conditional fields. This way, validation remains consistent and easier to update as the number of verification options grows.
194-195
: Provide user feedback or logging for missing parameters.When
option
,language
, orlicenseType
is missing, the function simply returns without any warning or error message. Consider displaying a user-facing toast or inline error to clarify why submission is aborted, as this helps users correct mistakes more efficiently.src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx (1)
16-56
: Use form error states instead of manual string checks.Currently, the error prop is derived from
field.value.name !== "" ? "" : "Required"
. This can conflict with the form library’s validation flow and lead to discrepancies. For better maintainability and consistency, rely on built-inerrors[fieldName]?.message
(from your Zod schema orrules
) rather than manually checking for an empty string.src/lib/pages/evm-contract-verify/components/OptimizerConfiguration.tsx (1)
25-64
: Align inline checks with global form validation.You’re conditionally setting the status to error when
enabled
is true butruns
is an empty string. While this is effective, consider consolidating it with your Zod schema validation so the UI and form states stay in sync. Doing so would also simplify the code by using a single source of truth for validations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md
(1 hunks)src/lib/pages/evm-contract-verify/components/ContractLibraries.tsx
(4 hunks)src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx
(1 hunks)src/lib/pages/evm-contract-verify/components/OptimizerConfiguration.tsx
(2 hunks)src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityContractCode.tsx
(1 hunks)src/lib/pages/evm-contract-verify/index.tsx
(2 hunks)src/lib/services/types/verification/evm.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityContractCode.tsx (1)
58-58
: LGTM! The name prop update improves form state management.The change to use a more specific path
verifyForm.solidityContractCode.contractLibraries
better reflects the form's data structure and aligns with the contract libraries state management refactoring.src/lib/pages/evm-contract-verify/components/ContractLibraries.tsx (3)
7-7
: LGTM! Improved type safety and component organization.The addition of
FieldArrayPath
import and newContractLibrary
component improves type safety and modularizes the library field logic.Also applies to: 13-13
26-26
: LGTM! Simplified state management.The changes improve state management by:
- Using a more specific path for enabled state
- Using proper field array typing
- Directly using field.value for checkbox state
- Simplifying display logic
Also applies to: 31-31, 47-48, 58-58
60-65
: LGTM! Improved component composition.The refactoring to use the dedicated
ContractLibrary
component simplifies the code and improves maintainability by encapsulating library field logic.src/lib/services/types/verification/evm.ts (6)
40-45
: LGTM! Added validation for optimizer configuration.The refinement ensures that if optimizer is enabled, the runs field must not be empty, preventing invalid configurations.
48-53
: LGTM! Added validation for constructor arguments.The refinement ensures that if constructor arguments are enabled, the value field must not be empty, preventing invalid configurations.
55-55
: LGTM! Added validation for EVM target version.The refinement ensures that the target version is not empty, preventing invalid configurations.
62-71
: LGTM! Added comprehensive validation for contract libraries.The refinement ensures that if libraries are enabled:
- Library names are not empty
- Addresses are valid hex addresses
This prevents invalid library configurations.
73-82
: LGTM! Added file validation schema.The new
zFile
schema with custom validation ensures that files are provided when required, improving form validation.
85-97
: LGTM! Improved schema organization and reusability.The changes to export schemas improve code organization and allow reuse across the codebase. The schemas now properly validate:
- File uploads
- Contract code
- JSON input
- Contract names
Also applies to: 99-105, 107-110, 113-117, 119-124, 126-129
CHANGELOG.md (1)
42-42
: LGTM! Clear and well-formatted changelog entry.The changelog entry accurately describes the EVM Verification form options validation feature and follows the proper format.
src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx (2)
34-34
: Improve type safety for nested field paths.String concatenation for field paths could lead to type safety issues. Consider using a type-safe approach.
Use a utility type to ensure type safety:
type NestedFieldPath<T, K extends string> = `${FieldPath<T>}.${K}`; // Usage name={name as NestedFieldPath<T, 'name'>} name={name as NestedFieldPath<T, 'address'>}Also applies to: 46-46
37-37
: Consider making the label background color configurable.The hardcoded
labelBgColor="gray.900"
might not work well with different themes.Make it configurable through props:
- labelBgColor="gray.900" + labelBgColor={props.labelBgColor ?? "gray.900"}Update the interface:
interface ContractLibraryProps<T extends FieldValues> { control: Control<T>; name: FieldPath<T>; + labelBgColor?: string; }Also applies to: 49-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractCode.tsx
(2 hunks)src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractLibraryList.tsx
(1 hunks)src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractLibraryList.tsx (1)
7-9
: LGTM! The renaming improves clarity.The renaming of the interface and component to
ContractLibraryListProps
andContractLibraryList
respectively better reflects the component's purpose of displaying a list of contract libraries.Also applies to: 13-15
src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractCode.tsx (1)
17-17
: LGTM! The changes are consistent.The import statement and component usage have been correctly updated to use the renamed
ContractLibraryList
component.Also applies to: 77-77
src/lib/pages/evm-contract-verify/components/ContractLibrary.tsx (1)
1-14
: LGTM! Clean imports and well-typed interface.The imports are well-organized and the generic interface is properly typed for form handling.
Summary by CodeRabbit
New Features
ContractLibrary
component for improved user input handling regarding contract libraries.Refactor