Fix bug allowing anyone to cancel an admin renounce

[Amxx]

Changes introduced in #4230 include a bug allowing anyone to reset the schedule of an admin renounce:

• Alice (admin) starts a transfer to address(0)
• Delay passes
• Bob (not admin) does `renounceRole(0, bob)
  • if (role == DEFAULT_ADMIN_ROLE) passes
    • newDefaultAdmin == address(0) is true
    • schedule is good
      • delete _pendingDefaultAdminSchedule
  • super.renounceRole
    • caller is the account (bob)
    • caller doesn't have the role: nothing happens (no-op does not revert)

So Bob was able to delete _pendingDefaultAdminSchedule, which shouldn't have been possible

[changeset-bot]

⚠️ No Changeset found

Latest commit: 79269df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[frangio]

We should have an additional unit test for this scenario.

[ernestognw]

Small NIT:

Suggested change

	it('no op if renouncing when not having the role', async function () {
	it('no-ops if renouncing when not having the role', async function () {

I personally like to read the tests as it('<verb> ... (eg. it('does something ...'))

[ernestognw]

LGTM

[frangio]

Note that the test description was updated but is still not following Ernesto's suggestion. But IMO we should come back to this later, there are many tests throughout that are following a different format.

[ernestognw]

Note that the test description was updated but is still not following Ernesto's suggestion. But IMO we should come back to this later, there are many tests throughout that are following a different format.

Agree, it's a minor readability improvement but a suggestion as well, not mandatory. Will try to enforce this in my reviews but don't feel we need to update every test written the same way