1185 add generalized dynamic abi types generator #1209
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis update introduces a new Solidity contract generator functionality that creates contract code from JSON inputs. It adds a new function along with supporting types and helper functions in the generator package, and provides corresponding tests. A new command-line interface has been implemented for generating Solidity contracts with detailed flag handling. In addition, a stoic quote handler package has been added with methods for decoding input, fetching remote quotes, and encoding outputs, plus its associated tests. The build script has been updated to include targets for both price prediction and stoic quote contracts. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as Command-Line Interface
participant FS as File System
participant Gen as GenerateContract
U->>CLI: Run command with flags
CLI->>FS: Read input JSON (if provided)
CLI->>FS: Read output JSON (required)
CLI->>Gen: Call GenerateContract with parameters
Gen-->>CLI: Return Solidity code or error
CLI->>FS: Write Solidity code to output file
CLI-->>U: Log success or error
sequenceDiagram
participant H as StoicQuote Handler
participant DI as decodeInput
participant FR as fetchRemoteQuote
participant EO as encodeOutput
H->>DI: Decode input data
DI-->>H: Return decoded data or error
H->>FR: Fetch remote Stoic quote
FR-->>H: Return author & quote or error
H->>EO: Encode output with quote info
EO-->>H: Return encoded output
H-->>Caller: Return final output or error
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (1)`**/*.go`: Review the Golang code for conformity with the Ub...
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (7)
✨ 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
Documentation and Community
|
|
Hey @Svetomech and thank you for opening this pull request! 👋🏼 It looks like you forgot to add a changelog entry for your changes. Make sure to add a changelog entry in the 'CHANGELOG.md' 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 (8)
soliditygen/utils.go (2)
5-10: Add documentation and consider improving empty string handling.The function should:
- Have a documentation comment explaining its purpose and behavior
- Consider returning an empty string for empty input to maintain consistent behavior
- Have a more descriptive name like
capitalizeFirstto better indicate its purpose+// capitalizeFirst returns the input string with its first character capitalized. +// Returns an empty string if the input is empty. -func capitalize(str string) string { +func capitalizeFirst(str string) string { if str == "" { - return "Empty" + return "" } return strings.ToUpper(str[:1]) + str[1:] }
12-23: Add documentation and improve variable naming.The function should:
- Have a documentation comment explaining its purpose and behavior
- Use more descriptive variable names
- Consider using
strings.ContainsRunefor delimiter checking+// toPascalCase converts a string to PascalCase format by splitting on delimiters +// (hyphen, underscore, space) and capitalizing the first letter of each part. func toPascalCase(s string) string { + delimiters := "-_ " parts := strings.FieldsFunc(s, func(r rune) bool { - return r == '-' || r == '_' || r == ' ' + return strings.ContainsRune(delimiters, r) }) - for i, p := range parts { - if len(p) == 0 { + for i, part := range parts { + if len(part) == 0 { continue } - parts[i] = strings.ToUpper(p[:1]) + strings.ToLower(p[1:]) + parts[i] = strings.ToUpper(part[:1]) + strings.ToLower(part[1:]) } return strings.Join(parts, "") }soliditygen/generator.go (4)
19-21: Clarify overwriting behavior and add explicit mention of file creation.Consider explicitly stating in the docstring that this function will create or overwrite an existing Solidity file. This transparency helps callers avoid unintentional overwrites and clarifies the expected output behavior.
49-70: Ensure contract naming is globally unique.Appending "Types" to the user-supplied base might cause naming conflicts if multiple URLs share the same base path. If the project deals with multiple external sources, consider adding a unique prefix or short hash derived from the URL to avoid collisions.
79-85: Consider user prompts for overwriting existing files.Although writing the generated
.solfile is straightforward, you might add an optional parameter to prompt or confirm overwriting if the file already exists. This reduces accidental loss of existing contract code.
116-154: Improve numeric type inference and array handling.
- Currently, decimals map to
int256, which might be inaccurate if the JSON represents monetary amounts best handled with fixed-point libraries.- Arrays are always unbounded. If the JSON data implies a certain length or a specialized type of array, you may want to further refine or verify the design.
soliditygen/StoicQuoteTypes.sol (2)
4-8: Consider including additional documentation.While the contract comment is helpful at a high level, future users might appreciate more context about how the contract is generated and what typical usage scenarios look like.
18-20: Assess whether this dummy function meets your testing or usage needs.
useAllTypesreferences the top-level struct but does nothing else. Confirm it is sufficient for your integration or testing, e.g., if you need to confirm run-time behavior rather than just type references.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
soliditygen/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
soliditygen/StoicQuoteTypes.sol(1 hunks)soliditygen/generator.go(1 hunks)soliditygen/generator_test.go(1 hunks)soliditygen/go.mod(1 hunks)soliditygen/utils.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- soliditygen/go.mod
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
soliditygen/utils.gosoliditygen/generator_test.gosoliditygen/generator.go
`**/*_test.go`: "Assess the unit test code assessing suffici...
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
soliditygen/generator_test.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
soliditygen/utils.go (1)
1-4: LGTM!Package structure and imports follow the Uber style guide.
soliditygen/generator_test.go (1)
19-42: Excellent coverage of generated contract structure.The assertions appropriately check for the presence of crucial contract elements (e.g., top-level struct, fields, and dummy function). This ensures fidelity to the generated code’s intended format.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
soliditygen/cmd/main.go (2)
16-18: Add input validation for command-line flags.Consider validating:
- URL format for
urlFlag- Contract name format (should be a valid Solidity identifier)
- Directory existence and write permissions for
contractDirFlagfunc main() { urlFlag := flag.String("url", "", "URL to fetch JSON from") contractFlag := flag.String("contractName", "", "Base contract name") contractDirFlag := flag.String("contractDir", ".", "Directory where the .sol file will be written") + + flag.Parse() + + if *urlFlag == "" || *contractFlag == "" { + fmt.Fprintln(os.Stderr, "Usage: go run cmd.go --url=<JSON_URL> --contractName=<ContractName> [--contractDir=<OutputDirectory>]") + os.Exit(1) + } + + if _, err := url.Parse(*urlFlag); err != nil { + log.Fatalf("Invalid URL format: %v", err) + } + + if !regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9]*$`).MatchString(*contractFlag) { + log.Fatal("Contract name must be a valid Solidity identifier") + } + + if info, err := os.Stat(*contractDirFlag); err != nil || !info.IsDir() { + log.Fatalf("Invalid output directory: %v", err) + }
27-30: Consider providing more detailed error information.The error message could include the URL and contract name to help with debugging.
- log.Fatalf("Failed to generate Solidity file: %v", err) + log.Fatalf("Failed to generate Solidity file for URL %s and contract %s: %v", *urlFlag, *contractFlag, err)prophet/justfile (1)
20-27: Add error handling and a clean target for generated files.Consider:
- Adding error handling for the go run command
- Creating a clean target to remove generated files
stoicquote: _tools - go run ../soliditygen/cmd/main.go --url="https://stoic.tekloon.net/stoic-quote" --contractName="StoicQuote" --contractDir=handlers/stoicquote/generated + go run ../soliditygen/cmd/main.go --url="https://stoic.tekloon.net/stoic-quote" --contractName="StoicQuote" --contractDir=handlers/stoicquote/generated || (echo "Failed to generate Solidity file" && exit 1) solc --abi --bin --overwrite handlers/stoicquote/generated/StoicQuoteTypes.sol -o build abigen \ --bin=build/StoicQuoteTypes.bin \ --abi=build/StoicQuoteTypes.abi \ --pkg=generated \ --type=StoicQuoteTypes \ --out=handlers/stoicquote/generated/stoicquote_bindings.go +clean: + rm -f handlers/stoicquote/generated/StoicQuoteTypes.sol + rm -f handlers/stoicquote/generated/stoicquote_bindings.go + rm -f build/StoicQuoteTypes.bin build/StoicQuoteTypes.abiprophet/handlers/stoicquote/stoicquote.go (1)
91-93: Remove commented code.The commented code in
encodeOutputshould either be implemented or removed.- // packed, err := method.Outputs.Pack(outStoicQuote) - // return packed, errsoliditygen/generator.go (1)
122-129: Improve numeric type inference.The current implementation uses int256/uint256 for all numbers. Consider using more specific types based on the value range.
case float64: if val == float64(int64(val)) { if val < 0 { - return "int256", nil + switch { + case val >= -128: return "int8", nil + case val >= -32768: return "int16", nil + case val >= -2147483648: return "int32", nil + case val >= -9223372036854775808: return "int64", nil + default: return "int256", nil + } } - return "uint256", nil + switch { + case val <= 255: return "uint8", nil + case val <= 65535: return "uint16", nil + case val <= 4294967295: return "uint32", nil + case val <= 18446744073709551615: return "uint64", nil + default: return "uint256", nil + } } return "int256", nil
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
prophet/handlers/stoicquote/generated/StoicQuoteTypes.solis excluded by!**/generated/**prophet/handlers/stoicquote/generated/stoicquote_bindings.gois excluded by!**/generated/**
📒 Files selected for processing (6)
prophet/handlers/stoicquote/stoicquote.go(1 hunks)prophet/handlers/stoicquote/stoicquote_test.go(1 hunks)prophet/justfile(2 hunks)soliditygen/cmd/main.go(1 hunks)soliditygen/generator.go(1 hunks)soliditygen/generator_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- soliditygen/generator_test.go
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
prophet/handlers/stoicquote/stoicquote_test.gosoliditygen/cmd/main.gosoliditygen/generator.goprophet/handlers/stoicquote/stoicquote.go
`**/*_test.go`: "Assess the unit test code assessing suffici...
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
prophet/handlers/stoicquote/stoicquote_test.go
🔇 Additional comments (2)
soliditygen/generator.go (2)
1-11: LGTM! Package and imports are well-organized.The package name is descriptive and imports are properly grouped.
154-159: LGTM! The capitalize function is well-implemented.The function properly handles edge cases and follows Go conventions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
soliditygen/cmd/cmd.go (1)
1-33: 🛠️ Refactor suggestionConsider supporting JSON input from stdin.
As suggested in the past review, the tool would be more flexible if it could accept JSON input from stdin in addition to URLs. This would allow local file processing and support for different HTTP methods.
Example implementation:
+ stdinFlag := flag.Bool("stdin", false, "Read JSON from stdin instead of URL") + flag.Parse() - if *urlFlag == "" || *contractFlag == "" { + if (*urlFlag == "" && !*stdinFlag) || *contractFlag == "" { fmt.Fprintln(os.Stderr, "Usage: go run cmd.go --url=<JSON_URL> --contractName=<ContractName> [--contractDir=<OutputDirectory>]") os.Exit(1) } - solFile, err := soliditygen.WriteSolidityFromURL(*urlFlag, *contractFlag, *contractDirFlag) + var err error + var solFile string + + if *stdinFlag { + solFile, err = soliditygen.WriteSolidityFromReader(os.Stdin, *contractFlag, *contractDirFlag) + } else { + solFile, err = soliditygen.WriteSolidityFromURL(*urlFlag, *contractFlag, *contractDirFlag) + }soliditygen/generator.go (4)
13-16: 🛠️ Refactor suggestionAdd documentation for exported Definition struct.
According to the Uber style guide, all exported types should be documented.
+// Definition represents a Solidity struct definition with its name and text representation. type Definition struct { + // Name is the identifier of the Solidity struct Name string + // Text is the complete Solidity struct definition Text string }
31-34: 🛠️ Refactor suggestionAdd size limit for response body.
To prevent potential memory exhaustion from large responses, consider using
io.LimitReader.-bodyBytes, err := io.ReadAll(resp.Body) +bodyBytes, err := io.ReadAll(io.LimitReader(resp.Body, 10*1024*1024)) // 10MB limit
113-114: 🛠️ Refactor suggestionUse int256 for all numeric values.
As noted in past review comments, assuming positive numbers based on the current value is unsafe. Always use int256 for consistency and safety.
90-95: 🛠️ Refactor suggestionValidate field names in struct generation.
Field names should be validated to ensure they follow Solidity naming conventions.
for key, val := range value { + if !isValidSolidityIdentifier(key) { + return nil, fmt.Errorf("invalid field name: %s", key) + } solType, nestedDefs := inferSolidityType(key, val)
🧹 Nitpick comments (8)
prophet/handlers/pricepred/pricepred_test.go (3)
17-27: Consider adding more test cases.While the type changes look good, the test suite currently only has a single test case. Consider adding more test cases to cover:
- Empty token list
- Maximum number of tokens
- Invalid metrics
- Edge cases for FalsePositiveRate
50-54: LGTM! Consider additional error cases.The test case is comprehensive and properly validates the output structure. The precision handling with big.Int is correct.
Consider adding test cases for:
- Error scenarios where buildOutputData might fail
- Edge cases with very large or very small numbers
- Cases with nil backtesting response
Also applies to: 58-59, 103-114
131-138: Add more detailed documentation.While the test logic is sound, the documentation could be improved:
- Add a more detailed description of the test's purpose
- Document the expected relationship between MetricName enum values and BacktestingMetrics struct fields
- Consider adding constants or examples showing the mapping
prophet/handlers/pricepred/pricepred.go (3)
114-143: LGTM! Function changes maintain proper error handling.The transition to generated types is implemented correctly with proper error handling and type safety.
Consider making the error message on line 126 more specific by including the actual method name:
- return in.InputData, fmt.Errorf("method 'solve' not found in generated ABI") + return in.InputData, fmt.Errorf("method 'solve' not found in PricePredictorTypes ABI")
145-162: LGTM! Function changes maintain consistency with decodeInput.The transition to generated types is implemented correctly with proper error handling.
Consider making the error message on line 153 more specific, similar to the suggestion for decodeInput:
- return nil, fmt.Errorf("method 'solve' not found in generated ABI") + return nil, fmt.Errorf("method 'solve' not found in PricePredictorTypes ABI")
164-242: Consider extracting metric conversion logic.The function is well-implemented but the large switch statement (lines 180-232) could be extracted into a separate function for better maintainability.
Consider extracting the metric conversion logic:
+func convertMetric(m MetricName, metrics TokenMetrics, decimals *big.Float) (*big.Int, error) { + switch m { + case Count: + return big.NewInt(int64(metrics.Count)), nil + case Mape: + return float64ToBigInt(metrics.Mape, decimals), nil + // ... other cases ... + default: + return nil, fmt.Errorf("invalid requested metric: %d", m) + } +} func buildOutputData(...) (generated.PricePredictorTypesOutputData, error) { // ... existing code ... for j, m := range inputData.Metrics { m := MetricNameFromBigInt(m) - switch m { - // ... large switch statement ... - } + metric, err := convertMetric(m, tokenMetrics.Metrics, decimals) + if err != nil { + return generated.PricePredictorTypesOutputData{}, err + } + metrics[i][j] = metric } // ... rest of the function ... }soliditygen/cmd/cmd.go (1)
12-15: Enhance usage documentation.The current usage example could be more comprehensive. Consider adding examples for all supported flags and their default values.
// Usage Example: // -// go run cmd.go --url=<JSON URL> --contractName=<Name> --contractDir=<OutputDirectory> +// Generate Solidity types from a JSON URL: +// go run cmd.go --url=<JSON URL> --contractName=<Name> +// +// Specify custom output directory: +// go run cmd.go --url=<JSON URL> --contractName=<Name> --contractDir=./contracts +// +// Flags: +// --url Required. URL to fetch JSON from +// --contractName Required. Base name for the generated contract +// --contractDir Optional. Output directory (default: current directory)soliditygen/generator_test.go (1)
28-28: Add cleanup for generated files in case of test failures.The deferred cleanup might not run if the test fails before reaching it. Consider using
t.Cleanup()instead.- defer os.Remove(solFile) + t.Cleanup(func() { + if solFile != "" { + os.Remove(solFile) + } + })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
prophet/handlers/pricepred/generated/pricepred_bindings.gois excluded by!**/generated/**
📒 Files selected for processing (8)
prophet/handlers/pricepred/PricePredictorTypes.sol(1 hunks)prophet/handlers/pricepred/pricepred.go(5 hunks)prophet/handlers/pricepred/pricepred_bindings.go(0 hunks)prophet/handlers/pricepred/pricepred_test.go(4 hunks)prophet/justfile(1 hunks)soliditygen/cmd/cmd.go(1 hunks)soliditygen/generator.go(1 hunks)soliditygen/generator_test.go(1 hunks)
💤 Files with no reviewable changes (1)
- prophet/handlers/pricepred/pricepred_bindings.go
✅ Files skipped from review due to trivial changes (1)
- prophet/handlers/pricepred/PricePredictorTypes.sol
🚧 Files skipped from review as they are similar to previous changes (1)
- prophet/justfile
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
soliditygen/cmd/cmd.gosoliditygen/generator_test.gosoliditygen/generator.goprophet/handlers/pricepred/pricepred.goprophet/handlers/pricepred/pricepred_test.go
`**/*_test.go`: "Assess the unit test code assessing suffici...
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
soliditygen/generator_test.goprophet/handlers/pricepred/pricepred_test.go
🔇 Additional comments (5)
prophet/handlers/pricepred/pricepred_test.go (2)
10-10: LGTM!The import statement for the generated types package is correctly formatted and follows Go conventions.
1-167: Test coverage looks good!The test suite provides good coverage of the type changes:
- Input/output validation
- Error handling
- Edge cases with big.Int
- Comprehensive backtesting scenarios
prophet/handlers/pricepred/pricepred.go (2)
11-13: LGTM! Import changes are well-structured.The import of the generated package follows Go conventions and is properly grouped.
249-252: Implement output verification.The TODO comment indicates missing verification logic. This could be a security concern as unverified outputs might lead to incorrect predictions being accepted.
Would you like me to help implement the verification logic for the Verify method?
soliditygen/generator.go (1)
73-80: Simplify output file handling.Follow CLI tool conventions by using a single output flag for the full file path.
- solFileName := filepath.Join(contractDirectory, contractName+".sol") - - if err := os.MkdirAll(contractDirectory, 0o755); err != nil { - return "", fmt.Errorf("creating directory: %w", err) - } if err := os.WriteFile(solFileName, []byte(solBuilder.String()), 0o644); err != nil { return "", fmt.Errorf("writing solidity file: %w", err) }
This reverts commit 628965b.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
prophet/justfile (1)
19-39: Consider making the API URL configurable.The Stoic quote API URL is hardcoded. Consider making it configurable through an environment variable or command-line argument for better flexibility and maintainability.
- curl "https://stoic.tekloon.net/stoic-quote" \ + curl "${STOIC_QUOTE_API_URL:-https://stoic.tekloon.net/stoic-quote}" \soliditygen/cmd/cmd.go (1)
104-109: Add timeout for stdin reading.When reading from stdin, the program could hang indefinitely. Consider adding a timeout.
func readAllOrStdin(filePath string) ([]byte, error) { if filePath == "-" { - return io.ReadAll(os.Stdin) + ch := make(chan []byte, 1) + errCh := make(chan error, 1) + go func() { + data, err := io.ReadAll(os.Stdin) + if err != nil { + errCh <- err + return + } + ch <- data + }() + select { + case data := <-ch: + return data, nil + case err := <-errCh: + return nil, err + case <-time.After(30 * time.Second): + return nil, fmt.Errorf("timeout reading from stdin") + } } return os.ReadFile(filePath) }soliditygen/generator.go (2)
144-149: Improve error message in capitalize function.The panic message could be more descriptive about the context and requirements.
func capitalize(str string) string { if len(str) == 0 { - panic("invalid input: string cannot be empty") + panic("invalid field name: cannot be empty") } return strings.ToUpper(str[:1]) + str[1:] }
65-86: Consider using a separate template file.The contract template is quite large. Consider moving it to a separate
.tmplfile for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
prophet/handlers/stoicquote/generated/StoicQuote.solis excluded by!**/generated/**prophet/handlers/stoicquote/generated/stoicquote_bindings.gois excluded by!**/generated/**
📒 Files selected for processing (6)
prophet/handlers/stoicquote/stoicquote.go(1 hunks)prophet/handlers/stoicquote/stoicquote_test.go(1 hunks)prophet/justfile(2 hunks)soliditygen/cmd/cmd.go(1 hunks)soliditygen/generator.go(1 hunks)soliditygen/generator_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- prophet/handlers/stoicquote/stoicquote_test.go
- soliditygen/generator_test.go
- prophet/handlers/stoicquote/stoicquote.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
soliditygen/cmd/cmd.gosoliditygen/generator.go
🔇 Additional comments (3)
prophet/justfile (1)
4-4: LGTM! Target updated to include new stoicquote.The
alltarget correctly includes bothpricepredandstoicquotetargets.soliditygen/generator.go (2)
114-115: Use int256 for all numeric values.As suggested in past reviews, using
int256for all numeric values is safer than trying to infer the type based on the value.
93-97: Add validation for Solidity field names.Field names should be validated to ensure they follow Solidity naming conventions.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
soliditygen/generator.go (1)
93-97: Add field name validation in struct generation.Field names should be validated to ensure they follow Solidity naming conventions.
for key, val := range value { + if !isValidSolidityIdentifier(key) { + return nil, fmt.Errorf("invalid field name: %s", key) + } solType, nestedDefs := inferSolidityType(key, val) childDefs = append(childDefs, nestedDefs...) lines = append(lines, fmt.Sprintf(" %s %s;", solType, key)) }📝 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.for key, val := range value { if !isValidSolidityIdentifier(key) { return nil, fmt.Errorf("invalid field name: %s", key) } solType, nestedDefs := inferSolidityType(key, val) childDefs = append(childDefs, nestedDefs...) lines = append(lines, fmt.Sprintf(" %s %s;", solType, key)) }
🧹 Nitpick comments (1)
soliditygen/generator_test.go (1)
22-33: Use more precise assertions instead of Contains.Using string contains for testing Solidity code structure is fragile. Consider using regex or AST parsing for more robust assertions.
- require.Contains(t, contract, "contract PricePredictor") + require.Regexp(t, `\bcontract\s+PricePredictor\s*{`, contract) - require.Contains(t, contract, "struct RequestData") + require.Regexp(t, `\bstruct\s+RequestData\s*{`, contract)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
prophet/handlers/stoicquote/generated/stoicquote_bindings.gois excluded by!**/generated/**
📒 Files selected for processing (4)
prophet/handlers/stoicquote/stoicquote.go(1 hunks)prophet/justfile(2 hunks)soliditygen/generator.go(1 hunks)soliditygen/generator_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- prophet/handlers/stoicquote/stoicquote.go
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Golang code for conformity with the Ub...
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
soliditygen/generator_test.gosoliditygen/generator.go
`**/*_test.go`: "Assess the unit test code assessing suffici...
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
soliditygen/generator_test.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
prophet/justfile (1)
8-8: Verify the fixed version of abigen.Let's verify if v1.15.0 is the latest stable version and check for any security advisories.
✅ Verification successful
🌐 Web query:
What is the latest stable version of go-ethereum's abigen tool?💡 Result:
The latest stable version of go-ethereum's abigen tool is v1.15.0, which comes bundled with Geth (Go Ethereum) as part of its official releases[9]. This version was listed as the current operative package on Ethereum's official downloads page when accessed in February 2025[3][9].
To verify and install this version:
# Install via Go modules go install github.com/ethereum/go-ethereum/cmd/[email protected]Key details about this release:
- Includes security updates and stability improvements from previous versions
- Maintains compatibility with Ethereum Improvement Proposals up to EIP-7212
- Supports generating bindings for Solidity contracts up to version 0.8.24[5][7]
The abigen tool remains part of Geth's core developer tools, with installation methods including both precompiled binaries and source builds through Go's module system[4][7]. For production use, Ethereum Foundation recommends verifying downloads using their PGP signatures listed in the release notes[3][9].
Citations:
- 1: https://blog.blockmagnates.com/deploy-smart-contract-and-connect-with-go-ethereum-935f4290249f?gi=aa7e3a47efd7
- 2: https://www.metachris.com/2021/05/creating-go-bindings-for-ethereum-smart-contracts/
- 3: https://ethereumpow.github.io/go-ethereum/downloads/
- 4: https://geth.ethereum.org/docs/tools/abigen
- 5: https://goethereumbook.org/en/smart-contract-compile/
- 6: https://www.quicknode.com/guides/infrastructure/node-setup/how-to-install-and-run-a-geth-node
- 7: https://geth.ethereum.org/docs/developers/dapp-developer/native-bindings
- 8: https://github.com/ethereum/go-ethereum/blob/master/cmd/abigen/main.go
- 9: https://geth.ethereum.org/downloads
- 10: https://build.avax.network/docs/dapps/smart-contract-dev/interact-golang-app
Fixed version of abigen is confirmed as v1.15.0.
Our web query shows that the latest stable version is indeed v1.15.0, with all associated security updates and improvements. No adjustments to the version are necessary.soliditygen/generator.go (1)
71-71: Consider using a more conservative Solidity version range.The pragma version range
>=0.8.25 <0.9.0might be too wide. Consider pinning to a specific version or using a narrower range.✅ Verification successful
🌐 Web query:
What is the latest stable version of Solidity?💡 Result:
The latest stable version of Solidity is v0.8.28, released on October 9, 2024[1][2][5]. This version introduces full support for transient storage state variables of value types, reduces memory usage by generating Yul AST JSON representations only on demand, and includes critical bug fixes such as resolving SMT logic errors in array assignments[1][2].
Key updates in v0.8.28:
- Transient storage support: State variables of value types can now use
tload/tstorefor temporary data storage during transactions[1][2].- Compiler optimizations: Reduced memory usage and improved efficiency via IR pipeline optimizations[1].
- Bug fixes: Addressed issues in SMTChecker array handling and Yul AST source location accuracy[1].
Earlier versions like v0.8.27 (September 2024) added legacy pipeline support for custom errors in
requirestatements[2], while v0.8.24 (January 2024) introduced Cancun upgrade readiness with EIP-1153 and EIP-4844 support[1]. Always use the latest stable release for security and feature updates[6].Citations:
- 1: https://github.com/ethereum/solidity/releases
- 2: https://soliditylang.org/blog/category/releases/
- 3: https://readthedocs.org/projects/solidity/
- 4: https://docs.openzeppelin.com/contracts/2.x/releases-stability
- 5: https://soliditylang.org
- 6: https://docs.soliditylang.org/en/latest/
- 7: https://docs.soliditylang.org/en/latest/installing-solidity.html
- 8: https://github.com/ethereum/solidity/blob/develop/Changelog.md
- 9: https://platon-solidity.readthedocs.io/en/latest/installing-solidity.html
Refine Solidity version range for tighter control
The current version range (>=0.8.25 <0.9.0) accepts multiple patch versions—including older ones—while the latest stable version is v0.8.28. To minimize potential inconsistencies and unintended behavior from older patches, consider pinning to a specific version (e.g.,pragma solidity 0.8.28;) or using a narrower range (e.g.,pragma solidity ^0.8.28;).
| func (h Handler) Execute(ctx context.Context, input []byte) ([]byte, error) { | ||
| inStoicQuote, err := decodeInput(input) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("decode input: %w", err) | ||
| } |
There was a problem hiding this comment.
what would be the input in this case? i think it should be empty as the request to the API doesn't require any parameter
| // packed, err := method.Outputs.Pack(outStoicQuote) | ||
| // return packed, err | ||
|
|
||
| return []byte{}, nil |
There was a problem hiding this comment.
looks like we are not returning the right thing?
| } | ||
|
|
||
| func fetchRemoteQuote(ctx context.Context, url string) (author, quote string, err error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) |
There was a problem hiding this comment.
let's make sure to have a hard timeout on the context to avoid being held forever by the server, 3 seconds should be plenty of time
Notes
Added JSON -> Solidity generator and an example "stoic-quotes" handler demonstrating its usage. Considering the differences between JSON and Solidity type systems, there are some corner/edge cases when this would not generate appropriate types and will use a string fallback instead
Summary by CodeRabbit
New Features
• Introduced a command-line tool that transforms JSON input into Solidity contracts with improved error handling.
• Integrated a feature to fetch and incorporate inspirational Stoic quotes from an external service into generated contracts.
• Enhanced build automation with updated targets to streamline multiple contract generation workflows.
• Added new methods for generating Solidity contracts based on structured JSON input.
• Implemented a handler for processing Stoic quotes, including input decoding and output encoding.
Bug Fixes
• Improved error handling during contract generation and data fetching processes.
Tests
• Added unit tests for contract generation and Stoic quote handling to ensure functionality and reliability.
• Enhanced test coverage for contract generation with various input scenarios.