Issues with DEFAULT_ADMIN_ROLE and modifier not working

Support Contracts
, ,
1

I am very confused about why the following is happening. So I have a contract that is using openZeppelin's AccessControl contracts. I initalise my contratc deployer to be an admin in the constructor like so:

    // ========= CONSTRUCTOR =========

    constructor(bytes32 _rootHash)
    {
        //set the owner to be an admin
        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);

        //initialise variables
        updateWhitelistRootHash(_rootHash);
    }

I made a modifier to check if the caller of the function is an amin like so:

    modifier onlyAdmin()
    {
        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender),
            "You are not an admin"
        );
        _;
    }

Then I have a getter method that is protected with the modifier like so

   // ============ USERS ============

    function getUser(address _address)
        public
        view
        onlyAdmin
        returns (User memory)
    {
        return _whitelist[_address];
    }

I am logged in with the same account that deployed the contracts in etherscan, there I am verifying that I am indeed an admin because I can query my address with the hasRole method using my wallet's address and the bytes32 value of the DEFAULT_ADMIN_ROLE (0x0000000000000000000000000000000000000000000000000000000000000000), this method returns "true".

Now the problem is, when I try to call the getUser() method in etherscan, I get the warning of my modifier:
![](https://sepolia.etherscan.io/images/svg/shapes/shape-1.svg) *tuple* Error: execution reverted: You are not an admin { "originalError": { "code": 3, "data": "0x0xxxxxxx", "message": "execution reverted: You are not an admin" } }

Why is this? I am clearly an admin, but somehow the modifier's require is triggered? Please help me, Thnx

2
  1. How exactly do you make yourself an admin when you call a read-only function through the Read Functions tab on Etherscan? There's no way to do that as far as I'm aware of, because technically speaking, read-only functions don't need to care about who's calling them to begin with.
  2. Following the above, if you've decided to restrict access for a read-only function, then there is likely something wrong with your design. All the data id publicly available anyway, so there should be no reason (or motivation) in attempting to hide it from specific users. If you're relying on it for some security issue, then be aware that you are not addressing this issue correctly.
3

Ok I understand your reasoning. Let me explain how I came to this point. I actually have a modifier called

    address public controller;

    modifier onlyAdminOrController()
    {
        require((hasRole(DEFAULT_ADMIN_ROLE, msg.sender) || msg.sender == controller),
            "You are not an admin or controller"
        );
        _;
    }

I have another contract 'controller' who is calling the getUser() method from this contract. In order for that to happen the method getUser() should be public or external is it not? I thought i'd also add the possibility inspect/use that method as an admin in code or using etherscan... So in that scenario my modifier would make sense no? But then when I tried to use it on etherscan I came to the conclusion that the require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender) was not working.

As for your first point. On etherscan there is a 'connect web3' button. I would assume that once connected with the same address that also deployed the contract and then calling a read function with the onlyAdmin modifier, that it would work no? Or am I missing something.

Thank you for the quick reply.

4

Hmmmm reading up on this issue.

So are you saying that it is impossible to have a view (read only) function with restrictive modifiers such as onlyAdmin onlyOwner etc? So this then only makes sense in write functions?

Still new to solidity development but thank you for this new knowledge. I will adapt my smartcontracts now with this new info.

5

Yes

IMHO, as a general concept, it doesn't make sense to restrict the permission on a Read function.

I would tend to guess that etherscan uses the connected wallet only for Write functions.

There's no way to know for sure without asking them directly I suppose, but the error which you're getting implies that etherscan doesn't use the connected wallet for Read functions.

Side note:

You can grant DEFAULT_ADMIN_ROLE to the controller contract, then get rid of the || msg.sender == controller part in your modifier. Just make sure that it doesn't contradict any other system requirement and/or design consideration.

6

Thank you so much for the insight!
And yes it does not pose any security risks... it's not like I want to prevent anyone from querying their address to see if they are an accepted user or something. I guess I can make those simply public view functions.

For a beginner it is difficult/confusing not to know that etherscan does not use the connected account for read functions. But I guess it makes sense.

Also thank you for the tip on simply granting DEFAULT_ADMIN_ROLE to controller.
One last question concerning that:

right now I am setting the controller at a later stage (since it is not yet deployed before this contract itself is deployed). I have a setter method like this:

    function updateController(address _controller)
        public
        onlyAdmin
    {
        controller = _controller;
    }

after the controller = _controller line here.. should I use
grantRole(DEFAULT_ADMIN_ROLE, controller);
or
_setupRole(DEFAULT_ADMIN_ROLE, controller);

7

If you look into the AccessControl contract, then you'll see that they are doing the exact same thing:

function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
    _grantRole(role, account);
}

function _setupRole(bytes32 role, address account) internal virtual {
    _grantRole(role, account);
}

But function grantRole is designated for external usage (by other contracts or accounts).

And function _setupRole is designated for internal usage (i.e., inside the contract).

So it makes more sense for you to use the latter.

8

Ok I Got it!

Thank you very very much.

1 Like
9

You could actually easily test this by defining a getter function that returns msg.sender.

I would bet that the issue is that Etherscan is not using the connected wallet for read functions.

10

Ah yeah, that one didn't even occur to me.
I guess I'm so "not used" to thinking about a Read function as something that you need an account to call it with. Even in standard contract testing, I don't recall ever trying that.

Thinking about it again, in order to call Read functions on etherscan, you don't even need to connect your wallet to begin with. So it is obviously not your wallet being used for that before you've connected it, and we can therefore assume that it is not your wallet being used for that after you've connected it.

11

It's just really confusing for beginners, they could easily just remove the connect-web-3 button from the read tab..