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.

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

feat(protocol): revert revert update open-zeppelin contracts (#15896) #15914

Closed
wants to merge 2 commits into from

Conversation

dionysuzx
Copy link
Collaborator

This reverts commit 994e29e.

@dionysuzx dionysuzx changed the title feat(protocol): revert revert update open-zeppelin contracts (#15896)" feat(protocol): revert revert update open-zeppelin contracts (#15896) Feb 19, 2024
@dionysuzx dionysuzx changed the title feat(protocol): revert revert update open-zeppelin contracts (#15896) feat(protocol): revert revert update open-zeppelin contracts (#15896) DO NOT MERGE Feb 19, 2024
@dantaik
Copy link
Contributor

dantaik commented Feb 19, 2024

The new version doesn't work?

@dionysuzx
Copy link
Collaborator Author

The new version doesn't work?

yea i had to cancel the build. it's stuck in an infinite loop, see:
image

@adaki2004
Copy link
Contributor

adaki2004 commented Feb 19, 2024

The new version doesn't work?
This PR aims to set back the "new" one.

So the problem was (in chronology):

  • Dave suggested to use a newer one (4.9.5)
  • We merged it (but has issues with DeployOnL1 workflow in CICD)
  • We reverted it so that it does not cause any issues on main
  • We eventually need this so it is a 2nd attempt to add it back and debug first that DeployOnL1 thing

@dionysuzx dionysuzx changed the title feat(protocol): revert revert update open-zeppelin contracts (#15896) DO NOT MERGE feat(protocol): revert revert update open-zeppelin contracts (#15896) Feb 19, 2024
@dionysuzx dionysuzx marked this pull request as draft February 19, 2024 04:27
@dantaik
Copy link
Contributor

dantaik commented Feb 19, 2024

@d1onys1us I think we are OK not to proceed with this upgrade.

@dionysuzx
Copy link
Collaborator Author

@d1onys1us I think we are OK not to proceed with this upgrade.

GHSA-93hq-5wgc-jc82

i think we do need to update so long as we are using GovernorCompatibilityBravo, but we can do 4.8.3, 4.9.3, or 5.0.1, but we should patch the security issue IMO.

@dantaik
Copy link
Contributor

dantaik commented Feb 20, 2024

Suggest to close this PR given #15947

@dionysuzx dionysuzx closed this Feb 20, 2024
@dionysuzx dionysuzx deleted the revert-again branch February 20, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants