Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add OPCM upgrade template #599

Closed
wants to merge 21 commits into from
Closed

Add OPCM upgrade template #599

wants to merge 21 commits into from
+409 −85

Conversation

prat-gpt
Copy link
Contributor

Description

  • Extend Multisig task and create opcm base task
  • Add opcm upgrade template
  • template uses a dummy OPCM contract on sepolia

Tests

  • Add test to check the complete flow: data signing by child multisig EOAs, hash approval by child multisig, final execution by parent multisig.

Metadata
Related issue: #571

@prat-gpt prat-gpt marked this pull request as ready for review February 13, 2025 20:12
@prat-gpt prat-gpt requested review from a team as code owners February 13, 2025 20:12
@prat-gpt prat-gpt requested a review from maurelian February 13, 2025 20:12
@blmalone blmalone requested review from mds1 and blmalone February 13, 2025 20:21
blmalone
blmalone reviewed Feb 13, 2025
View reviewed changes
Copy link
Contributor

@blmalone blmalone left a comment

Choose a reason for hiding this comment

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

still need to go through the rest of this.

ElliotFriedman
ElliotFriedman reviewed Feb 14, 2025
View reviewed changes
src/improvements/tasks/MultisigTask.sol Outdated
/// get the minimum depth of the calls, we only care about the top level calls
/// this is to avoid counting subcalls as actions
/// cannot set to constant as in tests, the depth will be constant + 1
/// for the same account access
Copy link
Contributor

Choose a reason for hiding this comment

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

give a concrete example of minDepth in tests vs running script depth

Copy link
Contributor Author

@prat-gpt prat-gpt Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think there is no need of this after the logic update.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this okay to be resolved @ElliotFriedman - I'm working through open comments.

@prat-gpt prat-gpt mentioned this pull request Feb 14, 2025
Open
@blmalone blmalone mentioned this pull request Feb 14, 2025
Merged
@blmalone blmalone mentioned this pull request Feb 14, 2025
Closed
@blmalone
Copy link
Contributor

Closing as #606 is finishing out PR comments. All commits have been cherry picked on new branch.

@blmalone blmalone closed this Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blmalone blmalone blmalone left review comments

@mds1 mds1 mds1 left review comments

@ElliotFriedman ElliotFriedman ElliotFriedman left review comments

@maurelian maurelian Awaiting requested review from maurelian maurelian is a code owner automatically assigned from ethereum-optimism/contract-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@prat-gpt @blmalone @mds1 @ElliotFriedman