-
Notifications
You must be signed in to change notification settings - Fork 50
Proxy contracts reset of the version and initializers ahead of the migration #2125
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ import "../proxy/Initializable.sol"; | |||||||||||||||||||||||||||||||||||||||||
| /// @title PolicyRegistry | ||||||||||||||||||||||||||||||||||||||||||
| /// @dev A contract to maintain a policy for each court. | ||||||||||||||||||||||||||||||||||||||||||
| contract PolicyRegistry is UUPSProxiable, Initializable { | ||||||||||||||||||||||||||||||||||||||||||
| string public constant override version = "0.8.0"; | ||||||||||||||||||||||||||||||||||||||||||
| string public constant override version = "2.0.0"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // ************************************* // | ||||||||||||||||||||||||||||||||||||||||||
| // * Events * // | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -47,14 +47,10 @@ contract PolicyRegistry is UUPSProxiable, Initializable { | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /// @dev Constructs the `PolicyRegistry` contract. | ||||||||||||||||||||||||||||||||||||||||||
| /// @param _owner The owner's address. | ||||||||||||||||||||||||||||||||||||||||||
| function initialize(address _owner) external reinitializer(1) { | ||||||||||||||||||||||||||||||||||||||||||
| function initialize(address _owner) external initializer { | ||||||||||||||||||||||||||||||||||||||||||
| owner = _owner; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+50
to
52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainGuard initialize() against zero owner and confirm initializer-vs-reinitializer migration plan.
Apply: function initialize(address _owner) external initializer {
- owner = _owner;
+ if (_owner == address(0)) revert ZeroAddress();
+ owner = _owner;
}Add the error: error OwnerOnly();
+error ZeroAddress();Optional: also validate in changeOwner: function changeOwner(address _owner) external onlyByOwner {
- owner = _owner;
+ if (_owner == address(0)) revert ZeroAddress();
+ owner = _owner;
}Run to sanity-check deploy/migration scripts: 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Any remaining initialize2/3/4 calls in deploy scripts?"
rg -n -C2 -g 'contracts/deploy/**' 'initialize[234]\s*\(' || true
echo "Places calling initialize(...) in deploy scripts:"
rg -n -C2 -g 'contracts/deploy/**' '\.initialize\s*\(' || true
echo "Contracts still using reinitializer():"
rg -n --type=sol 'reinitializer\s*\(' || trueLength of output: 540 Guard initialize and changeOwner against zero address & confirm initializer vs reinitializer migration
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function initialize2() external reinitializer(2) { | ||||||||||||||||||||||||||||||||||||||||||
| // NOP | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // ************************************* // | ||||||||||||||||||||||||||||||||||||||||||
| // * Governance * // | ||||||||||||||||||||||||||||||||||||||||||
| // ************************************* // | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ import "../../proxy/Initializable.sol"; | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// @title Evidence Module | ||||||||||||||||||||||||||
| contract EvidenceModule is IEvidence, Initializable, UUPSProxiable { | ||||||||||||||||||||||||||
| string public constant override version = "0.8.0"; | ||||||||||||||||||||||||||
| string public constant override version = "2.0.0"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // ************************************* // | ||||||||||||||||||||||||||
| // * Storage * // | ||||||||||||||||||||||||||
|
|
@@ -37,14 +37,10 @@ contract EvidenceModule is IEvidence, Initializable, UUPSProxiable { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// @dev Initializer. | ||||||||||||||||||||||||||
| /// @param _owner The owner's address. | ||||||||||||||||||||||||||
| function initialize(address _owner) external reinitializer(1) { | ||||||||||||||||||||||||||
| function initialize(address _owner) external initializer { | ||||||||||||||||||||||||||
| owner = _owner; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+40
to
42
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add zero-address guard in initialize(). function initialize(address _owner) external initializer {
- owner = _owner;
+ if (_owner == address(0)) revert ZeroAddress();
+ owner = _owner;
}And declare the error: error OwnerOnly();
+error ZeroAddress();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function initialize2() external reinitializer(2) { | ||||||||||||||||||||||||||
| // NOP | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // ************************ // | ||||||||||||||||||||||||||
| // * Governance * // | ||||||||||||||||||||||||||
| // ************************ // | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Initializer calls may not match target contract ABIs.
You changed four calls to
initializer: "reinitialize"(some with empty args). Ensure each target contract actually exposesfunction reinitialize(...) externalwith matching arity; several modules in this PR migrated to single-useinitializer, notreinitializerfunctions. Otherwise upgrades will revert atupgradeToAndCall.Run this audit to confirm ABI/usage alignment:
If any target lacks a matching
reinitialize, either:reinitializewrapper (preferred for migration), orinitializerandargshere per contract to invoke the actual available entry point.🏁 Script executed:
Length of output: 1171
🏁 Script executed:
Length of output: 49090
Replace
reinitializecalls with actual initializers and correct argsAt contracts/deploy/upgrade-all.ts (lines 101–106), the script invokes
reinitialize, but none of the target implementations define such a function. Either add lightweightreinitializewrappers or, more directly, call their existinginitialize(...)entry points with the proper parameters:initialize(owner)initialize(owner, core, maxDrawingTime, rng)initialize(owner, guardian, sortitionModuleAddress, wETH.address)🤖 Prompt for AI Agents