Skip to content

Fix CronUpkeep so that storage layout matches delegate#304

Merged
infiloop2 merged 2 commits into
developfrom
devsvcs-2436/cron-upkeep-fix
Nov 26, 2025
Merged

Fix CronUpkeep so that storage layout matches delegate#304
infiloop2 merged 2 commits into
developfrom
devsvcs-2436/cron-upkeep-fix

Conversation

@infiloop2
Copy link
Copy Markdown
Contributor

Followup from #293 that i found in testing. Since cron upkeep also has a delegate contract, their storage layout needs to match, fixing that

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 26, 2025

Static analysis results are available

Hey @infiloop2, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

@infiloop2 infiloop2 marked this pull request as ready for review November 26, 2025 20:27
@infiloop2 infiloop2 requested a review from a team as a code owner November 26, 2025 20:27
Copilot AI review requested due to automatic review settings November 26, 2025 20:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR attempts to fix the storage layout alignment between CronUpkeep and CronUpkeepDelegate by moving the s_permissionedForwarder variable to the end of the storage layout in both contracts. However, the fix is incomplete and the storage layouts still do not match.

Key Changes:

  • Moved s_permissionedForwarder from its original position (after s_activeCronJobIDs) to after all mappings in CronUpkeep
  • Added s_permissionedForwarder to the end of storage in CronUpkeepDelegate

Critical Issue: The delegate is still missing several storage variables that exist in CronUpkeep:

  • s_pendingOwner (from ConfirmedOwner parent contract)
  • _paused (from Pausable parent contract)
  • s_handlerSignatures mapping

For delegatecall to work correctly, the storage layouts must match exactly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
contracts/src/v0.8/automation/upkeeps/CronUpkeepDelegate.sol Adds s_permissionedForwarder at the end, but storage layout remains incomplete
contracts/src/v0.8/automation/upkeeps/CronUpkeep.sol Moves s_permissionedForwarder to after all mappings for alignment with delegate

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread contracts/src/v0.8/automation/upkeeps/CronUpkeepDelegate.sol Outdated
FelixFan1992
FelixFan1992 previously approved these changes Nov 26, 2025
@infiloop2 infiloop2 enabled auto-merge (squash) November 26, 2025 20:57
@infiloop2 infiloop2 merged commit 31bd57b into develop Nov 26, 2025
55 of 57 checks passed
@infiloop2 infiloop2 deleted the devsvcs-2436/cron-upkeep-fix branch November 26, 2025 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants