Conversation
|
This change is |
PR SummaryLow Risk Overview
Adds the Reviewed by Cursor Bugbot for commit ee13462. Bugbot is set up for automated code reviews on this repo. Configure here. |
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
|
Security scan complete — no issues detected. Generated by Claude Code |
Summary
The internal bindings generated by
deploy_for_test(class_hash,deployment_params,calldata) are renamed to__deploy_class_hash__,__deploy_params__, and__deploy_calldata__to avoid collisions with constructor parameter names that happen to share those identifiers.Type of change
Please check one:
Why is this change needed?
When a contract's constructor declared parameters named
class_hash,deployment_params, orcalldata, the generateddeploy_for_testfunction would either fail to compile with a parameter redefinition error (E2054) or silently drop the user's argument (e.g. thecalldataarray was overwritten by the internally declaredlet mut calldatabinding, causing serialization to operate on an empty array instead of the user-supplied one).What was the behavior or documentation before?
A constructor with parameters named
class_hash,deployment_params, orcalldatacaused the generateddeploy_for_testto either produce a compile error due to duplicate parameter names, or silently ignore the user-suppliedcalldataargument because the internallet mut calldatabinding shadowed it.What is the behavior or documentation after?
The generated
deploy_for_testuses mangled names (__deploy_class_hash__,__deploy_params__,__deploy_calldata__) for its internal bindings, so constructor parameters with any of those names are passed through correctly without collision or shadowing. A new test contract (reserved_ctor_arg_names) and accompanying test (test_deploy_for_test_reserved_ctor_arg_names) verify this behavior end-to-end.Related issue or discussion (if any)
Additional context
The fix is purely in the code generation template inside
generate_deploy_function. The double-underscore prefix/suffix convention is chosen to minimize the chance of a real constructor parameter ever colliding with the generated names.