feat: use geth abi parsing and inherit gobindings for ops gen#953
feat: use geth abi parsing and inherit gobindings for ops gen#953
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR updates the EVM operations generator to rely on go-ethereum’s ABI parsing and to generate operations that leverage existing abigen gobindings (interfaces + metadata) rather than generating a custom BoundContract wrapper per contract.
Changes:
- Add
gobindings_packageto EVM contract YAML config and pass it through to templates for importing gobindings. - Switch ABI parsing to
github.com/ethereum/go-ethereum/accounts/abiand update function/parameter extraction accordingly (including overload handling). - Update the EVM operations template + golden fixtures to use gobindings metadata/interfaces and adjust generator tests.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/operations-gen/testdata/evm/operations_gen_mcms_config.yaml | Adds gobindings_package to MCMS test config fixture. |
| tools/operations-gen/testdata/evm/operations_gen_config.yaml | Adds gobindings_package to LinkToken test config fixture. |
| tools/operations-gen/testdata/evm/many_chain_multi_sig.golden.go | Updates expected generated output for MCMS to use gobindings (golden). |
| tools/operations-gen/testdata/evm/link_token.golden.go | Updates expected generated output for LinkToken to use gobindings (golden). |
| tools/operations-gen/templates/evm/operations.tmpl | Changes the generated code shape to reference gobindings metadata/interfaces and new op constructors. |
| tools/operations-gen/internal/families/evm/evm.go | Simplifies access-control constants used by the EVM generator. |
| tools/operations-gen/internal/families/evm/contract.go | Adds gobindings package to ContractInfo, switches to geth ABI parsing, updates function extraction. |
| tools/operations-gen/internal/families/evm/config.go | Adds required gobindings_package field to YAML config struct. |
| tools/operations-gen/internal/families/evm/codegen_test.go | Updates tests to avoid removed ABI-entry parsing helpers. |
| tools/operations-gen/internal/families/evm/codegen.go | Updates template data types and adds multi-return packing support. |
| tools/operations-gen/internal/families/evm/abi_test.go | Reworks ABI type mapping tests to use geth abi.Type. |
| tools/operations-gen/internal/families/evm/abi.go | Replaces string-based ABI parsing/type mapping with geth ABI parsing + new mapping logic. |
| tools/operations-gen/go.mod | Adds go-ethereum dependency (and tidies module). |
| tools/operations-gen/go.sum | Adds corresponding checksums for new dependencies. |
Comments suppressed due to low confidence (1)
tools/operations-gen/internal/families/evm/contract.go:154
EvmFunctionConfig.Accessis taggedomitempty(implying it’s optional), butextractFunctionscurrently treats an empty access value as an error (falls into thedefaultcase). This is a breaking behavior change for configs that omitaccess. Consider treating""the same aspublic(as the previous implementation did), or removeomitempty/ document it as required.
switch funcCfg.Access {
case accessOwner:
fi.AccessControl = accessOwner
case accessPublic:
default:
return fmt.Errorf("unknown access control '%s' for function %s (use 'owner' or 'public')",
funcCfg.Access, funcCfg.Name)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
graham-chainlink
left a comment
There was a problem hiding this comment.
Some of the copliot comments may be worth looking into.
Also we can also update the readme to include gobindings_package or we can do it at the end as well
| version: "1.0.0" | ||
| abi_file: "link_token.json" |
There was a problem hiding this comment.
we cant get rid of abi_file field yet right?
There was a problem hiding this comment.
Not yet, I'll remove it in next PR to source it directly from the bindings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ractkit/chainlink-deployments-framework into chore/ops-gen-geth-abi-parsing
|


Summary
This PR introduces the following changes
Type-safe ABI parsing via
go-ethereum/accounts/abiPrevious implementation parsed the ABI using encoding/json and a static
typeMapfor Solidity→Go type conversion. The map only covers explicitly listed sizes (uint96,uint128,uint256;bytes4,bytes16,bytes32; etc.); any type not in the map silently falls back to any.The code now relies on
github.com/ethereum/go-ethereum/accounts/abidirectly, which gives the same complete, size-aware type resolution that abigen uses. It also givesTupleRawName(the exact struct name abigen assigns to tuple types) and automatic overloaded-function disambiguation.No generated contract struct or interface
Previous implementation generated a
XxxContractstruct wrappingbind.BoundContract, with aNewXxxContractconstructor and delegating implementations of every selected method plusAddress(),Owner(), etc.Go bindings already generate
XxxInterfacewith all contract methods. So the new generator instead usesgeth_bindings.XxxInterfacedirectly as the contract type parameter inWriteParamsandReadParams. Essentially, generated operations now rely as much as possible on the pre existinggobindings.