contracts-wizard icon indicating copy to clipboard operation
contracts-wizard copied to clipboard

Grant roles or owner using constructor arguments

Open ericglau opened this issue 3 years ago • 4 comments

When access control is used, the constructor currently grants roles such as:

        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(PAUSER_ROLE, msg.sender);

It would be better practice to set roles explicitly using constructor arguments instead.

ericglau avatar Dec 06 '22 15:12 ericglau

A question to ask is whether each role should have its own constructor argument, or if we add just one argument and reuse it for all roles. A third option could be to add one argument for the admin and a separate argument for all other roles.

If the address passed in is zero, the role shouldn't be granted.

frangio avatar Dec 06 '22 18:12 frangio

This should also apply for owner if using Ownable.

ericglau avatar Mar 08 '23 17:03 ericglau

This issue is important because if the contract is deployed using CREATE2 or via the "CREATE3" method, msg.sender's value is the address of the factory contract or a temporary proxy contract, instead of the expected account initiating the deployment. Then the factory contract ends up owning the deployed contract and receiving initially minted tokens (that then may be stuck forever).

SKYBITDev3 avatar Aug 23 '23 07:08 SKYBITDev3

tx.origin gives the expected value but Vitalik said it shouldn't be relied upon.

SKYBITDev3 avatar Aug 23 '23 07:08 SKYBITDev3

This is already done.

ericglau avatar Jan 28 '25 20:01 ericglau