Description

The BlackwingAaveDeployer contract has two functions, remove() and whitelistAsset(), both of which have duplicate role checking.

• remove checks for VAULT_ROLE with a modifier and a require

• whitelistAsset checks for OWNER_ROLE with a modifier and a require

function whitelistAsset(IERC20 asset, IPoolAddressesProvider pap) public onlyRole(OWNER_ROLE) {
  require(hasRole(OWNER_ROLE, msg.sender), UNAUTHORIZED_ERR);
  // ...
}

function remove(IERC20 asset, uint amount) public onlyRole(VAULT_ROLE) {
  require(hasRole(VAULT_ROLE, msg.sender), UNAUTHORIZED_ERR);
  requireAssetWhitelisted(asset);
  // ...
}

Impact

The redundant checks consume unnecessary gas and diminish the code's readability and overall maturity.

Recommendations

Consider removing the duplicate checks to ensure the role is only checked a single time.

Remediation

This issue has been acknowledged by Ferum Labs, and a fix was implemented in commit d7158213↗.

The team removed onlyRole(VAULT_ROLE) and used hasRole(VAULT_ROLE, msg.sender).