Functions is[ROLE] are not used inside BfxVault

[hats-bug-reporter[bot]]

Github username: -- Twitter username: 0xShitgem Submission hash (on-chain): 0x1e6b502bba0b8832c2d017bfdf70e8c1fe4b8a42b3f9b318e8b0c103123cc47d Severity: low

Description: Description\ Inside BfxVault.sol exist these functions:

function isAdmin(address user) public view returns (bool) {
        return signers[user][ADMIN_ROLE];
}

function isTrader(address user) public view returns (bool) {
        return signers[user][TRADER_ROLE];
}

function isTreasurer(address user) public view returns (bool) {
        return signers[user][TREASURER_ROLE];
}

They aren't used anywhere inside smart contract - they're even wrriten again in form of: require(signers[msg.sender][ROLE])

Examples:

Found in BfxVault [Line 205]

function addRole(address signer, uint256 role) public {
@>   require(signers[msg.sender][ADMIN_ROLE], "NOT_AN_ADMIN");
        signers[signer][role] = true;
        emit AddRole(signer, role);
}

Found in BfxVault [Line 222]

function removeRole(address signer, uint256 role) public {
@>   require(signers[msg.sender][ADMIN_ROLE], "NOT_AN_ADMIN");
        signers[signer][role] = false;
        emit RemoveRole(signer, role);
}

Found in BfxVault [Line 241]

function makeDeposit(uint256 amount) external {
@>   require(signers[msg.sender][TREASURER_ROLE], "NOT_A_TREASURER");
        _doDeposit(amount);
}

Recommendation

Consider making these functions as modifiers to have more clear code.

Example:

+    modifier isAdmin() {
+        require(signers[msg.sender][ADMIN_ROLE], "NOT_AN_ADMIN");
+        _;
+    }

-    function isAdmin(address user) public view returns (bool) {
-        return signers[user][ADMIN_ROLE];
-    }

Alternatively - just use those functions.

[alex-sumner]

This is not a bug. The functions are provided for convenience and are helpful, for example, when examining the contract from a block explorer.