Standard JSON input for Solidity and Vyper #1197
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis pull request introduces enhanced functionality for EVM contract verification by adding support for standard JSON input for both Solidity and Vyper contracts. The changes expand file type support in the dropzone component, modify form handling for JSON input, and update Zod schemas to accommodate new file upload options. The modifications improve the flexibility of contract verification processes by introducing new file types, optimizing configuration options, and refining the user interface for contract verification. Changes
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/lib/pages/evm-contract-verify/components/vyper/EvmContractVerifyVyperJsonInput.tsx (1)
28-35: Consider adding file size validation.While the file type validation is in place, consider adding file size limits to prevent large JSON files from being uploaded.
src/lib/pages/evm-contract-verify/components/vyper/EvmContractVerifyVyperUploadFile.tsx (1)
32-35: Consider supporting multiple file uploads.Since Vyper contracts might have dependencies or imports, consider supporting multiple file uploads.
<DropZone - setFiles={(files) => onChange(files[0])} + setFiles={onChange} fileType={["vy"]} + multiple />src/lib/pages/evm-contract-verify/helper.ts (1)
24-24: Consider improving type safety and configuration management.A few suggestions to enhance the code:
- Extract "default" as a constant to avoid magic strings
- Consider using a more specific type for the file field
- Add TypeScript type annotations for the return types
+ const DEFAULT_EVM_VERSION = "default"; + type VerifyFormValue = { + option: VerificationOptions; + constructorArgs: { + enabled: boolean; + value: string; + }; + evmVersion?: string; + file?: File; + }; - export const getVerifyFormInitialValue = ( + export const getVerifyFormInitialValue = ( + language: EvmProgrammingLanguage, + verificationOption: VerificationOptions + ): VerifyFormValue => {Also applies to: 45-45, 47-47
src/lib/pages/evm-contract-verify/components/EvmVersionToTarget.tsx (1)
45-57: Add aria-label for better accessibility.The SelectInput component could benefit from additional accessibility attributes.
<SelectInput label="EVM Version to target" + aria-label="Select EVM Version" menuPortalTarget={document.body} isRequired options={evmVersionOptions}src/lib/components/forms/SelectInput.tsx (1)
131-136: Consider using a higher z-index value from a theme constant.The current z-index of 2 might be too low for complex layouts with multiple stacking contexts. Consider using a theme constant for z-index values to maintain consistency.
styles={{ menuPortal: (provided) => ({ ...provided, - zIndex: 2, + zIndex: theme.zIndices.dropdown ?? 1000, }), }}src/lib/components/dropzone/index.tsx (1)
51-53: Extract file size limits to configuration constants.The file size limits are hardcoded. Consider moving these to a configuration file or environment variables for better maintainability.
+const FILE_SIZE_LIMITS = { + STANDARD: 10_000_000, + SCHEMA: 10_000_000, + MOVE: 10_000_000, + VYPER: 10_000_000, +} as const; const sizes: { [key in DropzoneFileType]: number; } = { - schema: 10_000_000, - move: 10_000_000, + schema: FILE_SIZE_LIMITS.SCHEMA, + move: FILE_SIZE_LIMITS.MOVE, wasm: wasm.enabled ? wasm.storeCodeMaxFileSize : 0, mv: move.enabled ? move.moduleMaxFileSize : 0, toml: 1_000_000, - vy: 10_000_000, - "standard-json": 10_000_000, + vy: FILE_SIZE_LIMITS.VYPER, + "standard-json": FILE_SIZE_LIMITS.STANDARD, };
🛑 Comments failed to post (5)
src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityJsonInput.tsx (1)
1-39: 🛠️ Refactor suggestion
Consider creating a shared component for JSON input.
This component is identical to
EvmContractVerifyVyperJsonInput. Consider extracting the common logic into a shared component to avoid duplication.Example implementation:
interface ContractVerifyJsonInputProps { control: Control<EvmContractVerifyForm>; title?: string; } const ContractVerifyJsonInput = ({ control, title = "Provide Standard JSON Input" }: ContractVerifyJsonInputProps) => { const { field: { value, onChange }, } = useController({ control, name: "verifyForm.form.jsonFile", }); return ( <Stack spacing={12}> <Stack spacing={4}> <Heading as="h6" variant="h6"> {title} </Heading> {value ? ( <UploadCard file={value} deleteFile={() => onChange("")} /> ) : ( <DropZone setFiles={(files) => onChange(files[0])} fileType={["standard-json"]} /> )} </Stack> <ConstructorArgs control={control} /> </Stack> ); }; // Usage in Solidity component export const EvmContractVerifySolidityJsonInput = ({ control }: EvmContractVerifySolidityJsonInputProps) => ( <ContractVerifyJsonInput control={control} /> ); // Usage in Vyper component export const EvmContractVerifyVyperJsonInput = ({ control }: EvmContractVerifyVyperJsonInputProps) => ( <ContractVerifyJsonInput control={control} /> );src/lib/components/dropzone/config.ts (1)
63-69: 💡 Codebase verification
Confirmed: Better distinction needed between JSON file types
The codebase shows that JSON file types are distinguished only through user selection, with no content validation. The
prettyFileTypefor standard-json should be more descriptive (e.g., "Solidity Standard JSON Input") to help users select the correct file type.🔗 Analysis chain
Consider distinguishing between JSON schema and standard JSON input files.
The
standard-jsontype uses the same extension (.json) and MIME type as the existingschematype. This might cause confusion in file type detection. Consider:
- Adding a more descriptive
prettyFileTypeto clearly indicate its purpose (e.g., "Solidity/Vyper Standard JSON Input")- Adding validation to distinguish between schema and standard JSON input files based on their content structure
Run this script to check for potential file type conflicts:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for duplicate file extensions across different file types # Extract and compare file extensions echo "Analyzing file extensions in DROPZONE_CONFIG..." rg -A 5 'accept:.*\[.*\]' src/lib/components/dropzone/config.tsLength of output: 1136
Script:
#!/bin/bash # Search for file type validation logic echo "Searching for file type validation..." rg -g '!*.test.*' -g '!*.spec.*' "standard-json|schema.*json" src/lib/components/dropzone/ # Check dropzone component implementation echo -e "\nChecking dropzone component implementation..." ast-grep --pattern 'function $_($$_) { $$$ dropzone $$$ }'Length of output: 542
src/lib/pages/evm-contract-verify/components/EvmVersionToTarget.tsx (1)
23-31: 🛠️ Refactor suggestion
Expand EVM version options beyond just "Default".
The current implementation only offers a single EVM version option. Consider adding commonly used EVM versions like "london", "berlin", "istanbul", etc.
const evmVersionOptions = useMemo( () => [ { label: "Default", value: "default", }, + { + label: "London", + value: "london", + }, + { + label: "Berlin", + value: "berlin", + }, + { + label: "Istanbul", + value: "istanbul", + }, ], [] );📝 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 evmVersionOptions = useMemo( () => [ { label: "Default", value: "default", }, { label: "London", value: "london", }, { label: "Berlin", value: "berlin", }, { label: "Istanbul", value: "istanbul", }, ], [] );src/lib/pages/evm-contract-verify/types.ts (1)
54-54: 🛠️ Refactor suggestion
Add file size and type validation to File schemas.
The current File validation only checks if the value is an instance of File. Consider adding size and type validation to prevent invalid uploads.
- jsonFile: z.instanceof(File).optional(), + jsonFile: z.instanceof(File) + .refine( + (file) => file.size <= 10_000_000, + "File size must be less than 10MB" + ) + .refine( + (file) => file.type === "application/json", + "File must be a JSON file" + ) + .optional(),Also applies to: 69-69, 81-81
src/lib/components/ContractInputSection.tsx (1)
29-29: 💡 Codebase verification
Inconsistent naming between interface and implementation
The rename from
contracttowasmContractcreates an inconsistency:
- Component's props interface uses
contract- All consuming components pass
contractprop- Only the internal implementation uses
wasmContractWhile this component is indeed WASM-specific (used only in WASM contexts), the rename should be consistent across both interface and implementation, or kept as
contractsince it's already in a WASM context.🔗 Analysis chain
LGTM! Verify component usage context.
The rename from
contracttowasmContractcorrectly reflects this component's WASM-specific nature.Let's confirm this component is only used in WASM contexts:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ContractInputSection usage rg -l "ContractInputSection" | grep -v "test"Length of output: 149
Script:
#!/bin/bash # Check ContractInputSection usage context in migrate and admin pages echo "=== migrate/index.tsx ===" rg -B 2 -A 2 "ContractInputSection" src/lib/pages/migrate/index.tsx echo -e "\n=== admin/index.tsx ===" rg -B 2 -A 2 "ContractInputSection" src/lib/pages/admin/index.tsxLength of output: 1332
Script:
#!/bin/bash # Check if these pages are WASM-specific by looking at their imports and key terms echo "=== migrate/index.tsx WASM context ===" rg -i "wasm|cosmwasm" src/lib/pages/migrate/index.tsx echo -e "\n=== admin/index.tsx WASM context ===" rg -i "wasm|cosmwasm" src/lib/pages/admin/index.tsx # Also check the component's props interface echo -e "\n=== ContractInputSection props ===" rg "interface.*Props" -A 5 src/lib/components/ContractInputSection.tsxLength of output: 993
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/components/json-schema/UploadTemplate.tsx (1)
Line range hint
1-1: Consider renaming the component directory.The component is located in the "json-schema" directory, but it now handles general JSON files. Consider renaming the directory to better reflect its current purpose.
src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityJsonInput.tsx (1)
31-35: Consider adding file validation.The component accepts JSON files but doesn't validate their content. Consider adding validation to ensure the file contains valid Solidity contract JSON input.
<DropZone setFiles={(files) => onChange(files[0])} fileType={["json"]} + onError={(error) => { + // Handle validation error + }} + validate={(file) => { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => { + try { + const content = JSON.parse(reader.result as string); + if (!content.language || content.language !== 'Solidity') { + reject(new Error('Invalid Solidity JSON input')); + } + resolve(true); + } catch { + reject(new Error('Invalid JSON format')); + } + }; + reader.readAsText(file); + }); + }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/components/dropzone/config.ts(2 hunks)src/lib/components/dropzone/index.tsx(2 hunks)src/lib/components/json-schema/UploadTemplate.tsx(1 hunks)src/lib/pages/custom-network/json/index.tsx(1 hunks)src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityJsonInput.tsx(1 hunks)src/lib/pages/evm-contract-verify/components/vyper/EvmContractVerifyVyperJsonInput.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/components/dropzone/index.tsx
- src/lib/pages/evm-contract-verify/components/vyper/EvmContractVerifyVyperJsonInput.tsx
🔇 Additional comments (5)
src/lib/components/dropzone/config.ts (2)
3-3: LGTM! Type definition updated for JSON support.The change from "schema" to "json" aligns with the PR's objective to support standard JSON input for contract verification.
19-24: LGTM! JSON configuration properly defined.The configuration correctly specifies:
- MIME type: "application/json"
- File extension: ".json"
src/lib/pages/custom-network/json/index.tsx (1)
105-105: LGTM! DropZone configuration updated for JSON files.The fileType prop change aligns with the updated dropzone configuration.
src/lib/components/json-schema/UploadTemplate.tsx (1)
117-117: LGTM! DropZone configuration updated for JSON files.The fileType prop change aligns with the updated dropzone configuration.
src/lib/pages/evm-contract-verify/components/solidity/EvmContractVerifySolidityJsonInput.tsx (1)
8-39: LGTM! Well-structured component implementation.The component follows React best practices:
- Clear props interface definition
- Proper form integration using react-hook-form
- Consistent file handling with other components
Fixes FRO-764, FRO-755
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes