Feat/shared script utilities #50
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- DeployUtils: shared _SWAP_FACILITY constant - ChainsBase: shared ETHEREUM/ETHEREUM_SEPOLIA chain IDs and isHub() - TimelockBatchBase: timelock batch transaction utility Consolidates duplicated script constants and utilities from m-portal, m-portal-lite, and evm-m-extensions.
There was a problem hiding this comment.
Pull request overview
This PR consolidates shared script utilities to be reused across multiple M0 protocol repositories. It introduces base contracts and libraries for common deployment, timelock batching, and chain identification functionality, along with dependency updates.
Changes:
- Adds three new utility contracts:
TimelockBatchBasefor timelock batch operations,DeployUtilsfor shared deployment constants, andChainsBasefor EVM chain ID management - Updates submodule dependencies for
forge-std,safe-utils, and adds newopenzeppelin-contractssubmodule
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| script/TimelockBatchBase.sol | Implements base contract for building and simulating timelock batch operations |
| script/DeployUtils.sol | Defines shared deployment constant for swap facility address |
| script/ChainsBase.sol | Provides library with common chain IDs and hub chain identification |
| lib/safe-utils | Updates safe-utils submodule to newer commit |
| lib/openzeppelin-contracts | Adds openzeppelin-contracts as new submodule dependency |
| lib/forge-std | Updates forge-std submodule to newer commit |
| .gitmodules | Registers openzeppelin-contracts submodule configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+43
| (bool success, ) = _timelockTargets[i].call{ value: _timelockValues[i] }(_timelockPayloads[i]); | ||
| require(success, "Simulation failed"); |
There was a problem hiding this comment.
The error message 'Simulation failed' is too generic and doesn't provide information about which call failed or why. Consider including the target address and index in the error message to aid debugging.
Comment on lines
+42
to
+43
| (bool success, ) = _timelockTargets[i].call{ value: _timelockValues[i] }(_timelockPayloads[i]); | ||
| require(success, "Simulation failed"); |
There was a problem hiding this comment.
The low-level call does not capture or handle the return data when the call fails. Consider capturing the return data to provide more context about failures, especially since this is a simulation function meant for testing.
Suggested change
| (bool success, ) = _timelockTargets[i].call{ value: _timelockValues[i] }(_timelockPayloads[i]); | |
| require(success, "Simulation failed"); | |
| (bool success, bytes memory returndata) = | |
| _timelockTargets[i].call{ value: _timelockValues[i] }(_timelockPayloads[i]); | |
| if (!success) { | |
| if (returndata.length > 0) { | |
| // Bubble up revert reason from the called contract | |
| assembly { | |
| revert(add(returndata, 32), mload(returndata)) | |
| } | |
| } else { | |
| revert("Simulation failed"); | |
| } | |
| } |
CI workflows set FOUNDRY_PROFILE=ci but the profile was not defined, causing build and coverage jobs to fail.
Foundry's auto-discovered remapping pointed to the safe-smart-account repo root, but sources live under contracts/. This caused CI build failures for safe-utils imports
Changes to gas cost
🧾 Summary (20% most significant diffs)
Full diff report 👇
|
LCOV of commit
|
JPMora89
approved these changes
Feb 10, 2026
JPMora89
left a comment
JPMora89
left a comment
There was a problem hiding this comment.
LGTM, error message suggested by copilot not relevant as we would have other layers give us necessary info.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Base changes to consolidate shared script utilities across other repos.