-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Avoid uninitialized proxies #5906
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
base: master
Are you sure you want to change the base?
Conversation
|
Reviewer's GuideThis PR enforces explicit initialization of ERC1967 proxies by default, modifying the proxy constructor to revert on uninitialized deployments, updating the Sequence diagram for ERC1967Proxy constructor enforcing initializationsequenceDiagram
participant User
participant ERC1967Proxy
participant ERC1967Utils
User->>ERC1967Proxy: Deploy with implementation, _data
ERC1967Proxy->>ERC1967Utils: upgradeToAndCall(implementation, _data)
ERC1967Utils-->>ERC1967Proxy: returns called (bool)
alt _unsafeAllowUninitialized() is false and called is false
ERC1967Proxy-->>User: revert with ERC1967ProxyUninitialized
else
ERC1967Proxy-->>User: deployment succeeds
end
Class diagram for updated ERC1967Proxy and ERC1967UtilsclassDiagram
class ERC1967Proxy {
+constructor(address implementation, bytes _data)
+_implementation() internal view returns (address)
+_unsafeAllowUninitialized() internal pure virtual returns (bool)
}
ERC1967Proxy --|> Proxy
ERC1967Proxy ..> ERC1967Utils
class ERC1967Utils {
+error ERC1967NonPayable()
+error ERC1967ProxyUninitialized()
+upgradeToAndCall(address newImplementation, bytes data) internal returns (bool)
+getImplementation() internal view returns (address)
}
Class diagram for ClashingImplementation mock contractclassDiagram
class ClashingImplementation {
+initialize() external payable
+upgradeToAndCall(address, bytes calldata) external payable
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Fixes #????
PR Checklist
npx changeset add
)Summary by Sourcery
Enforce mandatory initialization during ERC1967 proxy deployment by default and update tests to reflect the new requirement.
New Features:
Enhancements:
Tests: