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

Use default admin role in TimelockController #3799

Merged
merged 17 commits into from Nov 9, 2022

Conversation

JulissaDantes
Copy link
Contributor

@JulissaDantes JulissaDantes commented Nov 3, 2022

Fixes #3389

The TimelockController contract currently uses a custom non-zero TIMELOCK_ADMIN role, in order to make easier the introduction of new roles without custom migrations the DEFAULT_ADMIN_ROLE needs to be used.

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@socket-security
Copy link

socket-security bot commented Nov 3, 2022

Socket Security Pull Request Report

👍 No dependency changes detected in pull request

Pull request report summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

Powered by socket.dev

@Amxx
Copy link
Collaborator

Amxx commented Nov 3, 2022

That PR looks way bigger than it needs to be.

@frangio
Copy link
Contributor

frangio commented Nov 3, 2022

Seems like the PR includes everything from master. Those are things we want to merge into next-v5.0 because we need to keep it up to date but we should do it separately.

@JulissaDantes
Copy link
Contributor Author

Now it's ready!

@JulissaDantes JulissaDantes marked this pull request as ready for review November 4, 2022 18:24
@frangio
Copy link
Contributor

frangio commented Nov 4, 2022

@JulissaDantes Can you rebase on next-v5.0? Goal would be to remove these commits from the commit list:

image

@@ -28547,4 +28547,4 @@
"dev": true
}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want that ?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thank you!

@frangio frangio merged commit 8879854 into OpenZeppelin:next-v5.0 Nov 9, 2022
@gitpoap-bot
Copy link

gitpoap-bot bot commented Nov 9, 2022

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2022 OpenZeppelin Contracts Contributor:

GitPOAP: 2022 OpenZeppelin Contracts Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

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.

None yet

3 participants