-
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: wireup verifications #1232
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis pull request introduces a new EVM verification feature documented in the changelog, enabling verification with multiparts and standard JSON inputs for both Solidity and Vyper. It simplifies form field naming in the optimizer configuration component, adds new type definitions for various verification inputs, and implements additional API methods to handle file uploads and JSON input. Furthermore, a utility function is added to ensure a contract address is a valid 20-byte hex string before executing queries. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Interface
participant API as EVM Verification API
participant SR as Server
U->>UI: Submit contract for verification
UI->>API: Call submitEvmVerify with selected input option
alt Solidity File Upload
API->>SR: submitEvmVerifySolidityUploadFiles(FormData)
else Vyper File Upload
API->>SR: submitEvmVerifyVyperUploadFiles(FormData)
else JSON Input
API->>SR: submitEvmVerifyJsonInput(Read JSON file)
end
SR-->>API: Return response
API-->>UI: Deliver verification result
UI-->>U: Display result
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/lib/pages/evm-contract-verify/components/HardhatInfoAccordion.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/lib/services/verification/evm/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/lib/pages/evm-contract-verify/components/FoundryInfoAccordion.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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: 1
🧹 Nitpick comments (2)
src/lib/services/verification/evm/api.ts (2)
101-151
: Add file size and type validation for Solidity uploads.While the implementation handles basic validation, consider adding:
- Maximum file size checks
- File type validation
- Maximum number of files limit
const submitEvmVerifySolidityUploadFiles = async ({ ... }: SubmitEvmVerifySolidityUploadFilesArgs) => { if (files.length === 0) throw new Error( "At least one file is required (submitEvmVerifySolidityUploadFiles)" ); + const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB + const MAX_FILES = 10; + const ALLOWED_TYPES = ['text/plain', 'text/solidity']; + + if (files.length > MAX_FILES) { + throw new Error(`Maximum ${MAX_FILES} files allowed`); + } + + files.forEach(({file}) => { + if (file.size > MAX_FILE_SIZE) { + throw new Error(`File ${file.name} exceeds maximum size of 10MB`); + } + if (!ALLOWED_TYPES.includes(file.type)) { + throw new Error(`File ${file.name} has invalid type ${file.type}`); + } + });
189-229
: Add file size and type validation for Vyper uploads.Similar to Solidity uploads, add validation for Vyper file uploads.
const submitEvmVerifyVyperUploadFiles = async ({ ... }: SubmitEvmVerifyVyperUploadFilesArgs) => { if (!file) throw new Error("File is required (submitEvmVerifyVyperUploadFiles)"); + + const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB + const ALLOWED_TYPES = ['text/plain', 'text/x-python']; + + if (file.size > MAX_FILE_SIZE) { + throw new Error(`File exceeds maximum size of 10MB`); + } + if (!ALLOWED_TYPES.includes(file.type)) { + throw new Error(`Invalid file type ${file.type}`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md
(1 hunks)src/lib/pages/evm-contract-verify/components/OptimizerConfiguration.tsx
(1 hunks)src/lib/services/types/verification/evm.ts
(1 hunks)src/lib/services/verification/evm/api.ts
(5 hunks)src/lib/services/verification/evm/index.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
src/lib/services/verification/evm/index.ts (1)
6-6
: LGTM! Enhanced validation for contract address.The addition of
isHex20Bytes
validation improves the robustness of the query by ensuring the contract address is valid before execution.Also applies to: 28-28
src/lib/services/types/verification/evm.ts (1)
20-38
: LGTM! Well-structured type definitions for verification inputs.The new types maintain a consistent pattern by extending
SubmitEvmVerifyBaseArgs
and properly typing the form fields for both Solidity and Vyper verification methods.src/lib/pages/evm-contract-verify/components/OptimizerConfiguration.tsx (1)
54-54
: Verify form data structure compatibility.The simplified field path
${name}.runs
assumes a flatter form structure. Please ensure this change is consistent with the form data handling in the parent components.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for references to the old field path pattern rg -A 2 "optimizerConfig\.runs"Length of output: 457
Form Field Path Consistency Confirmed
The search results show that the API file (
src/lib/services/verification/evm/api.ts
) correctly referencesoptimizerConfig.runs
(e.g., converting it to a number). This confirms that the flattened field path (${name}.runs
) aligns with the expected structure used in parent components.
- Parent handling: The API expects a flat structure with
optimizerConfig.runs
—consistent with the update in the component.- No evidence of nesting: There’s no indication that the field was previously accessed in a nested manner elsewhere.
src/lib/services/verification/evm/api.ts (1)
272-319
: LGTM! Comprehensive switch statement for verification options.The switch statement properly handles all verification options and maintains consistent error handling.
CHANGELOG.md (2)
42-42
: LGTM! Well-documented feature addition.The new feature entry is clear, concise, and follows the changelog format guidelines. It properly documents the addition of EVM verification support with multiparts and standard JSON input for both Solidity and Vyper.
1-29
: LGTM! Well-structured changelog guidelines.The changelog follows best practices by:
- Including clear guiding principles
- Following the Keep a Changelog format
- Using proper categorization of changes
- Maintaining semantic versioning
- Providing clear usage instructions
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 (3)
src/lib/services/verification/evm/api.ts (3)
91-143
: Add file validation checks.Consider adding the following validations to prevent potential issues:
- File size limits
- File type validation
- Maximum number of files limit
const submitEvmVerifySolidityUploadFiles = async ({ ... }: SubmitEvmVerifySolidityUploadFilesArgs) => { if (files.length === 0) throw new Error( "At least one file is required (submitEvmVerifySolidityUploadFiles)" ); + if (files.length > 100) { + throw new Error("Maximum 100 files allowed"); + } + + for (const file of files) { + if (file.file.size > 10 * 1024 * 1024) { + throw new Error(`File ${file.file.name} exceeds maximum size of 10MB`); + } + if (!file.file.name.endsWith('.sol')) { + throw new Error(`File ${file.file.name} must be a Solidity file (.sol)`); + } + } const formData = new FormData(); ...
181-221
: Add file validation checks for Vyper uploads.Similar to Solidity uploads, consider adding file validations.
const submitEvmVerifyVyperUploadFiles = async ({ ... }: SubmitEvmVerifyVyperUploadFilesArgs) => { if (!file) throw new Error("File is required (submitEvmVerifyVyperUploadFiles)"); + + if (file.size > 10 * 1024 * 1024) { + throw new Error("File exceeds maximum size of 10MB"); + } + if (!file.name.endsWith('.vy')) { + throw new Error("File must be a Vyper file (.vy)"); + } const formData = new FormData(); ...
264-311
: Add form validation before submission.Consider validating the form data before submission to ensure all required fields are present and properly formatted.
export const submitEvmVerify = async ({ option, verifyForm, ...rest }: SubmitEvmVerifyArgs) => { const verifierUrl = getVerifierUrl(option); + + // Validate common fields + if (!rest.chainId || !rest.contractAddress) { + throw new Error("Chain ID and contract address are required"); + } + + // Validate form data based on option + const validateForm = (data: unknown) => { + if (!data) { + throw new Error(`Form data is required for option: ${option}`); + } + }; switch (option) { case EvmVerifyOptions.SolidityContractCode: + validateForm(verifyForm.solidityContractCode); return submitEvmVerifySolidityContractCode({ verifierUrl, ...rest, ...verifyForm.solidityContractCode, }); // Add validation for other cases...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/services/verification/evm/api.ts
(6 hunks)src/lib/services/verification/evm/utils.ts
(2 hunks)src/lib/types/evm.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
src/lib/services/verification/evm/api.ts (2)
3-22
: LGTM! Well-organized imports.The new type imports and utility functions are properly organized and aligned with the new verification features.
224-262
: Add JSON validation and error handling.The JSON input handling could be improved with validation and better error messages.
src/lib/services/verification/evm/utils.ts (1)
25-25
: LGTM!The constant is appropriately named and its value correctly indicates its use for contract creation bytecode.
src/lib/types/evm.ts (1)
88-88
: LGTM!The type is correctly inferred from a well-defined schema that includes proper validation rules.
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 (1)
src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx (1)
63-67
: Enhance constructor arguments validation.The current validation only checks for empty strings. Consider:
- Adding validation for hex string format
- Making the error message more descriptive (e.g., "Constructor arguments must be a valid hex string")
- Adding length validation for the hex string
Example implementation:
- error={ - constructorArgsValue === "" - ? "Invalid constructor arguments" - : undefined - } + error={ + !constructorArgsValue + ? "Constructor arguments are required" + : !/^0x[0-9a-fA-F]*$/.test(constructorArgsValue) + ? "Constructor arguments must be a valid hex string starting with 0x" + : undefined + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/lib/pages/evm-contract-verify/components/ConstructorArgs.tsx (3)
20-23
: LGTM! Clean form state management.The use of template literals with the
.enabled
suffix provides clear and type-safe form state management.
25-28
: LGTM! Clear variable naming.The variable name
constructorArgsValue
better describes its purpose, and the form field path is well-structured.
41-44
: LGTM! Simplified checkbox state management.The direct usage of
field.value
and concise event handling improves code readability.
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.
Add TODO to commented UserDoc
Summary by CodeRabbit