Add golang bindgen support #3239
Conversation
WalkthroughOhayo sensei! This update introduces support for generating Go language bindings in the build process. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (sozo build)
participant PluginManager
participant GolangPlugin
User->>CLI (sozo build): Run build command with --golang flag
CLI (sozo build)->>PluginManager: Initialize with Golang plugin
PluginManager->>GolangPlugin: Generate Go bindings
GolangPlugin-->>PluginManager: Return generated Go code
PluginManager-->>CLI (sozo build): Provide Go binding output
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
crates/dojo/bindgen/src/plugins/golang/mod.rs (2)
88-104: Consider making the package name configurable.Ohayo sensei! The imports are hardcoded to use specific StarkNet Go packages. Consider allowing configuration of the import paths for flexibility.
You might want to add a configuration option to customize the StarkNet library imports, as different users might use different versions or forks.
82-84: Consider using a more specific timestamp format.Ohayo sensei! The generated header uses RFC2822 format which might not be the most readable. Consider using ISO 8601 format for better consistency.
- chrono::Utc::now().to_rfc2822() + chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC").to_string()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/simple/Scarb.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
bin/sozo/src/commands/build.rs(4 hunks)crates/dojo/bindgen/src/lib.rs(2 hunks)crates/dojo/bindgen/src/plugins/golang/mod.rs(1 hunks)crates/dojo/bindgen/src/plugins/mod.rs(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
crates/dojo/bindgen/src/lib.rs (3)
crates/dojo/bindgen/src/plugins/mod.rs (1)
new(45-47)crates/dojo/bindgen/src/plugins/golang/mod.rs (1)
new(16-18)crates/dojo/bindgen/src/plugins/unrealengine/mod.rs (1)
new(16-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (10)
bin/sozo/src/commands/build.rs (4)
8-8: LGTM! Import reordering looks good.Ohayo sensei! The import reordering maintains functionality while improving organization.
145-147: LGTM! Plugin initialization follows the established pattern.The implementation correctly adds the Golang plugin to the builtin plugins list when the flag is set.
234-234: LGTM! Default value correctly set.The golang field is properly initialized to
falsein the Default implementation.
41-43:⚠️ Potential issueFlag description has incorrect copy-paste text.
Ohayo! The help text says "Generate Typescript bindings" but this is for Go bindings.
- #[arg(help = "Generate Typescript bindings.")] + #[arg(help = "Generate Go bindings.")]Likely an incorrect or invalid review comment.
crates/dojo/bindgen/src/lib.rs (2)
20-20: LGTM! Import follows the established pattern.Ohayo sensei! The GolangPlugin import is correctly placed with the other plugin imports.
109-109: LGTM! Match arm correctly instantiates the plugin.The implementation follows the same pattern as other plugins, using
Box::new(GolangPlugin::new()).crates/dojo/bindgen/src/plugins/mod.rs (3)
17-17: LGTM! Module declaration follows the pattern.Ohayo! The golang module is correctly declared alongside other plugin modules.
26-26: LGTM! Enum variant properly added.The Golang variant is correctly added to the BuiltinPlugins enum.
37-37: LGTM! Display implementation is consistent.The display implementation correctly returns "golang" following the naming convention of other plugins.
crates/dojo/bindgen/src/plugins/golang/mod.rs (1)
355-380: Struct flattening logic is well implemented.Ohayo sensei! The logic to flatten struct fields when generating calldata is elegant and handles the Cairo-to-Go translation nicely.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::{DojoData, DojoWorld}; | ||
|
|
||
| #[tokio::test] | ||
| async fn test_golang_plugin_generation() { | ||
| // Create a simple test data structure | ||
| let world = DojoWorld { name: "test_world".to_string() }; | ||
|
|
||
| let data = DojoData { | ||
| world, | ||
| models: HashMap::new(), | ||
| contracts: HashMap::new(), | ||
| events: HashMap::new(), | ||
| }; | ||
|
|
||
| let plugin = GolangPlugin::new(); | ||
| let result = plugin.generate_code(&data).await; | ||
|
|
||
| assert!(result.is_ok()); | ||
| let files = result.unwrap(); | ||
| assert_eq!(files.len(), 1); | ||
|
|
||
| // Check that the generated file has the correct name | ||
| let (path, content) = files.iter().next().unwrap(); | ||
| assert_eq!(path.to_str().unwrap(), "test_world.go"); | ||
|
|
||
| // Check that the content contains expected Go code | ||
| let content_str = String::from_utf8(content.clone()).unwrap(); | ||
| assert!(content_str.contains("package test_world")); | ||
| assert!(content_str.contains("import (")); | ||
| assert!(content_str.contains("type TestWorldClient struct")); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test coverage is minimal.
Ohayo! The test only verifies basic file generation but doesn't test the actual code generation logic for models, contracts, or type mappings.
Consider adding more comprehensive tests:
- Test type mapping for various Cairo types
- Test struct and enum generation
- Test contract method generation with different parameter types
- Test error cases
🤖 Prompt for AI Agents
In crates/dojo/bindgen/src/plugins/golang/mod.rs around lines 463 to 497, the
existing test only checks basic file generation and does not cover the detailed
code generation logic. To fix this, expand the test suite by adding tests that
verify type mappings for various Cairo types, struct and enum generation
correctness, contract method generation with diverse parameter types, and
handling of error cases. This will ensure comprehensive coverage of the
GolangPlugin's code generation functionality.
| "tuple" => { | ||
| if let Token::Tuple(tuple) = token { | ||
| // Go doesn't have native tuple support, so we'll create a struct | ||
| let fields = tuple | ||
| .inners | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, t)| format!("Field{} {}", i + 1, GolangPlugin::map_type(t))) | ||
| .collect::<Vec<String>>() | ||
| .join("; "); | ||
| format!("struct {{ {} }}", fields) | ||
| } else { | ||
| panic!("Invalid tuple token: {:?}", token); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tuple mapping creates anonymous structs which may cause issues.
Ohayo! Creating anonymous structs for tuples could lead to type compatibility issues in Go. Consider generating named tuple types instead.
"tuple" => {
if let Token::Tuple(tuple) = token {
- // Go doesn't have native tuple support, so we'll create a struct
- let fields = tuple
- .inners
- .iter()
- .enumerate()
- .map(|(i, t)| format!("Field{} {}", i + 1, GolangPlugin::map_type(t)))
- .collect::<Vec<String>>()
- .join("; ");
- format!("struct {{ {} }}", fields)
+ // Generate a named tuple type that can be defined separately
+ format!("Tuple{}Fields", tuple.inners.len())
} else {
panic!("Invalid tuple token: {:?}", token);
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/dojo/bindgen/src/plugins/golang/mod.rs between lines 43 and 57, the
current tuple mapping creates anonymous structs which can cause type
compatibility problems in Go. Modify the code to generate named struct types for
tuples instead of anonymous ones. This involves defining a named struct type
with fields corresponding to the tuple elements and using that named type
wherever the tuple is referenced.
| fn map_type(token: &Token) -> String { | ||
| match token.type_name().as_str() { | ||
| "bool" => "bool".to_string(), | ||
| "u8" => "uint8".to_string(), | ||
| "u16" => "uint16".to_string(), | ||
| "u32" => "uint32".to_string(), | ||
| "u64" => "uint64".to_string(), | ||
| "u128" => "*big.Int".to_string(), | ||
| "u256" => "*big.Int".to_string(), | ||
| "usize" => "uint".to_string(), | ||
| "felt252" => "string".to_string(), | ||
| "bytes31" => "string".to_string(), | ||
| "ClassHash" => "string".to_string(), | ||
| "ContractAddress" => "string".to_string(), | ||
| "ByteArray" => "string".to_string(), | ||
| "array" => { | ||
| if let Token::Array(array) = token { | ||
| format!("[]{}", GolangPlugin::map_type(&array.inner)) | ||
| } else { | ||
| panic!("Invalid array token: {:?}", token); | ||
| } | ||
| } | ||
| "tuple" => { | ||
| if let Token::Tuple(tuple) = token { | ||
| // Go doesn't have native tuple support, so we'll create a struct | ||
| let fields = tuple | ||
| .inners | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, t)| format!("Field{} {}", i + 1, GolangPlugin::map_type(t))) | ||
| .collect::<Vec<String>>() | ||
| .join("; "); | ||
| format!("struct {{ {} }}", fields) | ||
| } else { | ||
| panic!("Invalid tuple token: {:?}", token); | ||
| } | ||
| } | ||
| "generic_arg" => { | ||
| if let Token::GenericArg(generic_arg) = &token { | ||
| generic_arg.clone() | ||
| } else { | ||
| panic!("Invalid generic_arg token: {:?}", token); | ||
| } | ||
| } | ||
| _ => { | ||
| let mut type_name = token.type_name(); | ||
|
|
||
| if let Token::Composite(composite) = token { | ||
| if !composite.generic_args.is_empty() { | ||
| // Go doesn't support generics in the same way, we'll use any for now | ||
| type_name = format!("{}Generic", type_name); | ||
| } | ||
| } | ||
|
|
||
| type_name | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type mapping implementation needs improvements.
Ohayo sensei! The type mapping function has several issues:
- Using
panic!for error handling is not ideal in production code - The generic handling with "Generic" suffix is too simplistic
- Missing proper error propagation
Consider returning a Result<String, Error> instead:
- fn map_type(token: &Token) -> String {
+ fn map_type(token: &Token) -> Result<String, BindgenError> {
match token.type_name().as_str() {
- "bool" => "bool".to_string(),
+ "bool" => Ok("bool".to_string()),
// ... other simple types
"array" => {
if let Token::Array(array) = token {
- format!("[]{}", GolangPlugin::map_type(&array.inner))
+ Ok(format!("[]{}", GolangPlugin::map_type(&array.inner)?))
} else {
- panic!("Invalid array token: {:?}", token);
+ Err(BindgenError::InvalidToken(format!("Expected array token, got {:?}", token)))
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/dojo/bindgen/src/plugins/golang/mod.rs between lines 21 and 78, the
map_type function currently uses panic! for error handling, has simplistic
generic type handling, and lacks proper error propagation. Refactor the function
to return a Result<String, Error> instead of a plain String. Replace all panic!
calls with appropriate error returns. Improve generic type handling by providing
more meaningful mappings or error messages. Ensure all match arms return Result
types consistently and propagate errors properly.
| fn format_system_method( | ||
| contract_name: &str, | ||
| system: &Function, | ||
| handled_tokens: &[Composite], | ||
| ) -> String { | ||
| let method_name = system.name.to_case(convert_case::Case::Pascal); | ||
|
|
||
| // Generate method parameters | ||
| let params = system | ||
| .inputs | ||
| .iter() | ||
| .map(|(name, token)| { | ||
| let param_name = name.to_case(convert_case::Case::Camel); | ||
| let param_type = GolangPlugin::map_type(token); | ||
| format!("{} {}", param_name, param_type) | ||
| }) | ||
| .collect::<Vec<String>>(); | ||
|
|
||
| let params_str = if params.is_empty() { | ||
| "ctx context.Context".to_string() | ||
| } else { | ||
| format!("ctx context.Context, {}", params.join(", ")) | ||
| }; | ||
|
|
||
| // Generate calldata | ||
| let calldata = system | ||
| .inputs | ||
| .iter() | ||
| .map(|(name, token)| { | ||
| let param_name = name.to_case(convert_case::Case::Camel); | ||
|
|
||
| // Check if this is a struct that needs to be flattened | ||
| match handled_tokens.iter().find(|t| t.type_name() == token.type_name()) { | ||
| Some(t) if t.r#type == CompositeType::Struct => { | ||
| // Flatten struct fields | ||
| t.inners | ||
| .iter() | ||
| .map(|field| { | ||
| format!( | ||
| "{}.{}", | ||
| param_name, | ||
| field.name.to_case(convert_case::Case::Pascal) | ||
| ) | ||
| }) | ||
| .collect::<Vec<String>>() | ||
| .join(", ") | ||
| } | ||
| _ => param_name, | ||
| } | ||
| }) | ||
| .collect::<Vec<String>>(); | ||
|
|
||
| let calldata_str = if calldata.is_empty() { | ||
| "nil".to_string() | ||
| } else { | ||
| format!("[]any{{{}}}", calldata.join(", ")) | ||
| }; | ||
|
|
||
| format!( | ||
| "// {} executes the {} system | ||
| func (c *{}Contract) {}({}) error {{ | ||
| tx, err := c.account.Execute(ctx, []account.FunctionCall{{ | ||
| ContractAddress: c.address, | ||
| EntryPointSelector: \"{}\", | ||
| Calldata: {}, | ||
| }}, account.ExecuteOptions{{MaxFee: big.NewInt(0)}}) | ||
|
|
||
| if err != nil {{ | ||
| return fmt.Errorf(\"failed to execute %s: %w\", \"{}\", err) | ||
| }} | ||
|
|
||
| // Wait for transaction receipt | ||
| _, err = c.client.WaitForTransaction(ctx, tx.TransactionHash, 8*time.Second) | ||
| if err != nil {{ | ||
| return fmt.Errorf(\"transaction failed: %w\", err) | ||
| }} | ||
|
|
||
| return nil | ||
| }}", | ||
| method_name, | ||
| system.name, | ||
| contract_name, | ||
| method_name, | ||
| params_str, | ||
| system.name, | ||
| calldata_str, | ||
| system.name | ||
| ) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Contract method generation lacks proper error handling for type conversions.
Ohayo! The format_system_method function doesn't handle potential errors from map_type calls, which could panic if the type mapping changes to return Results.
Also, the hardcoded MaxFee: big.NewInt(0) might not be suitable for all use cases. Consider making this configurable.
🏁 Script executed:
#!/bin/bash
# Check if there are any other places in the codebase that set MaxFee
rg -A 2 "MaxFee.*big\.NewInt" Length of output: 252
#!/bin/bash
# Locate the map_type function and inspect its signature and body
rg -n "fn map_type" -n crates/dojo/bindgen/src/plugins/golang/mod.rs
rg -A5 "fn map_type" crates/dojo/bindgen/src/plugins/golang/mod.rsohayo sensei! Let’s inspect the map_type definition to confirm whether it can return an error or always yields a String.
#!/bin/bash
# Show the map_type implementation in GolangPlugin
rg -A20 "fn map_type" crates/dojo/bindgen/src/plugins/golang/mod.rsohayo sensei! Let’s verify whether map_type can fail or always returns a String:
#!/bin/bash
# Locate the signature and implementation of map_type in the Golang plugin
rg -n "fn map_type" crates/dojo/bindgen/src/plugins/golang/mod.rs
rg -A10 "fn map_type" crates/dojo/bindgen/src/plugins/golang/mod.rsohayo sensei! The first point can be dropped—GolangPlugin::map_type always returns a String and never yields a Result or panics—so no extra error handling is needed there. The MaxFee bit is still worth addressing:
• File: crates/dojo/bindgen/src/plugins/golang/mod.rs
Location: inside format_system_method, on the account.ExecuteOptions{MaxFee: big.NewInt(0)} line
Suggestion: expose a configurable default fee (e.g. via plugin config or an env var) instead of hard-coding zero.
🤖 Prompt for AI Agents
In crates/dojo/bindgen/src/plugins/golang/mod.rs between lines 330 and 418, the
MaxFee value in the ExecuteOptions struct is hardcoded to zero, which may not
suit all use cases. Modify the code to accept a configurable default fee, such
as passing it as a parameter to format_system_method or reading it from a plugin
configuration or environment variable, and use that value instead of
big.NewInt(0) to allow flexibility in transaction fee settings.
Description
Related issue
Tests
Added to documentation?
Checklist
scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit