OpenZeppelin / OpenZeppelin/openzeppelin-contracts
Consider making `_hasSchedulePassed` internal virtual in AccessControlDefaultAdminRules
Nobody has claimed this yet.
- Dominant language
- Solidity
- Stars
- 27.2k
- Forks
- 12.4k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 33
Description
In AccessControlDefaultAdminRules.sol, you can't execute on inclusion block + scheduled delay but rather, inclusion block + scheduled delay + 1. This is a rather strange pattern since time limits are usually enforced as block.timestamp >= expectedTimestamp. I presume this is intended to induce a delay when schedule timestamp == block.timestamp but has been left undocumented.
💻 Environment
v5.3.0 OZ
v1 Foundry
📝 Details
I think this should be documented or fixed. I don't strictly view it as a bug but it's a definite pitfall.
🔢 Code to reproduce bug
multicall([...], [...])beginDefaultAdminTransfer(...)acceptDefaultAdminTransfer()
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in contracts/access/extensions/AccessControlDefaultAdminRules.sol around _hasSchedulePassed at line 369, then reproduce the boundary case with beginDefaultAdminTransfer(...) and acceptDefaultAdminTransfer() in one multicall. Compare the observed inclusion-block timing with the intended behavior; done means the behavior is corrected or its timing is explicitly documented, as decided in the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- authorization, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100