feat: capped minter v2 minter role #4
Conversation
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
7483d1f to
f417601
Compare
marcomariscal
force-pushed
the
feat/capped-minter-v2
branch
from
d8da7d0 to
ca19636
Compare
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
bbaec92 to
31d626f
Compare
| /// @notice Error for when the cap is exceeded. | ||
| error ZkCappedMinterV2__CapExceeded(address minter, uint256 amount); | ||
|
|
||
| /// @notice Error for when the caller is not the admin. | ||
| /// @notice Error for when the account is unauthorized. | ||
| error ZkCappedMinterV2__Unauthorized(address account); |
There was a problem hiding this comment.
We can probably remove this error in favor of this one https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/IAccessControl.sol#L13
There was a problem hiding this comment.
sounds good; see this reply for more context:
| vm.prank(_cappedMinterAdmin); | ||
| cappedMinter.grantRole(MINTER_ROLE, _minter); |
There was a problem hiding this comment.
May be good to put this in a helper
| function _revertIfUnauthorized() internal view { | ||
| if (msg.sender != ADMIN) { | ||
| if (!hasRole(MINTER_ROLE, msg.sender)) { |
There was a problem hiding this comment.
We might want to use this instead. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/AccessControl.sol#L86
There was a problem hiding this comment.
Same in the upstream prs
There was a problem hiding this comment.
also removed the Unauthorized error since we are using _checkRole; this version of oz uses string reverts, so the test checks accordingly.
will update in the subsequent pr's.
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
725dc00 to
1295f88
Compare
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
89702f0 to
af16880
Compare
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
af16880 to
007784b
Compare
l2-contracts/foundry.toml
Outdated
| @@ -8,10 +9,10 @@ | |||
| "@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/", | |||
| "@openzeppelin/foundry-upgrades/=lib/openzeppelin-foundry-upgrades/src/", | |||
| "@murky/=lib/murky/", | |||
| "src/=src/", | |||
l2-contracts/foundry.toml
Outdated
| @@ -1,5 +1,6 @@ | |||
| [profile.default] | |||
| evm_version = "paris" | |||
| fs_permissions = [{ access = "read", path = "./zkout" }] | |||
There was a problem hiding this comment.
Also, why was this added?
marcomariscal
force-pushed
the
feat/capped-minter-v2-minter-role
branch
from
12e615f to
d0798f7
Compare
Description
Add
AccessControltoZkCappedMinterV2Kept all testing functionality the same as V1, but updated according to these changes. Also, included one more test for
testFuzz_RevertIf_AdminMintsByDefault